linux-kernel - Re: [PATCH v2] ALSA: hda/tegra: enable clock during probe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <80a45aa3-8a22-959c-0c73-03ad2d6b1f8c@nvidia.com>
Date:   Mon, 4 Feb 2019 10:04:50 +0000
From:   Jon Hunter <jonathanh@...dia.com>
To:     Sameer Pujar <spujar@...dia.com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Thierry Reding <thierry.reding@...il.com>
CC:     "Rafael J. Wysocki" <rafael@...nel.org>,
        Takashi Iwai <tiwai@...e.de>,
        Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        "moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..." 
        <alsa-devel@...a-project.org>, <mkumard@...dia.com>,
        <rlokhande@...dia.com>, <sharadg@...dia.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        <linux-tegra@...r.kernel.org>, Linux PM <linux-pm@...r.kernel.org>
Subject: Re: [PATCH v2] ALSA: hda/tegra: enable clock during probe


On 04/02/2019 08:16, Sameer Pujar wrote:

...

> Objective is to have things working with or without CONFIG_PM enabled.
> From previous comments and discussions it appears that there is mixed
> response
> for calling hda_tegra_runtime_resume() or runtime PM APIs in probe()
> call. Need
> to have consensus regarding the best practice to be followed, which
> would eventually
> can be used in other drivers too.
> 
> Rafael is suggesting to use CONFIG_PM check to do manual setup or
> runtime PM setup in probe,
> which would bring back the earlier above mentioned concern.
> 
> if (IS_ENABLED(CONFIG_PM)) {
> do setup based on pm-runtime
> } else {
>     do manual setup
> }
> Both if/else might end up doing the same here.
> Do we really need CONFIG_PM check here?
> 
> Instead does below proposal appear fine?
> 
> probe() {
>     hda_tegra_enable_clock();
> }
> 
> probe_work() {
>     /* hda setup */
>     . . .
>     pm_runtime_set_active(); /* initial state as active */
>     pm_runtime_enable();
>     return;
> }

I believe that this still does not work, because if there is a
power-domain that needs to be turned on, this does not guarantee this.
So I think that you need to call pm_runtime_get ...

 probe() {
 	if (!IS_ENABLED(CONFIG_PM))
 		hda_tegra_enable_clock();
 }


 probe_work() {
    	/* hda setup */
    	. . .
	pm_runtime_enable();
	pm_runtime_get_sync();
	return;
 }

The alternative here could just be ...

 probe() {
	pm_runtime_enable();
 	if (!pm_runtime_enabled())
 		ret = hda_tegra_enable_clock();
	else
		ret = pm_runtime_get_sync();

	if (ret < 0) {
		...
	}
 }

Very similar to what I was saying to begin with but not call the
pm_runtime_resume handler directly. Which I believe was Iwai-san's dislike.

Cheers
Jon

-- 
nvpublic