[PATCH v2] Makefile: Fix include directory for OF_UPSTREAM
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.
Link: https://lore.kernel.org/u-boot/20240528084224.113385-1-patrick.barsanti@amar... Signed-off-by: Patrick Barsanti patrick.barsanti@amarulasolutions.com Tested-by: Michael Trimarchi michael@amarulasolutions.com ---
Changes in v2: - Reword and correct the commit message following the discussion with Sumit, per Michael's suggestion.
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 Tom
On Mon, Jun 3, 2024 at 5:08 PM Patrick Barsanti patrick.barsanti@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.
Do you have some comments here? We would like to move upstream dts for imx6 boards too
Michael
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.
Link: https://lore.kernel.org/u-boot/20240528084224.113385-1-patrick.barsanti@amar... Signed-off-by: Patrick Barsanti patrick.barsanti@amarulasolutions.com Tested-by: Michael Trimarchi michael@amarulasolutions.com
Changes in v2:
- Reword and correct the commit message following the discussion
with Sumit, per Michael's suggestion.
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.45.1
On Wed, Jun 12, 2024 at 10:17:12AM +0200, Michael Nazzareno Trimarchi wrote:
Hi Tom
On Mon, Jun 3, 2024 at 5:08 PM Patrick Barsanti patrick.barsanti@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.
Do you have some comments here? We would like to move upstream dts for imx6 boards too
I've been waiting for the people that wanted the commit message clarified to Ack/Review the patch. Did I just miss that? Sorry.
HI
On Wed, Jun 12, 2024 at 7:25 PM Tom Rini trini@konsulko.com wrote:
On Wed, Jun 12, 2024 at 10:17:12AM +0200, Michael Nazzareno Trimarchi wrote:
Hi Tom
On Mon, Jun 3, 2024 at 5:08 PM Patrick Barsanti patrick.barsanti@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.
Do you have some comments here? We would like to move upstream dts for imx6 boards too
I've been waiting for the people that wanted the commit message clarified to Ack/Review the patch. Did I just miss that? Sorry.
Agreed, let's wait for more feedback
Michael
-- Tom
Hi Michael,
On Wed, Jun 12, 2024 at 12:46 PM Michael Nazzareno Trimarchi michael@amarulasolutions.com wrote:
HI
On Wed, Jun 12, 2024 at 7:25 PM Tom Rini trini@konsulko.com wrote:
On Wed, Jun 12, 2024 at 10:17:12AM +0200, Michael Nazzareno Trimarchi wrote:
Hi Tom
On Mon, Jun 3, 2024 at 5:08 PM Patrick Barsanti patrick.barsanti@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.
Do you have some comments here? We would like to move upstream dts for imx6 boards too
I've been waiting for the people that wanted the commit message clarified to Ack/Review the patch. Did I just miss that? Sorry.
Agreed, let's wait for more feedback
I have migrated to OF_UPSTREAM for ARCH_KIRWOOD boards, and also for a single board (DS116, Armada 385), and have given a lot of thoughts about this problem. I agreed with Michael. Either the whole Arch is migrated at the same time, or we need to prioritize upstream includes to avoid breakage for boards that have been migrated and also boards that have not been.
Reviewed-by: Tony Dinh mibodhi@gmail.com
All the best, Tony
Michael
-- Tom
On 6/3/24 5:07 PM, Patrick Barsanti 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.
... upstream directory ...
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.
Why not simply remove the headers ?
Add a WARNING if there are local duplicates .
[...]
Hi Marek
On Wed, Jun 12, 2024 at 11:32 AM Marek Vasut marek.vasut@mailbox.org wrote:
On 6/3/24 5:07 PM, Patrick Barsanti 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.
... upstream directory ...
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.
Why not simply remove the headers ?
If we remove the headers then other boards that are not migrated, can have some problem on building. We should consider not only new boards but even those that ones are still not migrated. I think that is already described here
Add a WARNING if there are local duplicates .
Why should a WARNING help here? We would like to have some migration plans but not break them.
Michael
[...]
On Mon, 3 Jun 2024 at 20:38, Patrick Barsanti patrick.barsanti@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.
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.
-Sumit
Link: https://lore.kernel.org/u-boot/20240528084224.113385-1-patrick.barsanti@amar... Signed-off-by: Patrick Barsanti patrick.barsanti@amarulasolutions.com Tested-by: Michael Trimarchi michael@amarulasolutions.com
Changes in v2:
- Reword and correct the commit message following the discussion
with Sumit, per Michael's suggestion.
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.45.1
Hi Sumit
On Thu, Jun 13, 2024 at 9:02 AM Sumit Garg sumit.garg@linaro.org wrote:
On Mon, 3 Jun 2024 at 20:38, Patrick Barsanti patrick.barsanti@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 imx6 board migration for us does not work
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.
Michael
-Sumit
Link: https://lore.kernel.org/u-boot/20240528084224.113385-1-patrick.barsanti@amar... Signed-off-by: Patrick Barsanti patrick.barsanti@amarulasolutions.com Tested-by: Michael Trimarchi michael@amarulasolutions.com
Changes in v2:
- Reword and correct the commit message following the discussion
with Sumit, per Michael's suggestion.
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.45.1
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@amarulasolutions.com wrote:
Hi Sumit
On Thu, Jun 13, 2024 at 9:02 AM Sumit Garg sumit.garg@linaro.org wrote:
On Mon, 3 Jun 2024 at 20:38, Patrick Barsanti patrick.barsanti@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/CABxfhNfWdD0NjPNskaxZxDTg9pDoDx5vBHrN9chvMh5X...
-Sumit
HI Sumit
On Mon, Jun 17, 2024 at 10:22 AM Sumit Garg sumit.garg@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@amarulasolutions.com wrote:
Hi Sumit
On Thu, Jun 13, 2024 at 9:02 AM Sumit Garg sumit.garg@linaro.org wrote:
On Mon, 3 Jun 2024 at 20:38, Patrick Barsanti patrick.barsanti@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/CABxfhNfWdD0NjPNskaxZxDTg9pDoDx5vBHrN9chvMh5X...
-Sumit
Michael
participants (6)
-
Marek Vasut -
Michael Nazzareno Trimarchi -
Patrick Barsanti -
Sumit Garg -
Tom Rini -
Tony Dinh