Re: [PATCH v2 3/3] soundwire: intel_auxdevice: Don't disable IRQs before removing children

[Charles Keepax <ckeepax@opensource.cirrus.com>]

On Tue, Sep 29, 2026 at 10:34:16AM +0200, Pierre-Louis Bossart wrote:
> On 9/28/26 10:39, Charles Keepax wrote:
> > On Sat, Sep 26, 2026 at 05:55:06PM +0200, Pierre-Louis Bossart wrote:
> >>  void sdw_bus_master_delete(struct sdw_bus *bus)
> >>  {
> >> -	device_for_each_child(bus->dev, NULL, sdw_delete_slave);
> >> +	sdw_bus_slaves_delete(bus); <<< this would be the second call?
> >> +	sdw_bus_slaves_put(bus);
> > 
> > Yeah seemed the simplest way to not have to change any existing
> > drivers, the second call is a complete no-op if the first was
> > done.
>
> humm, that seems to work but I get this layering violation after-taste,
> and I wonder if this works for AMD?

Nothing should have changed from AMD's perspective. I do think
perhaps a slightly better fix could be done moving significant
amounts of the IRQ handling over to the IRQ framework,
however that is a much bigger piece of work and more scary
from a regressions point of view due to it being a much bigger
refactoring.

> AMD use a platform device instead of an auxiliary one, but overall this
> looks like the same problem of disabling interrupts before the
> peripheral driver is removed, no?
> 
> static void amd_sdw_manager_remove(struct platform_device *pdev)
> {
> 	struct amd_sdw_manager *amd_manager = dev_get_drvdata(&pdev->dev);
> 	int ret;
> 
> 	pm_runtime_disable(&pdev->dev);
> 	cancel_work_sync(&amd_manager->amd_sdw_work);
> 	amd_disable_sdw_interrupts(amd_manager); <<< SAME PROBLEM ??
> 	sdw_bus_master_delete(&amd_manager->bus);
> 
> If this is indeed the same problem then the AMD driver should use the
> same solution...

I am not super familiar with the AMD stuff, so take this with a
massive pinch of salt but it depends on if that IRQ is actually
involved in SoundWire command transfers.

A quick look through amd_sdw_irq_thread() and amd_sdw_xfer_msg()
looks to me like the IRQ is not involved there. So I believe
the AMD host doesn't have the same problem as the Intel one.
Although there is perhaps debate if SoundWire alerts should
still function during remove, which would be broken on AMD,
but I am not aware of any devices that need that right now.

Thanks,
Charles