diff mbox series

[v5,07/23] FWU: STM32MP1: Add support to read boot index from backup register

Message ID 20220609123010.1017463-8-sughosh.ganu@linaro.org
State New
Headers show
Series FWU: Add FWU Multi Bank Update feature support | expand

Commit Message

Sughosh Ganu June 9, 2022, 12:29 p.m. UTC 
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>
---
 arch/arm/mach-stm32mp/include/mach/stm32.h | 4 ++++
 board/st/stm32mp1/stm32mp1.c               | 7 +++++++
 include/fwu.h                              | 2 +-
 3 files changed, 12 insertions(+), 1 deletion(-)

Comments

Ilias Apalodimas June 10, 2022, 12:02 p.m. UTC | #1 
That's looks ok to me, but I'd rather ST people to have a look

On Thu, 9 Jun 2022 at 15:31, 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>
> ---
>  arch/arm/mach-stm32mp/include/mach/stm32.h | 4 ++++
>  board/st/stm32mp1/stm32mp1.c               | 7 +++++++
>  include/fwu.h                              | 2 +-
>  3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-stm32mp/include/mach/stm32.h b/arch/arm/mach-stm32mp/include/mach/stm32.h
> index 47e88fc3dc..40995ee142 100644
> --- a/arch/arm/mach-stm32mp/include/mach/stm32.h
> +++ b/arch/arm/mach-stm32mp/include/mach/stm32.h
> @@ -100,6 +100,7 @@ enum boot_device {
>  #define TAMP_BACKUP_REGISTER(x)                (STM32_TAMP_BASE + 0x100 + 4 * x)
>  #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)
> @@ -118,6 +119,9 @@ enum boot_device {
>  #define TAMP_BOOT_INSTANCE_MASK                GENMASK(3, 0)
>  #define TAMP_BOOT_FORCED_MASK          GENMASK(7, 0)
>  #define TAMP_BOOT_DEBUG_ON             BIT(16)
> +#define TAMP_FWU_BOOT_IDX_MASK         GENMASK(3, 0)
> +
> +#define TAMP_FWU_BOOT_IDX_OFFSET       0
>
>  enum forced_boot_mode {
>         BOOT_NORMAL = 0x00,
> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> index e68bf09955..dff41ed6f6 100644
> --- a/board/st/stm32mp1/stm32mp1.c
> +++ b/board/st/stm32mp1/stm32mp1.c
> @@ -1081,4 +1081,11 @@ int fwu_plat_get_update_index(u32 *update_idx)
>         return ret;
>  }
>
> +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 36e58afa29..41774ff9e2 100644
> --- a/include/fwu.h
> +++ b/include/fwu.h
> @@ -46,7 +46,7 @@ int fwu_revert_boot_index(void);
>  int fwu_accept_image(efi_guid_t *img_type_id, u32 bank);
>  int fwu_clear_accept_image(efi_guid_t *img_type_id, u32 bank);
>
> -
> +void fwu_plat_get_bootidx(void *boot_idx);
>  int fwu_plat_get_alt_num(struct udevice *dev, efi_guid_t *image_guid,
>                          int *alt_num);
>  int fwu_plat_get_update_index(u32 *update_idx);
> --
> 2.25.1
>
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Patrick Delaunay June 21, 2022, 11:27 a.m. UTC | #2 
Hi,

On 6/9/22 14:29, Sughosh Ganu 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>
> ---
>   arch/arm/mach-stm32mp/include/mach/stm32.h | 4 ++++
>   board/st/stm32mp1/stm32mp1.c               | 7 +++++++
>   include/fwu.h                              | 2 +-
>   3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-stm32mp/include/mach/stm32.h b/arch/arm/mach-stm32mp/include/mach/stm32.h
> index 47e88fc3dc..40995ee142 100644
> --- a/arch/arm/mach-stm32mp/include/mach/stm32.h
> +++ b/arch/arm/mach-stm32mp/include/mach/stm32.h
> @@ -100,6 +100,7 @@ enum boot_device {
>   #define TAMP_BACKUP_REGISTER(x)		(STM32_TAMP_BASE + 0x100 + 4 * x)
>   #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)
> @@ -118,6 +119,9 @@ enum boot_device {
>   #define TAMP_BOOT_INSTANCE_MASK		GENMASK(3, 0)
>   #define TAMP_BOOT_FORCED_MASK		GENMASK(7, 0)
>   #define TAMP_BOOT_DEBUG_ON		BIT(16)
> +#define TAMP_FWU_BOOT_IDX_MASK		GENMASK(3, 0)
> +
> +#define TAMP_FWU_BOOT_IDX_OFFSET	0
>   


please don't mix the 2 TAMP_FWU defines with define and enum for TAMP_BOOT

=> move the 2 defines before TAMP_COPRO defines.


#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


>   enum forced_boot_mode {
>   	BOOT_NORMAL = 0x00,
> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> index e68bf09955..dff41ed6f6 100644
> --- a/board/st/stm32mp1/stm32mp1.c
> +++ b/board/st/stm32mp1/stm32mp1.c
> @@ -1081,4 +1081,11 @@ int fwu_plat_get_update_index(u32 *update_idx)
>   	return ret;
>   }
>   
> +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 36e58afa29..41774ff9e2 100644
> --- a/include/fwu.h
> +++ b/include/fwu.h
> @@ -46,7 +46,7 @@ int fwu_revert_boot_index(void);
>   int fwu_accept_image(efi_guid_t *img_type_id, u32 bank);
>   int fwu_clear_accept_image(efi_guid_t *img_type_id, u32 bank);
>   
> -
> +void fwu_plat_get_bootidx(void *boot_idx);
>   int fwu_plat_get_alt_num(struct udevice *dev, efi_guid_t *image_guid,
>   			 int *alt_num);
>   int fwu_plat_get_update_index(u32 *update_idx);

With the modifications:

Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>

Thanks
Patrick
Sughosh Ganu June 23, 2022, 6:30 a.m. UTC | #3 
On Tue, 21 Jun 2022 at 16:57, Patrick DELAUNAY
<patrick.delaunay@foss.st.com> wrote:
>
> Hi,
>
> On 6/9/22 14:29, Sughosh Ganu 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>
> > ---
> >   arch/arm/mach-stm32mp/include/mach/stm32.h | 4 ++++
> >   board/st/stm32mp1/stm32mp1.c               | 7 +++++++
> >   include/fwu.h                              | 2 +-
> >   3 files changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/mach-stm32mp/include/mach/stm32.h b/arch/arm/mach-stm32mp/include/mach/stm32.h
> > index 47e88fc3dc..40995ee142 100644
> > --- a/arch/arm/mach-stm32mp/include/mach/stm32.h
> > +++ b/arch/arm/mach-stm32mp/include/mach/stm32.h
> > @@ -100,6 +100,7 @@ enum boot_device {
> >   #define TAMP_BACKUP_REGISTER(x)             (STM32_TAMP_BASE + 0x100 + 4 * x)
> >   #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)
> > @@ -118,6 +119,9 @@ enum boot_device {
> >   #define TAMP_BOOT_INSTANCE_MASK             GENMASK(3, 0)
> >   #define TAMP_BOOT_FORCED_MASK               GENMASK(7, 0)
> >   #define TAMP_BOOT_DEBUG_ON          BIT(16)
> > +#define TAMP_FWU_BOOT_IDX_MASK               GENMASK(3, 0)
> > +
> > +#define TAMP_FWU_BOOT_IDX_OFFSET     0
> >
>
>
> please don't mix the 2 TAMP_FWU defines with define and enum for TAMP_BOOT
>
> => move the 2 defines before TAMP_COPRO defines.
>
>
> #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

Will change as per your suggestion. Thanks.

-sughosh

>
>
> #define TAMP_COPRO_STATE_OFF        0
> #define TAMP_COPRO_STATE_INIT        1
>
>
> >   enum forced_boot_mode {
> >       BOOT_NORMAL = 0x00,
> > diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> > index e68bf09955..dff41ed6f6 100644
> > --- a/board/st/stm32mp1/stm32mp1.c
> > +++ b/board/st/stm32mp1/stm32mp1.c
> > @@ -1081,4 +1081,11 @@ int fwu_plat_get_update_index(u32 *update_idx)
> >       return ret;
> >   }
> >
> > +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 36e58afa29..41774ff9e2 100644
> > --- a/include/fwu.h
> > +++ b/include/fwu.h
> > @@ -46,7 +46,7 @@ int fwu_revert_boot_index(void);
> >   int fwu_accept_image(efi_guid_t *img_type_id, u32 bank);
> >   int fwu_clear_accept_image(efi_guid_t *img_type_id, u32 bank);
> >
> > -
> > +void fwu_plat_get_bootidx(void *boot_idx);
> >   int fwu_plat_get_alt_num(struct udevice *dev, efi_guid_t *image_guid,
> >                        int *alt_num);
> >   int fwu_plat_get_update_index(u32 *update_idx);
>
> With the modifications:
>
> Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
>
> Thanks
> Patrick
>
>
diff mbox series

Patch

diff --git a/arch/arm/mach-stm32mp/include/mach/stm32.h b/arch/arm/mach-stm32mp/include/mach/stm32.h
index 47e88fc3dc..40995ee142 100644
--- a/arch/arm/mach-stm32mp/include/mach/stm32.h
+++ b/arch/arm/mach-stm32mp/include/mach/stm32.h
@@ -100,6 +100,7 @@  enum boot_device {
 #define TAMP_BACKUP_REGISTER(x)		(STM32_TAMP_BASE + 0x100 + 4 * x)
 #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)
@@ -118,6 +119,9 @@  enum boot_device {
 #define TAMP_BOOT_INSTANCE_MASK		GENMASK(3, 0)
 #define TAMP_BOOT_FORCED_MASK		GENMASK(7, 0)
 #define TAMP_BOOT_DEBUG_ON		BIT(16)
+#define TAMP_FWU_BOOT_IDX_MASK		GENMASK(3, 0)
+
+#define TAMP_FWU_BOOT_IDX_OFFSET	0
 
 enum forced_boot_mode {
 	BOOT_NORMAL = 0x00,
diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
index e68bf09955..dff41ed6f6 100644
--- a/board/st/stm32mp1/stm32mp1.c
+++ b/board/st/stm32mp1/stm32mp1.c
@@ -1081,4 +1081,11 @@  int fwu_plat_get_update_index(u32 *update_idx)
 	return ret;
 }
 
+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 36e58afa29..41774ff9e2 100644
--- a/include/fwu.h
+++ b/include/fwu.h
@@ -46,7 +46,7 @@  int fwu_revert_boot_index(void);
 int fwu_accept_image(efi_guid_t *img_type_id, u32 bank);
 int fwu_clear_accept_image(efi_guid_t *img_type_id, u32 bank);
 
-
+void fwu_plat_get_bootidx(void *boot_idx);
 int fwu_plat_get_alt_num(struct udevice *dev, efi_guid_t *image_guid,
 			 int *alt_num);
 int fwu_plat_get_update_index(u32 *update_idx);