On 23/08/2023 11:02, Siddharth Vadapalli wrote:
>
>
> On 23/08/23 13:22, Roger Quadros wrote:
>> Hi Siddharth,
>>
>> On 23/08/2023 07:35, Siddharth Vadapalli wrote:
>>> Roger,
>>>
>>> On 22/08/23 17:43, Roger Quadros wrote:
>>>> Beagleplay has a buggy Ethernet PHY implementation for the Gigabit
>>>> PHY in the sense that it is non responsive over MDIO immediately
>>>> after power-up/reset.
>>>>
>>>> We need to either try multiple times or wait sufficiently long enough
>>>> (couple of 10s of ms?) before the PHY begins to respond correctly.
>>>
>>> Based on the datasheet at:
>>> https://datasheet.lcsc.com/lcsc/1912111437_Realtek-Semicon-RTL8211F-CG_C187932.pdf
>>> in the section 7.17. PHY Reset (Hardware Reset), it is stated that
>>> software has to wait for at least 50 ms before accessing the PHY
>>> registers. Is this why the for-loop in the code below tries for at most
>>> 5 times with a delay of 10 ms before the next try? If yes, then isn't it
>>
>> Good catch. This might be the reason why PHY is not responding in the first
>> few attempts.
>>
>>> better to move the delay into the realtek PHY driver at
>>> drivers/net/phy/realtek.c
>>
>> We are currently at the MDIO bus driver where we don't even know what PHY
>> is on the bus. So this delay cannot come at the realtec PHY driver.
>> It has to come at the MDIO bus driver level.
>
> Thank you for clarifying.
>
Looking closer, this is already being addressed by drivers/net/eth-phy-uclass.c
in eth_phy_pre_probe()
linux/Documentation/devicetree/bindings/net/ethernet-phy.yaml
reset-assert-us:
description:
Delay after the reset was asserted in microseconds. If this
property is missing the delay will be skipped.
reset-deassert-us:
description:
Delay after the reset was deasserted in microseconds. If
this property is missing the delay will be skipped.
So we can drop this patch once we implement proper uclass driver for cpsw-mdio.
>>
>>> Shouldn't it be the PHY driver which ensures that any reads/writes to the
>>> PHY
>>> registers are valid? It can ensure this by having a one time 50ms delay for
>>> the
>>> very first access to the PHY registers.>
>
> ...
>
>>>
>>
>
--
cheers,
-roger
