[alsa-devel] [PATCH 0/4] ASoC: dapm: Various fixes for dapm kcontrols
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(-)
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
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; }
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
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;
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;
participants (2)
-
Charles Keepax -
Chen-Yu Tsai