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

[Pierre-Louis Bossart]

On 9/29/26 14:40, Charles Keepax wrote:
> 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.
I think you're right, the AMD messenging relies on polling, not on
interrupts, so they are 'safe'. I don't see anything that would prevent
communication with the peripheral.

For the alerts, I guess it's the same on remove, clock stop and system
suspend. When the manager made the decision to remove/stop the
clock/power-down, the transition has to happen. Alerts can only be
handled 'later'.