[PATCH net-next v11 12/13] dpll: sit9531x: add support to get phase offset on the connected input pin

[Ali Rouhi]

From: Oleg Zadorozhnyi <Oleg.Zadorozhnyi@xxxxxxxxxxxxxxxxx>

Report the phase difference between a PLL's reference and the PLL itself,
which is the loop's own residual error and therefore trends small on a
locked device -- that is the measurement, not an artefact of it.

The value comes from the on-chip time-to-digital converter, read through
the debug window: unlock the window, point it at the converter, then read
the trigger register, which latches a fresh sample and returns the
previous one.  It is read three times per sample for that reason; a single
read hands back the sample from the last call, so a repeated measurement
would look perfectly steady while saying nothing.

Only the input a PLL has actually selected has a phase offset against it.
For any other pin there is nothing to measure and zero is reported,
because the core abandons an entire pin dump on an error from any one pin.

Signed-off-by: Oleg Zadorozhnyi <Oleg.Zadorozhnyi@xxxxxxxxxxxxxxxxx>
Assisted-by: Claude:claude-4-opus [chat]
Signed-off-by: Ali Rouhi <arouhi@xxxxxxxxxx>
---
 drivers/dpll/sit9531x/core.c | 229 +++++++++++++++++++++++++++++++++++
 drivers/dpll/sit9531x/core.h |   4 +
 drivers/dpll/sit9531x/dpll.c | 130 ++++++++++++++++++++
 drivers/dpll/sit9531x/regs.h |  35 ++++++
 4 files changed, 398 insertions(+)

diff --git a/drivers/dpll/sit9531x/core.c b/drivers/dpll/sit9531x/core.c
index d2719ff24249..45baa703aa45 100644
--- a/drivers/dpll/sit9531x/core.c
+++ b/drivers/dpll/sit9531x/core.c
@@ -2760,6 +2760,235 @@ int sit9531x_clear_notifications(struct sit9531x_dev *sitdev)
 	return 0;
 }
 
+/**
+ * sit9531x_chan_selected_ref_read - read a PLL's active reference now
+ * @sitdev:	device pointer
+ * @pll_idx:	PLL index (0-3)
+ * @ref:	result, logical input index of the selected reference
+ *
+ * chan->selected_ref is refreshed by the monitor twice a second, which is
+ * close enough for reporting pin state but not for attributing a
+ * measurement: the device picks its own reference, so a sample taken now
+ * can belong to a pin the cache has not caught up with.
+ *
+ * Caller must hold sitdev->multiop_lock.
+ *
+ * Return: 0 on success, <0 on error
+ */
+int sit9531x_chan_selected_ref_read(struct sit9531x_dev *sitdev, u8 pll_idx,
+				    u8 *ref)
+{
+	u8 activesel_reg, input_sel;
+	int rc;
+
+	lockdep_assert_held(&sitdev->multiop_lock);
+
+	if (pll_idx >= SIT9531X_NUM_PLLS)
+		return -EINVAL;
+
+	activesel_reg = SIT9531X_PRIO_BASE_REG +
+			SIT9531X_PRIO_REGS_PER_PLL * pll_idx +
+			SIT9531X_PRIO_ACTIVESEL_OFF;
+	rc = sit9531x_read_u8(sitdev,
+			      SIT9531X_REG(SIT9531X_PAGE_PRIOSYS,
+					   activesel_reg),
+			      &input_sel);
+	if (rc)
+		return rc;
+
+	input_sel &= SIT9531X_PRIO_NIBBLE_MASK;
+	input_sel = sit9531x_prio_src_canon(sitdev, input_sel);
+	*ref = sit9531x_hw_src_input(input_sel);
+
+	return 0;
+}
+
+/**
+ * sit9531x_phase_offset_read - read phase difference via TDC
+ * @sitdev:	device pointer
+ * @pll_idx:	PLL index (0-3)
+ * @phase_ps:	output phase difference in picoseconds
+ *
+ * Reads the Time-to-Digital Converter (TDC) code from the PLL page
+ * registers -- a 35-bit magnitude with a separate sign bit -- then
+ * converts it to picoseconds using the VCO frequency:
+ * phase_diff = tdc_code / fvco.
+ *
+ * Caller must hold sitdev->multiop_lock.
+ *
+ * Return: 0 on success, -ENODATA when the PLL has no known VCO rate
+ * (nothing is sampled then), or the register access error
+ */
+int sit9531x_phase_offset_read(struct sit9531x_dev *sitdev, u8 pll_idx,
+			       s64 *phase_ps)
+{
+	u8 v, old_write_code, old_read_code;
+	bool have_old = false;
+	int rc, ret, i;
+	u64 fvco, mag_ps;
+	s64 tdc_signed;
+	u64 tdc_raw;
+	bool sign;
+
+	lockdep_assert_held(&sitdev->multiop_lock);
+
+	if (pll_idx >= SIT9531X_NUM_PLLS)
+		return -EINVAL;
+
+	/*
+	 * Get the VCO rate first.  -ENODATA means the PLL has no rate to
+	 * convert against (a divider this board leaves unprogrammed), and
+	 * there is then no reason to open the debug window at all -- which
+	 * also keeps a bus error while it is open from ever being reported
+	 * as that benign case.
+	 */
+	rc = sit9531x_get_fvco(sitdev, pll_idx, &fvco);
+	if (rc) {
+		if (rc == -ENODATA)
+			dev_dbg(sitdev->dev,
+				"PLL%c: Fvco unknown, skip TDC\n",
+				'A' + pll_idx);
+		return rc;
+	}
+
+	/* Unlock the debug page so the TDC registers are accessible. */
+	rc = sit9531x_write_pll_u8(sitdev, pll_idx,
+				   SIT9531X_PLL_REG_DEBUG,
+				   SIT9531X_PLL_DEBUG_UNLOCK);
+	if (rc)
+		goto relock;
+
+	/*
+	 * Remember the tap selection so it can be put back.  The key
+	 * register is re-locked below, but the mux is not part of the key:
+	 * leaving it parked on the TDC with a slow sampling clock selected
+	 * is a state change the caller did not ask for, and the next reader
+	 * of a different tap would have to know to undo it.
+	 */
+	rc = sit9531x_read_pll_u8(sitdev, pll_idx,
+				  SIT9531X_PLL_REG_DBG_WRITE_CODE,
+				  &old_write_code);
+	if (!rc)
+		rc = sit9531x_read_pll_u8(sitdev, pll_idx,
+					  SIT9531X_PLL_REG_DBG_READ_CODE,
+					  &old_read_code);
+	if (rc)
+		goto relock;
+
+	have_old = true;
+
+	/*
+	 * Select the debug clock for taps below 200 kHz, then point the
+	 * readback at the TDC.  Only the one bit is touched: writing the
+	 * modifier register whole would clear the fields belonging to
+	 * other taps.
+	 */
+	rc = sit9531x_update_pll_u8(sitdev, pll_idx,
+				    SIT9531X_PLL_REG_DBG_WRITE_CODE,
+				    SIT9531X_DBG_LOW_FREQ_CLK_BIT,
+				    SIT9531X_DBG_LOW_FREQ_CLK_BIT);
+	if (rc)
+		goto relock;
+	rc = sit9531x_write_pll_u8(sitdev, pll_idx,
+				   SIT9531X_PLL_REG_DBG_READ_CODE,
+				   SIT9531X_DBG_READ_CODE_TDC);
+	if (rc)
+		goto relock;
+
+	/*
+	 * Latch a sample by reading the trigger register.  A single
+	 * read returns the previous latch, so read it three times as
+	 * the documented phase-difference procedure does.
+	 */
+	for (i = 0; i < SIT9531X_DBG_LATCH_READS; i++) {
+		rc = sit9531x_read_pll_u8(sitdev, pll_idx,
+					  SIT9531X_PLL_REG_DBG_TRIGGER, &v);
+		if (rc)
+			goto relock;
+	}
+
+	tdc_raw = 0;
+
+	rc = sit9531x_read_pll_u8(sitdev, pll_idx,
+				  SIT9531X_PLL_REG_DBG_DATA_4, &v);
+	if (rc)
+		goto relock;
+	sign = !!(v & BIT(SIT9531X_TDC_SIGN_BIT));
+	tdc_raw = (u64)(v & SIT9531X_TDC_MAG_HI_MASK) << 32;
+
+	rc = sit9531x_read_pll_u8(sitdev, pll_idx,
+				  SIT9531X_PLL_REG_DBG_DATA_3, &v);
+	if (rc)
+		goto relock;
+	tdc_raw |= (u64)v << 24;
+
+	rc = sit9531x_read_pll_u8(sitdev, pll_idx,
+				  SIT9531X_PLL_REG_DBG_DATA_2, &v);
+	if (rc)
+		goto relock;
+	tdc_raw |= (u64)v << 16;
+
+	rc = sit9531x_read_pll_u8(sitdev, pll_idx,
+				  SIT9531X_PLL_REG_DBG_DATA_1, &v);
+	if (rc)
+		goto relock;
+	tdc_raw |= (u64)v << 8;
+
+	rc = sit9531x_read_pll_u8(sitdev, pll_idx,
+				  SIT9531X_PLL_REG_DBG_DATA_0, &v);
+	if (rc)
+		goto relock;
+	tdc_raw |= v;
+
+	/*
+	 * Apply sign.  Per the register map the sign bit is active-high
+	 * for a positive offset: bit set -> +code, bit clear -> -code.
+	 */
+	tdc_signed = sign ? (s64)tdc_raw : -(s64)tdc_raw;
+
+	/*
+	 * phase_diff (seconds) = tdc_code / fvco
+	 * phase_diff (ps) = tdc_code * 1e12 / fvco
+	 *
+	 * mul_u64_u64_div_u64() keeps the exact Hz denominator; dividing
+	 * by whole MHz instead would lose up to ~40 ppm of scale on a
+	 * fractional-DIVN Fvco.
+	 */
+	mag_ps = mul_u64_u64_div_u64(tdc_signed < 0 ? -tdc_signed : tdc_signed,
+				     1000000000000ULL, fvco);
+	*phase_ps = tdc_signed < 0 ? -(s64)mag_ps : (s64)mag_ps;
+
+	rc = 0;
+
+relock:
+	if (have_old) {
+		ret = sit9531x_write_pll_u8(sitdev, pll_idx,
+					    SIT9531X_PLL_REG_DBG_READ_CODE,
+					    old_read_code);
+		if (ret && !rc)
+			rc = ret;
+		ret = sit9531x_write_pll_u8(sitdev, pll_idx,
+					    SIT9531X_PLL_REG_DBG_WRITE_CODE,
+					    old_write_code);
+		if (ret && !rc)
+			rc = ret;
+	}
+
+	/*
+	 * Close the debug window again.  The key register opens every debug
+	 * register on this PLL while it holds the unlock value, and this read
+	 * runs on every pin-get of a connected input, so leaving it open
+	 * would mean normal monitoring permanently unlocks the block.
+	 */
+	ret = sit9531x_write_pll_u8(sitdev, pll_idx,
+				    SIT9531X_PLL_REG_DEBUG,
+				    SIT9531X_PLL_DEBUG_LOCK);
+	if (ret && !rc)
+		rc = ret;
+
+	return rc;
+}
+
 /*
  * sit9531x_ref_state_fetch - read input reference status from hardware
  * @index:	logical input index
diff --git a/drivers/dpll/sit9531x/core.h b/drivers/dpll/sit9531x/core.h
index 66c87e8f0387..bebdc9f5abce 100644
--- a/drivers/dpll/sit9531x/core.h
+++ b/drivers/dpll/sit9531x/core.h
@@ -296,6 +296,10 @@ int sit9531x_clear_notifications(struct sit9531x_dev *sitdev);
 /* ---- INTSYNC (inter-PLL synchronization) ---- */
 
 /* ---- Phase offset (TDC readback) ---- */
+int sit9531x_chan_selected_ref_read(struct sit9531x_dev *sitdev, u8 pll_idx,
+				    u8 *ref);
+int sit9531x_phase_offset_read(struct sit9531x_dev *sitdev, u8 pll_idx,
+			       s64 *phase_ps);
 
 /* ---- State helpers ---- */
 
diff --git a/drivers/dpll/sit9531x/dpll.c b/drivers/dpll/sit9531x/dpll.c
index 8a5e3a1decd0..6bf4efd3c633 100644
--- a/drivers/dpll/sit9531x/dpll.c
+++ b/drivers/dpll/sit9531x/dpll.c
@@ -605,6 +605,135 @@ sit9531x_dpll_input_pin_prio_set(const struct dpll_pin *pin, void *pin_priv,
 	return 0;
 }
 
+/*
+ * sit9531x_dpll_input_pin_phase_offset_get - phase offset of a reference
+ *
+ * What this reports, and what it deliberately does not:
+ *
+ * The ABI defines the attribute as the phase difference between the signal
+ * on a pin and its parent DPLL device, so this is the loop's own residual
+ * error, sampled with the loop closed.  On a locked DPLL it therefore
+ * trends small -- that is the measurement, not an artefact of it.  The
+ * documentation describes the reported value as one that may be averaged
+ * over prior measurements, which suits a closed-loop residual and not a
+ * one-shot open-loop capture; the core publishes whatever this callback
+ * returns, so the averaging, if any, would be this driver's to do.
+ *
+ * The chip can also measure the reference against the local oscillator
+ * with the outer loop's correction frozen, which is a different quantity
+ * and the one the documented phase-difference procedure produces.  That
+ * needs the digital loop filter held (and, on the 1PPS PLL, the automatic
+ * phase- and frequency-lock helpers held off), which leaves the PLL
+ * undisciplined until it is released.  A netlink read must not do that,
+ * so that measurement is not offered here at all; it belongs to a caller
+ * that can own the freeze and restore it.
+ *
+ * Precondition, which this callback cannot create: the TDC compares
+ * against a signal the PLL drives, so a PLL driving no output with its
+ * zero-delay buffer off has nothing to measure.  SiTime clock
+ * engineering confirms this is a property of the hardware, not of
+ * SiTime's TDC measurement procedure, which satisfies it by mapping a
+ * spare output and restarting the PLL -- side effects that do not belong
+ * in a getter, so a reading taken in that state is simply not meaningful.
+ *
+ * Non-selected pins and a PLL with no programmed divider report zero
+ * rather than an error: the DPLL core propagates any error from this
+ * callback and fails the whole pin dump with it.  The core has no per-pin
+ * "no data" for phase offset, as it has -ENODATA for the fractional
+ * frequency offset, so it is a value or no callback at all.
+ */
+static int
+sit9531x_dpll_input_pin_phase_offset_get(const struct dpll_pin *pin,
+					 void *pin_priv,
+					 const struct dpll_device *dpll,
+					 void *dpll_priv, s64 *phase_offset,
+					 struct netlink_ext_ack *extack)
+{
+	struct sit9531x_dpll_pin *dpin = pin_priv;
+	struct sit9531x_dpll *sitdpll = dpll_priv;
+	struct sit9531x_dev *sitdev = sitdpll->dev;
+	bool drives = false;
+	s64 offset;
+	u8 selected, i;
+	int rc;
+
+	mutex_lock(&sitdev->multiop_lock);
+
+	/*
+	 * The on-chip TDC is a per-PLL resource that always measures the
+	 * phase difference between the VCO and the PLL's currently
+	 * selected reference; it cannot be pointed at an arbitrary input,
+	 * so an input that is not the active reference reports 0 rather
+	 * than the active reference's value.
+	 *
+	 * The sample needs both: the PLL tracking this pin as the poll last
+	 * saw it (locked, outer loop running, not frozen), and the device
+	 * still naming this pin, read now -- the device selects on its own,
+	 * and a cache up to a poll period old could attribute a live
+	 * measurement to the pin that used to be selected.  The device is
+	 * read again after the sample for the same reason: it can switch
+	 * during the dozen transfers the sample takes.
+	 *
+	 * The TDC also compares against a signal the PLL drives, so a PLL
+	 * with no routed, driving output has nothing to measure and reports
+	 * 0 like any other pin without a reading.
+	 */
+	for (i = 0; i < sitdev->info->num_outputs; i++)
+		if (sitdev->out[i].routed && sitdev->out[i].enabled &&
+		    sitdev->out[i].pll_idx == sitdpll->id)
+			drives = true;
+
+	if (!drives ||
+	    !sit9531x_dpll_selection_active(sitdev, sitdpll, dpin->id)) {
+		mutex_unlock(&sitdev->multiop_lock);
+		*phase_offset = 0;
+		return 0;
+	}
+
+	rc = sit9531x_chan_selected_ref_read(sitdev, sitdpll->id,
+					     &selected);
+	if (!rc && selected == dpin->id) {
+		rc = sit9531x_phase_offset_read(sitdev, sitdpll->id, &offset);
+		if (!rc)
+			rc = sit9531x_chan_selected_ref_read(sitdev,
+							     sitdpll->id,
+							     &selected);
+		if (!rc && selected != dpin->id)
+			rc = -ENODATA;
+	} else if (!rc) {
+		rc = -ENODATA;
+	}
+	mutex_unlock(&sitdev->multiop_lock);
+
+	/*
+	 * -ENODATA means no reading: the PLL has no known VCO rate, or the
+	 * selection moved off this pin around the sample.  Report 0 so a
+	 * full pin-get dump does not fail over it.  Every other errno,
+	 * -ENODEV from a vanished adapter included, is a failure.
+	 */
+	if (rc == -ENODATA) {
+		*phase_offset = 0;
+		return 0;
+	}
+	if (rc) {
+		NL_SET_ERR_MSG(extack, "TDC phase readback failed");
+		return rc;
+	}
+
+	/*
+	 * The ABI reports phase offset in units of 1/DPLL_PHASE_OFFSET_DIVIDER
+	 * picoseconds: the integer part of the attribute is the value divided
+	 * by the divider, the remainder is the fraction.  The TDC resolves one
+	 * VCO period (hundreds of picoseconds), so the fractional digits are
+	 * always zero here, but the magnitude still has to be scaled or every
+	 * reading would be reported a thousand times too small.
+	 */
+	offset *= DPLL_PHASE_OFFSET_DIVIDER;
+
+	*phase_offset = offset;
+	return 0;
+}
+
 static const struct dpll_pin_ops sit9531x_dpll_input_pin_ops = {
 	.direction_get		= sit9531x_dpll_input_pin_direction_get,
 	.frequency_get		= sit9531x_dpll_input_pin_frequency_get,
@@ -613,6 +742,7 @@ static const struct dpll_pin_ops sit9531x_dpll_input_pin_ops = {
 	.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,
+	.phase_offset_get	= sit9531x_dpll_input_pin_phase_offset_get,
 };
 
 /*
diff --git a/drivers/dpll/sit9531x/regs.h b/drivers/dpll/sit9531x/regs.h
index b805cbc41fc1..0ba3c73aa3ed 100644
--- a/drivers/dpll/sit9531x/regs.h
+++ b/drivers/dpll/sit9531x/regs.h
@@ -283,6 +283,41 @@
 #define SIT9531X_PLL_REG_DIVN_NUM		0x32  /* 4 bytes (0x32-0x35) */
 #define SIT9531X_PLL_REG_DIVN_DEN		0x38  /* 4 bytes (0x38-0x3B) */
 
+/* Debug register unlock */
+#define SIT9531X_PLL_REG_DEBUG		0xBD
+#define SIT9531X_PLL_DEBUG_UNLOCK		0xC3
+#define SIT9531X_PLL_DEBUG_LOCK			0x00
+
+/*
+ * Signal pathway debug readback -- PLL page.  Dig_Sys_ReadCode selects
+ * which point of the pathway is tapped, Dig_Sys_WriteCode carries the
+ * modifiers for that read, Dig_Sys_read7..read0 hold the sampled bytes
+ * and the trigger register latches a sample.  The TDC phase
+ * measurement is one tap among several, reached through read code 69.
+ */
+#define SIT9531X_PLL_REG_DBG_READ_CODE	0xB3
+#define SIT9531X_PLL_REG_DBG_WRITE_CODE	0xB4
+#define SIT9531X_DBG_LOW_FREQ_CLK_BIT	BIT(7)
+#define SIT9531X_PLL_REG_DBG_DATA_0		0xB5  /* [7:0] */
+#define SIT9531X_PLL_REG_DBG_DATA_1		0xB6  /* [15:8] */
+#define SIT9531X_PLL_REG_DBG_DATA_2		0xB7  /* [23:16] */
+#define SIT9531X_PLL_REG_DBG_DATA_3		0xB8  /* [31:24] */
+#define SIT9531X_PLL_REG_DBG_DATA_4		0xB9  /* [34:32] + sign */
+/* read to latch a sample */
+#define SIT9531X_PLL_REG_DBG_TRIGGER		0xD0
+
+/*
+ * Reads of the trigger needed to latch a fresh sample.  One returns the
+ * previous latch, which the documented procedures work around by reading it
+ * three times.
+ */
+#define SIT9531X_DBG_LATCH_READS		3
+
+/* Read code of the TDC phase tap, and the sign bit of its sample */
+#define SIT9531X_DBG_READ_CODE_TDC		69
+#define SIT9531X_TDC_SIGN_BIT		3
+#define SIT9531X_TDC_MAG_HI_MASK	GENMASK(2, 0)
+
 /* DIVN is carried as fixed point, in steps of 1e-12 of a whole divider */
 #define SIT9531X_DIVN_SCALE		1000000000000ULL
 
-- 
2.43.0