Hi,
This series is a respin of an old series[0] parked for a while waiting for
a required SCMI specification change to be published.
The series, building on top of the SCMI System Power Protocol, adds a new
SCMI driver which, registering for SystemPower notifications, takes care to
satisfy SCMI plaform system-transitions graceful requests like shutdown or
reboot involving userspace interactions as needed.
Interaction with userspace boils down to the same orderly_ Kernel methods
used by ACPI to handle similar shutdown requests.
The latest SCMI v3.1 specification [1], which adds a new timeout field to
the graceful notifications payload, let the platform advertise for how long
it will possibly wait for the requested system state transition to happen
before forcibly enforcing it.
As a part of the series, patch 1/3 enforces, at the SCMI core level, the
creation of one single SCMI SystemPower device, to avoid promoting the
design of systems in which multiple SCMI platforms can advertise the
concurrent support of SystemPower protocol: when multiple SCMI platform
are defined, only one of them should be in charge of SystemPower comms
with the OSPM, so only one such SystemPower device across all platforms
is allowed to be created.
Thanks,
Cristian
[0]:https://lore.kernel.org/linux-arm-kernel/[email protected]/
[1]:https://developer.arm.com/documentation/den0056/d/?lang=en
---
v1 --> v2
- moved checks about single SystemPower device creation from bus into driver
- dropped patch about removing ida_simple_* obsolete calls
Cristian Marussi (4):
firmware: arm_scmi: Support only one single SystemPower device
firmware: arm_scmi: Add SCMIv3.1 SystemPower extensions
firmware: arm_scmi: Add devm_protocol_acquire helper
firmware: arm_scmi: Add SCMI System Power Control driver
drivers/firmware/arm_scmi/Kconfig | 12 +
drivers/firmware/arm_scmi/Makefile | 1 +
drivers/firmware/arm_scmi/bus.c | 1 +
drivers/firmware/arm_scmi/driver.c | 93 ++++-
.../firmware/arm_scmi/scmi_power_control.c | 362 ++++++++++++++++++
drivers/firmware/arm_scmi/system.c | 17 +-
include/linux/scmi_protocol.h | 7 +
7 files changed, 477 insertions(+), 16 deletions(-)
create mode 100644 drivers/firmware/arm_scmi/scmi_power_control.c
--
2.32.0
Add a method to get hold of a protocol, causing it to be initialized and
its resource accounting updated, without getting access to its operations
and handle.
Some protocols, like SCMI SystemPower, do not expose any protocol ops to
the Kernel OSPM agent but still need to be at least initialized: this
helper avoids the need to invoke a full devm_get_protocol() only to get
the protocol initialized while throwing away unused the protocol ops and
handle.
Signed-off-by: Cristian Marussi <[email protected]>
---
drivers/firmware/arm_scmi/driver.c | 70 +++++++++++++++++++++++-------
include/linux/scmi_protocol.h | 5 +++
2 files changed, 60 insertions(+), 15 deletions(-)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 48392a8a8dcd..38600e320320 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -1667,6 +1667,30 @@ static void scmi_devm_release_protocol(struct device *dev, void *res)
scmi_protocol_release(dres->handle, dres->protocol_id);
}
+static struct scmi_protocol_instance __must_check *
+scmi_devres_protocol_instance_get(struct scmi_device *sdev, u8 protocol_id)
+{
+ struct scmi_protocol_instance *pi;
+ struct scmi_protocol_devres *dres;
+
+ dres = devres_alloc(scmi_devm_release_protocol,
+ sizeof(*dres), GFP_KERNEL);
+ if (!dres)
+ return ERR_PTR(-ENOMEM);
+
+ pi = scmi_get_protocol_instance(sdev->handle, protocol_id);
+ if (IS_ERR(pi)) {
+ devres_free(dres);
+ return pi;
+ }
+
+ dres->handle = sdev->handle;
+ dres->protocol_id = protocol_id;
+ devres_add(&sdev->dev, dres);
+
+ return pi;
+}
+
/**
* scmi_devm_protocol_get - Devres managed get protocol operations and handle
* @sdev: A reference to an scmi_device whose embedded struct device is to
@@ -1690,32 +1714,47 @@ scmi_devm_protocol_get(struct scmi_device *sdev, u8 protocol_id,
struct scmi_protocol_handle **ph)
{
struct scmi_protocol_instance *pi;
- struct scmi_protocol_devres *dres;
- struct scmi_handle *handle = sdev->handle;
if (!ph)
return ERR_PTR(-EINVAL);
- dres = devres_alloc(scmi_devm_release_protocol,
- sizeof(*dres), GFP_KERNEL);
- if (!dres)
- return ERR_PTR(-ENOMEM);
-
- pi = scmi_get_protocol_instance(handle, protocol_id);
- if (IS_ERR(pi)) {
- devres_free(dres);
+ pi = scmi_devres_protocol_instance_get(sdev, protocol_id);
+ if (IS_ERR(pi))
return pi;
- }
-
- dres->handle = handle;
- dres->protocol_id = protocol_id;
- devres_add(&sdev->dev, dres);
*ph = &pi->ph;
return pi->proto->ops;
}
+/**
+ * scmi_devm_protocol_acquire - Devres managed helper to get hold of a protocol
+ * @sdev: A reference to an scmi_device whose embedded struct device is to
+ * be used for devres accounting.
+ * @protocol_id: The protocol being requested.
+ *
+ * Get hold of a protocol accounting for its usage, possibly triggering its
+ * initialization but without getting access to its protocol specific operations
+ * and handle.
+ *
+ * Being a devres based managed method, protocol hold will be automatically
+ * released, and possibly de-initialized on last user, once the SCMI driver
+ * owning the scmi_device is unbound from it.
+ *
+ * Return: 0 on SUCCESS
+ */
+static int __must_check scmi_devm_protocol_acquire(struct scmi_device *sdev,
+ u8 protocol_id)
+{
+ struct scmi_protocol_instance *pi;
+
+ pi = scmi_devres_protocol_instance_get(sdev, protocol_id);
+ if (IS_ERR(pi))
+ return PTR_ERR(pi);
+
+ return 0;
+}
+
static int scmi_devm_protocol_match(struct device *dev, void *res, void *data)
{
struct scmi_protocol_devres *dres = res;
@@ -2320,6 +2359,7 @@ static int scmi_probe(struct platform_device *pdev)
handle = &info->handle;
handle->dev = info->dev;
handle->version = &info->version;
+ handle->devm_protocol_acquire = scmi_devm_protocol_acquire;
handle->devm_protocol_get = scmi_devm_protocol_get;
handle->devm_protocol_put = scmi_devm_protocol_put;
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 10b9c0e9fdf5..7f4f9df1b20f 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -734,6 +734,9 @@ struct scmi_notify_ops {
*
* @dev: pointer to the SCMI device
* @version: pointer to the structure containing SCMI version information
+ * @devm_protocol_acquire: devres managed method to get hold of a protocol,
+ * causing its initialization and related resource
+ * accounting
* @devm_protocol_get: devres managed method to acquire a protocol and get specific
* operations and a dedicated protocol handler
* @devm_protocol_put: devres managed method to release a protocol
@@ -752,6 +755,8 @@ struct scmi_handle {
struct device *dev;
struct scmi_revision_info *version;
+ int __must_check (*devm_protocol_acquire)(struct scmi_device *sdev,
+ u8 proto);
const void __must_check *
(*devm_protocol_get)(struct scmi_device *sdev, u8 proto,
struct scmi_protocol_handle **ph);
--
2.32.0
In order to minimize SCMI platform fw-side complexity, only one single SCMI
platform should be in charge of SCMI SystemPower protocol communications
with the OSPM: enforce the existence of one single unique device associated
with SystemPower protocol across any possible number of SCMI platforms, and
warn if a system tries to register different SystemPower devices from
multiple platforms.
Signed-off-by: Cristian Marussi <[email protected]>
---
v1 --> v2
- move single device checks from bus to driver
---
drivers/firmware/arm_scmi/bus.c | 1 +
drivers/firmware/arm_scmi/driver.c | 23 +++++++++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index f6fe723ab869..8960fb5277db 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -201,6 +201,7 @@ scmi_device_create(struct device_node *np, struct device *parent, int protocol,
goto put_dev;
return scmi_dev;
+
put_dev:
kfree_const(scmi_dev->name);
put_device(&scmi_dev->dev);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 00b7f2aff4ec..48392a8a8dcd 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -61,6 +61,11 @@ static atomic_t transfer_last_id;
static DEFINE_IDR(scmi_requested_devices);
static DEFINE_MUTEX(scmi_requested_devices_mtx);
+/* Track globally the creation of SCMI SystemPower related devices */
+static bool scmi_syspower_registered;
+/* Protect access to scmi_syspower_registered */
+static DEFINE_MUTEX(scmi_syspower_mtx);
+
struct scmi_requested_dev {
const struct scmi_device_id *id_table;
struct list_head node;
@@ -2014,21 +2019,39 @@ scmi_get_protocol_device(struct device_node *np, struct scmi_info *info,
if (sdev)
return sdev;
+ mutex_lock(&scmi_syspower_mtx);
+ if (prot_id == SCMI_PROTOCOL_SYSTEM && scmi_syspower_registered) {
+ dev_warn(info->dev,
+ "SCMI SystemPower protocol device must be unique !\n");
+ mutex_unlock(&scmi_syspower_mtx);
+
+ return NULL;
+ }
+
pr_debug("Creating SCMI device (%s) for protocol %x\n", name, prot_id);
sdev = scmi_device_create(np, info->dev, prot_id, name);
if (!sdev) {
dev_err(info->dev, "failed to create %d protocol device\n",
prot_id);
+ mutex_unlock(&scmi_syspower_mtx);
+
return NULL;
}
if (scmi_txrx_setup(info, &sdev->dev, prot_id)) {
dev_err(&sdev->dev, "failed to setup transport\n");
scmi_device_destroy(sdev);
+ mutex_unlock(&scmi_syspower_mtx);
+
return NULL;
}
+ if (prot_id == SCMI_PROTOCOL_SYSTEM)
+ scmi_syspower_registered = true;
+
+ mutex_unlock(&scmi_syspower_mtx);
+
return sdev;
}
--
2.32.0
On Mon, 4 Jul 2022 11:19:29 +0100, Cristian Marussi wrote:
> This series is a respin of an old series[0] parked for a while waiting for
> a required SCMI specification change to be published.
>
> The series, building on top of the SCMI System Power Protocol, adds a new
> SCMI driver which, registering for SystemPower notifications, takes care to
> satisfy SCMI plaform system-transitions graceful requests like shutdown or
> reboot involving userspace interactions as needed.
>
> [...]
Applied to sudeep.holla/linux (for-next/scmi), thanks!
[1/4] firmware: arm_scmi: Support only one single SystemPower device
https://git.kernel.org/sudeep.holla/c/a0db3962fb
[2/4] firmware: arm_scmi: Add SCMIv3.1 SystemPower extensions
https://git.kernel.org/sudeep.holla/c/7097f29819
[3/4] firmware: arm_scmi: Add devm_protocol_acquire helper
https://git.kernel.org/sudeep.holla/c/d91079995f
[4/4] firmware: arm_scmi: Add SCMI System Power Control driver
https://git.kernel.org/sudeep.holla/c/2c4b97fee9
--
Regards,
Sudeep