> 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@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@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

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to