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

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