xairy / raw-gadget

USB Raw Gadget — a low-level interface for the Linux USB Gadget subsystem
350 stars 37 forks source link

Return other errno in raw_ioctl_ep_enable() #23

Closed AristoChen closed 2 years ago

AristoChen commented 2 years ago

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

https://github.com/xairy/raw-gadget/blob/f74b1c22d3698892ba13c56f8b2273d1eba762e2/raw_gadget/raw_gadget.c#L819

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 commented 2 years ago

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 commented 2 years ago

Hi,

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

thanks!

xairy commented 2 years ago

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