| Message ID | 35c01a524b2eb6c3f01bc08f16bdff2d72256a1f.1693188390.git.andreyknvl@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] usb: gadget: clarify usage of USB_GADGET_DELAYED_STATUS | expand |
On Mon, Aug 28, 2023 at 04:10:32AM +0200, andrey.konovalov@linux.dev wrote: > From: Andrey Konovalov <andreyknvl@gmail.com> > > Return USB_GADGET_DELAYED_STATUS from the setup() callback for 0-length > transfers as a workaround to stop some UDC drivers (e.g. dwc3) from > automatically proceeding with the status stage. > > This workaround should be removed once all UDC drivers are fixed to > always delay the status stage until a response is queued to EP0. > > Signed-off-by: Andrey Konovalov <andreyknvl@gmail.com> > --- > drivers/usb/gadget/legacy/inode.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c > index 28249d0bf062..154bbf578ba2 100644 > --- a/drivers/usb/gadget/legacy/inode.c > +++ b/drivers/usb/gadget/legacy/inode.c > @@ -31,6 +31,7 @@ > > #include <linux/usb/gadgetfs.h> > #include <linux/usb/gadget.h> > +#include <linux/usb/composite.h> Add: /* for USB_GADGET_DELAYED_STATUS */ > > > /* > @@ -241,6 +242,7 @@ static DEFINE_MUTEX(sb_mutex); /* Serialize superblock operations */ > #define xprintk(d,level,fmt,args...) \ > printk(level "%s: " fmt , shortname , ## args) > > +#undef DBG > #ifdef DEBUG > #define DBG(dev,fmt,args...) \ > xprintk(dev , KERN_DEBUG , fmt , ## args) > @@ -256,8 +258,10 @@ static DEFINE_MUTEX(sb_mutex); /* Serialize superblock operations */ > do { } while (0) > #endif /* DEBUG */ > > +#undef ERROR > #define ERROR(dev,fmt,args...) \ > xprintk(dev , KERN_ERR , fmt , ## args) > +#undef INFO Please move these #undef lines up, just after the new #include. And add a comment explaining briefly why they are needed. Aside from these changes, Reviewed-by: Alan Stern <stern@rowland.harvard.edu>
On Mon, Aug 28, 2023 at 4:52 PM Alan Stern <stern@rowland.harvard.edu> wrote: > > On Mon, Aug 28, 2023 at 04:10:32AM +0200, andrey.konovalov@linux.dev wrote: > > From: Andrey Konovalov <andreyknvl@gmail.com> > > > > Return USB_GADGET_DELAYED_STATUS from the setup() callback for 0-length > > transfers as a workaround to stop some UDC drivers (e.g. dwc3) from > > automatically proceeding with the status stage. > > > > This workaround should be removed once all UDC drivers are fixed to > > always delay the status stage until a response is queued to EP0. > > > > Signed-off-by: Andrey Konovalov <andreyknvl@gmail.com> > > --- > > drivers/usb/gadget/legacy/inode.c | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c > > index 28249d0bf062..154bbf578ba2 100644 > > --- a/drivers/usb/gadget/legacy/inode.c > > +++ b/drivers/usb/gadget/legacy/inode.c > > @@ -31,6 +31,7 @@ > > > > #include <linux/usb/gadgetfs.h> > > #include <linux/usb/gadget.h> > > +#include <linux/usb/composite.h> > > Add: /* for USB_GADGET_DELAYED_STATUS */ Will do in v2. > > +#undef ERROR > > #define ERROR(dev,fmt,args...) \ > > xprintk(dev , KERN_ERR , fmt , ## args) > > +#undef INFO > > Please move these #undef lines up, just after the new #include. And > add a comment explaining briefly why they are needed. Will do in v2. > > Aside from these changes, > > Reviewed-by: Alan Stern <stern@rowland.harvard.edu> Thank you, Alan!
diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c index 28249d0bf062..154bbf578ba2 100644 --- a/drivers/usb/gadget/legacy/inode.c +++ b/drivers/usb/gadget/legacy/inode.c @@ -31,6 +31,7 @@ #include <linux/usb/gadgetfs.h> #include <linux/usb/gadget.h> +#include <linux/usb/composite.h> /* @@ -241,6 +242,7 @@ static DEFINE_MUTEX(sb_mutex); /* Serialize superblock operations */ #define xprintk(d,level,fmt,args...) \ printk(level "%s: " fmt , shortname , ## args) +#undef DBG #ifdef DEBUG #define DBG(dev,fmt,args...) \ xprintk(dev , KERN_DEBUG , fmt , ## args) @@ -256,8 +258,10 @@ static DEFINE_MUTEX(sb_mutex); /* Serialize superblock operations */ do { } while (0) #endif /* DEBUG */ +#undef ERROR #define ERROR(dev,fmt,args...) \ xprintk(dev , KERN_ERR , fmt , ## args) +#undef INFO #define INFO(dev,fmt,args...) \ xprintk(dev , KERN_INFO , fmt , ## args) @@ -1511,7 +1515,16 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl) event->u.setup = *ctrl; ep0_readable (dev); spin_unlock (&dev->lock); - return 0; + /* + * Return USB_GADGET_DELAYED_STATUS as a workaround to + * stop some UDC drivers (e.g. dwc3) from automatically + * proceeding with the status stage for 0-length + * transfers. + * Should be removed once all UDC drivers are fixed to + * always delay the status stage until a response is + * queued to EP0. + */ + return w_length == 0 ? USB_GADGET_DELAYED_STATUS : 0; } }