| Message ID | 7a294f10532b5b1a78e4700a4acc2f346e7a8c63.1570199266.git.michal.simek@xilinx.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | xilinx: firmware: Synchronization firmware functions | expand |
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@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. 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@lucaceresoli.net>
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.simek@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 <luca@lucaceresoli.net> We can add debug message. It can't hurt. Thanks, Michal
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; + + 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) { + 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]; +} + +int __maybe_unused versal_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2, + u32 arg3, u32 *ret_payload) +{ + return invoke_smc(PM_SIP_SVC | api_id, arg0, arg1, arg2, arg3, + ret_payload); +} + static const struct udevice_id zynqmp_firmware_ids[] = { { .compatible = "xlnx,zynqmp-firmware" }, { .compatible = "xlnx,versal-firmware-wip"}, diff --git a/drivers/fpga/versalpl.c b/drivers/fpga/versalpl.c index 69617a9b1d7f..8337b8306d76 100644 --- a/drivers/fpga/versalpl.c +++ b/drivers/fpga/versalpl.c @@ -8,6 +8,7 @@ #include <asm/arch/sys_proto.h> #include <memalign.h> #include <versalpl.h> +#include <zynqmp_firmware.h> static ulong versal_align_dma_buffer(ulong *buf, u32 len) { diff --git a/include/zynqmp_firmware.h b/include/zynqmp_firmware.h index 742934814cb0..088ba30e0bb8 100644 --- a/include/zynqmp_firmware.h +++ b/include/zynqmp_firmware.h @@ -81,5 +81,9 @@ enum pm_api_id { unsigned int zynqmp_firmware_version(void); void zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size); +int invoke_smc(u32 pm_api_id, u32 arg0, u32 arg1, u32 arg2, u32 arg3, + u32 *ret_payload); +int versal_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2, + u32 arg3, u32 *ret_payload); #endif /* _ZYNQMP_FIRMWARE_H_ */
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@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(-)