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

[Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>]

On 9/28/26 10:39, Charles Keepax wrote:
> On Sat, Sep 26, 2026 at 05:55:06PM +0200, Pierre-Louis Bossart wrote:
>>
>>> diff --git a/drivers/soundwire/intel_auxdevice.c b/drivers/soundwire/intel_auxdevice.c
>>> index 901a71262094f..1439cef43462d 100644
>>> --- a/drivers/soundwire/intel_auxdevice.c
>>> +++ b/drivers/soundwire/intel_auxdevice.c
>>> @@ -508,8 +508,13 @@ static void intel_link_remove(struct auxiliary_device *auxdev)
>>>  	if (!bus->prop.hw_disabled) {
>>>  		sdw_intel_debugfs_exit(sdw);
>>>  		cancel_delayed_work_sync(&cdns->attach_dwork);
>>> -		sdw_cdns_enable_interrupt(cdns, false);
>>>  	}
>>> +
>>> +	sdw_bus_slaves_delete(bus);
>>> +
>>> +	if (!bus->prop.hw_disabled)
>>> +		sdw_cdns_enable_interrupt(cdns, false);
>>> +
>>>  	sdw_bus_master_delete(bus);
>>>  }
>>
>> sorry, not following - this sequence seems to rely on *two* calls to
>> sdw_bus_slaves_delete(), is this intentional or I am missing something?
>>
>>
>>  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?

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...