[PATCH v2] Makefile: Fix include directory for OF_UPSTREAM

[Sumit Garg]

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.

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.

-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
>