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

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

On 9/5/25 16:31, Charles Keepax wrote:
> SDCA interrupt handling is a bit odd, the SDCA IRQ registers are
> defined on a device level but the handling of the IRQ is at the
> Function level. As such the regmap IRQ's PM runtime operations need to
> be on the device itself to ensure those registers are available but an
> additional runtime get is required for the Function child when the IRQ
> is actually handled. Add the required manual PM runtime gets.
> 
> Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> ---
>  sound/soc/sdca/sdca_interrupts.c | 42 ++++++++++++++++++++++++--------
>  1 file changed, 32 insertions(+), 10 deletions(-)
> 
> diff --git a/sound/soc/sdca/sdca_interrupts.c b/sound/soc/sdca/sdca_interrupts.c
> index 79bf3042f57d4..6775f0f7b5659 100644
> --- a/sound/soc/sdca/sdca_interrupts.c
> +++ b/sound/soc/sdca/sdca_interrupts.c
> @@ -11,7 +11,9 @@
>  #include <linux/bits.h>
>  #include <linux/cleanup.h>
>  #include <linux/device.h>
> +#include <linux/dev_printk.h>
>  #include <linux/interrupt.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/regmap.h>
>  #include <linux/soundwire/sdw.h>
>  #include <linux/soundwire/sdw_registers.h>
> @@ -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?

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.

maybe use sdev and pdev as an arbitrary convention to help the reader?

> +	if (ret < 0) {
> +		dev_err(dev, "failed to resume for function status: %d\n", ret);
> +		goto error;
> +	}
> +
>  	reg = SDW_SDCA_CTL(interrupt->function->desc->adr, interrupt->entity->id,
>  			   interrupt->control->sel, 0);
>  
>  	ret = regmap_read(interrupt->component->regmap, reg, &val);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to read function status: %d\n", ret);
> -		return IRQ_NONE;
> +		goto error;
>  	}
>  
>  	dev_dbg(dev, "function status: %#x\n", val);
> @@ -130,10 +139,13 @@ static irqreturn_t function_status_handler(int irq, void *data)
>  	ret = regmap_write(interrupt->component->regmap, reg, val);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to clear function status: %d\n", ret);
> -		return IRQ_NONE;
> +		goto error;
>  	}
>  
> -	return IRQ_HANDLED;
> +	irqret = IRQ_HANDLED;
> +error:
> +	pm_runtime_put(dev);
> +	return irqret;
>  }