Re: [PATCH v11 08/11] firmware: xilinx: Add debugfs for query data API
From: Olof Johansson <hidden>
Date: 2018-09-09 01:15:24
Also in:
linux-arm-kernel, linux-clk, lkml
Hi, I noticed the below when I was reviewing the code for merge into arm-soc. Would you mind following up with an incremental patch? I don't think we need to ask Michal to respin for this: On Fri, Aug 3, 2018 at 10:53 AM, Jolly Shah [off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Rajan Vaja <redacted> Add debugfs file to query platform specific data from firmware using debugfs interface. Signed-off-by: Rajan Vaja <redacted> Signed-off-by: Jolly Shah <redacted> --- drivers/firmware/xilinx/zynqmp-debug.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)diff --git a/drivers/firmware/xilinx/zynqmp-debug.c b/drivers/firmware/xilinx/zynqmp-debug.c index fc11db9..4532bd0 100644 --- a/drivers/firmware/xilinx/zynqmp-debug.c +++ b/drivers/firmware/xilinx/zynqmp-debug.c@@ -33,6 +33,7 @@ static char debugfs_buf[PAGE_SIZE]; static struct pm_api_info pm_api_list[] = { PM_API(PM_GET_API_VERSION), PM_API(PM_IOCTL), + PM_API(PM_QUERY_DATA), }; /**@@ -105,6 +106,32 @@ static int process_api_request(u32 pm_id, u64 *pm_api_arg, u32 *pm_api_ret) sprintf(debugfs_buf, "IOCTL return value: %u\n", pm_api_ret[1]); break; + case PM_QUERY_DATA: + { + struct zynqmp_pm_query_data qdata = {0}; + + qdata.qid = pm_api_arg[0]; + qdata.arg1 = pm_api_arg[1]; + qdata.arg2 = pm_api_arg[2]; + qdata.arg3 = pm_api_arg[3];
This is usually a pattern we try to avoid (having full code blocks in a switch statement, and local variables). Please move the declaration of qdata to the top of the function so you can drop the braces.
+ + ret = eemi_ops->query_data(qdata, pm_api_ret); + if (ret) + break; + + if (qdata.qid == PM_QID_CLOCK_GET_NAME) + sprintf(debugfs_buf, "Clock name = %s\n", + (char *)pm_api_ret); + else if (qdata.qid == PM_QID_CLOCK_GET_FIXEDFACTOR_PARAMS) + sprintf(debugfs_buf, "Multiplier = %d, Divider = %d\n", + pm_api_ret[1], pm_api_ret[2]); + else + sprintf(debugfs_buf, + "data[0] = 0x%08x\ndata[1] = 0x%08x\n data[2] = 0x%08x\ndata[3] = 0x%08x\n", + pm_api_ret[0], pm_api_ret[1], + pm_api_ret[2], pm_api_ret[3]);
If you anticipate more qids here later, a switch could be nicer than a sequence of if/else. -Olof