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

On 08/08/2011 01:31 PM, Robin Holt wrote:

On Mon, Aug 08, 2011 at 10:37:58AM +0200, Wolfgang Grandegger wrote:

quoted

On 08/06/2011 04:34 PM, Robin Holt wrote:

quoted

flexcan driver needs the clk_get, clk_get_rate, etc functions
to work.  This patch provides the minimum functionality.

This needs some more general thoughts... apart from the question where
the code should go.

Like for the MSCAN on the MPC5200, the user should be *able* to select
an appropriate clock source and divider via DTS node properties.
Currently it seems, that the DTS properties must match some
pre-configured values, most likely set by the boot loader. Please
correct me if I'm wrong. For me this is generic and should go into the
Flexcan driver. From there, a platform specific function, e.g.
flexcan_set_clock() might be called.

OK.  Dug a bit more.  The p1010 built-in clocksource seems to be the
periphereal clock frequency which is system bus frequency divided
by 2.  The clock source can not be changed, but the clock divider can
by freezing the interface and setting the CTRL register.  This appears
to only be done by the boot loader.  I do not see why we can not leave

And likely Freescale's bootloader does also fixup the DTS Flexcan node.
Ah, oh, there's already someting in the mainline U-BOOT:

commit 65bb8b060a873fa4f5188f2951081f6011259614
Author: Bhaskar Upadhaya [off-list ref]
Date:   Fri Mar 4 20:27:58 2011 +0530

    powerpc/85xx: Fix up clock_freq property in CAN node of dts

    Fix up the device tree property associated with the Flexcan clock
    frequency. This property is used to calculate the bit timing parameters
    for Flexcan.

    Signed-off-by: Bhaskar Upadhaya [off-list ref]
    Signed-off-by: Kumar Gala [off-list ref]

that functionality in the boot loader and then go back to a variation
on my earlier flexcan_clk_* patch.  Is that close to the direction you
think we should go or have I completely misunderstood your wishes?

The boot loader might not chose the optimum clock source and frequency,
which might even be application dependent. Therefore it would be nice to
allow the user to change it if necessary. Some CAN interfaces do even
allow to use an external clock source. The main question is where we add
that functionality. As more as I think of it, the clock interface would
not be that bad, especially if it's available.

Furthermore, if the bootloader sets the clock source and divider, we do
not need device tree properties for it. A simply register lookup would
reveal what values are used. We may just need the input clock source.

Wolfgang.