[PATCH net-next v2 0/4] net: mvpp2: 1000BaseX and 2000BaseX support

[PATCH net-next v2 0/4] net: mvpp2: 1000BaseX and 2000BaseX support

[Antoine Tenart]

Hi all,

This series adds 1000BaseX and 2500BaseX support to the Marvell PPv2
driver. In order to use it, the 2.5 SGMII mode is added in the Marvell
common PHY driver (cp110-comphy).

This was tested on a mcbin.

All patches should probably go through net-next as patch 4/4 depends on
patch 1/4 to build and work.

Please note the two mvpp2 patches do not conflict with the ACPI series
v2 Marcin sent a few days ago, and the two series can be processed in
parallel. (Marcin is aware of me sending this series).

Thanks!
Antoine

Since v1:
  - s/PHY_MODE_SGMII_2_5G/PHY_MODE_2500SGMII/
  - Fixed a build error in 'net: mvpp2: 1000baseX support' (which was solved in
    the 2500baseX support one, but the bisection was broken).
  - Removed the dt patches, as the fourth network interface on the mcbin also
    needs PHYLINK support in the PPv2 driver to be correctly supported.

Antoine Tenart (4):
  phy: add 2.5G SGMII mode to the phy_mode enum
  phy: cp110-comphy: 2.5G SGMII mode
  net: mvpp2: 1000baseX support
  net: mvpp2: 2500baseX support

 drivers/net/ethernet/marvell/mvpp2.c         | 67 +++++++++++++++++++++++-----
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 17 +++++--
 include/linux/phy/phy.h                      |  1 +
 3 files changed, 72 insertions(+), 13 deletions(-)

-- 
2.14.3

[PATCH net-next v2 1/4] phy: add 2.5G SGMII mode to the phy_mode enum

[Antoine Tenart]

This patch adds one more generic PHY mode to the phy_mode enum, to allow
configuring generic PHYs to the 2.5G SGMII mode by using the set_mode
callback.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 include/linux/phy/phy.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 4f8423a948d5..5a80e9de3686 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -28,6 +28,7 @@ enum phy_mode {
 	PHY_MODE_USB_DEVICE,
 	PHY_MODE_USB_OTG,
 	PHY_MODE_SGMII,
+	PHY_MODE_2500SGMII,
 	PHY_MODE_10GKR,
 	PHY_MODE_UFS_HS_A,
 	PHY_MODE_UFS_HS_B,
-- 
2.14.3

[PATCH net-next v2 2/4] phy: cp110-comphy: 2.5G SGMII mode

[Antoine Tenart]

This patch allow the CP100 comphy to configure some lanes in the
2.5G SGMII mode. This mode is quite close to SGMII and uses nearly the
same code path.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
index a0d522154cdf..4ef429250d7b 100644
--- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
+++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
@@ -135,19 +135,25 @@ struct mvebu_comhy_conf {
 static const struct mvebu_comhy_conf mvebu_comphy_cp110_modes[] = {
 	/* lane 0 */
 	MVEBU_COMPHY_CONF(0, 1, PHY_MODE_SGMII, 0x1),
+	MVEBU_COMPHY_CONF(0, 1, PHY_MODE_2500SGMII, 0x1),
 	/* lane 1 */
 	MVEBU_COMPHY_CONF(1, 2, PHY_MODE_SGMII, 0x1),
+	MVEBU_COMPHY_CONF(1, 2, PHY_MODE_2500SGMII, 0x1),
 	/* lane 2 */
 	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_SGMII, 0x1),
+	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_2500SGMII, 0x1),
 	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_10GKR, 0x1),
 	/* lane 3 */
 	MVEBU_COMPHY_CONF(3, 1, PHY_MODE_SGMII, 0x2),
+	MVEBU_COMPHY_CONF(3, 1, PHY_MODE_2500SGMII, 0x2),
 	/* lane 4 */
 	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_SGMII, 0x2),
+	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_2500SGMII, 0x2),
 	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_10GKR, 0x2),
 	MVEBU_COMPHY_CONF(4, 1, PHY_MODE_SGMII, 0x1),
 	/* lane 5 */
 	MVEBU_COMPHY_CONF(5, 2, PHY_MODE_SGMII, 0x1),
+	MVEBU_COMPHY_CONF(5, 2, PHY_MODE_2500SGMII, 0x1),
 };
 
 struct mvebu_comphy_priv {
@@ -206,6 +212,10 @@ static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
 	if (mode == PHY_MODE_10GKR)
 		val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0xe) |
 		       MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0xe);
+	else if (mode == PHY_MODE_2500SGMII)
+		val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0x8) |
+		       MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0x8) |
+		       MVEBU_COMPHY_SERDES_CFG0_HALF_BUS;
 	else if (mode == PHY_MODE_SGMII)
 		val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0x6) |
 		       MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0x6) |
@@ -296,13 +306,13 @@ static int mvebu_comphy_init_plls(struct mvebu_comphy_lane *lane,
 	return 0;
 }
 
-static int mvebu_comphy_set_mode_sgmii(struct phy *phy)
+static int mvebu_comphy_set_mode_sgmii(struct phy *phy, enum phy_mode mode)
 {
 	struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
 	struct mvebu_comphy_priv *priv = lane->priv;
 	u32 val;
 
-	mvebu_comphy_ethernet_init_reset(lane, PHY_MODE_SGMII);
+	mvebu_comphy_ethernet_init_reset(lane, mode);
 
 	val = readl(priv->base + MVEBU_COMPHY_RX_CTRL1(lane->id));
 	val &= ~MVEBU_COMPHY_RX_CTRL1_CLK8T_EN;
@@ -487,7 +497,8 @@ static int mvebu_comphy_power_on(struct phy *phy)
 
 	switch (lane->mode) {
 	case PHY_MODE_SGMII:
-		ret = mvebu_comphy_set_mode_sgmii(phy);
+	case PHY_MODE_2500SGMII:
+		ret = mvebu_comphy_set_mode_sgmii(phy, lane->mode);
 		break;
 	case PHY_MODE_10GKR:
 		ret = mvebu_comphy_set_mode_10gkr(phy);
-- 
2.14.3

[PATCH net-next v2 3/4] net: mvpp2: 1000baseX support

[Antoine Tenart]

This patch adds the 1000Base-X PHY mode support in the Marvell PPv2
driver. 1000Base-X is quite close the SGMII and uses nearly the same
code path.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 45 ++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index a19760736b71..257a6b99b4ca 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -4501,6 +4501,7 @@ static int mvpp22_gop_init(struct mvpp2_port *port)
 		mvpp22_gop_init_rgmii(port);
 		break;
 	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_1000BASEX:
 		mvpp22_gop_init_sgmii(port);
 		break;
 	case PHY_INTERFACE_MODE_10GKR:
@@ -4538,7 +4539,8 @@ static void mvpp22_gop_unmask_irq(struct mvpp2_port *port)
 	u32 val;
 
 	if (phy_interface_mode_is_rgmii(port->phy_interface) ||
-	    port->phy_interface == PHY_INTERFACE_MODE_SGMII) {
+	    port->phy_interface == PHY_INTERFACE_MODE_SGMII ||
+	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX) {
 		/* Enable the GMAC link status irq for this port */
 		val = readl(port->base + MVPP22_GMAC_INT_SUM_MASK);
 		val |= MVPP22_GMAC_INT_SUM_MASK_LINK_STAT;
@@ -4568,7 +4570,8 @@ static void mvpp22_gop_mask_irq(struct mvpp2_port *port)
 	}
 
 	if (phy_interface_mode_is_rgmii(port->phy_interface) ||
-	    port->phy_interface == PHY_INTERFACE_MODE_SGMII) {
+	    port->phy_interface == PHY_INTERFACE_MODE_SGMII ||
+	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX) {
 		val = readl(port->base + MVPP22_GMAC_INT_SUM_MASK);
 		val &= ~MVPP22_GMAC_INT_SUM_MASK_LINK_STAT;
 		writel(val, port->base + MVPP22_GMAC_INT_SUM_MASK);
@@ -4580,7 +4583,8 @@ static void mvpp22_gop_setup_irq(struct mvpp2_port *port)
 	u32 val;
 
 	if (phy_interface_mode_is_rgmii(port->phy_interface) ||
-	    port->phy_interface == PHY_INTERFACE_MODE_SGMII) {
+	    port->phy_interface == PHY_INTERFACE_MODE_SGMII ||
+	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX) {
 		val = readl(port->base + MVPP22_GMAC_INT_MASK);
 		val |= MVPP22_GMAC_INT_MASK_LINK_STAT;
 		writel(val, port->base + MVPP22_GMAC_INT_MASK);
@@ -4605,6 +4609,7 @@ static int mvpp22_comphy_init(struct mvpp2_port *port)
 
 	switch (port->phy_interface) {
 	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_1000BASEX:
 		mode = PHY_MODE_SGMII;
 		break;
 	case PHY_INTERFACE_MODE_10GKR:
@@ -4625,7 +4630,8 @@ static void mvpp2_port_mii_gmac_configure_mode(struct mvpp2_port *port)
 {
 	u32 val;
 
-	if (port->phy_interface == PHY_INTERFACE_MODE_SGMII) {
+	if (port->phy_interface == PHY_INTERFACE_MODE_SGMII ||
+	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX) {
 		val = readl(port->base + MVPP22_GMAC_CTRL_4_REG);
 		val |= MVPP22_CTRL4_SYNC_BYPASS_DIS | MVPP22_CTRL4_DP_CLK_SEL |
 		       MVPP22_CTRL4_QSGMII_BYPASS_ACTIVE;
@@ -4640,9 +4646,11 @@ static void mvpp2_port_mii_gmac_configure_mode(struct mvpp2_port *port)
 		writel(val, port->base + MVPP22_GMAC_CTRL_4_REG);
 	}
 
-	/* The port is connected to a copper PHY */
 	val = readl(port->base + MVPP2_GMAC_CTRL_0_REG);
-	val &= ~MVPP2_GMAC_PORT_TYPE_MASK;
+	if (port->phy_interface == PHY_INTERFACE_MODE_1000BASEX)
+		val |= MVPP2_GMAC_PORT_TYPE_MASK;
+	else
+		val &= ~MVPP2_GMAC_PORT_TYPE_MASK;
 	writel(val, port->base + MVPP2_GMAC_CTRL_0_REG);
 
 	val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
@@ -4651,6 +4659,19 @@ static void mvpp2_port_mii_gmac_configure_mode(struct mvpp2_port *port)
 	       MVPP2_GMAC_AN_DUPLEX_EN;
 	if (port->phy_interface == PHY_INTERFACE_MODE_SGMII)
 		val |= MVPP2_GMAC_IN_BAND_AUTONEG;
+
+	if (port->phy_interface == PHY_INTERFACE_MODE_1000BASEX)
+		/* 1000BaseX port cannot negotiate speed nor can it
+		 * negotiate duplex: they are always operating with a
+		 * fixed speed of 1000Mbps in full duplex, so force
+		 * 1000 speed and full duplex here.
+		 */
+		val |= MVPP2_GMAC_CONFIG_GMII_SPEED |
+		       MVPP2_GMAC_CONFIG_FULL_DUPLEX;
+	else
+		val |= MVPP2_GMAC_AN_SPEED_EN |
+		       MVPP2_GMAC_AN_DUPLEX_EN;
+
 	writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
 }
 
@@ -4671,7 +4692,8 @@ static void mvpp2_port_mii_gmac_configure(struct mvpp2_port *port)
 
 	/* Configure the PCS and in-band AN */
 	val = readl(port->base + MVPP2_GMAC_CTRL_2_REG);
-	if (port->phy_interface == PHY_INTERFACE_MODE_SGMII) {
+	if (port->phy_interface == PHY_INTERFACE_MODE_SGMII ||
+	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX) {
 	        val |= MVPP2_GMAC_INBAND_AN_MASK | MVPP2_GMAC_PCS_ENABLE_MASK;
 	} else if (phy_interface_mode_is_rgmii(port->phy_interface)) {
 		val &= ~MVPP2_GMAC_PCS_ENABLE_MASK;
@@ -4733,7 +4755,8 @@ static void mvpp2_port_mii_set(struct mvpp2_port *port)
 		mvpp22_port_mii_set(port);
 
 	if (phy_interface_mode_is_rgmii(port->phy_interface) ||
-	    port->phy_interface == PHY_INTERFACE_MODE_SGMII)
+	    port->phy_interface == PHY_INTERFACE_MODE_SGMII ||
+	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX)
 		mvpp2_port_mii_gmac_configure(port);
 	else if (port->phy_interface == PHY_INTERFACE_MODE_10GKR)
 		mvpp2_port_mii_xlg_configure(port);
@@ -4810,7 +4833,8 @@ static void mvpp2_port_loopback_set(struct mvpp2_port *port)
 	else
 		val &= ~MVPP2_GMAC_GMII_LB_EN_MASK;
 
-	if (port->phy_interface == PHY_INTERFACE_MODE_SGMII)
+	if (port->phy_interface == PHY_INTERFACE_MODE_SGMII ||
+	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX)
 		val |= MVPP2_GMAC_PCS_LB_EN_MASK;
 	else
 		val &= ~MVPP2_GMAC_PCS_LB_EN_MASK;
@@ -6023,7 +6047,8 @@ static irqreturn_t mvpp2_link_status_isr(int irq, void *dev_id)
 				link = true;
 		}
 	} else if (phy_interface_mode_is_rgmii(port->phy_interface) ||
-		   port->phy_interface == PHY_INTERFACE_MODE_SGMII) {
+		   port->phy_interface == PHY_INTERFACE_MODE_SGMII ||
+		   port->phy_interface == PHY_INTERFACE_MODE_1000BASEX) {
 		val = readl(port->base + MVPP22_GMAC_INT_STAT);
 		if (val & MVPP22_GMAC_INT_STAT_LINK) {
 			event = true;
-- 
2.14.3

[PATCH net-next v2 4/4] net: mvpp2: 2500baseX support

[Antoine Tenart]

This patch adds the 2500Base-X PHY mode support in the Marvell PPv2
driver. 2500Base-X is quite close to 1000Base-X and SGMII modes and uses
nearly the same code path.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 40 ++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 257a6b99b4ca..d5e4bec98b2b 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -4502,6 +4502,7 @@ static int mvpp22_gop_init(struct mvpp2_port *port)
 		break;
 	case PHY_INTERFACE_MODE_SGMII:
 	case PHY_INTERFACE_MODE_1000BASEX:
+	case PHY_INTERFACE_MODE_2500BASEX:
 		mvpp22_gop_init_sgmii(port);
 		break;
 	case PHY_INTERFACE_MODE_10GKR:
@@ -4540,7 +4541,8 @@ static void mvpp22_gop_unmask_irq(struct mvpp2_port *port)
 
 	if (phy_interface_mode_is_rgmii(port->phy_interface) ||
 	    port->phy_interface == PHY_INTERFACE_MODE_SGMII ||
-	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX) {
+	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX ||
+	    port->phy_interface == PHY_INTERFACE_MODE_2500BASEX) {
 		/* Enable the GMAC link status irq for this port */
 		val = readl(port->base + MVPP22_GMAC_INT_SUM_MASK);
 		val |= MVPP22_GMAC_INT_SUM_MASK_LINK_STAT;
@@ -4571,7 +4573,8 @@ static void mvpp22_gop_mask_irq(struct mvpp2_port *port)
 
 	if (phy_interface_mode_is_rgmii(port->phy_interface) ||
 	    port->phy_interface == PHY_INTERFACE_MODE_SGMII ||
-	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX) {
+	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX ||
+	    port->phy_interface == PHY_INTERFACE_MODE_2500BASEX) {
 		val = readl(port->base + MVPP22_GMAC_INT_SUM_MASK);
 		val &= ~MVPP22_GMAC_INT_SUM_MASK_LINK_STAT;
 		writel(val, port->base + MVPP22_GMAC_INT_SUM_MASK);
@@ -4584,7 +4587,8 @@ static void mvpp22_gop_setup_irq(struct mvpp2_port *port)
 
 	if (phy_interface_mode_is_rgmii(port->phy_interface) ||
 	    port->phy_interface == PHY_INTERFACE_MODE_SGMII ||
-	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX) {
+	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX ||
+	    port->phy_interface == PHY_INTERFACE_MODE_2500BASEX) {
 		val = readl(port->base + MVPP22_GMAC_INT_MASK);
 		val |= MVPP22_GMAC_INT_MASK_LINK_STAT;
 		writel(val, port->base + MVPP22_GMAC_INT_MASK);
@@ -4612,6 +4616,9 @@ static int mvpp22_comphy_init(struct mvpp2_port *port)
 	case PHY_INTERFACE_MODE_1000BASEX:
 		mode = PHY_MODE_SGMII;
 		break;
+	case PHY_INTERFACE_MODE_2500BASEX:
+		mode = PHY_MODE_2500SGMII;
+		break;
 	case PHY_INTERFACE_MODE_10GKR:
 		mode = PHY_MODE_10GKR;
 		break;
@@ -4631,7 +4638,8 @@ static void mvpp2_port_mii_gmac_configure_mode(struct mvpp2_port *port)
 	u32 val;
 
 	if (port->phy_interface == PHY_INTERFACE_MODE_SGMII ||
-	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX) {
+	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX ||
+	    port->phy_interface == PHY_INTERFACE_MODE_2500BASEX) {
 		val = readl(port->base + MVPP22_GMAC_CTRL_4_REG);
 		val |= MVPP22_CTRL4_SYNC_BYPASS_DIS | MVPP22_CTRL4_DP_CLK_SEL |
 		       MVPP22_CTRL4_QSGMII_BYPASS_ACTIVE;
@@ -4647,7 +4655,8 @@ static void mvpp2_port_mii_gmac_configure_mode(struct mvpp2_port *port)
 	}
 
 	val = readl(port->base + MVPP2_GMAC_CTRL_0_REG);
-	if (port->phy_interface == PHY_INTERFACE_MODE_1000BASEX)
+	if (port->phy_interface == PHY_INTERFACE_MODE_1000BASEX ||
+	    port->phy_interface == PHY_INTERFACE_MODE_2500BASEX)
 		val |= MVPP2_GMAC_PORT_TYPE_MASK;
 	else
 		val &= ~MVPP2_GMAC_PORT_TYPE_MASK;
@@ -4660,6 +4669,11 @@ static void mvpp2_port_mii_gmac_configure_mode(struct mvpp2_port *port)
 	if (port->phy_interface == PHY_INTERFACE_MODE_SGMII)
 		val |= MVPP2_GMAC_IN_BAND_AUTONEG;
 
+	/* Clear all fields we may want to explicitly set below */
+	val &= ~(MVPP2_GMAC_CONFIG_FULL_DUPLEX | MVPP2_GMAC_CONFIG_GMII_SPEED |
+		 MVPP2_GMAC_CONFIG_MII_SPEED | MVPP2_GMAC_AN_SPEED_EN |
+		 MVPP2_GMAC_AN_DUPLEX_EN);
+
 	if (port->phy_interface == PHY_INTERFACE_MODE_1000BASEX)
 		/* 1000BaseX port cannot negotiate speed nor can it
 		 * negotiate duplex: they are always operating with a
@@ -4668,6 +4682,10 @@ static void mvpp2_port_mii_gmac_configure_mode(struct mvpp2_port *port)
 		 */
 		val |= MVPP2_GMAC_CONFIG_GMII_SPEED |
 		       MVPP2_GMAC_CONFIG_FULL_DUPLEX;
+	else if (port->phy_interface == PHY_INTERFACE_MODE_2500BASEX)
+		val |= MVPP2_GMAC_CONFIG_GMII_SPEED |
+		       MVPP2_GMAC_CONFIG_MII_SPEED |
+		       MVPP2_GMAC_CONFIG_FULL_DUPLEX;
 	else
 		val |= MVPP2_GMAC_AN_SPEED_EN |
 		       MVPP2_GMAC_AN_DUPLEX_EN;
@@ -4693,7 +4711,8 @@ static void mvpp2_port_mii_gmac_configure(struct mvpp2_port *port)
 	/* Configure the PCS and in-band AN */
 	val = readl(port->base + MVPP2_GMAC_CTRL_2_REG);
 	if (port->phy_interface == PHY_INTERFACE_MODE_SGMII ||
-	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX) {
+	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX ||
+	    port->phy_interface == PHY_INTERFACE_MODE_2500BASEX) {
 	        val |= MVPP2_GMAC_INBAND_AN_MASK | MVPP2_GMAC_PCS_ENABLE_MASK;
 	} else if (phy_interface_mode_is_rgmii(port->phy_interface)) {
 		val &= ~MVPP2_GMAC_PCS_ENABLE_MASK;
@@ -4756,7 +4775,8 @@ static void mvpp2_port_mii_set(struct mvpp2_port *port)
 
 	if (phy_interface_mode_is_rgmii(port->phy_interface) ||
 	    port->phy_interface == PHY_INTERFACE_MODE_SGMII ||
-	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX)
+	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX ||
+	    port->phy_interface == PHY_INTERFACE_MODE_2500BASEX)
 		mvpp2_port_mii_gmac_configure(port);
 	else if (port->phy_interface == PHY_INTERFACE_MODE_10GKR)
 		mvpp2_port_mii_xlg_configure(port);
@@ -4834,7 +4854,8 @@ static void mvpp2_port_loopback_set(struct mvpp2_port *port)
 		val &= ~MVPP2_GMAC_GMII_LB_EN_MASK;
 
 	if (port->phy_interface == PHY_INTERFACE_MODE_SGMII ||
-	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX)
+	    port->phy_interface == PHY_INTERFACE_MODE_1000BASEX ||
+	    port->phy_interface == PHY_INTERFACE_MODE_2500BASEX)
 		val |= MVPP2_GMAC_PCS_LB_EN_MASK;
 	else
 		val &= ~MVPP2_GMAC_PCS_LB_EN_MASK;
@@ -6048,7 +6069,8 @@ static irqreturn_t mvpp2_link_status_isr(int irq, void *dev_id)
 		}
 	} else if (phy_interface_mode_is_rgmii(port->phy_interface) ||
 		   port->phy_interface == PHY_INTERFACE_MODE_SGMII ||
-		   port->phy_interface == PHY_INTERFACE_MODE_1000BASEX) {
+		   port->phy_interface == PHY_INTERFACE_MODE_1000BASEX ||
+		   port->phy_interface == PHY_INTERFACE_MODE_2500BASEX) {
 		val = readl(port->base + MVPP22_GMAC_INT_STAT);
 		if (val & MVPP22_GMAC_INT_STAT_LINK) {
 			event = true;
-- 
2.14.3

Re: [PATCH net-next v2 4/4] net: mvpp2: 2500baseX support

[Andrew Lunn]

> @@ -4612,6 +4616,9 @@ static int mvpp22_comphy_init(struct mvpp2_port *port)
>  	case PHY_INTERFACE_MODE_1000BASEX:
>  		mode = PHY_MODE_SGMII;
>  		break;
> +	case PHY_INTERFACE_MODE_2500BASEX:
> +		mode = PHY_MODE_2500SGMII;
> +		break;

I think this is the source of confusion with linux/phy.h and
linux/phy/phy.h.

What would PHY_INTERFACE_MODE_2500SGMII use?

Where is this all getting confused? Should the caller to
mvpp22_comphy_init() actually be passing PHY_INTERFACE_MODE_2500SGMII?
What is the MAC actually doing at this point? 2500BASEX or 2500SGMII?

At minimum there needs to be a comment that this is not a typ0,
otherwise you are going to get patches submitted to 'fix' this.

	Thanks
		Andrew		     

Re: [PATCH net-next v2 4/4] net: mvpp2: 2500baseX support

[Antoine Tenart]

Hi Andrew,

On Wed, Jan 03, 2018 at 04:20:36PM +0100, Andrew Lunn wrote:
> > @@ -4612,6 +4616,9 @@ static int mvpp22_comphy_init(struct mvpp2_port *port)
> >  	case PHY_INTERFACE_MODE_1000BASEX:
> >  		mode = PHY_MODE_SGMII;
> >  		break;
> > +	case PHY_INTERFACE_MODE_2500BASEX:
> > +		mode = PHY_MODE_2500SGMII;
> > +		break;
> 
> I think this is the source of confusion with linux/phy.h and
> linux/phy/phy.h.
> 
> What would PHY_INTERFACE_MODE_2500SGMII use?
> 
> Where is this all getting confused? Should the caller to
> mvpp22_comphy_init() actually be passing PHY_INTERFACE_MODE_2500SGMII?
> What is the MAC actually doing at this point? 2500BASEX or 2500SGMII?

PHY_INTERFACE_MODE_2500BASEX is the PHY mode whereas PHY_MODE_2500SGMII
is the mode used by the common PHY driver (i.e. the one configuring the
serdes lanes).

There's no PHY_INTERFACE_MODE_2500SGMII mode.

> At minimum there needs to be a comment that this is not a typ0,
> otherwise you are going to get patches submitted to 'fix' this.

Sure, I can add a comment to state this function is a translation
between the net PHY mode and the generic PHY mode (it's a n-to-1
translation).

Thanks!
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

RE: [EXT] Re: [PATCH net-next v2 4/4] net: mvpp2: 2500baseX support

[Stefan Chulski]

> -----Original Message-----
> From: Antoine Tenart [mailto:antoine.tenart@free-electrons.com]
> Sent: Wednesday, January 03, 2018 5:32 PM
> To: Andrew Lunn <andrew@lunn.ch>
> Cc: Antoine Tenart <antoine.tenart@free-electrons.com>;
> davem@davemloft.net; kishon@ti.com; gregory.clement@free-electrons.com;
> linux@armlinux.org.uk; mw@semihalf.com; Stefan Chulski
> <stefanc@marvell.com>; Yan Markman <ymarkman@marvell.com>;
> thomas.petazzoni@free-electrons.com; miquel.raynal@free-electrons.com;
> Nadav Haklai <nadavh@marvell.com>; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: [EXT] Re: [PATCH net-next v2 4/4] net: mvpp2: 2500baseX support
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hi Andrew,
> 
> On Wed, Jan 03, 2018 at 04:20:36PM +0100, Andrew Lunn wrote:
> > > @@ -4612,6 +4616,9 @@ static int mvpp22_comphy_init(struct
> mvpp2_port *port)
> > >  	case PHY_INTERFACE_MODE_1000BASEX:
> > >  		mode = PHY_MODE_SGMII;
> > >  		break;
> > > +	case PHY_INTERFACE_MODE_2500BASEX:
> > > +		mode = PHY_MODE_2500SGMII;
> > > +		break;
> >
> > I think this is the source of confusion with linux/phy.h and
> > linux/phy/phy.h.
> >
> > What would PHY_INTERFACE_MODE_2500SGMII use?
> >
> > Where is this all getting confused? Should the caller to
> > mvpp22_comphy_init() actually be passing
> PHY_INTERFACE_MODE_2500SGMII?
> > What is the MAC actually doing at this point? 2500BASEX or 2500SGMII?
> 
> PHY_INTERFACE_MODE_2500BASEX is the PHY mode whereas
> PHY_MODE_2500SGMII is the mode used by the common PHY driver (i.e. the
> one configuring the serdes lanes).
> 
> There's no PHY_INTERFACE_MODE_2500SGMII mode.
> 
> > At minimum there needs to be a comment that this is not a typ0,
> > otherwise you are going to get patches submitted to 'fix' this.
> 
> Sure, I can add a comment to state this function is a translation between the
> net PHY mode and the generic PHY mode (it's a n-to-1 translation).
> 

Maybe we should rename enum phy_mode to comphy_mode and PHY_MODE_2500SGMII to COMPHY_MODE_2500SGMII.
Since this enum set MAC to PHY serdes communication mode, not PHY to PHY communication mode.

Stefan.

RE: [EXT] Re: [PATCH net-next v2 4/4] net: mvpp2: 2500baseX support

[Stefan Chulski]

> -----Original Message-----
> From: Antoine Tenart [mailto:antoine.tenart@free-electrons.com]
> Sent: Wednesday, January 03, 2018 5:32 PM
> To: Andrew Lunn <andrew@lunn.ch>
> Cc: Antoine Tenart <antoine.tenart@free-electrons.com>;
> davem@davemloft.net; kishon@ti.com; gregory.clement@free-electrons.com;
> linux@armlinux.org.uk; mw@semihalf.com; Stefan Chulski
> <stefanc@marvell.com>; Yan Markman <ymarkman@marvell.com>;
> thomas.petazzoni@free-electrons.com; miquel.raynal@free-electrons.com;
> Nadav Haklai <nadavh@marvell.com>; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: [EXT] Re: [PATCH net-next v2 4/4] net: mvpp2: 2500baseX support
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hi Andrew,
> 
> On Wed, Jan 03, 2018 at 04:20:36PM +0100, Andrew Lunn wrote:
> > > @@ -4612,6 +4616,9 @@ static int mvpp22_comphy_init(struct
> mvpp2_port *port)
> > >  	case PHY_INTERFACE_MODE_1000BASEX:
> > >  		mode = PHY_MODE_SGMII;
> > >  		break;
> > > +	case PHY_INTERFACE_MODE_2500BASEX:
> > > +		mode = PHY_MODE_2500SGMII;
> > > +		break;
> >
> > I think this is the source of confusion with linux/phy.h and
> > linux/phy/phy.h.
> >
> > What would PHY_INTERFACE_MODE_2500SGMII use?
> >
> > Where is this all getting confused? Should the caller to
> > mvpp22_comphy_init() actually be passing
> PHY_INTERFACE_MODE_2500SGMII?
> > What is the MAC actually doing at this point? 2500BASEX or 2500SGMII?
> 
> PHY_INTERFACE_MODE_2500BASEX is the PHY mode whereas
> PHY_MODE_2500SGMII is the mode used by the common PHY driver (i.e. the
> one configuring the serdes lanes).
> 
> There's no PHY_INTERFACE_MODE_2500SGMII mode.
> 
> > At minimum there needs to be a comment that this is not a typ0,
> > otherwise you are going to get patches submitted to 'fix' this.
> 
> Sure, I can add a comment to state this function is a translation between the
> net PHY mode and the generic PHY mode (it's a n-to-1 translation).
> 

Maybe we should rename enum phy_mode to comphy_mode and PHY_MODE_2500SGMII to COMPHY_MODE_2500SGMII.
Since this enum set MAC to PHY serdes communication mode, not PHY to PHY communication mode.

Stefan.

Re: [PATCH net-next v2 4/4] net: mvpp2: 2500baseX support

[Andrew Lunn]

On Wed, Jan 03, 2018 at 04:32:27PM +0100, Antoine Tenart wrote:
> Hi Andrew,
> 
> On Wed, Jan 03, 2018 at 04:20:36PM +0100, Andrew Lunn wrote:
> > > @@ -4612,6 +4616,9 @@ static int mvpp22_comphy_init(struct mvpp2_port *port)
> > >  	case PHY_INTERFACE_MODE_1000BASEX:
> > >  		mode = PHY_MODE_SGMII;
> > >  		break;
> > > +	case PHY_INTERFACE_MODE_2500BASEX:
> > > +		mode = PHY_MODE_2500SGMII;
> > > +		break;
> > 
> > I think this is the source of confusion with linux/phy.h and
> > linux/phy/phy.h.
> > 
> > What would PHY_INTERFACE_MODE_2500SGMII use?
> > 
> > Where is this all getting confused? Should the caller to
> > mvpp22_comphy_init() actually be passing PHY_INTERFACE_MODE_2500SGMII?
> > What is the MAC actually doing at this point? 2500BASEX or 2500SGMII?
> 
> PHY_INTERFACE_MODE_2500BASEX is the PHY mode whereas PHY_MODE_2500SGMII
> is the mode used by the common PHY driver (i.e. the one configuring the
> serdes lanes).

> There's no PHY_INTERFACE_MODE_2500SGMII mode.

Hi Antoine

At the moment there is no PHY_INTERFACE_MODE_2500SGMII. However,
there are some devices which can do 2.5G SGMII. So it will appear
sometime. This piece of code then looks even stranger.

> Sure, I can add a comment to state this function is a translation
> between the net PHY mode and the generic PHY mode (it's a n-to-1
> translation).

I think from an API design point of view, passing PHY_MODE_2500BASEX
to comphy makes more sense. That is what the MAC wants to do. How the
comphy achieves that should be internal to the comphy.

       Andrew

RE: [PATCH net-next v2 4/4] net: mvpp2: 2500baseX support

[Stefan Chulski]

> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Wednesday, January 03, 2018 5:53 PM
> To: Antoine Tenart <antoine.tenart@free-electrons.com>
> Cc: davem@davemloft.net; kishon@ti.com; gregory.clement@free-
> electrons.com; linux@armlinux.org.uk; mw@semihalf.com; Stefan Chulski
> <stefanc@marvell.com>; Yan Markman <ymarkman@marvell.com>;
> thomas.petazzoni@free-electrons.com; miquel.raynal@free-electrons.com;
> Nadav Haklai <nadavh@marvell.com>; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH net-next v2 4/4] net: mvpp2: 2500baseX support
> 
> On Wed, Jan 03, 2018 at 04:32:27PM +0100, Antoine Tenart wrote:
> > Hi Andrew,
> >
> > On Wed, Jan 03, 2018 at 04:20:36PM +0100, Andrew Lunn wrote:
> > > > @@ -4612,6 +4616,9 @@ static int mvpp22_comphy_init(struct
> mvpp2_port *port)
> > > >  	case PHY_INTERFACE_MODE_1000BASEX:
> > > >  		mode = PHY_MODE_SGMII;
> > > >  		break;
> > > > +	case PHY_INTERFACE_MODE_2500BASEX:
> > > > +		mode = PHY_MODE_2500SGMII;
> > > > +		break;
> > >
> > > I think this is the source of confusion with linux/phy.h and
> > > linux/phy/phy.h.
> > >
> > > What would PHY_INTERFACE_MODE_2500SGMII use?
> > >
> > > Where is this all getting confused? Should the caller to
> > > mvpp22_comphy_init() actually be passing
> PHY_INTERFACE_MODE_2500SGMII?
> > > What is the MAC actually doing at this point? 2500BASEX or 2500SGMII?
> >
> > PHY_INTERFACE_MODE_2500BASEX is the PHY mode whereas
> > PHY_MODE_2500SGMII is the mode used by the common PHY driver (i.e. the
> > one configuring the serdes lanes).
> 
> > There's no PHY_INTERFACE_MODE_2500SGMII mode.
> 
> Hi Antoine
> 
> At the moment there is no PHY_INTERFACE_MODE_2500SGMII. However,
> there are some devices which can do 2.5G SGMII. So it will appear sometime.
> This piece of code then looks even stranger.
> 
> > Sure, I can add a comment to state this function is a translation
> > between the net PHY mode and the generic PHY mode (it's a n-to-1
> > translation).
> 
> I think from an API design point of view, passing PHY_MODE_2500BASEX to
> comphy makes more sense. That is what the MAC wants to do. How the
> comphy achieves that should be internal to the comphy.
> 
>        Andrew

COMPHY driver configure SERDES lanes to high speed SGMII(2500SGMII) mode and doesn't
care about mode used by physical layer(2500BASEX in this case).
There are possibility that MAC SERDES lane is routed on a backplane to another SERDES. 
I think we should use SGMII naming.

Stefan.

RE: [PATCH net-next v2 4/4] net: mvpp2: 2500baseX support

[Stefan Chulski]

> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Wednesday, January 03, 2018 5:53 PM
> To: Antoine Tenart <antoine.tenart@free-electrons.com>
> Cc: davem@davemloft.net; kishon@ti.com; gregory.clement@free-
> electrons.com; linux@armlinux.org.uk; mw@semihalf.com; Stefan Chulski
> <stefanc@marvell.com>; Yan Markman <ymarkman@marvell.com>;
> thomas.petazzoni@free-electrons.com; miquel.raynal@free-electrons.com;
> Nadav Haklai <nadavh@marvell.com>; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH net-next v2 4/4] net: mvpp2: 2500baseX support
> 
> On Wed, Jan 03, 2018 at 04:32:27PM +0100, Antoine Tenart wrote:
> > Hi Andrew,
> >
> > On Wed, Jan 03, 2018 at 04:20:36PM +0100, Andrew Lunn wrote:
> > > > @@ -4612,6 +4616,9 @@ static int mvpp22_comphy_init(struct
> mvpp2_port *port)
> > > >  	case PHY_INTERFACE_MODE_1000BASEX:
> > > >  		mode = PHY_MODE_SGMII;
> > > >  		break;
> > > > +	case PHY_INTERFACE_MODE_2500BASEX:
> > > > +		mode = PHY_MODE_2500SGMII;
> > > > +		break;
> > >
> > > I think this is the source of confusion with linux/phy.h and
> > > linux/phy/phy.h.
> > >
> > > What would PHY_INTERFACE_MODE_2500SGMII use?
> > >
> > > Where is this all getting confused? Should the caller to
> > > mvpp22_comphy_init() actually be passing
> PHY_INTERFACE_MODE_2500SGMII?
> > > What is the MAC actually doing at this point? 2500BASEX or 2500SGMII?
> >
> > PHY_INTERFACE_MODE_2500BASEX is the PHY mode whereas
> > PHY_MODE_2500SGMII is the mode used by the common PHY driver (i.e. the
> > one configuring the serdes lanes).
> 
> > There's no PHY_INTERFACE_MODE_2500SGMII mode.
> 
> Hi Antoine
> 
> At the moment there is no PHY_INTERFACE_MODE_2500SGMII. However,
> there are some devices which can do 2.5G SGMII. So it will appear sometime.
> This piece of code then looks even stranger.
> 
> > Sure, I can add a comment to state this function is a translation
> > between the net PHY mode and the generic PHY mode (it's a n-to-1
> > translation).
> 
> I think from an API design point of view, passing PHY_MODE_2500BASEX to
> comphy makes more sense. That is what the MAC wants to do. How the
> comphy achieves that should be internal to the comphy.
> 
>        Andrew

COMPHY driver configure SERDES lanes to high speed SGMII(2500SGMII) mode and doesn't
care about mode used by physical layer(2500BASEX in this case).
There are possibility that MAC SERDES lane is routed on a backplane to another SERDES. 
I think we should use SGMII naming.

Stefan.

Re: [PATCH net-next v2 4/4] net: mvpp2: 2500baseX support

[Russell King - ARM Linux]

On Wed, Jan 03, 2018 at 04:20:36PM +0100, Andrew Lunn wrote:
> > @@ -4612,6 +4616,9 @@ static int mvpp22_comphy_init(struct mvpp2_port *port)
> >  	case PHY_INTERFACE_MODE_1000BASEX:
> >  		mode = PHY_MODE_SGMII;
> >  		break;
> > +	case PHY_INTERFACE_MODE_2500BASEX:
> > +		mode = PHY_MODE_2500SGMII;
> > +		break;
> 
> I think this is the source of confusion with linux/phy.h and
> linux/phy/phy.h.
> 
> What would PHY_INTERFACE_MODE_2500SGMII use?
> 
> Where is this all getting confused? Should the caller to
> mvpp22_comphy_init() actually be passing PHY_INTERFACE_MODE_2500SGMII?
> What is the MAC actually doing at this point? 2500BASEX or 2500SGMII?
> 
> At minimum there needs to be a comment that this is not a typ0,
> otherwise you are going to get patches submitted to 'fix' this.

Andrew,

I think the confusion comes from Marvell using "SGMII" as a generic name
for the serial gigabit ethernet interface, which includes both Cisco
SGMII and 802.3z BASE-X.

Remember, the only difference between these two modes is the contents of
the 16-bit control word and how that 16-bit control word is handled -
there's differences at the PCS level, but that's a level up from the
Serdes USB/PCIe/Ethernet PHY.

So, PHY_MODE_SGMII gets used to describe both Cisco SGMII and 1000BASE-X
modes, and PHY_MODE_2500SGMII is used to describe 2500BASE-X, and
possibly a fixed-speed 2.5G SGMII (the 2.5G SGMII information I've found
say that the speed bits not unused.)

It's confusing, but I'd say that the use of the "SGMII" is generally
confusing throughout this space, and I'd suggest people use "Cisco SGMII"
when they mean Cisco SGMII configuration word format and 802.3z BASE-X
when they mean the ethernet standard version, leaving "SGMII" available
as meaning the serdes stream of either.  Not ideal, but it seems that's
the way everyone else uses the "SGMII" term. :(

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH net-next v2 4/4] net: mvpp2: 2500baseX support

[Antoine Tenart]

Hi Andrew,

On Wed, Jan 03, 2018 at 04:53:11PM +0100, Andrew Lunn wrote:
> On Wed, Jan 03, 2018 at 04:32:27PM +0100, Antoine Tenart wrote:
> > On Wed, Jan 03, 2018 at 04:20:36PM +0100, Andrew Lunn wrote:
> > > > @@ -4612,6 +4616,9 @@ static int mvpp22_comphy_init(struct mvpp2_port *port)
> > > >  	case PHY_INTERFACE_MODE_1000BASEX:
> > > >  		mode = PHY_MODE_SGMII;
> > > >  		break;
> > > > +	case PHY_INTERFACE_MODE_2500BASEX:
> > > > +		mode = PHY_MODE_2500SGMII;
> > > > +		break;
> > > 
> > > I think this is the source of confusion with linux/phy.h and
> > > linux/phy/phy.h.
> > > 
> > > What would PHY_INTERFACE_MODE_2500SGMII use?
> > > 
> > > Where is this all getting confused? Should the caller to
> > > mvpp22_comphy_init() actually be passing PHY_INTERFACE_MODE_2500SGMII?
> > > What is the MAC actually doing at this point? 2500BASEX or 2500SGMII?
> > 
> > PHY_INTERFACE_MODE_2500BASEX is the PHY mode whereas PHY_MODE_2500SGMII
> > is the mode used by the common PHY driver (i.e. the one configuring the
> > serdes lanes).
> 
> > There's no PHY_INTERFACE_MODE_2500SGMII mode.
> 
> At the moment there is no PHY_INTERFACE_MODE_2500SGMII. However,
> there are some devices which can do 2.5G SGMII. So it will appear
> sometime. This piece of code then looks even stranger.

True. As said by Stefan the Marvell common PHY configures the serdes
lanes to a given mode, regardless of the actual use of the lanes by the
physical layer. So these modes are valid:

PPv2 (SGMII) - COMPHY (SGMII)
PPv2 (1000BaseX) - COMPHY (SGMII)
PPv2 (2500BaseX) - COMPHY (2500SGMII)

> > Sure, I can add a comment to state this function is a translation
> > between the net PHY mode and the generic PHY mode (it's a n-to-1
> > translation).
> 
> I think from an API design point of view, passing PHY_MODE_2500BASEX
> to comphy makes more sense. That is what the MAC wants to do. How the
> comphy achieves that should be internal to the comphy.

I though about that. The reason I did not is I wanted to use the
existing generic PHY framework helpers. They take a generic PHY mode in
input. I also wanted to avoid having a custom callback between the PPv2
driver and the COMPHY driver as phy_set_mode() seems to perfectly fit
the need.

The issue really is there are two PHY frameworks, one for network
devices and one for the other ones. The COMPHY is used by network
controllers, but also by SATA or USB ones.

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com