[PATCH] ASoC: ak4642: Add ak4642_fill_cache function

[PATCH] ASoC: ak4642: Add ak4642_fill_cache function

[Kuninori Morimoto]

ak4642 driver is used for ak4642/ak4643.
So, reading real register value is better than using constant value.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/codecs/ak4642.c |   50 +++++++++++++++-----------------------------
 1 files changed, 17 insertions(+), 33 deletions(-)

diff --git a/sound/soc/codecs/ak4642.c b/sound/soc/codecs/ak4642.c
index 8d56811..9a6157f 100644
--- a/sound/soc/codecs/ak4642.c
+++ b/sound/soc/codecs/ak4642.c
@@ -106,25 +106,19 @@ struct snd_soc_codec_device soc_codec_dev_ak4642;
 /* codec private data */
 struct ak4642_priv {
 	struct snd_soc_codec codec;
+	u8 reg_cache[AK4642_CACHEREGNUM];
 };
 
 static struct snd_soc_codec *ak4642_codec;
 
-/*
- * ak4642 register cache
- */
-static const u16 ak4642_reg[AK4642_CACHEREGNUM] = {
-	0x0000, 0x0000, 0x0001, 0x0000,
-	0x0002, 0x0000, 0x0000, 0x0000,
-	0x00e1, 0x00e1, 0x0018, 0x0000,
-	0x00e1, 0x0018, 0x0011, 0x0008,
-	0x0000, 0x0000, 0x0000, 0x0000,
-	0x0000, 0x0000, 0x0000, 0x0000,
-	0x0000, 0x0000, 0x0000, 0x0000,
-	0x0000, 0x0000, 0x0000, 0x0000,
-	0x0000, 0x0000, 0x0000, 0x0000,
-	0x0000,
-};
+static void ak4642_fill_cache(struct snd_soc_codec *codec)
+{
+	int i;
+	u8 *cache = codec->reg_cache;
+
+	for (i = 0; i < AK4642_CACHEREGNUM; i++)
+		i2c_master_recv(codec->control_data, cache + i, 1);
+}
 
 /*
  * read ak4642 register cache
@@ -132,7 +126,7 @@ static const u16 ak4642_reg[AK4642_CACHEREGNUM] = {
 static inline unsigned int ak4642_read_reg_cache(struct snd_soc_codec *codec,
 	unsigned int reg)
 {
-	u16 *cache = codec->reg_cache;
+	u8 *cache = codec->reg_cache;
 	if (reg >= AK4642_CACHEREGNUM)
 		return -1;
 	return cache[reg];
@@ -144,7 +138,7 @@ static inline unsigned int ak4642_read_reg_cache(struct snd_soc_codec *codec,
 static inline void ak4642_write_reg_cache(struct snd_soc_codec *codec,
 	u16 reg, unsigned int value)
 {
-	u16 *cache = codec->reg_cache;
+	u8 *cache = codec->reg_cache;
 	if (reg >= AK4642_CACHEREGNUM)
 		return;
 
@@ -175,7 +169,7 @@ static int ak4642_write(struct snd_soc_codec *codec, unsigned int reg,
 
 static int ak4642_sync(struct snd_soc_codec *codec)
 {
-	u16 *cache = codec->reg_cache;
+	u8 *cache = codec->reg_cache;
 	int i, r = 0;
 
 	for (i = 0; i < AK4642_CACHEREGNUM; i++)
@@ -432,12 +426,9 @@ static int ak4642_init(struct ak4642_priv *ak4642)
 	codec->dai		= &ak4642_dai;
 	codec->num_dai		= 1;
 	codec->hw_write		= (hw_write_t)i2c_master_send;
-	codec->reg_cache_size	= ARRAY_SIZE(ak4642_reg);
-	codec->reg_cache	= kmemdup(ak4642_reg,
-					  sizeof(ak4642_reg), GFP_KERNEL);
-
-	if (!codec->reg_cache)
-		return -ENOMEM;
+	codec->reg_cache_size	= AK4642_CACHEREGNUM;
+	codec->reg_cache	= ak4642->reg_cache;
+	ak4642_fill_cache(codec);
 
 	ak4642_dai.dev = codec->dev;
 	ak4642_codec = codec;
@@ -445,23 +436,17 @@ static int ak4642_init(struct ak4642_priv *ak4642)
 	ret = snd_soc_register_codec(codec);
 	if (ret) {
 		dev_err(codec->dev, "Failed to register codec: %d\n", ret);
-		goto reg_cache_err;
+		return ret;
 	}
 
 	ret = snd_soc_register_dai(&ak4642_dai);
 	if (ret) {
 		dev_err(codec->dev, "Failed to register DAI: %d\n", ret);
 		snd_soc_unregister_codec(codec);
-		goto reg_cache_err;
+		return ret;
 	}
 
 	return ret;
-
-reg_cache_err:
-	kfree(codec->reg_cache);
-	codec->reg_cache = NULL;
-
-	return ret;
 }
 
 #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
@@ -495,7 +480,6 @@ static int ak4642_i2c_remove(struct i2c_client *client)
 
 	snd_soc_unregister_dai(&ak4642_dai);
 	snd_soc_unregister_codec(&ak4642->codec);
-	kfree(ak4642->codec.reg_cache);
 	kfree(ak4642);
 	ak4642_codec = NULL;
 
-- 
1.7.0.4

Re: [PATCH] ASoC: ak4642: Add ak4642_fill_cache function

[Liam Girdwood]

On Fri, 2010-06-25 at 16:54 +0900, Kuninori Morimoto wrote:
> ak4642 driver is used for ak4642/ak4643.
> So, reading real register value is better than using constant value.
> 

To be honest, this is only really useful when the codec has some
volatile registers.  Forcing I2C reads for all IO will just slow down
the driver. 

Does this codec have volatile registers ?

Thanks

Liam
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH] ASoC: ak4642: Add ak4642_fill_cache function

[Mark Brown]

On Fri, Jun 25, 2010 at 04:54:32PM +0900, Kuninori Morimoto wrote:
> ak4642 driver is used for ak4642/ak4643.
> So, reading real register value is better than using constant value.

Better yet would be to just select the appropriate set of register
defaults based on the device being used at system startup - that way the
driver gets the benefits of the default table (eg, reducing the number
of writes done at system startup and resume) and doesn't need to delay
boot to read the full register map.

Re: [PATCH] ASoC: ak4642: Add ak4642_fill_cache function

[Kuninori Morimoto]

Dear Mark, Liam

> Better yet would be to just select the appropriate set of register
> defaults based on the device being used at system startup - that way the
> driver gets the benefits of the default table (eg, reducing the number
> of writes done at system startup and resume) and doesn't need to delay
> boot to read the full register map.
(snip)
> To be honest, this is only really useful when the codec has some
> volatile registers.  Forcing I2C reads for all IO will just slow down
> the driver. 

I understand about this.
sorry. I just didn't know that.

In my check, it seems don't have volatile registers.
so please drop it.

Best regards
--
Kuninori Morimoto