Re: Semantics for _SX controls

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

 





On 01/06/2022 16:24, Mark Brown wrote:
Hi,

The fixes for input validation have highlighed a bunch of
problems with the _SX controls.  These aren't widely used, it's
just some Cirrus Logic and Qualcomm CODECs plus the obsolte
ab8500 driver, but it has become apparent that at least some of
the drivers were relying on the lack of validation we used to
have to work.

For the Cirrus case 34198710f55b5f ("ASoC: Add info callback for
SX_TLV controls") says that the intended semantic is

     Currently every CODEC implementing these controls specifies the minimum
     as the non-zero value for the minimum and the maximum as the number of
     gain settings available.

which was from Charles at Cirrus so hopefully that's the semantic
used by Cirrus drivers.  However Tan Nayir pointed at some out of
tree patches from Qualcomm which change the info callback such
that max is instead the maximum register value that can be
written and reports that since we started validating written
values properly they can no longer set some valid values in some
controls on at least some Qualcomm devices (this was a wcd9340)
which clearly indicates that there's some problems that need
fixing.

This isn't helped by the fact that snd_soc_info_volsw_sx() is
implemented by calling snd_soc_info_volsw() and then applying an
offset to the maximum value which makes things harder to follow -
I think we should make snd_soc_info_volsw_sx() a standalone
function, and we probably need some fixes in some combination of
the shared functions and the drivers as well.

Can people please check how the values in the current drivers
correspond to what the actual bitfields in the registers have so
we can get a handle on what's going on here?  I've pasted a quick
grep for _SX controls below:

From Qualcomm codecs side of it usage of SOC_SINGLE_SX_TLV should be moved to SOC_SINGLE_S8_TLV() in sound/soc/codecs/msm8916-wcd-digital.c and sound/soc/codecs/wcd9335.c. using SOC_SINGLE_SX_TLV() will result in incorrect volume settings.

All of these gains have signed 7th bit in 8 bit register with gain range from -84dB to +40dB in 1dB steps.

Other WCD Codecs seems to have done correctly using S8_TLV.

I can send a patch to fix these.

Thanks,
Srini



sound/soc/codecs/ab8500-codec.c:	SOC_SINGLE_XR_SX("ANC Warp Delay Shift",
sound/soc/codecs/ab8500-codec.c:	SOC_SINGLE_XR_SX("ANC FIR Output Shift",
sound/soc/codecs/ab8500-codec.c:	SOC_SINGLE_XR_SX("ANC IIR Output Shift",
sound/soc/codecs/ab8500-codec.c:	SOC_SINGLE_XR_SX("ANC Warp Delay",
sound/soc/codecs/cs35l33.c:	SOC_SINGLE_SX_TLV("DAC Volume", CS35L33_DIG_VOL_CTL,
sound/soc/codecs/cs35l34.c:	SOC_SINGLE_SX_TLV("Digital Volume", CS35L34_AMP_DIG_VOL,
sound/soc/codecs/cs35l35.c:	SOC_SINGLE_SX_TLV("Digital Audio Volume", CS35L35_AMP_DIG_VOL,
sound/soc/codecs/cs35l35.c:	SOC_SINGLE_SX_TLV("Digital Advisory Volume", CS35L35_ADV_DIG_VOL,
sound/soc/codecs/cs35l36.c:	SOC_SINGLE_SX_TLV("Digital PCM Volume", CS35L36_AMP_DIG_VOL_CTRL,
sound/soc/codecs/cs35l41.c:	SOC_SINGLE_SX_TLV("Digital PCM Volume", CS35L41_AMP_DIG_VOL_CTRL,
sound/soc/codecs/cs4265.c:	SOC_DOUBLE_R_SX_TLV("PGA Volume", CS4265_CHA_PGA_CTL,
sound/soc/codecs/cs42l51.c:	SOC_DOUBLE_R_SX_TLV("PCM Playback Volume",
sound/soc/codecs/cs42l51.c:	SOC_DOUBLE_R_SX_TLV("Analog Playback Volume",
sound/soc/codecs/cs42l51.c:	SOC_DOUBLE_R_SX_TLV("ADC Mixer Volume",
sound/soc/codecs/cs42l51.c:	SOC_DOUBLE_R_SX_TLV("ADC Attenuator Volume",
sound/soc/codecs/cs42l51.c:	SOC_DOUBLE_R_SX_TLV("PGA Volume",
sound/soc/codecs/cs42l52.c:	SOC_DOUBLE_R_SX_TLV("Master Volume", CS42L52_MASTERA_VOL,
sound/soc/codecs/cs42l52.c:	SOC_DOUBLE_R_SX_TLV("Headphone Volume", CS42L52_HPA_VOL,
sound/soc/codecs/cs42l52.c:	SOC_DOUBLE_R_SX_TLV("Speaker Volume", CS42L52_SPKA_VOL,
sound/soc/codecs/cs42l52.c:	SOC_DOUBLE_R_SX_TLV("Bypass Volume", CS42L52_PASSTHRUA_VOL,
sound/soc/codecs/cs42l52.c:	SOC_DOUBLE_R_SX_TLV("ADC Volume", CS42L52_ADCA_VOL,
sound/soc/codecs/cs42l52.c:	SOC_DOUBLE_R_SX_TLV("ADC Mixer Volume",
sound/soc/codecs/cs42l52.c:	SOC_DOUBLE_R_SX_TLV("PGA Volume", CS42L52_PGAA_CTL,
sound/soc/codecs/cs42l52.c:	SOC_DOUBLE_R_SX_TLV("PCM Mixer Volume",
sound/soc/codecs/cs42l52.c:	SOC_SINGLE_SX_TLV("Beep Volume", CS42L52_BEEP_VOL,
sound/soc/codecs/cs42l56.c:	SOC_DOUBLE_R_SX_TLV("Master Volume", CS42L56_MASTER_A_VOLUME,
sound/soc/codecs/cs42l56.c:	SOC_DOUBLE_R_SX_TLV("ADC Mixer Volume", CS42L56_ADCA_MIX_VOLUME,
sound/soc/codecs/cs42l56.c:	SOC_DOUBLE_R_SX_TLV("PCM Mixer Volume", CS42L56_PCMA_MIX_VOLUME,
sound/soc/codecs/cs42l56.c:	SOC_DOUBLE_R_SX_TLV("PGA Volume", CS42L56_PGAA_MUX_VOLUME,
sound/soc/codecs/cs42l56.c:	SOC_DOUBLE_R_SX_TLV("Headphone Volume", CS42L56_HPA_VOLUME,
sound/soc/codecs/cs42l56.c:	SOC_DOUBLE_R_SX_TLV("LineOut Volume", CS42L56_LOA_VOLUME,
sound/soc/codecs/cs42l56.c:	SOC_SINGLE_SX_TLV("Beep Volume", CS42L56_BEEP_FREQ_OFFTIME,
sound/soc/codecs/cs42l73.c:	SOC_DOUBLE_R_SX_TLV("Headphone Analog Playback Volume",
sound/soc/codecs/cs42l73.c:	SOC_DOUBLE_R_SX_TLV("LineOut Analog Playback Volume", CS42L73_LOAAVOL,
sound/soc/codecs/cs42l73.c:	SOC_DOUBLE_R_SX_TLV("Input PGA Analog Volume", CS42L73_MICAPREPGAAVOL,
sound/soc/codecs/cs42l73.c:	SOC_DOUBLE_R_SX_TLV("Input Path Digital Volume", CS42L73_IPADVOL,
sound/soc/codecs/cs42l73.c:	SOC_DOUBLE_R_SX_TLV("HL Digital Playback Volume",
sound/soc/codecs/cs42l73.c:	SOC_SINGLE_SX_TLV("Speakerphone Digital Volume",
sound/soc/codecs/cs42l73.c:	SOC_SINGLE_SX_TLV("Ear Speaker Digital Volume",
sound/soc/codecs/cs53l30.c:	SOC_SINGLE_SX_TLV("ADC1A PGA Volume",
sound/soc/codecs/cs53l30.c:	SOC_SINGLE_SX_TLV("ADC1B PGA Volume",
sound/soc/codecs/cs53l30.c:	SOC_SINGLE_SX_TLV("ADC2A PGA Volume",
sound/soc/codecs/cs53l30.c:	SOC_SINGLE_SX_TLV("ADC2B PGA Volume",
sound/soc/codecs/cs53l30.c:	SOC_SINGLE_SX_TLV("ADC1A Digital Volume",
sound/soc/codecs/cs53l30.c:	SOC_SINGLE_SX_TLV("ADC1B Digital Volume",
sound/soc/codecs/cs53l30.c:	SOC_SINGLE_SX_TLV("ADC2A Digital Volume",
sound/soc/codecs/cs53l30.c:	SOC_SINGLE_SX_TLV("ADC2B Digital Volume",
sound/soc/codecs/msm8916-wcd-digital.c:	SOC_SINGLE_SX_TLV("IIR1 INP1 Volume", LPASS_CDC_IIR1_GAIN_B1_CTL,
sound/soc/codecs/msm8916-wcd-digital.c:	SOC_SINGLE_SX_TLV("IIR1 INP2 Volume", LPASS_CDC_IIR1_GAIN_B2_CTL,
sound/soc/codecs/msm8916-wcd-digital.c:	SOC_SINGLE_SX_TLV("IIR1 INP3 Volume", LPASS_CDC_IIR1_GAIN_B3_CTL,
sound/soc/codecs/msm8916-wcd-digital.c:	SOC_SINGLE_SX_TLV("IIR1 INP4 Volume", LPASS_CDC_IIR1_GAIN_B4_CTL,
sound/soc/codecs/msm8916-wcd-digital.c:	SOC_SINGLE_SX_TLV("IIR2 INP1 Volume", LPASS_CDC_IIR2_GAIN_B1_CTL,
sound/soc/codecs/msm8916-wcd-digital.c:	SOC_SINGLE_SX_TLV("IIR2 INP2 Volume", LPASS_CDC_IIR2_GAIN_B2_CTL,
sound/soc/codecs/msm8916-wcd-digital.c:	SOC_SINGLE_SX_TLV("IIR2 INP3 Volume", LPASS_CDC_IIR2_GAIN_B3_CTL,
sound/soc/codecs/msm8916-wcd-digital.c:	SOC_SINGLE_SX_TLV("IIR2 INP4 Volume", LPASS_CDC_IIR2_GAIN_B4_CTL,
sound/soc/codecs/wcd9335.c:	SOC_SINGLE_SX_TLV("RX0 Digital Volume", WCD9335_CDC_RX0_RX_VOL_CTL,
sound/soc/codecs/wcd9335.c:	SOC_SINGLE_SX_TLV("RX1 Digital Volume", WCD9335_CDC_RX1_RX_VOL_CTL,
sound/soc/codecs/wcd9335.c:	SOC_SINGLE_SX_TLV("RX2 Digital Volume", WCD9335_CDC_RX2_RX_VOL_CTL,
sound/soc/codecs/wcd9335.c:	SOC_SINGLE_SX_TLV("RX3 Digital Volume", WCD9335_CDC_RX3_RX_VOL_CTL,
sound/soc/codecs/wcd9335.c:	SOC_SINGLE_SX_TLV("RX4 Digital Volume", WCD9335_CDC_RX4_RX_VOL_CTL,
sound/soc/codecs/wcd9335.c:	SOC_SINGLE_SX_TLV("RX5 Digital Volume", WCD9335_CDC_RX5_RX_VOL_CTL,
sound/soc/codecs/wcd9335.c:	SOC_SINGLE_SX_TLV("RX6 Digital Volume", WCD9335_CDC_RX6_RX_VOL_CTL,
sound/soc/codecs/wcd9335.c:	SOC_SINGLE_SX_TLV("RX7 Digital Volume", WCD9335_CDC_RX7_RX_VOL_CTL,
sound/soc/codecs/wcd9335.c:	SOC_SINGLE_SX_TLV("RX8 Digital Volume", WCD9335_CDC_RX8_RX_VOL_CTL,
sound/soc/codecs/wcd9335.c:	SOC_SINGLE_SX_TLV("RX0 Mix Digital Volume",
sound/soc/codecs/wcd9335.c:	SOC_SINGLE_SX_TLV("RX1 Mix Digital Volume",
sound/soc/codecs/wcd9335.c:	SOC_SINGLE_SX_TLV("RX2 Mix Digital Volume",
sound/soc/codecs/wcd9335.c:	SOC_SINGLE_SX_TLV("RX3 Mix Digital Volume",
sound/soc/codecs/wcd9335.c:	SOC_SINGLE_SX_TLV("RX4 Mix Digital Volume",
sound/soc/codecs/wcd9335.c:	SOC_SINGLE_SX_TLV("RX5 Mix Digital Volume",
sound/soc/codecs/wcd9335.c:	SOC_SINGLE_SX_TLV("RX6 Mix Digital Volume",
sound/soc/codecs/wcd9335.c:	SOC_SINGLE_SX_TLV("RX7 Mix Digital Volume",
sound/soc/codecs/wcd9335.c:	SOC_SINGLE_SX_TLV("RX8 Mix Digital Volume",

Thanks,
Mark



[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