netdev - Re: [RFC PATCH v2 net-next 9/9] net: pcs: xpcs: convert to phylink_pcs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210602134749.GL30436@shell.armlinux.org.uk>
Date:   Wed, 2 Jun 2021 14:47:49 +0100
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     Wong Vee Khee <vee.khee.wong@...ux.intel.com>,
        Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        Ong Boon Leong <boon.leong.ong@...el.com>,
        Michael Sit Wei Hong <michael.wei.hong.sit@...el.com>,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        Jose Abreu <joabreu@...opsys.com>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Vladimir Oltean <vladimir.oltean@....com>
Subject: Re: [RFC PATCH v2 net-next 9/9] net: pcs: xpcs: convert to
 phylink_pcs_ops

On Wed, Jun 02, 2021 at 04:43:21PM +0300, Vladimir Oltean wrote:
> On Tue, Jun 01, 2021 at 01:10:33PM +0100, Russell King (Oracle) wrote:
> > On Tue, Jun 01, 2021 at 03:33:25AM +0300, Vladimir Oltean wrote:
> > >  static const struct phylink_mac_ops stmmac_phylink_mac_ops = {
> > >  	.validate = stmmac_validate,
> > > -	.mac_pcs_get_state = stmmac_mac_pcs_get_state,
> > > -	.mac_config = stmmac_mac_config,
> > 
> > mac_config is still a required function.
> 
> This is correct, thanks.
> 
> VK, would you mind testing again with this extra patch added to the mix?
> If it works, I will add it to the series in v3, ordered properly.
> 
> -----------------------------[ cut here]-----------------------------
> From a79863027998451c73d5bbfaf1b77cf6097a110c Mon Sep 17 00:00:00 2001
> From: Vladimir Oltean <vladimir.oltean@....com>
> Date: Wed, 2 Jun 2021 16:35:55 +0300
> Subject: [PATCH] net: phylink: allow the mac_config method to be missing if
>  pcs_ops are provided
> 
> The pcs_config method from struct phylink_pcs_ops does everything that
> the mac_config method from struct phylink_mac_ops used to do in the
> legacy approach of driving a MAC PCS. So allow drivers to not implement
> the mac_config method if there is nothing to do. Keep the method
> required for setups that do not provide pcs_ops.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> ---
>  drivers/net/phy/phylink.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 96d8e88b4e46..a8842c6ce3a2 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -415,6 +415,9 @@ static void phylink_resolve_flow(struct phylink_link_state *state)
>  static void phylink_mac_config(struct phylink *pl,
>  			       const struct phylink_link_state *state)
>  {
> +	if (!pl->mac_ops->mac_config)
> +		return;
> +
>  	phylink_dbg(pl,
>  		    "%s: mode=%s/%s/%s/%s adv=%*pb pause=%02x link=%u an=%u\n",
>  		    __func__, phylink_an_mode_str(pl->cur_link_an_mode),
> @@ -1192,6 +1195,12 @@ void phylink_start(struct phylink *pl)
>  
>  	ASSERT_RTNL();
>  
> +	/* The mac_ops::mac_config method may be absent only if the
> +	 * pcs_ops are present.
> +	 */
> +	if (WARN_ON_ONCE(!pl->mac_ops->mac_config && !pl->pcs_ops))
> +		return;
> +
>  	phylink_info(pl, "configuring for %s/%s link mode\n",
>  		     phylink_an_mode_str(pl->cur_link_an_mode),
>  		     phy_modes(pl->link_config.interface));

I would rather we didn't do that - I suspect your case is not the
common scenario, so please add a dummy function to stmmac instead.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!