[PATCH] net: phy: add Marvell 88X2222 transceiver support

[PATCH] net: phy: add Marvell 88X2222 transceiver support

[Ivan Bornyakov]

Add basic support for the Marvell 88X2222 multi-speed ethernet
transceiver.

This PHY provides data transmission over fiber-optic as well as Twinax
copper links. The 88X2222 supports 2 ports of 10GBase-R and 1000Base-X
on the line-side interface. The host-side interface supports 4 ports of
10GBase-R, RXAUI, 1000Base-X and 2 ports of XAUI.

This driver, however, supports only XAUI on the host-side and
1000Base-X/10GBase-R on the line-side, for now. Interrupts are not
supported also.

Internal registers access compliant with the Clause 45 specification.

Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
---
 drivers/net/phy/Kconfig           |   6 +
 drivers/net/phy/Makefile          |   1 +
 drivers/net/phy/marvell-88x2222.c | 480 ++++++++++++++++++++++++++++++
 include/linux/marvell_phy.h       |   1 +
 4 files changed, 488 insertions(+)
 create mode 100644 drivers/net/phy/marvell-88x2222.c

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 698bea312adc..a615b3660b05 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -201,6 +201,12 @@ config MARVELL_10G_PHY
 	help
 	  Support for the Marvell Alaska MV88X3310 and compatible PHYs.
 
+config MARVELL_88X2222_PHY
+	tristate "Marvell 88X2222 PHY"
+	help
+	  Support for the Marvell 88X2222 Dual-port Multi-speed Ethernet
+	  Transceiver.
+
 config MICREL_PHY
 	tristate "Micrel PHYs"
 	help
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index a13e402074cf..de683e3abe63 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -63,6 +63,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_88X2222_PHY)	+= marvell-88x2222.o
 obj-$(CONFIG_MESON_GXL_PHY)	+= meson-gxl.o
 obj-$(CONFIG_MICREL_KS8995MA)	+= spi_ks8995.o
 obj-$(CONFIG_MICREL_PHY)	+= micrel.o
diff --git a/drivers/net/phy/marvell-88x2222.c b/drivers/net/phy/marvell-88x2222.c
new file mode 100644
index 000000000000..e2c55db4769f
--- /dev/null
+++ b/drivers/net/phy/marvell-88x2222.c
@@ -0,0 +1,480 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Marvell 88x2222 dual-port multi-speed ethernet transceiver.
+ *
+ * Supports:
+ *	XAUI on the host side.
+ *	1000Base-X or 10GBase-R on the line side.
+ */
+#include <linux/module.h>
+#include <linux/phy.h>
+#include <linux/gpio.h>
+#include <linux/delay.h>
+#include <linux/mdio.h>
+#include <linux/marvell_phy.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/sfp.h>
+#include <linux/netdevice.h>
+
+/* Port PCS Configuration */
+#define	MV_PCS_CONFIG		0xF002
+#define	MV_PCS_HOST_XAUI	0x73
+#define	MV_PCS_LINE_10GBR	(0x71 << 8)
+#define	MV_PCS_LINE_1GBX_AN	(0x7B << 8)
+
+/* Port Reset and Power Down */
+#define	MV_PORT_RST	0xF003
+#define	MV_LINE_RST_SW	BIT(15)
+#define	MV_HOST_RST_SW	BIT(7)
+#define	MV_PORT_RST_SW	(MV_LINE_RST_SW | MV_HOST_RST_SW)
+
+/* LED0 Control */
+#define	MV_LED0_CTRL		0xF020
+#define	MV_LED0_SOLID_MASK	(0xf << 4)
+#define	MV_LED0_SOLID_OFF	(0x0 << 4)
+#define	MV_LED0_SOLID_ON	(0x7 << 4)
+
+/* PMD Transmit Disable */
+#define	MV_TX_DISABLE		0x0009
+#define	MV_TX_DISABLE_GLOBAL	BIT(0)
+
+/* 10GBASE-R PCS Status 1 */
+#define	MV_10GBR_STAT		MDIO_STAT1
+
+/* 10GBASE-R PCS Real Time Status Register */
+#define	MV_10GBR_STAT_RT	0x8002
+
+/* 1000Base-X/SGMII Control Register */
+#define	MV_1GBX_CTRL		0x2000
+
+/* 1000BASE-X/SGMII Status Register */
+#define	MV_1GBX_STAT		0x2001
+
+/* 1000Base-X Auto-Negotiation Advertisement Register */
+#define	MV_1GBX_ADVERTISE	0x2004
+
+/* 1000Base-X PHY Specific Status Register */
+#define	MV_1GBX_PHY_STAT		0xA003
+#define	MV_1GBX_PHY_STAT_LSTATUS_RT	BIT(10)
+#define	MV_1GBX_PHY_STAT_AN_RESOLVED	BIT(11)
+#define	MV_1GBX_PHY_STAT_DUPLEX		BIT(13)
+#define	MV_1GBX_PHY_STAT_SPEED100	BIT(14)
+#define	MV_1GBX_PHY_STAT_SPEED1000	BIT(15)
+
+struct mv2222_data {
+	bool sfp_inserted;
+	bool net_up;
+};
+
+/* SFI PMA transmit enable */
+static void mv2222_tx_enable(struct phy_device *phydev)
+{
+	phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD, MV_TX_DISABLE,
+			   MV_TX_DISABLE_GLOBAL);
+}
+
+/* SFI PMA transmit disable */
+static void mv2222_tx_disable(struct phy_device *phydev)
+{
+	phy_set_bits_mmd(phydev, MDIO_MMD_PMAPMD, MV_TX_DISABLE,
+			 MV_TX_DISABLE_GLOBAL);
+}
+
+static void mv2222_link_led_on(struct phy_device *phydev)
+{
+	phy_modify_mmd(phydev, MDIO_MMD_VEND2, MV_LED0_CTRL, MV_LED0_SOLID_MASK,
+		       MV_LED0_SOLID_ON);
+}
+
+static void mv2222_link_led_off(struct phy_device *phydev)
+{
+	phy_modify_mmd(phydev, MDIO_MMD_VEND2, MV_LED0_CTRL, MV_LED0_SOLID_MASK,
+		       MV_LED0_SOLID_OFF);
+}
+
+static int mv2222_soft_reset(struct phy_device *phydev)
+{
+	int ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PORT_RST, MV_PORT_RST_SW);
+
+	msleep(2000);
+
+	return ret;
+}
+
+static int sfp_module_insert(void *_priv, const struct sfp_eeprom_id *id)
+{
+	struct phy_device *phydev = _priv;
+	struct device *dev = &phydev->mdio.dev;
+	struct mv2222_data *priv = phydev->priv;
+	phy_interface_t interface;
+
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported) = { 0, };
+
+	sfp_parse_support(phydev->sfp_bus, id, supported);
+	interface = sfp_select_interface(phydev->sfp_bus, supported);
+
+	dev_info(dev, "%s SFP module inserted", phy_modes(interface));
+
+	switch (interface) {
+	case PHY_INTERFACE_MODE_10GBASER:
+		phydev->speed = SPEED_10000;
+		phydev->interface = PHY_INTERFACE_MODE_10GBASER;
+		linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+				 phydev->supported);
+
+		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
+			      MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
+		mv2222_soft_reset(phydev);
+		break;
+	case PHY_INTERFACE_MODE_1000BASEX:
+	default:
+		phydev->speed = SPEED_1000;
+		phydev->interface = PHY_INTERFACE_MODE_1000BASEX;
+		linkmode_clear_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+				   phydev->supported);
+
+		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
+			      MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
+		mv2222_soft_reset(phydev);
+	}
+
+	priv->sfp_inserted = true;
+
+	if (priv->net_up)
+		mv2222_tx_enable(phydev);
+
+	return 0;
+}
+
+static void sfp_module_remove(void *_priv)
+{
+	struct phy_device *phydev = _priv;
+	struct mv2222_data *priv = phydev->priv;
+
+	priv->sfp_inserted = false;
+	mv2222_tx_disable(phydev);
+}
+
+static const struct sfp_upstream_ops sfp_phy_ops = {
+	.module_insert = sfp_module_insert,
+	.module_remove = sfp_module_remove,
+	.attach = phy_sfp_attach,
+	.detach = phy_sfp_detach,
+};
+
+static int mv2222_config_init(struct phy_device *phydev)
+{
+	linkmode_zero(phydev->supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, phydev->supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, phydev->supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT, phydev->supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT, phydev->supported);
+
+	phydev->pause = 0;
+	phydev->asym_pause = 0;
+	phydev->duplex = DUPLEX_FULL;
+	phydev->autoneg = AUTONEG_DISABLE;
+
+	return 0;
+}
+
+static void mv2222_update_interface(struct phy_device *phydev)
+{
+	if ((phydev->speed == SPEED_1000 ||
+	     phydev->speed == SPEED_100 ||
+	     phydev->speed == SPEED_10) &&
+	    phydev->interface != PHY_INTERFACE_MODE_1000BASEX) {
+		phydev->interface = PHY_INTERFACE_MODE_1000BASEX;
+
+		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
+			      MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
+		mv2222_soft_reset(phydev);
+	} else if (phydev->speed == SPEED_10000 &&
+		   phydev->interface != PHY_INTERFACE_MODE_10GBASER) {
+		phydev->interface = PHY_INTERFACE_MODE_10GBASER;
+
+		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
+			      MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
+		mv2222_soft_reset(phydev);
+	}
+}
+
+/* Returns negative on error, 0 if link is down, 1 if link is up */
+static int mv2222_read_status_10g(struct phy_device *phydev)
+{
+	int val, link = 0;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_10GBR_STAT);
+	if (val < 0)
+		return val;
+
+	if (val & MDIO_STAT1_LSTATUS) {
+		link = 1;
+
+		/* 10GBASE-R do not support auto-negotiation */
+		phydev->autoneg = AUTONEG_DISABLE;
+		phydev->speed = SPEED_10000;
+		phydev->duplex = DUPLEX_FULL;
+	}
+
+	return link;
+}
+
+/* Returns negative on error, 0 if link is down, 1 if link is up */
+static int mv2222_read_status_1g(struct phy_device *phydev)
+{
+	int val, link = 0;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_STAT);
+	if (val < 0)
+		return val;
+
+	if (!(val & MDIO_STAT1_LSTATUS) ||
+	    (phydev->autoneg == AUTONEG_ENABLE && !(val & MDIO_AN_STAT1_COMPLETE)))
+		return 0;
+
+	link = 1;
+
+	if (phydev->autoneg == AUTONEG_DISABLE) {
+		phydev->speed = SPEED_1000;
+		phydev->duplex = DUPLEX_FULL;
+
+		return link;
+	}
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_PHY_STAT);
+	if (val < 0)
+		return val;
+
+	if (val & MV_1GBX_PHY_STAT_AN_RESOLVED) {
+		if (val & MV_1GBX_PHY_STAT_DUPLEX)
+			phydev->duplex = DUPLEX_FULL;
+		else
+			phydev->duplex = DUPLEX_HALF;
+
+		if (val & MV_1GBX_PHY_STAT_SPEED1000)
+			phydev->speed = SPEED_1000;
+		else if (val & MV_1GBX_PHY_STAT_SPEED100)
+			phydev->speed = SPEED_100;
+		else
+			phydev->speed = SPEED_10;
+	} else {
+		phydev->duplex = DUPLEX_UNKNOWN;
+		phydev->speed = SPEED_UNKNOWN;
+	}
+
+	return link;
+}
+
+static int mv2222_read_status(struct phy_device *phydev)
+{
+	int link;
+
+	linkmode_zero(phydev->lp_advertising);
+	phydev->pause = 0;
+	phydev->asym_pause = 0;
+
+	switch (phydev->interface) {
+	case PHY_INTERFACE_MODE_10GBASER:
+		link = mv2222_read_status_10g(phydev);
+		break;
+	case PHY_INTERFACE_MODE_1000BASEX:
+	default:
+		link = mv2222_read_status_1g(phydev);
+		break;
+	}
+
+	if (link < 0)
+		return link;
+
+	phydev->link = link;
+
+	if (phydev->link)
+		mv2222_link_led_on(phydev);
+	else
+		mv2222_link_led_off(phydev);
+
+	return 0;
+}
+
+static int mv2222_disable_aneg(struct phy_device *phydev)
+{
+	return phy_clear_bits_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
+				  MDIO_AN_CTRL1_ENABLE | MDIO_AN_CTRL1_RESTART);
+}
+
+static int mv2222_enable_aneg(struct phy_device *phydev)
+{
+	return phy_set_bits_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
+				MDIO_AN_CTRL1_ENABLE | MDIO_CTRL1_RESET);
+}
+
+static int mv2222_config_aneg(struct phy_device *phydev)
+{
+	int ret, adv;
+
+	if (phydev->autoneg == AUTONEG_DISABLE ||
+	    phydev->speed == SPEED_10000) {
+		if (phydev->speed == SPEED_10000 &&
+		    !linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+				      phydev->supported))
+			return -EINVAL;
+
+		/* Link partner advertising modes */
+		linkmode_copy(phydev->advertising, phydev->supported);
+
+		mv2222_update_interface(phydev);
+
+		return mv2222_disable_aneg(phydev);
+	}
+
+	/* Try 10G first */
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+			      phydev->supported)) {
+		phydev->speed = SPEED_10000;
+		mv2222_update_interface(phydev);
+
+		ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_10GBR_STAT_RT);
+		if (ret < 0)
+			return ret;
+
+		if (ret & MDIO_STAT1_LSTATUS) {
+			phydev->autoneg = AUTONEG_DISABLE;
+			linkmode_copy(phydev->advertising, phydev->supported);
+
+			return mv2222_disable_aneg(phydev);
+		} else {
+			phydev->speed = SPEED_1000;
+			mv2222_update_interface(phydev);
+		}
+	}
+
+	adv = 0;
+	linkmode_zero(phydev->advertising);
+
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
+			      phydev->supported)) {
+		adv |= ADVERTISE_1000XFULL;
+		linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
+				 phydev->advertising);
+	}
+
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
+			      phydev->supported)) {
+		adv |= ADVERTISE_1000XPAUSE;
+		linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT,
+				 phydev->advertising);
+	}
+
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+			      phydev->supported)) {
+		adv |= ADVERTISE_1000XPSE_ASYM;
+		linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+				 phydev->advertising);
+	}
+
+	ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_ADVERTISE,
+			     ADVERTISE_1000XFULL |
+			     ADVERTISE_1000XPAUSE | ADVERTISE_1000XPSE_ASYM,
+			     adv);
+	if (ret < 0)
+		return ret;
+
+	return mv2222_enable_aneg(phydev);
+}
+
+static int mv2222_aneg_done(struct phy_device *phydev)
+{
+	int ret;
+
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+			      phydev->supported)) {
+		ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_10GBR_STAT);
+		if (ret < 0)
+			return ret;
+
+		if (ret & MDIO_STAT1_LSTATUS)
+			return 1;
+	}
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_STAT);
+	if (ret < 0)
+		return ret;
+
+	return (ret & MDIO_AN_STAT1_COMPLETE);
+}
+
+static int mv2222_resume(struct phy_device *phydev)
+{
+	struct mv2222_data *priv = phydev->priv;
+
+	priv->net_up = true;
+
+	if (priv->sfp_inserted)
+		mv2222_tx_enable(phydev);
+
+	return 0;
+}
+
+static int mv2222_suspend(struct phy_device *phydev)
+{
+	struct mv2222_data *priv = phydev->priv;
+
+	priv->net_up = false;
+	mv2222_tx_disable(phydev);
+	mv2222_link_led_off(phydev);
+
+	return 0;
+}
+
+static int mv2222_probe(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct mv2222_data *priv = NULL;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	phydev->priv = priv;
+
+	return phy_sfp_probe(phydev, &sfp_phy_ops);
+}
+
+static void mv2222_remove(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct mv2222_data *priv = phydev->priv;
+
+	if (priv)
+		devm_kfree(dev, priv);
+}
+
+static struct phy_driver mv2222_drivers[] = {
+	{
+		.phy_id = MARVELL_PHY_ID_88X2222,
+		.phy_id_mask = MARVELL_PHY_ID_MASK,
+		.name = "Marvell 88X2222",
+		.soft_reset = mv2222_soft_reset,
+		.config_init = mv2222_config_init,
+		.config_aneg = mv2222_config_aneg,
+		.aneg_done = mv2222_aneg_done,
+		.probe = mv2222_probe,
+		.remove = mv2222_remove,
+		.suspend = mv2222_suspend,
+		.resume = mv2222_resume,
+		.read_status = mv2222_read_status,
+	},
+};
+module_phy_driver(mv2222_drivers);
+
+static struct mdio_device_id __maybe_unused mv2222_tbl[] = {
+	{ MARVELL_PHY_ID_88X2222, MARVELL_PHY_ID_MASK },
+	{ }
+};
+MODULE_DEVICE_TABLE(mdio, mv2222_tbl);
+
+MODULE_DESCRIPTION("Marvell 88x2222 ethernet transceiver driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/marvell_phy.h b/include/linux/marvell_phy.h
index 52b1610eae68..274abd5fbac3 100644
--- a/include/linux/marvell_phy.h
+++ b/include/linux/marvell_phy.h
@@ -24,6 +24,7 @@
 #define MARVELL_PHY_ID_88E3016		0x01410e60
 #define MARVELL_PHY_ID_88X3310		0x002b09a0
 #define MARVELL_PHY_ID_88E2110		0x002b09b0
+#define MARVELL_PHY_ID_88X2222		0x01410f10
 
 /* Marvel 88E1111 in Finisar SFP module with modified PHY ID */
 #define MARVELL_PHY_ID_88E1111_FINISAR	0x01ff0cc0
-- 
2.20.1

Re: [PATCH] net: phy: add Marvell 88X2222 transceiver support

[Andrew Lunn]

> +static int mv2222_config_init(struct phy_device *phydev)
> +{
> +	linkmode_zero(phydev->supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, phydev->supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, phydev->supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT, phydev->supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT, phydev->supported);
> +
> +	phydev->pause = 0;
> +	phydev->asym_pause = 0;
> +	phydev->duplex = DUPLEX_FULL;
> +	phydev->autoneg = AUTONEG_DISABLE;
> +
> +	return 0;
> +}
> +
> +static void mv2222_update_interface(struct phy_device *phydev)
> +{
> +	if ((phydev->speed == SPEED_1000 ||
> +	     phydev->speed == SPEED_100 ||
> +	     phydev->speed == SPEED_10) &&
> +	    phydev->interface != PHY_INTERFACE_MODE_1000BASEX) {
> +		phydev->interface = PHY_INTERFACE_MODE_1000BASEX;

The speeds 10 and 100 seem odd here. 1000BaseX only supports 1G. It
would have to be SGMII in order to also support 10Mbps and 100Mbps.
Plus you are not listing 10 and 100 as a supported value.

> +/* Returns negative on error, 0 if link is down, 1 if link is up */
> +static int mv2222_read_status_1g(struct phy_device *phydev)
> +{
> +	int val, link = 0;
> +
> +	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_STAT);
> +	if (val < 0)
> +		return val;
> +
> +	if (!(val & MDIO_STAT1_LSTATUS) ||
> +	    (phydev->autoneg == AUTONEG_ENABLE && !(val & MDIO_AN_STAT1_COMPLETE)))
> +		return 0;
> +
> +	link = 1;
> +
> +	if (phydev->autoneg == AUTONEG_DISABLE) {
> +		phydev->speed = SPEED_1000;
> +		phydev->duplex = DUPLEX_FULL;
> +
> +		return link;
> +	}
> +
> +	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_PHY_STAT);
> +	if (val < 0)
> +		return val;
> +
> +	if (val & MV_1GBX_PHY_STAT_AN_RESOLVED) {
> +		if (val & MV_1GBX_PHY_STAT_DUPLEX)
> +			phydev->duplex = DUPLEX_FULL;
> +		else
> +			phydev->duplex = DUPLEX_HALF;
> +
> +		if (val & MV_1GBX_PHY_STAT_SPEED1000)
> +			phydev->speed = SPEED_1000;
> +		else if (val & MV_1GBX_PHY_STAT_SPEED100)
> +			phydev->speed = SPEED_100;
> +		else
> +			phydev->speed = SPEED_10;

Are you sure it is not doing SGMII? Maybe it can do both, 1000BaseX
and SGMII? You would generally use 1000BaseX to connect to a fibre
SFP, and SGMII to connect to a copper SFP. So ideally you want to be
able to swap between these modes as needed.

> +static int mv2222_read_status(struct phy_device *phydev)
> +{
> +	int link;
> +
> +	linkmode_zero(phydev->lp_advertising);
> +	phydev->pause = 0;
> +	phydev->asym_pause = 0;
> +
> +	switch (phydev->interface) {
> +	case PHY_INTERFACE_MODE_10GBASER:
> +		link = mv2222_read_status_10g(phydev);
> +		break;
> +	case PHY_INTERFACE_MODE_1000BASEX:
> +	default:
> +		link = mv2222_read_status_1g(phydev);
> +		break;
> +	}
> +
> +	if (link < 0)
> +		return link;
> +
> +	phydev->link = link;
> +
> +	if (phydev->link)
> +		mv2222_link_led_on(phydev);
> +	else
> +		mv2222_link_led_off(phydev);

You have to manually control the LED? That is odd for a PHY. Normally
you just select a mode and it will do it all in hardware.

    Andrew

Re: [PATCH] net: phy: add Marvell 88X2222 transceiver support

[Ivan Bornyakov]

On Mon, Feb 01, 2021 at 11:56:01PM +0100, Andrew Lunn wrote:
> > +static int mv2222_config_init(struct phy_device *phydev)
> > +{
> > +	linkmode_zero(phydev->supported);
> > +	linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, phydev->supported);
> > +	linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, phydev->supported);
> > +	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT, phydev->supported);
> > +	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT, phydev->supported);
> > +
> > +	phydev->pause = 0;
> > +	phydev->asym_pause = 0;
> > +	phydev->duplex = DUPLEX_FULL;
> > +	phydev->autoneg = AUTONEG_DISABLE;
> > +
> > +	return 0;
> > +}
> > +
> > +static void mv2222_update_interface(struct phy_device *phydev)
> > +{
> > +	if ((phydev->speed == SPEED_1000 ||
> > +	     phydev->speed == SPEED_100 ||
> > +	     phydev->speed == SPEED_10) &&
> > +	    phydev->interface != PHY_INTERFACE_MODE_1000BASEX) {
> > +		phydev->interface = PHY_INTERFACE_MODE_1000BASEX;
> 
> The speeds 10 and 100 seem odd here. 1000BaseX only supports 1G. It
> would have to be SGMII in order to also support 10Mbps and 100Mbps.
> Plus you are not listing 10 and 100 as a supported value.
> 
> > +/* Returns negative on error, 0 if link is down, 1 if link is up */
> > +static int mv2222_read_status_1g(struct phy_device *phydev)
> > +{
> > +	int val, link = 0;
> > +
> > +	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_STAT);
> > +	if (val < 0)
> > +		return val;
> > +
> > +	if (!(val & MDIO_STAT1_LSTATUS) ||
> > +	    (phydev->autoneg == AUTONEG_ENABLE && !(val & MDIO_AN_STAT1_COMPLETE)))
> > +		return 0;
> > +
> > +	link = 1;
> > +
> > +	if (phydev->autoneg == AUTONEG_DISABLE) {
> > +		phydev->speed = SPEED_1000;
> > +		phydev->duplex = DUPLEX_FULL;
> > +
> > +		return link;
> > +	}
> > +
> > +	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_PHY_STAT);
> > +	if (val < 0)
> > +		return val;
> > +
> > +	if (val & MV_1GBX_PHY_STAT_AN_RESOLVED) {
> > +		if (val & MV_1GBX_PHY_STAT_DUPLEX)
> > +			phydev->duplex = DUPLEX_FULL;
> > +		else
> > +			phydev->duplex = DUPLEX_HALF;
> > +
> > +		if (val & MV_1GBX_PHY_STAT_SPEED1000)
> > +			phydev->speed = SPEED_1000;
> > +		else if (val & MV_1GBX_PHY_STAT_SPEED100)
> > +			phydev->speed = SPEED_100;
> > +		else
> > +			phydev->speed = SPEED_10;
> 
> Are you sure it is not doing SGMII? Maybe it can do both, 1000BaseX
> and SGMII? You would generally use 1000BaseX to connect to a fibre
> SFP, and SGMII to connect to a copper SFP. So ideally you want to be
> able to swap between these modes as needed.
> 
> > +static int mv2222_read_status(struct phy_device *phydev)
> > +{
> > +	int link;
> > +
> > +	linkmode_zero(phydev->lp_advertising);
> > +	phydev->pause = 0;
> > +	phydev->asym_pause = 0;
> > +
> > +	switch (phydev->interface) {
> > +	case PHY_INTERFACE_MODE_10GBASER:
> > +		link = mv2222_read_status_10g(phydev);
> > +		break;
> > +	case PHY_INTERFACE_MODE_1000BASEX:
> > +	default:
> > +		link = mv2222_read_status_1g(phydev);
> > +		break;
> > +	}
> > +
> > +	if (link < 0)
> > +		return link;
> > +
> > +	phydev->link = link;
> > +
> > +	if (phydev->link)
> > +		mv2222_link_led_on(phydev);
> > +	else
> > +		mv2222_link_led_off(phydev);
> 
> You have to manually control the LED? That is odd for a PHY. Normally
> you just select a mode and it will do it all in hardware.
> 
>     Andrew

Thanks for the review, Andrew, your remarks are all valid. I'll
implement 1000Base-X/SGMII swapping. As for the LED, I'll just drop it
for now, as it is not essential. I'll commit LEDs config when core
funtionality will be accepted.

Re: [PATCH] net: phy: add Marvell 88X2222 transceiver support

[Russell King - ARM Linux admin]

On Mon, Feb 01, 2021 at 10:22:51PM +0300, Ivan Bornyakov wrote:
> +/* PMD Transmit Disable */
> +#define	MV_TX_DISABLE		0x0009
> +#define	MV_TX_DISABLE_GLOBAL	BIT(0)

Please use MDIO_PMA_TXDIS and MDIO_PMD_TXDIS_GLOBAL; this is an
IEEE802.3 defined register.

> +/* 10GBASE-R PCS Status 1 */
> +#define	MV_10GBR_STAT		MDIO_STAT1

Nothing Marvell specific here, please use MDIO_STAT1 directly.

> +/* 1000Base-X/SGMII Control Register */
> +#define	MV_1GBX_CTRL		0x2000
> +
> +/* 1000BASE-X/SGMII Status Register */
> +#define	MV_1GBX_STAT		0x2001
> +
> +/* 1000Base-X Auto-Negotiation Advertisement Register */
> +#define	MV_1GBX_ADVERTISE	0x2004

Marvell have had a habbit of placing other PHY instances within the
register space. This also looks like Clause 22 layout rather than
Clause 45 layout - please use the Clause 22 definitions for the bits
rather than Clause 45. (so BMCR_ANENABLE, BMCR_ANRESTART for
MV_1GBX_CTRL, etc).

Please define these as:

+#define	MV_1GBX_CTRL		(0x2000 + MII_BMCR)
+#define	MV_1GBX_STAT		(0x2000 + MII_BMSR)
+#define	MV_1GBX_ADVERTISE	(0x2000 + MII_ADVERTISE)

to make it clear what is going on here.

> +static int sfp_module_insert(void *_priv, const struct sfp_eeprom_id *id)
> +{
> +	struct phy_device *phydev = _priv;
> +	struct device *dev = &phydev->mdio.dev;
> +	struct mv2222_data *priv = phydev->priv;
> +	phy_interface_t interface;
> +
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported) = { 0, };
> +
> +	sfp_parse_support(phydev->sfp_bus, id, supported);
> +	interface = sfp_select_interface(phydev->sfp_bus, supported);
> +
> +	dev_info(dev, "%s SFP module inserted", phy_modes(interface));
> +
> +	switch (interface) {
> +	case PHY_INTERFACE_MODE_10GBASER:
> +		phydev->speed = SPEED_10000;
> +		phydev->interface = PHY_INTERFACE_MODE_10GBASER;
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
> +				 phydev->supported);
> +
> +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
> +		mv2222_soft_reset(phydev);
> +		break;
> +	case PHY_INTERFACE_MODE_1000BASEX:
> +	default:
> +		phydev->speed = SPEED_1000;
> +		phydev->interface = PHY_INTERFACE_MODE_1000BASEX;
> +		linkmode_clear_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
> +				   phydev->supported);
> +
> +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
> +		mv2222_soft_reset(phydev);
> +	}
> +
> +	priv->sfp_inserted = true;
> +
> +	if (priv->net_up)
> +		mv2222_tx_enable(phydev);

This is racy. priv->net_up is modified via the suspend/resume
callbacks, which are called with phydev->lock held. No other locks
are guaranteed to be held.

However, this function is called with the SFP sm_mutex, and rtnl
held. Consequently, the use of sfp_inserted and net_up in this
function and the suspend/resume callbacks is racy.

Why are you disabling the transmitter anyway? Is this for power
saving?

> +static void mv2222_update_interface(struct phy_device *phydev)
> +{
> +	if ((phydev->speed == SPEED_1000 ||
> +	     phydev->speed == SPEED_100 ||
> +	     phydev->speed == SPEED_10) &&
> +	    phydev->interface != PHY_INTERFACE_MODE_1000BASEX) {
> +		phydev->interface = PHY_INTERFACE_MODE_1000BASEX;
> +
> +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
> +		mv2222_soft_reset(phydev);
> +	} else if (phydev->speed == SPEED_10000 &&
> +		   phydev->interface != PHY_INTERFACE_MODE_10GBASER) {
> +		phydev->interface = PHY_INTERFACE_MODE_10GBASER;
> +
> +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
> +		mv2222_soft_reset(phydev);
> +	}

This looks wrong. phydev->interface is the _host_ interface, which
you are clearly setting to XAUI here. Some network drivers depend
on this being correct (for instance, when used with the Marvell
88x3310 PHY which changes its host-side interface dynamically.)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH] net: phy: add Marvell 88X2222 transceiver support

[Ivan Bornyakov]

On Tue, Feb 02, 2021 at 04:48:01PM +0000, Russell King - ARM Linux admin wrote:
> On Mon, Feb 01, 2021 at 10:22:51PM +0300, Ivan Bornyakov wrote:
> > +/* PMD Transmit Disable */
> > +#define	MV_TX_DISABLE		0x0009
> > +#define	MV_TX_DISABLE_GLOBAL	BIT(0)
> 
> Please use MDIO_PMA_TXDIS and MDIO_PMD_TXDIS_GLOBAL; this is an
> IEEE802.3 defined register.
> 
> > +/* 10GBASE-R PCS Status 1 */
> > +#define	MV_10GBR_STAT		MDIO_STAT1
> 
> Nothing Marvell specific here, please use MDIO_STAT1 directly.
> 
> > +/* 1000Base-X/SGMII Control Register */
> > +#define	MV_1GBX_CTRL		0x2000
> > +
> > +/* 1000BASE-X/SGMII Status Register */
> > +#define	MV_1GBX_STAT		0x2001
> > +
> > +/* 1000Base-X Auto-Negotiation Advertisement Register */
> > +#define	MV_1GBX_ADVERTISE	0x2004
> 
> Marvell have had a habbit of placing other PHY instances within the
> register space. This also looks like Clause 22 layout rather than
> Clause 45 layout - please use the Clause 22 definitions for the bits
> rather than Clause 45. (so BMCR_ANENABLE, BMCR_ANRESTART for
> MV_1GBX_CTRL, etc).
> 
> Please define these as:
> 
> +#define	MV_1GBX_CTRL		(0x2000 + MII_BMCR)
> +#define	MV_1GBX_STAT		(0x2000 + MII_BMSR)
> +#define	MV_1GBX_ADVERTISE	(0x2000 + MII_ADVERTISE)
> 
> to make it clear what is going on here.
> 
> > +static int sfp_module_insert(void *_priv, const struct sfp_eeprom_id *id)
> > +{
> > +	struct phy_device *phydev = _priv;
> > +	struct device *dev = &phydev->mdio.dev;
> > +	struct mv2222_data *priv = phydev->priv;
> > +	phy_interface_t interface;
> > +
> > +	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported) = { 0, };
> > +
> > +	sfp_parse_support(phydev->sfp_bus, id, supported);
> > +	interface = sfp_select_interface(phydev->sfp_bus, supported);
> > +
> > +	dev_info(dev, "%s SFP module inserted", phy_modes(interface));
> > +
> > +	switch (interface) {
> > +	case PHY_INTERFACE_MODE_10GBASER:
> > +		phydev->speed = SPEED_10000;
> > +		phydev->interface = PHY_INTERFACE_MODE_10GBASER;
> > +		linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
> > +				 phydev->supported);
> > +
> > +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> > +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
> > +		mv2222_soft_reset(phydev);
> > +		break;
> > +	case PHY_INTERFACE_MODE_1000BASEX:
> > +	default:
> > +		phydev->speed = SPEED_1000;
> > +		phydev->interface = PHY_INTERFACE_MODE_1000BASEX;
> > +		linkmode_clear_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
> > +				   phydev->supported);
> > +
> > +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> > +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
> > +		mv2222_soft_reset(phydev);
> > +	}
> > +
> > +	priv->sfp_inserted = true;
> > +
> > +	if (priv->net_up)
> > +		mv2222_tx_enable(phydev);
> 
> This is racy. priv->net_up is modified via the suspend/resume
> callbacks, which are called with phydev->lock held. No other locks
> are guaranteed to be held.
> 
> However, this function is called with the SFP sm_mutex, and rtnl
> held. Consequently, the use of sfp_inserted and net_up in this
> function and the suspend/resume callbacks is racy.
> 
> Why are you disabling the transmitter anyway? Is this for power
> saving?
> 

Actually, the original thought was to down the link on the other side,
when network interface is down on our side. Power saving is a nice
side-effect.

> > +static void mv2222_update_interface(struct phy_device *phydev)
> > +{
> > +	if ((phydev->speed == SPEED_1000 ||
> > +	     phydev->speed == SPEED_100 ||
> > +	     phydev->speed == SPEED_10) &&
> > +	    phydev->interface != PHY_INTERFACE_MODE_1000BASEX) {
> > +		phydev->interface = PHY_INTERFACE_MODE_1000BASEX;
> > +
> > +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> > +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
> > +		mv2222_soft_reset(phydev);
> > +	} else if (phydev->speed == SPEED_10000 &&
> > +		   phydev->interface != PHY_INTERFACE_MODE_10GBASER) {
> > +		phydev->interface = PHY_INTERFACE_MODE_10GBASER;
> > +
> > +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> > +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
> > +		mv2222_soft_reset(phydev);
> > +	}
> 
> This looks wrong. phydev->interface is the _host_ interface, which
> you are clearly setting to XAUI here. Some network drivers depend
> on this being correct (for instance, when used with the Marvell
> 88x3310 PHY which changes its host-side interface dynamically.)
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Overall, thank you for in-depth review, Russell.

[PATCH v2] net: phy: add Marvell 88X2222 transceiver support

[Ivan Bornyakov]

Add basic support for the Marvell 88X2222 multi-speed ethernet
transceiver.

This PHY provides data transmission over fiber-optic as well as Twinax
copper links. The 88X2222 supports 2 ports of 10GBase-R and 1000Base-X
on the line-side interface. The host-side interface supports 4 ports of
10GBase-R, RXAUI, 1000Base-X and 2 ports of XAUI.

This driver, however, supports only XAUI on the host-side and
1000Base-X/10GBase-R on the line-side, for now. The SGMII is also
supported over 1000Base-X. Interrupts are not supported.

Internal registers access compliant with the Clause 45 specification.

Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
---
 drivers/net/phy/Kconfig           |   6 +
 drivers/net/phy/Makefile          |   1 +
 drivers/net/phy/marvell-88x2222.c | 510 ++++++++++++++++++++++++++++++
 include/linux/marvell_phy.h       |   1 +
 4 files changed, 518 insertions(+)
 create mode 100644 drivers/net/phy/marvell-88x2222.c

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 698bea312adc..a615b3660b05 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -201,6 +201,12 @@ config MARVELL_10G_PHY
 	help
 	  Support for the Marvell Alaska MV88X3310 and compatible PHYs.
 
+config MARVELL_88X2222_PHY
+	tristate "Marvell 88X2222 PHY"
+	help
+	  Support for the Marvell 88X2222 Dual-port Multi-speed Ethernet
+	  Transceiver.
+
 config MICREL_PHY
 	tristate "Micrel PHYs"
 	help
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index a13e402074cf..de683e3abe63 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -63,6 +63,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_88X2222_PHY)	+= marvell-88x2222.o
 obj-$(CONFIG_MESON_GXL_PHY)	+= meson-gxl.o
 obj-$(CONFIG_MICREL_KS8995MA)	+= spi_ks8995.o
 obj-$(CONFIG_MICREL_PHY)	+= micrel.o
diff --git a/drivers/net/phy/marvell-88x2222.c b/drivers/net/phy/marvell-88x2222.c
new file mode 100644
index 000000000000..5f1b6185e272
--- /dev/null
+++ b/drivers/net/phy/marvell-88x2222.c
@@ -0,0 +1,510 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Marvell 88x2222 dual-port multi-speed ethernet transceiver.
+ *
+ * Supports:
+ *	XAUI on the host side.
+ *	1000Base-X or 10GBase-R on the line side.
+ */
+#include <linux/module.h>
+#include <linux/phy.h>
+#include <linux/gpio.h>
+#include <linux/delay.h>
+#include <linux/mdio.h>
+#include <linux/marvell_phy.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/sfp.h>
+#include <linux/netdevice.h>
+
+/* Port PCS Configuration */
+#define	MV_PCS_CONFIG		0xF002
+#define	MV_PCS_HOST_XAUI	0x73
+#define	MV_PCS_LINE_10GBR	(0x71 << 8)
+#define	MV_PCS_LINE_1GBX_AN	(0x7B << 8)
+#define	MV_PCS_LINE_SGMII_AN	(0x7F << 8)
+
+/* Port Reset and Power Down */
+#define	MV_PORT_RST	0xF003
+#define	MV_LINE_RST_SW	BIT(15)
+#define	MV_HOST_RST_SW	BIT(7)
+#define	MV_PORT_RST_SW	(MV_LINE_RST_SW | MV_HOST_RST_SW)
+
+/* 10GBASE-R PCS Real Time Status Register */
+#define	MV_10GBR_STAT_RT	0x8002
+
+/* 1000Base-X/SGMII Control Register */
+#define	MV_1GBX_CTRL		(0x2000 + MII_BMCR)
+
+/* 1000BASE-X/SGMII Status Register */
+#define	MV_1GBX_STAT		(0x2000 + MII_BMSR)
+
+/* 1000Base-X Auto-Negotiation Advertisement Register */
+#define	MV_1GBX_ADVERTISE	(0x2000 + MII_ADVERTISE)
+
+/* 1000Base-X PHY Specific Status Register */
+#define	MV_1GBX_PHY_STAT		0xA003
+#define	MV_1GBX_PHY_STAT_AN_RESOLVED	BIT(11)
+#define	MV_1GBX_PHY_STAT_DUPLEX		BIT(13)
+#define	MV_1GBX_PHY_STAT_SPEED100	BIT(14)
+#define	MV_1GBX_PHY_STAT_SPEED1000	BIT(15)
+
+struct mv2222_data {
+	phy_interface_t line_interface;
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
+};
+
+/* SFI PMA transmit enable */
+static int mv2222_tx_enable(struct phy_device *phydev)
+{
+	return phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_TXDIS,
+				  MDIO_PMD_TXDIS_GLOBAL);
+}
+
+/* SFI PMA transmit disable */
+static int mv2222_tx_disable(struct phy_device *phydev)
+{
+	return phy_set_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_TXDIS,
+				MDIO_PMD_TXDIS_GLOBAL);
+}
+
+static int mv2222_soft_reset(struct phy_device *phydev)
+{
+	int val, ret;
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PORT_RST,
+			    MV_PORT_RST_SW);
+	if (ret < 0)
+		return ret;
+
+	return phy_read_mmd_poll_timeout(phydev, MDIO_MMD_VEND2, MV_PORT_RST,
+					 val, !(val & MV_PORT_RST_SW),
+					 5000, 1000000, true);
+}
+
+static int sfp_module_insert(void *_priv, const struct sfp_eeprom_id *id)
+{
+	struct phy_device *phydev = _priv;
+	struct device *dev = &phydev->mdio.dev;
+	struct mv2222_data *priv = phydev->priv;
+	phy_interface_t sfp_interface;
+
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(sfp_supported) = { 0, };
+
+	sfp_parse_support(phydev->sfp_bus, id, sfp_supported);
+	sfp_interface = sfp_select_interface(phydev->sfp_bus, sfp_supported);
+
+	dev_info(dev, "%s SFP module inserted", phy_modes(sfp_interface));
+
+	switch (sfp_interface) {
+	case PHY_INTERFACE_MODE_10GBASER:
+		phydev->speed = SPEED_10000;
+		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
+			      MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
+		break;
+	case PHY_INTERFACE_MODE_1000BASEX:
+		phydev->speed = SPEED_1000;
+		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
+			      MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
+		break;
+	case PHY_INTERFACE_MODE_SGMII:
+		phydev->speed = SPEED_1000;
+		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
+			      MV_PCS_HOST_XAUI | MV_PCS_LINE_SGMII_AN);
+		phy_modify_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
+			       BMCR_SPEED1000 | BMCR_SPEED100, BMCR_SPEED1000);
+		break;
+	default:
+		dev_err(dev, "Incompatible SFP module inserted\n");
+
+		return -EINVAL;
+	}
+
+	linkmode_and(phydev->supported, priv->supported, sfp_supported);
+	priv->line_interface = sfp_interface;
+
+	return mv2222_soft_reset(phydev);
+}
+
+static void sfp_module_remove(void *_priv)
+{
+	struct phy_device *phydev = _priv;
+	struct mv2222_data *priv = phydev->priv;
+
+	priv->line_interface = PHY_INTERFACE_MODE_NA;
+	linkmode_copy(phydev->supported, priv->supported);
+}
+
+static const struct sfp_upstream_ops sfp_phy_ops = {
+	.module_insert = sfp_module_insert,
+	.module_remove = sfp_module_remove,
+	.attach = phy_sfp_attach,
+	.detach = phy_sfp_detach,
+};
+
+static int mv2222_config_init(struct phy_device *phydev)
+{
+	if (phydev->interface != PHY_INTERFACE_MODE_XAUI)
+		return -EINVAL;
+
+	return 0;
+}
+
+/* switch line-side interface between 10GBase-R and 1GBase-X
+ * according to speed */
+static void mv2222_update_interface(struct phy_device *phydev)
+{
+	struct mv2222_data *priv = phydev->priv;
+
+	if (phydev->speed == SPEED_10000 &&
+	    priv->line_interface == PHY_INTERFACE_MODE_1000BASEX) {
+		priv->line_interface = PHY_INTERFACE_MODE_10GBASER;
+
+		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
+			      MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
+		mv2222_soft_reset(phydev);
+	}
+
+	if (phydev->speed == SPEED_1000 &&
+	    priv->line_interface == PHY_INTERFACE_MODE_10GBASER) {
+		priv->line_interface = PHY_INTERFACE_MODE_1000BASEX;
+
+		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
+			      MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
+		mv2222_soft_reset(phydev);
+	}
+}
+
+/* Returns negative on error, 0 if link is down, 1 if link is up */
+static int mv2222_read_status_10g(struct phy_device *phydev)
+{
+	int val, link = 0;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_STAT1);
+	if (val < 0)
+		return val;
+
+	if (val & MDIO_STAT1_LSTATUS) {
+		link = 1;
+
+		/* 10GBASE-R do not support auto-negotiation */
+		phydev->autoneg = AUTONEG_DISABLE;
+		phydev->speed = SPEED_10000;
+		phydev->duplex = DUPLEX_FULL;
+	}
+
+	return link;
+}
+
+/* Returns negative on error, 0 if link is down, 1 if link is up */
+static int mv2222_read_status_1g(struct phy_device *phydev)
+{
+	int val, link = 0;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_STAT);
+	if (val < 0)
+		return val;
+
+	if (!(val & BMSR_LSTATUS) ||
+	    (phydev->autoneg == AUTONEG_ENABLE &&
+	     !(val & BMSR_ANEGCOMPLETE)))
+		return 0;
+
+	link = 1;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_PHY_STAT);
+	if (val < 0)
+		return val;
+
+	if (val & MV_1GBX_PHY_STAT_AN_RESOLVED) {
+		if (val & MV_1GBX_PHY_STAT_DUPLEX)
+			phydev->duplex = DUPLEX_FULL;
+		else
+			phydev->duplex = DUPLEX_HALF;
+
+		if (val & MV_1GBX_PHY_STAT_SPEED1000)
+			phydev->speed = SPEED_1000;
+		else if (val & MV_1GBX_PHY_STAT_SPEED100)
+			phydev->speed = SPEED_100;
+		else
+			phydev->speed = SPEED_10;
+	} else {
+		phydev->duplex = DUPLEX_UNKNOWN;
+		phydev->speed = SPEED_UNKNOWN;
+	}
+
+	return link;
+}
+
+static int mv2222_read_status(struct phy_device *phydev)
+{
+	struct mv2222_data *priv = phydev->priv;
+	int link;
+
+	if (priv->line_interface == PHY_INTERFACE_MODE_10GBASER)
+		link = mv2222_read_status_10g(phydev);
+	else
+		link = mv2222_read_status_1g(phydev);
+
+	if (link < 0)
+		return link;
+
+	phydev->link = link;
+
+	return 0;
+}
+
+static int mv2222_disable_aneg(struct phy_device *phydev)
+{
+	return phy_clear_bits_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
+				  BMCR_ANENABLE | BMCR_ANRESTART);
+}
+
+static int mv2222_enable_aneg(struct phy_device *phydev)
+{
+	return phy_set_bits_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
+				BMCR_ANENABLE | BMCR_RESET);
+}
+
+static int mv2222_set_sgmii_speed(struct phy_device *phydev)
+{
+	switch (phydev->speed) {
+	case SPEED_1000:
+		if (!(linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+					phydev->supported) ||
+		      linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
+					phydev->supported)))
+			return -EINVAL;
+
+		phy_modify_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
+			       BMCR_SPEED1000 | BMCR_SPEED100, BMCR_SPEED1000);
+		break;
+	case SPEED_100:
+		if (!(linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+					phydev->supported) ||
+		      linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT,
+					phydev->supported)))
+			return -EINVAL;
+
+		phy_modify_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
+			       BMCR_SPEED1000 | BMCR_SPEED100, BMCR_SPEED100);
+		break;
+	case SPEED_10:
+		if (!(linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
+					phydev->supported) ||
+		      linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
+					phydev->supported)))
+			return -EINVAL;
+
+		phy_modify_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
+			       BMCR_SPEED1000 | BMCR_SPEED100, BMCR_SPEED10);
+		break;
+	default:
+		break;
+	}
+
+	return mv2222_soft_reset(phydev);
+}
+
+static bool mv2222_is_10g_capable(struct phy_device *phydev)
+{
+	return (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+				  phydev->supported) ||
+		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+				  phydev->supported) ||
+		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseCR_Full_BIT,
+				  phydev->supported) ||
+		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseSR_Full_BIT,
+				  phydev->supported) ||
+		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseLR_Full_BIT,
+				  phydev->supported) ||
+		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT,
+				  phydev->supported) ||
+		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseER_Full_BIT,
+				  phydev->supported));
+}
+
+static int mv2222_config_aneg(struct phy_device *phydev)
+{
+	struct mv2222_data *priv = phydev->priv;
+	int ret, adv;
+
+	/* SFP is not present, do nothing */
+	if (priv->line_interface == PHY_INTERFACE_MODE_NA)
+		return 0;
+
+	if (phydev->autoneg == AUTONEG_DISABLE ||
+	    phydev->speed == SPEED_10000) {
+		if (phydev->speed == SPEED_10000 &&
+		    !mv2222_is_10g_capable(phydev))
+			return -EINVAL;
+
+		if (priv->line_interface == PHY_INTERFACE_MODE_SGMII) {
+			ret = mv2222_set_sgmii_speed(phydev);
+			if (ret < 0)
+				return ret;
+		} else {
+			mv2222_update_interface(phydev);
+		}
+
+		return mv2222_disable_aneg(phydev);
+	}
+
+	/* Try 10G first */
+	if (mv2222_is_10g_capable(phydev)) {
+		phydev->speed = SPEED_10000;
+		mv2222_update_interface(phydev);
+
+		ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_10GBR_STAT_RT);
+		if (ret < 0)
+			return ret;
+
+		if (ret & MDIO_STAT1_LSTATUS) {
+			phydev->autoneg = AUTONEG_DISABLE;
+
+			return mv2222_disable_aneg(phydev);
+		}
+
+		/* 10G link was not established, switch back to 1G
+		 * and proceed with true autonegotiation */
+		phydev->speed = SPEED_1000;
+		mv2222_update_interface(phydev);
+	}
+
+	adv = 0;
+
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
+			      phydev->supported))
+		adv |= ADVERTISE_1000XFULL;
+
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
+			      phydev->supported))
+		adv |= ADVERTISE_1000XPAUSE;
+
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+			      phydev->supported))
+		adv |= ADVERTISE_1000XPSE_ASYM;
+
+	ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_ADVERTISE,
+			     ADVERTISE_1000XFULL |
+			     ADVERTISE_1000XPAUSE | ADVERTISE_1000XPSE_ASYM,
+			     adv);
+	if (ret < 0)
+		return ret;
+
+	return mv2222_enable_aneg(phydev);
+}
+
+static int mv2222_aneg_done(struct phy_device *phydev)
+{
+	int ret;
+
+	if (mv2222_is_10g_capable(phydev)) {
+		ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_STAT1);
+		if (ret < 0)
+			return ret;
+
+		if (ret & MDIO_STAT1_LSTATUS)
+			return 1;
+	}
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_STAT);
+	if (ret < 0)
+		return ret;
+
+	return (ret & BMSR_ANEGCOMPLETE);
+}
+
+static int mv2222_resume(struct phy_device *phydev)
+{
+	return mv2222_tx_enable(phydev);
+}
+
+static int mv2222_suspend(struct phy_device *phydev)
+{
+	return mv2222_tx_disable(phydev);
+}
+
+static int mv2222_get_features(struct phy_device *phydev)
+{
+	/* All supported linkmodes are set at probe
+	 * and adjusted at SFP module insert */
+
+	return 0;
+}
+
+static int mv2222_probe(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct mv2222_data *priv = NULL;
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported) = { 0, };
+
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_TP_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseCR_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseSR_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseLR_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseER_Full_BIT, supported);
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	linkmode_copy(phydev->supported, supported);
+	linkmode_copy(priv->supported, supported);
+	priv->line_interface = PHY_INTERFACE_MODE_NA;
+	phydev->priv = priv;
+
+	return phy_sfp_probe(phydev, &sfp_phy_ops);
+}
+
+static void mv2222_remove(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct mv2222_data *priv = phydev->priv;
+
+	if (priv)
+		devm_kfree(dev, priv);
+}
+
+static struct phy_driver mv2222_drivers[] = {
+	{
+		.phy_id = MARVELL_PHY_ID_88X2222,
+		.phy_id_mask = MARVELL_PHY_ID_MASK,
+		.name = "Marvell 88X2222",
+		.get_features = mv2222_get_features,
+		.soft_reset = mv2222_soft_reset,
+		.config_init = mv2222_config_init,
+		.config_aneg = mv2222_config_aneg,
+		.aneg_done = mv2222_aneg_done,
+		.probe = mv2222_probe,
+		.remove = mv2222_remove,
+		.suspend = mv2222_suspend,
+		.resume = mv2222_resume,
+		.read_status = mv2222_read_status,
+	},
+};
+module_phy_driver(mv2222_drivers);
+
+static struct mdio_device_id __maybe_unused mv2222_tbl[] = {
+	{ MARVELL_PHY_ID_88X2222, MARVELL_PHY_ID_MASK },
+	{ }
+};
+MODULE_DEVICE_TABLE(mdio, mv2222_tbl);
+
+MODULE_DESCRIPTION("Marvell 88x2222 ethernet transceiver driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/marvell_phy.h b/include/linux/marvell_phy.h
index 52b1610eae68..274abd5fbac3 100644
--- a/include/linux/marvell_phy.h
+++ b/include/linux/marvell_phy.h
@@ -24,6 +24,7 @@
 #define MARVELL_PHY_ID_88E3016		0x01410e60
 #define MARVELL_PHY_ID_88X3310		0x002b09a0
 #define MARVELL_PHY_ID_88E2110		0x002b09b0
+#define MARVELL_PHY_ID_88X2222		0x01410f10
 
 /* Marvel 88E1111 in Finisar SFP module with modified PHY ID */
 #define MARVELL_PHY_ID_88E1111_FINISAR	0x01ff0cc0
-- 
2.20.1

Re: [PATCH v2] net: phy: add Marvell 88X2222 transceiver support

[Russell King - ARM Linux admin]

On Sat, Feb 20, 2021 at 12:46:23PM +0300, Ivan Bornyakov wrote:
> Add basic support for the Marvell 88X2222 multi-speed ethernet
> transceiver.
> 
> This PHY provides data transmission over fiber-optic as well as Twinax
> copper links. The 88X2222 supports 2 ports of 10GBase-R and 1000Base-X
> on the line-side interface. The host-side interface supports 4 ports of
> 10GBase-R, RXAUI, 1000Base-X and 2 ports of XAUI.
> 
> This driver, however, supports only XAUI on the host-side and
> 1000Base-X/10GBase-R on the line-side, for now. The SGMII is also
> supported over 1000Base-X. Interrupts are not supported.
> 
> Internal registers access compliant with the Clause 45 specification.
> 
> Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> ---
>  drivers/net/phy/Kconfig           |   6 +
>  drivers/net/phy/Makefile          |   1 +
>  drivers/net/phy/marvell-88x2222.c | 510 ++++++++++++++++++++++++++++++
>  include/linux/marvell_phy.h       |   1 +
>  4 files changed, 518 insertions(+)
>  create mode 100644 drivers/net/phy/marvell-88x2222.c
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 698bea312adc..a615b3660b05 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -201,6 +201,12 @@ config MARVELL_10G_PHY
>  	help
>  	  Support for the Marvell Alaska MV88X3310 and compatible PHYs.
>  
> +config MARVELL_88X2222_PHY
> +	tristate "Marvell 88X2222 PHY"
> +	help
> +	  Support for the Marvell 88X2222 Dual-port Multi-speed Ethernet
> +	  Transceiver.
> +
>  config MICREL_PHY
>  	tristate "Micrel PHYs"
>  	help
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index a13e402074cf..de683e3abe63 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -63,6 +63,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_88X2222_PHY)	+= marvell-88x2222.o
>  obj-$(CONFIG_MESON_GXL_PHY)	+= meson-gxl.o
>  obj-$(CONFIG_MICREL_KS8995MA)	+= spi_ks8995.o
>  obj-$(CONFIG_MICREL_PHY)	+= micrel.o
> diff --git a/drivers/net/phy/marvell-88x2222.c b/drivers/net/phy/marvell-88x2222.c
> new file mode 100644
> index 000000000000..5f1b6185e272
> --- /dev/null
> +++ b/drivers/net/phy/marvell-88x2222.c
> @@ -0,0 +1,510 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Marvell 88x2222 dual-port multi-speed ethernet transceiver.
> + *
> + * Supports:
> + *	XAUI on the host side.
> + *	1000Base-X or 10GBase-R on the line side.
> + */
> +#include <linux/module.h>
> +#include <linux/phy.h>
> +#include <linux/gpio.h>
> +#include <linux/delay.h>
> +#include <linux/mdio.h>
> +#include <linux/marvell_phy.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>
> +#include <linux/sfp.h>
> +#include <linux/netdevice.h>
> +
> +/* Port PCS Configuration */
> +#define	MV_PCS_CONFIG		0xF002
> +#define	MV_PCS_HOST_XAUI	0x73
> +#define	MV_PCS_LINE_10GBR	(0x71 << 8)
> +#define	MV_PCS_LINE_1GBX_AN	(0x7B << 8)
> +#define	MV_PCS_LINE_SGMII_AN	(0x7F << 8)
> +
> +/* Port Reset and Power Down */
> +#define	MV_PORT_RST	0xF003
> +#define	MV_LINE_RST_SW	BIT(15)
> +#define	MV_HOST_RST_SW	BIT(7)
> +#define	MV_PORT_RST_SW	(MV_LINE_RST_SW | MV_HOST_RST_SW)
> +
> +/* 10GBASE-R PCS Real Time Status Register */
> +#define	MV_10GBR_STAT_RT	0x8002
> +
> +/* 1000Base-X/SGMII Control Register */
> +#define	MV_1GBX_CTRL		(0x2000 + MII_BMCR)
> +
> +/* 1000BASE-X/SGMII Status Register */
> +#define	MV_1GBX_STAT		(0x2000 + MII_BMSR)
> +
> +/* 1000Base-X Auto-Negotiation Advertisement Register */
> +#define	MV_1GBX_ADVERTISE	(0x2000 + MII_ADVERTISE)
> +
> +/* 1000Base-X PHY Specific Status Register */
> +#define	MV_1GBX_PHY_STAT		0xA003
> +#define	MV_1GBX_PHY_STAT_AN_RESOLVED	BIT(11)
> +#define	MV_1GBX_PHY_STAT_DUPLEX		BIT(13)
> +#define	MV_1GBX_PHY_STAT_SPEED100	BIT(14)
> +#define	MV_1GBX_PHY_STAT_SPEED1000	BIT(15)
> +
> +struct mv2222_data {
> +	phy_interface_t line_interface;
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
> +};
> +
> +/* SFI PMA transmit enable */
> +static int mv2222_tx_enable(struct phy_device *phydev)
> +{
> +	return phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_TXDIS,
> +				  MDIO_PMD_TXDIS_GLOBAL);
> +}
> +
> +/* SFI PMA transmit disable */
> +static int mv2222_tx_disable(struct phy_device *phydev)
> +{
> +	return phy_set_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_TXDIS,
> +				MDIO_PMD_TXDIS_GLOBAL);
> +}
> +
> +static int mv2222_soft_reset(struct phy_device *phydev)
> +{
> +	int val, ret;
> +
> +	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PORT_RST,
> +			    MV_PORT_RST_SW);
> +	if (ret < 0)
> +		return ret;
> +
> +	return phy_read_mmd_poll_timeout(phydev, MDIO_MMD_VEND2, MV_PORT_RST,
> +					 val, !(val & MV_PORT_RST_SW),
> +					 5000, 1000000, true);
> +}
> +
> +static int sfp_module_insert(void *_priv, const struct sfp_eeprom_id *id)

Please rename this function. We have a function named exactly the same
in the SFP layer, so if we have duplicate function names, it will make
parsing backtraces harder.

> +{
> +	struct phy_device *phydev = _priv;
> +	struct device *dev = &phydev->mdio.dev;
> +	struct mv2222_data *priv = phydev->priv;
> +	phy_interface_t sfp_interface;
> +
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(sfp_supported) = { 0, };
> +
> +	sfp_parse_support(phydev->sfp_bus, id, sfp_supported);
> +	sfp_interface = sfp_select_interface(phydev->sfp_bus, sfp_supported);
> +
> +	dev_info(dev, "%s SFP module inserted", phy_modes(sfp_interface));

Kernel messages normally have a "\n" terminating them.

> +
> +	switch (sfp_interface) {
> +	case PHY_INTERFACE_MODE_10GBASER:
> +		phydev->speed = SPEED_10000;
> +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
> +		break;
> +	case PHY_INTERFACE_MODE_1000BASEX:
> +		phydev->speed = SPEED_1000;
> +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
> +		break;
> +	case PHY_INTERFACE_MODE_SGMII:
> +		phydev->speed = SPEED_1000;
> +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_SGMII_AN);
> +		phy_modify_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
> +			       BMCR_SPEED1000 | BMCR_SPEED100, BMCR_SPEED1000);

Isn't this forcing 1000Mbit, but SGMII relies on AN for the slower
speeds.

> +		break;
> +	default:
> +		dev_err(dev, "Incompatible SFP module inserted\n");
> +
> +		return -EINVAL;
> +	}

I don't think you should set phydev->speed in this function - apart
from the rtnl lock, there is no other locking here, so this is fragile.

> +	linkmode_and(phydev->supported, priv->supported, sfp_supported);

I don't think this is a good idea; phylink does not expect the supported
mask to change, and I suspect _no_ network device expects it to change.
One of the things that network drivers and phylink does is to adjust the
supported mask for a PHY according to the capabilities of the network
device. For example, if they don't support pause modes, or something
else. Overriding it in this way has the possibility to re-introduce
modes that the network driver does not support.

> +	priv->line_interface = sfp_interface;
> +
> +	return mv2222_soft_reset(phydev);
> +}
> +
> +static void sfp_module_remove(void *_priv)

Please rename this function.

> +{
> +	struct phy_device *phydev = _priv;
> +	struct mv2222_data *priv = phydev->priv;
> +
> +	priv->line_interface = PHY_INTERFACE_MODE_NA;
> +	linkmode_copy(phydev->supported, priv->supported);
> +}
> +
> +static const struct sfp_upstream_ops sfp_phy_ops = {
> +	.module_insert = sfp_module_insert,
> +	.module_remove = sfp_module_remove,
> +	.attach = phy_sfp_attach,
> +	.detach = phy_sfp_detach,
> +};
> +
> +static int mv2222_config_init(struct phy_device *phydev)
> +{
> +	if (phydev->interface != PHY_INTERFACE_MODE_XAUI)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +/* switch line-side interface between 10GBase-R and 1GBase-X
> + * according to speed */
> +static void mv2222_update_interface(struct phy_device *phydev)
> +{
> +	struct mv2222_data *priv = phydev->priv;
> +
> +	if (phydev->speed == SPEED_10000 &&
> +	    priv->line_interface == PHY_INTERFACE_MODE_1000BASEX) {
> +		priv->line_interface = PHY_INTERFACE_MODE_10GBASER;
> +
> +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
> +		mv2222_soft_reset(phydev);
> +	}
> +
> +	if (phydev->speed == SPEED_1000 &&
> +	    priv->line_interface == PHY_INTERFACE_MODE_10GBASER) {
> +		priv->line_interface = PHY_INTERFACE_MODE_1000BASEX;
> +
> +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
> +		mv2222_soft_reset(phydev);
> +	}

Wouldn't it be better to have a single function to set the line
interface, used by both this function and your sfp_module_insert
function? I'm thinking something like:

static int mv2222_set_line_interface(struct phy_device *phydev,
				     phy_interface_t line_interface)
{
...
}

and calling that from both these locations to configure the PHY for
10GBASE-R, 1000BASE-X and SGMII modes.

> +}
> +
> +/* Returns negative on error, 0 if link is down, 1 if link is up */
> +static int mv2222_read_status_10g(struct phy_device *phydev)
> +{
> +	int val, link = 0;
> +
> +	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_STAT1);
> +	if (val < 0)
> +		return val;
> +
> +	if (val & MDIO_STAT1_LSTATUS) {
> +		link = 1;
> +
> +		/* 10GBASE-R do not support auto-negotiation */
> +		phydev->autoneg = AUTONEG_DISABLE;
> +		phydev->speed = SPEED_10000;
> +		phydev->duplex = DUPLEX_FULL;
> +	}
> +
> +	return link;
> +}
> +
> +/* Returns negative on error, 0 if link is down, 1 if link is up */
> +static int mv2222_read_status_1g(struct phy_device *phydev)
> +{
> +	int val, link = 0;
> +
> +	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_STAT);
> +	if (val < 0)
> +		return val;
> +
> +	if (!(val & BMSR_LSTATUS) ||
> +	    (phydev->autoneg == AUTONEG_ENABLE &&
> +	     !(val & BMSR_ANEGCOMPLETE)))
> +		return 0;
> +
> +	link = 1;
> +
> +	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_PHY_STAT);
> +	if (val < 0)
> +		return val;
> +
> +	if (val & MV_1GBX_PHY_STAT_AN_RESOLVED) {
> +		if (val & MV_1GBX_PHY_STAT_DUPLEX)
> +			phydev->duplex = DUPLEX_FULL;
> +		else
> +			phydev->duplex = DUPLEX_HALF;
> +
> +		if (val & MV_1GBX_PHY_STAT_SPEED1000)
> +			phydev->speed = SPEED_1000;
> +		else if (val & MV_1GBX_PHY_STAT_SPEED100)
> +			phydev->speed = SPEED_100;
> +		else
> +			phydev->speed = SPEED_10;
> +	} else {
> +		phydev->duplex = DUPLEX_UNKNOWN;
> +		phydev->speed = SPEED_UNKNOWN;
> +	}
> +
> +	return link;
> +}
> +
> +static int mv2222_read_status(struct phy_device *phydev)
> +{
> +	struct mv2222_data *priv = phydev->priv;
> +	int link;
> +
> +	if (priv->line_interface == PHY_INTERFACE_MODE_10GBASER)
> +		link = mv2222_read_status_10g(phydev);
> +	else
> +		link = mv2222_read_status_1g(phydev);
> +
> +	if (link < 0)
> +		return link;
> +
> +	phydev->link = link;
> +
> +	return 0;
> +}
> +
> +static int mv2222_disable_aneg(struct phy_device *phydev)
> +{
> +	return phy_clear_bits_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
> +				  BMCR_ANENABLE | BMCR_ANRESTART);
> +}
> +
> +static int mv2222_enable_aneg(struct phy_device *phydev)
> +{
> +	return phy_set_bits_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
> +				BMCR_ANENABLE | BMCR_RESET);
> +}
> +
> +static int mv2222_set_sgmii_speed(struct phy_device *phydev)
> +{
> +	switch (phydev->speed) {
> +	case SPEED_1000:
> +		if (!(linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
> +					phydev->supported) ||
> +		      linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
> +					phydev->supported)))
> +			return -EINVAL;
> +
> +		phy_modify_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
> +			       BMCR_SPEED1000 | BMCR_SPEED100, BMCR_SPEED1000);
> +		break;
> +	case SPEED_100:
> +		if (!(linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
> +					phydev->supported) ||
> +		      linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT,
> +					phydev->supported)))
> +			return -EINVAL;
> +
> +		phy_modify_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
> +			       BMCR_SPEED1000 | BMCR_SPEED100, BMCR_SPEED100);
> +		break;
> +	case SPEED_10:
> +		if (!(linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
> +					phydev->supported) ||
> +		      linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
> +					phydev->supported)))
> +			return -EINVAL;
> +
> +		phy_modify_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
> +			       BMCR_SPEED1000 | BMCR_SPEED100, BMCR_SPEED10);
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return mv2222_soft_reset(phydev);
> +}
> +
> +static bool mv2222_is_10g_capable(struct phy_device *phydev)
> +{
> +	return (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
> +				  phydev->supported) ||
> +		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
> +				  phydev->supported) ||
> +		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseCR_Full_BIT,
> +				  phydev->supported) ||
> +		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseSR_Full_BIT,
> +				  phydev->supported) ||
> +		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseLR_Full_BIT,
> +				  phydev->supported) ||
> +		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT,
> +				  phydev->supported) ||
> +		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseER_Full_BIT,
> +				  phydev->supported));
> +}
> +
> +static int mv2222_config_aneg(struct phy_device *phydev)
> +{
> +	struct mv2222_data *priv = phydev->priv;
> +	int ret, adv;
> +
> +	/* SFP is not present, do nothing */
> +	if (priv->line_interface == PHY_INTERFACE_MODE_NA)
> +		return 0;
> +
> +	if (phydev->autoneg == AUTONEG_DISABLE ||
> +	    phydev->speed == SPEED_10000) {
> +		if (phydev->speed == SPEED_10000 &&
> +		    !mv2222_is_10g_capable(phydev))
> +			return -EINVAL;
> +
> +		if (priv->line_interface == PHY_INTERFACE_MODE_SGMII) {
> +			ret = mv2222_set_sgmii_speed(phydev);
> +			if (ret < 0)
> +				return ret;
> +		} else {
> +			mv2222_update_interface(phydev);
> +		}
> +
> +		return mv2222_disable_aneg(phydev);
> +	}
> +
> +	/* Try 10G first */
> +	if (mv2222_is_10g_capable(phydev)) {
> +		phydev->speed = SPEED_10000;
> +		mv2222_update_interface(phydev);
> +
> +		ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_10GBR_STAT_RT);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (ret & MDIO_STAT1_LSTATUS) {
> +			phydev->autoneg = AUTONEG_DISABLE;
> +
> +			return mv2222_disable_aneg(phydev);
> +		}
> +
> +		/* 10G link was not established, switch back to 1G
> +		 * and proceed with true autonegotiation */
> +		phydev->speed = SPEED_1000;
> +		mv2222_update_interface(phydev);

This doesn't look right. If the user specifies that they want 10G,
why should we switch back to 1G?

> +	}
> +
> +	adv = 0;
> +
> +	if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
> +			      phydev->supported))
> +		adv |= ADVERTISE_1000XFULL;
> +
> +	if (linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
> +			      phydev->supported))
> +		adv |= ADVERTISE_1000XPAUSE;
> +
> +	if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
> +			      phydev->supported))
> +		adv |= ADVERTISE_1000XPSE_ASYM;
> +
> +	ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_ADVERTISE,
> +			     ADVERTISE_1000XFULL |
> +			     ADVERTISE_1000XPAUSE | ADVERTISE_1000XPSE_ASYM,
> +			     adv);
> +	if (ret < 0)
> +		return ret;
> +
> +	return mv2222_enable_aneg(phydev);
> +}
> +
> +static int mv2222_aneg_done(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	if (mv2222_is_10g_capable(phydev)) {
> +		ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_STAT1);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (ret & MDIO_STAT1_LSTATUS)
> +			return 1;
> +	}
> +
> +	ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_STAT);
> +	if (ret < 0)
> +		return ret;
> +
> +	return (ret & BMSR_ANEGCOMPLETE);
> +}
> +
> +static int mv2222_resume(struct phy_device *phydev)
> +{
> +	return mv2222_tx_enable(phydev);
> +}
> +
> +static int mv2222_suspend(struct phy_device *phydev)
> +{
> +	return mv2222_tx_disable(phydev);
> +}
> +
> +static int mv2222_get_features(struct phy_device *phydev)
> +{
> +	/* All supported linkmodes are set at probe
> +	 * and adjusted at SFP module insert */
> +
> +	return 0;
> +}
> +
> +static int mv2222_probe(struct phy_device *phydev)
> +{
> +	struct device *dev = &phydev->mdio.dev;
> +	struct mv2222_data *priv = NULL;
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported) = { 0, };
> +
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_TP_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseCR_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseSR_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseLR_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseER_Full_BIT, supported);
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	linkmode_copy(phydev->supported, supported);
> +	linkmode_copy(priv->supported, supported);
> +	priv->line_interface = PHY_INTERFACE_MODE_NA;
> +	phydev->priv = priv;
> +
> +	return phy_sfp_probe(phydev, &sfp_phy_ops);
> +}
> +
> +static void mv2222_remove(struct phy_device *phydev)
> +{
> +	struct device *dev = &phydev->mdio.dev;
> +	struct mv2222_data *priv = phydev->priv;
> +
> +	if (priv)
> +		devm_kfree(dev, priv);
> +}
> +
> +static struct phy_driver mv2222_drivers[] = {
> +	{
> +		.phy_id = MARVELL_PHY_ID_88X2222,
> +		.phy_id_mask = MARVELL_PHY_ID_MASK,
> +		.name = "Marvell 88X2222",
> +		.get_features = mv2222_get_features,
> +		.soft_reset = mv2222_soft_reset,
> +		.config_init = mv2222_config_init,
> +		.config_aneg = mv2222_config_aneg,
> +		.aneg_done = mv2222_aneg_done,
> +		.probe = mv2222_probe,
> +		.remove = mv2222_remove,
> +		.suspend = mv2222_suspend,
> +		.resume = mv2222_resume,
> +		.read_status = mv2222_read_status,
> +	},
> +};
> +module_phy_driver(mv2222_drivers);
> +
> +static struct mdio_device_id __maybe_unused mv2222_tbl[] = {
> +	{ MARVELL_PHY_ID_88X2222, MARVELL_PHY_ID_MASK },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(mdio, mv2222_tbl);
> +
> +MODULE_DESCRIPTION("Marvell 88x2222 ethernet transceiver driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/marvell_phy.h b/include/linux/marvell_phy.h
> index 52b1610eae68..274abd5fbac3 100644
> --- a/include/linux/marvell_phy.h
> +++ b/include/linux/marvell_phy.h
> @@ -24,6 +24,7 @@
>  #define MARVELL_PHY_ID_88E3016		0x01410e60
>  #define MARVELL_PHY_ID_88X3310		0x002b09a0
>  #define MARVELL_PHY_ID_88E2110		0x002b09b0
> +#define MARVELL_PHY_ID_88X2222		0x01410f10
>  
>  /* Marvel 88E1111 in Finisar SFP module with modified PHY ID */
>  #define MARVELL_PHY_ID_88E1111_FINISAR	0x01ff0cc0
> -- 
> 2.20.1
> 
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v2] net: phy: add Marvell 88X2222 transceiver support

[Ivan Bornyakov]

On Sat, Feb 20, 2021 at 11:53:04AM +0000, Russell King - ARM Linux admin wrote:
> On Sat, Feb 20, 2021 at 12:46:23PM +0300, Ivan Bornyakov wrote:
> > +
> > +	switch (sfp_interface) {
> > +	case PHY_INTERFACE_MODE_10GBASER:
> > +		phydev->speed = SPEED_10000;
> > +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> > +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
> > +		break;
> > +	case PHY_INTERFACE_MODE_1000BASEX:
> > +		phydev->speed = SPEED_1000;
> > +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> > +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
> > +		break;
> > +	case PHY_INTERFACE_MODE_SGMII:
> > +		phydev->speed = SPEED_1000;
> > +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> > +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_SGMII_AN);
> > +		phy_modify_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
> > +			       BMCR_SPEED1000 | BMCR_SPEED100, BMCR_SPEED1000);
> 
> Isn't this forcing 1000Mbit, but SGMII relies on AN for the slower
> speeds.
> 

It was intended as default, but you have a good point, there is no need
for this, I can just trigger config_aneg() instead.

> > +		break;
> > +	default:
> > +		dev_err(dev, "Incompatible SFP module inserted\n");
> > +
> > +		return -EINVAL;
> > +	}
> 
> I don't think you should set phydev->speed in this function - apart
> from the rtnl lock, there is no other locking here, so this is fragile.
> 
> > +	linkmode_and(phydev->supported, priv->supported, sfp_supported);
> 
> I don't think this is a good idea; phylink does not expect the supported
> mask to change, and I suspect _no_ network device expects it to change.
> One of the things that network drivers and phylink does is to adjust the
> supported mask for a PHY according to the capabilities of the network
> device. For example, if they don't support pause modes, or something
> else. Overriding it in this way has the possibility to re-introduce
> modes that the network driver does not support.
> 

OK, but how can I exclude modes unsupported by inserted SFP?
Or I shouldn't exclude any at all?

> > +/* switch line-side interface between 10GBase-R and 1GBase-X
> > + * according to speed */
> > +static void mv2222_update_interface(struct phy_device *phydev)
> > +{
> > +	struct mv2222_data *priv = phydev->priv;
> > +
> > +	if (phydev->speed == SPEED_10000 &&
> > +	    priv->line_interface == PHY_INTERFACE_MODE_1000BASEX) {
> > +		priv->line_interface = PHY_INTERFACE_MODE_10GBASER;
> > +
> > +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> > +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
> > +		mv2222_soft_reset(phydev);
> > +	}
> > +
> > +	if (phydev->speed == SPEED_1000 &&
> > +	    priv->line_interface == PHY_INTERFACE_MODE_10GBASER) {
> > +		priv->line_interface = PHY_INTERFACE_MODE_1000BASEX;
> > +
> > +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> > +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
> > +		mv2222_soft_reset(phydev);
> > +	}
> 
> Wouldn't it be better to have a single function to set the line
> interface, used by both this function and your sfp_module_insert
> function? I'm thinking something like:
> 
> static int mv2222_set_line_interface(struct phy_device *phydev,
> 				     phy_interface_t line_interface)
> {
> ...
> }
> 
> and calling that from both these locations to configure the PHY for
> 10GBASE-R, 1000BASE-X and SGMII modes.
> 

I'll think about it, thanks.

> > +
> > +static int mv2222_config_aneg(struct phy_device *phydev)
> > +{
> > +	struct mv2222_data *priv = phydev->priv;
> > +	int ret, adv;
> > +
> > +	/* SFP is not present, do nothing */
> > +	if (priv->line_interface == PHY_INTERFACE_MODE_NA)
> > +		return 0;
> > +
> > +	if (phydev->autoneg == AUTONEG_DISABLE ||
> > +	    phydev->speed == SPEED_10000) {
> > +		if (phydev->speed == SPEED_10000 &&
> > +		    !mv2222_is_10g_capable(phydev))
> > +			return -EINVAL;
> > +
> > +		if (priv->line_interface == PHY_INTERFACE_MODE_SGMII) {
> > +			ret = mv2222_set_sgmii_speed(phydev);
> > +			if (ret < 0)
> > +				return ret;
> > +		} else {
> > +			mv2222_update_interface(phydev);
> > +		}
> > +
> > +		return mv2222_disable_aneg(phydev);
> > +	}
> > +
> > +	/* Try 10G first */
> > +	if (mv2222_is_10g_capable(phydev)) {
> > +		phydev->speed = SPEED_10000;
> > +		mv2222_update_interface(phydev);
> > +
> > +		ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_10GBR_STAT_RT);
> > +		if (ret < 0)
> > +			return ret;
> > +
> > +		if (ret & MDIO_STAT1_LSTATUS) {
> > +			phydev->autoneg = AUTONEG_DISABLE;
> > +
> > +			return mv2222_disable_aneg(phydev);
> > +		}
> > +
> > +		/* 10G link was not established, switch back to 1G
> > +		 * and proceed with true autonegotiation */
> > +		phydev->speed = SPEED_1000;
> > +		mv2222_update_interface(phydev);
> 
> This doesn't look right. If the user specifies that they want 10G,
> why should we switch back to 1G?
> 

This is for enabled autoneg. Try 10g, if link is established - stay and
disable autoneg, otherwise continue with true autonegotiation.
For explicitly specified 10g mode there is case above.

Thanks again for in-depth review, Russell.

Re: [PATCH v2] net: phy: add Marvell 88X2222 transceiver support

[Andrew Lunn]

> > +/* switch line-side interface between 10GBase-R and 1GBase-X
> > + * according to speed */
> > +static void mv2222_update_interface(struct phy_device *phydev)
> > +{
> > +	struct mv2222_data *priv = phydev->priv;
> > +
> > +	if (phydev->speed == SPEED_10000 &&
> > +	    priv->line_interface == PHY_INTERFACE_MODE_1000BASEX) {
> > +		priv->line_interface = PHY_INTERFACE_MODE_10GBASER;
> > +
> > +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> > +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
> > +		mv2222_soft_reset(phydev);
> > +	}
> > +
> > +	if (phydev->speed == SPEED_1000 &&
> > +	    priv->line_interface == PHY_INTERFACE_MODE_10GBASER) {
> > +		priv->line_interface = PHY_INTERFACE_MODE_1000BASEX;
> > +
> > +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> > +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
> > +		mv2222_soft_reset(phydev);
> > +	}
> 
> Wouldn't it be better to have a single function to set the line
> interface, used by both this function and your sfp_module_insert
> function? I'm thinking something like:
> 
> static int mv2222_set_line_interface(struct phy_device *phydev,
> 				     phy_interface_t line_interface)
> {
> ...
> }
> 
> and calling that from both these locations to configure the PHY for
> 10GBASE-R, 1000BASE-X and SGMII modes.

Agreed. This got me confused, wondering where the SGMII handling was.

	Andrew

Re: [PATCH v2] net: phy: add Marvell 88X2222 transceiver support

[Andrew Lunn]

Hi Ivan

> +static int sfp_module_insert(void *_priv, const struct sfp_eeprom_id *id)
> +{
> +	struct phy_device *phydev = _priv;
> +	struct device *dev = &phydev->mdio.dev;
> +	struct mv2222_data *priv = phydev->priv;
> +	phy_interface_t sfp_interface;

Reverse Christmas tree please. Which means you will need to move some
of the assignments to the body of the function. Please also drop the _
from _priv.

> +
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(sfp_supported) = { 0, };
> +
> +	sfp_parse_support(phydev->sfp_bus, id, sfp_supported);
> +	sfp_interface = sfp_select_interface(phydev->sfp_bus, sfp_supported);
> +
> +	dev_info(dev, "%s SFP module inserted", phy_modes(sfp_interface));
> +
> +	switch (sfp_interface) {
> +	case PHY_INTERFACE_MODE_10GBASER:
> +		phydev->speed = SPEED_10000;
> +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
> +		break;
> +	case PHY_INTERFACE_MODE_1000BASEX:
> +		phydev->speed = SPEED_1000;
> +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
> +		break;
> +	case PHY_INTERFACE_MODE_SGMII:
> +		phydev->speed = SPEED_1000;

SPEED_UNKNOWN might be better here, until auto negotiation has
completed and you then know if it is 10/100/1G.

> +		phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> +			      MV_PCS_HOST_XAUI | MV_PCS_LINE_SGMII_AN);
> +		phy_modify_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
> +			       BMCR_SPEED1000 | BMCR_SPEED100, BMCR_SPEED1000);
> +		break;
> +	default:
> +		dev_err(dev, "Incompatible SFP module inserted\n");
> +
> +		return -EINVAL;
> +	}
> +
> +	linkmode_and(phydev->supported, priv->supported, sfp_supported);
> +	priv->line_interface = sfp_interface;
> +
> +	return mv2222_soft_reset(phydev);
> +}
> +
> +static void sfp_module_remove(void *_priv)
> +{
> +	struct phy_device *phydev = _priv;
> +	struct mv2222_data *priv = phydev->priv;

More reverse christmass tree.

> +
> +	priv->line_interface = PHY_INTERFACE_MODE_NA;
> +	linkmode_copy(phydev->supported, priv->supported);
> +}
> +
> +static int mv2222_config_aneg(struct phy_device *phydev)
> +{

> +	/* Try 10G first */
> +	if (mv2222_is_10g_capable(phydev)) {
> +		phydev->speed = SPEED_10000;
> +		mv2222_update_interface(phydev);
> +
> +		ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_10GBR_STAT_RT);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (ret & MDIO_STAT1_LSTATUS) {
> +			phydev->autoneg = AUTONEG_DISABLE;
> +
> +			return mv2222_disable_aneg(phydev);
> +		}
> +
> +		/* 10G link was not established, switch back to 1G
> +		 * and proceed with true autonegotiation */
> +		phydev->speed = SPEED_1000;
> +		mv2222_update_interface(phydev);

So you are assuming the cable is plugged in and the peer is ready to
go? Try 10G once, and then fall back to 1G? This does not seem very
reliable, and likely to cause confusion. It works sometimes, not
others. I'm not sure this is a good idea.

> +static void mv2222_remove(struct phy_device *phydev)
> +{
> +	struct device *dev = &phydev->mdio.dev;
> +	struct mv2222_data *priv = phydev->priv;
> +
> +	if (priv)
> +		devm_kfree(dev, priv);

Why can devm_kfree(). The point of devm_ is that it frees itself.

    Andrew

[PATCH v3] net: phy: add Marvell 88X2222 transceiver support

[Ivan Bornyakov]

Add basic support for the Marvell 88X2222 multi-speed ethernet
transceiver.

This PHY provides data transmission over fiber-optic as well as Twinax
copper links. The 88X2222 supports 2 ports of 10GBase-R and 1000Base-X
on the line-side interface. The host-side interface supports 4 ports of
10GBase-R, RXAUI, 1000Base-X and 2 ports of XAUI.

This driver, however, supports only XAUI on the host-side and
1000Base-X/10GBase-R on the line-side, for now. The SGMII is also
supported over 1000Base-X. Interrupts are not supported.

Internal registers access compliant with the Clause 45 specification.

Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
---
 drivers/net/phy/Kconfig           |   6 +
 drivers/net/phy/Makefile          |   1 +
 drivers/net/phy/marvell-88x2222.c | 533 ++++++++++++++++++++++++++++++
 include/linux/marvell_phy.h       |   1 +
 4 files changed, 541 insertions(+)
 create mode 100644 drivers/net/phy/marvell-88x2222.c

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 698bea312adc..a615b3660b05 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -201,6 +201,12 @@ config MARVELL_10G_PHY
 	help
 	  Support for the Marvell Alaska MV88X3310 and compatible PHYs.
 
+config MARVELL_88X2222_PHY
+	tristate "Marvell 88X2222 PHY"
+	help
+	  Support for the Marvell 88X2222 Dual-port Multi-speed Ethernet
+	  Transceiver.
+
 config MICREL_PHY
 	tristate "Micrel PHYs"
 	help
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index a13e402074cf..de683e3abe63 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -63,6 +63,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_88X2222_PHY)	+= marvell-88x2222.o
 obj-$(CONFIG_MESON_GXL_PHY)	+= meson-gxl.o
 obj-$(CONFIG_MICREL_KS8995MA)	+= spi_ks8995.o
 obj-$(CONFIG_MICREL_PHY)	+= micrel.o
diff --git a/drivers/net/phy/marvell-88x2222.c b/drivers/net/phy/marvell-88x2222.c
new file mode 100644
index 000000000000..aec7e299c165
--- /dev/null
+++ b/drivers/net/phy/marvell-88x2222.c
@@ -0,0 +1,533 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Marvell 88x2222 dual-port multi-speed ethernet transceiver.
+ *
+ * Supports:
+ *	XAUI on the host side.
+ *	1000Base-X or 10GBase-R on the line side.
+ *	SGMII over 1000Base-X.
+ */
+#include <linux/module.h>
+#include <linux/phy.h>
+#include <linux/gpio.h>
+#include <linux/delay.h>
+#include <linux/mdio.h>
+#include <linux/marvell_phy.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/sfp.h>
+#include <linux/netdevice.h>
+
+/* Port PCS Configuration */
+#define	MV_PCS_CONFIG		0xF002
+#define	MV_PCS_HOST_XAUI	0x73
+#define	MV_PCS_LINE_10GBR	(0x71 << 8)
+#define	MV_PCS_LINE_1GBX_AN	(0x7B << 8)
+#define	MV_PCS_LINE_SGMII_AN	(0x7F << 8)
+
+/* Port Reset and Power Down */
+#define	MV_PORT_RST	0xF003
+#define	MV_LINE_RST_SW	BIT(15)
+#define	MV_HOST_RST_SW	BIT(7)
+#define	MV_PORT_RST_SW	(MV_LINE_RST_SW | MV_HOST_RST_SW)
+
+/* 1000Base-X/SGMII Control Register */
+#define	MV_1GBX_CTRL		(0x2000 + MII_BMCR)
+
+/* 1000BASE-X/SGMII Status Register */
+#define	MV_1GBX_STAT		(0x2000 + MII_BMSR)
+
+/* 1000Base-X Auto-Negotiation Advertisement Register */
+#define	MV_1GBX_ADVERTISE	(0x2000 + MII_ADVERTISE)
+
+/* 1000Base-X PHY Specific Status Register */
+#define	MV_1GBX_PHY_STAT		0xA003
+#define	MV_1GBX_PHY_STAT_AN_RESOLVED	BIT(11)
+#define	MV_1GBX_PHY_STAT_DUPLEX		BIT(13)
+#define	MV_1GBX_PHY_STAT_SPEED100	BIT(14)
+#define	MV_1GBX_PHY_STAT_SPEED1000	BIT(15)
+
+struct mv2222_data {
+	phy_interface_t line_interface;
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
+};
+
+/* SFI PMA transmit enable */
+static int mv2222_tx_enable(struct phy_device *phydev)
+{
+	return phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_TXDIS,
+				  MDIO_PMD_TXDIS_GLOBAL);
+}
+
+/* SFI PMA transmit disable */
+static int mv2222_tx_disable(struct phy_device *phydev)
+{
+	return phy_set_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_TXDIS,
+				MDIO_PMD_TXDIS_GLOBAL);
+}
+
+static int mv2222_soft_reset(struct phy_device *phydev)
+{
+	int val, ret;
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PORT_RST,
+			    MV_PORT_RST_SW);
+	if (ret < 0)
+		return ret;
+
+	return phy_read_mmd_poll_timeout(phydev, MDIO_MMD_VEND2, MV_PORT_RST,
+					 val, !(val & MV_PORT_RST_SW),
+					 5000, 1000000, true);
+}
+
+/* Returns negative on error, 0 if link is down, 1 if link is up */
+static int mv2222_read_status_10g(struct phy_device *phydev)
+{
+	int val, link = 0;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_STAT1);
+	if (val < 0)
+		return val;
+
+	if (val & MDIO_STAT1_LSTATUS) {
+		link = 1;
+
+		/* 10GBASE-R do not support auto-negotiation */
+		phydev->autoneg = AUTONEG_DISABLE;
+		phydev->speed = SPEED_10000;
+		phydev->duplex = DUPLEX_FULL;
+	}
+
+	return link;
+}
+
+/* Returns negative on error, 0 if link is down, 1 if link is up */
+static int mv2222_read_status_1g(struct phy_device *phydev)
+{
+	int val, link = 0;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_STAT);
+	if (val < 0)
+		return val;
+
+	if (!(val & BMSR_LSTATUS) ||
+	    (phydev->autoneg == AUTONEG_ENABLE &&
+	     !(val & BMSR_ANEGCOMPLETE)))
+		return 0;
+
+	link = 1;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_PHY_STAT);
+	if (val < 0)
+		return val;
+
+	if (val & MV_1GBX_PHY_STAT_AN_RESOLVED) {
+		if (val & MV_1GBX_PHY_STAT_DUPLEX)
+			phydev->duplex = DUPLEX_FULL;
+		else
+			phydev->duplex = DUPLEX_HALF;
+
+		if (val & MV_1GBX_PHY_STAT_SPEED1000)
+			phydev->speed = SPEED_1000;
+		else if (val & MV_1GBX_PHY_STAT_SPEED100)
+			phydev->speed = SPEED_100;
+		else
+			phydev->speed = SPEED_10;
+	}
+
+	return link;
+}
+
+static int mv2222_read_status(struct phy_device *phydev)
+{
+	struct mv2222_data *priv = phydev->priv;
+	int link;
+
+	phydev->link = 0;
+	phydev->speed = SPEED_UNKNOWN;
+	phydev->duplex = DUPLEX_UNKNOWN;
+
+	if (priv->line_interface == PHY_INTERFACE_MODE_10GBASER)
+		link = mv2222_read_status_10g(phydev);
+	else
+		link = mv2222_read_status_1g(phydev);
+
+	if (link < 0)
+		return link;
+
+	phydev->link = link;
+
+	return 0;
+}
+
+static int mv2222_disable_aneg(struct phy_device *phydev)
+{
+	int ret = phy_clear_bits_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
+				     BMCR_ANENABLE | BMCR_ANRESTART);
+	if (ret < 0)
+		return ret;
+
+	return mv2222_soft_reset(phydev);
+}
+
+static int mv2222_enable_aneg(struct phy_device *phydev)
+{
+	int ret = phy_set_bits_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
+				   BMCR_ANENABLE | BMCR_RESET);
+	if (ret < 0)
+		return ret;
+
+	return mv2222_soft_reset(phydev);
+}
+
+static int mv2222_set_sgmii_speed(struct phy_device *phydev)
+{
+	struct mv2222_data *priv = phydev->priv;
+
+	switch (phydev->speed) {
+	default:
+	case SPEED_1000:
+		if ((linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+				       priv->supported) ||
+		     linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
+				       priv->supported)))
+			return phy_modify_mmd(phydev, MDIO_MMD_PCS,
+					      MV_1GBX_CTRL,
+					      BMCR_SPEED1000 | BMCR_SPEED100,
+					      BMCR_SPEED1000);
+
+		fallthrough;
+	case SPEED_100:
+		if ((linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+				       priv->supported) ||
+		     linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT,
+				       priv->supported)))
+			return phy_modify_mmd(phydev, MDIO_MMD_PCS,
+					      MV_1GBX_CTRL,
+					      BMCR_SPEED1000 | BMCR_SPEED100,
+					      BMCR_SPEED100);
+		fallthrough;
+	case SPEED_10:
+		if ((linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
+				       priv->supported) ||
+		     linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
+				       priv->supported)))
+			return phy_modify_mmd(phydev, MDIO_MMD_PCS,
+					      MV_1GBX_CTRL,
+					      BMCR_SPEED1000 | BMCR_SPEED100,
+					      BMCR_SPEED10);
+
+		return -EINVAL;
+	}
+}
+
+static bool mv2222_is_10g_capable(struct phy_device *phydev)
+{
+	struct mv2222_data *priv = phydev->priv;
+
+	return (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+				  priv->supported) ||
+		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+				  priv->supported) ||
+		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseCR_Full_BIT,
+				  priv->supported) ||
+		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseSR_Full_BIT,
+				  priv->supported) ||
+		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseLR_Full_BIT,
+				  priv->supported) ||
+		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT,
+				  priv->supported) ||
+		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseER_Full_BIT,
+				  priv->supported));
+}
+
+static bool mv2222_is_1gbx_capable(struct phy_device *phydev)
+{
+	struct mv2222_data *priv = phydev->priv;
+
+	return linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
+				 priv->supported);
+}
+
+static int mv2222_config_line(struct phy_device *phydev)
+{
+	struct mv2222_data *priv = phydev->priv;
+
+	switch (priv->line_interface) {
+	case PHY_INTERFACE_MODE_10GBASER:
+		return phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
+				     MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
+	case PHY_INTERFACE_MODE_1000BASEX:
+		return phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
+				     MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
+	case PHY_INTERFACE_MODE_SGMII:
+		return phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
+				     MV_PCS_HOST_XAUI | MV_PCS_LINE_SGMII_AN);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int mv2222_setup_forced(struct phy_device *phydev)
+{
+	struct mv2222_data *priv = phydev->priv;
+	bool changed = false;
+	int ret;
+
+	switch (priv->line_interface) {
+	case PHY_INTERFACE_MODE_10GBASER:
+		if (phydev->speed == SPEED_1000 &&
+		    mv2222_is_1gbx_capable(phydev)) {
+			priv->line_interface = PHY_INTERFACE_MODE_1000BASEX;
+			changed = true;
+		}
+
+		break;
+	case PHY_INTERFACE_MODE_1000BASEX:
+		if (phydev->speed == SPEED_10000 &&
+		    mv2222_is_10g_capable(phydev)) {
+			priv->line_interface = PHY_INTERFACE_MODE_10GBASER;
+			changed = true;
+		}
+
+		break;
+	case PHY_INTERFACE_MODE_SGMII:
+		ret = mv2222_set_sgmii_speed(phydev);
+		if (ret < 0)
+			return ret;
+
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (changed) {
+		ret = mv2222_config_line(phydev);
+		if (ret < 0)
+			return ret;
+	}
+
+	return mv2222_disable_aneg(phydev);
+}
+
+static int mv2222_config_aneg(struct phy_device *phydev)
+{
+	struct mv2222_data *priv = phydev->priv;
+	int ret, adv;
+
+	/* SFP is not present, do nothing */
+	if (priv->line_interface == PHY_INTERFACE_MODE_NA)
+		return 0;
+
+	if (phydev->autoneg == AUTONEG_DISABLE ||
+	    phydev->speed == SPEED_10000)
+		return mv2222_setup_forced(phydev);
+
+	if (priv->line_interface == PHY_INTERFACE_MODE_10GBASER &&
+	    mv2222_is_1gbx_capable(phydev)) {
+		priv->line_interface = PHY_INTERFACE_MODE_1000BASEX;
+		ret = mv2222_config_line(phydev);
+		if (ret < 0)
+			return ret;
+	}
+
+	adv = 0;
+
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
+			      priv->supported))
+		adv |= ADVERTISE_1000XFULL;
+
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
+			      priv->supported))
+		adv |= ADVERTISE_1000XPAUSE;
+
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+			      priv->supported))
+		adv |= ADVERTISE_1000XPSE_ASYM;
+
+	ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_ADVERTISE,
+			     ADVERTISE_1000XFULL |
+			     ADVERTISE_1000XPAUSE | ADVERTISE_1000XPSE_ASYM,
+			     adv);
+	if (ret < 0)
+		return ret;
+
+	return mv2222_enable_aneg(phydev);
+}
+
+static int mv2222_aneg_done(struct phy_device *phydev)
+{
+	int ret;
+
+	if (mv2222_is_10g_capable(phydev)) {
+		ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_STAT1);
+		if (ret < 0)
+			return ret;
+
+		if (ret & MDIO_STAT1_LSTATUS)
+			return 1;
+	}
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_STAT);
+	if (ret < 0)
+		return ret;
+
+	return (ret & BMSR_ANEGCOMPLETE);
+}
+
+static int mv2222_resume(struct phy_device *phydev)
+{
+	return mv2222_tx_enable(phydev);
+}
+
+static int mv2222_suspend(struct phy_device *phydev)
+{
+	return mv2222_tx_disable(phydev);
+}
+
+static int mv2222_get_features(struct phy_device *phydev)
+{
+	/* All supported linkmodes are set at probe */
+
+	return 0;
+}
+
+static int mv2222_config_init(struct phy_device *phydev)
+{
+	if (phydev->interface != PHY_INTERFACE_MODE_XAUI)
+		return -EINVAL;
+
+	phydev->autoneg = AUTONEG_DISABLE;
+
+	return 0;
+}
+
+static int mv2222_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
+{
+	struct phy_device *phydev = upstream;
+	phy_interface_t sfp_interface;
+	struct mv2222_data *priv;
+	struct device *dev;
+	int ret;
+
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(sfp_supported) = { 0, };
+
+	priv = (struct mv2222_data *)phydev->priv;
+	dev = &phydev->mdio.dev;
+
+	sfp_parse_support(phydev->sfp_bus, id, sfp_supported);
+	sfp_interface = sfp_select_interface(phydev->sfp_bus, sfp_supported);
+
+	dev_info(dev, "%s SFP module inserted\n", phy_modes(sfp_interface));
+
+	if (sfp_interface != PHY_INTERFACE_MODE_10GBASER &&
+	    sfp_interface != PHY_INTERFACE_MODE_1000BASEX &&
+	    sfp_interface != PHY_INTERFACE_MODE_SGMII) {
+		dev_err(dev, "Incompatible SFP module inserted\n");
+
+		return -EINVAL;
+	}
+
+	priv->line_interface = sfp_interface;
+	linkmode_and(priv->supported, phydev->supported, sfp_supported);
+
+	ret = mv2222_config_line(phydev);
+	if (ret < 0)
+		return ret;
+
+	if (mutex_trylock(&phydev->lock)) {
+		if (priv->line_interface == PHY_INTERFACE_MODE_10GBASER)
+			ret = mv2222_setup_forced(phydev);
+		else
+			ret = mv2222_config_aneg(phydev);
+
+		mutex_unlock(&phydev->lock);
+	}
+
+	return ret;
+}
+
+static void mv2222_sfp_remove(void *upstream)
+{
+	struct phy_device *phydev = upstream;
+	struct mv2222_data *priv;
+
+	priv = (struct mv2222_data *)phydev->priv;
+
+	priv->line_interface = PHY_INTERFACE_MODE_NA;
+	linkmode_zero(priv->supported);
+}
+
+static const struct sfp_upstream_ops sfp_phy_ops = {
+	.module_insert = mv2222_sfp_insert,
+	.module_remove = mv2222_sfp_remove,
+	.attach = phy_sfp_attach,
+	.detach = phy_sfp_detach,
+};
+
+static int mv2222_probe(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct mv2222_data *priv = NULL;
+
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported) = { 0, };
+
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_TP_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseCR_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseSR_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseLR_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseER_Full_BIT, supported);
+
+	linkmode_copy(phydev->supported, supported);
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->line_interface = PHY_INTERFACE_MODE_NA;
+	phydev->priv = priv;
+
+	return phy_sfp_probe(phydev, &sfp_phy_ops);
+}
+
+static struct phy_driver mv2222_drivers[] = {
+	{
+		.phy_id = MARVELL_PHY_ID_88X2222,
+		.phy_id_mask = MARVELL_PHY_ID_MASK,
+		.name = "Marvell 88X2222",
+		.get_features = mv2222_get_features,
+		.soft_reset = mv2222_soft_reset,
+		.config_init = mv2222_config_init,
+		.config_aneg = mv2222_config_aneg,
+		.aneg_done = mv2222_aneg_done,
+		.probe = mv2222_probe,
+		.suspend = mv2222_suspend,
+		.resume = mv2222_resume,
+		.read_status = mv2222_read_status,
+	},
+};
+module_phy_driver(mv2222_drivers);
+
+static struct mdio_device_id __maybe_unused mv2222_tbl[] = {
+	{ MARVELL_PHY_ID_88X2222, MARVELL_PHY_ID_MASK },
+	{ }
+};
+MODULE_DEVICE_TABLE(mdio, mv2222_tbl);
+
+MODULE_DESCRIPTION("Marvell 88x2222 ethernet transceiver driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/marvell_phy.h b/include/linux/marvell_phy.h
index 52b1610eae68..274abd5fbac3 100644
--- a/include/linux/marvell_phy.h
+++ b/include/linux/marvell_phy.h
@@ -24,6 +24,7 @@
 #define MARVELL_PHY_ID_88E3016		0x01410e60
 #define MARVELL_PHY_ID_88X3310		0x002b09a0
 #define MARVELL_PHY_ID_88E2110		0x002b09b0
+#define MARVELL_PHY_ID_88X2222		0x01410f10
 
 /* Marvel 88E1111 in Finisar SFP module with modified PHY ID */
 #define MARVELL_PHY_ID_88E1111_FINISAR	0x01ff0cc0
-- 
2.20.1

Re: [PATCH v3] net: phy: add Marvell 88X2222 transceiver support

[Russell King - ARM Linux admin]

Hi,

Mostly great, but just a couple more points.

On Wed, Mar 03, 2021 at 01:57:57PM +0300, Ivan Bornyakov wrote:
> +	adv = 0;
> +
> +	if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
> +			      priv->supported))
> +		adv |= ADVERTISE_1000XFULL;
> +
> +	if (linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
> +			      priv->supported))
> +		adv |= ADVERTISE_1000XPAUSE;
> +
> +	if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
> +			      priv->supported))
> +		adv |= ADVERTISE_1000XPSE_ASYM;

You could use the helper:

	adv = linkmode_adv_to_mii_adv_x(priv->supported,
					ETHTOOL_LINK_MODE_1000baseX_Full_BIT);

instead here. It's also a bit weird to set the advertisement based off
a "supported" mask.

> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT, supported);

Does the PHY support backplane links?

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v3] net: phy: add Marvell 88X2222 transceiver support

[Ivan Bornyakov]

On Wed, Mar 03, 2021 at 11:36:55AM +0000, Russell King - ARM Linux admin wrote:
> 
> > +	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT, supported);
> 
> Does the PHY support backplane links?
> 

It looks like it does, but our hardware only have SFP cages, so I'll
drop backplane link modes.

[PATCH v4] net: phy: add Marvell 88X2222 transceiver support

[Ivan Bornyakov]

Add basic support for the Marvell 88X2222 multi-speed ethernet
transceiver.

This PHY provides data transmission over fiber-optic as well as Twinax
copper links. The 88X2222 supports 2 ports of 10GBase-R and 1000Base-X
on the line-side interface. The host-side interface supports 4 ports of
10GBase-R, RXAUI, 1000Base-X and 2 ports of XAUI.

This driver, however, supports only XAUI on the host-side and
1000Base-X/10GBase-R on the line-side, for now. The SGMII is also
supported over 1000Base-X. Interrupts are not supported.

Internal registers access compliant with the Clause 45 specification.

Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
---
 drivers/net/phy/Kconfig           |   6 +
 drivers/net/phy/Makefile          |   1 +
 drivers/net/phy/marvell-88x2222.c | 519 ++++++++++++++++++++++++++++++
 include/linux/marvell_phy.h       |   1 +
 4 files changed, 527 insertions(+)
 create mode 100644 drivers/net/phy/marvell-88x2222.c

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 698bea312adc..a615b3660b05 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -201,6 +201,12 @@ config MARVELL_10G_PHY
 	help
 	  Support for the Marvell Alaska MV88X3310 and compatible PHYs.
 
+config MARVELL_88X2222_PHY
+	tristate "Marvell 88X2222 PHY"
+	help
+	  Support for the Marvell 88X2222 Dual-port Multi-speed Ethernet
+	  Transceiver.
+
 config MICREL_PHY
 	tristate "Micrel PHYs"
 	help
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index a13e402074cf..de683e3abe63 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -63,6 +63,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_88X2222_PHY)	+= marvell-88x2222.o
 obj-$(CONFIG_MESON_GXL_PHY)	+= meson-gxl.o
 obj-$(CONFIG_MICREL_KS8995MA)	+= spi_ks8995.o
 obj-$(CONFIG_MICREL_PHY)	+= micrel.o
diff --git a/drivers/net/phy/marvell-88x2222.c b/drivers/net/phy/marvell-88x2222.c
new file mode 100644
index 000000000000..eca8c2f20684
--- /dev/null
+++ b/drivers/net/phy/marvell-88x2222.c
@@ -0,0 +1,519 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Marvell 88x2222 dual-port multi-speed ethernet transceiver.
+ *
+ * Supports:
+ *	XAUI on the host side.
+ *	1000Base-X or 10GBase-R on the line side.
+ *	SGMII over 1000Base-X.
+ */
+#include <linux/module.h>
+#include <linux/phy.h>
+#include <linux/gpio.h>
+#include <linux/delay.h>
+#include <linux/mdio.h>
+#include <linux/marvell_phy.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/sfp.h>
+#include <linux/netdevice.h>
+
+/* Port PCS Configuration */
+#define	MV_PCS_CONFIG		0xF002
+#define	MV_PCS_HOST_XAUI	0x73
+#define	MV_PCS_LINE_10GBR	(0x71 << 8)
+#define	MV_PCS_LINE_1GBX_AN	(0x7B << 8)
+#define	MV_PCS_LINE_SGMII_AN	(0x7F << 8)
+
+/* Port Reset and Power Down */
+#define	MV_PORT_RST	0xF003
+#define	MV_LINE_RST_SW	BIT(15)
+#define	MV_HOST_RST_SW	BIT(7)
+#define	MV_PORT_RST_SW	(MV_LINE_RST_SW | MV_HOST_RST_SW)
+
+/* 1000Base-X/SGMII Control Register */
+#define	MV_1GBX_CTRL		(0x2000 + MII_BMCR)
+
+/* 1000BASE-X/SGMII Status Register */
+#define	MV_1GBX_STAT		(0x2000 + MII_BMSR)
+
+/* 1000Base-X Auto-Negotiation Advertisement Register */
+#define	MV_1GBX_ADVERTISE	(0x2000 + MII_ADVERTISE)
+
+/* 1000Base-X PHY Specific Status Register */
+#define	MV_1GBX_PHY_STAT		0xA003
+#define	MV_1GBX_PHY_STAT_AN_RESOLVED	BIT(11)
+#define	MV_1GBX_PHY_STAT_DUPLEX		BIT(13)
+#define	MV_1GBX_PHY_STAT_SPEED100	BIT(14)
+#define	MV_1GBX_PHY_STAT_SPEED1000	BIT(15)
+
+struct mv2222_data {
+	phy_interface_t line_interface;
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
+};
+
+/* SFI PMA transmit enable */
+static int mv2222_tx_enable(struct phy_device *phydev)
+{
+	return phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_TXDIS,
+				  MDIO_PMD_TXDIS_GLOBAL);
+}
+
+/* SFI PMA transmit disable */
+static int mv2222_tx_disable(struct phy_device *phydev)
+{
+	return phy_set_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_TXDIS,
+				MDIO_PMD_TXDIS_GLOBAL);
+}
+
+static int mv2222_soft_reset(struct phy_device *phydev)
+{
+	int val, ret;
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PORT_RST,
+			    MV_PORT_RST_SW);
+	if (ret < 0)
+		return ret;
+
+	return phy_read_mmd_poll_timeout(phydev, MDIO_MMD_VEND2, MV_PORT_RST,
+					 val, !(val & MV_PORT_RST_SW),
+					 5000, 1000000, true);
+}
+
+/* Returns negative on error, 0 if link is down, 1 if link is up */
+static int mv2222_read_status_10g(struct phy_device *phydev)
+{
+	int val, link = 0;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_STAT1);
+	if (val < 0)
+		return val;
+
+	if (val & MDIO_STAT1_LSTATUS) {
+		link = 1;
+
+		/* 10GBASE-R do not support auto-negotiation */
+		phydev->autoneg = AUTONEG_DISABLE;
+		phydev->speed = SPEED_10000;
+		phydev->duplex = DUPLEX_FULL;
+	}
+
+	return link;
+}
+
+/* Returns negative on error, 0 if link is down, 1 if link is up */
+static int mv2222_read_status_1g(struct phy_device *phydev)
+{
+	int val, link = 0;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_STAT);
+	if (val < 0)
+		return val;
+
+	if (!(val & BMSR_LSTATUS) ||
+	    (phydev->autoneg == AUTONEG_ENABLE &&
+	     !(val & BMSR_ANEGCOMPLETE)))
+		return 0;
+
+	link = 1;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_PHY_STAT);
+	if (val < 0)
+		return val;
+
+	if (val & MV_1GBX_PHY_STAT_AN_RESOLVED) {
+		if (val & MV_1GBX_PHY_STAT_DUPLEX)
+			phydev->duplex = DUPLEX_FULL;
+		else
+			phydev->duplex = DUPLEX_HALF;
+
+		if (val & MV_1GBX_PHY_STAT_SPEED1000)
+			phydev->speed = SPEED_1000;
+		else if (val & MV_1GBX_PHY_STAT_SPEED100)
+			phydev->speed = SPEED_100;
+		else
+			phydev->speed = SPEED_10;
+	}
+
+	return link;
+}
+
+static int mv2222_read_status(struct phy_device *phydev)
+{
+	struct mv2222_data *priv = phydev->priv;
+	int link;
+
+	phydev->link = 0;
+	phydev->speed = SPEED_UNKNOWN;
+	phydev->duplex = DUPLEX_UNKNOWN;
+
+	if (priv->line_interface == PHY_INTERFACE_MODE_10GBASER)
+		link = mv2222_read_status_10g(phydev);
+	else
+		link = mv2222_read_status_1g(phydev);
+
+	if (link < 0)
+		return link;
+
+	phydev->link = link;
+
+	return 0;
+}
+
+static int mv2222_disable_aneg(struct phy_device *phydev)
+{
+	int ret = phy_clear_bits_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
+				     BMCR_ANENABLE | BMCR_ANRESTART);
+	if (ret < 0)
+		return ret;
+
+	return mv2222_soft_reset(phydev);
+}
+
+static int mv2222_enable_aneg(struct phy_device *phydev)
+{
+	int ret = phy_set_bits_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
+				   BMCR_ANENABLE | BMCR_RESET);
+	if (ret < 0)
+		return ret;
+
+	return mv2222_soft_reset(phydev);
+}
+
+static int mv2222_set_sgmii_speed(struct phy_device *phydev)
+{
+	struct mv2222_data *priv = phydev->priv;
+
+	switch (phydev->speed) {
+	default:
+	case SPEED_1000:
+		if ((linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+				       priv->supported) ||
+		     linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
+				       priv->supported)))
+			return phy_modify_mmd(phydev, MDIO_MMD_PCS,
+					      MV_1GBX_CTRL,
+					      BMCR_SPEED1000 | BMCR_SPEED100,
+					      BMCR_SPEED1000);
+
+		fallthrough;
+	case SPEED_100:
+		if ((linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+				       priv->supported) ||
+		     linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT,
+				       priv->supported)))
+			return phy_modify_mmd(phydev, MDIO_MMD_PCS,
+					      MV_1GBX_CTRL,
+					      BMCR_SPEED1000 | BMCR_SPEED100,
+					      BMCR_SPEED100);
+		fallthrough;
+	case SPEED_10:
+		if ((linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
+				       priv->supported) ||
+		     linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
+				       priv->supported)))
+			return phy_modify_mmd(phydev, MDIO_MMD_PCS,
+					      MV_1GBX_CTRL,
+					      BMCR_SPEED1000 | BMCR_SPEED100,
+					      BMCR_SPEED10);
+
+		return -EINVAL;
+	}
+}
+
+static bool mv2222_is_10g_capable(struct phy_device *phydev)
+{
+	struct mv2222_data *priv = phydev->priv;
+
+	return (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+				  priv->supported) ||
+		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseCR_Full_BIT,
+				  priv->supported) ||
+		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseSR_Full_BIT,
+				  priv->supported) ||
+		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseLR_Full_BIT,
+				  priv->supported) ||
+		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT,
+				  priv->supported) ||
+		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseER_Full_BIT,
+				  priv->supported));
+}
+
+static bool mv2222_is_1gbx_capable(struct phy_device *phydev)
+{
+	struct mv2222_data *priv = phydev->priv;
+
+	return linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
+				 priv->supported);
+}
+
+static int mv2222_config_line(struct phy_device *phydev)
+{
+	struct mv2222_data *priv = phydev->priv;
+
+	switch (priv->line_interface) {
+	case PHY_INTERFACE_MODE_10GBASER:
+		return phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
+				     MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
+	case PHY_INTERFACE_MODE_1000BASEX:
+		return phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
+				     MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
+	case PHY_INTERFACE_MODE_SGMII:
+		return phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
+				     MV_PCS_HOST_XAUI | MV_PCS_LINE_SGMII_AN);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int mv2222_setup_forced(struct phy_device *phydev)
+{
+	struct mv2222_data *priv = phydev->priv;
+	bool changed = false;
+	int ret;
+
+	switch (priv->line_interface) {
+	case PHY_INTERFACE_MODE_10GBASER:
+		if (phydev->speed == SPEED_1000 &&
+		    mv2222_is_1gbx_capable(phydev)) {
+			priv->line_interface = PHY_INTERFACE_MODE_1000BASEX;
+			changed = true;
+		}
+
+		break;
+	case PHY_INTERFACE_MODE_1000BASEX:
+		if (phydev->speed == SPEED_10000 &&
+		    mv2222_is_10g_capable(phydev)) {
+			priv->line_interface = PHY_INTERFACE_MODE_10GBASER;
+			changed = true;
+		}
+
+		break;
+	case PHY_INTERFACE_MODE_SGMII:
+		ret = mv2222_set_sgmii_speed(phydev);
+		if (ret < 0)
+			return ret;
+
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (changed) {
+		ret = mv2222_config_line(phydev);
+		if (ret < 0)
+			return ret;
+	}
+
+	return mv2222_disable_aneg(phydev);
+}
+
+static int mv2222_config_aneg(struct phy_device *phydev)
+{
+	struct mv2222_data *priv = phydev->priv;
+	int ret, adv;
+
+	/* SFP is not present, do nothing */
+	if (priv->line_interface == PHY_INTERFACE_MODE_NA)
+		return 0;
+
+	if (phydev->autoneg == AUTONEG_DISABLE ||
+	    phydev->speed == SPEED_10000)
+		return mv2222_setup_forced(phydev);
+
+	if (priv->line_interface == PHY_INTERFACE_MODE_10GBASER &&
+	    mv2222_is_1gbx_capable(phydev)) {
+		priv->line_interface = PHY_INTERFACE_MODE_1000BASEX;
+		ret = mv2222_config_line(phydev);
+		if (ret < 0)
+			return ret;
+	}
+
+	adv = linkmode_adv_to_mii_adv_x(priv->supported,
+					ETHTOOL_LINK_MODE_1000baseX_Full_BIT);
+
+	ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_ADVERTISE,
+			     ADVERTISE_1000XFULL |
+			     ADVERTISE_1000XPAUSE | ADVERTISE_1000XPSE_ASYM,
+			     adv);
+	if (ret < 0)
+		return ret;
+
+	return mv2222_enable_aneg(phydev);
+}
+
+static int mv2222_aneg_done(struct phy_device *phydev)
+{
+	int ret;
+
+	if (mv2222_is_10g_capable(phydev)) {
+		ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_STAT1);
+		if (ret < 0)
+			return ret;
+
+		if (ret & MDIO_STAT1_LSTATUS)
+			return 1;
+	}
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_STAT);
+	if (ret < 0)
+		return ret;
+
+	return (ret & BMSR_ANEGCOMPLETE);
+}
+
+static int mv2222_resume(struct phy_device *phydev)
+{
+	return mv2222_tx_enable(phydev);
+}
+
+static int mv2222_suspend(struct phy_device *phydev)
+{
+	return mv2222_tx_disable(phydev);
+}
+
+static int mv2222_get_features(struct phy_device *phydev)
+{
+	/* All supported linkmodes are set at probe */
+
+	return 0;
+}
+
+static int mv2222_config_init(struct phy_device *phydev)
+{
+	if (phydev->interface != PHY_INTERFACE_MODE_XAUI)
+		return -EINVAL;
+
+	phydev->autoneg = AUTONEG_DISABLE;
+
+	return 0;
+}
+
+static int mv2222_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
+{
+	struct phy_device *phydev = upstream;
+	phy_interface_t sfp_interface;
+	struct mv2222_data *priv;
+	struct device *dev;
+	int ret;
+
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(sfp_supported) = { 0, };
+
+	priv = (struct mv2222_data *)phydev->priv;
+	dev = &phydev->mdio.dev;
+
+	sfp_parse_support(phydev->sfp_bus, id, sfp_supported);
+	sfp_interface = sfp_select_interface(phydev->sfp_bus, sfp_supported);
+
+	dev_info(dev, "%s SFP module inserted\n", phy_modes(sfp_interface));
+
+	if (sfp_interface != PHY_INTERFACE_MODE_10GBASER &&
+	    sfp_interface != PHY_INTERFACE_MODE_1000BASEX &&
+	    sfp_interface != PHY_INTERFACE_MODE_SGMII) {
+		dev_err(dev, "Incompatible SFP module inserted\n");
+
+		return -EINVAL;
+	}
+
+	priv->line_interface = sfp_interface;
+	linkmode_and(priv->supported, phydev->supported, sfp_supported);
+
+	ret = mv2222_config_line(phydev);
+	if (ret < 0)
+		return ret;
+
+	if (mutex_trylock(&phydev->lock)) {
+		if (priv->line_interface == PHY_INTERFACE_MODE_10GBASER)
+			ret = mv2222_setup_forced(phydev);
+		else
+			ret = mv2222_config_aneg(phydev);
+
+		mutex_unlock(&phydev->lock);
+	}
+
+	return ret;
+}
+
+static void mv2222_sfp_remove(void *upstream)
+{
+	struct phy_device *phydev = upstream;
+	struct mv2222_data *priv;
+
+	priv = (struct mv2222_data *)phydev->priv;
+
+	priv->line_interface = PHY_INTERFACE_MODE_NA;
+	linkmode_zero(priv->supported);
+}
+
+static const struct sfp_upstream_ops sfp_phy_ops = {
+	.module_insert = mv2222_sfp_insert,
+	.module_remove = mv2222_sfp_remove,
+	.attach = phy_sfp_attach,
+	.detach = phy_sfp_detach,
+};
+
+static int mv2222_probe(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct mv2222_data *priv = NULL;
+
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported) = { 0, };
+
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_TP_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseCR_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseSR_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseLR_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT, supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseER_Full_BIT, supported);
+
+	linkmode_copy(phydev->supported, supported);
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->line_interface = PHY_INTERFACE_MODE_NA;
+	phydev->priv = priv;
+
+	return phy_sfp_probe(phydev, &sfp_phy_ops);
+}
+
+static struct phy_driver mv2222_drivers[] = {
+	{
+		.phy_id = MARVELL_PHY_ID_88X2222,
+		.phy_id_mask = MARVELL_PHY_ID_MASK,
+		.name = "Marvell 88X2222",
+		.get_features = mv2222_get_features,
+		.soft_reset = mv2222_soft_reset,
+		.config_init = mv2222_config_init,
+		.config_aneg = mv2222_config_aneg,
+		.aneg_done = mv2222_aneg_done,
+		.probe = mv2222_probe,
+		.suspend = mv2222_suspend,
+		.resume = mv2222_resume,
+		.read_status = mv2222_read_status,
+	},
+};
+module_phy_driver(mv2222_drivers);
+
+static struct mdio_device_id __maybe_unused mv2222_tbl[] = {
+	{ MARVELL_PHY_ID_88X2222, MARVELL_PHY_ID_MASK },
+	{ }
+};
+MODULE_DEVICE_TABLE(mdio, mv2222_tbl);
+
+MODULE_DESCRIPTION("Marvell 88x2222 ethernet transceiver driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/marvell_phy.h b/include/linux/marvell_phy.h
index 52b1610eae68..274abd5fbac3 100644
--- a/include/linux/marvell_phy.h
+++ b/include/linux/marvell_phy.h
@@ -24,6 +24,7 @@
 #define MARVELL_PHY_ID_88E3016		0x01410e60
 #define MARVELL_PHY_ID_88X3310		0x002b09a0
 #define MARVELL_PHY_ID_88E2110		0x002b09b0
+#define MARVELL_PHY_ID_88X2222		0x01410f10
 
 /* Marvel 88E1111 in Finisar SFP module with modified PHY ID */
 #define MARVELL_PHY_ID_88E1111_FINISAR	0x01ff0cc0
-- 
2.20.1

Re: [PATCH v4] net: phy: add Marvell 88X2222 transceiver support

[Ivan Bornyakov]

On Wed, Mar 03, 2021 at 07:02:11PM +0300, Ivan Bornyakov wrote:
> Add basic support for the Marvell 88X2222 multi-speed ethernet
> transceiver.
> 
> This PHY provides data transmission over fiber-optic as well as Twinax
> copper links. The 88X2222 supports 2 ports of 10GBase-R and 1000Base-X
> on the line-side interface. The host-side interface supports 4 ports of
> 10GBase-R, RXAUI, 1000Base-X and 2 ports of XAUI.
> 
> This driver, however, supports only XAUI on the host-side and
> 1000Base-X/10GBase-R on the line-side, for now. The SGMII is also
> supported over 1000Base-X. Interrupts are not supported.
> 
> Internal registers access compliant with the Clause 45 specification.
> 
> Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> ---
>  drivers/net/phy/Kconfig           |   6 +
>  drivers/net/phy/Makefile          |   1 +
>  drivers/net/phy/marvell-88x2222.c | 519 ++++++++++++++++++++++++++++++
>  include/linux/marvell_phy.h       |   1 +
>  4 files changed, 527 insertions(+)
>  create mode 100644 drivers/net/phy/marvell-88x2222.c
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 698bea312adc..a615b3660b05 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -201,6 +201,12 @@ config MARVELL_10G_PHY
>  	help
>  	  Support for the Marvell Alaska MV88X3310 and compatible PHYs.
>  
> +config MARVELL_88X2222_PHY
> +	tristate "Marvell 88X2222 PHY"
> +	help
> +	  Support for the Marvell 88X2222 Dual-port Multi-speed Ethernet
> +	  Transceiver.
> +
>  config MICREL_PHY
>  	tristate "Micrel PHYs"
>  	help
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index a13e402074cf..de683e3abe63 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -63,6 +63,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_88X2222_PHY)	+= marvell-88x2222.o
>  obj-$(CONFIG_MESON_GXL_PHY)	+= meson-gxl.o
>  obj-$(CONFIG_MICREL_KS8995MA)	+= spi_ks8995.o
>  obj-$(CONFIG_MICREL_PHY)	+= micrel.o
> diff --git a/drivers/net/phy/marvell-88x2222.c b/drivers/net/phy/marvell-88x2222.c
> new file mode 100644
> index 000000000000..eca8c2f20684
> --- /dev/null
> +++ b/drivers/net/phy/marvell-88x2222.c
> @@ -0,0 +1,519 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Marvell 88x2222 dual-port multi-speed ethernet transceiver.
> + *
> + * Supports:
> + *	XAUI on the host side.
> + *	1000Base-X or 10GBase-R on the line side.
> + *	SGMII over 1000Base-X.
> + */
> +#include <linux/module.h>
> +#include <linux/phy.h>
> +#include <linux/gpio.h>
> +#include <linux/delay.h>
> +#include <linux/mdio.h>
> +#include <linux/marvell_phy.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>
> +#include <linux/sfp.h>
> +#include <linux/netdevice.h>
> +
> +/* Port PCS Configuration */
> +#define	MV_PCS_CONFIG		0xF002
> +#define	MV_PCS_HOST_XAUI	0x73
> +#define	MV_PCS_LINE_10GBR	(0x71 << 8)
> +#define	MV_PCS_LINE_1GBX_AN	(0x7B << 8)
> +#define	MV_PCS_LINE_SGMII_AN	(0x7F << 8)
> +
> +/* Port Reset and Power Down */
> +#define	MV_PORT_RST	0xF003
> +#define	MV_LINE_RST_SW	BIT(15)
> +#define	MV_HOST_RST_SW	BIT(7)
> +#define	MV_PORT_RST_SW	(MV_LINE_RST_SW | MV_HOST_RST_SW)
> +
> +/* 1000Base-X/SGMII Control Register */
> +#define	MV_1GBX_CTRL		(0x2000 + MII_BMCR)
> +
> +/* 1000BASE-X/SGMII Status Register */
> +#define	MV_1GBX_STAT		(0x2000 + MII_BMSR)
> +
> +/* 1000Base-X Auto-Negotiation Advertisement Register */
> +#define	MV_1GBX_ADVERTISE	(0x2000 + MII_ADVERTISE)
> +
> +/* 1000Base-X PHY Specific Status Register */
> +#define	MV_1GBX_PHY_STAT		0xA003
> +#define	MV_1GBX_PHY_STAT_AN_RESOLVED	BIT(11)
> +#define	MV_1GBX_PHY_STAT_DUPLEX		BIT(13)
> +#define	MV_1GBX_PHY_STAT_SPEED100	BIT(14)
> +#define	MV_1GBX_PHY_STAT_SPEED1000	BIT(15)
> +
> +struct mv2222_data {
> +	phy_interface_t line_interface;
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
> +};
> +
> +/* SFI PMA transmit enable */
> +static int mv2222_tx_enable(struct phy_device *phydev)
> +{
> +	return phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_TXDIS,
> +				  MDIO_PMD_TXDIS_GLOBAL);
> +}
> +
> +/* SFI PMA transmit disable */
> +static int mv2222_tx_disable(struct phy_device *phydev)
> +{
> +	return phy_set_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_TXDIS,
> +				MDIO_PMD_TXDIS_GLOBAL);
> +}
> +
> +static int mv2222_soft_reset(struct phy_device *phydev)
> +{
> +	int val, ret;
> +
> +	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PORT_RST,
> +			    MV_PORT_RST_SW);
> +	if (ret < 0)
> +		return ret;
> +
> +	return phy_read_mmd_poll_timeout(phydev, MDIO_MMD_VEND2, MV_PORT_RST,
> +					 val, !(val & MV_PORT_RST_SW),
> +					 5000, 1000000, true);
> +}
> +
> +/* Returns negative on error, 0 if link is down, 1 if link is up */
> +static int mv2222_read_status_10g(struct phy_device *phydev)
> +{
> +	int val, link = 0;
> +
> +	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_STAT1);
> +	if (val < 0)
> +		return val;
> +
> +	if (val & MDIO_STAT1_LSTATUS) {
> +		link = 1;
> +
> +		/* 10GBASE-R do not support auto-negotiation */
> +		phydev->autoneg = AUTONEG_DISABLE;
> +		phydev->speed = SPEED_10000;
> +		phydev->duplex = DUPLEX_FULL;
> +	}
> +
> +	return link;
> +}
> +
> +/* Returns negative on error, 0 if link is down, 1 if link is up */
> +static int mv2222_read_status_1g(struct phy_device *phydev)
> +{
> +	int val, link = 0;
> +
> +	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_STAT);
> +	if (val < 0)
> +		return val;
> +
> +	if (!(val & BMSR_LSTATUS) ||
> +	    (phydev->autoneg == AUTONEG_ENABLE &&
> +	     !(val & BMSR_ANEGCOMPLETE)))
> +		return 0;
> +
> +	link = 1;
> +
> +	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_PHY_STAT);
> +	if (val < 0)
> +		return val;
> +
> +	if (val & MV_1GBX_PHY_STAT_AN_RESOLVED) {
> +		if (val & MV_1GBX_PHY_STAT_DUPLEX)
> +			phydev->duplex = DUPLEX_FULL;
> +		else
> +			phydev->duplex = DUPLEX_HALF;
> +
> +		if (val & MV_1GBX_PHY_STAT_SPEED1000)
> +			phydev->speed = SPEED_1000;
> +		else if (val & MV_1GBX_PHY_STAT_SPEED100)
> +			phydev->speed = SPEED_100;
> +		else
> +			phydev->speed = SPEED_10;
> +	}
> +
> +	return link;
> +}
> +
> +static int mv2222_read_status(struct phy_device *phydev)
> +{
> +	struct mv2222_data *priv = phydev->priv;
> +	int link;
> +
> +	phydev->link = 0;
> +	phydev->speed = SPEED_UNKNOWN;
> +	phydev->duplex = DUPLEX_UNKNOWN;
> +
> +	if (priv->line_interface == PHY_INTERFACE_MODE_10GBASER)
> +		link = mv2222_read_status_10g(phydev);
> +	else
> +		link = mv2222_read_status_1g(phydev);
> +
> +	if (link < 0)
> +		return link;
> +
> +	phydev->link = link;
> +
> +	return 0;
> +}
> +
> +static int mv2222_disable_aneg(struct phy_device *phydev)
> +{
> +	int ret = phy_clear_bits_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
> +				     BMCR_ANENABLE | BMCR_ANRESTART);
> +	if (ret < 0)
> +		return ret;
> +
> +	return mv2222_soft_reset(phydev);
> +}
> +
> +static int mv2222_enable_aneg(struct phy_device *phydev)
> +{
> +	int ret = phy_set_bits_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_CTRL,
> +				   BMCR_ANENABLE | BMCR_RESET);
> +	if (ret < 0)
> +		return ret;
> +
> +	return mv2222_soft_reset(phydev);
> +}
> +
> +static int mv2222_set_sgmii_speed(struct phy_device *phydev)
> +{
> +	struct mv2222_data *priv = phydev->priv;
> +
> +	switch (phydev->speed) {
> +	default:
> +	case SPEED_1000:
> +		if ((linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
> +				       priv->supported) ||
> +		     linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
> +				       priv->supported)))
> +			return phy_modify_mmd(phydev, MDIO_MMD_PCS,
> +					      MV_1GBX_CTRL,
> +					      BMCR_SPEED1000 | BMCR_SPEED100,
> +					      BMCR_SPEED1000);
> +
> +		fallthrough;
> +	case SPEED_100:
> +		if ((linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
> +				       priv->supported) ||
> +		     linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT,
> +				       priv->supported)))
> +			return phy_modify_mmd(phydev, MDIO_MMD_PCS,
> +					      MV_1GBX_CTRL,
> +					      BMCR_SPEED1000 | BMCR_SPEED100,
> +					      BMCR_SPEED100);
> +		fallthrough;
> +	case SPEED_10:
> +		if ((linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
> +				       priv->supported) ||
> +		     linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
> +				       priv->supported)))
> +			return phy_modify_mmd(phydev, MDIO_MMD_PCS,
> +					      MV_1GBX_CTRL,
> +					      BMCR_SPEED1000 | BMCR_SPEED100,
> +					      BMCR_SPEED10);
> +
> +		return -EINVAL;
> +	}
> +}
> +
> +static bool mv2222_is_10g_capable(struct phy_device *phydev)
> +{
> +	struct mv2222_data *priv = phydev->priv;
> +
> +	return (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
> +				  priv->supported) ||
> +		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseCR_Full_BIT,
> +				  priv->supported) ||
> +		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseSR_Full_BIT,
> +				  priv->supported) ||
> +		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseLR_Full_BIT,
> +				  priv->supported) ||
> +		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT,
> +				  priv->supported) ||
> +		linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseER_Full_BIT,
> +				  priv->supported));
> +}
> +
> +static bool mv2222_is_1gbx_capable(struct phy_device *phydev)
> +{
> +	struct mv2222_data *priv = phydev->priv;
> +
> +	return linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
> +				 priv->supported);
> +}
> +
> +static int mv2222_config_line(struct phy_device *phydev)
> +{
> +	struct mv2222_data *priv = phydev->priv;
> +
> +	switch (priv->line_interface) {
> +	case PHY_INTERFACE_MODE_10GBASER:
> +		return phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> +				     MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
> +	case PHY_INTERFACE_MODE_1000BASEX:
> +		return phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> +				     MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
> +	case PHY_INTERFACE_MODE_SGMII:
> +		return phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> +				     MV_PCS_HOST_XAUI | MV_PCS_LINE_SGMII_AN);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int mv2222_setup_forced(struct phy_device *phydev)
> +{
> +	struct mv2222_data *priv = phydev->priv;
> +	bool changed = false;
> +	int ret;
> +
> +	switch (priv->line_interface) {
> +	case PHY_INTERFACE_MODE_10GBASER:
> +		if (phydev->speed == SPEED_1000 &&
> +		    mv2222_is_1gbx_capable(phydev)) {
> +			priv->line_interface = PHY_INTERFACE_MODE_1000BASEX;
> +			changed = true;
> +		}
> +
> +		break;
> +	case PHY_INTERFACE_MODE_1000BASEX:
> +		if (phydev->speed == SPEED_10000 &&
> +		    mv2222_is_10g_capable(phydev)) {
> +			priv->line_interface = PHY_INTERFACE_MODE_10GBASER;
> +			changed = true;
> +		}
> +
> +		break;
> +	case PHY_INTERFACE_MODE_SGMII:
> +		ret = mv2222_set_sgmii_speed(phydev);
> +		if (ret < 0)
> +			return ret;
> +
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (changed) {
> +		ret = mv2222_config_line(phydev);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return mv2222_disable_aneg(phydev);
> +}
> +
> +static int mv2222_config_aneg(struct phy_device *phydev)
> +{
> +	struct mv2222_data *priv = phydev->priv;
> +	int ret, adv;
> +
> +	/* SFP is not present, do nothing */
> +	if (priv->line_interface == PHY_INTERFACE_MODE_NA)
> +		return 0;
> +
> +	if (phydev->autoneg == AUTONEG_DISABLE ||
> +	    phydev->speed == SPEED_10000)
> +		return mv2222_setup_forced(phydev);
> +
> +	if (priv->line_interface == PHY_INTERFACE_MODE_10GBASER &&
> +	    mv2222_is_1gbx_capable(phydev)) {
> +		priv->line_interface = PHY_INTERFACE_MODE_1000BASEX;
> +		ret = mv2222_config_line(phydev);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	adv = linkmode_adv_to_mii_adv_x(priv->supported,
> +					ETHTOOL_LINK_MODE_1000baseX_Full_BIT);
> +
> +	ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_ADVERTISE,
> +			     ADVERTISE_1000XFULL |
> +			     ADVERTISE_1000XPAUSE | ADVERTISE_1000XPSE_ASYM,
> +			     adv);
> +	if (ret < 0)
> +		return ret;
> +
> +	return mv2222_enable_aneg(phydev);
> +}
> +
> +static int mv2222_aneg_done(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	if (mv2222_is_10g_capable(phydev)) {
> +		ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_STAT1);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (ret & MDIO_STAT1_LSTATUS)
> +			return 1;
> +	}
> +
> +	ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MV_1GBX_STAT);
> +	if (ret < 0)
> +		return ret;
> +
> +	return (ret & BMSR_ANEGCOMPLETE);
> +}
> +
> +static int mv2222_resume(struct phy_device *phydev)
> +{
> +	return mv2222_tx_enable(phydev);
> +}
> +
> +static int mv2222_suspend(struct phy_device *phydev)
> +{
> +	return mv2222_tx_disable(phydev);
> +}
> +
> +static int mv2222_get_features(struct phy_device *phydev)
> +{
> +	/* All supported linkmodes are set at probe */
> +
> +	return 0;
> +}
> +
> +static int mv2222_config_init(struct phy_device *phydev)
> +{
> +	if (phydev->interface != PHY_INTERFACE_MODE_XAUI)
> +		return -EINVAL;
> +
> +	phydev->autoneg = AUTONEG_DISABLE;
> +
> +	return 0;
> +}
> +
> +static int mv2222_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
> +{
> +	struct phy_device *phydev = upstream;
> +	phy_interface_t sfp_interface;
> +	struct mv2222_data *priv;
> +	struct device *dev;
> +	int ret;
> +
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(sfp_supported) = { 0, };
> +
> +	priv = (struct mv2222_data *)phydev->priv;
> +	dev = &phydev->mdio.dev;
> +
> +	sfp_parse_support(phydev->sfp_bus, id, sfp_supported);
> +	sfp_interface = sfp_select_interface(phydev->sfp_bus, sfp_supported);
> +
> +	dev_info(dev, "%s SFP module inserted\n", phy_modes(sfp_interface));
> +
> +	if (sfp_interface != PHY_INTERFACE_MODE_10GBASER &&
> +	    sfp_interface != PHY_INTERFACE_MODE_1000BASEX &&
> +	    sfp_interface != PHY_INTERFACE_MODE_SGMII) {
> +		dev_err(dev, "Incompatible SFP module inserted\n");
> +
> +		return -EINVAL;
> +	}
> +
> +	priv->line_interface = sfp_interface;
> +	linkmode_and(priv->supported, phydev->supported, sfp_supported);
> +
> +	ret = mv2222_config_line(phydev);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (mutex_trylock(&phydev->lock)) {
> +		if (priv->line_interface == PHY_INTERFACE_MODE_10GBASER)
> +			ret = mv2222_setup_forced(phydev);
> +		else
> +			ret = mv2222_config_aneg(phydev);
> +
> +		mutex_unlock(&phydev->lock);
> +	}
> +
> +	return ret;
> +}
> +
> +static void mv2222_sfp_remove(void *upstream)
> +{
> +	struct phy_device *phydev = upstream;
> +	struct mv2222_data *priv;
> +
> +	priv = (struct mv2222_data *)phydev->priv;
> +
> +	priv->line_interface = PHY_INTERFACE_MODE_NA;
> +	linkmode_zero(priv->supported);
> +}
> +
> +static const struct sfp_upstream_ops sfp_phy_ops = {
> +	.module_insert = mv2222_sfp_insert,
> +	.module_remove = mv2222_sfp_remove,
> +	.attach = phy_sfp_attach,
> +	.detach = phy_sfp_detach,
> +};
> +
> +static int mv2222_probe(struct phy_device *phydev)
> +{
> +	struct device *dev = &phydev->mdio.dev;
> +	struct mv2222_data *priv = NULL;
> +
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported) = { 0, };
> +
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_TP_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseCR_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseSR_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseLR_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseER_Full_BIT, supported);
> +
> +	linkmode_copy(phydev->supported, supported);
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->line_interface = PHY_INTERFACE_MODE_NA;
> +	phydev->priv = priv;
> +
> +	return phy_sfp_probe(phydev, &sfp_phy_ops);
> +}
> +
> +static struct phy_driver mv2222_drivers[] = {
> +	{
> +		.phy_id = MARVELL_PHY_ID_88X2222,
> +		.phy_id_mask = MARVELL_PHY_ID_MASK,
> +		.name = "Marvell 88X2222",
> +		.get_features = mv2222_get_features,
> +		.soft_reset = mv2222_soft_reset,
> +		.config_init = mv2222_config_init,
> +		.config_aneg = mv2222_config_aneg,
> +		.aneg_done = mv2222_aneg_done,
> +		.probe = mv2222_probe,
> +		.suspend = mv2222_suspend,
> +		.resume = mv2222_resume,
> +		.read_status = mv2222_read_status,
> +	},
> +};
> +module_phy_driver(mv2222_drivers);
> +
> +static struct mdio_device_id __maybe_unused mv2222_tbl[] = {
> +	{ MARVELL_PHY_ID_88X2222, MARVELL_PHY_ID_MASK },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(mdio, mv2222_tbl);
> +
> +MODULE_DESCRIPTION("Marvell 88x2222 ethernet transceiver driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/marvell_phy.h b/include/linux/marvell_phy.h
> index 52b1610eae68..274abd5fbac3 100644
> --- a/include/linux/marvell_phy.h
> +++ b/include/linux/marvell_phy.h
> @@ -24,6 +24,7 @@
>  #define MARVELL_PHY_ID_88E3016		0x01410e60
>  #define MARVELL_PHY_ID_88X3310		0x002b09a0
>  #define MARVELL_PHY_ID_88E2110		0x002b09b0
> +#define MARVELL_PHY_ID_88X2222		0x01410f10
>  
>  /* Marvel 88E1111 in Finisar SFP module with modified PHY ID */
>  #define MARVELL_PHY_ID_88E1111_FINISAR	0x01ff0cc0
> -- 
> 2.20.1
> 

Friendly ping.
Is this version good enough already?