[PATCH 1/6] ASoC: ad1980: Stay in 20bit mode for architectures other than blackfin

[PATCH 1/6] ASoC: ad1980: Stay in 20bit mode for architectures other than blackfin

[Uwe Kleine-König]

From: Sascha Hauer <s.hauer@pengutronix.de>

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 sound/soc/codecs/ad1980.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/sound/soc/codecs/ad1980.c b/sound/soc/codecs/ad1980.c
index 70cfaec..35c0580 100644
--- a/sound/soc/codecs/ad1980.c
+++ b/sound/soc/codecs/ad1980.c
@@ -168,13 +168,21 @@ retry:
 	}
 
 	soc_ac97_ops.reset(codec->ac97);
+
+#ifdef SND_BF5XX_SOC_AD1980
 	/* Set bit 16slot in register 74h, then every slot will has only 16
 	 * bits. This command is sent out in 20bit mode, in which case the
-	 * first nibble of data is eaten by the addr. (Tag is always 16 bit)*/
+	 * first nibble of data is eaten by the addr. (Tag is always 16 bit).
+	 *
+	 * This seems to be blackfin specific. On other systems we want 20bit
+	 * mode.
+	 */
 	ac97_write(codec, AC97_AD_SERIAL_CFG, 0x9900);
+#endif
 
 	if (ac97_read(codec, AC97_RESET)  != 0x0090)
 		goto err;
+
 	return 0;
 
 err:
-- 
1.7.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH 2/6] ASoC: ad1980: fix names of a few kcontrols

[Uwe Kleine-König]

From: Marcus Weigelt <marcus.weigelt@fwd-online.de>

The names used before keep alsamixer from showing the appropriate
controls for the AD1980.  Mic and phone volume controls were shown in
alsamixers' capture tab where they don´t belong to.  Phone playback
volume did not appear in playback tab and the microphone playback could
never be unmuted.  Further it was not possible to select phone as
capture source.  Changing the names fixes these problems.

Signed-off-by: Marcus Weigelt <marcus.weigelt@fwd-online.de>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 sound/soc/codecs/ad1980.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/ad1980.c b/sound/soc/codecs/ad1980.c
index 35c0580..c1550b8 100644
--- a/sound/soc/codecs/ad1980.c
+++ b/sound/soc/codecs/ad1980.c
@@ -82,11 +82,11 @@ SOC_SINGLE("PCM Capture Switch", AC97_REC_GAIN, 15, 1, 1),
 SOC_SINGLE("Mono Playback Volume", AC97_MASTER_MONO, 0, 31, 1),
 SOC_SINGLE("Mono Playback Switch", AC97_MASTER_MONO, 15, 1, 1),
 
-SOC_SINGLE("Phone Capture Volume", AC97_PHONE, 0, 31, 1),
-SOC_SINGLE("Phone Capture Switch", AC97_PHONE, 15, 1, 1),
+SOC_SINGLE("Phone Playback Volume", AC97_PHONE, 0, 31, 1),
+SOC_SINGLE("Phone Playback Switch", AC97_PHONE, 15, 1, 1),
 
-SOC_SINGLE("Mic Volume", AC97_MIC, 0, 31, 1),
-SOC_SINGLE("Mic Switch", AC97_MIC, 15, 1, 1),
+SOC_SINGLE("Mic Playback Volume", AC97_MIC, 0, 31, 1),
+SOC_SINGLE("Mic Playback Switch", AC97_MIC, 15, 1, 1),
 
 SOC_SINGLE("Stereo Mic Switch", AC97_AD_MISC, 6, 1, 0),
 SOC_DOUBLE("Line HP Swap Switch", AC97_AD_MISC, 10, 5, 1, 0),
-- 
1.7.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 2/6] ASoC: ad1980: fix names of a few kcontrols

[Mark Brown]

On Tue, Aug 24, 2010 at 03:09:40PM +0200, Uwe Kleine-K??nig wrote:
> From: Marcus Weigelt <marcus.weigelt@fwd-online.de>
> 
> The names used before keep alsamixer from showing the appropriate
> controls for the AD1980.  Mic and phone volume controls were shown in
> alsamixers' capture tab where they don??t belong to.  Phone playback

It's not massively obvious that microphone and phone aren't inputs and
therefore don't sensibly appear in the capture tab?

> volume did not appear in playback tab and the microphone playback could
> never be unmuted.  Further it was not possible to select phone as
> capture source.  Changing the names fixes these problems.

This doesn't sound real - misnamed controls might cause UI problems but
you'd have to try *really* hard to make the control name completely
prevent the setting of the control.

What is the actual problem that is is being fixed here?

> Signed-off-by: Marcus Weigelt <marcus.weigelt@fwd-online.de>
> Signed-off-by: Uwe Kleine-K??nig <u.kleine-koenig@pengutronix.de>

Please remember to CC maintaintainers on patches.

> ---
>  sound/soc/codecs/ad1980.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/codecs/ad1980.c b/sound/soc/codecs/ad1980.c
> index 35c0580..c1550b8 100644
> --- a/sound/soc/codecs/ad1980.c
> +++ b/sound/soc/codecs/ad1980.c
> @@ -82,11 +82,11 @@ SOC_SINGLE("PCM Capture Switch", AC97_REC_GAIN, 15, 1, 1),
>  SOC_SINGLE("Mono Playback Volume", AC97_MASTER_MONO, 0, 31, 1),
>  SOC_SINGLE("Mono Playback Switch", AC97_MASTER_MONO, 15, 1, 1),
>  
> -SOC_SINGLE("Phone Capture Volume", AC97_PHONE, 0, 31, 1),
> -SOC_SINGLE("Phone Capture Switch", AC97_PHONE, 15, 1, 1),
> +SOC_SINGLE("Phone Playback Volume", AC97_PHONE, 0, 31, 1),
> +SOC_SINGLE("Phone Playback Switch", AC97_PHONE, 15, 1, 1),
>  
> -SOC_SINGLE("Mic Volume", AC97_MIC, 0, 31, 1),
> -SOC_SINGLE("Mic Switch", AC97_MIC, 15, 1, 1),
> +SOC_SINGLE("Mic Playback Volume", AC97_MIC, 0, 31, 1),
> +SOC_SINGLE("Mic Playback Switch", AC97_MIC, 15, 1, 1),
>  
>  SOC_SINGLE("Stereo Mic Switch", AC97_AD_MISC, 6, 1, 0),
>  SOC_DOUBLE("Line HP Swap Switch", AC97_AD_MISC, 10, 5, 1, 0),
> -- 
> 1.7.1
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

-- 
"You grabbed my hand and we fell into it, like a daydream - or a fever."

[PATCH 3/6] ASoC: ad1980: remove unneeded function declaration

[Uwe Kleine-König]

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 sound/soc/codecs/ad1980.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/sound/soc/codecs/ad1980.c b/sound/soc/codecs/ad1980.c
index c1550b8..82267e5 100644
--- a/sound/soc/codecs/ad1980.c
+++ b/sound/soc/codecs/ad1980.c
@@ -33,11 +33,6 @@
 
 #include "ad1980.h"
 
-static unsigned int ac97_read(struct snd_soc_codec *codec,
-	unsigned int reg);
-static int ac97_write(struct snd_soc_codec *codec,
-	unsigned int reg, unsigned int val);
-
 /*
  * AD1980 register cache
  */
-- 
1.7.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 3/6] ASoC: ad1980: remove unneeded function declaration

[Mark Brown]

On Tue, Aug 24, 2010 at 03:09:41PM +0200, Uwe Kleine-K??nig wrote:

Please remember to CC maintainers on patches.  This looks OK, I've CCed
in Liam.

> Signed-off-by: Uwe Kleine-K??nig <u.kleine-koenig@pengutronix.de>
> ---
>  sound/soc/codecs/ad1980.c |    5 -----
>  1 files changed, 0 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/soc/codecs/ad1980.c b/sound/soc/codecs/ad1980.c
> index c1550b8..82267e5 100644
> --- a/sound/soc/codecs/ad1980.c
> +++ b/sound/soc/codecs/ad1980.c
> @@ -33,11 +33,6 @@
>  
>  #include "ad1980.h"
>  
> -static unsigned int ac97_read(struct snd_soc_codec *codec,
> -	unsigned int reg);
> -static int ac97_write(struct snd_soc_codec *codec,
> -	unsigned int reg, unsigned int val);
> -
>  /*
>   * AD1980 register cache
>   */
> -- 
> 1.7.1
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 3/6] ASoC: ad1980: remove unneeded function declaration

[Liam Girdwood]

On Fri, 2010-08-27 at 19:12 +0100, Mark Brown wrote:
> On Tue, Aug 24, 2010 at 03:09:41PM +0200, Uwe Kleine-K??nig wrote:
> 
> Please remember to CC maintainers on patches.  This looks OK, I've CCed
> in Liam.
> 
> > Signed-off-by: Uwe Kleine-K??nig <u.kleine-koenig@pengutronix.de>

Acked-by: Liam Girdwood <lrg@slimlogic.co.uk>
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH 3/6] ASoC: ad1980: remove unneeded function declaration

[Mark Brown]

On Sat, Aug 28, 2010 at 09:14:39AM +0100, Liam Girdwood wrote:
> On Fri, 2010-08-27 at 19:12 +0100, Mark Brown wrote:
> > On Tue, Aug 24, 2010 at 03:09:41PM +0200, Uwe Kleine-K??nig wrote:
> > 
> > Please remember to CC maintainers on patches.  This looks OK, I've CCed
> > in Liam.
> > 
> > > Signed-off-by: Uwe Kleine-K??nig <u.kleine-koenig@pengutronix.de>
> 
> Acked-by: Liam Girdwood <lrg@slimlogic.co.uk>

Applied, thanks.

[PATCH 4/6] ASoC: ad1980: make usage of register cache optional

[Uwe Kleine-König]

I'm hunting some problems where reading and writing to the codec doesn't
work.  In this case caching is in the way.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 sound/soc/codecs/ad1980.c |   36 +++++++++++++++++++++++++++++-------
 1 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/sound/soc/codecs/ad1980.c b/sound/soc/codecs/ad1980.c
index 82267e5..864e65d 100644
--- a/sound/soc/codecs/ad1980.c
+++ b/sound/soc/codecs/ad1980.c
@@ -33,6 +33,8 @@
 
 #include "ad1980.h"
 
+#define AC97_USE_CACHE 1
+#if AC97_USE_CACHE
 /*
  * AD1980 register cache
  */
@@ -54,6 +56,7 @@ static const u16 ad1980_reg[] = {
 	0x0000, 0x0000, 0x1001, 0x0000, /* 70 - 76 */
 	0x0000, 0x0000, 0x4144, 0x5370  /* 78 - 7e */
 };
+#endif
 
 static const char *ad1980_rec_sel[] = {"Mic", "CD", "NC", "AUX", "Line",
 		"Stereo Mix", "Mono Mix", "Phone"};
@@ -101,7 +104,9 @@ static unsigned int ac97_read(struct snd_soc_codec *codec,
 	unsigned int reg)
 {
 	u16 *cache = codec->reg_cache;
+	int ret;
 
+#if AC97_USE_CACHE
 	switch (reg) {
 	case AC97_RESET:
 	case AC97_INT_PAGING:
@@ -109,27 +114,38 @@ static unsigned int ac97_read(struct snd_soc_codec *codec,
 	case AC97_EXTENDED_STATUS:
 	case AC97_VENDOR_ID1:
 	case AC97_VENDOR_ID2:
-		return soc_ac97_ops.read(codec->ac97, reg);
+#endif
+		ret = soc_ac97_ops.read(codec->ac97, reg);
+#if AC97_USE_CACHE
 	default:
 		reg = reg >> 1;
 
 		if (reg >= ARRAY_SIZE(ad1980_reg))
-			return -EINVAL;
-
-		return cache[reg];
+			ret = -EINVAL;
+		else
+			ret = cache[reg];
 	}
+#endif
+
+	pr_debug("%s: reg=0x%02x, val=0x%04x\n", __func__, reg, ret);
+
+	return ret;
 }
 
 static int ac97_write(struct snd_soc_codec *codec, unsigned int reg,
 	unsigned int val)
 {
-	u16 *cache = codec->reg_cache;
+	pr_debug("%s: reg=0x%02x, val=0x%04x\n", __func__, reg, val);
 
 	soc_ac97_ops.write(codec->ac97, reg, val);
+
+#if AC97_USE_CACHE
 	reg = reg >> 1;
-	if (reg < ARRAY_SIZE(ad1980_reg))
+	if (reg < ARRAY_SIZE(ad1980_reg)) {
+		u16 *cache = codec->reg_cache;
 		cache[reg] = val;
-
+	}
+#endif
 	return 0;
 }
 
@@ -204,6 +220,7 @@ static int ad1980_soc_probe(struct platform_device *pdev)
 	codec = socdev->card->codec;
 	mutex_init(&codec->mutex);
 
+#if AC97_USE_CACHE
 	codec->reg_cache =
 		kzalloc(sizeof(u16) * ARRAY_SIZE(ad1980_reg), GFP_KERNEL);
 	if (codec->reg_cache == NULL) {
@@ -214,6 +231,7 @@ static int ad1980_soc_probe(struct platform_device *pdev)
 			ARRAY_SIZE(ad1980_reg));
 	codec->reg_cache_size = sizeof(u16) * ARRAY_SIZE(ad1980_reg);
 	codec->reg_cache_step = 2;
+#endif
 	codec->name = "AD1980";
 	codec->owner = THIS_MODULE;
 	codec->dai = &ad1980_dai;
@@ -279,9 +297,11 @@ pcm_err:
 	snd_soc_free_ac97_codec(codec);
 
 codec_err:
+#if AC97_USE_CACHE
 	kfree(codec->reg_cache);
 
 cache_err:
+#endif
 	kfree(socdev->card->codec);
 	socdev->card->codec = NULL;
 	return ret;
@@ -298,7 +318,9 @@ static int ad1980_soc_remove(struct platform_device *pdev)
 	snd_soc_dapm_free(socdev);
 	snd_soc_free_pcms(socdev);
 	snd_soc_free_ac97_codec(codec);
+#if AC97_USE_CACHE
 	kfree(codec->reg_cache);
+#endif
 	kfree(codec);
 	return 0;
 }
-- 
1.7.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 4/6] ASoC: ad1980: make usage of register cache optional

[Mark Brown]

On Tue, Aug 24, 2010 at 03:09:42PM +0200, Uwe Kleine-K??nig wrote:
> I'm hunting some problems where reading and writing to the codec doesn't
> work.  In this case caching is in the way.
> 
> Signed-off-by: Uwe Kleine-K??nig <u.kleine-koenig@pengutronix.de>

Once more please remember to CC maintainers on patches.

The commit message here could've done with a description of what the
actual change you've implemented to bypass the register cache here is.
What you've done is to add an ifdef AC97_USE_CACHE which cuts out the
cache parts of the I/O functions and the register defaults.  You've also
introduced a number of unrelated changes which add logging to the I/O
functions.

It strikes me that this is a fairly invasive way of implementing this
functionality and that it's probably at the wrong level - it'd seem
better to do this by supporting volatility and then marking all the
registers as volatile, and that doing this with a debugfs setting in the
core (or the soc-cache code anyway) would be even better.  This would
fall fairly naturally out of factoring the AC97 register I/O into
soc-cache.

> ---
>  sound/soc/codecs/ad1980.c |   36 +++++++++++++++++++++++++++++-------
>  1 files changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/sound/soc/codecs/ad1980.c b/sound/soc/codecs/ad1980.c
> index 82267e5..864e65d 100644
> --- a/sound/soc/codecs/ad1980.c
> +++ b/sound/soc/codecs/ad1980.c
> @@ -33,6 +33,8 @@
>  
>  #include "ad1980.h"
>  
> +#define AC97_USE_CACHE 1
> +#if AC97_USE_CACHE
>  /*
>   * AD1980 register cache
>   */
> @@ -54,6 +56,7 @@ static const u16 ad1980_reg[] = {
>  	0x0000, 0x0000, 0x1001, 0x0000, /* 70 - 76 */
>  	0x0000, 0x0000, 0x4144, 0x5370  /* 78 - 7e */
>  };
> +#endif
>  
>  static const char *ad1980_rec_sel[] = {"Mic", "CD", "NC", "AUX", "Line",
>  		"Stereo Mix", "Mono Mix", "Phone"};
> @@ -101,7 +104,9 @@ static unsigned int ac97_read(struct snd_soc_codec *codec,
>  	unsigned int reg)
>  {
>  	u16 *cache = codec->reg_cache;
> +	int ret;
>  
> +#if AC97_USE_CACHE
>  	switch (reg) {
>  	case AC97_RESET:
>  	case AC97_INT_PAGING:
> @@ -109,27 +114,38 @@ static unsigned int ac97_read(struct snd_soc_codec *codec,
>  	case AC97_EXTENDED_STATUS:
>  	case AC97_VENDOR_ID1:
>  	case AC97_VENDOR_ID2:
> -		return soc_ac97_ops.read(codec->ac97, reg);
> +#endif
> +		ret = soc_ac97_ops.read(codec->ac97, reg);
> +#if AC97_USE_CACHE
>  	default:
>  		reg = reg >> 1;
>  
>  		if (reg >= ARRAY_SIZE(ad1980_reg))
> -			return -EINVAL;
> -
> -		return cache[reg];
> +			ret = -EINVAL;
> +		else
> +			ret = cache[reg];
>  	}
> +#endif
> +
> +	pr_debug("%s: reg=0x%02x, val=0x%04x\n", __func__, reg, ret);
> +
> +	return ret;
>  }
>  
>  static int ac97_write(struct snd_soc_codec *codec, unsigned int reg,
>  	unsigned int val)
>  {
> -	u16 *cache = codec->reg_cache;
> +	pr_debug("%s: reg=0x%02x, val=0x%04x\n", __func__, reg, val);
>  
>  	soc_ac97_ops.write(codec->ac97, reg, val);
> +
> +#if AC97_USE_CACHE
>  	reg = reg >> 1;
> -	if (reg < ARRAY_SIZE(ad1980_reg))
> +	if (reg < ARRAY_SIZE(ad1980_reg)) {
> +		u16 *cache = codec->reg_cache;
>  		cache[reg] = val;
> -
> +	}
> +#endif
>  	return 0;
>  }
>  
> @@ -204,6 +220,7 @@ static int ad1980_soc_probe(struct platform_device *pdev)
>  	codec = socdev->card->codec;
>  	mutex_init(&codec->mutex);
>  
> +#if AC97_USE_CACHE
>  	codec->reg_cache =
>  		kzalloc(sizeof(u16) * ARRAY_SIZE(ad1980_reg), GFP_KERNEL);
>  	if (codec->reg_cache == NULL) {
> @@ -214,6 +231,7 @@ static int ad1980_soc_probe(struct platform_device *pdev)
>  			ARRAY_SIZE(ad1980_reg));
>  	codec->reg_cache_size = sizeof(u16) * ARRAY_SIZE(ad1980_reg);
>  	codec->reg_cache_step = 2;
> +#endif
>  	codec->name = "AD1980";
>  	codec->owner = THIS_MODULE;
>  	codec->dai = &ad1980_dai;
> @@ -279,9 +297,11 @@ pcm_err:
>  	snd_soc_free_ac97_codec(codec);
>  
>  codec_err:
> +#if AC97_USE_CACHE
>  	kfree(codec->reg_cache);
>  
>  cache_err:
> +#endif
>  	kfree(socdev->card->codec);
>  	socdev->card->codec = NULL;
>  	return ret;
> @@ -298,7 +318,9 @@ static int ad1980_soc_remove(struct platform_device *pdev)
>  	snd_soc_dapm_free(socdev);
>  	snd_soc_free_pcms(socdev);
>  	snd_soc_free_ac97_codec(codec);
> +#if AC97_USE_CACHE
>  	kfree(codec->reg_cache);
> +#endif
>  	kfree(codec);
>  	return 0;
>  }
> -- 
> 1.7.1
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

-- 
"You grabbed my hand and we fell into it, like a daydream - or a fever."

[PATCH 5/6] ASoC: ad1980: verify cache at probe time

[Uwe Kleine-König]

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 sound/soc/codecs/ad1980.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/sound/soc/codecs/ad1980.c b/sound/soc/codecs/ad1980.c
index 864e65d..a3e78d4 100644
--- a/sound/soc/codecs/ad1980.c
+++ b/sound/soc/codecs/ad1980.c
@@ -288,6 +288,19 @@ static int ad1980_soc_probe(struct platform_device *pdev)
 	snd_soc_add_controls(codec, ad1980_snd_ac97_controls,
 				ARRAY_SIZE(ad1980_snd_ac97_controls));
 
+#if AC97_USE_CACHE
+	pr_debug("%s: cache verify\n", __func__);
+	for (vendor_id2 = 0; vendor_id2 < 0x80; vendor_id2 += 2) {
+
+		u16 *cache = codec->reg_cache;
+		ret = soc_ac97_ops.read(codec->ac97, vendor_id2);
+		if (ret != cache[vendor_id2 >> 1])
+			pr_debug("%s: %02x -> %04x\n",
+					__func__, vendor_id2, ret);
+	}
+	pr_debug("%s: /cache verify\n", __func__);
+#endif
+
 	return 0;
 
 reset_err:
-- 
1.7.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 5/6] ASoC: ad1980: verify cache at probe time

[Mark Brown]

On Tue, Aug 24, 2010 at 03:09:43PM +0200, Uwe Kleine-K??nig wrote:

> Signed-off-by: Uwe Kleine-K??nig <u.kleine-koenig@pengutronix.de>

Pretty much the same comments apply to this as applied to the
immediately preceeding patch - doing it at driver level seems like the
wrong approacg and please always remember to CC maintainers on patches.
Also...

> ---
>  sound/soc/codecs/ad1980.c |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/sound/soc/codecs/ad1980.c b/sound/soc/codecs/ad1980.c
> index 864e65d..a3e78d4 100644
> --- a/sound/soc/codecs/ad1980.c
> +++ b/sound/soc/codecs/ad1980.c
> @@ -288,6 +288,19 @@ static int ad1980_soc_probe(struct platform_device *pdev)
>  	snd_soc_add_controls(codec, ad1980_snd_ac97_controls,
>  				ARRAY_SIZE(ad1980_snd_ac97_controls));
>  
> +#if AC97_USE_CACHE
> +	pr_debug("%s: cache verify\n", __func__);
> +	for (vendor_id2 = 0; vendor_id2 < 0x80; vendor_id2 += 2) {

...this should probably be selected separately and...

> +
> +		u16 *cache = codec->reg_cache;
> +		ret = soc_ac97_ops.read(codec->ac97, vendor_id2);
> +		if (ret != cache[vendor_id2 >> 1])
> +			pr_debug("%s: %02x -> %04x\n",
> +					__func__, vendor_id2, ret);

...this seems like the wrong priority if there are any inconsistencies.
Note that if the chip includes anything that might reasonably vary (like
revision information or status readback) you probably want to handle
that too.

> +	}
> +	pr_debug("%s: /cache verify\n", __func__);
> +#endif
> +
>  	return 0;
>  
>  reset_err:
> -- 
> 1.7.1
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

-- 
"You grabbed my hand and we fell into it, like a daydream - or a fever."

[PATCH 6/6] ASoC: ad1980: verify writes

[Uwe Kleine-König]

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 sound/soc/codecs/ad1980.c |   41 ++++++++++++++++++++++++++++++-----------
 1 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/sound/soc/codecs/ad1980.c b/sound/soc/codecs/ad1980.c
index a3e78d4..2851ca1 100644
--- a/sound/soc/codecs/ad1980.c
+++ b/sound/soc/codecs/ad1980.c
@@ -34,6 +34,8 @@
 #include "ad1980.h"
 
 #define AC97_USE_CACHE 1
+#define AC97_VERIFY_WRITE 0
+
 #if AC97_USE_CACHE
 /*
  * AD1980 register cache
@@ -100,13 +102,8 @@ SOC_ENUM("Capture Source", ad1980_cap_src),
 SOC_SINGLE("Mic Boost Switch", AC97_MIC, 6, 1, 0),
 };
 
-static unsigned int ac97_read(struct snd_soc_codec *codec,
-	unsigned int reg)
+static int ac97_reg_cacheable(unsigned int reg)
 {
-	u16 *cache = codec->reg_cache;
-	int ret;
-
-#if AC97_USE_CACHE
 	switch (reg) {
 	case AC97_RESET:
 	case AC97_INT_PAGING:
@@ -114,18 +111,30 @@ static unsigned int ac97_read(struct snd_soc_codec *codec,
 	case AC97_EXTENDED_STATUS:
 	case AC97_VENDOR_ID1:
 	case AC97_VENDOR_ID2:
-#endif
-		ret = soc_ac97_ops.read(codec->ac97, reg);
-#if AC97_USE_CACHE
+		return 0;
 	default:
-		reg = reg >> 1;
+		return 1;
+	}
+}
+
+static unsigned int ac97_read(struct snd_soc_codec *codec,
+	unsigned int reg)
+{
+	int ret;
+
+#if AC97_USE_CACHE
+	if (ac97_reg_cacheable(reg)) {
+		u16 *cache = codec->reg_cache;
+
+		reg >>= 1;
 
 		if (reg >= ARRAY_SIZE(ad1980_reg))
 			ret = -EINVAL;
 		else
 			ret = cache[reg];
-	}
+	} else
 #endif
+		ret = soc_ac97_ops.read(codec->ac97, reg);
 
 	pr_debug("%s: reg=0x%02x, val=0x%04x\n", __func__, reg, ret);
 
@@ -146,6 +155,16 @@ static int ac97_write(struct snd_soc_codec *codec, unsigned int reg,
 		cache[reg] = val;
 	}
 #endif
+
+#if AC97_VERIFY_WRITE
+	if (ac97_reg_cacheable(reg)) {
+		unsigned int valread = soc_ac97_ops.read(codec->ac97, reg);
+		if (valread != val)
+			pr_err("%s: reg=0x%02x, val=0x%04x, readback=0x%04x\n",
+					__func__, reg, val, valread);
+	}
+#endif
+
 	return 0;
 }
 
-- 
1.7.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 6/6] ASoC: ad1980: verify writes

[Mark Brown]

On Tue, Aug 24, 2010 at 03:09:44PM +0200, Uwe Kleine-K??nig wrote:
> Signed-off-by: Uwe Kleine-K??nig <u.kleine-koenig@pengutronix.de>

Again, please remember to CC maintainers on patches and doing this as
driver-specific code feels like we're solving things at the wrong level
- register I/O is something that could potentially be dodgy for any
device.

> ---
>  sound/soc/codecs/ad1980.c |   41 ++++++++++++++++++++++++++++++-----------
>  1 files changed, 30 insertions(+), 11 deletions(-)
> 
> diff --git a/sound/soc/codecs/ad1980.c b/sound/soc/codecs/ad1980.c
> index a3e78d4..2851ca1 100644
> --- a/sound/soc/codecs/ad1980.c
> +++ b/sound/soc/codecs/ad1980.c
> @@ -34,6 +34,8 @@
>  #include "ad1980.h"
>  
>  #define AC97_USE_CACHE 1
> +#define AC97_VERIFY_WRITE 0
> +
>  #if AC97_USE_CACHE
>  /*
>   * AD1980 register cache
> @@ -100,13 +102,8 @@ SOC_ENUM("Capture Source", ad1980_cap_src),
>  SOC_SINGLE("Mic Boost Switch", AC97_MIC, 6, 1, 0),
>  };
>  
> -static unsigned int ac97_read(struct snd_soc_codec *codec,
> -	unsigned int reg)
> +static int ac97_reg_cacheable(unsigned int reg)
>  {
> -	u16 *cache = codec->reg_cache;
> -	int ret;
> -
> -#if AC97_USE_CACHE
>  	switch (reg) {
>  	case AC97_RESET:
>  	case AC97_INT_PAGING:
> @@ -114,18 +111,30 @@ static unsigned int ac97_read(struct snd_soc_codec *codec,
>  	case AC97_EXTENDED_STATUS:
>  	case AC97_VENDOR_ID1:
>  	case AC97_VENDOR_ID2:
> -#endif
> -		ret = soc_ac97_ops.read(codec->ac97, reg);
> -#if AC97_USE_CACHE
> +		return 0;
>  	default:
> -		reg = reg >> 1;
> +		return 1;
> +	}
> +}
> +
> +static unsigned int ac97_read(struct snd_soc_codec *codec,
> +	unsigned int reg)
> +{
> +	int ret;
> +
> +#if AC97_USE_CACHE
> +	if (ac97_reg_cacheable(reg)) {
> +		u16 *cache = codec->reg_cache;
> +
> +		reg >>= 1;
>  
>  		if (reg >= ARRAY_SIZE(ad1980_reg))
>  			ret = -EINVAL;
>  		else
>  			ret = cache[reg];
> -	}
> +	} else
>  #endif
> +		ret = soc_ac97_ops.read(codec->ac97, reg);
>  
>  	pr_debug("%s: reg=0x%02x, val=0x%04x\n", __func__, reg, ret);
>  
> @@ -146,6 +155,16 @@ static int ac97_write(struct snd_soc_codec *codec, unsigned int reg,
>  		cache[reg] = val;
>  	}
>  #endif
> +
> +#if AC97_VERIFY_WRITE
> +	if (ac97_reg_cacheable(reg)) {
> +		unsigned int valread = soc_ac97_ops.read(codec->ac97, reg);
> +		if (valread != val)
> +			pr_err("%s: reg=0x%02x, val=0x%04x, readback=0x%04x\n",
> +					__func__, reg, val, valread);
> +	}
> +#endif
> +
>  	return 0;
>  }
>  
> -- 
> 1.7.1
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

-- 
"You grabbed my hand and we fell into it, like a daydream - or a fever."

Re: [PATCH 1/6] ASoC: ad1980: Stay in 20bit mode for architectures other than blackfin

[Zhang, Sonic]

Hi Sascha,

The ad1980 from ADI is obsolete. We have no plan to maintain this driver any more.


Thanks


Sonic

>-----Original Message-----
>From: u.kleine-koenig@pengutronix.de 
>[mailto:u.kleine-koenig@pengutronix.de] 
>Sent: Tuesday, August 24, 2010 9:10 PM
>To: alsa-devel@alsa-project.org; Zhang, Sonic
>Cc: Sascha Hauer
>Subject: [PATCH 1/6] ASoC: ad1980: Stay in 20bit mode for 
>architectures other than blackfin
>
>From: Sascha Hauer <s.hauer@pengutronix.de>
>
>Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>---
> sound/soc/codecs/ad1980.c |   10 +++++++++-
> 1 files changed, 9 insertions(+), 1 deletions(-)
>
>diff --git a/sound/soc/codecs/ad1980.c 
>b/sound/soc/codecs/ad1980.c index 70cfaec..35c0580 100644
>--- a/sound/soc/codecs/ad1980.c
>+++ b/sound/soc/codecs/ad1980.c
>@@ -168,13 +168,21 @@ retry:
> 	}
> 
> 	soc_ac97_ops.reset(codec->ac97);
>+
>+#ifdef SND_BF5XX_SOC_AD1980
> 	/* Set bit 16slot in register 74h, then every slot will 
>has only 16
> 	 * bits. This command is sent out in 20bit mode, in 
>which case the
>-	 * first nibble of data is eaten by the addr. (Tag is 
>always 16 bit)*/
>+	 * first nibble of data is eaten by the addr. (Tag is 
>always 16 bit).
>+	 *
>+	 * This seems to be blackfin specific. On other systems 
>we want 20bit
>+	 * mode.
>+	 */
> 	ac97_write(codec, AC97_AD_SERIAL_CFG, 0x9900);
>+#endif
> 
> 	if (ac97_read(codec, AC97_RESET)  != 0x0090)
> 		goto err;
>+
> 	return 0;
> 
> err:
>--
>1.7.1
>
>

Re: [PATCH 1/6] ASoC: ad1980: Stay in 20bit mode for architectures other than blackfin

[Uwe Kleine-König]

Hi Sonic,

On Wed, Aug 25, 2010 at 11:28:48AM +0800, Zhang, Sonic wrote:
> Hi Sascha,
s/Sascha/Uwe/
 
> The ad1980 from ADI is obsolete. We have no plan to maintain this
> driver any more.
I assume this means you don't object my patches to be merged?!

Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH 1/6] ASoC: ad1980: Stay in 20bit mode for architectures other than blackfin

[Mark Brown]

On Tue, Aug 24, 2010 at 03:09:39PM +0200, Uwe Kleine-K??nig wrote:
> From: Sascha Hauer <s.hauer@pengutronix.de>
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Uwe Kleine-K??nig <u.kleine-koenig@pengutronix.de>

Please remember to CC maintainers on patches.

> ---
>  sound/soc/codecs/ad1980.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/sound/soc/codecs/ad1980.c b/sound/soc/codecs/ad1980.c
> index 70cfaec..35c0580 100644
> --- a/sound/soc/codecs/ad1980.c
> +++ b/sound/soc/codecs/ad1980.c
> @@ -168,13 +168,21 @@ retry:
>  	}
>  
>  	soc_ac97_ops.reset(codec->ac97);
> +
> +#ifdef SND_BF5XX_SOC_AD1980
>  	/* Set bit 16slot in register 74h, then every slot will has only 16
>  	 * bits. This command is sent out in 20bit mode, in which case the
> -	 * first nibble of data is eaten by the addr. (Tag is always 16 bit)*/
> +	 * first nibble of data is eaten by the addr. (Tag is always 16 bit).
> +	 *
> +	 * This seems to be blackfin specific. On other systems we want 20bit
> +	 * mode.
> +	 */
>  	ac97_write(codec, AC97_AD_SERIAL_CFG, 0x9900);
> +#endif

If you're going to ifdef this out you should be ifdefing it based on the
architecture or the AC'97 controller rather than a specific machine
driver.

>  	if (ac97_read(codec, AC97_RESET)  != 0x0090)
>  		goto err;
> +
>  	return 0;

Random whitespace change...