On Mon, Jan 09, 2023 at 12:07:05PM +0100, Michal Simek wrote: > Hi, > > On 1/8/23 20:36, Simon Glass wrote: > > Hi Tom, > > > > On Sun, 8 Jan 2023 at 09:24, Tom Rini <tr...@konsulko.com> wrote: > > > > > > On Sun, Jan 08, 2023 at 09:20:09AM -0700, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Sun, 8 Jan 2023 at 09:06, Tom Rini <tr...@konsulko.com> wrote: > > > > > > > > > > On Sun, Jan 08, 2023 at 08:48:37AM -0700, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Sun, 8 Jan 2023 at 06:41, Tom Rini <tr...@konsulko.com> wrote: > > > > > > > > > > > > > > On Sat, Jan 07, 2023 at 02:07:21PM -0700, Simon Glass wrote: > > > > > > > > > > > > > > > This option is deprecated and only used by two boards. Enable > > > > > > > > it for just > > > > > > > > those two boards, so others don't accidentally enable it. > > > > > > > > > > > > > > > > Signed-off-by: Simon Glass <s...@chromium.org> > > > > > > > [snip] > > > > > > > > diff --git a/boot/Kconfig b/boot/Kconfig > > > > > > > > index 55f06761ef8..7ab0dd14211 100644 > > > > > > > > --- a/boot/Kconfig > > > > > > > > +++ b/boot/Kconfig > > > > > > > > @@ -282,12 +282,13 @@ config SPL_FIT_SOURCE > > > > > > > > config USE_SPL_FIT_GENERATOR > > > > > > > > bool "Use a script to generate the .its script" > > > > > > > > depends on SPL_FIT > > > > > > > > - default y if SPL_FIT && ARCH_ZYNQMP > > > > > > > > + help > > > > > > > > + This is deprecated. Please do not use it. Use binman > > > > > > > > instead. > > > > > > > > > > > > > > Lets remove the text around bool so it can't be enabled, and move > > > > > > > to > > > > > > > select'ing it from the two boards that need it. Michal, Luca, > > > > > > > what's > > > > > > > needed to move your two platforms > > > > > > > (avnet_ultrazedev_cc_v1_0_ultrazedev_som_v1_0 and > > > > > > > xilinx_zynqmp_virt off > > > > > > > of this very legacy option, given that other xilinx platforms have > > > > > > > already migrated to binman ? > > > > > > > > > > > > That's a good idea, but these two boards do not have TARGET Kconfig > > > > > > options so it is not possible without adding some Kconfig specific > > > > > > to > > > > > > those boards, then defining it in the defconfig files. > > > > > > > > > > > > We already have the legacy warning. > > > > > > > > > > Yes, but I swear these are new legacy users as when we started trying > > > > > to > > > > > kill off this option it was just a few i.MX platforms lagging behind. > > > > > > > > > > Maybe make ARCH_ZYNQMP select DEPRECATED, USE_SPL_FIT_GENERATOR depend > > > > > on DEPRECATED and add "DERECATED" to the end of the text line? Or > > > > > maybe > > > > > Michal or Luca will speak up soon and migrate these over quickly so we > > > > > can just nuke this. > > > > > > > > Isn't that just more tortuous? I can disable SPL_LOAD_FIT on these two > > > > boards since they don't appear in CI. Then they can convert them when > > > > ready. > > > > > > > > That way we can drop the option now, if that is your goal. > > > > > > I thought xilinx_zynqmp_virt was in CI, but I see I'm mistaken. I still > > > don't want to break platforms outright, and since it's Sunday right now > > > afterall, we should let Michal and Luca a chance to catch up and chime > > > in. I hope it's either going to be a quick conversion or expose > > > something missing and needed in binman, as to why these still haven't > > > been converted. > > > > > > > OK let's hold off on this patch for now. It is just a clean-up anyway. > > First of all. ZynqMP is not wired in CI simply because we are missing some > bits and pieces in upstream qemu to run it. If we can wire it with Xilinx > version we can do it. I have asked to fix it our qemu team but they have > never done it.
Ah yes, I remember you saying that now. I guess it comes down to how
hard it would be to patch that support in to 6.1.0 (or, move us up to a
newer release and patch on top of that) in tools/docker/Dockerfile. We
special-case the nokia_rx51 support in QEMU because it's so old, so I'd
rather avoid that for another platform if we can.
> In U-Boot SPL flow this script is used all the time. I use it all the time
> when I build work on ZynqMP. Also buildroot is using it.
>
> In connection to binman. I have looked at it 2/3 times in past. The biggest
> issue which I have with it is that DT node presence in DT which goes to the
> system.
>
> Here is example
>
> [u-boot](binman)$ make kontron_sl28_defconfig >/dev/null
> [u-boot](binman)$ make -j8 >/dev/null
> [u-boot](binman)$ dtc -I dtb -O dts dts/dt.dtb 2>/dev/null | grep binman
> binman {
> binman = "/binman";
> u_boot_rom = "/binman/u-boot-rom";
>
> where binmap is the part of target DT.
>
> It has side effects.
> 1. DT is bigger
> 2. It contains information how firmware was packed which is just additional
> information which don't need to be shared.
> 3. binman node as is placed in not documented in dt binding to be able to
> pass dtb check.
>
> I can call binman externally but that would mean additional build step which
> everybody wants to avoid.
> Can we update Makefile and add Kconfig option to pass configuration dtb file
> instead of u-boot.dtb by default?
>
> Something like:
>
> diff --git a/Makefile b/Makefile
> index 75a599b2c437..4d8d67c7d938 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1322,7 +1322,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if
> $(BINMAN_DEBUG),-D) \
> $(foreach f,$(BINMAN_TOOLPATHS),--toolpath $(f)) \
> --toolpath $(objtree)/tools \
> $(if $(BINMAN_VERBOSE),-v$(BINMAN_VERBOSE)) \
> - build -u -d u-boot.dtb -O . -m \
> + build -u -d $(CONFIG_BINMAN_CONFIG_DTB) -O . -m \
> $(if $(BINMAN_ALLOW_MISSING),--allow-missing
> --ignore-missing) \
> -I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \
> -I arch/$(ARCH)/dts -a of-list=$(CONFIG_OF_LIST) \
>
> Then by default u-boot.dtb can be used or platforms can use different file.
> Then I would add zynqmp-binman.dts with configurations which can replace
> existing arch/arm/mach-zynqmp/mkimage_fit_atf.sh script.
> What do you think?
>
> BTW: Is there a way to generate also capsules from binman?
This is the kind of feedback I think we've been needing here, and I'll
leave it to Simon to address, thanks!
--
Tom
signature.asc
Description: PGP signature
