[PATCH v2] Makefile: Fix include directory for OF_UPSTREAM

[Michael Nazzareno Trimarchi]

Hi Sumit

On Thu, Jun 13, 2024 at 9:02 AM Sumit Garg <sumit.garg at linaro.org> wrote:
>
> On Mon, 3 Jun 2024 at 20:38, Patrick Barsanti
> <patrick.barsanti at amarulasolutions.com> 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/20240528084224.113385-1-patrick.barsanti@amarulasolutions.com/
> > Signed-off-by: Patrick Barsanti <patrick.barsanti at amarulasolutions.com>
> > Tested-by: Michael Trimarchi <michael at amarulasolutions.com>
> > ---
> >
> > 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
michael at amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info at amarulasolutions.com
www.amarulasolutions.com