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

Ivan Vecera posted 6 patches 3 weeks, 1 day ago
[PATCH net-next v2 1/6] dpll: zl3073x: use struct_group to partition states
Posted by Ivan Vecera 3 weeks, 1 day ago
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