[PATCH 0/8] Make SCMI transport as standalone drivers

[PATCH 0/8] Make SCMI transport as standalone drivers

[Cristian Marussi]

Hi all,

SCMI transport layer was till now being embedded built into in the core
SCMI stack.

Some of these transports, despite being currently part of the main SCMI
module, are indeed also registered with different subsystems like optee or
virtio, and actvely probed also by those: this led to a few awkward and
convoluted tricks to properly handle such interactions at boot time in the
SCMI stack.

Moreover some partner expressed the desire to be able to fully modularize
the transports components.

This series aim to make all such transports as standalone drivers that can
be optionally loaded as modules.

In order to do this, at first some new mechanism is introduced to support
this new capability while maintaining, in parallel, the old legacy embedded
transports; then each transport, one by one, is transitioned to be a
standalone driver and finally the old legacy support for embedded transport
is removed.

Patch [1/8] is a mostly unrelated (but much needed) clean-up from Peng,
which I included in this series to avoid conflicts at merge.

Patch [2/8] simply collects the existing datagram manipulation helpers in a
pair of function pointers structures, in preparation for later reworks.

Patch [3/8] adds the bulk of the new logic to the core SCMI stack and then
each existing transport is transitioned to be a standalone driver in
patches 4,5,6,7 while shuffling around the compatibles. (no DT change is
needed of curse for backward compatibility)
While doing this I kept the module authorship in line with the main
author(S) as spitted out by git-blame.

Finally patch [8/8] removes all the legacy dead code from the core SCMI
stack.

No new symbol EXPORT has been added.

The new transport drivers have been tested, as built-in and LKM, as
follows:

- mailbox on JUNO
- virtio on emulation
- optee on QEMU/optee using Linaro setup

Exercised using the regular SCMI drivers stack and the SCMI ACS suite,
testing commands, replies, delayed responses and notification.

Multiple virtual SCMI instances support has been tested too.

SMC has NOT been tested/exercised at run-time, only compile-tested.
(due to lack of hardware)

Note that in this new setup, all the probe deferral and retries between the
SCMI core stack and the transports has been removed, since no more needed.

Moreover the new drivers have been tested also with a fully modularized
SCMI stack, i.e.:

  scmi-core.ko + scmi-module.ko + scmi_transport_*.ko [ + vendor modules ]

One thing that I have NOT done still, but that it's an easy pick, would
be to completely remove any dependency between the SCMI core stack and the
transports, leaving these latter only dependent on the mere presence of
the related subsystems like optee/virtio/mailbox/smc: this would allow to
compile the transports and the related subsys as built-in while keeping the
core SCMI stack as a module....not sure if it makes sense... but it can be
already easily done by simply patching further the Kconfig.

Based on sudeep/for-next/scmi/updates.

Any feedback, and especially testing (:D) is welcome.

Thanks,
Cristian

Cristian Marussi (7):
  firmware: arm_scmi: Introduce packet handling helpers
  firmware: arm_scmi: Add support for standalone transport drivers
  firmware: arm_scmi: Make MBOX transport a standalone driver
  firmware: arm_scmi: Make SMC transport a standalone driver
  firmware: arm_scmi: Make OPTEE transport a standalone driver
  firmware: arm_scmi: Make VirtIO transport a standalone driver
  firmware: arm_scmi: Remove legacy transport-layer code

Peng Fan (1):
  firmware: arm_scmi: Introduce setup_shmem_iomap

 drivers/firmware/arm_scmi/Kconfig             |  20 +-
 drivers/firmware/arm_scmi/Makefile            |   9 +-
 drivers/firmware/arm_scmi/common.h            | 183 +++++++++++++-----
 drivers/firmware/arm_scmi/driver.c            | 140 +++++---------
 drivers/firmware/arm_scmi/msg.c               |  34 +++-
 .../{mailbox.c => scmi_transport_mailbox.c}   |  69 ++++---
 .../{optee.c => scmi_transport_optee.c}       | 120 +++++-------
 .../arm_scmi/{smc.c => scmi_transport_smc.c}  |  52 ++---
 .../{virtio.c => scmi_transport_virtio.c}     |  95 +++++----
 drivers/firmware/arm_scmi/shmem.c             |  80 ++++++--
 10 files changed, 445 insertions(+), 357 deletions(-)
 rename drivers/firmware/arm_scmi/{mailbox.c => scmi_transport_mailbox.c} (87%)
 rename drivers/firmware/arm_scmi/{optee.c => scmi_transport_optee.c} (89%)
 rename drivers/firmware/arm_scmi/{smc.c => scmi_transport_smc.c} (88%)
 rename drivers/firmware/arm_scmi/{virtio.c => scmi_transport_virtio.c} (95%)

-- 
2.45.2

[PATCH 1/8] firmware: arm_scmi: Introduce setup_shmem_iomap

[Cristian Marussi]

From: Peng Fan <peng.fan@nxp.com>

To get the address of shmem could be generalized by introducing
setup_shmem_iomap. Then the duplicated code in mailbox.c, optee.c
and smc.c could be dropped.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
[ Cristian: make use of the new helper also in smc.c ]
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
 drivers/firmware/arm_scmi/common.h  |  2 ++
 drivers/firmware/arm_scmi/mailbox.c | 27 ++++------------------
 drivers/firmware/arm_scmi/optee.c   | 35 ++++------------------------
 drivers/firmware/arm_scmi/shmem.c   | 36 +++++++++++++++++++++++++++++
 drivers/firmware/arm_scmi/smc.c     | 23 +++---------------
 5 files changed, 49 insertions(+), 74 deletions(-)

diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 4b8c5250cdb5..b4c217641e3a 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -327,6 +327,8 @@ bool shmem_poll_done(struct scmi_shared_mem __iomem *shmem,
 		     struct scmi_xfer *xfer);
 bool shmem_channel_free(struct scmi_shared_mem __iomem *shmem);
 bool shmem_channel_intr_enabled(struct scmi_shared_mem __iomem *shmem);
+void __iomem *setup_shmem_iomap(struct scmi_chan_info *cinfo, struct device *dev,
+				bool tx);
 
 /* declarations for message passing transports */
 struct scmi_msg_payld;
diff --git a/drivers/firmware/arm_scmi/mailbox.c b/drivers/firmware/arm_scmi/mailbox.c
index 0219a12e3209..b0a579f31905 100644
--- a/drivers/firmware/arm_scmi/mailbox.c
+++ b/drivers/firmware/arm_scmi/mailbox.c
@@ -178,11 +178,8 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
 	const char *desc = tx ? "Tx" : "Rx";
 	struct device *cdev = cinfo->dev;
 	struct scmi_mailbox *smbox;
-	struct device_node *shmem;
-	int ret, a2p_rx_chan, p2a_chan, p2a_rx_chan, idx = tx ? 0 : 1;
+	int ret, a2p_rx_chan, p2a_chan, p2a_rx_chan;
 	struct mbox_client *cl;
-	resource_size_t size;
-	struct resource res;
 
 	ret = mailbox_chan_validate(cdev, &a2p_rx_chan, &p2a_chan, &p2a_rx_chan);
 	if (ret)
@@ -195,25 +192,9 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
 	if (!smbox)
 		return -ENOMEM;
 
-	shmem = of_parse_phandle(cdev->of_node, "shmem", idx);
-	if (!of_device_is_compatible(shmem, "arm,scmi-shmem")) {
-		of_node_put(shmem);
-		return -ENXIO;
-	}
-
-	ret = of_address_to_resource(shmem, 0, &res);
-	of_node_put(shmem);
-	if (ret) {
-		dev_err(cdev, "failed to get SCMI %s shared memory\n", desc);
-		return ret;
-	}
-
-	size = resource_size(&res);
-	smbox->shmem = devm_ioremap(dev, res.start, size);
-	if (!smbox->shmem) {
-		dev_err(dev, "failed to ioremap SCMI %s shared memory\n", desc);
-		return -EADDRNOTAVAIL;
-	}
+	smbox->shmem = setup_shmem_iomap(cinfo, dev, tx);
+	if (IS_ERR(smbox->shmem))
+		return PTR_ERR(smbox->shmem);
 
 	cl = &smbox->cl;
 	cl->dev = cdev;
diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c
index 4e7944b91e38..8abcd668108c 100644
--- a/drivers/firmware/arm_scmi/optee.c
+++ b/drivers/firmware/arm_scmi/optee.c
@@ -368,38 +368,11 @@ static int setup_dynamic_shmem(struct device *dev, struct scmi_optee_channel *ch
 static int setup_static_shmem(struct device *dev, struct scmi_chan_info *cinfo,
 			      struct scmi_optee_channel *channel)
 {
-	struct device_node *np;
-	resource_size_t size;
-	struct resource res;
-	int ret;
-
-	np = of_parse_phandle(cinfo->dev->of_node, "shmem", 0);
-	if (!of_device_is_compatible(np, "arm,scmi-shmem")) {
-		ret = -ENXIO;
-		goto out;
-	}
-
-	ret = of_address_to_resource(np, 0, &res);
-	if (ret) {
-		dev_err(dev, "Failed to get SCMI Tx shared memory\n");
-		goto out;
-	}
-
-	size = resource_size(&res);
+	channel->req.shmem = setup_shmem_iomap(cinfo, dev, true);
+	if (IS_ERR(channel->req.shmem))
+		return PTR_ERR(channel->req.shmem);
 
-	channel->req.shmem = devm_ioremap(dev, res.start, size);
-	if (!channel->req.shmem) {
-		dev_err(dev, "Failed to ioremap SCMI Tx shared memory\n");
-		ret = -EADDRNOTAVAIL;
-		goto out;
-	}
-
-	ret = 0;
-
-out:
-	of_node_put(np);
-
-	return ret;
+	return 0;
 }
 
 static int setup_shmem(struct device *dev, struct scmi_chan_info *cinfo,
diff --git a/drivers/firmware/arm_scmi/shmem.c b/drivers/firmware/arm_scmi/shmem.c
index b74e5a740f2c..c31f188d74ef 100644
--- a/drivers/firmware/arm_scmi/shmem.c
+++ b/drivers/firmware/arm_scmi/shmem.c
@@ -7,6 +7,8 @@
 
 #include <linux/ktime.h>
 #include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/processor.h>
 #include <linux/types.h>
 
@@ -133,3 +135,37 @@ bool shmem_channel_intr_enabled(struct scmi_shared_mem __iomem *shmem)
 {
 	return ioread32(&shmem->flags) & SCMI_SHMEM_FLAG_INTR_ENABLED;
 }
+
+void *__iomem
+setup_shmem_iomap(struct scmi_chan_info *cinfo, struct device *dev, bool tx)
+{
+	const char *desc = tx ? "Tx" : "Rx";
+	int ret, idx = tx ? 0 : 1;
+	struct device *cdev = cinfo->dev;
+	struct device_node *shmem;
+	resource_size_t size;
+	struct resource res;
+	void __iomem *addr;
+
+	shmem = of_parse_phandle(cdev->of_node, "shmem", idx);
+	if (!of_device_is_compatible(shmem, "arm,scmi-shmem")) {
+		of_node_put(shmem);
+		return ERR_PTR(-ENXIO);
+	}
+
+	ret = of_address_to_resource(shmem, 0, &res);
+	of_node_put(shmem);
+	if (ret) {
+		dev_err(cdev, "failed to get SCMI %s shared memory\n", desc);
+		return ERR_PTR(ret);
+	}
+
+	size = resource_size(&res);
+	addr = devm_ioremap(dev, res.start, size);
+	if (!addr) {
+		dev_err(dev, "failed to ioremap SCMI %s shared memory\n", desc);
+		return ERR_PTR(-EADDRNOTAVAIL);
+	}
+
+	return addr;
+}
diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c
index 39936e1dd30e..a640a4312472 100644
--- a/drivers/firmware/arm_scmi/smc.c
+++ b/drivers/firmware/arm_scmi/smc.c
@@ -132,7 +132,6 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
 	struct scmi_smc *scmi_info;
 	resource_size_t size;
 	struct resource res;
-	struct device_node *np;
 	u32 func_id;
 	int ret;
 
@@ -143,25 +142,9 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
 	if (!scmi_info)
 		return -ENOMEM;
 
-	np = of_parse_phandle(cdev->of_node, "shmem", 0);
-	if (!of_device_is_compatible(np, "arm,scmi-shmem")) {
-		of_node_put(np);
-		return -ENXIO;
-	}
-
-	ret = of_address_to_resource(np, 0, &res);
-	of_node_put(np);
-	if (ret) {
-		dev_err(cdev, "failed to get SCMI Tx shared memory\n");
-		return ret;
-	}
-
-	size = resource_size(&res);
-	scmi_info->shmem = devm_ioremap(dev, res.start, size);
-	if (!scmi_info->shmem) {
-		dev_err(dev, "failed to ioremap SCMI Tx shared memory\n");
-		return -EADDRNOTAVAIL;
-	}
+	scmi_info->shmem = setup_shmem_iomap(cinfo, dev, tx);
+	if (IS_ERR(scmi_info->shmem))
+		return PTR_ERR(scmi_info->shmem);
 
 	ret = of_property_read_u32(dev->of_node, "arm,smc-id", &func_id);
 	if (ret < 0)
-- 
2.45.2

[PATCH 2/8] firmware: arm_scmi: Introduce packet handling helpers

[Cristian Marussi]

Introduce a pair of structures initialized to contain ll the existing packet
handling helpers, both for transports based on shared memory and messages.

No functional change.

Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
 drivers/firmware/arm_scmi/common.h  | 77 ++++++++++++++++++++---------
 drivers/firmware/arm_scmi/mailbox.c | 20 ++++----
 drivers/firmware/arm_scmi/msg.c     | 29 +++++++----
 drivers/firmware/arm_scmi/optee.c   | 14 +++---
 drivers/firmware/arm_scmi/shmem.c   | 43 ++++++++++------
 drivers/firmware/arm_scmi/smc.c     |  8 +--
 drivers/firmware/arm_scmi/virtio.c  | 14 +++---
 7 files changed, 130 insertions(+), 75 deletions(-)

diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index b4c217641e3a..34df078d1cd3 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -4,7 +4,7 @@
  * driver common header file containing some definitions, structures
  * and function prototypes used in all the different SCMI protocols.
  *
- * Copyright (C) 2018-2022 ARM Ltd.
+ * Copyright (C) 2018-2024 ARM Ltd.
  */
 #ifndef _SCMI_COMMON_H
 #define _SCMI_COMMON_H
@@ -315,20 +315,38 @@ void scmi_bad_message_trace(struct scmi_chan_info *cinfo, u32 msg_hdr,
 /* shmem related declarations */
 struct scmi_shared_mem;
 
-void shmem_tx_prepare(struct scmi_shared_mem __iomem *shmem,
-		      struct scmi_xfer *xfer, struct scmi_chan_info *cinfo);
-u32 shmem_read_header(struct scmi_shared_mem __iomem *shmem);
-void shmem_fetch_response(struct scmi_shared_mem __iomem *shmem,
+/**
+ * struct scmi_shared_mem_operations  - Transport core operations for
+ * Shared Memory
+ *
+ * @tx_prepare: Prepare the @xfer message for transmission on the chosen @shmem
+ * @read_header: Read header of the message currently hold in @shmem
+ * @fetch_response: Copy the message response from @shmem into @xfer
+ * @fetch_notification: Copy the message notification from @shmem into @xfer
+ * @clear_channel: Clear the @shmem channel busy flag
+ * @poll_done: Check if poll has completed for @xfer on @shmem
+ * @channel_free: Check if @shmem channel is marked as free
+ * @channel_intr_enabled: Check is @shmem channel has requested a completion irq
+ * @setup_iomap: Setup IO shared memory for channel @cinfo
+ */
+struct scmi_shared_mem_operations {
+	void (*tx_prepare)(struct scmi_shared_mem __iomem *shmem,
+			   struct scmi_xfer *xfer,
+			   struct scmi_chan_info *cinfo);
+	u32 (*read_header)(struct scmi_shared_mem __iomem *shmem);
+
+	void (*fetch_response)(struct scmi_shared_mem __iomem *shmem,
+			       struct scmi_xfer *xfer);
+	void (*fetch_notification)(struct scmi_shared_mem __iomem *shmem,
+				   size_t max_len, struct scmi_xfer *xfer);
+	void (*clear_channel)(struct scmi_shared_mem __iomem *shmem);
+	bool (*poll_done)(struct scmi_shared_mem __iomem *shmem,
 			  struct scmi_xfer *xfer);
-void shmem_fetch_notification(struct scmi_shared_mem __iomem *shmem,
-			      size_t max_len, struct scmi_xfer *xfer);
-void shmem_clear_channel(struct scmi_shared_mem __iomem *shmem);
-bool shmem_poll_done(struct scmi_shared_mem __iomem *shmem,
-		     struct scmi_xfer *xfer);
-bool shmem_channel_free(struct scmi_shared_mem __iomem *shmem);
-bool shmem_channel_intr_enabled(struct scmi_shared_mem __iomem *shmem);
-void __iomem *setup_shmem_iomap(struct scmi_chan_info *cinfo, struct device *dev,
-				bool tx);
+	bool (*channel_free)(struct scmi_shared_mem __iomem *shmem);
+	bool (*channel_intr_enabled)(struct scmi_shared_mem __iomem *shmem);
+	void __iomem *(*setup_iomap)(struct scmi_chan_info *cinfo,
+				     struct device *dev, bool tx);
+};
 
 /* declarations for message passing transports */
 struct scmi_msg_payld;
@@ -336,14 +354,29 @@ struct scmi_msg_payld;
 /* Maximum overhead of message w.r.t. struct scmi_desc.max_msg_size */
 #define SCMI_MSG_MAX_PROT_OVERHEAD (2 * sizeof(__le32))
 
-size_t msg_response_size(struct scmi_xfer *xfer);
-size_t msg_command_size(struct scmi_xfer *xfer);
-void msg_tx_prepare(struct scmi_msg_payld *msg, struct scmi_xfer *xfer);
-u32 msg_read_header(struct scmi_msg_payld *msg);
-void msg_fetch_response(struct scmi_msg_payld *msg, size_t len,
-			struct scmi_xfer *xfer);
-void msg_fetch_notification(struct scmi_msg_payld *msg, size_t len,
-			    size_t max_len, struct scmi_xfer *xfer);
+/**
+ * struct scmi_message_operations  - Transport core operations for Message
+ *
+ * @response_size: Get calculated response size for @xfer
+ * @command_size: Get calculated command size for @xfer
+ * @tx_prepare: Prepare the @xfer message for transmission on the provided @msg
+ * @read_header: Read header of the message currently hold in @msg
+ * @fetch_response: Copy the message response from @msg into @xfer
+ * @fetch_notification: Copy the message notification from @msg into @xfer
+ */
+struct scmi_message_operations {
+	size_t (*response_size)(struct scmi_xfer *xfer);
+	size_t (*command_size)(struct scmi_xfer *xfer);
+	void (*tx_prepare)(struct scmi_msg_payld *msg, struct scmi_xfer *xfer);
+	u32 (*read_header)(struct scmi_msg_payld *msg);
+	void (*fetch_response)(struct scmi_msg_payld *msg, size_t len,
+			       struct scmi_xfer *xfer);
+	void (*fetch_notification)(struct scmi_msg_payld *msg, size_t len,
+				   size_t max_len, struct scmi_xfer *xfer);
+};
+
+extern const struct scmi_shared_mem_operations scmi_shmem_ops;
+extern const struct scmi_message_operations scmi_msg_ops;
 
 void scmi_notification_instance_data_set(const struct scmi_handle *handle,
 					 void *priv);
diff --git a/drivers/firmware/arm_scmi/mailbox.c b/drivers/firmware/arm_scmi/mailbox.c
index b0a579f31905..b891310d2670 100644
--- a/drivers/firmware/arm_scmi/mailbox.c
+++ b/drivers/firmware/arm_scmi/mailbox.c
@@ -40,7 +40,7 @@ static void tx_prepare(struct mbox_client *cl, void *m)
 {
 	struct scmi_mailbox *smbox = client_to_scmi_mailbox(cl);
 
-	shmem_tx_prepare(smbox->shmem, m, smbox->cinfo);
+	scmi_shmem_ops.tx_prepare(smbox->shmem, m, smbox->cinfo);
 }
 
 static void rx_callback(struct mbox_client *cl, void *m)
@@ -56,15 +56,15 @@ static void rx_callback(struct mbox_client *cl, void *m)
 	 * a previous timed-out reply which arrived late could be wrongly
 	 * associated with the next pending transaction.
 	 */
-	if (cl->knows_txdone && !shmem_channel_free(smbox->shmem)) {
+	if (cl->knows_txdone && !scmi_shmem_ops.channel_free(smbox->shmem)) {
 		dev_warn(smbox->cinfo->dev, "Ignoring spurious A2P IRQ !\n");
 		scmi_bad_message_trace(smbox->cinfo,
-				       shmem_read_header(smbox->shmem),
+				       scmi_shmem_ops.read_header(smbox->shmem),
 				       MSG_MBOX_SPURIOUS);
 		return;
 	}
 
-	scmi_rx_callback(smbox->cinfo, shmem_read_header(smbox->shmem), NULL);
+	scmi_rx_callback(smbox->cinfo, scmi_shmem_ops.read_header(smbox->shmem), NULL);
 }
 
 static bool mailbox_chan_available(struct device_node *of_node, int idx)
@@ -192,7 +192,7 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
 	if (!smbox)
 		return -ENOMEM;
 
-	smbox->shmem = setup_shmem_iomap(cinfo, dev, tx);
+	smbox->shmem = scmi_shmem_ops.setup_iomap(cinfo, dev, tx);
 	if (IS_ERR(smbox->shmem))
 		return PTR_ERR(smbox->shmem);
 
@@ -293,7 +293,7 @@ static void mailbox_fetch_response(struct scmi_chan_info *cinfo,
 {
 	struct scmi_mailbox *smbox = cinfo->transport_info;
 
-	shmem_fetch_response(smbox->shmem, xfer);
+	scmi_shmem_ops.fetch_response(smbox->shmem, xfer);
 }
 
 static void mailbox_fetch_notification(struct scmi_chan_info *cinfo,
@@ -301,7 +301,7 @@ static void mailbox_fetch_notification(struct scmi_chan_info *cinfo,
 {
 	struct scmi_mailbox *smbox = cinfo->transport_info;
 
-	shmem_fetch_notification(smbox->shmem, max_len, xfer);
+	scmi_shmem_ops.fetch_notification(smbox->shmem, max_len, xfer);
 }
 
 static void mailbox_clear_channel(struct scmi_chan_info *cinfo)
@@ -310,9 +310,9 @@ static void mailbox_clear_channel(struct scmi_chan_info *cinfo)
 	struct mbox_chan *intr_chan;
 	int ret;
 
-	shmem_clear_channel(smbox->shmem);
+	scmi_shmem_ops.clear_channel(smbox->shmem);
 
-	if (!shmem_channel_intr_enabled(smbox->shmem))
+	if (!scmi_shmem_ops.channel_intr_enabled(smbox->shmem))
 		return;
 
 	if (smbox->chan_platform_receiver)
@@ -335,7 +335,7 @@ mailbox_poll_done(struct scmi_chan_info *cinfo, struct scmi_xfer *xfer)
 {
 	struct scmi_mailbox *smbox = cinfo->transport_info;
 
-	return shmem_poll_done(smbox->shmem, xfer);
+	return scmi_shmem_ops.poll_done(smbox->shmem, xfer);
 }
 
 static const struct scmi_transport_ops scmi_mailbox_ops = {
diff --git a/drivers/firmware/arm_scmi/msg.c b/drivers/firmware/arm_scmi/msg.c
index d33a704e5814..f4ba38afe0bb 100644
--- a/drivers/firmware/arm_scmi/msg.c
+++ b/drivers/firmware/arm_scmi/msg.c
@@ -4,8 +4,8 @@
  *
  * Derived from shm.c.
  *
- * Copyright (C) 2019-2021 ARM Ltd.
- * Copyright (C) 2020-2021 OpenSynergy GmbH
+ * Copyright (C) 2019-2024 ARM Ltd.
+ * Copyright (C) 2020-2024 OpenSynergy GmbH
  */
 
 #include <linux/types.h>
@@ -30,7 +30,7 @@ struct scmi_msg_payld {
  *
  * Return: transport SDU size.
  */
-size_t msg_command_size(struct scmi_xfer *xfer)
+static size_t msg_command_size(struct scmi_xfer *xfer)
 {
 	return sizeof(struct scmi_msg_payld) + xfer->tx.len;
 }
@@ -42,7 +42,7 @@ size_t msg_command_size(struct scmi_xfer *xfer)
  *
  * Return: transport SDU size.
  */
-size_t msg_response_size(struct scmi_xfer *xfer)
+static size_t msg_response_size(struct scmi_xfer *xfer)
 {
 	return sizeof(struct scmi_msg_payld) + sizeof(__le32) + xfer->rx.len;
 }
@@ -53,7 +53,7 @@ size_t msg_response_size(struct scmi_xfer *xfer)
  * @msg: transport SDU for command
  * @xfer: message which is being sent
  */
-void msg_tx_prepare(struct scmi_msg_payld *msg, struct scmi_xfer *xfer)
+static void msg_tx_prepare(struct scmi_msg_payld *msg, struct scmi_xfer *xfer)
 {
 	msg->msg_header = cpu_to_le32(pack_scmi_header(&xfer->hdr));
 	if (xfer->tx.buf)
@@ -67,7 +67,7 @@ void msg_tx_prepare(struct scmi_msg_payld *msg, struct scmi_xfer *xfer)
  *
  * Return: SCMI header
  */
-u32 msg_read_header(struct scmi_msg_payld *msg)
+static u32 msg_read_header(struct scmi_msg_payld *msg)
 {
 	return le32_to_cpu(msg->msg_header);
 }
@@ -79,8 +79,8 @@ u32 msg_read_header(struct scmi_msg_payld *msg)
  * @len: transport SDU size
  * @xfer: message being responded to
  */
-void msg_fetch_response(struct scmi_msg_payld *msg, size_t len,
-			struct scmi_xfer *xfer)
+static void msg_fetch_response(struct scmi_msg_payld *msg,
+			       size_t len, struct scmi_xfer *xfer)
 {
 	size_t prefix_len = sizeof(*msg) + sizeof(msg->msg_payload[0]);
 
@@ -100,8 +100,8 @@ void msg_fetch_response(struct scmi_msg_payld *msg, size_t len,
  * @max_len: maximum SCMI payload size to fetch
  * @xfer: notification message
  */
-void msg_fetch_notification(struct scmi_msg_payld *msg, size_t len,
-			    size_t max_len, struct scmi_xfer *xfer)
+static void msg_fetch_notification(struct scmi_msg_payld *msg, size_t len,
+				   size_t max_len, struct scmi_xfer *xfer)
 {
 	xfer->rx.len = min_t(size_t, max_len,
 			     len >= sizeof(*msg) ? len - sizeof(*msg) : 0);
@@ -109,3 +109,12 @@ void msg_fetch_notification(struct scmi_msg_payld *msg, size_t len,
 	/* Take a copy to the rx buffer.. */
 	memcpy(xfer->rx.buf, msg->msg_payload, xfer->rx.len);
 }
+
+const struct scmi_message_operations scmi_msg_ops = {
+	.tx_prepare = msg_tx_prepare,
+	.command_size = msg_command_size,
+	.response_size = msg_response_size,
+	.read_header = msg_read_header,
+	.fetch_response = msg_fetch_response,
+	.fetch_notification = msg_fetch_notification,
+};
diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c
index 8abcd668108c..c26527f3b8f4 100644
--- a/drivers/firmware/arm_scmi/optee.c
+++ b/drivers/firmware/arm_scmi/optee.c
@@ -343,7 +343,7 @@ static void scmi_optee_clear_channel(struct scmi_chan_info *cinfo)
 	struct scmi_optee_channel *channel = cinfo->transport_info;
 
 	if (!channel->tee_shm)
-		shmem_clear_channel(channel->req.shmem);
+		scmi_shmem_ops.clear_channel(channel->req.shmem);
 }
 
 static int setup_dynamic_shmem(struct device *dev, struct scmi_optee_channel *channel)
@@ -368,7 +368,7 @@ static int setup_dynamic_shmem(struct device *dev, struct scmi_optee_channel *ch
 static int setup_static_shmem(struct device *dev, struct scmi_chan_info *cinfo,
 			      struct scmi_optee_channel *channel)
 {
-	channel->req.shmem = setup_shmem_iomap(cinfo, dev, true);
+	channel->req.shmem = scmi_shmem_ops.setup_iomap(cinfo, dev, true);
 	if (IS_ERR(channel->req.shmem))
 		return PTR_ERR(channel->req.shmem);
 
@@ -472,10 +472,10 @@ static int scmi_optee_send_message(struct scmi_chan_info *cinfo,
 	mutex_lock(&channel->mu);
 
 	if (channel->tee_shm) {
-		msg_tx_prepare(channel->req.msg, xfer);
-		ret = invoke_process_msg_channel(channel, msg_command_size(xfer));
+		scmi_msg_ops.tx_prepare(channel->req.msg, xfer);
+		ret = invoke_process_msg_channel(channel, scmi_msg_ops.command_size(xfer));
 	} else {
-		shmem_tx_prepare(channel->req.shmem, xfer, cinfo);
+		scmi_shmem_ops.tx_prepare(channel->req.shmem, xfer, cinfo);
 		ret = invoke_process_smt_channel(channel);
 	}
 
@@ -491,9 +491,9 @@ static void scmi_optee_fetch_response(struct scmi_chan_info *cinfo,
 	struct scmi_optee_channel *channel = cinfo->transport_info;
 
 	if (channel->tee_shm)
-		msg_fetch_response(channel->req.msg, channel->rx_len, xfer);
+		scmi_msg_ops.fetch_response(channel->req.msg, channel->rx_len, xfer);
 	else
-		shmem_fetch_response(channel->req.shmem, xfer);
+		scmi_shmem_ops.fetch_response(channel->req.shmem, xfer);
 }
 
 static void scmi_optee_mark_txdone(struct scmi_chan_info *cinfo, int ret,
diff --git a/drivers/firmware/arm_scmi/shmem.c b/drivers/firmware/arm_scmi/shmem.c
index c31f188d74ef..c84321ff5428 100644
--- a/drivers/firmware/arm_scmi/shmem.c
+++ b/drivers/firmware/arm_scmi/shmem.c
@@ -2,7 +2,7 @@
 /*
  * For transport using shared mem structure.
  *
- * Copyright (C) 2019 ARM Ltd.
+ * Copyright (C) 2019-2024 ARM Ltd.
  */
 
 #include <linux/ktime.h>
@@ -34,8 +34,9 @@ struct scmi_shared_mem {
 	u8 msg_payload[];
 };
 
-void shmem_tx_prepare(struct scmi_shared_mem __iomem *shmem,
-		      struct scmi_xfer *xfer, struct scmi_chan_info *cinfo)
+static void shmem_tx_prepare(struct scmi_shared_mem __iomem *shmem,
+			     struct scmi_xfer *xfer,
+			     struct scmi_chan_info *cinfo)
 {
 	ktime_t stop;
 
@@ -75,13 +76,13 @@ void shmem_tx_prepare(struct scmi_shared_mem __iomem *shmem,
 		memcpy_toio(shmem->msg_payload, xfer->tx.buf, xfer->tx.len);
 }
 
-u32 shmem_read_header(struct scmi_shared_mem __iomem *shmem)
+static u32 shmem_read_header(struct scmi_shared_mem __iomem *shmem)
 {
 	return ioread32(&shmem->msg_header);
 }
 
-void shmem_fetch_response(struct scmi_shared_mem __iomem *shmem,
-			  struct scmi_xfer *xfer)
+static void shmem_fetch_response(struct scmi_shared_mem __iomem *shmem,
+				 struct scmi_xfer *xfer)
 {
 	size_t len = ioread32(&shmem->length);
 
@@ -93,8 +94,8 @@ void shmem_fetch_response(struct scmi_shared_mem __iomem *shmem,
 	memcpy_fromio(xfer->rx.buf, shmem->msg_payload + 4, xfer->rx.len);
 }
 
-void shmem_fetch_notification(struct scmi_shared_mem __iomem *shmem,
-			      size_t max_len, struct scmi_xfer *xfer)
+static void shmem_fetch_notification(struct scmi_shared_mem __iomem *shmem,
+				     size_t max_len, struct scmi_xfer *xfer)
 {
 	size_t len = ioread32(&shmem->length);
 
@@ -105,13 +106,13 @@ void shmem_fetch_notification(struct scmi_shared_mem __iomem *shmem,
 	memcpy_fromio(xfer->rx.buf, shmem->msg_payload, xfer->rx.len);
 }
 
-void shmem_clear_channel(struct scmi_shared_mem __iomem *shmem)
+static void shmem_clear_channel(struct scmi_shared_mem __iomem *shmem)
 {
 	iowrite32(SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE, &shmem->channel_status);
 }
 
-bool shmem_poll_done(struct scmi_shared_mem __iomem *shmem,
-		     struct scmi_xfer *xfer)
+static bool shmem_poll_done(struct scmi_shared_mem __iomem *shmem,
+			    struct scmi_xfer *xfer)
 {
 	u16 xfer_id;
 
@@ -125,19 +126,19 @@ bool shmem_poll_done(struct scmi_shared_mem __iomem *shmem,
 		 SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE);
 }
 
-bool shmem_channel_free(struct scmi_shared_mem __iomem *shmem)
+static bool shmem_channel_free(struct scmi_shared_mem __iomem *shmem)
 {
 	return (ioread32(&shmem->channel_status) &
 			SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE);
 }
 
-bool shmem_channel_intr_enabled(struct scmi_shared_mem __iomem *shmem)
+static bool shmem_channel_intr_enabled(struct scmi_shared_mem __iomem *shmem)
 {
 	return ioread32(&shmem->flags) & SCMI_SHMEM_FLAG_INTR_ENABLED;
 }
 
-void *__iomem
-setup_shmem_iomap(struct scmi_chan_info *cinfo, struct device *dev, bool tx)
+static void *__iomem shmem_setup_iomap(struct scmi_chan_info *cinfo,
+				       struct device *dev, bool tx)
 {
 	const char *desc = tx ? "Tx" : "Rx";
 	int ret, idx = tx ? 0 : 1;
@@ -169,3 +170,15 @@ setup_shmem_iomap(struct scmi_chan_info *cinfo, struct device *dev, bool tx)
 
 	return addr;
 }
+
+const struct scmi_shared_mem_operations scmi_shmem_ops = {
+	.tx_prepare = shmem_tx_prepare,
+	.read_header = shmem_read_header,
+	.fetch_response = shmem_fetch_response,
+	.fetch_notification = shmem_fetch_notification,
+	.clear_channel = shmem_clear_channel,
+	.poll_done = shmem_poll_done,
+	.channel_free = shmem_channel_free,
+	.channel_intr_enabled = shmem_channel_intr_enabled,
+	.setup_iomap = shmem_setup_iomap,
+};
diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c
index a640a4312472..cb26b8aee01d 100644
--- a/drivers/firmware/arm_scmi/smc.c
+++ b/drivers/firmware/arm_scmi/smc.c
@@ -74,7 +74,7 @@ static irqreturn_t smc_msg_done_isr(int irq, void *data)
 	struct scmi_smc *scmi_info = data;
 
 	scmi_rx_callback(scmi_info->cinfo,
-			 shmem_read_header(scmi_info->shmem), NULL);
+			 scmi_shmem_ops.read_header(scmi_info->shmem), NULL);
 
 	return IRQ_HANDLED;
 }
@@ -142,7 +142,7 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
 	if (!scmi_info)
 		return -ENOMEM;
 
-	scmi_info->shmem = setup_shmem_iomap(cinfo, dev, tx);
+	scmi_info->shmem = scmi_shmem_ops.setup_iomap(cinfo, dev, tx);
 	if (IS_ERR(scmi_info->shmem))
 		return PTR_ERR(scmi_info->shmem);
 
@@ -226,7 +226,7 @@ static int smc_send_message(struct scmi_chan_info *cinfo,
 	 */
 	smc_channel_lock_acquire(scmi_info, xfer);
 
-	shmem_tx_prepare(scmi_info->shmem, xfer, cinfo);
+	scmi_shmem_ops.tx_prepare(scmi_info->shmem, xfer, cinfo);
 
 	if (scmi_info->cap_id != ULONG_MAX)
 		arm_smccc_1_1_invoke(scmi_info->func_id, scmi_info->cap_id, 0,
@@ -250,7 +250,7 @@ static void smc_fetch_response(struct scmi_chan_info *cinfo,
 {
 	struct scmi_smc *scmi_info = cinfo->transport_info;
 
-	shmem_fetch_response(scmi_info->shmem, xfer);
+	scmi_shmem_ops.fetch_response(scmi_info->shmem, xfer);
 }
 
 static void smc_mark_txdone(struct scmi_chan_info *cinfo, int ret,
diff --git a/drivers/firmware/arm_scmi/virtio.c b/drivers/firmware/arm_scmi/virtio.c
index 4892058445ce..736a0d41a458 100644
--- a/drivers/firmware/arm_scmi/virtio.c
+++ b/drivers/firmware/arm_scmi/virtio.c
@@ -295,7 +295,7 @@ static void scmi_vio_complete_cb(struct virtqueue *vqueue)
 		if (msg) {
 			msg->rx_len = length;
 			scmi_rx_callback(vioch->cinfo,
-					 msg_read_header(msg->input), msg);
+					 scmi_msg_ops.read_header(msg->input), msg);
 
 			scmi_finalize_message(vioch, msg);
 		}
@@ -340,7 +340,7 @@ static void scmi_vio_deferred_tx_worker(struct work_struct *work)
 		 */
 		if (msg->poll_status == VIO_MSG_NOT_POLLED)
 			scmi_rx_callback(vioch->cinfo,
-					 msg_read_header(msg->input), msg);
+					 scmi_msg_ops.read_header(msg->input), msg);
 
 		/* Free the processed message once done */
 		scmi_vio_msg_release(vioch, msg);
@@ -512,10 +512,10 @@ static int virtio_send_message(struct scmi_chan_info *cinfo,
 		return -EBUSY;
 	}
 
-	msg_tx_prepare(msg->request, xfer);
+	scmi_msg_ops.tx_prepare(msg->request, xfer);
 
-	sg_init_one(&sg_out, msg->request, msg_command_size(xfer));
-	sg_init_one(&sg_in, msg->input, msg_response_size(xfer));
+	sg_init_one(&sg_out, msg->request, scmi_msg_ops.command_size(xfer));
+	sg_init_one(&sg_in, msg->input, scmi_msg_ops.response_size(xfer));
 
 	spin_lock_irqsave(&vioch->lock, flags);
 
@@ -562,7 +562,7 @@ static void virtio_fetch_response(struct scmi_chan_info *cinfo,
 	struct scmi_vio_msg *msg = xfer->priv;
 
 	if (msg)
-		msg_fetch_response(msg->input, msg->rx_len, xfer);
+		scmi_msg_ops.fetch_response(msg->input, msg->rx_len, xfer);
 }
 
 static void virtio_fetch_notification(struct scmi_chan_info *cinfo,
@@ -571,7 +571,7 @@ static void virtio_fetch_notification(struct scmi_chan_info *cinfo,
 	struct scmi_vio_msg *msg = xfer->priv;
 
 	if (msg)
-		msg_fetch_notification(msg->input, msg->rx_len, max_len, xfer);
+		scmi_msg_ops.fetch_notification(msg->input, msg->rx_len, max_len, xfer);
 }
 
 /**
-- 
2.45.2

[PATCH 3/8] firmware: arm_scmi: Add support for standalone transport drivers

[Cristian Marussi]

Extend the core SCMI stack with structures and methods to allow for transports
to be split out as standalone drivers, while still supporting old style
transports, defined as built into the SCMI core stack.

No functional change.

Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
Old style transport support will be removed later in this series.
---
 drivers/firmware/arm_scmi/common.h | 84 ++++++++++++++++++++++++++++++
 drivers/firmware/arm_scmi/driver.c | 44 +++++++++++++++-
 drivers/firmware/arm_scmi/msg.c    |  5 ++
 drivers/firmware/arm_scmi/shmem.c  |  5 ++
 4 files changed, 136 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 34df078d1cd3..fda6f96b6750 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -348,6 +348,8 @@ struct scmi_shared_mem_operations {
 				     struct device *dev, bool tx);
 };
 
+const struct scmi_shared_mem_operations *scmi_shared_mem_operations_get(void);
+
 /* declarations for message passing transports */
 struct scmi_msg_payld;
 
@@ -375,6 +377,88 @@ struct scmi_message_operations {
 				   size_t max_len, struct scmi_xfer *xfer);
 };
 
+const struct scmi_message_operations *scmi_message_operations_get(void);
+
+/**
+ * struct scmi_transport_core_operations  - Transpoert core operations
+ *
+ * @bad_message_trace: An helper to report a malformed/unexpected message
+ * @rx_callback: Callback to report received messages
+ * @shmem: Datagram operations for shared memory based transports
+ * @msg: Datagram operations for message based transports
+ */
+struct scmi_transport_core_operations {
+	void (*bad_message_trace)(struct scmi_chan_info *cinfo,
+				  u32 msg_hdr, enum scmi_bad_msg err);
+	void (*rx_callback)(struct scmi_chan_info *cinfo, u32 msg_hdr,
+			    void *priv);
+	const struct scmi_shared_mem_operations *shmem;
+	const struct scmi_message_operations *msg;
+};
+
+/**
+ * struct scmi_transport  - A structure representing a configured transport
+ *
+ * @supplier: Device representimng the transport and acting as a supplier for
+ *	      the core SCMI stack
+ * @desc: Transport descriptor
+ * @core_ops: A pointer to a pointer used by the core SCMI stack to make the
+ *	      core transport operations accessible to the transports.
+ */
+struct scmi_transport {
+	struct device *supplier;
+	const struct scmi_desc *desc;
+	struct scmi_transport_core_operations **core_ops;
+};
+
+#define DEFINE_SCMI_TRANSPORT_DRIVER(__trans, __match_table, __core_ptr)\
+static int __trans##_probe(struct platform_device *pdev)		\
+{									\
+	struct scmi_transport *scmi_trans;				\
+	struct platform_device *scmi_pdev;				\
+	struct device *dev = &pdev->dev;				\
+									\
+	scmi_trans = devm_kzalloc(dev, sizeof(*scmi_trans), GFP_KERNEL);\
+	if (!scmi_trans)						\
+		return -ENOMEM;						\
+									\
+	scmi_pdev = devm_kzalloc(dev, sizeof(*scmi_pdev), GFP_KERNEL);	\
+	if (!scmi_pdev)							\
+		return -ENOMEM;						\
+									\
+	scmi_trans->supplier = dev;					\
+	scmi_trans->desc = &__trans##_desc;				\
+	scmi_trans->core_ops = __core_ptr;				\
+									\
+	scmi_pdev->name = "arm-scmi";					\
+	scmi_pdev->id = PLATFORM_DEVID_AUTO;				\
+	scmi_pdev->dev.platform_data = scmi_trans;			\
+									\
+	device_set_of_node_from_dev(&scmi_pdev->dev, dev);		\
+									\
+	dev_set_drvdata(dev, scmi_pdev);				\
+									\
+	return platform_device_register(scmi_pdev);			\
+}									\
+									\
+static void __trans##_remove(struct platform_device *pdev)		\
+{									\
+	struct platform_device *scmi_pdev;				\
+									\
+	scmi_pdev = dev_get_drvdata(&pdev->dev);			\
+									\
+	platform_device_unregister(scmi_pdev);				\
+}									\
+									\
+static struct platform_driver __trans##_driver = {			\
+	.driver = {							\
+		   .name = #__trans "_transport",			\
+		   .of_match_table = __match_table,			\
+		   },							\
+	.probe = __trans##_probe,					\
+	.remove_new = __trans##_remove,					\
+}
+
 extern const struct scmi_shared_mem_operations scmi_shmem_ops;
 extern const struct scmi_message_operations scmi_msg_ops;
 
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 6b6957f4743f..a1892d4d8c69 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -194,6 +194,11 @@ struct scmi_info {
 #define bus_nb_to_scmi_info(nb)	container_of(nb, struct scmi_info, bus_nb)
 #define req_nb_to_scmi_info(nb)	container_of(nb, struct scmi_info, dev_req_nb)
 
+static struct scmi_transport_core_operations scmi_trans_core_ops = {
+	.bad_message_trace = scmi_bad_message_trace,
+	.rx_callback = scmi_rx_callback,
+};
+
 static unsigned long
 scmi_vendor_protocol_signature(unsigned int protocol_id, char *vendor_id,
 			       char *sub_vendor_id, u32 impl_ver)
@@ -2950,6 +2955,28 @@ static int scmi_debugfs_raw_mode_setup(struct scmi_info *info)
 	return ret;
 }
 
+static const struct scmi_desc *scmi_transport_lookup(struct device *dev)
+{
+	struct scmi_transport *trans;
+
+	trans = dev_get_platdata(dev);
+	if (!trans || !trans->desc || !trans->supplier || !trans->core_ops)
+		return NULL;
+
+	if (!device_link_add(dev, trans->supplier, DL_FLAG_AUTOREMOVE_CONSUMER)) {
+		dev_err(dev,
+			"Adding link to supplier transport device failed\n");
+		return NULL;
+	}
+
+	/* Provide core transport ops */
+	*trans->core_ops = &scmi_trans_core_ops;
+
+	dev_info(dev, "Using %s\n", dev_driver_string(trans->supplier));
+
+	return trans->desc;
+}
+
 static int scmi_probe(struct platform_device *pdev)
 {
 	int ret;
@@ -2962,8 +2989,14 @@ static int scmi_probe(struct platform_device *pdev)
 	struct device_node *child, *np = dev->of_node;
 
 	desc = of_device_get_match_data(dev);
-	if (!desc)
-		return -EINVAL;
+	if (!desc) {
+		desc = scmi_transport_lookup(dev);
+		if (!desc) {
+			err_str = "transport invalid\n";
+			ret = -EINVAL;
+			goto out_err;
+		}
+	}
 
 	info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
 	if (!info)
@@ -3130,6 +3163,7 @@ static int scmi_probe(struct platform_device *pdev)
 clear_ida:
 	ida_free(&scmi_id, info->id);
 
+out_err:
 	return dev_err_probe(dev, ret, "%s", err_str);
 }
 
@@ -3321,6 +3355,12 @@ static int __init scmi_driver_init(void)
 	if (ret)
 		return ret;
 
+	if (IS_ENABLED(CONFIG_ARM_SCMI_HAVE_SHMEM))
+		scmi_trans_core_ops.shmem = scmi_shared_mem_operations_get();
+
+	if (IS_ENABLED(CONFIG_ARM_SCMI_HAVE_MSG))
+		scmi_trans_core_ops.msg = scmi_message_operations_get();
+
 	if (IS_ENABLED(CONFIG_ARM_SCMI_NEED_DEBUGFS))
 		scmi_top_dentry = scmi_debugfs_init();
 
diff --git a/drivers/firmware/arm_scmi/msg.c b/drivers/firmware/arm_scmi/msg.c
index f4ba38afe0bb..0bed1d2825af 100644
--- a/drivers/firmware/arm_scmi/msg.c
+++ b/drivers/firmware/arm_scmi/msg.c
@@ -118,3 +118,8 @@ const struct scmi_message_operations scmi_msg_ops = {
 	.fetch_response = msg_fetch_response,
 	.fetch_notification = msg_fetch_notification,
 };
+
+const struct scmi_message_operations *scmi_message_operations_get(void)
+{
+	return &scmi_msg_ops;
+}
diff --git a/drivers/firmware/arm_scmi/shmem.c b/drivers/firmware/arm_scmi/shmem.c
index c84321ff5428..feb4215831dc 100644
--- a/drivers/firmware/arm_scmi/shmem.c
+++ b/drivers/firmware/arm_scmi/shmem.c
@@ -182,3 +182,8 @@ const struct scmi_shared_mem_operations scmi_shmem_ops = {
 	.channel_intr_enabled = shmem_channel_intr_enabled,
 	.setup_iomap = shmem_setup_iomap,
 };
+
+const struct scmi_shared_mem_operations *scmi_shared_mem_operations_get(void)
+{
+	return &scmi_shmem_ops;
+}
-- 
2.45.2

[PATCH 4/8] firmware: arm_scmi: Make MBOX transport a standalone driver

[Cristian Marussi]

Make SCMI mailbox transport a standalne driver that can be optionally
loaded as a module.

Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
 drivers/firmware/arm_scmi/Kconfig             |  4 +-
 drivers/firmware/arm_scmi/Makefile            |  3 +-
 drivers/firmware/arm_scmi/common.h            |  3 --
 drivers/firmware/arm_scmi/driver.c            |  3 --
 .../{mailbox.c => scmi_transport_mailbox.c}   | 44 +++++++++++++------
 5 files changed, 36 insertions(+), 21 deletions(-)
 rename drivers/firmware/arm_scmi/{mailbox.c => scmi_transport_mailbox.c} (88%)

diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
index aa5842be19b2..135e34aefd70 100644
--- a/drivers/firmware/arm_scmi/Kconfig
+++ b/drivers/firmware/arm_scmi/Kconfig
@@ -75,7 +75,7 @@ config ARM_SCMI_HAVE_MSG
 	  available.
 
 config ARM_SCMI_TRANSPORT_MAILBOX
-	bool "SCMI transport based on Mailbox"
+	tristate "SCMI transport based on Mailbox"
 	depends on MAILBOX
 	select ARM_SCMI_HAVE_TRANSPORT
 	select ARM_SCMI_HAVE_SHMEM
@@ -85,6 +85,8 @@ config ARM_SCMI_TRANSPORT_MAILBOX
 
 	  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_mailbox.
 
 config ARM_SCMI_TRANSPORT_OPTEE
 	bool "SCMI transport based on OP-TEE service"
diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index fd59f58ce8a2..121612d75f0b 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -5,7 +5,6 @@ scmi-core-objs := $(scmi-bus-y)
 scmi-driver-y = driver.o notify.o
 scmi-driver-$(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT) += raw_mode.o
 scmi-transport-$(CONFIG_ARM_SCMI_HAVE_SHMEM) = shmem.o
-scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_MAILBOX) += mailbox.o
 scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += smc.o
 scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
 scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
@@ -14,6 +13,8 @@ scmi-protocols-y := base.o clock.o perf.o power.o reset.o sensors.o system.o vol
 scmi-protocols-y += pinctrl.o
 scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
 
+obj-$(CONFIG_ARM_SCMI_TRANSPORT_MAILBOX) += scmi_transport_mailbox.o
+
 obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
 obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-module.o
 
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index fda6f96b6750..c03f30db92e0 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -286,9 +286,6 @@ int scmi_xfer_raw_inflight_register(const struct scmi_handle *handle,
 int scmi_xfer_raw_wait_for_message_response(struct scmi_chan_info *cinfo,
 					    struct scmi_xfer *xfer,
 					    unsigned int timeout_ms);
-#ifdef CONFIG_ARM_SCMI_TRANSPORT_MAILBOX
-extern const struct scmi_desc scmi_mailbox_desc;
-#endif
 #ifdef CONFIG_ARM_SCMI_TRANSPORT_SMC
 extern const struct scmi_desc scmi_smc_desc;
 #endif
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index a1892d4d8c69..96cf8ab4421e 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -3251,9 +3251,6 @@ ATTRIBUTE_GROUPS(versions);
 
 /* Each compatible listed below must have descriptor associated with it */
 static const struct of_device_id scmi_of_match[] = {
-#ifdef CONFIG_ARM_SCMI_TRANSPORT_MAILBOX
-	{ .compatible = "arm,scmi", .data = &scmi_mailbox_desc },
-#endif
 #ifdef CONFIG_ARM_SCMI_TRANSPORT_OPTEE
 	{ .compatible = "linaro,scmi-optee", .data = &scmi_optee_desc },
 #endif
diff --git a/drivers/firmware/arm_scmi/mailbox.c b/drivers/firmware/arm_scmi/scmi_transport_mailbox.c
similarity index 88%
rename from drivers/firmware/arm_scmi/mailbox.c
rename to drivers/firmware/arm_scmi/scmi_transport_mailbox.c
index b891310d2670..4d8607c6df23 100644
--- a/drivers/firmware/arm_scmi/mailbox.c
+++ b/drivers/firmware/arm_scmi/scmi_transport_mailbox.c
@@ -11,6 +11,7 @@
 #include <linux/mailbox_client.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/platform_device.h>
 #include <linux/slab.h>
 
 #include "common.h"
@@ -36,11 +37,13 @@ struct scmi_mailbox {
 
 #define client_to_scmi_mailbox(c) container_of(c, struct scmi_mailbox, cl)
 
+static struct scmi_transport_core_operations *core;
+
 static void tx_prepare(struct mbox_client *cl, void *m)
 {
 	struct scmi_mailbox *smbox = client_to_scmi_mailbox(cl);
 
-	scmi_shmem_ops.tx_prepare(smbox->shmem, m, smbox->cinfo);
+	core->shmem->tx_prepare(smbox->shmem, m, smbox->cinfo);
 }
 
 static void rx_callback(struct mbox_client *cl, void *m)
@@ -56,15 +59,17 @@ static void rx_callback(struct mbox_client *cl, void *m)
 	 * a previous timed-out reply which arrived late could be wrongly
 	 * associated with the next pending transaction.
 	 */
-	if (cl->knows_txdone && !scmi_shmem_ops.channel_free(smbox->shmem)) {
+	if (cl->knows_txdone &&
+	    !core->shmem->channel_free(smbox->shmem)) {
 		dev_warn(smbox->cinfo->dev, "Ignoring spurious A2P IRQ !\n");
-		scmi_bad_message_trace(smbox->cinfo,
-				       scmi_shmem_ops.read_header(smbox->shmem),
-				       MSG_MBOX_SPURIOUS);
+		core->bad_message_trace(smbox->cinfo,
+			     core->shmem->read_header(smbox->shmem),
+							     MSG_MBOX_SPURIOUS);
 		return;
 	}
 
-	scmi_rx_callback(smbox->cinfo, scmi_shmem_ops.read_header(smbox->shmem), NULL);
+	core->rx_callback(smbox->cinfo,
+		      core->shmem->read_header(smbox->shmem), NULL);
 }
 
 static bool mailbox_chan_available(struct device_node *of_node, int idx)
@@ -192,7 +197,7 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
 	if (!smbox)
 		return -ENOMEM;
 
-	smbox->shmem = scmi_shmem_ops.setup_iomap(cinfo, dev, tx);
+	smbox->shmem = core->shmem->setup_iomap(cinfo, dev, tx);
 	if (IS_ERR(smbox->shmem))
 		return PTR_ERR(smbox->shmem);
 
@@ -293,7 +298,7 @@ static void mailbox_fetch_response(struct scmi_chan_info *cinfo,
 {
 	struct scmi_mailbox *smbox = cinfo->transport_info;
 
-	scmi_shmem_ops.fetch_response(smbox->shmem, xfer);
+	core->shmem->fetch_response(smbox->shmem, xfer);
 }
 
 static void mailbox_fetch_notification(struct scmi_chan_info *cinfo,
@@ -301,7 +306,7 @@ static void mailbox_fetch_notification(struct scmi_chan_info *cinfo,
 {
 	struct scmi_mailbox *smbox = cinfo->transport_info;
 
-	scmi_shmem_ops.fetch_notification(smbox->shmem, max_len, xfer);
+	core->shmem->fetch_notification(smbox->shmem, max_len, xfer);
 }
 
 static void mailbox_clear_channel(struct scmi_chan_info *cinfo)
@@ -310,9 +315,9 @@ static void mailbox_clear_channel(struct scmi_chan_info *cinfo)
 	struct mbox_chan *intr_chan;
 	int ret;
 
-	scmi_shmem_ops.clear_channel(smbox->shmem);
+	core->shmem->clear_channel(smbox->shmem);
 
-	if (!scmi_shmem_ops.channel_intr_enabled(smbox->shmem))
+	if (!core->shmem->channel_intr_enabled(smbox->shmem))
 		return;
 
 	if (smbox->chan_platform_receiver)
@@ -335,7 +340,7 @@ mailbox_poll_done(struct scmi_chan_info *cinfo, struct scmi_xfer *xfer)
 {
 	struct scmi_mailbox *smbox = cinfo->transport_info;
 
-	return scmi_shmem_ops.poll_done(smbox->shmem, xfer);
+	return core->shmem->poll_done(smbox->shmem, xfer);
 }
 
 static const struct scmi_transport_ops scmi_mailbox_ops = {
@@ -350,9 +355,22 @@ static const struct scmi_transport_ops scmi_mailbox_ops = {
 	.poll_done = mailbox_poll_done,
 };
 
-const struct scmi_desc scmi_mailbox_desc = {
+static const struct scmi_desc scmi_mailbox_desc = {
 	.ops = &scmi_mailbox_ops,
 	.max_rx_timeout_ms = 30, /* We may increase this if required */
 	.max_msg = 20, /* Limited by MBOX_TX_QUEUE_LEN */
 	.max_msg_size = 128,
 };
+
+static const struct of_device_id scmi_of_match[] = {
+	{ .compatible = "arm,scmi" },
+	{ /* Sentinel */ },
+};
+
+DEFINE_SCMI_TRANSPORT_DRIVER(scmi_mailbox, scmi_of_match, &core);
+module_platform_driver(scmi_mailbox_driver);
+
+MODULE_ALIAS("scmi-transport-mailbox");
+MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>");
+MODULE_DESCRIPTION("SCMI Mailbox Transport driver");
+MODULE_LICENSE("GPL");
-- 
2.45.2

[PATCH 5/8] firmware: arm_scmi: Make SMC transport a standalone driver

[Cristian Marussi]

Make SCMI SMC transport a standalone driver that can be optionally
loaded as a module.

CC: Peng Fan <peng.fan@nxp.com>
CC: Nikunj Kela <quic_nkela@quicinc.com>
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
 drivers/firmware/arm_scmi/Kconfig             |  4 ++-
 drivers/firmware/arm_scmi/Makefile            |  2 +-
 drivers/firmware/arm_scmi/common.h            |  3 --
 drivers/firmware/arm_scmi/driver.c            |  5 ---
 .../arm_scmi/{smc.c => scmi_transport_smc.c}  | 31 +++++++++++++++----
 5 files changed, 29 insertions(+), 16 deletions(-)
 rename drivers/firmware/arm_scmi/{smc.c => scmi_transport_smc.c} (89%)

diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
index 135e34aefd70..a4d44ef8bf45 100644
--- a/drivers/firmware/arm_scmi/Kconfig
+++ b/drivers/firmware/arm_scmi/Kconfig
@@ -102,7 +102,7 @@ config ARM_SCMI_TRANSPORT_OPTEE
 	  transport based on OP-TEE SCMI service, answer Y.
 
 config ARM_SCMI_TRANSPORT_SMC
-	bool "SCMI transport based on SMC"
+	tristate "SCMI transport based on SMC"
 	depends on HAVE_ARM_SMCCC_DISCOVERY
 	select ARM_SCMI_HAVE_TRANSPORT
 	select ARM_SCMI_HAVE_SHMEM
@@ -112,6 +112,8 @@ config ARM_SCMI_TRANSPORT_SMC
 
 	  If you want the ARM SCMI PROTOCOL stack to include support for a
 	  transport based on SMC, answer Y.
+	  This driver can also be built as a module.  If so, the module
+	  will be called scmi_transport_smc.
 
 config ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE
 	bool "Enable atomic mode support for SCMI SMC transport"
diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index 121612d75f0b..6868a47fa4ab 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -5,7 +5,6 @@ scmi-core-objs := $(scmi-bus-y)
 scmi-driver-y = driver.o notify.o
 scmi-driver-$(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT) += raw_mode.o
 scmi-transport-$(CONFIG_ARM_SCMI_HAVE_SHMEM) = shmem.o
-scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += smc.o
 scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
 scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
 scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
@@ -13,6 +12,7 @@ scmi-protocols-y := base.o clock.o perf.o power.o reset.o sensors.o system.o vol
 scmi-protocols-y += pinctrl.o
 scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
 
+obj-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += scmi_transport_smc.o
 obj-$(CONFIG_ARM_SCMI_TRANSPORT_MAILBOX) += scmi_transport_mailbox.o
 
 obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index c03f30db92e0..b5bd27eccf24 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -286,9 +286,6 @@ int scmi_xfer_raw_inflight_register(const struct scmi_handle *handle,
 int scmi_xfer_raw_wait_for_message_response(struct scmi_chan_info *cinfo,
 					    struct scmi_xfer *xfer,
 					    unsigned int timeout_ms);
-#ifdef CONFIG_ARM_SCMI_TRANSPORT_SMC
-extern const struct scmi_desc scmi_smc_desc;
-#endif
 #ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
 extern const struct scmi_desc scmi_virtio_desc;
 #endif
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 96cf8ab4421e..b14c5326930a 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -3254,11 +3254,6 @@ static const struct of_device_id scmi_of_match[] = {
 #ifdef CONFIG_ARM_SCMI_TRANSPORT_OPTEE
 	{ .compatible = "linaro,scmi-optee", .data = &scmi_optee_desc },
 #endif
-#ifdef CONFIG_ARM_SCMI_TRANSPORT_SMC
-	{ .compatible = "arm,scmi-smc", .data = &scmi_smc_desc},
-	{ .compatible = "arm,scmi-smc-param", .data = &scmi_smc_desc},
-	{ .compatible = "qcom,scmi-smc", .data = &scmi_smc_desc},
-#endif
 #ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
 	{ .compatible = "arm,scmi-virtio", .data = &scmi_virtio_desc},
 #endif
diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/scmi_transport_smc.c
similarity index 89%
rename from drivers/firmware/arm_scmi/smc.c
rename to drivers/firmware/arm_scmi/scmi_transport_smc.c
index cb26b8aee01d..44da1a8d5387 100644
--- a/drivers/firmware/arm_scmi/smc.c
+++ b/drivers/firmware/arm_scmi/scmi_transport_smc.c
@@ -3,7 +3,7 @@
  * System Control and Management Interface (SCMI) Message SMC/HVC
  * Transport driver
  *
- * Copyright 2020 NXP
+ * Copyright 2020-2024 NXP
  */
 
 #include <linux/arm-smccc.h>
@@ -16,6 +16,7 @@
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/limits.h>
+#include <linux/platform_device.h>
 #include <linux/processor.h>
 #include <linux/slab.h>
 
@@ -69,12 +70,14 @@ struct scmi_smc {
 	unsigned long cap_id;
 };
 
+static struct scmi_transport_core_operations *core;
+
 static irqreturn_t smc_msg_done_isr(int irq, void *data)
 {
 	struct scmi_smc *scmi_info = data;
 
-	scmi_rx_callback(scmi_info->cinfo,
-			 scmi_shmem_ops.read_header(scmi_info->shmem), NULL);
+	core->rx_callback(scmi_info->cinfo,
+			  core->shmem->read_header(scmi_info->shmem), NULL);
 
 	return IRQ_HANDLED;
 }
@@ -142,7 +145,7 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
 	if (!scmi_info)
 		return -ENOMEM;
 
-	scmi_info->shmem = scmi_shmem_ops.setup_iomap(cinfo, dev, tx);
+	scmi_info->shmem = core->shmem->setup_iomap(cinfo, dev, tx);
 	if (IS_ERR(scmi_info->shmem))
 		return PTR_ERR(scmi_info->shmem);
 
@@ -226,7 +229,7 @@ static int smc_send_message(struct scmi_chan_info *cinfo,
 	 */
 	smc_channel_lock_acquire(scmi_info, xfer);
 
-	scmi_shmem_ops.tx_prepare(scmi_info->shmem, xfer, cinfo);
+	core->shmem->tx_prepare(scmi_info->shmem, xfer, cinfo);
 
 	if (scmi_info->cap_id != ULONG_MAX)
 		arm_smccc_1_1_invoke(scmi_info->func_id, scmi_info->cap_id, 0,
@@ -250,7 +253,7 @@ static void smc_fetch_response(struct scmi_chan_info *cinfo,
 {
 	struct scmi_smc *scmi_info = cinfo->transport_info;
 
-	scmi_shmem_ops.fetch_response(scmi_info->shmem, xfer);
+	core->shmem->fetch_response(scmi_info->shmem, xfer);
 }
 
 static void smc_mark_txdone(struct scmi_chan_info *cinfo, int ret,
@@ -286,3 +289,19 @@ const struct scmi_desc scmi_smc_desc = {
 	.sync_cmds_completed_on_ret = true,
 	.atomic_enabled = IS_ENABLED(CONFIG_ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE),
 };
+
+static const struct of_device_id scmi_of_match[] = {
+	{ .compatible = "arm,scmi-smc" },
+	{ .compatible = "arm,scmi-smc-param" },
+	{ .compatible = "qcom,scmi-smc" },
+	{ /* Sentinel */ },
+};
+
+DEFINE_SCMI_TRANSPORT_DRIVER(scmi_smc, scmi_of_match, &core);
+module_platform_driver(scmi_smc_driver);
+
+MODULE_ALIAS("scmi-transport-smc");
+MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
+MODULE_AUTHOR("Nikunj Kela <quic_nkela@quicinc.com>");
+MODULE_DESCRIPTION("SCMI SMC Transport driver");
+MODULE_LICENSE("GPL");
-- 
2.45.2

[PATCH 6/8] firmware: arm_scmi: Make OPTEE transport a standalone driver

[Cristian Marussi]

Make SCMI OPTEE transport a standalone driver that can be optionally
loaded as a module.

CC: Etienne Carriere <etienne.carriere@foss.st.com>
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
 drivers/firmware/arm_scmi/Kconfig             |  6 +-
 drivers/firmware/arm_scmi/Makefile            |  2 +-
 drivers/firmware/arm_scmi/common.h            |  3 -
 drivers/firmware/arm_scmi/driver.c            |  3 -
 .../{optee.c => scmi_transport_optee.c}       | 87 +++++++++----------
 5 files changed, 47 insertions(+), 54 deletions(-)
 rename drivers/firmware/arm_scmi/{optee.c => scmi_transport_optee.c} (91%)

diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
index a4d44ef8bf45..def7e3f09356 100644
--- a/drivers/firmware/arm_scmi/Kconfig
+++ b/drivers/firmware/arm_scmi/Kconfig
@@ -89,8 +89,8 @@ config ARM_SCMI_TRANSPORT_MAILBOX
 	  will be called scmi_transport_mailbox.
 
 config ARM_SCMI_TRANSPORT_OPTEE
-	bool "SCMI transport based on OP-TEE service"
-	depends on OPTEE=y || OPTEE=ARM_SCMI_PROTOCOL
+	tristate "SCMI transport based on OP-TEE service"
+	depends on OPTEE
 	select ARM_SCMI_HAVE_TRANSPORT
 	select ARM_SCMI_HAVE_SHMEM
 	select ARM_SCMI_HAVE_MSG
@@ -100,6 +100,8 @@ config ARM_SCMI_TRANSPORT_OPTEE
 
 	  If you want the ARM SCMI PROTOCOL stack to include support for a
 	  transport based on OP-TEE SCMI service, answer Y.
+	  This driver can also be built as a module.  If so, the module
+	  will be called scmi_transport_optee.
 
 config ARM_SCMI_TRANSPORT_SMC
 	tristate "SCMI transport based on SMC"
diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index 6868a47fa4ab..b04119ce972f 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -7,13 +7,13 @@ scmi-driver-$(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT) += raw_mode.o
 scmi-transport-$(CONFIG_ARM_SCMI_HAVE_SHMEM) = shmem.o
 scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
 scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
-scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
 scmi-protocols-y := base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o powercap.o
 scmi-protocols-y += pinctrl.o
 scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
 
 obj-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += scmi_transport_smc.o
 obj-$(CONFIG_ARM_SCMI_TRANSPORT_MAILBOX) += scmi_transport_mailbox.o
+obj-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += scmi_transport_optee.o
 
 obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
 obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-module.o
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index b5bd27eccf24..bf6be0228646 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -289,9 +289,6 @@ int scmi_xfer_raw_wait_for_message_response(struct scmi_chan_info *cinfo,
 #ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
 extern const struct scmi_desc scmi_virtio_desc;
 #endif
-#ifdef CONFIG_ARM_SCMI_TRANSPORT_OPTEE
-extern const struct scmi_desc scmi_optee_desc;
-#endif
 
 void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr, void *priv);
 
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index b14c5326930a..67b416c7f2f5 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -3251,9 +3251,6 @@ ATTRIBUTE_GROUPS(versions);
 
 /* Each compatible listed below must have descriptor associated with it */
 static const struct of_device_id scmi_of_match[] = {
-#ifdef CONFIG_ARM_SCMI_TRANSPORT_OPTEE
-	{ .compatible = "linaro,scmi-optee", .data = &scmi_optee_desc },
-#endif
 #ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
 	{ .compatible = "arm,scmi-virtio", .data = &scmi_virtio_desc},
 #endif
diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/scmi_transport_optee.c
similarity index 91%
rename from drivers/firmware/arm_scmi/optee.c
rename to drivers/firmware/arm_scmi/scmi_transport_optee.c
index c26527f3b8f4..87c1b286042f 100644
--- a/drivers/firmware/arm_scmi/optee.c
+++ b/drivers/firmware/arm_scmi/scmi_transport_optee.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Copyright (C) 2019-2021 Linaro Ltd.
+ * Copyright (C) 2019-2024 Linaro Ltd.
  */
 
 #include <linux/io.h>
@@ -9,6 +9,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/tee_drv.h>
 #include <linux/uuid.h>
@@ -148,12 +149,11 @@ struct scmi_optee_agent {
 	struct list_head channel_list;
 };
 
+static struct scmi_transport_core_operations *core;
+
 /* There can be only 1 SCMI service in OP-TEE we connect to */
 static struct scmi_optee_agent *scmi_optee_private;
 
-/* Forward reference to scmi_optee transport initialization */
-static int scmi_optee_init(void);
-
 /* Open a session toward SCMI OP-TEE service with REE_KERNEL identity */
 static int open_session(struct scmi_optee_agent *agent, u32 *tee_session)
 {
@@ -312,24 +312,6 @@ static int invoke_process_msg_channel(struct scmi_optee_channel *channel, size_t
 	return 0;
 }
 
-static int scmi_optee_link_supplier(struct device *dev)
-{
-	if (!scmi_optee_private) {
-		if (scmi_optee_init())
-			dev_dbg(dev, "Optee bus not yet ready\n");
-
-		/* Wait for optee bus */
-		return -EPROBE_DEFER;
-	}
-
-	if (!device_link_add(dev, scmi_optee_private->dev, DL_FLAG_AUTOREMOVE_CONSUMER)) {
-		dev_err(dev, "Adding link to supplier optee device failed\n");
-		return -ECANCELED;
-	}
-
-	return 0;
-}
-
 static bool scmi_optee_chan_available(struct device_node *of_node, int idx)
 {
 	u32 channel_id;
@@ -343,7 +325,7 @@ static void scmi_optee_clear_channel(struct scmi_chan_info *cinfo)
 	struct scmi_optee_channel *channel = cinfo->transport_info;
 
 	if (!channel->tee_shm)
-		scmi_shmem_ops.clear_channel(channel->req.shmem);
+		core->shmem->clear_channel(channel->req.shmem);
 }
 
 static int setup_dynamic_shmem(struct device *dev, struct scmi_optee_channel *channel)
@@ -368,7 +350,7 @@ static int setup_dynamic_shmem(struct device *dev, struct scmi_optee_channel *ch
 static int setup_static_shmem(struct device *dev, struct scmi_chan_info *cinfo,
 			      struct scmi_optee_channel *channel)
 {
-	channel->req.shmem = scmi_shmem_ops.setup_iomap(cinfo, dev, true);
+	channel->req.shmem = core->shmem->setup_iomap(cinfo, dev, true);
 	if (IS_ERR(channel->req.shmem))
 		return PTR_ERR(channel->req.shmem);
 
@@ -472,10 +454,11 @@ static int scmi_optee_send_message(struct scmi_chan_info *cinfo,
 	mutex_lock(&channel->mu);
 
 	if (channel->tee_shm) {
-		scmi_msg_ops.tx_prepare(channel->req.msg, xfer);
-		ret = invoke_process_msg_channel(channel, scmi_msg_ops.command_size(xfer));
+		core->msg->tx_prepare(channel->req.msg, xfer);
+		ret = invoke_process_msg_channel(channel,
+						 core->msg->command_size(xfer));
 	} else {
-		scmi_shmem_ops.tx_prepare(channel->req.shmem, xfer, cinfo);
+		core->shmem->tx_prepare(channel->req.shmem, xfer, cinfo);
 		ret = invoke_process_smt_channel(channel);
 	}
 
@@ -491,9 +474,10 @@ static void scmi_optee_fetch_response(struct scmi_chan_info *cinfo,
 	struct scmi_optee_channel *channel = cinfo->transport_info;
 
 	if (channel->tee_shm)
-		scmi_msg_ops.fetch_response(channel->req.msg, channel->rx_len, xfer);
+		core->msg->fetch_response(channel->req.msg,
+					  channel->rx_len, xfer);
 	else
-		scmi_shmem_ops.fetch_response(channel->req.shmem, xfer);
+		core->shmem->fetch_response(channel->req.shmem, xfer);
 }
 
 static void scmi_optee_mark_txdone(struct scmi_chan_info *cinfo, int ret,
@@ -505,7 +489,6 @@ static void scmi_optee_mark_txdone(struct scmi_chan_info *cinfo, int ret,
 }
 
 static struct scmi_transport_ops scmi_optee_ops = {
-	.link_supplier = scmi_optee_link_supplier,
 	.chan_available = scmi_optee_chan_available,
 	.chan_setup = scmi_optee_chan_setup,
 	.chan_free = scmi_optee_chan_free,
@@ -520,6 +503,21 @@ static int scmi_optee_ctx_match(struct tee_ioctl_version_data *ver, const void *
 	return ver->impl_id == TEE_IMPL_ID_OPTEE;
 }
 
+const struct scmi_desc scmi_optee_desc = {
+	.ops = &scmi_optee_ops,
+	.max_rx_timeout_ms = 30,
+	.max_msg = 20,
+	.max_msg_size = SCMI_OPTEE_MAX_MSG_SIZE,
+	.sync_cmds_completed_on_ret = true,
+};
+
+static const struct of_device_id scmi_of_match[] = {
+	{ .compatible = "linaro,scmi-optee" },
+	{ /* Sentinel */ },
+};
+
+DEFINE_SCMI_TRANSPORT_DRIVER(scmi_optee, scmi_of_match, &core);
+
 static int scmi_optee_service_probe(struct device *dev)
 {
 	struct scmi_optee_agent *agent;
@@ -555,7 +553,7 @@ static int scmi_optee_service_probe(struct device *dev)
 	smp_mb();
 	scmi_optee_private = agent;
 
-	return 0;
+	return platform_driver_register(&scmi_optee_driver);
 
 err:
 	tee_client_close_context(tee_ctx);
@@ -573,6 +571,8 @@ static int scmi_optee_service_remove(struct device *dev)
 	if (!list_empty(&scmi_optee_private->channel_list))
 		return -EBUSY;
 
+	platform_driver_unregister(&scmi_optee_driver);
+
 	/* Ensure cleared reference is visible before resources are released */
 	smp_store_mb(scmi_optee_private, NULL);
 
@@ -591,7 +591,7 @@ static const struct tee_client_device_id scmi_optee_service_id[] = {
 
 MODULE_DEVICE_TABLE(tee, scmi_optee_service_id);
 
-static struct tee_client_driver scmi_optee_driver = {
+static struct tee_client_driver scmi_optee_service_driver = {
 	.id_table	= scmi_optee_service_id,
 	.driver		= {
 		.name = "scmi-optee",
@@ -601,22 +601,19 @@ static struct tee_client_driver scmi_optee_driver = {
 	},
 };
 
-static int scmi_optee_init(void)
+static int __init scmi_transport_optee_init(void)
 {
-	return driver_register(&scmi_optee_driver.driver);
+	return driver_register(&scmi_optee_service_driver.driver);
 }
+module_init(scmi_transport_optee_init);
 
-static void scmi_optee_exit(void)
+static void __exit scmi_transport_optee_exit(void)
 {
-	if (scmi_optee_private)
-		driver_unregister(&scmi_optee_driver.driver);
+	driver_unregister(&scmi_optee_service_driver.driver);
 }
+module_exit(scmi_transport_optee_exit);
 
-const struct scmi_desc scmi_optee_desc = {
-	.transport_exit = scmi_optee_exit,
-	.ops = &scmi_optee_ops,
-	.max_rx_timeout_ms = 30,
-	.max_msg = 20,
-	.max_msg_size = SCMI_OPTEE_MAX_MSG_SIZE,
-	.sync_cmds_completed_on_ret = true,
-};
+MODULE_ALIAS("scmi-transport-optee");
+MODULE_AUTHOR("Etienne Carriere <etienne.carriere@foss.st.com>");
+MODULE_DESCRIPTION("SCMI OPTEE Transport driver");
+MODULE_LICENSE("GPL");
-- 
2.45.2

[PATCH 7/8] firmware: arm_scmi: Make VirtIO transport a standalone driver

[Cristian Marussi]

Make SCMI VirtIO transport a standalone driver that can be optionally
loaded as a module.

CC: Igor Skalkin <igor.skalkin@opensynergy.com>
CC: Peter Hilber <peter.hilber@opensynergy.com>
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
 drivers/firmware/arm_scmi/Kconfig             |  6 +-
 drivers/firmware/arm_scmi/Makefile            |  2 +-
 drivers/firmware/arm_scmi/common.h            |  3 -
 drivers/firmware/arm_scmi/driver.c            |  3 -
 .../{virtio.c => scmi_transport_virtio.c}     | 95 +++++++++----------
 5 files changed, 50 insertions(+), 59 deletions(-)
 rename drivers/firmware/arm_scmi/{virtio.c => scmi_transport_virtio.c} (95%)

diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
index def7e3f09356..c18d86e9d8d7 100644
--- a/drivers/firmware/arm_scmi/Kconfig
+++ b/drivers/firmware/arm_scmi/Kconfig
@@ -132,8 +132,8 @@ config ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE
 	  primitives all over instead. If unsure say N.
 
 config ARM_SCMI_TRANSPORT_VIRTIO
-	bool "SCMI transport based on VirtIO"
-	depends on VIRTIO=y || VIRTIO=ARM_SCMI_PROTOCOL
+	tristate "SCMI transport based on VirtIO"
+	depends on VIRTIO
 	select ARM_SCMI_HAVE_TRANSPORT
 	select ARM_SCMI_HAVE_MSG
 	help
@@ -141,6 +141,8 @@ config ARM_SCMI_TRANSPORT_VIRTIO
 
 	  If you want the ARM SCMI PROTOCOL stack to include support for a
 	  transport based on VirtIO, answer Y.
+	  This driver can also be built as a module.  If so, the module
+	  will be called scmi_transport_virtio.
 
 config ARM_SCMI_TRANSPORT_VIRTIO_VERSION1_COMPLIANCE
 	bool "SCMI VirtIO transport Version 1 compliance"
diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index b04119ce972f..846b4939258d 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -6,12 +6,12 @@ scmi-driver-y = driver.o notify.o
 scmi-driver-$(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT) += raw_mode.o
 scmi-transport-$(CONFIG_ARM_SCMI_HAVE_SHMEM) = shmem.o
 scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
-scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
 scmi-protocols-y := base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o powercap.o
 scmi-protocols-y += pinctrl.o
 scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
 
 obj-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += scmi_transport_smc.o
+obj-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += scmi_transport_virtio.o
 obj-$(CONFIG_ARM_SCMI_TRANSPORT_MAILBOX) += scmi_transport_mailbox.o
 obj-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += scmi_transport_optee.o
 
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index bf6be0228646..102b28c3f22a 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -286,9 +286,6 @@ int scmi_xfer_raw_inflight_register(const struct scmi_handle *handle,
 int scmi_xfer_raw_wait_for_message_response(struct scmi_chan_info *cinfo,
 					    struct scmi_xfer *xfer,
 					    unsigned int timeout_ms);
-#ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
-extern const struct scmi_desc scmi_virtio_desc;
-#endif
 
 void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr, void *priv);
 
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 67b416c7f2f5..2a1f87c97abe 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -3251,9 +3251,6 @@ ATTRIBUTE_GROUPS(versions);
 
 /* Each compatible listed below must have descriptor associated with it */
 static const struct of_device_id scmi_of_match[] = {
-#ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
-	{ .compatible = "arm,scmi-virtio", .data = &scmi_virtio_desc},
-#endif
 	{ /* Sentinel */ },
 };
 
diff --git a/drivers/firmware/arm_scmi/virtio.c b/drivers/firmware/arm_scmi/scmi_transport_virtio.c
similarity index 95%
rename from drivers/firmware/arm_scmi/virtio.c
rename to drivers/firmware/arm_scmi/scmi_transport_virtio.c
index 736a0d41a458..7a4961049b28 100644
--- a/drivers/firmware/arm_scmi/virtio.c
+++ b/drivers/firmware/arm_scmi/scmi_transport_virtio.c
@@ -3,8 +3,8 @@
  * Virtio Transport driver for Arm System Control and Management Interface
  * (SCMI).
  *
- * Copyright (C) 2020-2022 OpenSynergy.
- * Copyright (C) 2021-2022 ARM Ltd.
+ * Copyright (C) 2020-2024 OpenSynergy.
+ * Copyright (C) 2021-2024 ARM Ltd.
  */
 
 /**
@@ -19,6 +19,7 @@
 
 #include <linux/completion.h>
 #include <linux/errno.h>
+#include <linux/platform_device.h>
 #include <linux/refcount.h>
 #include <linux/slab.h>
 #include <linux/virtio.h>
@@ -108,6 +109,8 @@ struct scmi_vio_msg {
 	refcount_t users;
 };
 
+static struct scmi_transport_core_operations *core;
+
 /* Only one SCMI VirtIO device can possibly exist */
 static struct virtio_device *scmi_vdev;
 
@@ -294,8 +297,9 @@ static void scmi_vio_complete_cb(struct virtqueue *vqueue)
 
 		if (msg) {
 			msg->rx_len = length;
-			scmi_rx_callback(vioch->cinfo,
-					 scmi_msg_ops.read_header(msg->input), msg);
+			core->rx_callback(vioch->cinfo,
+					  core->msg->read_header(msg->input),
+					  msg);
 
 			scmi_finalize_message(vioch, msg);
 		}
@@ -339,8 +343,9 @@ static void scmi_vio_deferred_tx_worker(struct work_struct *work)
 		 * is no more processed elsewhere so no poll_lock needed.
 		 */
 		if (msg->poll_status == VIO_MSG_NOT_POLLED)
-			scmi_rx_callback(vioch->cinfo,
-					 scmi_msg_ops.read_header(msg->input), msg);
+			core->rx_callback(vioch->cinfo,
+					  core->msg->read_header(msg->input),
+					  msg);
 
 		/* Free the processed message once done */
 		scmi_vio_msg_release(vioch, msg);
@@ -368,23 +373,6 @@ static unsigned int virtio_get_max_msg(struct scmi_chan_info *base_cinfo)
 	return vioch->max_msg;
 }
 
-static int virtio_link_supplier(struct device *dev)
-{
-	if (!scmi_vdev) {
-		dev_notice(dev,
-			   "Deferring probe after not finding a bound scmi-virtio device\n");
-		return -EPROBE_DEFER;
-	}
-
-	if (!device_link_add(dev, &scmi_vdev->dev,
-			     DL_FLAG_AUTOREMOVE_CONSUMER)) {
-		dev_err(dev, "Adding link to supplier virtio device failed\n");
-		return -ECANCELED;
-	}
-
-	return 0;
-}
-
 static bool virtio_chan_available(struct device_node *of_node, int idx)
 {
 	struct scmi_vio_channel *channels, *vioch = NULL;
@@ -512,10 +500,10 @@ static int virtio_send_message(struct scmi_chan_info *cinfo,
 		return -EBUSY;
 	}
 
-	scmi_msg_ops.tx_prepare(msg->request, xfer);
+	core->msg->tx_prepare(msg->request, xfer);
 
-	sg_init_one(&sg_out, msg->request, scmi_msg_ops.command_size(xfer));
-	sg_init_one(&sg_in, msg->input, scmi_msg_ops.response_size(xfer));
+	sg_init_one(&sg_out, msg->request, core->msg->command_size(xfer));
+	sg_init_one(&sg_in, msg->input, core->msg->response_size(xfer));
 
 	spin_lock_irqsave(&vioch->lock, flags);
 
@@ -562,7 +550,7 @@ static void virtio_fetch_response(struct scmi_chan_info *cinfo,
 	struct scmi_vio_msg *msg = xfer->priv;
 
 	if (msg)
-		scmi_msg_ops.fetch_response(msg->input, msg->rx_len, xfer);
+		core->msg->fetch_response(msg->input, msg->rx_len, xfer);
 }
 
 static void virtio_fetch_notification(struct scmi_chan_info *cinfo,
@@ -571,7 +559,8 @@ static void virtio_fetch_notification(struct scmi_chan_info *cinfo,
 	struct scmi_vio_msg *msg = xfer->priv;
 
 	if (msg)
-		scmi_msg_ops.fetch_notification(msg->input, msg->rx_len, max_len, xfer);
+		core->msg->fetch_notification(msg->input, msg->rx_len,
+					      max_len, xfer);
 }
 
 /**
@@ -671,7 +660,7 @@ static void virtio_mark_txdone(struct scmi_chan_info *cinfo, int ret,
  * the message we are polling for could be alternatively delivered via usual
  * IRQs callbacks on another core which happened to have IRQs enabled while we
  * are actively polling for it here: in such a case it will be handled as such
- * by scmi_rx_callback() and the polling loop in the SCMI Core TX path will be
+ * by rx_callback() and the polling loop in the SCMI Core TX path will be
  * transparently terminated anyway.
  *
  * Return: True once polling has successfully completed.
@@ -792,7 +781,6 @@ static bool virtio_poll_done(struct scmi_chan_info *cinfo,
 }
 
 static const struct scmi_transport_ops scmi_virtio_ops = {
-	.link_supplier = virtio_link_supplier,
 	.chan_available = virtio_chan_available,
 	.chan_setup = virtio_chan_setup,
 	.chan_free = virtio_chan_free,
@@ -804,6 +792,22 @@ static const struct scmi_transport_ops scmi_virtio_ops = {
 	.poll_done = virtio_poll_done,
 };
 
+const struct scmi_desc scmi_virtio_desc = {
+	.ops = &scmi_virtio_ops,
+	/* for non-realtime virtio devices */
+	.max_rx_timeout_ms = VIRTIO_MAX_RX_TIMEOUT_MS,
+	.max_msg = 0, /* overridden by virtio_get_max_msg() */
+	.max_msg_size = VIRTIO_SCMI_MAX_MSG_SIZE,
+	.atomic_enabled = IS_ENABLED(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO_ATOMIC_ENABLE),
+};
+
+static const struct of_device_id scmi_of_match[] = {
+	{ .compatible = "arm,scmi-virtio" },
+	{ /* Sentinel */ },
+};
+
+DEFINE_SCMI_TRANSPORT_DRIVER(scmi_virtio, scmi_of_match, &core);
+
 static int scmi_vio_probe(struct virtio_device *vdev)
 {
 	struct device *dev = &vdev->dev;
@@ -864,14 +868,17 @@ static int scmi_vio_probe(struct virtio_device *vdev)
 	}
 
 	vdev->priv = channels;
+
 	/* Ensure initialized scmi_vdev is visible */
 	smp_store_mb(scmi_vdev, vdev);
 
-	return 0;
+	return platform_driver_register(&scmi_virtio_driver);
 }
 
 static void scmi_vio_remove(struct virtio_device *vdev)
 {
+	platform_driver_unregister(&scmi_virtio_driver);
+
 	/*
 	 * Once we get here, virtio_chan_free() will have already been called by
 	 * the SCMI core for any existing channel and, as a consequence, all the
@@ -916,23 +923,11 @@ static struct virtio_driver virtio_scmi_driver = {
 	.validate = scmi_vio_validate,
 };
 
-static int __init virtio_scmi_init(void)
-{
-	return register_virtio_driver(&virtio_scmi_driver);
-}
+module_virtio_driver(virtio_scmi_driver);
 
-static void virtio_scmi_exit(void)
-{
-	unregister_virtio_driver(&virtio_scmi_driver);
-}
-
-const struct scmi_desc scmi_virtio_desc = {
-	.transport_init = virtio_scmi_init,
-	.transport_exit = virtio_scmi_exit,
-	.ops = &scmi_virtio_ops,
-	/* for non-realtime virtio devices */
-	.max_rx_timeout_ms = VIRTIO_MAX_RX_TIMEOUT_MS,
-	.max_msg = 0, /* overridden by virtio_get_max_msg() */
-	.max_msg_size = VIRTIO_SCMI_MAX_MSG_SIZE,
-	.atomic_enabled = IS_ENABLED(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO_ATOMIC_ENABLE),
-};
+MODULE_ALIAS("scmi-transport-virtio");
+MODULE_AUTHOR("Igor Skalkin <igor.skalkin@opensynergy.com>");
+MODULE_AUTHOR("Peter Hilber <peter.hilber@opensynergy.com>");
+MODULE_AUTHOR("Cristian Marussi <cristian.marussi@arm.com>");
+MODULE_DESCRIPTION("SCMI VirtIO Transport driver");
+MODULE_LICENSE("GPL");
-- 
2.45.2

[PATCH 8/8] firmware: arm_scmi: Remove legacy transport-layer code

[Cristian Marussi]

Since all SCMI transports have been made standalone drivers, remove all the
core SCMI stack legacy support that was needed to run transports as built
into the stack.

Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
 drivers/firmware/arm_scmi/common.h | 18 ------
 drivers/firmware/arm_scmi/driver.c | 96 ++++--------------------------
 drivers/firmware/arm_scmi/msg.c    |  2 +-
 drivers/firmware/arm_scmi/shmem.c  |  2 +-
 4 files changed, 15 insertions(+), 103 deletions(-)

diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 102b28c3f22a..993bd9df7dd4 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -183,7 +183,6 @@ struct scmi_chan_info {
 /**
  * struct scmi_transport_ops - Structure representing a SCMI transport ops
  *
- * @link_supplier: Optional callback to add link to a supplier device
  * @chan_available: Callback to check if channel is available or not
  * @chan_setup: Callback to allocate and setup a channel
  * @chan_free: Callback to free a channel
@@ -198,7 +197,6 @@ struct scmi_chan_info {
  * @poll_done: Callback to poll transfer status
  */
 struct scmi_transport_ops {
-	int (*link_supplier)(struct device *dev);
 	bool (*chan_available)(struct device_node *of_node, int idx);
 	int (*chan_setup)(struct scmi_chan_info *cinfo, struct device *dev,
 			  bool tx);
@@ -219,12 +217,6 @@ struct scmi_transport_ops {
 /**
  * struct scmi_desc - Description of SoC integration
  *
- * @transport_init: An optional function that a transport can provide to
- *		    initialize some transport-specific setup during SCMI core
- *		    initialization, so ahead of SCMI core probing.
- * @transport_exit: An optional function that a transport can provide to
- *		    de-initialize some transport-specific setup during SCMI core
- *		    de-initialization, so after SCMI core removal.
  * @ops: Pointer to the transport specific ops structure
  * @max_rx_timeout_ms: Timeout for communication with SoC (in Milliseconds)
  * @max_msg: Maximum number of messages for a channel type (tx or rx) that can
@@ -245,8 +237,6 @@ struct scmi_transport_ops {
  *		    when requested.
  */
 struct scmi_desc {
-	int (*transport_init)(void);
-	void (*transport_exit)(void);
 	const struct scmi_transport_ops *ops;
 	int max_rx_timeout_ms;
 	int max_msg;
@@ -287,8 +277,6 @@ int scmi_xfer_raw_wait_for_message_response(struct scmi_chan_info *cinfo,
 					    struct scmi_xfer *xfer,
 					    unsigned int timeout_ms);
 
-void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr, void *priv);
-
 enum scmi_bad_msg {
 	MSG_UNEXPECTED = -1,
 	MSG_INVALID = -2,
@@ -297,9 +285,6 @@ enum scmi_bad_msg {
 	MSG_MBOX_SPURIOUS = -5,
 };
 
-void scmi_bad_message_trace(struct scmi_chan_info *cinfo, u32 msg_hdr,
-			    enum scmi_bad_msg err);
-
 /* shmem related declarations */
 struct scmi_shared_mem;
 
@@ -447,9 +432,6 @@ static struct platform_driver __trans##_driver = {			\
 	.remove_new = __trans##_remove,					\
 }
 
-extern const struct scmi_shared_mem_operations scmi_shmem_ops;
-extern const struct scmi_message_operations scmi_msg_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);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 2a1f87c97abe..ea0595193dec 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -194,6 +194,11 @@ struct scmi_info {
 #define bus_nb_to_scmi_info(nb)	container_of(nb, struct scmi_info, bus_nb)
 #define req_nb_to_scmi_info(nb)	container_of(nb, struct scmi_info, dev_req_nb)
 
+static void scmi_rx_callback(struct scmi_chan_info *cinfo,
+			     u32 msg_hdr, void *priv);
+static void scmi_bad_message_trace(struct scmi_chan_info *cinfo,
+				   u32 msg_hdr, enum scmi_bad_msg err);
+
 static struct scmi_transport_core_operations scmi_trans_core_ops = {
 	.bad_message_trace = scmi_bad_message_trace,
 	.rx_callback = scmi_rx_callback,
@@ -838,8 +843,8 @@ scmi_xfer_lookup_unlocked(struct scmi_xfers_info *minfo, u16 xfer_id)
  * timed-out message that arrives and as such, can be traced only referring to
  * the header content, since the payload is missing/unreliable.
  */
-void scmi_bad_message_trace(struct scmi_chan_info *cinfo, u32 msg_hdr,
-			    enum scmi_bad_msg err)
+static void scmi_bad_message_trace(struct scmi_chan_info *cinfo, u32 msg_hdr,
+				   enum scmi_bad_msg err)
 {
 	char *tag;
 	struct scmi_info *info = handle_to_scmi_info(cinfo->handle);
@@ -1165,7 +1170,8 @@ static void scmi_handle_response(struct scmi_chan_info *cinfo,
  * NOTE: This function will be invoked in IRQ context, hence should be
  * as optimal as possible.
  */
-void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr, void *priv)
+static void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr,
+			     void *priv)
 {
 	u8 msg_type = MSG_XTRACT_TYPE(msg_hdr);
 
@@ -2988,14 +2994,11 @@ static int scmi_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct device_node *child, *np = dev->of_node;
 
-	desc = of_device_get_match_data(dev);
+	desc = scmi_transport_lookup(dev);
 	if (!desc) {
-		desc = scmi_transport_lookup(dev);
-		if (!desc) {
-			err_str = "transport invalid\n";
-			ret = -EINVAL;
-			goto out_err;
-		}
+		err_str = "transport invalid\n";
+		ret = -EINVAL;
+		goto out_err;
 	}
 
 	info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
@@ -3035,14 +3038,6 @@ static int scmi_probe(struct platform_device *pdev)
 			 info->atomic_threshold);
 	handle->is_transport_atomic = scmi_is_transport_atomic;
 
-	if (desc->ops->link_supplier) {
-		ret = desc->ops->link_supplier(dev);
-		if (ret) {
-			err_str = "transport not ready\n";
-			goto clear_ida;
-		}
-	}
-
 	/* Setup all channels described in the DT at first */
 	ret = scmi_channels_setup(info);
 	if (ret) {
@@ -3249,72 +3244,16 @@ static struct attribute *versions_attrs[] = {
 };
 ATTRIBUTE_GROUPS(versions);
 
-/* Each compatible listed below must have descriptor associated with it */
-static const struct of_device_id scmi_of_match[] = {
-	{ /* Sentinel */ },
-};
-
-MODULE_DEVICE_TABLE(of, scmi_of_match);
-
 static struct platform_driver scmi_driver = {
 	.driver = {
 		   .name = "arm-scmi",
 		   .suppress_bind_attrs = true,
-		   .of_match_table = scmi_of_match,
 		   .dev_groups = versions_groups,
 		   },
 	.probe = scmi_probe,
 	.remove_new = scmi_remove,
 };
 
-/**
- * __scmi_transports_setup  - Common helper to call transport-specific
- * .init/.exit code if provided.
- *
- * @init: A flag to distinguish between init and exit.
- *
- * Note that, if provided, we invoke .init/.exit functions for all the
- * transports currently compiled in.
- *
- * Return: 0 on Success.
- */
-static inline int __scmi_transports_setup(bool init)
-{
-	int ret = 0;
-	const struct of_device_id *trans;
-
-	for (trans = scmi_of_match; trans->data; trans++) {
-		const struct scmi_desc *tdesc = trans->data;
-
-		if ((init && !tdesc->transport_init) ||
-		    (!init && !tdesc->transport_exit))
-			continue;
-
-		if (init)
-			ret = tdesc->transport_init();
-		else
-			tdesc->transport_exit();
-
-		if (ret) {
-			pr_err("SCMI transport %s FAILED initialization!\n",
-			       trans->compatible);
-			break;
-		}
-	}
-
-	return ret;
-}
-
-static int __init scmi_transports_init(void)
-{
-	return __scmi_transports_setup(true);
-}
-
-static void __exit scmi_transports_exit(void)
-{
-	__scmi_transports_setup(false);
-}
-
 static struct dentry *scmi_debugfs_init(void)
 {
 	struct dentry *d;
@@ -3330,17 +3269,10 @@ static struct dentry *scmi_debugfs_init(void)
 
 static int __init scmi_driver_init(void)
 {
-	int ret;
-
 	/* Bail out if no SCMI transport was configured */
 	if (WARN_ON(!IS_ENABLED(CONFIG_ARM_SCMI_HAVE_TRANSPORT)))
 		return -EINVAL;
 
-	/* Initialize any compiled-in transport which provided an init/exit */
-	ret = scmi_transports_init();
-	if (ret)
-		return ret;
-
 	if (IS_ENABLED(CONFIG_ARM_SCMI_HAVE_SHMEM))
 		scmi_trans_core_ops.shmem = scmi_shared_mem_operations_get();
 
@@ -3380,8 +3312,6 @@ static void __exit scmi_driver_exit(void)
 	scmi_powercap_unregister();
 	scmi_pinctrl_unregister();
 
-	scmi_transports_exit();
-
 	platform_driver_unregister(&scmi_driver);
 
 	debugfs_remove_recursive(scmi_top_dentry);
diff --git a/drivers/firmware/arm_scmi/msg.c b/drivers/firmware/arm_scmi/msg.c
index 0bed1d2825af..ca916817878d 100644
--- a/drivers/firmware/arm_scmi/msg.c
+++ b/drivers/firmware/arm_scmi/msg.c
@@ -110,7 +110,7 @@ static void msg_fetch_notification(struct scmi_msg_payld *msg, size_t len,
 	memcpy(xfer->rx.buf, msg->msg_payload, xfer->rx.len);
 }
 
-const struct scmi_message_operations scmi_msg_ops = {
+static const struct scmi_message_operations scmi_msg_ops = {
 	.tx_prepare = msg_tx_prepare,
 	.command_size = msg_command_size,
 	.response_size = msg_response_size,
diff --git a/drivers/firmware/arm_scmi/shmem.c b/drivers/firmware/arm_scmi/shmem.c
index feb4215831dc..f450367e4386 100644
--- a/drivers/firmware/arm_scmi/shmem.c
+++ b/drivers/firmware/arm_scmi/shmem.c
@@ -171,7 +171,7 @@ static void *__iomem shmem_setup_iomap(struct scmi_chan_info *cinfo,
 	return addr;
 }
 
-const struct scmi_shared_mem_operations scmi_shmem_ops = {
+static const struct scmi_shared_mem_operations scmi_shmem_ops = {
 	.tx_prepare = shmem_tx_prepare,
 	.read_header = shmem_read_header,
 	.fetch_response = shmem_fetch_response,
-- 
2.45.2

Re: [PATCH 5/8] firmware: arm_scmi: Make SMC transport a standalone driver

[kernel test robot]

Hi Cristian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on soc/for-next]
[also build test WARNING on next-20240703]
[cannot apply to linus/master v6.10-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Cristian-Marussi/firmware-arm_scmi-Introduce-setup_shmem_iomap/20240707-082513
base:   https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link:    https://lore.kernel.org/r/20240707002055.1835121-6-cristian.marussi%40arm.com
patch subject: [PATCH 5/8] firmware: arm_scmi: Make SMC transport a standalone driver
config: arm-defconfig (https://download.01.org/0day-ci/archive/20240707/202407072316.PqU8yj52-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240707/202407072316.PqU8yj52-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407072316.PqU8yj52-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/firmware/arm_scmi/scmi_transport_smc.c:157:58: warning: variable 'size' is uninitialized when used here [-Wuninitialized]
                   void __iomem *ptr = (void __iomem *)scmi_info->shmem + size - 8;
                                                                          ^~~~
   drivers/firmware/arm_scmi/scmi_transport_smc.c:136:22: note: initialize the variable 'size' to silence this warning
           resource_size_t size;
                               ^
                                = 0
   1 warning generated.


vim +/size +157 drivers/firmware/arm_scmi/scmi_transport_smc.c

0bfdca8a8661aa drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  129  
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  130  static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  131  			  bool tx)
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  132  {
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  133  	struct device *cdev = cinfo->dev;
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  134  	unsigned long cap_id = ULONG_MAX;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  135  	struct scmi_smc *scmi_info;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  136  	resource_size_t size;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  137  	struct resource res;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  138  	u32 func_id;
d1ff11d7ad8704 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2023-07-19  139  	int ret;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  140  
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  141  	if (!tx)
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  142  		return -ENODEV;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  143  
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  144  	scmi_info = devm_kzalloc(dev, sizeof(*scmi_info), GFP_KERNEL);
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  145  	if (!scmi_info)
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  146  		return -ENOMEM;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  147  
728a057b6ab114 drivers/firmware/arm_scmi/scmi_transport_smc.c Cristian Marussi 2024-07-07  148  	scmi_info->shmem = core->shmem->setup_iomap(cinfo, dev, tx);
6c5d3315c40fa0 drivers/firmware/arm_scmi/smc.c                Peng Fan         2024-07-07  149  	if (IS_ERR(scmi_info->shmem))
6c5d3315c40fa0 drivers/firmware/arm_scmi/smc.c                Peng Fan         2024-07-07  150  		return PTR_ERR(scmi_info->shmem);
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  151  
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  152  	ret = of_property_read_u32(dev->of_node, "arm,smc-id", &func_id);
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  153  	if (ret < 0)
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  154  		return ret;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  155  
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  156  	if (of_device_is_compatible(dev->of_node, "qcom,scmi-smc")) {
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09 @157  		void __iomem *ptr = (void __iomem *)scmi_info->shmem + size - 8;
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  158  		/* The capability-id is kept in last 8 bytes of shmem.
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  159  		 *     +-------+ <-- 0
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  160  		 *     | shmem |
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  161  		 *     +-------+ <-- size - 8
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  162  		 *     | capId |
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  163  		 *     +-------+ <-- size
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  164  		 */
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  165  		memcpy_fromio(&cap_id, ptr, sizeof(cap_id));
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  166  	}
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  167  
5f2ea10a808aef drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-05-06  168  	if (of_device_is_compatible(dev->of_node, "arm,scmi-smc-param")) {
5f2ea10a808aef drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-05-06  169  		scmi_info->param_page = SHMEM_PAGE(res.start);
5f2ea10a808aef drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-05-06  170  		scmi_info->param_offset = SHMEM_OFFSET(res.start);
5f2ea10a808aef drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-05-06  171  	}
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  172  	/*
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  173  	 * If there is an interrupt named "a2p", then the service and
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  174  	 * completion of a message is signaled by an interrupt rather than by
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  175  	 * the return of the SMC call.
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  176  	 */
d1ff11d7ad8704 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2023-07-19  177  	scmi_info->irq = of_irq_get_byname(cdev->of_node, "a2p");
d1ff11d7ad8704 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2023-07-19  178  	if (scmi_info->irq > 0) {
d1ff11d7ad8704 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2023-07-19  179  		ret = request_irq(scmi_info->irq, smc_msg_done_isr,
d1ff11d7ad8704 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2023-07-19  180  				  IRQF_NO_SUSPEND, dev_name(dev), scmi_info);
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  181  		if (ret) {
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  182  			dev_err(dev, "failed to setup SCMI smc irq\n");
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  183  			return ret;
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  184  		}
f716cbd33f038a drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  185  	} else {
f716cbd33f038a drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  186  		cinfo->no_completion_irq = true;
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  187  	}
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  188  
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  189  	scmi_info->func_id = func_id;
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  190  	scmi_info->cap_id = cap_id;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  191  	scmi_info->cinfo = cinfo;
0bfdca8a8661aa drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  192  	smc_channel_lock_init(scmi_info);
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  193  	cinfo->transport_info = scmi_info;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  194  
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  195  	return 0;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  196  }
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  197  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/8] firmware: arm_scmi: Introduce setup_shmem_iomap

[Nikunj Kela]

On 7/6/2024 5:20 PM, Cristian Marussi wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> To get the address of shmem could be generalized by introducing
> setup_shmem_iomap. Then the duplicated code in mailbox.c, optee.c
> and smc.c could be dropped.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> [ Cristian: make use of the new helper also in smc.c ]
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
>  drivers/firmware/arm_scmi/common.h  |  2 ++
>  drivers/firmware/arm_scmi/mailbox.c | 27 ++++------------------
>  drivers/firmware/arm_scmi/optee.c   | 35 ++++------------------------
>  drivers/firmware/arm_scmi/shmem.c   | 36 +++++++++++++++++++++++++++++
>  drivers/firmware/arm_scmi/smc.c     | 23 +++---------------
>  5 files changed, 49 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
> index 4b8c5250cdb5..b4c217641e3a 100644
> --- a/drivers/firmware/arm_scmi/common.h
> +++ b/drivers/firmware/arm_scmi/common.h
> @@ -327,6 +327,8 @@ bool shmem_poll_done(struct scmi_shared_mem __iomem *shmem,
>  		     struct scmi_xfer *xfer);
>  bool shmem_channel_free(struct scmi_shared_mem __iomem *shmem);
>  bool shmem_channel_intr_enabled(struct scmi_shared_mem __iomem *shmem);
> +void __iomem *setup_shmem_iomap(struct scmi_chan_info *cinfo, struct device *dev,
> +				bool tx);
>  
>  /* declarations for message passing transports */
>  struct scmi_msg_payld;
> diff --git a/drivers/firmware/arm_scmi/mailbox.c b/drivers/firmware/arm_scmi/mailbox.c
> index 0219a12e3209..b0a579f31905 100644
> --- a/drivers/firmware/arm_scmi/mailbox.c
> +++ b/drivers/firmware/arm_scmi/mailbox.c
> @@ -178,11 +178,8 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
>  	const char *desc = tx ? "Tx" : "Rx";
>  	struct device *cdev = cinfo->dev;
>  	struct scmi_mailbox *smbox;
> -	struct device_node *shmem;
> -	int ret, a2p_rx_chan, p2a_chan, p2a_rx_chan, idx = tx ? 0 : 1;
> +	int ret, a2p_rx_chan, p2a_chan, p2a_rx_chan;
>  	struct mbox_client *cl;
> -	resource_size_t size;
> -	struct resource res;
>  
>  	ret = mailbox_chan_validate(cdev, &a2p_rx_chan, &p2a_chan, &p2a_rx_chan);
>  	if (ret)
> @@ -195,25 +192,9 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
>  	if (!smbox)
>  		return -ENOMEM;
>  
> -	shmem = of_parse_phandle(cdev->of_node, "shmem", idx);
> -	if (!of_device_is_compatible(shmem, "arm,scmi-shmem")) {
> -		of_node_put(shmem);
> -		return -ENXIO;
> -	}
> -
> -	ret = of_address_to_resource(shmem, 0, &res);
> -	of_node_put(shmem);
> -	if (ret) {
> -		dev_err(cdev, "failed to get SCMI %s shared memory\n", desc);
> -		return ret;
> -	}
> -
> -	size = resource_size(&res);
> -	smbox->shmem = devm_ioremap(dev, res.start, size);
> -	if (!smbox->shmem) {
> -		dev_err(dev, "failed to ioremap SCMI %s shared memory\n", desc);
> -		return -EADDRNOTAVAIL;
> -	}
> +	smbox->shmem = setup_shmem_iomap(cinfo, dev, tx);
> +	if (IS_ERR(smbox->shmem))
> +		return PTR_ERR(smbox->shmem);
>  
>  	cl = &smbox->cl;
>  	cl->dev = cdev;
> diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c
> index 4e7944b91e38..8abcd668108c 100644
> --- a/drivers/firmware/arm_scmi/optee.c
> +++ b/drivers/firmware/arm_scmi/optee.c
> @@ -368,38 +368,11 @@ static int setup_dynamic_shmem(struct device *dev, struct scmi_optee_channel *ch
>  static int setup_static_shmem(struct device *dev, struct scmi_chan_info *cinfo,
>  			      struct scmi_optee_channel *channel)
>  {
> -	struct device_node *np;
> -	resource_size_t size;
> -	struct resource res;
> -	int ret;
> -
> -	np = of_parse_phandle(cinfo->dev->of_node, "shmem", 0);
> -	if (!of_device_is_compatible(np, "arm,scmi-shmem")) {
> -		ret = -ENXIO;
> -		goto out;
> -	}
> -
> -	ret = of_address_to_resource(np, 0, &res);
> -	if (ret) {
> -		dev_err(dev, "Failed to get SCMI Tx shared memory\n");
> -		goto out;
> -	}
> -
> -	size = resource_size(&res);
> +	channel->req.shmem = setup_shmem_iomap(cinfo, dev, true);
> +	if (IS_ERR(channel->req.shmem))
> +		return PTR_ERR(channel->req.shmem);
>  
> -	channel->req.shmem = devm_ioremap(dev, res.start, size);
> -	if (!channel->req.shmem) {
> -		dev_err(dev, "Failed to ioremap SCMI Tx shared memory\n");
> -		ret = -EADDRNOTAVAIL;
> -		goto out;
> -	}
> -
> -	ret = 0;
> -
> -out:
> -	of_node_put(np);
> -
> -	return ret;
> +	return 0;
>  }
>  
>  static int setup_shmem(struct device *dev, struct scmi_chan_info *cinfo,
> diff --git a/drivers/firmware/arm_scmi/shmem.c b/drivers/firmware/arm_scmi/shmem.c
> index b74e5a740f2c..c31f188d74ef 100644
> --- a/drivers/firmware/arm_scmi/shmem.c
> +++ b/drivers/firmware/arm_scmi/shmem.c
> @@ -7,6 +7,8 @@
>  
>  #include <linux/ktime.h>
>  #include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
>  #include <linux/processor.h>
>  #include <linux/types.h>
>  
> @@ -133,3 +135,37 @@ bool shmem_channel_intr_enabled(struct scmi_shared_mem __iomem *shmem)
>  {
>  	return ioread32(&shmem->flags) & SCMI_SHMEM_FLAG_INTR_ENABLED;
>  }
> +
> +void *__iomem
> +setup_shmem_iomap(struct scmi_chan_info *cinfo, struct device *dev, bool tx)
> +{
> +	const char *desc = tx ? "Tx" : "Rx";
> +	int ret, idx = tx ? 0 : 1;
> +	struct device *cdev = cinfo->dev;
> +	struct device_node *shmem;
> +	resource_size_t size;
> +	struct resource res;
> +	void __iomem *addr;
> +
> +	shmem = of_parse_phandle(cdev->of_node, "shmem", idx);
> +	if (!of_device_is_compatible(shmem, "arm,scmi-shmem")) {
> +		of_node_put(shmem);
> +		return ERR_PTR(-ENXIO);
> +	}
> +
> +	ret = of_address_to_resource(shmem, 0, &res);
> +	of_node_put(shmem);
> +	if (ret) {
> +		dev_err(cdev, "failed to get SCMI %s shared memory\n", desc);
> +		return ERR_PTR(ret);
> +	}
> +
> +	size = resource_size(&res);
> +	addr = devm_ioremap(dev, res.start, size);
> +	if (!addr) {
> +		dev_err(dev, "failed to ioremap SCMI %s shared memory\n", desc);
> +		return ERR_PTR(-EADDRNOTAVAIL);
> +	}
> +
> +	return addr;
> +}
> diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c
> index 39936e1dd30e..a640a4312472 100644
> --- a/drivers/firmware/arm_scmi/smc.c
> +++ b/drivers/firmware/arm_scmi/smc.c
> @@ -132,7 +132,6 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
>  	struct scmi_smc *scmi_info;
>  	resource_size_t size;
>  	struct resource res;
> -	struct device_node *np;
>  	u32 func_id;
>  	int ret;
>  
> @@ -143,25 +142,9 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
>  	if (!scmi_info)
>  		return -ENOMEM;
>  
> -	np = of_parse_phandle(cdev->of_node, "shmem", 0);
> -	if (!of_device_is_compatible(np, "arm,scmi-shmem")) {
> -		of_node_put(np);
> -		return -ENXIO;
> -	}
> -
> -	ret = of_address_to_resource(np, 0, &res);
> -	of_node_put(np);
> -	if (ret) {
> -		dev_err(cdev, "failed to get SCMI Tx shared memory\n");
> -		return ret;
> -	}
> -
> -	size = resource_size(&res);

Hi Peng/Cristian,

This will break Qualcomm smc transport as we need shmem 'size' to get to
the cap-id. 

Thanks,

-Nikunj

> -	scmi_info->shmem = devm_ioremap(dev, res.start, size);
> -	if (!scmi_info->shmem) {
> -		dev_err(dev, "failed to ioremap SCMI Tx shared memory\n");
> -		return -EADDRNOTAVAIL;
> -	}
> +	scmi_info->shmem = setup_shmem_iomap(cinfo, dev, tx);
> +	if (IS_ERR(scmi_info->shmem))
> +		return PTR_ERR(scmi_info->shmem);
>  
>  	ret = of_property_read_u32(dev->of_node, "arm,smc-id", &func_id);
>  	if (ret < 0)

Re: [PATCH 5/8] firmware: arm_scmi: Make SMC transport a standalone driver

[Nikunj Kela]

On 7/6/2024 5:20 PM, Cristian Marussi wrote:
> Make SCMI SMC transport a standalone driver that can be optionally
> loaded as a module.
>
> CC: Peng Fan <peng.fan@nxp.com>
> CC: Nikunj Kela <quic_nkela@quicinc.com>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
>  drivers/firmware/arm_scmi/Kconfig             |  4 ++-
>  drivers/firmware/arm_scmi/Makefile            |  2 +-
>  drivers/firmware/arm_scmi/common.h            |  3 --
>  drivers/firmware/arm_scmi/driver.c            |  5 ---
>  .../arm_scmi/{smc.c => scmi_transport_smc.c}  | 31 +++++++++++++++----
>  5 files changed, 29 insertions(+), 16 deletions(-)
>  rename drivers/firmware/arm_scmi/{smc.c => scmi_transport_smc.c} (89%)
>
> diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
> index 135e34aefd70..a4d44ef8bf45 100644
> --- a/drivers/firmware/arm_scmi/Kconfig
> +++ b/drivers/firmware/arm_scmi/Kconfig
> @@ -102,7 +102,7 @@ config ARM_SCMI_TRANSPORT_OPTEE
>  	  transport based on OP-TEE SCMI service, answer Y.
>  
>  config ARM_SCMI_TRANSPORT_SMC
> -	bool "SCMI transport based on SMC"
> +	tristate "SCMI transport based on SMC"
>  	depends on HAVE_ARM_SMCCC_DISCOVERY
>  	select ARM_SCMI_HAVE_TRANSPORT
>  	select ARM_SCMI_HAVE_SHMEM
> @@ -112,6 +112,8 @@ config ARM_SCMI_TRANSPORT_SMC
>  
>  	  If you want the ARM SCMI PROTOCOL stack to include support for a
>  	  transport based on SMC, answer Y.
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called scmi_transport_smc.
>  
>  config ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE
>  	bool "Enable atomic mode support for SCMI SMC transport"
> diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
> index 121612d75f0b..6868a47fa4ab 100644
> --- a/drivers/firmware/arm_scmi/Makefile
> +++ b/drivers/firmware/arm_scmi/Makefile
> @@ -5,7 +5,6 @@ scmi-core-objs := $(scmi-bus-y)
>  scmi-driver-y = driver.o notify.o
>  scmi-driver-$(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT) += raw_mode.o
>  scmi-transport-$(CONFIG_ARM_SCMI_HAVE_SHMEM) = shmem.o
> -scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += smc.o
>  scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
>  scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
>  scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
> @@ -13,6 +12,7 @@ scmi-protocols-y := base.o clock.o perf.o power.o reset.o sensors.o system.o vol
>  scmi-protocols-y += pinctrl.o
>  scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
>  
> +obj-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += scmi_transport_smc.o
>  obj-$(CONFIG_ARM_SCMI_TRANSPORT_MAILBOX) += scmi_transport_mailbox.o
>  
>  obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
> diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
> index c03f30db92e0..b5bd27eccf24 100644
> --- a/drivers/firmware/arm_scmi/common.h
> +++ b/drivers/firmware/arm_scmi/common.h
> @@ -286,9 +286,6 @@ int scmi_xfer_raw_inflight_register(const struct scmi_handle *handle,
>  int scmi_xfer_raw_wait_for_message_response(struct scmi_chan_info *cinfo,
>  					    struct scmi_xfer *xfer,
>  					    unsigned int timeout_ms);
> -#ifdef CONFIG_ARM_SCMI_TRANSPORT_SMC
> -extern const struct scmi_desc scmi_smc_desc;
> -#endif
>  #ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
>  extern const struct scmi_desc scmi_virtio_desc;
>  #endif
> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> index 96cf8ab4421e..b14c5326930a 100644
> --- a/drivers/firmware/arm_scmi/driver.c
> +++ b/drivers/firmware/arm_scmi/driver.c
> @@ -3254,11 +3254,6 @@ static const struct of_device_id scmi_of_match[] = {
>  #ifdef CONFIG_ARM_SCMI_TRANSPORT_OPTEE
>  	{ .compatible = "linaro,scmi-optee", .data = &scmi_optee_desc },
>  #endif
> -#ifdef CONFIG_ARM_SCMI_TRANSPORT_SMC
> -	{ .compatible = "arm,scmi-smc", .data = &scmi_smc_desc},
> -	{ .compatible = "arm,scmi-smc-param", .data = &scmi_smc_desc},
> -	{ .compatible = "qcom,scmi-smc", .data = &scmi_smc_desc},
> -#endif
>  #ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
>  	{ .compatible = "arm,scmi-virtio", .data = &scmi_virtio_desc},
>  #endif
> diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/scmi_transport_smc.c
> similarity index 89%
> rename from drivers/firmware/arm_scmi/smc.c
> rename to drivers/firmware/arm_scmi/scmi_transport_smc.c
> index cb26b8aee01d..44da1a8d5387 100644
> --- a/drivers/firmware/arm_scmi/smc.c
> +++ b/drivers/firmware/arm_scmi/scmi_transport_smc.c
> @@ -3,7 +3,7 @@
>   * System Control and Management Interface (SCMI) Message SMC/HVC
>   * Transport driver
>   *
> - * Copyright 2020 NXP
> + * Copyright 2020-2024 NXP
>   */
>  
>  #include <linux/arm-smccc.h>
> @@ -16,6 +16,7 @@
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
>  #include <linux/limits.h>
> +#include <linux/platform_device.h>
>  #include <linux/processor.h>
>  #include <linux/slab.h>
>  
> @@ -69,12 +70,14 @@ struct scmi_smc {
>  	unsigned long cap_id;
>  };
>  
> +static struct scmi_transport_core_operations *core;
> +
>  static irqreturn_t smc_msg_done_isr(int irq, void *data)
>  {
>  	struct scmi_smc *scmi_info = data;
>  
> -	scmi_rx_callback(scmi_info->cinfo,
> -			 scmi_shmem_ops.read_header(scmi_info->shmem), NULL);
> +	core->rx_callback(scmi_info->cinfo,
> +			  core->shmem->read_header(scmi_info->shmem), NULL);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -142,7 +145,7 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
>  	if (!scmi_info)
>  		return -ENOMEM;
>  
> -	scmi_info->shmem = scmi_shmem_ops.setup_iomap(cinfo, dev, tx);
> +	scmi_info->shmem = core->shmem->setup_iomap(cinfo, dev, tx);
>  	if (IS_ERR(scmi_info->shmem))
>  		return PTR_ERR(scmi_info->shmem);
>  
> @@ -226,7 +229,7 @@ static int smc_send_message(struct scmi_chan_info *cinfo,
>  	 */
>  	smc_channel_lock_acquire(scmi_info, xfer);
>  
> -	scmi_shmem_ops.tx_prepare(scmi_info->shmem, xfer, cinfo);
> +	core->shmem->tx_prepare(scmi_info->shmem, xfer, cinfo);
>  
>  	if (scmi_info->cap_id != ULONG_MAX)
>  		arm_smccc_1_1_invoke(scmi_info->func_id, scmi_info->cap_id, 0,
> @@ -250,7 +253,7 @@ static void smc_fetch_response(struct scmi_chan_info *cinfo,
>  {
>  	struct scmi_smc *scmi_info = cinfo->transport_info;
>  
> -	scmi_shmem_ops.fetch_response(scmi_info->shmem, xfer);
> +	core->shmem->fetch_response(scmi_info->shmem, xfer);
>  }
>  
>  static void smc_mark_txdone(struct scmi_chan_info *cinfo, int ret,
> @@ -286,3 +289,19 @@ const struct scmi_desc scmi_smc_desc = {
>  	.sync_cmds_completed_on_ret = true,
>  	.atomic_enabled = IS_ENABLED(CONFIG_ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE),
>  };
> +
> +static const struct of_device_id scmi_of_match[] = {
> +	{ .compatible = "arm,scmi-smc" },
> +	{ .compatible = "arm,scmi-smc-param" },
> +	{ .compatible = "qcom,scmi-smc" },
> +	{ /* Sentinel */ },
> +};
> +

Hi Cristian,

Would it make sense to associate scmi descriptor(scmi_smc_desc) with
compatible as driver/platform data so we have flexibility to replicate
it and modify parameters such as max_timeout_ms etc. for our platform?

Thanks,

-Nikunj

> +DEFINE_SCMI_TRANSPORT_DRIVER(scmi_smc, scmi_of_match, &core);
> +module_platform_driver(scmi_smc_driver);
> +
> +MODULE_ALIAS("scmi-transport-smc");
> +MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
> +MODULE_AUTHOR("Nikunj Kela <quic_nkela@quicinc.com>");
> +MODULE_DESCRIPTION("SCMI SMC Transport driver");
> +MODULE_LICENSE("GPL");

Re: [PATCH 1/8] firmware: arm_scmi: Introduce setup_shmem_iomap

[kernel test robot]

Hi Cristian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on soc/for-next]
[also build test WARNING on next-20240703]
[cannot apply to linus/master v6.10-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Cristian-Marussi/firmware-arm_scmi-Introduce-setup_shmem_iomap/20240707-082513
base:   https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link:    https://lore.kernel.org/r/20240707002055.1835121-2-cristian.marussi%40arm.com
patch subject: [PATCH 1/8] firmware: arm_scmi: Introduce setup_shmem_iomap
config: arm-randconfig-r111-20240708 (https://download.01.org/0day-ci/archive/20240708/202407080937.AYEaDXCD-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project a0c6b8aef853eedaa0980f07c0a502a5a8a9740e)
reproduce: (https://download.01.org/0day-ci/archive/20240708/202407080937.AYEaDXCD-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407080937.AYEaDXCD-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/firmware/arm_scmi/shmem.c:170:16: sparse: sparse: incorrect type in return expression (different address spaces) @@     expected void * @@     got void [noderef] __iomem *[assigned] addr @@
   drivers/firmware/arm_scmi/shmem.c:170:16: sparse:     expected void *
   drivers/firmware/arm_scmi/shmem.c:170:16: sparse:     got void [noderef] __iomem *[assigned] addr
   drivers/firmware/arm_scmi/shmem.c:139:6: sparse: sparse: symbol 'setup_shmem_iomap' redeclared with different type (different address spaces):
   drivers/firmware/arm_scmi/shmem.c:139:6: sparse:    void *extern [addressable] [noderef] [toplevel] __iomem setup_shmem_iomap( ... )
   drivers/firmware/arm_scmi/shmem.c: note: in included file:
   drivers/firmware/arm_scmi/common.h:330:14: sparse: note: previously declared as:
   drivers/firmware/arm_scmi/common.h:330:14: sparse:    void [noderef] __iomem *extern [addressable] [toplevel] setup_shmem_iomap( ... )

vim +170 drivers/firmware/arm_scmi/shmem.c

   138	
   139	void *__iomem
   140	setup_shmem_iomap(struct scmi_chan_info *cinfo, struct device *dev, bool tx)
   141	{
   142		const char *desc = tx ? "Tx" : "Rx";
   143		int ret, idx = tx ? 0 : 1;
   144		struct device *cdev = cinfo->dev;
   145		struct device_node *shmem;
   146		resource_size_t size;
   147		struct resource res;
   148		void __iomem *addr;
   149	
   150		shmem = of_parse_phandle(cdev->of_node, "shmem", idx);
   151		if (!of_device_is_compatible(shmem, "arm,scmi-shmem")) {
   152			of_node_put(shmem);
   153			return ERR_PTR(-ENXIO);
   154		}
   155	
   156		ret = of_address_to_resource(shmem, 0, &res);
   157		of_node_put(shmem);
   158		if (ret) {
   159			dev_err(cdev, "failed to get SCMI %s shared memory\n", desc);
   160			return ERR_PTR(ret);
   161		}
   162	
   163		size = resource_size(&res);
   164		addr = devm_ioremap(dev, res.start, size);
   165		if (!addr) {
   166			dev_err(dev, "failed to ioremap SCMI %s shared memory\n", desc);
   167			return ERR_PTR(-EADDRNOTAVAIL);
   168		}
   169	
 > 170		return addr;

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/8] firmware: arm_scmi: Introduce packet handling helpers

[kernel test robot]

Hi Cristian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on soc/for-next]
[also build test WARNING on next-20240703]
[cannot apply to linus/master v6.10-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Cristian-Marussi/firmware-arm_scmi-Introduce-setup_shmem_iomap/20240707-082513
base:   https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link:    https://lore.kernel.org/r/20240707002055.1835121-3-cristian.marussi%40arm.com
patch subject: [PATCH 2/8] firmware: arm_scmi: Introduce packet handling helpers
config: arm-randconfig-r111-20240708 (https://download.01.org/0day-ci/archive/20240708/202407081158.9B8bXpH0-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project a0c6b8aef853eedaa0980f07c0a502a5a8a9740e)
reproduce: (https://download.01.org/0day-ci/archive/20240708/202407081158.9B8bXpH0-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407081158.9B8bXpH0-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
   drivers/firmware/arm_scmi/shmem.c:171:16: sparse: sparse: incorrect type in return expression (different address spaces) @@     expected void * @@     got void [noderef] __iomem *[assigned] addr @@
   drivers/firmware/arm_scmi/shmem.c:171:16: sparse:     expected void *
   drivers/firmware/arm_scmi/shmem.c:171:16: sparse:     got void [noderef] __iomem *[assigned] addr
>> drivers/firmware/arm_scmi/shmem.c:183:24: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected void [noderef] __iomem *( *setup_iomap )( ... ) @@     got void *( [noderef] __iomem * )( ... ) @@
   drivers/firmware/arm_scmi/shmem.c:183:24: sparse:     expected void [noderef] __iomem *( *setup_iomap )( ... )
   drivers/firmware/arm_scmi/shmem.c:183:24: sparse:     got void *( [noderef] __iomem * )( ... )
   drivers/firmware/arm_scmi/shmem.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/xarray.h, ...):
   include/linux/page-flags.h:240:46: sparse: sparse: self-comparison always evaluates to false
   include/linux/page-flags.h:240:46: sparse: sparse: self-comparison always evaluates to false

vim +183 drivers/firmware/arm_scmi/shmem.c

   173	
   174	const struct scmi_shared_mem_operations scmi_shmem_ops = {
   175		.tx_prepare = shmem_tx_prepare,
   176		.read_header = shmem_read_header,
   177		.fetch_response = shmem_fetch_response,
   178		.fetch_notification = shmem_fetch_notification,
   179		.clear_channel = shmem_clear_channel,
   180		.poll_done = shmem_poll_done,
   181		.channel_free = shmem_channel_free,
   182		.channel_intr_enabled = shmem_channel_intr_enabled,
 > 183		.setup_iomap = shmem_setup_iomap,

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 7/8] firmware: arm_scmi: Make VirtIO transport a standalone driver

[kernel test robot]

Hi Cristian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on soc/for-next]
[also build test WARNING on next-20240703]
[cannot apply to linus/master v6.10-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Cristian-Marussi/firmware-arm_scmi-Introduce-setup_shmem_iomap/20240707-082513
base:   https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link:    https://lore.kernel.org/r/20240707002055.1835121-8-cristian.marussi%40arm.com
patch subject: [PATCH 7/8] firmware: arm_scmi: Make VirtIO transport a standalone driver
config: arm-randconfig-r111-20240708 (https://download.01.org/0day-ci/archive/20240708/202407081344.9OceqFie-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project a0c6b8aef853eedaa0980f07c0a502a5a8a9740e)
reproduce: (https://download.01.org/0day-ci/archive/20240708/202407081344.9OceqFie-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407081344.9OceqFie-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/firmware/arm_scmi/scmi_transport_virtio.c:795:24: sparse: sparse: symbol 'scmi_virtio_desc' was not declared. Should it be static?
   drivers/firmware/arm_scmi/scmi_transport_virtio.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/xarray.h, ...):
   include/linux/page-flags.h:240:46: sparse: sparse: self-comparison always evaluates to false
   include/linux/page-flags.h:240:46: sparse: sparse: self-comparison always evaluates to false

vim +/scmi_virtio_desc +795 drivers/firmware/arm_scmi/scmi_transport_virtio.c

   794	
 > 795	const struct scmi_desc scmi_virtio_desc = {
   796		.ops = &scmi_virtio_ops,
   797		/* for non-realtime virtio devices */
   798		.max_rx_timeout_ms = VIRTIO_MAX_RX_TIMEOUT_MS,
   799		.max_msg = 0, /* overridden by virtio_get_max_msg() */
   800		.max_msg_size = VIRTIO_SCMI_MAX_MSG_SIZE,
   801		.atomic_enabled = IS_ENABLED(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO_ATOMIC_ENABLE),
   802	};
   803	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/8] firmware: arm_scmi: Introduce setup_shmem_iomap

[Cristian Marussi]

On Sun, Jul 07, 2024 at 09:48:58AM -0700, Nikunj Kela wrote:
> 
> On 7/6/2024 5:20 PM, Cristian Marussi wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > To get the address of shmem could be generalized by introducing
> > setup_shmem_iomap. Then the duplicated code in mailbox.c, optee.c
> > and smc.c could be dropped.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > [ Cristian: make use of the new helper also in smc.c ]
> > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> > ---
> >  drivers/firmware/arm_scmi/common.h  |  2 ++
> >  drivers/firmware/arm_scmi/mailbox.c | 27 ++++------------------
> >  drivers/firmware/arm_scmi/optee.c   | 35 ++++------------------------
> >  drivers/firmware/arm_scmi/shmem.c   | 36 +++++++++++++++++++++++++++++
> >  drivers/firmware/arm_scmi/smc.c     | 23 +++---------------
> >  5 files changed, 49 insertions(+), 74 deletions(-)
> >
> > diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
> > index 4b8c5250cdb5..b4c217641e3a 100644
> > --- a/drivers/firmware/arm_scmi/common.h
> > +++ b/drivers/firmware/arm_scmi/common.h
> > @@ -327,6 +327,8 @@ bool shmem_poll_done(struct scmi_shared_mem __iomem *shmem,
> >  		     struct scmi_xfer *xfer);
> >  bool shmem_channel_free(struct scmi_shared_mem __iomem *shmem);
> >  bool shmem_channel_intr_enabled(struct scmi_shared_mem __iomem *shmem);
> > +void __iomem *setup_shmem_iomap(struct scmi_chan_info *cinfo, struct device *dev,
> > +				bool tx);
> >  
> >  /* declarations for message passing transports */
> >  struct scmi_msg_payld;
> > diff --git a/drivers/firmware/arm_scmi/mailbox.c b/drivers/firmware/arm_scmi/mailbox.c
> > index 0219a12e3209..b0a579f31905 100644
> > --- a/drivers/firmware/arm_scmi/mailbox.c
> > +++ b/drivers/firmware/arm_scmi/mailbox.c
> > @@ -178,11 +178,8 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
> >  	const char *desc = tx ? "Tx" : "Rx";
> >  	struct device *cdev = cinfo->dev;
> >  	struct scmi_mailbox *smbox;
> > -	struct device_node *shmem;
> > -	int ret, a2p_rx_chan, p2a_chan, p2a_rx_chan, idx = tx ? 0 : 1;
> > +	int ret, a2p_rx_chan, p2a_chan, p2a_rx_chan;
> >  	struct mbox_client *cl;
> > -	resource_size_t size;
> > -	struct resource res;
> >  
> >  	ret = mailbox_chan_validate(cdev, &a2p_rx_chan, &p2a_chan, &p2a_rx_chan);
> >  	if (ret)
> > @@ -195,25 +192,9 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
> >  	if (!smbox)
> >  		return -ENOMEM;
> >  
> > -	shmem = of_parse_phandle(cdev->of_node, "shmem", idx);
> > -	if (!of_device_is_compatible(shmem, "arm,scmi-shmem")) {
> > -		of_node_put(shmem);
> > -		return -ENXIO;
> > -	}
> > -
> > -	ret = of_address_to_resource(shmem, 0, &res);
> > -	of_node_put(shmem);
> > -	if (ret) {
> > -		dev_err(cdev, "failed to get SCMI %s shared memory\n", desc);
> > -		return ret;
> > -	}
> > -
> > -	size = resource_size(&res);
> > -	smbox->shmem = devm_ioremap(dev, res.start, size);
> > -	if (!smbox->shmem) {
> > -		dev_err(dev, "failed to ioremap SCMI %s shared memory\n", desc);
> > -		return -EADDRNOTAVAIL;
> > -	}
> > +	smbox->shmem = setup_shmem_iomap(cinfo, dev, tx);
> > +	if (IS_ERR(smbox->shmem))
> > +		return PTR_ERR(smbox->shmem);
> >  
> >  	cl = &smbox->cl;
> >  	cl->dev = cdev;
> > diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c
> > index 4e7944b91e38..8abcd668108c 100644
> > --- a/drivers/firmware/arm_scmi/optee.c
> > +++ b/drivers/firmware/arm_scmi/optee.c
> > @@ -368,38 +368,11 @@ static int setup_dynamic_shmem(struct device *dev, struct scmi_optee_channel *ch
> >  static int setup_static_shmem(struct device *dev, struct scmi_chan_info *cinfo,
> >  			      struct scmi_optee_channel *channel)
> >  {
> > -	struct device_node *np;
> > -	resource_size_t size;
> > -	struct resource res;
> > -	int ret;
> > -
> > -	np = of_parse_phandle(cinfo->dev->of_node, "shmem", 0);
> > -	if (!of_device_is_compatible(np, "arm,scmi-shmem")) {
> > -		ret = -ENXIO;
> > -		goto out;
> > -	}
> > -
> > -	ret = of_address_to_resource(np, 0, &res);
> > -	if (ret) {
> > -		dev_err(dev, "Failed to get SCMI Tx shared memory\n");
> > -		goto out;
> > -	}
> > -
> > -	size = resource_size(&res);
> > +	channel->req.shmem = setup_shmem_iomap(cinfo, dev, true);
> > +	if (IS_ERR(channel->req.shmem))
> > +		return PTR_ERR(channel->req.shmem);
> >  
> > -	channel->req.shmem = devm_ioremap(dev, res.start, size);
> > -	if (!channel->req.shmem) {
> > -		dev_err(dev, "Failed to ioremap SCMI Tx shared memory\n");
> > -		ret = -EADDRNOTAVAIL;
> > -		goto out;
> > -	}
> > -
> > -	ret = 0;
> > -
> > -out:
> > -	of_node_put(np);
> > -
> > -	return ret;
> > +	return 0;
> >  }
> >  
> >  static int setup_shmem(struct device *dev, struct scmi_chan_info *cinfo,
> > diff --git a/drivers/firmware/arm_scmi/shmem.c b/drivers/firmware/arm_scmi/shmem.c
> > index b74e5a740f2c..c31f188d74ef 100644
> > --- a/drivers/firmware/arm_scmi/shmem.c
> > +++ b/drivers/firmware/arm_scmi/shmem.c
> > @@ -7,6 +7,8 @@
> >  
> >  #include <linux/ktime.h>
> >  #include <linux/io.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> >  #include <linux/processor.h>
> >  #include <linux/types.h>
> >  
> > @@ -133,3 +135,37 @@ bool shmem_channel_intr_enabled(struct scmi_shared_mem __iomem *shmem)
> >  {
> >  	return ioread32(&shmem->flags) & SCMI_SHMEM_FLAG_INTR_ENABLED;
> >  }
> > +
> > +void *__iomem
> > +setup_shmem_iomap(struct scmi_chan_info *cinfo, struct device *dev, bool tx)
> > +{
> > +	const char *desc = tx ? "Tx" : "Rx";
> > +	int ret, idx = tx ? 0 : 1;
> > +	struct device *cdev = cinfo->dev;
> > +	struct device_node *shmem;
> > +	resource_size_t size;
> > +	struct resource res;
> > +	void __iomem *addr;
> > +
> > +	shmem = of_parse_phandle(cdev->of_node, "shmem", idx);
> > +	if (!of_device_is_compatible(shmem, "arm,scmi-shmem")) {
> > +		of_node_put(shmem);
> > +		return ERR_PTR(-ENXIO);
> > +	}
> > +
> > +	ret = of_address_to_resource(shmem, 0, &res);
> > +	of_node_put(shmem);
> > +	if (ret) {
> > +		dev_err(cdev, "failed to get SCMI %s shared memory\n", desc);
> > +		return ERR_PTR(ret);
> > +	}
> > +
> > +	size = resource_size(&res);
> > +	addr = devm_ioremap(dev, res.start, size);
> > +	if (!addr) {
> > +		dev_err(dev, "failed to ioremap SCMI %s shared memory\n", desc);
> > +		return ERR_PTR(-EADDRNOTAVAIL);
> > +	}
> > +
> > +	return addr;
> > +}
> > diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c
> > index 39936e1dd30e..a640a4312472 100644
> > --- a/drivers/firmware/arm_scmi/smc.c
> > +++ b/drivers/firmware/arm_scmi/smc.c
> > @@ -132,7 +132,6 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
> >  	struct scmi_smc *scmi_info;
> >  	resource_size_t size;
> >  	struct resource res;
> > -	struct device_node *np;
> >  	u32 func_id;
> >  	int ret;
> >  
> > @@ -143,25 +142,9 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
> >  	if (!scmi_info)
> >  		return -ENOMEM;
> >  
> > -	np = of_parse_phandle(cdev->of_node, "shmem", 0);
> > -	if (!of_device_is_compatible(np, "arm,scmi-shmem")) {
> > -		of_node_put(np);
> > -		return -ENXIO;
> > -	}
> > -
> > -	ret = of_address_to_resource(np, 0, &res);
> > -	of_node_put(np);
> > -	if (ret) {
> > -		dev_err(cdev, "failed to get SCMI Tx shared memory\n");
> > -		return ret;
> > -	}
> > -
> > -	size = resource_size(&res);
> 
> Hi Peng/Cristian,
> 
> This will break Qualcomm smc transport as we need shmem 'size' to get to
> the cap-id. 
> 

...ops..sorry....that's on me I quickly extended Peng patch to address also SMC
and missed this...I'l review...

Thanks for havig a look !
Cristian

Re: [PATCH 5/8] firmware: arm_scmi: Make SMC transport a standalone driver

[Cristian Marussi]

On Sun, Jul 07, 2024 at 09:52:49AM -0700, Nikunj Kela wrote:
> 
> On 7/6/2024 5:20 PM, Cristian Marussi wrote:
> > Make SCMI SMC transport a standalone driver that can be optionally
> > loaded as a module.
> >
> > CC: Peng Fan <peng.fan@nxp.com>
> > CC: Nikunj Kela <quic_nkela@quicinc.com>
> > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> > ---
> >  drivers/firmware/arm_scmi/Kconfig             |  4 ++-
> >  drivers/firmware/arm_scmi/Makefile            |  2 +-
> >  drivers/firmware/arm_scmi/common.h            |  3 --
> >  drivers/firmware/arm_scmi/driver.c            |  5 ---
> >  .../arm_scmi/{smc.c => scmi_transport_smc.c}  | 31 +++++++++++++++----
> >  5 files changed, 29 insertions(+), 16 deletions(-)
> >  rename drivers/firmware/arm_scmi/{smc.c => scmi_transport_smc.c} (89%)
> >
> > diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
> > index 135e34aefd70..a4d44ef8bf45 100644
> > --- a/drivers/firmware/arm_scmi/Kconfig
> > +++ b/drivers/firmware/arm_scmi/Kconfig
> > @@ -102,7 +102,7 @@ config ARM_SCMI_TRANSPORT_OPTEE
> >  	  transport based on OP-TEE SCMI service, answer Y.
> >  
> >  config ARM_SCMI_TRANSPORT_SMC
> > -	bool "SCMI transport based on SMC"
> > +	tristate "SCMI transport based on SMC"
> >  	depends on HAVE_ARM_SMCCC_DISCOVERY
> >  	select ARM_SCMI_HAVE_TRANSPORT
> >  	select ARM_SCMI_HAVE_SHMEM
> > @@ -112,6 +112,8 @@ config ARM_SCMI_TRANSPORT_SMC
> >  
> >  	  If you want the ARM SCMI PROTOCOL stack to include support for a
> >  	  transport based on SMC, answer Y.
> > +	  This driver can also be built as a module.  If so, the module
> > +	  will be called scmi_transport_smc.
> >  
> >  config ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE
> >  	bool "Enable atomic mode support for SCMI SMC transport"
> > diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
> > index 121612d75f0b..6868a47fa4ab 100644
> > --- a/drivers/firmware/arm_scmi/Makefile
> > +++ b/drivers/firmware/arm_scmi/Makefile
> > @@ -5,7 +5,6 @@ scmi-core-objs := $(scmi-bus-y)
> >  scmi-driver-y = driver.o notify.o
> >  scmi-driver-$(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT) += raw_mode.o
> >  scmi-transport-$(CONFIG_ARM_SCMI_HAVE_SHMEM) = shmem.o
> > -scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += smc.o
> >  scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
> >  scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
> >  scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
> > @@ -13,6 +12,7 @@ scmi-protocols-y := base.o clock.o perf.o power.o reset.o sensors.o system.o vol
> >  scmi-protocols-y += pinctrl.o
> >  scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
> >  
> > +obj-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += scmi_transport_smc.o
> >  obj-$(CONFIG_ARM_SCMI_TRANSPORT_MAILBOX) += scmi_transport_mailbox.o
> >  
> >  obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
> > diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
> > index c03f30db92e0..b5bd27eccf24 100644
> > --- a/drivers/firmware/arm_scmi/common.h
> > +++ b/drivers/firmware/arm_scmi/common.h
> > @@ -286,9 +286,6 @@ int scmi_xfer_raw_inflight_register(const struct scmi_handle *handle,
> >  int scmi_xfer_raw_wait_for_message_response(struct scmi_chan_info *cinfo,
> >  					    struct scmi_xfer *xfer,
> >  					    unsigned int timeout_ms);
> > -#ifdef CONFIG_ARM_SCMI_TRANSPORT_SMC
> > -extern const struct scmi_desc scmi_smc_desc;
> > -#endif
> >  #ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
> >  extern const struct scmi_desc scmi_virtio_desc;
> >  #endif
> > diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> > index 96cf8ab4421e..b14c5326930a 100644
> > --- a/drivers/firmware/arm_scmi/driver.c
> > +++ b/drivers/firmware/arm_scmi/driver.c
> > @@ -3254,11 +3254,6 @@ static const struct of_device_id scmi_of_match[] = {
> >  #ifdef CONFIG_ARM_SCMI_TRANSPORT_OPTEE
> >  	{ .compatible = "linaro,scmi-optee", .data = &scmi_optee_desc },
> >  #endif
> > -#ifdef CONFIG_ARM_SCMI_TRANSPORT_SMC
> > -	{ .compatible = "arm,scmi-smc", .data = &scmi_smc_desc},
> > -	{ .compatible = "arm,scmi-smc-param", .data = &scmi_smc_desc},
> > -	{ .compatible = "qcom,scmi-smc", .data = &scmi_smc_desc},
> > -#endif
> >  #ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
> >  	{ .compatible = "arm,scmi-virtio", .data = &scmi_virtio_desc},
> >  #endif
> > diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/scmi_transport_smc.c
> > similarity index 89%
> > rename from drivers/firmware/arm_scmi/smc.c
> > rename to drivers/firmware/arm_scmi/scmi_transport_smc.c
> > index cb26b8aee01d..44da1a8d5387 100644
> > --- a/drivers/firmware/arm_scmi/smc.c
> > +++ b/drivers/firmware/arm_scmi/scmi_transport_smc.c
> > @@ -3,7 +3,7 @@
> >   * System Control and Management Interface (SCMI) Message SMC/HVC
> >   * Transport driver
> >   *
> > - * Copyright 2020 NXP
> > + * Copyright 2020-2024 NXP
> >   */
> >  
> >  #include <linux/arm-smccc.h>
> > @@ -16,6 +16,7 @@
> >  #include <linux/of_address.h>
> >  #include <linux/of_irq.h>
> >  #include <linux/limits.h>
> > +#include <linux/platform_device.h>
> >  #include <linux/processor.h>
> >  #include <linux/slab.h>
> >  
> > @@ -69,12 +70,14 @@ struct scmi_smc {
> >  	unsigned long cap_id;
> >  };
> >  
> > +static struct scmi_transport_core_operations *core;
> > +
> >  static irqreturn_t smc_msg_done_isr(int irq, void *data)
> >  {
> >  	struct scmi_smc *scmi_info = data;
> >  
> > -	scmi_rx_callback(scmi_info->cinfo,
> > -			 scmi_shmem_ops.read_header(scmi_info->shmem), NULL);
> > +	core->rx_callback(scmi_info->cinfo,
> > +			  core->shmem->read_header(scmi_info->shmem), NULL);
> >  
> >  	return IRQ_HANDLED;
> >  }
> > @@ -142,7 +145,7 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
> >  	if (!scmi_info)
> >  		return -ENOMEM;
> >  
> > -	scmi_info->shmem = scmi_shmem_ops.setup_iomap(cinfo, dev, tx);
> > +	scmi_info->shmem = core->shmem->setup_iomap(cinfo, dev, tx);
> >  	if (IS_ERR(scmi_info->shmem))
> >  		return PTR_ERR(scmi_info->shmem);
> >  
> > @@ -226,7 +229,7 @@ static int smc_send_message(struct scmi_chan_info *cinfo,
> >  	 */
> >  	smc_channel_lock_acquire(scmi_info, xfer);
> >  
> > -	scmi_shmem_ops.tx_prepare(scmi_info->shmem, xfer, cinfo);
> > +	core->shmem->tx_prepare(scmi_info->shmem, xfer, cinfo);
> >  
> >  	if (scmi_info->cap_id != ULONG_MAX)
> >  		arm_smccc_1_1_invoke(scmi_info->func_id, scmi_info->cap_id, 0,
> > @@ -250,7 +253,7 @@ static void smc_fetch_response(struct scmi_chan_info *cinfo,
> >  {
> >  	struct scmi_smc *scmi_info = cinfo->transport_info;
> >  
> > -	scmi_shmem_ops.fetch_response(scmi_info->shmem, xfer);
> > +	core->shmem->fetch_response(scmi_info->shmem, xfer);
> >  }
> >  
> >  static void smc_mark_txdone(struct scmi_chan_info *cinfo, int ret,
> > @@ -286,3 +289,19 @@ const struct scmi_desc scmi_smc_desc = {
> >  	.sync_cmds_completed_on_ret = true,
> >  	.atomic_enabled = IS_ENABLED(CONFIG_ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE),
> >  };
> > +
> > +static const struct of_device_id scmi_of_match[] = {
> > +	{ .compatible = "arm,scmi-smc" },
> > +	{ .compatible = "arm,scmi-smc-param" },
> > +	{ .compatible = "qcom,scmi-smc" },
> > +	{ /* Sentinel */ },
> > +};
> > +
> 
> Hi Cristian,
> 

Hi Nikunj,

thanks for having a look first of all !

> Would it make sense to associate scmi descriptor(scmi_smc_desc) with
> compatible as driver/platform data so we have flexibility to replicate
> it and modify parameters such as max_timeout_ms etc. for our platform?
> 

Mmmm...not sure to have understood, because the scmi_smc_desc is
effecetively passed from this driver to the core via a bit of
(questionable) magic in the mega-macro

DEFINE_SCMI_TRANSPORT_DRIVER(scmi_smc, scmi_of_match, &core);

...and it will end-up being set into the dev.platform_data and then
retrieved by the core SCMI stack driver in scmi_probe...

...OR...do you mean being able to somehow define 3 different
scmi_smc_desc* and then associate them to the different compatibles
and then, depending on which compatible is matched by this isame driver
at probe time, passing the related platform-specific desc to the core...

...in this latter case I suppose I can do it by playing with the macros
defs but maybe it is also the case to start thinking about splitting out
configuration stuff from the transport descriptor...

I'll give it a go at passing the data around, and see how it plays out
if you confirm that this is what you meant...

Thanks,
Cristian

Re: [PATCH 5/8] firmware: arm_scmi: Make SMC transport a standalone driver

[Nikunj Kela]

On 7/8/2024 7:27 AM, Cristian Marussi wrote:
> On Sun, Jul 07, 2024 at 09:52:49AM -0700, Nikunj Kela wrote:
>> On 7/6/2024 5:20 PM, Cristian Marussi wrote:
>>> Make SCMI SMC transport a standalone driver that can be optionally
>>> loaded as a module.
>>>
>>> CC: Peng Fan <peng.fan@nxp.com>
>>> CC: Nikunj Kela <quic_nkela@quicinc.com>
>>> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
>>> ---
>>>  drivers/firmware/arm_scmi/Kconfig             |  4 ++-
>>>  drivers/firmware/arm_scmi/Makefile            |  2 +-
>>>  drivers/firmware/arm_scmi/common.h            |  3 --
>>>  drivers/firmware/arm_scmi/driver.c            |  5 ---
>>>  .../arm_scmi/{smc.c => scmi_transport_smc.c}  | 31 +++++++++++++++----
>>>  5 files changed, 29 insertions(+), 16 deletions(-)
>>>  rename drivers/firmware/arm_scmi/{smc.c => scmi_transport_smc.c} (89%)
>>>
>>> diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
>>> index 135e34aefd70..a4d44ef8bf45 100644
>>> --- a/drivers/firmware/arm_scmi/Kconfig
>>> +++ b/drivers/firmware/arm_scmi/Kconfig
>>> @@ -102,7 +102,7 @@ config ARM_SCMI_TRANSPORT_OPTEE
>>>  	  transport based on OP-TEE SCMI service, answer Y.
>>>  
>>>  config ARM_SCMI_TRANSPORT_SMC
>>> -	bool "SCMI transport based on SMC"
>>> +	tristate "SCMI transport based on SMC"
>>>  	depends on HAVE_ARM_SMCCC_DISCOVERY
>>>  	select ARM_SCMI_HAVE_TRANSPORT
>>>  	select ARM_SCMI_HAVE_SHMEM
>>> @@ -112,6 +112,8 @@ config ARM_SCMI_TRANSPORT_SMC
>>>  
>>>  	  If you want the ARM SCMI PROTOCOL stack to include support for a
>>>  	  transport based on SMC, answer Y.
>>> +	  This driver can also be built as a module.  If so, the module
>>> +	  will be called scmi_transport_smc.
>>>  
>>>  config ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE
>>>  	bool "Enable atomic mode support for SCMI SMC transport"
>>> diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
>>> index 121612d75f0b..6868a47fa4ab 100644
>>> --- a/drivers/firmware/arm_scmi/Makefile
>>> +++ b/drivers/firmware/arm_scmi/Makefile
>>> @@ -5,7 +5,6 @@ scmi-core-objs := $(scmi-bus-y)
>>>  scmi-driver-y = driver.o notify.o
>>>  scmi-driver-$(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT) += raw_mode.o
>>>  scmi-transport-$(CONFIG_ARM_SCMI_HAVE_SHMEM) = shmem.o
>>> -scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += smc.o
>>>  scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
>>>  scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
>>>  scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
>>> @@ -13,6 +12,7 @@ scmi-protocols-y := base.o clock.o perf.o power.o reset.o sensors.o system.o vol
>>>  scmi-protocols-y += pinctrl.o
>>>  scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
>>>  
>>> +obj-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += scmi_transport_smc.o
>>>  obj-$(CONFIG_ARM_SCMI_TRANSPORT_MAILBOX) += scmi_transport_mailbox.o
>>>  
>>>  obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
>>> diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
>>> index c03f30db92e0..b5bd27eccf24 100644
>>> --- a/drivers/firmware/arm_scmi/common.h
>>> +++ b/drivers/firmware/arm_scmi/common.h
>>> @@ -286,9 +286,6 @@ int scmi_xfer_raw_inflight_register(const struct scmi_handle *handle,
>>>  int scmi_xfer_raw_wait_for_message_response(struct scmi_chan_info *cinfo,
>>>  					    struct scmi_xfer *xfer,
>>>  					    unsigned int timeout_ms);
>>> -#ifdef CONFIG_ARM_SCMI_TRANSPORT_SMC
>>> -extern const struct scmi_desc scmi_smc_desc;
>>> -#endif
>>>  #ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
>>>  extern const struct scmi_desc scmi_virtio_desc;
>>>  #endif
>>> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
>>> index 96cf8ab4421e..b14c5326930a 100644
>>> --- a/drivers/firmware/arm_scmi/driver.c
>>> +++ b/drivers/firmware/arm_scmi/driver.c
>>> @@ -3254,11 +3254,6 @@ static const struct of_device_id scmi_of_match[] = {
>>>  #ifdef CONFIG_ARM_SCMI_TRANSPORT_OPTEE
>>>  	{ .compatible = "linaro,scmi-optee", .data = &scmi_optee_desc },
>>>  #endif
>>> -#ifdef CONFIG_ARM_SCMI_TRANSPORT_SMC
>>> -	{ .compatible = "arm,scmi-smc", .data = &scmi_smc_desc},
>>> -	{ .compatible = "arm,scmi-smc-param", .data = &scmi_smc_desc},
>>> -	{ .compatible = "qcom,scmi-smc", .data = &scmi_smc_desc},
>>> -#endif
>>>  #ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
>>>  	{ .compatible = "arm,scmi-virtio", .data = &scmi_virtio_desc},
>>>  #endif
>>> diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/scmi_transport_smc.c
>>> similarity index 89%
>>> rename from drivers/firmware/arm_scmi/smc.c
>>> rename to drivers/firmware/arm_scmi/scmi_transport_smc.c
>>> index cb26b8aee01d..44da1a8d5387 100644
>>> --- a/drivers/firmware/arm_scmi/smc.c
>>> +++ b/drivers/firmware/arm_scmi/scmi_transport_smc.c
>>> @@ -3,7 +3,7 @@
>>>   * System Control and Management Interface (SCMI) Message SMC/HVC
>>>   * Transport driver
>>>   *
>>> - * Copyright 2020 NXP
>>> + * Copyright 2020-2024 NXP
>>>   */
>>>  
>>>  #include <linux/arm-smccc.h>
>>> @@ -16,6 +16,7 @@
>>>  #include <linux/of_address.h>
>>>  #include <linux/of_irq.h>
>>>  #include <linux/limits.h>
>>> +#include <linux/platform_device.h>
>>>  #include <linux/processor.h>
>>>  #include <linux/slab.h>
>>>  
>>> @@ -69,12 +70,14 @@ struct scmi_smc {
>>>  	unsigned long cap_id;
>>>  };
>>>  
>>> +static struct scmi_transport_core_operations *core;
>>> +
>>>  static irqreturn_t smc_msg_done_isr(int irq, void *data)
>>>  {
>>>  	struct scmi_smc *scmi_info = data;
>>>  
>>> -	scmi_rx_callback(scmi_info->cinfo,
>>> -			 scmi_shmem_ops.read_header(scmi_info->shmem), NULL);
>>> +	core->rx_callback(scmi_info->cinfo,
>>> +			  core->shmem->read_header(scmi_info->shmem), NULL);
>>>  
>>>  	return IRQ_HANDLED;
>>>  }
>>> @@ -142,7 +145,7 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
>>>  	if (!scmi_info)
>>>  		return -ENOMEM;
>>>  
>>> -	scmi_info->shmem = scmi_shmem_ops.setup_iomap(cinfo, dev, tx);
>>> +	scmi_info->shmem = core->shmem->setup_iomap(cinfo, dev, tx);
>>>  	if (IS_ERR(scmi_info->shmem))
>>>  		return PTR_ERR(scmi_info->shmem);
>>>  
>>> @@ -226,7 +229,7 @@ static int smc_send_message(struct scmi_chan_info *cinfo,
>>>  	 */
>>>  	smc_channel_lock_acquire(scmi_info, xfer);
>>>  
>>> -	scmi_shmem_ops.tx_prepare(scmi_info->shmem, xfer, cinfo);
>>> +	core->shmem->tx_prepare(scmi_info->shmem, xfer, cinfo);
>>>  
>>>  	if (scmi_info->cap_id != ULONG_MAX)
>>>  		arm_smccc_1_1_invoke(scmi_info->func_id, scmi_info->cap_id, 0,
>>> @@ -250,7 +253,7 @@ static void smc_fetch_response(struct scmi_chan_info *cinfo,
>>>  {
>>>  	struct scmi_smc *scmi_info = cinfo->transport_info;
>>>  
>>> -	scmi_shmem_ops.fetch_response(scmi_info->shmem, xfer);
>>> +	core->shmem->fetch_response(scmi_info->shmem, xfer);
>>>  }
>>>  
>>>  static void smc_mark_txdone(struct scmi_chan_info *cinfo, int ret,
>>> @@ -286,3 +289,19 @@ const struct scmi_desc scmi_smc_desc = {
>>>  	.sync_cmds_completed_on_ret = true,
>>>  	.atomic_enabled = IS_ENABLED(CONFIG_ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE),
>>>  };
>>> +
>>> +static const struct of_device_id scmi_of_match[] = {
>>> +	{ .compatible = "arm,scmi-smc" },
>>> +	{ .compatible = "arm,scmi-smc-param" },
>>> +	{ .compatible = "qcom,scmi-smc" },
>>> +	{ /* Sentinel */ },
>>> +};
>>> +
>> Hi Cristian,
>>
> Hi Nikunj,
>
> thanks for having a look first of all !
>
>> Would it make sense to associate scmi descriptor(scmi_smc_desc) with
>> compatible as driver/platform data so we have flexibility to replicate
>> it and modify parameters such as max_timeout_ms etc. for our platform?
>>
> Mmmm...not sure to have understood, because the scmi_smc_desc is
> effecetively passed from this driver to the core via a bit of
> (questionable) magic in the mega-macro
>
> DEFINE_SCMI_TRANSPORT_DRIVER(scmi_smc, scmi_of_match, &core);
>
> ...and it will end-up being set into the dev.platform_data and then
> retrieved by the core SCMI stack driver in scmi_probe...
>
> ...OR...do you mean being able to somehow define 3 different
> scmi_smc_desc* and then associate them to the different compatibles
> and then, depending on which compatible is matched by this isame driver
> at probe time, passing the related platform-specific desc to the core...
>
> ...in this latter case I suppose I can do it by playing with the macros
> defs but maybe it is also the case to start thinking about splitting out
> configuration stuff from the transport descriptor...
>
> I'll give it a go at passing the data around, and see how it plays out
> if you confirm that this is what you meant...

Hi Cristian,

I wanted to send a patch for review(with older driver code) that will
allow us to override transport parameters(e.g. max_timeout_ms,
max_msg_size etc.) on Qualcomm platform. There could be multiple
approaches- 1) add callbacks (similar to get_msg_size) in transport_ops
and override the default or 2) replicate the descriptors for different
compatible and change those values as needed. I was going with the
second option but then I saw your patch and thought of throwing this at
you ;) I don't want you to hold your patch series for this but if you
have a better way to achieve or a preferred way between the two
mentioned before, please let me know. If you do want to add this feature
in this patch series, that would be great!

Thanks,

-Nikunj

>
> Thanks,
> Cristian

Re: [PATCH 5/8] firmware: arm_scmi: Make SMC transport a standalone driver

[Cristian Marussi]

On Mon, Jul 08, 2024 at 08:23:56AM -0700, Nikunj Kela wrote:
> 
> On 7/8/2024 7:27 AM, Cristian Marussi wrote:
> > On Sun, Jul 07, 2024 at 09:52:49AM -0700, Nikunj Kela wrote:
> >> On 7/6/2024 5:20 PM, Cristian Marussi wrote:
> >>> Make SCMI SMC transport a standalone driver that can be optionally
> >>> loaded as a module.
> >>>
> >>> CC: Peng Fan <peng.fan@nxp.com>
> >>> CC: Nikunj Kela <quic_nkela@quicinc.com>
> >>> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> >>> ---
> >>>  drivers/firmware/arm_scmi/Kconfig             |  4 ++-
> >>>  drivers/firmware/arm_scmi/Makefile            |  2 +-
> >>>  drivers/firmware/arm_scmi/common.h            |  3 --
> >>>  drivers/firmware/arm_scmi/driver.c            |  5 ---
> >>>  .../arm_scmi/{smc.c => scmi_transport_smc.c}  | 31 +++++++++++++++----
> >>>  5 files changed, 29 insertions(+), 16 deletions(-)
> >>>  rename drivers/firmware/arm_scmi/{smc.c => scmi_transport_smc.c} (89%)
> >>>
> >>> diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
> >>> index 135e34aefd70..a4d44ef8bf45 100644
> >>> --- a/drivers/firmware/arm_scmi/Kconfig
> >>> +++ b/drivers/firmware/arm_scmi/Kconfig
> >>> @@ -102,7 +102,7 @@ config ARM_SCMI_TRANSPORT_OPTEE
> >>>  	  transport based on OP-TEE SCMI service, answer Y.
> >>>  
> >>>  config ARM_SCMI_TRANSPORT_SMC
> >>> -	bool "SCMI transport based on SMC"
> >>> +	tristate "SCMI transport based on SMC"
> >>>  	depends on HAVE_ARM_SMCCC_DISCOVERY
> >>>  	select ARM_SCMI_HAVE_TRANSPORT
> >>>  	select ARM_SCMI_HAVE_SHMEM
> >>> @@ -112,6 +112,8 @@ config ARM_SCMI_TRANSPORT_SMC
> >>>  
> >>>  	  If you want the ARM SCMI PROTOCOL stack to include support for a
> >>>  	  transport based on SMC, answer Y.
> >>> +	  This driver can also be built as a module.  If so, the module
> >>> +	  will be called scmi_transport_smc.
> >>>  
> >>>  config ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE
> >>>  	bool "Enable atomic mode support for SCMI SMC transport"
> >>> diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
> >>> index 121612d75f0b..6868a47fa4ab 100644
> >>> --- a/drivers/firmware/arm_scmi/Makefile
> >>> +++ b/drivers/firmware/arm_scmi/Makefile
> >>> @@ -5,7 +5,6 @@ scmi-core-objs := $(scmi-bus-y)
> >>>  scmi-driver-y = driver.o notify.o
> >>>  scmi-driver-$(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT) += raw_mode.o
> >>>  scmi-transport-$(CONFIG_ARM_SCMI_HAVE_SHMEM) = shmem.o
> >>> -scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += smc.o
> >>>  scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
> >>>  scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
> >>>  scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
> >>> @@ -13,6 +12,7 @@ scmi-protocols-y := base.o clock.o perf.o power.o reset.o sensors.o system.o vol
> >>>  scmi-protocols-y += pinctrl.o
> >>>  scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
> >>>  
> >>> +obj-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += scmi_transport_smc.o
> >>>  obj-$(CONFIG_ARM_SCMI_TRANSPORT_MAILBOX) += scmi_transport_mailbox.o
> >>>  
> >>>  obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
> >>> diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
> >>> index c03f30db92e0..b5bd27eccf24 100644
> >>> --- a/drivers/firmware/arm_scmi/common.h
> >>> +++ b/drivers/firmware/arm_scmi/common.h
> >>> @@ -286,9 +286,6 @@ int scmi_xfer_raw_inflight_register(const struct scmi_handle *handle,
> >>>  int scmi_xfer_raw_wait_for_message_response(struct scmi_chan_info *cinfo,
> >>>  					    struct scmi_xfer *xfer,
> >>>  					    unsigned int timeout_ms);
> >>> -#ifdef CONFIG_ARM_SCMI_TRANSPORT_SMC
> >>> -extern const struct scmi_desc scmi_smc_desc;
> >>> -#endif
> >>>  #ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
> >>>  extern const struct scmi_desc scmi_virtio_desc;
> >>>  #endif
> >>> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> >>> index 96cf8ab4421e..b14c5326930a 100644
> >>> --- a/drivers/firmware/arm_scmi/driver.c
> >>> +++ b/drivers/firmware/arm_scmi/driver.c
> >>> @@ -3254,11 +3254,6 @@ static const struct of_device_id scmi_of_match[] = {
> >>>  #ifdef CONFIG_ARM_SCMI_TRANSPORT_OPTEE
> >>>  	{ .compatible = "linaro,scmi-optee", .data = &scmi_optee_desc },
> >>>  #endif
> >>> -#ifdef CONFIG_ARM_SCMI_TRANSPORT_SMC
> >>> -	{ .compatible = "arm,scmi-smc", .data = &scmi_smc_desc},
> >>> -	{ .compatible = "arm,scmi-smc-param", .data = &scmi_smc_desc},
> >>> -	{ .compatible = "qcom,scmi-smc", .data = &scmi_smc_desc},
> >>> -#endif
> >>>  #ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
> >>>  	{ .compatible = "arm,scmi-virtio", .data = &scmi_virtio_desc},
> >>>  #endif
> >>> diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/scmi_transport_smc.c
> >>> similarity index 89%
> >>> rename from drivers/firmware/arm_scmi/smc.c
> >>> rename to drivers/firmware/arm_scmi/scmi_transport_smc.c
> >>> index cb26b8aee01d..44da1a8d5387 100644
> >>> --- a/drivers/firmware/arm_scmi/smc.c
> >>> +++ b/drivers/firmware/arm_scmi/scmi_transport_smc.c
> >>> @@ -3,7 +3,7 @@
> >>>   * System Control and Management Interface (SCMI) Message SMC/HVC
> >>>   * Transport driver
> >>>   *
> >>> - * Copyright 2020 NXP
> >>> + * Copyright 2020-2024 NXP
> >>>   */
> >>>  
> >>>  #include <linux/arm-smccc.h>
> >>> @@ -16,6 +16,7 @@
> >>>  #include <linux/of_address.h>
> >>>  #include <linux/of_irq.h>
> >>>  #include <linux/limits.h>
> >>> +#include <linux/platform_device.h>
> >>>  #include <linux/processor.h>
> >>>  #include <linux/slab.h>
> >>>  
> >>> @@ -69,12 +70,14 @@ struct scmi_smc {
> >>>  	unsigned long cap_id;
> >>>  };
> >>>  
> >>> +static struct scmi_transport_core_operations *core;
> >>> +
> >>>  static irqreturn_t smc_msg_done_isr(int irq, void *data)
> >>>  {
> >>>  	struct scmi_smc *scmi_info = data;
> >>>  
> >>> -	scmi_rx_callback(scmi_info->cinfo,
> >>> -			 scmi_shmem_ops.read_header(scmi_info->shmem), NULL);
> >>> +	core->rx_callback(scmi_info->cinfo,
> >>> +			  core->shmem->read_header(scmi_info->shmem), NULL);
> >>>  
> >>>  	return IRQ_HANDLED;
> >>>  }
> >>> @@ -142,7 +145,7 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
> >>>  	if (!scmi_info)
> >>>  		return -ENOMEM;
> >>>  
> >>> -	scmi_info->shmem = scmi_shmem_ops.setup_iomap(cinfo, dev, tx);
> >>> +	scmi_info->shmem = core->shmem->setup_iomap(cinfo, dev, tx);
> >>>  	if (IS_ERR(scmi_info->shmem))
> >>>  		return PTR_ERR(scmi_info->shmem);
> >>>  
> >>> @@ -226,7 +229,7 @@ static int smc_send_message(struct scmi_chan_info *cinfo,
> >>>  	 */
> >>>  	smc_channel_lock_acquire(scmi_info, xfer);
> >>>  
> >>> -	scmi_shmem_ops.tx_prepare(scmi_info->shmem, xfer, cinfo);
> >>> +	core->shmem->tx_prepare(scmi_info->shmem, xfer, cinfo);
> >>>  
> >>>  	if (scmi_info->cap_id != ULONG_MAX)
> >>>  		arm_smccc_1_1_invoke(scmi_info->func_id, scmi_info->cap_id, 0,
> >>> @@ -250,7 +253,7 @@ static void smc_fetch_response(struct scmi_chan_info *cinfo,
> >>>  {
> >>>  	struct scmi_smc *scmi_info = cinfo->transport_info;
> >>>  
> >>> -	scmi_shmem_ops.fetch_response(scmi_info->shmem, xfer);
> >>> +	core->shmem->fetch_response(scmi_info->shmem, xfer);
> >>>  }
> >>>  
> >>>  static void smc_mark_txdone(struct scmi_chan_info *cinfo, int ret,
> >>> @@ -286,3 +289,19 @@ const struct scmi_desc scmi_smc_desc = {
> >>>  	.sync_cmds_completed_on_ret = true,
> >>>  	.atomic_enabled = IS_ENABLED(CONFIG_ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE),
> >>>  };
> >>> +
> >>> +static const struct of_device_id scmi_of_match[] = {
> >>> +	{ .compatible = "arm,scmi-smc" },
> >>> +	{ .compatible = "arm,scmi-smc-param" },
> >>> +	{ .compatible = "qcom,scmi-smc" },
> >>> +	{ /* Sentinel */ },
> >>> +};
> >>> +
> >> Hi Cristian,
> >>
> > Hi Nikunj,
> >
> > thanks for having a look first of all !
> >
> >> Would it make sense to associate scmi descriptor(scmi_smc_desc) with
> >> compatible as driver/platform data so we have flexibility to replicate
> >> it and modify parameters such as max_timeout_ms etc. for our platform?
> >>
> > Mmmm...not sure to have understood, because the scmi_smc_desc is
> > effecetively passed from this driver to the core via a bit of
> > (questionable) magic in the mega-macro
> >
> > DEFINE_SCMI_TRANSPORT_DRIVER(scmi_smc, scmi_of_match, &core);
> >
> > ...and it will end-up being set into the dev.platform_data and then
> > retrieved by the core SCMI stack driver in scmi_probe...
> >
> > ...OR...do you mean being able to somehow define 3 different
> > scmi_smc_desc* and then associate them to the different compatibles
> > and then, depending on which compatible is matched by this isame driver
> > at probe time, passing the related platform-specific desc to the core...
> >
> > ...in this latter case I suppose I can do it by playing with the macros
> > defs but maybe it is also the case to start thinking about splitting out
> > configuration stuff from the transport descriptor...
> >
> > I'll give it a go at passing the data around, and see how it plays out
> > if you confirm that this is what you meant...
> 
> Hi Cristian,
> 

Hi,

> I wanted to send a patch for review(with older driver code) that will
> allow us to override transport parameters(e.g. max_timeout_ms,
> max_msg_size etc.) on Qualcomm platform. There could be multiple
> approaches- 1) add callbacks (similar to get_msg_size) in transport_ops
> and override the default or 2) replicate the descriptors for different
> compatible and change those values as needed. I was going with the
> second option but then I saw your patch and thought of throwing this at

:P 

> you ;) I don't want you to hold your patch series for this but if you
> have a better way to achieve or a preferred way between the two
> mentioned before, please let me know. If you do want to add this feature
> in this patch series, that would be great!
> 

Interesting, because there is also this thread flying around from Peng:

https://lore.kernel.org/linux-arm-kernel/20240703031715.379815-1-peng.fan@oss.nxp.com/

...which I was planning in embedding in this series; Peng's proposal is
limited to timeout and based on DT (and title is a bit misleading...the
series is not mailbox-only related)....

...now I am NOT saying that we should dump all configs into the DT, but
just that this issue about configurability of transports is already sort
of a known-problem, and it is a while that it floats in the back of my mind...
i.e. the fact that the transport runtime configurations should be *somehow*
independent of the transport descriptor and *somehow* configurable....so now
only remains to *somehow* figure out how to do it :D

... I'll have a though and maybe cannibalize your ideas and Peng's one and then
we could have a discussion around this on the list to address eveybody's needs...

...and maybe, in the meantime, you could also post your proposed series about
this even though based on the old code, but as an RFC, just to make a point and
detail the needs...but yeah only if it does not require a bunch of extra work from
you given that it is only to be used as a basis for discussion ....

...up to you what do you prefer.

Thanks,
Cristian

Re: [PATCH 5/8] firmware: arm_scmi: Make SMC transport a standalone driver

[Nikunj Kela]

On 7/8/2024 8:47 AM, Cristian Marussi wrote:
> On Mon, Jul 08, 2024 at 08:23:56AM -0700, Nikunj Kela wrote:
>> On 7/8/2024 7:27 AM, Cristian Marussi wrote:
>>> On Sun, Jul 07, 2024 at 09:52:49AM -0700, Nikunj Kela wrote:
>>>> On 7/6/2024 5:20 PM, Cristian Marussi wrote:
>>>>> Make SCMI SMC transport a standalone driver that can be optionally
>>>>> loaded as a module.
>>>>>
>>>>> CC: Peng Fan <peng.fan@nxp.com>
>>>>> CC: Nikunj Kela <quic_nkela@quicinc.com>
>>>>> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
>>>>> ---
>>>>>  drivers/firmware/arm_scmi/Kconfig             |  4 ++-
>>>>>  drivers/firmware/arm_scmi/Makefile            |  2 +-
>>>>>  drivers/firmware/arm_scmi/common.h            |  3 --
>>>>>  drivers/firmware/arm_scmi/driver.c            |  5 ---
>>>>>  .../arm_scmi/{smc.c => scmi_transport_smc.c}  | 31 +++++++++++++++----
>>>>>  5 files changed, 29 insertions(+), 16 deletions(-)
>>>>>  rename drivers/firmware/arm_scmi/{smc.c => scmi_transport_smc.c} (89%)
>>>>>
>>>>> diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
>>>>> index 135e34aefd70..a4d44ef8bf45 100644
>>>>> --- a/drivers/firmware/arm_scmi/Kconfig
>>>>> +++ b/drivers/firmware/arm_scmi/Kconfig
>>>>> @@ -102,7 +102,7 @@ config ARM_SCMI_TRANSPORT_OPTEE
>>>>>  	  transport based on OP-TEE SCMI service, answer Y.
>>>>>  
>>>>>  config ARM_SCMI_TRANSPORT_SMC
>>>>> -	bool "SCMI transport based on SMC"
>>>>> +	tristate "SCMI transport based on SMC"
>>>>>  	depends on HAVE_ARM_SMCCC_DISCOVERY
>>>>>  	select ARM_SCMI_HAVE_TRANSPORT
>>>>>  	select ARM_SCMI_HAVE_SHMEM
>>>>> @@ -112,6 +112,8 @@ config ARM_SCMI_TRANSPORT_SMC
>>>>>  
>>>>>  	  If you want the ARM SCMI PROTOCOL stack to include support for a
>>>>>  	  transport based on SMC, answer Y.
>>>>> +	  This driver can also be built as a module.  If so, the module
>>>>> +	  will be called scmi_transport_smc.
>>>>>  
>>>>>  config ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE
>>>>>  	bool "Enable atomic mode support for SCMI SMC transport"
>>>>> diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
>>>>> index 121612d75f0b..6868a47fa4ab 100644
>>>>> --- a/drivers/firmware/arm_scmi/Makefile
>>>>> +++ b/drivers/firmware/arm_scmi/Makefile
>>>>> @@ -5,7 +5,6 @@ scmi-core-objs := $(scmi-bus-y)
>>>>>  scmi-driver-y = driver.o notify.o
>>>>>  scmi-driver-$(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT) += raw_mode.o
>>>>>  scmi-transport-$(CONFIG_ARM_SCMI_HAVE_SHMEM) = shmem.o
>>>>> -scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += smc.o
>>>>>  scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
>>>>>  scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
>>>>>  scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
>>>>> @@ -13,6 +12,7 @@ scmi-protocols-y := base.o clock.o perf.o power.o reset.o sensors.o system.o vol
>>>>>  scmi-protocols-y += pinctrl.o
>>>>>  scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
>>>>>  
>>>>> +obj-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += scmi_transport_smc.o
>>>>>  obj-$(CONFIG_ARM_SCMI_TRANSPORT_MAILBOX) += scmi_transport_mailbox.o
>>>>>  
>>>>>  obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
>>>>> diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
>>>>> index c03f30db92e0..b5bd27eccf24 100644
>>>>> --- a/drivers/firmware/arm_scmi/common.h
>>>>> +++ b/drivers/firmware/arm_scmi/common.h
>>>>> @@ -286,9 +286,6 @@ int scmi_xfer_raw_inflight_register(const struct scmi_handle *handle,
>>>>>  int scmi_xfer_raw_wait_for_message_response(struct scmi_chan_info *cinfo,
>>>>>  					    struct scmi_xfer *xfer,
>>>>>  					    unsigned int timeout_ms);
>>>>> -#ifdef CONFIG_ARM_SCMI_TRANSPORT_SMC
>>>>> -extern const struct scmi_desc scmi_smc_desc;
>>>>> -#endif
>>>>>  #ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
>>>>>  extern const struct scmi_desc scmi_virtio_desc;
>>>>>  #endif
>>>>> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
>>>>> index 96cf8ab4421e..b14c5326930a 100644
>>>>> --- a/drivers/firmware/arm_scmi/driver.c
>>>>> +++ b/drivers/firmware/arm_scmi/driver.c
>>>>> @@ -3254,11 +3254,6 @@ static const struct of_device_id scmi_of_match[] = {
>>>>>  #ifdef CONFIG_ARM_SCMI_TRANSPORT_OPTEE
>>>>>  	{ .compatible = "linaro,scmi-optee", .data = &scmi_optee_desc },
>>>>>  #endif
>>>>> -#ifdef CONFIG_ARM_SCMI_TRANSPORT_SMC
>>>>> -	{ .compatible = "arm,scmi-smc", .data = &scmi_smc_desc},
>>>>> -	{ .compatible = "arm,scmi-smc-param", .data = &scmi_smc_desc},
>>>>> -	{ .compatible = "qcom,scmi-smc", .data = &scmi_smc_desc},
>>>>> -#endif
>>>>>  #ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
>>>>>  	{ .compatible = "arm,scmi-virtio", .data = &scmi_virtio_desc},
>>>>>  #endif
>>>>> diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/scmi_transport_smc.c
>>>>> similarity index 89%
>>>>> rename from drivers/firmware/arm_scmi/smc.c
>>>>> rename to drivers/firmware/arm_scmi/scmi_transport_smc.c
>>>>> index cb26b8aee01d..44da1a8d5387 100644
>>>>> --- a/drivers/firmware/arm_scmi/smc.c
>>>>> +++ b/drivers/firmware/arm_scmi/scmi_transport_smc.c
>>>>> @@ -3,7 +3,7 @@
>>>>>   * System Control and Management Interface (SCMI) Message SMC/HVC
>>>>>   * Transport driver
>>>>>   *
>>>>> - * Copyright 2020 NXP
>>>>> + * Copyright 2020-2024 NXP
>>>>>   */
>>>>>  
>>>>>  #include <linux/arm-smccc.h>
>>>>> @@ -16,6 +16,7 @@
>>>>>  #include <linux/of_address.h>
>>>>>  #include <linux/of_irq.h>
>>>>>  #include <linux/limits.h>
>>>>> +#include <linux/platform_device.h>
>>>>>  #include <linux/processor.h>
>>>>>  #include <linux/slab.h>
>>>>>  
>>>>> @@ -69,12 +70,14 @@ struct scmi_smc {
>>>>>  	unsigned long cap_id;
>>>>>  };
>>>>>  
>>>>> +static struct scmi_transport_core_operations *core;
>>>>> +
>>>>>  static irqreturn_t smc_msg_done_isr(int irq, void *data)
>>>>>  {
>>>>>  	struct scmi_smc *scmi_info = data;
>>>>>  
>>>>> -	scmi_rx_callback(scmi_info->cinfo,
>>>>> -			 scmi_shmem_ops.read_header(scmi_info->shmem), NULL);
>>>>> +	core->rx_callback(scmi_info->cinfo,
>>>>> +			  core->shmem->read_header(scmi_info->shmem), NULL);
>>>>>  
>>>>>  	return IRQ_HANDLED;
>>>>>  }
>>>>> @@ -142,7 +145,7 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
>>>>>  	if (!scmi_info)
>>>>>  		return -ENOMEM;
>>>>>  
>>>>> -	scmi_info->shmem = scmi_shmem_ops.setup_iomap(cinfo, dev, tx);
>>>>> +	scmi_info->shmem = core->shmem->setup_iomap(cinfo, dev, tx);
>>>>>  	if (IS_ERR(scmi_info->shmem))
>>>>>  		return PTR_ERR(scmi_info->shmem);
>>>>>  
>>>>> @@ -226,7 +229,7 @@ static int smc_send_message(struct scmi_chan_info *cinfo,
>>>>>  	 */
>>>>>  	smc_channel_lock_acquire(scmi_info, xfer);
>>>>>  
>>>>> -	scmi_shmem_ops.tx_prepare(scmi_info->shmem, xfer, cinfo);
>>>>> +	core->shmem->tx_prepare(scmi_info->shmem, xfer, cinfo);
>>>>>  
>>>>>  	if (scmi_info->cap_id != ULONG_MAX)
>>>>>  		arm_smccc_1_1_invoke(scmi_info->func_id, scmi_info->cap_id, 0,
>>>>> @@ -250,7 +253,7 @@ static void smc_fetch_response(struct scmi_chan_info *cinfo,
>>>>>  {
>>>>>  	struct scmi_smc *scmi_info = cinfo->transport_info;
>>>>>  
>>>>> -	scmi_shmem_ops.fetch_response(scmi_info->shmem, xfer);
>>>>> +	core->shmem->fetch_response(scmi_info->shmem, xfer);
>>>>>  }
>>>>>  
>>>>>  static void smc_mark_txdone(struct scmi_chan_info *cinfo, int ret,
>>>>> @@ -286,3 +289,19 @@ const struct scmi_desc scmi_smc_desc = {
>>>>>  	.sync_cmds_completed_on_ret = true,
>>>>>  	.atomic_enabled = IS_ENABLED(CONFIG_ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE),
>>>>>  };
>>>>> +
>>>>> +static const struct of_device_id scmi_of_match[] = {
>>>>> +	{ .compatible = "arm,scmi-smc" },
>>>>> +	{ .compatible = "arm,scmi-smc-param" },
>>>>> +	{ .compatible = "qcom,scmi-smc" },
>>>>> +	{ /* Sentinel */ },
>>>>> +};
>>>>> +
>>>> Hi Cristian,
>>>>
>>> Hi Nikunj,
>>>
>>> thanks for having a look first of all !
>>>
>>>> Would it make sense to associate scmi descriptor(scmi_smc_desc) with
>>>> compatible as driver/platform data so we have flexibility to replicate
>>>> it and modify parameters such as max_timeout_ms etc. for our platform?
>>>>
>>> Mmmm...not sure to have understood, because the scmi_smc_desc is
>>> effecetively passed from this driver to the core via a bit of
>>> (questionable) magic in the mega-macro
>>>
>>> DEFINE_SCMI_TRANSPORT_DRIVER(scmi_smc, scmi_of_match, &core);
>>>
>>> ...and it will end-up being set into the dev.platform_data and then
>>> retrieved by the core SCMI stack driver in scmi_probe...
>>>
>>> ...OR...do you mean being able to somehow define 3 different
>>> scmi_smc_desc* and then associate them to the different compatibles
>>> and then, depending on which compatible is matched by this isame driver
>>> at probe time, passing the related platform-specific desc to the core...
>>>
>>> ...in this latter case I suppose I can do it by playing with the macros
>>> defs but maybe it is also the case to start thinking about splitting out
>>> configuration stuff from the transport descriptor...
>>>
>>> I'll give it a go at passing the data around, and see how it plays out
>>> if you confirm that this is what you meant...
>> Hi Cristian,
>>
> Hi,
>
>> I wanted to send a patch for review(with older driver code) that will
>> allow us to override transport parameters(e.g. max_timeout_ms,
>> max_msg_size etc.) on Qualcomm platform. There could be multiple
>> approaches- 1) add callbacks (similar to get_msg_size) in transport_ops
>> and override the default or 2) replicate the descriptors for different
>> compatible and change those values as needed. I was going with the
>> second option but then I saw your patch and thought of throwing this at
> :P 
>
>> you ;) I don't want you to hold your patch series for this but if you
>> have a better way to achieve or a preferred way between the two
>> mentioned before, please let me know. If you do want to add this feature
>> in this patch series, that would be great!
>>
> Interesting, because there is also this thread flying around from Peng:
>
> https://lore.kernel.org/linux-arm-kernel/20240703031715.379815-1-peng.fan@oss.nxp.com/
Thanks Cristian for the link. I was under the impression that DT binding
that don't describe HW are not acceptable to DT maintainer. But if this
patch goes through, I am fine using it. I would also like to have
max_msg_size(and maybe max_msg) configurable.
>
> ...which I was planning in embedding in this series; Peng's proposal is
> limited to timeout and based on DT (and title is a bit misleading...the
> series is not mailbox-only related)....
>
> ...now I am NOT saying that we should dump all configs into the DT, but
> just that this issue about configurability of transports is already sort
> of a known-problem, and it is a while that it floats in the back of my mind...
> i.e. the fact that the transport runtime configurations should be *somehow*
> independent of the transport descriptor and *somehow* configurable....so now
> only remains to *somehow* figure out how to do it :D
>
> ... I'll have a though and maybe cannibalize your ideas and Peng's one and then
> we could have a discussion around this on the list to address eveybody's needs...
>
> ...and maybe, in the meantime, you could also post your proposed series about
> this even though based on the old code, but as an RFC, just to make a point and
> detail the needs...but yeah only if it does not require a bunch of extra work from
> you given that it is only to be used as a basis for discussion ....
>
> ...up to you what do you prefer.
>
> Thanks,
> Cristian

Re: [PATCH 6/8] firmware: arm_scmi: Make OPTEE transport a standalone driver

[Dan Carpenter]

On Sun, Jul 07, 2024 at 01:20:53AM +0100, Cristian Marussi wrote:
>  static int scmi_optee_service_probe(struct device *dev)
>  {
>  	struct scmi_optee_agent *agent;
> @@ -555,7 +553,7 @@ static int scmi_optee_service_probe(struct device *dev)
>  	smp_mb();
>  	scmi_optee_private = agent;
>  
> -	return 0;
> +	return platform_driver_register(&scmi_optee_driver);

There needs to be some cleanup if platform_driver_register() fails.

>  
>  err:
>  	tee_client_close_context(tee_ctx);

regards,
dan carpenter

Re: [PATCH 6/8] firmware: arm_scmi: Make OPTEE transport a standalone driver

[Cristian Marussi]

On Tue, Jul 09, 2024 at 04:21:46AM +0200, Dan Carpenter wrote:
> On Sun, Jul 07, 2024 at 01:20:53AM +0100, Cristian Marussi wrote:
> >  static int scmi_optee_service_probe(struct device *dev)
> >  {
> >  	struct scmi_optee_agent *agent;
> > @@ -555,7 +553,7 @@ static int scmi_optee_service_probe(struct device *dev)
> >  	smp_mb();
> >  	scmi_optee_private = agent;
> >  
> > -	return 0;
> > +	return platform_driver_register(&scmi_optee_driver);

Hi Dan,

thanks for having a look !

> 
> There needs to be some cleanup if platform_driver_register() fails.
> 


Yes, indeed....also smatch/sparse screamed a lot in some other
places...I will rework in V2.

Thanks,
Cristian

Re: [PATCH 5/8] firmware: arm_scmi: Make SMC transport a standalone driver

[Cristian Marussi]

On Mon, Jul 08, 2024 at 10:59:33AM -0700, Nikunj Kela wrote:
> 
> On 7/8/2024 8:47 AM, Cristian Marussi wrote:
> > On Mon, Jul 08, 2024 at 08:23:56AM -0700, Nikunj Kela wrote:
> >> On 7/8/2024 7:27 AM, Cristian Marussi wrote:
> >>> On Sun, Jul 07, 2024 at 09:52:49AM -0700, Nikunj Kela wrote:
> >>>> On 7/6/2024 5:20 PM, Cristian Marussi wrote:
> >>>>> Make SCMI SMC transport a standalone driver that can be optionally
> >>>>> loaded as a module.
> >>>>>
> >>>>> CC: Peng Fan <peng.fan@nxp.com>
> >>>>> CC: Nikunj Kela <quic_nkela@quicinc.com>
> >>>>> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> >>>>> ---
> >>>>>  drivers/firmware/arm_scmi/Kconfig             |  4 ++-
> >>>>>  drivers/firmware/arm_scmi/Makefile            |  2 +-
> >>>>>  drivers/firmware/arm_scmi/common.h            |  3 --
> >>>>>  drivers/firmware/arm_scmi/driver.c            |  5 ---
> >>>>>  .../arm_scmi/{smc.c => scmi_transport_smc.c}  | 31 +++++++++++++++----
> >>>>>  5 files changed, 29 insertions(+), 16 deletions(-)
> >>>>>  rename drivers/firmware/arm_scmi/{smc.c => scmi_transport_smc.c} (89%)
> >>>>>
> >>>>> diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
> >>>>> index 135e34aefd70..a4d44ef8bf45 100644
> >>>>> --- a/drivers/firmware/arm_scmi/Kconfig
> >>>>> +++ b/drivers/firmware/arm_scmi/Kconfig
> >>>>> @@ -102,7 +102,7 @@ config ARM_SCMI_TRANSPORT_OPTEE
> >>>>>  	  transport based on OP-TEE SCMI service, answer Y.
> >>>>>  
> >>>>>  config ARM_SCMI_TRANSPORT_SMC
> >>>>> -	bool "SCMI transport based on SMC"
> >>>>> +	tristate "SCMI transport based on SMC"
> >>>>>  	depends on HAVE_ARM_SMCCC_DISCOVERY
> >>>>>  	select ARM_SCMI_HAVE_TRANSPORT
> >>>>>  	select ARM_SCMI_HAVE_SHMEM
> >>>>> @@ -112,6 +112,8 @@ config ARM_SCMI_TRANSPORT_SMC
> >>>>>  
> >>>>>  	  If you want the ARM SCMI PROTOCOL stack to include support for a
> >>>>>  	  transport based on SMC, answer Y.
> >>>>> +	  This driver can also be built as a module.  If so, the module
> >>>>> +	  will be called scmi_transport_smc.
> >>>>>  
> >>>>>  config ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE
> >>>>>  	bool "Enable atomic mode support for SCMI SMC transport"
> >>>>> diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
> >>>>> index 121612d75f0b..6868a47fa4ab 100644
> >>>>> --- a/drivers/firmware/arm_scmi/Makefile
> >>>>> +++ b/drivers/firmware/arm_scmi/Makefile
> >>>>> @@ -5,7 +5,6 @@ scmi-core-objs := $(scmi-bus-y)
> >>>>>  scmi-driver-y = driver.o notify.o
> >>>>>  scmi-driver-$(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT) += raw_mode.o
> >>>>>  scmi-transport-$(CONFIG_ARM_SCMI_HAVE_SHMEM) = shmem.o
> >>>>> -scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += smc.o
> >>>>>  scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
> >>>>>  scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
> >>>>>  scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
> >>>>> @@ -13,6 +12,7 @@ scmi-protocols-y := base.o clock.o perf.o power.o reset.o sensors.o system.o vol
> >>>>>  scmi-protocols-y += pinctrl.o
> >>>>>  scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
> >>>>>  
> >>>>> +obj-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += scmi_transport_smc.o
> >>>>>  obj-$(CONFIG_ARM_SCMI_TRANSPORT_MAILBOX) += scmi_transport_mailbox.o
> >>>>>  
> >>>>>  obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
> >>>>> diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
> >>>>> index c03f30db92e0..b5bd27eccf24 100644
> >>>>> --- a/drivers/firmware/arm_scmi/common.h
> >>>>> +++ b/drivers/firmware/arm_scmi/common.h
> >>>>> @@ -286,9 +286,6 @@ int scmi_xfer_raw_inflight_register(const struct scmi_handle *handle,
> >>>>>  int scmi_xfer_raw_wait_for_message_response(struct scmi_chan_info *cinfo,
> >>>>>  					    struct scmi_xfer *xfer,
> >>>>>  					    unsigned int timeout_ms);
> >>>>> -#ifdef CONFIG_ARM_SCMI_TRANSPORT_SMC
> >>>>> -extern const struct scmi_desc scmi_smc_desc;
> >>>>> -#endif
> >>>>>  #ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
> >>>>>  extern const struct scmi_desc scmi_virtio_desc;
> >>>>>  #endif
> >>>>> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> >>>>> index 96cf8ab4421e..b14c5326930a 100644
> >>>>> --- a/drivers/firmware/arm_scmi/driver.c
> >>>>> +++ b/drivers/firmware/arm_scmi/driver.c
> >>>>> @@ -3254,11 +3254,6 @@ static const struct of_device_id scmi_of_match[] = {
> >>>>>  #ifdef CONFIG_ARM_SCMI_TRANSPORT_OPTEE
> >>>>>  	{ .compatible = "linaro,scmi-optee", .data = &scmi_optee_desc },
> >>>>>  #endif
> >>>>> -#ifdef CONFIG_ARM_SCMI_TRANSPORT_SMC
> >>>>> -	{ .compatible = "arm,scmi-smc", .data = &scmi_smc_desc},
> >>>>> -	{ .compatible = "arm,scmi-smc-param", .data = &scmi_smc_desc},
> >>>>> -	{ .compatible = "qcom,scmi-smc", .data = &scmi_smc_desc},
> >>>>> -#endif
> >>>>>  #ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
> >>>>>  	{ .compatible = "arm,scmi-virtio", .data = &scmi_virtio_desc},
> >>>>>  #endif
> >>>>> diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/scmi_transport_smc.c
> >>>>> similarity index 89%
> >>>>> rename from drivers/firmware/arm_scmi/smc.c
> >>>>> rename to drivers/firmware/arm_scmi/scmi_transport_smc.c
> >>>>> index cb26b8aee01d..44da1a8d5387 100644
> >>>>> --- a/drivers/firmware/arm_scmi/smc.c
> >>>>> +++ b/drivers/firmware/arm_scmi/scmi_transport_smc.c
> >>>>> @@ -3,7 +3,7 @@
> >>>>>   * System Control and Management Interface (SCMI) Message SMC/HVC
> >>>>>   * Transport driver
> >>>>>   *
> >>>>> - * Copyright 2020 NXP
> >>>>> + * Copyright 2020-2024 NXP
> >>>>>   */
> >>>>>  
> >>>>>  #include <linux/arm-smccc.h>
> >>>>> @@ -16,6 +16,7 @@
> >>>>>  #include <linux/of_address.h>
> >>>>>  #include <linux/of_irq.h>
> >>>>>  #include <linux/limits.h>
> >>>>> +#include <linux/platform_device.h>
> >>>>>  #include <linux/processor.h>
> >>>>>  #include <linux/slab.h>
> >>>>>  
> >>>>> @@ -69,12 +70,14 @@ struct scmi_smc {
> >>>>>  	unsigned long cap_id;
> >>>>>  };
> >>>>>  
> >>>>> +static struct scmi_transport_core_operations *core;
> >>>>> +
> >>>>>  static irqreturn_t smc_msg_done_isr(int irq, void *data)
> >>>>>  {
> >>>>>  	struct scmi_smc *scmi_info = data;
> >>>>>  
> >>>>> -	scmi_rx_callback(scmi_info->cinfo,
> >>>>> -			 scmi_shmem_ops.read_header(scmi_info->shmem), NULL);
> >>>>> +	core->rx_callback(scmi_info->cinfo,
> >>>>> +			  core->shmem->read_header(scmi_info->shmem), NULL);
> >>>>>  
> >>>>>  	return IRQ_HANDLED;
> >>>>>  }
> >>>>> @@ -142,7 +145,7 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
> >>>>>  	if (!scmi_info)
> >>>>>  		return -ENOMEM;
> >>>>>  
> >>>>> -	scmi_info->shmem = scmi_shmem_ops.setup_iomap(cinfo, dev, tx);
> >>>>> +	scmi_info->shmem = core->shmem->setup_iomap(cinfo, dev, tx);
> >>>>>  	if (IS_ERR(scmi_info->shmem))
> >>>>>  		return PTR_ERR(scmi_info->shmem);
> >>>>>  
> >>>>> @@ -226,7 +229,7 @@ static int smc_send_message(struct scmi_chan_info *cinfo,
> >>>>>  	 */
> >>>>>  	smc_channel_lock_acquire(scmi_info, xfer);
> >>>>>  
> >>>>> -	scmi_shmem_ops.tx_prepare(scmi_info->shmem, xfer, cinfo);
> >>>>> +	core->shmem->tx_prepare(scmi_info->shmem, xfer, cinfo);
> >>>>>  
> >>>>>  	if (scmi_info->cap_id != ULONG_MAX)
> >>>>>  		arm_smccc_1_1_invoke(scmi_info->func_id, scmi_info->cap_id, 0,
> >>>>> @@ -250,7 +253,7 @@ static void smc_fetch_response(struct scmi_chan_info *cinfo,
> >>>>>  {
> >>>>>  	struct scmi_smc *scmi_info = cinfo->transport_info;
> >>>>>  
> >>>>> -	scmi_shmem_ops.fetch_response(scmi_info->shmem, xfer);
> >>>>> +	core->shmem->fetch_response(scmi_info->shmem, xfer);
> >>>>>  }
> >>>>>  
> >>>>>  static void smc_mark_txdone(struct scmi_chan_info *cinfo, int ret,
> >>>>> @@ -286,3 +289,19 @@ const struct scmi_desc scmi_smc_desc = {
> >>>>>  	.sync_cmds_completed_on_ret = true,
> >>>>>  	.atomic_enabled = IS_ENABLED(CONFIG_ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE),
> >>>>>  };
> >>>>> +
> >>>>> +static const struct of_device_id scmi_of_match[] = {
> >>>>> +	{ .compatible = "arm,scmi-smc" },
> >>>>> +	{ .compatible = "arm,scmi-smc-param" },
> >>>>> +	{ .compatible = "qcom,scmi-smc" },
> >>>>> +	{ /* Sentinel */ },
> >>>>> +};
> >>>>> +
> >>>> Hi Cristian,
> >>>>
> >>> Hi Nikunj,
> >>>
> >>> thanks for having a look first of all !
> >>>
> >>>> Would it make sense to associate scmi descriptor(scmi_smc_desc) with
> >>>> compatible as driver/platform data so we have flexibility to replicate
> >>>> it and modify parameters such as max_timeout_ms etc. for our platform?
> >>>>
> >>> Mmmm...not sure to have understood, because the scmi_smc_desc is
> >>> effecetively passed from this driver to the core via a bit of
> >>> (questionable) magic in the mega-macro
> >>>
> >>> DEFINE_SCMI_TRANSPORT_DRIVER(scmi_smc, scmi_of_match, &core);
> >>>
> >>> ...and it will end-up being set into the dev.platform_data and then
> >>> retrieved by the core SCMI stack driver in scmi_probe...
> >>>
> >>> ...OR...do you mean being able to somehow define 3 different
> >>> scmi_smc_desc* and then associate them to the different compatibles
> >>> and then, depending on which compatible is matched by this isame driver
> >>> at probe time, passing the related platform-specific desc to the core...
> >>>
> >>> ...in this latter case I suppose I can do it by playing with the macros
> >>> defs but maybe it is also the case to start thinking about splitting out
> >>> configuration stuff from the transport descriptor...
> >>>
> >>> I'll give it a go at passing the data around, and see how it plays out
> >>> if you confirm that this is what you meant...
> >> Hi Cristian,
> >>
> > Hi,
> >
> >> I wanted to send a patch for review(with older driver code) that will
> >> allow us to override transport parameters(e.g. max_timeout_ms,
> >> max_msg_size etc.) on Qualcomm platform. There could be multiple
> >> approaches- 1) add callbacks (similar to get_msg_size) in transport_ops
> >> and override the default or 2) replicate the descriptors for different
> >> compatible and change those values as needed. I was going with the
> >> second option but then I saw your patch and thought of throwing this at
> > :P 
> >
> >> you ;) I don't want you to hold your patch series for this but if you
> >> have a better way to achieve or a preferred way between the two
> >> mentioned before, please let me know. If you do want to add this feature
> >> in this patch series, that would be great!
> >>
> > Interesting, because there is also this thread flying around from Peng:
> >
> > https://lore.kernel.org/linux-arm-kernel/20240703031715.379815-1-peng.fan@oss.nxp.com/
> Thanks Cristian for the link. I was under the impression that DT binding
> that don't describe HW are not acceptable to DT maintainer. But if this

...I was under the same impression...not sure if this can be considered a
sort of HW characteristic that actually depends on how the platform (and the SCMI
server fw) is designed...

> patch goes through, I am fine using it. I would also like to have
> max_msg_size(and maybe max_msg) configurable.

...I am indeed monitoring the situation :D...in general I think there
are some transport-related characteritic that could made sense to fine
tune at compile time (in whatever way), while others can only be
menaninngfully discovered at runtime...like the get_msg_size...

Thanks,
Cristian

Re: [PATCH 5/8] firmware: arm_scmi: Make SMC transport a standalone driver

[kernel test robot]

Hi Cristian,

kernel test robot noticed the following build errors:

[auto build test ERROR on soc/for-next]
[also build test ERROR on next-20240709]
[cannot apply to linus/master v6.10-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Cristian-Marussi/firmware-arm_scmi-Introduce-setup_shmem_iomap/20240707-082513
base:   https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link:    https://lore.kernel.org/r/20240707002055.1835121-6-cristian.marussi%40arm.com
patch subject: [PATCH 5/8] firmware: arm_scmi: Make SMC transport a standalone driver
config: arm-randconfig-003-20240709 (https://download.01.org/0day-ci/archive/20240709/202407092025.wqTyq8Er-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project a0c6b8aef853eedaa0980f07c0a502a5a8a9740e)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240709/202407092025.wqTyq8Er-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407092025.wqTyq8Er-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/firmware/arm_scmi/scmi_transport_smc.c:157:58: warning: variable 'size' is uninitialized when used here [-Wuninitialized]
     157 |                 void __iomem *ptr = (void __iomem *)scmi_info->shmem + size - 8;
         |                                                                        ^~~~
   drivers/firmware/arm_scmi/scmi_transport_smc.c:136:22: note: initialize the variable 'size' to silence this warning
     136 |         resource_size_t size;
         |                             ^
         |                              = 0
>> drivers/firmware/arm_scmi/scmi_transport_smc.c:235:3: error: write to reserved register 'R7'
     235 |                 arm_smccc_1_1_invoke(scmi_info->func_id, scmi_info->cap_id, 0,
         |                 ^
   include/linux/arm-smccc.h:570:4: note: expanded from macro 'arm_smccc_1_1_invoke'
     570 |                         arm_smccc_1_1_smc(__VA_ARGS__);                 \
         |                         ^
   include/linux/arm-smccc.h:513:48: note: expanded from macro 'arm_smccc_1_1_smc'
     513 | #define arm_smccc_1_1_smc(...)  __arm_smccc_1_1(SMCCC_SMC_INST, __VA_ARGS__)
         |                                                 ^
   include/linux/arm-smccc.h:400:24: note: expanded from macro 'SMCCC_SMC_INST'
     400 | #define SMCCC_SMC_INST  __SMC(0)
         |                         ^
   note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   arch/arm/include/asm/opcodes.h:215:2: note: expanded from macro '__inst_arm_thumb32'
     215 |         __inst_thumb32(thumb_opcode)
         |         ^
   arch/arm/include/asm/opcodes.h:205:27: note: expanded from macro '__inst_thumb32'
     205 | #define __inst_thumb32(x) ___inst_thumb32(                              \
         |                           ^
   arch/arm/include/asm/opcodes.h:230:2: note: expanded from macro '___inst_thumb32'
     230 |         ".short " __stringify(first) ", " __stringify(second) "\n\t"
         |         ^
>> drivers/firmware/arm_scmi/scmi_transport_smc.c:235:3: error: write to reserved register 'R7'
   include/linux/arm-smccc.h:567:4: note: expanded from macro 'arm_smccc_1_1_invoke'
     567 |                         arm_smccc_1_1_hvc(__VA_ARGS__);                 \
         |                         ^
   include/linux/arm-smccc.h:529:48: note: expanded from macro 'arm_smccc_1_1_hvc'
     529 | #define arm_smccc_1_1_hvc(...)  __arm_smccc_1_1(SMCCC_HVC_INST, __VA_ARGS__)
         |                                                 ^
   include/linux/arm-smccc.h:401:24: note: expanded from macro 'SMCCC_HVC_INST'
     401 | #define SMCCC_HVC_INST  __HVC(0)
         |                         ^
   note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   arch/arm/include/asm/opcodes.h:215:2: note: expanded from macro '__inst_arm_thumb32'
     215 |         __inst_thumb32(thumb_opcode)
         |         ^
   arch/arm/include/asm/opcodes.h:205:27: note: expanded from macro '__inst_thumb32'
     205 | #define __inst_thumb32(x) ___inst_thumb32(                              \
         |                           ^
   arch/arm/include/asm/opcodes.h:230:2: note: expanded from macro '___inst_thumb32'
     230 |         ".short " __stringify(first) ", " __stringify(second) "\n\t"
         |         ^
>> drivers/firmware/arm_scmi/scmi_transport_smc.c:235:3: error: write to reserved register 'R7'
   include/linux/arm-smccc.h:573:4: note: expanded from macro 'arm_smccc_1_1_invoke'
     573 |                         __fail_smccc_1_1(__VA_ARGS__);                  \
         |                         ^
   include/linux/arm-smccc.h:540:8: note: expanded from macro '__fail_smccc_1_1'
     540 |                 asm ("" :                                               \
         |                      ^
   drivers/firmware/arm_scmi/scmi_transport_smc.c:238:3: error: write to reserved register 'R7'
     238 |                 arm_smccc_1_1_invoke(scmi_info->func_id, scmi_info->param_page,
         |                 ^
   include/linux/arm-smccc.h:570:4: note: expanded from macro 'arm_smccc_1_1_invoke'
     570 |                         arm_smccc_1_1_smc(__VA_ARGS__);                 \
         |                         ^
   include/linux/arm-smccc.h:513:48: note: expanded from macro 'arm_smccc_1_1_smc'
     513 | #define arm_smccc_1_1_smc(...)  __arm_smccc_1_1(SMCCC_SMC_INST, __VA_ARGS__)
         |                                                 ^
   include/linux/arm-smccc.h:400:24: note: expanded from macro 'SMCCC_SMC_INST'
     400 | #define SMCCC_SMC_INST  __SMC(0)
         |                         ^
   note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   arch/arm/include/asm/opcodes.h:215:2: note: expanded from macro '__inst_arm_thumb32'
     215 |         __inst_thumb32(thumb_opcode)
         |         ^
   arch/arm/include/asm/opcodes.h:205:27: note: expanded from macro '__inst_thumb32'
     205 | #define __inst_thumb32(x) ___inst_thumb32(                              \
         |                           ^
   arch/arm/include/asm/opcodes.h:230:2: note: expanded from macro '___inst_thumb32'
     230 |         ".short " __stringify(first) ", " __stringify(second) "\n\t"
         |         ^
   drivers/firmware/arm_scmi/scmi_transport_smc.c:238:3: error: write to reserved register 'R7'
   include/linux/arm-smccc.h:567:4: note: expanded from macro 'arm_smccc_1_1_invoke'
     567 |                         arm_smccc_1_1_hvc(__VA_ARGS__);                 \
         |                         ^
   include/linux/arm-smccc.h:529:48: note: expanded from macro 'arm_smccc_1_1_hvc'
     529 | #define arm_smccc_1_1_hvc(...)  __arm_smccc_1_1(SMCCC_HVC_INST, __VA_ARGS__)
         |                                                 ^
   include/linux/arm-smccc.h:401:24: note: expanded from macro 'SMCCC_HVC_INST'
     401 | #define SMCCC_HVC_INST  __HVC(0)
         |                         ^
   note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   arch/arm/include/asm/opcodes.h:215:2: note: expanded from macro '__inst_arm_thumb32'
     215 |         __inst_thumb32(thumb_opcode)
         |         ^
   arch/arm/include/asm/opcodes.h:205:27: note: expanded from macro '__inst_thumb32'
     205 | #define __inst_thumb32(x) ___inst_thumb32(                              \
         |                           ^
   arch/arm/include/asm/opcodes.h:230:2: note: expanded from macro '___inst_thumb32'
     230 |         ".short " __stringify(first) ", " __stringify(second) "\n\t"
         |         ^
   drivers/firmware/arm_scmi/scmi_transport_smc.c:238:3: error: write to reserved register 'R7'
   include/linux/arm-smccc.h:573:4: note: expanded from macro 'arm_smccc_1_1_invoke'
     573 |                         __fail_smccc_1_1(__VA_ARGS__);                  \
         |                         ^
   include/linux/arm-smccc.h:540:8: note: expanded from macro '__fail_smccc_1_1'
     540 |                 asm ("" :                                               \
         |                      ^
   1 warning and 6 errors generated.


vim +/R7 +235 drivers/firmware/arm_scmi/scmi_transport_smc.c

0bfdca8a8661aa drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  129  
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  130  static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  131  			  bool tx)
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  132  {
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  133  	struct device *cdev = cinfo->dev;
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  134  	unsigned long cap_id = ULONG_MAX;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  135  	struct scmi_smc *scmi_info;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  136  	resource_size_t size;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  137  	struct resource res;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  138  	u32 func_id;
d1ff11d7ad8704 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2023-07-19  139  	int ret;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  140  
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  141  	if (!tx)
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  142  		return -ENODEV;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  143  
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  144  	scmi_info = devm_kzalloc(dev, sizeof(*scmi_info), GFP_KERNEL);
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  145  	if (!scmi_info)
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  146  		return -ENOMEM;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  147  
728a057b6ab114 drivers/firmware/arm_scmi/scmi_transport_smc.c Cristian Marussi 2024-07-07  148  	scmi_info->shmem = core->shmem->setup_iomap(cinfo, dev, tx);
6c5d3315c40fa0 drivers/firmware/arm_scmi/smc.c                Peng Fan         2024-07-07  149  	if (IS_ERR(scmi_info->shmem))
6c5d3315c40fa0 drivers/firmware/arm_scmi/smc.c                Peng Fan         2024-07-07  150  		return PTR_ERR(scmi_info->shmem);
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  151  
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  152  	ret = of_property_read_u32(dev->of_node, "arm,smc-id", &func_id);
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  153  	if (ret < 0)
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  154  		return ret;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  155  
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  156  	if (of_device_is_compatible(dev->of_node, "qcom,scmi-smc")) {
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09 @157  		void __iomem *ptr = (void __iomem *)scmi_info->shmem + size - 8;
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  158  		/* The capability-id is kept in last 8 bytes of shmem.
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  159  		 *     +-------+ <-- 0
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  160  		 *     | shmem |
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  161  		 *     +-------+ <-- size - 8
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  162  		 *     | capId |
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  163  		 *     +-------+ <-- size
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  164  		 */
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  165  		memcpy_fromio(&cap_id, ptr, sizeof(cap_id));
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  166  	}
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  167  
5f2ea10a808aef drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-05-06  168  	if (of_device_is_compatible(dev->of_node, "arm,scmi-smc-param")) {
5f2ea10a808aef drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-05-06  169  		scmi_info->param_page = SHMEM_PAGE(res.start);
5f2ea10a808aef drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-05-06  170  		scmi_info->param_offset = SHMEM_OFFSET(res.start);
5f2ea10a808aef drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-05-06  171  	}
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  172  	/*
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  173  	 * If there is an interrupt named "a2p", then the service and
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  174  	 * completion of a message is signaled by an interrupt rather than by
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  175  	 * the return of the SMC call.
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  176  	 */
d1ff11d7ad8704 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2023-07-19  177  	scmi_info->irq = of_irq_get_byname(cdev->of_node, "a2p");
d1ff11d7ad8704 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2023-07-19  178  	if (scmi_info->irq > 0) {
d1ff11d7ad8704 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2023-07-19  179  		ret = request_irq(scmi_info->irq, smc_msg_done_isr,
d1ff11d7ad8704 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2023-07-19  180  				  IRQF_NO_SUSPEND, dev_name(dev), scmi_info);
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  181  		if (ret) {
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  182  			dev_err(dev, "failed to setup SCMI smc irq\n");
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  183  			return ret;
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  184  		}
f716cbd33f038a drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  185  	} else {
f716cbd33f038a drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  186  		cinfo->no_completion_irq = true;
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  187  	}
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  188  
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  189  	scmi_info->func_id = func_id;
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  190  	scmi_info->cap_id = cap_id;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  191  	scmi_info->cinfo = cinfo;
0bfdca8a8661aa drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  192  	smc_channel_lock_init(scmi_info);
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  193  	cinfo->transport_info = scmi_info;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  194  
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  195  	return 0;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  196  }
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  197  
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  198  static int smc_chan_free(int id, void *p, void *data)
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  199  {
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  200  	struct scmi_chan_info *cinfo = p;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  201  	struct scmi_smc *scmi_info = cinfo->transport_info;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  202  
f1d71576d2c9ec drivers/firmware/arm_scmi/smc.c                Andre Przywara   2024-01-26  203  	/*
f1d71576d2c9ec drivers/firmware/arm_scmi/smc.c                Andre Przywara   2024-01-26  204  	 * Different protocols might share the same chan info, so a previous
f1d71576d2c9ec drivers/firmware/arm_scmi/smc.c                Andre Przywara   2024-01-26  205  	 * smc_chan_free call might have already freed the structure.
f1d71576d2c9ec drivers/firmware/arm_scmi/smc.c                Andre Przywara   2024-01-26  206  	 */
f1d71576d2c9ec drivers/firmware/arm_scmi/smc.c                Andre Przywara   2024-01-26  207  	if (!scmi_info)
f1d71576d2c9ec drivers/firmware/arm_scmi/smc.c                Andre Przywara   2024-01-26  208  		return 0;
f1d71576d2c9ec drivers/firmware/arm_scmi/smc.c                Andre Przywara   2024-01-26  209  
d1ff11d7ad8704 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2023-07-19  210  	/* Ignore any possible further reception on the IRQ path */
d1ff11d7ad8704 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2023-07-19  211  	if (scmi_info->irq > 0)
d1ff11d7ad8704 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2023-07-19  212  		free_irq(scmi_info->irq, scmi_info);
d1ff11d7ad8704 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2023-07-19  213  
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  214  	cinfo->transport_info = NULL;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  215  	scmi_info->cinfo = NULL;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  216  
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  217  	return 0;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  218  }
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  219  
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  220  static int smc_send_message(struct scmi_chan_info *cinfo,
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  221  			    struct scmi_xfer *xfer)
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  222  {
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  223  	struct scmi_smc *scmi_info = cinfo->transport_info;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  224  	struct arm_smccc_res res;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  225  
f716cbd33f038a drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  226  	/*
0bfdca8a8661aa drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  227  	 * Channel will be released only once response has been
f716cbd33f038a drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  228  	 * surely fully retrieved, so after .mark_txdone()
f716cbd33f038a drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  229  	 */
0bfdca8a8661aa drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  230  	smc_channel_lock_acquire(scmi_info, xfer);
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  231  
728a057b6ab114 drivers/firmware/arm_scmi/scmi_transport_smc.c Cristian Marussi 2024-07-07  232  	core->shmem->tx_prepare(scmi_info->shmem, xfer, cinfo);
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  233  
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  234  	if (scmi_info->cap_id != ULONG_MAX)
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09 @235  		arm_smccc_1_1_invoke(scmi_info->func_id, scmi_info->cap_id, 0,
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  236  				     0, 0, 0, 0, 0, &res);
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  237  	else
1f17395124a53a drivers/firmware/arm_scmi/smc.c                Sudeep Holla     2023-10-09  238  		arm_smccc_1_1_invoke(scmi_info->func_id, scmi_info->param_page,
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  239  				     scmi_info->param_offset, 0, 0, 0, 0, 0,
da405477e76707 drivers/firmware/arm_scmi/smc.c                Nikunj Kela      2023-10-09  240  				     &res);
dd820ee21d5e07 drivers/firmware/arm_scmi/smc.c                Jim Quinlan      2020-12-22  241  
f7199cf489027a drivers/firmware/arm_scmi/smc.c                Sudeep Holla     2020-04-17  242  	/* Only SMCCC_RET_NOT_SUPPORTED is valid error code */
f716cbd33f038a drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  243  	if (res.a0) {
0bfdca8a8661aa drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  244  		smc_channel_lock_release(scmi_info);
f7199cf489027a drivers/firmware/arm_scmi/smc.c                Sudeep Holla     2020-04-17  245  		return -EOPNOTSUPP;
f716cbd33f038a drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  246  	}
f716cbd33f038a drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  247  
f7199cf489027a drivers/firmware/arm_scmi/smc.c                Sudeep Holla     2020-04-17  248  	return 0;
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  249  }
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  250  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 5/8] firmware: arm_scmi: Make SMC transport a standalone driver

[kernel test robot]

Hi Cristian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on soc/for-next]
[also build test WARNING on next-20240709]
[cannot apply to linus/master v6.10-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Cristian-Marussi/firmware-arm_scmi-Introduce-setup_shmem_iomap/20240707-082513
base:   https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link:    https://lore.kernel.org/r/20240707002055.1835121-6-cristian.marussi%40arm.com
patch subject: [PATCH 5/8] firmware: arm_scmi: Make SMC transport a standalone driver
config: arm64-randconfig-r131-20240709 (https://download.01.org/0day-ci/archive/20240710/202407101157.p2OvSFcO-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240710/202407101157.p2OvSFcO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407101157.p2OvSFcO-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/firmware/arm_scmi/scmi_transport_smc.c:276:24: sparse: sparse: symbol 'scmi_smc_desc' was not declared. Should it be static?

vim +/scmi_smc_desc +276 drivers/firmware/arm_scmi/scmi_transport_smc.c

1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  275  
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08 @276  const struct scmi_desc scmi_smc_desc = {
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  277  	.ops = &scmi_smc_ops,
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  278  	.max_rx_timeout_ms = 30,
7adb2c8aaaa6a3 drivers/firmware/arm_scmi/smc.c                Etienne Carriere 2020-10-08  279  	.max_msg = 20,
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  280  	.max_msg_size = 128,
117542b81fe7b1 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  281  	/*
117542b81fe7b1 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  282  	 * Setting .sync_cmds_atomic_replies to true for SMC assumes that,
117542b81fe7b1 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  283  	 * once the SMC instruction has completed successfully, the issued
117542b81fe7b1 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  284  	 * SCMI command would have been already fully processed by the SCMI
117542b81fe7b1 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  285  	 * platform firmware and so any possible response value expected
117542b81fe7b1 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  286  	 * for the issued command will be immmediately ready to be fetched
117542b81fe7b1 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  287  	 * from the shared memory area.
117542b81fe7b1 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  288  	 */
117542b81fe7b1 drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  289  	.sync_cmds_completed_on_ret = true,
0bfdca8a8661aa drivers/firmware/arm_scmi/smc.c                Cristian Marussi 2021-12-20  290  	.atomic_enabled = IS_ENABLED(CONFIG_ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE),
1dc6558062dadf drivers/firmware/arm_scmi/smc.c                Peng Fan         2020-03-08  291  };
728a057b6ab114 drivers/firmware/arm_scmi/scmi_transport_smc.c Cristian Marussi 2024-07-07  292  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 5/8] firmware: arm_scmi: Make SMC transport a standalone driver

[Cristian Marussi]

On Mon, Jul 08, 2024 at 10:59:33AM -0700, Nikunj Kela wrote:
> 
> On 7/8/2024 8:47 AM, Cristian Marussi wrote:
> > On Mon, Jul 08, 2024 at 08:23:56AM -0700, Nikunj Kela wrote:
> >> On 7/8/2024 7:27 AM, Cristian Marussi wrote:
> >>> On Sun, Jul 07, 2024 at 09:52:49AM -0700, Nikunj Kela wrote:
> >>>> On 7/6/2024 5:20 PM, Cristian Marussi wrote:
> >>>>> Make SCMI SMC transport a standalone driver that can be optionally
> >>>>> loaded as a module.
> >>>>>
> >>>>> CC: Peng Fan <peng.fan@nxp.com>
> >>>>> CC: Nikunj Kela <quic_nkela@quicinc.com>
> >>>>> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> >>>>> ---
> >>>>>  drivers/firmware/arm_scmi/Kconfig             |  4 ++-
> >>>>>  drivers/firmware/arm_scmi/Makefile            |  2 +-
> >>>>>  drivers/firmware/arm_scmi/common.h            |  3 --
> >>>>>  drivers/firmware/arm_scmi/driver.c            |  5 ---
> >>>>>  .../arm_scmi/{smc.c => scmi_transport_smc.c}  | 31 +++++++++++++++----
> >>>>>  5 files changed, 29 insertions(+), 16 deletions(-)
> >>>>>  rename drivers/firmware/arm_scmi/{smc.c => scmi_transport_smc.c} (89%)
> >>>>>

Hi Nikunj,

> >>>>> diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig

[snip]

> >>>
> >>>> Would it make sense to associate scmi descriptor(scmi_smc_desc) with
> >>>> compatible as driver/platform data so we have flexibility to replicate
> >>>> it and modify parameters such as max_timeout_ms etc. for our platform?
> >>>>
> >>> Mmmm...not sure to have understood, because the scmi_smc_desc is
> >>> effecetively passed from this driver to the core via a bit of
> >>> (questionable) magic in the mega-macro
> >>>
> >>> DEFINE_SCMI_TRANSPORT_DRIVER(scmi_smc, scmi_of_match, &core);
> >>>
> >>> ...and it will end-up being set into the dev.platform_data and then
> >>> retrieved by the core SCMI stack driver in scmi_probe...
> >>>
> >>> ...OR...do you mean being able to somehow define 3 different
> >>> scmi_smc_desc* and then associate them to the different compatibles
> >>> and then, depending on which compatible is matched by this isame driver
> >>> at probe time, passing the related platform-specific desc to the core...
> >>>
> >>> ...in this latter case I suppose I can do it by playing with the macros
> >>> defs but maybe it is also the case to start thinking about splitting out
> >>> configuration stuff from the transport descriptor...
> >>>
> >>> I'll give it a go at passing the data around, and see how it plays out
> >>> if you confirm that this is what you meant...
> >> Hi Cristian,
> >>
> > Hi,
> >
> >> I wanted to send a patch for review(with older driver code) that will
> >> allow us to override transport parameters(e.g. max_timeout_ms,
> >> max_msg_size etc.) on Qualcomm platform. There could be multiple
> >> approaches- 1) add callbacks (similar to get_msg_size) in transport_ops
> >> and override the default or 2) replicate the descriptors for different
> >> compatible and change those values as needed. I was going with the
> >> second option but then I saw your patch and thought of throwing this at
> > :P 
> >
> >> you ;) I don't want you to hold your patch series for this but if you
> >> have a better way to achieve or a preferred way between the two
> >> mentioned before, please let me know. If you do want to add this feature
> >> in this patch series, that would be great!
> >>
> > Interesting, because there is also this thread flying around from Peng:
> >
> > https://lore.kernel.org/linux-arm-kernel/20240703031715.379815-1-peng.fan@oss.nxp.com/
> Thanks Cristian for the link. I was under the impression that DT binding
> that don't describe HW are not acceptable to DT maintainer. But if this
> patch goes through, I am fine using it. I would also like to have
> max_msg_size(and maybe max_msg) configurable.
> >
> > ...which I was planning in embedding in this series; Peng's proposal is
> > limited to timeout and based on DT (and title is a bit misleading...the
> > series is not mailbox-only related)....
> >
> > ...now I am NOT saying that we should dump all configs into the DT, but
> > just that this issue about configurability of transports is already sort
> > of a known-problem, and it is a while that it floats in the back of my mind...
> > i.e. the fact that the transport runtime configurations should be *somehow*
> > independent of the transport descriptor and *somehow* configurable....so now
> > only remains to *somehow* figure out how to do it :D
> >
> > ... I'll have a though and maybe cannibalize your ideas and Peng's one and then
> > we could have a discussion around this on the list to address eveybody's needs...
> >
> > ...and maybe, in the meantime, you could also post your proposed series about
> > this even though based on the old code, but as an RFC, just to make a point and
> > detail the needs...but yeah only if it does not require a bunch of extra work from
> > you given that it is only to be used as a basis for discussion ....
> >
> > ...up to you what do you prefer.

I am in the middle of some rework around these SCMI transport characteristics,
(max_msg / max_msg_size) that by itself currently are not so well defined in
terms of semantic and usage in the codebase, so I am trying to remove any
ambiguity and fix innaccuracies. (i.e. even before any DT transport-related
binding is introduced).

I was trying to understand better how you will use this new binding....
...I mean, of course you will use it to describe new sizes for the shmem areas
and the number of in-flight messages, but is this related to the RIDE platform
setup where you use a high number of SCMI instances ?

...if it is, should I guess that you will tailor somehow all of these shmem
areas' size to the specific/limited use of those scmi nodes ? (in terms of
protocols and messages I mean)...

...or you just need more space in general so you bumped the existing shmem
descriptors depending on the platform and you need to align max_msg_size
accordingly to the transport needs for that platform ?

..or I am overthinking ? :D

IOW what are your use-cases...so that I can try to fit them all in a general
way (possibly)...

Thanks,
Cristian