[PATCH 2/2] ASoC: topology: Only free TLV for volume mixers of a widget

[PATCH 2/2] ASoC: topology: Only free TLV for volume mixers of a widget

[mengdong.lin]

From: Mengdong Lin <mengdong.lin@linux.intel.com>

This patch will check the type of embedded controls for a widget, and
only free the TLV of volume mixers. Bytes controls don't have TLV.

Just free the private value which is used as struct soc_mixer_control
for volume mixers or soc_bytes_ext for bytes controls. No need to cast
to these types before freeing it.

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

diff --git a/include/sound/soc-topology.h b/include/sound/soc-topology.h
index d318fe4..86c884a 100644
--- a/include/sound/soc-topology.h
+++ b/include/sound/soc-topology.h
@@ -53,7 +53,7 @@ struct snd_soc_dobj_control {
 
 /* dynamic widget object */
 struct snd_soc_dobj_widget {
-	unsigned int kcontrol_enum:1;	/* this widget is an enum kcontrol */
+	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 63e1a50..11feb19 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -489,7 +489,7 @@ static void remove_widget(struct snd_soc_component *comp,
 	 * Dynamic Widgets either have 1..N enum kcontrols or mixers.
 	 * The enum may either have an array of values or strings.
 	 */
-	if (dobj->widget.kcontrol_enum) {
+	if (dobj->widget.kcontrol_type == SND_SOC_TPLG_TYPE_ENUM) {
 		/* enumerated widget mixer */
 		for (i = 0; i < w->num_kcontrols; i++) {
 			struct snd_kcontrol *kcontrol = w->kcontrols[i];
@@ -506,16 +506,21 @@ static void remove_widget(struct snd_soc_component *comp,
 		}
 		kfree(w->kcontrol_news);
 	} else {
-		/* non enumerated widget mixer */
+		/* volume mixer or bytes controls */
 		for (i = 0; i < w->num_kcontrols; i++) {
 			struct snd_kcontrol *kcontrol = w->kcontrols[i];
-			struct soc_mixer_control *sm =
-			(struct soc_mixer_control *) kcontrol->private_value;
 
-			kfree(w->kcontrols[i]->tlv.p);
+			if (dobj->widget.kcontrol_type
+			    == SND_SOC_TPLG_TYPE_MIXER)
+				kfree(kcontrol->tlv.p);
 
-			snd_ctl_remove(card, w->kcontrols[i]);
-			kfree(sm);
+			snd_ctl_remove(card, kcontrol);
+
+			/* Private value is used as struct soc_mixer_control
+			 * for volume mixers or soc_bytes_ext for bytes
+			 * controls.
+			 */
+			kfree((void *)kcontrol->private_value);
 		}
 		kfree(w->kcontrol_news);
 	}
@@ -1439,6 +1444,7 @@ 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;
 	int ret = 0;
 
 	if (strnlen(w->name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) ==
@@ -1494,6 +1500,7 @@ static int soc_tplg_dapm_widget_create(struct soc_tplg *tplg,
 	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 = w->num_kcontrols;
 		template.kcontrol_news =
 			soc_tplg_dapm_widget_dmixer_create(tplg,
@@ -1508,7 +1515,7 @@ static int soc_tplg_dapm_widget_create(struct soc_tplg *tplg,
 	case SND_SOC_TPLG_DAPM_CTL_ENUM_DOUBLE:
 	case SND_SOC_TPLG_DAPM_CTL_ENUM_VIRT:
 	case SND_SOC_TPLG_DAPM_CTL_ENUM_VALUE:
-		template.dobj.widget.kcontrol_enum = 1;
+		kcontrol_type = SND_SOC_TPLG_TYPE_ENUM;	/* enumerated mixer */
 		template.num_kcontrols = w->num_kcontrols;
 		template.kcontrol_news =
 			soc_tplg_dapm_widget_denum_create(tplg,
@@ -1519,6 +1526,7 @@ static int soc_tplg_dapm_widget_create(struct soc_tplg *tplg,
 		}
 		break;
 	case SND_SOC_TPLG_CTL_BYTES:
+		kcontrol_type = SND_SOC_TPLG_TYPE_BYTES; /* bytes control */
 		template.num_kcontrols = w->num_kcontrols;
 		template.kcontrol_news =
 			soc_tplg_dapm_widget_dbytes_create(tplg,
@@ -1555,6 +1563,7 @@ 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;
 	widget->dobj.index = tplg->index;
 	kfree(template.sname);
-- 
2.5.0

Re: [PATCH 2/2] ASoC: topology: Only free TLV for volume mixers of a widget

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2210 bytes --]

Hi Mengdong,

[auto build test WARNING on asoc/for-next]
[also build test WARNING on v4.9-rc6 next-20161124]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/mengdong-lin-linux-intel-com/ASoC-topology-Allow-a-widget-to-have-multiple-enum-controls/20161125-164058
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: i386-randconfig-x011-201647 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   sound/soc/soc-topology.c: In function 'soc_tplg_dapm_widget_create':
>> sound/soc/soc-topology.c:1566:36: warning: 'kcontrol_type' may be used uninitialized in this function [-Wmaybe-uninitialized]
     widget->dobj.widget.kcontrol_type = kcontrol_type;
     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~

vim +/kcontrol_type +1566 sound/soc/soc-topology.c

  1550			goto hdr_err;
  1551	
  1552		/* card dapm mutex is held by the core if we are loading topology
  1553		 * data during sound card init. */
  1554		if (card->instantiated)
  1555			widget = snd_soc_dapm_new_control(dapm, &template);
  1556		else
  1557			widget = snd_soc_dapm_new_control_unlocked(dapm, &template);
  1558		if (widget == NULL) {
  1559			dev_err(tplg->dev, "ASoC: failed to create widget %s controls\n",
  1560				w->name);
  1561			ret = -ENOMEM;
  1562			goto hdr_err;
  1563		}
  1564	
  1565		widget->dobj.type = SND_SOC_DOBJ_WIDGET;
> 1566		widget->dobj.widget.kcontrol_type = kcontrol_type;
  1567		widget->dobj.ops = tplg->ops;
  1568		widget->dobj.index = tplg->index;
  1569		kfree(template.sname);
  1570		kfree(template.name);
  1571		list_add(&widget->dobj.list, &tplg->comp->dobj_list);
  1572		return 0;
  1573	
  1574	hdr_err:

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28963 bytes --]

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Applied "ASoC: topology: Only free TLV for volume mixers of a widget" to the asoc tree

[Mark Brown]

The patch

   ASoC: topology: Only free TLV for volume mixers of a widget

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

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

>From eea3dd4f1247aa8654194fb19ade22c94c42e41a Mon Sep 17 00:00:00 2001
From: Mengdong Lin <mengdong.lin@linux.intel.com>
Date: Fri, 25 Nov 2016 16:09:17 +0800
Subject: [PATCH] ASoC: topology: Only free TLV for volume mixers of a widget

This patch will check the type of embedded controls for a widget, and
only free the TLV of volume mixers. Bytes controls don't have TLV.

Just free the private value which is used as struct soc_mixer_control
for volume mixers or soc_bytes_ext for bytes controls. No need to cast
to these types before freeing it.

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 include/sound/soc-topology.h |  2 +-
 sound/soc/soc-topology.c     | 25 +++++++++++++++++--------
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/include/sound/soc-topology.h b/include/sound/soc-topology.h
index b897b9d63161..f9cc7b9271ac 100644
--- a/include/sound/soc-topology.h
+++ b/include/sound/soc-topology.h
@@ -53,7 +53,7 @@ struct snd_soc_dobj_control {
 
 /* dynamic widget object */
 struct snd_soc_dobj_widget {
-	unsigned int kcontrol_enum:1;	/* this widget is an enum kcontrol */
+	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 63e1a50f2161..11feb19e9730 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -489,7 +489,7 @@ static void remove_widget(struct snd_soc_component *comp,
 	 * Dynamic Widgets either have 1..N enum kcontrols or mixers.
 	 * The enum may either have an array of values or strings.
 	 */
-	if (dobj->widget.kcontrol_enum) {
+	if (dobj->widget.kcontrol_type == SND_SOC_TPLG_TYPE_ENUM) {
 		/* enumerated widget mixer */
 		for (i = 0; i < w->num_kcontrols; i++) {
 			struct snd_kcontrol *kcontrol = w->kcontrols[i];
@@ -506,16 +506,21 @@ static void remove_widget(struct snd_soc_component *comp,
 		}
 		kfree(w->kcontrol_news);
 	} else {
-		/* non enumerated widget mixer */
+		/* volume mixer or bytes controls */
 		for (i = 0; i < w->num_kcontrols; i++) {
 			struct snd_kcontrol *kcontrol = w->kcontrols[i];
-			struct soc_mixer_control *sm =
-			(struct soc_mixer_control *) kcontrol->private_value;
 
-			kfree(w->kcontrols[i]->tlv.p);
+			if (dobj->widget.kcontrol_type
+			    == SND_SOC_TPLG_TYPE_MIXER)
+				kfree(kcontrol->tlv.p);
 
-			snd_ctl_remove(card, w->kcontrols[i]);
-			kfree(sm);
+			snd_ctl_remove(card, kcontrol);
+
+			/* Private value is used as struct soc_mixer_control
+			 * for volume mixers or soc_bytes_ext for bytes
+			 * controls.
+			 */
+			kfree((void *)kcontrol->private_value);
 		}
 		kfree(w->kcontrol_news);
 	}
@@ -1439,6 +1444,7 @@ 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;
 	int ret = 0;
 
 	if (strnlen(w->name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) ==
@@ -1494,6 +1500,7 @@ static int soc_tplg_dapm_widget_create(struct soc_tplg *tplg,
 	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 = w->num_kcontrols;
 		template.kcontrol_news =
 			soc_tplg_dapm_widget_dmixer_create(tplg,
@@ -1508,7 +1515,7 @@ static int soc_tplg_dapm_widget_create(struct soc_tplg *tplg,
 	case SND_SOC_TPLG_DAPM_CTL_ENUM_DOUBLE:
 	case SND_SOC_TPLG_DAPM_CTL_ENUM_VIRT:
 	case SND_SOC_TPLG_DAPM_CTL_ENUM_VALUE:
-		template.dobj.widget.kcontrol_enum = 1;
+		kcontrol_type = SND_SOC_TPLG_TYPE_ENUM;	/* enumerated mixer */
 		template.num_kcontrols = w->num_kcontrols;
 		template.kcontrol_news =
 			soc_tplg_dapm_widget_denum_create(tplg,
@@ -1519,6 +1526,7 @@ static int soc_tplg_dapm_widget_create(struct soc_tplg *tplg,
 		}
 		break;
 	case SND_SOC_TPLG_CTL_BYTES:
+		kcontrol_type = SND_SOC_TPLG_TYPE_BYTES; /* bytes control */
 		template.num_kcontrols = w->num_kcontrols;
 		template.kcontrol_news =
 			soc_tplg_dapm_widget_dbytes_create(tplg,
@@ -1555,6 +1563,7 @@ 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;
 	widget->dobj.index = tplg->index;
 	kfree(template.sname);
-- 
2.10.2