usb gadget driver initialization issue

[Zixun Li]

> -----Original Message-----
> From: Zixun Li
> Sent: Monday, July 22, 2024 17:53
> To: 'Marek Vasut' <marex at denx.de>; Lukasz Majewski <lukma at denx.de>;
> open list <u-boot at lists.denx.de>; Mattijs Korpershoek
> <mkorpershoek at baylibre.com>
> Subject: RE: usb gadget driver initialization issue
> 
> Hi, this issue only affects usb_ether, as g_dnl will enter event loop right after
> driver register so the host won't see an un responsive device.
> 
> I think one solution is revert half of 718f1d41 to move
>  usb_gadget_register_driver()/usb_gadget_unregister_driver()
> back to usb_eth_start()/usb_eth_stop().
> 
> It doesn't affect the ability of bind/unbind mentioned by the commit's
> description. In fact by reverting it usb_ether will behaves more like running
> 'ums' or 'dfu', since running a ethernet command like 'dhcp'
> will do register->event loop->unregister just like 'ums' does.

More precisely I'm thinking about the following patch:

Subject: [PATCH] usb: gadget: ether: Handle gadget driver registration in
 start and stop

Revert part of 718f1d41 to move
 usb_gadget_register_driver()/usb_gadget_unregister_driver()
back to usb_eth_start()/usb_eth_stop().

usb_gadget_register_driver() will initialize the USB controller and
enter ready to connect state.

>From the host's point of view, a device is ready to be enumerated.
However, since dm_usb_gadget_handle_interrupts() is only called when
ethernet function is started, at this stage USB events are not managed,
host's enumeration attempts will fail and resulting error like:
    usb 1-1: new high-speed USB device number 50 using xhci_hcd
    usb 1-1: device descriptor read/64, error -110
    usb 1-1: device descriptor read/64, error -110
    usb usb1-port1: attempt power cycle

With this change the USB controller will only be initialized when ethernet
function is used so the host won't see an unresponsive device.

Signed-off-by: Zixun LI <zli at ogga.fr>
---
 drivers/usb/gadget/ether.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index b7b7bacb00..ed55f12662 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -2277,6 +2277,9 @@ static int usb_eth_start(struct udevice *udev)
 	packet_received = 0;
 	packet_sent = 0;
 
+	if (usb_gadget_register_driver(&priv->eth_driver) < 0)
+		goto fail;
+
 	gadget = dev->gadget;
 	usb_gadget_connect(gadget);
 
@@ -2398,6 +2401,8 @@ static void usb_eth_stop(struct udevice *udev)
 		dm_usb_gadget_handle_interrupts(udev->parent);
 		dev->network_started = 0;
 	}
+
+	usb_gadget_unregister_driver(&priv->eth_driver);
 }
 
 static int usb_eth_recv(struct udevice *dev, int flags, uchar **packetp)
@@ -2503,15 +2508,6 @@ static int usb_eth_probe(struct udevice *dev)
 	priv->eth_driver.disconnect	= eth_disconnect;
 	priv->eth_driver.suspend	= eth_suspend;
 	priv->eth_driver.resume		= eth_resume;
-	return usb_gadget_register_driver(&priv->eth_driver);
-}
-
-static int usb_eth_remove(struct udevice *dev)
-{
-	struct ether_priv *priv = dev_get_priv(dev);
-
-	usb_gadget_unregister_driver(&priv->eth_driver);
-
 	return 0;
 }
 
@@ -2526,7 +2522,6 @@ U_BOOT_DRIVER(eth_usb) = {
 	.name	= "usb_ether",
 	.id	= UCLASS_ETH,
 	.probe	= usb_eth_probe,
-	.remove	= usb_eth_remove,
 	.unbind	= usb_eth_unbind,
 	.ops	= &usb_eth_ops,
 	.priv_auto	= sizeof(struct ether_priv),
-- 
2.45.2