From: Hans-Frieder Vogt <hfdevel@gmx.net>
This patch makes functions that were provided for aqr107 applicable to
aqr105, or replaces generic functions with specific ones. Since the aqr105
was introduced before NBASE-T was defined (or 802.3bz), there are a number
of vendor specific registers involved in the definition of the
advertisement, in auto-negotiation and in the setting of the speed. The
functions have been written following the downstream driver for TN4010
cards with aqr105 PHY, and use code from aqr107 functions wherever it
seemed to make sense.
Signed-off-by: Hans-Frieder Vogt <hfdevel@gmx.net>
---
drivers/net/phy/aquantia/aquantia_main.c | 242 ++++++++++++++++++++++++++++++-
1 file changed, 240 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
index 86b0e63de5d88fa1050919a8826bdbec4bbcf8ba..38c6cf7814da1fb9a4e715f242249eee15a3cc85 100644
--- a/drivers/net/phy/aquantia/aquantia_main.c
+++ b/drivers/net/phy/aquantia/aquantia_main.c
@@ -33,6 +33,9 @@
#define PHY_ID_AQR115C 0x31c31c33
#define PHY_ID_AQR813 0x31c31cb2
+#define MDIO_AN_10GBT_CTRL_ADV_LTIM BIT(0)
+#define ADVERTISE_XNP BIT(12)
+
#define MDIO_PHYXS_VEND_IF_STATUS 0xe812
#define MDIO_PHYXS_VEND_IF_STATUS_TYPE_MASK GENMASK(7, 3)
#define MDIO_PHYXS_VEND_IF_STATUS_TYPE_KR 0
@@ -50,6 +53,7 @@
#define MDIO_AN_VEND_PROV_1000BASET_HALF BIT(14)
#define MDIO_AN_VEND_PROV_5000BASET_FULL BIT(11)
#define MDIO_AN_VEND_PROV_2500BASET_FULL BIT(10)
+#define MDIO_AN_VEND_PROV_EXC_PHYID_INFO BIT(6)
#define MDIO_AN_VEND_PROV_DOWNSHIFT_EN BIT(4)
#define MDIO_AN_VEND_PROV_DOWNSHIFT_MASK GENMASK(3, 0)
#define MDIO_AN_VEND_PROV_DOWNSHIFT_DFLT 4
@@ -333,6 +337,238 @@ static int aqr_read_status(struct phy_device *phydev)
return genphy_c45_read_status(phydev);
}
+static int aqr105_get_features(struct phy_device *phydev)
+{
+ int ret;
+
+ /* Normal feature discovery */
+ ret = genphy_c45_pma_read_abilities(phydev);
+ if (ret)
+ return ret;
+
+ /* The AQR105 PHY misses to indicate the 2.5G and 5G modes, so add them
+ * here
+ */
+ linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
+ phydev->supported);
+ linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
+ phydev->supported);
+
+ /* The AQR105 PHY suppports both RJ45 and SFP+ interfaces */
+ linkmode_set_bit(ETHTOOL_LINK_MODE_TP_BIT, phydev->supported);
+ linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, phydev->supported);
+
+ return 0;
+}
+
+static int aqr105_config_speed(struct phy_device *phydev)
+{
+ int vend = MDIO_AN_VEND_PROV_EXC_PHYID_INFO;
+ int ctrl10 = MDIO_AN_10GBT_CTRL_ADV_LTIM;
+ int adv = ADVERTISE_CSMA;
+ int ret;
+
+ switch (phydev->speed) {
+ case SPEED_100:
+ adv |= ADVERTISE_100FULL;
+ break;
+ case SPEED_1000:
+ adv |= ADVERTISE_NPAGE;
+ if (phydev->duplex == DUPLEX_FULL)
+ vend |= MDIO_AN_VEND_PROV_1000BASET_FULL;
+ else
+ vend |= MDIO_AN_VEND_PROV_1000BASET_HALF;
+ break;
+ case SPEED_2500:
+ adv |= (ADVERTISE_NPAGE | ADVERTISE_XNP);
+ vend |= MDIO_AN_VEND_PROV_2500BASET_FULL;
+ break;
+ case SPEED_5000:
+ adv |= (ADVERTISE_NPAGE | ADVERTISE_XNP);
+ vend |= MDIO_AN_VEND_PROV_5000BASET_FULL;
+ break;
+ case SPEED_10000:
+ adv |= (ADVERTISE_NPAGE | ADVERTISE_XNP);
+ ctrl10 |= MDIO_AN_10GBT_CTRL_ADV10G;
+ break;
+ default:
+ return -EINVAL;
+ }
+ ret = phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_ADVERTISE, adv);
+ if (ret < 0)
+ return ret;
+ ret = phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_VEND_PROV, vend);
+ if (ret < 0)
+ return ret;
+ ret = phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_CTRL, ctrl10);
+ if (ret < 0)
+ return ret;
+
+ /* set by vendor driver, but should be on by default */
+ ret = phy_set_bits_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1,
+ MDIO_AN_CTRL1_XNP);
+ if (ret < 0)
+ return ret;
+
+ return genphy_c45_an_disable_aneg(phydev);
+}
+
+static int aqr105_config_aneg(struct phy_device *phydev)
+{
+ bool changed = false;
+ u16 reg;
+ int ret;
+
+ ret = aqr_set_mdix(phydev, phydev->mdix_ctrl);
+ if (ret < 0)
+ return ret;
+ if (ret > 0)
+ changed = true;
+
+ if (phydev->autoneg == AUTONEG_DISABLE)
+ return aqr105_config_speed(phydev);
+
+ ret = genphy_c45_an_config_aneg(phydev);
+ if (ret < 0)
+ return ret;
+ if (ret > 0)
+ changed = true;
+
+ /* Clause 45 has no standardized support for 1000BaseT, therefore
+ * use vendor registers for this mode.
+ */
+ reg = 0;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+ phydev->advertising))
+ reg |= MDIO_AN_VEND_PROV_1000BASET_FULL;
+
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
+ phydev->advertising))
+ reg |= MDIO_AN_VEND_PROV_1000BASET_HALF;
+
+ /* Handle the case when the 2.5G and 5G speeds are not advertised */
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
+ phydev->advertising))
+ reg |= MDIO_AN_VEND_PROV_2500BASET_FULL;
+
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
+ phydev->advertising))
+ reg |= MDIO_AN_VEND_PROV_5000BASET_FULL;
+
+ ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN, MDIO_AN_VEND_PROV,
+ MDIO_AN_VEND_PROV_1000BASET_HALF |
+ MDIO_AN_VEND_PROV_1000BASET_FULL |
+ MDIO_AN_VEND_PROV_2500BASET_FULL |
+ MDIO_AN_VEND_PROV_5000BASET_FULL, reg);
+ if (ret < 0)
+ return ret;
+ if (ret > 0)
+ changed = true;
+
+ return genphy_c45_check_and_restart_aneg(phydev, changed);
+}
+
+static int aqr105_read_rate(struct phy_device *phydev)
+{
+ int val;
+
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_TX_VEND_STATUS1);
+ if (val < 0)
+ return val;
+
+ if (val & MDIO_AN_TX_VEND_STATUS1_FULL_DUPLEX)
+ phydev->duplex = DUPLEX_FULL;
+ else
+ phydev->duplex = DUPLEX_HALF;
+
+ switch (FIELD_GET(MDIO_AN_TX_VEND_STATUS1_RATE_MASK, val)) {
+ case MDIO_AN_TX_VEND_STATUS1_10BASET:
+ phydev->speed = SPEED_10;
+ break;
+ case MDIO_AN_TX_VEND_STATUS1_100BASETX:
+ phydev->speed = SPEED_100;
+ break;
+ case MDIO_AN_TX_VEND_STATUS1_1000BASET:
+ phydev->speed = SPEED_1000;
+ break;
+ case MDIO_AN_TX_VEND_STATUS1_2500BASET:
+ phydev->speed = SPEED_2500;
+ break;
+ case MDIO_AN_TX_VEND_STATUS1_5000BASET:
+ phydev->speed = SPEED_5000;
+ break;
+ case MDIO_AN_TX_VEND_STATUS1_10GBASET:
+ phydev->speed = SPEED_10000;
+ break;
+ default:
+ phydev->speed = SPEED_UNKNOWN;
+ }
+
+ return 0;
+}
+
+static int aqr105_read_status(struct phy_device *phydev)
+{
+ int ret;
+ int val;
+
+ ret = aqr_read_status(phydev);
+ if (ret)
+ return ret;
+
+ if (!phydev->link || phydev->autoneg == AUTONEG_DISABLE)
+ return 0;
+
+ /**
+ * The status register is not immediately correct on line side link up.
+ * Poll periodically until it reflects the correct ON state.
+ * Only return fail for read error, timeout defaults to OFF state.
+ */
+ ret = phy_read_mmd_poll_timeout(phydev, MDIO_MMD_PHYXS,
+ MDIO_PHYXS_VEND_IF_STATUS, val,
+ (FIELD_GET(MDIO_PHYXS_VEND_IF_STATUS_TYPE_MASK, val) !=
+ MDIO_PHYXS_VEND_IF_STATUS_TYPE_OFF),
+ AQR107_OP_IN_PROG_SLEEP,
+ AQR107_OP_IN_PROG_TIMEOUT, false);
+ if (ret && ret != -ETIMEDOUT)
+ return ret;
+
+ switch (FIELD_GET(MDIO_PHYXS_VEND_IF_STATUS_TYPE_MASK, val)) {
+ case MDIO_PHYXS_VEND_IF_STATUS_TYPE_KR:
+ phydev->interface = PHY_INTERFACE_MODE_10GKR;
+ break;
+ case MDIO_PHYXS_VEND_IF_STATUS_TYPE_KX:
+ phydev->interface = PHY_INTERFACE_MODE_1000BASEKX;
+ break;
+ case MDIO_PHYXS_VEND_IF_STATUS_TYPE_XFI:
+ phydev->interface = PHY_INTERFACE_MODE_10GBASER;
+ break;
+ case MDIO_PHYXS_VEND_IF_STATUS_TYPE_USXGMII:
+ phydev->interface = PHY_INTERFACE_MODE_USXGMII;
+ break;
+ case MDIO_PHYXS_VEND_IF_STATUS_TYPE_XAUI:
+ phydev->interface = PHY_INTERFACE_MODE_XAUI;
+ break;
+ case MDIO_PHYXS_VEND_IF_STATUS_TYPE_SGMII:
+ phydev->interface = PHY_INTERFACE_MODE_SGMII;
+ break;
+ case MDIO_PHYXS_VEND_IF_STATUS_TYPE_RXAUI:
+ phydev->interface = PHY_INTERFACE_MODE_RXAUI;
+ break;
+ case MDIO_PHYXS_VEND_IF_STATUS_TYPE_OCSGMII:
+ phydev->interface = PHY_INTERFACE_MODE_2500BASEX;
+ break;
+ case MDIO_PHYXS_VEND_IF_STATUS_TYPE_OFF:
+ default:
+ phydev->link = false;
+ phydev->interface = PHY_INTERFACE_MODE_NA;
+ break;
+ }
+
+ /* Read rate from vendor register */
+ return aqr105_read_rate(phydev);
+}
+
static int aqr107_read_rate(struct phy_device *phydev)
{
u32 config_reg;
@@ -911,11 +1147,13 @@ static struct phy_driver aqr_driver[] = {
{
PHY_ID_MATCH_MODEL(PHY_ID_AQR105),
.name = "Aquantia AQR105",
- .config_aneg = aqr_config_aneg,
+ .get_features = aqr105_get_features,
.probe = aqr107_probe,
+ .config_init = aqr107_config_init,
+ .config_aneg = aqr105_config_aneg,
.config_intr = aqr_config_intr,
.handle_interrupt = aqr_handle_interrupt,
- .read_status = aqr_read_status,
+ .read_status = aqr105_read_status,
.suspend = aqr107_suspend,
.resume = aqr107_resume,
},
--
2.47.2Hi, On Sat, 22 Feb 2025 10:49:31 +0100 Hans-Frieder Vogt via B4 Relay <devnull+hfdevel.gmx.net@kernel.org> wrote: > From: Hans-Frieder Vogt <hfdevel@gmx.net> > > This patch makes functions that were provided for aqr107 applicable to > aqr105, or replaces generic functions with specific ones. Since the aqr105 > was introduced before NBASE-T was defined (or 802.3bz), there are a number > of vendor specific registers involved in the definition of the > advertisement, in auto-negotiation and in the setting of the speed. The > functions have been written following the downstream driver for TN4010 > cards with aqr105 PHY, and use code from aqr107 functions wherever it > seemed to make sense. > > Signed-off-by: Hans-Frieder Vogt <hfdevel@gmx.net> > --- > drivers/net/phy/aquantia/aquantia_main.c | 242 ++++++++++++++++++++++++++++++- > 1 file changed, 240 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c > index 86b0e63de5d88fa1050919a8826bdbec4bbcf8ba..38c6cf7814da1fb9a4e715f242249eee15a3cc85 100644 > --- a/drivers/net/phy/aquantia/aquantia_main.c > +++ b/drivers/net/phy/aquantia/aquantia_main.c > @@ -33,6 +33,9 @@ > #define PHY_ID_AQR115C 0x31c31c33 > #define PHY_ID_AQR813 0x31c31cb2 > > +#define MDIO_AN_10GBT_CTRL_ADV_LTIM BIT(0) This is a standard C45 definition, from : 45.2.7.10.15 10GBASE-T LD loop timing ability (7.32.0) So if you need this advertising capability, you should add that in the generic definitions for C45 registers in include/uapi/linux/mdio.h That being said, as it looks this is the first driver using this feature, do you actually need to advertise Loop Timing ability here ? I guess it comes from the vendor driver ? Thanks, Maxime
Hi Maxime, On 23.02.2025 11.32, Maxime Chevallier wrote: > Hi, > > On Sat, 22 Feb 2025 10:49:31 +0100 > Hans-Frieder Vogt via B4 Relay <devnull+hfdevel.gmx.net@kernel.org> > wrote: > >> From: Hans-Frieder Vogt <hfdevel@gmx.net> >> >> This patch makes functions that were provided for aqr107 applicable to >> aqr105, or replaces generic functions with specific ones. Since the aqr105 >> was introduced before NBASE-T was defined (or 802.3bz), there are a number >> of vendor specific registers involved in the definition of the >> advertisement, in auto-negotiation and in the setting of the speed. The >> functions have been written following the downstream driver for TN4010 >> cards with aqr105 PHY, and use code from aqr107 functions wherever it >> seemed to make sense. >> >> Signed-off-by: Hans-Frieder Vogt <hfdevel@gmx.net> >> --- >> drivers/net/phy/aquantia/aquantia_main.c | 242 ++++++++++++++++++++++++++++++- >> 1 file changed, 240 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c >> index 86b0e63de5d88fa1050919a8826bdbec4bbcf8ba..38c6cf7814da1fb9a4e715f242249eee15a3cc85 100644 >> --- a/drivers/net/phy/aquantia/aquantia_main.c >> +++ b/drivers/net/phy/aquantia/aquantia_main.c >> @@ -33,6 +33,9 @@ >> #define PHY_ID_AQR115C 0x31c31c33 >> #define PHY_ID_AQR813 0x31c31cb2 >> >> +#define MDIO_AN_10GBT_CTRL_ADV_LTIM BIT(0) > This is a standard C45 definition, from : > 45.2.7.10.15 10GBASE-T LD loop timing ability (7.32.0) > > So if you need this advertising capability, you should add that in the > generic definitions for C45 registers in include/uapi/linux/mdio.h Thanks. Wasn't aware this being a standard definition. Wouldn't the definition #define ADVERTISE_XNP BIT(12) then need to go to include/uapi/linux/mii.h accordingly? There, bit 12 is currently named ADVERTISE_RESV and commented as unused (which it obviously is not, because it is used in drivers/net/ethernet/sfc/falcon/mdio_10g.c I think, for now, I will just do the same as in the falcon driver and use ADVERTISE_RESV instead. Then it may be renamed later in all places. > > That being said, as it looks this is the first driver using this > feature, do you actually need to advertise Loop Timing ability here ? > I guess it comes from the vendor driver ? you are right. The code just tries to replicate the vendor code. However, I have now tested the driver without this flag and haven't noticed any unusual behavior. So, I guess, it works indeed without. I'll remove the flag in the next revision of the patch. > Thanks, > > Maxime Thanks as well, Hans-Frieder
On Sun, 23 Feb 2025 23:26:49 +0100 Hans-Frieder Vogt <hfdevel@gmx.net> wrote: > Hi Maxime, > > On 23.02.2025 11.32, Maxime Chevallier wrote: > > Hi, > > > > On Sat, 22 Feb 2025 10:49:31 +0100 > > Hans-Frieder Vogt via B4 Relay <devnull+hfdevel.gmx.net@kernel.org> > > wrote: > > > >> From: Hans-Frieder Vogt <hfdevel@gmx.net> > >> > >> This patch makes functions that were provided for aqr107 applicable to > >> aqr105, or replaces generic functions with specific ones. Since the aqr105 > >> was introduced before NBASE-T was defined (or 802.3bz), there are a number > >> of vendor specific registers involved in the definition of the > >> advertisement, in auto-negotiation and in the setting of the speed. The > >> functions have been written following the downstream driver for TN4010 > >> cards with aqr105 PHY, and use code from aqr107 functions wherever it > >> seemed to make sense. > >> > >> Signed-off-by: Hans-Frieder Vogt <hfdevel@gmx.net> > >> --- > >> drivers/net/phy/aquantia/aquantia_main.c | 242 ++++++++++++++++++++++++++++++- > >> 1 file changed, 240 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c > >> index 86b0e63de5d88fa1050919a8826bdbec4bbcf8ba..38c6cf7814da1fb9a4e715f242249eee15a3cc85 100644 > >> --- a/drivers/net/phy/aquantia/aquantia_main.c > >> +++ b/drivers/net/phy/aquantia/aquantia_main.c > >> @@ -33,6 +33,9 @@ > >> #define PHY_ID_AQR115C 0x31c31c33 > >> #define PHY_ID_AQR813 0x31c31cb2 > >> > >> +#define MDIO_AN_10GBT_CTRL_ADV_LTIM BIT(0) > > This is a standard C45 definition, from : > > 45.2.7.10.15 10GBASE-T LD loop timing ability (7.32.0) > > > > So if you need this advertising capability, you should add that in the > > generic definitions for C45 registers in include/uapi/linux/mdio.h > Thanks. Wasn't aware this being a standard definition. > > Wouldn't the definition > #define ADVERTISE_XNP BIT(12) > then need to go to include/uapi/linux/mii.h accordingly? Looks like this is indeed part of the standard now, in 28.2.4.1.3 Auto-Negotiation advertisement register (Register 4), so it seems to be the right move to modify ADVERTISE_RESV into ADVERTISE_XNP. > There, bit 12 is currently named ADVERTISE_RESV and commented as unused > (which it obviously is not, because it is used in > drivers/net/ethernet/sfc/falcon/mdio_10g.c One note is that this driver uses the C45 MMD 7 AN register layout : 45.2.7.1 AN control register (Register 7.0) in which the eXtended Next Page bit is BIT(13). That actually leads to an interesting point, as it appears the at803x.c driver mixes both, which looks incorrect to me : /* Ar803x extended next page bit is enabled by default. Cisco * multigig switches read this bit and attempt to negotiate 10Gbps * rates even if the next page bit is disabled. This is incorrect * behaviour but we still need to accommodate it. XNP is only needed * for 10Gbps support, so disable XNP. */ return phy_modify(phydev, MII_ADVERTISE, MDIO_AN_CTRL1_XNP, 0); In such case, BIT(13) fot MII_ADVERTISE is ADVERTISE_RFAULT, if my understanding of the spec is correct. > I think, for now, I will just do the same as in the falcon driver and > use ADVERTISE_RESV instead. Then it may be renamed later in all places. Make sure you use the BIT(12) in your case indeed, looks to be the right way in that case. > > > > That being said, as it looks this is the first driver using this > > feature, do you actually need to advertise Loop Timing ability here ? > > I guess it comes from the vendor driver ? > you are right. The code just tries to replicate the vendor code. > However, I have now tested the driver without this flag and haven't > noticed any unusual behavior. So, I guess, it works indeed without. > I'll remove the flag in the next revision of the patch. So in that case, no need to define MDIO_AN_10GBT_CTRL_ADV_LTIM at all :) Thanks, Maxime
© 2016 - 2026 Red Hat, Inc.