Hello Rob,
On Tue, Jul 12, 2022 at 09:53:57AM -0600, Rob Herring wrote:
> On Wed, Jun 29, 2022 at 3:20 PM Uwe Kleine-König <ukleinek@xxxxxxxxxx> wrote:
> >
> > Hello,
> >
> > I want to apply an overlay to a device tree that is compiled without -@
> > because that's how dtbs are shipped by the Debian kernel package.
>
> Does applying this overlay work as expected if -@ is used? I would
> imagine so as there is an assumption the base dtb was compiled with
> -@.
No it doesn't. I created a minimal (oh well, at least small) reproducer:
uwe@taurus:~/gsrc/oftreemerge$ cat base.dts
/dts-v1/;
/ {
#address-cells = <1>;
#size-cells = <1>;
node_a: a {
prop = "blub";
};
node_b: b {
a = <&node_a>;
};
};
uwe@taurus:~/gsrc/oftreemerge$ cat overlay.dts
/dts-v1/;
/plugin/;
/ {
fragment@0 {
target-path = "/";
__overlay__ {
node_a: a {
};
c {
a = <&node_a>;
};
};
};
};
When compiling without -@ (as reported initially) I get:
uwe@taurus:~/gsrc/oftreemerge$ dtc -I dts -O dtb -o base.dtb base.dts
uwe@taurus:~/gsrc/oftreemerge$ dtc -I dts -O dtb -o overlay.dtbo overlay.dts
uwe@taurus:~/gsrc/oftreemerge$ fdtoverlay -i base.dtb -o patched.dtb overlay.dtbo
uwe@taurus:~/gsrc/oftreemerge$ fdtdump patched.dtb
...
/ {
#address-cells = <0x00000001>;
#size-cells = <0x00000001>;
c {
a = <0x00000002>;
};
a {
prop = "blub";
phandle = <0x00000002>;
};
b {
a = <0x00000001>;
};
};
So b.a gets broken as the phandle for a got updated.
When doing the same with -@ the result is identical (apart from the
expected changes: more and different phandles, __symbols__ node etc):
uwe@taurus:~/gsrc/oftreemerge$ dtc -@ -I dts -O dtb -o base.dtb base.dts
uwe@taurus:~/gsrc/oftreemerge$ dtc -@ -I dts -O dtb -o overlay.dtbo overlay.dts
uwe@taurus:~/gsrc/oftreemerge$ fdtoverlay -i base.dtb -o patched.dtb overlay.dtbo
uwe@taurus:~/gsrc/oftreemerge$ fdtdump patched.dtb
...
/ {
#address-cells = <0x00000001>;
#size-cells = <0x00000001>;
c {
a = <0x00000003>;
};
a {
prop = "blub";
phandle = <0x00000003>;
};
b {
a = <0x00000001>;
phandle = <0x00000002>;
};
__symbols__ {
node_a = "/a";
node_b = "/b";
};
};
b.a still points to a non-existing node.
> The fix is to fix Debian dtbs
One of the blockers for that is that adding -@ to the kernel default
rules was rejected more than once.
(https://lore.kernel.org/linux-arm-kernel/CAK7LNAS5t1wew0MMFjdB5HGCAMerhU7pAGiFhcTtCRUAAjGLpw@xxxxxxxxxxxxxx/)
> (or don't use them as dtbs should come from firmware rather than
> distros).
In this case the distro provides the firmware, too. So well, you could
argue the dtbs should (in this case) be provided by the raspi-firmware
package and not the kernel image package, but that only makes things
harder to handle, because the kernel is effectively the upstream for the
device tree files.
> There might be sufficient information to make your case work because
> IIRC all the (resolved) overlay phandle values get adjusted when
> applied as they could collide with the base tree phandle values.
> However, doing so would mean users may start working around base DTs
> compiled without -@. That would remove labels being an ABI which I
> don't love, but it would also remove the abstraction that labels
> provide.
My point of view is a bit different. It would allow to practically apply
overlays to device trees without -@. It doesn't limit in any way the
semantic of labels in the case where -@ is used. I can accept we're
differing in that point and I don't think working on reaching an
agreement here is time spend well.
> Certainly, the base phandle value shouldn't just be silently overwritten.
We agree here, which is good enough for me.
Fixing that is a bit complicated, because currently fdtoverlay just
applies a fixed offset to the phandle values defined in the overlay. I
think it would need another pass over the device tree to determine
already existing phandles and maintaining a mapping for these.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
