Re: [PATCH v11 3/5] firmware: xilinx: Add RPU configuration APIs
From: Punit Agrawal <hidden>
Date: 2020-09-03 10:37:21
Also in:
linux-devicetree, linux-remoteproc, lkml
Hi Ben, Noticed some issues while going through the code. A couple of queries below. Ben Levinsky [off-list ref] writes:
quoted hunk
This patch adds APIs to access to configure RPU and its processor-specific memory. That is query the run-time mode of RPU as either split or lockstep as well as API to set this mode. In addition add APIs to access configuration of the RPUs' tightly coupled memory (TCM). Signed-off-by: Ben Levinsky <redacted> --- v3: - add xilinx-related platform mgmt fn's instead of wrapping around function pointer in xilinx eemi ops struct v4: - add default values for enums v9: - update commit message - for zynqmp_pm_set_tcm_config and zynqmp_pm_get_rpu_mode update docs for expected output, arguments as well removing unused args - remove unused fn zynqmp_pm_get_node_status v11: - update usage of zynqmp_pm_get_rpu_mode to return rpu mode in enum - update zynqmp_pm_set_tcm_config and zynqmp_pm_set_rpu_mode arguments to remove unused args --- drivers/firmware/xilinx/zynqmp.c | 59 ++++++++++++++++++++++++++++ include/linux/firmware/xlnx-zynqmp.h | 18 +++++++++ 2 files changed, 77 insertions(+)diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c index a966ee956573..807e404589f8 100644 --- a/drivers/firmware/xilinx/zynqmp.c +++ b/drivers/firmware/xilinx/zynqmp.c@@ -846,6 +846,65 @@ int zynqmp_pm_release_node(const u32 node) } EXPORT_SYMBOL_GPL(zynqmp_pm_release_node); +/** + * zynqmp_pm_get_rpu_mode() - Get RPU mode + * @node_id: Node ID of the device + * @rpu_mode: return by reference value + * either split or lockstep + * + * Return: return 0 on success or error+reason. + * if success, then rpu_mode will be set + * to current rpu mode. + */ +int zynqmp_pm_get_rpu_mode(u32 node_id, enum rpu_oper_mode *rpu_mode) +{ + u32 ret_payload[PAYLOAD_ARG_CNT]; + int ret; + + ret = zynqmp_pm_invoke_fn(PM_IOCTL, node_id, + IOCTL_GET_RPU_OPER_MODE, 0, 0, ret_payload); + if (ret < 0) + (void)rpu_mode;
There seems to be something missing from this statement. What is expected from "(void)rpu_mode" here.
+ else + *rpu_mode = ret_payload[0]; + + return ret; +} +EXPORT_SYMBOL_GPL(zynqmp_pm_get_rpu_mode); + +/** + * zynqmp_pm_set_rpu_mode() - Set RPU mode + * @node_id: Node ID of the device + * @arg1: Argument 1 to requested IOCTL call. This is expeted to + * to be value from enum rpu_oper_mode + * + * This function is used to set RPU mode to split or lockstep + * + * Return: Returns status, either success or error+reason + */ +int zynqmp_pm_set_rpu_mode(u32 node_id, u32 arg1)
If arg1 is expected to be "enum rpu_oper_mode" please have the function argument reflect that and do any conversion needed inside the function.
+{
+ return zynqmp_pm_invoke_fn(PM_IOCTL, node_id,
+ IOCTL_SET_RPU_OPER_MODE, arg1, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_set_rpu_mode);
+
+/**
+ * zynqmp_pm_set_tcm_config - configure TCM
+ * @arg1: Argument 1 to requested IOCTL call
+ * either PM_RPU_TCM_COMB or PM_RPU_TCM_SPLIT
+ *
+ * This function is used to set RPU mode to split or combined
+ *
+ * Return: status: 0 for success, else failure
+ */
+int zynqmp_pm_set_tcm_config(u32 node_id, u32 arg1)Same comment as above - with the appropriate enum ofcourse. Thanks, Punit
+{
+ return zynqmp_pm_invoke_fn(PM_IOCTL, node_id,
+ IOCTL_TCM_COMB_CONFIG, arg1, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_set_tcm_config);
+
/**
* zynqmp_pm_force_pwrdwn - PM call to request for another PU or subsystem to
* be powered down forcefully[...] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel