On Thu, Nov 24, 2022 at 02:54:15PM +0100, Guillermo Rodriguez Garcia wrote: > El jue, 24 nov 2022 a las 12:13, Charles Keepax > (<ckeepax@xxxxxxxxxxxxxxxxxxxxx>) 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@xxxxxxxxxxxxx/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@xxxxxxxxxx/ > > 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? > Yeah they definitely should, I have resent the two patches including that fixup, lets see what Mark says. You are CCed on them so be great if you could give them a test too. Thanks, Charles
