davidchisnall / dtc Goto Github PK

Hello,

It seems like DTC is not able to handle top level /delete-node/ directives:

converting /usr/src/sys/contrib/device-tree/src/arm64/rockchip/rk3326-odroid-go2.dts -> /usr/obj/usr/src/arm64.aarch64/sys/GENERIC/modules/usr/src/sys/modules/dtb/rockchip/rk3326-odroid-go2.dtb
Error at :2260:1: Expected { to start new device tree node.

/delete-node/ &dsi_in_vopl;
^
Error at :2260:12: Error parsing property. Expected property value
/delete-node/ &dsi_in_vopl;
^
Failed to parse tree.
*** [rockchip/rk3326-odroid-go2.dtb] Error code 1

This is from rk3326-odroid-go2.dts which includes rk3326.dtsi that contains the offending directives.

When trying to compiling u-boot-pine64 with this DTC I have :

./"board/sunxi/mksunxi_fit_atf.sh"
arch/arm/dts/sun50i-a64-pine64.dtb arch/arm/dts/sun50i-a64-pine64-plus.dtb > u-boot.its
MKIMAGE u-boot.itb
Error at u-boot.its:18:60: Cannot open binary include file
data = /incbin/("/usr/local/share/atf-allwinner/bl31.bin");
^
Error at u-boot.its:18:60: Failed to find root node /.
data = /incbin/("/usr/local/share/atf-allwinner/bl31.bin");
^
Failed to parse tree.
./tools/mkimage: Can't read u-boot.itb.tmp: Invalid argument

Obviously /usr/local/share/atf-allwinner/bl31.bin works and when setting dtc to the gpl dtc it works.

e.g.

input_buffer.cc:228:6: error: 'precedence' overrides a member function but is
      not marked 'override' [-Werror,-Winconsistent-missing-override]
        int precedence()
            ^
input_buffer.cc:157:14: note: overridden virtual function is here
        virtual int precedence() = 0;
                    ^
input_buffer.cc:444:11: error: redundant move in return statement
      [-Werror,-Wredundant-move]
                        return std::move(lhs);
                               ^
input_buffer.cc:444:11: note: remove std::move call here
                        return std::move(lhs);
                               ^~~~~~~~~~   ~

If there is a reference to a non-unique label dtc will complain, then crash. An example is:

/dts-v1/;

/ {
    baz {
        ref = <&bar0>;
    };

    bar0: bar {
    };

    bar0: bar {
    };
};

GNU dtc complains on this:

ERROR (duplicate_node_names): Duplicate node name /bar
ERROR: Input tree has errors, aborting (use -f to force output)

The -f option seems to give some output, but I don't think we need to copy it.

This can be used to delete a node, e.g. if a file is included, delete an unneeded node.

A simple example is:

/dts-v1/;

/ {
    foo {
    };

    bar {
    };
};

/ {
    /delete-node/ bar;
};

GNU dtc will then transform this to:

/dts-v1/;

/ {

    foo {
    };
};

This seems to be invalid for GNU dtc:

/dts-v1/;

/ {
    foo {
    };

    bar {
    };
    /delete-node/ bar;
};

In input_buffer.cc

. . .
template
struct divmod : public binary_operator<5, T>
{
using binary_operator<5, T>::binary_operator;
using binary_operator_base::result;
result operator()() override
. . .

needs to have a "typename" added:

. . .
template
struct divmod : public binary_operator<5, T>
{
using binary_operator<5, T>::binary_operator;
using typename binary_operator_base::result;
result operator()() override
. . .

This was discovered by my attempt to build head -r320133
for powerpc64 via devel/powerpc64-xtoolchain-gcc (and
so devel/powerpc64-gcc, which is at 6.3.0 as I remember).
After reporting it on the lists I was asked to report it
upstream (here).

Adding the "typename" to the source I was building allowed
a rebuild to complete.

This is like /delete-node/, but for properties. A test case would look like:

/dts-v1/;

/ {
    foo {
        bar;
    };
};

/ {
    foo {
        /delete-property/ bar;
    };
};

This would transform to:

/dts-v1/;

/ {

    foo {
    };
};

--- rpi.dtb ---
Generating rpi.dtb from rpi.dts
converting rpi.dts -> /scratch/tmp/emaste/obj/arm.armv6/scratch/tmp/emaste/freebsd/sys/GENERIC/modules/scratch/tmp/emaste/freebsd/sys/modules/dtb/rpi/rpi.dtb
Error on line 1214: Expected node name
  interrupt-affinity = <&{/cpus/cpu@0}>,
                         ^
Error on line 1214: Expected ; at end of property
  interrupt-affinity = <&{/cpus/cpu@0}>,
                         ^
Failed to parse tree.  Unhappy face!
*** [tegra124-jetson-tk1-fbsd.dtb] Error code 1

From sys/gnu/dts/arm/tegra124.dtsi:

                interrupt-affinity = <&{/cpus/cpu@0}>,
                                     <&{/cpus/cpu@1}>,
                                     <&{/cpus/cpu@2}>,
                                     <&{/cpus/cpu@3}>;

Dtc should filter division by zero, e.g.

/dts-v1/;

/ {
    foo = <(1/0)>;
};

GPL dtc have incbin directive that include a binary file
It's used by u-boot to generate image from .its file (FIT images) on aarch64.

Coverity reports the following:

*** CID 1385590:  Memory - illegal accesses  (WRAPPER_ESCAPE)
/usr.bin/dtc/fdt.cc: 1388 in dtc::fdt::device_tree::resolve_cross_references(unsigned int &)()
1382                                    path += node_name;
1383                                    if (node_address != string())
1384                                    {
1385                                            path += '@';
1386                                            path += node_address;
1387                                    }
>>>     CID 1385590:  Memory - illegal accesses  (WRAPPER_ESCAPE)
>>>     Using internal representation of destroyed object local "next".
1388                                    target = next;
1389                                    if (target == nullptr)
1390                                    {
1391                                            break;
1392                                    }
1393                            }

I'm not immediately sure what it's wanting here. I guess it might be upset because next is being assigned in the loop above to c.get() and c is local to the loop?

The following type of thing has been seen in Linux dts files:

/dts-v1/;

/ {

    bar0: bar1: bar {
    };
};

Line:

error: 'auto' not allowed in lambda parameter
                        auto expect = [&](auto token, const char *msg)
                                          ^~~~
1 error generated.
*** Error code 1

Stop.

Maybe, in the meantime, change it to int or change the Makefile to CXXSTD= c++14?

e.g. there are many places in the Linux DTS files where they have something like: /delete-node/ cpu@1;

The dts may contain output from cpp to indicate the source line and file. dtc should either parse this, or ignore it.

An example of this would be:

#1 "src/arm64/al/alpine-v2-evp.dts"
#1 "<built-in>" 1
  1 "src/arm64/al/alpine-v2-evp.dts" 2
#35 "src/arm64/al/alpine-v2-evp.dts"
#1 "src/arm64/al/alpine-v2.dtsi" 1
#35 "src/arm64/al/alpine-v2.dtsi"
/dts-v1/;

This causes dtc to complain with:

Error on line 1: Expected /dts-v1/; version string
#1 "src/arm64/al/alpine-v2-evp.dts"
^
Error on line 1: Failed to find root node /.
#1 "src/arm64/al/alpine-v2-evp.dts"
^
Error on line 1: Failed to find root node /.
#1 "src/arm64/al/alpine-v2-evp.dts"
^
Failed to parse tree.  Unhappy face!

dtb.cc uses exit(1). However, it doesn't include stdlib.h. Depending on what namespace pollution you have, this may or may not work by accident.

https://svnweb.freebsd.org/base?view=revision&revision=353961

fixes this. It's unclear to me the best way to submit this to you, so I'll start here but can shift to another method if you'd like.

Warner

As of FreeBSD r315031 (freebsd/freebsd-src@37c58cd) FreeBSD tools pass -@ for FDT overlay.

Hello!

If given -s option without a valid file, we get a segfault as of the following line presumably because we have NULL root to sort: https://github.com/davidchisnall/dtc/blob/master/fdt.hh#L907

Reproduction: dtc -I dts -O dts -s /tmp/avklsdo

When converting a dtb (converted via this dtc) to a dts with gpl dtc there is a lot of errors :

: ERROR (duplicate_property_names): Duplicate property name fixup in /local_fixups

Our dtc(1) writes an invalid /__local_fixups__ node, though I do like our consistent format better. =) Right now, we write a bunch of fix-up="path:name:offset" values, but it should be what looks like recreating the path structure to all nodes that need local fixups in the node. The libfdt reading code: https://github.com/dgibson/dtc/blob/master/libfdt/fdt_overlay.c#L231

Edited to add:

Precisely, this is the difference:

BSD dtc(1) [Incorrect]

	__local_fixups__ {

		fixup = "/fragment@1/__overlay__/foobar:foo:0";
		fixup = "/fragment@1/__overlay__/foobar:bar:0";
		fixup = "/fragment@1/__overlay__/foobar:foobar:0";
		fixup = "/fragment@1/__overlay__/foobar:foobar:4";
		fixup = "/fragment@1/__overlay__/foobar:foobar:12";
	};

GPL dtc(1) [Correct]

	__local_fixups__ {

		fragment@1 {

			__overlay__ {

				foobar {

					foo = <0x0>;
					bar = <0x0>;
					foobar = <0x0 0x4 0xc>;
				};
			};
		};
	};

The property values in /__local_fixups__ describing the offsets that need fixups.

Some dts files have a /include/ withing a node, e.g.:

/ {
  foo {
    compatible = "simple-bus";
    /include/ "bar.dtsi"
  };
};

An example of this from Linux is https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi where the smb0 node includes amd-seattle-clks.dtsi.

Almost all the Allwinner's dts have a node 'soc@01c00000' under which every device sits in.
When expanding the reference (for example in aliases for serial0 or ethernet0) the result is this :

aliases {

                ethernet0 = "/soc@01c00000", "/ethernet@01c50000";
                serial0 = "/soc@01c00000", "/serial@01c28000";
        };

Removing the '@01c00000' in the node name result in the correct path :

aliases {

                ethernet0 = "/soc/ethernet@01c50000";
                serial0 = "/soc/serial@01c28000";
        };

Multiple -i directories may be passed, to search each of them for an /include/d file. When the file is found in a later -i directory but not . or one of the earlier ones, a warning is emitted for the earlier paths searched.

E.g. omap44xx-clocks.dtsi exists in sys/gnu/dts/arm, the 2nd -i directory, and dtc outputs:

Unable to open file './omap44xx-clocks.dtsi'.  No such file or directory
Unable to open file 'sys/boot/fdt/dts/arm/omap44xx-clocks.dtsi'.  No such file or directory

The output is produced successfully though.

The following seems to be common in Linux dts files:

/dts-v1/;

/ {
    foo@0 {
    };

    bar {
        ref = <&{/foo@0}>;
    };
};

We fail with:

Failed to find node with label: /foo@0
Failed to parse tree.

We also incorrectly parse this:

/dts-v1/;

/ {
    foo@0 {
    };

    bar {
        ref = <&{/foo}>;
    };
};

It appears, that STM uses char literals in their DTS to resolve GPIO id:

https://github.com/freebsd/freebsd-src/blob/a69b6af2024fdd501b4bbc674092fb2b6d466364/sys/contrib/device-tree/src/arm/stm32mp15-pinctrl.dtsi#L11

When compiling corresponding DTB under FBSD source tree, dtc fails with the following errors:

root@fbsd:/usr/src # make builddtb FDT_DTS_FILE=stm32mp157c-ev1.dts
converting stm32mp157c-ev1.dts -> /root/world/usr/src/arm.armv7/sys/STM32MP1//stm32mp157c-ev1.dtb
Error at <standard input>:1615:19: Expected numbers in array of cells
   pinmux = <(((((('F') - 'A') * 0x10 + (12))) << 8) | (0x11))>;
                   ^
Error at <standard input>:1615:19: Expected ; at end of property
   pinmux = <(((((('F') - 'A') * 0x10 + (12))) << 8) | (0x11))>;
                   ^
Failed to parse tree.
*** Error code 1

Stop.
make[1]: stopped in /usr/src
*** Error code 1

Stop.
make: stopped in /usr/src