Skip to content

boards/arm64/bcm2711/raspberrypi-4b: Fix GPIO#18499

Merged
linguini1 merged 1 commit intoapache:masterfrom
resyfer:rpi
Mar 8, 2026
Merged

boards/arm64/bcm2711/raspberrypi-4b: Fix GPIO#18499
linguini1 merged 1 commit intoapache:masterfrom
resyfer:rpi

Conversation

@resyfer
Copy link
Contributor

@resyfer resyfer commented Mar 5, 2026

Note: Please adhere to Contributing Guidelines.

Summary

It PR fixes a bug where clearing a GPIO pin does not clear the set bit in its set register. Similarly, while setting a GPIO pin, it doesn't reset its clear bit. This might cause undefined behavior as, in some cases, both set and reset bits might be set.

Another bug is that pins of multiples of 10 (except GPIO 10 itself) don't work as expected as they get skipped while assigning alternate functions.

Impact

You can use any GPIO pin available on the board's headers for input and output.

Testing

I've tested the output functionality of every pin using a custom blink LED application that uses the registered GPIO driver for a pin. I executed it on each GPIO port possible in turn, verified both the GPIO registers (by getting the value from the GPIO register) and an actual LED connected to the pin(s). Further, I ensured no other pins accidentally set another.

I've not tested the input yet, as it's similar to output and the testing is quite a manual process. However, if needed, I can arrange for testing that as well.

Blink application:

#include <sys/ioctl.h>
#include <stdio.h>
#include <fcntl.h>
#include <nuttx/ioexpander/gpio.h>

void print_all_gpios(void)
{
	bool val = false;
	int ret = OK;

	printf("===========================\n");

	for(int i = 0; i <= 27; i++)
		{
			char dev[30];

			sprintf(dev, "/dev/gpio%d", i);

			int fd = open(dev, O_RDWR);
			if (fd < 0)
			{
				printf("GPIO %d:\tCould not open fd.\n", i);
				continue;
			}

			ret = ioctl(fd, GPIOC_READ, (unsigned long)((uintptr_t) &val));
			if (ret != OK)
				{
					printf("GPIO %d:\tBad return (%d)\n", i, ret);
					ret = OK;
				}
			else
				{
					printf("GPIO %d:\t%d\n", i, val);
					val = -1;
				}

			close(fd);
		}

	printf("===========================\n");
}

int main(int argc, char** argv) {
	int ret = OK;
	int fd;
	const char *dev = argv[1];

	fd = open(dev, O_RDWR);
	if (predict_false(fd < 0))
		{
			ret = 1;
			printf("Could not open %s", dev);
			goto errout;
		}

	for(; ;) {
		ret = ioctl(fd, GPIOC_WRITE, (unsigned long) 1);
		if (predict_false(ret < 0))
		{
			goto errout_with_fd;
		}

		sleep(1);

		print_all_gpios();

		sleep(1);

		ret = ioctl(fd, GPIOC_WRITE, (unsigned long) 0);
		if (predict_false(ret < 0))
		{
			goto errout_with_fd;
		}

		sleep(2);
	}

	close(fd);
	return ret;

errout_with_fd:
	close(fd);

errout:
	return ret;
}

@resyfer resyfer requested a review from linguini1 as a code owner March 5, 2026 17:52
@simbit18
Copy link
Contributor

simbit18 commented Mar 5, 2026

GitHub is having problems! :)

Multiple services are affected, service degradation
https://www.githubstatus.com/incidents/g5gnt5l5hf56

@github-actions github-actions bot added Arch: arm64 Issues related to ARM64 (64-bit) architecture Size: L The size of the change in this PR is large Board: arm64 labels Mar 5, 2026
@resyfer resyfer force-pushed the rpi branch 2 times, most recently from 6b5614a to 27da893 Compare March 5, 2026 18:31
@linguini1
Copy link
Contributor

Hi @resyfer !

Thank you for the contribution :) This approach is very different from traditional GPIO drivers in NuttX; usually there is just some list of GPIO pins for users to play with (more of a demo) and they can modify the driver to add more depending on use case. Do you have a use case for needing all of the GPIO pins to be configurable?

This PR will need to be tested for each GPIO pin type (input, output, interrupt). If you could include an image of your blinking LED/output logs, that would be great!

Also, please review the contributing guide for the commit message format :)

@acassis
Copy link
Contributor

acassis commented Mar 5, 2026

Hi @resyfer !

Thank you for the contribution :) This approach is very different from traditional GPIO drivers in NuttX; usually there is just some list of GPIO pins for users to play with (more of a demo) and they can modify the driver to add more depending on use case. Do you have a use case for needing all of the GPIO pins to be configurable?

This PR will need to be tested for each GPIO pin type (input, output, interrupt). If you could include an image of your blinking LED/output logs, that would be great!

Also, please review the contributing guide for the commit message format :)

Exactly! You can bringing all the complexity to ALT modes to inside Kconfig.

Unfortunately Kconfig is not flexible to implement a GPIO ALT MUX. It is not a programming language.

@resyfer
Copy link
Contributor Author

resyfer commented Mar 6, 2026

Hi @linguini1 @acassis. The reason why I have each pin's alternate functions listed is because of a personal opinion of an OS letting the user do what they want with their device (using the full capability), rather than dictating what they can do. However, if it conflicts in the way NuttX does it for other boards then I'll change it back.

Also, I had made changes according to the suggestions made by Alan on this...making this feature-centric rather than pin-centric. However, KConfig can cause conflicts in that. If, for example, I write a config where people can switch on features (say, UART 0 or 2) with a menu for all the unspecified pins to be chosen as input or output as well. However, due to cyclic dependencies in KConfig, I can't ensure that UART 2 (for eg.) and GPIO 3 OUTPUT (for eg.) are mutually exclusive. I can make it one way...selecting UART 2 disables GPIO 3 OUTPUT option, but the other way is not possible (cyclic dependencies... A can depend on !B, but B can't depend on !A too).

So I reverted back to being pin-centric. This way allows the user to chose what they want, and also, if improperly configured, will not enable features (for eg. UART 0 will not be enabled if one of its pins are not configured for UART).

I was experimenting this approach however. My reasons for adding full configurability is simply academic and allowing full usage of the the board. If you foresee this to cause maintainability issues or scaling issues, I'm fine with reverting the KConfig stuff. Let me know your feedback, thanks!

@acassis
Copy link
Contributor

acassis commented Mar 6, 2026

Hi @linguini1 @acassis. The reason why I have each pin's alternate functions listed is because of a personal opinion of an OS letting the user do what they want with their device (using the full capability), rather than dictating what they can do. However, if it conflicts in the way NuttX does it for other boards then I'll change it back.

Also, I had made changes according to the suggestions made by Alan on this...making this feature-centric rather than pin-centric. However, KConfig can cause conflicts in that. If, for example, I write a config where people can switch on features (say, UART 0 or 2) with a menu for all the unspecified pins to be chosen as input or output as well. However, due to cyclic dependencies in KConfig, I can't ensure that UART 2 (for eg.) and GPIO 3 OUTPUT (for eg.) are mutually exclusive. I can make it one way...selecting UART 2 disables GPIO 3 OUTPUT option, but the other way is not possible (cyclic dependencies... A can depend on !B, but B can't depend on !A too).

So I reverted back to being pin-centric. This way allows the user to chose what they want, and also, if improperly configured, will not enable features (for eg. UART 0 will not be enabled if one of its pins are not configured for UART).

I was experimenting this approach however. My reasons for adding full configurability is simply academic and allowing full usage of the the board. If you foresee this to cause maintainability issues or scaling issues, I'm fine with reverting the KConfig stuff. Let me know your feedback, thanks!

Hi @resyfer yes, I suggest not defining those CONFIG_RPI4B_GPIOn at Kconfig, notice that even RP2040 that defined many I2C and SPI pins doesn't define any GPIO pin directly, see: arch/arm/src/rp2040/Kconfig

Please revert it and use in the board like all other boards are doing.

@linguini1 maybe we need to write this rule in some place. Other users like @vrmay23 are also trying to define GPIO pins in the Kconfig. And in the past even a new developer tried to created a script to automatically map all pins to generate the Kconfig. Probably NuttX needs support to Device Tree to simplify it.

@acassis
Copy link
Contributor

acassis commented Mar 6, 2026

@simbit18 @raiden00pl @lupyuen @xiaoxiang781216 please help reviewing if possible

@vrmay23
Copy link
Contributor

vrmay23 commented Mar 6, 2026

I configured it and it worked quite well I would say. But in my case I did like this: I had a menu where I could select under SPI / I2C / which will be the alternative_function AND which set of pins for each bus. So easily I can select the CS pin (also the "polarity", DevID, MOSi, MISO, SDA, SCL.
This is quite fast to use and adjust the setup during the development phase. So far I did not found any issues and my ideia is to extend the same ideia for CAN / USB while there is no device tree available.

Nevertheless I just made it for one specific board and did not tested in any other place.. Hence, I am totally on what @acassis said we need a documentation on it.

@linguini1
Copy link
Contributor

linguini1 commented Mar 6, 2026

Hi @linguini1 @acassis. The reason why I have each pin's alternate functions listed is because of a personal opinion of an OS letting the user do what they want with their device (using the full capability), rather than dictating what they can do. However, if it conflicts in the way NuttX does it for other boards then I'll change it back.

That's a good opinion to have, but it does conflict with how NuttX works with other boards. I would go as far as to say the NuttX still lets the user do what they want with their device without this method of defining pins. The currently adopted method allows the user to declaratively select the interfaces they want enabled (i.e. I2C0) and adjust the pin assignments for that interface as well. This new method exposes the user to every option for the pins, yes, but probably 80% of those options are not supported by the OS anyways. It gives a false impression to the user that functionality is available (at least, from my point of view)

Also, I had made changes according to the suggestions made by Alan on this...making this feature-centric rather than pin-centric. However, KConfig can cause conflicts in that. If, for example, I write a config where people can switch on features (say, UART 0 or 2) with a menu for all the unspecified pins to be chosen as input or output as well. However, due to cyclic dependencies in KConfig, I can't ensure that UART 2 (for eg.) and GPIO 3 OUTPUT (for eg.) are mutually exclusive. I can make it one way...selecting UART 2 disables GPIO 3 OUTPUT option, but the other way is not possible (cyclic dependencies... A can depend on !B, but B can't depend on !A too).

Yeah, this is a big limitation with Kconfig. The options most boards take are:

  • Assume the user knows what they're doing and don't check conflicts
  • Use preprocessor logic to compile-time check conflicts, like
#if defined(CONFIG_UART2) && CONFIG_UART2_RX_PIN == 3 && CONFIG_GPOUT_PIN3
#error "Pin conflict"
#endif 

This gets very verbose in a case like yours though, where you have 27 possible GPIO output pins to check conflicts against.

So I reverted back to being pin-centric. This way allows the user to chose what they want, and also, if improperly configured, will not enable features (for eg. UART 0 will not be enabled if one of its pins are not configured for UART).

I was experimenting this approach however. My reasons for adding full configurability is simply academic and allowing full usage of the the board. If you foresee this to cause maintainability issues or scaling issues, I'm fine with reverting the KConfig stuff. Let me know your feedback, thanks!

I see your reason for this approach here, but I think for now it is best to revert to the "old way", because this method will be different from every other board in the tree and that will be confusing for users & maintainers. It is also a little bit easier to think about enabling interfaces as a whole instead of pin-by-pin in my opinion, although it is harder to compile-time check.

@resyfer
Copy link
Contributor Author

resyfer commented Mar 7, 2026

Yep understood. I'll change it to be in line with other boards.

Also, here's a bit of the logs (from the same blink application provided above) [NOTE: pin 14 & 15 are for UART]:

...
rpi4b_gpout_write: Writing 0 to pin 26
- Ready to Boot Primary CPU
- Boot from EL2
- Boot from EL1
- Boot to C runtime for OS Initialize
rpi4b_register_pins: Unsupported function '0' on GPIO 0
rpi4b_register_pins: Unsupported function '0' on GPIO 1
rpi4b_register_gpio_output: Registering GPIO pin 2 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio2
rpi4b_register_gpio_output: Registering GPIO pin 3 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio3
rpi4b_register_gpio_output: Registering GPIO pin 4 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio4
rpi4b_register_gpio_output: Registering GPIO pin 5 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio5
rpi4b_register_gpio_output: Registering GPIO pin 6 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio6
rpi4b_register_gpio_output: Registering GPIO pin 7 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio7
rpi4b_register_gpio_output: Registering GPIO pin 8 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio8
rpi4b_register_gpio_output: Registering GPIO pin 9 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio9
rpi4b_register_gpio_output: Registering GPIO pin 10 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio10
rpi4b_register_gpio_output: Registering GPIO pin 11 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio11
rpi4b_register_gpio_output: Registering GPIO pin 12 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio12
rpi4b_register_gpio_output: Registering GPIO pin 13 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio13
rpi4b_register_pins: Unsupported function '0' on GPIO 14
rpi4b_register_pins: Unsupported function '0' on GPIO 15
rpi4b_register_gpio_output: Registering GPIO pin 16 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio16
rpi4b_register_gpio_output: Registering GPIO pin 17 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio17
rpi4b_register_gpio_output: Registering GPIO pin 18 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio18
rpi4b_register_gpio_output: Registering GPIO pin 19 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio19
rpi4b_register_gpio_output: Registering GPIO pin 20 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio20
rpi4b_register_gpio_output: Registering GPIO pin 21 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio21
rpi4b_register_gpio_output: Registering GPIO pin 22 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio22
rpi4b_register_gpio_output: Registering GPIO pin 23 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio23
rpi4b_register_gpio_output: Registering GPIO pin 24 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio24
rpi4b_register_gpio_output: Registering GPIO pin 25 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio25
rpi4b_register_gpio_output: Registering GPIO pin 26 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio26
rpi4b_register_gpio_output: Registering GPIO pin 27 as OUTPUT
gpio_pin_register_byname: Registering /dev/gpio27

NuttShell (NSH) NuttX-12.12.0
nsh> blink /dev/gpio26
rpi4b_gpout_write: Writing 1 to pin 26
===========================
GPIO 0: Could not open fd.
GPIO 1: Could not open fd.
GPIO 2: 0
GPIO 3: 0
GPIO 4: 0
GPIO 5: 0
GPIO 6: 0
GPIO 7: 0
GPIO 8: 0
GPIO 9: 0
GPIO 10:        0
GPIO 11:        0
GPIO 12:        0
GPIO 13:        0
GPIO 14:        Could not open fd.
GPIO 15:        Could not open fd.
GPIO 16:        0
GPIO 17:        0
GPIO 18:        0
GPIO 19:        0
GPIO 20:        0
GPIO 21:        0
GPIO 22:        0
GPIO 23:        0
GPIO 24:        0
GPIO 25:        0
GPIO 26:        1
GPIO 27:        0
===========================
rpi4b_gpout_write: Writing 0 to pin 26
rpi4b_gpout_write: Writing 1 to pin 26
...

And here's a proof it's not AI and it's working:
image

UART for serial console + GPIO 26 as output (PS: I don't have any female-to-female jumper wires at hand :( )

@linguini1 linguini1 changed the title [boards/arm64/bcm2711/raspberrypi-4b] Full GPIO IN/OUT support boards/arm64/bcm2711/raspberrypi-4b: Full GPIO IN/OUT support Mar 7, 2026
* Fix GPIO unspecified behavior on some GPIO ports.
* Fix GPIO undefined behavior caused by uncleared set or reset bits.

Signed-off-by: Saurav Pal <resyfer.dev@gmail.com>
@resyfer resyfer changed the title boards/arm64/bcm2711/raspberrypi-4b: Full GPIO IN/OUT support boards/arm64/bcm2711/raspberrypi-4b: Fix GPIO Mar 8, 2026
@github-actions github-actions bot added Size: S The size of the change in this PR is small and removed Size: L The size of the change in this PR is large Board: arm64 labels Mar 8, 2026
@resyfer
Copy link
Contributor Author

resyfer commented Mar 8, 2026

@linguini1 Based on my talks with Alan, I've removed the configuration stuff, and left it to the user to define in the header files. I've kept the bug fixes though. PTAL.

@linguini1
Copy link
Contributor

@linguini1 Based on my talks with Alan, I've removed the configuration stuff, and left it to the user to define in the header files. I've kept the bug fixes though. PTAL.

Was a commit lost in your squash? The changes look like they don't modify much GPIO logic anymore.

@resyfer
Copy link
Contributor Author

resyfer commented Mar 8, 2026

@linguini1 Alan mentioned that users set the GPIO pins they want to use in the header files. In that case, a lot of my initial work in this PR (according to lines of code) to allow configuration in some way through Kconfig was not required anymore. Hence I removed them and kept the bug fixes.

Copy link
Contributor

@linguini1 linguini1 left a comment

Choose a reason for hiding this comment

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

Thank you for the fix @resyfer :)

And, thanks for testing on the 2GB variant; now all we need is a user with 1GB to cover everything!

@resyfer
Copy link
Contributor Author

resyfer commented Mar 8, 2026

Thanks for your patient review!

@linguini1 linguini1 merged commit 8ddcf81 into apache:master Mar 8, 2026
13 checks passed
@acassis
Copy link
Contributor

acassis commented Mar 8, 2026

@resyfer thank you for the improvement!

@cederom
Copy link
Contributor

cederom commented Mar 8, 2026

Big thank you @resyfer !! I just came back from trip sorry for being late :D

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

Labels

Arch: arm64 Issues related to ARM64 (64-bit) architecture Size: S The size of the change in this PR is small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants