From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: ning.n.jiang@gmail.com
Cc: linux@arm.linux.org.uk, kgene.kim@samsung.com,
davidb@codeaurora.org, dwalker@fifo99.com, bryanh@codeaurora.org,
john.stultz@linaro.org, tglx@linutronix.de,
linus.walleij@linaro.org, shawn.guo@linaro.org,
rob.herring@calxeda.com, arnd@arndb.de,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH] ARM: timer: Shutdown clock event device when stopping local timer
Date: Mon, 01 Apr 2013 01:41:44 +0200 [thread overview]
Message-ID: <5158C9B8.3070208@linaro.org> (raw)
In-Reply-To: <1364549049-29278-1-git-send-email-ning.n.jiang@gmail.com>
On 03/29/2013 10:24 AM, ning.n.jiang@gmail.com wrote:
> From: Ning Jiang <ning.n.jiang@gmail.com>
>
> Currently there are two problems when we try to stop local timer.
> First, it calls set_mode function directly so mode state is not
> updated for the clock event device. Second, it makes the device
> unused instead of shutdown.
>
> A subtle error will happen because of it. When a cpu is plugged out
> it will stop the local timer. It will call tick_nohz_idle_enter()
> in idle thread afterwards. It will cancel the sched timer and try
> to reprogram the next event. This is wrong since the local timer
> is supposed to be stopped.
>
> The right way to stop the local timer is to shutdown it by calling
> clockevents_set_mode(). Thus when we try to reprogram the clock
> event device, it will return directly without doing anything since
> the clock mode is CLOCK_EVT_MODE_SHUTDOWN.
>
> Signed-off-by: Ning Jiang <ning.n.jiang@gmail.com>
> ---
> arch/arm/kernel/smp_twd.c | 2 +-
> arch/arm/mach-exynos/mct.c | 2 +-
> arch/arm/mach-msm/timer.c | 2 +-
> drivers/clocksource/arm_arch_timer.c | 2 +-
> drivers/clocksource/time-armada-370-xp.c | 2 +-
> 5 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
> index 3f25650..c1d4ab4 100644
> --- a/arch/arm/kernel/smp_twd.c
> +++ b/arch/arm/kernel/smp_twd.c
> @@ -92,7 +92,7 @@ static int twd_timer_ack(void)
>
> static void twd_timer_stop(struct clock_event_device *clk)
> {
> - twd_set_mode(CLOCK_EVT_MODE_UNUSED, clk);
> + clockevents_set_mode(clk, CLOCK_EVT_MODE_SHUTDOWN);
> disable_percpu_irq(clk->irq);
Wouldn't be clockevents_shutdown more adequate here ? The next event
will be also set.
--
<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
WARNING: multiple messages have this Message-ID (diff)
From: daniel.lezcano@linaro.org (Daniel Lezcano)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: timer: Shutdown clock event device when stopping local timer
Date: Mon, 01 Apr 2013 01:41:44 +0200 [thread overview]
Message-ID: <5158C9B8.3070208@linaro.org> (raw)
In-Reply-To: <1364549049-29278-1-git-send-email-ning.n.jiang@gmail.com>
On 03/29/2013 10:24 AM, ning.n.jiang at gmail.com wrote:
> From: Ning Jiang <ning.n.jiang@gmail.com>
>
> Currently there are two problems when we try to stop local timer.
> First, it calls set_mode function directly so mode state is not
> updated for the clock event device. Second, it makes the device
> unused instead of shutdown.
>
> A subtle error will happen because of it. When a cpu is plugged out
> it will stop the local timer. It will call tick_nohz_idle_enter()
> in idle thread afterwards. It will cancel the sched timer and try
> to reprogram the next event. This is wrong since the local timer
> is supposed to be stopped.
>
> The right way to stop the local timer is to shutdown it by calling
> clockevents_set_mode(). Thus when we try to reprogram the clock
> event device, it will return directly without doing anything since
> the clock mode is CLOCK_EVT_MODE_SHUTDOWN.
>
> Signed-off-by: Ning Jiang <ning.n.jiang@gmail.com>
> ---
> arch/arm/kernel/smp_twd.c | 2 +-
> arch/arm/mach-exynos/mct.c | 2 +-
> arch/arm/mach-msm/timer.c | 2 +-
> drivers/clocksource/arm_arch_timer.c | 2 +-
> drivers/clocksource/time-armada-370-xp.c | 2 +-
> 5 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
> index 3f25650..c1d4ab4 100644
> --- a/arch/arm/kernel/smp_twd.c
> +++ b/arch/arm/kernel/smp_twd.c
> @@ -92,7 +92,7 @@ static int twd_timer_ack(void)
>
> static void twd_timer_stop(struct clock_event_device *clk)
> {
> - twd_set_mode(CLOCK_EVT_MODE_UNUSED, clk);
> + clockevents_set_mode(clk, CLOCK_EVT_MODE_SHUTDOWN);
> disable_percpu_irq(clk->irq);
Wouldn't be clockevents_shutdown more adequate here ? The next event
will be also set.
--
<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
next prev parent reply other threads:[~2013-03-31 23:41 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-29 9:24 [PATCH] ARM: timer: Shutdown clock event device when stopping local timer ning.n.jiang
2013-03-29 9:24 ` ning.n.jiang
2013-03-29 9:24 ` ning.n.jiang at gmail.com
2013-03-29 18:32 ` Stephen Boyd
2013-03-29 18:32 ` Stephen Boyd
2013-03-30 9:57 ` Ning Jiang
2013-03-30 9:57 ` Ning Jiang
2013-03-30 10:04 ` Russell King - ARM Linux
2013-03-30 10:04 ` Russell King - ARM Linux
2013-03-30 11:56 ` Ning Jiang
2013-03-30 11:56 ` Ning Jiang
2013-03-31 13:11 ` Ning Jiang
2013-03-31 13:11 ` Ning Jiang
2013-03-31 23:41 ` Daniel Lezcano [this message]
2013-03-31 23:41 ` Daniel Lezcano
2013-04-01 1:56 ` Ning Jiang
2013-04-01 1:56 ` Ning Jiang
2013-04-02 12:01 ` Ning Jiang
2013-04-02 12:01 ` Ning Jiang
2013-04-01 0:06 ` Daniel Lezcano
2013-04-01 0:06 ` Daniel Lezcano
2013-04-01 2:05 ` Ning Jiang
2013-04-01 2:05 ` Ning Jiang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5158C9B8.3070208@linaro.org \
--to=daniel.lezcano@linaro.org \
--cc=arnd@arndb.de \
--cc=bryanh@codeaurora.org \
--cc=davidb@codeaurora.org \
--cc=dwalker@fifo99.com \
--cc=john.stultz@linaro.org \
--cc=kgene.kim@samsung.com \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=ning.n.jiang@gmail.com \
--cc=rob.herring@calxeda.com \
--cc=shawn.guo@linaro.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.