Received: by 10.223.185.116 with SMTP id b49csp2634889wrg; Mon, 5 Mar 2018 06:21:58 -0800 (PST) X-Google-Smtp-Source: AG47ELsSHKkCtuhlGm6YZTNfD9pv2Xiaeyn3X3NaFtnGsU0ck3j47X3Fhlo4QH3C+qRStQDbhzHQ X-Received: by 10.99.126.24 with SMTP id z24mr12204962pgc.343.1520259718108; Mon, 05 Mar 2018 06:21:58 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1520259718; cv=none; d=google.com; s=arc-20160816; b=iV9KLQb7i4aPT3cLNasAExEn7lgulBu6/4gpdYFrLfCYc3X8dS9GosWX0K4+ziRb6z WKq7xV2ulGPIcRB7FTJIrZpWmDZJouCYUmMTIe5y5KpZUviNEr/9mvEC4U4m9ZJoQhUb GpYoyGByroM7FkKaXtUR4uP5jnvy0nrzn1UP4Leqm5gpRHprzAkty0ztTmU/qKaqhwh9 +2GCv2BwBHn12kR9lSt6yi4Ufm89gRFRChNMzLP81aTVza/Z6oN6D10lpSqZF2vieQcy j76ZLnGTlnrP/2lf6o/DdXGwc6hxIszCMsNJd28ikuM8OvebKBEI/SyS+WYsB86HpEGv PlKQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:subject:cc:to:from:date :arc-authentication-results; bh=E//JgVDEdxGlNGFaI7c5S3ZF1By4UpMaa1X+EfqXHOI=; b=XKuFiBoBKdATNDeWrOr62+PASGliCQBh2fHzcqxNstFh0k7IdR0TIsZdL/spAWfFGw lvK+ReVCrPvLPyPH0rWguGh81V7+zwFx3XXnQmEWbZ48YEZ15bu7tc1WrN7WlsVxcwJM OdH1vwXdiBXnsu30nUTovjhrgitX+qAQcKoGNWOh0aXKA0z92o+Xl3J1lEFmqPJswVbi aNQq0URG8NcG/bj1hnNn58X68sNasmonp1HItxkX48AmoCqnqj9fjIh2+jbZNuORjtma N/nC8W1IIjEIGbAUXP26lhttxjzeXKuZkggJsJJveXbI2hnjHHVM4YWxc0Id/1F9+KpD 677w== ARC-Authentication-Results: i=1; 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 Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id t11si477576pgn.624.2018.03.05.06.21.43; Mon, 05 Mar 2018 06:21:58 -0800 (PST) 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 S1752054AbeCEOMW (ORCPT + 99 others); Mon, 5 Mar 2018 09:12:22 -0500 Received: from szxga06-in.huawei.com ([45.249.212.32]:54858 "EHLO huawei.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1751777AbeCEOMU (ORCPT ); Mon, 5 Mar 2018 09:12:20 -0500 Received: from DGGEMS410-HUB.china.huawei.com (unknown [172.30.72.60]) by Forcepoint Email with ESMTP id 9E2AD1FE1C555; Mon, 5 Mar 2018 22:12:06 +0800 (CST) Received: from localhost (10.202.226.43) by DGGEMS410-HUB.china.huawei.com (10.3.19.210) with Microsoft SMTP Server (TLS) id 14.3.361.1; Mon, 5 Mar 2018 22:12:04 +0800 Date: Mon, 5 Mar 2018 14:11:54 +0000 From: Jonathan Cameron To: Sudeep Holla CC: ALKML , LKML , DTML , "Alexey Klimov" , Greg Kroah-Hartman , Arnd Bergmann Subject: Re: [PATCH v6 04/20] firmware: arm_scmi: add common infrastructure and support for base protocol Message-ID: <20180305151154.000051c2@huawei.com> In-Reply-To: <1519403030-21189-5-git-send-email-sudeep.holla@arm.com> References: <1519403030-21189-1-git-send-email-sudeep.holla@arm.com> <1519403030-21189-5-git-send-email-sudeep.holla@arm.com> X-Mailer: Claws Mail 3.16.0 (GTK+ 2.24.31; i686-w64-mingw32) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.202.226.43] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 23 Feb 2018 16:23:34 +0000 Sudeep Holla wrote: > The base protocol describes the properties of the implementation and > provide generic error management. The base protocol provides commands > to describe protocol version, discover implementation specific > attributes and vendor/sub-vendor identification, list of protocols > implemented and the various agents are in the system including OSPM > and the platform. It also supports registering for notifications of > platform errors. > > This protocol is mandatory. This patch adds support for the same along > with some basic infrastructure to add support for other protocols. > Minor comments inline. > diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c > index fa0e9cf31f4c..ba784b8ec170 100644 > --- a/drivers/firmware/arm_scmi/driver.c > +++ b/drivers/firmware/arm_scmi/driver.c > @@ -105,21 +105,27 @@ struct scmi_desc { > * @dev: Device pointer > * @desc: SoC description for this instance > * @handle: Instance of SCMI handle to send to clients > + * @version: SCMI revision information containing protocol version, > + * implementation version and (sub-)vendor identification. > * @cl: Mailbox Client > * @tx_chan: Transmit mailbox channel > * @tx_payload: Transmit mailbox channel payload area > * @minfo: Message info > + * @protocols_imp: list of protocols implemented, currently maximum of > + * MAX_PROTOCOLS_IMP elements allocated by the base protocol If it is fixed size, is there a benefit in not just putting it in here as a fixed size array and simplifying the allocation? > * @node: list head > * @users: Number of users of this instance > */ > struct scmi_info { > struct device *dev; > const struct scmi_desc *desc; > + struct scmi_revision_info version; > struct scmi_handle handle; > struct mbox_client cl; > struct mbox_chan *tx_chan; > void __iomem *tx_payload; > struct scmi_xfers_info minfo; > + u8 *protocols_imp; > struct list_head node; > int users; > }; > @@ -433,6 +439,45 @@ int scmi_one_xfer_init(const struct scmi_handle *handle, u8 msg_id, u8 prot_id, > } > > /** > + * scmi_version_get() - command to get the revision of the SCMI entity > + * > + * @handle: Handle to SCMI entity information > + * > + * Updates the SCMI information in the internal data structure. > + * > + * Return: 0 if all went fine, else return appropriate error. > + */ > +int scmi_version_get(const struct scmi_handle *handle, u8 protocol, > + u32 *version) > +{ > + int ret; > + __le32 *rev_info; > + struct scmi_xfer *t; > + > + ret = scmi_one_xfer_init(handle, PROTOCOL_VERSION, protocol, 0, > + sizeof(*version), &t); > + if (ret) > + return ret; > + > + ret = scmi_do_xfer(handle, t); > + if (!ret) { > + rev_info = t->rx.buf; > + *version = le32_to_cpu(*rev_info); > + } > + > + scmi_one_xfer_put(handle, t); blank line before all simple returns is common kernel coding style and does help for readability (a little bit) > + return ret; > +} > + > +void scmi_setup_protocol_implemented(const struct scmi_handle *handle, > + u8 *prot_imp) > +{ > + struct scmi_info *info = handle_to_scmi_info(handle); > + > + info->protocols_imp = prot_imp; > +} > + > +/** > * scmi_handle_get() - Get the SCMI handle for a device > * > * @dev: pointer to device for which we want SCMI handle > @@ -660,11 +705,19 @@ static int scmi_probe(struct platform_device *pdev) > > handle = &info->handle; > handle->dev = info->dev; > + handle->version = &info->version; > > ret = scmi_mbox_chan_setup(info); > if (ret) > return ret; > > + ret = scmi_base_protocol_init(handle); > + if (ret) { > + dev_err(dev, "unable to communicate with SCMI(%d)\n", ret); > + scmi_mbox_free_channel(info); > + return ret; > + } > + > mutex_lock(&scmi_list_mutex); > list_add_tail(&info->node, &scmi_list); > mutex_unlock(&scmi_list_mutex); > diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h > index 854ed2479993..664da3d763f2 100644 > --- a/include/linux/scmi_protocol.h > +++ b/include/linux/scmi_protocol.h > @@ -17,11 +17,48 @@ > */ > #include > > +#define SCMI_MAX_STR_SIZE 16 > + > +/** > + * struct scmi_revision_info - version information structure This isn't really just the version stuff. It's more about discovery of what is present. Perhaps the naming could make that clear if you want to keep this as one structure? > + * > + * @major_ver: Major ABI version. Change here implies risk of backward > + * compatibility break. > + * @minor_ver: Minor ABI version. Change here implies new feature addition, > + * or compatible change in ABI. > + * @num_protocols: Number of protocols that are implemented, excluding the > + * base protocol. > + * @num_agents: Number of agents in the system. > + * @impl_ver: A vendor-specific implementation version. > + * @vendor_id: A vendor identifier(Null terminated ASCII string) > + * @sub_vendor_id: A sub-vendor identifier(Null terminated ASCII string) > + */ > +struct scmi_revision_info { > + u16 major_ver; > + u16 minor_ver; > + u8 num_protocols; > + u8 num_agents; > + u32 impl_ver; > + char vendor_id[SCMI_MAX_STR_SIZE]; > + char sub_vendor_id[SCMI_MAX_STR_SIZE]; > +}; > + > /** > * struct scmi_handle - Handle returned to ARM SCMI clients for usage. > * > * @dev: pointer to the SCMI device > + * @version: pointer to the structure containing SCMI version information > */ > struct scmi_handle { > struct device *dev; > + struct scmi_revision_info *version; > +}; > + > +enum scmi_std_protocol { > + SCMI_PROTOCOL_BASE = 0x10, > + SCMI_PROTOCOL_POWER = 0x11, > + SCMI_PROTOCOL_SYSTEM = 0x12, > + SCMI_PROTOCOL_PERF = 0x13, > + SCMI_PROTOCOL_CLOCK = 0x14, > + SCMI_PROTOCOL_SENSOR = 0x15, > };