diff mbox

[2/9] PM / Domains: Remove dev->driver check for runtime PM

Message ID 1438731339-58317-3-git-send-email-lina.iyer@linaro.org
State Superseded
Headers show

Commit Message

Lina Iyer Aug. 4, 2015, 11:35 p.m. UTC
Remove check for driver of a device, for runtime PM. Device may be
suspended without an explicit driver. This check seems to be vestigial
and incorrect in the current context.

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Kevin Hilman <khilman@linaro.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 drivers/base/power/domain.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Lina Iyer Aug. 24, 2015, 7:50 p.m. UTC | #1
On Fri, Aug 21 2015 at 15:04 -0600, Kevin Hilman wrote:
>Geert Uytterhoeven <geert@linux-m68k.org> writes:
>
>> Hi Kevin,
>>
>> On Fri, Aug 14, 2015 at 7:19 PM, Kevin Hilman <khilman@kernel.org> wrote:
>>> On Fri, Aug 14, 2015 at 12:24 AM, Geert Uytterhoeven
>>> <geert@linux-m68k.org> wrote:
>>>> On Fri, Aug 14, 2015 at 5:40 AM, Kevin Hilman <khilman@kernel.org> wrote:
>>>>> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>>>>>> On Wed, Aug 12, 2015 at 9:50 PM, Kevin Hilman <khilman@kernel.org> wrote:
>>>>>>> This check might have made sense before PM domains, but with PM domains,
>>>>>>> it's entirely possible to have a simple device without a driver and the
>>>>>>> PM domain handles all the necesary PM, so I think this check
>>>>>>> could/should be removed.
>>>>>>>
>>>>>>> Thoughts?
>>>>>>
>>>>>> Simple devices without a driver aren't handled automatically.
>>>>>> At minimum, the driver should call pm_runtime_enable(), cfr.
>>>>>> drivers/bus/simple-pm-bus.c.
>>>>>
>>>>> That's correct, and in the proof-of-concept stuff I hacked up and in
>>>>> Lina's series, the CPU "devices" do indeed to this.  Without that, they
>>>>> wouldn't end up ever taking this codepath through genpd's
>>>>> runtime_suspend and power_off hooks.
>>>>>
>>>>> Also, I'm not sure if your comment was meant to be an objection to the
>>>>> patch?  or if you're OK with it.
>>>>
>>>> My comment was purely meant as a response to "it's entirely possible to have a
>>>> simple device without a driver and the PM domain handles all the necesary PM".
>>>
>>> Right, so if the PM domain does the pm_runtime_enable() for these
>>> "simple" devices without drivers, they can still exist without a
>>> driver, and the PM domain doing all the magic.
>>
>> Is it possible to let the PM Domain do the pm_runtime_enable() itself in
>> the absence of a driver?
>
>Well, I suppose it's possible, not sure it's recommended. :)
>
>> If yes, I wouldn't have needed simple-pm-bus.c.
>> What if a driver is bound later?
>
>Yeah, you're approach is better.
>
I am not sure I understand the approach? Initialize the CPU devices as
"simple-pm-bus" compatible?

Thanks,
Lina
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson Sept. 1, 2015, 1:28 p.m. UTC | #2
On 5 August 2015 at 01:35, Lina Iyer <lina.iyer@linaro.org> wrote:
> Remove check for driver of a device, for runtime PM. Device may be
> suspended without an explicit driver. This check seems to be vestigial
> and incorrect in the current context.
>
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>

I think this makes perfect sense!

Moreover in *theory*, I have considered both cases for how a device
currently gets attached/detached to its genpd and I can't find any
issues with $subject patch. With *both* cases I mean attaching the
device using *pm_genpd_[name]add_device() API or via the ->probe()
sequence using the dev_pm_domain_attach() API.

Now, of course that statement also relies on that drivers behaves
properly from a runtime PM point of view, for example make sure to do
pm_runtime_disable() at ->remove().

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

> ---
>  drivers/base/power/domain.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 5fd1306..ef8d19f 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -394,8 +394,7 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
>                 if (stat > PM_QOS_FLAGS_NONE)
>                         return -EBUSY;
>
> -               if (pdd->dev->driver && (!pm_runtime_suspended(pdd->dev)
> -                   || pdd->dev->power.irq_safe))
> +               if (!pm_runtime_suspended(pdd->dev) || pdd->dev->power.irq_safe)
>                         not_suspended++;
>         }
>
> --
> 2.1.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 5fd1306..ef8d19f 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -394,8 +394,7 @@  static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
 		if (stat > PM_QOS_FLAGS_NONE)
 			return -EBUSY;
 
-		if (pdd->dev->driver && (!pm_runtime_suspended(pdd->dev)
-		    || pdd->dev->power.irq_safe))
+		if (!pm_runtime_suspended(pdd->dev) || pdd->dev->power.irq_safe)
 			not_suspended++;
 	}