Re: [PATCH] ASoC: ops: Don't modify the driver's plaform_max when reading state

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On 20. 8. 2022, at 0:38, Mark Brown <broonie@xxxxxxxxxx> wrote:
> 
> On Fri, Aug 19, 2022 at 06:17:25PM +0200, Martin Povišer wrote:
>>> On 3. 6. 2022, at 13:25, Mark Brown <broonie@xxxxxxxxxx> wrote:
> 
>>> This means that platform_max is no longer treated as a direct register
>>> value for controls were min is non-zero. The put() callbacks already
>>> validate on this basis, and there do not appear to be any in tree users
>>> that would be affected.
> 
>> At least ‘put_volsw' seem to validate on the other conflicting interpretation
>> of platform_max [as was introduced in commit 9bdd10d57a88 (“ASoC: ops:
>> Shift tested values in snd_soc_put_volsw() by +min”)].
> 
> Ugh, so it does.  The patchwork of reuse and differing interpretations
> of these controls is causing all sorts of confusion :/
> 
>> Also, the soc.h definitions of SOC_SINGLE_*/SOC_DOUBLE_* set platform_max
>> to the register maximum, again interpreting platform_max the other way.
> 
> That use of platform_max has been removed since it was just obviously
> not sensible anyway, the whole purpose of platform_max is to override
> what was set in the control so having both max and platform_max set is
> just redundant and causing confusion.

Right, I was about to submit removal of it myself to address the issues
that surfaced with this patch, then I saw there’s more confusion with
platform_max. Anyway I see the removal’s been done in 26bdcc4b.

>>> Signed-off-by: Mark Brown <broonie@xxxxxxxxxx>
> 
>> This commit breaks controls with non-zero minimum.
> 
> Could you specify more exactly how it does that, and indeed where we
> have non-zero minimums - all the info callbacks report 0 as a minimum as
> far as I can see?  Life would be a lot simpler if the controls had all
> been defined to just be the register values, I've never been able to
> figure out why they're anything else since the ABI for controls supports
> negative values just fine.

Sure. What I meant are non-zero register value minimums, especially
negative ones, and the breaking was in interaction with the default
platform_max values from SOC_SINGLE_*/SOC_DOUBLE_*.

Taking for example

    SOC_SINGLE_S8_TLV("ADC Volume", CS42L42_ADC_VOLUME, -97, 12, adc_tlv),

of codecs/cs42l42.c, platform_max was set to 12 and the register value
was then clipped to -97..-85.

So this should be fixed by the removal of the default platform_max,
leaving us with few discrepancies that only manifest if platform_max
is being actively used (and in that only on controls with non-zero
register minimum).

Martin





[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux