diff mbox series

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

Message ID 20230125194649.3485731-11-amadeuszx.slawinski@linux.intel.com (mailing list archive)
State Superseded
Headers show
Series ASoC: topology: Fixes and cleanups | expand

Commit Message

Amadeusz Sławiński Jan. 25, 2023, 7:46 p.m. UTC 
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.

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(-)

Comments

Pierre-Louis Bossart Jan. 25, 2023, 3:13 p.m. UTC | #1 
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.
diff mbox series

Patch

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);
 	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);
 
 	snd_ctl_remove(card, dobj->control.kcontrol);
 	list_del(&dobj->list);
@@ -375,8 +375,8 @@  static void soc_tplg_remove_enum(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);
 
 	snd_ctl_remove(card, dobj->control.kcontrol);
 	list_del(&dobj->list);
@@ -391,8 +391,8 @@  static void soc_tplg_remove_bytes(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);
 
 	snd_ctl_remove(card, dobj->control.kcontrol);
 	list_del(&dobj->list);
@@ -405,8 +405,8 @@  static void soc_tplg_remove_route(struct snd_soc_component *comp,
 	if (pass != SOC_TPLG_PASS_GRAPH)
 		return;
 
-	if (dobj->ops && dobj->ops->dapm_route_unload)
-		dobj->ops->dapm_route_unload(comp, dobj);
+	if (dobj->unload)
+		dobj->unload(comp, dobj);
 
 	list_del(&dobj->list);
 }
@@ -423,8 +423,8 @@  static void soc_tplg_remove_widget(struct snd_soc_component *comp,
 	if (pass != SOC_TPLG_PASS_WIDGET)
 		return;
 
-	if (dobj->ops && dobj->ops->widget_unload)
-		dobj->ops->widget_unload(comp, dobj);
+	if (dobj->unload)
+		dobj->unload(comp, dobj);
 
 	if (!w->kcontrols)
 		goto free_news;
@@ -450,8 +450,8 @@  static void soc_tplg_remove_dai(struct snd_soc_component *comp,
 	if (pass != SOC_TPLG_PASS_PCM_DAI)
 		return;
 
-	if (dobj->ops && dobj->ops->dai_unload)
-		dobj->ops->dai_unload(comp, dobj);
+	if (dobj->unload)
+		dobj->unload(comp, dobj);
 
 	for_each_component_dais_safe(comp, dai, _dai)
 		if (dai->driver == dai_drv)
@@ -470,8 +470,8 @@  static void soc_tplg_remove_link(struct snd_soc_component *comp,
 	if (pass != SOC_TPLG_PASS_PCM_DAI)
 		return;
 
-	if (dobj->ops && dobj->ops->link_unload)
-		dobj->ops->link_unload(comp, dobj);
+	if (dobj->unload)
+		dobj->unload(comp, dobj);
 
 	list_del(&dobj->list);
 	snd_soc_remove_pcm_runtime(comp->card,
@@ -485,8 +485,8 @@  static void remove_backend_link(struct snd_soc_component *comp,
 	if (pass != SOC_TPLG_PASS_LINK)
 		return;
 
-	if (dobj->ops && dobj->ops->link_unload)
-		dobj->ops->link_unload(comp, dobj);
+	if (dobj->unload)
+		dobj->unload(comp, dobj);
 
 	/*
 	 * We don't free the link here as what soc_tplg_remove_link() do since BE
@@ -710,7 +710,8 @@  static int soc_tplg_dbytes_create(struct soc_tplg *tplg, size_t size)
 
 	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;
 	INIT_LIST_HEAD(&sbe->dobj.list);
 
 	/* map io handlers */
@@ -786,8 +787,9 @@  static int soc_tplg_dmixer_create(struct soc_tplg *tplg, size_t size)
 	sm->invert = le32_to_cpu(mc->invert);
 	sm->platform_max = le32_to_cpu(mc->platform_max);
 	sm->dobj.index = tplg->index;
-	sm->dobj.ops = tplg->ops;
 	sm->dobj.type = SND_SOC_DOBJ_MIXER;
+	if (tplg->ops)
+		sm->dobj.unload = tplg->ops->control_unload;
 	INIT_LIST_HEAD(&sm->dobj.list);
 
 	/* map io handlers */
@@ -932,7 +934,8 @@  static int soc_tplg_denum_create(struct soc_tplg *tplg, size_t size)
 	se->mask = le32_to_cpu(ec->mask);
 	se->dobj.index = tplg->index;
 	se->dobj.type = SND_SOC_DOBJ_ENUM;
-	se->dobj.ops = tplg->ops;
+	if (tplg->ops)
+		se->dobj.unload = tplg->ops->control_unload;
 	INIT_LIST_HEAD(&se->dobj.list);
 
 	switch (le32_to_cpu(ec->hdr.ops.info)) {
@@ -1109,7 +1112,8 @@  static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg,
 
 		/* add route dobj to dobj_list */
 		route->dobj.type = SND_SOC_DOBJ_GRAPH;
-		route->dobj.ops = tplg->ops;
+		if (tplg->ops)
+			route->dobj.unload = tplg->ops->control_unload;
 		route->dobj.index = tplg->index;
 		list_add(&route->dobj.list, &tplg->comp->dobj_list);
 
@@ -1475,7 +1479,8 @@  static int soc_tplg_dapm_widget_create(struct soc_tplg *tplg,
 
 	widget->dobj.type = SND_SOC_DOBJ_WIDGET;
 	widget->dobj.widget.kcontrol_type = kcontrol_type;
-	widget->dobj.ops = tplg->ops;
+	if (tplg->ops)
+		widget->dobj.unload = tplg->ops->widget_unload;
 	widget->dobj.index = tplg->index;
 	list_add(&widget->dobj.list, &tplg->comp->dobj_list);
 
@@ -1653,8 +1658,9 @@  static int soc_tplg_dai_create(struct soc_tplg *tplg,
 	}
 
 	dai_drv->dobj.index = tplg->index;
-	dai_drv->dobj.ops = tplg->ops;
 	dai_drv->dobj.type = SND_SOC_DOBJ_PCM;
+	if (tplg->ops)
+		dai_drv->dobj.unload = tplg->ops->dai_unload;
 	list_add(&dai_drv->dobj.list, &tplg->comp->dobj_list);
 
 	/* register the DAI to the component */
@@ -1723,8 +1729,9 @@  static int soc_tplg_fe_link_create(struct soc_tplg *tplg,
 	link->num_platforms = 1;
 
 	link->dobj.index = tplg->index;
-	link->dobj.ops = tplg->ops;
 	link->dobj.type = SND_SOC_DOBJ_DAI_LINK;
+	if (tplg->ops)
+		link->dobj.unload = tplg->ops->link_unload;
 
 	if (strlen(pcm->pcm_name)) {
 		link->name = devm_kstrdup(tplg->dev, pcm->pcm_name, GFP_KERNEL);
@@ -2131,8 +2138,9 @@  static int soc_tplg_link_config(struct soc_tplg *tplg,
 
 	/* for unloading it in snd_soc_tplg_component_remove */
 	link->dobj.index = tplg->index;
-	link->dobj.ops = tplg->ops;
 	link->dobj.type = SND_SOC_DOBJ_BACKEND_LINK;
+	if (tplg->ops)
+		link->dobj.unload = tplg->ops->link_unload;
 	list_add(&link->dobj.list, &tplg->comp->dobj_list);
 
 	return 0;