Like several of the other files in drivers/clocksource,
tegra20_timer.c contains code that can only compile when CONFIG_ARM is
enabled. This causes obvious problems when trying to compile this
code for NVIDIA ARM64-based SoCs, such as Tegra132. The same timer IP
blocks exist, so it seems appropriate to provide support for them.
So until we figure out a better way to partition this code, wrap the
delay_timer and persistent_clock support code with preprocessor tests
for CONFIG_ARM. (The delay_timer code should not be needed at all on
ARM64 due to the presence of the ARMv8 architected timer. The
persistent_clock support code could become important once power
management modes are implemented that turn off the CPU complex.)
Signed-off-by: Paul Walmsley <[email protected]>
Signed-off-by: Paul Walmsley <[email protected]>
Cc: Allen Martin <[email protected]>
Cc: Stephen Warren <[email protected]>
Cc: Thierry Reding <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Alexandre Courbot <[email protected]>
---
Applies against next-20141209.
Intended for v3.20.
Boot-tested on Tegra124 Jetson TK1 on next-20141209.
Also boot-tested on Tegra132 Norrin FFD on next-20141209 + extra,
unrelated patches.
drivers/clocksource/tegra20_timer.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/clocksource/tegra20_timer.c b/drivers/clocksource/tegra20_timer.c
index d2616ef16770..83a8f5c9e139 100644
--- a/drivers/clocksource/tegra20_timer.c
+++ b/drivers/clocksource/tegra20_timer.c
@@ -29,8 +29,10 @@
#include <linux/sched_clock.h>
#include <linux/delay.h>
+#ifdef CONFIG_ARM
#include <asm/mach/time.h>
#include <asm/smp_twd.h>
+#endif
#define RTC_SECONDS 0x08
#define RTC_SHADOW_SECONDS 0x0c
@@ -49,12 +51,14 @@
#define TIMER_PCR 0x4
static void __iomem *timer_reg_base;
+#ifdef CONFIG_ARM
static void __iomem *rtc_base;
static struct timespec persistent_ts;
static u64 persistent_ms, last_persistent_ms;
static struct delay_timer tegra_delay_timer;
+#endif
#define timer_writel(value, reg) \
__raw_writel(value, timer_reg_base + (reg))
@@ -106,6 +110,7 @@ static u64 notrace tegra_read_sched_clock(void)
return timer_readl(TIMERUS_CNTR_1US);
}
+#ifdef CONFIG_ARM
/*
* tegra_rtc_read - Reads the Tegra RTC registers
* Care must be taken that this funciton is not called while the
@@ -146,6 +151,8 @@ static unsigned long tegra_delay_timer_read_counter_long(void)
{
return readl(timer_reg_base + TIMERUS_CNTR_1US);
}
+#endif /* CONFIG_ARM */
+
static irqreturn_t tegra_timer_interrupt(int irq, void *dev_id)
{
@@ -214,10 +221,12 @@ static void __init tegra20_init_timer(struct device_node *np)
BUG();
}
+#ifdef CONFIG_ARM
tegra_delay_timer.read_current_timer =
tegra_delay_timer_read_counter_long;
tegra_delay_timer.freq = 1000000;
register_current_timer_delay(&tegra_delay_timer);
+#endif
ret = setup_irq(tegra_timer_irq.irq, &tegra_timer_irq);
if (ret) {
@@ -232,6 +241,7 @@ static void __init tegra20_init_timer(struct device_node *np)
}
CLOCKSOURCE_OF_DECLARE(tegra20_timer, "nvidia,tegra20-timer", tegra20_init_timer);
+#ifdef CONFIG_ARM
static void __init tegra20_init_rtc(struct device_node *np)
{
struct clk *clk;
@@ -255,6 +265,7 @@ static void __init tegra20_init_rtc(struct device_node *np)
register_persistent_clock(NULL, tegra_read_persistent_clock);
}
CLOCKSOURCE_OF_DECLARE(tegra20_rtc, "nvidia,tegra20-rtc", tegra20_init_rtc);
+#endif /* CONFIG_ARM */
#ifdef CONFIG_PM
static u32 usec_config;
--
2.1.3
On Tue, Dec 09, 2014 at 10:07:18PM +0000, Paul Walmsley wrote:
>
> Like several of the other files in drivers/clocksource,
> tegra20_timer.c contains code that can only compile when CONFIG_ARM is
> enabled. This causes obvious problems when trying to compile this
> code for NVIDIA ARM64-based SoCs, such as Tegra132. The same timer IP
> blocks exist, so it seems appropriate to provide support for them.
>
> So until we figure out a better way to partition this code, wrap the
> delay_timer and persistent_clock support code with preprocessor tests
> for CONFIG_ARM. (The delay_timer code should not be needed at all on
> ARM64 due to the presence of the ARMv8 architected timer. The
> persistent_clock support code could become important once power
> management modes are implemented that turn off the CPU complex.)
>
> Signed-off-by: Paul Walmsley <[email protected]>
> Signed-off-by: Paul Walmsley <[email protected]>
> Cc: Allen Martin <[email protected]>
> Cc: Stephen Warren <[email protected]>
> Cc: Thierry Reding <[email protected]>
> Cc: Daniel Lezcano <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Alexandre Courbot <[email protected]>
> ---
> Applies against next-20141209.
> Intended for v3.20.
> Boot-tested on Tegra124 Jetson TK1 on next-20141209.
> Also boot-tested on Tegra132 Norrin FFD on next-20141209 + extra,
> unrelated patches.
>
> drivers/clocksource/tegra20_timer.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
You might want to read the following thread:
https://lkml.org/lkml/2014/11/7/605
I haven't gotten around to look at this in detail, but it seems like we
may not need to register the RTC early here at all. If that's really the
case we can just get rid of this hackery and do everything within the
RTC driver rather than duplicate some of that code here.
On 32-bit ARM, Tegra114 and Tegra124 also support the architected timer,
so the only remaining issue would be for Tegra20 and Tegra30 hardware.
But I also seem to remember that on most boards the Tegra RTC can't be
used properly because it doesn't have a battery. So while it may still
work while the board is powered it is not very useful as RTC. Or for
timekeeping across suspend/resume for that matter. The majority of
boards seem to have a PMIC with an integrated RTC and will use that
instead.
Otherwise this looks good to me to get things going on 64-bit ARM while
we iron out the other issues. Can I assume that you plan on sending in
patches that enable 64-bit ARM Tegra in the 3.20 timeframe? If so it
might be best to take this patch through the Tegra tree, provided that
Daniel and Thomas have no objections, to make sure we don't have
intermittent build breakage depending on the order in which the trees
get pulled into linux-next and Linus' tree. Even more so because there
are a couple of other, similar patches which makes a stable branches
oriented approach to resolving these dependencies somewhat impractical.
Thierry
+ lakml
Hello Thierry
On Wed, 10 Dec 2014, Thierry Reding wrote:
> On Tue, Dec 09, 2014 at 10:07:18PM +0000, Paul Walmsley wrote:
> >
> > Like several of the other files in drivers/clocksource,
> > tegra20_timer.c contains code that can only compile when CONFIG_ARM is
> > enabled. This causes obvious problems when trying to compile this
> > code for NVIDIA ARM64-based SoCs, such as Tegra132. The same timer IP
> > blocks exist, so it seems appropriate to provide support for them.
> >
> > So until we figure out a better way to partition this code, wrap the
> > delay_timer and persistent_clock support code with preprocessor tests
> > for CONFIG_ARM. (The delay_timer code should not be needed at all on
> > ARM64 due to the presence of the ARMv8 architected timer. The
> > persistent_clock support code could become important once power
> > management modes are implemented that turn off the CPU complex.)
> >
> > Signed-off-by: Paul Walmsley <[email protected]>
> > Signed-off-by: Paul Walmsley <[email protected]>
> > Cc: Allen Martin <[email protected]>
> > Cc: Stephen Warren <[email protected]>
> > Cc: Thierry Reding <[email protected]>
> > Cc: Daniel Lezcano <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: Alexandre Courbot <[email protected]>
> > ---
> > Applies against next-20141209.
> > Intended for v3.20.
> > Boot-tested on Tegra124 Jetson TK1 on next-20141209.
> > Also boot-tested on Tegra132 Norrin FFD on next-20141209 + extra,
> > unrelated patches.
> >
> > drivers/clocksource/tegra20_timer.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
>
> You might want to read the following thread:
>
> https://lkml.org/lkml/2014/11/7/605
Thanks for the link, I hadn't seen that discussion.
> I haven't gotten around to look at this in detail, but it seems like we
> may not need to register the RTC early here at all. If that's really the
> case we can just get rid of this hackery and do everything within the
> RTC driver rather than duplicate some of that code here.
I'd suggest that we keep that project separate from just getting this file
to compile on ARM64.
Broadly speaking, I agree that tegra20_timer.c needs to be cleaned up; it
should not mix code for two distinct IP blocks into one driver.
> On 32-bit ARM, Tegra114 and Tegra124 also support the architected timer,
> so the only remaining issue would be for Tegra20 and Tegra30 hardware.
I believe on most NVIDIA chips (and probably most ARM SoCs) the ARM
architected timer is only usable as a clockevent timer when the CPU
doesn't enter deep low power states. So the architected timer won't be
enough.
> But I also seem to remember that on most boards the Tegra RTC can't be
> used properly because it doesn't have a battery. So while it may still
> work while the board is powered it is not very useful as RTC. Or for
> timekeeping across suspend/resume for that matter. The majority of
> boards seem to have a PMIC with an integrated RTC and will use that
> instead.
>From my point of view, the Tegra RTC IP block's primary use case is to
provide an always-on clockevent to wake the system from SC7 (aka LP0
idle)[1]. In SC7, the ARM architected timer comparator logic and Tegra
TMR IP blocks will be power-gated, and thus can't wake the system.
PMIC RTC timers are generally a bad fit for this purpose, since they tend
to be low-resolution (+/- 1 second) and slow to program (via some slow
serial bus). They are good for keeping calendars, though...
> Otherwise this looks good to me to get things going on 64-bit ARM while
> we iron out the other issues. Can I assume that you plan on sending in
> patches that enable 64-bit ARM Tegra in the 3.20 timeframe? If so it
> might be best to take this patch through the Tegra tree, provided that
> Daniel and Thomas have no objections, to make sure we don't have
> intermittent build breakage depending on the order in which the trees
> get pulled into linux-next and Linus' tree. Even more so because there
> are a couple of other, similar patches which makes a stable branches
> oriented approach to resolving these dependencies somewhat impractical.
Given the rather small number of mainline T132 users, I'll probably just
send these patches up via their corresponding trees and let them merge via
their respective maintainers. That keeps the risk of merge conflicts low.
At this point I don't think there will be any significant dependency
problems.
- Paul
1. Section 11.0 "Real-time Clock" of the _NVIDIA Tegra K1 Mobile Processor
Technical Reference Manual v03p (DP-06905-001_v03p)_,
https://developer.nvidia.com/tegra-k1-technical-reference-manual