> -----Original Message-----
> From: Russell King - ARM Linux admin <linux@xxxxxxxxxxxxxxx>
> Sent: Thursday, December 19, 2019 7:30 PM
> To: Madalin Bucur <madalin.bucur@xxxxxxx>
> Cc: davem@xxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; andrew@xxxxxxx;
> f.fainelli@xxxxxxxxx; hkallweit1@xxxxxxxxx; shawnguo@xxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; Madalin Bucur (OSS)
> <madalin.bucur@xxxxxxxxxxx>
> 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@xxxxxxxxxxx>
> > ---
> > 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
