[PATCH 0/4] ASoC: dapm: Various fixes for dapm kcontrols

[PATCH 0/4] ASoC: dapm: Various fixes for dapm kcontrols

[Chen-Yu Tsai]

Hi everyone,

This series is a bunch of fixes for some problems I encountered while
experimenting with various dapm kcontrols for Allwinner A31 support
in sun4i-codec.

Patch 1 fixes a possible uninitialized variable usage in
snd_soc_dapm_get_volsw(). This doesn't show up as a compiler warning
but I'm fairly certain it might happen.

Patch 2 fixes setting the value for an enum kcontrol's second channel.

Patch 3 fixes kcontrol creation for output driver widgets. An output
driver widget is the same as a PGA widget, just with a different
priority.

Patch 4 adds an error message if anyone attempts to add hardware backed
kcontrols on PGA widgets. Originally kcontrols on PGAs weren't supported
at all. Support for them on virtual widgets were added later, but a guard
for real widgets was missing.


Regards
ChenYu


Chen-Yu Tsai (4):
  ASoC: dapm: Fix possible uninitialized variable in
    snd_soc_dapm_get_volsw()
  ASoC: dapm: Fix value setting for _ENUM_DOUBLE MUX's second channel
  ASoC: dapm: Fix kcontrol creation for output driver widget
  ASoC: dapm: Give error when adding hardware backed controls for PGA
    widgets

 sound/soc/soc-dapm.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

-- 
2.9.3

[PATCH 1/4] ASoC: dapm: Fix possible uninitialized variable in snd_soc_dapm_get_volsw()

[Chen-Yu Tsai]

If soc_dapm_read() fails, val will be uninitialized, and bogus values
will be written later:

	ret = soc_dapm_read(dapm, reg, &val);
	val = (val >> shift) & mask;

However, the compiler does not give a warning. Return on error before
val is really used to avoid this.

This is similar to the commit 6912831623c5 ("ASoC: dapm: Fix
uninitialized variable in snd_soc_dapm_get_enum_double()")

Fixes: ce0fc93ae56e ("ASoC: Add DAPM support at the component level")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 sound/soc/soc-dapm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 8698c26773b3..0b442fee0b62 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -3049,6 +3049,9 @@ int snd_soc_dapm_get_volsw(struct snd_kcontrol *kcontrol,
 	}
 	mutex_unlock(&card->dapm_mutex);
 
+	if (ret)
+		return ret;
+
 	if (invert)
 		ucontrol->value.integer.value[0] = max - val;
 	else
-- 
2.9.3

[PATCH 2/4] ASoC: dapm: Fix value setting for _ENUM_DOUBLE MUX's second channel

[Chen-Yu Tsai]

The value for the second channel in _ENUM_DOUBLE (double channel) MUXs
is not correctly updated, due to using the wrong bit shift.

Use the correct bit shift, so both channels toggle together.

Fixes: 3727b4968453 ("ASoC: dapm: Consolidate MUXs and value MUXs")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 sound/soc/soc-dapm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 0b442fee0b62..9be076bc14b5 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -3203,7 +3203,7 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
 	if (e->shift_l != e->shift_r) {
 		if (item[1] > e->items)
 			return -EINVAL;
-		val |= snd_soc_enum_item_to_val(e, item[1]) << e->shift_l;
+		val |= snd_soc_enum_item_to_val(e, item[1]) << e->shift_r;
 		mask |= e->mask << e->shift_r;
 	}
 
-- 
2.9.3

[PATCH 3/4] ASoC: dapm: Fix kcontrol creation for output driver widget

[Chen-Yu Tsai]

Commit d88429a695a4 ("ASoC: dapm: Add output driver widget") added
the snd_soc_dapm_out_drv ID for the output driver widget, which is
the same as the PGA widget, with a later power sequence number.

Commit 19a2557b76d6 ("ASoC: dapm: Add kcontrol support for PGAs")
then added kcontrol support for PGA widgets, but failed to account
for output driver widgets. Attempts to use kcontrols with output
driver widgets result in silent failures, with the developer having
little idea about what went on.

Add snd_soc_dapm_out_drv to the switch/case block under snd_soc_dapm_pga
in dapm_create_or_share_kcontrol, since they are essentially the same.

Fixes: 19a2557b76d6 ("ASoC: dapm: Add kcontrol support for PGAs")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 sound/soc/soc-dapm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 9be076bc14b5..ed6b707cc7ce 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -823,6 +823,7 @@ static int dapm_create_or_share_kcontrol(struct snd_soc_dapm_widget *w,
 			case snd_soc_dapm_switch:
 			case snd_soc_dapm_mixer:
 			case snd_soc_dapm_pga:
+			case snd_soc_dapm_out_drv:
 				wname_in_long_name = true;
 				kcname_in_long_name = true;
 				break;
-- 
2.9.3

[PATCH 4/4] ASoC: dapm: Give error when adding hardware backed controls for PGA widgets

[Chen-Yu Tsai]

Commit 19a2557b76d6 ("ASoC: dapm: Add kcontrol support for PGAs") added
kcontrol support for PGA widgets, but did not add necessary changes to
snd_soc_dapm_put_volsw or soc_dapm_mixer_update_power to have hardware
register updates actually happen.

As this is a little used feature, this patch just adds an error message
when someone tries to use hardware backed controls with PGA or output
driver widgets.

Fixes: 19a2557b76d6 ("ASoC: dapm: Add kcontrol support for PGAs")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 sound/soc/soc-dapm.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index ed6b707cc7ce..f681a251d766 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -984,6 +984,14 @@ static int dapm_new_pga(struct snd_soc_dapm_widget *w)
 	int i, ret;
 
 	for (i = 0; i < w->num_kcontrols; i++) {
+		const struct soc_mixer_control *mc =
+			(struct soc_mixer_control *)w->kcontrol_news[i].private_value;
+
+		if (mc->reg != SND_SOC_NOPM)
+			dev_err(w->dapm->dev,
+				"ASoC: hardware backed PGA controls not supported: '%s'\n",
+				w->name);
+
 		ret = dapm_create_or_share_kcontrol(w, i);
 		if (ret < 0)
 			return ret;
-- 
2.9.3

Re: [alsa-devel] [PATCH 2/4] ASoC: dapm: Fix value setting for _ENUM_DOUBLE MUX's second channel

[Charles Keepax]

On Sat, Aug 27, 2016 at 07:27:59PM +0800, Chen-Yu Tsai wrote:
> The value for the second channel in _ENUM_DOUBLE (double channel) MUXs
> is not correctly updated, due to using the wrong bit shift.
> 
> Use the correct bit shift, so both channels toggle together.
> 
> Fixes: 3727b4968453 ("ASoC: dapm: Consolidate MUXs and value MUXs")
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---

Reviewed-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

Thanks,
Charles