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: > > 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? > > + - > > + 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. > > + - > > + 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. > > + 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 |