[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: Thursday, November 21, 2024 5:26 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/20/24 2:20 PM, Abbarapu, Venkatesh wrote:
> > Hi,
> >
> >> -----Original Message-----
> >> From: Marek Vasut <marex at denx.de>
> >> Sent: Wednesday, November 20, 2024 2:16 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/20/24 5:22 AM, Abbarapu, Venkatesh wrote:
> >>> 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/908f61bedb2c40c6d856bbf
> >>>>>>> d7
> >>>>>>> f8
> >>>>>>> 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.
> >> V13 also enables the hub driver for Xilinx hardware, does it not ?
> >> If so, won't doing so lead to incorrect behavior due to -- now -- too
> >> short delay ? I suspect you will need V14 which adds those board
> >> specific delays via DT for the Xilinx hardware.
> > There are many customers who use their own boards and they might not see the
> issue what we observe on Xilinx/AMD boards. Don't you think we are blocking them
> because of our board issues? What you think?
> Sure, drop 6 and 7 and the current state of 1..5 can go in I think.
Rather than dropping these patches...
To not block others, update the default reset delays mentioned in the USB5744 datasheet.
This way the customers can use these patch series and don’t wait for the Xilinx/AMD board issues to be fixed.
We can fix Xilinx/AMD board issues with the reset delays(via DT property) for USB5744 by introducing/ discussing with Linux community.
I think this way...you don't see any issue right?

Thanks
Venkatesh