From: daniel.lezcano@linaro.org (Daniel Lezcano)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v4] clocksource: arch_timer: Fix code to use physical timers when requested
Date: Thu, 20 Nov 2014 10:23:01 +0100 [thread overview]
Message-ID: <546DB2F5.8070309@linaro.org> (raw)
In-Reply-To: <1412753937-29343-1-git-send-email-sonnyrao@chromium.org>
On 10/08/2014 09:38 AM, Sonny Rao wrote:
> This is a bug fix for using physical arch timers when
> the arch_timer_use_virtual boolean is false. It restores the
> arch_counter_get_cntpct() function after removal in
>
> 0d651e4e "clocksource: arch_timer: use virtual counters"
>
> We need this on certain ARMv7 systems which are architected like this:
>
> * The firmware doesn't know and doesn't care about hypervisor mode and
> we don't want to add the complexity of hypervisor there.
>
> * The firmware isn't involved in SMP bringup or resume.
>
> * The ARCH timer come up with an uninitialized offset between the
> virtual and physical counters. Each core gets a different random
> offset.
>
> * The device boots in "Secure SVC" mode.
>
> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
> CNTHCTL.PL1PCTEN (both default to 1 at reset)
>
> One example of such as system is RK3288 where it is much simpler to
> use the physical counter since there's nobody managing the offset and
> each time a core goes down and comes back up it will get reinitialized
> to some other random value.
>
> Fixes: 0d651e4e65e9 ("clocksource: arch_timer: use virtual counters")
> Cc: stable at vger.kernel.org
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Acked-by: Olof Johansson <olof@lixom.net>
> ---
Hi Sonny,
the patch does not apply. Could you refresh it please ?
Thanks
-- Daniel
> v2: Add fixes tag to commit message, cc stable, copy Doug's
> description of the systems which need this in commit message.
> v3: Don't change the memory-mapped physical timer/counter code
> v4: remove the memory-mapped physical counter code since it's not used
> ---
> arch/arm/include/asm/arch_timer.h | 9 +++++++++
> arch/arm64/include/asm/arch_timer.h | 10 ++++++++++
> drivers/clocksource/arm_arch_timer.c | 10 +++++++---
> 3 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
> index 0704e0c..e72aa4d 100644
> --- a/arch/arm/include/asm/arch_timer.h
> +++ b/arch/arm/include/asm/arch_timer.h
> @@ -78,6 +78,15 @@ static inline u32 arch_timer_get_cntfrq(void)
> return val;
> }
>
> +static inline u64 arch_counter_get_cntpct(void)
> +{
> + u64 cval;
> +
> + isb();
> + asm volatile("mrrc p15, 0, %Q0, %R0, c14" : "=r" (cval));
> + return cval;
> +}
> +
> static inline u64 arch_counter_get_cntvct(void)
> {
> u64 cval;
> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> index 9400596..58657c4 100644
> --- a/arch/arm64/include/asm/arch_timer.h
> +++ b/arch/arm64/include/asm/arch_timer.h
> @@ -135,6 +135,16 @@ static inline void arch_timer_evtstrm_enable(int divider)
> #endif
> }
>
> +static inline u64 arch_counter_get_cntpct(void)
> +{
> + u64 cval;
> +
> + isb();
> + asm volatile("mrs %0, cntpct_el0" : "=r" (cval));
> +
> + return cval;
> +}
> +
> static inline u64 arch_counter_get_cntvct(void)
> {
> u64 cval;
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 6b50311..799139f 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -429,10 +429,14 @@ static void __init arch_counter_register(unsigned type)
> u64 start_count;
>
> /* Register the CP15 based counter if we have one */
> - if (type & ARCH_CP15_TIMER)
> - arch_timer_read_counter = arch_counter_get_cntvct;
> - else
> + if (type & ARCH_CP15_TIMER) {
> + if (arch_timer_use_virtual)
> + arch_timer_read_counter = arch_counter_get_cntvct;
> + else
> + arch_timer_read_counter = arch_counter_get_cntpct;
> + } else {
> arch_timer_read_counter = arch_counter_get_cntvct_mem;
> + }
>
> start_count = arch_timer_read_counter();
> clocksource_register_hz(&clocksource_counter, arch_timer_rate);
>
--
<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 <daniel.lezcano@linaro.org>
To: Sonny Rao <sonnyrao@chromium.org>, linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org, dianders@chromium.org,
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>,
Olof Johansson <olof@lixom.net>,
Thomas Gleixner <tglx@linutronix.de>,
Will Deacon <will.deacon@arm.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Russell King <linux@arm.linux.org.uk>,
Sudeep Holla <Sudeep.Holla@arm.com>,
Mark Rutland <Mark.Rutland@arm.com>,
Stephen Boyd <sboyd@codeaurora.org>,
Marc Zyngier <marc.zyngier@arm.com>,
stable@vger.kernel.org
Subject: Re: [PATCH v4] clocksource: arch_timer: Fix code to use physical timers when requested
Date: Thu, 20 Nov 2014 10:23:01 +0100 [thread overview]
Message-ID: <546DB2F5.8070309@linaro.org> (raw)
In-Reply-To: <1412753937-29343-1-git-send-email-sonnyrao@chromium.org>
On 10/08/2014 09:38 AM, Sonny Rao wrote:
> This is a bug fix for using physical arch timers when
> the arch_timer_use_virtual boolean is false. It restores the
> arch_counter_get_cntpct() function after removal in
>
> 0d651e4e "clocksource: arch_timer: use virtual counters"
>
> We need this on certain ARMv7 systems which are architected like this:
>
> * The firmware doesn't know and doesn't care about hypervisor mode and
> we don't want to add the complexity of hypervisor there.
>
> * The firmware isn't involved in SMP bringup or resume.
>
> * The ARCH timer come up with an uninitialized offset between the
> virtual and physical counters. Each core gets a different random
> offset.
>
> * The device boots in "Secure SVC" mode.
>
> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
> CNTHCTL.PL1PCTEN (both default to 1 at reset)
>
> One example of such as system is RK3288 where it is much simpler to
> use the physical counter since there's nobody managing the offset and
> each time a core goes down and comes back up it will get reinitialized
> to some other random value.
>
> Fixes: 0d651e4e65e9 ("clocksource: arch_timer: use virtual counters")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Acked-by: Olof Johansson <olof@lixom.net>
> ---
Hi Sonny,
the patch does not apply. Could you refresh it please ?
Thanks
-- Daniel
> v2: Add fixes tag to commit message, cc stable, copy Doug's
> description of the systems which need this in commit message.
> v3: Don't change the memory-mapped physical timer/counter code
> v4: remove the memory-mapped physical counter code since it's not used
> ---
> arch/arm/include/asm/arch_timer.h | 9 +++++++++
> arch/arm64/include/asm/arch_timer.h | 10 ++++++++++
> drivers/clocksource/arm_arch_timer.c | 10 +++++++---
> 3 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
> index 0704e0c..e72aa4d 100644
> --- a/arch/arm/include/asm/arch_timer.h
> +++ b/arch/arm/include/asm/arch_timer.h
> @@ -78,6 +78,15 @@ static inline u32 arch_timer_get_cntfrq(void)
> return val;
> }
>
> +static inline u64 arch_counter_get_cntpct(void)
> +{
> + u64 cval;
> +
> + isb();
> + asm volatile("mrrc p15, 0, %Q0, %R0, c14" : "=r" (cval));
> + return cval;
> +}
> +
> static inline u64 arch_counter_get_cntvct(void)
> {
> u64 cval;
> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> index 9400596..58657c4 100644
> --- a/arch/arm64/include/asm/arch_timer.h
> +++ b/arch/arm64/include/asm/arch_timer.h
> @@ -135,6 +135,16 @@ static inline void arch_timer_evtstrm_enable(int divider)
> #endif
> }
>
> +static inline u64 arch_counter_get_cntpct(void)
> +{
> + u64 cval;
> +
> + isb();
> + asm volatile("mrs %0, cntpct_el0" : "=r" (cval));
> +
> + return cval;
> +}
> +
> static inline u64 arch_counter_get_cntvct(void)
> {
> u64 cval;
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 6b50311..799139f 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -429,10 +429,14 @@ static void __init arch_counter_register(unsigned type)
> u64 start_count;
>
> /* Register the CP15 based counter if we have one */
> - if (type & ARCH_CP15_TIMER)
> - arch_timer_read_counter = arch_counter_get_cntvct;
> - else
> + if (type & ARCH_CP15_TIMER) {
> + if (arch_timer_use_virtual)
> + arch_timer_read_counter = arch_counter_get_cntvct;
> + else
> + arch_timer_read_counter = arch_counter_get_cntpct;
> + } else {
> arch_timer_read_counter = arch_counter_get_cntvct_mem;
> + }
>
> start_count = arch_timer_read_counter();
> clocksource_register_hz(&clocksource_counter, arch_timer_rate);
>
--
<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:[~2014-11-20 9:23 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-08 7:38 [PATCH v4] clocksource: arch_timer: Fix code to use physical timers when requested Sonny Rao
2014-10-08 7:38 ` Sonny Rao
2014-11-20 8:49 ` Maxime Ripard
2014-11-20 8:49 ` Maxime Ripard
2014-11-20 9:18 ` Sonny Rao
2014-11-20 9:18 ` Sonny Rao
2014-11-20 9:23 ` Daniel Lezcano [this message]
2014-11-20 9:23 ` Daniel Lezcano
2014-11-20 10:20 ` Daniel Lezcano
2014-11-20 10:20 ` Daniel Lezcano
2014-11-20 16:10 ` Catalin Marinas
2014-11-20 16:10 ` Catalin Marinas
2014-11-20 16:24 ` Doug Anderson
2014-11-20 16:24 ` Doug Anderson
2014-11-20 16:58 ` Catalin Marinas
2014-11-20 16:58 ` Catalin Marinas
2014-11-21 20:58 ` Olof Johansson
2014-11-21 20:58 ` Olof Johansson
2014-11-24 2:25 ` Sonny Rao
2014-11-24 2:25 ` Sonny Rao
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=546DB2F5.8070309@linaro.org \
--to=daniel.lezcano@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
/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.