diff mbox series

[v2] usb: udc: remove warning when queue disabled ep

Message ID 20240315013019.2711135-1-yuanlinyu@hihonor.com
State Superseded
Headers show
Series [v2] usb: udc: remove warning when queue disabled ep | expand

Commit Message

yuan linyu March 15, 2024, 1:30 a.m. UTC
It is possible trigger below warning message from mass storage function,

WARNING: CPU: 6 PID: 3839 at drivers/usb/gadget/udc/core.c:294 usb_ep_queue+0x7c/0x104
CPU: 6 PID: 3839 Comm: file-storage Tainted: G S      WC O       6.1.25-android14-11-g354e2a7e7cd9 #1
pstate: 22400005 (nzCv daif +PAN -UAO +TCO -DIT -SSBS BTYPE=--)
pc : usb_ep_queue+0x7c/0x104
lr : fsg_main_thread+0x494/0x1b3c

Root cause is mass storage function try to queue request from main thread,
but other thread may already disable ep when function disable.

As there is no function failure in the driver, in order to avoid effort
to fix warning, change WARN_ON_ONCE() in usb_ep_queue() to pr_debug().

Suggested-by: Alan Stern <stern@rowland.harvard.edu>
Cc: <stable@vger.kernel.org>
Signed-off-by: yuan linyu <yuanlinyu@hihonor.com>
---
v2: change WARN_ON_ONCE() in usb_ep_queue() to pr_debug()
v1: https://lore.kernel.org/linux-usb/20240314065949.2627778-1-yuanlinyu@hihonor.com/

 drivers/usb/gadget/udc/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Alan Stern March 15, 2024, 1:41 a.m. UTC | #1
On Fri, Mar 15, 2024 at 09:30:19AM +0800, yuan linyu wrote:
> It is possible trigger below warning message from mass storage function,
> 
> WARNING: CPU: 6 PID: 3839 at drivers/usb/gadget/udc/core.c:294 usb_ep_queue+0x7c/0x104
> CPU: 6 PID: 3839 Comm: file-storage Tainted: G S      WC O       6.1.25-android14-11-g354e2a7e7cd9 #1
> pstate: 22400005 (nzCv daif +PAN -UAO +TCO -DIT -SSBS BTYPE=--)
> pc : usb_ep_queue+0x7c/0x104
> lr : fsg_main_thread+0x494/0x1b3c
> 
> Root cause is mass storage function try to queue request from main thread,
> but other thread may already disable ep when function disable.
> 
> As there is no function failure in the driver, in order to avoid effort
> to fix warning, change WARN_ON_ONCE() in usb_ep_queue() to pr_debug().
> 
> Suggested-by: Alan Stern <stern@rowland.harvard.edu>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: yuan linyu <yuanlinyu@hihonor.com>
> ---
> v2: change WARN_ON_ONCE() in usb_ep_queue() to pr_debug()
> v1: https://lore.kernel.org/linux-usb/20240314065949.2627778-1-yuanlinyu@hihonor.com/
> 
>  drivers/usb/gadget/udc/core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> index 9d4150124fdb..2fbe5977c11d 100644
> --- a/drivers/usb/gadget/udc/core.c
> +++ b/drivers/usb/gadget/udc/core.c
> @@ -292,7 +292,8 @@ int usb_ep_queue(struct usb_ep *ep,
>  {
>  	int ret = 0;
>  
> -	if (WARN_ON_ONCE(!ep->enabled && ep->address)) {
> +	if (!ep->enabled && ep->address) {
> +		pr_debug("queue disabled ep %x\n", ep->address);
>  		ret = -ESHUTDOWN;
>  		goto out;
>  	}

Okay, this a lot better and a much simpler solution to the problem.

However, I think the message should be more clear.  Something like this:

		pr_debug("USB gadget: request queued to disabled ep 0x%x (%s)\n",
				ep->address, ep->name);

It's too bad that there isn't enough information available to print the 
names of the gadget and the function driver.  Luckily, most systems will 
only have one gadget active at a time.

Alan Stern
diff mbox series

Patch

diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 9d4150124fdb..2fbe5977c11d 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -292,7 +292,8 @@  int usb_ep_queue(struct usb_ep *ep,
 {
 	int ret = 0;
 
-	if (WARN_ON_ONCE(!ep->enabled && ep->address)) {
+	if (!ep->enabled && ep->address) {
+		pr_debug("queue disabled ep %x\n", ep->address);
 		ret = -ESHUTDOWN;
 		goto out;
 	}