* [PATCH v5 01/16] firmware: arm_scmi: Publish channel state before callbacks
2026-07-09 8:52 [PATCH v5 00/16] firmware: arm_scmi: Fix SCMI core cleanup paths Sudeep Holla
@ 2026-07-09 8:52 ` Sudeep Holla
2026-07-09 8:52 ` [PATCH v5 02/16] firmware: arm_scmi: Quiesce notifications before teardown Sudeep Holla
` (14 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Sudeep Holla @ 2026-07-09 8:52 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel; +Cc: Cristian Marussi
Transport setup can enable callbacks before the setup routine returns.
mailbox_chan_setup() registers the mailbox client with
mbox_request_channel(), and the mailbox controller startup path can enable
interrupt delivery before SCMI mailbox channel state has been published.
Similarly, smc_chan_setup() requests the optional A2P completion IRQ before
the SMC transport has made its cinfo pointer visible.
If a pending or spurious callback fires in those windows, the transport RX
callback can dereference a NULL transport cinfo pointer. Publishing only
the transport-private pointer is not sufficient either: an early callback
can enter the SCMI core before scmi_chan_setup() has assigned
cinfo->handle.
The core derives scmi_info from cinfo->handle in the RX path, so a NULL
handle can still fault even when the transport-private cinfo is valid.
Assign cinfo->handle before invoking the transport setup callback. Publish
the mailbox and SMC transport-private channel state before requesting the
mailbox channels or IRQ, and clear the early-published pointers again on
setup failure. Also unwind mailbox setup devres resources on failure so an
optional RX setup error that is ignored by the core does not leave stale
transport state behind.
Fixes: 5c8a47a5a91d ("firmware: arm_scmi: Make scmi core independent of the transport type")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/firmware/arm_scmi/driver.c | 2 +-
drivers/firmware/arm_scmi/transports/mailbox.c | 18 +++++++++++++-----
drivers/firmware/arm_scmi/transports/smc.c | 15 +++++++++------
3 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 3e0d975ec94c..1d1f5d25d773 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2782,6 +2782,7 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
cinfo->id = prot_id;
cinfo->dev = &tdev->dev;
+ cinfo->handle = &info->handle;
ret = info->desc->ops->chan_setup(cinfo, info->dev, tx);
if (ret) {
of_node_put(of_node);
@@ -2814,7 +2815,6 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
return ret;
}
- cinfo->handle = &info->handle;
return 0;
}
diff --git a/drivers/firmware/arm_scmi/transports/mailbox.c b/drivers/firmware/arm_scmi/transports/mailbox.c
index ae0f67e6cc45..b6459fbb8151 100644
--- a/drivers/firmware/arm_scmi/transports/mailbox.c
+++ b/drivers/firmware/arm_scmi/transports/mailbox.c
@@ -211,13 +211,18 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
cl->tx_block = false;
cl->knows_txdone = tx;
+ cinfo->transport_info = smbox;
+ smbox->cinfo = cinfo;
+ mutex_init(&smbox->chan_lock);
+
smbox->chan = mbox_request_channel(cl, tx ? 0 : p2a_chan);
if (IS_ERR(smbox->chan)) {
ret = PTR_ERR(smbox->chan);
+ smbox->chan = NULL;
if (ret != -EPROBE_DEFER)
dev_err(cdev,
"failed to request SCMI %s mailbox\n", desc);
- return ret;
+ goto err_clear_cinfo;
}
/* Additional unidirectional channel for TX if needed */
@@ -241,11 +246,14 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
}
}
- cinfo->transport_info = smbox;
- smbox->cinfo = cinfo;
- mutex_init(&smbox->chan_lock);
-
return 0;
+
+err_clear_cinfo:
+ cinfo->transport_info = NULL;
+ smbox->cinfo = NULL;
+ devm_iounmap(dev, smbox->shmem);
+ devm_kfree(dev, smbox);
+ return ret;
}
static int mailbox_chan_free(int id, void *p, void *data)
diff --git a/drivers/firmware/arm_scmi/transports/smc.c b/drivers/firmware/arm_scmi/transports/smc.c
index 21abb571e4f2..1fce3ccdeb7f 100644
--- a/drivers/firmware/arm_scmi/transports/smc.c
+++ b/drivers/firmware/arm_scmi/transports/smc.c
@@ -172,6 +172,13 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
scmi_info->param_page = SHMEM_PAGE(res.start);
scmi_info->param_offset = SHMEM_OFFSET(res.start);
}
+
+ scmi_info->func_id = func_id;
+ scmi_info->cap_id = cap_id;
+ scmi_info->cinfo = cinfo;
+ smc_channel_lock_init(scmi_info);
+ cinfo->transport_info = scmi_info;
+
/*
* If there is an interrupt named "a2p", then the service and
* completion of a message is signaled by an interrupt rather than by
@@ -183,18 +190,14 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
IRQF_NO_SUSPEND, dev_name(dev), scmi_info);
if (ret) {
dev_err(dev, "failed to setup SCMI smc irq\n");
+ cinfo->transport_info = NULL;
+ scmi_info->cinfo = NULL;
return ret;
}
} else {
cinfo->no_completion_irq = true;
}
- scmi_info->func_id = func_id;
- scmi_info->cap_id = cap_id;
- scmi_info->cinfo = cinfo;
- smc_channel_lock_init(scmi_info);
- cinfo->transport_info = scmi_info;
-
return 0;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v5 02/16] firmware: arm_scmi: Quiesce notifications before teardown
2026-07-09 8:52 [PATCH v5 00/16] firmware: arm_scmi: Fix SCMI core cleanup paths Sudeep Holla
2026-07-09 8:52 ` [PATCH v5 01/16] firmware: arm_scmi: Publish channel state before callbacks Sudeep Holla
@ 2026-07-09 8:52 ` Sudeep Holla
2026-07-09 8:52 ` [PATCH v5 03/16] firmware: arm_scmi: Clean up channels on setup failure Sudeep Holla
` (13 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Sudeep Holla @ 2026-07-09 8:52 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel; +Cc: Cristian Marussi
scmi_notification_exit() clears and releases the notification instance, but
transport callbacks can still deliver incoming notifications until the
TX/RX channels are freed. During remove, an RX interrupt in that window can
enter scmi_notify() while notification state is being torn down and then
dereference freed memory. The same ordering exists on the probe error path
after notification initialization.
The notification late-init worker has a separate lifetime issue: protocol
event registration queues ni->init_work on the system workqueue, so
destroying ni->notify_wq does not drain that work. If the devres group is
released while init_work is still pending or running, the late-init worker
can dereference the freed notification instance.
Unregister the requested device notifier first, quiesce the notification
core so no late-init work can run while channels are being torn down, then
clean up TX/RX channels before releasing the notification core resources.
The final notification exit path also cancels any pending late-init work
before destroying the notification workqueue and releasing the notification
devres group.
Fixes: 1e7cbfaa66d3 ("firmware: arm_scmi: Free mailbox channels if probe fails")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/firmware/arm_scmi/driver.c | 13 +++++++------
drivers/firmware/arm_scmi/notify.c | 32 +++++++++++++++++++++++++++++++-
drivers/firmware/arm_scmi/notify.h | 1 +
3 files changed, 39 insertions(+), 7 deletions(-)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 1d1f5d25d773..1354de59ea84 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -3325,7 +3325,7 @@ static int scmi_probe(struct platform_device *pdev)
dev_err(dev, "%s", err_str);
return 0;
}
- goto notification_exit;
+ goto raw_mode_cleanup;
}
mutex_lock(&scmi_list_mutex);
@@ -3367,17 +3367,18 @@ static int scmi_probe(struct platform_device *pdev)
return 0;
-notification_exit:
+raw_mode_cleanup:
if (IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT))
scmi_raw_mode_cleanup(info->raw);
- scmi_notification_exit(&info->handle);
clear_dev_req_notifier:
blocking_notifier_chain_unregister(&scmi_requested_devices_nh,
&info->dev_req_nb);
clear_bus_notifier:
bus_unregister_notifier(&scmi_bus_type, &info->bus_nb);
clear_txrx_setup:
+ scmi_notification_quiesce(&info->handle);
scmi_cleanup_txrx_channels(info);
+ scmi_notification_exit(&info->handle);
clear_ida:
ida_free(&scmi_id, info->id);
@@ -3401,6 +3402,9 @@ static void scmi_remove(struct platform_device *pdev)
list_del(&info->node);
mutex_unlock(&scmi_list_mutex);
+ /* Stop transport callbacks before tearing down notifications. */
+ scmi_notification_quiesce(&info->handle);
+ scmi_cleanup_txrx_channels(info);
scmi_notification_exit(&info->handle);
mutex_lock(&info->protocols_mtx);
@@ -3415,9 +3419,6 @@ static void scmi_remove(struct platform_device *pdev)
&info->dev_req_nb);
bus_unregister_notifier(&scmi_bus_type, &info->bus_nb);
- /* Safe to free channels since no more users */
- scmi_cleanup_txrx_channels(info);
-
ida_free(&scmi_id, info->id);
}
diff --git a/drivers/firmware/arm_scmi/notify.c b/drivers/firmware/arm_scmi/notify.c
index 40ec184eedae..2491c848d4f3 100644
--- a/drivers/firmware/arm_scmi/notify.c
+++ b/drivers/firmware/arm_scmi/notify.c
@@ -209,6 +209,7 @@ struct scmi_registered_events_desc;
* @init_work: A work item to perform final initializations of pending handlers
* @notify_wq: A reference to the allocated Kernel cmwq
* @pending_mtx: A mutex to protect @pending_events_handlers
+ * @shutting_down: Flag to block late-init work scheduling during teardown
* @registered_protocols: A statically allocated array containing pointers to
* all the registered protocol-level specific information
* related to events' handling
@@ -225,6 +226,7 @@ struct scmi_notify_instance {
struct workqueue_struct *notify_wq;
/* lock to protect pending_events_handlers */
struct mutex pending_mtx;
+ bool shutting_down;
struct scmi_registered_events_desc **registered_protocols;
DECLARE_HASHTABLE(pending_events_handlers, SCMI_PENDING_HASH_SZ);
};
@@ -845,7 +847,10 @@ int scmi_register_protocol_events(const struct scmi_handle *handle, u8 proto_id,
* Finalize any pending events' handler which could have been waiting
* for this protocol's events registration.
*/
- schedule_work(&ni->init_work);
+ mutex_lock(&ni->pending_mtx);
+ if (!ni->shutting_down)
+ schedule_work(&ni->init_work);
+ mutex_unlock(&ni->pending_mtx);
return 0;
}
@@ -1706,6 +1711,29 @@ int scmi_notification_init(struct scmi_handle *handle)
return -ENOMEM;
}
+/**
+ * scmi_notification_quiesce() - Stop notification late initialization
+ * @handle: The handle identifying the platform instance to quiesce
+ *
+ * Prevent new late-init work from being queued and wait for any already queued
+ * or running late-init work to complete before transport channels are torn
+ * down.
+ */
+void scmi_notification_quiesce(struct scmi_handle *handle)
+{
+ struct scmi_notify_instance *ni;
+
+ ni = scmi_notification_instance_data_get(handle);
+ if (!ni)
+ return;
+
+ mutex_lock(&ni->pending_mtx);
+ ni->shutting_down = true;
+ mutex_unlock(&ni->pending_mtx);
+
+ cancel_work_sync(&ni->init_work);
+}
+
/**
* scmi_notification_exit() - Shutdown and clean Notification core
* @handle: The handle identifying the platform instance to shutdown
@@ -1717,6 +1745,8 @@ void scmi_notification_exit(struct scmi_handle *handle)
ni = scmi_notification_instance_data_get(handle);
if (!ni)
return;
+
+ scmi_notification_quiesce(handle);
scmi_notification_instance_data_set(handle, NULL);
/* Destroy while letting pending work complete */
diff --git a/drivers/firmware/arm_scmi/notify.h b/drivers/firmware/arm_scmi/notify.h
index 76758a736cf4..f18f98c5ab3b 100644
--- a/drivers/firmware/arm_scmi/notify.h
+++ b/drivers/firmware/arm_scmi/notify.h
@@ -82,6 +82,7 @@ struct scmi_protocol_events {
};
int scmi_notification_init(struct scmi_handle *handle);
+void scmi_notification_quiesce(struct scmi_handle *handle);
void scmi_notification_exit(struct scmi_handle *handle);
int scmi_register_protocol_events(const struct scmi_handle *handle, u8 proto_id,
const struct scmi_protocol_handle *ph,
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v5 03/16] firmware: arm_scmi: Clean up channels on setup failure
2026-07-09 8:52 [PATCH v5 00/16] firmware: arm_scmi: Fix SCMI core cleanup paths Sudeep Holla
2026-07-09 8:52 ` [PATCH v5 01/16] firmware: arm_scmi: Publish channel state before callbacks Sudeep Holla
2026-07-09 8:52 ` [PATCH v5 02/16] firmware: arm_scmi: Quiesce notifications before teardown Sudeep Holla
@ 2026-07-09 8:52 ` Sudeep Holla
2026-07-09 8:52 ` [PATCH v5 04/16] firmware: arm_scmi: Free transport channel on IDR failure Sudeep Holla
` (12 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Sudeep Holla @ 2026-07-09 8:52 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel; +Cc: Cristian Marussi
scmi_channels_setup() can fail after the common BASE channel or earlier
protocol channels have already been registered in the TX/RX IDRs.
Route this failure through the existing channel cleanup label so the
transport channels, transport devices and IDR state created before the
failure are released before the probe error path frees the SCMI instance
ID.
Fixes: 05a2801d8b90 ("firmware: arm_scmi: Use dedicated devices to initialize channels")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/firmware/arm_scmi/driver.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 1354de59ea84..ef41b377d3de 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -3263,7 +3263,7 @@ static int scmi_probe(struct platform_device *pdev)
ret = scmi_channels_setup(info);
if (ret) {
err_str = "failed to setup channels\n";
- goto clear_ida;
+ goto clear_txrx_setup;
}
ret = bus_register_notifier(&scmi_bus_type, &info->bus_nb);
@@ -3379,7 +3379,6 @@ static int scmi_probe(struct platform_device *pdev)
scmi_notification_quiesce(&info->handle);
scmi_cleanup_txrx_channels(info);
scmi_notification_exit(&info->handle);
-clear_ida:
ida_free(&scmi_id, info->id);
out_err:
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v5 04/16] firmware: arm_scmi: Free transport channel on IDR failure
2026-07-09 8:52 [PATCH v5 00/16] firmware: arm_scmi: Fix SCMI core cleanup paths Sudeep Holla
` (2 preceding siblings ...)
2026-07-09 8:52 ` [PATCH v5 03/16] firmware: arm_scmi: Clean up channels on setup failure Sudeep Holla
@ 2026-07-09 8:52 ` Sudeep Holla
2026-07-09 8:52 ` [PATCH v5 05/16] firmware: arm_scmi: Avoid IDR updates while cleaning channels Sudeep Holla
` (11 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Sudeep Holla @ 2026-07-09 8:52 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel; +Cc: Cristian Marussi
If transport channel setup succeeds but the following IDR insertion fails,
the error path destroys the transport device and frees the channel info
without invoking the transport cleanup callback.
Call chan_free() before destroying the device so transport specific
resources such as IRQs, mailbox channels and mapped shared memory are
released consistently with the normal teardown path.
Fixes: 05a2801d8b90 ("firmware: arm_scmi: Use dedicated devices to initialize channels")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/firmware/arm_scmi/driver.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index ef41b377d3de..29c89aa4710b 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2808,6 +2808,7 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
"unable to allocate SCMI idr slot err %d\n", ret);
/* Destroy channel and device only if created by this call. */
if (tdev) {
+ info->desc->ops->chan_free(prot_id, cinfo, idr);
of_node_put(of_node);
scmi_device_destroy(info->dev, prot_id, name);
devm_kfree(info->dev, cinfo);
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v5 05/16] firmware: arm_scmi: Avoid IDR updates while cleaning channels
2026-07-09 8:52 [PATCH v5 00/16] firmware: arm_scmi: Fix SCMI core cleanup paths Sudeep Holla
` (3 preceding siblings ...)
2026-07-09 8:52 ` [PATCH v5 04/16] firmware: arm_scmi: Free transport channel on IDR failure Sudeep Holla
@ 2026-07-09 8:52 ` Sudeep Holla
2026-07-09 8:52 ` [PATCH v5 06/16] firmware: arm_scmi: Reject out of range DT protocol IDs Sudeep Holla
` (10 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Sudeep Holla @ 2026-07-09 8:52 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel; +Cc: Cristian Marussi
scmi_cleanup_channels() walks the TX/RX channel IDRs with
idr_for_each() to free transport resources and destroy the dedicated
transport devices before calling idr_destroy().
The destroy callback removed each entry from the same IDR being walked.
That is not needed for this cleanup path, and it is unsafe because
idr_for_each() has not advanced its radix-tree iterator while the
callback is running. Removing the current entry from the callback can
invalidate the iterator state. The callback also cannot be protected by
rcu_read_lock(), because scmi_device_destroy() may sleep.
Leave IDR teardown to the following idr_destroy() call and keep the
callback limited to device destruction.
Fixes: 05a2801d8b90 ("firmware: arm_scmi: Use dedicated devices to initialize channels")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
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 29c89aa4710b..ad4587d75a08 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2885,7 +2885,7 @@ static int scmi_channels_setup(struct scmi_info *info)
return 0;
}
-static int scmi_chan_destroy(int id, void *p, void *idr)
+static int scmi_chan_destroy(int id, void *p, void *data)
{
struct scmi_chan_info *cinfo = p;
@@ -2898,8 +2898,6 @@ static int scmi_chan_destroy(int id, void *p, void *idr)
cinfo->dev = NULL;
}
- idr_remove(idr, id);
-
return 0;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v5 06/16] firmware: arm_scmi: Reject out of range DT protocol IDs
2026-07-09 8:52 [PATCH v5 00/16] firmware: arm_scmi: Fix SCMI core cleanup paths Sudeep Holla
` (4 preceding siblings ...)
2026-07-09 8:52 ` [PATCH v5 05/16] firmware: arm_scmi: Avoid IDR updates while cleaning channels Sudeep Holla
@ 2026-07-09 8:52 ` Sudeep Holla
2026-07-09 8:52 ` [PATCH v5 07/16] firmware: arm_scmi: Use channel ID for transport teardown Sudeep Holla
` (9 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Sudeep Holla @ 2026-07-09 8:52 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel; +Cc: Cristian Marussi
SCMI protocol IDs carried in message headers are limited by
MSG_PROTOCOL_ID_MASK. The DT parsing paths noticed protocol IDs
outside that range, but only logged an error and then kept processing
the invalid value.
That lets a malformed 32-bit DT reg value reach helpers which take a u8
protocol ID, where it can be truncated and/or treated as a different
protocol.
For channel setup, two different out-of-range values can also be used as
distinct IDR keys while aliasing the generated SCMI protocol identity.
Skip DT protocol nodes whose reg value does not fit the SCMI protocol ID
field before setting up channels or creating protocol devices.
Fixes: 05a2801d8b90 ("firmware: arm_scmi: Use dedicated devices to initialize channels")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/firmware/arm_scmi/driver.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index ad4587d75a08..81939309ce7e 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2873,9 +2873,11 @@ static int scmi_channels_setup(struct scmi_info *info)
if (of_property_read_u32(child, "reg", &prot_id))
continue;
- if (!FIELD_FIT(MSG_PROTOCOL_ID_MASK, prot_id))
+ if (!FIELD_FIT(MSG_PROTOCOL_ID_MASK, prot_id)) {
dev_err(info->dev,
"Out of range protocol %d\n", prot_id);
+ continue;
+ }
ret = scmi_txrx_setup(info, child, prot_id);
if (ret)
@@ -3339,8 +3341,10 @@ static int scmi_probe(struct platform_device *pdev)
if (of_property_read_u32(child, "reg", &prot_id))
continue;
- if (!FIELD_FIT(MSG_PROTOCOL_ID_MASK, prot_id))
+ if (!FIELD_FIT(MSG_PROTOCOL_ID_MASK, prot_id)) {
dev_err(dev, "Out of range protocol %d\n", prot_id);
+ continue;
+ }
if (!scmi_is_protocol_implemented(handle, prot_id)) {
dev_err(dev, "SCMI protocol %d not implemented\n",
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v5 07/16] firmware: arm_scmi: Use channel ID for transport teardown
2026-07-09 8:52 [PATCH v5 00/16] firmware: arm_scmi: Fix SCMI core cleanup paths Sudeep Holla
` (5 preceding siblings ...)
2026-07-09 8:52 ` [PATCH v5 06/16] firmware: arm_scmi: Reject out of range DT protocol IDs Sudeep Holla
@ 2026-07-09 8:52 ` Sudeep Holla
2026-07-09 8:52 ` [PATCH v5 08/16] firmware: arm_scmi: Unregister device notifier before IDR teardown Sudeep Holla
` (8 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Sudeep Holla @ 2026-07-09 8:52 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel; +Cc: Cristian Marussi
SCMI protocols can share the BASE transport channel when firmware does
not describe a dedicated channel for the protocol. In that case multiple
IDR entries can point at the same scmi_chan_info, whose owning transport
device was created with cinfo->id.
scmi_chan_destroy() used the IDR iterator key when destroying the
transport device. If an alias entry is visited before the owning channel
entry, the lookup can miss the device because the iterator key does not
match the protocol ID used when the transport device was created. The
code then clears cinfo->dev, so the later owning entry skips teardown and
leaks the transport device.
Destroy the transport device using cinfo->id, which is the protocol ID
that owns the channel and was used when creating the transport device.
Fixes: 05a2801d8b90 ("firmware: arm_scmi: Use dedicated devices to initialize channels")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/firmware/arm_scmi/driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 81939309ce7e..0d93ea3ef4d5 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2896,7 +2896,7 @@ static int scmi_chan_destroy(int id, void *p, void *data)
struct scmi_device *sdev = to_scmi_dev(cinfo->dev);
of_node_put(cinfo->dev->of_node);
- scmi_device_destroy(info->dev, id, sdev->name);
+ scmi_device_destroy(info->dev, cinfo->id, sdev->name);
cinfo->dev = NULL;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v5 08/16] firmware: arm_scmi: Unregister device notifier before IDR teardown
2026-07-09 8:52 [PATCH v5 00/16] firmware: arm_scmi: Fix SCMI core cleanup paths Sudeep Holla
` (6 preceding siblings ...)
2026-07-09 8:52 ` [PATCH v5 07/16] firmware: arm_scmi: Use channel ID for transport teardown Sudeep Holla
@ 2026-07-09 8:52 ` Sudeep Holla
2026-07-09 8:52 ` [PATCH v5 09/16] firmware: arm_scmi: Protect device request lookup with RCU Sudeep Holla
` (7 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Sudeep Holla @ 2026-07-09 8:52 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel; +Cc: Cristian Marussi
The requested-devices notifier looks up protocol fwnodes from the
active_protocols IDR. During remove, unregister the notifier before
releasing and destroying active_protocols so no notifier callback can race
with the IDR teardown.
Keep the bus notifier registered until after the protocol state is torn
down, matching the existing remove ordering for SCMI bus users.
Fixes: 53b8c25df708 ("firmware: arm_scmi: Add common notifier helpers")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/firmware/arm_scmi/driver.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 0d93ea3ef4d5..f607557c04ad 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -3404,6 +3404,9 @@ static void scmi_remove(struct platform_device *pdev)
list_del(&info->node);
mutex_unlock(&scmi_list_mutex);
+ blocking_notifier_chain_unregister(&scmi_requested_devices_nh,
+ &info->dev_req_nb);
+
/* Stop transport callbacks before tearing down notifications. */
scmi_notification_quiesce(&info->handle);
scmi_cleanup_txrx_channels(info);
@@ -3417,8 +3420,6 @@ static void scmi_remove(struct platform_device *pdev)
of_node_put(child);
idr_destroy(&info->active_protocols);
- blocking_notifier_chain_unregister(&scmi_requested_devices_nh,
- &info->dev_req_nb);
bus_unregister_notifier(&scmi_bus_type, &info->bus_nb);
ida_free(&scmi_id, info->id);
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v5 09/16] firmware: arm_scmi: Protect device request lookup with RCU
2026-07-09 8:52 [PATCH v5 00/16] firmware: arm_scmi: Fix SCMI core cleanup paths Sudeep Holla
` (7 preceding siblings ...)
2026-07-09 8:52 ` [PATCH v5 08/16] firmware: arm_scmi: Unregister device notifier before IDR teardown Sudeep Holla
@ 2026-07-09 8:52 ` Sudeep Holla
2026-07-09 8:52 ` [PATCH v5 10/16] firmware: arm_scmi: Drop handle on protocol bind failures Sudeep Holla
` (6 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Sudeep Holla @ 2026-07-09 8:52 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel; +Cc: Cristian Marussi
The SCMI device request notifier looks up protocol OF nodes from the
active_protocols IDR. The IDR lookup can run concurrently with protocol
activation while probe is still registering protocols and creating their
SCMI devices.
Wrap the lookup in an RCU read-side critical section as required by the
IDR API for lockless readers.
Fixes: 53b8c25df708 ("firmware: arm_scmi: Add common notifier helpers")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/firmware/arm_scmi/driver.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index f607557c04ad..a2871dca24dc 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -33,6 +33,7 @@
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/processor.h>
+#include <linux/rcupdate.h>
#include <linux/refcount.h>
#include <linux/slab.h>
#include <linux/xarray.h>
@@ -2958,7 +2959,9 @@ static int scmi_device_request_notifier(struct notifier_block *nb,
struct scmi_device_id *id_table = data;
struct scmi_info *info = req_nb_to_scmi_info(nb);
+ rcu_read_lock();
np = idr_find(&info->active_protocols, id_table->protocol_id);
+ rcu_read_unlock();
if (!np)
return NOTIFY_DONE;
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v5 10/16] firmware: arm_scmi: Drop handle on protocol bind failures
2026-07-09 8:52 [PATCH v5 00/16] firmware: arm_scmi: Fix SCMI core cleanup paths Sudeep Holla
` (8 preceding siblings ...)
2026-07-09 8:52 ` [PATCH v5 09/16] firmware: arm_scmi: Protect device request lookup with RCU Sudeep Holla
@ 2026-07-09 8:52 ` Sudeep Holla
2026-07-09 8:52 ` [PATCH v5 11/16] firmware: arm_scmi: Clear SystemPower flag on create failure Sudeep Holla
` (5 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Sudeep Holla @ 2026-07-09 8:52 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel; +Cc: Cristian Marussi
The SCMI bus notifier acquires an SCMI handle when the driver core emits
BUS_NOTIFY_BIND_DRIVER, before invoking the protocol driver probe
callback. The protocol probe path only checks whether sdev->handle is
set.
If device_link_add() fails after the handle has been acquired, the
protocol device can still bind with a valid handle but without the
dependency link to the SCMI parent. A concurrent parent unbind can then
miss the child and tear down the SCMI instance while the child still
holds a handle into it.
If the protocol driver probe later fails, for example with
-EPROBE_DEFER, the driver core emits BUS_NOTIFY_DRIVER_NOT_BOUND rather
than BUS_NOTIFY_UNBOUND_DRIVER. The SCMI notifier only released the
handle on BUS_NOTIFY_UNBOUND_DRIVER, so each failed protocol-device bind
leaked the SCMI instance users refcount and left sdev->handle set after
the failed probe.
Make the link helper report failure and drop the acquired handle if the
link cannot be created. Also handle BUS_NOTIFY_DRIVER_NOT_BOUND in the
same cleanup path used for unbind so failed probes balance the earlier
BUS_NOTIFY_BIND_DRIVER acquisition.
Fixes: 971fc0665f13 ("firmware: arm_scmi: Move handle get/set helpers")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/firmware/arm_scmi/driver.c | 31 +++++++++++++++++++++++--------
1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index a2871dca24dc..84d1294f269c 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2629,21 +2629,31 @@ static int scmi_handle_put(const struct scmi_handle *handle)
return 0;
}
-static void scmi_device_link_add(struct device *consumer,
+static bool scmi_device_link_add(struct device *consumer,
struct device *supplier)
{
struct device_link *link;
link = device_link_add(consumer, supplier, DL_FLAG_AUTOREMOVE_CONSUMER);
- WARN_ON(!link);
+ return !WARN_ON(!link);
+}
+
+static void scmi_clear_handle(struct scmi_device *scmi_dev)
+{
+ if (!scmi_dev->handle)
+ return;
+
+ scmi_handle_put(scmi_dev->handle);
+ scmi_dev->handle = NULL;
}
static void scmi_set_handle(struct scmi_device *scmi_dev)
{
scmi_dev->handle = scmi_handle_get(&scmi_dev->dev);
- if (scmi_dev->handle)
- scmi_device_link_add(&scmi_dev->dev, scmi_dev->handle->dev);
+ if (scmi_dev->handle &&
+ !scmi_device_link_add(&scmi_dev->dev, scmi_dev->handle->dev))
+ scmi_clear_handle(scmi_dev);
}
static int __scmi_xfer_info_init(struct scmi_info *sinfo,
@@ -2927,6 +2937,7 @@ static int scmi_bus_notifier(struct notifier_block *nb,
{
struct scmi_info *info = bus_nb_to_scmi_info(nb);
struct scmi_device *sdev = to_scmi_dev(data);
+ const char *status;
/* Skip devices of different SCMI instances */
if (sdev->dev.parent != info->dev)
@@ -2936,18 +2947,22 @@ static int scmi_bus_notifier(struct notifier_block *nb,
case BUS_NOTIFY_BIND_DRIVER:
/* setup handle now as the transport is ready */
scmi_set_handle(sdev);
+ status = "about to be BOUND.";
+ break;
+ case BUS_NOTIFY_DRIVER_NOT_BOUND:
+ scmi_clear_handle(sdev);
+ status = "NOT BOUND.";
break;
case BUS_NOTIFY_UNBOUND_DRIVER:
- scmi_handle_put(sdev->handle);
- sdev->handle = NULL;
+ scmi_clear_handle(sdev);
+ status = "UNBOUND.";
break;
default:
return NOTIFY_DONE;
}
dev_dbg(info->dev, "Device %s (%s) is now %s\n", dev_name(&sdev->dev),
- sdev->name, action == BUS_NOTIFY_BIND_DRIVER ?
- "about to be BOUND." : "UNBOUND.");
+ sdev->name, status);
return NOTIFY_OK;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v5 11/16] firmware: arm_scmi: Clear SystemPower flag on create failure
2026-07-09 8:52 [PATCH v5 00/16] firmware: arm_scmi: Fix SCMI core cleanup paths Sudeep Holla
` (9 preceding siblings ...)
2026-07-09 8:52 ` [PATCH v5 10/16] firmware: arm_scmi: Drop handle on protocol bind failures Sudeep Holla
@ 2026-07-09 8:52 ` Sudeep Holla
2026-07-09 8:52 ` [PATCH v5 12/16] firmware: arm_scmi: Fix OF node reference handling Sudeep Holla
` (4 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Sudeep Holla @ 2026-07-09 8:52 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel; +Cc: Cristian Marussi
__scmi_device_create() reserves the singleton SystemPower protocol device
before registering the SCMI device. If any later step fails, a stale
reservation can make a later retry reject SystemPower device creation
permanently, for example after probe deferral.
A plain global boolean is not enough to track the reservation. A delayed
final release of an older SystemPower device could clear the boolean after
a newer device has already claimed it, breaking the singleton guarantee for
the active device.
Track the reservation with the scmi_device pointer itself. Claim it with
cmpxchg(NULL, scmi_dev) after allocating the device object, and release it
with cmpxchg(scmi_dev, NULL) from the common cleanup helper. This lets the
create-failure, explicit destroy and final release paths clear only the
reservation owned by the device being cleaned up.
Fixes: 2c3e674465e7 ("firmware: arm_scmi: Refactor device create/destroy helpers")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/firmware/arm_scmi/bus.c | 59 ++++++++++++++++++++++-------------------
1 file changed, 32 insertions(+), 27 deletions(-)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index 793be9eabaed..dcaefc1aa892 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -7,7 +7,6 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-#include <linux/atomic.h>
#include <linux/types.h>
#include <linux/module.h>
#include <linux/of.h>
@@ -33,8 +32,8 @@ struct scmi_requested_dev {
struct list_head node;
};
-/* Track globally the creation of SCMI SystemPower related devices */
-static atomic_t scmi_syspower_registered = ATOMIC_INIT(0);
+/* Track globally the SCMI SystemPower protocol device. */
+static struct scmi_device *scmi_syspower_registered;
/**
* scmi_protocol_device_request - Helper to request a device
@@ -391,10 +390,17 @@ void scmi_driver_unregister(struct scmi_driver *driver)
}
EXPORT_SYMBOL_GPL(scmi_driver_unregister);
+static void scmi_device_release_syspower(struct scmi_device *scmi_dev)
+{
+ if (scmi_dev->protocol_id == SCMI_PROTOCOL_SYSTEM)
+ cmpxchg(&scmi_syspower_registered, scmi_dev, NULL);
+}
+
static void scmi_device_release(struct device *dev)
{
struct scmi_device *scmi_dev = to_scmi_dev(dev);
+ scmi_device_release_syspower(scmi_dev);
kfree_const(scmi_dev->name);
kfree(scmi_dev);
}
@@ -406,9 +412,7 @@ static void __scmi_device_destroy(struct scmi_device *scmi_dev)
dev_name(&scmi_dev->dev), scmi_dev->protocol_id,
scmi_dev->name);
- if (scmi_dev->protocol_id == SCMI_PROTOCOL_SYSTEM)
- atomic_set(&scmi_syspower_registered, 0);
-
+ scmi_device_release_syspower(scmi_dev);
ida_free(&scmi_bus_id, scmi_dev->id);
device_unregister(&scmi_dev->dev);
}
@@ -419,6 +423,7 @@ __scmi_device_create(struct device_node *np, struct device *parent,
{
int id, retval;
struct scmi_device *scmi_dev;
+ bool syspower = (protocol == SCMI_PROTOCOL_SYSTEM);
/*
* If the same protocol/name device already exist under the same parent
@@ -431,39 +436,33 @@ __scmi_device_create(struct device_node *np, struct device *parent,
if (scmi_dev)
return scmi_dev;
+ scmi_dev = kzalloc_obj(*scmi_dev);
+ if (!scmi_dev)
+ return NULL;
+
+ scmi_dev->protocol_id = protocol;
+
/*
- * Ignore any possible subsequent failures while creating the device
- * since we are doomed anyway at that point; not using a mutex which
- * spans across this whole function to keep things simple and to avoid
- * to serialize all the __scmi_device_create calls across possibly
- * different SCMI server instances (parent)
+ * Reserve the singleton SystemPower protocol device using the device
+ * pointer itself, so delayed release of an older device cannot clear
+ * a reservation owned by a newer device.
*/
- if (protocol == SCMI_PROTOCOL_SYSTEM &&
- atomic_cmpxchg(&scmi_syspower_registered, 0, 1)) {
+ if (syspower && cmpxchg(&scmi_syspower_registered, NULL, scmi_dev)) {
dev_warn(parent,
"SCMI SystemPower protocol device must be unique !\n");
+ kfree(scmi_dev);
return NULL;
}
- scmi_dev = kzalloc_obj(*scmi_dev);
- if (!scmi_dev)
- return NULL;
-
scmi_dev->name = kstrdup_const(name ?: "unknown", GFP_KERNEL);
- if (!scmi_dev->name) {
- kfree(scmi_dev);
- return NULL;
- }
+ if (!scmi_dev->name)
+ goto free_dev;
id = ida_alloc_min(&scmi_bus_id, 1, GFP_KERNEL);
- if (id < 0) {
- kfree_const(scmi_dev->name);
- kfree(scmi_dev);
- return NULL;
- }
+ if (id < 0)
+ goto free_name;
scmi_dev->id = id;
- scmi_dev->protocol_id = protocol;
scmi_dev->dev.parent = parent;
device_set_node(&scmi_dev->dev, of_fwnode_handle(np));
scmi_dev->dev.bus = &scmi_bus_type;
@@ -482,6 +481,12 @@ __scmi_device_create(struct device_node *np, struct device *parent,
put_device(&scmi_dev->dev);
ida_free(&scmi_bus_id, id);
return NULL;
+free_name:
+ kfree_const(scmi_dev->name);
+free_dev:
+ scmi_device_release_syspower(scmi_dev);
+ kfree(scmi_dev);
+ return NULL;
}
static struct scmi_device *
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v5 12/16] firmware: arm_scmi: Fix OF node reference handling
2026-07-09 8:52 [PATCH v5 00/16] firmware: arm_scmi: Fix SCMI core cleanup paths Sudeep Holla
` (10 preceding siblings ...)
2026-07-09 8:52 ` [PATCH v5 11/16] firmware: arm_scmi: Clear SystemPower flag on create failure Sudeep Holla
@ 2026-07-09 8:52 ` Sudeep Holla
2026-07-09 8:52 ` [PATCH v5 13/16] firmware: arm_scmi: Unwind TX receiver mailbox setup failure Sudeep Holla
` (3 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Sudeep Holla @ 2026-07-09 8:52 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel; +Cc: Cristian Marussi
SCMI devices store the DT node in dev.of_node through
device_set_node(), but that helper only assigns the fwnode and
of_node pointers without taking an OF node reference.
Take a reference when assigning the node and release it from the
SCMI device release path. With the device owning that reference,
remove the separate channel-side get/put pair from the core driver.
Fixes: 96da4a99ce50 ("firmware: arm_scmi: Set fwnode for the scmi_device")
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/firmware/arm_scmi/bus.c | 3 ++-
drivers/firmware/arm_scmi/driver.c | 4 ----
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index dcaefc1aa892..a14df82a9310 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -401,6 +401,7 @@ static void scmi_device_release(struct device *dev)
struct scmi_device *scmi_dev = to_scmi_dev(dev);
scmi_device_release_syspower(scmi_dev);
+ of_node_put(dev->of_node);
kfree_const(scmi_dev->name);
kfree(scmi_dev);
}
@@ -464,7 +465,7 @@ __scmi_device_create(struct device_node *np, struct device *parent,
scmi_dev->id = id;
scmi_dev->dev.parent = parent;
- device_set_node(&scmi_dev->dev, of_fwnode_handle(np));
+ device_set_node(&scmi_dev->dev, of_fwnode_handle(of_node_get(np)));
scmi_dev->dev.bus = &scmi_bus_type;
scmi_dev->dev.release = scmi_device_release;
dev_set_name(&scmi_dev->dev, "scmi_dev.%d", id);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 84d1294f269c..532f1f65b2ab 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2789,14 +2789,12 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
devm_kfree(info->dev, cinfo);
return -EINVAL;
}
- of_node_get(of_node);
cinfo->id = prot_id;
cinfo->dev = &tdev->dev;
cinfo->handle = &info->handle;
ret = info->desc->ops->chan_setup(cinfo, info->dev, tx);
if (ret) {
- of_node_put(of_node);
scmi_device_destroy(info->dev, prot_id, name);
devm_kfree(info->dev, cinfo);
return ret;
@@ -2820,7 +2818,6 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
/* Destroy channel and device only if created by this call. */
if (tdev) {
info->desc->ops->chan_free(prot_id, cinfo, idr);
- of_node_put(of_node);
scmi_device_destroy(info->dev, prot_id, name);
devm_kfree(info->dev, cinfo);
}
@@ -2906,7 +2903,6 @@ static int scmi_chan_destroy(int id, void *p, void *data)
struct scmi_info *info = handle_to_scmi_info(cinfo->handle);
struct scmi_device *sdev = to_scmi_dev(cinfo->dev);
- of_node_put(cinfo->dev->of_node);
scmi_device_destroy(info->dev, cinfo->id, sdev->name);
cinfo->dev = NULL;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v5 13/16] firmware: arm_scmi: Unwind TX receiver mailbox setup failure
2026-07-09 8:52 [PATCH v5 00/16] firmware: arm_scmi: Fix SCMI core cleanup paths Sudeep Holla
` (11 preceding siblings ...)
2026-07-09 8:52 ` [PATCH v5 12/16] firmware: arm_scmi: Fix OF node reference handling Sudeep Holla
@ 2026-07-09 8:52 ` Sudeep Holla
2026-07-09 8:52 ` [PATCH v5 14/16] firmware: arm_scmi: Unwind P2A " Sudeep Holla
` (2 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Sudeep Holla @ 2026-07-09 8:52 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel; +Cc: Cristian Marussi
mailbox_chan_setup() can request an additional unidirectional TX
receiver channel after successfully acquiring the primary channel. If
that second request fails, the function returns immediately and leaves
the primary channel allocated.
Unwind the primary mailbox channel before returning the error so probe
deferral or other setup failures do not leave the channel busy for later
probe attempts.
Fixes: 9f68ff79ec2c ("firmware: arm_scmi: Add support for unidirectional mailbox channels")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/firmware/arm_scmi/transports/mailbox.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/arm_scmi/transports/mailbox.c b/drivers/firmware/arm_scmi/transports/mailbox.c
index b6459fbb8151..37e3eab529ea 100644
--- a/drivers/firmware/arm_scmi/transports/mailbox.c
+++ b/drivers/firmware/arm_scmi/transports/mailbox.c
@@ -230,9 +230,10 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
smbox->chan_receiver = mbox_request_channel(cl, a2p_rx_chan);
if (IS_ERR(smbox->chan_receiver)) {
ret = PTR_ERR(smbox->chan_receiver);
+ smbox->chan_receiver = NULL;
if (ret != -EPROBE_DEFER)
dev_err(cdev, "failed to request SCMI Tx Receiver mailbox\n");
- return ret;
+ goto err_free_chan;
}
}
@@ -248,6 +249,8 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
return 0;
+err_free_chan:
+ mbox_free_channel(smbox->chan);
err_clear_cinfo:
cinfo->transport_info = NULL;
smbox->cinfo = NULL;
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v5 14/16] firmware: arm_scmi: Unwind P2A receiver mailbox setup failure
2026-07-09 8:52 [PATCH v5 00/16] firmware: arm_scmi: Fix SCMI core cleanup paths Sudeep Holla
` (12 preceding siblings ...)
2026-07-09 8:52 ` [PATCH v5 13/16] firmware: arm_scmi: Unwind TX receiver mailbox setup failure Sudeep Holla
@ 2026-07-09 8:52 ` Sudeep Holla
2026-07-09 8:52 ` [PATCH v5 15/16] firmware: arm_scmi: Fix SCMI device destroy lifetimes Sudeep Holla
2026-07-09 8:52 ` [PATCH v5 16/16] firmware: arm_scmi: Fix transport device teardown lookup Sudeep Holla
15 siblings, 0 replies; 17+ messages in thread
From: Sudeep Holla @ 2026-07-09 8:52 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel; +Cc: Cristian Marussi
mailbox_chan_setup() can request an additional P2A receiver channel after
successfully acquiring the primary P2A channel. If that later request
fails, the function returns immediately and leaves the primary channel
allocated.
Unwind the primary mailbox channel before returning the error so probe
deferral or other setup failures do not leave the channel busy for later
probe attempts.
Fixes: fa8b28ba22d9 ("firmware: arm_scmi: Add support for platform to agent channel completion")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/firmware/arm_scmi/transports/mailbox.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/arm_scmi/transports/mailbox.c b/drivers/firmware/arm_scmi/transports/mailbox.c
index 37e3eab529ea..308736c3ead9 100644
--- a/drivers/firmware/arm_scmi/transports/mailbox.c
+++ b/drivers/firmware/arm_scmi/transports/mailbox.c
@@ -241,9 +241,10 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
smbox->chan_platform_receiver = mbox_request_channel(cl, p2a_rx_chan);
if (IS_ERR(smbox->chan_platform_receiver)) {
ret = PTR_ERR(smbox->chan_platform_receiver);
+ smbox->chan_platform_receiver = NULL;
if (ret != -EPROBE_DEFER)
dev_err(cdev, "failed to request SCMI P2A Receiver mailbox\n");
- return ret;
+ goto err_free_chan;
}
}
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v5 15/16] firmware: arm_scmi: Fix SCMI device destroy lifetimes
2026-07-09 8:52 [PATCH v5 00/16] firmware: arm_scmi: Fix SCMI core cleanup paths Sudeep Holla
` (13 preceding siblings ...)
2026-07-09 8:52 ` [PATCH v5 14/16] firmware: arm_scmi: Unwind P2A " Sudeep Holla
@ 2026-07-09 8:52 ` Sudeep Holla
2026-07-09 8:52 ` [PATCH v5 16/16] firmware: arm_scmi: Fix transport device teardown lookup Sudeep Holla
15 siblings, 0 replies; 17+ messages in thread
From: Sudeep Holla @ 2026-07-09 8:52 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel; +Cc: Cristian Marussi
scmi_child_dev_find() drops the reference returned by
device_find_child() before returning the scmi_device pointer. A
concurrent unregister can then release the device while the destroy path
is still using the returned pointer.
Make the lookup helper return the device_find_child() reference and keep
it until scmi_device_destroy() has finished unregistering the child.
Also split device_unregister() in __scmi_device_destroy() so the SCMI bus
ID is not made reusable until after device_del() has removed the old
scmi_dev.N name from sysfs. This avoids a new SCMI device reusing the
same ID while the old device is still registered.
The final device release callback is also a possible cleanup path when
SCMI children are deleted by driver core recursion rather than
__scmi_device_destroy(). Release the SCMI bus ID from a common helper
used by destroy, register-failure and final-release paths, and clear
scmi_dev->id after freeing it so the final release cannot free the same
ID again.
Fixes: 9ca67840c0dd ("firmware: arm_scmi: Balance device refcount when destroying devices")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/firmware/arm_scmi/bus.c | 39 +++++++++++++++++++++++----------------
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index a14df82a9310..e1f66c08c81d 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -237,8 +237,9 @@ static int scmi_match_by_id_table(struct device *dev, const void *data)
return scmi_dev_match_by_id_table(scmi_dev, id_table);
}
-static struct scmi_device *scmi_child_dev_find(struct device *parent,
- int prot_id, const char *name)
+/* Returns a device_find_child() reference which must be dropped by caller. */
+static struct scmi_device *
+scmi_child_dev_find_get(struct device *parent, int prot_id, const char *name)
{
struct scmi_device_id id_table[2] = { 0 };
struct device *dev;
@@ -250,9 +251,6 @@ static struct scmi_device *scmi_child_dev_find(struct device *parent,
if (!dev)
return NULL;
- /* Drop the refcnt bumped implicitly by device_find_child */
- put_device(dev);
-
return to_scmi_dev(dev);
}
@@ -390,17 +388,22 @@ void scmi_driver_unregister(struct scmi_driver *driver)
}
EXPORT_SYMBOL_GPL(scmi_driver_unregister);
-static void scmi_device_release_syspower(struct scmi_device *scmi_dev)
+static void scmi_device_release_resources(struct scmi_device *scmi_dev)
{
if (scmi_dev->protocol_id == SCMI_PROTOCOL_SYSTEM)
cmpxchg(&scmi_syspower_registered, scmi_dev, NULL);
+
+ if (scmi_dev->id) {
+ ida_free(&scmi_bus_id, scmi_dev->id);
+ scmi_dev->id = 0;
+ }
}
static void scmi_device_release(struct device *dev)
{
struct scmi_device *scmi_dev = to_scmi_dev(dev);
- scmi_device_release_syspower(scmi_dev);
+ scmi_device_release_resources(scmi_dev);
of_node_put(dev->of_node);
kfree_const(scmi_dev->name);
kfree(scmi_dev);
@@ -413,9 +416,9 @@ static void __scmi_device_destroy(struct scmi_device *scmi_dev)
dev_name(&scmi_dev->dev), scmi_dev->protocol_id,
scmi_dev->name);
- scmi_device_release_syspower(scmi_dev);
- ida_free(&scmi_bus_id, scmi_dev->id);
- device_unregister(&scmi_dev->dev);
+ device_del(&scmi_dev->dev);
+ scmi_device_release_resources(scmi_dev);
+ put_device(&scmi_dev->dev);
}
static struct scmi_device *
@@ -433,9 +436,11 @@ __scmi_device_create(struct device_node *np, struct device *parent,
* each DT defined protocol at probe time, and the concurrent
* registration of SCMI drivers.
*/
- scmi_dev = scmi_child_dev_find(parent, protocol, name);
- if (scmi_dev)
+ scmi_dev = scmi_child_dev_find_get(parent, protocol, name);
+ if (scmi_dev) {
+ put_device(&scmi_dev->dev);
return scmi_dev;
+ }
scmi_dev = kzalloc_obj(*scmi_dev);
if (!scmi_dev)
@@ -479,13 +484,13 @@ __scmi_device_create(struct device_node *np, struct device *parent,
return scmi_dev;
put_dev:
+ scmi_device_release_resources(scmi_dev);
put_device(&scmi_dev->dev);
- ida_free(&scmi_bus_id, id);
return NULL;
free_name:
kfree_const(scmi_dev->name);
free_dev:
- scmi_device_release_syspower(scmi_dev);
+ scmi_device_release_resources(scmi_dev);
kfree(scmi_dev);
return NULL;
}
@@ -567,9 +572,11 @@ void scmi_device_destroy(struct device *parent, int protocol, const char *name)
{
struct scmi_device *scmi_dev;
- scmi_dev = scmi_child_dev_find(parent, protocol, name);
- if (scmi_dev)
+ scmi_dev = scmi_child_dev_find_get(parent, protocol, name);
+ if (scmi_dev) {
__scmi_device_destroy(scmi_dev);
+ put_device(&scmi_dev->dev);
+ }
}
EXPORT_SYMBOL_GPL(scmi_device_destroy);
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v5 16/16] firmware: arm_scmi: Fix transport device teardown lookup
2026-07-09 8:52 [PATCH v5 00/16] firmware: arm_scmi: Fix SCMI core cleanup paths Sudeep Holla
` (14 preceding siblings ...)
2026-07-09 8:52 ` [PATCH v5 15/16] firmware: arm_scmi: Fix SCMI device destroy lifetimes Sudeep Holla
@ 2026-07-09 8:52 ` Sudeep Holla
15 siblings, 0 replies; 17+ messages in thread
From: Sudeep Holla @ 2026-07-09 8:52 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel; +Cc: Cristian Marussi
SCMI transport devices are deliberately excluded from normal SCMI bus
matching so protocol drivers cannot bind to the internal transport
children. However, scmi_device_destroy() uses the same protocol/name
lookup to find devices that must be unregistered during channel teardown.
Split the match helper so driver matching still skips transport devices,
while explicit child lookup can find them for teardown. Use a shared
transport-device name prefix macro for both matching and name generation.
Since transport-device names are derived from direction and protocol ID,
reject duplicate protocol channel setup before creating or finding a
transport device. This prevents malformed firmware with duplicate
protocol child nodes from reusing an existing transport device and then
destroying it when the duplicate IDR insertion fails.
Fixes: 9593804c44c2 ("firmware: arm_scmi: Exclude transport devices from bus matching")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/firmware/arm_scmi/bus.c | 22 +++++++++++++++++-----
drivers/firmware/arm_scmi/common.h | 2 ++
drivers/firmware/arm_scmi/driver.c | 5 ++++-
3 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index e1f66c08c81d..7f06d56e4905 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -200,21 +200,33 @@ scmi_protocol_table_unregister(const struct scmi_device_id *id_table)
scmi_protocol_device_unrequest(entry);
}
-static int scmi_dev_match_by_id_table(struct scmi_device *scmi_dev,
- const struct scmi_device_id *id_table)
+static bool scmi_device_is_transport(const struct scmi_device *scmi_dev)
+{
+ return !strncmp(scmi_dev->name, SCMI_TRANSPORT_DEVNAME_PREFIX,
+ strlen(SCMI_TRANSPORT_DEVNAME_PREFIX));
+}
+
+static int __scmi_dev_match_by_id_table(struct scmi_device *scmi_dev,
+ const struct scmi_device_id *id_table,
+ bool skip_transport)
{
if (!id_table || !id_table->name)
return 0;
- /* Always skip transport devices from matching */
for (; id_table->protocol_id && id_table->name; id_table++)
if (id_table->protocol_id == scmi_dev->protocol_id &&
- strncmp(scmi_dev->name, "__scmi_transport_device", 23) &&
+ !(skip_transport && scmi_device_is_transport(scmi_dev)) &&
!strcmp(id_table->name, scmi_dev->name))
return 1;
return 0;
}
+static int scmi_dev_match_by_id_table(struct scmi_device *scmi_dev,
+ const struct scmi_device_id *id_table)
+{
+ return __scmi_dev_match_by_id_table(scmi_dev, id_table, true);
+}
+
static int scmi_dev_match_id(struct scmi_device *scmi_dev,
const struct scmi_driver *scmi_drv)
{
@@ -234,7 +246,7 @@ static int scmi_match_by_id_table(struct device *dev, const void *data)
struct scmi_device *scmi_dev = to_scmi_dev(dev);
const struct scmi_device_id *id_table = data;
- return scmi_dev_match_by_id_table(scmi_dev, id_table);
+ return __scmi_dev_match_by_id_table(scmi_dev, id_table, false);
}
/* Returns a device_find_child() reference which must be dropped by caller. */
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index b9723c105fc1..fe8c22cfb9f7 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -34,6 +34,8 @@
#define SCMI_SHMEM_MAX_PAYLOAD_SIZE 104
+#define SCMI_TRANSPORT_DEVNAME_PREFIX "__scmi_transport_device"
+
enum scmi_error_codes {
SCMI_SUCCESS = 0, /* Success */
SCMI_ERR_SUPPORT = -1, /* Not supported */
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 532f1f65b2ab..ef29fd223287 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2762,6 +2762,9 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
idx = tx ? 0 : 1;
idr = tx ? &info->tx_idr : &info->rx_idr;
+ if (idr_find(idr, prot_id))
+ return -EEXIST;
+
if (!info->desc->ops->chan_available(of_node, idx)) {
cinfo = idr_find(idr, SCMI_PROTOCOL_BASE);
if (unlikely(!cinfo)) /* Possible only if platform has no Rx */
@@ -2779,7 +2782,7 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
cinfo->no_completion_irq = info->desc->no_completion_irq;
/* Create a unique name for this transport device */
- snprintf(name, 32, "__scmi_transport_device_%s_%02X",
+ snprintf(name, sizeof(name), SCMI_TRANSPORT_DEVNAME_PREFIX "_%s_%02X",
idx ? "rx" : "tx", prot_id);
/* Create a uniquely named, dedicated transport device for this chan */
tdev = scmi_device_create(of_node, info->dev, prot_id, name);
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread