diff mbox

[4/6] net: ethernet: ti: cpts: add ptp pps support

Message ID 20161128230428.6872-5-grygorii.strashko@ti.com
State Changes Requested, archived
Headers show

Commit Message

Grygorii Strashko Nov. 28, 2016, 11:04 p.m. UTC
The TS_COMP output in the CPSW CPTS module is asserted for
ts_comp_length[15:0] RCLK periods when the time_stamp value compares
with the ts_comp_val[31:0] and the length value is non-zero. The
TS_COMP pulse edge occurs three RCLK periods after the values
compare. A timestamp compare event is pushed into the event FIFO when
TS_COMP is asserted.

This patch adds support of Pulse-Per-Second (PPS) by using the
timestamp compare output. The CPTS driver adds one second of counter
value to the ts_comp_val register after each assertion of the TS_COMP
output. The TS_COMP pulse polarity and width are configurable in DT.

Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 .../devicetree/bindings/net/keystone-netcp.txt     |  10 +
 drivers/net/ethernet/ti/cpts.c                     | 237 ++++++++++++++++++++-
 drivers/net/ethernet/ti/cpts.h                     |  14 +-
 3 files changed, 251 insertions(+), 10 deletions(-)

Comments

Richard Cochran Nov. 30, 2016, 10:05 a.m. UTC | #1
On Mon, Nov 28, 2016 at 05:04:26PM -0600, Grygorii Strashko wrote:
> The TS_COMP output in the CPSW CPTS module is asserted for
> ts_comp_length[15:0] RCLK periods when the time_stamp value compares
> with the ts_comp_val[31:0] and the length value is non-zero. The
> TS_COMP pulse edge occurs three RCLK periods after the values
> compare. A timestamp compare event is pushed into the event FIFO when
> TS_COMP is asserted.
> 
> This patch adds support of Pulse-Per-Second (PPS) by using the
> timestamp compare output. The CPTS driver adds one second of counter
> value to the ts_comp_val register after each assertion of the TS_COMP
> output. The TS_COMP pulse polarity and width are configurable in DT.

I really dislike this patch.  You go through contortions to get from
the timecounter back to the raw HW counter.  That is rather ugly.

Can you adjust the frequency of the keystone devices in hardware?  If
so, then please implement it, and just disable PPS for the CPSW.

The only reason I used the timecounter for frequency adjustment was
because the am335x HW is broken.  But this shouldn't hold back other
newer HW without the same silicon flaws.

Thanks,
Richard


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Lübbe Nov. 30, 2016, 11:01 a.m. UTC | #2
On Mo, 2016-11-28 at 17:04 -0600, Grygorii Strashko wrote:
> --- a/Documentation/devicetree/bindings/net/keystone-netcp.txt
> +++ b/Documentation/devicetree/bindings/net/keystone-netcp.txt
> @@ -127,6 +127,16 @@ Optional properties:
>                 The number of external time stamp channels.
>                 The different CPTS versions might support up 8
>                 external time stamp channels. if absent - unsupported.
> +       - cpts-ts-comp-length:
> +               Enable time stamp comparison event and TS_COMP signal output
> +               generation when CPTS counter reaches a value written to
> +               the TS_COMP_VAL register.
> +               The generated pulse width is 3 refclk cycles if this property
> +               has no value (empty) or, otherwise, it should specify desired
> +               pulse width in number of refclk periods - max value 2^16.
> +               TS_COMP functionality will be disabled if not present.
> +       - cpts-ts-comp-polarity-low:
> +               Set polarity of TS_COMP signal to low. Default is hight.

Why is this configured via DT? Are the values fixed for a given board,
depending on external components? Couldn't this be configured somewhere
else?

Regards,
Jan
Richard Cochran Nov. 30, 2016, 6:45 p.m. UTC | #3
On Mon, Nov 28, 2016 at 05:04:26PM -0600, Grygorii Strashko wrote:
> +static cycle_t cpts_cc_ns2cyc(struct cpts *cpts, u64 nsecs)
> +{
> +	cycle_t cyc = (nsecs << cpts->cc.shift) + nsecs;
> +
> +	do_div(cyc, cpts->cc.mult);
> +
> +	return cyc;
> +}

So you set the comparison value once per second, based on cc.mult.
But when the clock is being actively synchronized, user space calls to
clock_adjtimex() will change cc.mult.  This can happen several times
per second, depending on the PTP Sync rate.

In order to produce the PPS edge correctly, you would have to adjust
the comparison value whenever cc.mult changes, but of course this is
unworkable.

So I'll have to say NAK for this patch.

Thanks,
Richard

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Grygorii Strashko Nov. 30, 2016, 8:43 p.m. UTC | #4
On 11/30/2016 12:45 PM, Richard Cochran wrote:
> On Mon, Nov 28, 2016 at 05:04:26PM -0600, Grygorii Strashko wrote:
>> +static cycle_t cpts_cc_ns2cyc(struct cpts *cpts, u64 nsecs)
>> +{
>> +	cycle_t cyc = (nsecs << cpts->cc.shift) + nsecs;
>> +
>> +	do_div(cyc, cpts->cc.mult);
>> +
>> +	return cyc;
>> +}
> 
> So you set the comparison value once per second, based on cc.mult.
> But when the clock is being actively synchronized, user space calls to
> clock_adjtimex() will change cc.mult.  This can happen several times
> per second, depending on the PTP Sync rate.
> 

Right.

> In order to produce the PPS edge correctly, you would have to adjust
> the comparison value whenever cc.mult changes, 

yes. And that is done in cpts_ptp_adjfreq()
	if (cpts->ts_comp_enabled)
		cpts->ts_comp_one_sec_cycs = cpts_cc_ns2cyc(cpts, NSEC_PER_SEC);
	^^^ re-calculate reload value for 
 
	cpts_ts_comp_settime(cpts, ns);
	^^^ adjust the ts_comp

> but of course this is unworkable.
> 

Sry, but this is questionable - code for pps comes from TI internal
branches (SDK releases) where it survived for a pretty long time.
I'm, of course, agree that without HW support for freq adjustment
this PPS feature is not super precise and has some limitation,
but that is what we agree to live with. 

Murali, do you have any comments regarding usability of SW
freq freq adjustment approach? 

> So I'll have to say NAK for this patch.
> 

:)
Richard Cochran Nov. 30, 2016, 10:17 p.m. UTC | #5
On Wed, Nov 30, 2016 at 02:43:57PM -0600, Grygorii Strashko wrote:
> > In order to produce the PPS edge correctly, you would have to adjust
> > the comparison value whenever cc.mult changes, 
> 
> yes. And that is done in cpts_ptp_adjfreq()
> 	if (cpts->ts_comp_enabled)
> 		cpts->ts_comp_one_sec_cycs = cpts_cc_ns2cyc(cpts, NSEC_PER_SEC);
> 	^^^ re-calculate reload value for 
>  
> 	cpts_ts_comp_settime(cpts, ns);
> 	^^^ adjust the ts_comp

And it races with the pulse itself.  You forgot about this part:

> @@ -172,14 +232,31 @@ static int cpts_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
>  	adj *= ppb;
>  	diff = div_u64(adj, 1000000000ULL);
>  
> +	mutex_lock(&cpts->ptp_clk_mutex);
> +
>  	spin_lock_irqsave(&cpts->lock, flags);
> +	if (cpts->ts_comp_enabled) {
> +		cpts_ts_comp_disable(cpts);

Sorry, but this is a train wreck.

> > but of course this is unworkable.
> > 
> 
> Sry, but this is questionable - code for pps comes from TI internal
> branches (SDK releases) where it survived for a pretty long time.

That doesn't mean the code is any good.  If you adjust at the right
moment, then no pulse occurs at all!

> I'm, of course, agree that without HW support for freq adjustment
> this PPS feature is not super precise and has some limitation,
> but that is what we agree to live with. 

I do NOT agree to live with this.  I am one who is going to have to
explain to the world why their beagle bone PPS sucks.
 
Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Richard Cochran Dec. 2, 2016, 9:58 a.m. UTC | #6
On Wed, Nov 30, 2016 at 11:17:38PM +0100, Richard Cochran wrote:
> On Wed, Nov 30, 2016 at 02:43:57PM -0600, Grygorii Strashko wrote:
> > Sry, but this is questionable - code for pps comes from TI internal
> > branches (SDK releases) where it survived for a pretty long time.

Actually, there is a way to get an accurate PPS from the am335x.  See
this recent thread:

   https://www.mail-archive.com/linuxptp-devel@lists.sourceforge.net/msg01726.html

That is the way to go, and so, please drop this present patch.

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Grygorii Strashko Dec. 2, 2016, 5:58 p.m. UTC | #7
Hi Richard,

On 12/02/2016 03:58 AM, Richard Cochran wrote:
> On Wed, Nov 30, 2016 at 11:17:38PM +0100, Richard Cochran wrote:
>> On Wed, Nov 30, 2016 at 02:43:57PM -0600, Grygorii Strashko wrote:
>>> Sry, but this is questionable - code for pps comes from TI internal
>>> branches (SDK releases) where it survived for a pretty long time.
> 
> Actually, there is a way to get an accurate PPS from the am335x.  See
> this recent thread:
> 
>    https://www.mail-archive.com/linuxptp-devel@lists.sourceforge.net/msg01726.html
> 
> That is the way to go, and so, please drop this present patch.
> 

thanks for the links - it sounds very interesting.

As I understood, people trying to enable PPS on am335 device with the
goal to have PPS signal generated on some SoC pin and therefore they use DMtimer.
Also, as i understood, the Timer Load Register (TLDR) is corrected once
a second at each HW_TS_PUSH - as result, if freq was corrected during current sec
there will be some HW_TS_PUSH generation jitter any way.

Above solution is a bit complex for keystone 2 SoCs, as CPTS itself on these SoCs has
output pin (ts_comp) which can be used for PPS signal generation. So, I think,
similar results can be achieved by removing PPS correction code from cpts_ptp_adjfreq()
and updating CPTS_TS_LOAD_VAL once a sec in cpts_overflow_check().

or I missed smth?
Richard Cochran Dec. 2, 2016, 7:28 p.m. UTC | #8
On Fri, Dec 02, 2016 at 11:58:34AM -0600, Grygorii Strashko wrote:
> or I missed smth?

You are missing three important points.

1. Unlike the code you posted, no edges will be lost.

2. The solution using the PWM is implemented in USER SPACE.  If people
   use this way, then they will be forced to understand the inherit
   limitations.  In addition, the behavior of servo will be under
   their control.

3. The update rate of the PHC is not once per second.  It can be any
   rate at all, like 16 Hz for the telecom profile.  You can't just
   blindly pick out an adjustment value once per second.  Using the
   feedback from the time stamped PWM and adjusting THAT at the PWM
   rate (also not necessarily 1 PPS) is the right way.  The second
   reply in that thread is an even better solution, leaving the PHC
   free running and adjusting the timer input clock (probably they
   used a VCO).

Just hacking in some kind of kernel PPS with unknown accuracy is just
asking for trouble later, since people will expect HW accuracy.

So just get the input time stamps working, and make PWM control
available to userspace in mainline (not sure about this, I guess it
isn't), and leave the PPS part to a userspace utility.

Thanks,
Richard



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Richard Cochran Dec. 6, 2016, 6:08 p.m. UTC | #9
On Wed, Nov 30, 2016 at 11:05:19AM +0100, Richard Cochran wrote:
> Can you adjust the frequency of the keystone devices in hardware?  If
> so, then please implement it, and just disable PPS for the CPSW.
> 
> The only reason I used the timecounter for frequency adjustment was
> because the am335x HW is broken.  But this shouldn't hold back other
> newer HW without the same silicon flaws.

I am talking here about the ADPLLLJ units.  Are they usable on the
keystone?

If so, please implement the frequency adjustment with them.

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Grygorii Strashko Dec. 6, 2016, 8:43 p.m. UTC | #10
On 12/06/2016 12:08 PM, Richard Cochran wrote:
> On Wed, Nov 30, 2016 at 11:05:19AM +0100, Richard Cochran wrote:
>> Can you adjust the frequency of the keystone devices in hardware?  If
>> so, then please implement it, and just disable PPS for the CPSW.
>>
>> The only reason I used the timecounter for frequency adjustment was
>> because the am335x HW is broken.  But this shouldn't hold back other
>> newer HW without the same silicon flaws.
>
> I am talking here about the ADPLLLJ units.  Are they usable on the
> keystone?
>
> If so, please implement the frequency adjustment with them.
>

No, I think it's not impossible (at least as I know now).
i'll drop this patch for now.

By the way, I've tested am335 (BBB)+ HW_TS_PUSH + PWM.
Seems works.
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/net/keystone-netcp.txt b/Documentation/devicetree/bindings/net/keystone-netcp.txt
index 1c805319..060af96 100644
--- a/Documentation/devicetree/bindings/net/keystone-netcp.txt
+++ b/Documentation/devicetree/bindings/net/keystone-netcp.txt
@@ -127,6 +127,16 @@  Optional properties:
 		The number of external time stamp channels.
 		The different CPTS versions might support up 8
 		external time stamp channels. if absent - unsupported.
+	- cpts-ts-comp-length:
+		Enable time stamp comparison event and TS_COMP signal output
+		generation when CPTS counter reaches a value written to
+		the TS_COMP_VAL register.
+		The generated pulse width is 3 refclk cycles if this property
+		has no value (empty) or, otherwise, it should specify desired
+		pulse width in number of refclk periods - max value 2^16.
+		TS_COMP functionality will be disabled if not present.
+	- cpts-ts-comp-polarity-low:
+		Set polarity of TS_COMP signal to low. Default is hight.
 
 NetCP interface properties: Interface specification for NetCP sub-modules.
 Required properties:
diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
index 2f7641a..8ff70cc 100644
--- a/drivers/net/ethernet/ti/cpts.c
+++ b/drivers/net/ethernet/ti/cpts.c
@@ -31,9 +31,13 @@ 
 
 #include "cpts.h"
 
+#define CPTS_TS_COMP_PULSE_LENGTH_DEF	3
+
 #define cpts_read32(c, r)	readl_relaxed(&c->reg->r)
 #define cpts_write32(c, v, r)	writel_relaxed(v, &c->reg->r)
 
+static int cpts_report_ts_events(struct cpts *cpts, bool pps_reload);
+
 static int cpts_event_port(struct cpts_event *event)
 {
 	return (event->high >> PORT_NUMBER_SHIFT) & PORT_NUMBER_MASK;
@@ -108,6 +112,7 @@  static int cpts_fifo_read(struct cpts *cpts, int match)
 		type = event_type(event);
 		switch (type) {
 		case CPTS_EV_HW:
+		case CPTS_EV_COMP:
 			event->tmo +=
 				msecs_to_jiffies(CPTS_EVENT_HWSTAMP_TIMEOUT);
 		case CPTS_EV_PUSH:
@@ -153,6 +158,60 @@  static cycle_t cpts_systim_read(const struct cyclecounter *cc)
 	return val;
 }
 
+static cycle_t cpts_cc_ns2cyc(struct cpts *cpts, u64 nsecs)
+{
+	cycle_t cyc = (nsecs << cpts->cc.shift) + nsecs;
+
+	do_div(cyc, cpts->cc.mult);
+
+	return cyc;
+}
+
+static void cpts_ts_comp_disable(struct cpts *cpts)
+{
+	cpts_write32(cpts, 0, ts_comp_length);
+}
+
+static void cpts_ts_comp_enable(struct cpts *cpts)
+{
+	/* TS_COMP_LENGTH should be 0 while the TS_COMP_VAL value is
+	 * being written
+	 */
+	cpts_write32(cpts, 0, ts_comp_length);
+	cpts_write32(cpts, cpts->ts_comp_next, ts_comp_val);
+	cpts_write32(cpts, cpts->ts_comp_length, ts_comp_length);
+}
+
+static void cpts_ts_comp_add_ns(struct cpts *cpts, s64 add_ns)
+{
+	cycle_t cyc_next;
+
+	if (add_ns == NSEC_PER_SEC)
+		/* avoid calculation */
+		cyc_next = cpts->ts_comp_one_sec_cycs;
+	else
+		cyc_next = cpts_cc_ns2cyc(cpts, add_ns);
+
+	cyc_next += cpts->ts_comp_next;
+	cpts->ts_comp_next = cyc_next & cpts->cc.mask;
+	pr_debug("cpts comp ts_comp_next: %u\n", cpts->ts_comp_next);
+}
+
+static void cpts_ts_comp_settime(struct cpts *cpts, s64 now_ns)
+{
+	struct timespec64 ts;
+
+	if (cpts->ts_comp_enabled) {
+		ts = ns_to_timespec64(now_ns);
+
+		/* align pulse to next sec boundary and add one sec */
+		cpts_ts_comp_add_ns(cpts, NSEC_PER_SEC - ts.tv_nsec);
+
+		/* enable ts_comp pulse */
+		cpts_ts_comp_enable(cpts);
+	}
+}
+
 /* PTP clock operations */
 
 static int cpts_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
@@ -162,6 +221,7 @@  static int cpts_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
 	int neg_adj = 0;
 	unsigned long flags;
 	struct cpts *cpts = container_of(ptp, struct cpts, info);
+	u64 ns;
 
 	if (ppb < 0) {
 		neg_adj = 1;
@@ -172,14 +232,31 @@  static int cpts_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
 	adj *= ppb;
 	diff = div_u64(adj, 1000000000ULL);
 
+	mutex_lock(&cpts->ptp_clk_mutex);
+
 	spin_lock_irqsave(&cpts->lock, flags);
+	if (cpts->ts_comp_enabled) {
+		cpts_ts_comp_disable(cpts);
+		/* if any, report existing pulse before adj */
+		cpts_fifo_read(cpts, CPTS_EV_COMP);
+		/* if any, report existing pulse before adj */
+		cpts_report_ts_events(cpts, false);
+	}
 
 	timecounter_read(&cpts->tc);
 
 	cpts->cc.mult = neg_adj ? mult - diff : mult + diff;
-
+	/* get updated time with adj */
+	ns = timecounter_read(&cpts->tc);
+	cpts->ts_comp_next = cpts->tc.cycle_last;
 	spin_unlock_irqrestore(&cpts->lock, flags);
 
+	if (cpts->ts_comp_enabled)
+		cpts->ts_comp_one_sec_cycs = cpts_cc_ns2cyc(cpts, NSEC_PER_SEC);
+	cpts_ts_comp_settime(cpts, ns);
+
+	mutex_unlock(&cpts->ptp_clk_mutex);
+
 	return 0;
 }
 
@@ -187,11 +264,28 @@  static int cpts_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 {
 	unsigned long flags;
 	struct cpts *cpts = container_of(ptp, struct cpts, info);
+	u64 ns;
+
+	mutex_lock(&cpts->ptp_clk_mutex);
 
 	spin_lock_irqsave(&cpts->lock, flags);
+	if (cpts->ts_comp_enabled) {
+		cpts_ts_comp_disable(cpts);
+		/* if any, report existing pulse before adj */
+		cpts_fifo_read(cpts, CPTS_EV_COMP);
+		/* if any, report existing pulse before adj */
+		cpts_report_ts_events(cpts, false);
+	}
+
 	timecounter_adjtime(&cpts->tc, delta);
+	ns = timecounter_read(&cpts->tc);
+	cpts->ts_comp_next = cpts->tc.cycle_last;
 	spin_unlock_irqrestore(&cpts->lock, flags);
 
+	cpts_ts_comp_settime(cpts, ns);
+
+	mutex_unlock(&cpts->ptp_clk_mutex);
+
 	return 0;
 }
 
@@ -213,25 +307,90 @@  static int cpts_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
 static int cpts_ptp_settime(struct ptp_clock_info *ptp,
 			    const struct timespec64 *ts)
 {
-	u64 ns;
-	unsigned long flags;
 	struct cpts *cpts = container_of(ptp, struct cpts, info);
+	unsigned long flags;
+	u64 ns;
 
 	ns = timespec64_to_ns(ts);
 
+	mutex_lock(&cpts->ptp_clk_mutex);
+
 	spin_lock_irqsave(&cpts->lock, flags);
+	if (cpts->ts_comp_enabled) {
+		cpts_ts_comp_disable(cpts);
+		/* if any, get existing pulse event before adj */
+		cpts_fifo_read(cpts, CPTS_EV_COMP);
+		/* if any, report existing pulse before adj */
+		cpts_report_ts_events(cpts, false);
+	}
+
 	timecounter_init(&cpts->tc, &cpts->cc, ns);
+	cpts->ts_comp_next = cpts->tc.cycle_last;
 	spin_unlock_irqrestore(&cpts->lock, flags);
 
+	cpts_ts_comp_settime(cpts, ns);
+
+	mutex_unlock(&cpts->ptp_clk_mutex);
+
 	return 0;
 }
 
-static int cpts_report_ts_events(struct cpts *cpts)
+static int cpts_pps_enable(struct cpts *cpts, int on)
+{
+	struct timespec64 ts;
+	unsigned long flags;
+	u64 ns;
+
+	if (cpts->ts_comp_enabled == on)
+		return 0;
+
+	mutex_lock(&cpts->ptp_clk_mutex);
+	cpts->ts_comp_enabled = on;
+
+	if (!on) {
+		cpts_ts_comp_disable(cpts);
+		if (!cpts->hw_ts_enable)
+			cpts->ov_check_period = cpts->ov_check_period_slow;
+		mutex_unlock(&cpts->ptp_clk_mutex);
+		return 0;
+	}
+
+	/* get current counter value */
+	spin_lock_irqsave(&cpts->lock, flags);
+	ns = timecounter_read(&cpts->tc);
+	cpts->ts_comp_next = cpts->tc.cycle_last;
+	spin_unlock_irqrestore(&cpts->lock, flags);
+
+	ts = ns_to_timespec64(ns);
+	cpts->ts_comp_one_sec_cycs = cpts_cc_ns2cyc(cpts, NSEC_PER_SEC);
+	/* align to next sec boundary and add one sec to avoid the situation
+	 * when the current time is very close to the next second point and
+	 * it might be possible that ts_comp_val will be configured to
+	 * the time in the past.
+	 */
+	cpts_ts_comp_add_ns(cpts, 2 * NSEC_PER_SEC - ts.tv_nsec);
+
+	/* enable ts_comp pulse */
+	cpts_ts_comp_enable(cpts);
+
+	/* poll for events faster - evry 200 ms */
+	cpts->ov_check_period = msecs_to_jiffies(CPTS_EVENT_HWSTAMP_TIMEOUT);
+
+	mod_delayed_work(system_wq, &cpts->overflow_work,
+			 cpts->ov_check_period);
+
+	mutex_unlock(&cpts->ptp_clk_mutex);
+
+	return 0;
+}
+
+static int cpts_report_ts_events(struct cpts *cpts, bool pps_reload)
 {
 	struct list_head *this, *next;
 	struct ptp_clock_event pevent;
 	struct cpts_event *event;
 	int reported = 0, ev;
+	u64 ns;
 
 	list_for_each_safe(this, next, &cpts->events) {
 		event = list_entry(this, struct cpts_event, list);
@@ -248,6 +407,33 @@  static int cpts_report_ts_events(struct cpts *cpts)
 			++reported;
 			continue;
 		}
+
+		if (event_type(event) == CPTS_EV_COMP) {
+			list_del_init(&event->list);
+			list_add(&event->list, &cpts->pool);
+			if (cpts->ts_comp_next != event->low) {
+				pr_err("cpts ts_comp mismatch: %08x %08x\n",
+				       cpts->ts_comp_next, event->low);
+				continue;
+			} else
+				pr_debug("cpts comp ev tstamp: %u\n",
+					 event->low);
+
+			/* report the event */
+			ns = timecounter_cyc2time(&cpts->tc, event->low);
+			pevent.type = PTP_CLOCK_PPSUSR;
+			pevent.pps_times.ts_real = ns_to_timespec64(ns);
+			ptp_clock_event(cpts->clock, &pevent);
+
+			if (pps_reload) {
+				/* reload: add ns to ts_comp */
+				cpts_ts_comp_add_ns(cpts, NSEC_PER_SEC);
+				/* enable ts_comp pulse with new val */
+				cpts_ts_comp_enable(cpts);
+			}
+			++reported;
+			continue;
+		}
 	}
 	return reported;
 }
@@ -264,6 +450,8 @@  static int cpts_extts_enable(struct cpts *cpts, u32 index, int on)
 	if (((cpts->hw_ts_enable & BIT(index)) >> index) == on)
 		return 0;
 
+	mutex_lock(&cpts->ptp_clk_mutex);
+
 	spin_lock_irqsave(&cpts->lock, flags);
 
 	v = cpts_read32(cpts, control);
@@ -282,12 +470,12 @@  static int cpts_extts_enable(struct cpts *cpts, u32 index, int on)
 		/* poll for events faster - evry 200 ms */
 		cpts->ov_check_period =
 			msecs_to_jiffies(CPTS_EVENT_HWSTAMP_TIMEOUT);
-	else
+	else if (!cpts->ts_comp_enabled)
 		cpts->ov_check_period = cpts->ov_check_period_slow;
 
 	mod_delayed_work(system_wq, &cpts->overflow_work,
 			 cpts->ov_check_period);
-
+	mutex_unlock(&cpts->ptp_clk_mutex);
 	return 0;
 }
 
@@ -299,6 +487,8 @@  static int cpts_ptp_enable(struct ptp_clock_info *ptp,
 	switch (rq->type) {
 	case PTP_CLK_REQ_EXTTS:
 		return cpts_extts_enable(cpts, rq->extts.index, on);
+	case PTP_CLK_REQ_PPS:
+		return cpts_pps_enable(cpts, on);
 	default:
 		break;
 	}
@@ -326,12 +516,15 @@  static void cpts_overflow_check(struct work_struct *work)
 	struct timespec64 ts;
 	unsigned long flags;
 
+	mutex_lock(&cpts->ptp_clk_mutex);
 	spin_lock_irqsave(&cpts->lock, flags);
 	ts = ns_to_timespec64(timecounter_read(&cpts->tc));
 	spin_unlock_irqrestore(&cpts->lock, flags);
 
-	if (cpts->hw_ts_enable)
-		cpts_report_ts_events(cpts);
+	if (cpts->hw_ts_enable || cpts->ts_comp_enabled)
+		cpts_report_ts_events(cpts, true);
+	mutex_unlock(&cpts->ptp_clk_mutex);
+
 	pr_debug("cpts overflow check at %lld.%09lu\n", ts.tv_sec, ts.tv_nsec);
 	schedule_delayed_work(&cpts->overflow_work, cpts->ov_check_period);
 }
@@ -445,6 +638,7 @@  EXPORT_SYMBOL_GPL(cpts_tx_timestamp);
 int cpts_register(struct cpts *cpts)
 {
 	int err, i;
+	u32 control;
 
 	INIT_LIST_HEAD(&cpts->events);
 	INIT_LIST_HEAD(&cpts->pool);
@@ -453,7 +647,14 @@  int cpts_register(struct cpts *cpts)
 
 	clk_enable(cpts->refclk);
 
-	cpts_write32(cpts, CPTS_EN, control);
+	control = CPTS_EN;
+	if (cpts->caps & CPTS_CAP_TS_COMP_EN) {
+		if (cpts->caps & CPTS_CAP_TS_COMP_POL_LOW_SEL)
+			control &= ~TS_COMP_POL;
+		else
+			control |= TS_COMP_POL;
+	}
+	cpts_write32(cpts, control, control);
 	cpts_write32(cpts, TS_PEND_EN, int_enable);
 
 	cpts->cc.mult = cpts->cc_mult;
@@ -558,6 +759,20 @@  static int cpts_of_parse(struct cpts *cpts, struct device_node *node)
 		cpts->rftclk_sel = prop & CPTS_RFTCLK_SEL_MASK;
 	}
 
+	if (of_property_read_bool(node, "cpts-ts-comp-length")) {
+		cpts->caps |= CPTS_CAP_TS_COMP_EN;
+		cpts->ts_comp_length = CPTS_TS_COMP_PULSE_LENGTH_DEF;
+	}
+
+	if (cpts->caps & CPTS_CAP_TS_COMP_EN) {
+		ret = of_property_read_u32(node, "cpts-ts-comp-length", &prop);
+		if (!ret)
+			cpts->ts_comp_length = prop;
+
+		if (of_property_read_bool(node, "cpts-ts-comp-polarity-low"))
+			cpts->caps |= CPTS_CAP_TS_COMP_POL_LOW_SEL;
+	}
+
 	if (!of_property_read_u32(node, "cpts-ext-ts-inputs", &prop))
 		cpts->ext_ts_inputs = prop;
 
@@ -584,6 +799,7 @@  struct cpts *cpts_create(struct device *dev, void __iomem *regs,
 	cpts->dev = dev;
 	cpts->reg = (struct cpsw_cpts __iomem *)regs;
 	spin_lock_init(&cpts->lock);
+	mutex_init(&cpts->ptp_clk_mutex);
 	INIT_DELAYED_WORK(&cpts->overflow_work, cpts_overflow_check);
 
 	ret = cpts_of_parse(cpts, node);
@@ -608,6 +824,9 @@  struct cpts *cpts_create(struct device *dev, void __iomem *regs,
 	if (cpts->ext_ts_inputs)
 		cpts->info.n_ext_ts = cpts->ext_ts_inputs;
 
+	if (cpts->caps & CPTS_CAP_TS_COMP_EN)
+		cpts->info.pps = 1;
+
 	cpts_calc_mult_shift(cpts);
 
 	return cpts;
diff --git a/drivers/net/ethernet/ti/cpts.h b/drivers/net/ethernet/ti/cpts.h
index ad80c95..a82520d 100644
--- a/drivers/net/ethernet/ti/cpts.h
+++ b/drivers/net/ethernet/ti/cpts.h
@@ -39,7 +39,8 @@  struct cpsw_cpts {
 	u32 ts_push;              /* Time stamp event push */
 	u32 ts_load_val;          /* Time stamp load value */
 	u32 ts_load_en;           /* Time stamp load enable */
-	u32 res2[2];
+	u32 ts_comp_val;          /* Time stamp comparison value, v1.5 & up */
+	u32 ts_comp_length;       /* Time stamp comp assert len, v1.5 & up */
 	u32 intstat_raw;          /* Time sync interrupt status raw */
 	u32 intstat_masked;       /* Time sync interrupt status masked */
 	u32 int_enable;           /* Time sync interrupt enable */
@@ -64,11 +65,14 @@  struct cpsw_cpts {
 #define HW3_TS_PUSH_EN       (1<<10) /* Hardware push 3 enable */
 #define HW2_TS_PUSH_EN       (1<<9)  /* Hardware push 2 enable */
 #define HW1_TS_PUSH_EN       (1<<8)  /* Hardware push 1 enable */
+#define TS_COMP_POL	     BIT(2)  /* TS_COMP Polarity */
 #define INT_TEST             (1<<1)  /* Interrupt Test */
 #define CPTS_EN              (1<<0)  /* Time Sync Enable */
 
 #define CPTS_RFTCLK_SEL_MASK 0x1f
 
+#define CPTS_TS_COMP_LENGTH_MASK 0xffff
+
 /*
  * Definitions for the single bit resisters:
  * TS_PUSH TS_LOAD_EN  INTSTAT_RAW INTSTAT_MASKED INT_ENABLE EVENT_POP
@@ -97,6 +101,7 @@  enum {
 	CPTS_EV_HW,   /* Hardware Time Stamp Push Event */
 	CPTS_EV_RX,   /* Ethernet Receive Event */
 	CPTS_EV_TX,   /* Ethernet Transmit Event */
+	CPTS_EV_COMP, /* Time Stamp Compare Event */
 };
 
 #define CPTS_FIFO_DEPTH 16
@@ -113,6 +118,8 @@  struct cpts_event {
 };
 
 #define CPTS_CAP_RFTCLK_SEL BIT(0)
+#define CPTS_CAP_TS_COMP_EN BIT(1)
+#define CPTS_CAP_TS_COMP_POL_LOW_SEL BIT(2)
 
 struct cpts {
 	struct device *dev;
@@ -137,6 +144,11 @@  struct cpts {
 	u32 ext_ts_inputs;
 	u32 hw_ts_enable;
 	u32 caps;
+	u32 ts_comp_next;	/* next time_stamp value to compare with */
+	u32 ts_comp_length;	/* TS_COMP Output pulse width */
+	u32 ts_comp_one_sec_cycs; /* number of counter cycles in one sec */
+	int ts_comp_enabled;
+	struct mutex ptp_clk_mutex; /* sync PTP interface with overflow_work */
 };
 
 void cpts_rx_timestamp(struct cpts *cpts, struct sk_buff *skb);