[PATCH net-next v2 1/2] net: phy: phy-c45: add SQI and SQI+ support for OATC14 10Base-T1S PHYs

Parthiban Veerasooran posted 2 patches 1 week, 6 days ago
There is a newer version of this series
[PATCH net-next v2 1/2] net: phy: phy-c45: add SQI and SQI+ support for OATC14 10Base-T1S PHYs
Posted by Parthiban Veerasooran 1 week, 6 days ago
Add support for reading Signal Quality Indicator (SQI) and enhanced SQI+
from OATC14 10Base-T1S PHYs.

- Introduce MDIO register definitions for DCQ_SQI and DCQ_SQIPLUS.
- Add `genphy_c45_oatc14_get_sqi_max()` to return the maximum supported
  SQI/SQI+ level.
- Add `genphy_c45_oatc14_get_sqi()` to return the current SQI or SQI+
  value.
- Update `include/linux/phy.h` to expose the new APIs.

SQI+ capability is read from the Advanced Diagnostic Features Capability
register (ADFCAP). If SQI+ is supported, the driver calculates the value
from the MSBs of the DCQ_SQIPLUS register; otherwise, it falls back to
basic SQI (0-7 levels). This enables ethtool to report the SQI value for
OATC14 10Base-T1S PHYs.

Open Alliance TC14 10BASE-T1S Advanced Diagnostic PHY Features
Specification ref:
https://opensig.org/wp-content/uploads/2025/06/OPEN_Alliance_10BASE-T1S_Advanced_PHY_features_for-automotive_Ethernet_V2.1b.pdf

Signed-off-by: Parthiban Veerasooran <parthiban.veerasooran@microchip.com>
---
 drivers/net/phy/mdio-open-alliance.h | 13 +++++
 drivers/net/phy/phy-c45.c            | 86 ++++++++++++++++++++++++++++
 include/linux/phy.h                  | 12 ++++
 3 files changed, 111 insertions(+)

diff --git a/drivers/net/phy/mdio-open-alliance.h b/drivers/net/phy/mdio-open-alliance.h
index 6850a3f0b31e..449d0fb67093 100644
--- a/drivers/net/phy/mdio-open-alliance.h
+++ b/drivers/net/phy/mdio-open-alliance.h
@@ -56,6 +56,8 @@
 /* Advanced Diagnostic Features Capability Register*/
 #define MDIO_OATC14_ADFCAP		0xcc00
 #define OATC14_ADFCAP_HDD_CAPABILITY	GENMASK(10, 8)
+#define OATC14_ADFCAP_SQIPLUS_CAPABILITY	GENMASK(4, 1)
+#define OATC14_ADFCAP_SQI_CAPABILITY	BIT(0)
 
 /* Harness Defect Detection Register */
 #define MDIO_OATC14_HDD			0xcc01
@@ -65,6 +67,17 @@
 #define OATC14_HDD_VALID		BIT(2)
 #define OATC14_HDD_SHORT_OPEN_STATUS	GENMASK(1, 0)
 
+/* Dynamic Channel Quality SQI Register */
+#define MDIO_OATC14_DCQ_SQI		0xcc03
+#define OATC14_DCQ_SQI_VALUE		GENMASK(2, 0)
+
+/* Dynamic Channel Quality SQI Plus Register */
+#define MDIO_OATC14_DCQ_SQIPLUS		0xcc04
+#define OATC14_DCQ_SQIPLUS_VALUE	GENMASK(7, 0)
+
+/* SQI is supported using 3 bits means 8 levels (0-7) */
+#define OATC14_SQI_MAX_LEVEL		7
+
 /* Bus Short/Open Status:
  * 0 0 - no fault; everything is ok. (Default)
  * 0 1 - detected as an open or missing termination(s)
diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index e8e5be4684ab..eb496a900780 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -1695,3 +1695,89 @@ int genphy_c45_oatc14_cable_test_start(struct phy_device *phydev)
 				OATC14_HDD_START_CONTROL);
 }
 EXPORT_SYMBOL(genphy_c45_oatc14_cable_test_start);
+
+/**
+ * genphy_c45_oatc14_get_sqi_max - Get maximum supported SQI or SQI+ level of
+ *				   OATC14 10Base-T1S PHY
+ * @phydev: pointer to the PHY device structure
+ *
+ * This function reads the advanced capability register to determine the maximum
+ * supported Signal Quality Indicator (SQI) or SQI+ level
+ *
+ * Return:
+ * * Maximum SQI/SQI+ level (≥0)
+ * * -EOPNOTSUPP if not supported
+ * * Negative errno on read failure
+ */
+int genphy_c45_oatc14_get_sqi_max(struct phy_device *phydev)
+{
+	u8 bits;
+	int ret;
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC14_ADFCAP);
+	if (ret < 0)
+		return ret;
+
+	/* Check for SQI+ capability
+	 * 0 - SQI+ is not supported
+	 * (3-8) bits for (8-256) SQI+ levels supported
+	 */
+	bits = FIELD_GET(OATC14_ADFCAP_SQIPLUS_CAPABILITY, ret);
+	if (bits) {
+		phydev->oatc14_sqiplus_bits = bits;
+		/* Max sqi+ level supported: (2 ^ bits) - 1 */
+		return BIT(bits) - 1;
+	}
+
+	/* Check for SQI capability
+	 * 0 - SQI is not supported
+	 * 1 - SQI is supported (0-7 levels)
+	 */
+	if (ret & OATC14_ADFCAP_SQI_CAPABILITY)
+		return OATC14_SQI_MAX_LEVEL;
+
+	return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL(genphy_c45_oatc14_get_sqi_max);
+
+/**
+ * genphy_c45_oatc14_get_sqi - Get Signal Quality Indicator (SQI) from an OATC14
+ *			       10Base-T1S PHY
+ * @phydev: pointer to the PHY device structure
+ *
+ * This function reads the SQI+ or SQI value from an OATC14-compatible
+ * 10Base-T1S PHY. If SQI+ capability is supported, the function returns the
+ * extended SQI+ value; otherwise, it returns the basic SQI value.
+ *
+ * Return:
+ * * SQI/SQI+ value on success
+ * * Negative errno on read failure
+ */
+int genphy_c45_oatc14_get_sqi(struct phy_device *phydev)
+{
+	u8 shift;
+	int ret;
+
+	/* Calculate and return SQI+ value if supported */
+	if (phydev->oatc14_sqiplus_bits) {
+		ret = phy_read_mmd(phydev, MDIO_MMD_VEND2,
+				   MDIO_OATC14_DCQ_SQIPLUS);
+		if (ret < 0)
+			return ret;
+
+		/* SQI+ uses N MSBs out of 8 bits, left-aligned with padding 1's
+		 * Calculate the right-shift needed to isolate the N bits.
+		 */
+		shift = 8 - phydev->oatc14_sqiplus_bits;
+
+		return (ret & OATC14_DCQ_SQIPLUS_VALUE) >> shift;
+	}
+
+	/* Read and return SQI value if SQI+ capability is not supported */
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC14_DCQ_SQI);
+	if (ret < 0)
+		return ret;
+
+	return ret & OATC14_DCQ_SQI_VALUE;
+}
+EXPORT_SYMBOL(genphy_c45_oatc14_get_sqi);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 65b0c3ca6a2b..841006fac16a 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -626,6 +626,14 @@ struct macsec_ops;
  * @link_down_events: Number of times link was lost
  * @shared: Pointer to private data shared by phys in one package
  * @priv: Pointer to driver private data
+ * @oatc14_sqiplus_bits: Number of bits for sqi+ level supported
+ *        0 - SQI+ is not supported
+ *        3 - SQI+ is supported, using 3 bits (8 levels)
+ *        4 - SQI+ is supported, using 4 bits (16 levels)
+ *        5 - SQI+ is supported, using 5 bits (32 levels)
+ *        6 - SQI+ is supported, using 6 bits (64 levels)
+ *        7 - SQI+ is supported, using 7 bits (128 levels)
+ *        8 - SQI+ is supported, using 8 bits (256 levels)
  *
  * interrupts currently only supports enabled or disabled,
  * but could be changed in the future to support enabling
@@ -772,6 +780,8 @@ struct phy_device {
 	/* MACsec management functions */
 	const struct macsec_ops *macsec_ops;
 #endif
+
+	u8 oatc14_sqiplus_bits;
 };
 
 /* Generic phy_device::dev_flags */
@@ -2257,6 +2267,8 @@ int genphy_c45_an_config_eee_aneg(struct phy_device *phydev);
 int genphy_c45_oatc14_cable_test_start(struct phy_device *phydev);
 int genphy_c45_oatc14_cable_test_get_status(struct phy_device *phydev,
 					    bool *finished);
+int genphy_c45_oatc14_get_sqi_max(struct phy_device *phydev);
+int genphy_c45_oatc14_get_sqi(struct phy_device *phydev);
 
 /* The gen10g_* functions are the old Clause 45 stub */
 int gen10g_config_aneg(struct phy_device *phydev);
-- 
2.34.1

Re: [PATCH net-next v2 1/2] net: phy: phy-c45: add SQI and SQI+ support for OATC14 10Base-T1S PHYs
Posted by Simon Horman 1 week, 2 days ago
On Tue, Nov 18, 2025 at 10:29:45AM +0530, Parthiban Veerasooran wrote:

...

> @@ -1695,3 +1695,89 @@ int genphy_c45_oatc14_cable_test_start(struct phy_device *phydev)
>  				OATC14_HDD_START_CONTROL);
>  }
>  EXPORT_SYMBOL(genphy_c45_oatc14_cable_test_start);
> +
> +/**
> + * genphy_c45_oatc14_get_sqi_max - Get maximum supported SQI or SQI+ level of
> + *				   OATC14 10Base-T1S PHY
> + * @phydev: pointer to the PHY device structure
> + *
> + * This function reads the advanced capability register to determine the maximum
> + * supported Signal Quality Indicator (SQI) or SQI+ level
> + *
> + * Return:
> + * * Maximum SQI/SQI+ level (≥0)
> + * * -EOPNOTSUPP if not supported
> + * * Negative errno on read failure
> + */
> +int genphy_c45_oatc14_get_sqi_max(struct phy_device *phydev)
> +{
> +	u8 bits;
> +	int ret;
> +
> +	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC14_ADFCAP);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Check for SQI+ capability
> +	 * 0 - SQI+ is not supported
> +	 * (3-8) bits for (8-256) SQI+ levels supported
> +	 */
> +	bits = FIELD_GET(OATC14_ADFCAP_SQIPLUS_CAPABILITY, ret);
> +	if (bits) {
> +		phydev->oatc14_sqiplus_bits = bits;
> +		/* Max sqi+ level supported: (2 ^ bits) - 1 */
> +		return BIT(bits) - 1;
> +	}
> +
> +	/* Check for SQI capability
> +	 * 0 - SQI is not supported
> +	 * 1 - SQI is supported (0-7 levels)
> +	 */
> +	if (ret & OATC14_ADFCAP_SQI_CAPABILITY)
> +		return OATC14_SQI_MAX_LEVEL;
> +
> +	return -EOPNOTSUPP;
> +}
> +EXPORT_SYMBOL(genphy_c45_oatc14_get_sqi_max);
> +
> +/**
> + * genphy_c45_oatc14_get_sqi - Get Signal Quality Indicator (SQI) from an OATC14
> + *			       10Base-T1S PHY
> + * @phydev: pointer to the PHY device structure
> + *
> + * This function reads the SQI+ or SQI value from an OATC14-compatible
> + * 10Base-T1S PHY. If SQI+ capability is supported, the function returns the
> + * extended SQI+ value; otherwise, it returns the basic SQI value.
> + *
> + * Return:
> + * * SQI/SQI+ value on success
> + * * Negative errno on read failure
> + */
> +int genphy_c45_oatc14_get_sqi(struct phy_device *phydev)
> +{
> +	u8 shift;
> +	int ret;
> +
> +	/* Calculate and return SQI+ value if supported */
> +	if (phydev->oatc14_sqiplus_bits) {

Hi Parthiban,

AFAICT oatc14_sqiplus_bits will always be 0 until
genphy_c45_oatc14_get_sqi_max() is called, after which
it may be some other value.

But according to the flow of linkstate_prepare_data()
it seems that genphy_c45_oatc14_get_sqi_max()
will be called after genphy_c45_oatc14_get_sqi().

In which case the condition above will be false
(unless genphy_c45_oatc14_get_sqi_max was somehow already
called by some other means.)

This doesn't seem to be in line with the intention of this code.

Flagged by Claude Code with https://github.com/masoncl/review-prompts/

> +		ret = phy_read_mmd(phydev, MDIO_MMD_VEND2,
> +				   MDIO_OATC14_DCQ_SQIPLUS);
> +		if (ret < 0)
> +			return ret;
> +
> +		/* SQI+ uses N MSBs out of 8 bits, left-aligned with padding 1's
> +		 * Calculate the right-shift needed to isolate the N bits.
> +		 */
> +		shift = 8 - phydev->oatc14_sqiplus_bits;
> +
> +		return (ret & OATC14_DCQ_SQIPLUS_VALUE) >> shift;
> +	}
> +
> +	/* Read and return SQI value if SQI+ capability is not supported */
> +	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC14_DCQ_SQI);
> +	if (ret < 0)
> +		return ret;
> +
> +	return ret & OATC14_DCQ_SQI_VALUE;
> +}
> +EXPORT_SYMBOL(genphy_c45_oatc14_get_sqi);

...
Re: [PATCH net-next v2 1/2] net: phy: phy-c45: add SQI and SQI+ support for OATC14 10Base-T1S PHYs
Posted by Parthiban.Veerasooran@microchip.com 6 days, 13 hours ago
Hi Simon,

Thank you for reviewing the patch.

On 22/11/25 5:16 pm, Simon Horman wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Tue, Nov 18, 2025 at 10:29:45AM +0530, Parthiban Veerasooran wrote:
> 
> ...
> 
>> @@ -1695,3 +1695,89 @@ int genphy_c45_oatc14_cable_test_start(struct phy_device *phydev)
>>                                OATC14_HDD_START_CONTROL);
>>   }
>>   EXPORT_SYMBOL(genphy_c45_oatc14_cable_test_start);
>> +
>> +/**
>> + * genphy_c45_oatc14_get_sqi_max - Get maximum supported SQI or SQI+ level of
>> + *                              OATC14 10Base-T1S PHY
>> + * @phydev: pointer to the PHY device structure
>> + *
>> + * This function reads the advanced capability register to determine the maximum
>> + * supported Signal Quality Indicator (SQI) or SQI+ level
>> + *
>> + * Return:
>> + * * Maximum SQI/SQI+ level (≥0)
>> + * * -EOPNOTSUPP if not supported
>> + * * Negative errno on read failure
>> + */
>> +int genphy_c45_oatc14_get_sqi_max(struct phy_device *phydev)
>> +{
>> +     u8 bits;
>> +     int ret;
>> +
>> +     ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC14_ADFCAP);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     /* Check for SQI+ capability
>> +      * 0 - SQI+ is not supported
>> +      * (3-8) bits for (8-256) SQI+ levels supported
>> +      */
>> +     bits = FIELD_GET(OATC14_ADFCAP_SQIPLUS_CAPABILITY, ret);
>> +     if (bits) {
>> +             phydev->oatc14_sqiplus_bits = bits;
>> +             /* Max sqi+ level supported: (2 ^ bits) - 1 */
>> +             return BIT(bits) - 1;
>> +     }
>> +
>> +     /* Check for SQI capability
>> +      * 0 - SQI is not supported
>> +      * 1 - SQI is supported (0-7 levels)
>> +      */
>> +     if (ret & OATC14_ADFCAP_SQI_CAPABILITY)
>> +             return OATC14_SQI_MAX_LEVEL;
>> +
>> +     return -EOPNOTSUPP;
>> +}
>> +EXPORT_SYMBOL(genphy_c45_oatc14_get_sqi_max);
>> +
>> +/**
>> + * genphy_c45_oatc14_get_sqi - Get Signal Quality Indicator (SQI) from an OATC14
>> + *                          10Base-T1S PHY
>> + * @phydev: pointer to the PHY device structure
>> + *
>> + * This function reads the SQI+ or SQI value from an OATC14-compatible
>> + * 10Base-T1S PHY. If SQI+ capability is supported, the function returns the
>> + * extended SQI+ value; otherwise, it returns the basic SQI value.
>> + *
>> + * Return:
>> + * * SQI/SQI+ value on success
>> + * * Negative errno on read failure
>> + */
>> +int genphy_c45_oatc14_get_sqi(struct phy_device *phydev)
>> +{
>> +     u8 shift;
>> +     int ret;
>> +
>> +     /* Calculate and return SQI+ value if supported */
>> +     if (phydev->oatc14_sqiplus_bits) {
> 
> Hi Parthiban,
> 
> AFAICT oatc14_sqiplus_bits will always be 0 until
> genphy_c45_oatc14_get_sqi_max() is called, after which
> it may be some other value.
> 
> But according to the flow of linkstate_prepare_data()
> it seems that genphy_c45_oatc14_get_sqi_max()
> will be called after genphy_c45_oatc14_get_sqi().
> 
> In which case the condition above will be false
> (unless genphy_c45_oatc14_get_sqi_max was somehow already
> called by some other means.)
> 
> This doesn't seem to be in line with the intention of this code.
> 
> Flagged by Claude Code with https://github.com/masoncl/review-prompts/
Good catch! oatc14_sqiplus_bits is not updated the first time 
genphy_c45_oatc14_get_sqi() is called, and is later updated by 
genphy_c45_oatc14_get_sqi_max(). Thank you for pointing it out; I will 
fix it in the next version.

Best regards,
Parthiban V
> 
>> +             ret = phy_read_mmd(phydev, MDIO_MMD_VEND2,
>> +                                MDIO_OATC14_DCQ_SQIPLUS);
>> +             if (ret < 0)
>> +                     return ret;
>> +
>> +             /* SQI+ uses N MSBs out of 8 bits, left-aligned with padding 1's
>> +              * Calculate the right-shift needed to isolate the N bits.
>> +              */
>> +             shift = 8 - phydev->oatc14_sqiplus_bits;
>> +
>> +             return (ret & OATC14_DCQ_SQIPLUS_VALUE) >> shift;
>> +     }
>> +
>> +     /* Read and return SQI value if SQI+ capability is not supported */
>> +     ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC14_DCQ_SQI);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     return ret & OATC14_DCQ_SQI_VALUE;
>> +}
>> +EXPORT_SYMBOL(genphy_c45_oatc14_get_sqi);
> 
> ...