Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751825AbbKJSa5 (ORCPT ); Tue, 10 Nov 2015 13:30:57 -0500 Received: from mail-pa0-f48.google.com ([209.85.220.48]:35084 "EHLO mail-pa0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751031AbbKJSaz (ORCPT ); Tue, 10 Nov 2015 13:30:55 -0500 Date: Tue, 10 Nov 2015 10:30:51 -0800 From: Eduardo Valentin To: Ulf Hansson Cc: Geert Uytterhoeven , Kuninori Morimoto , Zhang Rui , Linux-SH , Linux-Kernel , Linux PM list , Cao Minh Hiep , =?utf-8?B?RHVuZ++8muS6uuOCvQ==?= , Alan Stern , "Rafael J. Wysocki" Subject: Re: [PATCH 2/2] thermal: rcar_thermal: use pm_runtime_put_sync() Message-ID: <20151110183049.GB5240@localhost.localdomain> References: <87h9kulkfg.wl%kuninori.morimoto.gx@renesas.com> <87fv0elibh.wl%kuninori.morimoto.gx@renesas.com> <87d1vili9c.wl%kuninori.morimoto.gx@renesas.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2823 Lines: 70 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 wrote: > > Hi Ulf, > > > > On Tue, Nov 10, 2015 at 10:57 AM, Ulf Hansson wrote: > >> On 10 November 2015 at 09:18, Geert Uytterhoeven wrote: > >>> On Tue, Nov 10, 2015 at 3:12 AM, Kuninori Morimoto > >>> wrote: > >>>> From: Kuninori Morimoto > >>>> > >>>> 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 > >>>> --- > >>>> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/