Tony,
On 3/29/2011 3:51 AM, Tony Lindgren wrote:
Hi all,
This series continues the work to only initialize minimal omap code
in init_early and to cut down dependencies to code that should be
initialized later. It also cleans up the omap2+ timer init code to prepare
things for the later patches.
After this series further patches can make the dmtimer code a regular
device driver. Later on we can also move the clockource init happen
earlier to make sched_clock work properly with CONFIG_PRINTK_TIME.
Yesterday I started looking at this series but later because of my
mailer horror show, I gave up.
After going through entire series again, it looks very
good clean-up and right step towards moving rest of
the timer to drivers/ directory.
I also realized that the discussion we had before this series was
because of miss-understating about the 'wakeu_up' timer
terminology. After this series I see that you were mentioning
about the pm debug timer where as I was taling about
clock-event wakeups from CPUidle point of view.
I still have few concerns about this series.
1)We have removed the flexibility if choosing the timer from
from board files and hard-coded them in platform timer
header.
Below board related hard-coding in platform files looks more
hacky and the interface done by Pual still has a merits of
its own.
--- a/arch/arm/plat-omap/include/plat/common.h
+++ b/arch/arm/plat-omap/include/plat/common.h
@@ -34,7 +34,12 @@
struct sys_timer;
extern void omap_map_common_io(void);
-extern struct sys_timer omap_timer;
+extern struct sys_timer omap1_timer;
+extern struct sys_timer omap242x_timer;
+extern struct sys_timer omap243x_timer;
+extern struct sys_timer omap3_timer;
+extern struct sys_timer omap3_beagle_timer;
+extern struct sys_timer omap4_timer;
extern bool omap_32k_timer_init(void);
extern int __init omap_init_clocksource_32k(void);
extern unsigned long long notrace omap_32k_sched_clock(void);
--- a/arch/arm/mach-omap2/timer-gp.c
+++ b/arch/arm/mach-omap2/timer-gp.c
@@ -45,10 +45,33 @@
#include "timer-gp.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"
+
+#ifdef CONFIG_OMAP_32K_TIMER
+#define OMAP2_CLKEV_SOURCE OMAP2_32K_SOURCE
+#define OMAP3_CLKEV_SOURCE OMAP3_32K_SOURCE
+#define OMAP4_CLKEV_SOURCE OMAP4_32K_SOURCE
+#define OMAP3_BEAGLE_TIMER 12
+#else
+#define OMAP2_CLKEV_SOURCE OMAP2_MPU_SOURCE
+#define OMAP3_CLKEV_SOURCE OMAP3_MPU_SOURCE
+#define OMAP4_CLKEV_SOURCE OMAP4_MPU_SOURCE
+#define OMAP3_BEAGLE_TIMER 1
+#endif
[.....]
+/*
+ * Beagle based designs typically have an issue with gptimer1. Also note
+ * that GPTIMER12 can only use the secure 32KiHz clock source.
+ */
static void __init omap3_beagle_timer_init(void)
{
omap_dm_timer_init();
- omap2_gp_clockevent_init();
+ omap2_gp_clockevent_init(OMAP3_BEAGLE_TIMER, OMAP3_CLKEV_SOURCE);
omap2_gp_clocksource_init();
}
2) In patch [6/10], you have removed wakeup timer debug
capability with comment "Later on we can reserve gptimer1
for PM code only and have similar functionality".
I don't know how this will work on OMAP. GPTIMER1 is the
only timer which is in always on power domain and wakeup
OMAP from deeper power states like CORE OSWR, CORE OFF,
DEVICE OFF.
We do implement these C-state in CPUidle as well and hence
the clock-event is expected to be of wakeup-capable.
Hence GPTIMER1 is already reserved for clock-event.
The wakeup timer used was only in suspend scenario's
and that point of time the timer system is already
suspended. So during that even if TIMER1 is re-used
for wakeup (which is the case), I don't see any issue.
At least I don't see other solution than using GPT1
for wakeup.
3) You have created few functions so that they can
be used between system timer and dmtimer driver code.
When we move the dmtimer driver to say some drivers/
directory. Is it allowed to share such functions from
device drivers
Apart from above comments, I really liked this series.
If you like you can add my,
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@xxxxxx>
Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
