Re: [PATCH V2 01/14] ARM: OMAP: Add DMTIMER definitions for posted mode
On 11/07/2012 04:04 PM, Santosh Shilimkar wrote:
On Wednesday 07 November 2012 01:01 PM, Jon Hunter wrote:
For OMAP2+ devices, when using DMTIMERs for system timers
(clock-events and
clock-source) the posted mode configuration of the timers is used. To
allow
the compiler to optimise the functions for configuring and reading the
system
timers, the posted flag variable is hard-coded with the value 1. To
make it
clear that posted mode is being used add some definitions so that it
is more
readable.
Add separate definitions for the clock-events and clock-source timers
so that
we can change the posted mode of clock-events and clock-source
independently.
Signed-off-by: Jon Hunter jon-hun...@ti.com
---
arch/arm/mach-omap2/timer.c | 26
+++---
arch/arm/plat-omap/include/plat/dmtimer.h |4
2 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 0758bae..28c6078 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -82,6 +82,13 @@
#define INCREMENTER_DENUMERATOR_RELOAD_OFFSET0x14
#define NUMERATOR_DENUMERATOR_MASK0xf000
+/*
+ * For clock-events timer, always use posted mode to
+ * minimise CPU overhead for configuring the timer.
+ */
+#define OMAP_CLKEVT_POSTEDMODEOMAP_TIMER_POSTED
+#define OMAP_CLKSRC_POSTEDMODEOMAP_TIMER_POSTED
+
I don't see need of above defines. Just use OMAP_TIMER_POSTED directly
with API. Rest of the patch looks fine.
Apart from above one comment,
Acked-by: Santosh Shilimkar santosh.shilim...@ti.com
Ok, updated and dropped the additional definitions. Let me know if you
are ok with this.
Cheers
Jon
From f1c783b5af933374431bcb8acb01d0b5c79d5661 Mon Sep 17 00:00:00 2001
From: Jon Hunter jon-hun...@ti.com
Date: Thu, 27 Sep 2012 11:49:45 -0500
Subject: [PATCH 1/2] ARM: OMAP: Add DMTIMER definitions for posted mode
For OMAP2+ devices, when using DMTIMERs for system timers (clock-events and
clock-source) the posted mode configuration of the timers is used. To allow
the compiler to optimise the functions for configuring and reading the system
timers, the posted flag variable is hard-coded with the value 1. To make it
clear that posted mode is being used add some definitions so that it is more
readable.
Signed-off-by: Jon Hunter jon-hun...@ti.com
---
arch/arm/mach-omap2/timer.c | 17 ++---
arch/arm/plat-omap/include/plat/dmtimer.h |4
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 0758bae..ad427ba 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -107,7 +107,7 @@ static int omap2_gp_timer_set_next_event(unsigned long
cycles,
struct clock_event_device *evt)
{
__omap_dm_timer_load_start(clkev, OMAP_TIMER_CTRL_ST,
- 0x - cycles, 1);
+ 0x - cycles, OMAP_TIMER_POSTED);
return 0;
}
@@ -117,7 +117,7 @@ static void omap2_gp_timer_set_mode(enum clock_event_mode
mode,
{
u32 period;
- __omap_dm_timer_stop(clkev, 1, clkev.rate);
+ __omap_dm_timer_stop(clkev, OMAP_TIMER_POSTED, clkev.rate);
switch (mode) {
case CLOCK_EVT_MODE_PERIODIC:
@@ -125,10 +125,10 @@ static void omap2_gp_timer_set_mode(enum clock_event_mode
mode,
period -= 1;
/* Looks like we need to first set the load value separately */
__omap_dm_timer_write(clkev, OMAP_TIMER_LOAD_REG,
- 0x - period, 1);
+ 0x - period, OMAP_TIMER_POSTED);
__omap_dm_timer_load_start(clkev,
OMAP_TIMER_CTRL_AR | OMAP_TIMER_CTRL_ST,
- 0x - period, 1);
+ 0x - period, OMAP_TIMER_POSTED);
break;
case CLOCK_EVT_MODE_ONESHOT:
break;
@@ -358,7 +358,8 @@ static bool use_gptimer_clksrc;
*/
static cycle_t clocksource_read_cycles(struct clocksource *cs)
{
- return (cycle_t)__omap_dm_timer_read_counter(clksrc, 1);
+ return (cycle_t)__omap_dm_timer_read_counter(clksrc,
+OMAP_TIMER_POSTED);
}
static struct clocksource clocksource_gpt = {
@@ -372,7 +373,8 @@ static struct clocksource clocksource_gpt = {
static u32 notrace dmtimer_read_sched_clock(void)
{
if (clksrc.reserved)
- return __omap_dm_timer_read_counter(clksrc, 1);
+ return __omap_dm_timer_read_counter(clksrc,
+ OMAP_TIMER_POSTED);
return 0;
}
@@ -454,7 +456,8 @@
[PATCH V2 01/14] ARM: OMAP: Add DMTIMER definitions for posted mode
For OMAP2+ devices, when using DMTIMERs for system timers (clock-events and
clock-source) the posted mode configuration of the timers is used. To allow
the compiler to optimise the functions for configuring and reading the system
timers, the posted flag variable is hard-coded with the value 1. To make it
clear that posted mode is being used add some definitions so that it is more
readable.
Add separate definitions for the clock-events and clock-source timers so that
we can change the posted mode of clock-events and clock-source independently.
Signed-off-by: Jon Hunter jon-hun...@ti.com
---
arch/arm/mach-omap2/timer.c | 26 +++---
arch/arm/plat-omap/include/plat/dmtimer.h |4
2 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 0758bae..28c6078 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -82,6 +82,13 @@
#define INCREMENTER_DENUMERATOR_RELOAD_OFFSET 0x14
#define NUMERATOR_DENUMERATOR_MASK 0xf000
+/*
+ * For clock-events timer, always use posted mode to
+ * minimise CPU overhead for configuring the timer.
+ */
+#define OMAP_CLKEVT_POSTEDMODE OMAP_TIMER_POSTED
+#define OMAP_CLKSRC_POSTEDMODE OMAP_TIMER_POSTED
+
/* Clockevent code */
static struct omap_dm_timer clkev;
@@ -107,7 +114,7 @@ static int omap2_gp_timer_set_next_event(unsigned long
cycles,
struct clock_event_device *evt)
{
__omap_dm_timer_load_start(clkev, OMAP_TIMER_CTRL_ST,
- 0x - cycles, 1);
+ 0x - cycles, OMAP_CLKEVT_POSTEDMODE);
return 0;
}
@@ -117,7 +124,7 @@ static void omap2_gp_timer_set_mode(enum clock_event_mode
mode,
{
u32 period;
- __omap_dm_timer_stop(clkev, 1, clkev.rate);
+ __omap_dm_timer_stop(clkev, OMAP_CLKEVT_POSTEDMODE, clkev.rate);
switch (mode) {
case CLOCK_EVT_MODE_PERIODIC:
@@ -125,10 +132,12 @@ static void omap2_gp_timer_set_mode(enum clock_event_mode
mode,
period -= 1;
/* Looks like we need to first set the load value separately */
__omap_dm_timer_write(clkev, OMAP_TIMER_LOAD_REG,
- 0x - period, 1);
+ 0x - period,
+ OMAP_CLKEVT_POSTEDMODE);
__omap_dm_timer_load_start(clkev,
OMAP_TIMER_CTRL_AR | OMAP_TIMER_CTRL_ST,
- 0x - period, 1);
+ 0x - period,
+ OMAP_CLKEVT_POSTEDMODE);
break;
case CLOCK_EVT_MODE_ONESHOT:
break;
@@ -358,7 +367,8 @@ static bool use_gptimer_clksrc;
*/
static cycle_t clocksource_read_cycles(struct clocksource *cs)
{
- return (cycle_t)__omap_dm_timer_read_counter(clksrc, 1);
+ return (cycle_t)__omap_dm_timer_read_counter(clksrc,
+OMAP_CLKSRC_POSTEDMODE);
}
static struct clocksource clocksource_gpt = {
@@ -372,7 +382,8 @@ static struct clocksource clocksource_gpt = {
static u32 notrace dmtimer_read_sched_clock(void)
{
if (clksrc.reserved)
- return __omap_dm_timer_read_counter(clksrc, 1);
+ return __omap_dm_timer_read_counter(clksrc,
+ OMAP_CLKSRC_POSTEDMODE);
return 0;
}
@@ -454,7 +465,8 @@ static void __init omap2_gptimer_clocksource_init(int
gptimer_id,
BUG_ON(res);
__omap_dm_timer_load_start(clksrc,
- OMAP_TIMER_CTRL_ST | OMAP_TIMER_CTRL_AR, 0, 1);
+ OMAP_TIMER_CTRL_ST | OMAP_TIMER_CTRL_AR, 0,
+ OMAP_CLKSRC_POSTEDMODE);
setup_sched_clock(dmtimer_read_sched_clock, 32, clksrc.rate);
if (clocksource_register_hz(clocksource_gpt, clksrc.rate))
diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h
b/arch/arm/plat-omap/include/plat/dmtimer.h
index 348f855..835c3bdf 100644
--- a/arch/arm/plat-omap/include/plat/dmtimer.h
+++ b/arch/arm/plat-omap/include/plat/dmtimer.h
@@ -55,6 +55,10 @@
#define OMAP_TIMER_TRIGGER_OVERFLOW0x01
#define OMAP_TIMER_TRIGGER_OVERFLOW_AND_COMPARE0x02
+/* posted mode types */
+#define OMAP_TIMER_NONPOSTED 0x00
+#define OMAP_TIMER_POSTED 0x01
+
/* timer capabilities used in hwmod database */
#define OMAP_TIMER_SECURE 0x8000
#define OMAP_TIMER_ALWON 0x4000
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to
Re: [PATCH V2 01/14] ARM: OMAP: Add DMTIMER definitions for posted mode
On Wednesday 07 November 2012 01:01 PM, Jon Hunter wrote: For OMAP2+ devices, when using DMTIMERs for system timers (clock-events and clock-source) the posted mode configuration of the timers is used. To allow the compiler to optimise the functions for configuring and reading the system timers, the posted flag variable is hard-coded with the value 1. To make it clear that posted mode is being used add some definitions so that it is more readable. Add separate definitions for the clock-events and clock-source timers so that we can change the posted mode of clock-events and clock-source independently. Signed-off-by: Jon Hunter jon-hun...@ti.com --- arch/arm/mach-omap2/timer.c | 26 +++--- arch/arm/plat-omap/include/plat/dmtimer.h |4 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c index 0758bae..28c6078 100644 --- a/arch/arm/mach-omap2/timer.c +++ b/arch/arm/mach-omap2/timer.c @@ -82,6 +82,13 @@ #define INCREMENTER_DENUMERATOR_RELOAD_OFFSET 0x14 #define NUMERATOR_DENUMERATOR_MASK0xf000 +/* + * For clock-events timer, always use posted mode to + * minimise CPU overhead for configuring the timer. + */ +#define OMAP_CLKEVT_POSTEDMODE OMAP_TIMER_POSTED +#define OMAP_CLKSRC_POSTEDMODE OMAP_TIMER_POSTED + I don't see need of above defines. Just use OMAP_TIMER_POSTED directly with API. Rest of the patch looks fine. Apart from above one comment, Acked-by: Santosh Shilimkar santosh.shilim...@ti.com -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/14] ARM: OMAP: Add DMTIMER definitions for posted mode
On 11/07/2012 04:04 PM, Santosh Shilimkar wrote: On Wednesday 07 November 2012 01:01 PM, Jon Hunter wrote: For OMAP2+ devices, when using DMTIMERs for system timers (clock-events and clock-source) the posted mode configuration of the timers is used. To allow the compiler to optimise the functions for configuring and reading the system timers, the posted flag variable is hard-coded with the value 1. To make it clear that posted mode is being used add some definitions so that it is more readable. Add separate definitions for the clock-events and clock-source timers so that we can change the posted mode of clock-events and clock-source independently. Signed-off-by: Jon Hunter jon-hun...@ti.com --- arch/arm/mach-omap2/timer.c | 26 +++--- arch/arm/plat-omap/include/plat/dmtimer.h |4 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c index 0758bae..28c6078 100644 --- a/arch/arm/mach-omap2/timer.c +++ b/arch/arm/mach-omap2/timer.c @@ -82,6 +82,13 @@ #define INCREMENTER_DENUMERATOR_RELOAD_OFFSET0x14 #define NUMERATOR_DENUMERATOR_MASK0xf000 +/* + * For clock-events timer, always use posted mode to + * minimise CPU overhead for configuring the timer. + */ +#define OMAP_CLKEVT_POSTEDMODEOMAP_TIMER_POSTED +#define OMAP_CLKSRC_POSTEDMODEOMAP_TIMER_POSTED + I don't see need of above defines. Just use OMAP_TIMER_POSTED directly with API. Rest of the patch looks fine. Yes that's possible, however, in patch #2, I am disabling posted mode for clock-source (see changelog of patch #2 for details). Having these #defines makes it easier to change the posted configuration. That was the real motivation here. Cheers Jon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/14] ARM: OMAP: Add DMTIMER definitions for posted mode
On Wednesday 07 November 2012 04:11 PM, Jon Hunter wrote: On 11/07/2012 04:04 PM, Santosh Shilimkar wrote: On Wednesday 07 November 2012 01:01 PM, Jon Hunter wrote: For OMAP2+ devices, when using DMTIMERs for system timers (clock-events and clock-source) the posted mode configuration of the timers is used. To allow the compiler to optimise the functions for configuring and reading the system timers, the posted flag variable is hard-coded with the value 1. To make it clear that posted mode is being used add some definitions so that it is more readable. Add separate definitions for the clock-events and clock-source timers so that we can change the posted mode of clock-events and clock-source independently. Signed-off-by: Jon Hunter jon-hun...@ti.com --- arch/arm/mach-omap2/timer.c | 26 +++--- arch/arm/plat-omap/include/plat/dmtimer.h |4 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c index 0758bae..28c6078 100644 --- a/arch/arm/mach-omap2/timer.c +++ b/arch/arm/mach-omap2/timer.c @@ -82,6 +82,13 @@ #define INCREMENTER_DENUMERATOR_RELOAD_OFFSET0x14 #define NUMERATOR_DENUMERATOR_MASK0xf000 +/* + * For clock-events timer, always use posted mode to + * minimise CPU overhead for configuring the timer. + */ +#define OMAP_CLKEVT_POSTEDMODEOMAP_TIMER_POSTED +#define OMAP_CLKSRC_POSTEDMODEOMAP_TIMER_POSTED + I don't see need of above defines. Just use OMAP_TIMER_POSTED directly with API. Rest of the patch looks fine. Yes that's possible, however, in patch #2, I am disabling posted mode for clock-source (see changelog of patch #2 for details). Having these #defines makes it easier to change the posted configuration. That was the real motivation here. Sure but that is more confusing because you are flipping the meaning of the macro. Better to specify direct argument to avoid the confusion. Regards Santosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 01/14] ARM: OMAP: Add DMTIMER definitions for posted mode
On 11/07/2012 04:18 PM, Santosh Shilimkar wrote: On Wednesday 07 November 2012 04:11 PM, Jon Hunter wrote: On 11/07/2012 04:04 PM, Santosh Shilimkar wrote: On Wednesday 07 November 2012 01:01 PM, Jon Hunter wrote: For OMAP2+ devices, when using DMTIMERs for system timers (clock-events and clock-source) the posted mode configuration of the timers is used. To allow the compiler to optimise the functions for configuring and reading the system timers, the posted flag variable is hard-coded with the value 1. To make it clear that posted mode is being used add some definitions so that it is more readable. Add separate definitions for the clock-events and clock-source timers so that we can change the posted mode of clock-events and clock-source independently. Signed-off-by: Jon Hunter jon-hun...@ti.com --- arch/arm/mach-omap2/timer.c | 26 +++--- arch/arm/plat-omap/include/plat/dmtimer.h |4 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c index 0758bae..28c6078 100644 --- a/arch/arm/mach-omap2/timer.c +++ b/arch/arm/mach-omap2/timer.c @@ -82,6 +82,13 @@ #define INCREMENTER_DENUMERATOR_RELOAD_OFFSET0x14 #define NUMERATOR_DENUMERATOR_MASK0xf000 +/* + * For clock-events timer, always use posted mode to + * minimise CPU overhead for configuring the timer. + */ +#define OMAP_CLKEVT_POSTEDMODEOMAP_TIMER_POSTED +#define OMAP_CLKSRC_POSTEDMODEOMAP_TIMER_POSTED + I don't see need of above defines. Just use OMAP_TIMER_POSTED directly with API. Rest of the patch looks fine. Yes that's possible, however, in patch #2, I am disabling posted mode for clock-source (see changelog of patch #2 for details). Having these #defines makes it easier to change the posted configuration. That was the real motivation here. Sure but that is more confusing because you are flipping the meaning of the macro. Better to specify direct argument to avoid the confusion. Hmmm ... I guess I don't see it that way. The intent was that the definitions OMAP_CLKxxx_POSTEDMODE described the posted configuration (ie. posted or non-posted) and a user could change/flip it if so desired. I can use the OMAP_TIMER_POSTED/NONPOSTED directly, but my concern with that was if someone wanted to changed the posted mode then they have to change it in multiple places and there is a chance they could miss one. This way, as long as I have it right to begin with, then no one should be able to screw it up :-) Cheers Jon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
