Add a driver for the Marvell 88Q2220. This driver allows to detect the
link, switch between 100BASE-T1 and 1000BASE-T1 and switch between
master and slave mode. Autonegotiation is supported.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
---
 drivers/net/phy/marvell-88q2xxx.c | 210 +++++++++++++++++++++++++++++-
 include/linux/marvell_phy.h       |   1 +
 2 files changed, 205 insertions(+), 6 deletions(-)

diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
index dcebb4643aff..9829facde253 100644
--- a/drivers/net/phy/marvell-88q2xxx.c
+++ b/drivers/net/phy/marvell-88q2xxx.c
@@ -1,11 +1,17 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
  * Marvell 88Q2XXX automotive 100BASE-T1/1000BASE-T1 PHY driver
+ *
+ * Derived from Marvell Q222x API
+ *
+ * Copyright (C) 2024 Liebherr-Electronics and Drives GmbH
  */
 #include <linux/ethtool_netlink.h>
 #include <linux/marvell_phy.h>
 #include <linux/phy.h>
 
+#define PHY_ID_88Q2220_REVB0	(MARVELL_PHY_ID_88Q2220 | 0x1)
+
 #define MDIO_MMD_AN_MV_STAT			32769
 #define MDIO_MMD_AN_MV_STAT_ANEG		0x0100
 #define MDIO_MMD_AN_MV_STAT_LOCAL_RX		0x1000
@@ -13,6 +19,11 @@
 #define MDIO_MMD_AN_MV_STAT_LOCAL_MASTER	0x4000
 #define MDIO_MMD_AN_MV_STAT_MS_CONF_FAULT	0x8000
 
+#define MDIO_MMD_AN_MV_STAT2			32794
+#define MDIO_MMD_AN_MV_STAT2_AN_RESOLVED	0x0800
+#define MDIO_MMD_AN_MV_STAT2_100BT1		0x2000
+#define MDIO_MMD_AN_MV_STAT2_1000BT1		0x4000
+
 #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
@@ -29,6 +40,42 @@
 
 #define MDIO_MMD_PCS_MV_RX_STAT			33328
 
+struct mmd_val {
+	int devad;
+	u32 regnum;
+	u16 val;
+};
+
+static const struct mmd_val mv88q222x_revb0_init_seq0[] = {
+	{ MDIO_MMD_PCS, 0x8033, 0x6801 },
+	{ MDIO_MMD_AN, MDIO_AN_T1_CTRL, 0x0 },
+	{ MDIO_MMD_PMAPMD, MDIO_CTRL1,
+	  MDIO_CTRL1_LPOWER | MDIO_PMA_CTRL1_SPEED1000 },
+	{ MDIO_MMD_PCS, 0xfe1b, 0x48 },
+	{ MDIO_MMD_PCS, 0xffe4, 0x6b6 },
+	{ MDIO_MMD_PMAPMD, MDIO_CTRL1, 0x0 },
+	{ MDIO_MMD_PCS, MDIO_CTRL1, 0x0 },
+};
+
+static const struct mmd_val mv88q222x_revb0_init_seq1[] = {
+	{ MDIO_MMD_PCS, 0xfe79, 0x0 },
+	{ MDIO_MMD_PCS, 0xfe07, 0x125a },
+	{ MDIO_MMD_PCS, 0xfe09, 0x1288 },
+	{ MDIO_MMD_PCS, 0xfe08, 0x2588 },
+	{ MDIO_MMD_PCS, 0xfe11, 0x1105 },
+	{ MDIO_MMD_PCS, 0xfe72, 0x042c },
+	{ MDIO_MMD_PCS, 0xfbba, 0xcb2 },
+	{ MDIO_MMD_PCS, 0xfbbb, 0xc4a },
+	{ MDIO_MMD_AN, 0x8032, 0x2020 },
+	{ MDIO_MMD_AN, 0x8031, 0xa28 },
+	{ MDIO_MMD_AN, 0x8031, 0xc28 },
+	{ MDIO_MMD_PCS, 0xffdb, 0xfc10 },
+	{ MDIO_MMD_PCS, 0xfe1b, 0x58 },
+	{ MDIO_MMD_PCS, 0xfe79, 0x4 },
+	{ MDIO_MMD_PCS, 0xfe5f, 0xe8 },
+	{ MDIO_MMD_PCS, 0xfe05, 0x755c },
+};
+
 static int mv88q2xxx_soft_reset(struct phy_device *phydev)
 {
 	int ret;
@@ -125,24 +172,90 @@ static int mv88q2xxx_read_link_100m(struct phy_device *phydev)
 
 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);
+		return mv88q2xxx_read_link_gbit(phydev);
+	else if (phydev->speed == SPEED_100)
+		return mv88q2xxx_read_link_100m(phydev);
+
+	phydev->link = false;
+	return 0;
+}
+
+static int mv88q2xxx_read_master_slave_state(struct phy_device *phydev)
+{
+	int ret;
+
+	phydev->master_slave_state = MASTER_SLAVE_STATE_UNKNOWN;
+	ret = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_MMD_AN_MV_STAT);
+	if (ret < 0)
+		return ret;
+
+	if (ret & MDIO_MMD_AN_MV_STAT_LOCAL_MASTER)
+		phydev->master_slave_state = MASTER_SLAVE_STATE_MASTER;
 	else
-		ret = mv88q2xxx_read_link_100m(phydev);
+		phydev->master_slave_state = MASTER_SLAVE_STATE_SLAVE;
 
-	return ret;
+	return 0;
+}
+
+static int mv88q2xxx_read_aneg_speed(struct phy_device *phydev)
+{
+	int ret;
+
+	phydev->speed = SPEED_UNKNOWN;
+	ret = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_MMD_AN_MV_STAT2);
+	if (ret < 0)
+		return ret;
+
+	if (!(ret & MDIO_MMD_AN_MV_STAT2_AN_RESOLVED))
+		return 0;
+
+	if (ret & MDIO_MMD_AN_MV_STAT2_100BT1)
+		phydev->speed = SPEED_100;
+	else if (ret & MDIO_MMD_AN_MV_STAT2_1000BT1)
+		phydev->speed = SPEED_1000;
+
+	return 0;
 }
 
 static int mv88q2xxx_read_status(struct phy_device *phydev)
 {
 	int ret;
 
+	if (phydev->autoneg == AUTONEG_ENABLE) {
+		/* We have to get the negotiated speed first, otherwise we are
+		 * not able to read the link.
+		 */
+		ret = mv88q2xxx_read_aneg_speed(phydev);
+		if (ret < 0)
+			return ret;
+
+		ret = mv88q2xxx_read_link(phydev);
+		if (ret < 0)
+			return ret;
+
+		ret = genphy_c45_read_lpa(phydev);
+		if (ret < 0)
+			return ret;
+
+		ret = genphy_c45_baset1_read_status(phydev);
+		if (ret < 0)
+			return ret;
+
+		ret = mv88q2xxx_read_master_slave_state(phydev);
+		if (ret < 0)
+			return ret;
+
+		phy_resolve_aneg_linkmode(phydev);
+
+		return 0;
+	}
+
 	ret = mv88q2xxx_read_link(phydev);
 	if (ret < 0)
 		return ret;
@@ -171,7 +284,9 @@ static int mv88q2xxx_get_features(struct phy_device *phydev)
 	 * 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);
+	if (phydev->drv->phy_id == MARVELL_PHY_ID_88Q2110)
+		linkmode_clear_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
+				   phydev->supported);
 
 	return 0;
 }
@@ -241,6 +356,75 @@ static int mv88q2xxx_get_sqi_max(struct phy_device *phydev)
 	return 15;
 }
 
+static int mv88q222x_soft_reset(struct phy_device *phydev)
+{
+	int ret;
+
+	/* Enable RESET of DCL */
+	if (phydev->autoneg == AUTONEG_ENABLE || phydev->speed == SPEED_1000) {
+		ret = phy_write_mmd(phydev, MDIO_MMD_PCS, 0xfe1b, 0x48);
+		if (ret < 0)
+			return ret;
+	}
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_1000BT1_CTRL,
+			    MDIO_PCS_1000BT1_CTRL_RESET);
+	if (ret < 0)
+		return ret;
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_PCS, 0xffe4, 0xc);
+	if (ret < 0)
+		return ret;
+
+	/* Disable RESET of DCL */
+	if (phydev->autoneg == AUTONEG_ENABLE || phydev->speed == SPEED_1000)
+		return phy_write_mmd(phydev, MDIO_MMD_PCS, 0xfe1b, 0x58);
+
+	return 0;
+}
+
+static int mv88q222x_config_aneg(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = genphy_c45_config_aneg(phydev);
+	if (ret)
+		return ret;
+
+	return mv88q222x_soft_reset(phydev);
+}
+
+static int mv88q222x_revb0_config_init(struct phy_device *phydev)
+{
+	int ret, i;
+
+	for (i = 0; i < ARRAY_SIZE(mv88q222x_revb0_init_seq0); i++) {
+		ret = phy_write_mmd(phydev, mv88q222x_revb0_init_seq0[i].devad,
+				    mv88q222x_revb0_init_seq0[i].regnum,
+				    mv88q222x_revb0_init_seq0[i].val);
+		if (ret < 0)
+			return ret;
+	}
+
+	usleep_range(5000, 10000);
+
+	for (i = 0; i < ARRAY_SIZE(mv88q222x_revb0_init_seq1); i++) {
+		ret = phy_write_mmd(phydev, mv88q222x_revb0_init_seq1[i].devad,
+				    mv88q222x_revb0_init_seq1[i].regnum,
+				    mv88q222x_revb0_init_seq1[i].val);
+		if (ret < 0)
+			return 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;
+
+	return 0;
+}
+
 static struct phy_driver mv88q2xxx_driver[] = {
 	{
 		.phy_id			= MARVELL_PHY_ID_88Q2110,
@@ -255,12 +439,26 @@ static struct phy_driver mv88q2xxx_driver[] = {
 		.get_sqi		= mv88q2xxx_get_sqi,
 		.get_sqi_max		= mv88q2xxx_get_sqi_max,
 	},
+	{
+		PHY_ID_MATCH_EXACT(PHY_ID_88Q2220_REVB0),
+		.name			= "mv88q2220",
+		.get_features		= mv88q2xxx_get_features,
+		.config_aneg		= mv88q222x_config_aneg,
+		.aneg_done		= genphy_c45_aneg_done,
+		.config_init		= mv88q222x_revb0_config_init,
+		.read_status		= mv88q2xxx_read_status,
+		.soft_reset		= mv88q222x_soft_reset,
+		.set_loopback		= genphy_c45_loopback,
+		.get_sqi		= mv88q2xxx_get_sqi,
+		.get_sqi_max		= mv88q2xxx_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 },
+	{ PHY_ID_MATCH_EXACT(PHY_ID_88Q2220_REVB0), },
 	{ /*sentinel*/ }
 };
 MODULE_DEVICE_TABLE(mdio, mv88q2xxx_tbl);
diff --git a/include/linux/marvell_phy.h b/include/linux/marvell_phy.h
index 9b54c4f0677f..693eba9869e4 100644
--- a/include/linux/marvell_phy.h
+++ b/include/linux/marvell_phy.h
@@ -26,6 +26,7 @@
 #define MARVELL_PHY_ID_88E2110		0x002b09b0
 #define MARVELL_PHY_ID_88X2222		0x01410f10
 #define MARVELL_PHY_ID_88Q2110		0x002b0980
+#define MARVELL_PHY_ID_88Q2220		0x002b0b20
 
 /* Marvel 88E1111 in Finisar SFP module with modified PHY ID */
 #define MARVELL_PHY_ID_88E1111_FINISAR	0x01ff0cc0
-- 
2.39.2

Hi Dimitri,

On Tue, Feb 13, 2024 at 10:39:44PM +0100, Dimitri Fedrau wrote:
>  static struct phy_driver mv88q2xxx_driver[] = {
>  	{
>  		.phy_id			= MARVELL_PHY_ID_88Q2110,
> @@ -255,12 +439,26 @@ static struct phy_driver mv88q2xxx_driver[] = {
>  		.get_sqi		= mv88q2xxx_get_sqi,
>  		.get_sqi_max		= mv88q2xxx_get_sqi_max,
>  	},
> +	{
> +		PHY_ID_MATCH_EXACT(PHY_ID_88Q2220_REVB0),

I tested the series on a 88Q2220 REV B1 (which is id 0x002b0b22). The
driver works fine on this revision.

I understand that in the Marvell API the initialization for Rev B0 and
B1 differ. For B0 some additional init sequence is executed. I did not look
into the details of this sequence. However this patch seems to work on
Rev B1.

Would you consider adding compatibility for Rev B1 and following? I
tested with:
		.phy_id			= MARVELL_PHY_ID_88Q2220,
		.phy_id_mask		= MARVELL_PHY_ID_MASK,

Otherwise:

Tested-by: Gregor Herburger <gregor.herburger@ew.tq-group.com>

Best regards
Gregor
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/

Am Wed, Feb 14, 2024 at 02:20:37PM +0100 schrieb Gregor Herburger:
> Hi Dimitri,
>
Hi Gregor,

> On Tue, Feb 13, 2024 at 10:39:44PM +0100, Dimitri Fedrau wrote:
> >  static struct phy_driver mv88q2xxx_driver[] = {
> >  	{
> >  		.phy_id			= MARVELL_PHY_ID_88Q2110,
> > @@ -255,12 +439,26 @@ static struct phy_driver mv88q2xxx_driver[] = {
> >  		.get_sqi		= mv88q2xxx_get_sqi,
> >  		.get_sqi_max		= mv88q2xxx_get_sqi_max,
> >  	},
> > +	{
> > +		PHY_ID_MATCH_EXACT(PHY_ID_88Q2220_REVB0),
> 
> I tested the series on a 88Q2220 REV B1 (which is id 0x002b0b22). The
> driver works fine on this revision.
> 
> I understand that in the Marvell API the initialization for Rev B0 and
> B1 differ. For B0 some additional init sequence is executed. I did not look
> into the details of this sequence. However this patch seems to work on
> Rev B1.
>
> Would you consider adding compatibility for Rev B1 and following? I
> tested with:
> 		.phy_id			= MARVELL_PHY_ID_88Q2220,
> 		.phy_id_mask		= MARVELL_PHY_ID_MASK,
>

thanks for testing. I would stick to the exact initialization sequence
provided by the Marvell API. Registers and bits are mostly undocumented
and I think it is safest this way. Besides that it should be relatively
easy to add the support for rev. B1 by just adding the init sequence for
it.

Best regards,
Dimitri Fedrau

On Thu, Feb 15, 2024 at 09:24:03PM +0100, Dimitri Fedrau wrote:
> > Hi Dimitri,
> >
> Hi Gregor,
> 
> > On Tue, Feb 13, 2024 at 10:39:44PM +0100, Dimitri Fedrau wrote:
> > >  static struct phy_driver mv88q2xxx_driver[] = {
> > >  	{
> > >  		.phy_id			= MARVELL_PHY_ID_88Q2110,
> > > @@ -255,12 +439,26 @@ static struct phy_driver mv88q2xxx_driver[] = {
> > >  		.get_sqi		= mv88q2xxx_get_sqi,
> > >  		.get_sqi_max		= mv88q2xxx_get_sqi_max,
> > >  	},
> > > +	{
> > > +		PHY_ID_MATCH_EXACT(PHY_ID_88Q2220_REVB0),
> > 
> > I tested the series on a 88Q2220 REV B1 (which is id 0x002b0b22). The
> > driver works fine on this revision.
> > 
> > I understand that in the Marvell API the initialization for Rev B0 and
> > B1 differ. For B0 some additional init sequence is executed. I did not look
> > into the details of this sequence. However this patch seems to work on
> > Rev B1.
> >
> > Would you consider adding compatibility for Rev B1 and following? I
> > tested with:
> > 		.phy_id			= MARVELL_PHY_ID_88Q2220,
> > 		.phy_id_mask		= MARVELL_PHY_ID_MASK,
> >
> 
> thanks for testing. I would stick to the exact initialization sequence
> provided by the Marvell API. Registers and bits are mostly undocumented
> and I think it is safest this way. Besides that it should be relatively
> easy to add the support for rev. B1 by just adding the init sequence for
> it.

Ok. I will have an closer look at the marvell API and eventually come up
with a patch for Rev. B1.

There is also a Rev.B2 for which I cannot find any init sequence. But
Rev. B1 will no longer be produced so I need a solution for B2
eventually.

Best regards
Gregor

-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/

Am Fri, Feb 16, 2024 at 10:18:10AM +0100 schrieb Gregor Herburger:
> On Thu, Feb 15, 2024 at 09:24:03PM +0100, Dimitri Fedrau wrote:
> > > Hi Dimitri,
> > >
> > Hi Gregor,
> > 
> > > On Tue, Feb 13, 2024 at 10:39:44PM +0100, Dimitri Fedrau wrote:
> > > >  static struct phy_driver mv88q2xxx_driver[] = {
> > > >  	{
> > > >  		.phy_id			= MARVELL_PHY_ID_88Q2110,
> > > > @@ -255,12 +439,26 @@ static struct phy_driver mv88q2xxx_driver[] = {
> > > >  		.get_sqi		= mv88q2xxx_get_sqi,
> > > >  		.get_sqi_max		= mv88q2xxx_get_sqi_max,
> > > >  	},
> > > > +	{
> > > > +		PHY_ID_MATCH_EXACT(PHY_ID_88Q2220_REVB0),
> > > 
> > > I tested the series on a 88Q2220 REV B1 (which is id 0x002b0b22). The
> > > driver works fine on this revision.
> > > 
> > > I understand that in the Marvell API the initialization for Rev B0 and
> > > B1 differ. For B0 some additional init sequence is executed. I did not look
> > > into the details of this sequence. However this patch seems to work on
> > > Rev B1.
> > >
> > > Would you consider adding compatibility for Rev B1 and following? I
> > > tested with:
> > > 		.phy_id			= MARVELL_PHY_ID_88Q2220,
> > > 		.phy_id_mask		= MARVELL_PHY_ID_MASK,
> > >
> > 
> > thanks for testing. I would stick to the exact initialization sequence
> > provided by the Marvell API. Registers and bits are mostly undocumented
> > and I think it is safest this way. Besides that it should be relatively
> > easy to add the support for rev. B1 by just adding the init sequence for
> > it.
> 
> Ok. I will have an closer look at the marvell API and eventually come up
> with a patch for Rev. B1.
> 
> There is also a Rev.B2 for which I cannot find any init sequence. But
> Rev. B1 will no longer be produced so I need a solution for B2
> eventually.
>
After having a quick glance at the latest Marvell API release, the init
sequences for B1 and B2 are almost the same. It differs by a single
register write. Would be great if you can come up with a patch.

Dimitri