[PATCH v2] Makefile: Fix include directory for OF_UPSTREAM

[Sumit Garg]

Hi Michael,

Sorry for the delayed response as I had some health issues last week.

On Thu, 13 Jun 2024 at 12:44, Michael Nazzareno Trimarchi
<michael at amarulasolutions.com> wrote:
>
> 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 include priority only becomes an issue if you want to keep
redundant local copies of DT headers. And I have been advocating to
drop local copies from U-Boot as much as possible. Once we only have
upstream DT headers being used (Qcom platforms can be a good example
here), we don't need to care about trickier priorities.

> The imx6 board migration for us does not work

Patrick did mention here [1] that dropping
include/dt-bindings/clock/imx6ul-clock.h fixes the problem...

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

...however it's the ABI breakage part that worries Patrick. So if this
patch only solves that worry then I can live with it in the hope that
people will start to gain confidence in DT ABI stability.

[1] https://lore.kernel.org/u-boot/CABxfhNfWdD0NjPNskaxZxDTg9pDoDx5vBHrN9chvMh5Xo_7Kzg@mail.gmail.com/

-Sumit