Re: [PATCH v3 6/9] firmware: arm_scmi: Refactor protocol device creation logic
From: Jonathan Cameron <hidden>
Date: 2026-08-24 17:49:55
Also in:
arm-scmi
On Thu, 13 Aug 2026 12:33:01 +0100 Sudeep Holla [off-list ref] wrote:
Move the protocol validation and device creation logic in scmi_probe() into a reusable scmi_device_check_create() helper. The helper centralizes checks for the protocol ID range, implementation availability and duplicate activation before invoking scmi_create_protocol_devices(). This preserves the existing behavior while allowing the logic to be reused by the ACPI path, where protocol child fwnodes are absent. No functional change intended. Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
A trivial thing and one follow on from earlier. For that just be consistent with what you choose to do for the earlier comment. Reviewed-by: Jonathan Cameron <redacted>
quoted hunk ↗ jump to hunk
--- drivers/firmware/arm_scmi/driver.c | 60 ++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 25 deletions(-)diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index aad678db0f6e..951ba3df6ba6 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c@@ -3244,6 +3244,40 @@ static void scmi_enable_matching_quirks(struct scmi_info *info) rev->sub_vendor_id, rev->impl_ver); } +static void scmi_device_check_create(struct fwnode_handle *fwnode, int prot_id, + struct scmi_info *info) +{ + int ret; + struct device *dev = info->dev; + struct scmi_handle *handle = &info->handle; + + if (!FIELD_FIT(MSG_PROTOCOL_ID_MASK, prot_id)) { + dev_err(dev, "Out of range protocol %d\n", prot_id); + return; + } + + if (!scmi_is_protocol_implemented(handle, prot_id)) { + dev_err(dev, "SCMI protocol %d not implemented\n", + prot_id);
Fits on one line. I didn't check if you modify this later in the series though so if this is churn reduction, fine to leave it as it stands.
+ return;
+ }
+
+ /*
+ * Save this valid fwnode protocol descriptor amongst
+ * @active_protocols for this SCMI instance.
+ */
+ ret = idr_alloc(&info->active_protocols, fwnode,
+ prot_id, prot_id + 1, GFP_KERNEL);
+ if (ret != prot_id) {
+ dev_err(dev, "SCMI protocol %d already activated. Skip\n",
+ prot_id);
+ return;
+ }
+
+ fwnode_handle_get(fwnode);
+ scmi_create_protocol_devices(fwnode, info, prot_id, NULL);If you take the suggest I made earlier to make the hand over of ownership slightly more obvious by using the return value of fwmonde_handle_get() then applies here too.
+}