netdev - Re: [net-next PATCH v4 08/11] net: phylink: add .pcs_link

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: <bb2a379e-c701-4b50-932a-20dcfbb0ebea@linux.dev>
Date: Tue, 13 May 2025 14:44:42 -0400
From: Sean Anderson <sean.anderson@...ux.dev>
To: Christian Marangi <ansuelsmth@...il.com>,
 Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Lorenzo Bianconi <lorenzo@...nel.org>,
 Heiner Kallweit <hkallweit1@...il.com>, Russell King
 <linux@...linux.org.uk>, Philipp Zabel <p.zabel@...gutronix.de>,
 Nathan Chancellor <nathan@...nel.org>,
 Nick Desaulniers <nick.desaulniers+lkml@...il.com>,
 Bill Wendling <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>,
 Daniel Golle <daniel@...rotopia.org>, netdev@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org,
 llvm@...ts.linux.dev
Subject: Re: [net-next PATCH v4 08/11] net: phylink: add .pcs_link_down PCS OP

On 5/11/25 16:12, Christian Marangi wrote:
> Permit for PCS driver to define specific operation to torn down the link
> between the MAC and the PCS.
> 
> This might be needed for some PCS that reset counter

Counters must be preserved across link u/down.

> or require special
> reset to correctly work if the link needs to be restored later.

Can you describe this in more detail?

> On phylink_link_down() call, the additional phylink_pcs_link_down() will
> be called before .mac_link_down to torn down the link.

tear

> PCS driver will need to define .pcs_link_down to make use of this.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@...il.com>
> ---
>  drivers/net/phy/phylink.c | 8 ++++++++
>  include/linux/phylink.h   | 2 ++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 1a4df0d24aa2..39cd15e30598 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -1009,6 +1009,12 @@ static void phylink_pcs_link_up(struct phylink_pcs *pcs, unsigned int neg_mode,
>  		pcs->ops->pcs_link_up(pcs, neg_mode, interface, speed, duplex);
>  }
>  
> +static void phylink_pcs_link_down(struct phylink_pcs *pcs)
> +{
> +	if (pcs && pcs->ops->pcs_link_down)
> +		pcs->ops->pcs_link_down(pcs);
> +}
> +
>  static void phylink_pcs_disable_eee(struct phylink_pcs *pcs)
>  {
>  	if (pcs && pcs->ops->pcs_disable_eee)
> @@ -1686,6 +1692,8 @@ static void phylink_link_down(struct phylink *pl)
>  
>  	phylink_deactivate_lpi(pl);
>  
> +	phylink_pcs_link_down(pl->pcs);
> +
>  	pl->mac_ops->mac_link_down(pl->config, pl->act_link_an_mode,
>  				   pl->cur_interface);
>  	phylink_info(pl, "Link is Down\n");
> diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> index c5496c063b6a..8b3d1dfb83a1 100644
> --- a/include/linux/phylink.h
> +++ b/include/linux/phylink.h
> @@ -494,6 +494,7 @@ struct phylink_pcs {
>   * @pcs_an_restart: restart 802.3z BaseX autonegotiation.
>   * @pcs_link_up: program the PCS for the resolved link configuration
>   *               (where necessary).
> + * @pcs_link_down: torn down link between MAC and PCS.

ops documentation should use imperative verbs.

You are also missing the longer documentation below.

>   * @pcs_disable_eee: optional notification to PCS that EEE has been disabled
>   *		     at the MAC.
>   * @pcs_enable_eee: optional notification to PCS that EEE will be enabled at
> @@ -521,6 +522,7 @@ struct phylink_pcs_ops {
>  	void (*pcs_an_restart)(struct phylink_pcs *pcs);
>  	void (*pcs_link_up)(struct phylink_pcs *pcs, unsigned int neg_mode,
>  			    phy_interface_t interface, int speed, int duplex);
> +	void (*pcs_link_down)(struct phylink_pcs *pcs);
>  	void (*pcs_disable_eee)(struct phylink_pcs *pcs);
>  	void (*pcs_enable_eee)(struct phylink_pcs *pcs);
>  	int (*pcs_pre_init)(struct phylink_pcs *pcs);