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

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

[Peter Ujfalusi]

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;
 }
-- 
2.33.0

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

[Péter Ujfalusi]

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;
>  }
> 

-- 
Péter

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

[Mark Brown]

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

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

[Péter Ujfalusi]

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@linux.intel.com/

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@linux.intel.com/

> 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
> 

-- 
Péter

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

[Mark Brown]

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

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@linux.intel.com/

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Péter Ujfalusi]

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@linux.intel.com/
> 
> 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.

-- 
Péter

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

[Mark Brown]

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

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]