Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v5 0/4] dpll: add NCO pin type and zl3073x support
@ 2026-05-31 19:44 Ivan Vecera
  2026-05-31 19:44 ` [PATCH net-next v5 1/4] dpll: add DPLL_PIN_TYPE_INT_NCO pin type Ivan Vecera
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Ivan Vecera @ 2026-05-31 19:44 UTC (permalink / raw)
  To: netdev
  Cc: Arkadiusz Kubalewski, David S. Miller, Donald Hunter,
	Eric Dumazet, Jakub Kicinski, Jiri Pirko, Michal Schmidt,
	Paolo Abeni, Pasi Vaananen, Petr Oros, Prathosh Satish,
	Simon Horman, Vadim Fedorenko, linux-kernel

Add a new DPLL_PIN_TYPE_INT_NCO pin type for virtual pins representing
the NCO mode of a DPLL and implement support for it in the zl3073x driver.

Patch 1 adds the new pin type to the DPLL netlink spec and UAPI header.

Patch 2 replaces the single 2s poll timeout with per-operation timeouts
based on Microchip proprietary source code and own measurement.

Patch 3 adds a per-DPLL serialization mutex taken by all DPLL callbacks
and the periodic worker, establishing a single lock that protects all
per-channel state. The chan_state_update() call is moved under this lock.

Patch 4 adds a virtual NCO input pin to the zl3073x driver that allows
userspace to switch a DPLL channel into NCO mode. The pin reports
connected/active state when the channel is in NCO mode and handles
the hardware-specific details of mode transitions including automatic
df_offset capture and 1PPS phase preservation.

Changes:
v5:
  - Rebased on net-next after the freq_monitor per-device fix
    landed via net.
  - Move phase_offset_avg_factor_set notification loop outside lock
    to avoid lockdep false positive on multi-channel devices (patch 3).
  - Configure dpll_df_read register before NCO mode switch (patch 4).
  - Add 5ms delay before reading nco_auto_read captured df_offset,
    determined by HW testing (patch 4).
  - Mark df_offset as unknown at probe when firmware left channel
    in NCO mode (patch 4).
  - See individual patches for detailed changelogs.
v4:
  - New patch 2: per-operation poll timeouts
  - New patch 3: per-DPLL serialization lock
  - See individual patches for detailed changelogs.
v3:
  - fixed SoB position
v2:
  - See individual patches for detailed changelogs.

Ivan Vecera (4):
  dpll: add DPLL_PIN_TYPE_INT_NCO pin type
  dpll: zl3073x: use per-operation poll timeouts
  dpll: zl3073x: add per-DPLL serialization lock
  dpll: zl3073x: add NCO virtual input pin

 Documentation/netlink/specs/dpll.yaml |  13 +
 drivers/dpll/dpll_nl.c                |   2 +-
 drivers/dpll/zl3073x/chan.c           | 124 ++++++-
 drivers/dpll/zl3073x/chan.h           |  48 +++
 drivers/dpll/zl3073x/core.c           |  44 +--
 drivers/dpll/zl3073x/core.h           |  10 +-
 drivers/dpll/zl3073x/dpll.c           | 488 ++++++++++++++++++++++----
 drivers/dpll/zl3073x/dpll.h           |   4 +
 drivers/dpll/zl3073x/regs.h           |  11 +
 include/uapi/linux/dpll.h             |   4 +
 10 files changed, 651 insertions(+), 97 deletions(-)


base-commit: 8415598365503ced2e3d019491b0a2756c85c494
-- 
2.53.0


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

* [PATCH net-next v5 1/4] dpll: add DPLL_PIN_TYPE_INT_NCO pin type
  2026-05-31 19:44 [PATCH net-next v5 0/4] dpll: add NCO pin type and zl3073x support Ivan Vecera
@ 2026-05-31 19:44 ` Ivan Vecera
  2026-06-04  1:50   ` Jakub Kicinski
  2026-05-31 19:44 ` [PATCH net-next v5 2/4] dpll: zl3073x: use per-operation poll timeouts Ivan Vecera
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Ivan Vecera @ 2026-05-31 19:44 UTC (permalink / raw)
  To: netdev
  Cc: Jiri Pirko, Arkadiusz Kubalewski, David S. Miller, Donald Hunter,
	Eric Dumazet, Jakub Kicinski, Jiri Pirko, Michal Schmidt,
	Paolo Abeni, Pasi Vaananen, Petr Oros, Prathosh Satish,
	Simon Horman, Vadim Fedorenko, linux-kernel

Add DPLL_PIN_TYPE_INT_NCO pin type for virtual pins representing
the NCO mode of a DPLL. When connected as a DPLL input, the DPLL
enters NCO mode where the output frequency is adjusted by the host
via the PTP clock interface.

Update the fractional-frequency-offset and fractional-frequency-
offset-ppt attribute documentation to note that for INT_NCO pins
these attributes represent the DPLL's current output frequency
offset from its nominal frequency.

Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
v2:
  - Clarify int-nco pin type documentation to describe frequency
    control via the PTP clock interface instead of generic "controlled
    by the host".
  - Tighten FFO attribute documentation for INT_NCO pins to describe
    the DPLL's output frequency offset from nominal frequency.
  - Mention both fractional-frequency-offset (PPM) and
    fractional-frequency-offset-ppt attributes in the commit message.
---
 Documentation/netlink/specs/dpll.yaml | 13 +++++++++++++
 drivers/dpll/dpll_nl.c                |  2 +-
 include/uapi/linux/dpll.h             |  4 ++++
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/Documentation/netlink/specs/dpll.yaml b/Documentation/netlink/specs/dpll.yaml
index 91a172617b3a9..5cdb93e8649a0 100644
--- a/Documentation/netlink/specs/dpll.yaml
+++ b/Documentation/netlink/specs/dpll.yaml
@@ -162,6 +162,13 @@ definitions:
       -
         name: gnss
         doc: GNSS recovered clock
+      -
+        name: int-nco
+        doc: |
+          Device internal numerically controlled oscillator.
+          When connected as a DPLL input, the DPLL enters NCO mode
+          where the output frequency is adjusted by the host via
+          the PTP clock interface.
     render-max: true
   -
     type: enum
@@ -453,6 +460,9 @@ attribute-sets:
           offset on the media associated with the pin. Inside
           the pin-parent-device nest it represents the frequency
           offset between the pin and its parent DPLL device.
+          For pins of type PIN_TYPE_INT_NCO this represents
+          the DPLL's current output frequency offset from its
+          nominal frequency.
           Value is in PPM (parts per million).
           This is a lower-precision version of
           fractional-frequency-offset-ppt.
@@ -499,6 +509,9 @@ attribute-sets:
           offset on the media associated with the pin. Inside
           the pin-parent-device nest it represents the frequency
           offset between the pin and its parent DPLL device.
+          For pins of type PIN_TYPE_INT_NCO this represents
+          the DPLL's current output frequency offset from its
+          nominal frequency.
           Value is in PPT (parts per trillion, 10^-12).
           This is a higher-precision version of
           fractional-frequency-offset.
diff --git a/drivers/dpll/dpll_nl.c b/drivers/dpll/dpll_nl.c
index b1d9182c7802f..2dab99202764f 100644
--- a/drivers/dpll/dpll_nl.c
+++ b/drivers/dpll/dpll_nl.c
@@ -61,7 +61,7 @@ static const struct nla_policy dpll_pin_id_get_nl_policy[DPLL_A_PIN_TYPE + 1] =
 	[DPLL_A_PIN_BOARD_LABEL] = { .type = NLA_NUL_STRING, },
 	[DPLL_A_PIN_PANEL_LABEL] = { .type = NLA_NUL_STRING, },
 	[DPLL_A_PIN_PACKAGE_LABEL] = { .type = NLA_NUL_STRING, },
-	[DPLL_A_PIN_TYPE] = NLA_POLICY_RANGE(NLA_U32, 1, 5),
+	[DPLL_A_PIN_TYPE] = NLA_POLICY_RANGE(NLA_U32, 1, 6),
 };
 
 /* DPLL_CMD_PIN_GET - do */
diff --git a/include/uapi/linux/dpll.h b/include/uapi/linux/dpll.h
index cb363cccf2e2a..9245827de3cfd 100644
--- a/include/uapi/linux/dpll.h
+++ b/include/uapi/linux/dpll.h
@@ -127,6 +127,9 @@ enum dpll_type {
  * @DPLL_PIN_TYPE_SYNCE_ETH_PORT: ethernet port PHY's recovered clock
  * @DPLL_PIN_TYPE_INT_OSCILLATOR: device internal oscillator
  * @DPLL_PIN_TYPE_GNSS: GNSS recovered clock
+ * @DPLL_PIN_TYPE_INT_NCO: Device internal numerically controlled oscillator.
+ *   When connected as a DPLL input, the DPLL enters NCO mode where the output
+ *   frequency is adjusted by the host via the PTP clock interface.
  */
 enum dpll_pin_type {
 	DPLL_PIN_TYPE_MUX = 1,
@@ -134,6 +137,7 @@ enum dpll_pin_type {
 	DPLL_PIN_TYPE_SYNCE_ETH_PORT,
 	DPLL_PIN_TYPE_INT_OSCILLATOR,
 	DPLL_PIN_TYPE_GNSS,
+	DPLL_PIN_TYPE_INT_NCO,
 
 	/* private: */
 	__DPLL_PIN_TYPE_MAX,
-- 
2.53.0


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

* [PATCH net-next v5 2/4] dpll: zl3073x: use per-operation poll timeouts
  2026-05-31 19:44 [PATCH net-next v5 0/4] dpll: add NCO pin type and zl3073x support Ivan Vecera
  2026-05-31 19:44 ` [PATCH net-next v5 1/4] dpll: add DPLL_PIN_TYPE_INT_NCO pin type Ivan Vecera
@ 2026-05-31 19:44 ` Ivan Vecera
  2026-05-31 19:44 ` [PATCH net-next v5 3/4] dpll: zl3073x: add per-DPLL serialization lock Ivan Vecera
  2026-05-31 19:44 ` [PATCH net-next v5 4/4] dpll: zl3073x: add NCO virtual input pin Ivan Vecera
  3 siblings, 0 replies; 8+ messages in thread
From: Ivan Vecera @ 2026-05-31 19:44 UTC (permalink / raw)
  To: netdev
  Cc: Arkadiusz Kubalewski, David S. Miller, Donald Hunter,
	Eric Dumazet, Jakub Kicinski, Jiri Pirko, Michal Schmidt,
	Paolo Abeni, Pasi Vaananen, Petr Oros, Prathosh Satish,
	Simon Horman, Vadim Fedorenko, linux-kernel

Replace the single 2s timeout in zl3073x_poll_zero_u8() with a
per-caller timeout parameter. Different HW operations have different
expected completion times so using per-operation timeouts improves
error detection. The timeout values are based on proprietary source
code provided by Microchip and own measurement.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/chan.c |  6 ++++--
 drivers/dpll/zl3073x/core.c | 29 +++++++++++++++++------------
 drivers/dpll/zl3073x/core.h | 10 +++++++++-
 3 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/drivers/dpll/zl3073x/chan.c b/drivers/dpll/zl3073x/chan.c
index 2fe3c3da84bb5..677a920c16254 100644
--- a/drivers/dpll/zl3073x/chan.c
+++ b/drivers/dpll/zl3073x/chan.c
@@ -33,7 +33,8 @@ int zl3073x_chan_state_update(struct zl3073x_dev *zldev, u8 index)
 
 	/* Read df_offset vs tracked reference */
 	rc = zl3073x_poll_zero_u8(zldev, ZL_REG_DPLL_DF_READ(index),
-				  ZL_DPLL_DF_READ_SEM);
+				  ZL_DPLL_DF_READ_SEM,
+				  ZL_POLL_DF_READ_TIMEOUT_US);
 	if (rc)
 		return rc;
 
@@ -43,7 +44,8 @@ int zl3073x_chan_state_update(struct zl3073x_dev *zldev, u8 index)
 		return rc;
 
 	rc = zl3073x_poll_zero_u8(zldev, ZL_REG_DPLL_DF_READ(index),
-				  ZL_DPLL_DF_READ_SEM);
+				  ZL_DPLL_DF_READ_SEM,
+				  ZL_POLL_DF_READ_TIMEOUT_US);
 	if (rc)
 		return rc;
 
diff --git a/drivers/dpll/zl3073x/core.c b/drivers/dpll/zl3073x/core.c
index 8e6416a4741de..0b2050aa2ed92 100644
--- a/drivers/dpll/zl3073x/core.c
+++ b/drivers/dpll/zl3073x/core.c
@@ -311,17 +311,17 @@ int zl3073x_write_u48(struct zl3073x_dev *zldev, unsigned int reg, u64 val)
  * @zldev: zl3073x device pointer
  * @reg: register to poll (has to be 8bit register)
  * @mask: bit mask for polling
+ * @timeout_us: timeout in microseconds
  *
  * Waits for bits specified by @mask in register @reg value to be cleared
  * by the device.
  *
  * Returns: 0 on success, <0 on error
  */
-int zl3073x_poll_zero_u8(struct zl3073x_dev *zldev, unsigned int reg, u8 mask)
+int zl3073x_poll_zero_u8(struct zl3073x_dev *zldev, unsigned int reg,
+			 u8 mask, unsigned int timeout_us)
 {
-	/* Register polling sleep & timeout */
-#define ZL_POLL_SLEEP_US   10
-#define ZL_POLL_TIMEOUT_US 2000000
+#define ZL_POLL_SLEEP_US 10
 	unsigned int val;
 
 	/* Check the register is 8bit */
@@ -335,7 +335,7 @@ int zl3073x_poll_zero_u8(struct zl3073x_dev *zldev, unsigned int reg, u8 mask)
 	reg = ZL_REG_ADDR(reg) + ZL_RANGE_OFFSET;
 
 	return regmap_read_poll_timeout(zldev->regmap, reg, val, !(val & mask),
-					ZL_POLL_SLEEP_US, ZL_POLL_TIMEOUT_US);
+					ZL_POLL_SLEEP_US, timeout_us);
 }
 
 int zl3073x_mb_op(struct zl3073x_dev *zldev, unsigned int op_reg, u8 op_val,
@@ -354,7 +354,8 @@ int zl3073x_mb_op(struct zl3073x_dev *zldev, unsigned int op_reg, u8 op_val,
 		return rc;
 
 	/* Wait for the operation to actually finish */
-	return zl3073x_poll_zero_u8(zldev, op_reg, op_val);
+	return zl3073x_poll_zero_u8(zldev, op_reg, op_val,
+				    ZL_POLL_MB_TIMEOUT_US);
 }
 
 /**
@@ -377,8 +378,8 @@ zl3073x_do_hwreg_op(struct zl3073x_dev *zldev, u8 op)
 		return rc;
 
 	/* Poll for completion - pending bit cleared */
-	return zl3073x_poll_zero_u8(zldev, ZL_REG_HWREG_OP,
-				    ZL_HWREG_OP_PENDING);
+	return zl3073x_poll_zero_u8(zldev, ZL_REG_HWREG_OP, ZL_HWREG_OP_PENDING,
+				    ZL_POLL_HWREG_TIMEOUT_US);
 }
 
 /**
@@ -609,7 +610,8 @@ int zl3073x_ref_phase_offsets_update(struct zl3073x_dev *zldev, int channel)
 	 * to be zero to ensure that the measured data are coherent.
 	 */
 	rc = zl3073x_poll_zero_u8(zldev, ZL_REG_REF_PHASE_ERR_READ_RQST,
-				  ZL_REF_PHASE_ERR_READ_RQST_RD);
+				  ZL_REF_PHASE_ERR_READ_RQST_RD,
+				  ZL_POLL_PHASE_ERR_TIMEOUT_US);
 	if (rc)
 		return rc;
 
@@ -628,7 +630,8 @@ int zl3073x_ref_phase_offsets_update(struct zl3073x_dev *zldev, int channel)
 
 	/* Wait for finish */
 	return zl3073x_poll_zero_u8(zldev, ZL_REG_REF_PHASE_ERR_READ_RQST,
-				    ZL_REF_PHASE_ERR_READ_RQST_RD);
+				    ZL_REF_PHASE_ERR_READ_RQST_RD,
+				    ZL_POLL_PHASE_ERR_TIMEOUT_US);
 }
 
 /**
@@ -648,7 +651,8 @@ zl3073x_ref_freq_meas_latch(struct zl3073x_dev *zldev, u8 type)
 
 	/* Wait for previous measurement to finish */
 	rc = zl3073x_poll_zero_u8(zldev, ZL_REG_REF_FREQ_MEAS_CTRL,
-				  ZL_REF_FREQ_MEAS_CTRL);
+				  ZL_REF_FREQ_MEAS_CTRL,
+				  ZL_POLL_FREQ_MEAS_TIMEOUT_US);
 	if (rc)
 		return rc;
 
@@ -669,7 +673,8 @@ zl3073x_ref_freq_meas_latch(struct zl3073x_dev *zldev, u8 type)
 
 	/* Wait for finish */
 	return zl3073x_poll_zero_u8(zldev, ZL_REG_REF_FREQ_MEAS_CTRL,
-				    ZL_REF_FREQ_MEAS_CTRL);
+				    ZL_REF_FREQ_MEAS_CTRL,
+				    ZL_POLL_FREQ_MEAS_TIMEOUT_US);
 }
 
 /**
diff --git a/drivers/dpll/zl3073x/core.h b/drivers/dpll/zl3073x/core.h
index addba378b0df4..6b55a05a222ed 100644
--- a/drivers/dpll/zl3073x/core.h
+++ b/drivers/dpll/zl3073x/core.h
@@ -7,6 +7,7 @@
 #include <linux/kthread.h>
 #include <linux/list.h>
 #include <linux/mutex.h>
+#include <linux/time64.h>
 #include <linux/types.h>
 
 #include "chan.h"
@@ -19,6 +20,12 @@ struct device;
 struct regmap;
 struct zl3073x_dpll;
 
+/* Per-operation poll timeouts */
+#define ZL_POLL_DF_READ_TIMEOUT_US	(25 * USEC_PER_MSEC)
+#define ZL_POLL_FREQ_MEAS_TIMEOUT_US	(50 * USEC_PER_MSEC)
+#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)
 
 enum zl3073x_flags {
 	ZL3073X_FLAG_REF_PHASE_COMP_32_BIT,
@@ -127,7 +134,8 @@ struct zl3073x_hwreg_seq_item {
 
 int zl3073x_mb_op(struct zl3073x_dev *zldev, unsigned int op_reg, u8 op_val,
 		  unsigned int mask_reg, u16 mask_val);
-int zl3073x_poll_zero_u8(struct zl3073x_dev *zldev, unsigned int reg, u8 mask);
+int zl3073x_poll_zero_u8(struct zl3073x_dev *zldev, unsigned int reg,
+			 u8 mask, unsigned int timeout_us);
 int zl3073x_read_u8(struct zl3073x_dev *zldev, unsigned int reg, u8 *val);
 int zl3073x_read_u16(struct zl3073x_dev *zldev, unsigned int reg, u16 *val);
 int zl3073x_read_u32(struct zl3073x_dev *zldev, unsigned int reg, u32 *val);
-- 
2.53.0


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

* [PATCH net-next v5 3/4] dpll: zl3073x: add per-DPLL serialization lock
  2026-05-31 19:44 [PATCH net-next v5 0/4] dpll: add NCO pin type and zl3073x support Ivan Vecera
  2026-05-31 19:44 ` [PATCH net-next v5 1/4] dpll: add DPLL_PIN_TYPE_INT_NCO pin type Ivan Vecera
  2026-05-31 19:44 ` [PATCH net-next v5 2/4] dpll: zl3073x: use per-operation poll timeouts Ivan Vecera
@ 2026-05-31 19:44 ` Ivan Vecera
  2026-06-04  1:40   ` Jakub Kicinski
  2026-06-04  1:51   ` Jakub Kicinski
  2026-05-31 19:44 ` [PATCH net-next v5 4/4] dpll: zl3073x: add NCO virtual input pin Ivan Vecera
  3 siblings, 2 replies; 8+ messages in thread
From: Ivan Vecera @ 2026-05-31 19:44 UTC (permalink / raw)
  To: netdev
  Cc: Arkadiusz Kubalewski, David S. Miller, Donald Hunter,
	Eric Dumazet, Jakub Kicinski, Jiri Pirko, Michal Schmidt,
	Paolo Abeni, Pasi Vaananen, Petr Oros, Prathosh Satish,
	Simon Horman, Vadim Fedorenko, linux-kernel

Add a per-DPLL mutex that serializes all operations on a given DPLL
channel across DPLL netlink callbacks, the periodic kthread worker,
and (in subsequent patches) PTP clock callbacks.

All DPLL pin and device callbacks that access mutable state take the
lock as the first operation. The periodic worker holds it for the
entire check cycle of each channel, deferring change notifications
until after the lock is released to avoid ABBA deadlock with
dpll_lock. This establishes the lock ordering:
dpll_lock (subsystem, outer) -> zldpll->lock (driver, inner).

Move zl3073x_chan_state_update() from the per-device
zl3073x_dev_chan_states_update() loop into the per-DPLL
zl3073x_dpll_changes_check() so it runs under zldpll->lock.
This serializes df_offset writes with all readers and
eliminates the need for separate df_offset synchronization.

Change pin->freq_offset from atomic64_t to plain s64 since all
readers and writers are now serialized by zldpll->lock, making
atomic access unnecessary.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/core.c |  15 ---
 drivers/dpll/zl3073x/dpll.c | 188 ++++++++++++++++++++++++++++--------
 drivers/dpll/zl3073x/dpll.h |   2 +
 3 files changed, 149 insertions(+), 56 deletions(-)

diff --git a/drivers/dpll/zl3073x/core.c b/drivers/dpll/zl3073x/core.c
index 0b2050aa2ed92..27c71807e4eff 100644
--- a/drivers/dpll/zl3073x/core.c
+++ b/drivers/dpll/zl3073x/core.c
@@ -567,19 +567,7 @@ zl3073x_dev_ref_states_update(struct zl3073x_dev *zldev)
 	}
 }
 
-static void
-zl3073x_dev_chan_states_update(struct zl3073x_dev *zldev)
-{
-	int i, rc;
 
-	for (i = 0; i < zldev->info->num_channels; i++) {
-		rc = zl3073x_chan_state_update(zldev, i);
-		if (rc)
-			dev_warn(zldev->dev,
-				 "Failed to get DPLL%u state: %pe\n", i,
-				 ERR_PTR(rc));
-	}
-}
 
 /**
  * zl3073x_ref_phase_offsets_update - update reference phase offsets
@@ -720,9 +708,6 @@ zl3073x_dev_periodic_work(struct kthread_work *work)
 	/* Update input references' states */
 	zl3073x_dev_ref_states_update(zldev);
 
-	/* Update DPLL channels' states */
-	zl3073x_dev_chan_states_update(zldev);
-
 	/* Update DPLL-to-connected-ref phase offsets registers */
 	rc = zl3073x_ref_phase_offsets_update(zldev, -1);
 	if (rc)
diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
index 5e58ded5734d7..4bee3d0c46593 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0-only
 
-#include <linux/atomic.h>
 #include <linux/bits.h>
 #include <linux/bitfield.h>
 #include <linux/bug.h>
@@ -58,7 +57,7 @@ struct zl3073x_dpll_pin {
 	s32			phase_gran;
 	enum dpll_pin_operstate	operstate;
 	s64			phase_offset;
-	atomic64_t		freq_offset;
+	s64			freq_offset;
 	u32			measured_freq;
 };
 
@@ -134,6 +133,8 @@ zl3073x_dpll_input_pin_esync_get(const struct dpll_pin *dpll_pin,
 	const struct zl3073x_ref *ref;
 	u8 ref_id;
 
+	guard(mutex)(&zldpll->lock);
+
 	ref_id = zl3073x_input_pin_ref_get(pin->id);
 	ref = zl3073x_ref_state_get(zldev, ref_id);
 
@@ -170,6 +171,8 @@ zl3073x_dpll_input_pin_esync_set(const struct dpll_pin *dpll_pin,
 	struct zl3073x_ref ref;
 	u8 ref_id, sync_mode;
 
+	guard(mutex)(&zldpll->lock);
+
 	ref_id = zl3073x_input_pin_ref_get(pin->id);
 	ref = *zl3073x_ref_state_get(zldev, ref_id);
 
@@ -205,6 +208,8 @@ zl3073x_dpll_input_pin_ref_sync_get(const struct dpll_pin *dpll_pin,
 	const struct zl3073x_ref *ref;
 	u8 ref_id, mode, pair;
 
+	guard(mutex)(&zldpll->lock);
+
 	ref_id = zl3073x_input_pin_ref_get(pin->id);
 	ref = zl3073x_ref_state_get(zldev, ref_id);
 	mode = zl3073x_ref_sync_mode_get(ref);
@@ -236,6 +241,8 @@ zl3073x_dpll_input_pin_ref_sync_set(const struct dpll_pin *dpll_pin,
 	struct zl3073x_ref ref;
 	int rc;
 
+	guard(mutex)(&zldpll->lock);
+
 	ref_id = zl3073x_input_pin_ref_get(pin->id);
 	sync_ref_id = zl3073x_input_pin_ref_get(sync_pin->id);
 	ref = *zl3073x_ref_state_get(zldev, ref_id);
@@ -299,12 +306,15 @@ zl3073x_dpll_input_pin_ffo_get(const struct dpll_pin *dpll_pin, void *pin_priv,
 			       struct dpll_ffo_param *ffo,
 			       struct netlink_ext_ack *extack)
 {
+	struct zl3073x_dpll *zldpll = dpll_priv;
 	struct zl3073x_dpll_pin *pin = pin_priv;
 
+	guard(mutex)(&zldpll->lock);
+
 	if (pin->operstate != DPLL_PIN_OPERSTATE_ACTIVE)
 		return -ENODATA;
 
-	ffo->ffo = atomic64_read(&pin->freq_offset);
+	ffo->ffo = pin->freq_offset;
 
 	return 0;
 }
@@ -316,8 +326,11 @@ zl3073x_dpll_input_pin_measured_freq_get(const struct dpll_pin *dpll_pin,
 					 void *dpll_priv, u64 *measured_freq,
 					 struct netlink_ext_ack *extack)
 {
+	struct zl3073x_dpll *zldpll = dpll_priv;
 	struct zl3073x_dpll_pin *pin = pin_priv;
 
+	guard(mutex)(&zldpll->lock);
+
 	*measured_freq = pin->measured_freq;
 	*measured_freq *= DPLL_PIN_MEASURED_FREQUENCY_DIVIDER;
 
@@ -335,6 +348,8 @@ zl3073x_dpll_input_pin_frequency_get(const struct dpll_pin *dpll_pin,
 	struct zl3073x_dpll_pin *pin = pin_priv;
 	u8 ref_id;
 
+	guard(mutex)(&zldpll->lock);
+
 	ref_id = zl3073x_input_pin_ref_get(pin->id);
 	*frequency = zl3073x_dev_ref_freq_get(zldpll->dev, ref_id);
 
@@ -354,6 +369,8 @@ zl3073x_dpll_input_pin_frequency_set(const struct dpll_pin *dpll_pin,
 	struct zl3073x_ref ref;
 	u8 ref_id;
 
+	guard(mutex)(&zldpll->lock);
+
 	/* Get reference state */
 	ref_id = zl3073x_input_pin_ref_get(pin->id);
 	ref = *zl3073x_ref_state_get(zldev, ref_id);
@@ -402,6 +419,8 @@ zl3073x_dpll_input_pin_phase_offset_get(const struct dpll_pin *dpll_pin,
 	u8 conn_id, ref_id;
 	s64 ref_phase;
 
+	guard(mutex)(&zldpll->lock);
+
 	/* Get currently connected reference */
 	conn_id = zl3073x_dpll_connected_ref_get(zldpll);
 
@@ -459,6 +478,8 @@ zl3073x_dpll_input_pin_phase_adjust_get(const struct dpll_pin *dpll_pin,
 	s64 phase_comp;
 	u8 ref_id;
 
+	guard(mutex)(&zldpll->lock);
+
 	/* Read reference configuration */
 	ref_id = zl3073x_input_pin_ref_get(pin->id);
 	ref = zl3073x_ref_state_get(zldev, ref_id);
@@ -491,6 +512,8 @@ zl3073x_dpll_input_pin_phase_adjust_set(const struct dpll_pin *dpll_pin,
 	struct zl3073x_ref ref;
 	u8 ref_id;
 
+	guard(mutex)(&zldpll->lock);
+
 	/* Read reference configuration */
 	ref_id = zl3073x_input_pin_ref_get(pin->id);
 	ref = *zl3073x_ref_state_get(zldev, ref_id);
@@ -524,6 +547,8 @@ zl3073x_dpll_ref_operstate_get(struct zl3073x_dpll_pin *pin,
 	const struct zl3073x_ref *ref;
 	u8 ref_id;
 
+	lockdep_assert_held(&zldpll->lock);
+
 	ref_id = zl3073x_input_pin_ref_get(pin->id);
 
 	/* Check if this pin is the currently locked reference */
@@ -557,6 +582,8 @@ zl3073x_dpll_input_pin_state_on_dpll_get(const struct dpll_pin *dpll_pin,
 	const struct zl3073x_chan *chan;
 	u8 mode, ref;
 
+	guard(mutex)(&zldpll->lock);
+
 	chan = zl3073x_chan_state_get(zldpll->dev, zldpll->id);
 	ref = zl3073x_input_pin_ref_get(pin->id);
 	mode = zl3073x_chan_mode_get(chan);
@@ -590,8 +617,11 @@ zl3073x_dpll_input_pin_operstate_on_dpll_get(const struct dpll_pin *dpll_pin,
 					     enum dpll_pin_operstate *operstate,
 					     struct netlink_ext_ack *extack)
 {
+	struct zl3073x_dpll *zldpll = dpll_priv;
 	struct zl3073x_dpll_pin *pin = pin_priv;
 
+	guard(mutex)(&zldpll->lock);
+
 	return zl3073x_dpll_ref_operstate_get(pin, operstate);
 }
 
@@ -607,7 +637,9 @@ zl3073x_dpll_input_pin_state_on_dpll_set(const struct dpll_pin *dpll_pin,
 	struct zl3073x_dpll_pin *pin = pin_priv;
 	struct zl3073x_chan chan;
 	u8 mode, ref;
-	int rc;
+	int rc = 0;
+
+	mutex_lock(&zldpll->lock);
 
 	chan = *zl3073x_chan_state_get(zldpll->dev, zldpll->id);
 	ref = zl3073x_input_pin_ref_get(pin->id);
@@ -649,13 +681,13 @@ zl3073x_dpll_input_pin_state_on_dpll_set(const struct dpll_pin *dpll_pin,
 	case ZL_DPLL_MODE_REFSEL_MODE_AUTO:
 		if (state == DPLL_PIN_STATE_SELECTABLE) {
 			if (zl3073x_chan_ref_is_selectable(&chan, ref))
-				return 0; /* Pin is already selectable */
+				goto unlock; /* Pin is already selectable */
 
 			/* Restore pin priority in HW */
 			zl3073x_chan_ref_prio_set(&chan, ref, pin->prio);
 		} else if (state == DPLL_PIN_STATE_DISCONNECTED) {
 			if (!zl3073x_chan_ref_is_selectable(&chan, ref))
-				return 0; /* Pin is already disconnected */
+				goto unlock; /* Pin is already disconnected */
 
 			/* Set pin priority to none in HW */
 			zl3073x_chan_ref_prio_set(&chan, ref,
@@ -668,18 +700,20 @@ zl3073x_dpll_input_pin_state_on_dpll_set(const struct dpll_pin *dpll_pin,
 		/* In other modes we cannot change input reference */
 		NL_SET_ERR_MSG(extack,
 			       "Pin state cannot be changed in current mode");
-		return -EOPNOTSUPP;
+		rc = -EOPNOTSUPP;
+		goto unlock;
 	}
 
 	/* Commit DPLL channel changes */
 	rc = zl3073x_chan_state_set(zldpll->dev, zldpll->id, &chan);
-	if (rc)
-		return rc;
+	goto unlock;
 
-	return 0;
 invalid_state:
 	NL_SET_ERR_MSG_MOD(extack, "Invalid pin state for this device mode");
-	return -EINVAL;
+	rc = -EINVAL;
+unlock:
+	mutex_unlock(&zldpll->lock);
+	return rc;
 }
 
 static int
@@ -687,8 +721,11 @@ zl3073x_dpll_input_pin_prio_get(const struct dpll_pin *dpll_pin, void *pin_priv,
 				const struct dpll_device *dpll, void *dpll_priv,
 				u32 *prio, struct netlink_ext_ack *extack)
 {
+	struct zl3073x_dpll *zldpll = dpll_priv;
 	struct zl3073x_dpll_pin *pin = pin_priv;
 
+	guard(mutex)(&zldpll->lock);
+
 	*prio = pin->prio;
 
 	return 0;
@@ -705,6 +742,8 @@ zl3073x_dpll_input_pin_prio_set(const struct dpll_pin *dpll_pin, void *pin_priv,
 	u8 ref;
 	int rc;
 
+	guard(mutex)(&zldpll->lock);
+
 	if (prio > ZL_DPLL_REF_PRIO_MAX)
 		return -EINVAL;
 
@@ -740,6 +779,8 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
 	u32 synth_freq, out_freq;
 	u8 out_id;
 
+	guard(mutex)(&zldpll->lock);
+
 	out_id = zl3073x_output_pin_out_get(pin->id);
 	out = zl3073x_out_state_get(zldev, out_id);
 
@@ -797,6 +838,8 @@ zl3073x_dpll_output_pin_esync_set(const struct dpll_pin *dpll_pin,
 	u32 synth_freq;
 	u8 out_id;
 
+	guard(mutex)(&zldpll->lock);
+
 	out_id = zl3073x_output_pin_out_get(pin->id);
 	out = *zl3073x_out_state_get(zldev, out_id);
 
@@ -817,7 +860,7 @@ zl3073x_dpll_output_pin_esync_set(const struct dpll_pin *dpll_pin,
 
 	/* If esync is being disabled just write mailbox and finish */
 	if (!freq)
-		goto write_mailbox;
+		return zl3073x_out_state_set(zldev, out_id, &out);
 
 	/* Get attached synth frequency */
 	synth = zl3073x_synth_state_get(zldev, zl3073x_out_synth_get(&out));
@@ -834,7 +877,6 @@ zl3073x_dpll_output_pin_esync_set(const struct dpll_pin *dpll_pin,
 	 */
 	out.esync_n_width = out.div / 2;
 
-write_mailbox:
 	/* Commit output configuration */
 	return zl3073x_out_state_set(zldev, out_id, &out);
 }
@@ -849,6 +891,8 @@ zl3073x_dpll_output_pin_frequency_get(const struct dpll_pin *dpll_pin,
 	struct zl3073x_dpll *zldpll = dpll_priv;
 	struct zl3073x_dpll_pin *pin = pin_priv;
 
+	guard(mutex)(&zldpll->lock);
+
 	*frequency = zl3073x_dev_output_pin_freq_get(zldpll->dev, pin->id);
 
 	return 0;
@@ -869,6 +913,8 @@ zl3073x_dpll_output_pin_frequency_set(const struct dpll_pin *dpll_pin,
 	struct zl3073x_out out;
 	u8 out_id;
 
+	guard(mutex)(&zldpll->lock);
+
 	out_id = zl3073x_output_pin_out_get(pin->id);
 	out = *zl3073x_out_state_get(zldev, out_id);
 
@@ -942,6 +988,8 @@ zl3073x_dpll_output_pin_phase_adjust_get(const struct dpll_pin *dpll_pin,
 	const struct zl3073x_out *out;
 	u8 out_id;
 
+	guard(mutex)(&zldpll->lock);
+
 	out_id = zl3073x_output_pin_out_get(pin->id);
 	out = zl3073x_out_state_get(zldev, out_id);
 
@@ -965,6 +1013,8 @@ zl3073x_dpll_output_pin_phase_adjust_set(const struct dpll_pin *dpll_pin,
 	struct zl3073x_out out;
 	u8 out_id;
 
+	guard(mutex)(&zldpll->lock);
+
 	out_id = zl3073x_output_pin_out_get(pin->id);
 	out = *zl3073x_out_state_get(zldev, out_id);
 
@@ -998,6 +1048,8 @@ zl3073x_dpll_temp_get(const struct dpll_device *dpll, void *dpll_priv,
 	u16 val;
 	int rc;
 
+	guard(mutex)(&zldpll->lock);
+
 	rc = zl3073x_read_u16(zldev, ZL_REG_DIE_TEMP_STATUS, &val);
 	if (rc)
 		return rc;
@@ -1009,14 +1061,13 @@ zl3073x_dpll_temp_get(const struct dpll_device *dpll, void *dpll_priv,
 }
 
 static int
-zl3073x_dpll_lock_status_get(const struct dpll_device *dpll, void *dpll_priv,
-			     enum dpll_lock_status *status,
-			     enum dpll_lock_status_error *status_error,
-			     struct netlink_ext_ack *extack)
+__zl3073x_dpll_lock_status_get(struct zl3073x_dpll *zldpll,
+			       enum dpll_lock_status *status)
 {
-	struct zl3073x_dpll *zldpll = dpll_priv;
 	const struct zl3073x_chan *chan;
 
+	lockdep_assert_held(&zldpll->lock);
+
 	chan = zl3073x_chan_state_get(zldpll->dev, zldpll->id);
 
 	switch (zl3073x_chan_mode_get(chan)) {
@@ -1052,6 +1103,19 @@ zl3073x_dpll_lock_status_get(const struct dpll_device *dpll, void *dpll_priv,
 	return 0;
 }
 
+static int
+zl3073x_dpll_lock_status_get(const struct dpll_device *dpll, void *dpll_priv,
+			     enum dpll_lock_status *status,
+			     enum dpll_lock_status_error *status_error,
+			     struct netlink_ext_ack *extack)
+{
+	struct zl3073x_dpll *zldpll = dpll_priv;
+
+	guard(mutex)(&zldpll->lock);
+
+	return __zl3073x_dpll_lock_status_get(zldpll, status);
+}
+
 static int
 zl3073x_dpll_supported_modes_get(const struct dpll_device *dpll,
 				 void *dpll_priv, unsigned long *modes,
@@ -1060,6 +1124,8 @@ zl3073x_dpll_supported_modes_get(const struct dpll_device *dpll,
 	struct zl3073x_dpll *zldpll = dpll_priv;
 	const struct zl3073x_chan *chan;
 
+	guard(mutex)(&zldpll->lock);
+
 	chan = zl3073x_chan_state_get(zldpll->dev, zldpll->id);
 
 	/* We support switching between automatic and manual mode, except in
@@ -1082,6 +1148,8 @@ zl3073x_dpll_mode_get(const struct dpll_device *dpll, void *dpll_priv,
 	struct zl3073x_dpll *zldpll = dpll_priv;
 	const struct zl3073x_chan *chan;
 
+	guard(mutex)(&zldpll->lock);
+
 	chan = zl3073x_chan_state_get(zldpll->dev, zldpll->id);
 
 	switch (zl3073x_chan_mode_get(chan)) {
@@ -1112,6 +1180,8 @@ zl3073x_dpll_phase_offset_avg_factor_get(const struct dpll_device *dpll,
 {
 	struct zl3073x_dpll *zldpll = dpll_priv;
 
+	guard(mutex)(&zldpll->lock);
+
 	*factor = zl3073x_dev_phase_avg_factor_get(zldpll->dev);
 
 	return 0;
@@ -1125,9 +1195,12 @@ zl3073x_dpll_phase_offset_avg_factor_set(const struct dpll_device *dpll,
 	struct zl3073x_dpll *item, *zldpll = dpll_priv;
 	int rc;
 
+	mutex_lock(&zldpll->lock);
+
 	if (factor > 15) {
 		NL_SET_ERR_MSG_FMT(extack,
 				   "Phase offset average factor has to be from range <0,15>");
+		mutex_unlock(&zldpll->lock);
 		return -EINVAL;
 	}
 
@@ -1135,11 +1208,14 @@ zl3073x_dpll_phase_offset_avg_factor_set(const struct dpll_device *dpll,
 	if (rc) {
 		NL_SET_ERR_MSG_FMT(extack,
 				   "Failed to set phase offset averaging factor");
+		mutex_unlock(&zldpll->lock);
 		return rc;
 	}
 
-	/* The averaging factor is common for all DPLL channels so after change
-	 * we have to send a notification for other DPLL devices.
+	mutex_unlock(&zldpll->lock);
+
+	/* The averaging factor is common for all DPLL channels so after
+	 * change we have to send a notification for other DPLL devices.
 	 */
 	list_for_each_entry(item, &zldpll->dev->dplls, list) {
 		struct dpll_device *dpll_dev = READ_ONCE(item->dpll_dev);
@@ -1160,6 +1236,8 @@ zl3073x_dpll_mode_set(const struct dpll_device *dpll, void *dpll_priv,
 	u8 hw_mode, ref;
 	int rc;
 
+	guard(mutex)(&zldpll->lock);
+
 	chan = *zl3073x_chan_state_get(zldpll->dev, zldpll->id);
 	ref = zl3073x_chan_refsel_ref_get(&chan);
 
@@ -1221,6 +1299,8 @@ zl3073x_dpll_phase_offset_monitor_get(const struct dpll_device *dpll,
 {
 	struct zl3073x_dpll *zldpll = dpll_priv;
 
+	guard(mutex)(&zldpll->lock);
+
 	if (zldpll->phase_monitor)
 		*state = DPLL_FEATURE_STATE_ENABLE;
 	else
@@ -1237,6 +1317,8 @@ zl3073x_dpll_phase_offset_monitor_set(const struct dpll_device *dpll,
 {
 	struct zl3073x_dpll *zldpll = dpll_priv;
 
+	guard(mutex)(&zldpll->lock);
+
 	zldpll->phase_monitor = (state == DPLL_FEATURE_STATE_ENABLE);
 
 	return 0;
@@ -1697,6 +1779,8 @@ zl3073x_dpll_pin_phase_offset_check(struct zl3073x_dpll_pin *pin)
 	u8 ref_id;
 	int rc;
 
+	lockdep_assert_held(&zldpll->lock);
+
 	/* No phase offset if the ref monitor reports signal errors */
 	ref_id = zl3073x_input_pin_ref_get(pin->id);
 	if (!zl3073x_dev_ref_is_status_ok(zldev, ref_id))
@@ -1753,6 +1837,8 @@ zl3073x_dpll_pin_ffo_check(struct zl3073x_dpll_pin *pin)
 	const struct zl3073x_chan *chan;
 	s64 ffo;
 
+	lockdep_assert_held(&zldpll->lock);
+
 	if (pin->operstate != DPLL_PIN_OPERSTATE_ACTIVE)
 		return false;
 
@@ -1760,9 +1846,10 @@ zl3073x_dpll_pin_ffo_check(struct zl3073x_dpll_pin *pin)
 	ffo = mul_s64_u64_shr(zl3073x_chan_df_offset_get(chan),
 			      244140625, 36);
 
-	if (atomic64_xchg(&pin->freq_offset, ffo) != ffo) {
+	if (pin->freq_offset != ffo) {
 		dev_dbg(zldev->dev, "%s freq offset changed to: %lld\n",
 			pin->label, ffo);
+		pin->freq_offset = ffo;
 		return true;
 	}
 
@@ -1787,6 +1874,8 @@ zl3073x_dpll_pin_measured_freq_check(struct zl3073x_dpll_pin *pin)
 	u8 ref_id;
 	u32 freq;
 
+	lockdep_assert_held(&zldpll->lock);
+
 	if (!zldpll->dev->freq_monitor)
 		return false;
 
@@ -1817,27 +1906,37 @@ zl3073x_dpll_pin_measured_freq_check(struct zl3073x_dpll_pin *pin)
 void
 zl3073x_dpll_changes_check(struct zl3073x_dpll *zldpll)
 {
+	DECLARE_BITMAP(changed_pins, ZL3073X_NUM_INPUT_PINS);
 	struct zl3073x_dev *zldev = zldpll->dev;
 	enum dpll_lock_status lock_status;
 	struct device *dev = zldev->dev;
 	struct zl3073x_dpll_pin *pin;
+	bool dev_changed = false;
 	int rc;
 
+	bitmap_zero(changed_pins, ZL3073X_NUM_INPUT_PINS);
+
+	mutex_lock(&zldpll->lock);
+
 	zldpll->check_count++;
 
-	/* Get current lock status for the DPLL */
-	rc = zl3073x_dpll_lock_status_get(zldpll->dpll_dev, zldpll,
-					  &lock_status, NULL, NULL);
+	rc = zl3073x_chan_state_update(zldev, zldpll->id);
+	if (rc) {
+		dev_err(dev, "Failed to get DPLL%u state: %pe\n",
+			zldpll->id, ERR_PTR(rc));
+		goto unlock;
+	}
+
+	rc = __zl3073x_dpll_lock_status_get(zldpll, &lock_status);
 	if (rc) {
 		dev_err(dev, "Failed to get DPLL%u lock status: %pe\n",
 			zldpll->id, ERR_PTR(rc));
-		return;
+		goto unlock;
 	}
 
-	/* If lock status was changed then notify DPLL core */
 	if (zldpll->lock_status != lock_status) {
 		zldpll->lock_status = lock_status;
-		dpll_device_change_ntf(zldpll->dpll_dev);
+		dev_changed = true;
 	}
 
 	/* Update phase offset latch registers for this DPLL if the phase
@@ -1849,17 +1948,13 @@ zl3073x_dpll_changes_check(struct zl3073x_dpll *zldpll)
 			dev_err(zldev->dev,
 				"Failed to update phase offsets: %pe\n",
 				ERR_PTR(rc));
-			return;
+			goto unlock;
 		}
 	}
 
 	list_for_each_entry(pin, &zldpll->pins, list) {
 		enum dpll_pin_operstate operstate;
-		bool pin_changed = false;
 
-		/* Output pins change checks are not necessary because output
-		 * states are constant.
-		 */
 		if (!zl3073x_dpll_is_input_pin(pin))
 			continue;
 
@@ -1868,31 +1963,40 @@ zl3073x_dpll_changes_check(struct zl3073x_dpll *zldpll)
 			dev_err(dev,
 				"Failed to get %s on DPLL%u oper state: %pe\n",
 				pin->label, zldpll->id, ERR_PTR(rc));
-			return;
+			goto unlock;
 		}
 
 		if (operstate != pin->operstate) {
 			dev_dbg(dev, "%s oper state changed: %u->%u\n",
 				pin->label, pin->operstate, operstate);
 			pin->operstate = operstate;
-			pin_changed = true;
+			set_bit(pin->id, changed_pins);
 		}
 
-		/* Check for phase offset, ffo, and measured freq change
-		 * once per second.
-		 */
 		if (zldpll->check_count % 2 == 0) {
 			if (zl3073x_dpll_pin_phase_offset_check(pin))
-				pin_changed = true;
+				set_bit(pin->id, changed_pins);
 
 			if (zl3073x_dpll_pin_ffo_check(pin))
-				pin_changed = true;
+				set_bit(pin->id, changed_pins);
 
 			if (zl3073x_dpll_pin_measured_freq_check(pin))
-				pin_changed = true;
+				set_bit(pin->id, changed_pins);
 		}
+	}
+
+unlock:
+	mutex_unlock(&zldpll->lock);
 
-		if (pin_changed)
+	/* Send notifications outside the lock to avoid ABBA deadlock
+	 * with dpll_lock taken by notification functions.
+	 */
+	if (dev_changed)
+		dpll_device_change_ntf(zldpll->dpll_dev);
+
+	list_for_each_entry(pin, &zldpll->pins, list) {
+		if (zl3073x_dpll_is_input_pin(pin) &&
+		    test_bit(pin->id, changed_pins))
 			dpll_pin_change_ntf(pin->dpll_pin);
 	}
 }
@@ -1949,6 +2053,7 @@ zl3073x_dpll_alloc(struct zl3073x_dev *zldev, u8 ch)
 
 	zldpll->dev = zldev;
 	zldpll->id = ch;
+	mutex_init(&zldpll->lock);
 	INIT_LIST_HEAD(&zldpll->pins);
 
 	return zldpll;
@@ -1965,6 +2070,7 @@ zl3073x_dpll_free(struct zl3073x_dpll *zldpll)
 {
 	WARN(zldpll->dpll_dev, "DPLL device is still registered\n");
 
+	mutex_destroy(&zldpll->lock);
 	kfree(zldpll);
 }
 
diff --git a/drivers/dpll/zl3073x/dpll.h b/drivers/dpll/zl3073x/dpll.h
index 21adcc18e45e1..9f57c944a0077 100644
--- a/drivers/dpll/zl3073x/dpll.h
+++ b/drivers/dpll/zl3073x/dpll.h
@@ -18,6 +18,7 @@
  * @ops: DPLL device operations for this instance
  * @dpll_dev: pointer to registered DPLL device
  * @tracker: tracking object for the acquired reference
+ * @lock: per-DPLL mutex serializing all operations
  * @lock_status: last saved DPLL lock status
  * @pins: list of pins
  */
@@ -30,6 +31,7 @@ struct zl3073x_dpll {
 	struct dpll_device_ops		ops;
 	struct dpll_device		*dpll_dev;
 	dpll_tracker			tracker;
+	struct mutex			lock;
 	enum dpll_lock_status		lock_status;
 	struct list_head		pins;
 };
-- 
2.53.0


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

* [PATCH net-next v5 4/4] dpll: zl3073x: add NCO virtual input pin
  2026-05-31 19:44 [PATCH net-next v5 0/4] dpll: add NCO pin type and zl3073x support Ivan Vecera
                   ` (2 preceding siblings ...)
  2026-05-31 19:44 ` [PATCH net-next v5 3/4] dpll: zl3073x: add per-DPLL serialization lock Ivan Vecera
@ 2026-05-31 19:44 ` Ivan Vecera
  3 siblings, 0 replies; 8+ messages in thread
From: Ivan Vecera @ 2026-05-31 19:44 UTC (permalink / raw)
  To: netdev
  Cc: Petr Oros, Arkadiusz Kubalewski, David S. Miller, Donald Hunter,
	Eric Dumazet, Jakub Kicinski, Jiri Pirko, Michal Schmidt,
	Paolo Abeni, Pasi Vaananen, Prathosh Satish, Simon Horman,
	Vadim Fedorenko, linux-kernel

Add a virtual NCO (Numerically Controlled Oscillator) input pin that
lets userspace switch a DPLL channel into NCO mode. A single NCO pin
is shared across all DPLL channels - each channel has its own
independent connection state. The NCO pin is registered with the new
DPLL_PIN_TYPE_INT_NCO type and reports DPLL_PIN_STATE_CONNECTED /
DPLL_PIN_OPERSTATE_ACTIVE when the channel is in NCO mode.

At NCO pin registration the following bits are configured in
dpll_ctrl_x:
  - nco_auto_read: auto-capture tracking offset on NCO entry
  - tod_step_reset: apply negated ToD step accumulator on NCO exit
  - tie_clear: PPS DPLLs set 1 to re-align outputs on NCO exit,
               EEC DPLLs keep 0 to prevent an unwanted TIE write

Before switching to NCO mode, dpll_df_read_x is configured with
ref_ofst=0 and cmd=ACC_I so that nco_auto_read captures the
accumulated I-part offset relative to the master clock. Without
this, the captured df_offset would be near zero (offset relative
to the input reference after lock).

On NCO entry the df_offset captured by nco_auto_read is read from
the register. Per the datasheet, nco_auto_read only captures a valid
offset when entering NCO from reflock, auto or holdover mode; from
freerun the captured value is not meaningful and df_offset is marked as
ZL_DPLL_DF_OFFSET_UNKNOWN. The same sentinel is set in
chan_state_update() when the channel is not locked, and both FFO
consumers (NCO pin and input pin) guard against it.

Disconnecting the NCO pin switches to freerun rather than holdover
because holdover averaging is not updated during NCO mode.

When connecting the NCO pin displaces a previously connected input
pin (reflock mode), a change notification is sent for that input pin.

Input reference pins are now always registered regardless of the
initial DPLL mode. Previously they were skipped when the DPLL was
in NCO mode, but the NCO pin provides the proper mechanism for
mode transitions.

Reviewed-by: Petr Oros <poros@redhat.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
Changes:
v5:
   - Configure dpll_df_read register before NCO mode switch:
     ref_ofst=0 to read offset relative to master clock,
     cmd=ACC_I for accumulated I-part. Without this, nco_auto_read
     captures incorrect df_offset (reported by Chris du Quesnay).
v4:
   - Drop multiop_lock from chan_state_update() and nco_mode_set(),
     df_offset access is now serialized by the per-DPLL
     zldpll->lock introduced in the new lock patch.
   - Add zldpll->lock guard to all NCO pin callbacks for
     consistency with the lock patch.
   - Use mutex_lock/unlock in nco_pin_register,
     nco_pin_state_on_dpll_set, and input_pin_state_on_dpll_set
     instead of guard()/scoped_guard() to avoid mixing cleanup
     helpers with goto-based error handling.
   - Filter NCO pin in the deferred notification loop to match the
     monitoring loop filter.
   - Introduce ZL_DPLL_DF_OFFSET_UNKNOWN (S64_MIN) sentinel for
     df_offset: set on read failure, when entering NCO from a
     freerun mode, and when
     chan_state_update() finds the channel not locked. Guard both
     NCO pin and input pin FFO consumers against the sentinel.
   - Send __dpll_pin_change_ntf() for the displaced input pin when
     connecting the NCO pin from reflock mode.
   - Read df_offset from register at probe when firmware left the
     channel in NCO mode.
   - Add comment clarifying that nco_auto_read completes before the
     mode switch (specified by the datasheet and verified by
     HW testing).
   - Unify df_offset sign convention comments with datasheet
     reference (f_offset = f_nom * (-df_offset) / 2^48).
v3:
  - Fixed Signed-off-by position
v2:
  - Configure nco_auto_read, tod_step_reset and tie_clear once at
    NCO pin registration since these are persistent R/W bits.
    In v1 nco_auto_read was set at registration, while tod_step_reset
    and tie_clear were set on each NCO exit path.
  - Add zl3073x_chan_nco_mode_set() helper that writes mode_refsel
    directly and reads df_offset from the register without the
    DF_READ semaphore protocol. A short delay (~5 ms) is needed before
    nco_auto_read populates the df_offset register (determined
    by HW testing). In v1 the full DF_READ semaphore protocol with
    zl3073x_chan_state_update() was used.
  - Zero df_offset on read failure instead of keeping stale value.
  - Serialize zl3073x_chan_state_update() and
    zl3073x_chan_nco_mode_set() with multiop_lock to prevent
    concurrent df_offset access from the periodic worker.
  - Gate df_offset read in zl3073x_chan_state_update() on LOCK state
    instead of skipping NCO channels in chan_states_update(). This
    keeps mon_status and refsel_status fresh in all modes.
  - Send __dpll_pin_change_ntf() for the NCO pin when leaving NCO
    mode via mode_set() or input pin connect, since the periodic
    worker skips the NCO pin.
  - Add comments explaining the inverted sign convention of the
    dpll_df_offset register.
  - Document why NCO disconnect selects freerun over holdover, the
    shared NCO pin design, and the input pin registration change.
---
 drivers/dpll/zl3073x/chan.c | 118 +++++++++++++-
 drivers/dpll/zl3073x/chan.h |  48 ++++++
 drivers/dpll/zl3073x/dpll.c | 308 ++++++++++++++++++++++++++++++++----
 drivers/dpll/zl3073x/dpll.h |   2 +
 drivers/dpll/zl3073x/regs.h |  11 ++
 5 files changed, 458 insertions(+), 29 deletions(-)

diff --git a/drivers/dpll/zl3073x/chan.c b/drivers/dpll/zl3073x/chan.c
index 677a920c16254..1f0f904b57701 100644
--- a/drivers/dpll/zl3073x/chan.c
+++ b/drivers/dpll/zl3073x/chan.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 
 #include <linux/cleanup.h>
+#include <linux/delay.h>
 #include <linux/dev_printk.h>
 #include <linux/string.h>
 #include <linux/types.h>
@@ -31,7 +32,15 @@ int zl3073x_chan_state_update(struct zl3073x_dev *zldev, u8 index)
 	if (rc)
 		return rc;
 
-	/* Read df_offset vs tracked reference */
+	/* Read df_offset only when locked to a reference. In NCO mode
+	 * df_offset was captured at entry by nco_mode_set() - preserve it.
+	 */
+	if (!zl3073x_chan_is_locked(chan)) {
+		if (!zl3073x_chan_mode_is_nco(chan))
+			chan->df_offset = ZL_DPLL_DF_OFFSET_UNKNOWN;
+		return 0;
+	}
+
 	rc = zl3073x_poll_zero_u8(zldev, ZL_REG_DPLL_DF_READ(index),
 				  ZL_DPLL_DF_READ_SEM,
 				  ZL_POLL_DF_READ_TIMEOUT_US);
@@ -58,6 +67,92 @@ int zl3073x_chan_state_update(struct zl3073x_dev *zldev, u8 index)
 	return 0;
 }
 
+/**
+ * zl3073x_chan_nco_mode_set - switch DPLL channel to NCO mode
+ * @zldev: pointer to zl3073x_dev structure
+ * @index: DPLL channel index
+ *
+ * Switches the channel to NCO mode and reads the df_offset
+ * auto-captured by nco_auto_read directly from the register.
+ * No DF_READ handshake is needed as nco_auto_read populates
+ * the register before the mode switch completes.
+ *
+ * Return: 0 on success, <0 on error
+ */
+int zl3073x_chan_nco_mode_set(struct zl3073x_dev *zldev, u8 index)
+{
+	struct zl3073x_chan *chan = &zldev->chan[index];
+	u8 prev_mode, df_read;
+	u64 val;
+	int rc;
+
+	prev_mode = zl3073x_chan_mode_get(chan);
+
+	/* nco_auto_read captures the tracking offset at NCO entry only
+	 * from reflock, auto or holdover mode. From freerun the captured
+	 * value is not meaningful.
+	 */
+	if (prev_mode == ZL_DPLL_MODE_REFSEL_MODE_FREERUN) {
+		zl3073x_chan_mode_set(chan, ZL_DPLL_MODE_REFSEL_MODE_NCO);
+
+		rc = zl3073x_write_u8(zldev, ZL_REG_DPLL_MODE_REFSEL(index),
+				      chan->mode_refsel);
+		if (rc) {
+			zl3073x_chan_mode_set(chan, prev_mode);
+			return rc;
+		}
+
+		chan->df_offset = ZL_DPLL_DF_OFFSET_UNKNOWN;
+		return 0;
+	}
+
+	/* Configure df_read for nco_auto_read:
+	 * ref_ofst=0 - reads offset relative to master clock (not input ref)
+	 * cmd=CMD_ACC_I - accumulated I-part covering both locked and
+	 *                 holdover entry.
+	 *
+	 * No semaphore is set - this only configures what the df_offset
+	 * value represents after the mode switch; nco_auto_read performs
+	 * the actual read automatically.
+	 */
+	df_read = FIELD_PREP(ZL_DPLL_DF_READ_REF_OFST, 0) |
+		  FIELD_PREP(ZL_DPLL_DF_READ_CMD, ZL_DPLL_DF_READ_CMD_ACC_I);
+	rc = zl3073x_write_u8(zldev, ZL_REG_DPLL_DF_READ(index), df_read);
+	if (rc)
+		return rc;
+
+	zl3073x_chan_mode_set(chan, ZL_DPLL_MODE_REFSEL_MODE_NCO);
+	rc = zl3073x_write_u8(zldev, ZL_REG_DPLL_MODE_REFSEL(index),
+			      chan->mode_refsel);
+	if (rc) {
+		zl3073x_chan_mode_set(chan, prev_mode);
+		return rc;
+	}
+
+	/* Wait for nco_auto_read to populate df_offset. The datasheet
+	 * does not specify a delay but HW testing shows ~3 ms is needed
+	 * before the register contains the captured value. Use 5 ms to
+	 * provide margin.
+	 */
+	fsleep(5000);
+
+	/* Read df_offset captured by nco_auto_read during mode switch.
+	 * No DF_READ semaphore handshake needed. Mode switch already
+	 * succeeded, so don't propagate a read failure back to userspace.
+	 */
+	rc = zl3073x_read_u48(zldev, ZL_REG_DPLL_DF_OFFSET(index), &val);
+	if (rc) {
+		dev_warn(zldev->dev,
+			 "Failed to read DPLL%u df_offset: %pe\n",
+			 index, ERR_PTR(rc));
+		chan->df_offset = ZL_DPLL_DF_OFFSET_UNKNOWN;
+	} else {
+		chan->df_offset = sign_extend64(val, 47);
+	}
+
+	return 0;
+}
+
 /**
  * zl3073x_chan_state_fetch - fetch DPLL channel state from hardware
  * @zldev: pointer to zl3073x_dev structure
@@ -73,6 +168,10 @@ int zl3073x_chan_state_fetch(struct zl3073x_dev *zldev, u8 index)
 	struct zl3073x_chan *chan = &zldev->chan[index];
 	int rc, i;
 
+	rc = zl3073x_read_u8(zldev, ZL_REG_DPLL_CTRL(index), &chan->ctrl);
+	if (rc)
+		return rc;
+
 	rc = zl3073x_read_u8(zldev, ZL_REG_DPLL_MODE_REFSEL(index),
 			     &chan->mode_refsel);
 	if (rc)
@@ -85,6 +184,13 @@ int zl3073x_chan_state_fetch(struct zl3073x_dev *zldev, u8 index)
 	if (rc)
 		return rc;
 
+	/* If firmware left the channel in NCO mode, mark df_offset as
+	 * unknown - we cannot know whether the preconditions for a valid
+	 * nco_auto_read capture were met.
+	 */
+	if (zl3073x_chan_mode_is_nco(chan))
+		chan->df_offset = ZL_DPLL_DF_OFFSET_UNKNOWN;
+
 	dev_dbg(zldev->dev,
 		"DPLL%u lock_state: %u, ho: %u, sel_state: %u, sel_ref: %u\n",
 		index, zl3073x_chan_lock_state_get(chan),
@@ -147,7 +253,15 @@ int zl3073x_chan_state_set(struct zl3073x_dev *zldev, u8 index,
 	if (!memcmp(&dchan->cfg, &chan->cfg, sizeof(chan->cfg)))
 		return 0;
 
-	/* Direct register write for mode_refsel */
+	/* Direct register writes for ctrl and mode_refsel */
+	if (dchan->ctrl != chan->ctrl) {
+		rc = zl3073x_write_u8(zldev, ZL_REG_DPLL_CTRL(index),
+				      chan->ctrl);
+		if (rc)
+			return rc;
+		dchan->ctrl = chan->ctrl;
+	}
+
 	if (dchan->mode_refsel != chan->mode_refsel) {
 		rc = zl3073x_write_u8(zldev, ZL_REG_DPLL_MODE_REFSEL(index),
 				      chan->mode_refsel);
diff --git a/drivers/dpll/zl3073x/chan.h b/drivers/dpll/zl3073x/chan.h
index 4353809c69122..dc9c6d95bdee7 100644
--- a/drivers/dpll/zl3073x/chan.h
+++ b/drivers/dpll/zl3073x/chan.h
@@ -13,6 +13,7 @@ struct zl3073x_dev;
 
 /**
  * struct zl3073x_chan - DPLL channel state
+ * @ctrl: DPLL control register value
  * @mode_refsel: mode and reference selection register value
  * @ref_prio: reference priority registers (4 bits per ref, P/N packed)
  * @mon_status: monitor status register value
@@ -21,6 +22,7 @@ struct zl3073x_dev;
  */
 struct zl3073x_chan {
 	struct_group(cfg,
+		u8	ctrl;
 		u8	mode_refsel;
 		u8	ref_prio[ZL3073X_NUM_REFS / 2];
 	);
@@ -38,6 +40,7 @@ int zl3073x_chan_state_set(struct zl3073x_dev *zldev, u8 index,
 			   const struct zl3073x_chan *chan);
 
 int zl3073x_chan_state_update(struct zl3073x_dev *zldev, u8 index);
+int zl3073x_chan_nco_mode_set(struct zl3073x_dev *zldev, u8 index);
 
 /**
  * zl3073x_chan_df_offset_get - get cached df_offset vs tracked reference
@@ -152,6 +155,51 @@ static inline u8 zl3073x_chan_lock_state_get(const struct zl3073x_chan *chan)
 	return FIELD_GET(ZL_DPLL_MON_STATUS_STATE, chan->mon_status);
 }
 
+/**
+ * zl3073x_chan_is_locked - check if channel is locked to a reference
+ * @chan: pointer to channel state
+ *
+ * Return: true if channel is locked, false otherwise
+ */
+static inline bool zl3073x_chan_is_locked(const struct zl3073x_chan *chan)
+{
+	u8 lock_state = zl3073x_chan_lock_state_get(chan);
+	return lock_state == ZL_DPLL_MON_STATUS_STATE_LOCK;
+}
+
+/**
+ * zl3073x_chan_mode_is_auto - check if channel is in automatic mode
+ * @chan: pointer to channel state
+ *
+ * Return: true if channel is in automatic mode, false otherwise
+ */
+static inline bool zl3073x_chan_mode_is_auto(const struct zl3073x_chan *chan)
+{
+	return zl3073x_chan_mode_get(chan) == ZL_DPLL_MODE_REFSEL_MODE_AUTO;
+}
+
+/**
+ * zl3073x_chan_mode_is_nco - check if channel is in NCO mode
+ * @chan: pointer to channel state
+ *
+ * Return: true if channel is in NCO mode, false otherwise
+ */
+static inline bool zl3073x_chan_mode_is_nco(const struct zl3073x_chan *chan)
+{
+	return zl3073x_chan_mode_get(chan) == ZL_DPLL_MODE_REFSEL_MODE_NCO;
+}
+
+/**
+ * zl3073x_chan_mode_is_reflock - check if channel is in reflock mode
+ * @chan: pointer to channel state
+ *
+ * Return: true if channel is in reflock mode, false otherwise
+ */
+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_is_ho_ready - check if holdover is ready
  * @chan: pointer to channel state
diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
index 4bee3d0c46593..8c39cdb8f5723 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -80,6 +80,18 @@ zl3073x_dpll_is_input_pin(struct zl3073x_dpll_pin *pin)
 	return pin->dir == DPLL_PIN_DIRECTION_INPUT;
 }
 
+/**
+ * zl3073x_dpll_is_nco_pin - check if the pin is a virtual NCO pin
+ * @pin: pin to check
+ *
+ * Return: true if pin is a virtual NCO pin, false otherwise.
+ */
+static bool
+zl3073x_dpll_is_nco_pin(struct zl3073x_dpll_pin *pin)
+{
+	return pin->id == ZL3073X_NCO_PIN_ID;
+}
+
 /**
  * zl3073x_dpll_is_p_pin - check if the pin is P-pin
  * @pin: pin to check
@@ -119,6 +131,19 @@ zl3073x_dpll_pin_get_by_ref(struct zl3073x_dpll *zldpll, u8 ref_id)
 	return NULL;
 }
 
+static struct zl3073x_dpll_pin *
+zl3073x_dpll_nco_pin_get(struct zl3073x_dpll *zldpll)
+{
+	struct zl3073x_dpll_pin *pin;
+
+	list_for_each_entry(pin, &zldpll->pins, list) {
+		if (zl3073x_dpll_is_nco_pin(pin))
+			return pin;
+	}
+
+	return NULL;
+}
+
 static int
 zl3073x_dpll_input_pin_esync_get(const struct dpll_pin *dpll_pin,
 				 void *pin_priv,
@@ -635,6 +660,7 @@ zl3073x_dpll_input_pin_state_on_dpll_set(const struct dpll_pin *dpll_pin,
 {
 	struct zl3073x_dpll *zldpll = dpll_priv;
 	struct zl3073x_dpll_pin *pin = pin_priv;
+	struct zl3073x_dpll_pin *nco_pin = NULL;
 	struct zl3073x_chan chan;
 	u8 mode, ref;
 	int rc = 0;
@@ -666,6 +692,10 @@ zl3073x_dpll_input_pin_state_on_dpll_set(const struct dpll_pin *dpll_pin,
 			goto invalid_state;
 		}
 		break;
+	case ZL_DPLL_MODE_REFSEL_MODE_NCO:
+		if (state == DPLL_PIN_STATE_CONNECTED)
+			nco_pin = zl3073x_dpll_nco_pin_get(zldpll);
+		fallthrough;
 	case ZL_DPLL_MODE_REFSEL_MODE_FREERUN:
 	case ZL_DPLL_MODE_REFSEL_MODE_HOLDOVER:
 		if (state == DPLL_PIN_STATE_CONNECTED) {
@@ -713,6 +743,13 @@ zl3073x_dpll_input_pin_state_on_dpll_set(const struct dpll_pin *dpll_pin,
 	rc = -EINVAL;
 unlock:
 	mutex_unlock(&zldpll->lock);
+
+	/* If leaving NCO mode, notify userspace about the NCO pin
+	 * state change - the periodic worker skips the NCO pin.
+	 */
+	if (!rc && nco_pin)
+		__dpll_pin_change_ntf(nco_pin->dpll_pin);
+
 	return rc;
 }
 
@@ -1039,6 +1076,144 @@ zl3073x_dpll_output_pin_state_on_dpll_get(const struct dpll_pin *dpll_pin,
 	return 0;
 }
 
+static int
+zl3073x_dpll_nco_pin_operstate_on_dpll_get(const struct dpll_pin *dpll_pin,
+					   void *pin_priv,
+					   const struct dpll_device *dpll,
+					   void *dpll_priv,
+					   enum dpll_pin_operstate *operstate,
+					   struct netlink_ext_ack *extack)
+{
+	struct zl3073x_dpll *zldpll = dpll_priv;
+	const struct zl3073x_chan *chan;
+
+	guard(mutex)(&zldpll->lock);
+
+	chan = zl3073x_chan_state_get(zldpll->dev, zldpll->id);
+	if (zl3073x_chan_mode_is_nco(chan))
+		*operstate = DPLL_PIN_OPERSTATE_ACTIVE;
+	else
+		*operstate = DPLL_PIN_OPERSTATE_STANDBY;
+
+	return 0;
+}
+
+static int
+zl3073x_dpll_nco_pin_state_on_dpll_get(const struct dpll_pin *dpll_pin,
+				       void *pin_priv,
+				       const struct dpll_device *dpll,
+				       void *dpll_priv,
+				       enum dpll_pin_state *state,
+				       struct netlink_ext_ack *extack)
+{
+	struct zl3073x_dpll *zldpll = dpll_priv;
+	const struct zl3073x_chan *chan;
+
+	guard(mutex)(&zldpll->lock);
+
+	chan = zl3073x_chan_state_get(zldpll->dev, zldpll->id);
+	if (zl3073x_chan_mode_is_nco(chan))
+		*state = DPLL_PIN_STATE_CONNECTED;
+	else
+		*state = DPLL_PIN_STATE_DISCONNECTED;
+
+	return 0;
+}
+
+static int
+zl3073x_dpll_nco_pin_state_on_dpll_set(const struct dpll_pin *dpll_pin,
+				       void *pin_priv,
+				       const struct dpll_device *dpll,
+				       void *dpll_priv,
+				       enum dpll_pin_state state,
+				       struct netlink_ext_ack *extack)
+{
+	struct zl3073x_dpll_pin *ref_pin = NULL;
+	struct zl3073x_dpll *zldpll = dpll_priv;
+	struct zl3073x_chan chan;
+	u8 ref;
+	int rc;
+
+	mutex_lock(&zldpll->lock);
+
+	chan = *zl3073x_chan_state_get(zldpll->dev, zldpll->id);
+
+	switch (state) {
+	case DPLL_PIN_STATE_CONNECTED:
+		if (zl3073x_chan_mode_is_nco(&chan)) {
+			mutex_unlock(&zldpll->lock);
+			return 0;
+		}
+		if (zl3073x_chan_mode_is_auto(&chan)) {
+			NL_SET_ERR_MSG(extack,
+				       "NCO pin cannot be connected in automatic mode");
+			mutex_unlock(&zldpll->lock);
+			return -EINVAL;
+		}
+		if (zl3073x_chan_mode_is_reflock(&chan)) {
+			/* Get currently connected pin */
+			ref = zl3073x_chan_ref_get(&chan);
+			ref_pin = zl3073x_dpll_pin_get_by_ref(zldpll, ref);
+		}
+		rc = zl3073x_chan_nco_mode_set(zldpll->dev, zldpll->id);
+		break;
+	case DPLL_PIN_STATE_DISCONNECTED:
+		if (!zl3073x_chan_mode_is_nco(&chan)) {
+			mutex_unlock(&zldpll->lock);
+			return 0;
+		}
+		/* Switch to freerun - holdover averaging was not
+		 * updated during NCO mode.
+		 */
+		zl3073x_chan_mode_set(&chan,
+				      ZL_DPLL_MODE_REFSEL_MODE_FREERUN);
+		rc = zl3073x_chan_state_set(zldpll->dev, zldpll->id, &chan);
+		break;
+	default:
+		NL_SET_ERR_MSG(extack, "invalid pin state for NCO pin");
+		mutex_unlock(&zldpll->lock);
+		return -EINVAL;
+	}
+
+	mutex_unlock(&zldpll->lock);
+
+	if (!rc && ref_pin)
+		__dpll_pin_change_ntf(ref_pin->dpll_pin);
+
+	return rc;
+}
+
+static int
+zl3073x_dpll_nco_pin_ffo_get(const struct dpll_pin *dpll_pin, void *pin_priv,
+			     const struct dpll_device *dpll, void *dpll_priv,
+			     struct dpll_ffo_param *ffo,
+			     struct netlink_ext_ack *extack)
+{
+	struct zl3073x_dpll *zldpll = dpll_priv;
+	const struct zl3073x_chan *chan;
+	s64 df_offset;
+
+	guard(mutex)(&zldpll->lock);
+
+	chan = zl3073x_chan_state_get(zldpll->dev, zldpll->id);
+	if (!zl3073x_chan_mode_is_nco(chan))
+		return -ENODATA;
+
+	/* Do not report FFO if a failure occurred during switching to NCO. */
+	df_offset = zl3073x_chan_df_offset_get(chan);
+	if (df_offset == ZL_DPLL_DF_OFFSET_UNKNOWN)
+		return -ENODATA;
+
+	/* dpll_df_offset register has inverted sign per datasheet:
+	 * f_offset = f_nom * (-df_offset) / 2^48
+	 * NCO pin reports DPLL output offset from nominal, so negate.
+	 * Convert to PPT: ppt = -df * 5^12 / 2^36
+	 */
+	ffo->ffo = -mul_s64_u64_shr(df_offset, 244140625, 36);
+
+	return 0;
+}
+
 static int
 zl3073x_dpll_temp_get(const struct dpll_device *dpll, void *dpll_priv,
 		      s32 *temp, struct netlink_ext_ack *extack)
@@ -1121,21 +1296,7 @@ zl3073x_dpll_supported_modes_get(const struct dpll_device *dpll,
 				 void *dpll_priv, unsigned long *modes,
 				 struct netlink_ext_ack *extack)
 {
-	struct zl3073x_dpll *zldpll = dpll_priv;
-	const struct zl3073x_chan *chan;
-
-	guard(mutex)(&zldpll->lock);
-
-	chan = zl3073x_chan_state_get(zldpll->dev, zldpll->id);
-
-	/* We support switching between automatic and manual mode, except in
-	 * a case where the DPLL channel is configured to run in NCO mode.
-	 * In this case, report only the manual mode to which the NCO is mapped
-	 * as the only supported one.
-	 */
-	if (zl3073x_chan_mode_get(chan) != ZL_DPLL_MODE_REFSEL_MODE_NCO)
-		__set_bit(DPLL_MODE_AUTOMATIC, modes);
-
+	__set_bit(DPLL_MODE_AUTOMATIC, modes);
 	__set_bit(DPLL_MODE_MANUAL, modes);
 
 	return 0;
@@ -1232,11 +1393,12 @@ zl3073x_dpll_mode_set(const struct dpll_device *dpll, void *dpll_priv,
 		      enum dpll_mode mode, struct netlink_ext_ack *extack)
 {
 	struct zl3073x_dpll *zldpll = dpll_priv;
+	struct zl3073x_dpll_pin *nco_pin = NULL;
 	struct zl3073x_chan chan;
 	u8 hw_mode, ref;
 	int rc;
 
-	guard(mutex)(&zldpll->lock);
+	mutex_lock(&zldpll->lock);
 
 	chan = *zl3073x_chan_state_get(zldpll->dev, zldpll->id);
 	ref = zl3073x_chan_refsel_ref_get(&chan);
@@ -1257,6 +1419,9 @@ zl3073x_dpll_mode_set(const struct dpll_device *dpll, void *dpll_priv,
 		else
 			hw_mode = ZL_DPLL_MODE_REFSEL_MODE_HOLDOVER;
 	} else {
+		if (zl3073x_chan_mode_is_nco(&chan))
+			nco_pin = zl3073x_dpll_nco_pin_get(zldpll);
+
 		/* We are switching from manual to automatic mode:
 		 * - if there is a valid reference selected then ensure that
 		 *   it is selectable after switch to automatic mode
@@ -1285,9 +1450,18 @@ zl3073x_dpll_mode_set(const struct dpll_device *dpll, void *dpll_priv,
 	if (rc) {
 		NL_SET_ERR_MSG_MOD(extack,
 				   "failed to set reference selection mode");
+		mutex_unlock(&zldpll->lock);
 		return rc;
 	}
 
+	mutex_unlock(&zldpll->lock);
+
+	/* If leaving NCO mode, notify userspace about the NCO pin
+	 * state change - the periodic worker skips the NCO pin.
+	 */
+	if (nco_pin)
+		__dpll_pin_change_ntf(nco_pin->dpll_pin);
+
 	return 0;
 }
 
@@ -1395,6 +1569,15 @@ static const struct dpll_pin_ops zl3073x_dpll_output_pin_ops = {
 	.state_on_dpll_get = zl3073x_dpll_output_pin_state_on_dpll_get,
 };
 
+static const struct dpll_pin_ops zl3073x_dpll_nco_pin_ops = {
+	.supported_ffo = BIT(DPLL_FFO_PIN_DEVICE),
+	.direction_get = zl3073x_dpll_pin_direction_get,
+	.ffo_get = zl3073x_dpll_nco_pin_ffo_get,
+	.operstate_on_dpll_get = zl3073x_dpll_nco_pin_operstate_on_dpll_get,
+	.state_on_dpll_get = zl3073x_dpll_nco_pin_state_on_dpll_get,
+	.state_on_dpll_set = zl3073x_dpll_nco_pin_state_on_dpll_set,
+};
+
 static const struct dpll_device_ops zl3073x_dpll_device_ops = {
 	.lock_status_get = zl3073x_dpll_lock_status_get,
 	.mode_get = zl3073x_dpll_mode_get,
@@ -1542,7 +1725,9 @@ zl3073x_dpll_pin_unregister(struct zl3073x_dpll_pin *pin)
 
 	WARN(!pin->dpll_pin, "DPLL pin is not registered\n");
 
-	if (zl3073x_dpll_is_input_pin(pin))
+	if (zl3073x_dpll_is_nco_pin(pin))
+		ops = &zl3073x_dpll_nco_pin_ops;
+	else if (zl3073x_dpll_is_input_pin(pin))
 		ops = &zl3073x_dpll_input_pin_ops;
 	else
 		ops = &zl3073x_dpll_output_pin_ops;
@@ -1595,20 +1780,13 @@ zl3073x_dpll_pin_is_registrable(struct zl3073x_dpll *zldpll,
 				enum dpll_pin_direction dir, u8 index)
 {
 	struct zl3073x_dev *zldev = zldpll->dev;
-	const struct zl3073x_chan *chan;
 	bool is_diff, is_enabled;
 	const char *name;
 
-	chan = zl3073x_chan_state_get(zldev, zldpll->id);
-
 	if (dir == DPLL_PIN_DIRECTION_INPUT) {
 		u8 ref_id = zl3073x_input_pin_ref_get(index);
 		const struct zl3073x_ref *ref;
 
-		/* Skip the pin if the DPLL is running in NCO mode */
-		if (zl3073x_chan_mode_get(chan) == ZL_DPLL_MODE_REFSEL_MODE_NCO)
-			return false;
-
 		name = "REF";
 		ref = zl3073x_ref_state_get(zldev, ref_id);
 		is_diff = zl3073x_ref_is_diff(ref);
@@ -1649,6 +1827,66 @@ zl3073x_dpll_pin_is_registrable(struct zl3073x_dpll *zldpll,
 	return true;
 }
 
+static const struct dpll_pin_properties zl3073x_dpll_nco_pin_props = {
+	.type = DPLL_PIN_TYPE_INT_NCO,
+	.package_label = "NCO",
+	.capabilities = DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE,
+};
+
+static int
+zl3073x_dpll_nco_pin_register(struct zl3073x_dpll *zldpll)
+{
+	struct zl3073x_dpll_pin *pin;
+	struct zl3073x_chan chan;
+	int rc;
+
+	/* Ensure that ctrl bits are configured for NCO operation:
+	 * - nco_auto_read: auto-capture tracking offset on NCO entry
+	 * - tod_step_reset: apply negated ToD step on NCO exit
+	 * - tie_clear: PPS DPLLs re-align outputs on NCO exit
+	 */
+	mutex_lock(&zldpll->lock);
+	chan = *zl3073x_chan_state_get(zldpll->dev, zldpll->id);
+	FIELD_MODIFY(ZL_DPLL_CTRL_NCO_AUTO_READ, &chan.ctrl, 1);
+	FIELD_MODIFY(ZL_DPLL_CTRL_TOD_STEP_RST, &chan.ctrl, 1);
+	FIELD_MODIFY(ZL_DPLL_CTRL_TIE_CLEAR, &chan.ctrl,
+		     zldpll->type == DPLL_TYPE_PPS ? 1 : 0);
+	rc = zl3073x_chan_state_set(zldpll->dev, zldpll->id, &chan);
+	mutex_unlock(&zldpll->lock);
+	if (rc)
+		return rc;
+
+	pin = zl3073x_dpll_pin_alloc(zldpll, DPLL_PIN_DIRECTION_INPUT,
+				     ZL3073X_NCO_PIN_ID);
+	if (IS_ERR(pin))
+		return PTR_ERR(pin);
+
+	pin->dpll_pin = dpll_pin_get(zldpll->dev->clock_id, ZL3073X_NCO_PIN_ID,
+				     THIS_MODULE, &zl3073x_dpll_nco_pin_props,
+				     &pin->tracker);
+	if (IS_ERR(pin->dpll_pin)) {
+		rc = PTR_ERR(pin->dpll_pin);
+		goto err_pin_get;
+	}
+
+	rc = dpll_pin_register(zldpll->dpll_dev, pin->dpll_pin,
+			       &zl3073x_dpll_nco_pin_ops, pin);
+	if (rc)
+		goto err_register;
+
+	list_add(&pin->list, &zldpll->pins);
+
+	return 0;
+
+err_register:
+	dpll_pin_put(pin->dpll_pin, &pin->tracker);
+err_pin_get:
+	pin->dpll_pin = NULL;
+	kfree(pin);
+
+	return rc;
+}
+
 /**
  * zl3073x_dpll_pins_register - register all registerable DPLL pins
  * @zldpll: pointer to zl3073x_dpll structure
@@ -1696,6 +1934,11 @@ zl3073x_dpll_pins_register(struct zl3073x_dpll *zldpll)
 		list_add(&pin->list, &zldpll->pins);
 	}
 
+	/* Register NCO virtual input pin */
+	rc = zl3073x_dpll_nco_pin_register(zldpll);
+	if (rc)
+		goto error;
+
 	return 0;
 
 error:
@@ -1731,8 +1974,8 @@ zl3073x_dpll_device_register(struct zl3073x_dpll *zldpll)
 		return rc;
 	}
 
-	rc = dpll_device_register(zldpll->dpll_dev,
-				  zl3073x_prop_dpll_type_get(zldev, zldpll->id),
+	zldpll->type = zl3073x_prop_dpll_type_get(zldev, zldpll->id);
+	rc = dpll_device_register(zldpll->dpll_dev, zldpll->type,
 				  &zldpll->ops, zldpll);
 	if (rc) {
 		dpll_device_put(zldpll->dpll_dev, &zldpll->tracker);
@@ -1843,6 +2086,14 @@ zl3073x_dpll_pin_ffo_check(struct zl3073x_dpll_pin *pin)
 		return false;
 
 	chan = zl3073x_chan_state_get(zldpll->dev, zldpll->id);
+	if (zl3073x_chan_df_offset_get(chan) == ZL_DPLL_DF_OFFSET_UNKNOWN)
+		return false;
+
+	/* dpll_df_offset register has inverted sign per datasheet:
+	 * f_offset = f_nom * (-df_offset) / 2^48
+	 * Input pin FFO is pin-vs-DPLL (opposite of DPLL-vs-reference),
+	 * so the two inversions cancel out: ppt = df * 5^12 / 2^36
+	 */
 	ffo = mul_s64_u64_shr(zl3073x_chan_df_offset_get(chan),
 			      244140625, 36);
 
@@ -1955,7 +2206,9 @@ zl3073x_dpll_changes_check(struct zl3073x_dpll *zldpll)
 	list_for_each_entry(pin, &zldpll->pins, list) {
 		enum dpll_pin_operstate operstate;
 
-		if (!zl3073x_dpll_is_input_pin(pin))
+		/* Only physical input pins need monitoring */
+		if (!zl3073x_dpll_is_input_pin(pin) ||
+		    zl3073x_dpll_is_nco_pin(pin))
 			continue;
 
 		rc = zl3073x_dpll_ref_operstate_get(pin, &operstate);
@@ -1996,6 +2249,7 @@ zl3073x_dpll_changes_check(struct zl3073x_dpll *zldpll)
 
 	list_for_each_entry(pin, &zldpll->pins, list) {
 		if (zl3073x_dpll_is_input_pin(pin) &&
+		    !zl3073x_dpll_is_nco_pin(pin) &&
 		    test_bit(pin->id, changed_pins))
 			dpll_pin_change_ntf(pin->dpll_pin);
 	}
diff --git a/drivers/dpll/zl3073x/dpll.h b/drivers/dpll/zl3073x/dpll.h
index 9f57c944a0077..faebc402ba1b7 100644
--- a/drivers/dpll/zl3073x/dpll.h
+++ b/drivers/dpll/zl3073x/dpll.h
@@ -19,6 +19,7 @@
  * @dpll_dev: pointer to registered DPLL device
  * @tracker: tracking object for the acquired reference
  * @lock: per-DPLL mutex serializing all operations
+ * @type: DPLL type (PPS or EEC)
  * @lock_status: last saved DPLL lock status
  * @pins: list of pins
  */
@@ -32,6 +33,7 @@ struct zl3073x_dpll {
 	struct dpll_device		*dpll_dev;
 	dpll_tracker			tracker;
 	struct mutex			lock;
+	enum dpll_type			type;
 	enum dpll_lock_status		lock_status;
 	struct list_head		pins;
 };
diff --git a/drivers/dpll/zl3073x/regs.h b/drivers/dpll/zl3073x/regs.h
index 9578f00095282..b70ead7d4495b 100644
--- a/drivers/dpll/zl3073x/regs.h
+++ b/drivers/dpll/zl3073x/regs.h
@@ -5,6 +5,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/bits.h>
+#include <linux/limits.h>
 
 /*
  * Hardware limits for ZL3073x chip family
@@ -17,6 +18,7 @@
 #define ZL3073X_NUM_OUTPUT_PINS	(ZL3073X_NUM_OUTS * 2)
 #define ZL3073X_NUM_PINS	(ZL3073X_NUM_INPUT_PINS + \
 				 ZL3073X_NUM_OUTPUT_PINS)
+#define ZL3073X_NCO_PIN_ID	ZL3073X_NUM_PINS
 
 /*
  * Register address structure:
@@ -164,10 +166,18 @@
 #define ZL_DPLL_MODE_REFSEL_MODE_NCO		4
 #define ZL_DPLL_MODE_REFSEL_REF			GENMASK(7, 4)
 
+#define ZL_REG_DPLL_CTRL(_idx)						\
+	ZL_REG_IDX(_idx, 5, 0x05, 1, ZL3073X_MAX_CHANNELS, 4)
+#define ZL_DPLL_CTRL_TIE_CLEAR			BIT(0)
+#define ZL_DPLL_CTRL_TOD_STEP_RST		BIT(2)
+#define ZL_DPLL_CTRL_NCO_AUTO_READ		BIT(7)
+
 #define ZL_REG_DPLL_DF_READ(_idx)					\
 	ZL_REG_IDX(_idx, 5, 0x28, 1, ZL3073X_MAX_CHANNELS, 1)
 #define ZL_DPLL_DF_READ_SEM			BIT(4)
 #define ZL_DPLL_DF_READ_REF_OFST		BIT(3)
+#define ZL_DPLL_DF_READ_CMD			GENMASK(2, 0)
+#define ZL_DPLL_DF_READ_CMD_ACC_I		4
 
 #define ZL_REG_DPLL_MEAS_CTRL			ZL_REG(5, 0x50, 1)
 #define ZL_DPLL_MEAS_CTRL_EN			BIT(0)
@@ -190,6 +200,7 @@
 #define ZL_REG_DPLL_DF_OFFSET_4		ZL_REG(7, 0x00, 6)
 #define ZL_REG_DPLL_DF_OFFSET(_idx)					\
 	((_idx) < 4 ? ZL_REG_DPLL_DF_OFFSET_03(_idx) : ZL_REG_DPLL_DF_OFFSET_4)
+#define ZL_DPLL_DF_OFFSET_UNKNOWN	S64_MIN
 
 /***********************************
  * Register Page 9, Synth and Output
-- 
2.53.0


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

* Re: [PATCH net-next v5 3/4] dpll: zl3073x: add per-DPLL serialization lock
  2026-05-31 19:44 ` [PATCH net-next v5 3/4] dpll: zl3073x: add per-DPLL serialization lock Ivan Vecera
@ 2026-06-04  1:40   ` Jakub Kicinski
  2026-06-04  1:51   ` Jakub Kicinski
  1 sibling, 0 replies; 8+ messages in thread
From: Jakub Kicinski @ 2026-06-04  1:40 UTC (permalink / raw)
  To: Ivan Vecera
  Cc: netdev, Arkadiusz Kubalewski, David S. Miller, Donald Hunter,
	Eric Dumazet, Jiri Pirko, Michal Schmidt, Paolo Abeni,
	Pasi Vaananen, Petr Oros, Prathosh Satish, Simon Horman,
	Vadim Fedorenko, linux-kernel

On Sun, 31 May 2026 21:44:22 +0200 Ivan Vecera wrote:
> Add a per-DPLL mutex that serializes all operations on a given DPLL
> channel across DPLL netlink callbacks, the periodic kthread worker,

Sounds like there are already two entities accessing the state?
If so why is this not a fix?

> and (in subsequent patches) PTP clock callbacks.

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

* Re: [PATCH net-next v5 1/4] dpll: add DPLL_PIN_TYPE_INT_NCO pin type
  2026-05-31 19:44 ` [PATCH net-next v5 1/4] dpll: add DPLL_PIN_TYPE_INT_NCO pin type Ivan Vecera
@ 2026-06-04  1:50   ` Jakub Kicinski
  0 siblings, 0 replies; 8+ messages in thread
From: Jakub Kicinski @ 2026-06-04  1:50 UTC (permalink / raw)
  To: Arkadiusz Kubalewski, Vadim Fedorenko
  Cc: Ivan Vecera, netdev, Jiri Pirko, David S. Miller, Donald Hunter,
	Eric Dumazet, Jiri Pirko, Michal Schmidt, Paolo Abeni,
	Pasi Vaananen, Petr Oros, Prathosh Satish, Simon Horman,
	linux-kernel, Grzegorz Nitka

On Sun, 31 May 2026 21:44:20 +0200 Ivan Vecera wrote:
> Add DPLL_PIN_TYPE_INT_NCO pin type for virtual pins representing
> the NCO mode of a DPLL. When connected as a DPLL input, the DPLL
> enters NCO mode where the output frequency is adjusted by the host
> via the PTP clock interface.
> 
> Update the fractional-frequency-offset and fractional-frequency-
> offset-ppt attribute documentation to note that for INT_NCO pins
> these attributes represent the DPLL's current output frequency
> offset from its nominal frequency.
> 
> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>

Purely going on intuition here but feels like NCO should be a mode
(enum dpll_mode) rather than one of the input pins?

More acks here would be great, Vadim, Arkadiusz, Grzegorz... ?

> v2:
>   - Clarify int-nco pin type documentation to describe frequency
>     control via the PTP clock interface instead of generic "controlled
>     by the host".
>   - Tighten FFO attribute documentation for INT_NCO pins to describe
>     the DPLL's output frequency offset from nominal frequency.
>   - Mention both fractional-frequency-offset (PPM) and
>     fractional-frequency-offset-ppt attributes in the commit message.
> ---
>  Documentation/netlink/specs/dpll.yaml | 13 +++++++++++++
>  drivers/dpll/dpll_nl.c                |  2 +-
>  include/uapi/linux/dpll.h             |  4 ++++
>  3 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/netlink/specs/dpll.yaml b/Documentation/netlink/specs/dpll.yaml
> index 91a172617b3a9..5cdb93e8649a0 100644
> --- a/Documentation/netlink/specs/dpll.yaml
> +++ b/Documentation/netlink/specs/dpll.yaml
> @@ -162,6 +162,13 @@ definitions:
>        -
>          name: gnss
>          doc: GNSS recovered clock
> +      -
> +        name: int-nco
> +        doc: |
> +          Device internal numerically controlled oscillator.
> +          When connected as a DPLL input, the DPLL enters NCO mode
> +          where the output frequency is adjusted by the host via
> +          the PTP clock interface.
>      render-max: true
>    -
>      type: enum
> @@ -453,6 +460,9 @@ attribute-sets:
>            offset on the media associated with the pin. Inside
>            the pin-parent-device nest it represents the frequency
>            offset between the pin and its parent DPLL device.
> +          For pins of type PIN_TYPE_INT_NCO this represents
> +          the DPLL's current output frequency offset from its
> +          nominal frequency.
>            Value is in PPM (parts per million).
>            This is a lower-precision version of
>            fractional-frequency-offset-ppt.
> @@ -499,6 +509,9 @@ attribute-sets:
>            offset on the media associated with the pin. Inside
>            the pin-parent-device nest it represents the frequency
>            offset between the pin and its parent DPLL device.
> +          For pins of type PIN_TYPE_INT_NCO this represents
> +          the DPLL's current output frequency offset from its
> +          nominal frequency.
>            Value is in PPT (parts per trillion, 10^-12).
>            This is a higher-precision version of
>            fractional-frequency-offset.
> diff --git a/drivers/dpll/dpll_nl.c b/drivers/dpll/dpll_nl.c
> index b1d9182c7802f..2dab99202764f 100644
> --- a/drivers/dpll/dpll_nl.c
> +++ b/drivers/dpll/dpll_nl.c
> @@ -61,7 +61,7 @@ static const struct nla_policy dpll_pin_id_get_nl_policy[DPLL_A_PIN_TYPE + 1] =
>  	[DPLL_A_PIN_BOARD_LABEL] = { .type = NLA_NUL_STRING, },
>  	[DPLL_A_PIN_PANEL_LABEL] = { .type = NLA_NUL_STRING, },
>  	[DPLL_A_PIN_PACKAGE_LABEL] = { .type = NLA_NUL_STRING, },
> -	[DPLL_A_PIN_TYPE] = NLA_POLICY_RANGE(NLA_U32, 1, 5),
> +	[DPLL_A_PIN_TYPE] = NLA_POLICY_RANGE(NLA_U32, 1, 6),
>  };
>  
>  /* DPLL_CMD_PIN_GET - do */
> diff --git a/include/uapi/linux/dpll.h b/include/uapi/linux/dpll.h
> index cb363cccf2e2a..9245827de3cfd 100644
> --- a/include/uapi/linux/dpll.h
> +++ b/include/uapi/linux/dpll.h
> @@ -127,6 +127,9 @@ enum dpll_type {
>   * @DPLL_PIN_TYPE_SYNCE_ETH_PORT: ethernet port PHY's recovered clock
>   * @DPLL_PIN_TYPE_INT_OSCILLATOR: device internal oscillator
>   * @DPLL_PIN_TYPE_GNSS: GNSS recovered clock
> + * @DPLL_PIN_TYPE_INT_NCO: Device internal numerically controlled oscillator.
> + *   When connected as a DPLL input, the DPLL enters NCO mode where the output
> + *   frequency is adjusted by the host via the PTP clock interface.
>   */
>  enum dpll_pin_type {
>  	DPLL_PIN_TYPE_MUX = 1,
> @@ -134,6 +137,7 @@ enum dpll_pin_type {
>  	DPLL_PIN_TYPE_SYNCE_ETH_PORT,
>  	DPLL_PIN_TYPE_INT_OSCILLATOR,
>  	DPLL_PIN_TYPE_GNSS,
> +	DPLL_PIN_TYPE_INT_NCO,
>  
>  	/* private: */
>  	__DPLL_PIN_TYPE_MAX,


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

* Re: [PATCH net-next v5 3/4] dpll: zl3073x: add per-DPLL serialization lock
  2026-05-31 19:44 ` [PATCH net-next v5 3/4] dpll: zl3073x: add per-DPLL serialization lock Ivan Vecera
  2026-06-04  1:40   ` Jakub Kicinski
@ 2026-06-04  1:51   ` Jakub Kicinski
  1 sibling, 0 replies; 8+ messages in thread
From: Jakub Kicinski @ 2026-06-04  1:51 UTC (permalink / raw)
  To: ivecera
  Cc: Jakub Kicinski, netdev, poros, arkadiusz.kubalewski, davem,
	donald.hunter, edumazet, jiri, mschmidt, pabeni, pvaanane,
	Prathosh.Satish, horms, vadim.fedorenko, linux-kernel

This is an AI-generated review of your patch. The human sending this
email says "While we wait for more acks - claude says.."
---
dpll: zl3073x: add per-DPLL serialization lock

This patch adds a per-DPLL mutex serializing netlink callbacks and the
periodic worker, moves zl3073x_chan_state_update() under the new lock,
defers change notifications outside the lock to avoid ABBA with
dpll_lock, and converts pin->freq_offset from atomic64_t to plain s64.

> diff --git a/drivers/dpll/zl3073x/core.c b/drivers/dpll/zl3073x/core.c
> index 0b2050aa2ed9..27c71807e4ef 100644
> --- a/drivers/dpll/zl3073x/core.c
> +++ b/drivers/dpll/zl3073x/core.c
> @@ -567,19 +567,7 @@ zl3073x_dev_ref_states_update(struct zl3073x_dev *zldev)
>  	}
>  }
>  
> -static void
> -zl3073x_dev_chan_states_update(struct zl3073x_dev *zldev)
> -{
> -	int i, rc;
>  
> -	for (i = 0; i < zldev->info->num_channels; i++) {
> -		rc = zl3073x_chan_state_update(zldev, i);
> -		if (rc)
> -			dev_warn(zldev->dev,
> -				 "Failed to get DPLL%u state: %pe\n", i,
> -				 ERR_PTR(rc));
> -	}
> -}

[ ... ]

> diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
> index 5e58ded5734d..4bee3d0c4659 100644
> --- a/drivers/dpll/zl3073x/dpll.c
> +++ b/drivers/dpll/zl3073x/dpll.c

[ ... ]

> @@ -1817,27 +1906,37 @@ zl3073x_dpll_pin_measured_freq_check(struct zl3073x_dpll_pin *pin)
>  void
>  zl3073x_dpll_changes_check(struct zl3073x_dpll *zldpll)
>  {
> +	DECLARE_BITMAP(changed_pins, ZL3073X_NUM_INPUT_PINS);
>  	struct zl3073x_dev *zldev = zldpll->dev;
>  	enum dpll_lock_status lock_status;
>  	struct device *dev = zldev->dev;
>  	struct zl3073x_dpll_pin *pin;
> +	bool dev_changed = false;
>  	int rc;
>  
> +	bitmap_zero(changed_pins, ZL3073X_NUM_INPUT_PINS);
> +
> +	mutex_lock(&zldpll->lock);
> +
>  	zldpll->check_count++;
>  
> -	/* Get current lock status for the DPLL */
> -	rc = zl3073x_dpll_lock_status_get(zldpll->dpll_dev, zldpll,
> -					  &lock_status, NULL, NULL);
> +	rc = zl3073x_chan_state_update(zldev, zldpll->id);
> +	if (rc) {
> +		dev_err(dev, "Failed to get DPLL%u state: %pe\n",
> +			zldpll->id, ERR_PTR(rc));
> +		goto unlock;
> +	}

[Low]
The previous code path, zl3073x_dev_chan_states_update() in core.c, used
dev_warn() and continued the periodic update for the device, allowing the
subsequent lock-status, phase-offset and per-pin checks to run for every
DPLL even when an individual chan_state_update() failed.

After this change, a single failure of zl3073x_chan_state_update() inside
zl3073x_dpll_changes_check() is logged at dev_err and triggers
goto unlock, skipping all remaining work for that DPLL in that cycle:

	rc = zl3073x_chan_state_update(zldev, zldpll->id);
	if (rc) {
		dev_err(dev, "Failed to get DPLL%u state: %pe\n",
			zldpll->id, ERR_PTR(rc));
		goto unlock;
	}

The commit message describes the move only as a serialization change so
that it "runs under zldpll->lock". Should the log-level promotion from warn
to err and the new short-circuit of the rest of the cycle on transient
register-read failures be mentioned in the commit message, or alternatively
preserved as warn-and-continue?

> +
> +	rc = __zl3073x_dpll_lock_status_get(zldpll, &lock_status);
>  	if (rc) {
>  		dev_err(dev, "Failed to get DPLL%u lock status: %pe\n",
>  			zldpll->id, ERR_PTR(rc));
> -		return;
> +		goto unlock;
>  	}

[ ... ]

>  }

The commit message states:

    All DPLL pin and device callbacks that access mutable state take the
    lock as the first operation.

[Low]
Does this hold for zl3073x_dpll_freq_monitor_get() and
zl3073x_dpll_freq_monitor_set()? Both are device callbacks that read or
write zldpll->dev->freq_monitor, but neither takes zldpll->lock:

	static int
	zl3073x_dpll_freq_monitor_get(const struct dpll_device *dpll, ...)
	{
		struct zl3073x_dpll *zldpll = dpll_priv;

		if (zldpll->dev->freq_monitor)
			*state = DPLL_FEATURE_STATE_ENABLE;
		else
			*state = DPLL_FEATURE_STATE_DISABLE;

		return 0;
	}

	static int
	zl3073x_dpll_freq_monitor_set(const struct dpll_device *dpll, ...)
	{
		struct zl3073x_dpll *item, *zldpll = dpll_priv;

		zldpll->dev->freq_monitor = (state == DPLL_FEATURE_STATE_ENABLE);
		...
	}

The same field is read in zl3073x_dpll_pin_measured_freq_check() under
lockdep_assert_held(&zldpll->lock):

	static bool
	zl3073x_dpll_pin_measured_freq_check(struct zl3073x_dpll_pin *pin)
	{
		...
		lockdep_assert_held(&zldpll->lock);

		if (!zldpll->dev->freq_monitor)
			return false;
		...
	}

freq_monitor is a per-device bool shared by all channels, so the per-DPLL
lock would not provide cross-DPLL serialization for it anyway. Should the
commit message carve out an exception for this per-device field, or should
some appropriate lock be taken in the freq_monitor get/set callbacks to
match the stated invariant?

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

end of thread, other threads:[~2026-06-04  1:51 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-31 19:44 [PATCH net-next v5 0/4] dpll: add NCO pin type and zl3073x support Ivan Vecera
2026-05-31 19:44 ` [PATCH net-next v5 1/4] dpll: add DPLL_PIN_TYPE_INT_NCO pin type Ivan Vecera
2026-06-04  1:50   ` Jakub Kicinski
2026-05-31 19:44 ` [PATCH net-next v5 2/4] dpll: zl3073x: use per-operation poll timeouts Ivan Vecera
2026-05-31 19:44 ` [PATCH net-next v5 3/4] dpll: zl3073x: add per-DPLL serialization lock Ivan Vecera
2026-06-04  1:40   ` Jakub Kicinski
2026-06-04  1:51   ` Jakub Kicinski
2026-05-31 19:44 ` [PATCH net-next v5 4/4] dpll: zl3073x: add NCO virtual input pin Ivan Vecera

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