Re: [PATCH iproute2-next 2/2] dpll: add frequency monitoring support

[Petr Oros <poros@redhat.com>]

On 4/28/26 17:21, Ivan Vecera wrote:
> Add support for the new frequency monitoring feature from the kernel
> patch series "dpll: add actual frequency monitoring feature". This
> includes:
>
> - DPLL_A_FREQUENCY_MONITOR device attribute (enable/disable)
> - DPLL_A_PIN_MEASURED_FREQUENCY pin attribute displayed as fractional Hz
>    using DPLL_PR_MEASURED_FREQUENCY macro (kernel reports in mHz)
> - device set: frequency-monitor { enable | disable }
> - Refactor phase-offset-monitor to use new dpll_parse_attr_feature_state
>    helper shared with frequency-monitor
> - Update man page and bash-completion
>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
>   bash-completion/dpll |  4 +--
>   dpll/dpll.c          | 62 ++++++++++++++++++++++++++++++++++----------
>   man/man8/dpll.8      | 19 ++++++++++++--
>   3 files changed, 68 insertions(+), 17 deletions(-)
>
> diff --git a/bash-completion/dpll b/bash-completion/dpll
> index 542b99c2fce2..7ddcf529d429 100644
> --- a/bash-completion/dpll
> +++ b/bash-completion/dpll
> @@ -100,7 +100,7 @@ _dpll_device()
>                       COMPREPLY=( $( compgen -W "automatic manual" -- "$cur" ) )
>                       return 0
>                       ;;
> -                phase-offset-monitor)
> +                phase-offset-monitor|frequency-monitor)
>                       COMPREPLY=( $( compgen -W "enable disable true false 0 1" -- "$cur" ) )
>                       return 0
>                       ;;
> @@ -110,7 +110,7 @@ _dpll_device()
>                       ;;
>                   *)
>                       COMPREPLY=( $( compgen -W "id mode phase-offset-monitor \
> -                        phase-offset-avg-factor" -- "$cur" ) )
> +                        phase-offset-avg-factor frequency-monitor" -- "$cur" ) )
>                       return 0
>                       ;;
>               esac
> diff --git a/dpll/dpll.c b/dpll/dpll.c
> index e8056ff6a24b..6d8c0cbb8a34 100644
> --- a/dpll/dpll.c
> +++ b/dpll/dpll.c
> @@ -313,6 +313,26 @@ static int dpll_parse_attr_str(struct dpll *dpll, struct nlmsghdr *nlh,
>   	return 0;
>   }
>   
> +static int dpll_parse_attr_feature_state(struct dpll *dpll,
> +					struct nlmsghdr *nlh,
> +					const char *arg_name, int attr_id)
> +{
> +	const char *str = dpll_argv_next(dpll);
> +	bool val;
> +
> +	if (!str) {
> +		pr_err("%s requires an argument\n", arg_name);
> +		return -EINVAL;
> +	}
> +	if (str_to_bool(str, &val)) {
> +		pr_err("invalid %s value: %s (use enable/disable)\n",
> +		       arg_name, str);
> +		return -EINVAL;
> +	}
> +	mnl_attr_put_u32(nlh, attr_id, val ? 1 : 0);
> +	return 0;
> +}
> +
>   static int dpll_parse_attr_enum(struct dpll *dpll, struct nlmsghdr *nlh,
>   				const char *arg_name, int attr_id,
>   				int (*parse_func)(struct dpll *, __u32 *))
> @@ -425,6 +445,21 @@ static __s64 mnl_attr_get_sint(const struct nlattr *attr)
>   		}                                                              \
>   	} while (0)
>   
> +/* Measured frequency - JSON prints raw mHz value, FP prints fractional Hz */
> +#define DPLL_PR_MEASURED_FREQUENCY(tb, attr_id)                                \
> +	do {                                                                   \
> +		if (tb[attr_id]) {                                             \
> +			__u64 val = mnl_attr_get_u64(tb[attr_id]);             \
> +			lldiv_t d = lldiv(val,                                 \
> +				DPLL_PIN_MEASURED_FREQUENCY_DIVIDER);           \
> +			print_lluint(PRINT_JSON, "measured-frequency",         \
> +				     NULL, val);                                \
> +			print_s64(PRINT_FP, NULL,                              \
> +				  "  measured-frequency: %lld.", d.quot);       \
> +			print_s64(PRINT_FP, NULL, "%03lld Hz\n", d.rem);       \
> +		}                                                              \
> +	} while (0)
> +
>   /* Generic version with custom format */
>   #define DPLL_PR_ENUM_STR_FMT(tb, attr_id, name, format_str, name_func)         \
>   	do {                                                                   \
> @@ -657,6 +692,7 @@ static void cmd_device_help(void)
>   	pr_err("       dpll device set id DEVICE_ID [ mode { automatic | manual } ]\n");
>   	pr_err("                                    [ phase-offset-monitor { enable | disable } ]\n");
>   	pr_err("                                    [ phase-offset-avg-factor NUM ]\n");
> +	pr_err("                                    [ frequency-monitor { enable | disable } ]\n");
>   	pr_err("       dpll device id-get [ module-name NAME ] [ clock-id ID ] [ type TYPE ]\n");
>   }
>   
> @@ -1058,6 +1094,10 @@ static void dpll_device_print_attrs(const struct nlmsghdr *nlh,
>   			     str_enable_disable);
>   	DPLL_PR_UINT(tb, DPLL_A_PHASE_OFFSET_AVG_FACTOR,
>   		     "phase-offset-avg-factor");
> +	DPLL_PR_ENUM_STR_FMT(tb, DPLL_A_FREQUENCY_MONITOR,
> +			     "frequency-monitor",
> +			     "  frequency-monitor: %s\n",
> +			     str_enable_disable);
>   }
>   
>   /* Netlink callback - device get (single device) */
> @@ -1219,25 +1259,20 @@ static int cmd_device_set(struct dpll *dpll)
>   						 dpll_parse_mode))
>   				return -EINVAL;
>   		} else if (dpll_argv_match(dpll, "phase-offset-monitor")) {
> -			const char *str = dpll_argv_next(dpll);
> -			bool val;
> -
> -			if (!str) {
> -				pr_err("phase-offset-monitor requires an argument\n");
> -				return -EINVAL;
> -			}
> -			if (str_to_bool(str, &val)) {
> -				pr_err("invalid phase-offset-monitor value: %s (use enable/disable)\n",
> -				       str);
> +			if (dpll_parse_attr_feature_state(dpll, nlh,
> +							  "phase-offset-monitor",
> +							  DPLL_A_PHASE_OFFSET_MONITOR))
>   				return -EINVAL;
> -			}
> -			mnl_attr_put_u32(nlh, DPLL_A_PHASE_OFFSET_MONITOR,
> -					 val ? 1 : 0);
>   		} else if (dpll_argv_match(dpll, "phase-offset-avg-factor")) {
>   			if (dpll_parse_attr_u32(dpll, nlh,
>   						"phase-offset-avg-factor",
>   						DPLL_A_PHASE_OFFSET_AVG_FACTOR))
>   				return -EINVAL;
> +		} else if (dpll_argv_match(dpll, "frequency-monitor")) {
> +			if (dpll_parse_attr_feature_state(dpll, nlh,
> +							  "frequency-monitor",
> +							  DPLL_A_FREQUENCY_MONITOR))
> +				return -EINVAL;
>   		} else {
>   			pr_err("unknown option: %s\n", dpll_argv(dpll));
>   			return -EINVAL;
> @@ -1601,6 +1636,7 @@ static void dpll_pin_print_attrs(struct nlattr **tb)
>   	DPLL_PR_ENUM_STR(tb, DPLL_A_PIN_TYPE, "type", dpll_pin_type_name);
>   	DPLL_PR_U64_FMT(tb, DPLL_A_PIN_FREQUENCY, "frequency",
>   			"  frequency: %" PRIu64 " Hz\n");
> +	DPLL_PR_MEASURED_FREQUENCY(tb, DPLL_A_PIN_MEASURED_FREQUENCY);
>   
>   	dpll_pin_print_freq_supported(tb[DPLL_A_PIN_FREQUENCY_SUPPORTED]);
>   
> diff --git a/man/man8/dpll.8 b/man/man8/dpll.8
> index 89f17af74923..59ec4208f251 100644
> --- a/man/man8/dpll.8
> +++ b/man/man8/dpll.8
> @@ -111,7 +111,7 @@ Temperature (if supported)
>   Type (PPS or EEC)
>   .RE
>   
> -.SS dpll device set id ID [ mode { automatic | manual } ] [ phase-offset-monitor { enable | disable } ] [ phase-offset-avg-factor FACTOR ]
> +.SS dpll device set id ID [ mode { automatic | manual } ] [ phase-offset-monitor { enable | disable } ] [ phase-offset-avg-factor FACTOR ] [ frequency-monitor { enable | disable } ]
>   
>   Configure DPLL device parameters.
>   
> @@ -140,6 +140,14 @@ When enabled, the kernel continuously measures and reports phase differences.
>   Set the averaging factor (1-255) applied to phase offset calculations.
>   Higher values provide smoother but slower-responding measurements.
>   
> +.TP
> +.BI frequency-monitor " { enable | disable | true | false | 0 | 1 }"
> +Enable or disable frequency monitoring on the device. When enabled, the
> +kernel continuously measures and reports actual pin frequencies, which can
> +be read via the
> +.B measured-frequency
> +field in pin show output.
> +
>   .SS dpll device id-get [ module-name NAME ] [ clock-id ID ] [ type TYPE ]
>   
>   Retrieve the device ID based on identifying attributes. Useful for scripting
> @@ -233,7 +241,9 @@ Board label (hardware label from device tree or ACPI)
>   .IP \[bu]
>   Pin type (mux, ext, synce-eth-port, int-oscillator, gnss)
>   .IP \[bu]
> -Frequency and supported frequency ranges
> +Configured frequency and supported frequency ranges
> +.IP \[bu]
> +Measured frequency in Hz (when frequency monitoring is enabled)
>   .IP \[bu]
>   Capabilities (state-can-change, priority-can-change, direction-can-change)
>   .IP \[bu]
> @@ -372,6 +382,11 @@ Press Ctrl+C to stop monitoring.
>   .B dpll device set id 0 phase-offset-monitor enable
>   .fi
>   
> +.SS Enable frequency monitoring on device 0
> +.nf
> +.B dpll device set id 0 frequency-monitor enable
> +.fi
> +
>   .SS Show all EEC devices
>   .nf
>   .B dpll device show type eec
Reviewed-by: Petr Oros <poros@redhat.com>