ASoC: cs42l56: Fix new value argument in snd_soc_update_bits calls
Subsystems (3)
PATCH 1/1 ASoC: cs42l56: Fix new value argument in snd_soc_update_bits calls
The new value argument needs proper shift to match the mask bit fields.
---
Hi Brian,
This patch replaces
[PATCH RFT] ASoC: cs42l56: Fix update mute register bits in cs42l56_digital_mute. I just found more places of snd_soc_update_bits calls needs fix. Regards,
Axel
sound/soc/codecs/cs42l56.c | 71 ++++++++++++++++++---------------------------- sound/soc/codecs/cs42l56.h | 14 ++++----- 2 files changed, 35 insertions(+), 50 deletions(-)
sound/soc/codecs/cs42l56.c +28 -43
@@ -763,14 +763,14 @@ static int cs42l56_set_sysclk(struct snd_soc_dai *codec_dai,case CS42L56_MCLK_11P2896MHZ :case CS42L56_MCLK_12MHZ :case CS42L56_MCLK_12P288MHZ : - cs42l56->mclk_div2 =1 ; + cs42l56->mclk_div2 =CS42L56_MCLK_DIV2 ; cs42l56->mclk_prediv =0 ;break ;case CS42L56_MCLK_22P5792MHZ :case CS42L56_MCLK_24MHZ :case CS42L56_MCLK_24P576MHZ : - cs42l56->mclk_div2 =1 ; - cs42l56->mclk_prediv =1 ; + cs42l56->mclk_div2 =CS42L56_MCLK_DIV2 ; + cs42l56->mclk_prediv =CS42L56_MCLK_PREDIV ;break ;default :return -EINVAL ;
@@ -839,62 +839,47 @@ static int cs42l56_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)static int cs42l56_digital_mute (struct snd_soc_dai *dai,int mute) {struct snd_soc_codec *codec = dai->codec; +unsigned int mask;if (mute) {/* Hit the DSP Mixer first */ -snd_soc_update_bits (codec,CS42L56_DSP_MUTE_CTL , -CS42L56_ADCAMIX_MUTE_MASK | -CS42L56_ADCBMIX_MUTE_MASK | -CS42L56_PCMAMIX_MUTE_MASK | -CS42L56_PCMBMIX_MUTE_MASK | -CS42L56_MSTB_MUTE_MASK | -CS42L56_MSTA_MUTE_MASK , -CS42L56_MUTE ); + mask =CS42L56_ADCAMIX_MUTE_MASK |CS42L56_ADCBMIX_MUTE_MASK | +CS42L56_PCMAMIX_MUTE_MASK |CS42L56_PCMBMIX_MUTE_MASK | +CS42L56_MSTB_MUTE_MASK |CS42L56_MSTA_MUTE_MASK ; +snd_soc_update_bits (codec,CS42L56_DSP_MUTE_CTL , mask, mask); +/* Mute ADC's */ -snd_soc_update_bits (codec,CS42L56_MISC_ADC_CTL , -CS42L56_ADCA_MUTE_MASK | -CS42L56_ADCB_MUTE_MASK , -CS42L56_MUTE ); + mask =CS42L56_ADCA_MUTE_MASK |CS42L56_ADCB_MUTE_MASK ; +snd_soc_update_bits (codec,CS42L56_MISC_ADC_CTL , mask, mask); +/* HP And LO */ snd_soc_update_bits (codec,CS42L56_HPA_VOLUME , -CS42L56_HP_MUTE_MASK , -CS42L56_MUTE ); +CS42L56_HP_MUTE_MASK ,CS42L56_HP_MUTE_MASK );snd_soc_update_bits (codec,CS42L56_HPB_VOLUME , -CS42L56_HP_MUTE_MASK , -CS42L56_MUTE ); +CS42L56_HP_MUTE_MASK ,CS42L56_HP_MUTE_MASK );snd_soc_update_bits (codec,CS42L56_LOA_VOLUME , -CS42L56_LO_MUTE_MASK , -CS42L56_MUTE ); +CS42L56_LO_MUTE_MASK ,CS42L56_LO_MUTE_MASK );snd_soc_update_bits (codec,CS42L56_LOB_VOLUME , -CS42L56_LO_MUTE_MASK , -CS42L56_MUTE ); +CS42L56_LO_MUTE_MASK ,CS42L56_LO_MUTE_MASK ); }else { -snd_soc_update_bits (codec,CS42L56_DSP_MUTE_CTL , -CS42L56_ADCAMIX_MUTE_MASK | -CS42L56_ADCBMIX_MUTE_MASK | -CS42L56_PCMAMIX_MUTE_MASK | -CS42L56_PCMBMIX_MUTE_MASK | -CS42L56_MSTB_MUTE_MASK | -CS42L56_MSTA_MUTE_MASK , -CS42L56_UNMUTE ); -snd_soc_update_bits (codec,CS42L56_MISC_ADC_CTL , -CS42L56_ADCA_MUTE_MASK | -CS42L56_ADCB_MUTE_MASK , -CS42L56_UNMUTE ); + mask =CS42L56_ADCAMIX_MUTE_MASK |CS42L56_ADCBMIX_MUTE_MASK | +CS42L56_PCMAMIX_MUTE_MASK |CS42L56_PCMBMIX_MUTE_MASK | +CS42L56_MSTB_MUTE_MASK |CS42L56_MSTA_MUTE_MASK ; +snd_soc_update_bits (codec,CS42L56_DSP_MUTE_CTL , mask ,0 ); + + mask =CS42L56_ADCA_MUTE_MASK |CS42L56_ADCB_MUTE_MASK ; +snd_soc_update_bits (codec,CS42L56_MISC_ADC_CTL , mask,0 ); +snd_soc_update_bits (codec,CS42L56_HPA_VOLUME , -CS42L56_HP_MUTE_MASK , -CS42L56_UNMUTE ); +CS42L56_HP_MUTE_MASK ,0 );snd_soc_update_bits (codec,CS42L56_HPB_VOLUME , -CS42L56_HP_MUTE_MASK , -CS42L56_UNMUTE ); +CS42L56_HP_MUTE_MASK ,0 );snd_soc_update_bits (codec,CS42L56_LOA_VOLUME , -CS42L56_LO_MUTE_MASK , -CS42L56_UNMUTE ); +CS42L56_LO_MUTE_MASK ,0 );snd_soc_update_bits (codec,CS42L56_LOB_VOLUME , -CS42L56_LO_MUTE_MASK , -CS42L56_UNMUTE ); +CS42L56_LO_MUTE_MASK ,0 ); }return 0 ; }
sound/soc/codecs/cs42l56.h +7 -7
@@ -80,19 +80,21 @@#define CS42L56_PDN_HPB_MASK 0xc0/* serial port and clk masks */ -#define CS42L56_MASTER_MODE 1 -#define CS42L56_SLAVE_MODE 0 +#define CS42L56_MASTER_MODE (1 << 6) +#define CS42L56_SLAVE_MODE (0 << 6)#define CS42L56_MS_MODE_MASK 0x40 -#define CS42L56_SCLK_INV 1 +#define CS42L56_SCLK_INV (1 << 5)#define CS42L56_SCLK_INV_MASK 0x20#define CS42L56_SCLK_MCLK_MASK 0x18 +#define CS42L56_MCLK_PREDIV (1 << 6)#define CS42L56_MCLK_PREDIV_MASK 0x04 +#define CS42L56_MCLK_DIV2 (1 << 1)#define CS42L56_MCLK_DIV2_MASK 0x02#define CS42L56_MCLK_DIS_MASK 0x01#define CS42L56_CLK_AUTO_MASK 0x20#define CS42L56_CLK_RATIO_MASK 0x1f -#define CS42L56_DIG_FMT_I2S 0 -#define CS42L56_DIG_FMT_LEFT_J 1 +#define CS42L56_DIG_FMT_I2S (0 << 3) +#define CS42L56_DIG_FMT_LEFT_J (1 << 3)#define CS42L56_DIG_FMT_MASK 0x08/* Class H and misc ctl masks */
@@ -116,8 +118,6 @@#define CS42L56_DEEMPH_MASK 0x40#define CS42L56_PLYBCK_GANG_MASK 0x10#define CS42L56_PCM_INV_MASK 0x0c -#define CS42L56_MUTE 1 -#define CS42L56_UNMUTE 0#define CS42L56_ADCAMIX_MUTE_MASK 0x40#define CS42L56_ADCBMIX_MUTE_MASK 0x80#define CS42L56_PCMAMIX_MUTE_MASK 0x10
Signature · 1 line
1.8.3.2
Quoted 28 lines · Axel Lin
The new value argument needs proper shift to match the mask bit fields.
Signed-off-by Axel Lin <[email protected]>---
Hi Brian,
This patch replaces
[PATCH RFT] ASoC: cs42l56: Fix update mute register bits incs42l56_digital_mute. I just found more places ofsnd_soc_update_bitscalls needs fix. Regards,
Axelsound/soc/codecs/cs42l56.c| 71 ++++++++++++++++++----------------------------sound/soc/codecs/cs42l56.h| 14 ++++-----2 files changed, 35 insertions(+), 50 deletions(-)sound/soc/codecs/cs42l56.c +0
-0{ struct snd_soc_codec *codec = dai->codec;sound/soc/codecs/cs42l56.c +1
-0+unsigned int mask;if (mute) { /* Hit the DSP Mixer first */sound/soc/codecs/cs42l56.c +0
-3- snd_soc_update_bits(codec, CS42L56_DSP_MUTE_CTL, - CS42L56_ADCAMIX_MUTE_MASK | - CS42L56_ADCBMIX_MUTE_MASK |
sound/soc/codecs/cs42l56.c static int cs42l56_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt) · line 850
Quoted 10 lines
sound/soc/codecs/cs42l56.c +5
-5-CS42L56_PCMAMIX_MUTE_MASK | -CS42L56_PCMBMIX_MUTE_MASK | -CS42L56_MSTB_MUTE_MASK | -CS42L56_MSTA_MUTE_MASK , - CS42L56_MUTE); + mask =CS42L56_ADCAMIX_MUTE_MASK |CS42L56_ADCBMIX_MUTE_MASK | +CS42L56_PCMAMIX_MUTE_MASK |CS42L56_PCMBMIX_MUTE_MASK | +CS42L56_MSTB_MUTE_MASK |CS42L56_MSTA_MUTE_MASK ; +snd_soc_update_bits (codec,CS42L56_DSP_MUTE_CTL , mask, mask); +
I see where my MUTE is not actually doing what I want and since this is for just .digital_mute, I would rather just declare a MUTE_ALL define instead of adding variables to the function
sound/soc/codecs/cs42l56.c static int cs42l56_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt) · line 861
Quoted 4 lines
snd_soc_update_bits(codec, CS42L56_LOB_VOLUME,sound/soc/codecs/cs42l56.c +1
-2-CS42L56_LO_MUTE_MASK , - CS42L56_MUTE); + CS42L56_LO_MUTE_MASK, CS42L56_LO_MUTE_MASK);
Yes, this is my fault for not understanding this correctly. I don't like it and think odd you have to do it like this BTW.
Quoted 5 lines
} else {sound/soc/codecs/cs42l56.c +0
-2- snd_soc_update_bits(codec, CS42L56_DSP_MUTE_CTL, - CS42L56_ADCAMIX_MUTE_MASK |
sound/soc/codecs/cs42l56.c static int cs42l56_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt) · line 885
Quoted 10 lines
sound/soc/codecs/cs42l56.c +0
-10-CS42L56_ADCBMIX_MUTE_MASK | -CS42L56_PCMAMIX_MUTE_MASK | -CS42L56_PCMBMIX_MUTE_MASK | -CS42L56_MSTB_MUTE_MASK | -CS42L56_MSTA_MUTE_MASK , - CS42L56_UNMUTE); -snd_soc_update_bits (codec,CS42L56_MISC_ADC_CTL , -CS42L56_ADCA_MUTE_MASK | -CS42L56_ADCB_MUTE_MASK , -CS42L56_UNMUTE );
This should work fine.
sound/soc/codecs/cs42l56.c
Quoted 2 lines
sound/soc/codecs/cs42l56.c +1
-1- CS42L56_UNMUTE); + CS42L56_LO_MUTE_MASK, 0);
What is the difference here? Consistency?
Quoted 11 lines
} return 0; }sound/soc/codecs/cs42l56.h +0
-0@@ -80,19 +80,21 @@#define CS42L56_PDN_HPB_MASK 0xc0/* serial port and clk masks */
sound/soc/codecs/cs42l56.h line 89
Quoted 10 lines
sound/soc/codecs/cs42l56.h +4
-3-#define CS42L56_MASTER_MODE 1 -#define CS42L56_SLAVE_MODE 0 +#define CS42L56_MASTER_MODE (1 << 6) +#define CS42L56_SLAVE_MODE (0 << 6)#define CS42L56_MS_MODE_MASK 0x40 -#define CS42L56_SCLK_INV 1 +#define CS42L56_SCLK_INV (1 << 5)#define CS42L56_SCLK_INV_MASK 0x20#define CS42L56_SCLK_MCLK_MASK 0x18 +#define CS42L56_MCLK_PREDIV (1 << 6)
Your way off on this one. It is the third bit in the register
In general for myself I am trying to get away from the "shift" defines and use something more meaningful with just a value. I have not tested yet as I just got back into the office. This is just a quick look which is way overdue. My appologies.
I will test to verify the rest on Monday hopefully but as is there are errors that have to be corrected.
Thanks!
Brian
2014-05-23 1:56 GMT+08:00 Brian Austin <[email protected]>:
Quoted 59 lines · Axel Lin
Quoted 42 lines
The new value argument needs proper shift to match the mask bit fields.
Signed-off-by Axel Lin <[email protected]>---
Hi Brian,
This patch replaces
[PATCH RFT] ASoC: cs42l56: Fix update mute register bits incs42l56_digital_mute.
I just found more places ofsnd_soc_update_bitscalls needs fix. Regards,
Axelsound/soc/codecs/cs42l56.c| 71
++++++++++++++++++----------------------------sound/soc/codecs/cs42l56.h| 14 ++++-----2 files changed, 35 insertions(+), 50 deletions(-)sound/soc/codecs/cs42l56.c +0
-0{ struct snd_soc_codec *codec = dai->codec;sound/soc/codecs/cs42l56.c +2
-8+unsigned int mask;if (mute) {/* Hit the DSP Mixer first */ -snd_soc_update_bits (codec,CS42L56_DSP_MUTE_CTL , -CS42L56_ADCAMIX_MUTE_MASK | -CS42L56_ADCBMIX_MUTE_MASK | -CS42L56_PCMAMIX_MUTE_MASK | -CS42L56_PCMBMIX_MUTE_MASK | -CS42L56_MSTB_MUTE_MASK | -CS42L56_MSTA_MUTE_MASK , -CS42L56_MUTE ); + mask = CS42L56_ADCAMIX_MUTE_MASK |
CS42L56_ADCBMIX_MUTE_MASK|
+CS42L56_PCMAMIX_MUTE_MASK|CS42L56_PCMBMIX_MUTE_MASK|sound/soc/codecs/cs42l56.c +2
-0+CS42L56_MSTB_MUTE_MASK |CS42L56_MSTA_MUTE_MASK ; + snd_soc_update_bits(codec, CS42L56_DSP_MUTE_CTL, mask,Quoted 2 lines
mask);
+I see where my MUTE is not actually doing what I want and since this is for just .digital_mute, I would rather just declare a
MUTE_ALLdefine instead of adding variables to the functionQuoted 5 lines
snd_soc_update_bits(codec, CS42L56_LOB_VOLUME,sound/soc/codecs/cs42l56.c +1
-2-CS42L56_LO_MUTE_MASK , - CS42L56_MUTE); + CS42L56_LO_MUTE_MASK,
CS42L56_LO_MUTE_MASK);
Quoted 2 lines
Yes, this is my fault for not understanding this correctly. I don't like it and think odd you have to do it like this BTW.
It's to set CS42L56_LO_MUTE_MASK bit (0x80).
Quoted 22 lines
Quoted 7 lines
} else {sound/soc/codecs/cs42l56.c +0
-2- snd_soc_update_bits(codec, CS42L56_DSP_MUTE_CTL, - CS42L56_ADCAMIX_MUTE_MASK |sound/soc/codecs/cs42l56.c static int cs42l56_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt) · line 885
Quoted 10 lines
sound/soc/codecs/cs42l56.c +0
-10-CS42L56_ADCBMIX_MUTE_MASK | -CS42L56_PCMAMIX_MUTE_MASK | -CS42L56_PCMBMIX_MUTE_MASK | -CS42L56_MSTB_MUTE_MASK | -CS42L56_MSTA_MUTE_MASK , - CS42L56_UNMUTE); -snd_soc_update_bits (codec,CS42L56_MISC_ADC_CTL , -CS42L56_ADCA_MUTE_MASK | -CS42L56_ADCB_MUTE_MASK , -CS42L56_UNMUTE );This should work fine.
Quoted 6 lines
sound/soc/codecs/cs42l56.c
Quoted 2 lines
sound/soc/codecs/cs42l56.c +1
-1- CS42L56_UNMUTE); + CS42L56_LO_MUTE_MASK, 0);What is the difference here? Consistency?
From my point of view, pass 0 here means to clear the MASK bits.
Note, the mask bit is different in various snd_soc_update_bits calls here, so it only make sense if CS42L56_UNMUTE is 0.
In this case, CS42L56_UNMUTE actually is defined as 0. so either is ok.
Quoted 12 lines
Quoted 12 lines
} return 0; }sound/soc/codecs/cs42l56.h +0
-0@@ -80,19 +80,21 @@#define CS42L56_PDN_HPB_MASK 0xc0
Quoted 13 lines
sound/soc/codecs/cs42l56.h line 89
Quoted 11 lines
sound/soc/codecs/cs42l56.h +4
-3/* serial port and clk masks */ -#define CS42L56_MASTER_MODE 1 -#define CS42L56_SLAVE_MODE 0 +#define CS42L56_MASTER_MODE (1 << 6) +#define CS42L56_SLAVE_MODE (0 << 6)#define CS42L56_MS_MODE_MASK 0x40 -#define CS42L56_SCLK_INV 1 +#define CS42L56_SCLK_INV (1 << 5)#define CS42L56_SCLK_INV_MASK 0x20#define CS42L56_SCLK_MCLK_MASK 0x18 +#define CS42L56_MCLK_PREDIV (1 << 6)Your way off on this one. It is the third bit in the register
Fixed in v2.
Quoted 3 lines
In general for myself I am trying to get away from the "shift" defines and use something more meaningful with just a value. I have not tested yet as I
It's nothing wrong using the "shift" in defines.
I personal think using "shift" in the defines makes it easier to understand the meaning which is mapping
to the bits documented in datasheet.
But since you prefer not using the "shift", I updated the defines in v2.
Thanks for the review,
Axel