From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wolfgang Grandegger Subject: Re: [PATCH v11 4/5] powerpc: Add flexcan device support for p1010rdb. Date: Thu, 11 Aug 2011 09:35:39 +0200 Message-ID: <4E43864B.2050302@grandegger.com> References: <1312993670-23999-1-git-send-email-holt@sgi.com> <1312993670-23999-5-git-send-email-holt@sgi.com> <8E5FA886-038D-4DF4-8A54-DD60188A21A2@kernel.crashing.org> <4E42CB01.7030700@grandegger.com> <20110811035620.GB4926@sgi.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, U Bhaskar-B22300 , Kumar Gala , socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org, Marc Kleine-Budde , PPC list To: Robin Holt Return-path: In-Reply-To: <20110811035620.GB4926-sJ/iWh9BUns@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: socketcan-core-bounces-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org Errors-To: socketcan-core-bounces-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org List-Id: netdev.vger.kernel.org On 08/11/2011 05:56 AM, Robin Holt wrote: > On Wed, Aug 10, 2011 at 08:16:33PM +0200, Wolfgang Grandegger wrote: >> On 08/10/2011 07:01 PM, Kumar Gala wrote: >>> >>> On Aug 10, 2011, at 11:27 AM, Robin Holt wrote: >>> >>>> I added a simple clock source for the p1010rdb so the flexcan driver >>>> could determine a clock frequency. The p1010 flexcan device only has >>>> an oscillator of system bus frequency divided by 2. >>>> >>>> Signed-off-by: Robin Holt >>>> Acked-by: Marc Kleine-Budde , >>>> Acked-by: Wolfgang Grandegger , >>>> Cc: U Bhaskar-B22300 >>>> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org, >>>> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, >>>> Cc: PPC list >>>> Cc: Kumar Gala >>>> --- >>>> arch/powerpc/platforms/85xx/Kconfig | 2 + >>>> arch/powerpc/platforms/85xx/Makefile | 2 + >>>> arch/powerpc/platforms/85xx/clock.c | 52 ++++++++++++++++++++++++++++++++ >>>> arch/powerpc/platforms/85xx/p1010rdb.c | 8 +++++ >>>> 4 files changed, 64 insertions(+), 0 deletions(-) >>>> create mode 100644 arch/powerpc/platforms/85xx/clock.c >>> >>> I dont understand how mpc85xx_clk_functions() ends up being associated with the frequency the flexcan is running at. >> >> The function mpc85xx_clk_get_rate() returns "fsl_get_sys_freq() / 2" for >> Flexcan devices. >> >>> This either seems to global or I'm missing something. >> >> This patch extends the existing Flexcan platform driver for ARM for the >> PowerPC using the device tree. Due to the nice integration of the device >> tree (of-platform) into the platform driver and devices, the difference >> are quite small (see patches 1..3). Apart from the endianess issue, only >> the clock needs to be handled in a common way. As ARM already uses the >> clk interface, we found it straight-forward to implement it for the >> P1010, or more general for the 85xx, as well, instead of using an >> additional helper function. >> >>> I still think the clk / freq info should be in the device tree and handled in the driver and NOT arch/powerpc platform code. >> >> If I understand you correctly, you want the boot-loader to provide the >> relevant information by fixing up the device tree, which then can be >> handled arch-independently by the driver, right? > > Marc and Wolfgang, > > This is a very early swag at this which I worked up while driving home from dinner > this evening. It works with my current config, but that includes at least one > bogus patch. I will have to do more testing tomorrow. For now, it is something > to ponder. Yes, that's what Kumar is proposing. Fine for me with the P1010. It just needs some proper U-Boot implementation. > Thanks, > Robin > ------------------------------------------------------------------------ > > flexcan: Prefer device tree clock frequency if available. > > If our CAN device's device tree node has a clock-frequency property, > then use that value for the can devices clock frequency. If not, fall > back to asking the platform/mach code for the clock frequency associated > with the flexcan device. > > Too-early-to-be-signed-off-by: Robin Holt > > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c > index 662f832..d6a87c9 100644 > --- a/drivers/net/can/flexcan.c > +++ b/drivers/net/can/flexcan.c > @@ -33,6 +33,7 @@ > #include > #include > #include > +#include > #include > > #define DRV_NAME "flexcan" > @@ -929,12 +930,25 @@ static int __devinit flexcan_probe(struct platform_device *pdev) > void __iomem *base; > resource_size_t mem_size; > int err, irq; > + u32 clock_freq = 0; > > - clk = clk_get(&pdev->dev, NULL); > - if (IS_ERR(clk)) { > - dev_err(&pdev->dev, "no clock defined\n"); > - err = PTR_ERR(clk); > - goto failed_clock; > + if (pdev->dev.of_node) { > + u32 *clock_freq_p; Should be: const u32 *clock_freq_p; > + > + clk = NULL; > + clock_freq_p = (u32 *)of_get_property(pdev->dev.of_node, "clock-frequency", NULL); No cast please. > + if (clock_freq_p) > + clock_freq = *clock_freq_p; > + } Wolfgang.