Re: [RFC 4/4] [powerpc] Implement a p1010rdb clock source.

6 messages, 4 authors, 2011-08-08 · open the first message on its own page

Re: [RFC 4/4] [powerpc] Implement a p1010rdb clock source.

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: 2011-08-06 13:58:51 

On 08/06/2011 06:05 AM, Robin Holt wrote:
flexcan driver needs the clk_get, clk_get_rate, etc functions
to work.  This patch provides the minimum functionality.
This patch has to go via the powerpc git tree. Added
linuxppc-dev@lists.ozlabs.org on CC.
quoted hunk
Signed-off-by: Robin Holt <redacted>
To: Marc Kleine-Budde <mkl@pengutronix.de>
To: Wolfgang Grandegger <redacted>
To: U Bhaskar-B22300 <redacted>
Cc: socketcan-core@lists.berlios.de
Cc: netdev@vger.kernel.org
---
 arch/powerpc/platforms/85xx/p1010rdb.c |   78 ++++++++++++++++++++++++++++++++
 1 files changed, 78 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/85xx/p1010rdb.c b/arch/powerpc/platforms/85xx/p1010rdb.c
index 3540a88..8f78ddd 100644
--- a/arch/powerpc/platforms/85xx/p1010rdb.c
+++ b/arch/powerpc/platforms/85xx/p1010rdb.c
@@ -28,6 +28,7 @@
 #include <asm/udbg.h>
 #include <asm/mpic.h>
 #include <asm/swiotlb.h>
+#include <asm/clk_interface.h>
 
 #include <sysdev/fsl_soc.h>
 #include <sysdev/fsl_pci.h>
@@ -164,6 +165,82 @@ static void __init p1010_rdb_setup_arch(void)
 	printk(KERN_INFO "P1010 RDB board from Freescale Semiconductor\n");
 }
 
+/*
+ * p1010rdb needs to provide a clock source for the flexcan driver.
+ */
+struct clk {
+	unsigned long rate;
+} p1010rdb_system_clk;
+
+static struct clk *p1010_rdb_clk_get(struct device *dev, const char *id)
+{
+	struct clk *clk;
+	u32 *of_property;
+	unsigned long clock_freq, clock_divider;
+	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);
+
+	of_property = (u32 *)of_get_property(dev->of_node, "clock_freq", NULL);
+	if (!of_property)
+		return ERR_PTR(-ENOENT);
+	clock_freq = *of_property;
+
+	of_property = (u32 *)of_get_property(dev->of_node,
+					     "fsl,flexcan-clock-divider", NULL);
+	if (!of_property)
+		return ERR_PTR(-ENOENT);
+	clock_divider = *of_property;
+
+	clk = kmalloc(sizeof(struct clk), GFP_KERNEL);
+	if (!clk)
+		return ERR_PTR(-ENOMEM);
+
+	clk->rate = DIV_ROUND_CLOSEST(clock_freq / clock_divider, 1000);
+	clk->rate *= 1000;
+
+	return clk;
+}
+
+static void p1010_rdb_clk_put(struct clk *clk)
+{
+	kfree(clk);
+}
+
+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,
+};
+
+static void __init p1010_rdb_clk_init(void)
+{
+	clk_functions = p1010_rdb_clk_functions;
+}
+
+static void __init p1010_rdb_init(void)
+{
+	p1010_rdb_clk_init();
+}
+
 static struct of_device_id __initdata p1010rdb_ids[] = {
 	{ .type = "soc", },
 	{ .compatible = "soc", },
@@ -195,6 +272,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,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

Re: [RFC 4/4] [powerpc] Implement a p1010rdb clock source.

From: Kumar Gala <hidden>
Date: 2011-08-06 16:53:01 

On Aug 6, 2011, at 8:58 AM, Marc Kleine-Budde wrote:

On 08/06/2011 06:05 AM, Robin Holt wrote:
quoted
flexcan driver needs the clk_get, clk_get_rate, etc functions
to work.  This patch provides the minimum functionality.
=20
This patch has to go via the powerpc git tree. Added
linuxppc-dev@lists.ozlabs.org on CC.
=20
quoted
Signed-off-by: Robin Holt <redacted>
To: Marc Kleine-Budde <mkl@pengutronix.de>
To: Wolfgang Grandegger <redacted>
To: U Bhaskar-B22300 <redacted>
Cc: socketcan-core@lists.berlios.de
Cc: netdev@vger.kernel.org
---
arch/powerpc/platforms/85xx/p1010rdb.c |   78 =
++++++++++++++++++++++++++++++++
quoted
1 files changed, 78 insertions(+), 0 deletions(-)
NAK.

This doesn't look right at all.  We should be doing something based on =
the device tree node that isn't board specific.

I believe Bhaskar has a version of flexcan support that he's been =
working on cleanup up for upstream.

- k=

Re: [RFC 4/4] [powerpc] Implement a p1010rdb clock source.

From: Robin Holt <hidden>
Date: 2011-08-06 20:56:02 

On Sat, Aug 06, 2011 at 11:52:45AM -0500, Kumar Gala wrote:
On Aug 6, 2011, at 8:58 AM, Marc Kleine-Budde wrote:
quoted
On 08/06/2011 06:05 AM, Robin Holt wrote:
quoted
flexcan driver needs the clk_get, clk_get_rate, etc functions
to work.  This patch provides the minimum functionality.
This patch has to go via the powerpc git tree. Added
linuxppc-dev@lists.ozlabs.org on CC.
quoted
Signed-off-by: Robin Holt <redacted>
To: Marc Kleine-Budde <mkl@pengutronix.de>
To: Wolfgang Grandegger <redacted>
To: U Bhaskar-B22300 <redacted>
Cc: socketcan-core@lists.berlios.de
Cc: netdev@vger.kernel.org
---
arch/powerpc/platforms/85xx/p1010rdb.c |   78 ++++++++++++++++++++++++++++++++
1 files changed, 78 insertions(+), 0 deletions(-)
NAK.

This doesn't look right at all.  We should be doing something based on the device tree node that isn't board specific.

I believe Bhaskar has a version of flexcan support that he's been working on cleanup up for upstream.
That version may be similar to what is in the freescale BSP which puts
the clock functions inside flexcan.c

The powerpc arch already provides a means for individual boards to provide
the clock functions.  I am not posting this patch here for acceptance
for powerpc and I am sure I will get feedback there when I post to
their mailing list.  I am posting it here only to show that the flexcan
developers earlier assertion that this can and should be done in the arch
tree is correct and will work for the p1010 assuming we can get changes
into the arch/powerpc directory to implement these clk_* functions.

Thanks,
Robin

Re: [RFC 4/4] [powerpc] Implement a p1010rdb clock source.

From: Kumar Gala <hidden>
Date: 2011-08-06 20:59:58 

On Aug 6, 2011, at 3:50 PM, Robin Holt wrote:

On Sat, Aug 06, 2011 at 11:52:45AM -0500, Kumar Gala wrote:
quoted
=20
On Aug 6, 2011, at 8:58 AM, Marc Kleine-Budde wrote:
=20
quoted
On 08/06/2011 06:05 AM, Robin Holt wrote:
quoted
flexcan driver needs the clk_get, clk_get_rate, etc functions
to work.  This patch provides the minimum functionality.
=20
This patch has to go via the powerpc git tree. Added
linuxppc-dev@lists.ozlabs.org on CC.
=20
quoted
Signed-off-by: Robin Holt <redacted>
To: Marc Kleine-Budde <mkl@pengutronix.de>
To: Wolfgang Grandegger <redacted>
To: U Bhaskar-B22300 <redacted>
Cc: socketcan-core@lists.berlios.de
Cc: netdev@vger.kernel.org
---
arch/powerpc/platforms/85xx/p1010rdb.c |   78 =
++++++++++++++++++++++++++++++++
quoted
quoted
quoted
1 files changed, 78 insertions(+), 0 deletions(-)
=20
NAK.
=20
This doesn't look right at all.  We should be doing something based =
on the device tree node that isn't board specific.
quoted
=20
I believe Bhaskar has a version of flexcan support that he's been =
working on cleanup up for upstream.
=20
That version may be similar to what is in the freescale BSP which puts
the clock functions inside flexcan.c
=20
The powerpc arch already provides a means for individual boards to =
provide
the clock functions.  I am not posting this patch here for acceptance
for powerpc and I am sure I will get feedback there when I post to
their mailing list.  I am posting it here only to show that the =
flexcan
developers earlier assertion that this can and should be done in the =
arch
tree is correct and will work for the p1010 assuming we can get =
changes
into the arch/powerpc directory to implement these clk_* functions.
My point is that I don't think they should live in the arch code.  The =
clk_* functions you want to implement are tied more the FlexCAN IP than =
anything arch specific.  As such I believe they should be in the driver.

For example when FSL has a P9999 with FlexCAN on it, we should NOT have =
to add any arch code to support it.

- k=

Re: [RFC 4/4] [powerpc] Implement a p1010rdb clock source.

From: Wolfgang Grandegger <hidden>
Date: 2011-08-08 08:49:14 

On 08/06/2011 10:59 PM, Kumar Gala wrote:
On Aug 6, 2011, at 3:50 PM, Robin Holt wrote:
quoted
On Sat, Aug 06, 2011 at 11:52:45AM -0500, Kumar Gala wrote:
quoted
On Aug 6, 2011, at 8:58 AM, Marc Kleine-Budde wrote:
quoted
On 08/06/2011 06:05 AM, Robin Holt wrote:
quoted
flexcan driver needs the clk_get, clk_get_rate, etc functions
to work.  This patch provides the minimum functionality.
This patch has to go via the powerpc git tree. Added
linuxppc-dev@lists.ozlabs.org on CC.
quoted
Signed-off-by: Robin Holt <redacted>
To: Marc Kleine-Budde <mkl@pengutronix.de>
To: Wolfgang Grandegger <redacted>
To: U Bhaskar-B22300 <redacted>
Cc: socketcan-core@lists.berlios.de
Cc: netdev@vger.kernel.org
---
arch/powerpc/platforms/85xx/p1010rdb.c |   78 ++++++++++++++++++++++++++++++++
1 files changed, 78 insertions(+), 0 deletions(-)
NAK.

This doesn't look right at all.  We should be doing something based on the device tree node that isn't board specific.

I believe Bhaskar has a version of flexcan support that he's been working on cleanup up for upstream.
That version may be similar to what is in the freescale BSP which puts
the clock functions inside flexcan.c

The powerpc arch already provides a means for individual boards to provide
the clock functions.  I am not posting this patch here for acceptance
for powerpc and I am sure I will get feedback there when I post to
their mailing list.  I am posting it here only to show that the flexcan
developers earlier assertion that this can and should be done in the arch
tree is correct and will work for the p1010 assuming we can get changes
into the arch/powerpc directory to implement these clk_* functions.
My point is that I don't think they should live in the arch code.  The clk_* functions you want to implement are tied more the FlexCAN IP than anything arch specific.  As such I believe they should be in the driver.

For example when FSL has a P9999 with FlexCAN on it, we should NOT have to add any arch code to support it.
The Flexcan is found on ARM and now also on PowerPC SOCs. My current
understanding is that the ability to set the clock source and divider is
only available on PowerPC SOCs and therefore it's clearly arch specific
and should go to arch/powerpc/sysdev/fsl_soc.c if it's common for all
PowerPC platforms. What do you think?

Wolfgang.

Re: [RFC 4/4] [powerpc] Implement a p1010rdb clock source.

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: 2011-08-08 09:33:12 

On 08/08/2011 10:49 AM, Wolfgang Grandegger wrote:
On 08/06/2011 10:59 PM, Kumar Gala wrote:
quoted
On Aug 6, 2011, at 3:50 PM, Robin Holt wrote:
quoted
On Sat, Aug 06, 2011 at 11:52:45AM -0500, Kumar Gala wrote:
quoted
On Aug 6, 2011, at 8:58 AM, Marc Kleine-Budde wrote:
quoted
On 08/06/2011 06:05 AM, Robin Holt wrote:
quoted
flexcan driver needs the clk_get, clk_get_rate, etc functions
to work.  This patch provides the minimum functionality.
This patch has to go via the powerpc git tree. Added
linuxppc-dev@lists.ozlabs.org on CC.
quoted
Signed-off-by: Robin Holt <redacted>
To: Marc Kleine-Budde <mkl@pengutronix.de>
To: Wolfgang Grandegger <redacted>
To: U Bhaskar-B22300 <redacted>
Cc: socketcan-core@lists.berlios.de
Cc: netdev@vger.kernel.org
---
arch/powerpc/platforms/85xx/p1010rdb.c |   78 ++++++++++++++++++++++++++++++++
1 files changed, 78 insertions(+), 0 deletions(-)
NAK.

This doesn't look right at all.  We should be doing something based on the device tree node that isn't board specific.

I believe Bhaskar has a version of flexcan support that he's been working on cleanup up for upstream.
That version may be similar to what is in the freescale BSP which puts
the clock functions inside flexcan.c

The powerpc arch already provides a means for individual boards to provide
the clock functions.  I am not posting this patch here for acceptance
for powerpc and I am sure I will get feedback there when I post to
their mailing list.  I am posting it here only to show that the flexcan
developers earlier assertion that this can and should be done in the arch
tree is correct and will work for the p1010 assuming we can get changes
into the arch/powerpc directory to implement these clk_* functions.
My point is that I don't think they should live in the arch code.  The clk_* functions you want to implement are tied more the FlexCAN IP than anything arch specific.  As such I believe they should be in the driver.

For example when FSL has a P9999 with FlexCAN on it, we should NOT have to add any arch code to support it.
The Flexcan is found on ARM and now also on PowerPC SOCs. My current
understanding is that the ability to set the clock source and divider is
only available on PowerPC SOCs and therefore it's clearly arch specific
and should go to arch/powerpc/sysdev/fsl_soc.c if it's common for all
PowerPC platforms. What do you think?
There is a bit in the CAN-Controller that selects the clock (at least on
arm). The current driver just supports the bus clock. Support for the
other, the oscillator clock, has not been implemented so far.

IIRC the oscillator clock has a frequency that results in worse standard
timing than the bus clock.

cheers, Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help