Date:   Fri, 3 Jan 2020 12:22:58 +0000
From:   "Madalin Bucur (OSS)" <madalin.bucur@....nxp.com>
To:     Russell King - ARM Linux admin <linux@...linux.org.uk>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>
CC:     "David S. Miller" <davem@...emloft.net>,
        Jonathan Corbet <corbet@....net>,
        Kishon Vijay Abraham I <kishon@...com>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH net-next 0/2] Fix 10G PHY interface types

> -----Original Message-----
> From: netdev-owner@...r.kernel.org <netdev-owner@...r.kernel.org> On
> Behalf Of Russell King - ARM Linux admin
> Sent: Friday, January 3, 2020 1:51 PM
> To: Andrew Lunn <andrew@...n.ch>; Florian Fainelli <f.fainelli@...il.com>;
> Heiner Kallweit <hkallweit1@...il.com>
> Cc: David S. Miller <davem@...emloft.net>; Jonathan Corbet
> <corbet@....net>; Kishon Vijay Abraham I <kishon@...com>; linux-
> doc@...r.kernel.org; netdev@...r.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

Powered by blists - more mailing lists