[PATCH net-next 0/5] net: phy: aquantia: improve and extend driver

[PATCH net-next 0/5] net: phy: aquantia: improve and extend driver

[Heiner Kallweit]

This series improves and extends the Aquantia PHY driver.

Andrew Lunn (1):
  net: phy: aquantia: use genphy_c45_an_config_aneg

Heiner Kallweit (4):
  net: phy: aquantia: remove false 5G and 10G speed ability for AQCS109
  net: phy: don't change modes we don't care about in genphy_c45_read_lpa
  net: phy: add genphy_c45_read_status
  net: phy: aquantia: use genphy_c45_read_status

 drivers/net/phy/aquantia.c | 94 ++++++++++++++++++++++++++------------
 drivers/net/phy/phy-c45.c  | 35 +++++++++++++-
 include/linux/phy.h        |  1 +
 3 files changed, 99 insertions(+), 31 deletions(-)

-- 
2.20.1

[PATCH net-next 1/5] net: phy: aquantia: remove false 5G and 10G speed ability for AQCS109

[Heiner Kallweit]

AQCS109 belongs to a family of PHY's where certain members don't
support 5G or 10G. However for all members of the family the chip
reports 10G and 5G capability. Therefore remove the not supported
modes for AQCS109.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/aquantia.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia.c
index 34be44d00..9661ef4b4 100644
--- a/drivers/net/phy/aquantia.c
+++ b/drivers/net/phy/aquantia.c
@@ -152,6 +152,15 @@ static int aqr_read_status(struct phy_device *phydev)
 	return 0;
 }
 
+static int aqcs109_config_init(struct phy_device *phydev)
+{
+	/* AQCS109 belongs to a chip family partially supporting 10G and 5G.
+	 * PMA speed ability bits are the same for all members of the family,
+	 * AQCS109 however supports speeds up to 2.5G only.
+	 */
+	return phy_set_max_speed(phydev, SPEED_2500);
+}
+
 static struct phy_driver aqr_driver[] = {
 {
 	PHY_ID_MATCH_MODEL(PHY_ID_AQ1202),
@@ -208,6 +217,7 @@ static struct phy_driver aqr_driver[] = {
 	.name		= "Aquantia AQCS109",
 	.aneg_done	= genphy_c45_aneg_done,
 	.get_features	= genphy_c45_pma_read_abilities,
+	.config_init	= aqcs109_config_init,
 	.config_aneg    = aqr_config_aneg,
 	.config_intr	= aqr_config_intr,
 	.ack_interrupt	= aqr_ack_interrupt,
-- 
2.20.1

[PATCH net-next 2/5] net: phy: aquantia: add support for auto-negotiation configuration

[Heiner Kallweit]

From: Andrew Lunn <andrew@lunn.ch>
Make use of the generic c45 code, plus code specific to the Aquantia
phy for 1000BaseT negotiation.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/aquantia.c | 40 +++++++++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia.c
index 9661ef4b4..a1846daa3 100644
--- a/drivers/net/phy/aquantia.c
+++ b/drivers/net/phy/aquantia.c
@@ -20,6 +20,10 @@
 #define PHY_ID_AQCS109	0x03a1b5c2
 #define PHY_ID_AQR405	0x03a1b4b0
 
+#define MDIO_AN_VEND_PROV			0xc400
+#define MDIO_AN_VEND_PROV_1000BASET_FULL	BIT(15)
+#define MDIO_AN_VEND_PROV_1000BASET_HALF	BIT(14)
+
 #define MDIO_AN_TX_VEND_STATUS1			0xc800
 #define MDIO_AN_TX_VEND_STATUS1_10BASET		(0x0 << 1)
 #define MDIO_AN_TX_VEND_STATUS1_100BASETX	(0x1 << 1)
@@ -64,10 +68,40 @@
 
 static int aqr_config_aneg(struct phy_device *phydev)
 {
-	linkmode_copy(phydev->supported, phy_10gbit_features);
-	linkmode_copy(phydev->advertising, phydev->supported);
+	bool changed = false;
+	u16 reg;
+	int ret;
 
-	return 0;
+	if (phydev->autoneg == AUTONEG_DISABLE)
+		return genphy_c45_pma_setup_forced(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;
+
+	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, reg);
+	if (ret < 0)
+		return ret;
+	if (ret > 0)
+		changed = true;
+
+	return genphy_c45_check_and_restart_aneg(phydev, changed);
 }
 
 static int aqr_config_intr(struct phy_device *phydev)
-- 
2.20.1

Re: [PATCH net-next 2/5] net: phy: aquantia: add support for auto-negotiation configuration

[Florian Fainelli]

On 2/22/19 2:49 PM, Heiner Kallweit wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> Make use of the generic c45 code, plus code specific to the Aquantia
> phy for 1000BaseT negotiation.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/net/phy/aquantia.c | 40 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia.c
> index 9661ef4b4..a1846daa3 100644
> --- a/drivers/net/phy/aquantia.c
> +++ b/drivers/net/phy/aquantia.c
> @@ -20,6 +20,10 @@
>  #define PHY_ID_AQCS109	0x03a1b5c2
>  #define PHY_ID_AQR405	0x03a1b4b0
>  
> +#define MDIO_AN_VEND_PROV			0xc400
> +#define MDIO_AN_VEND_PROV_1000BASET_FULL	BIT(15)
> +#define MDIO_AN_VEND_PROV_1000BASET_HALF	BIT(14)
> +
>  #define MDIO_AN_TX_VEND_STATUS1			0xc800
>  #define MDIO_AN_TX_VEND_STATUS1_10BASET		(0x0 << 1)
>  #define MDIO_AN_TX_VEND_STATUS1_100BASETX	(0x1 << 1)
> @@ -64,10 +68,40 @@
>  
>  static int aqr_config_aneg(struct phy_device *phydev)
>  {
> -	linkmode_copy(phydev->supported, phy_10gbit_features);
> -	linkmode_copy(phydev->advertising, phydev->supported);
> +	bool changed = false;

You could entirely eliminate changed since this is just a shorthand for
ret > 0 after your recent changes to return that information. Other than
that:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH net-next 3/5] net: phy: don't change modes we don't care about in genphy_c45_read_lpa

[Heiner Kallweit]

Because 1000BaseT isn't covered by Clause 45, the 1000BaseT flags in
phydev->lp_advertising may have been set based on vendor registers
already. genphy_c45_read_lpa() would clear these flags as of today.
Therefore switch to mii_lpa_mod_linkmode_lpa_t.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy-c45.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index ce5fa5346..65ee33e34 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -268,7 +268,7 @@ int genphy_c45_read_lpa(struct phy_device *phydev)
 	if (val < 0)
 		return val;
 
-	mii_lpa_to_linkmode_lpa_t(phydev->lp_advertising, val);
+	mii_lpa_mod_linkmode_lpa_t(phydev->lp_advertising, val);
 	phydev->pause = val & LPA_PAUSE_CAP ? 1 : 0;
 	phydev->asym_pause = val & LPA_PAUSE_ASYM ? 1 : 0;
 
-- 
2.20.1

[PATCH net-next 4/5] net: phy: add genphy_c45_read_status

[Heiner Kallweit]

Similar to genphy_read_status() for Clause 22 add a generic read_status
function for Clause 45.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy-c45.c | 33 +++++++++++++++++++++++++++++++++
 include/linux/phy.h       |  1 +
 2 files changed, 34 insertions(+)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 65ee33e34..49e7cd08b 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -458,6 +458,39 @@ int genphy_c45_pma_read_abilities(struct phy_device *phydev)
 }
 EXPORT_SYMBOL_GPL(genphy_c45_pma_read_abilities);
 
+/**
+ * genphy_c45_read_status - read PHY status
+ * @phydev: target phy_device struct
+ *
+ * Reads status from PHY and sets phy_device members accordingly.
+ */
+int genphy_c45_read_status(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = genphy_c45_read_link(phydev);
+	if (ret)
+		return ret;
+
+	phydev->speed = SPEED_UNKNOWN;
+	phydev->duplex = DUPLEX_UNKNOWN;
+	phydev->pause = 0;
+	phydev->asym_pause = 0;
+
+	if (phydev->autoneg == AUTONEG_ENABLE) {
+		ret = genphy_c45_read_lpa(phydev);
+		if (ret)
+			return ret;
+
+		phy_resolve_aneg_linkmode(phydev);
+	} else {
+		ret = genphy_c45_read_pma(phydev);
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(genphy_c45_read_status);
+
 /* The gen10g_* functions are the old Clause 45 stub */
 
 int gen10g_config_aneg(struct phy_device *phydev)
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 8e9fc5764..a05ba366d 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1107,6 +1107,7 @@ int genphy_c45_an_config_aneg(struct phy_device *phydev);
 int genphy_c45_an_disable_aneg(struct phy_device *phydev);
 int genphy_c45_read_mdix(struct phy_device *phydev);
 int genphy_c45_pma_read_abilities(struct phy_device *phydev);
+int genphy_c45_read_status(struct phy_device *phydev);
 
 /* The gen10g_* functions are the old Clause 45 stub */
 int gen10g_config_aneg(struct phy_device *phydev);
-- 
2.20.1

[PATCH net-next 5/5] net: phy: aquantia: use genphy_c45_read_status

[Heiner Kallweit]

Use new function genphy_c45_read_status(). 1000BaseT link partner
advertisement needs to be read from vendor registers.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/aquantia.c | 46 +++++++++++++++-----------------------
 1 file changed, 18 insertions(+), 28 deletions(-)

diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia.c
index a1846daa3..0f0eb5682 100644
--- a/drivers/net/phy/aquantia.c
+++ b/drivers/net/phy/aquantia.c
@@ -39,6 +39,10 @@
 #define MDIO_AN_TX_VEND_INT_MASK2		0xd401
 #define MDIO_AN_TX_VEND_INT_MASK2_LINK		BIT(0)
 
+#define MDIO_AN_RX_LP_STAT1			0xe820
+#define MDIO_AN_RX_LP_STAT1_1000BASET_FULL	BIT(15)
+#define MDIO_AN_RX_LP_STAT1_1000BASET_HALF	BIT(14)
+
 /* Vendor specific 1, MDIO_MMD_VEND1 */
 #define VEND1_GLOBAL_INT_STD_STATUS		0xfc00
 #define VEND1_GLOBAL_INT_VEND_STATUS		0xfc01
@@ -154,36 +158,22 @@ static int aqr_ack_interrupt(struct phy_device *phydev)
 
 static int aqr_read_status(struct phy_device *phydev)
 {
-	int reg;
-
-	reg = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
-	reg = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
-	if (reg & MDIO_STAT1_LSTATUS)
-		phydev->link = 1;
-	else
-		phydev->link = 0;
-
-	reg = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_TX_VEND_STATUS1);
-	mdelay(10);
-	reg = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_TX_VEND_STATUS1);
-
-	switch (reg & MDIO_AN_TX_VEND_STATUS1_RATE_MASK) {
-	case MDIO_AN_TX_VEND_STATUS1_2500BASET:
-		phydev->speed = SPEED_2500;
-		break;
-	case MDIO_AN_TX_VEND_STATUS1_1000BASET:
-		phydev->speed = SPEED_1000;
-		break;
-	case MDIO_AN_TX_VEND_STATUS1_100BASETX:
-		phydev->speed = SPEED_100;
-		break;
-	default:
-		phydev->speed = SPEED_10000;
-		break;
+	int val;
+
+	if (phydev->autoneg == AUTONEG_ENABLE) {
+		val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_RX_LP_STAT1);
+		if (val < 0)
+			return val;
+
+		linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+				 phydev->lp_advertising,
+				 val & MDIO_AN_RX_LP_STAT1_1000BASET_FULL);
+		linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
+				 phydev->lp_advertising,
+				 val & MDIO_AN_RX_LP_STAT1_1000BASET_HALF);
 	}
-	phydev->duplex = DUPLEX_FULL;
 
-	return 0;
+	return genphy_c45_read_status(phydev);
 }
 
 static int aqcs109_config_init(struct phy_device *phydev)
-- 
2.20.1

Re: [PATCH net-next 0/5] net: phy: aquantia: improve and extend driver

[David Miller]

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Fri, 22 Feb 2019 23:45:53 +0100

> This series improves and extends the Aquantia PHY driver.

Series applied, thanks Heiner.