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.