diff mbox

[1/3] usb: gadget: f_fs: Only return delayed status when len is 0

Message ID 20180417011742.192184-2-zhangjerry@google.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jerry Zhang April 17, 2018, 1:17 a.m. UTC
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>
---
 drivers/usb/gadget/function/f_fs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jerry Zhang June 4, 2018, 7:44 p.m. UTC | #1
Hi Felipe,

I noticed this wasn't queued up for 4.18. Do you think there is
anything I need to do to get this patch set into 4.19? Also, can we at
least add just this patch ('usb: gadget: f_fs: Only return delayed
status when len is 0') to 4.18 as functionfs control requests won't
work without it?

Thanks,

Jerry
On Mon, Apr 16, 2018 at 6:18 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>
> ---
>  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 7d5b1c448eb2..4b2cb9d93176 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -3271,7 +3271,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.17.0.484.g0c8726318c-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
Felipe Balbi July 26, 2018, 10:45 a.m. UTC | #2
Hi,

Jerry Zhang <zhangjerry@google.com> writes:
> Hi Felipe,
>
> I noticed this wasn't queued up for 4.18. Do you think there is
> anything I need to do to get this patch set into 4.19? Also, can we at
> least add just this patch ('usb: gadget: f_fs: Only return delayed
> status when len is 0') to 4.18 as functionfs control requests won't
> work without it?

patch 1 is now queued for next merge window. What should I do with
patches 2 and 3?
diff mbox

Patch

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 7d5b1c448eb2..4b2cb9d93176 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -3271,7 +3271,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,