[PATCH] usb: gadget: dwc2_udc_otg: Fix dwc2_gadget_start()

[Patrice CHOTARD]

Hi Marek

On 2/11/21 12:26 PM, Marek Vasut wrote:
> On 2/11/21 10:58 AM, Patrice CHOTARD wrote:
>> Hi Marek
>>
>> On 2/10/21 3:26 PM, Marek Vasut wrote:
>>> On 2/10/21 3:17 PM, Patrice Chotard wrote:
>>>> Since commit 8745b9ebccae ("usb: gadget: add super speed support")
>>>> ums was no more functional on platform which use dwc2_udc_otg driver.
>>>>
>>>> Remove the speed test in dwc2_gadget_start() to fix it.
>>>> Tested on stm32mp157c-ev1 board.
>>>
>>> Isn't the speed check correct though ?
>>
>> I am not sure this speed test is needed.
>>
>>>
>>> What is really going on when this fails ?
>>
>>
>> Since 8745b9ebccae ("usb: gadget: add super speed support"),
>> driver->speed is now set to USB_SPEED_SUPER in drivers/usb/gadget/composite.c
>>
>> and this forbids dwc2_udc_otg.c to be registered.
> 
> That sounds like a bug in the USB gadget/otg core , no ?

After analysis, if i correctly understood, the speed test done in both usb_gadget_register_driver() 
and in dwc2_gadget_start() in dwc2_udc_otg.c is too restrictive and accepts only gadget driver with 
max speed equal to USB_SPEED_FULL or USB_SPEED_HIGH.

So all gadget driver with max speed set to higher speed than USB_SPEED_HIGH (USB_SPEED_SUPER in case of 
composite gadget driver) are not allowed, which is wrong.

This test should check that the gadget driver max speed is higher or equal to the min speed supported by 
dwc2_udc_otg driver, ie USB_SPEED_FULL.

So the test should be :

@@ -247,12 +247,11 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver)
 	unsigned long flags = 0;
 
 	debug_cond(DEBUG_SETUP != 0, "%s: %s\n", __func__, "no name");
 
 	if (!driver
-	    || (driver->speed != USB_SPEED_FULL
-		&& driver->speed != USB_SPEED_HIGH)
+	    || driver->speed < USB_SPEED_FULL
 	    || !driver->bind || !driver->disconnect || !driver->setup)
 		return -EINVAL;
 	if (!dev)
 		return -ENODEV;
 	if (dev->driver)
@@ -319,12 +318,11 @@ static int dwc2_gadget_start(struct usb_gadget *g,
 	struct dwc2_udc *dev = the_controller;
 
 	debug_cond(DEBUG_SETUP != 0, "%s: %s\n", __func__, "no name");
 
 	if (!driver ||
-	    (driver->speed != USB_SPEED_FULL &&
-	     driver->speed != USB_SPEED_HIGH) ||
+	    driver->speed < USB_SPEED_FULL ||
 	    !driver->bind || !driver->disconnect || !driver->setup)
 		return -EINVAL;
 
 	if (!dev)

I you are agree, i will send a v2 with this.

Patrice