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@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. > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
