On Sat, Aug 20, 2022 at 03:10:51AM +0200, Martin Povišer wrote:
> > On 20. 8. 2022, at 0:38, Mark Brown <broonie@xxxxxxxxxx> wrote:
> >> 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_*.
Ah, you mean the register field side - like I say the actual controls
themselves are always zero referenced.
> 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).
But only for controls using snd_soc_info_volsw(), not for controls in
general. We need to figure out if we want platform_max to be a the
register or user facing value since that's the confusion here and bring
things into line. Looking at the info callbacks _info_volsw() is
currently handling platform_max relative to the user visible control,
_info_volsw_sx() is too in that it doesn't support a non-zero register
minimum and _xr_sx() doesn't do platform_max at all which is fun.
We also still have snd_soc_info_volsw_range() modifying the control's
platform_max which ought to be fixed, it doesn't support non-zero
register minimums either.
I'm in two minds here, user facing feels cleaner but we had a long time
where the code was mostly doing register values and I think some out of
tree Qualcomm stuff that assumed that (not just machine drivers but core
changes) were using that. The usuability does feel like a bit of a toss
up.
Attachment:
signature.asc
Description: PGP signature
