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

[Marek Vasut]

On 11/14/24 5:16 AM, Abbarapu, Venkatesh wrote:
> Hi Marek,
> 
>> -----Original Message-----
>> From: Marek Vasut <marex at denx.de>
>> Sent: Wednesday, November 13, 2024 1:15 AM
>> 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 v11 3/7] usb: onboard-hub: add support for Microchip
>> USB5744
>>
>> On 11/12/24 7:42 AM, Abbarapu, Venkatesh wrote:
>>> Hi,
>>>
>>>> -----Original Message-----
>>>> From: Marek Vasut <marex at denx.de>
>>>> Sent: Tuesday, November 12, 2024 11:28 AM
>>>> 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 v11 3/7] usb: onboard-hub: add support for
>>>> Microchip
>>>> USB5744
>>>>
>>>> On 11/12/24 6:07 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.
>>>>> Also update the usb2514 hub_data with the reset delay as 1us.
>>>>>
>>>>> Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu at amd.com>
>>>>> Reviewed-by: Marek Vasut <marex at denx.de>
>>>>> ---
>>>>>     common/usb_onboard_hub.c | 20 ++++++++++++++++++--
>>>>>     1 file changed, 18 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/common/usb_onboard_hub.c b/common/usb_onboard_hub.c
>>>>> index 827ecf9b02..1d146eccee 100644
>>>>> --- a/common/usb_onboard_hub.c
>>>>> +++ b/common/usb_onboard_hub.c
>>>>> @@ -88,10 +88,26 @@ static int usb_onboard_hub_remove(struct udevice
>> *dev)
>>>>>     	return ret;
>>>>>     }
>>>>>
>>>>> +static const struct onboard_hub_data usb2514_data = {
>>>>> +	.reset_us = 1,
>>>>> +};
>>>>> +
>>>>> +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 */
>>>>> +		.data = (ulong)&usb2514_data,
>>>> This ^ hub has to be updated in 1/7 , otherwise if only 1/7 is
>>>> applied (e.g. during bisect), this hub reset will be operated out of specification.
>>>>
>>>> Also, looking at the USB2514 datasheet figure 5-3, it seems the hub
>>>> needs t4=500us recovery time in SMBus mode. Does that mean
>>>> usb2514_data .power_on_delay_us = 500 is missing too ?
>>>>
>>>> Also, it seems t_rstio is 5us in USB5477 datasheet figure 10-4 ,
>>>> where do these 10000us figures above come from ?
>>>
>>> We were seeing i2c failures when we update the reset delay and power
>>> on delay values mentioned from the datasheet so updated to 10000us,
>>> the linux reference is below
>>> https://github.com/torvalds/linux/commit/908f61bedb2c40c6d856bbfd7f870
>>> b967a4cb498
>> Is there a matching delay requirement specified in the USB hub datasheet or is this a
>> workaround for some board-specific behavior ?
> The matching delay is not specified in the USB5744 hub document, but based on testing on 2 boards with the above-mentioned delay i2c failures were not observed.
Is this 10ms a board-specific reset delay ?
Why is it in a generic driver ?