All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stepan Svatenko <ssvatenko@iit.org.ua>
To: Raju.Rangoju@amd.com, PrashanthKumar.K.R@amd.com
Cc: andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Stepan Svatenko <ssvatenko@iit.org.ua>,
	stable@vger.kernel.org
Subject: [PATCH net 2/2] amd-xgbe: fix an_irq leak causing permanent -EBUSY on PHY (re)start
Date: Fri, 28 Aug 2026 12:20:23 +0300	[thread overview]
Message-ID: <20260828092023.105405-3-ssvatenko@iit.org.ua> (raw)
In-Reply-To: <20260828092023.105405-1-ssvatenko@iit.org.ua>

xgbe_phy_start() requests the separate AN/PCS interrupt (an_irq) and,
on any failure past that point, is expected to free it again via the
err_irq/err_stop labels before returning an error. The last error path
skips that cleanup:

  pdata->phy_started = 1;
  xgbe_an_init(pdata);
  xgbe_an_enable_interrupts(pdata);
  return xgbe_phy_config_aneg(pdata);   <- returns directly on error

xgbe_phy_config_aneg() (via __xgbe_phy_config_aneg()) can genuinely
fail, e.g. when phy_impl.an_config() fails against a non-functional
SFP module. When it does, xgbe_phy_start() returns that error without
going through err_irq/err_stop, so:

 - an_irq is never freed with devm_free_irq(), and
 - pdata->phy_started is left set to 1, even though the caller
   (xgbe_start()) now treats this as a failed start and does not call
   phy_if->phy_stop() itself on that path.

Any later retry of xgbe_phy_start() (interface bring-up retried by
userspace, or the driver's own recovery logic) then calls
devm_request_irq() for the same still-registered an_irq and gets
-EBUSY every time, with no way to recover short of a reboot/power
cycle:

  genirq: Flags mismatch irq 63. 00200000 (enp8s0f3-pcs) vs. 00200000 (enp8s0f3-pcs)
  amd-xgbe 0000:08:00.3: error -EBUSY: request_irq(63) xgbe_an_isr [amd_xgbe] 0x0 enp8s0f3-pcs
  amd-xgbe 0000:08:00.3 enp8s0f3: phy irq request failed

Reproduced on a SolidRun Bedrock V3000 (AMD Ryzen Embedded V3C48) by
inserting a non-functional SFP module, then bringing the interface up.

Fix this by routing the xgbe_phy_config_aneg() failure through
xgbe_phy_stop(), which already contains the correct, symmetric
teardown (disables AN, frees an_irq if separate, cancels the bh work,
stops the PHY implementation) and is safe to call here because it is
gated on pdata->phy_started.

Fixes: 7c12aa08779c ("amd-xgbe: Move the PHY support into amd-xgbe")
Cc: stable@vger.kernel.org
Signed-off-by: Stepan Svatenko <ssvatenko@iit.org.ua>
Assisted-by: Claude Code:claude-sonnet-5 [Bash] [Read] [Edit]
---
 drivers/net/ethernet/amd/xgbe/xgbe-mdio.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
index 12770af031eb..638c24b9c83c 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
@@ -1445,7 +1445,19 @@ static int xgbe_phy_start(struct xgbe_prv_data *pdata)
 	xgbe_an_init(pdata);
 	xgbe_an_enable_interrupts(pdata);
 
-	return xgbe_phy_config_aneg(pdata);
+	ret = xgbe_phy_config_aneg(pdata);
+	if (ret) {
+		/* Tear down what was just brought up above (including
+		 * freeing the an_irq) instead of returning with phy_started
+		 * left set and an_irq still registered - otherwise a retry
+		 * calls devm_request_irq() on an already-owned an_irq and
+		 * gets stuck in a permanent -EBUSY loop.
+		 */
+		xgbe_phy_stop(pdata);
+		return ret;
+	}
+
+	return 0;
 
 err_irq:
 	if (pdata->dev_irq != pdata->an_irq)
-- 
2.55.0


      parent reply	other threads:[~2026-08-28  9:22 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-28  9:20 [PATCH net 0/2] amd-xgbe: fix two PHY/IRQ lifecycle bugs found on SolidRun Bedrock V3000 Stepan Svatenko
2026-08-28  9:20 ` [PATCH net 1/2] amd-xgbe: fix comm_ownership mutex deadlock on SFP module removal Stepan Svatenko
2026-09-03  2:40   ` Jakub Kicinski
2026-09-18 14:22     ` [PATCH v2] " Stepan Svatenko
2026-09-22 14:39       ` netdev-bot+sashiko
2026-08-28  9:20 ` Stepan Svatenko [this message]

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=20260828092023.105405-3-ssvatenko@iit.org.ua \
    --to=ssvatenko@iit.org.ua \
    --cc=PrashanthKumar.K.R@amd.com \
    --cc=Raju.Rangoju@amd.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=stable@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.