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/aa8cbce4_6b108535 : PS18, Line 175: ""
That would mean that all functions that work on the properties of a node always get the offset to the start of the properties instead of the offset to the node. Seems weird to be honest since you usually traverse all properties from a given node to get the property you want. I am also not sure if the naming scheme is correct then.
I thought you might want to avoid parsing the name twice for each level, because when you have a function that looks for a name the caller isn't interested in reading the name anymore afterwards. If you don't mind that inefficiency it's easy to adapt my suggestion to return the offset to the start of the node instead. (Same for the argument that you pass into `fdt_find_node()`, passing the property offset avoids reading the name in both the caller and callee instance of the function, but if you prefer passing the node offset for aesthetic reasons it's easy to write it that way as well.)
The idea originally was that you could continue the search from a given node without having to traverse all children nodes again.
Okay, I see. But it sounds like we agree that that doesn't make sense anymore after `fdt_find_subnodes_by_prefix()` became a separate function? So let's design `fdt_find_node()` such that it is as equivalent as possible in what it does to `dt_find_node()`, I think that makes understanding the design much easier.
Would `skip_name` imply that that the offset given to `fdt_skip_node()` points to the first property of the node instead of the name?
Yes, that was my idea, just make it skip the `fdt_next_node_name()` call. Anyway, I mostly proposed this because it sounded like you were very interested in maximizing performance for this code, but I understand now that you were referring to something else with that. If you don't think a few extra `strlen()` here and there matter, feel free to ignore those microoptimzations, I don't care that much either way.