[2/3] davinci: dm6467/T EVM: pass reference clock rate to dm646x_init()
diff mbox

Message ID 1307034677-27236-2-git-send-email-nsekhar@ti.com
State New, archived
Headers show

Commit Message

Sekhar Nori June 2, 2011, 5:11 p.m. UTC
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(-)

Comments

Olof Johansson June 2, 2011, 6:01 p.m. UTC | #1
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
Sekhar Nori June 3, 2011, 11:32 a.m. UTC | #2
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
Olof Johansson June 3, 2011, 4:59 p.m. UTC | #3
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
Sekhar Nori June 5, 2011, 11:24 a.m. UTC | #4
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
Sergei Shtylyov June 5, 2011, 1 p.m. UTC | #5
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
Sekhar Nori June 6, 2011, 11:31 a.m. UTC | #6
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

Patch
diff mbox

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);