On Sat, Aug 20, 2022 at 03:10:51AM +0200, Martin Povišer wrote:
On 20. 8. 2022, at 0:38, Mark Brown broonie@kernel.org 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.