hi Etienne,
On Tue, 6 Sept 2022 at 12:57, Etienne Carriere etienne.carriere@linaro.org wrote:
Hi Sughosh,
I have a last comment on this series, related to the below patch and patch "test: dm: Add test cases for FWU Metadata uclass".
On Fri, 26 Aug 2022 at 11:58, Sughosh Ganu sughosh.ganu@linaro.org wrote:
The FWU Multi Bank Update feature allows the platform to boot the firmware images from one of the partitions(banks). The first stage bootloader(fsbl) passes the value of the boot index, i.e. the bank from which the firmware images were booted from to U-Boot. On the STM32MP157C-DK2 board, this value is passed through one of the SoC's backup register. Add a function to read the boot index value from the backup register.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com Acked-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Changes since V8: None
arch/arm/mach-stm32mp/include/mach/stm32.h | 5 +++++ board/st/stm32mp1/stm32mp1.c | 23 ++++++++++++++++++++++ include/fwu.h | 12 +++++++++++ 3 files changed, 40 insertions(+)
diff --git a/arch/arm/mach-stm32mp/include/mach/stm32.h b/arch/arm/mach-stm32mp/include/mach/stm32.h index c70375a723..c85ae6a34e 100644 --- a/arch/arm/mach-stm32mp/include/mach/stm32.h +++ b/arch/arm/mach-stm32mp/include/mach/stm32.h @@ -112,11 +112,16 @@ enum boot_device { #ifdef CONFIG_STM32MP15x #define TAMP_BACKUP_MAGIC_NUMBER TAMP_BACKUP_REGISTER(4) #define TAMP_BACKUP_BRANCH_ADDRESS TAMP_BACKUP_REGISTER(5) +#define TAMP_FWU_BOOT_INFO_REG TAMP_BACKUP_REGISTER(10) #define TAMP_COPRO_RSC_TBL_ADDRESS TAMP_BACKUP_REGISTER(17) #define TAMP_COPRO_STATE TAMP_BACKUP_REGISTER(18) #define TAMP_BOOT_CONTEXT TAMP_BACKUP_REGISTER(20) #define TAMP_BOOTCOUNT TAMP_BACKUP_REGISTER(21)
+#define TAMP_FWU_BOOT_IDX_MASK GENMASK(3, 0)
+#define TAMP_FWU_BOOT_IDX_OFFSET 0
#define TAMP_COPRO_STATE_OFF 0 #define TAMP_COPRO_STATE_INIT 1 #define TAMP_COPRO_STATE_CRUN 2 diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c index bfec0a710d..dd95babb49 100644 --- a/board/st/stm32mp1/stm32mp1.c +++ b/board/st/stm32mp1/stm32mp1.c @@ -966,3 +966,26 @@ static void board_copro_image_process(ulong fw_image, size_t fw_size) }
U_BOOT_FIT_LOADABLE_HANDLER(IH_TYPE_COPRO, board_copro_image_process);
+#if defined(CONFIG_FWU_MULTI_BANK_UPDATE)
+#include <fwu.h>
+/**
- fwu_plat_get_bootidx() - Get the value of the boot index
- @boot_idx: Boot index value
- Get the value of the bank(partition) from which the platform
- has booted. This value is passed to U-Boot from the earlier
- stage bootloader which loads and boots all the relevant
- firmware images
- */
+void fwu_plat_get_bootidx(void *boot_idx) +{
u32 *bootidx = boot_idx;*bootidx = (readl(TAMP_FWU_BOOT_INFO_REG) >>TAMP_FWU_BOOT_IDX_OFFSET) & TAMP_FWU_BOOT_IDX_MASK;+} +#endif /* CONFIG_FWU_MULTI_BANK_UPDATE */ diff --git a/include/fwu.h b/include/fwu.h index 7ebd3a7115..b8c207deaf 100644 --- a/include/fwu.h +++ b/include/fwu.h @@ -240,4 +240,16 @@ int fwu_plat_get_alt_num(struct udevice *dev, efi_guid_t *image_guid,
*/ int fwu_plat_get_update_index(uint *update_idx);
+/**
- fwu_plat_get_bootidx() - Get the value of the boot index
- @boot_idx: Boot index value
- Get the value of the bank(partition) from which the platform
- has booted. This value is passed to U-Boot from the earlier
- stage bootloader which loads and boots all the relevant
- firmware images
- */
+void fwu_plat_get_bootidx(void *boot_idx);
It's maybe not clean to pass a void * to something callee shall write to. Could the reference be passed with an explicit type pointer as 'unsigned int *'?
The reason I chose this to be a void pointer is that different platforms might have different means of passing the boot index value -- on certain platforms this might be passed through a 32 bit register, 8 bit register on some other. Since this is a platform function, the corresponding platform would know how the index value is being passed and would use the correct sized pointer.
-sughosh
Best regards, Etienne
#endif /* _FWU_H_ */
2.34.1