drivers: ipm: add ipm_rpi_pico #92923

dsseng · 2025-07-09T17:35:32Z

dts: bindings: ipm: add raspberrypi,pico-sio-fifo
drivers: ipm: add ipm_rpi_pico
dts: arm: rp2350: add SIO FIFO

drivers/ipm/ipm_rpi_pico.c

dts/bindings/ipm/raspberrypi,pico-sio-fifo.yaml

drivers/ipm/ipm_rpi_pico.c

dancollins · 2025-07-20T09:08:46Z

drivers/ipm/ipm_rpi_pico.c

+};
+static struct rpi_pico_ipm_data rpi_pico_mailbox_data;
+
+static int rpi_pico_mailbox_send(const struct device *dev, int wait, uint32_t id, const void *data,


I had to look at the docs for ipm_send() to figure out that int wait is (per the API) being used as a boolean. Strange!

dancollins · 2025-07-20T09:29:04Z

I haven't tested these changes, but they look good to me.

dsseng · 2025-07-20T09:38:28Z

Rebased AMP on top of this, it works

dsseng · 2025-07-27T19:51:28Z

@soburi could you please take another look when you have time? Thank you!

khoa-nguyen-18

Hi, your work is good. I have some comments. I'd appreciate if you could take a look.

drivers/ipm/ipm_rpi_pico.c

khoa-nguyen-18 · 2025-07-28T08:53:24Z

drivers/ipm/ipm_rpi_pico.c

+	const struct rpi_pico_mailbox_config *config =
+		(const struct rpi_pico_mailbox_config *)dev->config;
+
+	if (!(config->sio_regs->fifo_st & SIO_FIFO_ST_RDY_BITS) && !wait) {


Since the wait parameter is not supported in this function, should we return it like this

zephyr/drivers/ipm/ipm_sedi.c

Lines 133 to 136 in ba3662a

if (wait == 0) {

LOG_ERR("not support no wait mode when sending ipm msg");

return -ENOTSUP;

}

Is it unsupported on the API side? On my side (maybe I should refactor or comment if something is unclear) I do this:

If must not wait and no space in FIFO: EBUSY
Else: spin on FIFO becoming available

Could you please elaborate what is wrong with this model?

I think the wait parameter represents microseconds for k_busy_wait. I inferred this because it mentions busy-wait

zephyr/include/zephyr/drivers/ipm.h

Lines 129 to 133 in ba3662a

* @param wait If nonzero, busy-wait for remote to consume the message. The

* message is considered consumed once the remote interrupt handler

* finishes. If there is deferred processing on the remote side,

* or you would like to queue outgoing messages and wait on an

* event/semaphore, you can implement that in a high-level driver

and this driver implementation seems to follow that approach.

zephyr/drivers/ipm/ipm_esp32.c

Lines 121 to 136 in ba3662a

while (!atomic_cas(&dev_data->control->lock,

ESP32_IPM_LOCK_FREE_VAL,

dev_data->this_core_id)) {

k_busy_wait(1);

if ((wait != -1) && (wait > 0)) {

/* lock could not be held this time, return */

wait--;

if (wait == 0) {

irq_unlock(key);

return -ETIMEDOUT;

}

}

}

Not sure, because there are at least 2 examples where it is used as a bool, not time duration:

zephyr/drivers/ipm/ipm_imx.c

Line 187 in 5ac8c48

if (wait) {

zephyr/drivers/ipm/ipm_imx.c

Line 215 in 5ac8c48

if (wait) {

zephyr/tests/drivers/ipm/src/ipm_dummy.c

Lines 75 to 79 in 5ac8c48

if (wait) {

while (driver_data->regs.busy) {

/* busy-wait */

}

}

@dcpleung could you please elaborate on this API aspect?

wait acts as a boolean and is NOT the time to wait.

Hm, it turns out we have no adequate means to check if TX FIFO is empty. We can know if TX FIFO is full or not, and if we tried to push to it while it was full. What could be the options here?

Should I implement something like an ACK the opposite side would return when it has consumed the message, and when we receive that ACK we set a readiness flag/sync object?

@dcpleung could you please suggest the correct implementation of IPM API for the device? Should it maybe not support wait or even be converted to MBOX?

An ACK mechanism should work. The driver can be made to never support waiting. As the doxygen said, you can implement such waiting on higher lever driver. You can then implement the ACK mechanism there. As long as it is documented correctly, it should be okay.

Used ARG_UNUSED(wait); just like many examples in the subsys

drivers/ipm/ipm_rpi_pico.c

soburi · 2025-08-03T11:38:34Z

dts/arm/raspberrypi/rpi_pico/rp2350.dtsi

 			status = "disabled";
 		};
+
+		ipm0: sio-fifo@d0000000 {


Other drivers cannot use the address range written here.
In other words, if you create a driver that uses other SIO addresses,
an address overlap will occur, and a DTS compilation error will occur.
It is best to limit the address range to that used by the FIFO.
(In fact, pico-sdk functions can escape this restriction. But expecting it is bad design.)

This makes sense. SIO also features spinlocks, doorbells, timer and other interesting parts. So perhaps this should just cover the FIFO register ranges? Will handle that

Yes. Otherwise, it would be impossible to coexist with drivers for other functions.
In fact, the pico-sdk implementation doesn't take DTS into account, so it's just a formal check.

How would I instantiate a sio_hw_t struct though? Shall I just subtract 0x50 from the address? Seems quite a bit hacky. Another option: just ignore the address from the DT, use a HAL-provided instance and assert on address other than SIO_BASE+SIO_FIFO_ST_OFFSET, will probably go this way for now to avoid overabstraction

I think subtracting the offset from the dts value is fine, but
I'm not particularly picky as long as the symbol name is clear.

dsseng · 2025-08-04T16:11:01Z

drivers/ipm/ipm_rpi_pico.c

+	/*
+	 * FIFO mailbox allows a single 32 bit value to be sent - and we
+	 * use that as the channel identifier.
+	 */
+	return 0;


Could it maybe make sense to do some kind of a split, like 1/3 or 2/2 byte split for ID/Data? 2^32 seems like quite a huge number of channels for most applications, while sending some data as well might be a cool feature for debugging or if any IPM/MBOX consumers want to transfer some data as well

dts/bindings/ipm/raspberrypi,pico-sio-fifo.yaml

This device uses SIO block of RP2 chips to pass messages between cores. Signed-off-by: Dmitrii Sharshakov <[email protected]>

This commit introduces an interprocess mailbox driver for the RP2 family. Tested on RP2350, but should be compatible with RP2040 as well. For IPM communication, the driver sends ID as a 32-bit value through the SIO FIFO. The data size is always 0. Co-authored-by: Dan Collins <[email protected]> Signed-off-by: Dan Collins <[email protected]> Signed-off-by: Dmitrii Sharshakov <[email protected]>

SIO block has a pair of FIFO queues for inter-processor communication. Signed-off-by: Dmitrii Sharshakov <[email protected]>

sonarqubecloud · 2025-08-17T14:37:23Z

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

soburi · 2025-08-18T10:59:27Z

dts/arm/raspberrypi/rpi_pico/rp2350.dtsi

+		ipm0: sio-fifo@d0000050 {
+			compatible = "raspberrypi,pico-sio-fifo";
+			reg = <0xd0000050 0xc>;
+			reg-names = "sio";


"sio_fifo" is better here.

Correct, thanks. Now this range only covers 3 FIFO registers

soburi · 2025-08-18T16:06:38Z

drivers/ipm/ipm_rpi_pico.c

+		k_busy_wait(1);
+	}
+
+	sio_hw->fifo_wr = id;


Is it okay not to use spinlock for exclusion here?

Hm, not sure, it could cause an issue when there're 3 items in the mailbox, we checked in 2 threads, one pushed and another is yet to push, which will discard the message. This fn has to be wrapped in some thread safety method, yes, thanks

github-actions · 2025-10-18T00:34:19Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

github-actions bot added platform: Raspberry Pi Pico Raspberry Pi Pico (RPi Pico) area: IPM Inter-Processor Mailbox labels Jul 9, 2025

github-actions bot requested review from ThreeEights, ajf58, dcpleung, soburi and yonsch July 9, 2025 17:36

github-actions bot assigned soburi Jul 9, 2025

dsseng commented Jul 9, 2025

View reviewed changes

drivers/ipm/ipm_rpi_pico.c Show resolved Hide resolved

dsseng force-pushed the ipm-rp2350 branch from b78fdae to c948791 Compare July 9, 2025 18:19

soburi reviewed Jul 14, 2025

View reviewed changes

dts/bindings/ipm/raspberrypi,pico-sio-fifo.yaml Outdated Show resolved Hide resolved

drivers/ipm/ipm_rpi_pico.c Outdated Show resolved Hide resolved

dsseng force-pushed the ipm-rp2350 branch from c948791 to 163b19c Compare July 19, 2025 20:44

dsseng requested a review from soburi July 19, 2025 20:45

dsseng force-pushed the ipm-rp2350 branch 2 times, most recently from 273b297 to 3d77304 Compare July 20, 2025 08:43

dancollins reviewed Jul 20, 2025

View reviewed changes

dsseng force-pushed the ipm-rp2350 branch 2 times, most recently from 62e7f37 to 5aad599 Compare July 20, 2025 12:21

dsseng mentioned this pull request Jul 23, 2025

TaskList for Supporting RaspberryPi Pico peripherals #53810

Open

57 tasks

khoa-nguyen-18 reviewed Jul 28, 2025

View reviewed changes

dsseng requested a review from khoa-nguyen-18 August 1, 2025 20:32

This was referenced Aug 2, 2025

ipm: clarify wait parameter interpretation #94020

Merged

drivers: ipm: esp32: properly handle send wait parameter #94021

Merged

dsseng commented Aug 2, 2025

View reviewed changes

drivers/ipm/ipm_rpi_pico.c Outdated Show resolved Hide resolved

soburi requested changes Aug 3, 2025

View reviewed changes

dsseng commented Aug 4, 2025

View reviewed changes

dsseng force-pushed the ipm-rp2350 branch from 5aad599 to c265ca7 Compare August 9, 2025 14:31

dsseng requested a review from soburi August 9, 2025 14:31

soburi mentioned this pull request Aug 14, 2025

driver: inter-processor mailbox for Raspberry Pi Pico #94502

Open

JarmouniA reviewed Aug 14, 2025

View reviewed changes

dts/bindings/ipm/raspberrypi,pico-sio-fifo.yaml Outdated Show resolved Hide resolved

dsseng force-pushed the ipm-rp2350 branch from c265ca7 to ccce489 Compare August 17, 2025 12:55

dsseng requested a review from JarmouniA August 17, 2025 12:57

dsseng added 3 commits August 17, 2025 16:12

dts: bindings: ipm: add raspberrypi,pico-sio-fifo

77b505c

This device uses SIO block of RP2 chips to pass messages between cores. Signed-off-by: Dmitrii Sharshakov <[email protected]>

dts: arm: rp2350: add SIO FIFO

09d388d

SIO block has a pair of FIFO queues for inter-processor communication. Signed-off-by: Dmitrii Sharshakov <[email protected]>

dsseng force-pushed the ipm-rp2350 branch from ccce489 to 09d388d Compare August 17, 2025 14:12

soburi reviewed Aug 18, 2025

View reviewed changes

github-actions bot added the Stale label Oct 18, 2025

soburi removed the Stale label Oct 18, 2025

	if (wait == 0) {
	LOG_ERR("not support no wait mode when sending ipm msg");
	return -ENOTSUP;
	}

	* @param wait If nonzero, busy-wait for remote to consume the message. The
	* message is considered consumed once the remote interrupt handler
	* finishes. If there is deferred processing on the remote side,
	* or you would like to queue outgoing messages and wait on an
	* event/semaphore, you can implement that in a high-level driver

	while (!atomic_cas(&dev_data->control->lock,
	ESP32_IPM_LOCK_FREE_VAL,
	dev_data->this_core_id)) {

	k_busy_wait(1);

	if ((wait != -1) && (wait > 0)) {
	/* lock could not be held this time, return */
	wait--;
	if (wait == 0) {
	irq_unlock(key);

	return -ETIMEDOUT;
	}
	}
	}

	if (wait) {
	while (driver_data->regs.busy) {
	/* busy-wait */
	}
	}

drivers: ipm: add ipm_rpi_pico #92923

Are you sure you want to change the base?

drivers: ipm: add ipm_rpi_pico #92923

Uh oh!

Conversation

dsseng commented Jul 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dancollins Jul 20, 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!

dancollins commented Jul 20, 2025

Uh oh!

dsseng commented Jul 20, 2025

Uh oh!

dsseng commented Jul 27, 2025

Uh oh!

khoa-nguyen-18 left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsseng Aug 9, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsseng Aug 4, 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!

Uh oh!

sonarqubecloud bot commented Aug 17, 2025

Quality Gate passed

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!

github-actions bot commented Oct 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

dancollins Jul 20, 2025 •

edited

Loading

dsseng Aug 9, 2025 •

edited

Loading

dsseng Aug 4, 2025 •

edited

Loading