linux-kernel - Re: [PATCH net-next v1 1/1] phy: micrel: add Signal Quality Indicator (SQI) support for KSZ9477 switch PHYs

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] [day] [month] [year] [list]

Message-ID: <8d5c6585-bc49-498d-9bb9-91d02e8e793f@redhat.com>
Date: Tue, 24 Jun 2025 10:39:31 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Oleksij Rempel <o.rempel@...gutronix.de>, Andrew Lunn <andrew@...n.ch>,
 Heiner Kallweit <hkallweit1@...il.com>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>
Cc: kernel@...gutronix.de, linux-kernel@...r.kernel.org,
 Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v1 1/1] phy: micrel: add Signal Quality Indicator
 (SQI) support for KSZ9477 switch PHYs

On 6/19/25 3:34 PM, Oleksij Rempel wrote:
> The KSZ9477 family of switch chips integrates PHYs that support a
> Signal Quality Indicator (SQI) feature. This feature provides a
> relative measure of receive signal quality, which approximates the
> signal-to-noise ratio and can help detect degraded cabling or
> noisy environments.
> 
> This commit implements the .get_sqi callback for these embedded PHYs
> in the Micrel PHY driver. It uses the MMD PMA/PMD device registers
> (0x01, 0xAC–0xAF) to read raw SQI values from each channel.
> 
> According to the KSZ9477S datasheet (DS00002392C), section 4.1.11:
>   - SQI registers update every 2 µs.
>   - Readings can vary significantly even in stable conditions.
>   - Averaging 30–50 samples is recommended for reliable results.
> 
> The implementation:
>   - Averages 40 samples per channel, with 3 µs delay between reads.
>   - Polls only channel A for 100BASE-TX links.
>   - Polls all four channels (A–D) for 1000BASE-T links.
>   - Returns the *worst* quality (highest raw SQI), inverted to match
>     the Linux convention where higher SQI indicates better signal quality.
> 
> Since there is no direct MDIO access to the PHYs, communication occurs
> via SPI, I2C, or MDIO interfaces with the switch core, which then provides
> an emulated MDIO bus to the integrated PHYs. Due to this level of
> indirection, and the number of reads required for stable SQI sampling,
> read latency becomes noticeable.
> 
> For example, on an i.MX8MP platform with a KSZ9893R switch connected
> via SPI, invoking `ethtool` to read the link status takes approximately
> 200 ms when SQI support is enabled.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@...gutronix.de>
> ---
> 
> This commit currently focuses on single-channel SQI support due to
> budget constraints that prevent immediate extension of the SQI API for
> full multichannel functionality. Despite this limitation, the feature
> still significantly improves diagnostic capabilities for users, and I
> intend to upstream it in its current form.

AFAICS the commit message reflects the 'wannabe'/future implementation
and not the actual one included into this patch.

I think you should reword the commit message, describing the current
implementation.

Also it looks like the SQI value is not inverted, I'm unsure if you
should update the commit message accordingly or actually do the invert
in the code.

Thanks,

Paolo