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

[Abbarapu, Venkatesh]

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.

Thanks
Venkatesh