Re: [PATCH] ASoC: amd: ps: fix snd_acp63_remove() teardown ordering

[Fan Wu]

On 9/25/26 13:35, Mukunda,Vijendar wrote:
> The fix open-codes the register offsets (ACP_EXTERNAL_INTR_STAT,
> ACP_EXTERNAL_INTR_CNTL, ACP_EXTERNAL_INTR_ENB) directly in
> snd_acp63_remove(). Future platforms may have different interrupt
> control register offsets, which would require changes in this remove
> path as well. Consider adding a disable_interrupts callback to struct
> acp_hw_ops and invoking it here instead. This keeps the remove path
> platform-agnostic and the interrupt masking logic co-located with its
> platform-specific counterpart in ps-common.c.

Thank you for the review. Agreed — I have reworked the patch exactly
along these lines for v2:

- struct acp_hw_ops gains a disable_interrupts callback plus an
  acp_hw_disable_interrupts() wrapper, following the interrupt-control
  ops pattern of the acp family in amd.h (en_interrupts member and
  acp_disable_interrupts() helper).
- acp63_hw_init_ops() and acp70_hw_init_ops() wire it to the existing
  static acp63_disable_interrupts()/acp70_disable_interrupts() helpers
  in ps-common.c, so the masking sits next to its platform-specific
  counterparts.
- snd_acp63_remove() now calls acp_hw_disable_interrupts() followed by
  devm_free_irq() before the first child device is unregistered; the
  register writes are gone from the remove path.

The MMIO sequence is unchanged (clear STAT, CNTL = 0, ENB = 0 before
freeing the shared IRQ), and the Fixes tag stays on eaf825037d6d. I
will send v2 as a new thread with a link to this version.

Separately, I noticed your "[PATCH V2 0/9] soundwire: amd: SoundWire
manager driver bug fixes" series: patch 3/9, adding the amd_sdw_irq_thread
drain in amd_sdw_manager_remove(), is exactly the soundwire-side companion
this fix needs. The v2 commit message now notes that the manager-side
drain is handled separately, and with both in place the chain is closed
from the ACP hardirq down to the manager work.

Best regards,
Fan Wu