Attention is currently required from: Douglas Anderson, Yu-Ping Wu.
Hello Douglas Anderson, Yu-Ping Wu,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/85964?usp=email
to review the following change.
Change subject: commonlib/device_tree: Return cells properties of parent node ......................................................................
commonlib/device_tree: Return cells properties of parent node
In Flattened Device Trees, there exist special properties called `#address-cells` and `#size-cells` that determine how large addresses and sizes in `reg` properties are. According to the FDT specification, each `reg` node cares about the `...cells` property in the _parent_ of its node. Our current implementation looks for those properties in the node it finds and returns, which would presumably be the node with the `reg` property itself. Therefore, we're returning the wrong `...cells` values.
This isn't really a problem in practice because we also allow inheriting these properties from the parent when they don't exist in the child, and nodes that contain `reg` properties usually don't contain `...cells` properties themselves (because those properties would be incorrect and useless there), so we usually just end up falling back to the (correct) value we inherited from the parent. But it's still better to just fix the mistake, and if we ever happen to have a situation where the node containing the `reg` property still has children that require different `...cells` values as well, it could make a difference. (The fact that we're inheriting these properties is also technically incorrect according to the spec, but we're doing that intentionally to match behavior in the Linux kernel.)
This issue was already correctly implemented in the recently added fdt_find_node(), and this patch also fixes it in the older dt_find_node().
Change-Id: I323066477a4d4be17225e0915a81ce2ff39c1e40 Signed-off-by: Julius Werner jwerner@chromium.org --- M src/commonlib/device_tree.c 1 file changed, 4 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/85964/1
diff --git a/src/commonlib/device_tree.c b/src/commonlib/device_tree.c index b2daac9..001633b 100644 --- a/src/commonlib/device_tree.c +++ b/src/commonlib/device_tree.c @@ -1059,12 +1059,13 @@ { struct device_tree_node *node, *found = NULL;
- /* Update #address-cells and #size-cells for this level. */ - dt_read_cell_props(parent, addrcp, sizecp); - if (!*path) return parent;
+ /* Update #address-cells and #size-cells for the parent level (cells + properties always count for the direct children of their node) . */ + dt_read_cell_props(parent, addrcp, sizecp); + /* Find the next node in the path, if it exists. */ list_for_each(node, parent->children, list_node) { if (!strcmp(node->name, *path)) {