[PATCH net-next v12 07/12] 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 that has signal.  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 the named pin is the only one whose priority changes and
the core's own notification covers that; when the new order moves the
active selection, the siblings' operational state follows on the next
poll.  A priority the table write refused is not kept: the configured
value goes back to what it was, so nothing reports a priority the
device never took.  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.

A pin's baseline for that watch -- state, operational state and priority
-- is taken when the pin is registered, under the device lock, rather
than by the first tick, so a change between the probe-time fetch and
that tick is reported instead of absorbed into the baseline.  The device
lock status is seeded the same way, so the first tick announces only
what moved since registration.

Signed-off-by: Oleg Zadorozhnyi <Oleg.Zadorozhnyi@devoxsoftware.com>
Assisted-by: LLM
Signed-off-by: Ali Rouhi <arouhi@sitime.com>
---
 drivers/dpll/sit9531x/core.c |  43 +++++++-
 drivers/dpll/sit9531x/core.h |   2 +
 drivers/dpll/sit9531x/dpll.c | 202 ++++++++++++++++++++++++++++++-----
 drivers/dpll/sit9531x/dpll.h |   4 +-
 drivers/dpll/sit9531x/prop.c |   4 +-
 5 files changed, 227 insertions(+), 28 deletions(-)

diff --git a/drivers/dpll/sit9531x/core.c b/drivers/dpll/sit9531x/core.c
index 999a6583853d..79520640dfc8 100644
--- a/drivers/dpll/sit9531x/core.c
+++ b/drivers/dpll/sit9531x/core.c
@@ -671,6 +671,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
@@ -2581,9 +2619,12 @@ static int sit9531x_dpll_pins_register(struct sit9531x_dpll *sitdpll)
 		/*
 		 * Pins registered before this one are already reachable from
 		 * netlink, and the walks over this list run under
-		 * multiop_lock, so the insertion does too.
+		 * multiop_lock, so the insertion does too.  The baseline the
+		 * poll compares the pin against is taken under the same
+		 * lock, from the cache the create notification just reported.
 		 */
 		mutex_lock(&sitdpll->dev->multiop_lock);
+		sit9531x_dpll_pin_baseline(pin);
 		list_add(&pin->list, &sitdpll->pins);
 		mutex_unlock(&sitdpll->dev->multiop_lock);
 	}
diff --git a/drivers/dpll/sit9531x/core.h b/drivers/dpll/sit9531x/core.h
index cf4dd5f82d05..fa1197bfc81c 100644
--- a/drivers/dpll/sit9531x/core.h
+++ b/drivers/dpll/sit9531x/core.h
@@ -255,6 +255,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 4c7cae4ce93e..0400444d91a5 100644
--- a/drivers/dpll/sit9531x/dpll.c
+++ b/drivers/dpll/sit9531x/dpll.c
@@ -34,6 +34,21 @@ sit9531x_dpll_is_xo_pin(const struct sit9531x_dpll_pin *pin)
 	       pin->id == SIT9531X_MAX_INPUTS;
 }
 
+/*
+ * The pins the poll watches: 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.
+ */
+static bool
+sit9531x_dpll_is_selection_pin(const struct sit9531x_dpll_pin *pin)
+{
+	return sit9531x_dpll_is_input_pin(pin) &&
+	       !sit9531x_dpll_is_xo_pin(pin);
+}
+
 /*
  * Status of a PLL whose outer loop is frozen in holdover.
  *
@@ -548,11 +563,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,
 };
 
 /*
@@ -636,6 +733,55 @@ sit9531x_dpll_pin_ops_get(const struct sit9531x_dpll_pin *pin)
 	return &sit9531x_dpll_input_pin_ops;
 }
 
+/*
+ * The state, operational state and priority of a selection-role pin as
+ * the cache has them now: the baseline at registration, and what the poll
+ * compares it against on every tick.
+ *
+ * Caller must hold sitdev->multiop_lock.
+ */
+static void sit9531x_dpll_pin_snapshot(struct sit9531x_dpll *sitdpll,
+				       const struct sit9531x_dpll_pin *pin,
+				       enum dpll_pin_state *state,
+				       enum dpll_pin_operstate *operstate,
+				       u8 *prio)
+{
+	struct sit9531x_dev *sitdev = sitdpll->dev;
+	u8 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;
+}
+
+/*
+ * sit9531x_dpll_pin_baseline - seed what the poll compares a pin against
+ *
+ * Called right after the pin is registered, under multiop_lock, so the
+ * poll's first comparison is against what the create notification
+ * carried.  A baseline taken on the first tick instead would absorb
+ * whatever moved between registration and that tick -- a lane losing its
+ * signal, a table rewritten and re-seeded -- and leave userspace with the
+ * snapshot from registration until the next change.
+ */
+void sit9531x_dpll_pin_baseline(struct sit9531x_dpll_pin *pin)
+{
+	struct sit9531x_dpll *sitdpll = pin->dpll;
+
+	lockdep_assert_held(&sitdpll->dev->multiop_lock);
+
+	if (!sit9531x_dpll_is_selection_pin(pin))
+		return;
+
+	sit9531x_dpll_pin_snapshot(sitdpll, pin, &pin->pin_state,
+				   &pin->operstate, &pin->prio);
+}
+
 /*
  * sit9531x_dpll_changes_check - check for state changes and notify
  *
@@ -671,39 +817,45 @@ 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 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.
-		 */
-		if (!sit9531x_dpll_is_input_pin(pin) ||
-		    sit9531x_dpll_is_xo_pin(pin))
+		if (!sit9531x_dpll_is_selection_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;
+		sit9531x_dpll_pin_snapshot(sitdpll, pin, &state, &operstate,
+					   &prio);
+
+		changed = 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);
+
+		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.
+		 * 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.
 		 */
-		changed = pin->seen && state != pin->pin_state;
-		pin->pin_state = state;
-		pin->seen = true;
-		if (changed) {
-			dev_dbg(sitdev->dev, "%s state changed\n", pin->label);
-			dpll_pin_change_ntf(pin->dpll_pin);
-		}
+		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 9eb26505f94e..c2706ced92cd 100644
--- a/drivers/dpll/sit9531x/dpll.h
+++ b/drivers/dpll/sit9531x/dpll.h
@@ -31,7 +31,8 @@ struct sit9531x_dpll_pin {
 	u8				id;		/* hardware index */
 	u8				prio;
 	enum dpll_pin_state		pin_state;
-	bool				seen;	/* baseline taken by the poll */
+	enum dpll_pin_operstate		operstate;
+	bool				ntf_pending;
 };
 
 /* Per-PLL DPLL device state. */
@@ -63,6 +64,7 @@ void sit9531x_dpll_unregister(struct sit9531x_dpll *sitdpll);
 
 /* ---- Periodic change detection ---- */
 void sit9531x_dpll_lock_status_baseline(struct sit9531x_dpll *sitdpll);
+void sit9531x_dpll_pin_baseline(struct sit9531x_dpll_pin *pin);
 void sit9531x_dpll_changes_check(struct sit9531x_dpll *sitdpll);
 
 #endif /* _SIT9531X_DPLL_H */
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.39.2 (Apple Git-143)