[PATCH 0/3 net-next] net: phy: aquantia: extend aqr_read_status

[PATCH 0/3 net-next] net: phy: aquantia: extend aqr_read_status

[Heiner Kallweit]

Extend aqr_read_status to read more parameters from the chip.

Heiner Kallweit (3):
  net: phy: aquantia: improve setting speed and duplex in aqr_read_status
  net: phy: aquantia: set interface mode in aqr_read_status
  net: phy: aquantia: use FIELD_GET for getting speed in aqr_read_status

 drivers/net/phy/aquantia.c | 66 ++++++++++++++++++++++++++++++++------
 1 file changed, 56 insertions(+), 10 deletions(-)

-- 
2.20.1

[PATCH 1/3 net-next] net: phy: aquantia: improve setting speed and duplex in aqr_read_status

[Heiner Kallweit]

Add support for speeds 10Mbps, 5Gbps, and 10Gbps. In addition don't
hardcode duplex but read it from the chip.

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

diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia.c
index 482004efa..51ae3feea 100644
--- a/drivers/net/phy/aquantia.c
+++ b/drivers/net/phy/aquantia.c
@@ -133,6 +133,12 @@ static int aqr_read_status(struct phy_device *phydev)
 	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_10GBASET:
+		phydev->speed = SPEED_10000;
+		break;
+	case MDIO_AN_TX_VEND_STATUS1_5000BASET:
+		phydev->speed = SPEED_5000;
+		break;
 	case MDIO_AN_TX_VEND_STATUS1_2500BASET:
 		phydev->speed = SPEED_2500;
 		break;
@@ -142,11 +148,15 @@ static int aqr_read_status(struct phy_device *phydev)
 	case MDIO_AN_TX_VEND_STATUS1_100BASETX:
 		phydev->speed = SPEED_100;
 		break;
+	case MDIO_AN_TX_VEND_STATUS1_10BASET:
+		phydev->speed = SPEED_10;
+		break;
 	default:
-		phydev->speed = SPEED_10000;
+		phydev->speed = SPEED_UNKNOWN;
 		break;
 	}
-	phydev->duplex = DUPLEX_FULL;
+
+	phydev->duplex = !!(reg & MDIO_AN_TX_VEND_STATUS1_FULL_DUPLEX);
 
 	return 0;
 }
-- 
2.20.1

[PATCH 2/3 net-next] net: phy: aquantia: set interface mode in aqr_read_status

[Heiner Kallweit]

Extend aqr_read_status to set the interface mode properly.

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

diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia.c
index 51ae3feea..281873c59 100644
--- a/drivers/net/phy/aquantia.c
+++ b/drivers/net/phy/aquantia.c
@@ -11,6 +11,7 @@
 #include <linux/module.h>
 #include <linux/delay.h>
 #include <linux/phy.h>
+#include <linux/bitfield.h>
 
 #define PHY_ID_AQ1202	0x03a1b445
 #define PHY_ID_AQ2104	0x03a1b460
@@ -34,6 +35,21 @@
 #define MDIO_AN_TX_VEND_INT_MASK2		0xd401
 #define MDIO_AN_TX_VEND_INT_MASK2_LINK		BIT(0)
 
+/* PHY XS System Interface Connection Status */
+#define MDIO_XS_SYSIF_STATUS			0xe812
+#define MDIO_XS_SYSIF_MODE_MASK			GENMASK(7, 3)
+#define MDIO_XS_SYSIF_MODE_BACKPLANE_KR		0
+#define MDIO_XS_SYSIF_MODE_BACKPLANE_KX		1
+#define MDIO_XS_SYSIF_MODE_XFI			2
+#define MDIO_XS_SYSIF_MODE_USXGMII		3
+#define MDIO_XS_SYSIF_MODE_XAUI			4
+#define MDIO_XS_SYSIF_MODE_XAUI_PAUSE		5
+#define MDIO_XS_SYSIF_MODE_SGMII		6
+#define MDIO_XS_SYSIF_MODE_RXAUI		7
+#define MDIO_XS_SYSIF_MODE_MAC			8
+#define MDIO_XS_SYSIF_MODE_OFF			9
+#define MDIO_XS_SYSIF_MODE_OCSGMII		10
+
 /* Vendor specific 1, MDIO_MMD_VEND1 */
 #define VEND1_GLOBAL_INT_STD_STATUS		0xfc00
 #define VEND1_GLOBAL_INT_VEND_STATUS		0xfc01
@@ -158,6 +174,27 @@ static int aqr_read_status(struct phy_device *phydev)
 
 	phydev->duplex = !!(reg & MDIO_AN_TX_VEND_STATUS1_FULL_DUPLEX);
 
+	reg = phy_read_mmd(phydev, MDIO_MMD_PHYXS, MDIO_XS_SYSIF_STATUS);
+
+	switch (FIELD_GET(MDIO_XS_SYSIF_MODE_MASK, reg)) {
+	case MDIO_XS_SYSIF_MODE_BACKPLANE_KR:
+		phydev->interface = PHY_INTERFACE_MODE_10GKR;
+		break;
+	case MDIO_XS_SYSIF_MODE_SGMII:
+		phydev->interface = PHY_INTERFACE_MODE_SGMII;
+		break;
+	case MDIO_XS_SYSIF_MODE_XAUI:
+	case MDIO_XS_SYSIF_MODE_XAUI_PAUSE:
+		phydev->interface = PHY_INTERFACE_MODE_XAUI;
+		break;
+	case MDIO_XS_SYSIF_MODE_RXAUI:
+		phydev->interface = PHY_INTERFACE_MODE_RXAUI;
+		break;
+	default:
+		phydev->interface = PHY_INTERFACE_MODE_NA;
+		break;
+	}
+
 	return 0;
 }
 
-- 
2.20.1

[PATCH 3/3 net-next] net: phy: aquantia: use FIELD_GET for getting speed in aqr_read_status

[Heiner Kallweit]

Change getting the speed to use FIELD_GET() too to be in line with the
rest of the code.

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

diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia.c
index 7dfcbae4f..d8332b128 100644
--- a/drivers/net/phy/aquantia.c
+++ b/drivers/net/phy/aquantia.c
@@ -21,13 +21,13 @@
 #define PHY_ID_AQR405	0x03a1b4b0
 
 #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)
-#define MDIO_AN_TX_VEND_STATUS1_1000BASET	(0x2 << 1)
-#define MDIO_AN_TX_VEND_STATUS1_10GBASET	(0x3 << 1)
-#define MDIO_AN_TX_VEND_STATUS1_2500BASET	(0x4 << 1)
-#define MDIO_AN_TX_VEND_STATUS1_5000BASET	(0x5 << 1)
-#define MDIO_AN_TX_VEND_STATUS1_RATE_MASK	(0x7 << 1)
+#define MDIO_AN_TX_VEND_STATUS1_RATE_MASK	GENMASK(3, 1)
+#define MDIO_AN_TX_VEND_STATUS1_10BASET		0
+#define MDIO_AN_TX_VEND_STATUS1_100BASETX	1
+#define MDIO_AN_TX_VEND_STATUS1_1000BASET	2
+#define MDIO_AN_TX_VEND_STATUS1_10GBASET	3
+#define MDIO_AN_TX_VEND_STATUS1_2500BASET	4
+#define MDIO_AN_TX_VEND_STATUS1_5000BASET	5
 #define MDIO_AN_TX_VEND_STATUS1_FULL_DUPLEX	BIT(0)
 
 #define MDIO_AN_TX_VEND_INT_STATUS2		0xcc01
@@ -148,7 +148,7 @@ static int aqr_read_status(struct phy_device *phydev)
 	mdelay(10);
 	reg = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_TX_VEND_STATUS1);
 
-	switch (reg & MDIO_AN_TX_VEND_STATUS1_RATE_MASK) {
+	switch (FIELD_GET(MDIO_AN_TX_VEND_STATUS1_RATE_MASK, reg)) {
 	case MDIO_AN_TX_VEND_STATUS1_10GBASET:
 		phydev->speed = SPEED_10000;
 		break;
-- 
2.20.1

Re: [PATCH 1/3 net-next] net: phy: aquantia: improve setting speed and duplex in aqr_read_status

[Andrew Lunn]

On Mon, Feb 04, 2019 at 10:03:21PM +0100, Heiner Kallweit wrote:
> Add support for speeds 10Mbps, 5Gbps, and 10Gbps. In addition don't
> hardcode duplex but read it from the chip.

Hi Heiner

The marvell10g does this differently. It gets the local and link
partner advertised link modes and from that works out what the PHY is
doing. If auto-neg is not being used, it then reads the link speed
from the PMA.

The question is, should the Aquantia PHY do the same, or should it
look an vendor registers? Apart from getting the 1G advertisement, all
the Marvell code uses generic registers. So we should be able to move
most of it into phy-c45 and reuse it. That is what i would prefer.

     Andrew

Re: [PATCH 3/3 net-next] net: phy: aquantia: use FIELD_GET for getting speed in aqr_read_status

[Andrew Lunn]

On Mon, Feb 04, 2019 at 10:09:06PM +0100, Heiner Kallweit wrote:
> Change getting the speed to use FIELD_GET() too to be in line with the
> rest of the code.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH 1/3 net-next] net: phy: aquantia: improve setting speed and duplex in aqr_read_status

[Heiner Kallweit]

On 04.02.2019 22:28, Andrew Lunn wrote:
> On Mon, Feb 04, 2019 at 10:03:21PM +0100, Heiner Kallweit wrote:
>> Add support for speeds 10Mbps, 5Gbps, and 10Gbps. In addition don't
>> hardcode duplex but read it from the chip.
> 
> Hi Heiner
> 
> The marvell10g does this differently. It gets the local and link
> partner advertised link modes and from that works out what the PHY is
> doing. If auto-neg is not being used, it then reads the link speed
> from the PMA.
> 
Right, it's the same mechanism we use in genphy_read_status() for
clause 22.

> The question is, should the Aquantia PHY do the same, or should it
> look an vendor registers? Apart from getting the 1G advertisement, all
> the Marvell code uses generic registers. So we should be able to move
> most of it into phy-c45 and reuse it. That is what i would prefer.
> 
I'd like to use standard registers wherever possible. This patch is
meant as a quick win to improve what we do already in aqr_read_status.
Once we have a generic c45 read_status function we should switch to it.
However I assume that information like interface mode we still have
to read from vendor registers.

>      Andrew
> 
Heiner

Re: [PATCH 1/3 net-next] net: phy: aquantia: improve setting speed and duplex in aqr_read_status

[Andrew Lunn]

> I'd like to use standard registers wherever possible. This patch is
> meant as a quick win to improve what we do already in aqr_read_status.
> Once we have a generic c45 read_status function we should switch to it.

Hi Heiner

I don't see much point in adding code which we know we are soon going
to replace. Just replace it.

> However I assume that information like interface mode we still have
> to read from vendor registers.

For the Aquantia PHY, yes. It appears the Marvell PHY does not have
any registers which indicate this, so it uses heuristics based on the
link speed.

    Andrew

Re: [PATCH 1/3 net-next] net: phy: aquantia: improve setting speed and duplex in aqr_read_status

[Heiner Kallweit]

On 04.02.2019 23:23, Andrew Lunn wrote:
>> I'd like to use standard registers wherever possible. This patch is
>> meant as a quick win to improve what we do already in aqr_read_status.
>> Once we have a generic c45 read_status function we should switch to it.
> 
> Hi Heiner
> 
> I don't see much point in adding code which we know we are soon going
> to replace. Just replace it.
> 
OK, let me have a closer look at the other patches you sent me.
To test them I need to get my DTU running first. And I need to check
what happens if certain standard registers don't report what they
should and how to deal with this. E.g. the AQCS109 according to the
datasheet reports in the speed ability register that it is 10G-capable,
what it is not.

>> However I assume that information like interface mode we still have
>> to read from vendor registers.
> 
> For the Aquantia PHY, yes. It appears the Marvell PHY does not have
> any registers which indicate this, so it uses heuristics based on the
> link speed.
> 
>     Andrew
> 
Heiner