Re: [PATCH 2/3] ASoC: core: Inline resume work back to resume function

From: Cezary Rojewski <cezary.rojewski@intel.com>
To: Mark Brown <broonie@kernel.org>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: hdegoede@redhat.com, alsa-devel@alsa-project.org,
	amadeuszx.slawinski@linux.intel.com, tiwai@suse.com
Subject: Re: [PATCH 2/3] ASoC: core: Inline resume work back to resume function
Date: Mon, 7 Nov 2022 10:26:14 +0100	[thread overview]
Message-ID: <3ebce110-1a76-e59b-2172-616a3c0f1984@intel.com> (raw)
In-Reply-To: <Y2WmRUBJ+7K5zBm7@sirena.org.uk>

On 2022-11-05 12:54 AM, Mark Brown wrote:
> On Fri, Nov 04, 2022 at 09:58:46AM -0400, Pierre-Louis Bossart wrote:

>> If I follow your logic, we should also remove the workqueue used for
>> probes for HDaudio devices, on the grounds that probe errors are not
>> propagated either.

(save)

>> Any time we have deferred processing to avoid blocking the rest of the
>> system, we incur the risk of not having errors propagated. It's a
>> compromise between having a system that's usable and a system that's
>> consistent.

> The other question is what we'd constructively do about a resume failure
> that we can't defer.  It feels like we should at least retain the
> ability to defer for devices where this is an issue (older components
> tend to be cheap and packaged in easier to assemble packaging and hence
> get used with lower end applications even well after they're no longer
> competitive at the high end), and if we are going to return some errors
> in line it'd be good to understand the benefits and tradeoffs.  I do see
> that it is a lot less useful for modern devices where we don't have to
> have any delays in startup, though like I say register I/O on slower
> buses like I2C could still be a concern.
> 
> I'm not keen on moving the support out of the core since there were
> originally a bunch of devices trying to open code and it wasn't good,
> both from a duplication/complexity point of view and from the point of
> view of integrating well with userspace APIs.

I believe that framework should be supporting both, the deferred and the 
instant resume options. 'void' in front of suspend/resume in ASoC 
hinders developer's options.

(load)
The HDAudio driver is actually a good example of how to do it right - we 
did not modify driver/base/ to have ->probe() return void. It remained 
as is, instead, a developer opt-ins for a delayed probe through a 
workqueue. This way, everyone is satisfied.
Cohesiveness is not to be forgotten too - keeping behavior and 
expectations of the standard set of functionalities aligned with the 
rest of the driver/base makes it easier to hop into ASoC.

We could provide some additional flags so that the ASoC core always 
defers PM-related work for certain components if they choose to.

Regards,
Czarek