[alsa-devel] [PATCH 3/3] ALSA: hda - delay resuming Haswell HDMI codec in system resume

[Lin, Mengdong]

Hi Takashi,

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai at suse.de]
> Sent: Monday, April 08, 2013 10:27 PM
> > +	wait_event_timeout(spec->resume_wq,
> > +			spec->ready_to_resume, msecs_to_jiffies(300));
> > +	if (!spec->ready_to_resume)
> > +		snd_printd(KERN_WARNING "HDMI: Haswell not ready to
> resume\n"); }
> 
> IMO, this should be shown no matter whether with or w/o debug option, i.e.
> use snd_printk().

Yes. Time out would be a serious error.

> > +static void intel_haswell_set_power_state(struct hda_codec *codec,
> hda_nid_t fg,
> > +				 unsigned int power_state)
> > +{
> > +	if (codec->resume_delayed && power_state ==  AC_PWRST_D0) {
> 
> A superfluous space after '=='
> 
> > +		intel_haswell_wait_ready_to_resume(codec);
> > +		codec->resume_delayed = 0;
> > +	}
> > +
> > +	snd_hda_codec_read(codec, fg, 0,
> > +			AC_VERB_SET_POWER_STATE,
> > +			power_state);
> > +
> > +	snd_hda_codec_set_power_to_all(codec, fg, power_state); }
> > +
> > +static inline void intel_haswell_allow_delay_resume(struct hda_codec
> > +*codec)
> 
> You don't have to put inline for such a case.
> Rely more on the compiler.  It shall do that anyway.
> 
> > +{
> > +	struct hdmi_spec *spec = codec->spec;
> > +
> > +	init_waitqueue_head(&spec->resume_wq);
> > +	codec->support_delay_resume = 1;
> > +
> > +	codec->patch_ops.set_power_state =
> > +				intel_haswell_set_power_state;
> 
> No need for a new line...
> 
> > +}
> > +#else
> > +define intel_haswell_allow_delay_resume NULL
> 
> This should be an empty function instead
> 
> #define intel_haswell_allow_delay_resume() do { } while (0)

I made an mistake here. Sorry! 

I've revised the 1st patch comments and 3rd patch as you suggested. Please have a review.

Many thanks
Mengdong