[PATCH net-next v11 08/13] dpll: sit9531x: add support to get and set priority on input pins

[Ali Rouhi <arouhi@sitime.com>]

From: Oleg Zadorozhnyi <Oleg.Zadorozhnyi@devoxsoftware.com>

When the device has to choose a reference, it takes the first slot of
its priority table whose signal is qualified.  The driver keeps a
priority per input and PLL and builds the table from it (previous
patch), so getting a priority reads that value without touching a
register, and setting one records it and, for an input the PLL lists,
rewrites the table in the new order.  When the new order puts a
different input with signal first, the PLL moves to it; a change further
down the table leaves the PLL where it is.

Priority and state are separate attributes, and neither moves the other.
A priority can be set on a disconnected input and takes effect when it is
connected; disconnecting an input and connecting it again brings back the
priority it had.  Setting one input's priority leaves every other input's
as it was, so only the named pin changes and the core's own notification
covers it.  The range is 0 to 255 against a table of eleven slots:
inputs of equal priority keep the order the table already has them in,
so distinct values are what fix an order.  An input the PLL has never
listed reports the lowest slot, the priority it would be connected at.

A table rewritten behind the driver -- a profile reload, a direct I2C
tool -- is taken as the new configuration and the priorities are
re-seeded from it, so the poll now watches priority and operational state
as well as state, and notifies a pin when any of them moves.  The
notifications go out after the walk over the pins, outside multiop_lock,
since the helper takes locks the callbacks run under.

Signed-off-by: Oleg Zadorozhnyi <Oleg.Zadorozhnyi@devoxsoftware.com>
Assisted-by: Claude:claude-4-opus [chat]
Signed-off-by: Ali Rouhi <arouhi@sitime.com>
---
 drivers/dpll/sit9531x/core.c |  38 +++++++++
 drivers/dpll/sit9531x/core.h |   2 +
 drivers/dpll/sit9531x/dpll.c | 152 ++++++++++++++++++++++++++++++-----
 drivers/dpll/sit9531x/dpll.h |   2 +
 drivers/dpll/sit9531x/prop.c |   4 +-
 5 files changed, 176 insertions(+), 22 deletions(-)

diff --git a/drivers/dpll/sit9531x/core.c b/drivers/dpll/sit9531x/core.c
index 231e2d71dd6f..f6c76cc78456 100644
--- a/drivers/dpll/sit9531x/core.c
+++ b/drivers/dpll/sit9531x/core.c
@@ -670,6 +670,44 @@ bool sit9531x_input_prio_present(struct sit9531x_dev *sitdev, u8 pll_idx,
 	return !!(sitdev->chan[pll_idx].prio_mask & BIT(input_idx));
 }
 
+/*
+ * sit9531x_input_prio_get - read an input's priority for a PLL
+ * @input_idx:	input source in hardware encoding (see
+ *		sit9531x_input_hw_src())
+ * @prio:	output priority (lower is preferred)
+ *
+ * Reports the priority configured for the source on this PLL, which is
+ * kept whether or not the source is currently in the table: state and
+ * priority are separate attributes, so disconnecting an input and
+ * connecting it again must not change the priority it reports.  The
+ * value is seeded from the hardware table, and re-seeded whenever the
+ * read-back shows the table was rewritten by something other than this
+ * driver.  A source that was never listed reports the lowest slot.
+ *
+ * Caller must hold sitdev->multiop_lock.
+ */
+int sit9531x_input_prio_get(struct sit9531x_dev *sitdev, u8 pll_idx,
+			    u8 input_idx, u8 *prio)
+{
+	const struct sit9531x_chan *chan;
+
+	lockdep_assert_held(&sitdev->multiop_lock);
+
+	if (pll_idx >= SIT9531X_NUM_PLLS)
+		return -EINVAL;
+	input_idx = sit9531x_prio_src_canon(sitdev, input_idx);
+	if (input_idx >= SIT9531X_PRIO_NUM_SRC)
+		return -EINVAL;
+
+	chan = &sitdev->chan[pll_idx];
+	if (chan->cfg_known & BIT(input_idx))
+		*prio = chan->cfg_prio[input_idx];
+	else
+		*prio = SIT9531X_PRIO_MAX_SLOTS - 1;
+
+	return 0;
+}
+
 /*
  * Take the configured priorities from a table the hardware holds: each
  * listed source gets the first slot it occupies.  A source the table does
diff --git a/drivers/dpll/sit9531x/core.h b/drivers/dpll/sit9531x/core.h
index adcdfc46c597..8aa505f84e87 100644
--- a/drivers/dpll/sit9531x/core.h
+++ b/drivers/dpll/sit9531x/core.h
@@ -250,6 +250,8 @@ bool sit9531x_input_prio_present(struct sit9531x_dev *sitdev,
 				 u8 pll_idx, u8 input_idx);
 int sit9531x_input_prio_set(struct sit9531x_dev *sitdev, u8 pll_idx,
 			    u8 input_idx, u8 prio);
+int sit9531x_input_prio_get(struct sit9531x_dev *sitdev, u8 pll_idx,
+			    u8 input_idx, u8 *prio);
 int sit9531x_input_prio_remove(struct sit9531x_dev *sitdev, u8 pll_idx,
 			       u8 input_idx);
 int sit9531x_input_prio_add(struct sit9531x_dev *sitdev, u8 pll_idx,
diff --git a/drivers/dpll/sit9531x/dpll.c b/drivers/dpll/sit9531x/dpll.c
index f961b7af28fe..ebbf7f5a3394 100644
--- a/drivers/dpll/sit9531x/dpll.c
+++ b/drivers/dpll/sit9531x/dpll.c
@@ -502,11 +502,93 @@ sit9531x_dpll_input_pin_state_on_dpll_set(const struct dpll_pin *pin,
 	return rc;
 }
 
+/*
+ * sit9531x_dpll_input_pin_prio_get - read input pin priority
+ *
+ * Reports the priority sit9531x_input_prio_get() keeps for the source on
+ * this PLL, connected or not; no register is read.
+ */
+static int
+sit9531x_dpll_input_pin_prio_get(const struct dpll_pin *pin, void *pin_priv,
+				 const struct dpll_device *dpll,
+				 void *dpll_priv, u32 *prio,
+				 struct netlink_ext_ack *extack)
+{
+	struct sit9531x_dpll_pin *dpin = pin_priv;
+	struct sit9531x_dpll *sitdpll = dpll_priv;
+	struct sit9531x_dev *sitdev = sitdpll->dev;
+	u8 slot;
+	int rc;
+
+	mutex_lock(&sitdev->multiop_lock);
+	rc = sit9531x_input_prio_get(sitdev, sitdpll->id,
+				     sit9531x_input_hw_src(dpin->id), &slot);
+	mutex_unlock(&sitdev->multiop_lock);
+	if (rc)
+		return rc;
+
+	/*
+	 * dpin->prio is not touched here: it is the poll's baseline for
+	 * spotting a change to notify, and a get refreshing it would hide
+	 * the change from the poll.
+	 */
+	*prio = slot;
+	return 0;
+}
+
+/*
+ * sit9531x_dpll_input_pin_prio_set - set input pin priority
+ *
+ * Records the priority and, for a pin in this PLL's table, rewrites the
+ * Page 1 table in priority order (sit9531x_input_prio_set()).  The other
+ * pins keep their priorities, so only the named pin changes and the core
+ * notifies it.  A pin that is not in the table keeps the priority for when
+ * it is connected.
+ */
+static int
+sit9531x_dpll_input_pin_prio_set(const struct dpll_pin *pin, void *pin_priv,
+				 const struct dpll_device *dpll,
+				 void *dpll_priv, u32 prio,
+				 struct netlink_ext_ack *extack)
+{
+	struct sit9531x_dpll_pin *dpin = pin_priv;
+	struct sit9531x_dpll *sitdpll = dpll_priv;
+	struct sit9531x_dev *sitdev = sitdpll->dev;
+	int rc;
+
+	if (dpin->dir != DPLL_PIN_DIRECTION_INPUT) {
+		NL_SET_ERR_MSG(extack, "Priority applies only to input pins");
+		return -EINVAL;
+	}
+
+	if (prio > U8_MAX) {
+		NL_SET_ERR_MSG(extack, "Priority out of range (0-255)");
+		return -EINVAL;
+	}
+
+	mutex_lock(&sitdev->multiop_lock);
+	rc = sit9531x_input_prio_set(sitdev, sitdpll->id,
+				     sit9531x_input_hw_src(dpin->id),
+				     (u8)prio);
+	if (!rc)
+		dpin->prio = prio;
+	mutex_unlock(&sitdev->multiop_lock);
+
+	if (rc) {
+		NL_SET_ERR_MSG(extack, "Failed to set input priority");
+		return rc;
+	}
+
+	return 0;
+}
+
 static const struct dpll_pin_ops sit9531x_dpll_input_pin_ops = {
 	.direction_get		= sit9531x_dpll_input_pin_direction_get,
 	.state_on_dpll_get	= sit9531x_dpll_input_pin_state_on_dpll_get,
 	.state_on_dpll_set	= sit9531x_dpll_input_pin_state_on_dpll_set,
 	.operstate_on_dpll_get	= sit9531x_dpll_input_pin_operstate_on_dpll_get,
+	.prio_get		= sit9531x_dpll_input_pin_prio_get,
+	.prio_set		= sit9531x_dpll_input_pin_prio_set,
 };
 
 /*
@@ -625,39 +707,67 @@ void sit9531x_dpll_changes_check(struct sit9531x_dpll *sitdpll)
 		dpll_device_change_ntf(sitdpll->dpll_dev);
 	}
 
+	mutex_lock(&sitdev->multiop_lock);
 	list_for_each_entry(pin, &sitdpll->pins, list) {
-		const struct dpll_pin_ops *ops;
+		enum dpll_pin_operstate operstate;
 		enum dpll_pin_state state;
 		bool changed;
+		u8 id, prio;
 
 		/*
-		 * Poll input pins whose state can change autonomously: regular
-		 * references and the INTSYNC destination pin.  Outputs (incl.
-		 * the INTSYNC source) change only through their own set
-		 * callback and the XO is permanently connected, so skip those.
-		 * Each pin's state_on_dpll_get resolves to the right getter.
+		 * Watch the selection-role pins -- regular references and the
+		 * INTSYNC destination -- whose state, operational state and
+		 * priority can move without a request: the device selects on
+		 * its own, the monitors follow the signal, and a table
+		 * rewritten behind the driver re-seeds the priorities.
+		 * Outputs (incl. the INTSYNC source) change only through their
+		 * own set callback and the XO is permanently connected.
 		 */
 		if (!sit9531x_dpll_is_input_pin(pin) ||
 		    sit9531x_dpll_is_xo_pin(pin))
 			continue;
 
-		ops = sit9531x_dpll_pin_ops_get(pin);
-		rc = ops->state_on_dpll_get(pin->dpll_pin, pin,
-					    sitdpll->dpll_dev, sitdpll,
-					    &state, NULL);
-		if (rc)
-			continue;
+		id = pin->id;
+		if (id == SIT9531X_INTSYNC_PIN_ID &&
+		    sitdev->intsync_src == sitdpll->id)
+			state = DPLL_PIN_STATE_DISCONNECTED;
+		else
+			sit9531x_dpll_selection_state_get(sitdev, sitdpll, id,
+							  &state);
+		sit9531x_dpll_selection_operstate_get(sitdev, sitdpll, id,
+						      &operstate);
+		if (sit9531x_input_prio_get(sitdev, sitdpll->id,
+					    sit9531x_input_hw_src(id), &prio))
+			prio = pin->prio;
+
+		changed = pin->seen &&
+			  (state != pin->pin_state ||
+			   operstate != pin->operstate || prio != pin->prio);
+		if (changed)
+			dev_dbg(sitdev->dev,
+				"%s: state %u->%u operstate %u->%u prio %u->%u\n",
+				pin->label, pin->pin_state, state,
+				pin->operstate, operstate, pin->prio, prio);
 
-		/*
-		 * The first pass only takes the baseline: the pin was
-		 * registered with this state, so nothing has changed yet.
-		 */
-		changed = pin->seen && state != pin->pin_state;
 		pin->pin_state = state;
+		pin->operstate = operstate;
+		pin->prio = prio;
 		pin->seen = true;
-		if (changed) {
-			dev_dbg(sitdev->dev, "%s state changed\n", pin->label);
-			dpll_pin_change_ntf(pin->dpll_pin);
-		}
+
+		/*
+		 * The notification helper takes DPLL-subsystem locks that the
+		 * callbacks run under, so it cannot be called with
+		 * multiop_lock held; mark the pin and send after the walk.
+		 */
+		if (changed)
+			pin->ntf_pending = true;
+	}
+	mutex_unlock(&sitdev->multiop_lock);
+
+	list_for_each_entry(pin, &sitdpll->pins, list) {
+		if (!pin->ntf_pending)
+			continue;
+		pin->ntf_pending = false;
+		dpll_pin_change_ntf(pin->dpll_pin);
 	}
 }
diff --git a/drivers/dpll/sit9531x/dpll.h b/drivers/dpll/sit9531x/dpll.h
index 1d320e183eb6..2e3df7608490 100644
--- a/drivers/dpll/sit9531x/dpll.h
+++ b/drivers/dpll/sit9531x/dpll.h
@@ -31,7 +31,9 @@ struct sit9531x_dpll_pin {
 	u8				id;		/* hardware index */
 	u8				prio;
 	enum dpll_pin_state		pin_state;
+	enum dpll_pin_operstate		operstate;
 	bool				seen;	/* baseline taken by the poll */
+	bool				ntf_pending;
 };
 
 /* Per-PLL DPLL device state. */
diff --git a/drivers/dpll/sit9531x/prop.c b/drivers/dpll/sit9531x/prop.c
index 3635ea497e41..934db566c002 100644
--- a/drivers/dpll/sit9531x/prop.c
+++ b/drivers/dpll/sit9531x/prop.c
@@ -187,10 +187,11 @@ sit9531x_pin_props_get(struct sit9531x_dev *sitdev,
 	    index == SIT9531X_INTSYNC_PIN_ID) {
 		/*
 		 * INTSYNC destination pin: a PLL locks to the INTSYNC net as a
-		 * reference, so it can be connected.
+		 * reference, so it can be connected and re-prioritised.
 		 */
 		props->dpll_props.type = DPLL_PIN_TYPE_INT_OSCILLATOR;
 		props->dpll_props.capabilities =
+			DPLL_PIN_CAPABILITIES_PRIORITY_CAN_CHANGE |
 			DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE;
 		curr_freq = 0;
 	} else if (dir == DPLL_PIN_DIRECTION_OUTPUT &&
@@ -214,6 +215,7 @@ sit9531x_pin_props_get(struct sit9531x_dev *sitdev,
 	} else if (dir == DPLL_PIN_DIRECTION_INPUT) {
 		props->dpll_props.type = DPLL_PIN_TYPE_EXT;
 		props->dpll_props.capabilities =
+			DPLL_PIN_CAPABILITIES_PRIORITY_CAN_CHANGE |
 			DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE;
 		curr_freq = sitdev->ref[index].freq;
 	} else {
-- 
2.43.0