On 6/17/22 04:44, Mark Brown wrote:
On Thu, Jun 16, 2022 at 04:08:25PM -0500, Pierre-Louis Bossart wrote:
Make sure that the bus and codecs are pm_runtime active when the card is registered/created. This avoid timeouts when accessing registers.
+static int max98373_sdw_probe(struct snd_soc_component *component) +{
- int ret;
- ret = pm_runtime_resume(component->dev);
- if (ret < 0 && ret != -EACCES)
return ret;
I'm not clear what the issue is here. Is something that's accessing the registers forgetting to do a pm_runtime_get(), or doing that rather than using pm_runtime_get_sync()? This doesn't feel safe or robust.
The context is that I have been trying to remove all timing dependencies between components, and make sure that you can bind/unbind drivers in any order, with the deferred probe making sure that all required components are already probed. I started this after seeing reports of kernel oopses when the machine driver was removed, and realizing that the SoundWire bus itself didn't support bind/unbind tests by design.
In the case where you bind the machine driver after a delay, then the bus might be suspended already, and there are cases where we see timeouts for registers that are not regmap-managed (usually vendor-specific stuff with an indirection mechanism), and even for regmap the register read-write are cache-based when the bus is suspended.
What this patch does it make sure that the bus is operation when the card is created. In usual cases, this is a no-op, this just helps with corner test cases. It's not plugging a major hole in the pm_runtime support, just fixing a programming sequence that was not tested before.
One possible objection is that we don't keep the reference and the bus active until all components are probed. I tried doing this at the ASoC core level, but that breaks all kinds of devices that have their own quirky way of dealing with pm_runtime - specifically HDaudio and HDMI. That's why I added this resume here.
Makes sense?