ARM: OMAP: Make OMAP clocksource source selection runtime

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. Option 3 will still work but it is no better than 32K sync-timer based clocksource.

So ideally we can kill the gptimer based clocksource option but there are some OMAP based derivative SoCs like AM33XX which doesn't have 32K sync-timer hardware IP and need to fallback on 32KHz based gptimer clocksource.

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 kernel parameter ("clocksource=") to override

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

@@ -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

@@ -184,7 +185,10 @@ static __init void omap_init_32k_timer(void)
  */

arch/arm/mach-omap2/timer.c +15 -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"
+ *
+ * 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;
+	/*

Hi,

Quoted 9 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.

Quoted 9 lines

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. Option 3 will still work but it is no better than 32K sync-timer based clocksource.

For some cases sys clock based timer is still valid if you don't care about PM. In that case deeper idle states need to be disabled, not the timer as discussed earlier. Please update the comments accordingly.

Quoted 4 lines

So ideally we can kill the gptimer based clocksource option but there are some OMAP based derivative SoCs like AM33XX which doesn't have 32K sync-timer hardware IP and need to fallback on 32KHz based gptimer clocksource.

Maybe just say: "We must support both sync timer and gptimer based clocksource as some AM33XX hardware does not have the sync timer."

Quoted 9 lines

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 kernel parameter ("clocksource=") to override

Maybe say: "Use standard kernel parameter ("clocksource=")..."

Quoted 21 lines

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

@@ -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

@@ -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;
+	omap_init_clocksource_32k(pbase, SZ_1K);
 	omap_init_32k_timer();
 

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

Quoted 1 line

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

 	return true;

Has this patch been tested on omap1?

Quoted 28 lines

arch/arm/mach-omap2/timer.c +21 -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"
+ *
+ * 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
+	 */
+	if (!strcmp(str, "gp timer"))
+		use_gptimer_clksrc = true;

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

Quoted 4 lines

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

+
+	return 0;
+}
+early_param("clocksource", omap2_override_clocksource);

Should say "For omap2plus architectures" and should say three options. If the sys clock based timer is not currently supported, please mention that in the comments.

Regards,

Tony

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. Option 3 will still work but it is no better than 32K sync-timer based clocksource.

So ideally we can kill the gptimer based clocksource option but there are some OMAP based derivative SoCs like AM33XX which doesn't have 32K sync-timer hardware IP and need to fallback on 32KHz based gptimer clocksource.
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 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]>

Cc Santosh Shilimkar <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Benoit Cousson <[email protected]>

Cc Tony Lindgren <[email protected]>

Cc Paul Walmsley <[email protected]>

Cc Tarun Kanti DebBarma <[email protected]>

Cc Ming Lei <[email protected]>

---

?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 +0 -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

@@ -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;
+ ? ? ? omap_init_clocksource_32k(pbase, SZ_1K);
? ? ? ?omap_init_32k_timer();

? ? ? ?return true;

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

index ecec873..2afc4ab 100644
@@ -243,22 +243,8 @@ static void __init omap2_gp_clockevent_init(int gptimer_id,
 ?}
 

?/* Clocksource code */

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

-
-#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

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

@@ -285,7 +271,33 @@ static u32 notrace dmtimer_read_sched_clock(void)
 ?}
 

?/* Setup free-running counter for clocksource */

arch/arm/mach-omap2/timer.c +27 -1

-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);
+ ? ? ? if (ret) {
+ ? ? ? ? ? ? ? pr_warn("%s: failed to get counter_32k resource (%d)\n",
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __func__, ret);
+ ? ? ? ? ? ? ? return ret;
+ ? ? ? }
+
+ ? ? ? 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;

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

@@ -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 +25 -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"
+ *
+ * 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
+ ? ? ? ?*/
+ ? ? ? 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 @@
 

?#include <plat/clock.h>

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

+/* OMAP2_32KSYNCNT_CR_OFF: offset of 32ksync counter register */
+#define OMAP2_32KSYNCNT_CR_OFF ? ? ? ? 0x10
+

?/*
?* 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. ?*/

arch/arm/plat-omap/counter_32k.c +1 -3

-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; ?}

?/**

arch/arm/plat-omap/counter_32k.c +50 -49

@@ -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;
+
+ ? ? ? /*
+ ? ? ? ?* 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);
+ ? ? ? 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 +0 -0

index a557b84..eed5335 100644
@@ -30,7 +30,7 @@

?#include <plat/i2c.h>
?#include <plat/omap_hwmod.h>

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

-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 36 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. Option 3 will still work but it is no better than 32K sync-timer based clocksource.

So ideally we can kill the gptimer based clocksource option but there are some OMAP based derivative SoCs like AM33XX which doesn't have 32K sync-timer hardware IP and need to fallback on 32KHz based gptimer clocksource.
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 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 9 lines

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

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

Cc Santosh Shilimkar <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Benoit Cousson <[email protected]>

Cc Tony Lindgren <[email protected]>

Cc Paul Walmsley <[email protected]>

Cc Tarun Kanti DebBarma <[email protected]>

Cc Ming Lei <[email protected]>

This fails to boot on my Mistral am37x-evm with omap2plus_defconfig

Quoted 47 lines · Russ Dill

Quoted 45 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. Option 3 will still work but it is no better than 32K sync-timer based clocksource.

So ideally we can kill the gptimer based clocksource option but there are some OMAP based derivative SoCs like AM33XX which doesn't have 32K sync-timer hardware IP and need to fallback on 32KHz based gptimer clocksource.
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 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]>

Cc Santosh Shilimkar <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Benoit Cousson <[email protected]>

Cc Tony Lindgren <[email protected]>

Cc Paul Walmsley <[email protected]>

Cc Tarun Kanti DebBarma <[email protected]>

Cc Ming Lei <[email protected]>

Quoted 2 lines

This fails to boot on my Mistral am37x-evm with omap2plus_defconfig

Thanks Russ, for validating it.

But I do not see any relation between your boot process stuck and this patch. What is the observation without these patches?

Quoted 51 lines · Vaibhav

Quoted 49 lines · Russ Dill

Quoted 36 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. Option 3 will still work but it is no better than 32K sync-timer based clocksource.

So ideally we can kill the gptimer based clocksource option but there are some OMAP based derivative SoCs like AM33XX which doesn't have 32K sync-timer hardware IP and need to fallback on 32KHz based gptimer clocksource.
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 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 9 lines

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

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

Cc Santosh Shilimkar <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Benoit Cousson <[email protected]>

Cc Tony Lindgren <[email protected]>

Cc Paul Walmsley <[email protected]>

Cc Tarun Kanti DebBarma <[email protected]>

Cc Ming Lei <[email protected]>

This fails to boot on my Mistral am37x-evm with omap2plus_defconfig

Quoted 4 lines

Thanks Russ, for validating it.

But I do not see any relation between your boot process stuck and this patch. What is the observation without these patches?

With no patches applied it boots, with 1/3 applied it boots, with 2/3 applied it boots, with 3/3 applied it gets hung. I also tested on my beagleboard b4, that booted fine.

Quoted 57 lines · Russ Dill

Quoted 55 lines · Vaibhav

Quoted 47 lines · Russ Dill

Quoted 36 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. Option 3 will still work but it is no better than 32K sync-timer based clocksource.

So ideally we can kill the gptimer based clocksource option but there are some OMAP based derivative SoCs like AM33XX which doesn't have 32K sync-timer hardware IP and need to fallback on 32KHz based gptimer clocksource.
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 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 9 lines

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

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

Cc Santosh Shilimkar <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Benoit Cousson <[email protected]>

Cc Tony Lindgren <[email protected]>

Cc Paul Walmsley <[email protected]>

Cc Tarun Kanti DebBarma <[email protected]>

Cc Ming Lei <[email protected]>

Quoted 2 lines

This fails to boot on my Mistral am37x-evm with omap2plus_defconfig

Thanks Russ, for validating it.

But I do not see any relation between your boot process stuck and this patch. What is the observation without these patches?

Quoted 4 lines

With no patches applied it boots, with 1/3 applied it boots, with 2/3 applied it boots, with 3/3 applied it gets hung. I also tested on my beagleboard b4, that booted fine.

You mean to say, on Beagleboard it boots up fine. And same fails on EVM?

Quoted 13 lines · Vaibhav

Quoted 12 lines · Russ Dill

Quoted 11 lines · Vaibhav

Quoted 10 lines · Russ Dill

Quoted 9 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.

Quoted 2 lines

You mean to say, on Beagleboard it boots up fine. And same fails on EVM?

yes

Quoted 14 lines · Russ Dill

Quoted 13 lines · Vaibhav

Quoted 12 lines · Russ Dill

Quoted 11 lines · Vaibhav

Quoted 10 lines · Russ Dill

Quoted 9 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.

<Snip>

Quoted 6 lines

Quoted 2 lines

You mean to say, on Beagleboard it boots up fine. And same fails on EVM?

yes

I am able to reproduce this issue, and it seems the issue is really weird and I am completely clueless here.

With below patch/change it always works, 100% success. The moment I add 0x10 offset to the ioremaped virtual base address, gone...95% of the time it fails.
I have observed that, the execution always stays at wfi (default_idle) thread. Really strange behavior...and I am clueless now.

- The 32k sync timer address space is part of iotable.
- The ioremap() internally masks the addr to page aligned addr and gets pfn, At the end, offset is being added. So I don't really understand how is it different than adding offset before and after ioremap().

Also, this issue is nothing to do with my patch, it also happens with the original code.
Just add 0x10 offset after ioremap(), and booooooom.... (95% of the time)

----------Patch which works (on top of this patch-series)----------

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

index 3e3cdab..b0ef8ce 100644

Hi Vaibhav, Russ,

Quoted 45 lines · Vaibhav

Quoted 14 lines · Russ Dill

Quoted 13 lines · Vaibhav

Quoted 12 lines · Russ Dill

Quoted 11 lines · Vaibhav

Quoted 10 lines · Russ Dill

Quoted 9 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.

<Snip>

Quoted 6 lines

Quoted 2 lines

You mean to say, on Beagleboard it boots up fine. And same fails on EVM?

yes

I am able to reproduce this issue, and it seems the issue is really weird and I am completely clueless here.

With below patch/change it always works, 100% success. The moment I add 0x10 offset to the ioremaped virtual base address, gone...95% of the time it fails.
I have observed that, the execution always stays at wfi (default_idle) thread. Really strange behavior...and I am clueless now.

- The 32k sync timer address space is part of iotable.
- The ioremap() internally masks the addr to page aligned addr and gets pfn, At the end, offset is being added. So I don't really understand how is it different than adding offset before and after ioremap().

Also, this issue is nothing to do with my patch, it also happens with the original code.
Just add 0x10 offset after ioremap(), and booooooom.... (95% of the time)

----------Patch which works (on top of this patch-series)----------

arch/arm/plat-omap/counter_32k.c int __init omap_init_clocksource_32k(u32 pbase, unsigned long size) · line 94

Quoted 10 lines

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

index 3e3cdab..b0ef8ce 100644
@@ -90,6 +90,7 @@ int __init omap_init_clocksource_32k(u32 pbase, unsigned long size)
         * For this to work we must have a static mapping in io.c
         * for this area
         */
+       pbase = pbase + OMAP2_32KSYNCNT_CR_OFF;
        base = ioremap(pbase, size);

What is size set to in the failing case on the am37xx device? Any chance we are not mapping enough space?

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).

arch/arm/plat-omap/counter_32k.c +6 -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;

Quoted 3 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).

[...]

Quoted 11 lines

arch/arm/plat-omap/counter_32k.c +11 -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;
+
+	/*
+	 * 120000 rough estimate from the calculations in
+	 * __clocksource_updatefreq_scale.
+	 */

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

Quoted 5 lines

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

+	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);

After discussing this a bit with Vaibhav offline, we found that 'base' here should be sync32k_cnt_reg.

Can you guys that were seeing the hang confirm that that fixes the issue?

If so, please reply with your Tested-by.

Thanks,

Kevin
--
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 3 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).

[...]

Quoted 6 lines

(quoted diff) +6 -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;

Review

Quoted 10 lines

(quoted diff) +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);

After discussing this a bit with Vaibhav offline, we found that 'base' here should be sync32k_cnt_reg.

Can you guys that were seeing the hang confirm that that fixes the issue?

If so, please reply with your Tested-by.

Quoted 3 lines · Kevin Hilman

Vaibhav Hiremath <[email protected]> writes:

Quoted 1 line

If so, please reply with your Tested-by.

Tested-by Russ Dill <[email protected]>

All three patches in series boot tested on:
Beagleboard xM
Beagleboard B4
Mistral AM37X-EVM

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)

This patch series cleans up the existing 32k-sync timer implementation without any major code changes, uses kernel parameter to override the default clocksource of "counter_32k", also in order to support some OMAP based derivative SoCs like AM33XX which doesn't have 32K sync-timer hardware IP, adds hwmod lookup for omap2+ devices, and if lookup fails then fall back to gp-timer.

if(use_gptimer_clksrc == true)
? ? ? ?gptimer clocksource init;
else if (counter_32 init == false)
? ? ? ?/* Fallback to gptimer */
? ? ? ?gptimer clocksource init(;

With this, we should be able to support multi-omap boot including devices with/without 32k-sync timer.

This patch-series has been boot tested on AM37xEVM platform, it would be helpful if somebody help me to validate it on OMAP1/2 platforms.

The patches are also available at -
https://github.com/hvaibhav/am335x-linux ? 32ksync-timer-cleanup

History:
========
Changes from V3:
http://www.mail-archive.com/linux-omap at vger.kernel.org/msg66462.html ? ? ? ?- Fixed all review comments from Kevin H
? ? ? ? ? ? ? ?* Moved counter_32k CR register offset handling to ? ? ? ? ? ? ? ? ?counter_32k.c file, so now, calling funtion don't have ? ? ? ? ? ? ? ? ?to maintain or add offset to base addr.
? ? ? ? ? ? ? ?* Added comment for funtion omap_init_clocksource_32k() ? ? ? ? ? ? ? ?* Used resource_size() for calculate size
? ? ? ? ? ? ? ?* Convert WARN_ON to pr_warn

Quoted 27 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)

This patch series cleans up the existing 32k-sync timer implementation without any major code changes, uses kernel parameter to override the default clocksource of "counter_32k", also in order to support some OMAP based derivative SoCs like AM33XX which doesn't have 32K sync-timer hardware IP, adds hwmod lookup for omap2+ devices, and if lookup fails then fall back to gp-timer.

if(use_gptimer_clksrc == true)
? ? ? ?gptimer clocksource init;
else if (counter_32 init == false)
? ? ? ?/* Fallback to gptimer */
? ? ? ?gptimer clocksource init(;

With this, we should be able to support multi-omap boot including devices with/without 32k-sync timer.

This patch-series has been boot tested on AM37xEVM platform, it would be helpful if somebody help me to validate it on OMAP1/2 platforms.

The patches are also available at -
https://github.com/hvaibhav/am335x-linux ? 32ksync-timer-cleanup

Quoted 12 lines

History:
========
Changes from V3:
http://www.mail-archive.com/linux-omap at vger.kernel.org/msg66462.html ? ? ? ?- Fixed all review comments from Kevin H
? ? ? ? ? ? ? ?* Moved counter_32k CR register offset handling to ? ? ? ? ? ? ? ? ?counter_32k.c file, so now, calling funtion don't have ? ? ? ? ? ? ? ? ?to maintain or add offset to base addr.
? ? ? ? ? ? ? ?* Added comment for funtion omap_init_clocksource_32k() ? ? ? ? ? ? ? ?* Used resource_size() for calculate size
? ? ? ? ? ? ? ?* Convert WARN_ON to pr_warn

Have given my reviewed-by on v2. Just quickly scanned the series based on above minor updates.
Feel free to add,