| Message ID | 20180702194808.157636-1-zhangjerry@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Felipe, Can we get this patch into 4.19-rc6? The patch is simple and well understood and solves a real problem -- functionfs control requests will not work without it. Thanks, Jerry On Mon, Jul 2, 2018 at 12:48 PM Jerry Zhang <zhangjerry@google.com> wrote: > > Commit 1b9ba000 ("Allow function drivers to pause control > transfers") states that USB_GADGET_DELAYED_STATUS is only > supported if data phase is 0 bytes. > > It seems that when the length is not 0 bytes, there is no > need to explicitly delay the data stage since the transfer > is not completed until the user responds. However, when the > length is 0, there is no data stage and the transfer is > finished once setup() returns, hence there is a need to > explicitly delay completion. > > This manifests as the following bugs: > > Prior to 946ef68ad4e4 ('Let setup() return > USB_GADGET_DELAYED_STATUS'), when setup is 0 bytes, ffs > would require user to queue a 0 byte request in order to > clear setup state. However, that 0 byte request was actually > not needed and would hang and cause errors in other setup > requests. > > After the above commit, 0 byte setups work since the gadget > now accepts empty queues to ep0 to clear the delay, but all > other setups hang. > > Fixes: 946ef68ad4e4 ("Let setup() return USB_GADGET_DELAYED_STATUS") > Signed-off-by: Jerry Zhang <zhangjerry@google.com> > --- > > This patch used to be in a set, but is being submitted > standalone for the next 4.18 rc. > > drivers/usb/gadget/function/f_fs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c > index 4e47e8c58067..a6868f936c78 100644 > --- a/drivers/usb/gadget/function/f_fs.c > +++ b/drivers/usb/gadget/function/f_fs.c > @@ -3281,7 +3281,7 @@ static int ffs_func_setup(struct usb_function *f, > __ffs_event_add(ffs, FUNCTIONFS_SETUP); > spin_unlock_irqrestore(&ffs->ev.waitq.lock, flags); > > - return USB_GADGET_DELAYED_STATUS; > + return creq->wLength == 0 ? USB_GADGET_DELAYED_STATUS : 0; > } > > static bool ffs_func_req_match(struct usb_function *f, > -- > 2.18.0.399.gad0ab374a1-goog > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, Jerry Zhang <zhangjerry@google.com> writes: > Commit 1b9ba000 ("Allow function drivers to pause control > transfers") states that USB_GADGET_DELAYED_STATUS is only > supported if data phase is 0 bytes. > > It seems that when the length is not 0 bytes, there is no > need to explicitly delay the data stage since the transfer > is not completed until the user responds. However, when the > length is 0, there is no data stage and the transfer is > finished once setup() returns, hence there is a need to > explicitly delay completion. > > This manifests as the following bugs: > > Prior to 946ef68ad4e4 ('Let setup() return > USB_GADGET_DELAYED_STATUS'), when setup is 0 bytes, ffs > would require user to queue a 0 byte request in order to > clear setup state. However, that 0 byte request was actually > not needed and would hang and cause errors in other setup > requests. > > After the above commit, 0 byte setups work since the gadget > now accepts empty queues to ep0 to clear the delay, but all > other setups hang. > > Fixes: 946ef68ad4e4 ("Let setup() return USB_GADGET_DELAYED_STATUS") > Signed-off-by: Jerry Zhang <zhangjerry@google.com> Greg, can you pick this one manually? Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com>
On Thu, Jul 19, 2018 at 08:48:32AM +0300, Felipe Balbi wrote: > > Hi, > > Jerry Zhang <zhangjerry@google.com> writes: > > Commit 1b9ba000 ("Allow function drivers to pause control > > transfers") states that USB_GADGET_DELAYED_STATUS is only > > supported if data phase is 0 bytes. > > > > It seems that when the length is not 0 bytes, there is no > > need to explicitly delay the data stage since the transfer > > is not completed until the user responds. However, when the > > length is 0, there is no data stage and the transfer is > > finished once setup() returns, hence there is a need to > > explicitly delay completion. > > > > This manifests as the following bugs: > > > > Prior to 946ef68ad4e4 ('Let setup() return > > USB_GADGET_DELAYED_STATUS'), when setup is 0 bytes, ffs > > would require user to queue a 0 byte request in order to > > clear setup state. However, that 0 byte request was actually > > not needed and would hang and cause errors in other setup > > requests. > > > > After the above commit, 0 byte setups work since the gadget > > now accepts empty queues to ep0 to clear the delay, but all > > other setups hang. > > > > Fixes: 946ef68ad4e4 ("Let setup() return USB_GADGET_DELAYED_STATUS") > > Signed-off-by: Jerry Zhang <zhangjerry@google.com> > > Greg, can you pick this one manually? > > Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com> Yes, I'll pick it up, thanks. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 4e47e8c58067..a6868f936c78 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -3281,7 +3281,7 @@ static int ffs_func_setup(struct usb_function *f, __ffs_event_add(ffs, FUNCTIONFS_SETUP); spin_unlock_irqrestore(&ffs->ev.waitq.lock, flags); - return USB_GADGET_DELAYED_STATUS; + return creq->wLength == 0 ? USB_GADGET_DELAYED_STATUS : 0; } static bool ffs_func_req_match(struct usb_function *f,
Commit 1b9ba000 ("Allow function drivers to pause control transfers") states that USB_GADGET_DELAYED_STATUS is only supported if data phase is 0 bytes. It seems that when the length is not 0 bytes, there is no need to explicitly delay the data stage since the transfer is not completed until the user responds. However, when the length is 0, there is no data stage and the transfer is finished once setup() returns, hence there is a need to explicitly delay completion. This manifests as the following bugs: Prior to 946ef68ad4e4 ('Let setup() return USB_GADGET_DELAYED_STATUS'), when setup is 0 bytes, ffs would require user to queue a 0 byte request in order to clear setup state. However, that 0 byte request was actually not needed and would hang and cause errors in other setup requests. After the above commit, 0 byte setups work since the gadget now accepts empty queues to ep0 to clear the delay, but all other setups hang. Fixes: 946ef68ad4e4 ("Let setup() return USB_GADGET_DELAYED_STATUS") Signed-off-by: Jerry Zhang <zhangjerry@google.com> --- This patch used to be in a set, but is being submitted standalone for the next 4.18 rc. drivers/usb/gadget/function/f_fs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)