Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return other errno in raw_ioctl_ep_enable() #23

Closed
AristoChen opened this issue Mar 4, 2022 · 3 comments
Closed

Return other errno in raw_ioctl_ep_enable() #23

AristoChen opened this issue Mar 4, 2022 · 3 comments

Comments

@AristoChen
Copy link
Contributor

Hi,

Orange Pi PC have endpoints from ep1-ep4 for both in and out direction, and if we try to use raw_ioctl_ep_enable() for ep5in, it will return -EBUSY here

ret = -EBUSY;

I guess it will be more intuitive if we return some other errno(such as -EINVAL)? Cuz -EBUSY sounds like EP5in is not available now, but might be available in the future

Here is the patch

diff --git a/raw_gadget/raw_gadget.c b/raw_gadget/raw_gadget.c
index 1378d6c..d5acc67 100644
--- a/raw_gadget/raw_gadget.c
+++ b/raw_gadget/raw_gadget.c
@@ -758,6 +758,7 @@ static int raw_ioctl_ep_enable(struct raw_dev *dev, unsigned long value)
        unsigned long flags;
        struct usb_endpoint_descriptor *desc;
        struct raw_ep *ep;
+       bool ep_num_matched = false;

        desc = memdup_user((void __user *)value, sizeof(*desc));
        if (IS_ERR(desc))
@@ -792,6 +793,7 @@ static int raw_ioctl_ep_enable(struct raw_dev *dev, unsigned long value)
                if (ep->addr != usb_endpoint_num(desc) &&
                                ep->addr != USB_RAW_EP_ADDR_ANY)
                        continue;
+               ep_num_matched = true;
                if (!usb_gadget_ep_match_desc(dev->gadget, ep->ep, desc, NULL))
                        continue;
                ep->ep->desc = desc;
@@ -815,6 +817,12 @@ static int raw_ioctl_ep_enable(struct raw_dev *dev, unsigned long value)
                goto out_unlock;
        }

+       if (!ep_num_matched) {
+               dev_dbg(&dev->gadget->dev, "fail, no proper ep address available\n");
+               ret = -EINVAL;
+               goto out_free;
+       }
+
        dev_dbg(&dev->gadget->dev, "fail, no gadget endpoints available\n");
        ret = -EBUSY;
@xairy
Copy link
Owner

xairy commented Mar 7, 2022

Returning EINVAL makes more sense indeed. However, this is technically a change to the interface, and USB maintainers might reject it. But feel free to send the patch and I'll ack it. Thanks!

@AristoChen
Copy link
Contributor Author

Hi,

I just sent a patch, do I need to send a PR to this repo?

thanks!

@xairy
Copy link
Owner

xairy commented Mar 10, 2022

Let's wait until the patch is at least in the USB tree, and then I'll pull it in with a script.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants