Re: [PATCH net-next v8 6/9] net: phy: c22: migrate to genphy_c45_write_eee_adv()

[Guenter Roeck <linux@roeck-us.net>]

On 2/27/23 05:08, Andrew Lunn wrote:
>>> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
>>> index f595acd0a895..67dac9f0e71d 100644
>>> --- a/drivers/net/phy/phy-c45.c
>>> +++ b/drivers/net/phy/phy-c45.c
>>> @@ -799,6 +799,7 @@ static int genphy_c45_read_eee_cap1(struct phy_device *phydev)
>>>            * (Register 3.20)
>>>            */
>>>           val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE);
>>> +       printk("MDIO_PCS_EEE_ABLE = 0x%04x", val);
>>>           if (val < 0)
>>>                   return val;
>>>
>>
>> For cubieboard:
>>
>> MDIO_PCS_EEE_ABLE = 0x0000
>>
>> qemu reports attempts to access unsupported registers.
> 
> MDIO is a serial bus with two lines, clock driven by the bus master
> and data. There is a pull up on the data line, so if the device does
> not respond to a read request, you get 0xffff. That value is all i've
> ever seen a real PHY do when asked to read a register which does not
> exist. So i would say QEMU could be better emulate this.
> 
> The code actually looks for the value 0xffff and then decides that EEE
> is not supporting in the PHY.
> 
> The value of 0x0 is probably being interpreted as meaning EEE is
> supported, but none of the link modes, 10Mbps, 100Mbps etc support
> EEE. I would say it is then legitimate to read/write other EEE
> registers, so long as those writes take into account that no link
> modes are actually supported.
> 
> Reading the other messages in this thread, a bug has been found in the
> patches. But i would also say QEMU could do better.
> 

Sure, it could. Always. That is why I checked the qemu code and
actually tried to implement some of the EEE handling, only to
realize that it didn't help. The emulated PHY does support EEE
and would return either 0x0001 or 0x0003 depending on the
underlying hardware. However, returning that and returning/
accepting reasonable values for other EEE registers didn't make
a difference due to the kernel bug.

Thanks,
Guenter