linux-kernel - Re: [RESEND PATCH net-next v3 3/4] net: phy: aquantia: wait for the GLOBAL

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <874f68e3-a5f4-4771-9d40-59d2efbf2693@nvidia.com>
Date: Thu, 18 Jul 2024 15:49:01 +0100
From: Jon Hunter <jonathanh@...dia.com>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
 Russell King <linux@...linux.org.uk>, "David S . Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
 "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
 Brad Griffis <bgriffis@...dia.com>
Subject: Re: [RESEND PATCH net-next v3 3/4] net: phy: aquantia: wait for the
 GLOBAL_CFG to start returning real values


On 18/07/2024 15:13, Bartosz Golaszewski wrote:
> On Thu, Jul 18, 2024 at 4:08 PM Jon Hunter <jonathanh@...dia.com> wrote:
>>
>>
>> On 18/07/2024 14:29, Bartosz Golaszewski wrote:
>>> On Thu, Jul 18, 2024 at 3:04 PM Bartosz Golaszewski <brgl@...ev.pl> wrote:
>>>>
>>>> On Thu, Jul 18, 2024 at 2:23 PM Jon Hunter <jonathanh@...dia.com> wrote:
>>>>>
>>>>>
>>>>> With the current -next and mainline we are seeing the following issue on
>>>>> our Tegra234 Jetson AGX Orin platform ...
>>>>>
>>>>>     Aquantia AQR113C stmmac-0:00: aqr107_fill_interface_modes failed: -110
>>>>>     tegra-mgbe 6800000.ethernet eth0: __stmmac_open: Cannot attach to PHY (error: -110)
>>>>>
>>>>>
>>>>> We have tracked it down to this change and looks like our PHY does not
>>>>> support 10M ...
>>>>>
>>>>> $ ethtool eth0
>>>>> Settings for eth0:
>>>>>            Supported ports: [  ]
>>>>>            Supported link modes:   100baseT/Full
>>>>>                                    1000baseT/Full
>>>>>                                    10000baseT/Full
>>>>>                                    1000baseKX/Full
>>>>>                                    10000baseKX4/Full
>>>>>                                    10000baseKR/Full
>>>>>                                    2500baseT/Full
>>>>>                                    5000baseT/Full
>>>>>
>>>>> The following fixes this for this platform ...
>>>>>
>>>>> diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
>>>>> index d12e35374231..0b2db486d8bd 100644
>>>>> --- a/drivers/net/phy/aquantia/aquantia_main.c
>>>>> +++ b/drivers/net/phy/aquantia/aquantia_main.c
>>>>> @@ -656,7 +656,7 @@ static int aqr107_fill_interface_modes(struct phy_device *phydev)
>>>>>            int i, val, ret;
>>>>>
>>>>>            ret = phy_read_mmd_poll_timeout(phydev, MDIO_MMD_VEND1,
>>>>> -                                       VEND1_GLOBAL_CFG_10M, val, val != 0,
>>>>> +                                       VEND1_GLOBAL_CFG_100M, val, val != 0,
>>>>>                                            1000, 100000, false);
>>>>>            if (ret)
>>>>>                    return ret;
>>>>>
>>>>>
>>>>> However, I am not sure if this is guaranteed to work for all?
>>>>
>>>> Ah cr*p. No, I don't think it is. We should take the first supported
>>>> mode for a given PHY I think.
>>>>
>>>
>>> TBH I only observed the issue on AQR115C. I don't have any other model
>>> to test with. Is it fine to fix it by implementing
>>> aqr115_fill_interface_modes() that would first wait for this register
>>> to return non-0 and then call aqr107_fill_interface_modes()?
>>
>> I am doing a bit more testing. We have seen a few issues with this PHY
>> driver and so I am wondering if we also need something similar for the
>> AQR113C variant too.
>>
>> Interestingly, the product brief for these PHYs [0] do show that both
>> the AQR113C and AQR115C both support 10M. So I wonder if it is our
>> ethernet controller that is not supporting 10M? I will check on this too.
>>
> 
> Oh you have an 113c? I didn't get this. Yeah, weird, all docs say it
> should support 10M. In fact all AQR PHYs should hence my initial
> change.


Yes we have an AQR113C. I agree it should support this, but for whatever 
reason this is not advertised. I do see that 10M is advertised as 
supported by the network ...

  Link partner advertised link modes:  10baseT/Half 10baseT/Full
                                       100baseT/Half 100baseT/Full
                                       1000baseT/Full

My PC that is on the same network supports 10M, but just not this Tegra 
device. I am checking to see if this is expected for this device.

Jon

-- 
nvpublic