Skip to content

Conversation

@JordanYates
Copy link
Contributor

Update the aarch32 linker file to automatically derive the name of memory regions from devicetree nodes instead of providing hardcoded values. This is required for automatic generation of regions based on compatibles, and also allows chosen and alias nodes to be used to control variable placement (as desired in #36152).

Board and SoC updates were performed automatically via regex find and replace.

Jordan Yates added 4 commits June 12, 2021 15:10
Add a label to `sramN` nodes (Where N is 1-4) to enable automatic
generation of memory region names from the devicetree node.

Performed by regex find and replace in vscode using:
find - `^(\t*)sram(1|2|3|4):([^}]*|\n*)\};`
replace - `$1sram$2:$3\tlabel = "SRAM$2";\n$1};`

Signed-off-by: Jordan Yates <[email protected]>
Add a label to `dtcm` nodes to enable automatic generation of memory
region names from the devicetree node.

Performed by regex find and replace in vscode using:
find - `^(\t*)dtcm:([^}]*|\n*)\};`
replace - `$1dtcm:$2\tlabel = "DTCM";\n$1};`

Signed-off-by: Jordan Yates <[email protected]>
Add a label to `itcm` nodes to enable automatic generation of memory
region names from the devicetree node.

Performed by regex find and replace in vscode using:
find - `^(\t*)itcm:([^}]*|\n*)\};`
replace - `$1itcm:$2\tlabel = "ITCM";\n$1};`

Signed-off-by: Jordan Yates <[email protected]>
Change the label of the `ti_ccfg` partition to match the expected name
of the outputted linker memory region.

Signed-off-by: Jordan Yates <[email protected]>
Derive the names of linker memory regions from the specified devicetree
node, instead of providing a hardcoded value. This is required to
automatically generate regions based on compatibles in the future. It
also lets systems derive the memory region name from a `chosen` or
`alias` node. For example:
```
chosen {
    stm32,eth-dma = &sram3;
}
```
```
SECTION_DATA_PROLOGUE(eth_stm32,(NOLOAD),)
{
    ...
} > GROUP_LINK_IN(DT_LABEL(DT_CHOSEN(stm32_eth_dma)), ...))
```

Signed-off-by: Jordan Yates <[email protected]>
@henrikbrixandersen
Copy link
Member

Why add labels instead of just using the DTS nodelabel?

@JordanYates
Copy link
Contributor Author

Why add labels instead of just using the DTS nodelabel?

That is the alternate option, but:

  1. There is currently no devicetree API to go from node to nodelabel.
  2. Memory region names are typically UPPER_CASE, whereas node labels are lower_case.
  3. Adding a label to all of these nodes is much less likely to be disruptive to other users than changing the nodelabel. A search on DT_NODELABEL(sram shows 36 hits already.

@JordanYates
Copy link
Contributor Author

Actually I think the biggest problem is from https://docs.zephyrproject.org/latest/guides/dts/intro.html#syntax-and-structure

A node can have zero, one, or multiple node labels.

@zephyrbot zephyrbot added platform: nRF Nordic nRFx platform: NXP NXP area: Xtensa Xtensa Architecture labels Jun 14, 2021
@zephyrbot zephyrbot requested review from ABOSTM and removed request for dcpleung June 14, 2021 12:47
@erwango
Copy link
Member

erwango commented Jun 14, 2021

Actually I think the biggest problem is from https://docs.zephyrproject.org/latest/guides/dts/intro.html#syntax-and-structure

A node can have zero, one, or multiple node labels.

Is it?
Just like you're adding label prop, we can make sure that there is one label.
Then, I don't think having multiple node labels is an issue in our case.
Also, a major advantage for node labels being that they are unique, 2 nodes can't have same node label (checked by dts).

@JordanYates
Copy link
Contributor Author

I am happy to transition to generating from the node labels if that is the generally preferred solution.
The remaining question is whether lower case region names are acceptable, or do they need to be upper-cased.

@erwango
Copy link
Member

erwango commented Jun 16, 2021

I am happy to transition to generating from the node labels if that is the generally preferred solution.

Thanks. I think that would indeed be preferred as in general we've tried to minimize their usage over the past years.

The remaining question is whether lower case region names are acceptable, or do they need to be upper-cased.

No preference on my part.
Maybe some have strong opinions on that though..

@JordanYates
Copy link
Contributor Author

Are you aware of any syntax that lets you change or add another nodelabel to a node in an overlay? Apart from just deleting the node and creating it again?

@henrikbrixandersen
Copy link
Member

Are you aware of any syntax that lets you change or add another nodelabel to a node in an overlay? Apart from just deleting the node and creating it again?

new_nodelabel: &existing_nodelabel {};

@JordanYates
Copy link
Contributor Author

new_nodelabel: &existing_nodelabel {};

Cheers. Interestingly it looks like doing this in overlays always results in new_nodelabel being after existing_nodelabel in node.lables in python. Assuming that the solution will just use index 0 to generate a region name, it won't be possible to change the name in an overlay without changing some edtlib parsing.

@erwango
Copy link
Member

erwango commented Jun 17, 2021

Cheers. Interestingly it looks like doing this in overlays always results in new_nodelabel being after existing_nodelabel in node.lables in python. Assuming that the solution will just use index 0 to generate a region name, it won't be possible to change the name in an overlay without changing some edtlib parsing.

@mbolivar-nordic Is that expected ?

@JordanYates
Copy link
Contributor Author

An overlay of

test1: &sram0 {};
test2: &sram0 {};

Will result in a zephyr.dts of

	sram0: test1: test2: memory@24000000 {

So it makes sense that the python labels have the order they do.
I feel like overlays should be able to override the name of the memory region, so I will look into the easiest way of just selecting the last property in a list.

@JordanYates
Copy link
Contributor Author

Superseded by #36365

@mbolivar-nordic
Copy link
Contributor

I haven't been following the discussion, but arbitrarily picking a node label as a canonical way to identify a node does not sound like a good approach. I will check #36365.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants