[U-Boot] [PATCH 0/4] xilinx: firmware: Synchronization firmware functions

Hi,
Versal and ZynqMP are using the same firmware interface that's why some synchronization should be done. Still there are zynqmp_mmio_read/write which should be converted. Patches are done in steps for easier reviewing.
Thanks, Michal
Michal Simek (4): arm64: versal: Clean pm_api_id usage arm64: xilinx: Move firmware functions from platform to driver arm64: versal: Rename versal_pm_request to xilinx_pm_request arm64: zynqmp: Convert invoke_smc() to xilinx_pm_request()
arch/arm/mach-versal/cpu.c | 25 -------- arch/arm/mach-versal/include/mach/sys_proto.h | 58 ------------------- arch/arm/mach-zynqmp/cpu.c | 37 ++---------- arch/arm/mach-zynqmp/include/mach/sys_proto.h | 2 - board/xilinx/zynqmp/cmds.c | 4 +- drivers/clk/clk_versal.c | 15 ++--- drivers/firmware/firmware-zynqmp.c | 33 ++++++++++- drivers/fpga/versalpl.c | 3 +- drivers/fpga/zynqmppl.c | 16 ++--- include/zynqmp_firmware.h | 54 ++++++++++++++++- 10 files changed, 107 insertions(+), 140 deletions(-)

Copy enum values from platform code to firmware code. IDs are shared between ZynqMP and Versal.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
arch/arm/mach-versal/cpu.c | 1 + arch/arm/mach-versal/include/mach/sys_proto.h | 55 ------------------- drivers/clk/clk_versal.c | 1 + include/zynqmp_firmware.h | 50 ++++++++++++++++- 4 files changed, 51 insertions(+), 56 deletions(-)
diff --git a/arch/arm/mach-versal/cpu.c b/arch/arm/mach-versal/cpu.c index 909949a76ab8..0d10e7f194db 100644 --- a/arch/arm/mach-versal/cpu.c +++ b/arch/arm/mach-versal/cpu.c @@ -10,6 +10,7 @@ #include <asm/sections.h> #include <asm/arch/hardware.h> #include <asm/arch/sys_proto.h> +#include <zynqmp_firmware.h>
DECLARE_GLOBAL_DATA_PTR;
diff --git a/arch/arm/mach-versal/include/mach/sys_proto.h b/arch/arm/mach-versal/include/mach/sys_proto.h index 2f5ad02bf476..c282078f8626 100644 --- a/arch/arm/mach-versal/include/mach/sys_proto.h +++ b/arch/arm/mach-versal/include/mach/sys_proto.h @@ -8,61 +8,6 @@ enum { TCM_SPLIT, };
-enum pm_api_id { - PM_GET_API_VERSION = 1, - PM_SET_CONFIGURATION, - PM_GET_NODE_STATUS, - PM_GET_OPERATING_CHARACTERISTIC, - PM_REGISTER_NOTIFIER, - PM_REQUEST_SUSPEND, - PM_SELF_SUSPEND, - PM_FORCE_POWERDOWN, - PM_ABORT_SUSPEND, - PM_REQUEST_WAKEUP, - PM_SET_WAKEUP_SOURCE, - PM_SYSTEM_SHUTDOWN, - PM_REQUEST_NODE, - PM_RELEASE_NODE, - PM_SET_REQUIREMENT, - PM_SET_MAX_LATENCY, - PM_RESET_ASSERT, - PM_RESET_GET_STATUS, - PM_MMIO_WRITE, - PM_MMIO_READ, - PM_PM_INIT_FINALIZE, - PM_FPGA_LOAD, - PM_FPGA_GET_STATUS, - PM_GET_CHIPID, - PM_SECURE_SHA = 26, - PM_SECURE_RSA, - PM_PINCTRL_REQUEST, - PM_PINCTRL_RELEASE, - PM_PINCTRL_GET_FUNCTION, - PM_PINCTRL_SET_FUNCTION, - PM_PINCTRL_CONFIG_PARAM_GET, - PM_PINCTRL_CONFIG_PARAM_SET, - PM_IOCTL, - PM_QUERY_DATA, - PM_CLOCK_ENABLE, - PM_CLOCK_DISABLE, - PM_CLOCK_GETSTATE, - PM_CLOCK_SETDIVIDER, - PM_CLOCK_GETDIVIDER, - PM_CLOCK_SETRATE, - PM_CLOCK_GETRATE, - PM_CLOCK_SETPARENT, - PM_CLOCK_GETPARENT, - PM_SECURE_IMAGE, - PM_FPGA_READ = 46, - PM_SECURE_AES, - PM_CLOCK_PLL_GETPARAM = 49, - PM_REGISTER_ACCESS = 52, - PM_EFUSE_ACCESS, - PM_FEATURE_CHECK = 63, - PM_API_MAX, -}; - -#define PM_SIP_SVC 0xC2000000 #define PAYLOAD_ARG_CNT 4U
void tcm_init(u8 mode); diff --git a/drivers/clk/clk_versal.c b/drivers/clk/clk_versal.c index df87645774c5..e0fa661be9e0 100644 --- a/drivers/clk/clk_versal.c +++ b/drivers/clk/clk_versal.c @@ -12,6 +12,7 @@ #include <clk.h> #include <dm.h> #include <asm/arch/sys_proto.h> +#include <zynqmp_firmware.h>
#define MAX_PARENT 100 #define MAX_NODES 6 diff --git a/include/zynqmp_firmware.h b/include/zynqmp_firmware.h index a20cbcdb869e..742934814cb0 100644 --- a/include/zynqmp_firmware.h +++ b/include/zynqmp_firmware.h @@ -11,7 +11,55 @@ enum pm_api_id { PM_GET_API_VERSION = 1, PM_SET_CONFIGURATION, - PM_SECURE_IMAGE = 45, + PM_GET_NODE_STATUS, + PM_GET_OPERATING_CHARACTERISTIC, + PM_REGISTER_NOTIFIER, + PM_REQUEST_SUSPEND, + PM_SELF_SUSPEND, + PM_FORCE_POWERDOWN, + PM_ABORT_SUSPEND, + PM_REQUEST_WAKEUP, + PM_SET_WAKEUP_SOURCE, + PM_SYSTEM_SHUTDOWN, + PM_REQUEST_NODE, + PM_RELEASE_NODE, + PM_SET_REQUIREMENT, + PM_SET_MAX_LATENCY, + PM_RESET_ASSERT, + PM_RESET_GET_STATUS, + PM_MMIO_WRITE, + PM_MMIO_READ, + PM_PM_INIT_FINALIZE, + PM_FPGA_LOAD, + PM_FPGA_GET_STATUS, + PM_GET_CHIPID, + PM_SECURE_SHA = 26, + PM_SECURE_RSA, + PM_PINCTRL_REQUEST, + PM_PINCTRL_RELEASE, + PM_PINCTRL_GET_FUNCTION, + PM_PINCTRL_SET_FUNCTION, + PM_PINCTRL_CONFIG_PARAM_GET, + PM_PINCTRL_CONFIG_PARAM_SET, + PM_IOCTL, + PM_QUERY_DATA, + PM_CLOCK_ENABLE, + PM_CLOCK_DISABLE, + PM_CLOCK_GETSTATE, + PM_CLOCK_SETDIVIDER, + PM_CLOCK_GETDIVIDER, + PM_CLOCK_SETRATE, + PM_CLOCK_GETRATE, + PM_CLOCK_SETPARENT, + PM_CLOCK_GETPARENT, + PM_SECURE_IMAGE, + PM_FPGA_READ = 46, + PM_SECURE_AES, + PM_CLOCK_PLL_GETPARAM = 49, + PM_REGISTER_ACCESS = 52, + PM_EFUSE_ACCESS, + PM_FEATURE_CHECK = 63, + PM_API_MAX, };
#define PM_SIP_SVC 0xc2000000

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; + + 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_ */

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

Use generic name instead of Versal specific because this should be also used on ZynqMP.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
drivers/clk/clk_versal.c | 14 +++++++------- drivers/firmware/firmware-zynqmp.c | 2 +- drivers/fpga/versalpl.c | 2 +- include/zynqmp_firmware.h | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/clk/clk_versal.c b/drivers/clk/clk_versal.c index e0fa661be9e0..7e97b0c4bf3a 100644 --- a/drivers/clk/clk_versal.c +++ b/drivers/clk/clk_versal.c @@ -363,7 +363,7 @@ static u32 versal_clock_get_div(u32 clk_id) u32 ret_payload[PAYLOAD_ARG_CNT]; u32 div;
- versal_pm_request(PM_CLOCK_GETDIVIDER, clk_id, 0, 0, 0, ret_payload); + xilinx_pm_request(PM_CLOCK_GETDIVIDER, clk_id, 0, 0, 0, ret_payload); div = ret_payload[1];
return div; @@ -373,7 +373,7 @@ static u32 versal_clock_set_div(u32 clk_id, u32 div) { u32 ret_payload[PAYLOAD_ARG_CNT];
- versal_pm_request(PM_CLOCK_SETDIVIDER, clk_id, div, 0, 0, ret_payload); + xilinx_pm_request(PM_CLOCK_SETDIVIDER, clk_id, div, 0, 0, ret_payload);
return div; } @@ -383,7 +383,7 @@ static u64 versal_clock_ref(u32 clk_id) u32 ret_payload[PAYLOAD_ARG_CNT]; int ref;
- versal_pm_request(PM_CLOCK_GETPARENT, clk_id, 0, 0, 0, ret_payload); + xilinx_pm_request(PM_CLOCK_GETPARENT, clk_id, 0, 0, 0, ret_payload); ref = ret_payload[0]; if (!(ref & 1)) return ref_clk; @@ -402,7 +402,7 @@ static u64 versal_clock_get_pll_rate(u32 clk_id) u32 parent_rate, parent_id; u32 id = clk_id & 0xFFF;
- versal_pm_request(PM_CLOCK_GETSTATE, clk_id, 0, 0, 0, ret_payload); + xilinx_pm_request(PM_CLOCK_GETSTATE, clk_id, 0, 0, 0, ret_payload); res = ret_payload[1]; if (!res) { printf("0%x PLL not enabled\n", clk_id); @@ -412,9 +412,9 @@ static u64 versal_clock_get_pll_rate(u32 clk_id) parent_id = clock[clock[id].parent[0].id].clk_id; parent_rate = versal_clock_ref(parent_id);
- versal_pm_request(PM_CLOCK_GETDIVIDER, clk_id, 0, 0, 0, ret_payload); + xilinx_pm_request(PM_CLOCK_GETDIVIDER, clk_id, 0, 0, 0, ret_payload); fbdiv = ret_payload[1]; - versal_pm_request(PM_CLOCK_PLL_GETPARAM, clk_id, 2, 0, 0, ret_payload); + xilinx_pm_request(PM_CLOCK_PLL_GETPARAM, clk_id, 2, 0, 0, ret_payload); frac = ret_payload[1];
freq = (fbdiv * parent_rate) >> (1 << frac); @@ -441,7 +441,7 @@ static u32 versal_clock_get_parentid(u32 clk_id) u32 id = clk_id & 0xFFF;
if (versal_clock_mux(clk_id)) { - versal_pm_request(PM_CLOCK_GETPARENT, clk_id, 0, 0, 0, + xilinx_pm_request(PM_CLOCK_GETPARENT, clk_id, 0, 0, 0, ret_payload); parent_id = ret_payload[1]; } diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c index 11f5030e85c7..e8fbf1f5644d 100644 --- a/drivers/firmware/firmware-zynqmp.c +++ b/drivers/firmware/firmware-zynqmp.c @@ -176,7 +176,7 @@ int __maybe_unused invoke_smc(u32 pm_api_id, u32 arg0, u32 arg1, u32 arg2, return regs.regs[0]; }
-int __maybe_unused versal_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2, +int __maybe_unused xilinx_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, diff --git a/drivers/fpga/versalpl.c b/drivers/fpga/versalpl.c index 8337b8306d76..4bcc2132432b 100644 --- a/drivers/fpga/versalpl.c +++ b/drivers/fpga/versalpl.c @@ -39,7 +39,7 @@ static int versal_load(xilinx_desc *desc, const void *buf, size_t bsize, buf_lo = lower_32_bits(bin_buf); buf_hi = upper_32_bits(bin_buf);
- ret = versal_pm_request(VERSAL_PM_LOAD_PDI, VERSAL_PM_PDI_TYPE, buf_lo, + ret = xilinx_pm_request(VERSAL_PM_LOAD_PDI, VERSAL_PM_PDI_TYPE, buf_lo, buf_hi, 0, ret_payload); if (ret) puts("PL FPGA LOAD fail\n"); diff --git a/include/zynqmp_firmware.h b/include/zynqmp_firmware.h index 088ba30e0bb8..84090208e50c 100644 --- a/include/zynqmp_firmware.h +++ b/include/zynqmp_firmware.h @@ -83,7 +83,7 @@ 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, +int xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 *ret_payload);
#endif /* _ZYNQMP_FIRMWARE_H_ */

Remove macros which use PM_SIP_SVC offset and convert invoke_smc() to xilinx_pm_request() which do calculation with PM_SIP_SVC already.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
arch/arm/mach-zynqmp/cpu.c | 11 ++++------- board/xilinx/zynqmp/cmds.c | 4 ++-- drivers/firmware/firmware-zynqmp.c | 15 ++++----------- drivers/fpga/zynqmppl.c | 16 +++++++++------- include/zynqmp_firmware.h | 4 ---- 5 files changed, 19 insertions(+), 31 deletions(-)
diff --git a/arch/arm/mach-zynqmp/cpu.c b/arch/arm/mach-zynqmp/cpu.c index ef73a75cf9a3..0d79ba5b1459 100644 --- a/arch/arm/mach-zynqmp/cpu.c +++ b/arch/arm/mach-zynqmp/cpu.c @@ -151,9 +151,6 @@ unsigned int zynqmp_get_silicon_version(void) return ZYNQMP_CSU_VERSION_SILICON; }
-#define ZYNQMP_MMIO_READ 0xC2000014 -#define ZYNQMP_MMIO_WRITE 0xC2000013 - static int zynqmp_mmio_rawwrite(const u32 address, const u32 mask, const u32 value) @@ -186,8 +183,8 @@ int zynqmp_mmio_write(const u32 address, if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) return zynqmp_mmio_rawwrite(address, mask, value); else - return invoke_smc(ZYNQMP_MMIO_WRITE, address, mask, - value, 0, NULL); + return xilinx_pm_request(PM_MMIO_WRITE, address, mask, + value, 0, NULL);
return -EINVAL; } @@ -203,8 +200,8 @@ int zynqmp_mmio_read(const u32 address, u32 *value) if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) { ret = zynqmp_mmio_rawread(address, value); } else { - ret = invoke_smc(ZYNQMP_MMIO_READ, address, 0, 0, - 0, ret_payload); + ret = xilinx_pm_request(PM_MMIO_READ, address, 0, 0, + 0, ret_payload); *value = ret_payload[1]; }
diff --git a/board/xilinx/zynqmp/cmds.c b/board/xilinx/zynqmp/cmds.c index f53a1b63bea6..d3bb57a2e94a 100644 --- a/board/xilinx/zynqmp/cmds.c +++ b/board/xilinx/zynqmp/cmds.c @@ -51,8 +51,8 @@ static int do_zynqmp_verify_secure(cmd_tbl_t *cmdtp, int flag, int argc, (ulong)(key_ptr + KEY_PTR_LEN)); }
- ret = invoke_smc(ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD, src_lo, src_hi, - key_lo, key_hi, ret_payload); + ret = xilinx_pm_request(PM_SECURE_IMAGE, src_lo, src_hi, + key_lo, key_hi, ret_payload); if (ret) { printf("Failed: secure op status:0x%x\n", ret); } else { diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c index e8fbf1f5644d..8c6b71cd7076 100644 --- a/drivers/firmware/firmware-zynqmp.c +++ b/drivers/firmware/firmware-zynqmp.c @@ -54,7 +54,7 @@ static int send_req(const u32 *req, size_t req_len, u32 *res, size_t res_maxlen) if (IS_ENABLED(CONFIG_SPL_BUILD)) return ipi_req(req, req_len, res, res_maxlen);
- return invoke_smc(req[0] + PM_SIP_SVC, 0, 0, 0, 0, res); + return xilinx_pm_request(req[0], 0, 0, 0, 0, res); }
unsigned int zynqmp_firmware_version(void) @@ -147,8 +147,8 @@ 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) +int __maybe_unused xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2, + u32 arg3, u32 *ret_payload) { /* * Added SIP service call Function Identifier @@ -159,7 +159,7 @@ int __maybe_unused invoke_smc(u32 pm_api_id, u32 arg0, u32 arg1, u32 arg2, if (current_el() == 3) return 0;
- regs.regs[0] = pm_api_id; + regs.regs[0] = PM_SIP_SVC | api_id; regs.regs[1] = ((u64)arg1 << 32) | arg0; regs.regs[2] = ((u64)arg3 << 32) | arg2;
@@ -176,13 +176,6 @@ int __maybe_unused invoke_smc(u32 pm_api_id, u32 arg0, u32 arg1, u32 arg2, return regs.regs[0]; }
-int __maybe_unused xilinx_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/zynqmppl.c b/drivers/fpga/zynqmppl.c index c2670271c8ea..d129b5459c00 100644 --- a/drivers/fpga/zynqmppl.c +++ b/drivers/fpga/zynqmppl.c @@ -227,11 +227,12 @@ static int zynqmp_load(xilinx_desc *desc, const void *buf, size_t bsize, buf_hi = upper_32_bits(bin_buf);
if (xilfpga_old) - ret = invoke_smc(ZYNQMP_SIP_SVC_PM_FPGA_LOAD, buf_lo, buf_hi, - (u32)(uintptr_t)bsizeptr, bstype, ret_payload); + ret = xilinx_pm_request(ZYNQMP_SIP_SVC_PM_FPGA_LOAD, buf_lo, + buf_hi, (u32)(uintptr_t)bsizeptr, + bstype, ret_payload); else - ret = invoke_smc(ZYNQMP_SIP_SVC_PM_FPGA_LOAD, buf_lo, buf_hi, - (u32)bsize, 0, ret_payload); + ret = xilinx_pm_request(ZYNQMP_SIP_SVC_PM_FPGA_LOAD, buf_lo, + buf_hi, (u32)bsize, 0, ret_payload);
if (ret) puts("PL FPGA LOAD fail\n"); @@ -272,7 +273,8 @@ static int zynqmp_loads(xilinx_desc *desc, const void *buf, size_t bsize, buf_lo = lower_32_bits((ulong)buf); buf_hi = upper_32_bits((ulong)buf);
- ret = invoke_smc(ZYNQMP_SIP_SVC_PM_FPGA_LOAD, buf_lo, buf_hi, + ret = xilinx_pm_request(ZYNQMP_SIP_SVC_PM_FPGA_LOAD, buf_lo, + buf_hi, (u32)(uintptr_t)fpga_sec_info->userkey_addr, flag, ret_payload); if (ret) @@ -289,8 +291,8 @@ static int zynqmp_pcap_info(xilinx_desc *desc) int ret; u32 ret_payload[PAYLOAD_ARG_CNT];
- ret = invoke_smc(ZYNQMP_SIP_SVC_PM_FPGA_STATUS, 0, 0, 0, - 0, ret_payload); + ret = xilinx_pm_request(ZYNQMP_SIP_SVC_PM_FPGA_STATUS, 0, 0, 0, + 0, ret_payload); if (!ret) printf("PCAP status\t0x%x\n", ret_payload[1]);
diff --git a/include/zynqmp_firmware.h b/include/zynqmp_firmware.h index 84090208e50c..93d771ece26a 100644 --- a/include/zynqmp_firmware.h +++ b/include/zynqmp_firmware.h @@ -63,8 +63,6 @@ enum pm_api_id { };
#define PM_SIP_SVC 0xc2000000 -#define ZYNQMP_SIP_SVC_PM_SECURE_IMG_LOAD \ - (PM_SIP_SVC + PM_SECURE_IMAGE)
#define ZYNQMP_PM_VERSION_MAJOR 1 #define ZYNQMP_PM_VERSION_MINOR 0 @@ -81,8 +79,6 @@ 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 xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 *ret_payload);
participants (2)
-
Luca Ceresoli
-
Michal Simek