diff mbox series

[net-next,v3,4/7] net: phy: aquantia: add essential functions to aqr105 driver

Message ID 20241217-tn9510-v3a-v3-4-4d5ef6f686e0@gmx.net (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: tn40xx: add support for AQR105 based cards | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 5 this patch: 5
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 246 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-12-19--12-00 (tests: 881)

Commit Message

Hans-Frieder Vogt via B4 Relay Dec. 17, 2024, 9:07 p.m. UTC 
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. To avoid duplication of code, aqr_config_aneg was
split in a common part and a specific part for aqr105 and other aqr PHYs.
A similar approach has been chosen for the aqr107_read_status function.
Here, the aqr generation (=1 for aqr105, and =2 for aqr107 and higher) is
used to decide whether aqr107(and up) specific registers can be used. I
know this is not particularly elegant, but it is doing the job.

Signed-off-by: Hans-Frieder Vogt <hfdevel@gmx.net>
---
 drivers/net/phy/aquantia/aquantia_main.c | 168 ++++++++++++++++++++++++++-----
 1 file changed, 144 insertions(+), 24 deletions(-)

Comments

Andrew Lunn Dec. 19, 2024, 4:39 p.m. UTC | #1 
On Tue, Dec 17, 2024 at 10:07:35PM +0100, Hans-Frieder Vogt via B4 Relay 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. To avoid duplication of code, aqr_config_aneg was
> split in a common part and a specific part for aqr105 and other aqr PHYs.
> A similar approach has been chosen for the aqr107_read_status function.
> Here, the aqr generation (=1 for aqr105, and =2 for aqr107 and higher) is
> used to decide whether aqr107(and up) specific registers can be used. I
> know this is not particularly elegant, but it is doing the job.
> 
> Signed-off-by: Hans-Frieder Vogt <hfdevel@gmx.net>
> ---
>  drivers/net/phy/aquantia/aquantia_main.c | 168 ++++++++++++++++++++++++++-----
>  1 file changed, 144 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
> index 81eeee29ba3e6fb11a476a5b51a8a8be061ca8c3..a112e3473e079822671535c313f3ae816fe186dd 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
> @@ -107,6 +111,30 @@
>  #define AQR107_OP_IN_PROG_SLEEP		1000
>  #define AQR107_OP_IN_PROG_TIMEOUT	100000
>  
> +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 aqr107_get_sset_count(struct phy_device *phydev)
>  {
>  	return AQR107_SGMII_STAT_SZ;
> @@ -164,6 +192,59 @@ static void aqr107_get_stats(struct phy_device *phydev,
>  	}
>  }
>  
> +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;
> +
> +	/* Half duplex is not supported */
> +	if (phydev->duplex != DUPLEX_FULL)
> +		return -EINVAL;

Do the half duplex modes make into the phydev->supported modes?  I
would expect the core to raise an error if asked to do half duplex but
it is not in phydev->supported.

> +static int aqr_common_read_rate(struct phy_device *phydev, int aqr_gen)
>  {
>  	u32 config_reg;
>  	int val;
> @@ -377,20 +482,22 @@ static int aqr107_read_rate(struct phy_device *phydev)
>  		return 0;
>  	}
>  
> -	val = phy_read_mmd(phydev, MDIO_MMD_VEND1, config_reg);
> -	if (val < 0)
> -		return val;
> +	if (aqr_gen > 1) {
> +		val = phy_read_mmd(phydev, MDIO_MMD_VEND1, config_reg);
> +		if (val < 0)
> +			return val;
>  
> -	if (FIELD_GET(VEND1_GLOBAL_CFG_RATE_ADAPT, val) ==
> -	    VEND1_GLOBAL_CFG_RATE_ADAPT_PAUSE)
> -		phydev->rate_matching = RATE_MATCH_PAUSE;
> -	else
> -		phydev->rate_matching = RATE_MATCH_NONE;
> +		if (FIELD_GET(VEND1_GLOBAL_CFG_RATE_ADAPT, val) ==
> +		    VEND1_GLOBAL_CFG_RATE_ADAPT_PAUSE)
> +			phydev->rate_matching = RATE_MATCH_PAUSE;
> +		else
> +			phydev->rate_matching = RATE_MATCH_NONE;
> +	}

This appears to be at the end of read_rate(), which is also called at
the end of read_status. 

> +static int aqr105_read_status(struct phy_device *phydev)
> +{
> +	return aqr_common_read_status(phydev, 1);
> +}
> +
> +static int aqr107_read_status(struct phy_device *phydev)
> +{
> +	return aqr_common_read_status(phydev, 2);

So rather than having this 1, 2, i think you can put the code here.

	Andrew
diff mbox series

Patch

diff --git a/drivers/net/phy/aquantia/aquantia_main.c b/drivers/net/phy/aquantia/aquantia_main.c
index 81eeee29ba3e6fb11a476a5b51a8a8be061ca8c3..a112e3473e079822671535c313f3ae816fe186dd 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
@@ -107,6 +111,30 @@ 
 #define AQR107_OP_IN_PROG_SLEEP		1000
 #define AQR107_OP_IN_PROG_TIMEOUT	100000
 
+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 aqr107_get_sset_count(struct phy_device *phydev)
 {
 	return AQR107_SGMII_STAT_SZ;
@@ -164,6 +192,59 @@  static void aqr107_get_stats(struct phy_device *phydev,
 	}
 }
 
+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;
+
+	/* Half duplex is not supported */
+	if (phydev->duplex != DUPLEX_FULL)
+		return -EINVAL;
+
+	switch (phydev->speed) {
+	case SPEED_100:
+		adv |= ADVERTISE_100FULL;
+		break;
+	case SPEED_1000:
+		adv |= ADVERTISE_NPAGE;
+		vend |= MDIO_AN_VEND_PROV_1000BASET_FULL;
+		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 aqr_set_mdix(struct phy_device *phydev, int mdix)
 {
 	u16 val = 0;
@@ -186,21 +267,11 @@  static int aqr_set_mdix(struct phy_device *phydev, int mdix)
 				      MDIO_AN_RESVD_VEND_PROV_MDIX_MASK, val);
 }
 
-static int aqr_config_aneg(struct phy_device *phydev)
+static int aqr_common_config_aneg(struct phy_device *phydev, bool changed)
 {
-	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 genphy_c45_pma_setup_forced(phydev);
-
 	ret = genphy_c45_an_config_aneg(phydev);
 	if (ret < 0)
 		return ret;
@@ -241,6 +312,40 @@  static int aqr_config_aneg(struct phy_device *phydev)
 	return genphy_c45_check_and_restart_aneg(phydev, changed);
 }
 
+static int aqr105_config_aneg(struct phy_device *phydev)
+{
+	bool changed = false;
+	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);
+
+	return aqr_common_config_aneg(phydev, changed);
+}
+
+static int aqr_config_aneg(struct phy_device *phydev)
+{
+	bool changed = false;
+	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 genphy_c45_pma_setup_forced(phydev);
+
+	return aqr_common_config_aneg(phydev, changed);
+}
+
 static int aqr_config_intr(struct phy_device *phydev)
 {
 	bool en = phydev->interrupts == PHY_INTERRUPT_ENABLED;
@@ -333,7 +438,7 @@  static int aqr_read_status(struct phy_device *phydev)
 	return genphy_c45_read_status(phydev);
 }
 
-static int aqr107_read_rate(struct phy_device *phydev)
+static int aqr_common_read_rate(struct phy_device *phydev, int aqr_gen)
 {
 	u32 config_reg;
 	int val;
@@ -377,20 +482,22 @@  static int aqr107_read_rate(struct phy_device *phydev)
 		return 0;
 	}
 
-	val = phy_read_mmd(phydev, MDIO_MMD_VEND1, config_reg);
-	if (val < 0)
-		return val;
+	if (aqr_gen > 1) {
+		val = phy_read_mmd(phydev, MDIO_MMD_VEND1, config_reg);
+		if (val < 0)
+			return val;
 
-	if (FIELD_GET(VEND1_GLOBAL_CFG_RATE_ADAPT, val) ==
-	    VEND1_GLOBAL_CFG_RATE_ADAPT_PAUSE)
-		phydev->rate_matching = RATE_MATCH_PAUSE;
-	else
-		phydev->rate_matching = RATE_MATCH_NONE;
+		if (FIELD_GET(VEND1_GLOBAL_CFG_RATE_ADAPT, val) ==
+		    VEND1_GLOBAL_CFG_RATE_ADAPT_PAUSE)
+			phydev->rate_matching = RATE_MATCH_PAUSE;
+		else
+			phydev->rate_matching = RATE_MATCH_NONE;
+	}
 
 	return 0;
 }
 
-static int aqr107_read_status(struct phy_device *phydev)
+static int aqr_common_read_status(struct phy_device *phydev, int aqr_gen)
 {
 	int val, ret;
 
@@ -415,6 +522,7 @@  static int aqr107_read_status(struct phy_device *phydev)
 	if (ret && ret != -ETIMEDOUT)
 		return ret;
 
+	phydev->rate_matching = RATE_MATCH_NONE;
 	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;
@@ -448,7 +556,17 @@  static int aqr107_read_status(struct phy_device *phydev)
 	}
 
 	/* Read possibly downshifted rate from vendor register */
-	return aqr107_read_rate(phydev);
+	return aqr_common_read_rate(phydev, aqr_gen);
+}
+
+static int aqr105_read_status(struct phy_device *phydev)
+{
+	return aqr_common_read_status(phydev, 1);
+}
+
+static int aqr107_read_status(struct phy_device *phydev)
+{
+	return aqr_common_read_status(phydev, 2);
 }
 
 static int aqr107_get_downshift(struct phy_device *phydev, u8 *data)
@@ -911,11 +1029,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,
 },