netdev - RE: [PATCH 5/6] net: fsl/fman: add support for PHY_INTERFACE_MODE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <VI1PR04MB5567899CDB9A9F36297CF236EC520@VI1PR04MB5567.eurprd04.prod.outlook.com>
Date:   Thu, 19 Dec 2019 18:50:16 +0000
From:   "Madalin Bucur (OSS)" <madalin.bucur@....nxp.com>
To:     Russell King - ARM Linux admin <linux@...linux.org.uk>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        "hkallweit1@...il.com" <hkallweit1@...il.com>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "Madalin Bucur (OSS)" <madalin.bucur@....nxp.com>
Subject: RE: [PATCH 5/6] net: fsl/fman: add support for PHY_INTERFACE_MODE_SFI

> -----Original Message-----
> From: Russell King - ARM Linux admin <linux@...linux.org.uk>
> Sent: Thursday, December 19, 2019 7:30 PM
> To: Madalin Bucur <madalin.bucur@....com>
> Cc: davem@...emloft.net; netdev@...r.kernel.org; andrew@...n.ch;
> f.fainelli@...il.com; hkallweit1@...il.com; shawnguo@...nel.org;
> devicetree@...r.kernel.org; Madalin Bucur (OSS)
> <madalin.bucur@....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@....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