[net-next,v4,00/18] net: stmmac: clean up and fix EEE implementation

Message

Russell King (Oracle) Jan. 8, 2025, 4:47 p.m. UTC

Hi,

This is a rework of stmmac's EEE support in light of the addition of EEE
management to phylib. It's slightly more than 15 patches, but I think it
makes sense to be so.

Patch 1 adds configuration of the receive clock phy_eee_rx_clock_stop()
(which was part of another series, but is necessary for this patch set.)

Patch 2 converts stmmac to use phylib's tracking of tx_lpi_timer.

Patch 3 corrects the data type used for things involving the LPI
timer. The user API uses u32, so stmmac should do too, rather than
blindly converting it to "int". eee_timer is left for patch 4.

Patch 4 (new) uses an unsigned int for eee_timer.

Patch 5 makes stmmac EEE state depend on phylib's enable_tx_lpi flag,
thus using phylib's resolution of EEE state.

Patch 6 removes redundant code from the ethtool EEE operations.

Patch 7 removes some redundant code in stmmac_disable_eee_mode()
and renames it to stmmac_disable_sw_eee_mode() to better reflect its
purpose.

Patch 8 removes the driver private tx_lpi_enabled, which is managed by
phylib since patch 4.

Patch 9 removes the dependence of EEE error statistics on the EEE
enable state, instead depending on whether EEE is supported by the
hardware.

Patch 10 removes phy_init_eee(), instead using phy_eee_rx_clock_stop()
to configure whether the PHY may stop the receive clock.

Patch 11 removes priv->eee_tw_timer, which is only ever set to one
value at probe time, effectively it is a constant. Hence this is
unnecessary complexity.

Patch 12 moves priv->eee_enabled into stmmac_eee_init(), and placing
it under the protection of priv->lock, except when EEE is not
supported (where it becomes constant-false.)

Patch 13 moves priv->eee_active also into stmmac_eee_init(), so
the indication whether EEE should be enabled or not is passed in
to this function.

Since both priv->eee_enabled and priv->eee_active are assigned
true/false values, they should be typed "bool". Make it sew in
patch 14. No Singer machine required.

Patch 15 moves the initialisation of priv->eee_ctrl_timer to the
probe function - it makes no sense to re-initialise the timer each
time we want to start using it.

Patch 16 removes the unnecessary EEE handling in the driver tear-down
method. The core net code will have brought the interface down
already, meaning EEE has already been disabled.

Patch 17 reorganises the code to split the hardware LPI timer
control paths from the software LPI timer paths.

Patch 18 works on this further by eliminating
stmmac_lpi_entry_timer_config() and making direct calls to the new
functions. This reveals a potential bug where priv->eee_sw_timer_en
is set true when EEE is disabled. This is not addressed in this
series, but will be in a future separate patch - so that if fixing
that causes a regression, it can be handled separately.

 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c  |   4 +-
 drivers/net/ethernet/stmicro/stmmac/hwif.h         |   2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac.h       |  10 +-
 .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c   |  25 +----
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 103 +++++++++++----------
 drivers/net/phy/phy.c                              |  27 +++++-
 include/linux/phy.h                                |   1 +
 7 files changed, 85 insertions(+), 87 deletions(-)

Changes in v3:
- fix kerneldoc issue in old patch 12.
- add Andrew's r-bs.
- split eee_timer from other u32 conversions, remove check for
  negative eee_timer.
Changes in v4:
- correct LPI timer initialisation bug, spotted by Choong Yong Liang
- added Choong Yong Liang tested-by to the series

Comments

[e-mail] Jan. 11, 2025, 2:10 a.m. UTC | #1

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <[e-mail]>:

On Wed, 8 Jan 2025 16:47:24 +0000 you wrote:
> Hi,
> 
> This is a rework of stmmac's EEE support in light of the addition of EEE
> management to phylib. It's slightly more than 15 patches, but I think it
> makes sense to be so.
> 
> Patch 1 adds configuration of the receive clock phy_eee_rx_clock_stop()
> (which was part of another series, but is necessary for this patch set.)
> 
> [...]

Here is the summary with links:
  - [net-next,v4,01/18] net: phy: add configuration of rx clock stop mode
    https://git.kernel.org/netdev/net-next/c/cf337105ad38
  - [net-next,v4,02/18] net: stmmac: move tx_lpi_timer tracking to phylib
    https://git.kernel.org/netdev/net-next/c/1991819debaa
  - [net-next,v4,03/18] net: stmmac: use correct type for tx_lpi_timer
    https://git.kernel.org/netdev/net-next/c/bba9f4765515
  - [net-next,v4,04/18] net: stmmac: use unsigned int for eee_timer
    https://git.kernel.org/netdev/net-next/c/7e19a351b22d
  - [net-next,v4,05/18] net: stmmac: make EEE depend on phy->enable_tx_lpi
    https://git.kernel.org/netdev/net-next/c/beb1e0148e6d
  - [net-next,v4,06/18] net: stmmac: remove redundant code from ethtool EEE ops
    https://git.kernel.org/netdev/net-next/c/80fada6c0d3e
  - [net-next,v4,07/18] net: stmmac: clean up stmmac_disable_eee_mode()
    https://git.kernel.org/netdev/net-next/c/e40dd46d2fc5
  - [net-next,v4,08/18] net: stmmac: remove priv->tx_lpi_enabled
    https://git.kernel.org/netdev/net-next/c/865ff410a071
  - [net-next,v4,09/18] net: stmmac: report EEE error statistics if EEE is supported
    https://git.kernel.org/netdev/net-next/c/517dc0450675
  - [net-next,v4,10/18] net: stmmac: convert to use phy_eee_rx_clock_stop()
    https://git.kernel.org/netdev/net-next/c/a3242177d9f2
  - [net-next,v4,11/18] net: stmmac: remove priv->eee_tw_timer
    https://git.kernel.org/netdev/net-next/c/2914a5cd811a
  - [net-next,v4,12/18] net: stmmac: move priv->eee_enabled into stmmac_eee_init()
    https://git.kernel.org/netdev/net-next/c/0a900ea89a0c
  - [net-next,v4,13/18] net: stmmac: move priv->eee_active into stmmac_eee_init()
    https://git.kernel.org/netdev/net-next/c/1797dd4e3e8e
  - [net-next,v4,14/18] net: stmmac: use boolean for eee_enabled and eee_active
    https://git.kernel.org/netdev/net-next/c/cfd49e5fc30c
  - [net-next,v4,15/18] net: stmmac: move setup of eee_ctrl_timer to stmmac_dvr_probe()
    https://git.kernel.org/netdev/net-next/c/84f2776e3919
  - [net-next,v4,16/18] net: stmmac: remove unnecessary EEE handling in stmmac_release()
    https://git.kernel.org/netdev/net-next/c/27af08164247
  - [net-next,v4,17/18] net: stmmac: split hardware LPI timer control
    https://git.kernel.org/netdev/net-next/c/17f47da103a6
  - [net-next,v4,18/18] net: stmmac: remove stmmac_lpi_entry_timer_config()
    https://git.kernel.org/netdev/net-next/c/1655a2279971

You are awesome, thank you!