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

[Abbarapu, Venkatesh]

Hi,

> -----Original Message-----
> From: Marek Vasut <marex at denx.de>
> Sent: Tuesday, November 19, 2024 7:49 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 v11 3/7] usb: onboard-hub: add support for Microchip
> USB5744
> 
> On 11/19/24 4:22 AM, Abbarapu, Venkatesh wrote:
> 
> [...]
> 
> >>>>>> 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 ?
> >>> On our boards we observed i2c failures when we set the reset delay
> >>> as 5us and power on delay as 1ms as per the USB5744 datasheet. The
> >>> reason of adding 10ms delay is because of i2c failures and the linux
> >>> reference is
> >>> https://github.com/torvalds/linux/commit/908f61bedb2c40c6d856bbfd7f8
> >>> 70
> >>> b967a4cb498 Do you have anything with respect to these delays, as
> >>> you might have tested on some other boards? If anything, please let me know.
> >> If the 10ms delay is a board specific delay, then this has to be
> >> handled in a board specific way, not hard-coded in the driver.
> >> Probably add some new property which specifies the extra board
> >> specific reset delay, but be sure to run that property by the Linux kernel
> maintainers too.
> > Thanks. I will check how to add the board specific delay as new property.
> 
> Add some reset-...-us , similar to what ethernet PHYs already do in DT.
Sure...will check and initiate discussion with the Linux team.

> 
> > As this delay might be specific to the boards which we tested.
> > There won't be any issue if we can add the default "reset_delay"(5us) and
> "power_on_delay" (1ms) from the USB5744 datasheet, as other boards will be
> working without any board specific delay?
> Right, the driver has to be generic , that means it should contain delays specified in
> the datasheet. Board specific delays have to be configured in board specific DTs.
Thanks. I have updated the delays based on usb5744 specification in the 
[PATCH v13 3/7] usb: onboard-hub: add support for Microchip USB5744
Please review.

Thanks
Venkatesh