[PATCHv3 4/5] fwu: DeveloperBox: add support for FWU
Jassi Brar
jassisinghbrar at gmail.com
Sat Jan 21 18:48:25 CET 2023
On Wed, Jan 18, 2023 at 8:46 AM Michal Simek <michal.simek at amd.com> wrote:
>
>
>
> On 1/9/23 02:07, Jassi Brar wrote:
> > Add code to support FWU_MULTI_BANK_UPDATE.
> > The platform does not have gpt-partition storage for
> > Banks and MetaData, rather it used SPI-NOR backed
> > mtd regions for the purpose.
> >
> > Signed-off-by: Jassi Brar <jaswinder.singh at linaro.org>
> > ---
> > board/socionext/developerbox/Makefile | 1 +
> > board/socionext/developerbox/developerbox.c | 8 ++
> > board/socionext/developerbox/fwu_plat.c | 57 ++++++++++++
> > configs/synquacer_developerbox_defconfig | 13 ++-
> > doc/board/socionext/developerbox.rst | 96 +++++++++++++++++++++
> > include/configs/synquacer.h | 10 +++
> > 6 files changed, 183 insertions(+), 2 deletions(-)
> > create mode 100644 board/socionext/developerbox/fwu_plat.c
> >
> > diff --git a/board/socionext/developerbox/Makefile b/board/socionext/developerbox/Makefile
> > index 4a46de995a..9b80ee38e7 100644
> > --- a/board/socionext/developerbox/Makefile
> > +++ b/board/socionext/developerbox/Makefile
> > @@ -7,3 +7,4 @@
> > #
> >
> > obj-y := developerbox.o
> > +obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu_plat.o
> > diff --git a/board/socionext/developerbox/developerbox.c b/board/socionext/developerbox/developerbox.c
> > index 6415c90c1c..2123914f21 100644
> > --- a/board/socionext/developerbox/developerbox.c
> > +++ b/board/socionext/developerbox/developerbox.c
> > @@ -20,6 +20,13 @@
> >
> > #if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> > struct efi_fw_image fw_images[] = {
> > +#if defined(CONFIG_FWU_MULTI_BANK_UPDATE)
> > + {
> > + .image_type_id = DEVELOPERBOX_FIP_IMAGE_GUID,
> > + .fw_name = u"DEVELOPERBOX-FIP",
> > + .image_index = 1,
> > + },
> > +#else
> > {
> > .image_type_id = DEVELOPERBOX_UBOOT_IMAGE_GUID,
> > .fw_name = u"DEVELOPERBOX-UBOOT",
> > @@ -35,6 +42,7 @@ struct efi_fw_image fw_images[] = {
> > .fw_name = u"DEVELOPERBOX-OPTEE",
> > .image_index = 3,
> > },
> > +#endif
> > };
> >
> > struct efi_capsule_update_info update_info = {
> > diff --git a/board/socionext/developerbox/fwu_plat.c b/board/socionext/developerbox/fwu_plat.c
> > new file mode 100644
> > index 0000000000..29b258f86c
> > --- /dev/null
> > +++ b/board/socionext/developerbox/fwu_plat.c
> > @@ -0,0 +1,57 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2023, Linaro Limited
> > + */
> > +
> > +#include <efi_loader.h>
> > +#include <fwu.h>
> > +#include <fwu_mdata.h>
> > +#include <memalign.h>
> > +#include <mtd.h>
> > +
> > +#define DFU_ALT_BUF_LEN 256
> > +
> > +/* Generate dfu_alt_info from partitions */
> > +void set_dfu_alt_info(char *interface, char *devstr)
> > +{
> > + int ret;
> > + struct mtd_info *mtd;
> > +
> > + ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
> > + memset(buf, 0, sizeof(buf));
> > +
> > + mtd_probe_devices();
>
>
> When you disable CONFIG_MTD for this board this code will fail.
> You need to cover this dependency via different Kconfig symbol or somewhere else.
> Maybe this file should be compiled only when CONFIG_FWU_MDATA_MTD is present.
>
Yes, i agree.
> > +
> > + mtd = get_mtd_device_nm("nor1");
> > + if (IS_ERR_OR_NULL(mtd))
> > + return;
> > +
> > + ret = fwu_gen_alt_info_from_mtd(buf, DFU_ALT_BUF_LEN, mtd);
> > + if (ret < 0) {
> > + log_err("Error: Failed to generate dfu_alt_info. (%d)\n", ret);
> > + return;
> > + }
> > + log_debug("Make dfu_alt_info: '%s'\n", buf);
> > +
> > + env_set("dfu_alt_info", buf);
> > +}
> > +
> > +int fwu_plat_get_alt_num(struct udevice __always_unused *dev,
> > + efi_guid_t *image_id, u8 *alt_num)
> > +{
> > + return fwu_mtd_get_alt_num(image_id, alt_num, "nor1");
> > +}
> > +
> > +void fwu_plat_get_bootidx(uint *boot_idx)
> > +{
> > + int ret;
> > + u32 active_idx;
> > + u32 *bootidx = boot_idx;
> > +
> > + ret = fwu_get_active_index(&active_idx);
> > +
> > + if (ret < 0)
> > + *bootidx = -1;
> > +
> > + *bootidx = active_idx;
> > +}
> > diff --git a/configs/synquacer_developerbox_defconfig b/configs/synquacer_developerbox_defconfig
> > index f69b873a36..b1085a388e 100644
> > --- a/configs/synquacer_developerbox_defconfig
> > +++ b/configs/synquacer_developerbox_defconfig
> > @@ -1,10 +1,11 @@
> > CONFIG_ARM=y
> > CONFIG_ARCH_SYNQUACER=y
> > -CONFIG_TEXT_BASE=0x08200000
> > +CONFIG_POSITION_INDEPENDENT=y
> > +CONFIG_SYS_TEXT_BASE=0
>
> How is this related to subject? I can't see any single line of description why
> you are doing this change.
>
> > CONFIG_SYS_MALLOC_LEN=0x1000000
> > CONFIG_SYS_MALLOC_F_LEN=0x400
> > CONFIG_ENV_SIZE=0x30000
> > -CONFIG_ENV_OFFSET=0x300000
> > +CONFIG_ENV_OFFSET=0x580000
>
> Why are you changing this offset?
>
Synquacer's boot flow is revamped with FWU support. So I think these
should probably come in a separate patch
> > diff --git a/doc/board/socionext/developerbox.rst b/doc/board/socionext/developerbox.rst
> > index 2d943c23be..be872aa79d 100644
> > --- a/doc/board/socionext/developerbox.rst
> > +++ b/doc/board/socionext/developerbox.rst
> > @@ -85,3 +85,99 @@ Once the flasher tool is running we are ready flash the UEFI image::
> >
> > After transferring the SPI_NOR_UBOOT.fd, turn off the DSW2-7 and reset the board.
> >
> > +
> > +Enable FWU Multi Bank Update
> > +============================
> > +
> > +DeveloperBox supports the FWU Multi Bank Update. You *MUST* update both *SCP firmware* and *TF-A* for this feature. This will change the layout and the boot process but you can switch back to the normal one by changing the DSW 1-4 off.
>
> Isn't here also certain limit for number of chars on the line?
>
ok.
> > +
> > +By default, the CONFIG_FWU_NUM_BANKS and COFNIG_FWU_NUM_IMAGES_PER_BANKS are set to 2 and 1 respectively. This uses FIP (Firmware Image Package) type image which contains TF-A, U-Boot and OP-TEE (the OP-TEE is optional.)
>
> long line and type CONFIG
>
ok
>
> I would also prefer to describe the logic used on TF-A side. I saw that you are
> using one register to find out how to boot it.
>
That is not a register, but a location in NOR that contains flag to
> commit a19382521c583b3dde89df14678b011960097f6c
> Author: Jassi Brar <jaswinder.singh at linaro.org>
> AuthorDate: Mon May 23 13:16:01 2022 -0500
> Commit: Jassi Brar <jaswinder.singh at linaro.org>
> CommitDate: Mon Jun 27 13:12:24 2022 -0500
>
> feat(synquacer): add FWU Multi Bank Update support
>
> Add FWU Multi Bank Update support. This reads the platform metadata
> and update the FIP base address so that BL2 can load correct BL3X
> based on the boot index.
>
> Cc: Sumit Garg <sumit.garg at linaro.org>
> Cc: Masahisa Kojima <masahisa.kojima at linaro.org>
> Cc: Manish V Badarkhe <manish.badarkhe at arm.com>
> Cc: Leonardo Sandoval <leonardo.sandoval at linaro.org>
> Change-Id: I5d96972bc4b3b9a12a8157117e53a05da5ce89f6
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu at linaro.org>
> Signed-off-by: Jassi Brar <jaswinder.singh at linaro.org>
>
>
> but it is not obviously the code which is decoding mdata structure.
> It means boot flow description would be good to have.
>
The 'platform metadata' is not the fwu-mdata. It is a choice of fip
(with different bootloader) to boot from. FWU A/B support is not yet
implemented.
thanks.
More information about the U-Boot
mailing list