[PATCH v4 20/27] Makefile: Warn against using CONFIG_SPL_FIT_GENERATOR
Simon Glass
sjg at chromium.org
Tue Aug 25 17:04:25 CEST 2020
Hi Michal,
On Mon, 24 Aug 2020 at 08:12, Michal Simek <michal.simek at xilinx.com> wrote:
>
> Hi Simon,
>
> On 22. 08. 20 17:08, Simon Glass wrote:
> > Hi Michal,
> >
> > On Mon, 17 Aug 2020 at 00:49, Michal Simek <michal.simek at xilinx.com> wrote:
> >>
> >> Hi Simon,
> >>
> >> On 16. 08. 20 5:39, Simon Glass wrote:
> >>> Hi Michal,
> >>>
> >>> On Fri, 14 Aug 2020 at 07:28, Michal Simek <monstr at monstr.eu> wrote:
> >>>>
> >>>> Hi Simon,
> >>>>
> >>>> ne 19. 7. 2020 v 22:06 odesílatel Simon Glass <sjg at chromium.org> napsal:
> >>>>>
> >>>>> This option is used to run arch-specific shell scripts which produce .its
> >>>>> files which are used to produce FIT images. We already have binman which
> >>>>> is designed to produce firmware images. It is more powerful and has tests.
> >>>>>
> >>>>> So this option should be deprecated and not used. Existing uses should be
> >>>>> migrated.
> >>>>>
> >>>>> Mentions of this in code reviews over the last year or so do not seem to
> >>>>> have resulted in action, and things are getting worse.
> >>>>>
> >>>>> So let's add a warning.
> >>>>>
> >>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
> >>>>> Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
> >>>>> ---
> >>>>>
> >>>>> (no changes since v1)
> >>>>>
> >>>>> Makefile | 9 +++++++++
> >>>>> 1 file changed, 9 insertions(+)
> >>>>>
> >>>>> diff --git a/Makefile b/Makefile
> >>>>> index f1b5be1882..d73c10a973 100644
> >>>>> --- a/Makefile
> >>>>> +++ b/Makefile
> >>>>> @@ -1148,6 +1148,13 @@ ifneq ($(CONFIG_DM_ETH),y)
> >>>>> @echo >&2 "See doc/driver-model/migration.rst for more info."
> >>>>> @echo >&2 "===================================================="
> >>>>> endif
> >>>>> +endif
> >>>>> +ifneq ($(CONFIG_SPL_FIT_GENERATOR),)
> >>>>> + @echo >&2 "===================== WARNING ======================"
> >>>>> + @echo >&2 "This board uses CONFIG_SPL_FIT_GENERATOR. Please migrate"
> >>>>> + @echo >&2 "to binman instead, to avoid the proliferation of"
> >>>>> + @echo >&2 "arch-specific scripts with no tests."
> >>>>> + @echo >&2 "===================================================="
> >>>>> endif
> >>>>> @# Check that this build does not use CONFIG options that we do not
> >>>>> @# know about unless they are in Kconfig. All the existing CONFIG
> >>>>> @@ -1345,6 +1352,8 @@ endif
> >>>>>
> >>>>> # Boards with more complex image requirements can provide an .its source file
> >>>>> # or a generator script
> >>>>> +# NOTE: Please do not use this. We are migrating away from Makefile rules to use
> >>>>> +# binman instead.
> >>>>> ifneq ($(CONFIG_SPL_FIT_SOURCE),"")
> >>>>> U_BOOT_ITS := u-boot.its
> >>>>> $(U_BOOT_ITS): $(subst ",,$(CONFIG_SPL_FIT_SOURCE))
> >>>>> --
> >>>>> 2.28.0.rc0.105.gf9edc3c819-goog
> >>>>>
> >>>>
> >>>> I just got to this conversion and I am curious how that transition
> >>>> should look like.
> >>>> I found how FIT image is created which is fine but I didn't find any
> >>>> reference on how to generate images based on CONFIG_OF_LIST.
> >>>> If you look at arch/arm/mach-zynqmp/mkimage_fit_atf.sh you will see
> >>>> that I loop over this entry and create multiple DT nodes and the same
> >>>> amount of configurations to cover it. Is this supported by binman?
> >>>> If yes, what's the syntax for it?
> >>>
> >>> The easiest way is probably to create a new entry type, like zynq-fit.
> >>> Then you can generate the DT using the sequence writer functions. See
> >>> _ReadSubNodes() in fit.py for an example.
> >>>
> >>> You can perhaps have a template subnode and use that in a for loop to
> >>> generate the nodes.
> >>>
> >>>>
> >>>> I tried several configurations and we can use that for generating qspi
> >>>> images and also images with different configurations to have them
> >>>> ready
> >>>> but first I need to be able to handle the case above.
> >>>
> >>> I was thinking of converting sunxi which has the same need, but it
> >>> sounds like you are on the case. Let me know if you need help.
> >>
> >> Nope. I just saw that message and started to play with it to find out
> >> what needs to be done and how this fits to bigger picture. If this
> >> doesn't work directly then the work needs to be planned which will take
> >> time especially when this utility is new for us and we could have issues
> >> with writing code in python. Would be good if you can do the first shot
> >> because you know this utility and I am more than happy to test it, try
> >> and adopt if needed for our case.
> >>
> >> Sunxi is very similar case as is zynqmp. Difference is they hardcode
> >> default configuration to config_1. ZynqMP is setting up default based on
> >> default DT configured at that time.
> >>
> >> In connection to binman I see that there would be a need to generate
> >> images with ATF and without ATF in configuration node and with different
> >> default configuration. There could be also a need to add additional
> >> loadable entry such as bitstreams.
> >>
> >> Back to zynq-fit new entry type. I don't think it should be zynq/zynqmp
> >> type because as was state in commit message u-boot.itb generation is
> >> very similar for all these boards that's why name for this new entry
> >> should be generic.
> >>
> >
> > I sent an initial series to add this to binman. I've since found a few
> > problems so will send a v2 at some point. You can try it out at
> > u-boot-dm/binman-working
>
> I looked at this branch and add my changes on the top.
>
> The first thing what I see is that I miss fit,fdt-list = "of-list"; in
> sunxi dt file. I had to add it to work for me.
Ah yes, I decided to add this at the last minute so it is not relying
on a convention.
>
> With BINMAN_FDT enabled I am getting error that there is no valid
> "binman node" in DT. I didn't study that code yet but that's the point
> of keeping this DT node out there?
Is this in SPL? Perhaps something is filtering out the node.
>
> This is my binman configuration.
>
> diff --git a/arch/arm/dts/zynqmp-u-boot.dtsi
> b/arch/arm/dts/zynqmp-u-boot.dtsi
> new file mode 100644
> index 000000000000..b3364d3e2df8
> --- /dev/null
> +++ b/arch/arm/dts/zynqmp-u-boot.dtsi
> @@ -0,0 +1,72 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 Xilinx, Inc.
> + */
> +
> +#include <config.h>
> +
> +/ {
> + binman: binman {
> + multiple-images;
> + };
> +};
> +
> +&binman {
> + u-boot-itb {
> + filename = "u-boot.itb";
> + fit {
> + fit,external-offset = <CONFIG_FIT_EXTERNAL_OFFSET>;
> + description = "FIT image with ATF support";
> + fit,fdt-list = "of-list";
> + #address-cells = <1>;
> +
> + images {
> + uboot {
> + description = "U-Boot (64-bit)";
> + type = "firmware";
> + os = "u-boot";
> + arch = "arm64";
> + compression = "none";
> + load = <CONFIG_SYS_TEXT_BASE>;
> + entry = <CONFIG_SYS_TEXT_BASE>;
> +
> + u-boot-nodtb {
> + };
> + };
> + atf {
> + description = "ARM Trusted Firmware";
> + type = "firmware";
> + os = "arm-trusted-firmware";
> + arch = "arm64";
> + compression = "none";
> + load = <0xfffea000>; /* FIXME */
> + entry = <0xfffea000>;
> +
> + blob-ext {
> + filename = "bl31.bin";
> + };
> + };
> + @fdt-SEQ {
> + description = "NAME";
> + type = "flat_dt";
> + arch = "arm64";
> + compression = "none";
> + };
> + };
> +
> + configurations {
> + default = "config-1";
> + @config-SEQ {
> + description = "NAME";
> + firmware = "atf";
> + loadables = "uboot";
> + fdt = "fdt-SEQ";
> + };
> + };
> + };
> + fdtmap{};
> + };
> +
> +};
>
> Anyway compare to current script default option is hardcoded to
> config-1.
> Current arch/arm/mach-zynqmp/mkimage_fit_atf.sh is also
> setting up default option based on selected default DT (I can fix this
> by implementing board_fit_config_name_match() but IIRC it is looping
> over all configurations and slowing down boot).
Is this using an environment variable to select the default? Would it
be OK to put this in the DT for each individual board?
>
> I will play with it a little bit more to get more experience with it
OK good luck!
Regards,
Simon
More information about the U-Boot
mailing list