From patchwork Thu Aug 25 16:40:10 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sudeep Holla X-Patchwork-Id: 9299573 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id A01FD60459 for ; Thu, 25 Aug 2016 16:40:51 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 83F1029376 for ; Thu, 25 Aug 2016 16:40:51 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 77AA0293A5; Thu, 25 Aug 2016 16:40:51 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id BC8A629376 for ; Thu, 25 Aug 2016 16:40:50 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1bcxhs-0005Gg-Ky; Thu, 25 Aug 2016 16:40:44 +0000 Received: from foss.arm.com ([217.140.101.70]) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1bcxhj-0004XV-5v; Thu, 25 Aug 2016 16:40:37 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D5E20444; Thu, 25 Aug 2016 09:41:54 -0700 (PDT) Received: from [10.1.210.28] (unknown [10.1.210.28]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B92FB3F252; Thu, 25 Aug 2016 09:40:12 -0700 (PDT) From: Sudeep Holla Subject: Re: [PATCH v2 0/7] scpi: Add support for legacy SCPI protocol To: Neil Armstrong , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org References: <1471952816-30877-1-git-send-email-narmstrong@baylibre.com> <73cce388-219d-2daa-811a-059c3d84c2e4@arm.com> Organization: ARM Message-ID: Date: Thu, 25 Aug 2016 17:40:10 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <73cce388-219d-2daa-811a-059c3d84c2e4@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160825_094035_358355_DC231CDF X-CRM114-Status: GOOD ( 15.81 ) X-BeenThere: linux-amlogic@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: heiko@sntech.de, frank.wang@rock-chips.com, khilman@baylibre.com, Sudeep Holla , linux-amlogic@lists.infradead.org, wxt@rock-chips.com Sender: "linux-amlogic" Errors-To: linux-amlogic-bounces+patchwork-linux-amlogic=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On 25/08/16 14:45, Sudeep Holla wrote: > > > On 25/08/16 14:18, Neil Armstrong wrote: [...] >> Here I used if(is_legacy) to stop duplicating functions, is this ok >> for you ? >> > > I am still thinking if it can be abstracted well, some kind of mapping > but haven't thought too much about that yet. Also I was thinking about > bitmap for high priority commands. I remember doing something before but > seem to have lost that copy. I will try to dig it out.. > OK how about something like: 1. in struct scpi_drvinfo DECLARE_BITMAP(cmd_priority, SCPI_CMD_COUNT); 2. scpi_send_message scpi_chan = test_bit(cmd, scpi_info->cmd_priority) ? scpi_info->channels + 1 : scpi_info->channels; 3. probe for (idx = 0; idx < ARRAY_SIZE(hpriority_cmds); idx++) set_bit(hpriority_cmds[idx], scpi_info->cmd_priority); For commands, I am thinking some kind of indirection like the below patch. See if that helps, I will add the description later, but you can build your patches on top of it if you think that works and keeps code simple. Regards, Sudeep -->8 From afde0d2f1d3381d443445301ab5ec111276934e5 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Thu, 25 Aug 2016 17:21:49 +0100 Subject: [PATCH] firmware: arm_scpi: create command indirection to support legacy commands Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scpi.c | 68 +++++++++++++++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 17 deletions(-) u32 protocol_version; u32 firmware_version; int num_chans; + int *cmds; atomic_t next_chan; struct scpi_ops *scpi_ops; struct scpi_chan *channels; @@ -397,7 +430,7 @@ scpi_clk_get_range(u16 clk_id, unsigned long *min, unsigned long *max) struct clk_get_info clk; __le16 le_clk_id = cpu_to_le16(clk_id); - ret = scpi_send_message(SCPI_CMD_GET_CLOCK_INFO, &le_clk_id, + ret = scpi_send_message(scpi_info->cmds[GET_CLOCK_INFO], &le_clk_id, sizeof(le_clk_id), &clk, sizeof(clk)); if (!ret) { *min = le32_to_cpu(clk.min_rate); @@ -412,7 +445,7 @@ static unsigned long scpi_clk_get_val(u16 clk_id) struct clk_get_value clk; __le16 le_clk_id = cpu_to_le16(clk_id); - ret = scpi_send_message(SCPI_CMD_GET_CLOCK_VALUE, &le_clk_id, + ret = scpi_send_message(scpi_info->cmds[GET_CLOCK_VALUE], &le_clk_id, sizeof(le_clk_id), &clk, sizeof(clk)); return ret ? ret : le32_to_cpu(clk.rate); } @@ -425,8 +458,8 @@ static int scpi_clk_set_val(u16 clk_id, unsigned long rate) .rate = cpu_to_le32(rate) }; - return scpi_send_message(SCPI_CMD_SET_CLOCK_VALUE, &clk, sizeof(clk), - &stat, sizeof(stat)); + return scpi_send_message(scpi_info->cmds[SET_CLOCK_VALUE], &clk, + sizeof(clk), &stat, sizeof(stat)); } static int scpi_dvfs_get_idx(u8 domain) @@ -434,8 +467,8 @@ static int scpi_dvfs_get_idx(u8 domain) int ret; u8 dvfs_idx; - ret = scpi_send_message(SCPI_CMD_GET_DVFS, &domain, sizeof(domain), - &dvfs_idx, sizeof(dvfs_idx)); + ret = scpi_send_message(scpi_info->cmds[GET_DVFS], &domain, + sizeof(domain), &dvfs_idx, sizeof(dvfs_idx)); return ret ? ret : dvfs_idx; } @@ -444,7 +477,7 @@ static int scpi_dvfs_set_idx(u8 domain, u8 index) int stat; struct dvfs_set dvfs = {domain, index}; - return scpi_send_message(SCPI_CMD_SET_DVFS, &dvfs, sizeof(dvfs), + return scpi_send_message(scpi_info->cmds[SET_DVFS], &dvfs, sizeof(dvfs), &stat, sizeof(stat)); } @@ -468,8 +501,8 @@ static struct scpi_dvfs_info *scpi_dvfs_get_info(u8 domain) if (scpi_info->dvfs[domain]) /* data already populated */ return scpi_info->dvfs[domain]; - ret = scpi_send_message(SCPI_CMD_GET_DVFS_INFO, &domain, sizeof(domain), - &buf, sizeof(buf)); + ret = scpi_send_message(scpi_info->cmds[GET_DVFS_INFO], &domain, + sizeof(domain), &buf, sizeof(buf)); if (ret) return ERR_PTR(ret); @@ -503,8 +536,8 @@ static int scpi_sensor_get_capability(u16 *sensors) struct sensor_capabilities cap_buf; int ret; - ret = scpi_send_message(SCPI_CMD_SENSOR_CAPABILITIES, NULL, 0, &cap_buf, - sizeof(cap_buf)); + ret = scpi_send_message(scpi_info->cmds[SENSOR_CAPABILITIES], NULL, 0, + &cap_buf, sizeof(cap_buf)); if (!ret) *sensors = le16_to_cpu(cap_buf.sensors); @@ -517,7 +550,7 @@ static int scpi_sensor_get_info(u16 sensor_id, struct scpi_sensor_info *info) struct _scpi_sensor_info _info; int ret; - ret = scpi_send_message(SCPI_CMD_SENSOR_INFO, &id, sizeof(id), + ret = scpi_send_message(scpi_info->cmds[SENSOR_INFO], &id, sizeof(id), &_info, sizeof(_info)); if (!ret) { memcpy(info, &_info, sizeof(*info)); @@ -533,7 +566,7 @@ static int scpi_sensor_get_value(u16 sensor, u64 *val) struct sensor_value buf; int ret; - ret = scpi_send_message(SCPI_CMD_SENSOR_VALUE, &id, sizeof(id), + ret = scpi_send_message(scpi_info->cmds[SENSOR_VALUE], &id, sizeof(id), &buf, sizeof(buf)); if (!ret) *val = (u64)le32_to_cpu(buf.hi_val) << 32 | @@ -548,7 +581,7 @@ static int scpi_device_get_power_state(u16 dev_id) u8 pstate; __le16 id = cpu_to_le16(dev_id); - ret = scpi_send_message(SCPI_CMD_GET_DEVICE_PWR_STATE, &id, + ret = scpi_send_message(scpi_info->cmds[GET_DEV_PWR_STATE], &id, sizeof(id), &pstate, sizeof(pstate)); return ret ? ret : pstate; } @@ -561,7 +594,7 @@ static int scpi_device_set_power_state(u16 dev_id, u8 pstate) .pstate = pstate, }; - return scpi_send_message(SCPI_CMD_SET_DEVICE_PWR_STATE, &dev_set, + return scpi_send_message(scpi_info->cmds[SET_DEV_PWR_STATE], &dev_set, sizeof(dev_set), &stat, sizeof(stat)); } @@ -591,7 +624,7 @@ static int scpi_init_versions(struct scpi_drvinfo *info) int ret; struct scp_capabilities caps; - ret = scpi_send_message(SCPI_CMD_SCPI_CAPABILITIES, NULL, 0, + ret = scpi_send_message(info->cmds[SCPI_CAPABILITIES], NULL, 0, &caps, sizeof(caps)); if (!ret) { info->protocol_version = le32_to_cpu(caps.protocol_version); @@ -754,6 +787,7 @@ static int scpi_probe(struct platform_device *pdev) scpi_info->channels = scpi_chan; scpi_info->num_chans = count; + scpi_info->cmds = scpi_std_commands; platform_set_drvdata(pdev, scpi_info); ret = scpi_init_versions(scpi_info); diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c index 438893762076..c2063cb76b08 100644 --- a/drivers/firmware/arm_scpi.c +++ b/drivers/firmware/arm_scpi.c @@ -129,7 +129,39 @@ enum scpi_std_cmd { SCPI_CMD_SENSOR_ASYNC_VALUE = 0x1a, SCPI_CMD_SET_DEVICE_PWR_STATE = 0x1b, SCPI_CMD_GET_DEVICE_PWR_STATE = 0x1c, - SCPI_CMD_COUNT +}; + +enum scpi_drv_cmd { + SCPI_CAPABILITIES = 0, + GET_DVFS_INFO = 1, + SET_DVFS = 2, + GET_DVFS = 3, + GET_CLOCK_INFO = 4, + SET_CLOCK_VALUE = 5, + GET_CLOCK_VALUE = 6, + PSU_CAPABILITIES = 7, + SENSOR_CAPABILITIES = 8, + SENSOR_INFO = 9, + SENSOR_VALUE = 10, + SET_DEV_PWR_STATE = 11, + GET_DEV_PWR_STATE = 12, + CMD_MAX_COUNT +}; + +static int scpi_std_commands[CMD_MAX_COUNT] = { + SCPI_CMD_SCPI_CAPABILITIES, + SCPI_CMD_GET_DVFS_INFO, + SCPI_CMD_SET_DVFS, + SCPI_CMD_GET_DVFS, + SCPI_CMD_GET_CLOCK_INFO, + SCPI_CMD_SET_CLOCK_VALUE, + SCPI_CMD_GET_CLOCK_VALUE, + SCPI_CMD_PSU_CAPABILITIES, + SCPI_CMD_SENSOR_CAPABILITIES, + SCPI_CMD_SENSOR_INFO, + SCPI_CMD_SENSOR_VALUE, + SCPI_CMD_SET_DEVICE_PWR_STATE, + SCPI_CMD_GET_DEVICE_PWR_STATE, }; struct scpi_xfer { @@ -161,6 +193,7 @@ struct scpi_drvinfo {