linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] ptp_qoriq: fix latency in ptp_qoriq_adjtime() operation.
@ 2023-01-16 10:24 nikhil.gupta
  2023-01-16 13:02 ` Vladimir Oltean
  0 siblings, 1 reply; 2+ messages in thread
From: nikhil.gupta @ 2023-01-16 10:24 UTC (permalink / raw)
  To: linux-arm-kernel, Yangbo Lu, netdev, linux-kernel
  Cc: vakul.garg, rajan.gupta, richardcochran, Nikhil Gupta

From: Nikhil Gupta <nikhil.gupta@nxp.com>

1588 driver loses about 1us in adjtime operation at PTP slave.
This is because adjtime operation uses a slow non-atomic tmr_cnt_read()
followed by tmr_cnt_write() operation.
In the above sequence, since the timer counter operation keeps
incrementing, it leads to latency.
The tmr_offset register (which is added to TMR_CNT_H/L register
gives the current time) must be programmed with the delta
nanoseconds.

Signed-off-by: Nikhil Gupta <nikhil.gupta@nxp.com>
Reviewed-by: Yangbo Lu <yangbo.lu@nxp.com>
---
 drivers/ptp/ptp_qoriq.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/drivers/ptp/ptp_qoriq.c b/drivers/ptp/ptp_qoriq.c
index 08f4cf0ad9e3..69fa77b99b45 100644
--- a/drivers/ptp/ptp_qoriq.c
+++ b/drivers/ptp/ptp_qoriq.c
@@ -48,6 +48,29 @@ static void tmr_cnt_write(struct ptp_qoriq *ptp_qoriq, u64 ns)
 	ptp_qoriq->write(&regs->ctrl_regs->tmr_cnt_h, hi);
 }
 
+static void tmr_offset_write(struct ptp_qoriq *ptp_qoriq, u64 delta_ns)
+{
+	struct ptp_qoriq_registers *regs = &ptp_qoriq->regs;
+	u32 hi = delta_ns >> 32;
+	u32 lo = delta_ns & 0xffffffff;
+
+	ptp_qoriq->write(&regs->ctrl_regs->tmroff_l, lo);
+	ptp_qoriq->write(&regs->ctrl_regs->tmroff_h, hi);
+}
+
+static u64 tmr_offset_read(struct ptp_qoriq *ptp_qoriq)
+{
+	struct ptp_qoriq_registers *regs = &ptp_qoriq->regs;
+	u64 ns;
+	u32 lo, hi;
+
+	lo = ptp_qoriq->read(&regs->ctrl_regs->tmroff_l);
+	hi = ptp_qoriq->read(&regs->ctrl_regs->tmroff_h);
+	ns = ((u64) hi) << 32;
+	ns |= lo;
+	return ns;
+}
+
 /* Caller must hold ptp_qoriq->lock. */
 static void set_alarm(struct ptp_qoriq *ptp_qoriq)
 {
@@ -55,7 +78,9 @@ static void set_alarm(struct ptp_qoriq *ptp_qoriq)
 	u64 ns;
 	u32 lo, hi;
 
-	ns = tmr_cnt_read(ptp_qoriq) + 1500000000ULL;
+	ns = tmr_cnt_read(ptp_qoriq) + tmr_offset_read(ptp_qoriq)
+				     + 1500000000ULL;
+
 	ns = div_u64(ns, 1000000000UL) * 1000000000ULL;
 	ns -= ptp_qoriq->tclk_period;
 	hi = ns >> 32;
@@ -212,10 +237,9 @@ int ptp_qoriq_adjtime(struct ptp_clock_info *ptp, s64 delta)
 	struct ptp_qoriq *ptp_qoriq = container_of(ptp, struct ptp_qoriq, caps);
 
 	spin_lock_irqsave(&ptp_qoriq->lock, flags);
-
-	now = tmr_cnt_read(ptp_qoriq);
+	now = tmr_offset_read(ptp_qoriq);
 	now += delta;
-	tmr_cnt_write(ptp_qoriq, now);
+	tmr_offset_write(ptp_qoriq, now);
 	set_fipers(ptp_qoriq);
 
 	spin_unlock_irqrestore(&ptp_qoriq->lock, flags);
@@ -232,7 +256,7 @@ int ptp_qoriq_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
 
 	spin_lock_irqsave(&ptp_qoriq->lock, flags);
 
-	ns = tmr_cnt_read(ptp_qoriq);
+	ns = tmr_cnt_read(ptp_qoriq) + tmr_offset_read(ptp_qoriq);
 
 	spin_unlock_irqrestore(&ptp_qoriq->lock, flags);
 
@@ -253,6 +277,7 @@ int ptp_qoriq_settime(struct ptp_clock_info *ptp,
 
 	spin_lock_irqsave(&ptp_qoriq->lock, flags);
 
+	tmr_offset_write(ptp_qoriq, 0);
 	tmr_cnt_write(ptp_qoriq, ns);
 	set_fipers(ptp_qoriq);
 
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v1] ptp_qoriq: fix latency in ptp_qoriq_adjtime() operation.
  2023-01-16 10:24 [PATCH v1] ptp_qoriq: fix latency in ptp_qoriq_adjtime() operation nikhil.gupta
@ 2023-01-16 13:02 ` Vladimir Oltean
  0 siblings, 0 replies; 2+ messages in thread
From: Vladimir Oltean @ 2023-01-16 13:02 UTC (permalink / raw)
  To: nikhil.gupta
  Cc: linux-arm-kernel, Yangbo Lu, netdev, linux-kernel, vakul.garg,
	rajan.gupta, richardcochran

Hi Nikhil,

On Mon, Jan 16, 2023 at 03:54:40PM +0530, nikhil.gupta@nxp.com wrote:
> From: Nikhil Gupta <nikhil.gupta@nxp.com>
> 
> 1588 driver loses about 1us in adjtime operation at PTP slave.
> This is because adjtime operation uses a slow non-atomic tmr_cnt_read()
> followed by tmr_cnt_write() operation.
> In the above sequence, since the timer counter operation keeps
> incrementing, it leads to latency.
> The tmr_offset register (which is added to TMR_CNT_H/L register
> gives the current time) must be programmed with the delta
> nanoseconds.
> 
> Signed-off-by: Nikhil Gupta <nikhil.gupta@nxp.com>
> Reviewed-by: Yangbo Lu <yangbo.lu@nxp.com>
> ---

Your patch breaks eTSEC1 [ and eTSEC2 ] on LS1021A.

Before:

root@black:~# ip link set eth1 up
root@black:~# [   54.321664] fsl-gianfar soc:ethernet@2d50000 eth1: Link is Up - 1Gbps/Full - flow control off
[   54.331331] IPv6: ADDRCONF(NETDEV_CHANGE): eth1: link becomes ready

root@black:~# ptp4l -i eth1 -2 -P -m
ptp4l[57.231]: selected /dev/ptp0 as PTP clock
ptp4l[57.351]: port 1: INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[57.353]: port 0: INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[57.355]: port 1: link down
ptp4l[57.356]: port 1: LISTENING to FAULTY on FAULT_DETECTED (FT_UNSPECIFIED)
ptp4l[57.455]: selected local clock 001f7b.fffe.630228 as best master
ptp4l[57.458]: port 1: assuming the grand master role
ptp4l[57.464]: selected local clock 001f7b.fffe.630228 as best master
ptp4l[57.466]: port 1: assuming the grand master role
[   60.086366] fsl-gianfar soc:ethernet@2d50000 eth1: Link is Up - 1Gbps/Full - flow control off
ptp4l[60.089]: port 1: link up
ptp4l[60.183]: port 1: FAULTY to LISTENING on INIT_COMPLETE
ptp4l[66.713]: port 1: LISTENING to MASTER on ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES
ptp4l[66.714]: selected local clock 001f7b.fffe.630228 as best master
ptp4l[66.715]: port 1: assuming the grand master role
ptp4l[102.753]: port 1: new foreign master 00049f.fffe.05f4ad-1
ptp4l[106.757]: selected best master clock 00049f.fffe.05f4ad
ptp4l[106.758]: port 1: MASTER to UNCALIBRATED on RS_SLAVE
ptp4l[107.763]: master offset -363290000824 s0 freq      +0 path delay       735
ptp4l[108.764]: master offset -363289992530 s1 freq   +8285 path delay       736
ptp4l[109.767]: master offset      -1648 s2 freq   +6637 path delay       736
ptp4l[109.768]: port 1: UNCALIBRATED to SLAVE on MASTER_CLOCK_SELECTED
ptp4l[110.770]: master offset         -6 s2 freq   +7784 path delay       736
ptp4l[111.772]: master offset        491 s2 freq   +8280 path delay       735
ptp4l[112.773]: master offset        495 s2 freq   +8431 path delay       735
ptp4l[113.776]: master offset        342 s2 freq   +8426 path delay       731
ptp4l[114.778]: master offset        198 s2 freq   +8385 path delay       724
ptp4l[115.781]: master offset        100 s2 freq   +8346 path delay       722

After:

root@black:~# ip link set eth1 up
root@black:~# [  311.001924] fsl-gianfar soc:ethernet@2d50000 eth1: Link is Up - 1Gbps/Full - flow control off
[  311.013236] IPv6: ADDRCONF(NETDEV_CHANGE): eth1: link becomes ready

root@black:~# ptp4l -i eth1 -2 -P -m
ptp4l[333.797]: selected /dev/ptp0 as PTP clock
ptp4l[333.916]: port 1: INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[333.918]: port 0: INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[333.919]: port 1: link down
ptp4l[333.919]: port 1: LISTENING to FAULTY on FAULT_DETECTED (FT_UNSPECIFIED)
ptp4l[334.012]: selected local clock 001f7b.fffe.630228 as best master
ptp4l[334.013]: port 1: assuming the grand master role
[  336.626213] fsl-gianfar soc:ethernet@2d50000 eth1: Link is Up - 1Gbps/Full - flow control off
ptp4l[336.631]: port 1: link up
ptp4l[336.713]: port 1: FAULTY to LISTENING on INIT_COMPLETE
ptp4l[343.832]: port 1: LISTENING to MASTER on ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES
ptp4l[343.833]: selected local clock 001f7b.fffe.630228 as best master
ptp4l[343.834]: port 1: assuming the grand master role
ptp4l[344.873]: port 1: new foreign master 00049f.fffe.05f4ad-1
ptp4l[348.879]: selected best master clock 00049f.fffe.05f4ad
ptp4l[348.879]: port 1: MASTER to UNCALIBRATED on RS_SLAVE
ptp4l[349.888]: master offset 215392521359 s0 freq      +0 path delay       737
ptp4l[350.890]: master offset 215392529808 s1 freq   +8433 path delay       738
ptp4l[351.896]: master offset 215392529845 s2 freq +32767999 path delay       735
ptp4l[351.896]: port 1: UNCALIBRATED to SLAVE on MASTER_CLOCK_SELECTED
ptp4l[352.897]: master offset 215359819000 s2 freq +32767999 path delay       731
ptp4l[353.899]: master offset 215326980590 s2 freq +32767999 path delay       731
ptp4l[354.900]: master offset 215294167912 s2 freq +32767999 path delay       729
ptp4l[355.902]: master offset 215261357366 s2 freq +32767999 path delay       725
ptp4l[356.905]: master offset 215228532618 s2 freq +32767999 path delay    -24741
ptp4l[357.907]: master offset 215195717627 s2 freq +32767999 path delay    -51970
ptp4l[358.909]: master offset 215162898062 s2 freq +32767999 path delay    -57014
ptp4l[359.912]: master offset 215130057602 s2 freq +32767999 path delay    -59714
ptp4l[360.915]: master offset 215097182978 s2 freq +32767999 path delay    -60737
ptp4l[361.917]: master offset 215064357344 s2 freq +32767999 path delay    -60783

I have forwarded you an internal email which gives more details about
how the 1588 block was integrated on the 3 eTSEC ports on LS1021A.
The general takeaway is: please don't break that SoC, and NACK for this
patch.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-01-16 13:04 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-16 10:24 [PATCH v1] ptp_qoriq: fix latency in ptp_qoriq_adjtime() operation nikhil.gupta
2023-01-16 13:02 ` Vladimir Oltean

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).