[PATCH 2/4] ARM: stm32: Factor out save_boot_params

[Patrick DELAUNAY]

Hi Marek,

On 12/6/22 03:33, Marek Vasut wrote:
> The STM32MP15xx platform currently comes with two incompatible
> implementations of save_boot_params() weak function override.
> Factor the save_boot_params() implementation into common cpu.c
> code and provide accessors to read out both ROM API table address
> and DT address from any place in the code instead.


good idea.


>
> Signed-off-by: Marek Vasut <marex at denx.de>
> ---
> Cc: Alexandru Gagniuc <mr.nuke.me at gmail.com>
> Cc: Patrice Chotard <patrice.chotard at foss.st.com>
> Cc: Patrick Delaunay <patrick.delaunay at foss.st.com>
> ---
>   arch/arm/mach-stm32mp/boot_params.c           | 20 ++---------
>   arch/arm/mach-stm32mp/cpu.c                   | 35 +++++++++++++++++++
>   arch/arm/mach-stm32mp/ecdsa_romapi.c          | 20 ++---------
>   .../arm/mach-stm32mp/include/mach/sys_proto.h |  3 ++
>   4 files changed, 42 insertions(+), 36 deletions(-)
>
> diff --git a/arch/arm/mach-stm32mp/boot_params.c b/arch/arm/mach-stm32mp/boot_params.c
> index e91ef1b2fc7..e40cca938ef 100644
> --- a/arch/arm/mach-stm32mp/boot_params.c
> +++ b/arch/arm/mach-stm32mp/boot_params.c
> @@ -11,30 +11,14 @@
>   #include <asm/sections.h>
>   #include <asm/system.h>
>   
> -/*
> - * Force data-section, as .bss will not be valid
> - * when save_boot_params is invoked.
> - */
> -static unsigned long nt_fw_dtb __section(".data");
> -
> -/*
> - * Save the FDT address provided by TF-A in r2 at boot time
> - * This function is called from start.S
> - */
> -void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2,
> -		      unsigned long r3)
> -{
> -	nt_fw_dtb = r2;
> -
> -	save_boot_params_ret();
> -}
> -
>   /*
>    * Use the saved FDT address provided by TF-A at boot time (NT_FW_CONFIG =
>    * Non Trusted Firmware configuration file) when the pointer is valid
>    */
>   void *board_fdt_blob_setup(int *err)
>   {
> +	unsigned long nt_fw_dtb = get_stm32mp_bl2_dtb();
> +
>   	log_debug("%s: nt_fw_dtb=%lx\n", __func__, nt_fw_dtb);
>   
>   	*err = 0;
> diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c
> index 855fc755fe0..fa02a11d867 100644
> --- a/arch/arm/mach-stm32mp/cpu.c
> +++ b/arch/arm/mach-stm32mp/cpu.c
> @@ -378,3 +378,38 @@ int arch_misc_init(void)
>   
>   	return 0;
>   }
> +
> +/*
> + * Without forcing the ".data" section, this would get saved in ".bss". BSS
> + * will be cleared soon after, so it's not suitable.
> + */
> +static uintptr_t rom_api_table __section(".data");
> +static uintptr_t nt_fw_dtb __section(".data");
> +
> +/*
> + * The ROM gives us the API location in r0 when starting. This is only available
> + * during SPL, as there isn't (yet) a mechanism to pass this on to u-boot. Save
> + * the FDT address provided by TF-A in r2 at boot time. This function is called
> + * from start.S
> + */
> +void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2,
> +		      unsigned long r3)
> +{
> +#if IS_ENABLED(CONFIG_STM32_ECDSA_VERIFY)
> +	rom_api_table = r0;
> +#endif
> +#if IS_ENABLED(CONFIG_TFABOOT)
> +	nt_fw_dtb = r2;
> +#endif

jut avoid #if when it is possible.


if (IS_ENABLED(CONFIG_STM32_ECDSA_VERIFY))

	rom_api_table = r0;

if (IS_ENABLED(CONFIG_TFABOOT))

	nt_fw_dtb = r2;


> +	save_boot_params_ret();
> +}
> +
> +uintptr_t get_stm32mp_rom_api_table(void)
> +{
> +	return rom_api_table;
> +}
> +
> +uintptr_t get_stm32mp_bl2_dtb(void)
> +{
> +	return nt_fw_dtb;
> +}
> diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c b/arch/arm/mach-stm32mp/ecdsa_romapi.c
> index 72b87bf2c64..32a7357ad56 100644
> --- a/arch/arm/mach-stm32mp/ecdsa_romapi.c
> +++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c
> @@ -24,26 +24,10 @@ struct ecdsa_rom_api {
>   					   uint32_t ecc_algo);
>   };
>   
> -/*
> - * Without forcing the ".data" section, this would get saved in ".bss". BSS
> - * will be cleared soon after, so it's not suitable.
> - */
> -static uintptr_t rom_api_loc __section(".data");
> -
> -/*
> - * The ROM gives us the API location in r0 when starting. This is only available
> - * during SPL, as there isn't (yet) a mechanism to pass this on to u-boot.
> - */
> -void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2,
> -		      unsigned long r3)
> -{
> -	rom_api_loc = r0;
> -	save_boot_params_ret();
> -}
> -
>   static void stm32mp_rom_get_ecdsa_functions(struct ecdsa_rom_api *rom)
>   {
> -	uintptr_t verify_ptr = rom_api_loc + ROM_API_OFFSET_ECDSA_VERIFY;
> +	uintptr_t verify_ptr = get_stm32mp_rom_api_table() +
> +			       ROM_API_OFFSET_ECDSA_VERIFY;
>   
>   	rom->ecdsa_verify_signature = *(void **)verify_ptr;
>   }
> diff --git a/arch/arm/mach-stm32mp/include/mach/sys_proto.h b/arch/arm/mach-stm32mp/include/mach/sys_proto.h
> index f19a70e53e0..0d39b67178e 100644
> --- a/arch/arm/mach-stm32mp/include/mach/sys_proto.h
> +++ b/arch/arm/mach-stm32mp/include/mach/sys_proto.h
> @@ -77,3 +77,6 @@ void stm32mp_misc_init(void);
>   
>   /* helper function: read data from OTP */
>   u32 get_otp(int index, int shift, int mask);
> +
> +uintptr_t get_stm32mp_rom_api_table(void);
> +uintptr_t get_stm32mp_bl2_dtb(void);


regards

Patrick