[PATCH 0/2] {vision/navigation}-mezzanine: Fix overlay root node

[PATCH 0/2] {vision/navigation}-mezzanine: Fix overlay root node

[Nikita Travkin]

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,
-- 
Nikita Travkin <nikita@trvn.ru>

[PATCH 1/2] arm64: dts: qcom: qrb5165-rb5-vision-mezzanine: Fix broken overlay root

[Nikita Travkin]

When converting to the overlay format, it was missed that "/" in the
overlay corresponds to the overlay's own root node and not the fragment
targeted to update root of the base dts, which should be "&{/}" instead.
This results in the cma node never actually being applied by libfdt.

Fix the overlay to use correct target node.

Fixes: 231c03c6119d ("arm64: dts: qcom: qrb5165-rb5-vision-mezzanine: Convert mezzanine riser to dtbo")
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
 arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso b/arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso
index ae256c713a36078afdadc67193f381a19ea8e5d3..254df3d518d8cbfb1082511f38e132435b7fdf59 100644
--- a/arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso
+++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso
@@ -9,7 +9,7 @@
 #include <dt-bindings/clock/qcom,camcc-sm8250.h>
 #include <dt-bindings/gpio/gpio.h>
 
-/ {
+&{/} {
 	reserved-memory {
 		linux,cma {
 			compatible = "shared-dma-pool";

-- 
2.48.1

Re: [PATCH 1/2] arm64: dts: qcom: qrb5165-rb5-vision-mezzanine: Fix broken overlay root

[Rob Herring]

On Wed, Feb 26, 2025 at 8:30 AM Nikita Travkin <nikita@trvn.ru> wrote:
>
> When converting to the overlay format, it was missed that "/" in the
> overlay corresponds to the overlay's own root node and not the fragment
> targeted to update root of the base dts, which should be "&{/}" instead.
> This results in the cma node never actually being applied by libfdt.
>
> Fix the overlay to use correct target node.
>
> Fixes: 231c03c6119d ("arm64: dts: qcom: qrb5165-rb5-vision-mezzanine: Convert mezzanine riser to dtbo")
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> ---
>  arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso b/arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso
> index ae256c713a36078afdadc67193f381a19ea8e5d3..254df3d518d8cbfb1082511f38e132435b7fdf59 100644
> --- a/arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso
> +++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso
> @@ -9,7 +9,7 @@
>  #include <dt-bindings/clock/qcom,camcc-sm8250.h>
>  #include <dt-bindings/gpio/gpio.h>
>
> -/ {
> +&{/} {
>         reserved-memory {

IMO, this should be applied to the /reserved-memory node rather than
the root node. Though I also think using overlays to set CMA size is
questionable. It's much easier to change the kernel command line than
apply an overlay.

>                 linux,cma {
>                         compatible = "shared-dma-pool";
>
> --
> 2.48.1
>

Re: [PATCH 1/2] arm64: dts: qcom: qrb5165-rb5-vision-mezzanine: Fix broken overlay root

[Nikita Travkin]

Rob Herring писал(а) 27.02.2025 00:22:
> On Wed, Feb 26, 2025 at 8:30 AM Nikita Travkin <nikita@trvn.ru> wrote:
>>
>> When converting to the overlay format, it was missed that "/" in the
>> overlay corresponds to the overlay's own root node and not the fragment
>> targeted to update root of the base dts, which should be "&{/}" instead.
>> This results in the cma node never actually being applied by libfdt.
>>
>> Fix the overlay to use correct target node.
>>
>> Fixes: 231c03c6119d ("arm64: dts: qcom: qrb5165-rb5-vision-mezzanine: Convert mezzanine riser to dtbo")
>> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
>> ---
>>  arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso b/arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso
>> index ae256c713a36078afdadc67193f381a19ea8e5d3..254df3d518d8cbfb1082511f38e132435b7fdf59 100644
>> --- a/arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso
>> +++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5-vision-mezzanine.dtso
>> @@ -9,7 +9,7 @@
>>  #include <dt-bindings/clock/qcom,camcc-sm8250.h>
>>  #include <dt-bindings/gpio/gpio.h>
>>
>> -/ {
>> +&{/} {
>>         reserved-memory {
> 
> IMO, this should be applied to the /reserved-memory node rather than

Hm, considering pretty much no qcom dtb will be without reserved-memory,
you're right...

> the root node. Though I also think using overlays to set CMA size is
> questionable. It's much easier to change the kernel command line than
> apply an overlay.

...and deleting the node altogether would be effectively keeping the
existing behavior, which, I just now realized, was introduced in an
extra commit after the conversion (which I assumed was the cause of
the mistake) so my Fixes: for both commits are wrong.

I think I will leave it to Bryan to decide whether this should be
fixed or original commit to be reverted.

> 
>>                 linux,cma {
>>                         compatible = "shared-dma-pool";
>>
>> --
>> 2.48.1
>>

[PATCH 2/2] arm64: dts: qcom: sdm845-db845c-navigation-mezzanine: Fix the overlay root

[Nikita Travkin]

When converting to the overlay format, it was missed that "/" in the
overlay corresponds to the overlay's own root node and not the fragment
targeted to update root of the base dts, which should be "&{/}" instead.
This results in the cma node never actually being applied by libfdt.

Fix the overlay to use correct target node.

Fixes: 30df676a31b7 ("arm64: dts: qcom: sdm845-db845c-navigation-mezzanine: Convert mezzanine riser to dtso")
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
 arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso
index 59970082da45203311146cc5249298f6188bf67a..d8c0021b23cb51bd19e1826b6cd8544f9e10a2fe 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso
+++ b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso
@@ -9,7 +9,7 @@
 #include <dt-bindings/clock/qcom,camcc-sdm845.h>
 #include <dt-bindings/gpio/gpio.h>
 
-/ {
+&{/} {
 	reserved-memory {
 		linux,cma {
 			compatible = "shared-dma-pool";

-- 
2.48.1

Re: [PATCH 0/2] {vision/navigation}-mezzanine: Fix overlay root node

[Dmitry Baryshkov]

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>
> 

-- 
With best wishes
Dmitry

Re: [PATCH 0/2] {vision/navigation}-mezzanine: Fix overlay root node

[Nikita Travkin]

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>
>>

Re: [PATCH 0/2] {vision/navigation}-mezzanine: Fix overlay root node

[Dmitry Baryshkov]

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>
> >>

-- 
With best wishes
Dmitry

Re: [PATCH 0/2] {vision/navigation}-mezzanine: Fix overlay root node

[Bryan O'Donoghue]

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