ARM: OMAP: Make OMAP clocksource source selection runtime

Add missing idle_st bit for 32k-sync timer into the prcm-common header file, required for hwmod data.

Signed-off-by Vaibhav Hiremath <[email protected]>

Reviewed-by Santosh Shilimkar <[email protected]>

Cc Felipe Balbi <[email protected]>

Cc Benoit Cousson <[email protected]>

Cc Tony Lindgren <[email protected]>

Cc Paul Walmsley <[email protected]>

Cc Kevin Hilman <[email protected]>

---
NOTE: This patch is same as first version, without any code changes.

Add 32k-sync timer hwmod-data and add ocp_if details to omap2 & 3 hwmod table.

Signed-off-by Vaibhav Hiremath <[email protected]>

Signed-off-by Felipe Balbi <[email protected]>

Reviewed-by Santosh Shilimkar <[email protected]>

Cc Benoit Cousson <[email protected]>

Cc Tony Lindgren <[email protected]>

Cc Paul Walmsley <[email protected]>

Cc Kevin Hilman <[email protected]>

Current OMAP code supports couple of clocksource options based on compilation flag (CONFIG_OMAP_32K_TIMER). The 32KHz sync-timer and a gptimer which can run on 32KHz or system clock (e.g 38.4 MHz). So there can be 3 options -

1. 32KHz sync-timer
2. Sys_clock based (e.g 13/19.2/26/38.4 MHz) gptimer
3. 32KHz based gptimer.

The optional gptimer based clocksource was added so that it can give the high precision than sync-timer, so expected usage was 2 and not 3.
Unfortunately option 2, clocksource doesn't meet the requirement of free-running clock as per clocksource need. It stops in low power states when sys_clock is cut. That makes gptimer based clocksource option useless for OMAP2/3/4 devices with sys_clock as a clock input. So, in order to use option 2, deeper idle state MUST be disabled.

Option 3 will still work but it is no better than 32K sync-timer based clocksource.

We must support both sync timer and gptimer based clocksource as some OMAP based derivative SoCs like AM33XX does not have the sync timer.

Considering above, make sync-timer and gptimer clocksource runtime selectable so that both OMAP and AMXXXX continue to use the same code.

Also, in order to precisely configure/setup sched_clock for given clocksource, decision has to be made early enough in boot sequence.

So, the solution is,

Use standard kernel parameter ("clocksource=") to override default 32k_sync-timer, in addition to this, we also use hwmod database lookup mechanism, through which at run-time we can identify availability of 32k-sync timer on the device, else fall back to gptimer.

arch/arm/plat-omap/counter_32k.c +20 -47

-int __init omap_init_clocksource_32k(void)
+/**
+ * omap_init_clocksource_32k - setup and register counter 32k as a
+ * kernel clocksource
+ * @pbase: base addr of counter_32k module
+ * @size: size of counter_32k to map
+ *
+ * Returns 0 upon success or negative error code upon failure.
+ *
+ */
+int __init omap_init_clocksource_32k(u32 pbase, unsigned long size)
 {
-	static char err[] __initdata = KERN_ERR
-			"%s: can't register clocksource!\n";
-
-	if (cpu_is_omap16xx() || cpu_class_is_omap2()) {
-		u32 pbase;
-		unsigned long size = SZ_4K;
-		void __iomem *base;
-		struct clk *sync_32k_ick;
-
-		if (cpu_is_omap16xx()) {
-			pbase = OMAP16XX_TIMER_32K_SYNCHRONIZED;
-			size = SZ_1K;
-		} else if (cpu_is_omap2420())
-			pbase = OMAP2420_32KSYNCT_BASE + 0x10;
-		else if (cpu_is_omap2430())
-			pbase = OMAP2430_32KSYNCT_BASE + 0x10;
-		else if (cpu_is_omap34xx())
-			pbase = OMAP3430_32KSYNCT_BASE + 0x10;
-		else if (cpu_is_omap44xx())
-			pbase = OMAP4430_32KSYNCT_BASE + 0x10;
-		else
-			return -ENODEV;
-
-		/* For this to work we must have a static mapping in io.c for this area */
-		base = ioremap(pbase, size);
-		if (!base)
-			return -ENODEV;
-
-		sync_32k_ick = clk_get(NULL, "omap_32ksync_ick");
-		if (!IS_ERR(sync_32k_ick))
-			clk_enable(sync_32k_ick);
-
-		timer_32k_base = base;
-
-		/*
-		 * 120000 rough estimate from the calculations in
-		 * __clocksource_updatefreq_scale.
-		 */
-		clocks_calc_mult_shift(&persistent_mult, &persistent_shift,
-				32768, NSEC_PER_SEC, 120000);
-
-		if (clocksource_mmio_init(base, "32k_counter", 32768, 250, 32,
-					  clocksource_mmio_readl_up))
-			printk(err, "32k_counter");
-
-		setup_sched_clock(omap_32k_read_sched_clock, 32, 32768);
+	int ret;
+	void __iomem *base;
+	struct clk *sync32k_ick;
+
+	if (!pbase || !size)
+		return -ENODEV;
+	/*
+	 * For this to work we must have a static mapping in io.c
+	 * for this area
+	 */

Hi

a question

Quoted 37 lines · Vaibhav Hiremath

Current OMAP code supports couple of clocksource options based on compilation flag (CONFIG_OMAP_32K_TIMER). The 32KHz sync-timer and a gptimer which can run on 32KHz or system clock (e.g 38.4 MHz). So there can be 3 options -

1. 32KHz sync-timer
2. Sys_clock based (e.g 13/19.2/26/38.4 MHz) gptimer
3. 32KHz based gptimer.

The optional gptimer based clocksource was added so that it can give the high precision than sync-timer, so expected usage was 2 and not 3.
Unfortunately option 2, clocksource doesn't meet the requirement of free-running clock as per clocksource need. It stops in low power states when sys_clock is cut. That makes gptimer based clocksource option useless for OMAP2/3/4 devices with sys_clock as a clock input. So, in order to use option 2, deeper idle state MUST be disabled.

Option 3 will still work but it is no better than 32K sync-timer based clocksource.

We must support both sync timer and gptimer based clocksource as some OMAP based derivative SoCs like AM33XX does not have the sync timer.

Considering above, make sync-timer and gptimer clocksource runtime selectable so that both OMAP and AMXXXX continue to use the same code.

Also, in order to precisely configure/setup sched_clock for given clocksource, decision has to be made early enough in boot sequence.

So, the solution is,

Use standard kernel parameter ("clocksource=") to override default 32k_sync-timer, in addition to this, we also use hwmod database lookup mechanism, through which at run-time we can identify availability of 32k-sync timer on the device, else fall back to gptimer.

...

Quoted 76 lines

(quoted diff) +26 -48

-int __init omap_init_clocksource_32k(void)
+/**
+ * omap_init_clocksource_32k - setup and register counter 32k as a
+ * kernel clocksource
+ * @pbase: base addr of counter_32k module
+ * @size: size of counter_32k to map
+ *
+ * Returns 0 upon success or negative error code upon failure.
+ *
+ */
+int __init omap_init_clocksource_32k(u32 pbase, unsigned long size)
 {
-	static char err[] __initdata = KERN_ERR
-			"%s: can't register clocksource!\n";
-
-	if (cpu_is_omap16xx() || cpu_class_is_omap2()) {
-		u32 pbase;
-		unsigned long size = SZ_4K;
-		void __iomem *base;
-		struct clk *sync_32k_ick;
-
-		if (cpu_is_omap16xx()) {
-			pbase = OMAP16XX_TIMER_32K_SYNCHRONIZED;
-			size = SZ_1K;
-		} else if (cpu_is_omap2420())
-			pbase = OMAP2420_32KSYNCT_BASE + 0x10;
-		else if (cpu_is_omap2430())
-			pbase = OMAP2430_32KSYNCT_BASE + 0x10;
-		else if (cpu_is_omap34xx())
-			pbase = OMAP3430_32KSYNCT_BASE + 0x10;
-		else if (cpu_is_omap44xx())
-			pbase = OMAP4430_32KSYNCT_BASE + 0x10;
-		else
-			return -ENODEV;
-
-		/* For this to work we must have a static mapping in io.c for this area */
-		base = ioremap(pbase, size);
-		if (!base)
-			return -ENODEV;
-
-		sync_32k_ick = clk_get(NULL, "omap_32ksync_ick");
-		if (!IS_ERR(sync_32k_ick))
-			clk_enable(sync_32k_ick);
-
-		timer_32k_base = base;
-
-		/*
-		 * 120000 rough estimate from the calculations in
-		 * __clocksource_updatefreq_scale.
-		 */
-		clocks_calc_mult_shift(&persistent_mult, &persistent_shift,
-				32768, NSEC_PER_SEC, 120000);
-
-		if (clocksource_mmio_init(base, "32k_counter", 32768, 250, 32,
-					  clocksource_mmio_readl_up))
-			printk(err, "32k_counter");
-
-		setup_sched_clock(omap_32k_read_sched_clock, 32, 32768);
+	int ret;
+	void __iomem *base;
+	struct clk *sync32k_ick;
+
+	if (!pbase || !size)
+		return -ENODEV;
+	/*
+	 * For this to work we must have a static mapping in io.c
+	 * for this area
+	 */
+	base = ioremap(pbase, size);
+	if (!base) {
+		pr_err("32k_counter: failed to map base addr\n");
+		return -ENODEV;
 	}
-	return 0;
+
+	sync32k_ick = clk_get(NULL, "omap_32ksync_ick");

Review

Quoted 2 lines

(quoted diff) +2 -0

+	if (!IS_ERR(sync32k_ick))
+		clk_enable(sync32k_ick);

You've added hwmod data for this IP block, which is good. This will presumably cause the IP block to be idled on boot. But you haven't converted this code to use either the hwmod enable code -- just using clk_get() isn't enough. (Better would be to use a driver and the PM runtime functions, of course, but maybe this runs too early?)

If the 32k sync timer is in OCP force-idle, then that might produce the hangs you're seeing.

Hi

a question

On Wed, 25 Apr 2012, Vaibhav Hiremath wrote:

arch/arm/plat-omap/counter_32k.c void read_persistent_clock(struct timespec *ts) · line 101

Quoted 4 lines

arch/arm/plat-omap/counter_32k.c +4 -0

+
+	sync32k_ick = clk_get(NULL, "omap_32ksync_ick");
+	if (!IS_ERR(sync32k_ick))
+		clk_enable(sync32k_ick);

You've added hwmod data for this IP block, which is good. This will presumably cause the IP block to be idled on boot. But you haven't converted this code to use either the hwmod enable code -- just using clk_get() isn't enough. (Better would be to use a driver and the PM runtime functions, of course, but maybe this runs too early?)

If the 32k sync timer is in OCP force-idle, then that might produce the hangs you're seeing.

Quoted 2 lines · Paul Walmsley

On Wed, 25 Apr 2012, Vaibhav Hiremath wrote:

<snip>

Quoted 11 lines

arch/arm/plat-omap/counter_32k.c void read_persistent_clock(struct timespec *ts) · line 101

Quoted 4 lines

arch/arm/plat-omap/counter_32k.c +4 -0

+
+	sync32k_ick = clk_get(NULL, "omap_32ksync_ick");
+	if (!IS_ERR(sync32k_ick))
+		clk_enable(sync32k_ick);

You've added hwmod data for this IP block, which is good. This will presumably cause the IP block to be idled on boot. But you haven't converted this code to use either the hwmod enable code -- just using clk_get() isn't enough. (Better would be to use a driver and the PM runtime functions, of course, but maybe this runs too early?)

Quoted 3 lines

If the 32k sync timer is in OCP force-idle, then that might produce the hangs you're seeing.

I tried disabling all idle related flags in hwmod entry but no change in observation, kernel still hangs.

I even conformed the register content by connecting debugger, and it is set to No-idle state.

Okay, thanks for testing. Please do update this patch to use omap_hwmod_enable(), etc.; see for example omap_dm_timer_init_one().

Quoted 2 lines · Paul Walmsley

Okay, thanks for testing. Please do update this patch to use omap_hwmod_enable(), etc.; see for example omap_dm_timer_init_one().

And, just to be explicit, the ioremap(), clk_get(), and clk_enable() should no longer be needed for OMAP2+, once you add the omap_hwmod_enable().

Quoted 5 lines · Paul Walmsley

Quoted 2 lines · Paul Walmsley

Okay, thanks for testing. Please do update this patch to use omap_hwmod_enable(), etc.; see for example omap_dm_timer_init_one().

Quoted 4 lines

And, just to be explicit, the ioremap(), clk_get(), and clk_enable() should no longer be needed for OMAP2+, once you add the omap_hwmod_enable().

What about OMAP1 architecture? Will it work?

Current OMAP code supports couple of clocksource options based on compilation flag (CONFIG_OMAP_32K_TIMER). The 32KHz sync-timer and a gptimer which can run on 32KHz or system clock (e.g 38.4 MHz). So there can be 3 options -

1. 32KHz sync-timer
2. Sys_clock based (e.g 13/19.2/26/38.4 MHz) gptimer
3. 32KHz based gptimer.

The optional gptimer based clocksource was added so that it can give the high precision than sync-timer, so expected usage was 2 and not 3.
Unfortunately option 2, clocksource doesn't meet the requirement of free-running clock as per clocksource need. It stops in low power states when sys_clock is cut. That makes gptimer based clocksource option useless for OMAP2/3/4 devices with sys_clock as a clock input. So, in order to use option 2, deeper idle state MUST be disabled.

Option 3 will still work but it is no better than 32K sync-timer based clocksource.

We must support both sync timer and gptimer based clocksource as some OMAP based derivative SoCs like AM33XX does not have the sync timer.

Considering above, make sync-timer and gptimer clocksource runtime selectable so that both OMAP and AMXXXX continue to use the same code.

Also, in order to precisely configure/setup sched_clock for given clocksource, decision has to be made early enough in boot sequence.

So, the solution is,

Use standard kernel parameter ("clocksource=") to override default 32k_sync-timer, in addition to this, we also use hwmod database lookup mechanism, through which at run-time we can identify availability of 32k-sync timer on the device, else fall back to gptimer.

Signed-off-by Vaibhav Hiremath <[email protected]>

Signed-off-by Felipe Balbi <[email protected]>

Reviewed-by Santosh Shilimkar <[email protected]>

Cc Benoit Cousson <[email protected]>

Cc Tony Lindgren <[email protected]>

Cc Paul Walmsley <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Tarun Kanti DebBarma <[email protected]>

Cc Ming Lei <[email protected]>

---
NOTE: This patch is same as V3, without any code changes. Only
commit description has been modified.

 arch/arm/mach-omap1/timer32k.c           |    6 ++-
 arch/arm/mach-omap2/timer.c              |   99 +++++++++++++++++++++------
 arch/arm/plat-omap/counter_32k.c         |  108 +++++++++++++++---------------
 arch/arm/plat-omap/include/plat/common.h |    2 +-
 4 files changed, 138 insertions(+), 77 deletions(-)

arch/arm/mach-omap1/timer32k.c +1 -0

index 325b9a0..6262e11 100644
@@ -71,6 +71,7 @@
 
 /* 16xx specific defines */
 #define OMAP1_32K_TIMER_BASE		0xfffb9000
+#define OMAP1_32KSYNC_TIMER_BASE	0xfffbc400
 #define OMAP1_32K_TIMER_CR		0x08
 #define OMAP1_32K_TIMER_TVR		0x00
 #define OMAP1_32K_TIMER_TCR		0x04

arch/arm/mach-omap1/timer32k.c +0 -0

 	omap_init_32k_timer();
 	return true;

arch/arm/mach-omap2/timer.c +8 -16

index ecec873..d720f58 100644
@@ -243,22 +243,8 @@ static void __init omap2_gp_clockevent_init(int gptimer_id,
 }
 
 /* Clocksource code */
-
-#ifdef CONFIG_OMAP_32K_TIMER
-/*
- * When 32k-timer is enabled, don't use GPTimer for clocksource
- * instead, just leave default clocksource which uses the 32k
- * sync counter.  See clocksource setup in plat-omap/counter_32k.c
- */
-
-static void __init omap2_gp_clocksource_init(int unused, const char *dummy)
-{
-	omap_init_clocksource_32k();
-}
-
-#else
-
 static struct omap_dm_timer clksrc;
+static bool use_gptimer_clksrc;
 
 /*
  * clocksource

@@ -285,7 +271,33 @@ static u32 notrace dmtimer_read_sched_clock(void)
 }
 
 /* Setup free-running counter for clocksource */
-static void __init omap2_gp_clocksource_init(int gptimer_id,
+static int __init omap2_sync32k_clocksource_init(void)
+{
+	int ret;
+	struct omap_hwmod *oh;
+	struct resource res;
+	const char *oh_name = "counter_32k";
+

arch/arm/mach-omap2/timer.c +33 -6

+	ret = omap_init_clocksource_32k(res.start, resource_size(&res));
+	if (ret)
+		pr_warn("%s: failed to initialize counter_32k (%d)\n",
+							__func__, ret);
+
+	return ret;
+}
+
+static void __init omap2_gptimer_clocksource_init(int gptimer_id,
 						const char *fck_source)
 {
 	int res;

@@ -293,9 +305,6 @@ static void __init omap2_gp_clocksource_init(int gptimer_id,
 	res = omap_dm_timer_init_one(&clksrc, gptimer_id, fck_source);
 	BUG_ON(res);
 
-	pr_info("OMAP clocksource: GPTIMER%d at %lu Hz\n",
-		gptimer_id, clksrc.rate);
-
 	__omap_dm_timer_load_start(&clksrc,
 			OMAP_TIMER_CTRL_ST | OMAP_TIMER_CTRL_AR, 0, 1);
 	setup_sched_clock(dmtimer_read_sched_clock, 32, clksrc.rate);

@@ -303,15 +312,36 @@ static void __init omap2_gp_clocksource_init(int gptimer_id,
 	if (clocksource_register_hz(&clocksource_gpt, clksrc.rate))
 		pr_err("Could not register clocksource %s\n",
 			clocksource_gpt.name);
+	else
+		pr_info("OMAP clocksource: GPTIMER%d at %lu Hz\n",
+			gptimer_id, clksrc.rate);
+}
+
+static void __init omap2_clocksource_init(int gptimer_id,
+						const char *fck_source)
+{
+	/*
+	 * First give preference to kernel parameter configuration
+	 * by user (clocksource="gp timer").
+	 *
+	 * In case of missing kernel parameter for clocksource,
+	 * first check for availability for 32k-sync timer, in case
+	 * of failure in finding 32k_counter module or registering
+	 * it as clocksource, execution will fallback to gp-timer.
+	 */
+	if (use_gptimer_clksrc == true)
+		omap2_gptimer_clocksource_init(gptimer_id, fck_source);
+	else if (omap2_sync32k_clocksource_init())
+		/* Fall back to gp-timer code */
+		omap2_gptimer_clocksource_init(gptimer_id, fck_source);
 }
-#endif
 
 #define OMAP_SYS_TIMER_INIT(name, clkev_nr, clkev_src,			\
 				clksrc_nr, clksrc_src)			\
 static void __init omap##name##_timer_init(void)			\
 {									\
 	omap2_gp_clockevent_init((clkev_nr), clkev_src);		\
-	omap2_gp_clocksource_init((clksrc_nr), clksrc_src);		\
+	omap2_clocksource_init((clksrc_nr), clksrc_src);		\
 }
 
 #define OMAP_SYS_TIMER(name)						\

@@ -342,7 +372,7 @@ static DEFINE_TWD_LOCAL_TIMER(twd_local_timer,
 static void __init omap4_timer_init(void)
 {
 	omap2_gp_clockevent_init(1, OMAP4_CLKEV_SOURCE);
-	omap2_gp_clocksource_init(2, OMAP4_MPU_SOURCE);
+	omap2_clocksource_init(2, OMAP4_MPU_SOURCE);
 #ifdef CONFIG_LOCAL_TIMERS
 	/* Local timers are not supprted on OMAP4430 ES1.0 */
 	if (omap_rev() != OMAP4430_REV_ES1_0) {

arch/arm/mach-omap2/timer.c +18 -0

+ * Note that, here we are using same standard kernel parameter "clocksource=",
+ * and not introducing any OMAP specific interface.
+ */
+static int __init omap2_override_clocksource(char *str)
+{
+	if (!str)
+		return 0;
+	/*
+	 * For OMAP architecture, we only have two options
+	 *    - sync_32k (default)
+	 *    - gp timer (sys_clk based)
+	 */
+	if (!strcmp(str, "gp timer"))
+		use_gptimer_clksrc = true;
+
+	return 0;
+}
+early_param("clocksource", omap2_override_clocksource);

arch/arm/plat-omap/counter_32k.c +0 -0

index 5068fe5..3e3cdab 100644

arch/arm/plat-omap/counter_32k.c +33 -53

 /*
  * 32KHz clocksource ... always available, on pretty most chips except
  * OMAP 730 and 1510.  Other timers could be used as clocksources, with
  * higher resolution in free-running counter modes (e.g. 12 MHz xtal),
  * but systems won't necessarily want to spend resources that way.
  */
-static void __iomem *timer_32k_base;
-
-#define OMAP16XX_TIMER_32K_SYNCHRONIZED		0xfffbc410
+static void __iomem *sync32k_cnt_reg;
 static u32 notrace omap_32k_read_sched_clock(void)
 {
-	return timer_32k_base ? __raw_readl(timer_32k_base) : 0;
+	return sync32k_cnt_reg ? __raw_readl(sync32k_cnt_reg) : 0;
 }
 
 /**

@@ -59,7 +60,7 @@ void read_persistent_clock(struct timespec *ts)
 	struct timespec *tsp = &persistent_ts;
 
 	last_cycles = cycles;
-	cycles = timer_32k_base ? __raw_readl(timer_32k_base) : 0;
+	cycles = sync32k_cnt_reg ? __raw_readl(sync32k_cnt_reg) : 0;
 	delta = cycles - last_cycles;
 
 	nsecs = clocksource_cyc2ns(delta, persistent_mult, persistent_shift);

@@ -68,54 +69,55 @@ void read_persistent_clock(struct timespec *ts)
 	*ts = *tsp;
 }
 
-int __init omap_init_clocksource_32k(void)
+/**
+ * omap_init_clocksource_32k - setup and register counter 32k as a
+ * kernel clocksource
+ * @pbase: base addr of counter_32k module
+ * @size: size of counter_32k to map
+ *
+ * Returns 0 upon success or negative error code upon failure.
+ *
+ */
+int __init omap_init_clocksource_32k(u32 pbase, unsigned long size)
 {
-	static char err[] __initdata = KERN_ERR
-			"%s: can't register clocksource!\n";
-
-	if (cpu_is_omap16xx() || cpu_class_is_omap2()) {
-		u32 pbase;
-		unsigned long size = SZ_4K;
-		void __iomem *base;
-		struct clk *sync_32k_ick;
-
-		if (cpu_is_omap16xx()) {
-			pbase = OMAP16XX_TIMER_32K_SYNCHRONIZED;
-			size = SZ_1K;
-		} else if (cpu_is_omap2420())
-			pbase = OMAP2420_32KSYNCT_BASE + 0x10;
-		else if (cpu_is_omap2430())
-			pbase = OMAP2430_32KSYNCT_BASE + 0x10;
-		else if (cpu_is_omap34xx())
-			pbase = OMAP3430_32KSYNCT_BASE + 0x10;
-		else if (cpu_is_omap44xx())
-			pbase = OMAP4430_32KSYNCT_BASE + 0x10;
-		else
-			return -ENODEV;
-
-		/* For this to work we must have a static mapping in io.c for this area */
-		base = ioremap(pbase, size);
-		if (!base)
-			return -ENODEV;
-
-		sync_32k_ick = clk_get(NULL, "omap_32ksync_ick");
-		if (!IS_ERR(sync_32k_ick))
-			clk_enable(sync_32k_ick);
-
-		timer_32k_base = base;
-
-		/*
-		 * 120000 rough estimate from the calculations in
-		 * __clocksource_updatefreq_scale.
-		 */
-		clocks_calc_mult_shift(&persistent_mult, &persistent_shift,
-				32768, NSEC_PER_SEC, 120000);
-
-		if (clocksource_mmio_init(base, "32k_counter", 32768, 250, 32,
-					  clocksource_mmio_readl_up))
-			printk(err, "32k_counter");
-
-		setup_sched_clock(omap_32k_read_sched_clock, 32, 32768);
+	int ret;
+	void __iomem *base;
+	struct clk *sync32k_ick;
+
+	if (!pbase || !size)
+		return -ENODEV;
+	/*
+	 * For this to work we must have a static mapping in io.c
+	 * for this area
+	 */
+	base = ioremap(pbase, size);
+	if (!base) {
+		pr_err("32k_counter: failed to map base addr\n");
+		return -ENODEV;
 	}
-	return 0;
+
+	sync32k_ick = clk_get(NULL, "omap_32ksync_ick");
+	if (!IS_ERR(sync32k_ick))
+		clk_enable(sync32k_ick);
+
+	sync32k_cnt_reg = base + OMAP2_32KSYNCNT_CR_OFF;

arch/arm/plat-omap/counter_32k.c +9 -0

+	if (ret) {
+		pr_err("32k_counter: can't register clocksource\n");
+		return ret;
+	}
+
+	setup_sched_clock(omap_32k_read_sched_clock, 32, 32768);
+	pr_info("OMAP clocksource: 32k_counter at 32768 Hz\n");
+
+	return ret;
 }

arch/arm/plat-omap/include/plat/common.h +1 -2

index a557b84..eed5335 100644
@@ -30,7 +30,7 @@
 #include <plat/i2c.h>
 #include <plat/omap_hwmod.h>
 
-extern int __init omap_init_clocksource_32k(void);
+extern int __init omap_init_clocksource_32k(u32 pbase, unsigned long size);
 
 extern void __init omap_check_revision(void);
 
--

1.7.0.4

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 47 lines · Vaibhav Hiremath

Current OMAP code supports couple of clocksource options based on compilation flag (CONFIG_OMAP_32K_TIMER). The 32KHz sync-timer and a gptimer which can run on 32KHz or system clock (e.g 38.4 MHz). So there can be 3 options -

1. 32KHz sync-timer
2. Sys_clock based (e.g 13/19.2/26/38.4 MHz) gptimer
3. 32KHz based gptimer.

The optional gptimer based clocksource was added so that it can give the high precision than sync-timer, so expected usage was 2 and not 3.
Unfortunately option 2, clocksource doesn't meet the requirement of free-running clock as per clocksource need. It stops in low power states when sys_clock is cut. That makes gptimer based clocksource option useless for OMAP2/3/4 devices with sys_clock as a clock input. So, in order to use option 2, deeper idle state MUST be disabled.

Option 3 will still work but it is no better than 32K sync-timer based clocksource.

We must support both sync timer and gptimer based clocksource as some OMAP based derivative SoCs like AM33XX does not have the sync timer.

Considering above, make sync-timer and gptimer clocksource runtime selectable so that both OMAP and AMXXXX continue to use the same code.

Also, in order to precisely configure/setup sched_clock for given clocksource, decision has to be made early enough in boot sequence.

So, the solution is,

Use standard kernel parameter ("clocksource=") to override default 32k_sync-timer, in addition to this, we also use hwmod database lookup mechanism, through which at run-time we can identify availability of 32k-sync timer on the device, else fall back to gptimer.

Signed-off-by Vaibhav Hiremath <[email protected]>

Signed-off-by Felipe Balbi <[email protected]>

Reviewed-by Santosh Shilimkar <[email protected]>

Cc Benoit Cousson <[email protected]>

Cc Tony Lindgren <[email protected]>

Cc Paul Walmsley <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Tarun Kanti DebBarma <[email protected]>

Cc Ming Lei <[email protected]>

Quoted 3 lines

---
NOTE: This patch is same as V3, without any code changes. Only
commit description has been modified.

I assume you menat v4 here, since you also change the ioremap() range in this version compared to v3?

Also, I boot tested this on OMAP1 (5912/OSK) and OMAP3530/Overo and with the fix for clocksource_mmio_init() below, it worked fine. I also tested the cmdline override on OMAP3.

That being said, a few minor comments below...

Quoted 19 lines

 arch/arm/mach-omap1/timer32k.c           |    6 ++-
 arch/arm/mach-omap2/timer.c              |   99 +++++++++++++++++++++------
 arch/arm/plat-omap/counter_32k.c         |  108 +++++++++++++++---------------
 arch/arm/plat-omap/include/plat/common.h |    2 +-
 4 files changed, 138 insertions(+), 77 deletions(-)

arch/arm/mach-omap1/timer32k.c +1 -0

index 325b9a0..6262e11 100644
@@ -71,6 +71,7 @@
 
 /* 16xx specific defines */
 #define OMAP1_32K_TIMER_BASE		0xfffb9000
+#define OMAP1_32KSYNC_TIMER_BASE	0xfffbc400
 #define OMAP1_32K_TIMER_CR		0x08
 #define OMAP1_32K_TIMER_TVR		0x00
 #define OMAP1_32K_TIMER_TCR		0x04

arch/arm/mach-omap1/timer32k.c static __init void omap_init_32k_timer(void) · line 190

Quoted 7 lines

arch/arm/mach-omap1/timer32k.c +3 -1

@@ -184,7 +185,10 @@ static __init void omap_init_32k_timer(void)
  */
 bool __init omap_32k_timer_init(void)
 {
-	omap_init_clocksource_32k();
+	u32 pbase;
+
+	pbase = cpu_is_omap16xx() ? OMAP1_32KSYNC_TIMER_BASE : NULL;

Compile testing on OMAP1 (using omap1_defconfig) gives a few compiler warnings:

/work/kernel/omap/dev/arch/arm/mach-omap1/timer32k.c: In function 'omap_32k_timer_init':
/work/kernel/omap/dev/arch/arm/mach-omap1/timer32k.c:190:37: warning: pointer/integer type mismatch in conditional expression [enabled by default] /work/kernel/omap/dev/arch/arm/mach-omap1/timer32k.c:190:8: warning: assignment makes integer from pointer without a cast [enabled by default]

arch/arm/mach-omap1/timer32k.c static __init void omap_init_32k_timer(void) · line 191

Quoted 1 line

arch/arm/mach-omap1/timer32k.c +1 -0

+	omap_init_clocksource_32k(pbase, SZ_1K);

Should this be called if pbase == NULL?

If this clocksource does fail, how does the MPU clocksource get registered?

Looking a bit closer, it appears the whole MPU vs. 32k init is even more messed up on OMAP1 than OMAP2+ and needs some cleanup. However, it's not directly related to this series since it was messed up before your series.

Quoted 47 lines

arch/arm/mach-omap1/timer32k.c +0 -0

 	omap_init_32k_timer();
 	return true;

arch/arm/mach-omap2/timer.c +13 -16

index ecec873..d720f58 100644
@@ -243,22 +243,8 @@ static void __init omap2_gp_clockevent_init(int gptimer_id,
 }
 
 /* Clocksource code */
-
-#ifdef CONFIG_OMAP_32K_TIMER
-/*
- * When 32k-timer is enabled, don't use GPTimer for clocksource
- * instead, just leave default clocksource which uses the 32k
- * sync counter.  See clocksource setup in plat-omap/counter_32k.c
- */
-
-static void __init omap2_gp_clocksource_init(int unused, const char *dummy)
-{
-	omap_init_clocksource_32k();
-}
-
-#else
-
 static struct omap_dm_timer clksrc;
+static bool use_gptimer_clksrc;
 
 /*
  * clocksource

@@ -285,7 +271,33 @@ static u32 notrace dmtimer_read_sched_clock(void)
 }
 
 /* Setup free-running counter for clocksource */
-static void __init omap2_gp_clocksource_init(int gptimer_id,
+static int __init omap2_sync32k_clocksource_init(void)
+{
+	int ret;
+	struct omap_hwmod *oh;
+	struct resource res;
+	const char *oh_name = "counter_32k";
+
+	oh = omap_hwmod_lookup(oh_name);
+	if (!oh || oh->slaves_cnt == 0)
+		return -ENODEV;
+
+	ret = omap_hwmod_get_resource_byname(oh, IORESOURCE_MEM, NULL, &res);

arch/arm/mach-omap2/timer.c static u32 notrace dmtimer_read_sched_clock(void) · line 290

Quoted 5 lines

arch/arm/mach-omap2/timer.c +5 -0

+	if (ret) {
+		pr_warn("%s: failed to get counter_32k resource (%d)\n",
+							__func__, ret);
+		return ret;
+	}

I think Paul mentioned this already, but you should really have an omap_hwmod_enable() here.

Quoted 71 lines

arch/arm/mach-omap2/timer.c +33 -6

+	ret = omap_init_clocksource_32k(res.start, resource_size(&res));
+	if (ret)
+		pr_warn("%s: failed to initialize counter_32k (%d)\n",
+							__func__, ret);
+
+	return ret;
+}
+
+static void __init omap2_gptimer_clocksource_init(int gptimer_id,
 						const char *fck_source)
 {
 	int res;

@@ -293,9 +305,6 @@ static void __init omap2_gp_clocksource_init(int gptimer_id,
 	res = omap_dm_timer_init_one(&clksrc, gptimer_id, fck_source);
 	BUG_ON(res);
 
-	pr_info("OMAP clocksource: GPTIMER%d at %lu Hz\n",
-		gptimer_id, clksrc.rate);
-
 	__omap_dm_timer_load_start(&clksrc,
 			OMAP_TIMER_CTRL_ST | OMAP_TIMER_CTRL_AR, 0, 1);
 	setup_sched_clock(dmtimer_read_sched_clock, 32, clksrc.rate);

@@ -303,15 +312,36 @@ static void __init omap2_gp_clocksource_init(int gptimer_id,
 	if (clocksource_register_hz(&clocksource_gpt, clksrc.rate))
 		pr_err("Could not register clocksource %s\n",
 			clocksource_gpt.name);
+	else
+		pr_info("OMAP clocksource: GPTIMER%d at %lu Hz\n",
+			gptimer_id, clksrc.rate);
+}
+
+static void __init omap2_clocksource_init(int gptimer_id,
+						const char *fck_source)
+{
+	/*
+	 * First give preference to kernel parameter configuration
+	 * by user (clocksource="gp timer").
+	 *
+	 * In case of missing kernel parameter for clocksource,
+	 * first check for availability for 32k-sync timer, in case
+	 * of failure in finding 32k_counter module or registering
+	 * it as clocksource, execution will fallback to gp-timer.
+	 */
+	if (use_gptimer_clksrc == true)
+		omap2_gptimer_clocksource_init(gptimer_id, fck_source);
+	else if (omap2_sync32k_clocksource_init())
+		/* Fall back to gp-timer code */
+		omap2_gptimer_clocksource_init(gptimer_id, fck_source);
 }
-#endif
 
 #define OMAP_SYS_TIMER_INIT(name, clkev_nr, clkev_src,			\
 				clksrc_nr, clksrc_src)			\
 static void __init omap##name##_timer_init(void)			\
 {									\
 	omap2_gp_clockevent_init((clkev_nr), clkev_src);		\
-	omap2_gp_clocksource_init((clksrc_nr), clksrc_src);		\
+	omap2_clocksource_init((clksrc_nr), clksrc_src);		\
 }
 
 #define OMAP_SYS_TIMER(name)						\

@@ -342,7 +372,7 @@ static DEFINE_TWD_LOCAL_TIMER(twd_local_timer,
 static void __init omap4_timer_init(void)
 {
 	omap2_gp_clockevent_init(1, OMAP4_CLKEV_SOURCE);
-	omap2_gp_clocksource_init(2, OMAP4_MPU_SOURCE);
+	omap2_clocksource_init(2, OMAP4_MPU_SOURCE);
 #ifdef CONFIG_LOCAL_TIMERS
 	/* Local timers are not supprted on OMAP4430 ES1.0 */
 	if (omap_rev() != OMAP4430_REV_ES1_0) {

arch/arm/mach-omap2/timer.c static int __init omap2_dm_timer_init(void) · line 548

Quoted 9 lines

arch/arm/mach-omap2/timer.c +6 -0

@@ -510,3 +540,28 @@ static int __init omap2_dm_timer_init(void)
 	return 0;
 }
 arch_initcall(omap2_dm_timer_init);
+
+/**
+ * omap2_override_clocksource - clocksource override with user configuration
+ *
+ * Allows user to override default clocksource, using kernel parameter
+ *   clocksource="gp timer"	(For all OMAP2PLUS architectures)

Passing command-line arguments with spaces might be a little problematic, depending on the bootloader. e.g. the u-boot on my 3530/Overo doesn't seem to pass along the quotes, so this function only gets the 'gp' part of the string, so the override never works.

I suggest changing the name of this timer to gp_timer to simplify kernel command-line handling.

Quoted 24 lines

arch/arm/mach-omap2/timer.c +18 -0

+ * Note that, here we are using same standard kernel parameter "clocksource=",
+ * and not introducing any OMAP specific interface.
+ */
+static int __init omap2_override_clocksource(char *str)
+{
+	if (!str)
+		return 0;
+	/*
+	 * For OMAP architecture, we only have two options
+	 *    - sync_32k (default)
+	 *    - gp timer (sys_clk based)
+	 */
+	if (!strcmp(str, "gp timer"))
+		use_gptimer_clksrc = true;
+
+	return 0;
+}
+early_param("clocksource", omap2_override_clocksource);

arch/arm/plat-omap/counter_32k.c +0 -0

index 5068fe5..3e3cdab 100644
@@ -27,19 +27,20 @@
 

arch/arm/plat-omap/counter_32k.c line 31

Quoted 4 lines

arch/arm/plat-omap/counter_32k.c +2 -0

 #include <plat/clock.h>
 
+/* OMAP2_32KSYNCNT_CR_OFF: offset of 32ksync counter register */
+#define OMAP2_32KSYNCNT_CR_OFF		0x10

This is valid for OMAP1 also.

Quoted 110 lines

arch/arm/plat-omap/counter_32k.c +33 -53

 /*
  * 32KHz clocksource ... always available, on pretty most chips except
  * OMAP 730 and 1510.  Other timers could be used as clocksources, with
  * higher resolution in free-running counter modes (e.g. 12 MHz xtal),
  * but systems won't necessarily want to spend resources that way.
  */
-static void __iomem *timer_32k_base;
-
-#define OMAP16XX_TIMER_32K_SYNCHRONIZED		0xfffbc410
+static void __iomem *sync32k_cnt_reg;
 static u32 notrace omap_32k_read_sched_clock(void)
 {
-	return timer_32k_base ? __raw_readl(timer_32k_base) : 0;
+	return sync32k_cnt_reg ? __raw_readl(sync32k_cnt_reg) : 0;
 }
 
 /**

@@ -59,7 +60,7 @@ void read_persistent_clock(struct timespec *ts)
 	struct timespec *tsp = &persistent_ts;
 
 	last_cycles = cycles;
-	cycles = timer_32k_base ? __raw_readl(timer_32k_base) : 0;
+	cycles = sync32k_cnt_reg ? __raw_readl(sync32k_cnt_reg) : 0;
 	delta = cycles - last_cycles;
 
 	nsecs = clocksource_cyc2ns(delta, persistent_mult, persistent_shift);

@@ -68,54 +69,55 @@ void read_persistent_clock(struct timespec *ts)
 	*ts = *tsp;
 }
 
-int __init omap_init_clocksource_32k(void)
+/**
+ * omap_init_clocksource_32k - setup and register counter 32k as a
+ * kernel clocksource
+ * @pbase: base addr of counter_32k module
+ * @size: size of counter_32k to map
+ *
+ * Returns 0 upon success or negative error code upon failure.
+ *
+ */
+int __init omap_init_clocksource_32k(u32 pbase, unsigned long size)
 {
-	static char err[] __initdata = KERN_ERR
-			"%s: can't register clocksource!\n";
-
-	if (cpu_is_omap16xx() || cpu_class_is_omap2()) {
-		u32 pbase;
-		unsigned long size = SZ_4K;
-		void __iomem *base;
-		struct clk *sync_32k_ick;
-
-		if (cpu_is_omap16xx()) {
-			pbase = OMAP16XX_TIMER_32K_SYNCHRONIZED;
-			size = SZ_1K;
-		} else if (cpu_is_omap2420())
-			pbase = OMAP2420_32KSYNCT_BASE + 0x10;
-		else if (cpu_is_omap2430())
-			pbase = OMAP2430_32KSYNCT_BASE + 0x10;
-		else if (cpu_is_omap34xx())
-			pbase = OMAP3430_32KSYNCT_BASE + 0x10;
-		else if (cpu_is_omap44xx())
-			pbase = OMAP4430_32KSYNCT_BASE + 0x10;
-		else
-			return -ENODEV;
-
-		/* For this to work we must have a static mapping in io.c for this area */
-		base = ioremap(pbase, size);
-		if (!base)
-			return -ENODEV;
-
-		sync_32k_ick = clk_get(NULL, "omap_32ksync_ick");
-		if (!IS_ERR(sync_32k_ick))
-			clk_enable(sync_32k_ick);
-
-		timer_32k_base = base;
-
-		/*
-		 * 120000 rough estimate from the calculations in
-		 * __clocksource_updatefreq_scale.
-		 */
-		clocks_calc_mult_shift(&persistent_mult, &persistent_shift,
-				32768, NSEC_PER_SEC, 120000);
-
-		if (clocksource_mmio_init(base, "32k_counter", 32768, 250, 32,
-					  clocksource_mmio_readl_up))
-			printk(err, "32k_counter");
-
-		setup_sched_clock(omap_32k_read_sched_clock, 32, 32768);
+	int ret;
+	void __iomem *base;
+	struct clk *sync32k_ick;
+
+	if (!pbase || !size)
+		return -ENODEV;
+	/*
+	 * For this to work we must have a static mapping in io.c
+	 * for this area
+	 */
+	base = ioremap(pbase, size);
+	if (!base) {
+		pr_err("32k_counter: failed to map base addr\n");
+		return -ENODEV;
 	}
-	return 0;
+
+	sync32k_ick = clk_get(NULL, "omap_32ksync_ick");
+	if (!IS_ERR(sync32k_ick))
+		clk_enable(sync32k_ick);
+
+	sync32k_cnt_reg = base + OMAP2_32KSYNCNT_CR_OFF;

arch/arm/plat-omap/counter_32k.c void read_persistent_clock(struct timespec *ts) · line 113

Quoted 10 lines

arch/arm/plat-omap/counter_32k.c +10 -0

+
+	/*
+	 * 120000 rough estimate from the calculations in
+	 * __clocksource_updatefreq_scale.
+	 */
+	clocks_calc_mult_shift(&persistent_mult, &persistent_shift,
+			32768, NSEC_PER_SEC, 120000);
+
+	ret = clocksource_mmio_init(base, "32k_counter", 32768, 250, 32,
+				clocksource_mmio_readl_up);

Just to be thorough, even though we already discussed this off-list:

s/base/sync32k_cnt_reg/