[PATCHv3 8/17] dmtimer: register mappings moved to hwmod database

[PATCHv3 8/17] dmtimer: register mappings moved to hwmod database

[Tarun Kanti DebBarma]

This patch adds dmtimer register mappings to hwmod database.
This is to avoid maintaining different several mappings within
the omap-plat. The mapping is made available to platform through
dev_attr during device build. The pointer to register map is
preserved in the platform data.

Please note that the cleanup of register map from plat-omap will
be removed in later patch after conversion to platform driver.

Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Signed-off-by: Partha Basak <p-basak2@ti.com>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Cousson, Benoit <b-cousson@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/omap_hwmod_2420_data.c |   25 ++++++++++
 arch/arm/mach-omap2/omap_hwmod_2430_data.c |   25 ++++++++++
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   25 ++++++++++
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |   67 ++++++++++++++++++++++++---
 arch/arm/plat-omap/dmtimer.c               |   13 -----
 arch/arm/plat-omap/include/plat/dmtimer.h  |   38 ++++++++++++++++
 6 files changed, 172 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_2420_data.c b/arch/arm/mach-omap2/omap_hwmod_2420_data.c
index 6003c2e..b3dd8ac 100644
--- a/arch/arm/mach-omap2/omap_hwmod_2420_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_2420_data.c
@@ -131,8 +131,33 @@ static char *timer_clk_src_names[] = {
 	NULL,
 };
 
+/* OMAP2/3 timers register map */
+static u32 omap_timer_reg_map_v1[] = {
+	[OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR << WPSHIFT)),
+	[OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR << WPSHIFT)),
+	[OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR << WPSHIFT)),
+	[OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR << WPSHIFT)),
+	[OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR << WPSHIFT)),
+	[OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR << WPSHIFT)),
+	[OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR << WPSHIFT)),
+	[OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR << WPSHIFT)),
+	[OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 | (WP_TOCR << WPSHIFT)),
+	[OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 | (WP_TOWR << WPSHIFT)),
+};
+
 static struct omap_timer_dev_attr timer_dev_attr = {
 	.clk_names	= timer_clk_src_names,
+	.reg_map	= omap_timer_reg_map_v1,
 };
 
 static struct omap_hwmod_class_sysconfig omap2420_timer_sysc = {
diff --git a/arch/arm/mach-omap2/omap_hwmod_2430_data.c b/arch/arm/mach-omap2/omap_hwmod_2430_data.c
index 4b43fb9..787d3ce 100644
--- a/arch/arm/mach-omap2/omap_hwmod_2430_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_2430_data.c
@@ -137,8 +137,33 @@ static char *timer_clk_src_names[] = {
 	NULL
 };
 
+/* OMAP2/3 timers register map */
+static u32 omap_timer_reg_map_v1[] = {
+	[OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR << WPSHIFT)),
+	[OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR << WPSHIFT)),
+	[OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR << WPSHIFT)),
+	[OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR << WPSHIFT)),
+	[OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR << WPSHIFT)),
+	[OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR << WPSHIFT)),
+	[OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR << WPSHIFT)),
+	[OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR << WPSHIFT)),
+	[OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 | (WP_TOCR << WPSHIFT)),
+	[OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 | (WP_TOWR << WPSHIFT)),
+};
+
 static struct omap_timer_dev_attr timer_dev_attr = {
 	.clk_names	= timer_clk_src_names,
+	.reg_map	= omap_timer_reg_map_v1,
 };
 
 static struct omap_hwmod_class_sysconfig omap2430_timer_sysc = {
diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 70446d7..e477ce8 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -109,8 +109,33 @@ static char *timer_clk_src_names[] = {
 	NULL,
 };
 
+/* OMAP2/3 timers register map */
+static u32 omap_timer_reg_map_v1[] = {
+	[OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR << WPSHIFT)),
+	[OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR << WPSHIFT)),
+	[OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR << WPSHIFT)),
+	[OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR << WPSHIFT)),
+	[OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR << WPSHIFT)),
+	[OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR << WPSHIFT)),
+	[OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR << WPSHIFT)),
+	[OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR << WPSHIFT)),
+	[OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 | (WP_TOCR << WPSHIFT)),
+	[OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 | (WP_TOWR << WPSHIFT)),
+};
+
 static struct omap_timer_dev_attr timer_dev_attr = {
 	.clk_names	= timer_clk_src_names,
+	.reg_map	= omap_timer_reg_map_v1,
 };
 
 /* timer class */
diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 7c68228..1f60f8a 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -471,12 +471,63 @@ static char *timer_clk_src_names_abe[] = {
 	NULL,
 };
 
-static struct omap_timer_dev_attr timer_dev_attr = {
+/* OMAP2/3 timers register map */
+static u32 omap_timer_reg_map_v1[] = {
+	[OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR << WPSHIFT)),
+	[OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR << WPSHIFT)),
+	[OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR << WPSHIFT)),
+	[OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR << WPSHIFT)),
+	[OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR << WPSHIFT)),
+	[OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR << WPSHIFT)),
+	[OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR << WPSHIFT)),
+	[OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR << WPSHIFT)),
+	[OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 | (WP_TOCR << WPSHIFT)),
+	[OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 | (WP_TOWR << WPSHIFT)),
+};
+
+/* OMAP4 timers register map */
+static u32 omap_timer_reg_map_v2[] = {
+	[OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_STAT_REG]           = (0x28 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_INT_EN_REG]         = (0x2c | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_WAKEUP_EN_REG]      = (0x34 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_CTRL_REG]           = (0x38 | (WP_TCLR << WPSHIFT)),
+	[OMAP_TIMER_COUNTER_REG]        = (0x3c | (WP_TCRR << WPSHIFT)),
+	[OMAP_TIMER_LOAD_REG]           = (0x40 | (WP_TLDR << WPSHIFT)),
+	[OMAP_TIMER_TRIGGER_REG]        = (0x44 | (WP_TTGR << WPSHIFT)),
+	[OMAP_TIMER_WRITE_PEND_REG]     = (0x48 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_MATCH_REG]          = (0x4c | (WP_TMAR << WPSHIFT)),
+	[OMAP_TIMER_CAPTURE_REG]        = (0x50 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_IF_CTRL_REG]        = (0x54 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_CAPTURE2_REG]       = (0x58 | (WP_NONE << WPSHIFT)),
+	[OMAP_TIMER_INT_CLR_REG]        = (0x30 | (WP_NONE << WPSHIFT)),
+};
+
+static struct omap_timer_dev_attr timer_dev_attr_v1 = {
+	.clk_names      = timer_clk_src_names,
+	.reg_map        = omap_timer_reg_map_v1,
+};
+
+static struct omap_timer_dev_attr timer_dev_attr_v2 = {
 	.clk_names	= timer_clk_src_names,
+	.reg_map	= omap_timer_reg_map_v2,
 };
 
 static struct omap_timer_dev_attr timer_dev_attr_abe = {
 	.clk_names	= timer_clk_src_names_abe,
+	.reg_map	= omap_timer_reg_map_v2,
 };
 
 static struct omap_hwmod_class_sysconfig omap44xx_timer_1ms_sysc = {
@@ -558,7 +609,7 @@ static struct omap_hwmod omap44xx_timer1_hwmod = {
 			.clkctrl_reg = OMAP4430_CM_WKUP_TIMER1_CLKCTRL,
 		},
 	},
-	.dev_attr	= &timer_dev_attr,
+	.dev_attr	= &timer_dev_attr_v1,
 	.slaves		= omap44xx_timer1_slaves,
 	.slaves_cnt	= ARRAY_SIZE(omap44xx_timer1_slaves),
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
@@ -604,7 +655,7 @@ static struct omap_hwmod omap44xx_timer2_hwmod = {
 			.clkctrl_reg = OMAP4430_CM_L4PER_DMTIMER2_CLKCTRL,
 		},
 	},
-	.dev_attr	= &timer_dev_attr,
+	.dev_attr	= &timer_dev_attr_v1,
 	.slaves		= omap44xx_timer2_slaves,
 	.slaves_cnt	= ARRAY_SIZE(omap44xx_timer2_slaves),
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
@@ -650,7 +701,7 @@ static struct omap_hwmod omap44xx_timer3_hwmod = {
 			.clkctrl_reg = OMAP4430_CM_L4PER_DMTIMER3_CLKCTRL,
 		},
 	},
-	.dev_attr	= &timer_dev_attr,
+	.dev_attr	= &timer_dev_attr_v2,
 	.slaves		= omap44xx_timer3_slaves,
 	.slaves_cnt	= ARRAY_SIZE(omap44xx_timer3_slaves),
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
@@ -696,7 +747,7 @@ static struct omap_hwmod omap44xx_timer4_hwmod = {
 			.clkctrl_reg = OMAP4430_CM_L4PER_DMTIMER4_CLKCTRL,
 		},
 	},
-	.dev_attr	= &timer_dev_attr,
+	.dev_attr	= &timer_dev_attr_v2,
 	.slaves		= omap44xx_timer4_slaves,
 	.slaves_cnt	= ARRAY_SIZE(omap44xx_timer4_slaves),
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
@@ -1002,7 +1053,7 @@ static struct omap_hwmod omap44xx_timer9_hwmod = {
 			.clkctrl_reg = OMAP4430_CM_L4PER_DMTIMER9_CLKCTRL,
 		},
 	},
-	.dev_attr	= &timer_dev_attr,
+	.dev_attr	= &timer_dev_attr_v2,
 	.slaves		= omap44xx_timer9_slaves,
 	.slaves_cnt	= ARRAY_SIZE(omap44xx_timer9_slaves),
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
@@ -1048,7 +1099,7 @@ static struct omap_hwmod omap44xx_timer10_hwmod = {
 			.clkctrl_reg = OMAP4430_CM_L4PER_DMTIMER10_CLKCTRL,
 		},
 	},
-	.dev_attr	= &timer_dev_attr,
+	.dev_attr	= &timer_dev_attr_v1,
 	.slaves		= omap44xx_timer10_slaves,
 	.slaves_cnt	= ARRAY_SIZE(omap44xx_timer10_slaves),
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
@@ -1094,7 +1145,7 @@ static struct omap_hwmod omap44xx_timer11_hwmod = {
 			.clkctrl_reg = OMAP4430_CM_L4PER_DMTIMER11_CLKCTRL,
 		},
 	},
-	.dev_attr	= &timer_dev_attr,
+	.dev_attr	= &timer_dev_attr_v2,
 	.slaves		= omap44xx_timer11_slaves,
 	.slaves_cnt	= ARRAY_SIZE(omap44xx_timer11_slaves),
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 2d4805d..9a22623 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -72,17 +72,6 @@
 #define _OMAP_TIMER_LOAD_OFFSET		0x2c
 #define _OMAP_TIMER_TRIGGER_OFFSET	0x30
 #define _OMAP_TIMER_WRITE_PEND_OFFSET	0x34
-#define		WP_NONE			0	/* no write pending bit */
-#define		WP_TCLR			(1 << 0)
-#define		WP_TCRR			(1 << 1)
-#define		WP_TLDR			(1 << 2)
-#define		WP_TTGR			(1 << 3)
-#define		WP_TMAR			(1 << 4)
-#define		WP_TPIR			(1 << 5)
-#define		WP_TNIR			(1 << 6)
-#define		WP_TCVR			(1 << 7)
-#define		WP_TOCR			(1 << 8)
-#define		WP_TOWR			(1 << 9)
 #define _OMAP_TIMER_MATCH_OFFSET	0x38
 #define _OMAP_TIMER_CAPTURE_OFFSET	0x3c
 #define _OMAP_TIMER_IF_CTRL_OFFSET	0x40
@@ -93,8 +82,6 @@
 #define _OMAP_TIMER_TICK_INT_MASK_SET_OFFSET	0x54	/* TOCR, 34xx only */
 #define _OMAP_TIMER_TICK_INT_MASK_COUNT_OFFSET	0x58	/* TOWR, 34xx only */
 
-/* register offsets with the write pending bit encoded */
-#define	WPSHIFT					16
 
 #define OMAP_TIMER_ID_REG			(_OMAP_TIMER_ID_OFFSET \
 							| (WP_NONE << WPSHIFT))
diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h b/arch/arm/plat-omap/include/plat/dmtimer.h
index 3ec17c5..d983ae3 100644
--- a/arch/arm/plat-omap/include/plat/dmtimer.h
+++ b/arch/arm/plat-omap/include/plat/dmtimer.h
@@ -58,6 +58,44 @@
  */
 #define NR_CLK_SOURCES			3
 
+/* register offsets with the write pending bit encoded */
+#define         WPSHIFT                 16
+
+#define         WP_NONE                 0       /* no write pending bit */
+#define         WP_TCLR                 BIT(0)
+#define         WP_TCRR                 BIT(1)
+#define         WP_TLDR                 BIT(2)
+#define         WP_TTGR                 BIT(3)
+#define         WP_TMAR                 BIT(4)
+#define         WP_TPIR                 BIT(5)
+#define         WP_TNIR                 BIT(6)
+#define         WP_TCVR                 BIT(7)
+#define         WP_TOCR                 BIT(8)
+#define         WP_TOWR                 BIT(9)
+
+enum {
+	OMAP_TIMER_ID_REG = 0,
+	OMAP_TIMER_OCP_CFG_REG,
+	OMAP_TIMER_SYS_STAT_REG,
+	OMAP_TIMER_STAT_REG,
+	OMAP_TIMER_INT_EN_REG,
+	OMAP_TIMER_WAKEUP_EN_REG,
+	OMAP_TIMER_CTRL_REG,
+	OMAP_TIMER_COUNTER_REG,
+	OMAP_TIMER_LOAD_REG,
+	OMAP_TIMER_TRIGGER_REG,
+	OMAP_TIMER_WRITE_PEND_REG,
+	OMAP_TIMER_MATCH_REG,
+	OMAP_TIMER_CAPTURE_REG,
+	OMAP_TIMER_IF_CTRL_REG,
+	OMAP_TIMER_CAPTURE2_REG,
+	OMAP_TIMER_TICK_POS_REG,
+	OMAP_TIMER_TICK_NEG_REG,
+	OMAP_TIMER_TICK_COUNT_REG,
+	OMAP_TIMER_TICK_INT_MASK_SET_REG,
+	OMAP_TIMER_TICK_INT_MASK_COUNT_REG,
+	OMAP_TIMER_INT_CLR_REG,
+};
 
 /**
  * omap_timer_dev_attr - timer device attribute
-- 
1.6.0.4

RE: [PATCHv3 8/17] dmtimer: register mappings moved to hwmod database

[G, Manjunath Kondaiah]

Hi Tarun,

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org 
> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of 
> DebBarma, Tarun Kanti
> Sent: Tuesday, September 21, 2010 2:24 PM
> To: linux-omap@vger.kernel.org
> Cc: DebBarma, Tarun Kanti; Basak, Partha; Shilimkar, Santosh; 
> Cousson, Benoit; Paul Walmsley; Kevin Hilman; Tony Lindgren
> Subject: [PATCHv3 8/17] dmtimer: register mappings moved to 
> hwmod database
> 
> This patch adds dmtimer register mappings to hwmod database.
> This is to avoid maintaining different several mappings 
> within the omap-plat. The mapping is made available to 
> platform through dev_attr during device build. The pointer to 
> register map is preserved in the platform data.
> 
> Please note that the cleanup of register map from plat-omap 
> will be removed in later patch after conversion to platform driver.
> 
> Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
> Signed-off-by: Partha Basak <p-basak2@ti.com>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Cousson, Benoit <b-cousson@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> Cc: Tony Lindgren <tony@atomide.com>
> ---
>  arch/arm/mach-omap2/omap_hwmod_2420_data.c |   25 ++++++++++
>  arch/arm/mach-omap2/omap_hwmod_2430_data.c |   25 ++++++++++
>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   25 ++++++++++
>  arch/arm/mach-omap2/omap_hwmod_44xx_data.c |   67 
> ++++++++++++++++++++++++---
>  arch/arm/plat-omap/dmtimer.c               |   13 -----
>  arch/arm/plat-omap/include/plat/dmtimer.h  |   38 ++++++++++++++++
>  6 files changed, 172 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod_2420_data.c 
> b/arch/arm/mach-omap2/omap_hwmod_2420_data.c
> index 6003c2e..b3dd8ac 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_2420_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_2420_data.c
> @@ -131,8 +131,33 @@ static char *timer_clk_src_names[] = {
>  	NULL,
>  };
>  
> +/* OMAP2/3 timers register map */
> +static u32 omap_timer_reg_map_v1[] = {
> +	[OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR << WPSHIFT)),
> +	[OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR << WPSHIFT)),
> +	[OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR << WPSHIFT)),
> +	[OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR << WPSHIFT)),
> +	[OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR << WPSHIFT)),
> +	[OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR << WPSHIFT)),
> +	[OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR << WPSHIFT)),
> +	[OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR << WPSHIFT)),
> +	[OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 | 
> (WP_TOCR << WPSHIFT)),
> +	[OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 | 
> (WP_TOWR << WPSHIFT)),
> +};
> +
>  static struct omap_timer_dev_attr timer_dev_attr = {
>  	.clk_names	= timer_clk_src_names,
> +	.reg_map	= omap_timer_reg_map_v1,
>  };
>  
>  static struct omap_hwmod_class_sysconfig omap2420_timer_sysc 
> = { diff --git a/arch/arm/mach-omap2/omap_hwmod_2430_data.c 
> b/arch/arm/mach-omap2/omap_hwmod_2430_data.c
> index 4b43fb9..787d3ce 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_2430_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_2430_data.c
> @@ -137,8 +137,33 @@ static char *timer_clk_src_names[] = {
>  	NULL
>  };
>  
> +/* OMAP2/3 timers register map */
> +static u32 omap_timer_reg_map_v1[] = {
> +	[OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR << WPSHIFT)),
> +	[OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR << WPSHIFT)),
> +	[OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR << WPSHIFT)),
> +	[OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR << WPSHIFT)),
> +	[OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR << WPSHIFT)),
> +	[OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR << WPSHIFT)),
> +	[OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR << WPSHIFT)),
> +	[OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR << WPSHIFT)),
> +	[OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 | 
> (WP_TOCR << WPSHIFT)),
> +	[OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 | 
> (WP_TOWR << WPSHIFT)),
> +};
> +
>  static struct omap_timer_dev_attr timer_dev_attr = {
>  	.clk_names	= timer_clk_src_names,
> +	.reg_map	= omap_timer_reg_map_v1,
>  };
>  
>  static struct omap_hwmod_class_sysconfig omap2430_timer_sysc 
> = { diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c 
> b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> index 70446d7..e477ce8 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> @@ -109,8 +109,33 @@ static char *timer_clk_src_names[] = {
>  	NULL,
>  };
>  
> +/* OMAP2/3 timers register map */
> +static u32 omap_timer_reg_map_v1[] = {
> +	[OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR << WPSHIFT)),
> +	[OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR << WPSHIFT)),
> +	[OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR << WPSHIFT)),
> +	[OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR << WPSHIFT)),
> +	[OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR << WPSHIFT)),
> +	[OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR << WPSHIFT)),
> +	[OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR << WPSHIFT)),
> +	[OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR << WPSHIFT)),
> +	[OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 | 
> (WP_TOCR << WPSHIFT)),
> +	[OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 | 
> (WP_TOWR << WPSHIFT)),
> +};
> +
>  static struct omap_timer_dev_attr timer_dev_attr = {
>  	.clk_names	= timer_clk_src_names,
> +	.reg_map	= omap_timer_reg_map_v1,
>  };
>  
>  /* timer class */
> diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c 
> b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> index 7c68228..1f60f8a 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> @@ -471,12 +471,63 @@ static char *timer_clk_src_names_abe[] = {
>  	NULL,
>  };
>  
> -static struct omap_timer_dev_attr timer_dev_attr = {
> +/* OMAP2/3 timers register map */
> +static u32 omap_timer_reg_map_v1[] = {
> +	[OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR << WPSHIFT)),
> +	[OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR << WPSHIFT)),
> +	[OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR << WPSHIFT)),
> +	[OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR << WPSHIFT)),
> +	[OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR << WPSHIFT)),
> +	[OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR << WPSHIFT)),
> +	[OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR << WPSHIFT)),
> +	[OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR << WPSHIFT)),
> +	[OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 | 
> (WP_TOCR << WPSHIFT)),
> +	[OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 | 
> (WP_TOWR << WPSHIFT)),
> +};
> +
> +/* OMAP4 timers register map */
> +static u32 omap_timer_reg_map_v2[] = {
> +	[OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_STAT_REG]           = (0x28 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_INT_EN_REG]         = (0x2c | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_WAKEUP_EN_REG]      = (0x34 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_CTRL_REG]           = (0x38 | (WP_TCLR << WPSHIFT)),
> +	[OMAP_TIMER_COUNTER_REG]        = (0x3c | (WP_TCRR << WPSHIFT)),
> +	[OMAP_TIMER_LOAD_REG]           = (0x40 | (WP_TLDR << WPSHIFT)),
> +	[OMAP_TIMER_TRIGGER_REG]        = (0x44 | (WP_TTGR << WPSHIFT)),
> +	[OMAP_TIMER_WRITE_PEND_REG]     = (0x48 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_MATCH_REG]          = (0x4c | (WP_TMAR << WPSHIFT)),
> +	[OMAP_TIMER_CAPTURE_REG]        = (0x50 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_IF_CTRL_REG]        = (0x54 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_CAPTURE2_REG]       = (0x58 | (WP_NONE << WPSHIFT)),
> +	[OMAP_TIMER_INT_CLR_REG]        = (0x30 | (WP_NONE << WPSHIFT)),
> +};
> +

Why not these defines in mach-omap2/dmtimer.c? since register offsets are
same for omap2plus except omap4 which has additional register offsets. Same
register offsets are getting repeated in all the omap2plus hwmod database.

As kevin suggested for DMA hwmod, i2c driver is already using these defines
in .c file even though register offsets are different within omap2plus processors.
through dev structure.

Can we have similar kind of usage for all upcoming driver hwmod adaptations?

-Manjunath

Re: [PATCHv3 8/17] dmtimer: register mappings moved to hwmod database

[Kevin Hilman]

"G, Manjunath Kondaiah" <manjugk@ti.com> writes:

> Hi Tarun,
>
>> -----Original Message-----
>> From: linux-omap-owner@vger.kernel.org 
>> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of 
>> DebBarma, Tarun Kanti
>> Sent: Tuesday, September 21, 2010 2:24 PM
>> To: linux-omap@vger.kernel.org
>> Cc: DebBarma, Tarun Kanti; Basak, Partha; Shilimkar, Santosh; 
>> Cousson, Benoit; Paul Walmsley; Kevin Hilman; Tony Lindgren
>> Subject: [PATCHv3 8/17] dmtimer: register mappings moved to 
>> hwmod database
>> 
>> This patch adds dmtimer register mappings to hwmod database.
>> This is to avoid maintaining different several mappings 
>> within the omap-plat. The mapping is made available to 
>> platform through dev_attr during device build. The pointer to 
>> register map is preserved in the platform data.
>> 
>> Please note that the cleanup of register map from plat-omap 
>> will be removed in later patch after conversion to platform driver.
>> 
>> Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
>> Signed-off-by: Partha Basak <p-basak2@ti.com>
>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>> Cc: Cousson, Benoit <b-cousson@ti.com>
>> Cc: Paul Walmsley <paul@pwsan.com>
>> Cc: Kevin Hilman <khilman@deeprootsystems.com>
>> Cc: Tony Lindgren <tony@atomide.com>
>> ---
>>  arch/arm/mach-omap2/omap_hwmod_2420_data.c |   25 ++++++++++
>>  arch/arm/mach-omap2/omap_hwmod_2430_data.c |   25 ++++++++++
>>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   25 ++++++++++
>>  arch/arm/mach-omap2/omap_hwmod_44xx_data.c |   67 
>> ++++++++++++++++++++++++---
>>  arch/arm/plat-omap/dmtimer.c               |   13 -----
>>  arch/arm/plat-omap/include/plat/dmtimer.h  |   38 ++++++++++++++++
>>  6 files changed, 172 insertions(+), 21 deletions(-)
>> 
>> diff --git a/arch/arm/mach-omap2/omap_hwmod_2420_data.c 
>> b/arch/arm/mach-omap2/omap_hwmod_2420_data.c
>> index 6003c2e..b3dd8ac 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod_2420_data.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod_2420_data.c
>> @@ -131,8 +131,33 @@ static char *timer_clk_src_names[] = {
>>  	NULL,
>>  };
>>  
>> +/* OMAP2/3 timers register map */
>> +static u32 omap_timer_reg_map_v1[] = {
>> +	[OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR << WPSHIFT)),
>> +	[OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR << WPSHIFT)),
>> +	[OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR << WPSHIFT)),
>> +	[OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR << WPSHIFT)),
>> +	[OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR << WPSHIFT)),
>> +	[OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR << WPSHIFT)),
>> +	[OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR << WPSHIFT)),
>> +	[OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR << WPSHIFT)),
>> +	[OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 | 
>> (WP_TOCR << WPSHIFT)),
>> +	[OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 | 
>> (WP_TOWR << WPSHIFT)),
>> +};
>> +
>>  static struct omap_timer_dev_attr timer_dev_attr = {
>>  	.clk_names	= timer_clk_src_names,
>> +	.reg_map	= omap_timer_reg_map_v1,
>>  };
>>  
>>  static struct omap_hwmod_class_sysconfig omap2420_timer_sysc 
>> = { diff --git a/arch/arm/mach-omap2/omap_hwmod_2430_data.c 
>> b/arch/arm/mach-omap2/omap_hwmod_2430_data.c
>> index 4b43fb9..787d3ce 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod_2430_data.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod_2430_data.c
>> @@ -137,8 +137,33 @@ static char *timer_clk_src_names[] = {
>>  	NULL
>>  };
>>  
>> +/* OMAP2/3 timers register map */
>> +static u32 omap_timer_reg_map_v1[] = {
>> +	[OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR << WPSHIFT)),
>> +	[OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR << WPSHIFT)),
>> +	[OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR << WPSHIFT)),
>> +	[OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR << WPSHIFT)),
>> +	[OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR << WPSHIFT)),
>> +	[OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR << WPSHIFT)),
>> +	[OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR << WPSHIFT)),
>> +	[OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR << WPSHIFT)),
>> +	[OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 | 
>> (WP_TOCR << WPSHIFT)),
>> +	[OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 | 
>> (WP_TOWR << WPSHIFT)),
>> +};
>> +
>>  static struct omap_timer_dev_attr timer_dev_attr = {
>>  	.clk_names	= timer_clk_src_names,
>> +	.reg_map	= omap_timer_reg_map_v1,
>>  };
>>  
>>  static struct omap_hwmod_class_sysconfig omap2430_timer_sysc 
>> = { diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c 
>> b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>> index 70446d7..e477ce8 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>> @@ -109,8 +109,33 @@ static char *timer_clk_src_names[] = {
>>  	NULL,
>>  };
>>  
>> +/* OMAP2/3 timers register map */
>> +static u32 omap_timer_reg_map_v1[] = {
>> +	[OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR << WPSHIFT)),
>> +	[OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR << WPSHIFT)),
>> +	[OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR << WPSHIFT)),
>> +	[OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR << WPSHIFT)),
>> +	[OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR << WPSHIFT)),
>> +	[OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR << WPSHIFT)),
>> +	[OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR << WPSHIFT)),
>> +	[OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR << WPSHIFT)),
>> +	[OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 | 
>> (WP_TOCR << WPSHIFT)),
>> +	[OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 | 
>> (WP_TOWR << WPSHIFT)),
>> +};
>> +
>>  static struct omap_timer_dev_attr timer_dev_attr = {
>>  	.clk_names	= timer_clk_src_names,
>> +	.reg_map	= omap_timer_reg_map_v1,
>>  };
>>  
>>  /* timer class */
>> diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c 
>> b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
>> index 7c68228..1f60f8a 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
>> @@ -471,12 +471,63 @@ static char *timer_clk_src_names_abe[] = {
>>  	NULL,
>>  };
>>  
>> -static struct omap_timer_dev_attr timer_dev_attr = {
>> +/* OMAP2/3 timers register map */
>> +static u32 omap_timer_reg_map_v1[] = {
>> +	[OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR << WPSHIFT)),
>> +	[OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR << WPSHIFT)),
>> +	[OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR << WPSHIFT)),
>> +	[OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR << WPSHIFT)),
>> +	[OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR << WPSHIFT)),
>> +	[OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR << WPSHIFT)),
>> +	[OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR << WPSHIFT)),
>> +	[OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR << WPSHIFT)),
>> +	[OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 | 
>> (WP_TOCR << WPSHIFT)),
>> +	[OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 | 
>> (WP_TOWR << WPSHIFT)),
>> +};
>> +
>> +/* OMAP4 timers register map */
>> +static u32 omap_timer_reg_map_v2[] = {
>> +	[OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_STAT_REG]           = (0x28 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_INT_EN_REG]         = (0x2c | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_WAKEUP_EN_REG]      = (0x34 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_CTRL_REG]           = (0x38 | (WP_TCLR << WPSHIFT)),
>> +	[OMAP_TIMER_COUNTER_REG]        = (0x3c | (WP_TCRR << WPSHIFT)),
>> +	[OMAP_TIMER_LOAD_REG]           = (0x40 | (WP_TLDR << WPSHIFT)),
>> +	[OMAP_TIMER_TRIGGER_REG]        = (0x44 | (WP_TTGR << WPSHIFT)),
>> +	[OMAP_TIMER_WRITE_PEND_REG]     = (0x48 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_MATCH_REG]          = (0x4c | (WP_TMAR << WPSHIFT)),
>> +	[OMAP_TIMER_CAPTURE_REG]        = (0x50 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_IF_CTRL_REG]        = (0x54 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_CAPTURE2_REG]       = (0x58 | (WP_NONE << WPSHIFT)),
>> +	[OMAP_TIMER_INT_CLR_REG]        = (0x30 | (WP_NONE << WPSHIFT)),
>> +};
>> +
>
> Why not these defines in mach-omap2/dmtimer.c? since register offsets are
> same for omap2plus except omap4 which has additional register offsets. Same
> register offsets are getting repeated in all the omap2plus hwmod database.

I agree with Manjunath.

> As kevin suggested for DMA hwmod, i2c driver is already using these defines
> in .c file even though register offsets are different within omap2plus processors.
> through dev structure.
>
> Can we have similar kind of usage for all upcoming driver hwmod adaptations?

Yes please.

hwmod is not the place for register definitions, specially when OMAP4 is
the only difference.

Kevin

RE: [PATCHv3 8/17] dmtimer: register mappings moved to hwmod database

[DebBarma, Tarun Kanti]

> >
> > Why not these defines in mach-omap2/dmtimer.c? since register offsets
> are
> > same for omap2plus except omap4 which has additional register offsets.
> Same
> > register offsets are getting repeated in all the omap2plus hwmod
> database.
> 
> I agree with Manjunath.
> 
> > As kevin suggested for DMA hwmod, i2c driver is already using these
> defines
> > in .c file even though register offsets are different within omap2plus
> processors.
> > through dev structure.
> >
> > Can we have similar kind of usage for all upcoming driver hwmod
> adaptations?
> 
> Yes please.
> 
> hwmod is not the place for register definitions, specially when OMAP4 is
> the only difference.

I can move it to its original place in mach-omap2.
The reason for re-location was if OMAP 5 and later version had different mappings we had to keep on adding new table in mach-omap2 in addition to applying sort if() checks for the different platforms.

-tarun

RE: [PATCHv3 8/17] dmtimer: register mappings moved to hwmod database

[Basak, Partha]

 

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@deeprootsystems.com] 
> Sent: Thursday, September 23, 2010 3:10 AM
> To: G, Manjunath Kondaiah
> Cc: DebBarma, Tarun Kanti; linux-omap@vger.kernel.org; Basak, 
> Partha; Shilimkar, Santosh; Cousson, Benoit; Paul Walmsley; 
> Tony Lindgren
> Subject: Re: [PATCHv3 8/17] dmtimer: register mappings moved 
> to hwmod database
> 
> "G, Manjunath Kondaiah" <manjugk@ti.com> writes:
> 
> > Hi Tarun,
> >
> >> -----Original Message-----
> >> From: linux-omap-owner@vger.kernel.org 
> >> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of 
> >> DebBarma, Tarun Kanti
> >> Sent: Tuesday, September 21, 2010 2:24 PM
> >> To: linux-omap@vger.kernel.org
> >> Cc: DebBarma, Tarun Kanti; Basak, Partha; Shilimkar, Santosh; 
> >> Cousson, Benoit; Paul Walmsley; Kevin Hilman; Tony Lindgren
> >> Subject: [PATCHv3 8/17] dmtimer: register mappings moved to 
> >> hwmod database
> >> 
> >> This patch adds dmtimer register mappings to hwmod database.
> >> This is to avoid maintaining different several mappings 
> >> within the omap-plat. The mapping is made available to 
> >> platform through dev_attr during device build. The pointer to 
> >> register map is preserved in the platform data.
> >> 
> >> Please note that the cleanup of register map from plat-omap 
> >> will be removed in later patch after conversion to platform driver.
> >> 
> >> Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
> >> Signed-off-by: Partha Basak <p-basak2@ti.com>
> >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> >> Cc: Cousson, Benoit <b-cousson@ti.com>
> >> Cc: Paul Walmsley <paul@pwsan.com>
> >> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> >> Cc: Tony Lindgren <tony@atomide.com>
> >> ---
> >>  arch/arm/mach-omap2/omap_hwmod_2420_data.c |   25 ++++++++++
> >>  arch/arm/mach-omap2/omap_hwmod_2430_data.c |   25 ++++++++++
> >>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   25 ++++++++++
> >>  arch/arm/mach-omap2/omap_hwmod_44xx_data.c |   67 
> >> ++++++++++++++++++++++++---
> >>  arch/arm/plat-omap/dmtimer.c               |   13 -----
> >>  arch/arm/plat-omap/include/plat/dmtimer.h  |   38 ++++++++++++++++
> >>  6 files changed, 172 insertions(+), 21 deletions(-)
> >> 
> >> diff --git a/arch/arm/mach-omap2/omap_hwmod_2420_data.c 
> >> b/arch/arm/mach-omap2/omap_hwmod_2420_data.c
> >> index 6003c2e..b3dd8ac 100644
> >> --- a/arch/arm/mach-omap2/omap_hwmod_2420_data.c
> >> +++ b/arch/arm/mach-omap2/omap_hwmod_2420_data.c
> >> @@ -131,8 +131,33 @@ static char *timer_clk_src_names[] = {
> >>  	NULL,
> >>  };
> >>  
> >> +/* OMAP2/3 timers register map */
> >> +static u32 omap_timer_reg_map_v1[] = {
> >> +	[OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR << WPSHIFT)),
> >> +	[OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR << WPSHIFT)),
> >> +	[OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR << WPSHIFT)),
> >> +	[OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR << WPSHIFT)),
> >> +	[OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR << WPSHIFT)),
> >> +	[OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR << WPSHIFT)),
> >> +	[OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR << WPSHIFT)),
> >> +	[OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR << WPSHIFT)),
> >> +	[OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 | 
> >> (WP_TOCR << WPSHIFT)),
> >> +	[OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 | 
> >> (WP_TOWR << WPSHIFT)),
> >> +};
> >> +
> >>  static struct omap_timer_dev_attr timer_dev_attr = {
> >>  	.clk_names	= timer_clk_src_names,
> >> +	.reg_map	= omap_timer_reg_map_v1,
> >>  };
> >>  
> >>  static struct omap_hwmod_class_sysconfig omap2420_timer_sysc 
> >> = { diff --git a/arch/arm/mach-omap2/omap_hwmod_2430_data.c 
> >> b/arch/arm/mach-omap2/omap_hwmod_2430_data.c
> >> index 4b43fb9..787d3ce 100644
> >> --- a/arch/arm/mach-omap2/omap_hwmod_2430_data.c
> >> +++ b/arch/arm/mach-omap2/omap_hwmod_2430_data.c
> >> @@ -137,8 +137,33 @@ static char *timer_clk_src_names[] = {
> >>  	NULL
> >>  };
> >>  
> >> +/* OMAP2/3 timers register map */
> >> +static u32 omap_timer_reg_map_v1[] = {
> >> +	[OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR << WPSHIFT)),
> >> +	[OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR << WPSHIFT)),
> >> +	[OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR << WPSHIFT)),
> >> +	[OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR << WPSHIFT)),
> >> +	[OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR << WPSHIFT)),
> >> +	[OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR << WPSHIFT)),
> >> +	[OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR << WPSHIFT)),
> >> +	[OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR << WPSHIFT)),
> >> +	[OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 | 
> >> (WP_TOCR << WPSHIFT)),
> >> +	[OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 | 
> >> (WP_TOWR << WPSHIFT)),
> >> +};
> >> +
> >>  static struct omap_timer_dev_attr timer_dev_attr = {
> >>  	.clk_names	= timer_clk_src_names,
> >> +	.reg_map	= omap_timer_reg_map_v1,
> >>  };
> >>  
> >>  static struct omap_hwmod_class_sysconfig omap2430_timer_sysc 
> >> = { diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c 
> >> b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> >> index 70446d7..e477ce8 100644
> >> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> >> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> >> @@ -109,8 +109,33 @@ static char *timer_clk_src_names[] = {
> >>  	NULL,
> >>  };
> >>  
> >> +/* OMAP2/3 timers register map */
> >> +static u32 omap_timer_reg_map_v1[] = {
> >> +	[OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR << WPSHIFT)),
> >> +	[OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR << WPSHIFT)),
> >> +	[OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR << WPSHIFT)),
> >> +	[OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR << WPSHIFT)),
> >> +	[OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR << WPSHIFT)),
> >> +	[OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR << WPSHIFT)),
> >> +	[OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR << WPSHIFT)),
> >> +	[OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR << WPSHIFT)),
> >> +	[OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 | 
> >> (WP_TOCR << WPSHIFT)),
> >> +	[OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 | 
> >> (WP_TOWR << WPSHIFT)),
> >> +};
> >> +
> >>  static struct omap_timer_dev_attr timer_dev_attr = {
> >>  	.clk_names	= timer_clk_src_names,
> >> +	.reg_map	= omap_timer_reg_map_v1,
> >>  };
> >>  
> >>  /* timer class */
> >> diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c 
> >> b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> >> index 7c68228..1f60f8a 100644
> >> --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> >> +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> >> @@ -471,12 +471,63 @@ static char *timer_clk_src_names_abe[] = {
> >>  	NULL,
> >>  };
> >>  
> >> -static struct omap_timer_dev_attr timer_dev_attr = {
> >> +/* OMAP2/3 timers register map */
> >> +static u32 omap_timer_reg_map_v1[] = {
> >> +	[OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR << WPSHIFT)),
> >> +	[OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR << WPSHIFT)),
> >> +	[OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR << WPSHIFT)),
> >> +	[OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR << WPSHIFT)),
> >> +	[OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR << WPSHIFT)),
> >> +	[OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR << WPSHIFT)),
> >> +	[OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR << WPSHIFT)),
> >> +	[OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR << WPSHIFT)),
> >> +	[OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 | 
> >> (WP_TOCR << WPSHIFT)),
> >> +	[OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 | 
> >> (WP_TOWR << WPSHIFT)),
> >> +};
> >> +
> >> +/* OMAP4 timers register map */
> >> +static u32 omap_timer_reg_map_v2[] = {
> >> +	[OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_STAT_REG]           = (0x28 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_INT_EN_REG]         = (0x2c | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_WAKEUP_EN_REG]      = (0x34 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_CTRL_REG]           = (0x38 | (WP_TCLR << WPSHIFT)),
> >> +	[OMAP_TIMER_COUNTER_REG]        = (0x3c | (WP_TCRR << WPSHIFT)),
> >> +	[OMAP_TIMER_LOAD_REG]           = (0x40 | (WP_TLDR << WPSHIFT)),
> >> +	[OMAP_TIMER_TRIGGER_REG]        = (0x44 | (WP_TTGR << WPSHIFT)),
> >> +	[OMAP_TIMER_WRITE_PEND_REG]     = (0x48 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_MATCH_REG]          = (0x4c | (WP_TMAR << WPSHIFT)),
> >> +	[OMAP_TIMER_CAPTURE_REG]        = (0x50 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_IF_CTRL_REG]        = (0x54 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_CAPTURE2_REG]       = (0x58 | (WP_NONE << WPSHIFT)),
> >> +	[OMAP_TIMER_INT_CLR_REG]        = (0x30 | (WP_NONE << WPSHIFT)),
> >> +};
> >> +
> >
> > Why not these defines in mach-omap2/dmtimer.c? since 
> register offsets are
> > same for omap2plus except omap4 which has additional 
> register offsets. Same
> > register offsets are getting repeated in all the omap2plus 
> hwmod database.
> 
> I agree with Manjunath.

Manju, the number of tables needed to manage the information are same really.
Its only where they are located changes from the mach layer to the hwmod
database. So, thats not the point. dev_attrs are pointing to the reg-map
tables, they are not duplicating them.


The offset differences are stemming from the IP differences. 
To my understanding, only IP-integration specific idiosyncrasies into a given
SOC qualifiy as dev_attrs.
IP specific details like reg-map information should be maintained within the driver.
So, this approach will break this paradigm & also not follow existing implementation
of drivers which support this. A given driver may support a set of IPs but still
may cater to even non-OMAP platforms not supporting hwmod.

However, if we see the general usage of dev_attrs, there is no clear line of demarcation
really what should make it and what should not, as is used to plug this sort of 
small gotchas and reduce CPU checks in the code.

We will revert back this change.

> 
> > As kevin suggested for DMA hwmod, i2c driver is already 
> using these defines
> > in .c file even though register offsets are different 
> within omap2plus processors.
> > through dev structure.
> >
> > Can we have similar kind of usage for all upcoming driver 
> hwmod adaptations?
> 
> Yes please.
> 
> hwmod is not the place for register definitions, specially 
> when OMAP4 is
> the only difference.
> 
> Kevin
> 
> 

Re: [PATCHv3 8/17] dmtimer: register mappings moved to hwmod database

[Cousson, Benoit]

On 9/23/2010 11:34 AM, Basak, Partha wrote:
>
>
>> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>> Sent: Thursday, September 23, 2010 3:10 AM
>>
>> "G, Manjunath Kondaiah"<manjugk@ti.com>  writes:
>>
>>> Hi Tarun,
>>>
>>>> From: linux-omap-owner@vger.kernel.org
>>>> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of
>>>> DebBarma, Tarun Kanti
>>>>

<...>

>>>> +static u32 omap_timer_reg_map_v1[] = {
>>>> +  [OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 |
>>>> (WP_TOCR<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 |
>>>> (WP_TOWR<<  WPSHIFT)),
>>>> +};
>>>> +
>>>> +/* OMAP4 timers register map */
>>>> +static u32 omap_timer_reg_map_v2[] = {
>>>> +  [OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_STAT_REG]           = (0x28 | (WP_NONE<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_INT_EN_REG]         = (0x2c | (WP_NONE<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_WAKEUP_EN_REG]      = (0x34 | (WP_NONE<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_CTRL_REG]           = (0x38 | (WP_TCLR<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_COUNTER_REG]        = (0x3c | (WP_TCRR<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_LOAD_REG]           = (0x40 | (WP_TLDR<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_TRIGGER_REG]        = (0x44 | (WP_TTGR<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_WRITE_PEND_REG]     = (0x48 | (WP_NONE<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_MATCH_REG]          = (0x4c | (WP_TMAR<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_CAPTURE_REG]        = (0x50 | (WP_NONE<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_IF_CTRL_REG]        = (0x54 | (WP_NONE<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_CAPTURE2_REG]       = (0x58 | (WP_NONE<<  WPSHIFT)),
>>>> +  [OMAP_TIMER_INT_CLR_REG]        = (0x30 | (WP_NONE<<  WPSHIFT)),
>>>> +};
>>>> +
>>>
>>> Why not these defines in mach-omap2/dmtimer.c? since
>> register offsets are
>>> same for omap2plus except omap4 which has additional
>> register offsets. Same
>>> register offsets are getting repeated in all the omap2plus
>> hwmod database.
>>
>> I agree with Manjunath.
>
> Manju, the number of tables needed to manage the information are same really.
> Its only where they are located changes from the mach layer to the hwmod
> database. So, thats not the point. dev_attrs are pointing to the reg-map
> tables, they are not duplicating them.
>
>
> The offset differences are stemming from the IP differences.
> To my understanding, only IP-integration specific idiosyncrasies into a given
> SOC qualifiy as dev_attrs.
> IP specific details like reg-map information should be maintained within the driver.
> So, this approach will break this paradigm&  also not follow existing implementation
> of drivers which support this. A given driver may support a set of IPs but still
> may cater to even non-OMAP platforms not supporting hwmod.
>
> However, if we see the general usage of dev_attrs, there is no clear line of demarcation
> really what should make it and what should not, as is used to plug this sort of
> small gotchas and reduce CPU checks in the code.

You do not really need that to reduce the CPU check in the code.
In that case, only the IP version should be stored in the dev_attr. 
Based on that IP version, you know exactly what register map you have to 
handle. For the moment you just have 2 layouts to handle + the extra 
register for the 1ms timers.

Also, I'm not sure that you have to handle each register separately 
considering that you have one offset to handle for the functional register.
The change in sysconfig and interrupt register are all standard mapping 
that stick to TI highlander standard.
Meaning, as soon as an IP is a v2 (highlander version) all these 
registers will be at the same place... at least in theory :-)

Here are the deltas between the legacy and the new one:

[OMAP_TIMER_ID_REG]             0x00,
[OMAP_TIMER_OCP_CFG_REG]        0x10, same
[OMAP_TIMER_SYS_STAT_REG]       0x14, (not used anymore)

You should not care about these ones, because hwmod will handle them.

[OMAP_TIMER_STAT_REG]           0x28, +10
[OMAP_TIMER_INT_EN_REG]         0x2c, +10
[OMAP_TIMER_INT_CLR_REG]        0x30, (new)

These ones are standard registers

[OMAP_TIMER_WAKEUP_EN_REG]      0x34, +14
[OMAP_TIMER_CTRL_REG]           0x38, +14
[OMAP_TIMER_COUNTER_REG]        0x3c, +14
[OMAP_TIMER_LOAD_REG]           0x40, +14
[OMAP_TIMER_TRIGGER_REG]        0x44, +14
[OMAP_TIMER_WRITE_PEND_REG]     0x48, +14
[OMAP_TIMER_MATCH_REG]          0x4c, +14
[OMAP_TIMER_CAPTURE_REG]        0x50, +14
[OMAP_TIMER_IF_CTRL_REG]        0x54, +14
[OMAP_TIMER_CAPTURE2_REG]       0x58, +14

You can probably handle that with only 2 offsets instead of having one 
address per registers.

Benoit

RE: [PATCHv3 8/17] dmtimer: register mappings moved to hwmod database

[DebBarma, Tarun Kanti]

Benoit,

> -----Original Message-----
> From: Cousson, Benoit
> Sent: Thursday, September 23, 2010 10:15 PM
> To: Basak, Partha
> Cc: Kevin Hilman; G, Manjunath Kondaiah; DebBarma, Tarun Kanti; linux-
> omap@vger.kernel.org; Shilimkar, Santosh; Paul Walmsley; Tony Lindgren
> Subject: Re: [PATCHv3 8/17] dmtimer: register mappings moved to hwmod
> database
> 
> On 9/23/2010 11:34 AM, Basak, Partha wrote:
> >
> >
> >> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
> >> Sent: Thursday, September 23, 2010 3:10 AM
> >>
> >> "G, Manjunath Kondaiah"<manjugk@ti.com>  writes:
> >>
> >>> Hi Tarun,
> >>>
> >>>> From: linux-omap-owner@vger.kernel.org
> >>>> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of
> >>>> DebBarma, Tarun Kanti
> >>>>
> 
> <...>
> 
> >>>> +static u32 omap_timer_reg_map_v1[] = {
> >>>> +  [OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 |
> >>>> (WP_TOCR<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 |
> >>>> (WP_TOWR<<  WPSHIFT)),
> >>>> +};
> >>>> +
> >>>> +/* OMAP4 timers register map */
> >>>> +static u32 omap_timer_reg_map_v2[] = {
> >>>> +  [OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_STAT_REG]           = (0x28 | (WP_NONE<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_INT_EN_REG]         = (0x2c | (WP_NONE<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_WAKEUP_EN_REG]      = (0x34 | (WP_NONE<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_CTRL_REG]           = (0x38 | (WP_TCLR<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_COUNTER_REG]        = (0x3c | (WP_TCRR<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_LOAD_REG]           = (0x40 | (WP_TLDR<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_TRIGGER_REG]        = (0x44 | (WP_TTGR<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_WRITE_PEND_REG]     = (0x48 | (WP_NONE<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_MATCH_REG]          = (0x4c | (WP_TMAR<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_CAPTURE_REG]        = (0x50 | (WP_NONE<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_IF_CTRL_REG]        = (0x54 | (WP_NONE<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_CAPTURE2_REG]       = (0x58 | (WP_NONE<<  WPSHIFT)),
> >>>> +  [OMAP_TIMER_INT_CLR_REG]        = (0x30 | (WP_NONE<<  WPSHIFT)),
> >>>> +};
> >>>> +
> >>>
> >>> Why not these defines in mach-omap2/dmtimer.c? since
> >> register offsets are
> >>> same for omap2plus except omap4 which has additional
> >> register offsets. Same
> >>> register offsets are getting repeated in all the omap2plus
> >> hwmod database.
> >>
> >> I agree with Manjunath.
> >
> > Manju, the number of tables needed to manage the information are same
> really.
> > Its only where they are located changes from the mach layer to the hwmod
> > database. So, thats not the point. dev_attrs are pointing to the reg-map
> > tables, they are not duplicating them.
> >
> >
> > The offset differences are stemming from the IP differences.
> > To my understanding, only IP-integration specific idiosyncrasies into a
> given
> > SOC qualifiy as dev_attrs.
> > IP specific details like reg-map information should be maintained within
> the driver.
> > So, this approach will break this paradigm&  also not follow existing
> implementation
> > of drivers which support this. A given driver may support a set of IPs
> but still
> > may cater to even non-OMAP platforms not supporting hwmod.
> >
> > However, if we see the general usage of dev_attrs, there is no clear
> line of demarcation
> > really what should make it and what should not, as is used to plug this
> sort of
> > small gotchas and reduce CPU checks in the code.
> 
> You do not really need that to reduce the CPU check in the code.
> In that case, only the IP version should be stored in the dev_attr.
> Based on that IP version, you know exactly what register map you have to
> handle. For the moment you just have 2 layouts to handle + the extra
> register for the 1ms timers.
> 
> Also, I'm not sure that you have to handle each register separately
> considering that you have one offset to handle for the functional
> register.
> The change in sysconfig and interrupt register are all standard mapping
> that stick to TI highlander standard.
> Meaning, as soon as an IP is a v2 (highlander version) all these
> registers will be at the same place... at least in theory :-)
> 
> Here are the deltas between the legacy and the new one:
> 
> [OMAP_TIMER_ID_REG]             0x00,
> [OMAP_TIMER_OCP_CFG_REG]        0x10, same
> [OMAP_TIMER_SYS_STAT_REG]       0x14, (not used anymore)
> 
> You should not care about these ones, because hwmod will handle them.
> 
> [OMAP_TIMER_STAT_REG]           0x28, +10
> [OMAP_TIMER_INT_EN_REG]         0x2c, +10
> [OMAP_TIMER_INT_CLR_REG]        0x30, (new)
> 
> These ones are standard registers
> 
> [OMAP_TIMER_WAKEUP_EN_REG]      0x34, +14
> [OMAP_TIMER_CTRL_REG]           0x38, +14
> [OMAP_TIMER_COUNTER_REG]        0x3c, +14
> [OMAP_TIMER_LOAD_REG]           0x40, +14
> [OMAP_TIMER_TRIGGER_REG]        0x44, +14
> [OMAP_TIMER_WRITE_PEND_REG]     0x48, +14
> [OMAP_TIMER_MATCH_REG]          0x4c, +14
> [OMAP_TIMER_CAPTURE_REG]        0x50, +14
> [OMAP_TIMER_IF_CTRL_REG]        0x54, +14
> [OMAP_TIMER_CAPTURE2_REG]       0x58, +14
> 
> You can probably handle that with only 2 offsets instead of having one
> address per registers.
> 
To keep aligned with other driver implementations, I would like to follow this:

1) move the table in mach-omap1/2 since
2) remove entries which are handled by hwmod.
However, I am not sure if there is any issue in keeping them in the table.

-tarun

RE: [PATCHv3 8/17] dmtimer: register mappings moved to hwmod database

[DebBarma, Tarun Kanti]

Benoit and Manju,

> -----Original Message-----
> From: DebBarma, Tarun Kanti
> Sent: Saturday, October 09, 2010 9:10 PM
> To: Cousson, Benoit; Basak, Partha
> Cc: Kevin Hilman; G, Manjunath Kondaiah; linux-omap@vger.kernel.org;
> Shilimkar, Santosh; Paul Walmsley; Tony Lindgren
> Subject: RE: [PATCHv3 8/17] dmtimer: register mappings moved to hwmod
> database
> 
> Benoit,
> 
> > -----Original Message-----
> > From: Cousson, Benoit
> > Sent: Thursday, September 23, 2010 10:15 PM
> > To: Basak, Partha
> > Cc: Kevin Hilman; G, Manjunath Kondaiah; DebBarma, Tarun Kanti; linux-
> > omap@vger.kernel.org; Shilimkar, Santosh; Paul Walmsley; Tony Lindgren
> > Subject: Re: [PATCHv3 8/17] dmtimer: register mappings moved to hwmod
> > database
> >
> > On 9/23/2010 11:34 AM, Basak, Partha wrote:
> > >
> > >
> > >> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
> > >> Sent: Thursday, September 23, 2010 3:10 AM
> > >>
> > >> "G, Manjunath Kondaiah"<manjugk@ti.com>  writes:
> > >>
> > >>> Hi Tarun,
> > >>>
> > >>>> From: linux-omap-owner@vger.kernel.org
> > >>>> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of
> > >>>> DebBarma, Tarun Kanti
> > >>>>
> >
> > <...>
> >
> > >>>> +static u32 omap_timer_reg_map_v1[] = {
> > >>>> +  [OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 |
> > >>>> (WP_TOCR<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 |
> > >>>> (WP_TOWR<<  WPSHIFT)),
> > >>>> +};
> > >>>> +
> > >>>> +/* OMAP4 timers register map */
> > >>>> +static u32 omap_timer_reg_map_v2[] = {
> > >>>> +  [OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_STAT_REG]           = (0x28 | (WP_NONE<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_INT_EN_REG]         = (0x2c | (WP_NONE<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_WAKEUP_EN_REG]      = (0x34 | (WP_NONE<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_CTRL_REG]           = (0x38 | (WP_TCLR<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_COUNTER_REG]        = (0x3c | (WP_TCRR<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_LOAD_REG]           = (0x40 | (WP_TLDR<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_TRIGGER_REG]        = (0x44 | (WP_TTGR<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_WRITE_PEND_REG]     = (0x48 | (WP_NONE<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_MATCH_REG]          = (0x4c | (WP_TMAR<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_CAPTURE_REG]        = (0x50 | (WP_NONE<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_IF_CTRL_REG]        = (0x54 | (WP_NONE<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_CAPTURE2_REG]       = (0x58 | (WP_NONE<<  WPSHIFT)),
> > >>>> +  [OMAP_TIMER_INT_CLR_REG]        = (0x30 | (WP_NONE<<  WPSHIFT)),
> > >>>> +};
> > >>>> +
> > >>>
> > >>> Why not these defines in mach-omap2/dmtimer.c? since
> > >> register offsets are
> > >>> same for omap2plus except omap4 which has additional
> > >> register offsets. Same
> > >>> register offsets are getting repeated in all the omap2plus
> > >> hwmod database.
> > >>
> > >> I agree with Manjunath.
> > >
> > > Manju, the number of tables needed to manage the information are same
> > really.
> > > Its only where they are located changes from the mach layer to the
> hwmod
> > > database. So, thats not the point. dev_attrs are pointing to the reg-
> map
> > > tables, they are not duplicating them.
> > >
> > >
> > > The offset differences are stemming from the IP differences.
> > > To my understanding, only IP-integration specific idiosyncrasies into
> a
> > given
> > > SOC qualifiy as dev_attrs.
> > > IP specific details like reg-map information should be maintained
> within
> > the driver.
> > > So, this approach will break this paradigm&  also not follow existing
> > implementation
> > > of drivers which support this. A given driver may support a set of IPs
> > but still
> > > may cater to even non-OMAP platforms not supporting hwmod.
> > >
> > > However, if we see the general usage of dev_attrs, there is no clear
> > line of demarcation
> > > really what should make it and what should not, as is used to plug
> this
> > sort of
> > > small gotchas and reduce CPU checks in the code.
> >
> > You do not really need that to reduce the CPU check in the code.
> > In that case, only the IP version should be stored in the dev_attr.
> > Based on that IP version, you know exactly what register map you have to
> > handle. For the moment you just have 2 layouts to handle + the extra
> > register for the 1ms timers.
> >
> > Also, I'm not sure that you have to handle each register separately
> > considering that you have one offset to handle for the functional
> > register.
> > The change in sysconfig and interrupt register are all standard mapping
> > that stick to TI highlander standard.
> > Meaning, as soon as an IP is a v2 (highlander version) all these
> > registers will be at the same place... at least in theory :-)
> >
> > Here are the deltas between the legacy and the new one:
> >
> > [OMAP_TIMER_ID_REG]             0x00,
> > [OMAP_TIMER_OCP_CFG_REG]        0x10, same
> > [OMAP_TIMER_SYS_STAT_REG]       0x14, (not used anymore)
> >
> > You should not care about these ones, because hwmod will handle them.
> >
> > [OMAP_TIMER_STAT_REG]           0x28, +10
> > [OMAP_TIMER_INT_EN_REG]         0x2c, +10
> > [OMAP_TIMER_INT_CLR_REG]        0x30, (new)
> >
> > These ones are standard registers
> >
> > [OMAP_TIMER_WAKEUP_EN_REG]      0x34, +14
> > [OMAP_TIMER_CTRL_REG]           0x38, +14
> > [OMAP_TIMER_COUNTER_REG]        0x3c, +14
> > [OMAP_TIMER_LOAD_REG]           0x40, +14
> > [OMAP_TIMER_TRIGGER_REG]        0x44, +14
> > [OMAP_TIMER_WRITE_PEND_REG]     0x48, +14
> > [OMAP_TIMER_MATCH_REG]          0x4c, +14
> > [OMAP_TIMER_CAPTURE_REG]        0x50, +14
> > [OMAP_TIMER_IF_CTRL_REG]        0x54, +14
> > [OMAP_TIMER_CAPTURE2_REG]       0x58, +14
> >
> > You can probably handle that with only 2 offsets instead of having one
> > address per registers.
> >
> To keep aligned with other driver implementations, I would like to follow
> this:
> 
> 1) move the table in mach-omap1/2 since
> 2) remove entries which are handled by hwmod.
> However, I am not sure if there is any issue in keeping them in the table.
Hmm... I have second thought on this implementation and I need your inputs.
If I go with the above implementation then I would end up having duplicate tables for mach-omap1 and mach-omap2. I have looked at the I2C implementation and would like to follow this implementation where they have two tables in plat:
One for OMAP1/2/3 and second for OMAP4.

-tarun

Re: [PATCHv3 8/17] dmtimer: register mappings moved to hwmod database

[Cousson, Benoit]

Hi Tarun,

On 10/9/2010 5:40 PM, DebBarma, Tarun Kanti wrote:
> Benoit,
>
>> From: Cousson, Benoit
>> Sent: Thursday, September 23, 2010 10:15 PM
>> To: Basak, Partha
>> Cc: Kevin Hilman; G, Manjunath Kondaiah; DebBarma, Tarun Kanti; linux-
>> omap@vger.kernel.org; Shilimkar, Santosh; Paul Walmsley; Tony Lindgren
>> Subject: Re: [PATCHv3 8/17] dmtimer: register mappings moved to hwmod
>> database
>>
>> On 9/23/2010 11:34 AM, Basak, Partha wrote:
>>>
>>>
>>>> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>> Sent: Thursday, September 23, 2010 3:10 AM
>>>>
>>>> "G, Manjunath Kondaiah"<manjugk@ti.com>   writes:
>>>>
>>>>> Hi Tarun,
>>>>>
>>>>>> From: linux-omap-owner@vger.kernel.org
>>>>>> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of
>>>>>> DebBarma, Tarun Kanti
>>>>>>
>>
>> <...>
>>
>>>>>> +static u32 omap_timer_reg_map_v1[] = {
>>>>>> +  [OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 |
>>>>>> (WP_TOCR<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 |
>>>>>> (WP_TOWR<<   WPSHIFT)),
>>>>>> +};
>>>>>> +
>>>>>> +/* OMAP4 timers register map */
>>>>>> +static u32 omap_timer_reg_map_v2[] = {
>>>>>> +  [OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_STAT_REG]           = (0x28 | (WP_NONE<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_INT_EN_REG]         = (0x2c | (WP_NONE<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_WAKEUP_EN_REG]      = (0x34 | (WP_NONE<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_CTRL_REG]           = (0x38 | (WP_TCLR<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_COUNTER_REG]        = (0x3c | (WP_TCRR<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_LOAD_REG]           = (0x40 | (WP_TLDR<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_TRIGGER_REG]        = (0x44 | (WP_TTGR<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_WRITE_PEND_REG]     = (0x48 | (WP_NONE<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_MATCH_REG]          = (0x4c | (WP_TMAR<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_CAPTURE_REG]        = (0x50 | (WP_NONE<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_IF_CTRL_REG]        = (0x54 | (WP_NONE<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_CAPTURE2_REG]       = (0x58 | (WP_NONE<<   WPSHIFT)),
>>>>>> +  [OMAP_TIMER_INT_CLR_REG]        = (0x30 | (WP_NONE<<   WPSHIFT)),
>>>>>> +};
>>>>>> +
>>>>>
>>>>> Why not these defines in mach-omap2/dmtimer.c? since
>>>> register offsets are
>>>>> same for omap2plus except omap4 which has additional
>>>> register offsets. Same
>>>>> register offsets are getting repeated in all the omap2plus
>>>> hwmod database.
>>>>
>>>> I agree with Manjunath.
>>>
>>> Manju, the number of tables needed to manage the information are same
>> really.
>>> Its only where they are located changes from the mach layer to the hwmod
>>> database. So, thats not the point. dev_attrs are pointing to the reg-map
>>> tables, they are not duplicating them.
>>>
>>>
>>> The offset differences are stemming from the IP differences.
>>> To my understanding, only IP-integration specific idiosyncrasies into a
>> given
>>> SOC qualifiy as dev_attrs.
>>> IP specific details like reg-map information should be maintained within
>> the driver.
>>> So, this approach will break this paradigm&   also not follow existing
>> implementation
>>> of drivers which support this. A given driver may support a set of IPs
>> but still
>>> may cater to even non-OMAP platforms not supporting hwmod.
>>>
>>> However, if we see the general usage of dev_attrs, there is no clear
>> line of demarcation
>>> really what should make it and what should not, as is used to plug this
>> sort of
>>> small gotchas and reduce CPU checks in the code.
>>
>> You do not really need that to reduce the CPU check in the code.
>> In that case, only the IP version should be stored in the dev_attr.
>> Based on that IP version, you know exactly what register map you have to
>> handle. For the moment you just have 2 layouts to handle + the extra
>> register for the 1ms timers.
>>
>> Also, I'm not sure that you have to handle each register separately
>> considering that you have one offset to handle for the functional
>> register.
>> The change in sysconfig and interrupt register are all standard mapping
>> that stick to TI highlander standard.
>> Meaning, as soon as an IP is a v2 (highlander version) all these
>> registers will be at the same place... at least in theory :-)
>>
>> Here are the deltas between the legacy and the new one:
>>
>> [OMAP_TIMER_ID_REG]             0x00,
>> [OMAP_TIMER_OCP_CFG_REG]        0x10, same
>> [OMAP_TIMER_SYS_STAT_REG]       0x14, (not used anymore)
>>
>> You should not care about these ones, because hwmod will handle them.
>>
>> [OMAP_TIMER_STAT_REG]           0x28, +10
>> [OMAP_TIMER_INT_EN_REG]         0x2c, +10
>> [OMAP_TIMER_INT_CLR_REG]        0x30, (new)
>>
>> These ones are standard registers
>>
>> [OMAP_TIMER_WAKEUP_EN_REG]      0x34, +14
>> [OMAP_TIMER_CTRL_REG]           0x38, +14
>> [OMAP_TIMER_COUNTER_REG]        0x3c, +14
>> [OMAP_TIMER_LOAD_REG]           0x40, +14
>> [OMAP_TIMER_TRIGGER_REG]        0x44, +14
>> [OMAP_TIMER_WRITE_PEND_REG]     0x48, +14
>> [OMAP_TIMER_MATCH_REG]          0x4c, +14
>> [OMAP_TIMER_CAPTURE_REG]        0x50, +14
>> [OMAP_TIMER_IF_CTRL_REG]        0x54, +14
>> [OMAP_TIMER_CAPTURE2_REG]       0x58, +14
>>
>> You can probably handle that with only 2 offsets instead of having one
>> address per registers.
>>
> To keep aligned with other driver implementations, I would like to follow this:
>
> 1) move the table in mach-omap1/2 since
> 2) remove entries which are handled by hwmod.
> However, I am not sure if there is any issue in keeping them in the table.

I don't think you need a table for that. All the functional registers 
are using a constant offset. IRQ is then the only exception.

Benoit

RE: [PATCHv3 8/17] dmtimer: register mappings moved to hwmod database

[DebBarma, Tarun Kanti]

Benoit,
> -----Original Message-----
> From: Cousson, Benoit
> Sent: Tuesday, October 12, 2010 4:42 AM
> To: DebBarma, Tarun Kanti
> Cc: Basak, Partha; Kevin Hilman; G, Manjunath Kondaiah; linux-
> omap@vger.kernel.org; Shilimkar, Santosh; Paul Walmsley; Tony Lindgren
> Subject: Re: [PATCHv3 8/17] dmtimer: register mappings moved to hwmod
> database
> 
> Hi Tarun,
> 
> On 10/9/2010 5:40 PM, DebBarma, Tarun Kanti wrote:
> > Benoit,
> >
> >> From: Cousson, Benoit
> >> Sent: Thursday, September 23, 2010 10:15 PM
> >> To: Basak, Partha
> >> Cc: Kevin Hilman; G, Manjunath Kondaiah; DebBarma, Tarun Kanti; linux-
> >> omap@vger.kernel.org; Shilimkar, Santosh; Paul Walmsley; Tony Lindgren
> >> Subject: Re: [PATCHv3 8/17] dmtimer: register mappings moved to hwmod
> >> database
> >>
> >> On 9/23/2010 11:34 AM, Basak, Partha wrote:
> >>>
> >>>
> >>>> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
> >>>> Sent: Thursday, September 23, 2010 3:10 AM
> >>>>
> >>>> "G, Manjunath Kondaiah"<manjugk@ti.com>   writes:
> >>>>
> >>>>> Hi Tarun,
> >>>>>
> >>>>>> From: linux-omap-owner@vger.kernel.org
> >>>>>> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of
> >>>>>> DebBarma, Tarun Kanti
> >>>>>>
> >>
> >> <...>
> >>
> >>>>>> +static u32 omap_timer_reg_map_v1[] = {
> >>>>>> +  [OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 |
> >>>>>> (WP_TOCR<<   WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 |
> >>>>>> (WP_TOWR<<   WPSHIFT)),
> >>>>>> +};
> >>>>>> +
> >>>>>> +/* OMAP4 timers register map */
> >>>>>> +static u32 omap_timer_reg_map_v2[] = {
> >>>>>> +  [OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_STAT_REG]           = (0x28 | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_INT_EN_REG]         = (0x2c | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_WAKEUP_EN_REG]      = (0x34 | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_CTRL_REG]           = (0x38 | (WP_TCLR<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_COUNTER_REG]        = (0x3c | (WP_TCRR<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_LOAD_REG]           = (0x40 | (WP_TLDR<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_TRIGGER_REG]        = (0x44 | (WP_TTGR<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_WRITE_PEND_REG]     = (0x48 | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_MATCH_REG]          = (0x4c | (WP_TMAR<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_CAPTURE_REG]        = (0x50 | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_IF_CTRL_REG]        = (0x54 | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_CAPTURE2_REG]       = (0x58 | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +  [OMAP_TIMER_INT_CLR_REG]        = (0x30 | (WP_NONE<<
> WPSHIFT)),
> >>>>>> +};
> >>>>>> +
> >>>>>
> >>>>> Why not these defines in mach-omap2/dmtimer.c? since
> >>>> register offsets are
> >>>>> same for omap2plus except omap4 which has additional
> >>>> register offsets. Same
> >>>>> register offsets are getting repeated in all the omap2plus
> >>>> hwmod database.
> >>>>
> >>>> I agree with Manjunath.
> >>>
> >>> Manju, the number of tables needed to manage the information are same
> >> really.
> >>> Its only where they are located changes from the mach layer to the
> hwmod
> >>> database. So, thats not the point. dev_attrs are pointing to the reg-
> map
> >>> tables, they are not duplicating them.
> >>>
> >>>
> >>> The offset differences are stemming from the IP differences.
> >>> To my understanding, only IP-integration specific idiosyncrasies into
> a
> >> given
> >>> SOC qualifiy as dev_attrs.
> >>> IP specific details like reg-map information should be maintained
> within
> >> the driver.
> >>> So, this approach will break this paradigm&   also not follow existing
> >> implementation
> >>> of drivers which support this. A given driver may support a set of IPs
> >> but still
> >>> may cater to even non-OMAP platforms not supporting hwmod.
> >>>
> >>> However, if we see the general usage of dev_attrs, there is no clear
> >> line of demarcation
> >>> really what should make it and what should not, as is used to plug
> this
> >> sort of
> >>> small gotchas and reduce CPU checks in the code.
> >>
> >> You do not really need that to reduce the CPU check in the code.
> >> In that case, only the IP version should be stored in the dev_attr.
> >> Based on that IP version, you know exactly what register map you have
> to
> >> handle. For the moment you just have 2 layouts to handle + the extra
> >> register for the 1ms timers.
> >>
> >> Also, I'm not sure that you have to handle each register separately
> >> considering that you have one offset to handle for the functional
> >> register.
> >> The change in sysconfig and interrupt register are all standard mapping
> >> that stick to TI highlander standard.
> >> Meaning, as soon as an IP is a v2 (highlander version) all these
> >> registers will be at the same place... at least in theory :-)
> >>
> >> Here are the deltas between the legacy and the new one:
> >>
> >> [OMAP_TIMER_ID_REG]             0x00,
> >> [OMAP_TIMER_OCP_CFG_REG]        0x10, same
> >> [OMAP_TIMER_SYS_STAT_REG]       0x14, (not used anymore)
> >>
> >> You should not care about these ones, because hwmod will handle them.
> >>
> >> [OMAP_TIMER_STAT_REG]           0x28, +10
> >> [OMAP_TIMER_INT_EN_REG]         0x2c, +10
> >> [OMAP_TIMER_INT_CLR_REG]        0x30, (new)
> >>
> >> These ones are standard registers
> >>
> >> [OMAP_TIMER_WAKEUP_EN_REG]      0x34, +14
> >> [OMAP_TIMER_CTRL_REG]           0x38, +14
> >> [OMAP_TIMER_COUNTER_REG]        0x3c, +14
> >> [OMAP_TIMER_LOAD_REG]           0x40, +14
> >> [OMAP_TIMER_TRIGGER_REG]        0x44, +14
> >> [OMAP_TIMER_WRITE_PEND_REG]     0x48, +14
> >> [OMAP_TIMER_MATCH_REG]          0x4c, +14
> >> [OMAP_TIMER_CAPTURE_REG]        0x50, +14
> >> [OMAP_TIMER_IF_CTRL_REG]        0x54, +14
> >> [OMAP_TIMER_CAPTURE2_REG]       0x58, +14
> >>
> >> You can probably handle that with only 2 offsets instead of having one
> >> address per registers.
> >>
> > To keep aligned with other driver implementations, I would like to
> follow this:
> >
> > 1) move the table in mach-omap1/2 since
> > 2) remove entries which are handled by hwmod.
> > However, I am not sure if there is any issue in keeping them in the
> table.
> 
> I don't think you need a table for that. All the functional registers
> are using a constant offset. IRQ is then the only exception.
Agreed!! But we have to have a check in the read/write routine to add this offset. This is an overhead I thought. Also, tomorrow when we have new silicon with further offset difference we would have to keep on adding additional checks. This would end up making the read/write routine (which is called frequently) bulky and inefficient.
So, my thinking was to keep the read/write routine generic. Also, keeping separate table makes it extensible easily.
At the end I am OK both ways so long as community feels ok with whichever approach.
-tarun

RE: [PATCHv3 8/17] dmtimer: register mappings moved to hwmod database

[G, Manjunath Kondaiah]

> -----Original Message-----
> From: DebBarma, Tarun Kanti 
> Sent: Monday, October 11, 2010 12:38 PM
> To: Cousson, Benoit
> Cc: Kevin Hilman; G, Manjunath Kondaiah; 
> linux-omap@vger.kernel.org; Shilimkar, Santosh; Paul 
> Walmsley; Tony Lindgren; Basak, Partha
> Subject: RE: [PATCHv3 8/17] dmtimer: register mappings moved 
> to hwmod database
> 
> Benoit and Manju,
[...]
> > >
> > > You do not really need that to reduce the CPU check in the code.
> > > In that case, only the IP version should be stored in the 
> dev_attr.
> > > Based on that IP version, you know exactly what register map you 
> > > have to handle. For the moment you just have 2 layouts to 
> handle + 
> > > the extra register for the 1ms timers.
> > >
> > > Also, I'm not sure that you have to handle each register 
> separately 
> > > considering that you have one offset to handle for the functional 
> > > register.
> > > The change in sysconfig and interrupt register are all standard 
> > > mapping that stick to TI highlander standard.
> > > Meaning, as soon as an IP is a v2 (highlander version) all these 
> > > registers will be at the same place... at least in theory :-)
> > >
> > > Here are the deltas between the legacy and the new one:
> > >
> > > [OMAP_TIMER_ID_REG]             0x00,
> > > [OMAP_TIMER_OCP_CFG_REG]        0x10, same
> > > [OMAP_TIMER_SYS_STAT_REG]       0x14, (not used anymore)
> > >
> > > You should not care about these ones, because hwmod will 
> handle them.
> > >
> > > [OMAP_TIMER_STAT_REG]           0x28, +10
> > > [OMAP_TIMER_INT_EN_REG]         0x2c, +10
> > > [OMAP_TIMER_INT_CLR_REG]        0x30, (new)
> > >
> > > These ones are standard registers
> > >
> > > [OMAP_TIMER_WAKEUP_EN_REG]      0x34, +14
> > > [OMAP_TIMER_CTRL_REG]           0x38, +14
> > > [OMAP_TIMER_COUNTER_REG]        0x3c, +14
> > > [OMAP_TIMER_LOAD_REG]           0x40, +14
> > > [OMAP_TIMER_TRIGGER_REG]        0x44, +14
> > > [OMAP_TIMER_WRITE_PEND_REG]     0x48, +14
> > > [OMAP_TIMER_MATCH_REG]          0x4c, +14
> > > [OMAP_TIMER_CAPTURE_REG]        0x50, +14
> > > [OMAP_TIMER_IF_CTRL_REG]        0x54, +14
> > > [OMAP_TIMER_CAPTURE2_REG]       0x58, +14
> > >
> > > You can probably handle that with only 2 offsets instead 
> of having 
> > > one address per registers.
> > >
> > To keep aligned with other driver implementations, I would like to 
> > follow
> > this:
> > 
> > 1) move the table in mach-omap1/2 since
> > 2) remove entries which are handled by hwmod.
> > However, I am not sure if there is any issue in keeping 
> them in the table.
> Hmm... I have second thought on this implementation and I 
> need your inputs.
> If I go with the above implementation then I would end up 
> having duplicate tables for mach-omap1 and mach-omap2. I have 
> looked at the I2C implementation and would like to follow 
> this implementation where they have two tables in plat:
> One for OMAP1/2/3 and second for OMAP4.

1. register offset tables are in i2c-omap.c under drivers/i2c/busses
and not in plat.

2. i2c is different type of driver where register offsets are required
only in i2c-omap.c and not in plat. If you don't require register offsets
in mach-omapx or other places then it doesnot matter where you define 
these tables. Otherwise, it has to be in mach-omap2 since it should feed
omap_device_build so that, offsets can be accessed at other places.

-Manjunath

RE: [PATCHv3 8/17] dmtimer: register mappings moved to hwmod database

[Basak, Partha]

 

> -----Original Message-----
> From: DebBarma, Tarun Kanti 
> Sent: Tuesday, October 12, 2010 11:19 AM
> To: Cousson, Benoit
> Cc: Basak, Partha; Kevin Hilman; G, Manjunath Kondaiah; 
> linux-omap@vger.kernel.org; Shilimkar, Santosh; Paul 
> Walmsley; Tony Lindgren
> Subject: RE: [PATCHv3 8/17] dmtimer: register mappings moved 
> to hwmod database
> 
> Benoit,
> > -----Original Message-----
> > From: Cousson, Benoit
> > Sent: Tuesday, October 12, 2010 4:42 AM
> > To: DebBarma, Tarun Kanti
> > Cc: Basak, Partha; Kevin Hilman; G, Manjunath Kondaiah; linux-
> > omap@vger.kernel.org; Shilimkar, Santosh; Paul Walmsley; 
> Tony Lindgren
> > Subject: Re: [PATCHv3 8/17] dmtimer: register mappings 
> moved to hwmod
> > database
> > 
> > Hi Tarun,
> > 
> > On 10/9/2010 5:40 PM, DebBarma, Tarun Kanti wrote:
> > > Benoit,
> > >
> > >> From: Cousson, Benoit
> > >> Sent: Thursday, September 23, 2010 10:15 PM
> > >> To: Basak, Partha
> > >> Cc: Kevin Hilman; G, Manjunath Kondaiah; DebBarma, Tarun 
> Kanti; linux-
> > >> omap@vger.kernel.org; Shilimkar, Santosh; Paul Walmsley; 
> Tony Lindgren
> > >> Subject: Re: [PATCHv3 8/17] dmtimer: register mappings 
> moved to hwmod
> > >> database
> > >>
> > >> On 9/23/2010 11:34 AM, Basak, Partha wrote:
> > >>>
> > >>>
> > >>>> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
> > >>>> Sent: Thursday, September 23, 2010 3:10 AM
> > >>>>
> > >>>> "G, Manjunath Kondaiah"<manjugk@ti.com>   writes:
> > >>>>
> > >>>>> Hi Tarun,
> > >>>>>
> > >>>>>> From: linux-omap-owner@vger.kernel.org
> > >>>>>> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of
> > >>>>>> DebBarma, Tarun Kanti
> > >>>>>>
> > >>
> > >> <...>
> > >>
> > >>>>>> +static u32 omap_timer_reg_map_v1[] = {
> > >>>>>> +  [OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_STAT_REG]           = (0x18 | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_INT_EN_REG]         = (0x1c | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_WAKEUP_EN_REG]      = (0x20 | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_CTRL_REG]           = (0x24 | (WP_TCLR<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_COUNTER_REG]        = (0x28 | (WP_TCRR<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_LOAD_REG]           = (0x2c | (WP_TLDR<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_TRIGGER_REG]        = (0x30 | (WP_TTGR<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_WRITE_PEND_REG]     = (0x34 | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_MATCH_REG]          = (0x38 | (WP_TMAR<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_CAPTURE_REG]        = (0x3c | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_IF_CTRL_REG]        = (0x40 | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_CAPTURE2_REG]       = (0x44 | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_TICK_POS_REG]       = (0x48 | (WP_TPIR<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_TICK_NEG_REG]       = (0x4c | (WP_TNIR<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_TICK_COUNT_REG]     = (0x50 | (WP_TCVR<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_TICK_INT_MASK_SET_REG]      = (0x54 |
> > >>>>>> (WP_TOCR<<   WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_TICK_INT_MASK_COUNT_REG]    = (0x58 |
> > >>>>>> (WP_TOWR<<   WPSHIFT)),
> > >>>>>> +};
> > >>>>>> +
> > >>>>>> +/* OMAP4 timers register map */
> > >>>>>> +static u32 omap_timer_reg_map_v2[] = {
> > >>>>>> +  [OMAP_TIMER_ID_REG]             = (0x00 | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_OCP_CFG_REG]        = (0x10 | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_SYS_STAT_REG]       = (0x14 | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_STAT_REG]           = (0x28 | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_INT_EN_REG]         = (0x2c | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_WAKEUP_EN_REG]      = (0x34 | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_CTRL_REG]           = (0x38 | (WP_TCLR<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_COUNTER_REG]        = (0x3c | (WP_TCRR<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_LOAD_REG]           = (0x40 | (WP_TLDR<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_TRIGGER_REG]        = (0x44 | (WP_TTGR<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_WRITE_PEND_REG]     = (0x48 | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_MATCH_REG]          = (0x4c | (WP_TMAR<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_CAPTURE_REG]        = (0x50 | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_IF_CTRL_REG]        = (0x54 | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_CAPTURE2_REG]       = (0x58 | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +  [OMAP_TIMER_INT_CLR_REG]        = (0x30 | (WP_NONE<<
> > WPSHIFT)),
> > >>>>>> +};
> > >>>>>> +
> > >>>>>
> > >>>>> Why not these defines in mach-omap2/dmtimer.c? since
> > >>>> register offsets are
> > >>>>> same for omap2plus except omap4 which has additional
> > >>>> register offsets. Same
> > >>>>> register offsets are getting repeated in all the omap2plus
> > >>>> hwmod database.
> > >>>>
> > >>>> I agree with Manjunath.
> > >>>
> > >>> Manju, the number of tables needed to manage the 
> information are same
> > >> really.
> > >>> Its only where they are located changes from the mach 
> layer to the
> > hwmod
> > >>> database. So, thats not the point. dev_attrs are 
> pointing to the reg-
> > map
> > >>> tables, they are not duplicating them.
> > >>>
> > >>>
> > >>> The offset differences are stemming from the IP differences.
> > >>> To my understanding, only IP-integration specific 
> idiosyncrasies into
> > a
> > >> given
> > >>> SOC qualifiy as dev_attrs.
> > >>> IP specific details like reg-map information should be 
> maintained
> > within
> > >> the driver.
> > >>> So, this approach will break this paradigm&   also not 
> follow existing
> > >> implementation
> > >>> of drivers which support this. A given driver may 
> support a set of IPs
> > >> but still
> > >>> may cater to even non-OMAP platforms not supporting hwmod.
> > >>>
> > >>> However, if we see the general usage of dev_attrs, 
> there is no clear
> > >> line of demarcation
> > >>> really what should make it and what should not, as is 
> used to plug
> > this
> > >> sort of
> > >>> small gotchas and reduce CPU checks in the code.
> > >>
> > >> You do not really need that to reduce the CPU check in the code.
> > >> In that case, only the IP version should be stored in 
> the dev_attr.
> > >> Based on that IP version, you know exactly what register 
> map you have
> > to
> > >> handle. For the moment you just have 2 layouts to handle 
> + the extra
> > >> register for the 1ms timers.
> > >>
> > >> Also, I'm not sure that you have to handle each register 
> separately
> > >> considering that you have one offset to handle for the functional
> > >> register.
> > >> The change in sysconfig and interrupt register are all 
> standard mapping
> > >> that stick to TI highlander standard.
> > >> Meaning, as soon as an IP is a v2 (highlander version) all these
> > >> registers will be at the same place... at least in theory :-)
> > >>
> > >> Here are the deltas between the legacy and the new one:
> > >>
> > >> [OMAP_TIMER_ID_REG]             0x00,
> > >> [OMAP_TIMER_OCP_CFG_REG]        0x10, same
> > >> [OMAP_TIMER_SYS_STAT_REG]       0x14, (not used anymore)
> > >>
> > >> You should not care about these ones, because hwmod will 
> handle them.
> > >>
> > >> [OMAP_TIMER_STAT_REG]           0x28, +10
> > >> [OMAP_TIMER_INT_EN_REG]         0x2c, +10
> > >> [OMAP_TIMER_INT_CLR_REG]        0x30, (new)
> > >>
> > >> These ones are standard registers
> > >>
> > >> [OMAP_TIMER_WAKEUP_EN_REG]      0x34, +14
> > >> [OMAP_TIMER_CTRL_REG]           0x38, +14
> > >> [OMAP_TIMER_COUNTER_REG]        0x3c, +14
> > >> [OMAP_TIMER_LOAD_REG]           0x40, +14
> > >> [OMAP_TIMER_TRIGGER_REG]        0x44, +14
> > >> [OMAP_TIMER_WRITE_PEND_REG]     0x48, +14
> > >> [OMAP_TIMER_MATCH_REG]          0x4c, +14
> > >> [OMAP_TIMER_CAPTURE_REG]        0x50, +14
> > >> [OMAP_TIMER_IF_CTRL_REG]        0x54, +14
> > >> [OMAP_TIMER_CAPTURE2_REG]       0x58, +14
> > >>
> > >> You can probably handle that with only 2 offsets instead 
> of having one
> > >> address per registers.
> > >>
> > > To keep aligned with other driver implementations, I would like to
> > follow this:
> > >
> > > 1) move the table in mach-omap1/2 since
> > > 2) remove entries which are handled by hwmod.
> > > However, I am not sure if there is any issue in keeping 
> them in the
> > table.
> > 
> > I don't think you need a table for that. All the functional 
> registers
> > are using a constant offset. IRQ is then the only exception.

Other than the offsets 0x14 for the Timer functional registers
and 0x10 for the IRQ registers, following differences exist
between the two versions:
1. OMAP_TIMER_SYS_STAT_REG is used only for v1, missing for v2.
2. OMAP_TIMER_INT_CLR_REG is new for v2, not used for v1.
3. Following registers are applicable only for v1.
[OMAP_TIMER_TICK_POS_REG]     		0x48
[OMAP_TIMER_TICK_NEG_REG]     		0x4c
[OMAP_TIMER_TICK_COUNT_REG]     		0x50
[OMAP_TIMER_TICK_INT_MASK_SET_REG]		0x54
[OMAP_TIMER_TICK_INT_MASK_COUNT_REG]	0x58

In the end, having separate tables is neater and consistent
with other drivers. But instead of duplicating for each mach,
keeping them in plat should be OK.

Additionally, the implementation should take care to prevent access
of non-existing registers for a particular version.

> Agreed!! But we have to have a check in the read/write 
> routine to add this offset. This is an overhead I thought. 
> Also, tomorrow when we have new silicon with further offset 
> difference we would have to keep on adding additional checks. 
> This would end up making the read/write routine (which is 
> called frequently) bulky and inefficient.
> So, my thinking was to keep the read/write routine generic. 
> Also, keeping separate table makes it extensible easily.
> At the end I am OK both ways so long as community feels ok 
> with whichever approach.
> -tarun
> 

Re: [PATCHv3 8/17] dmtimer: register mappings moved to hwmod database

[Cousson, Benoit]

On 10/12/2010 9:22 AM, Basak, Partha wrote:
>
>> From: DebBarma, Tarun Kanti
>> Sent: Tuesday, October 12, 2010 11:19 AM
>> To: Cousson, Benoit
>>
>> Benoit,
>>> From: Cousson, Benoit
>>> Sent: Tuesday, October 12, 2010 4:42 AM
>>> To: DebBarma, Tarun Kanti
>>>
>>> Hi Tarun,
>>>
>>> On 10/9/2010 5:40 PM, DebBarma, Tarun Kanti wrote:
>>>> Benoit,
>>>>
>>>>> From: Cousson, Benoit
>>>>> Sent: Thursday, September 23, 2010 10:15 PM
>>>>> To: Basak, Partha
>>>>>
>>>>> On 9/23/2010 11:34 AM, Basak, Partha wrote:
>>>>>>
>>>>>>> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>>>>> Sent: Thursday, September 23, 2010 3:10 AM
>>>>>>>
>>>>>>> "G, Manjunath Kondaiah"<manjugk@ti.com>    writes:
>>>>>>>
>>>>>>>> Hi Tarun,
>>>>>>>>
>>>>>>>>> From: linux-omap-owner@vger.kernel.org
>>>>>>>>> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of
>>>>>>>>> DebBarma, Tarun Kanti

<...>

>>>>>
>>>>> Also, I'm not sure that you have to handle each register
>> separately
>>>>> considering that you have one offset to handle for the functional
>>>>> register.
>>>>> The change in sysconfig and interrupt register are all
>> standard mapping
>>>>> that stick to TI highlander standard.
>>>>> Meaning, as soon as an IP is a v2 (highlander version) all these
>>>>> registers will be at the same place... at least in theory :-)
>>>>>
>>>>> Here are the deltas between the legacy and the new one:
>>>>>
>>>>> [OMAP_TIMER_ID_REG]             0x00,
>>>>> [OMAP_TIMER_OCP_CFG_REG]        0x10, same
>>>>> [OMAP_TIMER_SYS_STAT_REG]       0x14, (not used anymore)
>>>>>
>>>>> You should not care about these ones, because hwmod will
>> handle them.
>>>>>
>>>>> [OMAP_TIMER_STAT_REG]           0x28, +10
>>>>> [OMAP_TIMER_INT_EN_REG]         0x2c, +10
>>>>> [OMAP_TIMER_INT_CLR_REG]        0x30, (new)
>>>>>
>>>>> These ones are standard registers
>>>>>
>>>>> [OMAP_TIMER_WAKEUP_EN_REG]      0x34, +14
>>>>> [OMAP_TIMER_CTRL_REG]           0x38, +14
>>>>> [OMAP_TIMER_COUNTER_REG]        0x3c, +14
>>>>> [OMAP_TIMER_LOAD_REG]           0x40, +14
>>>>> [OMAP_TIMER_TRIGGER_REG]        0x44, +14
>>>>> [OMAP_TIMER_WRITE_PEND_REG]     0x48, +14
>>>>> [OMAP_TIMER_MATCH_REG]          0x4c, +14
>>>>> [OMAP_TIMER_CAPTURE_REG]        0x50, +14
>>>>> [OMAP_TIMER_IF_CTRL_REG]        0x54, +14
>>>>> [OMAP_TIMER_CAPTURE2_REG]       0x58, +14
>>>>>
>>>>> You can probably handle that with only 2 offsets instead
>> of having one
>>>>> address per registers.
>>>>>
>>>> To keep aligned with other driver implementations, I would like to
>>> follow this:
>>>>
>>>> 1) move the table in mach-omap1/2 since
>>>> 2) remove entries which are handled by hwmod.
>>>> However, I am not sure if there is any issue in keeping
>> them in the
>>> table.
>>>
>>> I don't think you need a table for that. All the functional
>> registers
>>> are using a constant offset. IRQ is then the only exception.
>
> Other than the offsets 0x14 for the Timer functional registers
> and 0x10 for the IRQ registers, following differences exist
> between the two versions:
> 1. OMAP_TIMER_SYS_STAT_REG is used only for v1, missing for v2.

This is handled by hwmod anyway.


> 2. OMAP_TIMER_INT_CLR_REG is new for v2, not used for v1.

Follow regular IRQ management for highlander IP. Should be some generic 
code in order to allow reuse.

> 3. Following registers are applicable only for v1.
> [OMAP_TIMER_TICK_POS_REG]     		0x48
> [OMAP_TIMER_TICK_NEG_REG]     		0x4c
> [OMAP_TIMER_TICK_COUNT_REG]     		0x50
> [OMAP_TIMER_TICK_INT_MASK_SET_REG]		0x54
> [OMAP_TIMER_TICK_INT_MASK_COUNT_REG]	0x58

These are only there in the 1ms version of this IP.

> In the end, having separate tables is neater and consistent
> with other drivers.

I don't see why? And what other driver are you referring too?

What the point of adding a table and an extra level of indirection, when 
a simple addition will do it?

> But instead of duplicating for each mach,
> keeping them in plat should be OK.
>
> Additionally, the implementation should take care to prevent access
> of non-existing registers for a particular version.

You just need the IP version to do that.

We have 3 timer variants to handle today:
Legacy timer, legacy 1ms timer and highlander timer. You can handle that 
with 2 flags and 2 offsets.

Benoit

>
>> Agreed!! But we have to have a check in the read/write
>> routine to add this offset. This is an overhead I thought.
>> Also, tomorrow when we have new silicon with further offset
>> difference we would have to keep on adding additional checks.
>> This would end up making the read/write routine (which is
>> called frequently) bulky and inefficient.
>> So, my thinking was to keep the read/write routine generic.
>> Also, keeping separate table makes it extensible easily.
>> At the end I am OK both ways so long as community feels ok
>> with whichever approach.
>> -tarun

RE: [PATCHv3 8/17] dmtimer: register mappings moved to hwmod database

[Basak, Partha]

 

> -----Original Message-----
> From: Cousson, Benoit 
> Sent: Tuesday, October 12, 2010 1:32 PM
> To: Basak, Partha
> Cc: DebBarma, Tarun Kanti; Kevin Hilman; G, Manjunath 
> Kondaiah; linux-omap@vger.kernel.org; Shilimkar, Santosh; 
> Paul Walmsley; Tony Lindgren
> Subject: Re: [PATCHv3 8/17] dmtimer: register mappings moved 
> to hwmod database
> 
> On 10/12/2010 9:22 AM, Basak, Partha wrote:
> >
> >> From: DebBarma, Tarun Kanti
> >> Sent: Tuesday, October 12, 2010 11:19 AM
> >> To: Cousson, Benoit
> >>
> >> Benoit,
> >>> From: Cousson, Benoit
> >>> Sent: Tuesday, October 12, 2010 4:42 AM
> >>> To: DebBarma, Tarun Kanti
> >>>
> >>> Hi Tarun,
> >>>
> >>> On 10/9/2010 5:40 PM, DebBarma, Tarun Kanti wrote:
> >>>> Benoit,
> >>>>
> >>>>> From: Cousson, Benoit
> >>>>> Sent: Thursday, September 23, 2010 10:15 PM
> >>>>> To: Basak, Partha
> >>>>>
> >>>>> On 9/23/2010 11:34 AM, Basak, Partha wrote:
> >>>>>>
> >>>>>>> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
> >>>>>>> Sent: Thursday, September 23, 2010 3:10 AM
> >>>>>>>
> >>>>>>> "G, Manjunath Kondaiah"<manjugk@ti.com>    writes:
> >>>>>>>
> >>>>>>>> Hi Tarun,
> >>>>>>>>
> >>>>>>>>> From: linux-omap-owner@vger.kernel.org
> >>>>>>>>> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of
> >>>>>>>>> DebBarma, Tarun Kanti
> 
> <...>
> 
> >>>>>
> >>>>> Also, I'm not sure that you have to handle each register
> >> separately
> >>>>> considering that you have one offset to handle for the 
> functional
> >>>>> register.
> >>>>> The change in sysconfig and interrupt register are all
> >> standard mapping
> >>>>> that stick to TI highlander standard.
> >>>>> Meaning, as soon as an IP is a v2 (highlander version) all these
> >>>>> registers will be at the same place... at least in theory :-)
> >>>>>
> >>>>> Here are the deltas between the legacy and the new one:
> >>>>>
> >>>>> [OMAP_TIMER_ID_REG]             0x00,
> >>>>> [OMAP_TIMER_OCP_CFG_REG]        0x10, same
> >>>>> [OMAP_TIMER_SYS_STAT_REG]       0x14, (not used anymore)
> >>>>>
> >>>>> You should not care about these ones, because hwmod will
> >> handle them.
> >>>>>
> >>>>> [OMAP_TIMER_STAT_REG]           0x28, +10
> >>>>> [OMAP_TIMER_INT_EN_REG]         0x2c, +10
> >>>>> [OMAP_TIMER_INT_CLR_REG]        0x30, (new)
> >>>>>
> >>>>> These ones are standard registers
> >>>>>
> >>>>> [OMAP_TIMER_WAKEUP_EN_REG]      0x34, +14
> >>>>> [OMAP_TIMER_CTRL_REG]           0x38, +14
> >>>>> [OMAP_TIMER_COUNTER_REG]        0x3c, +14
> >>>>> [OMAP_TIMER_LOAD_REG]           0x40, +14
> >>>>> [OMAP_TIMER_TRIGGER_REG]        0x44, +14
> >>>>> [OMAP_TIMER_WRITE_PEND_REG]     0x48, +14
> >>>>> [OMAP_TIMER_MATCH_REG]          0x4c, +14
> >>>>> [OMAP_TIMER_CAPTURE_REG]        0x50, +14
> >>>>> [OMAP_TIMER_IF_CTRL_REG]        0x54, +14
> >>>>> [OMAP_TIMER_CAPTURE2_REG]       0x58, +14
> >>>>>
> >>>>> You can probably handle that with only 2 offsets instead
> >> of having one
> >>>>> address per registers.
> >>>>>
> >>>> To keep aligned with other driver implementations, I 
> would like to
> >>> follow this:
> >>>>
> >>>> 1) move the table in mach-omap1/2 since
> >>>> 2) remove entries which are handled by hwmod.
> >>>> However, I am not sure if there is any issue in keeping
> >> them in the
> >>> table.
> >>>
> >>> I don't think you need a table for that. All the functional
> >> registers
> >>> are using a constant offset. IRQ is then the only exception.
> >
> > Other than the offsets 0x14 for the Timer functional registers
> > and 0x10 for the IRQ registers, following differences exist
> > between the two versions:
> > 1. OMAP_TIMER_SYS_STAT_REG is used only for v1, missing for v2.
> 
> This is handled by hwmod anyway.
> 
> 
> > 2. OMAP_TIMER_INT_CLR_REG is new for v2, not used for v1.
> 
> Follow regular IRQ management for highlander IP. Should be 
> some generic 
> code in order to allow reuse.
> 
> > 3. Following registers are applicable only for v1.
> > [OMAP_TIMER_TICK_POS_REG]     		0x48
> > [OMAP_TIMER_TICK_NEG_REG]     		0x4c
> > [OMAP_TIMER_TICK_COUNT_REG]     		0x50
> > [OMAP_TIMER_TICK_INT_MASK_SET_REG]		0x54
> > [OMAP_TIMER_TICK_INT_MASK_COUNT_REG]	0x58
> 
> These are only there in the 1ms version of this IP.
> 
> > In the end, having separate tables is neater and consistent
> > with other drivers.
> 
> I don't see why? And what other driver are you referring too?

I2c driver uses reg_map tables. Initially McSPI and MMC were using
similar reg_map tables which were later removed based on review comments.

However, the delta were minimal in this case.
e.g. MCSPI_HL_REV, MCSPI_HL_HWINFO, MCSPI_HL_SYSCONFIG were the only 
difference for McSPI, all the other functional registers
were moved forward by 0x100.

Maybe, it is relevant for I2C as the amount of delta is more. 

I also observed your comments on MMC. Now that both McSPI & MMC are
not having table based approach, my argument of consistency across drivers
does not hold good. It depends upon the IPs really.
> 
> What the point of adding a table and an extra level of 
> indirection, when 
> a simple addition will do it?
> 
> > But instead of duplicating for each mach,
> > keeping them in plat should be OK.
> >
> > Additionally, the implementation should take care to prevent access
> > of non-existing registers for a particular version.
> 
> You just need the IP version to do that.
> 
> We have 3 timer variants to handle today:
> Legacy timer, legacy 1ms timer and highlander timer. You can 
> handle that 
> with 2 flags and 2 offsets.

If you look at the previous version of the DMTimer code, this is indeed
based on a bunch of #defines and offset updation. We then need to revert
back to this model with the following:

1. Maintain IP revs as dev_attrs.

2. For each IP rev, define two offsets, interrupt_reg_offset &
functional_reg_offset. 0x10 for v2 interrupt regs, 0x14 for the v2 functional regs.

3. In addiiton, take care to grant access of valid registers and prevent
access of non-existing registers for any particular IP revision.

Benoit, Tarun, do you agree?

> 
> Benoit
> 
> >
> >> Agreed!! But we have to have a check in the read/write
> >> routine to add this offset. This is an overhead I thought.
> >> Also, tomorrow when we have new silicon with further offset
> >> difference we would have to keep on adding additional checks.
> >> This would end up making the read/write routine (which is
> >> called frequently) bulky and inefficient.
> >> So, my thinking was to keep the read/write routine generic.
> >> Also, keeping separate table makes it extensible easily.
> >> At the end I am OK both ways so long as community feels ok
> >> with whichever approach.
> >> -tarun
> 
> 

Re: [PATCHv3 8/17] dmtimer: register mappings moved to hwmod database

[Cousson, Benoit]

On 10/12/2010 11:35 AM, Basak, Partha wrote:
>
>> From: Cousson, Benoit
>> Sent: Tuesday, October 12, 2010 1:32 PM
>> To: Basak, Partha
>> Cc: DebBarma, Tarun Kanti; Kevin Hilman; G, Manjunath
>> Kondaiah; linux-omap@vger.kernel.org; Shilimkar, Santosh;
>> Paul Walmsley; Tony Lindgren
>> Subject: Re: [PATCHv3 8/17] dmtimer: register mappings moved
>> to hwmod database
>>
>> On 10/12/2010 9:22 AM, Basak, Partha wrote:
>>>
>>>> From: DebBarma, Tarun Kanti
>>>> Sent: Tuesday, October 12, 2010 11:19 AM
>>>> To: Cousson, Benoit
>>>>
>>>> Benoit,
>>>>> From: Cousson, Benoit
>>>>> Sent: Tuesday, October 12, 2010 4:42 AM
>>>>> To: DebBarma, Tarun Kanti
>>>>>
>>>>> Hi Tarun,
>>>>>
>>>>> On 10/9/2010 5:40 PM, DebBarma, Tarun Kanti wrote:
>>>>>> Benoit,
>>>>>>
>>>>>>> From: Cousson, Benoit
>>>>>>> Sent: Thursday, September 23, 2010 10:15 PM
>>>>>>> To: Basak, Partha
>>>>>>>
>>>>>>> On 9/23/2010 11:34 AM, Basak, Partha wrote:
>>>>>>>>
>>>>>>>>> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>>>>>>> Sent: Thursday, September 23, 2010 3:10 AM
>>>>>>>>>
>>>>>>>>> "G, Manjunath Kondaiah"<manjugk@ti.com>     writes:
>>>>>>>>>
>>>>>>>>>> Hi Tarun,
>>>>>>>>>>
>>>>>>>>>>> From: linux-omap-owner@vger.kernel.org
>>>>>>>>>>> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of
>>>>>>>>>>> DebBarma, Tarun Kanti
>>
>> <...>
>>
>>>>>>>
>>>>>>> Also, I'm not sure that you have to handle each register
>>>> separately
>>>>>>> considering that you have one offset to handle for the
>> functional
>>>>>>> register.
>>>>>>> The change in sysconfig and interrupt register are all
>>>> standard mapping
>>>>>>> that stick to TI highlander standard.
>>>>>>> Meaning, as soon as an IP is a v2 (highlander version) all these
>>>>>>> registers will be at the same place... at least in theory :-)
>>>>>>>
>>>>>>> Here are the deltas between the legacy and the new one:
>>>>>>>
>>>>>>> [OMAP_TIMER_ID_REG]             0x00,
>>>>>>> [OMAP_TIMER_OCP_CFG_REG]        0x10, same
>>>>>>> [OMAP_TIMER_SYS_STAT_REG]       0x14, (not used anymore)
>>>>>>>
>>>>>>> You should not care about these ones, because hwmod will
>>>> handle them.
>>>>>>>
>>>>>>> [OMAP_TIMER_STAT_REG]           0x28, +10
>>>>>>> [OMAP_TIMER_INT_EN_REG]         0x2c, +10
>>>>>>> [OMAP_TIMER_INT_CLR_REG]        0x30, (new)
>>>>>>>
>>>>>>> These ones are standard registers
>>>>>>>
>>>>>>> [OMAP_TIMER_WAKEUP_EN_REG]      0x34, +14
>>>>>>> [OMAP_TIMER_CTRL_REG]           0x38, +14
>>>>>>> [OMAP_TIMER_COUNTER_REG]        0x3c, +14
>>>>>>> [OMAP_TIMER_LOAD_REG]           0x40, +14
>>>>>>> [OMAP_TIMER_TRIGGER_REG]        0x44, +14
>>>>>>> [OMAP_TIMER_WRITE_PEND_REG]     0x48, +14
>>>>>>> [OMAP_TIMER_MATCH_REG]          0x4c, +14
>>>>>>> [OMAP_TIMER_CAPTURE_REG]        0x50, +14
>>>>>>> [OMAP_TIMER_IF_CTRL_REG]        0x54, +14
>>>>>>> [OMAP_TIMER_CAPTURE2_REG]       0x58, +14
>>>>>>>
>>>>>>> You can probably handle that with only 2 offsets instead
>>>> of having one
>>>>>>> address per registers.
>>>>>>>
>>>>>> To keep aligned with other driver implementations, I
>> would like to
>>>>> follow this:
>>>>>>
>>>>>> 1) move the table in mach-omap1/2 since
>>>>>> 2) remove entries which are handled by hwmod.
>>>>>> However, I am not sure if there is any issue in keeping
>>>> them in the
>>>>> table.
>>>>>
>>>>> I don't think you need a table for that. All the functional
>>>> registers
>>>>> are using a constant offset. IRQ is then the only exception.
>>>
>>> Other than the offsets 0x14 for the Timer functional registers
>>> and 0x10 for the IRQ registers, following differences exist
>>> between the two versions:
>>> 1. OMAP_TIMER_SYS_STAT_REG is used only for v1, missing for v2.
>>
>> This is handled by hwmod anyway.
>>
>>
>>> 2. OMAP_TIMER_INT_CLR_REG is new for v2, not used for v1.
>>
>> Follow regular IRQ management for highlander IP. Should be
>> some generic
>> code in order to allow reuse.
>>
>>> 3. Following registers are applicable only for v1.
>>> [OMAP_TIMER_TICK_POS_REG]     		0x48
>>> [OMAP_TIMER_TICK_NEG_REG]     		0x4c
>>> [OMAP_TIMER_TICK_COUNT_REG]     		0x50
>>> [OMAP_TIMER_TICK_INT_MASK_SET_REG]		0x54
>>> [OMAP_TIMER_TICK_INT_MASK_COUNT_REG]	0x58
>>
>> These are only there in the 1ms version of this IP.
>>
>>> In the end, having separate tables is neater and consistent
>>> with other drivers.
>>
>> I don't see why? And what other driver are you referring too?
>
> I2c driver uses reg_map tables. Initially McSPI and MMC were using
> similar reg_map tables which were later removed based on review comments.
>
> However, the delta were minimal in this case.
> e.g. MCSPI_HL_REV, MCSPI_HL_HWINFO, MCSPI_HL_SYSCONFIG were the only
> difference for McSPI, all the other functional registers
> were moved forward by 0x100.
>
> Maybe, it is relevant for I2C as the amount of delta is more.
>
> I also observed your comments on MMC. Now that both McSPI&  MMC are
> not having table based approach, my argument of consistency across drivers
> does not hold good. It depends upon the IPs really.

Yep, I do agree, for both SPI and MMC, it was really over-designed 
because the offset was constant.
For the timer, it is a little bit more complex, but still can be handled 
easily. At some point, the table approach can clearly make sense.
For for the timer, I still think it is a little bit over-designed.

>>
>> What the point of adding a table and an extra level of
>> indirection, when
>> a simple addition will do it?
>>
>>> But instead of duplicating for each mach,
>>> keeping them in plat should be OK.
>>>
>>> Additionally, the implementation should take care to prevent access
>>> of non-existing registers for a particular version.
>>
>> You just need the IP version to do that.
>>
>> We have 3 timer variants to handle today:
>> Legacy timer, legacy 1ms timer and highlander timer. You can
>> handle that
>> with 2 flags and 2 offsets.
>
> If you look at the previous version of the DMTimer code, this is indeed
> based on a bunch of #defines and offset updation. We then need to revert
> back to this model with the following:

I didn't check carefully, but if you think that there is a clear benefit 
for the readability point of view or if is simplify the code a lot maybe 
that worth it.

In that case, you can maybe hide that by providing 2 accessors like 
omap_timer_read and omap_timer_write, and 2 others for IRQ registers, 
and you will add the offset calculation in it.

> 1. Maintain IP revs as dev_attrs.
>
> 2. For each IP rev, define two offsets, interrupt_reg_offset&
> functional_reg_offset. 0x10 for v2 interrupt regs, 0x14 for the v2 functional regs.
>
> 3. In addiiton, take care to grant access of valid registers and prevent
> access of non-existing registers for any particular IP revision.
>
> Benoit, Tarun, do you agree?

That sound good to me.

Benoit