Re: [PATCH v16 6/7] spi: pxa2xx: restore LPSS private register state on S3 resume

On Tue, Jul 21, 2026 at 12:21:15AM +0800, Shih-Yuan Lee wrote:

Is this series AI-assisted?

> 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_noidle() to guarantee active clocks during MMIO
>   access and preserve PM reference counting.
> - Restrict LPSS private register save/restore to LPT, BYT, and BSW

^^^^ (1)

>   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.
> - On the resume error path, call pm_runtime_set_suspended() before
>   pm_runtime_put_noidle() to align the PM runtime state with the
>   already-disabled hardware clock, preventing pxa2xx_spi_runtime_suspend()
>   from attempting unclocked MMIO via pxa2xx_spi_off().

This is an ugly hack.

Saving context is done in drivers/acpi/x86/lpss.c (see #1 why this file).
If something wrong in the flow it has to be fixed there, not here.

-- 
With Best Regards,
Andy Shevchenko