Fix 10G PHY interface types
Subsystems (9)
Russell King - ARM Linux admin
maintainerv2 cover 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.
v2: add comment to mvpp2 driver.
Documentation/networking/phy.rst | 18 ++++++++++++++++++ drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 19 ++++++++++++++----- 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, 65 insertions(+), 29 deletions(-)
Signature · 3 lines
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
PATCH v2 1/2 net: phy: add PHY_INTERFACE_MODE_10GBASER
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.
--- Documentation/networking/phy.rst | 18 ++++++++++++++++++ include/linux/phy.h | 12 ++++++++---- 2 files changed, 26 insertions(+), 4 deletions(-)
Documentation/networking/phy.rst +18 -0
@@ -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 defines 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 ===========================
include/linux/phy.h +8 -4
@@ -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" ; }
Signature · 1 line
2.20.1
PATCH v2 2/2 net: switch to using PHY_INTERFACE_MODE_10GBASER rather than 10GKR
In reply to Russell King - ARM Linux adminSwitch 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.
--- .../net/ethernet/marvell/mvpp2/mvpp2_main.c | 19 +++++++++++++----- 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, 39 insertions(+), 25 deletions(-)
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +14 -5
@@ -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,15 @@ static int mvpp2_port_probe(struct platform_device *pdev, goto err_free_netdev; } +/* +* Rewrite 10GBASE-KR to 10GBASE-R for compatibility with existing DT. +* Existing usage of 10GBASE-KR is not correct; no backplane +* negotiation is done, and this driver does not actually support +* 10GBASE-KR. +*/ +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)) {
drivers/net/phy/aquantia_main.c +5 -2
@@ -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,
drivers/net/phy/bcm84881.c +2 -2
@@ -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 ) {
drivers/net/phy/marvell10g.c +6 -5
@@ -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 ;
drivers/net/phy/phylink.c +1 -0
@@ -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);
drivers/net/phy/sfp-bus.c +1 -1
@@ -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 ;
drivers/phy/marvell/phy-mvebu-cp110-comphy.c +10 -10
@@ -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 ;
Signature · 1 line
2.20.1
Re: [PATCH net-next v2 2/2] net: switch to using PHY_INTERFACE_MODE_10GBASER rather than 10GKR
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c static int mvpp2_port_probe(struct platform_device *pdev, · line 5258
Quoted 9 lines
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +9
-0+/* +* Rewrite 10GBASE-KR to 10GBASE-R for compatibility with existing DT. +* Existing usage of 10GBASE-KR is not correct; no backplane +* negotiation is done, and this driver does not actually support +* 10GBASE-KR. +*/ +if (phy_mode ==PHY_INTERFACE_MODE_10GKR ) + phy_mode =PHY_INTERFACE_MODE_10GBASER ; +
Thanks
Andrew
David Miller
maintainerFrom: Russell King - ARM Linux admin <[email protected]>
Date: Fri, 3 Jan 2020 20:42:42 +0000
Quoted 6 lines
Recent discussion has revealed that our current usage of the 10GKR
phy_interface_tis 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.
Quoted 6 lines
This series introduce
PHY_INTERFACE_MODE_10GBASERto 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.
...
Series applied, thank you.