Re: [RESEND PATCH net-next v3 3/4] net: phy: aquantia: wait for the GLOBAL_CFG to start returning real values

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Bartosz,

On 08/07/2024 08:50, Bartosz Golaszewski wrote:
From: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>

When the PHY is first coming up (or resuming from suspend), it's
possible that although the FW status shows as running, we still see
zeroes in the GLOBAL_CFG set of registers and cannot determine available
modes. Since all models support 10M, add a poll and wait the config to
become available.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
---
  drivers/net/phy/aquantia/aquantia_main.c | 8 +++++++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
index 974795bd0860..2c8ba2725a91 100644
--- a/drivers/net/phy/aquantia/aquantia_main.c
+++ b/drivers/net/phy/aquantia/aquantia_main.c
@@ -652,7 +652,13 @@ static int aqr107_fill_interface_modes(struct phy_device *phydev)
  	unsigned long *possible = phydev->possible_interfaces;
  	unsigned int serdes_mode, rate_adapt;
  	phy_interface_t interface;
-	int i, val;
+	int i, val, ret;
+
+	ret = phy_read_mmd_poll_timeout(phydev, MDIO_MMD_VEND1,
+					VEND1_GLOBAL_CFG_10M, val, val != 0,
+					1000, 100000, false);
+	if (ret)
+		return ret;
/* Walk the media-speed configuration registers to determine which
  	 * host-side serdes modes may be used by the PHY depending on the


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?

On a related note, we had also found an issue with this PHY where
the PHY reset is not working quite correctly. What we see is that
when polling for the firmware ID in aqr107_wait_reset_complete()
is that value in the firware ID registers transitions from 0 to
0xffff and then to the firmware ID. We have been testing the
following change to fix this ...

diff --git a/drivers/net/phy/aquantia/aquantia.h b/drivers/net/phy/aquantia/aquantia.h
index 2465345081f8..278e3b167c58 100644
--- a/drivers/net/phy/aquantia/aquantia.h
+++ b/drivers/net/phy/aquantia/aquantia.h
@@ -20,6 +20,7 @@
 #define VEND1_GLOBAL_FW_ID                     0x0020
 #define VEND1_GLOBAL_FW_ID_MAJOR               GENMASK(15, 8)
 #define VEND1_GLOBAL_FW_ID_MINOR               GENMASK(7, 0)
+#define VEND1_GLOBAL_FW_ID_MASK                        GENMASK(15, 0)
#define VEND1_GLOBAL_MAILBOX_INTERFACE1 0x0200
 #define VEND1_GLOBAL_MAILBOX_INTERFACE1_EXECUTE                BIT(15)
diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
index 0b2db486d8bd..5023fd70050d 100644
--- a/drivers/net/phy/aquantia/aquantia_main.c
+++ b/drivers/net/phy/aquantia/aquantia_main.c
@@ -447,7 +447,9 @@ int aqr_wait_reset_complete(struct phy_device *phydev)
        int val;
return phy_read_mmd_poll_timeout(phydev, MDIO_MMD_VEND1,
-                                        VEND1_GLOBAL_FW_ID, val, val != 0,
+                                        VEND1_GLOBAL_FW_ID, val,
+                                        ((val & VEND1_GLOBAL_FW_ID_MASK) != 0 &&
+                                        (val & VEND1_GLOBAL_FW_ID_MASK) != VEND1_GLOBAL_FW_ID_MASK),
                                         20000, 2000000, false);
 }

I have not tried the resume use-case, but curious if this may
also help here?

Cheers
Jon

--
nvpublic




[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux