Benoit, Paul, Kevin, > -----Original Message----- > From: linux-omap-owner@xxxxxxxxxxxxxxx [mailto:linux-omap- > owner@xxxxxxxxxxxxxxx] On Behalf Of DebBarma, Tarun Kanti > Sent: Saturday, October 09, 2010 8:29 PM > To: Cousson, Benoit; Paul Walmsley > Cc: linux-omap@xxxxxxxxxxxxxxx; Gopinath, Thara; Basak, Partha; Kevin > Hilman; Tony Lindgren > Subject: RE: [PATCHv3 3/17] dmtimer: add omap2420 hwmod database > > Benoit, Paul, > > > -----Original Message----- > > From: Cousson, Benoit > > Sent: Monday, October 04, 2010 1:20 PM > > To: Paul Walmsley > > Cc: DebBarma, Tarun Kanti; linux-omap@xxxxxxxxxxxxxxx; Gopinath, Thara; > > Basak, Partha; Kevin Hilman; Tony Lindgren > > Subject: Re: [PATCHv3 3/17] dmtimer: add omap2420 hwmod database > > > > Hi Paul, > > > > On 10/2/2010 12:25 AM, Paul Walmsley wrote: > > > On Thu, 30 Sep 2010, Cousson, Benoit wrote: > > > > > >> On 9/21/2010 10:51 AM, DebBarma, Tarun Kanti wrote: > > >> > > >>> #include "omap_hwmod_common_data.h" > > >>> > > >>> #include "prm-regbits-24xx.h" > > >>> @@ -121,6 +123,614 @@ static struct omap_hwmod > omap2420_l4_wkup_hwmod > > = { > > >>> .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP2420), > > >>> .flags = HWMOD_NO_IDLEST, > > >>> }; > > >>> +/* Timer Common */ > > >>> +static char *timer_clk_src_names[] = { > > >>> + "sys_ck", > > >>> + "func_32k_ck", > > >>> + "alt_ck", > > >>> + NULL, > > >>> +}; > > >> > > >> I have an issue with that, because this is a pure duplication of the > > clock_sel > > >> information already contained in the clock data: > > >> > > >> static const struct clksel omap24xx_gpt_clksel[] = { > > >> { .parent =&func_32k_ck, .rates = gpt_32k_rates }, > > >> { .parent =&sys_ck, .rates = gpt_sys_rates }, > > >> { .parent =&alt_ck, .rates = gpt_alt_rates }, > > >> { .parent = NULL }, > > >> }; > > >> > > >> And duplicating the same information somewhere else is most of the > time > > a bad > > >> idea. > > > > > > Yep, there's no way that info should be in the hwmod data, in the > > current > > > setup. It belongs in the clkdev tables. Example below. > > > > > >> That being said... I don't really know how to handle that properly :- > ) > > >> > > >> We have to find a better way to select the proper source clock in a > soc > > >> independent way. > > >> > > >> Maybe Paul will have some idea? > > > > > > Here's how it's done: > > > > > > http://marc.info/?l=linux-omap&m=128596931017785&w=2 > > > > > > and > > > > > > http://marc.info/?l=linux-omap&m=128596931417805&w=2 > > > > The famous clock alias... I don't know why but I always forgot that > > solution each time I have such concern:-( > > This is indeed the very clean and cool way to do that clock selection. > > We can even remove this #define to identified them and use the clock > > string name directly. > > > > I will incorporate the suggestions. Thanks!! In the present implementation there is inconsistency in the clock source names for the different platforms, viz. OMAP2, OMAP3 and OMAP4 as shown below. Therefore, I will have to modify the names so that they all have common name across the platforms for the same type of clock. In this regard I am proposing to modify the clock source names similar to OMAP4. Of course we also have to look around to see if there are other modules who are using the clock and make the necessary changes. I would like to hear from you and get your inputs! Thanks. -tarun OMAP2430 static char *timer_clk_src_names[] = { "sys_ck", "func_32k_ck", "alt_ck", NULL }; OMAP3xxx static char *timer_clk_src_names[] = { "sys_ck", "omap_32k_fck", NULL, }; OMAP4 static char *timer_clk_src_names[] = { "sys_clkin_ck", "sys_32k_ck", NULL, }; static char *timer_clk_src_names_abe[] = { "syc_clk_div_ck", "sys_32k_ck", NULL, }; > > -- > 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 -- 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
