Hi Patrick On Thu, May 30, 2024 at 11:13 AM Patrick Barsanti <[email protected]> wrote: > > Hi Sumit, > > On Wed, 29 May 2024 at 14:00, Sumit Garg <[email protected]> wrote: >> >> On Wed, 29 May 2024 at 14:45, Patrick Barsanti >> <[email protected]> wrote: >> > >> > Hi Sumit, >> > >> > On Wed, 29 May 2024 at 08:57, Sumit Garg <[email protected]> wrote: >> >> >> >> Hi Patrick, >> >> >> >> On Tue, 28 May 2024 at 14:16, Patrick Barsanti >> >> <[email protected]> wrote: >> >> > >> >> > Always prioritizing u-boot includes causes problems when trying to >> >> > migrate boards to OF_UPSTREAM that have divergent devicetree files with >> >> > respect to the upstream ones. >> >> > >> >> > For example, migrating a board based on `imx6ul.dtsi` to OF_UPSTREAM >> >> > breaks it, as there are some missing defines in the local dtsi file; >> >> > the solutions would be to either patch it, which defeats the purpose of >> >> > OF_UPSTREAM, or delete it entirely. This last option would then break >> >> > all >> >> > the other boards which have not yet been migrated to OF_UPSTREAM. >> >> >> >> Can you elaborate more here regarding which dt-bindings headers >> >> conflict? Also, is it only the DTS files consumer for those headers or >> >> there are U-Boot drivers depending on them too? >> >> >> >> -Sumit >> > >> > >> > Sorry, I think I have worded my commit message wrong. I should >> > have used differ instead of diverge, which is slightly misleading. >> > >> > The specific case I am talking about can be found in: >> > include/dt-bindings/clock/imx6ul-clock.h >> > dts/upstream/include/dt-bindings/clock/imx6ul-clock.h >> > >> > The local header is missing the last commit from the kernel, which is >> > 4e197ee880c2 ("clk: imx6ul: add ethernet refclock mux support"). >> > This added some new defines, which are not present in the u-boot >> > header. >> > Following this commit, the `imx6ul.dtsi` was patched in the kernel to >> > use one of the new defines. >> > >> > Because of this, at the current state, migrating a board which is >> > somehow based on `imx6ul.dtsi` will give a dtc error given by a value >> > being used in the upstream dtsi which is not defined in the local >> > header, because local includes always have priority with respect >> > to upstream ones even when setting OF_UPSTREAM. >> >> So you should just drop the local DT bindings header: >> include/dt-bindings/clock/imx6ul-clock.h and that should resolve the >> problem for you, right? > > > Yes, that indeed solves my problem. > But if I drop it, I will force all other boards which are not yet > migrated to OF_UPSTREAM and include `imx6ul.dtsi` to > use the upstream header instead of the local one. > I did not feel comfortable in doing so, because if any change is made > to the upstream header in the future which is somehow not backwards > compatible, then all boards which are still not using OF_UPSTREAM > would not compile. > > I also thought this would not be limited to the single header file which > caused my problem. I imagine there would be other cases of local > headers which are missing one or more commits from mainline kernel > and cause the same type of problem when moving to OF_UPSTREAM. > > The opposite problem also exists. > For example: > 675575880f ("phycore-am64x: Migrate to OF_UPSTREAM") > does not drop include/dt-bindings/net/ti-dp83867.h, and I think the > migration worked properly only because at the moment there is no > difference between local and upstream headers. > If and when the upstream headers and devicetrees will be patched, > this will cause problems, because the local include will be used > even if it's out-of-date. > I have tested this: by simulating a kernel patch to the upstream files, > which adds an extra define in ti-dp83867.h and updates > k3-am64-phycore-som.dtsi to use this new define, current state > u-boot will fail to build because that define is not present in the local > include header. > By including my patch, the build is successful. > > This is the reason why I proposed this Makefile patch, but of course I > am completely open to suggestions and ideas better than mine, which > I suspect are fairly easy to come by :) >
Based on the discussion so far, you can extend the commit message so other people can comment on it Michael > Thank you, > Regards, > Patrick > >> >> >> -Sumit >> >> >> > >> > Regards, >> > Patrick >> > >> >> >> >> > >> >> > The opposite problem also exists: by always prioritizing upstream >> >> > includes, if changes are made in the kernel headers and devicetree >> >> > files that are not backwards compatible, again all boards which have not >> >> > been migrated to OF_UPSTREAM will break. >> >> > >> >> > This patch fixes this problem by prioritizing upstream includes when >> >> > `CONFIG_OF_UPSTREAM=y`, while keeping current prioritization when >> >> > it is not. >> >> > >> >> > Signed-off-by: Patrick Barsanti <[email protected]> >> >> > --- >> >> > 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.43.0 >> >> > -- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 [email protected] __________________________________ Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 [email protected] www.amarulasolutions.com
