[XS] Change in coreboot[main]: commonlib/device_tree: Return cells properties of parent node
Attention is currently required from: Douglas Anderson, Maximilian Brune, Paul Menzel, Yu-Ping Wu.
Julius Werner has posted comments on this change by Julius Werner. ( https://review.coreboot.org/c/coreboot/+/85964?usp=email )
Change subject: commonlib/device_tree: Return cells properties of parent node ......................................................................
Patch Set 2:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/85964/comment/982af942_f965e2d8?usp... : PS1, Line 31: recently
Maybe just reference the commit?
Done
File src/commonlib/device_tree.c:
https://review.coreboot.org/c/coreboot/+/85964/comment/fb7917b5_fb77c51d?usp... : PS1, Line 1061:
Sure, it's useless for the purpose of finding a child DT node, but what if old code was using this function as a shortcut for finding the #address-cells and #size-cells of a given node. AKA:
I've looked through all callers (both in coreboot and depthcharge) already, there are very few cases that even care about cells at all and those don't do anything that would be affected by this. In general, cases that only want to probe the current node just call `dt_read_cell_props()` directly.
Maybe it would be nice to make sure props are always initialized to something but I don't think that could be done with the current recursive architecture of the API. We could do it in `dt_find_node_by_path()`, but then that just makes that function act differently from `dt_find_node()` which could also be confusing. Right now, callers seem to generally all initialize the variables before passing them in (although they unfortunately don't always do it to 2/1 like the spec says, but that's their problem), and don't expect the function to write back to them in all cases, which also matches the API documentation.
https://review.coreboot.org/c/coreboot/+/85964/comment/1aa344ec_43291cf4?usp... : PS1, Line 1066: .
Remove the space?
Done
-
Julius Werner (Code Review)