From: Shih-Yuan Lee <fourdollars@debian.org>
To: Mark Brown <broonie@kernel.org>
Cc: linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org,
Shih-Yuan Lee <fourdollars@debian.org>
Subject: [PATCH v14 3/3] spi: pxa2xx: restore LPSS private register state on S3 resume
Date: Sun, 19 Jul 2026 00:07:02 +0800 [thread overview]
Message-ID: <20260718160702.7861-4-fourdollars@debian.org> (raw)
In-Reply-To: <20260718160702.7861-1-fourdollars@debian.org>
Intel LPSS SPI controllers lose all private register state across S3
suspend because the LPSS power domain is fully removed. On resume the
driver only re-enables the SSP clock, leaving the LPSS private registers
in their power-on-reset state, which causes two problems:
1. LPSS_PRIV_RESETS (offset 0x04 within the LPSS private space) stays
zero, keeping the functional block in reset. Any MMIO access while
the block is held in reset causes a PCIe Completion Timeout and a
watchdog-triggered system reset. LPSS_PRIV_RESETS_FUNC and
LPSS_PRIV_RESETS_IDMA must be de-asserted before any other register
access on resume.
2. The LPSS software chip-select control register must not be blindly
restored from its suspend-time snapshot: if CS was asserted at the
moment of suspend, restoring that state corrupts the first
post-resume SPI transaction. Instead, call lpss_ssp_setup() which
unconditionally writes SW_MODE | CS_HIGH (idle/deasserted), matching
the state established at probe time.
To resolve these issues safely:
- Wrap S3 suspend/resume with pm_runtime_resume_and_get() and
pm_runtime_put_autosuspend() to guarantee active clocks during MMIO
access and preserve PM reference counting.
- Restrict LPSS private register save/restore to LPT, BYT, and BSW
platforms via pxa2xx_spi_need_lpss_restore() (newer platforms are
handled by intel-lpss.c).
- Save only the first 6 LPSS private registers (offsets 0x00..0x14) in
drv_data during suspend, avoiding reserved offsets beyond 0x14.
- On resume, de-assert resets first, restore saved registers, call
lpss_ssp_setup(), and clear drv_data->suspended to prevent unclocked
IRQ access.
- Add error recovery paths for spi_controller_suspend/resume failures.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=108331
Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
drivers/spi/spi-pxa2xx.c | 94 +++++++++++++++++++++++++++++++++++++---
drivers/spi/spi-pxa2xx.h | 1 +
2 files changed, 89 insertions(+), 6 deletions(-)
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 443800bffbeb..864b9ca61bb2 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -194,6 +194,18 @@ static bool is_lpss_ssp(const struct driver_data *drv_data)
}
}
+static bool pxa2xx_spi_need_lpss_restore(const struct driver_data *drv_data)
+{
+ switch (drv_data->ssp_type) {
+ case LPSS_LPT_SSP:
+ case LPSS_BYT_SSP:
+ case LPSS_BSW_SSP:
+ return true;
+ default:
+ return false;
+ }
+}
+
static bool is_quark_x1000_ssp(const struct driver_data *drv_data)
{
return drv_data->ssp_type == QUARK_X1000_SSP;
@@ -1532,16 +1544,46 @@ static int pxa2xx_spi_suspend(struct device *dev)
struct ssp_device *ssp = drv_data->ssp;
int status;
- status = spi_controller_suspend(drv_data->controller);
- if (status)
+ status = pm_runtime_resume_and_get(dev);
+ if (status < 0)
return status;
+ status = spi_controller_suspend(drv_data->controller);
+ if (status) {
+ spi_controller_resume(drv_data->controller);
+ goto out_put;
+ }
+
+ /* Disable SSP interrupt generation on hardware level while clock is active */
pxa_ssp_disable(ssp);
+
+ /* Mark as suspended and synchronize IRQ before disabling clock */
drv_data->suspended = true;
synchronize_irq(ssp->irq);
+ if (pxa2xx_spi_need_lpss_restore(drv_data)) {
+ unsigned int i;
+
+ /*
+ * Save the first 6 LPSS private registers (offsets 0x00 to 0x14)
+ * while the clock is still enabled. They are lost when the LPSS
+ * power domain is removed across S3 and must be restored on resume.
+ * Use drv_data->lpss_base so the correct per-platform offset
+ * is applied regardless of LPSS IP revision.
+ * Registers beyond 0x14 (except CS control at 0x18) are reserved
+ * or unimplemented on LPT, and accessing them triggers a PCIe
+ * Completion Timeout causing a system halt.
+ */
+ for (i = 0; i < 6; i++)
+ drv_data->lpss_priv_ctx[i] = readl(drv_data->lpss_base + i * 4);
+ }
+
pxa2xx_spi_clk_disable(drv_data);
return 0;
+
+out_put:
+ pm_runtime_put_noidle(dev);
+ return status;
}
static int pxa2xx_spi_resume(struct device *dev)
@@ -1554,12 +1596,47 @@ static int pxa2xx_spi_resume(struct device *dev)
if (!pm_runtime_suspended(dev)) {
status = pxa2xx_spi_clk_enable(drv_data);
if (status)
- return status;
+ goto out_put;
}
- if (is_lpss_ssp(drv_data))
+ if (pxa2xx_spi_need_lpss_restore(drv_data)) {
+ unsigned int i;
+
+ /*
+ * The LPSS power domain is removed across S3, taking
+ * all private registers with it. De-assert the
+ * functional block and IDMA resets first; any MMIO
+ * access while the block is held in reset causes a
+ * PCIe Completion Timeout and a watchdog-triggered
+ * system reset.
+ */
+ writel(LPSS_PRIV_RESETS_FUNC | LPSS_PRIV_RESETS_IDMA,
+ drv_data->lpss_base + LPSS_PRIV_RESETS);
+
+ /* Restore the other 5 saved private registers */
+ for (i = 0; i < 6; i++) {
+ if (i == LPSS_PRIV_RESETS / 4)
+ continue;
+ writel(drv_data->lpss_priv_ctx[i],
+ drv_data->lpss_base + i * 4);
+ }
+ }
+
+ if (is_lpss_ssp(drv_data)) {
+ /*
+ * Re-initialise the SW chip-select control register so
+ * CS starts deasserted (SW_MODE | CS_HIGH), regardless
+ * of the state it was in at suspend time. A stale
+ * asserted CS on the first post-resume transaction
+ * corrupts the write-status response from the device.
+ */
lpss_ssp_setup(drv_data);
+ }
+ /*
+ * Now that resets are de-asserted and registers are restored,
+ * it is safe to handle interrupts.
+ */
drv_data->suspended = false;
/* Start the queue running */
@@ -1568,10 +1645,15 @@ static int pxa2xx_spi_resume(struct device *dev)
drv_data->suspended = true;
synchronize_irq(ssp->irq);
pxa2xx_spi_clk_disable(drv_data);
- return status;
+ goto out_put;
}
- return 0;
+out_put:
+ /* Let runtime PM autosuspend again if needed */
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
+
+ return status;
}
static int pxa2xx_spi_runtime_suspend(struct device *dev)
diff --git a/drivers/spi/spi-pxa2xx.h b/drivers/spi/spi-pxa2xx.h
index 44f37bf9c519..48169494f74e 100644
--- a/drivers/spi/spi-pxa2xx.h
+++ b/drivers/spi/spi-pxa2xx.h
@@ -71,6 +71,7 @@ struct driver_data {
irqreturn_t (*transfer_handler)(struct driver_data *drv_data);
void __iomem *lpss_base;
+ u32 lpss_priv_ctx[6];
bool suspended;
bool clk_enabled;
--
2.39.5
prev parent reply other threads:[~2026-07-18 16:07 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-18 16:06 [PATCH v14 0/3] spi: pxa2xx: MacBook8,1 quirk, runtime PM, and LPSS S3 resume state fixes Shih-Yuan Lee
2026-07-18 16:07 ` [PATCH v14 1/3] spi: pxa2xx: disable DMA for Apple MacBook8,1 Shih-Yuan Lee
2026-07-18 20:01 ` Mark Brown
2026-07-18 16:07 ` [PATCH v14 2/3] spi: pxa2xx: fix runtime PM and interrupt handling in PIO mode Shih-Yuan Lee
2026-07-18 20:14 ` Mark Brown
2026-07-18 16:07 ` Shih-Yuan Lee [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=20260718160702.7861-4-fourdollars@debian.org \
--to=fourdollars@debian.org \
--cc=broonie@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-spi@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox