linux-kernel - Re: [PATCH net-next v2 5/5] net: phy: dp83td510: add MSE interface support for 10BASE-T1L

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aKLwdrqn-_9KqMaA@pengutronix.de>
Date: Mon, 18 Aug 2025 11:20:54 +0200
From: Oleksij Rempel <o.rempel@...gutronix.de>
To: Maxime Chevallier <maxime.chevallier@...tlin.com>
Cc: Andrew Lunn <andrew@...n.ch>, Jakub Kicinski <kuba@...nel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...nel.org>,
	Donald Hunter <donald.hunter@...il.com>,
	Jonathan Corbet <corbet@....net>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	Kory Maincent <kory.maincent@...tlin.com>,
	Nishanth Menon <nm@...com>, kernel@...gutronix.de,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	UNGLinuxDriver@...rochip.com, linux-doc@...r.kernel.org,
	Michal Kubecek <mkubecek@...e.cz>, Roan van Dijk <roan@...tonic.nl>
Subject: Re: [PATCH net-next v2 5/5] net: phy: dp83td510: add MSE interface
 support for 10BASE-T1L

Hi Maxime,

On Mon, Aug 18, 2025 at 10:15:56AM +0200, Maxime Chevallier wrote:
> Hi Oleksij,
> 
> On Friday, August 15, 2025 08:35 CEST, Oleksij Rempel <o.rempel@...gutronix.de> wrote:
> 
> > Implement get_mse_config() and get_mse_snapshot() for the DP83TD510E
> > to expose its Mean Square Error (MSE) register via the new PHY MSE
> > UAPI.
> > 
> > The DP83TD510E does not document any peak MSE values; it only exposes
> > a single average MSE register used internally to derive SQI. This
> > implementation therefore advertises only PHY_MSE_CAP_AVG, along with
> > LINK and channel-A selectors. Scaling is fixed to 0xFFFF, and the
> > refresh interval/number of symbols are estimated from 10BASE-T1L
> > symbol rate (7.5 MBd) and typical diagnostic intervals (~1 ms).
> > 
> > For 10BASE-T1L deployments, SQI is a reliable indicator of link
> > modulation quality once the link is established, but it does not
> > indicate whether autonegotiation pulses will be correctly received
> > in marginal conditions. MSE provides a direct measurement of slicer
> > error rate that can be used to evaluate if autonegotiation is likely
> > to succeed under a given cable length and condition. In practice,
> > testing such scenarios often requires forcing a fixed-link setup to
> > isolate MSE behaviour from the autonegotiation process.
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@...gutronix.de>
> 
> [...]
> 
> > +static int dp83td510_get_mse_snapshot(struct phy_device *phydev, u32 channel,
> > +				      struct phy_mse_snapshot *snapshot)
> > +{
> > +	int ret;
> > +
> > +	if (channel != PHY_MSE_CHANNEL_LINK &&
> > +	    channel != PHY_MSE_CHANNEL_A)
> > +		return -EOPNOTSUPP;
> 
> The doc in patch 1 says :
> 
>   > + * Link-wide mode:
>   > + *  - Some PHYs only expose a link-wide aggregate MSE, or cannot map their
>   > + *    measurement to a specific channel/pair (e.g. 100BASE-TX when MDI/MDI-X
>   > + *    resolution is unknown). In that case, callers must use the LINK selector.
> 
> The way I understand that is that PHYs will report either channel-specific values or
> link-wide values. Is that correct or are both valid ? In BaseT1 this is the same thing,
> but maybe for consistency, we should report either channel values or link-wide values ?

for 100Base-T1 the LINK and channel-A selectors are effectively the
same, since the PHY only has a single channel. In this case both are
valid, and the driver will return the same answer for either request.

I decided to expose both for consistency:
- on one side, the driver already reports pair_A information for the
  cable test, so it makes sense to allow channel-A here as well;
- on the other side, if a caller such as a generic link-status/health
  request asks for LINK, we can also provide that without special
  casing.

So the driver just answers what it can. For this PHY, LINK and
channel-A map to the same hardware register, and all other selectors
return -EOPNOTSUPP.

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 |