[PATCH net v3 0/4] dpll: zl3073x: fix output pin frequency, esync and sibling notifications

[PATCH net v3 0/4] dpll: zl3073x: fix output pin frequency, esync and sibling notifications

[Ivan Vecera]

This series fixes several issues in the zl3073x output pin handling.

Patch 1 rejects output frequencies whose divisor is too small for the
hardware (less than 2, or less than 4 for the N-pin of an N-divided
output) as well as a zero frequency; such frequencies were accepted and
offered to userspace before.

Patch 2 makes the embedded sync (eSync) configuration follow the output
carrier frequency. Changing an output pin's frequency did not update its
eSync period/width, so eSync ran at the wrong rate and, for a 1 Hz
carrier, could get stuck on because esync_get() hides it. This was hit
during development and confirmed on an oscilloscope.

Patch 3 rejects N-divided output frequencies that cannot be represented
exactly. Such requests used to be silently rounded, which could also
change the sibling N-pin frequency.

Patch 4 notifies the sibling pin of an output pair when a shared HW
setting (divisor, clock type, eSync period/width, phase compensation) is
changed through one pin, so userspace is told the change happened. It
also ties the zldpll->pins list membership to the DPLL registration
state so the sibling lookup cannot miss a registered pin during probe or
teardown.

Changes since v2:
- Added patch 1 ("reject output frequencies with too small divisor").
- Added patch 3 ("reject inexact frequencies for N-divided outputs"),
  ordered before the notification patch so that a successful P-pin
  frequency change is guaranteed to keep the N-pin frequency unchanged.
- Patch 2: simplify zl3073x_out_esync_is_enabled(); reword the
  esync_get() readback note to only claim what it does - it reports the
  driver's nominal 1 Hz / 25% eSync model and does not faithfully report
  a configuration the driver did not program.
- Patch 4: add/remove pins from zldpll->pins around dpll_pin_register()/
  dpll_pin_unregister() so a registered sibling is always findable by the
  notification lookup (fixes a lost notification during probe/teardown
  pointed out in the v2 review).

Changes since v1:
- Patch 2: pass the actual output carrier (synth_freq / out.div) to
  zl3073x_out_esync_enable() in the frequency change path, matching
  esync_set() (no functional change, output frequencies always divide
  the synth frequency); drop the redundant "? true : false" reported by
  coccinelle (boolconv); note in the commit message that esync_get() now
  reports the fixed 1 Hz / 25% values the driver programs; fix a comment
  typo.
- Patch 4: narrow the commit message wording about zldpll->lock to the
  new sibling lookup helper (no code change).
- Both original commit messages now describe how the issues were found
  and whether they were triggered, as requested on the v1 thread.

v2: https://lore.kernel.org/netdev/20261002074520.30048-1-ivecera@redhat.com/
v1: https://lore.kernel.org/netdev/20261001080648.1424172-1-ivecera@redhat.com/

Ivan Vecera (4):
  dpll: zl3073x: reject output frequencies with too small divisor
  dpll: zl3073x: make embedded sync follow the output carrier frequency
  dpll: zl3073x: reject inexact frequencies for N-divided outputs
  dpll: zl3073x: notify sibling pin when shared output config changes

 drivers/dpll/zl3073x/dpll.c | 244 +++++++++++++++++++++++++++---------
 drivers/dpll/zl3073x/out.h  |  36 ++++++
 drivers/dpll/zl3073x/prop.c |  29 +++--
 3 files changed, 241 insertions(+), 68 deletions(-)

-- 
2.55.0

[PATCH net v3 1/4] dpll: zl3073x: reject output frequencies with too small divisor

[Ivan Vecera]

The output divider has to be at least 2. In N-pin divide mode the
per-output divider must be 2 or more and the N-pin is divided once more
(also by 2 or more), so the N-pin needs an effective divisor of at least
4.

zl3073x_pin_check_freq() only checked that an output frequency divides the
synth frequency, so it accepted frequencies that would require a smaller
divisor. Reject them: require a divisor of at least 2 for normal outputs
and the P-pin of N-divided outputs, and 4 for the N-pin. Also reject a
zero frequency, which would otherwise divide by zero.

Fixes: a99a9f0ebdaa ("dpll: zl3073x: Read DPLL types and pin properties from system firmware")
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/prop.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/dpll/zl3073x/prop.c b/drivers/dpll/zl3073x/prop.c
index ac9d41d0f978..a4bdc3878590 100644
--- a/drivers/dpll/zl3073x/prop.c
+++ b/drivers/dpll/zl3073x/prop.c
@@ -22,7 +22,8 @@
  * The function checks the given frequency is valid for the device. For input
  * pins it checks that the frequency can be factorized using supported base
  * frequencies. For output pins it checks that the frequency divides connected
- * synth frequency without remainder.
+ * synth frequency without remainder and that the resulting divisor is within
+ * the range supported by the hardware.
  *
  * Return: true if the frequency is valid, false if not.
  */
@@ -30,7 +31,7 @@ static bool
 zl3073x_pin_check_freq(struct zl3073x_dev *zldev, enum dpll_pin_direction dir,
 		       u8 id, u64 freq)
 {
-	if (freq > U32_MAX)
+	if (!freq || freq > U32_MAX)
 		goto err_inv_freq;
 
 	if (dir == DPLL_PIN_DIRECTION_INPUT) {
@@ -41,18 +42,32 @@ zl3073x_pin_check_freq(struct zl3073x_dev *zldev, enum dpll_pin_direction dir,
 		if (rc)
 			goto err_inv_freq;
 	} else {
-		u32 synth_freq;
-		u8 out, synth;
+		const struct zl3073x_out *out;
+		u32 synth_freq, div, min_div, rem;
+		u8 out_id, synth;
 
 		/* Get output pin synthesizer */
-		out = zl3073x_output_pin_out_get(id);
-		synth = zl3073x_dev_out_synth_get(zldev, out);
+		out_id = zl3073x_output_pin_out_get(id);
+		synth = zl3073x_dev_out_synth_get(zldev, out_id);
 
 		/* Get synth frequency */
 		synth_freq = zl3073x_dev_synth_freq_get(zldev, synth);
 
 		/* Check the frequency divides synth frequency */
-		if (synth_freq % (u32)freq)
+		div = div_u64_rem(synth_freq, freq, &rem);
+		if (rem)
+			goto err_inv_freq;
+
+		/* The output divisor has to be at least 2. For the N-pin of an
+		 * N-divided output the effective divisor also includes the N
+		 * divider (>= 2), so the minimum is 4.
+		 */
+		out = zl3073x_out_state_get(zldev, out_id);
+		if (zl3073x_out_is_ndiv(out) && zl3073x_is_n_pin(id))
+			min_div = 4;
+		else
+			min_div = 2;
+		if (div < min_div)
 			goto err_inv_freq;
 	}
 
-- 
2.55.0

[PATCH net v3 2/4] dpll: zl3073x: make embedded sync follow the output carrier frequency

[Ivan Vecera]

Changing an output pin's frequency did not update its embedded sync
(eSync) configuration. The eSync period and width are computed relative
to the carrier:

    esync_n_period = out_freq / esync_freq
    esync_n_width  = div / 2

Once zl3073x_dpll_output_pin_frequency_set() changes div, both values no
longer match the new carrier, so the embedded sync runs at the wrong
frequency and duty cycle. Worse, if the new carrier is <= 1 Hz then
esync_get() returns -EOPNOTSUPP (out_freq <= 1), hiding the stale ESYNC
mode so the user can no longer turn it off - it is stuck on.

This was hit during development: after changing an output pin's
frequency the embedded sync output stopped working correctly, as
confirmed on an oscilloscope - it ran at the wrong frequency and duty
cycle because the eSync period and width still matched the old carrier.
For a new 1 Hz carrier esync_get() returned -EOPNOTSUPP, leaving eSync
enabled with no way to disable it.

Factor the eSync handling into zl3073x_out_esync_{is_enabled,enable,
disable}() helpers and reuse them from esync_get()/esync_set(), then
adjust the eSync parameters in the output frequency change path for
non-N-divided outputs after updating div/width:

- If the new frequency is 1 Hz, disable eSync. It cannot work at a 1 Hz
  carrier and esync_get() hides it, so it must be cleared rather than
  left stuck on.
- Otherwise, if eSync is active, recompute its period and width for the
  new carrier keeping the 1 Hz eSync frequency.

As a side effect of reusing the helpers, esync_get() now reports the
driver's eSync model - a 1 Hz frequency and a 25% pulse - from the clock
type when eSync is enabled, instead of deriving the values from the
cached esync_n_period/esync_n_width registers. The driver only ever
programs a 1 Hz / 25% eSync, so this matches what esync_set() configures
and keeps get and set symmetric. A configuration the driver did not
establish - a non-1 Hz eSync left in flash or by an older kernel, or the
truncated duty cycle of an odd divider - is therefore reported as the
nominal 1 Hz / 25% rather than its exact register values.

N-divided outputs are left untouched: there esync_n_period is the N
divider and eSync is not supported anyway.

Fixes: ce26d7ca50a5 ("dpll: zl3073x: Add support to get/set frequency on pins")
Fixes: 634ca2cb06d2 ("dpll: zl3073x: Add support to get/set esync on pins")
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/dpll.c | 68 ++++++++++++++++---------------------
 drivers/dpll/zl3073x/out.h  | 36 ++++++++++++++++++++
 2 files changed, 66 insertions(+), 38 deletions(-)

diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
index f2e137475b40..7c997966c3c3 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -852,7 +852,7 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
 	struct zl3073x_dpll_pin *pin = pin_priv;
 	const struct zl3073x_synth *synth;
 	const struct zl3073x_out *out;
-	u32 synth_freq, out_freq;
+	u32 synth_freq;
 	u8 out_id;
 
 	guard(mutex)(&zldpll->lock);
@@ -864,38 +864,28 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
 	 * for N-division is also used for the esync divider so both cannot
 	 * be used.
 	 */
-	if (zl3073x_out_is_ndiv(out))
+	if (zl3073x_out_is_ndiv(out) || !pin->esync_control)
 		return -EOPNOTSUPP;
 
 	/* Get attached synth frequency */
 	synth = zl3073x_synth_state_get(zldev, zl3073x_out_synth_get(out));
 	synth_freq = zl3073x_synth_freq_get(synth);
-	out_freq = synth_freq / out->div;
 
-	if (!pin->esync_control || out_freq <= 1)
+	/* The esync is not supported for 1 Hz base frequency */
+	if (synth_freq / out->div <= 1)
 		return -EOPNOTSUPP;
 
 	esync->range = esync_freq_ranges;
 	esync->range_num = ARRAY_SIZE(esync_freq_ranges);
 
-	if (zl3073x_out_clock_type_get(out) != ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC) {
-		/* No need to read esync data if it is not enabled */
+	if (zl3073x_out_esync_is_enabled(out)) {
+		esync->freq = 1;
+		esync->pulse = 25;
+	} else {
 		esync->freq = 0;
 		esync->pulse = 0;
-
-		return 0;
 	}
 
-	/* Compute esync frequency */
-	esync->freq = out_freq / out->esync_n_period;
-
-	/* By comparing the esync_pulse_width to the half of the pulse width
-	 * the esync pulse percentage can be determined.
-	 * Note that half pulse width is in units of half synth cycles, which
-	 * is why it reduces down to be output_div.
-	 */
-	esync->pulse = (50 * out->esync_n_width) / out->div;
-
 	return 0;
 }
 
@@ -926,32 +916,17 @@ zl3073x_dpll_output_pin_esync_set(const struct dpll_pin *dpll_pin,
 	if (zl3073x_out_is_ndiv(&out))
 		return -EOPNOTSUPP;
 
-	/* Update clock type in output mode */
-	if (freq)
-		zl3073x_out_clock_type_set(&out,
-					   ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC);
-	else
-		zl3073x_out_clock_type_set(&out,
-					   ZL_OUTPUT_MODE_CLOCK_TYPE_NORMAL);
-
-	/* If esync is being disabled just write mailbox and finish */
-	if (!freq)
+	if (!freq) {
+		zl3073x_out_esync_disable(&out);
 		return zl3073x_out_state_set(zldev, out_id, &out);
+	}
 
 	/* Get attached synth frequency */
 	synth = zl3073x_synth_state_get(zldev, zl3073x_out_synth_get(&out));
 	synth_freq = zl3073x_synth_freq_get(synth);
 
-	/* Compute and update esync period */
-	out.esync_n_period = synth_freq / (u32)freq / out.div;
-
-	/* Half of the period in units of 1/2 synth cycle can be represented by
-	 * the output_div. To get the supported esync pulse width of 25% of the
-	 * period the output_div can just be divided by 2. Note that this
-	 * assumes that output_div is even, otherwise some resolution will be
-	 * lost.
-	 */
-	out.esync_n_width = out.div / 2;
+	/* Enable 1PPS eSync for this pin frequency */
+	zl3073x_out_esync_enable(&out, synth_freq / out.div);
 
 	/* Commit output configuration */
 	return zl3073x_out_state_set(zldev, out_id, &out);
@@ -1009,6 +984,23 @@ zl3073x_dpll_output_pin_frequency_set(const struct dpll_pin *dpll_pin,
 		/* For 50/50 duty cycle the divisor is equal to width */
 		out.width = new_div;
 
+		/* The embedded sync period and width are computed relative to
+		 * the output carrier so they have to be adjusted to follow the
+		 * new frequency.
+		 */
+		if (zl3073x_out_esync_is_enabled(&out)) {
+			if (frequency == 1) {
+				/* Disable eSync if the new frequency is 1 Hz */
+				zl3073x_out_esync_disable(&out);
+			} else {
+				/* Update esync period and width according to
+				 * the new frequency.
+				 */
+				zl3073x_out_esync_enable(&out,
+							 synth_freq / out.div);
+			}
+		}
+
 		/* Commit output configuration */
 		return zl3073x_out_state_set(zldev, out_id, &out);
 	}
diff --git a/drivers/dpll/zl3073x/out.h b/drivers/dpll/zl3073x/out.h
index 660889c57bff..d2b8b4eb5a76 100644
--- a/drivers/dpll/zl3073x/out.h
+++ b/drivers/dpll/zl3073x/out.h
@@ -134,4 +134,40 @@ static inline u8 zl3073x_out_synth_get(const struct zl3073x_out *out)
 	return FIELD_GET(ZL_OUTPUT_CTRL_SYNTH_SEL, out->ctrl);
 }
 
+static inline bool zl3073x_out_esync_is_enabled(const struct zl3073x_out *out)
+{
+	u8 clock_type = zl3073x_out_clock_type_get(out);
+
+	return clock_type == ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC;
+}
+
+static inline
+void zl3073x_out_esync_enable(struct zl3073x_out *out, u32 base_freq)
+{
+	/* Period is expressed as the number of output divider clock cycles.
+	 * So for 1 Hz the period is equal to base frequency.
+	 */
+	out->esync_n_period = base_freq;
+
+	/* Half of the period in units of 1/2 synth cycle can be represented by
+	 * the output_div. To get the supported esync pulse width of 25% of the
+	 * period the output_div can just be divided by 2. Note that this
+	 * assumes that output_div is even, otherwise some resolution will be
+	 * lost.
+	 */
+	out->esync_n_width = out->div / 2;
+
+	/* Update the clock type */
+	zl3073x_out_clock_type_set(out, ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC);
+}
+
+static inline
+void zl3073x_out_esync_disable(struct zl3073x_out *out)
+{
+	/* Just update the clock type. The eSync period and width are
+	 * ignored for this clock type.
+	 */
+	zl3073x_out_clock_type_set(out, ZL_OUTPUT_MODE_CLOCK_TYPE_NORMAL);
+}
+
 #endif /* _ZL3073X_OUT_H */
-- 
2.55.0

[PATCH net v3 3/4] dpll: zl3073x: reject inexact frequencies for N-divided outputs

[Ivan Vecera]

For N-divided outputs the P-pin and N-pin share the output divisor and
the N-pin frequency is synth_freq / (div * esync_n_period). The frequency
set helper computed esync_n_period with a truncating division and only
rejected a zero result, so a frequency that does not divide evenly was
silently rounded. Setting the P-pin could then shift the N-pin frequency
too (e.g. 600 MHz synth, div=60, period=10 gives P=10 MHz, N=1 MHz;
setting P to 2.5 MHz moves N to 1.25 MHz).

Check the division remainder and reject the request if it is not exact,
and require esync_n_period >= 2 so the N-pin frequency stays below the
P-pin one.

Fixes: ce26d7ca50a5 ("dpll: zl3073x: Add support to get/set frequency on pins")
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/dpll.c | 37 +++++++++++++++++++++++++++++++------
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
index 7c997966c3c3..65107b4cc4f8 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -1007,15 +1007,28 @@ zl3073x_dpll_output_pin_frequency_set(const struct dpll_pin *dpll_pin,
 
 	if (zl3073x_dpll_is_p_pin(pin)) {
 		/* We are going to change output frequency for P-pin but
-		 * if the requested frequency is less than current N-pin
-		 * frequency then indicate a failure as we are not able
+		 * if the requested frequency is less or equal than current
+		 * N-pin frequency then indicate a failure as we are not able
 		 * to compute N-pin divisor to keep its frequency unchanged.
 		 *
 		 * Update divisor for N-pin to keep N-pin frequency.
 		 */
-		out.esync_n_period = (out.esync_n_period * out.div) / new_div;
-		if (!out.esync_n_period)
+		u64 prod = mul_u32_u32(out.esync_n_period, out.div);
+		u32 rem;
+
+		out.esync_n_period = div_u64_rem(prod, new_div, &rem);
+		if (rem != 0) {
+			NL_SET_ERR_MSG_FMT(extack,
+					   "OUT%uN freq must divide OUT%uP freq",
+					   out_id, out_id);
 			return -EINVAL;
+		}
+		if (out.esync_n_period < 2) {
+			NL_SET_ERR_MSG_FMT(extack,
+					   "OUT%uN freq must be less than OUT%uP freq",
+					   out_id, out_id);
+			return -EINVAL;
+		}
 
 		/* Update the output divisor */
 		out.div = new_div;
@@ -1030,9 +1043,21 @@ zl3073x_dpll_output_pin_frequency_set(const struct dpll_pin *dpll_pin,
 		 *
 		 * Update divisor for N-pin
 		 */
-		out.esync_n_period = div64_u64(synth_freq, frequency * out.div);
-		if (!out.esync_n_period)
+		u64 rem, prod = frequency * out.div;
+
+		out.esync_n_period = div64_u64_rem(synth_freq, prod, &rem);
+		if (rem != 0) {
+			NL_SET_ERR_MSG_FMT(extack,
+					   "OUT%uN freq must divide OUT%uP freq",
+					   out_id, out_id);
 			return -EINVAL;
+		}
+		if (out.esync_n_period < 2) {
+			NL_SET_ERR_MSG_FMT(extack,
+					   "OUT%uN freq must be less than OUT%uP freq",
+					   out_id, out_id);
+			return -EINVAL;
+		}
 	}
 
 	/* For 50/50 duty cycle the divisor is equal to width */
-- 
2.55.0

[PATCH net v3 4/4] dpll: zl3073x: notify sibling pin when shared output config changes

[Ivan Vecera]

Each zl3073x output has a P-pin and an N-pin that share a single HW
output and, outside N-pin divide mode, share the output's divisor,
clock type, esync period/width and phase compensation registers.
Changing one of these settings through one pin's dpll_pin therefore
also changes the other (sibling) pin's effective configuration, but
only the pin the change was requested on gets a dpll_pin_change_ntf()
notification - userspace listening on the sibling pin is never told
its frequency, esync configuration or phase adjustment changed.

This was found by code inspection rather than triggered at runtime: a
dpll pin-get on the sibling pin returns the correct current values, but
because no change notification is emitted for it, userspace is never
told that its configuration changed asynchronously through the other
pin.

Add zl3073x_dpll_output_pin_sibling_get() to look up the other pin of
an output pair, and use it in frequency_set() (for the non-N-divided
signal formats, where the output divisor is shared), esync_set() and
phase_adjust_set() to notify the sibling pin, if it is registered,
whenever the shared HW state actually changes.

The three callbacks are switched from guard(mutex) to explicit
mutex_lock()/mutex_unlock() with goto-based unwinding, because the
notification must run after zldpll->lock is released: the notification
re-enters the pin get callbacks, which take zldpll->lock again, so
calling it under the lock would deadlock. The DPLL subsystem's own
dpll_lock is still held across the callback, so the __ (lock-held)
notification variant is used.

The sibling lookup walks the zldpll->pins list, so the list membership
has to match the DPLL registration state. dpll_pin_register() publishes
a pin (and P is registered before N) before it used to be added to the
list, and teardown detached the whole list before unregistering any
pin, so a sibling that is registered - and thus reachable by a PIN_SET
on the other pin - could be missing from the list and never notified.
Add the pin to the list before dpll_pin_register() and remove it only
after dpll_pin_unregister(), both under zldpll->lock, and assert the
lock in the lookup helper. A pin that is transiently on the list while
not registered is harmless: __dpll_pin_change_ntf() is a no-op for a
pin that is not available, and the sibling cannot be freed under the
lookup because the freeing goes through dpll_pin_unregister(), which
takes dpll_lock that the notification path holds.

Fixes: 634ca2cb06d2 ("dpll: zl3073x: Add support to get/set esync on pins")
Fixes: ce26d7ca50a5 ("dpll: zl3073x: Add support to get/set frequency on pins")
Fixes: 6287262f761e ("dpll: zl3073x: Add support to adjust phase")
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/dpll.c | 187 ++++++++++++++++++++++++++++--------
 1 file changed, 146 insertions(+), 41 deletions(-)

diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
index 65107b4cc4f8..9c678acc3e77 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -123,6 +123,8 @@ zl3073x_dpll_pin_get_by_ref(struct zl3073x_dpll *zldpll, u8 ref_id)
 {
 	struct zl3073x_dpll_pin *pin;
 
+	lockdep_assert_held(&zldpll->lock);
+
 	list_for_each_entry(pin, &zldpll->pins, list) {
 		if (zl3073x_dpll_is_input_pin(pin) &&
 		    zl3073x_input_pin_ref_get(pin->id) == ref_id)
@@ -132,11 +134,39 @@ zl3073x_dpll_pin_get_by_ref(struct zl3073x_dpll *zldpll, u8 ref_id)
 	return NULL;
 }
 
+/**
+ * zl3073x_dpll_output_pin_sibling_get - get the other pin of an output pair
+ * @pin: output pin whose sibling is sought
+ *
+ * Output pin ids are allocated in P/N pairs (P even, N odd) that share a
+ * single HW output. Looks up the other pin of the pair, if it is
+ * registered as a dpll_pin on this DPLL.
+ *
+ * Return: pointer to sibling pin, or NULL if it is not registered
+ */
+static struct zl3073x_dpll_pin *
+zl3073x_dpll_output_pin_sibling_get(struct zl3073x_dpll_pin *pin)
+{
+	struct zl3073x_dpll_pin *sibling;
+
+	lockdep_assert_held(&pin->dpll->lock);
+
+	list_for_each_entry(sibling, &pin->dpll->pins, list) {
+		if (!zl3073x_dpll_is_input_pin(sibling) &&
+		    sibling->id == (pin->id ^ 1))
+			return sibling;
+	}
+
+	return NULL;
+}
+
 static struct zl3073x_dpll_pin *
 zl3073x_dpll_nco_pin_get(struct zl3073x_dpll *zldpll)
 {
 	struct zl3073x_dpll_pin *pin;
 
+	lockdep_assert_held(&zldpll->lock);
+
 	list_for_each_entry(pin, &zldpll->pins, list) {
 		if (zl3073x_dpll_is_nco_pin(pin))
 			return pin;
@@ -899,12 +929,14 @@ zl3073x_dpll_output_pin_esync_set(const struct dpll_pin *dpll_pin,
 	struct zl3073x_dpll *zldpll = dpll_priv;
 	struct zl3073x_dev *zldev = zldpll->dev;
 	struct zl3073x_dpll_pin *pin = pin_priv;
+	struct zl3073x_dpll_pin *sibling = NULL;
 	const struct zl3073x_synth *synth;
 	struct zl3073x_out out;
 	u32 synth_freq;
 	u8 out_id;
+	int rc;
 
-	guard(mutex)(&zldpll->lock);
+	mutex_lock(&zldpll->lock);
 
 	out_id = zl3073x_output_pin_out_get(pin->id);
 	out = *zl3073x_out_state_get(zldev, out_id);
@@ -913,12 +945,14 @@ zl3073x_dpll_output_pin_esync_set(const struct dpll_pin *dpll_pin,
 	 * for N-division is also used for the esync divider so both cannot
 	 * be used.
 	 */
-	if (zl3073x_out_is_ndiv(&out))
-		return -EOPNOTSUPP;
+	if (zl3073x_out_is_ndiv(&out)) {
+		rc = -EOPNOTSUPP;
+		goto unlock;
+	}
 
 	if (!freq) {
 		zl3073x_out_esync_disable(&out);
-		return zl3073x_out_state_set(zldev, out_id, &out);
+		goto commit;
 	}
 
 	/* Get attached synth frequency */
@@ -927,9 +961,24 @@ zl3073x_dpll_output_pin_esync_set(const struct dpll_pin *dpll_pin,
 
 	/* Enable 1PPS eSync for this pin frequency */
 	zl3073x_out_esync_enable(&out, synth_freq / out.div);
-
+commit:
 	/* Commit output configuration */
-	return zl3073x_out_state_set(zldev, out_id, &out);
+	rc = zl3073x_out_state_set(zldev, out_id, &out);
+	if (rc)
+		goto unlock;
+
+	/* The clock type, esync period and esync width are all shared by
+	 * both pins of the output pair, so the sibling pin's esync
+	 * configuration changes too and userspace has to be notified.
+	 */
+	sibling = zl3073x_dpll_output_pin_sibling_get(pin);
+unlock:
+	mutex_unlock(&zldpll->lock);
+
+	if (!rc && sibling)
+		__dpll_pin_change_ntf(sibling->dpll_pin);
+
+	return rc;
 }
 
 static int
@@ -959,12 +1008,14 @@ zl3073x_dpll_output_pin_frequency_set(const struct dpll_pin *dpll_pin,
 	struct zl3073x_dpll *zldpll = dpll_priv;
 	struct zl3073x_dev *zldev = zldpll->dev;
 	struct zl3073x_dpll_pin *pin = pin_priv;
+	struct zl3073x_dpll_pin *sibling = NULL;
 	const struct zl3073x_synth *synth;
 	u32 new_div, synth_freq;
 	struct zl3073x_out out;
 	u8 out_id;
+	int rc;
 
-	guard(mutex)(&zldpll->lock);
+	mutex_lock(&zldpll->lock);
 
 	out_id = zl3073x_output_pin_out_get(pin->id);
 	out = *zl3073x_out_state_get(zldev, out_id);
@@ -977,7 +1028,8 @@ zl3073x_dpll_output_pin_frequency_set(const struct dpll_pin *dpll_pin,
 	/* Check signal format */
 	if (!zl3073x_out_is_ndiv(&out)) {
 		/* For non N-divided signal formats the frequency is computed
-		 * as division of synth frequency and output divisor.
+		 * as division of synth frequency and output divisor, which
+		 * is shared by both pins of the output pair.
 		 */
 		out.div = new_div;
 
@@ -1002,7 +1054,16 @@ zl3073x_dpll_output_pin_frequency_set(const struct dpll_pin *dpll_pin,
 		}
 
 		/* Commit output configuration */
-		return zl3073x_out_state_set(zldev, out_id, &out);
+		rc = zl3073x_out_state_set(zldev, out_id, &out);
+		if (rc)
+			goto unlock;
+
+		/* The other pin's frequency changed too - it has to be
+		 * notified about the change.
+		 */
+		sibling = zl3073x_dpll_output_pin_sibling_get(pin);
+
+		goto unlock;
 	}
 
 	if (zl3073x_dpll_is_p_pin(pin)) {
@@ -1017,18 +1078,10 @@ zl3073x_dpll_output_pin_frequency_set(const struct dpll_pin *dpll_pin,
 		u32 rem;
 
 		out.esync_n_period = div_u64_rem(prod, new_div, &rem);
-		if (rem != 0) {
-			NL_SET_ERR_MSG_FMT(extack,
-					   "OUT%uN freq must divide OUT%uP freq",
-					   out_id, out_id);
-			return -EINVAL;
-		}
-		if (out.esync_n_period < 2) {
-			NL_SET_ERR_MSG_FMT(extack,
-					   "OUT%uN freq must be less than OUT%uP freq",
-					   out_id, out_id);
-			return -EINVAL;
-		}
+		if (rem != 0)
+			goto err_n_nondiv;
+		if (out.esync_n_period < 2)
+			goto err_n_toohigh;
 
 		/* Update the output divisor */
 		out.div = new_div;
@@ -1046,25 +1099,36 @@ zl3073x_dpll_output_pin_frequency_set(const struct dpll_pin *dpll_pin,
 		u64 rem, prod = frequency * out.div;
 
 		out.esync_n_period = div64_u64_rem(synth_freq, prod, &rem);
-		if (rem != 0) {
-			NL_SET_ERR_MSG_FMT(extack,
-					   "OUT%uN freq must divide OUT%uP freq",
-					   out_id, out_id);
-			return -EINVAL;
-		}
-		if (out.esync_n_period < 2) {
-			NL_SET_ERR_MSG_FMT(extack,
-					   "OUT%uN freq must be less than OUT%uP freq",
-					   out_id, out_id);
-			return -EINVAL;
-		}
+		if (rem != 0)
+			goto err_n_nondiv;
+		if (out.esync_n_period < 2)
+			goto err_n_toohigh;
 	}
 
 	/* For 50/50 duty cycle the divisor is equal to width */
 	out.esync_n_width = out.esync_n_period;
 
 	/* Commit output configuration */
-	return zl3073x_out_state_set(zldev, out_id, &out);
+	rc = zl3073x_out_state_set(zldev, out_id, &out);
+unlock:
+	mutex_unlock(&zldpll->lock);
+
+	if (!rc && sibling)
+		__dpll_pin_change_ntf(sibling->dpll_pin);
+
+	return rc;
+err_n_nondiv:
+	NL_SET_ERR_MSG_FMT(extack,
+			   "OUT%uN freq must divide OUT%uP freq",
+			   out_id, out_id);
+	rc = -EINVAL;
+	goto unlock;
+err_n_toohigh:
+	NL_SET_ERR_MSG_FMT(extack,
+			   "OUT%uN freq must be less than OUT%uP freq",
+			   out_id, out_id);
+	rc = -EINVAL;
+	goto unlock;
 }
 
 static int
@@ -1103,10 +1167,12 @@ zl3073x_dpll_output_pin_phase_adjust_set(const struct dpll_pin *dpll_pin,
 	struct zl3073x_dpll *zldpll = dpll_priv;
 	struct zl3073x_dev *zldev = zldpll->dev;
 	struct zl3073x_dpll_pin *pin = pin_priv;
+	struct zl3073x_dpll_pin *sibling = NULL;
 	struct zl3073x_out out;
 	u8 out_id;
+	int rc;
 
-	guard(mutex)(&zldpll->lock);
+	mutex_lock(&zldpll->lock);
 
 	out_id = zl3073x_output_pin_out_get(pin->id);
 	out = *zl3073x_out_state_get(zldev, out_id);
@@ -1115,7 +1181,21 @@ zl3073x_dpll_output_pin_phase_adjust_set(const struct dpll_pin *dpll_pin,
 	out.phase_comp = phase_adjust / pin->phase_gran;
 
 	/* Update output configuration from mailbox */
-	return zl3073x_out_state_set(zldev, out_id, &out);
+	rc = zl3073x_out_state_set(zldev, out_id, &out);
+	if (rc)
+		goto unlock;
+
+	/* The phase compensation register is shared by both pins of the
+	 * output pair, so the sibling pin's phase adjustment changes too.
+	 */
+	sibling = zl3073x_dpll_output_pin_sibling_get(pin);
+unlock:
+	mutex_unlock(&zldpll->lock);
+
+	if (!rc && sibling)
+		__dpll_pin_change_ntf(sibling->dpll_pin);
+
+	return rc;
 }
 
 static int
@@ -1739,6 +1819,13 @@ zl3073x_dpll_pin_register(struct zl3073x_dpll_pin *pin, u32 index)
 	else
 		ops = &zl3073x_dpll_output_pin_ops;
 
+	/* Add the pin to the list before registering it with the DPLL core so
+	 * that it is findable as a sibling as soon as the core publishes it.
+	 */
+	mutex_lock(&zldpll->lock);
+	list_add(&pin->list, &zldpll->pins);
+	mutex_unlock(&zldpll->lock);
+
 	/* Register the pin */
 	rc = dpll_pin_register(zldpll->dpll_dev, pin->dpll_pin, ops, pin);
 	if (rc)
@@ -1750,6 +1837,9 @@ zl3073x_dpll_pin_register(struct zl3073x_dpll_pin *pin, u32 index)
 	return 0;
 
 err_register:
+	mutex_lock(&zldpll->lock);
+	list_del(&pin->list);
+	mutex_unlock(&zldpll->lock);
 	dpll_pin_put(pin->dpll_pin, &pin->tracker);
 err_pin_get:
 	pin->dpll_pin = NULL;
@@ -1784,6 +1874,13 @@ zl3073x_dpll_pin_unregister(struct zl3073x_dpll_pin *pin)
 	/* Unregister the pin */
 	dpll_pin_unregister(zldpll->dpll_dev, pin->dpll_pin, ops, pin);
 
+	/* Remove the pin from the list only after it has been unregistered so
+	 * that a still-registered pin is always findable as a sibling.
+	 */
+	mutex_lock(&zldpll->lock);
+	list_del(&pin->list);
+	mutex_unlock(&zldpll->lock);
+
 	dpll_pin_put(pin->dpll_pin, &pin->tracker);
 	pin->dpll_pin = NULL;
 
@@ -1803,9 +1900,11 @@ zl3073x_dpll_pins_unregister(struct zl3073x_dpll *zldpll)
 {
 	struct zl3073x_dpll_pin *pin, *next;
 
+	/* Unregister each pin before removing it from the list so that a
+	 * still-registered pin is always findable as a sibling.
+	 */
 	list_for_each_entry_safe(pin, next, &zldpll->pins, list) {
 		zl3073x_dpll_pin_unregister(pin);
-		list_del(&pin->list);
 		zl3073x_dpll_pin_free(pin);
 	}
 }
@@ -1918,16 +2017,24 @@ zl3073x_dpll_nco_pin_register(struct zl3073x_dpll *zldpll)
 		goto err_pin_get;
 	}
 
+	/* Add the pin to the list before registering it with the DPLL core so
+	 * that the list reflects the DPLL registration state.
+	 */
+	mutex_lock(&zldpll->lock);
+	list_add(&pin->list, &zldpll->pins);
+	mutex_unlock(&zldpll->lock);
+
 	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:
+	mutex_lock(&zldpll->lock);
+	list_del(&pin->list);
+	mutex_unlock(&zldpll->lock);
 	dpll_pin_put(pin->dpll_pin, &pin->tracker);
 err_pin_get:
 	pin->dpll_pin = NULL;
@@ -1979,8 +2086,6 @@ zl3073x_dpll_pins_register(struct zl3073x_dpll *zldpll)
 			zl3073x_dpll_pin_free(pin);
 			goto error;
 		}
-
-		list_add(&pin->list, &zldpll->pins);
 	}
 
 	/* Register NCO virtual input pin */
-- 
2.55.0