RFC: Variants of DT_REG_ADDR() with explicit 'ranges' mapping

[57300]

Introduction

This is a proposal to introduce new cousins of the DT_REG_ADDR() macro, which would:

• Lookup a node's address within a particular address space.
• Distinguish between ranges-translated and untranslated addresses in code.
• When writing or refactoring DTS and overlays, help catch mistakes at compile time.
• Bring the usage of reg and ranges properties closer in line with Linux.

I've come up with a few possible approaches and I would appreciate some discussion on what would be the optimal solution.

Motivating problem

Consider these simplified DTS fragments:

/* soc-xxx.dts */

/ {
    soc: soc {
        ranges;

        peripheral: peripheral@50000000 {
            reg = < 0x50000000 0x10000000 >;
            ranges = < 0x0 0x50000000 0x10000000 >;

            flash_controller: flash-controller@60000 {
                reg = < 0x60000 0x1000 >;

                flash: flash@10000000 {
                    reg = < 0x10000000 0x20000 >;
                };
            };
        };

        sram: sram@30000000 {
            reg = < 0x30000000 0x10000 >;
            ranges = < 0x0 0x30000000 0x10000 >;
        };
    };
};

#include "soc-common.dtsi"

/* soc-common.dtsi */

&sram {
    shmem: shmem@e000 {
        reg < 0xe000 0x2000 >;
    };
};

Using Zephyr's devicetree API today, we can expect to get these node addresses:

DT_REG_ADDR(DT_NODELABEL(peripheral))       == 0x50000000; /* mapped to root via 'soc' */
DT_REG_ADDR(DT_NODELABEL(flash_controller)) == 0x50060000; /* mapped to root via 'soc', 'peripheral' */
DT_REG_ADDR(DT_NODELABEL(flash))            == 0x10000000; /* mapped to 'flash_controller' */
DT_REG_ADDR(DT_NODELABEL(sram))             == 0x30000000; /* mapped to root via 'soc' */
DT_REG_ADDR(DT_NODELABEL(shmem))            == 0x3000e000; /* mapped to root via 'soc', 'sram' */

For each node, the adjacent comment indicates which address space it is mapped to, through intermediate ranges-translations.

Now, suppose that we either modify soc-xxx.dts, or add soc-yyy.dts, with the following change:

&sram {
    ranges = < 0x0 0x30000000 0x8000 >;
};

This makes shmem fall outside of the ranges given by sram. The outcome is that the address of shmem can no longer be translated up to the address space of soc. Therefore, we get a different value:

DT_REG_ADDR(DT_NODELABEL(shmem)) == 0x0000e000; /* mapped to 'sram' */

From the DTS perspective, there is nothing wrong here. Although the semantics of a node being "out of range" have not been very well described in the DT spec, the consensus is that the shmem node still has a valid address - just not one in the physical address space of the root node. This is consistent with the fact that neither the Python tooling nor dtc will emit any kind of error in this situation.

From our perspective, though, this change can be accidental, and if we have a driver which depends on the address to shmem being fully translated up to the root, then using the untranslated value given by DT_REG_ADDR() will lead to a bus fault or unpredictable behavior. In this case, it should be clear that we made a subtle mistake by forgetting to move shmem to a different address. More importantly, such a mistake can be hard to find in general, so it would be valuable to be able to catch it at build time. Some users might expect this to be caught by the tooling itself, but this cannot happen, because the DTS is not wrong, as already explained.

The way I see it, the main problem is that DT_REG_ADDR() doesn't specify which address space the returned value belongs to. Typically, it's used as either a physical address, or the value of reg with no translation, but there is no way to reliably tell which one it is. Thus, I would consider DT_REG_ADDR() to be somewhat ambiguous, and this is where my proposal comes in.

Proposal

I would like to implement something to the effect of:

• DT_REG_RAW_ADDR(node_id) - address of node_id with no translation, as given in its reg.
• DT_REG_ABS_ADDR(node_id) - absolute/physical address of node_id, with all ranges-translations applied.

Optionally, I also thought about adding:

• DT_REG_REL_ADDR(node_id, ref_node_id) - address of node_id within the address space of ref_node_id.

(I'm not fully satisfied with those names, but I'll continue using them as examples.)

This would establish the following equivalences:

DT_REG_RAW_ADDR(node_id) == DT_REG_REL_ADDR(node_id, DT_PARENT(node_id));
DT_REG_ABS_ADDR(node_id) == DT_REG_REL_ADDR(node_id, DT_ROOT);
DT_REG_ADDR(node_id)     == DT_REG_REL_ADDR(node_id, node_id_picked_by_edtlib);

Today, node_id_picked_by_edtlib is the most distant ancestor of node_id up to which translation is possible.

Returning to the motivating problem above, we would get the following:

DT_REG_RAW_ADDR(DT_NODELABEL(peripheral))       == 0x50000000; /* untranslated */
DT_REG_ABS_ADDR(DT_NODELABEL(peripheral))       == 0x50000000; /* mapped to root via 'soc' */

DT_REG_RAW_ADDR(DT_NODELABEL(flash_controller)) == 0x00060000; /* untranslated */
DT_REG_ABS_ADDR(DT_NODELABEL(flash_controller)) == 0x50060000; /* mapped to root via 'soc', 'peripheral' */

DT_REG_RAW_ADDR(DT_NODELABEL(flash))            == 0x10000000; /* untranslated */
DT_REG_ABS_ADDR(DT_NODELABEL(flash))            ; /* ERROR: cannot be mapped to 'peripheral' */

DT_REG_RAW_ADDR(DT_NODELABEL(sram))             == 0x30000000; /* untranslated */
DT_REG_ABS_ADDR(DT_NODELABEL(sram))             == 0x30000000; /* mapped to root via 'soc' */

DT_REG_RAW_ADDR(DT_NODELABEL(shmem))            == 0x0000e000; /* untranslated */
DT_REG_ABS_ADDR(DT_NODELABEL(shmem))            ; /* ERROR: cannot be mapped to 'soc' */

With this, our driver code could explicitly state that it requires a physical address to shmem, and we would get a compile-time error if there isn't one provided by the devicetree structure.

Background

When I began trying to solve the motivating problem, I started reading the Linux kernel source code for inspiration. Its devicetree API, defined in drivers/of, includes these relevant (although undocumented) functions:

• of_get_address(), which returns an address with no translation.
• of_translate_address(), which returns an absolute/physical address, or an error code if it doesn't exist.

My understanding is that Linux developers are free to choose either function in their own drivers, platform code, etc. The corollary is that they get to choose at runtime whether a given node's address should be used with or without ranges-translation.

This proposal would bring something similar to Zephyr, where of_get_address() would correspond to DT_REG_RAW_ADDR(), and of_translate_address() to DT_REG_ABS_ADDR(). Of course, by extending the macro-based API, Zephyr developers would have the advantage of knowing at compile time whether a node address can be translated as desired.

Implementation details

Naturally, I'd like to update the API itself (in both C and CMake) and generator scripts to support the necessary variants of:

• DT_REG_ADDR()
• DT_REG_ADDR_BY_IDX()
• DT_REG_ADDR_BY_NAME()
• DT_INST_REG_ADDR()
• DT_INST_REG_ADDR_BY_IDX()
• DT_INST_REG_ADDR_BY_NAME()

Changes to edtlib would encompass:

1. Turning the internal _translate() function into a public API: translate_one(addr: int, node: dtlib_Node) -> Optional[int]. This would translate addr to the address space of node and return either a valid address or None. It would be called iteratively, taking the previous result and mapping it onto node.parent. In particular, this would be suitable for generating all possible values for DT_REG_REL_ADDR().
2. Adding a Node.raw_regs property to the public API, which would contain addresses with no translation. There is already Node.regs, but its values are translated in the manner of DT_REG_ADDR(), and it probably shouldn't be broken. Both properties would be set in _init_regs().

Impact

Initially, the rest of Zephyr will not be affected by this change. Eventually, I do hope that other developers can benefit from using the new API instead of DT_REG_ADDR() in their code, so that they can be more conscious of how certain addresses should be interpreted. This could result in more readable code that is also less susceptible to the type of mistakes outlined above.

Alternatives

I've considered that one could use DT_PROP_BY_IDX(node_id, reg, 0) in place of DT_REG_RAW_ADDR(node_id), but only for 32-bit addresses. This could be generalized by adding a different API for retrieving #address-cells and #size-cells, but I think that would be a bit clunky.

I've also thought about using the existing DT_RANGES_* macros to translate and validate addresses manually. This would still require at least the following additions:

• DT_REG_RAW_ADDR()
• Something like DT_FOREACH_ANCESTOR() for walking up the tree.
• A small bugfix: https://github.com/zephyrproject-rtos/zephyr/pull/58858

With this method, compile-time validation would probably result in a pretty gnarly-looking BUILD_ASSERT(). Runtime validation could be more reasonable, but I wouldn't consider that to be a desirable solution.

[github-actions[bot]]

Hi @57300! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

[mbolivar-ampere]

I have some problems of my own that I think would be addressed by this general approach as well. Sorry I did not have time to review in detail but I support the general idea as outlined in the overview and expect to have more time on Friday to look at the details. Sorry for the delay.

[mbolivar-ampere]

OK, I've read and thought about this and I think it makes perfect sense and you've got a great proposal.

I want to make a couple of minor suggestions

• DT_REG_RAW_ADDR(node_id): suggest renaming to DT_REG_ADDR_RAW, so that we can still grep for DT_REG_ADDR to search for address lookups. This is useful when digging around in drivers, for example.

• DT_REG_ABS_ADDR(node_id): "absolute" feels like a concept that will not age well. I am open to suggestions but here are a few ideas:

  • DT_REG_ADDR_CPU: makes it clearer that this is a physical CPU address
  • DT_REG_ADDR_CPUS if you're worried about clarity on SMP (I am not)
  • DT_REG_ADDR_ROOT: makes it clearer that this is requesting a register address in the address space of the root node (which is implicitly what /cpus looks at)
• additionally have edtlib error out if a translated register address or size won't fit in the root node's #address-cells or #size-cells, to avoid situations where errant ranges push node addresses outside of 32 bits on a 32-bit CPU, for example

[rruuaanng]

I'll think about it. Maybe soon :)

Edit It seems to have been added, I will close this issue. https://docs.zephyrproject.org/apidoc/latest/group__devicetree-reg-prop.html#gac6d8279c32351ced4c0ac7f32270974e

[rruuaanng]

They already exist in zephyr. So I closed them.

[57300]

They already exist in zephyr. So I closed them.

No they aren't. The macro DT_REG_ADDR_RAW was taken in #78766 for a different purpose than described in this RFC. Now, to implement a macro for an address with no ranges-translation, another macro name needs to be chosen.

[rruuaanng]

Sorry, maybe I didn't notice. If there are no other questions, I will implement it （I don't seem to understand what RFC needs. Maybe you can give a more accurate description. For example, the macro name you expect.）

[rruuaanng]

I will launch a PR about DT_FOREACH_ANCESTOR in the next few days. @57300 You can come and review it when you have time!

Edit My understanding of it: Get all the ancestor nodes of a given node, similar to DT_FOREACH_CHILD.