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 <[email protected]> wrote: > > 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 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_7...@mail.gmail.com/ -Sumit
