[RFC PATCH 1/2 v2] tools: binman: control.py: Propagate bootph-* properties to supernodes

[Simon Glass]

Hi Moteen,

On Thu, 27 Mar 2025 at 08:06, Moteen Shah <m-shah at ti.com> wrote:
>
> Add a function to scan through all the nodes in the device-tree
> recusively for bootph-* property. If found, propagate it to all
> of its parent nodes up the hierarchy.
>
> Signed-off-by: Moteen Shah <m-shah at ti.com>
> ---
>  tools/binman/control.py | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/tools/binman/control.py b/tools/binman/control.py
> index e73c598298c..e739949d366 100644
> --- a/tools/binman/control.py
> +++ b/tools/binman/control.py
> @@ -526,6 +526,35 @@ def _RemoveTemplates(parent):
>          if node.name.startswith('template'):
>              node.Delete()
>
> +def prop_bootph_to_parent(node, prop, dtb):

I think the 'prop_' is a bit confusing, since you are dealing with
properties! How about 'add_' as the prefix?

> +    """Propagates bootph-* property to all the parent
> +    nodes up the hierarchy

First line should be a summary

then blank line

then describe the args...you can see this if you look at a few other functions.

> +    """
> +    parent = node.parent
> +    if parent == None or parent.props.get(prop):

if not parent or ...

> +       return
> +
> +    while parent:
> +        parent.AddEmptyProp(prop, 0)
> +        parent = parent.parent
> +
> +def scan_and_prop_bootph(node, dtb):
> +    """Scan the device tree and set the bootph-* property if its present
> +    in subnode
> +
> +    This is used to set the bootph-* property in the parent node if a
> +    "bootph-*" property is found in any of the subnodes of the parent
> +    node.

comment style again

> +    """
> +    bootph_prop = ['bootph-all', 'bootph-some-ram', 'bootph-pre-ram', 'bootph-pre-sram']

>From my understanding the only ones that matter are bootph-all and
bootph-some-ram, since the SPL ones are handled by fdtgrep.

> +
> +    for props in bootph_prop:

for prop

(since it is just one)

> +        if node.props.get(props):
> +            prop_bootph_to_parent(node, props, dtb)
> +
> +    for subnode in node.subnodes:
> +       scan_and_prop_bootph(subnode, dtb)
> +
>  def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt, use_expanded, indir):
>      """Prepare the images to be processed and select the device tree
>
> @@ -563,7 +592,11 @@ def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt, use_expanded, ind
>          indir = []
>      dtb_fname = fdt_util.EnsureCompiled(dtb_fname, indir=indir)
>      fname = tools.get_output_filename('u-boot.dtb.out')
> -    tools.write_file(fname, tools.read_file(dtb_fname))
> +    dtb = fdt.FdtScan(dtb_fname)
> +    scan_and_prop_bootph(dtb.GetRoot(), dtb)
> +    dtb.Sync(True)
> +    tools.write_file(dtb_fname, dtb.GetContents())
> +    tools.write_file(fname, dtb.GetContents())

You shouldn't write back to the file created by the build. Do you need this?

>      dtb = fdt.FdtScan(fname)
>
>      node = _FindBinmanNode(dtb)
> --
> 2.34.1
>

The code looks fine to me apart from the nits.

This addition needs a test - see ftest.py for some examples there. But
basically just create a dts that has some props in it, then check that
they got added.

I think testSimpleFitEncryptedData() could be a good example?

Regards,
Simon