From dc7ccfc86c1af6406c5a0aeeac122e64711ea5da Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Thu, 5 Mar 2026 09:56:44 +0800 Subject: [PATCH 01/31] firmware: arm_scmi: imx: Support getting reset reason of MISC protocol MISC protocol supports getting reset reason per Logical Machine or System. Add the API for user to retrieve the information from System Manager. Signed-off-by: Peng Fan Reviewed-by: Daniel Baluta Link: https://patch.msgid.link/20260305-scmi-imx-reset-v1-1-18de78978ba9@nxp.com Signed-off-by: Sudeep Holla --- .../arm_scmi/vendors/imx/imx-sm-misc.c | 86 +++++++++++++++++++ include/linux/scmi_imx_protocol.h | 14 +++ 2 files changed, 100 insertions(+) diff --git a/drivers/firmware/arm_scmi/vendors/imx/imx-sm-misc.c b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-misc.c index 0ada753367ef..637973fb45e6 100644 --- a/drivers/firmware/arm_scmi/vendors/imx/imx-sm-misc.c +++ b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-misc.c @@ -27,6 +27,7 @@ enum scmi_imx_misc_protocol_cmd { SCMI_IMX_MISC_CTRL_GET = 0x4, SCMI_IMX_MISC_DISCOVER_BUILD_INFO = 0x6, SCMI_IMX_MISC_CTRL_NOTIFY = 0x8, + SCMI_IMX_MISC_RESET_REASON_GET = 0xA, SCMI_IMX_MISC_CFG_INFO_GET = 0xC, SCMI_IMX_MISC_SYSLOG_GET = 0xD, SCMI_IMX_MISC_BOARD_INFO = 0xE, @@ -89,6 +90,37 @@ struct scmi_imx_misc_cfg_info_out { u8 cfgname[MISC_MAX_CFGNAME]; }; +struct scmi_imx_misc_reset_reason_in { +#define MISC_REASON_FLAG_SYSTEM BIT(0) + __le32 flags; +}; + +struct scmi_imx_misc_reset_reason_out { + /* Boot reason flags */ +#define MISC_BOOT_FLAG_VLD BIT(31) +#define MISC_BOOT_FLAG_ORG_VLD BIT(28) +#define MISC_BOOT_FLAG_ORIGIN GENMASK(27, 24) +#define MISC_BOOT_FLAG_O_SHIFT 24 +#define MISC_BOOT_FLAG_ERR_VLD BIT(23) +#define MISC_BOOT_FLAG_ERR_ID GENMASK(22, 8) +#define MISC_BOOT_FLAG_E_SHIFT 8 +#define MISC_BOOT_FLAG_REASON GENMASK(7, 0) + __le32 b_flags; + /* Shutdown reason flags */ +#define MISC_SHUTDOWN_FLAG_VLD BIT(31) +#define MISC_SHUTDOWN_FLAG_EXT_LEN GENMASK(30, 29) +#define MISC_SHUTDOWN_FLAG_ORG_VLD BIT(28) +#define MISC_SHUTDOWN_FLAG_ORIGIN GENMASK(27, 24) +#define MISC_SHUTDOWN_FLAG_O_SHIFT 24 +#define MISC_SHUTDOWN_FLAG_ERR_VLD BIT(23) +#define MISC_SHUTDOWN_FLAG_ERR_ID GENMASK(22, 8) +#define MISC_SHUTDOWN_FLAG_E_SHIFT 8 +#define MISC_SHUTDOWN_FLAG_REASON GENMASK(7, 0) + __le32 s_flags; + /* Array of extended info words */ + __le32 extinfo[MISC_EXT_INFO_LEN_MAX]; +}; + struct scmi_imx_misc_syslog_in { __le32 flags; __le32 index; @@ -452,11 +484,65 @@ static int scmi_imx_misc_syslog_get(const struct scmi_protocol_handle *ph, u16 * return ph->hops->iter_response_run(iter); } +static int scmi_imx_misc_reset_reason(const struct scmi_protocol_handle *ph, bool system, + struct scmi_imx_misc_reset_reason *boot_r, + struct scmi_imx_misc_reset_reason *shut_r, + u32 *extinfo) +{ + struct scmi_imx_misc_reset_reason_in *in; + struct scmi_imx_misc_reset_reason_out *out; + struct scmi_xfer *t; + int ret; + + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_RESET_REASON_GET, sizeof(*in), + sizeof(*out), &t); + if (ret) + return ret; + + in = t->tx.buf; + if (system) + in->flags = le32_encode_bits(1, MISC_REASON_FLAG_SYSTEM); + else + in->flags = cpu_to_le32(0); + + ret = ph->xops->do_xfer(ph, t); + if (!ret) { + out = t->rx.buf; + if (boot_r) { + boot_r->valid = le32_get_bits(out->b_flags, MISC_BOOT_FLAG_VLD); + boot_r->orig_valid = le32_get_bits(out->b_flags, MISC_BOOT_FLAG_ORG_VLD); + boot_r->err_valid = le32_get_bits(out->b_flags, MISC_BOOT_FLAG_ERR_VLD); + boot_r->reason = le32_get_bits(out->b_flags, MISC_BOOT_FLAG_REASON); + boot_r->origin = le32_get_bits(out->b_flags, MISC_BOOT_FLAG_ORIGIN); + boot_r->errid = le32_get_bits(out->b_flags, MISC_BOOT_FLAG_ERR_ID); + } + + if (shut_r) { + shut_r->valid = le32_get_bits(out->s_flags, MISC_SHUTDOWN_FLAG_VLD); + shut_r->orig_valid = le32_get_bits(out->s_flags, + MISC_SHUTDOWN_FLAG_ORG_VLD); + shut_r->err_valid = le32_get_bits(out->s_flags, + MISC_SHUTDOWN_FLAG_ERR_VLD); + shut_r->reason = le32_get_bits(out->s_flags, MISC_SHUTDOWN_FLAG_REASON); + shut_r->origin = le32_get_bits(out->s_flags, MISC_SHUTDOWN_FLAG_ORIGIN); + shut_r->errid = le32_get_bits(out->s_flags, MISC_SHUTDOWN_FLAG_ERR_ID); + } + + if (extinfo) + memcpy_from_le32(extinfo, out->extinfo, MISC_EXT_INFO_LEN_MAX); + } + + ph->xops->xfer_put(ph, t); + + return ret; +} + static const struct scmi_imx_misc_proto_ops scmi_imx_misc_proto_ops = { .misc_ctrl_set = scmi_imx_misc_ctrl_set, .misc_ctrl_get = scmi_imx_misc_ctrl_get, .misc_ctrl_req_notify = scmi_imx_misc_ctrl_notify, .misc_syslog = scmi_imx_misc_syslog_get, + .misc_reset_reason = scmi_imx_misc_reset_reason, }; static int scmi_imx_misc_protocol_init(const struct scmi_protocol_handle *ph) diff --git a/include/linux/scmi_imx_protocol.h b/include/linux/scmi_imx_protocol.h index 2407d7693b6b..ab867463c08c 100644 --- a/include/linux/scmi_imx_protocol.h +++ b/include/linux/scmi_imx_protocol.h @@ -52,6 +52,17 @@ struct scmi_imx_misc_ctrl_notify_report { unsigned int flags; }; + +#define MISC_EXT_INFO_LEN_MAX 4 +struct scmi_imx_misc_reset_reason { + bool valid:1; + bool orig_valid:1; + bool err_valid:1; + u32 reason; + u32 origin; + u32 errid; +}; + struct scmi_imx_misc_proto_ops { int (*misc_ctrl_set)(const struct scmi_protocol_handle *ph, u32 id, u32 num, u32 *val); @@ -61,6 +72,9 @@ struct scmi_imx_misc_proto_ops { u32 ctrl_id, u32 evt_id, u32 flags); int (*misc_syslog)(const struct scmi_protocol_handle *ph, u16 *size, void *array); + int (*misc_reset_reason)(const struct scmi_protocol_handle *ph, + bool system, struct scmi_imx_misc_reset_reason *boot_r, + struct scmi_imx_misc_reset_reason *shut_r, u32 *extinfo); }; /* See LMM_ATTRIBUTES in imx95.rst */ From c17216c1a92241d622d2208cfebf62556c818789 Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Thu, 5 Mar 2026 09:56:45 +0800 Subject: [PATCH 02/31] firmware: imx: sm-misc: Print boot/shutdown reasons Add reset reason string table for i.MX95 and introduce a helper (scmi_imx_misc_get_reason) to query and print both system and LM (Logical Machine) reset reasons via the SCMI MISC protocol. Signed-off-by: Peng Fan Link: https://patch.msgid.link/20260305-scmi-imx-reset-v1-2-18de78978ba9@nxp.com Acked-by: Frank Li Signed-off-by: Sudeep Holla --- drivers/firmware/imx/sm-misc.c | 73 ++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/drivers/firmware/imx/sm-misc.c b/drivers/firmware/imx/sm-misc.c index 0a8ada329c9d..16b5ff833d21 100644 --- a/drivers/firmware/imx/sm-misc.c +++ b/drivers/firmware/imx/sm-misc.c @@ -18,6 +18,29 @@ static const struct scmi_imx_misc_proto_ops *imx_misc_ctrl_ops; static struct scmi_protocol_handle *ph; struct notifier_block scmi_imx_misc_ctrl_nb; +static const char * const rst_imx95[] = { + "cm33_lockup", "cm33_swreq", "cm7_lockup", "cm7_swreq", "fccu", + "jtag_sw", "ele", "tempsense", "wdog1", "wdog2", "wdog3", "wdog4", + "wdog5", "jtag", "cm33_exc", "bbm", "sw", "sm_err", "fusa_sreco", + "pmic", "unused", "unused", "unused", "unused", "unused", "unused", + "unused", "unused", "unused", "unused", "unused", "por", +}; + +static const char * const rst_imx94[] = { + "cm33_lockup", "cm33_swreq", "cm70_lockup", "cm70_swreq", "fccu", + "jtag_sw", "ele", "tempsense", "wdog1", "wdog2", "wdog3", "wdog4", + "wdog5", "jtag", "wdog6", "wdog7", "wdog8", "wo_netc", "cm33s_lockup", + "cm33s_swreq", "cm71_lockup", "cm71_swreq", "cm33_exc", "bbm", "sw", + "sm_err", "fusa_sreco", "pmic", "unused", "unused", "unused", "por", +}; + +static const struct of_device_id allowlist[] = { + { .compatible = "fsl,imx952", .data = rst_imx95 }, + { .compatible = "fsl,imx95", .data = rst_imx95 }, + { .compatible = "fsl,imx94", .data = rst_imx94 }, + { /* Sentinel */ } +}; + int scmi_imx_misc_ctrl_set(u32 id, u32 val) { if (!ph) @@ -75,6 +98,54 @@ static void scmi_imx_misc_put(void *p) debugfs_remove((struct dentry *)p); } +static int scmi_imx_misc_get_reason(struct scmi_device *sdev) +{ + struct scmi_imx_misc_reset_reason boot, shutdown; + const char **rst; + bool system = true; + int ret; + + if (!of_machine_device_match(allowlist)) + return 0; + + rst = (const char **)of_machine_get_match_data(allowlist); + + ret = imx_misc_ctrl_ops->misc_reset_reason(ph, system, &boot, &shutdown, NULL); + if (!ret) { + if (boot.valid) + dev_info(&sdev->dev, "%s Boot reason: %s, origin: %d, errid: %d\n", + system ? "SYS" : "LM", rst[boot.reason], + boot.orig_valid ? boot.origin : -1, + boot.err_valid ? boot.errid : -1); + if (shutdown.valid) + dev_info(&sdev->dev, "%s shutdown reason: %s, origin: %d, errid: %d\n", + system ? "SYS" : "LM", rst[shutdown.reason], + shutdown.orig_valid ? shutdown.origin : -1, + shutdown.err_valid ? shutdown.errid : -1); + } else { + dev_err(&sdev->dev, "Failed to get system reset reason: %d\n", ret); + } + + system = false; + ret = imx_misc_ctrl_ops->misc_reset_reason(ph, system, &boot, &shutdown, NULL); + if (!ret) { + if (boot.valid) + dev_info(&sdev->dev, "%s Boot reason: %s, origin: %d, errid: %d\n", + system ? "SYS" : "LM", rst[boot.reason], + boot.orig_valid ? boot.origin : -1, + boot.err_valid ? boot.errid : -1); + if (shutdown.valid) + dev_info(&sdev->dev, "%s shutdown reason: %s, origin: %d, errid: %d\n", + system ? "SYS" : "LM", rst[shutdown.reason], + shutdown.orig_valid ? shutdown.origin : -1, + shutdown.err_valid ? shutdown.errid : -1); + } else { + dev_err(&sdev->dev, "Failed to get lm reset reason: %d\n", ret); + } + + return 0; +} + static int scmi_imx_misc_ctrl_probe(struct scmi_device *sdev) { const struct scmi_handle *handle = sdev->handle; @@ -133,6 +204,8 @@ static int scmi_imx_misc_ctrl_probe(struct scmi_device *sdev) scmi_imx_dentry = debugfs_create_dir("scmi_imx", NULL); debugfs_create_file("syslog", 0444, scmi_imx_dentry, &sdev->dev, &syslog_fops); + scmi_imx_misc_get_reason(sdev); + return devm_add_action_or_reset(&sdev->dev, scmi_imx_misc_put, scmi_imx_dentry); } From 0c6eb5d019c1e4b9cfcfc47000e08858bbbc5e52 Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Mon, 6 Apr 2026 17:52:54 +0200 Subject: [PATCH 03/31] firmware: arm_scmi: Rename struct scmi_revision_info to scmi_base_info Rename struct scmi_revision_info to struct scmi_base_info , to accurately represent its content. The scmi_revision_info is no longer accurate, because the structure now contains more than only SCMI base protocol revision, it now also contains number of protocols, agents, vendor and subvendor strings. All those are fetched from the base protocol, so rename the structure to scmi_base_info, to match the other scmi_*_info structure names. No functional change. Signed-off-by: Marek Vasut Link: https://patch.msgid.link/20260406155343.72087-1-marek.vasut+renesas@mailbox.org Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/base.c | 10 +++++----- drivers/firmware/arm_scmi/common.h | 2 +- drivers/firmware/arm_scmi/driver.c | 14 +++++++------- include/linux/scmi_protocol.h | 6 +++--- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c index cd1331c2fc40..4df2620e3c5d 100644 --- a/drivers/firmware/arm_scmi/base.c +++ b/drivers/firmware/arm_scmi/base.c @@ -69,7 +69,7 @@ static int scmi_base_attributes_get(const struct scmi_protocol_handle *ph) int ret; struct scmi_xfer *t; struct scmi_msg_resp_base_attributes *attr_info; - struct scmi_revision_info *rev = ph->get_priv(ph); + struct scmi_base_info *rev = ph->get_priv(ph); ret = ph->xops->xfer_get_init(ph, PROTOCOL_ATTRIBUTES, 0, sizeof(*attr_info), &t); @@ -103,7 +103,7 @@ scmi_base_vendor_id_get(const struct scmi_protocol_handle *ph, bool sub_vendor) int ret, size; char *vendor_id; struct scmi_xfer *t; - struct scmi_revision_info *rev = ph->get_priv(ph); + struct scmi_base_info *rev = ph->get_priv(ph); if (sub_vendor) { cmd = BASE_DISCOVER_SUB_VENDOR; @@ -143,7 +143,7 @@ scmi_base_implementation_version_get(const struct scmi_protocol_handle *ph) int ret; __le32 *impl_ver; struct scmi_xfer *t; - struct scmi_revision_info *rev = ph->get_priv(ph); + struct scmi_base_info *rev = ph->get_priv(ph); ret = ph->xops->xfer_get_init(ph, BASE_DISCOVER_IMPLEMENT_VERSION, 0, sizeof(*impl_ver), &t); @@ -180,7 +180,7 @@ scmi_base_implementation_list_get(const struct scmi_protocol_handle *ph, __le32 *num_skip, *num_ret; u32 tot_num_ret = 0, loop_num_ret; struct device *dev = ph->dev; - struct scmi_revision_info *rev = ph->get_priv(ph); + struct scmi_base_info *rev = ph->get_priv(ph); ret = ph->xops->xfer_get_init(ph, BASE_DISCOVER_LIST_PROTOCOLS, sizeof(*num_skip), 0, &t); @@ -377,7 +377,7 @@ static int scmi_base_protocol_init(const struct scmi_protocol_handle *ph) u8 *prot_imp; char name[SCMI_SHORT_NAME_MAX_SIZE]; struct device *dev = ph->dev; - struct scmi_revision_info *rev = scmi_revision_area_get(ph); + struct scmi_base_info *rev = scmi_revision_area_get(ph); rev->major_ver = PROTOCOL_REV_MAJOR(ph->version); rev->minor_ver = PROTOCOL_REV_MINOR(ph->version); diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index 7c9617d080a0..07a127dec031 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -138,7 +138,7 @@ static inline void unpack_scmi_header(u32 msg_hdr, struct scmi_msg_hdr *hdr) xfer_; \ }) -struct scmi_revision_info * +struct scmi_base_info * scmi_revision_area_get(const struct scmi_protocol_handle *ph); void scmi_setup_protocol_implemented(const struct scmi_protocol_handle *ph, u8 *prot_imp); diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index f167194f7cf6..f7ee1b1495d7 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -133,7 +133,7 @@ struct scmi_protocol_instance { * usage. * @protocols_mtx: A mutex to protect protocols instances initialization. * @protocols_imp: List of protocols implemented, currently maximum of - * scmi_revision_info.num_protocols elements allocated by the + * scmi_base_info.num_protocols elements allocated by the * base protocol * @active_protocols: IDR storing device_nodes for protocols actually defined * in the DT and confirmed as implemented by fw. @@ -151,7 +151,7 @@ struct scmi_info { int id; struct device *dev; const struct scmi_desc *desc; - struct scmi_revision_info version; + struct scmi_base_info version; struct scmi_handle handle; struct scmi_xfers_info tx_minfo; struct scmi_xfers_info rx_minfo; @@ -265,7 +265,7 @@ scmi_vendor_protocol_lookup(int protocol_id, char *vendor_id, } static const struct scmi_protocol * -scmi_vendor_protocol_get(int protocol_id, struct scmi_revision_info *version) +scmi_vendor_protocol_get(int protocol_id, struct scmi_base_info *version) { const struct scmi_protocol *proto; @@ -303,7 +303,7 @@ scmi_vendor_protocol_get(int protocol_id, struct scmi_revision_info *version) } static const struct scmi_protocol * -scmi_protocol_get(int protocol_id, struct scmi_revision_info *version) +scmi_protocol_get(int protocol_id, struct scmi_base_info *version) { const struct scmi_protocol *proto = NULL; @@ -2063,7 +2063,7 @@ static const struct scmi_proto_helpers_ops helpers_ops = { * Return: A reference to the version memory area associated to the SCMI * instance underlying this protocol handle. */ -struct scmi_revision_info * +struct scmi_base_info * scmi_revision_area_get(const struct scmi_protocol_handle *ph) { const struct scmi_protocol_instance *pi = ph_to_pi(ph); @@ -2376,7 +2376,7 @@ scmi_is_protocol_implemented(const struct scmi_handle *handle, u8 prot_id) { int i; struct scmi_info *info = handle_to_scmi_info(handle); - struct scmi_revision_info *rev = handle->version; + struct scmi_base_info *rev = handle->version; if (!info->protocols_imp) return false; @@ -3171,7 +3171,7 @@ static const struct scmi_desc *scmi_transport_setup(struct device *dev) static void scmi_enable_matching_quirks(struct scmi_info *info) { - struct scmi_revision_info *rev = &info->version; + struct scmi_base_info *rev = &info->version; dev_dbg(info->dev, "Looking for quirks matching: %s/%s/0x%08X\n", rev->vendor_id, rev->sub_vendor_id, rev->impl_ver); diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h index aafaac1496b0..8d0b106caed8 100644 --- a/include/linux/scmi_protocol.h +++ b/include/linux/scmi_protocol.h @@ -18,7 +18,7 @@ #define SCMI_MAX_NUM_RATES 16 /** - * struct scmi_revision_info - version information structure + * struct scmi_base_info - version information structure * * @major_ver: Major ABI version. Change here implies risk of backward * compatibility break. @@ -31,7 +31,7 @@ * @vendor_id: A vendor identifier(Null terminated ASCII string) * @sub_vendor_id: A sub-vendor identifier(Null terminated ASCII string) */ -struct scmi_revision_info { +struct scmi_base_info { u16 major_ver; u16 minor_ver; u8 num_protocols; @@ -901,7 +901,7 @@ struct scmi_notify_ops { */ struct scmi_handle { struct device *dev; - struct scmi_revision_info *version; + struct scmi_base_info *version; int __must_check (*devm_protocol_acquire)(struct scmi_device *sdev, u8 proto); From 6991e5de9709b0df1f37029e5d0ada6df69aea99 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Fri, 3 Apr 2026 10:41:29 +0200 Subject: [PATCH 04/31] firmware: arm_scmi: quirk: Improve quirk range parsing When a range contains only an end ("-X"), the number string is parsed twice, as both "sep == first" and "sep != last" are true. Fix this by dropping the superfluous number parsing for "sep == first". This does have a harmless functional impact for the unbounded range: "-" is now accepted, while it was rejected before. Signed-off-by: Geert Uytterhoeven Link: https://patch.msgid.link/fe257b3b7b7b5c17fd0e5727bb9746c731bd7e3c.1775205358.git.geert+renesas@glider.be (sudeep.holla: Initialise ret to 0 as it will be uninitialise for "-" range) Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/quirks.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/firmware/arm_scmi/quirks.c b/drivers/firmware/arm_scmi/quirks.c index 03848283c2a0..b31f23e2bbbc 100644 --- a/drivers/firmware/arm_scmi/quirks.c +++ b/drivers/firmware/arm_scmi/quirks.c @@ -219,9 +219,9 @@ static unsigned int scmi_quirk_signature(const char *vend, const char *sub_vend) static int scmi_quirk_range_parse(struct scmi_quirk *quirk) { const char *last, *first __free(kfree) = NULL; + int ret = 0; size_t len; char *sep; - int ret; quirk->start_range = 0; quirk->end_range = 0xFFFFFFFF; @@ -238,16 +238,15 @@ static int scmi_quirk_range_parse(struct scmi_quirk *quirk) if (sep) *sep = '\0'; - if (sep == first) /* -X */ - ret = kstrtouint(first + 1, 0, &quirk->end_range); - else /* X OR X- OR X-y */ + if (sep != first) /* X OR X- OR X-y */ { ret = kstrtouint(first, 0, &quirk->start_range); - if (ret) - return ret; + if (ret) + return ret; + } if (!sep) quirk->end_range = quirk->start_range; - else if (sep != last) /* x-Y */ + else if (sep != last) /* -X OR x-Y */ ret = kstrtouint(sep + 1, 0, &quirk->end_range); if (quirk->start_range > quirk->end_range) From 5be6732f820c97ab2ab20a44770a9f85fa5c7099 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Fri, 3 Apr 2026 10:41:30 +0200 Subject: [PATCH 05/31] firmware: arm_scmi: quirk: Simplify quirk table iteration The current table entry is assigned in both the init and loop expressions of the for-statement. Merge this into a single assignment in the conditional expression, to simplify the code. While at it, make the loop counter unsigned and loop-local. Signed-off-by: Geert Uytterhoeven Link: https://patch.msgid.link/8577f4b103cf04420c3b67dcaad528daff867287.1775205358.git.geert+renesas@glider.be Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/quirks.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/firmware/arm_scmi/quirks.c b/drivers/firmware/arm_scmi/quirks.c index b31f23e2bbbc..9ed5ce4feded 100644 --- a/drivers/firmware/arm_scmi/quirks.c +++ b/drivers/firmware/arm_scmi/quirks.c @@ -258,10 +258,8 @@ static int scmi_quirk_range_parse(struct scmi_quirk *quirk) void scmi_quirks_initialize(void) { struct scmi_quirk *quirk; - int i; - for (i = 0, quirk = scmi_quirks_table[0]; quirk; - i++, quirk = scmi_quirks_table[i]) { + for (unsigned int i = 0; (quirk = scmi_quirks_table[i]); i++) { int ret; ret = scmi_quirk_range_parse(quirk); From 145728fb616a62e5a2642f2efd3829f7c8874958 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Fri, 3 Apr 2026 10:41:31 +0200 Subject: [PATCH 06/31] firmware: arm_scmi: Convert to list_for_each_entry() Simplify the loop in scmi_handle_get() by using list_for_each_entry(). Suggested-by: Marek Vasut Signed-off-by: Geert Uytterhoeven Link: https://patch.msgid.link/bccbd4a64ef4619afd5454e9e533073b00aeaba6.1775205358.git.geert+renesas@glider.be Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/driver.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index f7ee1b1495d7..53310beb67a8 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -2552,13 +2552,11 @@ static bool scmi_is_transport_atomic(const struct scmi_handle *handle, */ static struct scmi_handle *scmi_handle_get(struct device *dev) { - struct list_head *p; struct scmi_info *info; struct scmi_handle *handle = NULL; mutex_lock(&scmi_list_mutex); - list_for_each(p, &scmi_list) { - info = list_entry(p, struct scmi_info, node); + list_for_each_entry(info, &scmi_list, node) { if (dev->parent == info->dev) { info->users++; handle = &info->handle; From d0c81a38d06d446d341532700b3a4a43d3b00eb1 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Fri, 8 May 2026 16:32:46 +0100 Subject: [PATCH 07/31] clk: scmi: Fix clock rate rounding While the do_div() helper used for rounding expects its divisor argument to be a 32bits quantity, the currently provided divisor parameter is a 64bit value that, as a consequence, is silently truncated and a possible source of bugs. Fix by using the proper div64_ul helper. Cc: Michael Turquette Cc: Stephen Boyd Cc: linux-clk@vger.kernel.org Fixes: 7a8655e19bdb ("clk: scmi: Fix the rounding of clock rate") Signed-off-by: Cristian Marussi Tested-by: Florian Fainelli Tested-by: Geert Uytterhoeven Link: https://patch.msgid.link/20260508153300.2224715-2-cristian.marussi@arm.com Reviewed-by: Brian Masney Signed-off-by: Sudeep Holla --- drivers/clk/clk-scmi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c index 6b286ea6f121..b6a12f3bc123 100644 --- a/drivers/clk/clk-scmi.c +++ b/drivers/clk/clk-scmi.c @@ -10,9 +10,9 @@ #include #include #include +#include #include #include -#include #define NOT_ATOMIC false #define ATOMIC true @@ -83,7 +83,7 @@ static int scmi_clk_determine_rate(struct clk_hw *hw, ftmp = req->rate - fmin; ftmp += clk->info->range.step_size - 1; /* to round up */ - do_div(ftmp, clk->info->range.step_size); + ftmp = div64_ul(ftmp, clk->info->range.step_size); req->rate = ftmp * clk->info->range.step_size + fmin; From ecde921eb46022acbdbfff2ad4e4c6e6d0493430 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Fri, 8 May 2026 16:32:47 +0100 Subject: [PATCH 08/31] firmware: arm_scmi: Add clock determine_rate operation Add a clock operation to help determining the effective rate, closest to the required one, that a specific clock can support. Calculation is currently performed kernel side and the logic is taken directly from the SCMI Clock driver: embedding the determinate rate logic in the protocol layer enables simplifications in the SCMI Clock protocol interface and will more easily accommodate further evolutions where such determine_rate logic into is optionally delegated to the platform SCMI server. Signed-off-by: Cristian Marussi Tested-by: Florian Fainelli Tested-by: Geert Uytterhoeven Link: https://patch.msgid.link/20260508153300.2224715-3-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/clock.c | 42 +++++++++++++++++++++++++++++++ include/linux/scmi_protocol.h | 6 +++++ 2 files changed, 48 insertions(+) diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c index ab36871650a1..54b55517b759 100644 --- a/drivers/firmware/arm_scmi/clock.c +++ b/drivers/firmware/arm_scmi/clock.c @@ -5,6 +5,7 @@ * Copyright (C) 2018-2022 ARM Ltd. */ +#include #include #include #include @@ -624,6 +625,46 @@ static int scmi_clock_rate_set(const struct scmi_protocol_handle *ph, return ret; } +static int scmi_clock_determine_rate(const struct scmi_protocol_handle *ph, + u32 clk_id, unsigned long *rate) +{ + u64 fmin, fmax, ftmp; + struct scmi_clock_info *clk; + struct clock_info *ci = ph->get_priv(ph); + + if (!rate) + return -EINVAL; + + clk = scmi_clock_domain_lookup(ci, clk_id); + if (IS_ERR(clk)) + return PTR_ERR(clk); + + /* + * If we can't figure out what rate it will be, so just return the + * rate back to the caller. + */ + if (clk->rate_discrete) + return 0; + + fmin = clk->range.min_rate; + fmax = clk->range.max_rate; + if (*rate <= fmin) { + *rate = fmin; + return 0; + } else if (*rate >= fmax) { + *rate = fmax; + return 0; + } + + ftmp = *rate - fmin; + ftmp += clk->range.step_size - 1; /* to round up */ + ftmp = div64_ul(ftmp, clk->range.step_size); + + *rate = ftmp * clk->range.step_size + fmin; + + return 0; +} + static int scmi_clock_config_set(const struct scmi_protocol_handle *ph, u32 clk_id, enum clk_state state, @@ -936,6 +977,7 @@ static const struct scmi_clk_proto_ops clk_proto_ops = { .info_get = scmi_clock_info_get, .rate_get = scmi_clock_rate_get, .rate_set = scmi_clock_rate_set, + .determine_rate = scmi_clock_determine_rate, .enable = scmi_clock_enable, .disable = scmi_clock_disable, .state_get = scmi_clock_state_get, diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h index 8d0b106caed8..984117f51695 100644 --- a/include/linux/scmi_protocol.h +++ b/include/linux/scmi_protocol.h @@ -91,6 +91,10 @@ enum scmi_clock_oem_config { * @info_get: get the information of the specified clock * @rate_get: request the current clock rate of a clock * @rate_set: set the clock rate of a clock + * @determine_rate: determine the effective rate that can be supported by a + * clock calculating the closest allowed rate. + * Note that @rate is an input/output parameter used both to + * describe the requested rate and report the closest match * @enable: enables the specified clock * @disable: disables the specified clock * @state_get: get the status of the specified clock @@ -108,6 +112,8 @@ struct scmi_clk_proto_ops { u64 *rate); int (*rate_set)(const struct scmi_protocol_handle *ph, u32 clk_id, u64 rate); + int (*determine_rate)(const struct scmi_protocol_handle *ph, u32 clk_id, + unsigned long *rate); int (*enable)(const struct scmi_protocol_handle *ph, u32 clk_id, bool atomic); int (*disable)(const struct scmi_protocol_handle *ph, u32 clk_id, From af86c99170b771a3c763be38b01dc519501e907b Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Fri, 8 May 2026 16:32:48 +0100 Subject: [PATCH 09/31] clk: scmi: Use new determine_rate clock operation Use the Clock protocol layer determine_rate logic to calculate the closest rate that can be supported by a specific clock. No functional change. Cc: Brian Masney Cc: Michael Turquette Cc: Stephen Boyd Cc: linux-clk@vger.kernel.org Signed-off-by: Cristian Marussi Tested-by: Florian Fainelli Tested-by: Geert Uytterhoeven Link: https://patch.msgid.link/20260508153300.2224715-4-cristian.marussi@arm.com Reviewed-by: Brian Masney Signed-off-by: Sudeep Holla --- drivers/clk/clk-scmi.c | 31 ++++++------------------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c index b6a12f3bc123..c223e4ef1dd1 100644 --- a/drivers/clk/clk-scmi.c +++ b/drivers/clk/clk-scmi.c @@ -10,7 +10,6 @@ #include #include #include -#include #include #include @@ -57,35 +56,17 @@ static unsigned long scmi_clk_recalc_rate(struct clk_hw *hw, static int scmi_clk_determine_rate(struct clk_hw *hw, struct clk_rate_request *req) { - u64 fmin, fmax, ftmp; + int ret; struct scmi_clk *clk = to_scmi_clk(hw); /* - * We can't figure out what rate it will be, so just return the - * rate back to the caller. scmi_clk_recalc_rate() will be called - * after the rate is set and we'll know what rate the clock is + * If we could not get a better rate scmi_clk_recalc_rate() will be + * called after the rate is set and we'll know what rate the clock is * running at then. */ - if (clk->info->rate_discrete) - return 0; - - fmin = clk->info->range.min_rate; - fmax = clk->info->range.max_rate; - if (req->rate <= fmin) { - req->rate = fmin; - - return 0; - } else if (req->rate >= fmax) { - req->rate = fmax; - - return 0; - } - - ftmp = req->rate - fmin; - ftmp += clk->info->range.step_size - 1; /* to round up */ - ftmp = div64_ul(ftmp, clk->info->range.step_size); - - req->rate = ftmp * clk->info->range.step_size + fmin; + ret = scmi_proto_clk_ops->determine_rate(clk->ph, clk->id, &req->rate); + if (ret) + return ret; return 0; } From 0d76f62613cafecb7d326a5a45619024fa7e6e8e Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Fri, 8 May 2026 16:32:49 +0100 Subject: [PATCH 10/31] firmware: arm_scmi: Simplify clock rates exposed interface Introduce a new internal struct scmi_clock_desc so as to be able to hide, in the future, some of the needlessly public fields currently kept inside scmi_clock_info, while keeping exposed only the two new min_rate and max_rate fields for each clock. No functional change. Reviewed-by: Peng Fan Signed-off-by: Cristian Marussi Tested-by: Florian Fainelli Tested-by: Geert Uytterhoeven Link: https://patch.msgid.link/20260508153300.2224715-5-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/clock.c | 158 ++++++++++++++++-------------- include/linux/scmi_protocol.h | 2 + 2 files changed, 85 insertions(+), 75 deletions(-) diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c index 54b55517b759..512400122b85 100644 --- a/drivers/firmware/arm_scmi/clock.c +++ b/drivers/firmware/arm_scmi/clock.c @@ -157,13 +157,27 @@ struct scmi_clock_rate_notify_payld { __le32 rate_high; }; +struct scmi_clock_desc { + u32 id; + bool rate_discrete; + unsigned int num_rates; + u64 rates[SCMI_MAX_NUM_RATES]; +#define RATE_MIN 0 +#define RATE_MAX 1 +#define RATE_STEP 2 + struct scmi_clock_info info; +}; + +#define to_desc(p) (container_of(p, struct scmi_clock_desc, info)) + struct clock_info { int num_clocks; int max_async_req; bool notify_rate_changed_cmd; bool notify_rate_change_requested_cmd; atomic_t cur_async_req; - struct scmi_clock_info *clk; + struct scmi_clock_desc *clkds; +#define CLOCK_INFO(c, i) (&(((c)->clkds + (i))->info)) int (*clock_config_set)(const struct scmi_protocol_handle *ph, u32 clk_id, enum clk_state state, enum scmi_clock_oem_config oem_type, @@ -185,7 +199,7 @@ scmi_clock_domain_lookup(struct clock_info *ci, u32 clk_id) if (clk_id >= ci->num_clocks) return ERR_PTR(-EINVAL); - return ci->clk + clk_id; + return CLOCK_INFO(ci, clk_id); } static int @@ -226,8 +240,7 @@ scmi_clock_protocol_attributes_get(const struct scmi_protocol_handle *ph, struct scmi_clk_ipriv { struct device *dev; - u32 clk_id; - struct scmi_clock_info *clk; + struct scmi_clock_desc *clkd; }; static void iter_clk_possible_parents_prepare_message(void *message, unsigned int desc_index, @@ -236,7 +249,7 @@ static void iter_clk_possible_parents_prepare_message(void *message, unsigned in struct scmi_msg_clock_possible_parents *msg = message; const struct scmi_clk_ipriv *p = priv; - msg->id = cpu_to_le32(p->clk_id); + msg->id = cpu_to_le32(p->clkd->id); /* Set the number of OPPs to be skipped/already read */ msg->skip_parents = cpu_to_le32(desc_index); } @@ -246,7 +259,6 @@ static int iter_clk_possible_parents_update_state(struct scmi_iterator_state *st { const struct scmi_msg_resp_clock_possible_parents *r = response; struct scmi_clk_ipriv *p = priv; - struct device *dev = ((struct scmi_clk_ipriv *)p)->dev; u32 flags; flags = le32_to_cpu(r->num_parent_flags); @@ -258,12 +270,13 @@ static int iter_clk_possible_parents_update_state(struct scmi_iterator_state *st * assume it's returned+remaining on first call. */ if (!st->max_resources) { - p->clk->num_parents = st->num_returned + st->num_remaining; - p->clk->parents = devm_kcalloc(dev, p->clk->num_parents, - sizeof(*p->clk->parents), - GFP_KERNEL); - if (!p->clk->parents) { - p->clk->num_parents = 0; + p->clkd->info.num_parents = st->num_returned + st->num_remaining; + p->clkd->info.parents = devm_kcalloc(p->dev, + p->clkd->info.num_parents, + sizeof(*p->clkd->info.parents), + GFP_KERNEL); + if (!p->clkd->info.parents) { + p->clkd->info.num_parents = 0; return -ENOMEM; } st->max_resources = st->num_returned + st->num_remaining; @@ -280,29 +293,27 @@ static int iter_clk_possible_parents_process_response(const struct scmi_protocol const struct scmi_msg_resp_clock_possible_parents *r = response; struct scmi_clk_ipriv *p = priv; - u32 *parent = &p->clk->parents[st->desc_index + st->loop_idx]; + u32 *parent = &p->clkd->info.parents[st->desc_index + st->loop_idx]; *parent = le32_to_cpu(r->possible_parents[st->loop_idx]); return 0; } -static int scmi_clock_possible_parents(const struct scmi_protocol_handle *ph, u32 clk_id, - struct scmi_clock_info *clk) +static int scmi_clock_possible_parents(const struct scmi_protocol_handle *ph, + u32 clk_id, struct clock_info *cinfo) { struct scmi_iterator_ops ops = { .prepare_message = iter_clk_possible_parents_prepare_message, .update_state = iter_clk_possible_parents_update_state, .process_response = iter_clk_possible_parents_process_response, }; - + struct scmi_clock_desc *clkd = &cinfo->clkds[clk_id]; struct scmi_clk_ipriv ppriv = { - .clk_id = clk_id, - .clk = clk, + .clkd = clkd, .dev = ph->dev, }; void *iter; - int ret; iter = ph->hops->iter_response_init(ph, &ops, 0, CLOCK_POSSIBLE_PARENTS_GET, @@ -311,9 +322,7 @@ static int scmi_clock_possible_parents(const struct scmi_protocol_handle *ph, u3 if (IS_ERR(iter)) return PTR_ERR(iter); - ret = ph->hops->iter_response_run(iter); - - return ret; + return ph->hops->iter_response_run(iter); } static int @@ -352,7 +361,7 @@ static int scmi_clock_attributes_get(const struct scmi_protocol_handle *ph, u32 attributes; struct scmi_xfer *t; struct scmi_msg_resp_clock_attributes *attr; - struct scmi_clock_info *clk = cinfo->clk + clk_id; + struct scmi_clock_info *clk = CLOCK_INFO(cinfo, clk_id); ret = ph->xops->xfer_get_init(ph, CLOCK_ATTRIBUTES, sizeof(clk_id), sizeof(*attr), &t); @@ -394,7 +403,7 @@ static int scmi_clock_attributes_get(const struct scmi_protocol_handle *ph, clk->rate_change_requested_notifications = true; if (PROTOCOL_REV_MAJOR(ph->version) >= 0x3) { if (SUPPORTS_PARENT_CLOCK(attributes)) - scmi_clock_possible_parents(ph, clk_id, clk); + scmi_clock_possible_parents(ph, clk_id, cinfo); if (SUPPORTS_GET_PERMISSIONS(attributes)) scmi_clock_get_permissions(ph, clk_id, clk); if (SUPPORTS_EXTENDED_CONFIG(attributes)) @@ -424,7 +433,7 @@ static void iter_clk_describe_prepare_message(void *message, struct scmi_msg_clock_describe_rates *msg = message; const struct scmi_clk_ipriv *p = priv; - msg->id = cpu_to_le32(p->clk_id); + msg->id = cpu_to_le32(p->clkd->id); /* Set the number of rates to be skipped/already read */ msg->rate_index = cpu_to_le32(desc_index); } @@ -457,14 +466,15 @@ iter_clk_describe_update_state(struct scmi_iterator_state *st, flags = le32_to_cpu(r->num_rates_flags); st->num_remaining = NUM_REMAINING(flags); st->num_returned = NUM_RETURNED(flags); - p->clk->rate_discrete = RATE_DISCRETE(flags); + p->clkd->rate_discrete = RATE_DISCRETE(flags); + p->clkd->info.rate_discrete = p->clkd->rate_discrete; /* Warn about out of spec replies ... */ - if (!p->clk->rate_discrete && + if (!p->clkd->rate_discrete && (st->num_returned != 3 || st->num_remaining != 0)) { dev_warn(p->dev, "Out-of-spec CLOCK_DESCRIBE_RATES reply for %s - returned:%d remaining:%d rx_len:%zd\n", - p->clk->name, st->num_returned, st->num_remaining, + p->clkd->info.name, st->num_returned, st->num_remaining, st->rx_len); SCMI_QUIRK(clock_rates_triplet_out_of_spec, @@ -479,38 +489,19 @@ iter_clk_describe_process_response(const struct scmi_protocol_handle *ph, const void *response, struct scmi_iterator_state *st, void *priv) { - int ret = 0; struct scmi_clk_ipriv *p = priv; const struct scmi_msg_resp_clock_describe_rates *r = response; - if (!p->clk->rate_discrete) { - switch (st->desc_index + st->loop_idx) { - case 0: - p->clk->range.min_rate = RATE_TO_U64(r->rate[0]); - break; - case 1: - p->clk->range.max_rate = RATE_TO_U64(r->rate[1]); - break; - case 2: - p->clk->range.step_size = RATE_TO_U64(r->rate[2]); - break; - default: - ret = -EINVAL; - break; - } - } else { - u64 *rate = &p->clk->list.rates[st->desc_index + st->loop_idx]; + p->clkd->rates[st->desc_index + st->loop_idx] = + RATE_TO_U64(r->rate[st->loop_idx]); + p->clkd->num_rates++; - *rate = RATE_TO_U64(r->rate[st->loop_idx]); - p->clk->list.num_rates++; - } - - return ret; + return 0; } static int scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id, - struct scmi_clock_info *clk) + struct clock_info *cinfo) { int ret; void *iter; @@ -519,9 +510,9 @@ scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id, .update_state = iter_clk_describe_update_state, .process_response = iter_clk_describe_process_response, }; + struct scmi_clock_desc *clkd = &cinfo->clkds[clk_id]; struct scmi_clk_ipriv cpriv = { - .clk_id = clk_id, - .clk = clk, + .clkd = clkd, .dev = ph->dev, }; @@ -536,16 +527,31 @@ scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id, if (ret) return ret; - if (!clk->rate_discrete) { - dev_dbg(ph->dev, "Min %llu Max %llu Step %llu Hz\n", - clk->range.min_rate, clk->range.max_rate, - clk->range.step_size); - } else if (clk->list.num_rates) { - sort(clk->list.rates, clk->list.num_rates, - sizeof(clk->list.rates[0]), rate_cmp_func, NULL); - } + /* empty set ? */ + if (!clkd->num_rates) + return 0; - return ret; + if (!clkd->rate_discrete) { + clkd->info.range.min_rate = clkd->rates[RATE_MIN]; + clkd->info.range.max_rate = clkd->rates[RATE_MAX]; + clkd->info.range.step_size = clkd->rates[RATE_STEP]; + clkd->info.max_rate = clkd->rates[RATE_MAX]; + dev_dbg(ph->dev, "Min %llu Max %llu Step %llu Hz\n", + clkd->rates[RATE_MIN], clkd->rates[RATE_MAX], + clkd->rates[RATE_STEP]); + } else { + unsigned int i; + + sort(clkd->rates, clkd->num_rates, + sizeof(clkd->rates[0]), rate_cmp_func, NULL); + clkd->info.list.num_rates = clkd->num_rates; + for (i = 0; i < clkd->num_rates; i++) + clkd->info.list.rates[i] = clkd->rates[i]; + clkd->info.max_rate = clkd->rates[clkd->num_rates - 1]; + } + clkd->info.min_rate = clkd->rates[RATE_MIN]; + + return 0; } static int @@ -630,6 +636,7 @@ static int scmi_clock_determine_rate(const struct scmi_protocol_handle *ph, { u64 fmin, fmax, ftmp; struct scmi_clock_info *clk; + struct scmi_clock_desc *clkd; struct clock_info *ci = ph->get_priv(ph); if (!rate) @@ -639,15 +646,17 @@ static int scmi_clock_determine_rate(const struct scmi_protocol_handle *ph, if (IS_ERR(clk)) return PTR_ERR(clk); + clkd = to_desc(clk); + /* * If we can't figure out what rate it will be, so just return the * rate back to the caller. */ - if (clk->rate_discrete) + if (clkd->rate_discrete) return 0; - fmin = clk->range.min_rate; - fmax = clk->range.max_rate; + fmin = clk->min_rate; + fmax = clk->max_rate; if (*rate <= fmin) { *rate = fmin; return 0; @@ -657,10 +666,10 @@ static int scmi_clock_determine_rate(const struct scmi_protocol_handle *ph, } ftmp = *rate - fmin; - ftmp += clk->range.step_size - 1; /* to round up */ - ftmp = div64_ul(ftmp, clk->range.step_size); + ftmp += clkd->rates[RATE_STEP] - 1; /* to round up */ + ftmp = div64_ul(ftmp, clkd->rates[RATE_STEP]); - *rate = ftmp * clk->range.step_size + fmin; + *rate = ftmp * clkd->rates[RATE_STEP] + fmin; return 0; } @@ -1122,17 +1131,16 @@ static int scmi_clock_protocol_init(const struct scmi_protocol_handle *ph) if (ret) return ret; - cinfo->clk = devm_kcalloc(ph->dev, cinfo->num_clocks, - sizeof(*cinfo->clk), GFP_KERNEL); - if (!cinfo->clk) + cinfo->clkds = devm_kcalloc(ph->dev, cinfo->num_clocks, + sizeof(*cinfo->clkds), GFP_KERNEL); + if (!cinfo->clkds) return -ENOMEM; for (clkid = 0; clkid < cinfo->num_clocks; clkid++) { - struct scmi_clock_info *clk = cinfo->clk + clkid; - + cinfo->clkds[clkid].id = clkid; ret = scmi_clock_attributes_get(ph, clkid, cinfo); if (!ret) - scmi_clock_describe_rates_get(ph, clkid, clk); + scmi_clock_describe_rates_get(ph, clkid, cinfo); } if (PROTOCOL_REV_MAJOR(ph->version) >= 0x3) { diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h index 984117f51695..f82747adc8eb 100644 --- a/include/linux/scmi_protocol.h +++ b/include/linux/scmi_protocol.h @@ -51,6 +51,8 @@ struct scmi_clock_info { bool rate_ctrl_forbidden; bool parent_ctrl_forbidden; bool extended_config; + u64 min_rate; + u64 max_rate; union { struct { int num_rates; From cdcd2fc94936f78752e8e4829b1dc9962f0c2383 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Fri, 8 May 2026 16:32:50 +0100 Subject: [PATCH 11/31] clk: scmi: Use new simplified per-clock rate properties Use the new min_rate and max_rate unified properties that provide the proper values without having to consider the clock type. Cc: Michael Turquette Cc: Stephen Boyd Cc: linux-clk@vger.kernel.org Reviewed-by: Peng Fan Signed-off-by: Cristian Marussi Tested-by: Florian Fainelli Tested-by: Geert Uytterhoeven Link: https://patch.msgid.link/20260508153300.2224715-6-cristian.marussi@arm.com Reviewed-by: Brian Masney Signed-off-by: Sudeep Holla --- drivers/clk/clk-scmi.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c index c223e4ef1dd1..7c562559ad8b 100644 --- a/drivers/clk/clk-scmi.c +++ b/drivers/clk/clk-scmi.c @@ -202,7 +202,6 @@ static int scmi_clk_ops_init(struct device *dev, struct scmi_clk *sclk, const struct clk_ops *scmi_ops) { int ret; - unsigned long min_rate, max_rate; struct clk_init_data init = { .flags = CLK_GET_RATE_NOCACHE, @@ -217,20 +216,8 @@ static int scmi_clk_ops_init(struct device *dev, struct scmi_clk *sclk, if (ret) return ret; - if (sclk->info->rate_discrete) { - int num_rates = sclk->info->list.num_rates; - - if (num_rates <= 0) - return -EINVAL; - - min_rate = sclk->info->list.rates[0]; - max_rate = sclk->info->list.rates[num_rates - 1]; - } else { - min_rate = sclk->info->range.min_rate; - max_rate = sclk->info->range.max_rate; - } - - clk_hw_set_rate_range(&sclk->hw, min_rate, max_rate); + clk_hw_set_rate_range(&sclk->hw, sclk->info->min_rate, + sclk->info->max_rate); return ret; } From 2e757f71a5ab861478204e2907bb373ccb3ca087 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Fri, 8 May 2026 16:32:51 +0100 Subject: [PATCH 12/31] firmware: arm_scmi: Drop unused clock rate interfaces Only the unified interface exposing min_rate/max_rate is now used. Reviewed-by: Peng Fan Signed-off-by: Cristian Marussi Link: https://patch.msgid.link/20260508153300.2224715-7-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/clock.c | 9 --------- include/linux/scmi_protocol.h | 12 ------------ 2 files changed, 21 deletions(-) diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c index 512400122b85..467b13a3a18f 100644 --- a/drivers/firmware/arm_scmi/clock.c +++ b/drivers/firmware/arm_scmi/clock.c @@ -467,7 +467,6 @@ iter_clk_describe_update_state(struct scmi_iterator_state *st, st->num_remaining = NUM_REMAINING(flags); st->num_returned = NUM_RETURNED(flags); p->clkd->rate_discrete = RATE_DISCRETE(flags); - p->clkd->info.rate_discrete = p->clkd->rate_discrete; /* Warn about out of spec replies ... */ if (!p->clkd->rate_discrete && @@ -532,21 +531,13 @@ scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id, return 0; if (!clkd->rate_discrete) { - clkd->info.range.min_rate = clkd->rates[RATE_MIN]; - clkd->info.range.max_rate = clkd->rates[RATE_MAX]; - clkd->info.range.step_size = clkd->rates[RATE_STEP]; clkd->info.max_rate = clkd->rates[RATE_MAX]; dev_dbg(ph->dev, "Min %llu Max %llu Step %llu Hz\n", clkd->rates[RATE_MIN], clkd->rates[RATE_MAX], clkd->rates[RATE_STEP]); } else { - unsigned int i; - sort(clkd->rates, clkd->num_rates, sizeof(clkd->rates[0]), rate_cmp_func, NULL); - clkd->info.list.num_rates = clkd->num_rates; - for (i = 0; i < clkd->num_rates; i++) - clkd->info.list.rates[i] = clkd->rates[i]; clkd->info.max_rate = clkd->rates[clkd->num_rates - 1]; } clkd->info.min_rate = clkd->rates[RATE_MIN]; diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h index f82747adc8eb..9913b4f097d8 100644 --- a/include/linux/scmi_protocol.h +++ b/include/linux/scmi_protocol.h @@ -44,7 +44,6 @@ struct scmi_base_info { struct scmi_clock_info { char name[SCMI_MAX_STR_SIZE]; unsigned int enable_latency; - bool rate_discrete; bool rate_changed_notifications; bool rate_change_requested_notifications; bool state_ctrl_forbidden; @@ -53,17 +52,6 @@ struct scmi_clock_info { bool extended_config; u64 min_rate; u64 max_rate; - union { - struct { - int num_rates; - u64 rates[SCMI_MAX_NUM_RATES]; - } list; - struct { - u64 min_rate; - u64 max_rate; - u64 step_size; - } range; - }; int num_parents; u32 *parents; }; From 62ba967595e0b2599768f886851ba5e0d4bfb55b Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Fri, 8 May 2026 16:32:52 +0100 Subject: [PATCH 13/31] firmware: arm_scmi: Make clock rates allocation dynamic Leveraging SCMI Clock protocol dynamic discovery capabilities, move away from the static per-clock rates allocation model in favour of a dynamic runtime allocation based on effectively discovered resources. No functional change. Reviewed-by: Peng Fan Signed-off-by: Cristian Marussi Tested-by: Florian Fainelli Tested-by: Geert Uytterhoeven Link: https://patch.msgid.link/20260508153300.2224715-8-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/clock.c | 19 ++++++++++++++++--- include/linux/scmi_protocol.h | 1 - 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c index 467b13a3a18f..c9b62edce4fd 100644 --- a/drivers/firmware/arm_scmi/clock.c +++ b/drivers/firmware/arm_scmi/clock.c @@ -161,7 +161,7 @@ struct scmi_clock_desc { u32 id; bool rate_discrete; unsigned int num_rates; - u64 rates[SCMI_MAX_NUM_RATES]; + u64 *rates; #define RATE_MIN 0 #define RATE_MAX 1 #define RATE_STEP 2 @@ -480,6 +480,18 @@ iter_clk_describe_update_state(struct scmi_iterator_state *st, QUIRK_OUT_OF_SPEC_TRIPLET); } + if (!st->max_resources) { + int num_rates = st->num_returned + st->num_remaining; + + p->clkd->rates = devm_kcalloc(p->dev, num_rates, + sizeof(*p->clkd->rates), GFP_KERNEL); + if (!p->clkd->rates) + return -ENOMEM; + + /* max_resources is used by the iterators to control bounds */ + st->max_resources = st->num_returned + st->num_remaining; + } + return 0; } @@ -493,6 +505,8 @@ iter_clk_describe_process_response(const struct scmi_protocol_handle *ph, p->clkd->rates[st->desc_index + st->loop_idx] = RATE_TO_U64(r->rate[st->loop_idx]); + + /* Count only effectively discovered rates */ p->clkd->num_rates++; return 0; @@ -515,8 +529,7 @@ scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id, .dev = ph->dev, }; - iter = ph->hops->iter_response_init(ph, &ops, SCMI_MAX_NUM_RATES, - CLOCK_DESCRIBE_RATES, + iter = ph->hops->iter_response_init(ph, &ops, 0, CLOCK_DESCRIBE_RATES, sizeof(struct scmi_msg_clock_describe_rates), &cpriv); if (IS_ERR(iter)) diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h index 9913b4f097d8..55db9ba8fac3 100644 --- a/include/linux/scmi_protocol.h +++ b/include/linux/scmi_protocol.h @@ -15,7 +15,6 @@ #define SCMI_MAX_STR_SIZE 64 #define SCMI_SHORT_NAME_MAX_SIZE 16 -#define SCMI_MAX_NUM_RATES 16 /** * struct scmi_base_info - version information structure From bda40491e0cedcdf5b25a5e12d21d105cd4033bf Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Fri, 8 May 2026 16:32:53 +0100 Subject: [PATCH 14/31] firmware: arm_scmi: Harden clock parents discovery Fix clock parents enumeration to account only for effectively discovered parents during enumeration, avoiding to trust the total number of parents declared upfront by the platform. Reviewed-by: Peng Fan Signed-off-by: Cristian Marussi Tested-by: Florian Fainelli Tested-by: Geert Uytterhoeven Link: https://patch.msgid.link/20260508153300.2224715-9-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/clock.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c index c9b62edce4fd..d07cfef243fd 100644 --- a/drivers/firmware/arm_scmi/clock.c +++ b/drivers/firmware/arm_scmi/clock.c @@ -270,15 +270,15 @@ static int iter_clk_possible_parents_update_state(struct scmi_iterator_state *st * assume it's returned+remaining on first call. */ if (!st->max_resources) { - p->clkd->info.num_parents = st->num_returned + st->num_remaining; - p->clkd->info.parents = devm_kcalloc(p->dev, - p->clkd->info.num_parents, + int num_parents = st->num_returned + st->num_remaining; + + p->clkd->info.parents = devm_kcalloc(p->dev, num_parents, sizeof(*p->clkd->info.parents), GFP_KERNEL); - if (!p->clkd->info.parents) { - p->clkd->info.num_parents = 0; + if (!p->clkd->info.parents) return -ENOMEM; - } + + /* max_resources is used by the iterators to control bounds */ st->max_resources = st->num_returned + st->num_remaining; } @@ -293,9 +293,11 @@ static int iter_clk_possible_parents_process_response(const struct scmi_protocol const struct scmi_msg_resp_clock_possible_parents *r = response; struct scmi_clk_ipriv *p = priv; - u32 *parent = &p->clkd->info.parents[st->desc_index + st->loop_idx]; + p->clkd->info.parents[st->desc_index + st->loop_idx] = + le32_to_cpu(r->possible_parents[st->loop_idx]); - *parent = le32_to_cpu(r->possible_parents[st->loop_idx]); + /* Count only effectively discovered parents */ + p->clkd->info.num_parents++; return 0; } From e99ed72672637662aa0207af0204185203516b28 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Fri, 8 May 2026 16:32:54 +0100 Subject: [PATCH 15/31] firmware: arm_scmi: Refactor iterators internal allocation Use cleanup handlers to manage iterator data structures. No functional change. Reviewed-by: Peng Fan Signed-off-by: Cristian Marussi Tested-by: Florian Fainelli Tested-by: Geert Uytterhoeven Link: https://patch.msgid.link/20260508153300.2224715-10-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/driver.c | 35 +++++++++++++++--------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 53310beb67a8..143f0ec60aae 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -17,6 +17,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #include +#include #include #include #include @@ -1789,39 +1790,41 @@ static void *scmi_iterator_init(const struct scmi_protocol_handle *ph, size_t tx_size, void *priv) { int ret; - struct scmi_iterator *i; - i = devm_kzalloc(ph->dev, sizeof(*i), GFP_KERNEL); + struct scmi_iterator *i __free(kfree) = kzalloc(sizeof(*i), GFP_KERNEL); if (!i) return ERR_PTR(-ENOMEM); + if (!ops || !ph) + return ERR_PTR(-EINVAL); + i->ph = ph; i->ops = ops; i->priv = priv; ret = ph->xops->xfer_get_init(ph, msg_id, tx_size, 0, &i->t); - if (ret) { - devm_kfree(ph->dev, i); + if (ret) return ERR_PTR(ret); - } i->state.max_resources = max_resources; i->msg = i->t->tx.buf; i->resp = i->t->rx.buf; - return i; + return no_free_ptr(i); } static int scmi_iterator_run(void *iter) { - int ret = -EINVAL; + int ret; struct scmi_iterator_ops *iops; const struct scmi_protocol_handle *ph; struct scmi_iterator_state *st; - struct scmi_iterator *i = iter; - if (!i || !i->ops || !i->ph) - return ret; + if (!iter) + return -EINVAL; + + /* Take ownership of the iterator */ + struct scmi_iterator *i __free(kfree) = iter; iops = i->ops; ph = i->ph; @@ -1846,12 +1849,12 @@ static int scmi_iterator_run(void *iter) break; } - for (st->loop_idx = 0; st->loop_idx < st->num_returned; - st->loop_idx++) { + for (st->loop_idx = 0; !ret && st->loop_idx < st->num_returned; + st->loop_idx++) ret = iops->process_response(ph, i->resp, st, i->priv); - if (ret) - goto out; - } + + if (ret) + break; st->desc_index += st->num_returned; ph->xops->reset_rx_to_maxsz(ph, i->t); @@ -1861,10 +1864,8 @@ static int scmi_iterator_run(void *iter) */ } while (st->num_returned && st->num_remaining); -out: /* Finalize and destroy iterator */ ph->xops->xfer_put(ph, i->t); - devm_kfree(ph->dev, i); return ret; } From 4848d07ea9fc5e4c2239e10b3eb9fe7e647aaa12 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Fri, 8 May 2026 16:32:55 +0100 Subject: [PATCH 16/31] firmware: arm_scmi: Add bound iterators support SCMI core stack provides some common helpers to handle in a unified way multipart message replies: such iterator-helpers, when run, currently process by default the whole set of discovered resources. Introduce an alternative way to run the initialized iterator on a limited range of resources. Note that the subset of resources that can be chosen is anyway limited by the SCMI protocol specification, since you are only allowed to choose the start-index on a multi-part enumeration NOT the end-index, so that the effective number of returned items by a bound iterators depends really on platform side decisions. Suggested-by: Etienne Carriere Signed-off-by: Cristian Marussi Tested-by: Florian Fainelli Tested-by: Geert Uytterhoeven Link: https://patch.msgid.link/20260508153300.2224715-11-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/clock.c | 3 +- drivers/firmware/arm_scmi/driver.c | 58 +++++++++++++++++++-------- drivers/firmware/arm_scmi/protocols.h | 13 +++++- 3 files changed, 55 insertions(+), 19 deletions(-) diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c index d07cfef243fd..8ce889dfc87b 100644 --- a/drivers/firmware/arm_scmi/clock.c +++ b/drivers/firmware/arm_scmi/clock.c @@ -505,8 +505,7 @@ iter_clk_describe_process_response(const struct scmi_protocol_handle *ph, struct scmi_clk_ipriv *p = priv; const struct scmi_msg_resp_clock_describe_rates *r = response; - p->clkd->rates[st->desc_index + st->loop_idx] = - RATE_TO_U64(r->rate[st->loop_idx]); + p->clkd->rates[p->clkd->num_rates] = RATE_TO_U64(r->rate[st->loop_idx]); /* Count only effectively discovered rates */ p->clkd->num_rates++; diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 143f0ec60aae..a59d8f9219ed 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -1813,48 +1813,50 @@ static void *scmi_iterator_init(const struct scmi_protocol_handle *ph, return no_free_ptr(i); } -static int scmi_iterator_run(void *iter) +static int __scmi_iterator_run(void *iter, unsigned int *start, unsigned int *end) { int ret; struct scmi_iterator_ops *iops; const struct scmi_protocol_handle *ph; struct scmi_iterator_state *st; + struct scmi_iterator *i; if (!iter) return -EINVAL; - /* Take ownership of the iterator */ - struct scmi_iterator *i __free(kfree) = iter; - + i = iter; iops = i->ops; ph = i->ph; st = &i->state; + /* Reinitialize state for next run */ + st->num_returned = 0; + st->num_remaining = 0; + st->desc_index = start ? *start : 0; + do { iops->prepare_message(i->msg, st->desc_index, i->priv); ret = ph->xops->do_xfer(ph, i->t); if (ret) - break; + return ret; st->rx_len = i->t->rx.len; ret = iops->update_state(st, i->resp, i->priv); if (ret) - break; + return ret; if (st->num_returned > st->max_resources - st->desc_index) { dev_err(ph->dev, "No. of resources can't exceed %d\n", st->max_resources); - ret = -EINVAL; - break; + return -EINVAL; } - for (st->loop_idx = 0; !ret && st->loop_idx < st->num_returned; - st->loop_idx++) + for (st->loop_idx = 0; st->loop_idx < st->num_returned; st->loop_idx++) { ret = iops->process_response(ph, i->resp, st, i->priv); - - if (ret) - break; + if (ret) + return ret; + } st->desc_index += st->num_returned; ph->xops->reset_rx_to_maxsz(ph, i->t); @@ -1862,14 +1864,36 @@ static int scmi_iterator_run(void *iter) * check for both returned and remaining to avoid infinite * loop due to buggy firmware */ - } while (st->num_returned && st->num_remaining); + } while (st->num_returned && st->num_remaining && + (!end || (st->desc_index <= min(*end, st->max_resources - 1)))); - /* Finalize and destroy iterator */ - ph->xops->xfer_put(ph, i->t); + return 0; +} + +static void scmi_iterator_cleanup(void *iter) +{ + struct scmi_iterator *i = iter; + + i->ph->xops->xfer_put(i->ph, i->t); + kfree(i); +} + +static int scmi_iterator_run(void *iter) +{ + int ret; + + ret = __scmi_iterator_run(iter, NULL, NULL); + scmi_iterator_cleanup(iter); return ret; } +static int scmi_iterator_run_bound(void *iter, unsigned int *start, + unsigned int *end) +{ + return __scmi_iterator_run(iter, start, end); +} + struct scmi_msg_get_fc_info { __le32 domain; __le32 message_id; @@ -2048,6 +2072,8 @@ static const struct scmi_proto_helpers_ops helpers_ops = { .get_max_msg_size = scmi_common_get_max_msg_size, .iter_response_init = scmi_iterator_init, .iter_response_run = scmi_iterator_run, + .iter_response_run_bound = scmi_iterator_run_bound, + .iter_response_cleanup = scmi_iterator_cleanup, .protocol_msg_check = scmi_protocol_msg_check, .fastchannel_init = scmi_common_fastchannel_init, .fastchannel_db_ring = scmi_common_fastchannel_db_ring, diff --git a/drivers/firmware/arm_scmi/protocols.h b/drivers/firmware/arm_scmi/protocols.h index f51245aca259..e2ef604c16ef 100644 --- a/drivers/firmware/arm_scmi/protocols.h +++ b/drivers/firmware/arm_scmi/protocols.h @@ -259,7 +259,15 @@ struct scmi_fc_info { * multi-part responses using the custom operations * provided in @ops. * @iter_response_run: A common helper to trigger the run of a previously - * initialized iterator. + * initialized iterator. Note that unbound iterators are + * automatically cleaned up. + * @iter_response_run_bound: A common helper to trigger the run of a previously + * initialized iterator, but only within the + * specified, optional, @start and @end resource + * indexes. Note that these bound-iterators need + * explicit cleanup via @iter_response_bound_cleanup. + * @iter_response_bound_cleanup: A common helper to finally release the iterator + * for bound iterators. * @protocol_msg_check: A common helper to check is a specific protocol message * is supported. * @fastchannel_init: A common helper used to initialize FC descriptors by @@ -276,6 +284,9 @@ struct scmi_proto_helpers_ops { unsigned int max_resources, u8 msg_id, size_t tx_size, void *priv); int (*iter_response_run)(void *iter); + int (*iter_response_run_bound)(void *iter, + unsigned int *start, unsigned int *end); + void (*iter_response_cleanup)(void *iter); int (*protocol_msg_check)(const struct scmi_protocol_handle *ph, u32 message_id, u32 *attributes); void (*fastchannel_init)(const struct scmi_protocol_handle *ph, From ae4a088f13debc1d7bbb6a9b265a31d25b60ddd4 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Fri, 8 May 2026 16:32:56 +0100 Subject: [PATCH 17/31] firmware: arm_scmi: Fix bound iterators returning too many items When using a bound-iterator with an upper bound, commands are sent, and responses are received, until the upper bound is reached. However, it is up to the SCMI provider implementation to decide how many rates are returned in response to a single CLOCK_DESCRIBE_RATES command. If the last response contains rates beyond the specified upper bound, they are still passed up for further processing. This may lead to buffer overflows in unprepared callsites. While the imprecise bound handling may have been intentional (it was mentioned in the commit message introducing the code), it is still confusing for users, and may cause hard to debug crashes. Fix this by strictly enforcing the upper bound. Note that this may cause an increase in the number of CLOCK_DESCRIBE_RATES commands issued, as retrieving the last rate may no longer be done inadvertentently, but require its own command. Signed-off-by: Geert Uytterhoeven Signed-off-by: Cristian Marussi Tested-by: Florian Fainelli Link: https://patch.msgid.link/20260508153300.2224715-12-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/driver.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index a59d8f9219ed..595ce7b33c61 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -1820,6 +1820,7 @@ static int __scmi_iterator_run(void *iter, unsigned int *start, unsigned int *en const struct scmi_protocol_handle *ph; struct scmi_iterator_state *st; struct scmi_iterator *i; + unsigned int n; if (!iter) return -EINVAL; @@ -1852,13 +1853,17 @@ static int __scmi_iterator_run(void *iter, unsigned int *start, unsigned int *en return -EINVAL; } - for (st->loop_idx = 0; st->loop_idx < st->num_returned; st->loop_idx++) { + if (end) + n = min(st->num_returned, *end - st->desc_index + 1); + else + n = st->num_returned; + for (st->loop_idx = 0; st->loop_idx < n; st->loop_idx++) { ret = iops->process_response(ph, i->resp, st, i->priv); if (ret) return ret; } - st->desc_index += st->num_returned; + st->desc_index += n; ph->xops->reset_rx_to_maxsz(ph, i->t); /* * check for both returned and remaining to avoid infinite From 3065e26dac525696bd0ef2fdaff7724b1bd345f9 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Fri, 8 May 2026 16:32:57 +0100 Subject: [PATCH 18/31] firmware: arm_scmi: Use proper iter_response_bound_cleanup() name The documentation speaks of the "iter_response_bound_cleanup()" protocol helper, while the actual helper is called "iter_response_cleanup()". Settle on the former name, because the helper is only needed when using bound-iterators. Signed-off-by: Geert Uytterhoeven Signed-off-by: Cristian Marussi Tested-by: Florian Fainelli Link: https://patch.msgid.link/20260508153300.2224715-13-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/driver.c | 6 +++--- drivers/firmware/arm_scmi/protocols.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 595ce7b33c61..3e0d975ec94c 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -1875,7 +1875,7 @@ static int __scmi_iterator_run(void *iter, unsigned int *start, unsigned int *en return 0; } -static void scmi_iterator_cleanup(void *iter) +static void scmi_iterator_bound_cleanup(void *iter) { struct scmi_iterator *i = iter; @@ -1888,7 +1888,7 @@ static int scmi_iterator_run(void *iter) int ret; ret = __scmi_iterator_run(iter, NULL, NULL); - scmi_iterator_cleanup(iter); + scmi_iterator_bound_cleanup(iter); return ret; } @@ -2078,7 +2078,7 @@ static const struct scmi_proto_helpers_ops helpers_ops = { .iter_response_init = scmi_iterator_init, .iter_response_run = scmi_iterator_run, .iter_response_run_bound = scmi_iterator_run_bound, - .iter_response_cleanup = scmi_iterator_cleanup, + .iter_response_bound_cleanup = scmi_iterator_bound_cleanup, .protocol_msg_check = scmi_protocol_msg_check, .fastchannel_init = scmi_common_fastchannel_init, .fastchannel_db_ring = scmi_common_fastchannel_db_ring, diff --git a/drivers/firmware/arm_scmi/protocols.h b/drivers/firmware/arm_scmi/protocols.h index e2ef604c16ef..15ad5162e37a 100644 --- a/drivers/firmware/arm_scmi/protocols.h +++ b/drivers/firmware/arm_scmi/protocols.h @@ -286,7 +286,7 @@ struct scmi_proto_helpers_ops { int (*iter_response_run)(void *iter); int (*iter_response_run_bound)(void *iter, unsigned int *start, unsigned int *end); - void (*iter_response_cleanup)(void *iter); + void (*iter_response_bound_cleanup)(void *iter); int (*protocol_msg_check)(const struct scmi_protocol_handle *ph, u32 message_id, u32 *attributes); void (*fastchannel_init)(const struct scmi_protocol_handle *ph, From 26d04d592a47890d409d1bf77b9cbf5920f2efc8 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Fri, 8 May 2026 16:32:58 +0100 Subject: [PATCH 19/31] firmware: arm_scmi: Use bound iterators to minimize discovered rates Clock rates are guaranteed to be returned in ascending order for SCMI clock protocol versions greater than 1.0: in such a case, use bounded iterators to minimize the number of message exchanges needed to discover min and max rate. Signed-off-by: Cristian Marussi Link: https://patch.msgid.link/20260508153300.2224715-14-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/clock.c | 90 +++++++++++++++++++++++++++---- 1 file changed, 81 insertions(+), 9 deletions(-) diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c index 8ce889dfc87b..955bb9565ce3 100644 --- a/drivers/firmware/arm_scmi/clock.c +++ b/drivers/firmware/arm_scmi/clock.c @@ -160,6 +160,7 @@ struct scmi_clock_rate_notify_payld { struct scmi_clock_desc { u32 id; bool rate_discrete; + unsigned int tot_rates; unsigned int num_rates; u64 *rates; #define RATE_MIN 0 @@ -483,15 +484,16 @@ iter_clk_describe_update_state(struct scmi_iterator_state *st, } if (!st->max_resources) { - int num_rates = st->num_returned + st->num_remaining; + unsigned int tot_rates = st->num_returned + st->num_remaining; - p->clkd->rates = devm_kcalloc(p->dev, num_rates, + p->clkd->rates = devm_kcalloc(p->dev, tot_rates, sizeof(*p->clkd->rates), GFP_KERNEL); if (!p->clkd->rates) return -ENOMEM; /* max_resources is used by the iterators to control bounds */ - st->max_resources = st->num_returned + st->num_remaining; + p->clkd->tot_rates = tot_rates; + st->max_resources = tot_rates; } return 0; @@ -514,8 +516,8 @@ iter_clk_describe_process_response(const struct scmi_protocol_handle *ph, } static int -scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id, - struct clock_info *cinfo) +scmi_clock_describe_rates_get_full(const struct scmi_protocol_handle *ph, + struct scmi_clock_desc *clkd) { int ret; void *iter; @@ -524,7 +526,6 @@ scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id, .update_state = iter_clk_describe_update_state, .process_response = iter_clk_describe_process_response, }; - struct scmi_clock_desc *clkd = &cinfo->clkds[clk_id]; struct scmi_clk_ipriv cpriv = { .clkd = clkd, .dev = ph->dev, @@ -544,17 +545,88 @@ scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id, if (!clkd->num_rates) return 0; + if (clkd->rate_discrete) + sort(clkd->rates, clkd->num_rates, + sizeof(clkd->rates[0]), rate_cmp_func, NULL); + + return 0; +} + +static int +scmi_clock_describe_rates_get_lazy(const struct scmi_protocol_handle *ph, + struct scmi_clock_desc *clkd) +{ + struct scmi_iterator_ops ops = { + .prepare_message = iter_clk_describe_prepare_message, + .update_state = iter_clk_describe_update_state, + .process_response = iter_clk_describe_process_response, + }; + struct scmi_clk_ipriv cpriv = { + .clkd = clkd, + .dev = ph->dev, + }; + unsigned int first, last; + void *iter; + int ret; + + iter = ph->hops->iter_response_init(ph, &ops, 0, CLOCK_DESCRIBE_RATES, + sizeof(struct scmi_msg_clock_describe_rates), + &cpriv); + if (IS_ERR(iter)) + return PTR_ERR(iter); + + /* Try to grab a triplet, so that in case is NON-discrete we are done */ + first = 0; + last = 2; + ret = ph->hops->iter_response_run_bound(iter, &first, &last); + if (ret) + goto out; + + /* If discrete grab the last value, which should be the max */ + if (clkd->rate_discrete && clkd->tot_rates > 3) { + first = clkd->tot_rates - 1; + last = clkd->tot_rates - 1; + ret = ph->hops->iter_response_run_bound(iter, &first, &last); + } + +out: + ph->hops->iter_response_bound_cleanup(iter); + + return ret; +} + +static int +scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, + u32 clk_id, struct clock_info *cinfo) +{ + struct scmi_clock_desc *clkd = &cinfo->clkds[clk_id]; + int ret; + + /* + * Since only after SCMI Clock v1.0 the returned rates are guaranteed to + * be discovered in ascending order, lazy enumeration cannot be use for + * SCMI Clock v1.0 protocol. + */ + if (PROTOCOL_REV_MAJOR(ph->version) > 0x1) + ret = scmi_clock_describe_rates_get_lazy(ph, clkd); + else + ret = scmi_clock_describe_rates_get_full(ph, clkd); + + if (ret) + return ret; + + clkd->info.min_rate = clkd->rates[RATE_MIN]; if (!clkd->rate_discrete) { clkd->info.max_rate = clkd->rates[RATE_MAX]; dev_dbg(ph->dev, "Min %llu Max %llu Step %llu Hz\n", clkd->rates[RATE_MIN], clkd->rates[RATE_MAX], clkd->rates[RATE_STEP]); } else { - sort(clkd->rates, clkd->num_rates, - sizeof(clkd->rates[0]), rate_cmp_func, NULL); clkd->info.max_rate = clkd->rates[clkd->num_rates - 1]; + dev_dbg(ph->dev, "Clock:%s Num_Rates:%u -> Min %llu Max %llu\n", + clkd->info.name, clkd->tot_rates, + clkd->info.min_rate, clkd->info.max_rate); } - clkd->info.min_rate = clkd->rates[RATE_MIN]; return 0; } From 4a07036d615976354ac806017f23ea800f1fc489 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Fri, 8 May 2026 16:32:59 +0100 Subject: [PATCH 20/31] firmware: arm_scmi: Fix OOB in scmi_clock_describe_rates_get_lazy() Lazy discovery of discrete rates works as follows: A. Grab the first three rates, B. Grab the last rate, if there are more than three rates. It is up to the SCMI provider implementation to decide how many rates are returned in response to a single CLOCK_DESCRIBE_RATES command. Each rate received is stored in the scmi_clock_rates.rates[] array, and .num_rates is updated accordingly. When more than 3 rates have been received after step A, the last rate may have been received already, and stored in scmi_clock_rates.rates[] (which has space for scmi_clock_desc.tot_rates entries). Hence grabbing the last rate again will store it a second time, beyond the end of the array. Fix this by only grabbing the last rate when we don't already have it. Signed-off-by: Geert Uytterhoeven Signed-off-by: Cristian Marussi Link: https://patch.msgid.link/20260508153300.2224715-15-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/clock.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c index 955bb9565ce3..ab8c65ed785a 100644 --- a/drivers/firmware/arm_scmi/clock.c +++ b/drivers/firmware/arm_scmi/clock.c @@ -582,8 +582,11 @@ scmi_clock_describe_rates_get_lazy(const struct scmi_protocol_handle *ph, if (ret) goto out; - /* If discrete grab the last value, which should be the max */ - if (clkd->rate_discrete && clkd->tot_rates > 3) { + /* + * If discrete and we don't already have it, grab the last value, which + * should be the max + */ + if (clkd->rate_discrete && clkd->tot_rates > clkd->num_rates) { first = clkd->tot_rates - 1; last = clkd->tot_rates - 1; ret = ph->hops->iter_response_run_bound(iter, &first, &last); From d2488ff1a257342111e1be1348d52e8b4ecfaa36 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Fri, 8 May 2026 16:33:00 +0100 Subject: [PATCH 21/31] firmware: arm_scmi: Introduce all_rates_get clock operation Add a clock operation to get the whole set of rates available to a specific clock: when needed this request could transparently trigger a full rate discovery enumeration if this specific clock-rates were previously only lazily enumerated. Reviewed-by: Peng Fan Signed-off-by: Cristian Marussi Link: https://patch.msgid.link/20260508153300.2224715-16-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/clock.c | 83 +++++++++++++++++++++---------- include/linux/scmi_protocol.h | 9 ++++ 2 files changed, 66 insertions(+), 26 deletions(-) diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c index ab8c65ed785a..42e666a628c7 100644 --- a/drivers/firmware/arm_scmi/clock.c +++ b/drivers/firmware/arm_scmi/clock.c @@ -159,10 +159,8 @@ struct scmi_clock_rate_notify_payld { struct scmi_clock_desc { u32 id; - bool rate_discrete; unsigned int tot_rates; - unsigned int num_rates; - u64 *rates; + struct scmi_clock_rates r; #define RATE_MIN 0 #define RATE_MAX 1 #define RATE_STEP 2 @@ -469,10 +467,10 @@ iter_clk_describe_update_state(struct scmi_iterator_state *st, flags = le32_to_cpu(r->num_rates_flags); st->num_remaining = NUM_REMAINING(flags); st->num_returned = NUM_RETURNED(flags); - p->clkd->rate_discrete = RATE_DISCRETE(flags); + p->clkd->r.rate_discrete = RATE_DISCRETE(flags); /* Warn about out of spec replies ... */ - if (!p->clkd->rate_discrete && + if (!p->clkd->r.rate_discrete && (st->num_returned != 3 || st->num_remaining != 0)) { dev_warn(p->dev, "Out-of-spec CLOCK_DESCRIBE_RATES reply for %s - returned:%d remaining:%d rx_len:%zd\n", @@ -486,9 +484,9 @@ iter_clk_describe_update_state(struct scmi_iterator_state *st, if (!st->max_resources) { unsigned int tot_rates = st->num_returned + st->num_remaining; - p->clkd->rates = devm_kcalloc(p->dev, tot_rates, - sizeof(*p->clkd->rates), GFP_KERNEL); - if (!p->clkd->rates) + p->clkd->r.rates = devm_kcalloc(p->dev, tot_rates, + sizeof(*p->clkd->r.rates), GFP_KERNEL); + if (!p->clkd->r.rates) return -ENOMEM; /* max_resources is used by the iterators to control bounds */ @@ -507,10 +505,10 @@ iter_clk_describe_process_response(const struct scmi_protocol_handle *ph, struct scmi_clk_ipriv *p = priv; const struct scmi_msg_resp_clock_describe_rates *r = response; - p->clkd->rates[p->clkd->num_rates] = RATE_TO_U64(r->rate[st->loop_idx]); + p->clkd->r.rates[p->clkd->r.num_rates] = RATE_TO_U64(r->rate[st->loop_idx]); /* Count only effectively discovered rates */ - p->clkd->num_rates++; + p->clkd->r.num_rates++; return 0; } @@ -531,7 +529,13 @@ scmi_clock_describe_rates_get_full(const struct scmi_protocol_handle *ph, .dev = ph->dev, }; - iter = ph->hops->iter_response_init(ph, &ops, 0, CLOCK_DESCRIBE_RATES, + /* + * Using tot_rates as max_resources parameter here so as to trigger + * the dynamic allocation only when strictly needed: when trying a + * full enumeration after a lazy one tot_rates will be non-zero. + */ + iter = ph->hops->iter_response_init(ph, &ops, clkd->tot_rates, + CLOCK_DESCRIBE_RATES, sizeof(struct scmi_msg_clock_describe_rates), &cpriv); if (IS_ERR(iter)) @@ -542,12 +546,12 @@ scmi_clock_describe_rates_get_full(const struct scmi_protocol_handle *ph, return ret; /* empty set ? */ - if (!clkd->num_rates) + if (!clkd->r.num_rates) return 0; - if (clkd->rate_discrete) - sort(clkd->rates, clkd->num_rates, - sizeof(clkd->rates[0]), rate_cmp_func, NULL); + if (clkd->r.rate_discrete && PROTOCOL_REV_MAJOR(ph->version) == 0x1) + sort(clkd->r.rates, clkd->r.num_rates, + sizeof(clkd->r.rates[0]), rate_cmp_func, NULL); return 0; } @@ -586,7 +590,7 @@ scmi_clock_describe_rates_get_lazy(const struct scmi_protocol_handle *ph, * If discrete and we don't already have it, grab the last value, which * should be the max */ - if (clkd->rate_discrete && clkd->tot_rates > clkd->num_rates) { + if (clkd->r.rate_discrete && clkd->tot_rates > clkd->r.num_rates) { first = clkd->tot_rates - 1; last = clkd->tot_rates - 1; ret = ph->hops->iter_response_run_bound(iter, &first, &last); @@ -618,14 +622,14 @@ scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, if (ret) return ret; - clkd->info.min_rate = clkd->rates[RATE_MIN]; - if (!clkd->rate_discrete) { - clkd->info.max_rate = clkd->rates[RATE_MAX]; + clkd->info.min_rate = clkd->r.rates[RATE_MIN]; + if (!clkd->r.rate_discrete) { + clkd->info.max_rate = clkd->r.rates[RATE_MAX]; dev_dbg(ph->dev, "Min %llu Max %llu Step %llu Hz\n", - clkd->rates[RATE_MIN], clkd->rates[RATE_MAX], - clkd->rates[RATE_STEP]); + clkd->r.rates[RATE_MIN], clkd->r.rates[RATE_MAX], + clkd->r.rates[RATE_STEP]); } else { - clkd->info.max_rate = clkd->rates[clkd->num_rates - 1]; + clkd->info.max_rate = clkd->r.rates[clkd->r.num_rates - 1]; dev_dbg(ph->dev, "Clock:%s Num_Rates:%u -> Min %llu Max %llu\n", clkd->info.name, clkd->tot_rates, clkd->info.min_rate, clkd->info.max_rate); @@ -732,7 +736,7 @@ static int scmi_clock_determine_rate(const struct scmi_protocol_handle *ph, * If we can't figure out what rate it will be, so just return the * rate back to the caller. */ - if (clkd->rate_discrete) + if (clkd->r.rate_discrete) return 0; fmin = clk->min_rate; @@ -746,14 +750,40 @@ static int scmi_clock_determine_rate(const struct scmi_protocol_handle *ph, } ftmp = *rate - fmin; - ftmp += clkd->rates[RATE_STEP] - 1; /* to round up */ - ftmp = div64_ul(ftmp, clkd->rates[RATE_STEP]); + ftmp += clkd->r.rates[RATE_STEP] - 1; /* to round up */ + ftmp = div64_ul(ftmp, clkd->r.rates[RATE_STEP]); - *rate = ftmp * clkd->rates[RATE_STEP] + fmin; + *rate = ftmp * clkd->r.rates[RATE_STEP] + fmin; return 0; } +static const struct scmi_clock_rates * +scmi_clock_all_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id) +{ + struct clock_info *ci = ph->get_priv(ph); + struct scmi_clock_desc *clkd; + struct scmi_clock_info *clk; + + clk = scmi_clock_domain_lookup(ci, clk_id); + if (IS_ERR(clk) || !clk->name[0]) + return NULL; + + clkd = to_desc(clk); + /* Needs full enumeration ? */ + if (clkd->r.rate_discrete && clkd->tot_rates != clkd->r.num_rates) { + int ret; + + /* rates[] is already allocated BUT we need to re-enumerate */ + clkd->r.num_rates = 0; + ret = scmi_clock_describe_rates_get_full(ph, clkd); + if (ret) + return NULL; + } + + return &clkd->r; +} + static int scmi_clock_config_set(const struct scmi_protocol_handle *ph, u32 clk_id, enum clk_state state, @@ -1067,6 +1097,7 @@ static const struct scmi_clk_proto_ops clk_proto_ops = { .rate_get = scmi_clock_rate_get, .rate_set = scmi_clock_rate_set, .determine_rate = scmi_clock_determine_rate, + .all_rates_get = scmi_clock_all_rates_get, .enable = scmi_clock_enable, .disable = scmi_clock_disable, .state_get = scmi_clock_state_get, diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h index 55db9ba8fac3..5ab73b1ab9aa 100644 --- a/include/linux/scmi_protocol.h +++ b/include/linux/scmi_protocol.h @@ -40,6 +40,12 @@ struct scmi_base_info { char sub_vendor_id[SCMI_SHORT_NAME_MAX_SIZE]; }; +struct scmi_clock_rates { + bool rate_discrete; + unsigned int num_rates; + u64 *rates; +}; + struct scmi_clock_info { char name[SCMI_MAX_STR_SIZE]; unsigned int enable_latency; @@ -84,6 +90,7 @@ enum scmi_clock_oem_config { * clock calculating the closest allowed rate. * Note that @rate is an input/output parameter used both to * describe the requested rate and report the closest match + * @all_rates_get: get the list of all available rates for the specified clock. * @enable: enables the specified clock * @disable: disables the specified clock * @state_get: get the status of the specified clock @@ -103,6 +110,8 @@ struct scmi_clk_proto_ops { u64 rate); int (*determine_rate)(const struct scmi_protocol_handle *ph, u32 clk_id, unsigned long *rate); + const struct scmi_clock_rates __must_check *(*all_rates_get) + (const struct scmi_protocol_handle *ph, u32 clk_id); int (*enable)(const struct scmi_protocol_handle *ph, u32 clk_id, bool atomic); int (*disable)(const struct scmi_protocol_handle *ph, u32 clk_id, From b79d9b5747d961516c35ef4d5e91efa579fd3e9a Mon Sep 17 00:00:00 2001 From: Stepan Ionichev Date: Wed, 13 May 2026 14:09:00 +0500 Subject: [PATCH 22/31] clk: scpi: Unregister child clock providers on remove SCPI clock providers are registered for each child node in scpi_clk_add(), but scpi_clocks_remove() unregisters the parent node on each iteration. of_clk_del_provider() matches providers by the node used at registration time, so passing the parent node leaves the child providers registered. This leaks the provider allocations and the node references held by the clock provider core. Pass the child node to of_clk_del_provider() so the remove path matches the probe path. Fixes: cd52c2a4b5c4 ("clk: add support for clocks provided by SCP(System Control Processor)") Signed-off-by: Stepan Ionichev Link: https://patch.msgid.link/20260513090900.5323-1-sozdayvek@gmail.com (sudeep.holla: Updated commit title and message a bit) Signed-off-by: Sudeep Holla --- drivers/clk/clk-scpi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/clk-scpi.c b/drivers/clk/clk-scpi.c index 7806569cd0d5..24cee7c9fda6 100644 --- a/drivers/clk/clk-scpi.c +++ b/drivers/clk/clk-scpi.c @@ -258,7 +258,7 @@ static void scpi_clocks_remove(struct platform_device *pdev) } for_each_available_child_of_node(np, child) - of_clk_del_provider(np); + of_clk_del_provider(child); } static int scpi_clocks_probe(struct platform_device *pdev) From f6fe7c3c007df18afd289ff2d98c692fae9ab085 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Sun, 17 May 2026 20:02:40 +0100 Subject: [PATCH 23/31] firmware: arm_scmi: Read sensor config as 32-bit value The SENSOR_CONFIG_GET response contains a 32-bit sensor_config field, and the xfer is initialized with a 4-byte RX buffer. Reading it with get_unaligned_le64() can consume bytes past the returned payload. Use get_unaligned_le32() to match the protocol layout and the allocated response size. Fixes: 7b83c5f41088 ("firmware: arm_scmi: Add SCMI v3.0 sensor configuration support") Link: https://patch.msgid.link/20260517-scmi_fixes-v1-1-d86daec4defd@kernel.org Reviewed-by: Cristian Marussi Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/sensors.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c index 882d55f987d2..836c294a9f42 100644 --- a/drivers/firmware/arm_scmi/sensors.c +++ b/drivers/firmware/arm_scmi/sensors.c @@ -793,7 +793,7 @@ static int scmi_sensor_config_get(const struct scmi_protocol_handle *ph, if (!ret) { struct scmi_sensor_info *s = si->sensors + sensor_id; - *sensor_config = get_unaligned_le64(t->rx.buf); + *sensor_config = get_unaligned_le32(t->rx.buf); s->sensor_config = *sensor_config; } From 56e7e64cdd0e7209a58c8ec66028d63387402919 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Sun, 17 May 2026 20:02:41 +0100 Subject: [PATCH 24/31] firmware: arm_scmi: Validate BASE_ERROR_EVENT payload size BASE_ERROR_EVENT carries a variable number of message reports, with the count encoded in error_status. The notification parser used that count without checking whether the received payload contained all reported entries. Reject truncated payloads before copying the report array. Link: https://patch.msgid.link/20260517-scmi_fixes-v1-2-d86daec4defd@kernel.org Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/base.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c index 4df2620e3c5d..a9cea09355ff 100644 --- a/drivers/firmware/arm_scmi/base.c +++ b/drivers/firmware/arm_scmi/base.c @@ -325,6 +325,8 @@ static void *scmi_base_fill_custom_report(const struct scmi_protocol_handle *ph, void *report, u32 *src_id) { int i; + u32 error_status; + size_t expected_sz; const struct scmi_base_error_notify_payld *p = payld; struct scmi_base_error_report *r = report; @@ -338,10 +340,19 @@ static void *scmi_base_fill_custom_report(const struct scmi_protocol_handle *ph, if (evt_id != SCMI_EVENT_BASE_ERROR_EVENT || sizeof(*p) < payld_sz) return NULL; + expected_sz = offsetof(typeof(*p), msg_reports); + if (payld_sz < expected_sz) + return NULL; + r->timestamp = timestamp; r->agent_id = le32_to_cpu(p->agent_id); - r->fatal = IS_FATAL_ERROR(le32_to_cpu(p->error_status)); - r->cmd_count = ERROR_CMD_COUNT(le32_to_cpu(p->error_status)); + error_status = le32_to_cpu(p->error_status); + r->fatal = IS_FATAL_ERROR(error_status); + r->cmd_count = ERROR_CMD_COUNT(error_status); + expected_sz += r->cmd_count * sizeof(p->msg_reports[0]); + if (payld_sz < expected_sz) + return NULL; + for (i = 0; i < r->cmd_count; i++) r->reports[i] = le64_to_cpu(p->msg_reports[i]); *src_id = 0; From 32bc5496b48174dbca1f187f710955ee4d9527a1 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Sun, 17 May 2026 20:02:42 +0100 Subject: [PATCH 25/31] firmware: arm_scmi: Validate SENSOR_UPDATE payload size SENSOR_UPDATE carries one or more sensor readings after the fixed notification header. The parser derives the expected reading count from the sensor description, but it did not verify that the received payload contains those entries before parsing them. Reject truncated update notifications before reading the variable array. Link: https://patch.msgid.link/20260517-scmi_fixes-v1-3-d86daec4defd@kernel.org Reviewed-by: Cristian Marussi Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/sensors.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c index 836c294a9f42..b14bb1146356 100644 --- a/drivers/firmware/arm_scmi/sensors.c +++ b/drivers/firmware/arm_scmi/sensors.c @@ -1072,12 +1072,15 @@ scmi_sensor_fill_custom_report(const struct scmi_protocol_handle *ph, case SCMI_EVENT_SENSOR_UPDATE: { int i; + size_t expected_sz; struct scmi_sensor_info *s; const struct scmi_sensor_update_notify_payld *p = payld; struct scmi_sensor_update_report *r = report; struct sensors_info *sinfo = ph->get_priv(ph); - /* payld_sz is variable for this event */ + if (payld_sz < sizeof(*p)) + break; + r->sensor_id = le32_to_cpu(p->sensor_id); if (r->sensor_id >= sinfo->num_sensors) break; @@ -1091,6 +1094,11 @@ scmi_sensor_fill_custom_report(const struct scmi_protocol_handle *ph, * readings defined for this sensor or 1 for scalar sensors. */ r->readings_count = s->num_axis ?: 1; + expected_sz = sizeof(*p) + r->readings_count * + sizeof(p->readings[0]); + if (payld_sz < expected_sz) + break; + for (i = 0; i < r->readings_count; i++) scmi_parse_sensor_readings(&r->readings[i], &p->readings[i]); From fcca603c6a09c7fd041d9004f63421d3a4014db5 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Sun, 17 May 2026 20:02:43 +0100 Subject: [PATCH 26/31] firmware: arm_scmi: Validate Powercap domains before state access Powercap protocol v2 keeps local enable and last-cap state per domain. The v2 enable helpers and disabled-domain cap_set path indexed that state before checking that the supplied domain id was valid. Validate the domain before touching the per-domain state. Link: https://patch.msgid.link/20260517-scmi_fixes-v1-4-d86daec4defd@kernel.org Reviewed-by: Cristian Marussi Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/powercap.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/firmware/arm_scmi/powercap.c b/drivers/firmware/arm_scmi/powercap.c index ab9733f4458b..27e3c805e927 100644 --- a/drivers/firmware/arm_scmi/powercap.c +++ b/drivers/firmware/arm_scmi/powercap.c @@ -453,10 +453,14 @@ static int scmi_powercap_cap_set(const struct scmi_protocol_handle *ph, return -EINVAL; /* Just log the last set request if acting on a disabled domain */ - if (PROTOCOL_REV_MAJOR(ph->version) >= 0x2 && - !pi->states[domain_id].enabled) { - pi->states[domain_id].last_pcap = power_cap; - return 0; + if (PROTOCOL_REV_MAJOR(ph->version) >= 0x2) { + if (!scmi_powercap_dom_info_get(ph, domain_id)) + return -EINVAL; + + if (!pi->states[domain_id].enabled) { + pi->states[domain_id].last_pcap = power_cap; + return 0; + } } return __scmi_powercap_cap_set(ph, pi, domain_id, @@ -637,6 +641,9 @@ static int scmi_powercap_cap_enable_set(const struct scmi_protocol_handle *ph, if (PROTOCOL_REV_MAJOR(ph->version) < 0x2) return -EINVAL; + if (!scmi_powercap_dom_info_get(ph, domain_id)) + return -EINVAL; + if (enable == pi->states[domain_id].enabled) return 0; @@ -678,6 +685,9 @@ static int scmi_powercap_cap_enable_get(const struct scmi_protocol_handle *ph, if (PROTOCOL_REV_MAJOR(ph->version) < 0x2) return 0; + if (!scmi_powercap_dom_info_get(ph, domain_id)) + return -EINVAL; + /* * Report always real platform state; platform could have ignored * a previous disable request. Default true on any error. From f9ef3f66f4b18078e464b7606f9497e4dbeb9905 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Fri, 15 May 2026 11:59:15 +0200 Subject: [PATCH 27/31] firmware: arm_scmi: Fix OOB in scmi_power_name_get() scmi_power_name_get() does not validate the domain number passed by the external caller, which may lead to an out-of-bounds access. Fix this by returning "unknown" for invalid domains, like scmi_reset_name_get() does. Fixes: 76a6550990e296a7 ("firmware: arm_scmi: add initial support for power protocol") Signed-off-by: Geert Uytterhoeven Reviewed-by: Cristian Marussi Link: https://patch.msgid.link/75caae28bdffb55199a0bc6cac5df112a966c608.1778838987.git.geert+renesas@glider.be Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/power.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/arm_scmi/power.c b/drivers/firmware/arm_scmi/power.c index bb5062ab8280..28ef63a4ecc2 100644 --- a/drivers/firmware/arm_scmi/power.c +++ b/drivers/firmware/arm_scmi/power.c @@ -204,8 +204,12 @@ scmi_power_name_get(const struct scmi_protocol_handle *ph, u32 domain) { struct scmi_power_info *pi = ph->get_priv(ph); - struct power_dom_info *dom = pi->dom_info + domain; + struct power_dom_info *dom; + if (domain >= pi->num_domains) + return "unknown"; + + dom = pi->dom_info + domain; return dom->name; } From 9dfae7d2edb4422c2c11fb6f0198691ec58e1d79 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Sun, 10 May 2026 17:05:24 +0100 Subject: [PATCH 28/31] firmware: arm_scmi: Add transport instance handles SCMI transport drivers are initialized first and then the control is passed to the SCMI core stack: some of these transports are dependent also on some external subsytem which will have to be initialized upfront, before the transport driver itself can be deemed operational. Transport drivers like virtio or optee need a way to defer the core SCMI probing till they are fully initialized and operational and also a way to pass back the device reference to be used as a supplier while building the devlink relations. SCMI transport drivers can be probed multiple times when used in a multiple instance configuration but the capability to carry-on with multiple probes depends on the support provided by the underlying transport driver. This change will also allow for the removal of the frowned-upon trick of registering a platform driver only after the end of the transport drivers porbe to avoid explicit probe deferrals. Signed-off-by: Cristian Marussi Link: https://patch.msgid.link/20260510160527.3537474-2-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/common.h | 52 +++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 4 deletions(-) diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index 07a127dec031..9de8e4eb719a 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -462,6 +463,28 @@ struct scmi_transport_core_operations { const struct scmi_message_operations *msg; }; +/** + * struct scmi_transport_handle - Transport instance handle + * @supplier_get: A helper to retrieve the device descriptor, identifying the + * transport driver serving this SCMI instance, which will be + * used as a supplier for the core SCMI driver: returning an + * error here causes the probe sequence to be interrupted and + * return that same error code, so that each transport can decide + * which policy to implement by choosing an appropriate error. + * @supplier_put: A helper to signal that the specified transport supplier is + * no more being used and it is made available again. + * + * Note that these helpers are needed and provided only by those transports + * whose initialization relies on some other subsystem and whose relations to + * the core SCMI driver is not tracked by firmware descriptions. + */ +struct scmi_transport_handle { + struct device __must_check *(*supplier_get) + (const struct scmi_transport_handle *th); + int (*supplier_put)(const struct scmi_transport_handle *th, + struct device *dev); +}; + /** * struct scmi_transport - A structure representing a configured transport * @@ -470,35 +493,52 @@ struct scmi_transport_core_operations { * @desc: Transport descriptor * @core_ops: A pointer to a pointer used by the core SCMI stack to make the * core transport operations accessible to the transports. + * @th: An optional pointer to the transport handle */ struct scmi_transport { struct device *supplier; struct scmi_desc desc; struct scmi_transport_core_operations **core_ops; + const struct scmi_transport_handle *th; }; #define DEFINE_SCMI_TRANSPORT_DRIVER(__tag, __drv, __desc, __match, __core_ops)\ static void __tag##_dev_free(void *data) \ { \ struct platform_device *spdev = data; \ + struct scmi_transport *strans; \ + \ + strans = dev_get_platdata(&spdev->dev); \ + if (strans && strans->th) \ + strans->th->supplier_put(strans->th, strans->supplier); \ \ platform_device_unregister(spdev); \ } \ \ static int __tag##_probe(struct platform_device *pdev) \ { \ - struct device *dev = &pdev->dev; \ + struct device *dev = &pdev->dev, *supplier; \ struct platform_device *spdev; \ struct scmi_transport strans; \ int ret; \ \ + supplier = dev; \ + strans.th = device_get_match_data(dev); \ + if (strans.th) { \ + supplier = strans.th->supplier_get(strans.th); \ + if (IS_ERR(supplier)) \ + return PTR_ERR(supplier); \ + } \ + \ spdev = platform_device_alloc("arm-scmi", PLATFORM_DEVID_AUTO); \ - if (!spdev) \ - return -ENOMEM; \ + if (!spdev) { \ + ret = -ENOMEM; \ + goto err_mem; \ + } \ \ device_set_of_node_from_dev(&spdev->dev, dev); \ \ - strans.supplier = dev; \ + strans.supplier = supplier; \ memcpy(&strans.desc, &(__desc), sizeof(strans.desc)); \ strans.core_ops = &(__core_ops); \ \ @@ -515,6 +555,10 @@ static int __tag##_probe(struct platform_device *pdev) \ \ err: \ platform_device_put(spdev); \ +err_mem: \ + if (strans.th) \ + strans.th->supplier_put(strans.th, supplier); \ + \ return ret; \ } \ \ From ab6eb28a47d48c0c9edd885eca60fa5a39cc80b7 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Sun, 10 May 2026 17:05:25 +0100 Subject: [PATCH 29/31] firmware: arm_scmi: Add a generic transport supplier Add the capability to define a common generic transport supplier which embeds the logic needed to support one single unique instance of transport supplier. Signed-off-by: Cristian Marussi Link: https://patch.msgid.link/20260510160527.3537474-3-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/common.h | 111 +++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index 9de8e4eb719a..b9723c105fc1 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -502,6 +502,117 @@ struct scmi_transport { const struct scmi_transport_handle *th; }; +/** + * struct scmi_transport_supplier - Transport descriptor + * @mtx: A mutex to protect @available + * @available: A reference to an initialized transport device, when available. + * This reference is implicitly used to track the status of the + * supplier and it can cycle through the following 3 states: + * 1. NOT_READY - PTR_ERR(-EPROBE_DEFER): no supplier available; + * this is the transport initial state. + * 2. AVAILABLE - : a transport supplier has been + * initialized and it is available, ready to use. + * 3. BUSY _ PTR_ERR(-EBUSY): transport supplier is currently in use. + * @th: An embedded transport handle object that embeds the helpers + * implementing the above mentioned logic + * + * Note that this transport driver enforces single instance probing. + */ +struct scmi_transport_supplier { + /* Protect @available */ + struct mutex mtx; + struct device *available; + const struct scmi_transport_handle th; +}; + +#define to_sup(t) container_of(t, struct scmi_transport_supplier, th) + +/** + * scmi_transport_supplier_put - A helper to dispose of a supplier + * @th: A reference to the transport handle to use + * @supplier: A reference to the device supplier to manage, cannot be NULL + * or ERR_PTR. + * + * Note that putting a supplier will have different effect based on the + * current state of scmi_transport_supplier.available: + * - NOT_READY/BUSY: @supplier will be set as the new available device: this + * can be used to made available a supplier OR stop using one. + * - AVAILABLE: if the @supplier we are disposing of matches the currently + * available one, roll back to NOT_READY state. + * Any other attempt to override an available supplier with a + * new one is rejected, effectively enforcing one single supplier. + * + * Return: 0 on Success, errno otherwise. + */ +static inline int +scmi_transport_supplier_put(const struct scmi_transport_handle *th, + struct device *supplier) +{ + struct scmi_transport_supplier *sup = to_sup(th); + + /* Nothing to do when the provided supplier was never real */ + if (IS_ERR_OR_NULL(supplier)) + return 0; + + guard(mutex)(&sup->mtx); + switch (PTR_ERR_OR_ZERO(sup->available)) { + case -EPROBE_DEFER: + case -EBUSY: + sup->available = supplier; + break; + case 0: + /* Putting a supplier when in the AVAILABLE state causes a + * transition back to the NOT_READY state, BUT only if the + * supplier we are disposing of was exactly the device that was + * previously made readily available. + */ + if (supplier != sup->available) + return -EINVAL; + sup->available = ERR_PTR(-EPROBE_DEFER); + break; + default: + return -EINVAL; + } + + return 0; +} + +/** + * scmi_transport_supplier_get - A helper to get hold of a supplier + * @th: A reference to the transport handle to use + * + * Note that, trying to get a supplier device can return: + * - a ready to use supplier device, (subsequently made unavailable) + * - PTR_ERR(-EPROBE_DEFER): no supplier is available + * - PTR_ERR(-BUSY): supplier was already taken by a previous get + * + * This allows the probe to defer and wait when a possible device can + * be reasonably expected to appear. + * + * Return: a usable supplier device on Success or PTR_ERR on Failure. + */ +static inline struct device * +scmi_transport_supplier_get(const struct scmi_transport_handle *th) +{ + struct scmi_transport_supplier *sup = to_sup(th); + struct device *supplier; + + guard(mutex)(&sup->mtx); + supplier = sup->available; + if (!IS_ERR(sup->available)) + sup->available = ERR_PTR(-EBUSY); + + return supplier; +} + +#define DEFINE_SCMI_TRANSPORT_SUPPLIER(__supplier) \ +struct scmi_transport_supplier __supplier = { \ + .mtx = __MUTEX_INITIALIZER(__supplier.mtx), \ + .available = INIT_ERR_PTR(-EPROBE_DEFER), \ + .th.supplier_get = scmi_transport_supplier_get, \ + .th.supplier_put = scmi_transport_supplier_put, \ +} + #define DEFINE_SCMI_TRANSPORT_DRIVER(__tag, __drv, __desc, __match, __core_ops)\ static void __tag##_dev_free(void *data) \ { \ From c08051901a55f8574968b606f960a70415be303c Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Sun, 10 May 2026 17:05:26 +0100 Subject: [PATCH 30/31] firmware: arm_scmi: virtio: Rework transport probe sequence Use the new per-instance transport handles helpers to synchronize and optionally defer the core SCMI driver probe up until the transport driver has completely been initialized and it is fully operational as a supplier. Introduce proper module init/exit routines while removing the ugly trick of registering a driver from within the probe sequence of another one, just to avoid to have to deal with probe deferrals. Signed-off-by: Cristian Marussi Link: https://patch.msgid.link/20260510160527.3537474-4-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/transports/virtio.c | 52 +++++++++++++++---- 1 file changed, 43 insertions(+), 9 deletions(-) diff --git a/drivers/firmware/arm_scmi/transports/virtio.c b/drivers/firmware/arm_scmi/transports/virtio.c index 326c4a93e44b..3282d8271839 100644 --- a/drivers/firmware/arm_scmi/transports/virtio.c +++ b/drivers/firmware/arm_scmi/transports/virtio.c @@ -4,7 +4,7 @@ * (SCMI). * * Copyright (C) 2020-2022 OpenSynergy. - * Copyright (C) 2021-2024 ARM Ltd. + * Copyright (C) 2021-2026 ARM Ltd. */ /** @@ -116,6 +116,8 @@ static struct scmi_transport_core_operations *core; /* Only one SCMI VirtIO device can possibly exist */ static struct virtio_device *scmi_vdev; +static DEFINE_SCMI_TRANSPORT_SUPPLIER(scmi_virtio_supplier); + static void scmi_vio_channel_ready(struct scmi_vio_channel *vioch, struct scmi_chan_info *cinfo) { @@ -394,6 +396,10 @@ static bool virtio_chan_available(struct device_node *of_node, int idx) return false; } + dev_dbg(&scmi_vdev->dev, "%s Channel %sAVAILABLE on SCMI Virtio device.\n", + idx == VIRTIO_SCMI_VQ_TX ? "TX" : "RX", + (vioch && !vioch->cinfo) ? "" : "NOT "); + return vioch && !vioch->cinfo; } @@ -410,7 +416,7 @@ static int virtio_chan_setup(struct scmi_chan_info *cinfo, struct device *dev, int i; if (!scmi_vdev) - return -EPROBE_DEFER; + return -EINVAL; vioch = &((struct scmi_vio_channel *)scmi_vdev->priv)[index]; @@ -460,6 +466,9 @@ static int virtio_chan_setup(struct scmi_chan_info *cinfo, struct device *dev, scmi_vio_channel_ready(vioch, cinfo); + dev_dbg(&scmi_vdev->dev, "%s Channel SETUP on SCMI Virtio device.\n", + tx ? "TX" : "RX"); + return 0; } @@ -801,7 +810,7 @@ static struct scmi_desc scmi_virtio_desc = { }; static const struct of_device_id scmi_of_match[] = { - { .compatible = "arm,scmi-virtio" }, + { .compatible = "arm,scmi-virtio", .data = &scmi_virtio_supplier.th}, { /* Sentinel */ }, }; @@ -864,33 +873,33 @@ static int scmi_vio_probe(struct virtio_device *vdev) sz = MSG_TOKEN_MAX; } channels[i].max_msg = sz; + dev_dbg(dev, "VQ%d initialized with max_msg: %d\n", i, sz); } vdev->priv = channels; - /* Ensure initialized scmi_vdev is visible */ smp_store_mb(scmi_vdev, vdev); /* Set device ready */ virtio_device_ready(vdev); - ret = platform_driver_register(&scmi_virtio_driver); + ret = scmi_transport_supplier_put(&scmi_virtio_supplier.th, &vdev->dev); if (ret) { + virtio_reset_device(vdev); vdev->priv = NULL; vdev->config->del_vqs(vdev); /* Ensure NULLified scmi_vdev is visible */ smp_store_mb(scmi_vdev, NULL); - return ret; } + dev_dbg(dev, "Probed and initialized SCMI Virtio device.\n"); + return 0; } static void scmi_vio_remove(struct virtio_device *vdev) { - platform_driver_unregister(&scmi_virtio_driver); - /* * Once we get here, virtio_chan_free() will have already been called by * the SCMI core for any existing channel and, as a consequence, all the @@ -900,8 +909,10 @@ static void scmi_vio_remove(struct virtio_device *vdev) */ virtio_reset_device(vdev); vdev->config->del_vqs(vdev); + /* Ensure scmi_vdev is visible as NULL */ smp_store_mb(scmi_vdev, NULL); + scmi_transport_supplier_put(&scmi_virtio_supplier.th, &vdev->dev); } static int scmi_vio_validate(struct virtio_device *vdev) @@ -936,7 +947,30 @@ static struct virtio_driver virtio_scmi_driver = { .validate = scmi_vio_validate, }; -module_virtio_driver(virtio_scmi_driver); +static int __init scmi_transport_virtio_init(void) +{ + int ret; + + ret = register_virtio_driver(&virtio_scmi_driver); + if (ret) + return ret; + + ret = platform_driver_register(&scmi_virtio_driver); + if (ret) { + unregister_virtio_driver(&virtio_scmi_driver); + return ret; + } + + return ret; +} +module_init(scmi_transport_virtio_init); + +static void __exit scmi_transport_virtio_exit(void) +{ + platform_driver_unregister(&scmi_virtio_driver); + unregister_virtio_driver(&virtio_scmi_driver); +} +module_exit(scmi_transport_virtio_exit); MODULE_AUTHOR("Igor Skalkin "); MODULE_AUTHOR("Peter Hilber "); From 524abd2fa6907ebe2762342be339afcc5b227dc4 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Sun, 10 May 2026 17:05:27 +0100 Subject: [PATCH 31/31] firmware: arm_scmi: optee: Rework transport probe sequence Use the new per-instance transport handles helpers to synchronize and optionally defer the core SCMI driver probe up until the transport driver has completely been initialized and it is fully operational as a supplier. Introduce proper module init/exit routines while removing the ugly trick of registering a driver from within the probe sequence of another one, just to avoid to have to deal with probe deferrals. Signed-off-by: Cristian Marussi Link: https://patch.msgid.link/20260510160527.3537474-5-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/transports/optee.c | 46 +++++++++++++++----- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/drivers/firmware/arm_scmi/transports/optee.c b/drivers/firmware/arm_scmi/transports/optee.c index 07ae18d5279d..dbe32e141748 100644 --- a/drivers/firmware/arm_scmi/transports/optee.c +++ b/drivers/firmware/arm_scmi/transports/optee.c @@ -154,6 +154,8 @@ static struct scmi_transport_core_operations *core; /* There can be only 1 SCMI service in OP-TEE we connect to */ static struct scmi_optee_agent *scmi_optee_private; +static DEFINE_SCMI_TRANSPORT_SUPPLIER(scmi_optee_supplier); + /* Open a session toward SCMI OP-TEE service with REE_KERNEL identity */ static int open_session(struct scmi_optee_agent *agent, u32 *tee_session) { @@ -522,7 +524,7 @@ static struct scmi_desc scmi_optee_desc = { }; static const struct of_device_id scmi_of_match[] = { - { .compatible = "linaro,scmi-optee" }, + { .compatible = "linaro,scmi-optee", .data = &scmi_optee_supplier.th}, { /* Sentinel */ }, }; @@ -561,18 +563,20 @@ static int scmi_optee_service_probe(struct tee_client_device *scmi_pta) if (ret) goto err; - /* Ensure agent resources are all visible before scmi_optee_private is */ + /* Ensure initialized scmi_optee_private is visible */ smp_mb(); scmi_optee_private = agent; - ret = platform_driver_register(&scmi_optee_driver); - if (ret) { - scmi_optee_private = NULL; - goto err; - } + ret = scmi_transport_supplier_put(&scmi_optee_supplier.th, agent->dev); + if (ret) + goto err_put; return 0; +err_put: + /* Ensure cleared reference is visible before resources are released */ + smp_store_mb(scmi_optee_private, NULL); + err: tee_client_close_context(tee_ctx); @@ -586,13 +590,12 @@ static void scmi_optee_service_remove(struct tee_client_device *scmi_pta) if (!scmi_optee_private) return; - platform_driver_unregister(&scmi_optee_driver); - if (!list_empty(&scmi_optee_private->channel_list)) return; /* Ensure cleared reference is visible before resources are released */ smp_store_mb(scmi_optee_private, NULL); + scmi_transport_supplier_put(&scmi_optee_supplier.th, agent->dev); tee_client_close_context(agent->tee_ctx); } @@ -616,7 +619,30 @@ static struct tee_client_driver scmi_optee_service_driver = { }, }; -module_tee_client_driver(scmi_optee_service_driver); +static int __init scmi_transport_optee_init(void) +{ + int ret; + + ret = tee_client_driver_register(&scmi_optee_service_driver); + if (ret) + return ret; + + ret = platform_driver_register(&scmi_optee_driver); + if (ret) { + tee_client_driver_unregister(&scmi_optee_service_driver); + return ret; + } + + return ret; +} +module_init(scmi_transport_optee_init); + +static void __exit scmi_transport_optee_exit(void) +{ + platform_driver_unregister(&scmi_optee_driver); + tee_client_driver_unregister(&scmi_optee_service_driver); +} +module_exit(scmi_transport_optee_exit); MODULE_AUTHOR("Etienne Carriere "); MODULE_DESCRIPTION("SCMI OPTEE Transport driver");