NXP Add SCMI P2A workflow and BBNSM protocol #93305

yongxu-wang15 · 2025-07-18T07:07:50Z

SCMI P2A support: Adds RX channel handling to support notifications from platform to agent, including callback categorization for different message types.
Shared memory separation: Extends SCMI shared memory to an phandles to allow TX and RX to use different memory regions, which is required on platforms like i.MX95.
BBNSM protocol support: Adds initial support for the BBNSM domain protocol
test pass in imx95 m7 hello_word sample, The M7 core can continuously receive button messages from the SCMI platform.

yongxu-wang15 · 2025-07-18T08:19:05Z

v2: updated to fix twister build issue in imx95/imx943 m/a core

dbaluta · 2025-07-21T07:18:33Z

drivers/firmware/scmi/core.c

+		ret = k_mutex_lock(&proto->rx->lock, K_USEC(SCMI_CHAN_LOCK_TIMEOUT_USEC));
+		if (ret < 0) {
+			LOG_ERR("failed to acquire chan lock");
+			return ret;


Here you are returning with mutex locked. I think you might want to goto out_release_mutex.

if k_mutex_lock error, it not need to release mutex from my understand

drivers/firmware/scmi/core.c

dbaluta · 2025-07-21T07:21:59Z

drivers/firmware/scmi/mailbox.c

+
+#if defined(CONFIG_NXP_SCMI_BBM_HELPERS)
+	uint32_t flags;
+


This part should come in a different patch. Also, please abstract it under a function.

have removed this call back flow into another commit, thanks

dbaluta · 2025-07-21T07:24:23Z

doc/hardware/arch/arm-scmi.rst

 	This driver is NXP-specific. As such, it may only be used on NXP
 	system that uses SCMI for cpu domain management operations.

+NXP - BBNSM management


At least in the commit message you should explain what BBNSM acronym stands for so that you can allow reviewers to understand what is this all about.

sure, updated it in doc and commit description

I don't really see this scaling very well. So, how about moving all of the NXP vendor-specific stuff to a different page? Also, do we have a link to some sort of documentation fully describing the protocol? Kinda like what we have for Linux: https://elixir.bootlin.com/linux/v6.16.1/source/drivers/firmware/arm_scmi/vendors/imx/imx95.rst

CC: @kartben would we bloat the documentation page if we were to add a full explanation of each vendor-specific protocol?

so, do you mean that we need add an directory scmi/vendor/nxp into zephyr/doc/hardware/arch? I can organize it if need, thanks
ping: @kartben , can you give some suggestion about this? thanks a lot

yongxu-wang15 · 2025-07-22T09:29:44Z

V3: update pr according into @dbaluta comments

add more description about bbnsm
separate into another patch about p2a callback flow

yongxu-wang15 · 2025-08-11T09:01:54Z

hi @dbaluta, have updated it according into your suggestion, other comments about it? thanks a lot

LaurentiuM1234

Some questions/comments below.

None of your commits should break the build if applied incrementally. If I try to build the hello_world application with 6fbb947 and then 9a3dda1 applied, the build will break.

I believe @dbaluta has told you multiple times to break your changes into different patches, based on what the changes are trying to achieve. This, of course, applies to the whole PR, not just the areas on which @dbaluta commented.

As things stand now I don't believe this is ready for merge, which is why I'm blocking this.

LaurentiuM1234 · 2025-10-01T08:49:23Z

drivers/firmware/scmi/nxp/bbm.c

+
+DT_SCMI_PROTOCOL_DEFINE_NODEV(DT_INST(0, nxp_scmi_bbm), NULL);
+
+extern uint32_t scmi_p2a_header_token;


why is this driver added in the first commit? would it even compile? would it do anything useful without the notifications infrastructure?

LaurentiuM1234 · 2025-10-01T09:31:47Z

drivers/firmware/scmi/nxp/bbm.c

+	reply.len = sizeof(uint32_t);
+	reply.content = &status;
+
+	use_polling = k_is_pre_kernel();


no need for this variable - can just pass k_is_pre_kernel() to scmi_send_message()

sure, updated

LaurentiuM1234 · 2025-10-01T09:32:43Z

drivers/firmware/scmi/nxp/bbm.c

+
+	msg.hdr = SCMI_MESSAGE_HDR_MAKE(SCMI_PROTO_BBM_BBM_BUTTON_NOTIFY, SCMI_COMMAND,
+					proto->id, 0);
+	msg.len = sizeof(uint32_t);


sizeof(flags)?

LaurentiuM1234 · 2025-10-01T09:32:53Z

drivers/firmware/scmi/nxp/bbm.c

+	msg.content = &flags;
+
+	reply.hdr = msg.hdr;
+	reply.len = sizeof(uint32_t);


sizeof(status)?

LaurentiuM1234 · 2025-10-01T09:33:11Z

drivers/firmware/scmi/nxp/bbm.c

+
+	msg.hdr = SCMI_MESSAGE_HDR_MAKE(SCMI_PROTO_BBM_PROTOCOL_BUTTON_EVENT, SCMI_NOTIFICATION,
+			proto->id, scmi_p2a_header_token++);
+	msg.len = sizeof(uint32_t);


sizeof(flags)?

LaurentiuM1234 · 2025-10-01T11:24:07Z

drivers/firmware/scmi/nxp/bbm.c

 #include <zephyr/drivers/firmware/scmi/shmem.h>
 #include <string.h>
 #include <zephyr/kernel.h>
+#include <zephyr/logging/log.h>


I don't get the point of incrementally introducing these changes to the BBM driver? this is a new protocol so might as well have everything introduced in a single commit?

put bbm driver as only single commit now, keep its readability of the code logic

LaurentiuM1234 · 2025-10-01T11:27:34Z

drivers/firmware/scmi/core.c

 }

-static void scmi_core_reply_cb(struct scmi_channel *chan, struct scmi_message *msg)
+static void scmi_core_reply_cb(struct scmi_channel *chan, struct scmi_message msg)


this callback is meant for TX replies yet you're using it for notifications? I also don't understand the point of the second parameter and why it was randomly changed from a pointer to a struct scmi_message to just struct scmi_message in this commit?

yes, put tx/rx reply_cb into one scmi_core_reply_cb in core.c, but tx don't need another flow, just check its msg_type, its mainly deal with rx notification event, dispacher and send it into specific protocol

LaurentiuM1234 · 2025-10-01T12:38:04Z

drivers/firmware/scmi/core.c

 #define SCMI_CHAN_LOCK_TIMEOUT_USEC 500
 #define SCMI_CHAN_SEM_TIMEOUT_USEC 500

+static sys_slist_t scmi_event_list;


Do we really need a linked list? Can we not have each protocol keep an array of struct scmi_protocol_event for all registered events?

The definition of struct scmi_protocol_event would be:

struct scmi_protocol_event { uint32_t id; /* event ID */ scmi_evt_callback_t cb; /* event callback */ };

removed list, added events_ptr into DT_SCMI_PROTOCOL_DEFINE_NODEV now, thanks

LaurentiuM1234 · 2025-10-01T12:43:02Z

drivers/firmware/scmi/nxp/bbm.c

+
+static int scmi_nxp_bbm_event_init(void)
+{
+	return scmi_register_protocol_event_handler(&bbm_event);


do we need to also unregister protocol events? if not, then we can probably get around with binding the event array to the protocol structure at build time.

sure, updated it

LaurentiuM1234 · 2025-10-01T12:49:25Z

dts/bindings/firmware/arm,scmi.yaml

 properties:
  shmem:
-    type: phandle
+    type: phandle-array


can this not be phandles type?

thanks, its fine, corrected

yongxu-wang15 · 2025-10-13T10:48:17Z

Updated from LaurentiuM comments:
Hi @LaurentiuM1234 thanks your comments, I have redistributed the commit content and sequence of this PR to ensure the accuracy of the commit information and changes. I have also conducted a local hello_world test to guarantee that each commit does not affect the compilation of the platform, please help review it again when your time free, thanks

SCMI P2A (Platform to Agent) communication is critical in platform-agent interactions. Previously, only A2P (Agent to Platform) was supported. This patch introduces the P2A workflow by adding support for `scmi_channel->rx` and `scmi_mbox_channel->rx`, treating the RX channel as an independent channel. For P2A RX channels, initialization is optional and depends on platform design. If both the protocol node and the base transport node do not specify an RX shmem region, the RX channel will not be initialized. Signed-off-by: Yongxu Wang <[email protected]>

Introduce a dedicated RX callback at the mailbox transport layer so TX and RX are dispatched via separate callbacks. Signed-off-by: Yongxu Wang <[email protected]>

Add scmi_shmem_clear_channel_status to clear channel status which diff a2p and p2a direction Signed-off-by: Yongxu Wang <[email protected]>

In the SCMI core layer, add msg_type validation before dispatch (e.g. RESP vs NOTIFY vs CMD). Messages with an unexpected or invalid type for the current context are logged Signed-off-by: Yongxu Wang <[email protected]>

Add support for protocol-level registration of notification events during static registration. Each protocol declares the notifications it needs at static init time. In the SCMI core layer, upon receiving a P2A notification, the core iterates over all registered protocols to locate the matching event and dispatches it to the appropriate protocol handler. Signed-off-by: Yongxu Wang <[email protected]>

Introduce scmi_read_message() to allow protocols to read the associated event payload when a P2A notification is received. This enables protocol drivers to process notification-specific data after the SCMI core dispatches the event. Signed-off-by: Yongxu Wang <[email protected]>

This protocol is intended provide access to the battery-backed module, it contains persistent storage (GPR), an RTC, and the ON/OFF button. The protocol can also provide access to similar functions implemented via external board components. Added button P2A notification event for the BBM protocol. Signed-off-by: Yongxu Wang <[email protected]>

Extends the SCMI shared memory structure to phandles, allowing TX and RX channels to use different shared memory regions. This is necessary for platforms like i.MX95, where TX and RX notifications are mapped to different addresses. RX channel is not mandatory and depends on the platform's support Signed-off-by: Yongxu Wang <[email protected]>

Add scmi rx shmem and bbm node into imx95 m7 Signed-off-by: Yongxu Wang <[email protected]>

enable scmi bbm button function, m7 can get notification from scmi platform when press onoff button Signed-off-by: Yongxu Wang <[email protected]>

sonarqubecloud · 2025-10-14T02:54:58Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

dbaluta · 2025-10-14T07:39:37Z

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

 /**
- * @brief Get a reference to a protocol's SCMI TX channel
+ * @brief Get a reference to a protocol's SCMI TX/RX channel
 *


Dont merge the comments like this. Add a similar comment for RX channel.

dbaluta · 2025-10-14T07:40:07Z

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

 /**
- * @brief Declare a TX SCMI channel
+ * @brief Declare TX/RX SCMI channel
 *


Don't merge commments like this. Add a similar comment at the top of RX Channel declaration.

dbaluta · 2025-10-14T07:40:58Z

drivers/firmware/scmi/mailbox.c

 #include <zephyr/logging/log.h>
 #include "mailbox.h"

 LOG_MODULE_REGISTER(scmi_mbox);


s/callbak/callback/ in the commit subject.

dbaluta · 2025-10-14T07:45:39Z

drivers/firmware/scmi/mailbox.c

-			 void *user_data,
-			 struct mbox_msg *data)
+static void scmi_mbox_tx_reply_cb(const struct device *mbox,
+				mbox_channel_id_t channel_id,


Please split this into 2 patches:

Preparation for introducing the RX channel. Here you rename scmi_mbox_cb -> scmi_mbox_tx_reply_cb and implement scmi_mbox_cb with the logic for supporting tx/rx direction but only TX implemented so far.

Add the RX channel.

Much easier to review the transition like this.

dbaluta · 2025-10-14T07:50:07Z

drivers/firmware/scmi/core.c

+		status = scmi_core_handle_notification(hdr);
+		if (status) {
+			LOG_WRN("Scmi notification event not find");
+		}


s/not find/not found. Also, do you want to print the status here?

dbaluta · 2025-10-14T07:51:14Z

drivers/firmware/scmi/core.c

+		return -EINVAL;
+	}
+
+	ret = scmi_transport_read_message(proto->transport, proto->rx, msg);


No need for ret here. Judt directly return scmi_transport_read_message.

zephyrbot added platform: NXP MPU area: Architectures platform: NXP Drivers NXP Semiconductors, drivers labels Jul 18, 2025

zephyrbot requested review from JiafeiPan, Zhiqiang-Hou, dbaluta, dcpleung, decsny, dleach02, iuliana-prodan, manuargue, mmahadevan108, nashif and yangbolu1991 July 18, 2025 07:08

zephyrbot assigned dcpleung and nashif Jul 18, 2025

zephyrbot added the area: ARM64 ARM (64-bit) Architecture label Jul 18, 2025

zephyrbot requested review from SgrrZhf, carlocaione, npitre and povergoing July 18, 2025 07:39

yongxu-wang15 force-pushed the NXP-scmi-add-p2a-and-bbnsm-protocol branch from 8f29172 to e073dd8 Compare July 18, 2025 08:17

dbaluta reviewed Jul 21, 2025

View reviewed changes

drivers/firmware/scmi/core.c Show resolved Hide resolved

dbaluta reviewed Jul 21, 2025

View reviewed changes

yongxu-wang15 force-pushed the NXP-scmi-add-p2a-and-bbnsm-protocol branch from e073dd8 to ea7ed13 Compare July 22, 2025 08:33

LaurentiuM1234 requested changes Oct 1, 2025

View reviewed changes

yongxu-wang15 dismissed dbaluta’s stale review via 77defc5 October 13, 2025 10:28

yongxu-wang15 force-pushed the NXP-scmi-add-p2a-and-bbnsm-protocol branch from 158935d to 77defc5 Compare October 13, 2025 10:28

zephyrbot added area: Boards/SoCs platform: NXP NXP area: Devicetree Bindings area: Clock Control labels Oct 13, 2025

zephyrbot requested review from LaurentiuM1234, dleach02 and nordic-krch October 13, 2025 10:30

yongxu-wang15 force-pushed the NXP-scmi-add-p2a-and-bbnsm-protocol branch from 77defc5 to c76d77e Compare October 13, 2025 10:33

yongxu-wang15 added 10 commits October 14, 2025 10:14

drivers: firmware: scmi: Add RX callbak in mbox

749274f

Introduce a dedicated RX callback at the mailbox transport layer so TX and RX are dispatched via separate callbacks. Signed-off-by: Yongxu Wang <[email protected]>

drivers: firmware: scmi: add api to clear channel status

7ab7a84

Add scmi_shmem_clear_channel_status to clear channel status which diff a2p and p2a direction Signed-off-by: Yongxu Wang <[email protected]>

drivers: firmware: scmi: add msg_type validation in callback

a3233c5

In the SCMI core layer, add msg_type validation before dispatch (e.g. RESP vs NOTIFY vs CMD). Messages with an unexpected or invalid type for the current context are logged Signed-off-by: Yongxu Wang <[email protected]>

dts: arm: nxp: nxp_imx95_m7: add scmi rx shmem and bbm node

7fb6b89

Add scmi rx shmem and bbm node into imx95 m7 Signed-off-by: Yongxu Wang <[email protected]>

soc: nxp: imx95: enable scmi bbm button notification for m7 core

d876137

enable scmi bbm button function, m7 can get notification from scmi platform when press onoff button Signed-off-by: Yongxu Wang <[email protected]>

yongxu-wang15 force-pushed the NXP-scmi-add-p2a-and-bbnsm-protocol branch from c76d77e to d876137 Compare October 14, 2025 02:29

dbaluta reviewed Oct 14, 2025

View reviewed changes


		DT_SCMI_PROTOCOL_DEFINE_NODEV(DT_INST(0, nxp_scmi_bbm), NULL);

		extern uint32_t scmi_p2a_header_token;

NXP Add SCMI P2A workflow and BBNSM protocol #93305

Are you sure you want to change the base?

NXP Add SCMI P2A workflow and BBNSM protocol #93305

Conversation

yongxu-wang15 commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yongxu-wang15 commented Jul 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yongxu-wang15 Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yongxu-wang15 commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yongxu-wang15 commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LaurentiuM1234 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yongxu-wang15 commented Oct 13, 2025

Uh oh!

sonarqubecloud bot commented Oct 14, 2025

Quality Gate passed

Uh oh!

yongxu-wang15 commented Jul 18, 2025 •

edited

Loading

yongxu-wang15 Jul 22, 2025 •

edited

Loading

yongxu-wang15 commented Jul 22, 2025 •

edited

Loading

yongxu-wang15 commented Aug 11, 2025 •

edited

Loading