Re: [PATCH 16/16] ASoC: SOF: amd: add system and runtime PM ops for ACP7x
"Mukunda,Vijendar" <[email protected]> Fri, 3 Jul 2026 23:21:21 +0530
| Newsgroups | org.alsa-project.alsa-devel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-sound |
|---|---|
| Message-ID | <[email protected]> |
On 7/3/26 22:39, Mark Brown wrote: > On Fri, Jul 03, 2026 at 10:21:25PM +0530, Mukunda,Vijendar wrote: >> On 7/3/26 21:58, Mark Brown wrote: >>> On Wed, Jul 01, 2026 at 03:25:17PM +0530, Vijendar Mukunda wrote: >>> The interrupt is requested with IRQF_SHARED so probably worth double >>> checking if the interrupt handler is safe to run while the suspend and >>> resume callbacks are running, I see we reset the device during suspend >>> so there might be some risk of corrupted register reads? >> There won't be any corrupted register reads when acp reset sequence >> is executed, all the acp registers are set to default values. i.e Interrupt >> control registers are disabled. This sequence ensures that till interrupt > Including whatever the threaded handler is doing? I didn't actually > check properly, just saw the indirection through the SOF I/O functions > and the threaded handler. |The threaded handler (acp_sof_ipc_irq_thread) can only be scheduled when acp7x_irq_handler returns IRQ_WAKE_THREAD, which only happens when ACP_DSP_TO_HOST_IRQ is set in ACP_DSP_SW_INTR_STAT. By the time the platform suspend callback runs, the SOF core has already quiesced the DSP (ctx_save IPC, pipeline teardown), so no new DSP-to-host interrupt can arrive. The subsequent acp_reset() then zeroes all interrupt-enable registers, preventing any further IRQ_WAKE_THREAD from acp7x_irq_handler. If a thread was already queued before reset completes, it runs acp_sof_ipc_irq_thread which reads the scratch SRAM mailbox registers (dsp_msg_write, dsp_ack_write). After a soft reset those registers read 0, so the handler finds nothing to do and returns IRQ_HANDLED cleanly.| >> masks are enabled during resume sequence, no interrupt will be asserted >> for ACP IP. > Right, but IRQF_SHARED means it might be an interrupt for something > else. Correct. If another device asserts the shared INTx line during suspend, the kernel calls acp7x_irq_handler. In that case both ACP_DSP_SW_INTR_STAT and ACP_EXT_INTR_STAT read 0 (device is reset/idle), so the handler returns IRQ_NONE immediately without scheduling the thread or touching any other registers. No corrupted reads or harmful side effects occur. In practice the ACP PCI device on ACP7.B/7.F has the INTx line assigned exclusively to itself — we verified on target that no other device shares it. IRQF_SHARED is set solely because the kernel's PCI layer requires it for INTx interrupts; omitting it causes request_threaded_irq to fail even when the line is unshared. We can drop IRQF_SHARED flag, if that is preferred. Please let us know.