[PATCH 1/2] net: ti: am65-cpsw-nuss: Workaround for buggy PHY/Board

[Roger Quadros]

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