[alsa-devel] ASoC updates for 2.6.33
The following changes since commit 92dcffb916d309aa01778bf8963a6932e4014d07: Linus Torvalds (1): Linux 2.6.33-rc5
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound-2.6.git for-2.6.33
The pull request includes a much bigger delta against Linus' tree, I merged up since the last batch of fixes that went into his tree didn't seem to have shown up on your fix branch for some reason.
Guennadi Liakhovetski (1): ASoC: fix a memory-leak in wm8903
sound/soc/codecs/wm8903.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
At Mon, 25 Jan 2010 14:49:47 +0000, Mark Brown wrote:
The following changes since commit 92dcffb916d309aa01778bf8963a6932e4014d07: Linus Torvalds (1): Linux 2.6.33-rc5
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound-2.6.git for-2.6.33
The pull request includes a much bigger delta against Linus' tree, I merged up since the last batch of fixes that went into his tree didn't seem to have shown up on your fix branch for some reason.
Guennadi Liakhovetski (1): ASoC: fix a memory-leak in wm8903
Pulled now. Thanks.
But, looking through the change, I wonder whether we don't need to resume other registers.
static int wm8903_resume(struct platform_device *pdev) { ... u16 *reg_cache = codec->reg_cache; u16 *tmp_cache = kmemdup(reg_cache, sizeof(wm8903_reg_defaults), GFP_KERNEL);
/* Bring the codec back up to standby first to minimise pop/clicks */ wm8903_set_bias_level(codec, SND_SOC_BIAS_STANDBY); wm8903_set_bias_level(codec, codec->suspend_bias_level);
/* Sync back everything else */ if (tmp_cache) { for (i = 2; i < ARRAY_SIZE(wm8903_reg_defaults); i++) if (tmp_cache[i] != reg_cache[i]) snd_soc_write(codec, i, tmp_cache[i]); kfree(tmp_cache);
So, basically you restore the value changed between the beginning of the resume and after the call of wm8903_set_bias_levels(). What if the value was changed before the resume call? Shouldn't it be like
if (tmp_cache[i] != reg_cache[i] || tmp_cache[i] != wm8903_reg_defaults[i]) snd_soc_write(codec, i, tmp_cache[i]);
?? Or maybe I misread the code?
Takashi
On Mon, Jan 25, 2010 at 04:36:02PM +0100, Takashi Iwai wrote:
So, basically you restore the value changed between the beginning of the resume and after the call of wm8903_set_bias_levels(). What if the value was changed before the resume call? Shouldn't it be like
if (tmp_cache[i] != reg_cache[i] || tmp_cache[i] != wm8903_reg_defaults[i]) snd_soc_write(codec, i, tmp_cache[i]);
?? Or maybe I misread the code?
It's fine as-is - the resume will reset the register cache to the current state of the chip as part of the bringup so the effect of the existing code will be to write back anything in tmp_cache which is not present on the chip at the moment that the loop runs. This will include the effect of any changes prior to resume which weren't overwritten by resume.
At Mon, 25 Jan 2010 15:41:00 +0000, Mark Brown wrote:
On Mon, Jan 25, 2010 at 04:36:02PM +0100, Takashi Iwai wrote:
So, basically you restore the value changed between the beginning of the resume and after the call of wm8903_set_bias_levels(). What if the value was changed before the resume call? Shouldn't it be like
if (tmp_cache[i] != reg_cache[i] || tmp_cache[i] != wm8903_reg_defaults[i]) snd_soc_write(codec, i, tmp_cache[i]);?? Or maybe I misread the code?
It's fine as-is - the resume will reset the register cache to the current state of the chip as part of the bringup
Ah, OK, so that part is in the automation. I missed that.
so the effect of the existing code will be to write back anything in tmp_cache which is not present on the chip at the moment that the loop runs. This will include the effect of any changes prior to resume which weren't overwritten by resume.
thanks,
Takashi
participants (2)
-
Mark Brown -
Takashi Iwai