| Message ID | 20220130155225.726890-2-sjg@chromium.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [v2,1/2] Makefile: Create a file to indicate the config | expand |
On 1/30/22 16:52, Simon Glass wrote: > More than a year after this migration message appeared, we still have new > boards being added with this option. Add a check against this. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > Changes in v2: > - Rebase to master > > Makefile | 6 ++++ > scripts/fit_gen_whitelist.txt | 65 +++++++++++++++++++++++++++++++++++ > 2 files changed, 71 insertions(+) > create mode 100644 scripts/fit_gen_whitelist.txt > > diff --git a/Makefile b/Makefile > index 212124522e6..55faff3952f 100644 > --- a/Makefile > +++ b/Makefile > @@ -1110,6 +1110,12 @@ ifeq ($(CONFIG_OF_EMBED)$(CONFIG_EFI_APP),y) > @echo >&2 "====================================================" > endif > ifneq ($(CONFIG_SPL_FIT_GENERATOR),) > + # Only allow existing users of this deprecated option. Please migrate! > + @if ! grep -q $(shell cat .defconfig_name) \ > + $(srctree)/scripts/fit_gen_whitelist.txt; then \ > + echo >&2 "Error: CONFIG_SPL_FIT_GENERATOR is deprecated"; \ > + exit 1; \ > + fi > @echo >&2 "===================== WARNING ======================" > @echo >&2 "This board uses CONFIG_SPL_FIT_GENERATOR. Please migrate" > @echo >&2 "to binman instead, to avoid the proliferation of" > diff --git a/scripts/fit_gen_whitelist.txt b/scripts/fit_gen_whitelist.txt > new file mode 100644 > index 00000000000..ac0890b3f39 > --- /dev/null > +++ b/scripts/fit_gen_whitelist.txt > @@ -0,0 +1,65 @@ > +# List of boards that need to be migrated from SPL_FIT_GENERATOR to binman > +# See https://patchwork.ozlabs.org/project/uboot/list/?series=242992&state=* > +# for an example series (see patches 7 and 13 in particular) > + > +# Please do not add to this file > + > +# Some TI boards need migration > +am335x_evm_spiboot > +am64x_evm_a53 > +am64x_evm_r5 > +am65x_evm_r5_usbdfu > +am65x_evm_r5_usbmsc > + > +# MX8 needs migration > +cgtqmx8 > +imx8mm-icore-mx8mm-ctouch2 > +imx8mm-icore-mx8mm-edimm2.2 > +imx8qm_rom7720_a1_4G > + > +# Rockchip needs migration > +chromebook_bob > +evb-px30 > +evb-px5 > +evb-rk3308 > +evb-rk3328 > +evb-rk3399 > +evb-rk3568 > +ficus-rk3399 > +firefly-px30 > +firefly-rk3399 > +khadas-edge-captain-rk3399 > +khadas-edge-rk3399 > +khadas-edge-v-rk3399 > +leez-rk3399 > +lion-rk3368 > +nanopc-t4-rk3399 > +nanopi-m4-2gb-rk3399 > +nanopi-m4b-rk3399 > +nanopi-m4-rk3399 > +nanopi-neo4-rk3399 > +nanopi-r2s-rk3328 > +nanopi-r4s-rk3399 > +odroid-go2 > +roc-cc-rk3308 > +orangepi-rk3399 > +pinebook-pro-rk3399 > +puma-rk3399 > +px30-core-ctouch2-of10-px30 > +px30-core-ctouch2-px30 > +px30-core-edimm2.2-px30 > +roc-cc-rk3308 > +roc-cc-rk3328 > +rock64-rk3328 > +rock960-rk3399 > +rock-pi-4c-rk3399 > +rock-pi-4-rk3399 > +rock-pi-e-rk3328 > +rock-pi-n10-rk3399pro > +rockpro64-rk3399 > +roc-pc-mezzanine-rk3399 > +roc-pc-rk3399 > + > +# Zynqmp needs mnigration nit: I normally use zynqmp or ZynqMP. But there is typo. What's the migration path? Buildman? M
Hi Michal, On Sun, 30 Jan 2022 at 12:41, Michal Simek <monstr@monstr.eu> wrote: > > > > On 1/30/22 16:52, Simon Glass wrote: > > More than a year after this migration message appeared, we still have new > > boards being added with this option. Add a check against this. > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > > > Changes in v2: > > - Rebase to master > > > > Makefile | 6 ++++ > > scripts/fit_gen_whitelist.txt | 65 +++++++++++++++++++++++++++++++++++ > > 2 files changed, 71 insertions(+) > > create mode 100644 scripts/fit_gen_whitelist.txt > > > > diff --git a/Makefile b/Makefile > > index 212124522e6..55faff3952f 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -1110,6 +1110,12 @@ ifeq ($(CONFIG_OF_EMBED)$(CONFIG_EFI_APP),y) > > @echo >&2 "====================================================" > > endif > > ifneq ($(CONFIG_SPL_FIT_GENERATOR),) > > + # Only allow existing users of this deprecated option. Please migrate! > > + @if ! grep -q $(shell cat .defconfig_name) \ > > + $(srctree)/scripts/fit_gen_whitelist.txt; then \ > > + echo >&2 "Error: CONFIG_SPL_FIT_GENERATOR is deprecated"; \ > > + exit 1; \ > > + fi > > @echo >&2 "===================== WARNING ======================" > > @echo >&2 "This board uses CONFIG_SPL_FIT_GENERATOR. Please migrate" > > @echo >&2 "to binman instead, to avoid the proliferation of" > > diff --git a/scripts/fit_gen_whitelist.txt b/scripts/fit_gen_whitelist.txt > > new file mode 100644 > > index 00000000000..ac0890b3f39 > > --- /dev/null > > +++ b/scripts/fit_gen_whitelist.txt > > @@ -0,0 +1,65 @@ > > +# List of boards that need to be migrated from SPL_FIT_GENERATOR to binman > > +# See https://patchwork.ozlabs.org/project/uboot/list/?series=242992&state=* > > +# for an example series (see patches 7 and 13 in particular) > > + > > +# Please do not add to this file > > + > > +# Some TI boards need migration > > +am335x_evm_spiboot > > +am64x_evm_a53 > > +am64x_evm_r5 > > +am65x_evm_r5_usbdfu > > +am65x_evm_r5_usbmsc > > + > > +# MX8 needs migration > > +cgtqmx8 > > +imx8mm-icore-mx8mm-ctouch2 > > +imx8mm-icore-mx8mm-edimm2.2 > > +imx8qm_rom7720_a1_4G > > + > > +# Rockchip needs migration > > +chromebook_bob > > +evb-px30 > > +evb-px5 > > +evb-rk3308 > > +evb-rk3328 > > +evb-rk3399 > > +evb-rk3568 > > +ficus-rk3399 > > +firefly-px30 > > +firefly-rk3399 > > +khadas-edge-captain-rk3399 > > +khadas-edge-rk3399 > > +khadas-edge-v-rk3399 > > +leez-rk3399 > > +lion-rk3368 > > +nanopc-t4-rk3399 > > +nanopi-m4-2gb-rk3399 > > +nanopi-m4b-rk3399 > > +nanopi-m4-rk3399 > > +nanopi-neo4-rk3399 > > +nanopi-r2s-rk3328 > > +nanopi-r4s-rk3399 > > +odroid-go2 > > +roc-cc-rk3308 > > +orangepi-rk3399 > > +pinebook-pro-rk3399 > > +puma-rk3399 > > +px30-core-ctouch2-of10-px30 > > +px30-core-ctouch2-px30 > > +px30-core-edimm2.2-px30 > > +roc-cc-rk3308 > > +roc-cc-rk3328 > > +rock64-rk3328 > > +rock960-rk3399 > > +rock-pi-4c-rk3399 > > +rock-pi-4-rk3399 > > +rock-pi-e-rk3328 > > +rock-pi-n10-rk3399pro > > +rockpro64-rk3399 > > +roc-pc-mezzanine-rk3399 > > +roc-pc-rk3399 > > + > > +# Zynqmp needs mnigration > > nit: I normally use zynqmp or ZynqMP. > But there is typo. OK will fix. > > What's the migration path? Buildman? Actually it is a binman description, see the imx8 ones, for example. It can generate a FIT for you. Regards, Simon
On Sun, Jan 30, 2022 at 08:52:25AM -0700, Simon Glass wrote: > More than a year after this migration message appeared, we still have new > boards being added with this option. Add a check against this. > > Signed-off-by: Simon Glass <sjg@chromium.org> Please just make this an error in checkpatch.pl instead.
Hi Tom, On Mon, 31 Jan 2022 at 07:24, Tom Rini <trini@konsulko.com> wrote: > > On Sun, Jan 30, 2022 at 08:52:25AM -0700, Simon Glass wrote: > > > More than a year after this migration message appeared, we still have new > > boards being added with this option. Add a check against this. > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > Please just make this an error in checkpatch.pl instead. I couldn't think of a way of doing that...do you have an idea? Regards, Simon
On Mon, Jan 31, 2022 at 09:13:02AM -0700, Simon Glass wrote: > Hi Tom, > > On Mon, 31 Jan 2022 at 07:24, Tom Rini <trini@konsulko.com> wrote: > > > > On Sun, Jan 30, 2022 at 08:52:25AM -0700, Simon Glass wrote: > > > > > More than a year after this migration message appeared, we still have new > > > boards being added with this option. Add a check against this. > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > Please just make this an error in checkpatch.pl instead. > > I couldn't think of a way of doing that...do you have an idea? Yes, 2f3e8d6a86cb ("checkpatch: report ERROR only on disabling of fdt and initrd relocation") updates the check I had for fdt_high/initrd_high being in the file at all to only be for additions. And yes, I check every PR for new checkpatch ERROR lines and only ignore the ones for code imported from other projects.
Hi Tom, On Mon, 31 Jan 2022 at 09:15, Tom Rini <trini@konsulko.com> wrote: > > On Mon, Jan 31, 2022 at 09:13:02AM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Mon, 31 Jan 2022 at 07:24, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Sun, Jan 30, 2022 at 08:52:25AM -0700, Simon Glass wrote: > > > > > > > More than a year after this migration message appeared, we still have new > > > > boards being added with this option. Add a check against this. > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > > Please just make this an error in checkpatch.pl instead. > > > > I couldn't think of a way of doing that...do you have an idea? > > Yes, 2f3e8d6a86cb ("checkpatch: report ERROR only on disabling of fdt > and initrd relocation") updates the check I had for fdt_high/initrd_high > being in the file at all to only be for additions. And yes, I check > every PR for new checkpatch ERROR lines and only ignore the ones for > code imported from other projects. Yes, I understand that, but SPL_FIT_GENERATOR defaults to on for certain boards, so there is no need to mention it anywhere in the patch. Also someone could adjust the condition in the Kconfig to add other boards. Regards, Simon
On Mon, Jan 31, 2022 at 10:27:41AM -0700, Simon Glass wrote: > Hi Tom, > > On Mon, 31 Jan 2022 at 09:15, Tom Rini <trini@konsulko.com> wrote: > > > > On Mon, Jan 31, 2022 at 09:13:02AM -0700, Simon Glass wrote: > > > Hi Tom, > > > > > > On Mon, 31 Jan 2022 at 07:24, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Sun, Jan 30, 2022 at 08:52:25AM -0700, Simon Glass wrote: > > > > > > > > > More than a year after this migration message appeared, we still have new > > > > > boards being added with this option. Add a check against this. > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > > > > Please just make this an error in checkpatch.pl instead. > > > > > > I couldn't think of a way of doing that...do you have an idea? > > > > Yes, 2f3e8d6a86cb ("checkpatch: report ERROR only on disabling of fdt > > and initrd relocation") updates the check I had for fdt_high/initrd_high > > being in the file at all to only be for additions. And yes, I check > > every PR for new checkpatch ERROR lines and only ignore the ones for > > code imported from other projects. > > Yes, I understand that, but SPL_FIT_GENERATOR defaults to on for > certain boards, so there is no need to mention it anywhere in the > patch. Also someone could adjust the condition in the Kconfig to add > other boards. Then you want something a bit more like the fdt|initrd_high check now, along with updating the help around SPL_FIT_GENERATOR to note that this option is deprecated, is the path forward then I think.
Hi Tom, On Mon, 31 Jan 2022 at 11:00, Tom Rini <trini@konsulko.com> wrote: > > On Mon, Jan 31, 2022 at 10:27:41AM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Mon, 31 Jan 2022 at 09:15, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Mon, Jan 31, 2022 at 09:13:02AM -0700, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Mon, 31 Jan 2022 at 07:24, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > On Sun, Jan 30, 2022 at 08:52:25AM -0700, Simon Glass wrote: > > > > > > > > > > > More than a year after this migration message appeared, we still have new > > > > > > boards being added with this option. Add a check against this. > > > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > > > > > > Please just make this an error in checkpatch.pl instead. > > > > > > > > I couldn't think of a way of doing that...do you have an idea? > > > > > > Yes, 2f3e8d6a86cb ("checkpatch: report ERROR only on disabling of fdt > > > and initrd relocation") updates the check I had for fdt_high/initrd_high > > > being in the file at all to only be for additions. And yes, I check > > > every PR for new checkpatch ERROR lines and only ignore the ones for > > > code imported from other projects. > > > > Yes, I understand that, but SPL_FIT_GENERATOR defaults to on for > > certain boards, so there is no need to mention it anywhere in the > > patch. Also someone could adjust the condition in the Kconfig to add > > other boards. > > Then you want something a bit more like the fdt|initrd_high check now, > along with updating the help around SPL_FIT_GENERATOR to note that this > option is deprecated, is the path forward then I think. I'm still a bit lost. What I want: break the build if someone adds a new board that uses SPL_FIT_GENERATOR What you are offering: checkpatch check for people adding that option But the patch doesn't generally include that option. I can certainly mention in the Kconfig help that the option is deprecated, but without checking if it is defined for a NEW board, I cannot prevent it from growing. What am I missing? Can you be more specific? Regards, Simon
On Mon, Jan 31, 2022 at 12:57:57PM -0700, Simon Glass wrote: > Hi Tom, > > On Mon, 31 Jan 2022 at 11:00, Tom Rini <trini@konsulko.com> wrote: > > > > On Mon, Jan 31, 2022 at 10:27:41AM -0700, Simon Glass wrote: > > > Hi Tom, > > > > > > On Mon, 31 Jan 2022 at 09:15, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Mon, Jan 31, 2022 at 09:13:02AM -0700, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Mon, 31 Jan 2022 at 07:24, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > On Sun, Jan 30, 2022 at 08:52:25AM -0700, Simon Glass wrote: > > > > > > > > > > > > > More than a year after this migration message appeared, we still have new > > > > > > > boards being added with this option. Add a check against this. > > > > > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > > > > > > > > Please just make this an error in checkpatch.pl instead. > > > > > > > > > > I couldn't think of a way of doing that...do you have an idea? > > > > > > > > Yes, 2f3e8d6a86cb ("checkpatch: report ERROR only on disabling of fdt > > > > and initrd relocation") updates the check I had for fdt_high/initrd_high > > > > being in the file at all to only be for additions. And yes, I check > > > > every PR for new checkpatch ERROR lines and only ignore the ones for > > > > code imported from other projects. > > > > > > Yes, I understand that, but SPL_FIT_GENERATOR defaults to on for > > > certain boards, so there is no need to mention it anywhere in the > > > patch. Also someone could adjust the condition in the Kconfig to add > > > other boards. > > > > Then you want something a bit more like the fdt|initrd_high check now, > > along with updating the help around SPL_FIT_GENERATOR to note that this > > option is deprecated, is the path forward then I think. > > I'm still a bit lost. > > What I want: break the build if someone adds a new board that uses > SPL_FIT_GENERATOR > > What you are offering: checkpatch check for people adding that option > > But the patch doesn't generally include that option. > > I can certainly mention in the Kconfig help that the option is > deprecated, but without checking if it is defined for a NEW board, I > cannot prevent it from growing. > > What am I missing? Can you be more specific? How do you add a new board that enables SPL_FIT_GENERATOR without "SPL_FIT_GENERATOR" being in the resulting patch, other than being ARCH_ZYNQMP/ARCH_ROCKCHIP ?
Hi Tom, On Mon, 31 Jan 2022 at 13:40, Tom Rini <trini@konsulko.com> wrote: > > On Mon, Jan 31, 2022 at 12:57:57PM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Mon, 31 Jan 2022 at 11:00, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Mon, Jan 31, 2022 at 10:27:41AM -0700, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Mon, 31 Jan 2022 at 09:15, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > On Mon, Jan 31, 2022 at 09:13:02AM -0700, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Mon, 31 Jan 2022 at 07:24, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > On Sun, Jan 30, 2022 at 08:52:25AM -0700, Simon Glass wrote: > > > > > > > > > > > > > > > More than a year after this migration message appeared, we still have new > > > > > > > > boards being added with this option. Add a check against this. > > > > > > > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > > > > > > > > > > Please just make this an error in checkpatch.pl instead. > > > > > > > > > > > > I couldn't think of a way of doing that...do you have an idea? > > > > > > > > > > Yes, 2f3e8d6a86cb ("checkpatch: report ERROR only on disabling of fdt > > > > > and initrd relocation") updates the check I had for fdt_high/initrd_high > > > > > being in the file at all to only be for additions. And yes, I check > > > > > every PR for new checkpatch ERROR lines and only ignore the ones for > > > > > code imported from other projects. > > > > > > > > Yes, I understand that, but SPL_FIT_GENERATOR defaults to on for > > > > certain boards, so there is no need to mention it anywhere in the > > > > patch. Also someone could adjust the condition in the Kconfig to add > > > > other boards. > > > > > > Then you want something a bit more like the fdt|initrd_high check now, > > > along with updating the help around SPL_FIT_GENERATOR to note that this > > > option is deprecated, is the path forward then I think. > > > > I'm still a bit lost. > > > > What I want: break the build if someone adds a new board that uses > > SPL_FIT_GENERATOR > > > > What you are offering: checkpatch check for people adding that option > > > > But the patch doesn't generally include that option. > > > > I can certainly mention in the Kconfig help that the option is > > deprecated, but without checking if it is defined for a NEW board, I > > cannot prevent it from growing. > > > > What am I missing? Can you be more specific? > > How do you add a new board that enables SPL_FIT_GENERATOR without > "SPL_FIT_GENERATOR" being in the resulting patch, other than being > ARCH_ZYNQMP/ARCH_ROCKCHIP ? Well that's the case I am most concerned with, actually. Also, someone might add a new condition to SPL_FIT_GENERATOR. Regards, Simon
> Date: Mon, 31 Jan 2022 15:40:39 -0500 > From: Tom Rini <trini@konsulko.com> > > On Mon, Jan 31, 2022 at 12:57:57PM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Mon, 31 Jan 2022 at 11:00, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Mon, Jan 31, 2022 at 10:27:41AM -0700, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Mon, 31 Jan 2022 at 09:15, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > On Mon, Jan 31, 2022 at 09:13:02AM -0700, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Mon, 31 Jan 2022 at 07:24, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > On Sun, Jan 30, 2022 at 08:52:25AM -0700, Simon Glass wrote: > > > > > > > > > > > > > > > More than a year after this migration message appeared, we still have new > > > > > > > > boards being added with this option. Add a check against this. > > > > > > > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > > > > > > > > > > Please just make this an error in checkpatch.pl instead. > > > > > > > > > > > > I couldn't think of a way of doing that...do you have an idea? > > > > > > > > > > Yes, 2f3e8d6a86cb ("checkpatch: report ERROR only on disabling of fdt > > > > > and initrd relocation") updates the check I had for fdt_high/initrd_high > > > > > being in the file at all to only be for additions. And yes, I check > > > > > every PR for new checkpatch ERROR lines and only ignore the ones for > > > > > code imported from other projects. > > > > > > > > Yes, I understand that, but SPL_FIT_GENERATOR defaults to on for > > > > certain boards, so there is no need to mention it anywhere in the > > > > patch. Also someone could adjust the condition in the Kconfig to add > > > > other boards. > > > > > > Then you want something a bit more like the fdt|initrd_high check now, > > > along with updating the help around SPL_FIT_GENERATOR to note that this > > > option is deprecated, is the path forward then I think. > > > > I'm still a bit lost. > > > > What I want: break the build if someone adds a new board that uses > > SPL_FIT_GENERATOR > > > > What you are offering: checkpatch check for people adding that option > > > > But the patch doesn't generally include that option. > > > > I can certainly mention in the Kconfig help that the option is > > deprecated, but without checking if it is defined for a NEW board, I > > cannot prevent it from growing. > > > > What am I missing? Can you be more specific? > > How do you add a new board that enables SPL_FIT_GENERATOR without > "SPL_FIT_GENERATOR" being in the resulting patch, other than being > ARCH_ZYNQMP/ARCH_ROCKCHIP ? Well, that's a problem isn't it? Simon is basically saying no to any new rk3399 board, which doesn't make any sense since this isn't a board-specific issue but a SoC specific issue. Adding new boards doesn't make it more difficult to fix the issue. I guess he is hoping that saying no to a new rk3399 board will trick someone into actually doing the work of adding the necessary code to binman?
On Mon, Jan 31, 2022 at 02:22:41PM -0700, Simon Glass wrote: > Hi Tom, > > On Mon, 31 Jan 2022 at 13:40, Tom Rini <trini@konsulko.com> wrote: > > > > On Mon, Jan 31, 2022 at 12:57:57PM -0700, Simon Glass wrote: > > > Hi Tom, > > > > > > On Mon, 31 Jan 2022 at 11:00, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Mon, Jan 31, 2022 at 10:27:41AM -0700, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Mon, 31 Jan 2022 at 09:15, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > On Mon, Jan 31, 2022 at 09:13:02AM -0700, Simon Glass wrote: > > > > > > > Hi Tom, > > > > > > > > > > > > > > On Mon, 31 Jan 2022 at 07:24, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > On Sun, Jan 30, 2022 at 08:52:25AM -0700, Simon Glass wrote: > > > > > > > > > > > > > > > > > More than a year after this migration message appeared, we still have new > > > > > > > > > boards being added with this option. Add a check against this. > > > > > > > > > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > > > > > > > > > > > > Please just make this an error in checkpatch.pl instead. > > > > > > > > > > > > > > I couldn't think of a way of doing that...do you have an idea? > > > > > > > > > > > > Yes, 2f3e8d6a86cb ("checkpatch: report ERROR only on disabling of fdt > > > > > > and initrd relocation") updates the check I had for fdt_high/initrd_high > > > > > > being in the file at all to only be for additions. And yes, I check > > > > > > every PR for new checkpatch ERROR lines and only ignore the ones for > > > > > > code imported from other projects. > > > > > > > > > > Yes, I understand that, but SPL_FIT_GENERATOR defaults to on for > > > > > certain boards, so there is no need to mention it anywhere in the > > > > > patch. Also someone could adjust the condition in the Kconfig to add > > > > > other boards. > > > > > > > > Then you want something a bit more like the fdt|initrd_high check now, > > > > along with updating the help around SPL_FIT_GENERATOR to note that this > > > > option is deprecated, is the path forward then I think. > > > > > > I'm still a bit lost. > > > > > > What I want: break the build if someone adds a new board that uses > > > SPL_FIT_GENERATOR > > > > > > What you are offering: checkpatch check for people adding that option > > > > > > But the patch doesn't generally include that option. > > > > > > I can certainly mention in the Kconfig help that the option is > > > deprecated, but without checking if it is defined for a NEW board, I > > > cannot prevent it from growing. > > > > > > What am I missing? Can you be more specific? > > > > How do you add a new board that enables SPL_FIT_GENERATOR without > > "SPL_FIT_GENERATOR" being in the resulting patch, other than being > > ARCH_ZYNQMP/ARCH_ROCKCHIP ? > > Well that's the case I am most concerned with, actually. Also, someone > might add a new condition to SPL_FIT_GENERATOR. For the current cases, we just need to get them migrated since it's all the same logic? So it would I think be a one-and-done thing. For a new conditional, it should trip checkpatch by having the words in it, and also since the help would also say to not do this, and we already generate a warning, it shouldn't be an issue?
Hi Tom, (yes Mark I am trying to stop further boards going in that use the shell scripts) On Mon, 31 Jan 2022 at 15:05, Tom Rini <trini@konsulko.com> wrote: > > On Mon, Jan 31, 2022 at 02:22:41PM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Mon, 31 Jan 2022 at 13:40, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Mon, Jan 31, 2022 at 12:57:57PM -0700, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Mon, 31 Jan 2022 at 11:00, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > On Mon, Jan 31, 2022 at 10:27:41AM -0700, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Mon, 31 Jan 2022 at 09:15, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > On Mon, Jan 31, 2022 at 09:13:02AM -0700, Simon Glass wrote: > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > On Mon, 31 Jan 2022 at 07:24, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > > > On Sun, Jan 30, 2022 at 08:52:25AM -0700, Simon Glass wrote: > > > > > > > > > > > > > > > > > > > More than a year after this migration message appeared, we still have new > > > > > > > > > > boards being added with this option. Add a check against this. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > > > > > > > > > > > > > > Please just make this an error in checkpatch.pl instead. > > > > > > > > > > > > > > > > I couldn't think of a way of doing that...do you have an idea? > > > > > > > > > > > > > > Yes, 2f3e8d6a86cb ("checkpatch: report ERROR only on disabling of fdt > > > > > > > and initrd relocation") updates the check I had for fdt_high/initrd_high > > > > > > > being in the file at all to only be for additions. And yes, I check > > > > > > > every PR for new checkpatch ERROR lines and only ignore the ones for > > > > > > > code imported from other projects. > > > > > > > > > > > > Yes, I understand that, but SPL_FIT_GENERATOR defaults to on for > > > > > > certain boards, so there is no need to mention it anywhere in the > > > > > > patch. Also someone could adjust the condition in the Kconfig to add > > > > > > other boards. > > > > > > > > > > Then you want something a bit more like the fdt|initrd_high check now, > > > > > along with updating the help around SPL_FIT_GENERATOR to note that this > > > > > option is deprecated, is the path forward then I think. > > > > > > > > I'm still a bit lost. > > > > > > > > What I want: break the build if someone adds a new board that uses > > > > SPL_FIT_GENERATOR > > > > > > > > What you are offering: checkpatch check for people adding that option > > > > > > > > But the patch doesn't generally include that option. > > > > > > > > I can certainly mention in the Kconfig help that the option is > > > > deprecated, but without checking if it is defined for a NEW board, I > > > > cannot prevent it from growing. > > > > > > > > What am I missing? Can you be more specific? > > > > > > How do you add a new board that enables SPL_FIT_GENERATOR without > > > "SPL_FIT_GENERATOR" being in the resulting patch, other than being > > > ARCH_ZYNQMP/ARCH_ROCKCHIP ? > > > > Well that's the case I am most concerned with, actually. Also, someone > > might add a new condition to SPL_FIT_GENERATOR. > > For the current cases, we just need to get them migrated since it's all > the same logic? So it would I think be a one-and-done thing. For a new Yes I think so and some of them are done. These are what I can find: ./arch/riscv/lib/mkimage_fit_opensbi.sh ./arch/arm/mach-zynqmp/mkimage_fit_atf.sh ./arch/arm/mach-imx/mkimage_fit_atf.sh ./arch/arm/mach-rockchip/make_fit_atf.py but they are not used by that many boards. I feel that the amount of pending migration is somewhat overwhelming and we should take a stronger line in mainline. Perhaps I should send a patch to simply remove the option? Would that be acceptable? Regards, Simon > conditional, it should trip checkpatch by having the words in it, and > also since the help would also say to not do this, and we already > generate a warning, it shouldn't be an issue? > > -- > Tom
On Mon, Jan 31, 2022 at 03:59:08PM -0700, Simon Glass wrote: > Hi Tom, > > (yes Mark I am trying to stop further boards going in that use the > shell scripts) > > On Mon, 31 Jan 2022 at 15:05, Tom Rini <trini@konsulko.com> wrote: > > > > On Mon, Jan 31, 2022 at 02:22:41PM -0700, Simon Glass wrote: > > > Hi Tom, > > > > > > On Mon, 31 Jan 2022 at 13:40, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Mon, Jan 31, 2022 at 12:57:57PM -0700, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Mon, 31 Jan 2022 at 11:00, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > On Mon, Jan 31, 2022 at 10:27:41AM -0700, Simon Glass wrote: > > > > > > > Hi Tom, > > > > > > > > > > > > > > On Mon, 31 Jan 2022 at 09:15, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > On Mon, Jan 31, 2022 at 09:13:02AM -0700, Simon Glass wrote: > > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > > > On Mon, 31 Jan 2022 at 07:24, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > > > > > On Sun, Jan 30, 2022 at 08:52:25AM -0700, Simon Glass wrote: > > > > > > > > > > > > > > > > > > > > > More than a year after this migration message appeared, we still have new > > > > > > > > > > > boards being added with this option. Add a check against this. > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > > > > > > > > > > > > > > > > Please just make this an error in checkpatch.pl instead. > > > > > > > > > > > > > > > > > > I couldn't think of a way of doing that...do you have an idea? > > > > > > > > > > > > > > > > Yes, 2f3e8d6a86cb ("checkpatch: report ERROR only on disabling of fdt > > > > > > > > and initrd relocation") updates the check I had for fdt_high/initrd_high > > > > > > > > being in the file at all to only be for additions. And yes, I check > > > > > > > > every PR for new checkpatch ERROR lines and only ignore the ones for > > > > > > > > code imported from other projects. > > > > > > > > > > > > > > Yes, I understand that, but SPL_FIT_GENERATOR defaults to on for > > > > > > > certain boards, so there is no need to mention it anywhere in the > > > > > > > patch. Also someone could adjust the condition in the Kconfig to add > > > > > > > other boards. > > > > > > > > > > > > Then you want something a bit more like the fdt|initrd_high check now, > > > > > > along with updating the help around SPL_FIT_GENERATOR to note that this > > > > > > option is deprecated, is the path forward then I think. > > > > > > > > > > I'm still a bit lost. > > > > > > > > > > What I want: break the build if someone adds a new board that uses > > > > > SPL_FIT_GENERATOR > > > > > > > > > > What you are offering: checkpatch check for people adding that option > > > > > > > > > > But the patch doesn't generally include that option. > > > > > > > > > > I can certainly mention in the Kconfig help that the option is > > > > > deprecated, but without checking if it is defined for a NEW board, I > > > > > cannot prevent it from growing. > > > > > > > > > > What am I missing? Can you be more specific? > > > > > > > > How do you add a new board that enables SPL_FIT_GENERATOR without > > > > "SPL_FIT_GENERATOR" being in the resulting patch, other than being > > > > ARCH_ZYNQMP/ARCH_ROCKCHIP ? > > > > > > Well that's the case I am most concerned with, actually. Also, someone > > > might add a new condition to SPL_FIT_GENERATOR. > > > > For the current cases, we just need to get them migrated since it's all > > the same logic? So it would I think be a one-and-done thing. For a new > > Yes I think so and some of them are done. These are what I can find: > > ./arch/riscv/lib/mkimage_fit_opensbi.sh > ./arch/arm/mach-zynqmp/mkimage_fit_atf.sh > ./arch/arm/mach-imx/mkimage_fit_atf.sh > ./arch/arm/mach-rockchip/make_fit_atf.py > > but they are not used by that many boards. > > I feel that the amount of pending migration is somewhat overwhelming > and we should take a stronger line in mainline. > > Perhaps I should send a patch to simply remove the option? Would that > be acceptable? Is there something technically preventing their migration to buildman? Looking over examples for imx8* conversions, it's just adding a binman node and describing things there, yes?
On Mon, Jan 31, 2022 at 06:25:33PM -0500, Tom Rini wrote: > On Mon, Jan 31, 2022 at 03:59:08PM -0700, Simon Glass wrote: > > Hi Tom, > > > > (yes Mark I am trying to stop further boards going in that use the > > shell scripts) > > > > On Mon, 31 Jan 2022 at 15:05, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Mon, Jan 31, 2022 at 02:22:41PM -0700, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Mon, 31 Jan 2022 at 13:40, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > On Mon, Jan 31, 2022 at 12:57:57PM -0700, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Mon, 31 Jan 2022 at 11:00, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > On Mon, Jan 31, 2022 at 10:27:41AM -0700, Simon Glass wrote: > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > On Mon, 31 Jan 2022 at 09:15, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > > > On Mon, Jan 31, 2022 at 09:13:02AM -0700, Simon Glass wrote: > > > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > > > > > On Mon, 31 Jan 2022 at 07:24, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > > > > > > > On Sun, Jan 30, 2022 at 08:52:25AM -0700, Simon Glass wrote: > > > > > > > > > > > > > > > > > > > > > > > More than a year after this migration message appeared, we still have new > > > > > > > > > > > > boards being added with this option. Add a check against this. > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > > > > > > > > > > > > > > > > > > Please just make this an error in checkpatch.pl instead. > > > > > > > > > > > > > > > > > > > > I couldn't think of a way of doing that...do you have an idea? > > > > > > > > > > > > > > > > > > Yes, 2f3e8d6a86cb ("checkpatch: report ERROR only on disabling of fdt > > > > > > > > > and initrd relocation") updates the check I had for fdt_high/initrd_high > > > > > > > > > being in the file at all to only be for additions. And yes, I check > > > > > > > > > every PR for new checkpatch ERROR lines and only ignore the ones for > > > > > > > > > code imported from other projects. > > > > > > > > > > > > > > > > Yes, I understand that, but SPL_FIT_GENERATOR defaults to on for > > > > > > > > certain boards, so there is no need to mention it anywhere in the > > > > > > > > patch. Also someone could adjust the condition in the Kconfig to add > > > > > > > > other boards. > > > > > > > > > > > > > > Then you want something a bit more like the fdt|initrd_high check now, > > > > > > > along with updating the help around SPL_FIT_GENERATOR to note that this > > > > > > > option is deprecated, is the path forward then I think. > > > > > > > > > > > > I'm still a bit lost. > > > > > > > > > > > > What I want: break the build if someone adds a new board that uses > > > > > > SPL_FIT_GENERATOR > > > > > > > > > > > > What you are offering: checkpatch check for people adding that option > > > > > > > > > > > > But the patch doesn't generally include that option. > > > > > > > > > > > > I can certainly mention in the Kconfig help that the option is > > > > > > deprecated, but without checking if it is defined for a NEW board, I > > > > > > cannot prevent it from growing. > > > > > > > > > > > > What am I missing? Can you be more specific? > > > > > > > > > > How do you add a new board that enables SPL_FIT_GENERATOR without > > > > > "SPL_FIT_GENERATOR" being in the resulting patch, other than being > > > > > ARCH_ZYNQMP/ARCH_ROCKCHIP ? > > > > > > > > Well that's the case I am most concerned with, actually. Also, someone > > > > might add a new condition to SPL_FIT_GENERATOR. > > > > > > For the current cases, we just need to get them migrated since it's all > > > the same logic? So it would I think be a one-and-done thing. For a new > > > > Yes I think so and some of them are done. These are what I can find: > > > > ./arch/riscv/lib/mkimage_fit_opensbi.sh > > ./arch/arm/mach-zynqmp/mkimage_fit_atf.sh > > ./arch/arm/mach-imx/mkimage_fit_atf.sh > > ./arch/arm/mach-rockchip/make_fit_atf.py > > > > but they are not used by that many boards. > > > > I feel that the amount of pending migration is somewhat overwhelming > > and we should take a stronger line in mainline. > > > > Perhaps I should send a patch to simply remove the option? Would that > > be acceptable? > > Is there something technically preventing their migration to buildman? > Looking over examples for imx8* conversions, it's just adding a binman > node and describing things there, yes? Poking at this a bit more, it seems like the outstanding imx platforms to be converted still have pending patches and it's just part of the general imx backlog. The riscv one isn't used and should be removed. That leaves rockchip and zynqmp needing conversion. Michal has already commented in this thread and I'll leave it to him to say how long he needs to see how long zynqmp needs to update. That leaves Rockchip.
Hi Tom, On Mon, 31 Jan 2022 at 16:32, Tom Rini <trini@konsulko.com> wrote: > > On Mon, Jan 31, 2022 at 06:25:33PM -0500, Tom Rini wrote: > > On Mon, Jan 31, 2022 at 03:59:08PM -0700, Simon Glass wrote: > > > Hi Tom, > > > > > > (yes Mark I am trying to stop further boards going in that use the > > > shell scripts) > > > > > > On Mon, 31 Jan 2022 at 15:05, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Mon, Jan 31, 2022 at 02:22:41PM -0700, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Mon, 31 Jan 2022 at 13:40, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > On Mon, Jan 31, 2022 at 12:57:57PM -0700, Simon Glass wrote: > > > > > > > Hi Tom, > > > > > > > > > > > > > > On Mon, 31 Jan 2022 at 11:00, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > On Mon, Jan 31, 2022 at 10:27:41AM -0700, Simon Glass wrote: > > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > > > On Mon, 31 Jan 2022 at 09:15, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > > > > > On Mon, Jan 31, 2022 at 09:13:02AM -0700, Simon Glass wrote: > > > > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > > > > > > > On Mon, 31 Jan 2022 at 07:24, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > On Sun, Jan 30, 2022 at 08:52:25AM -0700, Simon Glass wrote: > > > > > > > > > > > > > > > > > > > > > > > > > More than a year after this migration message appeared, we still have new > > > > > > > > > > > > > boards being added with this option. Add a check against this. > > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > > > > > > > > > > > > > > > > > > > > Please just make this an error in checkpatch.pl instead. > > > > > > > > > > > > > > > > > > > > > > I couldn't think of a way of doing that...do you have an idea? > > > > > > > > > > > > > > > > > > > > Yes, 2f3e8d6a86cb ("checkpatch: report ERROR only on disabling of fdt > > > > > > > > > > and initrd relocation") updates the check I had for fdt_high/initrd_high > > > > > > > > > > being in the file at all to only be for additions. And yes, I check > > > > > > > > > > every PR for new checkpatch ERROR lines and only ignore the ones for > > > > > > > > > > code imported from other projects. > > > > > > > > > > > > > > > > > > Yes, I understand that, but SPL_FIT_GENERATOR defaults to on for > > > > > > > > > certain boards, so there is no need to mention it anywhere in the > > > > > > > > > patch. Also someone could adjust the condition in the Kconfig to add > > > > > > > > > other boards. > > > > > > > > > > > > > > > > Then you want something a bit more like the fdt|initrd_high check now, > > > > > > > > along with updating the help around SPL_FIT_GENERATOR to note that this > > > > > > > > option is deprecated, is the path forward then I think. > > > > > > > > > > > > > > I'm still a bit lost. > > > > > > > > > > > > > > What I want: break the build if someone adds a new board that uses > > > > > > > SPL_FIT_GENERATOR > > > > > > > > > > > > > > What you are offering: checkpatch check for people adding that option > > > > > > > > > > > > > > But the patch doesn't generally include that option. > > > > > > > > > > > > > > I can certainly mention in the Kconfig help that the option is > > > > > > > deprecated, but without checking if it is defined for a NEW board, I > > > > > > > cannot prevent it from growing. > > > > > > > > > > > > > > What am I missing? Can you be more specific? > > > > > > > > > > > > How do you add a new board that enables SPL_FIT_GENERATOR without > > > > > > "SPL_FIT_GENERATOR" being in the resulting patch, other than being > > > > > > ARCH_ZYNQMP/ARCH_ROCKCHIP ? > > > > > > > > > > Well that's the case I am most concerned with, actually. Also, someone > > > > > might add a new condition to SPL_FIT_GENERATOR. > > > > > > > > For the current cases, we just need to get them migrated since it's all > > > > the same logic? So it would I think be a one-and-done thing. For a new > > > > > > Yes I think so and some of them are done. These are what I can find: > > > > > > ./arch/riscv/lib/mkimage_fit_opensbi.sh > > > ./arch/arm/mach-zynqmp/mkimage_fit_atf.sh > > > ./arch/arm/mach-imx/mkimage_fit_atf.sh > > > ./arch/arm/mach-rockchip/make_fit_atf.py > > > > > > but they are not used by that many boards. > > > > > > I feel that the amount of pending migration is somewhat overwhelming > > > and we should take a stronger line in mainline. > > > > > > Perhaps I should send a patch to simply remove the option? Would that > > > be acceptable? > > > > Is there something technically preventing their migration to buildman? > > Looking over examples for imx8* conversions, it's just adding a binman > > node and describing things there, yes? > > Poking at this a bit more, it seems like the outstanding imx platforms > to be converted still have pending patches and it's just part of the > general imx backlog. The riscv one isn't used and should be removed. > That leaves rockchip and zynqmp needing conversion. Michal has already > commented in this thread and I'll leave it to him to say how long he > needs to see how long zynqmp needs to update. That leaves Rockchip. Ah good! One option might be that I could try to convert chromebook_bob, then see if we can just apply it to everything? +Kever Yang again for that but it is New Year over there Regards, Simon
-Philipp Hi Tom, On Tue, 1 Feb 2022 at 07:05, Simon Glass <sjg@chromium.org> wrote: > > Hi Tom, > > On Mon, 31 Jan 2022 at 16:32, Tom Rini <trini@konsulko.com> wrote: > > > > On Mon, Jan 31, 2022 at 06:25:33PM -0500, Tom Rini wrote: > > > On Mon, Jan 31, 2022 at 03:59:08PM -0700, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > (yes Mark I am trying to stop further boards going in that use the > > > > shell scripts) > > > > > > > > On Mon, 31 Jan 2022 at 15:05, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > On Mon, Jan 31, 2022 at 02:22:41PM -0700, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Mon, 31 Jan 2022 at 13:40, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > On Mon, Jan 31, 2022 at 12:57:57PM -0700, Simon Glass wrote: > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > On Mon, 31 Jan 2022 at 11:00, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > > > On Mon, Jan 31, 2022 at 10:27:41AM -0700, Simon Glass wrote: > > > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > > > > > On Mon, 31 Jan 2022 at 09:15, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > > > > > > > On Mon, Jan 31, 2022 at 09:13:02AM -0700, Simon Glass wrote: > > > > > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > > > > > > > > > On Mon, 31 Jan 2022 at 07:24, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > On Sun, Jan 30, 2022 at 08:52:25AM -0700, Simon Glass wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > More than a year after this migration message appeared, we still have new > > > > > > > > > > > > > > boards being added with this option. Add a check against this. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > > > > > > > > > > > > > > > > > > > > > > Please just make this an error in checkpatch.pl instead. > > > > > > > > > > > > > > > > > > > > > > > > I couldn't think of a way of doing that...do you have an idea? > > > > > > > > > > > > > > > > > > > > > > Yes, 2f3e8d6a86cb ("checkpatch: report ERROR only on disabling of fdt > > > > > > > > > > > and initrd relocation") updates the check I had for fdt_high/initrd_high > > > > > > > > > > > being in the file at all to only be for additions. And yes, I check > > > > > > > > > > > every PR for new checkpatch ERROR lines and only ignore the ones for > > > > > > > > > > > code imported from other projects. > > > > > > > > > > > > > > > > > > > > Yes, I understand that, but SPL_FIT_GENERATOR defaults to on for > > > > > > > > > > certain boards, so there is no need to mention it anywhere in the > > > > > > > > > > patch. Also someone could adjust the condition in the Kconfig to add > > > > > > > > > > other boards. > > > > > > > > > > > > > > > > > > Then you want something a bit more like the fdt|initrd_high check now, > > > > > > > > > along with updating the help around SPL_FIT_GENERATOR to note that this > > > > > > > > > option is deprecated, is the path forward then I think. > > > > > > > > > > > > > > > > I'm still a bit lost. > > > > > > > > > > > > > > > > What I want: break the build if someone adds a new board that uses > > > > > > > > SPL_FIT_GENERATOR > > > > > > > > > > > > > > > > What you are offering: checkpatch check for people adding that option > > > > > > > > > > > > > > > > But the patch doesn't generally include that option. > > > > > > > > > > > > > > > > I can certainly mention in the Kconfig help that the option is > > > > > > > > deprecated, but without checking if it is defined for a NEW board, I > > > > > > > > cannot prevent it from growing. > > > > > > > > > > > > > > > > What am I missing? Can you be more specific? > > > > > > > > > > > > > > How do you add a new board that enables SPL_FIT_GENERATOR without > > > > > > > "SPL_FIT_GENERATOR" being in the resulting patch, other than being > > > > > > > ARCH_ZYNQMP/ARCH_ROCKCHIP ? > > > > > > > > > > > > Well that's the case I am most concerned with, actually. Also, someone > > > > > > might add a new condition to SPL_FIT_GENERATOR. > > > > > > > > > > For the current cases, we just need to get them migrated since it's all > > > > > the same logic? So it would I think be a one-and-done thing. For a new > > > > > > > > Yes I think so and some of them are done. These are what I can find: > > > > > > > > ./arch/riscv/lib/mkimage_fit_opensbi.sh > > > > ./arch/arm/mach-zynqmp/mkimage_fit_atf.sh > > > > ./arch/arm/mach-imx/mkimage_fit_atf.sh > > > > ./arch/arm/mach-rockchip/make_fit_atf.py > > > > > > > > but they are not used by that many boards. > > > > > > > > I feel that the amount of pending migration is somewhat overwhelming > > > > and we should take a stronger line in mainline. > > > > > > > > Perhaps I should send a patch to simply remove the option? Would that > > > > be acceptable? > > > > > > Is there something technically preventing their migration to buildman? > > > Looking over examples for imx8* conversions, it's just adding a binman > > > node and describing things there, yes? > > > > Poking at this a bit more, it seems like the outstanding imx platforms > > to be converted still have pending patches and it's just part of the > > general imx backlog. The riscv one isn't used and should be removed. > > That leaves rockchip and zynqmp needing conversion. Michal has already > > commented in this thread and I'll leave it to him to say how long he > > needs to see how long zynqmp needs to update. That leaves Rockchip. > > Ah good! One option might be that I could try to convert > chromebook_bob, then see if we can just apply it to everything? > > +Kever Yang again for that but it is New Year over there It seems that rk3399 uses bl31.elf and splits out the sections into pieces. What a mess! I wonder if that is necessary for ATF to work? It seems to do the same for TEE. Regards, Simon
> From: Simon Glass <sjg@chromium.org> > Date: Tue, 1 Feb 2022 08:42:35 -0700 > > It seems that rk3399 uses bl31.elf and splits out the sections into > pieces. What a mess! I wonder if that is necessary for ATF to work? It > seems to do the same for TEE. That's because bl31.elf really consists of three binary blobs packed together into a single ELF object. This is done such that specific bits needed for suspend/resume land in the AP's SRAM and the PMU's SRAM while the majority lands in DRAM. The splitting happens to make U-Boot's ITS stuff happy. I suppose this could be done in a different way by packing the ELF binary itself into the FIT image, but then SPL has to parse the ELF headers and copy things in place which may be challenging. (I'm not familliar with the TEE stuff)
Hi Mark, On Tue, 1 Feb 2022 at 09:08, Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > > From: Simon Glass <sjg@chromium.org> > > Date: Tue, 1 Feb 2022 08:42:35 -0700 > > > > It seems that rk3399 uses bl31.elf and splits out the sections into > > pieces. What a mess! I wonder if that is necessary for ATF to work? It > > seems to do the same for TEE. > > That's because bl31.elf really consists of three binary blobs packed > together into a single ELF object. This is done such that specific > bits needed for suspend/resume land in the AP's SRAM and the PMU's > SRAM while the majority lands in DRAM. The splitting happens to make > U-Boot's ITS stuff happy. OK I see, thanks. I hadn't realised it was running from SDRAM. I suppose SPL has set that up already. It would help if I could get my firefly-rk3399 unblocked. > > I suppose this could be done in a different way by packing the ELF > binary itself into the FIT image, but then SPL has to parse the ELF > headers and copy things in place which may be challenging. We do have code for that which we could press into service, but it goes against the idea of SPL a bit, I think, when you can do it in advance. > > (I'm not familliar with the TEE stuff) Well if we need to do it for BL31, it is not really any more effort to do it for TEE, I suppose. Regards, Simon
diff --git a/Makefile b/Makefile index 212124522e6..55faff3952f 100644 --- a/Makefile +++ b/Makefile @@ -1110,6 +1110,12 @@ ifeq ($(CONFIG_OF_EMBED)$(CONFIG_EFI_APP),y) @echo >&2 "====================================================" endif ifneq ($(CONFIG_SPL_FIT_GENERATOR),) + # Only allow existing users of this deprecated option. Please migrate! + @if ! grep -q $(shell cat .defconfig_name) \ + $(srctree)/scripts/fit_gen_whitelist.txt; then \ + echo >&2 "Error: CONFIG_SPL_FIT_GENERATOR is deprecated"; \ + exit 1; \ + fi @echo >&2 "===================== WARNING ======================" @echo >&2 "This board uses CONFIG_SPL_FIT_GENERATOR. Please migrate" @echo >&2 "to binman instead, to avoid the proliferation of" diff --git a/scripts/fit_gen_whitelist.txt b/scripts/fit_gen_whitelist.txt new file mode 100644 index 00000000000..ac0890b3f39 --- /dev/null +++ b/scripts/fit_gen_whitelist.txt @@ -0,0 +1,65 @@ +# List of boards that need to be migrated from SPL_FIT_GENERATOR to binman +# See https://patchwork.ozlabs.org/project/uboot/list/?series=242992&state=* +# for an example series (see patches 7 and 13 in particular) + +# Please do not add to this file + +# Some TI boards need migration +am335x_evm_spiboot +am64x_evm_a53 +am64x_evm_r5 +am65x_evm_r5_usbdfu +am65x_evm_r5_usbmsc + +# MX8 needs migration +cgtqmx8 +imx8mm-icore-mx8mm-ctouch2 +imx8mm-icore-mx8mm-edimm2.2 +imx8qm_rom7720_a1_4G + +# Rockchip needs migration +chromebook_bob +evb-px30 +evb-px5 +evb-rk3308 +evb-rk3328 +evb-rk3399 +evb-rk3568 +ficus-rk3399 +firefly-px30 +firefly-rk3399 +khadas-edge-captain-rk3399 +khadas-edge-rk3399 +khadas-edge-v-rk3399 +leez-rk3399 +lion-rk3368 +nanopc-t4-rk3399 +nanopi-m4-2gb-rk3399 +nanopi-m4b-rk3399 +nanopi-m4-rk3399 +nanopi-neo4-rk3399 +nanopi-r2s-rk3328 +nanopi-r4s-rk3399 +odroid-go2 +roc-cc-rk3308 +orangepi-rk3399 +pinebook-pro-rk3399 +puma-rk3399 +px30-core-ctouch2-of10-px30 +px30-core-ctouch2-px30 +px30-core-edimm2.2-px30 +roc-cc-rk3308 +roc-cc-rk3328 +rock64-rk3328 +rock960-rk3399 +rock-pi-4c-rk3399 +rock-pi-4-rk3399 +rock-pi-e-rk3328 +rock-pi-n10-rk3399pro +rockpro64-rk3399 +roc-pc-mezzanine-rk3399 +roc-pc-rk3399 + +# Zynqmp needs mnigration +avnet_ultrazedev_cc_v1_0_ultrazedev_som_v1_0 +xilinx_zynqmp_virt
More than a year after this migration message appeared, we still have new boards being added with this option. Add a check against this. Signed-off-by: Simon Glass <sjg@chromium.org> --- Changes in v2: - Rebase to master Makefile | 6 ++++ scripts/fit_gen_whitelist.txt | 65 +++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 scripts/fit_gen_whitelist.txt