Attention is currently required from: Jakub Czapiga, Martin L Roth, Maximilian Brune.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81081?usp=email )
Change subject: lib/device_tree: Add some FDT helper functions ......................................................................
Patch Set 18:
(1 comment)
File src/lib/device_tree.c:
https://review.coreboot.org/c/coreboot/+/81081/comment/f3346b30_885b27fb : PS18, Line 175: ""
`dt_find_node` and `fdt_find_node` work inherently different. […]
Okay, sorry, I see your point now. I didn't realize that the offset passed here was the start of the child node array instead of the parent node. (BTW it would be good to add `@offset` to the parameter documentation here, because that's really the most confusing one.)
In that case, how about just making the function take both offsets (`parent_node_offset` and `child_array_offset`) as parameters instead? It's a bit redundant but I think it has a number of advantages:
1. It allows you to use NULL-termination instead of a separate count (so still the same amount of total parameters) without having to awkwardly "read-ahead" to check for termination.
2. It removes the need of special-casing the root node, which means you could just rely on `strtok_r()` to automatically split the path string the right way in `fdt_find_node_by_path()`.
3. It keeps the way this works as consistent as possible with `dt_find_node()`, making the whole file easier to follow.
-
Julius Werner (Code Review)