[PATCH] ARM: fix regression in RealView after the introduction of pclk

[PATCH] ARM: fix regression in RealView after the introduction of pclk

[Linus Walleij]

The patch to add the apb_pclk to the AMBA/PrimeCell bus broke
RealView, since the clockdevice is not registered at probe() time.
This moves clock initialization to interrupt initialization on
par with e.g ux500.

Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
---
 arch/arm/mach-realview/core.c            |    3 +--
 arch/arm/mach-realview/core.h            |    1 +
 arch/arm/mach-realview/realview_eb.c     |    2 ++
 arch/arm/mach-realview/realview_pb1176.c |    2 ++
 arch/arm/mach-realview/realview_pb11mp.c |    2 ++
 arch/arm/mach-realview/realview_pba8.c   |    2 ++
 arch/arm/mach-realview/realview_pbx.c    |    2 ++
 7 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-realview/core.c b/arch/arm/mach-realview/core.c
index d8179ea..154d10b 100644
--- a/arch/arm/mach-realview/core.c
+++ b/arch/arm/mach-realview/core.c
@@ -359,7 +359,7 @@ static struct clk_lookup lookups[] = {
 	}
 };
 
-static int __init clk_init(void)
+int __init realview_clk_init(void)
 {
 	if (machine_is_realview_pb1176())
 		oscvco_clk.vcoreg = __io_address(REALVIEW_SYS_BASE) + REALVIEW_SYS_OSC0_OFFSET;
@@ -370,7 +370,6 @@ static int __init clk_init(void)
 
 	return 0;
 }
-arch_initcall(clk_init);
 
 /*
  * CLCD support.
diff --git a/arch/arm/mach-realview/core.h b/arch/arm/mach-realview/core.h
index 781bca6..5536068 100644
--- a/arch/arm/mach-realview/core.h
+++ b/arch/arm/mach-realview/core.h
@@ -59,6 +59,7 @@ extern void __iomem *timer1_va_base;
 extern void __iomem *timer2_va_base;
 extern void __iomem *timer3_va_base;
 
+extern int realview_clk_init(void);
 extern void realview_leds_event(led_event_t ledevt);
 extern void realview_timer_init(unsigned int timer_irq);
 extern int realview_flash_register(struct resource *res, u32 num);
diff --git a/arch/arm/mach-realview/realview_eb.c b/arch/arm/mach-realview/realview_eb.c
index 991c1f8..200bec0 100644
--- a/arch/arm/mach-realview/realview_eb.c
+++ b/arch/arm/mach-realview/realview_eb.c
@@ -353,6 +353,8 @@ static struct platform_device char_lcd_device = {
 
 static void __init gic_init_irq(void)
 {
+	/* Initialize clocks early so they are available */
+	realview_clk_init();
 	if (core_tile_eb11mp() || core_tile_a9mp()) {
 		unsigned int pldctrl;
 
diff --git a/arch/arm/mach-realview/realview_pb1176.c b/arch/arm/mach-realview/realview_pb1176.c
index d2be12e..0ffc872 100644
--- a/arch/arm/mach-realview/realview_pb1176.c
+++ b/arch/arm/mach-realview/realview_pb1176.c
@@ -303,6 +303,8 @@ static struct platform_device char_lcd_device = {
 
 static void __init gic_init_irq(void)
 {
+	/* Initialize clocks early so they are available */
+	realview_clk_init();
 	/* ARM1176 DevChip GIC, primary */
 	gic_cpu_base_addr = __io_address(REALVIEW_DC1176_GIC_CPU_BASE);
 	gic_dist_init(0, __io_address(REALVIEW_DC1176_GIC_DIST_BASE), IRQ_DC1176_GIC_START);
diff --git a/arch/arm/mach-realview/realview_pb11mp.c b/arch/arm/mach-realview/realview_pb11mp.c
index d591bc0..a6d02e6 100644
--- a/arch/arm/mach-realview/realview_pb11mp.c
+++ b/arch/arm/mach-realview/realview_pb11mp.c
@@ -301,6 +301,8 @@ static void __init gic_init_irq(void)
 {
 	unsigned int pldctrl;
 
+	/* Initialize clocks early so they are available */
+	realview_clk_init();
 	/* new irq mode with no DCC */
 	writel(0x0000a05f, __io_address(REALVIEW_SYS_LOCK));
 	pldctrl = readl(__io_address(REALVIEW_SYS_BASE)	+ REALVIEW_PB11MP_SYS_PLD_CTRL1);
diff --git a/arch/arm/mach-realview/realview_pba8.c b/arch/arm/mach-realview/realview_pba8.c
index 6c37621..c226006 100644
--- a/arch/arm/mach-realview/realview_pba8.c
+++ b/arch/arm/mach-realview/realview_pba8.c
@@ -272,6 +272,8 @@ static struct platform_device pmu_device = {
 
 static void __init gic_init_irq(void)
 {
+	/* Initialize clocks early so they are available */
+	realview_clk_init();
 	/* ARM PB-A8 on-board GIC */
 	gic_cpu_base_addr = __io_address(REALVIEW_PBA8_GIC_CPU_BASE);
 	gic_dist_init(0, __io_address(REALVIEW_PBA8_GIC_DIST_BASE), IRQ_PBA8_GIC_START);
diff --git a/arch/arm/mach-realview/realview_pbx.c b/arch/arm/mach-realview/realview_pbx.c
index 9428eff..96bac42 100644
--- a/arch/arm/mach-realview/realview_pbx.c
+++ b/arch/arm/mach-realview/realview_pbx.c
@@ -311,6 +311,8 @@ static struct platform_device pmu_device = {
 
 static void __init gic_init_irq(void)
 {
+	/* Initialize clocks early so they are available */
+	realview_clk_init();
 	/* ARM PBX on-board GIC */
 	if (core_tile_pbx11mp() || core_tile_pbxa9mp()) {
 		gic_cpu_base_addr = __io_address(REALVIEW_PBX_TILE_GIC_CPU_BASE);
-- 
1.7.2

[PATCH] ARM: fix regression in RealView after the introduction of pclk

[Russell King - ARM Linux]

On Thu, Jul 29, 2010 at 03:05:22PM +0200, Linus Walleij wrote:
> diff --git a/arch/arm/mach-realview/core.c b/arch/arm/mach-realview/core.c
> index d8179ea..154d10b 100644
> --- a/arch/arm/mach-realview/core.c
> +++ b/arch/arm/mach-realview/core.c
> @@ -359,7 +359,7 @@ static struct clk_lookup lookups[] = {
>  	}
>  };
>  
> -static int __init clk_init(void)
> +int __init realview_clk_init(void)
>  {
>  	if (machine_is_realview_pb1176())
>  		oscvco_clk.vcoreg = __io_address(REALVIEW_SYS_BASE) + REALVIEW_SYS_OSC0_OFFSET;
> @@ -370,7 +370,6 @@ static int __init clk_init(void)
>  
>  	return 0;
>  }
> -arch_initcall(clk_init);

I think this can just be made core_initcall() rather than going to the
lengths that this patch does - all we need to do is ensure that the
clocks are registered before the machine init code is called, and
core_initcall() will do that for us.

[PATCH] ARM: fix regression in RealView after the introduction of pclk

[Linus Walleij]

2010/7/29 Russell King - ARM Linux <linux@arm.linux.org.uk>:

> I think this can just be made core_initcall() rather than going to the
> lengths that this patch does - all we need to do is ensure that the
> clocks are registered before the machine init code is called, and
> core_initcall() will do that for us.

Yep that is true, currently. More like this then:

From: Linus Walleij <linus.walleij@stericsson.com>
Date: Thu, 29 Jul 2010 23:04:20 +0200
Subject: [PATCH] ARM: fix regression in RealView after the
introduction of pclk v2

The patch to add the apb_pclk to the AMBA/PrimeCell bus broke
RealView, since the clockdevice is not registered at probe() time.
This moves clock initialization to a core_initcall()

Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
---
 arch/arm/mach-realview/core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-realview/core.c b/arch/arm/mach-realview/core.c
index d8179ea..a54fbda 100644
--- a/arch/arm/mach-realview/core.c
+++ b/arch/arm/mach-realview/core.c
@@ -370,7 +370,7 @@ static int __init clk_init(void)

        return 0;
 }
-arch_initcall(clk_init);
+core_initcall(clk_init);

 /*
  * CLCD support.
-- 
1.7.2

There is a reason why we have it on the interrupt init in ux500
though, and that is that we want to use the clock framework to
get the clock for the timer. If we want to fix up the hardcoded
way it is currently done in plat-versatile/timer-sp.c we would need
a change like this across the versatiles.

But I'll make a separate patch doing all that across the
versatiles so it get put in context, this fix is  fine in the meantime.

Yours,
Linus Walleij

[PATCH] ARM: fix regression in RealView after the introduction of pclk

[Russell King - ARM Linux]

On Thu, Jul 29, 2010 at 11:41:44PM +0200, Linus Walleij wrote:
> Yep that is true, currently. More like this then:

Yup - and the patch is a lot smaller.

> There is a reason why we have it on the interrupt init in ux500
> though, and that is that we want to use the clock framework to
> get the clock for the timer. If we want to fix up the hardcoded
> way it is currently done in plat-versatile/timer-sp.c we would need
> a change like this across the versatiles.

I don't think it makes sense to use the clk API for those timers -
it's rather unclear from the manuals where they get their clocks
from - or even what the magic bits in the system controller do in
terms of switching their clock source.

I've not found the magic bits which switch their clock source on
Versatile Express tiles yet either (which is why we're using the
V2M baseboard timers).

[PATCH] ARM: fix regression in RealView after the introduction of pclk

[Linus Walleij]

2010/7/30 Russell King - ARM Linux <linux@arm.linux.org.uk>:

> On Thu, Jul 29, 2010 at 11:41:44PM +0200, Linus Walleij wrote:
>> Yep that is true, currently. More like this then:
>
> Yup - and the patch is a lot smaller.

OK putting it in the tracker.

>> There is a reason why we have it on the interrupt init in ux500
>> though, and that is that we want to use the clock framework to
>> get the clock for the timer. If we want to fix up the hardcoded
>> way it is currently done in plat-versatile/timer-sp.c we would need
>> a change like this across the versatiles.
>
> I don't think it makes sense to use the clk API for those timers -
> it's rather unclear from the manuals where they get their clocks
> from - or even what the magic bits in the system controller do in
> terms of switching their clock source.

In the PB1176 manual there is a drawing of the clock tree, it
shows that the fixed 24MHz reference clock is divided by a binary
divider down to 1 MHz. So this one is likewise set in stone. I would
be surprised if some other scheme was used for the other boards.

I have a partial patch actually bringing these clocks into the
clkdev framwork but if it makes things complicated for no good
let's just skip it.

Thinking about it I however see that since some platforms are
initializing their clocks in .init_irq and some in core_initcall(), some
in arch_initcall() etc, and since most platforms have some clock
implementation these dats, it might be worth it to add an
optional .init_clocks() call to the struct machine_desc and
MACHINE_START() macro to get this in uniform and call
that even before .init_irq(). Do you think it's worth it?

Yours,
Linus Walleij

[PATCH] ARM: fix regression in RealView after the introduction of pclk

[Russell King - ARM Linux]

On Fri, Jul 30, 2010 at 05:27:28PM +0200, Linus Walleij wrote:
> In the PB1176 manual there is a drawing of the clock tree, it
> shows that the fixed 24MHz reference clock is divided by a binary
> divider down to 1 MHz. So this one is likewise set in stone. I would
> be surprised if some other scheme was used for the other boards.

That's not quite the full story, because there's this in the code:

        /*
         * set clock frequency:
         *      REALVIEW_REFCLK is 32KHz
         *      REALVIEW_TIMCLK is 1MHz
         */
        val = readl(__io_address(REALVIEW_SCTL_BASE));
        writel((REALVIEW_TIMCLK << REALVIEW_TIMER1_EnSel) |
               (REALVIEW_TIMCLK << REALVIEW_TIMER2_EnSel) |
               (REALVIEW_TIMCLK << REALVIEW_TIMER3_EnSel) |
               (REALVIEW_TIMCLK << REALVIEW_TIMER4_EnSel) | val,
               __io_address(REALVIEW_SCTL_BASE));

which isn't documented in the user manual.  Yet it's necessary in order
to run the timers@1MHz.

> Thinking about it I however see that since some platforms are
> initializing their clocks in .init_irq and some in core_initcall(), some
> in arch_initcall() etc, and since most platforms have some clock
> implementation these dats, it might be worth it to add an
> optional .init_clocks() call to the struct machine_desc and
> MACHINE_START() macro to get this in uniform and call
> that even before .init_irq(). Do you think it's worth it?

It may be, but it's rather a large change to do.  We can certainly
provide the .init_clocks callback so new stuff can use it.

[PATCH] ARM: fix regression in RealView after the introduction of pclk

[Linus Walleij]

2010/7/30 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Fri, Jul 30, 2010 at 05:27:28PM +0200, Linus Walleij wrote:
>> In the PB1176 manual there is a drawing of the clock tree, it
>> shows that the fixed 24MHz reference clock is divided by a binary
>> divider down to 1 MHz. So this one is likewise set in stone. I would
>> be surprised if some other scheme was used for the other boards.
>
> That's not quite the full story, because there's this in the code:
>
> ? ? ? ?/*
> ? ? ? ? * set clock frequency:
> ? ? ? ? * ? ? ?REALVIEW_REFCLK is 32KHz
> ? ? ? ? * ? ? ?REALVIEW_TIMCLK is 1MHz
> ? ? ? ? */
> ? ? ? ?val = readl(__io_address(REALVIEW_SCTL_BASE));
> ? ? ? ?writel((REALVIEW_TIMCLK << REALVIEW_TIMER1_EnSel) |
> ? ? ? ? ? ? ? (REALVIEW_TIMCLK << REALVIEW_TIMER2_EnSel) |
> ? ? ? ? ? ? ? (REALVIEW_TIMCLK << REALVIEW_TIMER3_EnSel) |
> ? ? ? ? ? ? ? (REALVIEW_TIMCLK << REALVIEW_TIMER4_EnSel) | val,
> ? ? ? ? ? ? ? __io_address(REALVIEW_SCTL_BASE));
>
> which isn't documented in the user manual. ?Yet it's necessary in order
> to run the timers at 1MHz.

Yeah that thing is fun, it looks like a plain mux. Setting the bits
to zero muxes in the 32768 Hz clock and setting one muxes in
the clock from the 24 Mhz / 24 divider. (Easy enough to verify,
I'll test it some day when I'm bored.)

>> Thinking about it I however see that since some platforms are
>> initializing their clocks in .init_irq and some in core_initcall(), some
>> in arch_initcall() etc, and since most platforms have some clock
>> implementation these dats, it might be worth it to add an
>> optional .init_clocks() call to the struct machine_desc and
>> MACHINE_START() macro to get this in uniform and call
>> that even before .init_irq(). Do you think it's worth it?
>
> It may be, but it's rather a large change to do. ?We can certainly
> provide the .init_clocks callback so new stuff can use it.

Yeah I meant it to be voluntary stuff, I'll see what I can do.

Yours,
Linus Walleij