linux-doc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] Fix 10G PHY interface types
@ 2020-01-03 11:51 Russell King - ARM Linux admin
  2020-01-03 11:52 ` [PATCH net-next 1/2] net: phy: add PHY_INTERFACE_MODE_10GBASER Russell King
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Russell King - ARM Linux admin @ 2020-01-03 11:51 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Heiner Kallweit
  Cc: David S. Miller, Jonathan Corbet, Kishon Vijay Abraham I,
	linux-doc, netdev

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH net-next 1/2] net: phy: add PHY_INTERFACE_MODE_10GBASER
  2020-01-03 11:51 [PATCH net-next 0/2] Fix 10G PHY interface types Russell King - ARM Linux admin
@ 2020-01-03 11:52 ` Russell King
  2020-01-03 20:22   ` Andrew Lunn
  2020-01-03 11:52 ` [PATCH net-next 2/2] net: switch to using PHY_INTERFACE_MODE_10GBASER rather than 10GKR Russell King
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Russell King @ 2020-01-03 11:52 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Heiner Kallweit
  Cc: David S. Miller, Jonathan Corbet, Kishon Vijay Abraham I,
	linux-doc, netdev

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 <rmk+kernel@armlinux.org.uk>
---
 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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net-next 2/2] net: switch to using PHY_INTERFACE_MODE_10GBASER rather than 10GKR
  2020-01-03 11:51 [PATCH net-next 0/2] Fix 10G PHY interface types Russell King - ARM Linux admin
  2020-01-03 11:52 ` [PATCH net-next 1/2] net: phy: add PHY_INTERFACE_MODE_10GBASER Russell King
@ 2020-01-03 11:52 ` Russell King
  2020-01-03 20:25   ` Andrew Lunn
  2020-01-03 12:22 ` [PATCH net-next 0/2] Fix 10G PHY interface types Madalin Bucur (OSS)
  2020-01-03 20:29 ` David Miller
  3 siblings, 1 reply; 13+ messages in thread
From: Russell King @ 2020-01-03 11:52 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Heiner Kallweit
  Cc: David S. Miller, Jonathan Corbet, Kishon Vijay Abraham I,
	linux-doc, netdev

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 <rmk+kernel@armlinux.org.uk>
---
 .../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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* RE: [PATCH net-next 0/2] Fix 10G PHY interface types
  2020-01-03 11:51 [PATCH net-next 0/2] Fix 10G PHY interface types Russell King - ARM Linux admin
  2020-01-03 11:52 ` [PATCH net-next 1/2] net: phy: add PHY_INTERFACE_MODE_10GBASER Russell King
  2020-01-03 11:52 ` [PATCH net-next 2/2] net: switch to using PHY_INTERFACE_MODE_10GBASER rather than 10GKR Russell King
@ 2020-01-03 12:22 ` Madalin Bucur (OSS)
  2020-01-03 12:57   ` Russell King - ARM Linux admin
  2020-01-03 14:17   ` Andrew Lunn
  2020-01-03 20:29 ` David Miller
  3 siblings, 2 replies; 13+ messages in thread
From: Madalin Bucur (OSS) @ 2020-01-03 12:22 UTC (permalink / raw)
  To: Russell King - ARM Linux admin, Andrew Lunn, Florian Fainelli,
	Heiner Kallweit
  Cc: David S. Miller, Jonathan Corbet, Kishon Vijay Abraham I,
	linux-doc, netdev

> -----Original Message-----
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> 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 <hkallweit1@gmail.com>
> Cc: David S. Miller <davem@davemloft.net>; Jonathan Corbet
> <corbet@lwn.net>; Kishon Vijay Abraham I <kishon@ti.com>; 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 0/2] Fix 10G PHY interface types
  2020-01-03 12:22 ` [PATCH net-next 0/2] Fix 10G PHY interface types Madalin Bucur (OSS)
@ 2020-01-03 12:57   ` Russell King - ARM Linux admin
  2020-01-03 14:17   ` Andrew Lunn
  1 sibling, 0 replies; 13+ messages in thread
From: Russell King - ARM Linux admin @ 2020-01-03 12:57 UTC (permalink / raw)
  To: Madalin Bucur (OSS)
  Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller,
	Jonathan Corbet, Kishon Vijay Abraham I, linux-doc, netdev

On Fri, Jan 03, 2020 at 12:22:58PM +0000, Madalin Bucur (OSS) wrote:
> > -----Original Message-----
> > From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> 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 <hkallweit1@gmail.com>
> > Cc: David S. Miller <davem@davemloft.net>; Jonathan Corbet
> > <corbet@lwn.net>; Kishon Vijay Abraham I <kishon@ti.com>; 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 0/2] Fix 10G PHY interface types
  2020-01-03 12:22 ` [PATCH net-next 0/2] Fix 10G PHY interface types Madalin Bucur (OSS)
  2020-01-03 12:57   ` Russell King - ARM Linux admin
@ 2020-01-03 14:17   ` Andrew Lunn
  2020-01-03 16:33     ` Madalin Bucur (OSS)
  1 sibling, 1 reply; 13+ messages in thread
From: Andrew Lunn @ 2020-01-03 14:17 UTC (permalink / raw)
  To: Madalin Bucur (OSS)
  Cc: Russell King - ARM Linux admin, Florian Fainelli,
	Heiner Kallweit, David S. Miller, Jonathan Corbet,
	Kishon Vijay Abraham I, linux-doc, netdev

> 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH net-next 0/2] Fix 10G PHY interface types
  2020-01-03 14:17   ` Andrew Lunn
@ 2020-01-03 16:33     ` Madalin Bucur (OSS)
  2020-01-03 16:47       ` Andrew Lunn
  0 siblings, 1 reply; 13+ messages in thread
From: Madalin Bucur (OSS) @ 2020-01-03 16:33 UTC (permalink / raw)
  To: Andrew Lunn, Madalin Bucur (OSS)
  Cc: Russell King - ARM Linux admin, Florian Fainelli,
	Heiner Kallweit, David S. Miller, Jonathan Corbet,
	Kishon Vijay Abraham I, linux-doc, netdev

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Friday, January 3, 2020 4:18 PM
> To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>; Florian
> Fainelli <f.fainelli@gmail.com>; Heiner Kallweit <hkallweit1@gmail.com>;
> David S. Miller <davem@davemloft.net>; Jonathan Corbet <corbet@lwn.net>;
> Kishon Vijay Abraham I <kishon@ti.com>; linux-doc@vger.kernel.org;
> netdev@vger.kernel.org
> Subject: Re: [PATCH net-next 0/2] Fix 10G PHY interface types
> 
> > 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?

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 0/2] Fix 10G PHY interface types
  2020-01-03 16:33     ` Madalin Bucur (OSS)
@ 2020-01-03 16:47       ` Andrew Lunn
  2020-01-06  9:16         ` Madalin Bucur (OSS)
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Lunn @ 2020-01-03 16:47 UTC (permalink / raw)
  To: Madalin Bucur (OSS)
  Cc: Russell King - ARM Linux admin, Florian Fainelli,
	Heiner Kallweit, David S. Miller, Jonathan Corbet,
	Kishon Vijay Abraham I, linux-doc, netdev

> 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 1/2] net: phy: add PHY_INTERFACE_MODE_10GBASER
  2020-01-03 11:52 ` [PATCH net-next 1/2] net: phy: add PHY_INTERFACE_MODE_10GBASER Russell King
@ 2020-01-03 20:22   ` Andrew Lunn
  0 siblings, 0 replies; 13+ messages in thread
From: Andrew Lunn @ 2020-01-03 20:22 UTC (permalink / raw)
  To: Russell King
  Cc: Florian Fainelli, Heiner Kallweit, David S. Miller,
	Jonathan Corbet, Kishon Vijay Abraham I, linux-doc, netdev

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 <rmk+kernel@armlinux.org.uk>

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

    Andrew

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 2/2] net: switch to using PHY_INTERFACE_MODE_10GBASER rather than 10GKR
  2020-01-03 11:52 ` [PATCH net-next 2/2] net: switch to using PHY_INTERFACE_MODE_10GBASER rather than 10GKR Russell King
@ 2020-01-03 20:25   ` Andrew Lunn
  2020-01-03 20:41     ` Russell King - ARM Linux admin
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Lunn @ 2020-01-03 20:25 UTC (permalink / raw)
  To: Russell King
  Cc: Florian Fainelli, Heiner Kallweit, David S. Miller,
	Jonathan Corbet, Kishon Vijay Abraham I, linux-doc, netdev

> 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 <rmk+kernel@armlinux.org.uk>
> @@ -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.

Apart from that:

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

    Andrew

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 0/2] Fix 10G PHY interface types
  2020-01-03 11:51 [PATCH net-next 0/2] Fix 10G PHY interface types Russell King - ARM Linux admin
                   ` (2 preceding siblings ...)
  2020-01-03 12:22 ` [PATCH net-next 0/2] Fix 10G PHY interface types Madalin Bucur (OSS)
@ 2020-01-03 20:29 ` David Miller
  3 siblings, 0 replies; 13+ messages in thread
From: David Miller @ 2020-01-03 20:29 UTC (permalink / raw)
  To: linux; +Cc: andrew, f.fainelli, hkallweit1, corbet, kishon, linux-doc, netdev

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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next 2/2] net: switch to using PHY_INTERFACE_MODE_10GBASER rather than 10GKR
  2020-01-03 20:25   ` Andrew Lunn
@ 2020-01-03 20:41     ` Russell King - ARM Linux admin
  0 siblings, 0 replies; 13+ messages in thread
From: Russell King - ARM Linux admin @ 2020-01-03 20:41 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Florian Fainelli, Heiner Kallweit, David S. Miller,
	Jonathan Corbet, Kishon Vijay Abraham I, linux-doc, netdev

On Fri, Jan 03, 2020 at 09:25:04PM +0100, Andrew Lunn wrote:
> > 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 <rmk+kernel@armlinux.org.uk>
> > @@ -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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH net-next 0/2] Fix 10G PHY interface types
  2020-01-03 16:47       ` Andrew Lunn
@ 2020-01-06  9:16         ` Madalin Bucur (OSS)
  0 siblings, 0 replies; 13+ messages in thread
From: Madalin Bucur (OSS) @ 2020-01-06  9:16 UTC (permalink / raw)
  To: Andrew Lunn, Madalin Bucur (OSS)
  Cc: Russell King - ARM Linux admin, Florian Fainelli,
	Heiner Kallweit, David S. Miller, Jonathan Corbet,
	Kishon Vijay Abraham I, linux-doc, netdev

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Friday, January 3, 2020 6:48 PM
> To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>; Florian
> Fainelli <f.fainelli@gmail.com>; Heiner Kallweit <hkallweit1@gmail.com>;
> David S. Miller <davem@davemloft.net>; Jonathan Corbet <corbet@lwn.net>;
> Kishon Vijay Abraham I <kishon@ti.com>; linux-doc@vger.kernel.org;
> netdev@vger.kernel.org
> Subject: Re: [PATCH net-next 0/2] Fix 10G PHY interface types
> 
> > 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-01-06  9:16 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-03 11:51 [PATCH net-next 0/2] Fix 10G PHY interface types Russell King - ARM Linux admin
2020-01-03 11:52 ` [PATCH net-next 1/2] net: phy: add PHY_INTERFACE_MODE_10GBASER Russell King
2020-01-03 20:22   ` Andrew Lunn
2020-01-03 11:52 ` [PATCH net-next 2/2] net: switch to using PHY_INTERFACE_MODE_10GBASER rather than 10GKR Russell King
2020-01-03 20:25   ` Andrew Lunn
2020-01-03 20:41     ` Russell King - ARM Linux admin
2020-01-03 12:22 ` [PATCH net-next 0/2] Fix 10G PHY interface types Madalin Bucur (OSS)
2020-01-03 12:57   ` Russell King - ARM Linux admin
2020-01-03 14:17   ` Andrew Lunn
2020-01-03 16:33     ` Madalin Bucur (OSS)
2020-01-03 16:47       ` Andrew Lunn
2020-01-06  9:16         ` Madalin Bucur (OSS)
2020-01-03 20:29 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).