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

[Patrice CHOTARD]

Hi Marek

I am preparing the next STM32 U-Boot pull request, during testing, buidlman is complaining with the following warning:

03: ARM: stm32: Factor out save_boot_params           
       arm:  +   stm32mp13
+../arch/arm/mach-stm32mp/boot_params.c: In function 'board_fdt_blob_setup':
+../arch/arm/mach-stm32mp/boot_params.c:20:35: error: implicit declaration of function 'get_stm32mp_bl2_dtb' [-Werror=implicit-function-declaration]
+   20 |         unsigned long nt_fw_dtb = get_stm32mp_bl2_dtb();
+      |                                   ^~~~~~~~~~~~~~~~~~~
+cc1: all warnings being treated as errors


Same issue with stm32mp15_defconfig and stm32mp15_trusted_defconfig.

Regards
Patrice


On 1/4/23 10:00, Patrice CHOTARD wrote:
> Hi Marek
> 
> On 12/7/22 20:24, 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.
>>
>> 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>
>> ---
>> V2: Avoid #if CONFIG... , use if (CONFIG... instead
>> V3: No change
>> ---
>>  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..ee59866bb73 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;
>> +
>> +	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 6156526253c..12b42b9d59c 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);
> 
> Reviewed-by: Patrice Chotard <patrice.chotard at foss.st.com>
> 
> Thanks
> Patrice