drivers/periph_gpio_ll: change API to access GPIO ports#20639
Merged
maribu merged 4 commits intoRIOT-OS:masterfrom Aug 2, 2024
Merged
drivers/periph_gpio_ll: change API to access GPIO ports#20639maribu merged 4 commits intoRIOT-OS:masterfrom
maribu merged 4 commits intoRIOT-OS:masterfrom
Conversation
maribu
added
Process: API change
maribu
requested review from
MichelRottleuthner,
MrKevinWeiss,
aabadie,
basilfx,
benpicco,
dylad,
gschorcht,
kYc0o,
keestux,
leandrolanzieri and
vincent-d
as code owners
github-actions
bot
added
Platform: ARM
maribu
changed the title
Member
Author
ESP32 |
maribu
force-pushed
the
drivers/periph_gpio_ll/api-change
branch
from
9dc939a to
cc1b7a1
Compare
Member
Author
STM32 |
maribu
force-pushed
the
drivers/periph_gpio_ll/api-change
branch
4 times, most recently
from
ae64e85 to
09e8d9e
Compare
1 task
benpicco
approved these changes
Aug 1, 2024
benpicco
reviewed
Aug 1, 2024
maribu
force-pushed
the
drivers/periph_gpio_ll/api-change
branch
2 times, most recently
from
3995f78 to
f928b46
Compare
Member
Author
|
Redoing the tests: STM32ESP32SAM0nRF5xATmegaGD32-VEFM32 |
maribu
force-pushed
the
drivers/periph_gpio_ll/api-change
branch
from
f928b46 to
2e8027a
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
maribu
force-pushed
the
drivers/periph_gpio_ll/api-change
branch
from
b9da730 to
dc0c710
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
The API was based on the assumption that GPIO ports are mapped in memory
sanely, so that a `GPIO_PORT(num)` macro would work allow for constant
folding when `num` is known and still be efficient when it is not.
Some MCUs, however, will need a look up tables to efficiently translate
GPIO port numbers to the port's base address. This will prevent the use
of such a `GPIO_PORT(num)` macro in constant initializers.
As a result, we rather provide `GPIO_PORT_0`, `GPIO_PORT_1`, etc. macros
for each GPIO port present (regardless of MCU naming scheme), as well as
`GPIO_PORT_A`, `GPIO_PORT_B`, etc. macros if (and only if) the MCU port
naming scheme uses letters rather than numbers.
These can be defined as macros to the peripheral base address even when
those are randomly mapped into the address space. In addition, a C
function `gpio_port()` replaces the role of the `GPIO_PORT()` and
`gpio_port_num()` the `GPIO_PORT_NUM()` macro. Those functions will
still be implemented as efficient as possible and will allow constant
folding where it was formerly possible. Hence, there is no downside for
MCUs with sane peripheral memory mapping, but it is highly beneficial
for the crazy ones.
There are also two benefits for the non-crazy MCUs:
1. We can now test for valid port numbers with `#ifdef GPIO_PORT_<NUM>`
- This directly benefits the test in `tests/periph/gpio_ll`, which
can now provide a valid GPIO port for each and every board
- Writing to invalid memory mapped I/O addresses was treated as
triggering undefined behavior by the compiler and used as a
optimization opportunity
2. We can now detect at compile time if the naming scheme of the MCU
uses letters or numbers, and produce more user friendly output.
- This is directly applied in the test app
This fixes a race in `LED<NUM>_TOGGLE`, which is a read-copy-write
operation. Any access to a GPIO pin on the same GPIO port that
happens concurrently could result in data corruption. Using the
GPIO LL API, which is thread-safe, fixes the issue.
Note: The used GPIO LL functions will work even in when the GPIO LL
module is not used.
A test intended to ensure that a configuration toggling the direction of a GPIO two times restores the original configuration not only compared the configuration at the two points in time, but also the value of the input buffer. Since a floating input reads back random values when not externally driven, the test was actually randomly failing. Apparently I got lucky before consistently and this never triggered until now. This now clears the input value from both the configuration reported before and after toggling the direction twice and should now indeed succeed consistently.
maribu
force-pushed
the
drivers/periph_gpio_ll/api-change
branch
from
dc0c710 to
4a09286
Compare
Member
Author
|
Thanks :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contribution description
The API was based on the assumption that GPIO ports are mapped in memory sanely, so that a
GPIO_PORT(num)macro would work allow for constant folding whennumis known and still be efficient when it is not.Some MCUs, however, will need a look up tables to efficiently translate GPIO port numbers to the port's base address. This will prevent the use of such a
GPIO_PORT(num)macro in constant initializers.As a result, we rather provide
GPIO_PORT_0,GPIO_PORT_1, etc. macros for each GPIO port present (regardless of MCU naming scheme), as well asGPIO_PORT_A,GPIO_PORT_B, etc. macros if (and only if) the MCU port naming scheme uses letters rather than numbers.These can be defined as macros to the peripheral base address even when those are randomly mapped into the address space. In addition, a C function
gpio_port()replaces the role of theGPIO_PORT()andgpio_port_num()theGPIO_PORT_NUM()macro. Those functions will still be implemented as efficient as possible and will allow constant folding where it was formerly possible. Hence, there is no downsidefor MCUs with sane peripheral memory mapping, but it is highly beneficial for the crazy ones.
There are also two benefits for the non-crazy MCUs:
#ifdef GPIO_PORT_<NUM>tests/periph/gpio_ll, which can now provide a valid GPIO port for each and every boardTesting procedure
The test app in
tests/periph/gpio_llshould still work.Issues/PRs references