Sender: Guenter Roeck <groeck7@gmail.com>
Message-ID: <52f8bb78-0913-6e9a-7816-f32cdad688f2@roeck-us.net>
Date:   Fri, 24 Feb 2023 16:09:40 -0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Content-Language: en-US
To:     Oleksij Rempel <o.rempel@pengutronix.de>
Cc:     Woojung Huh <woojung.huh@microchip.com>,
        Andrew Lunn <andrew@lunn.ch>, Arun.Ramadoss@microchip.com,
        Florian Fainelli <f.fainelli@gmail.com>,
        "David S. Miller" <davem@davemloft.net>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, UNGLinuxDriver@microchip.com,
        Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>, Wei Fang <wei.fang@nxp.com>,
        kernel@pengutronix.de, intel-wired-lan@lists.osuosl.org,
        Jakub Kicinski <kuba@kernel.org>,
        Vladimir Oltean <olteanv@gmail.com>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Heiner Kallweit <hkallweit1@gmail.com>
References: <20230211074113.2782508-1-o.rempel@pengutronix.de>
 <20230211074113.2782508-7-o.rempel@pengutronix.de>
 <20230224035553.GA1089605@roeck-us.net>
 <20230224041604.GA1353778@roeck-us.net>
 <20230224045340.GN19238@pengutronix.de>
 <363517fc-d16e-5bcd-763d-fc0e32c2301a@roeck-us.net>
 <20230224165213.GO19238@pengutronix.de>
 <20230224174132.GA1224969@roeck-us.net>
 <20230224183646.GA26307@pengutronix.de>
 <b0af4518-3c07-726e-79a0-19c53f799204@roeck-us.net>
 <20230224200207.GA8437@pengutronix.de>
From:   Guenter Roeck <linux@roeck-us.net>
Subject: Re: [PATCH net-next v8 6/9] net: phy: c22: migrate to
 genphy_c45_write_eee_adv()
In-Reply-To: <20230224200207.GA8437@pengutronix.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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.

Guenter