On Thu, Jul 18, 2024 at 4:49 PM Jon Hunter <jonathanh@xxxxxxxxxx> wrote: > > > On 18/07/2024 15:13, Bartosz Golaszewski wrote: > > On Thu, Jul 18, 2024 at 4:08 PM Jon Hunter <jonathanh@xxxxxxxxxx> wrote: > >> > >> > >> On 18/07/2024 14:29, Bartosz Golaszewski wrote: > >>> On Thu, Jul 18, 2024 at 3:04 PM Bartosz Golaszewski <brgl@xxxxxxxx> wrote: > >>>> > >>>> On Thu, Jul 18, 2024 at 2:23 PM Jon Hunter <jonathanh@xxxxxxxxxx> 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. > I sent a patch for you to test. I think that even if it doesn't fully fix the issue you're observing, it's worth picking it up as it reduces the impact of the workaround I introduced. I'll be off next week so I'm sending it quickly with the hope it will be useful. Bart
