ASoC: ad193x: Fix define of AD193X_PLL_INPUT_MASK

PATCH Applied Axel Lin 4 participants 11 messages Oct 14, 2011 – Oct 18, 2011

Subsystems (3)

Sound ASoC Analog devices inc ASoC codec drivers

lore

Share & tools

lore

Axel Lin

author

2011-10-14 09:01 UTC #0 lore raw

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 <[email protected]>

---
 sound/soc/codecs/ad193x.c |    2 +-
 sound/soc/codecs/ad193x.h |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

sound/soc/codecs/ad193x.c +1 -1

index f934670..39056ce 100644
@@ -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);

sound/soc/codecs/ad193x.h +1 -1

index 536e5f2..1507eaa 100644
@@ -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)

Signature · 1 line

1.7.4.1

Axel Lin

author

2011-10-14 09:03 UTC #1 lore raw

PATCH 2/2 ASoC: ad193x: Use snd_soc_update_bits for read-modify-write

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 <[email protected]>

---
 sound/soc/codecs/ad193x.c |   25 ++++++++++++-------------
 1 files changed, 12 insertions(+), 13 deletions(-)

sound/soc/codecs/ad193x.c +12 ~~-13~~

index 39056ce..1dc8404 100644
@@ -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;
 }

Signature · 1 line

1.7.4.1

Mark Brown

2011-10-14 19:10 UTC #2 lore raw

Re: [PATCH 2/2] ASoC: ad193x: Use snd_soc_update_bits for read-modify-write

sound/soc/codecs/ad193x.c static int ad193x_mute(struct snd_soc_dai *dai, int mute) · line 113

Quoted 8 lines · Axel Lin

sound/soc/codecs/ad193x.c +4 -2

 	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).

Axel Lin

author

2011-10-15 03:22 UTC #3 lore raw

Re: [PATCH 2/2] ASoC: ad193x: Use snd_soc_update_bits for read-modify-write

2011/10/15 Mark Brown <[email protected]>:

Quoted 11 lines · Axel Lin

Quoted 9 lines
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);

Quoted 3 lines

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

Mark Brown

2011-10-17 21:37 UTC #4 lore raw

Re: [PATCH 2/2] ASoC: ad193x: Use snd_soc_update_bits for read-modify-write

Quoted 2 lines · Axel Lin

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.

Barry Song

Acked-by on v1 2/2

2011-10-18 03:23 UTC #5 lore raw

Re: [PATCH 2/2] ASoC: ad193x: Use snd_soc_update_bits for read-modify-write

In reply to Axel Lin

2011/10/14 Axel Lin <[email protected]>:

Quoted 4 lines

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 <[email protected]>

Acked-by Barry Song <[email protected]>

and fix the changelog as Mark pointed out.

Quoted 4 lines

---
sound/soc/codecs/ad193x.c | 25 ++++++++++++-------------
 1 files changed, 12 insertions(+), 13 deletions(-)

-barry

Mark Brown

Applied

2011-10-14 19:38 UTC #6 lore raw

In reply to Axel Lin

Quoted 5 lines · Axel Lin

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?

Lars-Peter Clausen

maintainer

2011-10-14 19:48 UTC #7 lore raw

Quoted 2 lines · Mark Brown

[...] 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.

Mark Brown

2011-10-14 20:07 UTC #8 lore raw

Quoted 5 lines · Lars-Peter Clausen

Quoted 2 lines · Mark Brown
[...] 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 :)

Lars-Peter Clausen

maintainer

2011-10-14 20:09 UTC #9 lore raw

Quoted 8 lines · Mark Brown

Quoted 5 lines · Lars-Peter Clausen
Quoted 2 lines · Mark Brown
[...] 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

Mark Brown

2011-10-14 20:17 UTC #10 lore raw

Quoted 1 line · Lars-Peter Clausen

If it is ok with you, I'll send it on monday.

Sure, that's fine.