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

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

[Pierre-Louis Bossart]

On 1/27/23 17:11, Amadeusz Sławiński wrote:
> struct snd_soc_dobj only needs pointer to the unload function, instead
> however, there is pointer to all topology operations. Change code to use
> the function pointer instead of pointer to structure containing all
> operations.

I still don't see what we gain with this change.

what we had was:

	sbe->dobj.ops = tplg->ops;

	if (dobj->ops && dobj->ops->control_unload)
		dobj->ops->control_unload(comp, dobj);

	
and now we have this:

	if (tplg->ops)
		sbe->dobj.unload = tplg->ops->control_unload;
	
	if (dobj->unload)
		dobj->unload(comp, dobj);

The improvement is far from obvious to me.

Not going to lay on the tracks for this though.

Re: [PATCH 00/11] ASoC: topology: Fixes and cleanups

[Jaroslav Kysela]

On 28. 01. 23 0:11, Amadeusz Sławiński wrote:
> Following is series of fixes and cleanups for core topology code. Few
> patches fixing various problems all around and few fixing function
> names.

Just an info - the sent date seems to be in future in your git patch sets.

					Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

[PATCH 00/11] ASoC: topology: Fixes and cleanups

[Amadeusz Sławiński]

Following is series of fixes and cleanups for core topology code. Few
patches fixing various problems all around and few fixing function
names.

v2:
 - fix commit messages
 - change order of last two patches
 - as there is no code changes, add Reviewed-by from Ranjani

Amadeusz Sławiński (11):
  ASoC: topology: Properly access value coming from topology file
  ASoC: topology: Remove unused SOC_TPLG_PASS_PINS constant
  ASoC: topology: Fix typo in functions name
  ASoC: topology: Fix function name
  ASoC: topology: Rename remove_ handlers
  ASoC: topology: Remove unnecessary forward declarations
  ASoC: topology: Pass correct pointer instead of casting
  ASoC: topology: Return an error on complete() failure
  ASoC: Topology: Remove unnecessary check for EOF
  ASoC: topology: Unify kcontrol removal code
  ASoC: topology: Use unload() op directly

 include/sound/soc-topology.h |   2 +-
 sound/soc/soc-topology.c     | 183 ++++++++++++++---------------------
 2 files changed, 74 insertions(+), 111 deletions(-)

-- 
2.25.1

[PATCH 01/11] ASoC: topology: Properly access value coming from topology file

[Amadeusz Sławiński]

When accessing values coming from topology, le32_to_cpu should be used.
One of recent commits missed that.

Fixes: 86e2d14b6d1a ("ASoC: topology: Add header payload_size verification")
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/soc-topology.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index c3be24b2fac5..b9addbab2b3d 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -2404,7 +2404,7 @@ static int soc_valid_header(struct soc_tplg *tplg,
 		return -EINVAL;
 	}
 
-	if (soc_tplg_get_hdr_offset(tplg) + hdr->payload_size >= tplg->fw->size) {
+	if (soc_tplg_get_hdr_offset(tplg) + le32_to_cpu(hdr->payload_size) >= tplg->fw->size) {
 		dev_err(tplg->dev,
 			"ASoC: invalid header of type %d at offset %ld payload_size %d\n",
 			le32_to_cpu(hdr->type), soc_tplg_get_hdr_offset(tplg),
-- 
2.25.1

[PATCH 02/11] ASoC: topology: Remove unused SOC_TPLG_PASS_PINS constant

[Amadeusz Sławiński]

The constant is unused, so it can be safely removed.

Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/soc-topology.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index b9addbab2b3d..e9138ec4df8f 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -44,9 +44,8 @@
 #define SOC_TPLG_PASS_WIDGET		3
 #define SOC_TPLG_PASS_PCM_DAI		4
 #define SOC_TPLG_PASS_GRAPH		5
-#define SOC_TPLG_PASS_PINS		6
-#define SOC_TPLG_PASS_BE_DAI		7
-#define SOC_TPLG_PASS_LINK		8
+#define SOC_TPLG_PASS_BE_DAI		6
+#define SOC_TPLG_PASS_LINK		7
 
 #define SOC_TPLG_PASS_START	SOC_TPLG_PASS_MANIFEST
 #define SOC_TPLG_PASS_END	SOC_TPLG_PASS_LINK
-- 
2.25.1

[PATCH 03/11] ASoC: topology: Fix typo in functions name

[Amadeusz Sławiński]

Topology is being abbreviated to "tplg", not "tplc", however, few
functions have typo in name, fix it.

Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/soc-topology.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index e9138ec4df8f..b9c29effeb60 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -185,7 +185,7 @@ static const struct soc_tplg_map dapm_map[] = {
 	{SND_SOC_TPLG_DAPM_DECODER, snd_soc_dapm_decoder},
 };
 
-static int tplc_chan_get_reg(struct soc_tplg *tplg,
+static int tplg_chan_get_reg(struct soc_tplg *tplg,
 	struct snd_soc_tplg_channel *chan, int map)
 {
 	int i;
@@ -198,7 +198,7 @@ static int tplc_chan_get_reg(struct soc_tplg *tplg,
 	return -EINVAL;
 }
 
-static int tplc_chan_get_shift(struct soc_tplg *tplg,
+static int tplg_chan_get_shift(struct soc_tplg *tplg,
 	struct snd_soc_tplg_channel *chan, int map)
 {
 	int i;
@@ -779,10 +779,10 @@ static int soc_tplg_dmixer_create(struct soc_tplg *tplg, size_t size)
 	kc.access = le32_to_cpu(mc->hdr.access);
 
 	/* we only support FL/FR channel mapping atm */
-	sm->reg = tplc_chan_get_reg(tplg, mc->channel, SNDRV_CHMAP_FL);
-	sm->rreg = tplc_chan_get_reg(tplg, mc->channel, SNDRV_CHMAP_FR);
-	sm->shift = tplc_chan_get_shift(tplg, mc->channel, SNDRV_CHMAP_FL);
-	sm->rshift = tplc_chan_get_shift(tplg, mc->channel, SNDRV_CHMAP_FR);
+	sm->reg = tplg_chan_get_reg(tplg, mc->channel, SNDRV_CHMAP_FL);
+	sm->rreg = tplg_chan_get_reg(tplg, mc->channel, SNDRV_CHMAP_FR);
+	sm->shift = tplg_chan_get_shift(tplg, mc->channel, SNDRV_CHMAP_FL);
+	sm->rshift = tplg_chan_get_shift(tplg, mc->channel, SNDRV_CHMAP_FR);
 
 	sm->max = le32_to_cpu(mc->max);
 	sm->min = le32_to_cpu(mc->min);
@@ -926,10 +926,10 @@ static int soc_tplg_denum_create(struct soc_tplg *tplg, size_t size)
 	kc.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
 	kc.access = le32_to_cpu(ec->hdr.access);
 
-	se->reg = tplc_chan_get_reg(tplg, ec->channel, SNDRV_CHMAP_FL);
-	se->shift_l = tplc_chan_get_shift(tplg, ec->channel,
+	se->reg = tplg_chan_get_reg(tplg, ec->channel, SNDRV_CHMAP_FL);
+	se->shift_l = tplg_chan_get_shift(tplg, ec->channel,
 		SNDRV_CHMAP_FL);
-	se->shift_r = tplc_chan_get_shift(tplg, ec->channel,
+	se->shift_r = tplg_chan_get_shift(tplg, ec->channel,
 		SNDRV_CHMAP_FL);
 
 	se->mask = le32_to_cpu(ec->mask);
@@ -1160,13 +1160,13 @@ static int soc_tplg_dapm_widget_dmixer_create(struct soc_tplg *tplg, struct snd_
 	kc->access = le32_to_cpu(mc->hdr.access);
 
 	/* we only support FL/FR channel mapping atm */
-	sm->reg = tplc_chan_get_reg(tplg, mc->channel,
+	sm->reg = tplg_chan_get_reg(tplg, mc->channel,
 				    SNDRV_CHMAP_FL);
-	sm->rreg = tplc_chan_get_reg(tplg, mc->channel,
+	sm->rreg = tplg_chan_get_reg(tplg, mc->channel,
 				     SNDRV_CHMAP_FR);
-	sm->shift = tplc_chan_get_shift(tplg, mc->channel,
+	sm->shift = tplg_chan_get_shift(tplg, mc->channel,
 					SNDRV_CHMAP_FL);
-	sm->rshift = tplc_chan_get_shift(tplg, mc->channel,
+	sm->rshift = tplg_chan_get_shift(tplg, mc->channel,
 					 SNDRV_CHMAP_FR);
 
 	sm->max = le32_to_cpu(mc->max);
@@ -1232,10 +1232,10 @@ static int soc_tplg_dapm_widget_denum_create(struct soc_tplg *tplg, struct snd_k
 	kc->access = le32_to_cpu(ec->hdr.access);
 
 	/* we only support FL/FR channel mapping atm */
-	se->reg = tplc_chan_get_reg(tplg, ec->channel, SNDRV_CHMAP_FL);
-	se->shift_l = tplc_chan_get_shift(tplg, ec->channel,
+	se->reg = tplg_chan_get_reg(tplg, ec->channel, SNDRV_CHMAP_FL);
+	se->shift_l = tplg_chan_get_shift(tplg, ec->channel,
 					  SNDRV_CHMAP_FL);
-	se->shift_r = tplc_chan_get_shift(tplg, ec->channel,
+	se->shift_r = tplg_chan_get_shift(tplg, ec->channel,
 					  SNDRV_CHMAP_FR);
 
 	se->items = le32_to_cpu(ec->items);
-- 
2.25.1

[PATCH 04/11] ASoC: topology: Fix function name

[Amadeusz Sławiński]

Functions other than soc_valid_header have soc_tplg_ prefix. Rename
function to follow convention in file.

Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/soc-topology.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index b9c29effeb60..1fe4fa82fa2f 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -2389,7 +2389,7 @@ static int soc_tplg_manifest_load(struct soc_tplg *tplg,
 }
 
 /* validate header magic, size and type */
-static int soc_valid_header(struct soc_tplg *tplg,
+static int soc_tplg_valid_header(struct soc_tplg *tplg,
 	struct snd_soc_tplg_hdr *hdr)
 {
 	if (soc_tplg_get_hdr_offset(tplg) >= tplg->fw->size)
@@ -2526,7 +2526,7 @@ static int soc_tplg_process_headers(struct soc_tplg *tplg)
 		while (!soc_tplg_is_eof(tplg)) {
 
 			/* make sure header is valid before loading */
-			ret = soc_valid_header(tplg, hdr);
+			ret = soc_tplg_valid_header(tplg, hdr);
 			if (ret < 0) {
 				dev_err(tplg->dev,
 					"ASoC: topology: invalid header: %d\n", ret);
-- 
2.25.1

[PATCH 05/11] ASoC: topology: Rename remove_ handlers

[Amadeusz Sławiński]

Those are the only functions missing soc_tplg_ prefix, add it for
consistency.

Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/soc-topology.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 1fe4fa82fa2f..a952f7eaa646 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -354,7 +354,7 @@ static int soc_tplg_add_kcontrol(struct soc_tplg *tplg,
 }
 
 /* remove a mixer kcontrol */
-static void remove_mixer(struct snd_soc_component *comp,
+static void soc_tplg_remove_mixer(struct snd_soc_component *comp,
 	struct snd_soc_dobj *dobj, int pass)
 {
 	struct snd_card *card = comp->card->snd_card;
@@ -370,7 +370,7 @@ static void remove_mixer(struct snd_soc_component *comp,
 }
 
 /* remove an enum kcontrol */
-static void remove_enum(struct snd_soc_component *comp,
+static void soc_tplg_remove_enum(struct snd_soc_component *comp,
 	struct snd_soc_dobj *dobj, int pass)
 {
 	struct snd_card *card = comp->card->snd_card;
@@ -386,7 +386,7 @@ static void remove_enum(struct snd_soc_component *comp,
 }
 
 /* remove a byte kcontrol */
-static void remove_bytes(struct snd_soc_component *comp,
+static void soc_tplg_remove_bytes(struct snd_soc_component *comp,
 	struct snd_soc_dobj *dobj, int pass)
 {
 	struct snd_card *card = comp->card->snd_card;
@@ -402,7 +402,7 @@ static void remove_bytes(struct snd_soc_component *comp,
 }
 
 /* remove a route */
-static void remove_route(struct snd_soc_component *comp,
+static void soc_tplg_remove_route(struct snd_soc_component *comp,
 			 struct snd_soc_dobj *dobj, int pass)
 {
 	if (pass != SOC_TPLG_PASS_GRAPH)
@@ -415,7 +415,7 @@ static void remove_route(struct snd_soc_component *comp,
 }
 
 /* remove a widget and it's kcontrols - routes must be removed first */
-static void remove_widget(struct snd_soc_component *comp,
+static void soc_tplg_remove_widget(struct snd_soc_component *comp,
 	struct snd_soc_dobj *dobj, int pass)
 {
 	struct snd_card *card = comp->card->snd_card;
@@ -443,7 +443,7 @@ static void remove_widget(struct snd_soc_component *comp,
 }
 
 /* remove DAI configurations */
-static void remove_dai(struct snd_soc_component *comp,
+static void soc_tplg_remove_dai(struct snd_soc_component *comp,
 	struct snd_soc_dobj *dobj, int pass)
 {
 	struct snd_soc_dai_driver *dai_drv =
@@ -464,7 +464,7 @@ static void remove_dai(struct snd_soc_component *comp,
 }
 
 /* remove link configurations */
-static void remove_link(struct snd_soc_component *comp,
+static void soc_tplg_remove_link(struct snd_soc_component *comp,
 	struct snd_soc_dobj *dobj, int pass)
 {
 	struct snd_soc_dai_link *link =
@@ -492,7 +492,7 @@ static void remove_backend_link(struct snd_soc_component *comp,
 		dobj->ops->link_unload(comp, dobj);
 
 	/*
-	 * We don't free the link here as what remove_link() do since BE
+	 * We don't free the link here as what soc_tplg_remove_link() do since BE
 	 * links are not allocated by topology.
 	 * We however need to reset the dobj type to its initial values
 	 */
@@ -1492,7 +1492,7 @@ static int soc_tplg_dapm_widget_create(struct soc_tplg *tplg,
 	return 0;
 
 ready_err:
-	remove_widget(widget->dapm->component, &widget->dobj, SOC_TPLG_PASS_WIDGET);
+	soc_tplg_remove_widget(widget->dapm->component, &widget->dobj, SOC_TPLG_PASS_WIDGET);
 	snd_soc_dapm_free_widget(widget);
 hdr_err:
 	kfree(template.sname);
@@ -2627,25 +2627,25 @@ int snd_soc_tplg_component_remove(struct snd_soc_component *comp)
 
 			switch (dobj->type) {
 			case SND_SOC_DOBJ_MIXER:
-				remove_mixer(comp, dobj, pass);
+				soc_tplg_remove_mixer(comp, dobj, pass);
 				break;
 			case SND_SOC_DOBJ_ENUM:
-				remove_enum(comp, dobj, pass);
+				soc_tplg_remove_enum(comp, dobj, pass);
 				break;
 			case SND_SOC_DOBJ_BYTES:
-				remove_bytes(comp, dobj, pass);
+				soc_tplg_remove_bytes(comp, dobj, pass);
 				break;
 			case SND_SOC_DOBJ_GRAPH:
-				remove_route(comp, dobj, pass);
+				soc_tplg_remove_route(comp, dobj, pass);
 				break;
 			case SND_SOC_DOBJ_WIDGET:
-				remove_widget(comp, dobj, pass);
+				soc_tplg_remove_widget(comp, dobj, pass);
 				break;
 			case SND_SOC_DOBJ_PCM:
-				remove_dai(comp, dobj, pass);
+				soc_tplg_remove_dai(comp, dobj, pass);
 				break;
 			case SND_SOC_DOBJ_DAI_LINK:
-				remove_link(comp, dobj, pass);
+				soc_tplg_remove_link(comp, dobj, pass);
 				break;
 			case SND_SOC_DOBJ_BACKEND_LINK:
 				/*
-- 
2.25.1

[PATCH 06/11] ASoC: topology: Remove unnecessary forward declarations

[Amadeusz Sławiński]

There is no need to forward declare functions if their use is after
their definition.

Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/soc-topology.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index a952f7eaa646..766de161aa4a 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -76,9 +76,6 @@ struct soc_tplg {
 	struct snd_soc_tplg_ops *ops;
 };
 
-static int soc_tplg_process_headers(struct soc_tplg *tplg);
-static int soc_tplg_complete(struct soc_tplg *tplg);
-
 /* check we dont overflow the data for this control chunk */
 static int soc_tplg_check_elem_count(struct soc_tplg *tplg, size_t elem_size,
 	unsigned int count, size_t bytes, const char *elem_type)
-- 
2.25.1

[PATCH 07/11] ASoC: topology: Pass correct pointer instead of casting

[Amadeusz Sławiński]

Instead of passing address of structure, the containing structure is
cast to target structure. While it works - the expected structure is the
first field of containing one - it is bad practice, fix this by passing
pointer to structure field.

Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/soc-topology.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 766de161aa4a..c35c0c53a184 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -721,7 +721,7 @@ static int soc_tplg_dbytes_create(struct soc_tplg *tplg, size_t size)
 	}
 
 	/* pass control to driver for optional further init */
-	ret = soc_tplg_control_load(tplg, &kc, (struct snd_soc_tplg_ctl_hdr *)be);
+	ret = soc_tplg_control_load(tplg, &kc, &be->hdr);
 	if (ret < 0) {
 		dev_err(tplg->dev, "ASoC: failed to init %s\n", be->hdr.name);
 		goto err;
@@ -805,7 +805,7 @@ static int soc_tplg_dmixer_create(struct soc_tplg *tplg, size_t size)
 	}
 
 	/* pass control to driver for optional further init */
-	ret = soc_tplg_control_load(tplg, &kc, (struct snd_soc_tplg_ctl_hdr *)mc);
+	ret = soc_tplg_control_load(tplg, &kc, &mc->hdr);
 	if (ret < 0) {
 		dev_err(tplg->dev, "ASoC: failed to init %s\n", mc->hdr.name);
 		goto err;
@@ -973,7 +973,7 @@ static int soc_tplg_denum_create(struct soc_tplg *tplg, size_t size)
 	}
 
 	/* pass control to driver for optional further init */
-	ret = soc_tplg_control_load(tplg, &kc, (struct snd_soc_tplg_ctl_hdr *)ec);
+	ret = soc_tplg_control_load(tplg, &kc, &ec->hdr);
 	if (ret < 0) {
 		dev_err(tplg->dev, "ASoC: failed to init %s\n", ec->hdr.name);
 		goto err;
@@ -1189,7 +1189,7 @@ static int soc_tplg_dapm_widget_dmixer_create(struct soc_tplg *tplg, struct snd_
 	}
 
 	/* pass control to driver for optional further init */
-	err = soc_tplg_control_load(tplg, kc, (struct snd_soc_tplg_ctl_hdr *)mc);
+	err = soc_tplg_control_load(tplg, kc, &mc->hdr);
 	if (err < 0) {
 		dev_err(tplg->dev, "ASoC: failed to init %s\n",
 			mc->hdr.name);
@@ -1273,7 +1273,7 @@ static int soc_tplg_dapm_widget_denum_create(struct soc_tplg *tplg, struct snd_k
 	}
 
 	/* pass control to driver for optional further init */
-	err = soc_tplg_control_load(tplg, kc, (struct snd_soc_tplg_ctl_hdr *)ec);
+	err = soc_tplg_control_load(tplg, kc, &ec->hdr);
 	if (err < 0) {
 		dev_err(tplg->dev, "ASoC: failed to init %s\n",
 			ec->hdr.name);
@@ -1325,7 +1325,7 @@ static int soc_tplg_dapm_widget_dbytes_create(struct soc_tplg *tplg, struct snd_
 	}
 
 	/* pass control to driver for optional further init */
-	err = soc_tplg_control_load(tplg, kc, (struct snd_soc_tplg_ctl_hdr *)be);
+	err = soc_tplg_control_load(tplg, kc, &be->hdr);
 	if (err < 0) {
 		dev_err(tplg->dev, "ASoC: failed to init %s\n",
 			be->hdr.name);
-- 
2.25.1

[PATCH 08/11] ASoC: topology: Return an error on complete() failure

[Amadeusz Sławiński]

Function soc_tplg_dapm_complete() detects an error and logs it, but
doesn't return failure to the caller, fix it by returning the error.

Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/soc-topology.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index c35c0c53a184..08dd55f94584 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -1563,7 +1563,7 @@ static int soc_tplg_dapm_complete(struct soc_tplg *tplg)
 		dev_err(tplg->dev, "ASoC: failed to create new widgets %d\n",
 			ret);
 
-	return 0;
+	return ret;
 }
 
 static int set_stream_info(struct soc_tplg *tplg, struct snd_soc_pcm_stream *stream,
-- 
2.25.1

[PATCH 09/11] ASoC: Topology: Remove unnecessary check for EOF

[Amadeusz Sławiński]

Caller already checks if hdr_pos is behind EOF, before calling
soc_tplg_valid_header(), so there is no need to recheck it again. This
also allows to remove behaviour of return 0 - forcing the caller to
break out of while loop.

Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/soc-topology.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 08dd55f94584..6689cf44464c 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -2389,9 +2389,6 @@ static int soc_tplg_manifest_load(struct soc_tplg *tplg,
 static int soc_tplg_valid_header(struct soc_tplg *tplg,
 	struct snd_soc_tplg_hdr *hdr)
 {
-	if (soc_tplg_get_hdr_offset(tplg) >= tplg->fw->size)
-		return 0;
-
 	if (le32_to_cpu(hdr->size) != sizeof(*hdr)) {
 		dev_err(tplg->dev,
 			"ASoC: invalid header size for type %d at offset 0x%lx size 0x%zx.\n",
@@ -2442,7 +2439,7 @@ static int soc_tplg_valid_header(struct soc_tplg *tplg,
 		return -EINVAL;
 	}
 
-	return 1;
+	return 0;
 }
 
 /* check header type and call appropriate handler */
@@ -2528,8 +2525,6 @@ static int soc_tplg_process_headers(struct soc_tplg *tplg)
 				dev_err(tplg->dev,
 					"ASoC: topology: invalid header: %d\n", ret);
 				return ret;
-			} else if (ret == 0) {
-				break;
 			}
 
 			/* load the header object */
-- 
2.25.1

[PATCH 10/11] ASoC: topology: Unify kcontrol removal code

[Amadeusz Sławiński]

Functions removing bytes, enum and mixer kcontrols are identical. Unify
them under one function and use it to free associated kcontrols.

Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
---
 sound/soc/soc-topology.c | 48 +++++-----------------------------------
 1 file changed, 6 insertions(+), 42 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 6689cf44464c..9f527d9baf1c 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -350,41 +350,9 @@ static int soc_tplg_add_kcontrol(struct soc_tplg *tplg,
 				tplg->dev, k, comp->name_prefix, comp, kcontrol);
 }
 
-/* remove a mixer kcontrol */
-static void soc_tplg_remove_mixer(struct snd_soc_component *comp,
-	struct snd_soc_dobj *dobj, int pass)
-{
-	struct snd_card *card = comp->card->snd_card;
-
-	if (pass != SOC_TPLG_PASS_CONTROL)
-		return;
-
-	if (dobj->ops && dobj->ops->control_unload)
-		dobj->ops->control_unload(comp, dobj);
-
-	snd_ctl_remove(card, dobj->control.kcontrol);
-	list_del(&dobj->list);
-}
-
-/* remove an enum kcontrol */
-static void soc_tplg_remove_enum(struct snd_soc_component *comp,
-	struct snd_soc_dobj *dobj, int pass)
-{
-	struct snd_card *card = comp->card->snd_card;
-
-	if (pass != SOC_TPLG_PASS_CONTROL)
-		return;
-
-	if (dobj->ops && dobj->ops->control_unload)
-		dobj->ops->control_unload(comp, dobj);
-
-	snd_ctl_remove(card, dobj->control.kcontrol);
-	list_del(&dobj->list);
-}
-
-/* remove a byte kcontrol */
-static void soc_tplg_remove_bytes(struct snd_soc_component *comp,
-	struct snd_soc_dobj *dobj, int pass)
+/* remove kcontrol */
+static void soc_tplg_remove_kcontrol(struct snd_soc_component *comp, struct snd_soc_dobj *dobj,
+				     int pass)
 {
 	struct snd_card *card = comp->card->snd_card;
 
@@ -2618,14 +2586,10 @@ int snd_soc_tplg_component_remove(struct snd_soc_component *comp)
 			list) {
 
 			switch (dobj->type) {
-			case SND_SOC_DOBJ_MIXER:
-				soc_tplg_remove_mixer(comp, dobj, pass);
-				break;
-			case SND_SOC_DOBJ_ENUM:
-				soc_tplg_remove_enum(comp, dobj, pass);
-				break;
 			case SND_SOC_DOBJ_BYTES:
-				soc_tplg_remove_bytes(comp, dobj, pass);
+			case SND_SOC_DOBJ_ENUM:
+			case SND_SOC_DOBJ_MIXER:
+				soc_tplg_remove_kcontrol(comp, dobj, pass);
 				break;
 			case SND_SOC_DOBJ_GRAPH:
 				soc_tplg_remove_route(comp, dobj, pass);
-- 
2.25.1

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

[Amadeusz Sławiński]

struct snd_soc_dobj only needs pointer to the unload function, instead
however, there is pointer to all topology operations. Change code to use
the function pointer instead of pointer to structure containing all
operations.

Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.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     | 48 +++++++++++++++++++++---------------
 2 files changed, 29 insertions(+), 21 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);
 	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 9f527d9baf1c..e66b0d9e387a 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -359,8 +359,8 @@ static void soc_tplg_remove_kcontrol(struct snd_soc_component *comp, struct snd_
 	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);
@@ -373,8 +373,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);
 }
@@ -391,8 +391,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;
@@ -418,8 +418,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)
@@ -438,8 +438,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,
@@ -453,8 +453,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
@@ -678,7 +678,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 */
@@ -754,8 +755,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 */
@@ -900,7 +902,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)) {
@@ -1077,7 +1080,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);
 
@@ -1443,7 +1447,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);
 
@@ -1621,8 +1626,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 */
@@ -1691,8 +1697,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);
@@ -2099,8 +2106,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;
-- 
2.25.1

Re: [PATCH 00/11] ASoC: topology: Fixes and cleanups

[Amadeusz Sławiński]

On 1/27/2023 5:27 PM, Jaroslav Kysela wrote:
> On 28. 01. 23 0:11, Amadeusz Sławiński wrote:
>> Following is series of fixes and cleanups for core topology code. Few
>> patches fixing various problems all around and few fixing function
>> names.
> 
> Just an info - the sent date seems to be in future in your git patch sets.
> 

Eh..., I'm not sure how it got out of sync. Anyway fixed. Thanks!

Re: [PATCH 00/11] ASoC: topology: Fixes and cleanups

[Amadeusz Sławiński]

On 1/28/2023 12:11 AM, Amadeusz Sławiński wrote:
> Following is series of fixes and cleanups for core topology code. Few
> patches fixing various problems all around and few fixing function
> names.
> 
> v2:
>   - fix commit messages
>   - change order of last two patches
>   - as there is no code changes, add Reviewed-by from Ranjani
> 

And I've just realized that I forgot -v2 switch when sending... I hope I 
don't need to resend ;)

> Amadeusz Sławiński (11):
>    ASoC: topology: Properly access value coming from topology file
>    ASoC: topology: Remove unused SOC_TPLG_PASS_PINS constant
>    ASoC: topology: Fix typo in functions name
>    ASoC: topology: Fix function name
>    ASoC: topology: Rename remove_ handlers
>    ASoC: topology: Remove unnecessary forward declarations
>    ASoC: topology: Pass correct pointer instead of casting
>    ASoC: topology: Return an error on complete() failure
>    ASoC: Topology: Remove unnecessary check for EOF
>    ASoC: topology: Unify kcontrol removal code
>    ASoC: topology: Use unload() op directly
> 
>   include/sound/soc-topology.h |   2 +-
>   sound/soc/soc-topology.c     | 183 ++++++++++++++---------------------
>   2 files changed, 74 insertions(+), 111 deletions(-)
> 

Re: [PATCH 00/11] ASoC: topology: Fixes and cleanups

[Mark Brown]

On Sat, 28 Jan 2023 00:11:00 +0100, Amadeusz Sławiński wrote:
> Following is series of fixes and cleanups for core topology code. Few
> patches fixing various problems all around and few fixing function
> names.
> 
> v2:
>  - fix commit messages
>  - change order of last two patches
>  - as there is no code changes, add Reviewed-by from Ranjani
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[01/11] ASoC: topology: Properly access value coming from topology file
        commit: c5d184c92df2b631fb81fe2ce6e96bfc5ba720e5
[02/11] ASoC: topology: Remove unused SOC_TPLG_PASS_PINS constant
        commit: 6257d224b894b676540998ae9563c211410c9436
[03/11] ASoC: topology: Fix typo in functions name
        commit: 8f9974d9d767d11ce17280bec0d0f2e95e91954d
[04/11] ASoC: topology: Fix function name
        commit: 23e591dc0f8ce0298857a1445993fa7549a1f2e0
[05/11] ASoC: topology: Rename remove_ handlers
        commit: 2abfd4bd7b0700df4996ae2b60a12f22a0ef633d
[06/11] ASoC: topology: Remove unnecessary forward declarations
        commit: 70a7cd09a6368e0c9d351185a8fbfb3bae5a74f3
[07/11] ASoC: topology: Pass correct pointer instead of casting
        commit: 9e2ee00039a8ff236ae4db2366f4d2325658bea6
[08/11] ASoC: topology: Return an error on complete() failure
        commit: b784617a407c4f7e079e1694c3161ab29eb4bab1
[09/11] ASoC: Topology: Remove unnecessary check for EOF
        commit: d9b07b790a5c47dd4fd66c9264a3b38a103fa09b
[10/11] ASoC: topology: Unify kcontrol removal code
        commit: fdfa3661f830c98fb0f6380c3876fae33bc83b1d
[11/11] ASoC: topology: Use unload() op directly
        commit: 31e9273912bf5e4c23a876b5dfe0760fbecde92c

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH 00/11] ASoC: topology: Fixes and cleanups

[Ranjani Sridharan]

On Wed, 2023-01-25 at 20:46 +0100, Amadeusz Sławiński wrote:
> Following is series of fixes and cleanups for core topology code. Few
> patches fixing various problems all around and few fixing function
> names.
> 
> Amadeusz Sławiński (11):
>   ASoC: topology: Properly access value coming from topology file
>   ASoC: topology: Remove unused SOC_TPLG_PASS_PINS constant
>   ASoC: topology: Fix typo in functions name
>   ASoC: topology: Fix function name
>   ASoC: topology: Rename remove_ handlers
>   ASoC: topology: Remove unnecessary forward declarations
>   ASoC: topology: Pass correct pointer instead of casting
>   ASoC: topology: Return an error on complete() failure
>   ASoC: Topology: Remove unnecessary check for EOF
>   ASoC: topology: Use unload() op directly
>   ASoC: topology: Unify kcontrol removal code

 LGTM, thanks, Amadeusz!

Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

[PATCH 00/11] ASoC: topology: Fixes and cleanups

[Amadeusz Sławiński]

Following is series of fixes and cleanups for core topology code. Few
patches fixing various problems all around and few fixing function
names.

Amadeusz Sławiński (11):
  ASoC: topology: Properly access value coming from topology file
  ASoC: topology: Remove unused SOC_TPLG_PASS_PINS constant
  ASoC: topology: Fix typo in functions name
  ASoC: topology: Fix function name
  ASoC: topology: Rename remove_ handlers
  ASoC: topology: Remove unnecessary forward declarations
  ASoC: topology: Pass correct pointer instead of casting
  ASoC: topology: Return an error on complete() failure
  ASoC: Topology: Remove unnecessary check for EOF
  ASoC: topology: Use unload() op directly
  ASoC: topology: Unify kcontrol removal code

 include/sound/soc-topology.h |   2 +-
 sound/soc/soc-topology.c     | 183 ++++++++++++++---------------------
 2 files changed, 74 insertions(+), 111 deletions(-)

-- 
2.25.1