[PATCH] dtoc: Check that a parent is not missing

[Simon Glass]

Hi Walter,

On Sun, 25 Jul 2021 at 18:43, Walter Lozano <wlozano at collabora.com> wrote:
>
> Hi Simon,
>
> Thanks for the patch, it will be a nice way to spot errors and avoid
> headaches!
>
> On 7/7/21 8:22 AM, Simon Glass 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 at chromium.org>
> > ---
> >
> >   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 2d42480a9a5..a951a5a2264 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 at 0 {
> > +             compatible = "sandbox,i2c";
> > +             u-boot,dm-pre-tpl;
>
> Does u-boot,dm-pre-tpl exists? Probably I'm missing something here but I
> was expecting u-boot,dm-spl.

I'm not quite sure what happened here, but I think I corrected it when applying.

This patch has been applied at some point.


>
> > +             #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 at 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 0b2805feed2..863ede90b7a 100755
> > --- a/tools/dtoc/test_dtoc.py
> > +++ b/tools/dtoc/test_dtoc.py
> > @@ -1830,3 +1830,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 at 0/spl-test/pmic at 9' requires parent node "
> > +                      "'/i2c at 0/spl-test' but it is not in the valid list",
> > +                      str(exc.exception))
>

Regards,
Simon