On Mon, 04 Feb 2019 11:04:50 +0100, Jon Hunter wrote:
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.
Yes, exactly, what bothered me was really a nuance: calling hda_tegra_runtime_resume() there makes the code misleading (or confusing) as if the runtime PM were mandatory.
I hoped there could be some standard idiom for this expression, but apparently there isn't any, so far...
Obviously the easiest option is to enforce the dependency on CONFIG_PM. Would there be any platform that needs to run without PM, practically seen...?
But, now after lengthy discussions and the clarification of the current situation, I have no strong opinion on this any longer. So I leave the decision to you guys, and I'll apply it as-is.
thanks,
Takashi