Re: [PATCH 0/3] ASoC: core: Two step component registration - Lars-Peter Clausen

From: Lars-Peter Clausen <lars@metafoo.de>
To: Cezary Rojewski <cezary.rojewski@intel.com>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: alsa-devel@alsa-project.org, olivier.moysan@st.com,
	alexandre.torgue@st.com, lgirdwood@gmail.com,
	arnaud.pouliquen@st.com, tiwai@suse.com, broonie@kernel.org,
	mcoquelin.stm32@gmail.com
Subject: Re: [PATCH 0/3] ASoC: core: Two step component registration
Date: Fri, 31 Jul 2020 17:58:00 +0200	[thread overview]
Message-ID: <a6d3e9fb-4aa2-b75b-4535-037edb256112@metafoo.de> (raw)
In-Reply-To: <61faef6e-70e9-5574-5f3a-e333d0a71617@intel.com>

On 7/31/20 5:47 PM, Cezary Rojewski wrote:
> On 2020-07-31 5:07 PM, Pierre-Louis Bossart wrote:
>> On 7/31/20 9:41 AM, Cezary Rojewski wrote:
>>> Provide a mechanism for true two-step component registration. This
>>> mimics device registration flow where initialization is the first step
>>> while addition goes as second in line. Drivers may choose to modify
>>> component's fields before registering component to ASoC subsystem via
>>> snd_soc_add_component.
>>
>> I must admit I don't see where this might be used for Intel 
>> platforms, we've been happily using snd_soc_register_component() 
>> without hitting limitations.
>
> Patchset targets entire ASoC framework, not Intel catalog. As 
> _initialize and _add are already in place, having a two-step 
> registration is cohesive with other "frameworks" e.g. device one.
>
> New to ASoC? Trying to learn soc-components? Guess what, 
> creation/registration procedure is exactly the same as one you're used 
> to already!
>
>> Also the only two uses of snd_soc_add_component() seem mainly driven 
>> by memory allocation - and avoiding a devm_kzalloc in 
>> snd_soc_register_component().
>
> In general, code quality and improvements to its flow should not 
> require ton of usages. But hey, you got two already.
>
>> Out of curiosity, can you provide an example where this two-step 
>> would be required or beneficial? Thanks!
>
> Overridding component->name which is currently always tied to 
> fmt_single_name so you may choose a different name than the ->dev one.

For what it is worth, I think this is a sensible change for the outlined 
reasons. It's something I've always had in the back of my mind, but 
there was never enough of a need to actually do it.

One change I'd like to see is the addition of snd_soc_component_alloc(), 
which combines the step of kzalloc() and snd_soc_component_init() as 
these will be done pretty much always in lockstep. And it also matches 
the APIs that other frameworks offer.

- Lars