* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.