Hi, Mark,
On 10/29/25 16:37, Mark Brown wrote:
On Wed, Oct 29, 2025 at 04:11:33PM +0200, Claudiu wrote:
Since snd_soc_suspend() is invoked through snd_soc_pm_ops->suspend(), and snd_soc_pm_ops is associated with the soc_driver (defined in sound/soc/soc-core.c), and there is no parent-child relationship between the soc_driver and the DA7213 codec driver, the power management subsystem does not enforce a specific suspend/resume order between the DA7213 driver and the soc_driver.
The theory here is that the power management core uses the device instantiation order for both suspend and resume (reversed on suspend) so the fact that we use probe deferral to make sure that the card components are ready should ensure that the card suspends before anything in the card. If that is no longer the case then we need to ensure that all drivers have system PM ops which trigger the card, this won't be a driver specific issue.
I also saw the behavior described in this commit with the rz-ssi.c driver as well. The fix there was commit c1b0f5183a44 ("ASoC: renesas: rz-ssi: Use NOIRQ_SYSTEM_SLEEP_PM_OPS()").
In case of this this codec, I saw the code in da7213_runtime_resume() and soc_resume_deferred() racing each other on system resume.
static int da7213_runtime_resume(struct device *dev) { struct da7213_priv *da7213 = dev_get_drvdata(dev);
- int ret;
- ret = regulator_bulk_enable(DA7213_NUM_SUPPLIES, da7213->supplies);
- if (ret < 0)
return ret;- regcache_cache_only(da7213->regmap, false);
- return regcache_sync(da7213->regmap);
- return regulator_bulk_enable(DA7213_NUM_SUPPLIES, da7213->supplies); }
This seems obviously buggy, we just power on the device and don't sync the register state.
You're right! I'll revisit this.
If the device actually lost power during a runtime suspend then we'll end up having a bad time. There was also no mention of runtime PM in the patch description...
I had no issues with runtime PM, but only with suspend to RAM, when this function was called though struct dev_pm_ops::resume = pm_runtime_force_resume().
Would keeping the regcache_cache_only() + regcache_sync() here along with populating the struct snd_soc_component_driver::{suspend, resume} be an acceptable solution for you? I think that will work as well.
Thank you for your review, Claudiu