Re: [PATCH net-next v2 3/9] net: phy: add kr phy connection type

[Russell King - ARM Linux admin <linux@armlinux.org.uk>]

On Fri, Apr 24, 2020 at 03:42:36PM +0200, Andrew Lunn wrote:
> On Fri, Apr 24, 2020 at 03:46:25PM +0300, Florinel Iordache wrote:
> > Add support for backplane kr phy connection types currently available
> > (10gbase-kr, 40gbase-kr4) and the required phylink updates (cover all
> > the cases for KR modes which are clause 45 compatible to correctly assign
> > phy_interface and phylink#supported)
> > 
> > Signed-off-by: Florinel Iordache <florinel.iordache@nxp.com>
> > ---
> >  drivers/net/phy/phylink.c | 15 ++++++++++++---
> >  include/linux/phy.h       |  6 +++++-
> >  2 files changed, 17 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> > index 34ca12a..9a31f68 100644
> > --- a/drivers/net/phy/phylink.c
> > +++ b/drivers/net/phy/phylink.c
> > @@ -4,6 +4,7 @@
> >   * technologies such as SFP cages where the PHY is hot-pluggable.
> >   *
> >   * Copyright (C) 2015 Russell King
> > + * Copyright 2020 NXP
> >   */
> >  #include <linux/ethtool.h>
> >  #include <linux/export.h>
> > @@ -304,7 +305,6 @@ static int phylink_parse_mode(struct phylink *pl, struct fwnode_handle *fwnode)
> >  			break;
> >  
> >  		case PHY_INTERFACE_MODE_USXGMII:
> > -		case PHY_INTERFACE_MODE_10GKR:
> >  		case PHY_INTERFACE_MODE_10GBASER:
> >  			phylink_set(pl->supported, 10baseT_Half);
> >  			phylink_set(pl->supported, 10baseT_Full);
> 
> Hi Florinel
> 
> What about the issues pointed out in:
> 
> https://www.spinics.net/lists/netdev/msg641046.html

Having reviewed the situation, it seems that I added a translation
to mvpp2 driver for this, translating PHY_INTERFACE_MODE_10GKR to
PHY_INTERFACE_MODE_10GBASER, so anything using "10gbase-kr" in
arch/arm64/boot/dts/marvell/ is not a worry - however, those DT files
still need to be updated but my request to bootlin for help with
that has gone unanswered to date.  So, I'm tempted to change them
wholesale, since that's what we're doing in the mvpp2 driver anyway.

> > @@ -107,8 +108,9 @@
> >  	/* 10GBASE-R, XFI, SFI - single lane 10G Serdes */
> >  	PHY_INTERFACE_MODE_10GBASER,
> >  	PHY_INTERFACE_MODE_USXGMII,
> > -	/* 10GBASE-KR - with Clause 73 AN */
> > +	/* Backplane KR */
> >  	PHY_INTERFACE_MODE_10GKR,
> > +	PHY_INTERFACE_MODE_40GKR4,
> >  	PHY_INTERFACE_MODE_MAX,
> >  } phy_interface_t;

I would like to see these (re-)named to PHY_INTERFACE_MODE_*GBASE* as
we have the same for previous definitions such as 1000BASEX and
2500BASEX.

Also, please update Documentation/networking/phy.rst with a description
of the new 40GBASE-KR4 mode.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up