Re: [PATCH v2 04/17] uapi: Add ARM SCMI definitions

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Wed, 14 Jan 2026 11:46:08 +0000
Cristian Marussi <cristian.marussi@arm.com> wrote:

> Add a number of structures and ioctls definitions used by the ARM
> SCMI Telemetry protocol.
> 
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>

A few drive by comments.

> diff --git a/include/uapi/linux/scmi.h b/include/uapi/linux/scmi.h
> new file mode 100644
> index 000000000000..e4e9939a1bf8
> --- /dev/null
> +++ b/include/uapi/linux/scmi.h
> @@ -0,0 +1,287 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Copyright (C) 2026 ARM Ltd.
> + */
> +#ifndef _UAPI_LINUX_SCMI_H
> +#define _UAPI_LINUX_SCMI_H
> +
> +/*
> + * Userspace interface SCMI Telemetry
> + */
> +
> +#include <linux/ioctl.h>
> +#include <linux/types.h>


> +/**
> + * scmi_tlm_intervals  - Update intervals descriptor
> + *
> + * @discrete: Flag to indicate the nature of the intervals described in
> + *	      @update_intervals.
> + *	      When 'false' @update_intervals is a triplet: min/max/step
> + * @pad: Padding fields to enforce alignment.
> + * @num: Number of entries of @update_intervals
> + * @update_intervals: A variably-sized array containing the update intervals
> + *
> + * Used by:
> + *	RW - SCMI_TLM_GET_INTRVS
> + *
> + * Supported by:
> + *	control/
> + *	groups/<N>/control
> + */
> +struct scmi_tlm_intervals {
> +	__u8 discrete;
> +	__u8 pad[3];
> +	__u32 num;

Trivial but this seems a little inconsistent. In other
'num' entries (e.g. num_des) below a more specific name
is used.

> +#define SCMI_TLM_UPDATE_INTVL_SEGMENT_LOW	0
> +#define SCMI_TLM_UPDATE_INTVL_SEGMENT_HIGH	1
> +#define SCMI_TLM_UPDATE_INTVL_SEGMENT_STEP	2
> +	__u32 update_intervals[] __counted_by(num);
> +};

> +
> +/**
> + * scmi_tlm_des_list  - List of all defined DEs
> + *
> + * @num_des: Number of entries in @des
> + * @des: An array containing descriptors for all defined DEs
> + *
> + * Used by:
> + *	RW - SCMI_TLM_GET_DE_LIST
> + *
> + * Supported by:
> + *	control/
> + */
> +struct scmi_tlm_des_list {
> +	__u32 num_des;
> +	struct scmi_tlm_de_info des[] __counted_by(num_des);
> +};
> +
> +/**
> + * scmi_tlm_de_sample - A DE reading
> + *
> + * @id: DE identifier
> + * @tstamp: DE reading timestamp (equal 0 is NOT supported)
> + * @val: Reading of the DE data value
> + *
> + * Used by:
> + *	RW - SCMI_TLM_GET_DE_VALUE
> + *
> + * Supported by:
> + *	control/
> + */
> +struct scmi_tlm_de_sample {
> +	__u32 id;

Packing issues maybe if this ever ends up on 32 bit machines.
Even more so once it's in an array below.

> +	__u64 tstamp;
> +	__u64 val;
> +};
> +
> +/**
> + * scmi_tlm_data_read - Bulk read of multiple DEs
> + *
> + * @num_samples: Number of entries returned in @samples
> + * @samples: An array of samples containing an entry for each DE that was
> + *	     enabled when the single sample read request was issued.
> + *
> + * Used by:
> + *	RW - SCMI_TLM_SINGLE_SAMPLE
> + *	RW - SCMI_TLM_BULK_READ
> + *
> + * Supported by:
> + *	control/
> + *	groups/<N>/control
> + */
> +struct scmi_tlm_data_read {
> +	__u32 num_samples;
> +	struct scmi_tlm_de_sample samples[] __counted_by(num_samples);
> +};