Re: [PATCH net-next v2 2/3] dpll: add frequency monitoring callback ops

[Jakub Kicinski <kuba@kernel.org>]

On Mon, 30 Mar 2026 12:55:04 +0200 Ivan Vecera wrote:
> +dpll_msg_add_freq_monitor(struct sk_buff *msg, struct dpll_device *dpll,

> +	if (ops->freq_monitor_set && ops->freq_monitor_get) {
> +		ret = ops->freq_monitor_get(dpll, dpll_priv(dpll),
> +					    &state, extack);
> +		if (ret)
> +			return ret;
> +		if (nla_put_u32(msg, DPLL_A_FREQUENCY_MONITOR, state))
> +			return -EMSGSIZE;
> +	}
> +
> +	return 0;
> +}
> +
>  static int
>  dpll_msg_add_phase_offset_avg_factor(struct sk_buff *msg,
>  				     struct dpll_device *dpll,
> @@ -400,6 +420,38 @@ static int dpll_msg_add_ffo(struct sk_buff *msg, struct dpll_pin *pin,
>  			    ffo);
>  }
>  
> +static int dpll_msg_add_measured_freq(struct sk_buff *msg, struct dpll_pin *pin,
> +				      struct dpll_pin_ref *ref,
> +				    struct netlink_ext_ack *extack)
> +{
> +	const struct dpll_device_ops *dev_ops = dpll_device_ops(ref->dpll);
> +	const struct dpll_pin_ops *ops = dpll_pin_ops(ref);
> +	struct dpll_device *dpll = ref->dpll;
> +	enum dpll_feature_state state;
> +	u64 measured_freq;
> +	int ret;
> +
> +	if (!ops->measured_freq_get)
> +		return 0;
> +	if (dev_ops->freq_monitor_get) {

what are you trying to cater to by making freq_monitor_get optional
here? I thought maybe some devices would have it always enabled, but
then dpll_msg_add_freq_monitor() should presumably report enabled
if !freq_monitor_get && measured_freq_get ?

Maybe there's some precedent in surrounding code outside of the context
but the intention of the patch reads a bit off.

> +		ret = dev_ops->freq_monitor_get(dpll, dpll_priv(dpll),
> +						&state, extack);
> +		if (ret)
> +			return ret;
> +		if (state == DPLL_FEATURE_STATE_DISABLE)
> +			return 0;
> +	}
> +	ret = ops->measured_freq_get(pin, dpll_pin_on_dpll_priv(dpll, pin),
> +				   dpll, dpll_priv(dpll), &measured_freq, extack);