Skip to content

Conversation

r-c-n
Copy link

@r-c-n r-c-n commented Aug 14, 2025

Add support for the inter-processor mailbox peripheral for the RP2040 and RP2350 SoCs. This is a requirement for enabling multi processing (asymmetric) for the Raspberry Pi Pico boards.

Tested on a Raspberry Pi Pico 2W (RP2350).

Copy link

Hello @r-c-n, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

@soburi
Copy link
Member

soburi commented Aug 14, 2025

Duplicated ? > #92923

@dsseng
Copy link
Member

dsseng commented Aug 14, 2025

Duplicated ? > #92923

No, this is not a dupe. Work of Dan and mine is an IPM subsys driver while this is mailbox. They should probably coexist just like on other platforms like nRF or STM32, and I thought about implementing mbox myself as well once I land the IPM. Wdyt?

I will try to review this change when I have time

@r-c-n
Copy link
Author

r-c-n commented Aug 14, 2025

Ugh I wasn't aware of #92923
@dsseng , doesn't this commit target the same peripheral I'm targetting here?
That PR seems to define the SIO block differently in the device tree but, apart from that, I'd say we're doing the same thing. I'd be happy to contribute, so feel free to pick parts of this PR, or push this one instead as a preliminary step.

@dsseng
Copy link
Member

dsseng commented Aug 14, 2025

Ugh I wasn't aware of #92923 @dsseng , doesn't this commit target the same peripheral I'm targetting here? That PR seems to define the SIO block differently in the device tree but, apart from that, I'd say we're doing the same thing. I'd be happy to contribute, so feel free to pick parts of this PR, or push this one instead as a preliminary step.

It is targeting the same device with a different API. Later I will provide examples, but e.g. ESP32 implements both options to be chosen by the application. So this PR is likely useful

@r-c-n r-c-n force-pushed the rcn-rpi-pico-mbox branch 2 times, most recently from 560bcf1 to 1de3c99 Compare August 15, 2025 08:31
Comment on lines +72 to +80
LOG_DBG("CPU %d: send IP data: %d", sio_hw->cpuid, *((int *)msg->data));
sio_hw->fifo_wr = *((uint32_t *)(msg->data));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably send the channel number here. Most consumers expect more than one channel, but do not send data.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case the mailbox has only one channel from the point of view of the core that sends the message, right? Actually, the concept of "channel" isn't used in the driver at all.

But yes, it still may be useful to add it to the debug message to know how the caller called the function.

Copy link
Member

@dsseng dsseng Aug 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant for consumers it'll probably be better if the mailbox provided more channels, rather than data transfer.

What are you using it for? I use IPM in my branch for IPC subsystem, and so I need more than one channel, but signaling only, no data (data resides in shared memory and access is controlled by two mailbox channels)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're talking about different things here...

What I did here is a direct mapping of the hardware functionality, with no further abstractions. This handles the SIO mailboxes. According to the docs, there are only two FIFOs (let's call them A and B), one core can only read from A and write to B, while the other can only read from B and write to A. From the point of view of each of the cores, there's only one single channel. Unless we mean different things when we say "channel" in this context.

When you say the data resides in shared memory and the access is controlled by the mailbox channels, I understand that's handled by the IPC mechanism you implemented on top of the mailboxes (ie. a region of memory shared between the cores and a signalling mechanism using the mailboxes). When I mention "data" in the driver, I mean the data that goes through the mailboxes. In the RP2350 each mailbox can hold 4 4-byte messages, for instance. You can ignore the data and use a mailbox for signalling events only, or you can use it to signal an event and carry some data with it (a memory address, for instance).

In the end, the use case will be inter-processor communication, but what I'm doing here is to provide low-level access to the hardware peripheral so that the IPM can be implemented on top of it. For example, using zephyr,mbox-imp as an adaptor.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, let's leave this to maintainers, because that's really use-case dependent

@r-c-n r-c-n force-pushed the rcn-rpi-pico-mbox branch from 1de3c99 to a27985f Compare August 18, 2025 09:13
@r-c-n
Copy link
Author

r-c-n commented Aug 19, 2025

@dsseng @dancollins as an additional test, I applied this on top of #93379 and made some changes to do the core-to-core handshake using this driver with the zephyr-ipm,mbox driver as an adaptor.

I tested it successfully on a Raspberry Pi Pico 2W. The build steps are the same that Dmitrii shared here with an additional config -DCONFIG_IPM=y for the firmware of core 0.

@dsseng
Copy link
Member

dsseng commented Aug 19, 2025

same that Dmitrii shared here with an additional config -DCONFIG_IPM=y for the firmware of core 0.

Why enable IPM if your driver is mailbox? Try samples/subsys/ipc/ipc_service/icmsg with the mbox. Also, booting CPU1 via IPM/MBOX does not feel right: the official samples boot it in event (wfe/sev) mode, not interrupt mode, we should not use drivers in the SoC init code as well, so the additional commit is not likely useful.

Could you please make samples/subsys/ipc/ipc_service/icmsg run with this PR on top of #93379? Thanks!

@r-c-n
Copy link
Author

r-c-n commented Aug 19, 2025

same that Dmitrii shared here with an additional config -DCONFIG_IPM=y for the firmware of core 0.

Why enable IPM if your driver is mailbox? Try samples/subsys/ipc/ipc_service/icmsg with the mbox. Also, booting CPU1 via IPM/MBOX does not feel right: the official samples boot it in event (wfe/sev) mode, not interrupt mode, we should not use drivers in the SoC init code as well, so the additional commit is not likely useful.

Could you please make samples/subsys/ipc/ipc_service/icmsg run with this PR on top of #93379? Thanks!

Thanks for reviewing.

The commit I pointed to was just a quick test for the driver, to show how it works with a simple use case.

Why enable IPM if your driver is mailbox?

Because the test code (soc.c) uses the zephyr IPM driver with this mbox driver as a backend.

Also, booting CPU1 via IPM/MBOX does not feel right

As I said before, we're most likely referring to different things when we say IPM/MBOX. I think. The SIO mailboxes is the only way to run core 1 afaict. At least according to the SoC datasheet (section 5.3). I agree that any higher-level IPM mechanisms written on top of that aren't necessary to boot CPU1. This driver does nothing of that, it's simply an implementation of the zephyr mbox interface to the hardware registers of the RP2040/2350

the official samples boot it in event (wfe/sev) mode, not interrupt mode

That's true, but is there a strict reason for that? If core 0 is ready to handle interrupts when it does the core 1 boot handshake, why would it be a problem to have the hardware work as intended? Unless there's a risk or a reliability issue in using interrupts at this stage, which I don't know.

we should not use drivers in the SoC init code as well

That could be a blocker. I don't know if the code in soc.c is supposed to be self-contained. The driver doesn't use any OS service besides enabling/disabling interrupts, though.

Could you please make samples/subsys/ipc/ipc_service/icmsg run with this PR on top of #93379? Thanks!

I'll check that out, I'm not familiar with that sample. Thanks for the tip.

r-c-n added 2 commits August 19, 2025 15:18
This adds a driver for the interprocessor mailbox in the Raspberry Pi
Pico SoCs (RP2040/RP2350).

The SIO subsystem contains a set of low-latency peripherals, including a
pair of mailboxes (FIFOs) for inter-processor communication. One of the
FIFOs can only be written by core 0 and read by core 1, the other can
only be written by core 1 and read by core 0.

According to the datasheets [1] [2], the register interface is the
same in both SoCs and the only functional differences are that the
mailbox in the RP2040 is 8 entries deep vs 4 in the RP2350, and that the
RP2350 defines the same core-specific interrupt number, while the RP2040
defines two different IRQ numbers.

Tested on a Raspberry Pi Pico 2W (RP2350).

[1]: https://datasheets.raspberrypi.com/rp2350/rp2350-datasheet.pdf
[2]: https://datasheets.raspberrypi.com/rp2040/rp2040-datasheet.pdf

Signed-off-by: Ricardo Cañuelo Navarro <[email protected]>
Add a mailbox (Interprocessor FIFO) node to the RP2040 and RP2350 device
trees and a SIO block to hold this and any other future SIO peripherals.

This also includes an additional `zephyr,mbox-ipm` node that handles the
mailbox for core to core messaging.

The device is enabled for the Raspberry Pico Pi 2.

Signed-off-by: Ricardo Cañuelo Navarro <[email protected]>
@r-c-n r-c-n force-pushed the rcn-rpi-pico-mbox branch from a27985f to 31c9e2c Compare August 19, 2025 13:19
@dsseng
Copy link
Member

dsseng commented Aug 19, 2025

That could be a blocker. I don't know if the code in soc.c is supposed to be self-contained. The driver doesn't use any OS service besides enabling/disabling interrupts, though.

I don't know for sure, but usually the SoC code tries to use nothing but HAL + maybe CMSIS.

That's true, but is there a strict reason for that? If core 0 is ready to handle interrupts when it does the core 1 boot handshake, why would it be a problem to have the hardware work as intended? Unless there's a risk or a reliability issue in using interrupts at this stage, which I don't know.

No idea, Dan and I replicated the known good examples (which was the easiest and also makes soc.c self-contained making it upstreamable as a separate bit). https://github.com/raspberrypi/pico-bootrom-rp2350 could give you some insight into what happens at the reset vector for each core.

Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: mbox platform: Raspberry Pi Pico Raspberry Pi Pico (RPi Pico)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants