[PATCH 0/2] usb: gadget: f_ncm/f_ecm: do not notify from set_alt()

[PATCH 0/2] usb: gadget: f_ncm/f_ecm: do not notify from set_alt()

[Massimo Toscanelli via B4 Relay]

f_ncm and f_ecm queue a notification on the interrupt endpoint already
at SET_CONFIGURATION, when the data interface is in altsetting 0. If the
host loads no class driver, nobody reads it. While data is pending for
the host the device may answer NYET to an LPM request, and DWC3 does,
so the device never enters L1. The USB-IF USB3CV tool fails the
"L1 Suspend/Resume" test (TD 9.21) because of this.

For altsetting 1 the call in set_alt() is not needed: gether_connect()
already notifies through the open/close callbacks. The extra call also
makes the host see two connection notifications before the speed one.

Patch 1 fixes f_ncm, patch 2 makes the same change in f_ecm.

Signed-off-by: Massimo Toscanelli <massimo.toscanelli@leica-geosystems.com>
---
Massimo Toscanelli (2):
      usb: gadget: f_ncm: do not notify from ncm_set_alt()
      usb: gadget: f_ecm: do not notify from ecm_set_alt()

 drivers/usb/gadget/function/f_ecm.c | 8 --------
 drivers/usb/gadget/function/f_ncm.c | 4 ----
 2 files changed, 12 deletions(-)
---
base-commit: bdf12cde30830d46a8ef1ba93986356f1164b4f1
change-id: 20261023-ncm-ecm-lpm-d8bdaa05796e

Best regards,
--  
Massimo Toscanelli <massimo.toscanelli@leica-geosystems.com>

[PATCH 1/2] usb: gadget: f_ncm: do not notify from ncm_set_alt()

[Massimo Toscanelli via B4 Relay]

From: Massimo Toscanelli <massimo.toscanelli@leica-geosystems.com>

ncm_set_alt() calls ncm_notify() for both altsettings of the data
interface. So a notification is already queued at SET_CONFIGURATION,
when altsetting 0 is selected, and it waits until the host reads the
interrupt endpoint.

The USB 2.0 LPM ECN (section 2.2.2) lets a device answer NYET to an LPM
request when it has data pending for the host, and DWC3 does so. So
when the host loads no class driver, like the USB-IF test tool, the
device never enters L1 and the LPM L1 test fails.

The CDC NCM spec (version 1.1, section 9.2) [1] asks for the
notifications only when the host selects altsetting 1. There the call
is not needed: gether_connect() already calls ncm_open() or
ncm_close(), and both notify. The extra call only restarts the
sequence while the first notification is in flight, so the host gets
two connection notifications before the speed one.

The old ether.c also notified only for altsetting 1, until commit
da741b8c56d6 ("usb ethernet gadget: split CDC Ethernet function")
moved the call out of that branch in f_ecm. f_ncm copied that
pattern.

Remove the call.

Fixes: 9f6ce4240a2b ("usb: gadget: f_ncm.c added")
Link: https://www.usb.org/sites/default/files/NCM11.pdf # [1]
Signed-off-by: Massimo Toscanelli <massimo.toscanelli@leica-geosystems.com>
---
 drivers/usb/gadget/function/f_ncm.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
index 500c9c19c78..0c7ec74d390 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -921,10 +921,6 @@ static int ncm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 				return PTR_ERR(net);
 			ncm->netdev = net;
 		}
-
-		spin_lock(&ncm->lock);
-		ncm_notify(ncm);
-		spin_unlock(&ncm->lock);
 	} else
 		goto fail;
 

-- 
2.43.0

[PATCH 2/2] usb: gadget: f_ecm: do not notify from ecm_set_alt()

[Massimo Toscanelli via B4 Relay]

From: Massimo Toscanelli <massimo.toscanelli@leica-geosystems.com>

ecm_set_alt() calls ecm_notify() for both altsettings of the data
interface. So a notification is already queued at SET_CONFIGURATION,
when altsetting 0 is selected, and it waits until the host reads the
interrupt endpoint.

The USB 2.0 LPM ECN (section 2.2.2) lets a device answer NYET to an LPM
request when it has data pending for the host, and DWC3 does so. So
when the host loads no class driver, the device never enters L1.

For altsetting 1 the call is not needed: gether_connect() already
calls ecm_open() or ecm_close(), and both notify. The extra call only
restarts the sequence while the first notification is in flight, so
the host gets two connection notifications before the speed one. The
removed comment calls this a minor disagreement with the ECM spec.

Remove the call and the comment.

Fixes: da741b8c56d6 ("usb ethernet gadget: split CDC Ethernet function")
Signed-off-by: Massimo Toscanelli <massimo.toscanelli@leica-geosystems.com>
---
 drivers/usb/gadget/function/f_ecm.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c
index e495bac4efe..c8c3a7d1c80 100644
--- a/drivers/usb/gadget/function/f_ecm.c
+++ b/drivers/usb/gadget/function/f_ecm.c
@@ -581,14 +581,6 @@ static int ecm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 			if (IS_ERR(net))
 				return PTR_ERR(net);
 		}
-
-		/* NOTE this can be a minor disagreement with the ECM spec,
-		 * which says speed notifications will "always" follow
-		 * connection notifications.  But we allow one connect to
-		 * follow another (if the first is in flight), and instead
-		 * just guarantee that a speed notification is always sent.
-		 */
-		ecm_notify(ecm);
 	} else
 		goto fail;
 

-- 
2.43.0

Re: [PATCH 1/2] usb: gadget: f_ncm: do not notify from ncm_set_alt()

[Aldo Ariel Panzardo]

Reviewed-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>

Re: [PATCH 2/2] usb: gadget: f_ecm: do not notify from ecm_set_alt()

[Aldo Ariel Panzardo]

Reviewed-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>