[PATCH] ASoC: DaVinci: Update suspend/resume support for McASP driver

[PATCH] ASoC: DaVinci: Update suspend/resume support for McASP driver

[Chaithrika U S]

Add clock enable and disable calls to resume and suspend respectively.
Also add a member to the audio device data structure which tracks the clock
status.

Tested on DA850/OMAP-L138 EVM. For the purpose of testing, the patches[1] which 
add suspend-to-RAM support to DA850/OMAP-L138 SoC were applied.

[1] http://linux.davincidsp.com/pipermail/davinci-linux-open-source/
2009-November/016958.html

Signed-off-by: Chaithrika U S <chaithrika@ti.com>
---
Applies to ALSA GIT tree on branch topic/asoc at
http://git.kernel.org/?p=linux/kernel/git/tiwai/sound-2.6.git;a=shortlog;
h=topic/asoc

 sound/soc/davinci/davinci-mcasp.c |   18 ++++++++++++++++--
 sound/soc/davinci/davinci-mcasp.h |    1 +
 sound/soc/davinci/davinci-pcm.c   |    2 +-
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index 0a302e1..0d263f1 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -767,14 +767,27 @@ static int davinci_mcasp_trigger(struct snd_pcm_substream *substream,
 	int ret = 0;
 
 	switch (cmd) {
-	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
+		if (!dev->clk_active) {
+			clk_enable(dev->clk);
+			dev->clk_active = 1;
+		}
+
+	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 		davinci_mcasp_start(dev, substream->stream);
 		break;
 
-	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
+		davinci_mcasp_stop(dev, substream->stream);
+		if (dev->clk_active) {
+			clk_disable(dev->clk);
+			dev->clk_active = 0;
+		}
+
+		break;
+
+	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 		davinci_mcasp_stop(dev, substream->stream);
 		break;
@@ -866,6 +879,7 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
 	}
 
 	clk_enable(dev->clk);
+	dev->clk_active = 1;
 
 	dev->base = (void __iomem *)IO_ADDRESS(mem->start);
 	dev->op_mode = pdata->op_mode;
diff --git a/sound/soc/davinci/davinci-mcasp.h b/sound/soc/davinci/davinci-mcasp.h
index 582c924..e755b51 100644
--- a/sound/soc/davinci/davinci-mcasp.h
+++ b/sound/soc/davinci/davinci-mcasp.h
@@ -44,6 +44,7 @@ struct davinci_audio_dev {
 	int sample_rate;
 	struct clk *clk;
 	unsigned int codec_fmt;
+	u8 clk_active;
 
 	/* McASP specific data */
 	int	tdm_slots;
diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c
index ad4d7f4..80c7fdf 100644
--- a/sound/soc/davinci/davinci-pcm.c
+++ b/sound/soc/davinci/davinci-pcm.c
@@ -49,7 +49,7 @@ static void print_buf_info(int slot, char *name)
 static struct snd_pcm_hardware pcm_hardware_playback = {
 	.info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER |
 		 SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
-		 SNDRV_PCM_INFO_PAUSE),
+		 SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME),
 	.formats = (SNDRV_PCM_FMTBIT_S16_LE),
 	.rates = (SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_16000 |
 		  SNDRV_PCM_RATE_22050 | SNDRV_PCM_RATE_32000 |
-- 
1.5.6

Re: [PATCH] ASoC: DaVinci: Update suspend/resume support for McASP driver

[Takashi Iwai]

At Wed,  2 Dec 2009 12:39:00 +0530,
Chaithrika U S wrote:
> 
> diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c
> index ad4d7f4..80c7fdf 100644
> --- a/sound/soc/davinci/davinci-pcm.c
> +++ b/sound/soc/davinci/davinci-pcm.c
> @@ -49,7 +49,7 @@ static void print_buf_info(int slot, char *name)
>  static struct snd_pcm_hardware pcm_hardware_playback = {
>  	.info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER |
>  		 SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
> -		 SNDRV_PCM_INFO_PAUSE),
> +		 SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME),

Note that unless your driver supports the "full" resume,
SNDRV_PCM_INFO_RESUME shouldn't be set.  Here, the "full" resume means
that the hardware gets back to a completely sane state and the PCM
streams are resumed without extra PCM prepare call at PM resume.
If the PCM (or whatever) needs another re-initialization (like in many
cases), don't set this flag.

Just to be sure...


Takashi

Re: [PATCH] ASoC: DaVinci: Update suspend/resume support for McASP driver

[Chaithrika U S]

On Wed, Dec 02, 2009 at 14:56:31, Takashi Iwai wrote:
> At Wed,  2 Dec 2009 12:39:00 +0530,
> Chaithrika U S wrote:
> > 
> > diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c
> > index ad4d7f4..80c7fdf 100644
> > --- a/sound/soc/davinci/davinci-pcm.c
> > +++ b/sound/soc/davinci/davinci-pcm.c
> > @@ -49,7 +49,7 @@ static void print_buf_info(int slot, char *name)
> >  static struct snd_pcm_hardware pcm_hardware_playback = {
> >  	.info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER |
> >  		 SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
> > -		 SNDRV_PCM_INFO_PAUSE),
> > +		 SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME),
> 
> Note that unless your driver supports the "full" resume,
> SNDRV_PCM_INFO_RESUME shouldn't be set.  Here, the "full" resume means
> that the hardware gets back to a completely sane state and the PCM
> streams are resumed without extra PCM prepare call at PM resume.
> If the PCM (or whatever) needs another re-initialization (like in many
> cases), don't set this flag.
> 
> Just to be sure...
> 
> 
> Takashi
> 

PCM prepare call is not needed in this case. Testing was done with audio
loopback and after a suspend/resume cycle the audio output was proper. Hope
my understanding is correct. Is this test sufficient to prove that driver
supports full resume?

Regards, 
Chaithrika

Re: [PATCH] ASoC: DaVinci: Update suspend/resume support for McASP driver

[Takashi Iwai]

At Thu, 3 Dec 2009 17:58:08 +0530,
Chaithrika U S wrote:
> 
> On Wed, Dec 02, 2009 at 14:56:31, Takashi Iwai wrote:
> > At Wed,  2 Dec 2009 12:39:00 +0530,
> > Chaithrika U S wrote:
> > > 
> > > diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c
> > > index ad4d7f4..80c7fdf 100644
> > > --- a/sound/soc/davinci/davinci-pcm.c
> > > +++ b/sound/soc/davinci/davinci-pcm.c
> > > @@ -49,7 +49,7 @@ static void print_buf_info(int slot, char *name)
> > >  static struct snd_pcm_hardware pcm_hardware_playback = {
> > >  	.info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER |
> > >  		 SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
> > > -		 SNDRV_PCM_INFO_PAUSE),
> > > +		 SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME),
> > 
> > Note that unless your driver supports the "full" resume,
> > SNDRV_PCM_INFO_RESUME shouldn't be set.  Here, the "full" resume means
> > that the hardware gets back to a completely sane state and the PCM
> > streams are resumed without extra PCM prepare call at PM resume.
> > If the PCM (or whatever) needs another re-initialization (like in many
> > cases), don't set this flag.
> > 
> > Just to be sure...
> > 
> > 
> > Takashi
> > 
> 
> PCM prepare call is not needed in this case. Testing was done with audio
> loopback and after a suspend/resume cycle the audio output was proper. Hope
> my understanding is correct. Is this test sufficient to prove that driver
> supports full resume?

Yeah, if it worked with an actual app, it's fine :)
That was just a slight concern.


thanks,

Takashi

Re: [PATCH] ASoC: DaVinci: Update suspend/resume support for McASP driver

[Mark Brown]

On Thu, Dec 03, 2009 at 02:13:19PM +0100, Takashi Iwai wrote:

> Yeah, if it worked with an actual app, it's fine :)
> That was just a slight concern.

Chaithrika, could you please fix the issue with the comment for
fallthrough and resubmit?

RE: [alsa-devel] [PATCH] ASoC: DaVinci: Update suspend/resume support for McASP driver

[Chaithrika U S]

On Thu, Dec 03, 2009 at 18:57:17, Mark Brown wrote:
> On Thu, Dec 03, 2009 at 02:13:19PM +0100, Takashi Iwai wrote:
> 
> > Yeah, if it worked with an actual app, it's fine :)
> > That was just a slight concern.
> 
> Chaithrika, could you please fix the issue with the comment for
> fallthrough and resubmit?
> 

Mark,

I will resubmit this patch soon.

Regards, 
Chaithrika

Re: [PATCH] ASoC: DaVinci: Update suspend/resume support for McASP driver

[Sergei Shtylyov]

Hello.

Chaithrika U S wrote:

> Add clock enable and disable calls to resume and suspend respectively.
> Also add a member to the audio device data structure which tracks the clock
> status.

> Tested on DA850/OMAP-L138 EVM. For the purpose of testing, the patches[1] which 
> add suspend-to-RAM support to DA850/OMAP-L138 SoC were applied.

> [1] http://linux.davincidsp.com/pipermail/davinci-linux-open-source/
> 2009-November/016958.html

> Signed-off-by: Chaithrika U S <chaithrika@ti.com>
> ---

[...]

> diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
> index 0a302e1..0d263f1 100644
> --- a/sound/soc/davinci/davinci-mcasp.c
> +++ b/sound/soc/davinci/davinci-mcasp.c
> @@ -767,14 +767,27 @@ static int davinci_mcasp_trigger(struct snd_pcm_substream *substream,
>  	int ret = 0;
>  
>  	switch (cmd) {
> -	case SNDRV_PCM_TRIGGER_START:
>  	case SNDRV_PCM_TRIGGER_RESUME:
> +		if (!dev->clk_active) {
> +			clk_enable(dev->clk);
> +			dev->clk_active = 1;
> +		}

   You should add a comment in the cases where *break* is ommitted 
deliberately (if it indeed is), like:

		/* FALL THRU */

> +
> +	case SNDRV_PCM_TRIGGER_START:
>  	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
>  		davinci_mcasp_start(dev, substream->stream);
>  		break;
>  

WBR, Sergei