Re: [PATCH] checks: Avoid warnings for reg override in __overlay__

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 6753 bytes --]

On Wed, Jun 17, 2026 at 01:44:01PM -0700, Brian Norris wrote:
> A simple overlay case can hit a number of false warnings just because it
> provides a 'reg' property [1]:
> 
>  * avoid_default_addr_size: the generated fragment@0 node is an
>    artificial node, where #address-cells/#size-cells can't exist.
> 
>  * reg_format: we're assuming the wrong/default #address-cells here, so
>    the analysis is wrong. (We don't know how many cells should be in
>    this 'reg', in isolation.)
> 
>  * unit_address_vs_reg: we don't know the real node name here. (It's not
>    actually going to be "__overlay__".)
> 
> All these cannot be checked by the overlay compiler in isolation.
> Suppress them when we identify we're running in plugin mode, and we're
> likely looking at a generated __overlay__ hierarchy that can't be
> analyzed in this way.

Yeah, the check system was designed before runtime overlays and
doesn't work well with them.  Specifically there's not distinction
between checks that can be applied locally (just looking at one node),
checks that can be applied to a subtree (without looking outside the
subtree) and checks which require a full tree including a root node.

Just suppressing checks ad-hoc like this is not a great solution.

I had a loose plan for tackling this, amongst some other problems, by
making the assembly of a complete tree an explicitly separate step in
dtc from parsing each fragment (here referring to compile time dts
fragments).

The idea was that each of the (compile time) overlays would be
separately parsed into a temporary tree, rather than assembling them
together into a single tree as we go.  Then we'd have a pass applying
those checks that make sense for subtrees separately to each
subtree/overlay.

Then there would be a separate assembly stage.  For full trees that
would essentially apply each overlay.  For overlay output it would add
the fragment@XX/__overlay__ encoding.  Afterwards there would be
another checks pass for the assembled tree - that would have a
different set of checks from the individual fragment tests, and
different ones for complete trees versus overlay output.

Unfortunately, I will not realistically ever get to this.

> 
> [1]
> $ dtc -I dts -O dts tests/overlay-reg-override.dtso
> tests/overlay-reg-override.dtso:5.2-14: Warning (reg_format): /fragment@0/__overlay__:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
> tests/overlay-reg-override.dtso:4.6-6.3: Warning (unit_address_vs_reg): /fragment@0/__overlay__: node has a reg or ranges property, but no unit name
> <stdout>: Warning (pci_device_reg): Failed prerequisite 'reg_format'
> <stdout>: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
> <stdout>: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
> <stdout>: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
> <stdout>: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
> tests/overlay-reg-override.dtso:4.6-6.3: Warning (avoid_default_addr_size): /fragment@0/__overlay__: Relying on default #address-cells value
> tests/overlay-reg-override.dtso:4.6-6.3: Warning (avoid_default_addr_size): /fragment@0/__overlay__: Relying on default #size-cells value
> <stdout>: Warning (avoid_unnecessary_addr_size): Failed prerequisite 'avoid_default_addr_size'
> <stdout>: Warning (unique_unit_address): Failed prerequisite 'avoid_default_addr_size'
> /dts-v1/;
> 
> / {
> 
> 	fragment@0 {
> 		target = <&foo>;
> 
> 		__overlay__ {
> 			reg = <0x00>;
> 		};
> 	};
> 
> 	__fixups__ {
> 		foo = "/fragment@0:target:0";
> 	};
> };
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
>  checks.c                        | 17 ++++++++++++++---
>  tests/overlay-reg-override.dtso |  6 ++++++
>  tests/run_tests.sh              |  1 +
>  3 files changed, 21 insertions(+), 3 deletions(-)
>  create mode 100644 tests/overlay-reg-override.dtso
> 
> diff --git a/checks.c b/checks.c
> index a6037ed08000..dd6824dce6ea 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -368,9 +368,12 @@ static void check_unit_address_vs_reg(struct check *c, struct dt_info *dti,
>  	const char *unitname = get_unitname(node);
>  	struct property *prop = get_property(node, "reg");
>  
> -	if (get_subnode(node, "__overlay__")) {
> -		/* HACK: Overlay fragments are a special case */
> -		return;
> +	/* HACK: Overlay fragments are a special case */

Yeah... it's definitely a hack.

> +	if (dti->dtsflags & DTSF_PLUGIN) {
> +		if (get_subnode(node, "__overlay__"))
> +			return;
> +		if (streq(node->name, "__overlay__"))
> +			return;
>  	}
>  
>  	if (!prop) {
> @@ -774,6 +777,10 @@ static void check_reg_format(struct check *c, struct dt_info *dti,
>  	if (!prop)
>  		return; /* No "reg", that's fine */
>  
> +	/* Overlay nodes are special */
> +	if ((dti->dtsflags & DTSF_PLUGIN) && streq(node->name, "__overlay__"))
> +		return;
> +
>  	if (!node->parent) {
>  		FAIL(c, dti, node, "Root node has a \"reg\" property");
>  		return;
> @@ -1210,6 +1217,10 @@ static void check_avoid_default_addr_size(struct check *c, struct dt_info *dti,
>  	if (!node->parent)
>  		return; /* Ignore root node */
>  
> +	/* Overlay nodes are special */
> +	if ((dti->dtsflags & DTSF_PLUGIN) && streq(node->name, "__overlay__"))
> +		return;
> +
>  	reg = get_property(node, "reg");
>  	ranges = get_property(node, "ranges");
>  
> diff --git a/tests/overlay-reg-override.dtso b/tests/overlay-reg-override.dtso
> new file mode 100644
> index 000000000000..7e5e13883b51
> --- /dev/null
> +++ b/tests/overlay-reg-override.dtso
> @@ -0,0 +1,6 @@
> +/dts-v1/;
> +/plugin/;
> +
> +&foo {
> +	reg = <0x0>;
> +};
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index 842b543059bb..8f3265afa5cc 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -752,6 +752,7 @@ dtc_tests () {
>      check_tests "$SRCDIR/unit-addr-leading-0x.dts" unit_address_format
>      check_tests "$SRCDIR/unit-addr-leading-0s.dts" unit_address_format
>      check_tests "$SRCDIR/unit-addr-unique.dts" unique_unit_address
> +    check_tests "$SRCDIR/overlay-reg-override.dtso" -n reg_format unit_address_vs_reg avoid_default_addr_size
>      check_tests "$SRCDIR/bad-phandle-cells.dts" interrupts_extended_property
>      check_tests "$SRCDIR/bad-gpio.dts" gpios_property
>      check_tests "$SRCDIR/good-gpio.dts" -n gpios_property
> -- 
> 2.54.0.1189.g8c84645362-goog
> 
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]