| Message ID | 20211011144518.2518-1-peter.ujfalusi@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c3de683c4d1d68ff27f21606b921d92ffdea3352 |
| Headers | show |
| Series | [v3] ASoC: rt1011: Fix 'I2S Reference' enum control caused error | expand |
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@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 >
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.
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.
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.
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; }
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(-)