[alsa-devel] [PATCH] ASoC: davinci-mcasp: Allow complete shutdown of McASP when not in use

[Mark Brown]

On Tue, Mar 03, 2015 at 04:26:32PM +0200, Peter Ujfalusi wrote:

> @@ -553,6 +554,7 @@ static int __davinci_mcasp_set_clkdiv(struct snd_soc_dai *dai, int div_id,
>  		return -EINVAL;
>  	}
>  
> +	pm_runtime_put_sync(mcasp->dev);

Why the _put_sync()s?  I know it's more symmetrical but I don't see a
need to wait and it means we may be able to avoid some unneeded power
cycles.

> @@ -1078,6 +1082,9 @@ static int davinci_mcasp_suspend(struct snd_soc_dai *dai)
>  	u32 reg;
>  	int i;
>  
> +	if (!dai->active)
> +		pm_runtime_get_sync(mcasp->dev);
> +
>  	for (i = 0; i < ARRAY_SIZE(context_regs); i++)
>  		context->config_regs[i] = mcasp_get_reg(mcasp, context_regs[i]);
>  
> @@ -1094,6 +1101,8 @@ static int davinci_mcasp_suspend(struct snd_soc_dai *dai)
>  		context->xrsr_regs[i] = mcasp_get_reg(mcasp,
>  						DAVINCI_MCASP_XRSRCTL_REG(i));
>  
> +	pm_runtime_put_sync(mcasp->dev);
> +

On first glance this looks unbalanced without the active check in the
put case.  I'd also need to check if runtime resume works OK in suspend
context, I guess it must but there were issues in the past (the more
common idiom is to not suspend if we're already runtime powered down but
obviously here we're actually doing extra work).

I see there's actually a balanced put in the resume path...  I see
what's going on here but I'm thinking that perhaps something more
explicit that calls the ops directly and checks pm_runtime_is_enabled()
might be clearer?  Unconditionally do the power down/up at the end with
a direct call outside of runtime PM and then make the checks at the
top/bottom be "if it's runtime suspended then...".  That's also more
direct about what's actually being checked and hence less risk that some
future changes would go wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20150303/ee2aa439/attachment.sig>