| Message ID | 20250212100840.3812153-1-prashanth.k@oss.qualcomm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 17c2c87c37862c3e95b55f660681cc6e8d66660e |
| Headers | show |
| Series | usb: gadget: u_ether: Set is_suspend flag if remote wakeup fails | expand |
On Wed, Feb 12, 2025 at 03:38:40PM +0530, Prashanth K wrote: > Currently while UDC suspends, u_ether attempts to remote wakeup > the host if there are any pending transfers. However, if remote > wakeup fails, the UDC remains suspended but the is_suspend flag > is not set. And since is_suspend flag isn't set, the subsequent > eth_start_xmit() would queue USB requests to suspended UDC. > > To fix this, bail out from gether_suspend() only if remote wakeup > operation is successful. > > Cc: stable@vger.kernel.org > Fixes: 0a1af6dfa077 ("usb: gadget: f_ecm: Add suspend/resume and remote wakeup support") > Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com> > --- > drivers/usb/gadget/function/u_ether.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c > index 09e2838917e2..f58590bf5e02 100644 > --- a/drivers/usb/gadget/function/u_ether.c > +++ b/drivers/usb/gadget/function/u_ether.c > @@ -1052,8 +1052,8 @@ void gether_suspend(struct gether *link) > * There is a transfer in progress. So we trigger a remote > * wakeup to inform the host. > */ > - ether_wakeup_host(dev->port_usb); > - return; > + if (!ether_wakeup_host(dev->port_usb)) > + return; What about the other place in the driver where this function is called but the return value is ignored? thanks, greg k-h
On 14-02-25 01:30 pm, Greg Kroah-Hartman wrote: > On Wed, Feb 12, 2025 at 03:38:40PM +0530, Prashanth K wrote: >> Currently while UDC suspends, u_ether attempts to remote wakeup >> the host if there are any pending transfers. However, if remote >> wakeup fails, the UDC remains suspended but the is_suspend flag >> is not set. And since is_suspend flag isn't set, the subsequent >> eth_start_xmit() would queue USB requests to suspended UDC. >> >> To fix this, bail out from gether_suspend() only if remote wakeup >> operation is successful. >> >> Cc: stable@vger.kernel.org >> Fixes: 0a1af6dfa077 ("usb: gadget: f_ecm: Add suspend/resume and remote wakeup support") >> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com> >> --- >> drivers/usb/gadget/function/u_ether.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c >> index 09e2838917e2..f58590bf5e02 100644 >> --- a/drivers/usb/gadget/function/u_ether.c >> +++ b/drivers/usb/gadget/function/u_ether.c >> @@ -1052,8 +1052,8 @@ void gether_suspend(struct gether *link) >> * There is a transfer in progress. So we trigger a remote >> * wakeup to inform the host. >> */ >> - ether_wakeup_host(dev->port_usb); >> - return; >> + if (!ether_wakeup_host(dev->port_usb)) >> + return; > > What about the other place in the driver where this function is called > but the return value is ignored? > > thanks, > > greg k-h Other than above one, eth_start_xmit() tries to remote wakeup host when we have some data to send from n/w layer. In that case we try to wakeup host and directly return NETDEV_TX_BUSY to caller. If remote wakeup succeeds, then resume() would call netif_start_queue() which again lets n/w layer to queue skb. Hence we don't have to check for ret of ether_wakeup_host() in that case. Regards, Prashanth K
diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c index 09e2838917e2..f58590bf5e02 100644 --- a/drivers/usb/gadget/function/u_ether.c +++ b/drivers/usb/gadget/function/u_ether.c @@ -1052,8 +1052,8 @@ void gether_suspend(struct gether *link) * There is a transfer in progress. So we trigger a remote * wakeup to inform the host. */ - ether_wakeup_host(dev->port_usb); - return; + if (!ether_wakeup_host(dev->port_usb)) + return; } spin_lock_irqsave(&dev->lock, flags); link->is_suspend = true;
Currently while UDC suspends, u_ether attempts to remote wakeup the host if there are any pending transfers. However, if remote wakeup fails, the UDC remains suspended but the is_suspend flag is not set. And since is_suspend flag isn't set, the subsequent eth_start_xmit() would queue USB requests to suspended UDC. To fix this, bail out from gether_suspend() only if remote wakeup operation is successful. Cc: stable@vger.kernel.org Fixes: 0a1af6dfa077 ("usb: gadget: f_ecm: Add suspend/resume and remote wakeup support") Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com> --- drivers/usb/gadget/function/u_ether.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)