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