netdev - Re: [PATCH net-next 05/10] net: phylink: add EEE management

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] [day] [month] [year] [list]

Message-ID: <928c2613-b028-4073-818c-5cf38bd304ca@gmail.com>
Date: Sun, 15 Dec 2024 00:38:24 +0100
From: Heiner Kallweit <hkallweit1@...il.com>
To: "Russell King (Oracle)" <rmk+kernel@...linux.org.uk>,
 Andrew Lunn <andrew@...n.ch>
Cc: Andrew Lunn <andrew+netdev@...n.ch>,
 Bryan Whitehead <bryan.whitehead@...rochip.com>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Marcin Wojtas <marcin.s.wojtas@...il.com>,
 netdev@...r.kernel.org, Paolo Abeni <pabeni@...hat.com>,
 UNGLinuxDriver@...rochip.com
Subject: Re: [PATCH net-next 05/10] net: phylink: add EEE management

On 09.12.2024 15:23, Russell King (Oracle) wrote:
> Add EEE management to phylink, making use of the phylib implementation.
> This will only be used where a MAC driver populates the methods and
> capabilities bitfield, otherwise we keep our old behaviour.
> 
> Phylink will keep track of the EEE configuration, including the clock
> stop abilities at each end of the MAC to PHY link, programming the PHY
> appropriately and preserving the EEE configuration should the PHY go
> away.
> 
> It will also call into the MAC driver when LPI needs to be enabled or
> disabled, with the expectation that the MAC have LPI disabled during
> probe.
> 
> Support for phylink managed EEE is enabled by populating both tx_lpi
> MAC operations method pointers.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@...linux.org.uk>
> ---
>  drivers/net/phy/phylink.c | 123 ++++++++++++++++++++++++++++++++++++--
>  include/linux/phylink.h   |  44 ++++++++++++++
>  2 files changed, 163 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 03509fdaa1ec..750356b6a2e9 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -81,12 +81,19 @@ struct phylink {
>  	unsigned int pcs_state;
>  
>  	bool link_failed;
> +	bool mac_supports_eee;
> +	bool phy_enable_tx_lpi;
> +	bool mac_enable_tx_lpi;
> +	bool mac_tx_clk_stop;
> +	u32 mac_tx_lpi_timer;
>  
>  	struct sfp_bus *sfp_bus;
>  	bool sfp_may_have_phy;
>  	DECLARE_PHY_INTERFACE_MASK(sfp_interfaces);
>  	__ETHTOOL_DECLARE_LINK_MODE_MASK(sfp_support);
>  	u8 sfp_port;
> +
> +	struct eee_config eee_cfg;
>  };
>  
>  #define phylink_printk(level, pl, fmt, ...) \
> @@ -1563,6 +1570,47 @@ static const char *phylink_pause_to_str(int pause)
>  	}
>  }
>  
> +static void phylink_deactivate_lpi(struct phylink *pl)
> +{
> +	if (pl->mac_enable_tx_lpi) {
> +		pl->mac_enable_tx_lpi = false;
> +
> +		phylink_dbg(pl, "disabling LPI\n");
> +
> +		pl->mac_ops->mac_disable_tx_lpi(pl->config);
> +	}
> +}
> +
> +static void phylink_activate_lpi(struct phylink *pl)
> +{
> +	if (!test_bit(pl->cur_interface, pl->config->lpi_interfaces)) {
> +		phylink_dbg(pl, "MAC does not support LPI with %s\n",
> +			    phy_modes(pl->cur_interface));
> +		return;
> +	}
> +
> +	phylink_dbg(pl, "LPI timer %uus, tx clock stop %u\n",
> +		    pl->mac_tx_lpi_timer, pl->mac_tx_clk_stop);
> +
> +	pl->mac_ops->mac_enable_tx_lpi(pl->config, pl->mac_tx_lpi_timer,
> +				       pl->mac_tx_clk_stop);
> +
> +	pl->mac_enable_tx_lpi = true;
> +}
> +
> +static void phylink_phy_restrict_eee(struct phylink *pl, struct phy_device *phy)
> +{
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(eee_supported);
> +
> +	/* Convert the MAC's LPI capabilities to linkmodes */
> +	linkmode_zero(eee_supported);
> +	phylink_caps_to_linkmodes(eee_supported, pl->config->lpi_capabilities);
> +
> +	/* Mask out EEE modes that are not supported */
> +	linkmode_and(phy->supported_eee, phy->supported_eee, eee_supported);
> +	linkmode_and(phy->advertising_eee, phy->advertising_eee, eee_supported);
> +}
> +

Something similar we may need in phylib too. An example is cpsw MAC driver which
doesn't support EEE. Issues have been reported if the PHY's on both sides negotiate
EEE, workaround is to use property eee-broken-xxx in the respective DT's to disable
PHY EEE advertisement. I'm thinking of adding a simple phy_disable_eee() which can
be called by MAC drivers to clear EEE supported and advertising bitmaps.

A similar case is enetc (using phylink) which doesn't support EEE. See following in
enetc.c:

/* disable EEE autoneg, until ENETC driver supports it */
memset(&edata, 0, sizeof(struct ethtool_keee));
phylink_ethtool_set_eee(priv->phylink, &edata);

Russell, do you plan to change this driver too, based on phylink extensions?
I think already now, based on the quoted code piece, several (all?) eee-broken-xxx
properties can be removed under arch/arm64/boot/dts/freescale .