From: Oleg Zadorozhnyi 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 Assisted-by: Claude:claude-4-opus [chat] Signed-off-by: Ali Rouhi --- drivers/dpll/sit9531x/core.c | 148 +++++++++++++++++++++++++++++++++++ drivers/dpll/sit9531x/dpll.c | 102 ++++++++++++++++++++++++ drivers/dpll/sit9531x/regs.h | 38 +++++++++ 3 files changed, 288 insertions(+) diff --git a/drivers/dpll/sit9531x/core.c b/drivers/dpll/sit9531x/core.c index 46179a9e13c3..356d1eac4ec5 100644 --- a/drivers/dpll/sit9531x/core.c +++ b/drivers/dpll/sit9531x/core.c @@ -2132,6 +2132,154 @@ 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) signed 35-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, mag_ps; + s64 tdc_signed; + u64 tdc_raw; + int rc, lock_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) + goto relock; + + /* + * 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; + + /* + * Get VCO frequency for conversion. -ENODATA means DIVN is not + * programmed (PLL unused on this board) -- skip silently rather + * than spamming the log on every poll cycle. A register access + * failure is returned as an error, not folded into a fabricated + * 0 ps measurement. + */ + rc = sit9531x_get_fvco(sitdev, pll_idx, &fvco); + if (rc == -ENODATA) { + dev_dbg(sitdev->dev, "PLL%c: Fvco unknown, skip TDC\n", + 'A' + pll_idx); + rc = -ENODEV; + goto relock; + } + if (rc) + goto relock; + + /* + * 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: + /* + * 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. + */ + lock_rc = sit9531x_write_pll_u8(sitdev, pll_idx, + SIT9531X_PLL_REG_DEBUG, + SIT9531X_PLL_DEBUG_LOCK); + if (lock_rc && !rc) + rc = lock_rc; + + return rc; +} + /* * 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 0cad081eb599..1dfd83b635e2 100644 --- a/drivers/dpll/sit9531x/dpll.c +++ b/drivers/dpll/sit9531x/dpll.c @@ -603,6 +603,107 @@ 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 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 the open-loop measurement lives in a debugfs helper 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; + enum dpll_pin_state state; + 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. + */ + sit9531x_dpll_selection_state_get(sitdev, sitdpll, dpin->id, &state); + if (state != DPLL_PIN_STATE_CONNECTED) { + 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, @@ -610,6 +711,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, }; /* diff --git a/drivers/dpll/sit9531x/regs.h b/drivers/dpll/sit9531x/regs.h index 9d515e109feb..08eae8a2c279 100644 --- a/drivers/dpll/sit9531x/regs.h +++ b/drivers/dpll/sit9531x/regs.h @@ -271,6 +271,44 @@ #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 */ +#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 documented 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 +#define SIT9531X_TDC_MAG_HI_MASK GENMASK(2, 0) + /* * DIVN carried as fixed point, and the unit the DPLL ABI wants the * fractional frequency offset in. Equal in value, distinct in meaning. -- 2.39.2 (Apple Git-143)