Re: [PATCH 1/7] ASoC: soc-component: use dapm->component instead of container_of()

[Cezary Rojewski <cezary.rojewski@intel.com>]

On 2025-04-11 4:04 AM, Kuninori Morimoto wrote:
> 
> Hi Cezary, again
> 
>>> I'd rather focus on 'struct snd_soc_dapm_context' exclusively. A valid
>>> pointer to this type is required for many functions found in
>>> include/sound/ yet the struct is more of framework-internal entity.
>>> Strict separation between dapm-context "for the framework" from
>>> dapm-context "for the user" would grant better readability benefits.
> 
> I have checked each drivers which is using struct snd_soc_dapm_context.
> It seems not only framework but many user drivers need to use/access to it.
> 
> So separation seems impossible, and dropping wrapper is maybe not good idea.
> But I can understand that it will be very longer code :)
> 
> 	x = dapm->component;
> 	x = snd_soc_dapm_to_component(dapm);
> 
> For me personally, wrapper function/macro is not so bad idea.
> It is much safer to use function/macro rather than touching them directly.
> 
> Anyway, I will try to keep current patch-set style.
> But any comment is very welcome.
> Thank you for your help !!


I see two replies, hope it's fine if I just reply to this one. I love 
the enthusiasm though :D

What I meant is that even the name of the type - 'dapm_context' - speaks 
of framework-internal stuff. The fields that define that struct also are 
not for the users (drivers) really. Perhaps there is a way to get rid of 
the dapm_context pointer entirely in the argument lists of public 
functions found in include/sound/. The pointer could be obtained e.g.: 
via container_of() by the framework's internal handler when needed.

Otherwise, I do not see real benefit of using 'x = 
snd_soc_dapm_to_component()' over 'x = dapm->component'. In my opinion 
there's no need to provide wrapper for every field we have. No hard 
blocks though.


Kind regards,
Czarek