Add gpio_init_mask64 function #2907

2026-04-23T02:26:06-05:00

FLyrfors-MI commented

2026-04-23 02:26:06 -05:00

(Migrated from github.com)

Add gpio_init_mask64 function mirroring the other mask64/masked64 GPIO functions used by RP2350B with more than 32 GPIO's.

See #2643

Add `gpio_init_mask64` function mirroring the other `mask64`/`masked64` GPIO functions used by RP2350B with more than 32 GPIO's. See #2643

lurch (Migrated from github.com) reviewed

2026-04-23 03:15:22 -05:00

src/rp2_common/hardware_gpio/include/hardware/gpio.h

					
				@ -41,23 +41,23 @@ extern "C" {

				 * \brief General Purpose Input/Output (GPIO) API

 * The number of GPIO pins available depends on the package. There are 30 user GPIOs in bank 0 in the QFN-60 package (RP2350A), or 48 user GPIOs
 * in the QFN-80 package (RP2350B). Bank 1 contains the 6 QSPI pins and the USB DP/DM pins.

```suggestion * The number of GPIO pins available depends on the package. There are 30 user GPIOs in bank 0 in the QFN-60 package (RP2350A), or 48 user GPIOs * in the QFN-80 package (RP2350B). Bank 1 contains the 6 QSPI pins and the USB DP/DM pins. ```

lurch (Migrated from github.com) reviewed

2026-04-23 03:17:15 -05:00

src/rp2_common/hardware_gpio/gpio.c

					
				@ -281,29 +281,38 @@ void gpio_deinit(uint gpio) {

				    gpio_set_function(gpio, GPIO_FUNC_NULL);

gpio_set_function_masked does

for (uint i = 0; i < MIN(NUM_BANK0_GPIOS, 32u); i++) {

Probably makes sense to do the same here?

`gpio_set_function_masked` does ```c for (uint i = 0; i < MIN(NUM_BANK0_GPIOS, 32u); i++) { ``` Probably makes sense to do the same here?

lurch (Migrated from github.com) reviewed

2026-04-23 03:18:08 -05:00

src/rp2_common/hardware_gpio/gpio.c

					
				@ -281,29 +281,38 @@ void gpio_deinit(uint gpio) {

				    gpio_set_function(gpio, GPIO_FUNC_NULL);

Similarly, gpio_set_function_masked64 does

for (uint i = 0; i < MIN(NUM_BANK0_GPIOS, 64u); i++) {

Similarly, `gpio_set_function_masked64` does ```c for (uint i = 0; i < MIN(NUM_BANK0_GPIOS, 64u); i++) { ```

kilograham (Migrated from github.com) reviewed

2026-05-09 14:24:31 -05:00

src/rp2_common/hardware_gpio/gpio.c

					
				@ -281,29 +281,38 @@ void gpio_deinit(uint gpio) {

				    gpio_set_function(gpio, GPIO_FUNC_NULL);

i reimplemented

kilograham (Migrated from github.com) reviewed

2026-05-09 14:24:37 -05:00

src/rp2_common/hardware_gpio/gpio.c

					
				@ -281,29 +281,38 @@ void gpio_deinit(uint gpio) {

				    gpio_set_function(gpio, GPIO_FUNC_NULL);

i reimplemented

lurch (Migrated from github.com) reviewed

2026-05-09 15:28:50 -05:00

src/rp2_common/hardware_gpio/gpio.c

					
				@ -281,29 +281,38 @@ void gpio_deinit(uint gpio) {

				    gpio_set_function(gpio, GPIO_FUNC_NULL);

It's only a small optimisation, but is it worth doing:

void gpio_set_function_masked(uint32_t gpio_mask, gpio_function_t fn) {
    if (gpio_mask) {
        for (uint i = 0; i < NUM_BANK0_GPIOS; i++) {
            if (gpio_mask & 1u) {
                gpio_set_function(i, fn);
            }
            gpio_mask >>= 1;
            if (!gpio_mask) break;
        }
    }
}

(or is it better to assume that this function will never be called with an empty gpio_mask?)

It's only a small optimisation, but is it worth doing: ```c void gpio_set_function_masked(uint32_t gpio_mask, gpio_function_t fn) { if (gpio_mask) { for (uint i = 0; i < NUM_BANK0_GPIOS; i++) { if (gpio_mask & 1u) { gpio_set_function(i, fn); } gpio_mask >>= 1; if (!gpio_mask) break; } } } ``` (or is it better to assume that this function will never be called with an empty `gpio_mask`?)

will-v-pi (Migrated from github.com) reviewed

2026-05-11 06:55:13 -05:00

src/rp2_common/hardware_gpio/gpio.c

					
				@ -281,29 +281,38 @@ void gpio_deinit(uint gpio) {

				    gpio_set_function(gpio, GPIO_FUNC_NULL);

Is it worth just doing this, rather than duplicating the code?

#if NUM_BANK0_GPIOS <= 32
    gpio_init_mask((uint32_t)gpio_mask);
#else

Or even just a define?

#define gpio_init_mask64(gpio_mask) gpio_init_mask((uint32_t)gpio_mask)

Is it worth just doing this, rather than duplicating the code? ```suggestion #if NUM_BANK0_GPIOS <= 32 gpio_init_mask((uint32_t)gpio_mask); #else ``` Or even just a define? ```c #define gpio_init_mask64(gpio_mask) gpio_init_mask((uint32_t)gpio_mask) ```

kilograham (Migrated from github.com) reviewed

2026-05-11 12:41:52 -05:00

src/rp2_common/hardware_gpio/gpio.c

					
				@ -281,29 +281,38 @@ void gpio_deinit(uint gpio) {

				    gpio_set_function(gpio, GPIO_FUNC_NULL);

it is an unlikely parameter and the code still works, so we don't want to increase code size

kilograham (Migrated from github.com) reviewed

2026-05-11 12:44:22 -05:00

src/rp2_common/hardware_gpio/gpio.c

					
				@ -281,29 +281,38 @@ void gpio_deinit(uint gpio) {

				    gpio_set_function(gpio, GPIO_FUNC_NULL);

yeah probably cleaner to #ifdef it in the header - though some people may be sad that the function appears/disappears

will-v-pi (Migrated from github.com) reviewed

2026-05-11 12:49:18 -05:00

src/rp2_common/hardware_gpio/gpio.c

					
				@ -281,29 +281,38 @@ void gpio_deinit(uint gpio) {

				    gpio_set_function(gpio, GPIO_FUNC_NULL);

Maybe #ifdef it as an inline function in the header, so code using mask64 still compiles, and you can still do type checking? I agree having the mask64 functions disappear entirely would be bad, for anyone writing more universal code.

kilograham (Migrated from github.com) reviewed

2026-05-12 16:02:20 -05:00

src/rp2_common/hardware_gpio/gpio.c

					
				@ -281,29 +281,38 @@ void gpio_deinit(uint gpio) {

				    gpio_set_function(gpio, GPIO_FUNC_NULL);

yeah that's what i'm suggesting - some people may get confused/bitten by the fact that sometimes the outline function exists and sometimes it doesn't too, maybe I'll allow it with a #undef in the gpio.c

kilograham (Migrated from github.com) reviewed

2026-05-12 16:13:28 -05:00

src/rp2_common/hardware_gpio/gpio.c

					
				@ -281,29 +281,38 @@ void gpio_deinit(uint gpio) {

				    gpio_set_function(gpio, GPIO_FUNC_NULL);

actually i changed my mind; #defining one function to another makes the doxygen confusing, so using static inline instead

lurch (Migrated from github.com) reviewed

2026-05-13 04:27:56 -05:00

src/rp2_common/hardware_gpio/gpio.c

					
				@ -281,29 +281,38 @@ void gpio_deinit(uint gpio) {

				    gpio_set_function(gpio, GPIO_FUNC_NULL);

The condition in the header file is #if NUM_BANK0_GPIOS <= 32 so this line ought to be

#if NUM_BANK0_GPIOS > 32

?

The condition in the header file is `#if NUM_BANK0_GPIOS <= 32` so this line ought to be ```suggestion #if NUM_BANK0_GPIOS > 32 ``` ?

lurch (Migrated from github.com) reviewed

2026-05-13 05:52:02 -05:00

src/rp2_common/hardware_gpio/gpio.c

					
				@ -281,29 +281,38 @@ void gpio_deinit(uint gpio) {

				    gpio_set_function(gpio, GPIO_FUNC_NULL);

Hmmm...
The implementation of gpio_init_mask (and also gpio_init_mask64) simply calls gpio_init in a loop. But gpio_init does:

void gpio_init(uint gpio) {
    gpio_set_dir(gpio, GPIO_IN);
    gpio_put(gpio, 0);
    gpio_set_function(gpio, GPIO_FUNC_SIO);
}

and gpio_set_dir calls gpio_set_dir_in_masked, and gpio_put calls gpio_clr_mask.

So maybe it'd be more efficient if gpio_init_mask was changed to:

void gpio_init_mask(uint32_t gpio_mask) {
    gpio_set_dir_in_masked(gpio_mask);
    gpio_clr_mask(gpio_mask);
    gpio_set_function_masked(gpio_mask, GPIO_FUNC_SIO);
}

? 🤔

Hmmm... The implementation of `gpio_init_mask` (and also `gpio_init_mask64`) simply calls `gpio_init` in a loop. But `gpio_init` does: ```c void gpio_init(uint gpio) { gpio_set_dir(gpio, GPIO_IN); gpio_put(gpio, 0); gpio_set_function(gpio, GPIO_FUNC_SIO); } ``` and `gpio_set_dir` calls `gpio_set_dir_in_masked`, and `gpio_put` calls `gpio_clr_mask`. So maybe it'd be more efficient if `gpio_init_mask` was changed to: ```c void gpio_init_mask(uint32_t gpio_mask) { gpio_set_dir_in_masked(gpio_mask); gpio_clr_mask(gpio_mask); gpio_set_function_masked(gpio_mask, GPIO_FUNC_SIO); } ``` ? :thinking:

will-v-pi (Migrated from github.com) reviewed

2026-05-13 09:32:05 -05:00

src/rp2_common/hardware_gpio/gpio.c

					
				@ -281,29 +281,38 @@ void gpio_deinit(uint gpio) {

				    gpio_set_function(gpio, GPIO_FUNC_NULL);

Yeah, I agree with this - it's presumably a large speedup, given the set_dir_in and clr_mask can be single coprocessor instructions

Yeah, I agree with this - it's presumably a large speedup, given the `set_dir_in` and `clr_mask` can be single coprocessor instructions

kilograham (Migrated from github.com) reviewed

2026-05-13 10:59:43 -05:00

src/rp2_common/hardware_gpio/gpio.c

					
				@ -281,29 +281,38 @@ void gpio_deinit(uint gpio) {

				    gpio_set_function(gpio, GPIO_FUNC_NULL);

nice good catch - presumably applies to 64 bit variant too

will-v-pi (Migrated from github.com) reviewed

2026-05-13 11:10:51 -05:00

src/rp2_common/hardware_gpio/gpio.c

					
				@ -281,29 +281,38 @@ void gpio_deinit(uint gpio) {

				    gpio_set_function(gpio, GPIO_FUNC_NULL);

This comment probably belongs next to #if NUM_BANK0_GPIOS > 32 rather than here?

This comment probably belongs next to `#if NUM_BANK0_GPIOS > 32` rather than here?

lurch (Migrated from github.com) reviewed

2026-05-13 11:59:06 -05:00

src/rp2_common/hardware_gpio/gpio.c

					
				@ -281,29 +281,38 @@ void gpio_deinit(uint gpio) {

				    gpio_set_function(gpio, GPIO_FUNC_NULL);

IMHO this should be

    gpio_set_dir_in_masked(gpio_mask);

as per my original suggestion. Note that the second parameter to gpio_set_dir_masked isn't GPIO_IN/GPIO_OUT, but a per-gpio direction value. See https://www.raspberrypi.com/documentation/pico-sdk/hardware.html#group_hardware_gpio_1gaa64fdceff54dda03f4427f04c2f9ced2

IMHO this should be ```suggestion gpio_set_dir_in_masked(gpio_mask); ``` as per my original suggestion. Note that the second parameter to `gpio_set_dir_masked` isn't `GPIO_IN`/`GPIO_OUT`, but a per-gpio direction value. See https://www.raspberrypi.com/documentation/pico-sdk/hardware.html#group_hardware_gpio_1gaa64fdceff54dda03f4427f04c2f9ced2

kilograham (Migrated from github.com) reviewed

2026-05-13 15:56:28 -05:00

src/rp2_common/hardware_gpio/gpio.c

					
				@ -281,29 +281,38 @@ void gpio_deinit(uint gpio) {

				    gpio_set_function(gpio, GPIO_FUNC_NULL);

true dat

kilograham (Migrated from github.com) reviewed

2026-05-13 15:56:37 -05:00

src/rp2_common/hardware_gpio/gpio.c

					
				@ -281,29 +281,38 @@ void gpio_deinit(uint gpio) {

				    gpio_set_function(gpio, GPIO_FUNC_NULL);

si

lurch (Migrated from github.com) reviewed

2026-05-13 18:07:48 -05:00

src/rp2_common/hardware_gpio/gpio.c

Oops?

will-v-pi (Migrated from github.com) reviewed

2026-05-14 05:22:57 -05:00

will-v-pi (Migrated from github.com) left a comment

No description provided.

One possible improvement, but that can easily go in a separate PR if it's worth it, so approving

will-v-pi (Migrated from github.com) reviewed

2026-05-14 05:23:59 -05:00

src/rp2_common/hardware_gpio/gpio.c

```suggestion } ```

will-v-pi (Migrated from github.com) approved these changes

2026-05-14 05:25:10 -05:00

lurch (Migrated from github.com) reviewed

2026-05-14 08:02:46 -05:00

src/rp2_common/hardware_gpio/gpio.c

					
				@ -295,4 +286,4 @@

				        if (gpio_mask & 1u) {

				            gpio_set_function(i, fn);

				        }

				        gpio_mask >>= 1;

Thinking on this some more, I guess you could just iterate between the first-set-bit and last-set-bit by using both __builtin_ctz and __builtin_ltz, and then you wouldn't need the if (!gpio_mask) check inside the loop-body?
And then would

        if (gpio_mask & (1u << i)) {
            gpio_set_function(i, fn);
        }

be any more efficient than

        if (gpio_mask & 1u) {
            gpio_set_function(i, fn);
        }
        gpio_mask >>= 1;

?

Thinking on this some more, I guess you could just iterate between the first-set-bit and last-set-bit by using both `__builtin_ctz` and `__builtin_ltz`, and then you wouldn't need the `if (!gpio_mask)` check inside the loop-body? And then would ```c if (gpio_mask & (1u << i)) { gpio_set_function(i, fn); } ``` be any more efficient than ```c if (gpio_mask & 1u) { gpio_set_function(i, fn); } gpio_mask >>= 1; ``` ?

Sign in to join this conversation.

No reviewers

will-v-pi