Re: [PATCH v16 3/7] spi: pxa2xx: overhaul teardown and suspend sequence using pxa2xx_spi_off

[Shih-Yuan Lee (FourDollars)]

On Tue, Jul 21, 2026 at 3:53 AM Andy Shevchenko
<andriy.shevchenko@xxxxxxxxx> wrote:
>
> On Tue, Jul 21, 2026 at 12:21:12AM +0800, Shih-Yuan Lee wrote:
> > When removing the driver or suspending the device, the clock must not
> > be disabled while shared interrupts are still active. Gating the clock
> > before waiting for in-flight interrupt handlers to complete results
> > in race conditions where the handler performs unclocked MMIO accesses,
> > causing PCIe Completion Timeouts.
> >
> > Overhaul the remove, suspend, and runtime_suspend paths to use a strict
> > synchronized teardown order:
> > 1. Disable hardware interrupt generation at the controller level.
> > 2. Mark the device state as suspended (suspended = true) to prevent
> >    subsequent interrupt handlers from attempting MMIO reads.
> > 3. Call synchronize_irq() to wait for any active interrupt handlers
> >    to drain completely.
> > 4. Gate the clock via pxa2xx_spi_clk_disable().
> >
> > Additionally, commit 29d7e05c5f75 ("spi: pxa2xx: Avoid touching
> > SSCR0_SSE on MMP2") documented that disabling the hardware block via SSE on
> > MMP2 SoC platforms corrupts the RX/TX FIFO. Instead of calling
> > pxa_ssp_disable() directly, use the helper function pxa2xx_spi_off(),
> > which respects the MMP2 platform quirk by bypassing SSE register writes.
>
> ...
>
> > +     /* Wait for any pending interrupt handlers to complete */
> > +     synchronize_irq(ssp->irq);
>
> Unneeded. free_irq() implies that.
Hi Andy,

Thanks for pointing this out.

Good catch. Since free_irq() internally calls synchronize_irq() before
releasing the IRQ, calling synchronize_irq() right before free_irq()
in pxa2xx_spi_remove() is indeed redundant.

I will remove the explicit synchronize_irq() call from
pxa2xx_spi_remove() for the next version, while retaining it in the
suspend() and runtime_suspend() paths where free_irq() is not called.

Thanks,
Shih-Yuan