Patchwork [RFC,4/4,powerpc] Add flexcan device support for p1010rdb.

login
register
mail settings
Submitter holt@sgi.com
Date Aug. 9, 2011, 6:33 a.m.
Message ID <20110809063319.GK4926@sgi.com>
Download mbox | patch
Permalink /patch/109137/
State Not Applicable
Headers show

Comments

holt@sgi.com - Aug. 9, 2011, 6:33 a.m.
Argh.  I sent an earlier (non-working) version of this patch.  Here is
the correct one.

I added a clock source for the p1010rdb so the flexcan driver
could find its clock frequency.

Signed-off-by: Robin Holt <holt@sgi.com>
To: Marc Kleine-Budde <mkl@pengutronix.de>,
To: Wolfgang Grandegger <wg@grandegger.com>,
To: U Bhaskar-B22300 <B22300@freescale.com>
Cc: socketcan-core@lists.berlios.de,
Cc: netdev@vger.kernel.org,
Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
---
 arch/powerpc/platforms/85xx/Kconfig    |    6 +++
 arch/powerpc/platforms/85xx/Makefile   |    1 +
 arch/powerpc/platforms/85xx/clock.c    |   59 ++++++++++++++++++++++++++++++++
 arch/powerpc/platforms/85xx/p1010rdb.c |   10 +++++
 4 files changed, 76 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/platforms/85xx/clock.c
Wolfgang Grandegger - Aug. 9, 2011, 7:11 a.m.
On 08/09/2011 08:33 AM, Robin Holt wrote:
> Argh.  I sent an earlier (non-working) version of this patch.  Here is
> the correct one.

Please always resend the complete series of patches with an incremented
version number. Furthermore, this is not an RFC any more. A prefix
similar to [PATCH nfsl_get_sys_freq() et-next-2.6 v2] would be perfect.

> I added a clock source for the p1010rdb so the flexcan driver
> could find its clock frequency.
> 
> Signed-off-by: Robin Holt <holt@sgi.com>
> To: Marc Kleine-Budde <mkl@pengutronix.de>,
> To: Wolfgang Grandegger <wg@grandegger.com>,
> To: U Bhaskar-B22300 <B22300@freescale.com>
> Cc: socketcan-core@lists.berlios.de,
> Cc: netdev@vger.kernel.org,
> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> ---
>  arch/powerpc/platforms/85xx/Kconfig    |    6 +++
>  arch/powerpc/platforms/85xx/Makefile   |    1 +
>  arch/powerpc/platforms/85xx/clock.c    |   59 ++++++++++++++++++++++++++++++++
>  arch/powerpc/platforms/85xx/p1010rdb.c |   10 +++++
>  4 files changed, 76 insertions(+), 0 deletions(-)
>  create mode 100644 arch/powerpc/platforms/85xx/clock.c
> 
> diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
> index 498534c..ed4cf92 100644
> --- a/arch/powerpc/platforms/85xx/Kconfig
> +++ b/arch/powerpc/platforms/85xx/Kconfig
> @@ -26,6 +26,10 @@ config MPC8560_ADS
>  	help
>  	  This option enables support for the MPC 8560 ADS board
>  
> +config 85xx_HAVE_CAN_FLEXCAN
> +	bool
> +	select HAVE_CAN_FLEXCAN if NET && CAN
> +

Why do you need that? More below...

>  config MPC85xx_CDS
>  	bool "Freescale MPC85xx CDS"
>  	select DEFAULT_UIMAGE
> @@ -70,6 +74,8 @@ config MPC85xx_RDB
>  config P1010_RDB
>  	bool "Freescale P1010RDB"
>  	select DEFAULT_UIMAGE
> +	select 85xx_HAVE_CAN_FLEXCAN
> +	select PPC_CLOCK if CAN_FLEXCAN

	select HAVE_CAN_FLEXCAN
	select PPC_CLOCK

Should just be fine, or have I missed something.

>  	help
>  	  This option enables support for the MPC85xx RDB (P1010 RDB) board
>  
> diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
> index a971b32..64ad7a4 100644
> --- a/arch/powerpc/platforms/85xx/Makefile
> +++ b/arch/powerpc/platforms/85xx/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_MPC85xx_DS)  += mpc85xx_ds.o
>  obj-$(CONFIG_MPC85xx_MDS) += mpc85xx_mds.o
>  obj-$(CONFIG_MPC85xx_RDB) += mpc85xx_rdb.o
>  obj-$(CONFIG_P1010_RDB)   += p1010rdb.o
> +obj-$(CONFIG_PPC_CLOCK)   += clock.o

I would put that to the beginning or before the board settings.

>  obj-$(CONFIG_P1022_DS)    += p1022_ds.o
>  obj-$(CONFIG_P1023_RDS)   += p1023_rds.o
>  obj-$(CONFIG_P2040_RDB)   += p2040_rdb.o corenet_ds.o
> diff --git a/arch/powerpc/platforms/85xx/clock.c b/arch/powerpc/platforms/85xx/clock.c
> new file mode 100644
> index 0000000..a25cbf3
> --- /dev/null
> +++ b/arch/powerpc/platforms/85xx/clock.c
> @@ -0,0 +1,59 @@
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +
> +#include <asm/clk_interface.h>
> +
> +#include <sysdev/fsl_soc.h>
> +
> +/*
> + * p1010rdb needs to provide a clock source for the flexcan driver.
> + */
> +struct clk {
> +	unsigned long rate;
> +} p1010_rdb_system_clock;
> +
> +static struct clk *p1010_rdb_clk_get(struct device *dev, const char *id)
> +{
> +	const char *dev_init_name;
> +
> +	if (!dev)
> +		return ERR_PTR(-ENOENT);
> +
> +	/*
> +	 * The can devices are named ffe1c000.can0 and ffe1d000.can1 on
> +	 * the p1010rdb.  Check for the "can" portion of that name before
> +	 * returning a clock source.
> +	 */
> +	dev_init_name = dev_name(dev);
> +	if (strlen(dev_init_name) != 13)
> +		return ERR_PTR(-ENOENT);
> +	dev_init_name += 9;
> +	if (strncmp(dev_init_name, "can", 3))
> +		return ERR_PTR(-ENOENT);

What's that good for? Also it's wrong to rely on the special name of the
node. I think it can be removed.

> +	return &p1010_rdb_system_clock;

Just returning fsl_get_sys_freq() here would already be fine. I'm also
missing the factor of two here:

        return fsl_get_sys_freq() / 2; ????

> +}
> +
> +static void p1010_rdb_clk_put(struct clk *clk)
> +{
> +	return;
> +}
> +
> +static unsigned long p1010_rdb_clk_get_rate(struct clk *clk)
> +{
> +	return clk->rate;
> +}
> +
> +static struct clk_interface p1010_rdb_clk_functions = {
> +	.clk_get		= p1010_rdb_clk_get,
> +	.clk_get_rate		= p1010_rdb_clk_get_rate,
> +	.clk_put		= p1010_rdb_clk_put,
> +};
> +
> +void __init p1010_rdb_clk_init(void)
> +{
> +	p1010_rdb_system_clock.rate = fsl_get_sys_freq();

> +	clk_functions = p1010_rdb_clk_functions;
> +}

The name is too specific. The idea is that the interface could be used
for other 85xx processors as well, not only the p1010. The prefix
"mpc85xx_" instead of "p1010_rdb" seems more appropriate to me.

> diff --git a/arch/powerpc/platforms/85xx/p1010rdb.c b/arch/powerpc/platforms/85xx/p1010rdb.c
> index d7387fa..d0afbf9 100644
> --- a/arch/powerpc/platforms/85xx/p1010rdb.c
> +++ b/arch/powerpc/platforms/85xx/p1010rdb.c
> @@ -81,6 +81,15 @@ static void __init p1010_rdb_setup_arch(void)
>  	printk(KERN_INFO "P1010 RDB board from Freescale Semiconductor\n");
>  }
>  
> +extern void p1010_rdb_clk_init(void);
> +
> +static void __init p1010_rdb_init(void)
> +{
> +#ifdef CONFIG_PPC_CLOCK
> +	p1010_rdb_clk_init();
> +#endif
> +}

The #ifdef's are not needed if CONFIG_PPC_CLOCK is selected in the Kconfig.

>  static struct of_device_id __initdata p1010rdb_ids[] = {
>  	{ .type = "soc", },
>  	{ .compatible = "soc", },
> @@ -111,6 +120,7 @@ define_machine(p1010_rdb) {
>  	.name			= "P1010 RDB",
>  	.probe			= p1010_rdb_probe,
>  	.setup_arch		= p1010_rdb_setup_arch,
> +	.init			= p1010_rdb_init,
>  	.init_IRQ		= p1010_rdb_pic_init,
>  #ifdef CONFIG_PCI
>  	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,

We also need cleanup patches for the Freescale stuff.

Thanks,

Wolfgang.
holt@sgi.com - Aug. 9, 2011, 11:40 a.m.
On Tue, Aug 09, 2011 at 09:11:33AM +0200, Wolfgang Grandegger wrote:
> > +	return &p1010_rdb_system_clock;
> 
> Just returning fsl_get_sys_freq() here would already be fine. I'm also
> missing the factor of two here:
> 
>         return fsl_get_sys_freq() / 2; ????

I am working on the other comments right now as well, but this one
brought up a good question.  The old algorithm in the original freescale
patches I started with actually did, essentially:

	...clock.freq = DIV_ROUND_CLOSEST(fsl_get_sys_freq() / 2, 1000) * 1000

The end result was before:
	...clock.freq=0x0bebc200
After:
	...clock.freq=0x0bebc1fe

Is that rounding relavent?

Thanks,
Robin
Marc Kleine-Budde - Aug. 9, 2011, 12:04 p.m.
On 08/09/2011 01:40 PM, Robin Holt wrote:
> On Tue, Aug 09, 2011 at 09:11:33AM +0200, Wolfgang Grandegger wrote:
>>> +	return &p1010_rdb_system_clock;
>>
>> Just returning fsl_get_sys_freq() here would already be fine. I'm also
>> missing the factor of two here:
>>
>>         return fsl_get_sys_freq() / 2; ????
> 
> I am working on the other comments right now as well, but this one
> brought up a good question.  The old algorithm in the original freescale
> patches I started with actually did, essentially:
> 
> 	...clock.freq = DIV_ROUND_CLOSEST(fsl_get_sys_freq() / 2, 1000) * 1000
> 
> The end result was before:
> 	...clock.freq=0x0bebc200

That's exactly 200 MHz

> After:
> 	...clock.freq=0x0bebc1fe

this is 199 999 998 Hz

> Is that rounding relavent?

_If_ 200 MHz is correct, this would be an error of 0.000001%. But the
fsl code rounds to closest KHz. Rounding introduces errors in most of
the cases. IMHO it's better not to round here.

For example the usual at91can is clocked with 99.532800 MHz, this is a
least the value the arm clock framework reports.

Cheers, Marc
holt@sgi.com - Aug. 9, 2011, 12:04 p.m.
On Tue, Aug 09, 2011 at 09:11:33AM +0200, Wolfgang Grandegger wrote:
> On 08/09/2011 08:33 AM, Robin Holt wrote:
> > Argh.  I sent an earlier (non-working) version of this patch.  Here is
> > the correct one.
> 
> Please always resend the complete series of patches with an incremented
> version number. Furthermore, this is not an RFC any more. A prefix
> similar to [PATCH nfsl_get_sys_freq() et-next-2.6 v2] would be perfect.
> 
> > I added a clock source for the p1010rdb so the flexcan driver
> > could find its clock frequency.
> > 
> > Signed-off-by: Robin Holt <holt@sgi.com>
> > To: Marc Kleine-Budde <mkl@pengutronix.de>,
> > To: Wolfgang Grandegger <wg@grandegger.com>,
> > To: U Bhaskar-B22300 <B22300@freescale.com>
> > Cc: socketcan-core@lists.berlios.de,
> > Cc: netdev@vger.kernel.org,
> > Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> > ---
> >  arch/powerpc/platforms/85xx/Kconfig    |    6 +++
> >  arch/powerpc/platforms/85xx/Makefile   |    1 +
> >  arch/powerpc/platforms/85xx/clock.c    |   59 ++++++++++++++++++++++++++++++++
> >  arch/powerpc/platforms/85xx/p1010rdb.c |   10 +++++
> >  4 files changed, 76 insertions(+), 0 deletions(-)
> >  create mode 100644 arch/powerpc/platforms/85xx/clock.c
> > 
> > diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
> > index 498534c..ed4cf92 100644
> > --- a/arch/powerpc/platforms/85xx/Kconfig
> > +++ b/arch/powerpc/platforms/85xx/Kconfig
> > @@ -26,6 +26,10 @@ config MPC8560_ADS
> >  	help
> >  	  This option enables support for the MPC 8560 ADS board
> >  
> > +config 85xx_HAVE_CAN_FLEXCAN
> > +	bool
> > +	select HAVE_CAN_FLEXCAN if NET && CAN
> > +
> 
> Why do you need that? More below...

Happily removed.

> >  config MPC85xx_CDS
> >  	bool "Freescale MPC85xx CDS"
> >  	select DEFAULT_UIMAGE
> > @@ -70,6 +74,8 @@ config MPC85xx_RDB
> >  config P1010_RDB
> >  	bool "Freescale P1010RDB"
> >  	select DEFAULT_UIMAGE
> > +	select 85xx_HAVE_CAN_FLEXCAN
> > +	select PPC_CLOCK if CAN_FLEXCAN
> 
> 	select HAVE_CAN_FLEXCAN
> 	select PPC_CLOCK
> 
> Should just be fine, or have I missed something.

If I don't have the if NET && CAN, then 'make savedefconfig' will
complain.  Otherwise, removed 85xx_HAVE_CAN_FLEXCAN.

> >  	help
> >  	  This option enables support for the MPC85xx RDB (P1010 RDB) board
> >  
> > diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
> > index a971b32..64ad7a4 100644
> > --- a/arch/powerpc/platforms/85xx/Makefile
> > +++ b/arch/powerpc/platforms/85xx/Makefile
> > @@ -11,6 +11,7 @@ obj-$(CONFIG_MPC85xx_DS)  += mpc85xx_ds.o
> >  obj-$(CONFIG_MPC85xx_MDS) += mpc85xx_mds.o
> >  obj-$(CONFIG_MPC85xx_RDB) += mpc85xx_rdb.o
> >  obj-$(CONFIG_P1010_RDB)   += p1010rdb.o
> > +obj-$(CONFIG_PPC_CLOCK)   += clock.o
> 
> I would put that to the beginning or before the board settings.

Done.

> >  obj-$(CONFIG_P1022_DS)    += p1022_ds.o
> >  obj-$(CONFIG_P1023_RDS)   += p1023_rds.o
> >  obj-$(CONFIG_P2040_RDB)   += p2040_rdb.o corenet_ds.o
> > diff --git a/arch/powerpc/platforms/85xx/clock.c b/arch/powerpc/platforms/85xx/clock.c
> > new file mode 100644
> > index 0000000..a25cbf3
> > --- /dev/null
> > +++ b/arch/powerpc/platforms/85xx/clock.c
> > @@ -0,0 +1,59 @@
> > +
> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +
> > +#include <asm/clk_interface.h>
> > +
> > +#include <sysdev/fsl_soc.h>
> > +
> > +/*
> > + * p1010rdb needs to provide a clock source for the flexcan driver.
> > + */
> > +struct clk {
> > +	unsigned long rate;
> > +} p1010_rdb_system_clock;
> > +
> > +static struct clk *p1010_rdb_clk_get(struct device *dev, const char *id)
> > +{
> > +	const char *dev_init_name;
> > +
> > +	if (!dev)
> > +		return ERR_PTR(-ENOENT);
> > +
> > +	/*
> > +	 * The can devices are named ffe1c000.can0 and ffe1d000.can1 on
> > +	 * the p1010rdb.  Check for the "can" portion of that name before
> > +	 * returning a clock source.
> > +	 */
> > +	dev_init_name = dev_name(dev);
> > +	if (strlen(dev_init_name) != 13)
> > +		return ERR_PTR(-ENOENT);
> > +	dev_init_name += 9;
> > +	if (strncmp(dev_init_name, "can", 3))
> > +		return ERR_PTR(-ENOENT);
> 
> What's that good for? Also it's wrong to rely on the special name of the
> node. I think it can be removed.

Removed.

> 
> > +	return &p1010_rdb_system_clock;
> 
> Just returning fsl_get_sys_freq() here would already be fine. I'm also
> missing the factor of two here:
> 
>         return fsl_get_sys_freq() / 2; ????

If we were going that route, I would return NULL here and do that
return on the clk_get_rate() call, correct?  I just tested with that
and it worked.

> > +}
> > +
> > +static void p1010_rdb_clk_put(struct clk *clk)
> > +{
> > +	return;
> > +}
> > +
> > +static unsigned long p1010_rdb_clk_get_rate(struct clk *clk)
> > +{
> > +	return clk->rate;
> > +}
> > +
> > +static struct clk_interface p1010_rdb_clk_functions = {
> > +	.clk_get		= p1010_rdb_clk_get,
> > +	.clk_get_rate		= p1010_rdb_clk_get_rate,
> > +	.clk_put		= p1010_rdb_clk_put,
> > +};
> > +
> > +void __init p1010_rdb_clk_init(void)
> > +{
> > +	p1010_rdb_system_clock.rate = fsl_get_sys_freq();
> 
> > +	clk_functions = p1010_rdb_clk_functions;
> > +}
> 
> The name is too specific. The idea is that the interface could be used
> for other 85xx processors as well, not only the p1010. The prefix
> "mpc85xx_" instead of "p1010_rdb" seems more appropriate to me.

Done.

> 
> > diff --git a/arch/powerpc/platforms/85xx/p1010rdb.c b/arch/powerpc/platforms/85xx/p1010rdb.c
> > index d7387fa..d0afbf9 100644
> > --- a/arch/powerpc/platforms/85xx/p1010rdb.c
> > +++ b/arch/powerpc/platforms/85xx/p1010rdb.c
> > @@ -81,6 +81,15 @@ static void __init p1010_rdb_setup_arch(void)
> >  	printk(KERN_INFO "P1010 RDB board from Freescale Semiconductor\n");
> >  }
> >  
> > +extern void p1010_rdb_clk_init(void);
> > +
> > +static void __init p1010_rdb_init(void)
> > +{
> > +#ifdef CONFIG_PPC_CLOCK
> > +	p1010_rdb_clk_init();
> > +#endif
> > +}
> 
> The #ifdef's are not needed if CONFIG_PPC_CLOCK is selected in the Kconfig.

Done.

> 
> >  static struct of_device_id __initdata p1010rdb_ids[] = {
> >  	{ .type = "soc", },
> >  	{ .compatible = "soc", },
> > @@ -111,6 +120,7 @@ define_machine(p1010_rdb) {
> >  	.name			= "P1010 RDB",
> >  	.probe			= p1010_rdb_probe,
> >  	.setup_arch		= p1010_rdb_setup_arch,
> > +	.init			= p1010_rdb_init,
> >  	.init_IRQ		= p1010_rdb_pic_init,
> >  #ifdef CONFIG_PCI
> >  	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,

Thank you very much for your patience with me,
Robin

Patch

diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
index 498534c..ed4cf92 100644
--- a/arch/powerpc/platforms/85xx/Kconfig
+++ b/arch/powerpc/platforms/85xx/Kconfig
@@ -26,6 +26,10 @@  config MPC8560_ADS
 	help
 	  This option enables support for the MPC 8560 ADS board
 
+config 85xx_HAVE_CAN_FLEXCAN
+	bool
+	select HAVE_CAN_FLEXCAN if NET && CAN
+
 config MPC85xx_CDS
 	bool "Freescale MPC85xx CDS"
 	select DEFAULT_UIMAGE
@@ -70,6 +74,8 @@  config MPC85xx_RDB
 config P1010_RDB
 	bool "Freescale P1010RDB"
 	select DEFAULT_UIMAGE
+	select 85xx_HAVE_CAN_FLEXCAN
+	select PPC_CLOCK if CAN_FLEXCAN
 	help
 	  This option enables support for the MPC85xx RDB (P1010 RDB) board
 
diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
index a971b32..64ad7a4 100644
--- a/arch/powerpc/platforms/85xx/Makefile
+++ b/arch/powerpc/platforms/85xx/Makefile
@@ -11,6 +11,7 @@  obj-$(CONFIG_MPC85xx_DS)  += mpc85xx_ds.o
 obj-$(CONFIG_MPC85xx_MDS) += mpc85xx_mds.o
 obj-$(CONFIG_MPC85xx_RDB) += mpc85xx_rdb.o
 obj-$(CONFIG_P1010_RDB)   += p1010rdb.o
+obj-$(CONFIG_PPC_CLOCK)   += clock.o
 obj-$(CONFIG_P1022_DS)    += p1022_ds.o
 obj-$(CONFIG_P1023_RDS)   += p1023_rds.o
 obj-$(CONFIG_P2040_RDB)   += p2040_rdb.o corenet_ds.o
diff --git a/arch/powerpc/platforms/85xx/clock.c b/arch/powerpc/platforms/85xx/clock.c
new file mode 100644
index 0000000..a25cbf3
--- /dev/null
+++ b/arch/powerpc/platforms/85xx/clock.c
@@ -0,0 +1,59 @@ 
+
+#include <linux/device.h>
+#include <linux/err.h>
+
+#include <asm/clk_interface.h>
+
+#include <sysdev/fsl_soc.h>
+
+/*
+ * p1010rdb needs to provide a clock source for the flexcan driver.
+ */
+struct clk {
+	unsigned long rate;
+} p1010_rdb_system_clock;
+
+static struct clk *p1010_rdb_clk_get(struct device *dev, const char *id)
+{
+	const char *dev_init_name;
+
+	if (!dev)
+		return ERR_PTR(-ENOENT);
+
+	/*
+	 * The can devices are named ffe1c000.can0 and ffe1d000.can1 on
+	 * the p1010rdb.  Check for the "can" portion of that name before
+	 * returning a clock source.
+	 */
+	dev_init_name = dev_name(dev);
+	if (strlen(dev_init_name) != 13)
+		return ERR_PTR(-ENOENT);
+	dev_init_name += 9;
+	if (strncmp(dev_init_name, "can", 3))
+		return ERR_PTR(-ENOENT);
+
+	return &p1010_rdb_system_clock;
+}
+
+static void p1010_rdb_clk_put(struct clk *clk)
+{
+	return;
+}
+
+static unsigned long p1010_rdb_clk_get_rate(struct clk *clk)
+{
+	return clk->rate;
+}
+
+static struct clk_interface p1010_rdb_clk_functions = {
+	.clk_get		= p1010_rdb_clk_get,
+	.clk_get_rate		= p1010_rdb_clk_get_rate,
+	.clk_put		= p1010_rdb_clk_put,
+};
+
+void __init p1010_rdb_clk_init(void)
+{
+	p1010_rdb_system_clock.rate = fsl_get_sys_freq();
+	clk_functions = p1010_rdb_clk_functions;
+}
+
diff --git a/arch/powerpc/platforms/85xx/p1010rdb.c b/arch/powerpc/platforms/85xx/p1010rdb.c
index d7387fa..d0afbf9 100644
--- a/arch/powerpc/platforms/85xx/p1010rdb.c
+++ b/arch/powerpc/platforms/85xx/p1010rdb.c
@@ -81,6 +81,15 @@  static void __init p1010_rdb_setup_arch(void)
 	printk(KERN_INFO "P1010 RDB board from Freescale Semiconductor\n");
 }
 
+extern void p1010_rdb_clk_init(void);
+
+static void __init p1010_rdb_init(void)
+{
+#ifdef CONFIG_PPC_CLOCK
+	p1010_rdb_clk_init();
+#endif
+}
+
 static struct of_device_id __initdata p1010rdb_ids[] = {
 	{ .type = "soc", },
 	{ .compatible = "soc", },
@@ -111,6 +120,7 @@  define_machine(p1010_rdb) {
 	.name			= "P1010 RDB",
 	.probe			= p1010_rdb_probe,
 	.setup_arch		= p1010_rdb_setup_arch,
+	.init			= p1010_rdb_init,
 	.init_IRQ		= p1010_rdb_pic_init,
 #ifdef CONFIG_PCI
 	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,