Re: [PATCH 5/7] ASoC: SDCA: Add basic system suspend support

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

On Tue, Dec 09, 2025 at 12:11:27PM +0000, Pierre-Louis Bossart wrote:
> On 11/25/25 15:21, Charles Keepax wrote:
> > Add basic system suspend support. Disable the IRQs and force runtime
> > suspend, during system suspend, because the device will likely fully
> > power down during suspend.
> 
> Power-down during system suspend (seems natural) or power-down
> during pm_runtime suspend (depends on what the host does during
> clock_stop)?

At the moment this is written to support FDL after system
suspend only. Which is fairly typical of most devices I have seen
(Cirrus and non-Cirrus). Generally as runtime suspends happen
so often it is preferred not to download firmware there due to
time constraints.

This is definitely up for debate, my primary issue with allowing
firmware download at the runtime level is that SDCA gives you no
way to tell that the device is ready to rock. The only way I have
been able to divine to do this is to wait for an FDL irq and if
one doesn't come within a reasonable time out move on. However,
that waiting would add a considerable delay to runtime resume
even if no firmware was downloaded, which feels problematic.

I guess my two questions would be:
1) Do we really want to support downloading firmware on runtime
   suspend? I am doubtful it is really usable due to latency.
2) If we do, do you have any ideas about how to determine if the
   device needs firmware?

> > +	if (drv->suspended) {
> > +		sdca_irq_enable(drv->function, drv->core->irq_info, true);
> > +		sdca_irq_enable(drv->function, drv->core->irq_info, false);
> 
> and a comment here wouldn't hurt, not sure about the side/racy
> effects of turning the interrupt on before turning it off?

This is perhaps a little misleading the last parameter here is an
early flag not an enable/disable. I could perhaps replace that
with some defines rather than a bool to keep the calls more
self-documenting.

Thanks,
Charles