[PATCH 1/2] ASoC: ad193x: Fix define of AD193X_PLL_INPUT_MASK

[PATCH 1/2] ASoC: ad193x: Fix define of AD193X_PLL_INPUT_MASK

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

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

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

[Axel Lin]

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;
 }
-- 
1.7.4.1

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

[Mark Brown]

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

Re: [PATCH 1/2] ASoC: ad193x: Fix define of AD193X_PLL_INPUT_MASK

[Mark Brown]

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?

Re: [PATCH 1/2] ASoC: ad193x: Fix define of AD193X_PLL_INPUT_MASK

[Lars-Peter Clausen]

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.

Re: [PATCH 1/2] ASoC: ad193x: Fix define of AD193X_PLL_INPUT_MASK

[Mark Brown]

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

Re: [PATCH 1/2] ASoC: ad193x: Fix define of AD193X_PLL_INPUT_MASK

[Lars-Peter Clausen]

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

Re: [PATCH 1/2] ASoC: ad193x: Fix define of AD193X_PLL_INPUT_MASK

[Mark Brown]

On Fri, Oct 14, 2011 at 10:09:22PM +0200, Lars-Peter Clausen wrote:

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

Sure, that's fine.

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

[Axel Lin]

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

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

[Mark Brown]

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.

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

[Barry Song]

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