[PATCH v3] ASoC: rt1011: Fix 'I2S Reference' enum control caused error

Access to 'I2S Reference' enum causes alsamixer to fail to load: $ alsamixer cannot load mixer controls: Invalid argument
cml_rt1011_rt5682 cml_rt1011_rt5682: control 2:0:0:TL I2S Reference:0: access overflow
The reason is that the original patch adding the code was using ucontrol->value.integer.value[0] instead the correct ucontrol->value.enumerated.item[0]
for an ENUM control.
Fixes: 87f40af26c262 ("ASoC: rt1011: add i2s reference control for rt1011") Reported-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com --- Hi,
Changes since v2: - Fix typo in commit message s/Is@/I2S
Changes since v1: - Correct the ENUM declaration as well
Regards, Peter sound/soc/codecs/rt1011.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/sound/soc/codecs/rt1011.c b/sound/soc/codecs/rt1011.c index 508597866dff..bdfcbb81fa19 100644 --- a/sound/soc/codecs/rt1011.c +++ b/sound/soc/codecs/rt1011.c @@ -1311,12 +1311,11 @@ static int rt1011_r0_load_info(struct snd_kcontrol *kcontrol, .put = rt1011_r0_load_mode_put \ }
-static const char * const rt1011_i2s_ref[] = { +static const char * const rt1011_i2s_ref_texts[] = { "None", "Left Channel", "Right Channel" };
-static SOC_ENUM_SINGLE_DECL(rt1011_i2s_ref_enum, 0, 0, - rt1011_i2s_ref); +static SOC_ENUM_SINGLE_EXT_DECL(rt1011_i2s_ref_enum, rt1011_i2s_ref_texts);
static int rt1011_i2s_ref_put(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) @@ -1325,7 +1324,7 @@ static int rt1011_i2s_ref_put(struct snd_kcontrol *kcontrol, snd_soc_kcontrol_component(kcontrol); struct rt1011_priv *rt1011 = snd_soc_component_get_drvdata(component); - int i2s_ref_ch = ucontrol->value.integer.value[0]; + int i2s_ref_ch = ucontrol->value.enumerated.item[0];
switch (i2s_ref_ch) { case RT1011_I2S_REF_LEFT_CH: @@ -1344,7 +1343,7 @@ static int rt1011_i2s_ref_put(struct snd_kcontrol *kcontrol, dev_info(component->dev, "I2S Reference: Do nothing\n"); }
- rt1011->i2s_ref = ucontrol->value.integer.value[0]; + rt1011->i2s_ref = ucontrol->value.enumerated.item[0];
return 0; } @@ -1357,7 +1356,7 @@ static int rt1011_i2s_ref_get(struct snd_kcontrol *kcontrol, struct rt1011_priv *rt1011 = snd_soc_component_get_drvdata(component);
- ucontrol->value.integer.value[0] = rt1011->i2s_ref; + ucontrol->value.enumerated.item[0] = rt1011->i2s_ref;
return 0; }

Hi,
On 11/10/2021 17:45, Peter Ujfalusi wrote:
Access to 'I2S Reference' enum causes alsamixer to fail to load: $ alsamixer cannot load mixer controls: Invalid argument
cml_rt1011_rt5682 cml_rt1011_rt5682: control 2:0:0:TL I2S Reference:0: access overflow
The reason is that the original patch adding the code was using ucontrol->value.integer.value[0] instead the correct ucontrol->value.enumerated.item[0]
for an ENUM control.
Fixes: 87f40af26c262 ("ASoC: rt1011: add i2s reference control for rt1011") Reported-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com
Hi,
Changes since v2:
- Fix typo in commit message s/Is@/I2S
Changes since v1:
- Correct the ENUM declaration as well
After a third look, 87f40af26c262 appears mostly a broken patch, it will take a bit more patching to get it right.
I will send a new version with different subject to fix it, or it can be reverted (yes, it is that broken).
Regards, Peter sound/soc/codecs/rt1011.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/sound/soc/codecs/rt1011.c b/sound/soc/codecs/rt1011.c index 508597866dff..bdfcbb81fa19 100644 --- a/sound/soc/codecs/rt1011.c +++ b/sound/soc/codecs/rt1011.c @@ -1311,12 +1311,11 @@ static int rt1011_r0_load_info(struct snd_kcontrol *kcontrol, .put = rt1011_r0_load_mode_put \ }
-static const char * const rt1011_i2s_ref[] = { +static const char * const rt1011_i2s_ref_texts[] = { "None", "Left Channel", "Right Channel" };
-static SOC_ENUM_SINGLE_DECL(rt1011_i2s_ref_enum, 0, 0,
- rt1011_i2s_ref);
+static SOC_ENUM_SINGLE_EXT_DECL(rt1011_i2s_ref_enum, rt1011_i2s_ref_texts);
static int rt1011_i2s_ref_put(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) @@ -1325,7 +1324,7 @@ static int rt1011_i2s_ref_put(struct snd_kcontrol *kcontrol, snd_soc_kcontrol_component(kcontrol); struct rt1011_priv *rt1011 = snd_soc_component_get_drvdata(component);
- int i2s_ref_ch = ucontrol->value.integer.value[0];
int i2s_ref_ch = ucontrol->value.enumerated.item[0];
switch (i2s_ref_ch) { case RT1011_I2S_REF_LEFT_CH:
@@ -1344,7 +1343,7 @@ static int rt1011_i2s_ref_put(struct snd_kcontrol *kcontrol, dev_info(component->dev, "I2S Reference: Do nothing\n"); }
- rt1011->i2s_ref = ucontrol->value.integer.value[0];
rt1011->i2s_ref = ucontrol->value.enumerated.item[0];
return 0;
} @@ -1357,7 +1356,7 @@ static int rt1011_i2s_ref_get(struct snd_kcontrol *kcontrol, struct rt1011_priv *rt1011 = snd_soc_component_get_drvdata(component);
- ucontrol->value.integer.value[0] = rt1011->i2s_ref;
ucontrol->value.enumerated.item[0] = rt1011->i2s_ref;
return 0;
}

On Mon, 11 Oct 2021 17:45:18 +0300, Peter Ujfalusi wrote:
Access to 'I2S Reference' enum causes alsamixer to fail to load: $ alsamixer cannot load mixer controls: Invalid argument
cml_rt1011_rt5682 cml_rt1011_rt5682: control 2:0:0:TL I2S Reference:0: access overflow
The reason is that the original patch adding the code was using ucontrol->value.integer.value[0] instead the correct ucontrol->value.enumerated.item[0]
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: rt1011: Fix 'I2S Reference' enum control caused error commit: c3de683c4d1d68ff27f21606b921d92ffdea3352
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

Hi Mark,
On 12/10/2021 14:45, Mark Brown wrote:
On Mon, 11 Oct 2021 17:45:18 +0300, Peter Ujfalusi wrote:
Access to 'I2S Reference' enum causes alsamixer to fail to load: $ alsamixer cannot load mixer controls: Invalid argument
cml_rt1011_rt5682 cml_rt1011_rt5682: control 2:0:0:TL I2S Reference:0: access overflow
The reason is that the original patch adding the code was using ucontrol->value.integer.value[0] instead the correct ucontrol->value.enumerated.item[0]
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: rt1011: Fix 'I2S Reference' enum control caused error commit: c3de683c4d1d68ff27f21606b921d92ffdea3352
I have noted that this patch is not enough to fix the i2s reference support and a complete patch has been already sent:
https://lore.kernel.org/alsa-devel/20211012063113.3754-1-peter.ujfalusi@linu...
What keyword should I use next time to 'block' a patch applied? Fwiw, this was my note: https://lore.kernel.org/alsa-devel/e18ce962-736c-ea17-5ac2-1330026cdc90@linu...
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

On Tue, Oct 12, 2021 at 02:52:11PM +0300, Péter Ujfalusi wrote:
On 12/10/2021 14:45, Mark Brown wrote:
[1/1] ASoC: rt1011: Fix 'I2S Reference' enum control caused error commit: c3de683c4d1d68ff27f21606b921d92ffdea3352
I have noted that this patch is not enough to fix the i2s reference support and a complete patch has been already sent:
https://lore.kernel.org/alsa-devel/20211012063113.3754-1-peter.ujfalusi@linu...
Are you sure this isn't just b4 thinking your later version matches the earlier version when it's the later version that got applied (you'll have got multiple mails with one for the later version as well)?
What keyword should I use next time to 'block' a patch applied?
You can't, there's a gap between me queueing things and testing and pushing out and mail sent in that time might not get seen.
You should also take care that when you're sending things you're doing so in a standard fashion, occasionally I have seen people bury things in the middle of threads or whatever which causes b4 to think an earlier version is actually a later one.

On 13/10/2021 14:52, Mark Brown wrote:
On Tue, Oct 12, 2021 at 02:52:11PM +0300, Péter Ujfalusi wrote:
On 12/10/2021 14:45, Mark Brown wrote:
[1/1] ASoC: rt1011: Fix 'I2S Reference' enum control caused error commit: c3de683c4d1d68ff27f21606b921d92ffdea3352
I have noted that this patch is not enough to fix the i2s reference support and a complete patch has been already sent:
https://lore.kernel.org/alsa-devel/20211012063113.3754-1-peter.ujfalusi@linu...
Are you sure this isn't just b4 thinking your later version matches the earlier version when it's the later version that got applied (you'll have got multiple mails with one for the later version as well)?
linux-next has this v3 and not the the proper fix sent a bit later
What keyword should I use next time to 'block' a patch applied?
You can't, there's a gap between me queueing things and testing and pushing out and mail sent in that time might not get seen.
You should also take care that when you're sending things you're doing so in a standard fashion, occasionally I have seen people bury things in the middle of threads or whatever which causes b4 to think an earlier version is actually a later one.
I don't send patches as reply but in this particular case I did changed the commit subject since the original commit adding the i2s reference selection was mostly broken.
I can send an updated patch on top of next, but the one we have applied only fixes the alsamixer crash, the code remains broken.

On Wed, Oct 13, 2021 at 03:13:14PM +0300, Péter Ujfalusi wrote:
On 13/10/2021 14:52, Mark Brown wrote:
You should also take care that when you're sending things you're doing so in a standard fashion, occasionally I have seen people bury things in the middle of threads or whatever which causes b4 to think an earlier version is actually a later one.
I don't send patches as reply but in this particular case I did changed the commit subject since the original commit adding the i2s reference selection was mostly broken.
Oh, if you change the commit subject and reset back to version 1 that's not going to help anything. I imagine that what's happened is that when cleaning up the old version I'll have deleted the new patch and kept v3.
I can send an updated patch on top of next, but the one we have applied only fixes the alsamixer crash, the code remains broken.
Yes, please send an incremental fix.
participants (2)
-
Mark Brown
-
Peter Ujfalusi