Add new callback operations for a dpll device:
- freq_monitor_get(..) - to obtain current state of frequency monitor
feature from dpll device,
- freq_monitor_set(..) - to allow feature configuration.
Add new callback operation for a dpll pin:
- measured_freq_get(..) - to obtain the measured frequency in mHz.
Obtain the feature state value using the get callback and provide it to
the user if the device driver implements callbacks. The measured_freq_get
pin callback is only invoked when the frequency monitor is enabled.
The freq_monitor_get device callback is required when measured_freq_get
is provided by the driver.
Execute the set callback upon user requests.
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
Changes v2 -> v3:
- Made freq_monitor_get required when measured_freq_get is present (Jakub)
Changes v1 -> v2:
- Renamed actual-frequency to measured-frequency (Vadim)
---
drivers/dpll/dpll_netlink.c | 92 +++++++++++++++++++++++++++++++++++++
include/linux/dpll.h | 10 ++++
2 files changed, 102 insertions(+)
diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
index 83cbd64abf5a4..576d0cd074bd4 100644
--- a/drivers/dpll/dpll_netlink.c
+++ b/drivers/dpll/dpll_netlink.c
@@ -175,6 +175,26 @@ dpll_msg_add_phase_offset_monitor(struct sk_buff *msg, struct dpll_device *dpll,
return 0;
}
+static int
+dpll_msg_add_freq_monitor(struct sk_buff *msg, struct dpll_device *dpll,
+ struct netlink_ext_ack *extack)
+{
+ const struct dpll_device_ops *ops = dpll_device_ops(dpll);
+ enum dpll_feature_state state;
+ int ret;
+
+ 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,40 @@ 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 (WARN_ON(!dev_ops->freq_monitor_get))
+ return -EINVAL;
+ 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);
+ if (ret)
+ return ret;
+ if (nla_put_64bit(msg, DPLL_A_PIN_MEASURED_FREQUENCY,
+ sizeof(measured_freq), &measured_freq,
+ DPLL_A_PIN_PAD))
+ return -EMSGSIZE;
+
+ return 0;
+}
+
static int
dpll_msg_add_pin_freq(struct sk_buff *msg, struct dpll_pin *pin,
struct dpll_pin_ref *ref, struct netlink_ext_ack *extack)
@@ -670,6 +724,9 @@ dpll_cmd_pin_get_one(struct sk_buff *msg, struct dpll_pin *pin,
if (ret)
return ret;
ret = dpll_msg_add_ffo(msg, pin, ref, extack);
+ if (ret)
+ return ret;
+ ret = dpll_msg_add_measured_freq(msg, pin, ref, extack);
if (ret)
return ret;
ret = dpll_msg_add_pin_esync(msg, pin, ref, extack);
@@ -722,6 +779,9 @@ dpll_device_get_one(struct dpll_device *dpll, struct sk_buff *msg,
if (ret)
return ret;
ret = dpll_msg_add_phase_offset_avg_factor(msg, dpll, extack);
+ if (ret)
+ return ret;
+ ret = dpll_msg_add_freq_monitor(msg, dpll, extack);
if (ret)
return ret;
@@ -948,6 +1008,32 @@ dpll_phase_offset_avg_factor_set(struct dpll_device *dpll, struct nlattr *a,
extack);
}
+static int
+dpll_freq_monitor_set(struct dpll_device *dpll, struct nlattr *a,
+ struct netlink_ext_ack *extack)
+{
+ const struct dpll_device_ops *ops = dpll_device_ops(dpll);
+ enum dpll_feature_state state = nla_get_u32(a), old_state;
+ int ret;
+
+ if (!(ops->freq_monitor_set && ops->freq_monitor_get)) {
+ NL_SET_ERR_MSG_ATTR(extack, a,
+ "dpll device not capable of frequency monitor");
+ return -EOPNOTSUPP;
+ }
+ ret = ops->freq_monitor_get(dpll, dpll_priv(dpll), &old_state,
+ extack);
+ if (ret) {
+ NL_SET_ERR_MSG(extack,
+ "unable to get current state of frequency monitor");
+ return ret;
+ }
+ if (state == old_state)
+ return 0;
+
+ return ops->freq_monitor_set(dpll, dpll_priv(dpll), state, extack);
+}
+
static int
dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a,
struct netlink_ext_ack *extack)
@@ -1878,6 +1964,12 @@ dpll_set_from_nlattr(struct dpll_device *dpll, struct genl_info *info)
if (ret)
return ret;
break;
+ case DPLL_A_FREQUENCY_MONITOR:
+ ret = dpll_freq_monitor_set(dpll, a,
+ info->extack);
+ if (ret)
+ return ret;
+ break;
}
}
diff --git a/include/linux/dpll.h b/include/linux/dpll.h
index 2ce295b46b8cd..b7277a8b484d2 100644
--- a/include/linux/dpll.h
+++ b/include/linux/dpll.h
@@ -52,6 +52,12 @@ struct dpll_device_ops {
int (*phase_offset_avg_factor_get)(const struct dpll_device *dpll,
void *dpll_priv, u32 *factor,
struct netlink_ext_ack *extack);
+ int (*freq_monitor_set)(const struct dpll_device *dpll, void *dpll_priv,
+ enum dpll_feature_state state,
+ struct netlink_ext_ack *extack);
+ int (*freq_monitor_get)(const struct dpll_device *dpll, void *dpll_priv,
+ enum dpll_feature_state *state,
+ struct netlink_ext_ack *extack);
};
struct dpll_pin_ops {
@@ -110,6 +116,10 @@ struct dpll_pin_ops {
int (*ffo_get)(const struct dpll_pin *pin, void *pin_priv,
const struct dpll_device *dpll, void *dpll_priv,
s64 *ffo, struct netlink_ext_ack *extack);
+ int (*measured_freq_get)(const struct dpll_pin *pin, void *pin_priv,
+ const struct dpll_device *dpll,
+ void *dpll_priv, u64 *measured_freq,
+ struct netlink_ext_ack *extack);
int (*esync_set)(const struct dpll_pin *pin, void *pin_priv,
const struct dpll_device *dpll, void *dpll_priv,
u64 freq, struct netlink_ext_ack *extack);
--
2.52.0On 01/04/2026 10:12, Ivan Vecera wrote:
> Add new callback operations for a dpll device:
> - freq_monitor_get(..) - to obtain current state of frequency monitor
> feature from dpll device,
> - freq_monitor_set(..) - to allow feature configuration.
>
> Add new callback operation for a dpll pin:
> - measured_freq_get(..) - to obtain the measured frequency in mHz.
>
> Obtain the feature state value using the get callback and provide it to
> the user if the device driver implements callbacks. The measured_freq_get
> pin callback is only invoked when the frequency monitor is enabled.
> The freq_monitor_get device callback is required when measured_freq_get
> is provided by the driver.
>
> Execute the set callback upon user requests.
>
> Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
> Changes v2 -> v3:
> - Made freq_monitor_get required when measured_freq_get is present (Jakub)
>
> Changes v1 -> v2:
> - Renamed actual-frequency to measured-frequency (Vadim)
> ---
> drivers/dpll/dpll_netlink.c | 92 +++++++++++++++++++++++++++++++++++++
> include/linux/dpll.h | 10 ++++
> 2 files changed, 102 insertions(+)
>
> diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
> index 83cbd64abf5a4..576d0cd074bd4 100644
> --- a/drivers/dpll/dpll_netlink.c
> +++ b/drivers/dpll/dpll_netlink.c
> @@ -175,6 +175,26 @@ dpll_msg_add_phase_offset_monitor(struct sk_buff *msg, struct dpll_device *dpll,
> return 0;
> }
>
> +static int
> +dpll_msg_add_freq_monitor(struct sk_buff *msg, struct dpll_device *dpll,
> + struct netlink_ext_ack *extack)
> +{
> + const struct dpll_device_ops *ops = dpll_device_ops(dpll);
> + enum dpll_feature_state state;
> + int ret;
> +
> + 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,40 @@ 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 (WARN_ON(!dev_ops->freq_monitor_get))
> + return -EINVAL;
I think pin registration function has to be adjusted to not allow
measured_freq_get() callback if device doesn't have freq_monitor_get()
callback (or both freq_monitor_{s,g}et). Then this defensive part can
be completely removed.
> + ret = dev_ops->freq_monitor_get(dpll, dpll_priv(dpll),
Hi Vadim,
1. dubna 2026 16:47:21 SELČ, Vadim Fedorenko <vadim.fedorenko@linux.dev> napsal:
>On 01/04/2026 10:12, Ivan Vecera wrote:
>> Add new callback operations for a dpll device:
>> - freq_monitor_get(..) - to obtain current state of frequency monitor
>> feature from dpll device,
>> - freq_monitor_set(..) - to allow feature configuration.
>>
>> Add new callback operation for a dpll pin:
>> - measured_freq_get(..) - to obtain the measured frequency in mHz.
>>
>> Obtain the feature state value using the get callback and provide it to
>> the user if the device driver implements callbacks. The measured_freq_get
>> pin callback is only invoked when the frequency monitor is enabled.
>> The freq_monitor_get device callback is required when measured_freq_get
>> is provided by the driver.
>>
>> Execute the set callback upon user requests.
>>
>> Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
>> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>> ---
>> Changes v2 -> v3:
>> - Made freq_monitor_get required when measured_freq_get is present (Jakub)
>>
>> Changes v1 -> v2:
>> - Renamed actual-frequency to measured-frequency (Vadim)
>> ---
>> drivers/dpll/dpll_netlink.c | 92 +++++++++++++++++++++++++++++++++++++
>> include/linux/dpll.h | 10 ++++
>> 2 files changed, 102 insertions(+)
>>
>> diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
>> index 83cbd64abf5a4..576d0cd074bd4 100644
>> --- a/drivers/dpll/dpll_netlink.c
>> +++ b/drivers/dpll/dpll_netlink.c
>> @@ -175,6 +175,26 @@ dpll_msg_add_phase_offset_monitor(struct sk_buff *msg, struct dpll_device *dpll,
>> return 0;
>> }
>> +static int
>> +dpll_msg_add_freq_monitor(struct sk_buff *msg, struct dpll_device *dpll,
>> + struct netlink_ext_ack *extack)
>> +{
>> + const struct dpll_device_ops *ops = dpll_device_ops(dpll);
>> + enum dpll_feature_state state;
>> + int ret;
>> +
>> + 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,40 @@ 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 (WARN_ON(!dev_ops->freq_monitor_get))
>> + return -EINVAL;
>
>I think pin registration function has to be adjusted to not allow
>measured_freq_get() callback if device doesn't have freq_monitor_get()
>callback (or both freq_monitor_{s,g}et). Then this defensive part can
>be completely removed.
Ok, make sense... Will move such check to pin registration function...
Q: with WARN_ON or without?
Thanks
Ivan
On 01/04/2026 17:29, Ivan Vecera wrote:
> Hi Vadim,
>
> 1. dubna 2026 16:47:21 SELČ, Vadim Fedorenko <vadim.fedorenko@linux.dev> napsal:
>> On 01/04/2026 10:12, Ivan Vecera wrote:
>>> Add new callback operations for a dpll device:
>>> - freq_monitor_get(..) - to obtain current state of frequency monitor
>>> feature from dpll device,
>>> - freq_monitor_set(..) - to allow feature configuration.
>>>
>>> Add new callback operation for a dpll pin:
>>> - measured_freq_get(..) - to obtain the measured frequency in mHz.
>>>
>>> Obtain the feature state value using the get callback and provide it to
>>> the user if the device driver implements callbacks. The measured_freq_get
>>> pin callback is only invoked when the frequency monitor is enabled.
>>> The freq_monitor_get device callback is required when measured_freq_get
>>> is provided by the driver.
>>>
>>> Execute the set callback upon user requests.
>>>
>>> Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
>>> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>>> ---
>>> Changes v2 -> v3:
>>> - Made freq_monitor_get required when measured_freq_get is present (Jakub)
>>>
>>> Changes v1 -> v2:
>>> - Renamed actual-frequency to measured-frequency (Vadim)
>>> ---
>>> drivers/dpll/dpll_netlink.c | 92 +++++++++++++++++++++++++++++++++++++
>>> include/linux/dpll.h | 10 ++++
>>> 2 files changed, 102 insertions(+)
>>>
>>> diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
>>> index 83cbd64abf5a4..576d0cd074bd4 100644
>>> --- a/drivers/dpll/dpll_netlink.c
>>> +++ b/drivers/dpll/dpll_netlink.c
>>> @@ -175,6 +175,26 @@ dpll_msg_add_phase_offset_monitor(struct sk_buff *msg, struct dpll_device *dpll,
>>> return 0;
>>> }
>>> +static int
>>> +dpll_msg_add_freq_monitor(struct sk_buff *msg, struct dpll_device *dpll,
>>> + struct netlink_ext_ack *extack)
>>> +{
>>> + const struct dpll_device_ops *ops = dpll_device_ops(dpll);
>>> + enum dpll_feature_state state;
>>> + int ret;
>>> +
>>> + 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,40 @@ 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 (WARN_ON(!dev_ops->freq_monitor_get))
>>> + return -EINVAL;
>>
>> I think pin registration function has to be adjusted to not allow
>> measured_freq_get() callback if device doesn't have freq_monitor_get()
>> callback (or both freq_monitor_{s,g}et). Then this defensive part can
>> be completely removed.
>
> Ok, make sense... Will move such check to pin registration function...
>
> Q: with WARN_ON or without?
Well, we have to provide reason for blocking device registration
somehow, and the only way to this is via "WARN_ON"...
>
> Thanks
> Ivan
>
© 2016 - 2026 Red Hat, Inc.