[PATCH net-next 0/6] dpll: zl3073x: refactor state management

[PATCH net-next 0/6] dpll: zl3073x: refactor state management

[Ivan Vecera]

This series refactors the zl3073x DPLL driver to centralize hardware
state management behind dedicated per-module state interfaces, replacing
scattered direct register accesses in dpll.c with cached state and
proper accessor functions.

The driver already uses a fetch/get/set pattern for ref, out, and synth
modules. This series extends and refines that pattern:

First, struct_group() is applied to the existing ref, out, and synth
structures to partition fields into cfg (mutable configuration), inv
(invariants set at init), and stat (read-only status) groups. This
enables group-level memcmp for short-circuit checks and bulk copies in
state_set, and adds invariant validation guards.

A ref_state_update() helper is extracted to encapsulate the per-reference
monitor status register read, keeping direct register access behind the
ref module interface.

A new zl3073x_chan module is introduced following the same pattern,
caching the DPLL channel mode_refsel register with inline getters and
setters. The refsel_mode and forced_ref fields are removed from struct
zl3073x_dpll in favor of the cached channel state.

The chan module is then extended with cached mon_status and refsel_status
registers, converting lock_status_get and selected_ref_get from direct
HW reads to cached state lookups refreshed by the periodic worker.

Reference priority registers are cached in the chan cfg group, removing
the ad-hoc ref_prio_get/set functions and the redundant pin->selectable
flag, which is now derived from the cached priority. The
selected_ref_set function is inlined into input_pin_state_on_dpll_set,
unifying all mode paths through a single chan_state_set commit point.

Finally, selected_ref_get is dropped entirely since the refsel_status
register provides the selected reference regardless of mode, and
connected_ref_get is simplified to a direct refsel_state check.

Ivan Vecera (6):
  dpll: zl3073x: use struct_group to partition states
  dpll: zl3073x: add zl3073x_ref_state_update helper
  dpll: zl3073x: introduce zl3073x_chan for DPLL channel state
  dpll: zl3073x: add DPLL channel status fields to zl3073x_chan
  dpll: zl3073x: add reference priority to zl3073x_chan
  dpll: zl3073x: drop selected and simplify connected ref getter

 drivers/dpll/zl3073x/Makefile |   4 +-
 drivers/dpll/zl3073x/chan.c   | 164 ++++++++++++
 drivers/dpll/zl3073x/chan.h   | 179 +++++++++++++
 drivers/dpll/zl3073x/core.c   |  36 ++-
 drivers/dpll/zl3073x/core.h   |  14 +-
 drivers/dpll/zl3073x/dpll.c   | 490 +++++++++-------------------------
 drivers/dpll/zl3073x/dpll.h   |   4 -
 drivers/dpll/zl3073x/out.c    |  27 +-
 drivers/dpll/zl3073x/out.h    |  21 +-
 drivers/dpll/zl3073x/ref.c    |  58 ++--
 drivers/dpll/zl3073x/ref.h    |  33 ++-
 drivers/dpll/zl3073x/regs.h   |  12 +
 drivers/dpll/zl3073x/synth.h  |  16 +-
 13 files changed, 626 insertions(+), 432 deletions(-)
 create mode 100644 drivers/dpll/zl3073x/chan.c
 create mode 100644 drivers/dpll/zl3073x/chan.h

-- 
2.52.0

[PATCH net-next 1/6] dpll: zl3073x: use struct_group to partition states

[Ivan Vecera]

Organize the zl3073x_out, zl3073x_ref, and zl3073x_synth structures
using struct_group() to partition fields into semantic groups:

  * cfg:  mutable configuration written to HW via state_set
  * inv:  invariant fields set once during state_fetch
  * stat: read-only status

This enables group-level operations in place of field-by-field copies:

  * state_set validates invariants haven't changed (WARN_ON + -EINVAL)
  * state_set short-circuits when cfg is unchanged
  * state_set copy entire groups in a single assignment instead of
    enumerating each field

Add kernel doc for zl3073x_out_state_set and zl3073x_ref_state_set
documenting the new invariant validation and short-circuit semantics.

Remove forward declaration of zl3073x_synth_state_set().

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/out.c   | 27 +++++++++++++++++++------
 drivers/dpll/zl3073x/out.h   | 21 ++++++++++++--------
 drivers/dpll/zl3073x/ref.c   | 38 ++++++++++++++++++++++--------------
 drivers/dpll/zl3073x/ref.h   | 31 +++++++++++++++++------------
 drivers/dpll/zl3073x/synth.h | 16 +++++++--------
 5 files changed, 84 insertions(+), 49 deletions(-)

diff --git a/drivers/dpll/zl3073x/out.c b/drivers/dpll/zl3073x/out.c
index 86829a0c1c022..eb5628aebcee8 100644
--- a/drivers/dpll/zl3073x/out.c
+++ b/drivers/dpll/zl3073x/out.c
@@ -106,12 +106,32 @@ const struct zl3073x_out *zl3073x_out_state_get(struct zl3073x_dev *zldev,
 	return &zldev->out[index];
 }
 
+/**
+ * zl3073x_out_state_set - commit output state changes to hardware
+ * @zldev: pointer to zl3073x_dev structure
+ * @index: output index to set state for
+ * @out: desired output state
+ *
+ * Validates that invariant fields have not been modified, skips the HW
+ * write if the mutable configuration is unchanged, and otherwise writes
+ * only the changed cfg fields to hardware via the mailbox interface.
+ *
+ * Return: 0 on success, -EINVAL if invariants changed, <0 on HW error
+ */
 int zl3073x_out_state_set(struct zl3073x_dev *zldev, u8 index,
 			  const struct zl3073x_out *out)
 {
 	struct zl3073x_out *dout = &zldev->out[index];
 	int rc;
 
+	/* Reject attempts to change invariant fields (set at fetch only) */
+	if (WARN_ON(memcmp(&dout->inv, &out->inv, sizeof(out->inv))))
+		return -EINVAL;
+
+	/* Skip HW write if configuration hasn't changed */
+	if (!memcmp(&dout->cfg, &out->cfg, sizeof(out->cfg)))
+		return 0;
+
 	guard(mutex)(&zldev->multiop_lock);
 
 	/* Read output configuration into mailbox */
@@ -146,12 +166,7 @@ int zl3073x_out_state_set(struct zl3073x_dev *zldev, u8 index,
 		return rc;
 
 	/* After successful commit store new state */
-	dout->div = out->div;
-	dout->width = out->width;
-	dout->esync_n_period = out->esync_n_period;
-	dout->esync_n_width = out->esync_n_width;
-	dout->mode = out->mode;
-	dout->phase_comp = out->phase_comp;
+	dout->cfg = out->cfg;
 
 	return 0;
 }
diff --git a/drivers/dpll/zl3073x/out.h b/drivers/dpll/zl3073x/out.h
index 318f9bb8da3a0..edf40432bba5f 100644
--- a/drivers/dpll/zl3073x/out.h
+++ b/drivers/dpll/zl3073x/out.h
@@ -4,6 +4,7 @@
 #define _ZL3073X_OUT_H
 
 #include <linux/bitfield.h>
+#include <linux/stddef.h>
 #include <linux/types.h>
 
 #include "regs.h"
@@ -17,17 +18,21 @@ struct zl3073x_dev;
  * @esync_n_period: embedded sync or n-pin period (for n-div formats)
  * @esync_n_width: embedded sync or n-pin pulse width
  * @phase_comp: phase compensation
- * @ctrl: output control
  * @mode: output mode
+ * @ctrl: output control
  */
 struct zl3073x_out {
-	u32	div;
-	u32	width;
-	u32	esync_n_period;
-	u32	esync_n_width;
-	s32	phase_comp;
-	u8	ctrl;
-	u8	mode;
+	struct_group(cfg, /* Config */
+		u32	div;
+		u32	width;
+		u32	esync_n_period;
+		u32	esync_n_width;
+		s32	phase_comp;
+		u8	mode;
+	);
+	struct_group(inv, /* Invariants */
+		u8	ctrl;
+	);
 };
 
 int zl3073x_out_state_fetch(struct zl3073x_dev *zldev, u8 index);
diff --git a/drivers/dpll/zl3073x/ref.c b/drivers/dpll/zl3073x/ref.c
index 6b65e61039999..8b4c4807bcc44 100644
--- a/drivers/dpll/zl3073x/ref.c
+++ b/drivers/dpll/zl3073x/ref.c
@@ -73,14 +73,8 @@ int zl3073x_ref_state_fetch(struct zl3073x_dev *zldev, u8 index)
 		struct zl3073x_ref *p_ref = ref - 1; /* P-pin counterpart*/
 
 		/* Copy the shared items from the P-pin */
-		ref->config = p_ref->config;
-		ref->esync_n_div = p_ref->esync_n_div;
-		ref->freq_base = p_ref->freq_base;
-		ref->freq_mult = p_ref->freq_mult;
-		ref->freq_ratio_m = p_ref->freq_ratio_m;
-		ref->freq_ratio_n = p_ref->freq_ratio_n;
-		ref->phase_comp = p_ref->phase_comp;
-		ref->sync_ctrl = p_ref->sync_ctrl;
+		ref->cfg = p_ref->cfg;
+		ref->inv = p_ref->inv;
 
 		return 0; /* Finish - no non-shared items for now */
 	}
@@ -154,12 +148,32 @@ zl3073x_ref_state_get(struct zl3073x_dev *zldev, u8 index)
 	return &zldev->ref[index];
 }
 
+/**
+ * zl3073x_ref_state_set - commit input reference state changes to hardware
+ * @zldev: pointer to zl3073x_dev structure
+ * @index: input reference index to set state for
+ * @ref: desired reference state
+ *
+ * Validates that invariant fields have not been modified, skips the HW
+ * write if the mutable configuration is unchanged, and otherwise writes
+ * only the changed cfg fields to hardware via the mailbox interface.
+ *
+ * Return: 0 on success, -EINVAL if invariants changed, <0 on HW error
+ */
 int zl3073x_ref_state_set(struct zl3073x_dev *zldev, u8 index,
 			  const struct zl3073x_ref *ref)
 {
 	struct zl3073x_ref *dref = &zldev->ref[index];
 	int rc;
 
+	/* Reject attempts to change invariant fields (set at init only) */
+	if (WARN_ON(memcmp(&dref->inv, &ref->inv, sizeof(ref->inv))))
+		return -EINVAL;
+
+	/* Skip HW write if configuration hasn't changed */
+	if (!memcmp(&dref->cfg, &ref->cfg, sizeof(ref->cfg)))
+		return 0;
+
 	guard(mutex)(&zldev->multiop_lock);
 
 	/* Read reference configuration into mailbox */
@@ -207,13 +221,7 @@ int zl3073x_ref_state_set(struct zl3073x_dev *zldev, u8 index,
 		return rc;
 
 	/* After successful commit store new state */
-	dref->freq_base = ref->freq_base;
-	dref->freq_mult = ref->freq_mult;
-	dref->freq_ratio_m = ref->freq_ratio_m;
-	dref->freq_ratio_n = ref->freq_ratio_n;
-	dref->esync_n_div = ref->esync_n_div;
-	dref->sync_ctrl = ref->sync_ctrl;
-	dref->phase_comp = ref->phase_comp;
+	dref->cfg = ref->cfg;
 
 	return 0;
 }
diff --git a/drivers/dpll/zl3073x/ref.h b/drivers/dpll/zl3073x/ref.h
index 0d8618f5ce8df..ab3a816c29108 100644
--- a/drivers/dpll/zl3073x/ref.h
+++ b/drivers/dpll/zl3073x/ref.h
@@ -5,6 +5,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/math64.h>
+#include <linux/stddef.h>
 #include <linux/types.h>
 
 #include "regs.h"
@@ -13,28 +14,34 @@ struct zl3073x_dev;
 
 /**
  * struct zl3073x_ref - input reference state
- * @ffo: current fractional frequency offset
  * @phase_comp: phase compensation
  * @esync_n_div: divisor for embedded sync or n-divided signal formats
  * @freq_base: frequency base
  * @freq_mult: frequnecy multiplier
  * @freq_ratio_m: FEC mode multiplier
  * @freq_ratio_n: FEC mode divisor
- * @config: reference config
  * @sync_ctrl: reference sync control
+ * @config: reference config
+ * @ffo: current fractional frequency offset
  * @mon_status: reference monitor status
  */
 struct zl3073x_ref {
-	s64	ffo;
-	u64	phase_comp;
-	u32	esync_n_div;
-	u16	freq_base;
-	u16	freq_mult;
-	u16	freq_ratio_m;
-	u16	freq_ratio_n;
-	u8	config;
-	u8	sync_ctrl;
-	u8	mon_status;
+	struct_group(cfg, /* Configuration */
+		u64	phase_comp;
+		u32	esync_n_div;
+		u16	freq_base;
+		u16	freq_mult;
+		u16	freq_ratio_m;
+		u16	freq_ratio_n;
+		u8	sync_ctrl;
+	);
+	struct_group(inv, /* Invariants */
+		u8	config;
+	);
+	struct_group(stat, /* Status */
+		s64	ffo;
+		u8	mon_status;
+	);
 };
 
 int zl3073x_ref_state_fetch(struct zl3073x_dev *zldev, u8 index);
diff --git a/drivers/dpll/zl3073x/synth.h b/drivers/dpll/zl3073x/synth.h
index 6c55eb8a888c2..89e13ea2e6d94 100644
--- a/drivers/dpll/zl3073x/synth.h
+++ b/drivers/dpll/zl3073x/synth.h
@@ -5,6 +5,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/math64.h>
+#include <linux/stddef.h>
 #include <linux/types.h>
 
 #include "regs.h"
@@ -20,11 +21,13 @@ struct zl3073x_dev;
  * @ctrl: synth control
  */
 struct zl3073x_synth {
-	u32	freq_mult;
-	u16	freq_base;
-	u16	freq_m;
-	u16	freq_n;
-	u8	ctrl;
+	struct_group(inv, /* Invariants */
+		u32	freq_mult;
+		u16	freq_base;
+		u16	freq_m;
+		u16	freq_n;
+		u8	ctrl;
+	);
 };
 
 int zl3073x_synth_state_fetch(struct zl3073x_dev *zldev, u8 synth_id);
@@ -32,9 +35,6 @@ int zl3073x_synth_state_fetch(struct zl3073x_dev *zldev, u8 synth_id);
 const struct zl3073x_synth *zl3073x_synth_state_get(struct zl3073x_dev *zldev,
 						    u8 synth_id);
 
-int zl3073x_synth_state_set(struct zl3073x_dev *zldev, u8 synth_id,
-			    const struct zl3073x_synth *synth);
-
 /**
  * zl3073x_synth_dpll_get - get DPLL ID the synth is driven by
  * @synth: pointer to synth state
-- 
2.52.0

[PATCH net-next 2/6] dpll: zl3073x: add zl3073x_ref_state_update helper

[Ivan Vecera]

Extract the per-reference monitor status HW read into a dedicated
zl3073x_ref_state_update() helper in the ref module. Rename
zl3073x_dev_ref_status_update() to zl3073x_dev_ref_states_update()
and use the new helper in it. Call it from zl3073x_ref_state_fetch()
as well so that mon_status is initialized at device startup. This
keeps direct register access and struct field writes behind the ref
module's interface, consistent with the state management pattern
used for other ref operations.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/core.c |  9 ++++-----
 drivers/dpll/zl3073x/ref.c  | 20 ++++++++++++++++++++
 drivers/dpll/zl3073x/ref.h  |  2 ++
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/dpll/zl3073x/core.c b/drivers/dpll/zl3073x/core.c
index 10e036ccf08f0..07626082aae3b 100644
--- a/drivers/dpll/zl3073x/core.c
+++ b/drivers/dpll/zl3073x/core.c
@@ -543,13 +543,12 @@ zl3073x_dev_state_fetch(struct zl3073x_dev *zldev)
 }
 
 static void
-zl3073x_dev_ref_status_update(struct zl3073x_dev *zldev)
+zl3073x_dev_ref_states_update(struct zl3073x_dev *zldev)
 {
 	int i, rc;
 
 	for (i = 0; i < ZL3073X_NUM_REFS; i++) {
-		rc = zl3073x_read_u8(zldev, ZL_REG_REF_MON_STATUS(i),
-				     &zldev->ref[i].mon_status);
+		rc = zl3073x_ref_state_update(zldev, i);
 		if (rc)
 			dev_warn(zldev->dev,
 				 "Failed to get REF%u status: %pe\n", i,
@@ -679,8 +678,8 @@ zl3073x_dev_periodic_work(struct kthread_work *work)
 	struct zl3073x_dpll *zldpll;
 	int rc;
 
-	/* Update input references status */
-	zl3073x_dev_ref_status_update(zldev);
+	/* Update input references' states */
+	zl3073x_dev_ref_states_update(zldev);
 
 	/* Update DPLL-to-connected-ref phase offsets registers */
 	rc = zl3073x_ref_phase_offsets_update(zldev, -1);
diff --git a/drivers/dpll/zl3073x/ref.c b/drivers/dpll/zl3073x/ref.c
index 8b4c4807bcc44..825ac30bcd6f7 100644
--- a/drivers/dpll/zl3073x/ref.c
+++ b/drivers/dpll/zl3073x/ref.c
@@ -51,6 +51,21 @@ zl3073x_ref_freq_factorize(u32 freq, u16 *base, u16 *mult)
 	return -EINVAL;
 }
 
+/**
+ * zl3073x_ref_state_update - update input reference status from HW
+ * @zldev: pointer to zl3073x_dev structure
+ * @index: input reference index
+ *
+ * Return: 0 on success, <0 on error
+ */
+int zl3073x_ref_state_update(struct zl3073x_dev *zldev, u8 index)
+{
+	struct zl3073x_ref *ref = &zldev->ref[index];
+
+	return zl3073x_read_u8(zldev, ZL_REG_REF_MON_STATUS(index),
+			       &ref->mon_status);
+}
+
 /**
  * zl3073x_ref_state_fetch - fetch input reference state from hardware
  * @zldev: pointer to zl3073x_dev structure
@@ -79,6 +94,11 @@ int zl3073x_ref_state_fetch(struct zl3073x_dev *zldev, u8 index)
 		return 0; /* Finish - no non-shared items for now */
 	}
 
+	/* Read reference status */
+	rc = zl3073x_ref_state_update(zldev, index);
+	if (rc)
+		return rc;
+
 	guard(mutex)(&zldev->multiop_lock);
 
 	/* Read reference configuration */
diff --git a/drivers/dpll/zl3073x/ref.h b/drivers/dpll/zl3073x/ref.h
index ab3a816c29108..06d8d4d97ea26 100644
--- a/drivers/dpll/zl3073x/ref.h
+++ b/drivers/dpll/zl3073x/ref.h
@@ -52,6 +52,8 @@ const struct zl3073x_ref *zl3073x_ref_state_get(struct zl3073x_dev *zldev,
 int zl3073x_ref_state_set(struct zl3073x_dev *zldev, u8 index,
 			  const struct zl3073x_ref *ref);
 
+int zl3073x_ref_state_update(struct zl3073x_dev *zldev, u8 index);
+
 int zl3073x_ref_freq_factorize(u32 freq, u16 *base, u16 *mult);
 
 /**
-- 
2.52.0

[PATCH net-next 3/6] dpll: zl3073x: introduce zl3073x_chan for DPLL channel state

[Ivan Vecera]

Extract DPLL channel state management into a dedicated zl3073x_chan
module, following the pattern already established by zl3073x_ref,
zl3073x_out and zl3073x_synth.

The new struct zl3073x_chan caches the raw mode_refsel register value
in a cfg group with inline getters and setters to extract and update
the bitfields. Three standard state management functions are provided:

 - zl3073x_chan_state_fetch: read the mode_refsel register from HW
 - zl3073x_chan_state_get: return cached channel state
 - zl3073x_chan_state_set: write changed state to HW, skip if unchanged

The channel state array chan[ZL3073X_MAX_CHANNELS] is added to struct
zl3073x_dev. Channel state is fetched as part of
zl3073x_dev_state_fetch, using the chip-specific channel count.

The refsel_mode and forced_ref fields are removed from struct
zl3073x_dpll and all direct register accesses in dpll.c are replaced
with the new chan state operations.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/Makefile |   4 +-
 drivers/dpll/zl3073x/chan.c   |  79 ++++++++++++++++++++++++++
 drivers/dpll/zl3073x/chan.h   |  74 ++++++++++++++++++++++++
 drivers/dpll/zl3073x/core.c   |  10 ++++
 drivers/dpll/zl3073x/core.h   |   3 +
 drivers/dpll/zl3073x/dpll.c   | 104 ++++++++++++++++------------------
 drivers/dpll/zl3073x/dpll.h   |   4 --
 7 files changed, 217 insertions(+), 61 deletions(-)
 create mode 100644 drivers/dpll/zl3073x/chan.c
 create mode 100644 drivers/dpll/zl3073x/chan.h

diff --git a/drivers/dpll/zl3073x/Makefile b/drivers/dpll/zl3073x/Makefile
index bd324c7fe7101..906ec3fbcc202 100644
--- a/drivers/dpll/zl3073x/Makefile
+++ b/drivers/dpll/zl3073x/Makefile
@@ -1,8 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_ZL3073X)		+= zl3073x.o
-zl3073x-objs			:= core.o devlink.o dpll.o flash.o fw.o	\
-				   out.o prop.o ref.o synth.o
+zl3073x-objs			:= chan.o core.o devlink.o dpll.o	\
+				   flash.o fw.o out.o prop.o ref.o synth.o
 
 obj-$(CONFIG_ZL3073X_I2C)	+= zl3073x_i2c.o
 zl3073x_i2c-objs		:= i2c.o
diff --git a/drivers/dpll/zl3073x/chan.c b/drivers/dpll/zl3073x/chan.c
new file mode 100644
index 0000000000000..6f383d489fab7
--- /dev/null
+++ b/drivers/dpll/zl3073x/chan.c
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/dev_printk.h>
+#include <linux/string.h>
+#include <linux/types.h>
+
+#include "chan.h"
+#include "core.h"
+
+/**
+ * zl3073x_chan_state_fetch - fetch DPLL channel state from hardware
+ * @zldev: pointer to zl3073x_dev structure
+ * @index: DPLL channel index to fetch state for
+ *
+ * Reads the mode_refsel register for the given DPLL channel and stores
+ * the raw value for later use.
+ *
+ * Return: 0 on success, <0 on error
+ */
+int zl3073x_chan_state_fetch(struct zl3073x_dev *zldev, u8 index)
+{
+	struct zl3073x_chan *chan = &zldev->chan[index];
+	int rc;
+
+	rc = zl3073x_read_u8(zldev, ZL_REG_DPLL_MODE_REFSEL(index),
+			     &chan->mode_refsel);
+	if (rc)
+		return rc;
+
+	dev_dbg(zldev->dev, "DPLL%u mode: %u, ref: %u\n", index,
+		zl3073x_chan_mode_get(chan), zl3073x_chan_ref_get(chan));
+
+	return 0;
+}
+
+/**
+ * zl3073x_chan_state_get - get current DPLL channel state
+ * @zldev: pointer to zl3073x_dev structure
+ * @index: DPLL channel index to get state for
+ *
+ * Return: pointer to given DPLL channel state
+ */
+const struct zl3073x_chan *zl3073x_chan_state_get(struct zl3073x_dev *zldev,
+						 u8 index)
+{
+	return &zldev->chan[index];
+}
+
+/**
+ * zl3073x_chan_state_set - commit DPLL channel state changes to hardware
+ * @zldev: pointer to zl3073x_dev structure
+ * @index: DPLL channel index to set state for
+ * @chan: desired channel state
+ *
+ * Skips the HW write if the configuration is unchanged, and otherwise
+ * writes the mode_refsel register to hardware.
+ *
+ * Return: 0 on success, <0 on HW error
+ */
+int zl3073x_chan_state_set(struct zl3073x_dev *zldev, u8 index,
+			   const struct zl3073x_chan *chan)
+{
+	struct zl3073x_chan *dchan = &zldev->chan[index];
+	int rc;
+
+	/* Skip HW write if configuration hasn't changed */
+	if (!memcmp(&dchan->cfg, &chan->cfg, sizeof(chan->cfg)))
+		return 0;
+
+	rc = zl3073x_write_u8(zldev, ZL_REG_DPLL_MODE_REFSEL(index),
+			      chan->mode_refsel);
+	if (rc)
+		return rc;
+
+	/* After successful write store new state */
+	dchan->cfg = chan->cfg;
+
+	return 0;
+}
diff --git a/drivers/dpll/zl3073x/chan.h b/drivers/dpll/zl3073x/chan.h
new file mode 100644
index 0000000000000..3e6ffaef0c743
--- /dev/null
+++ b/drivers/dpll/zl3073x/chan.h
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _ZL3073X_CHAN_H
+#define _ZL3073X_CHAN_H
+
+#include <linux/bitfield.h>
+#include <linux/stddef.h>
+#include <linux/types.h>
+
+#include "regs.h"
+
+struct zl3073x_dev;
+
+/**
+ * struct zl3073x_chan - DPLL channel state
+ * @mode_refsel: mode and reference selection register value
+ */
+struct zl3073x_chan {
+	struct_group(cfg,
+		u8	mode_refsel;
+	);
+};
+
+int zl3073x_chan_state_fetch(struct zl3073x_dev *zldev, u8 index);
+const struct zl3073x_chan *zl3073x_chan_state_get(struct zl3073x_dev *zldev,
+						 u8 index);
+int zl3073x_chan_state_set(struct zl3073x_dev *zldev, u8 index,
+			   const struct zl3073x_chan *chan);
+
+/**
+ * zl3073x_chan_mode_get - get DPLL channel operating mode
+ * @chan: pointer to channel state
+ *
+ * Return: reference selection mode of the given DPLL channel
+ */
+static inline u8 zl3073x_chan_mode_get(const struct zl3073x_chan *chan)
+{
+	return FIELD_GET(ZL_DPLL_MODE_REFSEL_MODE, chan->mode_refsel);
+}
+
+/**
+ * zl3073x_chan_ref_get - get manually selected reference
+ * @chan: pointer to channel state
+ *
+ * Return: reference selected in forced reference lock mode
+ */
+static inline u8 zl3073x_chan_ref_get(const struct zl3073x_chan *chan)
+{
+	return FIELD_GET(ZL_DPLL_MODE_REFSEL_REF, chan->mode_refsel);
+}
+
+/**
+ * zl3073x_chan_mode_set - set DPLL channel operating mode
+ * @chan: pointer to channel state
+ * @mode: mode to set
+ */
+static inline void zl3073x_chan_mode_set(struct zl3073x_chan *chan, u8 mode)
+{
+	chan->mode_refsel &= ~ZL_DPLL_MODE_REFSEL_MODE;
+	chan->mode_refsel |= FIELD_PREP(ZL_DPLL_MODE_REFSEL_MODE, mode);
+}
+
+/**
+ * zl3073x_chan_ref_set - set manually selected reference
+ * @chan: pointer to channel state
+ * @ref: reference to set
+ */
+static inline void zl3073x_chan_ref_set(struct zl3073x_chan *chan, u8 ref)
+{
+	chan->mode_refsel &= ~ZL_DPLL_MODE_REFSEL_REF;
+	chan->mode_refsel |= FIELD_PREP(ZL_DPLL_MODE_REFSEL_REF, ref);
+}
+
+#endif /* _ZL3073X_CHAN_H */
diff --git a/drivers/dpll/zl3073x/core.c b/drivers/dpll/zl3073x/core.c
index 07626082aae3b..b03e59fa0834b 100644
--- a/drivers/dpll/zl3073x/core.c
+++ b/drivers/dpll/zl3073x/core.c
@@ -539,6 +539,16 @@ zl3073x_dev_state_fetch(struct zl3073x_dev *zldev)
 		}
 	}
 
+	for (i = 0; i < zldev->info->num_channels; i++) {
+		rc = zl3073x_chan_state_fetch(zldev, i);
+		if (rc) {
+			dev_err(zldev->dev,
+				"Failed to fetch channel state: %pe\n",
+				ERR_PTR(rc));
+			return rc;
+		}
+	}
+
 	return rc;
 }
 
diff --git a/drivers/dpll/zl3073x/core.h b/drivers/dpll/zl3073x/core.h
index b6f22ee1c0bd1..2cfb9dd74aa53 100644
--- a/drivers/dpll/zl3073x/core.h
+++ b/drivers/dpll/zl3073x/core.h
@@ -9,6 +9,7 @@
 #include <linux/mutex.h>
 #include <linux/types.h>
 
+#include "chan.h"
 #include "out.h"
 #include "ref.h"
 #include "regs.h"
@@ -61,6 +62,7 @@ struct zl3073x_chip_info {
  * @ref: array of input references' invariants
  * @out: array of outs' invariants
  * @synth: array of synths' invariants
+ * @chan: array of DPLL channels' state
  * @dplls: list of DPLLs
  * @kworker: thread for periodic work
  * @work: periodic work
@@ -77,6 +79,7 @@ struct zl3073x_dev {
 	struct zl3073x_ref	ref[ZL3073X_NUM_REFS];
 	struct zl3073x_out	out[ZL3073X_NUM_OUTS];
 	struct zl3073x_synth	synth[ZL3073X_NUM_SYNTHS];
+	struct zl3073x_chan	chan[ZL3073X_MAX_CHANNELS];
 
 	/* DPLL channels */
 	struct list_head	dplls;
diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
index c201c974a7f9a..f56f073e57df4 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -259,10 +259,13 @@ static int
 zl3073x_dpll_selected_ref_get(struct zl3073x_dpll *zldpll, u8 *ref)
 {
 	struct zl3073x_dev *zldev = zldpll->dev;
+	const struct zl3073x_chan *chan;
 	u8 state, value;
 	int rc;
 
-	switch (zldpll->refsel_mode) {
+	chan = zl3073x_chan_state_get(zldev, zldpll->id);
+
+	switch (zl3073x_chan_mode_get(chan)) {
 	case ZL_DPLL_MODE_REFSEL_MODE_AUTO:
 		/* For automatic mode read refsel_status register */
 		rc = zl3073x_read_u8(zldev,
@@ -282,7 +285,7 @@ zl3073x_dpll_selected_ref_get(struct zl3073x_dpll *zldpll, u8 *ref)
 		break;
 	case ZL_DPLL_MODE_REFSEL_MODE_REFLOCK:
 		/* For manual mode return stored value */
-		*ref = zldpll->forced_ref;
+		*ref = zl3073x_chan_ref_get(chan);
 		break;
 	default:
 		/* For other modes like NCO, freerun... there is no input ref */
@@ -307,10 +310,11 @@ static int
 zl3073x_dpll_selected_ref_set(struct zl3073x_dpll *zldpll, u8 ref)
 {
 	struct zl3073x_dev *zldev = zldpll->dev;
-	u8 mode, mode_refsel;
-	int rc;
+	struct zl3073x_chan chan;
+	u8 mode;
 
-	mode = zldpll->refsel_mode;
+	chan = *zl3073x_chan_state_get(zldev, zldpll->id);
+	mode = zl3073x_chan_mode_get(&chan);
 
 	switch (mode) {
 	case ZL_DPLL_MODE_REFSEL_MODE_REFLOCK:
@@ -328,8 +332,8 @@ zl3073x_dpll_selected_ref_set(struct zl3073x_dpll *zldpll, u8 ref)
 				break;
 			}
 			/* Keep selected reference */
-			ref = zldpll->forced_ref;
-		} else if (ref == zldpll->forced_ref) {
+			ref = zl3073x_chan_ref_get(&chan);
+		} else if (ref == zl3073x_chan_ref_get(&chan)) {
 			/* No register update - same mode and same ref */
 			return 0;
 		}
@@ -351,21 +355,10 @@ zl3073x_dpll_selected_ref_set(struct zl3073x_dpll *zldpll, u8 ref)
 		return -EOPNOTSUPP;
 	}
 
-	/* Build mode_refsel value */
-	mode_refsel = FIELD_PREP(ZL_DPLL_MODE_REFSEL_MODE, mode) |
-		      FIELD_PREP(ZL_DPLL_MODE_REFSEL_REF, ref);
-
-	/* Update dpll_mode_refsel register */
-	rc = zl3073x_write_u8(zldev, ZL_REG_DPLL_MODE_REFSEL(zldpll->id),
-			      mode_refsel);
-	if (rc)
-		return rc;
-
-	/* Store new mode and forced reference */
-	zldpll->refsel_mode = mode;
-	zldpll->forced_ref = ref;
+	zl3073x_chan_mode_set(&chan, mode);
+	zl3073x_chan_ref_set(&chan, ref);
 
-	return rc;
+	return zl3073x_chan_state_set(zldev, zldpll->id, &chan);
 }
 
 /**
@@ -624,9 +617,11 @@ zl3073x_dpll_ref_state_get(struct zl3073x_dpll_pin *pin,
 {
 	struct zl3073x_dpll *zldpll = pin->dpll;
 	struct zl3073x_dev *zldev = zldpll->dev;
+	const struct zl3073x_chan *chan;
 	u8 ref, ref_conn;
 	int rc;
 
+	chan = zl3073x_chan_state_get(zldev, zldpll->id);
 	ref = zl3073x_input_pin_ref_get(pin->id);
 
 	/* Get currently connected reference */
@@ -643,7 +638,7 @@ zl3073x_dpll_ref_state_get(struct zl3073x_dpll_pin *pin,
 	 * selectable and its monitor does not report any error then report
 	 * pin as selectable.
 	 */
-	if (zldpll->refsel_mode == ZL_DPLL_MODE_REFSEL_MODE_AUTO &&
+	if (zl3073x_chan_mode_get(chan) == ZL_DPLL_MODE_REFSEL_MODE_AUTO &&
 	    zl3073x_dev_ref_is_status_ok(zldev, ref) && pin->selectable) {
 		*state = DPLL_PIN_STATE_SELECTABLE;
 		return 0;
@@ -678,10 +673,13 @@ 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;
+	const struct zl3073x_chan *chan;
 	u8 new_ref;
 	int rc;
 
-	switch (zldpll->refsel_mode) {
+	chan = zl3073x_chan_state_get(zldpll->dev, zldpll->id);
+
+	switch (zl3073x_chan_mode_get(chan)) {
 	case ZL_DPLL_MODE_REFSEL_MODE_REFLOCK:
 	case ZL_DPLL_MODE_REFSEL_MODE_FREERUN:
 	case ZL_DPLL_MODE_REFSEL_MODE_HOLDOVER:
@@ -1092,10 +1090,13 @@ zl3073x_dpll_lock_status_get(const struct dpll_device *dpll, void *dpll_priv,
 {
 	struct zl3073x_dpll *zldpll = dpll_priv;
 	struct zl3073x_dev *zldev = zldpll->dev;
+	const struct zl3073x_chan *chan;
 	u8 mon_status, state;
 	int rc;
 
-	switch (zldpll->refsel_mode) {
+	chan = zl3073x_chan_state_get(zldev, zldpll->id);
+
+	switch (zl3073x_chan_mode_get(chan)) {
 	case ZL_DPLL_MODE_REFSEL_MODE_FREERUN:
 	case ZL_DPLL_MODE_REFSEL_MODE_NCO:
 		/* In FREERUN and NCO modes the DPLL is always unlocked */
@@ -1140,13 +1141,16 @@ zl3073x_dpll_supported_modes_get(const struct dpll_device *dpll,
 				 struct netlink_ext_ack *extack)
 {
 	struct zl3073x_dpll *zldpll = dpll_priv;
+	const struct zl3073x_chan *chan;
+
+	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 (zldpll->refsel_mode != ZL_DPLL_MODE_REFSEL_MODE_NCO)
+	if (zl3073x_chan_mode_get(chan) != ZL_DPLL_MODE_REFSEL_MODE_NCO)
 		__set_bit(DPLL_MODE_AUTOMATIC, modes);
 
 	__set_bit(DPLL_MODE_MANUAL, modes);
@@ -1159,8 +1163,11 @@ zl3073x_dpll_mode_get(const struct dpll_device *dpll, void *dpll_priv,
 		      enum dpll_mode *mode, struct netlink_ext_ack *extack)
 {
 	struct zl3073x_dpll *zldpll = dpll_priv;
+	const struct zl3073x_chan *chan;
 
-	switch (zldpll->refsel_mode) {
+	chan = zl3073x_chan_state_get(zldpll->dev, zldpll->id);
+
+	switch (zl3073x_chan_mode_get(chan)) {
 	case ZL_DPLL_MODE_REFSEL_MODE_FREERUN:
 	case ZL_DPLL_MODE_REFSEL_MODE_HOLDOVER:
 	case ZL_DPLL_MODE_REFSEL_MODE_NCO:
@@ -1239,7 +1246,8 @@ 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;
-	u8 hw_mode, mode_refsel, ref;
+	struct zl3073x_chan chan;
+	u8 hw_mode, ref;
 	int rc;
 
 	rc = zl3073x_dpll_selected_ref_get(zldpll, &ref);
@@ -1287,26 +1295,18 @@ zl3073x_dpll_mode_set(const struct dpll_device *dpll, void *dpll_priv,
 		hw_mode = ZL_DPLL_MODE_REFSEL_MODE_AUTO;
 	}
 
-	/* Build mode_refsel value */
-	mode_refsel = FIELD_PREP(ZL_DPLL_MODE_REFSEL_MODE, hw_mode);
-
+	chan = *zl3073x_chan_state_get(zldpll->dev, zldpll->id);
+	zl3073x_chan_mode_set(&chan, hw_mode);
 	if (ZL3073X_DPLL_REF_IS_VALID(ref))
-		mode_refsel |= FIELD_PREP(ZL_DPLL_MODE_REFSEL_REF, ref);
+		zl3073x_chan_ref_set(&chan, ref);
 
-	/* Update dpll_mode_refsel register */
-	rc = zl3073x_write_u8(zldpll->dev, ZL_REG_DPLL_MODE_REFSEL(zldpll->id),
-			      mode_refsel);
+	rc = zl3073x_chan_state_set(zldpll->dev, zldpll->id, &chan);
 	if (rc) {
 		NL_SET_ERR_MSG_MOD(extack,
 				   "failed to set reference selection mode");
 		return rc;
 	}
 
-	zldpll->refsel_mode = hw_mode;
-
-	if (ZL3073X_DPLL_REF_IS_VALID(ref))
-		zldpll->forced_ref = ref;
-
 	return 0;
 }
 
@@ -1559,15 +1559,18 @@ 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 (zldpll->refsel_mode == ZL_DPLL_MODE_REFSEL_MODE_NCO)
+		if (zl3073x_chan_mode_get(chan) == ZL_DPLL_MODE_REFSEL_MODE_NCO)
 			return false;
 
 		name = "REF";
@@ -1675,21 +1678,8 @@ static int
 zl3073x_dpll_device_register(struct zl3073x_dpll *zldpll)
 {
 	struct zl3073x_dev *zldev = zldpll->dev;
-	u8 dpll_mode_refsel;
 	int rc;
 
-	/* Read DPLL mode and forcibly selected reference */
-	rc = zl3073x_read_u8(zldev, ZL_REG_DPLL_MODE_REFSEL(zldpll->id),
-			     &dpll_mode_refsel);
-	if (rc)
-		return rc;
-
-	/* Extract mode and selected input reference */
-	zldpll->refsel_mode = FIELD_GET(ZL_DPLL_MODE_REFSEL_MODE,
-					dpll_mode_refsel);
-	zldpll->forced_ref = FIELD_GET(ZL_DPLL_MODE_REFSEL_REF,
-				       dpll_mode_refsel);
-
 	zldpll->ops = zl3073x_dpll_device_ops;
 	if (zldev->info->flags & ZL3073X_FLAG_DIE_TEMP)
 		zldpll->ops.temp_get = zl3073x_dpll_temp_get;
@@ -1844,8 +1834,10 @@ zl3073x_dpll_changes_check(struct zl3073x_dpll *zldpll)
 	struct zl3073x_dev *zldev = zldpll->dev;
 	enum dpll_lock_status lock_status;
 	struct device *dev = zldev->dev;
+	const struct zl3073x_chan *chan;
 	struct zl3073x_dpll_pin *pin;
 	int rc;
+	u8 mode;
 
 	zldpll->check_count++;
 
@@ -1867,8 +1859,10 @@ zl3073x_dpll_changes_check(struct zl3073x_dpll *zldpll)
 	/* Input pin monitoring does make sense only in automatic
 	 * or forced reference modes.
 	 */
-	if (zldpll->refsel_mode != ZL_DPLL_MODE_REFSEL_MODE_AUTO &&
-	    zldpll->refsel_mode != ZL_DPLL_MODE_REFSEL_MODE_REFLOCK)
+	chan = zl3073x_chan_state_get(zldev, zldpll->id);
+	mode = zl3073x_chan_mode_get(chan);
+	if (mode != ZL_DPLL_MODE_REFSEL_MODE_AUTO &&
+	    mode != ZL_DPLL_MODE_REFSEL_MODE_REFLOCK)
 		return;
 
 	/* Update phase offset latch registers for this DPLL if the phase
diff --git a/drivers/dpll/zl3073x/dpll.h b/drivers/dpll/zl3073x/dpll.h
index 278a24f357c9b..115ee4f67e7ab 100644
--- a/drivers/dpll/zl3073x/dpll.h
+++ b/drivers/dpll/zl3073x/dpll.h
@@ -13,8 +13,6 @@
  * @list: this DPLL list entry
  * @dev: pointer to multi-function parent device
  * @id: DPLL index
- * @refsel_mode: reference selection mode
- * @forced_ref: selected reference in forced reference lock mode
  * @check_count: periodic check counter
  * @phase_monitor: is phase offset monitor enabled
  * @ops: DPLL device operations for this instance
@@ -28,8 +26,6 @@ struct zl3073x_dpll {
 	struct list_head		list;
 	struct zl3073x_dev		*dev;
 	u8				id;
-	u8				refsel_mode;
-	u8				forced_ref;
 	u8				check_count;
 	bool				phase_monitor;
 	struct dpll_device_ops		ops;
-- 
2.52.0

[PATCH net-next 4/6] dpll: zl3073x: add DPLL channel status fields to zl3073x_chan

[Ivan Vecera]

Add mon_status and refsel_status fields to struct zl3073x_chan in a
stat group to cache the 'dpll_mon_status' and 'dpll_refsel_status'
registers.

Add zl3073x_chan_lock_state_get(), zl3073x_chan_is_ho_ready(),
zl3073x_chan_refsel_state_get() and zl3073x_chan_refsel_ref_get()
inline helpers for reading cached state, and zl3073x_chan_state_update()
for refreshing both registers from hardware. Call it from
zl3073x_chan_state_fetch() as well so that channel status is
initialized at device startup.

Call zl3073x_dev_chan_states_update() from the periodic work to
keep the cached state up to date and convert
zl3073x_dpll_lock_status_get() and zl3073x_dpll_selected_ref_get()
to use the cached state via the new helpers instead of direct register
reads.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/chan.c | 32 +++++++++++++++++++++++
 drivers/dpll/zl3073x/chan.h | 52 +++++++++++++++++++++++++++++++++++++
 drivers/dpll/zl3073x/core.c | 17 ++++++++++++
 drivers/dpll/zl3073x/dpll.c | 41 +++++++----------------------
 4 files changed, 111 insertions(+), 31 deletions(-)

diff --git a/drivers/dpll/zl3073x/chan.c b/drivers/dpll/zl3073x/chan.c
index 6f383d489fab7..10189c8a5ded3 100644
--- a/drivers/dpll/zl3073x/chan.c
+++ b/drivers/dpll/zl3073x/chan.c
@@ -7,6 +7,27 @@
 #include "chan.h"
 #include "core.h"
 
+/**
+ * zl3073x_chan_state_update - update DPLL channel status from HW
+ * @zldev: pointer to zl3073x_dev structure
+ * @index: DPLL channel index
+ *
+ * Return: 0 on success, <0 on error
+ */
+int zl3073x_chan_state_update(struct zl3073x_dev *zldev, u8 index)
+{
+	struct zl3073x_chan *chan = &zldev->chan[index];
+	int rc;
+
+	rc = zl3073x_read_u8(zldev, ZL_REG_DPLL_MON_STATUS(index),
+			     &chan->mon_status);
+	if (rc)
+		return rc;
+
+	return zl3073x_read_u8(zldev, ZL_REG_DPLL_REFSEL_STATUS(index),
+			       &chan->refsel_status);
+}
+
 /**
  * zl3073x_chan_state_fetch - fetch DPLL channel state from hardware
  * @zldev: pointer to zl3073x_dev structure
@@ -30,6 +51,17 @@ int zl3073x_chan_state_fetch(struct zl3073x_dev *zldev, u8 index)
 	dev_dbg(zldev->dev, "DPLL%u mode: %u, ref: %u\n", index,
 		zl3073x_chan_mode_get(chan), zl3073x_chan_ref_get(chan));
 
+	rc = zl3073x_chan_state_update(zldev, index);
+	if (rc)
+		return rc;
+
+	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),
+		zl3073x_chan_is_ho_ready(chan) ? 1 : 0,
+		zl3073x_chan_refsel_state_get(chan),
+		zl3073x_chan_refsel_ref_get(chan));
+
 	return 0;
 }
 
diff --git a/drivers/dpll/zl3073x/chan.h b/drivers/dpll/zl3073x/chan.h
index 3e6ffaef0c743..f73a076108551 100644
--- a/drivers/dpll/zl3073x/chan.h
+++ b/drivers/dpll/zl3073x/chan.h
@@ -14,11 +14,17 @@ struct zl3073x_dev;
 /**
  * struct zl3073x_chan - DPLL channel state
  * @mode_refsel: mode and reference selection register value
+ * @mon_status: monitor status register value
+ * @refsel_status: reference selection status register value
  */
 struct zl3073x_chan {
 	struct_group(cfg,
 		u8	mode_refsel;
 	);
+	struct_group(stat,
+		u8	mon_status;
+		u8	refsel_status;
+	);
 };
 
 int zl3073x_chan_state_fetch(struct zl3073x_dev *zldev, u8 index);
@@ -27,6 +33,8 @@ const struct zl3073x_chan *zl3073x_chan_state_get(struct zl3073x_dev *zldev,
 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);
+
 /**
  * zl3073x_chan_mode_get - get DPLL channel operating mode
  * @chan: pointer to channel state
@@ -71,4 +79,48 @@ static inline void zl3073x_chan_ref_set(struct zl3073x_chan *chan, u8 ref)
 	chan->mode_refsel |= FIELD_PREP(ZL_DPLL_MODE_REFSEL_REF, ref);
 }
 
+/**
+ * zl3073x_chan_lock_state_get - get DPLL channel lock state
+ * @chan: pointer to channel state
+ *
+ * Return: lock state of the given DPLL channel
+ */
+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_ho_ready - check if holdover is ready
+ * @chan: pointer to channel state
+ *
+ * Return: true if holdover is ready, false otherwise
+ */
+static inline bool zl3073x_chan_is_ho_ready(const struct zl3073x_chan *chan)
+{
+	return !!FIELD_GET(ZL_DPLL_MON_STATUS_HO_READY, chan->mon_status);
+}
+
+/**
+ * zl3073x_chan_refsel_state_get - get reference selection state
+ * @chan: pointer to channel state
+ *
+ * Return: reference selection state of the given DPLL channel
+ */
+static inline u8 zl3073x_chan_refsel_state_get(const struct zl3073x_chan *chan)
+{
+	return FIELD_GET(ZL_DPLL_REFSEL_STATUS_STATE, chan->refsel_status);
+}
+
+/**
+ * zl3073x_chan_refsel_ref_get - get currently selected reference in auto mode
+ * @chan: pointer to channel state
+ *
+ * Return: reference selected by the DPLL in automatic mode
+ */
+static inline u8 zl3073x_chan_refsel_ref_get(const struct zl3073x_chan *chan)
+{
+	return FIELD_GET(ZL_DPLL_REFSEL_STATUS_REFSEL, chan->refsel_status);
+}
+
 #endif /* _ZL3073X_CHAN_H */
diff --git a/drivers/dpll/zl3073x/core.c b/drivers/dpll/zl3073x/core.c
index b03e59fa0834b..6363002d48d46 100644
--- a/drivers/dpll/zl3073x/core.c
+++ b/drivers/dpll/zl3073x/core.c
@@ -566,6 +566,20 @@ 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
  * @zldev: pointer to zl3073x_dev structure
@@ -691,6 +705,9 @@ 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 f56f073e57df4..49e3f9f130848 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -258,28 +258,16 @@ zl3073x_dpll_input_pin_frequency_set(const struct dpll_pin *dpll_pin,
 static int
 zl3073x_dpll_selected_ref_get(struct zl3073x_dpll *zldpll, u8 *ref)
 {
-	struct zl3073x_dev *zldev = zldpll->dev;
 	const struct zl3073x_chan *chan;
-	u8 state, value;
-	int rc;
 
-	chan = zl3073x_chan_state_get(zldev, zldpll->id);
+	chan = zl3073x_chan_state_get(zldpll->dev, zldpll->id);
 
 	switch (zl3073x_chan_mode_get(chan)) {
 	case ZL_DPLL_MODE_REFSEL_MODE_AUTO:
-		/* For automatic mode read refsel_status register */
-		rc = zl3073x_read_u8(zldev,
-				     ZL_REG_DPLL_REFSEL_STATUS(zldpll->id),
-				     &value);
-		if (rc)
-			return rc;
-
-		/* Extract reference state */
-		state = FIELD_GET(ZL_DPLL_REFSEL_STATUS_STATE, value);
-
 		/* Return the reference only if the DPLL is locked to it */
-		if (state == ZL_DPLL_REFSEL_STATUS_STATE_LOCK)
-			*ref = FIELD_GET(ZL_DPLL_REFSEL_STATUS_REFSEL, value);
+		if (zl3073x_chan_refsel_state_get(chan) ==
+		    ZL_DPLL_REFSEL_STATUS_STATE_LOCK)
+			*ref = zl3073x_chan_refsel_ref_get(chan);
 		else
 			*ref = ZL3073X_DPLL_REF_NONE;
 		break;
@@ -1089,12 +1077,9 @@ zl3073x_dpll_lock_status_get(const struct dpll_device *dpll, void *dpll_priv,
 			     struct netlink_ext_ack *extack)
 {
 	struct zl3073x_dpll *zldpll = dpll_priv;
-	struct zl3073x_dev *zldev = zldpll->dev;
 	const struct zl3073x_chan *chan;
-	u8 mon_status, state;
-	int rc;
 
-	chan = zl3073x_chan_state_get(zldev, zldpll->id);
+	chan = zl3073x_chan_state_get(zldpll->dev, zldpll->id);
 
 	switch (zl3073x_chan_mode_get(chan)) {
 	case ZL_DPLL_MODE_REFSEL_MODE_FREERUN:
@@ -1107,16 +1092,9 @@ zl3073x_dpll_lock_status_get(const struct dpll_device *dpll, void *dpll_priv,
 		break;
 	}
 
-	/* Read DPLL monitor status */
-	rc = zl3073x_read_u8(zldev, ZL_REG_DPLL_MON_STATUS(zldpll->id),
-			     &mon_status);
-	if (rc)
-		return rc;
-	state = FIELD_GET(ZL_DPLL_MON_STATUS_STATE, mon_status);
-
-	switch (state) {
+	switch (zl3073x_chan_lock_state_get(chan)) {
 	case ZL_DPLL_MON_STATUS_STATE_LOCK:
-		if (FIELD_GET(ZL_DPLL_MON_STATUS_HO_READY, mon_status))
+		if (zl3073x_chan_is_ho_ready(chan))
 			*status = DPLL_LOCK_STATUS_LOCKED_HO_ACQ;
 		else
 			*status = DPLL_LOCK_STATUS_LOCKED;
@@ -1126,8 +1104,9 @@ zl3073x_dpll_lock_status_get(const struct dpll_device *dpll, void *dpll_priv,
 		*status = DPLL_LOCK_STATUS_HOLDOVER;
 		break;
 	default:
-		dev_warn(zldev->dev, "Unknown DPLL monitor status: 0x%02x\n",
-			 mon_status);
+		dev_warn(zldpll->dev->dev,
+			 "Unknown DPLL monitor status: 0x%02x\n",
+			 chan->mon_status);
 		*status = DPLL_LOCK_STATUS_UNLOCKED;
 		break;
 	}
-- 
2.52.0

[PATCH net-next 5/6] dpll: zl3073x: add reference priority to zl3073x_chan

[Ivan Vecera]

Cache the ZL_REG_DPLL_REF_PRIO registers in the zl3073x_chan cfg group.
These mailbox-based registers store per-reference priority values
(4 bits each, P/N packed) used for automatic reference selection.

Add ref_prio[] array to struct zl3073x_chan and provide inline helpers
zl3073x_chan_ref_prio_get(), zl3073x_chan_ref_prio_set(), and
zl3073x_chan_ref_is_selectable() for nibble-level access and priority
queries. Extend state_fetch and state_set with DPLL mailbox operations
to read and write the priority registers.

Replace the ad-hoc zl3073x_dpll_ref_prio_get/set functions in dpll.c
with the cached state pattern, removing direct mailbox access from
the DPLL layer. This also simplifies pin registration since reading
priority from cached state cannot fail.

Remove the pin->selectable flag from struct zl3073x_dpll_pin and
derive the selectable state from the cached ref priority via
zl3073x_chan_ref_is_selectable(), eliminating a redundant cache.

Inline zl3073x_dpll_selected_ref_set() into
zl3073x_dpll_input_pin_state_on_dpll_set(), unifying all manual and
automatic mode paths to commit changes through a single
zl3073x_chan_state_set() call at the end of the function.

Move hardware limit constants from core.h to regs.h so that chan.h
can reference ZL3073X_NUM_REFS for the ref_prio array size.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/chan.c |  67 +++++++-
 drivers/dpll/zl3073x/chan.h |  53 +++++++
 drivers/dpll/zl3073x/core.h |  11 --
 drivers/dpll/zl3073x/dpll.c | 299 +++++++++---------------------------
 drivers/dpll/zl3073x/regs.h |  12 ++
 5 files changed, 201 insertions(+), 241 deletions(-)

diff --git a/drivers/dpll/zl3073x/chan.c b/drivers/dpll/zl3073x/chan.c
index 10189c8a5ded3..74d00cee624ef 100644
--- a/drivers/dpll/zl3073x/chan.c
+++ b/drivers/dpll/zl3073x/chan.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 
+#include <linux/cleanup.h>
 #include <linux/dev_printk.h>
 #include <linux/string.h>
 #include <linux/types.h>
@@ -33,15 +34,15 @@ int zl3073x_chan_state_update(struct zl3073x_dev *zldev, u8 index)
  * @zldev: pointer to zl3073x_dev structure
  * @index: DPLL channel index to fetch state for
  *
- * Reads the mode_refsel register for the given DPLL channel and stores
- * the raw value for later use.
+ * Reads the mode_refsel register and reference priority registers for
+ * the given DPLL channel and stores the raw values for later use.
  *
  * Return: 0 on success, <0 on error
  */
 int zl3073x_chan_state_fetch(struct zl3073x_dev *zldev, u8 index)
 {
 	struct zl3073x_chan *chan = &zldev->chan[index];
-	int rc;
+	int rc, i;
 
 	rc = zl3073x_read_u8(zldev, ZL_REG_DPLL_MODE_REFSEL(index),
 			     &chan->mode_refsel);
@@ -62,6 +63,22 @@ int zl3073x_chan_state_fetch(struct zl3073x_dev *zldev, u8 index)
 		zl3073x_chan_refsel_state_get(chan),
 		zl3073x_chan_refsel_ref_get(chan));
 
+	guard(mutex)(&zldev->multiop_lock);
+
+	/* Read DPLL configuration from mailbox */
+	rc = zl3073x_mb_op(zldev, ZL_REG_DPLL_MB_SEM, ZL_DPLL_MB_SEM_RD,
+			   ZL_REG_DPLL_MB_MASK, BIT(index));
+	if (rc)
+		return rc;
+
+	/* Read reference priority registers */
+	for (i = 0; i < ARRAY_SIZE(chan->ref_prio); i++) {
+		rc = zl3073x_read_u8(zldev, ZL_REG_DPLL_REF_PRIO(i),
+				     &chan->ref_prio[i]);
+		if (rc)
+			return rc;
+	}
+
 	return 0;
 }
 
@@ -85,7 +102,9 @@ const struct zl3073x_chan *zl3073x_chan_state_get(struct zl3073x_dev *zldev,
  * @chan: desired channel state
  *
  * Skips the HW write if the configuration is unchanged, and otherwise
- * writes the mode_refsel register to hardware.
+ * writes only the changed registers to hardware. The mode_refsel register
+ * is written directly, while the reference priority registers are written
+ * via the DPLL mailbox interface.
  *
  * Return: 0 on success, <0 on HW error
  */
@@ -93,14 +112,48 @@ int zl3073x_chan_state_set(struct zl3073x_dev *zldev, u8 index,
 			   const struct zl3073x_chan *chan)
 {
 	struct zl3073x_chan *dchan = &zldev->chan[index];
-	int rc;
+	int rc, i;
 
 	/* Skip HW write if configuration hasn't changed */
 	if (!memcmp(&dchan->cfg, &chan->cfg, sizeof(chan->cfg)))
 		return 0;
 
-	rc = zl3073x_write_u8(zldev, ZL_REG_DPLL_MODE_REFSEL(index),
-			      chan->mode_refsel);
+	/* Direct register write for mode_refsel */
+	if (dchan->mode_refsel != chan->mode_refsel) {
+		rc = zl3073x_write_u8(zldev, ZL_REG_DPLL_MODE_REFSEL(index),
+				      chan->mode_refsel);
+		if (rc)
+			return rc;
+	}
+
+	/* Mailbox write for ref_prio if changed */
+	if (!memcmp(dchan->ref_prio, chan->ref_prio, sizeof(chan->ref_prio))) {
+		dchan->cfg = chan->cfg;
+		return 0;
+	}
+
+	guard(mutex)(&zldev->multiop_lock);
+
+	/* Read DPLL configuration into mailbox */
+	rc = zl3073x_mb_op(zldev, ZL_REG_DPLL_MB_SEM, ZL_DPLL_MB_SEM_RD,
+			   ZL_REG_DPLL_MB_MASK, BIT(index));
+	if (rc)
+		return rc;
+
+	/* Update changed ref_prio registers */
+	for (i = 0; i < ARRAY_SIZE(chan->ref_prio); i++) {
+		if (dchan->ref_prio[i] != chan->ref_prio[i]) {
+			rc = zl3073x_write_u8(zldev,
+					      ZL_REG_DPLL_REF_PRIO(i),
+					      chan->ref_prio[i]);
+			if (rc)
+				return rc;
+		}
+	}
+
+	/* Commit DPLL configuration */
+	rc = zl3073x_mb_op(zldev, ZL_REG_DPLL_MB_SEM, ZL_DPLL_MB_SEM_WR,
+			   ZL_REG_DPLL_MB_MASK, BIT(index));
 	if (rc)
 		return rc;
 
diff --git a/drivers/dpll/zl3073x/chan.h b/drivers/dpll/zl3073x/chan.h
index f73a076108551..e0f02d3432086 100644
--- a/drivers/dpll/zl3073x/chan.h
+++ b/drivers/dpll/zl3073x/chan.h
@@ -14,12 +14,14 @@ struct zl3073x_dev;
 /**
  * struct zl3073x_chan - DPLL channel state
  * @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
  * @refsel_status: reference selection status register value
  */
 struct zl3073x_chan {
 	struct_group(cfg,
 		u8	mode_refsel;
+		u8	ref_prio[ZL3073X_NUM_REFS / 2];
 	);
 	struct_group(stat,
 		u8	mon_status;
@@ -79,6 +81,57 @@ static inline void zl3073x_chan_ref_set(struct zl3073x_chan *chan, u8 ref)
 	chan->mode_refsel |= FIELD_PREP(ZL_DPLL_MODE_REFSEL_REF, ref);
 }
 
+/**
+ * zl3073x_chan_ref_prio_get - get reference priority
+ * @chan: pointer to channel state
+ * @ref: input reference index
+ *
+ * Return: priority of the given reference <0, 15>
+ */
+static inline u8
+zl3073x_chan_ref_prio_get(const struct zl3073x_chan *chan, u8 ref)
+{
+	u8 val = chan->ref_prio[ref / 2];
+
+	if (!(ref & 1))
+		return FIELD_GET(ZL_DPLL_REF_PRIO_REF_P, val);
+	else
+		return FIELD_GET(ZL_DPLL_REF_PRIO_REF_N, val);
+}
+
+/**
+ * zl3073x_chan_ref_prio_set - set reference priority
+ * @chan: pointer to channel state
+ * @ref: input reference index
+ * @prio: priority to set
+ */
+static inline void
+zl3073x_chan_ref_prio_set(struct zl3073x_chan *chan, u8 ref, u8 prio)
+{
+	u8 *val = &chan->ref_prio[ref / 2];
+
+	if (!(ref & 1)) {
+		*val &= ~ZL_DPLL_REF_PRIO_REF_P;
+		*val |= FIELD_PREP(ZL_DPLL_REF_PRIO_REF_P, prio);
+	} else {
+		*val &= ~ZL_DPLL_REF_PRIO_REF_N;
+		*val |= FIELD_PREP(ZL_DPLL_REF_PRIO_REF_N, prio);
+	}
+}
+
+/**
+ * zl3073x_chan_ref_is_selectable - check if reference is selectable
+ * @chan: pointer to channel state
+ * @ref: input reference index
+ *
+ * Return: true if the reference priority is not NONE, false otherwise
+ */
+static inline bool
+zl3073x_chan_ref_is_selectable(const struct zl3073x_chan *chan, u8 ref)
+{
+	return zl3073x_chan_ref_prio_get(chan, ref) != ZL_DPLL_REF_PRIO_NONE;
+}
+
 /**
  * zl3073x_chan_lock_state_get - get DPLL channel lock state
  * @chan: pointer to channel state
diff --git a/drivers/dpll/zl3073x/core.h b/drivers/dpll/zl3073x/core.h
index 2cfb9dd74aa53..99440620407da 100644
--- a/drivers/dpll/zl3073x/core.h
+++ b/drivers/dpll/zl3073x/core.h
@@ -19,17 +19,6 @@ struct device;
 struct regmap;
 struct zl3073x_dpll;
 
-/*
- * Hardware limits for ZL3073x chip family
- */
-#define ZL3073X_MAX_CHANNELS	5
-#define ZL3073X_NUM_REFS	10
-#define ZL3073X_NUM_OUTS	10
-#define ZL3073X_NUM_SYNTHS	5
-#define ZL3073X_NUM_INPUT_PINS	ZL3073X_NUM_REFS
-#define ZL3073X_NUM_OUTPUT_PINS	(ZL3073X_NUM_OUTS * 2)
-#define ZL3073X_NUM_PINS	(ZL3073X_NUM_INPUT_PINS + \
-				 ZL3073X_NUM_OUTPUT_PINS)
 
 enum zl3073x_flags {
 	ZL3073X_FLAG_REF_PHASE_COMP_32_BIT,
diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
index 49e3f9f130848..31e4ccff96d89 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -34,7 +34,6 @@
  * @dir: pin direction
  * @id: pin id
  * @prio: pin priority <0, 14>
- * @selectable: pin is selectable in automatic mode
  * @esync_control: embedded sync is controllable
  * @phase_gran: phase adjustment granularity
  * @pin_state: last saved pin state
@@ -50,7 +49,6 @@ struct zl3073x_dpll_pin {
 	enum dpll_pin_direction	dir;
 	u8			id;
 	u8			prio;
-	bool			selectable;
 	bool			esync_control;
 	s32			phase_gran;
 	enum dpll_pin_state	pin_state;
@@ -284,71 +282,6 @@ zl3073x_dpll_selected_ref_get(struct zl3073x_dpll *zldpll, u8 *ref)
 	return 0;
 }
 
-/**
- * zl3073x_dpll_selected_ref_set - select reference in manual mode
- * @zldpll: pointer to zl3073x_dpll
- * @ref: input reference to be selected
- *
- * Selects the given reference for the DPLL channel it should be
- * locked to.
- *
- * Return: 0 on success, <0 on error
- */
-static int
-zl3073x_dpll_selected_ref_set(struct zl3073x_dpll *zldpll, u8 ref)
-{
-	struct zl3073x_dev *zldev = zldpll->dev;
-	struct zl3073x_chan chan;
-	u8 mode;
-
-	chan = *zl3073x_chan_state_get(zldev, zldpll->id);
-	mode = zl3073x_chan_mode_get(&chan);
-
-	switch (mode) {
-	case ZL_DPLL_MODE_REFSEL_MODE_REFLOCK:
-		/* Manual mode with ref selected */
-		if (ref == ZL3073X_DPLL_REF_NONE) {
-			switch (zldpll->lock_status) {
-			case DPLL_LOCK_STATUS_LOCKED_HO_ACQ:
-			case DPLL_LOCK_STATUS_HOLDOVER:
-				/* Switch to forced holdover */
-				mode = ZL_DPLL_MODE_REFSEL_MODE_HOLDOVER;
-				break;
-			default:
-				/* Switch to freerun */
-				mode = ZL_DPLL_MODE_REFSEL_MODE_FREERUN;
-				break;
-			}
-			/* Keep selected reference */
-			ref = zl3073x_chan_ref_get(&chan);
-		} else if (ref == zl3073x_chan_ref_get(&chan)) {
-			/* No register update - same mode and same ref */
-			return 0;
-		}
-		break;
-	case ZL_DPLL_MODE_REFSEL_MODE_FREERUN:
-	case ZL_DPLL_MODE_REFSEL_MODE_HOLDOVER:
-		/* Manual mode without no ref */
-		if (ref == ZL3073X_DPLL_REF_NONE)
-			/* No register update - keep current mode */
-			return 0;
-
-		/* Switch to reflock mode and update ref selection */
-		mode = ZL_DPLL_MODE_REFSEL_MODE_REFLOCK;
-		break;
-	default:
-		/* For other modes like automatic or NCO ref cannot be selected
-		 * manually
-		 */
-		return -EOPNOTSUPP;
-	}
-
-	zl3073x_chan_mode_set(&chan, mode);
-	zl3073x_chan_ref_set(&chan, ref);
-
-	return zl3073x_chan_state_set(zldev, zldpll->id, &chan);
-}
-
 /**
  * zl3073x_dpll_connected_ref_get - get currently connected reference
  * @zldpll: pointer to zl3073x_dpll
@@ -497,98 +430,6 @@ zl3073x_dpll_input_pin_phase_adjust_set(const struct dpll_pin *dpll_pin,
 	return zl3073x_ref_state_set(zldev, ref_id, &ref);
 }
 
-/**
- * zl3073x_dpll_ref_prio_get - get priority for given input pin
- * @pin: pointer to pin
- * @prio: place to store priority
- *
- * Reads current priority for the given input pin and stores the value
- * to @prio.
- *
- * Return: 0 on success, <0 on error
- */
-static int
-zl3073x_dpll_ref_prio_get(struct zl3073x_dpll_pin *pin, u8 *prio)
-{
-	struct zl3073x_dpll *zldpll = pin->dpll;
-	struct zl3073x_dev *zldev = zldpll->dev;
-	u8 ref, ref_prio;
-	int rc;
-
-	guard(mutex)(&zldev->multiop_lock);
-
-	/* Read DPLL configuration */
-	rc = zl3073x_mb_op(zldev, ZL_REG_DPLL_MB_SEM, ZL_DPLL_MB_SEM_RD,
-			   ZL_REG_DPLL_MB_MASK, BIT(zldpll->id));
-	if (rc)
-		return rc;
-
-	/* Read reference priority - one value for P&N pins (4 bits/pin) */
-	ref = zl3073x_input_pin_ref_get(pin->id);
-	rc = zl3073x_read_u8(zldev, ZL_REG_DPLL_REF_PRIO(ref / 2),
-			     &ref_prio);
-	if (rc)
-		return rc;
-
-	/* Select nibble according pin type */
-	if (zl3073x_dpll_is_p_pin(pin))
-		*prio = FIELD_GET(ZL_DPLL_REF_PRIO_REF_P, ref_prio);
-	else
-		*prio = FIELD_GET(ZL_DPLL_REF_PRIO_REF_N, ref_prio);
-
-	return rc;
-}
-
-/**
- * zl3073x_dpll_ref_prio_set - set priority for given input pin
- * @pin: pointer to pin
- * @prio: place to store priority
- *
- * Sets priority for the given input pin.
- *
- * Return: 0 on success, <0 on error
- */
-static int
-zl3073x_dpll_ref_prio_set(struct zl3073x_dpll_pin *pin, u8 prio)
-{
-	struct zl3073x_dpll *zldpll = pin->dpll;
-	struct zl3073x_dev *zldev = zldpll->dev;
-	u8 ref, ref_prio;
-	int rc;
-
-	guard(mutex)(&zldev->multiop_lock);
-
-	/* Read DPLL configuration into mailbox */
-	rc = zl3073x_mb_op(zldev, ZL_REG_DPLL_MB_SEM, ZL_DPLL_MB_SEM_RD,
-			   ZL_REG_DPLL_MB_MASK, BIT(zldpll->id));
-	if (rc)
-		return rc;
-
-	/* Read reference priority - one value shared between P&N pins */
-	ref = zl3073x_input_pin_ref_get(pin->id);
-	rc = zl3073x_read_u8(zldev, ZL_REG_DPLL_REF_PRIO(ref / 2), &ref_prio);
-	if (rc)
-		return rc;
-
-	/* Update nibble according pin type */
-	if (zl3073x_dpll_is_p_pin(pin)) {
-		ref_prio &= ~ZL_DPLL_REF_PRIO_REF_P;
-		ref_prio |= FIELD_PREP(ZL_DPLL_REF_PRIO_REF_P, prio);
-	} else {
-		ref_prio &= ~ZL_DPLL_REF_PRIO_REF_N;
-		ref_prio |= FIELD_PREP(ZL_DPLL_REF_PRIO_REF_N, prio);
-	}
-
-	/* Update reference priority */
-	rc = zl3073x_write_u8(zldev, ZL_REG_DPLL_REF_PRIO(ref / 2), ref_prio);
-	if (rc)
-		return rc;
-
-	/* Commit configuration */
-	return zl3073x_mb_op(zldev, ZL_REG_DPLL_MB_SEM, ZL_DPLL_MB_SEM_WR,
-			     ZL_REG_DPLL_MB_MASK, BIT(zldpll->id));
-}
-
 /**
  * zl3073x_dpll_ref_state_get - get status for given input pin
  * @pin: pointer to pin
@@ -627,7 +468,8 @@ zl3073x_dpll_ref_state_get(struct zl3073x_dpll_pin *pin,
 	 * pin as selectable.
 	 */
 	if (zl3073x_chan_mode_get(chan) == ZL_DPLL_MODE_REFSEL_MODE_AUTO &&
-	    zl3073x_dev_ref_is_status_ok(zldev, ref) && pin->selectable) {
+	    zl3073x_dev_ref_is_status_ok(zldev, ref) &&
+	    zl3073x_chan_ref_is_selectable(chan, ref)) {
 		*state = DPLL_PIN_STATE_SELECTABLE;
 		return 0;
 	}
@@ -661,71 +503,81 @@ 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;
-	const struct zl3073x_chan *chan;
-	u8 new_ref;
+	struct zl3073x_chan chan;
+	u8 mode, ref;
 	int rc;
 
-	chan = zl3073x_chan_state_get(zldpll->dev, zldpll->id);
+	chan = *zl3073x_chan_state_get(zldpll->dev, zldpll->id);
+	ref = zl3073x_input_pin_ref_get(pin->id);
+	mode = zl3073x_chan_mode_get(&chan);
 
-	switch (zl3073x_chan_mode_get(chan)) {
+	switch (mode) {
 	case ZL_DPLL_MODE_REFSEL_MODE_REFLOCK:
-	case ZL_DPLL_MODE_REFSEL_MODE_FREERUN:
-	case ZL_DPLL_MODE_REFSEL_MODE_HOLDOVER:
 		if (state == DPLL_PIN_STATE_CONNECTED) {
 			/* Choose the pin as new selected reference */
-			new_ref = zl3073x_input_pin_ref_get(pin->id);
+			zl3073x_chan_ref_set(&chan, ref);
 		} else if (state == DPLL_PIN_STATE_DISCONNECTED) {
-			/* No reference */
-			new_ref = ZL3073X_DPLL_REF_NONE;
+			/* Choose new mode based on lock status */
+			switch (zldpll->lock_status) {
+			case DPLL_LOCK_STATUS_LOCKED_HO_ACQ:
+			case DPLL_LOCK_STATUS_HOLDOVER:
+				mode = ZL_DPLL_MODE_REFSEL_MODE_HOLDOVER;
+				break;
+			default:
+				mode = ZL_DPLL_MODE_REFSEL_MODE_FREERUN;
+				break;
+			}
+			zl3073x_chan_mode_set(&chan, mode);
 		} else {
-			NL_SET_ERR_MSG_MOD(extack,
-					   "Invalid pin state for manual mode");
-			return -EINVAL;
+			goto invalid_state;
+		}
+		break;
+	case ZL_DPLL_MODE_REFSEL_MODE_FREERUN:
+	case ZL_DPLL_MODE_REFSEL_MODE_HOLDOVER:
+		if (state == DPLL_PIN_STATE_CONNECTED) {
+			/* Choose the pin as new selected reference */
+			zl3073x_chan_ref_set(&chan, ref);
+			/* Switch to reflock mode */
+			zl3073x_chan_mode_set(&chan,
+					      ZL_DPLL_MODE_REFSEL_MODE_REFLOCK);
+		} else if (state != DPLL_PIN_STATE_DISCONNECTED) {
+			goto invalid_state;
 		}
-
-		rc = zl3073x_dpll_selected_ref_set(zldpll, new_ref);
 		break;
-
 	case ZL_DPLL_MODE_REFSEL_MODE_AUTO:
 		if (state == DPLL_PIN_STATE_SELECTABLE) {
-			if (pin->selectable)
+			if (zl3073x_chan_ref_is_selectable(&chan, ref))
 				return 0; /* Pin is already selectable */
 
 			/* Restore pin priority in HW */
-			rc = zl3073x_dpll_ref_prio_set(pin, pin->prio);
-			if (rc)
-				return rc;
-
-			/* Mark pin as selectable */
-			pin->selectable = true;
+			zl3073x_chan_ref_prio_set(&chan, ref, pin->prio);
 		} else if (state == DPLL_PIN_STATE_DISCONNECTED) {
-			if (!pin->selectable)
+			if (!zl3073x_chan_ref_is_selectable(&chan, ref))
 				return 0; /* Pin is already disconnected */
 
 			/* Set pin priority to none in HW */
-			rc = zl3073x_dpll_ref_prio_set(pin,
-						       ZL_DPLL_REF_PRIO_NONE);
-			if (rc)
-				return rc;
-
-			/* Mark pin as non-selectable */
-			pin->selectable = false;
+			zl3073x_chan_ref_prio_set(&chan, ref,
+						  ZL_DPLL_REF_PRIO_NONE);
 		} else {
-			NL_SET_ERR_MSG(extack,
-				       "Invalid pin state for automatic mode");
-			return -EINVAL;
+			goto invalid_state;
 		}
 		break;
-
 	default:
 		/* In other modes we cannot change input reference */
 		NL_SET_ERR_MSG(extack,
 			       "Pin state cannot be changed in current mode");
-		rc = -EOPNOTSUPP;
-		break;
+		return -EOPNOTSUPP;
 	}
 
-	return rc;
+	/* Commit DPLL channel changes */
+	rc = zl3073x_chan_state_set(zldpll->dev, zldpll->id, &chan);
+	if (rc)
+		return rc;
+
+	return 0;
+invalid_state:
+	NL_SET_ERR_MSG_MOD(extack, "Invalid pin state for this device mode");
+	return -EINVAL;
 }
 
 static int
@@ -745,15 +597,21 @@ zl3073x_dpll_input_pin_prio_set(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;
+	struct zl3073x_chan chan;
+	u8 ref;
 	int rc;
 
 	if (prio > ZL_DPLL_REF_PRIO_MAX)
 		return -EINVAL;
 
 	/* If the pin is selectable then update HW registers */
-	if (pin->selectable) {
-		rc = zl3073x_dpll_ref_prio_set(pin, prio);
+	chan = *zl3073x_chan_state_get(zldpll->dev, zldpll->id);
+	ref = zl3073x_input_pin_ref_get(pin->id);
+	if (zl3073x_chan_ref_is_selectable(&chan, ref)) {
+		zl3073x_chan_ref_prio_set(&chan, ref, prio);
+		rc = zl3073x_chan_state_set(zldpll->dev, zldpll->id, &chan);
 		if (rc)
 			return rc;
 	}
@@ -1235,6 +1093,8 @@ zl3073x_dpll_mode_set(const struct dpll_device *dpll, void *dpll_priv,
 		return rc;
 	}
 
+	chan = *zl3073x_chan_state_get(zldpll->dev, zldpll->id);
+
 	if (mode == DPLL_MODE_MANUAL) {
 		/* We are switching from automatic to manual mode:
 		 * - if we have a valid reference selected during auto mode then
@@ -1256,25 +1116,21 @@ zl3073x_dpll_mode_set(const struct dpll_device *dpll, void *dpll_priv,
 		 *   it is selectable after switch to automatic mode
 		 * - switch to automatic mode
 		 */
-		struct zl3073x_dpll_pin *pin;
-
-		pin = zl3073x_dpll_pin_get_by_ref(zldpll, ref);
-		if (pin && !pin->selectable) {
-			/* Restore pin priority in HW */
-			rc = zl3073x_dpll_ref_prio_set(pin, pin->prio);
-			if (rc) {
-				NL_SET_ERR_MSG_MOD(extack,
-						   "failed to restore pin priority");
-				return rc;
+		if (ZL3073X_DPLL_REF_IS_VALID(ref) &&
+		    !zl3073x_chan_ref_is_selectable(&chan, ref)) {
+			struct zl3073x_dpll_pin *pin;
+
+			pin = zl3073x_dpll_pin_get_by_ref(zldpll, ref);
+			if (pin) {
+				/* Restore pin priority in HW */
+				zl3073x_chan_ref_prio_set(&chan, ref,
+							 pin->prio);
 			}
-
-			pin->selectable = true;
 		}
 
 		hw_mode = ZL_DPLL_MODE_REFSEL_MODE_AUTO;
 	}
 
-	chan = *zl3073x_chan_state_get(zldpll->dev, zldpll->id);
 	zl3073x_chan_mode_set(&chan, hw_mode);
 	if (ZL3073X_DPLL_REF_IS_VALID(ref))
 		zl3073x_chan_ref_set(&chan, ref);
@@ -1426,18 +1282,16 @@ zl3073x_dpll_pin_register(struct zl3073x_dpll_pin *pin, u32 index)
 	pin->phase_gran = props->dpll_props.phase_gran;
 
 	if (zl3073x_dpll_is_input_pin(pin)) {
-		rc = zl3073x_dpll_ref_prio_get(pin, &pin->prio);
-		if (rc)
-			goto err_prio_get;
+		const struct zl3073x_chan *chan;
+		u8 ref;
+
+		chan = zl3073x_chan_state_get(zldpll->dev, zldpll->id);
+		ref = zl3073x_input_pin_ref_get(pin->id);
+		pin->prio = zl3073x_chan_ref_prio_get(chan, ref);
 
-		if (pin->prio == ZL_DPLL_REF_PRIO_NONE) {
-			/* Clamp prio to max value & mark pin non-selectable */
+		if (pin->prio == ZL_DPLL_REF_PRIO_NONE)
+			/* Clamp prio to max value */
 			pin->prio = ZL_DPLL_REF_PRIO_MAX;
-			pin->selectable = false;
-		} else {
-			/* Mark pin as selectable */
-			pin->selectable = true;
-		}
 	}
 
 	/* Create or get existing DPLL pin */
@@ -1466,7 +1320,6 @@ zl3073x_dpll_pin_register(struct zl3073x_dpll_pin *pin, u32 index)
 
 err_register:
 	dpll_pin_put(pin->dpll_pin, &pin->tracker);
-err_prio_get:
 	pin->dpll_pin = NULL;
 err_pin_get:
 	zl3073x_pin_props_put(props);
diff --git a/drivers/dpll/zl3073x/regs.h b/drivers/dpll/zl3073x/regs.h
index 19c598daa784c..5ae50cb761a97 100644
--- a/drivers/dpll/zl3073x/regs.h
+++ b/drivers/dpll/zl3073x/regs.h
@@ -6,6 +6,18 @@
 #include <linux/bitfield.h>
 #include <linux/bits.h>
 
+/*
+ * Hardware limits for ZL3073x chip family
+ */
+#define ZL3073X_MAX_CHANNELS	5
+#define ZL3073X_NUM_REFS	10
+#define ZL3073X_NUM_OUTS	10
+#define ZL3073X_NUM_SYNTHS	5
+#define ZL3073X_NUM_INPUT_PINS	ZL3073X_NUM_REFS
+#define ZL3073X_NUM_OUTPUT_PINS	(ZL3073X_NUM_OUTS * 2)
+#define ZL3073X_NUM_PINS	(ZL3073X_NUM_INPUT_PINS + \
+				 ZL3073X_NUM_OUTPUT_PINS)
+
 /*
  * Register address structure:
  * ===========================
-- 
2.52.0

[PATCH net-next 6/6] dpll: zl3073x: drop selected and simplify connected ref getter

[Ivan Vecera]

The HW reports the currently selected reference in the
dpll_refsel_status register regardless of the DPLL mode. Use this to
delete zl3073x_dpll_selected_ref_get() and have callers read the
register directly via the cached channel state.

Simplify zl3073x_dpll_connected_ref_get() to check refsel_state for
LOCK directly and return the reference index, changing the return
type from int to u8. The redundant ref_is_status_ok check is removed
since the DPLL cannot be in LOCK state with a failed reference.

In zl3073x_dpll_mode_set(), replace the selected_ref_get() call with
zl3073x_chan_refsel_ref_get() to read the currently selected
reference directly from the cached channel state.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/dpll.c | 94 +++++++------------------------------
 1 file changed, 18 insertions(+), 76 deletions(-)

diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
index 31e4ccff96d89..8a85e8be2d829 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -243,72 +243,27 @@ zl3073x_dpll_input_pin_frequency_set(const struct dpll_pin *dpll_pin,
 	return zl3073x_ref_state_set(zldev, ref_id, &ref);
 }
 
-/**
- * zl3073x_dpll_selected_ref_get - get currently selected reference
- * @zldpll: pointer to zl3073x_dpll
- * @ref: place to store selected reference
- *
- * Check for currently selected reference the DPLL should be locked to
- * and stores its index to given @ref.
- *
- * Return: 0 on success, <0 on error
- */
-static int
-zl3073x_dpll_selected_ref_get(struct zl3073x_dpll *zldpll, u8 *ref)
-{
-	const struct zl3073x_chan *chan;
-
-	chan = zl3073x_chan_state_get(zldpll->dev, zldpll->id);
-
-	switch (zl3073x_chan_mode_get(chan)) {
-	case ZL_DPLL_MODE_REFSEL_MODE_AUTO:
-		/* Return the reference only if the DPLL is locked to it */
-		if (zl3073x_chan_refsel_state_get(chan) ==
-		    ZL_DPLL_REFSEL_STATUS_STATE_LOCK)
-			*ref = zl3073x_chan_refsel_ref_get(chan);
-		else
-			*ref = ZL3073X_DPLL_REF_NONE;
-		break;
-	case ZL_DPLL_MODE_REFSEL_MODE_REFLOCK:
-		/* For manual mode return stored value */
-		*ref = zl3073x_chan_ref_get(chan);
-		break;
-	default:
-		/* For other modes like NCO, freerun... there is no input ref */
-		*ref = ZL3073X_DPLL_REF_NONE;
-		break;
-	}
-
-	return 0;
-}
-
 /**
  * zl3073x_dpll_connected_ref_get - get currently connected reference
  * @zldpll: pointer to zl3073x_dpll
- * @ref: place to store selected reference
  *
- * Looks for currently connected the DPLL is locked to and stores its index
- * to given @ref.
+ * Looks for currently connected reference the DPLL is locked to.
  *
- * Return: 0 on success, <0 on error
+ * Return: reference index if locked, ZL3073X_DPLL_REF_NONE otherwise
  */
-static int
-zl3073x_dpll_connected_ref_get(struct zl3073x_dpll *zldpll, u8 *ref)
+static u8
+zl3073x_dpll_connected_ref_get(struct zl3073x_dpll *zldpll)
 {
-	struct zl3073x_dev *zldev = zldpll->dev;
-	int rc;
-
-	/* Get currently selected input reference */
-	rc = zl3073x_dpll_selected_ref_get(zldpll, ref);
-	if (rc)
-		return rc;
+	const struct zl3073x_chan *chan = zl3073x_chan_state_get(zldpll->dev,
+								 zldpll->id);
+	u8 state;
 
-	/* If the monitor indicates an error nothing is connected */
-	if (ZL3073X_DPLL_REF_IS_VALID(*ref) &&
-	    !zl3073x_dev_ref_is_status_ok(zldev, *ref))
-		*ref = ZL3073X_DPLL_REF_NONE;
+	/* A reference is connected only when the DPLL is locked to it */
+	state = zl3073x_chan_refsel_state_get(chan);
+	if (state == ZL_DPLL_REFSEL_STATUS_STATE_LOCK)
+		return zl3073x_chan_refsel_ref_get(chan);
 
-	return 0;
+	return ZL3073X_DPLL_REF_NONE;
 }
 
 static int
@@ -324,12 +279,9 @@ zl3073x_dpll_input_pin_phase_offset_get(const struct dpll_pin *dpll_pin,
 	const struct zl3073x_ref *ref;
 	u8 conn_id, ref_id;
 	s64 ref_phase;
-	int rc;
 
 	/* Get currently connected reference */
-	rc = zl3073x_dpll_connected_ref_get(zldpll, &conn_id);
-	if (rc)
-		return rc;
+	conn_id = zl3073x_dpll_connected_ref_get(zldpll);
 
 	/* Report phase offset only for currently connected pin if the phase
 	 * monitor feature is disabled and only if the input pin signal is
@@ -367,7 +319,7 @@ zl3073x_dpll_input_pin_phase_offset_get(const struct dpll_pin *dpll_pin,
 
 	*phase_offset = ref_phase * DPLL_PHASE_OFFSET_DIVIDER;
 
-	return rc;
+	return 0;
 }
 
 static int
@@ -447,18 +399,13 @@ zl3073x_dpll_ref_state_get(struct zl3073x_dpll_pin *pin,
 	struct zl3073x_dpll *zldpll = pin->dpll;
 	struct zl3073x_dev *zldev = zldpll->dev;
 	const struct zl3073x_chan *chan;
-	u8 ref, ref_conn;
-	int rc;
+	u8 ref;
 
 	chan = zl3073x_chan_state_get(zldev, zldpll->id);
 	ref = zl3073x_input_pin_ref_get(pin->id);
 
-	/* Get currently connected reference */
-	rc = zl3073x_dpll_connected_ref_get(zldpll, &ref_conn);
-	if (rc)
-		return rc;
-
-	if (ref == ref_conn) {
+	/* Check if the pin reference is connected */
+	if (ref == zl3073x_dpll_connected_ref_get(zldpll)) {
 		*state = DPLL_PIN_STATE_CONNECTED;
 		return 0;
 	}
@@ -1087,13 +1034,8 @@ zl3073x_dpll_mode_set(const struct dpll_device *dpll, void *dpll_priv,
 	u8 hw_mode, ref;
 	int rc;
 
-	rc = zl3073x_dpll_selected_ref_get(zldpll, &ref);
-	if (rc) {
-		NL_SET_ERR_MSG_MOD(extack, "failed to get selected reference");
-		return rc;
-	}
-
 	chan = *zl3073x_chan_state_get(zldpll->dev, zldpll->id);
+	ref = zl3073x_chan_refsel_ref_get(&chan);
 
 	if (mode == DPLL_MODE_MANUAL) {
 		/* We are switching from automatic to manual mode:
-- 
2.52.0

Re: [net-next,5/6] dpll: zl3073x: add reference priority to zl3073x_chan

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email says "Is this legit? Quite hard to follow the logic of this change :/"

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
dpll: zl3073x: add reference priority to zl3073x_chan

This commit caches the ZL_REG_DPLL_REF_PRIO registers in the zl3073x_chan
cfg group, adding a ref_prio[] array and inline helpers for accessing
priority values. It extends state_fetch and state_set with DPLL mailbox
operations to read and write these priority registers.

> diff --git a/drivers/dpll/zl3073x/chan.c b/drivers/dpll/zl3073x/chan.c
> index 10189c8a5ded..74d00cee624e 100644
> --- a/drivers/dpll/zl3073x/chan.c
> +++ b/drivers/dpll/zl3073x/chan.c

[ ... ]

> @@ -85,7 +102,9 @@ const struct zl3073x_chan *zl3073x_chan_state_get(struct zl3073x_dev *zldev,
>   * @chan: desired channel state
>   *
>   * Skips the HW write if the configuration is unchanged, and otherwise
> - * writes the mode_refsel register to hardware.
> + * writes only the changed registers to hardware. The mode_refsel register
> + * is written directly, while the reference priority registers are written
> + * via the DPLL mailbox interface.
>   *
>   * Return: 0 on success, <0 on HW error
>   */
> @@ -93,14 +112,48 @@ int zl3073x_chan_state_set(struct zl3073x_dev *zldev, u8 index,
>  			   const struct zl3073x_chan *chan)
>  {
>  	struct zl3073x_chan *dchan = &zldev->chan[index];
> -	int rc;
> +	int rc, i;
>
>  	/* Skip HW write if configuration hasn't changed */
>  	if (!memcmp(&dchan->cfg, &chan->cfg, sizeof(chan->cfg)))
>  		return 0;
>
> -	rc = zl3073x_write_u8(zldev, ZL_REG_DPLL_MODE_REFSEL(index),
> -			      chan->mode_refsel);
> +	/* Direct register write for mode_refsel */
> +	if (dchan->mode_refsel != chan->mode_refsel) {
> +		rc = zl3073x_write_u8(zldev, ZL_REG_DPLL_MODE_REFSEL(index),
> +				      chan->mode_refsel);
> +		if (rc)
> +			return rc;
> +	}
> +
> +	/* Mailbox write for ref_prio if changed */
> +	if (!memcmp(dchan->ref_prio, chan->ref_prio, sizeof(chan->ref_prio))) {
> +		dchan->cfg = chan->cfg;
> +		return 0;
> +	}
> +
> +	guard(mutex)(&zldev->multiop_lock);
> +
> +	/* Read DPLL configuration into mailbox */
> +	rc = zl3073x_mb_op(zldev, ZL_REG_DPLL_MB_SEM, ZL_DPLL_MB_SEM_RD,
> +			   ZL_REG_DPLL_MB_MASK, BIT(index));
> +	if (rc)
> +		return rc;
> +
> +	/* Update changed ref_prio registers */
> +	for (i = 0; i < ARRAY_SIZE(chan->ref_prio); i++) {
> +		if (dchan->ref_prio[i] != chan->ref_prio[i]) {
> +			rc = zl3073x_write_u8(zldev,
> +					      ZL_REG_DPLL_REF_PRIO(i),
> +					      chan->ref_prio[i]);
> +			if (rc)
> +				return rc;
> +		}
> +	}
> +
> +	/* Commit DPLL configuration */
> +	rc = zl3073x_mb_op(zldev, ZL_REG_DPLL_MB_SEM, ZL_DPLL_MB_SEM_WR,
> +			   ZL_REG_DPLL_MB_MASK, BIT(index));
>  	if (rc)
>  		return rc;
>

Could this function leave the cache and hardware out of sync?

When both mode_refsel and ref_prio have changed, the function writes
mode_refsel to hardware first. If the subsequent mailbox operation fails
(during the mailbox read, a ref_prio write, or the mailbox commit), the
function returns an error without executing 'dchan->cfg = chan->cfg'.

This leaves dchan->mode_refsel cached at the old value while the hardware
has the new value:

zl3073x_chan_state_set()
    dchan->mode_refsel != chan->mode_refsel is true
    zl3073x_write_u8(... mode_refsel) succeeds
        hardware now has new mode_refsel value
    zl3073x_mb_op(...READ...) fails
    return rc without updating dchan->cfg
        dchan->mode_refsel still has old value

Later calls that get state via zl3073x_chan_state_get() will see the stale
cached mode_refsel. If they try to set mode_refsel back to the old value,
the comparison 'dchan->mode_refsel != chan->mode_refsel' evaluates to false
(both match the old value), skipping the write, while hardware retains the
new value.

Before this patch, mode_refsel was written in a single atomic step with no
possibility of partial failure, so this desynchronization scenario couldn't
occur.

Re: [net-next,5/6] dpll: zl3073x: add reference priority to zl3073x_chan

[Ivan Vecera]

On 3/14/26 8:53 PM, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email says "Is this legit? Quite hard to follow the logic of this change :/"
> 
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> dpll: zl3073x: add reference priority to zl3073x_chan
> 
> This commit caches the ZL_REG_DPLL_REF_PRIO registers in the zl3073x_chan
> cfg group, adding a ref_prio[] array and inline helpers for accessing
> priority values. It extends state_fetch and state_set with DPLL mailbox
> operations to read and write these priority registers.
> 
>> diff --git a/drivers/dpll/zl3073x/chan.c b/drivers/dpll/zl3073x/chan.c
>> index 10189c8a5ded..74d00cee624e 100644
>> --- a/drivers/dpll/zl3073x/chan.c
>> +++ b/drivers/dpll/zl3073x/chan.c
> 
> [ ... ]
> 
>> @@ -85,7 +102,9 @@ const struct zl3073x_chan *zl3073x_chan_state_get(struct zl3073x_dev *zldev,
>>    * @chan: desired channel state
>>    *
>>    * Skips the HW write if the configuration is unchanged, and otherwise
>> - * writes the mode_refsel register to hardware.
>> + * writes only the changed registers to hardware. The mode_refsel register
>> + * is written directly, while the reference priority registers are written
>> + * via the DPLL mailbox interface.
>>    *
>>    * Return: 0 on success, <0 on HW error
>>    */
>> @@ -93,14 +112,48 @@ int zl3073x_chan_state_set(struct zl3073x_dev *zldev, u8 index,
>>   			   const struct zl3073x_chan *chan)
>>   {
>>   	struct zl3073x_chan *dchan = &zldev->chan[index];
>> -	int rc;
>> +	int rc, i;
>>
>>   	/* Skip HW write if configuration hasn't changed */
>>   	if (!memcmp(&dchan->cfg, &chan->cfg, sizeof(chan->cfg)))
>>   		return 0;
>>
>> -	rc = zl3073x_write_u8(zldev, ZL_REG_DPLL_MODE_REFSEL(index),
>> -			      chan->mode_refsel);
>> +	/* Direct register write for mode_refsel */
>> +	if (dchan->mode_refsel != chan->mode_refsel) {
>> +		rc = zl3073x_write_u8(zldev, ZL_REG_DPLL_MODE_REFSEL(index),
>> +				      chan->mode_refsel);
>> +		if (rc)
>> +			return rc;
>> +	}
>> +
>> +	/* Mailbox write for ref_prio if changed */
>> +	if (!memcmp(dchan->ref_prio, chan->ref_prio, sizeof(chan->ref_prio))) {
>> +		dchan->cfg = chan->cfg;
>> +		return 0;
>> +	}
>> +
>> +	guard(mutex)(&zldev->multiop_lock);
>> +
>> +	/* Read DPLL configuration into mailbox */
>> +	rc = zl3073x_mb_op(zldev, ZL_REG_DPLL_MB_SEM, ZL_DPLL_MB_SEM_RD,
>> +			   ZL_REG_DPLL_MB_MASK, BIT(index));
>> +	if (rc)
>> +		return rc;
>> +
>> +	/* Update changed ref_prio registers */
>> +	for (i = 0; i < ARRAY_SIZE(chan->ref_prio); i++) {
>> +		if (dchan->ref_prio[i] != chan->ref_prio[i]) {
>> +			rc = zl3073x_write_u8(zldev,
>> +					      ZL_REG_DPLL_REF_PRIO(i),
>> +					      chan->ref_prio[i]);
>> +			if (rc)
>> +				return rc;
>> +		}
>> +	}
>> +
>> +	/* Commit DPLL configuration */
>> +	rc = zl3073x_mb_op(zldev, ZL_REG_DPLL_MB_SEM, ZL_DPLL_MB_SEM_WR,
>> +			   ZL_REG_DPLL_MB_MASK, BIT(index));
>>   	if (rc)
>>   		return rc;
>>
> 
> Could this function leave the cache and hardware out of sync?
> 
> When both mode_refsel and ref_prio have changed, the function writes
> mode_refsel to hardware first. If the subsequent mailbox operation fails
> (during the mailbox read, a ref_prio write, or the mailbox commit), the
> function returns an error without executing 'dchan->cfg = chan->cfg'.

This potential situation is very unusual (mode_refsel updated and
mailbox access erroneous) but yes, after successful write of mode_refsel
dchan->mon_refsel should be updated to avoid this issue.

Will fix in v2.

Ivan