> -----Original Message----- > From: Robin Holt [mailto:h...@sgi.com] > Sent: Tuesday, August 09, 2011 7:06 PM > To: Wolfgang Grandegger > Cc: Robin Holt; U Bhaskar-B22300; socketcan-c...@lists.berlios.de; > net...@vger.kernel.org; Devicetree-discuss@lists.ozlabs.org; Marc Kleine- > Budde > Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source. > > On Tue, Aug 09, 2011 at 03:03:50PM +0200, Wolfgang Grandegger wrote: > > On 08/09/2011 02:49 PM, Robin Holt wrote: > > > On Tue, Aug 09, 2011 at 12:41:39PM +0000, U Bhaskar-B22300 wrote: > > >> > > >> > > >>> -----Original Message----- > > >>> From: Wolfgang Grandegger [mailto:w...@grandegger.com] > > >>> Sent: Tuesday, August 09, 2011 4:19 PM > > >>> To: U Bhaskar-B22300 > > >>> Cc: Marc Kleine-Budde; socketcan-c...@lists.berlios.de; > > >>> net...@vger.kernel.org; Devicetree-discuss@lists.ozlabs.org > > >>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source. > > >>> > > >>> On 08/09/2011 11:27 AM, U Bhaskar-B22300 wrote: > > >>>> > > >>>> > > >>>>> -----Original Message----- > > >>>>> From: Wolfgang Grandegger [mailto:w...@grandegger.com] > > >>>>> Sent: Tuesday, August 09, 2011 2:03 PM > > >>>>> To: U Bhaskar-B22300 > > >>>>> Cc: Marc Kleine-Budde; socketcan-c...@lists.berlios.de; > > >>>>> net...@vger.kernel.org; Devicetree-discuss@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:m...@pengutronix.de] > > >>>>>>> Sent: Tuesday, August 09, 2011 12:23 AM > > >>>>>>> To: Wolfgang Grandegger > > >>>>>>> Cc: socketcan-c...@lists.berlios.de; net...@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.gi > > >>>>>> t;a=b > > >>>>>> lo > > >>>>>> b;f=Documentation/devicetree/bindings/net/can/fsl-flexcan.txt;h > > >>>>>> =1a72 > > >>>>>> 9f > > >>>>>> 089866259ef82d0db5893ff7a8c54d5ccf;hb=94ed5b4788a7cdbe68bc7cb85 > > >>>>>> 16972 > > >>>>>> cb > > >>>>>> 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. > > >>> > > >>> The frequency is fixed to CCB-frequency / 2. Will that ever > > >>> change? What can we expect from future Flexcan hardware? Will it > > >>> support further clock sources? > > >> [Bhaskar] Yes the frequency will always be CCB-frequency/2.Even if > the CCB gets changed that will be taken care by the u-boot fixup code for > > >> clock-frequency. clock-frequency is not filled by somebody in the > dts file. It will be done by u-boot. > > >> For clock source,I can't say right now, that's why I have kept a > property for this in the can node. So that in future, we need to fill it > > >> appropriately > > > > > > Speaking of the dts file, I have left the p1010si.dtsi file with the > > > fsl,flexcan-v1.0 .compatible definition. The flexcan folks (IIRC > > > Wolfgang) objected to that as it does not follow the standard which > > > should be just fsl,flexcan. > > > > > > How would you like to change that? Should I add it as part of this > > > patch, add another patch to the series, or let you take care of it? > > > > > > Also, I assume the uboot project will need to be changed as well to > > > reflect the corrected name. > > > > I think you should provide patches within this series to cleanup the > > obsolete stuff, dts and binding doc. > > It reads to me that the binding doc now reduces just the required > properties. Should I remove the file entirely? [Bhaskar] I think the binding doc should atleast be present with the required properties to give the clarity about the CAN functionality can0@1c000 { compatible = "fsl,flexcan"; reg = <0x1c000 0x1000>; interrupts = <48 0x2>; interrupt-parent = <&mpic>; clock-frequency = <fixed by u-boot>; }; > > Robin
_______________________________________________ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
