On 1/31/2019 5:40 PM, Rafael J. Wysocki wrote:
On Thu, Jan 31, 2019 at 12:59 PM Takashi Iwai <tiwai@xxxxxxx> wrote:
On Thu, 31 Jan 2019 12:46:54 +0100,
Rafael J. Wysocki wrote:
On Thu, Jan 31, 2019 at 12:21 PM Takashi Iwai <tiwai@xxxxxxx> wrote:
On Thu, 31 Jan 2019 12:05:30 +0100,
Thierry Reding wrote:
On Wed, Jan 30, 2019 at 05:40:42PM +0100, Takashi Iwai wrote:
[cut]
If I understand correctly the code, the pm domain is already activated
at calling driver's probe callback.
As far as I can tell, the domain will also be powered off again after
probe finished, unless the device grabs a runtime PM reference. This is
what happens via the dev->pm_domain->sync() call after successful probe
of a driver.
Ah, a good point. This can be a problem with a probe work like this
case.
Are you suggesting, whether runtime PM is enabled/disabled, after
successful probe the
domain would be powered off?
For CONFIG_PM enabled case, probe() can call get_sync() and put_sync()
can be in probe_work.
How this needs to be handled for CONFG_PM disabled case? (just calling
clock_enable() may
not be sufficient as per previous comments)
It seems to me like it's not a very well defined case what to do when a
device needs to be powered up but runtime PM is not enabled.
Adding Rafael and linux-pm, maybe they can provide some guidance on what
to do in these situations.
To summarize, what we're debating here is how to handle powering up a
device if the pm_runtime infrastructure doesn't take care of it. Jon's
proposal here was, and we use this elsewhere, to do something like this:
pm_runtime_enable(dev);
if (!pm_runtime_enabled(dev)) {
err = foo_runtime_resume(dev);
if (err < 0)
goto fail;
}
So basically when runtime PM is not available, we explicitly "resume"
the device to power it up.
It seems to me like that's a fairly common problem, so I'm wondering if
there's something that the runtime PM core could do to help with this.
Or perhaps there's already a way to achieve this that we're all
overlooking?
Rafael, any suggestions?
If any, a common helper would be appreciated, indeed.
I'm not sure that I understand the problem correctly, so let me
restate it the way I understand it.
What we're talking about is a driver ->probe() callback. Runtime PM
is disabled initially and the device is off. It needs to be powered
up, but the way to do that depends on some configuration of the board
etc., so ideally
pm_runtime_enable(dev);
ret = pm_runtime_resume(dev);
should just work, but the question is what to do if runtime PM doesn't
work as expected. That is, CONFIG_PM_RUNTIME is unset? Or something
else?
Yes, the question is how to write the code for both with and without
CONFIG_PM (or CONFIG_PM_RUNTIME).
This basically is about setup, because after that point all should
just work in both cases.
Personally, I would do
if (IS_ENABLED(CONFIG_PM)) {
do setup based on pm-runtime
} else {
do manual setup
}
do we really need config check here?
The debate was, whether to call hda_tegra_runtime_resume() or
hda_tegra_enable_clocks() unconditionally here.
It would take care of both CONFIG_PM enabled/disabled cases. Then enable
runtime PM.
Right now, we have a code like below, pushing the initialization in an
async work and let the probe returning quickly.
hda_tegra_probe() {
....
So why don't you do
if (!IS_ENABLED(CONFIG_PM)) {
do manual clock setup
}
here?
pm_runtime_enable();
schedule_work();
return;
}
hda_tegra_probe_work() {
pm_runtime_get_sync();
....
pm_runtime_put_sync();
}
Then it truned outhis code lacks of the clock initialization when
runtime PM isn't enabled. Normally it's done via runtime resume
hda_tegra_runtime_resume() {
hda_tegra_enable_clocks();
....
}
And now the question is what is the standard idiom in such a case.
IMO, calling pm_runtime_resume() inside the probe function looks
weird, and my preference was to initialize the clocks explicitly, then
enable runtime PM. But if using pm_runtime_resume() in the proc
should be seen as a standard procedure, I'm fine with that.
I think reference here is, whether calling hda_tegra_runtime_resume() in
probe() is
a standard procedure or not.
Well, people do pm_runtime_resume() in ->probe() too, but
pm_runtime_resume() returns 1 for CONFIG_PM unset, so that won't give
you what you want anyway. :-)
Cheers,
Rafael
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
