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

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

On 2025-04-09 4:57 AM, Kuninori Morimoto wrote:
> Now, snd_soc_dapm_to_component() (A) will convert dapm to component by
> container_of() (a).
> 
> (A)	static inline struct snd_soc_component *snd_soc_dapm_to_component(
> 		struct snd_soc_dapm_context *dapm)
> 	{
> (a)		return container_of(dapm, struct snd_soc_component, dapm);
> 	}
> 
> Because (a) uses container_of(), dapm of component works, but
> dapm of others will be "unknown" pointer (= not NULL).
> 
> OTOH, we will call snd_soc_dapm_init() (X) to initialize dapm, and
> it will be called from snd_soc_bind_card() (p) or
> soc_probe_component() (q) with pointer (component/NULL).
> 
> (p)	static int snd_soc_bind_card(...)
> 	{
> 		...
> (X)		snd_soc_dapm_init(..., NULL);
> 		...                    ^^^^
> 	}
> 
> (q)	static int soc_probe_component(...)
> 	{
> 		...
> (X)		snd_soc_dapm_init(, component);
> 		...                 ^^^^^^^^^
> 	}
> 
> And snd_soc_dapm_init() (X) will fill dapm->component (x)
> 
> (X)	void snd_soc_dapm_init(..., component, ...)
> 	{
> 		...
> (x)		dapm->component	= component;
> 		...
> 	}
> 
> We can simply use dapm->component in snd_soc_dapm_to_component() (A).
> In this case, dapm of others (p) will be just NULL.
> 
> Use dapm->component instead of container_of().
> The picky note can be removed by this patch.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Cc: Benjamin Bara <benjamin.bara@skidata.com>
> ---
>   include/sound/soc-component.h | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h
> index 61534ac0edd1..63928773df32 100644
> --- a/include/sound/soc-component.h
> +++ b/include/sound/soc-component.h
> @@ -265,15 +265,11 @@ struct snd_soc_component {
>    * snd_soc_dapm_to_component() - Casts a DAPM context to the component it is
>    *  embedded in
>    * @dapm: The DAPM context to cast to the component
> - *
> - * This function must only be used on DAPM contexts that are known to be part of
> - * a component (e.g. in a component driver). Otherwise the behavior is
> - * undefined.
>    */
>   static inline struct snd_soc_component *snd_soc_dapm_to_component(
>   	struct snd_soc_dapm_context *dapm)
>   {
> -	return container_of(dapm, struct snd_soc_component, dapm);
> +	return dapm->component;
>   }
>   
>   /**

Why not drop the wrapper all along? Personally, I find assignment 'comp 
= dapm->component' more readable than the wrapper above. We have very 
long prefixes in sound/soc/ in general, and the devs end up reaching 
char line-limits.

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.

Kind regards,
Czarek