[PATCH 0/8] firmware: arm_scmi: Refactoring and enablement of ACPI PCC transport

[PATCH 0/8] firmware: arm_scmi: Refactoring and enablement of ACPI PCC transport

[Sudeep Holla]

The SCMI can be utilized in systems using either the FDT or ACPI specification.
While FDT-based systems can natively use SCMI, ACPI-based systems often
need to abstract the functionality provided by SCMI under ASL methods.
So far, there has been no need to support SCMI natively on ACPI systems.

However, with the addition of a few new protocols such as Powercap and Telemetry,
which lack abstractions in the ACPI specification, there is now a need to
run SCMI natively for those use cases.

This patch series introduces ACPI PCC transport support for the Arm SCMI
framework, alongside several foundational refactors and enhancements to
achieve firmware-node neutrality between Device Tree (DT) and ACPI systems.

The key changes include:

1. ACPI/DT abstraction and fwnode transition

   Converted the core SCMI code to use `fwnode_handle` instead of DT-specific
   structures, ensuring seamless operation across both ACPI and DT
   environments. All property lookups, child enumeration, and device
   association paths have been updated accordingly.

2. Unified transport registration for ACPI and DT

   Extended the SCMI transport driver macros to support ACPI match tables,
   enabling transports to probe using ACPI device IDs while maintaining
   backward compatibility with DT-only systems.

3. Protocol device initialization and refactoring

   Refactored the protocol device creation and validation logic into a new
   helper for improved readability and maintainability. Enhanced the
   initialization logic to handle ACPI-based SCMI devices without explicit
   child fwnodes.

4. Introduction of ACPI PCC transport

   Added a new SCMI transport driver leveraging ACPI PCCT (Platform
   Communications Channel Table) subspaces via the Linux PCC mailbox
   framework. This enables SCMI communication over PCC on ACPI-based
   platforms.

Collectively, these changes lay the groundwork for robust SCMI operation on
ACPI platforms, achieving near parity with DT systems where applicable,
while enabling the new PCC transport path for firmware communication.

Note: SCMI will not be run natively for all existing protocols other
than Powercap and Telemetry, as they must continue to use ACPI abstractions.
The ACPI SCMI device document is not hosted publicly yet, but the ACPI
namespace snippet below provides a reference example for reviewing these patches.

There is one dependency on the SCMI Telemetry support posted by Cristian [1].
It is required only for the definition of `SCMI_PROTOCOL_TELEMETRY` in the
`scmi_std_protocol` enumeration.

-->8

Device (SCM0) {
  Name(_HID, "ARML0001")
  Name (_UID, 0)
  Name (_DSD, Package () {
    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),  // Device Properties UUID
      Package () {
        Package(2) {
          "arm-arml0001-transport-pcc", // Key
          Package () {               // Value
              1,                     // Revision
              3,                     // Count
              Package(){4, 0, 1},    // PCCT Idx, TransportUID, A2P Common
              Package(){7, 1, 0},    // PCCT Idx, TransportUID, A2P Exclusive
              Package(){9, 2, 2}     // PCCT Idx, TransportUID, P2A Exclusive
          } // Value
        },
        Package(2) {
          "arm-arml0001-protocol-pcap",   // Key
          Package () {                // Value
             1,                       // Revision
             Package() {              // Protocol Exclusive Transport Package
                                      // Empty; A2P Common
             },
             Package() {}             // Protocol Association Package
          } // Value
        },
        Package(2) {
          "arm-arml0001-protocol-telemetry", // Key
          Package () {                // Value
            1,                        // Revision
            Package() {               // Protocol Exclusive Transport Package
              Package (2) {1, 0},     // UID=1; Flags=0; A2P Exclusive
              Package (2) {2, 0}      // UID=2; Flags=0; P2A Exclusive
            },
            Package() {}              // Protocol Association Package
          } // Value
        },
      } // Device Properties
    }) // _DSD
} // SCM0

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
Sudeep Holla (8):
      firmware: arm_scmi: Set fwnode for the genrated SCMI platform device
      firmware: arm_scmi: Extend transport driver macro to support ACPI
      firmware: arm_scmi: Convert OF-only paths to generic fwnode in SCMI core
      firmware: arm_scmi: Fall back to ACPI HID when "compatible" is absent
      firmware: arm_scmi: Pass protocol ID to chan_available() transport callback
      firmware: arm_scmi: Refactor protocol device creation logic
      firmware: arm_scmi: transport: Add ACPI PCC transport
      firmware: arm_scmi: Initialise all protocol devices and transport channels

 drivers/firmware/arm_scmi/bus.c                |  33 +--
 drivers/firmware/arm_scmi/common.h             |  33 ++-
 drivers/firmware/arm_scmi/driver.c             | 173 ++++++-----
 drivers/firmware/arm_scmi/transports/Kconfig   |  12 +
 drivers/firmware/arm_scmi/transports/Makefile  |   2 +
 drivers/firmware/arm_scmi/transports/mailbox.c |   4 +-
 drivers/firmware/arm_scmi/transports/optee.c   |   4 +-
 drivers/firmware/arm_scmi/transports/pcc.c     | 390 +++++++++++++++++++++++++
 drivers/firmware/arm_scmi/transports/smc.c     |   4 +-
 drivers/firmware/arm_scmi/transports/virtio.c  |   3 +-
 10 files changed, 561 insertions(+), 97 deletions(-)
---
base-commit: 7ea30958b3054f5e488fa0b33c352723f7ab3a2a
change-id: 20251017-acpi_scmi_pcc-e44e1233eae3


-- 
Regards,
Sudeep

[PATCH 1/8] firmware: arm_scmi: Set fwnode for the genrated SCMI platform device

[Sudeep Holla]

Add a call to device_set_node() in the SCMI probe helper to associate
generated SCMI platform device with the firmware node of its supplier
transport device.

This complements device_set_of_node_from_dev() and ensures that
firmware node information is propagated correctly for both Device Tree
and non-DT (e.g. ACPI) based systems.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scmi/common.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 07b9e629276d..911941e6885d 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -473,6 +473,7 @@ static int __tag##_probe(struct platform_device *pdev)			       \
 		return -ENOMEM;						       \
 									       \
 	device_set_of_node_from_dev(&spdev->dev, dev);			       \
+	device_set_node(&spdev->dev, dev_fwnode(dev));			       \
 									       \
 	strans.supplier = dev;						       \
 	memcpy(&strans.desc, &(__desc), sizeof(strans.desc));		       \

-- 
2.34.1

[PATCH 2/8] firmware: arm_scmi: Extend transport driver macro to support ACPI

[Sudeep Holla]

Extend the SCMI transport driver helper to also support ACPI-based
systems. Introduce an internal helper macro that accepts both OF and
ACPI match tables, and expose two wrappers:

  - DEFINE_SCMI_TRANSPORT_DRIVER(...)      -> DT/OF wrapper (unchanged ABI)
  - DEFINE_SCMI_ACPI_TRANSPORT_DRIVER(...) -> ACPI wrapper

For ACPI, the generated platform_driver now sets .acpi_match_table via
ACPI_PTR() so that builds without CONFIG_ACPI remain safe (becomes NULL).
The spawned platform_device inherits the parent device’s ACPI companion
to ensure proper firmware-node association (i.e.subsequent lookups or
fwnode use see the correct firmware node).

This keeps existing DT users unchanged and continues to function as
expected while allowing transports to be probed using the structure
`acpi_device_id` tables on ACPI platforms.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scmi/common.h | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 911941e6885d..d038fec72360 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -9,6 +9,7 @@
 #ifndef _SCMI_COMMON_H
 #define _SCMI_COMMON_H
 
+#include <linux/acpi.h>
 #include <linux/bitfield.h>
 #include <linux/completion.h>
 #include <linux/device.h>
@@ -453,7 +454,8 @@ struct scmi_transport {
 	struct scmi_transport_core_operations **core_ops;
 };
 
-#define DEFINE_SCMI_TRANSPORT_DRIVER(__tag, __drv, __desc, __match, __core_ops)\
+#define __DEFINE_SCMI_TRANSPORT_DRIVER(__tag, __drv, __desc, __of_match,       \
+					__acpi_match, __core_ops)	       \
 static void __tag##_dev_free(void *data)				       \
 {									       \
 	struct platform_device *spdev = data;				       \
@@ -474,6 +476,7 @@ static int __tag##_probe(struct platform_device *pdev)			       \
 									       \
 	device_set_of_node_from_dev(&spdev->dev, dev);			       \
 	device_set_node(&spdev->dev, dev_fwnode(dev));			       \
+	ACPI_COMPANION_SET(&spdev->dev, ACPI_COMPANION(dev));		       \
 									       \
 	strans.supplier = dev;						       \
 	memcpy(&strans.desc, &(__desc), sizeof(strans.desc));		       \
@@ -498,11 +501,18 @@ err:									       \
 static struct platform_driver __drv = {					       \
 	.driver = {							       \
 		   .name = #__tag "_transport",				       \
-		   .of_match_table = __match,				       \
+		   .of_match_table = __of_match,			       \
+		   .acpi_match_table = ACPI_PTR(__acpi_match),		       \
 		   },							       \
 	.probe = __tag##_probe,						       \
 }
 
+#define DEFINE_SCMI_TRANSPORT_DRIVER(__tag, __drv, __desc, __match, __core_ops)\
+	__DEFINE_SCMI_TRANSPORT_DRIVER(__tag, __drv, __desc, __match, NULL, __core_ops)
+
+#define DEFINE_SCMI_ACPI_TRANSPORT_DRIVER(__tag, __drv, __desc, __match, __core_ops)\
+	__DEFINE_SCMI_TRANSPORT_DRIVER(__tag, __drv, __desc, NULL, __match, __core_ops)
+
 void scmi_notification_instance_data_set(const struct scmi_handle *handle,
 					 void *priv);
 void *scmi_notification_instance_data_get(const struct scmi_handle *handle);

-- 
2.34.1

[PATCH 3/8] firmware: arm_scmi: Convert OF-only paths to generic fwnode in SCMI core

[Sudeep Holla]

Switch SCMI core plumbing from struct device_node* to struct
fwnode_handle* so transports and core code work with both DT and
ACPI firmware descriptions.

This change:
  - Replaces of_* property lookups with fwnode_property_*() helpers.
  - Switches child enumeration to
    fwnode_for_each_available_child_node_scoped().
  - Plumbs fwnode through the SCMI device creation and channel setup
    paths and updates transport ->chan_available() signatures to take a
    fwnode.
  - Stores the per-protocol child fwnodes in info->active_protocols so
    the core can later locate the descriptor for a given protocol ID.
  - Update mailbox/optee/smc/virtio transports to accept fwnode and
    map to OF nodes where needed

DT-only transports (mailbox/optee/smc) still parse DT properties by
mapping the fwnode back to an OF node; on non-DT (e.g. ACPI) systems
these transports will report no channel available.

This refactor is a prerequisite for adding an ACPI-first transport like
PCC and brings the SCMI core closer to DT/ACPI parity. This is a mechanical
step towards firmware-node neutrality; DT users continue to work unchanged,
and ACPI paths can be enabled on top.

No functional change is expected on DT platforms; ACPI platforms can now
discover and participate in SCMI where a suitable transport is present.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scmi/bus.c                |  33 ++++----
 drivers/firmware/arm_scmi/common.h             |   6 +-
 drivers/firmware/arm_scmi/driver.c             | 104 ++++++++++++-------------
 drivers/firmware/arm_scmi/transports/mailbox.c |   3 +-
 drivers/firmware/arm_scmi/transports/optee.c   |   3 +-
 drivers/firmware/arm_scmi/transports/smc.c     |   3 +-
 drivers/firmware/arm_scmi/transports/virtio.c  |   2 +-
 7 files changed, 78 insertions(+), 76 deletions(-)

diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index c7698cfaa4e8..a20f2ac65a65 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -401,10 +401,9 @@ static void scmi_device_release(struct device *dev)
 
 static void __scmi_device_destroy(struct scmi_device *scmi_dev)
 {
-	pr_debug("(%pOF) Destroying SCMI device '%s' for protocol 0x%x (%s)\n",
-		 scmi_dev->dev.parent->of_node,
-		 dev_name(&scmi_dev->dev), scmi_dev->protocol_id,
-		 scmi_dev->name);
+	pr_debug("(%pfwf) Destroying SCMI device '%s' for protocol 0x%x (%s)\n",
+		 dev_fwnode(&scmi_dev->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);
@@ -414,7 +413,7 @@ static void __scmi_device_destroy(struct scmi_device *scmi_dev)
 }
 
 static struct scmi_device *
-__scmi_device_create(struct device_node *np, struct device *parent,
+__scmi_device_create(struct fwnode_handle *fwnode, struct device *parent,
 		     int protocol, const char *name)
 {
 	int id, retval;
@@ -424,7 +423,7 @@ __scmi_device_create(struct device_node *np, struct device *parent,
 	 * If the same protocol/name device already exist under the same parent
 	 * (i.e. SCMI instance) just return the existent device.
 	 * This avoids any race between the SCMI driver, creating devices for
-	 * each DT defined protocol at probe time, and the concurrent
+	 * each fwnode defined protocol at probe time, and the concurrent
 	 * registration of SCMI drivers.
 	 */
 	scmi_dev = scmi_child_dev_find(parent, protocol, name);
@@ -465,7 +464,7 @@ __scmi_device_create(struct device_node *np, struct device *parent,
 	scmi_dev->id = id;
 	scmi_dev->protocol_id = protocol;
 	scmi_dev->dev.parent = parent;
-	device_set_node(&scmi_dev->dev, of_fwnode_handle(np));
+	device_set_node(&scmi_dev->dev, fwnode);
 	scmi_dev->dev.bus = &scmi_bus_type;
 	scmi_dev->dev.release = scmi_device_release;
 	dev_set_name(&scmi_dev->dev, "scmi_dev.%d", id);
@@ -474,8 +473,8 @@ __scmi_device_create(struct device_node *np, struct device *parent,
 	if (retval)
 		goto put_dev;
 
-	pr_debug("(%pOF) Created SCMI device '%s' for protocol 0x%x (%s)\n",
-		 parent->of_node, dev_name(&scmi_dev->dev), protocol, name);
+	pr_debug("(%pfwf) Created SCMI device '%s' - protocol 0x%x (%s)\n",
+		 fwnode, dev_name(&scmi_dev->dev), protocol, name);
 
 	return scmi_dev;
 put_dev:
@@ -485,15 +484,15 @@ __scmi_device_create(struct device_node *np, struct device *parent,
 }
 
 static struct scmi_device *
-_scmi_device_create(struct device_node *np, struct device *parent,
+_scmi_device_create(struct fwnode_handle *fwnode, struct device *parent,
 		    int protocol, const char *name)
 {
 	struct scmi_device *sdev;
 
-	sdev = __scmi_device_create(np, parent, protocol, name);
+	sdev = __scmi_device_create(fwnode, parent, protocol, name);
 	if (!sdev)
-		pr_err("(%pOF) Failed to create device for protocol 0x%x (%s)\n",
-		       parent->of_node, protocol, name);
+		pr_err("(%pfwf) Failed to create device - protocol 0x%x (%s)\n",
+		       fwnode, protocol, name);
 
 	return sdev;
 }
@@ -501,7 +500,7 @@ _scmi_device_create(struct device_node *np, struct device *parent,
 /**
  * scmi_device_create  - A method to create one or more SCMI devices
  *
- * @np: A reference to the device node to use for the new device(s)
+ * @fwnode: A reference to the device node to use for the new device(s)
  * @parent: The parent device to use identifying a specific SCMI instance
  * @protocol: The SCMI protocol to be associated with this device
  * @name: The requested-name of the device to be created; this is optional
@@ -521,7 +520,7 @@ _scmi_device_create(struct device_node *np, struct device *parent,
  *	   could have been potentially created for a whole protocol, unless no
  *	   device was found to have been requested for that specific protocol.
  */
-struct scmi_device *scmi_device_create(struct device_node *np,
+struct scmi_device *scmi_device_create(struct fwnode_handle *fwnode,
 				       struct device *parent, int protocol,
 				       const char *name)
 {
@@ -530,7 +529,7 @@ struct scmi_device *scmi_device_create(struct device_node *np,
 	struct scmi_device *scmi_dev = NULL;
 
 	if (name)
-		return _scmi_device_create(np, parent, protocol, name);
+		return _scmi_device_create(fwnode, parent, protocol, name);
 
 	mutex_lock(&scmi_requested_devices_mtx);
 	phead = idr_find(&scmi_requested_devices, protocol);
@@ -544,7 +543,7 @@ struct scmi_device *scmi_device_create(struct device_node *np,
 	list_for_each_entry(rdev, phead, node) {
 		struct scmi_device *sdev;
 
-		sdev = _scmi_device_create(np, parent,
+		sdev = _scmi_device_create(fwnode, parent,
 					   rdev->id_table->protocol_id,
 					   rdev->id_table->name);
 		if (sdev)
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index d038fec72360..4e30283036e5 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -150,7 +150,7 @@ extern const struct bus_type scmi_bus_type;
 #define SCMI_BUS_NOTIFY_DEVICE_UNREQUEST	1
 extern struct blocking_notifier_head scmi_requested_devices_nh;
 
-struct scmi_device *scmi_device_create(struct device_node *np,
+struct scmi_device *scmi_device_create(struct fwnode_handle *fwnode,
 				       struct device *parent, int protocol,
 				       const char *name);
 void scmi_device_destroy(struct device *parent, int protocol, const char *name);
@@ -204,7 +204,7 @@ struct scmi_chan_info {
  * @poll_done: Callback to poll transfer status
  */
 struct scmi_transport_ops {
-	bool (*chan_available)(struct device_node *of_node, int idx);
+	bool (*chan_available)(struct fwnode_handle *fwnode, int idx);
 	int (*chan_setup)(struct scmi_chan_info *cinfo, struct device *dev,
 			  bool tx);
 	int (*chan_free)(int id, void *p, void *data);
@@ -230,7 +230,7 @@ struct scmi_transport_ops {
  *	be pending simultaneously in the system. May be overridden by the
  *	get_max_msg op.
  * @max_msg_size: Maximum size of data payload per message that can be handled.
- * @atomic_threshold: Optional system wide DT-configured threshold, expressed
+ * @atomic_threshold: Optional system wide fwnode-configured threshold, expressed
  *		      in microseconds, for atomic operations.
  *		      Only SCMI synchronous commands reported by the platform
  *		      to have an execution latency lesser-equal to the threshold
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index bd56a877fdfc..bc5fea11b5db 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -29,9 +29,9 @@
 #include <linux/hashtable.h>
 #include <linux/list.h>
 #include <linux/module.h>
-#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/processor.h>
+#include <linux/property.h>
 #include <linux/refcount.h>
 #include <linux/slab.h>
 #include <linux/xarray.h>
@@ -100,7 +100,7 @@ struct scmi_xfers_info {
  *	initialization code to identify this instance.
  *
  * Each protocol is initialized independently once for each SCMI platform in
- * which is defined by DT and implemented by the SCMI server fw.
+ * which is defined by fwnode and implemented by the SCMI server fw.
  */
 struct scmi_protocol_instance {
 	const struct scmi_handle	*handle;
@@ -152,7 +152,7 @@ struct scmi_debug_info {
  *		   scmi_revision_info.num_protocols elements allocated by the
  *		   base protocol
  * @active_protocols: IDR storing device_nodes for protocols actually defined
- *		      in the DT and confirmed as implemented by fw.
+ *		      in the fwnode and confirmed as implemented by fw.
  * @notify_priv: Pointer to private data structure specific to notifications.
  * @node: List head
  * @users: Number of users of this instance
@@ -430,19 +430,19 @@ EXPORT_SYMBOL_GPL(scmi_protocol_unregister);
  * scmi_create_protocol_devices  - Create devices for all pending requests for
  * this SCMI instance.
  *
- * @np: The device node describing the protocol
+ * @fwnode: The device node describing the protocol
  * @info: The SCMI instance descriptor
  * @prot_id: The protocol ID
  * @name: The optional name of the device to be created: if not provided this
  *	  call will lead to the creation of all the devices currently requested
  *	  for the specified protocol.
  */
-static void scmi_create_protocol_devices(struct device_node *np,
+static void scmi_create_protocol_devices(struct fwnode_handle *fwnode,
 					 struct scmi_info *info,
 					 int prot_id, const char *name)
 {
 	mutex_lock(&info->devreq_mtx);
-	scmi_device_create(np, info->dev, prot_id, name);
+	scmi_device_create(fwnode, info->dev, prot_id, name);
 	mutex_unlock(&info->devreq_mtx);
 }
 
@@ -766,9 +766,9 @@ struct scmi_xfer *scmi_xfer_raw_get(const struct scmi_handle *handle)
  * @protocol_id: Identifier of the protocol
  *
  * Note that in a regular SCMI stack, usually, a protocol has to be defined in
- * the DT to have an associated channel and be usable; but in Raw mode any
+ * the fwnode to have an associated channel and be usable; but in Raw mode any
  * protocol in range is allowed, re-using the Base channel, so as to enable
- * fuzzing on any protocol without the need of a fully compiled DT.
+ * fuzzing on any protocol without the need of a fully compiled device node.
  *
  * Return: A reference to the channel to use, or an ERR_PTR
  */
@@ -782,7 +782,7 @@ scmi_xfer_raw_channel_get(const struct scmi_handle *handle, u8 protocol_id)
 	if (!cinfo) {
 		if (protocol_id == SCMI_PROTOCOL_BASE)
 			return ERR_PTR(-EINVAL);
-		/* Use Base channel for protocols not defined for DT */
+		/* Use Base channel for protocols not defined for fwnode */
 		cinfo = idr_find(&info->tx_idr, SCMI_PROTOCOL_BASE);
 		if (!cinfo)
 			return ERR_PTR(-EINVAL);
@@ -2667,7 +2667,7 @@ static int scmi_xfer_info_init(struct scmi_info *sinfo)
 	return ret;
 }
 
-static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
+static int scmi_chan_setup(struct scmi_info *info, struct fwnode_handle *fwnode,
 			   int prot_id, bool tx)
 {
 	int ret, idx;
@@ -2680,7 +2680,7 @@ 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 (!info->desc->ops->chan_available(of_node, idx)) {
+	if (!info->desc->ops->chan_available(fwnode, idx)) {
 		cinfo = idr_find(idr, SCMI_PROTOCOL_BASE);
 		if (unlikely(!cinfo)) /* Possible only if platform has no Rx */
 			return -EINVAL;
@@ -2699,20 +2699,20 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
 	snprintf(name, 32, "__scmi_transport_device_%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);
+	tdev = scmi_device_create(fwnode, info->dev, prot_id, name);
 	if (!tdev) {
 		dev_err(info->dev,
 			"failed to create transport device (%s)\n", name);
 		devm_kfree(info->dev, cinfo);
 		return -EINVAL;
 	}
-	of_node_get(of_node);
 
 	cinfo->id = prot_id;
 	cinfo->dev = &tdev->dev;
+	fwnode_handle_get(fwnode);
 	ret = info->desc->ops->chan_setup(cinfo, info->dev, tx);
 	if (ret) {
-		of_node_put(of_node);
+		fwnode_handle_put(fwnode);
 		scmi_device_destroy(info->dev, prot_id, name);
 		devm_kfree(info->dev, cinfo);
 		return ret;
@@ -2735,7 +2735,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) {
-			of_node_put(of_node);
+			fwnode_handle_put(fwnode);
 			scmi_device_destroy(info->dev, prot_id, name);
 			devm_kfree(info->dev, cinfo);
 		}
@@ -2747,14 +2747,14 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
 }
 
 static inline int
-scmi_txrx_setup(struct scmi_info *info, struct device_node *of_node,
+scmi_txrx_setup(struct scmi_info *info, struct fwnode_handle *fwnode,
 		int prot_id)
 {
-	int ret = scmi_chan_setup(info, of_node, prot_id, true);
+	int ret = scmi_chan_setup(info, fwnode, prot_id, true);
 
 	if (!ret) {
 		/* Rx is optional, report only memory errors */
-		ret = scmi_chan_setup(info, of_node, prot_id, false);
+		ret = scmi_chan_setup(info, fwnode, prot_id, false);
 		if (ret && ret != -ENOMEM)
 			ret = 0;
 	}
@@ -2771,15 +2771,15 @@ scmi_txrx_setup(struct scmi_info *info, struct device_node *of_node,
  *
  * @info: The SCMI instance descriptor.
  *
- * Initialize all the channels found described in the DT against the underlying
+ * Initialize all the channels found described in fwnode against the underlying
  * configured transport using custom defined dedicated devices instead of
  * borrowing devices from the SCMI drivers; this way channels are initialized
  * upfront during core SCMI stack probing and are no more coupled with SCMI
  * devices used by SCMI drivers.
  *
  * Note that, even though a pair of TX/RX channels is associated to each
- * protocol defined in the DT, a distinct freshly initialized channel is
- * created only if the DT node for the protocol at hand describes a dedicated
+ * protocol defined in the fwnode, a distinct freshly initialized channel is
+ * created only if the fwnode for the protocol at hand describes a dedicated
  * channel: in all the other cases the common BASE protocol channel is reused.
  *
  * Return: 0 on Success
@@ -2787,17 +2787,17 @@ scmi_txrx_setup(struct scmi_info *info, struct device_node *of_node,
 static int scmi_channels_setup(struct scmi_info *info)
 {
 	int ret;
-	struct device_node *top_np = info->dev->of_node;
+	struct fwnode_handle *fwnode = dev_fwnode(info->dev);
 
 	/* Initialize a common generic channel at first */
-	ret = scmi_txrx_setup(info, top_np, SCMI_PROTOCOL_BASE);
+	ret = scmi_txrx_setup(info, fwnode, SCMI_PROTOCOL_BASE);
 	if (ret)
 		return ret;
 
-	for_each_available_child_of_node_scoped(top_np, child) {
+	fwnode_for_each_available_child_node_scoped(fwnode, child) {
 		u32 prot_id;
 
-		if (of_property_read_u32(child, "reg", &prot_id))
+		if (fwnode_property_read_u32(child, "reg", &prot_id))
 			continue;
 
 		if (!FIELD_FIT(MSG_PROTOCOL_ID_MASK, prot_id))
@@ -2820,7 +2820,7 @@ static int scmi_chan_destroy(int id, void *p, void *idr)
 		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);
+		fwnode_handle_put(dev_fwnode(cinfo->dev));
 		scmi_device_destroy(info->dev, id, sdev->name);
 		cinfo->dev = NULL;
 	}
@@ -2881,12 +2881,12 @@ static int scmi_bus_notifier(struct notifier_block *nb,
 static int scmi_device_request_notifier(struct notifier_block *nb,
 					unsigned long action, void *data)
 {
-	struct device_node *np;
+	struct fwnode_handle *fwnode;
 	struct scmi_device_id *id_table = data;
 	struct scmi_info *info = req_nb_to_scmi_info(nb);
 
-	np = idr_find(&info->active_protocols, id_table->protocol_id);
-	if (!np)
+	fwnode = idr_find(&info->active_protocols, id_table->protocol_id);
+	if (!fwnode)
 		return NOTIFY_DONE;
 
 	dev_dbg(info->dev, "%sRequested device (%s) for protocol 0x%x\n",
@@ -2895,7 +2895,7 @@ static int scmi_device_request_notifier(struct notifier_block *nb,
 
 	switch (action) {
 	case SCMI_BUS_NOTIFY_DEVICE_REQUEST:
-		scmi_create_protocol_devices(np, info, id_table->protocol_id,
+		scmi_create_protocol_devices(fwnode, info, id_table->protocol_id,
 					     id_table->name);
 		break;
 	case SCMI_BUS_NOTIFY_DEVICE_UNREQUEST:
@@ -2982,13 +2982,14 @@ static struct scmi_debug_info *scmi_debugfs_common_setup(struct scmi_info *info)
 	if (!dbg)
 		return NULL;
 
-	dbg->name = kstrdup(of_node_full_name(info->dev->of_node), GFP_KERNEL);
+	dbg->name = kstrdup(fwnode_get_name(dev_fwnode(info->dev)), GFP_KERNEL);
 	if (!dbg->name) {
 		devm_kfree(info->dev, dbg);
 		return NULL;
 	}
 
-	of_property_read_string(info->dev->of_node, "compatible", &c_ptr);
+	fwnode_property_read_string(dev_fwnode(info->dev), "compatible",
+				    &c_ptr);
 	dbg->type = kstrdup(c_ptr, GFP_KERNEL);
 	if (!dbg->type) {
 		kfree(dbg->name);
@@ -3097,20 +3098,20 @@ static const struct scmi_desc *scmi_transport_setup(struct device *dev)
 
 	dev_info(dev, "Using %s\n", dev_driver_string(trans->supplier));
 
-	ret = of_property_read_u32(dev->of_node, "arm,max-rx-timeout-ms",
-				   &trans->desc.max_rx_timeout_ms);
+	ret = fwnode_property_read_u32(dev_fwnode(dev), "arm,max-rx-timeout-ms",
+				       &trans->desc.max_rx_timeout_ms);
 	if (ret && ret != -EINVAL)
-		dev_err(dev, "Malformed arm,max-rx-timeout-ms DT property.\n");
+		dev_err(dev, "Malformed arm,max-rx-timeout-ms property.\n");
 
-	ret = of_property_read_u32(dev->of_node, "arm,max-msg-size",
-				   &trans->desc.max_msg_size);
+	ret = fwnode_property_read_u32(dev_fwnode(dev), "arm,max-msg-size",
+				       &trans->desc.max_msg_size);
 	if (ret && ret != -EINVAL)
-		dev_err(dev, "Malformed arm,max-msg-size DT property.\n");
+		dev_err(dev, "Malformed arm,max-msg-size property.\n");
 
-	ret = of_property_read_u32(dev->of_node, "arm,max-msg",
-				   &trans->desc.max_msg);
+	ret = fwnode_property_read_u32(dev_fwnode(dev), "arm,max-msg",
+				       &trans->desc.max_msg);
 	if (ret && ret != -EINVAL)
-		dev_err(dev, "Malformed arm,max-msg DT property.\n");
+		dev_err(dev, "Malformed arm,max-msg property.\n");
 
 	dev_info(dev,
 		 "SCMI max-rx-timeout: %dms / max-msg-size: %dbytes / max-msg: %d\n",
@@ -3118,8 +3119,8 @@ static const struct scmi_desc *scmi_transport_setup(struct device *dev)
 		 trans->desc.max_msg);
 
 	/* System wide atomic threshold for atomic ops .. if any */
-	if (!of_property_read_u32(dev->of_node, "atomic-threshold-us",
-				  &trans->desc.atomic_threshold))
+	if (!fwnode_property_read_u32(dev_fwnode(dev), "atomic-threshold-us",
+				      &trans->desc.atomic_threshold))
 		dev_info(dev,
 			 "SCMI System wide atomic threshold set to %u us\n",
 			 trans->desc.atomic_threshold);
@@ -3148,7 +3149,6 @@ static int scmi_probe(struct platform_device *pdev)
 	struct scmi_info *info;
 	bool coex = IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT_COEX);
 	struct device *dev = &pdev->dev;
-	struct device_node *child, *np = dev->of_node;
 
 	desc = scmi_transport_setup(dev);
 	if (!desc) {
@@ -3187,7 +3187,7 @@ static int scmi_probe(struct platform_device *pdev)
 	handle->devm_protocol_put = scmi_devm_protocol_put;
 	handle->is_transport_atomic = scmi_is_transport_atomic;
 
-	/* Setup all channels described in the DT at first */
+	/* Setup all channels described in the fwnode at first */
 	ret = scmi_channels_setup(info);
 	if (ret) {
 		err_str = "failed to setup channels\n";
@@ -3262,10 +3262,10 @@ static int scmi_probe(struct platform_device *pdev)
 
 	scmi_enable_matching_quirks(info);
 
-	for_each_available_child_of_node(np, child) {
+	fwnode_for_each_available_child_node_scoped(dev_fwnode(dev), child) {
 		u32 prot_id;
 
-		if (of_property_read_u32(child, "reg", &prot_id))
+		if (fwnode_property_read_u32(child, "reg", &prot_id))
 			continue;
 
 		if (!FIELD_FIT(MSG_PROTOCOL_ID_MASK, prot_id))
@@ -3278,10 +3278,11 @@ static int scmi_probe(struct platform_device *pdev)
 		}
 
 		/*
-		 * Save this valid DT protocol descriptor amongst
+		 * Save this valid fwnode protocol descriptor amongst
 		 * @active_protocols for this SCMI instance/
 		 */
-		ret = idr_alloc(&info->active_protocols, child,
+		ret = idr_alloc(&info->active_protocols,
+				fwnode_handle_get(child),
 				prot_id, prot_id + 1, GFP_KERNEL);
 		if (ret != prot_id) {
 			dev_err(dev, "SCMI protocol %d already activated. Skip\n",
@@ -3289,7 +3290,6 @@ static int scmi_probe(struct platform_device *pdev)
 			continue;
 		}
 
-		of_node_get(child);
 		scmi_create_protocol_devices(child, info, prot_id, NULL);
 	}
 
@@ -3317,7 +3317,7 @@ static void scmi_remove(struct platform_device *pdev)
 {
 	int id;
 	struct scmi_info *info = platform_get_drvdata(pdev);
-	struct device_node *child;
+	struct fwnode_handle *child;
 
 	if (IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT))
 		scmi_raw_mode_cleanup(info->raw);
@@ -3336,7 +3336,7 @@ static void scmi_remove(struct platform_device *pdev)
 	mutex_unlock(&info->protocols_mtx);
 
 	idr_for_each_entry(&info->active_protocols, child, id)
-		of_node_put(child);
+		fwnode_handle_put(child);
 	idr_destroy(&info->active_protocols);
 
 	blocking_notifier_chain_unregister(&scmi_requested_devices_nh,
diff --git a/drivers/firmware/arm_scmi/transports/mailbox.c b/drivers/firmware/arm_scmi/transports/mailbox.c
index ae0f67e6cc45..bc27505879af 100644
--- a/drivers/firmware/arm_scmi/transports/mailbox.c
+++ b/drivers/firmware/arm_scmi/transports/mailbox.c
@@ -77,9 +77,10 @@ static void rx_callback(struct mbox_client *cl, void *m)
 		      core->shmem->read_header(smbox->shmem), NULL);
 }
 
-static bool mailbox_chan_available(struct device_node *of_node, int idx)
+static bool mailbox_chan_available(struct fwnode_handle *fwnode, int idx)
 {
 	int num_mb;
+	struct device_node *of_node = to_of_node(fwnode);
 
 	/*
 	 * Just check if bidirrectional channels are involved, and check the
diff --git a/drivers/firmware/arm_scmi/transports/optee.c b/drivers/firmware/arm_scmi/transports/optee.c
index dc0f46340153..846d28472923 100644
--- a/drivers/firmware/arm_scmi/transports/optee.c
+++ b/drivers/firmware/arm_scmi/transports/optee.c
@@ -312,9 +312,10 @@ static int invoke_process_msg_channel(struct scmi_optee_channel *channel, size_t
 	return 0;
 }
 
-static bool scmi_optee_chan_available(struct device_node *of_node, int idx)
+static bool scmi_optee_chan_available(struct fwnode_handle *fwnode, int idx)
 {
 	u32 channel_id;
+	struct device_node *of_node = to_of_node(fwnode);
 
 	return !of_property_read_u32_index(of_node, "linaro,optee-channel-id",
 					   idx, &channel_id);
diff --git a/drivers/firmware/arm_scmi/transports/smc.c b/drivers/firmware/arm_scmi/transports/smc.c
index 21abb571e4f2..75e539578670 100644
--- a/drivers/firmware/arm_scmi/transports/smc.c
+++ b/drivers/firmware/arm_scmi/transports/smc.c
@@ -84,8 +84,9 @@ static irqreturn_t smc_msg_done_isr(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static bool smc_chan_available(struct device_node *of_node, int idx)
+static bool smc_chan_available(struct fwnode_handle *fwnode, int idx)
 {
+	struct device_node *of_node = to_of_node(fwnode);
 	struct device_node *np __free(device_node) =
 					of_parse_phandle(of_node, "shmem", 0);
 	if (!np)
diff --git a/drivers/firmware/arm_scmi/transports/virtio.c b/drivers/firmware/arm_scmi/transports/virtio.c
index 326c4a93e44b..cb749ac7ab46 100644
--- a/drivers/firmware/arm_scmi/transports/virtio.c
+++ b/drivers/firmware/arm_scmi/transports/virtio.c
@@ -373,7 +373,7 @@ static unsigned int virtio_get_max_msg(struct scmi_chan_info *base_cinfo)
 	return vioch->max_msg;
 }
 
-static bool virtio_chan_available(struct device_node *of_node, int idx)
+static bool virtio_chan_available(struct fwnode_handle *fwnode, int idx)
 {
 	struct scmi_vio_channel *channels, *vioch = NULL;
 

-- 
2.34.1

[PATCH 4/8] firmware: arm_scmi: Fall back to ACPI HID when "compatible" is absent

[Sudeep Holla]

scmi_debugfs_common_setup() previously called fwnode_property_read_string()
without checking its return value, which could leave c_ptr uninitialized
and lead to kstrdup() on garbage.

Handle the failure case and use the ACPI HID via acpi_device_hid()
using ACPI_COMPANION(dev) as a fallback for ACPI-described systems.
This ensures dbg->type is either a valid string or we hit the existing
error path cleanly.

This improves robustness on non-DT platforms where "compatible" is not
present.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 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 bc5fea11b5db..148b9a13d43f 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2988,8 +2988,9 @@ static struct scmi_debug_info *scmi_debugfs_common_setup(struct scmi_info *info)
 		return NULL;
 	}
 
-	fwnode_property_read_string(dev_fwnode(info->dev), "compatible",
-				    &c_ptr);
+	if (fwnode_property_read_string(dev_fwnode(info->dev), "compatible",
+					&c_ptr))
+		c_ptr = acpi_device_hid(ACPI_COMPANION(info->dev)) ?: "unknown";
 	dbg->type = kstrdup(c_ptr, GFP_KERNEL);
 	if (!dbg->type) {
 		kfree(dbg->name);

-- 
2.34.1

[PATCH 5/8] firmware: arm_scmi: Pass protocol ID to chan_available() transport callback

[Sudeep Holla]

Extend the SCMI transport_ops chan_available() callback to include the
protocol ID (prot_id) as an argument. This allows transports to determine
channel availability based on the specific protocol being used, improving
flexibility in platforms that share transport channels across multiple
protocols. This will be useful when ACPI PCC transport gets added.

Updated all existing users and definitions of chan_available() in
SCMI core and transport drivers (mailbox, optee, etc.) accordingly.

No functional change.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scmi/common.h             | 3 ++-
 drivers/firmware/arm_scmi/driver.c             | 2 +-
 drivers/firmware/arm_scmi/transports/mailbox.c | 3 ++-
 drivers/firmware/arm_scmi/transports/optee.c   | 3 ++-
 drivers/firmware/arm_scmi/transports/smc.c     | 3 ++-
 drivers/firmware/arm_scmi/transports/virtio.c  | 3 ++-
 6 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 4e30283036e5..11831da27d31 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -204,7 +204,8 @@ struct scmi_chan_info {
  * @poll_done: Callback to poll transfer status
  */
 struct scmi_transport_ops {
-	bool (*chan_available)(struct fwnode_handle *fwnode, int idx);
+	bool (*chan_available)(struct fwnode_handle *fwnode, int prot_id,
+			       int idx);
 	int (*chan_setup)(struct scmi_chan_info *cinfo, struct device *dev,
 			  bool tx);
 	int (*chan_free)(int id, void *p, void *data);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 148b9a13d43f..ac51726f24db 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2680,7 +2680,7 @@ static int scmi_chan_setup(struct scmi_info *info, struct fwnode_handle *fwnode,
 	idx = tx ? 0 : 1;
 	idr = tx ? &info->tx_idr : &info->rx_idr;
 
-	if (!info->desc->ops->chan_available(fwnode, idx)) {
+	if (!info->desc->ops->chan_available(fwnode, prot_id, idx)) {
 		cinfo = idr_find(idr, SCMI_PROTOCOL_BASE);
 		if (unlikely(!cinfo)) /* Possible only if platform has no Rx */
 			return -EINVAL;
diff --git a/drivers/firmware/arm_scmi/transports/mailbox.c b/drivers/firmware/arm_scmi/transports/mailbox.c
index bc27505879af..76e609e674e5 100644
--- a/drivers/firmware/arm_scmi/transports/mailbox.c
+++ b/drivers/firmware/arm_scmi/transports/mailbox.c
@@ -77,7 +77,8 @@ static void rx_callback(struct mbox_client *cl, void *m)
 		      core->shmem->read_header(smbox->shmem), NULL);
 }
 
-static bool mailbox_chan_available(struct fwnode_handle *fwnode, int idx)
+static bool
+mailbox_chan_available(struct fwnode_handle *fwnode, int prot_id, int idx)
 {
 	int num_mb;
 	struct device_node *of_node = to_of_node(fwnode);
diff --git a/drivers/firmware/arm_scmi/transports/optee.c b/drivers/firmware/arm_scmi/transports/optee.c
index 846d28472923..d7d72138d6cc 100644
--- a/drivers/firmware/arm_scmi/transports/optee.c
+++ b/drivers/firmware/arm_scmi/transports/optee.c
@@ -312,7 +312,8 @@ static int invoke_process_msg_channel(struct scmi_optee_channel *channel, size_t
 	return 0;
 }
 
-static bool scmi_optee_chan_available(struct fwnode_handle *fwnode, int idx)
+static bool
+scmi_optee_chan_available(struct fwnode_handle *fwnode, int prot_id, int idx)
 {
 	u32 channel_id;
 	struct device_node *of_node = to_of_node(fwnode);
diff --git a/drivers/firmware/arm_scmi/transports/smc.c b/drivers/firmware/arm_scmi/transports/smc.c
index 75e539578670..075be0b9921a 100644
--- a/drivers/firmware/arm_scmi/transports/smc.c
+++ b/drivers/firmware/arm_scmi/transports/smc.c
@@ -84,7 +84,8 @@ static irqreturn_t smc_msg_done_isr(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static bool smc_chan_available(struct fwnode_handle *fwnode, int idx)
+static bool
+smc_chan_available(struct fwnode_handle *fwnode, int prot_id, int idx)
 {
 	struct device_node *of_node = to_of_node(fwnode);
 	struct device_node *np __free(device_node) =
diff --git a/drivers/firmware/arm_scmi/transports/virtio.c b/drivers/firmware/arm_scmi/transports/virtio.c
index cb749ac7ab46..7137165dc204 100644
--- a/drivers/firmware/arm_scmi/transports/virtio.c
+++ b/drivers/firmware/arm_scmi/transports/virtio.c
@@ -373,7 +373,8 @@ static unsigned int virtio_get_max_msg(struct scmi_chan_info *base_cinfo)
 	return vioch->max_msg;
 }
 
-static bool virtio_chan_available(struct fwnode_handle *fwnode, int idx)
+static bool
+virtio_chan_available(struct fwnode_handle *fwnode, int prot_id, int idx)
 {
 	struct scmi_vio_channel *channels, *vioch = NULL;
 

-- 
2.34.1

[PATCH 6/8] firmware: arm_scmi: Refactor protocol device creation logic

[Sudeep Holla]

Refactor the protocol validation and device creation logic in
scmi_probe() into a new helper function, scmi_device_check_create(),
to improve readability and reduce code duplication.

The new helper consolidates checks for protocol ID range, implementation
availability, and duplicate activation, before invoking
scmi_create_protocol_devices(). This refactor simplifies the SCMI probe
path while preserving existing behavior.

No functional changes intended. This refactoring is required to enable
ACPI PCC transport.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scmi/driver.c | 56 ++++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 23 deletions(-)

diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index ac51726f24db..f679a769fc87 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -3141,6 +3141,38 @@ static void scmi_enable_matching_quirks(struct scmi_info *info)
 			   rev->sub_vendor_id, rev->impl_ver);
 }
 
+static void scmi_device_check_create(struct fwnode_handle *fwnode, int prot_id,
+				     struct scmi_info *info)
+{
+	int ret;
+	struct device *dev = info->dev;
+	struct scmi_handle *handle = &info->handle;
+
+	if (!FIELD_FIT(MSG_PROTOCOL_ID_MASK, prot_id))
+		dev_err(dev, "Out of range protocol %d\n", prot_id);
+
+	if (!scmi_is_protocol_implemented(handle, prot_id)) {
+		dev_err(dev, "SCMI protocol %d not implemented\n",
+			prot_id);
+		return;
+	}
+
+	/*
+	 * Save this valid fwnode protocol descriptor amongst
+	 * @active_protocols for this SCMI instance/
+	 */
+	ret = idr_alloc(&info->active_protocols,
+			fwnode_handle_get(fwnode),
+			prot_id, prot_id + 1, GFP_KERNEL);
+	if (ret != prot_id) {
+		dev_err(dev, "SCMI protocol %d already activated. Skip\n",
+			prot_id);
+		return;
+	}
+
+	scmi_create_protocol_devices(fwnode, info, prot_id, NULL);
+}
+
 static int scmi_probe(struct platform_device *pdev)
 {
 	int ret;
@@ -3269,29 +3301,7 @@ static int scmi_probe(struct platform_device *pdev)
 		if (fwnode_property_read_u32(child, "reg", &prot_id))
 			continue;
 
-		if (!FIELD_FIT(MSG_PROTOCOL_ID_MASK, prot_id))
-			dev_err(dev, "Out of range protocol %d\n", prot_id);
-
-		if (!scmi_is_protocol_implemented(handle, prot_id)) {
-			dev_err(dev, "SCMI protocol %d not implemented\n",
-				prot_id);
-			continue;
-		}
-
-		/*
-		 * Save this valid fwnode protocol descriptor amongst
-		 * @active_protocols for this SCMI instance/
-		 */
-		ret = idr_alloc(&info->active_protocols,
-				fwnode_handle_get(child),
-				prot_id, prot_id + 1, GFP_KERNEL);
-		if (ret != prot_id) {
-			dev_err(dev, "SCMI protocol %d already activated. Skip\n",
-				prot_id);
-			continue;
-		}
-
-		scmi_create_protocol_devices(child, info, prot_id, NULL);
+		scmi_device_check_create(child, prot_id, info);
 	}
 
 	return 0;

-- 
2.34.1

[PATCH 7/8] firmware: arm_scmi: transport: Add ACPI PCC transport

[Sudeep Holla]

Introduce a new SCMI transport that uses ACPI PCCT (PCC) subspaces via
the Linux PCC mailbox layer. The driver parses ACPI _DSD data to map
protocols to PCC subspace UIDs, supports shared TX/RX channels, and
optionally sets up a P2A channel for notifications.

Key points:
- new CONFIG_ARM_SCMI_TRANSPORT_PCC option
- integration with SCMI core via scmi_desc and transport ops
- response/notification fetch from PCC shared memory header/payload
- ACPI device matching and registration via the ACPI transport macro

This enables SCMI to be exercised over PCC on ACPI platforms.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scmi/common.h            |  11 +
 drivers/firmware/arm_scmi/transports/Kconfig  |  12 +
 drivers/firmware/arm_scmi/transports/Makefile |   2 +
 drivers/firmware/arm_scmi/transports/pcc.c    | 390 ++++++++++++++++++++++++++
 4 files changed, 415 insertions(+)

diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 11831da27d31..9afab6fb78be 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -440,6 +440,17 @@ struct scmi_transport_core_operations {
 	const struct scmi_message_operations *msg;
 };
 
+struct scmi_dsd_info {
+	u32 protocol_id;
+	const char *const property_name;
+};
+
+static struct scmi_dsd_info scmi_dsd_info_list[] = {
+	{ SCMI_PROTOCOL_BASE, "arm-arml0001-transport-pcc"},
+	{ SCMI_PROTOCOL_POWERCAP, "arm-arml0001-protocol-pcap"},
+	{ SCMI_PROTOCOL_TELEMETRY, "arm-arml0001-protocol-telemetry"},
+};
+
 /**
  * struct scmi_transport  - A structure representing a configured transport
  *
diff --git a/drivers/firmware/arm_scmi/transports/Kconfig b/drivers/firmware/arm_scmi/transports/Kconfig
index 57eccf316e26..4936078f279f 100644
--- a/drivers/firmware/arm_scmi/transports/Kconfig
+++ b/drivers/firmware/arm_scmi/transports/Kconfig
@@ -77,6 +77,18 @@ config ARM_SCMI_TRANSPORT_OPTEE
 	  This driver can also be built as a module. If so, the module
 	  will be called scmi_transport_optee.
 
+config ARM_SCMI_TRANSPORT_PCC
+	tristate "SCMI transport based on ACPI PCC"
+	depends on PCC
+	select ARM_SCMI_HAVE_TRANSPORT
+	help
+	  Enable ACPI PCC mailbox based transport for SCMI.
+
+	  If you want the ARM SCMI PROTOCOL stack to include support for a
+	  transport based on mailboxes, answer Y.
+	  This driver can also be built as a module. If so, the module
+	  will be called scmi_transport_pcc.
+
 config ARM_SCMI_TRANSPORT_VIRTIO
 	tristate "SCMI transport based on VirtIO"
 	depends on VIRTIO
diff --git a/drivers/firmware/arm_scmi/transports/Makefile b/drivers/firmware/arm_scmi/transports/Makefile
index 3ba3d3bee151..e7c4b8de6251 100644
--- a/drivers/firmware/arm_scmi/transports/Makefile
+++ b/drivers/firmware/arm_scmi/transports/Makefile
@@ -7,6 +7,8 @@ scmi_transport_mailbox-objs := mailbox.o
 obj-$(CONFIG_ARM_SCMI_TRANSPORT_MAILBOX) += scmi_transport_mailbox.o
 scmi_transport_optee-objs := optee.o
 obj-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += scmi_transport_optee.o
+scmi_transport_pcc-objs := pcc.o
+obj-$(CONFIG_ARM_SCMI_TRANSPORT_PCC) += scmi_transport_pcc.o
 scmi_transport_virtio-objs := virtio.o
 obj-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += scmi_transport_virtio.o
 
diff --git a/drivers/firmware/arm_scmi/transports/pcc.c b/drivers/firmware/arm_scmi/transports/pcc.c
new file mode 100644
index 000000000000..39ef83e2dfd4
--- /dev/null
+++ b/drivers/firmware/arm_scmi/transports/pcc.c
@@ -0,0 +1,390 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * System Control and Management Interface (SCMI) Message ACPI PCC
+ * Transport Driver
+ *
+ * This transport uses ACPI PCC (PCCT Type 3/4) subspaces via the Linux
+ * PCC mailbox controller to exchange SCMI messages over the standard
+ * SCMI Shared Memory Transport (SMT) layout.
+ *
+ * PCC subspace selection is conveyed via ACPI SCMI namespace device.
+ *
+ * Copyright (C) 2025
+ */
+
+#include <linux/acpi.h>
+#include <linux/err.h>
+#include <linux/device.h>
+#include <linux/hashtable.h>
+#include <linux/io.h>
+#include <linux/mailbox_client.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <acpi/pcc.h>
+
+#include "../common.h"
+
+#define SCMI_SHMEM_FLAG_INTR_ENABLED		BIT(0)
+#define SCMI_TRANSPORT_PACKAGE_MAX_VERSION	(1)
+#define SCMI_PROTOCOL_PACKAGE_MAX_VERSION	(1)
+#define SCMI_TRANSPORT_SHARED_CHANNEL		BIT(0)
+#define SCMI_TRANSPORT_P2A_CHANNEL		BIT(1)
+
+/*
+ * SCMI specification requires all parameters, message headers, return
+ * arguments or any protocol data to be expressed in little endian
+ * format only.
+ */
+struct pcc_shared_mem {
+	struct acpi_pcct_ext_pcc_shared_memory header;
+	u8 msg_payload[];
+};
+
+/**
+ * struct scmi_pcc - Structure representing a SCMI mailbox transport
+ *
+ * @cl: Mailbox Client
+ * @pchan: Transmit/Receive PCC/mailbox channel
+ * @cinfo: SCMI channel info
+ * @shmem: Transmit/Receive shared memory area
+ */
+struct scmi_pcc {
+	struct mbox_client cl;
+	struct pcc_mbox_chan *pchan;
+	struct scmi_chan_info *cinfo;
+};
+
+struct pcc_transport {
+	u32 pcc_ss_id;
+	u32 protocol_id;
+	u32 flags;
+	struct hlist_node hnode;
+};
+
+/* Not all MAX_PCC_SUBSPACES will be used for SCMI, keeping it at 16 for now */
+static DECLARE_HASHTABLE(pcc_id_hash, ilog2(MAX_PCC_SUBSPACES / 8));
+
+#define client_to_scmi_pcc(c) container_of(c, struct scmi_pcc, cl)
+
+static struct scmi_transport_core_operations *core;
+
+static int acpi_scmi_dsd_parse_transport_package(const union acpi_object *obj)
+{
+	unsigned int revision = obj->package.elements[0].integer.value;
+	unsigned int pkg_cnt = obj->package.elements[1].integer.value;
+	int idx;
+
+	if (revision > SCMI_TRANSPORT_PACKAGE_MAX_VERSION)
+		return -EINVAL;
+
+	for (idx = 0; idx < pkg_cnt; idx++) {
+		union acpi_object *pack = &obj->package.elements[idx + 2];
+		struct pcc_transport *p, *tmp;
+		u32 uid;
+
+		p = kzalloc(sizeof(*p), GFP_KERNEL);
+		if (!p)
+			return -ENOMEM;
+
+		if (pack->type != ACPI_TYPE_PACKAGE || pack->package.count != 3)
+			pr_info("Invalid transport properties pkg %d\n", idx);
+
+		uid = pack->package.elements[1].integer.value;
+		p->pcc_ss_id = pack->package.elements[0].integer.value;
+		p->flags = pack->package.elements[2].integer.value;
+
+		hash_for_each_possible(pcc_id_hash, tmp, hnode, uid)
+			if (tmp) {
+				pr_info("Duplicate UID %d\n", uid);
+				return -EEXIST;
+			}
+
+		if (p->flags & SCMI_TRANSPORT_SHARED_CHANNEL)
+			p->protocol_id = SCMI_PROTOCOL_BASE;
+
+		hash_add(pcc_id_hash, &p->hnode, uid);
+	}
+
+	return 0;
+}
+
+static int acpi_scmi_dsd_parse_protocol_subpackage(const union acpi_object *obj,
+						   int prot_id)
+{
+	u32 uid;
+	int idx, ret = 0;
+	struct pcc_transport *p;
+	unsigned int pkg_cnt = obj->package.count;
+
+	if (pkg_cnt > 2) {
+		pr_warn("Only 2 channels: one Tx and one Rx needed\n");
+		return -EINVAL;
+	}
+
+	for (idx = 0; idx < pkg_cnt; idx++) {
+		union acpi_object *pack = &obj->package.elements[idx];
+
+		/* Flags(pack->package.elements[1]) must be always 0 for now */
+		uid = pack->package.elements[0].integer.value;
+		hash_for_each_possible(pcc_id_hash, p, hnode, uid) {
+			if (p->flags & SCMI_TRANSPORT_SHARED_CHANNEL) {
+				pr_info("Invalid! %d channel is shared\n",
+					p->pcc_ss_id);
+				ret = -EINVAL;
+				break;
+			}
+			p->protocol_id = prot_id;
+		}
+	}
+
+	return ret;
+}
+
+static int
+acpi_scmi_dsd_parse_protocol_package(const union acpi_object *obj, int prot_id)
+{
+	unsigned int revision = obj->package.elements[0].integer.value;
+	union acpi_object *pack = &obj->package.elements[1];
+
+	if (revision > SCMI_PROTOCOL_PACKAGE_MAX_VERSION)
+		return -EINVAL;
+
+	if (pack->type != ACPI_TYPE_PACKAGE) {
+		pr_info("Invalid protocol transport package\n");
+		return -EINVAL;
+	}
+
+	/* Empty protocol specific transport package allowed */
+	if (pack->package.count != 0)
+		acpi_scmi_dsd_parse_protocol_subpackage(pack, prot_id);
+
+	pack = &obj->package.elements[2];
+	if (pack->type != ACPI_TYPE_PACKAGE)
+		pr_info("Invalid protocol transport association package\n");
+
+	if (pack->package.count != 0)
+		pr_info("Non-empty association package not supported\n");
+
+	return 0;
+}
+
+static int acpi_scmi_namespace_device_parse(struct fwnode_handle *fwnode)
+{
+	struct acpi_device *adev = to_acpi_device_node(fwnode);
+	const union acpi_object *obj;
+	int ret, idx;
+
+	hash_init(pcc_id_hash);
+
+	for (idx = 0; idx < ARRAY_SIZE(scmi_dsd_info_list); idx++) {
+		int prot_id = scmi_dsd_info_list[idx].protocol_id;
+		const char *propname = scmi_dsd_info_list[idx].property_name;
+
+		ret = acpi_dev_get_property(adev, propname, ACPI_TYPE_ANY, &obj);
+		if (ret)
+			continue;
+
+		if (obj->type != ACPI_TYPE_PACKAGE) {
+			dev_warn(&adev->dev, "PACKAGE property expected!\n");
+			break;
+		}
+
+		if (prot_id == SCMI_PROTOCOL_BASE)
+			acpi_scmi_dsd_parse_transport_package(obj);
+		else
+			acpi_scmi_dsd_parse_protocol_package(obj, prot_id);
+	}
+
+	return 0;
+}
+
+static int pcc_get_ss_id(u32 prot_id, bool tx)
+{
+	struct pcc_transport *p;
+	int idx;
+
+	hash_for_each(pcc_id_hash, idx, p, hnode)
+		if (p->protocol_id == prot_id) {
+			if ((!tx && (p->flags & SCMI_TRANSPORT_P2A_CHANNEL)) ||
+			    (tx && !(p->flags & SCMI_TRANSPORT_P2A_CHANNEL)))
+				return p->pcc_ss_id;
+		}
+
+	return -ENOENT;
+}
+
+static bool
+pcc_chan_available(struct fwnode_handle *fwnode, int prot_id, int idx)
+{
+	/*
+	 * Just parse the ACPI SCMI namespace device once for idx = 0,
+	 * defer full validation until pcc_chan_setup() for simplicity.
+	 */
+	if (!idx)
+		acpi_scmi_namespace_device_parse(fwnode);
+
+	if (pcc_get_ss_id(prot_id, idx ? false : true) < 0)
+		return false;
+
+	return true;
+}
+
+static void tx_prepare(struct mbox_client *cl, void *m)
+{
+	struct scmi_pcc *smbox = client_to_scmi_pcc(cl);
+	struct pcc_shared_mem __iomem *shmem = smbox->pchan->shmem;
+	struct scmi_xfer *xfer = m;
+
+	/* PCC take cares not to call tx_prepare until last transmit is done */
+	/* Mark channel busy + clear error */
+	iowrite32(SCMI_SHMEM_FLAG_INTR_ENABLED, &shmem->header.flags);
+	iowrite32(sizeof(shmem->header.command) + xfer->tx.len,
+		  &shmem->header.length);
+	iowrite32(pack_scmi_header(&xfer->hdr), &shmem->header.command);
+	if (xfer->tx.buf)
+		memcpy_toio(shmem->msg_payload, xfer->tx.buf, xfer->tx.len);
+}
+
+static void rx_callback(struct mbox_client *cl, void *m)
+{
+	struct scmi_pcc *smbox = client_to_scmi_pcc(cl);
+	struct pcc_shared_mem __iomem *shmem = smbox->pchan->shmem;
+
+	core->rx_callback(smbox->cinfo, ioread32(&shmem->header.command), NULL);
+}
+
+static int pcc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
+			  bool tx)
+{
+	const char *desc = tx ? "Tx" : "Rx";
+	struct device *cdev = cinfo->dev;
+	struct scmi_pcc *smbox;
+	int ret, ss_id;
+	struct mbox_client *cl;
+
+	smbox = devm_kzalloc(dev, sizeof(*smbox), GFP_KERNEL);
+	if (!smbox)
+		return -ENOMEM;
+
+	cl = &smbox->cl;
+	cl->dev = cdev;
+	cl->tx_prepare = tx ? tx_prepare : NULL;
+	cl->rx_callback = rx_callback;
+	cl->tx_block = false;
+
+	ss_id = pcc_get_ss_id(cinfo->id, tx);
+	if (ss_id < 0)
+		return ss_id;
+
+	smbox->pchan = pcc_mbox_request_channel(cl, ss_id);
+	if (IS_ERR(smbox->pchan)) {
+		ret = PTR_ERR(smbox->pchan);
+		if (ret != -EPROBE_DEFER)
+			dev_err(cdev,
+				"failed to request SCMI %s mailbox\n", desc);
+		return ret;
+	}
+
+	cinfo->transport_info = smbox;
+	smbox->cinfo = cinfo;
+
+	return 0;
+}
+
+static int pcc_chan_free(int id, void *p, void *data)
+{
+	struct scmi_chan_info *cinfo = p;
+	struct scmi_pcc *smbox = cinfo->transport_info;
+
+	if (smbox && !IS_ERR(smbox->pchan)) {
+		pcc_mbox_free_channel(smbox->pchan);
+		cinfo->transport_info = NULL;
+		smbox->pchan = NULL;
+		smbox->cinfo = NULL;
+	}
+
+	return 0;
+}
+
+static int
+pcc_send_message(struct scmi_chan_info *cinfo, struct scmi_xfer *xfer)
+{
+	struct scmi_pcc *smbox = cinfo->transport_info;
+	int ret;
+
+	/*
+	 * The mailbox layer has its own queue. However the mailbox queue
+	 * confuses the per message SCMI timeouts since the clock starts when
+	 * the message is submitted into the mailbox queue. So when multiple
+	 * messages are queued up the clock starts on all messages instead of
+	 * only the one inflight.
+	 */
+	ret = mbox_send_message(smbox->pchan->mchan, xfer);
+	/* mbox_send_message returns non-negative value on success */
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static void pcc_fetch_response(struct scmi_chan_info *cinfo,
+			       struct scmi_xfer *xfer)
+{
+	struct scmi_pcc *smbox = cinfo->transport_info;
+	struct pcc_shared_mem __iomem *shmem = smbox->pchan->shmem;
+	size_t len = ioread32(&shmem->header.length);
+
+	xfer->hdr.status = ioread32(shmem->msg_payload);
+	/* Skip the length of header and status in shmem area i.e 8 bytes */
+	xfer->rx.len = min_t(size_t, xfer->rx.len, len > 8 ? len - 8 : 0);
+
+	/* Take a copy to the rx buffer.. */
+	memcpy_fromio(xfer->rx.buf, shmem->msg_payload + 4, xfer->rx.len);
+}
+
+static void pcc_fetch_notification(struct scmi_chan_info *cinfo, size_t max_len,
+				   struct scmi_xfer *xfer)
+{
+	struct scmi_pcc *smbox = cinfo->transport_info;
+	struct pcc_shared_mem __iomem *shmem = smbox->pchan->shmem;
+	size_t len = ioread32(&shmem->header.length);
+
+	/* Skip only the length of header in shmem area i.e 4 bytes */
+	xfer->rx.len = min_t(size_t, max_len, len > 4 ? len - 4 : 0);
+
+	/* Take a copy to the rx buffer.. */
+	memcpy_fromio(xfer->rx.buf, shmem->msg_payload, xfer->rx.len);
+}
+
+static const struct scmi_transport_ops scmi_pcc_ops = {
+	.chan_available = pcc_chan_available,
+	.chan_setup = pcc_chan_setup,
+	.chan_free = pcc_chan_free,
+	.send_message = pcc_send_message,
+	.fetch_response = pcc_fetch_response,
+	.fetch_notification = pcc_fetch_notification,
+};
+
+static struct scmi_desc scmi_pcc_desc = {
+	.ops = &scmi_pcc_ops,
+	.max_rx_timeout_ms = 30, /* We may increase this if required */
+	.max_msg = 20, /* Limited by MBOX_TX_QUEUE_LEN */
+	.max_msg_size = SCMI_SHMEM_MAX_PAYLOAD_SIZE,
+};
+
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id scmi_acpi_ids[] = {
+	{"ARML0001", 0},
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, scmi_acpi_ids);
+#endif
+
+DEFINE_SCMI_ACPI_TRANSPORT_DRIVER(scmi_pcc, scmi_pcc_driver,
+				  scmi_pcc_desc, scmi_acpi_ids, core);
+module_platform_driver(scmi_pcc_driver);
+
+MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>");
+MODULE_DESCRIPTION("SCMI ACPI PCC Transport driver");
+MODULE_LICENSE("GPL");

-- 
2.34.1

[PATCH 8/8] firmware: arm_scmi: Initialise all protocol devices and transport channels

[Sudeep Holla]

Unlike Device Tree, the ACPI SCMI namespace device does not provide child
fwnodes to represent each protocol. To initialise all protocol devices and
their associated transport channels, we must iterate over the supported DSD
entries defined in the device.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scmi/driver.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index f679a769fc87..9470be5de692 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2786,7 +2786,7 @@ scmi_txrx_setup(struct scmi_info *info, struct fwnode_handle *fwnode,
  */
 static int scmi_channels_setup(struct scmi_info *info)
 {
-	int ret;
+	int ret, idx;
 	struct fwnode_handle *fwnode = dev_fwnode(info->dev);
 
 	/* Initialize a common generic channel at first */
@@ -2809,6 +2809,17 @@ static int scmi_channels_setup(struct scmi_info *info)
 			return ret;
 	}
 
+	if (acpi_disabled)
+		return 0;
+
+	for (idx = 1; idx < ARRAY_SIZE(scmi_dsd_info_list); idx++) {
+		int prot_id = scmi_dsd_info_list[idx].protocol_id;
+
+		ret = scmi_txrx_setup(info, fwnode, prot_id);
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }
 
@@ -3175,7 +3186,7 @@ static void scmi_device_check_create(struct fwnode_handle *fwnode, int prot_id,
 
 static int scmi_probe(struct platform_device *pdev)
 {
-	int ret;
+	int ret, idx;
 	char *err_str = "probe failure\n";
 	struct scmi_handle *handle;
 	const struct scmi_desc *desc;
@@ -3304,6 +3315,15 @@ static int scmi_probe(struct platform_device *pdev)
 		scmi_device_check_create(child, prot_id, info);
 	}
 
+	if (acpi_disabled)
+		return 0;
+
+	for (idx = 1; idx < ARRAY_SIZE(scmi_dsd_info_list); idx++) {
+		int prot_id = scmi_dsd_info_list[idx].protocol_id;
+
+		scmi_device_check_create(dev_fwnode(dev), prot_id, info);
+	}
+
 	return 0;
 
 notification_exit:

-- 
2.34.1

Re: [PATCH 7/8] firmware: arm_scmi: transport: Add ACPI PCC transport

[Dan Carpenter]

On Fri, Oct 17, 2025 at 02:23:50PM +0100, Sudeep Holla wrote:
> +static int pcc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
> +			  bool tx)
> +{
> +	const char *desc = tx ? "Tx" : "Rx";
> +	struct device *cdev = cinfo->dev;
> +	struct scmi_pcc *smbox;
> +	int ret, ss_id;
> +	struct mbox_client *cl;
> +
> +	smbox = devm_kzalloc(dev, sizeof(*smbox), GFP_KERNEL);
> +	if (!smbox)
> +		return -ENOMEM;
> +
> +	cl = &smbox->cl;
> +	cl->dev = cdev;
> +	cl->tx_prepare = tx ? tx_prepare : NULL;
> +	cl->rx_callback = rx_callback;
> +	cl->tx_block = false;
> +
> +	ss_id = pcc_get_ss_id(cinfo->id, tx);
> +	if (ss_id < 0)
> +		return ss_id;
> +
> +	smbox->pchan = pcc_mbox_request_channel(cl, ss_id);
> +	if (IS_ERR(smbox->pchan)) {
> +		ret = PTR_ERR(smbox->pchan);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(cdev,
> +				"failed to request SCMI %s mailbox\n", desc);
> +		return ret;
> +	}

Use the dev_err_probe() function for this:

	smbox->pchan = pcc_mbox_request_channel(cl, ss_id);
	if (IS_ERR(smbox->pchan)
		return dev_err_probe(cdev, PTR_ERR(smbox->pchan),
				     "failed to request SCMI %s mailbox\n",
				     desc);

regards,
dan carpenter

> +
> +	cinfo->transport_info = smbox;
> +	smbox->cinfo = cinfo;
> +
> +	return 0;
> +}

Re: [PATCH 7/8] firmware: arm_scmi: transport: Add ACPI PCC transport

[Sudeep Holla]

On Mon, Oct 20, 2025 at 11:20:08AM +0300, Dan Carpenter wrote:
> On Fri, Oct 17, 2025 at 02:23:50PM +0100, Sudeep Holla wrote:
> > +static int pcc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
> > +			  bool tx)
> > +{
> > +	const char *desc = tx ? "Tx" : "Rx";
> > +	struct device *cdev = cinfo->dev;
> > +	struct scmi_pcc *smbox;
> > +	int ret, ss_id;
> > +	struct mbox_client *cl;
> > +
> > +	smbox = devm_kzalloc(dev, sizeof(*smbox), GFP_KERNEL);
> > +	if (!smbox)
> > +		return -ENOMEM;
> > +
> > +	cl = &smbox->cl;
> > +	cl->dev = cdev;
> > +	cl->tx_prepare = tx ? tx_prepare : NULL;
> > +	cl->rx_callback = rx_callback;
> > +	cl->tx_block = false;
> > +
> > +	ss_id = pcc_get_ss_id(cinfo->id, tx);
> > +	if (ss_id < 0)
> > +		return ss_id;
> > +
> > +	smbox->pchan = pcc_mbox_request_channel(cl, ss_id);
> > +	if (IS_ERR(smbox->pchan)) {
> > +		ret = PTR_ERR(smbox->pchan);
> > +		if (ret != -EPROBE_DEFER)
> > +			dev_err(cdev,
> > +				"failed to request SCMI %s mailbox\n", desc);
> > +		return ret;
> > +	}
> 
> Use the dev_err_probe() function for this:
> 
> 	smbox->pchan = pcc_mbox_request_channel(cl, ss_id);
> 	if (IS_ERR(smbox->pchan)
> 		return dev_err_probe(cdev, PTR_ERR(smbox->pchan),
> 				     "failed to request SCMI %s mailbox\n",
> 				     desc);
> 

Good point, every time when review I think of it but apparently not when
writing code myself 😁, will fix it. Thanks!

-- 
Regards,
Sudeep

Re: [PATCH 1/8] firmware: arm_scmi: Set fwnode for the genrated SCMI platform device

[Jonathan Cameron]

On Fri, 17 Oct 2025 14:23:44 +0100
Sudeep Holla <sudeep.holla@arm.com> wrote:

generated (in patch title).

> Add a call to device_set_node() in the SCMI probe helper to associate
> generated SCMI platform device with the firmware node of its supplier
> transport device.
> 
> This complements device_set_of_node_from_dev() and ensures that
> firmware node information is propagated correctly for both Device Tree
> and non-DT (e.g. ACPI) based systems.
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/firmware/arm_scmi/common.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
> index 07b9e629276d..911941e6885d 100644
> --- a/drivers/firmware/arm_scmi/common.h
> +++ b/drivers/firmware/arm_scmi/common.h
> @@ -473,6 +473,7 @@ static int __tag##_probe(struct platform_device *pdev)			       \
>  		return -ENOMEM;						       \
>  									       \
>  	device_set_of_node_from_dev(&spdev->dev, dev);			       \
> +	device_set_node(&spdev->dev, dev_fwnode(dev));			       \

device_set_node() is supposed to handle both dt and ACPI. So this surprised me
and I went digging.

dev_fnode for acpi is dev->fwnode, for of it is of_fwnode_handle(dev->of_node)
Which is dev->of_node->fwnode

So for acpi this is
	device_set_node(&spdev->dev, dev->fwnode);
which is:
	spdev->dev->fwnode = fwnode;
	spdev->dev->of_node = NULL; 

For dt
	device_set_node(&spdev->dev, dev->of_node->fwnode);
which is
	spdev->dev->fwnode = dev->of_node->fwnode;
	spdev->dev->opf-node = dev->of_node; (via some container of magic)


The device_set_of_node_from_dev(&spdev->dev, dev)
is same as:
	of_node_put(spdev->dev->of_node);
	spdev->dev->of_node = of_node_get(dev->of_node);
	spdev->dev->of_node_reused = true;

So subject to some reference counting that I don't think you need as the
spdev->dev parent is the dev here, the first call does nothing extra.

Maybe I missed something?

Jonathan




>  									       \
>  	strans.supplier = dev;						       \
>  	memcpy(&strans.desc, &(__desc), sizeof(strans.desc));		       \
> 

Re: [PATCH 2/8] firmware: arm_scmi: Extend transport driver macro to support ACPI

[Jonathan Cameron]

On Fri, 17 Oct 2025 14:23:45 +0100
Sudeep Holla <sudeep.holla@arm.com> wrote:

> Extend the SCMI transport driver helper to also support ACPI-based
> systems. Introduce an internal helper macro that accepts both OF and
> ACPI match tables, and expose two wrappers:
> 
>   - DEFINE_SCMI_TRANSPORT_DRIVER(...)      -> DT/OF wrapper (unchanged ABI)
>   - DEFINE_SCMI_ACPI_TRANSPORT_DRIVER(...) -> ACPI wrapper
> 
> For ACPI, the generated platform_driver now sets .acpi_match_table via
> ACPI_PTR() so that builds without CONFIG_ACPI remain safe (becomes NULL).
In general it doesn't normally matter if ACPI_PTR() isn't used.

What specifically is this preventing?  If CONFIG_ACPI isn't set then we'll never
use those paths anyway.  Using ACPI_PTR() tends to need ifdef magic or
__maybe_unused markings that are rarely worth the effort for these tiny
tables.

> The spawned platform_device inherits the parent device’s ACPI companion
> to ensure proper firmware-node association (i.e.subsequent lookups or
> fwnode use see the correct firmware node).
> 
> This keeps existing DT users unchanged and continues to function as
> expected while allowing transports to be probed using the structure
> `acpi_device_id` tables on ACPI platforms.
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/firmware/arm_scmi/common.h | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
> index 911941e6885d..d038fec72360 100644
> --- a/drivers/firmware/arm_scmi/common.h
> +++ b/drivers/firmware/arm_scmi/common.h
> @@ -9,6 +9,7 @@
>  #ifndef _SCMI_COMMON_H
>  #define _SCMI_COMMON_H
>  
> +#include <linux/acpi.h>
>  #include <linux/bitfield.h>
>  #include <linux/completion.h>
>  #include <linux/device.h>
> @@ -453,7 +454,8 @@ struct scmi_transport {
>  	struct scmi_transport_core_operations **core_ops;
>  };
>  
> -#define DEFINE_SCMI_TRANSPORT_DRIVER(__tag, __drv, __desc, __match, __core_ops)\
> +#define __DEFINE_SCMI_TRANSPORT_DRIVER(__tag, __drv, __desc, __of_match,       \
> +					__acpi_match, __core_ops)	       \
>  static void __tag##_dev_free(void *data)				       \
>  {									       \
>  	struct platform_device *spdev = data;				       \
> @@ -474,6 +476,7 @@ static int __tag##_probe(struct platform_device *pdev)			       \
>  									       \
>  	device_set_of_node_from_dev(&spdev->dev, dev);			       \
>  	device_set_node(&spdev->dev, dev_fwnode(dev));			       \
> +	ACPI_COMPANION_SET(&spdev->dev, ACPI_COMPANION(dev));		       \
>  									       \
>  	strans.supplier = dev;						       \
>  	memcpy(&strans.desc, &(__desc), sizeof(strans.desc));		       \
> @@ -498,11 +501,18 @@ err:									       \
>  static struct platform_driver __drv = {					       \
>  	.driver = {							       \
>  		   .name = #__tag "_transport",				       \
> -		   .of_match_table = __match,				       \
> +		   .of_match_table = __of_match,			       \
> +		   .acpi_match_table = ACPI_PTR(__acpi_match),		       \
>  		   },							       \
>  	.probe = __tag##_probe,						       \
>  }
>  
> +#define DEFINE_SCMI_TRANSPORT_DRIVER(__tag, __drv, __desc, __match, __core_ops)\
> +	__DEFINE_SCMI_TRANSPORT_DRIVER(__tag, __drv, __desc, __match, NULL, __core_ops)
> +
> +#define DEFINE_SCMI_ACPI_TRANSPORT_DRIVER(__tag, __drv, __desc, __match, __core_ops)\
> +	__DEFINE_SCMI_TRANSPORT_DRIVER(__tag, __drv, __desc, NULL, __match, __core_ops)
> +
>  void scmi_notification_instance_data_set(const struct scmi_handle *handle,
>  					 void *priv);
>  void *scmi_notification_instance_data_get(const struct scmi_handle *handle);
> 

Re: [PATCH 3/8] firmware: arm_scmi: Convert OF-only paths to generic fwnode in SCMI core

[Jonathan Cameron]

On Fri, 17 Oct 2025 14:23:46 +0100
Sudeep Holla <sudeep.holla@arm.com> wrote:

> Switch SCMI core plumbing from struct device_node* to struct
> fwnode_handle* so transports and core code work with both DT and
> ACPI firmware descriptions.
> 
> This change:
>   - Replaces of_* property lookups with fwnode_property_*() helpers.
>   - Switches child enumeration to
>     fwnode_for_each_available_child_node_scoped().
>   - Plumbs fwnode through the SCMI device creation and channel setup
>     paths and updates transport ->chan_available() signatures to take a
>     fwnode.
>   - Stores the per-protocol child fwnodes in info->active_protocols so
>     the core can later locate the descriptor for a given protocol ID.
>   - Update mailbox/optee/smc/virtio transports to accept fwnode and
>     map to OF nodes where needed
> 
> DT-only transports (mailbox/optee/smc) still parse DT properties by
> mapping the fwnode back to an OF node; on non-DT (e.g. ACPI) systems
> these transports will report no channel available.
> 
> This refactor is a prerequisite for adding an ACPI-first transport like
> PCC and brings the SCMI core closer to DT/ACPI parity. This is a mechanical
> step towards firmware-node neutrality; DT users continue to work unchanged,
> and ACPI paths can be enabled on top.
> 
> No functional change is expected on DT platforms; ACPI platforms can now
> discover and participate in SCMI where a suitable transport is present.
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Hi Sudeep

A few comments inline. The reference counting on fwnodes gets a bit complex in
here so my review more or less skips that bit (it's end of day!)

> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> index bd56a877fdfc..bc5fea11b5db 100644
> --- a/drivers/firmware/arm_scmi/driver.c
> +++ b/drivers/firmware/arm_scmi/driver.c

> @@ -2820,7 +2820,7 @@ static int scmi_chan_destroy(int id, void *p, void *idr)
>  		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);
> +		fwnode_handle_put(dev_fwnode(cinfo->dev));

This may follow on from earlier thing about device_set_node().
I think this is freeing a reference that will never have been gotten if you
follow what I suggest there.  Note that I'm fairly sure it was never
gotten for acpi anyway.  However this might be a different fwnode, I'm lost
on that front.

>  		scmi_device_destroy(info->dev, id, sdev->name);
>  		cinfo->dev = NULL;
>  	}

> @@ -3118,8 +3119,8 @@ static const struct scmi_desc *scmi_transport_setup(struct device *dev)
>  		 trans->desc.max_msg);
>  
>  	/* System wide atomic threshold for atomic ops .. if any */
> -	if (!of_property_read_u32(dev->of_node, "atomic-threshold-us",
> -				  &trans->desc.atomic_threshold))
> +	if (!fwnode_property_read_u32(dev_fwnode(dev), "atomic-threshold-us",

device_property_read_u32() Same for all the other places where the fwnode
is simple dev_fwnode(dev) and there is a suitable helper.


> +				      &trans->desc.atomic_threshold))
>  		dev_info(dev,
>  			 "SCMI System wide atomic threshold set to %u us\n",
>  			 trans->desc.atomic_threshold);

> @@ -3262,10 +3262,10 @@ static int scmi_probe(struct platform_device *pdev)
>  
>  	scmi_enable_matching_quirks(info);
>  
> -	for_each_available_child_of_node(np, child) {
> +	fwnode_for_each_available_child_node_scoped(dev_fwnode(dev), child) {
I don't think there is an exit path in here, so this is functionally the same
as the non scoped version.

Also, if you are gong to use dev_fwnode use
	device_for_each_child_node() and don't worry about the available.
I think the patch merged that made device_for_each_child_node() only
consider the available ones for all firmware types.

>  		u32 prot_id;
>  
> -		if (of_property_read_u32(child, "reg", &prot_id))
> +		if (fwnode_property_read_u32(child, "reg", &prot_id))
>  			continue;
>  
>  		if (!FIELD_FIT(MSG_PROTOCOL_ID_MASK, prot_id))
> @@ -3278,10 +3278,11 @@ static int scmi_probe(struct platform_device *pdev)
>  		}
>  
>  		/*
> -		 * Save this valid DT protocol descriptor amongst
> +		 * Save this valid fwnode protocol descriptor amongst
>  		 * @active_protocols for this SCMI instance/
>  		 */
> -		ret = idr_alloc(&info->active_protocols, child,
> +		ret = idr_alloc(&info->active_protocols,
> +				fwnode_handle_get(child),

This change is a little subtle to be buried in here and I'm fairly sure
it is an unintended functional change.  If idr_alloc() fails the continue
and loop iterator magic, will drop the reference held by the loop but
not this one.  So it will leak a reference.

If this does make sense, do it in a precursor patch before changing away
from of only.

>  				prot_id, prot_id + 1, GFP_KERNEL);
>  		if (ret != prot_id) {
>  			dev_err(dev, "SCMI protocol %d already activated. Skip\n",
> @@ -3289,7 +3290,6 @@ static int scmi_probe(struct platform_device *pdev)
>  			continue;
>  		}
>  
> -		of_node_get(child);
>  		scmi_create_protocol_devices(child, info, prot_id, NULL);
>  	}
>  

Re: [PATCH 7/8] firmware: arm_scmi: transport: Add ACPI PCC transport

[Jonathan Cameron]

On Fri, 17 Oct 2025 14:23:50 +0100
Sudeep Holla <sudeep.holla@arm.com> wrote:

> Introduce a new SCMI transport that uses ACPI PCCT (PCC) subspaces via
> the Linux PCC mailbox layer. The driver parses ACPI _DSD data to map
> protocols to PCC subspace UIDs, supports shared TX/RX channels, and
> optionally sets up a P2A channel for notifications.
> 
> Key points:
> - new CONFIG_ARM_SCMI_TRANSPORT_PCC option
> - integration with SCMI core via scmi_desc and transport ops
> - response/notification fetch from PCC shared memory header/payload
> - ACPI device matching and registration via the ACPI transport macro
> 
> This enables SCMI to be exercised over PCC on ACPI platforms.
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Hi Sudeep,

Just a very quick look in what is definitely a drive by style review
A few things below.

> diff --git a/drivers/firmware/arm_scmi/transports/pcc.c b/drivers/firmware/arm_scmi/transports/pcc.c
> new file mode 100644
> index 000000000000..39ef83e2dfd4
> --- /dev/null
> +++ b/drivers/firmware/arm_scmi/transports/pcc.c
> @@ -0,0 +1,390 @@

> +
> +/**
> + * struct scmi_pcc - Structure representing a SCMI mailbox transport
> + *
> + * @cl: Mailbox Client
> + * @pchan: Transmit/Receive PCC/mailbox channel
> + * @cinfo: SCMI channel info
> + * @shmem: Transmit/Receive shared memory area
run kernel-doc over the file (shmem doesn't exist).

> + */
> +struct scmi_pcc {
> +	struct mbox_client cl;
> +	struct pcc_mbox_chan *pchan;
> +	struct scmi_chan_info *cinfo;
> +};

> +
> +static int acpi_scmi_dsd_parse_protocol_subpackage(const union acpi_object *obj,
> +						   int prot_id)
> +{
> +	u32 uid;
> +	int idx, ret = 0;
> +	struct pcc_transport *p;
> +	unsigned int pkg_cnt = obj->package.count;
> +
> +	if (pkg_cnt > 2) {
> +		pr_warn("Only 2 channels: one Tx and one Rx needed\n");
> +		return -EINVAL;
> +	}
> +
> +	for (idx = 0; idx < pkg_cnt; idx++) {
> +		union acpi_object *pack = &obj->package.elements[idx];
> +
> +		/* Flags(pack->package.elements[1]) must be always 0 for now */
> +		uid = pack->package.elements[0].integer.value;
> +		hash_for_each_possible(pcc_id_hash, p, hnode, uid) {
> +			if (p->flags & SCMI_TRANSPORT_SHARED_CHANNEL) {
> +				pr_info("Invalid! %d channel is shared\n",
> +					p->pcc_ss_id);
> +				ret = -EINVAL;

This breaks out of the hash_for_each_... but the outer loop might continue
and it's just possible pass.  however we leave ret set.  Why not bail out
on first error?  E.g. return -EINVAL; here.

> +				break;
> +			}
> +			p->protocol_id = prot_id;
> +		}
> +	}
> +
> +	return ret;
> +}

> +#ifdef CONFIG_ACPI

This is the bit I'd avoid by not use ACPI_PTR() in the earlier patch.

> +static const struct acpi_device_id scmi_acpi_ids[] = {
> +	{"ARML0001", 0},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(acpi, scmi_acpi_ids);
> +#endif
> +
> +DEFINE_SCMI_ACPI_TRANSPORT_DRIVER(scmi_pcc, scmi_pcc_driver,
> +				  scmi_pcc_desc, scmi_acpi_ids, core);
> +module_platform_driver(scmi_pcc_driver);
> +
> +MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>");
> +MODULE_DESCRIPTION("SCMI ACPI PCC Transport driver");
> +MODULE_LICENSE("GPL");
> 

Re: [PATCH 1/8] firmware: arm_scmi: Set fwnode for the genrated SCMI platform device

[Sudeep Holla]

On Mon, Oct 20, 2025 at 06:07:02PM +0100, Jonathan Cameron wrote:
> On Fri, 17 Oct 2025 14:23:44 +0100
> Sudeep Holla <sudeep.holla@arm.com> wrote:
> 
> generated (in patch title).
> 
> > Add a call to device_set_node() in the SCMI probe helper to associate
> > generated SCMI platform device with the firmware node of its supplier
> > transport device.
> > 
> > This complements device_set_of_node_from_dev() and ensures that
> > firmware node information is propagated correctly for both Device Tree
> > and non-DT (e.g. ACPI) based systems.
> > 
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> >  drivers/firmware/arm_scmi/common.h | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
> > index 07b9e629276d..911941e6885d 100644
> > --- a/drivers/firmware/arm_scmi/common.h
> > +++ b/drivers/firmware/arm_scmi/common.h
> > @@ -473,6 +473,7 @@ static int __tag##_probe(struct platform_device *pdev)			       \
> >  		return -ENOMEM;						       \
> >  									       \
> >  	device_set_of_node_from_dev(&spdev->dev, dev);			       \
> > +	device_set_node(&spdev->dev, dev_fwnode(dev));			       \
> 
> device_set_node() is supposed to handle both dt and ACPI. So this surprised me
> and I went digging.
> 
> dev_fnode for acpi is dev->fwnode, for of it is of_fwnode_handle(dev->of_node)
> Which is dev->of_node->fwnode
> 
> So for acpi this is
> 	device_set_node(&spdev->dev, dev->fwnode);
> which is:
> 	spdev->dev->fwnode = fwnode;
> 	spdev->dev->of_node = NULL; 
> 
> For dt
> 	device_set_node(&spdev->dev, dev->of_node->fwnode);
> which is
> 	spdev->dev->fwnode = dev->of_node->fwnode;
> 	spdev->dev->opf-node = dev->of_node; (via some container of magic)
> 
> 
> The device_set_of_node_from_dev(&spdev->dev, dev)
> is same as:
> 	of_node_put(spdev->dev->of_node);
> 	spdev->dev->of_node = of_node_get(dev->of_node);
> 	spdev->dev->of_node_reused = true;
> 
> So subject to some reference counting that I don't think you need as the
> spdev->dev parent is the dev here, the first call does nothing extra.
> 
> Maybe I missed something?
> 

No, you didn’t miss anything. You’ve articulated my analysis well - the same
points I had intended to include in my commit messages but had forgotten those
details of at the time of writing the commit message.

I retained the call to device_set_of_node_from_dev() primarily because it sets
dev->of_node_reused = true.

I’m not entirely certain about the implications of not setting this flag, and
it wouldn’t be set if we used only device_set_node().

Thank you for noticing this and bringing it up for discussion. I had planned
to raise this when I first observed it and made the change, but unfortunately
didn’t document it properly - that would have saved you from having to
rediscover the details yourself.

I’m fine with whichever solution we agree on. I retained both calls
intentionally to prompt exactly this kind of discussion.

-- 
Regards,
Sudeep

Re: [PATCH 2/8] firmware: arm_scmi: Extend transport driver macro to support ACPI

[Sudeep Holla]

On Mon, Oct 20, 2025 at 06:11:06PM +0100, Jonathan Cameron wrote:
> On Fri, 17 Oct 2025 14:23:45 +0100
> Sudeep Holla <sudeep.holla@arm.com> wrote:
> 
> > Extend the SCMI transport driver helper to also support ACPI-based
> > systems. Introduce an internal helper macro that accepts both OF and
> > ACPI match tables, and expose two wrappers:
> > 
> >   - DEFINE_SCMI_TRANSPORT_DRIVER(...)      -> DT/OF wrapper (unchanged ABI)
> >   - DEFINE_SCMI_ACPI_TRANSPORT_DRIVER(...) -> ACPI wrapper
> > 
> > For ACPI, the generated platform_driver now sets .acpi_match_table via
> > ACPI_PTR() so that builds without CONFIG_ACPI remain safe (becomes NULL).
> In general it doesn't normally matter if ACPI_PTR() isn't used.
> 
> What specifically is this preventing?  If CONFIG_ACPI isn't set then we'll never
> use those paths anyway.  Using ACPI_PTR() tends to need ifdef magic or
> __maybe_unused markings that are rarely worth the effort for these tiny
> tables.
> 

You are right, my bad. I had assumed acpi_device_id is defined only when
CONFIG_ACPI=y which clearly is not the case. I will update the last patch
and this commit message accordingly.

-- 
Regards,
Sudeep

Re: [PATCH 3/8] firmware: arm_scmi: Convert OF-only paths to generic fwnode in SCMI core

[Sudeep Holla]

On Mon, Oct 20, 2025 at 06:29:49PM +0100, Jonathan Cameron wrote:
> On Fri, 17 Oct 2025 14:23:46 +0100
> Sudeep Holla <sudeep.holla@arm.com> wrote:
> 
> > Switch SCMI core plumbing from struct device_node* to struct
> > fwnode_handle* so transports and core code work with both DT and
> > ACPI firmware descriptions.
> > 
> > This change:
> >   - Replaces of_* property lookups with fwnode_property_*() helpers.
> >   - Switches child enumeration to
> >     fwnode_for_each_available_child_node_scoped().
> >   - Plumbs fwnode through the SCMI device creation and channel setup
> >     paths and updates transport ->chan_available() signatures to take a
> >     fwnode.
> >   - Stores the per-protocol child fwnodes in info->active_protocols so
> >     the core can later locate the descriptor for a given protocol ID.
> >   - Update mailbox/optee/smc/virtio transports to accept fwnode and
> >     map to OF nodes where needed
> > 
> > DT-only transports (mailbox/optee/smc) still parse DT properties by
> > mapping the fwnode back to an OF node; on non-DT (e.g. ACPI) systems
> > these transports will report no channel available.
> > 
> > This refactor is a prerequisite for adding an ACPI-first transport like
> > PCC and brings the SCMI core closer to DT/ACPI parity. This is a mechanical
> > step towards firmware-node neutrality; DT users continue to work unchanged,
> > and ACPI paths can be enabled on top.
> > 
> > No functional change is expected on DT platforms; ACPI platforms can now
> > discover and participate in SCMI where a suitable transport is present.
> > 
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> Hi Sudeep
> 
> A few comments inline. The reference counting on fwnodes gets a bit complex in
> here so my review more or less skips that bit (it's end of day!)
> 

No, I had exactly same thoughts and I did mention about this briefly to
Cristian before posting as I couldn't follow the existing refcounting 🙁.
I need to catch up with Cristian and align ourselves before the next version.

> > diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> > index bd56a877fdfc..bc5fea11b5db 100644
> > --- a/drivers/firmware/arm_scmi/driver.c
> > +++ b/drivers/firmware/arm_scmi/driver.c
> 
> > @@ -2820,7 +2820,7 @@ static int scmi_chan_destroy(int id, void *p, void *idr)
> >  		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);
> > +		fwnode_handle_put(dev_fwnode(cinfo->dev));
> 
> This may follow on from earlier thing about device_set_node().
> I think this is freeing a reference that will never have been gotten if you
> follow what I suggest there.  Note that I'm fairly sure it was never
> gotten for acpi anyway.  However this might be a different fwnode, I'm lost
> on that front.
> 

Even I was lost and wanted to revisit this whole reference counting once again
before these changes find its way upstream.

> >  		scmi_device_destroy(info->dev, id, sdev->name);
> >  		cinfo->dev = NULL;
> >  	}
> 
> > @@ -3118,8 +3119,8 @@ static const struct scmi_desc *scmi_transport_setup(struct device *dev)
> >  		 trans->desc.max_msg);
> >  
> >  	/* System wide atomic threshold for atomic ops .. if any */
> > -	if (!of_property_read_u32(dev->of_node, "atomic-threshold-us",
> > -				  &trans->desc.atomic_threshold))
> > +	if (!fwnode_property_read_u32(dev_fwnode(dev), "atomic-threshold-us",
> 
> device_property_read_u32() Same for all the other places where the fwnode
> is simple dev_fwnode(dev) and there is a suitable helper.
> 

Ah OK, didn't notice that, thanks for the pointer.

> 
> > +				      &trans->desc.atomic_threshold))
> >  		dev_info(dev,
> >  			 "SCMI System wide atomic threshold set to %u us\n",
> >  			 trans->desc.atomic_threshold);
> 
> > @@ -3262,10 +3262,10 @@ static int scmi_probe(struct platform_device *pdev)
> >  
> >  	scmi_enable_matching_quirks(info);
> >  
> > -	for_each_available_child_of_node(np, child) {
> > +	fwnode_for_each_available_child_node_scoped(dev_fwnode(dev), child) {
> I don't think there is an exit path in here, so this is functionally the same
> as the non scoped version.
>

I get you point.

> Also, if you are gong to use dev_fwnode use
> 	device_for_each_child_node() and don't worry about the available.
> I think the patch merged that made device_for_each_child_node() only
> consider the available ones for all firmware types.
> 

Not sure if I fully understand yet, but will revisit this again before
next version.

> >  		u32 prot_id;
> >  
> > -		if (of_property_read_u32(child, "reg", &prot_id))
> > +		if (fwnode_property_read_u32(child, "reg", &prot_id))
> >  			continue;
> >  
> >  		if (!FIELD_FIT(MSG_PROTOCOL_ID_MASK, prot_id))
> > @@ -3278,10 +3278,11 @@ static int scmi_probe(struct platform_device *pdev)
> >  		}
> >  
> >  		/*
> > -		 * Save this valid DT protocol descriptor amongst
> > +		 * Save this valid fwnode protocol descriptor amongst
> >  		 * @active_protocols for this SCMI instance/
> >  		 */
> > -		ret = idr_alloc(&info->active_protocols, child,
> > +		ret = idr_alloc(&info->active_protocols,
> > +				fwnode_handle_get(child),
> 
> This change is a little subtle to be buried in here and I'm fairly sure
> it is an unintended functional change.  If idr_alloc() fails the continue
> and loop iterator magic, will drop the reference held by the loop but
> not this one.  So it will leak a reference.
> 
> If this does make sense, do it in a precursor patch before changing away
> from of only.
> 

Indeed, I can't recall why I have added the reference counting here. It can
be dropped, will give it another thought to see if anything strikes my mind.

> >  				prot_id, prot_id + 1, GFP_KERNEL);
> >  		if (ret != prot_id) {
> >  			dev_err(dev, "SCMI protocol %d already activated. Skip\n",
> > @@ -3289,7 +3290,6 @@ static int scmi_probe(struct platform_device *pdev)
> >  			continue;
> >  		}
> >  
> > -		of_node_get(child);
> >  		scmi_create_protocol_devices(child, info, prot_id, NULL);
> >  	}
> >  
> 

-- 
Regards,
Sudeep

Re: [PATCH 7/8] firmware: arm_scmi: transport: Add ACPI PCC transport

[Sudeep Holla]

On Mon, Oct 20, 2025 at 06:37:31PM +0100, Jonathan Cameron wrote:
> On Fri, 17 Oct 2025 14:23:50 +0100
> Sudeep Holla <sudeep.holla@arm.com> wrote:
> 
> > Introduce a new SCMI transport that uses ACPI PCCT (PCC) subspaces via
> > the Linux PCC mailbox layer. The driver parses ACPI _DSD data to map
> > protocols to PCC subspace UIDs, supports shared TX/RX channels, and
> > optionally sets up a P2A channel for notifications.
> > 
> > Key points:
> > - new CONFIG_ARM_SCMI_TRANSPORT_PCC option
> > - integration with SCMI core via scmi_desc and transport ops
> > - response/notification fetch from PCC shared memory header/payload
> > - ACPI device matching and registration via the ACPI transport macro
> > 
> > This enables SCMI to be exercised over PCC on ACPI platforms.
> > 
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> Hi Sudeep,
> 
> Just a very quick look in what is definitely a drive by style review
> A few things below.
> 

Thanks for the quick look though, you have found quite a few things to
fix 😄.

-- 
Regards,
Sudeep

Re: [PATCH 0/8] firmware: arm_scmi: Refactoring and enablement of ACPI PCC transport

[Punit Agrawal]

Hi Sudeep,

Sudeep Holla <sudeep.holla@arm.com> writes:

> The SCMI can be utilized in systems using either the FDT or ACPI specification.
> While FDT-based systems can natively use SCMI, ACPI-based systems often
> need to abstract the functionality provided by SCMI under ASL methods.
> So far, there has been no need to support SCMI natively on ACPI systems.
>
> However, with the addition of a few new protocols such as Powercap and Telemetry,
> which lack abstractions in the ACPI specification, there is now a need to
> run SCMI natively for those use cases.
>
> This patch series introduces ACPI PCC transport support for the Arm SCMI
> framework, alongside several foundational refactors and enhancements to
> achieve firmware-node neutrality between Device Tree (DT) and ACPI systems.
>
> The key changes include:
>
> 1. ACPI/DT abstraction and fwnode transition
>
>    Converted the core SCMI code to use `fwnode_handle` instead of DT-specific
>    structures, ensuring seamless operation across both ACPI and DT
>    environments. All property lookups, child enumeration, and device
>    association paths have been updated accordingly.
>
> 2. Unified transport registration for ACPI and DT
>
>    Extended the SCMI transport driver macros to support ACPI match tables,
>    enabling transports to probe using ACPI device IDs while maintaining
>    backward compatibility with DT-only systems.
>
> 3. Protocol device initialization and refactoring
>
>    Refactored the protocol device creation and validation logic into a new
>    helper for improved readability and maintainability. Enhanced the
>    initialization logic to handle ACPI-based SCMI devices without explicit
>    child fwnodes.
>
> 4. Introduction of ACPI PCC transport
>
>    Added a new SCMI transport driver leveraging ACPI PCCT (Platform
>    Communications Channel Table) subspaces via the Linux PCC mailbox
>    framework. This enables SCMI communication over PCC on ACPI-based
>    platforms.
>
> Collectively, these changes lay the groundwork for robust SCMI operation on
> ACPI platforms, achieving near parity with DT systems where applicable,
> while enabling the new PCC transport path for firmware communication.

I was interested in taking a closer look at the patches here but they
have a dependency on support for the telemetry protocol patches. I get a
compile error.

I was wondering if you have a branch with the dependencies included. Are
the v1 telemetry support patches[0] the right version to be using?

Thanks,
Punit

[0] https://lore.kernel.org/all/20250925203554.482371-1-cristian.marussi@arm.com/


[...]

Re: [PATCH 0/8] firmware: arm_scmi: Refactoring and enablement of ACPI PCC transport

[Sudeep Holla]

On Wed, Nov 05, 2025 at 11:49:38AM +0000, Punit Agrawal wrote:
> Hi Sudeep,
> 
> Sudeep Holla <sudeep.holla@arm.com> writes:
> 
> > The SCMI can be utilized in systems using either the FDT or ACPI specification.
> > While FDT-based systems can natively use SCMI, ACPI-based systems often
> > need to abstract the functionality provided by SCMI under ASL methods.
> > So far, there has been no need to support SCMI natively on ACPI systems.
> >
> > However, with the addition of a few new protocols such as Powercap and Telemetry,
> > which lack abstractions in the ACPI specification, there is now a need to
> > run SCMI natively for those use cases.
> >
> > This patch series introduces ACPI PCC transport support for the Arm SCMI
> > framework, alongside several foundational refactors and enhancements to
> > achieve firmware-node neutrality between Device Tree (DT) and ACPI systems.
> >
> > The key changes include:
> >
> > 1. ACPI/DT abstraction and fwnode transition
> >
> >    Converted the core SCMI code to use `fwnode_handle` instead of DT-specific
> >    structures, ensuring seamless operation across both ACPI and DT
> >    environments. All property lookups, child enumeration, and device
> >    association paths have been updated accordingly.
> >
> > 2. Unified transport registration for ACPI and DT
> >
> >    Extended the SCMI transport driver macros to support ACPI match tables,
> >    enabling transports to probe using ACPI device IDs while maintaining
> >    backward compatibility with DT-only systems.
> >
> > 3. Protocol device initialization and refactoring
> >
> >    Refactored the protocol device creation and validation logic into a new
> >    helper for improved readability and maintainability. Enhanced the
> >    initialization logic to handle ACPI-based SCMI devices without explicit
> >    child fwnodes.
> >
> > 4. Introduction of ACPI PCC transport
> >
> >    Added a new SCMI transport driver leveraging ACPI PCCT (Platform
> >    Communications Channel Table) subspaces via the Linux PCC mailbox
> >    framework. This enables SCMI communication over PCC on ACPI-based
> >    platforms.
> >
> > Collectively, these changes lay the groundwork for robust SCMI operation on
> > ACPI platforms, achieving near parity with DT systems where applicable,
> > while enabling the new PCC transport path for firmware communication.
> 
> I was interested in taking a closer look at the patches here but they
> have a dependency on support for the telemetry protocol patches. I get a
> compile error.
> 
> I was wondering if you have a branch with the dependencies included. Are
> the v1 telemetry support patches[0] the right version to be using?
>

I can create one but that is not strictly necessary. As mentioned in the
cover letter, the only dependency is on value of SCMI_PROTOCOL_TELEMETRY
ins `scmi_std_protocol` enumeration. Just add one and you must be able to
compile the series independent of Cristian series.

-- 
Regards,
Sudeep

Re: [PATCH 0/8] firmware: arm_scmi: Refactoring and enablement of ACPI PCC transport

[Punit Agrawal]

Hi Sudeep,

Sudeep Holla <sudeep.holla@arm.com> writes:

> On Wed, Nov 05, 2025 at 11:49:38AM +0000, Punit Agrawal wrote:
>> Hi Sudeep,
>> 
>> Sudeep Holla <sudeep.holla@arm.com> writes:
>> 
>> > The SCMI can be utilized in systems using either the FDT or ACPI specification.
>> > While FDT-based systems can natively use SCMI, ACPI-based systems often
>> > need to abstract the functionality provided by SCMI under ASL methods.
>> > So far, there has been no need to support SCMI natively on ACPI systems.
>> >
>> > However, with the addition of a few new protocols such as Powercap and Telemetry,
>> > which lack abstractions in the ACPI specification, there is now a need to
>> > run SCMI natively for those use cases.
>> >
>> > This patch series introduces ACPI PCC transport support for the Arm SCMI
>> > framework, alongside several foundational refactors and enhancements to
>> > achieve firmware-node neutrality between Device Tree (DT) and ACPI systems.
>> >
>> > The key changes include:
>> >
>> > 1. ACPI/DT abstraction and fwnode transition
>> >
>> >    Converted the core SCMI code to use `fwnode_handle` instead of DT-specific
>> >    structures, ensuring seamless operation across both ACPI and DT
>> >    environments. All property lookups, child enumeration, and device
>> >    association paths have been updated accordingly.
>> >
>> > 2. Unified transport registration for ACPI and DT
>> >
>> >    Extended the SCMI transport driver macros to support ACPI match tables,
>> >    enabling transports to probe using ACPI device IDs while maintaining
>> >    backward compatibility with DT-only systems.
>> >
>> > 3. Protocol device initialization and refactoring
>> >
>> >    Refactored the protocol device creation and validation logic into a new
>> >    helper for improved readability and maintainability. Enhanced the
>> >    initialization logic to handle ACPI-based SCMI devices without explicit
>> >    child fwnodes.
>> >
>> > 4. Introduction of ACPI PCC transport
>> >
>> >    Added a new SCMI transport driver leveraging ACPI PCCT (Platform
>> >    Communications Channel Table) subspaces via the Linux PCC mailbox
>> >    framework. This enables SCMI communication over PCC on ACPI-based
>> >    platforms.
>> >
>> > Collectively, these changes lay the groundwork for robust SCMI operation on
>> > ACPI platforms, achieving near parity with DT systems where applicable,
>> > while enabling the new PCC transport path for firmware communication.
>> 
>> I was interested in taking a closer look at the patches here but they
>> have a dependency on support for the telemetry protocol patches. I get a
>> compile error.
>> 
>> I was wondering if you have a branch with the dependencies included. Are
>> the v1 telemetry support patches[0] the right version to be using?
>>
>
> I can create one but that is not strictly necessary. As mentioned in the
> cover letter, the only dependency is on value of SCMI_PROTOCOL_TELEMETRY
> ins `scmi_std_protocol` enumeration. Just add one and you must be able to
> compile the series independent of Cristian series.

Thanks, I managed to get the patches building. Looking forward to the
series progressing. If possible, do keep me in the loop for future
updates.

Re: [PATCH 0/8] firmware: arm_scmi: Refactoring and enablement of ACPI PCC transport

[Sudeep Holla]

On Wed, Dec 03, 2025 at 11:04:09AM +0000, Punit Agrawal wrote:
> Hi Sudeep,
> 
> Sudeep Holla <sudeep.holla@arm.com> writes:
> 
> > I can create one but that is not strictly necessary. As mentioned in the
> > cover letter, the only dependency is on value of SCMI_PROTOCOL_TELEMETRY
> > ins `scmi_std_protocol` enumeration. Just add one and you must be able to
> > compile the series independent of Cristian series.
> 
> Thanks, I managed to get the patches building. Looking forward to the
> series progressing. If possible, do keep me in the loop for future
> updates.

Sure - I’ll try to remember to copy you on the next updates. I don’t have any
immediate plans, though, unless the spec moves into beta, changes come up, or
someone specifically requests important updates that would help them make
progress.

Do you have immediate plans to use this on a platform ? Or you are reviewing
just out of interest/curiosity 😉 ?

-- 
Regards,
Sudeep

Re: [PATCH 0/8] firmware: arm_scmi: Refactoring and enablement of ACPI PCC transport

[Punit Agrawal]

Sudeep Holla <sudeep.holla@arm.com> writes:

> On Wed, Dec 03, 2025 at 11:04:09AM +0000, Punit Agrawal wrote:
>> Hi Sudeep,
>> 
>> Sudeep Holla <sudeep.holla@arm.com> writes:
>> 
>> > I can create one but that is not strictly necessary. As mentioned in the
>> > cover letter, the only dependency is on value of SCMI_PROTOCOL_TELEMETRY
>> > ins `scmi_std_protocol` enumeration. Just add one and you must be able to
>> > compile the series independent of Cristian series.
>> 
>> Thanks, I managed to get the patches building. Looking forward to the
>> series progressing. If possible, do keep me in the loop for future
>> updates.
>
> Sure - I’ll try to remember to copy you on the next updates. I don’t have any
> immediate plans, though, unless the spec moves into beta, changes come up, or
> someone specifically requests important updates that would help them make
> progress.
>
> Do you have immediate plans to use this on a platform ? Or you are reviewing
> just out of interest/curiosity 😉 ?

Ah right - although I'd read it in the cover letter it slipped my mind.

I am hoping to migrate some prototype code on top of your patches. Only
worry is if there are big changes planned in your patches. I should be
able to use the posted version for now.