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

Parthiban Veerasooran posted 2 patches 2 months, 4 weeks ago
There is a newer version of this series
[PATCH net-next 1/2] net: phy: phy-c45: add SQI and SQI+ support for OATC14 10Base-T1S PHYs
Posted by Parthiban Veerasooran 2 months, 4 weeks 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).

These changes allow higher-layer drivers and diagnostic tools to assess
link quality more accurately for OATC14 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 | 14 +++++
 drivers/net/phy/phy-c45.c            | 94 ++++++++++++++++++++++++++++
 include/linux/phy.h                  |  2 +
 3 files changed, 110 insertions(+)

diff --git a/drivers/net/phy/mdio-open-alliance.h b/drivers/net/phy/mdio-open-alliance.h
index 6850a3f0b31e..dcf7b685da5c 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,18 @@
 #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_UPDATE		BIT(15)
+#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..47854c8fb9da 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -1695,3 +1695,97 @@ 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
+ *
+ * Reads the Signal Quality Indicator (SQI) or enhanced SQI+ capability and
+ * returns the highest supported 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)
+{
+	int bits;
+	int ret;
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC14_ADFCAP);
+	if (ret < 0)
+		return ret;
+
+	/* SQI+ capability
+	 * 0 - SQI+ is not supported
+	 * X - SQI+ is supported using X bits (0-255 levels)
+	 */
+	bits = FIELD_GET(OATC14_ADFCAP_SQIPLUS_CAPABILITY, ret);
+	if (bits)
+		/* Max sqi+ level supported: (2 ^ bits) - 1 */
+		return BIT(bits) - 1;
+
+	/* 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
+ *
+ * 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:
+ * * Positive SQI/SQI+ value on success
+ * * 0 if SQI update is not available
+ * * Negative errno on failure
+ */
+int genphy_c45_oatc14_get_sqi(struct phy_device *phydev)
+{
+	u8 shift;
+	int ret;
+
+	/* Read SQI value */
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC14_DCQ_SQI);
+	if (ret < 0)
+		return ret;
+
+	/* Check for SQI update */
+	if (!(ret & OATC14_DCQ_SQI_UPDATE))
+		return 0;
+
+	/* Read SQI capability */
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC14_ADFCAP);
+	if (ret < 0)
+		return ret;
+
+	/* Calculate and return SQI+ value if supported */
+	if (ret & OATC14_ADFCAP_SQIPLUS_CAPABILITY) {
+		/* 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 - FIELD_GET(OATC14_ADFCAP_SQIPLUS_CAPABILITY, ret);
+		ret = phy_read_mmd(phydev, MDIO_MMD_VEND2,
+				   MDIO_OATC14_DCQ_SQIPLUS);
+		if (ret < 0)
+			return ret;
+
+		return (ret & OATC14_DCQ_SQIPLUS_VALUE) >> shift;
+	}
+
+	/* Return SQI value if SQI+ capability is not supported */
+	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 bf5457341ca8..a3a8ff099f94 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -2254,6 +2254,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 1/2] net: phy: phy-c45: add SQI and SQI+ support for OATC14 10Base-T1S PHYs
Posted by kernel test robot 2 months, 3 weeks ago
Hi Parthiban,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 68fa5b092efab37a4f08a47b22bb8ca98f7f6223]

url:    https://github.com/intel-lab-lkp/linux/commits/Parthiban-Veerasooran/net-phy-phy-c45-add-SQI-and-SQI-support-for-OATC14-10Base-T1S-PHYs/20251113-195905
base:   68fa5b092efab37a4f08a47b22bb8ca98f7f6223
patch link:    https://lore.kernel.org/r/20251113115206.140339-2-parthiban.veerasooran%40microchip.com
patch subject: [PATCH net-next 1/2] net: phy: phy-c45: add SQI and SQI+ support for OATC14 10Base-T1S PHYs
config: openrisc-defconfig (https://download.01.org/0day-ci/archive/20251114/202511140914.gSDRcCEL-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251114/202511140914.gSDRcCEL-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511140914.gSDRcCEL-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: drivers/net/phy/phy-c45.c:1700 bad line:                                    OATC14 10Base-T1S PHY
>> Warning: drivers/net/phy/phy-c45.c:1742 bad line:                                10Base-T1S PHY

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH net-next 1/2] net: phy: phy-c45: add SQI and SQI+ support for OATC14 10Base-T1S PHYs
Posted by Andrew Lunn 2 months, 4 weeks ago
> +/**
> + * genphy_c45_oatc14_get_sqi - Get Signal Quality Indicator (SQI) from an OATC14
> +			       10Base-T1S PHY
> + * @phydev: pointer to the PHY device structure
> + *
> + * 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:
> + * * Positive SQI/SQI+ value on success
> + * * 0 if SQI update is not available

I thought 0 represented a very bad link? How is the call supposed to
know the difference between a bad link, and no new value, try again
later?

I had a very quick look at the standard. All that update seems to
indicate is that value has been updated since the last poll of that
bit. There is no indication you cannot read the 'old' SQI value if
there has not been an update. So i think you should always return an
SQI value, if it is if 'old'. 

> +	/* Read SQI capability */
> +	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC14_ADFCAP);
> +	if (ret < 0)
> +		return ret;

I wounder if this should be cached somewhere. You don't expect it to
change.

	Andrew
Re: [PATCH net-next 1/2] net: phy: phy-c45: add SQI and SQI+ support for OATC14 10Base-T1S PHYs
Posted by Parthiban.Veerasooran@microchip.com 2 months, 3 weeks ago
Hi Andrew,

On 13/11/25 9:45 pm, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
>> +/**
>> + * genphy_c45_oatc14_get_sqi - Get Signal Quality Indicator (SQI) from an OATC14
>> +                            10Base-T1S PHY
>> + * @phydev: pointer to the PHY device structure
>> + *
>> + * 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:
>> + * * Positive SQI/SQI+ value on success
>> + * * 0 if SQI update is not available
> 
> I thought 0 represented a very bad link? How is the call supposed to
> know the difference between a bad link, and no new value, try again
> later?
Yes, you are right. A value of 0 represents very poor SQI. Thank you for 
pointing out the need to distinguish between bad SQI and ‘no update’ 
values. Anyway, as per your comment below, this code will be removed 
since there is no need to check the update status.
> 
> I had a very quick look at the standard. All that update seems to
> indicate is that value has been updated since the last poll of that
> bit. There is no indication you cannot read the 'old' SQI value if
> there has not been an update. So i think you should always return an
> SQI value, if it is if 'old'.
Yes, I agree. I will update the code so that the reported SQI value is 
returned regardless of the update status in the next version.
> 
>> +     /* Read SQI capability */
>> +     ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC14_ADFCAP);
>> +     if (ret < 0)
>> +             return ret;
> 
> I wounder if this should be cached somewhere. You don't expect it to
> change.
If I understand correctly, do you mean to store the capability details 
in the phydev structure when genphy_c45_oatc14_get_sqi_max() is called, 
and then use them in the genphy_c45_oatc14_get_sqi() function?

In that case, I may need to introduce new parameters in the phydev 
structure. Do you think introducing new parameters in the phydev 
structure is still necessary for this?

Best regards,
Parthiban V
> 
>          Andrew

Re: [PATCH net-next 1/2] net: phy: phy-c45: add SQI and SQI+ support for OATC14 10Base-T1S PHYs
Posted by Andrew Lunn 2 months, 3 weeks ago
> If I understand correctly, do you mean to store the capability details 
> in the phydev structure when genphy_c45_oatc14_get_sqi_max() is called, 
> and then use them in the genphy_c45_oatc14_get_sqi() function?
> 
> In that case, I may need to introduce new parameters in the phydev 
> structure. Do you think introducing new parameters in the phydev 
> structure is still necessary for this?

I'm not sure it is worth it. Do we expect an SNMP agent polling the
SQI value once per second? Once per minute? One extra read per minute
costs nothing. If it was happening more frequently, then it might be
worth caching the capabilities.

How do you see this being used?

	Andrew
Re: [PATCH net-next 1/2] net: phy: phy-c45: add SQI and SQI+ support for OATC14 10Base-T1S PHYs
Posted by Parthiban.Veerasooran@microchip.com 2 months, 3 weeks ago
Hi Andrew,

On 14/11/25 8:02 pm, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
>> If I understand correctly, do you mean to store the capability details
>> in the phydev structure when genphy_c45_oatc14_get_sqi_max() is called,
>> and then use them in the genphy_c45_oatc14_get_sqi() function?
>>
>> In that case, I may need to introduce new parameters in the phydev
>> structure. Do you think introducing new parameters in the phydev
>> structure is still necessary for this?
> 
> I'm not sure it is worth it. Do we expect an SNMP agent polling the
> SQI value once per second? Once per minute? One extra read per minute
> costs nothing. If it was happening more frequently, then it might be
> worth caching the capabilities.
> 
> How do you see this being used?

Good point — thanks for pointing it out.

Currently, ethtool reads the SQI value and displays it as part of the 
statistics. It looks like the current SNMP agent implementation isn’t 
polling the SQI value. However, it is possible that some vendors may 
implement this in their networks, in which case we might run into the 
issue of reading SQI capabilities multiple times.

To be on the safe side, and per your suggestion, I will cache/store the 
SQI capability details in the PHY device structure in the next version.

Best regards,
Parthiban V
> 
>          Andrew