All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH-V5 0/3] ARM: OMAP: Make OMAP clocksource source selection runtime
@ 2012-04-25 13:58 ` Vaibhav Hiremath
  0 siblings, 0 replies; 30+ messages in thread
From: Vaibhav Hiremath @ 2012-04-25 13:58 UTC (permalink / raw)
  To: linux-omap
  Cc: tony, khilman, paul, santosh.shilimkar, 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.

The patches are also available at (based on linux-omap/master) -
https://github.com/hvaibhav/am335x-linux   32ksync-timer-cleanup

History:
========
Changes from V4 (No Code Change at all):
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg67019.html
	- Updated commit description as per Tony's comment

Changes from V3:
http://www.mail-archive.com/linux-omap@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 function omap_init_clocksource_32k()
		* Used resource_size() for calculate size
		* Convert WARN_ON to pr_warn

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                        |   99 ++++++++++++++----
 arch/arm/plat-omap/counter_32k.c                   |  108 ++++++++++----------
 arch/arm/plat-omap/include/plat/common.h           |    2 +-
 10 files changed, 254 insertions(+), 77 deletions(-)


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

* [PATCH-V5 0/3] ARM: OMAP: Make OMAP clocksource source selection runtime
@ 2012-04-25 13:58 ` Vaibhav Hiremath
  0 siblings, 0 replies; 30+ messages in thread
From: Vaibhav Hiremath @ 2012-04-25 13:58 UTC (permalink / raw)
  To: linux-arm-kernel

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 (based on linux-omap/master) -
https://github.com/hvaibhav/am335x-linux   32ksync-timer-cleanup

History:
========
Changes from V4 (No Code Change at all):
http://www.mail-archive.com/linux-omap at vger.kernel.org/msg67019.html
	- Updated commit description as per Tony's comment

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 function omap_init_clocksource_32k()
		* Used resource_size() for calculate size
		* Convert WARN_ON to pr_warn

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                        |   99 ++++++++++++++----
 arch/arm/plat-omap/counter_32k.c                   |  108 ++++++++++----------
 arch/arm/plat-omap/include/plat/common.h           |    2 +-
 10 files changed, 254 insertions(+), 77 deletions(-)

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

* [PATCH-V5 1/3] ARM: OMAP2/3: Add idle_st bits for ST_32KSYNC timer to prcm-common header
  2012-04-25 13:58 ` Vaibhav Hiremath
@ 2012-04-25 13:58   ` Vaibhav Hiremath
  -1 siblings, 0 replies; 30+ messages in thread
From: Vaibhav Hiremath @ 2012-04-25 13:58 UTC (permalink / raw)
  To: linux-omap
  Cc: tony, khilman, paul, santosh.shilimkar, 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>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@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>
Cc: Kevin Hilman <khilman@ti.com>
---
NOTE: This patch is same as first version, without any code changes.

 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] 30+ messages in thread

* [PATCH-V5 1/3] ARM: OMAP2/3: Add idle_st bits for ST_32KSYNC timer to prcm-common header
@ 2012-04-25 13:58   ` Vaibhav Hiremath
  0 siblings, 0 replies; 30+ messages in thread
From: Vaibhav Hiremath @ 2012-04-25 13:58 UTC (permalink / raw)
  To: linux-arm-kernel

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>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@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>
Cc: Kevin Hilman <khilman@ti.com>
---
NOTE: This patch is same as first version, without any code changes.

 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] 30+ messages in thread

* [PATCH-V5 2/3] ARM: OMAP2/3: hwmod data: Add 32k-sync timer data to hwmod database
  2012-04-25 13:58 ` Vaibhav Hiremath
@ 2012-04-25 13:58   ` Vaibhav Hiremath
  -1 siblings, 0 replies; 30+ messages in thread
From: Vaibhav Hiremath @ 2012-04-25 13:58 UTC (permalink / raw)
  To: linux-omap
  Cc: tony, khilman, paul, santosh.shilimkar, 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>
Reviewed-by: 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: Kevin Hilman <khilman@ti.com>
---
NOTE: This patch is same as first version, without any code changes.

 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 71d9f88..c9ac3ec 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 0c65079..f55dc6a 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] 30+ messages in thread

* [PATCH-V5 2/3] ARM: OMAP2/3: hwmod data: Add 32k-sync timer data to hwmod database
@ 2012-04-25 13:58   ` Vaibhav Hiremath
  0 siblings, 0 replies; 30+ messages in thread
From: Vaibhav Hiremath @ 2012-04-25 13:58 UTC (permalink / raw)
  To: linux-arm-kernel

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>
Reviewed-by: 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: Kevin Hilman <khilman@ti.com>
---
NOTE: This patch is same as first version, without any code changes.

 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 71d9f88..c9ac3ec 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 0c65079..f55dc6a 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] 30+ messages in thread

* [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
  2012-04-25 13:58 ` Vaibhav Hiremath
@ 2012-04-25 13:59   ` Vaibhav Hiremath
  -1 siblings, 0 replies; 30+ messages in thread
From: Vaibhav Hiremath @ 2012-04-25 13:59 UTC (permalink / raw)
  To: linux-omap
  Cc: tony, khilman, paul, santosh.shilimkar, b-cousson,
	linux-arm-kernel, Vaibhav Hiremath, Felipe Balbi,
	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.
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 <hvaibhav@ti.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Reviewed-by: 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: Kevin Hilman <khilman@ti.com>
Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Cc: Ming Lei <tom.leiming@gmail.com>
---
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(-)

diff --git a/arch/arm/mach-omap1/timer32k.c b/arch/arm/mach-omap1/timer32k.c
index 325b9a0..6262e11 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	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();

 	return true;
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index ecec873..d720f58 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -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);
+	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;
@@ -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) {
@@ -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)
+ *
+ * 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);
diff --git a/arch/arm/plat-omap/counter_32k.c b/arch/arm/plat-omap/counter_32k.c
index 5068fe5..3e3cdab 100644
--- a/arch/arm/plat-omap/counter_32k.c
+++ b/arch/arm/plat-omap/counter_32k.c
@@ -27,19 +27,20 @@

 #include <plat/clock.h>

+/* 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.
  */
-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;
+
+	/*
+	 * 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;
 }
diff --git a/arch/arm/plat-omap/include/plat/common.h b/arch/arm/plat-omap/include/plat/common.h
index a557b84..eed5335 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 __init omap_check_revision(void);

--
1.7.0.4


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

* [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
@ 2012-04-25 13:59   ` Vaibhav Hiremath
  0 siblings, 0 replies; 30+ messages in thread
From: Vaibhav Hiremath @ 2012-04-25 13:59 UTC (permalink / raw)
  To: linux-arm-kernel

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 <hvaibhav@ti.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Reviewed-by: 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: Kevin Hilman <khilman@ti.com>
Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Cc: Ming Lei <tom.leiming@gmail.com>
---
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(-)

diff --git a/arch/arm/mach-omap1/timer32k.c b/arch/arm/mach-omap1/timer32k.c
index 325b9a0..6262e11 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	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();

 	return true;
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index ecec873..d720f58 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -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);
+	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;
@@ -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) {
@@ -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)
+ *
+ * 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);
diff --git a/arch/arm/plat-omap/counter_32k.c b/arch/arm/plat-omap/counter_32k.c
index 5068fe5..3e3cdab 100644
--- a/arch/arm/plat-omap/counter_32k.c
+++ b/arch/arm/plat-omap/counter_32k.c
@@ -27,19 +27,20 @@

 #include <plat/clock.h>

+/* 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.
  */
-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;
+
+	/*
+	 * 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;
 }
diff --git a/arch/arm/plat-omap/include/plat/common.h b/arch/arm/plat-omap/include/plat/common.h
index a557b84..eed5335 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 __init omap_check_revision(void);

--
1.7.0.4

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

* Re: [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
  2012-04-25 13:59   ` Vaibhav Hiremath
@ 2012-04-26 15:10     ` Paul Walmsley
  -1 siblings, 0 replies; 30+ messages in thread
From: Paul Walmsley @ 2012-04-26 15:10 UTC (permalink / raw)
  To: Vaibhav Hiremath
  Cc: khilman, b-cousson, tony, Ming Lei, Felipe Balbi,
	santosh.shilimkar, linux-omap, Tarun Kanti DebBarma,
	linux-arm-kernel

Hi

a question

On Wed, 25 Apr 2012, 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).
> 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.

...

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

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.


- Paul

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

* [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
@ 2012-04-26 15:10     ` Paul Walmsley
  0 siblings, 0 replies; 30+ messages in thread
From: Paul Walmsley @ 2012-04-26 15:10 UTC (permalink / raw)
  To: linux-arm-kernel

Hi

a question

On Wed, 25 Apr 2012, 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).
> 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.

...

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

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.


- Paul

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

* RE: [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
  2012-04-26 15:10     ` Paul Walmsley
@ 2012-04-26 16:20       ` Hiremath, Vaibhav
  -1 siblings, 0 replies; 30+ messages in thread
From: Hiremath, Vaibhav @ 2012-04-26 16:20 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: linux-omap, tony, Hilman, Kevin, Shilimkar, Santosh, Cousson,
	Benoit, linux-arm-kernel, Balbi, Felipe, DebBarma, Tarun Kanti,
	Ming Lei

On Thu, Apr 26, 2012 at 20:40:34, Paul Walmsley wrote:
> Hi
> 
> a question
> 
> On Wed, 25 Apr 2012, 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).
> > 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.
> 
> ...
> 
> > -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);
> 
> 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?)

Yes, this runs very early in the boot sequence.

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

Not really, I reverted all my patch-sets and just added changed for,

-----------------Change ontop of linux-omap/master--------

diff --git a/arch/arm/plat-omap/counter_32k.c b/arch/arm/plat-omap/counter_32k.c
index 5068fe5..12e5777 100644
--- a/arch/arm/plat-omap/counter_32k.c
+++ b/arch/arm/plat-omap/counter_32k.c
@@ -87,7 +87,7 @@ int __init omap_init_clocksource_32k(void)
                else if (cpu_is_omap2430())
                        pbase = OMAP2430_32KSYNCT_BASE + 0x10;
                else if (cpu_is_omap34xx())
-                       pbase = OMAP3430_32KSYNCT_BASE + 0x10;
+                       pbase = OMAP3430_32KSYNCT_BASE;
                else if (cpu_is_omap44xx())
                        pbase = OMAP4430_32KSYNCT_BASE + 0x10;
                else
@@ -102,7 +102,7 @@ int __init omap_init_clocksource_32k(void)
                if (!IS_ERR(sync_32k_ick))
                        clk_enable(sync_32k_ick);

-               timer_32k_base = base;
+               timer_32k_base = base + 0x10;

                /*
                 * 120000 rough estimate from the calculations in

---------------------------------------------

This itself results in kernel hang, means execution gets stuck in 
default_idle(),


Boot Log -
============================

Bytes transferred = 2022580 (1edcb4 hex)
## Booting kernel from Legacy Image at 81000000 ...
   Image Name:   Linux-3.4.0-rc3-11786-g1e32b7e-d
   Image Type:   ARM Linux Kernel Image (uncompressed)
   Data Size:    3391152 Bytes = 3.2 MiB
   Load Address: 80008000
   Entry Point:  80008000
   Verifying Checksum ... OK
   Loading Kernel Image ... OK
OK

Starting kernel ...

Uncompressing Linux... done, booting the kernel.
[    0.000000] Booting Linux on physical CPU 0
[    0.000000] Linux version 3.4.0-rc3-11786-g1e32b7e-dirty (a0393758@psplinux064) (gcc version 4.5.3 20110311 (prerelease) (GCC) ) #24 SMP Thu Apr 26 21:46:45 IST 2012
[    0.000000] CPU: ARMv7 Processor [413fc082] revision 2 (ARMv7), cr=10c53c7d
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
[    0.000000] Machine: OMAP3 EVM
[    0.000000] Memory policy: ECC disabled, Data cache writeback
[    0.000000] OMAP3630 ES1.2 (l2cache iva sgx neon isp 192mhz_clk )
[    0.000000] Clocking rate (Crystal/Core/MPU): 26.0/400/600 MHz
[    0.000000] PERCPU: Embedded 8 pages/cpu @c0d1f000 s11456 r8192 d13120 u32768
[    0.000000] Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 32256
[    0.000000] Kernel command line: console=ttyO0,115200n8 mem=128M root=/dev/ram rw initrd=0x82000000,16MB ramdisk_size=65536 earlyprintk=serial
[    0.000000] PID hash table entries: 512 (order: -1, 2048 bytes)
[    0.000000] Dentry cache hash table entries: 16384 (order: 4, 65536 bytes)
[    0.000000] Inode-cache hash table entries: 8192 (order: 3, 32768 bytes)
[    0.000000] Memory: 127MB = 127MB total
[    0.000000] Memory: 100096k/100096k available, 30976k reserved, 0K highmem
[    0.000000] Virtual kernel memory layout:
[    0.000000]     vector  : 0xffff0000 - 0xffff1000   (   4 kB)
[    0.000000]     fixmap  : 0xfff00000 - 0xfffe0000   ( 896 kB)
[    0.000000]     vmalloc : 0xc8800000 - 0xff000000   ( 872 MB)
[    0.000000]     lowmem  : 0xc0000000 - 0xc8000000   ( 128 MB)
[    0.000000]     modules : 0xbf000000 - 0xc0000000   (  16 MB)
[    0.000000]       .text : 0xc0008000 - 0xc05e3ca0   (6000 kB)
[    0.000000]       .init : 0xc05e4000 - 0xc0631cc0   ( 312 kB)
[    0.000000]       .data : 0xc0632000 - 0xc06c6898   ( 595 kB)
[    0.000000]        .bss : 0xc06c68bc - 0xc0c1ac60   (5457 kB)
[    0.000000] Hierarchical RCU implementation.
[    0.000000] NR_IRQS:474
[    0.000000] IRQ: Found an INTC at 0xfa200000 (revision 4.0) with 96 interrupts
[    0.000000] Total of 96 interrupts on 1 active controller
[    0.000000] OMAP clockevent source: GPTIMER1 at 32768 Hz
[    0.000000] sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every 131071999ms
[    0.000000] Console: colour dummy device 80x30
[    0.000000] Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar
[    0.000000] ... MAX_LOCKDEP_SUBCLASSES:  8
[    0.000000] ... MAX_LOCK_DEPTH:          48
[    0.000000] ... MAX_LOCKDEP_KEYS:        8191
[    0.000000] ... CLASSHASH_SIZE:          4096
[    0.000000] ... MAX_LOCKDEP_ENTRIES:     16384
[    0.000000] ... MAX_LOCKDEP_CHAINS:      32768
[    0.000000] ... CHAINHASH_SIZE:          16384
[    0.000000]  memory used by lock dependency info: 3695 kB
[    0.000000]  per task-struct memory footprint: 1152 bytes
[    0.000885] Calibrating delay loop... 597.64 BogoMIPS (lpj=2334720)
[    0.085937] pid_max: default: 32768 minimum: 301
[    0.086669] Security Framework initialized
[    0.086944] Mount-cache hash table entries: 512
[    0.092224] CPU: Testing write buffer coherency: ok
[    0.093139] CPU0: thread -1, cpu 0, socket -1, mpidr 0
[    0.093200] Setting up static identity map for 0x80433d68 - 0x80433dd8
[    0.094909] Brought up 1 CPUs
[    0.094940] SMP: Total of 1 processors activated (597.64 BogoMIPS).
[    0.116607] dummy:
[    0.118927] NET: Registered protocol family 16
[    0.120208] GPMC revision 5.0
[    0.120452] gpmc: irq-20 could not claim: err -22
[    0.130798] gpiochip_add: registered GPIOs 0 to 31 on device: gpio
[    0.131347] OMAP GPIO hardware version 2.5
[    0.132415] gpiochip_add: registered GPIOs 32 to 63 on device: gpio
[    0.134368] gpiochip_add: registered GPIOs 64 to 95 on device: gpio
[    0.135833] gpiochip_add: registered GPIOs 96 to 127 on device: gpio
[    0.137298] gpiochip_add: registered GPIOs 128 to 159 on device: gpio
[    0.138732] gpiochip_add: registered GPIOs 160 to 191 on device: gpio
[    0.146514] omap_mux_init: Add partition: #1: core, flags: 0
[    0.168121] Reprogramming SDRC clock to 400000000 Hz
[    0.168151] dpll3_m2_clk rate change failed: -22
[    0.170196] hw-breakpoint: debug architecture 0x4 unsupported.
[    0.185394]  omap-mcbsp.2: alias fck already exists
[    0.186157]  omap-mcbsp.3: alias fck already exists
[    0.191101] OMAP DMA hardware revision 5.0
[    0.257019] bio: create slab <bio-0> at 0
[    0.261627] fixed-dummy:
[    0.267669] SCSI subsystem initialized
[    0.269744] omap2_mcspi omap2_mcspi.1: master is unqueued, this is deprecated
[    0.272705] omap2_mcspi omap2_mcspi.2: master is unqueued, this is deprecated
[    0.274291] omap2_mcspi omap2_mcspi.3: master is unqueued, this is deprecated
[    0.275451] omap2_mcspi omap2_mcspi.4: master is unqueued, this is deprecated
[    0.278686] usbcore: registered new interface driver usbfs
[    0.279724] usbcore: registered new interface driver hub
[    0.280426] usbcore: registered new device driver usb
[    0.297424] omap_i2c omap_i2c.1: bus 1 rev1.4.0 at 2600 kHz
[    0.307464] twl 1-0048: PIH (irq 7) chaining IRQs 320..328
[    0.308166] twl 1-0048: power (irq 325) chaining IRQs 328..335
[    0.310638] twl4030_gpio twl4030_gpio: gpio (irq 320) chaining IRQs 336..353
[    0.312103] gpiochip_add: registered GPIOs 192 to 211 on device: twl4030
[    0.323120] VIO: 1800 mV normal standby
[    0.325561] vdd_mpu_iva: 600 <--> 1450 mV normal
[    0.327758] vdd_core: 600 <--> 1450 mV normal
[    0.330444] VMMC1: 1850 <--> 3150 mV at 3000 mV normal standby
[    0.332885] VDAC: 1800 mV normal standby
[    0.335205] VPLL2: 1800 mV normal standby
[    0.337921] VSIM: 1800 <--> 3000 mV at 1800 mV normal standby
[    0.351806] omap_i2c omap_i2c.2: bus 2 rev1.4.0 at 400 kHz
[    0.367370] omap_i2c omap_i2c.3: bus 3 rev1.4.0 at 400 kHz
[    0.375946] Switching to clocksource 32k_counter
[    0.488311] NET: Registered protocol family 2
[    0.489227] IP route cache hash table entries: 1024 (order: 0, 4096 bytes)
[    0.491394] TCP established hash table entries: 4096 (order: 3, 32768 bytes)
[    0.491699] TCP bind hash table entries: 4096 (order: 5, 147456 bytes)
[    0.493957] TCP: Hash tables configured (established 4096 bind 4096)
[    0.494049] TCP: reno registered
[    0.494079] UDP hash table entries: 64 (order: 0, 5120 bytes)
[    0.494445] UDP-Lite hash table entries: 64 (order: 0, 5120 bytes)
[    0.495422] NET: Registered protocol family 1
[    0.496978] RPC: Registered named UNIX socket transport module.
[    0.497009] RPC: Registered udp transport module.
[    0.497039] RPC: Registered tcp transport module.
[    0.497039] RPC: Registered tcp NFSv4.1 backchannel transport module.
[    0.497924] Trying to unpack rootfs image as initramfs...
[    0.499999] rootfs image is not initramfs (no cpio magic); looks like an initrd
[    0.620941] Freeing initrd memory: 16384K
[    0.621124] NetWinder Floating Point Emulator V0.97 (double precision)
[    0.786560] VFS: Disk quotas dquot_6.5.2
[    0.786895] Dquot-cache hash table entries: 1024 (order 0, 4096 bytes)
[    0.788635] NFS: Registering the id_resolver key type
[    0.790588] jffs2: version 2.2. (NAND) (SUMMARY)  (c) 2001-2006 Red Hat, Inc.
[    0.791748] msgmni has been set to 227
[    0.795349] io scheduler noop registered
[    0.795379] io scheduler deadline registered
[    0.795623] io scheduler cfq registered (default)
[    0.798309] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
[    0.805297] omap_uart.0: ttyO0 at MMIO 0x4806a000 (irq = 72) is a OMAP UART0
[    1.511840] console [ttyO0] enabled
[    1.517364] omap_uart.1: ttyO1 at MMIO 0x4806c000 (irq = 73) is a OMAP UART1
[    1.526306] omap_uart.2: ttyO2 at MMIO 0x49020000 (irq = 74) is a OMAP UART2
[    1.535186] omap_uart.3: ttyO3 at MMIO 0x49042000 (irq = 80) is a OMAP UART3
[    1.575927] brd: module loaded
[    1.598480] loop: module loaded
[    1.609771] mtdoops: mtd device (mtddev=name/number) must be supplied
[    1.617431] OneNAND driver initializing
[    1.627929] smsc911x: Driver version 2008-10-21
[    1.641845] smsc911x-mdio: probed
[    1.645599] smsc911x smsc911x.0: eth0: attached PHY driver [SMSC LAN8700] (mii_bus:phy_addr=smsc911x-0:01, irq=-1)
[    1.656829] smsc911x smsc911x.0: eth0: MAC Address: 00:50:c2:7e:8f:d9
[    1.664978] usbcore: registered new interface driver asix
[    1.671203] usbcore: registered new interface driver cdc_ether
[    1.677947] usbcore: registered new interface driver net1080
[    1.684417] usbcore: registered new interface driver cdc_subset
[    1.691223] usbcore: registered new interface driver zaurus
[    1.697662] usbcore: registered new interface driver cdc_ncm
[    1.705780] usbcore: registered new interface driver cdc_wdm
[    1.711791] Initializing USB Mass Storage driver...
[    1.717529] usbcore: registered new interface driver usb-storage
[    1.723876] USB Mass Storage support registered.
[    1.729949] usbcore: registered new interface driver libusual
[    1.736663] usbcore: registered new interface driver usbtest
[    1.744323] mousedev: PS/2 mouse device common for all mice
[    1.752960] input: TWL4030 Keypad as /devices/platform/omap_i2c.1/i2c-1/1-004a/twl4030_keypad/input/input0
[    1.770904] ads7846 spi1.0: touchscreen, irq 271
[    1.778656] input: ADS7846 Touchscreen as /devices/platform/omap2_mcspi.1/spi_master/spi1/spi1.0/input/input1
[    1.793945] input: twl4030_pwrbutton as /devices/platform/omap_i2c.1/i2c-1/1-0049/twl4030_pwrbutton/input/input2
[    1.806579] twl_rtc twl_rtc: Power up reset detected.
[    1.812042] twl_rtc twl_rtc: Enabling TWL-RTC
[    1.820617] twl_rtc twl_rtc: rtc core: registered twl_rtc as rtc0
[    1.828491] i2c /dev entries driver
[    1.836090] Driver for 1-wire Dallas network protocol.
[    1.844848] omap_wdt: OMAP Watchdog Timer Rev 0x31: initial timeout 60 sec
[    1.853210] twl4030_wdt twl4030_wdt: Failed to register misc device
[    1.859863] twl4030_wdt: probe of twl4030_wdt failed with error -16

<Stuck in default idle>

Thanks,
Vaibhav

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

* [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
@ 2012-04-26 16:20       ` Hiremath, Vaibhav
  0 siblings, 0 replies; 30+ messages in thread
From: Hiremath, Vaibhav @ 2012-04-26 16:20 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Apr 26, 2012 at 20:40:34, Paul Walmsley wrote:
> Hi
> 
> a question
> 
> On Wed, 25 Apr 2012, 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).
> > 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.
> 
> ...
> 
> > -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);
> 
> 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?)

Yes, this runs very early in the boot sequence.

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

Not really, I reverted all my patch-sets and just added changed for,

-----------------Change ontop of linux-omap/master--------

diff --git a/arch/arm/plat-omap/counter_32k.c b/arch/arm/plat-omap/counter_32k.c
index 5068fe5..12e5777 100644
--- a/arch/arm/plat-omap/counter_32k.c
+++ b/arch/arm/plat-omap/counter_32k.c
@@ -87,7 +87,7 @@ int __init omap_init_clocksource_32k(void)
                else if (cpu_is_omap2430())
                        pbase = OMAP2430_32KSYNCT_BASE + 0x10;
                else if (cpu_is_omap34xx())
-                       pbase = OMAP3430_32KSYNCT_BASE + 0x10;
+                       pbase = OMAP3430_32KSYNCT_BASE;
                else if (cpu_is_omap44xx())
                        pbase = OMAP4430_32KSYNCT_BASE + 0x10;
                else
@@ -102,7 +102,7 @@ int __init omap_init_clocksource_32k(void)
                if (!IS_ERR(sync_32k_ick))
                        clk_enable(sync_32k_ick);

-               timer_32k_base = base;
+               timer_32k_base = base + 0x10;

                /*
                 * 120000 rough estimate from the calculations in

---------------------------------------------

This itself results in kernel hang, means execution gets stuck in 
default_idle(),


Boot Log -
============================

Bytes transferred = 2022580 (1edcb4 hex)
## Booting kernel from Legacy Image at 81000000 ...
   Image Name:   Linux-3.4.0-rc3-11786-g1e32b7e-d
   Image Type:   ARM Linux Kernel Image (uncompressed)
   Data Size:    3391152 Bytes = 3.2 MiB
   Load Address: 80008000
   Entry Point:  80008000
   Verifying Checksum ... OK
   Loading Kernel Image ... OK
OK

Starting kernel ...

Uncompressing Linux... done, booting the kernel.
[    0.000000] Booting Linux on physical CPU 0
[    0.000000] Linux version 3.4.0-rc3-11786-g1e32b7e-dirty (a0393758 at psplinux064) (gcc version 4.5.3 20110311 (prerelease) (GCC) ) #24 SMP Thu Apr 26 21:46:45 IST 2012
[    0.000000] CPU: ARMv7 Processor [413fc082] revision 2 (ARMv7), cr=10c53c7d
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
[    0.000000] Machine: OMAP3 EVM
[    0.000000] Memory policy: ECC disabled, Data cache writeback
[    0.000000] OMAP3630 ES1.2 (l2cache iva sgx neon isp 192mhz_clk )
[    0.000000] Clocking rate (Crystal/Core/MPU): 26.0/400/600 MHz
[    0.000000] PERCPU: Embedded 8 pages/cpu @c0d1f000 s11456 r8192 d13120 u32768
[    0.000000] Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 32256
[    0.000000] Kernel command line: console=ttyO0,115200n8 mem=128M root=/dev/ram rw initrd=0x82000000,16MB ramdisk_size=65536 earlyprintk=serial
[    0.000000] PID hash table entries: 512 (order: -1, 2048 bytes)
[    0.000000] Dentry cache hash table entries: 16384 (order: 4, 65536 bytes)
[    0.000000] Inode-cache hash table entries: 8192 (order: 3, 32768 bytes)
[    0.000000] Memory: 127MB = 127MB total
[    0.000000] Memory: 100096k/100096k available, 30976k reserved, 0K highmem
[    0.000000] Virtual kernel memory layout:
[    0.000000]     vector  : 0xffff0000 - 0xffff1000   (   4 kB)
[    0.000000]     fixmap  : 0xfff00000 - 0xfffe0000   ( 896 kB)
[    0.000000]     vmalloc : 0xc8800000 - 0xff000000   ( 872 MB)
[    0.000000]     lowmem  : 0xc0000000 - 0xc8000000   ( 128 MB)
[    0.000000]     modules : 0xbf000000 - 0xc0000000   (  16 MB)
[    0.000000]       .text : 0xc0008000 - 0xc05e3ca0   (6000 kB)
[    0.000000]       .init : 0xc05e4000 - 0xc0631cc0   ( 312 kB)
[    0.000000]       .data : 0xc0632000 - 0xc06c6898   ( 595 kB)
[    0.000000]        .bss : 0xc06c68bc - 0xc0c1ac60   (5457 kB)
[    0.000000] Hierarchical RCU implementation.
[    0.000000] NR_IRQS:474
[    0.000000] IRQ: Found an INTC at 0xfa200000 (revision 4.0) with 96 interrupts
[    0.000000] Total of 96 interrupts on 1 active controller
[    0.000000] OMAP clockevent source: GPTIMER1 at 32768 Hz
[    0.000000] sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every 131071999ms
[    0.000000] Console: colour dummy device 80x30
[    0.000000] Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar
[    0.000000] ... MAX_LOCKDEP_SUBCLASSES:  8
[    0.000000] ... MAX_LOCK_DEPTH:          48
[    0.000000] ... MAX_LOCKDEP_KEYS:        8191
[    0.000000] ... CLASSHASH_SIZE:          4096
[    0.000000] ... MAX_LOCKDEP_ENTRIES:     16384
[    0.000000] ... MAX_LOCKDEP_CHAINS:      32768
[    0.000000] ... CHAINHASH_SIZE:          16384
[    0.000000]  memory used by lock dependency info: 3695 kB
[    0.000000]  per task-struct memory footprint: 1152 bytes
[    0.000885] Calibrating delay loop... 597.64 BogoMIPS (lpj=2334720)
[    0.085937] pid_max: default: 32768 minimum: 301
[    0.086669] Security Framework initialized
[    0.086944] Mount-cache hash table entries: 512
[    0.092224] CPU: Testing write buffer coherency: ok
[    0.093139] CPU0: thread -1, cpu 0, socket -1, mpidr 0
[    0.093200] Setting up static identity map for 0x80433d68 - 0x80433dd8
[    0.094909] Brought up 1 CPUs
[    0.094940] SMP: Total of 1 processors activated (597.64 BogoMIPS).
[    0.116607] dummy:
[    0.118927] NET: Registered protocol family 16
[    0.120208] GPMC revision 5.0
[    0.120452] gpmc: irq-20 could not claim: err -22
[    0.130798] gpiochip_add: registered GPIOs 0 to 31 on device: gpio
[    0.131347] OMAP GPIO hardware version 2.5
[    0.132415] gpiochip_add: registered GPIOs 32 to 63 on device: gpio
[    0.134368] gpiochip_add: registered GPIOs 64 to 95 on device: gpio
[    0.135833] gpiochip_add: registered GPIOs 96 to 127 on device: gpio
[    0.137298] gpiochip_add: registered GPIOs 128 to 159 on device: gpio
[    0.138732] gpiochip_add: registered GPIOs 160 to 191 on device: gpio
[    0.146514] omap_mux_init: Add partition: #1: core, flags: 0
[    0.168121] Reprogramming SDRC clock to 400000000 Hz
[    0.168151] dpll3_m2_clk rate change failed: -22
[    0.170196] hw-breakpoint: debug architecture 0x4 unsupported.
[    0.185394]  omap-mcbsp.2: alias fck already exists
[    0.186157]  omap-mcbsp.3: alias fck already exists
[    0.191101] OMAP DMA hardware revision 5.0
[    0.257019] bio: create slab <bio-0> at 0
[    0.261627] fixed-dummy:
[    0.267669] SCSI subsystem initialized
[    0.269744] omap2_mcspi omap2_mcspi.1: master is unqueued, this is deprecated
[    0.272705] omap2_mcspi omap2_mcspi.2: master is unqueued, this is deprecated
[    0.274291] omap2_mcspi omap2_mcspi.3: master is unqueued, this is deprecated
[    0.275451] omap2_mcspi omap2_mcspi.4: master is unqueued, this is deprecated
[    0.278686] usbcore: registered new interface driver usbfs
[    0.279724] usbcore: registered new interface driver hub
[    0.280426] usbcore: registered new device driver usb
[    0.297424] omap_i2c omap_i2c.1: bus 1 rev1.4.0 at 2600 kHz
[    0.307464] twl 1-0048: PIH (irq 7) chaining IRQs 320..328
[    0.308166] twl 1-0048: power (irq 325) chaining IRQs 328..335
[    0.310638] twl4030_gpio twl4030_gpio: gpio (irq 320) chaining IRQs 336..353
[    0.312103] gpiochip_add: registered GPIOs 192 to 211 on device: twl4030
[    0.323120] VIO: 1800 mV normal standby
[    0.325561] vdd_mpu_iva: 600 <--> 1450 mV normal
[    0.327758] vdd_core: 600 <--> 1450 mV normal
[    0.330444] VMMC1: 1850 <--> 3150 mV at 3000 mV normal standby
[    0.332885] VDAC: 1800 mV normal standby
[    0.335205] VPLL2: 1800 mV normal standby
[    0.337921] VSIM: 1800 <--> 3000 mV at 1800 mV normal standby
[    0.351806] omap_i2c omap_i2c.2: bus 2 rev1.4.0 at 400 kHz
[    0.367370] omap_i2c omap_i2c.3: bus 3 rev1.4.0 at 400 kHz
[    0.375946] Switching to clocksource 32k_counter
[    0.488311] NET: Registered protocol family 2
[    0.489227] IP route cache hash table entries: 1024 (order: 0, 4096 bytes)
[    0.491394] TCP established hash table entries: 4096 (order: 3, 32768 bytes)
[    0.491699] TCP bind hash table entries: 4096 (order: 5, 147456 bytes)
[    0.493957] TCP: Hash tables configured (established 4096 bind 4096)
[    0.494049] TCP: reno registered
[    0.494079] UDP hash table entries: 64 (order: 0, 5120 bytes)
[    0.494445] UDP-Lite hash table entries: 64 (order: 0, 5120 bytes)
[    0.495422] NET: Registered protocol family 1
[    0.496978] RPC: Registered named UNIX socket transport module.
[    0.497009] RPC: Registered udp transport module.
[    0.497039] RPC: Registered tcp transport module.
[    0.497039] RPC: Registered tcp NFSv4.1 backchannel transport module.
[    0.497924] Trying to unpack rootfs image as initramfs...
[    0.499999] rootfs image is not initramfs (no cpio magic); looks like an initrd
[    0.620941] Freeing initrd memory: 16384K
[    0.621124] NetWinder Floating Point Emulator V0.97 (double precision)
[    0.786560] VFS: Disk quotas dquot_6.5.2
[    0.786895] Dquot-cache hash table entries: 1024 (order 0, 4096 bytes)
[    0.788635] NFS: Registering the id_resolver key type
[    0.790588] jffs2: version 2.2. (NAND) (SUMMARY)  (c) 2001-2006 Red Hat, Inc.
[    0.791748] msgmni has been set to 227
[    0.795349] io scheduler noop registered
[    0.795379] io scheduler deadline registered
[    0.795623] io scheduler cfq registered (default)
[    0.798309] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
[    0.805297] omap_uart.0: ttyO0 at MMIO 0x4806a000 (irq = 72) is a OMAP UART0
[    1.511840] console [ttyO0] enabled
[    1.517364] omap_uart.1: ttyO1 at MMIO 0x4806c000 (irq = 73) is a OMAP UART1
[    1.526306] omap_uart.2: ttyO2 at MMIO 0x49020000 (irq = 74) is a OMAP UART2
[    1.535186] omap_uart.3: ttyO3 at MMIO 0x49042000 (irq = 80) is a OMAP UART3
[    1.575927] brd: module loaded
[    1.598480] loop: module loaded
[    1.609771] mtdoops: mtd device (mtddev=name/number) must be supplied
[    1.617431] OneNAND driver initializing
[    1.627929] smsc911x: Driver version 2008-10-21
[    1.641845] smsc911x-mdio: probed
[    1.645599] smsc911x smsc911x.0: eth0: attached PHY driver [SMSC LAN8700] (mii_bus:phy_addr=smsc911x-0:01, irq=-1)
[    1.656829] smsc911x smsc911x.0: eth0: MAC Address: 00:50:c2:7e:8f:d9
[    1.664978] usbcore: registered new interface driver asix
[    1.671203] usbcore: registered new interface driver cdc_ether
[    1.677947] usbcore: registered new interface driver net1080
[    1.684417] usbcore: registered new interface driver cdc_subset
[    1.691223] usbcore: registered new interface driver zaurus
[    1.697662] usbcore: registered new interface driver cdc_ncm
[    1.705780] usbcore: registered new interface driver cdc_wdm
[    1.711791] Initializing USB Mass Storage driver...
[    1.717529] usbcore: registered new interface driver usb-storage
[    1.723876] USB Mass Storage support registered.
[    1.729949] usbcore: registered new interface driver libusual
[    1.736663] usbcore: registered new interface driver usbtest
[    1.744323] mousedev: PS/2 mouse device common for all mice
[    1.752960] input: TWL4030 Keypad as /devices/platform/omap_i2c.1/i2c-1/1-004a/twl4030_keypad/input/input0
[    1.770904] ads7846 spi1.0: touchscreen, irq 271
[    1.778656] input: ADS7846 Touchscreen as /devices/platform/omap2_mcspi.1/spi_master/spi1/spi1.0/input/input1
[    1.793945] input: twl4030_pwrbutton as /devices/platform/omap_i2c.1/i2c-1/1-0049/twl4030_pwrbutton/input/input2
[    1.806579] twl_rtc twl_rtc: Power up reset detected.
[    1.812042] twl_rtc twl_rtc: Enabling TWL-RTC
[    1.820617] twl_rtc twl_rtc: rtc core: registered twl_rtc as rtc0
[    1.828491] i2c /dev entries driver
[    1.836090] Driver for 1-wire Dallas network protocol.
[    1.844848] omap_wdt: OMAP Watchdog Timer Rev 0x31: initial timeout 60 sec
[    1.853210] twl4030_wdt twl4030_wdt: Failed to register misc device
[    1.859863] twl4030_wdt: probe of twl4030_wdt failed with error -16

<Stuck in default idle>

Thanks,
Vaibhav

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

* RE: [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
  2012-04-26 15:10     ` Paul Walmsley
@ 2012-04-26 16:40       ` Hiremath, Vaibhav
  -1 siblings, 0 replies; 30+ messages in thread
From: Hiremath, Vaibhav @ 2012-04-26 16:40 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: linux-omap, tony, Hilman, Kevin, Shilimkar, Santosh, Cousson,
	Benoit, linux-arm-kernel, Balbi, Felipe, DebBarma, Tarun Kanti,
	Ming Lei

On Thu, Apr 26, 2012 at 20:40:34, Paul Walmsley wrote:
> Hi
> 
> a question
> 
> On Wed, 25 Apr 2012, Vaibhav Hiremath wrote:
> 
<snip>
> > +
> > +	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.
> 

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.

Thanks,
Vaibhav

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

* [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
@ 2012-04-26 16:40       ` Hiremath, Vaibhav
  0 siblings, 0 replies; 30+ messages in thread
From: Hiremath, Vaibhav @ 2012-04-26 16:40 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Apr 26, 2012 at 20:40:34, Paul Walmsley wrote:
> Hi
> 
> a question
> 
> On Wed, 25 Apr 2012, Vaibhav Hiremath wrote:
> 
<snip>
> > +
> > +	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.
> 

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.

Thanks,
Vaibhav

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

* Re: [PATCH-V5 1/3] ARM: OMAP2/3: Add idle_st bits for ST_32KSYNC timer to prcm-common header
  2012-04-25 13:58   ` Vaibhav Hiremath
@ 2012-04-26 19:25     ` Paul Walmsley
  -1 siblings, 0 replies; 30+ messages in thread
From: Paul Walmsley @ 2012-04-26 19:25 UTC (permalink / raw)
  To: Vaibhav Hiremath
  Cc: linux-omap, tony, khilman, santosh.shilimkar, b-cousson,
	linux-arm-kernel, Felipe Balbi

On Wed, 25 Apr 2012, Vaibhav Hiremath wrote:

> 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>
> Reviewed-by: Santosh Shilimkar <santosh.shilimkar@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>
> Cc: Kevin Hilman <khilman@ti.com>
> ---
> NOTE: This patch is same as first version, without any code changes.

Thanks, queued for 3.5.


- Paul

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

* [PATCH-V5 1/3] ARM: OMAP2/3: Add idle_st bits for ST_32KSYNC timer to prcm-common header
@ 2012-04-26 19:25     ` Paul Walmsley
  0 siblings, 0 replies; 30+ messages in thread
From: Paul Walmsley @ 2012-04-26 19:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 25 Apr 2012, Vaibhav Hiremath wrote:

> 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>
> Reviewed-by: Santosh Shilimkar <santosh.shilimkar@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>
> Cc: Kevin Hilman <khilman@ti.com>
> ---
> NOTE: This patch is same as first version, without any code changes.

Thanks, queued for 3.5.


- Paul

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

* Re: [PATCH-V5 2/3] ARM: OMAP2/3: hwmod data: Add 32k-sync timer data to hwmod database
  2012-04-25 13:58   ` Vaibhav Hiremath
@ 2012-04-26 19:26     ` Paul Walmsley
  -1 siblings, 0 replies; 30+ messages in thread
From: Paul Walmsley @ 2012-04-26 19:26 UTC (permalink / raw)
  To: Vaibhav Hiremath
  Cc: linux-omap, tony, khilman, santosh.shilimkar, b-cousson,
	linux-arm-kernel, Felipe Balbi

On Wed, 25 Apr 2012, Vaibhav Hiremath wrote:

> 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>
> Reviewed-by: 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: Kevin Hilman <khilman@ti.com>

Thanks, queued for 3.5.

- Paul

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

* [PATCH-V5 2/3] ARM: OMAP2/3: hwmod data: Add 32k-sync timer data to hwmod database
@ 2012-04-26 19:26     ` Paul Walmsley
  0 siblings, 0 replies; 30+ messages in thread
From: Paul Walmsley @ 2012-04-26 19:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 25 Apr 2012, Vaibhav Hiremath wrote:

> 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>
> Reviewed-by: 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: Kevin Hilman <khilman@ti.com>

Thanks, queued for 3.5.

- Paul

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

* RE: [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
  2012-04-26 16:40       ` Hiremath, Vaibhav
@ 2012-04-26 19:30         ` Paul Walmsley
  -1 siblings, 0 replies; 30+ messages in thread
From: Paul Walmsley @ 2012-04-26 19:30 UTC (permalink / raw)
  To: Hiremath, Vaibhav
  Cc: linux-omap, tony, Hilman, Kevin, Shilimkar, Santosh, Cousson,
	Benoit, linux-arm-kernel, Balbi, Felipe, DebBarma, Tarun Kanti,
	Ming Lei

On Thu, 26 Apr 2012, Hiremath, Vaibhav wrote:

> On Thu, Apr 26, 2012 at 20:40:34, Paul Walmsley wrote:
> > On Wed, 25 Apr 2012, Vaibhav Hiremath wrote:
> > 
> <snip>
> > > +
> > > +	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.
> > 
> 
> 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().


- Paul

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

* [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
@ 2012-04-26 19:30         ` Paul Walmsley
  0 siblings, 0 replies; 30+ messages in thread
From: Paul Walmsley @ 2012-04-26 19:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 26 Apr 2012, Hiremath, Vaibhav wrote:

> On Thu, Apr 26, 2012 at 20:40:34, Paul Walmsley wrote:
> > On Wed, 25 Apr 2012, Vaibhav Hiremath wrote:
> > 
> <snip>
> > > +
> > > +	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.
> > 
> 
> 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().


- Paul

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

* Re: [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
  2012-04-25 13:59   ` Vaibhav Hiremath
@ 2012-04-26 22:26     ` Kevin Hilman
  -1 siblings, 0 replies; 30+ messages in thread
From: Kevin Hilman @ 2012-04-26 22:26 UTC (permalink / raw)
  To: Vaibhav Hiremath
  Cc: linux-omap, tony, paul, santosh.shilimkar, b-cousson,
	linux-arm-kernel, Felipe Balbi, 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.
> 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 <hvaibhav@ti.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> Reviewed-by: 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: Kevin Hilman <khilman@ti.com>
> Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
> Cc: Ming Lei <tom.leiming@gmail.com>
> ---
> 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...

>  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(-)
>
> diff --git a/arch/arm/mach-omap1/timer32k.c b/arch/arm/mach-omap1/timer32k.c
> index 325b9a0..6262e11 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	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;

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]

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

>  	omap_init_32k_timer();
>  	return true;
> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index ecec873..d720f58 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -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);
> +	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.

> +	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) {
> @@ -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.

> + * 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);
> diff --git a/arch/arm/plat-omap/counter_32k.c b/arch/arm/plat-omap/counter_32k.c
> index 5068fe5..3e3cdab 100644
> --- a/arch/arm/plat-omap/counter_32k.c
> +++ b/arch/arm/plat-omap/counter_32k.c
> @@ -27,19 +27,20 @@
>
>  #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.

>  /*
>   * 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;
> +
> +	/*
> +	 * 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/

Kevin

> +	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;
>  }
> diff --git a/arch/arm/plat-omap/include/plat/common.h b/arch/arm/plat-omap/include/plat/common.h
> index a557b84..eed5335 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 __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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
@ 2012-04-26 22:26     ` Kevin Hilman
  0 siblings, 0 replies; 30+ messages in thread
From: Kevin Hilman @ 2012-04-26 22:26 UTC (permalink / raw)
  To: linux-arm-kernel

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.
> 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 <hvaibhav@ti.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> Reviewed-by: 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: Kevin Hilman <khilman@ti.com>
> Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
> Cc: Ming Lei <tom.leiming@gmail.com>
> ---
> 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...

>  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(-)
>
> diff --git a/arch/arm/mach-omap1/timer32k.c b/arch/arm/mach-omap1/timer32k.c
> index 325b9a0..6262e11 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	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;

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]

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

>  	omap_init_32k_timer();
>  	return true;
> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index ecec873..d720f58 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -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);
> +	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.

> +	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) {
> @@ -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.

> + * 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);
> diff --git a/arch/arm/plat-omap/counter_32k.c b/arch/arm/plat-omap/counter_32k.c
> index 5068fe5..3e3cdab 100644
> --- a/arch/arm/plat-omap/counter_32k.c
> +++ b/arch/arm/plat-omap/counter_32k.c
> @@ -27,19 +27,20 @@
>
>  #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.

>  /*
>   * 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;
> +
> +	/*
> +	 * 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/

Kevin

> +	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;
>  }
> diff --git a/arch/arm/plat-omap/include/plat/common.h b/arch/arm/plat-omap/include/plat/common.h
> index a557b84..eed5335 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 __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

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

* RE: [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
  2012-04-26 19:30         ` Paul Walmsley
@ 2012-04-26 22:38           ` Paul Walmsley
  -1 siblings, 0 replies; 30+ messages in thread
From: Paul Walmsley @ 2012-04-26 22:38 UTC (permalink / raw)
  To: Hiremath, Vaibhav
  Cc: linux-omap, tony, Hilman, Kevin, Shilimkar, Santosh, Cousson,
	Benoit, linux-arm-kernel, Balbi, Felipe, DebBarma, Tarun Kanti,
	Ming Lei

On Thu, 26 Apr 2012, Paul Walmsley wrote:

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


- Paul

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

* [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
@ 2012-04-26 22:38           ` Paul Walmsley
  0 siblings, 0 replies; 30+ messages in thread
From: Paul Walmsley @ 2012-04-26 22:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 26 Apr 2012, Paul Walmsley wrote:

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


- Paul

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

* RE: [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
  2012-04-26 22:38           ` Paul Walmsley
@ 2012-04-27  5:31             ` Hiremath, Vaibhav
  -1 siblings, 0 replies; 30+ messages in thread
From: Hiremath, Vaibhav @ 2012-04-27  5:31 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: linux-omap, tony, Hilman, Kevin, Shilimkar, Santosh, Cousson,
	Benoit, linux-arm-kernel, Balbi, Felipe, DebBarma, Tarun Kanti,
	Ming Lei

On Fri, Apr 27, 2012 at 04:08:07, Paul Walmsley wrote:
> On Thu, 26 Apr 2012, Paul Walmsley wrote:
> 
> > 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().
> 

What about OMAP1 architecture? Will it work? 

Thanks,
Vaibhav


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

* [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
@ 2012-04-27  5:31             ` Hiremath, Vaibhav
  0 siblings, 0 replies; 30+ messages in thread
From: Hiremath, Vaibhav @ 2012-04-27  5:31 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Apr 27, 2012 at 04:08:07, Paul Walmsley wrote:
> On Thu, 26 Apr 2012, Paul Walmsley wrote:
> 
> > 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().
> 

What about OMAP1 architecture? Will it work? 

Thanks,
Vaibhav

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

* RE: [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
  2012-04-26 22:26     ` Kevin Hilman
@ 2012-04-27  5:51       ` Hiremath, Vaibhav
  -1 siblings, 0 replies; 30+ messages in thread
From: Hiremath, Vaibhav @ 2012-04-27  5:51 UTC (permalink / raw)
  To: Hilman, Kevin
  Cc: linux-omap, tony, paul, Shilimkar, Santosh, Cousson, Benoit,
	linux-arm-kernel, Balbi, Felipe, DebBarma, Tarun Kanti, Ming Lei

On Fri, Apr 27, 2012 at 03:56:56, Hilman, Kevin wrote:
> 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.
> > 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 <hvaibhav@ti.com>
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > Reviewed-by: 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: Kevin Hilman <khilman@ti.com>
> > Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
> > Cc: Ming Lei <tom.leiming@gmail.com>
> > ---
> > 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?
> 

Yes, I meant V4 here.

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

Great, thanks for testing it on OMAP1.

> That being said, a few minor comments below...
> 
> >  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(-)
> >
> > diff --git a/arch/arm/mach-omap1/timer32k.c b/arch/arm/mach-omap1/timer32k.c
> > index 325b9a0..6262e11 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	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;
> 
> 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]
> 

Oops. I only tested with omap2plus_defconfig, I should have also checked 
omap1_defconfig. I will fix this in next version.

> > +	omap_init_clocksource_32k(pbase, SZ_1K);
> 
> Should this be called if pbase == NULL?
> 

The first thing in the function we do is to check for pbase != NULL, and
returns error; so I did not put check again here.


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

I believe, the original code itself was written with the assumption that,
there will not be any failures from omap_32k_timer_init(), I did not modify
it, since I can not test it here at my end. Don't have omap1 board with me.

If you help me in validating it, I will create a patch for this cleanup.

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

Some of the things I know, they need cleanup,

	- omap1:
		- Handle return value check omap_init_clocksource_32k()
		- Return value check for omap_32k_timer_init()
		- Remove usage of OMAP_32K_TIMER in omap_32k_timer_usable()

I volunteer myself for this, but need your help in validating them. Please 
let me know if you have anything else in your mind.


> >  	omap_init_32k_timer();
> >  	return true;
> > diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> > index ecec873..d720f58 100644
> > --- a/arch/arm/mach-omap2/timer.c
> > +++ b/arch/arm/mach-omap2/timer.c
> > @@ -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);
> > +	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.
> 

I have doubt on omap1, will this work on omap1? Do we have these api's 
available on omap1?

> > +	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) {
> > @@ -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.
> 

Ok, I will change it accordingly in my next version.

> > + * 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);
> > diff --git a/arch/arm/plat-omap/counter_32k.c b/arch/arm/plat-omap/counter_32k.c
> > index 5068fe5..3e3cdab 100644
> > --- a/arch/arm/plat-omap/counter_32k.c
> > +++ b/arch/arm/plat-omap/counter_32k.c
> > @@ -27,19 +27,20 @@
> >
> >  #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.
> 

Not sure whether it is question or information, Yes, this applies to omap1 
also.

> >  /*
> >   * 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;
> > +
> > +	/*
> > +	 * 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/
> 

Thanks,
Vaibhav


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

* [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
@ 2012-04-27  5:51       ` Hiremath, Vaibhav
  0 siblings, 0 replies; 30+ messages in thread
From: Hiremath, Vaibhav @ 2012-04-27  5:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Apr 27, 2012 at 03:56:56, Hilman, Kevin wrote:
> 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.
> > 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 <hvaibhav@ti.com>
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > Reviewed-by: 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: Kevin Hilman <khilman@ti.com>
> > Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
> > Cc: Ming Lei <tom.leiming@gmail.com>
> > ---
> > 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?
> 

Yes, I meant V4 here.

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

Great, thanks for testing it on OMAP1.

> That being said, a few minor comments below...
> 
> >  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(-)
> >
> > diff --git a/arch/arm/mach-omap1/timer32k.c b/arch/arm/mach-omap1/timer32k.c
> > index 325b9a0..6262e11 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	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;
> 
> 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]
> 

Oops. I only tested with omap2plus_defconfig, I should have also checked 
omap1_defconfig. I will fix this in next version.

> > +	omap_init_clocksource_32k(pbase, SZ_1K);
> 
> Should this be called if pbase == NULL?
> 

The first thing in the function we do is to check for pbase != NULL, and
returns error; so I did not put check again here.


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

I believe, the original code itself was written with the assumption that,
there will not be any failures from omap_32k_timer_init(), I did not modify
it, since I can not test it here at my end. Don't have omap1 board with me.

If you help me in validating it, I will create a patch for this cleanup.

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

Some of the things I know, they need cleanup,

	- omap1:
		- Handle return value check omap_init_clocksource_32k()
		- Return value check for omap_32k_timer_init()
		- Remove usage of OMAP_32K_TIMER in omap_32k_timer_usable()

I volunteer myself for this, but need your help in validating them. Please 
let me know if you have anything else in your mind.


> >  	omap_init_32k_timer();
> >  	return true;
> > diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> > index ecec873..d720f58 100644
> > --- a/arch/arm/mach-omap2/timer.c
> > +++ b/arch/arm/mach-omap2/timer.c
> > @@ -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);
> > +	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.
> 

I have doubt on omap1, will this work on omap1? Do we have these api's 
available on omap1?

> > +	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) {
> > @@ -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.
> 

Ok, I will change it accordingly in my next version.

> > + * 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);
> > diff --git a/arch/arm/plat-omap/counter_32k.c b/arch/arm/plat-omap/counter_32k.c
> > index 5068fe5..3e3cdab 100644
> > --- a/arch/arm/plat-omap/counter_32k.c
> > +++ b/arch/arm/plat-omap/counter_32k.c
> > @@ -27,19 +27,20 @@
> >
> >  #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.
> 

Not sure whether it is question or information, Yes, this applies to omap1 
also.

> >  /*
> >   * 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;
> > +
> > +	/*
> > +	 * 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/
> 

Thanks,
Vaibhav

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

* Re: [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
  2012-04-27  5:31             ` Hiremath, Vaibhav
@ 2012-05-01 17:03               ` Kevin Hilman
  -1 siblings, 0 replies; 30+ messages in thread
From: Kevin Hilman @ 2012-05-01 17:03 UTC (permalink / raw)
  To: Hiremath, Vaibhav
  Cc: Paul Walmsley, linux-omap, tony, Shilimkar, Santosh, Cousson,
	Benoit, linux-arm-kernel, Balbi, Felipe, DebBarma, Tarun Kanti,
	Ming Lei

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

> On Fri, Apr 27, 2012 at 04:08:07, Paul Walmsley wrote:
>> On Thu, 26 Apr 2012, Paul Walmsley wrote:
>> 
>> > 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().
>> 
>
> What about OMAP1 architecture? Will it work? 

You'll need to move the ioremap/clk* usage into OMAP1 code
(mach-omap1/timer32k.c), and move the omap_hwmod_enable() into OMAP2+
code (mach-omap2/timer.c).

Then make the omap_init_clocksource_32k() just take the base address.

If counter_32k were a real driver (and it should be) this is how things
would work.  For example, the GPIO driver is shared between OMAP1 and
OMAP2+ and any SoC specific init is done in the SoC specific device init
code instead of the driver.

Hope this helps,

Kevin


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

* [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
@ 2012-05-01 17:03               ` Kevin Hilman
  0 siblings, 0 replies; 30+ messages in thread
From: Kevin Hilman @ 2012-05-01 17:03 UTC (permalink / raw)
  To: linux-arm-kernel

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

> On Fri, Apr 27, 2012 at 04:08:07, Paul Walmsley wrote:
>> On Thu, 26 Apr 2012, Paul Walmsley wrote:
>> 
>> > 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().
>> 
>
> What about OMAP1 architecture? Will it work? 

You'll need to move the ioremap/clk* usage into OMAP1 code
(mach-omap1/timer32k.c), and move the omap_hwmod_enable() into OMAP2+
code (mach-omap2/timer.c).

Then make the omap_init_clocksource_32k() just take the base address.

If counter_32k were a real driver (and it should be) this is how things
would work.  For example, the GPIO driver is shared between OMAP1 and
OMAP2+ and any SoC specific init is done in the SoC specific device init
code instead of the driver.

Hope this helps,

Kevin

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

end of thread, other threads:[~2012-05-01 17:03 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-25 13:58 [PATCH-V5 0/3] ARM: OMAP: Make OMAP clocksource source selection runtime Vaibhav Hiremath
2012-04-25 13:58 ` Vaibhav Hiremath
2012-04-25 13:58 ` [PATCH-V5 1/3] ARM: OMAP2/3: Add idle_st bits for ST_32KSYNC timer to prcm-common header Vaibhav Hiremath
2012-04-25 13:58   ` Vaibhav Hiremath
2012-04-26 19:25   ` Paul Walmsley
2012-04-26 19:25     ` Paul Walmsley
2012-04-25 13:58 ` [PATCH-V5 2/3] ARM: OMAP2/3: hwmod data: Add 32k-sync timer data to hwmod database Vaibhav Hiremath
2012-04-25 13:58   ` Vaibhav Hiremath
2012-04-26 19:26   ` Paul Walmsley
2012-04-26 19:26     ` Paul Walmsley
2012-04-25 13:59 ` [PATCH-V5 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param Vaibhav Hiremath
2012-04-25 13:59   ` Vaibhav Hiremath
2012-04-26 15:10   ` Paul Walmsley
2012-04-26 15:10     ` Paul Walmsley
2012-04-26 16:20     ` Hiremath, Vaibhav
2012-04-26 16:20       ` Hiremath, Vaibhav
2012-04-26 16:40     ` Hiremath, Vaibhav
2012-04-26 16:40       ` Hiremath, Vaibhav
2012-04-26 19:30       ` Paul Walmsley
2012-04-26 19:30         ` Paul Walmsley
2012-04-26 22:38         ` Paul Walmsley
2012-04-26 22:38           ` Paul Walmsley
2012-04-27  5:31           ` Hiremath, Vaibhav
2012-04-27  5:31             ` Hiremath, Vaibhav
2012-05-01 17:03             ` Kevin Hilman
2012-05-01 17:03               ` Kevin Hilman
2012-04-26 22:26   ` Kevin Hilman
2012-04-26 22:26     ` Kevin Hilman
2012-04-27  5:51     ` Hiremath, Vaibhav
2012-04-27  5:51       ` Hiremath, Vaibhav

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.