On Thu, Sep 11, 2025 at 07:23:18PM -0700, Jakub Kicinski wrote:

On Mon,  8 Sep 2025 14:46:06 +0200 Oleksij Rempel wrote:

quoted

Add the base infrastructure for Mean Square Error (MSE) diagnostics,
as proposed by the OPEN Alliance "Advanced diagnostic features for
100BASE-T1 automotive Ethernet PHYs" [1] specification.

The OPEN Alliance spec defines only average MSE and average peak MSE
over a fixed number of symbols. However, other PHYs, such as the
KSZ9131, additionally expose a worst-peak MSE value latched since the
last channel capture. This API accounts for such vendor extensions by
adding a distinct capability bit and snapshot field.

Channel-to-pair mapping is normally straightforward, but in some cases
(e.g. 100BASE-TX with MDI-X resolution unknown) the mapping is ambiguous.
If hardware does not expose MDI-X status, the exact pair cannot be
determined. To avoid returning misleading per-channel data in this case,
a LINK selector is defined for aggregate MSE measurements.

All investigated devices differ in MSE configuration parameters, such
as sample rate, number of analyzed symbols, and scaling factors.
For example, the KSZ9131 uses different scaling for MSE and pMSE.
To make this visible to userspace, scale limits and timing information
are returned via get_mse_config().

But the parameter set is set by the standard? If not we should annotate
which one is and which isn't.

Do you mean we should show which parameters are defined by a standard
(for example Open-Alliance - MSE/pMSE) or which parts of the measurement
method - like how many samples in what time - are vendor or product
specific?

And should we only write this in comments/docs, or add a flag/enum so
user space can detect it?

quoted

+  -
+    name: phy-mse-capability
+    doc: |
+      Bitmask flags for MSE capabilities.
+
+      These flags are used in the 'supported_caps' field of struct
+      phy_mse_config to indicate which measurement capabilities are supported
+      by the PHY hardware.
+    type: flags
+    name-prefix: phy-mse-cap-
+    entries:
+      -
+        name: avg
+        doc: Average MSE value is supported.
+      -
+        name: peak
+        doc: Current peak MSE value is supported.
+      -
+        name: worst-peak
+        doc: Worst-case peak MSE (latched high-water mark) is supported.
+      -
+        name: channel-a
+        doc: Diagnostics for Channel A are supported.
+      -
+        name: channel-b
+        doc: Diagnostics for Channel B are supported.
+      -
+        name: channel-c
+        doc: Diagnostics for Channel C are supported.
+      -
+        name: channel-d
+        doc: Diagnostics for Channel D are supported.
+      -
+        name: worst-channel
+        doc: |
+          Hardware or drivers can identify the single worst-performing channel
+          without needing to query each one individually.
+      -
+        name: link
+        doc: |
+          Hardware provides only a link-wide aggregate MSE or cannot map
+          the measurement to a specific channel/pair. Typical for media where
+          the MDI/MDI-X resolution or pair mapping is unknown (e.g. 100BASE-TX).

Should we invert the order here? I think it's more likely we'd
encounter new statistical measures rather than new channels.
So channels should go first, and then the measures?

ack, sounds good.

quoted

+  -
+    name: phy-mse-channel
+    doc: |
+      Identifiers for the 'channel' parameter used to select which diagnostic
+      data to retrieve.
+    type: enum
+    name-prefix: phy-mse-channel-
+    entries:
+      -
+        name: a
+        value: 0

Don't enums default to starting from 0?  I think setting value is unnecessary

ack.

quoted

+        doc: Request data for channel A.

-- 
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 |