Messages in this thread | | | Date | Tue, 10 Nov 2015 10:30:51 -0800 | From | Eduardo Valentin <> | Subject | Re: [PATCH 2/2] thermal: rcar_thermal: use pm_runtime_put_sync() |
| |
Hi,
On Tue, Nov 10, 2015 at 02:00:38PM +0100, Ulf Hansson wrote: > +Rafael, Alan > > On 10 November 2015 at 11:10, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Ulf, > > > > On Tue, Nov 10, 2015 at 10:57 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > >> On 10 November 2015 at 09:18, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > >>> On Tue, Nov 10, 2015 at 3:12 AM, Kuninori Morimoto > >>> <kuninori.morimoto.gx@renesas.com> wrote: > >>>> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > >>>> > >>>> It is using pm_runtime_get_sync() on probe(). Let's use > >>>> pm_runtime_put_sync() instead of pm_runtime_put(). Otherwise thermal > >>>> sensor doesn't work after unbind/re-bind > >>>> > >>>> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > >>>> --- > >>>> drivers/thermal/rcar_thermal.c | 2 +- > >>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>> > >>>> diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c > >>>> index 13d01ed..f7cf2d7 100644 > >>>> --- a/drivers/thermal/rcar_thermal.c > >>>> +++ b/drivers/thermal/rcar_thermal.c > >>>> @@ -373,7 +373,7 @@ static int rcar_thermal_remove(struct platform_device *pdev) > >>>> thermal_zone_device_unregister(priv->zone); > >>>> } > >>>> > >>>> - pm_runtime_put(dev); > >>>> + pm_runtime_put_sync(dev); > >>>> pm_runtime_disable(dev); > >> > >> For the reasons explained by Geert, this is to me also a "workaround". > >> > >> I would replace pm_runtime_put() and pm_runtime_disable() with a call > >> to pm_runtime_force_suspend(). > >> > >> In that way, you will make sure you device get runtime suspended > >> (clock domain will gate the clock). Additionally, the runtime PM > >> status will properly reflect the status of the device. > > > > That still sounds like a workaround to me, which we have to apply to all > > drivers relying on Runtime PM? > > Definitely not all drivers, but those that runs pm_runtime_get_sync() > during ->probe() and expects the ->runtime_resume() callback to always > be invoked because of that. I guess we need to check upon which > drivers that may suffer from this. > > I wouldn't be surprised if at least a subset of those cases we find, > are poorly designed from PM point of view and won't even probe > successfully unless CONFIG_PM is set. Whatever that means...
Yeah, if it is the case this is a bug in runtime pm core, I would prefer this to be properly fixed, and not only this driver benefits of it.
Rafael? Any thoughts?
BR,
Eduardo Valentin
| |