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

[Petr Oros <poros@redhat.com>]

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>