Hi Anshul,
On 2026-07-09T10:24:07, Anshul Dalal <[email protected]> wrote:
> fdt: fix phandles not copying when using templates
>
> The phandles used inside a template were not being copied to the node
> inserting the template, leading to a missing phandle error.
>
> The following example can be used to reproduce the issue:
>
> &binman {
> some_template: template-0 {
> ti-secure-rom {
> content = <&some_data>;
> keyfile = "some_key";
> };
> some_data: blob-ext {
> optional;
> };
> };
> output-bin {
> insert-template = <&some_template>;
> };
> [...]
>
> tools/binman/ftest.py | 17 +++++++++++++++++
> tools/binman/test/fdt/template_phandle_copy.dts | 22 ++++++++++++++++++++++
> tools/dtoc/fdt.py | 2 +-
> 3 files changed, 40 insertions(+), 1 deletion(-)
> fdt: fix phandles not copying when using templates
Please capitalise after the tag ('fdt: Fix ...'), as is usual in U-Boot.
> The phandles used inside a template were not being copied to the node
> inserting the template, leading to a missing phandle error.
Please use present tense ('are not copied') since the code is still in
this state before the commit.
> diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py
> @@ -780,7 +780,7 @@ class Node:
> for node in parent.subnodes.__reversed__():
> - dst = self.copy_node(node)
> + _ = self.copy_node(node, True)
This breaks an existing test in tools/dtoc/test_fdt.py which checks
that the phandle is not copied at this level:
FAIL: test_copy_subnodes_from_phandles
(__main__.TestNode.test_copy_subnodes_from_phandles)
...
AssertionError: 'phandle' unexpectedly found in
dict_keys(['compatible', 'bootph-all', 'reg', 'low-power', 'phandle'])
Please can you update that test to check the new behaviour, i.e. that
the phandle is copied? Since the change is in fdt.py it needs coverage
at that level, as well as the binman functional test. Please run the
dtoc tests before sending v2:
./tools/dtoc/test_fdt.py
The fix itself looks right to me - copy_node() already passes True for
its recursive calls, so the first level was the odd one out, and the
binman docs ('Dealing with phandles') already say that phandles in
templates are copied. It would be worth noting in the
copy_subnodes_from_phandles() docstring that the source nodes must be
removed before the tree is scanned, to avoid duplicate phandles.
There is no need to assign the return value to _ - just call
self.copy_node(node, True) directly.
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> @@ -8552,5 +8552,22 @@ fdt fdtmap Extract the
> devicetree blob from the fdtmap
> + def testTemplatePhandleCopy(self):
> + """Test if phandles are copied properly when inserting template"""
> + _, _, _, out_dtb_fname =
> self._DoReadFileDtb("fdt/template_phandle_copy.dts")
Please use single quotes for strings, as elsewhere in this file, and
keep to 80 columns - this line is 85 characters and the two
assertEqual() calls below also overflow. See the nearby tests for the
usual continuation style, rather than putting the closing parenthesis
on its own line.
Regards,
Simon
