Skip to content

Commit

Permalink
Merge branch 'Fix-10G-PHY-interface-types'
Browse files Browse the repository at this point in the history
Russell King says:

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

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Jan 5, 2020
2 parents aea6a1e + e0f909b commit 8bd17dc
Show file tree
Hide file tree
Showing 9 changed files with 65 additions and 29 deletions.
18 changes: 18 additions & 0 deletions Documentation/networking/phy.rst
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,24 @@ Some of the interface modes are described below:
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
===========================
Expand Down
19 changes: 14 additions & 5 deletions drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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)) {
Expand Down
7 changes: 5 additions & 2 deletions drivers/net/phy/aquantia_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions drivers/net/phy/bcm84881.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
11 changes: 6 additions & 5 deletions drivers/net/phy/marvell10g.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions drivers/net/phy/phylink.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion drivers/net/phy/sfp-bus.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
20 changes: 10 additions & 10 deletions drivers/phy/marvell/phy-mvebu-cp110-comphy.c
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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),
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
12 changes: 8 additions & 4 deletions include/linux/phy.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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";
}
Expand Down

0 comments on commit 8bd17dc

Please sign in to comment.