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-core@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.git;a=blob;f=Documentation/devicetree/bindings/net/can/fsl-flexcan.txt;h=1a729f089866259ef82d0db5893ff7a8c54d5ccf;hb=94ed5b4788a7cdbe68bc7cb8516972cbebdc8274 >
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. >>>> >>>> - 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. Please join the discussion on the mailing list helping to get this driver mainline. Thanks, Wolfgang. _______________________________________________ Socketcan-core mailing list Socketcan-core@lists.berlios.de https://lists.berlios.de/mailman/listinfo/socketcan-core