Netdev List
 help / color / mirror / Atom feed
From: Parthiban Veerasooran <parthiban.veerasooran@microchip.com>
To: <andrew+netdev@lunn.ch>, <davem@davemloft.net>,
	<edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>,
	<hkallweit1@gmail.com>, <linux@armlinux.org.uk>
Cc: <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<UNGLinuxDriver@microchip.com>,
	<Parthiban.Veerasooran@microchip.com>,
	Parthiban Veerasooran <parthiban.veerasooran@microchip.com>
Subject: [PATCH net-next v3 1/4] net: phy: microchip_t1s: fix collision detection on PLCA status change
Date: Fri, 18 Sep 2026 20:02:59 +0530	[thread overview]
Message-ID: <20260918143302.76909-2-parthiban.veerasooran@microchip.com> (raw)
In-Reply-To: <20260918143302.76909-1-parthiban.veerasooran@microchip.com>

lan86xx_plca_set_cfg() only adjusted collision detection on explicit
ethtool PLCA changes, missing the autonomous online/offline transitions
the PHY performs based on BEACON availability. When PLCA went offline,
collision detection stayed disabled, leaving CSMA/CD running unprotected.

Fix this by monitoring the PLCA Status Changed (PSTC) interrupt.
lan86xx_config_intr() enables/disables PSTCM in IMSK1;
lan86xx_handle_interrupt() reads live PLCA status on each interrupt and
toggles CDEN in COL_DET_CTRL0 accordingly. CDEN is re-synced against
current PLCA status right after unmasking PSTCM, since a transition
during the masked window is otherwise silently dropped by the STS1
read-to-clear.

lan86xx_read_clear_sts1() and lan86xx_set_intr_mask() factor out the
shared STS1/IMSK1 sequences; both are reused by the Rev.D0 handling
added later in this series.

Wired to LAN867X Rev.B1, C1, C2 and LAN865X Rev.B0/B1. Rev.D0 needs
separate handling (follow-on patch). Boards without a routed interrupt
(phydev->irq == PHY_POLL, including LAN865X until later patches) keep
the static CDEN write in lan86xx_plca_set_cfg() as a baseline, with the
known limitation that autonomous transitions between ethtool calls
aren't tracked there.

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 | 142 +++++++++++++++++++++++++++++++-
 1 file changed, 139 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/microchip_t1s.c b/drivers/net/phy/microchip_t1s.c
index 73c23d311d72..3333e4801aae 100644
--- a/drivers/net/phy/microchip_t1s.c
+++ b/drivers/net/phy/microchip_t1s.c
@@ -27,6 +27,14 @@
 #define LAN865X_REG_CFGPARAM_CTRL 0x00DA
 #define LAN865X_REG_STS2 0x0019
 
+/* PHY interrupt status and mask registers (MDIO_MMD_VEND2). The status bits
+ * are read-to-clear; a mask bit is enabled by writing 0.
+ */
+#define LAN86XX_REG_STS1		0x0018
+#define LAN86XX_REG_IMSK1		0x001C
+
+#define LAN86XX_STS1_PLCA_STS_CHANGED	BIT(11)
+
 /* Collision Detector Control 0 Register */
 #define LAN86XX_REG_COL_DET_CTRL0	0x0087
 #define COL_DET_CTRL0_ENABLE_BIT_MASK	BIT(15)
@@ -458,14 +466,29 @@ static int lan86xx_plca_set_cfg(struct phy_device *phydev,
 	if (ret)
 		return ret;
 
-	if (plca_cfg->enabled)
+	/* phylib dispatches handle_interrupt() only for PHYs with a real IRQ
+	 * number (phy_interrupt_is_valid()). For PHY_POLL and PHY_MAC_INTERRUPT
+	 * handle_interrupt() is never called, so apply the static CDEN write
+	 * here as a baseline on every ethtool PLCA reconfiguration. The
+	 * limitation is that autonomous PLCA mode transitions between ethtool
+	 * reconfigurations are not tracked on such boards.
+	 */
+	if (phy_interrupt_is_valid(phydev))
+		return 0;
+
+	if (plca_cfg->enabled > 0)
 		return phy_modify_mmd(phydev, MDIO_MMD_VEND2,
 				      LAN86XX_REG_COL_DET_CTRL0,
 				      COL_DET_CTRL0_ENABLE_BIT_MASK,
 				      COL_DET_DISABLE);
 
-	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 == 0)
+		return phy_modify_mmd(phydev, MDIO_MMD_VEND2,
+				      LAN86XX_REG_COL_DET_CTRL0,
+				      COL_DET_CTRL0_ENABLE_BIT_MASK,
+				      COL_DET_ENABLE);
+
+	return 0;
 }
 
 static int lan867x_revd0_config_init(struct phy_device *phydev)
@@ -506,6 +529,111 @@ static int lan86xx_read_status(struct phy_device *phydev)
 	return 0;
 }
 
+/* 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 phy_plca_status plca_st;
+	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.
+		 */
+		ret = genphy_c45_plca_get_status(phydev, &plca_st);
+		if (ret < 0)
+			return ret;
+
+		ret = phy_modify_mmd(phydev, MDIO_MMD_VEND2,
+				     LAN86XX_REG_COL_DET_CTRL0,
+				     COL_DET_CTRL0_ENABLE_BIT_MASK,
+				     plca_st.pst ? COL_DET_DISABLE :
+				     COL_DET_ENABLE);
+		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 phy_plca_status plca_st;
+	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) {
+		ret = genphy_c45_plca_get_status(phydev, &plca_st);
+		if (ret < 0) {
+			phy_error(phydev);
+			return IRQ_NONE;
+		}
+
+		/* AN1760/AN1699: disable collision detection in PLCA mode to
+		 * improve signal quality; re-enable it in CSMA/CD mode.
+		 *
+		 * https://www.microchip.com/en-us/application-notes/an1760
+		 * https://www.microchip.com/en-us/application-notes/an1699
+		 */
+		ret = phy_modify_mmd(phydev, MDIO_MMD_VEND2,
+				     LAN86XX_REG_COL_DET_CTRL0,
+				     COL_DET_CTRL0_ENABLE_BIT_MASK,
+				     plca_st.pst ? COL_DET_DISABLE :
+				     COL_DET_ENABLE);
+		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),
@@ -513,6 +641,8 @@ static struct phy_driver microchip_t1s_driver[] = {
 		.features           = PHY_BASIC_T1S_P2MP_FEATURES,
 		.config_init        = lan867x_revb1_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	    = genphy_c45_plca_set_cfg,
 		.get_plca_status    = genphy_c45_plca_get_status,
@@ -523,6 +653,8 @@ static struct phy_driver microchip_t1s_driver[] = {
 		.features           = PHY_BASIC_T1S_P2MP_FEATURES,
 		.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,
@@ -533,6 +665,8 @@ static struct phy_driver microchip_t1s_driver[] = {
 		.features           = PHY_BASIC_T1S_P2MP_FEATURES,
 		.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,
@@ -556,6 +690,8 @@ static struct phy_driver microchip_t1s_driver[] = {
 		.features           = PHY_BASIC_T1S_P2MP_FEATURES,
 		.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


  reply	other threads:[~2026-09-18 14:33 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-18 14:32 [PATCH net-next v3 0/4] net: microchip_t1s: fix collision detection on PLCA status change Parthiban Veerasooran
2026-09-18 14:32 ` Parthiban Veerasooran [this message]
2026-09-22 15:08   ` [PATCH net-next v3 1/4] net: phy: " netdev-bot+sashiko
2026-09-18 14:33 ` [PATCH net-next v3 2/4] net: ethernet: oa_tc6: deliver the PHY interrupt to phylib Parthiban Veerasooran
2026-09-22 15:08   ` netdev-bot+sashiko
2026-09-18 14:33 ` [PATCH net-next v3 3/4] microchip: lan865x: enable PHY interrupt via virtual IRQ Parthiban Veerasooran
2026-09-22 15:08   ` netdev-bot+sashiko
2026-09-18 14:33 ` [PATCH net-next v3 4/4] net: phy: microchip_t1s: fix collision detection for LAN867X Rev.D0 Parthiban Veerasooran
2026-09-22 15:08   ` netdev-bot+sashiko
2026-09-24 15:33 ` [PATCH net-next v3 0/4] net: microchip_t1s: fix collision detection on PLCA status change Jakub Kicinski
2026-09-25 10:57   ` Parthiban Veerasooran

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260918143302.76909-2-parthiban.veerasooran@microchip.com \
    --to=parthiban.veerasooran@microchip.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox