[alsa-devel] [PATCH 1/2] ASoC: ad193x: Fix define of AD193X_PLL_INPUT_MASK
Current code defines AD193X_PLL_INPUT_MASK as (~0x6) which is quite different from other MASK defines. To make it consistent with other mask defines, define AD193X_PLL_INPUT_MASK as 0x6 and change the code accordingly. I think this change improves the readability.
Signed-off-by: Axel Lin axel.lin@gmail.com --- sound/soc/codecs/ad193x.c | 2 +- sound/soc/codecs/ad193x.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/ad193x.c b/sound/soc/codecs/ad193x.c index f934670..39056ce 100644 --- a/sound/soc/codecs/ad193x.c +++ b/sound/soc/codecs/ad193x.c @@ -298,7 +298,7 @@ static int ad193x_hw_params(struct snd_pcm_substream *substream, }
reg = snd_soc_read(codec, AD193X_PLL_CLK_CTRL0); - reg = (reg & AD193X_PLL_INPUT_MASK) | master_rate; + reg = (reg & (~AD193X_PLL_INPUT_MASK)) | master_rate; snd_soc_write(codec, AD193X_PLL_CLK_CTRL0, reg);
reg = snd_soc_read(codec, AD193X_DAC_CTRL2); diff --git a/sound/soc/codecs/ad193x.h b/sound/soc/codecs/ad193x.h index 536e5f2..1507eaa 100644 --- a/sound/soc/codecs/ad193x.h +++ b/sound/soc/codecs/ad193x.h @@ -11,7 +11,7 @@
#define AD193X_PLL_CLK_CTRL0 0x00 #define AD193X_PLL_POWERDOWN 0x01 -#define AD193X_PLL_INPUT_MASK (~0x6) +#define AD193X_PLL_INPUT_MASK 0x6 #define AD193X_PLL_INPUT_256 (0 << 1) #define AD193X_PLL_INPUT_384 (1 << 1) #define AD193X_PLL_INPUT_512 (2 << 1)
Use snd_soc_update_bits for read-modify-write register access instead of open-coding it using snd_soc_read and snd_soc_write
Signed-off-by: Axel Lin axel.lin@gmail.com --- sound/soc/codecs/ad193x.c | 25 ++++++++++++------------- 1 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/sound/soc/codecs/ad193x.c b/sound/soc/codecs/ad193x.c index 39056ce..1dc8404 100644 --- a/sound/soc/codecs/ad193x.c +++ b/sound/soc/codecs/ad193x.c @@ -106,8 +106,10 @@ static int ad193x_mute(struct snd_soc_dai *dai, int mute) int reg;
reg = snd_soc_read(codec, AD193X_DAC_CTRL2); - reg = (mute > 0) ? reg | AD193X_DAC_MASTER_MUTE : reg & - (~AD193X_DAC_MASTER_MUTE); + if (mute) + reg |= AD193X_DAC_MASTER_MUTE; + else + reg &= ~AD193X_DAC_MASTER_MUTE; snd_soc_write(codec, AD193X_DAC_CTRL2, reg);
return 0; @@ -262,7 +264,7 @@ static int ad193x_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, struct snd_soc_dai *dai) { - int word_len = 0, reg = 0, master_rate = 0; + int word_len = 0, master_rate = 0;
struct snd_soc_pcm_runtime *rtd = substream->private_data; struct snd_soc_codec *codec = rtd->codec; @@ -297,18 +299,15 @@ static int ad193x_hw_params(struct snd_pcm_substream *substream, break; }
- reg = snd_soc_read(codec, AD193X_PLL_CLK_CTRL0); - reg = (reg & (~AD193X_PLL_INPUT_MASK)) | master_rate; - snd_soc_write(codec, AD193X_PLL_CLK_CTRL0, reg); + snd_soc_update_bits(codec, AD193X_PLL_CLK_CTRL0, + AD193X_PLL_INPUT_MASK, master_rate);
- reg = snd_soc_read(codec, AD193X_DAC_CTRL2); - reg = (reg & (~AD193X_DAC_WORD_LEN_MASK)) - | (word_len << AD193X_DAC_WORD_LEN_SHFT); - snd_soc_write(codec, AD193X_DAC_CTRL2, reg); + snd_soc_update_bits(codec, AD193X_DAC_CTRL2, + AD193X_DAC_WORD_LEN_MASK, + word_len << AD193X_DAC_WORD_LEN_SHFT);
- reg = snd_soc_read(codec, AD193X_ADC_CTRL1); - reg = (reg & (~AD193X_ADC_WORD_LEN_MASK)) | word_len; - snd_soc_write(codec, AD193X_ADC_CTRL1, reg); + snd_soc_update_bits(codec, AD193X_ADC_CTRL1, + AD193X_ADC_WORD_LEN_MASK, word_len);
return 0; }
On Fri, Oct 14, 2011 at 05:03:07PM +0800, Axel Lin wrote:
reg = snd_soc_read(codec, AD193X_DAC_CTRL2);
- reg = (mute > 0) ? reg | AD193X_DAC_MASTER_MUTE : reg &
(~AD193X_DAC_MASTER_MUTE);
- if (mute)
reg |= AD193X_DAC_MASTER_MUTE;
- else
snd_soc_write(codec, AD193X_DAC_CTRL2, reg);reg &= ~AD193X_DAC_MASTER_MUTE;
This is still using snd_soc_read() and snd_soc_write() (though it is a cleanup).
2011/10/15 Mark Brown broonie@opensource.wolfsonmicro.com:
On Fri, Oct 14, 2011 at 05:03:07PM +0800, Axel Lin wrote:
reg = snd_soc_read(codec, AD193X_DAC_CTRL2);
- reg = (mute > 0) ? reg | AD193X_DAC_MASTER_MUTE : reg &
- (~AD193X_DAC_MASTER_MUTE);
- if (mute)
- reg |= AD193X_DAC_MASTER_MUTE;
- else
- reg &= ~AD193X_DAC_MASTER_MUTE;
snd_soc_write(codec, AD193X_DAC_CTRL2, reg);
This is still using snd_soc_read() and snd_soc_write() (though it is a cleanup).
hi Mark, One of the major reason that I do the convert to snd_soc_update_bits is for better readability. In this case, it seems ok to me with this change. But if you prefer also convert it to snd_soc_update_bits, I can do it. Let me know if I need to send a v2 for it.
Thanks, Axel
On Sat, Oct 15, 2011 at 11:21:49AM +0800, Axel Lin wrote:
But if you prefer also convert it to snd_soc_update_bits, I can do it. Let me know if I need to send a v2 for it.
It'd be nice, if only because the patch clearly isn't doing what the changelog claims it's doing.
2011/10/14 Axel Lin axel.lin@gmail.com:
Use snd_soc_update_bits for read-modify-write register access instead of open-coding it using snd_soc_read and snd_soc_write
Signed-off-by: Axel Lin axel.lin@gmail.com
Acked-by: Barry Song 21cnbao@gmail.com
and fix the changelog as Mark pointed out.
sound/soc/codecs/ad193x.c | 25 ++++++++++++------------- 1 files changed, 12 insertions(+), 13 deletions(-)
-barry
On Fri, Oct 14, 2011 at 05:01:59PM +0800, Axel Lin wrote:
Current code defines AD193X_PLL_INPUT_MASK as (~0x6) which is quite different from other MASK defines. To make it consistent with other mask defines, define AD193X_PLL_INPUT_MASK as 0x6 and change the code accordingly. I think this change improves the readability.
Applied, thanks. For the ADI drivers please CC Lars-Peter. Lars-Peter, could you please send a MAINTAINERS update if you've not done so already via some other tree?
On Fri, Oct 14, 2011 at 09:48:16PM +0200, Lars-Peter Clausen wrote:
On 10/14/2011 09:38 PM, Mark Brown wrote:
[...] Lars-Peter, could you please send a MAINTAINERS update if you've not done so already via some other tree?
Sure, I'll send one next week.
Given the nearness of the merge window sooner would be better than later :)
On 10/14/2011 10:07 PM, Mark Brown wrote:
On Fri, Oct 14, 2011 at 09:48:16PM +0200, Lars-Peter Clausen wrote:
On 10/14/2011 09:38 PM, Mark Brown wrote:
[...] Lars-Peter, could you please send a MAINTAINERS update if you've not done so already via some other tree?
Sure, I'll send one next week.
Given the nearness of the merge window sooner would be better than later :)
If it is ok with you, I'll send it on monday.
- Lars
participants (4)
-
Axel Lin
-
Barry Song
-
Lars-Peter Clausen
-
Mark Brown