NXP Add SCMI P2A workflow and BBNSM protocol

drivers/firmware/scmi/core.c

@@ -63,11 +63,6 @@ static int scmi_core_setup_chan(const struct device *transport,
 		return 0;
 	}
 
-	/* no support for RX channels ATM */
-	if (!tx) {
-		return -ENOTSUP;
-	}
-
 	k_mutex_init(&chan->lock);
 	k_sem_init(&chan->sem, 0, 1);
 
@@ -226,6 +221,7 @@ static int scmi_core_protocol_setup(const struct device *transport)
 #ifndef CONFIG_ARM_SCMI_TRANSPORT_HAS_STATIC_CHANNELS
 		/* no static channel allocation, attempt dynamic binding */
 		it->tx = scmi_transport_request_channel(transport, it->id, true);
+		it->rx = scmi_transport_request_channel(transport, it->id, false);
 #endif /* CONFIG_ARM_SCMI_TRANSPORT_HAS_STATIC_CHANNELS */
 
 		if (!it->tx) {
@@ -236,6 +232,14 @@ static int scmi_core_protocol_setup(const struct device *transport)
 		if (ret < 0) {
 			return ret;
 		}
+
+		/* notification/delayed reply channel is optional */
+		if (it->rx) {
+			ret = scmi_core_setup_chan(transport, it->rx, false);
+			if (ret < 0) {
+				return ret;
+			}
+		}
 	}
 
 	return 0;

drivers/firmware/scmi/mailbox.c

@@ -9,10 +9,22 @@
 
 LOG_MODULE_REGISTER(scmi_mbox);
 
-static void scmi_mbox_cb(const struct device *mbox,
-			 mbox_channel_id_t channel_id,
-			 void *user_data,
-			 struct mbox_msg *data)
+static void scmi_mbox_tx_reply_cb(const struct device *mbox,
+				mbox_channel_id_t channel_id,
+				void *user_data,
+				struct mbox_msg *data)
+{
+	struct scmi_channel *scmi_chan = user_data;
+
+	if (scmi_chan->cb) {
+		scmi_chan->cb(scmi_chan);
+	}
+}
+
+static void scmi_mbox_notify_cb(const struct device *mbox,
+				mbox_channel_id_t channel_id,
+				void *user_data,
+				struct mbox_msg *data)
 {
 	struct scmi_channel *scmi_chan = user_data;
 
@@ -71,29 +83,34 @@ static int scmi_mbox_setup_chan(const struct device *transport,
 {
 	int ret;
 	struct scmi_mbox_channel *mbox_chan;
-	struct mbox_dt_spec *tx_reply;
+	struct mbox_dt_spec *mbox_spec;
 
 	mbox_chan = chan->data;
 
-	if (!tx) {
-		return -ENOTSUP;
-	}
-
-	if (mbox_chan->tx_reply.dev) {
-		tx_reply = &mbox_chan->tx_reply;
+	if (tx) {
+		mbox_spec = mbox_chan->tx_reply.dev ? &mbox_chan->tx_reply : &mbox_chan->tx;
+		ret = mbox_register_callback_dt(mbox_spec, scmi_mbox_tx_reply_cb, chan);
+		if (ret < 0) {
+			LOG_ERR("failed to register reply cb on %s",
+					mbox_chan->tx_reply.dev ? "tx_reply" : "tx");
+			return ret;
+		}
 	} else {
-		tx_reply = &mbox_chan->tx;
-	}
-
-	ret = mbox_register_callback_dt(tx_reply, scmi_mbox_cb, chan);
-	if (ret < 0) {
-		LOG_ERR("failed to register tx reply cb");
-		return ret;
+		if (!mbox_chan->rx.dev) {
+			LOG_ERR("RX channel not defined");
+			return -ENOTSUP;
+		}
+		mbox_spec = &mbox_chan->rx;
+		ret = mbox_register_callback_dt(&mbox_chan->rx, scmi_mbox_notify_cb, chan);
+		if (ret < 0) {
+			LOG_ERR("failed to register notify cb on rx");
+			return ret;
+		}
 	}
 
-	ret = mbox_set_enabled_dt(tx_reply, true);
+	ret = mbox_set_enabled_dt(mbox_spec, true);
 	if (ret < 0) {
-		LOG_ERR("failed to enable tx reply dbell");
+		LOG_ERR("failed to enable %s dbell", tx ? "tx" : "rx");
 	}
 
 	/* enable interrupt-based communication */

drivers/firmware/scmi/mailbox.h

@@ -40,17 +40,34 @@
 		.tx_reply = _SCMI_MBOX_CHAN_DBELL(node_id, tx_reply),	\
 	}
 
+/* define private data for a protocol RX channel */
+#define _SCMI_MBOX_CHAN_DEFINE_PRIV_RX(node_id, proto)			\
+	static struct scmi_mbox_channel _SCMI_MBOX_CHAN_NAME(proto, 1) =\
+	{								\
+		.shmem = _SCMI_MBOX_SHMEM_BY_IDX(node_id, 1),		\
+		.rx = _SCMI_MBOX_CHAN_DBELL(node_id, rx),		\
+	}
+
 /*
  * Define a mailbox channel. This does two things:
  *	1) Define the mandatory `struct scmi_channel` structure
  *	2) Define the mailbox-specific private data for said
  *	channel (i.e: a struct scmi_mbox_channel)
  */
-#define _SCMI_MBOX_CHAN_DEFINE(node_id, proto, idx)				\
+#define _SCMI_MBOX_CHAN_DEFINE_TX(node_id, proto, idx)				\
 	_SCMI_MBOX_CHAN_DEFINE_PRIV_TX(node_id, proto);				\
 	DT_SCMI_TRANSPORT_CHAN_DEFINE(node_id, idx, proto,			\
 				    &(_SCMI_MBOX_CHAN_NAME(proto, idx)));	\
 
+#define _SCMI_MBOX_CHAN_DEFINE_RX(node_id, proto, idx)				\
+	_SCMI_MBOX_CHAN_DEFINE_PRIV_RX(node_id, proto);				\
+	DT_SCMI_TRANSPORT_CHAN_DEFINE(node_id, idx, proto,			\
+				    &(_SCMI_MBOX_CHAN_NAME(proto, idx)));	\
+
+#define _SCMI_MBOX_CHAN_DEFINE_RX_OPTIONAL(node_id, proto, idx)			\
+	COND_CODE_1(DT_PROP_HAS_IDX(node_id, shmem, idx),			\
+			(_SCMI_MBOX_CHAN_DEFINE_RX(node_id, proto, idx)),	\
+			())
 /*
  * Optionally define a mailbox channel for a protocol. This is optional
  * because a protocol might not have a dedicated channel.
@@ -69,24 +86,26 @@
 	_SCMI_MBOX_CHAN_DEFINE_OPTIONAL(node_id, DT_REG_ADDR(node_id), 0)
 
 /* define and validate base protocol TX channel */
-#define DT_INST_SCMI_MBOX_BASE_CHAN_DEFINE(inst)			\
-	BUILD_ASSERT(DT_INST_PROP_LEN(inst, mboxes) != 1 ||		\
-		     (DT_INST_PROP_HAS_IDX(inst, shmem, 0) &&		\
-		     DT_INST_PROP_HAS_NAME(inst, mboxes, tx)),		\
-		     "bad bidirectional channel description");		\
-									\
-	BUILD_ASSERT(DT_INST_PROP_LEN(inst, mboxes) != 2 ||		\
-		     (DT_INST_PROP_HAS_NAME(inst, mboxes, tx) &&	\
-		     DT_INST_PROP_HAS_NAME(inst, mboxes, tx_reply)),	\
-		     "bad unidirectional channel description");		\
-									\
-	BUILD_ASSERT(DT_INST_PROP_LEN(inst, shmem) == 1,		\
-		     "bad SHMEM count");				\
-									\
-	BUILD_ASSERT(DT_INST_PROP_LEN(inst, mboxes) <= 2,		\
-		     "bad mbox count");					\
-									\
-	_SCMI_MBOX_CHAN_DEFINE(DT_INST(inst, DT_DRV_COMPAT), SCMI_PROTOCOL_BASE, 0)
+#define DT_INST_SCMI_MBOX_BASE_CHAN_DEFINE(inst)						\
+	BUILD_ASSERT(DT_INST_PROP_LEN(inst, mboxes) != 1 ||					\
+		     (DT_INST_PROP_HAS_IDX(inst, shmem, 0) &&					\
+		     DT_INST_PROP_HAS_NAME(inst, mboxes, tx)),					\
+		     "bad bidirectional channel description");					\
+												\
+	BUILD_ASSERT(DT_INST_PROP_LEN(inst, mboxes) != 2 ||					\
+		     ((DT_INST_PROP_HAS_NAME(inst, mboxes, tx) &&				\
+		     (DT_INST_PROP_HAS_NAME(inst, mboxes, tx_reply) ||				\
+		      DT_INST_PROP_HAS_NAME(inst, mboxes, rx)))),				\
+		     "bad unidirectional channel description");					\
+												\
+	BUILD_ASSERT(DT_INST_PROP_LEN(inst, shmem) <= 2,					\
+		     "bad SHMEM count");							\
+												\
+	BUILD_ASSERT(DT_INST_PROP_LEN(inst, mboxes) <= 2,					\
+		     "bad mbox count");								\
+												\
+	_SCMI_MBOX_CHAN_DEFINE_TX(DT_INST(inst, DT_DRV_COMPAT), SCMI_PROTOCOL_BASE, 0)		\
+	_SCMI_MBOX_CHAN_DEFINE_RX_OPTIONAL(DT_INST(inst, DT_DRV_COMPAT), SCMI_PROTOCOL_BASE, 1) \
 
 /*
  * Define the mailbox-based transport layer. What this does is:
@@ -112,6 +131,8 @@ struct scmi_mbox_channel {
 	struct mbox_dt_spec tx;
 	/* TX reply dbell */
 	struct mbox_dt_spec tx_reply;
+	/* RX dbell */
+	struct mbox_dt_spec rx;
 };
 
 #endif /* _ZEPHYR_DRIVERS_FIRMWARE_SCMI_MAILBOX_H_ */

include/zephyr/drivers/firmware/scmi/protocol.h

@@ -76,6 +76,8 @@ struct scmi_protocol {
 	uint32_t id;
 	/** TX channel */
 	struct scmi_channel *tx;
+	/** RX channel */
+	struct scmi_channel *rx;
 	/** transport layer device */
 	const struct device *transport;
 	/** protocol private data */

include/zephyr/drivers/firmware/scmi/util.h

@@ -53,7 +53,7 @@
 #define SCMI_TRANSPORT_CHAN_NAME(proto, idx) CONCAT(scmi_channel_, proto, _, idx)
 
 /**
- * @brief Declare a TX SCMI channel
+ * @brief Declare TX/RX SCMI channel
  *
  * Given a node_id for a protocol, this macro declares the SCMI
  * TX channel statically bound to said protocol via the "extern"
@@ -70,6 +70,15 @@
 		    (extern struct scmi_channel					\
 		     SCMI_TRANSPORT_CHAN_NAME(SCMI_PROTOCOL_BASE, 0);))		\
 
+#define DT_SCMI_TRANSPORT_RX_CHAN_DECLARE(node_id)				\
+	COND_CODE_1(DT_SCMI_TRANSPORT_PROTO_HAS_CHAN(node_id, 1),		\
+		    (extern struct scmi_channel					\
+		     SCMI_TRANSPORT_CHAN_NAME(DT_REG_ADDR_RAW(node_id), 1);),	\
+		     (COND_CODE_1(DT_PROP_HAS_IDX(DT_PARENT(node_id), shmem, 1),\
+				  (extern struct scmi_channel			\
+				   SCMI_TRANSPORT_CHAN_NAME(SCMI_PROTOCOL_BASE, 1);), \
+				   (/* no decl when NULL */))))
+
 /**
  * @brief Declare SCMI TX/RX channels
  *
@@ -84,6 +93,7 @@
  */
 #define DT_SCMI_TRANSPORT_CHANNELS_DECLARE(node_id)				\
 	DT_SCMI_TRANSPORT_TX_CHAN_DECLARE(node_id)				\
+	DT_SCMI_TRANSPORT_RX_CHAN_DECLARE(node_id)				\
 
 /**
  * @brief Declare SCMI TX/RX channels using node instance number
@@ -97,22 +107,28 @@
 	DT_SCMI_TRANSPORT_CHANNELS_DECLARE(DT_INST(inst, DT_DRV_COMPAT))
 
 /**
- * @brief Get a reference to a protocol's SCMI TX channel
+ * @brief Get a reference to a protocol's SCMI TX/RX channel
  *
  * Given a node_id for a protocol, this macro returns a
  * reference to an SCMI TX channel statically bound to said
  * protocol.
  *
  * @param node_id protocol node identifier
  *
- * @return reference to the struct scmi_channel of the TX channel
+ * @return reference to the struct scmi_channel of the TX/RX channel
  * bound to the protocol identifier by node_id
  */
 #define DT_SCMI_TRANSPORT_TX_CHAN(node_id)					\
 	COND_CODE_1(DT_SCMI_TRANSPORT_PROTO_HAS_CHAN(node_id, 0),		\
 		    (&SCMI_TRANSPORT_CHAN_NAME(DT_REG_ADDR_RAW(node_id), 0)),	\
 		    (&SCMI_TRANSPORT_CHAN_NAME(SCMI_PROTOCOL_BASE, 0)))
 
+#define DT_SCMI_TRANSPORT_RX_CHAN(node_id)						\
+	COND_CODE_1(DT_SCMI_TRANSPORT_PROTO_HAS_CHAN(node_id, 1),			\
+		    (&SCMI_TRANSPORT_CHAN_NAME(DT_REG_ADDR_RAW(node_id), 1)),		\
+			(COND_CODE_1(DT_PROP_HAS_IDX(DT_PARENT(node_id), shmem, 1),	\
+				(&SCMI_TRANSPORT_CHAN_NAME(SCMI_PROTOCOL_BASE, 1)),	\
+				(NULL))))
 /**
  * @brief Define an SCMI channel for a protocol
  *
@@ -151,6 +167,7 @@
 	{									\
 		.id = proto,							\
 		.tx = DT_SCMI_TRANSPORT_TX_CHAN(node_id),			\
+		.rx = DT_SCMI_TRANSPORT_RX_CHAN(node_id),			\
 		.data = pdata,							\
 	}