[PATCH] Introduce mips_late_time_init

[PATCH] Introduce mips_late_time_init

[Anoop P A]

This patch moves plat_time_init and clocksoure init funtion calls to
late_time_init. 

Signed-off-by: Anoop P A <anoop.pa@gmail.com>
---
diff --git a/arch/mips/kernel/time.c b/arch/mips/kernel/time.c
index fb74974..dbd1ac5 100644
--- a/arch/mips/kernel/time.c
+++ b/arch/mips/kernel/time.c
@@ -117,10 +117,16 @@ static __init int cpu_has_mfc0_count_bug(void)
 	return 0;
 }
 
-void __init time_init(void)
+void __init mips_late_time_init(void)
 {
 	plat_time_init();
 
 	if (!mips_clockevent_init() || !cpu_has_mfc0_count_bug())
 		init_mips_clocksource();
 }
+
+
+void __init time_init(void)
+{
+	late_time_init = mips_late_time_init;
+}

Re: [PATCH] Introduce mips_late_time_init

[David Daney]

On 12/06/2010 12:23 AM, Anoop P A wrote:
> This patch moves plat_time_init and clocksoure init funtion calls to
> late_time_init.
>

Why would you want to do this?

The current code works perfectly, so I see no reason to change it.

David Daney


> Signed-off-by: Anoop P A<anoop.pa@gmail.com>
> ---
> diff --git a/arch/mips/kernel/time.c b/arch/mips/kernel/time.c
> index fb74974..dbd1ac5 100644
> --- a/arch/mips/kernel/time.c
> +++ b/arch/mips/kernel/time.c
> @@ -117,10 +117,16 @@ static __init int cpu_has_mfc0_count_bug(void)
>   	return 0;
>   }
>
> -void __init time_init(void)
> +void __init mips_late_time_init(void)
>   {
>   	plat_time_init();
>
>   	if (!mips_clockevent_init() || !cpu_has_mfc0_count_bug())
>   		init_mips_clocksource();
>   }
> +
> +
> +void __init time_init(void)
> +{
> +	late_time_init = mips_late_time_init;
> +}
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCH] Introduce mips_late_time_init

[Ralf Baechle]

On Mon, Dec 06, 2010 at 09:42:45AM -0800, David Daney wrote:

> On 12/06/2010 12:23 AM, Anoop P A wrote:
> >This patch moves plat_time_init and clocksoure init funtion calls to
> >late_time_init.
> >
> 
> Why would you want to do this?
> 
> The current code works perfectly, so I see no reason to change it.

Well, not really.  By the time time_init is called kmalloc isn't ready yet.
That's why mips_clockevent_device pretty much had to be statically
allocated and is also why interrupts have to use setup_irq instead of
request_irq.

Keeping mips_clockevent_device statically allocated as per-CPU makes sense.
Less for the struct irqaction and he'll have to allocate one for each
VPE (think CPU) he installs a clockevent device on.

Running everything from late_time_init() instead allows the use of kmalloc.
X86 has the same issue with requiring kmalloc in time_init which is why
they had moved everything to late_time_init.

So the real question is, why can't we just move the call of time_init()
in setup_kernel() to where late_time_init() is getting called from for
all architectures, does anything rely on it getting called early?

  Ralf

Re: [PATCH] Introduce mips_late_time_init

[Thomas Gleixner]

On Wed, 8 Dec 2010, Ralf Baechle wrote:
> Running everything from late_time_init() instead allows the use of kmalloc.
> X86 has the same issue with requiring kmalloc in time_init which is why
> they had moved everything to late_time_init.

It's more ioremap, but yeah.
 
> So the real question is, why can't we just move the call of time_init()
> in setup_kernel() to where late_time_init() is getting called from for
> all architectures, does anything rely on it getting called early?

That's a good question and I asked it myself already. I can't see a
real reason why something would need it early. Definitely worth to
try.

Thanks,

	tglx

Re: [PATCH] Introduce mips_late_time_init

[Benjamin Herrenschmidt]

On Wed, 2010-12-08 at 22:21 +0100, Thomas Gleixner wrote:
> On Wed, 8 Dec 2010, Ralf Baechle wrote:
> > Running everything from late_time_init() instead allows the use of kmalloc.
> > X86 has the same issue with requiring kmalloc in time_init which is why
> > they had moved everything to late_time_init.
> 
> It's more ioremap, but yeah.
>  
> > So the real question is, why can't we just move the call of time_init()
> > in setup_kernel() to where late_time_init() is getting called from for
> > all architectures, does anything rely on it getting called early?
> 
> That's a good question and I asked it myself already. I can't see a
> real reason why something would need it early. Definitely worth to
> try.

Well, I can see some reasons at least...

On ppc at least, we calibrate the timebase/decrementer in time_init, so
things like udelay etc... are going to be unreliable until we've done
that, which could be a problem if done too late due to sensitive HW
accessors that might rely on these.

So we'd probably need to move that to a different (early) arch callback
if time_init is moved.

Also, still on server PPC, you can't really disable the decrementer
(only delay it). So if interrupts are enabled, we will eventually get
timer ones.

So we'd have to be careful about keeping some state, knowing that the
stuff isn't initialized yet and just set the decrementer to fire again
as late as possible, until it's properly configured.

Besides, we can use kmalloc that early nowadays, can't we ? That's what
the gfp_allowed_mask is all about ...

Cheers,
Ben.

Re: [PATCH] Introduce mips_late_time_init

[Paul Mundt]

On Thu, Dec 09, 2010 at 09:45:46AM +1100, Benjamin Herrenschmidt wrote:
> On Wed, 2010-12-08 at 22:21 +0100, Thomas Gleixner wrote:
> > On Wed, 8 Dec 2010, Ralf Baechle wrote:
> > > Running everything from late_time_init() instead allows the use of kmalloc.
> > > X86 has the same issue with requiring kmalloc in time_init which is why
> > > they had moved everything to late_time_init.
> > 
> > It's more ioremap, but yeah.
> >  
> > > So the real question is, why can't we just move the call of time_init()
> > > in setup_kernel() to where late_time_init() is getting called from for
> > > all architectures, does anything rely on it getting called early?
> > 
> > That's a good question and I asked it myself already. I can't see a
> > real reason why something would need it early. Definitely worth to
> > try.
> 
> Well, I can see some reasons at least...
> 
> On ppc at least, we calibrate the timebase/decrementer in time_init, so
> things like udelay etc... are going to be unreliable until we've done
> that, which could be a problem if done too late due to sensitive HW
> accessors that might rely on these.
> 
The SH case is similar. We bring up the clock framework during
time_init() which we need for per-CPU loops_per_jiffy calculation in
addition to initializing the clocksource/clockevent timers that come up
during late_time_init.

Presently we have a slab_is_available() check in the SH clkdev
implementation mostly for board PLLs and so on, and our ioremap()
implementation also transparently bolts on to fixed ioremaps through
fixmap if we're really early anyways. It's possible that kmalloc is
usable outright now after the reordering happened, I haven't gone back
and checked it again since then.

> So we'd probably need to move that to a different (early) arch callback
> if time_init is moved.
> 
late_time_init has always struck me as a bit of a misnomer, since it's
still quite early as far as the rest of the system state is concerned,
particularly the driver core (all SH and ARM SH-Mobile platforms use
platform devices for clocksources/clockevents). In any event, SH would
also need this.

On ARM SH-Mobile we use the same approach although each platform
explicitly calls in to clock framework initialization prior to assigning
a late_time_init, which in turn gets all wrapped up in the ARM sys_timer
abstraction.