[PATCH v5] ASoC: topology: Add support for multiple kcontrol types to a widget
Current dapm widget has a single variable to describe its kcontrol's type. As there can be many kcontrols in one widget it is inherently presumed that the types are the same.
Lately there has been use cases where different types of kcontrols would be needed for a single widget. Thus add pointer to dapm widget to hold an array for different kcontrol types and modify the kcontrol creation to operate in a loop based on individual kcontrol type.
Change control creation and deletion to use individual kcontrol types in SOF driver. This is done in the same patch for not breaking bisect. SOF driver is also currently the only one using the dapm widget kcontrol_type.
Signed-off-by: Jaska Uimonen jaska.uimonen@linux.intel.com Reviewed-by: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com ---
Hi,
This is a patch for enabling multiple different types of kcontrols for a dapm widget. It refactors the dapm kcontrol parsing by lifting the creation loop and memory allocation up one level and making the type variable to hold multiple type values. Sof driver is modified to use this type information in the same patch as it is quite cumbersome to split the changes nicely.
changes since v4: - change title from dapm -> topology - remove cover letter - cc maintainers, me learning list behavior, apologies for the noise
changes since v3: - fix sparse warnings for not using le32_to_cpu
changes since v2: - only 1 patch for not breaking bisect - sof internal reviews -> minor changes + reviewed by tags - no upstream comments after v2 -> assuming I'm on the right track here
Changes since v1: - don't add kcontrol type to core i.e. to kcontrol and kcontrol_news - extend existing dapm widget kcontrol_type into holding an array of types
include/sound/soc-topology.h | 2 +- sound/soc/soc-topology.c | 467 +++++++++++++++++------------------ sound/soc/sof/topology.c | 15 +- 3 files changed, 234 insertions(+), 250 deletions(-)
diff --git a/include/sound/soc-topology.h b/include/sound/soc-topology.h index 328cf763d9b4..4afd667e124c 100644 --- a/include/sound/soc-topology.h +++ b/include/sound/soc-topology.h @@ -54,7 +54,7 @@ struct snd_soc_dobj_control {
/* dynamic widget object */ struct snd_soc_dobj_widget { - unsigned int kcontrol_type; /* kcontrol type: mixer, enum, bytes */ + unsigned int *kcontrol_type; /* kcontrol type: mixer, enum, bytes */ };
/* generic dynamic object - all dynamic objects belong to this struct */ diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index 73076d425efb..2a10bf2cfb00 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -1203,249 +1203,216 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg, return ret; }
-static struct snd_kcontrol_new *soc_tplg_dapm_widget_dmixer_create( - struct soc_tplg *tplg, int num_kcontrols) +static int soc_tplg_dapm_widget_dmixer_create(struct soc_tplg *tplg, struct snd_kcontrol_new *kc) { - struct snd_kcontrol_new *kc; struct soc_mixer_control *sm; struct snd_soc_tplg_mixer_control *mc; - int i, err; - - kc = devm_kcalloc(tplg->dev, num_kcontrols, sizeof(*kc), GFP_KERNEL); - if (kc == NULL) - return NULL; - - for (i = 0; i < num_kcontrols; i++) { - mc = (struct snd_soc_tplg_mixer_control *)tplg->pos; - - /* validate kcontrol */ - if (strnlen(mc->hdr.name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) == - SNDRV_CTL_ELEM_ID_NAME_MAXLEN) - goto err_sm; + int err;
- sm = devm_kzalloc(tplg->dev, sizeof(*sm), GFP_KERNEL); - if (sm == NULL) - goto err_sm; + mc = (struct snd_soc_tplg_mixer_control *)tplg->pos;
- tplg->pos += (sizeof(struct snd_soc_tplg_mixer_control) + - le32_to_cpu(mc->priv.size)); + /* validate kcontrol */ + if (strnlen(mc->hdr.name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) == + SNDRV_CTL_ELEM_ID_NAME_MAXLEN) + return -EINVAL;
- dev_dbg(tplg->dev, " adding DAPM widget mixer control %s at %d\n", - mc->hdr.name, i); + sm = devm_kzalloc(tplg->dev, sizeof(*sm), GFP_KERNEL); + if (!sm) + return -ENOMEM;
- kc[i].private_value = (long)sm; - kc[i].name = devm_kstrdup(tplg->dev, mc->hdr.name, GFP_KERNEL); - if (kc[i].name == NULL) - goto err_sm; - kc[i].iface = SNDRV_CTL_ELEM_IFACE_MIXER; - kc[i].access = le32_to_cpu(mc->hdr.access); + tplg->pos += sizeof(struct snd_soc_tplg_mixer_control) + + le32_to_cpu(mc->priv.size);
- /* 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); + dev_dbg(tplg->dev, " adding DAPM widget mixer control %s\n", + mc->hdr.name);
- sm->max = le32_to_cpu(mc->max); - sm->min = le32_to_cpu(mc->min); - sm->invert = le32_to_cpu(mc->invert); - sm->platform_max = le32_to_cpu(mc->platform_max); - sm->dobj.index = tplg->index; - INIT_LIST_HEAD(&sm->dobj.list); - - /* map io handlers */ - err = soc_tplg_kcontrol_bind_io(&mc->hdr, &kc[i], tplg); - if (err) { - soc_control_err(tplg, &mc->hdr, mc->hdr.name); - goto err_sm; - } + kc->private_value = (long)sm; + kc->name = devm_kstrdup(tplg->dev, mc->hdr.name, GFP_KERNEL); + if (!kc->name) + return -ENOMEM; + kc->iface = SNDRV_CTL_ELEM_IFACE_MIXER; + 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->max = le32_to_cpu(mc->max); + sm->min = le32_to_cpu(mc->min); + sm->invert = le32_to_cpu(mc->invert); + sm->platform_max = le32_to_cpu(mc->platform_max); + sm->dobj.index = tplg->index; + INIT_LIST_HEAD(&sm->dobj.list); + + /* map io handlers */ + err = soc_tplg_kcontrol_bind_io(&mc->hdr, kc, tplg); + if (err) { + soc_control_err(tplg, &mc->hdr, mc->hdr.name); + return err; + }
- /* create any TLV data */ - err = soc_tplg_create_tlv(tplg, &kc[i], &mc->hdr); - if (err < 0) { - dev_err(tplg->dev, "ASoC: failed to create TLV %s\n", - mc->hdr.name); - goto err_sm; - } + /* create any TLV data */ + err = soc_tplg_create_tlv(tplg, kc, &mc->hdr); + if (err < 0) { + dev_err(tplg->dev, "ASoC: failed to create TLV %s\n", + mc->hdr.name); + return err; + }
- /* pass control to driver for optional further init */ - err = soc_tplg_init_kcontrol(tplg, &kc[i], - (struct snd_soc_tplg_ctl_hdr *)mc); - if (err < 0) { - dev_err(tplg->dev, "ASoC: failed to init %s\n", - mc->hdr.name); - goto err_sm; - } + /* pass control to driver for optional further init */ + err = soc_tplg_init_kcontrol(tplg, kc, + (struct snd_soc_tplg_ctl_hdr *)mc); + if (err < 0) { + dev_err(tplg->dev, "ASoC: failed to init %s\n", + mc->hdr.name); + return err; } - return kc;
-err_sm: - return NULL; + return 0; }
-static struct snd_kcontrol_new *soc_tplg_dapm_widget_denum_create( - struct soc_tplg *tplg, int num_kcontrols) +static int soc_tplg_dapm_widget_denum_create(struct soc_tplg *tplg, struct snd_kcontrol_new *kc) { - struct snd_kcontrol_new *kc; struct snd_soc_tplg_enum_control *ec; struct soc_enum *se; - int i, err; - - kc = devm_kcalloc(tplg->dev, num_kcontrols, sizeof(*kc), GFP_KERNEL); - if (kc == NULL) - return NULL; - - for (i = 0; i < num_kcontrols; i++) { - ec = (struct snd_soc_tplg_enum_control *)tplg->pos; - /* validate kcontrol */ - if (strnlen(ec->hdr.name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) == - SNDRV_CTL_ELEM_ID_NAME_MAXLEN) - goto err_se; + int err;
- se = devm_kzalloc(tplg->dev, sizeof(*se), GFP_KERNEL); - if (se == NULL) - goto err_se; + ec = (struct snd_soc_tplg_enum_control *)tplg->pos; + /* validate kcontrol */ + if (strnlen(ec->hdr.name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) == + SNDRV_CTL_ELEM_ID_NAME_MAXLEN) + return -EINVAL;
- tplg->pos += (sizeof(struct snd_soc_tplg_enum_control) + - le32_to_cpu(ec->priv.size)); + se = devm_kzalloc(tplg->dev, sizeof(*se), GFP_KERNEL); + if (!se) + return -ENOMEM;
- dev_dbg(tplg->dev, " adding DAPM widget enum control %s\n", - ec->hdr.name); + tplg->pos += (sizeof(struct snd_soc_tplg_enum_control) + + le32_to_cpu(ec->priv.size));
- kc[i].private_value = (long)se; - kc[i].name = devm_kstrdup(tplg->dev, ec->hdr.name, GFP_KERNEL); - if (kc[i].name == NULL) - goto err_se; - kc[i].iface = SNDRV_CTL_ELEM_IFACE_MIXER; - kc[i].access = le32_to_cpu(ec->hdr.access); + dev_dbg(tplg->dev, " adding DAPM widget enum control %s\n", + ec->hdr.name);
- /* 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, - SNDRV_CHMAP_FL); - se->shift_r = tplc_chan_get_shift(tplg, ec->channel, - SNDRV_CHMAP_FR); + kc->private_value = (long)se; + kc->name = devm_kstrdup(tplg->dev, ec->hdr.name, GFP_KERNEL); + if (!kc->name) + return -ENOMEM; + kc->iface = SNDRV_CTL_ELEM_IFACE_MIXER; + kc->access = le32_to_cpu(ec->hdr.access);
- se->items = le32_to_cpu(ec->items); - se->mask = le32_to_cpu(ec->mask); - se->dobj.index = tplg->index; + /* 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, + SNDRV_CHMAP_FL); + se->shift_r = tplc_chan_get_shift(tplg, ec->channel, + SNDRV_CHMAP_FR);
- switch (le32_to_cpu(ec->hdr.ops.info)) { - case SND_SOC_TPLG_CTL_ENUM_VALUE: - case SND_SOC_TPLG_DAPM_CTL_ENUM_VALUE: - err = soc_tplg_denum_create_values(tplg, se, ec); - if (err < 0) { - dev_err(tplg->dev, "ASoC: could not create values for %s\n", - ec->hdr.name); - goto err_se; - } - fallthrough; - case SND_SOC_TPLG_CTL_ENUM: - case SND_SOC_TPLG_DAPM_CTL_ENUM_DOUBLE: - case SND_SOC_TPLG_DAPM_CTL_ENUM_VIRT: - err = soc_tplg_denum_create_texts(tplg, se, ec); - if (err < 0) { - dev_err(tplg->dev, "ASoC: could not create texts for %s\n", - ec->hdr.name); - goto err_se; - } - break; - default: - dev_err(tplg->dev, "ASoC: invalid enum control type %d for %s\n", - ec->hdr.ops.info, ec->hdr.name); - goto err_se; - } + se->items = le32_to_cpu(ec->items); + se->mask = le32_to_cpu(ec->mask); + se->dobj.index = tplg->index;
- /* map io handlers */ - err = soc_tplg_kcontrol_bind_io(&ec->hdr, &kc[i], tplg); - if (err) { - soc_control_err(tplg, &ec->hdr, ec->hdr.name); - goto err_se; + switch (le32_to_cpu(ec->hdr.ops.info)) { + case SND_SOC_TPLG_CTL_ENUM_VALUE: + case SND_SOC_TPLG_DAPM_CTL_ENUM_VALUE: + err = soc_tplg_denum_create_values(tplg, se, ec); + if (err < 0) { + dev_err(tplg->dev, "ASoC: could not create values for %s\n", + ec->hdr.name); + return err; } - - /* pass control to driver for optional further init */ - err = soc_tplg_init_kcontrol(tplg, &kc[i], - (struct snd_soc_tplg_ctl_hdr *)ec); + fallthrough; + case SND_SOC_TPLG_CTL_ENUM: + case SND_SOC_TPLG_DAPM_CTL_ENUM_DOUBLE: + case SND_SOC_TPLG_DAPM_CTL_ENUM_VIRT: + err = soc_tplg_denum_create_texts(tplg, se, ec); if (err < 0) { - dev_err(tplg->dev, "ASoC: failed to init %s\n", + dev_err(tplg->dev, "ASoC: could not create texts for %s\n", ec->hdr.name); - goto err_se; + return err; } + break; + default: + dev_err(tplg->dev, "ASoC: invalid enum control type %d for %s\n", + ec->hdr.ops.info, ec->hdr.name); + return -EINVAL; }
- return kc; + /* map io handlers */ + err = soc_tplg_kcontrol_bind_io(&ec->hdr, kc, tplg); + if (err) { + soc_control_err(tplg, &ec->hdr, ec->hdr.name); + return err; + }
-err_se: - return NULL; + /* pass control to driver for optional further init */ + err = soc_tplg_init_kcontrol(tplg, kc, + (struct snd_soc_tplg_ctl_hdr *)ec); + if (err < 0) { + dev_err(tplg->dev, "ASoC: failed to init %s\n", + ec->hdr.name); + return err; + } + + return 0; }
-static struct snd_kcontrol_new *soc_tplg_dapm_widget_dbytes_create( - struct soc_tplg *tplg, int num_kcontrols) +static int soc_tplg_dapm_widget_dbytes_create(struct soc_tplg *tplg, struct snd_kcontrol_new *kc) { struct snd_soc_tplg_bytes_control *be; struct soc_bytes_ext *sbe; - struct snd_kcontrol_new *kc; - int i, err; - - kc = devm_kcalloc(tplg->dev, num_kcontrols, sizeof(*kc), GFP_KERNEL); - if (!kc) - return NULL; + int err;
- for (i = 0; i < num_kcontrols; i++) { - be = (struct snd_soc_tplg_bytes_control *)tplg->pos; + be = (struct snd_soc_tplg_bytes_control *)tplg->pos;
- /* validate kcontrol */ - if (strnlen(be->hdr.name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) == - SNDRV_CTL_ELEM_ID_NAME_MAXLEN) - goto err_sbe; + /* validate kcontrol */ + if (strnlen(be->hdr.name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) == + SNDRV_CTL_ELEM_ID_NAME_MAXLEN) + return -EINVAL;
- sbe = devm_kzalloc(tplg->dev, sizeof(*sbe), GFP_KERNEL); - if (sbe == NULL) - goto err_sbe; + sbe = devm_kzalloc(tplg->dev, sizeof(*sbe), GFP_KERNEL); + if (!sbe) + return -ENOMEM;
- tplg->pos += (sizeof(struct snd_soc_tplg_bytes_control) + - le32_to_cpu(be->priv.size)); + tplg->pos += (sizeof(struct snd_soc_tplg_bytes_control) + + le32_to_cpu(be->priv.size));
- dev_dbg(tplg->dev, - "ASoC: adding bytes kcontrol %s with access 0x%x\n", - be->hdr.name, be->hdr.access); + dev_dbg(tplg->dev, + "ASoC: adding bytes kcontrol %s with access 0x%x\n", + be->hdr.name, be->hdr.access);
- kc[i].private_value = (long)sbe; - kc[i].name = devm_kstrdup(tplg->dev, be->hdr.name, GFP_KERNEL); - if (kc[i].name == NULL) - goto err_sbe; - kc[i].iface = SNDRV_CTL_ELEM_IFACE_MIXER; - kc[i].access = le32_to_cpu(be->hdr.access); + kc->private_value = (long)sbe; + kc->name = devm_kstrdup(tplg->dev, be->hdr.name, GFP_KERNEL); + if (!kc->name) + return -ENOMEM; + kc->iface = SNDRV_CTL_ELEM_IFACE_MIXER; + kc->access = le32_to_cpu(be->hdr.access);
- sbe->max = le32_to_cpu(be->max); - INIT_LIST_HEAD(&sbe->dobj.list); + sbe->max = le32_to_cpu(be->max); + INIT_LIST_HEAD(&sbe->dobj.list);
- /* map standard io handlers and check for external handlers */ - err = soc_tplg_kcontrol_bind_io(&be->hdr, &kc[i], tplg); - if (err) { - soc_control_err(tplg, &be->hdr, be->hdr.name); - goto err_sbe; - } - - /* pass control to driver for optional further init */ - err = soc_tplg_init_kcontrol(tplg, &kc[i], - (struct snd_soc_tplg_ctl_hdr *)be); - if (err < 0) { - dev_err(tplg->dev, "ASoC: failed to init %s\n", - be->hdr.name); - goto err_sbe; - } + /* map standard io handlers and check for external handlers */ + err = soc_tplg_kcontrol_bind_io(&be->hdr, kc, tplg); + if (err) { + soc_control_err(tplg, &be->hdr, be->hdr.name); + return err; }
- return kc; - -err_sbe: + /* pass control to driver for optional further init */ + err = soc_tplg_init_kcontrol(tplg, kc, + (struct snd_soc_tplg_ctl_hdr *)be); + if (err < 0) { + dev_err(tplg->dev, "ASoC: failed to init %s\n", + be->hdr.name); + return err; + }
- return NULL; + return 0; }
static int soc_tplg_dapm_widget_create(struct soc_tplg *tplg, @@ -1455,8 +1422,13 @@ static int soc_tplg_dapm_widget_create(struct soc_tplg *tplg, struct snd_soc_dapm_widget template, *widget; struct snd_soc_tplg_ctl_hdr *control_hdr; struct snd_soc_card *card = tplg->comp->card; - unsigned int kcontrol_type; + unsigned int *kcontrol_type; + struct snd_kcontrol_new *kc; + int mixer_count = 0; + int bytes_count = 0; + int enum_count = 0; int ret = 0; + int i;
if (strnlen(w->name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) == SNDRV_CTL_ELEM_ID_NAME_MAXLEN) @@ -1499,7 +1471,6 @@ static int soc_tplg_dapm_widget_create(struct soc_tplg *tplg, le32_to_cpu(w->priv.size));
if (w->num_kcontrols == 0) { - kcontrol_type = 0; template.num_kcontrols = 0; goto widget; } @@ -1508,57 +1479,66 @@ static int soc_tplg_dapm_widget_create(struct soc_tplg *tplg, dev_dbg(tplg->dev, "ASoC: template %s has %d controls of type %x\n", w->name, w->num_kcontrols, control_hdr->type);
- switch (le32_to_cpu(control_hdr->ops.info)) { - case SND_SOC_TPLG_CTL_VOLSW: - case SND_SOC_TPLG_CTL_STROBE: - case SND_SOC_TPLG_CTL_VOLSW_SX: - case SND_SOC_TPLG_CTL_VOLSW_XR_SX: - case SND_SOC_TPLG_CTL_RANGE: - case SND_SOC_TPLG_DAPM_CTL_VOLSW: - kcontrol_type = SND_SOC_TPLG_TYPE_MIXER; /* volume mixer */ - template.num_kcontrols = le32_to_cpu(w->num_kcontrols); - template.kcontrol_news = - soc_tplg_dapm_widget_dmixer_create(tplg, - template.num_kcontrols); - if (!template.kcontrol_news) { - ret = -ENOMEM; - goto hdr_err; - } - break; - case SND_SOC_TPLG_CTL_ENUM: - case SND_SOC_TPLG_CTL_ENUM_VALUE: - case SND_SOC_TPLG_DAPM_CTL_ENUM_DOUBLE: - case SND_SOC_TPLG_DAPM_CTL_ENUM_VIRT: - case SND_SOC_TPLG_DAPM_CTL_ENUM_VALUE: - kcontrol_type = SND_SOC_TPLG_TYPE_ENUM; /* enumerated mixer */ - template.num_kcontrols = le32_to_cpu(w->num_kcontrols); - template.kcontrol_news = - soc_tplg_dapm_widget_denum_create(tplg, - template.num_kcontrols); - if (!template.kcontrol_news) { - ret = -ENOMEM; - goto hdr_err; - } - break; - case SND_SOC_TPLG_CTL_BYTES: - kcontrol_type = SND_SOC_TPLG_TYPE_BYTES; /* bytes control */ - template.num_kcontrols = le32_to_cpu(w->num_kcontrols); - template.kcontrol_news = - soc_tplg_dapm_widget_dbytes_create(tplg, - template.num_kcontrols); - if (!template.kcontrol_news) { - ret = -ENOMEM; + template.num_kcontrols = le32_to_cpu(w->num_kcontrols); + kc = devm_kcalloc(tplg->dev, le32_to_cpu(w->num_kcontrols), sizeof(*kc), GFP_KERNEL); + if (!kc) + goto err; + + kcontrol_type = devm_kcalloc(tplg->dev, le32_to_cpu(w->num_kcontrols), sizeof(unsigned int), + GFP_KERNEL); + if (!kcontrol_type) + goto err; + + for (i = 0; i < w->num_kcontrols; i++) { + control_hdr = (struct snd_soc_tplg_ctl_hdr *)tplg->pos; + switch (le32_to_cpu(control_hdr->ops.info)) { + case SND_SOC_TPLG_CTL_VOLSW: + case SND_SOC_TPLG_CTL_STROBE: + case SND_SOC_TPLG_CTL_VOLSW_SX: + case SND_SOC_TPLG_CTL_VOLSW_XR_SX: + case SND_SOC_TPLG_CTL_RANGE: + case SND_SOC_TPLG_DAPM_CTL_VOLSW: + /* volume mixer */ + kc[i].index = mixer_count; + kcontrol_type[i] = SND_SOC_TPLG_TYPE_MIXER; + mixer_count++; + ret = soc_tplg_dapm_widget_dmixer_create(tplg, &kc[i]); + if (ret < 0) + goto hdr_err; + break; + case SND_SOC_TPLG_CTL_ENUM: + case SND_SOC_TPLG_CTL_ENUM_VALUE: + case SND_SOC_TPLG_DAPM_CTL_ENUM_DOUBLE: + case SND_SOC_TPLG_DAPM_CTL_ENUM_VIRT: + case SND_SOC_TPLG_DAPM_CTL_ENUM_VALUE: + /* enumerated mixer */ + kc[i].index = enum_count; + kcontrol_type[i] = SND_SOC_TPLG_TYPE_ENUM; + enum_count++; + ret = soc_tplg_dapm_widget_denum_create(tplg, &kc[i]); + if (ret < 0) + goto hdr_err; + break; + case SND_SOC_TPLG_CTL_BYTES: + /* bytes control */ + kc[i].index = bytes_count; + kcontrol_type[i] = SND_SOC_TPLG_TYPE_BYTES; + bytes_count++; + ret = soc_tplg_dapm_widget_dbytes_create(tplg, &kc[i]); + if (ret < 0) + goto hdr_err; + break; + default: + dev_err(tplg->dev, "ASoC: invalid widget control type %d:%d:%d\n", + control_hdr->ops.get, control_hdr->ops.put, + le32_to_cpu(control_hdr->ops.info)); + ret = -EINVAL; goto hdr_err; } - break; - default: - dev_err(tplg->dev, "ASoC: invalid widget control type %d:%d:%d\n", - control_hdr->ops.get, control_hdr->ops.put, - le32_to_cpu(control_hdr->ops.info)); - ret = -EINVAL; - goto hdr_err; }
+ template.kcontrol_news = kc; + widget: ret = soc_tplg_widget_load(tplg, &template, w); if (ret < 0) @@ -1595,6 +1575,7 @@ static int soc_tplg_dapm_widget_create(struct soc_tplg *tplg, snd_soc_dapm_free_widget(widget); hdr_err: kfree(template.sname); + kfree(kc); err: kfree(template.name); return ret; diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index 59abcfc9bd55..92d346bbd357 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -1063,6 +1063,7 @@ static int sof_control_load_volume(struct snd_soc_component *scomp, scontrol->min_volume_step = le32_to_cpu(mc->min); scontrol->max_volume_step = le32_to_cpu(mc->max); scontrol->num_channels = le32_to_cpu(mc->num_channels); + scontrol->control_data->index = kc->index;
/* set cmd for mixer control */ if (le32_to_cpu(mc->max) == 1) { @@ -1140,7 +1141,7 @@ static int sof_control_load_enum(struct snd_soc_component *scomp,
scontrol->comp_id = sdev->next_comp_id; scontrol->num_channels = le32_to_cpu(ec->num_channels); - + scontrol->control_data->index = kc->index; scontrol->cmd = SOF_CTRL_CMD_ENUM;
dev_dbg(scomp->dev, "tplg: load kcontrol index %d chans %d comp_id %d\n", @@ -1188,6 +1189,7 @@ static int sof_control_load_bytes(struct snd_soc_component *scomp,
scontrol->comp_id = sdev->next_comp_id; scontrol->cmd = SOF_CTRL_CMD_BINARY; + scontrol->control_data->index = kc->index;
dev_dbg(scomp->dev, "tplg: load kcontrol index %d chans %d\n", scontrol->comp_id, scontrol->num_channels); @@ -2133,7 +2135,7 @@ static int sof_get_control_data(struct snd_soc_component *scomp, for (i = 0; i < widget->num_kcontrols; i++) { kc = &widget->kcontrol_news[i];
- switch (widget->dobj.widget.kcontrol_type) { + switch (widget->dobj.widget.kcontrol_type[i]) { case SND_SOC_TPLG_TYPE_MIXER: sm = (struct soc_mixer_control *)kc->private_value; wdata[i].control = sm->dobj.private; @@ -2147,8 +2149,8 @@ static int sof_get_control_data(struct snd_soc_component *scomp, wdata[i].control = se->dobj.private; break; default: - dev_err(scomp->dev, "error: unknown kcontrol type %d in widget %s\n", - widget->dobj.widget.kcontrol_type, + dev_err(scomp->dev, "error: unknown kcontrol type %u in widget %s\n", + widget->dobj.widget.kcontrol_type[i], widget->name); return -EINVAL; } @@ -2164,7 +2166,8 @@ static int sof_get_control_data(struct snd_soc_component *scomp, return -EINVAL;
/* make sure data is valid - data can be updated at runtime */ - if (wdata[i].pdata->magic != SOF_ABI_MAGIC) + if (widget->dobj.widget.kcontrol_type[i] == SND_SOC_TPLG_TYPE_BYTES && + wdata[i].pdata->magic != SOF_ABI_MAGIC) return -EINVAL;
*size += wdata[i].pdata->size; @@ -2605,7 +2608,7 @@ static int sof_widget_unload(struct snd_soc_component *scomp, } for (i = 0; i < widget->num_kcontrols; i++) { kc = &widget->kcontrol_news[i]; - switch (dobj->widget.kcontrol_type) { + switch (widget->dobj.widget.kcontrol_type[i]) { case SND_SOC_TPLG_TYPE_MIXER: sm = (struct soc_mixer_control *)kc->private_value; scontrol = sm->dobj.private;
Hi Jaska,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on asoc/for-next] [also build test WARNING on v5.12 next-20210506] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Jaska-Uimonen/ASoC-topology-Add-sup... base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next config: i386-allyesconfig (attached as .config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
cocci warnings: (new ones prefixed by >>)
sound/soc/soc-topology.c:1578:1-6: WARNING: invalid free of devm_ allocated data
vim +1578 sound/soc/soc-topology.c
1417 1418 static int soc_tplg_dapm_widget_create(struct soc_tplg *tplg, 1419 struct snd_soc_tplg_dapm_widget *w) 1420 { 1421 struct snd_soc_dapm_context *dapm = &tplg->comp->dapm; 1422 struct snd_soc_dapm_widget template, *widget; 1423 struct snd_soc_tplg_ctl_hdr *control_hdr; 1424 struct snd_soc_card *card = tplg->comp->card; 1425 unsigned int *kcontrol_type; 1426 struct snd_kcontrol_new *kc; 1427 int mixer_count = 0; 1428 int bytes_count = 0; 1429 int enum_count = 0; 1430 int ret = 0; 1431 int i; 1432 1433 if (strnlen(w->name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) == 1434 SNDRV_CTL_ELEM_ID_NAME_MAXLEN) 1435 return -EINVAL; 1436 if (strnlen(w->sname, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) == 1437 SNDRV_CTL_ELEM_ID_NAME_MAXLEN) 1438 return -EINVAL; 1439 1440 dev_dbg(tplg->dev, "ASoC: creating DAPM widget %s id %d\n", 1441 w->name, w->id); 1442 1443 memset(&template, 0, sizeof(template)); 1444 1445 /* map user to kernel widget ID */ 1446 template.id = get_widget_id(le32_to_cpu(w->id)); 1447 if ((int)template.id < 0) 1448 return template.id; 1449 1450 /* strings are allocated here, but used and freed by the widget */ 1451 template.name = kstrdup(w->name, GFP_KERNEL); 1452 if (!template.name) 1453 return -ENOMEM; 1454 template.sname = kstrdup(w->sname, GFP_KERNEL); 1455 if (!template.sname) { 1456 ret = -ENOMEM; 1457 goto err; 1458 } 1459 template.reg = le32_to_cpu(w->reg); 1460 template.shift = le32_to_cpu(w->shift); 1461 template.mask = le32_to_cpu(w->mask); 1462 template.subseq = le32_to_cpu(w->subseq); 1463 template.on_val = w->invert ? 0 : 1; 1464 template.off_val = w->invert ? 1 : 0; 1465 template.ignore_suspend = le32_to_cpu(w->ignore_suspend); 1466 template.event_flags = le16_to_cpu(w->event_flags); 1467 template.dobj.index = tplg->index; 1468 1469 tplg->pos += 1470 (sizeof(struct snd_soc_tplg_dapm_widget) + 1471 le32_to_cpu(w->priv.size)); 1472 1473 if (w->num_kcontrols == 0) { 1474 template.num_kcontrols = 0; 1475 goto widget; 1476 } 1477 1478 control_hdr = (struct snd_soc_tplg_ctl_hdr *)tplg->pos; 1479 dev_dbg(tplg->dev, "ASoC: template %s has %d controls of type %x\n", 1480 w->name, w->num_kcontrols, control_hdr->type); 1481 1482 template.num_kcontrols = le32_to_cpu(w->num_kcontrols); 1483 kc = devm_kcalloc(tplg->dev, le32_to_cpu(w->num_kcontrols), sizeof(*kc), GFP_KERNEL); 1484 if (!kc) 1485 goto err; 1486 1487 kcontrol_type = devm_kcalloc(tplg->dev, le32_to_cpu(w->num_kcontrols), sizeof(unsigned int), 1488 GFP_KERNEL); 1489 if (!kcontrol_type) 1490 goto err; 1491 1492 for (i = 0; i < w->num_kcontrols; i++) { 1493 control_hdr = (struct snd_soc_tplg_ctl_hdr *)tplg->pos; 1494 switch (le32_to_cpu(control_hdr->ops.info)) { 1495 case SND_SOC_TPLG_CTL_VOLSW: 1496 case SND_SOC_TPLG_CTL_STROBE: 1497 case SND_SOC_TPLG_CTL_VOLSW_SX: 1498 case SND_SOC_TPLG_CTL_VOLSW_XR_SX: 1499 case SND_SOC_TPLG_CTL_RANGE: 1500 case SND_SOC_TPLG_DAPM_CTL_VOLSW: 1501 /* volume mixer */ 1502 kc[i].index = mixer_count; 1503 kcontrol_type[i] = SND_SOC_TPLG_TYPE_MIXER; 1504 mixer_count++; 1505 ret = soc_tplg_dapm_widget_dmixer_create(tplg, &kc[i]); 1506 if (ret < 0) 1507 goto hdr_err; 1508 break; 1509 case SND_SOC_TPLG_CTL_ENUM: 1510 case SND_SOC_TPLG_CTL_ENUM_VALUE: 1511 case SND_SOC_TPLG_DAPM_CTL_ENUM_DOUBLE: 1512 case SND_SOC_TPLG_DAPM_CTL_ENUM_VIRT: 1513 case SND_SOC_TPLG_DAPM_CTL_ENUM_VALUE: 1514 /* enumerated mixer */ 1515 kc[i].index = enum_count; 1516 kcontrol_type[i] = SND_SOC_TPLG_TYPE_ENUM; 1517 enum_count++; 1518 ret = soc_tplg_dapm_widget_denum_create(tplg, &kc[i]); 1519 if (ret < 0) 1520 goto hdr_err; 1521 break; 1522 case SND_SOC_TPLG_CTL_BYTES: 1523 /* bytes control */ 1524 kc[i].index = bytes_count; 1525 kcontrol_type[i] = SND_SOC_TPLG_TYPE_BYTES; 1526 bytes_count++; 1527 ret = soc_tplg_dapm_widget_dbytes_create(tplg, &kc[i]); 1528 if (ret < 0) 1529 goto hdr_err; 1530 break; 1531 default: 1532 dev_err(tplg->dev, "ASoC: invalid widget control type %d:%d:%d\n", 1533 control_hdr->ops.get, control_hdr->ops.put, 1534 le32_to_cpu(control_hdr->ops.info)); 1535 ret = -EINVAL; 1536 goto hdr_err; 1537 } 1538 } 1539 1540 template.kcontrol_news = kc; 1541 1542 widget: 1543 ret = soc_tplg_widget_load(tplg, &template, w); 1544 if (ret < 0) 1545 goto hdr_err; 1546 1547 /* card dapm mutex is held by the core if we are loading topology 1548 * data during sound card init. */ 1549 if (card->instantiated) 1550 widget = snd_soc_dapm_new_control(dapm, &template); 1551 else 1552 widget = snd_soc_dapm_new_control_unlocked(dapm, &template); 1553 if (IS_ERR(widget)) { 1554 ret = PTR_ERR(widget); 1555 goto hdr_err; 1556 } 1557 1558 widget->dobj.type = SND_SOC_DOBJ_WIDGET; 1559 widget->dobj.widget.kcontrol_type = kcontrol_type; 1560 widget->dobj.ops = tplg->ops; 1561 widget->dobj.index = tplg->index; 1562 list_add(&widget->dobj.list, &tplg->comp->dobj_list); 1563 1564 ret = soc_tplg_widget_ready(tplg, widget, w); 1565 if (ret < 0) 1566 goto ready_err; 1567 1568 kfree(template.sname); 1569 kfree(template.name); 1570 1571 return 0; 1572 1573 ready_err: 1574 remove_widget(widget->dapm->component, &widget->dobj, SOC_TPLG_PASS_WIDGET); 1575 snd_soc_dapm_free_widget(widget); 1576 hdr_err: 1577 kfree(template.sname);
1578 kfree(kc);
1579 err: 1580 kfree(template.name); 1581 return ret; 1582 } 1583
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
participants (2)
-
Jaska Uimonen -
kernel test robot