Re: [PATCH net-next 1/2] dpll: add pin operational state

Petr Oros
2026-04-30T11:58:33Z

On 4/28/26 17:49, Ivan Vecera wrote:
> Add pin-operstate enum and operstate_on_dpll_get callback to report
> the actual hardware status of a pin with respect to its parent DPLL
> device. Unlike pin-state (which reflects administrative intent set
> by the user), operstate reflects what the hardware is actually doing.
>
> Defined operational states:
> - active: pin is qualified and actively used by the DPLL
> - standby: pin is qualified but not actively used by the DPLL
> - no-signal: pin does not have a valid signal
> - qual-failed: pin signal failed qualification
>
> The operstate is reported inside the pin-parent-device nested
> attribute alongside the existing state and phase-offset attributes.
>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
> Documentation/driver-api/dpll.rst | 38 ++++++++++++++++-----------
> Documentation/netlink/specs/dpll.yaml | 31 ++++++++++++++++++++++
> drivers/dpll/dpll_netlink.c | 27 +++++++++++++++++++
> drivers/dpll/dpll_nl.c | 3 ++-
> drivers/dpll/dpll_nl.h | 2 +-
> include/linux/dpll.h | 6 +++++
> include/uapi/linux/dpll.h | 23 ++++++++++++++++
> 7 files changed, 113 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/driver-api/dpll.rst b/Documentation/driver-api/dpll.rst
> index 93c191b2d0898..37eaef785e304 100644
> --- a/Documentation/driver-api/dpll.rst
> +++ b/Documentation/driver-api/dpll.rst
> @@ -65,35 +65,43 @@ request, where user provides attributes that result in single pin match.
> Pin selection
> =============
>
> -In general, selected pin (the one which signal is driving the dpll
> -device) can be obtained from ``DPLL_A_PIN_STATE`` attribute, and only
> -one pin shall be in ``DPLL_PIN_STATE_CONNECTED`` state for any dpll
> -device.
> +Pin state (``DPLL_A_PIN_STATE``) reflects the administrative intent set
> +by the user. Pin operational state (``DPLL_A_PIN_OPERSTATE``) reflects
> +what the hardware is actually doing with the pin.
>
> Pin selection can be done either manually or automatically, depending
> on hardware capabilities and active dpll device work mode
> (``DPLL_A_MODE`` attribute). The consequence is that there are
> -differences for each mode in terms of available pin states, as well as
> -for the states the user can request for a dpll device.
> +differences for each mode in terms of available pin states the user can
> +request for a dpll device.
>
> -In manual mode (``DPLL_MODE_MANUAL``) the user can request or receive
> -one of following pin states:
> +In manual mode (``DPLL_MODE_MANUAL``) the user can request one of
> +following pin states:
>
> -- ``DPLL_PIN_STATE_CONNECTED`` - the pin is used to drive dpll device
> -- ``DPLL_PIN_STATE_DISCONNECTED`` - the pin is not used to drive dpll
> +- ``DPLL_PIN_STATE_CONNECTED`` - the pin is selected to drive dpll
> device
> +- ``DPLL_PIN_STATE_DISCONNECTED`` - the pin is not selected to drive
> + dpll device
>
> -In automatic mode (``DPLL_MODE_AUTOMATIC``) the user can request or
> -receive one of following pin states:
> +In automatic mode (``DPLL_MODE_AUTOMATIC``) the user can request one of
> +following pin states:
>
> - ``DPLL_PIN_STATE_SELECTABLE`` - the pin shall be considered as valid
> input for automatic selection algorithm
> - ``DPLL_PIN_STATE_DISCONNECTED`` - the pin shall be not considered as
> a valid input for automatic selection algorithm
>
> -In automatic mode (``DPLL_MODE_AUTOMATIC``) the user can only receive
> -pin state ``DPLL_PIN_STATE_CONNECTED`` once automatic selection
> -algorithm locks a dpll device with one of the inputs.
> +The actual hardware status of a pin is reported via the operational
> +state (``DPLL_A_PIN_OPERSTATE``) attribute nested under the parent
> +device:
> +
> +- ``DPLL_PIN_OPERSTATE_ACTIVE`` - pin is qualified and actively used
> + by the DPLL
> +- ``DPLL_PIN_OPERSTATE_STANDBY`` - pin is qualified but not actively
> + used by the DPLL
> +- ``DPLL_PIN_OPERSTATE_NO_SIGNAL`` - pin does not have a valid signal
> +- ``DPLL_PIN_OPERSTATE_QUAL_FAILED`` - pin signal failed qualification
> + checks
>
> Shared pins
> ===========
> diff --git a/Documentation/netlink/specs/dpll.yaml b/Documentation/netlink/specs/dpll.yaml
> index 40465a3d7fc20..c45de70a47ce6 100644
> --- a/Documentation/netlink/specs/dpll.yaml
> +++ b/Documentation/netlink/specs/dpll.yaml
> @@ -212,6 +212,27 @@ definitions:
> name: selectable
> doc: pin enabled for automatic input selection
> render-max: true
> + -
> + type: enum
> + name: pin-operstate
> + doc: |
> + defines possible operational states of a pin with respect to its
> + parent DPLL device, valid values for DPLL_A_PIN_OPERSTATE attribute
> + entries:
> + -
> + name: active
> + doc: pin is qualified and actively used by the DPLL
> + value: 1
> + -
> + name: standby
> + doc: pin is qualified but not actively used by the DPLL
> + -
> + name: no-signal
> + doc: pin does not have a valid signal
> + -
> + name: qual-failed
> + doc: pin signal failed qualification (e.g. frequency or phase monitor)
> + render-max: true
> -
> type: flags
> name: pin-capabilities
> @@ -488,6 +509,14 @@ attribute-sets:
> Value of (DPLL_A_PIN_MEASURED_FREQUENCY %
> DPLL_PIN_MEASURED_FREQUENCY_DIVIDER) is a fractional part
> of a measured frequency value.
> + -
> + name: operstate
> + type: u32
> + enum: pin-operstate
> + doc: |
> + Operational state of the pin with respect to its parent DPLL
> + device. Unlike state (which reflects the administrative intent),
> + operstate reflects the actual hardware status.
>
> -
> name: pin-parent-device
> @@ -501,6 +530,8 @@ attribute-sets:
> name: prio
> -
> name: state
> + -
> + name: operstate
> -
> name: phase-offset
> -
> diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
> index af7ce62ec55ca..05cf946b4be5e 100644
> --- a/drivers/dpll/dpll_netlink.c
> +++ b/drivers/dpll/dpll_netlink.c
> @@ -324,6 +324,30 @@ dpll_msg_add_pin_on_dpll_state(struct sk_buff *msg, struct dpll_pin *pin,
> return 0;
> }
>
> +static int
> +dpll_msg_add_pin_operstate(struct sk_buff *msg, struct dpll_pin *pin,
> + struct dpll_pin_ref *ref,
> + struct netlink_ext_ack *extack)
> +{
> + const struct dpll_pin_ops *ops = dpll_pin_ops(ref);
> + struct dpll_device *dpll = ref->dpll;
> + enum dpll_pin_operstate operstate;
> + int ret;
> +
> + if (!ops->operstate_on_dpll_get)
> + return 0;
> + ret = ops->operstate_on_dpll_get(pin,
> + dpll_pin_on_dpll_priv(dpll, pin),
> + dpll, dpll_priv(dpll),
> + &operstate, extack);
> + if (ret)
> + return ret;
> + if (nla_put_u32(msg, DPLL_A_PIN_OPERSTATE, operstate))
> + return -EMSGSIZE;
> +
> + return 0;
> +}
> +
> static int
> dpll_msg_add_pin_direction(struct sk_buff *msg, struct dpll_pin *pin,
> struct dpll_pin_ref *ref,
> @@ -650,6 +674,9 @@ dpll_msg_add_pin_dplls(struct sk_buff *msg, struct dpll_pin *pin,
> if (ret)
> goto nest_cancel;
> ret = dpll_msg_add_pin_on_dpll_state(msg, pin, ref, extack);
> + if (ret)
> + goto nest_cancel;
> + ret = dpll_msg_add_pin_operstate(msg, pin, ref, extack);
> if (ret)
> goto nest_cancel;
> ret = dpll_msg_add_pin_prio(msg, pin, ref, extack);
> diff --git a/drivers/dpll/dpll_nl.c b/drivers/dpll/dpll_nl.c
> index 1e652340a5d73..58235845fa3d5 100644
> --- a/drivers/dpll/dpll_nl.c
> +++ b/drivers/dpll/dpll_nl.c
> @@ -12,11 +12,12 @@
> #include <uapi/linux/dpll.h>
>
> /* Common nested types */
> -const struct nla_policy dpll_pin_parent_device_nl_policy[DPLL_A_PIN_PHASE_OFFSET + 1] = {
> +const struct nla_policy dpll_pin_parent_device_nl_policy[DPLL_A_PIN_OPERSTATE + 1] = {
> [DPLL_A_PIN_PARENT_ID] = { .type = NLA_U32, },
> [DPLL_A_PIN_DIRECTION] = NLA_POLICY_RANGE(NLA_U32, 1, 2),
> [DPLL_A_PIN_PRIO] = { .type = NLA_U32, },
> [DPLL_A_PIN_STATE] = NLA_POLICY_RANGE(NLA_U32, 1, 3),
> + [DPLL_A_PIN_OPERSTATE] = NLA_POLICY_RANGE(NLA_U32, 1, 4),
> [DPLL_A_PIN_PHASE_OFFSET] = { .type = NLA_S64, },
> };
>
> diff --git a/drivers/dpll/dpll_nl.h b/drivers/dpll/dpll_nl.h
> index 7419679b69779..fa8280e3dd14c 100644
> --- a/drivers/dpll/dpll_nl.h
> +++ b/drivers/dpll/dpll_nl.h
> @@ -13,7 +13,7 @@
> #include <uapi/linux/dpll.h>
>
> /* Common nested types */
> -extern const struct nla_policy dpll_pin_parent_device_nl_policy[DPLL_A_PIN_PHASE_OFFSET + 1];
> +extern const struct nla_policy dpll_pin_parent_device_nl_policy[DPLL_A_PIN_OPERSTATE + 1];
> extern const struct nla_policy dpll_pin_parent_pin_nl_policy[DPLL_A_PIN_STATE + 1];
> extern const struct nla_policy dpll_reference_sync_nl_policy[DPLL_A_PIN_STATE + 1];
>
> diff --git a/include/linux/dpll.h b/include/linux/dpll.h
> index b7277a8b484d2..b6f16c884b99e 100644
> --- a/include/linux/dpll.h
> +++ b/include/linux/dpll.h
> @@ -85,6 +85,12 @@ struct dpll_pin_ops {
> const struct dpll_device *dpll,
> void *dpll_priv, enum dpll_pin_state *state,
> struct netlink_ext_ack *extack);
> + int (*operstate_on_dpll_get)(const struct dpll_pin *pin,
> + void *pin_priv,
> + const struct dpll_device *dpll,
> + void *dpll_priv,
> + enum dpll_pin_operstate *operstate,
> + struct netlink_ext_ack *extack);
> int (*state_on_pin_set)(const struct dpll_pin *pin, void *pin_priv,
> const struct dpll_pin *parent_pin,
> void *parent_pin_priv,
> diff --git a/include/uapi/linux/dpll.h b/include/uapi/linux/dpll.h
> index 871685f7c353b..cb363cccf2e2a 100644
> --- a/include/uapi/linux/dpll.h
> +++ b/include/uapi/linux/dpll.h
> @@ -178,6 +178,28 @@ enum dpll_pin_state {
> DPLL_PIN_STATE_MAX = (__DPLL_PIN_STATE_MAX - 1)
> };
>
> +/**
> + * enum dpll_pin_operstate - defines possible operational states of a pin with
> + * respect to its parent DPLL device, valid values for DPLL_A_PIN_OPERSTATE
> + * attribute
> + * @DPLL_PIN_OPERSTATE_ACTIVE: pin is qualified and actively used by the DPLL
> + * @DPLL_PIN_OPERSTATE_STANDBY: pin is qualified but not actively used by the
> + * DPLL
> + * @DPLL_PIN_OPERSTATE_NO_SIGNAL: pin does not have a valid signal
> + * @DPLL_PIN_OPERSTATE_QUAL_FAILED: pin signal failed qualification (e.g.
> + * frequency or phase monitor)
> + */
> +enum dpll_pin_operstate {
> + DPLL_PIN_OPERSTATE_ACTIVE = 1,
> + DPLL_PIN_OPERSTATE_STANDBY,
> + DPLL_PIN_OPERSTATE_NO_SIGNAL,
> + DPLL_PIN_OPERSTATE_QUAL_FAILED,
> +
> + /* private: */
> + __DPLL_PIN_OPERSTATE_MAX,
> + DPLL_PIN_OPERSTATE_MAX = (__DPLL_PIN_OPERSTATE_MAX - 1)
> +};
> +
> /**
> * enum dpll_pin_capabilities - defines possible capabilities of a pin, valid
> * flags on DPLL_A_PIN_CAPABILITIES attribute
> @@ -257,6 +279,7 @@ enum dpll_a_pin {
> DPLL_A_PIN_PHASE_ADJUST_GRAN,
> DPLL_A_PIN_FRACTIONAL_FREQUENCY_OFFSET_PPT,
> DPLL_A_PIN_MEASURED_FREQUENCY,
> + DPLL_A_PIN_OPERSTATE,
>
> __DPLL_A_PIN_MAX,
> DPLL_A_PIN_MAX = (__DPLL_A_PIN_MAX - 1)

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