[PATCH net-next v4 0/4] net: microchip_t1s: fix collision detection on PLCA status change

[PATCH net-next v4 0/4] net: microchip_t1s: fix collision detection on PLCA status change

[Parthiban Veerasooran]

In a 10BASE-T1S multidrop network, the PHY can autonomously change its PLCA
state based on BEACON availability. The existing collision detection logic
only synchronized CDEN on explicit PLCA configuration changes, leaving it
stale across autonomous PLCA status transitions.

This series fixes that gap by tracking PLCA status changes through the PHY
interrupt path, providing a polling fallback for PHYs without a routed
interrupt, and improving collision detection and link-status handling for
LAN867X Rev.D0.

The OA TC6 MAC-PHY SPI driver delivers the in-band PHY interrupt to phylib
through a nested virtual IRQ. LAN865X enables this path for its internal
PHY, which has no dedicated interrupt line. This allows the PHY driver to
receive PLCA status change interrupts through the MAC-PHY SPI interface.

For LAN86XX PHYs, collision detection state is synchronized across explicit
PLCA configuration changes, PHY interrupt handling, and the polling status
path. A per-PHY mutex serializes collision-detection control updates
between PHY configuration and interrupt handling.

LAN867X Rev.D0 uses its hardware CCMFC mechanism to autonomously gate
collision forwarding based on live PLCA_Status, avoiding the software CDEN
toggling used on older revisions. Its link-status handling also accounts
for the optional PRSCTL1 CSMA/CD fallback, selecting the semaphore source
when the PHY can autonomously fall back to CSMA/CD.

Changes in v4:
Addresses Sashiko AI review feedback on v3.
- Add a polling fallback to synchronize CDEN from the live PLCA status for
  PHYs without a routed PHY interrupt.
- Serialize accesses to the collision-detection control register across
  PLCA configuration, interrupt configuration, interrupt handling, and
  status polling to avoid races.
- Preserve the tri-state semantics of plca_cfg->enabled, so an ethtool
  request with the attribute set to -1 does not unintentionally modify CDEN
  or Rev.D0 link-status configuration.
- Fix the OA TC6 virtual IRQ masking path so disabling the nested PHY IRQ
  also masks the in-band PHY interrupt source, and retain deferred dispatch
  to phylib.
- Enable the OA TC6 virtual PHY interrupt for LAN865X and clarify that this
  completes the collision-detection fix for LAN865X. Add the corresponding
  Fixes tag.
- Update LAN867X Rev.D0 interrupt handling to use the cached PLCA enable
  state and current CSMA/CD fallback configuration instead of re-reading
  the complete PLCA configuration on every PLCA status change.
- Preserve cable-test polling when the Rev.D0 PHY interrupt path is
  enabled.
- Clarify the Rev.D0 CDEN/CCMFC behavior and document that CDEN remains
  enabled by default while CCMFC autonomously gates collision forwarding
  from PLCA_Status.
- Correct commit-message and register-comment details identified during
  review.

The LAN865X fix depends on the OA TC6 virtual IRQ support, so the series
should be applied together.

Changes in v3:
Addresses Sashiko AI review feedback on v2.
- Patch 1: Synchronize CDEN with live PLCA status before unmasking
  PSTCM, closing a window where a status change could be silently
  dropped. Use phy_interrupt_is_valid() instead of testing PHY_POLL
  alone. Treat plca_cfg->enabled as tri-state so an ethtool call that
  omits the enable attribute no longer disables collision detection.
  Factor shared STS1/IMSK1 sequences into helpers reused by patch 4.
- Patch 2: Replace dummy_irq_chip with a proper irq_chip implementing
  mask/unmask via bus_lock/bus_sync_unlock, closing an interrupt-storm
  risk. Select IRQ_DOMAIN in Kconfig. Defer PHY interrupt dispatch to a
  workqueue so the chunk-processing thread stays independent of
  phydev->lock.
- Patch 3: Add Fixes: 78341049fbcd, since this patch is required for
  the fix to take effect on LAN865X. Document that CDEN correctness
  relies on the hardware reset default.
- Patch 4: Give Rev.D0 its own config_intr() instead of branching
  inside the shared one, so CCMFC-owned CDEN can never be touched by
  the shared resync. Skip link-status updates when enabled == -1.
  Correct the AN1760 -> AN1699 reference. Resync Rev.D0 link status on
  interrupt (re-)enable, closing the same dropped-edge window as patch
  1. Account for Rev.D0's autonomous PLCA-to-CSMA/CD fallback
  (PRSCTL1): force semaphore mode when that fallback is enabled, since
  PLCA_Status is meaningless once the PHY has already fallen back.

Changes in v2:
- Patch 2: Introduce OA_TC6_PHY_INT quirk flag to guard the virtual IRQ
  infrastructure; PHYINT is optional per the OA TC6 standard. Move
  oa_tc6_phy_irq_setup()/oa_tc6_phy_irq_teardown() into
  oa_tc6_mdiobus_register()/oa_tc6_mdiobus_unregister(). Populate all
  mii_bus->irq[] entries with the virtual IRQ before mdiobus_register()
  so phy_device_create() picks it up regardless of the PHY's MDIO
  address, keeping mii_bus->irq[] and phydev->irq consistent.
- Patch 3 (new): Pass OA_TC6_PHY_INT quirk flag from the LAN865X driver
  to enable the virtual IRQ infrastructure for LAN865X.


Parthiban Veerasooran (4):
  net: phy: microchip_t1s: fix collision detection on PLCA status change
  net: ethernet: oa_tc6: deliver the PHY interrupt to phylib
  microchip: lan865x: enable PHY interrupt via virtual IRQ
  net: phy: microchip_t1s: fix collision detection for LAN867X Rev.D0

 drivers/net/ethernet/Kconfig                  |   1 +
 .../net/ethernet/microchip/lan865x/lan865x.c  |   4 +-
 drivers/net/ethernet/oa_tc6.c                 | 227 +++++++++++
 drivers/net/phy/microchip_t1s.c               | 365 +++++++++++++++++-
 include/linux/oa_tc6.h                        |   3 +
 5 files changed, 587 insertions(+), 13 deletions(-)


base-commit: c66d93e68728cfb5f40b40d0f24129d7768faf43
-- 
2.43.0

[PATCH net-next v4 1/4] net: phy: microchip_t1s: fix collision detection on PLCA status change

[Parthiban Veerasooran]

Collision detection was only updated on explicit ethtool PLCA changes,
missing autonomous PLCA-status transitions driven by BEACON presence.
When PLCA went offline, CDEN stayed disabled, leaving CSMA/CD running
unprotected.

Add lan86xx_config_intr()/lan86xx_handle_interrupt() to track PSTC and
update CDEN in COL_DET_CTRL0 accordingly, resyncing before unmasking to
avoid racing a genuine interrupt. CDEN also depends on PRSCTL1's
autonomous CSMA/CD fallback bit: a missing BEACON only means CSMA/CD is
active if fallback is enabled. read_status() also resynchronizes CDEN,
providing a polling-based fallback when the PHY interrupt is
unavailable. A per-PHY lock serializes COL_DET_CTRL0 access, since
config_intr() isn't guaranteed to run under phydev->lock while
handle_interrupt() is.

Also fix plca_cfg->enabled being treated as boolean instead of tri-state
(-1 = "don't change"), which could disable CDEN on an unrelated ethtool
write.

Wired to LAN867X Rev.C1, C2 and LAN865X Rev.B0/B1. Rev.B1 is
unsupported/undocumented silicon and stays out of scope. Rev.D0 is
handled separately (follow-on patch).

Fixes: 78341049fbcd ("net: phy: microchip_t1s: configure collision detection based on PLCA mode")
Signed-off-by: Parthiban Veerasooran <parthiban.veerasooran@microchip.com>
---
 drivers/net/phy/microchip_t1s.c | 238 ++++++++++++++++++++++++++++++--
 1 file changed, 227 insertions(+), 11 deletions(-)

diff --git a/drivers/net/phy/microchip_t1s.c b/drivers/net/phy/microchip_t1s.c
index 73c23d311d72..5ce0304bf095 100644
--- a/drivers/net/phy/microchip_t1s.c
+++ b/drivers/net/phy/microchip_t1s.c
@@ -18,8 +18,8 @@
 /* Both Rev.B0 and B1 clause 22 PHYID's are same due to B1 chip limitation */
 #define PHY_ID_LAN865X_REVB 0x0007C1B3
 
+/* PHY interrupt status 2 register */
 #define LAN867X_REG_STS2 0x0019
-
 #define LAN867x_RESET_COMPLETE_STS BIT(11)
 
 #define LAN865X_REG_CFGPARAM_ADDR 0x00D8
@@ -27,6 +27,21 @@
 #define LAN865X_REG_CFGPARAM_CTRL 0x00DA
 #define LAN865X_REG_STS2 0x0019
 
+/* PHY interrupt status 1 register */
+#define LAN86XX_REG_STS1		0x0018
+#define LAN86XX_STS1_PLCA_STS_CHANGED	BIT(11)
+
+/* PHY interrupt mask 1 register */
+#define LAN86XX_REG_IMSK1		0x001C
+
+/* PLCA Reconciliation Sublayer Control 1 Register (PRSCTL1). Bit 10 controls
+ * whether the PHY autonomously falls back to CSMA/CD mode when no BEACON is
+ * observed while PLCA is enabled, versus staying pinned to PLCA mode regardless
+ * of BEACON presence.
+ */
+#define LAN86XX_REG_PRSCTL1               0x0035
+#define PRSCTL1_PLCA_FALLB_TO_CSMACD_EN   BIT(10)
+
 /* Collision Detector Control 0 Register */
 #define LAN86XX_REG_COL_DET_CTRL0	0x0087
 #define COL_DET_CTRL0_ENABLE_BIT_MASK	BIT(15)
@@ -136,6 +151,30 @@ static const u16 lan867x_revd0_fixup_values[8] = {
 	0x001C, 0x0C0B, 0x8C07, 0x9660,
 };
 
+struct lan86xx_priv {
+	/* Serializes CDEN state synchronization. */
+	struct mutex cden_lock;
+	int plca_enabled;
+};
+
+static int lan86xx_probe(struct phy_device *phydev)
+{
+	struct lan86xx_priv *priv;
+	int ret;
+
+	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	ret = devm_mutex_init(&phydev->mdio.dev, &priv->cden_lock);
+	if (ret)
+		return ret;
+
+	phydev->priv = priv;
+
+	return 0;
+}
+
 /* Pulled from AN1760 describing 'indirect read'
  *
  * write_register(0x4, 0x00D8, addr)
@@ -430,6 +469,57 @@ static int lan867x_revd0_link_active_selection(struct phy_device *phydev,
 			     LAN867X_REG_LINK_STATUS_CTRL, value);
 }
 
+static int lan86xx_fallback_to_csmacd(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, LAN86XX_REG_PRSCTL1);
+	if (ret < 0)
+		return ret;
+
+	return !!(ret & PRSCTL1_PLCA_FALLB_TO_CSMACD_EN);
+}
+
+/* Collision detection must stay disabled while the device is actually operating
+ * in PLCA mode, and enabled while it is actually operating in CSMA/CD.
+ * A missing BEACON (pst == 0) only means the device is running CSMA/CD if
+ * autonomous fallback is enabled (PRSCTL1 bit 10); if fallback is disabled,
+ * the device stays pinned to PLCA mode regardless of BEACON presence,
+ * so collision detection must remain disabled.
+ */
+static int lan86xx_update_cden(struct phy_device *phydev)
+{
+	struct lan86xx_priv *priv = phydev->priv;
+	struct phy_plca_status plca_st;
+	int fallback, ret;
+	u16 cden;
+
+	fallback = lan86xx_fallback_to_csmacd(phydev);
+	if (fallback < 0)
+		return fallback;
+
+	ret = genphy_c45_plca_get_status(phydev, &plca_st);
+	if (ret < 0)
+		return ret;
+
+	/* PLCA disabled                          -> CDEN enabled
+	 * PLCA enabled + BEACON                  -> CDEN disabled
+	 * PLCA enabled + no BEACON + fallback    -> CDEN enabled
+	 * PLCA enabled + no BEACON + no fallback -> CDEN disabled
+	 */
+	if (!priv->plca_enabled)
+		cden = COL_DET_ENABLE;
+	else if (plca_st.pst)
+		cden = COL_DET_DISABLE;
+	else if (fallback)
+		cden = COL_DET_ENABLE;
+	else
+		cden = COL_DET_DISABLE;
+
+	return phy_modify_mmd(phydev, MDIO_MMD_VEND2, LAN86XX_REG_COL_DET_CTRL0,
+			      COL_DET_CTRL0_ENABLE_BIT_MASK, cden);
+}
+
 /* As per LAN8650/1 Rev.B0/B1 AN1760 (Revision F (DS60001760G - June 2024)) and
  * LAN8670/1/2 Rev.C1/C2 AN1699 (Revision E (DS60001699F - June 2024)), under
  * normal operation, the device should be operated in PLCA mode. Disabling
@@ -444,10 +534,14 @@ static int lan867x_revd0_link_active_selection(struct phy_device *phydev,
 static int lan86xx_plca_set_cfg(struct phy_device *phydev,
 				const struct phy_plca_cfg *plca_cfg)
 {
+	struct lan86xx_priv *priv = phydev->priv;
 	int ret;
 
-	/* Link status selection must be configured for LAN8670/1/2 Rev.D0 */
-	if (phydev->phy_id == PHY_ID_LAN867X_REVD0) {
+	/* Link status selection must be configured for LAN8670/1/2 Rev.D0.
+	 * Only update link status selection if enabled is explicitly specified
+	 * (not -1, which means "don't change").
+	 */
+	if (phydev->phy_id == PHY_ID_LAN867X_REVD0 && plca_cfg->enabled != -1) {
 		ret = lan867x_revd0_link_active_selection(phydev,
 							  plca_cfg->enabled);
 		if (ret)
@@ -458,14 +552,18 @@ static int lan86xx_plca_set_cfg(struct phy_device *phydev,
 	if (ret)
 		return ret;
 
-	if (plca_cfg->enabled)
-		return phy_modify_mmd(phydev, MDIO_MMD_VEND2,
-				      LAN86XX_REG_COL_DET_CTRL0,
-				      COL_DET_CTRL0_ENABLE_BIT_MASK,
-				      COL_DET_DISABLE);
+	if (plca_cfg->enabled != -1)
+		priv->plca_enabled = plca_cfg->enabled;
 
-	return phy_modify_mmd(phydev, MDIO_MMD_VEND2, LAN86XX_REG_COL_DET_CTRL0,
-			      COL_DET_CTRL0_ENABLE_BIT_MASK, COL_DET_ENABLE);
+	if (plca_cfg->enabled != -1) {
+		mutex_lock(&priv->cden_lock);
+		ret = lan86xx_update_cden(phydev);
+		mutex_unlock(&priv->cden_lock);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
 }
 
 static int lan867x_revd0_config_init(struct phy_device *phydev)
@@ -493,6 +591,9 @@ static int lan867x_revd0_config_init(struct phy_device *phydev)
 
 static int lan86xx_read_status(struct phy_device *phydev)
 {
+	struct lan86xx_priv *priv = phydev->priv;
+	int ret;
+
 	/* The phy has some limitations, namely:
 	 *  - always reports link up
 	 *  - only supports 10MBit half duplex
@@ -503,7 +604,112 @@ static int lan86xx_read_status(struct phy_device *phydev)
 	phydev->speed = SPEED_10;
 	phydev->autoneg = AUTONEG_DISABLE;
 
-	return 0;
+	/* LAN867X Rev.B1 is unsupported/undocumented silicon (absent from the
+	 * current AN1699 and datasheet) and is kept out of scope for the CDEN
+	 * tracking below.
+	 */
+	if (phydev->phy_id == PHY_ID_LAN867X_REVB1)
+		return 0;
+
+	/* When no PHY interrupt is available, phylib polls read_status().
+	 * Use the PLCA status from that poll to resync CDEN.
+	 */
+	mutex_lock(&priv->cden_lock);
+	ret = lan86xx_update_cden(phydev);
+	mutex_unlock(&priv->cden_lock);
+	return ret;
+}
+
+/* Read LAN86XX_REG_STS1, which clears the latched status bits on read. */
+static int lan86xx_read_clear_sts1(struct phy_device *phydev)
+{
+	return phy_read_mmd(phydev, MDIO_MMD_VEND2, LAN86XX_REG_STS1);
+}
+
+/* Mask (mask bit = 1) or unmask (mask bit = 0) the given STS1 bits in
+ * IMSK1.
+ */
+static int lan86xx_set_intr_mask(struct phy_device *phydev, u16 mask,
+				 bool enable)
+{
+	if (enable)
+		/* A mask bit of 0 enables the corresponding interrupt. */
+		return phy_clear_bits_mmd(phydev, MDIO_MMD_VEND2,
+					  LAN86XX_REG_IMSK1, mask);
+
+	return phy_set_bits_mmd(phydev, MDIO_MMD_VEND2, LAN86XX_REG_IMSK1,
+				mask);
+}
+
+static int lan86xx_config_intr(struct phy_device *phydev)
+{
+	struct lan86xx_priv *priv = phydev->priv;
+	int ret;
+
+	if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+		/* Read to clear any pending status before enabling. */
+		ret = lan86xx_read_clear_sts1(phydev);
+		if (ret < 0)
+			return ret;
+
+		/* STS1 may have cleared a PSTC event that occurred while the
+		 * interrupt was masked, so synchronize CDEN with the current
+		 * PLCA state before enabling PSTC.
+		 */
+		mutex_lock(&priv->cden_lock);
+		ret = lan86xx_update_cden(phydev);
+		mutex_unlock(&priv->cden_lock);
+		if (ret)
+			return ret;
+
+		return lan86xx_set_intr_mask(phydev,
+					     LAN86XX_STS1_PLCA_STS_CHANGED,
+					     true);
+	}
+
+	ret = lan86xx_set_intr_mask(phydev, LAN86XX_STS1_PLCA_STS_CHANGED,
+				    false);
+	if (ret)
+		return ret;
+
+	/* Read to clear any pending status after disabling. */
+	ret = lan86xx_read_clear_sts1(phydev);
+	return ret < 0 ? ret : 0;
+}
+
+static irqreturn_t lan86xx_handle_interrupt(struct phy_device *phydev)
+{
+	struct lan86xx_priv *priv = phydev->priv;
+	irqreturn_t ret_irq = IRQ_NONE;
+	int sts1, ret;
+
+	/* Reading the status register clears the latched event bits. */
+	sts1 = lan86xx_read_clear_sts1(phydev);
+	if (sts1 < 0) {
+		phy_error(phydev);
+		return IRQ_NONE;
+	}
+
+	if (sts1 & LAN86XX_STS1_PLCA_STS_CHANGED) {
+		/* AN1760/AN1699: disable collision detection while actually
+		 * operating in PLCA mode; re-enable it only once actually
+		 * operating in CSMA/CD (see lan86xx_update_cden()).
+		 *
+		 * https://www.microchip.com/en-us/application-notes/an1760
+		 * https://www.microchip.com/en-us/application-notes/an1699
+		 */
+		mutex_lock(&priv->cden_lock);
+		ret = lan86xx_update_cden(phydev);
+		mutex_unlock(&priv->cden_lock);
+		if (ret < 0) {
+			phy_error(phydev);
+			return IRQ_NONE;
+		}
+
+		ret_irq = IRQ_HANDLED;
+	}
+
+	return ret_irq;
 }
 
 static struct phy_driver microchip_t1s_driver[] = {
@@ -521,8 +727,11 @@ static struct phy_driver microchip_t1s_driver[] = {
 		PHY_ID_MATCH_EXACT(PHY_ID_LAN867X_REVC1),
 		.name               = "LAN867X Rev.C1",
 		.features           = PHY_BASIC_T1S_P2MP_FEATURES,
+		.probe              = lan86xx_probe,
 		.config_init        = lan867x_revc_config_init,
 		.read_status        = lan86xx_read_status,
+		.config_intr        = lan86xx_config_intr,
+		.handle_interrupt   = lan86xx_handle_interrupt,
 		.get_plca_cfg	    = genphy_c45_plca_get_cfg,
 		.set_plca_cfg	    = lan86xx_plca_set_cfg,
 		.get_plca_status    = genphy_c45_plca_get_status,
@@ -531,8 +740,11 @@ static struct phy_driver microchip_t1s_driver[] = {
 		PHY_ID_MATCH_EXACT(PHY_ID_LAN867X_REVC2),
 		.name               = "LAN867X Rev.C2",
 		.features           = PHY_BASIC_T1S_P2MP_FEATURES,
+		.probe              = lan86xx_probe,
 		.config_init        = lan867x_revc_config_init,
 		.read_status        = lan86xx_read_status,
+		.config_intr        = lan86xx_config_intr,
+		.handle_interrupt   = lan86xx_handle_interrupt,
 		.get_plca_cfg	    = genphy_c45_plca_get_cfg,
 		.set_plca_cfg	    = lan86xx_plca_set_cfg,
 		.get_plca_status    = genphy_c45_plca_get_status,
@@ -541,6 +753,7 @@ static struct phy_driver microchip_t1s_driver[] = {
 		PHY_ID_MATCH_EXACT(PHY_ID_LAN867X_REVD0),
 		.name               = "LAN867X Rev.D0",
 		.features           = PHY_BASIC_T1S_P2MP_FEATURES,
+		.probe              = lan86xx_probe,
 		.config_init        = lan867x_revd0_config_init,
 		.get_plca_cfg	    = genphy_c45_plca_get_cfg,
 		.set_plca_cfg	    = lan86xx_plca_set_cfg,
@@ -554,8 +767,11 @@ static struct phy_driver microchip_t1s_driver[] = {
 		PHY_ID_MATCH_EXACT(PHY_ID_LAN865X_REVB),
 		.name               = "LAN865X Rev.B0/B1 Internal Phy",
 		.features           = PHY_BASIC_T1S_P2MP_FEATURES,
+		.probe              = lan86xx_probe,
 		.config_init        = lan865x_revb_config_init,
 		.read_status        = lan86xx_read_status,
+		.config_intr        = lan86xx_config_intr,
+		.handle_interrupt   = lan86xx_handle_interrupt,
 		.read_mmd           = genphy_read_mmd_c45,
 		.write_mmd          = genphy_write_mmd_c45,
 		.get_plca_cfg	    = genphy_c45_plca_get_cfg,
-- 
2.43.0

[PATCH net-next v4 2/4] net: ethernet: oa_tc6: deliver the PHY interrupt to phylib

[Parthiban Veerasooran]

The OA TC6 standard defines PHY interrupt delivery via the SPI status
register as optional. The PHY has no dedicated interrupt line in that
case; its interrupt is signalled through the MAC-PHY SPI interface, but
phy_mac_interrupt() can't make the PHY driver read and acknowledge its
own status registers. Expose it to phylib instead as a nested virtual
IRQ, gated by a new OA_TC6_PHY_INT quirk flag for drivers that route
PHY interrupts in-band.

When set, a nested virtual IRQ is created in oa_tc6_mdiobus_register()
before mdiobus_register(), and all mii_bus->irq[] entries are populated
with it so phy_device_create() picks it up regardless of MDIO address.
Teardown is integrated into oa_tc6_mdiobus_unregister().

A custom irq_chip (oa_tc6_phy_irq_chip) implements mask/unmask via
irq_bus_lock/irq_bus_sync_unlock, writing the mask bit to hardware over
SPI. The interrupt starts masked (hardware reset default) and is only
unmasked when phylib requests it, so disabling the nested IRQ actually
masks the hardware source too, preventing interrupt storms.

Dispatch is deferred to a workqueue rather than run synchronously from
the threaded IRQ: phy_interrupt() takes phydev->lock and PHY
handle_interrupt() issues synchronous SPI transfers, either of which
would otherwise stall the single thread pumping every TX/RX data chunk.
PHYINT is level triggered and stays asserted until acked, so a no-op
reschedule on an already-pending work item can't lose or duplicate an
event.

Select IRQ_DOMAIN in Kconfig for the irq_domain APIs used here.

Prerequisite for "net: phy: microchip_t1s: fix collision detection on
PLCA status change" (Fixes: 78341049fbcd) to fully cover the LAN865X
internal PHY.

Signed-off-by: Parthiban Veerasooran <parthiban.veerasooran@microchip.com>
---
 drivers/net/ethernet/Kconfig  |   1 +
 drivers/net/ethernet/oa_tc6.c | 227 ++++++++++++++++++++++++++++++++++
 include/linux/oa_tc6.h        |   3 +
 3 files changed, 231 insertions(+)

diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index c2b0161d0bec..9b102a91c36a 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -151,6 +151,7 @@ config OA_TC6
 	tristate "OPEN Alliance TC6 10BASE-T1x MAC-PHY support" if COMPILE_TEST
 	depends on SPI
 	select PHYLIB
+	select IRQ_DOMAIN
 	help
 	  This library implements OPEN Alliance TC6 10BASE-T1x MAC-PHY
 	  Serial Interface protocol for supporting 10BASE-T1x MAC-PHYs.
diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
index 364027c39fa4..74fe65b76359 100644
--- a/drivers/net/ethernet/oa_tc6.c
+++ b/drivers/net/ethernet/oa_tc6.c
@@ -10,6 +10,8 @@
 #include <linux/gpio/consumer.h>
 #include <linux/iopoll.h>
 #include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
 #include <linux/mdio.h>
 #include <linux/phy.h>
 #include <linux/oa_tc6.h>
@@ -72,6 +74,11 @@ struct oa_tc6 {
 	struct phy_device *phydev;
 	struct mii_bus *mdiobus;
 	struct spi_device *spi;
+	struct mutex phy_irq_lock; /* Serialises irq_bus_lock/sync_unlock */
+	bool phy_irq_masked; /* Shadow of OA_TC6_INT_MASK0_PHY_INT_MASK */
+	struct irq_domain *phy_irq_domain;
+	int phy_virq;
+	struct work_struct phy_irq_work;
 	struct mutex spi_ctrl_lock; /* Protects spi control transfer */
 	spinlock_t tx_skb_lock; /* Protects tx skb handling */
 	void *spi_ctrl_tx_buf;
@@ -531,6 +538,178 @@ int oa_tc6_mdiobus_write_c45(struct mii_bus *bus, int addr, int devnum,
 }
 EXPORT_SYMBOL_GPL(oa_tc6_mdiobus_write_c45);
 
+static int oa_tc6_phy_irq_unmask_hw(struct oa_tc6 *tc6)
+{
+	u32 regval;
+	int ret;
+
+	mutex_lock(&tc6->phy_irq_lock);
+
+	if (READ_ONCE(tc6->phy_irq_masked)) {
+		ret = 0;
+		goto unlock;
+	}
+
+	ret = oa_tc6_read_register(tc6, OA_TC6_REG_INT_MASK0, &regval);
+	if (ret)
+		goto unlock;
+
+	regval &= ~OA_TC6_INT_MASK0_PHY_INT_MASK;
+	ret = oa_tc6_write_register(tc6, OA_TC6_REG_INT_MASK0, regval);
+
+unlock:
+	mutex_unlock(&tc6->phy_irq_lock);
+
+	return ret;
+}
+
+static void oa_tc6_phy_irq_work(struct work_struct *work)
+{
+	struct oa_tc6 *tc6 = container_of(work, struct oa_tc6, phy_irq_work);
+	int ret;
+
+	/* Dispatched off the SPI chunk-processing thread so that
+	 * phy_interrupt() taking phydev->lock and issuing synchronous SPI
+	 * control transfers from PHY handle_interrupt() cannot stall the single
+	 * thread pumping TX/RX data chunks.
+	 */
+	handle_nested_irq(tc6->phy_virq);
+
+	ret = oa_tc6_phy_irq_unmask_hw(tc6);
+	if (ret)
+		dev_err(&tc6->spi->dev, "Failed to unmask PHY interrupt: %d\n",
+			ret);
+}
+
+static int oa_tc6_phy_irq_mask_hw(struct oa_tc6 *tc6)
+{
+	u32 regval;
+	int ret;
+
+	mutex_lock(&tc6->phy_irq_lock);
+
+	ret = oa_tc6_read_register(tc6, OA_TC6_REG_INT_MASK0, &regval);
+	if (ret)
+		goto unlock;
+
+	regval |= OA_TC6_INT_MASK0_PHY_INT_MASK;
+	ret = oa_tc6_write_register(tc6, OA_TC6_REG_INT_MASK0, regval);
+
+unlock:
+	mutex_unlock(&tc6->phy_irq_lock);
+
+	return ret;
+}
+
+static void oa_tc6_phy_irq_mask(struct irq_data *irqd)
+{
+	struct oa_tc6 *tc6 = irq_data_get_irq_chip_data(irqd);
+
+	WRITE_ONCE(tc6->phy_irq_masked, true);
+}
+
+static void oa_tc6_phy_irq_unmask(struct irq_data *irqd)
+{
+	struct oa_tc6 *tc6 = irq_data_get_irq_chip_data(irqd);
+
+	WRITE_ONCE(tc6->phy_irq_masked, false);
+}
+
+static void oa_tc6_phy_irq_disable(struct irq_data *irqd)
+{
+	struct oa_tc6 *tc6 = irq_data_get_irq_chip_data(irqd);
+
+	WRITE_ONCE(tc6->phy_irq_masked, true);
+}
+
+static void oa_tc6_phy_irq_bus_lock(struct irq_data *irqd)
+{
+	struct oa_tc6 *tc6 = irq_data_get_irq_chip_data(irqd);
+
+	mutex_lock(&tc6->phy_irq_lock);
+}
+
+static void oa_tc6_phy_irq_bus_sync_unlock(struct irq_data *irqd)
+{
+	struct oa_tc6 *tc6 = irq_data_get_irq_chip_data(irqd);
+	u32 regval;
+	int ret;
+
+	ret = oa_tc6_read_register(tc6, OA_TC6_REG_INT_MASK0, &regval);
+	if (ret) {
+		dev_err(&tc6->spi->dev, "Failed to read INT_MASK0: %d\n", ret);
+		goto unlock;
+	}
+
+	if (READ_ONCE(tc6->phy_irq_masked))
+		regval |= OA_TC6_INT_MASK0_PHY_INT_MASK;
+	else
+		regval &= ~OA_TC6_INT_MASK0_PHY_INT_MASK;
+
+	ret = oa_tc6_write_register(tc6, OA_TC6_REG_INT_MASK0, regval);
+	if (ret) {
+		dev_err(&tc6->spi->dev, "Failed to write INT_MASK0: %d\n", ret);
+		/* Note: on SPI failure, mask state is undefined until next
+		 * sync. This follows genirq's regmap_irq_sync_unlock() pattern
+		 * since the callback returns void and has nowhere to propagate
+		 * errors.
+		 */
+	}
+
+unlock:
+	mutex_unlock(&tc6->phy_irq_lock);
+}
+
+static struct irq_chip oa_tc6_phy_irq_chip = {
+	.name                   = "oa_tc6_phy",
+	.irq_mask               = oa_tc6_phy_irq_mask,
+	.irq_unmask             = oa_tc6_phy_irq_unmask,
+	.irq_disable            = oa_tc6_phy_irq_disable,
+	.irq_bus_lock           = oa_tc6_phy_irq_bus_lock,
+	.irq_bus_sync_unlock    = oa_tc6_phy_irq_bus_sync_unlock,
+};
+
+static int oa_tc6_phy_irq_map(struct irq_domain *domain, unsigned int irq,
+			      irq_hw_number_t hwirq)
+{
+	irq_set_chip_data(irq, domain->host_data);
+	irq_set_chip_and_handler(irq, &oa_tc6_phy_irq_chip, handle_simple_irq);
+	irq_set_nested_thread(irq, true);
+	irq_set_noprobe(irq);
+
+	return 0;
+}
+
+static const struct irq_domain_ops oa_tc6_phy_irq_domain_ops = {
+	.map = oa_tc6_phy_irq_map,
+};
+
+static int oa_tc6_phy_irq_setup(struct oa_tc6 *tc6)
+{
+	INIT_WORK(&tc6->phy_irq_work, oa_tc6_phy_irq_work);
+
+	tc6->phy_irq_domain =
+		irq_domain_create_linear(NULL, 1,
+					 &oa_tc6_phy_irq_domain_ops, tc6);
+	if (!tc6->phy_irq_domain)
+		return -ENOMEM;
+
+	tc6->phy_virq = irq_create_mapping(tc6->phy_irq_domain, 0);
+	WRITE_ONCE(tc6->phy_irq_masked, true);
+	if (!tc6->phy_virq) {
+		irq_domain_remove(tc6->phy_irq_domain);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static void oa_tc6_phy_irq_teardown(struct oa_tc6 *tc6)
+{
+	irq_dispose_mapping(tc6->phy_virq);
+	irq_domain_remove(tc6->phy_irq_domain);
+}
+
 static int oa_tc6_mdiobus_register(struct oa_tc6 *tc6)
 {
 	int ret;
@@ -562,9 +741,25 @@ static int oa_tc6_mdiobus_register(struct oa_tc6 *tc6)
 	snprintf(tc6->mdiobus->id, ARRAY_SIZE(tc6->mdiobus->id), "%s",
 		 dev_name(&tc6->spi->dev));
 
+	if (tc6->quirk_flags & OA_TC6_PHY_INT) {
+		ret = oa_tc6_phy_irq_setup(tc6);
+		if (ret) {
+			mdiobus_free(tc6->mdiobus);
+			return ret;
+		}
+		/* Populate all irq[] entries before registration so
+		 * phy_device_create() picks up the virtual IRQ regardless of
+		 * the PHY's MDIO address.
+		 */
+		for (int i = 0; i < PHY_MAX_ADDR; i++)
+			tc6->mdiobus->irq[i] = tc6->phy_virq;
+	}
+
 	ret = mdiobus_register(tc6->mdiobus);
 	if (ret) {
 		netdev_err(tc6->netdev, "Could not register MDIO bus\n");
+		if (tc6->quirk_flags & OA_TC6_PHY_INT)
+			oa_tc6_phy_irq_teardown(tc6);
 		mdiobus_free(tc6->mdiobus);
 		return ret;
 	}
@@ -575,6 +770,8 @@ static int oa_tc6_mdiobus_register(struct oa_tc6 *tc6)
 static void oa_tc6_mdiobus_unregister(struct oa_tc6 *tc6)
 {
 	mdiobus_unregister(tc6->mdiobus);
+	if (tc6->quirk_flags & OA_TC6_PHY_INT)
+		oa_tc6_phy_irq_teardown(tc6);
 	mdiobus_free(tc6->mdiobus);
 }
 
@@ -624,6 +821,7 @@ static void oa_tc6_phy_exit(struct oa_tc6 *tc6)
 	if (tc6->quirk_flags & OA_TC6_BROKEN_PHY)
 		return;
 
+	cancel_work_sync(&tc6->phy_irq_work);
 	phy_disconnect(tc6->phydev);
 	oa_tc6_mdiobus_unregister(tc6);
 }
@@ -780,7 +978,12 @@ static void oa_tc6_disable_traffic(struct oa_tc6 *tc6)
 	netif_tx_disable(tc6->netdev);
 	oa_tc6_drop_tx_skb(tc6, skb);
 	oa_tc6_free_ongoing_skbs(tc6);
+	/* Serialize INT_MASK0 write with phylib's mask/unmask to prevent
+	 * read-modify-write races in oa_tc6_phy_irq_bus_sync_unlock().
+	 */
+	mutex_lock(&tc6->phy_irq_lock);
 	oa_tc6_write_register(tc6, OA_TC6_REG_INT_MASK0, regval);
+	mutex_unlock(&tc6->phy_irq_lock);
 	oa_tc6_read_register(tc6, OA_TC6_REG_STATUS0, &regval);
 	oa_tc6_write_register(tc6, OA_TC6_REG_STATUS0, regval);
 	dev_err(&tc6->spi->dev, "Device interrupt disabled to avoid interrupt storm");
@@ -813,6 +1016,29 @@ static int oa_tc6_process_extended_status(struct oa_tc6 *tc6)
 		return ret;
 	}
 
+	/* Dispatch the PHY interrupt to phylib via the nested virtual IRQ so
+	 * the PHY driver reads and acknowledges its status. This is deferred
+	 * to a workqueue rather than dispatched synchronously here, since
+	 * phy_interrupt() takes phydev->lock and PHY handle_interrupt() issues
+	 * synchronous SPI control transfers, which would otherwise block this
+	 * thread.
+	 *
+	 * Mask the hardware interrupt immediately to avoid wasting SPI cycles
+	 * on redundant STATUS0 reads until the worker runs and phylib acks it.
+	 * PHYINT is level triggered and stays asserted until acked, so every
+	 * RX chunk footer would re-read STATUS0 until the worker schedules.
+	 * Gate on phy_virq (the actual resource) rather than just the flag to
+	 * be self-consistent if OA_TC6_BROKEN_PHY skips initialization.
+	 */
+	if (tc6->phy_virq && FIELD_GET(OA_TC6_STATUS0_PHY_INT, value)) {
+		ret = oa_tc6_phy_irq_mask_hw(tc6);
+		if (ret)
+			dev_err(&tc6->spi->dev,
+				"Failed to mask PHY interrupt: %d\n", ret);
+		else
+			schedule_work(&tc6->phy_irq_work);
+	}
+
 	if (FIELD_GET(OA_TC6_STATUS0_RX_BUFFER_OVERFLOW_ERROR, value)) {
 		oa_tc6_look_for_new_frame(tc6);
 		net_err_ratelimited("%s: Receive buffer overflow error\n",
@@ -1471,6 +1697,7 @@ struct oa_tc6 *oa_tc6_init(struct spi_device *spi, struct net_device *netdev,
 	tc6->spi = spi;
 	tc6->netdev = netdev;
 	SET_NETDEV_DEV(netdev, &spi->dev);
+	mutex_init(&tc6->phy_irq_lock);
 	mutex_init(&tc6->spi_ctrl_lock);
 	spin_lock_init(&tc6->tx_skb_lock);
 
diff --git a/include/linux/oa_tc6.h b/include/linux/oa_tc6.h
index 27f652d4920b..1c8cc263c5e5 100644
--- a/include/linux/oa_tc6.h
+++ b/include/linux/oa_tc6.h
@@ -30,6 +30,7 @@
 
 /* Status Register #0 */
 #define OA_TC6_REG_STATUS0			0x0008
+#define OA_TC6_STATUS0_PHY_INT			BIT(7)
 #define OA_TC6_STATUS0_RESETC			BIT(6)	/* Reset Complete */
 #define OA_TC6_STATUS0_HEADER_ERROR		BIT(5)
 #define OA_TC6_STATUS0_LOSS_OF_FRAME_ERROR	BIT(4)
@@ -43,6 +44,7 @@
 
 /* Interrupt Mask Register #0 */
 #define OA_TC6_REG_INT_MASK0				0x000C
+#define OA_TC6_INT_MASK0_PHY_INT_MASK			BIT(7)
 #define OA_TC6_INT_MASK0_HEADER_ERR_MASK		BIT(5)
 #define OA_TC6_INT_MASK0_LOSS_OF_FRAME_ERR_MASK		BIT(4)
 #define OA_TC6_INT_MASK0_RX_BUFFER_OVERFLOW_ERR_MASK	BIT(3)
@@ -68,6 +70,7 @@ struct oa_tc6;
 
 enum oa_tc6_quirk_flag {
 	OA_TC6_BROKEN_PHY = BIT(0),
+	OA_TC6_PHY_INT    = BIT(1),
 };
 
 struct oa_tc6_quirks {
-- 
2.43.0

[PATCH net-next v4 3/4] microchip: lan865x: enable PHY interrupt via virtual IRQ

[Parthiban Veerasooran]

The LAN865X integrates the PHY directly into the MAC-PHY SPI device.
The PHY has no dedicated interrupt line; its interrupt is always
delivered as the PHYINT bit in STATUS0.

Pass the OA_TC6_PHY_INT quirk flag to oa_tc6_init() to enable the
virtual IRQ infrastructure. This lets phylib operate in interrupt mode
and drives the PHY driver's config_intr/handle_interrupt callbacks for
the LAN865X internal PHY.

This completes the collision-detection fix started in "net: phy:
microchip_t1s: fix collision detection on PLCA status change"; without
this patch (and its prerequisite, "net: ethernet: oa_tc6: deliver the
PHY interrupt to phylib"), LAN865X stays on that commit's polling-based
CDEN synchronization/fallback instead of tracking PLCA transitions
dynamically.

CDEN is synchronized to the live PLCA status when lan86xx_config_intr()
runs during phy_enable_interrupts(), then tracked on each subsequent
PLCA transition via lan86xx_handle_interrupt(). This PHY interrupt path
is critical for keeping CDEN correct across PLCA transitions.

Fixes: 78341049fbcd ("net: phy: microchip_t1s: configure collision detection based on PLCA mode")
Signed-off-by: Parthiban Veerasooran <parthiban.veerasooran@microchip.com>
---
 drivers/net/ethernet/microchip/lan865x/lan865x.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/microchip/lan865x/lan865x.c b/drivers/net/ethernet/microchip/lan865x/lan865x.c
index 127afb9e9f14..d47560f9847d 100644
--- a/drivers/net/ethernet/microchip/lan865x/lan865x.c
+++ b/drivers/net/ethernet/microchip/lan865x/lan865x.c
@@ -332,6 +332,7 @@ static const struct net_device_ops lan865x_netdev_ops = {
 
 static int lan865x_probe(struct spi_device *spi)
 {
+	struct oa_tc6_quirks tc6_quirks = {};
 	struct net_device *netdev;
 	struct lan865x_priv *priv;
 	int ret;
@@ -346,7 +347,8 @@ static int lan865x_probe(struct spi_device *spi)
 	spi_set_drvdata(spi, priv);
 	INIT_WORK(&priv->multicast_work, lan865x_multicast_work_handler);
 
-	priv->tc6 = oa_tc6_init(spi, netdev, NULL);
+	tc6_quirks.quirk_flags = OA_TC6_PHY_INT;
+	priv->tc6 = oa_tc6_init(spi, netdev, &tc6_quirks);
 	if (IS_ERR(priv->tc6)) {
 		ret = PTR_ERR(priv->tc6);
 		goto free_netdev;
-- 
2.43.0

[PATCH net-next v4 4/4] net: phy: microchip_t1s: fix collision detection for LAN867X Rev.D0

[Parthiban Veerasooran]

LAN867X Rev.D0 adds a Collision Counting and MAC Forwarding Control
field (CCMFC, bits 10:9) in COL_DET_CTRL0 (0x0087). Set to the OA
default (0x1), the hardware autonomously gates collision forwarding
based on live PLCA_Status, removing the delay a software-driven CDEN
toggle had on older revisions. The PSTC interrupt handler for Rev.D0
therefore only needs to update the link status selection on each PLCA
transition.

Configure CCMFC to the OA default in lan867x_revd0_config_init(). Add
lan867x_revd0_handle_interrupt() for two events: Link Status Change
triggers the phylib state machine; PLCA Status Change re-evaluates the
link status selection using the cached PLCA enable state and current
CSMA/CD fallback configuration.

Rev.D0 can also be configured (PRSCTL1, 0x0035, bit 10) to autonomously
fall back to CSMA/CD when no BEACON is seen. When that's active, the
PHY's own hardware transition already handles CSMA/CD correctly, so
driving link status from PLCA_Status reports nothing meaningful. Force
the semaphore (forced-active) source whenever fallback is enabled; only
when fallback is disabled - the PHY stays pinned to PLCA mode - does
tracking PLCA_Status serve its purpose.

Wire up .config_intr/.handle_interrupt for Rev.D0 via a dedicated
lan867x_revd0_config_intr(), reusing the STS1/IMSK1 helpers from the
other LAN86XX PHYs.

Preserve cable-test polling when Rev.D0 uses its PHY interrupt path.
Once interrupt handling is enabled, phydev->irq may be valid and the
normal PHY polling path is no longer used, so PHY_POLL_CABLE_TEST
ensures cable-test state continues to be polled while the test is active.

Fixes: 07f5765f26c3 ("net: phy: microchip_t1s: configure link status control for LAN867x Rev.D0")
Signed-off-by: Parthiban Veerasooran <parthiban.veerasooran@microchip.com>
---
 drivers/net/phy/microchip_t1s.c | 127 +++++++++++++++++++++++++++++++-
 1 file changed, 126 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/microchip_t1s.c b/drivers/net/phy/microchip_t1s.c
index 5ce0304bf095..d667f57aa4a8 100644
--- a/drivers/net/phy/microchip_t1s.c
+++ b/drivers/net/phy/microchip_t1s.c
@@ -29,6 +29,7 @@
 
 /* PHY interrupt status 1 register */
 #define LAN86XX_REG_STS1		0x0018
+#define LAN86XX_STS1_LINK_STS_CHANGED	BIT(13)
 #define LAN86XX_STS1_PLCA_STS_CHANGED	BIT(11)
 
 /* PHY interrupt mask 1 register */
@@ -47,6 +48,9 @@
 #define COL_DET_CTRL0_ENABLE_BIT_MASK	BIT(15)
 #define COL_DET_ENABLE			BIT(15)
 #define COL_DET_DISABLE			0x0000
+#define COL_DET_CTRL0_CCMFC_MASK	GENMASK(10, 9)
+/* OA default: collisions gated by PLCA_Status in hardware */
+#define COL_DET_CTRL0_CCMFC_OA_DEFAULT	BIT(9)
 
 /* LAN8670/1/2 Rev.D0 Link Status Selection Register */
 #define LAN867X_REG_LINK_STATUS_CTRL	0x0012
@@ -520,6 +524,29 @@ static int lan86xx_update_cden(struct phy_device *phydev)
 			      COL_DET_CTRL0_ENABLE_BIT_MASK, cden);
 }
 
+/* When the PHY autonomously falls back to CSMA/CD once BEACONs stop (PRSCTL1
+ * bit 10 set), the hardware fallback already provides correct CSMA/CD
+ * operation; selecting link status from PLCA_STATUS in that case reports
+ * nothing meaningful, since the PHY may already be running CSMA/CD regardless
+ * of the stale PLCA_STATUS value. Force the semaphore (forced-active) source
+ * in that case instead. Only when fallback is disabled - the PHY is pinned
+ * to PLCA mode - does tracking PLCA_STATUS serve its intended purpose.
+ */
+static int lan867x_revd0_update_link_selection(struct phy_device *phydev,
+					       int plca_enabled)
+{
+	int fallback;
+
+	fallback = lan86xx_fallback_to_csmacd(phydev);
+	if (fallback < 0)
+		return fallback;
+
+	if (fallback)
+		return lan867x_revd0_link_active_selection(phydev, false);
+
+	return lan867x_revd0_link_active_selection(phydev, plca_enabled);
+}
+
 /* As per LAN8650/1 Rev.B0/B1 AN1760 (Revision F (DS60001760G - June 2024)) and
  * LAN8670/1/2 Rev.C1/C2 AN1699 (Revision E (DS60001699F - June 2024)), under
  * normal operation, the device should be operated in PLCA mode. Disabling
@@ -528,6 +555,11 @@ static int lan86xx_update_cden(struct phy_device *phydev)
  * distortion cause poor signal quality. Collision detection must be re-enabled
  * if the device is configured to operate in CSMA/CD mode.
  *
+ * LAN867X Rev.D0 has autonomous collision detection gating via CCMFC and
+ * does not toggle CDEN in the interrupt handler. CDEN remains permanently
+ * enabled in config_init(), so no software-driven CDEN toggling is needed
+ * here.
+ *
  * AN1760: https://www.microchip.com/en-us/application-notes/an1760
  * AN1699: https://www.microchip.com/en-us/application-notes/an1699
  */
@@ -542,7 +574,7 @@ static int lan86xx_plca_set_cfg(struct phy_device *phydev,
 	 * (not -1, which means "don't change").
 	 */
 	if (phydev->phy_id == PHY_ID_LAN867X_REVD0 && plca_cfg->enabled != -1) {
-		ret = lan867x_revd0_link_active_selection(phydev,
+		ret = lan867x_revd0_update_link_selection(phydev,
 							  plca_cfg->enabled);
 		if (ret)
 			return ret;
@@ -555,6 +587,12 @@ static int lan86xx_plca_set_cfg(struct phy_device *phydev,
 	if (plca_cfg->enabled != -1)
 		priv->plca_enabled = plca_cfg->enabled;
 
+	/* LAN867X Rev.D0 uses CCMFC for autonomous collision detection
+	 * gating; CDEN remains enabled and does not require software toggling.
+	 */
+	if (phydev->phy_id == PHY_ID_LAN867X_REVD0)
+		return 0;
+
 	if (plca_cfg->enabled != -1) {
 		mutex_lock(&priv->cden_lock);
 		ret = lan86xx_update_cden(phydev);
@@ -582,6 +620,20 @@ static int lan867x_revd0_config_init(struct phy_device *phydev)
 			return ret;
 	}
 
+	/* AN1699: Configure CCMFC (Collision Counting and MAC Forwarding
+	 * Control) to OA default (0x1) so that the hardware autonomously gates
+	 * collision forwarding to the MAC based on the live PLCA_Status:
+	 * collisions are neither counted nor forwarded when PLCA_Status is OK,
+	 * and are counted/forwarded when not OK. This eliminates the need for
+	 * software-driven CDEN toggling. CDEN defaults to enabled on Rev.D0
+	 * and remains enabled.
+	 */
+	ret = phy_modify_mmd(phydev, MDIO_MMD_VEND2, LAN86XX_REG_COL_DET_CTRL0,
+			     COL_DET_CTRL0_CCMFC_MASK,
+			     COL_DET_CTRL0_CCMFC_OA_DEFAULT);
+	if (ret)
+		return ret;
+
 	/* Initially the PHY will be in CSMA/CD mode by default. So it is
 	 * required to set the link always active as it doesn't support
 	 * autoneg.
@@ -712,6 +764,76 @@ static irqreturn_t lan86xx_handle_interrupt(struct phy_device *phydev)
 	return ret_irq;
 }
 
+static int lan867x_revd0_config_intr(struct phy_device *phydev)
+{
+	u16 mask = LAN86XX_STS1_PLCA_STS_CHANGED |
+		   LAN86XX_STS1_LINK_STS_CHANGED;
+	struct lan86xx_priv *priv = phydev->priv;
+	int sts1, ret;
+
+	if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+		/* Read to clear any pending status before enabling. */
+		sts1 = lan86xx_read_clear_sts1(phydev);
+		if (sts1 < 0)
+			return sts1;
+
+		/* STS1 may have cleared a pending PSTC while masked, and a
+		 * missed PSTC leaves no trace to key off, so unconditionally
+		 * resync the link-status-selection source from the current
+		 * PLCA enable state and fallback configuration.
+		 */
+		ret = lan867x_revd0_update_link_selection(phydev,
+							  priv->plca_enabled);
+		if (ret < 0)
+			return ret;
+
+		return lan86xx_set_intr_mask(phydev, mask, true);
+	}
+
+	ret = lan86xx_set_intr_mask(phydev, mask, false);
+	if (ret)
+		return ret;
+
+	/* Read to clear any pending status after disabling. */
+	ret = lan86xx_read_clear_sts1(phydev);
+	return ret < 0 ? ret : 0;
+}
+
+static irqreturn_t lan867x_revd0_handle_interrupt(struct phy_device *phydev)
+{
+	struct lan86xx_priv *priv = phydev->priv;
+	irqreturn_t ret_irq = IRQ_NONE;
+	int sts1, ret;
+
+	sts1 = lan86xx_read_clear_sts1(phydev);
+	if (sts1 < 0) {
+		phy_error(phydev);
+		return IRQ_NONE;
+	}
+
+	if (sts1 & LAN86XX_STS1_LINK_STS_CHANGED) {
+		phy_trigger_machine(phydev);
+		ret_irq = IRQ_HANDLED;
+	}
+
+	if (sts1 & LAN86XX_STS1_PLCA_STS_CHANGED) {
+		/* Re-evaluate the link-status selection when PLCA status
+		 * changes. A resulting link-status change raises
+		 * LINK_STS_CHANGED, which triggers the PHY state machine.
+		 */
+		ret = lan867x_revd0_update_link_selection(phydev,
+							  priv->plca_enabled);
+		if (ret < 0) {
+			phy_error(phydev);
+			return IRQ_NONE;
+		}
+
+		ret_irq = IRQ_HANDLED;
+	}
+
+	return ret_irq;
+}
+
 static struct phy_driver microchip_t1s_driver[] = {
 	{
 		PHY_ID_MATCH_EXACT(PHY_ID_LAN867X_REVB1),
@@ -754,7 +876,10 @@ static struct phy_driver microchip_t1s_driver[] = {
 		.name               = "LAN867X Rev.D0",
 		.features           = PHY_BASIC_T1S_P2MP_FEATURES,
 		.probe              = lan86xx_probe,
+		.flags              = PHY_POLL_CABLE_TEST,
 		.config_init        = lan867x_revd0_config_init,
+		.config_intr        = lan867x_revd0_config_intr,
+		.handle_interrupt   = lan867x_revd0_handle_interrupt,
 		.get_plca_cfg	    = genphy_c45_plca_get_cfg,
 		.set_plca_cfg	    = lan86xx_plca_set_cfg,
 		.get_plca_status    = genphy_c45_plca_get_status,
-- 
2.43.0

Re: [PATCH net-next v4 0/4] net: microchip_t1s: fix collision detection on PLCA status change

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net-next v4 0/4] net: microchip_t1s: fix collision detection on PLCA status change

[Parthiban Veerasooran]

Hi,

Thanks for the feedback.

The issue was identified during production level testing of the
LAN865X/LAN867X-based system. The issue was observed when the device
transitioned between PLCA mode and CSMA/CD mode based on BEACON
availability. The existing collision-detection handling did not
correctly track these autonomous PLCA state transitions.

The changes were tested on a Raspberry Pi 4 with the LAN865X/LAN867X
devices.

Best regards,
Parthiban V

On 29/09/26 6:35 pm, netdev-bot+sinfo@kernel.org wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi!
> 
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
> 
>   - How the issue was discovered, e.g. hit in production, hit during
>     development, syzbot report, manual code inspection, LLM or static
>     analysis tool scan.
> 
>   - Whether the issue was actually triggered, or is only theoretical
>     (e.g. found by code inspection). If it was triggered please include
>     the symptoms, like the stack trace or error messages.
> 
>   - What hardware the change was tested on. For driver fixes please
>     mention the device (and if relevant firmware version) used for
>     testing, or say that the change was not tested on real hardware.
> 
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
> 
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

Re: [PATCH net-next v4 1/4] net: phy: microchip_t1s: fix collision detection on PLCA status change

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The collision-detect (CDEN) setting is chosen from a software
  copy of the PLCA enable state (priv->plca_enabled), not from the PHY's…
- [Low] [Baseline decision: false.
- [Low] [Baseline decision: false.

--

net: phy: microchip_t1s: fix collision detection on PLCA status change

This patch makes the collision detection enable bit (CDEN) follow
autonomous PLCA/CSMA-CD transitions. It adds a PLCA status change
interrupt and a resync in read_status(), and it takes the PRSCTL1
fallback bit into account. It also adds a probe routine that allocates
a per-PHY mutex and a cached plca_enabled value, and it treats
plca_cfg->enabled as tri-state.

> diff --git a/drivers/net/phy/microchip_t1s.c b/drivers/net/phy/microchip_t1s.c
> index 73c23d311d72a..5ce0304bf0952 100644
> --- a/drivers/net/phy/microchip_t1s.c
> +++ b/drivers/net/phy/microchip_t1s.c

[ ... ]

> @@ -430,6 +469,57 @@ static int lan867x_revd0_link_active_selection(struct phy_device *phydev,

[ ... ]

> +static int lan86xx_update_cden(struct phy_device *phydev)
> +{
> +	struct lan86xx_priv *priv = phydev->priv;
> +	struct phy_plca_status plca_st;
> +	int fallback, ret;
> +	u16 cden;
> +
> +	fallback = lan86xx_fallback_to_csmacd(phydev);
> +	if (fallback < 0)
> +		return fallback;
> +
> +	ret = genphy_c45_plca_get_status(phydev, &plca_st);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* PLCA disabled                          -> CDEN enabled
> +	 * PLCA enabled + BEACON                  -> CDEN disabled
> +	 * PLCA enabled + no BEACON + fallback    -> CDEN enabled
> +	 * PLCA enabled + no BEACON + no fallback -> CDEN disabled
> +	 */
> +	if (!priv->plca_enabled)
> +		cden = COL_DET_ENABLE;
> +	else if (plca_st.pst)
> +		cden = COL_DET_DISABLE;
> +	else if (fallback)
> +		cden = COL_DET_ENABLE;
> +	else
> +		cden = COL_DET_DISABLE;

[Severity: Medium]
Can priv->plca_enabled get out of sync with the PLCA_CTRL0 enable bit in
the hardware? This decision uses the cached copy. read_status(),
config_intr() and the PSTC handler all rewrite CDEN from it, so a
mismatch gets written back on every poll.

The copy is only updated in lan86xx_plca_set_cfg(), and only after
genphy_c45_plca_set_cfg() fully succeeds. When enabled == 0, the generic
helper clears PLCA_EN first and then does more MDIO accesses that can
fail:

drivers/net/phy/phy-c45.c:genphy_c45_plca_set_cfg() {
    ...
	if (plca_cfg->enabled == 0) {
		ret = phy_clear_bits_mmd(phydev, MDIO_MMD_VEND2,
					 MDIO_OATC14_PLCA_CTRL0,
					 MDIO_OATC14_PLCA_EN);
    ...
		ret = phy_write_mmd(phydev, MDIO_MMD_VEND2,
				    MDIO_OATC14_PLCA_CTRL1, val);

		if (ret < 0)
			return ret;
    ...
}

If one of the later writes fails, PLCA is off in the hardware but
plca_enabled is still 1.

A PHY reset while the driver stays bound has the same effect. For MACs
that connect the PHY in ndo_open and have a reset GPIO or reset
controller, ifdown goes through:

drivers/net/phy/phy_device.c:phy_detach() {
    ...
	/* Assert the reset signal */
	phy_device_reset(phydev, 1);
    ...
}

A resume after the PHY lost power has the same effect. PLCA_CTRL0 goes
back to its default, but priv is devm-allocated at probe, so it keeps
plca_enabled = 1. lan867x_revc_config_init() does not touch PLCA_CTRL0.

In both cases this function sees plca_enabled = 1 and pst = 0. If
PRSCTL1 bit 10 (fallback) is clear, it writes COL_DET_DISABLE while the
PHY is actually running CSMA/CD. That is the situation the commit
message says it fixes.

The opposite can happen at probe. lan86xx_probe() zero-allocates priv
without reading PLCA_CTRL0. If PLCA is already enabled in the hardware
(for example, a module reload with no reset line), read_status() forces
CDEN on in PLCA mode, even though genphy_c45_plca_get_cfg() reports PLCA
as enabled.

The same logic is still there at the end of the series. Would it be
more robust for lan86xx_update_cden() to read the PLCA_CTRL0 enable bit
directly, instead of using the cached value?

> +
> +	return phy_modify_mmd(phydev, MDIO_MMD_VEND2, LAN86XX_REG_COL_DET_CTRL0,
> +			      COL_DET_CTRL0_ENABLE_BIT_MASK, cden);
> +}

[ ... ]

> @@ -458,14 +552,18 @@ static int lan86xx_plca_set_cfg(struct phy_device *phydev,
>  	if (ret)
>  		return ret;
>  
> -	if (plca_cfg->enabled)
> -		return phy_modify_mmd(phydev, MDIO_MMD_VEND2,
> -				      LAN86XX_REG_COL_DET_CTRL0,
> -				      COL_DET_CTRL0_ENABLE_BIT_MASK,
> -				      COL_DET_DISABLE);
> +	if (plca_cfg->enabled != -1)
> +		priv->plca_enabled = plca_cfg->enabled;

[Severity: Low]
This isn't a bug, but priv->plca_enabled is written here under
phydev->lock only. lan86xx_config_intr() reads it through
lan86xx_update_cden() under cden_lock only.

In practice, the ethtool path (rtnl or the netdev instance lock) and
the config_intr() callers that reach lan86xx_update_cden() are
serialized. Also, the locked lan86xx_update_cden() call below writes
CDEN last using the new value.

Would it be cleaner to do this store inside the cden_lock section, so
that the lock covers both the cached state and the register update?

>  
> -	return phy_modify_mmd(phydev, MDIO_MMD_VEND2, LAN86XX_REG_COL_DET_CTRL0,
> -			      COL_DET_CTRL0_ENABLE_BIT_MASK, COL_DET_ENABLE);
> +	if (plca_cfg->enabled != -1) {
> +		mutex_lock(&priv->cden_lock);
> +		ret = lan86xx_update_cden(phydev);
> +		mutex_unlock(&priv->cden_lock);
> +		if (ret)
> +			return ret;
> +	}

[Severity: Low]
The commit message says:

    Rev.D0 is handled separately (follow-on patch).

At this commit, though, LAN867X Rev.D0 also uses lan86xx_plca_set_cfg(),
and this block calls lan86xx_update_cden() for it too. The Rev.D0 entry
in microchip_t1s_driver[] also gets .probe = lan86xx_probe in this
patch.

Rev.D0 has no read_status, config_intr or handle_interrupt at this
point. Enabling PLCA while pst reads 0 and the PRSCTL1 fallback bit is
set would therefore leave CDEN enabled after PLCA comes up. Is that
expected for this intermediate commit?

The follow-on patch, "net: phy: microchip_t1s: fix collision detection
for LAN867X Rev.D0", adds an early return for PHY_ID_LAN867X_REVD0
before the CDEN update, so this only affects bisection.

Rev.D0 does need .probe here, because the shared setter now dereferences
phydev->priv. Could the commit message mention this Rev.D0 change?

> +
> +	return 0;
>  }

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929125928.611784-1-parthiban.veerasooran%40microchip.com

Re: [PATCH net-next v4 2/4] net: ethernet: oa_tc6: deliver the PHY interrupt to phylib

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 6 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 3 · Low: 3

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] oa_tc6_phy_exit() now calls
  cancel_work_sync(&tc6->phy_irq_work) for every device that does not…
- [Medium] In oa_tc6_process_extended_status(), the hot path sets
  INT_MASK0 bit 7 in hardware (oa_tc6_phy_irq_mask_hw()) and leaves…
- [Medium] oa_tc6_phy_irq_setup() sets tc6->phy_irq_masked = true only
  after irq_create_mapping() returns.
- [Low] No MAC driver sets the new OA_TC6_PHY_INT quirk in this commit.
- [Low] oa_tc6_disable_traffic() writes INT_MASK0 =
  OA_TC6_INT_MASK0_ALL_INTERRUPTS as a "point of no return" and now…
- [Low] In oa_tc6_process_extended_status(), oa_tc6_phy_irq_mask_hw() is
  called every time STATUS0 shows PHY_INT, even when INT_MASK0 bit 7 has…

--

net: ethernet: oa_tc6: deliver the PHY interrupt to phylib

This commit adds an OA_TC6_PHY_INT quirk. When the quirk is set, the
in-band PHY interrupt signalled through STATUS0 is passed to phylib as a
nested virtual IRQ. The PHYINT mask bit in INT_MASK0 is driven over SPI by
a custom irq_chip using bus_lock/bus_sync_unlock, and the dispatch from
oa_tc6_process_extended_status() is deferred to a workqueue.

> diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
> index 364027c39fa4a..74fe65b76359e 100644
> --- a/drivers/net/ethernet/oa_tc6.c
> +++ b/drivers/net/ethernet/oa_tc6.c

[ ... ]

> +static void oa_tc6_phy_irq_work(struct work_struct *work)
> +{
> +	struct oa_tc6 *tc6 = container_of(work, struct oa_tc6, phy_irq_work);
> +	int ret;
> +
> +	/* Dispatched off the SPI chunk-processing thread so that
> +	 * phy_interrupt() taking phydev->lock and issuing synchronous SPI
> +	 * control transfers from PHY handle_interrupt() cannot stall the single
> +	 * thread pumping TX/RX data chunks.
> +	 */
> +	handle_nested_irq(tc6->phy_virq);
> +
> +	ret = oa_tc6_phy_irq_unmask_hw(tc6);
> +	if (ret)
> +		dev_err(&tc6->spi->dev, "Failed to unmask PHY interrupt: %d\n",
> +			ret);
> +}

[Severity: Medium]
What re-arms PHYINT if oa_tc6_phy_irq_unmask_hw() fails here?

The hot path in oa_tc6_process_extended_status() sets INT_MASK0 bit 7 in
hardware through oa_tc6_phy_irq_mask_hw() and leaves phy_irq_masked false.
This worker is the only place that clears the bit again.

If the INT_MASK0 read or write in oa_tc6_phy_irq_unmask_hw() fails (a
spi_sync error, or -EPROTO from oa_tc6_check_ctrl_read_reply() or
oa_tc6_check_ctrl_write_reply()), we only get the dev_err(). The MAC-PHY
then keeps PHYINT masked, while genirq and phylib still consider the IRQ
enabled.

Nothing retries. genirq has no pending mask change to sync, and phylib
does not poll a PHY that is in interrupt mode. Later link and PLCA status
changes would then be lost until some unrelated extended-status event, or
a disable_irq()/enable_irq() cycle, re-syncs the bit.

The mask side can end up in the same state. If the device applies the
INT_MASK0 write but the echoed reply fails validation,
oa_tc6_process_extended_status() logs an error and skips schedule_work().
Hardware stays masked and no worker is queued.

[ ... ]

> +static int oa_tc6_phy_irq_setup(struct oa_tc6 *tc6)
> +{
> +	INIT_WORK(&tc6->phy_irq_work, oa_tc6_phy_irq_work);
> +
> +	tc6->phy_irq_domain =
> +		irq_domain_create_linear(NULL, 1,
> +					 &oa_tc6_phy_irq_domain_ops, tc6);
> +	if (!tc6->phy_irq_domain)
> +		return -ENOMEM;
> +
> +	tc6->phy_virq = irq_create_mapping(tc6->phy_irq_domain, 0);
> +	WRITE_ONCE(tc6->phy_irq_masked, true);

[Severity: Medium]
Is phy_irq_masked set too late here?

irq_create_mapping() calls into the .map callback, and
irq_set_chip_and_handler() takes the chip bus lock in __irq_set_handler():

irq_create_mapping()
  oa_tc6_phy_irq_map()
    irq_set_chip_and_handler()
      irq_set_chip()           /* installs oa_tc6_phy_irq_chip */
      __irq_set_handler()
        scoped_irqdesc_get_and_buslock(irq, 0)
          oa_tc6_phy_irq_bus_lock()
          oa_tc6_phy_irq_bus_sync_unlock()

At that point phy_irq_masked is still false from devm_kzalloc(), so
oa_tc6_phy_irq_bus_sync_unlock() takes this branch:

	else
		regval &= ~OA_TC6_INT_MASK0_PHY_INT_MASK;

It then writes INT_MASK0 over SPI, unmasking PHYINT in the MAC-PHY before
phylib has requested the IRQ. Only after that is the shadow set to
masked.

That seems to contradict this part of the commit message:

    The interrupt starts masked (hardware reset default) and is only
    unmasked when phylib requests it

If phylib never requests the IRQ (for example, the PHY driver has no
interrupt support and phy_probe() falls back to PHY_POLL), hardware PHYINT
stays unmasked while the shadow says masked. The same happens if
mdiobus_register() fails and the mapping is disposed.

Would setting phy_irq_masked to true before irq_create_mapping() avoid
this?

> +	if (!tc6->phy_virq) {
> +		irq_domain_remove(tc6->phy_irq_domain);
> +		return -ENOMEM;
> +	}
> +
> +	return 0;
> +}

[ ... ]

> @@ -624,6 +821,7 @@ static void oa_tc6_phy_exit(struct oa_tc6 *tc6)
>  	if (tc6->quirk_flags & OA_TC6_BROKEN_PHY)
>  		return;
>  
> +	cancel_work_sync(&tc6->phy_irq_work);

[Severity: Medium]
Can this hit the WARN_ON(!work->func) in __flush_work()?

phy_irq_work is only initialized by the INIT_WORK() in
oa_tc6_phy_irq_setup(). oa_tc6_mdiobus_register() only calls that when
OA_TC6_PHY_INT is set, and tc6 comes from devm_kzalloc().

At this commit, lan865x_probe() does:

	priv->tc6 = oa_tc6_init(spi, netdev, NULL);

so quirk_flags is 0. Removal then goes through:

oa_tc6_exit()
  oa_tc6_phy_exit()
    cancel_work_sync()
      __cancel_work_sync()
        __flush_work()
          if (WARN_ON(!work->func))

with a zeroed work_struct.

That looks reachable on every lan865x remove or unbind. It also looks
reachable on the error paths that call oa_tc6_phy_exit(), both in
oa_tc6_init() and on lan865x probe failure after oa_tc6_init().

The follow-up patch "microchip: lan865x: enable PHY interrupt via virtual
IRQ" sets OA_TC6_PHY_INT for lan865x. The library would still warn for any
user that passes NULL or zero quirks, though, and this commit on its own
breaks bisection for lan865x.

Could INIT_WORK() move to oa_tc6_init() unconditionally? Alternatively,
cancel_work_sync() could be gated on OA_TC6_PHY_INT or tc6->phy_virq.

>  	phy_disconnect(tc6->phydev);
>  	oa_tc6_mdiobus_unregister(tc6);
>  }
> @@ -780,7 +978,12 @@ static void oa_tc6_disable_traffic(struct oa_tc6 *tc6)
>  	netif_tx_disable(tc6->netdev);
>  	oa_tc6_drop_tx_skb(tc6, skb);
>  	oa_tc6_free_ongoing_skbs(tc6);
> +	/* Serialize INT_MASK0 write with phylib's mask/unmask to prevent
> +	 * read-modify-write races in oa_tc6_phy_irq_bus_sync_unlock().
> +	 */
> +	mutex_lock(&tc6->phy_irq_lock);
>  	oa_tc6_write_register(tc6, OA_TC6_REG_INT_MASK0, regval);
> +	mutex_unlock(&tc6->phy_irq_lock);

[Severity: Low]
Does this lock keep PHYINT masked after the point of no return?

The mutex serializes this single write. However, phy_irq_work is not
cancelled, phy_irq_masked is not updated, and later read-modify-writes are
not blocked.

In oa_tc6_process_extended_status(), the PHY_INT dispatch runs before the
TX protocol, loss of frame and header error checks that return -ENODEV. A
single STATUS0 value carrying PHY_INT plus one of those error bits will
queue the work. It then leads oa_tc6_macphy_threaded_irq() into
disable_irq_nosync() and oa_tc6_disable_traffic(). Any SPI failure while
the work is already pending gets to the same place.

The work can then run concurrently, or after this write:

oa_tc6_phy_irq_work()
  handle_nested_irq()          /* PHY driver SPI control transfers */
  oa_tc6_phy_irq_unmask_hw()
    phy_irq_masked is false, INT_MASK0 reads back all-masked
    regval &= ~OA_TC6_INT_MASK0_PHY_INT_MASK;
    oa_tc6_write_register(tc6, OA_TC6_REG_INT_MASK0, regval);

This unmasks PHYINT again and keeps issuing PHY control transfers on a
device that has been shut down. Neither oa_tc6_phy_irq_unmask_hw() nor
oa_tc6_phy_irq_bus_sync_unlock() checks tc6->disable_traffic.

The host IRQ is already disabled by disable_irq_nosync(), so there is no
host interrupt storm. Should the PHY work still be stopped here, or should
the unmask paths check tc6->disable_traffic?

>  	oa_tc6_read_register(tc6, OA_TC6_REG_STATUS0, &regval);
>  	oa_tc6_write_register(tc6, OA_TC6_REG_STATUS0, regval);
>  	dev_err(&tc6->spi->dev, "Device interrupt disabled to avoid interrupt storm");
> @@ -813,6 +1016,29 @@ static int oa_tc6_process_extended_status(struct oa_tc6 *tc6)
>  		return ret;
>  	}
>  
> +	/* Dispatch the PHY interrupt to phylib via the nested virtual IRQ so

[ ... ]

> +	 * Gate on phy_virq (the actual resource) rather than just the flag to
> +	 * be self-consistent if OA_TC6_BROKEN_PHY skips initialization.
> +	 */
> +	if (tc6->phy_virq && FIELD_GET(OA_TC6_STATUS0_PHY_INT, value)) {
> +		ret = oa_tc6_phy_irq_mask_hw(tc6);

[Severity: Low]
Does this repeat a full INT_MASK0 read-modify-write for every chunk footer
while PHYINT is still asserted?

According to the commit message, PHYINT is level triggered and stays
asserted until the worker services the PHY. So the STATUS0 write-back does
not clear it, and the "if (!value) return 0;" shortcut never fires.

Footers are processed after the whole data transaction completes. Every
EXST footer captured after the PHY event therefore costs a STATUS0 read, a
STATUS0 write, and the two INT_MASK0 transfers in oa_tc6_phy_irq_mask_hw(),
even though bit 7 is already set. It also costs a no-op schedule_work().

The same redundant read-modify-write happens when an unrelated
extended-status event arrives while PHYINT is already masked. That seems
to work against the goal of not stalling the thread that pumps TX/RX
chunks.

Would it make sense to skip the mask while a PHY dispatch is already
pending?

> +		if (ret)
> +			dev_err(&tc6->spi->dev,
> +				"Failed to mask PHY interrupt: %d\n", ret);
> +		else
> +			schedule_work(&tc6->phy_irq_work);
> +	}

[ ... ]

> diff --git a/include/linux/oa_tc6.h b/include/linux/oa_tc6.h
> index 27f652d4920b2..1c8cc263c5e53 100644
> --- a/include/linux/oa_tc6.h
> +++ b/include/linux/oa_tc6.h

[ ... ]

> @@ -68,6 +70,7 @@ struct oa_tc6;
>  
>  enum oa_tc6_quirk_flag {
>  	OA_TC6_BROKEN_PHY = BIT(0),
> +	OA_TC6_PHY_INT    = BIT(1),
>  };

[Severity: Low]
This isn't a bug, but no in-tree MAC driver sets OA_TC6_PHY_INT in this
commit. lan865x passes NULL quirks and adin1140 passes OA_TC6_BROKEN_PHY,
so the new irq_chip, irq_domain and workqueue path is unused at this point
in the series.

The next patch, "microchip: lan865x: enable PHY interrupt via virtual
IRQ", sets tc6_quirks.quirk_flags = OA_TC6_PHY_INT in lan865x_probe(). It
also carries the Fixes: 78341049fbcd tag and names this commit as its
prerequisite. So within the series this looks like the usual
infrastructure-then-user split.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929125928.611784-1-parthiban.veerasooran%40microchip.com

Re: [PATCH net-next v4 3/4] microchip: lan865x: enable PHY interrupt via virtual IRQ

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] This patch sets OA_TC6_PHY_INT for LAN865X, which moves the
  internal PHY from phylib polling to interrupt mode.

--

microchip: lan865x: enable PHY interrupt via virtual IRQ

This patch passes the OA_TC6_PHY_INT quirk flag to oa_tc6_init() from
lan865x_probe(). The internal LAN865X PHY then runs phylib in interrupt
mode through the oa_tc6 virtual IRQ, so CDEN follows PLCA transitions via
lan86xx_handle_interrupt() and no longer depends on the polling fallback.

> diff --git a/drivers/net/ethernet/microchip/lan865x/lan865x.c b/drivers/net/ethernet/microchip/lan865x/lan865x.c
> index 127afb9e9f141..d47560f9847d5 100644
> --- a/drivers/net/ethernet/microchip/lan865x/lan865x.c
> +++ b/drivers/net/ethernet/microchip/lan865x/lan865x.c

[ ... ]

> @@ -346,7 +347,8 @@ static int lan865x_probe(struct spi_device *spi)
>  	spi_set_drvdata(spi, priv);
>  	INIT_WORK(&priv->multicast_work, lan865x_multicast_work_handler);
>  
> -	priv->tc6 = oa_tc6_init(spi, netdev, NULL);
> +	tc6_quirks.quirk_flags = OA_TC6_PHY_INT;
> +	priv->tc6 = oa_tc6_init(spi, netdev, &tc6_quirks);

[Severity: Medium]
Can one failed SPI transfer permanently stop CDEN tracking for LAN865X
once this flag is set?

With OA_TC6_PHY_INT, oa_tc6_mdiobus_register() puts tc6->phy_virq into
mii_bus->irq[]. phy_polling_mode() then returns false, and
_phy_state_machine() stops requeueing every PHY_STATE_TIME.

That means lan86xx_read_status()->lan86xx_update_cden() no longer runs
periodically. That periodic resync is the self-healing fallback added by
"net: phy: microchip_t1s: fix collision detection on PLCA status change".

In interrupt mode, oa_tc6_process_extended_status() masks PHYINT in
hardware before it schedules the worker:

drivers/net/ethernet/oa_tc6.c:oa_tc6_process_extended_status() {
    ...
	if (tc6->phy_virq && FIELD_GET(OA_TC6_STATUS0_PHY_INT, value)) {
		ret = oa_tc6_phy_irq_mask_hw(tc6);
		...
		else
			schedule_work(&tc6->phy_irq_work);
	}
    ...
}

The worker then tries to unmask it, but it only logs a failure:

drivers/net/ethernet/oa_tc6.c:oa_tc6_phy_irq_work() {
    ...
	handle_nested_irq(tc6->phy_virq);

	ret = oa_tc6_phy_irq_unmask_hw(tc6);
	if (ret)
		dev_err(&tc6->spi->dev, "Failed to unmask PHY interrupt: %d\n",
			ret);
}

Suppose the INT_MASK0 read or write in oa_tc6_phy_irq_unmask_hw() returns
an error. INT_MASK0.PHYINT then stays set in hardware, while
tc6->phy_irq_masked is still false.

After that, later PLCA status changes never raise EXST, so
lan86xx_handle_interrupt() stops running. Nothing retries or reschedules
the work, and polling is off.

The normal ifdown/ifup path (phy_stop()/phy_start()) does not go through
disable_irq()/enable_irq(), so oa_tc6_phy_irq_bus_sync_unlock() is never
called to restore the mask. Only suspend/resume or unbinding and
re-probing the driver seems to recover it.

Before this patch, the 1 second poll would have fixed CDEN shortly after
such an error. Would it make sense to retry or reschedule the unmask on
failure, or to keep some periodic CDEN resync for this case?

>  	if (IS_ERR(priv->tc6)) {
>  		ret = PTR_ERR(priv->tc6);
>  		goto free_netdev;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929125928.611784-1-parthiban.veerasooran%40microchip.com

Re: [PATCH net-next v4 4/4] net: phy: microchip_t1s: fix collision detection for LAN867X Rev.D0

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 (1) · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] On LAN867X Rev.D0 the driver never actually enables CDEN
  (collision detection, COL_DET_CTRL0 bit 15), although the design…
- [Medium] lan867x_revd0_config_intr() and
  lan867x_revd0_handle_interrupt() choose the Rev.D0 link-status source…
- [Low] In steady state, the PLCA_STS_CHANGED branch of
  lan867x_revd0_handle_interrupt() changes nothing.
- [Low] lan867x_revd0_config_intr() reads priv->plca_enabled and does a
  read-then-write of PRSCTL1 and LINK_STATUS_CTRL without taking…

Pre-existing issues:
- [Medium] In polling mode (no PHY IRQ), the Rev.D0 link-status selection
  is not resynced after phy_init_hw().

--

net: phy: microchip_t1s: fix collision detection for LAN867X Rev.D0

This commit configures CCMFC in COL_DET_CTRL0 on LAN867X Rev.D0 so that
collision forwarding is gated by PLCA_Status in hardware. It also stops
software CDEN toggling on Rev.D0. It adds Rev.D0 specific config_intr and
handle_interrupt callbacks that re-evaluate the link status selection
(forcing the semaphore source when CSMA/CD fallback is enabled), and sets
PHY_POLL_CABLE_TEST for Rev.D0.

> diff --git a/drivers/net/phy/microchip_t1s.c b/drivers/net/phy/microchip_t1s.c
> index 5ce0304bf0952..d667f57aa4a8f 100644
> --- a/drivers/net/phy/microchip_t1s.c
> +++ b/drivers/net/phy/microchip_t1s.c

[ ... ]

> @@ -542,7 +574,7 @@ static int lan86xx_plca_set_cfg(struct phy_device *phydev,
>  	 * (not -1, which means "don't change").
>  	 */
>  	if (phydev->phy_id == PHY_ID_LAN867X_REVD0 && plca_cfg->enabled != -1) {
> -		ret = lan867x_revd0_link_active_selection(phydev,
> +		ret = lan867x_revd0_update_link_selection(phydev,
>  							  plca_cfg->enabled);
>  		if (ret)
>  			return ret;
> @@ -555,6 +587,12 @@ static int lan86xx_plca_set_cfg(struct phy_device *phydev,
>  	if (plca_cfg->enabled != -1)
>  		priv->plca_enabled = plca_cfg->enabled;
>  

[Severity: Low]
Can the Rev.D0 link selection end up based on the old enable state here?
lan86xx_plca_set_cfg() writes the selection from plca_cfg->enabled above.
It updates priv->plca_enabled only here and does not recompute afterwards.

lan867x_revd0_config_intr() reads priv->plca_enabled and then does the
PRSCTL1 read and the LINK_STATUS_CTRL write without taking phydev->lock
or priv->cden_lock. The sibling lan86xx_config_intr() does take cden_lock
for its resync. The commit that added it notes that config_intr() is not
guaranteed to run under phydev->lock.

If lan867x_revd0_config_intr() runs between those two steps, it would
write a selection based on the old cached value. Nothing would correct
that until the next PSTC event. The unlocked read of priv->plca_enabled
is also a data race.

I couldn't find a concrete caller for this PHY that runs phy_init_hw()
at the same time as an ethtool PLCA set. Some MAC drivers do call
phy_init_hw() from their own work or reset paths, though.

> +	/* LAN867X Rev.D0 uses CCMFC for autonomous collision detection
> +	 * gating; CDEN remains enabled and does not require software toggling.
> +	 */
> +	if (phydev->phy_id == PHY_ID_LAN867X_REVD0)
> +		return 0;
> +

[ ... ]

> @@ -582,6 +620,20 @@ static int lan867x_revd0_config_init(struct phy_device *phydev)
>  			return ret;
>  	}
>  
> +	/* AN1699: Configure CCMFC (Collision Counting and MAC Forwarding
> +	 * Control) to OA default (0x1) so that the hardware autonomously gates
> +	 * collision forwarding to the MAC based on the live PLCA_Status:
> +	 * collisions are neither counted nor forwarded when PLCA_Status is OK,
> +	 * and are counted/forwarded when not OK. This eliminates the need for
> +	 * software-driven CDEN toggling. CDEN defaults to enabled on Rev.D0
> +	 * and remains enabled.
> +	 */
> +	ret = phy_modify_mmd(phydev, MDIO_MMD_VEND2, LAN86XX_REG_COL_DET_CTRL0,
> +			     COL_DET_CTRL0_CCMFC_MASK,
> +			     COL_DET_CTRL0_CCMFC_OA_DEFAULT);

[Severity: Medium]
This call only changes bits 10:9 of COL_DET_CTRL0, so bit 15 (CDEN)
keeps whatever value the PHY already has. Does anything on Rev.D0
actually set CDEN?

The comment added above lan86xx_plca_set_cfg() says:

  CDEN remains permanently enabled in config_init()

However, this phy_modify_mmd() does not write COL_DET_ENABLE, and
lan867x_revd0_fixup_regs[] has no entry for 0x0087. Rev.D0 also has no
.soft_reset, so phy_init_hw() does not reset the PHY back to its
defaults before config_init() runs.

Before this patch, lan86xx_plca_set_cfg() cleared CDEN on Rev.D0
whenever PLCA was enabled. It did this through lan86xx_update_cden(),
and through a direct COL_DET_DISABLE write since commit 78341049fbcd.
With the new early return in lan86xx_plca_set_cfg():

	if (phydev->phy_id == PHY_ID_LAN867X_REVD0)
		return 0;

no Rev.D0 path writes CDEN any more.

Suppose the PHY comes up with CDEN=0 and is not power cycled. Two
examples are a warm reboot or kexec from an older kernel after
"ethtool --set-plca-cfg ... enable on", and a bootloader that followed
the older AN1699 advice. Would CDEN then stay disabled for good?

CCMFC only controls whether detected collisions are counted and
forwarded. In that case, CSMA/CD operation (PLCA disabled, or the
autonomous fallback) would run with no collision detection.

Could COL_DET_CTRL0_ENABLE_BIT_MASK and COL_DET_ENABLE be added to this
phy_modify_mmd() call?

[ ... ]

> @@ -712,6 +764,76 @@ static irqreturn_t lan86xx_handle_interrupt(struct phy_device *phydev)
>  	return ret_irq;
>  }
>  
> +static int lan867x_revd0_config_intr(struct phy_device *phydev)
> +{
> +	u16 mask = LAN86XX_STS1_PLCA_STS_CHANGED |
> +		   LAN86XX_STS1_LINK_STS_CHANGED;
> +	struct lan86xx_priv *priv = phydev->priv;
> +	int sts1, ret;
> +
> +	if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
> +		/* Read to clear any pending status before enabling. */
> +		sts1 = lan86xx_read_clear_sts1(phydev);
> +		if (sts1 < 0)
> +			return sts1;
> +
> +		/* STS1 may have cleared a pending PSTC while masked, and a
> +		 * missed PSTC leaves no trace to key off, so unconditionally
> +		 * resync the link-status-selection source from the current
> +		 * PLCA enable state and fallback configuration.
> +		 */
> +		ret = lan867x_revd0_update_link_selection(phydev,
> +							  priv->plca_enabled);

[Severity: Medium]
Can priv->plca_enabled be stale at this point? Only
lan86xx_plca_set_cfg() writes it, and the PHY can be hardware reset
while the driver stays bound.

For example, phy_detach() asserts the reset GPIO with
phy_device_reset(phydev, 1). A MAC that connects the PHY in ndo_open and
disconnects it in ndo_stop therefore resets it on every ifdown/ifup.
Losing power across suspend has the same effect.

Suppose PLCA was enabled with ethtool, so priv->plca_enabled == 1. The
next attach then does:

phy_attach_direct()
  phy_init_hw()
    lan867x_revd0_config_init()
      lan867x_revd0_link_active_selection(phydev, false)  /* SEMAPHORE */
...
phy_request_interrupt()
  phy_enable_interrupts()
    phy_config_interrupt()
      lan867x_revd0_config_intr()
        lan867x_revd0_update_link_selection(phydev, 1)    /* PLCA_STATUS */

If PRSCTL1 fallback reads 0, this selects LINK_STATUS_CONFIG_PLCA_STATUS
while PLCA_EN is 0 in hardware. Would the BMSR link then stay down until
userspace runs ethtool --set-plca-cfg again? The PSTC branch in
lan867x_revd0_handle_interrupt() uses the same cached value, so it would
not recover this either.

Before this patch, Rev.D0 had no config_intr. The SEMAPHORE selection
from config_init() stayed in place and the link came up.

Would reading the PLCA_EN bit from MDIO_OATC14_PLCA_CTRL0 here, or
resyncing priv->plca_enabled from hardware in config_init(), avoid this?

> +		if (ret < 0)
> +			return ret;
> +
> +		return lan86xx_set_intr_mask(phydev, mask, true);
> +	}
> +
> +	ret = lan86xx_set_intr_mask(phydev, mask, false);

[Severity: Medium]
This is a pre-existing issue and was not introduced by this patch.
The resync above only runs in the PHY_INTERRUPT_ENABLED branch.

In polling mode, phy_init_hw() calls lan867x_revd0_config_init(), which
always forces the SEMAPHORE source. It then calls
lan867x_revd0_config_intr(), which takes this disable branch and never
calls lan867x_revd0_update_link_selection().

If the PHY was not reset (resume without power loss, or re-attach
without a reset GPIO), PLCA_EN and priv->plca_enabled both remain 1.
With fallback disabled, is the link then forced up instead of following
PLCA_Status until ethtool --set-plca-cfg runs again?

config_init() has forced SEMAPHORE on every re-init since commit
07f5765f26c3. This patch adds the resync only for interrupt mode.

[ ... ]

> +static irqreturn_t lan867x_revd0_handle_interrupt(struct phy_device *phydev)
> +{

[ ... ]

> +	if (sts1 & LAN86XX_STS1_PLCA_STS_CHANGED) {
> +		/* Re-evaluate the link-status selection when PLCA status
> +		 * changes. A resulting link-status change raises
> +		 * LINK_STS_CHANGED, which triggers the PHY state machine.
> +		 */
> +		ret = lan867x_revd0_update_link_selection(phydev,
> +							  priv->plca_enabled);

[Severity: Low]
Can this call ever change LINK_STATUS_CTRL in steady state?
lan867x_revd0_update_link_selection() only looks at PRSCTL1 bit 10
(through lan86xx_fallback_to_csmacd()) and the cached
priv->plca_enabled. It never reads PLCA_STATUS.

This driver only reads PRSCTL1 and never writes it. priv->plca_enabled
only changes in lan86xx_plca_set_cfg(), which has already written the
selection for the new value. So each BEACON gain or loss seems to cost a
PRSCTL1 read plus a rewrite of the same LINK_STATUS_CTRL value.

The comment says a resulting link-status change raises
LINK_STS_CHANGED. When the selection is PLCA_STATUS, though, the hardware
already changes the link and raises LINK_STS_CHANGED on its own.

The commit message also says:

  The PSTC interrupt handler for Rev.D0 therefore only needs to update
  the link status selection on each PLCA transition.

Does Rev.D0 need LAN86XX_STS1_PLCA_STS_CHANGED unmasked at all? If it
does, could the commit message and this comment be updated to say what
the branch actually does?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929125928.611784-1-parthiban.veerasooran%40microchip.com

Re: [PATCH net-next v4 1/4] net: phy: microchip_t1s: fix collision detection on PLCA status change

[Parthiban Veerasooran]

Hi,

Thanks for the review.

I’ll look into the reported comments in all the patches in this series 
and get back with the necessary updates as soon as possible.

Thanks,
Parthiban V

On 03/10/26 7:06 pm, netdev-bot+sashiko@kernel.org wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 2
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [Medium] The collision-detect (CDEN) setting is chosen from a software
>    copy of the PLCA enable state (priv->plca_enabled), not from the PHY's…
> - [Low] [Baseline decision: false.
> - [Low] [Baseline decision: false.
> 
> --
> 
> net: phy: microchip_t1s: fix collision detection on PLCA status change
> 
> This patch makes the collision detection enable bit (CDEN) follow
> autonomous PLCA/CSMA-CD transitions. It adds a PLCA status change
> interrupt and a resync in read_status(), and it takes the PRSCTL1
> fallback bit into account. It also adds a probe routine that allocates
> a per-PHY mutex and a cached plca_enabled value, and it treats
> plca_cfg->enabled as tri-state.
> 
>> diff --git a/drivers/net/phy/microchip_t1s.c b/drivers/net/phy/microchip_t1s.c
>> index 73c23d311d72a..5ce0304bf0952 100644
>> --- a/drivers/net/phy/microchip_t1s.c
>> +++ b/drivers/net/phy/microchip_t1s.c