lkml.org 
[lkml]   [2017]   [Sep]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 04/18] firmware: arm_scmi: add common infrastructure and support for base protocol
From
Date
Hi Sudeep,

On 04/08/17 15:31, 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.
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/firmware/arm_scmi/Makefile | 2 +-
> drivers/firmware/arm_scmi/base.c | 293 +++++++++++++++++++++++++++++++++++++
> drivers/firmware/arm_scmi/common.h | 46 ++++++
> drivers/firmware/arm_scmi/driver.c | 67 +++++++++
> include/linux/scmi_protocol.h | 28 ++++
> 5 files changed, 435 insertions(+), 1 deletion(-)
> create mode 100644 drivers/firmware/arm_scmi/base.c
>
> diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
> index 58e94c95e523..21d01d1d6b9c 100644
> --- a/drivers/firmware/arm_scmi/Makefile
> +++ b/drivers/firmware/arm_scmi/Makefile
> @@ -1,2 +1,2 @@
> obj-$(CONFIG_ARM_SCMI_PROTOCOL) = arm_scmi.o
> -arm_scmi-y = driver.o
> +arm_scmi-y = base.o driver.o
> diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c
> new file mode 100644
> index 000000000000..9bfffbe95c21
> --- /dev/null
> +++ b/drivers/firmware/arm_scmi/base.c
> @@ -0,0 +1,293 @@
> +/*
> + * System Control and Management Interface (SCMI) Base Protocol
> + *
> + * Copyright (C) 2017 ARM Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "common.h"
> +
> +enum scmi_base_protocol_cmd {
> + BASE_DISCOVER_VENDOR = 0x3,
> + BASE_DISCOVER_SUB_VENDOR = 0x4,
> + BASE_DISCOVER_IMPLEMENT_VERSION = 0x5,
> + BASE_DISCOVER_LIST_PROTOCOLS = 0x6,
> + BASE_DISCOVER_AGENT = 0x7,
> + BASE_NOTIFY_ERRORS = 0x8,
> +};
> +
> +struct scmi_msg_resp_base_attributes {
> + u8 num_protocols;
> + u8 num_agents;
> + __le16 reserved;
> +};
> +
> +/**
> + * scmi_base_attributes_get() - gets the implementation details
> + * that are associated with the base protocol.
> + *
> + * @handle - SCMI entity handle
> + *
> + * Return: 0 on success, else appropriate SCMI error.
> + */
> +static int scmi_base_attributes_get(const struct scmi_handle *handle)
> +{
> + int ret;
> + struct scmi_xfer *t;
> + struct scmi_msg_resp_base_attributes *attr_info;
> + struct scmi_revision_info *rev = handle->version;
> +
> + ret = scmi_one_xfer_init(handle, PROTOCOL_ATTRIBUTES,
> + SCMI_PROTOCOL_BASE, 0, sizeof(*attr_info), &t);
> + if (ret)
> + return ret;
> +
> + ret = scmi_do_xfer(handle, t);
> + if (!ret) {
> + attr_info = t->rx.buf;
> + rev->num_protocols = attr_info->num_protocols;
> + rev->num_agents = attr_info->num_agents;
> + }
> +
> + scmi_one_xfer_put(handle, t);
> + return ret;
> +}
> +
> +/**
> + * scmi_base_vendor_id_get() - gets vendor/subvendor identifier ASCII string.
> + *
> + * @handle - SCMI entity handle
> + * @sub_vendor - specify true if sub-vendor ID is needed
> + *
> + * Return: 0 on success, else appropriate SCMI error.
> + */
> +static int
> +scmi_base_vendor_id_get(const struct scmi_handle *handle, bool sub_vendor)
> +{
> + u8 cmd;
> + int ret, size;
> + char *vendor_id;
> + struct scmi_xfer *t;
> + struct scmi_revision_info *rev = handle->version;
> +
> + if (sub_vendor) {
> + cmd = BASE_DISCOVER_SUB_VENDOR;
> + vendor_id = rev->sub_vendor_id;
> + size = ARRAY_SIZE(rev->sub_vendor_id);
> + } else {
> + cmd = BASE_DISCOVER_VENDOR;
> + vendor_id = rev->vendor_id;
> + size = ARRAY_SIZE(rev->vendor_id);
> + }
> +
> + ret = scmi_one_xfer_init(handle, cmd, SCMI_PROTOCOL_BASE, 0, size, &t);
> + if (ret)
> + return ret;
> +
> + ret = scmi_do_xfer(handle, t);
> + if (!ret)
> + memcpy(vendor_id, t->rx.buf, size);
> +
> + scmi_one_xfer_put(handle, t);
> + return ret;
> +}
> +
> +/**
> + * scmi_base_implementation_version_get() - gets a vendor-specific
> + * implementation 32-bit version. The format of the version number is
> + * vendor-specific
> + *
> + * @handle - SCMI entity handle
> + *
> + * Return: 0 on success, else appropriate SCMI error.
> + */
> +static int
> +scmi_base_implementation_version_get(const struct scmi_handle *handle)
> +{
> + int ret;
> + u32 *impl_ver;
> + struct scmi_xfer *t;
> + struct scmi_revision_info *rev = handle->version;
> +
> + ret = scmi_one_xfer_init(handle, BASE_DISCOVER_IMPLEMENT_VERSION,
> + SCMI_PROTOCOL_BASE, 0, sizeof(*impl_ver), &t);
> + if (ret)
> + return ret;
> +
> + ret = scmi_do_xfer(handle, t);
> + if (!ret) {
> + impl_ver = t->rx.buf;
> + rev->impl_ver = le32_to_cpu(*impl_ver);
> + }
> +
> + scmi_one_xfer_put(handle, t);
> + return ret;
> +}
> +
> +/**
> + * scmi_base_implementation_list_get() - gets the list of protocols it is
> + * OSPM is allowed to access
> + *
> + * @handle - SCMI entity handle
> + * @protocols_imp - pointer to hold the list of protocol identifiers
> + *
> + * Return: 0 on success, else appropriate SCMI error.
> + */
> +static int scmi_base_implementation_list_get(const struct scmi_handle *handle,
> + u8 *protocols_imp)
> +{
> + u8 *list;
> + int ret, loop;
> + struct scmi_xfer *t;
> + __le32 *num_skip, *num_ret;
> + u32 tot_num_ret = 0, loop_num_ret;
> + struct device *dev = handle->dev;
> +
> + ret = scmi_one_xfer_init(handle, BASE_DISCOVER_LIST_PROTOCOLS,
> + SCMI_PROTOCOL_BASE, sizeof(*num_skip), 0, &t);
> + if (ret)
> + return ret;
> +
> + num_skip = t->tx.buf;
> + num_ret = t->rx.buf;
> + list = t->rx.buf + sizeof(*num_ret);
> +
> + do {
> + /* Set the number of protocols to be skipped/already read */
> + *num_skip = cpu_to_le32(tot_num_ret);
> +
> + ret = scmi_do_xfer(handle, t);
> + if (ret)
> + break;
> +
> + loop_num_ret = le32_to_cpu(*num_ret);
> + if (tot_num_ret + loop_num_ret > MAX_PROTOCOLS_IMP) {
> + dev_err(dev, "No. of Protocol > MAX_PROTOCOLS_IMP");
> + break;
> + }
> +
> + for (loop = 0; loop < loop_num_ret; loop++)
> + protocols_imp[tot_num_ret + loop] = *(list + loop);
> +
> + tot_num_ret += loop_num_ret;
> + } while (loop_num_ret);
> +
> + scmi_one_xfer_put(handle, t);
> + return ret;
> +}
> +
> +/**
> + * scmi_base_discover_agent_get() - discover the name of an agent
> + *
> + * @handle - SCMI entity handle
> + * @id - Agent identifier
> + * @name - Agent identifier ASCII string
> + *
> + * An agent id of 0 is reserved to identify the platform itself.
> + * Generally operating system is represented as "OSPM"
> + *
> + * Return: 0 on success, else appropriate SCMI error.
> + */
> +static int scmi_base_discover_agent_get(const struct scmi_handle *handle,
> + int id, char *name)
> +{
> + int ret;
> + struct scmi_xfer *t;
> +
> + ret = scmi_one_xfer_init(handle, BASE_DISCOVER_AGENT,
> + SCMI_PROTOCOL_BASE, sizeof(__le32),
> + SCMI_MAX_STR_SIZE, &t);
> + if (ret)
> + return ret;
> +
> + *(__le32 *)t->tx.buf = cpu_to_le32(id);
> +
> + ret = scmi_do_xfer(handle, t);
> + if (!ret)
> + memcpy(name, t->rx.buf, SCMI_MAX_STR_SIZE);
> +
> + scmi_one_xfer_put(handle, t);
> + return ret;
> +}
> +
> +/**
> + * scmi_base_error_notifications_enable() - register/unregister for
> + * notifications of errors in the platform
> + *
> + * @handle - SCMI entity handle
> + * @enable - Enable/Disable the notification
> + *
> + * Return: 0 on success, else appropriate SCMI error.
> + */
> +static int
> +scmi_base_error_notifications_enable(const struct scmi_handle *handle, bool en)
> +{
> + int ret;
> + struct scmi_xfer *t;
> +
> + ret = scmi_one_xfer_init(handle, BASE_NOTIFY_ERRORS, SCMI_PROTOCOL_BASE,
> + sizeof(__le32), 0, &t);
> + if (ret)
> + return ret;
> +
> + *(__le32 *)t->tx.buf = cpu_to_le32(en & BIT(0));
> +
> + ret = scmi_do_xfer(handle, t);
> +
> + scmi_one_xfer_put(handle, t);
> + return ret;
> +}
> +
> +int scmi_base_protocol_init(struct scmi_handle *h)
> +{
> + int id, ret;
> + u8 *prot_imp;
> + u32 version;
> + char name[SCMI_MAX_STR_SIZE];
> + const struct scmi_handle *handle = h;
> + struct device *dev = handle->dev;
> + struct scmi_revision_info *rev = handle->version;
> +
> + ret = scmi_version_get(handle, SCMI_PROTOCOL_BASE, &version);
> + if (ret)
> + return ret;
> +
> + prot_imp = devm_kcalloc(dev, MAX_PROTOCOLS_IMP, sizeof(u8), GFP_KERNEL);
> + if (!prot_imp)
> + return -ENOMEM;
> +
> + rev->major_ver = PROTOCOL_REV_MAJOR(version),
> + rev->minor_ver = PROTOCOL_REV_MINOR(version);
> +
> + scmi_base_attributes_get(handle);
> + scmi_base_vendor_id_get(handle, false);
> + scmi_base_vendor_id_get(handle, true);
> + scmi_base_implementation_version_get(handle);
> + scmi_base_implementation_list_get(handle, prot_imp);
> + scmi_base_error_notifications_enable(handle, true);
> + scmi_setup_protocol_implemented(handle, prot_imp);
> +
> + dev_info(dev, "SCMI Protocol v%d.%d '%s:%s' Firmware version 0x%x\n",
> + rev->major_ver, rev->minor_ver, rev->vendor_id,
> + rev->sub_vendor_id, rev->impl_ver);
> + dev_dbg(dev, "Found %d protocol(s) %d agent(s)\n", rev->num_protocols,
> + rev->num_agents);
> +
> + for (id = 0; id < rev->num_agents; id++) {
> + scmi_base_discover_agent_get(handle, id, name);
> + dev_dbg(dev, "Agent %d: %s\n", id, name);
> + }
> +
> + return 0;
> +}
> diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
> index a6829afc17e3..e3fe5d9acc82 100644
> --- a/drivers/firmware/arm_scmi/common.h
> +++ b/drivers/firmware/arm_scmi/common.h
> @@ -19,9 +19,50 @@
> */
>
> #include <linux/completion.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/kernel.h>
> #include <linux/scmi_protocol.h>
> #include <linux/types.h>
>
> +#define PROTOCOL_REV_MINOR_BITS 16
> +#define PROTOCOL_REV_MINOR_MASK ((1U << PROTOCOL_REV_MINOR_BITS) - 1)
> +#define PROTOCOL_REV_MAJOR(x) ((x) >> PROTOCOL_REV_MINOR_BITS)
> +#define PROTOCOL_REV_MINOR(x) ((x) & PROTOCOL_REV_MINOR_MASK)
> +#define MAX_PROTOCOLS_IMP 16
> +
> +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,
> +};
> +
> +enum scmi_common_cmd {
> + PROTOCOL_VERSION = 0x0,
> + PROTOCOL_ATTRIBUTES = 0x1,
> + PROTOCOL_MESSAGE_ATTRIBUTES = 0x2,
> +};
> +
> +/**
> + * struct scmi_msg_resp_prot_version - Response for a message
> + *
> + * @major_version: Major version of the ABI that firmware supports
> + * @minor_version: Minor version of the ABI that firmware supports
> + *
> + * In general, ABI version changes follow the rule that minor version increments
> + * are backward compatible. Major revision changes in ABI may not be
> + * backward compatible.
> + *
> + * Response to a generic message with message type SCMI_MSG_VERSION
> + */
> +struct scmi_msg_resp_prot_version {
> + __le16 minor_version;
> + __le16 major_version;
> +};
> +
> /**
> * struct scmi_msg_hdr - Message(Tx/Rx) header
> *
> @@ -72,3 +113,8 @@ void scmi_one_xfer_put(const struct scmi_handle *h, struct scmi_xfer *xfer);
> int scmi_do_xfer(const struct scmi_handle *h, struct scmi_xfer *xfer);
> int scmi_one_xfer_init(const struct scmi_handle *h, u8 msg_id, u8 prot_id,
> size_t tx_size, size_t rx_size, struct scmi_xfer **p);
> +int scmi_version_get(const struct scmi_handle *h, u8 protocol, u32 *version);
> +void scmi_setup_protocol_implemented(const struct scmi_handle *handle,
> + u8 *prot_imp);
> +
> +int scmi_base_protocol_init(struct scmi_handle *h);
> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> index 139d6980f270..601d0d7210d9 100644
> --- a/drivers/firmware/arm_scmi/driver.c
> +++ b/drivers/firmware/arm_scmi/driver.c
> @@ -108,18 +108,22 @@ 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
> * @rx_chan: Receive mailbox channel
> * @tx_payload: Transmit mailbox channel payload area
> * @rx_payload: Receive mailbox channel payload area
> * @minfo: Message info
> + * @protocols_imp: list of protocols implemented
> * @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;
> @@ -127,6 +131,7 @@ struct scmi_info {
> void __iomem *tx_payload;
> void __iomem *rx_payload;
> struct scmi_xfers_info minfo;
> + u8 *protocols_imp;

Both the base protocol and driver part rely on this being of size
MAX_PROTOCOLS_IMP (if existing).

Could this be a "u8 protocols_imp[MAX_PROTOCOLS_IMP]" instead? Or at
least add a comment making it clearer at the scmi_info definition that
this array will have MAX_PROTOCOLS_IMP elements in all scmi_info instances?

Other than that, patch seems fine.

Cheers,

--
Julien Thierry

\
 
 \ /
  Last update: 2017-09-05 15:40    [W:0.328 / U:0.712 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site