On 10. 10. 19 10:22, Luca Ceresoli wrote:
> 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.si...@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(®s);
>> -
>> - 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(®s);
>> -
>> - 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.
As you see it is a merge of that two functions together.
>
> 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 <l...@lucaceresoli.net>
We can add debug message. It can't hurt.
Thanks,
Michal
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
