[PATCH net-next v2 1/1] Documentation: networking: expand and clarify EEE_GET/EEE_SET documentation

[PATCH net-next v2 1/1] Documentation: networking: expand and clarify EEE_GET/EEE_SET documentation

[Oleksij Rempel]

Improve the documentation for ETHTOOL_MSG_EEE_GET and ETHTOOL_MSG_EEE_SET
to provide accurate descriptions of all netlink attributes involved.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 Documentation/networking/ethtool-netlink.rst | 111 ++++++++++++++++---
 include/uapi/linux/ethtool.h                 |   3 +
 2 files changed, 96 insertions(+), 18 deletions(-)

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index b6e9af4d0f1b..78ee481437a4 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -1215,20 +1215,16 @@ Kernel response contents:
 
   =====================================  ======  ==========================
   ``ETHTOOL_A_EEE_HEADER``               nested  request header
-  ``ETHTOOL_A_EEE_MODES_OURS``           bool    supported/advertised modes
-  ``ETHTOOL_A_EEE_MODES_PEER``           bool    peer advertised link modes
+  ``ETHTOOL_A_EEE_MODES_OURS``           bitset  supported/advertised modes
+  ``ETHTOOL_A_EEE_MODES_PEER``           bitset  peer advertised link modes
   ``ETHTOOL_A_EEE_ACTIVE``               bool    EEE is actively used
   ``ETHTOOL_A_EEE_ENABLED``              bool    EEE is enabled
-  ``ETHTOOL_A_EEE_TX_LPI_ENABLED``       bool    Tx lpi enabled
-  ``ETHTOOL_A_EEE_TX_LPI_TIMER``         u32     Tx lpi timeout (in us)
+  ``ETHTOOL_A_EEE_TX_LPI_ENABLED``       bool    Tx LPI enabled
+  ``ETHTOOL_A_EEE_TX_LPI_TIMER``         u32     Tx LPI timeout (in us)
   =====================================  ======  ==========================
 
-In ``ETHTOOL_A_EEE_MODES_OURS``, mask consists of link modes for which EEE is
-enabled, value of link modes for which EEE is advertised. Link modes for which
-peer advertises EEE are listed in ``ETHTOOL_A_EEE_MODES_PEER`` (no mask). The
-netlink interface allows reporting EEE status for all link modes but only
-first 32 are provided by the ``ethtool_ops`` callback.
-
+For detailed explanation of each attribute, see the ``EEE Attributes``
+section.
 
 EEE_SET
 =======
@@ -1239,17 +1235,96 @@ Request contents:
 
   =====================================  ======  ==========================
   ``ETHTOOL_A_EEE_HEADER``               nested  request header
-  ``ETHTOOL_A_EEE_MODES_OURS``           bool    advertised modes
+  ``ETHTOOL_A_EEE_MODES_OURS``           bitset  advertised modes
   ``ETHTOOL_A_EEE_ENABLED``              bool    EEE is enabled
-  ``ETHTOOL_A_EEE_TX_LPI_ENABLED``       bool    Tx lpi enabled
-  ``ETHTOOL_A_EEE_TX_LPI_TIMER``         u32     Tx lpi timeout (in us)
+  ``ETHTOOL_A_EEE_TX_LPI_ENABLED``       bool    Tx LPI enabled
+  ``ETHTOOL_A_EEE_TX_LPI_TIMER``         u32     Tx LPI timeout (in us)
   =====================================  ======  ==========================
 
-``ETHTOOL_A_EEE_MODES_OURS`` is used to either list link modes to advertise
-EEE for (if there is no mask) or specify changes to the list (if there is
-a mask). The netlink interface allows reporting EEE status for all link modes
-but only first 32 can be set at the moment as that is what the ``ethtool_ops``
-callback supports.
+For detailed explanation of each attribute, see the ``EEE Attributes``
+section.
+
+EEE Attributes
+==============
+
+Limitations:
+
+The netlink interface allows configuring all link modes up to
+``__ETHTOOL_LINK_MODE_MASK_NBITS``, but if the driver relies on legacy
+``ethtool_ops``, only the first 32 link modes are supported.
+
+The following structure is used for the ioctl interface (``ETHTOOL_GEEE`` and
+``ETHTOOL_SEEE``):
+
+.. kernel-doc:: include/uapi/linux/ethtool.h
+    :identifiers: ethtool_eee
+
+Mapping between netlink attributes and struct fields:
+
+  ================================  ================================
+  Netlink attribute                 struct ethtool_eee field
+  ================================  ================================
+  ``ETHTOOL_A_EEE_MODES_OURS``       advertised
+  ``ETHTOOL_A_EEE_MODES_PEER``       lp_advertised
+  ``ETHTOOL_A_EEE_ACTIVE``           eee_active
+  ``ETHTOOL_A_EEE_ENABLED``          eee_enabled
+  ``ETHTOOL_A_EEE_TX_LPI_ENABLED``   tx_lpi_enabled
+  ``ETHTOOL_A_EEE_TX_LPI_TIMER``     tx_lpi_timer
+  ================================  ================================
+
+
+``ETHTOOL_A_EEE_MODES_OURS`` (bitset)
+-------------------------------------
+- Value: link modes that the driver intends to advertise for EEE.
+- Mask: subset of link modes supported for EEE by the interface.
+
+The advertised EEE capabilities are maintained in software state and persist
+across toggling EEE on or off. If ``ETHTOOL_A_EEE_ENABLED`` is false, the PHY
+does not advertise EEE, but the configured value is reported.
+
+``ETHTOOL_A_EEE_MODES_PEER`` (bitset)
+-------------------------------------
+- Value: link modes that the link partner advertises for EEE.
+- Mask: empty.
+
+This value is typically reported by the hardware and may represent only a
+subset of the actual capabilities supported and advertised by the link partner.
+The local hardware may not be able to detect or represent all EEE-capable modes
+of the peer.
+
+``ETHTOOL_A_EEE_ACTIVE`` (bool)
+-------------------------------
+Indicates whether EEE is currently active on the link. EEE is considered active
+if:
+
+ - ``ETHTOOL_A_EEE_ENABLED`` is true,
+ - Autonegotiation is enabled,
+ - The current link mode is EEE-capable,
+ - Both the local advertisement and the peer advertisement include this link
+   mode.
+
+``ETHTOOL_A_EEE_ENABLED`` (bool)
+--------------------------------
+A software-controlled flag.
+
+When ``ETHTOOL_A_EEE_ENABLED`` is set to true and autonegotiation is active,
+the kernel programs the EEE advertisement settings into the PHY hardware
+registers. This enables negotiation of EEE capability with the link partner.
+
+When ``ETHTOOL_A_EEE_ENABLED`` is set to false, EEE advertisement is disabled.
+The PHY will not include EEE capability in its autonegotiation pages, and EEE
+will not be negotiated even if it remains configured in software state.
+
+``ETHTOOL_A_EEE_TX_LPI_ENABLED`` (bool)
+---------------------------------------
+Controls whether the system may enter the Low Power Idle (LPI) state after
+transmission has stopped.
+
+``ETHTOOL_A_EEE_TX_LPI_TIMER`` (u32)
+------------------------------------
+Defines the delay in microseconds after the last transmitted frame before the
+MAC may enter the Low Power Idle (LPI) state. This value applies globally to
+all link modes. A higher timer value delays LPI entry.
 
 
 TSINFO_GET
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 84833cca29fe..c596618633bc 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -366,6 +366,9 @@ struct ethtool_eeprom {
  *	its tx lpi (after reaching 'idle' state). Effective only when eee
  *	was negotiated and tx_lpi_enabled was set.
  * @reserved: Reserved for future use; see the note on reserved space.
+ *
+ * More detailed documentation can be found in
+ * Documentation/networking/ethtool-netlink.rst section "EEE Attributes".
  */
 struct ethtool_eee {
 	__u32	cmd;
-- 
2.39.5

Re: [PATCH net-next v2 1/1] Documentation: networking: expand and clarify EEE_GET/EEE_SET documentation

[Paolo Abeni]

On 4/27/25 3:40 PM, Oleksij Rempel wrote:
> Improve the documentation for ETHTOOL_MSG_EEE_GET and ETHTOOL_MSG_EEE_SET
> to provide accurate descriptions of all netlink attributes involved.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

This looks like an almost complete rewrite WRT v1, a changelog would
have helped reviewing. I'm unsure if it captures all the feedback from
Russell,

/P

Re: [PATCH net-next v2 1/1] Documentation: networking: expand and clarify EEE_GET/EEE_SET documentation

[Russell King (Oracle)]

On Fri, May 02, 2025 at 10:46:02AM +0200, Paolo Abeni wrote:
> On 4/27/25 3:40 PM, Oleksij Rempel wrote:
> > Improve the documentation for ETHTOOL_MSG_EEE_GET and ETHTOOL_MSG_EEE_SET
> > to provide accurate descriptions of all netlink attributes involved.
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> 
> This looks like an almost complete rewrite WRT v1, a changelog would
> have helped reviewing. I'm unsure if it captures all the feedback from
> Russell,

Indeed, because I'm still of the opinion that we shouldn't be trying to
document the same thing in two different places, but differently, which
will only add confusion, and over time the two descriptions will diverge
making the problem harder.

We need to document this in exactly one place, not two places.

So please, choose one of:

* Documentation/devicetree/bindings/net/ethernet-phy.yaml
* Documentation/networking/phy.rst

and reference one from the other, if necessary improving the
documentation.

Given that phylib is not a DT thing, I believe it should not be
documented in the DT bindings, but people directed to the phylib
documentation (the second) for the clarification of our implementation.

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

Re: [PATCH net-next v2 1/1] Documentation: networking: expand and clarify EEE_GET/EEE_SET documentation

[Oleksij Rempel]

On Fri, May 02, 2025 at 12:06:45PM +0100, Russell King (Oracle) wrote:
> On Fri, May 02, 2025 at 10:46:02AM +0200, Paolo Abeni wrote:
> > On 4/27/25 3:40 PM, Oleksij Rempel wrote:
> > > Improve the documentation for ETHTOOL_MSG_EEE_GET and ETHTOOL_MSG_EEE_SET
> > > to provide accurate descriptions of all netlink attributes involved.
> > > 
> > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > 
> > This looks like an almost complete rewrite WRT v1, a changelog would
> > have helped reviewing. I'm unsure if it captures all the feedback from
> > Russell,
> 
> Indeed, because I'm still of the opinion that we shouldn't be trying to
> document the same thing in two different places, but differently, which
> will only add confusion, and over time the two descriptions will diverge
> making the problem harder.
> 
> We need to document this in exactly one place, not two places.
> 
> So please, choose one of:
> 
> * Documentation/devicetree/bindings/net/ethernet-phy.yaml
> * Documentation/networking/phy.rst
> 
> and reference one from the other, if necessary improving the
> documentation.
> 
> Given that phylib is not a DT thing, I believe it should not be
> documented in the DT bindings, but people directed to the phylib
> documentation (the second) for the clarification of our implementation.

Ok, I see - I wrongly interpreted previous mail. phy.rst - is a good
option. It should replace (extend) documentation in:
Documentation/networking/ethtool-netlink.rst
include/uapi/linux/ethtool.h

Are there any other missing documentation artifacts which should be
replaced?

Best Regards,
Oleksij
-- 
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 |