Re: [PATCH net-next 2/2] dpll: zl3073x: implement pin operational state reporting

Petr Oros
2026-04-30T11:59:02Z

On 4/28/26 17:49, Ivan Vecera wrote:
> Implement operstate_on_dpll_get callback for input pins to report
> the actual hardware status:
>
> - active: pin is the currently locked reference
> - standby: signal is valid but pin is not actively used
> - no-signal: reference monitor reports Loss of Signal (LOS)
> - qual-failed: reference monitor reports a qualification failure
> (SCM, CFM, GST, PFM, eSync or Split-XO)
>
> Separate administrative state (state_on_dpll_get) from operational
> state: admin state now reports purely the user-requested intent
> (connected in reflock mode, selectable in auto mode).
>
> Switch periodic monitoring to track operstate changes instead of
> the mixed admin/oper state that was previously reported.
>
> Add ref_mon_status bit definitions to regs.h.
>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
> drivers/dpll/zl3073x/dpll.c | 108 ++++++++++++++++++++++++------------
> drivers/dpll/zl3073x/regs.h | 9 ++-
> 2 files changed, 79 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
> index c95e93ef3ab04..6fd718696de0d 100644
> --- a/drivers/dpll/zl3073x/dpll.c
> +++ b/drivers/dpll/zl3073x/dpll.c
> @@ -38,7 +38,7 @@
> * @prio: pin priority <0, 14>
> * @esync_control: embedded sync is controllable
> * @phase_gran: phase adjustment granularity
> - * @pin_state: last saved pin state
> + * @operstate: last saved operational state
> * @phase_offset: last saved pin phase offset
> * @freq_offset: last saved fractional frequency offset
> * @measured_freq: last saved measured frequency
> @@ -55,7 +55,7 @@ struct zl3073x_dpll_pin {
> u8 prio;
> bool esync_control;
> s32 phase_gran;
> - enum dpll_pin_state pin_state;
> + enum dpll_pin_operstate operstate;
> s64 phase_offset;
> s64 freq_offset;
> u32 measured_freq;
> @@ -500,46 +500,41 @@ zl3073x_dpll_input_pin_phase_adjust_set(const struct dpll_pin *dpll_pin,
> }
>
> /**
> - * zl3073x_dpll_ref_state_get - get status for given input pin
> + * zl3073x_dpll_ref_operstate_get - get operational state for input pin
> * @pin: pointer to pin
> - * @state: place to store status
> + * @operstate: place to store operational state
> *
> - * Checks current status for the given input pin and stores the value
> - * to @state.
> + * Returns the actual hardware state of the pin: whether it is actively
> + * used by the DPLL, has no signal, failed qualification, or is simply
> + * not in use.
> *
> * Return: 0 on success, <0 on error
> */
> static int
> -zl3073x_dpll_ref_state_get(struct zl3073x_dpll_pin *pin,
> - enum dpll_pin_state *state)
> +zl3073x_dpll_ref_operstate_get(struct zl3073x_dpll_pin *pin,
> + enum dpll_pin_operstate *operstate)
> {
> struct zl3073x_dpll *zldpll = pin->dpll;
> struct zl3073x_dev *zldev = zldpll->dev;
> - const struct zl3073x_chan *chan;
> - u8 ref;
> -
> - chan = zl3073x_chan_state_get(zldev, zldpll->id);
> - ref = zl3073x_input_pin_ref_get(pin->id);
> + const struct zl3073x_ref *ref;
> + u8 ref_id;
>
> - /* Check if the pin reference is connected */
> - if (ref == zl3073x_dpll_connected_ref_get(zldpll)) {
> - *state = DPLL_PIN_STATE_CONNECTED;
> - return 0;
> - }
> + ref_id = zl3073x_input_pin_ref_get(pin->id);
>
> - /* If the DPLL is running in automatic mode and the reference is
> - * selectable and its monitor does not report any error then report
> - * pin as selectable.
> - */
> - if (zl3073x_chan_mode_get(chan) == ZL_DPLL_MODE_REFSEL_MODE_AUTO &&
> - zl3073x_dev_ref_is_status_ok(zldev, ref) &&
> - zl3073x_chan_ref_is_selectable(chan, ref)) {
> - *state = DPLL_PIN_STATE_SELECTABLE;
> + /* Check if this pin is the currently locked reference */
> + if (ref_id == zl3073x_dpll_connected_ref_get(zldpll)) {
> + *operstate = DPLL_PIN_OPERSTATE_ACTIVE;
> return 0;
> }
>
> - /* Otherwise report the pin as disconnected */
> - *state = DPLL_PIN_STATE_DISCONNECTED;
> + /* Check reference monitor status */
> + ref = zl3073x_ref_state_get(zldev, ref_id);
> + if (ref->mon_status & ZL_REF_MON_STATUS_LOS)
> + *operstate = DPLL_PIN_OPERSTATE_NO_SIGNAL;
> + else if (!zl3073x_ref_is_status_ok(ref))
> + *operstate = DPLL_PIN_OPERSTATE_QUAL_FAILED;
> + else
> + *operstate = DPLL_PIN_OPERSTATE_STANDBY;
>
> return 0;
> }
> @@ -551,10 +546,48 @@ zl3073x_dpll_input_pin_state_on_dpll_get(const struct dpll_pin *dpll_pin,
> void *dpll_priv,
> enum dpll_pin_state *state,
> struct netlink_ext_ack *extack)
> +{
> + struct zl3073x_dpll *zldpll = dpll_priv;
> + struct zl3073x_dpll_pin *pin = pin_priv;
> + const struct zl3073x_chan *chan;
> + u8 mode, ref;
> +
> + chan = zl3073x_chan_state_get(zldpll->dev, zldpll->id);
> + ref = zl3073x_input_pin_ref_get(pin->id);
> + mode = zl3073x_chan_mode_get(chan);
> +
> + switch (mode) {
> + case ZL_DPLL_MODE_REFSEL_MODE_REFLOCK:
> + if (ref == zl3073x_chan_ref_get(chan))
> + *state = DPLL_PIN_STATE_CONNECTED;
> + else
> + *state = DPLL_PIN_STATE_DISCONNECTED;
> + break;
> + case ZL_DPLL_MODE_REFSEL_MODE_AUTO:
> + if (zl3073x_chan_ref_is_selectable(chan, ref))
> + *state = DPLL_PIN_STATE_SELECTABLE;
> + else
> + *state = DPLL_PIN_STATE_DISCONNECTED;
> + break;
> + default:
> + *state = DPLL_PIN_STATE_DISCONNECTED;
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +zl3073x_dpll_input_pin_operstate_on_dpll_get(const struct dpll_pin *dpll_pin,
> + void *pin_priv,
> + const struct dpll_device *dpll,
> + void *dpll_priv,
> + enum dpll_pin_operstate *operstate,
> + struct netlink_ext_ack *extack)
> {
> struct zl3073x_dpll_pin *pin = pin_priv;
>
> - return zl3073x_dpll_ref_state_get(pin, state);
> + return zl3073x_dpll_ref_operstate_get(pin, operstate);
> }
>
> static int
> @@ -1248,6 +1281,7 @@ static const struct dpll_pin_ops zl3073x_dpll_input_pin_ops = {
> .frequency_get = zl3073x_dpll_input_pin_frequency_get,
> .frequency_set = zl3073x_dpll_input_pin_frequency_set,
> .measured_freq_get = zl3073x_dpll_input_pin_measured_freq_get,
> + .operstate_on_dpll_get = zl3073x_dpll_input_pin_operstate_on_dpll_get,
> .phase_offset_get = zl3073x_dpll_input_pin_phase_offset_get,
> .phase_adjust_get = zl3073x_dpll_input_pin_phase_adjust_get,
> .phase_adjust_set = zl3073x_dpll_input_pin_phase_adjust_set,
> @@ -1663,7 +1697,7 @@ zl3073x_dpll_pin_phase_offset_check(struct zl3073x_dpll_pin *pin)
> * 2) For other pins use appropriate ref_phase register if the phase
> * monitor feature is enabled.
> */
> - if (pin->pin_state == DPLL_PIN_STATE_CONNECTED)
> + if (pin->operstate == DPLL_PIN_OPERSTATE_ACTIVE)
> reg = ZL_REG_DPLL_PHASE_ERR_DATA(zldpll->id);
> else if (zldpll->phase_monitor)
> reg = ZL_REG_REF_PHASE(ref_id);
> @@ -1828,7 +1862,7 @@ zl3073x_dpll_changes_check(struct zl3073x_dpll *zldpll)
> }
>
> list_for_each_entry(pin, &zldpll->pins, list) {
> - enum dpll_pin_state state;
> + enum dpll_pin_operstate operstate;
> bool pin_changed = false;
>
> /* Output pins change checks are not necessary because output
> @@ -1837,18 +1871,18 @@ zl3073x_dpll_changes_check(struct zl3073x_dpll *zldpll)
> if (!zl3073x_dpll_is_input_pin(pin))
> continue;
>
> - rc = zl3073x_dpll_ref_state_get(pin, &state);
> + rc = zl3073x_dpll_ref_operstate_get(pin, &operstate);
> if (rc) {
> dev_err(dev,
> - "Failed to get %s on DPLL%u state: %pe\n",
> + "Failed to get %s on DPLL%u oper state: %pe\n",
> pin->label, zldpll->id, ERR_PTR(rc));
> return;
> }
>
> - if (state != pin->pin_state) {
> - dev_dbg(dev, "%s state changed: %u->%u\n", pin->label,
> - pin->pin_state, state);
> - pin->pin_state = state;
> + if (operstate != pin->operstate) {
> + dev_dbg(dev, "%s oper state changed: %u->%u\n",
> + pin->label, pin->operstate, operstate);
> + pin->operstate = operstate;
> pin_changed = true;
> }
>
> diff --git a/drivers/dpll/zl3073x/regs.h b/drivers/dpll/zl3073x/regs.h
> index d425dc67250fe..8015808bdf548 100644
> --- a/drivers/dpll/zl3073x/regs.h
> +++ b/drivers/dpll/zl3073x/regs.h
> @@ -98,7 +98,14 @@
>
> #define ZL_REG_REF_MON_STATUS(_idx) \
> ZL_REG_IDX(_idx, 2, 0x02, 1, ZL3073X_NUM_REFS, 1)
> -#define ZL_REF_MON_STATUS_OK 0 /* all bits zeroed */
> +#define ZL_REF_MON_STATUS_OK 0
> +#define ZL_REF_MON_STATUS_LOS BIT(0)
> +#define ZL_REF_MON_STATUS_SCM BIT(1)
> +#define ZL_REF_MON_STATUS_CFM BIT(2)
> +#define ZL_REF_MON_STATUS_GST BIT(3)
> +#define ZL_REF_MON_STATUS_PFM BIT(4)
> +#define ZL_REF_MON_STATUS_ESYNC BIT(6)
> +#define ZL_REF_MON_STATUS_SPLIT_XO BIT(7)
>
> #define ZL_REG_DPLL_MON_STATUS(_idx) \
> ZL_REG_IDX(_idx, 2, 0x10, 1, ZL3073X_MAX_CHANNELS, 1)

Reviewed-by: Petr Oros <poros@redhat.com>