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. Bart > Jon > > [0] > https://www.marvell.com/content/dam/marvell/en/public-collateral/transceivers/marvell-phys-transceivers-aqrate-gen4-product-brief.pdf > > > -- > nvpublic