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

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]

Date:   Fri, 3 Jan 2020 16:21:09 +0000
From:   "Madalin Bucur (OSS)" <madalin.bucur@....nxp.com>
To:     Andrew Lunn <andrew@...n.ch>,
        Russell King - ARM Linux admin <linux@...linux.org.uk>
CC:     "Madalin Bucur (OSS)" <madalin.bucur@....nxp.com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        "hkallweit1@...il.com" <hkallweit1@...il.com>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>
Subject: RE: [PATCH 1/6] net: phy: add interface modes for XFI, SFI

> -----Original Message-----
> From: Andrew Lunn <andrew@...n.ch>
> Sent: Friday, January 3, 2020 3:35 PM
> To: Russell King - ARM Linux admin <linux@...linux.org.uk>
> Cc: Madalin Bucur (OSS) <madalin.bucur@....nxp.com>;
> devicetree@...r.kernel.org; davem@...emloft.net; netdev@...r.kernel.org;
> f.fainelli@...il.com; hkallweit1@...il.com; shawnguo@...nel.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@...x.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@...x.de>
    Cc: Stefan Chulski <stefanc@...vell.com>
    Cc: Kostya Porotchkin <kostap@...vell.com>
    Cc: Nadav Haklai <nadavh@...vell.com>
    Acked-by: Joe Hershberger <joe.hershberger@...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