On 11/20/24 09:46, Marek Vasut wrote:
On 11/20/24 5:22 AM, Abbarapu, Venkatesh wrote:
Hi,
-----Original Message-----
From: Marek Vasut <[email protected]>
Sent: Tuesday, November 19, 2024 7:49 PM
To: Abbarapu, Venkatesh <[email protected]>; [email protected]
Cc: Simek, Michal <[email protected]>; [email protected]; git
(AMD-Xilinx) <[email protected]>
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.
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.
Feel free to drop these patches from this series.
Thanks,
Michal
