[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABDcavbRryENG58LO7+gbJeKbBBPP1uG1Xc00yXUBNzspWpWoA@mail.gmail.com>
Date: Thu, 24 Nov 2022 14:54:15 +0100
From: Guillermo Rodriguez Garcia <guille.rodriguez@...il.com>
To: Charles Keepax <ckeepax@...nsource.cirrus.com>
Cc: linux-kernel@...r.kernel.org,
Tan Nayır <tannayir@...il.com>,
Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
Banajit Goswami <bgoswami@...eaurora.org>,
Richard Fitzgerald <rf@...nsource.cirrus.com>,
Lucas Tanure <tanureal@...nsource.cirrus.com>,
David Rhodes <david.rhodes@...rus.com>,
James Schulman <james.schulman@...rus.com>,
patches@...nsource.cirrus.com, alsa-devel@...a-project.org,
Mark Brown <broonie@...nel.org>
Subject: Re: SOC_DOUBLE_R_SX_TLV controls broken in cs24l51 driver
El jue, 24 nov 2022 a las 12:13, Charles Keepax
(<ckeepax@...nsource.cirrus.com>) escribió:
>
> On Thu, Nov 24, 2022 at 10:57:34AM +0100, Guillermo Rodriguez Garcia wrote:
> > Hi all,
> >
> > I am using a dev board with a Cirrus Logic cs24l51 codec.
> >
> > This used to work fine prior to kernel version 5.x, however after 5.x
> > it is not possible to set certain values for ALSA controls from
> > userspace.
> >
> > I believe this is related to the input validation that is mentioned in
> > this thread: https://lore.kernel.org/all/Yph8C3bRxcr6ogW7@sirena.org.uk/T/,
> > and possibly in this commit: 4f1e50d6a9cf9c1b8c859d449b5031cacfa8404e
> > ("ASoC: ops: Reject out of bounds values in snd_soc_put_volsw_sx()")
> >
> > For the cs24l51, all the controls that fail are using the
> > SOC_DOUBLE_R_SX_TLV macro.
> >
> > I have traced this to the checks in snd_soc_put_volsw_sx, specifically
> > the (val > max - min) check:
> >
>
> Can you try these two patches:
>
> https://lore.kernel.org/all/165236477046.1016627.15470197691244479154.b4-ty@kernel.org/
Thanks.
In my tests, these patches seem to fix the problem for some values,
but not for all of them:
$ amixer cset name='Analog Playback Volume' '208','208'
numid=3,iface=MIXER,name='Analog Playback Volume'
; type=INTEGER,access=rw---R--,values=2,min=0,max=228,step=0
: values=208,208
| dBscale-min=-102.00dB,step=0.50dB,mute=0
$ amixer cset name='Analog Playback Volume' '180','180'
amixer: Control default element write error: Invalid argument
Looking at the code I'd say that patch 1/2 is correct however I have
doubts about patch 2/2:
val_mask = mask << rshift;
val2 = (ucontrol->value.integer.value[1] + min) & mask;
+
+ if (mc->platform_max && val2 > mc->platform_max)
+ return -EINVAL;
+ if (val2 > max)
+ return -EINVAL;
+
val2 = val2 << rshift;
err = snd_soc_component_update_bits(component, reg2, val_mask,
The checks for max and platform_max are done on val2, but val2 is
already the result of adding the minimum value ('min') and applying
the mask.
Shouldn't the checks be done on ucontrol->value.integer.value[1] instead?
Thanks,
Guillermo Rodriguez Garcia
guille.rodriguez@...il.com
Powered by blists - more mailing lists