[PATCH v2] powerpc/dts: Add and fix 1588 timer node for eTSEC

[Lu Y.B.]

> On Wed, 2015-07-15 at 21:37 -0500, Lu Yangbo-B47093 wrote:
> > Any comments?
> > Thanks.
> 
> Sorry, I must have missed this on my last time through the patch queue.
> I see you've decimalized the fiper and max-adj properties, which is
> good... but does it really make sense for tmr-add?  I'm not familiar with
> what this value represents, but the numbers look more natural as hex (e.g.
> 0xaaaaaaab versus 2863311531).

Yes, the fiper value would be writed into fiper registers. And max-adj value would be used in ptp driver in driver/ptp/.
But you insisted that values should be in decimalism in the v1 patch... :)

See the history :)

################# history ##################
> > +	ptp_clock at b0e00 {
> > +		compatible = "fsl,etsec-ptp";
> > +		reg = <0xb0e00 0xb0>;
> > +		interrupts = <68 2 0 0 69 2 0 0>;
> > +		fsl,tclk-period	= <5>;
> > +		fsl,tmr-prsc	= <2>;
> > +		fsl,tmr-add	= <0xcccccccd>;
> > +		fsl,tmr-fiper1	= <0x3b9ac9fb>;
> > +		fsl,tmr-fiper2	= <0x00018696>;
> > +		fsl,max-adj	= <249999999>;
> 
> Please don't use hex for numbers that make more sense as decimal.
> [Lu Yangbo-B47093] The hex value is register value, I think it's better to use hex.

Whether it goes into a register doesn't matter.  Hex values are useful for values which are subdivided into various bitfields, or whose hex representation is simpler than decimal.  I'm not familiar with the details of this hardware, but I doubt the former is the case for 0x3b9ac9fb == 9999999995 or 0x18696 == 99990.

-Scott
######################################


> 
> > > diff --git a/arch/powerpc/boot/dts/p2020rdb-pc.dtsi
> > > b/arch/powerpc/boot/dts/p2020rdb-pc.dtsi
> > > index c21d1c7..363172d 100644
> > > --- a/arch/powerpc/boot/dts/p2020rdb-pc.dtsi
> > > +++ b/arch/powerpc/boot/dts/p2020rdb-pc.dtsi
> > > @@ -215,12 +215,12 @@
> > >     };
> > >
> > >      ptp_clock at 24e00{
> > > -           fsl,tclk-period = <5>;
> > > -           fsl,tmr-prsc = <200>;
> > > -           fsl,tmr-add = <0xCCCCCCCD>;
> > > -           fsl,tmr-fiper1 = <0x3B9AC9FB>;
> > > -           fsl,tmr-fiper2 = <0x0001869B>;
> > > -           fsl,max-adj = <249999999>;
> > > +           fsl,tclk-period = <5>;
> > > +           fsl,tmr-prsc    = <2>;
> > > +           fsl,tmr-add     = <2863311531>;
> > > +           fsl,tmr-fiper1  = <999999995>;
> > > +           fsl,tmr-fiper2  = <99990>;
> > > +           fsl,max-adj     = <299999999>;
> > >     };
> 
> And here, you're changing the value of fsl,tmr-add and fsl,max-adj.  Why?

The old values maybe not calculated base on the default eTSEC system clock value.
1588 timer couldn’t be adjusted correctly by old values.

> 
> -Scott