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.

quoted

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

[ ... ]

quoted

@@ -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