On 5/3/22 16:42, Rob Herring wrote: > On Tue, May 3, 2022 at 4:20 PM <frowand.list@xxxxxxxxx> wrote: >> >> From: Frank Rowand <frank.rowand@xxxxxxxx> >> >> In drivers/of/unittest-data/: >> - Rename .dts overlay source files to use .dtso suffix. >> - Add Makefile rule to build .dtbo.o assembly file from overlay >> .dtso source file. >> - Update Makefile to build .dtbo.o objects instead of .dtb.o from >> unittest overlay source files. >> >> Modify driver/of/unitest.c to use .dtbo.o based symbols instead of >> .dtb.o >> >> Modify scripts/Makefile.lib %.dtbo rule to depend upon %.dtso instead >> of %.dts >> >> Rename .dts overlay source files to use .dtso suffix in: >> arch/arm64/boot/dts/freescale/ >> arch/arm64/boot/dts/xilinx/ >> >> Signed-off-by: Frank Rowand <frank.rowand@xxxxxxxx> >> --- >> >> Testing by arm64 people would be much appreciated. >> >> Applies on branch dt/next, commit 4fb74186cee8 of Rob's kernel.org tree. >> git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux >> >> version 1 patch: >> https://lore.kernel.org/r/20210324223713.1334666-1-frowand.list@xxxxxxxxx >> >> changes from version 1: >> - rebase to 5.18-rc1 plus many patches already accepted by Rob >> Applies on branch dt/next, commit 4fb74186cee8 of Rob's kernel.org tree. >> - Add new overlay source files in: >> arch/arm64/boot/dts/freescale/ >> arch/arm64/boot/dts/xilinx/ > > I can't apply these. They need to be applied separately. And probably > at end of the merge window or right after rc1 (IOW, coordinated with > SoC maintainers in advance). Or we support both forms for a cycle. If applied separately then git bisect is broken. I don't see this change as being big enough to be considered a "flag day" change, but if I can't get the SoC maintainers to ack you pulling these renames then I can easily re-spin in a way to support both forms for a release cycle. > > [...] > >> diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile >> index d072f3ba3971..df2ca1820273 100644 >> --- a/drivers/of/unittest-data/Makefile >> +++ b/drivers/of/unittest-data/Makefile >> @@ -1,38 +1,58 @@ >> # SPDX-License-Identifier: GPL-2.0 >> -obj-y += testcases.dtb.o >> >> -obj-$(CONFIG_OF_OVERLAY) += overlay.dtb.o \ >> - overlay_0.dtb.o \ >> - overlay_1.dtb.o \ >> - overlay_2.dtb.o \ >> - overlay_3.dtb.o \ >> - overlay_4.dtb.o \ >> - overlay_5.dtb.o \ >> - overlay_6.dtb.o \ >> - overlay_7.dtb.o \ >> - overlay_8.dtb.o \ >> - overlay_9.dtb.o \ >> - overlay_10.dtb.o \ >> - overlay_11.dtb.o \ >> - overlay_12.dtb.o \ >> - overlay_13.dtb.o \ >> - overlay_15.dtb.o \ >> - overlay_16.dtb.o \ >> - overlay_17.dtb.o \ >> - overlay_18.dtb.o \ >> - overlay_19.dtb.o \ >> - overlay_20.dtb.o \ >> - overlay_bad_add_dup_node.dtb.o \ >> - overlay_bad_add_dup_prop.dtb.o \ >> - overlay_bad_phandle.dtb.o \ >> - overlay_bad_symbol.dtb.o \ >> - overlay_base.dtb.o \ >> - overlay_gpio_01.dtb.o \ >> - overlay_gpio_02a.dtb.o \ >> - overlay_gpio_02b.dtb.o \ >> - overlay_gpio_03.dtb.o \ >> - overlay_gpio_04a.dtb.o \ >> - overlay_gpio_04b.dtb.o >> +# Generate an assembly file to wrap the output of the device tree compiler >> +quiet_cmd_dt_S_dtbo= DTB $@ >> +cmd_dt_S_dtbo=\ >> +{ \ >> + echo '\#include <asm-generic/vmlinux.lds.h>'; \ >> + echo '.section .dtb.init.rodata,"a"'; \ >> + echo '.balign STRUCT_ALIGNMENT'; \ >> + echo '.global __dtbo_$(subst -,_,$(*F))_begin'; \ >> + echo '__dtbo_$(subst -,_,$(*F))_begin:'; \ >> + echo '.incbin "$<" '; \ >> + echo '__dtbo_$(subst -,_,$(*F))_end:'; \ >> + echo '.global __dtbo_$(subst -,_,$(*F))_end'; \ >> + echo '.balign STRUCT_ALIGNMENT'; \ >> +} > $@ >> + >> + >> +$(obj)/%.dtbo.S: $(obj)/%.dtbo FORCE >> + $(call if_changed,dt_S_dtbo) > > Humm, this belongs in scripts/Makefile.lib. I would rather keep it isolated to just the use in unittest. We just now got rid of the final driver use of of_overlay_fdt_apply() by the grandfathered legacy user in: 841281fe52a7 drm: rcar-du: Drop LVDS device tree backward compatibility That driver was the only use of %.dtb.S for an overlay. I can't eliminate the rule for %.dtb.S because that is used in several cases for building the system FDT into the kernel. But having this rule in drivers/of/unittest-data/Makefile provides consistent naming of overlays withing unittest. Helping to make it clear that they are not a system FDT. > > I don't think we need a different section name with __dtbo_* instead > of __dtb_* if that simplifies the Makefile. A different section name is not needed if the rule is moved to scripts/Makefile.lib but even if moved there, I prefer to keep the overlay data clearly delineated from the system FDT data. There is also a kernel test robot email warning about randconfig arc build errors: >> make[4]: *** No rule to make target 'drivers/of/unittest-data/static_base_1.dtb', needed by 'drivers/of/unittest-data/static_test_1.dtb'. >> make[4]: *** No rule to make target 'drivers/of/unittest-data/static_base_2.dtb', needed by 'drivers/of/unittest-data/static_test_2.dtb'. They look like a pre-existing problem, not new. I'll get their build environment and see what's going on. -Frank > >> + >> +obj-y += testcases.dtbo.o >> + >> +obj-$(CONFIG_OF_OVERLAY) += overlay.dtbo.o \ >> + overlay_0.dtbo.o \ >> + overlay_1.dtbo.o \ >> + overlay_2.dtbo.o \ >> + overlay_3.dtbo.o \ >> + overlay_4.dtbo.o \ >> + overlay_5.dtbo.o \ >> + overlay_6.dtbo.o \ >> + overlay_7.dtbo.o \ >> + overlay_8.dtbo.o \ >> + overlay_9.dtbo.o \ >> + overlay_10.dtbo.o \ >> + overlay_11.dtbo.o \ >> + overlay_12.dtbo.o \ >> + overlay_13.dtbo.o \ >> + overlay_15.dtbo.o \ >> + overlay_16.dtbo.o \ >> + overlay_17.dtbo.o \ >> + overlay_18.dtbo.o \ >> + overlay_19.dtbo.o \ >> + overlay_20.dtbo.o \ >> + overlay_bad_add_dup_node.dtbo.o \ >> + overlay_bad_add_dup_prop.dtbo.o \ >> + overlay_bad_phandle.dtbo.o \ >> + overlay_bad_symbol.dtbo.o \ >> + overlay_base.dtbo.o \ >> + overlay_gpio_01.dtbo.o \ >> + overlay_gpio_02a.dtbo.o \ >> + overlay_gpio_02b.dtbo.o \ >> + overlay_gpio_03.dtbo.o \ >> + overlay_gpio_04a.dtbo.o \ >> + overlay_gpio_04b.dtbo.o >> >> # enable creation of __symbols__ node >> DTC_FLAGS_overlay += -@ > > >> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c >> index dff55ae09d97..1d3c170d95db 100644 >> --- a/drivers/of/unittest.c >> +++ b/drivers/of/unittest.c >> @@ -1423,12 +1423,12 @@ static int __init unittest_data_add(void) >> void *unittest_data_align; >> struct device_node *unittest_data_node = NULL, *np; >> /* >> - * __dtb_testcases_begin[] and __dtb_testcases_end[] are magically >> - * created by cmd_dt_S_dtb in scripts/Makefile.lib >> + * __dtbo_testcases_begin[] and __dtbo_testcases_end[] are >> + * created by cmd_dt_S_dtbo in Makefile >> */ >> - extern uint8_t __dtb_testcases_begin[]; >> - extern uint8_t __dtb_testcases_end[]; >> - const int size = __dtb_testcases_end - __dtb_testcases_begin; >> + extern uint8_t __dtbo_testcases_begin[]; >> + extern uint8_t __dtbo_testcases_end[]; >> + const int size = __dtbo_testcases_end - __dtbo_testcases_begin; >> int rc; >> void *ret; >> >> @@ -1443,7 +1443,7 @@ static int __init unittest_data_add(void) >> return -ENOMEM; >> >> unittest_data_align = PTR_ALIGN(unittest_data, FDT_ALIGN_SIZE); >> - memcpy(unittest_data_align, __dtb_testcases_begin, size); >> + memcpy(unittest_data_align, __dtbo_testcases_begin, size); >> >> ret = of_fdt_unflatten_tree(unittest_data_align, NULL, &unittest_data_node); >> if (!ret) { >> @@ -3009,24 +3009,24 @@ static inline void __init of_unittest_overlay(void) { } >> #ifdef CONFIG_OF_OVERLAY >> >> /* >> - * __dtb_ot_begin[] and __dtb_ot_end[] are created by cmd_dt_S_dtb >> - * in scripts/Makefile.lib >> + * __dtbo_##overlay_name##_begin[] and __dtbo_##overlay_name##_end[] are >> + * created by cmd_dt_S_dtbo in Makefile >> */ >> >> -#define OVERLAY_INFO_EXTERN(name) \ >> - extern uint8_t __dtb_##name##_begin[]; \ >> - extern uint8_t __dtb_##name##_end[] >> +#define OVERLAY_INFO_EXTERN(overlay_name) \ >> + extern uint8_t __dtbo_##overlay_name##_begin[]; \ >> + extern uint8_t __dtbo_##overlay_name##_end[] >> >> -#define OVERLAY_INFO(overlay_name, expected) \ >> -{ .dtb_begin = __dtb_##overlay_name##_begin, \ >> - .dtb_end = __dtb_##overlay_name##_end, \ >> - .expected_result = expected, \ >> - .name = #overlay_name, \ >> +#define OVERLAY_INFO(overlay_name, expected) \ >> +{ .dtbo_begin = __dtbo_##overlay_name##_begin, \ >> + .dtbo_end = __dtbo_##overlay_name##_end, \ >> + .expected_result = expected, \ >> + .name = #overlay_name, \ >> } >> >> struct overlay_info { >> - uint8_t *dtb_begin; >> - uint8_t *dtb_end; >> + uint8_t *dtbo_begin; >> + uint8_t *dtbo_end; > > Is this rename really needed? > >> int expected_result; >> int ovcs_id; >> char *name; >> @@ -3100,7 +3100,7 @@ static struct overlay_info overlays[] = { >> OVERLAY_INFO(overlay_bad_phandle, -EINVAL), >> OVERLAY_INFO(overlay_bad_symbol, -EINVAL), >> /* end marker */ >> - {.dtb_begin = NULL, .dtb_end = NULL, .expected_result = 0, .name = NULL} >> + {.dtbo_begin = NULL, .dtbo_end = NULL, .expected_result = 0, .name = NULL} >> }; >> >> static struct device_node *overlay_base_root; >> @@ -3157,13 +3157,13 @@ void __init unittest_unflatten_overlay_base(void) >> return; >> } >> >> - data_size = info->dtb_end - info->dtb_begin; >> + data_size = info->dtbo_end - info->dtbo_begin; >> if (!data_size) { >> pr_err("No dtb 'overlay_base' to attach\n"); >> return; >> } >> >> - size = fdt_totalsize(info->dtb_begin); >> + size = fdt_totalsize(info->dtbo_begin); >> if (size != data_size) { >> pr_err("dtb 'overlay_base' header totalsize != actual size"); >> return; >> @@ -3175,7 +3175,7 @@ void __init unittest_unflatten_overlay_base(void) >> return; >> } >> >> - memcpy(new_fdt, info->dtb_begin, size); >> + memcpy(new_fdt, info->dtbo_begin, size); >> >> __unflatten_device_tree(new_fdt, NULL, &overlay_base_root, >> dt_alloc_memory, true); >> @@ -3210,11 +3210,11 @@ static int __init overlay_data_apply(const char *overlay_name, int *ovcs_id) >> return 0; >> } >> >> - size = info->dtb_end - info->dtb_begin; >> + size = info->dtbo_end - info->dtbo_begin; >> if (!size) >> pr_err("no overlay data for %s\n", overlay_name); >> >> - ret = of_overlay_fdt_apply(info->dtb_begin, size, &info->ovcs_id); >> + ret = of_overlay_fdt_apply(info->dtbo_begin, size, &info->ovcs_id); >> if (ovcs_id) >> *ovcs_id = info->ovcs_id; >> if (ret < 0) >> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib >> index 9f69ecdd7977..b409bec5fa45 100644 >> --- a/scripts/Makefile.lib >> +++ b/scripts/Makefile.lib >> @@ -363,7 +363,7 @@ endef >> $(obj)/%.dtb: $(src)/%.dts $(DTC) $(DT_TMP_SCHEMA) FORCE >> $(call if_changed_rule,dtc) >> >> -$(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE >> +$(obj)/%.dtbo: $(src)/%.dtso $(DTC) FORCE >> $(call if_changed_dep,dtc) >> >> dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp) >> -- >> Frank Rowand <frank.rowand@xxxxxxxx> >>
