* [PATCH v2] clocksource: sh_cmt: Only perform clocksource suspend/resume if enabled
@ 2015-07-14 11:41 ` Geert Uytterhoeven
0 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2015-07-14 11:41 UTC (permalink / raw)
To: Daniel Lezcano, Thomas Gleixner
Cc: linux-pm, linux-sh, linux-kernel, Geert Uytterhoeven
Currently the sh_cmt clocksource timer is disabled or enabled
unconditionally on clocksource suspend resp. resume, even if a better
clocksource is present (e.g. arch_sys_counter) and the sh_cmt
clocksource is not enabled.
As sh_cmt is a syscore device when its timer is enabled, this may lead
to a genpd.prepared_count imbalance in the presence of PM Domains, which
may cause a lock-up during reboot after s2ram.
During suspend:
- pm_genpd_prepare() is called for all non-syscore devices (incl.
sh_cmt), increasing genpd.prepared_count for each device,
- clocksource.suspend() is called for all clocksource devices,
- sh_cmt_clocksource_suspend() calls sh_cmt_stop(), which is a no-op
as the clocksource was not enabled.
During resume:
- clocksource.resume() is called for all clocksource devices,
- sh_cmt_clocksource_resume() calls sh_cmt_start(), which enables the
clocksource timer, and turns sh_cmt into a syscore device,
- pm_genpd_complete() is called for all non-syscore devices (excl.
sh_cmt now!), decreasing genpd.prepared_count for each device but
sh_cmt.
Now genpd.prepared_count of the PM Domain containing sh_cmt is still 1
instead of zero. On subsequent suspend/resume cycles, sh_cmt is still a
syscore device, hence it's skipped for pm_genpd_{prepare,complete}(),
keeping the imbalance of genpd.prepared_count at 1.
During reboot:
- platform_drv_shutdown() is called for any platform device that has
a driver with a .shutdown() method (only rcar-dmac on R-Car Gen2),
- platform_drv_shutdown() calls dev_pm_domain_detach(), which
calls genpd_dev_pm_detach(),
- genpd_dev_pm_detach() keeps calling pm_genpd_remove_device() until
it doesn't return -EAGAIN[*],
- If the device is part of the same PM Domain as sh_cmt,
pm_genpd_remove_device() always fails with -EAGAIN due to
genpd.prepared_count > 0.
- Infinite loop in genpd_dev_pm_detach()[*].
[*] Commit 93af5e9354432828 ("PM / Domains: Avoid infinite loops in
attach/detach code") already limited the number of loop iterations,
avoiding the lock-up.
To fix this, only disable or enable the clocksource timer on clocksource
suspend resp. resume if the clocksource was enabled.
This was tested on r8a7791/koelsch with the CPG Clock Domain:
- using arch_sys_counter as the clocksource, which is the default, and
which showed the problem,
- using sh_cmt as a clocksource ("echo ffca0000.timer > \
/sys/devices/system/clocksource/clocksource0/current_clocksource"),
which behaves the same as before.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
v1 was part of "[PATCH 0/2] PM / Domains: Infinite loop during reboot"
(https://lkml.org/lkml/2015/6/18/212). The other part has been applied
already (commit 93af5e9354432828).
v2:
- Add Acked-by,
- Add a reference to the commit that fixed the infinite loop in the
core PM Domain code.
---
drivers/clocksource/sh_cmt.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/clocksource/sh_cmt.c b/drivers/clocksource/sh_cmt.c
index d56d4e0e3fb3956d..ef799532a7a7963e 100644
--- a/drivers/clocksource/sh_cmt.c
+++ b/drivers/clocksource/sh_cmt.c
@@ -661,6 +661,9 @@ static void sh_cmt_clocksource_suspend(struct clocksource *cs)
{
struct sh_cmt_channel *ch = cs_to_sh_cmt(cs);
+ if (!ch->cs_enabled)
+ return;
+
sh_cmt_stop(ch, FLAG_CLOCKSOURCE);
pm_genpd_syscore_poweroff(&ch->cmt->pdev->dev);
}
@@ -669,6 +672,9 @@ static void sh_cmt_clocksource_resume(struct clocksource *cs)
{
struct sh_cmt_channel *ch = cs_to_sh_cmt(cs);
+ if (!ch->cs_enabled)
+ return;
+
pm_genpd_syscore_poweron(&ch->cmt->pdev->dev);
sh_cmt_start(ch, FLAG_CLOCKSOURCE);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2] clocksource: sh_cmt: Only perform clocksource suspend/resume if enabled
@ 2015-07-14 11:41 ` Geert Uytterhoeven
0 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2015-07-14 11:41 UTC (permalink / raw)
To: Daniel Lezcano, Thomas Gleixner
Cc: linux-pm, linux-sh, linux-kernel, Geert Uytterhoeven
Currently the sh_cmt clocksource timer is disabled or enabled
unconditionally on clocksource suspend resp. resume, even if a better
clocksource is present (e.g. arch_sys_counter) and the sh_cmt
clocksource is not enabled.
As sh_cmt is a syscore device when its timer is enabled, this may lead
to a genpd.prepared_count imbalance in the presence of PM Domains, which
may cause a lock-up during reboot after s2ram.
During suspend:
- pm_genpd_prepare() is called for all non-syscore devices (incl.
sh_cmt), increasing genpd.prepared_count for each device,
- clocksource.suspend() is called for all clocksource devices,
- sh_cmt_clocksource_suspend() calls sh_cmt_stop(), which is a no-op
as the clocksource was not enabled.
During resume:
- clocksource.resume() is called for all clocksource devices,
- sh_cmt_clocksource_resume() calls sh_cmt_start(), which enables the
clocksource timer, and turns sh_cmt into a syscore device,
- pm_genpd_complete() is called for all non-syscore devices (excl.
sh_cmt now!), decreasing genpd.prepared_count for each device but
sh_cmt.
Now genpd.prepared_count of the PM Domain containing sh_cmt is still 1
instead of zero. On subsequent suspend/resume cycles, sh_cmt is still a
syscore device, hence it's skipped for pm_genpd_{prepare,complete}(),
keeping the imbalance of genpd.prepared_count at 1.
During reboot:
- platform_drv_shutdown() is called for any platform device that has
a driver with a .shutdown() method (only rcar-dmac on R-Car Gen2),
- platform_drv_shutdown() calls dev_pm_domain_detach(), which
calls genpd_dev_pm_detach(),
- genpd_dev_pm_detach() keeps calling pm_genpd_remove_device() until
it doesn't return -EAGAIN[*],
- If the device is part of the same PM Domain as sh_cmt,
pm_genpd_remove_device() always fails with -EAGAIN due to
genpd.prepared_count > 0.
- Infinite loop in genpd_dev_pm_detach()[*].
[*] Commit 93af5e9354432828 ("PM / Domains: Avoid infinite loops in
attach/detach code") already limited the number of loop iterations,
avoiding the lock-up.
To fix this, only disable or enable the clocksource timer on clocksource
suspend resp. resume if the clocksource was enabled.
This was tested on r8a7791/koelsch with the CPG Clock Domain:
- using arch_sys_counter as the clocksource, which is the default, and
which showed the problem,
- using sh_cmt as a clocksource ("echo ffca0000.timer > \
/sys/devices/system/clocksource/clocksource0/current_clocksource"),
which behaves the same as before.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
v1 was part of "[PATCH 0/2] PM / Domains: Infinite loop during reboot"
(https://lkml.org/lkml/2015/6/18/212). The other part has been applied
already (commit 93af5e9354432828).
v2:
- Add Acked-by,
- Add a reference to the commit that fixed the infinite loop in the
core PM Domain code.
---
drivers/clocksource/sh_cmt.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/clocksource/sh_cmt.c b/drivers/clocksource/sh_cmt.c
index d56d4e0e3fb3956d..ef799532a7a7963e 100644
--- a/drivers/clocksource/sh_cmt.c
+++ b/drivers/clocksource/sh_cmt.c
@@ -661,6 +661,9 @@ static void sh_cmt_clocksource_suspend(struct clocksource *cs)
{
struct sh_cmt_channel *ch = cs_to_sh_cmt(cs);
+ if (!ch->cs_enabled)
+ return;
+
sh_cmt_stop(ch, FLAG_CLOCKSOURCE);
pm_genpd_syscore_poweroff(&ch->cmt->pdev->dev);
}
@@ -669,6 +672,9 @@ static void sh_cmt_clocksource_resume(struct clocksource *cs)
{
struct sh_cmt_channel *ch = cs_to_sh_cmt(cs);
+ if (!ch->cs_enabled)
+ return;
+
pm_genpd_syscore_poweron(&ch->cmt->pdev->dev);
sh_cmt_start(ch, FLAG_CLOCKSOURCE);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] clocksource: sh_cmt: Only perform clocksource suspend/resume if enabled
2015-07-14 11:41 ` Geert Uytterhoeven
@ 2015-07-16 13:41 ` Daniel Lezcano
-1 siblings, 0 replies; 10+ messages in thread
From: Daniel Lezcano @ 2015-07-16 13:41 UTC (permalink / raw)
To: Geert Uytterhoeven, Thomas Gleixner; +Cc: linux-pm, linux-sh, linux-kernel
Hi Geert,
do you mean to test if the patch [1] solves your problem.
-- Daniel
[1] https://lkml.org/lkml/2015/1/16/81
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] clocksource: sh_cmt: Only perform clocksource suspend/resume if enabled
@ 2015-07-16 13:41 ` Daniel Lezcano
0 siblings, 0 replies; 10+ messages in thread
From: Daniel Lezcano @ 2015-07-16 13:41 UTC (permalink / raw)
To: Geert Uytterhoeven, Thomas Gleixner; +Cc: linux-pm, linux-sh, linux-kernel
Hi Geert,
do you mean to test if the patch [1] solves your problem.
-- Daniel
[1] https://lkml.org/lkml/2015/1/16/81
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] clocksource: sh_cmt: Only perform clocksource suspend/resume if enabled
2015-07-16 13:41 ` Daniel Lezcano
@ 2015-07-16 14:03 ` Geert Uytterhoeven
-1 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2015-07-16 14:03 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Geert Uytterhoeven, Thomas Gleixner, Linux PM list,
Linux-sh list, linux-kernel
Hi Daniel,
On Thu, Jul 16, 2015 at 3:41 PM, Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
> do you mean to test if the patch [1] solves your problem.
>
> -- Daniel
>
> [1] https://lkml.org/lkml/2015/1/16/81
I'll give it a try after my holidays...
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] 10+ messages in thread
* Re: [PATCH v2] clocksource: sh_cmt: Only perform clocksource suspend/resume if enabled
@ 2015-07-16 14:03 ` Geert Uytterhoeven
0 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2015-07-16 14:03 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Geert Uytterhoeven, Thomas Gleixner, Linux PM list,
Linux-sh list, linux-kernel
Hi Daniel,
On Thu, Jul 16, 2015 at 3:41 PM, Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
> do you mean to test if the patch [1] solves your problem.
>
> -- Daniel
>
> [1] https://lkml.org/lkml/2015/1/16/81
I'll give it a try after my holidays...
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] 10+ messages in thread
* Re: [PATCH v2] clocksource: sh_cmt: Only perform clocksource suspend/resume if enabled
2015-07-16 14:03 ` Geert Uytterhoeven
@ 2015-08-05 13:56 ` Geert Uytterhoeven
-1 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2015-08-05 13:56 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Geert Uytterhoeven, Thomas Gleixner, Linux PM list,
Linux-sh list, linux-kernel
Hi Daniel,
On Thu, Jul 16, 2015 at 4:03 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Thu, Jul 16, 2015 at 3:41 PM, Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
>> do you mean to test if the patch [1] solves your problem.
>>
>> -- Daniel
>>
>> [1] https://lkml.org/lkml/2015/1/16/81
>
> I'll give it a try after my holidays...
Apparently that patch was integrated in v4.2-rc1, while I still see the problem
on recent kernels.
Furthermore, that patch handles clockevents, not clocksources.
struct clocksource doesn't seem to have a way to check if it's enabled,
so I still need my patch.
Thanks!
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] 10+ messages in thread
* Re: [PATCH v2] clocksource: sh_cmt: Only perform clocksource suspend/resume if enabled
@ 2015-08-05 13:56 ` Geert Uytterhoeven
0 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2015-08-05 13:56 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Geert Uytterhoeven, Thomas Gleixner, Linux PM list,
Linux-sh list, linux-kernel
Hi Daniel,
On Thu, Jul 16, 2015 at 4:03 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Thu, Jul 16, 2015 at 3:41 PM, Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
>> do you mean to test if the patch [1] solves your problem.
>>
>> -- Daniel
>>
>> [1] https://lkml.org/lkml/2015/1/16/81
>
> I'll give it a try after my holidays...
Apparently that patch was integrated in v4.2-rc1, while I still see the problem
on recent kernels.
Furthermore, that patch handles clockevents, not clocksources.
struct clocksource doesn't seem to have a way to check if it's enabled,
so I still need my patch.
Thanks!
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] 10+ messages in thread
* Re: [PATCH v2] clocksource: sh_cmt: Only perform clocksource suspend/resume if enabled
2015-08-05 13:56 ` Geert Uytterhoeven
@ 2015-08-05 16:59 ` Daniel Lezcano
-1 siblings, 0 replies; 10+ messages in thread
From: Daniel Lezcano @ 2015-08-05 16:59 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Geert Uytterhoeven, Thomas Gleixner, Linux PM list,
Linux-sh list, linux-kernel
On 08/05/2015 03:56 PM, Geert Uytterhoeven wrote:
> Hi Daniel,
>
> On Thu, Jul 16, 2015 at 4:03 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Thu, Jul 16, 2015 at 3:41 PM, Daniel Lezcano
>> <daniel.lezcano@linaro.org> wrote:
>>> do you mean to test if the patch [1] solves your problem.
>>>
>>> -- Daniel
>>>
>>> [1] https://lkml.org/lkml/2015/1/16/81
>>
>> I'll give it a try after my holidays...
>
> Apparently that patch was integrated in v4.2-rc1, while I still see the problem
> on recent kernels.
>
> Furthermore, that patch handles clockevents, not clocksources.
> struct clocksource doesn't seem to have a way to check if it's enabled,
> so I still need my patch.
Hmm, I see. I will take this patch as a fix then.
Thanks.
-- Daniel
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] clocksource: sh_cmt: Only perform clocksource suspend/resume if enabled
@ 2015-08-05 16:59 ` Daniel Lezcano
0 siblings, 0 replies; 10+ messages in thread
From: Daniel Lezcano @ 2015-08-05 16:59 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Geert Uytterhoeven, Thomas Gleixner, Linux PM list,
Linux-sh list, linux-kernel
On 08/05/2015 03:56 PM, Geert Uytterhoeven wrote:
> Hi Daniel,
>
> On Thu, Jul 16, 2015 at 4:03 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Thu, Jul 16, 2015 at 3:41 PM, Daniel Lezcano
>> <daniel.lezcano@linaro.org> wrote:
>>> do you mean to test if the patch [1] solves your problem.
>>>
>>> -- Daniel
>>>
>>> [1] https://lkml.org/lkml/2015/1/16/81
>>
>> I'll give it a try after my holidays...
>
> Apparently that patch was integrated in v4.2-rc1, while I still see the problem
> on recent kernels.
>
> Furthermore, that patch handles clockevents, not clocksources.
> struct clocksource doesn't seem to have a way to check if it's enabled,
> so I still need my patch.
Hmm, I see. I will take this patch as a fix then.
Thanks.
-- Daniel
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-08-05 16:59 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-14 11:41 [PATCH v2] clocksource: sh_cmt: Only perform clocksource suspend/resume if enabled Geert Uytterhoeven
2015-07-14 11:41 ` Geert Uytterhoeven
2015-07-16 13:41 ` Daniel Lezcano
2015-07-16 13:41 ` Daniel Lezcano
2015-07-16 14:03 ` Geert Uytterhoeven
2015-07-16 14:03 ` Geert Uytterhoeven
2015-08-05 13:56 ` Geert Uytterhoeven
2015-08-05 13:56 ` Geert Uytterhoeven
2015-08-05 16:59 ` Daniel Lezcano
2015-08-05 16:59 ` Daniel Lezcano
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.