Re: [PATCH v3 3/3] ptp: Added a clock that uses the eTSEC found on the MPC85xx.
On Mon, May 17, 2010 at 01:05:54PM -0500, Scott Wood wrote: + - tmr_fiper1 Fixed interval period pulse generator. + - tmr_fiper2 Fixed interval period pulse generator. MPC8572 and P2020 have fiper3 as well. I doubt they really have a third fiper. First of all, this signal is not routed anywhere on the boards. Also, according to the documentation, it has no bit in the TMR_CTRL or the TMR_TEMASK registers. Unless there is a bit in TMR_TEMASK, you cannot get an interrupt from it. If you cannot use the signal externally (in the real world) and you cannot get an interrupt, what good is it to have such a periodic signal? Polling the bit in the TMR_TEVENT to see when a pulse occurs seems pointless. Scott, you have connections, right? Can you clarify this for me? Thanks, Richard ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3 3/3] ptp: Added a clock that uses the eTSEC found on the MPC85xx.
On 05/18/2010 01:36 AM, Richard Cochran wrote: On Mon, May 17, 2010 at 01:05:54PM -0500, Scott Wood wrote: + - tmr_fiper1 Fixed interval period pulse generator. + - tmr_fiper2 Fixed interval period pulse generator. MPC8572 and P2020 have fiper3 as well. I doubt they really have a third fiper. First of all, this signal is not routed anywhere on the boards. OK, but that's a separate issue from whether it exists on the chip and could be used on a different board. Also, according to the documentation, it has no bit in the TMR_CTRL or the TMR_TEMASK registers. It does seem inconsistent -- but could just be bad docs. Unless there is a bit in TMR_TEMASK, you cannot get an interrupt from it. If you cannot use the signal externally (in the real world) and you cannot get an interrupt, what good is it to have such a periodic signal? Polling the bit in the TMR_TEVENT to see when a pulse occurs seems pointless. Scott, you have connections, right? Can you clarify this for me? I'll ask around. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3 3/3] ptp: Added a clock that uses the eTSEC found on the MPC85xx.
On Fri, May 14, 2010 at 12:46:57PM -0500, Scott Wood wrote:
On 05/14/2010 11:46 AM, Richard Cochran wrote:
diff --git a/Documentation/powerpc/dts-bindings/fsl/tsec.txt
b/Documentation/powerpc/dts-bindings/fsl/tsec.txt
Get rid of both device_type and model, and specify a compatible
string instead (e.g. fsl,etsec-ptp).
Okay, will do. I really am at a loss at understanding all the rules in
the whole device tree world. I just tried to follow
Documentation/powerpc and what is already present in the kernel.
Or perhaps this should just be some additional properties on the
existing gianfar nodes, rather than presenting it as a separate
device? How do you associate a given ptp block with the
corresponding gianfar node?
There only one PTP clock. Its registers repeat in each port's memory
space, but you are only supposed to touch the first set of PTP
registers. If you consider how PTP works, there can never be per port
clocks, since this would make it impossible to make a boundary clock,
for example.
The whole idea of this PTP clock framework is to keep the clock
drivers separate from the MAC drivers, even when they use the same
hardware. The functionality is logically divided into two parts. The
MAC provides time stamps, and the clock provides a way to control its
offset and frequency.
Up until this point, people have simply hacked new private ioctls into
the driver for each MAC that supports PTP. That is not a good long
term solution for PTP support in Linux.
In general, I think it will not be hard to keep the MAC and the clock
drivers from stepping on each other's toes. The eTSEC hardware is
certainly able to be used in this way.
If there are differences in ptp implementation between different
versions of etsec, can the ptp driver see the etsec version
register?
There are no differences (that I know of) in how the PTP clocks
work. I have in house the mpc8313, the mpc8572, and the p2020. The
mpc8572 appears to lack some of the TMR_CTRL bits, but this is
probably a documentation bug. I will check it.
+ - tclk_period Timer reference clock period in nanoseconds.
+ - tmr_prsc Prescaler, divides the output clock.
+ - tmr_add Frequency compensation value.
+ - cksel0= external clock, 1= eTSEC system clock, 3= RTC clock
input.
+ Currently the driver only supports choice 1.
+ - tmr_fiper1 Fixed interval period pulse generator.
+ - tmr_fiper2 Fixed interval period pulse generator.
Dashes are more typical in OF names than underscores, and it's
generally better to be a little more verbose -- these aren't local
loop iterators.
The names come from the register mnemonics from the documentation. I
prefer to use the same names as is found in the manuals. That way, a
person working with docu in hand will have an easier job.
They should probably have an fsl,ptp- prefix as well.
Okay, but must I then change the following code in order to find them?
Does adding the prefix just mean that I also add it to my search
strings, or is it preprocessed (stripped) somehow?
static int get_of_u32(struct device_node *node, char *str, u32 *val)
{
int plen;
const u32 *prop = of_get_property(node, str, plen);
if (!prop || plen != sizeof(*prop))
return -1;
*val = *prop;
return 0;
}
...
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;
+ These properties set the operational parameters for the PTP
+ clock. You must choose these carefully for the clock to work right.
Do these values describe the way the hardware is, or how it's been
configured by firmware, or a set of values that are clearly optimal
for this particular board? If it's just configuration for the Linux
driver, that could reasonably differ based on what a given user or
OS will want, the device tree probably isn't the right place for it.
The values are related to the board. One important parameter is the
input clock, and the rest reflect some engineering decisions/tradeoffs
related to the signals to and from the PTP clock. There is not just
one optimal choice, so I wanted to let the designer set the
values. In any case, the parameters are definitely related to the
board (not to the cpu or to linux), so I think the device tree is the
right place for them.
This one has 3 interrupts? The driver supports only two.
The documentation does not specify the IRQ line that each event
belongs to. After some trial and error, it appears that all of the
ancillary clock interrupts arrive on the first interrupt. The other
lines (one per port) must be for the Tx/Rx packet time stamp
indication, but we don't need
Re: [PATCH v3 3/3] ptp: Added a clock that uses the eTSEC found on the MPC85xx.
On 05/14/2010 06:46 PM, Richard Cochran wrote: The eTSEC includes a PTP clock with quite a few features. This patch adds support for the basic clock adjustment functions, plus two external time stamps and one alarm. Signed-off-by: Richard Cochran richard.coch...@omicron.at Tested-by: Wolfgang Grandegger w...@denx.de on my Freescale MPC8313 setup with ptpd and ptpv2d. FYI: checkplatch.pl reports various errors for this patch series. Wolfgang. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3 3/3] ptp: Added a clock that uses the eTSEC found on the MPC85xx.
On 05/17/2010 03:27 AM, Richard Cochran wrote:
On Fri, May 14, 2010 at 12:46:57PM -0500, Scott Wood wrote:
On 05/14/2010 11:46 AM, Richard Cochran wrote:
diff --git a/Documentation/powerpc/dts-bindings/fsl/tsec.txt
b/Documentation/powerpc/dts-bindings/fsl/tsec.txt
Get rid of both device_type and model, and specify a compatible
string instead (e.g. fsl,etsec-ptp).
Okay, will do. I really am at a loss at understanding all the rules in
the whole device tree world. I just tried to follow
Documentation/powerpc and what is already present in the kernel.
There's some stuff in there that isn't how we'd do it now, but is slow
to change for compatibility reasons.
Or perhaps this should just be some additional properties on the
existing gianfar nodes, rather than presenting it as a separate
device? How do you associate a given ptp block with the
corresponding gianfar node?
There only one PTP clock. Its registers repeat in each port's memory
space, but you are only supposed to touch the first set of PTP
registers.
OK. I'm not too familiar with PTP itself, was looking more at the
device tree and similar structural bits.
There are no differences (that I know of) in how the PTP clocks
work. I have in house the mpc8313, the mpc8572, and the p2020. The
mpc8572 appears to lack some of the TMR_CTRL bits, but this is
probably a documentation bug. I will check it.
If there's any possibility of needing to make a distinction (which
probably can't be ruled out with future chips), the chip name could be
made part of the compatible string, with a secondary compatible showing
a canonical part name for that version of the PTP block. E.g. p2020
might have:
compatble = fsl,p2020-etsec-ptp, fsl,mpc8313-etsec-ptp;
The driver would bind only on the mpc8313 version.
There are several examples of this, such as the Freescale i2c driver and
binding (ignore the legacy fsl-i2c).
+ - tmr_fiper1 Fixed interval period pulse generator.
+ - tmr_fiper2 Fixed interval period pulse generator.
MPC8572 and P2020 have fiper3 as well.
They should probably have an fsl,ptp- prefix as well.
Okay, but must I then change the following code in order to find them?
Does adding the prefix just mean that I also add it to my search
strings, or is it preprocessed (stripped) somehow?
It is not stripped; you have to change the code as well.
You've got two IRQs, with the same handler, and the same dev_id?
From the manual it looks like there's one PTP interrupt per eTSEC
(which would explain 3 interrupts on p2020).
Will reduce to just one IRQ.
The device tree should still contain all of the interrupts, in case
they're needed later -- and put a comment in the driver saying why the
first interrupt seems sufficient.
+static struct of_device_id match_table[] = {
+ { .type = ptp_clock },
+ {},
+};
This driver controls every possible PTP implementation?
No, I only want to match with the eTSEC clock device. Given the
compatible string above (fsl,etsec-ptp), what is the correct way to
do this? (pointer to an existing driver to emulate would be enough)
Put .compatible = fsl,etsec-ptp (or fsl,mpc8313-etsec-ptp) where you
have .type = ptp_clock.
-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3 3/3] ptp: Added a clock that uses the eTSEC found on the MPC85xx.
On 05/14/2010 11:46 AM, Richard Cochran wrote:
diff --git a/Documentation/powerpc/dts-bindings/fsl/tsec.txt
b/Documentation/powerpc/dts-bindings/fsl/tsec.txt
index edb7ae1..b09ba66 100644
--- a/Documentation/powerpc/dts-bindings/fsl/tsec.txt
+++ b/Documentation/powerpc/dts-bindings/fsl/tsec.txt
@@ -74,3 +74,59 @@ Example:
interrupt-parent =mpic;
phy-handle =phy0
};
+
+* Gianfar PTP clock nodes
+
+General Properties:
+
+ - device_type Should be ptp_clock
Device_type is deprecated in most contexts for flat device trees.
+ - modelModel of the device. Must be eTSEC
Model, while abused by the current gianfar binding code, is not supposed
to be something that is ordinarily used to bind on. It is supposed to
be a freeform field for indicating the specific model of hardware,
mainly for human consumption or as a last resort for working around
problems.
Get rid of both device_type and model, and specify a compatible string
instead (e.g. fsl,etsec-ptp). Or perhaps this should just be some
additional properties on the existing gianfar nodes, rather than
presenting it as a separate device? How do you associate a given ptp
block with the corresponding gianfar node? If there are differences in
ptp implementation between different versions of etsec, can the ptp
driver see the etsec version register?
+ - reg Offset and length of the register set for the device
+ - interrupts There should be at least two and as many as four
+ PTP related interrupts
+
+Clock Properties:
+
+ - tclk_period Timer reference clock period in nanoseconds.
+ - tmr_prsc Prescaler, divides the output clock.
+ - tmr_add Frequency compensation value.
+ - cksel0= external clock, 1= eTSEC system clock, 3= RTC clock input.
+ Currently the driver only supports choice 1.
+ - tmr_fiper1 Fixed interval period pulse generator.
+ - tmr_fiper2 Fixed interval period pulse generator.
Dashes are more typical in OF names than underscores, and it's generally
better to be a little more verbose -- these aren't local loop iterators.
They should probably have an fsl,ptp- prefix as well.
+ These properties set the operational parameters for the PTP
+ clock. You must choose these carefully for the clock to work right.
Do these values describe the way the hardware is, or how it's been
configured by firmware, or a set of values that are clearly optimal for
this particular board? If it's just configuration for the Linux driver,
that could reasonably differ based on what a given user or OS will want,
the device tree probably isn't the right place for it.
diff --git a/arch/powerpc/boot/dts/p2020ds.dts
b/arch/powerpc/boot/dts/p2020ds.dts
index 1101914..f72353a 100644
--- a/arch/powerpc/boot/dts/p2020ds.dts
+++ b/arch/powerpc/boot/dts/p2020ds.dts
@@ -336,6 +336,20 @@
phy_type = ulpi;
};
+ ptp_cl...@24e00 {
+ device_type = ptp_clock;
+ model = eTSEC;
+ reg = 0x24E00 0xB0;
+ interrupts = 68 2 69 2 70 2;
+ interrupt-parent = mpic ;
+ tclk_period = 5;
+ tmr_prsc = 200;
+ tmr_add = 0xCCCD;
+ cksel = 1;
+ tmr_fiper1 = 0x3B9AC9FB;
+ tmr_fiper2 = 0x0001869B;
+ };
+
This one has 3 interrupts? The driver supports only two.
+/* Private globals */
+static struct ptp_clock *gianfar_clock;
Do you not support more than one of these?
+static struct etsects the_clock;
The clock? As oppsed to the other clock one line above? :-)
+static irqreturn_t isr(int irq, void *priv)
+{
+ struct etsects *etsects = priv;
+ struct ptp_clock_event event;
+ u64 ns;
+ u32 ack=0, lo, hi, mask, val;
+
+ val = gfar_read(etsects-regs-tmr_tevent);
+
+ if (val ETS1) {
+ ack |= ETS1;
+ hi = gfar_read(etsects-regs-tmr_etts1_h);
+ lo = gfar_read(etsects-regs-tmr_etts1_l);
+ event.type = PTP_CLOCK_EXTTS;
+ event.index = 0;
+ event.timestamp = ((u64) hi) 32;
+ event.timestamp |= lo;
+ ptp_clock_event(gianfar_clock,event);
+ }
+
+ if (val ETS2) {
+ ack |= ETS2;
+ hi = gfar_read(etsects-regs-tmr_etts2_h);
+ lo = gfar_read(etsects-regs-tmr_etts2_l);
+ event.type = PTP_CLOCK_EXTTS;
+ event.index = 1;
+ event.timestamp = ((u64) hi) 32;
+ event.timestamp |= lo;
+ ptp_clock_event(gianfar_clock,event);
+ }
+
+ if (val ALM2) {
+ ack |= ALM2;
+ if (etsects-alarm_value) {
+ event.type = PTP_CLOCK_ALARM;
+
