On Wed, 17 Jan 2024 at 19:36, Sibi Sankar <quic_sibis@xxxxxxxxxxx> wrote: > > From: Shivnandan Kumar <quic_kshivnan@xxxxxxxxxxx> > > SCMI QCOM vendor protocol provides interface to communicate with SCMI > controller and enable vendor specific features like bus scaling capable > of running on it. > > Signed-off-by: Shivnandan Kumar <quic_kshivnan@xxxxxxxxxxx> > Co-developed-by: Ramakrishna Gottimukkula <quic_rgottimu@xxxxxxxxxxx> > Signed-off-by: Ramakrishna Gottimukkula <quic_rgottimu@xxxxxxxxxxx> > Co-developed-by: Amir Vajid <avajid@xxxxxxxxxxx> > Signed-off-by: Amir Vajid <avajid@xxxxxxxxxxx> > Co-developed-by: Sibi Sankar <quic_sibis@xxxxxxxxxxx> > Signed-off-by: Sibi Sankar <quic_sibis@xxxxxxxxxxx> > --- > drivers/firmware/arm_scmi/Kconfig | 11 ++ > drivers/firmware/arm_scmi/Makefile | 1 + > drivers/firmware/arm_scmi/qcom_scmi_vendor.c | 160 +++++++++++++++++++ > include/linux/qcom_scmi_vendor.h | 36 +++++ > 4 files changed, 208 insertions(+) > create mode 100644 drivers/firmware/arm_scmi/qcom_scmi_vendor.c > create mode 100644 include/linux/qcom_scmi_vendor.h > > diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig > index aa5842be19b2..86b5d6c18ec4 100644 > --- a/drivers/firmware/arm_scmi/Kconfig > +++ b/drivers/firmware/arm_scmi/Kconfig > @@ -180,4 +180,15 @@ config ARM_SCMI_POWER_CONTROL > called scmi_power_control. Note this may needed early in boot to catch > early shutdown/reboot SCMI requests. > > +config QCOM_SCMI_VENDOR_PROTOCOL > + tristate "Qualcomm Technologies, Inc. Qcom SCMI vendor Protocol" > + depends on ARM || ARM64 || COMPILE_TEST > + depends on ARM_SCMI_PROTOCOL > + help > + The SCMI QCOM vendor protocol provides interface to communicate with SCMI > + controller and enable vendor specific features like bus scaling. > + > + This driver defines the commands or message ID's used for this > + communication and also exposes the ops used by the clients. > + > endmenu > diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile > index a7bc4796519c..eaeb788b93c6 100644 > --- a/drivers/firmware/arm_scmi/Makefile > +++ b/drivers/firmware/arm_scmi/Makefile > @@ -17,6 +17,7 @@ obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o > obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-module.o > > obj-$(CONFIG_ARM_SCMI_POWER_CONTROL) += scmi_power_control.o > +obj-$(CONFIG_QCOM_SCMI_VENDOR_PROTOCOL) += qcom_scmi_vendor.o > > ifeq ($(CONFIG_THUMB2_KERNEL)$(CONFIG_CC_IS_CLANG),yy) > # The use of R7 in the SMCCC conflicts with the compiler's use of R7 as a frame > diff --git a/drivers/firmware/arm_scmi/qcom_scmi_vendor.c b/drivers/firmware/arm_scmi/qcom_scmi_vendor.c > new file mode 100644 > index 000000000000..878b99f0d1ef > --- /dev/null > +++ b/drivers/firmware/arm_scmi/qcom_scmi_vendor.c > @@ -0,0 +1,160 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2024, The Linux Foundation. All rights reserved. > + */ > + > +#include <linux/qcom_scmi_vendor.h> > + > +#include "common.h" > + > +#define EXTENDED_MSG_ID 0 > +#define SCMI_MAX_TX_RX_SIZE 128 > +#define PROTOCOL_PAYLOAD_SIZE 16 > +#define SET_PARAM 0x10 > +#define GET_PARAM 0x11 > +#define START_ACTIVITY 0x12 > +#define STOP_ACTIVITY 0x13 > + > +static int qcom_scmi_set_param(const struct scmi_protocol_handle *ph, void *buf, u64 algo_str, > + u32 param_id, size_t size) > +{ > + int ret = -EINVAL; > + struct scmi_xfer *t; > + u32 *msg; > + > + if (!ph || !ph->xops) > + return ret; Drop init of ret, return -EINVAL directly here. > + > + ret = ph->xops->xfer_get_init(ph, SET_PARAM, size + PROTOCOL_PAYLOAD_SIZE, > + SCMI_MAX_TX_RX_SIZE, &t); > + if (ret) > + return ret; > + > + msg = t->tx.buf; > + *msg++ = cpu_to_le32(EXTENDED_MSG_ID); > + *msg++ = cpu_to_le32(algo_str & GENMASK(31, 0)); > + *msg++ = cpu_to_le32((algo_str & GENMASK(63, 32)) >> 32); > + *msg++ = cpu_to_le32(param_id); First, this header ops looks like a generic code which can be extracted. Second, using GENMASK here in the ops doesn't make any sense. The values will be limited to u32 anyway. > + memcpy(msg, buf, size); > + ret = ph->xops->do_xfer(ph, t); > + ph->xops->xfer_put(ph, t); > + > + return ret; > +} > + > +static int qcom_scmi_get_param(const struct scmi_protocol_handle *ph, void *buf, u64 algo_str, > + u32 param_id, size_t tx_size, size_t rx_size) > +{ > + int ret = -EINVAL; > + struct scmi_xfer *t; > + u32 *msg; > + > + if (!ph || !ph->xops || !buf) > + return ret; Drop init of ret, return -EINVAL directly here. > + > + ret = ph->xops->xfer_get_init(ph, GET_PARAM, tx_size + PROTOCOL_PAYLOAD_SIZE, > + SCMI_MAX_TX_RX_SIZE, &t); > + if (ret) > + return ret; > + > + msg = t->tx.buf; > + *msg++ = cpu_to_le32(EXTENDED_MSG_ID); > + *msg++ = cpu_to_le32(algo_str & GENMASK(31, 0)); > + *msg++ = cpu_to_le32((algo_str & GENMASK(63, 32)) >> 32); > + *msg++ = cpu_to_le32(param_id); > + memcpy(msg, buf, tx_size); > + ret = ph->xops->do_xfer(ph, t); > + if (t->rx.len > rx_size) { > + pr_err("SCMI received buffer size %zu is more than expected size %zu\n", > + t->rx.len, rx_size); > + return -EMSGSIZE; > + } > + memcpy(buf, t->rx.buf, t->rx.len); > + ph->xops->xfer_put(ph, t); > + > + return ret; > +} > + > +static int qcom_scmi_start_activity(const struct scmi_protocol_handle *ph, > + void *buf, u64 algo_str, u32 param_id, size_t size) > +{ > + int ret = -EINVAL; > + struct scmi_xfer *t; > + u32 *msg; > + > + if (!ph || !ph->xops) > + return ret; You can guess the comment here. > + > + ret = ph->xops->xfer_get_init(ph, START_ACTIVITY, size + PROTOCOL_PAYLOAD_SIZE, > + SCMI_MAX_TX_RX_SIZE, &t); > + if (ret) > + return ret; > + > + msg = t->tx.buf; > + *msg++ = cpu_to_le32(EXTENDED_MSG_ID); > + *msg++ = cpu_to_le32(algo_str & GENMASK(31, 0)); > + *msg++ = cpu_to_le32((algo_str & GENMASK(63, 32)) >> 32); > + *msg++ = cpu_to_le32(param_id); > + memcpy(msg, buf, size); > + ret = ph->xops->do_xfer(ph, t); > + ph->xops->xfer_put(ph, t); > + > + return ret; > +} > + > +static int qcom_scmi_stop_activity(const struct scmi_protocol_handle *ph, void *buf, u64 algo_str, > + u32 param_id, size_t size) > +{ > + int ret = -EINVAL; > + struct scmi_xfer *t; > + u32 *msg; > + > + if (!ph || !ph->xops) > + return ret; > + > + ret = ph->xops->xfer_get_init(ph, STOP_ACTIVITY, size + PROTOCOL_PAYLOAD_SIZE, > + SCMI_MAX_TX_RX_SIZE, &t); > + if (ret) > + return ret; > + > + msg = t->tx.buf; > + *msg++ = cpu_to_le32(EXTENDED_MSG_ID); > + *msg++ = cpu_to_le32(algo_str & GENMASK(31, 0)); > + *msg++ = cpu_to_le32((algo_str & GENMASK(63, 32)) >> 32); > + *msg++ = cpu_to_le32(param_id); > + memcpy(msg, buf, size); > + ret = ph->xops->do_xfer(ph, t); > + ph->xops->xfer_put(ph, t); > + > + return ret; > +} > + > +static struct qcom_scmi_vendor_ops qcom_proto_ops = { > + .set_param = qcom_scmi_set_param, > + .get_param = qcom_scmi_get_param, > + .start_activity = qcom_scmi_start_activity, > + .stop_activity = qcom_scmi_stop_activity, > +}; > + > +static int qcom_scmi_vendor_protocol_init(const struct scmi_protocol_handle *ph) > +{ > + u32 version; > + > + ph->xops->version_get(ph, &version); > + > + dev_info(ph->dev, "qcom scmi version %d.%d\n", > + PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version)); > + > + return 0; > +} > + > +static const struct scmi_protocol qcom_scmi_vendor = { > + .id = QCOM_SCMI_VENDOR_PROTOCOL, > + .owner = THIS_MODULE, > + .instance_init = &qcom_scmi_vendor_protocol_init, > + .ops = &qcom_proto_ops, > +}; > +module_scmi_protocol(qcom_scmi_vendor); > + > +MODULE_DESCRIPTION("QTI SCMI vendor protocol"); > +MODULE_LICENSE("GPL"); > diff --git a/include/linux/qcom_scmi_vendor.h b/include/linux/qcom_scmi_vendor.h > new file mode 100644 > index 000000000000..bde57bb18367 > --- /dev/null > +++ b/include/linux/qcom_scmi_vendor.h > @@ -0,0 +1,36 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * QTI SCMI vendor protocol's header > + * > + * Copyright (c) 2024, The Linux Foundation. All rights reserved. > + */ > + > +#ifndef _QCOM_SCMI_VENDOR_H > +#define _QCOM_SCMI_VENDOR_H > + > +#include <linux/bitfield.h> > +#include <linux/device.h> > +#include <linux/types.h> > + > +#define QCOM_SCMI_VENDOR_PROTOCOL 0x80 > + > +struct scmi_protocol_handle; > +extern struct scmi_device *get_qcom_scmi_device(void); > + > +/** > + * struct qcom_scmi_vendor_ops - represents the various operations provided > + * by qcom scmi vendor protocol > + */ > +struct qcom_scmi_vendor_ops { > + int (*set_param)(const struct scmi_protocol_handle *ph, void *buf, u64 algo_str, > + u32 param_id, size_t size); > + int (*get_param)(const struct scmi_protocol_handle *ph, void *buf, u64 algo_str, > + u32 param_id, size_t tx_size, size_t rx_size); > + int (*start_activity)(const struct scmi_protocol_handle *ph, void *buf, u64 algo_str, > + u32 param_id, size_t size); > + int (*stop_activity)(const struct scmi_protocol_handle *ph, void *buf, u64 algo_str, > + u32 param_id, size_t size); > +}; > + > +#endif /* _QCOM_SCMI_VENDOR_H */ > + > -- > 2.34.1 > > -- With best wishes Dmitry