On Wed, Jan 18, 2023 at 8:46 AM Michal Simek <michal.si...@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.si...@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.si...@linaro.org> > AuthorDate: Mon May 23 13:16:01 2022 -0500 > Commit: Jassi Brar <jaswinder.si...@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.g...@linaro.org> > Cc: Masahisa Kojima <masahisa.koj...@linaro.org> > Cc: Manish V Badarkhe <manish.badar...@arm.com> > Cc: Leonardo Sandoval <leonardo.sando...@linaro.org> > Change-Id: I5d96972bc4b3b9a12a8157117e53a05da5ce89f6 > Signed-off-by: Masami Hiramatsu <masami.hirama...@linaro.org> > Signed-off-by: Jassi Brar <jaswinder.si...@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.