diff mbox series

[v2,15/16] dtoc: Check that a parent is not missing

Message ID 20210424165534.v2.15.I11cd6003245319bf9c867b3b6e0feadb1a5d09b1@changeid
State New
Delegated to: Bin Meng
Headers show
Series misc: Some more misc patches | expand

Commit Message

Simon Glass April 24, 2021, 4:56 a.m. UTC
With of-platdata-inst we want to set up a reference to each devices'
parent device, if there is one. If we find that the device has a parent
(i.e. is not a root node) but it is not in the list of devices being
written, then we cannot create the reference.

Report an error in this case, since it indicates that the parent node
is either missing a compatible string, is disabled, or perhaps does not
have any properties because it was not tagged for SPL.

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

(no changes since v1)

 tools/dtoc/dtb_platdata.py             |  9 ++++++++
 tools/dtoc/test/dtoc_test_noparent.dts | 32 ++++++++++++++++++++++++++
 tools/dtoc/test_dtoc.py                | 10 ++++++++
 3 files changed, 51 insertions(+)
 create mode 100644 tools/dtoc/test/dtoc_test_noparent.dts

Comments

Bin Meng May 10, 2021, 5:52 a.m. UTC | #1
Hi Simon,

On Sat, Apr 24, 2021 at 1:00 PM Simon Glass <sjg@chromium.org> wrote:
>
> With of-platdata-inst we want to set up a reference to each devices'
> parent device, if there is one. If we find that the device has a parent
> (i.e. is not a root node) but it is not in the list of devices being
> written, then we cannot create the reference.
>
> Report an error in this case, since it indicates that the parent node
> is either missing a compatible string, is disabled, or perhaps does not
> have any properties because it was not tagged for SPL.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v1)
>
>  tools/dtoc/dtb_platdata.py             |  9 ++++++++
>  tools/dtoc/test/dtoc_test_noparent.dts | 32 ++++++++++++++++++++++++++
>  tools/dtoc/test_dtoc.py                | 10 ++++++++
>  3 files changed, 51 insertions(+)
>  create mode 100644 tools/dtoc/test/dtoc_test_noparent.dts
>
> diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py
> index 1374f01c707..c303224e295 100644
> --- a/tools/dtoc/dtb_platdata.py
> +++ b/tools/dtoc/dtb_platdata.py
> @@ -749,6 +749,15 @@ class DtbPlatdata():
>                      break
>
>          if node.parent and node.parent.parent:
> +            if node.parent not in self._valid_nodes:
> +                # This might indicate that the parent node is not in the
> +                # SPL/TPL devicetree but the child is. For example if we are
> +                # dealing with of-platdata in TPL, the parent has a
> +                # u-boot,dm-spl tag but the child has u-boot,dm-pre-reloc. In

Should this be: "u-boot,dm-tpl"?

> +                # this case the child node exists in TPL but the parent does
> +                # not.
> +                raise ValueError("Node '%s' requires parent node '%s' but it is not in the valid list" %
> +                                 (node.path, node.parent.path))
>              self.buf('\t.parent\t\t= DM_DEVICE_REF(%s),\n' %
>                       node.parent.var_name)
>          if priv_name:
> diff --git a/tools/dtoc/test/dtoc_test_noparent.dts b/tools/dtoc/test/dtoc_test_noparent.dts
> new file mode 100644
> index 00000000000..5a820301e72
> --- /dev/null
> +++ b/tools/dtoc/test/dtoc_test_noparent.dts
> @@ -0,0 +1,32 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Test device tree file for dtoc
> + *
> + * Copyright 2017 Google, Inc
> + */
> +
> +/dts-v1/;
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +       i2c@0 {
> +               compatible = "sandbox,i2c";
> +               u-boot,dm-pre-tpl;

I can't find any reference to this "u-boot,dm-pre-tpl" in the source tree.

> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +               spl-test {
> +                       u-boot,dm-pre-reloc;
> +                       compatible = "sandbox,spl-test";
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       status = "disabled";
> +                       pmic@9 {
> +                               compatible = "sandbox,pmic";
> +                               u-boot,dm-pre-reloc;
> +                               reg = <9>;
> +                               low-power;
> +                       };
> +               };
> +       };
> +};
> diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py
> index a05e3d9ed65..e7397ae554b 100755
> --- a/tools/dtoc/test_dtoc.py
> +++ b/tools/dtoc/test_dtoc.py
> @@ -1839,3 +1839,13 @@ U_BOOT_DRVINFO(spl_test2) = {
>          dtb_file = get_dtb_file('dtoc_test_single_reg.dts')
>          output = tools.GetOutputFilename('output')
>          self.run_test(['struct'], dtb_file, output)
> +
> +    def test_missing_parent(self):
> +        """Test detection of a parent node with no properties"""
> +        dtb_file = get_dtb_file('dtoc_test_noparent.dts', capture_stderr=True)
> +        output = tools.GetOutputFilename('output')
> +        with self.assertRaises(ValueError) as exc:
> +            self.run_test(['device'], dtb_file, output, instantiate=True)
> +        self.assertIn("Node '/i2c@0/spl-test/pmic@9' requires parent node "
> +                      "'/i2c@0/spl-test' but it is not in the valid list",
> +                      str(exc.exception))
> --

Regards,
Bin
diff mbox series

Patch

diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py
index 1374f01c707..c303224e295 100644
--- a/tools/dtoc/dtb_platdata.py
+++ b/tools/dtoc/dtb_platdata.py
@@ -749,6 +749,15 @@  class DtbPlatdata():
                     break
 
         if node.parent and node.parent.parent:
+            if node.parent not in self._valid_nodes:
+                # This might indicate that the parent node is not in the
+                # SPL/TPL devicetree but the child is. For example if we are
+                # dealing with of-platdata in TPL, the parent has a
+                # u-boot,dm-spl tag but the child has u-boot,dm-pre-reloc. In
+                # this case the child node exists in TPL but the parent does
+                # not.
+                raise ValueError("Node '%s' requires parent node '%s' but it is not in the valid list" %
+                                 (node.path, node.parent.path))
             self.buf('\t.parent\t\t= DM_DEVICE_REF(%s),\n' %
                      node.parent.var_name)
         if priv_name:
diff --git a/tools/dtoc/test/dtoc_test_noparent.dts b/tools/dtoc/test/dtoc_test_noparent.dts
new file mode 100644
index 00000000000..5a820301e72
--- /dev/null
+++ b/tools/dtoc/test/dtoc_test_noparent.dts
@@ -0,0 +1,32 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Test device tree file for dtoc
+ *
+ * Copyright 2017 Google, Inc
+ */
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+	i2c@0 {
+		compatible = "sandbox,i2c";
+		u-boot,dm-pre-tpl;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		spl-test {
+			u-boot,dm-pre-reloc;
+			compatible = "sandbox,spl-test";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+			pmic@9 {
+				compatible = "sandbox,pmic";
+				u-boot,dm-pre-reloc;
+				reg = <9>;
+				low-power;
+			};
+		};
+	};
+};
diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py
index a05e3d9ed65..e7397ae554b 100755
--- a/tools/dtoc/test_dtoc.py
+++ b/tools/dtoc/test_dtoc.py
@@ -1839,3 +1839,13 @@  U_BOOT_DRVINFO(spl_test2) = {
         dtb_file = get_dtb_file('dtoc_test_single_reg.dts')
         output = tools.GetOutputFilename('output')
         self.run_test(['struct'], dtb_file, output)
+
+    def test_missing_parent(self):
+        """Test detection of a parent node with no properties"""
+        dtb_file = get_dtb_file('dtoc_test_noparent.dts', capture_stderr=True)
+        output = tools.GetOutputFilename('output')
+        with self.assertRaises(ValueError) as exc:
+            self.run_test(['device'], dtb_file, output, instantiate=True)
+        self.assertIn("Node '/i2c@0/spl-test/pmic@9' requires parent node "
+                      "'/i2c@0/spl-test' but it is not in the valid list",
+                      str(exc.exception))