[PATCH 1/3] ASoC: alc5623: Convert codec->hw_read to regmap_read

[PATCH 1/3] ASoC: alc5623: Convert codec->hw_read to regmap_read

[Axel Lin]

codec->hw_read is broken now, let's covert to regmap_read.

Signed-off-by: Axel Lin <axel.lin@gmail.com>
---
 sound/soc/codecs/alc5623.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/alc5623.c b/sound/soc/codecs/alc5623.c
index 557b3af..8c156cb 100644
--- a/sound/soc/codecs/alc5623.c
+++ b/sound/soc/codecs/alc5623.c
@@ -51,10 +51,13 @@ static void alc5623_fill_cache(struct snd_soc_codec *codec)
 {
 	int i, step = codec->driver->reg_cache_step;
 	u16 *cache = codec->reg_cache;
+	unsigned int val;
 
 	/* not really efficient ... */
-	for (i = 0 ; i < codec->driver->reg_cache_size ; i += step)
-		cache[i] = codec->hw_read(codec, i);
+	for (i = 0 ; i < codec->driver->reg_cache_size ; i += step) {
+		regmap_read(codec->control_data, i, &val);
+		cache[i] = val;
+	}
 }
 
 static inline int alc5623_reset(struct snd_soc_codec *codec)
-- 
1.7.4.1

[PATCH 2/3] ASoC: tlv320aic3x: Convert codec->hw_read to regmap_read

[Axel Lin]

codec->hw_read is broken now, let's covert to regmap_read.

Signed-off-by: Axel Lin <axel.lin@gmail.com>
---
 sound/soc/codecs/tlv320aic3x.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c
index be55b7f..8775d52 100644
--- a/sound/soc/codecs/tlv320aic3x.c
+++ b/sound/soc/codecs/tlv320aic3x.c
@@ -131,13 +131,19 @@ static int aic3x_read(struct snd_soc_codec *codec, unsigned int reg,
 		      u8 *value)
 {
 	u8 *cache = codec->reg_cache;
+	unsigned int val;
+	int ret;
 
 	if (codec->cache_only)
 		return -EINVAL;
 	if (reg >= AIC3X_CACHEREGNUM)
 		return -1;
 
-	*value = codec->hw_read(codec, reg);
+	ret = regmap_read(codec->control_data, reg, &val);
+	if (ret)
+		return ret;
+
+	*value = val;
 	cache[reg] = *value;
 
 	return 0;
-- 
1.7.4.1

[PATCH 3/3] ASoC: wm8961: Convert codec->hw_read to regmap_read

[Axel Lin]

codec->hw_read is broken now, let's covert to regmap_read.

Signed-off-by: Axel Lin <axel.lin@gmail.com>
---
 sound/soc/codecs/wm8961.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/wm8961.c b/sound/soc/codecs/wm8961.c
index cdee810..2fa1672 100644
--- a/sound/soc/codecs/wm8961.c
+++ b/sound/soc/codecs/wm8961.c
@@ -960,6 +960,7 @@ static int wm8961_probe(struct snd_soc_codec *codec)
 	struct snd_soc_dapm_context *dapm = &codec->dapm;
 	int ret = 0;
 	u16 reg;
+	unsigned int val;
 
 	ret = snd_soc_codec_set_cache_io(codec, 8, 16, SND_SOC_I2C);
 	if (ret != 0) {
@@ -974,10 +975,10 @@ static int wm8961_probe(struct snd_soc_codec *codec)
 	}
 
 	/* This isn't volatile - readback doesn't correspond to write */
-	reg = codec->hw_read(codec, WM8961_RIGHT_INPUT_VOLUME);
+	regmap_read(codec->control_data, WM8961_RIGHT_INPUT_VOLUME, &val);
 	dev_info(codec->dev, "WM8961 family %d revision %c\n",
-		 (reg & WM8961_DEVICE_ID_MASK) >> WM8961_DEVICE_ID_SHIFT,
-		 ((reg & WM8961_CHIP_REV_MASK) >> WM8961_CHIP_REV_SHIFT)
+		 (val & WM8961_DEVICE_ID_MASK) >> WM8961_DEVICE_ID_SHIFT,
+		 ((val & WM8961_CHIP_REV_MASK) >> WM8961_CHIP_REV_SHIFT)
 		 + 'A');
 
 	ret = wm8961_reset(codec);
-- 
1.7.4.1

Re: [PATCH 1/3] ASoC: alc5623: Convert codec->hw_read to regmap_read

[Mark Brown]

On Thu, Oct 13, 2011 at 03:22:55PM +0800, Axel Lin wrote:
> codec->hw_read is broken now, let's covert to regmap_read.
> 
> Signed-off-by: Axel Lin <axel.lin@gmail.com>

I'd rather not fix these like this because when we convert these drivers
to use regmap directly it won't be quite so obvious what's going on and
we'll easily introduce bugs where we start using the cached values from
a regmap level cache.

Probably the best thing here is to convert all these drivers to use
cache_bypass to read directly through the cache - the same thing will
then work with direct regmap usage.

Re: [PATCH 1/3] ASoC: alc5623: Convert codec->hw_read to regmap_read

[Axel Lin]

2011/10/14 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Thu, Oct 13, 2011 at 03:22:55PM +0800, Axel Lin wrote:
>> codec->hw_read is broken now, let's covert to regmap_read.
>>
>> Signed-off-by: Axel Lin <axel.lin@gmail.com>
>
> I'd rather not fix these like this because when we convert these drivers
> to use regmap directly it won't be quite so obvious what's going on and
> we'll easily introduce bugs where we start using the cached values from
> a regmap level cache.
>
> Probably the best thing here is to convert all these drivers to use
> cache_bypass to read directly through the cache - the same thing will
> then work with direct regmap usage.
>
I don't complete understand "use cache_bypass to read directly through
the cache".
Can you illustrate it?
The purpose of original implementation is to read h/w then fill the cache.
The cache is zeroed before we call alc5623_fill_cache().

Or do you mean changes like below instead of calling regmap_read directly?

diff --git a/sound/soc/codecs/alc5623.c b/sound/soc/codecs/alc5623.c
index 557b3af..43006b3 100644
--- a/sound/soc/codecs/alc5623.c
+++ b/sound/soc/codecs/alc5623.c
@@ -51,10 +51,15 @@ static void alc5623_fill_cache(struct snd_soc_codec *codec)
 {
        int i, step = codec->driver->reg_cache_step;
        u16 *cache = codec->reg_cache;
+       unsigned int val;

        /* not really efficient ... */
-       for (i = 0 ; i < codec->driver->reg_cache_size ; i += step)
-               cache[i] = codec->hw_read(codec, i);
+       codec->cache_bypass = 1;
+       for (i = 0 ; i < codec->driver->reg_cache_size ; i += step) {
+               snd_soc_read(codec, i);
+               cache[i] = val;
+       }
+       codec->cache_bypass = 0;
 }

 static inline int alc5623_reset(struct snd_soc_codec *codec)

Thanks,
Axel

Re: [PATCH 1/3] ASoC: alc5623: Convert codec->hw_read to regmap_read

[Mark Brown]

On Fri, Oct 14, 2011 at 06:39:43AM +0800, Axel Lin wrote:
> 2011/10/14 Mark Brown <broonie@opensource.wolfsonmicro.com>:

> > Probably the best thing here is to convert all these drivers to use
> > cache_bypass to read directly through the cache - the same thing will
> > then work with direct regmap usage.

> I don't complete understand "use cache_bypass to read directly through
> the cache".

Set the cache_bypass flag in the device struct.

> Or do you mean changes like below instead of calling regmap_read directly?

No, this isn't going to work.  snd_soc_read() will use the cache.

Re: [PATCH 1/3] ASoC: alc5623: Convert codec->hw_read to regmap_read

[Axel Lin]

2011/10/14 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Fri, Oct 14, 2011 at 06:39:43AM +0800, Axel Lin wrote:
>> 2011/10/14 Mark Brown <broonie@opensource.wolfsonmicro.com>:
>
>> > Probably the best thing here is to convert all these drivers to use
>> > cache_bypass to read directly through the cache - the same thing will
>> > then work with direct regmap usage.
>
>> I don't complete understand "use cache_bypass to read directly through
>> the cache".
>
> Set the cache_bypass flag in the device struct.
>
Sorry. I still don't get it.

I check the regmap code, it seems cache_bypsss is only set
if map->cache_type == REGCACHE_NONE or by regcache_cache_bypass().

I'm wondering if we can fix it this way:

diff --git a/sound/soc/codecs/alc5623.c b/sound/soc/codecs/alc5623.c
index 557b3af..9e43bb9 100644
--- a/sound/soc/codecs/alc5623.c
+++ b/sound/soc/codecs/alc5623.c
@@ -51,10 +51,15 @@ static void alc5623_fill_cache(struct snd_soc_codec *codec)
 {
        int i, step = codec->driver->reg_cache_step;
        u16 *cache = codec->reg_cache;
+       unsigned int val;

        /* not really efficient ... */
-       for (i = 0 ; i < codec->driver->reg_cache_size ; i += step)
-               cache[i] = codec->hw_read(codec, i);
+       regcache_cache_bypass(codec->control_data, true);
+       for (i = 0 ; i < codec->driver->reg_cache_size ; i += step) {
+               regmap_read(codec->control_data, i, &val);
+               cache[i] = val;
+       }
+       regcache_cache_bypass(codec->control_data, false);
 }

 static inline int alc5623_reset(struct snd_soc_codec *codec)

Re: [PATCH 1/3] ASoC: alc5623: Convert codec->hw_read to regmap_read

[Mark Brown]

On Fri, Oct 14, 2011 at 07:29:33AM +0800, Axel Lin wrote:

> I check the regmap code, it seems cache_bypsss is only set
> if map->cache_type == REGCACHE_NONE or by regcache_cache_bypass().

That's in regmap.  Any ASoC drivers which haven't been converted to
directly use regmap will still use the ASoC copy of the cache code.

> +       regcache_cache_bypass(codec->control_data, true);
> +       for (i = 0 ; i < codec->driver->reg_cache_size ; i += step) {
> +               regmap_read(codec->control_data, i, &val);
> +               cache[i] = val;
> +       }
> +       regcache_cache_bypass(codec->control_data, false);

This is what a driver which uses regmap directly should do.

Re: [PATCH 1/3] ASoC: alc5623: Convert codec->hw_read to regmap_read

[Mark Brown]

On Fri, Oct 14, 2011 at 06:39:43AM +0800, Axel Lin wrote:

> +       codec->cache_bypass = 1;
> +       for (i = 0 ; i < codec->driver->reg_cache_size ; i += step) {
> +               snd_soc_read(codec, i);
> +               cache[i] = val;
> +       }
> +       codec->cache_bypass = 0;

Oh, sorry - I misread this and this is what I'm expecting.