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

[U Bhaskar-B22300]

> -----Original Message-----
> From: Wolfgang Grandegger [mailto:wg at grandegger.com]
> Sent: Tuesday, August 09, 2011 2:03 PM
> To: U Bhaskar-B22300
> Cc: Marc Kleine-Budde; socketcan-core at lists.berlios.de;
> netdev at vger.kernel.org; Devicetree-discuss at lists.ozlabs.org
> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
> 
> Hi Bhaskar,
> 
> On 08/09/2011 09:57 AM, U Bhaskar-B22300 wrote:
> >
> >
> >> -----Original Message-----
> >> From: Marc Kleine-Budde [mailto:mkl at pengutronix.de]
> >> Sent: Tuesday, August 09, 2011 12:23 AM
> >> To: Wolfgang Grandegger
> >> Cc: socketcan-core at lists.berlios.de; netdev at vger.kernel.org; U
> >> Bhaskar- B22300
> >> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
> >>
> >> On 08/08/2011 05:33 PM, Wolfgang Grandegger wrote:
> >>>> ACK - The device tree bindings as in mainline's Documentation is a
> >> mess.
> >>>> If the powerpc guys are happy with a clock interfaces based
> >>>> approach somewhere in arch/ppc, I'm more than happy to remove:
> >>>> - fsl,flexcan-clock-source (not implemented, even in the fsl
> >>>> driver)
> > [Bhaskar]I have pushed the FlexCAN series of patches, It contains the
> > usage of all the fields posted in the FlexCAN bindings at
> > http://git.kernel.org/?p=linux/kernel/git/stable/linux-3.0.y.git;a=blo
> > b;f=Documentation/devicetree/bindings/net/can/fsl-flexcan.txt;h=1a729f
> > 089866259ef82d0db5893ff7a8c54d5ccf;hb=94ed5b4788a7cdbe68bc7cb8516972cb
> > ebdc8274
> 
> As Marc already pointed out, Robin already has a much more advanced patch
> stack in preparation. Especially your patches do not care about the
> already existing Flexcan core on the Freescale's ARM socks.
[Bhaskar] No, the patches are taking care of the existing ARM functionality.
	I have not tested on the ARM based board, but the patches are made in a 
      Manner that it should not break the ARM based functionality.
> 
> >>>>
> >>>> - fsl,flexcan-clock-divider \__ replace with code in arch/ppc, or
> >>>> - clock-frequency           /   a single clock-frequency attribute
> >>>
> >>> In the "net-next-2.6" tree there is also:
> >>>
> >>>  $ grep flexcan arch/powerpc/boots/dts/*.dts
> >>>   p1010rdb.dts:			fsl,flexcan-clock-source =
> "platform";
> >>>   p1010rdb.dts:			fsl,flexcan-clock-source =
> "platform";
> >>>   p1010si.dtsi:			compatible = "fsl,flexcan-v1.0";
> >>>   p1010si.dtsi:			fsl,flexcan-clock-divider = <2>;
> >>>   p1010si.dtsi:			compatible = "fsl,flexcan-v1.0";
> >>>   p1010si.dtsi:			fsl,flexcan-clock-divider = <2>;
> >>>
> >>> Especially the fsl,flexcan-clock-divider = <2>; might make people
> >>> think, that they could set something else.
> >>
> > [Bhaskar] As it is mentioned in the Flexcan bindings, the need of
> fsl,flexcan-clock-divider = <2>;
> > 	    But I kept it as "2" because FlexCan clock source is the
> platform clock and it is CCB/2
> > 	    If the "2" is misleading, the bindings can be changed or some
> text can be written to make the meaning of "2"
> >           Understandable , Please suggest ..
> 
> The clock source and frequency is fixed. Why do we need an extra
> properties for that. We have panned to remove these bogus bindings from
> the Linux kernel, which sneaked in *without* any review on the relevant
> mailing lists (at least I have not realized any posting). We do not think
> they are really needed. They just confuse the user. We also prefer to use
> the compatibility string "fsl,flexcan" instead "fsl,flexcan-v1.0". It's
> unusual to add a version number, which is  for the Flexcan on the PowerPC
> cores only, I assume, but there will be device tree for ARM soon. A
> proper compatibility string would be "fsl,p1010-flexcan" if we really
> need to distinguish.
> 
[Bhaskar] About clock source.. There can be two sources of clock for the CAN.
	Oscillator or the platform clock, but at present only platform clock is supported
	in P1010.If we remove the fsl,flexcan-clock-source property, we will lost the flexibility
	of changing the clock source ..
	   
          About clock-frequency... it is also not fixed. It depends on the platform clock which in turns
          Depends on the CCB clock. So it will be better to keep clock-frequency property which is getting fixed via u-boot.   	

	    Agreed on the discussion of changing "fsl,flexcan-v1.0" to "fsl,flexcan"
	
> Please join the discussion on the mailing list helping to get this driver
> mainline.
> 
> Thanks,
> 
> Wolfgang.