Siddharth, On 25/08/2023 08:42, Siddharth Vadapalli wrote: > Roger, > > On 25/08/23 00:39, Roger Quadros wrote: >> >> >> On 24/08/2023 21:24, Tom Rini wrote: >>> On Thu, Aug 24, 2023 at 11:34:29PM +0530, Siddharth Vadapalli wrote: >>>> Hello Roger, >>>> >>>> On 22-08-2023 17:43, Roger Quadros wrote: > > ... > >> >> Even a single "PHY not found" print is not OK. It looks like an >> error while it should not. >> >> The correct solution is to use the MDIO uclass framework and add >> some generic handling in the class driver. drivers/net/eth-phy-uclass.c >> >> We could provide the delay time in the 'reset-deassert-us' property. >> Although I'm not sure if this is the correct property for this case >> as there is no RESET GPIO on the board. What we really want is delay >> from power-on-reset. Which means we might have to introduce a new property >> and use time from boot to determine if PHY is ready or not? >> >> NOTE: PHY ready time is different for hardware reset and power-on-reset. >> 50ms vs 150ms > > Another alternative could be that of implementing the for-loop within > phy_connect() along with the delay, by updating the function with a new > parameter "tries" which indicates the number of retries before finally > printing > "PHY not found" in case of an error. Optionally, another parameter "delay" can > be added, which indicates the delay within the for-loop. > > This implementation will require updating all drivers in drivers/net which use > phy_connect(), with the "tries" parameter set to 1 for them. The > am65-cpsw-nuss > driver can set "tries" to 5 as done in the current patch. > > The idea behind moving the for-loop within phy_connect() is that it will help > solve the issue for other drivers as well, if they potentially encounter such > buggy PHYs in future boards. >
This doesn't sound like a clean solution. All hardware specific details (like reset ready-time) should come from the device tree and not passed around in PHY APIs. Why to do such an intrusive change instead of fixing the am65-cpsw-nuss driver to properly use the PHY uclass driver model and encode the required delays in the device tree? -- cheers, -roger