Re: [PATCH V7 6/6] of: unittest: Statically apply overlays using fdtoverlay

[Rob Herring]

On Fri, Jan 29, 2021 at 12:54:10PM +0530, Viresh Kumar wrote:
> Now that fdtoverlay is part of the kernel build, start using it to test
> the unitest overlays we have by applying them statically. Create two new
> base files static_base_1.dts and static_base_2.dts which includes other
> .dtsi files.
> 
> Some unittest overlays deliberately contain errors that unittest checks
> for. These overlays will cause fdtoverlay to fail, and are thus not
> included for static builds.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
> ---
>  drivers/of/unittest-data/Makefile          | 56 ++++++++++++++++++++++
>  drivers/of/unittest-data/static_base_1.dts |  4 ++
>  drivers/of/unittest-data/static_base_2.dts |  4 ++
>  3 files changed, 64 insertions(+)
>  create mode 100644 drivers/of/unittest-data/static_base_1.dts
>  create mode 100644 drivers/of/unittest-data/static_base_2.dts

The first 4 patches look good to me, no need to resend those if you 
don't want to.

> 
> diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
> index 009f4045c8e4..fc286224b2d1 100644
> --- a/drivers/of/unittest-data/Makefile
> +++ b/drivers/of/unittest-data/Makefile
> @@ -34,7 +34,63 @@ DTC_FLAGS_overlay += -@
>  DTC_FLAGS_overlay_bad_phandle += -@
>  DTC_FLAGS_overlay_bad_symbol += -@
>  DTC_FLAGS_overlay_base += -@
> +DTC_FLAGS_static_base_1 += -@
> +DTC_FLAGS_static_base_2 += -@
>  DTC_FLAGS_testcases += -@
>  
>  # suppress warnings about intentional errors
>  DTC_FLAGS_testcases += -Wno-interrupts_property
> +
> +# Apply overlays statically with fdtoverlay.  This is a build time test that
> +# the overlays can be applied successfully by fdtoverlay.  This does not
> +# guarantee that the overlays can be applied successfully at run time by
> +# unittest, but it provides a bit of build time test coverage for those
> +# who do not execute unittest.
> +#
> +# The overlays are applied on top of static_base_1.dtb and static_base_2.dtb to
> +# create static_test_1.dtb and static_test_2.dtb.  If fdtoverlay detects an
> +# error than the kernel build will fail.  static_test_1.dtb and
> +# static_test_2.dtb are not consumed by unittest.
> +#
> +# Some unittest overlays deliberately contain errors that unittest checks for.
> +# These overlays will cause fdtoverlay to fail, and are thus not included
> +# in the static test:
> +#			  overlay_bad_add_dup_node.dtb \
> +#			  overlay_bad_add_dup_prop.dtb \
> +#			  overlay_bad_phandle.dtb \
> +#			  overlay_bad_symbol.dtb \
> +
> +apply_static_overlay_1 := overlay_0.dtb \
> +			  overlay_1.dtb \
> +			  overlay_2.dtb \
> +			  overlay_3.dtb \
> +			  overlay_4.dtb \
> +			  overlay_5.dtb \
> +			  overlay_6.dtb \
> +			  overlay_7.dtb \
> +			  overlay_8.dtb \
> +			  overlay_9.dtb \
> +			  overlay_10.dtb \
> +			  overlay_11.dtb \
> +			  overlay_12.dtb \
> +			  overlay_13.dtb \
> +			  overlay_15.dtb \
> +			  overlay_gpio_01.dtb \
> +			  overlay_gpio_02a.dtb \
> +			  overlay_gpio_02b.dtb \
> +			  overlay_gpio_03.dtb \
> +			  overlay_gpio_04a.dtb \
> +			  overlay_gpio_04b.dtb

As these are overlays, .dtbo

> +
> +apply_static_overlay_2 := overlay.dtb
> +
> +quiet_cmd_fdtoverlay = FDTOVERLAY $@
> +      cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $^
> +
> +$(obj)/static_test_1.dtb: $(obj)/static_base_1.dtb $(addprefix $(obj)/,$(apply_static_overlay_1))
> +	$(call if_changed,fdtoverlay)
> +
> +$(obj)/static_test_2.dtb: $(obj)/static_base_2.dtb $(addprefix $(obj)/,$(apply_static_overlay_2))
> +	$(call if_changed,fdtoverlay)
> +
> +always-$(CONFIG_OF_OVERLAY) += static_test_1.dtb static_test_2.dtb

We can't be leaving all this for ordinary folks to copy-n-paste in 
every directory with overlays. It needs to be simple for users like 
multi-file modules:

test1-dtbs := static_base_1.dtb $(apply_static_overlay_1)
test2-dtbs := static_base_2.dtb $(apply_static_overlay_2)

dtb-$(CONFIG_OF_OVERLAY) += test1.dtb test2.dtb


I've gotten something working with the patch below. Hopefully, Masahiro 
has some comments on it. I couldn't get multiple '-dtbs' to work without 
the '.SECONDEXPANSION'.

Rob

8<-------------------------------------------------------------------