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



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

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

Powered by blists - more mailing lists