[PATCH] Makefile: Fix include directory for OF_UPSTREAM

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.
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 patrick.barsanti@amarulasolutions.com --- 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)

Hi Patrick,
On Tue, 28 May 2024 at 14:16, Patrick Barsanti patrick.barsanti@amarulasolutions.com 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
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 patrick.barsanti@amarulasolutions.com
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

Hi Sumit,
On Wed, 29 May 2024 at 08:57, Sumit Garg sumit.garg@linaro.org wrote:
Hi Patrick,
On Tue, 28 May 2024 at 14:16, Patrick Barsanti patrick.barsanti@amarulasolutions.com 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.
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 patrick.barsanti@amarulasolutions.com
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

On Wed, 29 May 2024 at 14:45, Patrick Barsanti patrick.barsanti@amarulasolutions.com wrote:
Hi Sumit,
On Wed, 29 May 2024 at 08:57, Sumit Garg sumit.garg@linaro.org wrote:
Hi Patrick,
On Tue, 28 May 2024 at 14:16, Patrick Barsanti patrick.barsanti@amarulasolutions.com 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?
-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 patrick.barsanti@amarulasolutions.com
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

Hi Sumit,
On Wed, 29 May 2024 at 14:00, Sumit Garg sumit.garg@linaro.org wrote:
On Wed, 29 May 2024 at 14:45, Patrick Barsanti patrick.barsanti@amarulasolutions.com wrote:
Hi Sumit,
On Wed, 29 May 2024 at 08:57, Sumit Garg sumit.garg@linaro.org wrote:
Hi Patrick,
On Tue, 28 May 2024 at 14:16, Patrick Barsanti patrick.barsanti@amarulasolutions.com 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 :)
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 <
patrick.barsanti@amarulasolutions.com>
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

On Thu, 30 May 2024 at 14:43, Patrick Barsanti patrick.barsanti@amarulasolutions.com wrote:
Hi Sumit,
On Wed, 29 May 2024 at 14:00, Sumit Garg sumit.garg@linaro.org wrote:
On Wed, 29 May 2024 at 14:45, Patrick Barsanti patrick.barsanti@amarulasolutions.com wrote:
Hi Sumit,
On Wed, 29 May 2024 at 08:57, Sumit Garg sumit.garg@linaro.org wrote:
Hi Patrick,
On Tue, 28 May 2024 at 14:16, Patrick Barsanti patrick.barsanti@amarulasolutions.com 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.
DT headers backwards compatibility is something that upstream DT maintainers assure us about. And we try to closely monitor any DT ABI breakage while syncing upstream DT. Till now after we added OF_UPSTREAM support, fortunately there hasn't been any DT ABI breakage reports. So we should switch to upstream DT headers wherever we can and drop any redundant local DT headers. Migration to OF_UPSTREAM can surely be the next step but we want to encourage more and more code reuse from upstream DT sources.
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 redundant local DT headers should be dropped wherever we can. Adding further defines or new clocks in your case isn't going to break backwards compatibility but rather dropping any existing clock defines would be a DT ABI breakage.
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.
That's a leftover from migration, feel free to propose a patch to drop it.
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 :)
The additional complexity added by your patch is just needed to keep local DT headers but that goes against using upstream DT as much as we can and thereby reducing maintenance burden.
-Sumit

Hi Patrick
On Thu, May 30, 2024 at 11:13 AM Patrick Barsanti patrick.barsanti@amarulasolutions.com wrote:
Hi Sumit,
On Wed, 29 May 2024 at 14:00, Sumit Garg sumit.garg@linaro.org wrote:
On Wed, 29 May 2024 at 14:45, Patrick Barsanti patrick.barsanti@amarulasolutions.com wrote:
Hi Sumit,
On Wed, 29 May 2024 at 08:57, Sumit Garg sumit.garg@linaro.org wrote:
Hi Patrick,
On Tue, 28 May 2024 at 14:16, Patrick Barsanti patrick.barsanti@amarulasolutions.com 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 patrick.barsanti@amarulasolutions.com
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
participants (3)
-
Michael Nazzareno Trimarchi
-
Patrick Barsanti
-
Sumit Garg