[PATCH] Change of cold reset and ac97 register cache

[PATCH] Change of cold reset and ac97 register cache

[Robert Jarzmik]

[-- Attachment #1: Type: text/plain, Size: 497 bytes --]

This patch synchronizes wm9713 AC97 register cache and hardware registers 
upon reset.

The actual implementation of the wm9713 codec provides a cache of AC97
registers. After cold reset, the cache and hardware are not in sync. When
using wm9713 logic, oring a GPIO triggers many other GPIOS.

The solution of this patch reads from hardware to cache upon cold reset.
Another one would be to write the cache to hardware upon init. If decided so,
I would gladly change my patch.

--
Robert Jarzmik


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Cold reset of wm9713 --]
[-- Type: text/x-diff, Size: 888 bytes --]

diff --git a/sound/soc/codecs/wm9713.c b/sound/soc/codecs/wm9713.c
index 6e3c6af..62c6c02 100644
--- a/sound/soc/codecs/wm9713.c
+++ b/sound/soc/codecs/wm9713.c
@@ -1014,6 +1014,9 @@ EXPORT_SYMBOL_GPL(wm9713_dai);
 
 int wm9713_reset(struct snd_soc_codec *codec, int try_warm)
 {
+	int i;
+	u16 *cache = codec->reg_cache;
+
 	if (try_warm && soc_ac97_ops.warm_reset) {
 		soc_ac97_ops.warm_reset(codec->ac97);
 		if (!(ac97_read(codec, 0) & 0x8000))
@@ -1023,6 +1026,14 @@ int wm9713_reset(struct snd_soc_codec *codec, int try_warm)
 	soc_ac97_ops.reset(codec->ac97);
 	if (ac97_read(codec, 0) & 0x8000)
 		return -EIO;
+
+	for (i = 2; i < ARRAY_SIZE(wm9713_reg) << 1; i+=2) {
+		if (i == AC97_POWERDOWN || i == AC97_EXTENDED_MID ||
+		    i == AC97_EXTENDED_MSTATUS || i > 0x66)
+			continue;
+		cache[i>>1] = ac97_read(codec, i);
+	}
+	
 	return 0;
 }
 EXPORT_SYMBOL_GPL(wm9713_reset);

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] Change of cold reset and ac97 register cache

[Mark Brown]

On Wed, Jan 16, 2008 at 11:24:56PM +0100, Robert Jarzmik wrote:

> The actual implementation of the wm9713 codec provides a cache of AC97
> registers. After cold reset, the cache and hardware are not in sync. When
> using wm9713 logic, oring a GPIO triggers many other GPIOS.

> The solution of this patch reads from hardware to cache upon cold reset.
> Another one would be to write the cache to hardware upon init. If decided so,
> I would gladly change my patch.

Neither option is entirely safe - something more involved like the code
in wm9713_soc_resume() would be required to bring the chip back safely.
Writing the current cache values is unsafe due to ordering requirements
and in any case the information required to configure the PLL can't be
recovered from the cache alone.  Reading the values currently set in the
chip for the cache is safer will leave the chip without basic
initialisation and the driver out of sync with the state of the chip.

Could you outline why you need to reset the codec?  I'd like to
understand the use case here.

Re: [PATCH] Change of cold reset and ac97 register cache

[Robert Jarzmik]

Mark Brown <broonie@opensource.wolfsonmicro.com> writes:

> Just to clarify, it is the initial reset on chip probe that is causing
> problems for you rather than a call later on?  If the driver state and
> chip state are out of sync after that then that suggests that there is
> an error in the register defaults or some other similar issue.
It's the initial on chip probe in my case.

> > After that, I need to setup a specific GPIO to trigger loud speaker
> > amplificator. The trouble is that I do not want to trigger another GPIO which
> > powers another amplificator.
> > My code basically reads de GPIO register, put the right bit, and stores back.
> > But because of cache/chip difference, I trigger the other GPIO.
> 
> How exactly are you doing these accesses and which registers is it that
> trigger the problem - is it setting the GPIO state or configuring the
> GPIO pin modes?

I'm setting up a wm9713 GPIO (GPIO8) as input, and then set the pull-up
register to power my speaker amplification.

As we said, I'll implement a machine dependent solution, assuming there'll be in
the future a mecanism in ASoC v2 allowing hook inscription for the reset phase.
I'll synchronize the cache in this specific function, and will wait ASoC to
catch up or propose a patch.

--
Robert

PS: Sorry for the list, in my last reply I forgot alsa-devel.

Re: [PATCH] Change of cold reset and ac97 register cache

[Mark Brown]

On Fri, Jan 18, 2008 at 08:50:42PM +0100, Robert Jarzmik wrote:

> As we said, I'll implement a machine dependent solution, assuming there'll be in
> the future a mecanism in ASoC v2 allowing hook inscription for the reset phase.
> I'll synchronize the cache in this specific function, and will wait ASoC to
> catch up or propose a patch.

For the benefit of the archives: the root issue here is that AC97 reset
has problems working as documented with the SoC CPU being used here so
it is normally done using a GPIO line on the CPU.  Robert is working on
a consumer device for which he doesn't have hardware documentation so is
unable to generate a working reset to the chip.