linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH-V3 0/3] ARM: OMAP: Make OMAP clocksource source selection runtime
@ 2012-04-13  9:39 Vaibhav Hiremath
  2012-04-13  9:39 ` [PATCH-V3 1/3] ARM: OMAP2/3: Add idle_st bits for ST_32KSYNC timer to prcm-common header Vaibhav Hiremath
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Vaibhav Hiremath @ 2012-04-13  9:39 UTC (permalink / raw)
  To: linux-omap
  Cc: tony, khilman, paul, rnayak, b-cousson, linux-arm-kernel,
	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.

History:
========
Changes from V2:
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-March/092037.html
        - Added early_param support to read clocksource selection
	  from user through kernel parameter ("clocksource=")
	- Converted to ocp_if changes from Paul

Changes from V1:
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-January/081037.html
        - Based on Tony's comment, added pbase & size argument to
	  omap_init_clocksource_32k(), to avoid cpu_is_xxx() check.
	- Added commit description based on discussion on list
          (Thanks to Santosh here)
	- Reorder patch sequence


Vaibhav Hiremath (3):
  ARM: OMAP2/3: Add idle_st bits for ST_32KSYNC timer to prcm-common
    header
  ARM: OMAP2/3: hwmod data: Add 32k-sync timer data to hwmod database
  ARM: OMAP: Make OMAP clocksource source selection using kernel param

 arch/arm/mach-omap1/timer32k.c                     |    6 +-
 arch/arm/mach-omap2/omap_hwmod_2420_data.c         |   19 ++++
 arch/arm/mach-omap2/omap_hwmod_2430_data.c         |   19 ++++
 arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c |   19 ++++
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c         |   54 +++++++++++
 arch/arm/mach-omap2/omap_hwmod_common_data.h       |    1 +
 arch/arm/mach-omap2/prcm-common.h                  |    4 +
 arch/arm/mach-omap2/timer.c                        |  100 +++++++++++++++-----
 arch/arm/plat-omap/counter_32k.c                   |   85 +++++++----------
 arch/arm/plat-omap/include/plat/common.h           |    2 +-
 10 files changed, 236 insertions(+), 73 deletions(-)


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH-V3 1/3] ARM: OMAP2/3: Add idle_st bits for ST_32KSYNC timer to prcm-common header
  2012-04-13  9:39 [PATCH-V3 0/3] ARM: OMAP: Make OMAP clocksource source selection runtime Vaibhav Hiremath
@ 2012-04-13  9:39 ` Vaibhav Hiremath
  2012-04-13  9:39 ` [PATCH-V3 2/3] ARM: OMAP2/3: hwmod data: Add 32k-sync timer data to hwmod database Vaibhav Hiremath
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Vaibhav Hiremath @ 2012-04-13  9:39 UTC (permalink / raw)
  To: linux-omap
  Cc: tony, khilman, paul, rnayak, b-cousson, linux-arm-kernel,
	Vaibhav Hiremath, Felipe Balbi

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

Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
Cc: Felipe Balbi <balbi@ti.com>
Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/prcm-common.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/prcm-common.h b/arch/arm/mach-omap2/prcm-common.h
index 5aa5435..29955d5 100644
--- a/arch/arm/mach-omap2/prcm-common.h
+++ b/arch/arm/mach-omap2/prcm-common.h
@@ -177,6 +177,8 @@
 /* PM_WKST_WKUP, CM_IDLEST_WKUP shared bits */
 #define OMAP24XX_ST_GPIOS_SHIFT				2
 #define OMAP24XX_ST_GPIOS_MASK				(1 << 2)
+#define OMAP24XX_ST_32KSYNC_SHIFT			1
+#define OMAP24XX_ST_32KSYNC_MASK			(1 << 1)
 #define OMAP24XX_ST_GPT1_SHIFT				0
 #define OMAP24XX_ST_GPT1_MASK				(1 << 0)

@@ -307,6 +309,8 @@
 #define OMAP3430_ST_SR1_MASK				(1 << 6)
 #define OMAP3430_ST_GPIO1_SHIFT				3
 #define OMAP3430_ST_GPIO1_MASK				(1 << 3)
+#define OMAP3430_ST_32KSYNC_SHIFT			2
+#define OMAP3430_ST_32KSYNC_MASK			(1 << 2)
 #define OMAP3430_ST_GPT12_SHIFT				1
 #define OMAP3430_ST_GPT12_MASK				(1 << 1)
 #define OMAP3430_ST_GPT1_SHIFT				0
--
1.7.0.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH-V3 2/3] ARM: OMAP2/3: hwmod data: Add 32k-sync timer data to hwmod database
  2012-04-13  9:39 [PATCH-V3 0/3] ARM: OMAP: Make OMAP clocksource source selection runtime Vaibhav Hiremath
  2012-04-13  9:39 ` [PATCH-V3 1/3] ARM: OMAP2/3: Add idle_st bits for ST_32KSYNC timer to prcm-common header Vaibhav Hiremath
@ 2012-04-13  9:39 ` Vaibhav Hiremath
  2012-04-13  9:39 ` [PATCH-V3 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param Vaibhav Hiremath
  2012-04-16  8:40 ` [PATCH-V3 0/3] ARM: OMAP: Make OMAP clocksource source selection runtime Santosh Shilimkar
  3 siblings, 0 replies; 8+ messages in thread
From: Vaibhav Hiremath @ 2012-04-13  9:39 UTC (permalink / raw)
  To: linux-omap
  Cc: tony, khilman, paul, rnayak, b-cousson, linux-arm-kernel,
	Vaibhav Hiremath, Felipe Balbi

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

Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Paul Walmsley <paul@pwsan.com>
---
This patch has been updated/migrated from last couple of revisions,
as recently Paul had submitted hwmod clean-up patches, which
changes the hwmod registration mechanism from
omap_hwmod => omap_hwmod_ocp_if.

 arch/arm/mach-omap2/omap_hwmod_2420_data.c         |   19 +++++++
 arch/arm/mach-omap2/omap_hwmod_2430_data.c         |   19 +++++++
 arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c |   19 +++++++
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c         |   54 ++++++++++++++++++++
 arch/arm/mach-omap2/omap_hwmod_common_data.h       |    1 +
 5 files changed, 112 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_2420_data.c b/arch/arm/mach-omap2/omap_hwmod_2420_data.c
index 2c087ff..b961b0d 100644
--- a/arch/arm/mach-omap2/omap_hwmod_2420_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_2420_data.c
@@ -428,6 +428,24 @@ static struct omap_hwmod_ocp_if omap2420_l4_core__mcbsp2 = {
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };

+/* l4_wkup -> 32ksync_counter */
+static struct omap_hwmod_addr_space omap2420_counter_32k_addrs[] = {
+	{
+		.pa_start	= 0x48004000,
+		.pa_end		= 0x4800401f,
+		.flags		= ADDR_TYPE_RT
+	},
+	{ }
+};
+
+static struct omap_hwmod_ocp_if omap2420_l4_wkup__counter_32k = {
+	.master		= &omap2xxx_l4_wkup_hwmod,
+	.slave		= &omap2xxx_counter_32k_hwmod,
+	.clk		= "sync_32k_ick",
+	.addr		= omap2420_counter_32k_addrs,
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
 static struct omap_hwmod_ocp_if *omap2420_hwmod_ocp_ifs[] __initdata = {
 	&omap2xxx_l3_main__l4_core,
 	&omap2xxx_mpu__l3_main,
@@ -468,6 +486,7 @@ static struct omap_hwmod_ocp_if *omap2420_hwmod_ocp_ifs[] __initdata = {
 	&omap2420_l4_core__mailbox,
 	&omap2420_l4_core__mcbsp1,
 	&omap2420_l4_core__mcbsp2,
+	&omap2420_l4_wkup__counter_32k,
 	NULL,
 };

diff --git a/arch/arm/mach-omap2/omap_hwmod_2430_data.c b/arch/arm/mach-omap2/omap_hwmod_2430_data.c
index b359bfb..c658ccd 100644
--- a/arch/arm/mach-omap2/omap_hwmod_2430_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_2430_data.c
@@ -838,6 +838,24 @@ static struct omap_hwmod_ocp_if omap2430_l4_core__mcbsp5 = {
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };

+/* l4_wkup -> 32ksync_counter */
+static struct omap_hwmod_addr_space omap2430_counter_32k_addrs[] = {
+	{
+		.pa_start	= 0x49020000,
+		.pa_end		= 0x4902001f,
+		.flags		= ADDR_TYPE_RT
+	},
+	{ }
+};
+
+static struct omap_hwmod_ocp_if omap2430_l4_wkup__counter_32k = {
+	.master		= &omap2xxx_l4_wkup_hwmod,
+	.slave		= &omap2xxx_counter_32k_hwmod,
+	.clk		= "sync_32k_ick",
+	.addr		= omap2430_counter_32k_addrs,
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
 static struct omap_hwmod_ocp_if *omap2430_hwmod_ocp_ifs[] __initdata = {
 	&omap2xxx_l3_main__l4_core,
 	&omap2xxx_mpu__l3_main,
@@ -886,6 +904,7 @@ static struct omap_hwmod_ocp_if *omap2430_hwmod_ocp_ifs[] __initdata = {
 	&omap2430_l4_core__mcbsp3,
 	&omap2430_l4_core__mcbsp4,
 	&omap2430_l4_core__mcbsp5,
+	&omap2430_l4_wkup__counter_32k,
 	NULL,
 };

diff --git a/arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c b/arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c
index 45aaa07..8c37cb5 100644
--- a/arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c
@@ -732,3 +732,22 @@ struct omap_hwmod omap2xxx_mcspi2_hwmod = {
 	.class		= &omap2xxx_mcspi_class,
 	.dev_attr	= &omap_mcspi2_dev_attr,
 };
+
+static struct omap_hwmod_class omap2xxx_counter_hwmod_class = {
+	.name	= "counter",
+};
+
+struct omap_hwmod omap2xxx_counter_32k_hwmod = {
+	.name		= "counter_32k",
+	.main_clk	= "func_32k_ck",
+	.prcm		= {
+		.omap2	= {
+			.module_offs = WKUP_MOD,
+			.prcm_reg_id = 1,
+			.module_bit = OMAP24XX_ST_32KSYNC_SHIFT,
+			.idlest_reg_id = 1,
+			.idlest_idle_bit = OMAP24XX_ST_32KSYNC_SHIFT,
+		},
+	},
+	.class		= &omap2xxx_counter_hwmod_class,
+};
diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index e6d944e..b3e4afb 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -1981,6 +1981,40 @@ static struct omap_hwmod omap3xxx_usb_tll_hs_hwmod = {
 };

 /*
+ * '32K sync counter' class
+ * 32-bit ordinary counter, clocked by the falling edge of the 32 khz clock
+ */
+static struct omap_hwmod_class_sysconfig omap3xxx_counter_sysc = {
+	.rev_offs	= 0x0000,
+	.sysc_offs	= 0x0004,
+	.sysc_flags	= SYSC_HAS_SIDLEMODE,
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO),
+	.sysc_fields	= &omap_hwmod_sysc_type1,
+};
+
+static struct omap_hwmod_class omap3xxx_counter_hwmod_class = {
+	.name	= "counter",
+	.sysc	= &omap3xxx_counter_sysc,
+};
+
+static struct omap_hwmod omap3xxx_counter_32k_hwmod = {
+	.name		= "counter_32k",
+	.class		= &omap3xxx_counter_hwmod_class,
+	.clkdm_name	= "wkup_clkdm",
+	.flags		= HWMOD_SWSUP_SIDLE,
+	.main_clk	= "wkup_32k_fck",
+	.prcm		= {
+		.omap2	= {
+			.module_offs = WKUP_MOD,
+			.prcm_reg_id = 1,
+			.module_bit = OMAP3430_ST_32KSYNC_SHIFT,
+			.idlest_reg_id = 1,
+			.idlest_idle_bit = OMAP3430_ST_32KSYNC_SHIFT,
+		},
+	},
+};
+
+/*
  * interfaces
  */

@@ -3059,6 +3093,25 @@ static struct omap_hwmod_ocp_if omap3xxx_l4_core__usb_tll_hs = {
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };

+/* l4_wkup -> 32ksync_counter */
+static struct omap_hwmod_addr_space omap3xxx_counter_32k_addrs[] = {
+	{
+		.pa_start	= 0x48320000,
+		.pa_end		= 0x4832001f,
+		.flags		= ADDR_TYPE_RT
+	},
+	{ }
+};
+
+static struct omap_hwmod_ocp_if omap3xxx_l4_wkup__counter_32k = {
+	.master		= &omap3xxx_l4_wkup_hwmod,
+	.slave		= &omap3xxx_counter_32k_hwmod,
+	.clk		= "omap_32ksync_ick",
+	.addr		= omap3xxx_counter_32k_addrs,
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+
 static struct omap_hwmod_ocp_if *omap3xxx_hwmod_ocp_ifs[] __initdata = {
 	&omap3xxx_l3_main__l4_core,
 	&omap3xxx_l3_main__l4_per,
@@ -3103,6 +3156,7 @@ static struct omap_hwmod_ocp_if *omap3xxx_hwmod_ocp_ifs[] __initdata = {
 	&omap34xx_l4_core__mcspi2,
 	&omap34xx_l4_core__mcspi3,
 	&omap34xx_l4_core__mcspi4,
+	&omap3xxx_l4_wkup__counter_32k,
 	NULL,
 };

diff --git a/arch/arm/mach-omap2/omap_hwmod_common_data.h b/arch/arm/mach-omap2/omap_hwmod_common_data.h
index 7aa9156..fd4fc62 100644
--- a/arch/arm/mach-omap2/omap_hwmod_common_data.h
+++ b/arch/arm/mach-omap2/omap_hwmod_common_data.h
@@ -74,6 +74,7 @@ extern struct omap_hwmod omap2xxx_gpio3_hwmod;
 extern struct omap_hwmod omap2xxx_gpio4_hwmod;
 extern struct omap_hwmod omap2xxx_mcspi1_hwmod;
 extern struct omap_hwmod omap2xxx_mcspi2_hwmod;
+extern struct omap_hwmod omap2xxx_counter_32k_hwmod;

 /* Common interface data across OMAP2xxx */
 extern struct omap_hwmod_ocp_if omap2xxx_l3_main__l4_core;
--
1.7.0.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH-V3 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
  2012-04-13  9:39 [PATCH-V3 0/3] ARM: OMAP: Make OMAP clocksource source selection runtime Vaibhav Hiremath
  2012-04-13  9:39 ` [PATCH-V3 1/3] ARM: OMAP2/3: Add idle_st bits for ST_32KSYNC timer to prcm-common header Vaibhav Hiremath
  2012-04-13  9:39 ` [PATCH-V3 2/3] ARM: OMAP2/3: hwmod data: Add 32k-sync timer data to hwmod database Vaibhav Hiremath
@ 2012-04-13  9:39 ` Vaibhav Hiremath
  2012-04-20  0:34   ` Kevin Hilman
  2012-04-16  8:40 ` [PATCH-V3 0/3] ARM: OMAP: Make OMAP clocksource source selection runtime Santosh Shilimkar
  3 siblings, 1 reply; 8+ messages in thread
From: Vaibhav Hiremath @ 2012-04-13  9:39 UTC (permalink / raw)
  To: linux-omap
  Cc: tony, khilman, paul, rnayak, b-cousson, linux-arm-kernel,
	Vaibhav Hiremath, Felipe Balbi, Santosh Shilimkar,
	Tarun Kanti DebBarma, Ming Lei

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 <hvaibhav@ti.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
CC: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Cc: Ming Lei <tom.leiming@gmail.com>
---
 arch/arm/mach-omap1/timer32k.c           |    6 ++-
 arch/arm/mach-omap2/timer.c              |  100 +++++++++++++++++++++++-------
 arch/arm/plat-omap/counter_32k.c         |   85 +++++++++++---------------
 arch/arm/plat-omap/include/plat/common.h |    2 +-
 4 files changed, 120 insertions(+), 73 deletions(-)

diff --git a/arch/arm/mach-omap1/timer32k.c b/arch/arm/mach-omap1/timer32k.c
index 325b9a0..1c164cd 100644
--- a/arch/arm/mach-omap1/timer32k.c
+++ b/arch/arm/mach-omap1/timer32k.c
@@ -71,6 +71,7 @@

 /* 16xx specific defines */
 #define OMAP1_32K_TIMER_BASE		0xfffb9000
+#define OMAP1_32KSYNC_TIMER_BASE	0xfffbc410
 #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();

 	return true;
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index ecec873..cde80d4 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -71,6 +71,8 @@

 /* MAX_GPTIMER_ID: number of GPTIMERs on the chip */
 #define MAX_GPTIMER_ID		12
+/* OMAP2_32KSYNCNT_CR_OFF: offset of 32ksyn counter register */
+#define OMAP2_32KSYNCNT_CR_OFF	0x10

 static u32 sys_timer_reserved;

@@ -243,22 +245,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 +273,32 @@ 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 res;
+	u32 pbase, size;
+	struct omap_hwmod *oh;
+	struct resource mem_rsrc;
+	const char *oh_name = "counter_32k";
+
+	oh = omap_hwmod_lookup(oh_name);
+	if (!oh || oh->slaves_cnt == 0)
+		return -ENODEV;
+
+	res = omap_hwmod_get_resource_byname(oh, IORESOURCE_MEM,
+				NULL, &mem_rsrc);
+	if (!res) {
+		pbase = mem_rsrc.start + OMAP2_32KSYNCNT_CR_OFF;
+		size = mem_rsrc.end - mem_rsrc.start;
+
+		res = omap_init_clocksource_32k(pbase, size);
+	}
+
+	WARN_ON(res);
+	return res;
+}
+
+static void __init omap2_gptimer_clocksource_init(int gptimer_id,
 						const char *fck_source)
 {
 	int res;
@@ -293,9 +306,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 +313,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 +373,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) {
@@ -510,3 +541,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);
diff --git a/arch/arm/plat-omap/counter_32k.c b/arch/arm/plat-omap/counter_32k.c
index 5068fe5..bd4a3fd 100644
--- a/arch/arm/plat-omap/counter_32k.c
+++ b/arch/arm/plat-omap/counter_32k.c
@@ -35,8 +35,6 @@
  */
 static void __iomem *timer_32k_base;

-#define OMAP16XX_TIMER_32K_SYNCHRONIZED		0xfffbc410
-
 static u32 notrace omap_32k_read_sched_clock(void)
 {
 	return timer_32k_base ? __raw_readl(timer_32k_base) : 0;
@@ -68,54 +66,43 @@ void read_persistent_clock(struct timespec *ts)
 	*ts = *tsp;
 }

-int __init omap_init_clocksource_32k(void)
+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);
+	void __iomem *base;
+	struct clk *sync_32k_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;
 	}
+
+	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))
+		pr_err("32k_counter: can't register clocksource\n");
+	else
+		pr_info("OMAP clocksource: 32k_counter at 32768 Hz\n");
+
+	setup_sched_clock(omap_32k_read_sched_clock, 32, 32768);
+
 	return 0;
 }
diff --git a/arch/arm/plat-omap/include/plat/common.h b/arch/arm/plat-omap/include/plat/common.h
index b4d7ec3..6cc1e43 100644
--- a/arch/arm/plat-omap/include/plat/common.h
+++ b/arch/arm/plat-omap/include/plat/common.h
@@ -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 omap_reserve(void);
 extern int omap_dss_reset(struct omap_hwmod *);
--
1.7.0.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH-V3 0/3] ARM: OMAP: Make OMAP clocksource source selection runtime
  2012-04-13  9:39 [PATCH-V3 0/3] ARM: OMAP: Make OMAP clocksource source selection runtime Vaibhav Hiremath
                   ` (2 preceding siblings ...)
  2012-04-13  9:39 ` [PATCH-V3 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param Vaibhav Hiremath
@ 2012-04-16  8:40 ` Santosh Shilimkar
  3 siblings, 0 replies; 8+ messages in thread
From: Santosh Shilimkar @ 2012-04-16  8:40 UTC (permalink / raw)
  To: Vaibhav Hiremath
  Cc: linux-omap, tony, khilman, paul, rnayak, b-cousson,
	linux-arm-kernel

Vaibhav,

On Friday 13 April 2012 03:09 PM, Vaibhav Hiremath wrote:
> 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.
> 
> History:
> ========
> Changes from V2:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-March/092037.html
>         - Added early_param support to read clocksource selection
> 	  from user through kernel parameter ("clocksource=")
> 	- Converted to ocp_if changes from Paul
> 
> Changes from V1:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-January/081037.html
>         - Based on Tony's comment, added pbase & size argument to
> 	  omap_init_clocksource_32k(), to avoid cpu_is_xxx() check.
> 	- Added commit description based on discussion on list
>           (Thanks to Santosh here)
> 	- Reorder patch sequence
> 
> 
> Vaibhav Hiremath (3):
>   ARM: OMAP2/3: Add idle_st bits for ST_32KSYNC timer to prcm-common
>     header
>   ARM: OMAP2/3: hwmod data: Add 32k-sync timer data to hwmod database
>   ARM: OMAP: Make OMAP clocksource source selection using kernel param
> 
This versions of series seems to fine with me.
Feel free to add,
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

Regards
Santosh



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH-V3 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
  2012-04-13  9:39 ` [PATCH-V3 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param Vaibhav Hiremath
@ 2012-04-20  0:34   ` Kevin Hilman
  2012-04-23 17:35     ` Hiremath, Vaibhav
  0 siblings, 1 reply; 8+ messages in thread
From: Kevin Hilman @ 2012-04-20  0:34 UTC (permalink / raw)
  To: Vaibhav Hiremath
  Cc: linux-omap, tony, paul, rnayak, b-cousson, linux-arm-kernel,
	Felipe Balbi, Santosh Shilimkar, Tarun Kanti DebBarma, Ming Lei

Vaibhav Hiremath <hvaibhav@ti.com> writes:

> 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 <hvaibhav@ti.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> CC: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Benoit Cousson <b-cousson@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
> Cc: Ming Lei <tom.leiming@gmail.com>

[...]

> @@ -285,7 +273,32 @@ 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 res;
> +	u32 pbase, size;
> +	struct omap_hwmod *oh;
> +	struct resource mem_rsrc;

minor nit: personaly, I find 'rsrc' hard on the eyes.  I suggest just
using 'res' for this variable name.

> +	const char *oh_name = "counter_32k";
> +	oh = omap_hwmod_lookup(oh_name);
> +	if (!oh || oh->slaves_cnt == 0)
> +		return -ENODEV;
> +
> +	res = omap_hwmod_get_resource_byname(oh, IORESOURCE_MEM,
> +				NULL, &mem_rsrc);
> +	if (!res) {
> +		pbase = mem_rsrc.start + OMAP2_32KSYNCNT_CR_OFF;
> +		size = mem_rsrc.end - mem_rsrc.start;

This should be (end - start + 1), but instead, use the resource_size()
helper should be used for this.

Also note that because you're adding an offset of 0x10 to the start, the
ioremap later is actually mapping 0x10 past the end.  More on the base
address later...

> +
> +		res = omap_init_clocksource_32k(pbase, size);
> +	}
> +
> +	WARN_ON(res);

Just use pr_warn() here instead of WARN_ON() which will dump a full
backtrace, and isn't necessary.

[...]

> @@ -510,3 +541,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.
> + */

Are you sure the clocksource= setup function is getting called in
kernel/time/clocksource.c as well?  IOW, do both the generic one and
this one get called?

To be honest, I still don't like this.  The clocksource core will
already sets the right clocksource, and this is duplicating that.

I do see why you're doing this though because both clocksource init
functions call setup_sched_clock().

Ideally, we would just wait to call setup_sched_clock() until later
(possibly in a late_initcall) after the clocksource selection is
decided.   I'm not sure we have the right hooks to do that though.

Possibly by setting a flag in the ->enable method of each clocksource,
and checking that in a late_initcall before calling setup_sched_clock.
That would be easy for the GP timer, but the 32k timer is setting up an
mmio clocksource, so we don't have an ->enable method.

Any other ideas?

[...]

> diff --git a/arch/arm/plat-omap/counter_32k.c b/arch/arm/plat-omap/counter_32k.c
> index 5068fe5..bd4a3fd 100644
> --- a/arch/arm/plat-omap/counter_32k.c
> +++ b/arch/arm/plat-omap/counter_32k.c
> @@ -35,8 +35,6 @@
>   */
>  static void __iomem *timer_32k_base;
>
> -#define OMAP16XX_TIMER_32K_SYNCHRONIZED		0xfffbc410
> -
>  static u32 notrace omap_32k_read_sched_clock(void)
>  {
>  	return timer_32k_base ? __raw_readl(timer_32k_base) : 0;
> @@ -68,54 +66,43 @@ void read_persistent_clock(struct timespec *ts)
>  	*ts = *tsp;
>  }
>
> -int __init omap_init_clocksource_32k(void)
> +int __init omap_init_clocksource_32k(u32 pbase, unsigned long size)

Now that this takes some arguments, this function wants some kerneldoc.

In particular, you should document that 'pbase' is the address of the
counter register, not the base of the IP.

That being said, I think the pbase passed in should probably be the base
of the IP register, not the address of the counter register. It seems to
me that it's at offset 0x10 on all SoCs anyways.

Doing that will also ensure that the ioremap'd range covers the whole
IP, and not from the counter register to 0x10 past the end.

Kevin

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH-V3 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
  2012-04-20  0:34   ` Kevin Hilman
@ 2012-04-23 17:35     ` Hiremath, Vaibhav
  2012-04-24  0:32       ` Kevin Hilman
  0 siblings, 1 reply; 8+ messages in thread
From: Hiremath, Vaibhav @ 2012-04-23 17:35 UTC (permalink / raw)
  To: Hilman, Kevin
  Cc: linux-omap@vger.kernel.org, tony@atomide.com, paul@pwsan.com,
	Nayak, Rajendra, Cousson, Benoit,
	linux-arm-kernel@lists.infradead.org, Balbi, Felipe,
	Shilimkar, Santosh, DebBarma, Tarun Kanti, Ming Lei

On Fri, Apr 20, 2012 at 06:04:20, Hilman, Kevin wrote:
> Vaibhav Hiremath <hvaibhav@ti.com> writes:
> 

Thanks Kevin for the review comments and sorry for delayed response... 
Please find my response in-lined below -


> > 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 <hvaibhav@ti.com>
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > CC: Santosh Shilimkar <santosh.shilimkar@ti.com>
> > Cc: Benoit Cousson <b-cousson@ti.com>
> > Cc: Tony Lindgren <tony@atomide.com>
> > Cc: Paul Walmsley <paul@pwsan.com>
> > Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
> > Cc: Ming Lei <tom.leiming@gmail.com>
> 
> [...]
> 
> > @@ -285,7 +273,32 @@ 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 res;
> > +	u32 pbase, size;
> > +	struct omap_hwmod *oh;
> > +	struct resource mem_rsrc;
> 
> minor nit: personaly, I find 'rsrc' hard on the eyes.  I suggest just
> using 'res' for this variable name.
> 

Ok, I will change it accordingly.


> > +	const char *oh_name = "counter_32k";
> > +	oh = omap_hwmod_lookup(oh_name);
> > +	if (!oh || oh->slaves_cnt == 0)
> > +		return -ENODEV;
> > +
> > +	res = omap_hwmod_get_resource_byname(oh, IORESOURCE_MEM,
> > +				NULL, &mem_rsrc);
> > +	if (!res) {
> > +		pbase = mem_rsrc.start + OMAP2_32KSYNCNT_CR_OFF;
> > +		size = mem_rsrc.end - mem_rsrc.start;
> 
> This should be (end - start + 1), but instead, use the resource_size()
> helper should be used for this.
> 

Good point. I will change this for resource_size().


> Also note that because you're adding an offset of 0x10 to the start, the
> ioremap later is actually mapping 0x10 past the end.  More on the base
> address later...
> 

Comment on top of this code will enough, isn't it?


> > +
> > +		res = omap_init_clocksource_32k(pbase, size);
> > +	}
> > +
> > +	WARN_ON(res);
> 
> Just use pr_warn() here instead of WARN_ON() which will dump a full
> backtrace, and isn't necessary.
> 

Ok, will change it.

> [...]
> 
> > @@ -510,3 +541,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.
> > + */
> 
> Are you sure the clocksource= setup function is getting called in
> kernel/time/clocksource.c as well?  IOW, do both the generic one and
> this one get called?
> 

Yes, 100%.

> To be honest, I still don't like this.  The clocksource core will
> already sets the right clocksource, and this is duplicating that.
> 
> I do see why you're doing this though because both clocksource init
> functions call setup_sched_clock().
> 
> Ideally, we would just wait to call setup_sched_clock() until later
> (possibly in a late_initcall) after the clocksource selection is
> decided.   I'm not sure we have the right hooks to do that though.
> 
> Possibly by setting a flag in the ->enable method of each clocksource,
> and checking that in a late_initcall before calling setup_sched_clock.
> That would be easy for the GP timer, but the 32k timer is setting up an
> mmio clocksource, so we don't have an ->enable method.
> 
> Any other ideas?
> 

Based on all the discussion we had on this, I believe we do not have any 
other option here; this is the only solution we could come up with.


> [...]
> 
> > diff --git a/arch/arm/plat-omap/counter_32k.c b/arch/arm/plat-omap/counter_32k.c
> > index 5068fe5..bd4a3fd 100644
> > --- a/arch/arm/plat-omap/counter_32k.c
> > +++ b/arch/arm/plat-omap/counter_32k.c
> > @@ -35,8 +35,6 @@
> >   */
> >  static void __iomem *timer_32k_base;
> >
> > -#define OMAP16XX_TIMER_32K_SYNCHRONIZED		0xfffbc410
> > -
> >  static u32 notrace omap_32k_read_sched_clock(void)
> >  {
> >  	return timer_32k_base ? __raw_readl(timer_32k_base) : 0;
> > @@ -68,54 +66,43 @@ void read_persistent_clock(struct timespec *ts)
> >  	*ts = *tsp;
> >  }
> >
> > -int __init omap_init_clocksource_32k(void)
> > +int __init omap_init_clocksource_32k(u32 pbase, unsigned long size)
> 
> Now that this takes some arguments, this function wants some kerneldoc.
> 

Should I create new documentation inside Documentation/arm/OMAP/
with name "counter_32k"?

> In particular, you should document that 'pbase' is the address of the
> counter register, not the base of the IP.
> 
> That being said, I think the pbase passed in should probably be the base
> of the IP register, not the address of the counter register. It seems to
> me that it's at offset 0x10 on all SoCs anyways.
> 
> Doing that will also ensure that the ioremap'd range covers the whole
> IP, and not from the counter register to 0x10 past the end.
> 

Yeah, you are right, I can get rid of this offset and move it to 
counter_32k.c file.

Thanks,
Vaibhav

> Kevin
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH-V3 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
  2012-04-23 17:35     ` Hiremath, Vaibhav
@ 2012-04-24  0:32       ` Kevin Hilman
  0 siblings, 0 replies; 8+ messages in thread
From: Kevin Hilman @ 2012-04-24  0:32 UTC (permalink / raw)
  To: Hiremath, Vaibhav
  Cc: linux-omap@vger.kernel.org, tony@atomide.com, paul@pwsan.com,
	Nayak, Rajendra, Cousson, Benoit,
	linux-arm-kernel@lists.infradead.org, Balbi, Felipe,
	Shilimkar, Santosh, DebBarma, Tarun Kanti, Ming Lei

"Hiremath, Vaibhav" <hvaibhav@ti.com> writes:

> On Fri, Apr 20, 2012 at 06:04:20, Hilman, Kevin wrote:

[...]

>> Also note that because you're adding an offset of 0x10 to the start, the
>> ioremap later is actually mapping 0x10 past the end.  More on the base
>> address later...
>> 
>
> Comment on top of this code will enough, isn't it?
>

No.  ioremapping() the range that goes past the end of the device is
just wrong.  But if you fix the offset as described below, that would be
fine.

[...]

>> > @@ -68,54 +66,43 @@ void read_persistent_clock(struct timespec *ts)
>> >  	*ts = *tsp;
>> >  }
>> >
>> > -int __init omap_init_clocksource_32k(void)
>> > +int __init omap_init_clocksource_32k(u32 pbase, unsigned long size)
>> 
>> Now that this takes some arguments, this function wants some kerneldoc.
>> 
>
> Should I create new documentation inside Documentation/arm/OMAP/
> with name "counter_32k"?

by kerneldoc, I mean comments in the code, not in Documenation.

IOW, just add a comment before this function in the code, but make sure
it is of the format described in Documentation/kernel-doc-nano-HOWTO.txt.

>> In particular, you should document that 'pbase' is the address of the
>> counter register, not the base of the IP.
>> 
>> That being said, I think the pbase passed in should probably be the base
>> of the IP register, not the address of the counter register. It seems to
>> me that it's at offset 0x10 on all SoCs anyways.
>> 
>> Doing that will also ensure that the ioremap'd range covers the whole
>> IP, and not from the counter register to 0x10 past the end.
>> 
>
> Yeah, you are right, I can get rid of this offset and move it to 
> counter_32k.c file.

Perfect.

Kevin


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2012-04-24  0:32 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-13  9:39 [PATCH-V3 0/3] ARM: OMAP: Make OMAP clocksource source selection runtime Vaibhav Hiremath
2012-04-13  9:39 ` [PATCH-V3 1/3] ARM: OMAP2/3: Add idle_st bits for ST_32KSYNC timer to prcm-common header Vaibhav Hiremath
2012-04-13  9:39 ` [PATCH-V3 2/3] ARM: OMAP2/3: hwmod data: Add 32k-sync timer data to hwmod database Vaibhav Hiremath
2012-04-13  9:39 ` [PATCH-V3 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param Vaibhav Hiremath
2012-04-20  0:34   ` Kevin Hilman
2012-04-23 17:35     ` Hiremath, Vaibhav
2012-04-24  0:32       ` Kevin Hilman
2012-04-16  8:40 ` [PATCH-V3 0/3] ARM: OMAP: Make OMAP clocksource source selection runtime Santosh Shilimkar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).