[PATCH net v4 1/2] net: dsa: mt7530: populate lpi_interfaces to fix EEE support

[Maxime Chevallier]

Hi Aleksei,

On 9/2/26 10:05, Aleksei Sviridkin wrote:
> phylink_create() decides once and for all that a MAC supports managed
> EEE, and it requires the tx_lpi ops plus non-empty lpi_capabilities and
> lpi_interfaces. mt753x_phylink_get_caps() leaves lpi_interfaces empty.
> 
> So ever since the conversion to phylink managed EEE, ethtool has
> answered "Not supported" on every mt753x port, and phy_disable_eee()
> has locked userspace out of turning EEE on. That undoes what
> commit 06dfcd4098cf ("net: dsa: mt7530: fix enabling EEE on MT7531
> switch on all boards") arranged: EEE off by default, but reachable
> with ethtool.
> 
> Leave the speeds above 1 Gbps out of both bitmaps. PMCR folds
> SPEED_2500 and SPEED_10000 onto PMCR_FORCE_SPEED_1000, so
> PMCR_FORCE_EEE1G would govern LPI on such a link, and that is
> unvalidated rather than known unsupported: MediaTek's SDK driver sets
> the EEE force bits for 100 Mbps and 1 Gbps only, and the unit of the
> wakeup timers is undocumented with the port clock at 2.5 times the
> rate.
> 
> LPI stays off until userspace enables it, but the EEE advertisement of
> a PHY that advertises it out of reset comes back, since phylink stops
> force-clearing it.
> 
> Fixes: 9cf21773f535 ("net: dsa: mt7530: convert to phylink managed EEE")
> Signed-off-by: Aleksei Sviridkin <f at lex.la>
> ---
> Ports that support neither 100 Mbps nor 1 Gbps are skipped because on
> MT7988, EN7581 and AN7583 port 6 is 10 Gbps only and shares
> PHY_INTERFACE_MODE_INTERNAL with the user ports, so the interface mask
> alone cannot tell them apart. MT7531's internal PHYs keep the
> advertisement mt7531_setup() zeroed and EN7528 keeps both bitmaps
> empty, so EEE stays fully off there.
> 
> Two pre-existing things this patch makes live, neither addressed here:
> 
> - The unit of LPI_THRESH is still unspecified, as the comment above
>   lpi_timer_default says. Before the phylink conversion
>   mt753x_set_mac_eee() wrote tx_lpi_timer into the field directly, so
>   the raw unit has always been what ethtool showed on these ports.
> 
> - mt753x_phylink_mac_enable_tx_lpi() sets the PMCR force-EEE bits
>   without checking the resolved speed or interface, relying entirely on
>   phylink never calling it above 1 Gbps. A check there would make the
>   driver robust independently of lpi_interfaces being right.
> 
> v2, with the full argument for leaving the higher speeds out:
> https://lore.kernel.org/netdev/20260824024117.46154-2-f@lex.la/
> 
> v4: lpi_capabilities is mac_capabilities masked to the two speeds
> instead of both asserted. Every port that reaches the guard today has
> both speeds, so nothing changes; it only stops the bitmap from claiming
> a speed a port cannot link at.
> 
>  drivers/net/dsa/mt7530.c | 25 ++++++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 2b7be091c056..a188a391b650 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -3172,23 +3172,38 @@ static void mt753x_phylink_get_caps(struct dsa_switch *ds, int port,
>  
>  	config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE;
>  
> +	priv->info->mac_port_get_caps(ds, port, config);
> +
>  	/* The EN7528 GPHYs report EEE capability, but negotiating EEE with
>  	 * common link partners (e.g. Realtek GbE NICs) results in an unstable
>  	 * link with dropped frames. Leave the LPI capabilities empty so that
>  	 * phylink disables EEE on these PHYs and refuses to enable it from
> -	 * userspace.
> +	 * userspace. Ports that run at neither 100 Mbps nor 1 Gbps are left
> +	 * empty too, since LPI above 1 Gbps is unvalidated.
>  	 */
> -	if (priv->id != ID_EN7528) {
> +	if (priv->id != ID_EN7528 &&
> +	    config->mac_capabilities & (MAC_100FD | MAC_1000FD)) {
>  		u32 eeecr = mt7530_read(priv, MT753X_PMEEECR_P(port));
>  
> -		config->lpi_capabilities = MAC_100FD | MAC_1000FD | MAC_2500FD;
> +		/* PMCR folds SPEED_2500 and SPEED_10000 onto
> +		 * PMCR_FORCE_SPEED_1000, so LPI above 1 Gbps would be
> +		 * governed by PMCR_FORCE_EEE1G and is unvalidated rather than
> +		 * unsupported. Leave it out of both bitmaps: lpi_capabilities
> +		 * gates on the media speed a rate matching PHY reports, not
> +		 * on the speed the MAC runs at.
> +		 */

This is still too verbose IMO... 2500Mbps LPI isn't uspported, that's all we
need to know :)

It's very common that 2500 uses the same settings as 1000 with just a random
register somewhere switching the speed to 2.5x, and also very common that lots
of features break when using 2500, so no need for the verbosity here. Same
goes for the next patch as well.

Sorry for nitpicking on this, but comment verbosity overall has been rising-up lately,
LLMs may have something to do with it, and it doesn't necessarily make things more
readable :(

The code itself looks fine though,

Maxime