Hi Sumit On Thu, Jun 13, 2024 at 9:02 AM Sumit Garg <[email protected]> wrote: > > On Mon, 3 Jun 2024 at 20:38, Patrick Barsanti > <[email protected]> wrote: > > > > Always prioritizing u-boot includes causes problems when trying to migrate > > boards to OF_UPSTREAM that have different local devicetree files with > > respect to the upstream ones, if local DT headers are not dropped. > > At the same time if local and upstream files are the same, migrations > > can be, and have been, introduced without dropping local DT headers. > > This also causes problems whenever upstream DTS and DT headers are patched. > > > > For example, now migrating a board based on `imx6ul.dtsi` to OF_UPSTREAM > > breaks it, as there are some missing defines in a local DT header file > > (`imx6ul-clock.h`); the solution would be to drop it, which has not always > > been done in previous OF_UPSTREAM migration patches for other boards > > because most of the time the two are the same. This solution is also > > vulnerable to ABI breakage, although this has not yet happened since the > > introduction of OF_UPSTREAM support and is unlikely. > > > > Maintainers assure backwards compatibility for DT headers when syncing the > > upstream folder with the kernel. > > The problem is that, at the current state, all boards that have not dropped > > their local headers when migrating to OF_UPSTREAM will break once the > > upstream devicetrees are patched, for example, to use any newly added > > define which is not present in the local DT header file, even if those > > changes are backwards compatible. > > > > This patch fixes these problems by prioritizing upstream includes when > > `CONFIG_OF_UPSTREAM=y`, while keeping current prioritization when it is > > not. > > > > Also in case of ABI breakage in the kernel, keeping redundant header files > > (only until strictly necessary, e.g. until all boards including them have > > been migrated to OF_UPSTREAM) together with this selective prioritization > > of the include folder is a solution for keeping not-yet-migrated boards > > from breaking. > > Let's just not try to make assumptions about DT ABI breakage due to > using upstream headers for not-yet-migrated boards but rather talk > about some real world examples. Have you come across any DT ABI > breakage with usage of upstream headers? The breakage you have > reported is due to usage of an old local copy of DT header. >
The include priority is broken, and this a Makefile problem anyway. This patch fix anyway includes priorities. The imx6 board migration for us does not work > However, if this patch is only needed to address fear of DT ABI > breakage (which hasn't happened in the past) then I can live with it > such that it can help convince more people for OF_UPSTREAM migration. > We can drop local DT headers as and when people feel comfortable with > upstream. It's not abi breakage only but path inclusion order. Michael > > -Sumit > > > > > Link: > > https://lore.kernel.org/u-boot/[email protected]/ > > Signed-off-by: Patrick Barsanti <[email protected]> > > Tested-by: Michael Trimarchi <[email protected]> > > --- > > > > Changes in v2: > > - Reword and correct the commit message following the discussion > > with Sumit, per Michael's suggestion. > > > > Makefile | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/Makefile b/Makefile > > index 79b28c2d81..899ae664ca 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -826,6 +826,19 @@ KBUILD_HOSTCFLAGS += $(if $(CONFIG_TOOLS_DEBUG),-g) > > > > # Use UBOOTINCLUDE when you must reference the include/ directory. > > # Needed to be compatible with the O= option > > +ifeq ($(CONFIG_OF_UPSTREAM),y) > > +UBOOTINCLUDE := \ > > + -I$(srctree)/dts/upstream/include \ > > + -Iinclude \ > > + $(if $(KBUILD_SRC), -I$(srctree)/include) \ > > + $(if $(CONFIG_$(SPL_)SYS_THUMB_BUILD), \ > > + $(if $(CONFIG_HAS_THUMB2), \ > > + $(if $(CONFIG_CPU_V7M), \ > > + -I$(srctree)/arch/arm/thumb1/include), \ > > + -I$(srctree)/arch/arm/thumb1/include)) \ > > + -I$(srctree)/arch/$(ARCH)/include \ > > + -include $(srctree)/include/linux/kconfig.h > > +else > > UBOOTINCLUDE := \ > > -Iinclude \ > > $(if $(KBUILD_SRC), -I$(srctree)/include) \ > > @@ -837,6 +850,7 @@ UBOOTINCLUDE := \ > > -I$(srctree)/arch/$(ARCH)/include \ > > -include $(srctree)/include/linux/kconfig.h \ > > -I$(srctree)/dts/upstream/include > > +endif > > > > NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) > > -print-file-name=include) > > > > -- > > 2.45.1 > > -- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 [email protected] __________________________________ Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 [email protected] www.amarulasolutions.com

