Hi Li,
On Thu, Feb 28, 2013 at 10:02 AM, Li Fei <[email protected]> wrote:
>
> Even in failed case of pm_runtime_get_sync, the usage_count
> is incremented. In order to keep the usage_count with correct
> value and runtime power management to behave correctly, call
> pm_runtime_put(_sync) in such case.
Is it better then to call pm_runtime_put_noidle instead? This way
we're sure to only take care of usage_count without ever calling any
underlying pm handler.
Thanks,
Ohad.
> In __hwspin_lock_request, module_put is also called before
> return in pm_runtime_get_sync failed case.
>
> Signed-off-by Liu Chuansheng <[email protected]>
> Signed-off-by: Li Fei <[email protected]>
> ---
> drivers/hwspinlock/hwspinlock_core.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/hwspinlock/hwspinlock_core.c b/drivers/hwspinlock/hwspinlock_core.c
> index db713c0..5a5076d 100644
> --- a/drivers/hwspinlock/hwspinlock_core.c
> +++ b/drivers/hwspinlock/hwspinlock_core.c
> @@ -416,6 +416,8 @@ static int __hwspin_lock_request(struct hwspinlock *hwlock)
> ret = pm_runtime_get_sync(dev);
> if (ret < 0) {
> dev_err(dev, "%s: can't power on device\n", __func__);
> + pm_runtime_put(dev);
> + module_put(dev->driver->owner);
> return ret;
> }
>
> --
> 1.7.4.1
>
>
>
On Friday, April 05, 2013 09:27:40 AM Ohad Ben-Cohen wrote:
> Hi Li,
>
> On Thu, Feb 28, 2013 at 10:02 AM, Li Fei <[email protected]> wrote:
> >
> > Even in failed case of pm_runtime_get_sync, the usage_count
> > is incremented. In order to keep the usage_count with correct
> > value and runtime power management to behave correctly, call
> > pm_runtime_put(_sync) in such case.
>
> Is it better then to call pm_runtime_put_noidle instead? This way
> we're sure to only take care of usage_count without ever calling any
> underlying pm handler.
Both would break code that does
pm_runtime_get_sync(dev);
<device access>
pm_runtime_put(dev);
without checking the result of pm_runtime_get_sync() - which BTW is completely
unnecessary in the majority of cases.
So no, it's not a good idea at all.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
On Friday, April 05, 2013 01:39:58 PM Rafael J. Wysocki wrote:
> On Friday, April 05, 2013 09:27:40 AM Ohad Ben-Cohen wrote:
> > Hi Li,
> >
> > On Thu, Feb 28, 2013 at 10:02 AM, Li Fei <[email protected]> wrote:
> > >
> > > Even in failed case of pm_runtime_get_sync, the usage_count
> > > is incremented. In order to keep the usage_count with correct
> > > value and runtime power management to behave correctly, call
> > > pm_runtime_put(_sync) in such case.
> >
> > Is it better then to call pm_runtime_put_noidle instead? This way
> > we're sure to only take care of usage_count without ever calling any
> > underlying pm handler.
>
> Both would break code that does
>
> pm_runtime_get_sync(dev);
>
> <device access>
>
> pm_runtime_put(dev);
>
> without checking the result of pm_runtime_get_sync() - which BTW is completely
> unnecessary in the majority of cases.
Sorry, scratch that. I should have had a closer look at the context.
Yes, it better to call pm_runtime_put_noidle() in this case.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
> -----Original Message-----
> From: Rafael J. Wysocki [mailto:[email protected]]
> Sent: Friday, April 05, 2013 7:42 PM
> To: Ohad Ben-Cohen; Liu, Chuansheng
> Cc: Li, Fei; [email protected]
> Subject: Re: [PATCH 5/5] hwspinlock/core: call pm_runtime_put in
> pm_runtime_get_sync failed case
>
> On Friday, April 05, 2013 01:39:58 PM Rafael J. Wysocki wrote:
> > On Friday, April 05, 2013 09:27:40 AM Ohad Ben-Cohen wrote:
> > > Hi Li,
> > >
> > > On Thu, Feb 28, 2013 at 10:02 AM, Li Fei <[email protected]> wrote:
> > > >
> > > > Even in failed case of pm_runtime_get_sync, the usage_count
> > > > is incremented. In order to keep the usage_count with correct
> > > > value and runtime power management to behave correctly, call
> > > > pm_runtime_put(_sync) in such case.
> > >
> > > Is it better then to call pm_runtime_put_noidle instead? This way
> > > we're sure to only take care of usage_count without ever calling any
> > > underlying pm handler.
> >
> > Both would break code that does
> >
> > pm_runtime_get_sync(dev);
> >
> > <device access>
> >
> > pm_runtime_put(dev);
> >
> > without checking the result of pm_runtime_get_sync() - which BTW is
> completely
> > unnecessary in the majority of cases.
>
> Sorry, scratch that. I should have had a closer look at the context.
>
> Yes, it better to call pm_runtime_put_noidle() in this case.
>
Thanks for your feedback.
I'll upload patch V2 for this topic.
Thanks,
Fei
> Thanks,
> Rafael
>
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?