Skip to content
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

boards/ek-lm4f120x: Change internal LED macro for C2Rust compatibility #20831

Conversation

chrysn
Copy link
Member

@chrysn chrysn commented Aug 23, 2024

Contribution description

boards/ek-lm4f120x: Change internal LED macro for C2Rust compatibility

C preprocessor defines in non-function form are assumed by C2Rust to be
constant if they are an expression and not a statement; the LED_PORT was
the only place in the code where that was wrong, and led to compiler
errors due to the value not being constant.

Altering the internal macro to use function form sidesteps that issue.
The generally preferred alternative of using a `const` is unavailable in
this case because the dereferencing operator is already part of the
vendor header file cpu/stellaris_common/include/vendor/cortex-m4-def.h.

The changed macro is documented as required by doccheck. The doccheck
rule that grandfathered in the LED_PORT macro as allowed undocumented is
not removed because it is also used in other board.h files.

With that applied, a bunch of BOARD_BLACKLIST entries can be removed (from all Rust Makefiles we have).

Testing procedure

I did not test LED blinking on that board as I don't have one, but the change should be sufficiently easy to verify on a theoretical level (supported by the compiler passing).

@github-actions github-actions bot added Area: tests Area: tests and testing framework Area: boards Area: Board ports Area: examples Area: Example Applications labels Aug 23, 2024
@chrysn chrysn added CI: run tests If set, CI server will run tests on hardware for the labeled PR CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Aug 23, 2024
@riot-ci
Copy link

riot-ci commented Aug 23, 2024

Murdock results

✔️ PASSED

b802a19 rust: Remove ek-lm4f120x board from blacklist

Success Failures Total Runtime
487 0 487 03m:47s

Artifacts

@chrysn chrysn force-pushed the ek-lm4f120x-led-rust-const-avoidance branch from e8f2168 to 121785b Compare August 23, 2024 11:31
chrysn added 2 commits August 23, 2024 13:48
C preprocessor defines in non-function form are assumed by C2Rust to be
constant if they are an expression and not a statement; the LED_PORT was
the only place in the code where that was wrong, and led to compiler
errors due to the value not being constant.

Altering the internal macro to use function form sidesteps that issue.
The generally preferred alternative of using a `const` is unavailable in
this case because the dereferencing operator is already part of the
vendor header file cpu/stellaris_common/include/vendor/cortex-m4-def.h.

The changed macro is documented as required by doccheck. The doccheck
rule that grandfathered in the LED_PORT macro as allowed undocumented is
not removed because it is also used in other board.h files.
@chrysn chrysn force-pushed the ek-lm4f120x-led-rust-const-avoidance branch from 121785b to b802a19 Compare August 23, 2024 11:48
@@ -46,22 +46,26 @@ extern "C" {
#define LED1_PIN GPIO_PIN(5, 2)
#define LED2_PIN GPIO_PIN(5, 3)

#define LED_PORT (GPIO_PORTF_DATA_R)
/**
* @brief Port used for `LED0_ON` and similar implementations
Copy link
Member Author

Choose a reason for hiding this comment

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

This is LED0_ON and not @ref LED0_ON because Doxygen could not resolve that.

@chrysn chrysn marked this pull request as ready for review August 23, 2024 11:49
@chrysn chrysn requested review from benpicco and maribu August 24, 2024 20:57
Copy link
Contributor

@mguetschow mguetschow left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! No hardware to test it here though, either.

@mguetschow mguetschow added this pull request to the merge queue Aug 26, 2024
Merged via the queue into RIOT-OS:master with commit d7669fa Aug 26, 2024
27 checks passed
@chrysn chrysn deleted the ek-lm4f120x-led-rust-const-avoidance branch August 26, 2024 13:10
@benpicco benpicco added this to the Release 2024.10 milestone Nov 27, 2024
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: examples Area: Example Applications 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants