| Message ID | 20250226-qcom-nonroot-overlays-v1-0-26c6e7605833@trvn.ru (mailing list archive) |
|---|---|
| Headers | show |
| Series | {vision/navigation}-mezzanine: Fix overlay root node | expand |
On Wed, Feb 26, 2025 at 07:29:54PM +0500, Nikita Travkin wrote: > While considering to propose WoA EL2 dt overlays upstream I was looking > at existing overlays and noticed that some of them are broken: they put > seemingly meaningful fixups into the overlay's "/" node, which places > them into the overlay "metadata" itself, not into a fixup fragment to be > applied to the actual dtb. This series fixes those two by changing to > full path "&{/}" which should work as it was initially intended. > > See demonstration of the problem below: > > $ cat base.dts > /dts-v1/; > / { > compatible = "fake,board"; > fake,value = <42>; > }; > > $ cat extra.dtso > /dts-v1/; > /plugin/; > > / { > foo; > bar { baz; }; > }; > &{/} { whatever-comes-next-after-baz; }; > > $ dtc base.dts -o base.dtb > $ dtc extra.dtso -o extra.dtbo > $ fdtoverlay -i base.dtb -o combine.dtb extra.dtbo > $ dtc base.dtb > /dts-v1/; > > / { > compatible = "fake,board"; > fake,value = <0x2a>; > }; > > $ dtc extra.dtbo > /dts-v1/; > > / { > foo; > > bar { > baz; > }; Is this behaviour documented somewhere? I'd say, it would be a surprise to me. > > fragment@0 { > target-path = "/"; > > __overlay__ { > whatever-comes-next-after-baz; > }; > }; > }; > > $ dtc combine.dtb > /dts-v1/; > > / { > whatever-comes-next-after-baz; > compatible = "fake,board"; > fake,value = <0x2a>; > }; > > In the resulting dtb foo bar and baz are missing. > > Signed-off-by: Nikita Travkin <nikita@trvn.ru> > --- > Nikita Travkin (2): > arm64: dts: qcom: qrb5165-rb5-vision-mezzanine: Fix broken overlay root > arm64: dts: qcom: sdm845-db845c-navigation-mezzanine: Fix the overlay root > > arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso | 2 +- > arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > --- > base-commit: 8433c776e1eb1371f5cd40b5fd3a61f9c7b7f3ad > change-id: 20250226-qcom-nonroot-overlays-bfe21d33be8c > > Best regards, > -- > Nikita Travkin <nikita@trvn.ru> >
Dmitry Baryshkov писал(а) 27.02.2025 09:16: > On Wed, Feb 26, 2025 at 07:29:54PM +0500, Nikita Travkin wrote: >> While considering to propose WoA EL2 dt overlays upstream I was looking >> at existing overlays and noticed that some of them are broken: they put >> seemingly meaningful fixups into the overlay's "/" node, which places >> them into the overlay "metadata" itself, not into a fixup fragment to be >> applied to the actual dtb. This series fixes those two by changing to >> full path "&{/}" which should work as it was initially intended. >> >> See demonstration of the problem below: >> [...] >> $ dtc extra.dtbo >> /dts-v1/; >> >> / { >> foo; >> >> bar { >> baz; >> }; > > Is this behaviour documented somewhere? I'd say, it would be a surprise > to me. > According to dtc docs [1], 3.b) The Device Tree fragments must be compiled with the same option but they must also have a tag (/plugin/) that allows undefined references to nodes that are not present at compilation time to be recorded so that the runtime loader can fix them. so per my understanding "plugin" directive only changes the meaning of references (i.e. stuff with "&"), to generate fragments/fixups, which are the only way libfdt combines overlays into the base dtb. I suppose the old way of "manually" writing fragments (and thus writing to / as raw nodes) was kept because phandle/path based updates were added later to dtc and many overlays were still defining raw fragments... "/" also allows one to write "raw" nodes into the overlay, which is sometimes used by downstreams. (i.e. they put custom extensions to the overlay format [2] or add metadata into / of the dtbo like "compatible" values to reject incompatible overlays from applying. [3]) This is actually why I started looking here in the first place as for woa el2 overlays I was asked to add compatible metadata as, apparently, NixOS tooling requires it to validate the overlays [4]. [1] https://web.git.kernel.org/pub/scm/utils/dtc/dtc.git/tree/Documentation/dt-object-internal.txt#n120 [2] https://github.com/raspberrypi/linux/blob/rpi-6.6.y/arch/arm/boot/dts/overlays/adafruit-st7735r-overlay.dts#L73 [3] https://github.com/radxa-pkg/radxa-overlays/blob/main/arch/arm64/boot/dts/rockchip/overlays/rk3588-i2c0-m1.dts#L5 [4] https://github.com/TravMurav/slbounce/blob/main/dtbo/x1e-el2.dtso#L12 >> >> fragment@0 { >> target-path = "/"; >> >> __overlay__ { >> whatever-comes-next-after-baz; >> }; >> }; >> }; >> >> $ dtc combine.dtb >> /dts-v1/; >> >> / { >> whatever-comes-next-after-baz; >> compatible = "fake,board"; >> fake,value = <0x2a>; >> }; >> >> In the resulting dtb foo bar and baz are missing. >> >> Signed-off-by: Nikita Travkin <nikita@trvn.ru> >> --- >> Nikita Travkin (2): >> arm64: dts: qcom: qrb5165-rb5-vision-mezzanine: Fix broken overlay root >> arm64: dts: qcom: sdm845-db845c-navigation-mezzanine: Fix the overlay root >> >> arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso | 2 +- >> arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> --- >> base-commit: 8433c776e1eb1371f5cd40b5fd3a61f9c7b7f3ad >> change-id: 20250226-qcom-nonroot-overlays-bfe21d33be8c >> >> Best regards, >> -- >> Nikita Travkin <nikita@trvn.ru> >>
On 26/02/2025 14:29, Nikita Travkin wrote: > While considering to propose WoA EL2 dt overlays upstream I was looking > at existing overlays and noticed that some of them are broken: they put > seemingly meaningful fixups into the overlay's "/" node, which places > them into the overlay "metadata" itself, not into a fixup fragment to be > applied to the actual dtb. This series fixes those two by changing to > full path "&{/}" which should work as it was initially intended. > > See demonstration of the problem below: > > $ cat base.dts > /dts-v1/; > / { > compatible = "fake,board"; > fake,value = <42>; > }; > > $ cat extra.dtso > /dts-v1/; > /plugin/; > > / { > foo; > bar { baz; }; > }; > &{/} { whatever-comes-next-after-baz; }; > > $ dtc base.dts -o base.dtb > $ dtc extra.dtso -o extra.dtbo > $ fdtoverlay -i base.dtb -o combine.dtb extra.dtbo > $ dtc base.dtb > /dts-v1/; > > / { > compatible = "fake,board"; > fake,value = <0x2a>; > }; > > $ dtc extra.dtbo > /dts-v1/; > > / { > foo; > > bar { > baz; > }; > > fragment@0 { > target-path = "/"; > > __overlay__ { > whatever-comes-next-after-baz; > }; > }; > }; > > $ dtc combine.dtb > /dts-v1/; > > / { > whatever-comes-next-after-baz; > compatible = "fake,board"; > fake,value = <0x2a>; > }; > > In the resulting dtb foo bar and baz are missing. > > Signed-off-by: Nikita Travkin <nikita@trvn.ru> > --- > Nikita Travkin (2): > arm64: dts: qcom: qrb5165-rb5-vision-mezzanine: Fix broken overlay root > arm64: dts: qcom: sdm845-db845c-navigation-mezzanine: Fix the overlay root > > arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso | 2 +- > arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > --- > base-commit: 8433c776e1eb1371f5cd40b5fd3a61f9c7b7f3ad > change-id: 20250226-qcom-nonroot-overlays-bfe21d33be8c > > Best regards, This is a good find however, we should just drop these additional CMA heap nodes as the reason for their original addition has been superseded. libcamera can now use UDMA buf for the same purpose as the CMA heap, without the security concerns of CMA to distros. You only need the CMA heap when you require uncached physically contiguous buffers - for example Hantro on iMX but, this is not the case on qcom hardware. We can safely drop these CMA nodes for CAMSS. --- bod
On Thu, Feb 27, 2025 at 02:01:41PM +0500, Nikita Travkin wrote: > Dmitry Baryshkov писал(а) 27.02.2025 09:16: > > On Wed, Feb 26, 2025 at 07:29:54PM +0500, Nikita Travkin wrote: > >> While considering to propose WoA EL2 dt overlays upstream I was looking > >> at existing overlays and noticed that some of them are broken: they put > >> seemingly meaningful fixups into the overlay's "/" node, which places > >> them into the overlay "metadata" itself, not into a fixup fragment to be > >> applied to the actual dtb. This series fixes those two by changing to > >> full path "&{/}" which should work as it was initially intended. > >> > >> See demonstration of the problem below: > >> > [...] > >> $ dtc extra.dtbo > >> /dts-v1/; > >> > >> / { > >> foo; > >> > >> bar { > >> baz; > >> }; > > > > Is this behaviour documented somewhere? I'd say, it would be a surprise > > to me. > > > > According to dtc docs [1], > > 3.b) The Device Tree fragments must be compiled with the same option but they > must also have a tag (/plugin/) that allows undefined references to nodes > that are not present at compilation time to be recorded so that the runtime > loader can fix them. > > so per my understanding "plugin" directive only changes the meaning of > references (i.e. stuff with "&"), to generate fragments/fixups, which > are the only way libfdt combines overlays into the base dtb. > > I suppose the old way of "manually" writing fragments (and thus writing > to / as raw nodes) was kept because phandle/path based updates were > added later to dtc and many overlays were still defining raw fragments... > > "/" also allows one to write "raw" nodes into the overlay, which is > sometimes used by downstreams. (i.e. they put custom extensions to the > overlay format [2] or add metadata into / of the dtbo like "compatible" > values to reject incompatible overlays from applying. [3]) This is > actually why I started looking here in the first place as for woa el2 > overlays I was asked to add compatible metadata as, apparently, NixOS > tooling requires it to validate the overlays [4]. I see. Thanks a lot for the explanation and for the pointers! > > [1] https://web.git.kernel.org/pub/scm/utils/dtc/dtc.git/tree/Documentation/dt-object-internal.txt#n120 > [2] https://github.com/raspberrypi/linux/blob/rpi-6.6.y/arch/arm/boot/dts/overlays/adafruit-st7735r-overlay.dts#L73 > [3] https://github.com/radxa-pkg/radxa-overlays/blob/main/arch/arm64/boot/dts/rockchip/overlays/rk3588-i2c0-m1.dts#L5 > [4] https://github.com/TravMurav/slbounce/blob/main/dtbo/x1e-el2.dtso#L12 > > >> > >> fragment@0 { > >> target-path = "/"; > >> > >> __overlay__ { > >> whatever-comes-next-after-baz; > >> }; > >> }; > >> }; > >> > >> $ dtc combine.dtb > >> /dts-v1/; > >> > >> / { > >> whatever-comes-next-after-baz; > >> compatible = "fake,board"; > >> fake,value = <0x2a>; > >> }; > >> > >> In the resulting dtb foo bar and baz are missing. > >> > >> Signed-off-by: Nikita Travkin <nikita@trvn.ru> > >> --- > >> Nikita Travkin (2): > >> arm64: dts: qcom: qrb5165-rb5-vision-mezzanine: Fix broken overlay root > >> arm64: dts: qcom: sdm845-db845c-navigation-mezzanine: Fix the overlay root > >> > >> arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso | 2 +- > >> arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso | 2 +- > >> 2 files changed, 2 insertions(+), 2 deletions(-) > >> --- > >> base-commit: 8433c776e1eb1371f5cd40b5fd3a61f9c7b7f3ad > >> change-id: 20250226-qcom-nonroot-overlays-bfe21d33be8c > >> > >> Best regards, > >> -- > >> Nikita Travkin <nikita@trvn.ru> > >>
While considering to propose WoA EL2 dt overlays upstream I was looking at existing overlays and noticed that some of them are broken: they put seemingly meaningful fixups into the overlay's "/" node, which places them into the overlay "metadata" itself, not into a fixup fragment to be applied to the actual dtb. This series fixes those two by changing to full path "&{/}" which should work as it was initially intended. See demonstration of the problem below: $ cat base.dts /dts-v1/; / { compatible = "fake,board"; fake,value = <42>; }; $ cat extra.dtso /dts-v1/; /plugin/; / { foo; bar { baz; }; }; &{/} { whatever-comes-next-after-baz; }; $ dtc base.dts -o base.dtb $ dtc extra.dtso -o extra.dtbo $ fdtoverlay -i base.dtb -o combine.dtb extra.dtbo $ dtc base.dtb /dts-v1/; / { compatible = "fake,board"; fake,value = <0x2a>; }; $ dtc extra.dtbo /dts-v1/; / { foo; bar { baz; }; fragment@0 { target-path = "/"; __overlay__ { whatever-comes-next-after-baz; }; }; }; $ dtc combine.dtb /dts-v1/; / { whatever-comes-next-after-baz; compatible = "fake,board"; fake,value = <0x2a>; }; In the resulting dtb foo bar and baz are missing. Signed-off-by: Nikita Travkin <nikita@trvn.ru> --- Nikita Travkin (2): arm64: dts: qcom: qrb5165-rb5-vision-mezzanine: Fix broken overlay root arm64: dts: qcom: sdm845-db845c-navigation-mezzanine: Fix the overlay root arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso | 2 +- arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) --- base-commit: 8433c776e1eb1371f5cd40b5fd3a61f9c7b7f3ad change-id: 20250226-qcom-nonroot-overlays-bfe21d33be8c Best regards,