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

[Ivan Vecera <ivecera@redhat.com>]

On 4/1/26 5:10 AM, Jakub Kicinski wrote:
>> +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.

You're right, making it optional was not well thought out. I'm going to 
change it so that .freq_monitor_get() will be required when
.measured_freq_get() is provided.

Thanks,
Ivan