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

dts: gen_defines: Generate PARTITION_ID only for "okay" nodes #36394

Closed

Conversation

de-nordic
Copy link
Collaborator

The commit modifies how the PARTITION_ID is generated, to provide
an ID only for partition nodes that are "okay".
The change will cause code trying to obtain ID of non-"okay"ed
partition to fail compilation, which allows compile time detection
of addressing non-existing partitions.
This also allows to produce, at compile time, arrays of objects
that can be indexed by the ID of partition, while not including
disabled nodes.

Currently when user defines, for example 80 partitions, and enables only 2, the identifiers for them may be for example 3 and 49, although only two exist within system; these are auto generated from DTS after it has been generated from all DTS overlays, and so on, and is never literally stated within DTS; any change to the DTS may actually give these IDs some other random values.
The commit changes how the IDs are generated so that they are generated starting at 0 and will be numbered in continuous way up to N-1 where N is number of enabled partitions.

This has been taken from #34530

Signed-off-by: Dominik Ermel dominik.ermel@nordicsemi.no

The commit modifies how the PARTITION_ID is generated, to provide
an ID only for partition nodes that are "okay".
The change will cause code trying to obtain ID of non-"okay"ed
partition to fail compilation, which allows compile time detection
of addressing non-existing partitions.
This also allows to produce, at compile time, arrays of objects
that can be indexed by the ID of partition, while not including
disabled nodes.

Signed-off-by: Dominik Ermel <dominik.ermel@nordicsemi.no>
@github-actions github-actions bot added area: API Changes to public APIs area: Devicetree area: Devicetree Tooling PR modifies or adds a Device Tree tooling labels Jun 18, 2021
@@ -63,6 +63,9 @@ extern "C" {

/**
* @brief Get a numeric identifier for a fixed partition
*
* The partition ID will be only available from status "okay" partition entries
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a backwards-incompatible change, and devicetree.h was just marked stable, so this will need to go through the stable API change process now, with an RFC issue, email to devel, etc. etc.

https://docs.zephyrproject.org/latest/reference/api/api_lifecycle.html#introducing-incompatible-changes

@mbolivar-nordic mbolivar-nordic added the DNM This PR should not be merged (Do Not Merge) label Jun 18, 2021
@mbolivar-nordic
Copy link
Contributor

No objections to the idea, but we need to follow the stable API process, so adding a DNM until all that boilerplate work is done too.

@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Aug 19, 2021
@github-actions github-actions bot closed this Sep 2, 2021
@carlescufi carlescufi reopened this Nov 16, 2021
@carlescufi carlescufi added the Breaking API Change Breaking changes to stable, public APIs label Nov 16, 2021
@carlescufi
Copy link
Member

@galak please take a look

@galak
Copy link
Collaborator

galak commented Nov 16, 2021

@galak please take a look

So I'd like to see if we could phase out the PARTITION_ID usage from the generation script. We might a new form of DT_FOREACH_STATUS_OKAY maybe having a DT_FOREACH_IDX_STATUS_OKAY. (mimic behavior of FOR_EACH_IDX macro)

@github-actions github-actions bot removed the Stale label Nov 17, 2021
@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Jan 16, 2022
@github-actions github-actions bot closed this Jan 31, 2022
@de-nordic de-nordic deleted the gen-partition-id branch January 12, 2023 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: Devicetree Tooling PR modifies or adds a Device Tree tooling area: Devicetree Breaking API Change Breaking changes to stable, public APIs DNM This PR should not be merged (Do Not Merge) Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants