Re: [PATCH 02/15] ASoC: SDCA: Add manual PM runtime gets to IRQ handlers

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

On 9/8/25 15:31, Charles Keepax wrote:
> On Mon, Sep 08, 2025 at 01:42:40PM +0200, Pierre-Louis Bossart wrote:
>> On 9/5/25 16:31, Charles Keepax wrote:
>>> @@ -86,18 +88,25 @@ static irqreturn_t function_status_handler(int irq, void *data)
>>>  {
>>>  	struct sdca_interrupt *interrupt = data;
>>>  	struct device *dev = interrupt->component->dev;
>>> +	irqreturn_t irqret = IRQ_NONE;
>>>  	unsigned int reg, val;
>>>  	unsigned long status;
>>>  	unsigned int mask;
>>>  	int ret;
>>>  
>>> +	ret = pm_runtime_get_sync(dev);
>>
>> what does 'dev' represent? The SDCA function device or the
>> parent SoundWire peripheral device?
> 
> SDCA function device here.
> 
>> I would think it's the former, with the parent-child
>> relationship used by the runtime_pm framework to resume the
>> parent peripheral device if needed.
> 
> Yes this would cause the parent to power up, but there is
> nuance here. The parent has actually already been powered up
> by regmap IRQ. The thing here is SDCA is weird, the interrupts
> are defined at the device level, but all the handling is at the
> function level.
> 
> To some extent this is also a consequence of having a regmap per
> function. Regmap IRQ is registered against the SoundWire device,
> as it needs to access the device level IRQ registers. But the
> handlers are per function and need to make sure an individual
> functions register map is out of cache_only so need to do a
> runtime get on the function specifically. In the past with
> multi-function codecs we have been able to cheat a little here
> as resuming the parent actually did everything necessary to
> communicate with a part of the device.

ok, makes sense.

Capturing some of this paragraph in the commit message would be good to explain what resume operations mean for an SDCA child device. 
>> maybe use sdev and pdev as an arbitrary convention to help the reader?
>>
> 
> I quite like the idea of this as a naming convention, will have
> a look at updating stuff.
dev and sdev are fine as well, as suggested in the other email.