-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
drivers/periph_gpio_ll: change API to access GPIO ports #20639
Open
maribu
wants to merge
2
commits into
RIOT-OS:master
Choose a base branch
from
maribu:drivers/periph_gpio_ll/api-change
base: master
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains 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
maribu
added
Process: API change
Integration Process: PR contains or issue proposes an API change. Should be handled with care.
CI: ready for build
If set, CI server will compile all applications for all available boards for the labeled PR
labels
Apr 29, 2024
maribu
requested review from
leandrolanzieri,
aabadie,
MichelRottleuthner,
MrKevinWeiss,
vincent-d,
benpicco,
dylad,
keestux,
gschorcht,
basilfx and
kYc0o
as code owners
April 29, 2024 18:29
github-actions
bot
added
Platform: ARM
Platform: This PR/issue effects ARM-based platforms
Area: tests
Area: tests and testing framework
Platform: AVR
Platform: This PR/issue effects AVR-based platforms
Area: drivers
Area: Device drivers
Area: boards
Area: Board ports
Platform: ESP
Platform: This PR/issue effects ESP-based platforms
Area: cpu
Area: CPU/MCU ports
labels
Apr 29, 2024
maribu
changed the title
Drivers/periph gpio ll/api change
drivers/periph_gpio_ll: change API to access GPIO ports
Apr 29, 2024
ESP32
|
maribu
force-pushed
the
drivers/periph_gpio_ll/api-change
branch
from
April 29, 2024 19:43
9dc939a
to
cc1b7a1
Compare
STM32
|
maribu
force-pushed
the
drivers/periph_gpio_ll/api-change
branch
3 times, most recently
from
April 30, 2024 17:16
e5956fd
to
ae64e85
Compare
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.
maribu
force-pushed
the
drivers/periph_gpio_ll/api-change
branch
from
April 30, 2024 19:45
ae64e85
to
09e8d9e
Compare
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Area: boards
Area: Board ports
Area: cpu
Area: CPU/MCU ports
Area: drivers
Area: Device drivers
Area: tests
Area: tests and testing framework
CI: ready for build
If set, CI server will compile all applications for all available boards for the labeled PR
Platform: ARM
Platform: This PR/issue effects ARM-based platforms
Platform: AVR
Platform: This PR/issue effects AVR-based platforms
Platform: ESP
Platform: This PR/issue effects ESP-based platforms
Process: API change
Integration Process: PR contains or issue proposes an API change. Should be handled with care.
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 whennum
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 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_ll
should still work.Issues/PRs references