[PATCH 0/5] ARM: OMAP2+: System timer updates

[PATCH 0/5] ARM: OMAP2+: System timer updates

[Jon Hunter]

This series consists mainly of clean-ups for clockevents and
clocksource timers on OMAP2+ devices. The most significant change
in functionality comes from the 5th patch which is changing the
selection of the clocksource timer for OMAP3 and AM335x devices
when gptimers are used for clocksource. This change came about from
Vaibhav Bedia's series for AM335x [1]. See patch for more details on
the exact nature of the change.

Boot tested with and without  device-tree on OMAP2420 H4,
OMAP3430 SDP, OMAP3430 Beagle Board, OMAP4430 SDP and
AM335x EVM (AM335x only supports device-tree boot).

This series is based upon ARM-SoC next branch.

[1] https://patchwork.kernel.org/patch/1921421/

Jon Hunter (5):
  ARM: OMAP2+: Display correct system timer name
  ARM: OMAP2+: Remove hard-coded test on timer ID
  ARM: OMAP2+: Simplify system timer clock definitions
  ARM: OMAP2+: Simplify system timers definitions
  ARM: OMAP3: Update clocksource timer selection

 arch/arm/mach-omap2/board-cm-t3517.c |    2 +-
 arch/arm/mach-omap2/board-generic.c  |    2 +-
 arch/arm/mach-omap2/common.h         |    3 +-
 arch/arm/mach-omap2/timer.c          |  134 ++++++++++++++++------------------
 4 files changed, 67 insertions(+), 74 deletions(-)

-- 
1.7.10.4

[PATCH 1/5] ARM: OMAP2+: Display correct system timer name

[Jon Hunter]

Currently on boot, when displaying the name of the gptimer used for
clockevents and clocksource timers, the timer ID is shown. However,
when booting with device-tree, the timer ID is not used to select a
gptimer but a timer property. Hence, it is possible that the timer
selected when booting with device-tree does not match the ID shown.
Therefore, instead display the HWMOD name of the gptimer and use
the HWMOD name as the name of clockevent and clocksource timer (if a
gptimer is used).

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/timer.c |   44 +++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 72c2ca1..18cb856 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -71,6 +71,9 @@
 #define INCREMENTER_DENUMERATOR_RELOAD_OFFSET		0x14
 #define NUMERATOR_DENUMERATOR_MASK			0xfffff000
 
+/* Timer name needs to be big enough to store a string of "timerXX" */
+static char timer_name[10];
+
 /* Clockevent code */
 
 static struct omap_dm_timer clkev;
@@ -129,7 +132,6 @@ static void omap2_gp_timer_set_mode(enum clock_event_mode mode,
 }
 
 static struct clock_event_device clockevent_gpt = {
-	.name		= "gp_timer",
 	.features       = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
 	.rating		= 300,
 	.set_next_event	= omap2_gp_timer_set_next_event,
@@ -214,13 +216,12 @@ static u32 __init omap_dm_timer_get_errata(void)
 }
 
 static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
-						int gptimer_id,
-						const char *fck_source,
-						const char *property,
-						int posted)
+					 int gptimer_id,
+					 const char **name,
+					 const char *fck_source,
+					 const char *property,
+					 int posted)
 {
-	char name[10]; /* 10 = sizeof("gptXX_Xck0") */
-	const char *oh_name;
 	struct device_node *np;
 	struct omap_hwmod *oh;
 	struct resource irq, mem;
@@ -231,8 +232,8 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 		if (!np)
 			return -ENODEV;
 
-		of_property_read_string_index(np, "ti,hwmods", 0, &oh_name);
-		if (!oh_name)
+		of_property_read_string_index(np, "ti,hwmods", 0, name);
+		if (!name)
 			return -ENODEV;
 
 		timer->irq = irq_of_parse_and_map(np, 0);
@@ -246,11 +247,11 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 		if (omap_dm_timer_reserve_systimer(gptimer_id))
 			return -ENODEV;
 
-		sprintf(name, "timer%d", gptimer_id);
-		oh_name = name;
+		sprintf(timer_name, "timer%d", gptimer_id);
+		*name = timer_name;
 	}
 
-	oh = omap_hwmod_lookup(oh_name);
+	oh = omap_hwmod_lookup(*name);
 	if (!oh)
 		return -ENODEV;
 
@@ -294,7 +295,7 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 		}
 	}
 
-	omap_hwmod_setup_one(oh_name);
+	omap_hwmod_setup_one(*name);
 	omap_hwmod_enable(oh);
 	__omap_dm_timer_init_regs(timer);
 
@@ -326,8 +327,8 @@ static void __init omap2_gp_clockevent_init(int gptimer_id,
 	 */
 	__omap_dm_timer_override_errata(&clkev, OMAP_TIMER_ERRATA_I103_I767);
 
-	res = omap_dm_timer_init_one(&clkev, gptimer_id, fck_source, property,
-				     OMAP_TIMER_POSTED);
+	res = omap_dm_timer_init_one(&clkev, gptimer_id, &clockevent_gpt.name,
+				     fck_source, property, OMAP_TIMER_POSTED);
 	BUG_ON(res);
 
 	omap2_gp_timer_irq.dev_id = &clkev;
@@ -341,8 +342,8 @@ static void __init omap2_gp_clockevent_init(int gptimer_id,
 					3, /* Timer internal resynch latency */
 					0xffffffff);
 
-	pr_info("OMAP clockevent source: GPTIMER%d at %lu Hz\n",
-		gptimer_id, clkev.rate);
+	pr_info("OMAP clockevent source: %s at %lu Hz\n", clockevent_gpt.name,
+		clkev.rate);
 }
 
 /* Clocksource code */
@@ -359,7 +360,6 @@ static cycle_t clocksource_read_cycles(struct clocksource *cs)
 }
 
 static struct clocksource clocksource_gpt = {
-	.name		= "gp_timer",
 	.rating		= 300,
 	.read		= clocksource_read_cycles,
 	.mask		= CLOCKSOURCE_MASK(32),
@@ -448,8 +448,8 @@ static void __init omap2_gptimer_clocksource_init(int gptimer_id,
 
 	clksrc.errata = omap_dm_timer_get_errata();
 
-	res = omap_dm_timer_init_one(&clksrc, gptimer_id, fck_source, NULL,
-				     OMAP_TIMER_NONPOSTED);
+	res = omap_dm_timer_init_one(&clksrc, gptimer_id, &clocksource_gpt.name,
+				     fck_source, NULL, OMAP_TIMER_NONPOSTED);
 	BUG_ON(res);
 
 	__omap_dm_timer_load_start(&clksrc,
@@ -461,8 +461,8 @@ static void __init omap2_gptimer_clocksource_init(int gptimer_id,
 		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);
+		pr_info("OMAP clocksource: %s at %lu Hz\n",
+			clocksource_gpt.name, clksrc.rate);
 }
 
 #ifdef CONFIG_SOC_HAS_REALTIME_COUNTER
-- 
1.7.10.4

[PATCH 1/5] ARM: OMAP2+: Display correct system timer name

[Bedia, Vaibhav]

Hi Jon,

On Wed, Jan 30, 2013 at 22:34:27, Hunter, Jon wrote:
> Currently on boot, when displaying the name of the gptimer used for
> clockevents and clocksource timers, the timer ID is shown. However,
> when booting with device-tree, the timer ID is not used to select a
> gptimer but a timer property. Hence, it is possible that the timer
> selected when booting with device-tree does not match the ID shown.
> Therefore, instead display the HWMOD name of the gptimer and use
> the HWMOD name as the name of clockevent and clocksource timer (if a
> gptimer is used).
> 
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> ---
>  arch/arm/mach-omap2/timer.c |   44 +++++++++++++++++++++----------------------
>  1 file changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index 72c2ca1..18cb856 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -71,6 +71,9 @@
>  #define INCREMENTER_DENUMERATOR_RELOAD_OFFSET		0x14
>  #define NUMERATOR_DENUMERATOR_MASK			0xfffff000
>  
> +/* Timer name needs to be big enough to store a string of "timerXX" */
> +static char timer_name[10];
> +

Why not move this inside omap_dm_timer_init_one()?

Regards,
Vaibhav

[PATCH 1/5] ARM: OMAP2+: Display correct system timer name

[Jon Hunter]

Hi Vaibhav,

On 02/01/2013 02:41 AM, Bedia, Vaibhav wrote:
> Hi Jon,
> 
> On Wed, Jan 30, 2013 at 22:34:27, Hunter, Jon wrote:
>> Currently on boot, when displaying the name of the gptimer used for
>> clockevents and clocksource timers, the timer ID is shown. However,
>> when booting with device-tree, the timer ID is not used to select a
>> gptimer but a timer property. Hence, it is possible that the timer
>> selected when booting with device-tree does not match the ID shown.
>> Therefore, instead display the HWMOD name of the gptimer and use
>> the HWMOD name as the name of clockevent and clocksource timer (if a
>> gptimer is used).no
>>
>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>> ---
>>  arch/arm/mach-omap2/timer.c |   44 +++++++++++++++++++++----------------------
>>  1 file changed, 22 insertions(+), 22 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
>> index 72c2ca1..18cb856 100644
>> --- a/arch/arm/mach-omap2/timer.c
>> +++ b/arch/arm/mach-omap2/timer.c
>> @@ -71,6 +71,9 @@
>>  #define INCREMENTER_DENUMERATOR_RELOAD_OFFSET		0x14
>>  #define NUMERATOR_DENUMERATOR_MASK			0xfffff000
>>  
>> +/* Timer name needs to be big enough to store a string of "timerXX" */
>> +static char timer_name[10];
>> +
> 
> Why not move this inside omap_dm_timer_init_one()?

In the non-DT case, the name member of the clocksource/event struct will
point to this array and so it needs to reside in memory permanently and
not just temporary. Once we migrate completely to DT then we will be
able to remove this completely. See following snippet ...

-		sprintf(name, "timer%d", gptimer_id);
-		oh_name = name;
+		sprintf(timer_name, "timer%d", gptimer_id);
+		*name = timer_name;

Cheers
Jon

[PATCH 1/5] ARM: OMAP2+: Display correct system timer name

[Bedia, Vaibhav]

On Fri, Feb 01, 2013 at 14:23:43, Hunter, Jon wrote:
[...]
> >>  
> >> +/* Timer name needs to be big enough to store a string of "timerXX" */
> >> +static char timer_name[10];
> >> +
> > 
> > Why not move this inside omap_dm_timer_init_one()?
> 
> In the non-DT case, the name member of the clocksource/event struct will
> point to this array and so it needs to reside in memory permanently and
> not just temporary. Once we migrate completely to DT then we will be
> able to remove this completely. See following snippet ...
> 
> -		sprintf(name, "timer%d", gptimer_id);
> -		oh_name = name;
> +		sprintf(timer_name, "timer%d", gptimer_id);
> +		*name = timer_name;

Ok. But in case of non-DT boot if someone selects gptimers for both clkevt and
clksrc, both the name members will end up pointing to the same memory location.
To be specific, in the current code the clkevt timer name will point to the clksrc
name. This won't be noticeable during boot since the clkevt name gets printed
before it is over-written.

Regards,
Vaibhav

[PATCH 1/5] ARM: OMAP2+: Display correct system timer name

[Jon Hunter]

On 02/01/2013 03:31 AM, Bedia, Vaibhav wrote:
> On Fri, Feb 01, 2013 at 14:23:43, Hunter, Jon wrote:
> [...]
>>>>  
>>>> +/* Timer name needs to be big enough to store a string of "timerXX" */
>>>> +static char timer_name[10];
>>>> +
>>>
>>> Why not move this inside omap_dm_timer_init_one()?
>>
>> In the non-DT case, the name member of the clocksource/event struct will
>> point to this array and so it needs to reside in memory permanently and
>> not just temporary. Once we migrate completely to DT then we will be
>> able to remove this completely. See following snippet ...
>>
>> -		sprintf(name, "timer%d", gptimer_id);
>> -		oh_name = name;
>> +		sprintf(timer_name, "timer%d", gptimer_id);
>> +		*name = timer_name;
> 
> Ok. But in case of non-DT boot if someone selects gptimers for both clkevt and
> clksrc, both the name members will end up pointing to the same memory location.
> To be specific, in the current code the clkevt timer name will point to the clksrc
> name. This won't be noticeable during boot since the clkevt name gets printed
> before it is over-written.

Yes you are right! Good catch. Will fix that.

Cheers
Jon

[PATCH 2/5] ARM: OMAP2+: Remove hard-coded test on timer ID

[Jon Hunter]

Currently, when configuring the clock-events and clock-source timers
for OMAP2+ devices, we check whether the timer ID is 12 before
attempting to set the parent clock for the timer.

This test was added for OMAP3 general purpose devices (no security
features enabled) that a 12th timer available but unlike the other
timers only has a single functional clock source. Calling
clk_set_parent() for this 12th timer would always return an error
because there is only one choice for a parent clock. Therefore,
this hard-coded timer ID test was added.

To avoid this timer ID test, simply check to see if the timer's current
parent clock is the desired parent clock and only call clk_set_parent()
if this is not the case.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/timer.c |   26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 18cb856..69fe623b 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -225,6 +225,7 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 	struct device_node *np;
 	struct omap_hwmod *oh;
 	struct resource irq, mem;
+	struct clk *src;
 	int r = 0;
 
 	if (of_have_populated_dt()) {
@@ -279,22 +280,19 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 	if (IS_ERR(timer->fclk))
 		return -ENODEV;
 
-	/* FIXME: Need to remove hard-coded test on timer ID */
-	if (gptimer_id != 12) {
-		struct clk *src;
-
-		src = clk_get(NULL, fck_source);
-		if (IS_ERR(src)) {
-			r = -EINVAL;
-		} else {
-			r = clk_set_parent(timer->fclk, src);
-			if (IS_ERR_VALUE(r))
-				pr_warn("%s: %s cannot set source\n",
-					__func__, oh->name);
-			clk_put(src);
-		}
+	src = clk_get(NULL, fck_source);
+	if (IS_ERR(src))
+		return -EINVAL;
+
+	if (clk_get_parent(timer->fclk) != src) {
+		r = clk_set_parent(timer->fclk, src);
+		if (IS_ERR_VALUE(r))
+			pr_warn("%s: %s cannot set source\n",
+				__func__, oh->name);
 	}
 
+	clk_put(src);
+
 	omap_hwmod_setup_one(*name);
 	omap_hwmod_enable(oh);
 	__omap_dm_timer_init_regs(timer);
-- 
1.7.10.4

[PATCH 3/5] ARM: OMAP2+: Simplify system timer clock definitions

[Jon Hunter]

In commit c59b537 (ARM: OMAP2+: Simplify dmtimer clock aliases), new
clock aliases for dmtimers were added to simplify the code. These clock
aliases can also be used when configuring the system timers and allow us
to remove the current definitions, simplifying the code.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/timer.c |   37 ++++++++++++++-----------------------
 1 file changed, 14 insertions(+), 23 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 69fe623b..e7df00a 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -57,15 +57,6 @@
 #include "common.h"
 #include "powerdomain.h"
 
-/* Parent clocks, eventually these will come from the clock framework */
-
-#define OMAP2_MPU_SOURCE	"sys_ck"
-#define OMAP3_MPU_SOURCE	OMAP2_MPU_SOURCE
-#define OMAP4_MPU_SOURCE	"sys_clkin_ck"
-#define OMAP2_32K_SOURCE	"func_32k_ck"
-#define OMAP3_32K_SOURCE	"omap_32k_fck"
-#define OMAP4_32K_SOURCE	"sys_32k_ck"
-
 #define REALTIME_COUNTER_BASE				0x48243200
 #define INCREMENTER_NUMERATOR_OFFSET			0x10
 #define INCREMENTER_DENUMERATOR_RELOAD_OFFSET		0x14
@@ -564,27 +555,27 @@ void __init omap##name##_sync32k_timer_init(void)		\
 }
 
 #ifdef CONFIG_ARCH_OMAP2
-OMAP_SYS_32K_TIMER_INIT(2, 1, OMAP2_32K_SOURCE, "ti,timer-alwon",
-			2, OMAP2_MPU_SOURCE);
+OMAP_SYS_32K_TIMER_INIT(2, 1, "timer_32k_ck", "ti,timer-alwon",
+			2, "timer_sys_ck");
 #endif /* CONFIG_ARCH_OMAP2 */
 
 #ifdef CONFIG_ARCH_OMAP3
-OMAP_SYS_32K_TIMER_INIT(3, 1, OMAP3_32K_SOURCE, "ti,timer-alwon",
-			2, OMAP3_MPU_SOURCE);
-OMAP_SYS_32K_TIMER_INIT(3_secure, 12, OMAP3_32K_SOURCE, "ti,timer-secure",
-			2, OMAP3_MPU_SOURCE);
-OMAP_SYS_GP_TIMER_INIT(3_gp, 1, OMAP3_MPU_SOURCE, "ti,timer-alwon",
-		       2, OMAP3_MPU_SOURCE);
+OMAP_SYS_32K_TIMER_INIT(3, 1, "timer_32k_ck", "ti,timer-alwon",
+			2, "timer_sys_ck");
+OMAP_SYS_32K_TIMER_INIT(3_secure, 12, "secure_32k_fck", "ti,timer-secure",
+			2, "timer_sys_ck");
+OMAP_SYS_GP_TIMER_INIT(3_gp, 1, "timer_sys_ck", "ti,timer-alwon",
+		       2, "timer_sys_ck");
 #endif /* CONFIG_ARCH_OMAP3 */
 
 #ifdef CONFIG_SOC_AM33XX
-OMAP_SYS_GP_TIMER_INIT(3_am33xx, 1, OMAP4_MPU_SOURCE, "ti,timer-alwon",
-		       2, OMAP4_MPU_SOURCE);
+OMAP_SYS_GP_TIMER_INIT(3_am33xx, 1, "timer_sys_ck", "ti,timer-alwon",
+		       2, "timer_sys_ck");
 #endif /* CONFIG_SOC_AM33XX */
 
 #ifdef CONFIG_ARCH_OMAP4
-OMAP_SYS_32K_TIMER_INIT(4, 1, OMAP4_32K_SOURCE, "ti,timer-alwon",
-			2, OMAP4_MPU_SOURCE);
+OMAP_SYS_32K_TIMER_INIT(4, 1, "timer_32k_ck", "ti,timer-alwon",
+			2, "timer_sys_ck");
 #ifdef CONFIG_LOCAL_TIMERS
 static DEFINE_TWD_LOCAL_TIMER(twd_local_timer, OMAP44XX_LOCAL_TWD_BASE, 29);
 void __init omap4_local_timer_init(void)
@@ -613,8 +604,8 @@ void __init omap4_local_timer_init(void)
 #endif /* CONFIG_ARCH_OMAP4 */
 
 #ifdef CONFIG_SOC_OMAP5
-OMAP_SYS_32K_TIMER_INIT(5, 1, OMAP4_32K_SOURCE, "ti,timer-alwon",
-			2, OMAP4_MPU_SOURCE);
+OMAP_SYS_32K_TIMER_INIT(5, 1, "timer_32k_ck", "ti,timer-alwon",
+			2, "timer_sys_ck");
 void __init omap5_realtime_timer_init(void)
 {
 	int err;
-- 
1.7.10.4

[PATCH 4/5] ARM: OMAP2+: Simplify system timers definitions

[Jon Hunter]

There is a lot of redundancy in the definitions for the various system
timers for OMAP2+ devices. For example, the omap3_am33xx_gptimer_timer_init()
function is the same as the omap3_gp_gptimer_timer_init() function and the
function omap2_sync32k_timer_init() can be re-used for OMAP4/5 devices.
Therefore, consolidate the definitions to simplify the code.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/board-cm-t3517.c |    2 +-
 arch/arm/mach-omap2/board-generic.c  |    2 +-
 arch/arm/mach-omap2/common.h         |    3 +--
 arch/arm/mach-omap2/timer.c          |   23 +++++++++--------------
 4 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-omap2/board-cm-t3517.c b/arch/arm/mach-omap2/board-cm-t3517.c
index 6a9529a..7c1ad68 100644
--- a/arch/arm/mach-omap2/board-cm-t3517.c
+++ b/arch/arm/mach-omap2/board-cm-t3517.c
@@ -297,6 +297,6 @@ MACHINE_START(CM_T3517, "Compulab CM-T3517")
 	.handle_irq	= omap3_intc_handle_irq,
 	.init_machine	= cm_t3517_init,
 	.init_late	= am35xx_init_late,
-	.init_time	= omap3_gp_gptimer_timer_init,
+	.init_time	= omap3_gptimer_timer_init,
 	.restart	= omap3xxx_restart,
 MACHINE_END
diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
index 2590463..dfd9f48 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -138,7 +138,7 @@ DT_MACHINE_START(AM33XX_DT, "Generic AM33XX (Flattened Device Tree)")
 	.init_irq	= omap_intc_of_init,
 	.handle_irq	= omap3_intc_handle_irq,
 	.init_machine	= omap_generic_init,
-	.init_time	= omap3_am33xx_gptimer_timer_init,
+	.init_time	= omap3_gptimer_timer_init,
 	.dt_compat	= am33xx_boards_compat,
 MACHINE_END
 #endif
diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
index b435027..594ab3b 100644
--- a/arch/arm/mach-omap2/common.h
+++ b/arch/arm/mach-omap2/common.h
@@ -82,8 +82,7 @@ extern void omap2_init_common_infrastructure(void);
 extern void omap2_sync32k_timer_init(void);
 extern void omap3_sync32k_timer_init(void);
 extern void omap3_secure_sync32k_timer_init(void);
-extern void omap3_gp_gptimer_timer_init(void);
-extern void omap3_am33xx_gptimer_timer_init(void);
+extern void omap3_gptimer_timer_init(void);
 extern void omap4_local_timer_init(void);
 extern void omap5_realtime_timer_init(void);
 
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index e7df00a..af20be7 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -554,33 +554,30 @@ void __init omap##name##_sync32k_timer_init(void)		\
 		omap2_sync32k_clocksource_init();			\
 }
 
-#ifdef CONFIG_ARCH_OMAP2
+#if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP4) ||		\
+	defined(CONFIG_SOC_OMAP5)
 OMAP_SYS_32K_TIMER_INIT(2, 1, "timer_32k_ck", "ti,timer-alwon",
 			2, "timer_sys_ck");
-#endif /* CONFIG_ARCH_OMAP2 */
+#endif
 
 #ifdef CONFIG_ARCH_OMAP3
 OMAP_SYS_32K_TIMER_INIT(3, 1, "timer_32k_ck", "ti,timer-alwon",
 			2, "timer_sys_ck");
 OMAP_SYS_32K_TIMER_INIT(3_secure, 12, "secure_32k_fck", "ti,timer-secure",
 			2, "timer_sys_ck");
-OMAP_SYS_GP_TIMER_INIT(3_gp, 1, "timer_sys_ck", "ti,timer-alwon",
-		       2, "timer_sys_ck");
 #endif /* CONFIG_ARCH_OMAP3 */
 
-#ifdef CONFIG_SOC_AM33XX
-OMAP_SYS_GP_TIMER_INIT(3_am33xx, 1, "timer_sys_ck", "ti,timer-alwon",
+#if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_SOC_AM33XX)
+OMAP_SYS_GP_TIMER_INIT(3, 1, "timer_sys_ck", "ti,timer-alwon",
 		       2, "timer_sys_ck");
-#endif /* CONFIG_SOC_AM33XX */
+#endif
 
 #ifdef CONFIG_ARCH_OMAP4
-OMAP_SYS_32K_TIMER_INIT(4, 1, "timer_32k_ck", "ti,timer-alwon",
-			2, "timer_sys_ck");
 #ifdef CONFIG_LOCAL_TIMERS
 static DEFINE_TWD_LOCAL_TIMER(twd_local_timer, OMAP44XX_LOCAL_TWD_BASE, 29);
 void __init omap4_local_timer_init(void)
 {
-	omap4_sync32k_timer_init();
+	omap2_sync32k_timer_init();
 	/* Local timers are not supprted on OMAP4430 ES1.0 */
 	if (omap_rev() != OMAP4430_REV_ES1_0) {
 		int err;
@@ -598,19 +595,17 @@ void __init omap4_local_timer_init(void)
 #else /* CONFIG_LOCAL_TIMERS */
 void __init omap4_local_timer_init(void)
 {
-	omap4_sync32k_timer_init();
+	omap2_sync32k_timer_init();
 }
 #endif /* CONFIG_LOCAL_TIMERS */
 #endif /* CONFIG_ARCH_OMAP4 */
 
 #ifdef CONFIG_SOC_OMAP5
-OMAP_SYS_32K_TIMER_INIT(5, 1, "timer_32k_ck", "ti,timer-alwon",
-			2, "timer_sys_ck");
 void __init omap5_realtime_timer_init(void)
 {
 	int err;
 
-	omap5_sync32k_timer_init();
+	omap2_sync32k_timer_init();
 	realtime_counter_init();
 
 	err = arch_timer_of_register();
-- 
1.7.10.4

[PATCH 4/5] ARM: OMAP2+: Simplify system timers definitions

[Igor Grinberg]

On 01/30/13 19:04, Jon Hunter wrote:
> There is a lot of redundancy in the definitions for the various system
> timers for OMAP2+ devices. For example, the omap3_am33xx_gptimer_timer_init()
> function is the same as the omap3_gp_gptimer_timer_init() function and the
> function omap2_sync32k_timer_init() can be re-used for OMAP4/5 devices.
> Therefore, consolidate the definitions to simplify the code.
> 
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>

Acked-by: Igor Grinberg <grinberg@compulab.co.il>


-- 
Regards,
Igor.

[PATCH 4/5] ARM: OMAP2+: Simplify system timers definitions

[Jon Hunter]

On 01/30/2013 11:04 AM, Jon Hunter wrote:
> There is a lot of redundancy in the definitions for the various system
> timers for OMAP2+ devices. For example, the omap3_am33xx_gptimer_timer_init()
> function is the same as the omap3_gp_gptimer_timer_init() function and the
> function omap2_sync32k_timer_init() can be re-used for OMAP4/5 devices.

After further testing, I have found that using the
omap2_sync32k_timer_init() for omap4/5 devices does not work for the
case where we boot with boot parameter "clocksource=gp_timer". The
problem is omap4/5 devices, unlike omap2/3 devices, does not have a
clock alias for "timer_sys_ck" with NULL as the device name. Therefore
we fail to find the parent clock and boot fails.

So I am going to update this patch so that omap4 and omap5 both use
omap4_sync32k_timer_init() and just get rid of the extra function
defined for omap5.

Hope this makes sense.

Cheers
Jon

[PATCH 5/5] ARM: OMAP3: Update clocksource timer selection

[Jon Hunter]

When booting with device-tree for OMAP3 and AM335x devices and a gptimer
is used as the clocksource (which is always the case for AM335x), a
gptimer located in a power domain that is not always-on is selected.
Ideally we should use a gptimer located in a power domain that is always
on (such as the wake-up domain) so that time can be maintained during a
kernel suspend without keeping on additional power domains unnecessarily.

In order to fix this so that we can select a gptimer located in a power
domain that is always-on, the following changes were made ...
1. Currently, only when selecting a gptimer to use for a clockevent
   timer, do we pass a timer property that can be used to select a
   specific gptimer. Change this so that we can pass a property when
   selecting a gptimer to use for a clocksource timer too.
2. Currently, when selecting either a gptimer to use for a clockevent
   timer or a clocksource timer and no timer property is passed, then
   the first available timer is selected regardless of the properties
   it has. Change this so that if no properties are passed, then a timer
   that does not have additional features (such as always-on, dsp-irq,
   pwm, and secure) is selected.

Please note that using a gptimer for both clocksource and clockevents
can have a system power impact during idle. The reason being is that
OMAP and AMxxx devices typically only have one gptimer in a power domain
that is always-on. Therefore when the kernel is idle both the clocksource
and clockevent timers will be active and this will keep additional power
domains on. During kernel suspend, only the clocksource timer is active
and therefore, it is better to use a gptimer in a power domain that is
always-on for clocksource.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/timer.c |   32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index af20be7..acf9f82 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -163,6 +163,12 @@ static struct device_node * __init omap_get_timer_dt(struct of_device_id *match,
 		if (property && !of_get_property(np, property, NULL))
 			continue;
 
+		if (!property && (of_get_property(np, "ti,timer-alwon", NULL) ||
+				  of_get_property(np, "ti,timer-dsp", NULL) ||
+				  of_get_property(np, "ti,timer-pwm", NULL) ||
+				  of_get_property(np, "ti,timer-secure", NULL)))
+			continue;
+
 		of_add_property(np, &device_disabled);
 		return np;
 	}
@@ -431,14 +437,16 @@ static int __init __maybe_unused omap2_sync32k_clocksource_init(void)
 }
 
 static void __init omap2_gptimer_clocksource_init(int gptimer_id,
-						const char *fck_source)
+						  const char *fck_source,
+						  const char *property)
 {
 	int res;
 
 	clksrc.errata = omap_dm_timer_get_errata();
 
 	res = omap_dm_timer_init_one(&clksrc, gptimer_id, &clocksource_gpt.name,
-				     fck_source, NULL, OMAP_TIMER_NONPOSTED);
+				     fck_source, property,
+				     OMAP_TIMER_NONPOSTED);
 	BUG_ON(res);
 
 	__omap_dm_timer_load_start(&clksrc,
@@ -533,23 +541,25 @@ static inline void __init realtime_counter_init(void)
 #endif
 
 #define OMAP_SYS_GP_TIMER_INIT(name, clkev_nr, clkev_src, clkev_prop,	\
-			       clksrc_nr, clksrc_src)			\
+			       clksrc_nr, clksrc_src, clksrc_prop)	\
 void __init omap##name##_gptimer_timer_init(void)			\
 {									\
 	omap_dmtimer_init();						\
 	omap2_gp_clockevent_init((clkev_nr), clkev_src, clkev_prop);	\
-	omap2_gptimer_clocksource_init((clksrc_nr), clksrc_src);	\
+	omap2_gptimer_clocksource_init((clksrc_nr), clksrc_src,		\
+					clksrc_prop);			\
 }
 
 #define OMAP_SYS_32K_TIMER_INIT(name, clkev_nr, clkev_src, clkev_prop,	\
-				clksrc_nr, clksrc_src)			\
+				clksrc_nr, clksrc_src, clksrc_prop)	\
 void __init omap##name##_sync32k_timer_init(void)		\
 {									\
 	omap_dmtimer_init();						\
 	omap2_gp_clockevent_init((clkev_nr), clkev_src, clkev_prop);	\
 	/* Enable the use of clocksource="gp_timer" kernel parameter */	\
 	if (use_gptimer_clksrc)						\
-		omap2_gptimer_clocksource_init((clksrc_nr), clksrc_src);\
+		omap2_gptimer_clocksource_init((clksrc_nr), clksrc_src,	\
+						clksrc_prop);		\
 	else								\
 		omap2_sync32k_clocksource_init();			\
 }
@@ -557,19 +567,19 @@ void __init omap##name##_sync32k_timer_init(void)		\
 #if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP4) ||		\
 	defined(CONFIG_SOC_OMAP5)
 OMAP_SYS_32K_TIMER_INIT(2, 1, "timer_32k_ck", "ti,timer-alwon",
-			2, "timer_sys_ck");
+			2, "timer_sys_ck", NULL);
 #endif
 
 #ifdef CONFIG_ARCH_OMAP3
 OMAP_SYS_32K_TIMER_INIT(3, 1, "timer_32k_ck", "ti,timer-alwon",
-			2, "timer_sys_ck");
+			2, "timer_sys_ck", NULL);
 OMAP_SYS_32K_TIMER_INIT(3_secure, 12, "secure_32k_fck", "ti,timer-secure",
-			2, "timer_sys_ck");
+			2, "timer_sys_ck", NULL);
 #endif /* CONFIG_ARCH_OMAP3 */
 
 #if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_SOC_AM33XX)
-OMAP_SYS_GP_TIMER_INIT(3, 1, "timer_sys_ck", "ti,timer-alwon",
-		       2, "timer_sys_ck");
+OMAP_SYS_GP_TIMER_INIT(3, 2, "timer_sys_ck", NULL,
+		       1, "timer_sys_ck", "ti,timer-alwon");
 #endif
 
 #ifdef CONFIG_ARCH_OMAP4
-- 
1.7.10.4

[PATCH 5/5] ARM: OMAP3: Update clocksource timer selection

[Igor Grinberg]

On 01/30/13 19:04, Jon Hunter wrote:
> When booting with device-tree for OMAP3 and AM335x devices and a gptimer
> is used as the clocksource (which is always the case for AM335x), a
> gptimer located in a power domain that is not always-on is selected.
> Ideally we should use a gptimer located in a power domain that is always
> on (such as the wake-up domain) so that time can be maintained during a
> kernel suspend without keeping on additional power domains unnecessarily.
> 
> In order to fix this so that we can select a gptimer located in a power
> domain that is always-on, the following changes were made ...
> 1. Currently, only when selecting a gptimer to use for a clockevent
>    timer, do we pass a timer property that can be used to select a
>    specific gptimer. Change this so that we can pass a property when
>    selecting a gptimer to use for a clocksource timer too.
> 2. Currently, when selecting either a gptimer to use for a clockevent
>    timer or a clocksource timer and no timer property is passed, then
>    the first available timer is selected regardless of the properties
>    it has. Change this so that if no properties are passed, then a timer
>    that does not have additional features (such as always-on, dsp-irq,
>    pwm, and secure) is selected.
> 
> Please note that using a gptimer for both clocksource and clockevents
> can have a system power impact during idle. The reason being is that
> OMAP and AMxxx devices typically only have one gptimer in a power domain
> that is always-on. Therefore when the kernel is idle both the clocksource
> and clockevent timers will be active and this will keep additional power
> domains on. During kernel suspend, only the clocksource timer is active
> and therefore, it is better to use a gptimer in a power domain that is
> always-on for clocksource.

This should explain the gptimer number switch in the
#if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_SOC_AM33XX)
section below, right?
I would really like to see that clearly stated in the commit message.
For instance:
... it is better to use a gptimer in a power domain that is
always-on for clocksource. Therefore we switch to use the first gptimer
for clocksource and the second for clockevents.

> 
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>

Apart from above,
Acked-by: Igor Grinberg <grinberg@compulab.co.il>

> ---
>  arch/arm/mach-omap2/timer.c |   32 +++++++++++++++++++++-----------
>  1 file changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index af20be7..acf9f82 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c

[...]

> @@ -557,19 +567,19 @@ void __init omap##name##_sync32k_timer_init(void)		\
>  #if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP4) ||		\
>  	defined(CONFIG_SOC_OMAP5)
>  OMAP_SYS_32K_TIMER_INIT(2, 1, "timer_32k_ck", "ti,timer-alwon",
> -			2, "timer_sys_ck");
> +			2, "timer_sys_ck", NULL);
>  #endif
>  
>  #ifdef CONFIG_ARCH_OMAP3
>  OMAP_SYS_32K_TIMER_INIT(3, 1, "timer_32k_ck", "ti,timer-alwon",
> -			2, "timer_sys_ck");
> +			2, "timer_sys_ck", NULL);
>  OMAP_SYS_32K_TIMER_INIT(3_secure, 12, "secure_32k_fck", "ti,timer-secure",
> -			2, "timer_sys_ck");
> +			2, "timer_sys_ck", NULL);
>  #endif /* CONFIG_ARCH_OMAP3 */
>  
>  #if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_SOC_AM33XX)
> -OMAP_SYS_GP_TIMER_INIT(3, 1, "timer_sys_ck", "ti,timer-alwon",
> -		       2, "timer_sys_ck");
> +OMAP_SYS_GP_TIMER_INIT(3, 2, "timer_sys_ck", NULL,
> +		       1, "timer_sys_ck", "ti,timer-alwon");
>  #endif
>  
>  #ifdef CONFIG_ARCH_OMAP4
> 

-- 
Regards,
Igor.

[PATCH 5/5] ARM: OMAP3: Update clocksource timer selection

[Jon Hunter]

On 01/31/2013 03:08 AM, Igor Grinberg wrote:
> On 01/30/13 19:04, Jon Hunter wrote:
>> When booting with device-tree for OMAP3 and AM335x devices and a gptimer
>> is used as the clocksource (which is always the case for AM335x), a
>> gptimer located in a power domain that is not always-on is selected.
>> Ideally we should use a gptimer located in a power domain that is always
>> on (such as the wake-up domain) so that time can be maintained during a
>> kernel suspend without keeping on additional power domains unnecessarily.
>>
>> In order to fix this so that we can select a gptimer located in a power
>> domain that is always-on, the following changes were made ...
>> 1. Currently, only when selecting a gptimer to use for a clockevent
>>    timer, do we pass a timer property that can be used to select a
>>    specific gptimer. Change this so that we can pass a property when
>>    selecting a gptimer to use for a clocksource timer too.
>> 2. Currently, when selecting either a gptimer to use for a clockevent
>>    timer or a clocksource timer and no timer property is passed, then
>>    the first available timer is selected regardless of the properties
>>    it has. Change this so that if no properties are passed, then a timer
>>    that does not have additional features (such as always-on, dsp-irq,
>>    pwm, and secure) is selected.
>>
>> Please note that using a gptimer for both clocksource and clockevents
>> can have a system power impact during idle. The reason being is that
>> OMAP and AMxxx devices typically only have one gptimer in a power domain
>> that is always-on. Therefore when the kernel is idle both the clocksource
>> and clockevent timers will be active and this will keep additional power
>> domains on. During kernel suspend, only the clocksource timer is active
>> and therefore, it is better to use a gptimer in a power domain that is
>> always-on for clocksource.
> 
> This should explain the gptimer number switch in the
> #if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_SOC_AM33XX)
> section below, right?
> I would really like to see that clearly stated in the commit message.
> For instance:
> ... it is better to use a gptimer in a power domain that is
> always-on for clocksource. Therefore we switch to use the first gptimer
> for clocksource and the second for clockevents.

Yes exactly. Good point I can make that bit explicit.

>>
>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> 
> Apart from above,
> Acked-by: Igor Grinberg <grinberg@compulab.co.il>

Thanks
Jon

[PATCH 5/5] ARM: OMAP3: Update clocksource timer selection

[Bedia, Vaibhav]

Hi Jon,

On Wed, Jan 30, 2013 at 22:34:31, Hunter, Jon wrote:
> When booting with device-tree for OMAP3 and AM335x devices and a gptimer
> is used as the clocksource (which is always the case for AM335x), a
> gptimer located in a power domain that is not always-on is selected.
> Ideally we should use a gptimer located in a power domain that is always
> on (such as the wake-up domain) so that time can be maintained during a
> kernel suspend without keeping on additional power domains unnecessarily.
> 
> In order to fix this so that we can select a gptimer located in a power
> domain that is always-on, the following changes were made ...
> 1. Currently, only when selecting a gptimer to use for a clockevent
>    timer, do we pass a timer property that can be used to select a
>    specific gptimer. Change this so that we can pass a property when
>    selecting a gptimer to use for a clocksource timer too.
> 2. Currently, when selecting either a gptimer to use for a clockevent
>    timer or a clocksource timer and no timer property is passed, then
>    the first available timer is selected regardless of the properties
>    it has. Change this so that if no properties are passed, then a timer
>    that does not have additional features (such as always-on, dsp-irq,
>    pwm, and secure) is selected.
> 
> Please note that using a gptimer for both clocksource and clockevents
> can have a system power impact during idle. The reason being is that
> OMAP and AMxxx devices typically only have one gptimer in a power domain
> that is always-on. Therefore when the kernel is idle both the clocksource
> and clockevent timers will be active and this will keep additional power
> domains on. During kernel suspend, only the clocksource timer is active
> and therefore, it is better to use a gptimer in a power domain that is
> always-on for clocksource.
> 

It's always a pleasure reading the changelog in your patches :)

[...]
>  
>  #if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_SOC_AM33XX)
> -OMAP_SYS_GP_TIMER_INIT(3, 1, "timer_sys_ck", "ti,timer-alwon",
> -		       2, "timer_sys_ck");
> +OMAP_SYS_GP_TIMER_INIT(3, 2, "timer_sys_ck", NULL,
> +		       1, "timer_sys_ck", "ti,timer-alwon");
>  #endif
>

Minor point... was the intention of changing of clkev_nr and clksrc_nr to make
it consistent with what happens on AM33xx which is DT-only?

Regards,
Vaibhav

[PATCH 5/5] ARM: OMAP3: Update clocksource timer selection

[Jon Hunter]

On 02/01/2013 02:41 AM, Bedia, Vaibhav wrote:
> Hi Jon,
> 
> On Wed, Jan 30, 2013 at 22:34:31, Hunter, Jon wrote:
>> When booting with device-tree for OMAP3 and AM335x devices and a gptimer
>> is used as the clocksource (which is always the case for AM335x), a
>> gptimer located in a power domain that is not always-on is selected.
>> Ideally we should use a gptimer located in a power domain that is always
>> on (such as the wake-up domain) so that time can be maintained during a
>> kernel suspend without keeping on additional power domains unnecessarily.
>>
>> In order to fix this so that we can select a gptimer located in a power
>> domain that is always-on, the following changes were made ...
>> 1. Currently, only when selecting a gptimer to use for a clockevent
>>    timer, do we pass a timer property that can be used to select a
>>    specific gptimer. Change this so that we can pass a property when
>>    selecting a gptimer to use for a clocksource timer too.
>> 2. Currently, when selecting either a gptimer to use for a clockevent
>>    timer or a clocksource timer and no timer property is passed, then
>>    the first available timer is selected regardless of the properties
>>    it has. Change this so that if no properties are passed, then a timer
>>    that does not have additional features (such as always-on, dsp-irq,
>>    pwm, and secure) is selected.
>>
>> Please note that using a gptimer for both clocksource and clockevents
>> can have a system power impact during idle. The reason being is that
>> OMAP and AMxxx devices typically only have one gptimer in a power domain
>> that is always-on. Therefore when the kernel is idle both the clocksource
>> and clockevent timers will be active and this will keep additional power
>> domains on. During kernel suspend, only the clocksource timer is active
>> and therefore, it is better to use a gptimer in a power domain that is
>> always-on for clocksource.
>>
> 
> It's always a pleasure reading the changelog in your patches :)

Thanks.

> [...]
>>  
>>  #if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_SOC_AM33XX)
>> -OMAP_SYS_GP_TIMER_INIT(3, 1, "timer_sys_ck", "ti,timer-alwon",
>> -		       2, "timer_sys_ck");
>> +OMAP_SYS_GP_TIMER_INIT(3, 2, "timer_sys_ck", NULL,
>> +		       1, "timer_sys_ck", "ti,timer-alwon");
>>  #endif
>>
> 
> Minor point... was the intention of changing of clkev_nr and clksrc_nr to make
> it consistent with what happens on AM33xx which is DT-only?

I don't see this as being DT specific. It is more of a policy change to
ensure a wake-up domain timer is used for clocksource when we are using
gptimers for both clocksource and clockevents. It was your patch for
AM335x that made me see the need for this, if that makes sense. May be I
should have referenced that in the changelog.

Cheers
Jon

[PATCH 5/5] ARM: OMAP3: Update clocksource timer selection

[Jon Hunter]

On 02/01/2013 02:59 AM, Jon Hunter wrote:
> 
> On 02/01/2013 02:41 AM, Bedia, Vaibhav wrote:
>> Hi Jon,
>>
>> On Wed, Jan 30, 2013 at 22:34:31, Hunter, Jon wrote:
>>> When booting with device-tree for OMAP3 and AM335x devices and a gptimer
>>> is used as the clocksource (which is always the case for AM335x), a
>>> gptimer located in a power domain that is not always-on is selected.
>>> Ideally we should use a gptimer located in a power domain that is always
>>> on (such as the wake-up domain) so that time can be maintained during a
>>> kernel suspend without keeping on additional power domains unnecessarily.
>>>
>>> In order to fix this so that we can select a gptimer located in a power
>>> domain that is always-on, the following changes were made ...
>>> 1. Currently, only when selecting a gptimer to use for a clockevent
>>>    timer, do we pass a timer property that can be used to select a
>>>    specific gptimer. Change this so that we can pass a property when
>>>    selecting a gptimer to use for a clocksource timer too.
>>> 2. Currently, when selecting either a gptimer to use for a clockevent
>>>    timer or a clocksource timer and no timer property is passed, then
>>>    the first available timer is selected regardless of the properties
>>>    it has. Change this so that if no properties are passed, then a timer
>>>    that does not have additional features (such as always-on, dsp-irq,
>>>    pwm, and secure) is selected.
>>>
>>> Please note that using a gptimer for both clocksource and clockevents
>>> can have a system power impact during idle. The reason being is that
>>> OMAP and AMxxx devices typically only have one gptimer in a power domain
>>> that is always-on. Therefore when the kernel is idle both the clocksource
>>> and clockevent timers will be active and this will keep additional power
>>> domains on. During kernel suspend, only the clocksource timer is active
>>> and therefore, it is better to use a gptimer in a power domain that is
>>> always-on for clocksource.
>>>
>>
>> It's always a pleasure reading the changelog in your patches :)
> 
> Thanks.
> 
>> [...]
>>>  
>>>  #if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_SOC_AM33XX)
>>> -OMAP_SYS_GP_TIMER_INIT(3, 1, "timer_sys_ck", "ti,timer-alwon",
>>> -		       2, "timer_sys_ck");
>>> +OMAP_SYS_GP_TIMER_INIT(3, 2, "timer_sys_ck", NULL,
>>> +		       1, "timer_sys_ck", "ti,timer-alwon");
>>>  #endif
>>>
>>
>> Minor point... was the intention of changing of clkev_nr and clksrc_nr to make
>> it consistent with what happens on AM33xx which is DT-only?
> 
> I don't see this as being DT specific. It is more of a policy change to
> ensure a wake-up domain timer is used for clocksource when we are using
> gptimers for both clocksource and clockevents. It was your patch for
> AM335x that made me see the need for this, if that makes sense. May be I
> should have referenced that in the changelog.

Sorry to be clear, I don't see the policy change as DT specific.

In answer to your question, yes clkev_nr and clksrc_nr are changed so
the policy it is consistent regardless of whether you use DT or not. In
other words, an OMAP3 board using a gptimer for clocksource  (such as
cm-t3517) and does not use DT, would work the same as AM335x with DT.

Cheers
Jon

[PATCH 5/5] ARM: OMAP3: Update clocksource timer selection

[Bedia, Vaibhav]

On Fri, Feb 01, 2013 at 14:55:31, Hunter, Jon wrote:
[...]
> > 
> > I don't see this as being DT specific. It is more of a policy change to
> > ensure a wake-up domain timer is used for clocksource when we are using
> > gptimers for both clocksource and clockevents. It was your patch for
> > AM335x that made me see the need for this, if that makes sense. May be I
> > should have referenced that in the changelog.
> 
> Sorry to be clear, I don't see the policy change as DT specific.
> 
> In answer to your question, yes clkev_nr and clksrc_nr are changed so
> the policy it is consistent regardless of whether you use DT or not. In
> other words, an OMAP3 board using a gptimer for clocksource  (such as
> cm-t3517) and does not use DT, would work the same as AM335x with DT.
> 

Ok. Thanks for clarifying.

Regards,
Vaibhav

[PATCH 0/5] ARM: OMAP2+: System timer updates

[Santosh Shilimkar]

On Wednesday 30 January 2013 10:34 PM, Jon Hunter wrote:
> This series consists mainly of clean-ups for clockevents and
> clocksource timers on OMAP2+ devices. The most significant change
> in functionality comes from the 5th patch which is changing the
> selection of the clocksource timer for OMAP3 and AM335x devices
> when gptimers are used for clocksource. This change came about from
> Vaibhav Bedia's series for AM335x [1]. See patch for more details on
> the exact nature of the change.
>
> Boot tested with and without  device-tree on OMAP2420 H4,
> OMAP3430 SDP, OMAP3430 Beagle Board, OMAP4430 SDP and
> AM335x EVM (AM335x only supports device-tree boot).
>
> This series is based upon ARM-SoC next branch.
>
> [1] https://patchwork.kernel.org/patch/1921421/
>
> Jon Hunter (5):
>    ARM: OMAP2+: Display correct system timer name
>    ARM: OMAP2+: Remove hard-coded test on timer ID
>    ARM: OMAP2+: Simplify system timer clock definitions
>    ARM: OMAP2+: Simplify system timers definitions
>    ARM: OMAP3: Update clocksource timer selection
>
Nice work Jon. All the patches in the series looks good
to my eyes.

Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

[PATCH 0/5] ARM: OMAP2+: System timer updates

[Bedia, Vaibhav]

Hi Jon,

On Wed, Jan 30, 2013 at 22:34:26, Hunter, Jon wrote:
> This series consists mainly of clean-ups for clockevents and
> clocksource timers on OMAP2+ devices. The most significant change
> in functionality comes from the 5th patch which is changing the
> selection of the clocksource timer for OMAP3 and AM335x devices
> when gptimers are used for clocksource. This change came about from
> Vaibhav Bedia's series for AM335x [1]. See patch for more details on
> the exact nature of the change.
> 
> Boot tested with and without  device-tree on OMAP2420 H4,
> OMAP3430 SDP, OMAP3430 Beagle Board, OMAP4430 SDP and
> AM335x EVM (AM335x only supports device-tree boot).
> 

Thanks for working on this. I have couple of minor comments but with this
series in place I can drop the patch for interchanging the timers for AM33xx
and the suspend-resume handlers for the clockevent also don't need the
sprintf() that I had :)

Reviewed-and-Tested-by: Vaibhav Bedia <vaibhav.bedia@ti.com>