[PATCH] ASoC: Fix wm8753 register cache size and initialization

[PATCH] ASoC: Fix wm8753 register cache size and initialization

[Lars-Peter Clausen]

The register cache size is of by one. There are 63 registers in use but the
register cache size has only space for 62.
Furthermore the codec's reg_cache_size is of by another one. Since the wm8753
register cache uses one-based indexing we have to add one to its size.

Register cache initialization only copied the first sizeof(void*) elements leaving the others
uninitialized. Fix it by using the size of the reg cache template.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/codecs/wm8753.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/wm8753.c b/sound/soc/codecs/wm8753.c
index d28eeac..18a3ef9 100644
--- a/sound/soc/codecs/wm8753.c
+++ b/sound/soc/codecs/wm8753.c
@@ -79,7 +79,7 @@ static const u16 wm8753_reg[] = {
 	0x0097, 0x0097, 0x0000, 0x0004,
 	0x0000, 0x0083, 0x0024, 0x01ba,
 	0x0000, 0x0083, 0x0024, 0x01ba,
-	0x0000, 0x0000
+	0x0000, 0x0000, 0x0000
 };
 
 /* codec private data */
@@ -1660,11 +1660,11 @@ static int wm8753_register(struct wm8753_priv *wm8753)
 	codec->set_bias_level = wm8753_set_bias_level;
 	codec->dai = wm8753_dai;
 	codec->num_dai = 2;
-	codec->reg_cache_size = ARRAY_SIZE(wm8753->reg_cache);
+	codec->reg_cache_size = ARRAY_SIZE(wm8753->reg_cache) + 1;
 	codec->reg_cache = &wm8753->reg_cache;
 	codec->private_data = wm8753;
 
-	memcpy(codec->reg_cache, wm8753_reg, sizeof(codec->reg_cache));
+	memcpy(codec->reg_cache, wm8753_reg, sizeof(wm8753_reg));
 	INIT_DELAYED_WORK(&codec->delayed_work, wm8753_work);
 
 	ret = wm8753_reset(codec);
-- 
1.5.6.5

[PATCH] ASoC: Fix wm8753 register cache size and initialization

[Lars-Peter Clausen]

The register cache size is of by one. There are 63 registers in use but the
register cache size has only space for 62.
Furthermore the codec's reg_cache_size is of by another one. Since the wm8753
register cache uses one-based indexing we have to add one to its size.

Register cache initialization only copied the first sizeof(void*) elements leaving the others
uninitialized. Fix it by using the size of the reg cache template.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/codecs/wm8753.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/wm8753.c b/sound/soc/codecs/wm8753.c
index d28eeac..18a3ef9 100644
--- a/sound/soc/codecs/wm8753.c
+++ b/sound/soc/codecs/wm8753.c
@@ -79,7 +79,7 @@ static const u16 wm8753_reg[] = {
 	0x0097, 0x0097, 0x0000, 0x0004,
 	0x0000, 0x0083, 0x0024, 0x01ba,
 	0x0000, 0x0083, 0x0024, 0x01ba,
-	0x0000, 0x0000
+	0x0000, 0x0000, 0x0000
 };
 
 /* codec private data */
@@ -1660,11 +1660,11 @@ static int wm8753_register(struct wm8753_priv *wm8753)
 	codec->set_bias_level = wm8753_set_bias_level;
 	codec->dai = wm8753_dai;
 	codec->num_dai = 2;
-	codec->reg_cache_size = ARRAY_SIZE(wm8753->reg_cache);
+	codec->reg_cache_size = ARRAY_SIZE(wm8753->reg_cache) + 1;
 	codec->reg_cache = &wm8753->reg_cache;
 	codec->private_data = wm8753;
 
-	memcpy(codec->reg_cache, wm8753_reg, sizeof(codec->reg_cache));
+	memcpy(codec->reg_cache, wm8753_reg, sizeof(wm8753_reg));
 	INIT_DELAYED_WORK(&codec->delayed_work, wm8753_work);
 
 	ret = wm8753_reset(codec);
-- 
1.5.6.5

Re: [PATCH] ASoC: Fix wm8753 register cache size and initialization

[Mark Brown]

On Fri, Jul 03, 2009 at 12:46:08AM +0200, Lars-Peter Clausen wrote:

> The register cache size is of by one. There are 63 registers in use but the
> register cache size has only space for 62.

Only 62 of the registers are cached - please see the register cache
access code.

> Furthermore the codec's reg_cache_size is of by another one. Since the wm8753
> register cache uses one-based indexing we have to add one to its size.

I'm not 100% sure what you mean here but I suspect you're misreading the
register cache access code?

> Register cache initialization only copied the first sizeof(void*) elements leaving the others
> uninitialized. Fix it by using the size of the reg cache template.

This fix is good (but should use wm8753->reg_cache for the size).  I'll
fix this.

Re: [PATCH] ASoC: Fix wm8753 register cache size and initialization

[Lars-Peter Clausen]

Mark Brown wrote:
> On Fri, Jul 03, 2009 at 12:46:08AM +0200, Lars-Peter Clausen wrote:
>
>   
>> The register cache size is of by one. There are 63 registers in use but the
>> register cache size has only space for 62.
>>     
>
> Only 62 of the registers are cached - please see the register cache
> access code.
>   
Yes. Thats the problem. The register cache holds place for 62 elements 
where as there are 63 register which should be cached.
In the register cache access code you subtract one from the registers 
index to get it's index in the register cache array, so the last 
register has the index 62. Which means it is the 63th element of the 
array and thus the array has to consist of 63 elements.

Currently reads to WM8753_ADCTL2 will always return -1 as it is out of 
the register cache bounds which causes the OUT4 control to be unusable 
and as a side effect audio volume will be quite low.
>   
>> Furthermore the codec's reg_cache_size is of by another one. Since the wm8753
>> register cache uses one-based indexing we have to add one to its size.
>>     
>
> I'm not 100% sure what you mean here but I suspect you're misreading the
> register cache access code?
>   
The register cache access code uses one-based indexing so the first 
element in the register cache array will be the register with the number 
1 and the last register has the number ARRAY_SIZE(wm8753_reg).
Now if you set reg_cache_size to ARRAY_SIZE(wm8753_reg) the last 
register will never show up in devices codec_reg sysfs file as its index 
is not lower the reg_cache_size.

- Lars

Re: [PATCH] ASoC: Fix wm8753 register cache size and initialization

[Mark Brown]

On Fri, Jul 03, 2009 at 12:15:45PM +0200, Lars-Peter Clausen wrote:
> Mark Brown wrote:

> >Only 62 of the registers are cached - please see the register cache
> >access code.

> Yes. Thats the problem. The register cache holds place for 62
> elements where as there are 63 register which should be cached.
> In the register cache access code you subtract one from the
> registers index to get it's index in the register cache array, so
> the last register has the index 62. Which means it is the 63th
> element of the array and thus the array has to consist of 63
> elements.

OK, that makes sense.  I will apply your patch with a rewritten commit
message which explains the issue - the problem is not that there is one
less register in the cache than is in use, it's that there is one less
register in the cache than is *cached*.  As I say the code deliberately
doesn't cache one of the registers.

> >I'm not 100% sure what you mean here but I suspect you're misreading the
> >register cache access code?

> The register cache access code uses one-based indexing so the first

Right, that makes sense with the above.