Thread (40 messages) 40 messages, 3 authors, 2026-09-08

Re: [PATCH v10 06/24] firmware: arm_scmi: Add basic Telemetry support

flat view

From: Jonathan Cameron <hidden>
Date: 2026-08-24 21:05:06
Also in: arm-scmi, linux-doc, lkml

On Sun, 16 Aug 2026 00:25:46 +0100
Cristian Marussi [off-list ref] wrote:
Add SCMIv4.0 Telemetry basic support to enable initialization and resources
enumeration: add all the telemetry messages definitions and parsing logic
but only a few simple state gathering protocol operations.

Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
Obviously this is huge. I'm not going to look through it all today, but
some quick comments form the part I did look at.

Jonathan

...
quoted hunk ↗ jump to hunk
+static void scmi_telemetry_resources_free(void *arg)
+{
+	struct telemetry_info *ti = arg;
+	struct scmi_telemetry_res_info *rinfo = ti->rinfo;
+
+	/* Ensure rinfo is no more accessible upfront */
+	smp_store_release(&ti->rinfo, NULL);
+
+	for (int i = 0; i < rinfo->num_des; i++) {
+		struct telemetry_de *tde = to_tde(rinfo->des[i]);
+
+		scmi_telemetry_free_tde_put(ti, tde);
		scmi_telemetry_free_tde_put(ti, to_tde(rinfo->des[i]));
Seems resonable to me and avoids the need for the local variable.
quoted hunk ↗ jump to hunk
+	}
+	xa_destroy(&ti->xa_des);
+	kfree(ti->tdes);
+	kfree(rinfo->des);
+	kfree(rinfo->dei_store);
+	scmi_telemetry_groups_free(rinfo);
+	kfree(rinfo->grps);
+	kfree(rinfo->grps_store);
+
+	kfree(rinfo);
+}
quoted hunk ↗ jump to hunk
+
+/**
+ * scmi_telemetry_resources_enumerate  - Enumeration helper
+ * @ti: A reference to the telemetry info descriptor for this instance
+ *
+ * This helper is configured to be called once on the first enumeration
+ * attempt, when triggered by invoking ti->res_get() from somewhere else.
+ * Once run it substitues itself in ti->res_get() with the simple accessor
+ * __scmi_telemetry_resources_get, which returns a descriptor to the resources
+ * that were possibly discovered.
+ *
+ * Note that, while it attempts to fully enumerate Data Events and Groups, it
+ * does NOT fail when such enumerations fail, instead it simply gives up with
+ * the end result that only a partially populated, but consistent, resources
+ * descriptor will be returned; in such a case the incomplete descriptor will
+ * be marked as NOT fully_enumerated: this design enables the kernel to deal
+ * with badly implemented out-of-spec firmware support while keep on providing
+ * a minimal sane, albeit possibly incomplete, set of telemetry respources.
+ *
+ * Return: A reference to a fully or partially populated resources descriptor
+ */
+static struct scmi_telemetry_res_info *
+scmi_telemetry_resources_enumerate(struct telemetry_info *ti)
+{
+	struct scmi_telemetry_res_info *rinfo;
+	struct device *dev = ti->ph->dev;
+	int ret;
+
+	/* Ensure local rinfo is initialized */
+	rinfo = smp_load_acquire(&ti->rinfo);
+
+	/*
+	 * Ensure this init function can be called only once and
+	 * handles properly concurrent calls.
+	 */
+	if (atomic_cmpxchg(&ti->rinfo_initializing, 0, 1)) {
+		if (!completion_done(&ti->rinfo_initdone))
What's the logic here?  This waits only if others are already
waiting. Why?
+			wait_for_completion(&ti->rinfo_initdone);
+		goto out;
return 0;
quoted hunk ↗ jump to hunk
+	}
+
+	ret = scmi_telemetry_de_descriptors_get(ti);
+	if (ret) {
+		dev_err(dev, FW_BUG "Cannot fully enumerate DEs resources. Degraded system.\n");
+		goto done;
+	}
+
+	ret = scmi_telemetry_enumerate_groups_intervals(ti);
+	if (ret) {
+		dev_err(dev, FW_BUG "Cannot fully enumerate group intervals. Degraded system.\n");
+		goto done;
+	}
+
+	/* Enumeration was fully successful, ensure this is visbile */
+	smp_store_release(&rinfo->fully_enumerated, true);
+done:
+	/* Disable initialization permanently */
+	smp_store_mb(ti->res_get, __scmi_telemetry_resources_get);
+	complete_all(&ti->rinfo_initdone);
+
+out:
labels that just result in returns rarely add to readability of code.
I'd just return early instead.
quoted hunk ↗ jump to hunk
+	return rinfo;
+}
+
+/**
+ * scmi_telemetry_instance_init  - Instance initializer
+ * @ti: A reference to the telemetry info descriptor for this instance
+ *
+ * Note that this allocates and initialize all the resources possibly needed
+ * and then setups the @scmi_telemetry_resources_enumerate helper as the
sets up
quoted hunk ↗ jump to hunk
+ * default method for the first call to ti->res_get(): this mechanism enables
+ * the possibility of optionally implementing deferred enumeration policies
+ * which optionally delay the discovery phase and related SCMI message exchanges
+ * to a later point in time.
+ *
+ * Return: 0 on Success, errno otherwise
+ */
+static int scmi_telemetry_instance_init(struct telemetry_info *ti)
+{
...
quoted hunk ↗ jump to hunk
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 5ab73b1ab9aa..2850b018da0d 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -2,16 +2,18 @@
 /*
  * SCMI Message Protocol driver header
  *
- * Copyright (C) 2018-2021 ARM Ltd.
+ * Copyright (C) 2018-2026 ARM Ltd.
  */
 
 #ifndef _LINUX_SCMI_PROTOCOL_H
 #define _LINUX_SCMI_PROTOCOL_H
 
 #include <linux/bitfield.h>
+#include <linux/bitops.h>
 #include <linux/device.h>
 #include <linux/notifier.h>
 #include <linux/types.h>
+#include <linux/uuid.h>
 
 #define SCMI_MAX_STR_SIZE		64
 #define SCMI_SHORT_NAME_MAX_SIZE	16
@@ -824,6 +826,184 @@ struct scmi_pinctrl_proto_ops {
 	int (*pin_free)(const struct scmi_protocol_handle *ph, u32 pin);
 };
 
+enum scmi_telemetry_de_type {
There is quite a bit of stuff here. Seems only related to telemetry
so maybe a more specific header makes sense for just scmi_telemetry?

The same applies for some of the other stuff already in this header
like the sensors protocol elements.
quoted hunk ↗ jump to hunk
+	SCMI_TLM_DE_TYPE_USPECIFIED,
+	SCMI_TLM_DE_TYPE_ACCUMUL_IDLE_RESIDENCY,
+	SCMI_TLM_DE_TYPE_ACCUMUL_IDLE_COUNTS,
+	SCMI_TLM_DE_TYPE_ACCUMUL_OTHERS,
+	SCMI_TLM_DE_TYPE_INSTA_IDLE_STATE,
+	SCMI_TLM_DE_TYPE_INSTA_OTHERS,
+	SCMI_TLM_DE_TYPE_AVERAGE,
+	SCMI_TLM_DE_TYPE_STATUS,
+	SCMI_TLM_DE_TYPE_RESERVED_START,
+	SCMI_TLM_DE_TYPE_RESERVED_END = 0xef,
+	SCMI_TLM_DE_TYPE_OEM_START = 0xf0,
+	SCMI_TLM_DE_TYPE_OEM_END = 0xff,
+};
+
+enum scmi_telemetry_compo_type {
+	SCMI_TLM_COMPO_TYPE_USPECIFIED,
+	SCMI_TLM_COMPO_TYPE_CPU,
+	SCMI_TLM_COMPO_TYPE_CLUSTER,
+	SCMI_TLM_COMPO_TYPE_GPU,
+	SCMI_TLM_COMPO_TYPE_NPU,
+	SCMI_TLM_COMPO_TYPE_INTERCONNECT,
+	SCMI_TLM_COMPO_TYPE_MEM_CNTRL,
+	SCMI_TLM_COMPO_TYPE_L1_CACHE,
+	SCMI_TLM_COMPO_TYPE_L2_CACHE,
+	SCMI_TLM_COMPO_TYPE_L3_CACHE,
+	SCMI_TLM_COMPO_TYPE_LL_CACHE,
+	SCMI_TLM_COMPO_TYPE_SYS_CACHE,
+	SCMI_TLM_COMPO_TYPE_DISP_CNTRL,
+	SCMI_TLM_COMPO_TYPE_IPU,
+	SCMI_TLM_COMPO_TYPE_CHIPLET,
+	SCMI_TLM_COMPO_TYPE_PACKAGE,
+	SCMI_TLM_COMPO_TYPE_SOC,
+	SCMI_TLM_COMPO_TYPE_SYSTEM,
+	SCMI_TLM_COMPO_TYPE_SMCU,
+	SCMI_TLM_COMPO_TYPE_ACCEL,
+	SCMI_TLM_COMPO_TYPE_BATTERY,
+	SCMI_TLM_COMPO_TYPE_CHARGER,
+	SCMI_TLM_COMPO_TYPE_PMIC,
+	SCMI_TLM_COMPO_TYPE_BOARD,
+	SCMI_TLM_COMPO_TYPE_MEMORY,
+	SCMI_TLM_COMPO_TYPE_PERIPH,
+	SCMI_TLM_COMPO_TYPE_PERIPH_SUBC,
+	SCMI_TLM_COMPO_TYPE_LID,
+	SCMI_TLM_COMPO_TYPE_DISPLAY,
+	SCMI_TLM_COMPO_TYPE_RESERVED_START = 0x1d,
+	SCMI_TLM_COMPO_TYPE_RESERVED_END = 0xdf,
+	SCMI_TLM_COMPO_TYPE_OEM_START = 0xe0,
+	SCMI_TLM_COMPO_TYPE_OEM_END = 0xff,
+};
+
+#define	SCMI_TLM_GET_UPDATE_INTERVAL_SECS(x)	(FIELD_GET(GENMASK(20, 5), (x)))
+#define SCMI_TLM_GET_UPDATE_INTERVAL_EXP(x)	(sign_extend32((x), 4))
+
+#define SCMI_TLM_GET_UPDATE_INTERVAL(x)		(FIELD_GET(GENMASK(20, 0), (x)))
+#define SCMI_TLM_BUILD_UPDATE_INTERVAL(s, e)				    \
+	(FIELD_PREP(GENMASK(20, 5), (s)) | FIELD_PREP(GENMASK(4, 0), (e)))
+
+enum scmi_telemetry_collection {
+	SCMI_TLM_ONDEMAND,
+	SCMI_TLM_NOTIFICATION,
+	SCMI_TLM_SINGLE_READ,
+};
+
+#define SCMI_TLM_GRP_INVALID		0xFFFFFFFF
+
+struct scmi_telemetry_intervals {
+	unsigned int grp_id;
+	bool discrete;
+	unsigned int num_intervals;
+#define SCMI_TLM_UPDATE_INTVL_SEGMENT_LOW	0
+#define SCMI_TLM_UPDATE_INTVL_SEGMENT_HIGH	1
+#define SCMI_TLM_UPDATE_INTVL_SEGMENT_STEP	2
+	unsigned int update_intervals[] __counted_by(num_intervals);
+};
+
+struct scmi_telemetry_grp_info {
+	unsigned int grp_id;
+	unsigned int num_des;
+	unsigned int num_intervals;
+};
+
+struct scmi_telemetry_group {
+	bool enabled;
+	bool tstamp_enabled;
+	unsigned int *des;
+	char *des_str;
+	struct scmi_telemetry_grp_info *info;
+	unsigned int active_update_interval;
+	struct scmi_telemetry_intervals *intervals;
+	enum scmi_telemetry_collection current_mode;
+};
+
+struct scmi_telemetry_de_info {
+	unsigned int id;
+	unsigned int grp_id;
+	unsigned int data_sz;
+	unsigned int type;
+	unsigned int unit;
+	int unit_exp;
+	unsigned int ts_rate;
+	unsigned int instance_id;
+	unsigned int compo_instance_id;
+	unsigned int compo_type;
+	bool persistent;
+	char name[16];
+};
+
+struct scmi_telemetry_de {
+	bool tstamp_support;
+	bool fc_support;
+	bool name_support;
+	struct scmi_telemetry_de_info *info;
+	struct scmi_telemetry_group *grp;
+	bool enabled;
+	bool tstamp_enabled;
+};
+
+struct scmi_telemetry_res_info {
+	bool fully_enumerated;
+	unsigned int num_des;
+	struct scmi_telemetry_de **des;
+	struct scmi_telemetry_de_info *dei_store;
+	unsigned int num_groups;
+	struct scmi_telemetry_group *grps;
+	struct scmi_telemetry_grp_info *grps_store;
+};
+
+struct scmi_telemetry_base_info {
+	unsigned int version;
+	uuid_t primary_revision;
+	unsigned int num_des;
+	unsigned int num_groups;
+	unsigned int num_intervals;
+	unsigned int num_shmtis;
+};
+
+struct scmi_telemetry_shmti_info {
+	unsigned int sid;
+	unsigned int len;
+	unsigned long offset;
+	phys_addr_t phys;
+};
+
+struct scmi_telemetry_info {
+	bool single_read_support;
+	bool continuos_update_support;
+	bool per_group_config_support;
+	bool reset_support;
+	bool fc_support;
+	struct scmi_telemetry_base_info base;
+	unsigned int active_update_interval;
+	struct scmi_telemetry_intervals *intervals;
+	struct scmi_telemetry_shmti_info **shmtis;
+	unsigned int num_uuids;
+	uuid_t **uuids;
I'm going to guess a __counted_by_ptr marking makes sense here.
(I haven't checked!) I won't bother calling out other cases, but
looks like there may well be some above.
quoted hunk ↗ jump to hunk
+	bool enabled;
+	bool notif_enabled;
+	enum scmi_telemetry_collection current_mode;
+};
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help