[alsa-devel] [PATCH] ASoC: soc-core: Add support for NULL default register caches
The infrastructure for handling NULL default register maps is already included in the soc-cache code, just ensure that we don't try to dereference a NULL pointer while accessing a NULL register cache.
Signed-off-by: Dimitris Papastamos dp@opensource.wolfsonmicro.com --- sound/soc/soc-core.c | 12 +++++++----- 1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index bac7291..a9f19c1 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -3503,11 +3503,13 @@ int snd_soc_register_codec(struct device *dev, * kernel might have freed the array by the time we initialize * the cache. */ - codec->reg_def_copy = kmemdup(codec_drv->reg_cache_default, - reg_size, GFP_KERNEL); - if (!codec->reg_def_copy) { - ret = -ENOMEM; - goto fail; + if (codec_drv->reg_cache_default) { + codec->reg_def_copy = kmemdup(codec_drv->reg_cache_default, + reg_size, GFP_KERNEL); + if (!codec->reg_def_copy) { + ret = -ENOMEM; + goto fail; + } } }
On Mon, 2011-01-10 at 10:10 +0000, Dimitris Papastamos wrote:
The infrastructure for handling NULL default register maps is already included in the soc-cache code, just ensure that we don't try to dereference a NULL pointer while accessing a NULL register cache.
Signed-off-by: Dimitris Papastamos dp@opensource.wolfsonmicro.com
sound/soc/soc-core.c | 12 +++++++----- 1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index bac7291..a9f19c1 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -3503,11 +3503,13 @@ int snd_soc_register_codec(struct device *dev, * kernel might have freed the array by the time we initialize * the cache. */
codec->reg_def_copy = kmemdup(codec_drv->reg_cache_default,reg_size, GFP_KERNEL);if (!codec->reg_def_copy) {ret = -ENOMEM;goto fail;
if (codec_drv->reg_cache_default) {codec->reg_def_copy = kmemdup(codec_drv->reg_cache_default,reg_size, GFP_KERNEL);if (!codec->reg_def_copy) {ret = -ENOMEM;goto fail;}
Acked-by: Liam Girdwood lrg@slimlogic.co.uk
Dimitris Papastamos wrote:
The infrastructure for handling NULL default register maps is already included in the soc-cache code, just ensure that we don't try to dereference a NULL pointer while accessing a NULL register cache.
Signed-off-by: Dimitris Papastamos dp@opensource.wolfsonmicro.com
Acked-by: Timur Tabi timur@freescale.com
This fixes the kernel panic I was seeing earlier with my CS4270 driver, but it's not a replacement for my patch, "[v3] ASoC: cs4270: use the built-in register cache support".
With Dimitris' patch alone, the CS4270 still has other bugs:
# cat ./devices/platform/soc-audio/playback/codec_reg cs4270-codec.0-004f registers 0: <no data: -5> 1: c3 2: 0 3: 30 4: 0 5: 0 6: 0 7: 0
So both patches need to be applied, however, I can't say whether Dimitris' patch is sufficient for handling NULL default register maps, since I removed that from my driver.
On Mon, Jan 10, 2011 at 04:10:52PM -0600, Timur Tabi wrote:
# cat ./devices/platform/soc-audio/playback/codec_reg cs4270-codec.0-004f registers 0: <no data: -5> 1: c3 2: 0 3: 30 4: 0 5: 0 6: 0 7: 0
So both patches need to be applied, however, I can't say whether Dimitris' patch is sufficient for handling NULL default register maps, since I removed that from my driver.
You've not identified what the issue is with the above? Is it just that nothing else ensured the cache was set up (eg, by doing reads from the hardware)?
Mark Brown wrote:
You've not identified what the issue is with the above? Is it just that nothing else ensured the cache was set up (eg, by doing reads from the hardware)?
The CS4270 registers are numbered from 1 through 8, so codec_reg should look like this:
1: c3 2: 0 3: 30 4: 0 5: 0 6: 0 7: 0 8: 0
What I don't know is why the bad output, but it's probably because the register cache code broke during the various changes applied to it since multi-component was introduced. Originally, the driver handled the register cache completely internally. Then, as register caching was added to ASoC itself, the driver was modified to use it. I believe that those modifications were not really tested.
And since I removed all that code for my patch, I'm not really inclined to go back and see what specifically was wrong with it.
On Mon, Jan 10, 2011 at 04:40:00PM -0600, Timur Tabi wrote:
The CS4270 registers are numbered from 1 through 8, so codec_reg should look like this:
1: c3 2: 0 3: 30 4: 0 5: 0 6: 0 7: 0 8: 0
What I don't know is why the bad output, but it's probably because the register cache code broke during the various changes applied to it since multi-component was introduced. Originally, the driver handled the register cache completely internally. Then, as register caching was added to ASoC itself, the driver was modified to use it. I believe that those modifications were not really tested.
The issue is that you were using 1 based array indexing and the core uses zero based indexing - nothing dramatically wrong and it should've fallen through to using the hardware I/O when it went beyond the cache so I'd expect things to work fine. The read failure for register zero was handled gracefully, so it looks like everything did what I'd expect here.
On Mon, Jan 10, 2011 at 10:10:56AM +0000, Dimitris Papastamos wrote:
The infrastructure for handling NULL default register maps is already included in the soc-cache code, just ensure that we don't try to dereference a NULL pointer while accessing a NULL register cache.
Signed-off-by: Dimitris Papastamos dp@opensource.wolfsonmicro.com
Applied, thanks.
participants (4)
-
Dimitris Papastamos -
Liam Girdwood -
Mark Brown -
Timur Tabi