Re: [PATCH v2] Makefile: Fix include directory for OF_UPSTREAM
HI Sumit On Mon, Jun 17, 2024 at 10:22 AM Sumit Garg 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 > wrote: > > > > Hi Sumit > > > > On Thu, Jun 13, 2024 at 9:02 AM Sumit Garg wrote: > > > > > > On Mon, 3 Jun 2024 at 20:38, 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. > > > > 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_7...@mail.gmail.com/ > > -Sumit Michael
Re: [PATCH v2] Makefile: Fix include directory for OF_UPSTREAM
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 wrote: > > Hi Sumit > > On Thu, Jun 13, 2024 at 9:02 AM Sumit Garg wrote: > > > > On Mon, 3 Jun 2024 at 20:38, 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. > > > 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
Re: [PATCH v2] Makefile: Fix include directory for OF_UPSTREAM
Hi Sumit On Thu, Jun 13, 2024 at 9:02 AM Sumit Garg wrote: > > On Mon, 3 Jun 2024 at 20:38, 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. > > 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.barsa...@amarulasolutions.com/ > > Signed-off-by: Patrick Barsanti > > Tested-by: Michael Trimarchi > > --- > > > > 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 > > -- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 mich...@amarulasolutions.com __ Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 i...@amarulasolutions.com www.amarulasolutions.com
Re: [PATCH v2] Makefile: Fix include directory for OF_UPSTREAM
On Mon, 3 Jun 2024 at 20:38, 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. > 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.barsa...@amarulasolutions.com/ > Signed-off-by: Patrick Barsanti > Tested-by: Michael Trimarchi > --- > > 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 >
Re: [PATCH v2] Makefile: Fix include directory for OF_UPSTREAM
Hi Michael, On Wed, Jun 12, 2024 at 12:46 PM Michael Nazzareno Trimarchi wrote: > > HI > > On Wed, Jun 12, 2024 at 7:25 PM Tom Rini 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 > > > 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 All the best, Tony > > Michael > > > -- > > Tom
Re: [PATCH v2] Makefile: Fix include directory for OF_UPSTREAM
HI On Wed, Jun 12, 2024 at 7:25 PM Tom Rini 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 > > 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
Re: [PATCH v2] Makefile: Fix include directory for OF_UPSTREAM
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 > 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. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2] Makefile: Fix include directory for OF_UPSTREAM
Hi Marek On Wed, Jun 12, 2024 at 11:32 AM Marek Vasut 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 > [...]
Re: [PATCH v2] Makefile: Fix include directory for OF_UPSTREAM
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 . [...]
Re: [PATCH v2] Makefile: Fix include directory for OF_UPSTREAM
Hi Tom On Mon, Jun 3, 2024 at 5:08 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. > 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.barsa...@amarulasolutions.com/ > Signed-off-by: Patrick Barsanti > Tested-by: Michael Trimarchi > --- > > 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 > -- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 mich...@amarulasolutions.com __ Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 i...@amarulasolutions.com www.amarulasolutions.com
[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.barsa...@amarulasolutions.com/ Signed-off-by: Patrick Barsanti Tested-by: Michael Trimarchi --- 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
