[PATCH v10 3/7] usb: onboard-hub: add support for Microchip USB5744

[Marek Vasut]

On 11/11/24 3:10 PM, Abbarapu, Venkatesh wrote:
> Hi Marek,
> 
>> -----Original Message-----
>> From: Marek Vasut <marex at denx.de>
>> Sent: Monday, November 11, 2024 5:44 PM
>> To: Abbarapu, Venkatesh <venkatesh.abbarapu at amd.com>; u-boot at lists.denx.de
>> Cc: Simek, Michal <michal.simek at amd.com>; fabrice.gasnier at foss.st.com; git
>> (AMD-Xilinx) <git at amd.com>
>> Subject: Re: [PATCH v10 3/7] usb: onboard-hub: add support for Microchip
>> USB5744
>>
>> On 11/8/24 4:59 AM, Venkatesh Yadav Abbarapu wrote:
>>> Add support for the Microchip USB5744 USB3.0 and USB2.0 Hub.
>>> The usb5744 driver trigger hub reset signal after soft reset.
>>> The usb5744 hub need to reset after the phy initialization, which
>>> toggles the gpio.
>>>
>>> Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu at amd.com>
>>> Reviewed-by: Marek Vasut <marex at denx.de>
>>> ---
>>>    common/usb_onboard_hub.c | 15 +++++++++++++--
>>>    1 file changed, 13 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/common/usb_onboard_hub.c b/common/usb_onboard_hub.c index
>>> 827ecf9b02..e604c86d8a 100644
>>> --- a/common/usb_onboard_hub.c
>>> +++ b/common/usb_onboard_hub.c
>>> @@ -88,10 +88,21 @@ static int usb_onboard_hub_remove(struct udevice *dev)
>>>    	return ret;
>>>    }
>>>
>>> +static const struct onboard_hub_data usb5744_data = {
>>> +	.power_on_delay_us = 10000,
>>> +	.reset_us = 10000,
>>> +};
>>> +
>>>    static const struct udevice_id usb_onboard_hub_ids[] = {
>>>    	/* Use generic usbVID,PID dt-bindings (usb-device.yaml) */
>>> -	{ .compatible = "usb424,2514" }, /* USB2514B USB 2.0 */
>>> -	{ }
>>> +	{	.compatible = "usb424,2514" /* USB2514B USB 2.0 */
>>
>> Does this hub need any power_on_delay_us / reset_us delay too ?
>> Probably yes ? If so, please add the delay in 1/7 , otherwise there will be a fast reset
>> pulse on the reset GPIO line which the hub may either not recognize, or worse, may
>> make the hub latch up.
> 
> The hub "usb424,2514" is not part of this series,

That does not mean it is OK to break support for that chip by adding 
more functionality into a driver.

[...]

> Am not sure about the power_on_delay_us / reset_us delay for this hub.
I think it is in the table below Figure 5-1 of the public documentation 
for this Microchip USB2514 hub. Page 36 .