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

[Oleksij Rempel <o.rempel@pengutronix.de>]

On Wed, Jun 25, 2025 at 05:33:23PM +0200, Maxime Chevallier wrote:
> Hi Oleksij,
> 
> On Wed, 25 Jun 2025 14:41:26 +0200
> Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> 
> > Add support for the Signal Quality Index (SQI) feature on KSZ9477 family
> > switches. This feature provides a relative measure of receive signal
> > quality.
> > 
> > The KSZ9477 PHY provides four separate SQI values for a 1000BASE-T link,
> > one for each differential pair (Channel A-D). Since the current get_sqi
> > UAPI only supports returning a single value per port, this
> > implementation reads the SQI from Channel A as a representative metric.
> > This can be extended to provide per-channel readings once the UAPI is
> > enhanced for multi-channel support.
> > 
> > The hardware provides a raw 7-bit SQI value (0-127), where lower is
> > better. This raw value is converted to the standard 0-7 scale to provide
> > a usable, interoperable metric for userspace tools, abstracting away
> > hardware-specifics. The mapping to the standard 0-7 SQI scale was
> > determined empirically by injecting a 30MHz sine wave into the receive
> > pair with a signal generator. It was observed that the link becomes
> > unstable and drops when the raw SQI value reaches 8. This
> > implementation is based on these test results.
> 
> [...]
> 
> > +/**
> > + * kszphy_get_sqi - Read, average, and map Signal Quality Index (SQI)
> > + * @phydev: the PHY device
> > + *
> > + * This function reads and processes the raw Signal Quality Index from the
> > + * PHY. Based on empirical testing, a raw value of 8 or higher indicates a
> > + * pre-failure state and is mapped to SQI 0. Raw values from 0-7 are
> > + * mapped to the standard 0-7 SQI scale via a lookup table.
> > + *
> > + * Return: SQI value (0–7), or a negative errno on failure.
> > + */
> > +static int kszphy_get_sqi(struct phy_device *phydev)
> > +{
> > +	int sum = 0;
> > +	int i, val, raw_sqi, avg_raw_sqi;
> > +	u8 channels;
> > +
> > +	/* Determine applicable channels based on link speed */
> > +	if (phydev->speed == SPEED_1000)
> > +		/* TODO: current SQI API only supports 1 channel. */
> > +		channels = 1;
> > +	else if (phydev->speed == SPEED_100)
> > +		channels = 1;
> 
> I understand the placeholder logic waiting for some improved uAPI, but
> this triggers an unused variable warning :( I think the commit log and
> the comment below are enough to explain that this can be improved later
> on.

Grr.. sorry.. 

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