2023-01-16 11:09:55

by Nikhil Gupta

[permalink] [raw]
Subject: [PATCH v1] ptp_qoriq: fix latency in ptp_qoriq_adjtime() operation.

From: Nikhil Gupta <[email protected]>

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 <[email protected]>
Reviewed-by: Yangbo Lu <[email protected]>
---
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


2023-01-16 13:22:00

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH v1] ptp_qoriq: fix latency in ptp_qoriq_adjtime() operation.

Hi Nikhil,

On Mon, Jan 16, 2023 at 03:54:40PM +0530, [email protected] wrote:
> From: Nikhil Gupta <[email protected]>
>
> 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 <[email protected]>
> Reviewed-by: Yangbo Lu <[email protected]>
> ---

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.