Re: [PATCH net-next v2 2/2] dpll: zl3073x: add NCO virtual input pin
From: Ivan Vecera
Date: Wed May 20 2026 - 05:28:58 EST
Sashiko findings clarification inline below...
On 5/16/26 7:57 PM, Ivan Vecera wrote:
Add a virtual NCO (Numerically Controlled Oscillator) input pin that
lets userspace switch a DPLL channel into NCO mode. A single NCO pin
is shared across all DPLL channels — each channel has its own
independent connection state. The NCO pin is registered with the new
DPLL_PIN_TYPE_INT_NCO type and reports DPLL_PIN_STATE_CONNECTED /
DPLL_PIN_OPERSTATE_ACTIVE when the channel is in NCO mode.
...
diff --git a/drivers/dpll/zl3073x/chan.c b/drivers/dpll/zl3073x/chan.c
index 2fe3c3da84bb5..f5d6c7917e57a 100644
--- a/drivers/dpll/zl3073x/chan.c
+++ b/drivers/dpll/zl3073x/chan.c
@@ -21,6 +21,11 @@ int zl3073x_chan_state_update(struct zl3073x_dev *zldev, u8 index)
u64 val;
int rc;
+ /* Serialize with zl3073x_chan_nco_mode_set() which also
+ * modifies chan->mode_refsel and chan->df_offset.
+ */
+ guard(mutex)(&zldev->multiop_lock);
+
rc = zl3073x_read_u8(zldev, ZL_REG_DPLL_MON_STATUS(index),
&chan->mon_status);
if (rc)
@@ -31,7 +36,10 @@ int zl3073x_chan_state_update(struct zl3073x_dev *zldev, u8 index)
if (rc)
return rc;
- /* Read df_offset vs tracked reference */
+ /* Read df_offset only when locked to a reference */
+ if (zl3073x_chan_lock_state_get(chan) != ZL_DPLL_MON_STATUS_STATE_LOCK)
+ return 0;
+
Sashiko commented:
If the device boots already in NCO mode, can chan->df_offset ever be
populated?
Walking through zl3073x_chan_state_fetch() at probe time with the
hardware already in NCO mode:
- state_fetch() reads ctrl and mode_refsel, then calls
zl3073x_chan_state_update()
- state_update() now early-returns above when lock_state is not
STATE_LOCK, and an NCO-mode channel is not LOCK, so df_offset
keeps its kzalloc default of 0
- zl3073x_dpll_nco_pin_register() updates ctrl bits via
zl3073x_chan_state_set() but does not touch mode_refsel, so the
edge-triggered nco_auto_read does not fire (it captures only on
the transition into NCO)
- zl3073x_dpll_nco_pin_ffo_get() then reports
-mul_s64_u64_shr(0, 244140625, 36) = 0
The commit message states "Input reference pins are now always
registered regardless of the initial DPLL mode", so boot-in-NCO is a
supported configuration.
Should zl3073x_dpll_nco_pin_register() (or state_fetch) read
ZL_REG_DPLL_DF_OFFSET when the channel is already in NCO mode, similar
to the read in the new zl3073x_chan_nco_mode_set()?
In boot-in-NCO configuration the df_offset is zero... so this behavior
is correct.
diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
index cff85cdb9d0e5..2efefb1d7e224 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -605,6 +630,7 @@ zl3073x_dpll_input_pin_state_on_dpll_set(const struct dpll_pin *dpll_pin,
{
struct zl3073x_dpll *zldpll = dpll_priv;
struct zl3073x_dpll_pin *pin = pin_priv;
+ struct zl3073x_dpll_pin *nco_pin = NULL;
struct zl3073x_chan chan;
u8 mode, ref;
int rc;
@@ -634,6 +660,10 @@ zl3073x_dpll_input_pin_state_on_dpll_set(const struct dpll_pin *dpll_pin,
goto invalid_state;
}
break;
+ case ZL_DPLL_MODE_REFSEL_MODE_NCO:
+ if (state == DPLL_PIN_STATE_CONNECTED)
+ nco_pin = zl3073x_dpll_nco_pin_get(zldpll);
+ fallthrough;
case ZL_DPLL_MODE_REFSEL_MODE_FREERUN:
case ZL_DPLL_MODE_REFSEL_MODE_HOLDOVER:
if (state == DPLL_PIN_STATE_CONNECTED) {
Sashiko commented:
When a REF pin is connected while the channel is in REFLOCK and the
NCO pin is then connected, the channel transitions REFLOCK -> NCO via
zl3073x_chan_nco_mode_set(). The previously-locked REF pin now
reports DPLL_PIN_STATE_DISCONNECTED via the default branch in
zl3073x_dpll_input_pin_state_on_dpll_get(), but no immediate
__dpll_pin_change_ntf() is sent for that REF pin.
The periodic worker zl3073x_dpll_changes_check() will eventually pick
up the operstate change and call dpll_pin_change_ntf() for the REF
pin so userspace re-queries.
Should the same explicit-immediate-notification pattern that this
patch introduces for the NCO pin also apply to the previously-
connected REF pin when its connection state changes due to a mode
transition?
No, explicit notification for NCO pin is sent because NCO-type pin is
not monitored. For regulard HW input references the implicit
notification will be sent by peridic monitor function.
...
+static int
+zl3073x_dpll_nco_pin_ffo_get(const struct dpll_pin *dpll_pin, void *pin_priv,
+ const struct dpll_device *dpll, void *dpll_priv,
+ struct dpll_ffo_param *ffo,
+ struct netlink_ext_ack *extack)
+{
+ struct zl3073x_dpll *zldpll = dpll_priv;
+ const struct zl3073x_chan *chan;
+
+ chan = zl3073x_chan_state_get(zldpll->dev, zldpll->id);
+ if (!zl3073x_chan_mode_is_nco(chan))
+ return -ENODATA;
+
+ /* NCO register has inverted sign: f_offset = -df_offset / 2^48
+ * Convert to PPT: ppt = -df * 5^12 / 2^36
+ */
+ ffo->ffo = -mul_s64_u64_shr(zl3073x_chan_df_offset_get(chan),
+ 244140625, 36);
+
+ return 0;
+}
Sashiko commented:
The new pin is registered with type DPLL_PIN_TYPE_INT_NCO. The UAPI
header include/uapi/linux/dpll.h documents that type as: "When
connected as a DPLL input, the DPLL enters NCO mode where the output
frequency is adjusted by the host via the PTP clock interface."
Does this driver satisfy that contract?
There is no PTP clock device registered anywhere under
drivers/dpll/zl3073x (no ptp_clock_register, no ptp_clock_info, no
adjfine/adjphase). The new zl3073x_dpll_nco_pin_ops exposes only
ffo_get, state_on_dpll_get/set, operstate_on_dpll_get and
direction_get, with no frequency or phase control. The pin properties
declare only DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE.
The commit message also says "The value is static for the duration of
NCO mode and serves as the FFO snapshot reported to userspace", so
zl3073x_chan_nco_mode_set() captures df_offset once and never updates
it.
Generic userspace recognizing DPLL_PIN_TYPE_INT_NCO will reasonably
expect a controllable NCO. Should this mode either expose a frequency
adjustment path (PTP or otherwise) or document that this device
exposes a read-only frozen-offset variant of the NCO type?
The PTP support for zl3073x will follow this series.
Thanks,
Ivan