Re: [PATCH net-next v1] r8169: set eee_enable_default base on LPI cap

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] Undisclosed regression fix: the commit message describes the
  change as a new capability-derived default ("if LPI capabilities is…

--

r8169: set eee_enable_default base on LPI cap

This sets phylink_config.eee_enabled_default in rtl_init_phylink() when
rtl8169_get_lpi_caps() returns a non-zero LPI capability mask, so phylink
starts with EEE enabled on hardware that supports it.

> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 5415ff62a2869..1d06ddff98327 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -5945,6 +5945,7 @@ static int rtl_init_phylink(struct rtl8169_private *tp)
>  	tp->phylink_config.type = PHYLINK_NETDEV;
>  	tp->phylink_config.mac_managed_pm = true;
>  	tp->phylink_config.lpi_capabilities = rtl8169_get_lpi_caps(tp);
> +	tp->phylink_config.eee_enabled_default = !!tp->phylink_config.lpi_capabilities;

[Severity: Medium]

Is this actually restoring a default that was lost in 8a3c76523e44
("r8169: add support for phylink") rather than introducing a new one?

Before the phylink conversion, r8169_mdio_register() did:

drivers/net/ethernet/realtek/r8169_main.c:r8169_mdio_register() {
	...
	tp->phydev->mac_managed_pm = true;
	if (rtl_supports_eee(tp))
		phy_support_eee(tp->phydev);
	...
}

and phy_support_eee() sets both eee_enabled and tx_lpi_enabled to true,
so EEE was on by default on every EEE-capable part. Commit 8a3c76523e44
removed that call and added rtl_init_phylink() without setting
eee_enabled_default, and phylink_create() derives its initial state from
that field:

drivers/net/phy/phylink.c:phylink_create() {
	...
	/* Set the default EEE configuration */
	pl->eee_cfg.eee_enabled = pl->config->eee_enabled_default;
	pl->eee_cfg.tx_lpi_enabled = pl->eee_cfg.eee_enabled;
	...
}

with phylink_bringup_phy() only calling phy_support_eee(phy) when
pl->eee_cfg.eee_enabled is set.

So between the two commits EEE/LPI is off by default and ethtool
--show-eee reports it disabled until userspace turns it back on.

Should this carry Fixes: 8a3c76523e44 ("r8169: add support for phylink"),
and should the changelog say that the phylink conversion dropped the
EEE-enabled-by-default behaviour instead of describing this as a new
capability-derived default? Without the tag, a tree that picks up the
conversion alone keeps the changed default.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260916081306.184-1-javen_xu%40realsil.com.cn