Re: [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2)

On Thu, Dec 12, 2024 at 09:48:53PM +0200, Vladimir Oltean wrote:

On Tue, Dec 10, 2024 at 02:25:44PM +0000, Russell King (Oracle) wrote:

quoted

This is part 2 of the DSA EEE cleanups, and is being sent out becaues it
is relevant for the review of part 1, but would make part 1 too large.

Patch 1 removes the useless setting of tx_lpi parameters in the
ksz driver.

Patch 2 removes the DSA core code that calls the get_mac_eee() operation.
This needs to be done before removing the implementations because doing
otherwise would cause dsa_user_get_eee() to return -EOPNOTSUPP.

Patches 3..6 remove the trivial get_mac_eee() implementations from DSA
drivers.

Patch 7 finally removes the get_mac_eee() method from struct
dsa_switch_ops.

I appreciate the splitting of the get_mac_eee() removal into multiple
patches per driver and 2 for the DSA framework. It should help BSP
backporters which target only a subset of DSA drivers. Monolithic
patches are harder to digest, and may have trivial context conflicts due
to unrelated changes.

The set looks good, please don't forget to also update the documentation.

Sorry, but which documentation are you referring to?

$ grep get_mac_eee Documentation/networking/ drivers/net include/ net/ -r
$ 

No references to get_mac_eee() anywhere (except in the mt7530 driver
which I've missed - patches now included, will be in v2.)

Even looking for "et_mac_eee" in case of Documentation using
[sg]et_mac_eee() reveals nothing.

So, I don't think there's any documentation that these patches change.

There is this in Documentation/networking/dsa/dsa.rst:

- ``get_eee``: ethtool function which is used to query a switch port EEE settings,
  this function should return the EEE state of the switch port MAC controller
  and data-processing logic as well as query the PHY for its currently configured
  EEE settings

First, realise that my patch set actually changes nothing - all these
implementations were entirely useless because everything they have been
doing has been overwritten by phylib since phylib's EEE support was
changed. Therefore, the above as wrong both before and after this
patch set.

Second, when phylink managed EEE gets merged, the above becomes true
again.

Think of the current phylib behaviour as a user visible regression
(because that's exactly what it is), and this patch set is part of a
move towards resolving that regression.

IMHO, phylib managed EEE support should never have been merged - not
in the way it was. There should have been either a way to transition
drivers to it without causing all this breakage, or there should have
been a commitment to getting this fixed in a timely manner (in the
same cycle that it was merged.)

What we have right now is a total trainwreck.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!