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:

quoted

On 05/14/2010 11:46 AM, Richard Cochran wrote:

quoted

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.

quoted

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").

quoted

quoted

quoted

+  - tmr_fiper1   Fixed interval period pulse generator.
+  - tmr_fiper2   Fixed interval period pulse generator.

MPC8572 and P2020 have fiper3 as well.

quoted

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.

quoted

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.

quoted

quoted

+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