* [PATCH] PM / Domains: Correct comment in irq_safe_dev_in_no_sleep_domain()
@ 2017-01-31 16:01 Ulf Hansson
2017-01-31 16:17 ` Geert Uytterhoeven
2017-01-31 16:17 ` Lina Iyer
0 siblings, 2 replies; 6+ messages in thread
From: Ulf Hansson @ 2017-01-31 16:01 UTC (permalink / raw)
To: Rafael J. Wysocki, Ulf Hansson, linux-pm
Cc: Len Brown, Pavel Machek, Kevin Hilman, Geert Uytterhoeven,
Lina Iyer
The earlier comment stated that the dev_warn_once() was going to be printed
once per device. Let's fix that, as dev_warn_once() is printed only once,
no matter of the device.
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/base/power/domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 6b23d82..271e208 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -130,7 +130,7 @@ static inline bool irq_safe_dev_in_no_sleep_domain(struct device *dev,
ret = pm_runtime_is_irq_safe(dev) && !genpd_is_irq_safe(genpd);
- /* Warn once for each IRQ safe dev in no sleep domain */
+ /* Warn once if IRQ safe dev in no sleep domain */
if (ret)
dev_warn_once(dev, "PM domain %s will not be powered off\n",
genpd->name);
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] PM / Domains: Correct comment in irq_safe_dev_in_no_sleep_domain()
2017-01-31 16:01 [PATCH] PM / Domains: Correct comment in irq_safe_dev_in_no_sleep_domain() Ulf Hansson
@ 2017-01-31 16:17 ` Geert Uytterhoeven
2017-01-31 16:41 ` Ulf Hansson
2017-01-31 16:17 ` Lina Iyer
1 sibling, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2017-01-31 16:17 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rafael J. Wysocki, Linux PM list, Len Brown, Pavel Machek,
Kevin Hilman, Lina Iyer
Hi Ulf,
On Tue, Jan 31, 2017 at 5:01 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> The earlier comment stated that the dev_warn_once() was going to be printed
> once per device. Let's fix that, as dev_warn_once() is printed only once,
> no matter of the device.
While I agree this makes the comment match the code, I think we would serve
the users better by printing the warning once per PM domain.
Currently the user cannot know if two or more PM domains cannot be powered
off due to IRQ safe devices.
Perhaps a flag can be added to generic_pm_domain.flags to remember
that the warning has been printed before?
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> drivers/base/power/domain.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 6b23d82..271e208 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -130,7 +130,7 @@ static inline bool irq_safe_dev_in_no_sleep_domain(struct device *dev,
>
> ret = pm_runtime_is_irq_safe(dev) && !genpd_is_irq_safe(genpd);
>
> - /* Warn once for each IRQ safe dev in no sleep domain */
> + /* Warn once if IRQ safe dev in no sleep domain */
> if (ret)
> dev_warn_once(dev, "PM domain %s will not be powered off\n",
> genpd->name);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] PM / Domains: Correct comment in irq_safe_dev_in_no_sleep_domain()
2017-01-31 16:17 ` Geert Uytterhoeven
@ 2017-01-31 16:41 ` Ulf Hansson
2017-01-31 17:17 ` Geert Uytterhoeven
0 siblings, 1 reply; 6+ messages in thread
From: Ulf Hansson @ 2017-01-31 16:41 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Rafael J. Wysocki, Linux PM list, Len Brown, Pavel Machek,
Kevin Hilman, Lina Iyer
On 31 January 2017 at 17:17, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Ulf,
>
> On Tue, Jan 31, 2017 at 5:01 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> The earlier comment stated that the dev_warn_once() was going to be printed
>> once per device. Let's fix that, as dev_warn_once() is printed only once,
>> no matter of the device.
>
> While I agree this makes the comment match the code, I think we would serve
> the users better by printing the warning once per PM domain.
> Currently the user cannot know if two or more PM domains cannot be powered
> off due to IRQ safe devices.
Right.
>
> Perhaps a flag can be added to generic_pm_domain.flags to remember
> that the warning has been printed before?
That seems like a reasonable adjustment. Allow me to cook a patch on
top of this one.
Moreover, I was thinking of considering to check for always on domains
and perhaps skip printing this message in such cases. Would that make
sense as well?
Kind regards
Uffe
>
>> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>> drivers/base/power/domain.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index 6b23d82..271e208 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -130,7 +130,7 @@ static inline bool irq_safe_dev_in_no_sleep_domain(struct device *dev,
>>
>> ret = pm_runtime_is_irq_safe(dev) && !genpd_is_irq_safe(genpd);
>>
>> - /* Warn once for each IRQ safe dev in no sleep domain */
>> + /* Warn once if IRQ safe dev in no sleep domain */
>> if (ret)
>> dev_warn_once(dev, "PM domain %s will not be powered off\n",
>> genpd->name);
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] PM / Domains: Correct comment in irq_safe_dev_in_no_sleep_domain()
2017-01-31 16:41 ` Ulf Hansson
@ 2017-01-31 17:17 ` Geert Uytterhoeven
2017-02-02 9:55 ` Ulf Hansson
0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2017-01-31 17:17 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rafael J. Wysocki, Linux PM list, Len Brown, Pavel Machek,
Kevin Hilman, Lina Iyer
Hi Ulf,
On Tue, Jan 31, 2017 at 5:41 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 31 January 2017 at 17:17, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Tue, Jan 31, 2017 at 5:01 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>> The earlier comment stated that the dev_warn_once() was going to be printed
>>> once per device. Let's fix that, as dev_warn_once() is printed only once,
>>> no matter of the device.
>>
>> While I agree this makes the comment match the code, I think we would serve
>> the users better by printing the warning once per PM domain.
>> Currently the user cannot know if two or more PM domains cannot be powered
>> off due to IRQ safe devices.
>
> Right.
>
>> Perhaps a flag can be added to generic_pm_domain.flags to remember
>> that the warning has been printed before?
>
> That seems like a reasonable adjustment. Allow me to cook a patch on
> top of this one.
Thanks!
> Moreover, I was thinking of considering to check for always on domains
> and perhaps skip printing this message in such cases. Would that make
> sense as well?
That would make sense.
But how would you check that?
By comparing its governor with &pm_domain_always_on_gov?
Note that that is not sufficient, as I think its power_off() callback will still
be called. Only if it fails to power off and returns -EBUSY, it's a real
always-on domain.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] PM / Domains: Correct comment in irq_safe_dev_in_no_sleep_domain()
2017-01-31 17:17 ` Geert Uytterhoeven
@ 2017-02-02 9:55 ` Ulf Hansson
0 siblings, 0 replies; 6+ messages in thread
From: Ulf Hansson @ 2017-02-02 9:55 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Rafael J. Wysocki, Linux PM list, Len Brown, Pavel Machek,
Kevin Hilman, Lina Iyer
On 31 January 2017 at 18:17, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Ulf,
>
> On Tue, Jan 31, 2017 at 5:41 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 31 January 2017 at 17:17, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>> On Tue, Jan 31, 2017 at 5:01 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>>> The earlier comment stated that the dev_warn_once() was going to be printed
>>>> once per device. Let's fix that, as dev_warn_once() is printed only once,
>>>> no matter of the device.
>>>
>>> While I agree this makes the comment match the code, I think we would serve
>>> the users better by printing the warning once per PM domain.
>>> Currently the user cannot know if two or more PM domains cannot be powered
>>> off due to IRQ safe devices.
>>
>> Right.
>>
>>> Perhaps a flag can be added to generic_pm_domain.flags to remember
>>> that the warning has been printed before?
>>
>> That seems like a reasonable adjustment. Allow me to cook a patch on
>> top of this one.
>
> Thanks!
>
>> Moreover, I was thinking of considering to check for always on domains
>> and perhaps skip printing this message in such cases. Would that make
>> sense as well?
>
> That would make sense.
>
> But how would you check that?
> By comparing its governor with &pm_domain_always_on_gov?
> Note that that is not sufficient, as I think its power_off() callback will still
> be called. Only if it fails to power off and returns -EBUSY, it's a real
> always-on domain.
Hi Geert,
Correct! At this point, I am looking into how I in general can improve
the code in genpd dealing with always on PM domains.
It seems like the genpd client, shouldn't need to implement the
->power_off|on() callbacks at all, but instead just configure the
genpd at init time as an always on PM domain.
Then that information can be used in genpd at various places, to do
optimizations and likely to avoid printing non-needed errors/warnings.
I keep you posted.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PM / Domains: Correct comment in irq_safe_dev_in_no_sleep_domain()
2017-01-31 16:01 [PATCH] PM / Domains: Correct comment in irq_safe_dev_in_no_sleep_domain() Ulf Hansson
2017-01-31 16:17 ` Geert Uytterhoeven
@ 2017-01-31 16:17 ` Lina Iyer
1 sibling, 0 replies; 6+ messages in thread
From: Lina Iyer @ 2017-01-31 16:17 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rafael J. Wysocki, linux-pm, Len Brown, Pavel Machek,
Kevin Hilman, Geert Uytterhoeven
On Tue, Jan 31 2017 at 09:01 -0700, Ulf Hansson wrote:
>The earlier comment stated that the dev_warn_once() was going to be printed
>once per device. Let's fix that, as dev_warn_once() is printed only once,
>no matter of the device.
>
>Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
>Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Acked-by: Lina Iyer <lina.iyer@linaro.org>
>---
> drivers/base/power/domain.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>index 6b23d82..271e208 100644
>--- a/drivers/base/power/domain.c
>+++ b/drivers/base/power/domain.c
>@@ -130,7 +130,7 @@ static inline bool irq_safe_dev_in_no_sleep_domain(struct device *dev,
>
> ret = pm_runtime_is_irq_safe(dev) && !genpd_is_irq_safe(genpd);
>
>- /* Warn once for each IRQ safe dev in no sleep domain */
>+ /* Warn once if IRQ safe dev in no sleep domain */
> if (ret)
> dev_warn_once(dev, "PM domain %s will not be powered off\n",
> genpd->name);
>--
>1.9.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-02-02 9:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-31 16:01 [PATCH] PM / Domains: Correct comment in irq_safe_dev_in_no_sleep_domain() Ulf Hansson
2017-01-31 16:17 ` Geert Uytterhoeven
2017-01-31 16:41 ` Ulf Hansson
2017-01-31 17:17 ` Geert Uytterhoeven
2017-02-02 9:55 ` Ulf Hansson
2017-01-31 16:17 ` Lina Iyer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).