* [PATCH net-next v2 0/2] Add SQI and SQI+ support for OATC14 10Base-T1S PHYs and Microchip T1S driver
@ 2025-11-18 4:59 Parthiban Veerasooran
2025-11-18 4:59 ` [PATCH net-next v2 1/2] net: phy: phy-c45: add SQI and SQI+ support for OATC14 10Base-T1S PHYs Parthiban Veerasooran
2025-11-18 4:59 ` [PATCH net-next v2 2/2] net: phy: microchip_t1s: add SQI support for LAN867x Rev.D0 PHYs Parthiban Veerasooran
0 siblings, 2 replies; 5+ messages in thread
From: Parthiban Veerasooran @ 2025-11-18 4:59 UTC (permalink / raw)
To: Parthiban.Veerasooran, piergiorgio.beruto, andrew, hkallweit1,
linux, davem, edumazet, kuba, pabeni
Cc: netdev, linux-kernel, Parthiban Veerasooran
This patch series adds Signal Quality Indicator (SQI) and enhanced SQI+
support for OATC14 10Base-T1S PHYs, along with integration into the
Microchip T1S PHY driver. This enables ethtool to report the SQI value for
OATC14 10Base-T1S PHYs.
Patch Summary:
1. add SQI and SQI+ support for OATC14 10Base-T1S PHYs
- Introduces MDIO register definitions for DCQ_SQI and DCQ_SQIPLUS.
- Adds genphy_c45_oatc14_get_sqi_max() to report the maximum SQI/SQI+
level.
- Adds genphy_c45_oatc14_get_sqi() to return the current SQI or SQI+
value.
- Updates include/linux/phy.h to expose the new APIs.
- SQI+ capability is read from the Advanced Diagnostic Features
Capability register (ADFCAP). If unsupported, the driver falls back
to basic SQI (0–7 levels).
- If SQI+ capability is supported, the function returns the extended
SQI+ value; otherwise, it returns the basic SQI value.
- Open Alliance TC14 10BASE-T1S Advanced Diagnostic PHY Features.
https://opensig.org/wp-content/uploads/2025/06/OPEN_Alliance_10BASE-T1S_Advanced_PHY_features_for-automotive_Ethernet_V2.1b.pdf
2. add SQI support for LAN867x Rev.D0 PHYs
- Registers .get_sqi and .get_sqi_max callbacks in the Microchip T1S
driver.
- Enables network drivers and diagnostic tools to query link signal
quality for LAN867x Rev.D0 PHYs.
- Existing PHY functionality remains unchanged.
v2:
- Updated cover letter description for better clarity.
- Added oatc14_sqiplus_bits variable to cache the SQI+ capability in the
phy device structure.
- Fixed function description comment style warnings reported by the
kernel test robot.
Parthiban Veerasooran (2):
net: phy: phy-c45: add SQI and SQI+ support for OATC14 10Base-T1S PHYs
net: phy: microchip_t1s: add SQI support for LAN867x Rev.D0 PHYs
drivers/net/phy/mdio-open-alliance.h | 13 +++++
drivers/net/phy/microchip_t1s.c | 2 +
drivers/net/phy/phy-c45.c | 86 ++++++++++++++++++++++++++++
include/linux/phy.h | 12 ++++
4 files changed, 113 insertions(+)
base-commit: 7c898b71e59c51ba356aab095ea4ee1f867ad595
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net-next v2 1/2] net: phy: phy-c45: add SQI and SQI+ support for OATC14 10Base-T1S PHYs
2025-11-18 4:59 [PATCH net-next v2 0/2] Add SQI and SQI+ support for OATC14 10Base-T1S PHYs and Microchip T1S driver Parthiban Veerasooran
@ 2025-11-18 4:59 ` Parthiban Veerasooran
2025-11-22 11:46 ` Simon Horman
2025-11-18 4:59 ` [PATCH net-next v2 2/2] net: phy: microchip_t1s: add SQI support for LAN867x Rev.D0 PHYs Parthiban Veerasooran
1 sibling, 1 reply; 5+ messages in thread
From: Parthiban Veerasooran @ 2025-11-18 4:59 UTC (permalink / raw)
To: Parthiban.Veerasooran, piergiorgio.beruto, andrew, hkallweit1,
linux, davem, edumazet, kuba, pabeni
Cc: netdev, linux-kernel, Parthiban Veerasooran
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net-next v2 2/2] net: phy: microchip_t1s: add SQI support for LAN867x Rev.D0 PHYs
2025-11-18 4:59 [PATCH net-next v2 0/2] Add SQI and SQI+ support for OATC14 10Base-T1S PHYs and Microchip T1S driver Parthiban Veerasooran
2025-11-18 4:59 ` [PATCH net-next v2 1/2] net: phy: phy-c45: add SQI and SQI+ support for OATC14 10Base-T1S PHYs Parthiban Veerasooran
@ 2025-11-18 4:59 ` Parthiban Veerasooran
1 sibling, 0 replies; 5+ messages in thread
From: Parthiban Veerasooran @ 2025-11-18 4:59 UTC (permalink / raw)
To: Parthiban.Veerasooran, piergiorgio.beruto, andrew, hkallweit1,
linux, davem, edumazet, kuba, pabeni
Cc: netdev, linux-kernel, Parthiban Veerasooran
Add support for Signal Quality Index (SQI) reporting in the
Microchip T1S PHY driver for LAN867x Rev.D0 (OATC14-compliant) PHYs.
This patch registers the following callbacks in the microchip_t1s driver
structure:
- .get_sqi - returns the current SQI value
- .get_sqi_max - returns the maximum SQI value
This enables ethtool to report the SQI value for LAN867x Rev.D0 PHYs.
Signed-off-by: Parthiban Veerasooran <parthiban.veerasooran@microchip.com>
---
drivers/net/phy/microchip_t1s.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/phy/microchip_t1s.c b/drivers/net/phy/microchip_t1s.c
index 5a0a66778977..e601d56b2507 100644
--- a/drivers/net/phy/microchip_t1s.c
+++ b/drivers/net/phy/microchip_t1s.c
@@ -575,6 +575,8 @@ static struct phy_driver microchip_t1s_driver[] = {
.get_plca_status = genphy_c45_plca_get_status,
.cable_test_start = genphy_c45_oatc14_cable_test_start,
.cable_test_get_status = genphy_c45_oatc14_cable_test_get_status,
+ .get_sqi = genphy_c45_oatc14_get_sqi,
+ .get_sqi_max = genphy_c45_oatc14_get_sqi_max,
},
{
PHY_ID_MATCH_EXACT(PHY_ID_LAN865X_REVB),
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next v2 1/2] net: phy: phy-c45: add SQI and SQI+ support for OATC14 10Base-T1S PHYs
2025-11-18 4:59 ` [PATCH net-next v2 1/2] net: phy: phy-c45: add SQI and SQI+ support for OATC14 10Base-T1S PHYs Parthiban Veerasooran
@ 2025-11-22 11:46 ` Simon Horman
2025-11-25 13:10 ` Parthiban.Veerasooran
0 siblings, 1 reply; 5+ messages in thread
From: Simon Horman @ 2025-11-22 11:46 UTC (permalink / raw)
To: Parthiban Veerasooran
Cc: piergiorgio.beruto, andrew, hkallweit1, linux, davem, edumazet,
kuba, pabeni, netdev, linux-kernel
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);
...
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next v2 1/2] net: phy: phy-c45: add SQI and SQI+ support for OATC14 10Base-T1S PHYs
2025-11-22 11:46 ` Simon Horman
@ 2025-11-25 13:10 ` Parthiban.Veerasooran
0 siblings, 0 replies; 5+ messages in thread
From: Parthiban.Veerasooran @ 2025-11-25 13:10 UTC (permalink / raw)
To: horms
Cc: piergiorgio.beruto, andrew, hkallweit1, linux, davem, edumazet,
kuba, pabeni, netdev, linux-kernel
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);
>
> ...
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-11-25 13:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-18 4:59 [PATCH net-next v2 0/2] Add SQI and SQI+ support for OATC14 10Base-T1S PHYs and Microchip T1S driver Parthiban Veerasooran
2025-11-18 4:59 ` [PATCH net-next v2 1/2] net: phy: phy-c45: add SQI and SQI+ support for OATC14 10Base-T1S PHYs Parthiban Veerasooran
2025-11-22 11:46 ` Simon Horman
2025-11-25 13:10 ` Parthiban.Veerasooran
2025-11-18 4:59 ` [PATCH net-next v2 2/2] net: phy: microchip_t1s: add SQI support for LAN867x Rev.D0 PHYs Parthiban Veerasooran
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).