# Re: \[PATCH v2 02/11\] ASoC: SDCA: export PM helpers keyed on sdca\_class\_drv

- From: Srinivas Kandagatla \<hidden\>
- Date: 2026-09-07 09:43:42 
- List: linux-devicetree
- Page: https://ratatoskr.run/linux-devicetree/2026/09/17524002
- Thread: https://ratatoskr.run/sashiko-reviews/2026/09/17524002/t.md

```text
On 9/7/26 9:50 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [Low] Exporting `sdca_class_pm_ops` unconditionally with `RUNTIME_PM_OPS` causes dead code retention when CONFIG_PM is disabled.
> 
> Pre-existing issues:
> - [High] Data race and invalid hardware access between `boot_work` and system suspend.
> --
> 
> commit 0f2e2cef51e8b87316164c383139022824389918
> Author: Srinivas Kandagatla [ref]
> 
> ASoC: SDCA: export PM helpers keyed on sdca_class_drv
> 
> The class PM callbacks pull sdca_class_drv out of drvdata, so the
> built-in class_sdw_driver owns the drvdata slot. That works for the
> generic case but blocks codec drivers that want to embed sdca_class_drv
> in their own private struct. Split the four callbacks into exported
> helpers that take a struct sdca_class_drv * directly.
> 
>> diff --git a/sound/soc/sdca/sdca_class.c b/sound/soc/sdca/sdca_class.c
>> index d7444f442c71c..88a29116a334e 100644
>> --- a/sound/soc/sdca/sdca_class.c
>> +++ b/sound/soc/sdca/sdca_class.c
> [ ... ]
>> -static int class_suspend(struct device *dev)
>> +/**
>> + * sdca_class_system_suspend - SDCA class system suspend helper
>> + * @drv: caller-owned sdca_class_drv.
>> + *
>> + * Codec drivers compose this into their own dev_pm_ops.  Disables the
>> + * SoundWire interrupt and forces runtime suspend of the underlying
>> + * class regmap.
>> + */
>> +int sdca_class_system_suspend(struct sdca_class_drv *drv)
>>  {
>> -	struct sdca_class_drv *drv = dev_get_drvdata(dev);
>>  	int ret;
>>  
>>  	disable_irq(drv->sdw->irq);
>>  
>> -	ret = pm_runtime_force_suspend(dev);
>> +	ret = pm_runtime_force_suspend(drv->dev);
>>  	if (ret) {
>> -		dev_err(dev, "failed to force suspend: %d\n", ret);
>> +		dev_err(drv->dev, "failed to force suspend: %d\n", ret);
>>  		return ret;
>>  	}
>>  
>>  	return 0;
>>  }
>> +EXPORT_SYMBOL_NS_GPL(sdca_class_system_suspend, "SND_SOC_SDCA_CLASS");
> 
> [Severity: High]
> This is a pre-existing issue, but does sdca_class_system_suspend() fail to
> serialize against boot_work?
> 
> Because boot_work is queued on system_long_wq (an unbounded, non-freezable
> workqueue), it can race with system suspend. If the system initiates suspend
> while boot_work is waiting in sdw_slave_wait_for_init(),
> sdca_class_system_suspend() will force the device into a suspended state by
> disabling interrupts and setting regcache_cache_only(true).
> 
> Since sdca_class_system_suspend() does not flush or cancel the pending
> initialization work, boot_work will eventually resume, blindly set
> regcache_cache_only(false), and attempt hardware access on a physically
> suspended bus, which could lead to bus timeouts or crashes.

This is existing issue, it should not be necessary for Tambora support
series to address these issue.

> 
> [ ... ]
>> +static int class_pm_runtime_resume(struct device *dev)
>> +{
>> +	return sdca_class_runtime_resume(dev_get_drvdata(dev));
>> +}
>>  
>> -static const struct dev_pm_ops class_pm_ops = {
>> -	SYSTEM_SLEEP_PM_OPS(class_suspend, class_resume)
>> -	RUNTIME_PM_OPS(class_runtime_suspend, class_runtime_resume, NULL)
>> +const struct dev_pm_ops sdca_class_pm_ops = {
>> +	SYSTEM_SLEEP_PM_OPS(class_pm_system_suspend, class_pm_system_resume)
>> +	RUNTIME_PM_OPS(class_pm_runtime_suspend, class_pm_runtime_resume, NULL)
>>  };
>> +EXPORT_SYMBOL_NS_GPL(sdca_class_pm_ops, "SND_SOC_SDCA_CLASS");
> 
> [Severity: Low]
> Does exporting sdca_class_pm_ops unconditionally with RUNTIME_PM_OPS cause
> dead code retention when CONFIG_PM is disabled?
> 
> When CONFIG_PM is disabled, runtime PM callbacks are never used and should be
> eliminated by the compiler. Because sdca_class_pm_ops is globally exported in
Yes, compiler should strip of the static unused functions.

> this patch and populated using the unconditional RUNTIME_PM_OPS macro instead
> of conditionally-compiled macros, the compiler will retain the structure and
> its referenced static functions (class_pm_runtime_suspend and
> class_pm_runtime_resume) as dead code in the final binary.
> 

```