Message-ID: <20200216000028.GU25745@shell.armlinux.org.uk>
Date:   Sun, 16 Feb 2020 00:00:28 +0000
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 00/10] Pause updates for phylib and phylink

On Sat, Feb 15, 2020 at 01:11:16PM -0800, Florian Fainelli wrote:
> 
> 
> On 2/15/2020 7:48 AM, Russell King - ARM Linux admin wrote:
> > Currently, phylib resolves the speed and duplex settings, which MAC
> > drivers use directly. phylib also extracts the "Pause" and "AsymPause"
> > bits from the link partner's advertisement, and stores them in struct
> > phy_device's pause and asym_pause members with no further processing.
> > It is left up to each MAC driver to implement decoding for this
> > information.
> > 
> > phylink converted drivers are able to take advantage of code therein
> > which resolves the pause advertisements for the MAC driver, but this
> > does nothing for unconverted drivers. It also does not allow us to
> > make use of hardware-resolved pause states offered by several PHYs.
> > 
> > This series aims to address this by:
> > 
> > 1. Providing a generic implementation, linkmode_resolve_pause(), that
> >    takes the ethtool linkmode arrays for the link partner and local
> >    advertisements, decoding the result to whether pause frames should
> >    be allowed to be transmitted or received and acted upon.  I call
> >    this the pause enablement state.
> > 
> > 2. Providing a phylib implementation, phy_get_pause(), which allows
> >    MAC drivers to request the pause enablement state from phylib.
> > 
> > 3. Providing a generic linkmode_set_pause() for setting the pause
> >    advertisement according to the ethtool tx/rx flags - note that this
> >    design has some shortcomings, see the comments in the kerneldoc for
> >    this function.
> > 
> > 4. Remove the ability in phylink to set the pause states for fixed
> >    links, which brings them into line with how we deal with the speed
> >    and duplex parameters; we can reintroduce this later if anyone
> >    requires it.  This could be a userspace-visible change.
> > 
> > 5. Split application of manual pause enablement state from phylink's
> >    resolution of the same to allow use of phylib's new phy_get_pause()
> >    interface by phylink, and make that switch.
> > 
> > 6. Resolve the fixed-link pause enablement state using the generic
> >    linkmode_resolve_pause() helper introduced earlier. This, in
> >    connection with the previous commits, allows us to kill the
> >    MLO_PAUSE_SYM and MLO_PAUSE_ASYM flags.
> > 
> > 7. make phylink's ethtool pause setting implementation update the
> >    pause advertisement in the same way that phylib does, with the
> >    same caveats that are present there (as mentioned above w.r.t
> >    linkmode_set_pause()).
> > 
> > 8. create a more accurate initial configuration for MACs, used when
> >    phy_start() is called or a SFP is detected. In particular, this
> >    ensures that the pause bits seen by MAC drivers in state->pause
> >    are accurate for SGMII.
> > 
> > 9. finally, update the kerneldoc descriptions for mac_config() for
> >    the above changes.
> > 
> > This series has been build-tested against net-next; the boot tested
> > patches are in my "phy" branch against v5.5 plus the queued phylink
> > changes that were merged for 5.6.
> > 
> > The next series will introduce the ability for phylib drivers to
> > provide hardware resolved pause enablement state.  These patches can
> > be found in my "phy" branch.
> 
> I do not think that patch #1 made it to the mailing-list though, so if
> nothing else you may want to resend it:
> 
> http://patchwork.ozlabs.org/project/netdev/list/?series=158739

You're right, it was missing the Cc line.  Resent just that - which
makes the series complete in patchwork, although not sure if the
order is correct.  Does it sort a patch series by sent date?

> 
> > 
> >  drivers/net/phy/Makefile     |   3 +-
> >  drivers/net/phy/linkmode.c   |  95 +++++++++++++++++++++++++
> >  drivers/net/phy/phy_device.c |  43 +++++++-----
> >  drivers/net/phy/phylink.c    | 162 +++++++++++++++++++++++++++----------------
> >  include/linux/linkmode.h     |   8 ++-
> >  include/linux/phy.h          |   3 +
> >  include/linux/phylink.h      |  34 +++++----
> >  7 files changed, 258 insertions(+), 90 deletions(-)
> >  create mode 100644 drivers/net/phy/linkmode.c
> > 
> 
> -- 
> Florian
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Powered by blists - more mailing lists