[PATCH net-next v8 12/15] 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 | 125 +++++++++++++++++++++++++++++++++++
 drivers/dpll/sit9531x/dpll.c | 100 ++++++++++++++++++++++++++++
 drivers/dpll/sit9531x/regs.h |  36 ++++++++++
 3 files changed, 261 insertions(+)

diff --git a/drivers/dpll/sit9531x/core.c b/drivers/dpll/sit9531x/core.c
index c93578f3b750..f552a9c73796 100644
--- a/drivers/dpll/sit9531x/core.c
+++ b/drivers/dpll/sit9531x/core.c
@@ -1586,6 +1586,131 @@ int sit9531x_output_pulse_ctrl_set(struct sit9531x_dev *sitdev,
 	return rc;
 }
 
+/*
+ * sit9531x_phase_offset_read - read phase difference via TDC
+ * @phase_ps:	output phase difference in picoseconds
+ *
+ * Reads the Time-to-Digital Converter (TDC) 40-bit code from the
+ * PLL page registers, then converts to picoseconds using the VCO
+ * frequency: phase_diff = tdc_code / fvco.
+ *
+ * Caller must hold sitdev->multiop_lock.
+ */
+int sit9531x_phase_offset_read(struct sit9531x_dev *sitdev, u8 pll_idx,
+			       s64 *phase_ps)
+{
+	u64 fvco, fvco_mhz;
+	s64 tdc_signed;
+	u64 tdc_raw;
+	int rc, i;
+	bool sign;
+	u8 v;
+
+	lockdep_assert_held(&sitdev->multiop_lock);
+
+	if (pll_idx >= SIT9531X_NUM_PLLS)
+		return -EINVAL;
+
+	/* 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)
+		return rc;
+
+	/*
+	 * 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)
+		return rc;
+	rc = sit9531x_write_pll_u8(sitdev, pll_idx,
+				   SIT9531X_PLL_REG_DBG_READ_CODE,
+				   SIT9531X_DBG_READ_CODE_TDC);
+	if (rc)
+		return rc;
+
+	/* Latch a sample by reading the trigger register; see dbg_sample(). */
+	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)
+			return rc;
+	}
+
+	tdc_raw = 0;
+
+	rc = sit9531x_read_pll_u8(sitdev, pll_idx,
+				  SIT9531X_PLL_REG_DBG_DATA_4, &v);
+	if (rc)
+		return rc;
+	sign = !!(v & BIT(SIT9531X_TDC_SIGN_BIT));
+	tdc_raw = (u64)(v & 0x07) << 32;
+
+	rc = sit9531x_read_pll_u8(sitdev, pll_idx,
+				  SIT9531X_PLL_REG_DBG_DATA_3, &v);
+	if (rc)
+		return rc;
+	tdc_raw |= (u64)v << 24;
+
+	rc = sit9531x_read_pll_u8(sitdev, pll_idx,
+				  SIT9531X_PLL_REG_DBG_DATA_2, &v);
+	if (rc)
+		return rc;
+	tdc_raw |= (u64)v << 16;
+
+	rc = sit9531x_read_pll_u8(sitdev, pll_idx,
+				  SIT9531X_PLL_REG_DBG_DATA_1, &v);
+	if (rc)
+		return rc;
+	tdc_raw |= (u64)v << 8;
+
+	rc = sit9531x_read_pll_u8(sitdev, pll_idx,
+				  SIT9531X_PLL_REG_DBG_DATA_0, &v);
+	if (rc)
+		return rc;
+	tdc_raw |= v;
+
+	/*
+	 * Apply sign.  Per the vendor reference 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;
+
+	/*
+	 * Get VCO frequency for conversion.  Fvco==0 means DIVN is not
+	 * programmed (PLL unused on this board) -- skip silently rather
+	 * than spamming the log on every poll cycle.
+	 */
+	fvco = sit9531x_get_fvco(sitdev, pll_idx);
+	if (!fvco) {
+		dev_dbg(sitdev->dev, "PLL%c: Fvco unknown, skip TDC\n",
+			'A' + pll_idx);
+		return -ENODEV;
+	}
+
+	/*
+	 * phase_diff (seconds) = tdc_code / fvco
+	 * phase_diff (ps) = tdc_code * 1e12 / fvco
+	 *
+	 * To avoid 64-bit overflow:
+	 *   phase_ps = tdc_code * 1e6 / (fvco / 1e6)
+	 */
+	fvco_mhz = div64_u64(fvco, 1000000ULL);
+	if (!fvco_mhz)
+		return -EIO;
+
+	*phase_ps = div64_s64(tdc_signed * 1000000LL, (s64)fvco_mhz);
+
+	return 0;
+}
+
 /*
  * sit9531x_ref_state_fetch - read input reference status from hardware
  * @index:	logical input index
diff --git a/drivers/dpll/sit9531x/dpll.c b/drivers/dpll/sit9531x/dpll.c
index 76d7d7d21d3d..f813126077e1 100644
--- a/drivers/dpll/sit9531x/dpll.c
+++ b/drivers/dpll/sit9531x/dpll.c
@@ -528,6 +528,105 @@ 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
+ * framework expects successive values to be averaged, which suits a
+ * closed-loop residual and not a one-shot open-loop capture.
+ *
+ * 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 vendor's 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 the open-loop measurement lives behind a debugfs operation that owns
+ * the freeze and restores it; it is not this callback.
+ *
+ * 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 confirms this is
+ * a property of the hardware rather than of their measurement script.
+ * The script 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, unlike the frequency
+ * offset getter, where -ENODATA makes the core omit the attribute.  There
+ * is no per-pin "no data" for phase 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;
+	s64 offset;
+	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.
+	 * For any input that is not the active reference there is no
+	 * meaningful per-pin phase offset, so report 0 instead of the
+	 * active reference's value.
+	 */
+	if (sitdev->chan[sitdpll->id].selected_ref != dpin->id) {
+		mutex_unlock(&sitdev->multiop_lock);
+		dpin->phase_offset = 0;
+		*phase_offset = 0;
+		return 0;
+	}
+
+	rc = sit9531x_phase_offset_read(sitdev, sitdpll->id, &offset);
+	mutex_unlock(&sitdev->multiop_lock);
+
+	/*
+	 * -ENODEV means the PLL has no programmed DIVN (unused on this
+	 * board); report phase_offset = 0 so a full pin-get dump does not
+	 * fail just because one DPLL is dormant.
+	 */
+	if (rc == -ENODEV) {
+		dpin->phase_offset = 0;
+		*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;
+
+	dpin->phase_offset = offset;
+	*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,
@@ -535,6 +634,7 @@ static const struct dpll_pin_ops sit9531x_dpll_input_pin_ops = {
 	.state_on_dpll_set	= sit9531x_dpll_input_pin_state_on_dpll_set,
 	.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,
 	/*
 	 * The measurement compares the PLL's running feedback divider with
 	 * its configured one, so it describes the device's own reference
diff --git a/drivers/dpll/sit9531x/regs.h b/drivers/dpll/sit9531x/regs.h
index 251c98dc7cbe..aa60be243e6d 100644
--- a/drivers/dpll/sit9531x/regs.h
+++ b/drivers/dpll/sit9531x/regs.h
@@ -256,6 +256,42 @@
 #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
+
+/*
+ * 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  /* [39:32] + sign */
+#define SIT9531X_PLL_REG_DBG_DATA_5		0xBA  /* [47:40] */
+#define SIT9531X_PLL_REG_DBG_DATA_6		0xBB
+#define SIT9531X_PLL_REG_DBG_DATA_7		0xBC
+#define SIT9531X_PLL_REG_DBG_TRIGGER		0xD0  /* read to latch a sample */
+
+/*
+ * Reads of the trigger needed to latch a fresh sample.  One returns the
+ * previous latch, which the vendor procedures work around by reading it
+ * three times.
+ */
+#define SIT9531X_DBG_LATCH_READS		3
+#define SIT9531X_DBG_DATA_BYTES		8
+
+/* 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
+
 /*
  * DIVN carried as fixed point, and the unit the DPLL ABI wants the
  * fractional frequency offset in.  Equal in value, distinct in meaning.
-- 
2.43.0