Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Rework SCMI transport drivers probing sequence
@ 2026-05-10 16:05 Cristian Marussi
  2026-05-10 16:05 ` [PATCH v2 1/4] firmware: arm_scmi: Add transport instance handles Cristian Marussi
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Cristian Marussi @ 2026-05-10 16:05 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, arm-scmi
  Cc: sudeep.holla, philip.radford, james.quinlan, f.fainelli,
	vincent.guittot, etienne.carriere, peng.fan, michal.simek,
	gatien.chevallier, u.kleine-koenig, dakr, Cristian Marussi

Hi,

when the SCMI transports were split out into standalone drivers [1] the
probe sequence was laid out in such a way that:

 - the transport drivers would have probed first, triggered by the firmware
   driven discovery process (DT/ACPI)

 - afterwards the control would have been passed to the core SCMI stack
   driver via the creation of a dedicated device that would have inherited
   the original firmware descriptor (since that same DT/ACPI node would
   have been still needed by the SCMI core driver to be parsed)

The tricky part came around with some transport driver like virtio and
optee since they are, in turn, upfront dependent on an external distinct
kernel subsystem; IOW these have first to undergo their own subsystem
specific probe/initialization to become fully operational as transports:
this kind of initialization sequencing of course must deal with the
possibility of probe deferrals BUT at that time we avoided this by using
the trick in virtio/optee transports to register the next stage drivers
ONLY at the end of the subsystem specific probe routine, from within the
probe itself.

This register_while_probing behaviour is ugly and has 2 main issues:

 - it is frowned upon and can lead to hangs in the driver core whenever
   some core locking is changed as exposed in [2][3]

 - it limits these transport drivers to a single instance probing since of
   course you cannot register the same driver more than once

Note also that such dependencies are NOT explicitly represented in any way
within the firmware description tables: i.e. we cannot play the fw_devlink
card and enjoy correct sequencing out of the box.

With this series we remove the ugly register_while_probing trick and
introduce some basic mechanism to allow the probe to be deferred until the
underlying transport has probed and it is fully operational so as that can
be used as a supplier device. This is obtained by:

 - moving the problematic platform driver registration away from the probe
   into its own module_init/exit

 - adding a few common well-known optional helpers that can be invoked
   to retrieve the supplier reference, if ready, OR defer the probe when
   neeeded.

Instead, we do NOT introduce in this series (as we attempted in the RFC)
a mechanism to support multiple instance probing also for optee and virtio
transports, because there is currently NO possible way to bind such probed
transport driver instances to the related SCMI instances, so that this
would narrow down the applicability of this multiple instance scenario to
the case in which each underlying SCMI server instance is setup in exactly
the same way. (same protocols for each instance node)

Based on v7.1-rc2

Tested on:
 - an emulated environment against a mock SCMI Server (virtio)
 - a QEMU based setup against SCP-based server running in OPTEE (optee)
 - JUNO with the standard SCP reference firmware (mailbox)
 - RADXA ROCK_5B with Rockchip fw (smc) 

Thanks,
Cristian

[1]: https://lore.kernel.org/arm-scmi/20240812173340.3912830-1-cristian.marussi@arm.com/
[2]: https://lore.kernel.org/lkml/aaA6t-J2gRy3dE1_@pluto/
[3]: https://lore.kernel.org/all/ad9cglZCwtsVsGmq@monoceros/

---
v1 -> v2
 - fixed __MUTEX_INITILIAZER() usage
 - reworked supplier state machine
 - introduce common transport_supplier logic
 - get rid of init/cleanup wrappers
 - use common generic supplier definitions in virtio/optee
 - optee: use proper barrier on scmi_optee_agent
 - virtio: fixed possible race between supplier made available
   and scmi_dev made visible
 - virtio: restored initial virtio_device_ready() logic
 - virtio: issue a proper reset device on probe failure

Cristian Marussi (4):
  firmware: arm_scmi: Add transport instance handles
  firmware: arm_scmi: Add a generic transport supplier
  firmware: arm_scmi: virtio: Rework transport probe sequence
  firmware: arm_scmi: optee: Rework transport probe sequence

 drivers/firmware/arm_scmi/common.h            | 163 +++++++++++++++++-
 drivers/firmware/arm_scmi/transports/optee.c  |  46 +++--
 drivers/firmware/arm_scmi/transports/virtio.c |  52 +++++-
 3 files changed, 238 insertions(+), 23 deletions(-)

-- 
2.53.0



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/4] firmware: arm_scmi: Add transport instance handles
  2026-05-10 16:05 [PATCH v2 0/4] Rework SCMI transport drivers probing sequence Cristian Marussi
@ 2026-05-10 16:05 ` Cristian Marussi
  2026-05-10 16:05 ` [PATCH v2 2/4] firmware: arm_scmi: Add a generic transport supplier Cristian Marussi
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Cristian Marussi @ 2026-05-10 16:05 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, arm-scmi
  Cc: sudeep.holla, philip.radford, james.quinlan, f.fainelli,
	vincent.guittot, etienne.carriere, peng.fan, michal.simek,
	gatien.chevallier, u.kleine-koenig, dakr, Cristian Marussi

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 <cristian.marussi@arm.com>
---
v1 -> v2
 - saving *th handle into scmi_transport to simplify freeing path
 - add a return value to supplier_put
---
 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 7c9617d080a0..af6f9f498e14 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -17,6 +17,7 @@
 #include <linux/hashtable.h>
 #include <linux/list.h>
 #include <linux/module.h>
+#include <linux/property.h>
 #include <linux/refcount.h>
 #include <linux/scmi_protocol.h>
 #include <linux/spinlock.h>
@@ -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;							       \
 }									       \
 									       \
-- 
2.53.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/4] firmware: arm_scmi: Add a generic transport supplier
  2026-05-10 16:05 [PATCH v2 0/4] Rework SCMI transport drivers probing sequence Cristian Marussi
  2026-05-10 16:05 ` [PATCH v2 1/4] firmware: arm_scmi: Add transport instance handles Cristian Marussi
@ 2026-05-10 16:05 ` Cristian Marussi
  2026-05-10 16:05 ` [PATCH v2 3/4] firmware: arm_scmi: virtio: Rework transport probe sequence Cristian Marussi
  2026-05-10 16:05 ` [PATCH v2 4/4] firmware: arm_scmi: optee: " Cristian Marussi
  3 siblings, 0 replies; 5+ messages in thread
From: Cristian Marussi @ 2026-05-10 16:05 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, arm-scmi
  Cc: sudeep.holla, philip.radford, james.quinlan, f.fainelli,
	vincent.guittot, etienne.carriere, peng.fan, michal.simek,
	gatien.chevallier, u.kleine-koenig, dakr, Cristian Marussi

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 <cristian.marussi@arm.com>
---
 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 af6f9f498e14..e2885173594a 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 - <supplier_dev>: 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)				       \
 {									       \
-- 
2.53.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 3/4] firmware: arm_scmi: virtio: Rework transport probe sequence
  2026-05-10 16:05 [PATCH v2 0/4] Rework SCMI transport drivers probing sequence Cristian Marussi
  2026-05-10 16:05 ` [PATCH v2 1/4] firmware: arm_scmi: Add transport instance handles Cristian Marussi
  2026-05-10 16:05 ` [PATCH v2 2/4] firmware: arm_scmi: Add a generic transport supplier Cristian Marussi
@ 2026-05-10 16:05 ` Cristian Marussi
  2026-05-10 16:05 ` [PATCH v2 4/4] firmware: arm_scmi: optee: " Cristian Marussi
  3 siblings, 0 replies; 5+ messages in thread
From: Cristian Marussi @ 2026-05-10 16:05 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, arm-scmi
  Cc: sudeep.holla, philip.radford, james.quinlan, f.fainelli,
	vincent.guittot, etienne.carriere, peng.fan, michal.simek,
	gatien.chevallier, u.kleine-koenig, dakr, Cristian Marussi

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 <cristian.marussi@arm.com>
---
v1 -> v2
 - fixed __MUTEX_INITILIAZER() usage
 - use common generic supplier definitions
 - get rid of init/cleanup wrappers
 - fixed possible race between supplier made available and scmi_dev
   made visible
 - restored initial virtio_device_ready() logic, taking care to reset
   device on failure too
---
 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 <igor.skalkin@opensynergy.com>");
 MODULE_AUTHOR("Peter Hilber <peter.hilber@opensynergy.com>");
-- 
2.53.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 4/4] firmware: arm_scmi: optee: Rework transport probe sequence
  2026-05-10 16:05 [PATCH v2 0/4] Rework SCMI transport drivers probing sequence Cristian Marussi
                   ` (2 preceding siblings ...)
  2026-05-10 16:05 ` [PATCH v2 3/4] firmware: arm_scmi: virtio: Rework transport probe sequence Cristian Marussi
@ 2026-05-10 16:05 ` Cristian Marussi
  3 siblings, 0 replies; 5+ messages in thread
From: Cristian Marussi @ 2026-05-10 16:05 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, arm-scmi
  Cc: sudeep.holla, philip.radford, james.quinlan, f.fainelli,
	vincent.guittot, etienne.carriere, peng.fan, michal.simek,
	gatien.chevallier, u.kleine-koenig, dakr, Cristian Marussi

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 <cristian.marussi@arm.com>
---
v1 -> v2
 - fixed __MUTEX_INITILIAZER() usage
 - use common generic supplier definitions
 - get rid of init/cleanup wrappers
 - use proper barrier on scmi_optee_agent
---
 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 <etienne.carriere@foss.st.com>");
 MODULE_DESCRIPTION("SCMI OPTEE Transport driver");
-- 
2.53.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-05-10 16:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-10 16:05 [PATCH v2 0/4] Rework SCMI transport drivers probing sequence Cristian Marussi
2026-05-10 16:05 ` [PATCH v2 1/4] firmware: arm_scmi: Add transport instance handles Cristian Marussi
2026-05-10 16:05 ` [PATCH v2 2/4] firmware: arm_scmi: Add a generic transport supplier Cristian Marussi
2026-05-10 16:05 ` [PATCH v2 3/4] firmware: arm_scmi: virtio: Rework transport probe sequence Cristian Marussi
2026-05-10 16:05 ` [PATCH v2 4/4] firmware: arm_scmi: optee: " Cristian Marussi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox