[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9173a7a6-5531-6b41-1fb8-a8eb4b8c5d2c@roeck-us.net>
Date: Fri, 24 Feb 2023 22:32:07 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Oleksij Rempel <o.rempel@...gutronix.de>
Cc: Woojung Huh <woojung.huh@...rochip.com>,
Andrew Lunn <andrew@...n.ch>, Arun.Ramadoss@...rochip.com,
Florian Fainelli <f.fainelli@...il.com>,
"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, UNGLinuxDriver@...rochip.com,
Eric Dumazet <edumazet@...gle.com>,
Paolo Abeni <pabeni@...hat.com>, Wei Fang <wei.fang@....com>,
kernel@...gutronix.de, intel-wired-lan@...ts.osuosl.org,
Jakub Kicinski <kuba@...nel.org>,
Vladimir Oltean <olteanv@...il.com>,
Vivien Didelot <vivien.didelot@...il.com>,
Heiner Kallweit <hkallweit1@...il.com>
Subject: Re: [PATCH net-next v8 6/9] net: phy: c22: migrate to
genphy_c45_write_eee_adv()
On 2/24/23 22:08, Oleksij Rempel wrote:
> On Fri, Feb 24, 2023 at 04:09:40PM -0800, Guenter Roeck wrote:
>> On 2/24/23 12:02, Oleksij Rempel wrote:
>> [ ... ]
>>>>
>>>> For cubieboard:
>>>>
>>>> MDIO_PCS_EEE_ABLE = 0x0000
>>>>
>>>> qemu reports attempts to access unsupported registers.
>>>>
>>>> I had a look at the Allwinner mdio driver. There is no indication suggesting
>>>> what the real hardware would return when trying to access unsupported registers,
>>>> and the Ethernet controller datasheet is not public.
>>>
>>> These are PHY accesses over MDIO bus. Ethernet controller should not
>>> care about content of this operations. But on qemu side, it is implemented as
>>> part of Ethernet controller emulation...
>>>
>>> Since MDIO_PCS_EEE_ABLE == 0x0000, phydev->supported_eee should prevent
>>> other EEE related operations. But may be actual phy_read_mmd() went
>>> wrong. It is a combination of simple phy_read/write to different
>>> registers.
>>>
>>
>> Adding MDD read/write support in qemu doesn't help. Something else in your patch
>> prevents the PHY from coming up. After reverting your patch, I see
>>
>> sun4i-emac 1c0b000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off
>> IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
>>
>> in the log. This is missing with your patch in place.
>>
>> Anyway, the key difference is not really the qemu emulation, but the added
>> unconditional call to genphy_c45_write_eee_adv() in your patch. If you look
>> closely into that function, you may notice that the 'changed' variable is
>> never set to 0.
>>
>> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
>> index 3813b86689d0..fee514b96ab1 100644
>> --- a/drivers/net/phy/phy-c45.c
>> +++ b/drivers/net/phy/phy-c45.c
>> @@ -672,7 +672,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_read_mdix);
>> */
>> int genphy_c45_write_eee_adv(struct phy_device *phydev, unsigned long *adv)
>> {
>> - int val, changed;
>> + int val, changed = 0;
>>
>> if (linkmode_intersects(phydev->supported_eee, PHY_EEE_CAP1_FEATURES)) {
>> val = linkmode_to_mii_eee_cap1_t(adv);
>>
>> fixes the problem, both for cubieboard and xtensa.
>
> Good point! Thx for finding it!
>
> Do you wont to send the fix against net?
No, please go ahead and do it.
Guenter
Powered by blists - more mailing lists