[PATCH v2] Makefile: Fix include directory for OF_UPSTREAM

[Michael Nazzareno Trimarchi]

HI Sumit

On Mon, Jun 17, 2024 at 10:22 AM Sumit Garg <sumit.garg at linaro.org> wrote:
>
> Hi Michael,
>
> Sorry for the delayed response as I had some health issues last week.
>

We are not in a hurry and I think discussion has his own time. Glad that you are
getting better

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

Yes but this is the case. All we build now can include the local
redundant copy and we can
not guarantee that already board builds are built using the latest
version of the include.

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

Yes but suppose that the include was ok in the sense of compilation
but not consistent
in term of functionality. When you build using upstream headers you
must use them
to be consistent and it's not as it is for now. Anyway we can not drop
for one board upstream
that file, unless we force everyone to have broken board or all upstream now

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

Michael