[PATCH] ARM: local timers: Allow boot CPU to have its timer running early

[PATCH] ARM: local timers: Allow boot CPU to have its timer running early

[Marc Zyngier]

Currently, the boot CPU has its local timer enabled long after
the delay loop has been calibrated. This makes it impossible to
boot a system that only uses local timers, like the A15.

Use late_time_init hook to initialize the boot CPU local timer.
Since shmobile is already using this hook, add an ARM specific
arm_late_time_init hook that platforms can use instead.

Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/mach/time.h |    1 +
 arch/arm/kernel/smp.c            |    6 ------
 arch/arm/kernel/time.c           |   14 ++++++++++++++
 arch/arm/mach-shmobile/timer.c   |    2 +-
 4 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/arch/arm/include/asm/mach/time.h b/arch/arm/include/asm/mach/time.h
index d5adaae..f46ca39 100644
--- a/arch/arm/include/asm/mach/time.h
+++ b/arch/arm/include/asm/mach/time.h
@@ -43,5 +43,6 @@ struct sys_timer {
 };
 
 extern void timer_tick(void);
+extern void (* __initdata arm_late_time_init)(void);
 
 #endif
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 344e52b..70badae 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -364,12 +364,6 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 
 	if (max_cpus > 1) {
 		/*
-		 * Enable the local timer or broadcast device for the
-		 * boot CPU, but only if we have more than one CPU.
-		 */
-		percpu_timer_setup();
-
-		/*
 		 * Initialise the SCU if there are more than one CPU
 		 * and let them know where to start.
 		 */
diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c
index cb634c3..ba85044 100644
--- a/arch/arm/kernel/time.c
+++ b/arch/arm/kernel/time.c
@@ -28,6 +28,7 @@
 #include <linux/mc146818rtc.h>
 
 #include <asm/leds.h>
+#include <asm/localtimer.h>
 #include <asm/thread_info.h>
 #include <asm/sched_clock.h>
 #include <asm/stacktrace.h>
@@ -147,6 +148,18 @@ static int __init timer_init_syscore_ops(void)
 
 device_initcall(timer_init_syscore_ops);
 
+void (* __initdata arm_late_time_init)(void);
+
+static void __init __arm_late_time_init(void)
+{
+	if (arm_late_time_init)
+		arm_late_time_init();
+#ifdef CONFIG_LOCAL_TIMERS
+	/* Init the local timer on the boot CPU */
+	percpu_timer_setup();
+#endif
+}
+
 void __init time_init(void)
 {
 	system_timer = machine_desc->timer;
@@ -154,5 +167,6 @@ void __init time_init(void)
 #ifdef CONFIG_HAVE_SCHED_CLOCK
 	sched_clock_postinit();
 #endif
+	late_time_init = __arm_late_time_init;
 }
 
diff --git a/arch/arm/mach-shmobile/timer.c b/arch/arm/mach-shmobile/timer.c
index 895794b..835baa4 100644
--- a/arch/arm/mach-shmobile/timer.c
+++ b/arch/arm/mach-shmobile/timer.c
@@ -38,7 +38,7 @@ static void __init shmobile_late_time_init(void)
 
 static void __init shmobile_timer_init(void)
 {
-	late_time_init = shmobile_late_time_init;
+	arm_late_time_init = shmobile_late_time_init;
 }
 
 struct sys_timer shmobile_timer = {
-- 
1.7.0.4

[PATCH] ARM: local timers: Allow boot CPU to have its timer running early

[Paul Mundt]

On Wed, Jun 01, 2011 at 03:03:26PM +0100, Marc Zyngier wrote:
> Currently, the boot CPU has its local timer enabled long after
> the delay loop has been calibrated. This makes it impossible to
> boot a system that only uses local timers, like the A15.
> 
> Use late_time_init hook to initialize the boot CPU local timer.
> Since shmobile is already using this hook, add an ARM specific
> arm_late_time_init hook that platforms can use instead.
> 
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

Fine with me.

Acked-by: Paul Mundt <lethal@linux-sh.org>

[PATCH] ARM: local timers: Allow boot CPU to have its timer running early

[Jean-Christophe PLAGNIOL-VILLARD]

On 15:03 Wed 01 Jun     , Marc Zyngier wrote:
> Currently, the boot CPU has its local timer enabled long after
> the delay loop has been calibrated. This makes it impossible to
> boot a system that only uses local timers, like the A15.
> 
> Use late_time_init hook to initialize the boot CPU local timer.
> Since shmobile is already using this hook, add an ARM specific
> arm_late_time_init hook that platforms can use instead.
> 
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
I propose to switch to early platform devce and earlytimer

this will avoid the arm_late_time_init hook

and will make it cross arch

Best Regards,
J.

[PATCH] ARM: local timers: Allow boot CPU to have its timer running early

[Marc Zyngier]

On Thu, 2011-06-02 at 18:31 +0200, Jean-Christophe PLAGNIOL-VILLARD
wrote:
> On 15:03 Wed 01 Jun     , Marc Zyngier wrote:
> > Currently, the boot CPU has its local timer enabled long after
> > the delay loop has been calibrated. This makes it impossible to
> > boot a system that only uses local timers, like the A15.
> > 
> > Use late_time_init hook to initialize the boot CPU local timer.
> > Since shmobile is already using this hook, add an ARM specific
> > arm_late_time_init hook that platforms can use instead.
> > 
> > Cc: Paul Mundt <lethal@linux-sh.org>
> > Cc: Magnus Damm <magnus.damm@gmail.com>
> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> I propose to switch to early platform devce and earlytimer
> 
> this will avoid the arm_late_time_init hook
> 
> and will make it cross arch

I believe this is orthogonal. shmobile (the only ARM user of
late_time_init) is already doing some early_platform stuff for its
timers.

What I'm trying to achieve here is to make sure the timer on CPU0 is
actually up, running and registered as a clock_event_device before we
hit the delay loop.

Or maybe I've misunderstood what you're pointing me to?

Cheers,

	M.
-- 
Reality is an implementation detail.

[PATCH] ARM: local timers: Allow boot CPU to have its timer running early

[Rob Herring]

On 06/02/2011 11:56 AM, Marc Zyngier wrote:
> On Thu, 2011-06-02 at 18:31 +0200, Jean-Christophe PLAGNIOL-VILLARD
> wrote:
>> On 15:03 Wed 01 Jun     , Marc Zyngier wrote:
>>> Currently, the boot CPU has its local timer enabled long after
>>> the delay loop has been calibrated. This makes it impossible to
>>> boot a system that only uses local timers, like the A15.
>>>
>>> Use late_time_init hook to initialize the boot CPU local timer.
>>> Since shmobile is already using this hook, add an ARM specific
>>> arm_late_time_init hook that platforms can use instead.
>>>
>>> Cc: Paul Mundt<lethal@linux-sh.org>
>>> Cc: Magnus Damm<magnus.damm@gmail.com>
>>> Signed-off-by: Marc Zyngier<marc.zyngier@arm.com>
>> I propose to switch to early platform devce and earlytimer
>>
>> this will avoid the arm_late_time_init hook
>>
>> and will make it cross arch
>
> I believe this is orthogonal. shmobile (the only ARM user of
> late_time_init) is already doing some early_platform stuff for its
> timers.
>
> What I'm trying to achieve here is to make sure the timer on CPU0 is
> actually up, running and registered as a clock_event_device before we
> hit the delay loop.
>
> Or maybe I've misunderstood what you're pointing me to?
>

I believe he is referring to this patch which generically enables the 
shmobile code for ARM:

http://www.spinics.net/lists/arm-kernel/msg123736.html

I don't think it has been pulled into mainline yet.

Rob

[PATCH] ARM: local timers: Allow boot CPU to have its timer running early

[Marc Zyngier]

On Thu, 2011-06-02 at 14:38 -0500, Rob Herring wrote:
> On 06/02/2011 11:56 AM, Marc Zyngier wrote:
> > On Thu, 2011-06-02 at 18:31 +0200, Jean-Christophe PLAGNIOL-VILLARD
> > wrote:
> >> On 15:03 Wed 01 Jun     , Marc Zyngier wrote:
> >>> Currently, the boot CPU has its local timer enabled long after
> >>> the delay loop has been calibrated. This makes it impossible to
> >>> boot a system that only uses local timers, like the A15.
> >>>
> >>> Use late_time_init hook to initialize the boot CPU local timer.
> >>> Since shmobile is already using this hook, add an ARM specific
> >>> arm_late_time_init hook that platforms can use instead.
> >>>
> >>> Cc: Paul Mundt<lethal@linux-sh.org>
> >>> Cc: Magnus Damm<magnus.damm@gmail.com>
> >>> Signed-off-by: Marc Zyngier<marc.zyngier@arm.com>
> >> I propose to switch to early platform devce and earlytimer
> >>
> >> this will avoid the arm_late_time_init hook
> >>
> >> and will make it cross arch
> >
> > I believe this is orthogonal. shmobile (the only ARM user of
> > late_time_init) is already doing some early_platform stuff for its
> > timers.
> >
> > What I'm trying to achieve here is to make sure the timer on CPU0 is
> > actually up, running and registered as a clock_event_device before we
> > hit the delay loop.
> >
> > Or maybe I've misunderstood what you're pointing me to?
> >
> 
> I believe he is referring to this patch which generically enables the 
> shmobile code for ARM:
> 
> http://www.spinics.net/lists/arm-kernel/msg123736.html

Ah, I see. With that patch, I can indeed loose the additional hook.

> I don't think it has been pulled into mainline yet.

I'll rebase my patch on top of this one if there's a consensus around
it.

Thanks,

	M.
-- 
Reality is an implementation detail.

[PATCH] ARM: local timers: Allow boot CPU to have its timer running early

[Russell King - ARM Linux]

On Thu, Jun 02, 2011 at 02:38:23PM -0500, Rob Herring wrote:
> On 06/02/2011 11:56 AM, Marc Zyngier wrote:
>> On Thu, 2011-06-02 at 18:31 +0200, Jean-Christophe PLAGNIOL-VILLARD
>> wrote:
>>> On 15:03 Wed 01 Jun     , Marc Zyngier wrote:
>>>> Currently, the boot CPU has its local timer enabled long after
>>>> the delay loop has been calibrated. This makes it impossible to
>>>> boot a system that only uses local timers, like the A15.
>>>>
>>>> Use late_time_init hook to initialize the boot CPU local timer.
>>>> Since shmobile is already using this hook, add an ARM specific
>>>> arm_late_time_init hook that platforms can use instead.
>>>>
>>>> Cc: Paul Mundt<lethal@linux-sh.org>
>>>> Cc: Magnus Damm<magnus.damm@gmail.com>
>>>> Signed-off-by: Marc Zyngier<marc.zyngier@arm.com>
>>> I propose to switch to early platform devce and earlytimer
>>>
>>> this will avoid the arm_late_time_init hook
>>>
>>> and will make it cross arch
>>
>> I believe this is orthogonal. shmobile (the only ARM user of
>> late_time_init) is already doing some early_platform stuff for its
>> timers.
>>
>> What I'm trying to achieve here is to make sure the timer on CPU0 is
>> actually up, running and registered as a clock_event_device before we
>> hit the delay loop.
>>
>> Or maybe I've misunderstood what you're pointing me to?
>>
>
> I believe he is referring to this patch which generically enables the  
> shmobile code for ARM:
>
> http://www.spinics.net/lists/arm-kernel/msg123736.html
>
> I don't think it has been pulled into mainline yet.

And I really don't like it:
1. It adds an additional layer of complexity.
2. We end up needing more ways to initialize stuff even earlier in the
   kernel boot sequence.
3. Tracking down what gets called when becomes a lot harder.

At one time there used to be a good rule to follow: Keep it Simple, Damnit.
This doesn't follow that.

And so far, no one has explained _why_ shmobile uses this stuff... so
my presumption at the moment is that there's no real good reason.

[PATCH] ARM: local timers: Allow boot CPU to have its timer running early

[Russell King - ARM Linux]

On Wed, Jun 01, 2011 at 03:03:26PM +0100, Marc Zyngier wrote:
> Currently, the boot CPU has its local timer enabled long after
> the delay loop has been calibrated. This makes it impossible to
> boot a system that only uses local timers, like the A15.
> 
> Use late_time_init hook to initialize the boot CPU local timer.
> Since shmobile is already using this hook, add an ARM specific
> arm_late_time_init hook that platforms can use instead.

Why do we need to initialize the per-cpu timer in late_time_init() ?
Is there a reason it doesn't work at time_init(), and if not can it
be fixed to work there?

[PATCH] ARM: local timers: Allow boot CPU to have its timer running early

[Marc Zyngier]

On Fri, 2011-06-03 at 09:57 +0100, Russell King - ARM Linux wrote:
> On Wed, Jun 01, 2011 at 03:03:26PM +0100, Marc Zyngier wrote:
> > Currently, the boot CPU has its local timer enabled long after
> > the delay loop has been calibrated. This makes it impossible to
> > boot a system that only uses local timers, like the A15.
> > 
> > Use late_time_init hook to initialize the boot CPU local timer.
> > Since shmobile is already using this hook, add an ARM specific
> > arm_late_time_init hook that platforms can use instead.
> 
> Why do we need to initialize the per-cpu timer in late_time_init() ?
> Is there a reason it doesn't work at time_init(), and if not can it
> be fixed to work there?

It's an ugly mess: TWD needs to be calibrated, and this requires that
jiffies are progressing. For that, you need another timer to be already
registered (fine, we have that on all TWD platforms). But time_init() is
run with interrupts disabled, and you'd hang calibrating TWD.

This can be solved by merging Colin's scalable TWD, and make sure all
platforms provide a "smp_twd" clock. Then we can get rid of the
calibration, and time_init() becomes the natural spot for
percpu_timer_setup().

	M.
-- 
Reality is an implementation detail.

[PATCH] ARM: local timers: Allow boot CPU to have its timer running early

[Rob Herring]

On 06/03/2011 03:55 AM, Russell King - ARM Linux wrote:
> On Thu, Jun 02, 2011 at 02:38:23PM -0500, Rob Herring wrote:
>> On 06/02/2011 11:56 AM, Marc Zyngier wrote:
>>> On Thu, 2011-06-02 at 18:31 +0200, Jean-Christophe PLAGNIOL-VILLARD
>>> wrote:
>>>> On 15:03 Wed 01 Jun     , Marc Zyngier wrote:
>>>>> Currently, the boot CPU has its local timer enabled long after
>>>>> the delay loop has been calibrated. This makes it impossible to
>>>>> boot a system that only uses local timers, like the A15.
>>>>>
>>>>> Use late_time_init hook to initialize the boot CPU local timer.
>>>>> Since shmobile is already using this hook, add an ARM specific
>>>>> arm_late_time_init hook that platforms can use instead.
>>>>>
>>>>> Cc: Paul Mundt<lethal@linux-sh.org>
>>>>> Cc: Magnus Damm<magnus.damm@gmail.com>
>>>>> Signed-off-by: Marc Zyngier<marc.zyngier@arm.com>
>>>> I propose to switch to early platform devce and earlytimer
>>>>
>>>> this will avoid the arm_late_time_init hook
>>>>
>>>> and will make it cross arch
>>>
>>> I believe this is orthogonal. shmobile (the only ARM user of
>>> late_time_init) is already doing some early_platform stuff for its
>>> timers.
>>>
>>> What I'm trying to achieve here is to make sure the timer on CPU0 is
>>> actually up, running and registered as a clock_event_device before we
>>> hit the delay loop.
>>>
>>> Or maybe I've misunderstood what you're pointing me to?
>>>
>>
>> I believe he is referring to this patch which generically enables the
>> shmobile code for ARM:
>>
>> http://www.spinics.net/lists/arm-kernel/msg123736.html
>>
>> I don't think it has been pulled into mainline yet.
>
> And I really don't like it:
> 1. It adds an additional layer of complexity.
> 2. We end up needing more ways to initialize stuff even earlier in the
>     kernel boot sequence.
> 3. Tracking down what gets called when becomes a lot harder.
>
> At one time there used to be a good rule to follow: Keep it Simple, Damnit.
> This doesn't follow that.
>
> And so far, no one has explained _why_ shmobile uses this stuff... so
> my presumption at the moment is that there's no real good reason.

I believe the primary reason was to align sh and shmobile platforms' 
timer code.

In addition, other reasons I see are eliminate timer init in every 
single platform and move timer code out of arch/arm to 
drivers/clocksource. Getting the per platform timer setup to just be 
data enables putting that information in devicetree.

Rob

[PATCH] ARM: local timers: Allow boot CPU to have its timer running early

[Magnus Damm]

Hi Russell,

On Fri, Jun 3, 2011 at 5:55 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Jun 02, 2011 at 02:38:23PM -0500, Rob Herring wrote:
>> On 06/02/2011 11:56 AM, Marc Zyngier wrote:
>>> On Thu, 2011-06-02 at 18:31 +0200, Jean-Christophe PLAGNIOL-VILLARD
>>> wrote:
>>>> On 15:03 Wed 01 Jun ? ? , Marc Zyngier wrote:
>>>>> Currently, the boot CPU has its local timer enabled long after
>>>>> the delay loop has been calibrated. This makes it impossible to
>>>>> boot a system that only uses local timers, like the A15.
>>>>>
>>>>> Use late_time_init hook to initialize the boot CPU local timer.
>>>>> Since shmobile is already using this hook, add an ARM specific
>>>>> arm_late_time_init hook that platforms can use instead.
>>>>>
>>>>> Cc: Paul Mundt<lethal@linux-sh.org>
>>>>> Cc: Magnus Damm<magnus.damm@gmail.com>
>>>>> Signed-off-by: Marc Zyngier<marc.zyngier@arm.com>
>>>> I propose to switch to early platform devce and earlytimer
>>>>
>>>> this will avoid the arm_late_time_init hook
>>>>
>>>> and will make it cross arch
>>>
>>> I believe this is orthogonal. shmobile (the only ARM user of
>>> late_time_init) is already doing some early_platform stuff for its
>>> timers.
>>>
>>> What I'm trying to achieve here is to make sure the timer on CPU0 is
>>> actually up, running and registered as a clock_event_device before we
>>> hit the delay loop.
>>>
>>> Or maybe I've misunderstood what you're pointing me to?
>>>
>>
>> I believe he is referring to this patch which generically enables the
>> shmobile code for ARM:
>>
>> http://www.spinics.net/lists/arm-kernel/msg123736.html
>>
>> I don't think it has been pulled into mainline yet.
>
> And I really don't like it:
> 1. It adds an additional layer of complexity.
> 2. We end up needing more ways to initialize stuff even earlier in the
> ? kernel boot sequence.
> 3. Tracking down what gets called when becomes a lot harder.
>
> At one time there used to be a good rule to follow: Keep it Simple, Damnit.
> This doesn't follow that.

I'm a big fan of KISS too, and I do agree the early platform code adds
a certain level of complexity. That aside, I believe the benefits of
early platform drivers justify the added complexity.

> And so far, no one has explained _why_ shmobile uses this stuff... so
> my presumption at the moment is that there's no real good reason.

The reason behind the early platform driver code is to reuse part of
the driver model early during boot. The normal driver model allows
platform devices and drivers to split device configuration and the
actual device driver that operates on the device configuration. This
is for instance very useful on SoCs where you have multiple instances
of hardware blocks and you'd like to allow the user to select instance
via platform data and/or kernel command line.

On the sh architecture and on shmobile ARM we use early platform
drivers for timers and for early console support. In practice this
means we allow the kernel to associate device data with a certain
driver before the regular driver model is fully initialized. Early
code is by definition rather fragile, but one nice outcome of all this
is that the serial driver now allows the user to select serial port
instance on the kernel command line. This while the I/O base and IRQ
configuration is kept with per-SoC code and the serial driver is a
slightly enhanced normal platform driver.

There may of course be better ways to achieve what we want, and I am
open to alternative solutions. But so far I have not seen any other
code that allows us to separate device configuration from driver code
early on during boot.

For more information, please have a look at:

commit 13977091a988fb0d21821c2221ddc920eba36b79
Author: Magnus Damm <damm@igel.co.jp>
Date:   Mon Mar 30 14:37:25 2009 -0700

    Driver Core: early platform driver

Cheers,

/ magnus

[PATCH] ARM: local timers: Allow boot CPU to have its timer running early

[Paul Mundt]

On Fri, Jun 03, 2011 at 09:57:35AM +0100, Russell King - ARM Linux wrote:
> On Wed, Jun 01, 2011 at 03:03:26PM +0100, Marc Zyngier wrote:
> > Currently, the boot CPU has its local timer enabled long after
> > the delay loop has been calibrated. This makes it impossible to
> > boot a system that only uses local timers, like the A15.
> > 
> > Use late_time_init hook to initialize the boot CPU local timer.
> > Since shmobile is already using this hook, add an ARM specific
> > arm_late_time_init hook that platforms can use instead.
> 
> Why do we need to initialize the per-cpu timer in late_time_init() ?
> Is there a reason it doesn't work at time_init(), and if not can it
> be fixed to work there?

There are a number of ordering issues. It's necessary to have the clock
framework and everything else initialized before the per-cpu timer can
come up. While we did originally have this at the end of the time_init()
path there were some interactivity issues with regards to lockdep and
having IRQs initialized too early, which was the principle rationale for
deferring it.