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

[Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>]

On 08/09/2011 11:34 AM, U Bhaskar-B22300 wrote:
> 
> 
>> -----Original Message-----
>> From: Marc Kleine-Budde [mailto:mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
>> Sent: Tuesday, August 09, 2011 1:43 PM
>> To: U Bhaskar-B22300
>> Cc: Wolfgang Grandegger; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
>> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
>>
>> On 08/09/2011 09:57 AM, U Bhaskar-B22300 wrote:
>>>> 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
>>
>> I've commented the patches. They are in a very bad shape. Please test
>> Robin's patches.
>>
>>>>>>
>>>>>> - 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 devider is crap. Why not specify the clockrate that goes into
>> the flexcan core?
> [Bhaskar] The reason why I placed the "fsl,flexcan-clock-divider" property is just because the earlier implementations
> 		 Of CAN also follows the same approach. Please see below the approach of mscan.
>                 compatible = "fsl,mpc5121-mscan";
>                 interrupts = <13 0x8>;
>                 interrupt-parent = <&ipic>;
>                 reg = <0x1380 0x80>;
>                 fsl,mscan-clock-source = "ref";
>                 fsl,mscan-clock-divider = <3>;
>         };
> 	If you want we can remove the fsl,flexcan-clock-divider property. Please comment ..

For that platform the user can *change* these properties so select
another clock-source or clock-divider. This is not the case for the
P1010. Therefore these properties are not needed.

Wolfgang.