While UDC suspends, u_ether attempts to remote wakeup the host if there
are any pending transfers. If there are no pending transfers, the
is_suspend flag is set. If the is_suspend flag is set, it attempts to
wakeup the host when start to transmit skbs. However, if wakeup fails,
for example, wakeup is not supported, skbs will never be sent.

To fix this, stop to queue skbs and return NETDEV_TX_BUSY only if remote
wakeup operation is successful.

Fixes: 17c2c87c3786 ("usb: gadget: u_ether: Set is_suspend flag if remote wakeup fails")
Signed-off-by: Zhilin Yang <zlyang_001@163.com>
---
 drivers/usb/gadget/function/u_ether.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
index f58590bf5e02..9d746ed3f072 100644
--- a/drivers/usb/gadget/function/u_ether.c
+++ b/drivers/usb/gadget/function/u_ether.c
@@ -473,10 +473,11 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
 
 	if (dev->port_usb && dev->port_usb->is_suspend) {
 		DBG(dev, "Port suspended. Triggering wakeup\n");
-		netif_stop_queue(net);
-		spin_unlock_irqrestore(&dev->lock, flags);
-		ether_wakeup_host(dev->port_usb);
-		return NETDEV_TX_BUSY;
+		if (!ether_wakeup_host(dev->port_usb)) {
+			netif_stop_queue(net);
+			spin_unlock_irqrestore(&dev->lock, flags);
+			return NETDEV_TX_BUSY;
+		}
 	}
 
 	spin_unlock_irqrestore(&dev->lock, flags);
-- 
2.25.1

On 03-05-25 07:49 pm, Zhilin Yang wrote:
> While UDC suspends, u_ether attempts to remote wakeup the host if there
> are any pending transfers. If there are no pending transfers, the
> is_suspend flag is set. If the is_suspend flag is set, it attempts to
> wakeup the host when start to transmit skbs. However, if wakeup fails,
> for example, wakeup is not supported, skbs will never be sent.
> 
AFAIK, we shouldn't send any data over the bus until host resumes UDC.
So either the remote wakeup has to be successful here, or we need to
remain suspended until resume signal comes.

And the SKB won't be lost here since we return NETDEV_TX_BUSY, and
gether_resume() calls netif_start_queue() which starts tx again.

> To fix this, stop to queue skbs and return NETDEV_TX_BUSY only if remote
> wakeup operation is successful.
> 
> Fixes: 17c2c87c3786 ("usb: gadget: u_ether: Set is_suspend flag if remote wakeup fails")
Is it really "fixing" the above commit?

> Signed-off-by: Zhilin Yang <zlyang_001@163.com>
> ---
>  drivers/usb/gadget/function/u_ether.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
> index f58590bf5e02..9d746ed3f072 100644
> --- a/drivers/usb/gadget/function/u_ether.c
> +++ b/drivers/usb/gadget/function/u_ether.c
> @@ -473,10 +473,11 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
>  
>  	if (dev->port_usb && dev->port_usb->is_suspend) {
>  		DBG(dev, "Port suspended. Triggering wakeup\n");
> -		netif_stop_queue(net);
> -		spin_unlock_irqrestore(&dev->lock, flags);
> -		ether_wakeup_host(dev->port_usb);
> -		return NETDEV_TX_BUSY;
> +		if (!ether_wakeup_host(dev->port_usb)) {
> +			netif_stop_queue(net);
> +			spin_unlock_irqrestore(&dev->lock, flags);
> +			return NETDEV_TX_BUSY;
> +		}
>  	}
>  
>  	spin_unlock_irqrestore(&dev->lock, flags);
Regards,
Prashanth K