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

[PATCH net-next 0/6] 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).

Thanks to theses patches the fourth network interface can be used on the
mcbin, and two patches are attached: one to describe the interface in
the mcbin device tree, and another one adding Ethernet aliases now that
the four interfaces are described.

This was tested on a mcbin.

Patches 1 and 2 should go through the PHY tree, patches 3 and 4 through
the net-next tree and patches 5 and 6 through the mvebu one.

Please note the two mvpp2 patches do not conflict with the ACPI series
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

Antoine Tenart (5):
  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
  arm64: dts: marvell: mcbin: enable the fourth network interface

Yan Markman (1):
  arm64: dts: marvell: add Ethernet aliases

 arch/arm64/boot/dts/marvell/armada-7040-db.dts    |  6 ++
 arch/arm64/boot/dts/marvell/armada-8040-db.dts    |  7 +++
 arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts | 15 +++++
 drivers/net/ethernet/marvell/mvpp2.c              | 67 +++++++++++++++++++----
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c      | 17 +++++-
 include/linux/phy/phy.h                           |  1 +
 6 files changed, 100 insertions(+), 13 deletions(-)

-- 
2.14.3

[PATCH net-next 0/6] 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).

Thanks to theses patches the fourth network interface can be used on the
mcbin, and two patches are attached: one to describe the interface in
the mcbin device tree, and another one adding Ethernet aliases now that
the four interfaces are described.

This was tested on a mcbin.

Patches 1 and 2 should go through the PHY tree, patches 3 and 4 through
the net-next tree and patches 5 and 6 through the mvebu one.

Please note the two mvpp2 patches do not conflict with the ACPI series
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

Antoine Tenart (5):
  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
  arm64: dts: marvell: mcbin: enable the fourth network interface

Yan Markman (1):
  arm64: dts: marvell: add Ethernet aliases

 arch/arm64/boot/dts/marvell/armada-7040-db.dts    |  6 ++
 arch/arm64/boot/dts/marvell/armada-8040-db.dts    |  7 +++
 arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts | 15 +++++
 drivers/net/ethernet/marvell/mvpp2.c              | 67 +++++++++++++++++++----
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c      | 17 +++++-
 include/linux/phy/phy.h                           |  1 +
 6 files changed, 100 insertions(+), 13 deletions(-)

-- 
2.14.3

[PATCH net-next 1/6] 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..70459a28f3a1 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_SGMII_2_5G,
 	PHY_MODE_10GKR,
 	PHY_MODE_UFS_HS_A,
 	PHY_MODE_UFS_HS_B,
-- 
2.14.3

[PATCH net-next 1/6] 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..70459a28f3a1 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_SGMII_2_5G,
 	PHY_MODE_10GKR,
 	PHY_MODE_UFS_HS_A,
 	PHY_MODE_UFS_HS_B,
-- 
2.14.3

[PATCH net-next 2/6] 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..946a6ed7b66f 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_SGMII_2_5G, 0x1),
 	/* lane 1 */
 	MVEBU_COMPHY_CONF(1, 2, PHY_MODE_SGMII, 0x1),
+	MVEBU_COMPHY_CONF(1, 2, PHY_MODE_SGMII_2_5G, 0x1),
 	/* lane 2 */
 	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_SGMII, 0x1),
+	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_SGMII_2_5G, 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_SGMII_2_5G, 0x2),
 	/* lane 4 */
 	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_SGMII, 0x2),
+	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_SGMII_2_5G, 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_SGMII_2_5G, 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_SGMII_2_5G)
+		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_SGMII_2_5G:
+		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 2/6] 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..946a6ed7b66f 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_SGMII_2_5G, 0x1),
 	/* lane 1 */
 	MVEBU_COMPHY_CONF(1, 2, PHY_MODE_SGMII, 0x1),
+	MVEBU_COMPHY_CONF(1, 2, PHY_MODE_SGMII_2_5G, 0x1),
 	/* lane 2 */
 	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_SGMII, 0x1),
+	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_SGMII_2_5G, 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_SGMII_2_5G, 0x2),
 	/* lane 4 */
 	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_SGMII, 0x2),
+	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_SGMII_2_5G, 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_SGMII_2_5G, 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_SGMII_2_5G)
+		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_SGMII_2_5G:
+		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 3/6] 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..094db9dd633f 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 3/6] 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..094db9dd633f 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 4/6] 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 094db9dd633f..5d2a6f5a52b6 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_SGMII_2_5G;
+		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

[PATCH net-next 4/6] 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 094db9dd633f..5d2a6f5a52b6 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_SGMII_2_5G;
+		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

[PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Antoine Tenart]

This patch enables the fourth network interface on the Marvell
Macchiatobin. It is configured in the 2500Base-X PHY mode.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
index b3350827ee55..c51efd511324 100644
--- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
+++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
@@ -279,6 +279,14 @@
 	phys = <&cps_comphy0 1>;
 };
 
+&cps_eth2 {
+	/* CPS Lane 5 */
+	status = "okay";
+	phy-mode = "2500base-x";
+	/* Generic PHY, providing serdes lanes */
+	phys = <&cps_comphy5 2>;
+};
+
 &cps_pinctrl {
 	cps_spi1_pins: spi1-pins {
 		marvell,pins = "mpp12", "mpp13", "mpp14", "mpp15", "mpp16";
-- 
2.14.3

[PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Antoine Tenart]

This patch enables the fourth network interface on the Marvell
Macchiatobin. It is configured in the 2500Base-X PHY mode.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
index b3350827ee55..c51efd511324 100644
--- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
+++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
@@ -279,6 +279,14 @@
 	phys = <&cps_comphy0 1>;
 };
 
+&cps_eth2 {
+	/* CPS Lane 5 */
+	status = "okay";
+	phy-mode = "2500base-x";
+	/* Generic PHY, providing serdes lanes */
+	phys = <&cps_comphy5 2>;
+};
+
 &cps_pinctrl {
 	cps_spi1_pins: spi1-pins {
 		marvell,pins = "mpp12", "mpp13", "mpp14", "mpp15", "mpp16";
-- 
2.14.3

[PATCH net-next 6/6] arm64: dts: marvell: add Ethernet aliases

[Antoine Tenart]

From: Yan Markman <ymarkman@marvell.com>

This patch adds Ethernet aliases in the Marvell Aramada 7040 DB, 8040 DB
and 8040 mcbin device trees so that the bootloader setup the MAC
addresses correctly.

Signed-off-by: Yan Markman <ymarkman@marvell.com>
[Antoine: commit message, small fixes]
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 arch/arm64/boot/dts/marvell/armada-7040-db.dts    | 6 ++++++
 arch/arm64/boot/dts/marvell/armada-8040-db.dts    | 7 +++++++
 arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts | 7 +++++++
 3 files changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-7040-db.dts b/arch/arm64/boot/dts/marvell/armada-7040-db.dts
index 52b5341cb270..62b83416b30c 100644
--- a/arch/arm64/boot/dts/marvell/armada-7040-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-7040-db.dts
@@ -61,6 +61,12 @@
 		reg = <0x0 0x0 0x0 0x80000000>;
 	};
 
+	aliases {
+		ethernet0 = &cpm_eth0;
+		ethernet1 = &cpm_eth1;
+		ethernet2 = &cpm_eth2;
+	};
+
 	cpm_reg_usb3_0_vbus: cpm-usb3-0-vbus {
 		compatible = "regulator-fixed";
 		regulator-name = "usb3h0-vbus";
diff --git a/arch/arm64/boot/dts/marvell/armada-8040-db.dts b/arch/arm64/boot/dts/marvell/armada-8040-db.dts
index d97b72bed662..d9fffde64c44 100644
--- a/arch/arm64/boot/dts/marvell/armada-8040-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-8040-db.dts
@@ -61,6 +61,13 @@
 		reg = <0x0 0x0 0x0 0x80000000>;
 	};
 
+	aliases {
+		ethernet0 = &cpm_eth0;
+		ethernet1 = &cpm_eth2;
+		ethernet2 = &cps_eth0;
+		ethernet3 = &cps_eth1;
+	};
+
 	cpm_reg_usb3_0_vbus: cpm-usb3-0-vbus {
 		compatible = "regulator-fixed";
 		regulator-name = "cpm-usb3h0-vbus";
diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
index c51efd511324..7a23bb279e1c 100644
--- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
+++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
@@ -62,6 +62,13 @@
 		reg = <0x0 0x0 0x0 0x80000000>;
 	};
 
+	aliases {
+		ethernet0 = &cpm_eth0;
+		ethernet1 = &cps_eth0;
+		ethernet2 = &cps_eth1;
+		ethernet3 = &cps_eth2;
+	};
+
 	/* Regulator labels correspond with schematics */
 	v_3_3: regulator-3-3v {
 		compatible = "regulator-fixed";
-- 
2.14.3

[PATCH net-next 6/6] arm64: dts: marvell: add Ethernet aliases

[Antoine Tenart]

From: Yan Markman <ymarkman@marvell.com>

This patch adds Ethernet aliases in the Marvell Aramada 7040 DB, 8040 DB
and 8040 mcbin device trees so that the bootloader setup the MAC
addresses correctly.

Signed-off-by: Yan Markman <ymarkman@marvell.com>
[Antoine: commit message, small fixes]
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 arch/arm64/boot/dts/marvell/armada-7040-db.dts    | 6 ++++++
 arch/arm64/boot/dts/marvell/armada-8040-db.dts    | 7 +++++++
 arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts | 7 +++++++
 3 files changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-7040-db.dts b/arch/arm64/boot/dts/marvell/armada-7040-db.dts
index 52b5341cb270..62b83416b30c 100644
--- a/arch/arm64/boot/dts/marvell/armada-7040-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-7040-db.dts
@@ -61,6 +61,12 @@
 		reg = <0x0 0x0 0x0 0x80000000>;
 	};
 
+	aliases {
+		ethernet0 = &cpm_eth0;
+		ethernet1 = &cpm_eth1;
+		ethernet2 = &cpm_eth2;
+	};
+
 	cpm_reg_usb3_0_vbus: cpm-usb3-0-vbus {
 		compatible = "regulator-fixed";
 		regulator-name = "usb3h0-vbus";
diff --git a/arch/arm64/boot/dts/marvell/armada-8040-db.dts b/arch/arm64/boot/dts/marvell/armada-8040-db.dts
index d97b72bed662..d9fffde64c44 100644
--- a/arch/arm64/boot/dts/marvell/armada-8040-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-8040-db.dts
@@ -61,6 +61,13 @@
 		reg = <0x0 0x0 0x0 0x80000000>;
 	};
 
+	aliases {
+		ethernet0 = &cpm_eth0;
+		ethernet1 = &cpm_eth2;
+		ethernet2 = &cps_eth0;
+		ethernet3 = &cps_eth1;
+	};
+
 	cpm_reg_usb3_0_vbus: cpm-usb3-0-vbus {
 		compatible = "regulator-fixed";
 		regulator-name = "cpm-usb3h0-vbus";
diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
index c51efd511324..7a23bb279e1c 100644
--- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
+++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
@@ -62,6 +62,13 @@
 		reg = <0x0 0x0 0x0 0x80000000>;
 	};
 
+	aliases {
+		ethernet0 = &cpm_eth0;
+		ethernet1 = &cps_eth0;
+		ethernet2 = &cps_eth1;
+		ethernet3 = &cps_eth2;
+	};
+
 	/* Regulator labels correspond with schematics */
 	v_3_3: regulator-3-3v {
 		compatible = "regulator-fixed";
-- 
2.14.3

Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Russell King - ARM Linux]

On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
> This patch enables the fourth network interface on the Marvell
> Macchiatobin. It is configured in the 2500Base-X PHY mode.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> ---
>  arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
> index b3350827ee55..c51efd511324 100644
> --- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
> @@ -279,6 +279,14 @@
>  	phys = <&cps_comphy0 1>;
>  };
>  
> +&cps_eth2 {
> +	/* CPS Lane 5 */
> +	status = "okay";
> +	phy-mode = "2500base-x";
> +	/* Generic PHY, providing serdes lanes */
> +	phys = <&cps_comphy5 2>;
> +};
> +

This is wrong.  This lane is connected to a SFP cage which can support
more than just 2500base-X.  Tying it in this way to 2500base-X means
that this port does not support conenctions at 1000base-X, despite
that's one of the most popular and more standardised speeds.

So, I feel I have to NAK this patch in its current form.

-- 
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

[PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Russell King - ARM Linux]

On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
> This patch enables the fourth network interface on the Marvell
> Macchiatobin. It is configured in the 2500Base-X PHY mode.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> ---
>  arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
> index b3350827ee55..c51efd511324 100644
> --- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
> @@ -279,6 +279,14 @@
>  	phys = <&cps_comphy0 1>;
>  };
>  
> +&cps_eth2 {
> +	/* CPS Lane 5 */
> +	status = "okay";
> +	phy-mode = "2500base-x";
> +	/* Generic PHY, providing serdes lanes */
> +	phys = <&cps_comphy5 2>;
> +};
> +

This is wrong.  This lane is connected to a SFP cage which can support
more than just 2500base-X.  Tying it in this way to 2500base-X means
that this port does not support conenctions at 1000base-X, despite
that's one of the most popular and more standardised speeds.

So, I feel I have to NAK this patch in its current form.

-- 
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 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Antoine Tenart]

Hi Russell,

On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote:
> On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
> >  
> > +&cps_eth2 {
> > +	/* CPS Lane 5 */
> > +	status = "okay";
> > +	phy-mode = "2500base-x";
> > +	/* Generic PHY, providing serdes lanes */
> > +	phys = <&cps_comphy5 2>;
> > +};
> > +
> 
> This is wrong.  This lane is connected to a SFP cage which can support
> more than just 2500base-X.  Tying it in this way to 2500base-X means
> that this port does not support conenctions at 1000base-X, despite
> that's one of the most popular and more standardised speeds.

What do you suggest to describe this in the dt, to enable a port using
the current PPv2 driver?

Antoine

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

[PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Antoine Tenart]

Hi Russell,

On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote:
> On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
> >  
> > +&cps_eth2 {
> > +	/* CPS Lane 5 */
> > +	status = "okay";
> > +	phy-mode = "2500base-x";
> > +	/* Generic PHY, providing serdes lanes */
> > +	phys = <&cps_comphy5 2>;
> > +};
> > +
> 
> This is wrong.  This lane is connected to a SFP cage which can support
> more than just 2500base-X.  Tying it in this way to 2500base-X means
> that this port does not support conenctions at 1000base-X, despite
> that's one of the most popular and more standardised speeds.

What do you suggest to describe this in the dt, to enable a port using
the current PPv2 driver?

Antoine

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

Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Russell King - ARM Linux]

On Wed, Dec 27, 2017 at 11:42:52PM +0100, Antoine Tenart wrote:
> Hi Russell,
> 
> On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote:
> > On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
> > >  
> > > +&cps_eth2 {
> > > +	/* CPS Lane 5 */
> > > +	status = "okay";
> > > +	phy-mode = "2500base-x";
> > > +	/* Generic PHY, providing serdes lanes */
> > > +	phys = <&cps_comphy5 2>;
> > > +};
> > > +
> > 
> > This is wrong.  This lane is connected to a SFP cage which can support
> > more than just 2500base-X.  Tying it in this way to 2500base-X means
> > that this port does not support conenctions at 1000base-X, despite
> > that's one of the most popular and more standardised speeds.
> 
> What do you suggest to describe this in the dt, to enable a port using
> the current PPv2 driver?

I don't - I'm merely pointing out that you're bodging support for the
SFP cage rather than productively discussing phylink for mvpp2.

As far as I remember, the discussion stalled at this point:

- You think there's modes that mvpp2 supports that are not supportable
  if you use phylink.

- I've described what phylink supports, and I've asked you for details
  about what you can't support.

Unfortunately, no details have been forthcoming, and no further
discussion has occurred - the ball is entirely in your court to
progress this issue since I requested information from you and that
is where things seem to have stalled.

The result is that, with your patch, you're locking the port to 2.5G
speeds, meaning that only 4.3Mbps Fibrechannel SFPs can be used with
the port, and it can only be used with another device that supports
2.5G speeds.  You can't use a copper RJ45 module, and you can't use
a standard 1000base-X module either in this configuration.

What I'm most concerned about, given the bindings for comphy that
have been merged, is that Free Electrons is pushing forward seemingly
with no regard to the requirement that the serdes lanes are dynamically
reconfigurable, and that's a basic requirement for SFP, and for the
88x3310 PHYs on the Macchiatobin platform.

So, my question to you is: what is Free Electrons plans to properly
support the ethernet ports on the Macchiatobin platform?


For those on the Cc list who don't know, phylink is part of full support
for SFP and SFP+ cages, sponsored (in terms of hardware including SFP
modules) by SolidRun on both SolidRun's Clearfog and Macchiatobin
platforms, supporting a wide range of SFP modules including:

- 1G Optical ethernet modules (duplex and bidi modules)
- 10/100/1G RJ45 modules
- 10G SFP+ modules
- 2.5Gbase-X using 4.3Mbps Fibrechannel modules
- Direct attach cables

There is work ongoing between Florian, Andrew and myself to switch DSA
to phylink, and have SFP modules working with both Marvell and Broadcom
DSA switches.

Phylink and SFP was already merged into mainline, and has been usable
(provided that the network driver is converted to phylink rather than
phylib) since 4.14-rc1.

-- 
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

[PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Russell King - ARM Linux]

On Wed, Dec 27, 2017 at 11:42:52PM +0100, Antoine Tenart wrote:
> Hi Russell,
> 
> On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote:
> > On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
> > >  
> > > +&cps_eth2 {
> > > +	/* CPS Lane 5 */
> > > +	status = "okay";
> > > +	phy-mode = "2500base-x";
> > > +	/* Generic PHY, providing serdes lanes */
> > > +	phys = <&cps_comphy5 2>;
> > > +};
> > > +
> > 
> > This is wrong.  This lane is connected to a SFP cage which can support
> > more than just 2500base-X.  Tying it in this way to 2500base-X means
> > that this port does not support conenctions at 1000base-X, despite
> > that's one of the most popular and more standardised speeds.
> 
> What do you suggest to describe this in the dt, to enable a port using
> the current PPv2 driver?

I don't - I'm merely pointing out that you're bodging support for the
SFP cage rather than productively discussing phylink for mvpp2.

As far as I remember, the discussion stalled at this point:

- You think there's modes that mvpp2 supports that are not supportable
  if you use phylink.

- I've described what phylink supports, and I've asked you for details
  about what you can't support.

Unfortunately, no details have been forthcoming, and no further
discussion has occurred - the ball is entirely in your court to
progress this issue since I requested information from you and that
is where things seem to have stalled.

The result is that, with your patch, you're locking the port to 2.5G
speeds, meaning that only 4.3Mbps Fibrechannel SFPs can be used with
the port, and it can only be used with another device that supports
2.5G speeds.  You can't use a copper RJ45 module, and you can't use
a standard 1000base-X module either in this configuration.

What I'm most concerned about, given the bindings for comphy that
have been merged, is that Free Electrons is pushing forward seemingly
with no regard to the requirement that the serdes lanes are dynamically
reconfigurable, and that's a basic requirement for SFP, and for the
88x3310 PHYs on the Macchiatobin platform.

So, my question to you is: what is Free Electrons plans to properly
support the ethernet ports on the Macchiatobin platform?


For those on the Cc list who don't know, phylink is part of full support
for SFP and SFP+ cages, sponsored (in terms of hardware including SFP
modules) by SolidRun on both SolidRun's Clearfog and Macchiatobin
platforms, supporting a wide range of SFP modules including:

- 1G Optical ethernet modules (duplex and bidi modules)
- 10/100/1G RJ45 modules
- 10G SFP+ modules
- 2.5Gbase-X using 4.3Mbps Fibrechannel modules
- Direct attach cables

There is work ongoing between Florian, Andrew and myself to switch DSA
to phylink, and have SFP modules working with both Marvell and Broadcom
DSA switches.

Phylink and SFP was already merged into mainline, and has been usable
(provided that the network driver is converted to phylink rather than
phylib) since 4.14-rc1.

-- 
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 1/6] phy: add 2.5G SGMII mode to the phy_mode enum

[Andrew Lunn]

On Wed, Dec 27, 2017 at 11:14:41PM +0100, Antoine Tenart wrote:
> 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..70459a28f3a1 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_SGMII_2_5G,
>  	PHY_MODE_10GKR,
>  	PHY_MODE_UFS_HS_A,
>  	PHY_MODE_UFS_HS_B,

Hi Antoine

There was a discussion maybe last month about adding 2.5G SGMII. I
would prefer 2500SGMII. Putting the number first makes it uniform with
the other defines, 1000BASEX, 25000BASEX, 10GKR.

    Andrew

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

[Andrew Lunn]

On Wed, Dec 27, 2017 at 11:14:41PM +0100, Antoine Tenart wrote:
> 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..70459a28f3a1 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_SGMII_2_5G,
>  	PHY_MODE_10GKR,
>  	PHY_MODE_UFS_HS_A,
>  	PHY_MODE_UFS_HS_B,

Hi Antoine

There was a discussion maybe last month about adding 2.5G SGMII. I
would prefer 2500SGMII. Putting the number first makes it uniform with
the other defines, 1000BASEX, 25000BASEX, 10GKR.

    Andrew

Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Andrew Lunn]

On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote:
> On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
> > This patch enables the fourth network interface on the Marvell
> > Macchiatobin. It is configured in the 2500Base-X PHY mode.
> > 
> > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> > ---
> >  arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
> > index b3350827ee55..c51efd511324 100644
> > --- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
> > +++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
> > @@ -279,6 +279,14 @@
> >  	phys = <&cps_comphy0 1>;
> >  };
> >  
> > +&cps_eth2 {
> > +	/* CPS Lane 5 */
> > +	status = "okay";
> > +	phy-mode = "2500base-x";
> > +	/* Generic PHY, providing serdes lanes */
> > +	phys = <&cps_comphy5 2>;
> > +};
> > +
> 
> This is wrong.  This lane is connected to a SFP cage which can support
> more than just 2500base-X.  Tying it in this way to 2500base-X means
> that this port does not support conenctions at 1000base-X, despite
> that's one of the most popular and more standardised speeds.
> 

Hi Antoine

I agree with Russell here. SFP modules are hot pluggable, and support
a range of interface modes. You need to query what the SFP module is
in order to know how to configure the SERDES interface. The phylink
infrastructure does that for you.

   Andrew

Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Andrew Lunn]

On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote:
> On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
> > This patch enables the fourth network interface on the Marvell
> > Macchiatobin. It is configured in the 2500Base-X PHY mode.
> > 
> > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> > ---
> >  arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
> > index b3350827ee55..c51efd511324 100644
> > --- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
> > +++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
> > @@ -279,6 +279,14 @@
> >  	phys = <&cps_comphy0 1>;
> >  };
> >  
> > +&cps_eth2 {
> > +	/* CPS Lane 5 */
> > +	status = "okay";
> > +	phy-mode = "2500base-x";
> > +	/* Generic PHY, providing serdes lanes */
> > +	phys = <&cps_comphy5 2>;
> > +};
> > +
> 
> This is wrong.  This lane is connected to a SFP cage which can support
> more than just 2500base-X.  Tying it in this way to 2500base-X means
> that this port does not support conenctions at 1000base-X, despite
> that's one of the most popular and more standardised speeds.
> 

Hi Antoine

I agree with Russell here. SFP modules are hot pluggable, and support
a range of interface modes. You need to query what the SFP module is
in order to know how to configure the SERDES interface. The phylink
infrastructure does that for you.

   Andrew

[PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Andrew Lunn]

On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote:
> On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
> > This patch enables the fourth network interface on the Marvell
> > Macchiatobin. It is configured in the 2500Base-X PHY mode.
> > 
> > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> > ---
> >  arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
> > index b3350827ee55..c51efd511324 100644
> > --- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
> > +++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
> > @@ -279,6 +279,14 @@
> >  	phys = <&cps_comphy0 1>;
> >  };
> >  
> > +&cps_eth2 {
> > +	/* CPS Lane 5 */
> > +	status = "okay";
> > +	phy-mode = "2500base-x";
> > +	/* Generic PHY, providing serdes lanes */
> > +	phys = <&cps_comphy5 2>;
> > +};
> > +
> 
> This is wrong.  This lane is connected to a SFP cage which can support
> more than just 2500base-X.  Tying it in this way to 2500base-X means
> that this port does not support conenctions at 1000base-X, despite
> that's one of the most popular and more standardised speeds.
> 

Hi Antoine

I agree with Russell here. SFP modules are hot pluggable, and support
a range of interface modes. You need to query what the SFP module is
in order to know how to configure the SERDES interface. The phylink
infrastructure does that for you.

   Andrew

Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Antoine Tenart]

Hi Russell,

On Wed, Dec 27, 2017 at 11:20:00PM +0000, Russell King - ARM Linux wrote:
> On Wed, Dec 27, 2017 at 11:42:52PM +0100, Antoine Tenart wrote:
> > 
> > What do you suggest to describe this in the dt, to enable a port using
> > the current PPv2 driver?
> 
> I don't - I'm merely pointing out that you're bodging support for the
> SFP cage rather than productively discussing phylink for mvpp2.
> 
> As far as I remember, the discussion stalled at this point:
> 
> - You think there's modes that mvpp2 supports that are not supportable
>   if you use phylink.
> 
> - I've described what phylink supports, and I've asked you for details
>   about what you can't support.

That's not what I remembered. You had some valid points, and others
related to PHY modes the driver wasn't supporting before the phylink
transition. My understanding of this was that you wanted a full
featured support while I only wanted to convert the already supported
modes.

> Unfortunately, no details have been forthcoming, and no further
> discussion has occurred - the ball is entirely in your court to
> progress this issue since I requested information from you and that
> is where things seem to have stalled.
> 
> The result is that, with your patch, you're locking the port to 2.5G
> speeds, meaning that only 4.3Mbps Fibrechannel SFPs can be used with
> the port, and it can only be used with another device that supports
> 2.5G speeds.  You can't use a copper RJ45 module, and you can't use
> a standard 1000base-X module either in this configuration.

You're probably right about not wanting this dt patch. The non-dt
patches still are relevant regardless of phylink being supported in the
PPv2 driver. I'll send a v2 without the dt parts.

Regarding the phylink patch it's stalled for now as I have other
priorities, but I do agree it's a topic that needs to be worked on for a
proper support. I initially made a patch to be nice as it was mentioned
on a previous series, but given the feedback I got I decided to delay it
until my other tasks were completed.

So let's delay the fourth interface support on the mcbin for now.

> What I'm most concerned about, given the bindings for comphy that
> have been merged, is that Free Electrons is pushing forward seemingly
> with no regard to the requirement that the serdes lanes are dynamically
> reconfigurable, and that's a basic requirement for SFP, and for the
> 88x3310 PHYs on the Macchiatobin platform.

The main idea behind the comphy driver is to provide a way to
reconfigure the serdes lanes at runtime. Could you develop what are
blocking points to properly support SFP, regarding the current comphy
support?

Thanks,
Antoine

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

[PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Antoine Tenart]

Hi Russell,

On Wed, Dec 27, 2017 at 11:20:00PM +0000, Russell King - ARM Linux wrote:
> On Wed, Dec 27, 2017 at 11:42:52PM +0100, Antoine Tenart wrote:
> > 
> > What do you suggest to describe this in the dt, to enable a port using
> > the current PPv2 driver?
> 
> I don't - I'm merely pointing out that you're bodging support for the
> SFP cage rather than productively discussing phylink for mvpp2.
> 
> As far as I remember, the discussion stalled at this point:
> 
> - You think there's modes that mvpp2 supports that are not supportable
>   if you use phylink.
> 
> - I've described what phylink supports, and I've asked you for details
>   about what you can't support.

That's not what I remembered. You had some valid points, and others
related to PHY modes the driver wasn't supporting before the phylink
transition. My understanding of this was that you wanted a full
featured support while I only wanted to convert the already supported
modes.

> Unfortunately, no details have been forthcoming, and no further
> discussion has occurred - the ball is entirely in your court to
> progress this issue since I requested information from you and that
> is where things seem to have stalled.
> 
> The result is that, with your patch, you're locking the port to 2.5G
> speeds, meaning that only 4.3Mbps Fibrechannel SFPs can be used with
> the port, and it can only be used with another device that supports
> 2.5G speeds.  You can't use a copper RJ45 module, and you can't use
> a standard 1000base-X module either in this configuration.

You're probably right about not wanting this dt patch. The non-dt
patches still are relevant regardless of phylink being supported in the
PPv2 driver. I'll send a v2 without the dt parts.

Regarding the phylink patch it's stalled for now as I have other
priorities, but I do agree it's a topic that needs to be worked on for a
proper support. I initially made a patch to be nice as it was mentioned
on a previous series, but given the feedback I got I decided to delay it
until my other tasks were completed.

So let's delay the fourth interface support on the mcbin for now.

> What I'm most concerned about, given the bindings for comphy that
> have been merged, is that Free Electrons is pushing forward seemingly
> with no regard to the requirement that the serdes lanes are dynamically
> reconfigurable, and that's a basic requirement for SFP, and for the
> 88x3310 PHYs on the Macchiatobin platform.

The main idea behind the comphy driver is to provide a way to
reconfigure the serdes lanes at runtime. Could you develop what are
blocking points to properly support SFP, regarding the current comphy
support?

Thanks,
Antoine

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

Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Antoine Tenart]

Hi Andrew,

On Thu, Dec 28, 2017 at 08:46:23AM +0100, Andrew Lunn wrote:
> On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote:
> > On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
> > >  
> > > +&cps_eth2 {
> > > +	/* CPS Lane 5 */
> > > +	status = "okay";
> > > +	phy-mode = "2500base-x";
> > > +	/* Generic PHY, providing serdes lanes */
> > > +	phys = <&cps_comphy5 2>;
> > > +};
> > > +
> > 
> > This is wrong.  This lane is connected to a SFP cage which can support
> > more than just 2500base-X.  Tying it in this way to 2500base-X means
> > that this port does not support conenctions at 1000base-X, despite
> > that's one of the most popular and more standardised speeds.
> > 
> 
> I agree with Russell here. SFP modules are hot pluggable, and support
> a range of interface modes. You need to query what the SFP module is
> in order to know how to configure the SERDES interface. The phylink
> infrastructure does that for you.

Sure, I understand. We'll be able to support such interfaces only when
the phylink PPv2 support lands in.

Thanks!
Antoine

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

[PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Antoine Tenart]

Hi Andrew,

On Thu, Dec 28, 2017 at 08:46:23AM +0100, Andrew Lunn wrote:
> On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote:
> > On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
> > >  
> > > +&cps_eth2 {
> > > +	/* CPS Lane 5 */
> > > +	status = "okay";
> > > +	phy-mode = "2500base-x";
> > > +	/* Generic PHY, providing serdes lanes */
> > > +	phys = <&cps_comphy5 2>;
> > > +};
> > > +
> > 
> > This is wrong.  This lane is connected to a SFP cage which can support
> > more than just 2500base-X.  Tying it in this way to 2500base-X means
> > that this port does not support conenctions at 1000base-X, despite
> > that's one of the most popular and more standardised speeds.
> > 
> 
> I agree with Russell here. SFP modules are hot pluggable, and support
> a range of interface modes. You need to query what the SFP module is
> in order to know how to configure the SERDES interface. The phylink
> infrastructure does that for you.

Sure, I understand. We'll be able to support such interfaces only when
the phylink PPv2 support lands in.

Thanks!
Antoine

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

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

[Antoine Tenart]

Hi Andrew,

On Thu, Dec 28, 2017 at 08:20:53AM +0100, Andrew Lunn wrote:
> On Wed, Dec 27, 2017 at 11:14:41PM +0100, Antoine Tenart wrote:
> > 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..70459a28f3a1 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_SGMII_2_5G,
> >  	PHY_MODE_10GKR,
> >  	PHY_MODE_UFS_HS_A,
> >  	PHY_MODE_UFS_HS_B,
> 
> There was a discussion maybe last month about adding 2.5G SGMII. I
> would prefer 2500SGMII. Putting the number first makes it uniform with
> the other defines, 1000BASEX, 25000BASEX, 10GKR.

Good to know. I wasn't completely sure how to name this mode properly,
but I'm fine with PHY_MODE_2500SGMII. I'll update the patches and send a
v2 (without the dt part).

Thanks!
Antoine

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

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

[Antoine Tenart]

Hi Andrew,

On Thu, Dec 28, 2017 at 08:20:53AM +0100, Andrew Lunn wrote:
> On Wed, Dec 27, 2017 at 11:14:41PM +0100, Antoine Tenart wrote:
> > 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..70459a28f3a1 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_SGMII_2_5G,
> >  	PHY_MODE_10GKR,
> >  	PHY_MODE_UFS_HS_A,
> >  	PHY_MODE_UFS_HS_B,
> 
> There was a discussion maybe last month about adding 2.5G SGMII. I
> would prefer 2500SGMII. Putting the number first makes it uniform with
> the other defines, 1000BASEX, 25000BASEX, 10GKR.

Good to know. I wasn't completely sure how to name this mode properly,
but I'm fine with PHY_MODE_2500SGMII. I'll update the patches and send a
v2 (without the dt part).

Thanks!
Antoine

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

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

[Florian Fainelli]

On 12/28/2017 02:06 AM, Antoine Tenart wrote:
> Hi Andrew,
> 
> On Thu, Dec 28, 2017 at 08:20:53AM +0100, Andrew Lunn wrote:
>> On Wed, Dec 27, 2017 at 11:14:41PM +0100, Antoine Tenart wrote:
>>> 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..70459a28f3a1 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_SGMII_2_5G,
>>>  	PHY_MODE_10GKR,
>>>  	PHY_MODE_UFS_HS_A,
>>>  	PHY_MODE_UFS_HS_B,
>>
>> There was a discussion maybe last month about adding 2.5G SGMII. I
>> would prefer 2500SGMII. Putting the number first makes it uniform with
>> the other defines, 1000BASEX, 25000BASEX, 10GKR.
> 
> Good to know. I wasn't completely sure how to name this mode properly,
> but I'm fine with PHY_MODE_2500SGMII. I'll update the patches and send a
> v2 (without the dt part).

And since you are respinning, please make sure you update phy_modes() in
the same header file as well as
Documentation/devicetree/bindings/net/ethernet.txt with the newly added
PHY interface mode.
-- 
Florian

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

[Florian Fainelli]

On 12/28/2017 02:06 AM, Antoine Tenart wrote:
> Hi Andrew,
> 
> On Thu, Dec 28, 2017 at 08:20:53AM +0100, Andrew Lunn wrote:
>> On Wed, Dec 27, 2017 at 11:14:41PM +0100, Antoine Tenart wrote:
>>> 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..70459a28f3a1 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_SGMII_2_5G,
>>>  	PHY_MODE_10GKR,
>>>  	PHY_MODE_UFS_HS_A,
>>>  	PHY_MODE_UFS_HS_B,
>>
>> There was a discussion maybe last month about adding 2.5G SGMII. I
>> would prefer 2500SGMII. Putting the number first makes it uniform with
>> the other defines, 1000BASEX, 25000BASEX, 10GKR.
> 
> Good to know. I wasn't completely sure how to name this mode properly,
> but I'm fine with PHY_MODE_2500SGMII. I'll update the patches and send a
> v2 (without the dt part).

And since you are respinning, please make sure you update phy_modes() in
the same header file as well as
Documentation/devicetree/bindings/net/ethernet.txt with the newly added
PHY interface mode.
-- 
Florian

Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Florian Fainelli]

On 12/28/2017 02:05 AM, Antoine Tenart wrote:
> Hi Andrew,
> 
> On Thu, Dec 28, 2017 at 08:46:23AM +0100, Andrew Lunn wrote:
>> On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote:
>>> On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
>>>>  
>>>> +&cps_eth2 {
>>>> +	/* CPS Lane 5 */
>>>> +	status = "okay";
>>>> +	phy-mode = "2500base-x";
>>>> +	/* Generic PHY, providing serdes lanes */
>>>> +	phys = <&cps_comphy5 2>;
>>>> +};
>>>> +
>>>
>>> This is wrong.  This lane is connected to a SFP cage which can support
>>> more than just 2500base-X.  Tying it in this way to 2500base-X means
>>> that this port does not support conenctions at 1000base-X, despite
>>> that's one of the most popular and more standardised speeds.
>>>
>>
>> I agree with Russell here. SFP modules are hot pluggable, and support
>> a range of interface modes. You need to query what the SFP module is
>> in order to know how to configure the SERDES interface. The phylink
>> infrastructure does that for you.
> 
> Sure, I understand. We'll be able to support such interfaces only when
> the phylink PPv2 support lands in.

Should we expect PHYLINK support to make it as the first patch in your
v2 of this patch series, or is someone else doing that?
-- 
Florian

[PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Florian Fainelli]

On 12/28/2017 02:05 AM, Antoine Tenart wrote:
> Hi Andrew,
> 
> On Thu, Dec 28, 2017 at 08:46:23AM +0100, Andrew Lunn wrote:
>> On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote:
>>> On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
>>>>  
>>>> +&cps_eth2 {
>>>> +	/* CPS Lane 5 */
>>>> +	status = "okay";
>>>> +	phy-mode = "2500base-x";
>>>> +	/* Generic PHY, providing serdes lanes */
>>>> +	phys = <&cps_comphy5 2>;
>>>> +};
>>>> +
>>>
>>> This is wrong.  This lane is connected to a SFP cage which can support
>>> more than just 2500base-X.  Tying it in this way to 2500base-X means
>>> that this port does not support conenctions at 1000base-X, despite
>>> that's one of the most popular and more standardised speeds.
>>>
>>
>> I agree with Russell here. SFP modules are hot pluggable, and support
>> a range of interface modes. You need to query what the SFP module is
>> in order to know how to configure the SERDES interface. The phylink
>> infrastructure does that for you.
> 
> Sure, I understand. We'll be able to support such interfaces only when
> the phylink PPv2 support lands in.

Should we expect PHYLINK support to make it as the first patch in your
v2 of this patch series, or is someone else doing that?
-- 
Florian

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

[Antoine Tenart]

Hi Florian,

On Thu, Dec 28, 2017 at 06:16:51AM -0800, Florian Fainelli wrote:
> 
> And since you are respinning, please make sure you update phy_modes() in
> the same header file as well as
> Documentation/devicetree/bindings/net/ethernet.txt with the newly added
> PHY interface mode.

You're right. Thanks for pointing this out!

Antoine

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

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

[Antoine Tenart]

Hi Florian,

On Thu, Dec 28, 2017 at 06:16:51AM -0800, Florian Fainelli wrote:
> 
> And since you are respinning, please make sure you update phy_modes() in
> the same header file as well as
> Documentation/devicetree/bindings/net/ethernet.txt with the newly added
> PHY interface mode.

You're right. Thanks for pointing this out!

Antoine

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

Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Antoine Tenart]

Hi Florian,

On Thu, Dec 28, 2017 at 07:02:09AM -0800, Florian Fainelli wrote:
> On 12/28/2017 02:05 AM, Antoine Tenart wrote:
> > On Thu, Dec 28, 2017 at 08:46:23AM +0100, Andrew Lunn wrote:
> >> On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote:
> >>> On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
> >>>>  
> >>>> +&cps_eth2 {
> >>>> +	/* CPS Lane 5 */
> >>>> +	status = "okay";
> >>>> +	phy-mode = "2500base-x";
> >>>> +	/* Generic PHY, providing serdes lanes */
> >>>> +	phys = <&cps_comphy5 2>;
> >>>> +};
> >>>> +
> >>>
> >>> This is wrong.  This lane is connected to a SFP cage which can support
> >>> more than just 2500base-X.  Tying it in this way to 2500base-X means
> >>> that this port does not support conenctions at 1000base-X, despite
> >>> that's one of the most popular and more standardised speeds.
> >>>
> >>
> >> I agree with Russell here. SFP modules are hot pluggable, and support
> >> a range of interface modes. You need to query what the SFP module is
> >> in order to know how to configure the SERDES interface. The phylink
> >> infrastructure does that for you.
> > 
> > Sure, I understand. We'll be able to support such interfaces only when
> > the phylink PPv2 support lands in.
> 
> Should we expect PHYLINK support to make it as the first patch in your
> v2 of this patch series, or is someone else doing that?

No, the phylink patch conflicts with Marcin's ACPI series and we agreed
to let him get his series merged first. And I will probably work on a
few other topics before having the chance to work on it. So it'll
probably be me doing that, but not right now.

This isn't an issue regarding the PPv2 and PHY patches of this series,
but yes we probably won't get the fourth network interface supported on
the mcbin during this release.

Thanks!
Antoine

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

[PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Antoine Tenart]

Hi Florian,

On Thu, Dec 28, 2017 at 07:02:09AM -0800, Florian Fainelli wrote:
> On 12/28/2017 02:05 AM, Antoine Tenart wrote:
> > On Thu, Dec 28, 2017 at 08:46:23AM +0100, Andrew Lunn wrote:
> >> On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote:
> >>> On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
> >>>>  
> >>>> +&cps_eth2 {
> >>>> +	/* CPS Lane 5 */
> >>>> +	status = "okay";
> >>>> +	phy-mode = "2500base-x";
> >>>> +	/* Generic PHY, providing serdes lanes */
> >>>> +	phys = <&cps_comphy5 2>;
> >>>> +};
> >>>> +
> >>>
> >>> This is wrong.  This lane is connected to a SFP cage which can support
> >>> more than just 2500base-X.  Tying it in this way to 2500base-X means
> >>> that this port does not support conenctions at 1000base-X, despite
> >>> that's one of the most popular and more standardised speeds.
> >>>
> >>
> >> I agree with Russell here. SFP modules are hot pluggable, and support
> >> a range of interface modes. You need to query what the SFP module is
> >> in order to know how to configure the SERDES interface. The phylink
> >> infrastructure does that for you.
> > 
> > Sure, I understand. We'll be able to support such interfaces only when
> > the phylink PPv2 support lands in.
> 
> Should we expect PHYLINK support to make it as the first patch in your
> v2 of this patch series, or is someone else doing that?

No, the phylink patch conflicts with Marcin's ACPI series and we agreed
to let him get his series merged first. And I will probably work on a
few other topics before having the chance to work on it. So it'll
probably be me doing that, but not right now.

This isn't an issue regarding the PPv2 and PHY patches of this series,
but yes we probably won't get the fourth network interface supported on
the mcbin during this release.

Thanks!
Antoine

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

Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Russell King - ARM Linux]

On Thu, Dec 28, 2017 at 07:27:39PM +0100, Antoine Tenart wrote:
> Hi Florian,
> 
> On Thu, Dec 28, 2017 at 07:02:09AM -0800, Florian Fainelli wrote:
> > On 12/28/2017 02:05 AM, Antoine Tenart wrote:
> > > On Thu, Dec 28, 2017 at 08:46:23AM +0100, Andrew Lunn wrote:
> > >> On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote:
> > >>> On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
> > >>>>  
> > >>>> +&cps_eth2 {
> > >>>> +	/* CPS Lane 5 */
> > >>>> +	status = "okay";
> > >>>> +	phy-mode = "2500base-x";
> > >>>> +	/* Generic PHY, providing serdes lanes */
> > >>>> +	phys = <&cps_comphy5 2>;
> > >>>> +};
> > >>>> +
> > >>>
> > >>> This is wrong.  This lane is connected to a SFP cage which can support
> > >>> more than just 2500base-X.  Tying it in this way to 2500base-X means
> > >>> that this port does not support conenctions at 1000base-X, despite
> > >>> that's one of the most popular and more standardised speeds.
> > >>>
> > >>
> > >> I agree with Russell here. SFP modules are hot pluggable, and support
> > >> a range of interface modes. You need to query what the SFP module is
> > >> in order to know how to configure the SERDES interface. The phylink
> > >> infrastructure does that for you.
> > > 
> > > Sure, I understand. We'll be able to support such interfaces only when
> > > the phylink PPv2 support lands in.
> > 
> > Should we expect PHYLINK support to make it as the first patch in your
> > v2 of this patch series, or is someone else doing that?
> 
> No, the phylink patch conflicts with Marcin's ACPI series and we agreed
> to let him get his series merged first. And I will probably work on a
> few other topics before having the chance to work on it. So it'll
> probably be me doing that, but not right now.

ACPI is going to be a problem with phylink for a while.  There's patches
queued in net-next which convert phylink and SFP mostly to the fwnode
and property based systems, but phylib and i2c do not seem to have the
necessary bits to be able to deal with those.

Specifically, in DT we have "of_find_i2c_adapter_by_node()" but afaics
there is no equivalent in ACPI - which means in an ACPI based system
we have no way to determine the I2C bus associated with a SFP socket,
which is a rather fundamental issue for SFP modules.

For phylib side, there's "of_phy_attach()" but again there is no
equivalent in ACPI. This should not be that much of a problem, because
network drivers using the DT phylib calls (eg, "of_phy_connect()") are
already restricted by this. That may have been solved by Marcin's
series, but I've not seen it to know.

-- 
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

[PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Russell King - ARM Linux]

On Thu, Dec 28, 2017 at 07:27:39PM +0100, Antoine Tenart wrote:
> Hi Florian,
> 
> On Thu, Dec 28, 2017 at 07:02:09AM -0800, Florian Fainelli wrote:
> > On 12/28/2017 02:05 AM, Antoine Tenart wrote:
> > > On Thu, Dec 28, 2017 at 08:46:23AM +0100, Andrew Lunn wrote:
> > >> On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote:
> > >>> On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
> > >>>>  
> > >>>> +&cps_eth2 {
> > >>>> +	/* CPS Lane 5 */
> > >>>> +	status = "okay";
> > >>>> +	phy-mode = "2500base-x";
> > >>>> +	/* Generic PHY, providing serdes lanes */
> > >>>> +	phys = <&cps_comphy5 2>;
> > >>>> +};
> > >>>> +
> > >>>
> > >>> This is wrong.  This lane is connected to a SFP cage which can support
> > >>> more than just 2500base-X.  Tying it in this way to 2500base-X means
> > >>> that this port does not support conenctions at 1000base-X, despite
> > >>> that's one of the most popular and more standardised speeds.
> > >>>
> > >>
> > >> I agree with Russell here. SFP modules are hot pluggable, and support
> > >> a range of interface modes. You need to query what the SFP module is
> > >> in order to know how to configure the SERDES interface. The phylink
> > >> infrastructure does that for you.
> > > 
> > > Sure, I understand. We'll be able to support such interfaces only when
> > > the phylink PPv2 support lands in.
> > 
> > Should we expect PHYLINK support to make it as the first patch in your
> > v2 of this patch series, or is someone else doing that?
> 
> No, the phylink patch conflicts with Marcin's ACPI series and we agreed
> to let him get his series merged first. And I will probably work on a
> few other topics before having the chance to work on it. So it'll
> probably be me doing that, but not right now.

ACPI is going to be a problem with phylink for a while.  There's patches
queued in net-next which convert phylink and SFP mostly to the fwnode
and property based systems, but phylib and i2c do not seem to have the
necessary bits to be able to deal with those.

Specifically, in DT we have "of_find_i2c_adapter_by_node()" but afaics
there is no equivalent in ACPI - which means in an ACPI based system
we have no way to determine the I2C bus associated with a SFP socket,
which is a rather fundamental issue for SFP modules.

For phylib side, there's "of_phy_attach()" but again there is no
equivalent in ACPI. This should not be that much of a problem, because
network drivers using the DT phylib calls (eg, "of_phy_connect()") are
already restricted by this. That may have been solved by Marcin's
series, but I've not seen it to know.

-- 
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 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Russell King - ARM Linux]

On Thu, Dec 28, 2017 at 11:04:16AM +0100, Antoine Tenart wrote:
> Hi Russell,
> 
> On Wed, Dec 27, 2017 at 11:20:00PM +0000, Russell King - ARM Linux wrote:
> > On Wed, Dec 27, 2017 at 11:42:52PM +0100, Antoine Tenart wrote:
> > > 
> > > What do you suggest to describe this in the dt, to enable a port using
> > > the current PPv2 driver?
> > 
> > I don't - I'm merely pointing out that you're bodging support for the
> > SFP cage rather than productively discussing phylink for mvpp2.
> > 
> > As far as I remember, the discussion stalled at this point:
> > 
> > - You think there's modes that mvpp2 supports that are not supportable
> >   if you use phylink.
> > 
> > - I've described what phylink supports, and I've asked you for details
> >   about what you can't support.
> 
> That's not what I remembered. You had some valid points, and others
> related to PHY modes the driver wasn't supporting before the phylink
> transition. My understanding of this was that you wanted a full
> featured support while I only wanted to convert the already supported
> modes.

You are mistaken - you can get a full refresher on where things were
at via https://patchwork.kernel.org/patch/9963971/

There are two points in that thread where discussion stopped awaiting
input:

1. I asked for details about what mvpp2.c supports that phylink does
   not (as you indicated that there were certain things that mvpp2
   supports that phylink does not.)  I'm still awaiting a response.

2. 25th Sept, you indicated that you would get someone to test
   an issue related to in-band AN. No results of that testing have
   been forthcoming.

Consequently, the ball is in your court on both these issues.

I am not after a full featured support, what I'm after is ensuring
that phylink is (a) used correctly and (b) implementations using it
are correct.  Part of that is ensuring that users don't introduce
unexpected failure conditions.

So, when you do this in the validate() callback:

 +       phylink_set(mask, 1000baseX_Full);

and then do this in the mac_config() callback:

 +       if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
 +           port->phy_interface != PHY_INTERFACE_MODE_SGMII)
 +               return;

and this in the link_state() callback:

 +       if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
 +           port->phy_interface != PHY_INTERFACE_MODE_SGMII)
 +               return 0;

the result is that phylink thinks that you support 1000base-X modes,
and it will call mac_config() asking for 1000base-X, but you silently
ignore that, leaving the hardware configured in whatever state it was.
That leads to a silent failure as far as the user is concerned.

So, if you do not intend to support 1000base-X initially, don't
allow it in the validate callback until you do.

It gets worse, because the return in link_state() means that phylink
thinks that the link is up if it has requested 1000base-X, which it
won't be unless you've properly configured it.

It's this kind of unreliability that I was concerned about in your
patch.  I'm not demanding "full featured implementation" but I do
want you to use it correctly.

> You're probably right about not wanting this dt patch. The non-dt
> patches still are relevant regardless of phylink being supported in the
> PPv2 driver. I'll send a v2 without the dt parts.

Thanks.

> > What I'm most concerned about, given the bindings for comphy that
> > have been merged, is that Free Electrons is pushing forward seemingly
> > with no regard to the requirement that the serdes lanes are dynamically
> > reconfigurable, and that's a basic requirement for SFP, and for the
> > 88x3310 PHYs on the Macchiatobin platform.
> 
> The main idea behind the comphy driver is to provide a way to
> reconfigure the serdes lanes at runtime. Could you develop what are
> blocking points to properly support SFP, regarding the current comphy
> support?

If it supports serdes lane mode reconfiguration (iow, switching between
1000base-X, 2500base-X, SGMII, 10G-KR), then that's all that's required.
Note that you need comphy to switch between SGMII / 10G-KR to support
the 88x3310 fully too.

Having looked deeper at this, it seems it does have the capability of
doing what's required, sorry for the noise.

-- 
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

[PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Russell King - ARM Linux]

On Thu, Dec 28, 2017 at 11:04:16AM +0100, Antoine Tenart wrote:
> Hi Russell,
> 
> On Wed, Dec 27, 2017 at 11:20:00PM +0000, Russell King - ARM Linux wrote:
> > On Wed, Dec 27, 2017 at 11:42:52PM +0100, Antoine Tenart wrote:
> > > 
> > > What do you suggest to describe this in the dt, to enable a port using
> > > the current PPv2 driver?
> > 
> > I don't - I'm merely pointing out that you're bodging support for the
> > SFP cage rather than productively discussing phylink for mvpp2.
> > 
> > As far as I remember, the discussion stalled at this point:
> > 
> > - You think there's modes that mvpp2 supports that are not supportable
> >   if you use phylink.
> > 
> > - I've described what phylink supports, and I've asked you for details
> >   about what you can't support.
> 
> That's not what I remembered. You had some valid points, and others
> related to PHY modes the driver wasn't supporting before the phylink
> transition. My understanding of this was that you wanted a full
> featured support while I only wanted to convert the already supported
> modes.

You are mistaken - you can get a full refresher on where things were
at via https://patchwork.kernel.org/patch/9963971/

There are two points in that thread where discussion stopped awaiting
input:

1. I asked for details about what mvpp2.c supports that phylink does
   not (as you indicated that there were certain things that mvpp2
   supports that phylink does not.)  I'm still awaiting a response.

2. 25th Sept, you indicated that you would get someone to test
   an issue related to in-band AN. No results of that testing have
   been forthcoming.

Consequently, the ball is in your court on both these issues.

I am not after a full featured support, what I'm after is ensuring
that phylink is (a) used correctly and (b) implementations using it
are correct.  Part of that is ensuring that users don't introduce
unexpected failure conditions.

So, when you do this in the validate() callback:

 +       phylink_set(mask, 1000baseX_Full);

and then do this in the mac_config() callback:

 +       if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
 +           port->phy_interface != PHY_INTERFACE_MODE_SGMII)
 +               return;

and this in the link_state() callback:

 +       if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
 +           port->phy_interface != PHY_INTERFACE_MODE_SGMII)
 +               return 0;

the result is that phylink thinks that you support 1000base-X modes,
and it will call mac_config() asking for 1000base-X, but you silently
ignore that, leaving the hardware configured in whatever state it was.
That leads to a silent failure as far as the user is concerned.

So, if you do not intend to support 1000base-X initially, don't
allow it in the validate callback until you do.

It gets worse, because the return in link_state() means that phylink
thinks that the link is up if it has requested 1000base-X, which it
won't be unless you've properly configured it.

It's this kind of unreliability that I was concerned about in your
patch.  I'm not demanding "full featured implementation" but I do
want you to use it correctly.

> You're probably right about not wanting this dt patch. The non-dt
> patches still are relevant regardless of phylink being supported in the
> PPv2 driver. I'll send a v2 without the dt parts.

Thanks.

> > What I'm most concerned about, given the bindings for comphy that
> > have been merged, is that Free Electrons is pushing forward seemingly
> > with no regard to the requirement that the serdes lanes are dynamically
> > reconfigurable, and that's a basic requirement for SFP, and for the
> > 88x3310 PHYs on the Macchiatobin platform.
> 
> The main idea behind the comphy driver is to provide a way to
> reconfigure the serdes lanes at runtime. Could you develop what are
> blocking points to properly support SFP, regarding the current comphy
> support?

If it supports serdes lane mode reconfiguration (iow, switching between
1000base-X, 2500base-X, SGMII, 10G-KR), then that's all that's required.
Note that you need comphy to switch between SGMII / 10G-KR to support
the 88x3310 fully too.

Having looked deeper at this, it seems it does have the capability of
doing what's required, sorry for the noise.

-- 
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 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Marcin Wojtas]

Hi Russell,

2017-12-28 19:46 GMT+01:00 Russell King - ARM Linux <linux@armlinux.org.uk>:
> On Thu, Dec 28, 2017 at 07:27:39PM +0100, Antoine Tenart wrote:
>> Hi Florian,
>>
>> On Thu, Dec 28, 2017 at 07:02:09AM -0800, Florian Fainelli wrote:
>> > On 12/28/2017 02:05 AM, Antoine Tenart wrote:
>> > > On Thu, Dec 28, 2017 at 08:46:23AM +0100, Andrew Lunn wrote:
>> > >> On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote:
>> > >>> On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
>> > >>>>
>> > >>>> +&cps_eth2 {
>> > >>>> +      /* CPS Lane 5 */
>> > >>>> +      status = "okay";
>> > >>>> +      phy-mode = "2500base-x";
>> > >>>> +      /* Generic PHY, providing serdes lanes */
>> > >>>> +      phys = <&cps_comphy5 2>;
>> > >>>> +};
>> > >>>> +
>> > >>>
>> > >>> This is wrong.  This lane is connected to a SFP cage which can support
>> > >>> more than just 2500base-X.  Tying it in this way to 2500base-X means
>> > >>> that this port does not support conenctions at 1000base-X, despite
>> > >>> that's one of the most popular and more standardised speeds.
>> > >>>
>> > >>
>> > >> I agree with Russell here. SFP modules are hot pluggable, and support
>> > >> a range of interface modes. You need to query what the SFP module is
>> > >> in order to know how to configure the SERDES interface. The phylink
>> > >> infrastructure does that for you.
>> > >
>> > > Sure, I understand. We'll be able to support such interfaces only when
>> > > the phylink PPv2 support lands in.
>> >
>> > Should we expect PHYLINK support to make it as the first patch in your
>> > v2 of this patch series, or is someone else doing that?
>>
>> No, the phylink patch conflicts with Marcin's ACPI series and we agreed
>> to let him get his series merged first. And I will probably work on a
>> few other topics before having the chance to work on it. So it'll
>> probably be me doing that, but not right now.
>
> ACPI is going to be a problem with phylink for a while.  There's patches
> queued in net-next which convert phylink and SFP mostly to the fwnode
> and property based systems, but phylib and i2c do not seem to have the
> necessary bits to be able to deal with those.
>
> Specifically, in DT we have "of_find_i2c_adapter_by_node()" but afaics
> there is no equivalent in ACPI - which means in an ACPI based system
> we have no way to determine the I2C bus associated with a SFP socket,
> which is a rather fundamental issue for SFP modules.
>
> For phylib side, there's "of_phy_attach()" but again there is no
> equivalent in ACPI. This should not be that much of a problem, because
> network drivers using the DT phylib calls (eg, "of_phy_connect()") are
> already restricted by this. That may have been solved by Marcin's
> series, but I've not seen it to know.
>

I see that I misspelled your email address, hence the series remained unnoticed:
https://lkml.org/lkml/2017/12/18/216

In terms of the phylink support, I think the most important are:
* 3/8
https://lkml.org/lkml/2017/12/18/211
* 7/8
https://lkml.org/lkml/2017/12/18/207

I think the way of obtaining PHY fwnode and connecting it from the
latter patch could be incorporated to the phylink code. Although I
didn't get much feedback, the whole ACPI-handling of MDIO bus and the
PHYs touch ACPI specification and I expect it a slower to get merged.
Hence my idea is following:
* Send v2 with ACPI supporting link-irq only in mvpp2.c
* Extract MDIO bus handling for ACPI and propose PHY handling
modifications in phylink.

This way we may push the two things forwards in more efficient way.
I'm looking forward to your opinion.

Best regards,
Marcin

[PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Marcin Wojtas]

Hi Russell,

2017-12-28 19:46 GMT+01:00 Russell King - ARM Linux <linux@armlinux.org.uk>:
> On Thu, Dec 28, 2017 at 07:27:39PM +0100, Antoine Tenart wrote:
>> Hi Florian,
>>
>> On Thu, Dec 28, 2017 at 07:02:09AM -0800, Florian Fainelli wrote:
>> > On 12/28/2017 02:05 AM, Antoine Tenart wrote:
>> > > On Thu, Dec 28, 2017 at 08:46:23AM +0100, Andrew Lunn wrote:
>> > >> On Wed, Dec 27, 2017 at 10:24:01PM +0000, Russell King - ARM Linux wrote:
>> > >>> On Wed, Dec 27, 2017 at 11:14:45PM +0100, Antoine Tenart wrote:
>> > >>>>
>> > >>>> +&cps_eth2 {
>> > >>>> +      /* CPS Lane 5 */
>> > >>>> +      status = "okay";
>> > >>>> +      phy-mode = "2500base-x";
>> > >>>> +      /* Generic PHY, providing serdes lanes */
>> > >>>> +      phys = <&cps_comphy5 2>;
>> > >>>> +};
>> > >>>> +
>> > >>>
>> > >>> This is wrong.  This lane is connected to a SFP cage which can support
>> > >>> more than just 2500base-X.  Tying it in this way to 2500base-X means
>> > >>> that this port does not support conenctions at 1000base-X, despite
>> > >>> that's one of the most popular and more standardised speeds.
>> > >>>
>> > >>
>> > >> I agree with Russell here. SFP modules are hot pluggable, and support
>> > >> a range of interface modes. You need to query what the SFP module is
>> > >> in order to know how to configure the SERDES interface. The phylink
>> > >> infrastructure does that for you.
>> > >
>> > > Sure, I understand. We'll be able to support such interfaces only when
>> > > the phylink PPv2 support lands in.
>> >
>> > Should we expect PHYLINK support to make it as the first patch in your
>> > v2 of this patch series, or is someone else doing that?
>>
>> No, the phylink patch conflicts with Marcin's ACPI series and we agreed
>> to let him get his series merged first. And I will probably work on a
>> few other topics before having the chance to work on it. So it'll
>> probably be me doing that, but not right now.
>
> ACPI is going to be a problem with phylink for a while.  There's patches
> queued in net-next which convert phylink and SFP mostly to the fwnode
> and property based systems, but phylib and i2c do not seem to have the
> necessary bits to be able to deal with those.
>
> Specifically, in DT we have "of_find_i2c_adapter_by_node()" but afaics
> there is no equivalent in ACPI - which means in an ACPI based system
> we have no way to determine the I2C bus associated with a SFP socket,
> which is a rather fundamental issue for SFP modules.
>
> For phylib side, there's "of_phy_attach()" but again there is no
> equivalent in ACPI. This should not be that much of a problem, because
> network drivers using the DT phylib calls (eg, "of_phy_connect()") are
> already restricted by this. That may have been solved by Marcin's
> series, but I've not seen it to know.
>

I see that I misspelled your email address, hence the series remained unnoticed:
https://lkml.org/lkml/2017/12/18/216

In terms of the phylink support, I think the most important are:
* 3/8
https://lkml.org/lkml/2017/12/18/211
* 7/8
https://lkml.org/lkml/2017/12/18/207

I think the way of obtaining PHY fwnode and connecting it from the
latter patch could be incorporated to the phylink code. Although I
didn't get much feedback, the whole ACPI-handling of MDIO bus and the
PHYs touch ACPI specification and I expect it a slower to get merged.
Hence my idea is following:
* Send v2 with ACPI supporting link-irq only in mvpp2.c
* Extract MDIO bus handling for ACPI and propose PHY handling
modifications in phylink.

This way we may push the two things forwards in more efficient way.
I'm looking forward to your opinion.

Best regards,
Marcin

Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Russell King - ARM Linux]

On Fri, Dec 29, 2017 at 12:12:15PM +0100, Marcin Wojtas wrote:
> Hi Russell,
> 
> I see that I misspelled your email address, hence the series remained unnoticed:
> https://lkml.org/lkml/2017/12/18/216
> 
> In terms of the phylink support, I think the most important are:
> * 3/8
> https://lkml.org/lkml/2017/12/18/211
> * 7/8
> https://lkml.org/lkml/2017/12/18/207
> 
> I think the way of obtaining PHY fwnode and connecting it from the
> latter patch could be incorporated to the phylink code. Although I
> didn't get much feedback, the whole ACPI-handling of MDIO bus and the
> PHYs touch ACPI specification and I expect it a slower to get merged.
> Hence my idea is following:
> * Send v2 with ACPI supporting link-irq only in mvpp2.c
> * Extract MDIO bus handling for ACPI and propose PHY handling
> modifications in phylink.
> 
> This way we may push the two things forwards in more efficient way.
> I'm looking forward to your opinion.

Agreed - as we have very few users of phylink at the moment (they're
mostly all in external trees) we can easily change the phylink
interfaces.  The first step is solving the ACPI representation of the
MDIO bus and attached devices, and until that is settled, not much can
be done.

However, it seems to me that the issues of adding ACPI to mvpp2 vs
adding phylink to mvpp2 are two entirely separate problems that don't
really conflict with each other - since the "phy" problem afflicts
both.

However, I'm not sure what this "link-irq" thing is that you talk
about (and I suspect it's one of the things that I've been trying for
months to find out about from Antoine when he says that there's stuff
that mvpp2 supports that phylink doesn't.)  So, I'm left to guess, and
I guess it's the mvpp2-variant of mvneta's in-band autonegotiation.
Continuing to guess from the mvpp2 phylink conversion patch, this mvpp2
variant is selected by not providing a phy handle in DT, whereas
mvneta's variant is selected using the ethernet-standard property
'managed = "in-band-status"'.

If my guessing is correct, I have to wonder why mvpp2 invented a
different way to represent this from mvneta?  This makes it much more
difficult to convert mvpp2 to phylink, and it also makes it difficult
to add SFP support ignoring the phylink issue (since there is no phy
handle there either.)

-- 
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

[PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Russell King - ARM Linux]

On Fri, Dec 29, 2017 at 12:12:15PM +0100, Marcin Wojtas wrote:
> Hi Russell,
> 
> I see that I misspelled your email address, hence the series remained unnoticed:
> https://lkml.org/lkml/2017/12/18/216
> 
> In terms of the phylink support, I think the most important are:
> * 3/8
> https://lkml.org/lkml/2017/12/18/211
> * 7/8
> https://lkml.org/lkml/2017/12/18/207
> 
> I think the way of obtaining PHY fwnode and connecting it from the
> latter patch could be incorporated to the phylink code. Although I
> didn't get much feedback, the whole ACPI-handling of MDIO bus and the
> PHYs touch ACPI specification and I expect it a slower to get merged.
> Hence my idea is following:
> * Send v2 with ACPI supporting link-irq only in mvpp2.c
> * Extract MDIO bus handling for ACPI and propose PHY handling
> modifications in phylink.
> 
> This way we may push the two things forwards in more efficient way.
> I'm looking forward to your opinion.

Agreed - as we have very few users of phylink at the moment (they're
mostly all in external trees) we can easily change the phylink
interfaces.  The first step is solving the ACPI representation of the
MDIO bus and attached devices, and until that is settled, not much can
be done.

However, it seems to me that the issues of adding ACPI to mvpp2 vs
adding phylink to mvpp2 are two entirely separate problems that don't
really conflict with each other - since the "phy" problem afflicts
both.

However, I'm not sure what this "link-irq" thing is that you talk
about (and I suspect it's one of the things that I've been trying for
months to find out about from Antoine when he says that there's stuff
that mvpp2 supports that phylink doesn't.)  So, I'm left to guess, and
I guess it's the mvpp2-variant of mvneta's in-band autonegotiation.
Continuing to guess from the mvpp2 phylink conversion patch, this mvpp2
variant is selected by not providing a phy handle in DT, whereas
mvneta's variant is selected using the ethernet-standard property
'managed = "in-band-status"'.

If my guessing is correct, I have to wonder why mvpp2 invented a
different way to represent this from mvneta?  This makes it much more
difficult to convert mvpp2 to phylink, and it also makes it difficult
to add SFP support ignoring the phylink issue (since there is no phy
handle there either.)

-- 
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 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Antoine Tenart]

Hi Russell,

On Thu, Dec 28, 2017 at 06:59:21PM +0000, Russell King - ARM Linux wrote:
> On Thu, Dec 28, 2017 at 11:04:16AM +0100, Antoine Tenart wrote:
> > 
> > That's not what I remembered. You had some valid points, and others
> > related to PHY modes the driver wasn't supporting before the phylink
> > transition. My understanding of this was that you wanted a full
> > featured support while I only wanted to convert the already supported
> > modes.
> 
> You are mistaken - you can get a full refresher on where things were
> at via https://patchwork.kernel.org/patch/9963971/

I read it again and still have the same feeling. There's been a
misunderstanding at some point. Anyway, let's move forward :)

> 1. I asked for details about what mvpp2.c supports that phylink does
>    not (as you indicated that there were certain things that mvpp2
>    supports that phylink does not.)  I'm still awaiting a response.

I don't remember PHY modes supported in the PPv2 driver that aren't
supported in PHYLINK. I think this point is the main misunderstanding. I
thought you wanted me to support modes unsupported in the PPv2 driver
before. But you explained quite well what these comments were about
below.

So I guess this point is resolved (aka I'll have to take your comments
into account for the v2).

> 2. 25th Sept, you indicated that you would get someone to test
>    an issue related to in-band AN. No results of that testing have
>    been forthcoming.

That's right. I asked someone to make a test, but did not get an answer.
And because the PHYLINK patch stalled on my side I kinda forget about
it. I'll try again to have this test made.

> I am not after a full featured support, what I'm after is ensuring
> that phylink is (a) used correctly and (b) implementations using it
> are correct.  Part of that is ensuring that users don't introduce
> unexpected failure conditions.
> 
> So, when you do this in the validate() callback:
> 
>  +       phylink_set(mask, 1000baseX_Full);
> 
> and then do this in the mac_config() callback:
> 
>  +       if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
>  +           port->phy_interface != PHY_INTERFACE_MODE_SGMII)
>  +               return;
> 
> and this in the link_state() callback:
> 
>  +       if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
>  +           port->phy_interface != PHY_INTERFACE_MODE_SGMII)
>  +               return 0;
> 
> the result is that phylink thinks that you support 1000base-X modes,
> and it will call mac_config() asking for 1000base-X, but you silently
> ignore that, leaving the hardware configured in whatever state it was.
> That leads to a silent failure as far as the user is concerned.
> 
> So, if you do not intend to support 1000base-X initially, don't
> allow it in the validate callback until you do.
> 
> It gets worse, because the return in link_state() means that phylink
> thinks that the link is up if it has requested 1000base-X, which it
> won't be unless you've properly configured it.
> 
> It's this kind of unreliability that I was concerned about in your
> patch.  I'm not demanding "full featured implementation" but I do
> want you to use it correctly.

Thanks for the detailed explanations!

> > > What I'm most concerned about, given the bindings for comphy that
> > > have been merged, is that Free Electrons is pushing forward seemingly
> > > with no regard to the requirement that the serdes lanes are dynamically
> > > reconfigurable, and that's a basic requirement for SFP, and for the
> > > 88x3310 PHYs on the Macchiatobin platform.
> > 
> > The main idea behind the comphy driver is to provide a way to
> > reconfigure the serdes lanes at runtime. Could you develop what are
> > blocking points to properly support SFP, regarding the current comphy
> > support?
> 
> If it supports serdes lane mode reconfiguration (iow, switching between
> 1000base-X, 2500base-X, SGMII, 10G-KR), then that's all that's required.

It does, and the PPv2 driver already ask the COMPHY driver to perform
these reconfigurations (when using the 10G/1G interface on the mcbin for
example).

Thanks!
Antoine

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

[PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Antoine Tenart]

Hi Russell,

On Thu, Dec 28, 2017 at 06:59:21PM +0000, Russell King - ARM Linux wrote:
> On Thu, Dec 28, 2017 at 11:04:16AM +0100, Antoine Tenart wrote:
> > 
> > That's not what I remembered. You had some valid points, and others
> > related to PHY modes the driver wasn't supporting before the phylink
> > transition. My understanding of this was that you wanted a full
> > featured support while I only wanted to convert the already supported
> > modes.
> 
> You are mistaken - you can get a full refresher on where things were
> at via https://patchwork.kernel.org/patch/9963971/

I read it again and still have the same feeling. There's been a
misunderstanding at some point. Anyway, let's move forward :)

> 1. I asked for details about what mvpp2.c supports that phylink does
>    not (as you indicated that there were certain things that mvpp2
>    supports that phylink does not.)  I'm still awaiting a response.

I don't remember PHY modes supported in the PPv2 driver that aren't
supported in PHYLINK. I think this point is the main misunderstanding. I
thought you wanted me to support modes unsupported in the PPv2 driver
before. But you explained quite well what these comments were about
below.

So I guess this point is resolved (aka I'll have to take your comments
into account for the v2).

> 2. 25th Sept, you indicated that you would get someone to test
>    an issue related to in-band AN. No results of that testing have
>    been forthcoming.

That's right. I asked someone to make a test, but did not get an answer.
And because the PHYLINK patch stalled on my side I kinda forget about
it. I'll try again to have this test made.

> I am not after a full featured support, what I'm after is ensuring
> that phylink is (a) used correctly and (b) implementations using it
> are correct.  Part of that is ensuring that users don't introduce
> unexpected failure conditions.
> 
> So, when you do this in the validate() callback:
> 
>  +       phylink_set(mask, 1000baseX_Full);
> 
> and then do this in the mac_config() callback:
> 
>  +       if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
>  +           port->phy_interface != PHY_INTERFACE_MODE_SGMII)
>  +               return;
> 
> and this in the link_state() callback:
> 
>  +       if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
>  +           port->phy_interface != PHY_INTERFACE_MODE_SGMII)
>  +               return 0;
> 
> the result is that phylink thinks that you support 1000base-X modes,
> and it will call mac_config() asking for 1000base-X, but you silently
> ignore that, leaving the hardware configured in whatever state it was.
> That leads to a silent failure as far as the user is concerned.
> 
> So, if you do not intend to support 1000base-X initially, don't
> allow it in the validate callback until you do.
> 
> It gets worse, because the return in link_state() means that phylink
> thinks that the link is up if it has requested 1000base-X, which it
> won't be unless you've properly configured it.
> 
> It's this kind of unreliability that I was concerned about in your
> patch.  I'm not demanding "full featured implementation" but I do
> want you to use it correctly.

Thanks for the detailed explanations!

> > > What I'm most concerned about, given the bindings for comphy that
> > > have been merged, is that Free Electrons is pushing forward seemingly
> > > with no regard to the requirement that the serdes lanes are dynamically
> > > reconfigurable, and that's a basic requirement for SFP, and for the
> > > 88x3310 PHYs on the Macchiatobin platform.
> > 
> > The main idea behind the comphy driver is to provide a way to
> > reconfigure the serdes lanes at runtime. Could you develop what are
> > blocking points to properly support SFP, regarding the current comphy
> > support?
> 
> If it supports serdes lane mode reconfiguration (iow, switching between
> 1000base-X, 2500base-X, SGMII, 10G-KR), then that's all that's required.

It does, and the PPv2 driver already ask the COMPHY driver to perform
these reconfigurations (when using the 10G/1G interface on the mcbin for
example).

Thanks!
Antoine

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

Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Marcin Wojtas]

Hi Russell and Stefan,

2017-12-29 12:38 GMT+01:00 Russell King - ARM Linux <linux@armlinux.org.uk>:
> On Fri, Dec 29, 2017 at 12:12:15PM +0100, Marcin Wojtas wrote:
>> Hi Russell,
>>
>> I see that I misspelled your email address, hence the series remained unnoticed:
>> https://lkml.org/lkml/2017/12/18/216
>>
>> In terms of the phylink support, I think the most important are:
>> * 3/8
>> https://lkml.org/lkml/2017/12/18/211
>> * 7/8
>> https://lkml.org/lkml/2017/12/18/207
>>
>> I think the way of obtaining PHY fwnode and connecting it from the
>> latter patch could be incorporated to the phylink code. Although I
>> didn't get much feedback, the whole ACPI-handling of MDIO bus and the
>> PHYs touch ACPI specification and I expect it a slower to get merged.
>> Hence my idea is following:
>> * Send v2 with ACPI supporting link-irq only in mvpp2.c
>> * Extract MDIO bus handling for ACPI and propose PHY handling
>> modifications in phylink.
>>
>> This way we may push the two things forwards in more efficient way.
>> I'm looking forward to your opinion.
>
> Agreed - as we have very few users of phylink at the moment (they're
> mostly all in external trees) we can easily change the phylink
> interfaces.  The first step is solving the ACPI representation of the
> MDIO bus and attached devices, and until that is settled, not much can
> be done.
>
> However, it seems to me that the issues of adding ACPI to mvpp2 vs
> adding phylink to mvpp2 are two entirely separate problems that don't
> really conflict with each other - since the "phy" problem afflicts
> both.
>

Yes, I already split the series and will send first one right away. I
will be followed by MDIO bus / PHY handling proposal, including the
bits related to phylink. I'm looking forward to your opinion on that
once sent.

> However, I'm not sure what this "link-irq" thing is that you talk
> about (and I suspect it's one of the things that I've been trying for
> months to find out about from Antoine when he says that there's stuff
> that mvpp2 supports that phylink doesn't.)  So, I'm left to guess, and
> I guess it's the mvpp2-variant of mvneta's in-band autonegotiation.
> Continuing to guess from the mvpp2 phylink conversion patch, this mvpp2
> variant is selected by not providing a phy handle in DT, whereas
> mvneta's variant is selected using the ethernet-standard property
> 'managed = "in-band-status"'.

This my understanding of how the PP2 HW works in terms of signalling
the link interrupt:

The full in-band management, similar to mvneta is supported only in
the SGMII mode (1G, not sure how it looks like in 2.5G mode). Such
handling is not yet implemented in the mvpp2.c

10G:
The XGMII MAC (XLG) is capable of generating link status change
interrupt upon information provided from the reconciliation layer (RS)
of the interface.

2.5G/1G SGMII:
Apart from the in-band management, the MAC is also capable of
generating IRQ during link-status change.

1G RGMII:
I was a bit surprised, but checked on my own - the link change IRQ can
be generated here as well.

In addition to above the clause 22 PHYs can be automatically polled
via SMI bus and provide complete information about link status, speed,
etc., reflecting it directly in GMAC status registers. However, this
feature had to be disabled, in order not to conflict with SW PHY
management of the phylib.

Stefan, is above correct?

>
> If my guessing is correct, I have to wonder why mvpp2 invented a
> different way to represent this from mvneta?  This makes it much more
> difficult to convert mvpp2 to phylink, and it also makes it difficult
> to add SFP support ignoring the phylink issue (since there is no phy
> handle there either.)

Doesn't SFP require the fwnode handle to the sfp node? This is what I
understand at least from the phylink_register_sfp.

Anyway, once the phylink is introduced in mvpp2.c, its presence will
simply be detected by port->phylink pointer. In such case the link IRQ
will no be used. In longer perspective, link IRQ should be used only
by ACPI and once MDIO bus is supported in generic way in this world,
it could remain as the 'last resort' option.

Best regards,
Marcin

[PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Marcin Wojtas]

Hi Russell and Stefan,

2017-12-29 12:38 GMT+01:00 Russell King - ARM Linux <linux@armlinux.org.uk>:
> On Fri, Dec 29, 2017 at 12:12:15PM +0100, Marcin Wojtas wrote:
>> Hi Russell,
>>
>> I see that I misspelled your email address, hence the series remained unnoticed:
>> https://lkml.org/lkml/2017/12/18/216
>>
>> In terms of the phylink support, I think the most important are:
>> * 3/8
>> https://lkml.org/lkml/2017/12/18/211
>> * 7/8
>> https://lkml.org/lkml/2017/12/18/207
>>
>> I think the way of obtaining PHY fwnode and connecting it from the
>> latter patch could be incorporated to the phylink code. Although I
>> didn't get much feedback, the whole ACPI-handling of MDIO bus and the
>> PHYs touch ACPI specification and I expect it a slower to get merged.
>> Hence my idea is following:
>> * Send v2 with ACPI supporting link-irq only in mvpp2.c
>> * Extract MDIO bus handling for ACPI and propose PHY handling
>> modifications in phylink.
>>
>> This way we may push the two things forwards in more efficient way.
>> I'm looking forward to your opinion.
>
> Agreed - as we have very few users of phylink at the moment (they're
> mostly all in external trees) we can easily change the phylink
> interfaces.  The first step is solving the ACPI representation of the
> MDIO bus and attached devices, and until that is settled, not much can
> be done.
>
> However, it seems to me that the issues of adding ACPI to mvpp2 vs
> adding phylink to mvpp2 are two entirely separate problems that don't
> really conflict with each other - since the "phy" problem afflicts
> both.
>

Yes, I already split the series and will send first one right away. I
will be followed by MDIO bus / PHY handling proposal, including the
bits related to phylink. I'm looking forward to your opinion on that
once sent.

> However, I'm not sure what this "link-irq" thing is that you talk
> about (and I suspect it's one of the things that I've been trying for
> months to find out about from Antoine when he says that there's stuff
> that mvpp2 supports that phylink doesn't.)  So, I'm left to guess, and
> I guess it's the mvpp2-variant of mvneta's in-band autonegotiation.
> Continuing to guess from the mvpp2 phylink conversion patch, this mvpp2
> variant is selected by not providing a phy handle in DT, whereas
> mvneta's variant is selected using the ethernet-standard property
> 'managed = "in-band-status"'.

This my understanding of how the PP2 HW works in terms of signalling
the link interrupt:

The full in-band management, similar to mvneta is supported only in
the SGMII mode (1G, not sure how it looks like in 2.5G mode). Such
handling is not yet implemented in the mvpp2.c

10G:
The XGMII MAC (XLG) is capable of generating link status change
interrupt upon information provided from the reconciliation layer (RS)
of the interface.

2.5G/1G SGMII:
Apart from the in-band management, the MAC is also capable of
generating IRQ during link-status change.

1G RGMII:
I was a bit surprised, but checked on my own - the link change IRQ can
be generated here as well.

In addition to above the clause 22 PHYs can be automatically polled
via SMI bus and provide complete information about link status, speed,
etc., reflecting it directly in GMAC status registers. However, this
feature had to be disabled, in order not to conflict with SW PHY
management of the phylib.

Stefan, is above correct?

>
> If my guessing is correct, I have to wonder why mvpp2 invented a
> different way to represent this from mvneta?  This makes it much more
> difficult to convert mvpp2 to phylink, and it also makes it difficult
> to add SFP support ignoring the phylink issue (since there is no phy
> handle there either.)

Doesn't SFP require the fwnode handle to the sfp node? This is what I
understand at least from the phylink_register_sfp.

Anyway, once the phylink is introduced in mvpp2.c, its presence will
simply be detected by port->phylink pointer. In such case the link IRQ
will no be used. In longer perspective, link IRQ should be used only
by ACPI and once MDIO bus is supported in generic way in this world,
it could remain as the 'last resort' option.

Best regards,
Marcin

Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Russell King - ARM Linux]

Hi Marcin,

On Sat, Dec 30, 2017 at 05:34:23PM +0100, Marcin Wojtas wrote:
> Yes, I already split the series and will send first one right away. I
> will be followed by MDIO bus / PHY handling proposal, including the
> bits related to phylink. I'm looking forward to your opinion on that
> once sent.

I'm looking forward to the patches. :)

> This my understanding of how the PP2 HW works in terms of signalling
> the link interrupt:
> 
> The full in-band management, similar to mvneta is supported only in
> the SGMII mode (1G, not sure how it looks like in 2.5G mode). Such
> handling is not yet implemented in the mvpp2.c
> 
> 10G:
> The XGMII MAC (XLG) is capable of generating link status change
> interrupt upon information provided from the reconciliation layer (RS)
> of the interface.
> 
> 2.5G/1G SGMII:
> Apart from the in-band management, the MAC is also capable of
> generating IRQ during link-status change.
> 
> 1G RGMII:
> I was a bit surprised, but checked on my own - the link change IRQ can
> be generated here as well.
> 
> In addition to above the clause 22 PHYs can be automatically polled
> via SMI bus and provide complete information about link status, speed,
> etc., reflecting it directly in GMAC status registers. However, this
> feature had to be disabled, in order not to conflict with SW PHY
> management of the phylib.
> 
> Stefan, is above correct?

This sounds very much like mvneta's 'managed = "in-band"' mode.

Having done some research earlier this month on the "2.5G SGMII" I have
a number of comments about this:

1. Beware of "SGMII" being used as a generic term for single lane serdes
   based ethernet. Marvell seem to use this for 802.3z BASE-X in their
   code, but it is not. SGMII is a modification of 802.3z BASE-X by
   Cisco.  This leads to some confusion!

2. For Cisco SGMII running at 2.5G, PHYs such as those from Xilinx do not
   support the speed bits, because the speed is defined to be 2.5G.  IOW,
   they do not support 250Mbps or 25Mbps speeds by data replication as is
   done with 100Mbps and 10Mbps over 1G SGMII.

3. There is also 1000BASE-X upclocked to 2.5G speeds, which mvneta and
   mvpp2 both support by appropriate configuration of the comphy.  I've
   already tested this with 4.3Mbps Fiberchannel SFPs between clearfog
   and mcbin - but needing devmem2 to reconfigure the clearfog comphy.

> > If my guessing is correct, I have to wonder why mvpp2 invented a
> > different way to represent this from mvneta?  This makes it much more
> > difficult to convert mvpp2 to phylink, and it also makes it difficult
> > to add SFP support ignoring the phylink issue (since there is no phy
> > handle there either.)
> 
> Doesn't SFP require the fwnode handle to the sfp node? This is what I
> understand at least from the phylink_register_sfp.

Yes, internally within phylink.  What I'm concerned about is the
following disparity between mvneta and mvpp2 - I'll try to explain it
more clearly with DT examples:

1.1. mvneta phy
	&eth {
		phy = <&phy>;
		phy-mode = "whatever";
	};
1.2. mvneta fixed-link
	&eth {
		fixed-link {
			speed = <1000>;
			full-duplex;
		};
	};
1.3. mvneta in-band
	&eth {
		phy-mode = "sgmii";
		managed = "in-band-status";
	};
2.1. mvpp2 phy
	&eth {
		phy = <&phy>;
		phy-mode = "whatever";
	};
2.2. mvpp2 fixed-link
	&eth {
		fixed-link {
			speed = <1000>;
			full-duplex;
		};
	};
2.3. mvpp2 in-band (guess)
	&eth {
		phy-mode = "sgmii";
	};

In both cases, the representation for phy and fixed-link mode are the
same, but the in-band are different.  In mvneta in-band, the generic
"managed" property must be specified as specified by
Documentation/devicetree/bindings/net/ethernet.txt.  However, for mvpp2,
this mode is currently selected by omission of both a "phy" property and
a "fixed-link" sub-node/property - and that goes against the description
of the "managed" property in the ethernet.txt binding doc.

Phylink won't recognise the mvpp2's style of "in-band" because phylink,
being a piece of generic code, is written to follow the generic binding
documentation, rather than accomodating driver's individual quirks.

So, if what I think is correct (basically what I've said above) there is
a problem converting mvpp2 to use phylink - any existing DT files that
use the "2.3 mvpp2 in-band" method instantly break, and I think that's
what Antoine referred to when I picked out that the previous patches
avoided using phylink when there was no "phy" node present.

However, I haven't spotted anything using the 2.3 method, but it's not
that easy to find the lack of a property amongst the maze of .dts*
files - trying to track down which use mvpp2 and which do not specify
a phy or fixed-link node can't be done by grep alone due to the
includes etc.  I think the only possible way would be to build all DT
files, then reverse them back to dts and search those for the mvpp2
compatible strings, and then manually check each one.

> Anyway, once the phylink is introduced in mvpp2.c, its presence will
> simply be detected by port->phylink pointer. In such case the link IRQ
> will no be used. In longer perspective, link IRQ should be used only
> by ACPI and once MDIO bus is supported in generic way in this world,
> it could remain as the 'last resort' option.

It's not though - there are SFP modules that are SGMII and we have no
access to the PHY onboard, so the only way we know what they're doing
is from the inband status sent as part of the SGMII in-band
configuration.  So, even when using phylink, we need the in-band
stuff to work, and so we need those link IRQs.

There's also additional complexities around Cisco SGMII and "extended"
SGMII concerning the flow control settings - in Cisco SGMII, there
are no bits in the 16-bit control word for communicating the flow
control to the MAC.  In extended SGMII (which appears in some Marvell
devices) you can configure flow control to appear in the 16-bit
control word, and in some cases, also EEE.  When implemented correctly
by the MAC, phylink supports the "Cisco" method when it knows that
in-band AN is being used along with a PHY - it knows to read the
settings from the MAC but combine the flow control with what has been
read from the PHY.  If this is not done, we're likely to end up with
the link partner believing that FC is supported (eg, because the PHY
has defaulted to advertising FC) but the local MAC having FC disabled.

Note that there's another quirk as far as SGMII goes - some PHYs will
not pass data until their "negotiation" (iow, passing and acknowledgement
of the SGMII control word by the MAC) has completed.  Disabling SGMII
"AN" on the MAC causes some SGMII PHYs to apparently be in "link up"
state but with no traffic flow possible in either direction.  This is
a particularly important point if using phylib - the temptation is to
use phylib to pass the results of AN to the MAC for SGMII and disable
AN on the MAC, but this is, in fact, wrong for the reason set out in
this paragraph.

There are bits present that allow AN bypass if it doesn't complete in
a certain time, but that's an entirely separate issue - especially
when there's SGMII PHYs that we have no access to!

Sorting out these nuances over the life of phylink so far has been
"interesting".

-- 
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

[PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Russell King - ARM Linux]

Hi Marcin,

On Sat, Dec 30, 2017 at 05:34:23PM +0100, Marcin Wojtas wrote:
> Yes, I already split the series and will send first one right away. I
> will be followed by MDIO bus / PHY handling proposal, including the
> bits related to phylink. I'm looking forward to your opinion on that
> once sent.

I'm looking forward to the patches. :)

> This my understanding of how the PP2 HW works in terms of signalling
> the link interrupt:
> 
> The full in-band management, similar to mvneta is supported only in
> the SGMII mode (1G, not sure how it looks like in 2.5G mode). Such
> handling is not yet implemented in the mvpp2.c
> 
> 10G:
> The XGMII MAC (XLG) is capable of generating link status change
> interrupt upon information provided from the reconciliation layer (RS)
> of the interface.
> 
> 2.5G/1G SGMII:
> Apart from the in-band management, the MAC is also capable of
> generating IRQ during link-status change.
> 
> 1G RGMII:
> I was a bit surprised, but checked on my own - the link change IRQ can
> be generated here as well.
> 
> In addition to above the clause 22 PHYs can be automatically polled
> via SMI bus and provide complete information about link status, speed,
> etc., reflecting it directly in GMAC status registers. However, this
> feature had to be disabled, in order not to conflict with SW PHY
> management of the phylib.
> 
> Stefan, is above correct?

This sounds very much like mvneta's 'managed = "in-band"' mode.

Having done some research earlier this month on the "2.5G SGMII" I have
a number of comments about this:

1. Beware of "SGMII" being used as a generic term for single lane serdes
   based ethernet. Marvell seem to use this for 802.3z BASE-X in their
   code, but it is not. SGMII is a modification of 802.3z BASE-X by
   Cisco.  This leads to some confusion!

2. For Cisco SGMII running at 2.5G, PHYs such as those from Xilinx do not
   support the speed bits, because the speed is defined to be 2.5G.  IOW,
   they do not support 250Mbps or 25Mbps speeds by data replication as is
   done with 100Mbps and 10Mbps over 1G SGMII.

3. There is also 1000BASE-X upclocked to 2.5G speeds, which mvneta and
   mvpp2 both support by appropriate configuration of the comphy.  I've
   already tested this with 4.3Mbps Fiberchannel SFPs between clearfog
   and mcbin - but needing devmem2 to reconfigure the clearfog comphy.

> > If my guessing is correct, I have to wonder why mvpp2 invented a
> > different way to represent this from mvneta?  This makes it much more
> > difficult to convert mvpp2 to phylink, and it also makes it difficult
> > to add SFP support ignoring the phylink issue (since there is no phy
> > handle there either.)
> 
> Doesn't SFP require the fwnode handle to the sfp node? This is what I
> understand at least from the phylink_register_sfp.

Yes, internally within phylink.  What I'm concerned about is the
following disparity between mvneta and mvpp2 - I'll try to explain it
more clearly with DT examples:

1.1. mvneta phy
	&eth {
		phy = <&phy>;
		phy-mode = "whatever";
	};
1.2. mvneta fixed-link
	&eth {
		fixed-link {
			speed = <1000>;
			full-duplex;
		};
	};
1.3. mvneta in-band
	&eth {
		phy-mode = "sgmii";
		managed = "in-band-status";
	};
2.1. mvpp2 phy
	&eth {
		phy = <&phy>;
		phy-mode = "whatever";
	};
2.2. mvpp2 fixed-link
	&eth {
		fixed-link {
			speed = <1000>;
			full-duplex;
		};
	};
2.3. mvpp2 in-band (guess)
	&eth {
		phy-mode = "sgmii";
	};

In both cases, the representation for phy and fixed-link mode are the
same, but the in-band are different.  In mvneta in-band, the generic
"managed" property must be specified as specified by
Documentation/devicetree/bindings/net/ethernet.txt.  However, for mvpp2,
this mode is currently selected by omission of both a "phy" property and
a "fixed-link" sub-node/property - and that goes against the description
of the "managed" property in the ethernet.txt binding doc.

Phylink won't recognise the mvpp2's style of "in-band" because phylink,
being a piece of generic code, is written to follow the generic binding
documentation, rather than accomodating driver's individual quirks.

So, if what I think is correct (basically what I've said above) there is
a problem converting mvpp2 to use phylink - any existing DT files that
use the "2.3 mvpp2 in-band" method instantly break, and I think that's
what Antoine referred to when I picked out that the previous patches
avoided using phylink when there was no "phy" node present.

However, I haven't spotted anything using the 2.3 method, but it's not
that easy to find the lack of a property amongst the maze of .dts*
files - trying to track down which use mvpp2 and which do not specify
a phy or fixed-link node can't be done by grep alone due to the
includes etc.  I think the only possible way would be to build all DT
files, then reverse them back to dts and search those for the mvpp2
compatible strings, and then manually check each one.

> Anyway, once the phylink is introduced in mvpp2.c, its presence will
> simply be detected by port->phylink pointer. In such case the link IRQ
> will no be used. In longer perspective, link IRQ should be used only
> by ACPI and once MDIO bus is supported in generic way in this world,
> it could remain as the 'last resort' option.

It's not though - there are SFP modules that are SGMII and we have no
access to the PHY onboard, so the only way we know what they're doing
is from the inband status sent as part of the SGMII in-band
configuration.  So, even when using phylink, we need the in-band
stuff to work, and so we need those link IRQs.

There's also additional complexities around Cisco SGMII and "extended"
SGMII concerning the flow control settings - in Cisco SGMII, there
are no bits in the 16-bit control word for communicating the flow
control to the MAC.  In extended SGMII (which appears in some Marvell
devices) you can configure flow control to appear in the 16-bit
control word, and in some cases, also EEE.  When implemented correctly
by the MAC, phylink supports the "Cisco" method when it knows that
in-band AN is being used along with a PHY - it knows to read the
settings from the MAC but combine the flow control with what has been
read from the PHY.  If this is not done, we're likely to end up with
the link partner believing that FC is supported (eg, because the PHY
has defaulted to advertising FC) but the local MAC having FC disabled.

Note that there's another quirk as far as SGMII goes - some PHYs will
not pass data until their "negotiation" (iow, passing and acknowledgement
of the SGMII control word by the MAC) has completed.  Disabling SGMII
"AN" on the MAC causes some SGMII PHYs to apparently be in "link up"
state but with no traffic flow possible in either direction.  This is
a particularly important point if using phylib - the temptation is to
use phylib to pass the results of AN to the MAC for SGMII and disable
AN on the MAC, but this is, in fact, wrong for the reason set out in
this paragraph.

There are bits present that allow AN bypass if it doesn't complete in
a certain time, but that's an entirely separate issue - especially
when there's SGMII PHYs that we have no access to!

Sorting out these nuances over the life of phylink so far has been
"interesting".

-- 
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 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Marcin Wojtas]

Hi Russell,

2017-12-30 18:31 GMT+01:00 Russell King - ARM Linux <linux@armlinux.org.uk>:
> Hi Marcin,
>
> On Sat, Dec 30, 2017 at 05:34:23PM +0100, Marcin Wojtas wrote:
>> Yes, I already split the series and will send first one right away. I
>> will be followed by MDIO bus / PHY handling proposal, including the
>> bits related to phylink. I'm looking forward to your opinion on that
>> once sent.
>
> I'm looking forward to the patches. :)
>
>> This my understanding of how the PP2 HW works in terms of signalling
>> the link interrupt:
>>
>> The full in-band management, similar to mvneta is supported only in
>> the SGMII mode (1G, not sure how it looks like in 2.5G mode). Such
>> handling is not yet implemented in the mvpp2.c
>>
>> 10G:
>> The XGMII MAC (XLG) is capable of generating link status change
>> interrupt upon information provided from the reconciliation layer (RS)
>> of the interface.
>>
>> 2.5G/1G SGMII:
>> Apart from the in-band management, the MAC is also capable of
>> generating IRQ during link-status change.
>>
>> 1G RGMII:
>> I was a bit surprised, but checked on my own - the link change IRQ can
>> be generated here as well.
>>
>> In addition to above the clause 22 PHYs can be automatically polled
>> via SMI bus and provide complete information about link status, speed,
>> etc., reflecting it directly in GMAC status registers. However, this
>> feature had to be disabled, in order not to conflict with SW PHY
>> management of the phylib.
>>
>> Stefan, is above correct?
>
> This sounds very much like mvneta's 'managed = "in-band"' mode.

Indeed. RGMII MAC behaves same way, although it shouldn't be named as
'in-band' to be on par with the specifications. Anyway - this one is
rather a stub for being able to work with ACPI, so once the MDIO bus
works there, this will be out of any concerns.

>
> Having done some research earlier this month on the "2.5G SGMII" I have
> a number of comments about this:
>
> 1. Beware of "SGMII" being used as a generic term for single lane serdes
>    based ethernet. Marvell seem to use this for 802.3z BASE-X in their
>    code, but it is not. SGMII is a modification of 802.3z BASE-X by
>    Cisco.  This leads to some confusion!
>
> 2. For Cisco SGMII running at 2.5G, PHYs such as those from Xilinx do not
>    support the speed bits, because the speed is defined to be 2.5G.  IOW,
>    they do not support 250Mbps or 25Mbps speeds by data replication as is
>    done with 100Mbps and 10Mbps over 1G SGMII.
>
> 3. There is also 1000BASE-X upclocked to 2.5G speeds, which mvneta and
>    mvpp2 both support by appropriate configuration of the comphy.  I've
>    already tested this with 4.3Mbps Fiberchannel SFPs between clearfog
>    and mcbin - but needing devmem2 to reconfigure the clearfog comphy.

As for 3. I tested 2.5G link on Clearfog (FreeBSD) and Armada 8040
(UEFI), so yes - the comphy is a crucial component here.

>
>> > If my guessing is correct, I have to wonder why mvpp2 invented a
>> > different way to represent this from mvneta?  This makes it much more
>> > difficult to convert mvpp2 to phylink, and it also makes it difficult
>> > to add SFP support ignoring the phylink issue (since there is no phy
>> > handle there either.)
>>
>> Doesn't SFP require the fwnode handle to the sfp node? This is what I
>> understand at least from the phylink_register_sfp.
>
> Yes, internally within phylink.  What I'm concerned about is the
> following disparity between mvneta and mvpp2 - I'll try to explain it
> more clearly with DT examples:
>
> 1.1. mvneta phy
>         &eth {
>                 phy = <&phy>;
>                 phy-mode = "whatever";
>         };
> 1.2. mvneta fixed-link
>         &eth {
>                 fixed-link {
>                         speed = <1000>;
>                         full-duplex;
>                 };
>         };
> 1.3. mvneta in-band
>         &eth {
>                 phy-mode = "sgmii";
>                 managed = "in-band-status";
>         };
> 2.1. mvpp2 phy
>         &eth {
>                 phy = <&phy>;
>                 phy-mode = "whatever";
>         };
> 2.2. mvpp2 fixed-link
>         &eth {
>                 fixed-link {
>                         speed = <1000>;
>                         full-duplex;
>                 };
>         };
> 2.3. mvpp2 in-band (guess)
>         &eth {
>                 phy-mode = "sgmii";
>         };
>
> In both cases, the representation for phy and fixed-link mode are the
> same, but the in-band are different.  In mvneta in-band, the generic
> "managed" property must be specified as specified by
> Documentation/devicetree/bindings/net/ethernet.txt.  However, for mvpp2,
> this mode is currently selected by omission of both a "phy" property and
> a "fixed-link" sub-node/property - and that goes against the description
> of the "managed" property in the ethernet.txt binding doc.
>
> Phylink won't recognise the mvpp2's style of "in-band" because phylink,
> being a piece of generic code, is written to follow the generic binding
> documentation, rather than accomodating driver's individual quirks.
>
> So, if what I think is correct (basically what I've said above) there is
> a problem converting mvpp2 to use phylink - any existing DT files that
> use the "2.3 mvpp2 in-band" method instantly break, and I think that's
> what Antoine referred to when I picked out that the previous patches
> avoided using phylink when there was no "phy" node present.
>
> However, I haven't spotted anything using the 2.3 method, but it's not
> that easy to find the lack of a property amongst the maze of .dts*
> files - trying to track down which use mvpp2 and which do not specify
> a phy or fixed-link node can't be done by grep alone due to the
> includes etc.  I think the only possible way would be to build all DT
> files, then reverse them back to dts and search those for the mvpp2
> compatible strings, and then manually check each one.
>

Thanks for detailed explanation. I agree with you - for the link IRQ,
the 'managed' property should be used. IMO this change should be a
part of mvpp2 -> phylink conversion patchset.

Best regards,
Marcin

>> Anyway, once the phylink is introduced in mvpp2.c, its presence will
>> simply be detected by port->phylink pointer. In such case the link IRQ
>> will no be used. In longer perspective, link IRQ should be used only
>> by ACPI and once MDIO bus is supported in generic way in this world,
>> it could remain as the 'last resort' option.
>
> It's not though - there are SFP modules that are SGMII and we have no
> access to the PHY onboard, so the only way we know what they're doing
> is from the inband status sent as part of the SGMII in-band
> configuration.  So, even when using phylink, we need the in-band
> stuff to work, and so we need those link IRQs.
>
> There's also additional complexities around Cisco SGMII and "extended"
> SGMII concerning the flow control settings - in Cisco SGMII, there
> are no bits in the 16-bit control word for communicating the flow
> control to the MAC.  In extended SGMII (which appears in some Marvell
> devices) you can configure flow control to appear in the 16-bit
> control word, and in some cases, also EEE.  When implemented correctly
> by the MAC, phylink supports the "Cisco" method when it knows that
> in-band AN is being used along with a PHY - it knows to read the
> settings from the MAC but combine the flow control with what has been
> read from the PHY.  If this is not done, we're likely to end up with
> the link partner believing that FC is supported (eg, because the PHY
> has defaulted to advertising FC) but the local MAC having FC disabled.
>
> Note that there's another quirk as far as SGMII goes - some PHYs will
> not pass data until their "negotiation" (iow, passing and acknowledgement
> of the SGMII control word by the MAC) has completed.  Disabling SGMII
> "AN" on the MAC causes some SGMII PHYs to apparently be in "link up"
> state but with no traffic flow possible in either direction.  This is
> a particularly important point if using phylib - the temptation is to
> use phylib to pass the results of AN to the MAC for SGMII and disable
> AN on the MAC, but this is, in fact, wrong for the reason set out in
> this paragraph.
>
> There are bits present that allow AN bypass if it doesn't complete in
> a certain time, but that's an entirely separate issue - especially
> when there's SGMII PHYs that we have no access to!
>
> Sorting out these nuances over the life of phylink so far has been
> "interesting".
>

There's ton of quirks here and there, thanks for the efforts, which
help to sort them out.

Marcin

[PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Marcin Wojtas]

Hi Russell,

2017-12-30 18:31 GMT+01:00 Russell King - ARM Linux <linux@armlinux.org.uk>:
> Hi Marcin,
>
> On Sat, Dec 30, 2017 at 05:34:23PM +0100, Marcin Wojtas wrote:
>> Yes, I already split the series and will send first one right away. I
>> will be followed by MDIO bus / PHY handling proposal, including the
>> bits related to phylink. I'm looking forward to your opinion on that
>> once sent.
>
> I'm looking forward to the patches. :)
>
>> This my understanding of how the PP2 HW works in terms of signalling
>> the link interrupt:
>>
>> The full in-band management, similar to mvneta is supported only in
>> the SGMII mode (1G, not sure how it looks like in 2.5G mode). Such
>> handling is not yet implemented in the mvpp2.c
>>
>> 10G:
>> The XGMII MAC (XLG) is capable of generating link status change
>> interrupt upon information provided from the reconciliation layer (RS)
>> of the interface.
>>
>> 2.5G/1G SGMII:
>> Apart from the in-band management, the MAC is also capable of
>> generating IRQ during link-status change.
>>
>> 1G RGMII:
>> I was a bit surprised, but checked on my own - the link change IRQ can
>> be generated here as well.
>>
>> In addition to above the clause 22 PHYs can be automatically polled
>> via SMI bus and provide complete information about link status, speed,
>> etc., reflecting it directly in GMAC status registers. However, this
>> feature had to be disabled, in order not to conflict with SW PHY
>> management of the phylib.
>>
>> Stefan, is above correct?
>
> This sounds very much like mvneta's 'managed = "in-band"' mode.

Indeed. RGMII MAC behaves same way, although it shouldn't be named as
'in-band' to be on par with the specifications. Anyway - this one is
rather a stub for being able to work with ACPI, so once the MDIO bus
works there, this will be out of any concerns.

>
> Having done some research earlier this month on the "2.5G SGMII" I have
> a number of comments about this:
>
> 1. Beware of "SGMII" being used as a generic term for single lane serdes
>    based ethernet. Marvell seem to use this for 802.3z BASE-X in their
>    code, but it is not. SGMII is a modification of 802.3z BASE-X by
>    Cisco.  This leads to some confusion!
>
> 2. For Cisco SGMII running at 2.5G, PHYs such as those from Xilinx do not
>    support the speed bits, because the speed is defined to be 2.5G.  IOW,
>    they do not support 250Mbps or 25Mbps speeds by data replication as is
>    done with 100Mbps and 10Mbps over 1G SGMII.
>
> 3. There is also 1000BASE-X upclocked to 2.5G speeds, which mvneta and
>    mvpp2 both support by appropriate configuration of the comphy.  I've
>    already tested this with 4.3Mbps Fiberchannel SFPs between clearfog
>    and mcbin - but needing devmem2 to reconfigure the clearfog comphy.

As for 3. I tested 2.5G link on Clearfog (FreeBSD) and Armada 8040
(UEFI), so yes - the comphy is a crucial component here.

>
>> > If my guessing is correct, I have to wonder why mvpp2 invented a
>> > different way to represent this from mvneta?  This makes it much more
>> > difficult to convert mvpp2 to phylink, and it also makes it difficult
>> > to add SFP support ignoring the phylink issue (since there is no phy
>> > handle there either.)
>>
>> Doesn't SFP require the fwnode handle to the sfp node? This is what I
>> understand at least from the phylink_register_sfp.
>
> Yes, internally within phylink.  What I'm concerned about is the
> following disparity between mvneta and mvpp2 - I'll try to explain it
> more clearly with DT examples:
>
> 1.1. mvneta phy
>         &eth {
>                 phy = <&phy>;
>                 phy-mode = "whatever";
>         };
> 1.2. mvneta fixed-link
>         &eth {
>                 fixed-link {
>                         speed = <1000>;
>                         full-duplex;
>                 };
>         };
> 1.3. mvneta in-band
>         &eth {
>                 phy-mode = "sgmii";
>                 managed = "in-band-status";
>         };
> 2.1. mvpp2 phy
>         &eth {
>                 phy = <&phy>;
>                 phy-mode = "whatever";
>         };
> 2.2. mvpp2 fixed-link
>         &eth {
>                 fixed-link {
>                         speed = <1000>;
>                         full-duplex;
>                 };
>         };
> 2.3. mvpp2 in-band (guess)
>         &eth {
>                 phy-mode = "sgmii";
>         };
>
> In both cases, the representation for phy and fixed-link mode are the
> same, but the in-band are different.  In mvneta in-band, the generic
> "managed" property must be specified as specified by
> Documentation/devicetree/bindings/net/ethernet.txt.  However, for mvpp2,
> this mode is currently selected by omission of both a "phy" property and
> a "fixed-link" sub-node/property - and that goes against the description
> of the "managed" property in the ethernet.txt binding doc.
>
> Phylink won't recognise the mvpp2's style of "in-band" because phylink,
> being a piece of generic code, is written to follow the generic binding
> documentation, rather than accomodating driver's individual quirks.
>
> So, if what I think is correct (basically what I've said above) there is
> a problem converting mvpp2 to use phylink - any existing DT files that
> use the "2.3 mvpp2 in-band" method instantly break, and I think that's
> what Antoine referred to when I picked out that the previous patches
> avoided using phylink when there was no "phy" node present.
>
> However, I haven't spotted anything using the 2.3 method, but it's not
> that easy to find the lack of a property amongst the maze of .dts*
> files - trying to track down which use mvpp2 and which do not specify
> a phy or fixed-link node can't be done by grep alone due to the
> includes etc.  I think the only possible way would be to build all DT
> files, then reverse them back to dts and search those for the mvpp2
> compatible strings, and then manually check each one.
>

Thanks for detailed explanation. I agree with you - for the link IRQ,
the 'managed' property should be used. IMO this change should be a
part of mvpp2 -> phylink conversion patchset.

Best regards,
Marcin

>> Anyway, once the phylink is introduced in mvpp2.c, its presence will
>> simply be detected by port->phylink pointer. In such case the link IRQ
>> will no be used. In longer perspective, link IRQ should be used only
>> by ACPI and once MDIO bus is supported in generic way in this world,
>> it could remain as the 'last resort' option.
>
> It's not though - there are SFP modules that are SGMII and we have no
> access to the PHY onboard, so the only way we know what they're doing
> is from the inband status sent as part of the SGMII in-band
> configuration.  So, even when using phylink, we need the in-band
> stuff to work, and so we need those link IRQs.
>
> There's also additional complexities around Cisco SGMII and "extended"
> SGMII concerning the flow control settings - in Cisco SGMII, there
> are no bits in the 16-bit control word for communicating the flow
> control to the MAC.  In extended SGMII (which appears in some Marvell
> devices) you can configure flow control to appear in the 16-bit
> control word, and in some cases, also EEE.  When implemented correctly
> by the MAC, phylink supports the "Cisco" method when it knows that
> in-band AN is being used along with a PHY - it knows to read the
> settings from the MAC but combine the flow control with what has been
> read from the PHY.  If this is not done, we're likely to end up with
> the link partner believing that FC is supported (eg, because the PHY
> has defaulted to advertising FC) but the local MAC having FC disabled.
>
> Note that there's another quirk as far as SGMII goes - some PHYs will
> not pass data until their "negotiation" (iow, passing and acknowledgement
> of the SGMII control word by the MAC) has completed.  Disabling SGMII
> "AN" on the MAC causes some SGMII PHYs to apparently be in "link up"
> state but with no traffic flow possible in either direction.  This is
> a particularly important point if using phylib - the temptation is to
> use phylib to pass the results of AN to the MAC for SGMII and disable
> AN on the MAC, but this is, in fact, wrong for the reason set out in
> this paragraph.
>
> There are bits present that allow AN bypass if it doesn't complete in
> a certain time, but that's an entirely separate issue - especially
> when there's SGMII PHYs that we have no access to!
>
> Sorting out these nuances over the life of phylink so far has been
> "interesting".
>

There's ton of quirks here and there, thanks for the efforts, which
help to sort them out.

Marcin

RE: [EXT] Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Stefan Chulski]

> -----Original Message-----
> From: Marcin Wojtas [mailto:mw@semihalf.com]
> Sent: Monday, January 01, 2018 12:18 PM
> To: Russell King - ARM Linux <linux@armlinux.org.uk>
> Cc: Stefan Chulski <stefanc@marvell.com>; Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com>; Andrew Lunn <andrew@lunn.ch>;
> Florian Fainelli <f.fainelli@gmail.com>; Yan Markman
> <ymarkman@marvell.com>; Jason Cooper <jason@lakedaemon.net>; netdev
> <netdev@vger.kernel.org>; Antoine Tenart <antoine.tenart@free-
> electrons.com>; linux-kernel@vger.kernel.org; kishon@ti.com; Nadav Haklai
> <nadavh@marvell.com>; Miquèl Raynal <miquel.raynal@free-electrons.com>;
> Gregory Clément <gregory.clement@free-electrons.com>; David S. Miller
> <davem@davemloft.net>; linux-arm-kernel@lists.infradead.org; Sebastian
> Hesselbarth <sebastian.hesselbarth@gmail.com>
> Subject: [EXT] Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the
> fourth network interface
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hi Russell,
> 
> 2017-12-30 18:31 GMT+01:00 Russell King - ARM Linux
> <linux@armlinux.org.uk>:
> > Hi Marcin,
> >
> > On Sat, Dec 30, 2017 at 05:34:23PM +0100, Marcin Wojtas wrote:
> >> Yes, I already split the series and will send first one right away. I
> >> will be followed by MDIO bus / PHY handling proposal, including the
> >> bits related to phylink. I'm looking forward to your opinion on that
> >> once sent.
> >
> > I'm looking forward to the patches. :)
> >
> >> This my understanding of how the PP2 HW works in terms of signalling
> >> the link interrupt:
> >>
> >> The full in-band management, similar to mvneta is supported only in
> >> the SGMII mode (1G, not sure how it looks like in 2.5G mode). Such
> >> handling is not yet implemented in the mvpp2.c
> >>
> >> 10G:
> >> The XGMII MAC (XLG) is capable of generating link status change
> >> interrupt upon information provided from the reconciliation layer
> >> (RS) of the interface.
> >>
> >> 2.5G/1G SGMII:
> >> Apart from the in-band management, the MAC is also capable of
> >> generating IRQ during link-status change.
> >>
> >> 1G RGMII:
> >> I was a bit surprised, but checked on my own - the link change IRQ
> >> can be generated here as well.
> >>
> >> In addition to above the clause 22 PHYs can be automatically polled
> >> via SMI bus and provide complete information about link status,
> >> speed, etc., reflecting it directly in GMAC status registers.
> >> However, this feature had to be disabled, in order not to conflict
> >> with SW PHY management of the phylib.
> >>
> >> Stefan, is above correct?
> >
> > This sounds very much like mvneta's 'managed = "in-band"' mode.
> 
> Indeed. RGMII MAC behaves same way, although it shouldn't be named as 'in-
> band' to be on par with the specifications. Anyway - this one is rather a stub for
> being able to work with ACPI, so once the MDIO bus works there, this will be
> out of any concerns.

Hi Marcin,

This is correct.
"in-band" supported only for SGMII mode.
IRQ link interrupt depend on "in-band"' auto negation only if "in-band"' enabled.
But IRQ link interrupt could be triggered with "in-band", "out-band" or with specific fixed speed/duplex/flow_contol.

Best Regards.

[EXT] Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Stefan Chulski]

> -----Original Message-----
> From: Marcin Wojtas [mailto:mw at semihalf.com]
> Sent: Monday, January 01, 2018 12:18 PM
> To: Russell King - ARM Linux <linux@armlinux.org.uk>
> Cc: Stefan Chulski <stefanc@marvell.com>; Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com>; Andrew Lunn <andrew@lunn.ch>;
> Florian Fainelli <f.fainelli@gmail.com>; Yan Markman
> <ymarkman@marvell.com>; Jason Cooper <jason@lakedaemon.net>; netdev
> <netdev@vger.kernel.org>; Antoine Tenart <antoine.tenart@free-
> electrons.com>; linux-kernel at vger.kernel.org; kishon at ti.com; Nadav Haklai
> <nadavh@marvell.com>; Miqu?l Raynal <miquel.raynal@free-electrons.com>;
> Gregory Cl?ment <gregory.clement@free-electrons.com>; David S. Miller
> <davem@davemloft.net>; linux-arm-kernel at lists.infradead.org; Sebastian
> Hesselbarth <sebastian.hesselbarth@gmail.com>
> Subject: [EXT] Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the
> fourth network interface
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hi Russell,
> 
> 2017-12-30 18:31 GMT+01:00 Russell King - ARM Linux
> <linux@armlinux.org.uk>:
> > Hi Marcin,
> >
> > On Sat, Dec 30, 2017 at 05:34:23PM +0100, Marcin Wojtas wrote:
> >> Yes, I already split the series and will send first one right away. I
> >> will be followed by MDIO bus / PHY handling proposal, including the
> >> bits related to phylink. I'm looking forward to your opinion on that
> >> once sent.
> >
> > I'm looking forward to the patches. :)
> >
> >> This my understanding of how the PP2 HW works in terms of signalling
> >> the link interrupt:
> >>
> >> The full in-band management, similar to mvneta is supported only in
> >> the SGMII mode (1G, not sure how it looks like in 2.5G mode). Such
> >> handling is not yet implemented in the mvpp2.c
> >>
> >> 10G:
> >> The XGMII MAC (XLG) is capable of generating link status change
> >> interrupt upon information provided from the reconciliation layer
> >> (RS) of the interface.
> >>
> >> 2.5G/1G SGMII:
> >> Apart from the in-band management, the MAC is also capable of
> >> generating IRQ during link-status change.
> >>
> >> 1G RGMII:
> >> I was a bit surprised, but checked on my own - the link change IRQ
> >> can be generated here as well.
> >>
> >> In addition to above the clause 22 PHYs can be automatically polled
> >> via SMI bus and provide complete information about link status,
> >> speed, etc., reflecting it directly in GMAC status registers.
> >> However, this feature had to be disabled, in order not to conflict
> >> with SW PHY management of the phylib.
> >>
> >> Stefan, is above correct?
> >
> > This sounds very much like mvneta's 'managed = "in-band"' mode.
> 
> Indeed. RGMII MAC behaves same way, although it shouldn't be named as 'in-
> band' to be on par with the specifications. Anyway - this one is rather a stub for
> being able to work with ACPI, so once the MDIO bus works there, this will be
> out of any concerns.

Hi Marcin,

This is correct.
"in-band" supported only for SGMII mode.
IRQ link interrupt depend on "in-band"' auto negation only if "in-band"' enabled.
But IRQ link interrupt could be triggered with "in-band", "out-band" or with specific fixed speed/duplex/flow_contol.

Best Regards.

Re: [EXT] Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Russell King - ARM Linux]

On Mon, Jan 01, 2018 at 10:35:25AM +0000, Stefan Chulski wrote:
> 
> > -----Original Message-----
> > Hi Russell,
> > 
> > Indeed. RGMII MAC behaves same way, although it shouldn't be named as 'in-
> > band' to be on par with the specifications. Anyway - this one is rather a stub for
> > being able to work with ACPI, so once the MDIO bus works there, this will be
> > out of any concerns.
> 
> Hi Marcin,
> 
> This is correct.
> "in-band" supported only for SGMII mode.
> IRQ link interrupt depend on "in-band"' auto negation only if "in-band"' enabled.
> But IRQ link interrupt could be triggered with "in-band", "out-band" or with specific fixed speed/duplex/flow_contol.

Hi Stefan,

How does this work in RGMII mode - is this handled by the PP2 polling
the PHY to get the speed, duplex and flow control settings?

Thanks.

-- 
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: [EXT] Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Russell King - ARM Linux]

On Mon, Jan 01, 2018 at 10:35:25AM +0000, Stefan Chulski wrote:
> 
> > -----Original Message-----
> > Hi Russell,
> > 
> > Indeed. RGMII MAC behaves same way, although it shouldn't be named as 'in-
> > band' to be on par with the specifications. Anyway - this one is rather a stub for
> > being able to work with ACPI, so once the MDIO bus works there, this will be
> > out of any concerns.
> 
> Hi Marcin,
> 
> This is correct.
> "in-band" supported only for SGMII mode.
> IRQ link interrupt depend on "in-band"' auto negation only if "in-band"' enabled.
> But IRQ link interrupt could be triggered with "in-band", "out-band" or with specific fixed speed/duplex/flow_contol.

Hi Stefan,

How does this work in RGMII mode - is this handled by the PP2 polling
the PHY to get the speed, duplex and flow control settings?

Thanks.

-- 
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

[EXT] Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Russell King - ARM Linux]

On Mon, Jan 01, 2018 at 10:35:25AM +0000, Stefan Chulski wrote:
> 
> > -----Original Message-----
> > Hi Russell,
> > 
> > Indeed. RGMII MAC behaves same way, although it shouldn't be named as 'in-
> > band' to be on par with the specifications. Anyway - this one is rather a stub for
> > being able to work with ACPI, so once the MDIO bus works there, this will be
> > out of any concerns.
> 
> Hi Marcin,
> 
> This is correct.
> "in-band" supported only for SGMII mode.
> IRQ link interrupt depend on "in-band"' auto negation only if "in-band"' enabled.
> But IRQ link interrupt could be triggered with "in-band", "out-band" or with specific fixed speed/duplex/flow_contol.

Hi Stefan,

How does this work in RGMII mode - is this handled by the PP2 polling
the PHY to get the speed, duplex and flow control settings?

Thanks.

-- 
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 1/6] phy: add 2.5G SGMII mode to the phy_mode enum

[Antoine Tenart]

Hi Florian,

On Thu, Dec 28, 2017 at 06:16:51AM -0800, Florian Fainelli wrote:
> On 12/28/2017 02:06 AM, Antoine Tenart wrote:
> > On Thu, Dec 28, 2017 at 08:20:53AM +0100, Andrew Lunn wrote:
> >> On Wed, Dec 27, 2017 at 11:14:41PM +0100, Antoine Tenart wrote:
> >>> 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..70459a28f3a1 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_SGMII_2_5G,
> >>>  	PHY_MODE_10GKR,
> >>>  	PHY_MODE_UFS_HS_A,
> >>>  	PHY_MODE_UFS_HS_B,
> >>
> >> There was a discussion maybe last month about adding 2.5G SGMII. I
> >> would prefer 2500SGMII. Putting the number first makes it uniform with
> >> the other defines, 1000BASEX, 25000BASEX, 10GKR.
> > 
> > Good to know. I wasn't completely sure how to name this mode properly,
> > but I'm fine with PHY_MODE_2500SGMII. I'll update the patches and send a
> > v2 (without the dt part).
> 
> And since you are respinning, please make sure you update phy_modes() in
> the same header file as well as
> Documentation/devicetree/bindings/net/ethernet.txt with the newly added
> PHY interface mode.

Actually it's a generic PHY mode I'm adding, not a network PHY mode.
There's no phy_modes() function for generic PHYs (and this 2500BaseX
mode already is supported in the network PHY modes).

Thanks!
Antoine

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

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

[Antoine Tenart]

Hi Florian,

On Thu, Dec 28, 2017 at 06:16:51AM -0800, Florian Fainelli wrote:
> On 12/28/2017 02:06 AM, Antoine Tenart wrote:
> > On Thu, Dec 28, 2017 at 08:20:53AM +0100, Andrew Lunn wrote:
> >> On Wed, Dec 27, 2017 at 11:14:41PM +0100, Antoine Tenart wrote:
> >>> 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..70459a28f3a1 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_SGMII_2_5G,
> >>>  	PHY_MODE_10GKR,
> >>>  	PHY_MODE_UFS_HS_A,
> >>>  	PHY_MODE_UFS_HS_B,
> >>
> >> There was a discussion maybe last month about adding 2.5G SGMII. I
> >> would prefer 2500SGMII. Putting the number first makes it uniform with
> >> the other defines, 1000BASEX, 25000BASEX, 10GKR.
> > 
> > Good to know. I wasn't completely sure how to name this mode properly,
> > but I'm fine with PHY_MODE_2500SGMII. I'll update the patches and send a
> > v2 (without the dt part).
> 
> And since you are respinning, please make sure you update phy_modes() in
> the same header file as well as
> Documentation/devicetree/bindings/net/ethernet.txt with the newly added
> PHY interface mode.

Actually it's a generic PHY mode I'm adding, not a network PHY mode.
There's no phy_modes() function for generic PHYs (and this 2500BaseX
mode already is supported in the network PHY modes).

Thanks!
Antoine

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

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

[Andrew Lunn]

> > >>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> > >>> index 4f8423a948d5..70459a28f3a1 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_SGMII_2_5G,
> > >>>  	PHY_MODE_10GKR,
> > >>>  	PHY_MODE_UFS_HS_A,
> > >>>  	PHY_MODE_UFS_HS_B,
> > >>
> > >> There was a discussion maybe last month about adding 2.5G SGMII. I
> > >> would prefer 2500SGMII. Putting the number first makes it uniform with
> > >> the other defines, 1000BASEX, 25000BASEX, 10GKR.
> > > 
> > > Good to know. I wasn't completely sure how to name this mode properly,
> > > but I'm fine with PHY_MODE_2500SGMII. I'll update the patches and send a
> > > v2 (without the dt part).
> > 
> > And since you are respinning, please make sure you update phy_modes() in
> > the same header file as well as
> > Documentation/devicetree/bindings/net/ethernet.txt with the newly added
> > PHY interface mode.
> 
> Actually it's a generic PHY mode I'm adding, not a network PHY mode.
> There's no phy_modes() function for generic PHYs (and this 2500BaseX
> mode already is supported in the network PHY modes).

Hi Antoine

Don't you need it in both include/linux/phy/phy.h and
include/linux/phy.h?

	Andrew

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

[Andrew Lunn]

> > >>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> > >>> index 4f8423a948d5..70459a28f3a1 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_SGMII_2_5G,
> > >>>  	PHY_MODE_10GKR,
> > >>>  	PHY_MODE_UFS_HS_A,
> > >>>  	PHY_MODE_UFS_HS_B,
> > >>
> > >> There was a discussion maybe last month about adding 2.5G SGMII. I
> > >> would prefer 2500SGMII. Putting the number first makes it uniform with
> > >> the other defines, 1000BASEX, 25000BASEX, 10GKR.
> > > 
> > > Good to know. I wasn't completely sure how to name this mode properly,
> > > but I'm fine with PHY_MODE_2500SGMII. I'll update the patches and send a
> > > v2 (without the dt part).
> > 
> > And since you are respinning, please make sure you update phy_modes() in
> > the same header file as well as
> > Documentation/devicetree/bindings/net/ethernet.txt with the newly added
> > PHY interface mode.
> 
> Actually it's a generic PHY mode I'm adding, not a network PHY mode.
> There's no phy_modes() function for generic PHYs (and this 2500BaseX
> mode already is supported in the network PHY modes).

Hi Antoine

Don't you need it in both include/linux/phy/phy.h and
include/linux/phy.h?

	Andrew

RE: [EXT] Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Stefan Chulski]

> > > -----Original Message-----
> > > Hi Russell,
> > >
> > > Indeed. RGMII MAC behaves same way, although it shouldn't be named
> > > as 'in- band' to be on par with the specifications. Anyway - this
> > > one is rather a stub for being able to work with ACPI, so once the
> > > MDIO bus works there, this will be out of any concerns.
> >
> > Hi Marcin,
> >
> > This is correct.
> > "in-band" supported only for SGMII mode.
> > IRQ link interrupt depend on "in-band"' auto negation only if "in-band"'
> enabled.
> > But IRQ link interrupt could be triggered with "in-band", "out-band" or with
> specific fixed speed/duplex/flow_contol.
> 
> Hi Stefan,
> 
> How does this work in RGMII mode - is this handled by the PP2 polling the PHY
> to get the speed, duplex and flow control settings?
> 

IRQ interrupt doesn't handled speed, duplex and flow control settings.
It's just raised if number of criterions met:
1) Physical signal detected by MAC
2) MAC auto negotiation succeeded(valid only auto negotiation enabled in MAC and "in-band" bypass disabled)

So if auto negotiation mechanism disabled in MAC or bypassed, link status would changes to up and IRQ interrupt be triggered.

In case of RGMII mode obviously we don't have "in-band" auto negotiation and "out-band" cannot be used in Kernel(due to missed locks).
So auto negotiation should be disabled on MAC level and speed/duplex/flow_contol would be negotiate by PHY. 
phylink/phylib infrastructure should provide speed/duplex/flow_contol(that were agreed between PHY's) to ppv2 driver.

Stefan.

RE: [EXT] Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Stefan Chulski]

> > > -----Original Message-----
> > > Hi Russell,
> > >
> > > Indeed. RGMII MAC behaves same way, although it shouldn't be named
> > > as 'in- band' to be on par with the specifications. Anyway - this
> > > one is rather a stub for being able to work with ACPI, so once the
> > > MDIO bus works there, this will be out of any concerns.
> >
> > Hi Marcin,
> >
> > This is correct.
> > "in-band" supported only for SGMII mode.
> > IRQ link interrupt depend on "in-band"' auto negation only if "in-band"'
> enabled.
> > But IRQ link interrupt could be triggered with "in-band", "out-band" or with
> specific fixed speed/duplex/flow_contol.
> 
> Hi Stefan,
> 
> How does this work in RGMII mode - is this handled by the PP2 polling the PHY
> to get the speed, duplex and flow control settings?
> 

IRQ interrupt doesn't handled speed, duplex and flow control settings.
It's just raised if number of criterions met:
1) Physical signal detected by MAC
2) MAC auto negotiation succeeded(valid only auto negotiation enabled in MAC and "in-band" bypass disabled)

So if auto negotiation mechanism disabled in MAC or bypassed, link status would changes to up and IRQ interrupt be triggered.

In case of RGMII mode obviously we don't have "in-band" auto negotiation and "out-band" cannot be used in Kernel(due to missed locks).
So auto negotiation should be disabled on MAC level and speed/duplex/flow_contol would be negotiate by PHY. 
phylink/phylib infrastructure should provide speed/duplex/flow_contol(that were agreed between PHY's) to ppv2 driver.

Stefan.

[EXT] Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Stefan Chulski]

> > > -----Original Message-----
> > > Hi Russell,
> > >
> > > Indeed. RGMII MAC behaves same way, although it shouldn't be named
> > > as 'in- band' to be on par with the specifications. Anyway - this
> > > one is rather a stub for being able to work with ACPI, so once the
> > > MDIO bus works there, this will be out of any concerns.
> >
> > Hi Marcin,
> >
> > This is correct.
> > "in-band" supported only for SGMII mode.
> > IRQ link interrupt depend on "in-band"' auto negation only if "in-band"'
> enabled.
> > But IRQ link interrupt could be triggered with "in-band", "out-band" or with
> specific fixed speed/duplex/flow_contol.
> 
> Hi Stefan,
> 
> How does this work in RGMII mode - is this handled by the PP2 polling the PHY
> to get the speed, duplex and flow control settings?
> 

IRQ interrupt doesn't handled speed, duplex and flow control settings.
It's just raised if number of criterions met:
1) Physical signal detected by MAC
2) MAC auto negotiation succeeded(valid only auto negotiation enabled in MAC and "in-band" bypass disabled)

So if auto negotiation mechanism disabled in MAC or bypassed, link status would changes to up and IRQ interrupt be triggered.

In case of RGMII mode obviously we don't have "in-band" auto negotiation and "out-band" cannot be used in Kernel(due to missed locks).
So auto negotiation should be disabled on MAC level and speed/duplex/flow_contol would be negotiate by PHY. 
phylink/phylib infrastructure should provide speed/duplex/flow_contol(that were agreed between PHY's) to ppv2 driver.

Stefan.

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

[Russell King - ARM Linux]

On Wed, Jan 03, 2018 at 04:08:30PM +0100, Andrew Lunn wrote:
> > > >>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> > > >>> index 4f8423a948d5..70459a28f3a1 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_SGMII_2_5G,
> > > >>>  	PHY_MODE_10GKR,
> > > >>>  	PHY_MODE_UFS_HS_A,
> > > >>>  	PHY_MODE_UFS_HS_B,
> > > >>
> > > >> There was a discussion maybe last month about adding 2.5G SGMII. I
> > > >> would prefer 2500SGMII. Putting the number first makes it uniform with
> > > >> the other defines, 1000BASEX, 25000BASEX, 10GKR.
> > > > 
> > > > Good to know. I wasn't completely sure how to name this mode properly,
> > > > but I'm fine with PHY_MODE_2500SGMII. I'll update the patches and send a
> > > > v2 (without the dt part).
> > > 
> > > And since you are respinning, please make sure you update phy_modes() in
> > > the same header file as well as
> > > Documentation/devicetree/bindings/net/ethernet.txt with the newly added
> > > PHY interface mode.
> > 
> > Actually it's a generic PHY mode I'm adding, not a network PHY mode.
> > There's no phy_modes() function for generic PHYs (and this 2500BaseX
> > mode already is supported in the network PHY modes).
> 
> Hi Antoine
> 
> Don't you need it in both include/linux/phy/phy.h and
> include/linux/phy.h?

Is it really 2.5G SGMII or 2.5G Base-X - iow, which configuration word
is permissible at 2.5G speeds for the pp2?  We already have 2500BASEX
in linux/phy.h, which is the 802.3z compliant 16-bit configuration
word rather than the Cisco variant.

-- 
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

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

[Russell King - ARM Linux]

On Wed, Jan 03, 2018 at 04:08:30PM +0100, Andrew Lunn wrote:
> > > >>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> > > >>> index 4f8423a948d5..70459a28f3a1 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_SGMII_2_5G,
> > > >>>  	PHY_MODE_10GKR,
> > > >>>  	PHY_MODE_UFS_HS_A,
> > > >>>  	PHY_MODE_UFS_HS_B,
> > > >>
> > > >> There was a discussion maybe last month about adding 2.5G SGMII. I
> > > >> would prefer 2500SGMII. Putting the number first makes it uniform with
> > > >> the other defines, 1000BASEX, 25000BASEX, 10GKR.
> > > > 
> > > > Good to know. I wasn't completely sure how to name this mode properly,
> > > > but I'm fine with PHY_MODE_2500SGMII. I'll update the patches and send a
> > > > v2 (without the dt part).
> > > 
> > > And since you are respinning, please make sure you update phy_modes() in
> > > the same header file as well as
> > > Documentation/devicetree/bindings/net/ethernet.txt with the newly added
> > > PHY interface mode.
> > 
> > Actually it's a generic PHY mode I'm adding, not a network PHY mode.
> > There's no phy_modes() function for generic PHYs (and this 2500BaseX
> > mode already is supported in the network PHY modes).
> 
> Hi Antoine
> 
> Don't you need it in both include/linux/phy/phy.h and
> include/linux/phy.h?

Is it really 2.5G SGMII or 2.5G Base-X - iow, which configuration word
is permissible at 2.5G speeds for the pp2?  We already have 2500BASEX
in linux/phy.h, which is the 802.3z compliant 16-bit configuration
word rather than the Cisco variant.

-- 
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: [EXT] Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Russell King - ARM Linux]

On Wed, Jan 03, 2018 at 05:00:47PM +0000, Stefan Chulski wrote:
> > > > -----Original Message-----
> > > > Hi Russell,
> > > >
> > > > Indeed. RGMII MAC behaves same way, although it shouldn't be named
> > > > as 'in- band' to be on par with the specifications. Anyway - this
> > > > one is rather a stub for being able to work with ACPI, so once the
> > > > MDIO bus works there, this will be out of any concerns.
> > >
> > > Hi Marcin,
> > >
> > > This is correct.
> > > "in-band" supported only for SGMII mode.
> > > IRQ link interrupt depend on "in-band"' auto negation only if "in-band"'
> > enabled.
> > > But IRQ link interrupt could be triggered with "in-band", "out-band" or with
> > specific fixed speed/duplex/flow_contol.
> > 
> > Hi Stefan,
> > 
> > How does this work in RGMII mode - is this handled by the PP2 polling the PHY
> > to get the speed, duplex and flow control settings?
> > 
> 
> IRQ interrupt doesn't handled speed, duplex and flow control settings.
> It's just raised if number of criterions met:
> 1) Physical signal detected by MAC
> 2) MAC auto negotiation succeeded(valid only auto negotiation enabled in MAC and "in-band" bypass disabled)
> 
> So if auto negotiation mechanism disabled in MAC or bypassed, link status would changes to up and IRQ interrupt be triggered.
> 
> In case of RGMII mode obviously we don't have "in-band" auto negotiation and "out-band" cannot be used in Kernel(due to missed locks).
> So auto negotiation should be disabled on MAC level and speed/duplex/flow_contol would be negotiate by PHY. 
> phylink/phylib infrastructure should provide speed/duplex/flow_contol(that were agreed between PHY's) to ppv2 driver.

Sorry, I find this very confusing.

It seems we have some people telling me that when there's no PHY
described in DT, we use this link interrupt, and have a functional
network interface (presumably at whatever speed.)

I can't see this working from what you describe - what you describe
basically tells me that when in-band autonegotiation is disabled, and we
have no PHY in the kernel, then effectively we are in fixed-link mode -
since we need to know what speed, duplex and flow control settings to
use.

So, this means that mvpp2 should be enforcing the presence of a
fixed-link description in DT if there is no PHY node at the moment, but
it doesn't.

Instead, it looks to me like the speed and duplex settings are inherited
from the boot loader or whatever was running before - I can't find
anything that configures MVPP2_GMAC_AUTONEG_CONFIG in this case.  That
seems quite a mess.

Maybe I'm missing something, but I don't see how mvpp2 can be converted
to phylink given this without causing some kind of regression, unless
someone can be much clearer about exactly what kinds of link mvpp2
supports and how they work (which is basically what I asked back in
October.)

-- 
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: [EXT] Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Russell King - ARM Linux]

On Wed, Jan 03, 2018 at 05:00:47PM +0000, Stefan Chulski wrote:
> > > > -----Original Message-----
> > > > Hi Russell,
> > > >
> > > > Indeed. RGMII MAC behaves same way, although it shouldn't be named
> > > > as 'in- band' to be on par with the specifications. Anyway - this
> > > > one is rather a stub for being able to work with ACPI, so once the
> > > > MDIO bus works there, this will be out of any concerns.
> > >
> > > Hi Marcin,
> > >
> > > This is correct.
> > > "in-band" supported only for SGMII mode.
> > > IRQ link interrupt depend on "in-band"' auto negation only if "in-band"'
> > enabled.
> > > But IRQ link interrupt could be triggered with "in-band", "out-band" or with
> > specific fixed speed/duplex/flow_contol.
> > 
> > Hi Stefan,
> > 
> > How does this work in RGMII mode - is this handled by the PP2 polling the PHY
> > to get the speed, duplex and flow control settings?
> > 
> 
> IRQ interrupt doesn't handled speed, duplex and flow control settings.
> It's just raised if number of criterions met:
> 1) Physical signal detected by MAC
> 2) MAC auto negotiation succeeded(valid only auto negotiation enabled in MAC and "in-band" bypass disabled)
> 
> So if auto negotiation mechanism disabled in MAC or bypassed, link status would changes to up and IRQ interrupt be triggered.
> 
> In case of RGMII mode obviously we don't have "in-band" auto negotiation and "out-band" cannot be used in Kernel(due to missed locks).
> So auto negotiation should be disabled on MAC level and speed/duplex/flow_contol would be negotiate by PHY. 
> phylink/phylib infrastructure should provide speed/duplex/flow_contol(that were agreed between PHY's) to ppv2 driver.

Sorry, I find this very confusing.

It seems we have some people telling me that when there's no PHY
described in DT, we use this link interrupt, and have a functional
network interface (presumably at whatever speed.)

I can't see this working from what you describe - what you describe
basically tells me that when in-band autonegotiation is disabled, and we
have no PHY in the kernel, then effectively we are in fixed-link mode -
since we need to know what speed, duplex and flow control settings to
use.

So, this means that mvpp2 should be enforcing the presence of a
fixed-link description in DT if there is no PHY node at the moment, but
it doesn't.

Instead, it looks to me like the speed and duplex settings are inherited
from the boot loader or whatever was running before - I can't find
anything that configures MVPP2_GMAC_AUTONEG_CONFIG in this case.  That
seems quite a mess.

Maybe I'm missing something, but I don't see how mvpp2 can be converted
to phylink given this without causing some kind of regression, unless
someone can be much clearer about exactly what kinds of link mvpp2
supports and how they work (which is basically what I asked back in
October.)

-- 
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

[EXT] Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Russell King - ARM Linux]

On Wed, Jan 03, 2018 at 05:00:47PM +0000, Stefan Chulski wrote:
> > > > -----Original Message-----
> > > > Hi Russell,
> > > >
> > > > Indeed. RGMII MAC behaves same way, although it shouldn't be named
> > > > as 'in- band' to be on par with the specifications. Anyway - this
> > > > one is rather a stub for being able to work with ACPI, so once the
> > > > MDIO bus works there, this will be out of any concerns.
> > >
> > > Hi Marcin,
> > >
> > > This is correct.
> > > "in-band" supported only for SGMII mode.
> > > IRQ link interrupt depend on "in-band"' auto negation only if "in-band"'
> > enabled.
> > > But IRQ link interrupt could be triggered with "in-band", "out-band" or with
> > specific fixed speed/duplex/flow_contol.
> > 
> > Hi Stefan,
> > 
> > How does this work in RGMII mode - is this handled by the PP2 polling the PHY
> > to get the speed, duplex and flow control settings?
> > 
> 
> IRQ interrupt doesn't handled speed, duplex and flow control settings.
> It's just raised if number of criterions met:
> 1) Physical signal detected by MAC
> 2) MAC auto negotiation succeeded(valid only auto negotiation enabled in MAC and "in-band" bypass disabled)
> 
> So if auto negotiation mechanism disabled in MAC or bypassed, link status would changes to up and IRQ interrupt be triggered.
> 
> In case of RGMII mode obviously we don't have "in-band" auto negotiation and "out-band" cannot be used in Kernel(due to missed locks).
> So auto negotiation should be disabled on MAC level and speed/duplex/flow_contol would be negotiate by PHY. 
> phylink/phylib infrastructure should provide speed/duplex/flow_contol(that were agreed between PHY's) to ppv2 driver.

Sorry, I find this very confusing.

It seems we have some people telling me that when there's no PHY
described in DT, we use this link interrupt, and have a functional
network interface (presumably at whatever speed.)

I can't see this working from what you describe - what you describe
basically tells me that when in-band autonegotiation is disabled, and we
have no PHY in the kernel, then effectively we are in fixed-link mode -
since we need to know what speed, duplex and flow control settings to
use.

So, this means that mvpp2 should be enforcing the presence of a
fixed-link description in DT if there is no PHY node at the moment, but
it doesn't.

Instead, it looks to me like the speed and duplex settings are inherited
from the boot loader or whatever was running before - I can't find
anything that configures MVPP2_GMAC_AUTONEG_CONFIG in this case.  That
seems quite a mess.

Maybe I'm missing something, but I don't see how mvpp2 can be converted
to phylink given this without causing some kind of regression, unless
someone can be much clearer about exactly what kinds of link mvpp2
supports and how they work (which is basically what I asked back in
October.)

-- 
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: [EXT] Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Marcin Wojtas]

Russell,

2018-01-03 18:54 GMT+01:00 Russell King - ARM Linux <linux@armlinux.org.uk>:
> On Wed, Jan 03, 2018 at 05:00:47PM +0000, Stefan Chulski wrote:
>> > > > -----Original Message-----
>> > > > Hi Russell,
>> > > >
>> > > > Indeed. RGMII MAC behaves same way, although it shouldn't be named
>> > > > as 'in- band' to be on par with the specifications. Anyway - this
>> > > > one is rather a stub for being able to work with ACPI, so once the
>> > > > MDIO bus works there, this will be out of any concerns.
>> > >
>> > > Hi Marcin,
>> > >
>> > > This is correct.
>> > > "in-band" supported only for SGMII mode.
>> > > IRQ link interrupt depend on "in-band"' auto negation only if "in-band"'
>> > enabled.
>> > > But IRQ link interrupt could be triggered with "in-band", "out-band" or with
>> > specific fixed speed/duplex/flow_contol.
>> >
>> > Hi Stefan,
>> >
>> > How does this work in RGMII mode - is this handled by the PP2 polling the PHY
>> > to get the speed, duplex and flow control settings?
>> >
>>
>> IRQ interrupt doesn't handled speed, duplex and flow control settings.
>> It's just raised if number of criterions met:
>> 1) Physical signal detected by MAC
>> 2) MAC auto negotiation succeeded(valid only auto negotiation enabled in MAC and "in-band" bypass disabled)
>>
>> So if auto negotiation mechanism disabled in MAC or bypassed, link status would changes to up and IRQ interrupt be triggered.
>>
>> In case of RGMII mode obviously we don't have "in-band" auto negotiation and "out-band" cannot be used in Kernel(due to missed locks).
>> So auto negotiation should be disabled on MAC level and speed/duplex/flow_contol would be negotiate by PHY.
>> phylink/phylib infrastructure should provide speed/duplex/flow_contol(that were agreed between PHY's) to ppv2 driver.
>
> Sorry, I find this very confusing.
>
> It seems we have some people telling me that when there's no PHY
> described in DT, we use this link interrupt, and have a functional
> network interface (presumably at whatever speed.)
>
> I can't see this working from what you describe - what you describe
> basically tells me that when in-band autonegotiation is disabled, and we
> have no PHY in the kernel, then effectively we are in fixed-link mode -
> since we need to know what speed, duplex and flow control settings to
> use.
>
> So, this means that mvpp2 should be enforcing the presence of a
> fixed-link description in DT if there is no PHY node at the moment, but
> it doesn't.
>
> Instead, it looks to me like the speed and duplex settings are inherited
> from the boot loader or whatever was running before - I can't find
> anything that configures MVPP2_GMAC_AUTONEG_CONFIG in this case.  That
> seems quite a mess.

Just 3 cents from me about the RGMII + link IRQ, which is only a
temporary solution for ACPI. It works basing on the results of UEFI HW
PHY polling and inherited GMAC settings. Once this MDIO bus / PHY
handling lands, this configuration will be out of any question and
usage in the pp2 driver.

Marcin

>
> Maybe I'm missing something, but I don't see how mvpp2 can be converted
> to phylink given this without causing some kind of regression, unless
> someone can be much clearer about exactly what kinds of link mvpp2
> supports and how they work (which is basically what I asked back in
> October.)
>
> --
> 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

[EXT] Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Marcin Wojtas]

Russell,

2018-01-03 18:54 GMT+01:00 Russell King - ARM Linux <linux@armlinux.org.uk>:
> On Wed, Jan 03, 2018 at 05:00:47PM +0000, Stefan Chulski wrote:
>> > > > -----Original Message-----
>> > > > Hi Russell,
>> > > >
>> > > > Indeed. RGMII MAC behaves same way, although it shouldn't be named
>> > > > as 'in- band' to be on par with the specifications. Anyway - this
>> > > > one is rather a stub for being able to work with ACPI, so once the
>> > > > MDIO bus works there, this will be out of any concerns.
>> > >
>> > > Hi Marcin,
>> > >
>> > > This is correct.
>> > > "in-band" supported only for SGMII mode.
>> > > IRQ link interrupt depend on "in-band"' auto negation only if "in-band"'
>> > enabled.
>> > > But IRQ link interrupt could be triggered with "in-band", "out-band" or with
>> > specific fixed speed/duplex/flow_contol.
>> >
>> > Hi Stefan,
>> >
>> > How does this work in RGMII mode - is this handled by the PP2 polling the PHY
>> > to get the speed, duplex and flow control settings?
>> >
>>
>> IRQ interrupt doesn't handled speed, duplex and flow control settings.
>> It's just raised if number of criterions met:
>> 1) Physical signal detected by MAC
>> 2) MAC auto negotiation succeeded(valid only auto negotiation enabled in MAC and "in-band" bypass disabled)
>>
>> So if auto negotiation mechanism disabled in MAC or bypassed, link status would changes to up and IRQ interrupt be triggered.
>>
>> In case of RGMII mode obviously we don't have "in-band" auto negotiation and "out-band" cannot be used in Kernel(due to missed locks).
>> So auto negotiation should be disabled on MAC level and speed/duplex/flow_contol would be negotiate by PHY.
>> phylink/phylib infrastructure should provide speed/duplex/flow_contol(that were agreed between PHY's) to ppv2 driver.
>
> Sorry, I find this very confusing.
>
> It seems we have some people telling me that when there's no PHY
> described in DT, we use this link interrupt, and have a functional
> network interface (presumably at whatever speed.)
>
> I can't see this working from what you describe - what you describe
> basically tells me that when in-band autonegotiation is disabled, and we
> have no PHY in the kernel, then effectively we are in fixed-link mode -
> since we need to know what speed, duplex and flow control settings to
> use.
>
> So, this means that mvpp2 should be enforcing the presence of a
> fixed-link description in DT if there is no PHY node at the moment, but
> it doesn't.
>
> Instead, it looks to me like the speed and duplex settings are inherited
> from the boot loader or whatever was running before - I can't find
> anything that configures MVPP2_GMAC_AUTONEG_CONFIG in this case.  That
> seems quite a mess.

Just 3 cents from me about the RGMII + link IRQ, which is only a
temporary solution for ACPI. It works basing on the results of UEFI HW
PHY polling and inherited GMAC settings. Once this MDIO bus / PHY
handling lands, this configuration will be out of any question and
usage in the pp2 driver.

Marcin

>
> Maybe I'm missing something, but I don't see how mvpp2 can be converted
> to phylink given this without causing some kind of regression, unless
> someone can be much clearer about exactly what kinds of link mvpp2
> supports and how they work (which is basically what I asked back in
> October.)
>
> --
> 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: [EXT] Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Stefan Chulski]

> Sorry, I find this very confusing.
> 
> It seems we have some people telling me that when there's no PHY described
> in DT, we use this link interrupt, and have a functional network interface
> (presumably at whatever speed.)

I give you couple of examples when we have functional network interface with
link interrupt:

1) 10G XLG mode without PHY - since XLG doesn't have auto negation mechanism
2) SGMII with in-band  auto negation
3) RGMII with auto negation done previously by boot loader or by change default fixed
    speed same as speed on cable.

Of course correct way to do it in RGMII mode is use values provided by phylink/phylib.

Stefan.

> 
> I can't see this working from what you describe - what you describe basically
> tells me that when in-band autonegotiation is disabled, and we have no PHY in
> the kernel, then effectively we are in fixed-link mode - since we need to know
> what speed, duplex and flow control settings to use.
> 
> So, this means that mvpp2 should be enforcing the presence of a fixed-link
> description in DT if there is no PHY node at the moment, but it doesn't.
> 
> Instead, it looks to me like the speed and duplex settings are inherited from the
> boot loader or whatever was running before - I can't find anything that
> configures MVPP2_GMAC_AUTONEG_CONFIG in this case.  That seems quite a
> mess.
> 
> Maybe I'm missing something, but I don't see how mvpp2 can be converted to
> phylink given this without causing some kind of regression, unless someone can
> be much clearer about exactly what kinds of link mvpp2 supports and how they
> work (which is basically what I asked back in
> October.)
> 
> --
> 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: [EXT] Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Stefan Chulski]

> Sorry, I find this very confusing.
> 
> It seems we have some people telling me that when there's no PHY described
> in DT, we use this link interrupt, and have a functional network interface
> (presumably at whatever speed.)

I give you couple of examples when we have functional network interface with
link interrupt:

1) 10G XLG mode without PHY - since XLG doesn't have auto negation mechanism
2) SGMII with in-band  auto negation
3) RGMII with auto negation done previously by boot loader or by change default fixed
    speed same as speed on cable.

Of course correct way to do it in RGMII mode is use values provided by phylink/phylib.

Stefan.

> 
> I can't see this working from what you describe - what you describe basically
> tells me that when in-band autonegotiation is disabled, and we have no PHY in
> the kernel, then effectively we are in fixed-link mode - since we need to know
> what speed, duplex and flow control settings to use.
> 
> So, this means that mvpp2 should be enforcing the presence of a fixed-link
> description in DT if there is no PHY node at the moment, but it doesn't.
> 
> Instead, it looks to me like the speed and duplex settings are inherited from the
> boot loader or whatever was running before - I can't find anything that
> configures MVPP2_GMAC_AUTONEG_CONFIG in this case.  That seems quite a
> mess.
> 
> Maybe I'm missing something, but I don't see how mvpp2 can be converted to
> phylink given this without causing some kind of regression, unless someone can
> be much clearer about exactly what kinds of link mvpp2 supports and how they
> work (which is basically what I asked back in
> October.)
> 
> --
> 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

[EXT] Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface

[Stefan Chulski]

> Sorry, I find this very confusing.
> 
> It seems we have some people telling me that when there's no PHY described
> in DT, we use this link interrupt, and have a functional network interface
> (presumably at whatever speed.)

I give you couple of examples when we have functional network interface with
link interrupt:

1) 10G XLG mode without PHY - since XLG doesn't have auto negation mechanism
2) SGMII with in-band  auto negation
3) RGMII with auto negation done previously by boot loader or by change default fixed
    speed same as speed on cable.

Of course correct way to do it in RGMII mode is use values provided by phylink/phylib.

Stefan.

> 
> I can't see this working from what you describe - what you describe basically
> tells me that when in-band autonegotiation is disabled, and we have no PHY in
> the kernel, then effectively we are in fixed-link mode - since we need to know
> what speed, duplex and flow control settings to use.
> 
> So, this means that mvpp2 should be enforcing the presence of a fixed-link
> description in DT if there is no PHY node at the moment, but it doesn't.
> 
> Instead, it looks to me like the speed and duplex settings are inherited from the
> boot loader or whatever was running before - I can't find anything that
> configures MVPP2_GMAC_AUTONEG_CONFIG in this case.  That seems quite a
> mess.
> 
> Maybe I'm missing something, but I don't see how mvpp2 can be converted to
> phylink given this without causing some kind of regression, unless someone can
> be much clearer about exactly what kinds of link mvpp2 supports and how they
> work (which is basically what I asked back in
> October.)
> 
> --
> 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