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

[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

[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>
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>
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>
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>
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>
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>
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 containg structure is
casted 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>
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

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

[Pierre-Louis Bossart]

On 1/25/23 13:46, Amadeusz Sławiński wrote:
> Instead of passing address of structure, the containg structure is

containing?

> casted to target structure. While it works - the expected structure is

cast?

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

[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>
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>
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: Use unload() op directly

[Amadeusz Sławiński]

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

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;
-- 
2.25.1

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

[Pierre-Louis Bossart]

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.

[PATCH 11/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>
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 eb49037d86ae..e66b0d9e387a 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->unload)
-		dobj->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->unload)
-		dobj->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;
 
@@ -2626,14 +2594,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

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

[Pierre-Louis Bossart]

On 1/25/23 13:46, Amadeusz Sławiński wrote:
> Functions removing bytes, enum and mixer kcontrols are identical. Unify

they are identical because of the change in patch10.

Please clarify that this is not a cleanup removing duplicated code
that's been there forever, it's become useless as a result of the
previous patch.

> them under one function and use it to free associated kcontrols.
> 
> Reviewed-by: Cezary Rojewski <cezary.rojewski@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 eb49037d86ae..e66b0d9e387a 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->unload)
> -		dobj->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->unload)
> -		dobj->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;
>  
> @@ -2626,14 +2594,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);

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

[Amadeusz Sławiński]

On 1/25/2023 4:15 PM, Pierre-Louis Bossart wrote:
> 
> 
> On 1/25/23 13:46, Amadeusz Sławiński wrote:
>> Functions removing bytes, enum and mixer kcontrols are identical. Unify
> 
> they are identical because of the change in patch10.
> 
> Please clarify that this is not a cleanup removing duplicated code
> that's been there forever, it's become useless as a result of the
> previous patch.
> 

There is no dependency on previous patch - it is just order I've send 
them in - those functions have same implementation in current code.

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

[Pierre-Louis Bossart]

On 1/27/23 05:12, Amadeusz Sławiński wrote:
> On 1/25/2023 4:15 PM, Pierre-Louis Bossart wrote:
>>
>>
>> On 1/25/23 13:46, Amadeusz Sławiński wrote:
>>> Functions removing bytes, enum and mixer kcontrols are identical. Unify
>>
>> they are identical because of the change in patch10.
>>
>> Please clarify that this is not a cleanup removing duplicated code
>> that's been there forever, it's become useless as a result of the
>> previous patch.
>>
> 
> There is no dependency on previous patch - it is just order I've send
> them in - those functions have same implementation in current code.

Not following, sorry. What is this addition in patch 10?

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

That's not in 'current code', is it? How is this not a dependency?

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

[Amadeusz Sławiński]

On 1/27/2023 2:38 PM, Pierre-Louis Bossart wrote:
> 
> 
> On 1/27/23 05:12, Amadeusz Sławiński wrote:
>> On 1/25/2023 4:15 PM, Pierre-Louis Bossart wrote:
>>>
>>>
>>> On 1/25/23 13:46, Amadeusz Sławiński wrote:
>>>> Functions removing bytes, enum and mixer kcontrols are identical. Unify
>>>
>>> they are identical because of the change in patch10.
>>>
>>> Please clarify that this is not a cleanup removing duplicated code
>>> that's been there forever, it's become useless as a result of the
>>> previous patch.
>>>
>>
>> There is no dependency on previous patch - it is just order I've send
>> them in - those functions have same implementation in current code.
> 
> Not following, sorry. What is this addition in patch 10?
> 
> 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);
> 
> That's not in 'current code', is it? How is this not a dependency?

It only depends on it because of order I did changes in, but there is 
really no dependency here. I will send v2 with reversed order.

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>