dgibson / dtc

Device Tree Compiler
235 stars 134 forks source link

unit_address_vs_reg: Wrong warning for /memory node #107

Closed tq-steina closed 10 months ago

tq-steina commented 1 year ago

When parsing a .dtb (read from /sys/firmware/fdt) and printing as .dts, I get an invalid warning for /memory node:

$ dtc --version
Version: DTC 1.7.0-gccf1f62d
$ dtc -I dtb -O dts tmp.dtb
<stdout>: Warning (unit_address_vs_reg): /memory: node has a reg or ranges property, but no unit name
[...]

AFAICS this check ensures that the node-name has a unit-addr if reg-Property (or ranges) is present. But this cannot be uphold for /memory according to devicetree spec. Devicetree spec v0.4 section 3.4 (/memory node) states that:

The unit-name component of the node name (see Section 2.2.1) shall be memory.

While table 3.3 ( /memory Node Properties) states that reg property is required. So this check has to exclude /memory node

dgibson commented 1 year ago

The unit-name component of the node name (see Section 2.2.1) shall be memory.

Uh.. doesn't the "unit-name component" specifically mean the bit before the @. In which case the test is correct.

If that really is the intended meaning of that piece of the devicetree spec, it's a departure from IEEE 1275 with no clear reason for it. Traditionally memory nodes absolutely did include unit addresses (e.g. /memory@0). That's important if you have multiple memory nodes which can be used to encode non-contiguous memory space, NUMA nodes or sometimes other differences between blocks of memory.

@glikely, care to weigh in?

tq-steina commented 1 year ago

The unit-name component of the node name (see Section 2.2.1) shall be memory.

Uh.. doesn't the "unit-name component" specifically mean the bit before the @. In which case the test is correct.

True, it doesn't state that there shouldn't be any unit-address.

If that really is the intended meaning of that piece of the devicetree spec, it's a departure from IEEE 1275 with no clear reason for it. Traditionally memory nodes absolutely did include unit addresses (e.g. /memory@0). That's important if you have multiple memory nodes which can be used to encode non-contiguous memory space, NUMA nodes or sometimes other differences between blocks of memory.

@glikely, care to weigh in?

Independent from the intention, there are a lot of devicetrees which do not use a unit-address, but a reg-property. Changing this is not backward compatible, as. e.g. bootloaders need to access this specific node. How to deal with those?

dgibson commented 1 year ago

Independent from the intention, there are a lot of devicetrees which do not use a unit-address, but a reg-property. Changing this is not backward compatible, as. e.g. bootloaders need to access this specific node. How to deal with those?

Either put up with the warnings or suppress them with -W and -E options. The fact that there are many trees that have got this wrong in the past only strengthens the case for having it as a warning in dtc.

Note, however that adding a correct unit address wouldn't necessarily break bootloaders. If they're based on libfdt, then a fdt_path_offset("/memory") will find /memory@0 as long as its the only /memory@* node.