[alsa-devel] [PATCH 2/2] ASoC: max98090: Enforce correct device sequencing when configuring a new

[Fang, Yang A]

> -----Original Message-----
> From: Mark Brown [mailto:broonie at kernel.org]
> Sent: Friday, August 07, 2015 3:43 AM
> To: Fang, Yang A
> Cc: lgirdwood at gmail.com; alsa-devel at alsa-project.org;
> dgreid at chromium.org; Nujella, Sathyanarayana;
> kevin.strasser at linux.intel.com; Sripathi, Srinivas; Iriawan, Denny; Jain,
> Praveen K; Anish.Kumar at maximintegrated.com; Eoff, Ullysses A
> Subject: Re: [PATCH 2/2] ASoC: max98090: Enforce correct device sequencing
> when configuring a new
> 
> On Thu, Aug 06, 2015 at 04:15:45PM -0700, yang.a.fang at intel.com wrote:
> 
> > +static int max98090_shdn_event(struct snd_soc_dapm_widget *w,
> > +				 struct snd_kcontrol *kcontrol, int event) {
> > +	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
> > +	struct max98090_priv *max98090 =
> snd_soc_codec_get_drvdata(codec);
> > +
> > +	if (event & SND_SOC_DAPM_POST_PMU)
> > +		max98090->shdn_pending = TRUE;
> 
> TRUE?  Please use normal C99 booleans like we normally do in the kernel.
I will fix this
> 
> > +static void max98090_seq_notifier(struct snd_soc_dapm_context *dapm,
> > +	enum snd_soc_dapm_type event, int subseq, bool power_up) {
> > +	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(dapm);
> > +	struct max98090_priv *max98090 =
> snd_soc_codec_get_drvdata(codec);
> > +
> > +	if (max98090->shdn_pending && power_up) {
> > +		snd_soc_update_bits(codec,
> M98090_REG_DEVICE_SHUTDOWN,
> > +				M98090_SHDNN_MASK, 0);
> > +		msleep(40);
> > +		snd_soc_update_bits(codec,
> M98090_REG_DEVICE_SHUTDOWN,
> > +				M98090_SHDNN_MASK,
> M98090_SHDNN_MASK);
> > +		max98090->shdn_pending = FALSE;
> > +	}
> > +}
> 
> Why did you change the API for this?  As far as I can tell power_up is
> redundant here, we already know we're powering things up because we got
> a _PMU event, the main effect here appears to be to just run this at the next
> sequence point after we've powered up a widget with a shutdown event.
> The general concept with the seq_notifier is that we record things we need
> to do in the per-widget events and then implement them in the seq_notifier
> and I'm not seeing anything else going on here.
Thanks Mark.   Right , _PMU already indicates it is power_up event.. I will drop
This API change.