[PATCH net-next 0/2] Fix 10G PHY interface types

Recent discussion has revealed that our current usage of the 10GKR
phy_interface_t is not correct. This is based on a misunderstanding
caused in part by the various specifications being difficult to
obtain. Now that a better understanding has been reached, we ought
to correct this.

This series introduce PHY_INTERFACE_MODE_10GBASER to replace the
existing usage of 10GKR mode, and document their differences in the
phylib documentation. Then switch PHY, SFP/phylink, the Marvell
PP2 network driver, and its associated comphy driver over to use
the correct interface mode. None of the existing platform usage
was actually using 10GBASE-KR.

In order to maintain compatibility with existing DT files, arrange
for the Marvell PP2 driver to rewrite the phy interface mode; this
allows other drivers to adopt correct behaviour w.r.t whether the
10G connection conforms to the backplane 10GBASE-KR protocol vs
normal 10GBASE-R protocol.

After applying these locally to net-next I've validated that the
only places which mention the old PHY_INTERFACE_MODE_10GKR
definition are:

Documentation/networking/phy.rst:``PHY_INTERFACE_MODE_10GKR``
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:        if (phy_mode == PHY_INTERFACE_MODE_10GKR)
drivers/net/phy/aquantia_main.c:                phydev->interface = PHY_INTERFACE_MODE_10GKR;
drivers/net/phy/aquantia_main.c:            phydev->interface != PHY_INTERFACE_MODE_10GKR &&
include/linux/phy.h:    PHY_INTERFACE_MODE_10GKR,
include/linux/phy.h:    case PHY_INTERFACE_MODE_10GKR:

which is as expected.  The only users of "10gbase-kr" in DT are:

arch/arm64/boot/dts/marvell/armada-7040-db.dts: phy-mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts:     phy-mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/armada-8040-db.dts: phy-mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/armada-8040-db.dts: phy-mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/armada-8040-mcbin-singleshot.dts:   phy-mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/armada-8040-mcbin-singleshot.dts:   phy-mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts:      phy-mode = "10gbase-kr";arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts:      phy-mode = "10gbase-kr";arch/arm64/boot/dts/marvell/cn9130-db.dts:      phy-mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/cn9131-db.dts:      phy-mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/cn9132-db.dts:      phy-mode = "10gbase-kr";

which all use the mvpp2 driver, and these will be updated in a
separate patch to be submitted in the following kernel cycle.

 Documentation/networking/phy.rst                | 18 ++++++++++++++++++
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 13 ++++++++-----
 drivers/net/phy/aquantia_main.c                 |  7 +++++--
 drivers/net/phy/bcm84881.c                      |  4 ++--
 drivers/net/phy/marvell10g.c                    | 11 ++++++-----
 drivers/net/phy/phylink.c                       |  1 +
 drivers/net/phy/sfp-bus.c                       |  2 +-
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c    | 20 ++++++++++----------
 include/linux/phy.h                             | 12 ++++++++----
 9 files changed, 59 insertions(+), 29 deletions(-)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Recent discussion has revealed that the use of PHY_INTERFACE_MODE_10GKR
is incorrect. Add a 10GBASE-R definition, document both the -R and -KR
versions, and the fact that 10GKR was used incorrectly.

Signed-off-by: Russell King <redacted>
---
 Documentation/networking/phy.rst | 18 ++++++++++++++++++
 include/linux/phy.h              | 12 ++++++++----
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/Documentation/networking/phy.rst b/Documentation/networking/phy.rst
index e0a7c7af6525..6af94dadf132 100644
--- a/Documentation/networking/phy.rst
+++ b/Documentation/networking/phy.rst

@@ -267,6 +267,24 @@ negotiation results.
     duplex, pause or other settings.  This is dependent on the MAC and/or
     PHY behaviour.
 
+``PHY_INTERFACE_MODE_10GBASER``
+    This is the IEEE 802.3 Clause 49 defined 10GBASE-R protocol used with
+    various different mediums. Please refer to the IEEE standard for a
+    definition of this.
+
+    Note: 10GBASE-R is just one protocol that can be used with XFI and SFI.
+    XFI and SFI permit multiple protocols over a single SERDES lane, and
+    also define the electrical characteristics of the signals with a host
+    compliance board plugged into the host XFP/SFP connector. Therefore,
+    XFI and SFI are not PHY interface types in their own right.
+
+``PHY_INTERFACE_MODE_10GKR``
+    This is the IEEE 802.3 Clause 49 defined 10GBASE-R with Clause 73
+    autonegotiation. Please refer to the IEEE standard for further
+    information.
+
+    Note: due to legacy usage, some 10GBASE-R usage incorrectly makes
+    use of this definition.
 
 Pause frames / flow control
 ===========================

diff --git a/include/linux/phy.h b/include/linux/phy.h
index 30e599c454db..5932bb8e9c35 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h

@@ -100,9 +100,11 @@ typedef enum {
 	PHY_INTERFACE_MODE_2500BASEX,
 	PHY_INTERFACE_MODE_RXAUI,
 	PHY_INTERFACE_MODE_XAUI,
-	/* 10GBASE-KR, XFI, SFI - single lane 10G Serdes */
-	PHY_INTERFACE_MODE_10GKR,
+	/* 10GBASE-R, XFI, SFI - single lane 10G Serdes */
+	PHY_INTERFACE_MODE_10GBASER,
 	PHY_INTERFACE_MODE_USXGMII,
+	/* 10GBASE-KR - with Clause 73 AN */
+	PHY_INTERFACE_MODE_10GKR,
 	PHY_INTERFACE_MODE_MAX,
 } phy_interface_t;
 

@@ -176,10 +178,12 @@ static inline const char *phy_modes(phy_interface_t interface)
 		return "rxaui";
 	case PHY_INTERFACE_MODE_XAUI:
 		return "xaui";
-	case PHY_INTERFACE_MODE_10GKR:
-		return "10gbase-kr";
+	case PHY_INTERFACE_MODE_10GBASER:
+		return "10gbase-r";
 	case PHY_INTERFACE_MODE_USXGMII:
 		return "usxgmii";
+	case PHY_INTERFACE_MODE_10GKR:
+		return "10gbase-kr";
 	default:
 		return "unknown";
 	}

-- 
2.20.1

Switch network drivers, phy drivers, and SFP/phylink over to use the
more correct 10GBASE-R, rather than 10GBASE-KR. 10GBASE-KR is backplane
ethernet, which is 10GBASE-R with autonegotiation on top, which our
current usage on the affected platforms does not have.

The only remaining user of PHY_INTERFACE_MODE_10GKR is the Aquantia
PHY, which has a separate mode for 10GBASE-KR.

For Marvell mvpp2, we detect 10GBASE-KR, and rewrite it to 10GBASE-R
for compatibility with existing DT - this is the only network driver
at present that makes use of PHY_INTERFACE_MODE_10GKR.

Signed-off-by: Russell King <redacted>
---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 13 +++++++-----
 drivers/net/phy/aquantia_main.c               |  7 +++++--
 drivers/net/phy/bcm84881.c                    |  4 ++--
 drivers/net/phy/marvell10g.c                  | 11 +++++-----
 drivers/net/phy/phylink.c                     |  1 +
 drivers/net/phy/sfp-bus.c                     |  2 +-
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c  | 20 +++++++++----------
 7 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index cd32c7196a7f..5113dae11906 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c

@@ -1114,7 +1114,7 @@ mvpp2_shared_interrupt_mask_unmask(struct mvpp2_port *port, bool mask)
 /* Port configuration routines */
 static bool mvpp2_is_xlg(phy_interface_t interface)
 {
-	return interface == PHY_INTERFACE_MODE_10GKR ||
+	return interface == PHY_INTERFACE_MODE_10GBASER ||
 	       interface == PHY_INTERFACE_MODE_XAUI;
 }
 

@@ -1200,7 +1200,7 @@ static int mvpp22_gop_init(struct mvpp2_port *port)
 	case PHY_INTERFACE_MODE_2500BASEX:
 		mvpp22_gop_init_sgmii(port);
 		break;
-	case PHY_INTERFACE_MODE_10GKR:
+	case PHY_INTERFACE_MODE_10GBASER:
 		if (port->gop_id != 0)
 			goto invalid_conf;
 		mvpp22_gop_init_10gkr(port);

@@ -1649,7 +1649,7 @@ static void mvpp22_pcs_reset_deassert(struct mvpp2_port *port)
 	xpcs = priv->iface_base + MVPP22_XPCS_BASE(port->gop_id);
 
 	switch (port->phy_interface) {
-	case PHY_INTERFACE_MODE_10GKR:
+	case PHY_INTERFACE_MODE_10GBASER:
 		val = readl(mpcs + MVPP22_MPCS_CLK_RESET);
 		val |= MAC_CLK_RESET_MAC | MAC_CLK_RESET_SD_RX |
 		       MAC_CLK_RESET_SD_TX;

@@ -4758,7 +4758,7 @@ static void mvpp2_phylink_validate(struct phylink_config *config,
 
 	/* Invalid combinations */
 	switch (state->interface) {
-	case PHY_INTERFACE_MODE_10GKR:
+	case PHY_INTERFACE_MODE_10GBASER:
 	case PHY_INTERFACE_MODE_XAUI:
 		if (port->gop_id != 0)
 			goto empty_set;

@@ -4780,7 +4780,7 @@ static void mvpp2_phylink_validate(struct phylink_config *config,
 	phylink_set(mask, Asym_Pause);
 
 	switch (state->interface) {
-	case PHY_INTERFACE_MODE_10GKR:
+	case PHY_INTERFACE_MODE_10GBASER:
 	case PHY_INTERFACE_MODE_XAUI:
 	case PHY_INTERFACE_MODE_NA:
 		if (port->gop_id == 0) {

@@ -5247,6 +5247,9 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 		goto err_free_netdev;
 	}
 
+	if (phy_mode == PHY_INTERFACE_MODE_10GKR)
+		phy_mode = PHY_INTERFACE_MODE_10GBASER;
+
 	if (port_node) {
 		comphy = devm_of_phy_get(&pdev->dev, port_node, NULL);
 		if (IS_ERR(comphy)) {

diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia_main.c
index 975789d9349d..31927b2c7d5a 100644
--- a/drivers/net/phy/aquantia_main.c
+++ b/drivers/net/phy/aquantia_main.c

@@ -358,9 +358,11 @@ static int aqr107_read_status(struct phy_device *phydev)
 
 	switch (FIELD_GET(MDIO_PHYXS_VEND_IF_STATUS_TYPE_MASK, val)) {
 	case MDIO_PHYXS_VEND_IF_STATUS_TYPE_KR:
-	case MDIO_PHYXS_VEND_IF_STATUS_TYPE_XFI:
 		phydev->interface = PHY_INTERFACE_MODE_10GKR;
 		break;
+	case MDIO_PHYXS_VEND_IF_STATUS_TYPE_XFI:
+		phydev->interface = PHY_INTERFACE_MODE_10GBASER;
+		break;
 	case MDIO_PHYXS_VEND_IF_STATUS_TYPE_USXGMII:
 		phydev->interface = PHY_INTERFACE_MODE_USXGMII;
 		break;

@@ -493,7 +495,8 @@ static int aqr107_config_init(struct phy_device *phydev)
 	    phydev->interface != PHY_INTERFACE_MODE_2500BASEX &&
 	    phydev->interface != PHY_INTERFACE_MODE_XGMII &&
 	    phydev->interface != PHY_INTERFACE_MODE_USXGMII &&
-	    phydev->interface != PHY_INTERFACE_MODE_10GKR)
+	    phydev->interface != PHY_INTERFACE_MODE_10GKR &&
+	    phydev->interface != PHY_INTERFACE_MODE_10GBASER)
 		return -ENODEV;
 
 	WARN(phydev->interface == PHY_INTERFACE_MODE_XGMII,

diff --git a/drivers/net/phy/bcm84881.c b/drivers/net/phy/bcm84881.c
index db59911b9b3c..14d55a77eb28 100644
--- a/drivers/net/phy/bcm84881.c
+++ b/drivers/net/phy/bcm84881.c

@@ -53,7 +53,7 @@ static int bcm84881_config_init(struct phy_device *phydev)
 	switch (phydev->interface) {
 	case PHY_INTERFACE_MODE_SGMII:
 	case PHY_INTERFACE_MODE_2500BASEX:
-	case PHY_INTERFACE_MODE_10GKR:
+	case PHY_INTERFACE_MODE_10GBASER:
 		break;
 	default:
 		return -ENODEV;

@@ -218,7 +218,7 @@ static int bcm84881_read_status(struct phy_device *phydev)
 	if (mode == 1 || mode == 2)
 		phydev->interface = PHY_INTERFACE_MODE_SGMII;
 	else if (mode == 3)
-		phydev->interface = PHY_INTERFACE_MODE_10GKR;
+		phydev->interface = PHY_INTERFACE_MODE_10GBASER;
 	else if (mode == 4)
 		phydev->interface = PHY_INTERFACE_MODE_2500BASEX;
 	switch (mode & 7) {

diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 512f27b0b5cd..64c9f3bba2cd 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c

@@ -216,7 +216,7 @@ static int mv3310_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
 	sfp_parse_support(phydev->sfp_bus, id, support);
 	iface = sfp_select_interface(phydev->sfp_bus, support);
 
-	if (iface != PHY_INTERFACE_MODE_10GKR) {
+	if (iface != PHY_INTERFACE_MODE_10GBASER) {
 		dev_err(&phydev->mdio.dev, "incompatible SFP module inserted\n");
 		return -EINVAL;
 	}

@@ -304,7 +304,7 @@ static int mv3310_config_init(struct phy_device *phydev)
 	    phydev->interface != PHY_INTERFACE_MODE_2500BASEX &&
 	    phydev->interface != PHY_INTERFACE_MODE_XAUI &&
 	    phydev->interface != PHY_INTERFACE_MODE_RXAUI &&
-	    phydev->interface != PHY_INTERFACE_MODE_10GKR)
+	    phydev->interface != PHY_INTERFACE_MODE_10GBASER)
 		return -ENODEV;
 
 	return 0;

@@ -386,16 +386,17 @@ static void mv3310_update_interface(struct phy_device *phydev)
 {
 	if ((phydev->interface == PHY_INTERFACE_MODE_SGMII ||
 	     phydev->interface == PHY_INTERFACE_MODE_2500BASEX ||
-	     phydev->interface == PHY_INTERFACE_MODE_10GKR) && phydev->link) {
+	     phydev->interface == PHY_INTERFACE_MODE_10GBASER) &&
+	    phydev->link) {
 		/* The PHY automatically switches its serdes interface (and
-		 * active PHYXS instance) between Cisco SGMII, 10GBase-KR and
+		 * active PHYXS instance) between Cisco SGMII, 10GBase-R and
 		 * 2500BaseX modes according to the speed.  Florian suggests
 		 * setting phydev->interface to communicate this to the MAC.
 		 * Only do this if we are already in one of the above modes.
 		 */
 		switch (phydev->speed) {
 		case SPEED_10000:
-			phydev->interface = PHY_INTERFACE_MODE_10GKR;
+			phydev->interface = PHY_INTERFACE_MODE_10GBASER;
 			break;
 		case SPEED_2500:
 			phydev->interface = PHY_INTERFACE_MODE_2500BASEX;

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index ba9468cc8e13..cf9ce4b67826 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c

@@ -298,6 +298,7 @@ static int phylink_parse_mode(struct phylink *pl, struct fwnode_handle *fwnode)
 			break;
 
 		case PHY_INTERFACE_MODE_10GKR:
+		case PHY_INTERFACE_MODE_10GBASER:
 			phylink_set(pl->supported, 10baseT_Half);
 			phylink_set(pl->supported, 10baseT_Full);
 			phylink_set(pl->supported, 100baseT_Half);

diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
index 06e6429b8b71..d949ea7b4f8c 100644
--- a/drivers/net/phy/sfp-bus.c
+++ b/drivers/net/phy/sfp-bus.c

@@ -373,7 +373,7 @@ phy_interface_t sfp_select_interface(struct sfp_bus *bus,
 	    phylink_test(link_modes, 10000baseLRM_Full) ||
 	    phylink_test(link_modes, 10000baseER_Full) ||
 	    phylink_test(link_modes, 10000baseT_Full))
-		return PHY_INTERFACE_MODE_10GKR;
+		return PHY_INTERFACE_MODE_10GBASER;
 
 	if (phylink_test(link_modes, 2500baseX_Full))
 		return PHY_INTERFACE_MODE_2500BASEX;

diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
index e3b87c94aaf6..e41367f36ee1 100644
--- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
+++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c

@@ -221,7 +221,7 @@ static const struct mvebu_comphy_conf mvebu_comphy_cp110_modes[] = {
 	ETH_CONF(2, 0, PHY_INTERFACE_MODE_SGMII, 0x1, COMPHY_FW_MODE_SGMII),
 	ETH_CONF(2, 0, PHY_INTERFACE_MODE_2500BASEX, 0x1, COMPHY_FW_MODE_HS_SGMII),
 	ETH_CONF(2, 0, PHY_INTERFACE_MODE_RXAUI, 0x1, COMPHY_FW_MODE_RXAUI),
-	ETH_CONF(2, 0, PHY_INTERFACE_MODE_10GKR, 0x1, COMPHY_FW_MODE_XFI),
+	ETH_CONF(2, 0, PHY_INTERFACE_MODE_10GBASER, 0x1, COMPHY_FW_MODE_XFI),
 	GEN_CONF(2, 0, PHY_MODE_USB_HOST_SS, COMPHY_FW_MODE_USB3H),
 	GEN_CONF(2, 0, PHY_MODE_SATA, COMPHY_FW_MODE_SATA),
 	GEN_CONF(2, 0, PHY_MODE_PCIE, COMPHY_FW_MODE_PCIE),

@@ -235,14 +235,14 @@ static const struct mvebu_comphy_conf mvebu_comphy_cp110_modes[] = {
 	/* lane 4 */
 	ETH_CONF(4, 0, PHY_INTERFACE_MODE_SGMII, 0x2, COMPHY_FW_MODE_SGMII),
 	ETH_CONF(4, 0, PHY_INTERFACE_MODE_2500BASEX, 0x2, COMPHY_FW_MODE_HS_SGMII),
-	ETH_CONF(4, 0, PHY_INTERFACE_MODE_10GKR, 0x2, COMPHY_FW_MODE_XFI),
+	ETH_CONF(4, 0, PHY_INTERFACE_MODE_10GBASER, 0x2, COMPHY_FW_MODE_XFI),
 	ETH_CONF(4, 0, PHY_INTERFACE_MODE_RXAUI, 0x2, COMPHY_FW_MODE_RXAUI),
 	GEN_CONF(4, 0, PHY_MODE_USB_DEVICE_SS, COMPHY_FW_MODE_USB3D),
 	GEN_CONF(4, 1, PHY_MODE_USB_HOST_SS, COMPHY_FW_MODE_USB3H),
 	GEN_CONF(4, 1, PHY_MODE_PCIE, COMPHY_FW_MODE_PCIE),
 	ETH_CONF(4, 1, PHY_INTERFACE_MODE_SGMII, 0x1, COMPHY_FW_MODE_SGMII),
 	ETH_CONF(4, 1, PHY_INTERFACE_MODE_2500BASEX, -1, COMPHY_FW_MODE_HS_SGMII),
-	ETH_CONF(4, 1, PHY_INTERFACE_MODE_10GKR, -1, COMPHY_FW_MODE_XFI),
+	ETH_CONF(4, 1, PHY_INTERFACE_MODE_10GBASER, -1, COMPHY_FW_MODE_XFI),
 	/* lane 5 */
 	ETH_CONF(5, 1, PHY_INTERFACE_MODE_RXAUI, 0x2, COMPHY_FW_MODE_RXAUI),
 	GEN_CONF(5, 1, PHY_MODE_SATA, COMPHY_FW_MODE_SATA),

@@ -342,7 +342,7 @@ static int mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane)
 		 MVEBU_COMPHY_SERDES_CFG0_RXAUI_MODE);
 
 	switch (lane->submode) {
-	case PHY_INTERFACE_MODE_10GKR:
+	case PHY_INTERFACE_MODE_10GBASER:
 		val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0xe) |
 		       MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0xe);
 		break;

@@ -417,7 +417,7 @@ static int mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane)
 	/* refclk selection */
 	val = readl(priv->base + MVEBU_COMPHY_MISC_CTRL0(lane->id));
 	val &= ~MVEBU_COMPHY_MISC_CTRL0_REFCLK_SEL;
-	if (lane->submode == PHY_INTERFACE_MODE_10GKR)
+	if (lane->submode == PHY_INTERFACE_MODE_10GBASER)
 		val |= MVEBU_COMPHY_MISC_CTRL0_ICP_FORCE;
 	writel(val, priv->base + MVEBU_COMPHY_MISC_CTRL0(lane->id));
 

@@ -564,7 +564,7 @@ static int mvebu_comphy_set_mode_rxaui(struct phy *phy)
 	return mvebu_comphy_init_plls(lane);
 }
 
-static int mvebu_comphy_set_mode_10gkr(struct phy *phy)
+static int mvebu_comphy_set_mode_10gbaser(struct phy *phy)
 {
 	struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
 	struct mvebu_comphy_priv *priv = lane->priv;

@@ -735,8 +735,8 @@ static int mvebu_comphy_power_on_legacy(struct phy *phy)
 	case PHY_INTERFACE_MODE_RXAUI:
 		ret = mvebu_comphy_set_mode_rxaui(phy);
 		break;
-	case PHY_INTERFACE_MODE_10GKR:
-		ret = mvebu_comphy_set_mode_10gkr(phy);
+	case PHY_INTERFACE_MODE_10GBASER:
+		ret = mvebu_comphy_set_mode_10gbaser(phy);
 		break;
 	default:
 		return -ENOTSUPP;

@@ -782,8 +782,8 @@ static int mvebu_comphy_power_on(struct phy *phy)
 				lane->id);
 			fw_speed = COMPHY_FW_SPEED_3125;
 			break;
-		case PHY_INTERFACE_MODE_10GKR:
-			dev_dbg(priv->dev, "set lane %d to 10G-KR mode\n",
+		case PHY_INTERFACE_MODE_10GBASER:
+			dev_dbg(priv->dev, "set lane %d to 10GBASE-R mode\n",
 				lane->id);
 			fw_speed = COMPHY_FW_SPEED_103125;
 			break;

-- 
2.20.1

-----Original Message-----
From: netdev-owner@vger.kernel.org <redacted> On
Behalf Of Russell King - ARM Linux admin
Sent: Friday, January 3, 2020 1:51 PM
To: Andrew Lunn <andrew@lunn.ch>; Florian Fainelli <f.fainelli@gmail.com>;
Heiner Kallweit [off-list ref]
Cc: David S. Miller <davem@davemloft.net>; Jonathan Corbet
[off-list ref]; Kishon Vijay Abraham I [off-list ref]; linux-
doc@vger.kernel.org; netdev@vger.kernel.org
Subject: [PATCH net-next 0/2] Fix 10G PHY interface types

Recent discussion has revealed that our current usage of the 10GKR
phy_interface_t is not correct. This is based on a misunderstanding
caused in part by the various specifications being difficult to
obtain. Now that a better understanding has been reached, we ought
to correct this.

This series introduce PHY_INTERFACE_MODE_10GBASER to replace the
existing usage of 10GKR mode, and document their differences in the
phylib documentation. Then switch PHY, SFP/phylink, the Marvell
PP2 network driver, and its associated comphy driver over to use
the correct interface mode. None of the existing platform usage
was actually using 10GBASE-KR.

In order to maintain compatibility with existing DT files, arrange
for the Marvell PP2 driver to rewrite the phy interface mode; this
allows other drivers to adopt correct behaviour w.r.t whether the
10G connection conforms to the backplane 10GBASE-KR protocol vs
normal 10GBASE-R protocol.

After applying these locally to net-next I've validated that the
only places which mention the old PHY_INTERFACE_MODE_10GKR
definition are:

Documentation/networking/phy.rst:``PHY_INTERFACE_MODE_10GKR``
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:        if (phy_mode ==
PHY_INTERFACE_MODE_10GKR)
drivers/net/phy/aquantia_main.c:                phydev->interface =
PHY_INTERFACE_MODE_10GKR;
drivers/net/phy/aquantia_main.c:            phydev->interface !=
PHY_INTERFACE_MODE_10GKR &&
include/linux/phy.h:    PHY_INTERFACE_MODE_10GKR,
include/linux/phy.h:    case PHY_INTERFACE_MODE_10GKR:

which is as expected.  The only users of "10gbase-kr" in DT are:

arch/arm64/boot/dts/marvell/armada-7040-db.dts: phy-mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts:     phy-mode =
"10gbase-kr";
arch/arm64/boot/dts/marvell/armada-8040-db.dts: phy-mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/armada-8040-db.dts: phy-mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/armada-8040-mcbin-singleshot.dts:   phy-mode =
"10gbase-kr";
arch/arm64/boot/dts/marvell/armada-8040-mcbin-singleshot.dts:   phy-mode =
"10gbase-kr";
arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts:      phy-mode =
"10gbase-kr";arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts:      phy-
mode = "10gbase-kr";arch/arm64/boot/dts/marvell/cn9130-db.dts:      phy-
mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/cn9131-db.dts:      phy-mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/cn9132-db.dts:      phy-mode = "10gbase-kr";

which all use the mvpp2 driver, and these will be updated in a
separate patch to be submitted in the following kernel cycle.

 Documentation/networking/phy.rst                | 18 ++++++++++++++++++
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 13 ++++++++-----
 drivers/net/phy/aquantia_main.c                 |  7 +++++--
 drivers/net/phy/bcm84881.c                      |  4 ++--
 drivers/net/phy/marvell10g.c                    | 11 ++++++-----
 drivers/net/phy/phylink.c                       |  1 +
 drivers/net/phy/sfp-bus.c                       |  2 +-
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c    | 20 ++++++++++----------
 include/linux/phy.h                             | 12 ++++++++----
 9 files changed, 59 insertions(+), 29 deletions(-)

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps
up
According to speedtest.net: 11.9Mbps down 500kbps up

Hi,

we should conclude our discussions on the initial thread before we do this.
The current use on 10GBASE_KR is not correct, I agree but changing this to
10GBASE-R may not be the way to go. We need to determine if the existing
device tree binding corelated type is the one we need to change or maybe a
more complex solution is required. There are multiple paths forward:

Extending this enum that has a mix of things in it that are barely related.

For 10G Ethernet there is already an XGMII entry that describes the MII, if
this should stick to strict MIIs. This is of little value for chips that
have part of the traditional PHY blocks grouped along with the MAC, the XGMII
is not exposed outside the RTL (if it even exists there) and the actual
visible interfaces are completely different.

Describing the actual interface at chip to chip level (RGMII, SGMII, XAUI,
XFI, etc.). This may be incomplete for people trying to configure their HW
that supports multiple modes (reminder - device trees describe HW, they do
not configure SW). More details would be required and the list would be...
eclectic.

Moving to something different altogether, that would not conflict existing
device trees but permit a more thorough classification and less ambiguity.
We need more work on clearing a path towards that.

Regards,
Madalin

On Fri, Jan 03, 2020 at 12:22:58PM +0000, Madalin Bucur (OSS) wrote:

quoted

-----Original Message-----
From: netdev-owner@vger.kernel.org <redacted> On
Behalf Of Russell King - ARM Linux admin
Sent: Friday, January 3, 2020 1:51 PM
To: Andrew Lunn <andrew@lunn.ch>; Florian Fainelli <f.fainelli@gmail.com>;
Heiner Kallweit [off-list ref]
Cc: David S. Miller <davem@davemloft.net>; Jonathan Corbet
[off-list ref]; Kishon Vijay Abraham I [off-list ref]; linux-
doc@vger.kernel.org; netdev@vger.kernel.org
Subject: [PATCH net-next 0/2] Fix 10G PHY interface types

Recent discussion has revealed that our current usage of the 10GKR
phy_interface_t is not correct. This is based on a misunderstanding
caused in part by the various specifications being difficult to
obtain. Now that a better understanding has been reached, we ought
to correct this.

This series introduce PHY_INTERFACE_MODE_10GBASER to replace the
existing usage of 10GKR mode, and document their differences in the
phylib documentation. Then switch PHY, SFP/phylink, the Marvell
PP2 network driver, and its associated comphy driver over to use
the correct interface mode. None of the existing platform usage
was actually using 10GBASE-KR.

In order to maintain compatibility with existing DT files, arrange
for the Marvell PP2 driver to rewrite the phy interface mode; this
allows other drivers to adopt correct behaviour w.r.t whether the
10G connection conforms to the backplane 10GBASE-KR protocol vs
normal 10GBASE-R protocol.

After applying these locally to net-next I've validated that the
only places which mention the old PHY_INTERFACE_MODE_10GKR
definition are:

Documentation/networking/phy.rst:``PHY_INTERFACE_MODE_10GKR``
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:        if (phy_mode ==
PHY_INTERFACE_MODE_10GKR)
drivers/net/phy/aquantia_main.c:                phydev->interface =
PHY_INTERFACE_MODE_10GKR;
drivers/net/phy/aquantia_main.c:            phydev->interface !=
PHY_INTERFACE_MODE_10GKR &&
include/linux/phy.h:    PHY_INTERFACE_MODE_10GKR,
include/linux/phy.h:    case PHY_INTERFACE_MODE_10GKR:

which is as expected.  The only users of "10gbase-kr" in DT are:

arch/arm64/boot/dts/marvell/armada-7040-db.dts: phy-mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts:     phy-mode =
"10gbase-kr";
arch/arm64/boot/dts/marvell/armada-8040-db.dts: phy-mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/armada-8040-db.dts: phy-mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/armada-8040-mcbin-singleshot.dts:   phy-mode =
"10gbase-kr";
arch/arm64/boot/dts/marvell/armada-8040-mcbin-singleshot.dts:   phy-mode =
"10gbase-kr";
arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts:      phy-mode =
"10gbase-kr";arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts:      phy-
mode = "10gbase-kr";arch/arm64/boot/dts/marvell/cn9130-db.dts:      phy-
mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/cn9131-db.dts:      phy-mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/cn9132-db.dts:      phy-mode = "10gbase-kr";

which all use the mvpp2 driver, and these will be updated in a
separate patch to be submitted in the following kernel cycle.

 Documentation/networking/phy.rst                | 18 ++++++++++++++++++
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 13 ++++++++-----
 drivers/net/phy/aquantia_main.c                 |  7 +++++--
 drivers/net/phy/bcm84881.c                      |  4 ++--
 drivers/net/phy/marvell10g.c                    | 11 ++++++-----
 drivers/net/phy/phylink.c                       |  1 +
 drivers/net/phy/sfp-bus.c                       |  2 +-
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c    | 20 ++++++++++----------
 include/linux/phy.h                             | 12 ++++++++----
 9 files changed, 59 insertions(+), 29 deletions(-)

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps
up
According to speedtest.net: 11.9Mbps down 500kbps up

Hi,

we should conclude our discussions on the initial thread before we do this.
The current use on 10GBASE_KR is not correct, I agree but changing this to
10GBASE-R may not be the way to go. We need to determine if the existing
device tree binding corelated type is the one we need to change or maybe a
more complex solution is required. There are multiple paths forward:

Extending this enum that has a mix of things in it that are barely related.

For 10G Ethernet there is already an XGMII entry that describes the MII, if
this should stick to strict MIIs. This is of little value for chips that
have part of the traditional PHY blocks grouped along with the MAC, the XGMII
is not exposed outside the RTL (if it even exists there) and the actual
visible interfaces are completely different.

Describing the actual interface at chip to chip level (RGMII, SGMII, XAUI,
XFI, etc.). This may be incomplete for people trying to configure their HW
that supports multiple modes (reminder - device trees describe HW, they do
not configure SW). More details would be required and the list would be...
eclectic.

Moving to something different altogether, that would not conflict existing
device trees but permit a more thorough classification and less ambiguity.
We need more work on clearing a path towards that.

I think we've reached stalemate in this discussion. I don't think we
can make any useful forward progress at this point.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Describing the actual interface at chip to chip level (RGMII, SGMII, XAUI,
XFI, etc.). This may be incomplete for people trying to configure their HW
that supports multiple modes (reminder - device trees describe HW, they do
not configure SW). More details would be required and the list would be...
eclectic.

Hi Madalin

Please forget the existing DT binding for the moment. Please describe
what values you need to program into your hardware to make it
work. Please don't use the existing DT naming when describing what you
need. Maybe use the terms from the reference manual?

Once we have a list, we can figure out what could be generic, what
could be vendor specific, and how to describe it in ACPI, DT, etc.

At LPC 2019, Claudiu and Vladimir talked about wanting to describe the
eye configuration for some hardware. It would be interesting if there
is any overlap. Aquantia also have some values used to configure the
SERDES of their PHYs. I think this is a board specific binary blob
which is loaded into the PHY as part of the firmware. That then limits
their firmware to a specific board, which is not so nice. But does
that also indicate that how the MAC is configured might also depend on
how the PHY configures its electrical properties?

    Andrew

-----Original Message-----
From: Andrew Lunn <andrew@lunn.ch>
Sent: Friday, January 3, 2020 4:18 PM
To: Madalin Bucur (OSS) <redacted>
Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>; Florian
Fainelli [off-list ref]; Heiner Kallweit [off-list ref];
David S. Miller [off-list ref]; Jonathan Corbet [off-list ref];
Kishon Vijay Abraham I [off-list ref]; linux-doc@vger.kernel.org;
netdev@vger.kernel.org
Subject: Re: [PATCH net-next 0/2] Fix 10G PHY interface types

quoted

Describing the actual interface at chip to chip level (RGMII, SGMII,

XAUI,

quoted

XFI, etc.). This may be incomplete for people trying to configure their

HW

quoted

that supports multiple modes (reminder - device trees describe HW, they

do

quoted

not configure SW). More details would be required and the list would

be...

quoted

eclectic.

Hi Madalin

Please forget the existing DT binding for the moment. Please describe
what values you need to program into your hardware to make it
work. Please don't use the existing DT naming when describing what you
need. Maybe use the terms from the reference manual?

Here is a PHY family document mentioning XFI:

https://www.marvell.com/documents/o67oxbpfhwx806cawedj/

And here is a LS1046A SoC document that describes XFI and 10GBASE_KR:

https://www.mouser.com/pdfdocs/LS1046A.pdf

(see sections 3.10.3 and 3.10.4).

Once we have a list, we can figure out what could be generic, what
could be vendor specific, and how to describe it in ACPI, DT, etc.

At LPC 2019, Claudiu and Vladimir talked about wanting to describe the
eye configuration for some hardware. It would be interesting if there
is any overlap. Aquantia also have some values used to configure the
SERDES of their PHYs. I think this is a board specific binary blob
which is loaded into the PHY as part of the firmware. That then limits
their firmware to a specific board, which is not so nice. But does
that also indicate that how the MAC is configured might also depend on
how the PHY configures its electrical properties?

    Andrew

There are several NXP engineers addressing related issues, there is a vast
networking SoC family to support, with a large set of features, ranging
from the tried and true PPC SoCs to the new and feature rich ARM based SoC
that boast 1/2.5/10/40G Ethernet, backplane, TSN and so on. The PHY area
is often an excursion outside the direct area of responsibility but we need
to do this to get the complete systems working. Often the PHYs we use are
not as well supported upstream as we'd need so we have to get involved.

Regards,
Madalin

And here is a LS1046A SoC document that describes XFI and 10GBASE_KR:

https://www.mouser.com/pdfdocs/LS1046A.pdf

I don't see any register descriptions here. So you failed to answer my
question. What do you actually need to configure? What needs to go
into DT? 

You keep avoiding the questions Russell and I pose, which is not
helping your argument. We need details, not hand waiving.

	Andrew

On Fri, Jan 03, 2020 at 11:52:22AM +0000, Russell King wrote:

Recent discussion has revealed that the use of PHY_INTERFACE_MODE_10GKR
is incorrect. Add a 10GBASE-R definition, document both the -R and -KR
versions, and the fact that 10GKR was used incorrectly.

Signed-off-by: Russell King <redacted>

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

    Andrew

Hi Russell

Maybe consider adding a comment here, or suggest readers to read the
commit message of the patch that added these two lines to get the full
story.

Apart from that:

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

    Andrew

From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Date: Fri, 3 Jan 2020 11:51:25 +0000

Recent discussion has revealed that our current usage of the 10GKR
phy_interface_t is not correct. This is based on a misunderstanding
caused in part by the various specifications being difficult to
obtain. Now that a better understanding has been reached, we ought
to correct this.

This series introduce PHY_INTERFACE_MODE_10GBASER to replace the
existing usage of 10GKR mode, and document their differences in the
phylib documentation. Then switch PHY, SFP/phylink, the Marvell
PP2 network driver, and its associated comphy driver over to use
the correct interface mode. None of the existing platform usage
was actually using 10GBASE-KR.

 ...

Please address Andrew's feedback on patch #2 and I'll apply this.

Thanks.

On Fri, Jan 03, 2020 at 09:25:04PM +0100, Andrew Lunn wrote:

quoted

For Marvell mvpp2, we detect 10GBASE-KR, and rewrite it to 10GBASE-R
for compatibility with existing DT - this is the only network driver
at present that makes use of PHY_INTERFACE_MODE_10GKR.

Signed-off-by: Russell King <redacted>

@@ -5247,6 +5247,9 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 		goto err_free_netdev;
 	}
 
+	if (phy_mode == PHY_INTERFACE_MODE_10GKR)
+		phy_mode = PHY_INTERFACE_MODE_10GBASER;

Hi Russell

Maybe consider adding a comment here, or suggest readers to read the
commit message of the patch that added these two lines to get the full
story.

It's a bit difficult to refer to the commit SHA in the commit itself,
but yes, that's a good idea. I'll send v2 shortly.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

-----Original Message-----
From: Andrew Lunn <andrew@lunn.ch>
Sent: Friday, January 3, 2020 6:48 PM
To: Madalin Bucur (OSS) <redacted>
Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>; Florian
Fainelli [off-list ref]; Heiner Kallweit [off-list ref];
David S. Miller [off-list ref]; Jonathan Corbet [off-list ref];
Kishon Vijay Abraham I [off-list ref]; linux-doc@vger.kernel.org;
netdev@vger.kernel.org
Subject: Re: [PATCH net-next 0/2] Fix 10G PHY interface types

quoted

And here is a LS1046A SoC document that describes XFI and 10GBASE_KR:

https://www.mouser.com/pdfdocs/LS1046A.pdf

You need to register, unfortunately, but the information is in the reference
manual (18.3 MB of download):

https://www.nxp.com/products/processors-and-microcontrollers/arm-processors/layerscape-communication-process/qoriq-layerscape-1046a-and-1026a-multicore-communications-processors:LS1046A?tab=Documentation_Tab

You can find there also an application note on the backplane ethernet
support on this device, with a description of the current solution, describing
the SoC internal PCS (manageable through an MDIO bus) as a PHY to be able
to use the PHYlib infrastructure to control it. The code is still under
refinement, not upstream yet, but it's available and in use by customers.

I don't see any register descriptions here. So you failed to answer my
question. What do you actually need to configure? What needs to go
into DT?

You keep avoiding the questions Russell and I pose, which is not
helping your argument. We need details, not hand waiving.

	Andrew

Answers are there, spreading the conversation on multiple threads and
removing history is not helping.

Madalin