Re: [PATCH net-next v2 1/1] net: dsa: microchip: Remove ineffective checks from ksz_set_mac_eee()

Oleksij Rempel
2025-04-25T14:27:06Z

On Fri, Apr 25, 2025 at 02:41:21PM +0100, Russell King (Oracle) wrote:
> On Fri, Apr 25, 2025 at 01:08:45PM +0200, Oleksij Rempel wrote:
> > KSZ switches handle EEE internally via PHY advertisement and do not
> > support MAC-level configuration. The ksz_set_mac_eee() handler previously
> > rejected Tx LPI disable and timer changes, but provided no real control.
>
> Err what?
>
> ksz does not set phylink_config->eee_enabled_default, so the default
> state in phylink is eee_enabled = false, tx_lpi_enabled = false. It
> doesn't set the default LPI timer, so tx_lpi_timer = 0.
>
> As the driver does not implement the ability to change the LPI timer
> enable nor the timer value, this seemed reasonable as the values are
> not reported (being reported as zeros) and thus prevents modification
> thereof.
>
> Why do you want to allow people to change parameters that have no
> effect?

The original ksz_get_mac_eee() used to report tx_lpi_enabled = true,
which correctly reflected the internal EEE/LPI activity of the hardware.

After commit [0945a7b44220 ("net: dsa: ksz: remove setting of tx_lpi
parameters")], ksz_get_mac_eee() was removed, and now tx_lpi_enabled defaults
to false via the phylink fallback.

This leads to two problems:

- Userspace is unable to disable EEE cleanly (ethtool --set-eee lan1 eee off
fails with -EINVAL).

- At the same time, userspace sees tx_lpi_enabled = false, which does not match
hardware behavior (EEE/LPI is active).

At the moment, keeping the old validation checks blocks userspace from
disabling EEE at all.

But removing all validation risks accepting nonsensical parameter combinations.

Right now, I am not fully sure what the best way forward is.

--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |