[PATCH net-next v3 0/5] Add a driver for the Marvell 88Q2110 PHY

[PATCH net-next v3 0/5] Add a driver for the Marvell 88Q2110 PHY

[Stefan Eichenberger]

Add support for 1000BASE-T1 to the phy-c45 helper and add a first
1000BASE-T1 driver for the Marvell 88Q2110 PHY.

v3:
  - Read the BASE-T1 capabilities from the ability register (Andrew)
  - Fix several missing return values (Francesco)
  - Poll the reset bit to be sure the soft reset was done (Andrew)
  - Fix reading the latched link status wrongly (Andrew/Russell)
  - Remove probe function (Francesco)
  - Add defines for Marvell specific registers (Andrew)
  - Move the BASE-T1 ability reading to a separate function (Andrew)

v2:
 - Use the same pattern in Kconfig as for 88X2222 (Andrew)
 - Sort Kconfig and Makefile entries (Andrew)
 - Add generic registers to mdio.h (Andrew)
 - Move generic functionality to phy-c45.c (Andrew)
 - Document where proprietary registers are used (Andrew)
 - Remove unnecessary c45 check (Andrew)
 - Remove cable tests which were not implemented (Andrew)
 - Remove comma for terminator entry (Francesco)
 - Sort include files (Francesco)
 - Return phy_write_mmd value in soft_reset (Francesco)

Stefan Eichenberger (5):
  net: phy: add registers to support 1000BASE-T1
  net: phy: c45: add support for 1000BASE-T1 forced setup
  net: phy: c45: add a separate function to read BASE-T1 abilities
  net: phy: c45: detect the BASE-T1 speed from the ability register
  net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2110 PHY

 drivers/net/phy/Kconfig           |   6 +
 drivers/net/phy/Makefile          |   1 +
 drivers/net/phy/marvell-88q2xxx.c | 265 ++++++++++++++++++++++++++++++
 drivers/net/phy/phy-c45.c         |  63 +++++--
 include/linux/phy.h               |   1 +
 include/uapi/linux/mdio.h         |  18 +-
 6 files changed, 339 insertions(+), 15 deletions(-)
 create mode 100644 drivers/net/phy/marvell-88q2xxx.c

-- 
2.39.2

[PATCH net-next v3 1/5] net: phy: add registers to support 1000BASE-T1

[Stefan Eichenberger]

Add registers and definitions to support 1000BASE-T1. This includes the
PCS Control and Status registers (3.2304 and 3.2305) as well as some
missing bits on the PMA/PMD extended ability register (1.18) and PMA/PMD
CTRL (1.2100) register.

Signed-off-by: Stefan Eichenberger <eichest@gmail.com>
---
 include/uapi/linux/mdio.h | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
index b826598d1e94c..d03863da180e7 100644
--- a/include/uapi/linux/mdio.h
+++ b/include/uapi/linux/mdio.h
@@ -82,6 +82,8 @@
 #define MDIO_AN_10BT1_AN_CTRL	526	/* 10BASE-T1 AN control register */
 #define MDIO_AN_10BT1_AN_STAT	527	/* 10BASE-T1 AN status register */
 #define MDIO_PMA_PMD_BT1_CTRL	2100	/* BASE-T1 PMA/PMD control register */
+#define MDIO_PCS_1000BT1_CTRL	2304	/* 1000BASE-T1 PCS control register */
+#define MDIO_PCS_1000BT1_STAT	2305	/* 1000BASE-T1 PCS status register */
 
 /* LASI (Link Alarm Status Interrupt) registers, defined by XENPAK MSA. */
 #define MDIO_PMA_LASI_RXCTRL	0x9000	/* RX_ALARM control */
@@ -332,6 +334,8 @@
 #define MDIO_PCS_10T1L_CTRL_RESET	0x8000	/* PCS reset */
 
 /* BASE-T1 PMA/PMD extended ability register. */
+#define MDIO_PMA_PMD_BT1_B100_ABLE	0x0001	/* 100BASE-T1 Ability */
+#define MDIO_PMA_PMD_BT1_B1000_ABLE	0x0002	/* 1000BASE-T1 Ability */
 #define MDIO_PMA_PMD_BT1_B10L_ABLE	0x0004	/* 10BASE-T1L Ability */
 
 /* BASE-T1 auto-negotiation advertisement register [15:0] */
@@ -373,7 +377,19 @@
 #define MDIO_AN_10BT1_AN_STAT_LPA_EEE_T1L	0x4000 /* 10BASE-T1L LP EEE ability advertisement */
 
 /* BASE-T1 PMA/PMD control register */
-#define MDIO_PMA_PMD_BT1_CTRL_CFG_MST	0x4000 /* MASTER-SLAVE config value */
+#define MDIO_PMA_PMD_BT1_CTRL_STRAP		0x000F /* Type selection (Strap) */
+#define MDIO_PMA_PMD_BT1_CTRL_STRAP_B1000	0x0001 /* Select 1000BASE-T1 */
+#define MDIO_PMA_PMD_BT1_CTRL_CFG_MST		0x4000 /* MASTER-SLAVE config value */
+
+/* 1000BASE-T1 PCS control register */
+#define MDIO_PCS_1000BT1_CTRL_LOW_POWER		0x0800 /* Low power mode */
+#define MDIO_PCS_1000BT1_CTRL_DISABLE_TX	0x4000 /* Global PMA transmit disable */
+#define MDIO_PCS_1000BT1_CTRL_RESET		0x8000 /* Software reset value */
+
+/* 1000BASE-T1 PCS status register */
+#define MDIO_PCS_1000BT1_STAT_LINK	0x0004 /* PCS Link is up */
+#define MDIO_PCS_1000BT1_STAT_FAULT	0x0080 /* There is a fault condition */
+
 
 /* EEE Supported/Advertisement/LP Advertisement registers.
  *
-- 
2.39.2

Re: [PATCH net-next v3 1/5] net: phy: add registers to support 1000BASE-T1

[Andrew Lunn]

On Mon, Jul 17, 2023 at 09:33:46PM +0200, Stefan Eichenberger wrote:
> Add registers and definitions to support 1000BASE-T1. This includes the
> PCS Control and Status registers (3.2304 and 3.2305) as well as some
> missing bits on the PMA/PMD extended ability register (1.18) and PMA/PMD
> CTRL (1.2100) register.
> 
> Signed-off-by: Stefan Eichenberger <eichest@gmail.com>

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

    Andrew

[PATCH net-next v3 2/5] net: phy: c45: add support for 1000BASE-T1 forced setup

[Stefan Eichenberger]

Add support to force 1000BASE-T1 by setting the correct control bit in
the MDIO_MMD_PMA_PMD_BT1_CTRL register.

Signed-off-by: Stefan Eichenberger <eichest@gmail.com>
---
 drivers/net/phy/phy-c45.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 93ed072233779..b73c428a15663 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -108,7 +108,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_pma_baset1_setup_master_slave);
  */
 int genphy_c45_pma_setup_forced(struct phy_device *phydev)
 {
-	int ctrl1, ctrl2, ret;
+	int bt1_ctrl, ctrl1, ctrl2, ret;
 
 	/* Half duplex is not supported */
 	if (phydev->duplex != DUPLEX_FULL)
@@ -176,6 +176,15 @@ int genphy_c45_pma_setup_forced(struct phy_device *phydev)
 		ret = genphy_c45_pma_baset1_setup_master_slave(phydev);
 		if (ret < 0)
 			return ret;
+
+		bt1_ctrl = 0;
+		if (phydev->speed == SPEED_1000)
+			bt1_ctrl = MDIO_PMA_PMD_BT1_CTRL_STRAP_B1000;
+
+		ret = phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_PMD_BT1_CTRL,
+				     MDIO_PMA_PMD_BT1_CTRL_STRAP, bt1_ctrl);
+		if (ret < 0)
+			return ret;
 	}
 
 	return genphy_c45_an_disable_aneg(phydev);
-- 
2.39.2

Re: [PATCH net-next v3 2/5] net: phy: c45: add support for 1000BASE-T1 forced setup

[Andrew Lunn]

On Mon, Jul 17, 2023 at 09:33:47PM +0200, Stefan Eichenberger wrote:
> Add support to force 1000BASE-T1 by setting the correct control bit in
> the MDIO_MMD_PMA_PMD_BT1_CTRL register.
> 
> Signed-off-by: Stefan Eichenberger <eichest@gmail.com>

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

    Andrew

[PATCH net-next v3 3/5] net: phy: c45: add a separate function to read BASE-T1 abilities

[Stefan Eichenberger]

Add a separate function to read the BASE-T1 abilities. Some PHYs do not
indicate the availability of the extended BASE-T1 ability register, so
this function must be called separately.

Signed-off-by: Stefan Eichenberger <eichest@gmail.com>
---
 drivers/net/phy/phy-c45.c | 44 +++++++++++++++++++++++++++------------
 include/linux/phy.h       |  1 +
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index b73c428a15663..58a6bbbe8a70c 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -881,6 +881,36 @@ int genphy_c45_an_config_eee_aneg(struct phy_device *phydev)
 	return genphy_c45_write_eee_adv(phydev, phydev->advertising_eee);
 }
 
+/**
+ * genphy_c45_pma_baset1_read_abilities - read supported baset1 link modes from PMA
+ * @phydev: target phy_device struct
+ *
+ * Read the supported link modes from the extended BASE-T1 ability register
+ */
+int genphy_c45_pma_baset1_read_abilities(struct phy_device *phydev)
+{
+	int val;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_PMD_BT1);
+	if (val < 0)
+		return val;
+
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT,
+			 phydev->supported,
+			 val & MDIO_PMA_PMD_BT1_B10L_ABLE);
+
+	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_T1_STAT);
+	if (val < 0)
+		return val;
+
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
+			 phydev->supported,
+			 val & MDIO_AN_STAT1_ABLE);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(genphy_c45_pma_baset1_read_abilities);
+
 /**
  * genphy_c45_pma_read_abilities - read supported link modes from PMA
  * @phydev: target phy_device struct
@@ -977,21 +1007,9 @@ int genphy_c45_pma_read_abilities(struct phy_device *phydev)
 		}
 
 		if (val & MDIO_PMA_EXTABLE_BT1) {
-			val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_PMD_BT1);
+			val = genphy_c45_pma_baset1_read_abilities(phydev);
 			if (val < 0)
 				return val;
-
-			linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT,
-					 phydev->supported,
-					 val & MDIO_PMA_PMD_BT1_B10L_ABLE);
-
-			val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_T1_STAT);
-			if (val < 0)
-				return val;
-
-			linkmode_mod_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
-					 phydev->supported,
-					 val & MDIO_AN_STAT1_ABLE);
 		}
 	}
 
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 11c1e91563d47..b254848a9c99a 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1826,6 +1826,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_pma_baset1_read_abilities(struct phy_device *phydev);
 int genphy_c45_read_eee_abilities(struct phy_device *phydev);
 int genphy_c45_pma_baset1_read_master_slave(struct phy_device *phydev);
 int genphy_c45_read_status(struct phy_device *phydev);
-- 
2.39.2

Re: [PATCH net-next v3 3/5] net: phy: c45: add a separate function to read BASE-T1 abilities

[Andrew Lunn]

On Mon, Jul 17, 2023 at 09:33:48PM +0200, Stefan Eichenberger wrote:
> Add a separate function to read the BASE-T1 abilities. Some PHYs do not
> indicate the availability of the extended BASE-T1 ability register, so
> this function must be called separately.
> 
> Signed-off-by: Stefan Eichenberger <eichest@gmail.com>

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

    Andrew

[PATCH net-next v3 4/5] net: phy: c45: detect the BASE-T1 speed from the ability register

[Stefan Eichenberger]

Read the ability to do 100BASE-T1 and 1000BASE-T1 from the extended
BASE-T1 ability register of the PHY.

Signed-off-by: Stefan Eichenberger <eichest@gmail.com>
---
 drivers/net/phy/phy-c45.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 58a6bbbe8a70c..8e6fd4962c486 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -899,6 +899,14 @@ int genphy_c45_pma_baset1_read_abilities(struct phy_device *phydev)
 			 phydev->supported,
 			 val & MDIO_PMA_PMD_BT1_B10L_ABLE);
 
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_100baseT1_Full_BIT,
+			 phydev->supported,
+			 val & MDIO_PMA_PMD_BT1_B100_ABLE);
+
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT1_Full_BIT,
+			 phydev->supported,
+			 val & MDIO_PMA_PMD_BT1_B1000_ABLE);
+
 	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_T1_STAT);
 	if (val < 0)
 		return val;
-- 
2.39.2

Re: [PATCH net-next v3 4/5] net: phy: c45: detect the BASE-T1 speed from the ability register

[Andrew Lunn]

On Mon, Jul 17, 2023 at 09:33:49PM +0200, Stefan Eichenberger wrote:
> Read the ability to do 100BASE-T1 and 1000BASE-T1 from the extended
> BASE-T1 ability register of the PHY.
> 
> Signed-off-by: Stefan Eichenberger <eichest@gmail.com>

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

    Andrew

[PATCH net-next v3 5/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2110 PHY

[Stefan Eichenberger]

Add a driver for the Marvell 88Q2110. This driver allows to detect the
link, switch between 100BASE-T1 and 1000BASE-T1 and switch between
master and slave mode. Autonegotiation supported by the PHY does not yet
work.

Signed-off-by: Stefan Eichenberger <eichest@gmail.com>
---
 drivers/net/phy/Kconfig           |   6 +
 drivers/net/phy/Makefile          |   1 +
 drivers/net/phy/marvell-88q2xxx.c | 265 ++++++++++++++++++++++++++++++
 3 files changed, 272 insertions(+)
 create mode 100644 drivers/net/phy/marvell-88q2xxx.c

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 78e6981650d94..87b8238587173 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -217,6 +217,12 @@ config MARVELL_10G_PHY
 	help
 	  Support for the Marvell Alaska MV88X3310 and compatible PHYs.
 
+config MARVELL_88Q2XXX_PHY
+	tristate "Marvell 88Q2XXX PHY"
+	help
+	  Support for the Marvell 88Q2XXX 100/1000BASE-T1 Automotive Ethernet
+	  PHYs.
+
 config MARVELL_88X2222_PHY
 	tristate "Marvell 88X2222 PHY"
 	help
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 2fe51ea83babe..35142780fc9da 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -66,6 +66,7 @@ obj-$(CONFIG_LSI_ET1011C_PHY)	+= et1011c.o
 obj-$(CONFIG_LXT_PHY)		+= lxt.o
 obj-$(CONFIG_MARVELL_10G_PHY)	+= marvell10g.o
 obj-$(CONFIG_MARVELL_PHY)	+= marvell.o
+obj-$(CONFIG_MARVELL_88Q2XXX_PHY)	+= marvell-88q2xxx.o
 obj-$(CONFIG_MARVELL_88X2222_PHY)	+= marvell-88x2222.o
 obj-$(CONFIG_MAXLINEAR_GPHY)	+= mxl-gpy.o
 obj-$(CONFIG_MEDIATEK_GE_PHY)	+= mediatek-ge.o
diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
new file mode 100644
index 0000000000000..e675b251e01bc
--- /dev/null
+++ b/drivers/net/phy/marvell-88q2xxx.c
@@ -0,0 +1,265 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Marvell 88Q2XXX automotive 100BASE-T1/1000BASE-T1 PHY driver
+ */
+#include <linux/ethtool_netlink.h>
+#include <linux/marvell_phy.h>
+#include <linux/phy.h>
+
+#define MARVELL_PHY_ID_88Q2110	0x002b0981
+
+#define MDIO_MMD_AN_MV_STAT			32769
+#define MDIO_MMD_AN_MV_STAT_ANEG		0x0100
+#define MDIO_MMD_AN_MV_STAT_LOCAL_RX		0x1000
+#define MDIO_MMD_AN_MV_STAT_REMOTE_RX		0x2000
+#define MDIO_MMD_AN_MV_STAT_LOCAL_MASTER	0x4000
+#define MDIO_MMD_AN_MV_STAT_MS_CONF_FAULT	0x8000
+
+#define MDIO_MMD_PCS_MV_100BT1_STAT1			33032
+#define MDIO_MMD_PCS_MV_100BT1_STAT1_IDLE_ERROR	0x00FF
+#define MDIO_MMD_PCS_MV_100BT1_STAT1_JABBER		0x0100
+#define MDIO_MMD_PCS_MV_100BT1_STAT1_LINK		0x0200
+#define MDIO_MMD_PCS_MV_100BT1_STAT1_LOCAL_RX		0x1000
+#define MDIO_MMD_PCS_MV_100BT1_STAT1_REMOTE_RX		0x2000
+#define MDIO_MMD_PCS_MV_100BT1_STAT1_LOCAL_MASTER	0x4000
+
+#define MDIO_MMD_PCS_MV_100BT1_STAT2		33033
+#define MDIO_MMD_PCS_MV_100BT1_STAT2_JABBER	0x0001
+#define MDIO_MMD_PCS_MV_100BT1_STAT2_POL	0x0002
+#define MDIO_MMD_PCS_MV_100BT1_STAT2_LINK	0x0004
+#define MDIO_MMD_PCS_MV_100BT1_STAT2_ANGE	0x0008
+
+static int mv88q2xxx_soft_reset(struct phy_device *phydev)
+{
+	int ret;
+	int val;
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_PCS,
+			    MDIO_PCS_1000BT1_CTRL, MDIO_PCS_1000BT1_CTRL_RESET);
+	if (ret < 0)
+		return ret;
+
+	return phy_read_mmd_poll_timeout(phydev, MDIO_MMD_PCS,
+					 MDIO_PCS_1000BT1_CTRL, val,
+					 !(val & MDIO_PCS_1000BT1_CTRL_RESET),
+					 50000, 600000, true);
+}
+
+static int mv88q2xxx_read_link_gbit(struct phy_device *phydev)
+{
+	int ret;
+	bool link = false;
+
+	/* Read vendor specific Auto-Negotiation status register to get local
+	 * and remote receiver status according to software initialization
+	 * guide.
+	 */
+	ret = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_MMD_AN_MV_STAT);
+	if (ret < 0) {
+		return ret;
+	} else if ((ret & MDIO_MMD_AN_MV_STAT_LOCAL_RX) &&
+		   (ret & MDIO_MMD_AN_MV_STAT_REMOTE_RX)) {
+		/* The link state is latched low so that momentary link
+		 * drops can be detected. Do not double-read the status
+		 * in polling mode to detect such short link drops except
+		 * the link was already down.
+		 */
+		if (!phy_polling_mode(phydev) || !phydev->link) {
+			ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_1000BT1_STAT);
+			if (ret < 0)
+				return ret;
+			else if (ret & MDIO_PCS_1000BT1_STAT_LINK)
+				link = true;
+		}
+
+		if (!link) {
+			ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_1000BT1_STAT);
+			if (ret < 0)
+				return ret;
+			else if (ret & MDIO_PCS_1000BT1_STAT_LINK)
+				link = true;
+		}
+	}
+
+	phydev->link = link;
+
+	return 0;
+}
+
+static int mv88q2xxx_read_link_100m(struct phy_device *phydev)
+{
+	int ret;
+
+	/* The link state is latched low so that momentary link
+	 * drops can be detected. Do not double-read the status
+	 * in polling mode to detect such short link drops except
+	 * the link was already down. In case we are not polling,
+	 * we always read the realtime status.
+	 */
+	if (!phy_polling_mode(phydev) || !phydev->link) {
+		ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_MMD_PCS_MV_100BT1_STAT1);
+		if (ret < 0)
+			return ret;
+		else if (ret & MDIO_MMD_PCS_MV_100BT1_STAT1_LINK)
+			goto out;
+	}
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_MMD_PCS_MV_100BT1_STAT1);
+	if (ret < 0)
+		return ret;
+
+out:
+	/* Check if we have link and if the remote and local receiver are ok */
+	if ((ret & MDIO_MMD_PCS_MV_100BT1_STAT1_LINK) &&
+	    (ret & MDIO_MMD_PCS_MV_100BT1_STAT1_LOCAL_RX) &&
+	    (ret & MDIO_MMD_PCS_MV_100BT1_STAT1_REMOTE_RX))
+		phydev->link = true;
+	else
+		phydev->link = false;
+
+	return 0;
+}
+
+static int mv88q2xxx_read_link(struct phy_device *phydev)
+{
+	int ret;
+
+	/* The 88Q2XXX PHYs do not have the PMA/PMD status register available,
+	 * therefore we need to read the link status from the vendor specific
+	 * registers depending on the speed.
+	 */
+	if (phydev->speed == SPEED_1000)
+		ret = mv88q2xxx_read_link_gbit(phydev);
+	else
+		ret = mv88q2xxx_read_link_100m(phydev);
+
+	return ret;
+}
+
+static int mv88q2xxx_read_status(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = mv88q2xxx_read_link(phydev);
+	if (ret < 0)
+		return ret;
+
+	return genphy_c45_read_pma(phydev);
+}
+
+static int mv88q2xxx_get_features(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = genphy_c45_pma_read_abilities(phydev);
+	if (ret)
+		return ret;
+
+	/* We need to read the baset1 extended abilities manually because the
+	 * PHY does not signalize it has the extended abilities register
+	 * available.
+	 */
+	ret = genphy_c45_pma_baset1_read_abilities(phydev);
+	if (ret)
+		return ret;
+
+	/* The PHY signalizes it supports autonegotiation. Unfortunately, so
+	 * far it was not possible to get a link even when following the init
+	 * sequence provided by Marvell. Disable it for now until a proper
+	 * workaround is found or a new PHY revision is released.
+	 */
+	linkmode_clear_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, phydev->supported);
+
+	return 0;
+}
+
+static int mv88q2xxx_config_aneg(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = genphy_c45_config_aneg(phydev);
+	if (ret)
+		return ret;
+
+	return mv88q2xxx_soft_reset(phydev);
+}
+
+static int mv88q2xxx_config_init(struct phy_device *phydev)
+{
+	int ret;
+
+	/* The 88Q2XXX PHYs do have the extended ability register available, but
+	 * register MDIO_PMA_EXTABLE where they should signalize it does not
+	 * work according to specification. Therefore, we force it here.
+	 */
+	phydev->pma_extable = MDIO_PMA_EXTABLE_BT1;
+
+	/* Read the current PHY configuration */
+	ret = genphy_c45_read_pma(phydev);
+	if (ret)
+		return ret;
+
+	return mv88q2xxx_config_aneg(phydev);
+}
+
+static int mv88q2xxxx_get_sqi(struct phy_device *phydev)
+{
+	int ret;
+
+	if (phydev->speed == SPEED_100) {
+		/* Read the SQI from the vendor specific receiver status
+		 * register
+		 */
+		ret = phy_read_mmd(phydev, MDIO_MMD_PCS, 0x8230);
+		if (ret < 0)
+			return ret;
+
+		ret = ret >> 12;
+	} else {
+		/* Read from vendor specific registers, they are not documented
+		 * but can be found in the Software Initialization Guide. Only
+		 * revisions >= A0 are supported.
+		 */
+		ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, 0xFC5D, 0x00FF, 0x00AC);
+		if (ret < 0)
+			return ret;
+
+		ret = phy_read_mmd(phydev, MDIO_MMD_PCS, 0xfc88);
+		if (ret < 0)
+			return ret;
+	}
+
+	return ret & 0x0F;
+}
+
+static int mv88q2xxxx_get_sqi_max(struct phy_device *phydev)
+{
+	return 15;
+}
+
+static struct phy_driver mv88q2xxx_driver[] = {
+	{
+		.phy_id			= MARVELL_PHY_ID_88Q2110,
+		.phy_id_mask		= MARVELL_PHY_ID_MASK,
+		.name			= "mv88q2110",
+		.get_features		= mv88q2xxx_get_features,
+		.config_aneg		= mv88q2xxx_config_aneg,
+		.config_init		= mv88q2xxx_config_init,
+		.read_status		= mv88q2xxx_read_status,
+		.soft_reset		= mv88q2xxx_soft_reset,
+		.set_loopback		= genphy_c45_loopback,
+		.get_sqi		= mv88q2xxxx_get_sqi,
+		.get_sqi_max		= mv88q2xxxx_get_sqi_max,
+	},
+};
+
+module_phy_driver(mv88q2xxx_driver);
+
+static struct mdio_device_id __maybe_unused mv88q2xxx_tbl[] = {
+	{ MARVELL_PHY_ID_88Q2110, MARVELL_PHY_ID_MASK },
+	{ /*sentinel*/ }
+};
+MODULE_DEVICE_TABLE(mdio, mv88q2xxx_tbl);
+
+MODULE_DESCRIPTION("Marvell 88Q2XXX 100/1000BASE-T1 Automotive Ethernet PHY driver");
+MODULE_LICENSE("GPL");
-- 
2.39.2

Re: [PATCH net-next v3 5/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2110 PHY

[Andrew Lunn]

> +#define MARVELL_PHY_ID_88Q2110	0x002b0981

> +
> +static struct phy_driver mv88q2xxx_driver[] = {
> +	{
> +		.phy_id			= MARVELL_PHY_ID_88Q2110,
> +		.phy_id_mask		= MARVELL_PHY_ID_MASK,

Probably not an issue...

The ID you have above is for revision 1 of the PHY. But the mask will
cause the revision to be ignored. Do you want to ignore the revision?
Are there different errata for revision 0 and 1?

    Andrew

Re: [PATCH net-next v3 5/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2110 PHY

[Stefan Eichenberger]

Hi Andrew,

On Mon, Jul 17, 2023 at 11:54:39PM +0200, Andrew Lunn wrote:
> > +#define MARVELL_PHY_ID_88Q2110	0x002b0981
> 
> > +
> > +static struct phy_driver mv88q2xxx_driver[] = {
> > +	{
> > +		.phy_id			= MARVELL_PHY_ID_88Q2110,
> > +		.phy_id_mask		= MARVELL_PHY_ID_MASK,
> 
> Probably not an issue...
> 
> The ID you have above is for revision 1 of the PHY. But the mask will
> cause the revision to be ignored. Do you want to ignore the revision?
> Are there different errata for revision 0 and 1?

A0 to A2 (Rev 1-3) are the same software wise for the current scenarios.
Z0 (Rev 0) might behave slightly different in the reset scenario but
most likely it works as well. Unfortunately, I could not test it because
I don't have such a device.

Regards,
Stefan

Re: [PATCH net-next v3 5/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2110 PHY

[Andrew Lunn]

On Tue, Jul 18, 2023 at 09:16:32AM +0200, Stefan Eichenberger wrote:
> Hi Andrew,
> 
> On Mon, Jul 17, 2023 at 11:54:39PM +0200, Andrew Lunn wrote:
> > > +#define MARVELL_PHY_ID_88Q2110	0x002b0981
> > 
> > > +
> > > +static struct phy_driver mv88q2xxx_driver[] = {
> > > +	{
> > > +		.phy_id			= MARVELL_PHY_ID_88Q2110,
> > > +		.phy_id_mask		= MARVELL_PHY_ID_MASK,
> > 
> > Probably not an issue...
> > 
> > The ID you have above is for revision 1 of the PHY. But the mask will
> > cause the revision to be ignored. Do you want to ignore the revision?
> > Are there different errata for revision 0 and 1?
> 
> A0 to A2 (Rev 1-3) are the same software wise for the current scenarios.
> Z0 (Rev 0) might behave slightly different in the reset scenario but
> most likely it works as well. Unfortunately, I could not test it because
> I don't have such a device.

Thanks.

Please consider Marek comment, but:

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

    Andrew

Re: [PATCH net-next v3 5/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2110 PHY

[Marek Behún]

On Mon, 17 Jul 2023 21:33:50 +0200
Stefan Eichenberger <eichest@gmail.com> wrote:

> +#define MARVELL_PHY_ID_88Q2110	0x002b0981

Why not put the PHY ID macro to include/linux/marvell_phy.h where all
Marvell PHY IDs reside?

Marek