[PATCH] usb: gadget: u_ether: Continue to send skbs if remote wakeup fails

[PATCH] usb: gadget: u_ether: Continue to send skbs if remote wakeup fails

[zhilin.yang]

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 remote wakeup the host when start to
send skbs. However, if host does not support remote wakeup, skbs
will never be sent.

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

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 f58590bf5..9d746ed3f 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

Re: [PATCH] usb: gadget: u_ether: Continue to send skbs if remote wakeup fails

[Greg KH]

On Fri, May 02, 2025 at 10:53:52PM +0800, 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 remote wakeup the host when start to
> send skbs. However, if host does not support remote wakeup, skbs
> will never be sent.

Please properly wrap your changelog at 72 columns.

> To fix this, stop to queue skbs and return NETDEV_TX_BUSY only if
> remote wakeup operation is successful.
> 
> Signed-off-by: zhilin.yang <zlyang_001@163.com>

Please use your name, which I doubt has a "." in it, right?  :)

Also, what git commit id does this fix?

thanks,

greg k-h

[PATCH] usb: gadget: u_ether: Continue to send skbs if remote wakeup fails

[Zhilin Yang]

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

Re: [PATCH] usb: gadget: u_ether: Continue to send skbs if remote wakeup fails

[Prashanth K]

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

Re: [PATCH] usb: gadget: u_ether: Continue to send skbs if remote wakeup fails

[Prashanth K]

On 05-05-25 10:50 am, zlyang_001@163.com wrote:
> If  'ether_wakeup_host' fails due to 'not supported
> operation'，'gether_resume' may never be called. I met this issue when
> use raspberry pi5 with debian12(kernel version 6.12.20). May be continue
> to send skbs only when 'ether_wakeup_host' fails due to 'not supported
> operation' is more reasonable.
Please don't top-post
https://subspace.kernel.org/etiquette.html#do-not-top-post-when-replying

As per my understanding, if remote wakeup is not supported, then device
has to remain suspended, and it should not send any data until host
resumes it. If host did not send resume, they you may need to check from
that perspective.
> I also consider another solution，when 'gether_suspend'  is called, we
> may check if ether_wakeup_host is supported regarless if there are any
> transmiting. If it fails due to 'not supported',  is_suspend should not
> be set.
> 
I think this is also not a viable solution here. If suspend() is
received, then the bus activity might have ceased due to the host
entering suspend mode of its own [USB2 spec, 9.1.1.6 Suspended]

> The suspend/resume feature was introduced by  earlier commits, but
> 17c2c87c3786 is the latest commit for u_ether.c. I'm not sure which
> commit should be referenced.
> 
> 
> 
> 
> ---- Replied Message ----
> From 	Prashanth K<prashanth.k@oss.qualcomm.com>
> <mailto:prashanth.k@oss.qualcomm.com>
> Date 	05/05/2025 12:32
> To 	Zhilin Yang<zlyang_001@163.com> <mailto:zlyang_001@163.com>、
> gregkh@linuxfoundation.org <mailto:gregkh@linuxfoundation.org>
> Cc 	linux-usb@vger.kernel.org <mailto:linux-usb@vger.kernel.org>、linux-
> kernel@vger.kernel.org <mailto:linux-kernel@vger.kernel.org>
> Subject 	Re: [PATCH] usb: gadget: u_ether: Continue to send skbs if
> remote wakeup fails
> 
> 
> 
> 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