boards/arm64/bcm2711/raspberrypi-4b: Fix GPIO#18499
Conversation
|
GitHub is having problems! :) Multiple services are affected, service degradation |
6b5614a to
27da893
Compare
|
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. |
|
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. |
|
@simbit18 @raiden00pl @lupyuen @xiaoxiang781216 please help reviewing if possible |
|
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. 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. |
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)
Yeah, this is a big limitation with Kconfig. The options most boards take are:
#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.
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. |
* 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>
|
@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. |
|
@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. |
|
Thanks for your patient review! |
|
@resyfer thank you for the improvement! |
|
Big thank you @resyfer !! I just came back from trip sorry for being late :D |

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: