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

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

[Wolfgang Grandegger]

Hi Bhaskar,

On 08/09/2011 09:57 AM, U Bhaskar-B22300 wrote:
> 
> 
>> -----Original Message-----
>> From: Marc Kleine-Budde [mailto:mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
>> Sent: Tuesday, August 09, 2011 12:23 AM
>> To: Wolfgang Grandegger
>> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.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.

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

[U Bhaskar-B22300]

> -----Original Message-----
> From: Wolfgang Grandegger [mailto:wg@grandegger.com]
> Sent: Tuesday, August 09, 2011 2:03 PM
> To: U Bhaskar-B22300
> Cc: Marc Kleine-Budde; socketcan-core@lists.berlios.de;
> netdev@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:mkl@pengutronix.de]
> >> Sent: Tuesday, August 09, 2011 12:23 AM
> >> To: Wolfgang Grandegger
> >> Cc: socketcan-core@lists.berlios.de; netdev@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.

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

[Wolfgang Grandegger]

On 08/09/2011 11:27 AM, U Bhaskar-B22300 wrote:
> 
> 
>> -----Original Message-----
>> From: Wolfgang Grandegger [mailto:wg@grandegger.com]
>> Sent: Tuesday, August 09, 2011 2:03 PM
>> To: U Bhaskar-B22300
>> Cc: Marc Kleine-Budde; socketcan-core@lists.berlios.de;
>> netdev@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:mkl@pengutronix.de]
>>>> Sent: Tuesday, August 09, 2011 12:23 AM
>>>> To: Wolfgang Grandegger
>>>> Cc: socketcan-core@lists.berlios.de; netdev@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.   	

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?

Wolfgang.

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

[U Bhaskar-B22300]

> -----Original Message-----
> From: Wolfgang Grandegger [mailto:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
> Sent: Tuesday, August 09, 2011 4:19 PM
> To: U Bhaskar-B22300
> Cc: Marc Kleine-Budde; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
> >> Sent: Tuesday, August 09, 2011 2:03 PM
> >> To: U Bhaskar-B22300
> >> Cc: Marc Kleine-Budde; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
> >> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
> >>>> Sent: Tuesday, August 09, 2011 12:23 AM
> >>>> To: Wolfgang Grandegger
> >>>> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.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=b
> >>> lo
> >>> b;f=Documentation/devicetree/bindings/net/can/fsl-flexcan.txt;h=1a72
> >>> 9f
> >>> 089866259ef82d0db5893ff7a8c54d5ccf;hb=94ed5b4788a7cdbe68bc7cb8516972
> >>> 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
> 
> Wolfgang.

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

[Robin Holt]

On Tue, Aug 09, 2011 at 12:41:39PM +0000, U Bhaskar-B22300 wrote:
> 
> 
> > -----Original Message-----
> > From: Wolfgang Grandegger [mailto:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
> > Sent: Tuesday, August 09, 2011 4:19 PM
> > To: U Bhaskar-B22300
> > Cc: Marc Kleine-Budde; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
> > netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
> > >> Sent: Tuesday, August 09, 2011 2:03 PM
> > >> To: U Bhaskar-B22300
> > >> Cc: Marc Kleine-Budde; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
> > >> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
> > >>>> Sent: Tuesday, August 09, 2011 12:23 AM
> > >>>> To: Wolfgang Grandegger
> > >>>> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.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=b
> > >>> lo
> > >>> b;f=Documentation/devicetree/bindings/net/can/fsl-flexcan.txt;h=1a72
> > >>> 9f
> > >>> 089866259ef82d0db5893ff7a8c54d5ccf;hb=94ed5b4788a7cdbe68bc7cb8516972
> > >>> 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.

Thanks,
Robin

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

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 1077 bytes --]

Bhaskar,

On 08/09/2011 02:41 PM, U Bhaskar-B22300 wrote:
>> 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

are you actually following the discussion about Robin's patches? Robin
has provided patch that work without the clock frequency in the DT at all..

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

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

[U Bhaskar-B22300]

> -----Original Message-----
> From: Marc Kleine-Budde [mailto:mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
> Sent: Tuesday, August 09, 2011 6:21 PM
> To: U Bhaskar-B22300
> Cc: Wolfgang Grandegger; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> Subject: Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
> 
> Bhaskar,
> 
> On 08/09/2011 02:41 PM, U Bhaskar-B22300 wrote:
> >> 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
> 
> are you actually following the discussion about Robin's patches? Robin
> has provided patch that work without the clock frequency in the DT at
> all..
[Bhaskar] Hi Marc,
	Yes I am following the discussion on Robin's patch. Just going tru the patches. I will respond
> 
> Marc
> 
> --
> Pengutronix e.K.                  | Marc Kleine-Budde           |
> Industrial Linux Solutions        | Phone: +49-231-2826-924     |
> Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

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

[Wolfgang Grandegger]

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:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
>>> Sent: Tuesday, August 09, 2011 4:19 PM
>>> To: U Bhaskar-B22300
>>> Cc: Marc Kleine-Budde; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
>>> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
>>>>> Sent: Tuesday, August 09, 2011 2:03 PM
>>>>> To: U Bhaskar-B22300
>>>>> Cc: Marc Kleine-Budde; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
>>>>> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
>>>>>>> Sent: Tuesday, August 09, 2011 12:23 AM
>>>>>>> To: Wolfgang Grandegger
>>>>>>> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.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=b
>>>>>> lo
>>>>>> b;f=Documentation/devicetree/bindings/net/can/fsl-flexcan.txt;h=1a72
>>>>>> 9f
>>>>>> 089866259ef82d0db5893ff7a8c54d5ccf;hb=94ed5b4788a7cdbe68bc7cb8516972
>>>>>> 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.

Wolfgang.

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

[Robin Holt]

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:wg@grandegger.com]
> >>> Sent: Tuesday, August 09, 2011 4:19 PM
> >>> To: U Bhaskar-B22300
> >>> Cc: Marc Kleine-Budde; socketcan-core@lists.berlios.de;
> >>> netdev@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:wg@grandegger.com]
> >>>>> Sent: Tuesday, August 09, 2011 2:03 PM
> >>>>> To: U Bhaskar-B22300
> >>>>> Cc: Marc Kleine-Budde; socketcan-core@lists.berlios.de;
> >>>>> netdev@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:mkl@pengutronix.de]
> >>>>>>> Sent: Tuesday, August 09, 2011 12:23 AM
> >>>>>>> To: Wolfgang Grandegger
> >>>>>>> Cc: socketcan-core@lists.berlios.de; netdev@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=b
> >>>>>> lo
> >>>>>> b;f=Documentation/devicetree/bindings/net/can/fsl-flexcan.txt;h=1a72
> >>>>>> 9f
> >>>>>> 089866259ef82d0db5893ff7a8c54d5ccf;hb=94ed5b4788a7cdbe68bc7cb8516972
> >>>>>> 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.

Will do.

Robin

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

[Wolfgang Grandegger]

On 08/09/2011 02:41 PM, U Bhaskar-B22300 wrote:
> 
> 
>> -----Original Message-----
>> From: Wolfgang Grandegger [mailto:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
...
>> 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.

U-Boot fills in "clock_freq", anyway...

> 	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

This just confirms that we currently can live without clock-frequency,
clock-source and clock-divider properties. If it's really required
sometime in the future, we can add it, I think.

Wolfgang.

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

[Robin Holt]

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:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
> >>> Sent: Tuesday, August 09, 2011 4:19 PM
> >>> To: U Bhaskar-B22300
> >>> Cc: Marc Kleine-Budde; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
> >>> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
> >>>>> Sent: Tuesday, August 09, 2011 2:03 PM
> >>>>> To: U Bhaskar-B22300
> >>>>> Cc: Marc Kleine-Budde; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
> >>>>> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
> >>>>>>> Sent: Tuesday, August 09, 2011 12:23 AM
> >>>>>>> To: Wolfgang Grandegger
> >>>>>>> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.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=b
> >>>>>> lo
> >>>>>> b;f=Documentation/devicetree/bindings/net/can/fsl-flexcan.txt;h=1a72
> >>>>>> 9f
> >>>>>> 089866259ef82d0db5893ff7a8c54d5ccf;hb=94ed5b4788a7cdbe68bc7cb8516972
> >>>>>> 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?

Robin

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

[U Bhaskar-B22300]

> -----Original Message-----
> From: Robin Holt [mailto:holt-sJ/iWh9BUns@public.gmane.org]
> Sent: Tuesday, August 09, 2011 7:06 PM
> To: Wolfgang Grandegger
> Cc: Robin Holt; U Bhaskar-B22300; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
> > >>> Sent: Tuesday, August 09, 2011 4:19 PM
> > >>> To: U Bhaskar-B22300
> > >>> Cc: Marc Kleine-Budde; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
> > >>> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
> > >>>>> Sent: Tuesday, August 09, 2011 2:03 PM
> > >>>>> To: U Bhaskar-B22300
> > >>>>> Cc: Marc Kleine-Budde; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
> > >>>>> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
> > >>>>>>> Sent: Tuesday, August 09, 2011 12:23 AM
> > >>>>>>> To: Wolfgang Grandegger
> > >>>>>>> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.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

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

[Robin Holt]

On Tue, Aug 09, 2011 at 01:44:16PM +0000, U Bhaskar-B22300 wrote:
> 
> 
> > -----Original Message-----
> > From: Robin Holt [mailto:holt-sJ/iWh9BUns@public.gmane.org]
> > Sent: Tuesday, August 09, 2011 7:06 PM
> > To: Wolfgang Grandegger
> > Cc: Robin Holt; U Bhaskar-B22300; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
> > netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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:
...
> > > > 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>;
>          	}; 

I am not sure what clarity we get for it.  Here it is as a work in progress:


CAN Device Tree Bindings
------------------------
2011 Freescale Semiconductor, Inc.

fsl,flexcan nodes
-----------------------
Only the required compatible-, reg- and interrupt-properties are supported.

Examples:
	can0@1c000 {
		compatible = "fsl,flexcan";
		reg = <0x1c000 0x1000>;
		interrupts = <48 0x2>;
		interrupt-parent = <&mpic>;
	};

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

[Wolfgang Grandegger]

On 08/09/2011 03:44 PM, U Bhaskar-B22300 wrote:
> 
> 
>> -----Original Message-----
>> From: Robin Holt [mailto:holt@sgi.com]
>> Sent: Tuesday, August 09, 2011 7:06 PM
>> To: Wolfgang Grandegger
>> Cc: Robin Holt; U Bhaskar-B22300; socketcan-core@lists.berlios.de;
>> netdev@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:wg@grandegger.com]
>>>>>> Sent: Tuesday, August 09, 2011 4:19 PM
>>>>>> To: U Bhaskar-B22300
>>>>>> Cc: Marc Kleine-Budde; socketcan-core@lists.berlios.de;
>>>>>> netdev@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:wg@grandegger.com]
>>>>>>>> Sent: Tuesday, August 09, 2011 2:03 PM
>>>>>>>> To: U Bhaskar-B22300
>>>>>>>> Cc: Marc Kleine-Budde; socketcan-core@lists.berlios.de;
>>>>>>>> netdev@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:mkl@pengutronix.de]
>>>>>>>>>> Sent: Tuesday, August 09, 2011 12:23 AM
>>>>>>>>>> To: Wolfgang Grandegger
>>>>>>>>>> Cc: socketcan-core@lists.berlios.de; netdev@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>;
>          	}; 

Yes, I also find the introduction is quite useful, with some related
correction.

Wolfgang.

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

[Robin Holt]

On Tue, Aug 09, 2011 at 04:03:38PM +0200, Wolfgang Grandegger wrote:
> On 08/09/2011 03:44 PM, U Bhaskar-B22300 wrote:
> > 
> > 
> >> -----Original Message-----
> >> From: Robin Holt [mailto:holt@sgi.com]
> >> Sent: Tuesday, August 09, 2011 7:06 PM
> >> To: Wolfgang Grandegger
> >> Cc: Robin Holt; U Bhaskar-B22300; socketcan-core@lists.berlios.de;
> >> netdev@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:wg@grandegger.com]
> >>>>>> Sent: Tuesday, August 09, 2011 4:19 PM
> >>>>>> To: U Bhaskar-B22300
> >>>>>> Cc: Marc Kleine-Budde; socketcan-core@lists.berlios.de;
> >>>>>> netdev@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:wg@grandegger.com]
> >>>>>>>> Sent: Tuesday, August 09, 2011 2:03 PM
> >>>>>>>> To: U Bhaskar-B22300
> >>>>>>>> Cc: Marc Kleine-Budde; socketcan-core@lists.berlios.de;
> >>>>>>>> netdev@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:mkl@pengutronix.de]
> >>>>>>>>>> Sent: Tuesday, August 09, 2011 12:23 AM
> >>>>>>>>>> To: Wolfgang Grandegger
> >>>>>>>>>> Cc: socketcan-core@lists.berlios.de; netdev@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>;
> >          	}; 
> 
> Yes, I also find the introduction is quite useful, with some related
> correction.

I am not sure what is useful.  The clock source bits are all wrong.
When that is removed, you end up with a discussion about the prescaler
which is actually related to the flexcan.c file and has nothing
to do with the device node.  Maybe I am just going back into my
not-communicating-well mode.  Could you follow up with what you think
belongs in the introduction of the binding file?

Thanks,
Robin

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

[Wolfgang Grandegger]

On 08/09/2011 04:09 PM, Robin Holt wrote:
> On Tue, Aug 09, 2011 at 04:03:38PM +0200, Wolfgang Grandegger wrote:
>> On 08/09/2011 03:44 PM, U Bhaskar-B22300 wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Robin Holt [mailto:holt-sJ/iWh9BUns@public.gmane.org]
>>>> Sent: Tuesday, August 09, 2011 7:06 PM
>>>> To: Wolfgang Grandegger
>>>> Cc: Robin Holt; U Bhaskar-B22300; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
>>>> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
>>>>>>>> Sent: Tuesday, August 09, 2011 4:19 PM
>>>>>>>> To: U Bhaskar-B22300
>>>>>>>> Cc: Marc Kleine-Budde; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
>>>>>>>> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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:wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org]
>>>>>>>>>> Sent: Tuesday, August 09, 2011 2:03 PM
>>>>>>>>>> To: U Bhaskar-B22300
>>>>>>>>>> Cc: Marc Kleine-Budde; socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org;
>>>>>>>>>> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org]
>>>>>>>>>>>> Sent: Tuesday, August 09, 2011 12:23 AM
>>>>>>>>>>>> To: Wolfgang Grandegger
>>>>>>>>>>>> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.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>;
>>>          	}; 
>>
>> Yes, I also find the introduction is quite useful, with some related
>> correction.
> 
> I am not sure what is useful.  The clock source bits are all wrong.
> When that is removed, you end up with a discussion about the prescaler
> which is actually related to the flexcan.c file and has nothing
> to do with the device node.  Maybe I am just going back into my
> not-communicating-well mode.  Could you follow up with what you think
> belongs in the introduction of the binding file?

Yep, you are right. Keep it simple...

Wolfgang,