diff mbox series

usb: cdns3: gadget: Configure speed in udc_start

Message ID 20230719085908.1283-1-r-gunasekaran@ti.com
State Accepted
Commit 15cba56dc80092c397be8bbe086abb926808857c
Delegated to: Marek Vasut
Headers show
Series usb: cdns3: gadget: Configure speed in udc_start | expand

Commit Message

Ravi Gunasekaran July 19, 2023, 8:59 a.m. UTC
When one of the functions does not support super speed, the composite
driver forces the gadget to high speed. But the speed is never
configured in the cdns3 gadget driver. So configure the speed
in cdns3_gadget_udc_start just like in the kernel.

Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com>
---
 drivers/usb/cdns3/gadget.c | 4 ++++
 1 file changed, 4 insertions(+)


base-commit: 76c61f29d63163d178b1584ecc9fc2c96c538ff0

Comments

Ravi Gunasekaran July 19, 2023, 11:06 a.m. UTC | #1
On 7/19/23 4:03 PM, Marek Vasut wrote:
> On 7/19/23 10:59, Ravi Gunasekaran wrote:
>> When one of the functions does not support super speed, the composite
>> driver forces the gadget to high speed. But the speed is never
>> configured in the cdns3 gadget driver. So configure the speed
>> in cdns3_gadget_udc_start just like in the kernel.
>>
>> Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com>
> 
> Is this a patch picked from the kernel ?
> Is there a matching kernel commit ID ?

The commit 4df50f89f5 ("usb: composite: force gadget to be USB2 for HS only function")
in u-boot, forces the gadget's max speed to high speed and had a mention in the commit
description that the gadget's udc_start would configure itself in USB 2.0. 

I checked the cdns3 gadget driver in kernel, and the cdns3_gadget_udc_start()
configures speed. So this is not a patch picked directly from the kernel. I used 
kernel code for reference.

> 
>> ---
>>   drivers/usb/cdns3/gadget.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
>> index fcaeab9cc1..cae570cf59 100644
>> --- a/drivers/usb/cdns3/gadget.c
>> +++ b/drivers/usb/cdns3/gadget.c
>> @@ -82,6 +82,9 @@ static int __cdns3_gadget_ep_queue(struct usb_ep *ep,
>>                      struct usb_request *request,
>>                      gfp_t gfp_flags);
>>   +static void cdns3_gadget_udc_set_speed(struct usb_gadget *gadget,
>> +                       enum usb_device_speed speed);
>> +
>>   /**
>>    * cdns3_set_register_bit - set bit in given register.
>>    * @ptr: address of device controller register to be read and changed
>> @@ -2341,6 +2344,7 @@ static int cdns3_gadget_udc_start(struct usb_gadget *gadget,
>>         spin_lock_irqsave(&priv_dev->lock, flags);
>>       priv_dev->gadget_driver = driver;
>> +    cdns3_gadget_udc_set_speed(gadget, gadget->max_speed);
>>       cdns3_gadget_config(priv_dev);
>>       spin_unlock_irqrestore(&priv_dev->lock, flags);
>>       return 0;
>>
>> base-commit: 76c61f29d63163d178b1584ecc9fc2c96c538ff0
> 
> Where did this ^ come from ?

I'm based on origin/next. Please let me know if I should base on origin/master?
Marek Vasut July 21, 2023, 12:02 a.m. UTC | #2
On 7/19/23 13:06, Ravi Gunasekaran wrote:
> 
> 
> On 7/19/23 4:03 PM, Marek Vasut wrote:
>> On 7/19/23 10:59, Ravi Gunasekaran wrote:
>>> When one of the functions does not support super speed, the composite
>>> driver forces the gadget to high speed. But the speed is never
>>> configured in the cdns3 gadget driver. So configure the speed
>>> in cdns3_gadget_udc_start just like in the kernel.
>>>
>>> Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com>
>>
>> Is this a patch picked from the kernel ?
>> Is there a matching kernel commit ID ?
> 
> The commit 4df50f89f5 ("usb: composite: force gadget to be USB2 for HS only function")
> in u-boot, forces the gadget's max speed to high speed and had a mention in the commit
> description that the gadget's udc_start would configure itself in USB 2.0.
> 
> I checked the cdns3 gadget driver in kernel, and the cdns3_gadget_udc_start()
> configures speed. So this is not a patch picked directly from the kernel. I used
> kernel code for reference.

Thanks for clarification.

Reviewed-by: Marek Vasut <marex@denx.de>
Roger Quadros July 22, 2023, 8 a.m. UTC | #3
On 19/07/2023 11:59, Ravi Gunasekaran wrote:
> When one of the functions does not support super speed, the composite
> driver forces the gadget to high speed. But the speed is never
> configured in the cdns3 gadget driver. So configure the speed
> in cdns3_gadget_udc_start just like in the kernel.
> 
> Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com>

Reviewed-by: Roger Quadros <rogerq@kernel.org>
diff mbox series

Patch

diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
index fcaeab9cc1..cae570cf59 100644
--- a/drivers/usb/cdns3/gadget.c
+++ b/drivers/usb/cdns3/gadget.c
@@ -82,6 +82,9 @@  static int __cdns3_gadget_ep_queue(struct usb_ep *ep,
 				   struct usb_request *request,
 				   gfp_t gfp_flags);
 
+static void cdns3_gadget_udc_set_speed(struct usb_gadget *gadget,
+				       enum usb_device_speed speed);
+
 /**
  * cdns3_set_register_bit - set bit in given register.
  * @ptr: address of device controller register to be read and changed
@@ -2341,6 +2344,7 @@  static int cdns3_gadget_udc_start(struct usb_gadget *gadget,
 
 	spin_lock_irqsave(&priv_dev->lock, flags);
 	priv_dev->gadget_driver = driver;
+	cdns3_gadget_udc_set_speed(gadget, gadget->max_speed);
 	cdns3_gadget_config(priv_dev);
 	spin_unlock_irqrestore(&priv_dev->lock, flags);
 	return 0;