asoc-v2 ac97-wm9713 suspend/resume

asoc-v2 ac97-wm9713 suspend/resume

[Oliver Ford]

I have been porting linux to the iPAQ 214 which uses a wm9713 codec on 
the ac97 of a pxa310. It uses it for audio and touchscreen.
I'm working from the git tree of arm.linux.org.uk pxa branch and have 
merged in the asoc-v2-dev of wolfson-asoc (all last thursday).

I have the audio and touchscreen working via a platform driver I've put 
together (sound/soc/pxa/hpipaq214-audio.c) from copying 
sound/soc/pxa/zylonite.c etc a few months ago.

I'm now trying to get the audio working through suspend/resume.
To save as much power as possible in D3 suspend the wm9713 power is 
completely turned off in hpipaq214-audio.c via a gpio. This is in 
suspend_late() and resume_early() to make sure the power is back before 
the AC97/WM9713 drivers try to wake stuff up.

To make the touchscreen work after suspend I had to add a cold reset to 
sound/soc/wm9713.c : wm9713_resume().

Also I have to remove the calls to snd_soc_card_suspend_pcms() and 
snd_soc_card_resume_pcms() from my hpipaq214-audio.c's normal 
suspend/resume functions. These calls were originally in 
sound/soc/pxa/zylonite.c but I notice there is now no suspend/resume 
functions in there. Some of the other platform audio drivers call 
snd_soc_suspend_pcms() some call snd_soc_suspend() and some 
snd_soc_card_suspend_pcms().

Only snd_soc_suspend_pcms() seems to actually exists. If I do actually 
call snd_soc_card_suspend/resume_pcms() then any use of the audio or 
touchscreen later on freezes the process doing it.

Which am I actually supposed to do, and is there a more 'proper' way way 
of bringing the wm9713 back up from complete power down on resume?

Oliver Ford

Re: asoc-v2 ac97-wm9713 suspend/resume

[Robert Jarzmik]

Oliver Ford <ipaqlinux@oliford.co.uk> writes:
> ...

Same questions for me, with a different setup : pxa272 + wm9713.
I have the same issues on the mioa701 board, with same symptoms :
 - snd_soc_suspend_* do not work ATM
 - I had to add a double reset in wm9713 resume() function as you did

> Which am I actually supposed to do, and is there a more 'proper' way way 
> of bringing the wm9713 back up from complete power down on resume?
Even if asoc-v2 is still in development, I have the same concerns. Marc, Liam, any idea ?

--
Robert

Re: asoc-v2 ac97-wm9713 suspend/resume

[Oliver Ford]

Robert Jarzmik wrote:
>  - I had to add a double reset in wm9713 resume() function as you did
>
>   
Does this actually work for you?

I forgot to say, for me it allows the touchscreen to work, and 'amixer 
cset xxx xxx' commands seem to make things click, but I get no sound 
from aplay where I do before the suspend.

Oliver

Re: asoc-v2 ac97-wm9713 suspend/resume

[Robert Jarzmik]

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

Oliver Ford <ipaqlinux@oliford.co.uk> writes:

> Robert Jarzmik wrote:
>>  - I had to add a double reset in wm9713 resume() function as you did
>>
>>   
> Does this actually work for you?
Yes, indeed.
>
> I forgot to say, for me it allows the touchscreen to work, and 'amixer cset xxx
> xxx' commands seem to make things click, but I get no sound from aplay where I
> do before the suspend.
I joined here all my modifications to asoc-v2 and my board code. Hope this will
help you a bit.

For the other listening here, this is _not_ a submission, I'm still waiting for
the official go of asoc-v2 to clean and submit my patches.

--
Robert


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Dirty-hack-to-use-GPIO95-in-AC97-reset.patch --]
[-- Type: text/x-diff, Size: 2002 bytes --]

>From 78a4b1aafe8c0b36f9504d70cc3914265eba1d36 Mon Sep 17 00:00:00 2001
From: Robert Jarzmik <rjarzmik@free.fr>
Date: Sun, 16 Mar 2008 22:44:17 +0100
Subject: [PATCH] Dirty hack to use GPIO95 in AC97 reset.

Mio Mitac A701 uses GPIO95 instead of GPIO113 as the reset
line connected to the WM9713 chip. This is dirty, and should
be replaced by an elegant solution where cold and warm reset
are implemented in machine driver mioa701_wm9713.c

Signed-off-by: Robert Jarzmik <rjarzmik@free.fr>
---
 sound/soc/pxa/pxa2xx-ac97.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/sound/soc/pxa/pxa2xx-ac97.c b/sound/soc/pxa/pxa2xx-ac97.c
index cb12544..86fb9c2 100644
--- a/sound/soc/pxa/pxa2xx-ac97.c
+++ b/sound/soc/pxa/pxa2xx-ac97.c
@@ -145,10 +145,10 @@ static void pxa2xx_ac97_warm_reset(struct snd_ac97 *ac97)
 #ifdef CONFIG_PXA27x
 	/* warm reset broken on Bulverde,
 	   so manually keep AC97 reset high */
-	pxa_gpio_mode(113 | GPIO_OUT | GPIO_DFLT_HIGH);
+	pxa_gpio_mode(95 | GPIO_OUT | GPIO_DFLT_HIGH);
 	udelay(10);
 	GCR |= GCR_WARM_RST;
-	pxa_gpio_mode(113 | GPIO_ALT_FN_2_OUT);
+	pxa_gpio_mode(95 | GPIO_ALT_FN_1_OUT);
 	udelay(500);
 #elif defined(CONFIG_PXA3xx)
 	/* Can't use interrupts */
@@ -303,8 +303,8 @@ static int pxa2xx_ac97_resume(struct platform_device *dev)
 	pxa_gpio_mode(GPIO29_SDATA_IN_AC97_MD);
 #endif
 #ifdef CONFIG_PXA27x
-	/* Use GPIO 113 as AC97 Reset on Bulverde */
-	pxa_gpio_mode(113 | GPIO_ALT_FN_2_OUT);
+	/* Use GPIO 95 as AC97 Reset on Bulverde */
+	pxa_gpio_mode(95 | GPIO_ALT_FN_1_OUT);
 #endif
 	clk_enable(ac97_clk);
 	return 0;
@@ -457,8 +457,8 @@ static int pxa2xx_ac97_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 #ifdef CONFIG_PXA27x
-	/* Use GPIO 113 as AC97 Reset on Bulverde */
-	pxa_gpio_mode(113 | GPIO_ALT_FN_2_OUT);
+	/* Use GPIO 95 as AC97 Reset on Bulverde */
+	pxa_gpio_mode(95 | GPIO_ALT_FN_1_OUT);
 
 	ac97conf_clk = clk_get(&pdev->dev, "AC97CONFCLK");
 	if (IS_ERR(ac97conf_clk)) {
-- 
1.5.5.3


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Dirty-hack-to-workaround-pxa27x-buggy-hardreset.patch --]
[-- Type: text/x-diff, Size: 855 bytes --]

>From 86e28a3258b8715c23c1e7eefd259fe0238d099e Mon Sep 17 00:00:00 2001
From: Robert Jarzmik <rjarzmik@free.fr>
Date: Fri, 25 Apr 2008 22:48:39 +0200
Subject: [PATCH] Dirty hack to workaround pxa27x buggy hardreset.

Signed-off-by: Robert Jarzmik <rjarzmik@free.fr>
---
 sound/soc/codecs/wm9713.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/sound/soc/codecs/wm9713.c b/sound/soc/codecs/wm9713.c
index 2cb4fb8..8ec7ce9 100644
--- a/sound/soc/codecs/wm9713.c
+++ b/sound/soc/codecs/wm9713.c
@@ -975,6 +975,7 @@ static int wm9713_resume(struct platform_device *pdev)
 	u16 id;
 
 	/* give the codec an AC97 warm reset to start the link */
+	codec->ac97->bus->ops->reset(codec->ac97);
 	codec->ac97->bus->ops->warm_reset(codec->ac97);
 	id = codec->soc_phys_read(codec->ac97, AC97_VENDOR_ID2); 
 	if (id != 0x4c13) {
-- 
1.5.5.3


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-Added-double-ac97-reset-in-pxa27x-resume-path.patch --]
[-- Type: text/x-diff, Size: 984 bytes --]

>From 2d8aa2d11131c7f68ffb3f031964bb98866ca511 Mon Sep 17 00:00:00 2001
From: Robert Jarzmik <rjarzmik@free.fr>
Date: Mon, 28 Apr 2008 19:46:57 +0200
Subject: [PATCH] Added double ac97 reset in pxa27x resume path.

Signed-off-by: Robert Jarzmik <rjarzmik@free.fr>
---
 sound/soc/codecs/wm9713.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/sound/soc/codecs/wm9713.c b/sound/soc/codecs/wm9713.c
index 8ec7ce9..5defac8 100644
--- a/sound/soc/codecs/wm9713.c
+++ b/sound/soc/codecs/wm9713.c
@@ -977,6 +977,8 @@ static int wm9713_resume(struct platform_device *pdev)
 	/* give the codec an AC97 warm reset to start the link */
 	codec->ac97->bus->ops->reset(codec->ac97);
 	codec->ac97->bus->ops->warm_reset(codec->ac97);
+	codec->ac97->bus->ops->reset(codec->ac97);
+	codec->ac97->bus->ops->warm_reset(codec->ac97);
 	id = codec->soc_phys_read(codec->ac97, AC97_VENDOR_ID2); 
 	if (id != 0x4c13) {
 		printk(KERN_ERR "wm9713 failed to resume");
-- 
1.5.5.3


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0004-Include-shuffle-in-2.6.25-kernels.patch --]
[-- Type: text/x-diff, Size: 721 bytes --]

>From 81b4171dd0ba62391898240424102abdb9db67a8 Mon Sep 17 00:00:00 2001
From: Robert Jarzmik <rjarzmik@free.fr>
Date: Wed, 30 Apr 2008 23:54:11 +0200
Subject: [PATCH] Include shuffle in 2.6.25 kernels.

Signed-off-by: Robert Jarzmik <rjarzmik@free.fr>
---
 sound/soc/pxa/pxa2xx-ac97.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/sound/soc/pxa/pxa2xx-ac97.c b/sound/soc/pxa/pxa2xx-ac97.c
index 86fb9c2..04156d6 100644
--- a/sound/soc/pxa/pxa2xx-ac97.c
+++ b/sound/soc/pxa/pxa2xx-ac97.c
@@ -28,6 +28,7 @@
 #include <linux/mutex.h>
 #include <asm/hardware.h>
 #include <asm/arch/pxa-regs.h>
+#include <asm/arch/pxa2xx-gpio.h>
 #include <asm/arch/audio.h>
 
 #include "pxa2xx-pcm.h"
-- 
1.5.5.3


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #6: 0005-Revert-wm9713-Expose-the-PLL-configuration.patch --]
[-- Type: text/x-diff, Size: 1870 bytes --]

>From e07ea80f6d8bbac6ab20b525a0c37269eb6bb741 Mon Sep 17 00:00:00 2001
From: Robert Jarzmik <rjarzmik@free.fr>
Date: Sun, 1 Jun 2008 14:49:31 +0200
Subject: [PATCH] Revert "wm9713: Expose the PLL configuration"

This reverts commit 0d03f161680832984dd03f6bfc9a3423fc67501d.
There's a bug in set_pll call from resume function.
---
 sound/soc/codecs/wm9713.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/sound/soc/codecs/wm9713.c b/sound/soc/codecs/wm9713.c
index 5defac8..eae042c 100644
--- a/sound/soc/codecs/wm9713.c
+++ b/sound/soc/codecs/wm9713.c
@@ -659,7 +659,7 @@ struct pll_ {
 	unsigned int k;
 };
 
-static struct pll_ pll[] = {
+struct pll_ pll[] = {
 	{13000000, 0, 1, 0, 0, 7, 0x23f488},
 	{2048000,  1, 0, 0, 0, 12, 0x0},
 	{4096000,  1, 0, 0, 0, 6, 0x0},
@@ -667,10 +667,9 @@ static struct pll_ pll[] = {
 	/* liam - add more entries */
 };
 
-static int wm9713_set_pll(struct snd_soc_dai *codec_dai,
+static int wm9713_set_pll(struct snd_soc_codec *codec,
 	int pll_id, unsigned int freq_in, unsigned int freq_out)
 {
-	struct snd_soc_codec *codec = codec_dai->codec;
 	struct wm9713_data *wm9713 = codec->private_data;
 	int i;
 	u16 reg, reg2;
@@ -1056,13 +1055,11 @@ static struct snd_soc_dai_caps wm9713_voice_playback = {
 static struct snd_soc_dai_ops wm9713_hifi_dai_ops = {
 	/* alsa ops */
 	.prepare = ac97_hifi_prepare,
-	.set_pll = wm9713_set_pll,
 };
 
 static struct snd_soc_dai_ops wm9713_aux_dai_ops = {
 	/* alsa ops */
 	.prepare = ac97_aux_prepare,
-	.set_pll = wm9713_set_pll,
 };
 
 static struct snd_soc_dai_ops wm9713_voice_dai_ops = {
@@ -1074,7 +1071,6 @@ static struct snd_soc_dai_ops wm9713_voice_dai_ops = {
 	.set_clkdiv = wm9713_set_dai_clkdiv,
 	.set_fmt = wm9713_set_dai_fmt,
 	.set_tristate = wm9713_set_dai_tristate,
-	.set_pll = wm9713_set_pll,
 };
 
 /* for modprobe */
-- 
1.5.5.3


[-- Attachment #7: mioa701 board code --]
[-- Type: application/octet-stream, Size: 7441 bytes --]

[-- Attachment #8: 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: asoc-v2 ac97-wm9713 suspend/resume

[Mark Brown]

On Sun, Jun 22, 2008 at 06:17:01PM +0100, Oliver Ford wrote:

> To make the touchscreen work after suspend I had to add a cold reset to 
> sound/soc/wm9713.c : wm9713_resume().

You should do this in your machine driver - the decision about doing a
cold reset depends heavily on the system.  Some systems will require it
but others (especially those where the WM9713 is a wakeup source) would
be broken by extra cold resets.  This is as much about what the AC97
controller needs on a given system as anything else.

> Also I have to remove the calls to snd_soc_card_suspend_pcms() and
> snd_soc_card_resume_pcms() from my hpipaq214-audio.c's normal
> suspend/resume functions. These calls were originally in
> sound/soc/pxa/zylonite.c but I notice there is now no suspend/resume
> functions in there. Some of the other platform audio drivers call

There are (or were at the time I last worked on ASoC v2 Zylonite)
suspend and resume issues in the base Zylonite machine support so I
removed the suspend/resume code until it can be tested - IIRC the code
was never even compiled.

> Only snd_soc_suspend_pcms() seems to actually exists. If I do actually 

snd_soc_card_resume_pcms() appears to exist in the current branch?

> call snd_soc_card_suspend/resume_pcms() then any use of the audio or 
> touchscreen later on freezes the process doing it.

The audio needs to be the very last thing that is suspended since the
codec driver does the final power off of the AC97 bus - once that has
happened nothing will be able to access the codec until at least a warm
reset has been completed.

> Which am I actually supposed to do, and is there a more 'proper' way way 
> of bringing the wm9713 back up from complete power down on resume?

What you're doing seems reasonable.

The most complete worked example of an ASoC v2 driver is the
i.MX31ADS/WM8350 driver in sound/soc/imxa31/imx31ads-wm8350.c in the
imx31 branch of

	git://opensource.wolfsonmicro.com/linux-2.6-audioplus

(it's in a separate place because the base i.MX machine support
currently needs a Freescale vendor kernel).