From: Vincent Cheng vincent.cheng.xh@renesas.com
This series adds adjust phase to the PTP Hardware Clock device interface.
Some PTP hardware clocks have a write phase mode that has a built-in hardware filtering capability. The write phase mode utilizes a phase offset control word instead of a frequency offset control word. Add adjust phase function to take advantage of this capability.
Changes since v1: - As suggested by Richard Cochran: 1. ops->adjphase is new so need to check for non-null function pointer. 2. Kernel coding style uses lower_case_underscores. 3. Use existing PTP clock API for delayed worker.
Vincent Cheng (3): ptp: Add adjphase function to support phase offset control. ptp: Add adjust_phase to ptp_clock_caps capability. ptp: ptp_clockmatrix: Add adjphase() to support PHC write phase mode.
drivers/ptp/ptp_chardev.c | 1 + drivers/ptp/ptp_clock.c | 3 ++ drivers/ptp/ptp_clockmatrix.c | 92 +++++++++++++++++++++++++++++++++++ drivers/ptp/ptp_clockmatrix.h | 8 ++- include/linux/ptp_clock_kernel.h | 6 ++- include/uapi/linux/ptp_clock.h | 4 +- tools/testing/selftests/ptp/testptp.c | 6 ++- 7 files changed, 114 insertions(+), 6 deletions(-)
From: Vincent Cheng vincent.cheng.xh@renesas.com
Adds adjust phase function to take advantage of a PHC clock's hardware filtering capability that uses phase offset control word instead of frequency offset control word.
Signed-off-by: Vincent Cheng vincent.cheng.xh@renesas.com --- drivers/ptp/ptp_clock.c | 3 +++ include/linux/ptp_clock_kernel.h | 6 +++++- 2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c index acabbe7..fc984a8 100644 --- a/drivers/ptp/ptp_clock.c +++ b/drivers/ptp/ptp_clock.c @@ -146,6 +146,9 @@ static int ptp_clock_adjtime(struct posix_clock *pc, struct __kernel_timex *tx) else err = ops->adjfreq(ops, ppb); ptp->dialed_frequency = tx->freq; + } else if (tx->modes & ADJ_OFFSET) { + if (ops->adjphase) + err = ops->adjphase(ops, tx->offset); } else if (tx->modes == 0) { tx->freq = ptp->dialed_frequency; err = 0; diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h index 121a7ed..31144d9 100644 --- a/include/linux/ptp_clock_kernel.h +++ b/include/linux/ptp_clock_kernel.h @@ -36,7 +36,7 @@ struct ptp_system_timestamp { };
/** - * struct ptp_clock_info - decribes a PTP hardware clock + * struct ptp_clock_info - describes a PTP hardware clock * * @owner: The clock driver should set to THIS_MODULE. * @name: A short "friendly name" to identify the clock and to @@ -65,6 +65,9 @@ struct ptp_system_timestamp { * parameter delta: Desired frequency offset from nominal frequency * in parts per billion * + * @adjphase: Adjusts the phase offset of the hardware clock. + * parameter delta: Desired change in nanoseconds. + * * @adjtime: Shifts the time of the hardware clock. * parameter delta: Desired change in nanoseconds. * @@ -128,6 +131,7 @@ struct ptp_clock_info { struct ptp_pin_desc *pin_config; int (*adjfine)(struct ptp_clock_info *ptp, long scaled_ppm); int (*adjfreq)(struct ptp_clock_info *ptp, s32 delta); + int (*adjphase)(struct ptp_clock_info *ptp, s32 phase); int (*adjtime)(struct ptp_clock_info *ptp, s64 delta); int (*gettime64)(struct ptp_clock_info *ptp, struct timespec64 *ts); int (*gettimex64)(struct ptp_clock_info *ptp, struct timespec64 *ts,
On Fri, May 01, 2020 at 11:35:36PM -0400, vincent.cheng.xh@renesas.com wrote:
From: Vincent Cheng vincent.cheng.xh@renesas.com
Adds adjust phase function to take advantage of a PHC clock's hardware filtering capability that uses phase offset control word instead of frequency offset control word.
Signed-off-by: Vincent Cheng vincent.cheng.xh@renesas.com
Reviewed-by: Richard Cochran richardcochran@gmail.com
On 5/2/2020 6:35 AM, vincent.cheng.xh@renesas.com wrote:
From: Vincent Cheng vincent.cheng.xh@renesas.com
Adds adjust phase function to take advantage of a PHC clock's hardware filtering capability that uses phase offset control word instead of frequency offset control word.
Signed-off-by: Vincent Cheng vincent.cheng.xh@renesas.com
drivers/ptp/ptp_clock.c | 3 +++ include/linux/ptp_clock_kernel.h | 6 +++++- 2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c index acabbe7..fc984a8 100644 --- a/drivers/ptp/ptp_clock.c +++ b/drivers/ptp/ptp_clock.c @@ -146,6 +146,9 @@ static int ptp_clock_adjtime(struct posix_clock *pc, struct __kernel_timex *tx) else err = ops->adjfreq(ops, ppb); ptp->dialed_frequency = tx->freq;
- } else if (tx->modes & ADJ_OFFSET) {
if (ops->adjphase)err = ops->adjphase(ops, tx->offset);diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h index 121a7ed..31144d9 100644 --- a/include/linux/ptp_clock_kernel.h +++ b/include/linux/ptp_clock_kernel.h @@ -36,7 +36,7 @@ struct ptp_system_timestamp { }; /**
- struct ptp_clock_info - decribes a PTP hardware clock
- struct ptp_clock_info - describes a PTP hardware clock
- @owner: The clock driver should set to THIS_MODULE.
- @name: A short "friendly name" to identify the clock and to
@@ -65,6 +65,9 @@ struct ptp_system_timestamp {
parameter delta: Desired frequency offset from nominal frequencyin parts per billion
- @adjphase: Adjusts the phase offset of the hardware clock.
parameter delta: Desired change in nanoseconds.
- @adjtime: Shifts the time of the hardware clock.
parameter delta: Desired change in nanoseconds.@@ -128,6 +131,7 @@ struct ptp_clock_info { struct ptp_pin_desc *pin_config; int (*adjfine)(struct ptp_clock_info *ptp, long scaled_ppm); int (*adjfreq)(struct ptp_clock_info *ptp, s32 delta);
- int (*adjphase)(struct ptp_clock_info *ptp, s32 phase);
Hi,
Please explain the difference in the output between adjphase and adjtime. I'd expect both to add delta to current time. Am I missing something?
Thanks, Aya
int (*adjtime)(struct ptp_clock_info *ptp, s64 delta); int (*gettime64)(struct ptp_clock_info *ptp, struct timespec64 *ts); int (*gettimex64)(struct ptp_clock_info *ptp, struct timespec64 *ts,
Hi Aya,
- @adjphase: Adjusts the phase offset of the hardware clock.
parameter delta: Desired change in nanoseconds.
- @adjtime: Shifts the time of the hardware clock.
parameter delta: Desired change in nanoseconds.@@ -128,6 +131,7 @@ struct ptp_clock_info { struct ptp_pin_desc *pin_config; int (*adjfine)(struct ptp_clock_info *ptp, long scaled_ppm); int (*adjfreq)(struct ptp_clock_info *ptp, s32 delta);
- int (*adjphase)(struct ptp_clock_info *ptp, s32 phase);
Hi,
Please explain the difference in the output between adjphase and adjtime. I'd expect both to add delta to current time. Am I missing something?
Yes, both add delta to the current time and the 1 PPS should arrive at the same location.
adjtime modifies HW counter with a value to move the 1 PPS abruptly to new location. adjphase modifies the frequency to quickly nudge the 1 PPS to new location and also includes a HW filter to smooth out the adjustments and fine tune frequency.
Continuous small offset adjustments using adjtime, likley see sudden shifts of the 1 PPS. The 1 PPS probably disappears and re-appears. Continuous small offset adjustments using adjphase, should see continuous 1 PPS.
adjtime is good for large offset corrections adjphase is good for small offset corrections to allow HW filter to control the frequency instead of relying on SW filter.
On interruption of timestamps, adjphase HW can hold the frequency steady better since it has history of corrections.
ptp4l will switch to adjphase only if servo_offset_threshold and servo_num_offset_values conditions are met, ie. when offsets are stabalized below servo_offset_threshold.
adjphase is most useful for scnearios with minimal PDV.
Hope that helps.
Thanks, Vincent
Thanks, Aya
int (*adjtime)(struct ptp_clock_info *ptp, s64 delta); int (*gettime64)(struct ptp_clock_info *ptp, struct timespec64 *ts); int (*gettimex64)(struct ptp_clock_info *ptp, struct timespec64 *ts,
From: Vincent Cheng vincent.cheng.xh@renesas.com
Add adjust_phase to ptp_clock_caps capability to allow user to query if a PHC driver supports adjust phase with ioctl PTP_CLOCK_GETCAPS command.
Signed-off-by: Vincent Cheng vincent.cheng.xh@renesas.com Reviewed-by: Richard Cochran richardcochran@gmail.com --- drivers/ptp/ptp_chardev.c | 1 + include/uapi/linux/ptp_clock.h | 4 +++- tools/testing/selftests/ptp/testptp.c | 6 ++++-- 3 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c index 93d574f..375cd6e 100644 --- a/drivers/ptp/ptp_chardev.c +++ b/drivers/ptp/ptp_chardev.c @@ -136,6 +136,7 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg) caps.pps = ptp->info->pps; caps.n_pins = ptp->info->n_pins; caps.cross_timestamping = ptp->info->getcrosststamp != NULL; + caps.adjust_phase = ptp->info->adjphase != NULL; if (copy_to_user((void __user *)arg, &caps, sizeof(caps))) err = -EFAULT; break; diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h index 9dc9d00..ff070aa 100644 --- a/include/uapi/linux/ptp_clock.h +++ b/include/uapi/linux/ptp_clock.h @@ -89,7 +89,9 @@ struct ptp_clock_caps { int n_pins; /* Number of input/output pins. */ /* Whether the clock supports precise system-device cross timestamps */ int cross_timestamping; - int rsv[13]; /* Reserved for future use. */ + /* Whether the clock supports adjust phase */ + int adjust_phase; + int rsv[12]; /* Reserved for future use. */ };
struct ptp_extts_request { diff --git a/tools/testing/selftests/ptp/testptp.c b/tools/testing/selftests/ptp/testptp.c index c0dd102..da7a9dd 100644 --- a/tools/testing/selftests/ptp/testptp.c +++ b/tools/testing/selftests/ptp/testptp.c @@ -269,14 +269,16 @@ int main(int argc, char *argv[]) " %d programmable periodic signals\n" " %d pulse per second\n" " %d programmable pins\n" - " %d cross timestamping\n", + " %d cross timestamping\n" + " %d adjust_phase\n", caps.max_adj, caps.n_alarm, caps.n_ext_ts, caps.n_per_out, caps.pps, caps.n_pins, - caps.cross_timestamping); + caps.cross_timestamping, + caps.adjust_phase); } }
From: Vincent Cheng vincent.cheng.xh@renesas.com
Add idtcm_adjphase() to support PHC write phase mode.
Signed-off-by: Vincent Cheng vincent.cheng.xh@renesas.com --- drivers/ptp/ptp_clockmatrix.c | 92 +++++++++++++++++++++++++++++++++++++++++++ drivers/ptp/ptp_clockmatrix.h | 8 +++- 2 files changed, 98 insertions(+), 2 deletions(-)
diff --git a/drivers/ptp/ptp_clockmatrix.c b/drivers/ptp/ptp_clockmatrix.c index a3f6088..ceb6bc5 100644 --- a/drivers/ptp/ptp_clockmatrix.c +++ b/drivers/ptp/ptp_clockmatrix.c @@ -10,6 +10,7 @@ #include <linux/module.h> #include <linux/ptp_clock_kernel.h> #include <linux/delay.h> +#include <linux/jiffies.h> #include <linux/kernel.h> #include <linux/timekeeping.h>
@@ -24,6 +25,16 @@ MODULE_LICENSE("GPL");
#define SETTIME_CORRECTION (0)
+static long set_write_phase_ready(struct ptp_clock_info *ptp) +{ + struct idtcm_channel *channel = + container_of(ptp, struct idtcm_channel, caps); + + channel->write_phase_ready = 1; + + return 0; +} + static int char_array_to_timespec(u8 *buf, u8 count, struct timespec64 *ts) @@ -871,6 +882,64 @@ static int idtcm_set_pll_mode(struct idtcm_channel *channel,
/* PTP Hardware Clock interface */
+/** + * @brief Maximum absolute value for write phase offset in picoseconds + * + * Destination signed register is 32-bit register in resolution of 50ps + * + * 0x7fffffff * 50 = 2147483647 * 50 = 107374182350 + */ +static int _idtcm_adjphase(struct idtcm_channel *channel, s32 delta_ns) +{ + struct idtcm *idtcm = channel->idtcm; + + int err; + u8 i; + u8 buf[4] = {0}; + s32 phase_50ps; + s64 offset_ps; + + if (channel->pll_mode != PLL_MODE_WRITE_PHASE) { + + err = idtcm_set_pll_mode(channel, PLL_MODE_WRITE_PHASE); + + if (err) + return err; + + channel->write_phase_ready = 0; + + ptp_schedule_worker(channel->ptp_clock, + msecs_to_jiffies(WR_PHASE_SETUP_MS)); + } + + if (!channel->write_phase_ready) + delta_ns = 0; + + offset_ps = (s64)delta_ns * 1000; + + /* + * Check for 32-bit signed max * 50: + * + * 0x7fffffff * 50 = 2147483647 * 50 = 107374182350 + */ + if (offset_ps > MAX_ABS_WRITE_PHASE_PICOSECONDS) + offset_ps = MAX_ABS_WRITE_PHASE_PICOSECONDS; + else if (offset_ps < -MAX_ABS_WRITE_PHASE_PICOSECONDS) + offset_ps = -MAX_ABS_WRITE_PHASE_PICOSECONDS; + + phase_50ps = DIV_ROUND_CLOSEST(div64_s64(offset_ps, 50), 1); + + for (i = 0; i < 4; i++) { + buf[i] = phase_50ps & 0xff; + phase_50ps >>= 8; + } + + err = idtcm_write(idtcm, channel->dpll_phase, DPLL_WR_PHASE, + buf, sizeof(buf)); + + return err; +} + static int idtcm_adjfreq(struct ptp_clock_info *ptp, s32 ppb) { struct idtcm_channel *channel = @@ -977,6 +1046,24 @@ static int idtcm_adjtime(struct ptp_clock_info *ptp, s64 delta) return err; }
+static int idtcm_adjphase(struct ptp_clock_info *ptp, s32 delta) +{ + struct idtcm_channel *channel = + container_of(ptp, struct idtcm_channel, caps); + + struct idtcm *idtcm = channel->idtcm; + + int err; + + mutex_lock(&idtcm->reg_lock); + + err = _idtcm_adjphase(channel, delta); + + mutex_unlock(&idtcm->reg_lock); + + return err; +} + static int idtcm_enable(struct ptp_clock_info *ptp, struct ptp_clock_request *rq, int on) { @@ -1055,13 +1142,16 @@ static const struct ptp_clock_info idtcm_caps = { .owner = THIS_MODULE, .max_adj = 244000, .n_per_out = 1, + .adjphase = &idtcm_adjphase, .adjfreq = &idtcm_adjfreq, .adjtime = &idtcm_adjtime, .gettime64 = &idtcm_gettime, .settime64 = &idtcm_settime, .enable = &idtcm_enable, + .do_aux_work = &set_write_phase_ready, };
+ static int idtcm_enable_channel(struct idtcm *idtcm, u32 index) { struct idtcm_channel *channel; @@ -1146,6 +1236,8 @@ static int idtcm_enable_channel(struct idtcm *idtcm, u32 index) if (!channel->ptp_clock) return -ENOTSUPP;
+ channel->write_phase_ready = 0; + dev_info(&idtcm->client->dev, "PLL%d registered as ptp%d\n", index, channel->ptp_clock->index);
diff --git a/drivers/ptp/ptp_clockmatrix.h b/drivers/ptp/ptp_clockmatrix.h index 6c1f93a..3de0eb7 100644 --- a/drivers/ptp/ptp_clockmatrix.h +++ b/drivers/ptp/ptp_clockmatrix.h @@ -15,6 +15,8 @@ #define FW_FILENAME "idtcm.bin" #define MAX_PHC_PLL 4
+#define MAX_ABS_WRITE_PHASE_PICOSECONDS (107374182350LL) + #define PLL_MASK_ADDR (0xFFA5) #define DEFAULT_PLL_MASK (0x04)
@@ -33,8 +35,9 @@
#define POST_SM_RESET_DELAY_MS (3000) #define PHASE_PULL_IN_THRESHOLD_NS (150000) -#define TOD_WRITE_OVERHEAD_COUNT_MAX (5) -#define TOD_BYTE_COUNT (11) +#define TOD_WRITE_OVERHEAD_COUNT_MAX (5) +#define TOD_BYTE_COUNT (11) +#define WR_PHASE_SETUP_MS (5000)
/* Values of DPLL_N.DPLL_MODE.PLL_MODE */ enum pll_mode { @@ -77,6 +80,7 @@ struct idtcm_channel { u16 hw_dpll_n; enum pll_mode pll_mode; u16 output_mask; + int write_phase_ready; };
struct idtcm {
On Fri, May 01, 2020 at 11:35:38PM -0400, vincent.cheng.xh@renesas.com wrote:
From: Vincent Cheng vincent.cheng.xh@renesas.com
Add idtcm_adjphase() to support PHC write phase mode.
Signed-off-by: Vincent Cheng vincent.cheng.xh@renesas.com
Acked-by: Richard Cochran richardcochran@gmail.com
From: vincent.cheng.xh@renesas.com Date: Fri, 1 May 2020 23:35:35 -0400
From: Vincent Cheng vincent.cheng.xh@renesas.com
This series adds adjust phase to the PTP Hardware Clock device interface.
Some PTP hardware clocks have a write phase mode that has a built-in hardware filtering capability. The write phase mode utilizes a phase offset control word instead of a frequency offset control word. Add adjust phase function to take advantage of this capability.
Changes since v1:
- As suggested by Richard Cochran:
- ops->adjphase is new so need to check for non-null function pointer.
- Kernel coding style uses lower_case_underscores.
- Use existing PTP clock API for delayed worker.
Series applied.
linux-kselftest-mirror@lists.linaro.org
-
Aya Levin -
David Miller -
Richard Cochran -
Vincent Cheng -
vincent.cheng.xh＠renesas.com