| Message ID | 1307034677-27236-2-git-send-email-nsekhar@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Thu, Jun 2, 2011 at 10:11 AM, Sekhar Nori <nsekhar@ti.com> wrote: > From: Bob Dunlop <bob.dunlop@xyzzy.org.uk> > > The DM6467 and DM6467T EVMs use different reference clock > frequencies. This difference is currently supported by having > the SoC code call a public board routine which sets up the reference > clock frequency. This does not scale as more boards are added. > > Instead, pass the reference clock frequency as a parameter > to dm646x_init(). Boards which do not need the default reference > clock changed can pass NULL to this function. > > Signed-off-by: Bob Dunlop <bob.dunlop@xyzzy.org.uk> > Signed-off-by: Sekhar Nori <nsekhar@ti.com> > --- > I have marked Bob as the author of this patch since it is essentially > his idea. Hope he doesnt mind that :) > > arch/arm/mach-davinci/board-dm646x-evm.c | 23 +++++++++++------------ > arch/arm/mach-davinci/dm646x.c | 6 ++++-- > arch/arm/mach-davinci/include/mach/common.h | 10 ++++++++++ > arch/arm/mach-davinci/include/mach/dm646x.h | 4 ++-- > 4 files changed, 27 insertions(+), 16 deletions(-) > > diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c > index f6ac9ba..37c49a9 100644 > --- a/arch/arm/mach-davinci/board-dm646x-evm.c > +++ b/arch/arm/mach-davinci/board-dm646x-evm.c > @@ -719,9 +719,19 @@ static void __init cdce_clk_init(void) > } > } > > +#define DM646X_EVM_REF_FREQ 27000000 > +#define DM6467T_EVM_REF_FREQ 33000000 > + > +static struct davinci_board_info dm646x_evm; > + > static void __init davinci_map_io(void) > { > - dm646x_init(); > + if (machine_is_davinci_dm6467tevm()) > + dm646x_evm.ref_clk_rate = DM6467T_EVM_REF_FREQ; > + else > + dm646x_evm.ref_clk_rate = DM646X_EVM_REF_FREQ; > + > + dm646x_init(&dm646x_evm); Do you foresee needing more members of the davinci_board_info struct in the near future? If so, this approach is OK. If this is the only anticipated use today, just passing in the refclk rate as an integer argument would be just as clean, with less infrastructure. -Olof
Hi Olof, On Thu, Jun 02, 2011 at 23:31:05, Olof Johansson wrote: > Do you foresee needing more members of the davinci_board_info struct > in the near future? If so, this approach is OK. If this is the only > anticipated use today, just passing in the refclk rate as an integer > argument would be just as clean, with less infrastructure. There was some discussion on passing PLL configuration information too. So yes, the structure is likely extended in future. Also, most boards end up using the reference clock used by the EVM. In that case, with this approach, most boards just pass NULL instead of replicating the reference clock frequency in all board files. Thanks, Sekhar
Hi, On Fri, Jun 3, 2011 at 4:32 AM, Nori, Sekhar <nsekhar@ti.com> wrote: > Hi Olof, > > On Thu, Jun 02, 2011 at 23:31:05, Olof Johansson wrote: > >> Do you foresee needing more members of the davinci_board_info struct >> in the near future? If so, this approach is OK. If this is the only >> anticipated use today, just passing in the refclk rate as an integer >> argument would be just as clean, with less infrastructure. > > There was some discussion on passing PLL configuration information too. > So yes, the structure is likely extended in future. > > Also, most boards end up using the reference clock used by the EVM. > In that case, with this approach, most boards just pass NULL instead of > replicating the reference clock frequency in all board files. OK, that sounds reasonable. Only change I would like to request in that case (sorry, not commenting on the patch directly here), is to mark dm646x_evm __initdata (or even make it a local variable in the function), so for multiboard kernels you won't need to keep the static struct around for hardware you're not running on. -Olof
On Fri, Jun 03, 2011 at 22:29:26, Olof Johansson wrote: > Only change I would like to request in that case (sorry, not > commenting on the patch directly here), is to mark dm646x_evm > __initdata (or even make it a local variable in the function), so for > multiboard kernels you won't need to keep the static struct around for > hardware you're not running on. > Fixed that and sent a new version. Thanks for the review. Regards, Sekhar
Hello. On 02-06-2011 21:11, Sekhar Nori wrote: > From: Bob Dunlop<bob.dunlop@xyzzy.org.uk> > The DM6467 and DM6467T EVMs use different reference clock > frequencies. This difference is currently supported by having > the SoC code call a public board routine which sets up the reference > clock frequency. This does not scale as more boards are added. > Instead, pass the reference clock frequency as a parameter > to dm646x_init(). Boards which do not need the default reference > clock changed can pass NULL to this function. > Signed-off-by: Bob Dunlop<bob.dunlop@xyzzy.org.uk> > Signed-off-by: Sekhar Nori<nsekhar@ti.com> > --- > I have marked Bob as the author of this patch since it is essentially > his idea. Hope he doesnt mind that :) I don't think you should ascribe *your* patch to Bob. There's Suggested-by: line if you want to credit Bob. WBR, Sergei
On Sun, Jun 05, 2011 at 18:30:04, Sergei Shtylyov wrote: > Hello. > > On 02-06-2011 21:11, Sekhar Nori wrote: > > > From: Bob Dunlop<bob.dunlop@xyzzy.org.uk> > > > The DM6467 and DM6467T EVMs use different reference clock > > frequencies. This difference is currently supported by having > > the SoC code call a public board routine which sets up the reference > > clock frequency. This does not scale as more boards are added. > > > Instead, pass the reference clock frequency as a parameter > > to dm646x_init(). Boards which do not need the default reference > > clock changed can pass NULL to this function. > > > Signed-off-by: Bob Dunlop<bob.dunlop@xyzzy.org.uk> > > Signed-off-by: Sekhar Nori<nsekhar@ti.com> > > --- > > I have marked Bob as the author of this patch since it is essentially > > his idea. Hope he doesnt mind that :) > > I don't think you should ascribe *your* patch to Bob. There's > Suggested-by: line if you want to credit Bob. Well, in this case I actually took a patch authored by Bob, split it into two and moved around the code to other files. But there is also code written by me included in this patch. Bob, do you have an opinion on this? If not, I will go with what Sergei is suggesting. Thanks, Sekhar
diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c index f6ac9ba..37c49a9 100644 --- a/arch/arm/mach-davinci/board-dm646x-evm.c +++ b/arch/arm/mach-davinci/board-dm646x-evm.c @@ -719,9 +719,19 @@ static void __init cdce_clk_init(void) } } +#define DM646X_EVM_REF_FREQ 27000000 +#define DM6467T_EVM_REF_FREQ 33000000 + +static struct davinci_board_info dm646x_evm; + static void __init davinci_map_io(void) { - dm646x_init(); + if (machine_is_davinci_dm6467tevm()) + dm646x_evm.ref_clk_rate = DM6467T_EVM_REF_FREQ; + else + dm646x_evm.ref_clk_rate = DM646X_EVM_REF_FREQ; + + dm646x_init(&dm646x_evm); cdce_clk_init(); } @@ -785,17 +795,6 @@ static __init void evm_init(void) soc_info->emac_pdata->phy_id = DM646X_EVM_PHY_ID; } -#define DM646X_EVM_REF_FREQ 27000000 -#define DM6467T_EVM_REF_FREQ 33000000 - -void __init dm646x_board_setup_refclk(struct clk *clk) -{ - if (machine_is_davinci_dm6467tevm()) - clk->rate = DM6467T_EVM_REF_FREQ; - else - clk->rate = DM646X_EVM_REF_FREQ; -} - MACHINE_START(DAVINCI_DM6467_EVM, "DaVinci DM646x EVM") .boot_params = (0x80000100), .map_io = davinci_map_io, diff --git a/arch/arm/mach-davinci/dm646x.c b/arch/arm/mach-davinci/dm646x.c index 1e0f809..871af17 100644 --- a/arch/arm/mach-davinci/dm646x.c +++ b/arch/arm/mach-davinci/dm646x.c @@ -899,9 +899,11 @@ int __init dm646x_init_edma(struct edma_rsv_info *rsv) return platform_device_register(&dm646x_edma_device); } -void __init dm646x_init(void) +void __init dm646x_init(struct davinci_board_info *board) { - dm646x_board_setup_refclk(&ref_clk); + if (board && board->ref_clk_rate) + ref_clk.rate = board->ref_clk_rate; + davinci_common_init(&davinci_soc_info_dm646x); } diff --git a/arch/arm/mach-davinci/include/mach/common.h b/arch/arm/mach-davinci/include/mach/common.h index a57cba2..fbe650b 100644 --- a/arch/arm/mach-davinci/include/mach/common.h +++ b/arch/arm/mach-davinci/include/mach/common.h @@ -83,6 +83,16 @@ struct davinci_soc_info { extern struct davinci_soc_info davinci_soc_info; +/* + * DaVinci board info. + * + * This structure is used to pass board information to + * early SoC specific initialization code. + */ +struct davinci_board_info { + unsigned long ref_clk_rate; +}; + extern void davinci_common_init(struct davinci_soc_info *soc_info); extern void davinci_init_ide(void); diff --git a/arch/arm/mach-davinci/include/mach/dm646x.h b/arch/arm/mach-davinci/include/mach/dm646x.h index 5e95b02..b92db82 100644 --- a/arch/arm/mach-davinci/include/mach/dm646x.h +++ b/arch/arm/mach-davinci/include/mach/dm646x.h @@ -16,6 +16,7 @@ #include <linux/clk.h> #include <linux/davinci_emac.h> +#include <mach/common.h> #include <mach/hardware.h> #include <mach/asp.h> @@ -29,10 +30,9 @@ #define DM646X_ASYNC_EMIF_CONTROL_BASE 0x20008000 #define DM646X_ASYNC_EMIF_CS2_SPACE_BASE 0x42000000 -void __init dm646x_init(void); +void __init dm646x_init(struct davinci_board_info *board); void __init dm646x_init_mcasp0(struct snd_platform_data *pdata); void __init dm646x_init_mcasp1(struct snd_platform_data *pdata); -void __init dm646x_board_setup_refclk(struct clk *clk); int __init dm646x_init_edma(struct edma_rsv_info *rsv); void dm646x_video_init(void);