Re: [PATCH 2/2] ASoC: codecs: soundwire: call pm_runtime_resume() in component probe

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

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?