[alsa-devel] [PATCH 1/3] ASoC: alc5623: Convert codec->hw_read to regmap_read
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)
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;
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);
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.
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
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.
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)
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.
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.
participants (2)
-
Axel Lin -
Mark Brown