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@xxxxxxxxxxxxxxx; 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@xxxxxxxxxxxxxxxxxxx]
Sent: Thursday, September 23, 2010 3:10 AM
"G, Manjunath Kondaiah"<manjugk@xxxxxx> writes:
Hi Tarun,
From: linux-omap-owner@xxxxxxxxxxxxxxx
[mailto:linux-omap-owner@xxxxxxxxxxxxxxx] 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.
