Message-ID: <CA+h21hrR1Xkx9gwAT2FHqcH38L=xjWiPxmF2Er7-4fHFTrA8pQ@mail.gmail.com>
Date:   Wed, 26 Feb 2020 13:06:06 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     Russell King <rmk+kernel@...linux.org.uk>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Alexandre Torgue <alexandre.torgue@...com>,
        "David S. Miller" <davem@...emloft.net>,
        Felix Fietkau <nbd@....name>,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Hauke Mehrtens <hauke@...ke-m.de>,
        Ioana Ciornei <ioana.ciornei@....com>,
        Jakub Kicinski <kuba@...nel.org>,
        John Crispin <john@...ozen.org>,
        Jonathan Corbet <corbet@....net>,
        Jose Abreu <joabreu@...opsys.com>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-arm-kernel@...ts.infradead.org>,
        Linux Doc Mailing List <linux-doc@...r.kernel.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@...ts.infradead.org>,
        linux-stm32@...md-mailman.stormreply.com,
        Mark Lee <Mark-MC.Lee@...iatek.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Michal Simek <michal.simek@...inx.com>,
        netdev <netdev@...r.kernel.org>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Radhey Shyam Pandey <radhey.shyam.pandey@...inx.com>,
        Sean Wang <sean.wang@...iatek.com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Ioana Radulescu <ruxandra.radulescu@....com>
Subject: Re: [PATCH net-next v2 1/8] net: phylink: propagate resolved link
 config via mac_link_up()

Hi Russell,

On Wed, 26 Feb 2020 at 12:23, Russell King <rmk+kernel@...linux.org.uk> wrote:
>
> Propagate the resolved link parameters via the mac_link_up() call for
> MACs that do not automatically track their PCS state. We propagate the
> link parameters via function arguments so that inappropriate members
> of struct phylink_link_state can't be accessed, and creating a new
> structure just for this adds needless complexity to the API.
>
> Tested-by: Andre Przywara <andre.przywara@....com>
> Tested-by: Alexandre Belloni <alexandre.belloni@...tlin.com>
> Tested-by: Vladimir Oltean <vladimir.oltean@....com>
> Signed-off-by: Russell King <rmk+kernel@...linux.org.uk>
> ---
>  Documentation/networking/sfp-phylink.rst      | 17 +++++-
>  drivers/net/ethernet/cadence/macb_main.c      |  7 ++-
>  .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  |  7 ++-
>  drivers/net/ethernet/marvell/mvneta.c         |  8 ++-
>  .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 19 +++++--
>  drivers/net/ethernet/mediatek/mtk_eth_soc.c   |  7 ++-
>  .../net/ethernet/stmicro/stmmac/stmmac_main.c |  4 +-
>  .../net/ethernet/xilinx/xilinx_axienet_main.c |  7 ++-
>  drivers/net/phy/phylink.c                     |  9 ++-
>  include/linux/phylink.h                       | 57 ++++++++++++++-----
>  net/dsa/port.c                                |  4 +-
>  11 files changed, 105 insertions(+), 41 deletions(-)
>
> diff --git a/Documentation/networking/sfp-phylink.rst b/Documentation/networking/sfp-phylink.rst
> index d753a309f9d1..8d7af28cd835 100644
> --- a/Documentation/networking/sfp-phylink.rst
> +++ b/Documentation/networking/sfp-phylink.rst
> @@ -74,10 +74,13 @@ phylib to the sfp/phylink support.  Please send patches to improve
>  this documentation.
>
>  1. Optionally split the network driver's phylib update function into
> -   three parts dealing with link-down, link-up and reconfiguring the
> -   MAC settings. This can be done as a separate preparation commit.
> +   two parts dealing with link-down and link-up. This can be done as
> +   a separate preparation commit.
>
> -   An example of this preparation can be found in git commit fc548b991fb0.
> +   An older example of this preparation can be found in git commit
> +   fc548b991fb0, although this was splitting into three parts; the
> +   link-up part now includes configuring the MAC for the link settings.
> +   Please see :c:func:`mac_link_up` for more information on this.
>
>  2. Replace::
>
> @@ -207,6 +210,14 @@ this documentation.
>     using. This is particularly important for in-band negotiation
>     methods such as 1000base-X and SGMII.
>
> +   The :c:func:`mac_link_up` method is used to inform the MAC that the
> +   link has come up. The call includes the negotiation mode and interface
> +   for reference only. The finalised link parameters are also supplied
> +   (speed, duplex and flow control/pause enablement settings) which
> +   should be used to configure the MAC when the MAC and PCS are not
> +   tightly integrated, or when the settings are not coming from in-band
> +   negotiation.
> +
>     The :c:func:`mac_config` method is used to update the MAC with the
>     requested state, and must avoid unnecessarily taking the link down
>     when making changes to the MAC configuration.  This means the

Just to make sure I understand the changes:
- A MAC with no PCS can be configured in either .mac_config or .mac_link_up
- A MAC that needs to be manually reconfigured to the link mode
negotiated by its PCS needs to have the PCS configured in .mac_config
and the MAC in .mac_link_up
- A MAC with PCS where the MAC follows the PCS negotiation
automatically in hardware is basically equivalent with a MAC with no
PCS, and therefore can be configured in either .mac_config or
.mac_link_up
Is this correct?

Regards,
-Vladimir

Powered by blists - more mailing lists