Hi Moteen,
On 5/12/25 1:50 PM, Moteen Shah wrote:
Add a function to scan through all the nodes in the device-tree
for bootph-* property. If found, propagate it to all of its parent
It's not scanning for bootph-* properties. It's scanning for two
specific bootph properties, so try to update the commit log and comments
so it's not misleading.
The commit log is lacking information as to why we need to do this, what
this helps with and why this is specific to bootph-all and bootph-some-ram.
We need to be able to look at this patch in 5 years and have a clue of
why this was required.
nodes up the hierarchy.
Signed-off-by: Moteen Shah <m-s...@ti.com>
Signed-off-by: Simon Glass <s...@chromium.org>
---
tools/binman/control.py | 51 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/tools/binman/control.py b/tools/binman/control.py
index 81f61e3e152..ccf081cb686 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -530,6 +530,54 @@ def _RemoveTemplates(parent):
for node in del_nodes:
node.Delete()
+def propagate_bootph(node, prop):
+ """Propagate bootph-* property to all the parent nodes up the hierarchy
+
Misleading, it's not propagating bootph properties, it's propagating the
prop passed as an argument to the function, so rename to something like
propagate_prop and update the comment.
Be clear also that this is expected to be a boolean property (because of
AddEmptyProp).
+ Args:
+ node (fdt.Node): Node to propagate the property to
Specify that all its parents (recursively) will have this property
propagated to.
+ prop (str): Property to propagate
+
+ Return:
+ True if any change was made, else False
+ """
+ changed = False
+ while node:
+ if prop not in node.props:
+ node.AddEmptyProp(prop, 0)
+ changed = True
+ node = node.parent
+ return changed
+
+def scan_and_prop_bootph(node):
+ """Propagate bootph properties from children to parents
+
+ The bootph schema indicates that bootph nodes in children should be implied
s/nodes/properties/
+ in their parents, all the way up the hierarchy. This is expensive to
+ implement in U-Boot before relocation, so this function explicitly
I think the point here is "at runtime" which is missing.
+ propagates these bootph properties upwards.
at build time :)
+
+ This is used to set the bootph-* property in the parent node if a
+ "bootph-*" property is found in any of the parent's subnodes.
+
This is all misleading, we aren't propagating all bootph properties
+ Args:
+ node (fdt.Node): Node to propagate the property to
Same remarks as for the propagate_bootph method above.
+ prop (str): Property name to propagate
+
+ Return:
+ True if any change was made, else False
+
+ """
+ bootph_prop = ['bootph-all', 'bootph-some-ram']
+
Please explain why those only.
+ changed = False
+ for prop in bootph_prop:
+ if prop in node.props:
+ changed |= propagate_bootph(node.parent, prop)
+
+ for subnode in node.subnodes:
+ changed |= scan_and_prop_bootph(subnode)
+ return changed
+
I have a gut feeling this is suboptimal, though simple and easy to maintain.
We currently go up to the root node for each and every found
bootph-{all,some-ram} property. We could simply divide this by half (or
more if there are more properties to propagate in the future) by passing
the props to propagate instead of just one. We can keep track of which
node has propagated the property already and, for its children, stop the
propagation at that level instead of going up to the root node all the
time. I'm not sure typical Device Trees are deep enough for us to care
about that kind of optimization for now, especially since it'll be
running at build time only?
Cheers,
Quentin
