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

[Moteen Shah]

Hey Simon and Quentin,

Should I send one RFC for this for folks not using binman?

Regards,
Moteen

On 01/04/25 21:12, Simon Glass wrote:
> Hi Manorit,
>
> On Tue, 1 Apr 2025 at 18:04, Manorit Chawdhry <m-chawdhry at ti.com> wrote:
>> Hi Quentin,
>>
>> On 15:46-20250328, Quentin Schulz wrote:
>>> Hi Moteen,
>>>
>>> On 3/27/25 9:06 AM, Moteen Shah 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):
>>>> +    """Propagates bootph-* property to all the parent
>>>> +    nodes up the hierarchy
>>>> +    """
>>>> +    parent = node.parent
>>>> +    if parent == None or parent.props.get(prop):
>>>> +       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.
>>>> +    """
>>>> +    bootph_prop = ['bootph-all', 'bootph-some-ram', 'bootph-pre-ram', 'bootph-pre-sram']
>>>> +
>>> This should only be done for U-Boot proper DTB I believe, otherwise it'll
>>> cause FDT size increase for TPL/SPL/VPL/whatever where we may be extremely
>>> size-constrained. Furthermore, TPL/SPL/VPL should not need this since they
>>> get a subset of the actual full DTB (see scripts/Makefile.lib and
>>> fdtgrep_props), only containing the DT node that are relevant for the stage
>>> in question. The only exception I can think of right now is if we use
>>> bloblist or any other mechanism having the full DT passed from stage to
>>> stage.
>>>
>>> Maybe the solution is to have it as part of tools/binman/etype/u_boot_dtb.py
>>> only?
>>>
>>> Also, I imagine we want to support this for systems which do not use binman
>>> (yet?)?
>> I think if we really want to unblock platforms that are facing this then
>> maybe the v1 of this patchset along with the v2 can be merged. ( keeping
>> v1 config as disabled by default for now ). This can help the platforms
>> wanting to fix this issue without binman by enabling that config in
>> their defconfig ( ofcourse with a boot time penalty which would force them to
>> migrate to binman ). Though not sure if this is the right way to go
>> about it but just a suggestion.
> Yes that seems like a good idea to me.
>
> Regards,
> Simon