firmware: arm_scmi: Clear SystemPower flag on create failure

__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: 2c3e674465 ("firmware: arm_scmi: Refactor device create/destroy helpers")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://patch.msgid.link/20260714-scmi_core_fixes-v6-11-3afe499d46e3@kernel.org
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
This commit is contained in:
Sudeep Holla 2026-07-14 13:56:30 +01:00
parent e3a5c30d23
commit e4c16ae24c

View File

@ -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;
/*
* 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)
*/
if (protocol == SCMI_PROTOCOL_SYSTEM &&
atomic_cmpxchg(&scmi_syspower_registered, 0, 1)) {
dev_warn(parent,
"SCMI SystemPower protocol device must be unique !\n");
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) {
scmi_dev->protocol_id = protocol;
/*
* 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 (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->name = kstrdup_const(name ?: "unknown", GFP_KERNEL);
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 *