Re: [PATCH v2 2/4] ASoC: SDCA: Add basic system suspend support

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

On 1/7/26 11:04, Charles Keepax wrote:
> On Tue, Jan 06, 2026 at 06:23:17PM +0100, Pierre-Louis Bossart wrote:
>>
>>> +static int class_suspend(struct device *dev)
>>> +{
>>> +	struct sdca_class_drv *drv = dev_get_drvdata(dev);
>>> +	int ret;
>>> +
>>> +	disable_irq(drv->sdw->irq);
>>> +
>>> +	ret = pm_runtime_force_suspend(dev);
>>> +	if (ret) {
>>> +		dev_err(dev, "Failed to force suspend: %d\n", ret);
>>
>> suggestion: since this is going to be fairly generic code
>> used by multiple devices, it'd be useful to add a device
>> identifier or string to the error log, no?
> 
> Doesn't the dev_err already do that, it prints the device name in
> the log?

Yes but the device name could be confusing, not everyone can figure out what "sdw:0:0:025d:0711:01:" means

>>> +	if (drv->suspended) {
>>> +		sdca_irq_enable_early(drv->function, drv->core->irq_info);
>>> +		sdca_irq_enable(drv->function, drv->core->irq_info);
>>
>> maybe add a comment on why the two calls are done
>> sequentially. You did mention it somewhere, it'd be good to
>> capture the design.
> 
> Yeah can do, its a little bit of one of these intermediate step
> things. Becomes come clear in the next patch.

ok