Commit 8bd17dc6 authored by David S. Miller's avatar David S. Miller
Browse files

Merge branch 'Fix-10G-PHY-interface-types'



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: default avatarDavid S. Miller <davem@davemloft.net>
parents aea6a1eb e0f909bc
Loading
Loading
Loading
Loading
+18 −0
Original line number Diff line number Diff line
@@ -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
===========================
+14 −5
Original line number Diff line number Diff line
@@ -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)) {
+5 −2
Original line number Diff line number Diff line
@@ -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,
+2 −2
Original line number Diff line number Diff line
@@ -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) {
+6 −5
Original line number Diff line number Diff line
@@ -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;
Loading