[alsa-devel] [PATCH] ASoC: Fix wm8753 register cache size and initialization
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);
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.
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
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.
participants (2)
-
Lars-Peter Clausen -
Mark Brown