U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] fdt: Fix phandles not copying when using templates
@ 2026-07-29 10:24 Anshul Dalal
  2026-07-29 10:42 ` Quentin Schulz
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Anshul Dalal @ 2026-07-29 10:24 UTC (permalink / raw)
  To: u-boot; +Cc: Tom Rini, Simon Glass, Moteen Shah, Anshul Dalal

The phandles used inside a template are not 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>;
  };
};

With the error 'binman: Node '/binman/output-bin/ti-secure-rom': Cannot
find node for phandle 103' observed.

The test_copy_subnodes_from_phandles was also updated to verify the new
behavior of phandles being copied.

Reviewed-by: Moteen Shah <m-shah@ti.com>
Signed-off-by: Anshul Dalal <anshuld@ti.com>
---
Changes in v3:
- Add the missing 'R/by' tag, that was missed on v2
- Reword commit message
- Fix test_copy_subnodes_from_phandles
- Refactor lines within 80 column length
- Link to v2: https://patch.msgid.link/20260709-binman_template_phandle_copy_fix-v2-1-6c4017eec04d@ti.com

Changes in v2:
- Add 'testTemplatePhandleCopy' binman test
- Change assignment in fdt.py from unused 'dst' to _
- Link to v1: https://patch.msgid.link/20260629-binman_template_phandle_copy_fix-v1-1-196f95a13388@ti.com
---
 tools/binman/ftest.py                           | 12 ++++++++++++
 tools/binman/test/fdt/template_phandle_copy.dts | 22 ++++++++++++++++++++++
 tools/dtoc/fdt.py                               |  2 +-
 tools/dtoc/test_fdt.py                          |  4 ++--
 4 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index bf98b268ac15..5e20b5c78056 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -8552,5 +8552,17 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
         self.assertEqual(len(subnode4.props), 0,
                         "subnode shouldn't have any properties")
 
+    def testTemplatePhandleCopy(self):
+        """Test if phandles are copied properly when inserting template"""
+        _, _, _, fname = self._DoReadFileDtb('fdt/template_phandle_copy.dts')
+        dtb = fdt.Fdt(fname)
+        dtb.Scan()
+        parent = dtb.GetNode("/binman/section@0/section@1")
+        child = parent.FindNode("section@2")
+        parent_phandle = fdt_util.fdt32_to_cpu(child.props["parent"].value)
+        child_phandle = fdt_util.fdt32_to_cpu(parent.props["child"].value)
+        self.assertEqual(parent, dtb.LookupPhandle(parent_phandle))
+        self.assertEqual(child, dtb.LookupPhandle(child_phandle))
+
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/fdt/template_phandle_copy.dts b/tools/binman/test/fdt/template_phandle_copy.dts
new file mode 100644
index 000000000000..5c6782372a48
--- /dev/null
+++ b/tools/binman/test/fdt/template_phandle_copy.dts
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman: binman {
+		some_template: template {
+			parent: section@1 {
+				child = <&child>;
+				child: section@2 {
+					parent = <&parent>;
+				};
+			};
+		};
+
+		section@0 {
+			insert-template = <&some_template>;
+		};
+	};
+};
diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py
index 991a36b98796..64123227d7bf 100644
--- a/tools/dtoc/fdt.py
+++ b/tools/dtoc/fdt.py
@@ -780,7 +780,7 @@ class Node:
             parent = self.GetFdt().LookupPhandle(phandle)
             tout.debug(f'adding template {parent.path} to node {self.path}')
             for node in parent.subnodes.__reversed__():
-                dst = self.copy_node(node)
+                self.copy_node(node, True)
 
             tout.debug(f'merge props from {parent.path} to {self.path}')
             self.merge_props(parent, False)
diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index f141f931a949..a858da127bfb 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -426,10 +426,10 @@ class TestNode(unittest.TestCase):
              '/dest/second', '/dest/existing', '/dest/base'],
             [n.path for n in dst.subnodes])
 
-        # Make sure that the phandle for 'over' is not copied
+        # Make sure that the phandle for 'over' is copied
         over = dst.FindNode('over')
         tout.debug(f'keys: {over.props.keys()}')
-        self.assertNotIn('phandle', over.props.keys())
+        self.assertIn('phandle', over.props.keys())
 
         # Check the merged properties, first the base ones in '/dest'
         expect = {'bootph-all', 'compatible', 'stringarray', 'longbytearray',

---
base-commit: 63f6cc8ba618396cb9c0161bb5c6d217604ae1d0
change-id: 20260629-binman_template_phandle_copy_fix-956bc3ce48d6

Best regards,
--  
Anshul Dalal <anshuld@ti.com>


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] fdt: Fix phandles not copying when using templates
  2026-07-29 10:24 [PATCH v3] fdt: Fix phandles not copying when using templates Anshul Dalal
@ 2026-07-29 10:42 ` Quentin Schulz
       [not found]   ` <DKL91OUU5LDQ.V2B9UQREH116@ti.com>
  2026-08-08 17:33 ` Simon Glass
  2026-08-28 18:07 ` Tom Rini
  2 siblings, 1 reply; 5+ messages in thread
From: Quentin Schulz @ 2026-07-29 10:42 UTC (permalink / raw)
  To: Anshul Dalal, u-boot; +Cc: Tom Rini, Simon Glass, Moteen Shah

Hi Anshul,

On 7/29/26 12:24 PM, Anshul Dalal wrote:
> The phandles used inside a template are not 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>;
>    };
> };
> 

What happens if you insert the template *twice*.

e.g. have:

  &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>;
     };
     output-bin2 {
       insert-template = <&some_template>;
     };
  };

? Gut feeling is that this will fail to build because we now have the 
same label for two different nodes.

Cheers,
Quentin

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] fdt: Fix phandles not copying when using templates
  2026-07-29 10:24 [PATCH v3] fdt: Fix phandles not copying when using templates Anshul Dalal
  2026-07-29 10:42 ` Quentin Schulz
@ 2026-08-08 17:33 ` Simon Glass
  2026-08-28 18:07 ` Tom Rini
  2 siblings, 0 replies; 5+ messages in thread
From: Simon Glass @ 2026-08-08 17:33 UTC (permalink / raw)
  To: anshuld; +Cc: u-boot, u-boot

On 2026-07-29T10:24:57, Anshul Dalal <anshuld@ti.com> wrote:
> fdt: Fix phandles not copying when using templates
>
> The phandles used inside a template are not 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>;
>   };
> };
>
> With the error 'binman: Node '/binman/output-bin/ti-secure-rom': Cannot
> find node for phandle 103' observed.
>
> The test_copy_subnodes_from_phandles was also updated to verify the new
> behavior of phandles being copied.
>
> Reviewed-by: Moteen Shah <m-shah@ti.com>
> Signed-off-by: Anshul Dalal <anshuld@ti.com>
>
> tools/binman/ftest.py                           | 12 ++++++++++++
>  tools/binman/test/fdt/template_phandle_copy.dts | 22 ++++++++++++++++++++++
>  tools/dtoc/fdt.py                               |  2 +-
>  tools/dtoc/test_fdt.py                          |  4 ++--
>  4 files changed, 37 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] fdt: Fix phandles not copying when using templates
       [not found]   ` <DKL91OUU5LDQ.V2B9UQREH116@ti.com>
@ 2026-08-24  7:06     ` Anshul Dalal
  0 siblings, 0 replies; 5+ messages in thread
From: Anshul Dalal @ 2026-08-24  7:06 UTC (permalink / raw)
  To: Anshul Dalal, Quentin Schulz, u-boot, u-boot
  Cc: Tom Rini, Simon Glass, Moteen Shah

I guess this reply got dropped due to the list migration, just a gentle
ping to ask if everyone's okay with this change.

Thanks!

On Mon Aug 10, 2026 at 5:36 PM IST, Anshul Dalal wrote:
> On Wed Jul 29, 2026 at 4:12 PM IST, Quentin Schulz wrote:
>> Hi Anshul,
>>
>> On 7/29/26 12:24 PM, Anshul Dalal wrote:
>>> The phandles used inside a template are not 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>;
>>>    };
>>> };
>>> 
>>
>> What happens if you insert the template *twice*.
>>
>> e.g. have:
>>
>>   &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>;
>>      };
>>      output-bin2 {
>>        insert-template = <&some_template>;
>>      };
>>   };
>>
>> ? Gut feeling is that this will fail to build because we now have the 
>> same label for two different nodes.
>
> Yes, it will cause a failure due to duplicated phandles. And as per the
> docs[1], it seems to be an expected qwirk of how templates work.
>
> [1]: https://docs.u-boot.org/en/v2026.04/develop/package/binman.html#dealing-with-phandles
>
> Regards,
> Anshul


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] fdt: Fix phandles not copying when using templates
  2026-07-29 10:24 [PATCH v3] fdt: Fix phandles not copying when using templates Anshul Dalal
  2026-07-29 10:42 ` Quentin Schulz
  2026-08-08 17:33 ` Simon Glass
@ 2026-08-28 18:07 ` Tom Rini
  2 siblings, 0 replies; 5+ messages in thread
From: Tom Rini @ 2026-08-28 18:07 UTC (permalink / raw)
  To: u-boot, Anshul Dalal; +Cc: Simon Glass, Moteen Shah

On Wed, 29 Jul 2026 15:54:57 +0530, Anshul Dalal wrote:

> The phandles used inside a template are not 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>;
>   };
> };
> 
> [...]

Applied to u-boot/next, thanks!

[1/1] fdt: Fix phandles not copying when using templates
      commit: b8d9223a454374681bf7d0d4384bb101be92110d
-- 
Tom



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-08-28 18:07 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-29 10:24 [PATCH v3] fdt: Fix phandles not copying when using templates Anshul Dalal
2026-07-29 10:42 ` Quentin Schulz
     [not found]   ` <DKL91OUU5LDQ.V2B9UQREH116@ti.com>
2026-08-24  7:06     ` Anshul Dalal
2026-08-08 17:33 ` Simon Glass
2026-08-28 18:07 ` Tom Rini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox