[PATCH 0/6] Add PHY connection types for XFI and SFI

[PATCH 0/6] Add PHY connection types for XFI and SFI

[Madalin Bucur]

For a long time the only 10G interface mode registered in the code was
XGMII and some PHYs and MACs that actually supported other modes just
used that to denote 10G. Recently more modes were added (USXGMII,
10GKR) to better match the actual HW support. In this respect, the use
of 10GBase-KR to denote not only backplane but also XFI and SFI can be
improved upon. This patch series introduces XFI and SFI PHY connection
types and initial users for them.

Madalin Bucur (6):
  net: phy: add interface modes for XFI, SFI
  arm64: dts: ls104xardb: set correct PHY interface mode
  net: fsl/fman: rename IF_MODE_XGMII to IF_MODE_10G
  net: fsl/fman: add support for PHY_INTERFACE_MODE_XFI
  net: fsl/fman: add support for PHY_INTERFACE_MODE_SFI
  net: phy: aquantia: add support for PHY_INTERFACE_MODE_XFI

 arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts |  2 +-
 arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts |  4 ++--
 drivers/net/ethernet/freescale/fman/fman_memac.c  | 10 +++++++---
 drivers/net/ethernet/freescale/fman/mac.c         | 10 +++++++---
 drivers/net/phy/aquantia_main.c                   |  5 ++++-
 include/linux/phy.h                               |  7 ++++++-
 6 files changed, 27 insertions(+), 11 deletions(-)

-- 
2.1.0

[PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Madalin Bucur]

From: Madalin Bucur <madalin.bucur@nxp.com>

Add explicit entries for XFI, SFI to make sure the device
tree entries for phy-connection-type "xfi" or "sfi" are
properly parsed and differentiated against the existing
backplane 10GBASE-KR mode.

Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
---
 include/linux/phy.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index 5032d453ac66..ebb793621f0b 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -99,7 +99,8 @@ 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_XFI,
+	PHY_INTERFACE_MODE_SFI,
 	PHY_INTERFACE_MODE_10GKR,
 	PHY_INTERFACE_MODE_USXGMII,
 	PHY_INTERFACE_MODE_MAX,
@@ -175,6 +176,10 @@ static inline const char *phy_modes(phy_interface_t interface)
 		return "rxaui";
 	case PHY_INTERFACE_MODE_XAUI:
 		return "xaui";
+	case PHY_INTERFACE_MODE_XFI:
+		return "xfi";
+	case PHY_INTERFACE_MODE_SFI:
+		return "sfi";
 	case PHY_INTERFACE_MODE_10GKR:
 		return "10gbase-kr";
 	case PHY_INTERFACE_MODE_USXGMII:
-- 
2.1.0

Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Russell King - ARM Linux admin]

On Thu, Dec 19, 2019 at 05:21:16PM +0200, Madalin Bucur wrote:
> From: Madalin Bucur <madalin.bucur@nxp.com>
> 
> Add explicit entries for XFI, SFI to make sure the device
> tree entries for phy-connection-type "xfi" or "sfi" are
> properly parsed and differentiated against the existing
> backplane 10GBASE-KR mode.

10GBASE-KR is actually used for XFI and SFI (due to a slight mistake on
my part, it should've been just 10GBASE-R).

Please explain exactly what the difference is between XFI, SFI and
10GBASE-R. I have not been able to find definitive definitions for
XFI and SFI anywhere, and they appear to be precisely identical to
10GBASE-R. It seems that it's just a terminology thing, with
different groups wanting to "own" what is essentially exactly the
same interface type.

> 
> Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
> ---
>  include/linux/phy.h | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 5032d453ac66..ebb793621f0b 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -99,7 +99,8 @@ 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_XFI,
> +	PHY_INTERFACE_MODE_SFI,
>  	PHY_INTERFACE_MODE_10GKR,
>  	PHY_INTERFACE_MODE_USXGMII,
>  	PHY_INTERFACE_MODE_MAX,
> @@ -175,6 +176,10 @@ static inline const char *phy_modes(phy_interface_t interface)
>  		return "rxaui";
>  	case PHY_INTERFACE_MODE_XAUI:
>  		return "xaui";
> +	case PHY_INTERFACE_MODE_XFI:
> +		return "xfi";
> +	case PHY_INTERFACE_MODE_SFI:
> +		return "sfi";
>  	case PHY_INTERFACE_MODE_10GKR:
>  		return "10gbase-kr";
>  	case PHY_INTERFACE_MODE_USXGMII:
> -- 
> 2.1.0
> 
> 

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

RE: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Madalin Bucur]

> -----Original Message-----
> From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Sent: Thursday, December 19, 2019 7:29 PM
> To: Madalin Bucur <madalin.bucur@nxp.com>
> Cc: davem@davemloft.net; netdev@vger.kernel.org; andrew@lunn.ch;
> f.fainelli@gmail.com; hkallweit1@gmail.com; shawnguo@kernel.org;
> devicetree@vger.kernel.org
> Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> 
> On Thu, Dec 19, 2019 at 05:21:16PM +0200, Madalin Bucur wrote:
> > From: Madalin Bucur <madalin.bucur@nxp.com>
> >
> > Add explicit entries for XFI, SFI to make sure the device
> > tree entries for phy-connection-type "xfi" or "sfi" are
> > properly parsed and differentiated against the existing
> > backplane 10GBASE-KR mode.
> 
> 10GBASE-KR is actually used for XFI and SFI (due to a slight mistake on
> my part, it should've been just 10GBASE-R).
> 
> Please explain exactly what the difference is between XFI, SFI and
> 10GBASE-R. I have not been able to find definitive definitions for
> XFI and SFI anywhere, and they appear to be precisely identical to
> 10GBASE-R. It seems that it's just a terminology thing, with
> different groups wanting to "own" what is essentially exactly the
> same interface type.

Hi Russell,

10GBase-R could be used as a common nominator but just as well 10G and
remove the rest while we're at it. There are/may be differences in
features, differences in the way the HW is configured (the most
important aspect) and one should be able to determine what interface
type is in use to properly configure the HW. SFI does not have the CDR
function in the PMD, relying on the PMA signal conditioning vs the XFI
that requires this in the PMD. We kept the xgmii compatible for so long
without much issues until someone started cleaning up the PHY supported
modes. Since we're doing that, let's be rigorous. The 10GBase-KR is
important too, we have some backplane code in preparation and having it
there could pave the way for a simpler integration.

Thank you,
Madalin

Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Russell King - ARM Linux admin]

On Thu, Dec 19, 2019 at 06:32:51PM +0000, Madalin Bucur wrote:
> > -----Original Message-----
> > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > Sent: Thursday, December 19, 2019 7:29 PM
> > To: Madalin Bucur <madalin.bucur@nxp.com>
> > Cc: davem@davemloft.net; netdev@vger.kernel.org; andrew@lunn.ch;
> > f.fainelli@gmail.com; hkallweit1@gmail.com; shawnguo@kernel.org;
> > devicetree@vger.kernel.org
> > Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> > 
> > On Thu, Dec 19, 2019 at 05:21:16PM +0200, Madalin Bucur wrote:
> > > From: Madalin Bucur <madalin.bucur@nxp.com>
> > >
> > > Add explicit entries for XFI, SFI to make sure the device
> > > tree entries for phy-connection-type "xfi" or "sfi" are
> > > properly parsed and differentiated against the existing
> > > backplane 10GBASE-KR mode.
> > 
> > 10GBASE-KR is actually used for XFI and SFI (due to a slight mistake on
> > my part, it should've been just 10GBASE-R).
> > 
> > Please explain exactly what the difference is between XFI, SFI and
> > 10GBASE-R. I have not been able to find definitive definitions for
> > XFI and SFI anywhere, and they appear to be precisely identical to
> > 10GBASE-R. It seems that it's just a terminology thing, with
> > different groups wanting to "own" what is essentially exactly the
> > same interface type.
> 
> Hi Russell,
> 
> 10GBase-R could be used as a common nominator but just as well 10G and
> remove the rest while we're at it. There are/may be differences in
> features, differences in the way the HW is configured (the most
> important aspect) and one should be able to determine what interface
> type is in use to properly configure the HW. SFI does not have the CDR
> function in the PMD, relying on the PMA signal conditioning vs the XFI
> that requires this in the PMD. We kept the xgmii compatible for so long
> without much issues until someone started cleaning up the PHY supported
> modes. Since we're doing that, let's be rigorous. The 10GBase-KR is
> important too, we have some backplane code in preparation and having it
> there could pave the way for a simpler integration.

The problem we currently have is:

$ grep '10gbase-kr' arch/*/boot/dts -r

virtually none of those are actually backplane. For the mcbin matches,
these are either to a 88x3310 PHY for the doubleshot, which dynamically
operates between XFI, 5GBASE-R, 2500BASE-X, or SGMII according to the
datasheet.

If we add something else, then the problem becomes what to do about
that lot - one of the problems is, it seems we're going to be breaking
DT compatibility by redefining 10gbase-kr to be correct.

It's interesting to hear what the difference is between XFI and SFI,
but it's weird that PHYs such as 88x3310 have no configuration of their
fiber interface to enable or disable the CDR, yet it supports fiber
interfaces, and explicitly shows applications involving "XFI/SFI".
There's no mention of the CDR in the datasheet either.

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

RE: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Madalin Bucur (OSS)]

> -----Original Message-----
> From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Sent: Thursday, December 19, 2019 9:03 PM
> To: Madalin Bucur <madalin.bucur@nxp.com>
> Cc: davem@davemloft.net; netdev@vger.kernel.org; andrew@lunn.ch;
> f.fainelli@gmail.com; hkallweit1@gmail.com; shawnguo@kernel.org;
> devicetree@vger.kernel.org
> Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> 
> On Thu, Dec 19, 2019 at 06:32:51PM +0000, Madalin Bucur wrote:
> > > -----Original Message-----
> > > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > > Sent: Thursday, December 19, 2019 7:29 PM
> > > To: Madalin Bucur <madalin.bucur@nxp.com>
> > > Cc: davem@davemloft.net; netdev@vger.kernel.org; andrew@lunn.ch;
> > > f.fainelli@gmail.com; hkallweit1@gmail.com; shawnguo@kernel.org;
> > > devicetree@vger.kernel.org
> > > Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> > >
> > > On Thu, Dec 19, 2019 at 05:21:16PM +0200, Madalin Bucur wrote:
> > > > From: Madalin Bucur <madalin.bucur@nxp.com>
> > > >
> > > > Add explicit entries for XFI, SFI to make sure the device
> > > > tree entries for phy-connection-type "xfi" or "sfi" are
> > > > properly parsed and differentiated against the existing
> > > > backplane 10GBASE-KR mode.
> > >
> > > 10GBASE-KR is actually used for XFI and SFI (due to a slight mistake
> on
> > > my part, it should've been just 10GBASE-R).
> > >
> > > Please explain exactly what the difference is between XFI, SFI and
> > > 10GBASE-R. I have not been able to find definitive definitions for
> > > XFI and SFI anywhere, and they appear to be precisely identical to
> > > 10GBASE-R. It seems that it's just a terminology thing, with
> > > different groups wanting to "own" what is essentially exactly the
> > > same interface type.
> >
> > Hi Russell,
> >
> > 10GBase-R could be used as a common nominator but just as well 10G and
> > remove the rest while we're at it. There are/may be differences in
> > features, differences in the way the HW is configured (the most
> > important aspect) and one should be able to determine what interface
> > type is in use to properly configure the HW. SFI does not have the CDR
> > function in the PMD, relying on the PMA signal conditioning vs the XFI
> > that requires this in the PMD. We kept the xgmii compatible for so long
> > without much issues until someone started cleaning up the PHY supported
> > modes. Since we're doing that, let's be rigorous. The 10GBase-KR is
> > important too, we have some backplane code in preparation and having it
> > there could pave the way for a simpler integration.
> 
> The problem we currently have is:
> 
> $ grep '10gbase-kr' arch/*/boot/dts -r
> 
> virtually none of those are actually backplane. For the mcbin matches,
> these are either to a 88x3310 PHY for the doubleshot, which dynamically
> operates between XFI, 5GBASE-R, 2500BASE-X, or SGMII according to the
> datasheet.

Yes, I've seen it's used already in several places:

$ grep PHY_INTERFACE_MODE_10GKR drivers/net -nr
drivers/net/phy/marvell10g.c:219:       if (iface != PHY_INTERFACE_MODE_10GKR) {
drivers/net/phy/marvell10g.c:307:           phydev->interface != PHY_INTERFACE_MODE_10GKR)
drivers/net/phy/marvell10g.c:389:            phydev->interface == PHY_INTERFACE_MODE_10GKR) && phydev->link) {
drivers/net/phy/marvell10g.c:398:                       phydev->interface = PHY_INTERFACE_MODE_10GKR;
drivers/net/phy/phylink.c:296:          case PHY_INTERFACE_MODE_10GKR:
drivers/net/phy/aquantia_main.c:361:            phydev->interface = PHY_INTERFACE_MODE_10GKR;
drivers/net/phy/aquantia_main.c:499:        phydev->interface != PHY_INTERFACE_MODE_10GKR)
drivers/net/phy/sfp-bus.c:340:          return PHY_INTERFACE_MODE_10GKR;
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:1117:   return interface == PHY_INTERFACE_MODE_10GKR ||
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:1203:   case PHY_INTERFACE_MODE_10GKR:
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:1652:   case PHY_INTERFACE_MODE_10GKR:
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:4761:   case PHY_INTERFACE_MODE_10GKR:
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:4783:   case PHY_INTERFACE_MODE_10GKR:

We should fix this, if it's incorrect.

> If we add something else, then the problem becomes what to do about
> that lot - one of the problems is, it seems we're going to be breaking
> DT compatibility by redefining 10gbase-kr to be correct.

We need the committer/maintainer to update that to a correct value.
 
> It's interesting to hear what the difference is between XFI and SFI,
> but it's weird that PHYs such as 88x3310 have no configuration of their
> fiber interface to enable or disable the CDR, yet it supports fiber
> interfaces, and explicitly shows applications involving "XFI/SFI".
> There's no mention of the CDR in the datasheet either.

I understand SFI came later, with the advantage of cheaper and less power
consuming SFP+ modules.

Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Russell King - ARM Linux admin]

On Thu, Dec 19, 2019 at 09:34:57PM +0000, Madalin Bucur (OSS) wrote:
> > -----Original Message-----
> > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > Sent: Thursday, December 19, 2019 9:03 PM
> > To: Madalin Bucur <madalin.bucur@nxp.com>
> > Cc: davem@davemloft.net; netdev@vger.kernel.org; andrew@lunn.ch;
> > f.fainelli@gmail.com; hkallweit1@gmail.com; shawnguo@kernel.org;
> > devicetree@vger.kernel.org
> > Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> > 
> > On Thu, Dec 19, 2019 at 06:32:51PM +0000, Madalin Bucur wrote:
> > > > -----Original Message-----
> > > > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > > > Sent: Thursday, December 19, 2019 7:29 PM
> > > > To: Madalin Bucur <madalin.bucur@nxp.com>
> > > > Cc: davem@davemloft.net; netdev@vger.kernel.org; andrew@lunn.ch;
> > > > f.fainelli@gmail.com; hkallweit1@gmail.com; shawnguo@kernel.org;
> > > > devicetree@vger.kernel.org
> > > > Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> > > >
> > > > On Thu, Dec 19, 2019 at 05:21:16PM +0200, Madalin Bucur wrote:
> > > > > From: Madalin Bucur <madalin.bucur@nxp.com>
> > > > >
> > > > > Add explicit entries for XFI, SFI to make sure the device
> > > > > tree entries for phy-connection-type "xfi" or "sfi" are
> > > > > properly parsed and differentiated against the existing
> > > > > backplane 10GBASE-KR mode.
> > > >
> > > > 10GBASE-KR is actually used for XFI and SFI (due to a slight mistake
> > on
> > > > my part, it should've been just 10GBASE-R).
> > > >
> > > > Please explain exactly what the difference is between XFI, SFI and
> > > > 10GBASE-R. I have not been able to find definitive definitions for
> > > > XFI and SFI anywhere, and they appear to be precisely identical to
> > > > 10GBASE-R. It seems that it's just a terminology thing, with
> > > > different groups wanting to "own" what is essentially exactly the
> > > > same interface type.
> > >
> > > Hi Russell,
> > >
> > > 10GBase-R could be used as a common nominator but just as well 10G and
> > > remove the rest while we're at it. There are/may be differences in
> > > features, differences in the way the HW is configured (the most
> > > important aspect) and one should be able to determine what interface
> > > type is in use to properly configure the HW. SFI does not have the CDR
> > > function in the PMD, relying on the PMA signal conditioning vs the XFI
> > > that requires this in the PMD. We kept the xgmii compatible for so long
> > > without much issues until someone started cleaning up the PHY supported
> > > modes. Since we're doing that, let's be rigorous. The 10GBase-KR is
> > > important too, we have some backplane code in preparation and having it
> > > there could pave the way for a simpler integration.
> > 
> > The problem we currently have is:
> > 
> > $ grep '10gbase-kr' arch/*/boot/dts -r
> > 
> > virtually none of those are actually backplane. For the mcbin matches,
> > these are either to a 88x3310 PHY for the doubleshot, which dynamically
> > operates between XFI, 5GBASE-R, 2500BASE-X, or SGMII according to the
> > datasheet.
> 
> Yes, I've seen it's used already in several places:
> 
> $ grep PHY_INTERFACE_MODE_10GKR drivers/net -nr
> drivers/net/phy/marvell10g.c:219:       if (iface != PHY_INTERFACE_MODE_10GKR) {
> drivers/net/phy/marvell10g.c:307:           phydev->interface != PHY_INTERFACE_MODE_10GKR)
> drivers/net/phy/marvell10g.c:389:            phydev->interface == PHY_INTERFACE_MODE_10GKR) && phydev->link) {
> drivers/net/phy/marvell10g.c:398:                       phydev->interface = PHY_INTERFACE_MODE_10GKR;
> drivers/net/phy/phylink.c:296:          case PHY_INTERFACE_MODE_10GKR:
> drivers/net/phy/aquantia_main.c:361:            phydev->interface = PHY_INTERFACE_MODE_10GKR;
> drivers/net/phy/aquantia_main.c:499:        phydev->interface != PHY_INTERFACE_MODE_10GKR)
> drivers/net/phy/sfp-bus.c:340:          return PHY_INTERFACE_MODE_10GKR;
> drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:1117:   return interface == PHY_INTERFACE_MODE_10GKR ||
> drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:1203:   case PHY_INTERFACE_MODE_10GKR:
> drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:1652:   case PHY_INTERFACE_MODE_10GKR:
> drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:4761:   case PHY_INTERFACE_MODE_10GKR:
> drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:4783:   case PHY_INTERFACE_MODE_10GKR:
> 
> We should fix this, if it's incorrect.
> 
> > If we add something else, then the problem becomes what to do about
> > that lot - one of the problems is, it seems we're going to be breaking
> > DT compatibility by redefining 10gbase-kr to be correct.
> 
> We need the committer/maintainer to update that to a correct value.

The general principle is, we don't break existing DT - in that, we
expect DT files from current kernels to work with future kernels. So,
we're kind of stuck with "10gbase-kr" being used for this at least in
the medium term.

By all means introduce "xfi" and "sfi" if you think that there is a
need to discriminate between the two, but I've seen no hardware which
that treats them any differently from 10gbase-r.

If we want to support real 10gbase-kr, then I think we need to consider
how to do that without affecting compatibility with what we already
have.

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

RE: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Madalin Bucur (OSS)]

> -----Original Message-----
> From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> On Thu, Dec 19, 2019 at 09:34:57PM +0000, Madalin Bucur (OSS) wrote:
> > > -----Original Message-----
> > > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > > On Thu, Dec 19, 2019 at 06:32:51PM +0000, Madalin Bucur wrote:
> > > > > -----Original Message-----
> > > > > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > > > >
> > > > > On Thu, Dec 19, 2019 at 05:21:16PM +0200, Madalin Bucur wrote:
> > > > > > From: Madalin Bucur <madalin.bucur@nxp.com>
> > > > > >
> > > > > > Add explicit entries for XFI, SFI to make sure the device
> > > > > > tree entries for phy-connection-type "xfi" or "sfi" are
> > > > > > properly parsed and differentiated against the existing
> > > > > > backplane 10GBASE-KR mode.
> > > > >
> > > > > 10GBASE-KR is actually used for XFI and SFI (due to a slight
> > > > > mistake on my part, it should've been just 10GBASE-R).
> > > > >
> > > > > Please explain exactly what the difference is between XFI, SFI
> > > > > and 10GBASE-R. I have not been able to find definitive definitions
> > > > > for XFI and SFI anywhere, and they appear to be precisely identical
> > > > > to 10GBASE-R. It seems that it's just a terminology thing, with
> > > > > different groups wanting to "own" what is essentially exactly the
> > > > > same interface type.
> > > >
> > > > Hi Russell,
> > > >
> > > > 10GBase-R could be used as a common nominator but just as well 10G
> > > > and remove the rest while we're at it. There are/may be differences in
> > > > features, differences in the way the HW is configured (the most
> > > > important aspect) and one should be able to determine what interface
> > > > type is in use to properly configure the HW. SFI does not have the
> > > > CDR function in the PMD, relying on the PMA signal conditioning vs the
> > > > XFI that requires this in the PMD. We kept the xgmii compatible for so
> > > > long without much issues until someone started cleaning up the PHY
> > > > supported modes. Since we're doing that, let's be rigorous. The 10GBase-KR
> > > > is important too, we have some backplane code in preparation and
> > > > having it there could pave the way for a simpler integration.
> > >
> > > The problem we currently have is:
> > >
> > > $ grep '10gbase-kr' arch/*/boot/dts -r
> > >
> > > virtually none of those are actually backplane. For the mcbin
> > > matches, these are either to a 88x3310 PHY for the doubleshot, which
> > > dynamically operates between XFI, 5GBASE-R, 2500BASE-X, or SGMII according
> > > to the datasheet.
> >
> > Yes, I've seen it's used already in several places:
> >
> > $ grep PHY_INTERFACE_MODE_10GKR drivers/net -nr
> > drivers/net/phy/marvell10g.c:219:       if (iface !=
> PHY_INTERFACE_MODE_10GKR) {
> > drivers/net/phy/marvell10g.c:307:           phydev->interface !=
> PHY_INTERFACE_MODE_10GKR)
> > drivers/net/phy/marvell10g.c:389:            phydev->interface ==
> PHY_INTERFACE_MODE_10GKR) && phydev->link) {
> > drivers/net/phy/marvell10g.c:398:                       phydev-
> >interface = PHY_INTERFACE_MODE_10GKR;
> > drivers/net/phy/phylink.c:296:          case PHY_INTERFACE_MODE_10GKR:
> > drivers/net/phy/aquantia_main.c:361:            phydev->interface =
> PHY_INTERFACE_MODE_10GKR;
> > drivers/net/phy/aquantia_main.c:499:        phydev->interface !=
> PHY_INTERFACE_MODE_10GKR)
> > drivers/net/phy/sfp-bus.c:340:          return
> PHY_INTERFACE_MODE_10GKR;
> > drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:1117:   return
> interface == PHY_INTERFACE_MODE_10GKR ||
> > drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:1203:   case
> PHY_INTERFACE_MODE_10GKR:
> > drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:1652:   case
> PHY_INTERFACE_MODE_10GKR:
> > drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:4761:   case
> PHY_INTERFACE_MODE_10GKR:
> > drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:4783:   case
> PHY_INTERFACE_MODE_10GKR:
> >
> > We should fix this, if it's incorrect.
> >
> > > If we add something else, then the problem becomes what to do about
> > > that lot - one of the problems is, it seems we're going to be
> > > breaking DT compatibility by redefining 10gbase-kr to be correct.
> >
> > We need the committer/maintainer to update that to a correct value.
> 
> The general principle is, we don't break existing DT - in that, we
> expect DT files from current kernels to work with future kernels. So,
> we're kind of stuck with "10gbase-kr" being used for this at least in
> the medium term.
> 
> By all means introduce "xfi" and "sfi" if you think that there is a
> need to discriminate between the two, but I've seen no hardware which
> that treats them any differently from 10gbase-r.
> 
> If we want to support real 10gbase-kr, then I think we need to consider
> how to do that without affecting compatibility with what we already
> have.
> 
> --
> 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

I've looked at the device tree entries using 10GBase-KR:

all these are disabled:

// disabled, commit mentions interface is SFI, jaz@semihalf.com
arch/arm64/boot/dts/marvell/cn9132-db.dts:107:  phy-mode = "10gbase-kr";

// disabled, SFI with SFP cage, jaz@semihalf.com
arch/arm64/boot/dts/marvell/cn9130-db.dts:131:  phy-mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/cn9131-db.dts:89:   phy-mode = "10gbase-kr";

these are used:

// SFP ports, antoine.tenart@free-electrons.com
arch/arm64/boot/dts/marvell/armada-7040-db.dts:279:     phy-mode = "10gbase-kr"; 
arch/arm64/boot/dts/marvell/armada-8040-db.dts:190:     phy-mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/armada-8040-db.dts:334:     phy-mode = "10gbase-kr";

// SFP, 10GKR, antoine.tenart@free-electrons.com
arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts:37:   phy-mode = "10gbase-kr";
arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts:44:   phy-mode = "10gbase-kr";

// SFP, baruch@tkos.co.il
arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts:279: phy-mode = "10gbase-kr";

// SFP+, rmk+kernel@armlinux.org.uk
arch/arm64/boot/dts/marvell/armada-8040-mcbin-singleshot.dts:19:        phy-mode = "10gbase-kr"; 
arch/arm64/boot/dts/marvell/armada-8040-mcbin-singleshot.dts:26:        phy-mode = "10gbase-kr"; 

I've added the information I could derive from the commit message.
Maybe the original authors of the commits can help us with more
information on the actual HW capabilities/operation mode.

Regards,
Madalin

Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Russell King - ARM Linux admin]

On Fri, Dec 20, 2019 at 07:38:45AM +0000, Madalin Bucur (OSS) wrote:
> > -----Original Message-----
> > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > On Thu, Dec 19, 2019 at 09:34:57PM +0000, Madalin Bucur (OSS) wrote:
> > > > -----Original Message-----
> > > > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > > > On Thu, Dec 19, 2019 at 06:32:51PM +0000, Madalin Bucur wrote:
> > > > > > -----Original Message-----
> > > > > > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > > > > >
> > > > > > On Thu, Dec 19, 2019 at 05:21:16PM +0200, Madalin Bucur wrote:
> > > > > > > From: Madalin Bucur <madalin.bucur@nxp.com>
> > > > > > >
> > > > > > > Add explicit entries for XFI, SFI to make sure the device
> > > > > > > tree entries for phy-connection-type "xfi" or "sfi" are
> > > > > > > properly parsed and differentiated against the existing
> > > > > > > backplane 10GBASE-KR mode.
> > > > > >
> > > > > > 10GBASE-KR is actually used for XFI and SFI (due to a slight
> > > > > > mistake on my part, it should've been just 10GBASE-R).
> > > > > >
> > > > > > Please explain exactly what the difference is between XFI, SFI
> > > > > > and 10GBASE-R. I have not been able to find definitive definitions
> > > > > > for XFI and SFI anywhere, and they appear to be precisely identical
> > > > > > to 10GBASE-R. It seems that it's just a terminology thing, with
> > > > > > different groups wanting to "own" what is essentially exactly the
> > > > > > same interface type.
> > > > >
> > > > > Hi Russell,
> > > > >
> > > > > 10GBase-R could be used as a common nominator but just as well 10G
> > > > > and remove the rest while we're at it. There are/may be differences in
> > > > > features, differences in the way the HW is configured (the most
> > > > > important aspect) and one should be able to determine what interface
> > > > > type is in use to properly configure the HW. SFI does not have the
> > > > > CDR function in the PMD, relying on the PMA signal conditioning vs the
> > > > > XFI that requires this in the PMD. We kept the xgmii compatible for so
> > > > > long without much issues until someone started cleaning up the PHY
> > > > > supported modes. Since we're doing that, let's be rigorous. The 10GBase-KR
> > > > > is important too, we have some backplane code in preparation and
> > > > > having it there could pave the way for a simpler integration.
> > > >
> > > > The problem we currently have is:
> > > >
> > > > $ grep '10gbase-kr' arch/*/boot/dts -r
> > > >
> > > > virtually none of those are actually backplane. For the mcbin
> > > > matches, these are either to a 88x3310 PHY for the doubleshot, which
> > > > dynamically operates between XFI, 5GBASE-R, 2500BASE-X, or SGMII according
> > > > to the datasheet.
> > >
> > > Yes, I've seen it's used already in several places:
> > >
> > > $ grep PHY_INTERFACE_MODE_10GKR drivers/net -nr
> > > drivers/net/phy/marvell10g.c:219:       if (iface !=
> > PHY_INTERFACE_MODE_10GKR) {
> > > drivers/net/phy/marvell10g.c:307:           phydev->interface !=
> > PHY_INTERFACE_MODE_10GKR)
> > > drivers/net/phy/marvell10g.c:389:            phydev->interface ==
> > PHY_INTERFACE_MODE_10GKR) && phydev->link) {
> > > drivers/net/phy/marvell10g.c:398:                       phydev-
> > >interface = PHY_INTERFACE_MODE_10GKR;
> > > drivers/net/phy/phylink.c:296:          case PHY_INTERFACE_MODE_10GKR:
> > > drivers/net/phy/aquantia_main.c:361:            phydev->interface =
> > PHY_INTERFACE_MODE_10GKR;
> > > drivers/net/phy/aquantia_main.c:499:        phydev->interface !=
> > PHY_INTERFACE_MODE_10GKR)
> > > drivers/net/phy/sfp-bus.c:340:          return
> > PHY_INTERFACE_MODE_10GKR;
> > > drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:1117:   return
> > interface == PHY_INTERFACE_MODE_10GKR ||
> > > drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:1203:   case
> > PHY_INTERFACE_MODE_10GKR:
> > > drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:1652:   case
> > PHY_INTERFACE_MODE_10GKR:
> > > drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:4761:   case
> > PHY_INTERFACE_MODE_10GKR:
> > > drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:4783:   case
> > PHY_INTERFACE_MODE_10GKR:
> > >
> > > We should fix this, if it's incorrect.
> > >
> > > > If we add something else, then the problem becomes what to do about
> > > > that lot - one of the problems is, it seems we're going to be
> > > > breaking DT compatibility by redefining 10gbase-kr to be correct.
> > >
> > > We need the committer/maintainer to update that to a correct value.
> > 
> > The general principle is, we don't break existing DT - in that, we
> > expect DT files from current kernels to work with future kernels. So,
> > we're kind of stuck with "10gbase-kr" being used for this at least in
> > the medium term.
> > 
> > By all means introduce "xfi" and "sfi" if you think that there is a
> > need to discriminate between the two, but I've seen no hardware which
> > that treats them any differently from 10gbase-r.
> > 
> > If we want to support real 10gbase-kr, then I think we need to consider
> > how to do that without affecting compatibility with what we already
> > have.
> > 
> > --
> > 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
> 
> I've looked at the device tree entries using 10GBase-KR:
> 
> all these are disabled:
> 
> // disabled, commit mentions interface is SFI, jaz@semihalf.com
> arch/arm64/boot/dts/marvell/cn9132-db.dts:107:  phy-mode = "10gbase-kr";
> 
> // disabled, SFI with SFP cage, jaz@semihalf.com
> arch/arm64/boot/dts/marvell/cn9130-db.dts:131:  phy-mode = "10gbase-kr";
> arch/arm64/boot/dts/marvell/cn9131-db.dts:89:   phy-mode = "10gbase-kr";
> 
> these are used:
> 
> // SFP ports, antoine.tenart@free-electrons.com
> arch/arm64/boot/dts/marvell/armada-7040-db.dts:279:     phy-mode = "10gbase-kr"; 
> arch/arm64/boot/dts/marvell/armada-8040-db.dts:190:     phy-mode = "10gbase-kr";
> arch/arm64/boot/dts/marvell/armada-8040-db.dts:334:     phy-mode = "10gbase-kr";
> 
> // SFP, 10GKR, antoine.tenart@free-electrons.com
> arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts:37:   phy-mode = "10gbase-kr";
> arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts:44:   phy-mode = "10gbase-kr";
> 
> // SFP, baruch@tkos.co.il
> arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts:279: phy-mode = "10gbase-kr";
> 
> // SFP+, rmk+kernel@armlinux.org.uk
> arch/arm64/boot/dts/marvell/armada-8040-mcbin-singleshot.dts:19:        phy-mode = "10gbase-kr"; 
> arch/arm64/boot/dts/marvell/armada-8040-mcbin-singleshot.dts:26:        phy-mode = "10gbase-kr"; 
> 
> I've added the information I could derive from the commit message.
> Maybe the original authors of the commits can help us with more
> information on the actual HW capabilities/operation mode.

How does this help us when we can't simply change the existing usage?
We can update the DT but we can't free up the usage of "10gbase-kr".

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

Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Andrew Lunn]

> How does this help us when we can't simply change the existing usage?
> We can update the DT but we can't free up the usage of "10gbase-kr".

Agreed. Code needs to keep on interpreting "10gbase-kr" as any 10G
link. If we ever have a true 10gbase-kr, 802.3ap, one meter of copper
and two connectors, we are going to have to add a new mode to
represent true 10gbase-kr.

	Andrew

RE: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Madalin Bucur (OSS)]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Friday, December 20, 2019 11:29 AM
> To: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Cc: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>; antoine.tenart@free-
> electrons.com; jaz@semihalf.com; baruch@tkos.co.il; davem@davemloft.net;
> netdev@vger.kernel.org; f.fainelli@gmail.com; hkallweit1@gmail.com;
> shawnguo@kernel.org; devicetree@vger.kernel.org
> Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> 
> > How does this help us when we can't simply change the existing usage?
> > We can update the DT but we can't free up the usage of "10gbase-kr".
> 
> Agreed. Code needs to keep on interpreting "10gbase-kr" as any 10G
> link. If we ever have a true 10gbase-kr, 802.3ap, one meter of copper
> and two connectors, we are going to have to add a new mode to
> represent true 10gbase-kr.
> 
> 	Andrew

Hi, actually we do have that. What would be the name of the new mode
representing true 10GBase-KR that we will need to add when we upstream
support for that?

Thanks,
Madalin

Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Andrew Lunn]

On Fri, Dec 20, 2019 at 09:39:08AM +0000, Madalin Bucur (OSS) wrote:
> > -----Original Message-----
> > From: Andrew Lunn <andrew@lunn.ch>
> > Sent: Friday, December 20, 2019 11:29 AM
> > To: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > Cc: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>; antoine.tenart@free-
> > electrons.com; jaz@semihalf.com; baruch@tkos.co.il; davem@davemloft.net;
> > netdev@vger.kernel.org; f.fainelli@gmail.com; hkallweit1@gmail.com;
> > shawnguo@kernel.org; devicetree@vger.kernel.org
> > Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> > 
> > > How does this help us when we can't simply change the existing usage?
> > > We can update the DT but we can't free up the usage of "10gbase-kr".
> > 
> > Agreed. Code needs to keep on interpreting "10gbase-kr" as any 10G
> > link. If we ever have a true 10gbase-kr, 802.3ap, one meter of copper
> > and two connectors, we are going to have to add a new mode to
> > represent true 10gbase-kr.
> > 
> > 	Andrew
> 
> Hi, actually we do have that. What would be the name of the new mode
> representing true 10GBase-KR that we will need to add when we upstream
> support for that?

Ah!

This is going to be messy.

Do you really need to differentiate? What seems to make 802.3ap
different is the FEC, autoneg and link training. Does you hardware
support this? Do you need to know you are supposed to be using 802.3ap
in order to configure these features?

What are we going to report to user space? 10gbase-kr, or
10gbase-kr-true? How do we handle the mess this makes with firmware
based cards which correctly report
ETHTOOL_LINK_MODE_10000baseKR_Full_BIT to user space?

What do we currently report to user space? Is it possible for us to
split DT from user space? DT says 10gbase-kr-true but to user space we
say ETHTOOL_LINK_MODE_10000baseKR_Full_BIT?

I think in order to work through these issues, somebody probably needs
the hardware, and the desire to see it working. So it might actually
be you who makes a proposal how we sort this out, with help from
Russell and I.

	Andrew

RE: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Madalin Bucur (OSS)]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Friday, December 20, 2019 12:06 PM
> To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>;
> antoine.tenart@free-electrons.com; jaz@semihalf.com; baruch@tkos.co.il;
> davem@davemloft.net; netdev@vger.kernel.org; f.fainelli@gmail.com;
> hkallweit1@gmail.com; shawnguo@kernel.org; devicetree@vger.kernel.org
> Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> 
> On Fri, Dec 20, 2019 at 09:39:08AM +0000, Madalin Bucur (OSS) wrote:
> > > -----Original Message-----
> > > From: Andrew Lunn <andrew@lunn.ch>
> > > Sent: Friday, December 20, 2019 11:29 AM
> > > To: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > > Cc: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>;
> antoine.tenart@free-
> > > electrons.com; jaz@semihalf.com; baruch@tkos.co.il;
> davem@davemloft.net;
> > > netdev@vger.kernel.org; f.fainelli@gmail.com; hkallweit1@gmail.com;
> > > shawnguo@kernel.org; devicetree@vger.kernel.org
> > > Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> > >
> > > > How does this help us when we can't simply change the existing
> usage?
> > > > We can update the DT but we can't free up the usage of "10gbase-kr".
> > >
> > > Agreed. Code needs to keep on interpreting "10gbase-kr" as any 10G
> > > link. If we ever have a true 10gbase-kr, 802.3ap, one meter of copper
> > > and two connectors, we are going to have to add a new mode to
> > > represent true 10gbase-kr.
> > >
> > > 	Andrew
> >
> > Hi, actually we do have that. What would be the name of the new mode
> > representing true 10GBase-KR that we will need to add when we upstream
> > support for that?
> 
> Ah!
> 
> This is going to be messy.
> 
> Do you really need to differentiate? What seems to make 802.3ap
> different is the FEC, autoneg and link training. Does you hardware
> support this? Do you need to know you are supposed to be using 802.3ap
> in order to configure these features?

Yes, it does.

> What are we going to report to user space? 10gbase-kr, or
> 10gbase-kr-true? How do we handle the mess this makes with firmware
> based cards which correctly report
> ETHTOOL_LINK_MODE_10000baseKR_Full_BIT to user space?
> 
> What do we currently report to user space? Is it possible for us to
> split DT from user space? DT says 10gbase-kr-true but to user space we
> say ETHTOOL_LINK_MODE_10000baseKR_Full_BIT?
> 
> I think in order to work through these issues, somebody probably needs
> the hardware, and the desire to see it working. So it might actually
> be you who makes a proposal how we sort this out, with help from
> Russell and I.
> 
> 	Andrew

We're overcomplicating the fix. As far as I can see only some Marvell boards
declared 10GBase-KR as PHY interface type. These either support 10GBase-KR or
they don't. When we learn this, we'll need to set things straight in the device
trees and code. Until then it will remain as is, there is no trouble with that.
I'll reorder and resend the patches that introduce the XFI and SFI types.

Regards,
Madalin

Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Russell King - ARM Linux admin]

On Mon, Dec 23, 2019 at 07:50:08AM +0000, Madalin Bucur (OSS) wrote:
> > -----Original Message-----
> > From: Andrew Lunn <andrew@lunn.ch>
> > Sent: Friday, December 20, 2019 12:06 PM
> > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> > Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>;
> > antoine.tenart@free-electrons.com; jaz@semihalf.com; baruch@tkos.co.il;
> > davem@davemloft.net; netdev@vger.kernel.org; f.fainelli@gmail.com;
> > hkallweit1@gmail.com; shawnguo@kernel.org; devicetree@vger.kernel.org
> > Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> > 
> > On Fri, Dec 20, 2019 at 09:39:08AM +0000, Madalin Bucur (OSS) wrote:
> > > > -----Original Message-----
> > > > From: Andrew Lunn <andrew@lunn.ch>
> > > > Sent: Friday, December 20, 2019 11:29 AM
> > > > To: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > > > Cc: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>;
> > antoine.tenart@free-
> > > > electrons.com; jaz@semihalf.com; baruch@tkos.co.il;
> > davem@davemloft.net;
> > > > netdev@vger.kernel.org; f.fainelli@gmail.com; hkallweit1@gmail.com;
> > > > shawnguo@kernel.org; devicetree@vger.kernel.org
> > > > Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> > > >
> > > > > How does this help us when we can't simply change the existing
> > usage?
> > > > > We can update the DT but we can't free up the usage of "10gbase-kr".
> > > >
> > > > Agreed. Code needs to keep on interpreting "10gbase-kr" as any 10G
> > > > link. If we ever have a true 10gbase-kr, 802.3ap, one meter of copper
> > > > and two connectors, we are going to have to add a new mode to
> > > > represent true 10gbase-kr.
> > > >
> > > > 	Andrew
> > >
> > > Hi, actually we do have that. What would be the name of the new mode
> > > representing true 10GBase-KR that we will need to add when we upstream
> > > support for that?
> > 
> > Ah!
> > 
> > This is going to be messy.
> > 
> > Do you really need to differentiate? What seems to make 802.3ap
> > different is the FEC, autoneg and link training. Does you hardware
> > support this? Do you need to know you are supposed to be using 802.3ap
> > in order to configure these features?
> 
> Yes, it does.
> 
> > What are we going to report to user space? 10gbase-kr, or
> > 10gbase-kr-true? How do we handle the mess this makes with firmware
> > based cards which correctly report
> > ETHTOOL_LINK_MODE_10000baseKR_Full_BIT to user space?
> > 
> > What do we currently report to user space? Is it possible for us to
> > split DT from user space? DT says 10gbase-kr-true but to user space we
> > say ETHTOOL_LINK_MODE_10000baseKR_Full_BIT?
> > 
> > I think in order to work through these issues, somebody probably needs
> > the hardware, and the desire to see it working. So it might actually
> > be you who makes a proposal how we sort this out, with help from
> > Russell and I.
> > 
> > 	Andrew
> 
> We're overcomplicating the fix. As far as I can see only some Marvell boards
> declared 10GBase-KR as PHY interface type. These either support 10GBase-KR or
> they don't. When we learn this, we'll need to set things straight in the device
> trees and code. Until then it will remain as is, there is no trouble with that.

No we aren't.

You think we can just change the existing DT, switching them to use
XFI/SFI and free up the "10gbase-kr" definition.  Yes, we can change
the existing DT.  What we *can't* do is free up the existing definition
for "10gbase-kr" because old device trees must continue to work with
new kernels.  That is one of the rules we abide by with the kernel.

Now, looking at the Armada 8040 data, it only mentions XFI.  It's
described as "10GBASE-R (10GbE on single SERDES, including XFI)"
and goes on to say that it is compliant with "IEEE 802.3 standard".
However, there is no mention of a CDR, except for XAUI/XSGS mode,
not for 10GBASE-R/XFI mode.

So, it really isn't clear whether Marvell uses "XFI" to refer to a
port with a CDR or not.

Marvell's original MVPP2.2 and comphy drivers did used to distinguish
between XFI and SFI, but there was absolutely no difference in the
way the hardware was programmed.

Then there's the matter that (I believe with some firmware) it can
also support 10GBASE-KR (with clause 73 AN) after all.

So, we can't just replace the existing usage in DT with "SFI" just
because it's connected to a SFP cage, or "XFI" if it's connected
to a PHY which also mentiones "XFI" in its datasheet. We can't tell
whether the hardware _actually_ supports 1EEE 10GBASE-R, XFI or SFI.

Given that XFI and SFI are not actually documented (afaics) this is
really not that surprising - and I would not be surprised if these
three terms were used interchangably by some device manufacturers
as a result.

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

RE: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Madalin Bucur (OSS)]

> -----Original Message-----
> From: devicetree-owner@vger.kernel.org <devicetree-owner@vger.kernel.org>
> On Behalf Of Russell King - ARM Linux admin
> Sent: Monday, December 23, 2019 10:27 AM
> To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> 
> On Mon, Dec 23, 2019 at 07:50:08AM +0000, Madalin Bucur (OSS) wrote:
> > > -----Original Message-----
> > > From: Andrew Lunn <andrew@lunn.ch>
> > > Sent: Friday, December 20, 2019 12:06 PM
> > > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> > > Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>;
> > > antoine.tenart@free-electrons.com; jaz@semihalf.com;
> baruch@tkos.co.il;
> > > davem@davemloft.net; netdev@vger.kernel.org; f.fainelli@gmail.com;
> > > hkallweit1@gmail.com; shawnguo@kernel.org; devicetree@vger.kernel.org
> > > Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> > >
> > > On Fri, Dec 20, 2019 at 09:39:08AM +0000, Madalin Bucur (OSS) wrote:
> > > > > -----Original Message-----
> > > > > From: Andrew Lunn <andrew@lunn.ch>
> > > > > Sent: Friday, December 20, 2019 11:29 AM
> > > > > To: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > > > > Cc: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>;
> > > antoine.tenart@free-
> > > > > electrons.com; jaz@semihalf.com; baruch@tkos.co.il;
> > > davem@davemloft.net;
> > > > > netdev@vger.kernel.org; f.fainelli@gmail.com;
> hkallweit1@gmail.com;
> > > > > shawnguo@kernel.org; devicetree@vger.kernel.org
> > > > > Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI,
> SFI
> > > > >
> > > > > > How does this help us when we can't simply change the existing
> > > usage?
> > > > > > We can update the DT but we can't free up the usage of "10gbase-
> kr".
> > > > >
> > > > > Agreed. Code needs to keep on interpreting "10gbase-kr" as any 10G
> > > > > link. If we ever have a true 10gbase-kr, 802.3ap, one meter of
> copper
> > > > > and two connectors, we are going to have to add a new mode to
> > > > > represent true 10gbase-kr.
> > > > >
> > > > > 	Andrew
> > > >
> > > > Hi, actually we do have that. What would be the name of the new mode
> > > > representing true 10GBase-KR that we will need to add when we
> upstream
> > > > support for that?
> > >
> > > Ah!
> > >
> > > This is going to be messy.
> > >
> > > Do you really need to differentiate? What seems to make 802.3ap
> > > different is the FEC, autoneg and link training. Does you hardware
> > > support this? Do you need to know you are supposed to be using 802.3ap
> > > in order to configure these features?
> >
> > Yes, it does.
> >
> > > What are we going to report to user space? 10gbase-kr, or
> > > 10gbase-kr-true? How do we handle the mess this makes with firmware
> > > based cards which correctly report
> > > ETHTOOL_LINK_MODE_10000baseKR_Full_BIT to user space?
> > >
> > > What do we currently report to user space? Is it possible for us to
> > > split DT from user space? DT says 10gbase-kr-true but to user space we
> > > say ETHTOOL_LINK_MODE_10000baseKR_Full_BIT?
> > >
> > > I think in order to work through these issues, somebody probably needs
> > > the hardware, and the desire to see it working. So it might actually
> > > be you who makes a proposal how we sort this out, with help from
> > > Russell and I.
> > >
> > > 	Andrew
> >
> > We're overcomplicating the fix. As far as I can see only some Marvell
> boards
> > declared 10GBase-KR as PHY interface type. These either support 10GBase-
> KR or
> > they don't. When we learn this, we'll need to set things straight in the
> device
> > trees and code. Until then it will remain as is, there is no trouble
> with that.
> 
> No we aren't.
> 
> You think we can just change the existing DT, switching them to use
> XFI/SFI and free up the "10gbase-kr" definition.  Yes, we can change
> the existing DT.  What we *can't* do is free up the existing definition
> for "10gbase-kr" because old device trees must continue to work with
> new kernels.  That is one of the rules we abide by with the kernel.

We do not need to "free up" the definition, if that particular device
does use 10GBase-KR then it does not need changing. Please note 10GBase-KR
is quite well established in the Ethernet nomenclature (802.3ap, 2007),
it's clear what it does so there is no need to "free it up" somewhere.

The phy-connection-type was introduced back in the day when this
connection was more aligned to the MAC-PHY MII. That resulted in
XGMII being used for all 10G interfaces when they were added.
But the clean MII separation does not align with today's HW, where
the asic/SoC to external PHY chip interface has evolved towards
high speed serial interfaces with obvious benefits. We're now seeing
the PCS, PMA blocks that are part of the PHY layer moved together with
the MAC.

I've recently seen how a certain PHY driver started using USXGMII and
rejected XGMII (please note the MII is still XGMII but it's buried
somewhere in the peer SoC) as a phy-conection-type. Some HW platforms
may use that PHY in USXGMII mode, others use it in one of the other
supported modes - XFI. Why is USXGMII a valid phy-connection-type
but XFI is not? Is it correct that I describe that HW as using
10GBase-KR instead of XFI, something that has a clearly different
meaning for anyone familiar with IEEE 802.3ap just because there was
a typo in a patch, be it a device tree one? I'm puzzled.

> Now, looking at the Armada 8040 data, it only mentions XFI.  It's
> described as "10GBASE-R (10GbE on single SERDES, including XFI)"
> and goes on to say that it is compliant with "IEEE 802.3 standard".
> However, there is no mention of a CDR, except for XAUI/XSGS mode,
> not for 10GBASE-R/XFI mode.

Is "10GBASE-R" in wide use? Although you can extract that from the
Ethernet nomenclature to mean 10G with 64B/66B encoding, I did not
see it used much by vendors. I do see more of the XFI, SFI terminology
in use.
 
> So, it really isn't clear whether Marvell uses "XFI" to refer to a
> port with a CDR or not.
> 
> Marvell's original MVPP2.2 and comphy drivers did used to distinguish
> between XFI and SFI, but there was absolutely no difference in the
> way the hardware was programmed.
> 
> Then there's the matter that (I believe with some firmware) it can
> also support 10GBASE-KR (with clause 73 AN) after all.

Then there's not a real need to change that device tree, it's correct.
The HW can be used in 10GBASE-KR with appropriate SW so the HW description
is fine.
 
> So, we can't just replace the existing usage in DT with "SFI" just
> because it's connected to a SFP cage, or "XFI" if it's connected
> to a PHY which also mentiones "XFI" in its datasheet. We can't tell
> whether the hardware _actually_ supports 1EEE 10GBASE-R, XFI or SFI.

We don't need to replace it. But if a certain HW is certain to use XFI
(or SFI), we just need to describe it as such in the device tree.

> Given that XFI and SFI are not actually documented (afaics) this is
> really not that surprising - and I would not be surprised if these
> three terms were used interchangably by some device manufacturers
> as a result.

XFI (and SFI?) are the result of MSA, multi-source agreements so they
must be documented somewhere (see http://www.xfpmsa.org/). Whether that
document is available for free or behind a paywall/membership it's
another discussion.

> --
> 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
OT: FTTB 1Gbps link, my phone reports 279Mbps down, 169Mbps up, limited by WiFi
Internet provider services vary wildly across the globe

Best regards,
Madalin

Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Russell King - ARM Linux admin]

On Mon, Dec 23, 2019 at 09:57:51AM +0000, Madalin Bucur (OSS) wrote:
> > -----Original Message-----
> > From: devicetree-owner@vger.kernel.org <devicetree-owner@vger.kernel.org>
> > On Behalf Of Russell King - ARM Linux admin
> > Sent: Monday, December 23, 2019 10:27 AM
> > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> > Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> > 
> > On Mon, Dec 23, 2019 at 07:50:08AM +0000, Madalin Bucur (OSS) wrote:
> > > > -----Original Message-----
> > > > From: Andrew Lunn <andrew@lunn.ch>
> > > > Sent: Friday, December 20, 2019 12:06 PM
> > > > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> > > > Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>;
> > > > antoine.tenart@free-electrons.com; jaz@semihalf.com;
> > baruch@tkos.co.il;
> > > > davem@davemloft.net; netdev@vger.kernel.org; f.fainelli@gmail.com;
> > > > hkallweit1@gmail.com; shawnguo@kernel.org; devicetree@vger.kernel.org
> > > > Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> > > >
> > > > On Fri, Dec 20, 2019 at 09:39:08AM +0000, Madalin Bucur (OSS) wrote:
> > > > > > -----Original Message-----
> > > > > > From: Andrew Lunn <andrew@lunn.ch>
> > > > > > Sent: Friday, December 20, 2019 11:29 AM
> > > > > > To: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > > > > > Cc: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>;
> > > > antoine.tenart@free-
> > > > > > electrons.com; jaz@semihalf.com; baruch@tkos.co.il;
> > > > davem@davemloft.net;
> > > > > > netdev@vger.kernel.org; f.fainelli@gmail.com;
> > hkallweit1@gmail.com;
> > > > > > shawnguo@kernel.org; devicetree@vger.kernel.org
> > > > > > Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI,
> > SFI
> > > > > >
> > > > > > > How does this help us when we can't simply change the existing
> > > > usage?
> > > > > > > We can update the DT but we can't free up the usage of "10gbase-
> > kr".
> > > > > >
> > > > > > Agreed. Code needs to keep on interpreting "10gbase-kr" as any 10G
> > > > > > link. If we ever have a true 10gbase-kr, 802.3ap, one meter of
> > copper
> > > > > > and two connectors, we are going to have to add a new mode to
> > > > > > represent true 10gbase-kr.
> > > > > >
> > > > > > 	Andrew
> > > > >
> > > > > Hi, actually we do have that. What would be the name of the new mode
> > > > > representing true 10GBase-KR that we will need to add when we
> > upstream
> > > > > support for that?
> > > >
> > > > Ah!
> > > >
> > > > This is going to be messy.
> > > >
> > > > Do you really need to differentiate? What seems to make 802.3ap
> > > > different is the FEC, autoneg and link training. Does you hardware
> > > > support this? Do you need to know you are supposed to be using 802.3ap
> > > > in order to configure these features?
> > >
> > > Yes, it does.
> > >
> > > > What are we going to report to user space? 10gbase-kr, or
> > > > 10gbase-kr-true? How do we handle the mess this makes with firmware
> > > > based cards which correctly report
> > > > ETHTOOL_LINK_MODE_10000baseKR_Full_BIT to user space?
> > > >
> > > > What do we currently report to user space? Is it possible for us to
> > > > split DT from user space? DT says 10gbase-kr-true but to user space we
> > > > say ETHTOOL_LINK_MODE_10000baseKR_Full_BIT?
> > > >
> > > > I think in order to work through these issues, somebody probably needs
> > > > the hardware, and the desire to see it working. So it might actually
> > > > be you who makes a proposal how we sort this out, with help from
> > > > Russell and I.
> > > >
> > > > 	Andrew
> > >
> > > We're overcomplicating the fix. As far as I can see only some Marvell
> > boards
> > > declared 10GBase-KR as PHY interface type. These either support 10GBase-
> > KR or
> > > they don't. When we learn this, we'll need to set things straight in the
> > device
> > > trees and code. Until then it will remain as is, there is no trouble
> > with that.
> > 
> > No we aren't.
> > 
> > You think we can just change the existing DT, switching them to use
> > XFI/SFI and free up the "10gbase-kr" definition.  Yes, we can change
> > the existing DT.  What we *can't* do is free up the existing definition
> > for "10gbase-kr" because old device trees must continue to work with
> > new kernels.  That is one of the rules we abide by with the kernel.
> 
> We do not need to "free up" the definition, if that particular device
> does use 10GBase-KR then it does not need changing. Please note 10GBase-KR
> is quite well established in the Ethernet nomenclature (802.3ap, 2007),
> it's clear what it does so there is no need to "free it up" somewhere.

I'm fully aware of what is in IEE 802.3.

> The phy-connection-type was introduced back in the day when this
> connection was more aligned to the MAC-PHY MII. That resulted in
> XGMII being used for all 10G interfaces when they were added.
> But the clean MII separation does not align with today's HW, where
> the asic/SoC to external PHY chip interface has evolved towards
> high speed serial interfaces with obvious benefits. We're now seeing
> the PCS, PMA blocks that are part of the PHY layer moved together with
> the MAC.
> 
> I've recently seen how a certain PHY driver started using USXGMII and
> rejected XGMII (please note the MII is still XGMII but it's buried
> somewhere in the peer SoC) as a phy-conection-type. Some HW platforms
> may use that PHY in USXGMII mode, others use it in one of the other
> supported modes - XFI. Why is USXGMII a valid phy-connection-type
> but XFI is not? Is it correct that I describe that HW as using
> 10GBase-KR instead of XFI, something that has a clearly different
> meaning for anyone familiar with IEEE 802.3ap just because there was
> a typo in a patch, be it a device tree one? I'm puzzled.

I think I've already explained why several times.  In the kernel, we
do not break new kernels running with existing DT files.

Therefore, today's usage of "10gbase-kr" must continue to work in
the current setups.

> > Now, looking at the Armada 8040 data, it only mentions XFI.  It's
> > described as "10GBASE-R (10GbE on single SERDES, including XFI)"
> > and goes on to say that it is compliant with "IEEE 802.3 standard".
> > However, there is no mention of a CDR, except for XAUI/XSGS mode,
> > not for 10GBASE-R/XFI mode.
> 
> Is "10GBASE-R" in wide use? Although you can extract that from the
> Ethernet nomenclature to mean 10G with 64B/66B encoding, I did not
> see it used much by vendors. I do see more of the XFI, SFI terminology
> in use.

Only Intel, Xilinx, Synopsys on the first page of google for
"10GBASE-R".  For "XFI" and "SFI" there's nothing relating to ethernet
on the first page.  So, rather hard to know unless you're in the
hardware industry in that sector.

> > So, it really isn't clear whether Marvell uses "XFI" to refer to a
> > port with a CDR or not.
> > 
> > Marvell's original MVPP2.2 and comphy drivers did used to distinguish
> > between XFI and SFI, but there was absolutely no difference in the
> > way the hardware was programmed.
> > 
> > Then there's the matter that (I believe with some firmware) it can
> > also support 10GBASE-KR (with clause 73 AN) after all.
> 
> Then there's not a real need to change that device tree, it's correct.

Sorry.  Why do you believe it's correct?  As far as I know, the
firmware to support 10GBASE-KR on the Armada 8040 was never
actually released.

Given that some of these are connected directly to a SFP cage.
Given that some of these are connected directly to a PHY that
doesn't do Clause 73 AN over the link.

> The HW can be used in 10GBASE-KR with appropriate SW so the HW description
> is fine.

No it is not.

Just because the hardware _can_ be used with 10GBASE-KR (with AN)
does not mean that the existing usage is correct from the information
you have given.

> > So, we can't just replace the existing usage in DT with "SFI" just
> > because it's connected to a SFP cage, or "XFI" if it's connected
> > to a PHY which also mentiones "XFI" in its datasheet. We can't tell
> > whether the hardware _actually_ supports 1EEE 10GBASE-R, XFI or SFI.
> 
> We don't need to replace it. But if a certain HW is certain to use XFI
> (or SFI), we just need to describe it as such in the device tree.
> 
> > Given that XFI and SFI are not actually documented (afaics) this is
> > really not that surprising - and I would not be surprised if these
> > three terms were used interchangably by some device manufacturers
> > as a result.
> 
> XFI (and SFI?) are the result of MSA, multi-source agreements so they
> must be documented somewhere (see http://www.xfpmsa.org/). Whether that
> document is available for free or behind a paywall/membership it's
> another discussion.

SFI in the SFF8431 defines the electrical characteristics required
in terms of the eye mask and other electrical parameters at various
defined points, such as at the SFP cage connector. That is no
different from other standards such as SATA, HDMI etc. Achieving
those parameters are board layout dependent, and are a matter for
the hardware design team for the board to work out how to setup the
hardware to achieve compliance with that specification.

SFI doesn't rigidly define the protocol in use, because it isn't
limited to just 10GBASE-R - it can be clause 49 10GBASE-R for LAN
ethernet, clause 50 10GBASE-W for WAN ethernet, 10GFC, or 10GBASE-R
with FEC.

10GBASE-R on the other hand does not define any electrical
characteristics. It is the underlying protocol specification only.

Hence, from what I can see, SFI for clause 49 ethernet is just
10GBASE-R with electrical compliance at the cage determined by the
board manufacturer. What that means is that to meet SFI compliance,
it is a whole-system integration problem. The SFP cage connector and
the PCB layout all have an impact on how the serdes needs to be tuned
to achieve compliance in meeting the specification. Every board likely
needs different tuning to achieve compliance.

The XFP MSA is not available to people like me, unlike the SFP
specifications. Effectively, that means it doesn't exist, and I
can't gain the knowledge of what its requirements are. So, choices
_had_ to be made without its knowledge, which is why we've ended
up where we are. Had the XFP MSA been more freely available, then
maybe better choices could be made - but alas, that is the cost of
keeping specifications behind paywalls/membership-only.  Those
sectors of the industry lose out when it comes to one of the
biggest most successful Open Source projects on the planet.

Therefore, for XFI, I don't care what we do - I have no basis on
which to make any decision on that.

phy_interface_t describes the protocol *only*, it doesn't describe
the electrical characteristics of the interface.  So, if we exclude
the electrical characteristics of SFI, we're back to 10GBASE-R,
10GBASE-W, 10GFC or 10GBASE-R with FEC.  That's what phy_interface_t
is, not SFI.

So, I propose that we add 10GBASE-R to the list and *not* SFI.

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

Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Russell King - ARM Linux admin]

On Thu, Dec 19, 2019 at 06:32:51PM +0000, Madalin Bucur wrote:
> 10GBase-R could be used as a common nominator but just as well 10G and
> remove the rest while we're at it. There are/may be differences in
> features, differences in the way the HW is configured (the most
> important aspect) and one should be able to determine what interface
> type is in use to properly configure the HW. SFI does not have the CDR
> function in the PMD, relying on the PMA signal conditioning vs the XFI
> that requires this in the PMD.

I've now found a copy of INF-8077i on the net, which seems to be the
document that defines XFI. The definition in there seems to be very
similar to SFI in that it is an electrical specification, not a
protocol specification, and, just like SFI, it defines the electrical
characteristics at the cage, not at the serdes. Therefore, the effects
of the board layout come into play to achieve compliance with XFI.

Just like SFI, XFI can be used with multiple different underlying
protocols. I quote:

  "The XFI interface is designed to support SONET OC-192,
  IEEE.Std-802.3ae, 10GFC and G.709(OTU-2) applications."

Therefore, to describe 10GBASE-R as "XFI" is most definitely incorrect.
10GBASE-R is just _one_ protocol that can be run over XFI, but it is
not the only one.

As for CDR, INF-8077i says:

  "The XFP module shall include a Signal Conditioner based on CDR (Clock
  Data Recovery) technology for complete regeneration."

Whereas for SFP modules, SFF-8472 revision 11.4 added optional support
for CDR on the modules.

In any case, the CDR is a matter for the module itself, not for the
host, so it seems that isn't relevent.

Everything that I've said concerning SFI in my previous email (in date
order), and why we should not be defining that as a phy_interface_t
seems to also apply to XFI from what I've read in INF-8077i, and it
seems my original decision that we should not separately define
XFI and SFI from 10GBASE-R is well founded.

Given that meeting these electrical characteristics involves the
effects of the board, and it is impractical for a board to switch
between different electrical characteristics at runtime (routing serdes
lanes to both a SFP+ and XFP cage is impractical due to reflections on
unterminated paths) I really don't see any reason why we need two
different phy_interface_t definitions for these.  As mentioned, the
difference between XFI and SFI is electrical, and involves the board
layout, which is a platform specific issue.

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

Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Andrew Lunn]

> Given that meeting these electrical characteristics involves the
> effects of the board, and it is impractical for a board to switch
> between different electrical characteristics at runtime (routing serdes
> lanes to both a SFP+ and XFP cage is impractical due to reflections on
> unterminated paths) I really don't see any reason why we need two
> different phy_interface_t definitions for these.  As mentioned, the
> difference between XFI and SFI is electrical, and involves the board
> layout, which is a platform specific issue.

Hi Russell

So we make phy_interface_t define the protocol. We should clearly
document that.

Are we going to need another well defined DT property for the
electrical interface? What is where we have XFI and SFI, etc?

	   Andrew

Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Russell King - ARM Linux admin]

On Mon, Dec 23, 2019 at 02:46:34PM +0100, Andrew Lunn wrote:
> > Given that meeting these electrical characteristics involves the
> > effects of the board, and it is impractical for a board to switch
> > between different electrical characteristics at runtime (routing serdes
> > lanes to both a SFP+ and XFP cage is impractical due to reflections on
> > unterminated paths) I really don't see any reason why we need two
> > different phy_interface_t definitions for these.  As mentioned, the
> > difference between XFI and SFI is electrical, and involves the board
> > layout, which is a platform specific issue.
> 
> Hi Russell
> 
> So we make phy_interface_t define the protocol. We should clearly
> document that.

I really think so.

> Are we going to need another well defined DT property for the
> electrical interface? What is where we have XFI and SFI, etc?

It is conceivable that board firmware or a serdes driver needs to know
what settings to apply, but I think we need to think long and hard
about that. As I've pointed out, it's probably not going to be a one
size fits all thing, because the properties of the PCB come into play.

On the Armada 8040, the electrical configuration is part of the comphy
block. The kernel does contain code to configure it using fixed
settings, but that is just a fallback when there is no support in the
ATF firmware. ATF firmware added support, and I know that the
Macchiatobin support has different electrical values for the 10G
Ethernet ports compared to the Marvell default - because I did a lot
of reverse engineering with the poor Marvell documentation to figure
out how to get an eye diagram out of the hardware, and used that to
improve the reliability of the SFP+ ports on the Macchiatobin single
shot and GT-8k boards.

Marvell implemented automatic comphy tuning in u-boot/firwmare, which
improves the performance of comphy on Marvell's hardware, but actually
ends up making things worse on the Macchiatobin platforms compared to
my hand-tuned parameters, with errors and link failures becoming
common.

There are multiple parameters that need to be configured, it isn't
a simple "just use XFI" or "just use SFI" settings.

Given that the electrical parameters are board dependent, they can
even be interface dependent if a board has several different
interfaces, so what is compliant for XFI on one port may not be
compliant for XFI on a different port. Different routing of the
Serdes lines may mean crosstalk is different, or different trace
capacitance needing different emphasis settings.

What I'm getting at is, basically, I don't think a one-size-fits-all
"XFI" or "SFI" specifier makes any sense what so ever.

I also can't think how we'd generalise it - the parameters required
to set the serdes hardware are likely to be implementation dependent.
If you've ever looked at Marvell's COMPHY documentation, it has a
hundred or more registers controlling all sorts of different serdes
parameters, some of them in the analogue domain others in the digital
domain. Some FFE, CTLE, DFE etc parameters - and many without
anything but a brief description of the register.

There are some simpler implementations too.  For example, SATA is
another example of serdes, and just like XFI and SFI, it also has
its own specifications for the electrical side... and then there's
eSATA as well.  For the eSATA port on the Cubox-i, I ended up with:

        fsl,transmit-level-mV = <1104>;
        fsl,transmit-boost-mdB = <0>;
        fsl,transmit-atten-16ths = <9>;
        fsl,no-spread-spectrum;

which I arrived at by blind analysis and successive approximation,
and was later confirmed to be correct when Rabeeh eventually got
around to proving the eye mask. I doubt that having an "eSATA" vs
"SATA" mode setting for the driver would have worked: just as I'm
saying for XFI vs SFI, the correct set of parameters for one platform
is likely not correct for another platform.

So, I think it's up to the serdes/comphy/firmware to figure out how
to configure the electrical properties for a serdes lane to meet the
specification for the _platform_ in question, including where it gets
the data for that configuration, be it from DT or board firmware.

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

RE: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Madalin Bucur (OSS)]

> -----Original Message-----
> From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Sent: Monday, December 23, 2019 2:08 PM
> To: Madalin Bucur <madalin.bucur@nxp.com>
> Cc: davem@davemloft.net; netdev@vger.kernel.org; andrew@lunn.ch;
> f.fainelli@gmail.com; hkallweit1@gmail.com; shawnguo@kernel.org;
> devicetree@vger.kernel.org
> Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> 
> On Thu, Dec 19, 2019 at 06:32:51PM +0000, Madalin Bucur wrote:
> > 10GBase-R could be used as a common nominator but just as well 10G and
> > remove the rest while we're at it. There are/may be differences in
> > features, differences in the way the HW is configured (the most
> > important aspect) and one should be able to determine what interface
> > type is in use to properly configure the HW. SFI does not have the CDR
> > function in the PMD, relying on the PMA signal conditioning vs the XFI
> > that requires this in the PMD.
> 
> I've now found a copy of INF-8077i on the net, which seems to be the
> document that defines XFI. The definition in there seems to be very
> similar to SFI in that it is an electrical specification, not a
> protocol specification, and, just like SFI, it defines the electrical
> characteristics at the cage, not at the serdes. Therefore, the effects
> of the board layout come into play to achieve compliance with XFI.

I think we're missing the point here: we need to start from the device
tree and that is supposed to describe the board, the hardware, not to
configure the software. Please re-read the paragraph above in this key:
the device tree needs to describe the HW features, those electrical
properties you are discussing above. The fact that we use a certain
protocol over it, by choice in software, does not change the HW and it
should not change the device tree describing it.

> Just like SFI, XFI can be used with multiple different underlying
> protocols. I quote:
> 
>   "The XFI interface is designed to support SONET OC-192,
>   IEEE.Std-802.3ae, 10GFC and G.709(OTU-2) applications."
> 
> Therefore, to describe 10GBASE-R as "XFI" is most definitely incorrect.
> 10GBASE-R is just _one_ protocol that can be run over XFI, but it is
> not the only one.

Exactly why the chip to chip interface described by the device tree needs
to be xfi not 10GBASE-R, that would really only provide information on the
PCS block that we're not featuring in device trees. Here's a rehash of
Ethernet nomenclature sourced from [1]:

Data rate (R):
  1000 → 1000 Mbps or 1 Gbps; Megabit unit is eliminated in the data rate reference
  10G → 10 Gbps
  10/1G → 10 Gbps downstream, 1 Gbps upstream
Modulation type (mTYPE): BASE → Baseband
Medium types / wavelength / reach (L):
  B → Bidirectional optics, with downstream (D) or upstream (U) asymmetric qualifiers
  C → Twin axial copper
  D → Parallel single mode (500 m)
  E → Extra-long optical wavelength λ (1510/1550 nm) / reach (40 km)
  F → Fiber (2 km)
  K → Backplane
  L → Long optical wavelength λ (~1310 nm) / reach (10 km)
  P → Passive optics, with single or multiple downstream (D) or upstream (U) asymmetric qualifiers, as well as eXternal sourced coding (X) of 4B/5B or 8B/10B
  RH → Red LED plastic optical fiber with PAM16 coding and different transmit power optics
  S → Short optical wavelength λ (850 nm) / reach (100 m)
  T → Twisted pair
PCS coding (C):
  R → scRambled coding (64B/66B)
  X → eXternal sourced coding (4B/5B, 8B/10B)
Number of Lanes (n):
  Blank space without lane number → defaults as 1-lane
  4 → 4-lanes 

There were no clear names attributed to the interfaced between the blocks
that make up the PHY, the PCS, PMA, PMD. When the MII was the clear
separation point, it was enough to describe the MII type and that was
the initial set of values for the device tree parameter: RGMII, SGMII,
XGMII. One can argue that the correct value still is XGMII, as that is
the real MAC-PHY interface for 10G. Unfortunately, as this interface
does not map to the chip to chip interfaces on our HW, it provides little
to no information on the HW. This is the reason I say we need to describe
the HW. 

> As for CDR, INF-8077i says:
> 
>   "The XFP module shall include a Signal Conditioner based on CDR (Clock
>   Data Recovery) technology for complete regeneration."
> 
> Whereas for SFP modules, SFF-8472 revision 11.4 added optional support
> for CDR on the modules.
> 
> In any case, the CDR is a matter for the module itself, not for the
> host, so it seems that isn't relevent.
> 
> Everything that I've said concerning SFI in my previous email (in date
> order), and why we should not be defining that as a phy_interface_t
> seems to also apply to XFI from what I've read in INF-8077i, and it
> seems my original decision that we should not separately define
> XFI and SFI from 10GBASE-R is well founded.
> 
> Given that meeting these electrical characteristics involves the
> effects of the board, and it is impractical for a board to switch
> between different electrical characteristics at runtime (routing serdes
> lanes to both a SFP+ and XFP cage is impractical due to reflections on
> unterminated paths) I really don't see any reason why we need two
> different phy_interface_t definitions for these.  As mentioned, the
> difference between XFI and SFI is electrical, and involves the board
> layout, which is a platform specific issue.

The signal integrity issues you mention are real but there are solutions
for that, there are high speed mezzanine connectors that allow that level
of flexibility.

[1] https://www.synopsys.com/designware-ip/technical-bulletin/ethernet-dwtb-q117.html

Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Russell King - ARM Linux admin]

On Fri, Jan 03, 2020 at 07:01:50AM +0000, Madalin Bucur (OSS) wrote:
> > -----Original Message-----
> > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > Sent: Monday, December 23, 2019 2:08 PM
> > To: Madalin Bucur <madalin.bucur@nxp.com>
> > Cc: davem@davemloft.net; netdev@vger.kernel.org; andrew@lunn.ch;
> > f.fainelli@gmail.com; hkallweit1@gmail.com; shawnguo@kernel.org;
> > devicetree@vger.kernel.org
> > Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> > 
> > On Thu, Dec 19, 2019 at 06:32:51PM +0000, Madalin Bucur wrote:
> > > 10GBase-R could be used as a common nominator but just as well 10G and
> > > remove the rest while we're at it. There are/may be differences in
> > > features, differences in the way the HW is configured (the most
> > > important aspect) and one should be able to determine what interface
> > > type is in use to properly configure the HW. SFI does not have the CDR
> > > function in the PMD, relying on the PMA signal conditioning vs the XFI
> > > that requires this in the PMD.
> > 
> > I've now found a copy of INF-8077i on the net, which seems to be the
> > document that defines XFI. The definition in there seems to be very
> > similar to SFI in that it is an electrical specification, not a
> > protocol specification, and, just like SFI, it defines the electrical
> > characteristics at the cage, not at the serdes. Therefore, the effects
> > of the board layout come into play to achieve compliance with XFI.
> 
> I think we're missing the point here: we need to start from the device
> tree and that is supposed to describe the board, the hardware, not to
> configure the software. Please re-read the paragraph above in this key:
> the device tree needs to describe the HW features, those electrical
> properties you are discussing above. The fact that we use a certain
> protocol over it, by choice in software, does not change the HW and it
> should not change the device tree describing it.

phy_interface_t does *NOT* describe the electrical properties of the
link; it describes the protocol. The protocol for 10GBASE-R, SFI and
XFI are *all* the same. Therefore, phy_interface_t does *not*
distinguish between these.

Yes, DT may need to describe the electrical properties. That needs to
be done independently of the phy_interface_t and therefore phy-mode
definition.

Just like it is done for SATA interfaces that need the eye mask
(electrical properties of the serdes) adjusted for the board.

> > Just like SFI, XFI can be used with multiple different underlying
> > protocols. I quote:
> > 
> >   "The XFI interface is designed to support SONET OC-192,
> >   IEEE.Std-802.3ae, 10GFC and G.709(OTU-2) applications."
> > 
> > Therefore, to describe 10GBASE-R as "XFI" is most definitely incorrect.
> > 10GBASE-R is just _one_ protocol that can be run over XFI, but it is
> > not the only one.
> 
> Exactly why the chip to chip interface described by the device tree needs
> to be xfi not 10GBASE-R,

Sorry no.

Merely specifying "xfi" does not tell you what you need to do to achieve
XFI compliance at the point defined in INF8077i.  Plus, XFI can also be
protocols _other_ than 10GBASE-R.

Claiming that "XFI" properly defines the interface is utter rubbish. It
does not. XFI defines the electrical characteristics *only* and not
the underlying protocol. It is not limited to 10GBASE-R, but includes
other protocols as well.

XFI is not a phy interface type.  Sorry.

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

Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Russell King - ARM Linux admin]

On Fri, Jan 03, 2020 at 09:27:18AM +0000, Russell King - ARM Linux admin wrote:
> Merely specifying "xfi" does not tell you what you need to do to achieve
> XFI compliance at the point defined in INF8077i.  Plus, XFI can also be
> protocols _other_ than 10GBASE-R.
> 
> Claiming that "XFI" properly defines the interface is utter rubbish. It
> does not. XFI defines the electrical characteristics *only* and not
> the underlying protocol. It is not limited to 10GBASE-R, but includes
> other protocols as well.

Let me quote from INF-8077i, which is the XFP MSA, the document
responsible for defining XFI:

3.1 INTRODUCTION
   XFI signaling is based on high speed low voltage logic, with nominal 100
   Ohms differential impedance and AC coupled in the module. XFI was de-
   veloped with the primary goal of low power and low Electro-Magnetic-In-

   terference (EMI). To satisfy this requirement the nominal differential signal   levels are 500 mV p-p with edge speed control to reduce EMI.

3.2 XFI APPLICATIONS DEFINITION
   The application reference model for XFI, which connects a high speed
   ASIC/SERDES to the XFP module is shown in Figure 4. The XFI interface
   is designed to support SONET OC-192, IEEE.Std-802.3ae, 10GFC and
   G.709(OTU-2) applications. The SERDES is required to meet the applica-
   tion requirements for jitter generation and transfer when interfaced with a
   compliant XFP module through an XFP compliant channel. Modules or

   hosts designed only for 10 Gigabit Ethernet or 10GFC are not required to
   meet more stringent Telecom jitter requirements. XFI supported data
   rates are listed in Table 5. XFP compliant module are not required to sup-
   port all the rates listed in Table 5 in simultaneously.

   Standard                            Description           Nominal Bit Rate     Units
   OC-192/SDH-64                         SONET                     9.95         Gigabaud
   IEEE std-802.3ae             10 Gb/s Ethernet LAN PHY           10.31        Gigabaud
   INCITS/T11 Project 1413-D             10GFC                     10.52        Gigabaud
   ITU G.709(OTU-2)                 OC-192 Over FEC                10.70        Gigabaud
   Emerging                     10Gb/s Ethernet Over G.709         11.09        Gigabaud

So here, we can clearly see that it's possible to run SONET, 10GBASE-R,
10G Fiberchannel, OC-192, and G.709 over XFI, so XFI does not describe
_just_ ethernet. If we're going to be configuring a serdes to output
XFI, we need to know a lot more than just "XFI".

   XFI Compliance points are defined as the following:

   •   A: SerDes transmitter output at ASIC/SerDes package pin on a DUT
       board 3.6 and A.1
   •   B: Host system SerDes output across the host board and connector
       at the Host Compliance Test Card 3.7.1 and A.2
   •   B': XFP transmitter input across the Module Compliance Test Board
       3.8.1 and A.3.
   •   C: Host system input at the Host Compliance Test Card input 3.7.2
       and A.2
   •   C': XFP module output across the Module Compliance Test Board
       3.8.2 and A.3.

   •   D: ASIC/SerDes input package pin on the DUT board 3.6.2 and A.1.

   ASIC/SerDes compliance points are informative.

So the electrical points that count are B, B', C and C'. A and D
are merely "informative".  Hence, compliance with XFI is required
to take the entire platform into account, not just the output of
the serdes/asic.  That means the performance of the PCB needs to
be described in DT if you want to achieve compliance with XFI.
phy_interface_t can't do that.

So, let me re-iterate: neither XFI nor SFI are suitable for
phy_interface_t. XFI defines merely a group of possible protocols
and an electrical specification. It doesn't tell you which of those
protocols you should be using.

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

RE: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Madalin Bucur (OSS)]

> -----Original Message-----
> From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Sent: Friday, January 3, 2020 11:42 AM
> To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> Cc: devicetree@vger.kernel.org; davem@davemloft.net;
> netdev@vger.kernel.org; andrew@lunn.ch; f.fainelli@gmail.com;
> hkallweit1@gmail.com; shawnguo@kernel.org
> Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> 
> On Fri, Jan 03, 2020 at 09:27:18AM +0000, Russell King - ARM Linux admin
> wrote:
> > Merely specifying "xfi" does not tell you what you need to do to achieve
> > XFI compliance at the point defined in INF8077i.  Plus, XFI can also be
> > protocols _other_ than 10GBASE-R.
> >
> > Claiming that "XFI" properly defines the interface is utter rubbish. It
> > does not. XFI defines the electrical characteristics *only* and not
> > the underlying protocol. It is not limited to 10GBASE-R, but includes
> > other protocols as well.
> 
> Let me quote from INF-8077i, which is the XFP MSA, the document
> responsible for defining XFI:
> 
> 3.1 INTRODUCTION
>    XFI signaling is based on high speed low voltage logic, with nominal
> 100
>    Ohms differential impedance and AC coupled in the module. XFI was de-
>    veloped with the primary goal of low power and low Electro-Magnetic-In-
> 
>    terference (EMI). To satisfy this requirement the nominal differential
> signal   levels are 500 mV p-p with edge speed control to reduce EMI.
> 
> 3.2 XFI APPLICATIONS DEFINITION
>    The application reference model for XFI, which connects a high speed
>    ASIC/SERDES to the XFP module is shown in Figure 4. The XFI interface
>    is designed to support SONET OC-192, IEEE.Std-802.3ae, 10GFC and
>    G.709(OTU-2) applications. The SERDES is required to meet the applica-
>    tion requirements for jitter generation and transfer when interfaced
> with a
>    compliant XFP module through an XFP compliant channel. Modules or
> 
>    hosts designed only for 10 Gigabit Ethernet or 10GFC are not required
> to
>    meet more stringent Telecom jitter requirements. XFI supported data
>    rates are listed in Table 5. XFP compliant module are not required to
> sup-
>    port all the rates listed in Table 5 in simultaneously.
> 
>    Standard                            Description           Nominal Bit
> Rate     Units
>    OC-192/SDH-64                         SONET                     9.95
> Gigabaud
>    IEEE std-802.3ae             10 Gb/s Ethernet LAN PHY           10.31
> Gigabaud
>    INCITS/T11 Project 1413-D             10GFC                     10.52
> Gigabaud
>    ITU G.709(OTU-2)                 OC-192 Over FEC                10.70
> Gigabaud
>    Emerging                     10Gb/s Ethernet Over G.709         11.09
> Gigabaud
> 
> So here, we can clearly see that it's possible to run SONET, 10GBASE-R,
> 10G Fiberchannel, OC-192, and G.709 over XFI, so XFI does not describe
> _just_ ethernet. If we're going to be configuring a serdes to output
> XFI, we need to know a lot more than just "XFI".
> 
>    XFI Compliance points are defined as the following:
> 
>    •   A: SerDes transmitter output at ASIC/SerDes package pin on a DUT
>        board 3.6 and A.1
>    •   B: Host system SerDes output across the host board and connector
>        at the Host Compliance Test Card 3.7.1 and A.2
>    •   B': XFP transmitter input across the Module Compliance Test Board
>        3.8.1 and A.3.
>    •   C: Host system input at the Host Compliance Test Card input 3.7.2
>        and A.2
>    •   C': XFP module output across the Module Compliance Test Board
>        3.8.2 and A.3.
> 
>    •   D: ASIC/SerDes input package pin on the DUT board 3.6.2 and A.1.
> 
>    ASIC/SerDes compliance points are informative.
> 
> So the electrical points that count are B, B', C and C'. A and D
> are merely "informative".  Hence, compliance with XFI is required
> to take the entire platform into account, not just the output of
> the serdes/asic.  That means the performance of the PCB needs to
> be described in DT if you want to achieve compliance with XFI.
> phy_interface_t can't do that.
> 
> So, let me re-iterate: neither XFI nor SFI are suitable for
> phy_interface_t. XFI defines merely a group of possible protocols
> and an electrical specification. It doesn't tell you which of those
> protocols you should be using.

The disconnect is you are focused on phy_interface_t and I'm looking at
the device tree as there's where one starts (actually at the device tree
binding document). Your concern is with configuring the HW to use a certain
PCS setting, thus 10GBASE-R, while I'm concerned with the fact the device
tree must not configure software but describe HW. So let's do some archeology
on the matter, to try to understand where this is coming from.

A device tree entry that described the electrical interface between the chip
harboring an Ethernet MAC bloc and another chip that served the purpose of an
Ethernet PHY was needed. In the past this parameter was "phy-connection-type".
We find it detailed in kernel v3.0 in 
Documentation/devicetree/bindings/net/fsl-tsec-phy.txt:52

  - phy-connection-type : a string naming the controller/PHY interface type,
    i.e., "mii" (default), "rmii", "gmii", "rgmii", "rgmii-id", "sgmii",
    "tbi", or "rtbi".  This property is only really needed if the connection
    is of type "rgmii-id", as all other connection types are detected by
    hardware.

Later, in kernel version v4.0 we find it described in 
Documentation/devicetree/bindings/net/ethernet.txt:16

- phy-mode: string, operation mode of the PHY interface; supported values are
  "mii", "gmii", "sgmii", "qsgmii", "tbi", "rev-mii", "rmii", "rgmii", "rgmii-id",
  "rgmii-rxid", "rgmii-txid", "rtbi", "smii", "xgmii"; this is now a de-facto
  standard property;
- phy-connection-type: the same as "phy-mode" property but described in ePAPR;

Now (v5.5-rc3) we find it moved to
Documentation/devicetree/bindings/net/ethernet-controller.yaml:57:

  phy-connection-type:
    description:
      Operation mode of the PHY interface
    enum:
      # There is not a standard bus between the MAC and the PHY,
      # something proprietary is being used to embed the PHY in the
      # MAC.
      - internal
      - mii
      - gmii
      - sgmii
      - qsgmii
      - tbi
      - rev-mii
      - rmii

      # RX and TX delays are added by the MAC when required
      - rgmii

      # RGMII with internal RX and TX delays provided by the PHY,
      # the MAC should not add the RX or TX delays in this case
      - rgmii-id

      # RGMII with internal RX delay provided by the PHY, the MAC
      # should not add an RX delay in this case
      - rgmii-rxid

      # RGMII with internal TX delay provided by the PHY, the MAC
      # should not add an TX delay in this case
      - rgmii-txid
      - rtbi
      - smii
      - xgmii 
      - trgmii
      - 1000base-x
      - 2500base-x
      - rxaui
      - xaui

      # 10GBASE-KR, XFI, SFI
      - 10gbase-kr
      - usxgmii

  phy-mode:
    $ref: "#/properties/phy-connection-type"

At each step, it was changed a bit. It started by describing the actual MII
connection (RGMII, SGMII, XGMII). Later is was changed to denote "operation
mode" of the interface. There is no reference here to PCS configuration (it
could not be as the device tree does not configure but describes the HW). I
see no reference about this device tree entry describing the protocol only
(I'm referring to your second reply on this here). If the device tree binding
does not describe the protocol only, but when it's parsed in software, into
the phy_interface_t it describes only the protocol and not the actual interface
type("mode"), then we have a disconnect here. This type is described as:

/* Interface Mode definitions */
typedef enum {
        PHY_INTERFACE_MODE_NA,
        PHY_INTERFACE_MODE_INTERNAL,
        PHY_INTERFACE_MODE_MII,
        PHY_INTERFACE_MODE_GMII,
        PHY_INTERFACE_MODE_SGMII,
        PHY_INTERFACE_MODE_TBI,
        PHY_INTERFACE_MODE_REVMII,
        PHY_INTERFACE_MODE_RMII,
        PHY_INTERFACE_MODE_RGMII,
        PHY_INTERFACE_MODE_RGMII_ID,
        PHY_INTERFACE_MODE_RGMII_RXID,
        PHY_INTERFACE_MODE_RGMII_TXID,
        PHY_INTERFACE_MODE_RTBI,
        PHY_INTERFACE_MODE_SMII,
        PHY_INTERFACE_MODE_XGMII,
...
} phy_interface_t;


So we can notice that is in sync with the device tree binding document.
Please note the RGMII, RGMII_ID, RGMII_RXID, RGMII_TXID. The only
difference there is in the delays on the electrical connections between
the chips. Take a step back, look at the list of existing entries, at
the history of this and see if it maps to one story or another.

Regards,
Madalin

Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Russell King - ARM Linux admin]

On Fri, Jan 03, 2020 at 12:03:44PM +0000, Madalin Bucur (OSS) wrote:
> > -----Original Message-----
> > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > Sent: Friday, January 3, 2020 11:42 AM
> > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> > Cc: devicetree@vger.kernel.org; davem@davemloft.net;
> > netdev@vger.kernel.org; andrew@lunn.ch; f.fainelli@gmail.com;
> > hkallweit1@gmail.com; shawnguo@kernel.org
> > Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> > 
> > On Fri, Jan 03, 2020 at 09:27:18AM +0000, Russell King - ARM Linux admin
> > wrote:
> > > Merely specifying "xfi" does not tell you what you need to do to achieve
> > > XFI compliance at the point defined in INF8077i.  Plus, XFI can also be
> > > protocols _other_ than 10GBASE-R.
> > >
> > > Claiming that "XFI" properly defines the interface is utter rubbish. It
> > > does not. XFI defines the electrical characteristics *only* and not
> > > the underlying protocol. It is not limited to 10GBASE-R, but includes
> > > other protocols as well.
> > 
> > Let me quote from INF-8077i, which is the XFP MSA, the document
> > responsible for defining XFI:
> > 
> > 3.1 INTRODUCTION
> >    XFI signaling is based on high speed low voltage logic, with nominal
> > 100
> >    Ohms differential impedance and AC coupled in the module. XFI was de-
> >    veloped with the primary goal of low power and low Electro-Magnetic-In-
> > 
> >    terference (EMI). To satisfy this requirement the nominal differential
> > signal   levels are 500 mV p-p with edge speed control to reduce EMI.
> > 
> > 3.2 XFI APPLICATIONS DEFINITION
> >    The application reference model for XFI, which connects a high speed
> >    ASIC/SERDES to the XFP module is shown in Figure 4. The XFI interface
> >    is designed to support SONET OC-192, IEEE.Std-802.3ae, 10GFC and
> >    G.709(OTU-2) applications. The SERDES is required to meet the applica-
> >    tion requirements for jitter generation and transfer when interfaced
> > with a
> >    compliant XFP module through an XFP compliant channel. Modules or
> > 
> >    hosts designed only for 10 Gigabit Ethernet or 10GFC are not required
> > to
> >    meet more stringent Telecom jitter requirements. XFI supported data
> >    rates are listed in Table 5. XFP compliant module are not required to
> > sup-
> >    port all the rates listed in Table 5 in simultaneously.
> > 
> >    Standard                            Description           Nominal Bit
> > Rate     Units
> >    OC-192/SDH-64                         SONET                     9.95
> > Gigabaud
> >    IEEE std-802.3ae             10 Gb/s Ethernet LAN PHY           10.31
> > Gigabaud
> >    INCITS/T11 Project 1413-D             10GFC                     10.52
> > Gigabaud
> >    ITU G.709(OTU-2)                 OC-192 Over FEC                10.70
> > Gigabaud
> >    Emerging                     10Gb/s Ethernet Over G.709         11.09
> > Gigabaud
> > 
> > So here, we can clearly see that it's possible to run SONET, 10GBASE-R,
> > 10G Fiberchannel, OC-192, and G.709 over XFI, so XFI does not describe
> > _just_ ethernet. If we're going to be configuring a serdes to output
> > XFI, we need to know a lot more than just "XFI".
> > 
> >    XFI Compliance points are defined as the following:
> > 
> >    •   A: SerDes transmitter output at ASIC/SerDes package pin on a DUT
> >        board 3.6 and A.1
> >    •   B: Host system SerDes output across the host board and connector
> >        at the Host Compliance Test Card 3.7.1 and A.2
> >    •   B': XFP transmitter input across the Module Compliance Test Board
> >        3.8.1 and A.3.
> >    •   C: Host system input at the Host Compliance Test Card input 3.7.2
> >        and A.2
> >    •   C': XFP module output across the Module Compliance Test Board
> >        3.8.2 and A.3.
> > 
> >    •   D: ASIC/SerDes input package pin on the DUT board 3.6.2 and A.1.
> > 
> >    ASIC/SerDes compliance points are informative.
> > 
> > So the electrical points that count are B, B', C and C'. A and D
> > are merely "informative".  Hence, compliance with XFI is required
> > to take the entire platform into account, not just the output of
> > the serdes/asic.  That means the performance of the PCB needs to
> > be described in DT if you want to achieve compliance with XFI.
> > phy_interface_t can't do that.
> > 
> > So, let me re-iterate: neither XFI nor SFI are suitable for
> > phy_interface_t. XFI defines merely a group of possible protocols
> > and an electrical specification. It doesn't tell you which of those
> > protocols you should be using.
> 
> The disconnect is you are focused on phy_interface_t and I'm looking at
> the device tree as there's where one starts (actually at the device tree
> binding document). Your concern is with configuring the HW to use a certain
> PCS setting, thus 10GBASE-R, while I'm concerned with the fact the device
> tree must not configure software but describe HW. So let's do some archeology
> on the matter, to try to understand where this is coming from.
> 
> A device tree entry that described the electrical interface between the chip
> harboring an Ethernet MAC bloc and another chip that served the purpose of an
> Ethernet PHY was needed. In the past this parameter was "phy-connection-type".
> We find it detailed in kernel v3.0 in 
> Documentation/devicetree/bindings/net/fsl-tsec-phy.txt:52
> 
>   - phy-connection-type : a string naming the controller/PHY interface type,
>     i.e., "mii" (default), "rmii", "gmii", "rgmii", "rgmii-id", "sgmii",
>     "tbi", or "rtbi".  This property is only really needed if the connection
>     is of type "rgmii-id", as all other connection types are detected by
>     hardware.
> 
> Later, in kernel version v4.0 we find it described in 
> Documentation/devicetree/bindings/net/ethernet.txt:16
> 
> - phy-mode: string, operation mode of the PHY interface; supported values are
>   "mii", "gmii", "sgmii", "qsgmii", "tbi", "rev-mii", "rmii", "rgmii", "rgmii-id",
>   "rgmii-rxid", "rgmii-txid", "rtbi", "smii", "xgmii"; this is now a de-facto
>   standard property;
> - phy-connection-type: the same as "phy-mode" property but described in ePAPR;
> 
> Now (v5.5-rc3) we find it moved to
> Documentation/devicetree/bindings/net/ethernet-controller.yaml:57:
> 
>   phy-connection-type:
>     description:
>       Operation mode of the PHY interface
>     enum:
>       # There is not a standard bus between the MAC and the PHY,
>       # something proprietary is being used to embed the PHY in the
>       # MAC.
>       - internal
>       - mii
>       - gmii
>       - sgmii
>       - qsgmii
>       - tbi
>       - rev-mii
>       - rmii
> 
>       # RX and TX delays are added by the MAC when required
>       - rgmii
> 
>       # RGMII with internal RX and TX delays provided by the PHY,
>       # the MAC should not add the RX or TX delays in this case
>       - rgmii-id
> 
>       # RGMII with internal RX delay provided by the PHY, the MAC
>       # should not add an RX delay in this case
>       - rgmii-rxid
> 
>       # RGMII with internal TX delay provided by the PHY, the MAC
>       # should not add an TX delay in this case
>       - rgmii-txid
>       - rtbi
>       - smii
>       - xgmii 
>       - trgmii
>       - 1000base-x
>       - 2500base-x
>       - rxaui
>       - xaui
> 
>       # 10GBASE-KR, XFI, SFI
>       - 10gbase-kr
>       - usxgmii
> 
>   phy-mode:
>     $ref: "#/properties/phy-connection-type"
> 
> At each step, it was changed a bit. It started by describing the actual MII
> connection (RGMII, SGMII, XGMII). Later is was changed to denote "operation
> mode" of the interface. There is no reference here to PCS configuration (it
> could not be as the device tree does not configure but describes the HW). I
> see no reference about this device tree entry describing the protocol only
> (I'm referring to your second reply on this here). If the device tree binding
> does not describe the protocol only, but when it's parsed in software, into
> the phy_interface_t it describes only the protocol and not the actual interface
> type("mode"), then we have a disconnect here. This type is described as:
> 
> /* Interface Mode definitions */
> typedef enum {
>         PHY_INTERFACE_MODE_NA,
>         PHY_INTERFACE_MODE_INTERNAL,
>         PHY_INTERFACE_MODE_MII,
>         PHY_INTERFACE_MODE_GMII,
>         PHY_INTERFACE_MODE_SGMII,
>         PHY_INTERFACE_MODE_TBI,
>         PHY_INTERFACE_MODE_REVMII,
>         PHY_INTERFACE_MODE_RMII,
>         PHY_INTERFACE_MODE_RGMII,
>         PHY_INTERFACE_MODE_RGMII_ID,
>         PHY_INTERFACE_MODE_RGMII_RXID,
>         PHY_INTERFACE_MODE_RGMII_TXID,
>         PHY_INTERFACE_MODE_RTBI,
>         PHY_INTERFACE_MODE_SMII,
>         PHY_INTERFACE_MODE_XGMII,
> ...
> } phy_interface_t;
> 
> 
> So we can notice that is in sync with the device tree binding document.
> Please note the RGMII, RGMII_ID, RGMII_RXID, RGMII_TXID. The only
> difference there is in the delays on the electrical connections between
> the chips. Take a step back, look at the list of existing entries, at
> the history of this and see if it maps to one story or another.

You have the author of the SFP/phylink layers disagreeing with you,
and you have one of the maintainers of phylib also disagreeing with
you.

You seem to grasp at straws to justify your position. Initially, you
were stating that XFI/SFI are defined by the MSAs and using that as
a justification. Now, when I state what the MSAs say, you then go off
and try to justify your position with some 3rd party description of
what the various bits of 10GBASE-*R mean. Now you're trying to make
out that your position is justified by the omission of the term "PCS"
in the kernel's documentation.

I am well aware that DT describes the hardware; I am not a newbie to
kernel development, but have been involved with it for near on 27
years as ARM maintainer, getting into the details of platform
support. So please stop telling me that DT describes the hardware.
I totally accept that.

What I don't accept is the idea that "XFI" needs to be a PHY interface
mode when there's a hell of a lot more to it than just three letters.

If it was that simple, then we could use "SATA" to support all SATA
connections, but we can't. Just like "XFI" or "SFI", sata is a
single channel serdes connection with electrical performance
requirements defined at a certain point. In the case of eSATA, they
are defined at the connector. In order to achieve those performance
requirements, we need to specify the electrical parameters in DT to
achieve compliance. As an example, here is what is required for the
cubox-i4:

&sata {
        status = "okay";
        fsl,transmit-level-mV = <1104>;
        fsl,transmit-boost-mdB = <0>;
        fsl,transmit-atten-16ths = <9>;
        fsl,no-spread-spectrum;
};

These parameters configure the interface to produce a waveform at
the serdes output that, when modified by the characteristics of the
PCB layout, result in compliance with the eSATA connection at the
connector - which is what is required.

XFI and SFI are no different; these are electrical specifications.
The correct set of electrical parameters to meet the specification
is more than just three letters, and it will be board specific.
Hence, on their own, they are completely meaningless.

We have already ascertained that "XFI" and "SFI" do nothing to
describe the format of the protocol - that protocol being one of
10GBASE-W, 10GBASE-R, fibrechannel or G.709.

So, "XFI" or "SFI" as a phy_interface_t is meaningless. As a phy-mode,
it is meaningless. As a phy-connection-type, it is meaningless.

You claim that I'm looking at it from a phy_interface_t perspective.
Sorry, but that is where you are mistaken. I'm looking at it from a
high level, both from the software-protocol point of view and the
hardware-electrical point of view.

XFI and SFI are electrical specifications only. They do not come
close to specifying the protocol. They don't uniquely specify the
baud rate of the data on the link. They don't uniquely specify the
format of that data. You can't have two "XFI" configured devices,
one using XFI/10GBASE-R connected to another using XFI/10GBASE-W
have a working system.

What I might be willing to accept is if we were to introduce
XFI_10GBASER, XFI_10GBASEW, XFI_10GFC, XFI_G709 and their SFI
counterparts - but, there would remain one HUGE problem with that,
which is the total lack of specification of the board characteristics
required to achieve XFI electrical compliance.

As I've stated many times, "XFI" and "SFI" are electrical
specifications which include the platform PCB layout. The platform
part of it needs to be described in DT as well, and you can't do
that by just a simple three-letter "XFI" or "SFI" neumonic. Just like
my SATA example above, it takes much more.

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

Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Andrew Lunn]

> What I might be willing to accept is if we were to introduce
> XFI_10GBASER, XFI_10GBASEW, XFI_10GFC, XFI_G709 and their SFI
> counterparts - but, there would remain one HUGE problem with that,
> which is the total lack of specification of the board characteristics
> required to achieve XFI electrical compliance.

Hi Russell

The four RGMII variants are precedents for mixing protocol and
'electrical' properties, in terms of clock delays. But having four
RGMII variants has been a pain point, implementations getting it
wrong, etc.

So i would avoid mixing them in one property. I would prefer we keep
phy-mode as a protocol property, and we define additional DT
properties to describe the electrical parts of the SERDES interface.

Madalin, what electrical properties do you actually need in DT?  I
guess you need to know if it is using XFI or SFI. But that is only the
start. Do you want to place all the other properties in DT as well, or
are they in a board specific firmware blobs, and you just need to know
if you should use the XFI blob or the SFI blob?

We can probably define a vendor neutral DT property for XFI vs SFI,
but i expect all the other electrical properties are going to be
vendor specific.

       Andrew

RE: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Madalin Bucur (OSS)]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Friday, January 3, 2020 3:35 PM
> To: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Cc: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>;
> devicetree@vger.kernel.org; davem@davemloft.net; netdev@vger.kernel.org;
> f.fainelli@gmail.com; hkallweit1@gmail.com; shawnguo@kernel.org
> Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> 
> > What I might be willing to accept is if we were to introduce
> > XFI_10GBASER, XFI_10GBASEW, XFI_10GFC, XFI_G709 and their SFI
> > counterparts - but, there would remain one HUGE problem with that,
> > which is the total lack of specification of the board characteristics
> > required to achieve XFI electrical compliance.
> 
> Hi Russell
> 
> The four RGMII variants are precedents for mixing protocol and
> 'electrical' properties, in terms of clock delays. But having four
> RGMII variants has been a pain point, implementations getting it
> wrong, etc.

I always thought that a separate property would of simplified code a lot,
there are things to be done for RGMII and one ends up with ugly code:

        /* check RGMII support */
        if (iface == PHY_INTERFACE_MODE_RGMII ||
            iface == PHY_INTERFACE_MODE_RGMII_ID ||
            iface == PHY_INTERFACE_MODE_RGMII_RXID ||
            iface == PHY_INTERFACE_MODE_RGMII_TXID)

That's the reason I've commented on the recent patch set, that maybe it's
time to stop and fix this mess properly, provided that we find a better
solution.

> So i would avoid mixing them in one property. I would prefer we keep
> phy-mode as a protocol property, and we define additional DT
> properties to describe the electrical parts of the SERDES interface.
> 
> Madalin, what electrical properties do you actually need in DT?  I
> guess you need to know if it is using XFI or SFI. But that is only the
> start. Do you want to place all the other properties in DT as well, or
> are they in a board specific firmware blobs, and you just need to know
> if you should use the XFI blob or the SFI blob?

I do not have other electrical propertied to set. Nor do I have a different
blob to load. We're using u-boot as a a bootloader. It supports SFI as a
valid mode for a long time now:

commit d11e9347461cff9ce89e6e65764f73fad0f19c6f
Author: Stefan Roese <sr@denx.de>
Date:   Thu Feb 23 11:58:26 2017 +0100

    net: include/phy.h: Add new PHY interface modes

    This patch adds the new PHY interface modes XAUI, RXAUI and SFI that will
    be used by the PPv2.2 support in the Marvell mvpp2 ethernet driver.

    Signed-off-by: Stefan Roese <sr@denx.de>
    Cc: Stefan Chulski <stefanc@marvell.com>
    Cc: Kostya Porotchkin <kostap@marvell.com>
    Cc: Nadav Haklai <nadavh@marvell.com>
    Acked-by: Joe Hershberger <joe.hershberger@ni.com>

There are many things pushed down to u-boot so the Linux driver has less
work to do, that's one of the reasons there is little left there. Ideally
this dependency should be removed but then we'd need to make a clearer
distinction. As you've noticed, currently I don't even need to distinguish
XFI from SFI because for basic scenarios the code does the same thing.
Problem is, if I select a common denominator now, and later I need to
make this distinction, I'll need to update device trees, code, etc. Just
like "xgmii" was just fine as a placeholder until recently. I'd rather use
a correct description now.

> We can probably define a vendor neutral DT property for XFI vs SFI,
> but i expect all the other electrical properties are going to be
> vendor specific.
> 
>        Andrew

Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Andrew Lunn]

> There are many things pushed down to u-boot so the Linux driver has less
> work to do, that's one of the reasons there is little left there.

I prefer barebox. Which is a problem, since it sounds like Ethernet
will be broken on your boards if i swap to it.

If you are going to offload setting up the hardware, please do it to
firmware. That is independent of the bootloader. The Marvell SoCs do
this for their low level SERDES setup, making SMC calls into the
firmware.

https://patchwork.kernel.org/cover/10880297/

> Ideally this dependency should be removed but then we'd need to make
> a clearer distinction. As you've noticed, currently I don't even
> need to distinguish XFI from SFI because for basic scenarios the
> code does the same thing.  Problem is, if I select a common
> denominator now, and later I need to make this distinction, I'll
> need to update device trees, code, etc. Just like "xgmii" was just
> fine as a placeholder until recently. I'd rather use a correct
> description now.

So it seems like you need two properties. You need a property to tell
your bootloader how to configure the electrical properties of the
SERDES, XFI, SFI, etc.

And you need a property to configure the protocol running over the
SERDES, which is phy-mode.

	Andrew

RE: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Madalin Bucur (OSS)]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Friday, January 3, 2020 7:17 PM
> To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>;
> devicetree@vger.kernel.org; davem@davemloft.net; netdev@vger.kernel.org;
> f.fainelli@gmail.com; hkallweit1@gmail.com; shawnguo@kernel.org
> Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> 
> > There are many things pushed down to u-boot so the Linux driver has
> less
> > work to do, that's one of the reasons there is little left there.
> 
> I prefer barebox. Which is a problem, since it sounds like Ethernet
> will be broken on your boards if i swap to it.

It may be, but customers had success porting that support of various
other bootloaders. Most of this is loading the firmware and device-tree
fix-ups but there may be some minor tweaks of the platforms done in
u-boot that need porting.

> If you are going to offload setting up the hardware, please do it to
> firmware. That is independent of the bootloader. The Marvell SoCs do
> this for their low level SERDES setup, making SMC calls into the
> firmware.
> 
> https://patchwork.kernel.org/cover/10880297/

Firmware has the (dis)advantage of usually being closed source, having
settings done in open source code makes everyone's life easier, but the
one's who needs to send that upstream :)
Firmware does allow any hacks to go unnoticed so it may be preferable by
some in that respect. Ideally it should all be done in the drivers, in
plain sight, imho.
 
> > Ideally this dependency should be removed but then we'd need to make
> > a clearer distinction. As you've noticed, currently I don't even
> > need to distinguish XFI from SFI because for basic scenarios the
> > code does the same thing.  Problem is, if I select a common
> > denominator now, and later I need to make this distinction, I'll
> > need to update device trees, code, etc. Just like "xgmii" was just
> > fine as a placeholder until recently. I'd rather use a correct
> > description now.
> 
> So it seems like you need two properties. You need a property to tell
> your bootloader how to configure the electrical properties of the
> SERDES, XFI, SFI, etc.

That's what the RCW (reset configuration word) does, it's the first thing
read from FLASH by the SoC on boot (you can consider it a sort of firmware).

> 
> And you need a property to configure the protocol running over the
> SERDES, which is phy-mode.
> 
> 	Andrew

The protocol is clear and related to the speed, some other aspects we do
need to control, such as AN on the system interface, that we need to
disable for 2500Gbps Ethernet, or to know which PCS we need to talk to
in case of QSGMII and so on. Also many resources (FIFOs, internal controller
tasks, DMAs) are sized accordingly to the interface type by SW. The protocol
is the least of my concerns here. I'm using the electrical interface type
to derive a series of parameters for the SW (including what PCS block I talk
to). Used to be "xgmii" (incorrect), today it may be just as well "10gbase-kr"
(still incorrect), tomorrow it may be "10gbase-r" (incomplete, it says nothing
about the HW) or "xfi" (sufficient for this particular HW, maybe incomplete for
others). So we do need to decide if we are going to separate the HW/electrical
description that has a place in the device tree and the SW configuration that
has no place there.

Regards,
Madalin

RE: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Madalin Bucur (OSS)]

> -----Original Message-----
> From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Sent: Friday, January 3, 2020 2:53 PM
> To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> Cc: devicetree@vger.kernel.org; davem@davemloft.net;
> netdev@vger.kernel.org; andrew@lunn.ch; f.fainelli@gmail.com;
> hkallweit1@gmail.com; shawnguo@kernel.org
> Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> 
> On Fri, Jan 03, 2020 at 12:03:44PM +0000, Madalin Bucur (OSS) wrote:
> > > -----Original Message-----
> > > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > > Sent: Friday, January 3, 2020 11:42 AM
> > > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> > > Cc: devicetree@vger.kernel.org; davem@davemloft.net;
> > > netdev@vger.kernel.org; andrew@lunn.ch; f.fainelli@gmail.com;
> > > hkallweit1@gmail.com; shawnguo@kernel.org
> > > Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> > >
> > > On Fri, Jan 03, 2020 at 09:27:18AM +0000, Russell King - ARM Linux
> admin
> > > wrote:
> > > > Merely specifying "xfi" does not tell you what you need to do to
> achieve
> > > > XFI compliance at the point defined in INF8077i.  Plus, XFI can also
> be
> > > > protocols _other_ than 10GBASE-R.
> > > >
> > > > Claiming that "XFI" properly defines the interface is utter rubbish.
> It
> > > > does not. XFI defines the electrical characteristics *only* and not
> > > > the underlying protocol. It is not limited to 10GBASE-R, but
> includes
> > > > other protocols as well.
> > >
> > > Let me quote from INF-8077i, which is the XFP MSA, the document
> > > responsible for defining XFI:
> > >
> > > 3.1 INTRODUCTION
> > >    XFI signaling is based on high speed low voltage logic, with
> nominal
> > > 100
> > >    Ohms differential impedance and AC coupled in the module. XFI was
> de-
> > >    veloped with the primary goal of low power and low Electro-
> Magnetic-In-
> > >
> > >    terference (EMI). To satisfy this requirement the nominal
> differential
> > > signal   levels are 500 mV p-p with edge speed control to reduce EMI.
> > >
> > > 3.2 XFI APPLICATIONS DEFINITION
> > >    The application reference model for XFI, which connects a high
> speed
> > >    ASIC/SERDES to the XFP module is shown in Figure 4. The XFI
> interface
> > >    is designed to support SONET OC-192, IEEE.Std-802.3ae, 10GFC and
> > >    G.709(OTU-2) applications. The SERDES is required to meet the
> applica-
> > >    tion requirements for jitter generation and transfer when
> interfaced
> > > with a
> > >    compliant XFP module through an XFP compliant channel. Modules or
> > >
> > >    hosts designed only for 10 Gigabit Ethernet or 10GFC are not
> required
> > > to
> > >    meet more stringent Telecom jitter requirements. XFI supported data
> > >    rates are listed in Table 5. XFP compliant module are not required
> to
> > > sup-
> > >    port all the rates listed in Table 5 in simultaneously.
> > >
> > >    Standard                            Description           Nominal
> Bit
> > > Rate     Units
> > >    OC-192/SDH-64                         SONET
> 9.95
> > > Gigabaud
> > >    IEEE std-802.3ae             10 Gb/s Ethernet LAN PHY
> 10.31
> > > Gigabaud
> > >    INCITS/T11 Project 1413-D             10GFC
> 10.52
> > > Gigabaud
> > >    ITU G.709(OTU-2)                 OC-192 Over FEC
> 10.70
> > > Gigabaud
> > >    Emerging                     10Gb/s Ethernet Over G.709
> 11.09
> > > Gigabaud
> > >
> > > So here, we can clearly see that it's possible to run SONET, 10GBASE-
> R,
> > > 10G Fiberchannel, OC-192, and G.709 over XFI, so XFI does not describe
> > > _just_ ethernet. If we're going to be configuring a serdes to output
> > > XFI, we need to know a lot more than just "XFI".
> > >
> > >    XFI Compliance points are defined as the following:
> > >
> > >    •   A: SerDes transmitter output at ASIC/SerDes package pin on a
> DUT
> > >        board 3.6 and A.1
> > >    •   B: Host system SerDes output across the host board and
> connector
> > >        at the Host Compliance Test Card 3.7.1 and A.2
> > >    •   B': XFP transmitter input across the Module Compliance Test
> Board
> > >        3.8.1 and A.3.
> > >    •   C: Host system input at the Host Compliance Test Card input
> 3.7.2
> > >        and A.2
> > >    •   C': XFP module output across the Module Compliance Test Board
> > >        3.8.2 and A.3.
> > >
> > >    •   D: ASIC/SerDes input package pin on the DUT board 3.6.2 and
> A.1.
> > >
> > >    ASIC/SerDes compliance points are informative.
> > >
> > > So the electrical points that count are B, B', C and C'. A and D
> > > are merely "informative".  Hence, compliance with XFI is required
> > > to take the entire platform into account, not just the output of
> > > the serdes/asic.  That means the performance of the PCB needs to
> > > be described in DT if you want to achieve compliance with XFI.
> > > phy_interface_t can't do that.
> > >
> > > So, let me re-iterate: neither XFI nor SFI are suitable for
> > > phy_interface_t. XFI defines merely a group of possible protocols
> > > and an electrical specification. It doesn't tell you which of those
> > > protocols you should be using.
> >
> > The disconnect is you are focused on phy_interface_t and I'm looking at
> > the device tree as there's where one starts (actually at the device tree
> > binding document). Your concern is with configuring the HW to use a
> certain
> > PCS setting, thus 10GBASE-R, while I'm concerned with the fact the
> device
> > tree must not configure software but describe HW. So let's do some
> archeology
> > on the matter, to try to understand where this is coming from.
> >
> > A device tree entry that described the electrical interface between the
> chip
> > harboring an Ethernet MAC bloc and another chip that served the purpose
> of an
> > Ethernet PHY was needed. In the past this parameter was "phy-connection-
> type".
> > We find it detailed in kernel v3.0 in
> > Documentation/devicetree/bindings/net/fsl-tsec-phy.txt:52
> >
> >   - phy-connection-type : a string naming the controller/PHY interface
> type,
> >     i.e., "mii" (default), "rmii", "gmii", "rgmii", "rgmii-id", "sgmii",
> >     "tbi", or "rtbi".  This property is only really needed if the
> connection
> >     is of type "rgmii-id", as all other connection types are detected by
> >     hardware.
> >
> > Later, in kernel version v4.0 we find it described in
> > Documentation/devicetree/bindings/net/ethernet.txt:16
> >
> > - phy-mode: string, operation mode of the PHY interface; supported
> values are
> >   "mii", "gmii", "sgmii", "qsgmii", "tbi", "rev-mii", "rmii", "rgmii",
> "rgmii-id",
> >   "rgmii-rxid", "rgmii-txid", "rtbi", "smii", "xgmii"; this is now a de-
> facto
> >   standard property;
> > - phy-connection-type: the same as "phy-mode" property but described in
> ePAPR;
> >
> > Now (v5.5-rc3) we find it moved to
> > Documentation/devicetree/bindings/net/ethernet-controller.yaml:57:
> >
> >   phy-connection-type:
> >     description:
> >       Operation mode of the PHY interface
> >     enum:
> >       # There is not a standard bus between the MAC and the PHY,
> >       # something proprietary is being used to embed the PHY in the
> >       # MAC.
> >       - internal
> >       - mii
> >       - gmii
> >       - sgmii
> >       - qsgmii
> >       - tbi
> >       - rev-mii
> >       - rmii
> >
> >       # RX and TX delays are added by the MAC when required
> >       - rgmii
> >
> >       # RGMII with internal RX and TX delays provided by the PHY,
> >       # the MAC should not add the RX or TX delays in this case
> >       - rgmii-id
> >
> >       # RGMII with internal RX delay provided by the PHY, the MAC
> >       # should not add an RX delay in this case
> >       - rgmii-rxid
> >
> >       # RGMII with internal TX delay provided by the PHY, the MAC
> >       # should not add an TX delay in this case
> >       - rgmii-txid
> >       - rtbi
> >       - smii
> >       - xgmii
> >       - trgmii
> >       - 1000base-x
> >       - 2500base-x
> >       - rxaui
> >       - xaui
> >
> >       # 10GBASE-KR, XFI, SFI
> >       - 10gbase-kr
> >       - usxgmii
> >
> >   phy-mode:
> >     $ref: "#/properties/phy-connection-type"
> >
> > At each step, it was changed a bit. It started by describing the actual
> MII
> > connection (RGMII, SGMII, XGMII). Later is was changed to denote
> "operation
> > mode" of the interface. There is no reference here to PCS configuration
> (it
> > could not be as the device tree does not configure but describes the
> HW). I
> > see no reference about this device tree entry describing the protocol
> only
> > (I'm referring to your second reply on this here). If the device tree
> binding
> > does not describe the protocol only, but when it's parsed in software,
> into
> > the phy_interface_t it describes only the protocol and not the actual
> interface
> > type("mode"), then we have a disconnect here. This type is described as:
> >
> > /* Interface Mode definitions */
> > typedef enum {
> >         PHY_INTERFACE_MODE_NA,
> >         PHY_INTERFACE_MODE_INTERNAL,
> >         PHY_INTERFACE_MODE_MII,
> >         PHY_INTERFACE_MODE_GMII,
> >         PHY_INTERFACE_MODE_SGMII,
> >         PHY_INTERFACE_MODE_TBI,
> >         PHY_INTERFACE_MODE_REVMII,
> >         PHY_INTERFACE_MODE_RMII,
> >         PHY_INTERFACE_MODE_RGMII,
> >         PHY_INTERFACE_MODE_RGMII_ID,
> >         PHY_INTERFACE_MODE_RGMII_RXID,
> >         PHY_INTERFACE_MODE_RGMII_TXID,
> >         PHY_INTERFACE_MODE_RTBI,
> >         PHY_INTERFACE_MODE_SMII,
> >         PHY_INTERFACE_MODE_XGMII,
> > ...
> > } phy_interface_t;
> >
> >
> > So we can notice that is in sync with the device tree binding document.
> > Please note the RGMII, RGMII_ID, RGMII_RXID, RGMII_TXID. The only
> > difference there is in the delays on the electrical connections between
> > the chips. Take a step back, look at the list of existing entries, at
> > the history of this and see if it maps to one story or another.
> 
> You have the author of the SFP/phylink layers disagreeing with you,
> and you have one of the maintainers of phylib also disagreeing with
> you.

Hi, I'm aware of the parties involved in the discussion but the messages
are meant for the whole community so the details included are meant for
that. We're having a conversation on the mailing list, not in private
and the end result of this will be of use, I hope, to others that have the
same dilemmas, be them justified or not. I had the pleasure to meet Andrew
in person a couple of years ago and I'm looking forward to meet you as well.

> You seem to grasp at straws to justify your position. Initially, you
> were stating that XFI/SFI are defined by the MSAs and using that as
> a justification. Now, when I state what the MSAs say, you then go off
> and try to justify your position with some 3rd party description of
> what the various bits of 10GBASE-*R mean. Now you're trying to make
> out that your position is justified by the omission of the term "PCS"
> in the kernel's documentation.
> 
> I am well aware that DT describes the hardware; I am not a newbie to
> kernel development, but have been involved with it for near on 27
> years as ARM maintainer, getting into the details of platform
> support. So please stop telling me that DT describes the hardware.
> I totally accept that.
> 
> What I don't accept is the idea that "XFI" needs to be a PHY interface
> mode when there's a hell of a lot more to it than just three letters.
> 
> If it was that simple, then we could use "SATA" to support all SATA
> connections, but we can't. Just like "XFI" or "SFI", sata is a
> single channel serdes connection with electrical performance
> requirements defined at a certain point. In the case of eSATA, they
> are defined at the connector. In order to achieve those performance
> requirements, we need to specify the electrical parameters in DT to
> achieve compliance. As an example, here is what is required for the
> cubox-i4:
> 
> &sata {
>         status = "okay";
>         fsl,transmit-level-mV = <1104>;
>         fsl,transmit-boost-mdB = <0>;
>         fsl,transmit-atten-16ths = <9>;
>         fsl,no-spread-spectrum;
> };
> 
> These parameters configure the interface to produce a waveform at
> the serdes output that, when modified by the characteristics of the
> PCB layout, result in compliance with the eSATA connection at the
> connector - which is what is required.
> 
> XFI and SFI are no different; these are electrical specifications.
> The correct set of electrical parameters to meet the specification
> is more than just three letters, and it will be board specific.
> Hence, on their own, they are completely meaningless.
> 
> We have already ascertained that "XFI" and "SFI" do nothing to
> describe the format of the protocol - that protocol being one of
> 10GBASE-W, 10GBASE-R, fibrechannel or G.709.
> 
> So, "XFI" or "SFI" as a phy_interface_t is meaningless. As a phy-mode,
> it is meaningless. As a phy-connection-type, it is meaningless.
> 
> You claim that I'm looking at it from a phy_interface_t perspective.
> Sorry, but that is where you are mistaken. I'm looking at it from a
> high level, both from the software-protocol point of view and the
> hardware-electrical point of view.
> 
> XFI and SFI are electrical specifications only. They do not come
> close to specifying the protocol. They don't uniquely specify the
> baud rate of the data on the link. They don't uniquely specify the
> format of that data. You can't have two "XFI" configured devices,
> one using XFI/10GBASE-R connected to another using XFI/10GBASE-W
> have a working system.
> 
> What I might be willing to accept is if we were to introduce
> XFI_10GBASER, XFI_10GBASEW, XFI_10GFC, XFI_G709 and their SFI
> counterparts - but, there would remain one HUGE problem with that,
> which is the total lack of specification of the board characteristics
> required to achieve XFI electrical compliance.
> 
> As I've stated many times, "XFI" and "SFI" are electrical
> specifications which include the platform PCB layout. The platform
> part of it needs to be described in DT as well, and you can't do
> that by just a simple three-letter "XFI" or "SFI" neumonic. Just like
> my SATA example above, it takes much more.
> 
> --
> 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

This conversation started a while ago, when I tried to introduce XFI and SFI
as valid compatibles describing the phy-connection-type device tree parameter
for some platforms. Alternatives are "xgmii", "usxgmii" or "10gbase-kr". I can
no longer use "xgmii" because at least one PHY is rejecting that as a PHY
"interface":

	WARN(phydev->interface == PHY_INTERFACE_MODE_XGMII,...

Thus the search for a correct value. Reading the PHY datasheet, these are
providing support on the line side for 10GBASET/5GBASE-T/2.5GBASE-T/
1000BASE-T/100BASE-TX while on the system side they claim:

 * High-Performance full KR (with autonegotiation)/
 * XFI/USXGMII/2500BASE-X/SGMII I/F w/ AC-JTAC 
 * Capable of rate adapting all rates into KR/XFI via PAUSE and 100M/1G
into 2500BASE-X

The SoC that connects to one such PHY can have his SERDES configured in XFI
mode, using something called a reset configuration word (it's hardcoded).
Other supported modes are SGMII, 10GBASE-KR, new devices will support SFI.
Here is a sample from an available datasheet:

Eight SerDes lanes for high-speed peripheral interfaces
– Three PCI Express 3.0 controllers
– One Serial ATA (SATA 6 Gbit/s) controller
– Up to two XFI (10 GbE) interfaces
– Up to five SGMII interfaces supporting 1000 Mbps
– Up to three SGMII interfaces supporting 2500 Mbps
– Up to one QSGMII interface
– Supports 10GBase-KR
– Supports 1000Base-KX

No USXGMII, unfortunately. There is a 2500 Mbps mode, but that's not the
topic of today's discussion. The actual mode used, according to both SoC
and PHY datasheets is XFI.

You say this is not sufficient, and it may not be, but for this PHY and for
this SoC pair in particular, it's all it's needed. There are not that many
things to set up in SW but some things need to be put in place and a
indication that this mode is used is required.

Sorry, I could not agree with the recommendation to describe this as
10GBASE-KR and your explanation that you meant 10GBASE-R but made a typo
is making it even less desirable - when the typo will be addressed I'd
need to update again drivers and, worse, device trees to indicate a PHY
connection type that I'm "using".

We have a solution that supports backplane (10gbase_kr - 802.3ap-2007,
with AN) on custom HW with the same SoC (and others). That mode needs to
be distinguished from the XFI one, as there are different settings to be
made.

Regards,
Madalin

Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Russell King - ARM Linux admin]

On Fri, Jan 03, 2020 at 03:57:45PM +0000, Madalin Bucur (OSS) wrote:
> Hi, I'm aware of the parties involved in the discussion but the messages
> are meant for the whole community so the details included are meant for
> that. We're having a conversation on the mailing list, not in private
> and the end result of this will be of use, I hope, to others that have the
> same dilemmas, be them justified or not. I had the pleasure to meet Andrew
> in person a couple of years ago and I'm looking forward to meet you as well.

The most frustrating part of this discussion for me is this:

It started by you stating that you wanted to use XFI as a
phy_interface_t, and through that initial discussion you were stating
that XFI is defined by the XFP MSA - which is quite right. XFI is the
electrical interface used for XFP modules, as defined by the XFP MSA.
SFI is the electrical interface used for SFP+ modules, as defined by
the SFP+ MSA (_not_ the SFP MSA, if you want to be pedantic.)

When I found a copy of the XFP MSA, along with its definition of what
the XFI interface is, _all_ the points that I brought up about the
XFI interface have been mostly ignored, such as my statement that XFI
includes the electrical characteristics of the platform, because the
stated specifications are valid only at the XFP mating connector.

I also found that both XFI and SFI do not define the format of the
electrical signals; indeed, there are several baud rates that are
used which result in different bit periods of the signal, and even
a case where 10GBASE-R can be carried on XFI in two different forms.

Rather than provide a counter-argument, you have instead switched to
using other arguments to justify your position, such as using a
breakdown of what 10GBASE-*R means.

Yet again, as I see from the below, you have failed to counter any
of the points that I have raised below.

> > You seem to grasp at straws to justify your position. Initially, you
> > were stating that XFI/SFI are defined by the MSAs and using that as
> > a justification. Now, when I state what the MSAs say, you then go off
> > and try to justify your position with some 3rd party description of
> > what the various bits of 10GBASE-*R mean. Now you're trying to make
> > out that your position is justified by the omission of the term "PCS"
> > in the kernel's documentation.
> > 
> > I am well aware that DT describes the hardware; I am not a newbie to
> > kernel development, but have been involved with it for near on 27
> > years as ARM maintainer, getting into the details of platform
> > support. So please stop telling me that DT describes the hardware.
> > I totally accept that.
> > 
> > What I don't accept is the idea that "XFI" needs to be a PHY interface
> > mode when there's a hell of a lot more to it than just three letters.
> > 
> > If it was that simple, then we could use "SATA" to support all SATA
> > connections, but we can't. Just like "XFI" or "SFI", sata is a
> > single channel serdes connection with electrical performance
> > requirements defined at a certain point. In the case of eSATA, they
> > are defined at the connector. In order to achieve those performance
> > requirements, we need to specify the electrical parameters in DT to
> > achieve compliance. As an example, here is what is required for the
> > cubox-i4:
> > 
> > &sata {
> >         status = "okay";
> >         fsl,transmit-level-mV = <1104>;
> >         fsl,transmit-boost-mdB = <0>;
> >         fsl,transmit-atten-16ths = <9>;
> >         fsl,no-spread-spectrum;
> > };
> > 
> > These parameters configure the interface to produce a waveform at
> > the serdes output that, when modified by the characteristics of the
> > PCB layout, result in compliance with the eSATA connection at the
> > connector - which is what is required.
> > 
> > XFI and SFI are no different; these are electrical specifications.
> > The correct set of electrical parameters to meet the specification
> > is more than just three letters, and it will be board specific.
> > Hence, on their own, they are completely meaningless.
> > 
> > We have already ascertained that "XFI" and "SFI" do nothing to
> > describe the format of the protocol - that protocol being one of
> > 10GBASE-W, 10GBASE-R, fibrechannel or G.709.
> > 
> > So, "XFI" or "SFI" as a phy_interface_t is meaningless. As a phy-mode,
> > it is meaningless. As a phy-connection-type, it is meaningless.
> > 
> > You claim that I'm looking at it from a phy_interface_t perspective.
> > Sorry, but that is where you are mistaken. I'm looking at it from a
> > high level, both from the software-protocol point of view and the
> > hardware-electrical point of view.
> > 
> > XFI and SFI are electrical specifications only. They do not come
> > close to specifying the protocol. They don't uniquely specify the
> > baud rate of the data on the link. They don't uniquely specify the
> > format of that data. You can't have two "XFI" configured devices,
> > one using XFI/10GBASE-R connected to another using XFI/10GBASE-W
> > have a working system.
> > 
> > What I might be willing to accept is if we were to introduce
> > XFI_10GBASER, XFI_10GBASEW, XFI_10GFC, XFI_G709 and their SFI
> > counterparts - but, there would remain one HUGE problem with that,
> > which is the total lack of specification of the board characteristics
> > required to achieve XFI electrical compliance.
> > 
> > As I've stated many times, "XFI" and "SFI" are electrical
> > specifications which include the platform PCB layout. The platform
> > part of it needs to be described in DT as well, and you can't do
> > that by just a simple three-letter "XFI" or "SFI" neumonic. Just like
> > my SATA example above, it takes much more.
> > 
> > --
> > 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
> 
> This conversation started a while ago, when I tried to introduce XFI and SFI
> as valid compatibles describing the phy-connection-type device tree parameter
> for some platforms. Alternatives are "xgmii", "usxgmii" or "10gbase-kr". I can
> no longer use "xgmii" because at least one PHY is rejecting that as a PHY
> "interface":
> 
> 	WARN(phydev->interface == PHY_INTERFACE_MODE_XGMII,...

Hang on, so because a PHY driver now rejects XGMII, you're saying you
can't use it. Let's have a look at what XGMII is. It is a four data
line interface using 8b/10b encoding.

Instead of using that, you want to use XFI instead, which is a single
data line interface. The encoding is not defined, except by the
underlying protocol. In the case of clause 49 Ethernet over XFI, aka
10GBASE-R, that is using 64b/66b encoding.

You've gone on about DT describing the hardware - and yes it does
that. Describing the PHY interface correctly is part of that, and
that includes the number of data lanes and the encoding of the
signal.

So, if you were using "xgmii", and now wish to use "xfi" for the
_same_ hardware, something is very very wrong with how you are
approaching this. The two interfaces are _entirely_ different. If
it is four data lanes, then it is not XFI. If it is a single data
lane, then it _may_ be 10GBASE-R using something _like_ XFI
electrical levels.

> Thus the search for a correct value. Reading the PHY datasheet, these are
> providing support on the line side for 10GBASET/5GBASE-T/2.5GBASE-T/
> 1000BASE-T/100BASE-TX while on the system side they claim:
> 
>  * High-Performance full KR (with autonegotiation)/
>  * XFI/USXGMII/2500BASE-X/SGMII I/F w/ AC-JTAC 
>  * Capable of rate adapting all rates into KR/XFI via PAUSE and 100M/1G
> into 2500BASE-X
> 
> The SoC that connects to one such PHY can have his SERDES configured in XFI
> mode, using something called a reset configuration word (it's hardcoded).
> Other supported modes are SGMII, 10GBASE-KR, new devices will support SFI.

Right, but, as I've stated several times already, XFI in itself is
insufficient to describe the interface. What you are seeing in the
various datasheets is an appropriation of a term that is actually
meaningless.

Let's go over what XFI as defined by the XFP MSA is again:

- It is an electrical specification with parameters defined at the
  XFP socket for both the XFP host system, and the XFP module. It
  is a fact that the electrical signals reach the socket over the
  PCB, and the PCB will modify the characteristics of those signals.

- Electrical recommendations are given for the Serdes/ASIC inputs
  and outputs; these are "informative" which means they are not
  a specification.

- XFI can carry several different protocols: 10GBASE-W, 10GBASE-R,
  10G Fibrechannel, 10GBASE-R over G.709.

So, a PHY or SoC datasheet claiming that it has a XFI interface is
actually an abuse of the term:

- There is no XFP socket involved
- They certainly do not involve the electrical characteristics of
  the PCB that their device will be placed upon
- They haven't specified which of the four protocols that XFI can
  carry is to be used

The reality is, which I'm saying having spent quite a long time with
the Marvell Armada 8040 SoC, Marvell 88x3310 PHY, and SFP+ modules
including spending a lot of time analysing the eye pattern and
adjusting the electrical characteristics to bring it more into
compliance, is that this term "XFI" that is banded around so much in
datasheets bears very little relation to the XFI as specified in the
XFP MSA.

This can be seen if I compare the serdes interface electrical
specification for the Marvell 88x3310 PHY with that given in the XFP
MSA. The two are not identical; there are some subtle differences
between what is given for the 88x3310 PHY and the XFP MSA for the
"Serdes/ASIC" test points A and D, but on the whole, it meets the
"informative" "recommended" parameters given in the XFP MSA for XFI.

That doesn't mean that it _is_ XFI. As stated above, XFI could be
carrying 10GBASE-W or any of the other three.

In this instance, the PHY only accepts and generates 10GBASE-R over
its "XFI" interface.

This is my point: this PHY, which uses "XFI" through out its datasheet
is actually using XFI electrical characteristics _with_ 10GBASE-R as
the underlying protocol (because this PHY is designed to be an
Ethernet LAN PHY.)

To see how silly this is, please read some XFP module specifications,
which are, after all, what the XFI interface is there to support given
that XFI is defined by the XFP MSA:

https://eoptolink.com/pdf/XFP-ZR-1310-70km-optical-transceiver.pdf

Here, we have an optical transceiver with an XFI interface which can
carry any of 10GBASE-ZW, 10GBASE-ZR, 10G Ethernet over G.709. Each
of these is a different baud rate, and requires the XFI to run at a
slightly different speed.

So, "XFI" is nothing but a mis-nomer when it comes to PHY and SoC
datasheets; it is an incomplete specification of what they actually
support, which is 10GBASE-R with a compliance with the XFI Serdes/
ASIC test point *recommendations*.

> You say this is not sufficient, and it may not be, but for this PHY and for
> this SoC pair in particular, it's all it's needed. There are not that many
> things to set up in SW but some things need to be put in place and a
> indication that this mode is used is required.

So, let's do a thought experiment. Let's say that we adopt your
proposal to use "XFI" for this instance.  Great.

How do we handle 10GBASE-W over XFI when that comes along? That's
still XFI, but it's incompatible with _your_ version of XFI. "But
my datasheet says it is so it must be correct".  Yes, it's as
correct as your current usage of XFI.

And that's my point: XFI is not 10GBASE-R. It is not 10GBASE-W. XFI
is an electrical specification which may be one of several different
protocols.

To properly describe it, you need _both_ the electrical specification
and the protocol specification - but manufacturers omit that, because
it's implied in the datasheeet.  If you're looking at a LAN 10G PHY
that says XFI, it is actually meaning 10GBASE-R over a link that
complies with the XFI recommendations.

If you're looking at a SONET OC-192 device which has an XFI interface,
it is actually meaning 10GBASE-W over a link that complies with the
XFI recommendations.

I hope this finally clears up exactly the point I'm making.

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

RE: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Madalin Bucur (OSS)]

> -----Original Message-----
> From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Sent: Friday, January 3, 2020 7:20 PM
> To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> Cc: devicetree@vger.kernel.org; davem@davemloft.net;
> netdev@vger.kernel.org; andrew@lunn.ch; f.fainelli@gmail.com;
> hkallweit1@gmail.com; shawnguo@kernel.org
> Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> 
> On Fri, Jan 03, 2020 at 03:57:45PM +0000, Madalin Bucur (OSS) wrote:
> > Hi, I'm aware of the parties involved in the discussion but the
> messages
> > are meant for the whole community so the details included are meant for
> > that. We're having a conversation on the mailing list, not in private
> > and the end result of this will be of use, I hope, to others that have
> the
> > same dilemmas, be them justified or not. I had the pleasure to meet
> Andrew
> > in person a couple of years ago and I'm looking forward to meet you as
> well.
> 
> The most frustrating part of this discussion for me is this:
> 
> It started by you stating that you wanted to use XFI as a
> phy_interface_t, and through that initial discussion you were stating
> that XFI is defined by the XFP MSA - which is quite right. XFI is the
> electrical interface used for XFP modules, as defined by the XFP MSA.
> SFI is the electrical interface used for SFP+ modules, as defined by
> the SFP+ MSA (_not_ the SFP MSA, if you want to be pedantic.)
> 
> When I found a copy of the XFP MSA, along with its definition of what
> the XFI interface is, _all_ the points that I brought up about the
> XFI interface have been mostly ignored, such as my statement that XFI
> includes the electrical characteristics of the platform, because the
> stated specifications are valid only at the XFP mating connector.
> 
> I also found that both XFI and SFI do not define the format of the
> electrical signals; indeed, there are several baud rates that are
> used which result in different bit periods of the signal, and even
> a case where 10GBASE-R can be carried on XFI in two different forms.
>

Glad you found these things, they are the base of our discussion here.
 
> Rather than provide a counter-argument, you have instead switched to
> using other arguments to justify your position, such as using a
> breakdown of what 10GBASE-*R means.
> 
> Yet again, as I see from the below, you have failed to counter any
> of the points that I have raised below.

You missed my argument about the device tree describing the HW (thus the
wires, electrical aspects too) and not configuring a certain protocol (the
device tree does not configure HW, it describes HW).
 
> > > You seem to grasp at straws to justify your position. Initially, you
> > > were stating that XFI/SFI are defined by the MSAs and using that as
> > > a justification. Now, when I state what the MSAs say, you then go off
> > > and try to justify your position with some 3rd party description of
> > > what the various bits of 10GBASE-*R mean. Now you're trying to make
> > > out that your position is justified by the omission of the term "PCS"
> > > in the kernel's documentation.

From your wording I sense you are taking this a bit too personal, I'm trying
to state my views on this matter. The phy-connection-type parameter was
introduced to allow describing the (electrical) interface with the PHY, not
to configure a certain protocol over the same wires. It's up to the SW to do
that, not the device tree. In this respect, saying that I have a XFI connection
up to a certain PHY's pins may be an excursion outside the MSA (no SFP connector)
but it's not the first "non-standard" (if it really were a standard) thing
done in the industry - see 2500 Gbps Ethernet or even USXGMII. Things move
faster than standards at times, we can wait for a clear standard or spec
(that may never appear) or try to do as good as possible of a job describing
what's on the table today. I do not even insist that mixing xfi, sfi (or xaui,
tbi, qsgmii) with 1000base-x, 2500base-x is a good idea, but having some and
rejecting others seems arbitrary to me. Please note the protocols seem to be
the ones joining late an electrical bus party here...

> > > I am well aware that DT describes the hardware; I am not a newbie to
> > > kernel development, but have been involved with it for near on 27
> > > years as ARM maintainer, getting into the details of platform
> > > support. So please stop telling me that DT describes the hardware.
> > > I totally accept that.
> > >
> > > What I don't accept is the idea that "XFI" needs to be a PHY
> interface
> > > mode when there's a hell of a lot more to it than just three letters.
> > >
> > > If it was that simple, then we could use "SATA" to support all SATA
> > > connections, but we can't. Just like "XFI" or "SFI", sata is a
> > > single channel serdes connection with electrical performance
> > > requirements defined at a certain point. In the case of eSATA, they
> > > are defined at the connector. In order to achieve those performance
> > > requirements, we need to specify the electrical parameters in DT to
> > > achieve compliance. As an example, here is what is required for the
> > > cubox-i4:
> > >
> > > &sata {
> > >         status = "okay";
> > >         fsl,transmit-level-mV = <1104>;
> > >         fsl,transmit-boost-mdB = <0>;
> > >         fsl,transmit-atten-16ths = <9>;
> > >         fsl,no-spread-spectrum;
> > > };
> > >
> > > These parameters configure the interface to produce a waveform at
> > > the serdes output that, when modified by the characteristics of the
> > > PCB layout, result in compliance with the eSATA connection at the
> > > connector - which is what is required.
> > >
> > > XFI and SFI are no different; these are electrical specifications.
> > > The correct set of electrical parameters to meet the specification
> > > is more than just three letters, and it will be board specific.
> > > Hence, on their own, they are completely meaningless.
> > >
> > > We have already ascertained that "XFI" and "SFI" do nothing to
> > > describe the format of the protocol - that protocol being one of
> > > 10GBASE-W, 10GBASE-R, fibrechannel or G.709.
> > >
> > > So, "XFI" or "SFI" as a phy_interface_t is meaningless. As a phy-
> mode,
> > > it is meaningless. As a phy-connection-type, it is meaningless.
> > >
> > > You claim that I'm looking at it from a phy_interface_t perspective.
> > > Sorry, but that is where you are mistaken. I'm looking at it from a
> > > high level, both from the software-protocol point of view and the
> > > hardware-electrical point of view.
> > >
> > > XFI and SFI are electrical specifications only. They do not come
> > > close to specifying the protocol. They don't uniquely specify the
> > > baud rate of the data on the link. They don't uniquely specify the
> > > format of that data. You can't have two "XFI" configured devices,
> > > one using XFI/10GBASE-R connected to another using XFI/10GBASE-W
> > > have a working system.
> > >
> > > What I might be willing to accept is if we were to introduce
> > > XFI_10GBASER, XFI_10GBASEW, XFI_10GFC, XFI_G709 and their SFI
> > > counterparts - but, there would remain one HUGE problem with that,
> > > which is the total lack of specification of the board characteristics
> > > required to achieve XFI electrical compliance.
> > >
> > > As I've stated many times, "XFI" and "SFI" are electrical
> > > specifications which include the platform PCB layout. The platform
> > > part of it needs to be described in DT as well, and you can't do
> > > that by just a simple three-letter "XFI" or "SFI" neumonic. Just like
> > > my SATA example above, it takes much more.
> > >
> > > --
> > > 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
> >
> > This conversation started a while ago, when I tried to introduce XFI
> and SFI
> > as valid compatibles describing the phy-connection-type device tree
> parameter
> > for some platforms. Alternatives are "xgmii", "usxgmii" or "10gbase-
> kr". I can
> > no longer use "xgmii" because at least one PHY is rejecting that as a
> PHY
> > "interface":
> >
> > 	WARN(phydev->interface == PHY_INTERFACE_MODE_XGMII,...
> 
> Hang on, so because a PHY driver now rejects XGMII, you're saying you
> can't use it. Let's have a look at what XGMII is. It is a four data
> line interface using 8b/10b encoding.
> 
> Instead of using that, you want to use XFI instead, which is a single
> data line interface. The encoding is not defined, except by the
> underlying protocol. In the case of clause 49 Ethernet over XFI, aka
> 10GBASE-R, that is using 64b/66b encoding.
> 
> You've gone on about DT describing the hardware - and yes it does
> that. Describing the PHY interface correctly is part of that, and
> that includes the number of data lanes and the encoding of the
> signal.
> 
> So, if you were using "xgmii", and now wish to use "xfi" for the
> _same_ hardware, something is very very wrong with how you are
> approaching this. The two interfaces are _entirely_ different. If
> it is four data lanes, then it is not XFI. If it is a single data
> lane, then it _may_ be 10GBASE-R using something _like_ XFI
> electrical levels.

You could have saved typing that much if you read my previous statement
about xgmii being used incorrectly there. See:

https://marc.info/?l=linux-netdev&m=157709606631603&w=2
https://marc.info/?l=linux-netdev&m=157709607031607&w=2

Did you review these patches in the the patch set?

> > Thus the search for a correct value. Reading the PHY datasheet, these
> are
> > providing support on the line side for 10GBASET/5GBASE-T/2.5GBASE-T/
> > 1000BASE-T/100BASE-TX while on the system side they claim:
> >
> >  * High-Performance full KR (with autonegotiation)/
> >  * XFI/USXGMII/2500BASE-X/SGMII I/F w/ AC-JTAC
> >  * Capable of rate adapting all rates into KR/XFI via PAUSE and 100M/1G
> > into 2500BASE-X
> >
> > The SoC that connects to one such PHY can have his SERDES configured in
> XFI
> > mode, using something called a reset configuration word (it's
> hardcoded).
> > Other supported modes are SGMII, 10GBASE-KR, new devices will support
> SFI.
> 
> Right, but, as I've stated several times already, XFI in itself is
> insufficient to describe the interface. What you are seeing in the
> various datasheets is an appropriation of a term that is actually
> meaningless.
> 
> Let's go over what XFI as defined by the XFP MSA is again:
> 
> - It is an electrical specification with parameters defined at the
>   XFP socket for both the XFP host system, and the XFP module. It
>   is a fact that the electrical signals reach the socket over the
>   PCB, and the PCB will modify the characteristics of those signals.
> 
> - Electrical recommendations are given for the Serdes/ASIC inputs
>   and outputs; these are "informative" which means they are not
>   a specification.
> 
> - XFI can carry several different protocols: 10GBASE-W, 10GBASE-R,
>   10G Fibrechannel, 10GBASE-R over G.709.
> 
> So, a PHY or SoC datasheet claiming that it has a XFI interface is
> actually an abuse of the term:
> 
> - There is no XFP socket involved
> - They certainly do not involve the electrical characteristics of
>   the PCB that their device will be placed upon
> - They haven't specified which of the four protocols that XFI can
>   carry is to be used

I agree with you on this, it looks like there is a HW "conspiration" here
to make thinks work together and I assume that if another vendor would
try to build from scratch something interoperable would need more than
what's available in the (public, at least) datasheets.

> The reality is, which I'm saying having spent quite a long time with
> the Marvell Armada 8040 SoC, Marvell 88x3310 PHY, and SFP+ modules
> including spending a lot of time analysing the eye pattern and
> adjusting the electrical characteristics to bring it more into
> compliance, is that this term "XFI" that is banded around so much in
> datasheets bears very little relation to the XFI as specified in the
> XFP MSA.

I'm sorry to hear that there's so much trouble with that particular HW,
we're not having such issues here but we did spend some time making the
backplane drivers work in any conditions for both 1000BASE-KX, 10GBASE-KR
or 40GBASE-KR4. HW retimers do help in some designs (and provide another
headache in mapping them in the already complicated phylib/phylink world).
 
> This can be seen if I compare the serdes interface electrical
> specification for the Marvell 88x3310 PHY with that given in the XFP
> MSA. The two are not identical; there are some subtle differences
> between what is given for the 88x3310 PHY and the XFP MSA for the
> "Serdes/ASIC" test points A and D, but on the whole, it meets the
> "informative" "recommended" parameters given in the XFP MSA for XFI.
> 
> That doesn't mean that it _is_ XFI. As stated above, XFI could be
> carrying 10GBASE-W or any of the other three.
> 
> In this instance, the PHY only accepts and generates 10GBASE-R over
> its "XFI" interface.

It's the same for us but if it were to support both, selecting one versus
the other would be a SW configuration decision, the board would look exactly
the same for both options thus the device tree should be the same too, not
changed according to the SW configuration scenario.
 
> This is my point: this PHY, which uses "XFI" through out its datasheet
> is actually using XFI electrical characteristics _with_ 10GBASE-R as
> the underlying protocol (because this PHY is designed to be an
> Ethernet LAN PHY.)
> 
> To see how silly this is, please read some XFP module specifications,
> which are, after all, what the XFI interface is there to support given
> that XFI is defined by the XFP MSA:
> 
> https://eoptolink.com/pdf/XFP-ZR-1310-70km-optical-transceiver.pdf
> 
> Here, we have an optical transceiver with an XFI interface which can
> carry any of 10GBASE-ZW, 10GBASE-ZR, 10G Ethernet over G.709. Each
> of these is a different baud rate, and requires the XFI to run at a
> slightly different speed.
> 
> So, "XFI" is nothing but a mis-nomer when it comes to PHY and SoC
> datasheets; it is an incomplete specification of what they actually
> support, which is 10GBASE-R with a compliance with the XFI Serdes/
> ASIC test point *recommendations*.

Agree, but the same XFI is perfectly suited to denote the electrical
interface in the device tree.

> > You say this is not sufficient, and it may not be, but for this PHY and
> for
> > this SoC pair in particular, it's all it's needed. There are not that
> many
> > things to set up in SW but some things need to be put in place and a
> > indication that this mode is used is required.
> 
> So, let's do a thought experiment. Let's say that we adopt your
> proposal to use "XFI" for this instance.  Great.
> 
> How do we handle 10GBASE-W over XFI when that comes along? That's
> still XFI, but it's incompatible with _your_ version of XFI. "But
> my datasheet says it is so it must be correct".  Yes, it's as
> correct as your current usage of XFI.
> 
> And that's my point: XFI is not 10GBASE-R. It is not 10GBASE-W. XFI
> is an electrical specification which may be one of several different
> protocols.

Indeed, I could not agree more. So we need to describe the link as XFI
in the device tree and find ways to configure the protocol used on top,
if required by that particular device. I know my "sufficient for me now"
stance is not going to help that case but as that is still hypothetical,
we may decide to deal with it when it occurs. Or build something new,
that splits the electrical bus denomination from the protocol used over it.

> To properly describe it, you need _both_ the electrical specification
> and the protocol specification - but manufacturers omit that, because
> it's implied in the datasheeet.  If you're looking at a LAN 10G PHY
> that says XFI, it is actually meaning 10GBASE-R over a link that
> complies with the XFI recommendations.
> 
> If you're looking at a SONET OC-192 device which has an XFI interface,
> it is actually meaning 10GBASE-W over a link that complies with the
> XFI recommendations.
> 
> I hope this finally clears up exactly the point I'm making.

So we do agree on the problem but not on the way forward. Let's work
on that.

Regards,
Madalin

Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Andrew Lunn]

> You missed my argument about the device tree describing the HW (thus the
> wires, electrical aspects too) and not configuring a certain protocol (the
> device tree does not configure HW, it describes HW).

Hi Madalin

You have lots of different points here. I'm just picking out one.

I would say this is a grey area. You need to ensure both devices on
the XFI bus are using the same protocol. There are a few ways you
could do this:

The MAC and the PHY tells phylink what each is capable of, and phylink
picks a common protocol.

Leave it to the boot loader/firmware and cross your fingers.

Make a design decision, this board will use protocol X, and put that
in device tree. It is describing how we expect the hardware to be
used.

The Marvell SERDES interfaces are pretty generic. They can be used for
SATA, USB3, or networking. But these are all protocols running on top
of SERDES. So would you argue we cannot describe in device tree that
one SERDES is to be used for USB and another for SATA?

    Andrew

RE: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

[Madalin Bucur (OSS)]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Monday, January 6, 2020 3:58 PM
> To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> Cc: Russell King - ARM Linux admin <linux@armlinux.org.uk>;
> devicetree@vger.kernel.org; davem@davemloft.net; netdev@vger.kernel.org;
> f.fainelli@gmail.com; hkallweit1@gmail.com; shawnguo@kernel.org
> Subject: Re: [PATCH 1/6] net: phy: add interface modes for XFI, SFI
> 
> > You missed my argument about the device tree describing the HW (thus
> the
> > wires, electrical aspects too) and not configuring a certain protocol
> (the
> > device tree does not configure HW, it describes HW).
> 
> Hi Madalin
> 
> You have lots of different points here. I'm just picking out one.
> 
> I would say this is a grey area. You need to ensure both devices on
> the XFI bus are using the same protocol. There are a few ways you
> could do this:
> 
> The MAC and the PHY tells phylink what each is capable of, and phylink
> picks a common protocol.
> 
> Leave it to the boot loader/firmware and cross your fingers.
> 
> Make a design decision, this board will use protocol X, and put that
> in device tree. It is describing how we expect the hardware to be
> used.
> 
> The Marvell SERDES interfaces are pretty generic. They can be used for
> SATA, USB3, or networking. But these are all protocols running on top
> of SERDES. So would you argue we cannot describe in device tree that
> one SERDES is to be used for USB and another for SATA?
> 
>     Andrew

That's the case with the SERDES on most (all?) SoCs nowadays. I say we
need to describe them as they are used, which, if I believe the SoC
documentation authors, the PHY documentation authors, the board
documentation author, in my case it's XFI. If it were USB3, SATA, and
a description was needed, why should you not describe it as such? I
see a difference between the XFI and the protocol on top. Not much
data will come through a system if the eye diagram is not open, although
the protocol is the same. Unless you get both right, it does not work.
In my case, the 10GBASE-R part is implicit/redundant information, the
electrical part has the potential to add some information to it. On the
other hand, it's not like someone will solder there a different PHY and
hope it will work because it says "xfi" or it says "10gbase-r" somewhere.
There are a hundred other conditions to be met: voltages, power and reset
sequencing, clock frequencies and stability and so on. It was all taken
care by the board designer, we just need to describe it so that SW can
make the best use of it.

I wanted to describe the interfaces as close to the documentation a
developer adding support for a custom board would be likely to use.
For now a blind replace "xgmii" to "10gbase-r" would be enough to get
things going as they already are and avoid a warning in the AQR probing.
But I feel that we'd still be off from the best description we can
and the above mentioned developer would be left a bit puzzled by that.

I also have the concern that this device tree parameter started life
as an MII bus type enumerator and now we say it should describe the
protocol and only that. Sure, XFI is not an MII interface type, as
it's not aligned to the MAC-PHY interface but rather a PHY sub-block
interface but its frequent use by the industry I thought could warrant
a place for it in that list, unless we decide to build something better.

While we're at it, should we have XAUI, RXAUI there or 10GBASE-X4,
10GBASE-X2?

Madalin

[PATCH 2/6] arm64: dts: ls104xardb: set correct PHY interface mode

[Madalin Bucur]

From: Madalin Bucur <madalin.bucur@nxp.com>

The DPAA 1 based LS1043ARDB and LS1046ARDB boards are using
XFI for the 10G interfaces. Since at the moment of the addition
of the first DPAA platforms the only 10G PHY interface type used
was XGMII, although the boards were actually using XFI, they were
wrongly declared as xgmii. This has propagated along the DPAA
family of SoCs, all 10G interfaces being declared wrongly as
XGMII. This patch addresses the problem for the ARM based DPAA
SoCs. After the introduction of XFI PHY interface type we can
address this issue.

Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts | 2 +-
 arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts
index 4223a2352d45..8d99dd423720 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts
@@ -139,7 +139,7 @@
 
 	ethernet@f0000 { /* 10GEC1 */
 		phy-handle = <&aqr105_phy>;
-		phy-connection-type = "xgmii";
+		phy-connection-type = "xfi";
 	};
 
 	mdio@fc000 {
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts
index 0c742befb761..e193e2bc2b91 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts
@@ -151,12 +151,12 @@
 
 	ethernet@f0000 { /* 10GEC1 */
 		phy-handle = <&aqr106_phy>;
-		phy-connection-type = "xgmii";
+		phy-connection-type = "xfi";
 	};
 
 	ethernet@f2000 { /* 10GEC2 */
 		fixed-link = <0 1 1000 0 0>;
-		phy-connection-type = "xgmii";
+		phy-connection-type = "xfi";
 	};
 
 	mdio@fc000 {
-- 
2.1.0

Re: [PATCH 2/6] arm64: dts: ls104xardb: set correct PHY interface mode

[Andrew Lunn]

On Thu, Dec 19, 2019 at 05:21:17PM +0200, Madalin Bucur wrote:
> From: Madalin Bucur <madalin.bucur@nxp.com>
> 
> The DPAA 1 based LS1043ARDB and LS1046ARDB boards are using
> XFI for the 10G interfaces. Since at the moment of the addition
> of the first DPAA platforms the only 10G PHY interface type used
> was XGMII, although the boards were actually using XFI, they were
> wrongly declared as xgmii. This has propagated along the DPAA
> family of SoCs, all 10G interfaces being declared wrongly as
> XGMII. This patch addresses the problem for the ARM based DPAA
> SoCs. After the introduction of XFI PHY interface type we can
> address this issue.

Hi Madalin

This patch should come at the end, after you have added support for
these new modes. Otherwise anybody doing a git bisect could land on
code which has broken ethernet.

     Andrew

RE: [PATCH 2/6] arm64: dts: ls104xardb: set correct PHY interface mode

[Madalin Bucur (OSS)]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Thursday, December 19, 2019 6:06 PM
> To: Madalin Bucur <madalin.bucur@nxp.com>
> Cc: davem@davemloft.net; netdev@vger.kernel.org; linux@armlinux.org.uk;
> f.fainelli@gmail.com; hkallweit1@gmail.com; shawnguo@kernel.org;
> devicetree@vger.kernel.org
> Subject: Re: [PATCH 2/6] arm64: dts: ls104xardb: set correct PHY interface
> mode
> 
> On Thu, Dec 19, 2019 at 05:21:17PM +0200, Madalin Bucur wrote:
> > From: Madalin Bucur <madalin.bucur@nxp.com>
> >
> > The DPAA 1 based LS1043ARDB and LS1046ARDB boards are using
> > XFI for the 10G interfaces. Since at the moment of the addition
> > of the first DPAA platforms the only 10G PHY interface type used
> > was XGMII, although the boards were actually using XFI, they were
> > wrongly declared as xgmii. This has propagated along the DPAA
> > family of SoCs, all 10G interfaces being declared wrongly as
> > XGMII. This patch addresses the problem for the ARM based DPAA
> > SoCs. After the introduction of XFI PHY interface type we can
> > address this issue.
> 
> Hi Madalin
> 
> This patch should come at the end, after you have added support for
> these new modes. Otherwise anybody doing a git bisect could land on
> code which has broken ethernet.
> 
>      Andrew

Hi Andrew,

you are right, if the device tree would arrive intact to the kernel that
could occur. Unfortunately there are some fix-ups performed by the bootloader
(u-boot) that are likely to override the compatible so this ordering issue
is shadowed by that other problem.

Madalin

[PATCH 3/6] net: fsl/fman: rename IF_MODE_XGMII to IF_MODE_10G

[Madalin Bucur]

As the only 10G PHY interface type defined at the moment the code
was developed was XGMII, although the PHY interface mode used was
not XGMII, XGMII was used everywhere in the code. This patch
renames the 10G interface mode to remove the ambiguity as more
PHY interface modes are possible.

Signed-off-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
---
 drivers/net/ethernet/freescale/fman/fman_memac.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c b/drivers/net/ethernet/freescale/fman/fman_memac.c
index 41c6fa200e74..e1901874c19f 100644
--- a/drivers/net/ethernet/freescale/fman/fman_memac.c
+++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
@@ -110,7 +110,7 @@ do {									\
 /* Interface Mode Register (IF_MODE) */
 
 #define IF_MODE_MASK		0x00000003 /* 30-31 Mask on i/f mode bits */
-#define IF_MODE_XGMII		0x00000000 /* 30-31 XGMII (10G) interface */
+#define IF_MODE_10G		0x00000000 /* 30-31 10G interface */
 #define IF_MODE_GMII		0x00000002 /* 30-31 GMII (1G) interface */
 #define IF_MODE_RGMII		0x00000004
 #define IF_MODE_RGMII_AUTO	0x00008000
@@ -440,7 +440,7 @@ static int init(struct memac_regs __iomem *regs, struct memac_cfg *cfg,
 	tmp = 0;
 	switch (phy_if) {
 	case PHY_INTERFACE_MODE_XGMII:
-		tmp |= IF_MODE_XGMII;
+		tmp |= IF_MODE_10G;
 		break;
 	default:
 		tmp |= IF_MODE_GMII;
-- 
2.1.0

[PATCH 4/6] net: fsl/fman: add support for PHY_INTERFACE_MODE_XFI

[Madalin Bucur]

Add support in the code for the XFI PHY interface mode. Although
the interfaces did not use XGMII, the device trees described the
interface type as XGMII and the code was written with XGMI as an
indicator for 10G. The patch does not remove XGMII as a transition
time will be required until the device trees, fix-ups performed
by bootloaders are all aligned.

Signed-off-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
---
 drivers/net/ethernet/freescale/fman/fman_memac.c | 4 +++-
 drivers/net/ethernet/freescale/fman/mac.c        | 8 +++++---
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c b/drivers/net/ethernet/freescale/fman/fman_memac.c
index e1901874c19f..d0b12efadd6c 100644
--- a/drivers/net/ethernet/freescale/fman/fman_memac.c
+++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
@@ -439,6 +439,7 @@ static int init(struct memac_regs __iomem *regs, struct memac_cfg *cfg,
 	/* IF_MODE */
 	tmp = 0;
 	switch (phy_if) {
+	case PHY_INTERFACE_MODE_XFI:
 	case PHY_INTERFACE_MODE_XGMII:
 		tmp |= IF_MODE_10G;
 		break;
@@ -454,7 +455,8 @@ static int init(struct memac_regs __iomem *regs, struct memac_cfg *cfg,
 
 	/* TX_FIFO_SECTIONS */
 	tmp = 0;
-	if (phy_if == PHY_INTERFACE_MODE_XGMII) {
+	if (phy_if == PHY_INTERFACE_MODE_XFI ||
+	    phy_if == PHY_INTERFACE_MODE_XGMII) {
 		if (slow_10g_if) {
 			tmp |= (TX_FIFO_SECTIONS_TX_AVAIL_SLOW_10G |
 				TX_FIFO_SECTIONS_TX_EMPTY_DEFAULT_10G);
diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index f0806ace1ae2..2944188c19b3 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -221,7 +221,7 @@ static int memac_initialization(struct mac_device *mac_dev)
 	set_fman_mac_params(mac_dev, &params);
 
 	if (priv->max_speed == SPEED_10000)
-		params.phy_if = PHY_INTERFACE_MODE_XGMII;
+		params.phy_if = PHY_INTERFACE_MODE_XFI;
 
 	mac_dev->fman_mac = memac_config(&params);
 	if (!mac_dev->fman_mac) {
@@ -540,7 +540,8 @@ static const u16 phy2speed[] = {
 	[PHY_INTERFACE_MODE_RGMII_TXID]	= SPEED_1000,
 	[PHY_INTERFACE_MODE_RTBI]		= SPEED_1000,
 	[PHY_INTERFACE_MODE_QSGMII]		= SPEED_1000,
-	[PHY_INTERFACE_MODE_XGMII]		= SPEED_10000
+	[PHY_INTERFACE_MODE_XGMII]		= SPEED_10000,
+	[PHY_INTERFACE_MODE_XFI]		= SPEED_10000,
 };
 
 static struct platform_device *dpaa_eth_add_device(int fman_id,
@@ -798,7 +799,8 @@ static int mac_probe(struct platform_device *_of_dev)
 		mac_dev->if_support |= SUPPORTED_1000baseT_Full;
 
 	/* The 10G interface only supports one mode */
-	if (mac_dev->phy_if == PHY_INTERFACE_MODE_XGMII)
+	if (mac_dev->phy_if == PHY_INTERFACE_MODE_XFI ||
+	    mac_dev->phy_if == PHY_INTERFACE_MODE_XGMII)
 		mac_dev->if_support = SUPPORTED_10000baseT_Full;
 
 	/* Get the rest of the PHY information */
-- 
2.1.0

[PATCH 5/6] net: fsl/fman: add support for PHY_INTERFACE_MODE_SFI

[Madalin Bucur]

Add support for the SFI PHY interface mode.

Signed-off-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
---
 drivers/net/ethernet/freescale/fman/fman_memac.c | 2 ++
 drivers/net/ethernet/freescale/fman/mac.c        | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c b/drivers/net/ethernet/freescale/fman/fman_memac.c
index d0b12efadd6c..09fdec935bf2 100644
--- a/drivers/net/ethernet/freescale/fman/fman_memac.c
+++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
@@ -440,6 +440,7 @@ static int init(struct memac_regs __iomem *regs, struct memac_cfg *cfg,
 	tmp = 0;
 	switch (phy_if) {
 	case PHY_INTERFACE_MODE_XFI:
+	case PHY_INTERFACE_MODE_SFI:
 	case PHY_INTERFACE_MODE_XGMII:
 		tmp |= IF_MODE_10G;
 		break;
@@ -456,6 +457,7 @@ static int init(struct memac_regs __iomem *regs, struct memac_cfg *cfg,
 	/* TX_FIFO_SECTIONS */
 	tmp = 0;
 	if (phy_if == PHY_INTERFACE_MODE_XFI ||
+	    phy_if == PHY_INTERFACE_MODE_SFI ||
 	    phy_if == PHY_INTERFACE_MODE_XGMII) {
 		if (slow_10g_if) {
 			tmp |= (TX_FIFO_SECTIONS_TX_AVAIL_SLOW_10G |
diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
index 2944188c19b3..5e6317742c38 100644
--- a/drivers/net/ethernet/freescale/fman/mac.c
+++ b/drivers/net/ethernet/freescale/fman/mac.c
@@ -542,6 +542,7 @@ static const u16 phy2speed[] = {
 	[PHY_INTERFACE_MODE_QSGMII]		= SPEED_1000,
 	[PHY_INTERFACE_MODE_XGMII]		= SPEED_10000,
 	[PHY_INTERFACE_MODE_XFI]		= SPEED_10000,
+	[PHY_INTERFACE_MODE_SFI]		= SPEED_10000,
 };
 
 static struct platform_device *dpaa_eth_add_device(int fman_id,
@@ -800,6 +801,7 @@ static int mac_probe(struct platform_device *_of_dev)
 
 	/* The 10G interface only supports one mode */
 	if (mac_dev->phy_if == PHY_INTERFACE_MODE_XFI ||
+	    mac_dev->phy_if == PHY_INTERFACE_MODE_SFI ||
 	    mac_dev->phy_if == PHY_INTERFACE_MODE_XGMII)
 		mac_dev->if_support = SUPPORTED_10000baseT_Full;
 
-- 
2.1.0

Re: [PATCH 5/6] net: fsl/fman: add support for PHY_INTERFACE_MODE_SFI

[Russell King - ARM Linux admin]

On Thu, Dec 19, 2019 at 05:21:20PM +0200, Madalin Bucur wrote:
> Add support for the SFI PHY interface mode.
> 
> Signed-off-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
> ---
>  drivers/net/ethernet/freescale/fman/fman_memac.c | 2 ++
>  drivers/net/ethernet/freescale/fman/mac.c        | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c b/drivers/net/ethernet/freescale/fman/fman_memac.c
> index d0b12efadd6c..09fdec935bf2 100644
> --- a/drivers/net/ethernet/freescale/fman/fman_memac.c
> +++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
> @@ -440,6 +440,7 @@ static int init(struct memac_regs __iomem *regs, struct memac_cfg *cfg,
>  	tmp = 0;
>  	switch (phy_if) {
>  	case PHY_INTERFACE_MODE_XFI:
> +	case PHY_INTERFACE_MODE_SFI:

No difference between these two.

>  	case PHY_INTERFACE_MODE_XGMII:
>  		tmp |= IF_MODE_10G;
>  		break;
> @@ -456,6 +457,7 @@ static int init(struct memac_regs __iomem *regs, struct memac_cfg *cfg,
>  	/* TX_FIFO_SECTIONS */
>  	tmp = 0;
>  	if (phy_if == PHY_INTERFACE_MODE_XFI ||
> +	    phy_if == PHY_INTERFACE_MODE_SFI ||

Again, no difference between these two.

>  	    phy_if == PHY_INTERFACE_MODE_XGMII) {
>  		if (slow_10g_if) {
>  			tmp |= (TX_FIFO_SECTIONS_TX_AVAIL_SLOW_10G |
> diff --git a/drivers/net/ethernet/freescale/fman/mac.c b/drivers/net/ethernet/freescale/fman/mac.c
> index 2944188c19b3..5e6317742c38 100644
> --- a/drivers/net/ethernet/freescale/fman/mac.c
> +++ b/drivers/net/ethernet/freescale/fman/mac.c
> @@ -542,6 +542,7 @@ static const u16 phy2speed[] = {
>  	[PHY_INTERFACE_MODE_QSGMII]		= SPEED_1000,
>  	[PHY_INTERFACE_MODE_XGMII]		= SPEED_10000,
>  	[PHY_INTERFACE_MODE_XFI]		= SPEED_10000,
> +	[PHY_INTERFACE_MODE_SFI]		= SPEED_10000,

Again, no difference between these two.

>  };
>  
>  static struct platform_device *dpaa_eth_add_device(int fman_id,
> @@ -800,6 +801,7 @@ static int mac_probe(struct platform_device *_of_dev)
>  
>  	/* The 10G interface only supports one mode */
>  	if (mac_dev->phy_if == PHY_INTERFACE_MODE_XFI ||
> +	    mac_dev->phy_if == PHY_INTERFACE_MODE_SFI ||

Again, no difference between these two.

I just don't see the point of perpetuating the XFI and SFI names for
something that is just plain 10GBASE-R.

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

RE: [PATCH 5/6] net: fsl/fman: add support for PHY_INTERFACE_MODE_SFI

[Madalin Bucur (OSS)]

> -----Original Message-----
> From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Sent: Thursday, December 19, 2019 7:30 PM
> To: Madalin Bucur <madalin.bucur@nxp.com>
> Cc: davem@davemloft.net; netdev@vger.kernel.org; andrew@lunn.ch;
> f.fainelli@gmail.com; hkallweit1@gmail.com; shawnguo@kernel.org;
> devicetree@vger.kernel.org; Madalin Bucur (OSS)
> <madalin.bucur@oss.nxp.com>
> Subject: Re: [PATCH 5/6] net: fsl/fman: add support for
> PHY_INTERFACE_MODE_SFI
> 
> On Thu, Dec 19, 2019 at 05:21:20PM +0200, Madalin Bucur wrote:
> > Add support for the SFI PHY interface mode.
> >
> > Signed-off-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
> > ---
> >  drivers/net/ethernet/freescale/fman/fman_memac.c | 2 ++
> >  drivers/net/ethernet/freescale/fman/mac.c        | 2 ++
> >  2 files changed, 4 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c
> b/drivers/net/ethernet/freescale/fman/fman_memac.c
> > index d0b12efadd6c..09fdec935bf2 100644
> > --- a/drivers/net/ethernet/freescale/fman/fman_memac.c
> > +++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
> > @@ -440,6 +440,7 @@ static int init(struct memac_regs __iomem *regs,
> struct memac_cfg *cfg,
> >  	tmp = 0;
> >  	switch (phy_if) {
> >  	case PHY_INTERFACE_MODE_XFI:
> > +	case PHY_INTERFACE_MODE_SFI:
> 
> No difference between these two.

Until we get to some erratum implementation or other HW that may need it
there's no difference. Then we'll add it and churn code wondering if it's
really XFI only or also SFI. Just as I wonder if AQR do support 10GBase-KR
backplane as a PHY-MAC interface...

> >  	case PHY_INTERFACE_MODE_XGMII:
> >  		tmp |= IF_MODE_10G;
> >  		break;
> > @@ -456,6 +457,7 @@ static int init(struct memac_regs __iomem *regs,
> struct memac_cfg *cfg,
> >  	/* TX_FIFO_SECTIONS */
> >  	tmp = 0;
> >  	if (phy_if == PHY_INTERFACE_MODE_XFI ||
> > +	    phy_if == PHY_INTERFACE_MODE_SFI ||
> 
> Again, no difference between these two.
> 
> >  	    phy_if == PHY_INTERFACE_MODE_XGMII) {
> >  		if (slow_10g_if) {
> >  			tmp |= (TX_FIFO_SECTIONS_TX_AVAIL_SLOW_10G |
> > diff --git a/drivers/net/ethernet/freescale/fman/mac.c
> b/drivers/net/ethernet/freescale/fman/mac.c
> > index 2944188c19b3..5e6317742c38 100644
> > --- a/drivers/net/ethernet/freescale/fman/mac.c
> > +++ b/drivers/net/ethernet/freescale/fman/mac.c
> > @@ -542,6 +542,7 @@ static const u16 phy2speed[] = {
> >  	[PHY_INTERFACE_MODE_QSGMII]		= SPEED_1000,
> >  	[PHY_INTERFACE_MODE_XGMII]		= SPEED_10000,
> >  	[PHY_INTERFACE_MODE_XFI]		= SPEED_10000,
> > +	[PHY_INTERFACE_MODE_SFI]		= SPEED_10000,
> 
> Again, no difference between these two.
> 
> >  };
> >
> >  static struct platform_device *dpaa_eth_add_device(int fman_id,
> > @@ -800,6 +801,7 @@ static int mac_probe(struct platform_device
> *_of_dev)
> >
> >  	/* The 10G interface only supports one mode */
> >  	if (mac_dev->phy_if == PHY_INTERFACE_MODE_XFI ||
> > +	    mac_dev->phy_if == PHY_INTERFACE_MODE_SFI ||
> 
> Again, no difference between these two.
> 
> I just don't see the point of perpetuating the XFI and SFI names for
> something that is just plain 10GBASE-R.

If we do not let current maintainers properly describe their HW now,
when it's fresh and in active development, if we ever decide to
disambiguate later who will pick up the task of determining what 10G*
it's actually meant there? I kind of went through this exercise for
a similar change in u-boot (old platforms used XGMII instead of XFI/XAUI)
and tracking down each of them, making sure they work is a pain (still not
done testing all of them thus no patch sent yet).

Regards,
Madalin 

[PATCH 6/6] net: phy: aquantia: add support for PHY_INTERFACE_MODE_XFI

[Madalin Bucur]

The PHY_INTERFACE_MODE_10GKR was introduced as a catch-all
value for 10GBase-KR, XFI, SFI, to differentiate against XGMII.
The AQR PHYs support XFI as a PHY connection type, adding it
to the list of supported types. I'm not sure whether 10GBase-KR
is actually supported or not on the AQR devices so I'm not
touching that.

Signed-off-by: Madalin Bucur <madalin.bucur@oss.nxp.com>
---
 drivers/net/phy/aquantia_main.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia_main.c
index 975789d9349d..688a637f1aba 100644
--- a/drivers/net/phy/aquantia_main.c
+++ b/drivers/net/phy/aquantia_main.c
@@ -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_XFI;
+		break;
 	case MDIO_PHYXS_VEND_IF_STATUS_TYPE_USXGMII:
 		phydev->interface = PHY_INTERFACE_MODE_USXGMII;
 		break;
@@ -493,6 +495,7 @@ 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_XFI &&
 	    phydev->interface != PHY_INTERFACE_MODE_10GKR)
 		return -ENODEV;
 
-- 
2.1.0