diff mbox series

[BUG,REPORT] USB UDC/gadget regression

Message ID 20230201192022.GC2415@darkstar.musicnaut.iki.fi (mailing list archive)
State New, archived
Headers show
Series [BUG,REPORT] USB UDC/gadget regression | expand

Commit Message

Aaro Koskinen Feb. 1, 2023, 7:20 p.m. UTC
Hi,

After commit fc274c1e9973 ("USB: gadget: Add a new bus for gadgets"),
g_ether started to crash on modprobe when used with OMAP UDC.

It seems that because of drv->bus getting set to NULL in the middle of
bus_add_driver() things go wrong.

The below simple change seems to fix it, but I'm not sure if this is
the correct way? Many drivers seem to be doing the same in UDC start -
are they really all broken?

A.

Comments

Alan Stern Feb. 1, 2023, 9:04 p.m. UTC | #1
On Wed, Feb 01, 2023 at 09:20:22PM +0200, Aaro Koskinen wrote:
> Hi,
> 
> After commit fc274c1e9973 ("USB: gadget: Add a new bus for gadgets"),
> g_ether started to crash on modprobe when used with OMAP UDC.
> 
> It seems that because of drv->bus getting set to NULL in the middle of
> bus_add_driver() things go wrong.
> 
> The below simple change seems to fix it, but I'm not sure if this is
> the correct way? Many drivers seem to be doing the same in UDC start -
> are they really all broken?

Gosh, yes... it's pretty surprising.  None of the UDC drivers should 
change driver->driver.bus.  I don't know why they ever did this; you 
would have expected that field to be NULL anyway before the fc274c1e9973 
commit because it would never get set to anything.

> diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c
> index 6e9314e95be3..47d83bbb09e3 100644
> --- a/drivers/usb/gadget/udc/omap_udc.c
> +++ b/drivers/usb/gadget/udc/omap_udc.c
> @@ -2062,7 +2062,6 @@ static int omap_udc_start(struct usb_gadget *g,
>  	udc->softconnect = 1;
>  
>  	/* hook up the driver */
> -	driver->driver.bus = NULL;
>  	udc->driver = driver;
>  	spin_unlock_irqrestore(&udc->lock, flags);

If you would like to submit a patch fixing all the UDC drivers that do 
this, it would be great.

Alan Stern
diff mbox series

Patch

diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c
index 6e9314e95be3..47d83bbb09e3 100644
--- a/drivers/usb/gadget/udc/omap_udc.c
+++ b/drivers/usb/gadget/udc/omap_udc.c
@@ -2062,7 +2062,6 @@  static int omap_udc_start(struct usb_gadget *g,
 	udc->softconnect = 1;
 
 	/* hook up the driver */
-	driver->driver.bus = NULL;
 	udc->driver = driver;
 	spin_unlock_irqrestore(&udc->lock, flags);