I think this is a better way to handle the RP register port layout. Avoids the extra indirection through a table of pointers.
I left the masking in, but I think it's not necessary. I can't see any reason it's needed, except to deal with code that is trying to randomly write to GPIOs and manages to hit the SPI or USB bits. But such code is buggy and should be fixed. And will still crash if it tries a random GPIO > 63 and goes off the end of the port table. And writing to random valid non-SPI/USB GPIOs can cause all sorts of problems too. So masking the bits doesn't really fix anything.
Besides making GPIO operations more costly, there are other downsides. It makes it impossible to use the USB pins as GPIOs, which isn't common but is a perfectly reasonable thing to do if you know what you are doing. One might say, "it's possible to write to the SIO registers directly and do it that way." But then there's another problem. Any code that does a read-modify-write to change only certain GPIO lines in the high register will also mask off the SPI/USB bits and set them to zero! So even if you set those bits directly via SIO->..., perfectly well written code elsewhere that is trying to NOT clobber bits it doesn't want to modify will still clear them out.
Code: Select all
From 6190bea228f19d09932c52e856da2f8bce19a681 Mon Sep 17 00:00:00 2001
From: Trent Piepho <[email protected]>
Date: Sun, 5 Apr 2026 14:43:58 -0700
Subject: [PATCH] More efficient RP GPIO port code
Instead of going through an extra indirection table to get the correct
register address for the low and hi port, take advantage of the offset
between the low and hi registers being the same for every register.
On the RP2350, every hi register immediately follows the low register.
While on the RP2040, they are offset by eight registers.
---
os/hal/ports/RP/LLD/GPIOv1/hal_pal_lld.c | 18 ++----------------
os/hal/ports/RP/LLD/GPIOv1/hal_pal_lld.h | 12 ++++++------
os/hal/ports/RP/RP2040/rp_pal_lld.h | 10 ++++++++++
os/hal/ports/RP/RP2350/rp_pal_lld.h | 10 ++++++++++
4 files changed, 28 insertions(+), 22 deletions(-)
diff --git a/os/hal/ports/RP/LLD/GPIOv1/hal_pal_lld.c b/os/hal/ports/RP/LLD/GPIOv1/hal_pal_lld.c
index 7741a0e8f3..8d0a14f2dd 100644
--- a/os/hal/ports/RP/LLD/GPIOv1/hal_pal_lld.c
+++ b/os/hal/ports/RP/LLD/GPIOv1/hal_pal_lld.c
@@ -52,24 +52,10 @@
const rp_pal_port_t _pal_ports[RP_PAL_IOPORTS_COUNT] = {
{
- .in = &SIO->GPIO_IN,
- .out = &SIO->GPIO_OUT,
- .out_set = &SIO->GPIO_OUT_SET,
- .out_clr = &SIO->GPIO_OUT_CLR,
- .out_xor = &SIO->GPIO_OUT_XOR,
- .oe_set = &SIO->GPIO_OE_SET,
- .oe_clr = &SIO->GPIO_OE_CLR,
.valid_mask = RP_PAL_PORT0_VALID_MASK,
},
#if RP_GPIO_NUM_LINES > 32U
{
- .in = &SIO->GPIO_HI_IN,
- .out = &SIO->GPIO_HI_OUT,
- .out_set = &SIO->GPIO_HI_OUT_SET,
- .out_clr = &SIO->GPIO_HI_OUT_CLR,
- .out_xor = &SIO->GPIO_HI_OUT_XOR,
- .oe_set = &SIO->GPIO_HI_OE_SET,
- .oe_clr = &SIO->GPIO_HI_OE_CLR,
.valid_mask = RP_PAL_PORT1_VALID_MASK,
},
#endif
@@ -119,13 +105,13 @@ static void rp_pal_pad_set_mode(ioportid_t port,
}
/* Release the output driver while reprogramming mux and pad control. */
- *rpp->oe_clr = bit;
+ _SIO_REG_PORT(GPIO_OE_CLR, port) = bit;
IO_BANK0->GPIO[abspad].CTRL = ctrlbits;
PADS_BANK0->GPIO[abspad] = padbits;
if (oebits != 0U) {
- *rpp->oe_set = bit;
+ _SIO_REG_PORT(GPIO_OE_SET, port) = bit;
}
}
diff --git a/os/hal/ports/RP/LLD/GPIOv1/hal_pal_lld.h b/os/hal/ports/RP/LLD/GPIOv1/hal_pal_lld.h
index 47350faea3..70ece86a10 100644
--- a/os/hal/ports/RP/LLD/GPIOv1/hal_pal_lld.h
+++ b/os/hal/ports/RP/LLD/GPIOv1/hal_pal_lld.h
@@ -329,7 +329,7 @@ extern const rp_pal_port_t _pal_ports[RP_PAL_IOPORTS_COUNT];
* @notapi
*/
#define pal_lld_readport(port) \
- ((ioportmask_t)(*_pal_ports[(uint32_t)(port)].in) & \
+ (_SIO_REG_PORT(GPIO_IN, (port)) & \
(ioportmask_t)(_pal_ports[(uint32_t)(port)].valid_mask))
/**
@@ -343,7 +343,7 @@ extern const rp_pal_port_t _pal_ports[RP_PAL_IOPORTS_COUNT];
* @notapi
*/
#define pal_lld_readlatch(port) \
- ((ioportmask_t)(*_pal_ports[(uint32_t)(port)].out) & \
+ (_SIO_REG_PORT(GPIO_OUT, (port)) & \
(ioportmask_t)(_pal_ports[(uint32_t)(port)].valid_mask))
/**
@@ -363,7 +363,7 @@ extern const rp_pal_port_t _pal_ports[RP_PAL_IOPORTS_COUNT];
*/
#define pal_lld_writeport(port, bits) \
do { \
- *_pal_ports[(uint32_t)(port)].out = \
+ _SIO_REG_PORT(GPIO_OUT, (port)) = \
(uint32_t)(bits) & _pal_ports[(uint32_t)(port)].valid_mask; \
} while (false)
@@ -380,7 +380,7 @@ extern const rp_pal_port_t _pal_ports[RP_PAL_IOPORTS_COUNT];
*/
#define pal_lld_setport(port, bits) \
do { \
- *_pal_ports[(uint32_t)(port)].out_set = \
+ _SIO_REG_PORT(GPIO_OUT_SET, (port)) = \
(uint32_t)(bits) & _pal_ports[(uint32_t)(port)].valid_mask; \
} while (false)
@@ -397,7 +397,7 @@ extern const rp_pal_port_t _pal_ports[RP_PAL_IOPORTS_COUNT];
*/
#define pal_lld_clearport(port, bits) \
do { \
- *_pal_ports[(uint32_t)(port)].out_clr = \
+ _SIO_REG_PORT(GPIO_OUT_CLR, (port)) = \
(uint32_t)(bits) & _pal_ports[(uint32_t)(port)].valid_mask; \
} while (false)
@@ -414,7 +414,7 @@ extern const rp_pal_port_t _pal_ports[RP_PAL_IOPORTS_COUNT];
*/
#define pal_lld_toggleport(port, bits) \
do { \
- *_pal_ports[(uint32_t)(port)].out_xor = \
+ _SIO_REG_PORT(GPIO_OUT_XOR, (port)) = \
(uint32_t)(bits) & _pal_ports[(uint32_t)(port)].valid_mask; \
} while (false)
diff --git a/os/hal/ports/RP/RP2040/rp_pal_lld.h b/os/hal/ports/RP/RP2040/rp_pal_lld.h
index 3001f402ee..3d605e9e7f 100644
--- a/os/hal/ports/RP/RP2040/rp_pal_lld.h
+++ b/os/hal/ports/RP/RP2040/rp_pal_lld.h
@@ -37,4 +37,14 @@
PAL_RP_PAD_SCHMITT)
/** @} */
+/**
+ * @brief Access a SIO GPIO register by port.
+ * @details Given a register name (field of SIO) and port (0, 1), produce a
+ * reference to the correct GPIO register for the port. On the RP2040
+ * every "hi" register is eight registers after the "low" register.
+ *
+ * @notapi
+ */
+#define _SIO_REG_PORT(reg, port) *(&SIO->reg + ((port) ? 8 : 0))
+
#endif /* RP2040_RP_PAL_LLD_H */
diff --git a/os/hal/ports/RP/RP2350/rp_pal_lld.h b/os/hal/ports/RP/RP2350/rp_pal_lld.h
index 9c6d42b3bf..8ce27bc5af 100644
--- a/os/hal/ports/RP/RP2350/rp_pal_lld.h
+++ b/os/hal/ports/RP/RP2350/rp_pal_lld.h
@@ -43,4 +43,14 @@
PAL_RP_PAD_SCHMITT)
/** @} */
+/**
+ * @brief Access a SIO GPIO register by port.
+ * @details Given a register name (field of SIO) and port (0, 1), produce a
+ * reference to the correct GPIO register for the port. On the RP2350
+ * every "hi" register is immediately after the "low" register.
+ *
+ * @notapi
+ */
+#define _SIO_REG_PORT(reg, port) *(&SIO->reg + (port))
+
#endif /* RP2350_RP_PAL_LLD_H */
--
2.49.0