| Message ID | 1306937006-26597-1-git-send-email-marc.zyngier@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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>
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.
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.
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
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.
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.
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?
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.
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
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
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.
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 = {
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(-)