Re: [alsa-devel] ASoC: bug report for ssm2602
On 08/22/2014 11:21 AM, Scott Jiang wrote:
Hi Mark and Lars,
commit 9d863b88ec371491e926e0828dbe3d36ead0f6f9 Author: Lars-Peter Clausen lars@metafoo.de Date: Sat Aug 31 18:15:23 2013 +0200
ASoC: ssm2602: Fix cache sync The ssm2602 uses regmap for caching not soc-cache, so we need to use regcache_sync() instead of snd_soc_cache_sync().According my test, regcache_sync() can't recover register values when resume. While snd_soc_cache_sync() works fine. I used regmap_read() to read registers in regmap layer and found it's the same as suspend. But regcache_sync() didn't sync these registers correctly to hardware.
That does not make too much sense. snd_soc_cache_sync() is definitely a no-op when using regmap. Which kernel version are you on? Are there any additional changes to this driver in your tree?
- Lars
2014-08-22 17:47 GMT+08:00 Lars-Peter Clausen lars@metafoo.de:
On 08/22/2014 11:21 AM, Scott Jiang wrote:
Hi Mark and Lars,
commit 9d863b88ec371491e926e0828dbe3d36ead0f6f9 Author: Lars-Peter Clausen lars@metafoo.de Date: Sat Aug 31 18:15:23 2013 +0200
ASoC: ssm2602: Fix cache sync The ssm2602 uses regmap for caching not soc-cache, so we need to use regcache_sync() instead of snd_soc_cache_sync().According my test, regcache_sync() can't recover register values when resume. While snd_soc_cache_sync() works fine. I used regmap_read() to read registers in regmap layer and found it's the same as suspend. But regcache_sync() didn't sync these registers correctly to hardware.
That does not make too much sense. snd_soc_cache_sync() is definitely a no-op when using regmap. Which kernel version are you on? Are there any additional changes to this driver in your tree?
My version is 3.16. I compared with the latest 3.17 rc2 kernel, there is only one patch.
commit 00a37032a099572302ae89c893bd5c90b924c537 Author: Mark Brown broonie@linaro.org Date: Thu Jul 31 12:47:13 2014 +0100
ASoC: ssm2602: Convert to params_width()
The CODEC doesn't care how data is laid out in memory.
Signed-off-by: Mark Brown broonie@linaro.org Acked-by: Lars-Peter Clausen lars@metafoo.de
I believe this patch isn't related to the pm bug here. By the way, have you ever tested your patches on any blackfin board? Your patches for adau1761, ssm2602 and bf5xx-i2s all failed to run.
Scott
On Tue, Aug 26, 2014 at 02:11:20PM +0800, Scott Jiang wrote:
2014-08-22 17:47 GMT+08:00 Lars-Peter Clausen lars@metafoo.de:
ASoC: ssm2602: Fix cache sync
The ssm2602 uses regmap for caching not soc-cache, so we need to use regcache_sync() instead of snd_soc_cache_sync().
According my test, regcache_sync() can't recover register values when resume. While snd_soc_cache_sync() works fine. I used regmap_read() to read registers in regmap layer and found it's the same as suspend. But regcache_sync() didn't sync these registers correctly to hardware.
That does not make too much sense. snd_soc_cache_sync() is definitely a no-op when using regmap. Which kernel version are you on? Are there any additional changes to this driver in your tree?
If it is the above patch is the problem perhaps that the register map isn't getting marked as dirty so the cache sync is being suppressed?
I believe this patch isn't related to the pm bug here. By the way, have you ever tested your patches on any blackfin board? Your patches for adau1761, ssm2602 and bf5xx-i2s all failed to run.
I guess this is addressed to Lars-Peter but could you be more specific about the problems you are seeing?
On 08/26/2014 08:28 AM, Mark Brown wrote:
On Tue, Aug 26, 2014 at 02:11:20PM +0800, Scott Jiang wrote:
2014-08-22 17:47 GMT+08:00 Lars-Peter Clausen lars@metafoo.de:
ASoC: ssm2602: Fix cache syncThe ssm2602 uses regmap for caching not soc-cache, so we need to use regcache_sync() instead of snd_soc_cache_sync().According my test, regcache_sync() can't recover register values when resume. While snd_soc_cache_sync() works fine. I used regmap_read() to read registers in regmap layer and found it's the same as suspend. But regcache_sync() didn't sync these registers correctly to hardware.
That does not make too much sense. snd_soc_cache_sync() is definitely a no-op when using regmap. Which kernel version are you on? Are there any additional changes to this driver in your tree?
If it is the above patch is the problem perhaps that the register map isn't getting marked as dirty so the cache sync is being suppressed?
The core should mark it as dirty after calling the suspend callback. But there might be bugs preventing this. Try to make sure that regmap_mark_dirty() gets called for this device and also try to check what goes wrong in regcache_sync() and why it does not sync the registers.
- Lars
On Tue, Aug 26, 2014 at 08:43:14AM +0200, Lars-Peter Clausen wrote:
On 08/26/2014 08:28 AM, Mark Brown wrote:
If it is the above patch is the problem perhaps that the register map isn't getting marked as dirty so the cache sync is being suppressed?
The core should mark it as dirty after calling the suspend callback. But there might be bugs preventing this. Try to make sure that regmap_mark_dirty() gets called for this device and also try to check what goes wrong in regcache_sync() and why it does not sync the registers.
Indeed, I can't see any obvious breakage there and don't remember any fixes though I've not checked the old code properly.
Hi Mark and Lars,
If it is the above patch is the problem perhaps that the register map isn't getting marked as dirty so the cache sync is being suppressed?
I print some info in regcache_sync_block_single() and found some registers are skipped because the regmap thinks it's the same as default. I guess it's a bug here. When I use regmap_read(), these values are not equal to default.
I believe this patch isn't related to the pm bug here. By the way, have you ever tested your patches on any blackfin board? Your patches for adau1761, ssm2602 and bf5xx-i2s all failed to run.
I guess this is addressed to Lars-Peter but could you be more specific about the problems you are seeing?
bf5xx-i2s-pcm, I have sent a patch to Mark for this.
commit 30443408fd7201fd1911b09daccf92fae3cc700d Author: Scott Jiang scott.jiang.linux@gmail.com Date: Fri Jul 18 16:14:57 2014 +0800
ASoC: blackfin: use samples to set silence
The third parameter for snd_pcm_format_set_silence needs the number of samples instead of sample bytes.
In fact, test cases still report bugs for this driver after I fix this bug. There are many Xrun for TDM codecs. I guess you change the parameters in bf5xx_pcm_hardware for TDM to I2S.
ssm2602, can't recover registers after resume. adau1761, I'm not sure what the problem is. I'll take a look later.
Thanks, Scott
On 08/26/2014 08:11 AM, Scott Jiang wrote:
2014-08-22 17:47 GMT+08:00 Lars-Peter Clausen lars@metafoo.de:
On 08/22/2014 11:21 AM, Scott Jiang wrote:
Hi Mark and Lars,
commit 9d863b88ec371491e926e0828dbe3d36ead0f6f9 Author: Lars-Peter Clausen lars@metafoo.de Date: Sat Aug 31 18:15:23 2013 +0200
ASoC: ssm2602: Fix cache sync The ssm2602 uses regmap for caching not soc-cache, so we need to use regcache_sync() instead of snd_soc_cache_sync().According my test, regcache_sync() can't recover register values when resume. While snd_soc_cache_sync() works fine. I used regmap_read() to read registers in regmap layer and found it's the same as suspend. But regcache_sync() didn't sync these registers correctly to hardware.
That does not make too much sense. snd_soc_cache_sync() is definitely a no-op when using regmap. Which kernel version are you on? Are there any additional changes to this driver in your tree?
My version is 3.16. I compared with the latest 3.17 rc2 kernel, there is only one patch.
And it is vanilla v3.16, no patches to the driver or the ASoC core? snd_soc_cache_sync() will definitely do nothing for this driver on v3.16 since reg_cache_size is 0 for this driver's snd_soc_codec_driver.
[...]
By the way, have you ever tested your patches on any blackfin board? Your patches for adau1761, ssm2602 and bf5xx-i2s all failed to run.
Yes, I've tested them before submitting them upstream on a ezkit board. What kind of errors are you seeing?
- Lars
participants (3)
-
Lars-Peter Clausen -
Mark Brown -
Scott Jiang