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

cmake: extensions: Check status of "zephyr,memory-region" DT nodes #63683

Merged
merged 2 commits into from
Oct 11, 2023

Conversation

57300
Copy link
Contributor

@57300 57300 commented Oct 9, 2023

Functions zephyr_linker_dts_memory() and zephyr_linker_dts_section() are described as defining a memory region or section based on a DT node, as long as it "exists and has status okay". However, only the existence of the node is actually checked, using dt_node_exists(). To fix that, employ dt_node_has_status() instead, which can check both conditions.

The status check is important, because both functions require the given DT node to contain a zephyr,memory-region property (not to be confused with the compatible string). This property is required by the associated binding as well, but required properties can be omitted from nodes which don't have status "okay". In those cases, edtlib won't raise an error, and neither should CMake, because those nodes should be ignored.

Speaking of that property, add a missing error check for it as a bonus (tucked behind the status check, of course).

Calling this function with an output variable named `var` or `okay`
could produce an incorrect result, due to the variable being mishandled.
Add simple changes to avoid this.

Signed-off-by: Grzegorz Swiderski <grzegorz.swiderski@nordicsemi.no>
Functions `zephyr_linker_dts_memory()` and `zephyr_linker_dts_section()`
are described as defining a memory region or section based on a DT node,
as long as it "exists and has status okay". However, only the existence
of the node is actually checked, using `dt_node_exists()`. To fix that,
employ `dt_node_has_status()` instead, which can check both conditions.

The status check is important, because both functions require the given
DT node to contain a `zephyr,memory-region` property (not to be confused
with the compatible string). This property is required by the associated
binding as well, but required properties can be omitted from nodes which
don't have status "okay". In those cases, edtlib won't raise an error,
and neither should CMake, because those nodes should be ignored.

Speaking of that property, add a missing error check for it as a bonus
(tucked behind the status check, of course).

Signed-off-by: Grzegorz Swiderski <grzegorz.swiderski@nordicsemi.no>
@57300 57300 added bug The issue is a bug, or the PR is fixing a bug Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc. labels Oct 9, 2023
@tejlmand tejlmand added this to the v3.5.0 milestone Oct 11, 2023
@jhedberg jhedberg merged commit 5cf5282 into zephyrproject-rtos:main Oct 11, 2023
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Build System bug The issue is a bug, or the PR is fixing a bug Trivial Changes that can be reviewed by anyone, i.e. doc changes, minor build system tweaks, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants