Re: [PATCH 10/11] ASoC: topology: Use unload() op directly

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

On 1/25/23 13:46, Amadeusz Sławiński wrote:
> Generic dynamic object (struct snd_soc_dobj) needs pointer to unload
> function, however, instead of using function pointer to point at it
> directly it points to all topology operations. Change code to use the
> function pointer instead.

This is a convoluted explanation, and the code does not support this
last sentence.

There is no existing dobj 'function pointer', it's added by this patch ....

> Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> ---
>  include/sound/soc-topology.h |  2 +-
>  sound/soc/soc-topology.c     | 56 ++++++++++++++++++++----------------
>  2 files changed, 33 insertions(+), 25 deletions(-)
> 
> diff --git a/include/sound/soc-topology.h b/include/sound/soc-topology.h
> index b4b896f83b94..f055c6917f6c 100644
> --- a/include/sound/soc-topology.h
> +++ b/include/sound/soc-topology.h
> @@ -62,7 +62,7 @@ struct snd_soc_dobj {
>  	enum snd_soc_dobj_type type;
>  	unsigned int index;	/* objects can belong in different groups */
>  	struct list_head list;
> -	struct snd_soc_tplg_ops *ops;
> +	int (*unload)(struct snd_soc_component *comp, struct snd_soc_dobj *dobj);

.... here

So what this changes is that instead of following the indirections ...

>  	union {
>  		struct snd_soc_dobj_control control;
>  		struct snd_soc_dobj_widget widget;
> diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
> index 6689cf44464c..eb49037d86ae 100644
> --- a/sound/soc/soc-topology.c
> +++ b/sound/soc/soc-topology.c
> @@ -359,8 +359,8 @@ static void soc_tplg_remove_mixer(struct snd_soc_component *comp,
>  	if (pass != SOC_TPLG_PASS_CONTROL)
>  		return;
>  
> -	if (dobj->ops && dobj->ops->control_unload)
> -		dobj->ops->control_unload(comp, dobj);
> +	if (dobj->unload)
> +		dobj->unload(comp, dobj);

... here, you first need to set that pointer ....

>  
>  	sbe->max = le32_to_cpu(be->max);
>  	sbe->dobj.type = SND_SOC_DOBJ_BYTES;
> -	sbe->dobj.ops = tplg->ops;
> +	if (tplg->ops)
> +		sbe->dobj.unload = tplg->ops->control_unload;

... here.

I don't see the gain, sorry.

Edit: This removal only makes sense with the patch 11 added, where the
same function can be used to remove multiple types of control.

Please revisit the commit message here and explain the intent, otherwise
this change in isolation isn't really useful.