Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v8 0/3] dpll: zl3073x: add PTP clock support
@ 2026-08-14  8:26 Ivan Vecera
  2026-08-14  8:26 ` [PATCH net-next v8 1/3] dpll: zl3073x: scale poll interval proportionally to timeout Ivan Vecera
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Ivan Vecera @ 2026-08-14  8:26 UTC (permalink / raw)
  To: netdev
  Cc: Arkadiusz Kubalewski, Jakub Kicinski, Jiri Pirko, Min Li,
	Paolo Abeni, Petr Oros, Richard Cochran, Vadim Fedorenko,
	linux-kernel

Add PTP hardware clock support to the zl3073x DPLL driver.

Patch 1 scales the poll sleep interval in zl3073x_poll_zero_u8()
proportionally to the timeout to avoid excessive bus traffic for
the longer PTP-related timeouts.

Patch 2 adds low-level channel operations for ToD read/write/adjust,
output phase step, delta frequency offset write and TIE write as
building blocks for PTP callbacks.

Patch 3 registers a PTP clock device for each DPLL channel with
gettimex64, settime64, adjtime, adjfine, adjphase and getmaxphase
callbacks. Callback availability adapts to the current channel
state - adjfine requires NCO pin connected, adjphase uses TIE write
when tracking a reference, and adjtime selects the appropriate
mechanism automatically. Periodic output support will be added in a
follow-up series.

Link: https://lore.kernel.org/netdev/20260811134700.1211010-1-ivecera@redhat.com/
Link: https://lore.kernel.org/netdev/20260807094430.120683-1-ivecera@redhat.com/
Link: https://lore.kernel.org/netdev/20260803140637.102339-1-ivecera@redhat.com/
Link: https://lore.kernel.org/netdev/20260730132150.371376-1-ivecera@redhat.com/
Link: https://lore.kernel.org/netdev/20260713103739.1553284-1-ivecera@redhat.com/
Link: https://lore.kernel.org/netdev/20260708170527.916035-1-ivecera@redhat.com/

Changes:
v8:
  - Drop WARN_ON from tod_ready_wait default case and drop tie_write
    range guard entirely (Vadim)
  - Use timespec64_valid_settod() in tod_adjust (Vadim)
v7:
  - Check SEM bit before CMD in tod_ready_wait so stale CMD bits
    from a prior boot stage do not block ToD operations
  - Move ptp_info template assignment from alloc to register to
    avoid stale getcycles64 pointers on devlink reload
  - Update kernel-doc for tod_ready_wait, adjtime, adjtime_phase_step
    and tie_write to match actual return semantics
  - Update poll interval changelog to mention effect on all callers
v6:
  - Fix kernel-doc warning: move ZL_TOD_MAX_RETRIES define inside
    the function body to avoid confusing the kernel-doc parser
v5:
  - Add prep patch to scale poll interval with timeout
  - Enumerate all ToD commands in tod_ready_wait switch, return
    -EOPNOTSUPP with WARN_ON in default case (Vadim)
  - Handle CMD=0 (power-on default) in tod_ready_wait
  - Rewrite tod_adjust rollover guard as a retry loop that re-reads
    until the margin is sufficient, replacing manual fixup and fsleep
  - Fix "lowest-ID synth" comments to "first enabled synth"
  - Include dev_name() in PTP clock name for multi-device systems
  - Update kernel-doc for adjfine/adjphase to document zero no-op
v4:
  - Move out_step_time_mask to struct zl3073x_dev (device-global)
  - Fix tod_adjust rollover guard for tick-crossing case
  - Drop trailing tod_ready_wait from tod_adjust
  - Fix TOD_CTRL register item count
  - Move phase step registers into existing page 9 section in regs.h
  - Align ToD command define values
  - Update kernel-doc for chan_state_fetch
  - Drop perout support (will rework as follow-up)
  - Replace abs(delta) with explicit range check to avoid S64_MIN UB
  - Add locking context doc to zl3073x_chan_df_offset_set
  - Export zl3073x_chan_tod_ready_wait and wait for ToD completion in
    adjtime before phase step/TIE write to prevent WR_NEXT_1HZ from
    overwriting the sub-second adjustment
v3:
  - Use PTP_1588_CLOCK instead of PTP_1588_CLOCK_OPTIONAL (Jakub)
  - Factor out output pin frequency helper to a separate commit (Jakub)
  - Remove spurious #include <linux/mutex.h> from dpll.h (Jakub)
v2:
  - Squash low-level ops patches (1+3) into patch 1.
  - Squash PTP callback patches (2+4) into patch 2, register PTP
    clock regardless of initial channel state.

Ivan Vecera (3):
  dpll: zl3073x: scale poll interval proportionally to timeout
  dpll: zl3073x: add channel ToD, phase step and TIE operations
  dpll: zl3073x: add PTP clock support

 drivers/dpll/zl3073x/Kconfig |   6 +-
 drivers/dpll/zl3073x/chan.c  | 321 ++++++++++++++++++++++++-
 drivers/dpll/zl3073x/chan.h  |  32 +++
 drivers/dpll/zl3073x/core.c  |  17 +-
 drivers/dpll/zl3073x/core.h  |  23 ++
 drivers/dpll/zl3073x/dpll.c  | 439 +++++++++++++++++++++++++++++++++--
 drivers/dpll/zl3073x/dpll.h  |   5 +
 drivers/dpll/zl3073x/regs.h  |  52 +++++
 8 files changed, 863 insertions(+), 32 deletions(-)


base-commit: 4f93b12cf7b25fbf8e73d222722805b049f0a6d3
-- 
2.54.0


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

* [PATCH net-next v8 1/3] dpll: zl3073x: scale poll interval proportionally to timeout
  2026-08-14  8:26 [PATCH net-next v8 0/3] dpll: zl3073x: add PTP clock support Ivan Vecera
@ 2026-08-14  8:26 ` Ivan Vecera
  2026-08-14  8:26 ` [PATCH net-next v8 2/3] dpll: zl3073x: add channel ToD, phase step and TIE operations Ivan Vecera
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Ivan Vecera @ 2026-08-14  8:26 UTC (permalink / raw)
  To: netdev
  Cc: Vadim Fedorenko, Arkadiusz Kubalewski, Jakub Kicinski, Jiri Pirko,
	Min Li, Paolo Abeni, Petr Oros, Richard Cochran, linux-kernel

Replace the fixed 10 us poll sleep in zl3073x_poll_zero_u8() with
timeout_us / 50, scaling the sleep interval proportionally to the
timeout for all callers.

Testing showed that existing callers (mailbox, HWREG, DF read,
frequency measurement and phase error polls with 25-50 ms timeouts)
typically completed in low hundreds of sleep cycles with the fixed
10 us interval. With the scaled interval the cycle count drops to
single digits. The longer PTP-related timeouts (up to 3000 ms for
phase step) added in the following patches benefit most, avoiding
on the order of 10^5 bus transactions per wait.

Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dpll/zl3073x/core.c b/drivers/dpll/zl3073x/core.c
index 5b2d77f2c2288e..2e8b52c8de5e26 100644
--- a/drivers/dpll/zl3073x/core.c
+++ b/drivers/dpll/zl3073x/core.c
@@ -322,7 +322,7 @@ int zl3073x_write_u48(struct zl3073x_dev *zldev, unsigned int reg, u64 val)
 int zl3073x_poll_zero_u8(struct zl3073x_dev *zldev, unsigned int reg,
 			 u8 mask, unsigned int timeout_us)
 {
-#define ZL_POLL_SLEEP_US 10
+	unsigned int sleep_us = timeout_us / 50;
 	unsigned int val;
 
 	/* Check the register is 8bit */
@@ -336,7 +336,7 @@ int zl3073x_poll_zero_u8(struct zl3073x_dev *zldev, unsigned int reg,
 	reg = ZL_REG_ADDR(reg) + ZL_RANGE_OFFSET;
 
 	return regmap_read_poll_timeout(zldev->regmap, reg, val, !(val & mask),
-					ZL_POLL_SLEEP_US, timeout_us);
+					sleep_us, timeout_us);
 }
 
 int zl3073x_mb_op(struct zl3073x_dev *zldev, unsigned int op_reg, u8 op_val,
-- 
2.54.0


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

* [PATCH net-next v8 2/3] dpll: zl3073x: add channel ToD, phase step and TIE operations
  2026-08-14  8:26 [PATCH net-next v8 0/3] dpll: zl3073x: add PTP clock support Ivan Vecera
  2026-08-14  8:26 ` [PATCH net-next v8 1/3] dpll: zl3073x: scale poll interval proportionally to timeout Ivan Vecera
@ 2026-08-14  8:26 ` Ivan Vecera
  2026-08-16 14:47   ` Ivan Vecera
  2026-08-18 16:37   ` Jakub Kicinski
  2026-08-14  8:26 ` [PATCH net-next v8 3/3] dpll: zl3073x: add PTP clock support Ivan Vecera
  2026-08-18 17:00 ` [PATCH net-next v8 0/3] " patchwork-bot+netdevbpf
  3 siblings, 2 replies; 10+ messages in thread
From: Ivan Vecera @ 2026-08-14  8:26 UTC (permalink / raw)
  To: netdev
  Cc: Petr Oros, Chris du Quesnay, Arkadiusz Kubalewski, Jakub Kicinski,
	Jiri Pirko, Min Li, Paolo Abeni, Richard Cochran, Vadim Fedorenko,
	linux-kernel

Add low-level DPLL channel operations for ToD read/write/adjust,
output phase step, delta frequency offset write and TIE (Time
Interval Error) write. These serve as building blocks for the PTP
clock callbacks added in the next patch.

ToD operations use a wait-before-write pattern to avoid blocking
after each operation.

The tod_ready_wait helper selects the poll timeout based on the
current ToD command - write operations use a longer timeout (1000 ms)
than reads (30 ms).

The ToD read captures system timestamps (ptp_system_timestamp) around
the HW command and completion poll to support cross-timestamping.

The TIE write operation provides sub-picosecond resolution phase
adjustment for modes where the DPLL is tracking a reference
(AUTO and REFLOCK).

Add output step-time mask to struct zl3073x_dev and
zl3073x_dev_out_is_stepped() helper to check if an output
participates in step-time operations.

Reviewed-by: Petr Oros <poros@redhat.com>
Tested-by: Chris du Quesnay <Chris.duQuesnay@microchip.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/chan.c | 321 +++++++++++++++++++++++++++++++++++-
 drivers/dpll/zl3073x/chan.h |  32 ++++
 drivers/dpll/zl3073x/core.c |  13 ++
 drivers/dpll/zl3073x/core.h |  23 +++
 drivers/dpll/zl3073x/regs.h |  52 ++++++
 5 files changed, 439 insertions(+), 2 deletions(-)

diff --git a/drivers/dpll/zl3073x/chan.c b/drivers/dpll/zl3073x/chan.c
index 4ec2cf53dad468..ba4d303d41b47d 100644
--- a/drivers/dpll/zl3073x/chan.c
+++ b/drivers/dpll/zl3073x/chan.c
@@ -3,6 +3,7 @@
 #include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/dev_printk.h>
+#include <linux/ptp_clock_kernel.h>
 #include <linux/string.h>
 #include <linux/types.h>
 
@@ -162,8 +163,8 @@ int zl3073x_chan_nco_mode_set(struct zl3073x_dev *zldev, u8 index)
  * @zldev: pointer to zl3073x_dev structure
  * @index: DPLL channel index to fetch state for
  *
- * Reads the mode_refsel register and reference priority registers for
- * the given DPLL channel and stores the raw values for later use.
+ * Reads the mode_refsel, status and reference priority registers for
+ * the given DPLL channel and stores the values for later use.
  *
  * Return: 0 on success, <0 on error
  */
@@ -234,6 +235,322 @@ const struct zl3073x_chan *zl3073x_chan_state_get(struct zl3073x_dev *zldev,
 	return &zldev->chan[index];
 }
 
+/**
+ * zl3073x_chan_tod_ready_wait - wait for ToD semaphore to clear
+ * @zldev: pointer to zl3073x device
+ * @ch: DPLL channel index
+ *
+ * Checks the ToD control register semaphore bit. If clear, returns
+ * immediately. Otherwise polls until the bit is cleared by the device.
+ *
+ * Return:
+ * * 0			- success
+ * * %-EBUSY		- timeout
+ * * %-EOPNOTSUPP	- unknown command detected
+ * * negative		- other error
+ */
+int zl3073x_chan_tod_ready_wait(struct zl3073x_dev *zldev, u8 ch)
+{
+	unsigned int timeout;
+	u8 tod_ctrl;
+	int rc;
+
+	rc = zl3073x_read_u8(zldev, ZL_REG_DPLL_TOD_CTRL(ch), &tod_ctrl);
+	if (rc)
+		return rc;
+
+	if (!(tod_ctrl & ZL_DPLL_TOD_CTRL_SEM))
+		return 0;
+
+	switch (FIELD_GET(ZL_DPLL_TOD_CTRL_CMD, tod_ctrl)) {
+	case ZL_DPLL_TOD_CTRL_CMD_WR_NEXT_1HZ:
+		timeout = ZL_POLL_TOD_WR_TIMEOUT_US;
+		break;
+	case ZL_DPLL_TOD_CTRL_CMD_RD_CURRENT:
+	case ZL_DPLL_TOD_CTRL_CMD_RD_NEXT_1HZ:
+		timeout = ZL_POLL_TOD_RD_TIMEOUT_US;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	rc = zl3073x_poll_zero_u8(zldev, ZL_REG_DPLL_TOD_CTRL(ch),
+				  ZL_DPLL_TOD_CTRL_SEM, timeout);
+
+	return rc == -ETIMEDOUT ? -EBUSY : rc;
+}
+
+/**
+ * zl3073x_chan_tod_ctrl - issue ToD command
+ * @zldev: pointer to zl3073x device
+ * @ch: DPLL channel index
+ * @cmd: ToD command to execute
+ *
+ * Writes the semaphore and command to dpll_tod_ctrl. The caller must
+ * ensure the device is ready (semaphore clear) before calling and
+ * must wait for completion if needed.
+ *
+ * Return: 0 on success, <0 on error
+ */
+static int zl3073x_chan_tod_ctrl(struct zl3073x_dev *zldev, u8 ch, u8 cmd)
+{
+	return zl3073x_write_u8(zldev, ZL_REG_DPLL_TOD_CTRL(ch),
+				ZL_DPLL_TOD_CTRL_SEM | cmd);
+}
+
+/**
+ * zl3073x_chan_tod_read - read ToD registers after issuing a command
+ * @zldev: pointer to zl3073x device
+ * @ch: DPLL channel index
+ * @next_hz: if true, read predicted ToD at next 1 Hz; otherwise read current
+ * @ts: timespec to store the result
+ * @sts: optional system timestamp pair for cross-timestamping
+ *
+ * Context: Caller must serialize all zl3073x_chan_tod_* calls externally.
+ * Return: 0 on success, <0 on error
+ */
+int zl3073x_chan_tod_read(struct zl3073x_dev *zldev, u8 ch,
+			  bool next_hz, struct timespec64 *ts,
+			  struct ptp_system_timestamp *sts)
+{
+	u32 nsec;
+	u64 sec;
+	u8 cmd;
+	int rc;
+
+	if (next_hz)
+		cmd = ZL_DPLL_TOD_CTRL_CMD_RD_NEXT_1HZ;
+	else
+		cmd = ZL_DPLL_TOD_CTRL_CMD_RD_CURRENT;
+
+	/* Wait for any previous ToD operation to complete */
+	rc = zl3073x_chan_tod_ready_wait(zldev, ch);
+	if (rc)
+		return rc;
+
+	ptp_read_system_prets(sts);
+	rc = zl3073x_chan_tod_ctrl(zldev, ch, cmd);
+	if (rc)
+		return rc;
+
+	rc = zl3073x_chan_tod_ready_wait(zldev, ch);
+	if (rc)
+		return rc;
+	ptp_read_system_postts(sts);
+
+	rc = zl3073x_read_u48(zldev, ZL_REG_DPLL_TOD_SEC(ch), &sec);
+	if (rc)
+		return rc;
+
+	/* HW nanoseconds are always in [0, NSEC_PER_SEC) range */
+	rc = zl3073x_read_u32(zldev, ZL_REG_DPLL_TOD_NS(ch), &nsec);
+	if (rc)
+		return rc;
+
+	ts->tv_sec = sec;
+	ts->tv_nsec = nsec;
+
+	return 0;
+}
+
+/**
+ * zl3073x_chan_tod_write - write ToD registers and trigger 1 Hz update
+ * @zldev: pointer to zl3073x device
+ * @ch: DPLL channel index
+ * @ts: time to set
+ *
+ * Context: Caller must serialize all zl3073x_chan_tod_* calls externally.
+ * Return: 0 on success, <0 on error
+ */
+int zl3073x_chan_tod_write(struct zl3073x_dev *zldev, u8 ch,
+			   struct timespec64 ts)
+{
+	int rc;
+
+	/* Wait for any previous ToD operation to complete */
+	rc = zl3073x_chan_tod_ready_wait(zldev, ch);
+	if (rc)
+		return rc;
+
+	rc = zl3073x_write_u48(zldev, ZL_REG_DPLL_TOD_SEC(ch), ts.tv_sec);
+	if (rc)
+		return rc;
+
+	rc = zl3073x_write_u32(zldev, ZL_REG_DPLL_TOD_NS(ch), ts.tv_nsec);
+	if (rc)
+		return rc;
+
+	return zl3073x_chan_tod_ctrl(zldev, ch,
+				     ZL_DPLL_TOD_CTRL_CMD_WR_NEXT_1HZ);
+}
+
+/**
+ * zl3073x_chan_tod_adjust - atomic ToD read-modify-write with rollover guard
+ * @zldev: pointer to zl3073x device
+ * @ch: DPLL channel index
+ * @delta: time adjustment to apply
+ *
+ * Reads the next-Hz ToD and current ToD, then checks whether enough time
+ * remains before the next 1 Hz rollover to safely complete the write.
+ * Re-reads if the 1 Hz tick crossed between the two reads or if less
+ * than 20 ms remains before the next rollover. Applies @delta and writes
+ * the result back.
+ *
+ * Context: Caller must serialize all zl3073x_chan_tod_* calls externally.
+ * Return: 0 on success, <0 on error
+ */
+int zl3073x_chan_tod_adjust(struct zl3073x_dev *zldev, u8 ch,
+			    struct timespec64 delta)
+{
+#define ZL_TOD_MAX_RETRIES	20
+	static const long threshold_ns = 20 * NSEC_PER_MSEC;
+	struct timespec64 ts_next, ts_cur, diff;
+	int rc, i;
+
+	for (i = 0; i < ZL_TOD_MAX_RETRIES; i++) {
+		rc = zl3073x_chan_tod_read(zldev, ch, true, &ts_next, NULL);
+		if (rc)
+			return rc;
+
+		rc = zl3073x_chan_tod_read(zldev, ch, false, &ts_cur, NULL);
+		if (rc)
+			return rc;
+
+		/* Ensure the 1 Hz tick did not cross between the two reads
+		 * and that enough margin remains to complete the write.
+		 */
+		diff = timespec64_sub(ts_next, ts_cur);
+		if (diff.tv_sec > 0 ||
+		    (!diff.tv_sec && diff.tv_nsec >= threshold_ns))
+			break;
+	}
+	if (i == ZL_TOD_MAX_RETRIES) {
+		dev_warn(zldev->dev,
+			 "DPLL%u ToD adjust failed to get stable margin\n",
+			 ch);
+		return -EBUSY;
+	}
+
+	/* Apply delta to the next-Hz ToD */
+	ts_next = timespec64_add(ts_next, delta);
+	if (!timespec64_valid_settod(&ts_next))
+		return -EINVAL;
+
+	return zl3073x_chan_tod_write(zldev, ch, ts_next);
+#undef ZL_TOD_MAX_RETRIES
+}
+
+/**
+ * zl3073x_chan_df_offset_set - write delta frequency offset to hardware
+ * @zldev: pointer to zl3073x device
+ * @ch: DPLL channel index
+ * @offset: frequency offset in 2^-48 steps
+ *
+ * Context: Caller must hold the per-DPLL lock.
+ * Return: 0 on success, <0 on error
+ */
+int zl3073x_chan_df_offset_set(struct zl3073x_dev *zldev, u8 ch, s64 offset)
+{
+	int rc;
+
+	rc = zl3073x_write_u48(zldev, ZL_REG_DPLL_DF_OFFSET(ch), offset);
+	if (!rc)
+		zldev->chan[ch].df_offset = offset;
+
+	return rc;
+}
+
+/**
+ * zl3073x_chan_tie_write - adjust DPLL phase using TIE write
+ * @zldev: pointer to zl3073x device
+ * @ch: DPLL channel index
+ * @delta_ns: phase adjustment in nanoseconds (must be in (-1s, 1s))
+ *
+ * Converts nanoseconds to TIE units (0.01 ps) and writes TIE data
+ * to the specified channel.
+ *
+ * Return: 0 on success, <0 on error
+ */
+int zl3073x_chan_tie_write(struct zl3073x_dev *zldev, u8 ch, s64 delta_ns)
+{
+	s64 tie_data;
+	int rc;
+
+	guard(mutex)(&zldev->tie_lock);
+
+	/* Wait for any previous TIE operation to complete */
+	rc = zl3073x_poll_zero_u8(zldev, ZL_REG_DPLL_TIE_CTRL,
+				  ZL_DPLL_TIE_CTRL_OP,
+				  ZL_POLL_TIE_WR_TIMEOUT_US);
+	if (rc)
+		return rc;
+
+	/* Convert ns to TIE units (0.01 ps = 10^-14 s) */
+	tie_data = delta_ns * 100000LL;
+
+	rc = zl3073x_write_u48(zldev, ZL_REG_DPLL_TIE_DATA(ch), tie_data);
+	if (rc)
+		return rc;
+
+	rc = zl3073x_write_u8(zldev, ZL_REG_DPLL_TIE_CTRL_MASK, BIT(ch));
+	if (rc)
+		return rc;
+
+	return zl3073x_write_u8(zldev, ZL_REG_DPLL_TIE_CTRL,
+				ZL_DPLL_TIE_CTRL_OP_WR);
+}
+
+/**
+ * zl3073x_chan_phase_step - execute one output phase step operation
+ * @zldev: pointer to zl3073x device
+ * @ch: DPLL channel index
+ * @out_mask: bitmask of outputs to step
+ * @step_cycles: phase step in synthesizer clock cycles
+ * @tod_step: also step the ToD counter
+ *
+ * All masked outputs must use synthesizers of the same frequency since
+ * the step value is in synthesizer clock cycles.
+ *
+ * Return: 0 on success, <0 on error
+ */
+int zl3073x_chan_phase_step(struct zl3073x_dev *zldev, u8 ch,
+			    u16 out_mask, s32 step_cycles,
+			    bool tod_step)
+{
+	u8 ctrl;
+	int rc;
+
+	guard(mutex)(&zldev->phase_step_lock);
+
+	/* Wait for any previous phase step operation to complete */
+	rc = zl3073x_poll_zero_u8(zldev, ZL_REG_OUTPUT_PHASE_STEP_CTRL,
+				  ZL_OUTPUT_PHASE_STEP_CTRL_OP,
+				  ZL_POLL_PHASE_STEP_TIMEOUT_US);
+	if (rc)
+		return rc;
+
+	rc = zl3073x_write_u32(zldev, ZL_REG_OUTPUT_PHASE_STEP_DATA,
+			       step_cycles);
+	if (rc)
+		return rc;
+
+	rc = zl3073x_write_u16(zldev, ZL_REG_OUTPUT_PHASE_STEP_MASK, out_mask);
+	if (rc)
+		return rc;
+
+	rc = zl3073x_write_u8(zldev, ZL_REG_OUTPUT_PHASE_STEP_NUMBER, 1);
+	if (rc)
+		return rc;
+
+	ctrl = FIELD_PREP(ZL_OUTPUT_PHASE_STEP_CTRL_DPLL, ch) |
+	       FIELD_PREP(ZL_OUTPUT_PHASE_STEP_CTRL_OP,
+			  ZL_OUTPUT_PHASE_STEP_CTRL_OP_WRITE);
+	if (tod_step)
+		ctrl |= ZL_OUTPUT_PHASE_STEP_CTRL_TOD_STEP;
+
+	return zl3073x_write_u8(zldev, ZL_REG_OUTPUT_PHASE_STEP_CTRL, ctrl);
+}
+
 /**
  * zl3073x_chan_state_set - commit DPLL channel state changes to hardware
  * @zldev: pointer to zl3073x_dev structure
diff --git a/drivers/dpll/zl3073x/chan.h b/drivers/dpll/zl3073x/chan.h
index dc9c6d95bdee75..99c3b7088f678f 100644
--- a/drivers/dpll/zl3073x/chan.h
+++ b/drivers/dpll/zl3073x/chan.h
@@ -5,10 +5,12 @@
 
 #include <linux/bitfield.h>
 #include <linux/stddef.h>
+#include <linux/time64.h>
 #include <linux/types.h>
 
 #include "regs.h"
 
+struct ptp_system_timestamp;
 struct zl3073x_dev;
 
 /**
@@ -42,6 +44,21 @@ int zl3073x_chan_state_set(struct zl3073x_dev *zldev, u8 index,
 int zl3073x_chan_state_update(struct zl3073x_dev *zldev, u8 index);
 int zl3073x_chan_nco_mode_set(struct zl3073x_dev *zldev, u8 index);
 
+int zl3073x_chan_tod_ready_wait(struct zl3073x_dev *zldev, u8 ch);
+int zl3073x_chan_tod_read(struct zl3073x_dev *zldev, u8 ch,
+			  bool next_hz, struct timespec64 *ts,
+			  struct ptp_system_timestamp *sts);
+int zl3073x_chan_tod_write(struct zl3073x_dev *zldev, u8 ch,
+			   struct timespec64 ts);
+int zl3073x_chan_tod_adjust(struct zl3073x_dev *zldev, u8 ch,
+			    struct timespec64 delta);
+int zl3073x_chan_phase_step(struct zl3073x_dev *zldev, u8 ch,
+			    u16 out_mask, s32 step_cycles, bool tod_step);
+
+int zl3073x_chan_df_offset_set(struct zl3073x_dev *zldev, u8 ch, s64 offset);
+
+int zl3073x_chan_tie_write(struct zl3073x_dev *zldev, u8 ch, s64 delta_ns);
+
 /**
  * zl3073x_chan_df_offset_get - get cached df_offset vs tracked reference
  * @chan: pointer to channel state
@@ -200,6 +217,21 @@ static inline bool zl3073x_chan_mode_is_reflock(const struct zl3073x_chan *chan)
 	return zl3073x_chan_mode_get(chan) == ZL_DPLL_MODE_REFSEL_MODE_REFLOCK;
 }
 
+/**
+ * zl3073x_chan_mode_supports_tie - check if channel mode supports TIE write
+ * @chan: pointer to channel state
+ *
+ * TIE write is supported in AUTO and REFLOCK modes regardless of lock state.
+ *
+ * Return: true if TIE write is supported, false otherwise
+ */
+static inline bool
+zl3073x_chan_mode_supports_tie(const struct zl3073x_chan *chan)
+{
+	return zl3073x_chan_mode_is_auto(chan) ||
+		zl3073x_chan_mode_is_reflock(chan);
+}
+
 /**
  * zl3073x_chan_is_ho_ready - check if holdover is ready
  * @chan: pointer to channel state
diff --git a/drivers/dpll/zl3073x/core.c b/drivers/dpll/zl3073x/core.c
index 2e8b52c8de5e26..230df08e27cd19 100644
--- a/drivers/dpll/zl3073x/core.c
+++ b/drivers/dpll/zl3073x/core.c
@@ -511,6 +511,11 @@ zl3073x_dev_state_fetch(struct zl3073x_dev *zldev)
 	int rc;
 	u8 i;
 
+	rc = zl3073x_read_u16(zldev, ZL_REG_OUTPUT_STEP_TIME_MASK,
+			      &zldev->out_step_time_mask);
+	if (rc)
+		return rc;
+
 	for (i = 0; i < ZL3073X_NUM_REFS; i++) {
 		rc = zl3073x_ref_state_fetch(zldev, i);
 		if (rc) {
@@ -1034,6 +1039,14 @@ int zl3073x_dev_probe(struct zl3073x_dev *zldev)
 	 * and/or polls are required to be done atomically.
 	 */
 	rc = devm_mutex_init(zldev->dev, &zldev->multiop_lock);
+	if (rc)
+		return dev_err_probe(zldev->dev, rc,
+				     "Failed to initialize mutex\n");
+	rc = devm_mutex_init(zldev->dev, &zldev->phase_step_lock);
+	if (rc)
+		return dev_err_probe(zldev->dev, rc,
+				     "Failed to initialize mutex\n");
+	rc = devm_mutex_init(zldev->dev, &zldev->tie_lock);
 	if (rc)
 		return dev_err_probe(zldev->dev, rc,
 				     "Failed to initialize mutex\n");
diff --git a/drivers/dpll/zl3073x/core.h b/drivers/dpll/zl3073x/core.h
index 78dc208f3eea2a..67c10e2595118c 100644
--- a/drivers/dpll/zl3073x/core.h
+++ b/drivers/dpll/zl3073x/core.h
@@ -26,6 +26,10 @@ struct zl3073x_dpll;
 #define ZL_POLL_HWREG_TIMEOUT_US	(50 * USEC_PER_MSEC)
 #define ZL_POLL_MB_TIMEOUT_US		(30 * USEC_PER_MSEC)
 #define ZL_POLL_PHASE_ERR_TIMEOUT_US	(50 * USEC_PER_MSEC)
+#define ZL_POLL_PHASE_STEP_TIMEOUT_US	(3000 * USEC_PER_MSEC)
+#define ZL_POLL_TIE_WR_TIMEOUT_US	(1000 * USEC_PER_MSEC)
+#define ZL_POLL_TOD_RD_TIMEOUT_US	(30 * USEC_PER_MSEC)
+#define ZL_POLL_TOD_WR_TIMEOUT_US	(1000 * USEC_PER_MSEC)
 
 enum zl3073x_flags {
 	ZL3073X_FLAG_REF_PHASE_COMP_32_BIT,
@@ -55,6 +59,8 @@ struct zl3073x_chip_info {
  * @regmap: regmap to access device registers
  * @info: detected chip info
  * @multiop_lock: to serialize multiple register operations
+ * @tie_lock: to serialize TIE write operations
+ * @phase_step_lock: to serialize output phase step operations
  * @ref: array of input references' invariants
  * @out: array of outs' invariants
  * @synth: array of synths' invariants
@@ -63,6 +69,7 @@ struct zl3073x_chip_info {
  * @kworker: thread for periodic work
  * @work: periodic work
  * @clock_id: clock id of the device
+ * @out_step_time_mask: output step-time mask (device-global)
  * @phase_avg_factor: phase offset measurement averaging factor
  * @freq_monitor: is frequency monitor enabled
  */
@@ -71,6 +78,8 @@ struct zl3073x_dev {
 	struct regmap			*regmap;
 	const struct zl3073x_chip_info	*info;
 	struct mutex			multiop_lock;
+	struct mutex			tie_lock;
+	struct mutex			phase_step_lock;
 
 	/* Invariants */
 	struct zl3073x_ref	ref[ZL3073X_NUM_REFS];
@@ -87,6 +96,7 @@ struct zl3073x_dev {
 
 	/* Per-chip parameters */
 	u64			clock_id;
+	u16			out_step_time_mask;
 	u8			phase_avg_factor;
 	bool			freq_monitor;
 };
@@ -308,6 +318,19 @@ zl3073x_dev_out_is_enabled(struct zl3073x_dev *zldev, u8 index)
 	return zl3073x_synth_is_enabled(synth) && zl3073x_out_is_enabled(out);
 }
 
+/**
+ * zl3073x_dev_out_is_stepped - check if output is in step-time mask
+ * @zldev: pointer to zl3073x device
+ * @index: output index
+ *
+ * Return: true if output is affected by step-time operations
+ */
+static inline bool
+zl3073x_dev_out_is_stepped(struct zl3073x_dev *zldev, u8 index)
+{
+	return !!(zldev->out_step_time_mask & BIT(index));
+}
+
 /**
  * zl3073x_dev_out_dpll_get - get DPLL ID the output is driven by
  * @zldev: pointer to zl3073x device
diff --git a/drivers/dpll/zl3073x/regs.h b/drivers/dpll/zl3073x/regs.h
index b70ead7d4495b2..f3a5e1215aa36b 100644
--- a/drivers/dpll/zl3073x/regs.h
+++ b/drivers/dpll/zl3073x/regs.h
@@ -179,6 +179,20 @@
 #define ZL_DPLL_DF_READ_CMD			GENMASK(2, 0)
 #define ZL_DPLL_DF_READ_CMD_ACC_I		4
 
+#define ZL_REG_DPLL_TIE_CTRL			ZL_REG(5, 0x30, 1)
+#define ZL_DPLL_TIE_CTRL_OP			GENMASK(2, 0)
+#define ZL_DPLL_TIE_CTRL_OP_WR			4
+
+#define ZL_REG_DPLL_TIE_CTRL_MASK		ZL_REG(5, 0x31, 1)
+
+#define ZL_REG_DPLL_TOD_CTRL(_idx)					\
+	ZL_REG_IDX(_idx, 5, 0x38, 1, ZL3073X_MAX_CHANNELS, 1)
+#define ZL_DPLL_TOD_CTRL_SEM			BIT(4)
+#define ZL_DPLL_TOD_CTRL_CMD			GENMASK(3, 0)
+#define ZL_DPLL_TOD_CTRL_CMD_WR_NEXT_1HZ	1
+#define ZL_DPLL_TOD_CTRL_CMD_RD_CURRENT		8
+#define ZL_DPLL_TOD_CTRL_CMD_RD_NEXT_1HZ	9
+
 #define ZL_REG_DPLL_MEAS_CTRL			ZL_REG(5, 0x50, 1)
 #define ZL_DPLL_MEAS_CTRL_EN			BIT(0)
 #define ZL_DPLL_MEAS_CTRL_AVG_FACTOR		GENMASK(7, 4)
@@ -193,6 +207,9 @@
 
 /*******************************
  * Register Pages 6-7, DPLL Data
+ *
+ * Per-channel registers with stride 0x20. Channels 0-3 reside on page 6,
+ * channel 4 on page 7.
  *******************************/
 
 #define ZL_REG_DPLL_DF_OFFSET_03(_idx)					\
@@ -202,6 +219,24 @@
 	((_idx) < 4 ? ZL_REG_DPLL_DF_OFFSET_03(_idx) : ZL_REG_DPLL_DF_OFFSET_4)
 #define ZL_DPLL_DF_OFFSET_UNKNOWN	S64_MIN
 
+#define ZL_REG_DPLL_TIE_DATA_03(_idx)					\
+	ZL_REG_IDX(_idx, 6, 0x0C, 6, 4, 0x20)
+#define ZL_REG_DPLL_TIE_DATA_4			ZL_REG(7, 0x0C, 6)
+#define ZL_REG_DPLL_TIE_DATA(_idx)					\
+	((_idx) < 4 ? ZL_REG_DPLL_TIE_DATA_03(_idx) : ZL_REG_DPLL_TIE_DATA_4)
+
+#define ZL_REG_DPLL_TOD_SEC_03(_idx)					\
+	ZL_REG_IDX(_idx, 6, 0x12, 6, 4, 0x20)
+#define ZL_REG_DPLL_TOD_SEC_4			ZL_REG(7, 0x12, 6)
+#define ZL_REG_DPLL_TOD_SEC(_idx)					\
+	((_idx) < 4 ? ZL_REG_DPLL_TOD_SEC_03(_idx) : ZL_REG_DPLL_TOD_SEC_4)
+
+#define ZL_REG_DPLL_TOD_NS_03(_idx)					\
+	ZL_REG_IDX(_idx, 6, 0x18, 4, 4, 0x20)
+#define ZL_REG_DPLL_TOD_NS_4			ZL_REG(7, 0x18, 4)
+#define ZL_REG_DPLL_TOD_NS(_idx)					\
+	((_idx) < 4 ? ZL_REG_DPLL_TOD_NS_03(_idx) : ZL_REG_DPLL_TOD_NS_4)
+
 /***********************************
  * Register Page 9, Synth and Output
  ***********************************/
@@ -221,6 +256,23 @@
 #define ZL_OUTPUT_CTRL_EN			BIT(0)
 #define ZL_OUTPUT_CTRL_SYNTH_SEL		GENMASK(6, 4)
 
+#define ZL_REG_OUTPUT_STEP_TIME_MASK		ZL_REG(9, 0x36, 2)
+
+#define ZL_REG_OUTPUT_PHASE_STEP_CTRL		ZL_REG(9, 0x38, 1)
+#define ZL_OUTPUT_PHASE_STEP_CTRL_DPLL		GENMASK(6, 4)
+#define ZL_OUTPUT_PHASE_STEP_CTRL_TOD_STEP	BIT(3)
+#define ZL_OUTPUT_PHASE_STEP_CTRL_OP		GENMASK(1, 0)
+#define ZL_OUTPUT_PHASE_STEP_CTRL_OP_NONE	0
+#define ZL_OUTPUT_PHASE_STEP_CTRL_OP_RESET	1
+#define ZL_OUTPUT_PHASE_STEP_CTRL_OP_READ	2
+#define ZL_OUTPUT_PHASE_STEP_CTRL_OP_WRITE	3
+
+#define ZL_REG_OUTPUT_PHASE_STEP_NUMBER		ZL_REG(9, 0x39, 1)
+
+#define ZL_REG_OUTPUT_PHASE_STEP_MASK		ZL_REG(9, 0x3a, 2)
+
+#define ZL_REG_OUTPUT_PHASE_STEP_DATA		ZL_REG(9, 0x3c, 4)
+
 /*******************************
  * Register Page 10, Ref Mailbox
  *******************************/
-- 
2.54.0


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

* [PATCH net-next v8 3/3] dpll: zl3073x: add PTP clock support
  2026-08-14  8:26 [PATCH net-next v8 0/3] dpll: zl3073x: add PTP clock support Ivan Vecera
  2026-08-14  8:26 ` [PATCH net-next v8 1/3] dpll: zl3073x: scale poll interval proportionally to timeout Ivan Vecera
  2026-08-14  8:26 ` [PATCH net-next v8 2/3] dpll: zl3073x: add channel ToD, phase step and TIE operations Ivan Vecera
@ 2026-08-14  8:26 ` Ivan Vecera
  2026-08-16 14:52   ` Ivan Vecera
  2026-08-18 16:37   ` Jakub Kicinski
  2026-08-18 17:00 ` [PATCH net-next v8 0/3] " patchwork-bot+netdevbpf
  3 siblings, 2 replies; 10+ messages in thread
From: Ivan Vecera @ 2026-08-14  8:26 UTC (permalink / raw)
  To: netdev
  Cc: Petr Oros, Vadim Fedorenko, Chris du Quesnay,
	Arkadiusz Kubalewski, Jakub Kicinski, Jiri Pirko, Min Li,
	Paolo Abeni, Richard Cochran, linux-kernel

Add PTP clock support for the ZL3073x DPLL driver. A PTP clock device
is registered for each DPLL channel regardless of the initial channel
state, providing gettimex64, settime64, adjtime, adjfine, adjphase
and getmaxphase callbacks.

Callback availability depends on the current channel state:
- adjfine: when NCO pin is connected (returns -EOPNOTSUPP otherwise)
- adjphase: available when tracking a reference, uses TIE write
- adjtime: always available and uses
  * phase step for sub-second deltas when NCO pin is connected
  * TIE write when tracking a reference
  * plain ToD read-modify-write otherwise
- gettime/settime: always available

The adjtime callback splits multi-second adjustments into a ToD
read-modify-write for the seconds part and a sub-second mechanism
(phase step or TIE write) for the remainder. On partial failure
where seconds were already committed, success is returned to
prevent the PTP servo from retrying and applying seconds again.

All PTP callbacks are serialized by the existing per-DPLL zldpll->lock
mutex, which is also used by DPLL pin and device callbacks.

Reviewed-by: Petr Oros <poros@redhat.com>
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
Tested-by: Chris du Quesnay <Chris.duQuesnay@microchip.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/Kconfig |   6 +-
 drivers/dpll/zl3073x/dpll.c  | 439 +++++++++++++++++++++++++++++++++--
 drivers/dpll/zl3073x/dpll.h  |   5 +
 3 files changed, 422 insertions(+), 28 deletions(-)

diff --git a/drivers/dpll/zl3073x/Kconfig b/drivers/dpll/zl3073x/Kconfig
index 5bbca140058131..da1731ace15f02 100644
--- a/drivers/dpll/zl3073x/Kconfig
+++ b/drivers/dpll/zl3073x/Kconfig
@@ -2,7 +2,7 @@
 
 config ZL3073X
 	tristate "Microchip Azurite DPLL/PTP/SyncE devices" if COMPILE_TEST
-	depends on NET
+	depends on NET && PTP_1588_CLOCK
 	select DPLL
 	select NET_DEVLINK
 	select REGMAP
@@ -16,7 +16,7 @@ config ZL3073X
 
 config ZL3073X_I2C
 	tristate "I2C bus implementation for Microchip Azurite devices"
-	depends on I2C && NET
+	depends on I2C && NET && PTP_1588_CLOCK
 	select REGMAP_I2C
 	select ZL3073X
 	help
@@ -28,7 +28,7 @@ config ZL3073X_I2C
 
 config ZL3073X_SPI
 	tristate "SPI bus implementation for Microchip Azurite devices"
-	depends on NET && SPI
+	depends on NET && SPI && PTP_1588_CLOCK
 	select REGMAP_SPI
 	select ZL3073X
 	help
diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
index 83bd3027dbaa1e..f2e137475b40ff 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -2,6 +2,7 @@
 
 #include <linux/bits.h>
 #include <linux/bitfield.h>
+#include <linux/cleanup.h>
 #include <linux/bug.h>
 #include <linux/container_of.h>
 #include <linux/dev_printk.h>
@@ -13,6 +14,7 @@
 #include <linux/netlink.h>
 #include <linux/platform_device.h>
 #include <linux/property.h>
+#include <linux/ptp_clock_kernel.h>
 #include <linux/slab.h>
 #include <linux/sprintf.h>
 
@@ -2313,45 +2315,384 @@ zl3073x_dpll_init_fine_phase_adjust(struct zl3073x_dev *zldev)
 	return zl3073x_write_u8(zldev, ZL_REG_SYNTH_PHASE_SHIFT_CTRL, 0x01);
 }
 
+/* Maximum frequency adjustment: +-1% of nominal in ppb */
+#define ZL3073X_DPLL_PTP_MAX_ADJ	10000000
+
 /**
- * zl3073x_dpll_alloc - allocate DPLL device
- * @zldev: pointer to zl3073x device
- * @ch: DPLL channel number
+ * zl3073x_dpll_ptp_gettimex64 - read current time from ToD counters
+ * @info: PTP clock info
+ * @ts: timespec to store current time
+ * @sts: optional system timestamp pair for cross-timestamping
  *
- * Allocates DPLL device structure for given DPLL channel.
+ * Return: 0 on success, <0 on error
+ */
+static int zl3073x_dpll_ptp_gettimex64(struct ptp_clock_info *info,
+				       struct timespec64 *ts,
+				       struct ptp_system_timestamp *sts)
+{
+	struct zl3073x_dpll *zldpll = container_of(info, struct zl3073x_dpll,
+						   ptp_info);
+
+	guard(mutex)(&zldpll->lock);
+
+	return zl3073x_chan_tod_read(zldpll->dev, zldpll->id, false, ts, sts);
+}
+
+/**
+ * zl3073x_dpll_ptp_settime64 - set ToD counters to given time
+ * @info: PTP clock info
+ * @ts: timespec with time to set
  *
- * Return: pointer to DPLL device on success, error pointer on error
+ * Return: 0 on success, <0 on error
  */
-struct zl3073x_dpll *
-zl3073x_dpll_alloc(struct zl3073x_dev *zldev, u8 ch)
+static int zl3073x_dpll_ptp_settime64(struct ptp_clock_info *info,
+				      const struct timespec64 *ts)
 {
-	struct zl3073x_dpll *zldpll;
+	struct zl3073x_dpll *zldpll = container_of(info, struct zl3073x_dpll,
+						   ptp_info);
 
-	zldpll = kzalloc_obj(*zldpll);
-	if (!zldpll)
-		return ERR_PTR(-ENOMEM);
+	guard(mutex)(&zldpll->lock);
 
-	zldpll->dev = zldev;
-	zldpll->id = ch;
-	mutex_init(&zldpll->lock);
-	INIT_LIST_HEAD(&zldpll->pins);
+	return zl3073x_chan_tod_write(zldpll->dev, zldpll->id, *ts);
+}
 
-	return zldpll;
+/**
+ * zl3073x_dpll_ptp_adjtime_phase_step - adjust sub-second time via phase step
+ * @zldpll: DPLL channel
+ * @delta: time adjustment in nanoseconds (must be within (-NSEC_PER_SEC,
+ *         NSEC_PER_SEC))
+ *
+ * Uses the output phase step mechanism with tod_step=1 to adjust both
+ * the output clock phase and the ToD counter simultaneously. This keeps
+ * outputs and ToD coherent. Only valid for NCO.
+ *
+ * Outputs are grouped by synthesizer since the phase step value is in
+ * synthesizer clock cycles. The first synth group with enabled outputs
+ * uses tod_step to adjust both outputs and the ToD counter. Remaining
+ * groups step outputs only. If no synth has enabled outputs, the ToD
+ * counter is stepped alone using an empty output mask (the FW uses
+ * the first enabled synth's period for the conversion).
+ *
+ * Return:
+ * * %0			- success (or partial success if a later synth group
+ *			  failed after the first was already stepped)
+ * * %-EOPNOTSUPP	- no synths available
+ * * negative		- error
+ */
+static int zl3073x_dpll_ptp_adjtime_phase_step(struct zl3073x_dpll *zldpll,
+					       s64 delta)
+{
+	u16 synth_mask[ZL3073X_NUM_SYNTHS] = {};
+	struct zl3073x_dev *zldev = zldpll->dev;
+	const struct zl3073x_synth *synth;
+	struct zl3073x_dpll_pin *pin;
+	u32 first_synth_freq = 0;
+	bool tod_stepped = false;
+	s32 step_cycles;
+	u32 synth_freq;
+	int rc;
+	u8 i;
+
+	/* Build per-synth output masks from registered output pins */
+	list_for_each_entry(pin, &zldpll->pins, list) {
+		u8 out_id, synth_id;
+
+		if (zl3073x_dpll_is_input_pin(pin))
+			continue;
+
+		out_id = zl3073x_output_pin_out_get(pin->id);
+
+		if (!zl3073x_dev_out_is_stepped(zldev, out_id))
+			continue;
+
+		synth_id = zl3073x_dev_out_synth_get(zldev, out_id);
+		if (synth_id >= ZL3073X_NUM_SYNTHS) {
+			dev_warn(zldev->dev, "Unexpected synth id for OUT%u\n",
+				 out_id);
+			continue;
+		}
+		synth_mask[synth_id] |= BIT(out_id);
+	}
+
+	/* Process each synth group */
+	for (i = 0; i < ZL3073X_NUM_SYNTHS; i++) {
+		synth = zl3073x_synth_state_get(zldev, i);
+		if (!zl3073x_synth_is_enabled(synth) ||
+		    zl3073x_synth_dpll_get(synth) != zldpll->id)
+			continue;
+
+		synth_freq = zl3073x_synth_freq_get(synth);
+
+		/* Remember first enabled synth freq for ToD-only fallback */
+		if (!first_synth_freq)
+			first_synth_freq = synth_freq;
+
+		if (!synth_mask[i])
+			continue;
+
+		/* Safe for s32: max synth freq is 750 MHz */
+		step_cycles = div_s64(delta * synth_freq, NSEC_PER_SEC);
+
+		rc = zl3073x_chan_phase_step(zldev, zldpll->id,
+					     synth_mask[i], step_cycles,
+					     !tod_stepped);
+		if (rc) {
+			if (tod_stepped) {
+				dev_warn(zldev->dev,
+					 "Partial phase step failure\n");
+				return 0;
+			}
+			return rc;
+		}
+		tod_stepped = true;
+	}
+
+	if (!first_synth_freq)
+		return -EOPNOTSUPP;
+
+	/* No enabled outputs found; step ToD counter only using the
+	 * first enabled synth's period (empty output mask).
+	 */
+	if (!tod_stepped) {
+		step_cycles = div_s64(delta * first_synth_freq, NSEC_PER_SEC);
+		return zl3073x_chan_phase_step(zldev, zldpll->id, 0,
+					       step_cycles, true);
+	}
+
+	return 0;
 }
 
 /**
- * zl3073x_dpll_free - free DPLL device
- * @zldpll: pointer to zl3073x_dpll structure
+ * zl3073x_dpll_ptp_adjtime - adjust PTP clock time
+ * @info: PTP clock info
+ * @delta: time adjustment in nanoseconds
  *
- * Deallocates given DPLL device previously allocated by @zl3073x_dpll_alloc.
+ * For NCO, large deltas (>= 1 second) are split into a ToD
+ * read-modify-write for the seconds part and an output phase step for
+ * the sub-second remainder. Sub-second deltas use phase step directly,
+ * falling back to ToD read-modify-write if phase step or TIE write
+ * fails. In AUTO/REFLOCK modes, large deltas are split into ToD
+ * read-modify-write for seconds and TIE write for the sub-second
+ * remainder. Sub-second deltas use TIE write directly.
+ *
+ * If the seconds part was already committed when the sub-second
+ * mechanism fails, returns 0 to prevent the PTP servo from retrying
+ * the full delta and applying seconds again.
+ *
+ * Return: 0 on success (or partial success), <0 on error
  */
-void
-zl3073x_dpll_free(struct zl3073x_dpll *zldpll)
+static int zl3073x_dpll_ptp_adjtime(struct ptp_clock_info *info, s64 delta)
 {
-	WARN(zldpll->dpll_dev, "DPLL device is still registered\n");
+	struct zl3073x_dpll *zldpll = container_of(info, struct zl3073x_dpll,
+						   ptp_info);
+	struct zl3073x_dev *zldev = zldpll->dev;
+	const struct zl3073x_chan *chan;
+	bool sec_adjusted = false;
+	struct timespec64 ts;
+	int rc;
 
-	mutex_destroy(&zldpll->lock);
-	kfree(zldpll);
+	if (!delta)
+		return 0;
+
+	guard(mutex)(&zldpll->lock);
+
+	/* Modes without phase step or TIE use plain ToD adjust */
+	chan = zl3073x_chan_state_get(zldev, zldpll->id);
+	if (!zl3073x_chan_mode_is_nco(chan) &&
+	    !zl3073x_chan_mode_supports_tie(chan))
+		return zl3073x_chan_tod_adjust(zldev, zldpll->id,
+					       ns_to_timespec64(delta));
+
+	/* Split off seconds via ToD read-modify-write so the sub-second
+	 * remainder can be applied through the output-coherent mechanism
+	 * (phase step or TIE write).
+	 */
+	if (delta >= NSEC_PER_SEC || delta <= -NSEC_PER_SEC) {
+		s32 remainder;
+
+		ts.tv_sec = div_s64_rem(delta, NSEC_PER_SEC, &remainder);
+		ts.tv_nsec = 0;
+		delta = remainder;
+
+		rc = zl3073x_chan_tod_adjust(zldev, zldpll->id, ts);
+		if (rc)
+			return rc;
+
+		/* No sub-second remainder, done */
+		if (!delta)
+			return 0;
+
+		/* Wait for the ToD write to be applied at the 1 Hz edge
+		 * before issuing phase step or TIE write, so the pending
+		 * WR_NEXT_1HZ does not overwrite the sub-second adjustment.
+		 */
+		rc = zl3073x_chan_tod_ready_wait(zldev, zldpll->id);
+		if (rc)
+			return rc;
+
+		sec_adjusted = true;
+	}
+
+	/* Apply sub-second delta via phase step (NCO) or TIE write */
+	if (zl3073x_chan_mode_is_nco(chan)) {
+		rc = zl3073x_dpll_ptp_adjtime_phase_step(zldpll, delta);
+		if (!rc)
+			return 0;
+	} else {
+		rc = zl3073x_chan_tie_write(zldev, zldpll->id, delta);
+		if (!rc)
+			return 0;
+	}
+
+	/* Phase step or TIE write failed, fall back to ToD adjust */
+	rc = zl3073x_chan_tod_adjust(zldev, zldpll->id,
+				     ns_to_timespec64(delta));
+
+	/* In the unlikely event that both phase step/TIE write and fallback
+	 * ToD adjust fail after seconds were already committed, return
+	 * success to prevent the PTP servo from retrying the full delta and
+	 * applying seconds again. The sub-second residual will self-correct
+	 * in the next servo cycle.
+	 */
+	if (rc && sec_adjusted) {
+		dev_warn(zldev->dev,
+			 "Sub-second adjustment failed after seconds applied\n");
+		return 0;
+	}
+
+	return rc;
+}
+
+/**
+ * zl3073x_dpll_ptp_adjfine - adjust PTP clock frequency
+ * @info: PTP clock info
+ * @scaled_ppm: frequency adjustment in scaled ppm (ppm * 2^16)
+ *
+ * Only supported for NCO. Writes the delta frequency offset register.
+ *
+ * Return:
+ * * %0			- success or @scaled_ppm is zero (no-op)
+ * * %-EOPNOTSUPP	- NCO pin is not connected and @scaled_ppm is non-zero
+ * * negative		- other error
+ */
+static int
+zl3073x_dpll_ptp_adjfine(struct ptp_clock_info *info, long scaled_ppm)
+{
+	struct zl3073x_dpll *zldpll = container_of(info, struct zl3073x_dpll,
+						   ptp_info);
+	const struct zl3073x_chan *chan;
+	s64 offset;
+
+	/* Convert scaled_ppm to df_offset in 2^-48 steps:
+	 *   df_offset = -(scaled_ppm * 2^32) / 10^6
+	 *
+	 * Simplify to avoid overflow:
+	 *   df_offset = -(scaled_ppm * 2^26) / 5^6
+	 *   df_offset = -(scaled_ppm * 67108864) / 15625
+	 */
+	offset = -div_s64((s64)scaled_ppm * 67108864LL, 15625);
+
+	guard(mutex)(&zldpll->lock);
+
+	chan = zl3073x_chan_state_get(zldpll->dev, zldpll->id);
+	if (!zl3073x_chan_mode_is_nco(chan))
+		return scaled_ppm ? -EOPNOTSUPP : 0;
+	if (offset == chan->df_offset)
+		return 0;
+
+	return zl3073x_chan_df_offset_set(zldpll->dev, zldpll->id, offset);
+}
+
+/**
+ * zl3073x_dpll_ptp_adjphase - adjust PTP clock phase
+ * @info: PTP clock info
+ * @delta: phase adjustment in nanoseconds
+ *
+ * Only supported in AUTO and REFLOCK modes. Uses TIE write for
+ * nanosecond resolution phase adjustment.
+ *
+ * Return:
+ * * %0			- success or @delta is zero (no-op)
+ * * %-EOPNOTSUPP	- mode does not support TIE and @delta is non-zero
+ * * negative		- other error
+ */
+static int zl3073x_dpll_ptp_adjphase(struct ptp_clock_info *info, s32 delta)
+{
+	struct zl3073x_dpll *zldpll = container_of(info, struct zl3073x_dpll,
+						   ptp_info);
+	struct zl3073x_dev *zldev = zldpll->dev;
+	const struct zl3073x_chan *chan;
+
+	if (!delta)
+		return 0;
+
+	guard(mutex)(&zldpll->lock);
+
+	chan = zl3073x_chan_state_get(zldev, zldpll->id);
+
+	if (!zl3073x_chan_mode_supports_tie(chan))
+		return -EOPNOTSUPP;
+
+	return zl3073x_chan_tie_write(zldev, zldpll->id, delta);
+}
+
+static s32
+zl3073x_dpll_ptp_getmaxphase(struct ptp_clock_info *info __always_unused)
+{
+	/* HW limits TIE write to +-1 second. Return the constant HW
+	 * limit and let adjphase handle mode-specific checks.
+	 */
+	return NSEC_PER_SEC - 1;
+}
+
+static const struct ptp_clock_info zl3073x_dpll_ptp_clock_info = {
+	.owner		= THIS_MODULE,
+	.max_adj	= ZL3073X_DPLL_PTP_MAX_ADJ,
+	.gettimex64	= zl3073x_dpll_ptp_gettimex64,
+	.settime64	= zl3073x_dpll_ptp_settime64,
+	.adjtime	= zl3073x_dpll_ptp_adjtime,
+	.adjfine	= zl3073x_dpll_ptp_adjfine,
+	.adjphase	= zl3073x_dpll_ptp_adjphase,
+	.getmaxphase	= zl3073x_dpll_ptp_getmaxphase,
+};
+
+/**
+ * zl3073x_dpll_ptp_register - register PTP clock for a DPLL channel
+ * @zldpll: DPLL channel to register PTP clock for
+ *
+ * Return: 0 on success, <0 on error
+ */
+static int zl3073x_dpll_ptp_register(struct zl3073x_dpll *zldpll)
+{
+	struct zl3073x_dev *zldev = zldpll->dev;
+	struct ptp_clock *ptp_clock;
+
+	zldpll->ptp_info = zl3073x_dpll_ptp_clock_info;
+	snprintf(zldpll->ptp_info.name, sizeof(zldpll->ptp_info.name),
+		 "%s-dpll%u", dev_name(zldev->dev), zldpll->id);
+
+	ptp_clock = ptp_clock_register(&zldpll->ptp_info, zldev->dev);
+	if (IS_ERR(ptp_clock)) {
+		dev_err(zldev->dev, "Failed to register PTP clock for DPLL%u\n",
+			zldpll->id);
+		return PTR_ERR(ptp_clock);
+	}
+
+	zldpll->ptp_clock = ptp_clock;
+
+	return 0;
+}
+
+/**
+ * zl3073x_dpll_ptp_unregister - unregister PTP clock for a DPLL channel
+ * @zldpll: DPLL channel to unregister PTP clock for
+ */
+static void zl3073x_dpll_ptp_unregister(struct zl3073x_dpll *zldpll)
+{
+	if (!IS_ERR_OR_NULL(zldpll->ptp_clock)) {
+		ptp_clock_unregister(zldpll->ptp_clock);
+		zldpll->ptp_clock = NULL;
+	}
 }
 
 /**
@@ -2436,6 +2777,47 @@ zl3073x_dpll_ref_sync_pairs_register(struct zl3073x_dpll *zldpll)
 	return 0;
 }
 
+/**
+ * zl3073x_dpll_alloc - allocate DPLL device
+ * @zldev: pointer to zl3073x device
+ * @ch: DPLL channel number
+ *
+ * Allocates DPLL device structure for given DPLL channel.
+ *
+ * Return: pointer to DPLL device on success, error pointer on error
+ */
+struct zl3073x_dpll *
+zl3073x_dpll_alloc(struct zl3073x_dev *zldev, u8 ch)
+{
+	struct zl3073x_dpll *zldpll;
+
+	zldpll = kzalloc_obj(*zldpll);
+	if (!zldpll)
+		return ERR_PTR(-ENOMEM);
+
+	zldpll->dev = zldev;
+	zldpll->id = ch;
+	mutex_init(&zldpll->lock);
+	INIT_LIST_HEAD(&zldpll->pins);
+
+	return zldpll;
+}
+
+/**
+ * zl3073x_dpll_free - free DPLL device
+ * @zldpll: pointer to zl3073x_dpll structure
+ *
+ * Deallocates given DPLL device previously allocated by @zl3073x_dpll_alloc.
+ */
+void
+zl3073x_dpll_free(struct zl3073x_dpll *zldpll)
+{
+	WARN(zldpll->dpll_dev, "DPLL device is still registered\n");
+
+	mutex_destroy(&zldpll->lock);
+	kfree(zldpll);
+}
+
 /**
  * zl3073x_dpll_register - register DPLL device and all its pins
  * @zldpll: pointer to zl3073x_dpll structure
@@ -2466,6 +2848,13 @@ zl3073x_dpll_register(struct zl3073x_dpll *zldpll)
 		return rc;
 	}
 
+	rc = zl3073x_dpll_ptp_register(zldpll);
+	if (rc) {
+		zl3073x_dpll_pins_unregister(zldpll);
+		zl3073x_dpll_device_unregister(zldpll);
+		return rc;
+	}
+
 	return 0;
 }
 
@@ -2479,7 +2868,7 @@ zl3073x_dpll_register(struct zl3073x_dpll *zldpll)
 void
 zl3073x_dpll_unregister(struct zl3073x_dpll *zldpll)
 {
-	/* Unregister all pins and dpll */
+	zl3073x_dpll_ptp_unregister(zldpll);
 	zl3073x_dpll_pins_unregister(zldpll);
 	zl3073x_dpll_device_unregister(zldpll);
 }
diff --git a/drivers/dpll/zl3073x/dpll.h b/drivers/dpll/zl3073x/dpll.h
index faebc402ba1b70..993221dc63249d 100644
--- a/drivers/dpll/zl3073x/dpll.h
+++ b/drivers/dpll/zl3073x/dpll.h
@@ -5,6 +5,7 @@
 
 #include <linux/dpll.h>
 #include <linux/list.h>
+#include <linux/ptp_clock_kernel.h>
 
 #include "core.h"
 
@@ -22,6 +23,8 @@
  * @type: DPLL type (PPS or EEC)
  * @lock_status: last saved DPLL lock status
  * @pins: list of pins
+ * @ptp_info: PTP clock info
+ * @ptp_clock: registered PTP clock (or NULL)
  */
 struct zl3073x_dpll {
 	struct list_head		list;
@@ -36,6 +39,8 @@ struct zl3073x_dpll {
 	enum dpll_type			type;
 	enum dpll_lock_status		lock_status;
 	struct list_head		pins;
+	struct ptp_clock_info		ptp_info;
+	struct ptp_clock		*ptp_clock;
 };
 
 struct zl3073x_dpll *zl3073x_dpll_alloc(struct zl3073x_dev *zldev, u8 ch);
-- 
2.54.0


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

* Re: [PATCH net-next v8 2/3] dpll: zl3073x: add channel ToD, phase step and TIE operations
  2026-08-14  8:26 ` [PATCH net-next v8 2/3] dpll: zl3073x: add channel ToD, phase step and TIE operations Ivan Vecera
@ 2026-08-16 14:47   ` Ivan Vecera
  2026-08-18 16:37   ` Jakub Kicinski
  1 sibling, 0 replies; 10+ messages in thread
From: Ivan Vecera @ 2026-08-16 14:47 UTC (permalink / raw)
  To: netdev
  Cc: Petr Oros, Chris du Quesnay, Arkadiusz Kubalewski, Jakub Kicinski,
	Jiri Pirko, Min Li, Paolo Abeni, Richard Cochran, Vadim Fedorenko,
	linux-kernel

Sashiko comments with my replies:

 > Can this loop spin out its retry budget without ever giving the
 > condition a chance to become true? [...]
 > On a fast bus where the ToD command completes before the semaphore
 > is re-read, all 20 iterations can finish well inside the 20 ms window

Testing on I2C at both 100 kHz and 400 kHz shows that a single
iteration takes approximately 17-19 ms regardless of bus speed.
The iteration time is dominated by the device's internal processing
of the ToD read commands, not bus transfer time. With 20 retries
the budget is 340-380 ms, well beyond the 20 ms margin window.

 > the kerneldoc for zl3073x_chan_tie_write() states delta_ns "must be
 > in (-1s, 1s)" and nothing in the function enforces it. [...]
 > Would an explicit range check on delta_ns be worth adding here?

Both in-tree callers already bound the input as you noted. The range
guard was intentionally removed in this version at the reviewer's
request — the internal API should trust its callers rather than
re-validate inputs already checked by the PTP core. The kernel-doc
documents the precondition for future callers.

Thanks,
Ivan


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

* Re: [PATCH net-next v8 3/3] dpll: zl3073x: add PTP clock support
  2026-08-14  8:26 ` [PATCH net-next v8 3/3] dpll: zl3073x: add PTP clock support Ivan Vecera
@ 2026-08-16 14:52   ` Ivan Vecera
  2026-08-18 16:37   ` Jakub Kicinski
  1 sibling, 0 replies; 10+ messages in thread
From: Ivan Vecera @ 2026-08-16 14:52 UTC (permalink / raw)
  To: netdev
  Cc: Petr Oros, Vadim Fedorenko, Chris du Quesnay,
	Arkadiusz Kubalewski, Jakub Kicinski, Jiri Pirko, Min Li,
	Paolo Abeni, Richard Cochran, linux-kernel

Sashiko comments with my replies:

 > Should PTP_1588_CLOCK be a hard dependency here?

Yes. This was explicitly requested by Jakub in the v2 review [1].

[1] https://lore.kernel.org/netdev/20260722135848.2d401ada@kernel.org/

 > Does settime64() need to compensate for the deferred ToD load?
 > [...] does this leave the clock up to a second behind the requested
 > time on every clock_settime()?

settime64 is a coarse "slam the clock" operation. gettimex64() does
not return stale values — it calls tod_ready_wait() which blocks
until the pending WR_NEXT_1HZ completes. The PTP servo handles any
residual offset in subsequent adjtime calls.

 > Also, ts.tv_nsec is written raw. If a caller passes a non-zero
 > tv_nsec, does the ToD second boundary end up displaced from the
 > DPLL 1 Hz / output 1PPS phase?

Yes, a non-zero tv_nsec displaces the ToD second boundary from the
1PPS edge. This matches the hardware behavior — settime64 sets
the ToD counter to whatever value is requested. Phase coherence
between ToD and outputs is maintained by adjtime which routes
sub-second adjustments through phase step or TIE write.

 > Is this error return reachable after the seconds are already
 > committed? [...] Does the caller then get a failure for an
 > adjustment whose seconds part is already applied?

If tod_ready_wait fails, we cannot be certain that WR_NEXT_1HZ was
actually applied. Returning the error is the safer choice — masking
it would silently report success when the adjustment may not have
been applied at all.

 > Does the same pending WR_NEXT_1HZ hazard exist across callback
 > invocations? [...]
 > Would calling zl3073x_chan_tod_ready_wait() unconditionally before
 > the phase step / TIE write cover this?

Phase step and TIE write operate on independent hardware blocks
(output phase adjustment and DPLL input phase respectively) and do
not conflict with a pending ToD write. Any subsequent ToD operation
goes through tod_ready_wait which waits for the pending write to
complete. The cross-callback sequence described (settime64 followed
immediately by adjphase) does not occur in practice — these are
fundamentally different operations used at different stages of PTP
clock management.

 > zl3073x_dpll_alloc() and zl3073x_dpll_free() are moved verbatim
 > [...] Could it be dropped or split into a separate
 > no-functional-change patch?

The move is needed because the new PTP registration helper
zl3073x_dpll_ptp_register() must be defined before
zl3073x_dpll_alloc() which references ptp_info. Splitting this
into a separate patch was done in v3 at Jakub's request but was
squashed back when the refactored helper was dropped along with
perout support.

Thanks,
Ivan


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

* Re: [PATCH net-next v8 2/3] dpll: zl3073x: add channel ToD, phase step and TIE operations
  2026-08-14  8:26 ` [PATCH net-next v8 2/3] dpll: zl3073x: add channel ToD, phase step and TIE operations Ivan Vecera
  2026-08-16 14:47   ` Ivan Vecera
@ 2026-08-18 16:37   ` Jakub Kicinski
  2026-08-18 16:51     ` Jakub Kicinski
  1 sibling, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2026-08-18 16:37 UTC (permalink / raw)
  To: ivecera
  Cc: Jakub Kicinski, netdev, poros, vadim.fedorenko, Chris.duQuesnay,
	arkadiusz.kubalewski, jiri, min.li, pabeni, richardcochran,
	linux-kernel

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
dpll: zl3073x: add channel ToD, phase step and TIE operations

Adds low-level DPLL channel helpers for ToD read/write/adjust, output phase
step, delta frequency offset write and TIE write, plus a device-global
output step-time mask.  These are used by the PTP clock callbacks added in
the following patch.

> diff --git a/drivers/dpll/zl3073x/chan.c b/drivers/dpll/zl3073x/chan.c
> index 4ec2cf53dad46..ba4d303d41b47 100644
> --- a/drivers/dpll/zl3073x/chan.c
> +++ b/drivers/dpll/zl3073x/chan.c

[ ... ]

> @@ -234,6 +235,322 @@ const struct zl3073x_chan *zl3073x_chan_state_get(struct zl3073x_dev *zldev,
>  	return &zldev->chan[index];
>  }
>  
> +/**
> + * zl3073x_chan_tod_ready_wait - wait for ToD semaphore to clear
> + * @zldev: pointer to zl3073x device
> + * @ch: DPLL channel index
> + *
> + * Checks the ToD control register semaphore bit. If clear, returns
> + * immediately. Otherwise polls until the bit is cleared by the device.
> + *
> + * Return:
> + * * 0			- success
> + * * %-EBUSY		- timeout
> + * * %-EOPNOTSUPP	- unknown command detected
> + * * negative		- other error
> + */
> +int zl3073x_chan_tod_ready_wait(struct zl3073x_dev *zldev, u8 ch)
> +{
> +	unsigned int timeout;
> +	u8 tod_ctrl;
> +	int rc;
> +
> +	rc = zl3073x_read_u8(zldev, ZL_REG_DPLL_TOD_CTRL(ch), &tod_ctrl);
> +	if (rc)
> +		return rc;
> +
> +	if (!(tod_ctrl & ZL_DPLL_TOD_CTRL_SEM))
> +		return 0;

[ ... ]

> +int zl3073x_chan_tod_adjust(struct zl3073x_dev *zldev, u8 ch,
> +			    struct timespec64 delta)
> +{
> +#define ZL_TOD_MAX_RETRIES	20
> +	static const long threshold_ns = 20 * NSEC_PER_MSEC;
> +	struct timespec64 ts_next, ts_cur, diff;
> +	int rc, i;
> +
> +	for (i = 0; i < ZL_TOD_MAX_RETRIES; i++) {
> +		rc = zl3073x_chan_tod_read(zldev, ch, true, &ts_next, NULL);
> +		if (rc)
> +			return rc;
> +
> +		rc = zl3073x_chan_tod_read(zldev, ch, false, &ts_cur, NULL);
> +		if (rc)
> +			return rc;
> +
> +		/* Ensure the 1 Hz tick did not cross between the two reads
> +		 * and that enough margin remains to complete the write.
> +		 */
> +		diff = timespec64_sub(ts_next, ts_cur);
> +		if (diff.tv_sec > 0 ||
> +		    (!diff.tv_sec && diff.tv_nsec >= threshold_ns))
> +			break;
> +	}
> +	if (i == ZL_TOD_MAX_RETRIES) {
> +		dev_warn(zldev->dev,
> +			 "DPLL%u ToD adjust failed to get stable margin\n",
> +			 ch);
> +		return -EBUSY;
> +	}

[Severity: Medium]

Can this loop spin out its retry budget without ever giving the condition a
chance to become true?

The value being tested, diff = ts_next - ts_cur, is the time remaining until
the next 1 Hz edge.  When the call lands in the last 20 ms of a second, the
loop can only succeed once that residual has elapsed, but nothing in the
loop body waits.  zl3073x_chan_tod_ready_wait() returns straight away when
the semaphore is already clear:

	if (!(tod_ctrl & ZL_DPLL_TOD_CTRL_SEM))
		return 0;

and regmap_read_poll_timeout() inside zl3073x_poll_zero_u8() does its first
read before any usleep_range().  On a fast bus where the ToD command
completes before the semaphore is re-read, all 20 iterations can finish well
inside the 20 ms window, so i == ZL_TOD_MAX_RETRIES and the adjustment is
rejected with -EBUSY plus an unratelimited dev_warn() rather than being
retried a few milliseconds later.

In the next patch this propagates directly to userspace:

drivers/dpll/zl3073x/dpll.c:zl3073x_dpll_ptp_adjtime() {
	...
	return zl3073x_chan_tod_adjust(zldev, zldpll->id,
				       ns_to_timespec64(delta));
}

so clock_adjtime(ADJ_SETOFFSET) sees the -EBUSY.

Would it make sense to sleep for the remaining margin (or a fixed few
milliseconds) between iterations, so the loop actually waits out the
rollover window instead of burning the retries inside it?

[ ... ]

> +int zl3073x_chan_tie_write(struct zl3073x_dev *zldev, u8 ch, s64 delta_ns)
> +{
> +	s64 tie_data;
> +	int rc;
> +
> +	guard(mutex)(&zldev->tie_lock);
> +
> +	/* Wait for any previous TIE operation to complete */
> +	rc = zl3073x_poll_zero_u8(zldev, ZL_REG_DPLL_TIE_CTRL,
> +				  ZL_DPLL_TIE_CTRL_OP,
> +				  ZL_POLL_TIE_WR_TIMEOUT_US);
> +	if (rc)
> +		return rc;
> +
> +	/* Convert ns to TIE units (0.01 ps = 10^-14 s) */
> +	tie_data = delta_ns * 100000LL;
> +
> +	rc = zl3073x_write_u48(zldev, ZL_REG_DPLL_TIE_DATA(ch), tie_data);
> +	if (rc)
> +		return rc;

[Severity: Low]

This isn't a bug with the current callers, but the kerneldoc for
zl3073x_chan_tie_write() states delta_ns "must be in (-1s, 1s)" and nothing
in the function enforces it.  For |delta_ns| >= about 1.4e9 the product
overflows the signed 48-bit TIE field, and zl3073x_write_u48() accepts the
union of the signed and unsigned 48-bit ranges:

drivers/dpll/zl3073x/core.c:zl3073x_write_u48() {
	if (val > GENMASK_ULL(47, 0) && val < GENMASK_ULL(63, 47)) {
		dev_err(zldev->dev, "Value 0x%0llx out of range\n", val);
		return -EINVAL;
	}
	...
}

so a positive tie_data in [2^47, 2^48) would be programmed with the sign bit
set.

Both in-tree callers do bound the input: zl3073x_dpll_ptp_adjphase() is
gated by ptp_clock_adjtime(), which rejects offsets beyond
getmaxphase() == NSEC_PER_SEC - 1, and zl3073x_dpll_ptp_adjtime() passes
only the sub-second remainder from div_s64_rem(delta, NSEC_PER_SEC, ...).

Would an explicit range check on delta_ns be worth adding here so the
documented precondition is enforced by the helper itself?
-- 
pw-bot: cr

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

* Re: [PATCH net-next v8 3/3] dpll: zl3073x: add PTP clock support
  2026-08-14  8:26 ` [PATCH net-next v8 3/3] dpll: zl3073x: add PTP clock support Ivan Vecera
  2026-08-16 14:52   ` Ivan Vecera
@ 2026-08-18 16:37   ` Jakub Kicinski
  1 sibling, 0 replies; 10+ messages in thread
From: Jakub Kicinski @ 2026-08-18 16:37 UTC (permalink / raw)
  To: ivecera
  Cc: Jakub Kicinski, netdev, poros, vadim.fedorenko, Chris.duQuesnay,
	arkadiusz.kubalewski, jiri, min.li, pabeni, richardcochran,
	linux-kernel

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
dpll: zl3073x: add PTP clock support

This registers a PTP clock device per DPLL channel and implements
gettimex64, settime64, adjtime, adjfine, adjphase and getmaxphase on top
of the existing ToD, TIE and output phase step helpers.  All callbacks are
serialized by the per-DPLL zldpll->lock mutex, and PTP_1588_CLOCK becomes
a hard dependency of the driver.

A few questions below.

> diff --git a/drivers/dpll/zl3073x/Kconfig b/drivers/dpll/zl3073x/Kconfig
> index 5bbca14005813..da1731ace15f0 100644
> --- a/drivers/dpll/zl3073x/Kconfig
> +++ b/drivers/dpll/zl3073x/Kconfig
> @@ -2,7 +2,7 @@
>  
>  config ZL3073X
>  	tristate "Microchip Azurite DPLL/PTP/SyncE devices" if COMPILE_TEST
> -	depends on NET
> +	depends on NET && PTP_1588_CLOCK
>  	select DPLL
>  	select NET_DEVLINK
>  	select REGMAP
> @@ -16,7 +16,7 @@ config ZL3073X
>  
>  config ZL3073X_I2C
>  	tristate "I2C bus implementation for Microchip Azurite devices"
> -	depends on I2C && NET
> +	depends on I2C && NET && PTP_1588_CLOCK
>  	select REGMAP_I2C
>  	select ZL3073X
>  	help
> @@ -28,7 +28,7 @@ config ZL3073X_I2C
>  
>  config ZL3073X_SPI
>  	tristate "SPI bus implementation for Microchip Azurite devices"
> -	depends on NET && SPI
> +	depends on NET && SPI && PTP_1588_CLOCK
>  	select REGMAP_SPI
>  	select ZL3073X

[Severity: Low]
Should PTP_1588_CLOCK be a hard dependency here?

ZL3073X is only user-visible with COMPILE_TEST and is otherwise selected by
ZL3073X_I2C/ZL3073X_SPI, so on a tree with CONFIG_PTP_1588_CLOCK=n the
ZL3073X_I2C and ZL3073X_SPI prompts disappear on oldconfig and the whole
driver is dropped, including the DPLL, devlink/flash and SyncE
functionality that does not need PTP.

The new code compiles against the !PTP stubs (only ptp_clock_register()
returns NULL, which zl3073x_dpll_ptp_register() already treats as
success), which is what PTP_1588_CLOCK_OPTIONAL in drivers/ptp/Kconfig is
for:

	config PTP_1588_CLOCK_OPTIONAL
		tristate
		default y if PTP_1588_CLOCK=n
		default PTP_1588_CLOCK

Would depends on PTP_1588_CLOCK_OPTIONAL work here instead?  If the hard
dependency is intended, could the changelog mention that PTP becomes
mandatory for existing users?

> diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
> index 83bd3027dbaa1..f2e137475b40f 100644
> --- a/drivers/dpll/zl3073x/dpll.c
> +++ b/drivers/dpll/zl3073x/dpll.c

[ ... ]

> @@ -2313,45 +2315,384 @@ zl3073x_dpll_init_fine_phase_adjust(struct zl3073x_dev *zldev)

[ ... ]

> +/**
> + * zl3073x_dpll_ptp_settime64 - set ToD counters to given time
> + * @info: PTP clock info
> + * @ts: timespec with time to set
>   *
> - * Return: pointer to DPLL device on success, error pointer on error
> + * Return: 0 on success, <0 on error
>   */
> -struct zl3073x_dpll *
> -zl3073x_dpll_alloc(struct zl3073x_dev *zldev, u8 ch)
> +static int zl3073x_dpll_ptp_settime64(struct ptp_clock_info *info,
> +				      const struct timespec64 *ts)
>  {
> -	struct zl3073x_dpll *zldpll;
> +	struct zl3073x_dpll *zldpll = container_of(info, struct zl3073x_dpll,
> +						   ptp_info);
>  
> -	zldpll = kzalloc_obj(*zldpll);
> -	if (!zldpll)
> -		return ERR_PTR(-ENOMEM);
> +	guard(mutex)(&zldpll->lock);
>  
> -	zldpll->dev = zldev;
> -	zldpll->id = ch;
> -	mutex_init(&zldpll->lock);
> -	INIT_LIST_HEAD(&zldpll->pins);
> +	return zl3073x_chan_tod_write(zldpll->dev, zldpll->id, *ts);
> +}

[Severity: High]
Does settime64() need to compensate for the deferred ToD load?

zl3073x_chan_tod_write() only ever issues the next-1Hz-edge command:

drivers/dpll/zl3073x/chan.c:zl3073x_chan_tod_write() {
	...
	return zl3073x_chan_tod_ctrl(zldev, ch,
				     ZL_DPLL_TOD_CTRL_CMD_WR_NEXT_1HZ);
}

so the value written becomes the counter value at the coming 1 Hz edge,
0..1 s after the register write.  zl3073x_chan_tod_ready_wait() budgets
ZL_POLL_TOD_WR_TIMEOUT_US (1000 ms) for exactly that pending write.

zl3073x_chan_tod_adjust() in the same driver compensates for this by
basing the write on the next-1Hz read:

drivers/dpll/zl3073x/chan.c:zl3073x_chan_tod_adjust() {
	...
	rc = zl3073x_chan_tod_read(zldev, ch, true, &ts_next, NULL);
	...
	/* Apply delta to the next-Hz ToD */
	ts_next = timespec64_add(ts_next, delta);
	...
	return zl3073x_chan_tod_write(zldev, ch, ts_next);
}

Here the caller's timespec is written verbatim, so after the edge the
counter reads ts while real time has advanced by the distance from the
call to that edge.  Does this leave the clock up to a second behind the
requested time on every clock_settime()?

Also, ts.tv_nsec is written raw.  If a caller passes a non-zero tv_nsec,
does the ToD second boundary end up displaced from the DPLL 1 Hz / output
1PPS phase, i.e. the coherence that adjtime() takes care to preserve by
routing sub-second deltas through phase step or TIE write?

> +/**
> + * zl3073x_dpll_ptp_adjtime_phase_step - adjust sub-second time via phase step
> + * @zldpll: DPLL channel
> + * @delta: time adjustment in nanoseconds (must be within (-NSEC_PER_SEC,
> + *         NSEC_PER_SEC))
> + *
> + * Uses the output phase step mechanism with tod_step=1 to adjust both
> + * the output clock phase and the ToD counter simultaneously. This keeps
> + * outputs and ToD coherent. Only valid for NCO.
> + *
> + * Outputs are grouped by synthesizer since the phase step value is in
> + * synthesizer clock cycles. The first synth group with enabled outputs
> + * uses tod_step to adjust both outputs and the ToD counter. Remaining
> + * groups step outputs only. If no synth has enabled outputs, the ToD
> + * counter is stepped alone using an empty output mask (the FW uses
> + * the first enabled synth's period for the conversion).

[Severity: Low]
This isn't a bug, but the kernel-doc of
zl3073x_dpll_ptp_adjtime_phase_step() speaks of "enabled outputs" while
the code filters on the device-global step-time mask instead:

	if (!zl3073x_dev_out_is_stepped(zldev, out_id))
		continue;

drivers/dpll/zl3073x/core.h:zl3073x_dev_out_is_stepped() {
	return !!(zldev->out_step_time_mask & BIT(index));
}

out_step_time_mask comes from ZL_REG_OUTPUT_STEP_TIME_MASK and says
nothing about whether an output is enabled, so an enabled, DPLL-owned
output that is absent from that mask is excluded from the phase step and
can also trigger the !tod_stepped ToD-only fallback.  Could the wording be
changed to refer to step-time-capable outputs?

> +	/* Process each synth group */
> +	for (i = 0; i < ZL3073X_NUM_SYNTHS; i++) {

[ ... ]

> +/**
> + * zl3073x_dpll_ptp_adjtime - adjust PTP clock time

[ ... ]

> +static int zl3073x_dpll_ptp_adjtime(struct ptp_clock_info *info, s64 delta)
>  {
> -	WARN(zldpll->dpll_dev, "DPLL device is still registered\n");
> +	struct zl3073x_dpll *zldpll = container_of(info, struct zl3073x_dpll,
> +						   ptp_info);
> +	struct zl3073x_dev *zldev = zldpll->dev;
> +	const struct zl3073x_chan *chan;
> +	bool sec_adjusted = false;
> +	struct timespec64 ts;
> +	int rc;
>  
> -	mutex_destroy(&zldpll->lock);
> -	kfree(zldpll);
> +	if (!delta)
> +		return 0;
> +
> +	guard(mutex)(&zldpll->lock);

[ ... ]

> +	if (delta >= NSEC_PER_SEC || delta <= -NSEC_PER_SEC) {
> +		s32 remainder;
> +
> +		ts.tv_sec = div_s64_rem(delta, NSEC_PER_SEC, &remainder);
> +		ts.tv_nsec = 0;
> +		delta = remainder;
> +
> +		rc = zl3073x_chan_tod_adjust(zldev, zldpll->id, ts);
> +		if (rc)
> +			return rc;
> +
> +		/* No sub-second remainder, done */
> +		if (!delta)
> +			return 0;
> +
> +		/* Wait for the ToD write to be applied at the 1 Hz edge
> +		 * before issuing phase step or TIE write, so the pending
> +		 * WR_NEXT_1HZ does not overwrite the sub-second adjustment.
> +		 */
> +		rc = zl3073x_chan_tod_ready_wait(zldev, zldpll->id);
> +		if (rc)
> +			return rc;
> +
> +		sec_adjusted = true;
> +	}

[Severity: Medium]
Is this error return reachable after the seconds are already committed?

zl3073x_chan_tod_adjust() ends in zl3073x_chan_tod_write(), which arms
ZL_DPLL_TOD_CTRL_CMD_WR_NEXT_1HZ, so once it returns 0 the whole-second
step will latch at the next 1 Hz edge and cannot be revoked.  The
following zl3073x_chan_tod_ready_wait() can then fail:

drivers/dpll/zl3073x/chan.c:zl3073x_chan_tod_ready_wait() {
	...
	case ZL_DPLL_TOD_CTRL_CMD_WR_NEXT_1HZ:
		timeout = ZL_POLL_TOD_WR_TIMEOUT_US;
	...
	return rc == -ETIMEDOUT ? -EBUSY : rc;
}

with ZL_POLL_TOD_WR_TIMEOUT_US == 1000 ms while zl3073x_chan_tod_adjust()
only guarantees a 20 ms margin before the edge, so the pending write can
need nearly the full second to retire.  A plain regmap read error gives
the same exit.

That return happens before sec_adjusted = true, so this is the one error
path that is not covered by the compensating logic further down:

	if (rc && sec_adjusted) {
		dev_warn(zldev->dev,
			 "Sub-second adjustment failed after seconds applied\n");
		return 0;
	}

Does the caller then get a failure for an adjustment whose seconds part is
already applied, with the sub-second remainder dropped?  If the servo or a
clock_adjtime(ADJ_SETOFFSET) user re-issues the same delta after the edge
has passed, is the whole second applied twice?

> +
> +	/* Apply sub-second delta via phase step (NCO) or TIE write */
> +	if (zl3073x_chan_mode_is_nco(chan)) {
> +		rc = zl3073x_dpll_ptp_adjtime_phase_step(zldpll, delta);
> +		if (!rc)
> +			return 0;
> +	} else {
> +		rc = zl3073x_chan_tie_write(zldev, zldpll->id, delta);
> +		if (!rc)
> +			return 0;
> +	}

[Severity: Medium]
Does the same pending WR_NEXT_1HZ hazard exist across callback
invocations?

The comment above acknowledges the hazard within one adjtime() call, but
the drain only runs when the seconds branch was taken.  For |delta| < 1 s
sec_adjusted stays false and zl3073x_chan_tod_ready_wait() is never
called, and zl3073x_dpll_ptp_adjphase() does not drain either:

	chan = zl3073x_chan_state_get(zldev, zldpll->id);

	if (!zl3073x_chan_mode_supports_tie(chan))
		return -EOPNOTSUPP;

	return zl3073x_chan_tie_write(zldev, zldpll->id, delta);

Meanwhile zl3073x_dpll_ptp_settime64(), and adjtime() on an exact
whole-second delta ("No sub-second remainder, done"), both drop
zldpll->lock while a WR_NEXT_1HZ is still armed, because
zl3073x_chan_tod_write() only waits for a previous operation on entry:

CPU0
zl3073x_dpll_ptp_settime64()
   zl3073x_chan_tod_write()
      zl3073x_chan_tod_ctrl(..., ZL_DPLL_TOD_CTRL_CMD_WR_NEXT_1HZ)
   /* mutex released, write still pending */

CPU0 (or another task), same 1 Hz interval
zl3073x_dpll_ptp_adjphase()          /* or sub-second adjtime() */
   zl3073x_chan_tie_write()          /* no ToD semaphore poll */
   /* 1 Hz edge: pending absolute ToD write lands, sub-second
      adjustment discarded, outputs already stepped */

The mutex serializes register accesses but not the retirement of the
asynchronous device command, so does 0 get returned for an adjustment that
was silently dropped, leaving ToD and outputs incoherent for the NCO phase
step case?  Would calling zl3073x_chan_tod_ready_wait() unconditionally
before the phase step / TIE write cover this?

> +
> +	/* Phase step or TIE write failed, fall back to ToD adjust */
> +	rc = zl3073x_chan_tod_adjust(zldev, zldpll->id,
> +				     ns_to_timespec64(delta));

[ ... ]

> +/**
> + * zl3073x_dpll_ptp_adjfine - adjust PTP clock frequency
> + * @info: PTP clock info
> + * @scaled_ppm: frequency adjustment in scaled ppm (ppm * 2^16)
> + *
> + * Only supported for NCO. Writes the delta frequency offset register.
> + *
> + * Return:
> + * * %0			- success or @scaled_ppm is zero (no-op)
> + * * %-EOPNOTSUPP	- NCO pin is not connected and @scaled_ppm is non-zero
> + * * negative		- other error
> + */

[Severity: Low]
This isn't a bug, but does the documented "@scaled_ppm is zero (no-op)"
match the code?

> +	guard(mutex)(&zldpll->lock);
> +
> +	chan = zl3073x_chan_state_get(zldpll->dev, zldpll->id);
> +	if (!zl3073x_chan_mode_is_nco(chan))
> +		return scaled_ppm ? -EOPNOTSUPP : 0;
> +	if (offset == chan->df_offset)
> +		return 0;
> +
> +	return zl3073x_chan_df_offset_set(zldpll->dev, zldpll->id, offset);
> +}

With scaled_ppm == 0 on an NCO channel whose cached df_offset is non-zero,
offset becomes 0 and zl3073x_chan_df_offset_set() performs a 48-bit write
to ZL_REG_DPLL_DF_OFFSET, which changes the NCO frequency and can return a
negative errno.  Could the Return section be reworded to describe the
absolute setpoint semantics?

> @@ -2436,6 +2777,47 @@ zl3073x_dpll_ref_sync_pairs_register(struct zl3073x_dpll *zldpll)
>  	return 0;
>  }
>  
> +/**
> + * zl3073x_dpll_alloc - allocate DPLL device
> + * @zldev: pointer to zl3073x device
> + * @ch: DPLL channel number
> + *
> + * Allocates DPLL device structure for given DPLL channel.
> + *
> + * Return: pointer to DPLL device on success, error pointer on error
> + */
> +struct zl3073x_dpll *
> +zl3073x_dpll_alloc(struct zl3073x_dev *zldev, u8 ch)
> +{
> +	struct zl3073x_dpll *zldpll;
> +
> +	zldpll = kzalloc_obj(*zldpll);
> +	if (!zldpll)
> +		return ERR_PTR(-ENOMEM);
> +
> +	zldpll->dev = zldev;
> +	zldpll->id = ch;
> +	mutex_init(&zldpll->lock);
> +	INIT_LIST_HEAD(&zldpll->pins);
> +
> +	return zldpll;
> +}
> +
> +/**
> + * zl3073x_dpll_free - free DPLL device
> + * @zldpll: pointer to zl3073x_dpll structure
> + *
> + * Deallocates given DPLL device previously allocated by @zl3073x_dpll_alloc.
> + */
> +void
> +zl3073x_dpll_free(struct zl3073x_dpll *zldpll)
> +{
> +	WARN(zldpll->dpll_dev, "DPLL device is still registered\n");
> +
> +	mutex_destroy(&zldpll->lock);
> +	kfree(zldpll);
> +}
> +

[Severity: Low]
This isn't a bug, but zl3073x_dpll_alloc() and zl3073x_dpll_free() are
moved verbatim from where the new PTP helpers were inserted to this spot,
with no functional change and no mention in the changelog.  That accounts
for roughly 35 of the 439 added lines and mixes code motion with new
logic.  The new PTP helpers are all static and self-contained, so is the
move needed?  Could it be dropped or split into a separate no-functional-change
patch?

[ ... ]

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

* Re: [PATCH net-next v8 2/3] dpll: zl3073x: add channel ToD, phase step and TIE operations
  2026-08-18 16:37   ` Jakub Kicinski
@ 2026-08-18 16:51     ` Jakub Kicinski
  0 siblings, 0 replies; 10+ messages in thread
From: Jakub Kicinski @ 2026-08-18 16:51 UTC (permalink / raw)
  To: ivecera
  Cc: netdev, poros, vadim.fedorenko, Chris.duQuesnay,
	arkadiusz.kubalewski, jiri, min.li, pabeni, richardcochran,
	linux-kernel

On Tue, 18 Aug 2026 09:37:46 -0700 Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.

Ignore, got confused between feedback on patches 2 and 3 and thought
you(r LLM) were just responding to Gemi-shiko.

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

* Re: [PATCH net-next v8 0/3] dpll: zl3073x: add PTP clock support
  2026-08-14  8:26 [PATCH net-next v8 0/3] dpll: zl3073x: add PTP clock support Ivan Vecera
                   ` (2 preceding siblings ...)
  2026-08-14  8:26 ` [PATCH net-next v8 3/3] dpll: zl3073x: add PTP clock support Ivan Vecera
@ 2026-08-18 17:00 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 10+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-08-18 17:00 UTC (permalink / raw)
  To: Ivan Vecera
  Cc: netdev, arkadiusz.kubalewski, kuba, jiri, min.li, pabeni, poros,
	richardcochran, vadim.fedorenko, linux-kernel

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 14 Aug 2026 10:26:53 +0200 you wrote:
> Add PTP hardware clock support to the zl3073x DPLL driver.
> 
> Patch 1 scales the poll sleep interval in zl3073x_poll_zero_u8()
> proportionally to the timeout to avoid excessive bus traffic for
> the longer PTP-related timeouts.
> 
> Patch 2 adds low-level channel operations for ToD read/write/adjust,
> output phase step, delta frequency offset write and TIE write as
> building blocks for PTP callbacks.
> 
> [...]

Here is the summary with links:
  - [net-next,v8,1/3] dpll: zl3073x: scale poll interval proportionally to timeout
    https://git.kernel.org/netdev/net-next/c/c36461825469
  - [net-next,v8,2/3] dpll: zl3073x: add channel ToD, phase step and TIE operations
    https://git.kernel.org/netdev/net-next/c/2dbf9b756279
  - [net-next,v8,3/3] dpll: zl3073x: add PTP clock support
    https://git.kernel.org/netdev/net-next/c/1db73ce01856

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2026-08-18 17:01 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-14  8:26 [PATCH net-next v8 0/3] dpll: zl3073x: add PTP clock support Ivan Vecera
2026-08-14  8:26 ` [PATCH net-next v8 1/3] dpll: zl3073x: scale poll interval proportionally to timeout Ivan Vecera
2026-08-14  8:26 ` [PATCH net-next v8 2/3] dpll: zl3073x: add channel ToD, phase step and TIE operations Ivan Vecera
2026-08-16 14:47   ` Ivan Vecera
2026-08-18 16:37   ` Jakub Kicinski
2026-08-18 16:51     ` Jakub Kicinski
2026-08-14  8:26 ` [PATCH net-next v8 3/3] dpll: zl3073x: add PTP clock support Ivan Vecera
2026-08-16 14:52   ` Ivan Vecera
2026-08-18 16:37   ` Jakub Kicinski
2026-08-18 17:00 ` [PATCH net-next v8 0/3] " patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox