Re: [PATCH net-next 0/5] add phylink support for PCS

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

On Wed, Mar 11, 2020 at 02:46:33PM +0200, Vladimir Oltean wrote:
> Hi Russell,
> 
> On Wed, 11 Mar 2020 at 14:09, Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > Hi,
> >
> > This series adds support for IEEE 802.3 register set compliant PCS
> > for phylink.  In order to do this, we:
> >
> > 1. convert the existing (unused) mii_lpa_to_ethtool_lpa_x() function
> >    to a linkmode variant.
> > 2. add a helper for clause 37 advertisements, supporting both the
> >    1000baseX and defacto 2500baseX variants. Note that ethtool does
> >    not support half duplex for either of these, and we make no effort
> >    to do so.
> > 3. add accessors for modifying a MDIO device register, and use them in
> >    phylib, rather than duplicating the code from phylib.
> 
> Have you considered accessing the PCS as a phy_device structure, a la
> drivers/net/dsa/ocelot/felix_vsc9959.c?

I don't want to tie this into phylib, because I don't think phylib
should be dealing with PCS.  It brings with it many problems, such as:

1. how do we know whether the Clause 22 registers are supposed to be
   Clause 37 format.
2. how do we program the PCS appropriately for the negotiation results
   (which phylib doesn't support).
3. how do we deal with selecting the appropriate device for the mode
   selected (LX2160A has multiple different PCS which depend on the
   mode selected.)

Note that a phy_device structure embeds a mdio_device structure, and
so these helpers can be used inside phylib if one desires - so this
approach is more flexible than "bolt it into phylib" approach would
be.

> > 4. add support for decoding the advertisement from clause 22 compatible
> >    register sets for clause 37 advertisements and SGMII advertisements.
> > 5. add support for clause 45 register sets for 10GBASE-R PCS.
> >
> > These have been tested on the LX2160A Clearfog-CX platform.
> >
> >  drivers/net/phy/mdio_bus.c |  55 +++++++++++
> >  drivers/net/phy/phy-core.c |  31 ------
> >  drivers/net/phy/phylink.c  | 236 +++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/mdio.h       |   4 +
> >  include/linux/mii.h        |  57 +++++++----
> >  include/linux/phy.h        |  19 ++++
> >  include/linux/phylink.h    |   8 ++
> >  include/uapi/linux/mii.h   |   5 +
> >  8 files changed, 366 insertions(+), 49 deletions(-)
> >
> > --
> > 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

-- 
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