> -----Original Message----- > From: Andrew Lunn <andrew@xxxxxxx> > Sent: Friday, January 3, 2020 3:35 PM > To: Russell King - ARM Linux admin <linux@xxxxxxxxxxxxxxx> > Cc: Madalin Bucur (OSS) <madalin.bucur@xxxxxxxxxxx>; > devicetree@xxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; > f.fainelli@xxxxxxxxx; hkallweit1@xxxxxxxxx; shawnguo@xxxxxxxxxx > 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@xxxxxxx> 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@xxxxxxx> Cc: Stefan Chulski <stefanc@xxxxxxxxxxx> Cc: Kostya Porotchkin <kostap@xxxxxxxxxxx> Cc: Nadav Haklai <nadavh@xxxxxxxxxxx> Acked-by: Joe Hershberger <joe.hershberger@xxxxxx> 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