Richard Cochran wrote: > On Wed, Apr 28, 2010 at 04:31:35PM +0200, Wolfgang Grandegger wrote: >> That's because some 1588_PPS related bits are not yet setup in the >> platform code of mainline kernel. > > So did you get it working?
Yes, it works now :-). With master: ptpd -b eth1 -y 0 -a 3,12 -p slave : ptpd -b eth1 -y 0 -a 3,12 I see a PPS jitter of approximately +-100ns on the oscilloscopes. That's the same value I get with the Freescale's old PTP 1588 implementation. > I am reworking this patch set to post again, but perhaps you might > take a look at the patch below. It configures the gianfar PTP clock I will comment on your previous patches later today. > parameters via the device tree. > > Richard > > [PATCH] ptp: gianfar clock uses device tree parameters > Signed-off-by: Richard Cochran <[email protected]> > --- > arch/powerpc/boot/dts/mpc8313erdb.dts | 14 +++++ > arch/powerpc/boot/dts/p2020ds.dts | 13 ++++ > arch/powerpc/boot/dts/p2020rdb.dts | 14 +++++ > drivers/net/gianfar_ptp.c | 102 ++++++++++++++++++++++---------- > 4 files changed, 111 insertions(+), 32 deletions(-) > > diff --git a/arch/powerpc/boot/dts/mpc8313erdb.dts > b/arch/powerpc/boot/dts/mpc8313erdb.dts > index 183f2aa..b760aee 100644 > --- a/arch/powerpc/boot/dts/mpc8313erdb.dts > +++ b/arch/powerpc/boot/dts/mpc8313erdb.dts > @@ -208,6 +208,20 @@ > sleep = <&pmc 0x00300000>; > }; > > + ptp_cl...@24e00 { > + device_type = "ptp_clock"; > + model = "eTSEC"; > + reg = <0x24E00 0xB0>; > + interrupts = <0x0C 2 0x0D 2>; The interrupt number is usually specified in decimal notation. > + interrupt-parent = < &ipic >; > + tclk_period = <10>; > + tmr_prsc = <100>; > + tmr_add = <0x999999A4>; > + cksel = <0x1>; > + tmr_fiper1 = <0x3B9AC9F6>; > + tmr_fiper2 = <0x00018696>; > + }; You should use the prefix "fsl," for the MPC-specific properties, at least. A few of the values could be calculated from the clock frequency. OF people usually prefer human readable values, if feasible, e.g. ptp-clock-source = "sys"; ptp-clock-source = "ext"; ptp-clock-frequency = "100000000"; Not sure it that works for other PTP hardware but it would be nice to have a generic set of properties. I have added a CC to the device-tree discuss mailing for further feedback. > + > enet0: ether...@24000 { > #address-cells = <1>; > #size-cells = <1>; > diff --git a/arch/powerpc/boot/dts/p2020ds.dts > b/arch/powerpc/boot/dts/p2020ds.dts > index 1101914..1dcf790 100644 > --- a/arch/powerpc/boot/dts/p2020ds.dts > +++ b/arch/powerpc/boot/dts/p2020ds.dts > @@ -336,6 +336,19 @@ > phy_type = "ulpi"; > }; > > + ptp_cl...@24e00 { > + device_type = "ptp_clock"; > + model = "eTSEC"; > + reg = <0x24E00 0xB0>; > + interrupts = <0x0C 2 0x0D 2>; > + interrupt-parent = < &mpic >; > + tclk_period = <5>; > + tmr_prsc = <200>; > + tmr_add = <0xCCCCCCCD>; > + tmr_fiper1 = <0x3B9AC9FB>; > + tmr_fiper2 = <0x0001869B>; > + }; > + > enet0: ether...@24000 { > #address-cells = <1>; > #size-cells = <1>; > diff --git a/arch/powerpc/boot/dts/p2020rdb.dts > b/arch/powerpc/boot/dts/p2020rdb.dts > index da4cb0d..ba61e8e 100644 > --- a/arch/powerpc/boot/dts/p2020rdb.dts > +++ b/arch/powerpc/boot/dts/p2020rdb.dts > @@ -396,6 +396,20 @@ > phy_type = "ulpi"; > }; > > + ptp_cl...@24e00 { > + device_type = "ptp_clock"; > + model = "eTSEC"; > + reg = <0x24E00 0xB0>; > + interrupts = <0x0C 2 0x0D 2>; > + interrupt-parent = < &mpic >; > + tclk_period = <5>; > + tmr_prsc = <200>; > + tmr_add = <0xCCCCCCCD>; > + cksel = <1>; > + tmr_fiper1 = <0x3B9AC9FB>; > + tmr_fiper2 = <0x0001869B>; > + }; > + > enet0: ether...@24000 { > #address-cells = <1>; > #size-cells = <1>; > diff --git a/drivers/net/gianfar_ptp.c b/drivers/net/gianfar_ptp.c > index eed3246..ed6234c 100644 > --- a/drivers/net/gianfar_ptp.c > +++ b/drivers/net/gianfar_ptp.c > @@ -22,6 +22,8 @@ > #include <linux/init.h> > #include <linux/kernel.h> > #include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_platform.h> > #include <linux/timex.h> > #include <asm/io.h> > > @@ -29,29 +31,16 @@ > > #include "gianfar_ptp_reg.h" > > -/* > - * > - * TODO - get the following from device tree > - * > - */ > -#define TMR_BASE_KERNEL 0xe0024e00 // CONFIG_PPC_85xx 0xffe24e00 > -#define TIMER_OSC 166666666 > -#define TCLK_PERIOD 10 > -#define NOMINAL_FREQ 100000000 > -#define DEF_TMR_PRSC 100 > -#define DEF_TMR_ADD 0x999999A4 > -#define DEFAULT_CKSEL 1 > - > #define REG_SIZE (4 + TMR_ETTS2_L) > > struct etsects { > void *regs; > - u32 timer_osc; /* Hz */ > u32 tclk_period; /* nanoseconds */ > - s64 nominal_freq; /* Hz */ > u32 tmr_prsc; > u32 tmr_add; > u32 cksel; > + u32 tmr_fiper1; > + u32 tmr_fiper2; > }; > > /* Private globals */ > @@ -111,8 +100,8 @@ static void set_fipers(struct etsects *etsects) > > reg_write(etsects, TMR_CTRL, tmr_ctrl & (~TE)); > reg_write(etsects, TMR_PRSC, etsects->tmr_prsc); > - reg_write(etsects, TMR_FIPER1, 0x3B9AC9F6); > - reg_write(etsects, TMR_FIPER2, 0x00018696); > + reg_write(etsects, TMR_FIPER1, etsects->tmr_fiper1); > + reg_write(etsects, TMR_FIPER2, etsects->tmr_fiper2); > set_alarm(etsects); > reg_write(etsects, TMR_CTRL, tmr_ctrl|TE); > } > @@ -213,34 +202,51 @@ struct ptp_clock_info ptp_gianfar_caps = { > .enable = ptp_gianfar_enable, > }; > > -/* module operations */ > +/* OF device tree */ > > -static void __exit ptp_gianfar_exit(void) > +static int get_of_u32(struct device_node *node, char *str, u32 *val) > { > - ptp_clock_unregister(&ptp_gianfar_caps); > - iounmap(the_clock.regs); > + int plen; > + const u32 *prop = of_get_property(node, str, &plen); > + > + if (!prop || plen != sizeof(*prop)) > + return -1; > + *val = *prop; > + return 0; > } > > -static int __init ptp_gianfar_init(void) > +static int gianfar_ptp_probe(struct of_device* dev, > + const struct of_device_id *match) > { > + u64 addr, size; > + struct device_node *node = dev->node; > struct etsects *etsects = &the_clock; > struct timespec now; > - phys_addr_t reg_addr = TMR_BASE_KERNEL; > - unsigned long reg_size = REG_SIZE; > + phys_addr_t reg_addr; > + unsigned long reg_size; > u32 tmr_ctrl; > int err; > > + if (get_of_u32(node, "tclk_period", &etsects->tclk_period) || > + get_of_u32(node, "tmr_prsc", &etsects->tmr_prsc) || > + get_of_u32(node, "tmr_add", &etsects->tmr_add) || > + get_of_u32(node, "cksel", &etsects->cksel) || > + get_of_u32(node, "tmr_fiper1", &etsects->tmr_fiper1) || > + get_of_u32(node, "tmr_fiper2", &etsects->tmr_fiper2)) > + return -ENODEV; > + > + addr = of_translate_address(node, of_get_address(node, 0, &size, NULL)); > + reg_addr = addr; > + reg_size = size; > + if (reg_size < REG_SIZE) { > + pr_warning("device tree reg range %lu too small\n", reg_size); > + reg_size = REG_SIZE; > + } > etsects->regs = ioremap(reg_addr, reg_size); > if (!etsects->regs) { > pr_err("ioremap ptp registers failed\n"); > return -EINVAL; > } > - etsects->timer_osc = TIMER_OSC; > - etsects->tclk_period = TCLK_PERIOD; > - etsects->nominal_freq = NOMINAL_FREQ; > - etsects->tmr_prsc = DEF_TMR_PRSC; > - etsects->tmr_add = DEF_TMR_ADD; > - etsects->cksel = DEFAULT_CKSEL; > > tmr_ctrl = > (etsects->tclk_period & TCLK_PERIOD_MASK) << TCLK_PERIOD_SHIFT | > @@ -252,8 +258,8 @@ static int __init ptp_gianfar_init(void) > reg_write(etsects, TMR_CTRL, tmr_ctrl); > reg_write(etsects, TMR_ADD, etsects->tmr_add); > reg_write(etsects, TMR_PRSC, etsects->tmr_prsc); > - reg_write(etsects, TMR_FIPER1, 0x3B9AC9F6); > - reg_write(etsects, TMR_FIPER2, 0x00018696); > + reg_write(etsects, TMR_FIPER1, etsects->tmr_fiper1); > + reg_write(etsects, TMR_FIPER2, etsects->tmr_fiper2); > set_alarm(etsects); > reg_write(etsects, TMR_CTRL, tmr_ctrl|FS|RTPE|TE); > > @@ -261,6 +267,38 @@ static int __init ptp_gianfar_init(void) > return err; > } > > +static int gianfar_ptp_remove(struct of_device* dev) > +{ > + ptp_clock_unregister(&ptp_gianfar_caps); > + iounmap(the_clock.regs); > + return 0; > +} > + > +static struct of_device_id match_table[] = { > + { .type = "ptp_clock" }, > + {}, > +}; > + > +static struct of_platform_driver gianfar_ptp_driver = { > + .name = "gianfar_ptp", > + .match_table = match_table, > + .owner = THIS_MODULE, > + .probe = gianfar_ptp_probe, > + .remove = gianfar_ptp_remove, > +}; > + > +/* module operations */ > + > +static void __exit ptp_gianfar_exit(void) > +{ > + of_unregister_platform_driver(&gianfar_ptp_driver); > +} > + > +static int __init ptp_gianfar_init(void) > +{ > + return of_register_platform_driver(&gianfar_ptp_driver); > +} > + > subsys_initcall(ptp_gianfar_init); > module_exit(ptp_gianfar_exit); Wolfgang. _______________________________________________ devicetree-discuss mailing list [email protected] https://lists.ozlabs.org/listinfo/devicetree-discuss
