At Fri, 31 Aug 2012 01:57:09 +0000, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Thursday, August 30, 2012 10:45 PM
Yeah, the check seems slipped from my previous post by some reason. Maybe I cherry-picked a wrong branch. It should be if (codec->power_on) hda_call_pm_notify(codec->bus, false); and it should be called before module_put().
It's not needed. In the case of snd_hda_codec_free(), the only interest is whether pm_notify down was already called for this this particular codec. And this can be checked by codec->power_on flag.
Yes, an extra power flag is not needed.
I think 'codec->d3_stop_clk_ok' flag is more suitable than 'codec->power_on' for this check: if (!codec->d3_stop_clk_ok) ... either not suspended or d3_stop_clk_ok is not set for last suspending hda_call_pm_notify(codec->bus, false);
Right, this should be something like that.
It's overall confusing to check codec->d3_stop_clk_ok at that point, though. And the confusion comes from the fact that we are using this flag for multiple purposes -- the flag indicating the hardware status and the flag indicating whether the pm-notification has been done.
So I did a few clean ups in my tree now. Essentially I replaced codec->d3_stop_clk_ok with codec->pm_down_notified to make the meaning clearer. Also the check of D3_STOP_CLK_OK bit was moved to the hda_power_down_work() so that it's performed only in the power save.
thanks,
Takashi