[PATCH 0/6] Fixup Cirrus SX control usage
Mostly the usage of the SX controls seems to match the lowest gain value + number of gain levels expected. The one notable exception there being cs53l30 as David noted. However, there are a couple of other places where the minimum value/TLVs are slightly incorrectly specified.
There are two I couldn't quite work out yet, any input on these greatly appreciated:
SOC_DOUBLE_R_SX_TLV("Speaker Volume", CS42L52_SPKA_VOL, CS42L52_SPKB_VOL, 0, 0x40, 0xC0, hl_tlv),
0.5dB step 0x40 - -96dB 0x00 - 0dB
This one is weird, the datasheet says 0x40 is the minimum, which matches, but the datasheet also says 0.5dB steps to 0x00 being 0dB which doesn't match with 0x40 being the minimum, or 0xC0 being the number of levels. Some clarification will needed from the hardware guys.
SOC_SINGLE_SX_TLV("Beep Volume", CS42L56_BEEP_FREQ_OFFTIME, 0, 0x07, 0x23, beep_tlv),
2dB step 0x07 - -50dB 0x00 - 0dB 0x06 - +6dB
The minimum checks out at 0x07, but rest of this is weird. The values in the datasheet don't make sense, based on starting at -50dB with a 2dB step, 0x06 should equate to 12dB, over 31 volume levels, not 6dB over 35 volume levels as stated. The control specifies 0x23(35) levels, which takes us up to a final value of 0x0A at 20dB. Again some clarification from the hardware guys might be needed here.
Finally it is worth noting I don't actually have hardware for most of these devices, so mostly this is just build tested.
Thanks, Charles
Charles Keepax (6): ASoC: cs42l52: Fix TLV scales for mixer controls ASoC: cs35l36: Update digital volume TLV ASoC: cs53l30: Correct number of volume levels on SX controls ASoC: cs42l52: Correct TLV for Bypass Volume ASoC: cs42l56: Correct typo in minimum level for SX volume controls ASoC: cs42l51: Correct minimum value for SX volume control
sound/soc/codecs/cs35l36.c | 3 ++- sound/soc/codecs/cs42l51.c | 2 +- sound/soc/codecs/cs42l52.c | 8 +++++--- sound/soc/codecs/cs42l56.c | 4 ++-- sound/soc/codecs/cs53l30.c | 16 ++++++++-------- 5 files changed, 18 insertions(+), 15 deletions(-)
The datasheet specifies the range of the mixer volumes as between -51.5dB and 12dB with a 0.5dB step. Update the TLVs for this.
Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com --- sound/soc/codecs/cs42l52.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/cs42l52.c b/sound/soc/codecs/cs42l52.c index 9b182b585be4c..02c25399cf8a2 100644 --- a/sound/soc/codecs/cs42l52.c +++ b/sound/soc/codecs/cs42l52.c @@ -137,7 +137,7 @@ static DECLARE_TLV_DB_SCALE(mic_tlv, 1600, 100, 0);
static DECLARE_TLV_DB_SCALE(pga_tlv, -600, 50, 0);
-static DECLARE_TLV_DB_SCALE(mix_tlv, -50, 50, 0); +static DECLARE_TLV_DB_SCALE(mix_tlv, -5150, 50, 0);
static DECLARE_TLV_DB_SCALE(beep_tlv, -56, 200, 0);
@@ -364,7 +364,7 @@ static const struct snd_kcontrol_new cs42l52_snd_controls[] = { CS42L52_ADCB_VOL, 0, 0xA0, 0x78, ipd_tlv), SOC_DOUBLE_R_SX_TLV("ADC Mixer Volume", CS42L52_ADCA_MIXER_VOL, CS42L52_ADCB_MIXER_VOL, - 0, 0x19, 0x7F, ipd_tlv), + 0, 0x19, 0x7F, mix_tlv),
SOC_DOUBLE("ADC Switch", CS42L52_ADC_MISC_CTL, 0, 1, 1, 0),
The digital volume TLV specifies the step as 0.25dB but the actual step of the control is 0.125dB. Update the TLV to correct this.
Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com --- sound/soc/codecs/cs35l36.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/cs35l36.c b/sound/soc/codecs/cs35l36.c index 920190daa4d1b..dfe85dc2cd20f 100644 --- a/sound/soc/codecs/cs35l36.c +++ b/sound/soc/codecs/cs35l36.c @@ -444,7 +444,8 @@ static bool cs35l36_volatile_reg(struct device *dev, unsigned int reg) } }
-static DECLARE_TLV_DB_SCALE(dig_vol_tlv, -10200, 25, 0); +static const DECLARE_TLV_DB_RANGE(dig_vol_tlv, 0, 912, + TLV_DB_MINMAX_ITEM(-10200, 1200)); static DECLARE_TLV_DB_SCALE(amp_gain_tlv, 0, 1, 1);
static const char * const cs35l36_pcm_sftramp_text[] = {
This driver specified the maximum value rather than the number of volume levels on the SX controls, this is incorrect, so correct them.
Reported-by: David Rhodes david.rhodes@cirrus.com Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com --- sound/soc/codecs/cs53l30.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/sound/soc/codecs/cs53l30.c b/sound/soc/codecs/cs53l30.c index 703545273900f..360ca2ffd5069 100644 --- a/sound/soc/codecs/cs53l30.c +++ b/sound/soc/codecs/cs53l30.c @@ -348,22 +348,22 @@ static const struct snd_kcontrol_new cs53l30_snd_controls[] = { SOC_ENUM("ADC2 NG Delay", adc2_ng_delay_enum),
SOC_SINGLE_SX_TLV("ADC1A PGA Volume", - CS53L30_ADC1A_AFE_CTL, 0, 0x34, 0x18, pga_tlv), + CS53L30_ADC1A_AFE_CTL, 0, 0x34, 0x24, pga_tlv), SOC_SINGLE_SX_TLV("ADC1B PGA Volume", - CS53L30_ADC1B_AFE_CTL, 0, 0x34, 0x18, pga_tlv), + CS53L30_ADC1B_AFE_CTL, 0, 0x34, 0x24, pga_tlv), SOC_SINGLE_SX_TLV("ADC2A PGA Volume", - CS53L30_ADC2A_AFE_CTL, 0, 0x34, 0x18, pga_tlv), + CS53L30_ADC2A_AFE_CTL, 0, 0x34, 0x24, pga_tlv), SOC_SINGLE_SX_TLV("ADC2B PGA Volume", - CS53L30_ADC2B_AFE_CTL, 0, 0x34, 0x18, pga_tlv), + CS53L30_ADC2B_AFE_CTL, 0, 0x34, 0x24, pga_tlv),
SOC_SINGLE_SX_TLV("ADC1A Digital Volume", - CS53L30_ADC1A_DIG_VOL, 0, 0xA0, 0x0C, dig_tlv), + CS53L30_ADC1A_DIG_VOL, 0, 0xA0, 0x6C, dig_tlv), SOC_SINGLE_SX_TLV("ADC1B Digital Volume", - CS53L30_ADC1B_DIG_VOL, 0, 0xA0, 0x0C, dig_tlv), + CS53L30_ADC1B_DIG_VOL, 0, 0xA0, 0x6C, dig_tlv), SOC_SINGLE_SX_TLV("ADC2A Digital Volume", - CS53L30_ADC2A_DIG_VOL, 0, 0xA0, 0x0C, dig_tlv), + CS53L30_ADC2A_DIG_VOL, 0, 0xA0, 0x6C, dig_tlv), SOC_SINGLE_SX_TLV("ADC2B Digital Volume", - CS53L30_ADC2B_DIG_VOL, 0, 0xA0, 0x0C, dig_tlv), + CS53L30_ADC2B_DIG_VOL, 0, 0xA0, 0x6C, dig_tlv), };
static const struct snd_soc_dapm_widget cs53l30_dapm_widgets[] = {
The Bypass Volume is accidentally using a -6dB minimum TLV rather than the correct -60dB minimum. Add a new TLV to correct this.
Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com --- sound/soc/codecs/cs42l52.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/cs42l52.c b/sound/soc/codecs/cs42l52.c index 02c25399cf8a2..10e696406a71b 100644 --- a/sound/soc/codecs/cs42l52.c +++ b/sound/soc/codecs/cs42l52.c @@ -137,6 +137,8 @@ static DECLARE_TLV_DB_SCALE(mic_tlv, 1600, 100, 0);
static DECLARE_TLV_DB_SCALE(pga_tlv, -600, 50, 0);
+static DECLARE_TLV_DB_SCALE(pass_tlv, -6000, 50, 0); + static DECLARE_TLV_DB_SCALE(mix_tlv, -5150, 50, 0);
static DECLARE_TLV_DB_SCALE(beep_tlv, -56, 200, 0); @@ -351,7 +353,7 @@ static const struct snd_kcontrol_new cs42l52_snd_controls[] = { CS42L52_SPKB_VOL, 0, 0x40, 0xC0, hl_tlv),
SOC_DOUBLE_R_SX_TLV("Bypass Volume", CS42L52_PASSTHRUA_VOL, - CS42L52_PASSTHRUB_VOL, 0, 0x88, 0x90, pga_tlv), + CS42L52_PASSTHRUB_VOL, 0, 0x88, 0x90, pass_tlv),
SOC_DOUBLE("Bypass Mute", CS42L52_MISC_CTL, 4, 5, 1, 0),
A couple of the SX volume controls specify 0x84 as the lowest volume value, however the correct value from the datasheet is 0x44. The datasheet don't include spaces in the value it displays as binary so this was almost certainly just a typo reading 1000100.
Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com --- sound/soc/codecs/cs42l56.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/cs42l56.c b/sound/soc/codecs/cs42l56.c index dc23007336c58..510c94265b1f0 100644 --- a/sound/soc/codecs/cs42l56.c +++ b/sound/soc/codecs/cs42l56.c @@ -391,9 +391,9 @@ static const struct snd_kcontrol_new cs42l56_snd_controls[] = { SOC_DOUBLE("ADC Boost Switch", CS42L56_GAIN_BIAS_CTL, 3, 2, 1, 1),
SOC_DOUBLE_R_SX_TLV("Headphone Volume", CS42L56_HPA_VOLUME, - CS42L56_HPB_VOLUME, 0, 0x84, 0x48, hl_tlv), + CS42L56_HPB_VOLUME, 0, 0x44, 0x48, hl_tlv), SOC_DOUBLE_R_SX_TLV("LineOut Volume", CS42L56_LOA_VOLUME, - CS42L56_LOB_VOLUME, 0, 0x84, 0x48, hl_tlv), + CS42L56_LOB_VOLUME, 0, 0x44, 0x48, hl_tlv),
SOC_SINGLE_TLV("Bass Shelving Volume", CS42L56_TONE_CTL, 0, 0x00, 1, tone_tlv),
The minimum value for the PGA Volume is given as 0x1A, however the values from there to 0x19 are all the same volume and this is not represented in the TLV structure. The number of volumes given is correct so this leads to all the volumes being shifted. Move the minimum value up to 0x19 to fix this.
Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com --- sound/soc/codecs/cs42l51.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/cs42l51.c b/sound/soc/codecs/cs42l51.c index aff618513c753..0e933181b5dbb 100644 --- a/sound/soc/codecs/cs42l51.c +++ b/sound/soc/codecs/cs42l51.c @@ -143,7 +143,7 @@ static const struct snd_kcontrol_new cs42l51_snd_controls[] = { 0, 0xA0, 96, adc_att_tlv), SOC_DOUBLE_R_SX_TLV("PGA Volume", CS42L51_ALC_PGA_CTL, CS42L51_ALC_PGB_CTL, - 0, 0x1A, 30, pga_tlv), + 0, 0x19, 30, pga_tlv), SOC_SINGLE("Playback Deemphasis Switch", CS42L51_DAC_CTL, 3, 1, 0), SOC_SINGLE("Auto-Mute Switch", CS42L51_DAC_CTL, 2, 1, 0), SOC_SINGLE("Soft Ramp Switch", CS42L51_DAC_CTL, 1, 1, 0),
On Thu, 2 Jun 2022 17:21:13 +0100, Charles Keepax wrote:
Mostly the usage of the SX controls seems to match the lowest gain value + number of gain levels expected. The one notable exception there being cs53l30 as David noted. However, there are a couple of other places where the minimum value/TLVs are slightly incorrectly specified.
There are two I couldn't quite work out yet, any input on these greatly appreciated:
[...]
Applied to
broonie/sound.git for-linus
Thanks!
[1/6] ASoC: cs42l52: Fix TLV scales for mixer controls commit: 8bf5aabf524eec61013e506f764a0b2652dc5665 [2/6] ASoC: cs35l36: Update digital volume TLV commit: 5005a2345825eb8346546d99bfe669f73111b5c5 [3/6] ASoC: cs53l30: Correct number of volume levels on SX controls commit: 7fbd6dd68127927e844912a16741016d432a0737 [4/6] ASoC: cs42l52: Correct TLV for Bypass Volume commit: 91e90c712fade0b69cdff7cc6512f6099bd18ae5 [5/6] ASoC: cs42l56: Correct typo in minimum level for SX volume controls commit: a8928ada9b96944cadd8b65d191e33199fd38782 [6/6] ASoC: cs42l51: Correct minimum value for SX volume control commit: fcb3b5a58926d16d9a338841b74af06d4c29be15
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (2)
-
Charles Keepax -
Mark Brown