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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



 

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx] 
> Sent: Thursday, September 23, 2010 3:10 AM
> To: G, Manjunath Kondaiah
> Cc: DebBarma, Tarun Kanti; linux-omap@xxxxxxxxxxxxxxx; 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@xxxxxx> writes:
> 
> > Hi Tarun,
> >
> >> -----Original Message-----
> >> From: linux-omap-owner@xxxxxxxxxxxxxxx 
> >> [mailto:linux-omap-owner@xxxxxxxxxxxxxxx] On Behalf Of 
> >> DebBarma, Tarun Kanti
> >> Sent: Tuesday, September 21, 2010 2:24 PM
> >> To: linux-omap@xxxxxxxxxxxxxxx
> >> 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@xxxxxx>
> >> Signed-off-by: Partha Basak <p-basak2@xxxxxx>
> >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@xxxxxx>
> >> Cc: Cousson, Benoit <b-cousson@xxxxxx>
> >> Cc: Paul Walmsley <paul@xxxxxxxxx>
> >> Cc: Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx>
> >> Cc: Tony Lindgren <tony@xxxxxxxxxxx>
> >> ---
> >>  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
> 
> --
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


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux