[PATCH 00/11] ASoC: topology: Fixes and cleanups
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(-)
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
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!
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),
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
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);
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);
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: /*
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)
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);
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,
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 */
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);
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;
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.
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(-)
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
participants (4)
-
Amadeusz Sławiński -
Jaroslav Kysela -
Mark Brown -
Pierre-Louis Bossart