From: Raju Rangoju <Raju.Rangoju@amd.com>
To: <netdev@vger.kernel.org>
Cc: <linux-kernel@vger.kernel.org>, <pabeni@redhat.com>,
<kuba@kernel.org>, <edumazet@google.com>, <davem@davemloft.net>,
<andrew+netdev@lunn.ch>, <Thomas.Lendacky@amd.com>,
Raju Rangoju <Raju.Rangoju@amd.com>
Subject: [PATCH net 1/2] amd-xgbe: fix sleep while atomic on suspend/resume
Date: Wed, 25 Feb 2026 16:30:00 +0530 [thread overview]
Message-ID: <20260225110001.1000014-2-Raju.Rangoju@amd.com> (raw)
In-Reply-To: <20260225110001.1000014-1-Raju.Rangoju@amd.com>
The xgbe_powerdown() and xgbe_powerup() functions use spinlocks
(spin_lock_irqsave) while calling functions that may sleep:
- napi_disable() can sleep waiting for NAPI polling to complete
- flush_workqueue() can sleep waiting for pending work items
This causes a "BUG: scheduling while atomic" error during suspend/resume
cycles on systems using the AMD XGBE Ethernet controller.
The spinlock protection in these functions is unnecessary because:
1. The functions are called from suspend/resume paths which are already
serialized by the PM core
2. The caller parameter was used to differentiate contexts, but the
only current usage is from the driver context (suspend/resume)
3. The power_down flag provides sufficient synchronization
Fix this by:
- Removing the spinlock from xgbe_powerdown() and xgbe_powerup()
- Simplifying the function signatures by removing the unused caller
parameter
- Removing the unused XGMAC_DRIVER_CONTEXT and XGMAC_IOCTL_CONTEXT macros
- Reordering operations in xgbe_powerdown() to disable NAPI before
stopping TX/RX (matching the order used in xgbe_stop())
Fixes: c5aa9e3b8156 ("amd-xgbe: Initial AMD 10GbE platform driver")
Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
---
drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 57 +++++++------------
drivers/net/ethernet/amd/xgbe/xgbe-pci.c | 10 ++--
drivers/net/ethernet/amd/xgbe/xgbe-platform.c | 4 +-
drivers/net/ethernet/amd/xgbe/xgbe.h | 8 +--
4 files changed, 29 insertions(+), 50 deletions(-)
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
index 62bb4b8a68e1..d57daf6306e1 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
@@ -1116,79 +1116,62 @@ static int xgbe_phy_reset(struct xgbe_prv_data *pdata)
return pdata->phy_if.phy_reset(pdata);
}
-int xgbe_powerdown(struct net_device *netdev, unsigned int caller)
+int xgbe_powerdown(struct net_device *netdev)
{
struct xgbe_prv_data *pdata = netdev_priv(netdev);
struct xgbe_hw_if *hw_if = &pdata->hw_if;
- unsigned long flags;
- DBGPR("-->xgbe_powerdown\n");
-
- if (!netif_running(netdev) ||
- (caller == XGMAC_IOCTL_CONTEXT && pdata->power_down)) {
- netdev_alert(netdev, "Device is already powered down\n");
- DBGPR("<--xgbe_powerdown\n");
+ if (!netif_running(netdev)) {
+ netdev_dbg(netdev, "Device is not running, skipping powerdown\n");
return -EINVAL;
}
- spin_lock_irqsave(&pdata->lock, flags);
-
- if (caller == XGMAC_DRIVER_CONTEXT)
- netif_device_detach(netdev);
+ if (pdata->power_down) {
+ netdev_dbg(netdev, "Device is already powered down\n");
+ return -EINVAL;
+ }
+ netif_device_detach(netdev);
netif_tx_stop_all_queues(netdev);
xgbe_stop_timers(pdata);
flush_workqueue(pdata->dev_workqueue);
+ xgbe_napi_disable(pdata, 0);
+
hw_if->powerdown_tx(pdata);
hw_if->powerdown_rx(pdata);
- xgbe_napi_disable(pdata, 0);
-
pdata->power_down = 1;
- spin_unlock_irqrestore(&pdata->lock, flags);
-
- DBGPR("<--xgbe_powerdown\n");
-
return 0;
}
-int xgbe_powerup(struct net_device *netdev, unsigned int caller)
+int xgbe_powerup(struct net_device *netdev)
{
struct xgbe_prv_data *pdata = netdev_priv(netdev);
struct xgbe_hw_if *hw_if = &pdata->hw_if;
- unsigned long flags;
-
- DBGPR("-->xgbe_powerup\n");
- if (!netif_running(netdev) ||
- (caller == XGMAC_IOCTL_CONTEXT && !pdata->power_down)) {
- netdev_alert(netdev, "Device is already powered up\n");
- DBGPR("<--xgbe_powerup\n");
+ if (!netif_running(netdev)) {
+ netdev_dbg(netdev, "Device is not running, skipping powerup\n");
return -EINVAL;
}
- spin_lock_irqsave(&pdata->lock, flags);
-
- pdata->power_down = 0;
-
- xgbe_napi_enable(pdata, 0);
+ if (!pdata->power_down) {
+ netdev_dbg(netdev, "Device is already powered up\n");
+ return -EINVAL;
+ }
hw_if->powerup_tx(pdata);
hw_if->powerup_rx(pdata);
- if (caller == XGMAC_DRIVER_CONTEXT)
- netif_device_attach(netdev);
+ xgbe_napi_enable(pdata, 0);
netif_tx_start_all_queues(netdev);
-
xgbe_start_timers(pdata);
+ netif_device_attach(netdev);
- spin_unlock_irqrestore(&pdata->lock, flags);
-
- DBGPR("<--xgbe_powerup\n");
+ pdata->power_down = 0;
return 0;
}
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
index e3e1dca9856a..112d7697174c 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
@@ -352,14 +352,14 @@ static void xgbe_pci_remove(struct pci_dev *pdev)
xgbe_free_pdata(pdata);
}
-static int __maybe_unused xgbe_pci_suspend(struct device *dev)
+static int xgbe_pci_suspend(struct device *dev)
{
struct xgbe_prv_data *pdata = dev_get_drvdata(dev);
struct net_device *netdev = pdata->netdev;
int ret = 0;
if (netif_running(netdev))
- ret = xgbe_powerdown(netdev, XGMAC_DRIVER_CONTEXT);
+ ret = xgbe_powerdown(netdev);
pdata->lpm_ctrl = XMDIO_READ(pdata, MDIO_MMD_PCS, MDIO_CTRL1);
pdata->lpm_ctrl |= MDIO_CTRL1_LPOWER;
@@ -368,7 +368,7 @@ static int __maybe_unused xgbe_pci_suspend(struct device *dev)
return ret;
}
-static int __maybe_unused xgbe_pci_resume(struct device *dev)
+static int xgbe_pci_resume(struct device *dev)
{
struct xgbe_prv_data *pdata = dev_get_drvdata(dev);
struct net_device *netdev = pdata->netdev;
@@ -380,7 +380,7 @@ static int __maybe_unused xgbe_pci_resume(struct device *dev)
XMDIO_WRITE(pdata, MDIO_MMD_PCS, MDIO_CTRL1, pdata->lpm_ctrl);
if (netif_running(netdev)) {
- ret = xgbe_powerup(netdev, XGMAC_DRIVER_CONTEXT);
+ ret = xgbe_powerup(netdev);
/* Schedule a restart in case the link or phy state changed
* while we were powered down.
@@ -453,7 +453,7 @@ static const struct pci_device_id xgbe_pci_table[] = {
};
MODULE_DEVICE_TABLE(pci, xgbe_pci_table);
-static SIMPLE_DEV_PM_OPS(xgbe_pci_pm_ops, xgbe_pci_suspend, xgbe_pci_resume);
+DEFINE_SIMPLE_DEV_PM_OPS(xgbe_pci_pm_ops, xgbe_pci_suspend, xgbe_pci_resume);
static struct pci_driver xgbe_driver = {
.name = XGBE_DRV_NAME,
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-platform.c b/drivers/net/ethernet/amd/xgbe/xgbe-platform.c
index 47d53e59ccf6..98b03a3f3a95 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-platform.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-platform.c
@@ -384,7 +384,7 @@ static int xgbe_platform_suspend(struct device *dev)
DBGPR("-->xgbe_suspend\n");
if (netif_running(netdev))
- ret = xgbe_powerdown(netdev, XGMAC_DRIVER_CONTEXT);
+ ret = xgbe_powerdown(netdev);
pdata->lpm_ctrl = XMDIO_READ(pdata, MDIO_MMD_PCS, MDIO_CTRL1);
pdata->lpm_ctrl |= MDIO_CTRL1_LPOWER;
@@ -407,7 +407,7 @@ static int xgbe_platform_resume(struct device *dev)
XMDIO_WRITE(pdata, MDIO_MMD_PCS, MDIO_CTRL1, pdata->lpm_ctrl);
if (netif_running(netdev)) {
- ret = xgbe_powerup(netdev, XGMAC_DRIVER_CONTEXT);
+ ret = xgbe_powerup(netdev);
/* Schedule a restart in case the link or phy state changed
* while we were powered down.
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h b/drivers/net/ethernet/amd/xgbe/xgbe.h
index 1269b8ce9249..4e1ab4172abb 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe.h
+++ b/drivers/net/ethernet/amd/xgbe/xgbe.h
@@ -146,10 +146,6 @@
#define XGBE_MAX_PPS_OUT 4
#define XGBE_MAX_AUX_SNAP 4
-/* Driver PMT macros */
-#define XGMAC_DRIVER_CONTEXT 1
-#define XGMAC_IOCTL_CONTEXT 2
-
#define XGMAC_FIFO_MIN_ALLOC 2048
#define XGMAC_FIFO_UNIT 256
#define XGMAC_FIFO_ALIGN(_x) \
@@ -1289,8 +1285,8 @@ void xgbe_dump_rx_desc(struct xgbe_prv_data *, struct xgbe_ring *,
unsigned int);
void xgbe_print_pkt(struct net_device *, struct sk_buff *, bool);
void xgbe_get_all_hw_features(struct xgbe_prv_data *);
-int xgbe_powerup(struct net_device *, unsigned int);
-int xgbe_powerdown(struct net_device *, unsigned int);
+int xgbe_powerup(struct net_device *netdev);
+int xgbe_powerdown(struct net_device *netdev);
void xgbe_init_rx_coalesce(struct xgbe_prv_data *);
void xgbe_init_tx_coalesce(struct xgbe_prv_data *);
void xgbe_restart_dev(struct xgbe_prv_data *pdata);
--
2.34.1
next prev parent reply other threads:[~2026-02-25 11:01 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-25 10:59 [PATCH net 0/2] amd-xgbe: fix S0i3 suspend/resume issues Raju Rangoju
2026-02-25 11:00 ` Raju Rangoju [this message]
2026-02-26 12:37 ` [PATCH net 1/2] amd-xgbe: fix sleep while atomic on suspend/resume Simon Horman
2026-02-26 15:37 ` Rangoju, Raju
2026-02-26 12:40 ` Simon Horman
2026-02-26 15:39 ` Rangoju, Raju
2026-02-25 11:00 ` [PATCH net 2/2] amd-xgbe: add PCI power management for S0i3 support Raju Rangoju
2026-02-26 12:38 ` Simon Horman
2026-02-26 15:42 ` Rangoju, Raju
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=20260225110001.1000014-2-Raju.Rangoju@amd.com \
--to=raju.rangoju@amd.com \
--cc=Thomas.Lendacky@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 \
/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.