[U-Boot] [PATCH 2/4] arm64: xilinx: Move firmware functions from platform to driver

[Luca Ceresoli]

Hi Michal,

On 04/10/19 16:27, Michal Simek wrote:
> versal_pm_request() and invoke_smc() are almost the same. Only one
> difference is that versal_pm_request is adding PM_SIP_SVC offset to api_id.
> The patch is moving platform implementation to firmware driver code for
> synchronization.
> 
> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
> ---
> 
>  arch/arm/mach-versal/cpu.c                    | 26 -------------
>  arch/arm/mach-versal/include/mach/sys_proto.h |  3 --
>  arch/arm/mach-zynqmp/cpu.c                    | 26 -------------
>  arch/arm/mach-zynqmp/include/mach/sys_proto.h |  2 -
>  drivers/firmware/firmware-zynqmp.c            | 38 ++++++++++++++++++-
>  drivers/fpga/versalpl.c                       |  1 +
>  include/zynqmp_firmware.h                     |  4 ++
>  7 files changed, 42 insertions(+), 58 deletions(-)
> 
> diff --git a/arch/arm/mach-versal/cpu.c b/arch/arm/mach-versal/cpu.c
> index 0d10e7f194db..46ab5348d732 100644
> --- a/arch/arm/mach-versal/cpu.c
> +++ b/arch/arm/mach-versal/cpu.c
> @@ -10,7 +10,6 @@
>  #include <asm/sections.h>
>  #include <asm/arch/hardware.h>
>  #include <asm/arch/sys_proto.h>
> -#include <zynqmp_firmware.h>
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> @@ -109,28 +108,3 @@ int reserve_mmu(void)
>  	return 0;
>  }
>  #endif
> -
> -int versal_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2,
> -		      u32 arg3, u32 *ret_payload)
> -{
> -	struct pt_regs regs;
> -
> -	if (current_el() == 3)
> -		return 0;
> -
> -	regs.regs[0] = PM_SIP_SVC | api_id;
> -	regs.regs[1] = ((u64)arg1 << 32) | arg0;
> -	regs.regs[2] = ((u64)arg3 << 32) | arg2;
> -
> -	smc_call(&regs);
> -
> -	if (ret_payload) {
> -		ret_payload[0] = (u32)regs.regs[0];
> -		ret_payload[1] = upper_32_bits(regs.regs[0]);
> -		ret_payload[2] = (u32)regs.regs[1];
> -		ret_payload[3] = upper_32_bits(regs.regs[1]);
> -		ret_payload[4] = (u32)regs.regs[2];
> -	}
> -
> -	return regs.regs[0];
> -}
> diff --git a/arch/arm/mach-versal/include/mach/sys_proto.h b/arch/arm/mach-versal/include/mach/sys_proto.h
> index c282078f8626..31af049a21c9 100644
> --- a/arch/arm/mach-versal/include/mach/sys_proto.h
> +++ b/arch/arm/mach-versal/include/mach/sys_proto.h
> @@ -12,6 +12,3 @@ enum {
>  
>  void tcm_init(u8 mode);
>  void mem_map_fill(void);
> -
> -int versal_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2,
> -		      u32 arg3, u32 *ret_payload);
> diff --git a/arch/arm/mach-zynqmp/cpu.c b/arch/arm/mach-zynqmp/cpu.c
> index bb21cbcadf69..ef73a75cf9a3 100644
> --- a/arch/arm/mach-zynqmp/cpu.c
> +++ b/arch/arm/mach-zynqmp/cpu.c
> @@ -154,32 +154,6 @@ unsigned int zynqmp_get_silicon_version(void)
>  #define ZYNQMP_MMIO_READ	0xC2000014
>  #define ZYNQMP_MMIO_WRITE	0xC2000013
>  
> -int __maybe_unused invoke_smc(u32 pm_api_id, u32 arg0, u32 arg1, u32 arg2,
> -			      u32 arg3, u32 *ret_payload)
> -{
> -	/*
> -	 * Added SIP service call Function Identifier
> -	 * Make sure to stay in x0 register
> -	 */
> -	struct pt_regs regs;
> -
> -	regs.regs[0] = pm_api_id;
> -	regs.regs[1] = ((u64)arg1 << 32) | arg0;
> -	regs.regs[2] = ((u64)arg3 << 32) | arg2;
> -
> -	smc_call(&regs);
> -
> -	if (ret_payload != NULL) {
> -		ret_payload[0] = (u32)regs.regs[0];
> -		ret_payload[1] = upper_32_bits(regs.regs[0]);
> -		ret_payload[2] = (u32)regs.regs[1];
> -		ret_payload[3] = upper_32_bits(regs.regs[1]);
> -		ret_payload[4] = (u32)regs.regs[2];
> -	}
> -
> -	return regs.regs[0];
> -}
> -
>  static int zynqmp_mmio_rawwrite(const u32 address,
>  		      const u32 mask,
>  		      const u32 value)
> diff --git a/arch/arm/mach-zynqmp/include/mach/sys_proto.h b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
> index 69e729fb7625..10b70761de4a 100644
> --- a/arch/arm/mach-zynqmp/include/mach/sys_proto.h
> +++ b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
> @@ -50,8 +50,6 @@ void handoff_setup(void);
>  
>  int zynqmp_mmio_write(const u32 address, const u32 mask, const u32 value);
>  int zynqmp_mmio_read(const u32 address, u32 *value);
> -int invoke_smc(u32 pm_api_id, u32 arg0, u32 arg1, u32 arg2, u32 arg3,
> -	       u32 *ret_payload);
>  
>  void initialize_tcm(bool mode);
>  void mem_map_fill(void);
> diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
> index 42a627e1dd05..11f5030e85c7 100644
> --- a/drivers/firmware/firmware-zynqmp.c
> +++ b/drivers/firmware/firmware-zynqmp.c
> @@ -7,10 +7,10 @@
>  
>  #include <common.h>
>  #include <dm.h>
> +#include <zynqmp_firmware.h>
>  
>  #if defined(CONFIG_ZYNQMP_IPI)
>  #include <mailbox.h>
> -#include <zynqmp_firmware.h>
>  #include <asm/arch/sys_proto.h>
>  
>  #define PMUFW_PAYLOAD_ARG_CNT	8
> @@ -147,6 +147,42 @@ U_BOOT_DRIVER(zynqmp_power) = {
>  };
>  #endif
>  
> +int __maybe_unused invoke_smc(u32 pm_api_id, u32 arg0, u32 arg1, u32 arg2,
> +			      u32 arg3, u32 *ret_payload)
> +{
> +	/*
> +	 * Added SIP service call Function Identifier
> +	 * Make sure to stay in x0 register
> +	 */
> +	struct pt_regs regs;
> +
> +	if (current_el() == 3)
> +		return 0;

Not stated in the log message, but this check does not exist in current
invoke_smc(). It's useful to check as it avoids a big explosion in case
invoke_smc is called in the wrong context.

But, if possible, it would be good to emit an error here, or the failure
would be silent. Does this look correct?
But that would be another patch, so:

Reviewed-by: Luca Ceresoli <luca at lucaceresoli.net>

-- 
Luca