From patchwork Thu May 26 16:29:04 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sudeep Holla X-Patchwork-Id: 68710 Delivered-To: patch@linaro.org Received: by 10.140.92.199 with SMTP id b65csp495933qge; Thu, 26 May 2016 09:29:13 -0700 (PDT) X-Received: by 10.66.119.177 with SMTP id kv17mr15363985pab.57.1464280153260; Thu, 26 May 2016 09:29:13 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id n85si7573413pfj.132.2016.05.26.09.29.12; Thu, 26 May 2016 09:29:13 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754238AbcEZQ3K (ORCPT + 30 others); Thu, 26 May 2016 12:29:10 -0400 Received: from foss.arm.com ([217.140.101.70]:40520 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750939AbcEZQ3I (ORCPT ); Thu, 26 May 2016 12:29:08 -0400 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 390CB5F; Thu, 26 May 2016 09:29:32 -0700 (PDT) Received: from [10.1.207.160] (unknown [10.1.207.160]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id CC8863F529; Thu, 26 May 2016 09:29:06 -0700 (PDT) From: Sudeep Holla Subject: Re: [RFC PATCH 0/2] scpi: Add SCPI framework to handle vendors variants To: Neil Armstrong References: <1464255491-18503-1-git-send-email-narmstrong@baylibre.com> Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Sudeep Holla Organization: ARM Message-ID: <57472450.4000709@arm.com> Date: Thu, 26 May 2016 17:29:04 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <1464255491-18503-1-git-send-email-narmstrong@baylibre.com> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Neil, On 26/05/16 10:38, Neil Armstrong wrote: > Since the current SCPI implementation, based on [0]: > - is (at leat) JUNO specific Agreed. > - does not specify a strong "standard" Not exactly, it's extensible. Refer section 3.2.2. Get SCP capability Extended Set Enabled bit. > - does not specify a strong MHU interface specification > Not really required, any mailbox must do. > SoC vendors could implement a variant with slight changes in message > indexes, I assume you mean command index here > new messages types, Also fine with extended command set. > different messages data format or you mean the header or payload ? If they don't follow the header, then how can be group them as same protocol ? > different MHU communication scheme. Not a problem as I mentioned above. > > To keep the spirit of the SCPI interface, add a thin "register" layer to get > the ops from the parent node and switch the drivers using the ops to use > the new of_scpi_ops_get() call. > All I can see is that you share the code to register such drivers which is hardly anything. It's hard to say all drivers use same protocol or interface after this change. I may be missing to see something here so it would be easy to appreciate/review this change with one user of this. My idea of extending this driver if vendor implements extensions based on SCPI specification is something like below: -- Regards, Sudeep -->8 diff --git i/drivers/firmware/arm_scpi.c w/drivers/firmware/arm_scpi.c index 7e3e595c9f30..7e46aa103690 100644 --- i/drivers/firmware/arm_scpi.c +++ w/drivers/firmware/arm_scpi.c @@ -46,6 +46,8 @@ #define CMD_ID_SHIFT 0 #define CMD_ID_MASK 0x7f +#define CMD_SET_SHIFT 7 +#define CMD_SET_MASK 0x1 #define CMD_TOKEN_ID_SHIFT 8 #define CMD_TOKEN_ID_MASK 0xff #define CMD_DATA_SIZE_SHIFT 16 @@ -53,6 +55,10 @@ #define PACK_SCPI_CMD(cmd_id, tx_sz) \ ((((cmd_id) & CMD_ID_MASK) << CMD_ID_SHIFT) | \ (((tx_sz) & CMD_DATA_SIZE_MASK) << CMD_DATA_SIZE_SHIFT)) +#define PACK_EXT_SCPI_CMD(cmd_id, tx_sz) \ + ((((cmd_id) & CMD_ID_MASK) << CMD_ID_SHIFT) | \ + (CMD_SET_MASK << CMD_SET_SHIFT) | \ + (((tx_sz) & CMD_DATA_SIZE_MASK) << CMD_DATA_SIZE_SHIFT)) #define ADD_SCPI_TOKEN(cmd, token) \ ((cmd) |= (((token) & CMD_TOKEN_ID_MASK) << CMD_TOKEN_ID_SHIFT)) @@ -132,6 +138,9 @@ enum scpi_std_cmd { SCPI_CMD_COUNT }; +enum scpi_vendor_ext_cmd { +}; + struct scpi_xfer { u32 slot; /* has to be first element */ u32 cmd; @@ -165,6 +174,7 @@ struct scpi_drvinfo { struct scpi_ops *scpi_ops; struct scpi_chan *channels; struct scpi_dvfs_info *dvfs[MAX_DVFS_DOMAINS]; + void *scpi_ext_ops; }; /* @@ -343,8 +353,8 @@ static void put_scpi_xfer(struct scpi_xfer *t, struct scpi_chan *ch) mutex_unlock(&ch->xfers_lock); } -static int scpi_send_message(u8 cmd, void *tx_buf, unsigned int tx_len, - void *rx_buf, unsigned int rx_len) +static int __scpi_send_message(u8 cmd, void *tx_buf, unsigned int tx_len, + void *rx_buf, unsigned int rx_len, bool extn) { int ret; u8 chan; @@ -359,7 +369,8 @@ static int scpi_send_message(u8 cmd, void *tx_buf, unsigned int tx_len, return -ENOMEM; msg->slot = BIT(SCPI_SLOT); - msg->cmd = PACK_SCPI_CMD(cmd, tx_len); + msg->cmd = extn ? PACK_EXT_SCPI_CMD(cmd, tx_len) : + PACK_SCPI_CMD(cmd, tx_len); msg->tx_buf = tx_buf; msg->tx_len = tx_len; msg->rx_buf = rx_buf; @@ -384,6 +395,18 @@ static int scpi_send_message(u8 cmd, void *tx_buf, unsigned int tx_len, return ret > 0 ? scpi_to_linux_errno(ret) : ret; } +static int scpi_send_message(u8 cmd, void *tx_buf, unsigned int tx_len, + void *rx_buf, unsigned int rx_len) +{ + return __scpi_send_message(cmd, tx_buf, tx_len, rx_buf, rx_len, false); +} + +static int scpi_send_ext_message(u8 cmd, void *tx_buf, unsigned int tx_len, + void *rx_buf, unsigned int rx_len) +{ + return __scpi_send_message(cmd, tx_buf, tx_len, rx_buf, rx_len, true); +} + static u32 scpi_get_version(void) { return scpi_info->protocol_version; @@ -574,6 +597,29 @@ static int scpi_init_versions(struct scpi_drvinfo *info) return ret; } +static struct scpi_vendor_ext_ops scpi_vendor_ext_ops = { +}; + +void *get_scpi_ext_ops(void) +{ + return scpi_info ? scpi_info->scpi_ext_ops : NULL; +} +EXPORT_SYMBOL_GPL(get_scpi_ext_ops); + +static const struct of_device_id scpi_extentions_of_match[] = { + {.compatible = "vendor,scpi", .data = &scpi_vendor_ext_ops}, + {}, +}; + +static void +scpi_init_extensions(struct scpi_drvinfo *info, struct device_node *np) +{ + const struct of_device_id *of_id; + + if (np && (of_id = of_match_node(scpi_extentions_of_match, np))) + info->scpi_ext_ops = (void *)of_id->data; +} + static ssize_t protocol_version_show(struct device *dev, struct device_attribute *attr, char *buf) { @@ -745,6 +791,8 @@ static int scpi_probe(struct platform_device *pdev) FW_REV_PATCH(scpi_info->firmware_version)); scpi_info->scpi_ops = &scpi_ops; + scpi_init_extensions(scpi_info, np); + ret = sysfs_create_groups(&dev->kobj, versions_groups); if (ret) dev_err(dev, "unable to create sysfs version group\n"); diff --git i/include/linux/scpi_protocol.h w/include/linux/scpi_protocol.h index 35de50a65665..052f6aa1ae4b 100644 --- i/include/linux/scpi_protocol.h +++ w/include/linux/scpi_protocol.h @@ -72,8 +72,13 @@ struct scpi_ops { int (*sensor_get_value)(u16, u64 *); }; +struct scpi_vendor_ext_ops { +}; + #if IS_REACHABLE(CONFIG_ARM_SCPI_PROTOCOL) struct scpi_ops *get_scpi_ops(void); +void *get_scpi_ext_ops(void); #else static inline struct scpi_ops *get_scpi_ops(void) { return NULL; } +static inline void *get_scpi_ext_ops(void) { return NULL; } #endif