* [PATCH 1/4] firmware: add new driver for SCMI firmwares
[not found] <20200717153731.10643-1-etienne.carriere@linaro.org>
@ 2020-07-20 2:01 ` Peng Fan
2020-07-24 9:46 ` Etienne Carriere
[not found] ` <20200717153731.10643-2-etienne.carriere@linaro.org>
` (3 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Peng Fan @ 2020-07-20 2:01 UTC (permalink / raw)
To: u-boot
Hi Etienne,
+Sudeep
> Subject: [PATCH 1/4] firmware: add new driver for SCMI firmwares
Thanks for posting this out.
>
> This change introduces SCMI agent driver in U-Boot in the firmware U-class.
>
> SCMI agent driver is designed for platforms that embed a SCMI server in a
> firmware hosted for example by a companion co-processor or the secure
> world of the executing processor.
>
> SCMI protocols allow an SCMI agent to discover and access external
> resources as clock, reset controllers and many more. SCMI agent and server
> communicate following the SCMI specification [1]. SCMI agent complies with
> the DT bindings defined in the Linux kernel source tree regarding SCMI agent
> description since v5.8-rc1.
>
> These bindings describe 2 supported message transport layer: using mailbox
> uclass devices or using Arm SMC invocation instruction. Both use a piece or
> shared memory for message data exchange.
>
> In the current state, the SCMI agent driver does not bind to any SCMI protocol
> to a U-Boot device driver. Former changes will implement dedicated driver (i.e.
> an SCMI clock driver or an SCMI reset controller
> driver) and add bind supported SCMI protocols in scmi_agent_bind().
>
> Links: [1]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdevelo
> per.arm.com%2Farchitectures%2Fsystem-architectures%2Fsoftware-standar
> ds%2Fscmi&data=02%7C01%7Cpeng.fan%40nxp.com%7C39c55064be5
> 04bf248a708d82a6775cd%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
> C0%7C637305971142916174&sdata=5kCgz3kzzk4qHy598u79zz3hV17yV
> zdPxM531sOAnUs%3D&reserved=0
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
>
> drivers/firmware/Kconfig | 15 ++
> drivers/firmware/Makefile | 1 +
> drivers/firmware/scmi.c | 439
> ++++++++++++++++++++++++++++++++++++++
> include/scmi.h | 82 +++++++
> 4 files changed, 537 insertions(+)
> create mode 100644 drivers/firmware/scmi.c create mode 100644
> include/scmi.h
>
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index
> b70a2063551..f7c7ee7a5aa 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -1,6 +1,21 @@
> config FIRMWARE
> bool "Enable Firmware driver support"
>
> +config SCMI_FIRMWARE
> + bool "Enable SCMI support"
> + select FIRMWARE
> + select OF_TRANSLATE
> + depends on DM_MAILBOX || ARM_SMCCC
> + help
> + An SCMI agent communicates with a related SCMI server firmware
> + located in another sub-system, as a companion micro controller
> + or a companion host in the CPU system.
> +
> + Communications between agent (client) and the SCMI server are
> + based on message exchange. Messages can be exchange over tranport
> + channels as a mailbox device or an Arm SMCCC service with some
> + piece of identified shared memory.
> +
> config SPL_FIRMWARE
> bool "Enable Firmware driver support in SPL"
> depends on FIRMWARE
> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile index
> a0c250a473e..3965838179f 100644
> --- a/drivers/firmware/Makefile
> +++ b/drivers/firmware/Makefile
> @@ -2,4 +2,5 @@ obj-$(CONFIG_FIRMWARE) += firmware-uclass.o
> obj-$(CONFIG_$(SPL_)ARM_PSCI_FW) += psci.o
> obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o
> obj-$(CONFIG_SANDBOX) += firmware-sandbox.o
> +obj-$(CONFIG_SCMI_FIRMWARE) += scmi.o
> obj-$(CONFIG_ZYNQMP_FIRMWARE) += firmware-zynqmp.o
> diff --git a/drivers/firmware/scmi.c b/drivers/firmware/scmi.c new file mode
> 100644 index 00000000000..fa8a91c3f3d
> --- /dev/null
> +++ b/drivers/firmware/scmi.c
> @@ -0,0 +1,439 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2015-2019, Arm Limited and Contributors. All rights
> reserved.
> + * Copyright (C) 2019-2020 Linaro Limited.
> + */
> +
> +#include <common.h>
> +#include <cpu_func.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <mailbox.h>
> +#include <memalign.h>
> +#include <scmi.h>
> +#include <asm/system.h>
> +#include <asm/types.h>
> +#include <dm/device-internal.h>
> +#include <dm/devres.h>
> +#include <dm/lists.h>
> +#include <dm/ofnode.h>
> +#include <linux/arm-smccc.h>
> +#include <linux/compat.h>
> +#include <linux/errno.h>
> +#include <linux/io.h>
> +#include <linux/ioport.h>
> +
> +#define TIMEOUT_US_10MS 10000
> +
> +struct error_code {
> + int scmi;
> + int errno;
> +};
> +
> +static const struct error_code scmi_linux_errmap[] = {
> + { .scmi = SCMI_NOT_SUPPORTED, .errno = -EOPNOTSUPP, },
> + { .scmi = SCMI_INVALID_PARAMETERS, .errno = -EINVAL, },
> + { .scmi = SCMI_DENIED, .errno = -EACCES, },
> + { .scmi = SCMI_NOT_FOUND, .errno = -ENOENT, },
> + { .scmi = SCMI_OUT_OF_RANGE, .errno = -ERANGE, },
> + { .scmi = SCMI_BUSY, .errno = -EBUSY, },
> + { .scmi = SCMI_COMMS_ERROR, .errno = -ECOMM, },
> + { .scmi = SCMI_GENERIC_ERROR, .errno = -EIO, },
> + { .scmi = SCMI_HARDWARE_ERROR, .errno = -EREMOTEIO, },
> + { .scmi = SCMI_PROTOCOL_ERROR, .errno = -EPROTO, }, };
> +
> +int scmi_to_linux_errno(s32 scmi_code)
> +{
> + int n;
> +
> + if (scmi_code == 0)
!scmi_code
> + return 0;
> +
> + for (n = 0; n < ARRAY_SIZE(scmi_linux_errmap); n++)
> + if (scmi_code == scmi_linux_errmap[n].scmi)
> + return scmi_linux_errmap[1].errno;
> +
> + return -EPROTO;
> +}
> +
> +struct method_ops {
> + int (*process_msg)(struct udevice *dev, struct scmi_msg *msg);
> + int (*remove_agent)(struct udevice *dev); };
> +
> +struct scmi_agent {
> + struct method_ops *method_ops;
> + void *method_priv;
> +};
> +
> +/*
> + * Shared Memory based Transport (SMT) message buffer management
> + *
> + * SMT uses 28 byte header prior message payload to handle the state of
> + * the communication channel realized by the shared memory area and
> + * to define SCMI protocol information the payload relates to.
> + */
> +struct scmi_smt_header {
> + __le32 reserved;
> + __le32 channel_status;
> +#define SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR BIT(1)
> +#define SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE BIT(0)
> + __le32 reserved1[2];
> + __le32 flags;
> +#define SCMI_SHMEM_FLAG_INTR_ENABLED BIT(0)
> + __le32 length;
> + __le32 msg_header;
> + u8 msg_payload[0];
> +};
> +
> +#define SMT_HEADER_TOKEN(token) (((token) << 18) & GENMASK(31,
> 18))
> +#define SMT_HEADER_PROTOCOL_ID(proto) (((proto) << 10) &
> GENMASK(17, 10))
> +#define SMT_HEADER_MESSAGE_TYPE(type) (((type) << 18) & GENMASK(9,
> 8))
> +#define SMT_HEADER_MESSAGE_ID(id) ((id) & GENMASK(7, 0))
> +
> +struct scmi_shm_buf {
> + u8 *buf;
> + size_t size;
> +};
> +
> +static int get_shm_buffer(struct udevice *dev, struct scmi_shm_buf
> +*shm) {
> + int rc;
> + struct ofnode_phandle_args args;
> + struct resource resource;
> + fdt32_t faddr;
> + phys_addr_t paddr;
> +
> + rc = dev_read_phandle_with_args(dev, "shmem", NULL, 0, 0, &args);
> + if (rc)
> + return rc;
> +
> + rc = ofnode_read_resource(args.node, 0, &resource);
> + if (rc)
> + return rc;
> +
> + faddr = cpu_to_fdt32(resource.start);
> + paddr = ofnode_translate_address(args.node, &faddr);
> +
> + shm->size = resource_size(&resource);
> + if (shm->size < sizeof(struct scmi_smt_header)) {
> + dev_err(dev, "Shared memory buffer too small\n");
> + return -EINVAL;
> + }
> +
> + shm->buf = devm_ioremap(dev, paddr, shm->size);
> + if (!shm->buf)
> + return -ENOMEM;
> +
> + if (dcache_status())
> + mmu_set_region_dcache_behaviour((uintptr_t)shm->buf,
> + shm->size, DCACHE_OFF);
Could dev_read_addr be used to replace the upper code block?
> +
> + return 0;
> +}
> +
> +static int write_msg_to_smt(struct udevice *dev, struct scmi_shm_buf
> *shm_buf,
> + struct scmi_msg *msg)
> +{
> + struct scmi_smt_header *hdr = (void *)shm_buf->buf;
> +
> + if ((!msg->in_msg && msg->in_msg_sz) ||
> + (!msg->out_msg && msg->out_msg_sz))
> + return -EINVAL;
> +
> + if (!(hdr->channel_status &
> SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE)) {
> + dev_dbg(dev, "Channel busy\n");
> + return -EBUSY;
> + }
> +
> + if (shm_buf->size < (sizeof(*hdr) + msg->in_msg_sz) ||
> + shm_buf->size < (sizeof(*hdr) + msg->out_msg_sz)) {
> + dev_dbg(dev, "Buffer too small\n");
> + return -ETOOSMALL;
> + }
> +
> + /* Load message in shared memory */
> + hdr->channel_status &= ~SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE;
> + hdr->length = msg->in_msg_sz + sizeof(hdr->msg_header);
> + hdr->msg_header = SMT_HEADER_TOKEN(0) |
> + SMT_HEADER_MESSAGE_TYPE(0) |
> + SMT_HEADER_PROTOCOL_ID(msg->protocol_id) |
> + SMT_HEADER_MESSAGE_ID(msg->message_id);
> +
> + memcpy(hdr->msg_payload, msg->in_msg, msg->in_msg_sz);
memcpy_toio?
> +
> + return 0;
> +}
> +
> +static int read_resp_from_smt(struct udevice *dev, struct scmi_shm_buf
> *shm_buf,
> + struct scmi_msg *msg)
> +{
> + struct scmi_smt_header *hdr = (void *)shm_buf->buf;
> +
> + if (!(hdr->channel_status &
> SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE)) {
> + dev_err(dev, "Channel unexpectedly busy\n");
> + return -EBUSY;
> + }
> +
> + if (hdr->channel_status & SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR)
> {
> + dev_err(dev, "Channel error reported, reset channel\n");
> + return -ECOMM;
> + }
> +
> + if (hdr->length > msg->out_msg_sz + sizeof(hdr->msg_header)) {
> + dev_err(dev, "Buffer to small\n");
> + return -ETOOSMALL;
> + }
> +
> + /* Get the data */
> + msg->out_msg_sz = hdr->length - sizeof(hdr->msg_header);
> + memcpy(msg->out_msg, hdr->msg_payload, msg->out_msg_sz);
> +
> + return 0;
> +}
> +
> +static void clear_smt_channel(struct scmi_shm_buf *shm_buf) {
> + struct scmi_smt_header *hdr = (void *)shm_buf->buf;
> +
> + hdr->channel_status &=
> ~SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR;
> +}
> +
> +struct scmi_mbox_channel {
> + struct scmi_shm_buf shm_buf;
> + struct mbox_chan mbox;
> + ulong timeout_us;
> +};
> +
> +static int mbox_process_msg(struct udevice *dev, struct scmi_msg *msg)
> +{
> + struct scmi_agent *agent = dev_get_priv(dev);
> + struct scmi_mbox_channel *chan = agent->method_priv;
> + int rc;
> +
> + rc = write_msg_to_smt(dev, &chan->shm_buf, msg);
> + if (rc)
> + return rc;
> +
> + /* Give shm addr to mbox in case it is meaningful */
> + rc = mbox_send(&chan->mbox, chan->shm_buf.buf);
> + if (rc) {
> + dev_err(dev, "Message send failed: %d\n", rc);
> + goto out;
> + }
> +
> + /* Receive the response */
> + rc = mbox_recv(&chan->mbox, chan->shm_buf.buf, chan->timeout_us);
> + if (rc) {
> + dev_err(dev, "Response failed: %d, abort\n", rc);
> + goto out;
> + }
> +
> + rc = read_resp_from_smt(dev, &chan->shm_buf, msg);
> +
> +out:
> + clear_smt_channel(&chan->shm_buf);
> +
> + return rc;
> +}
> +
> +struct method_ops mbox_channel_ops = {
> + .process_msg = mbox_process_msg,
> +};
> +
> +static int probe_mailbox_channel(struct udevice *dev) {
> + struct scmi_agent *agent = dev_get_priv(dev);
> + struct scmi_mbox_channel *chan;
> + int rc;
> +
> + chan = devm_kzalloc(dev, sizeof(*chan), GFP_KERNEL);
> + if (!chan)
> + return -ENOMEM;
> +
> + chan->timeout_us = TIMEOUT_US_10MS;
> +
> + rc = mbox_get_by_index(dev, 0, &chan->mbox);
> + if (rc) {
> + dev_err(dev, "Failed to find mailbox: %d\n", rc);
> + goto out;
> + }
> +
> + rc = get_shm_buffer(dev, &chan->shm_buf);
> + if (rc)
> + dev_err(dev, "Failed to get shm resources: %d\n", rc);
> +
> +out:
> + if (rc) {
> + devm_kfree(dev, chan);
> + } else {
> + agent->method_ops = &mbox_channel_ops;
> + agent->method_priv = (void *)chan;
> + }
> +
> + return rc;
> +}
> +
> +struct scmi_arm_smc_channel {
> + ulong func_id;
> + struct scmi_shm_buf shm_buf;
> +};
> +
> +#define SMCCC_RET_NOT_SUPPORTED ((unsigned long)-1)
> +
> +static int arm_smc_process_msg(struct udevice *dev, struct scmi_msg
> +*msg) {
> + struct scmi_agent *agent = dev_get_priv(dev);
> + struct scmi_arm_smc_channel *chan = agent->method_priv;
> + struct arm_smccc_res res;
> + int rc;
> +
> + rc = write_msg_to_smt(dev, &chan->shm_buf, msg);
> + if (rc)
> + return rc;
> +
> + arm_smccc_smc(chan->func_id, 0, 0, 0, 0, 0, 0, 0, &res);
> + if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
> + rc = -EINVAL;
> + else
> + rc = read_resp_from_smt(dev, &chan->shm_buf, msg);
> +
> + clear_smt_channel(&chan->shm_buf);
Should the clear be done in firmware side?
> +
> + return rc;
> +}
> +
> +struct method_ops arm_smc_channel_ops = {
> + .process_msg = arm_smc_process_msg,
> +};
> +
> +static int probe_arm_smc_channel(struct udevice *dev) {
> + struct scmi_agent *agent = dev_get_priv(dev);
> + struct scmi_arm_smc_channel *chan;
> + ofnode node = dev_ofnode(dev);
> + u32 func_id;
> + int rc;
> +
> + chan = devm_kzalloc(dev, sizeof(*chan), GFP_KERNEL);
> + if (!chan)
> + return -ENOMEM;
> +
> + if (ofnode_read_u32(node, "arm,smc-id", &func_id)) {
> + dev_err(dev, "Missing property func-id\n");
> + return -EINVAL;
> + }
> +
> + chan->func_id = func_id;
> +
> + rc = get_shm_buffer(dev, &chan->shm_buf);
> + if (rc) {
> + dev_err(dev, "Failed to get shm resources: %d\n", rc);
> + return rc;
> + }
> +
> + agent->method_ops = &arm_smc_channel_ops;
> + agent->method_priv = (void *)chan;
> +
> + return rc;
> +}
> +
> +/*
> + * Exported functions by the SCMI agent */
> +
> +int scmi_send_and_process_msg(struct udevice *dev, struct scmi_msg
> +*msg) {
> + struct scmi_agent *agent = dev_get_priv(dev);
> +
> + return agent->method_ops->process_msg(dev, msg); }
> +
> +static int scmi_remove(struct udevice *dev) {
> + struct scmi_agent *agent = dev_get_priv(dev);
> +
> + if (agent->method_ops->remove_agent)
> + return agent->method_ops->remove_agent(dev);
> +
> + return 0;
> +}
> +
> +enum scmi_transport_channel {
> + SCMI_MAILBOX_TRANSPORT,
> + SCMI_ARM_SMCCC_TRANSPORT,
> +};
> +
> +static int scmi_probe(struct udevice *dev) {
> + switch (dev_get_driver_data(dev)) {
> + case SCMI_MAILBOX_TRANSPORT:
> + if (IS_ENABLED(CONFIG_DM_MAILBOX))
> + return probe_mailbox_channel(dev);
> + break;
> + case SCMI_ARM_SMCCC_TRANSPORT:
> + if (IS_ENABLED(CONFIG_ARM_SMCCC))
> + return probe_arm_smc_channel(dev);
> + break;
> + default:
> + break;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int scmi_bind(struct udevice *dev) {
> + int rc = 0;
> + ofnode node;
> + struct driver *drv;
> +
> + dev_for_each_subnode(node, dev) {
> + u32 protocol_id;
> +
> + if (!ofnode_is_available(node))
> + continue;
> +
> + if (ofnode_read_u32(node, "reg", &protocol_id))
> + continue;
> +
> + switch (protocol_id) {
> + default:
> + dev_info(dev, "Ignore unsupported SCMI protocol %u\n",
> + protocol_id);
> + continue;
> + }
> +
> + rc = device_bind_ofnode(dev, drv, ofnode_get_name(node),
> + NULL, node, NULL);
> + if (rc)
> + break;
> + }
> +
> + if (rc)
> + device_unbind(dev);
> +
> + return rc;
> +}
> +
> +static const struct udevice_id scmi_ids[] = { #ifdef CONFIG_DM_MAILBOX
> + { .compatible = "arm,scmi", .data = SCMI_MAILBOX_TRANSPORT },
> #endif
> +#ifdef CONFIG_ARM_SMCCC
> + { .compatible = "arm,scmi-smc", .data =
> SCMI_ARM_SMCCC_TRANSPORT },
> +#endif
To minimize the code base, how about following Linux kernel to split mailbox
and smc to different files?
Regards,
Peng.
> + { }
> +};
> +
> +U_BOOT_DRIVER(scmi) = {
> + .name = "scmi",
> + .id = UCLASS_FIRMWARE,
> + .of_match = scmi_ids,
> + .priv_auto_alloc_size = sizeof(struct scmi_agent),
> + .bind = scmi_bind,
> + .probe = scmi_probe,
> + .remove = scmi_remove,
> + .flags = DM_FLAG_OS_PREPARE,
> +};
> diff --git a/include/scmi.h b/include/scmi.h new file mode 100644 index
> 00000000000..e12e322991d
> --- /dev/null
> +++ b/include/scmi.h
> @@ -0,0 +1,82 @@
> +/* SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause */
> +/*
> + * Copyright (c) 2015-2019, Arm Limited and Contributors. All rights
> reserved.
> + * Copyright (C) 2019, Linaro Limited
> + */
> +#ifndef SCMI_H
> +#define SCMI_H
> +
> +#include <asm/types.h>
> +
> +/**
> + * An SCMI agent represent on communication path from a device driver
> +to
> + * the remote SCMI server which driver sends messages to and receives
> + * response messages from.
> + */
> +struct scmi_agent;
> +
> +enum scmi_std_protocol {
> + SCMI_PROTOCOL_ID_BASE = 0x10,
> + SCMI_PROTOCOL_ID_POWER_DOMAIN = 0x11,
> + SCMI_PROTOCOL_ID_SYSTEM = 0x12,
> + SCMI_PROTOCOL_ID_PERF = 0x13,
> + SCMI_PROTOCOL_ID_CLOCK = 0x14,
> + SCMI_PROTOCOL_ID_SENSOR = 0x15,
> + SCMI_PROTOCOL_ID_RESET_DOMAIN = 0x16,
> +};
> +
> +enum scmi_status_code {
> + SCMI_SUCCESS = 0,
> + SCMI_NOT_SUPPORTED = -1,
> + SCMI_INVALID_PARAMETERS = -2,
> + SCMI_DENIED = -3,
> + SCMI_NOT_FOUND = -4,
> + SCMI_OUT_OF_RANGE = -5,
> + SCMI_BUSY = -6,
> + SCMI_COMMS_ERROR = -7,
> + SCMI_GENERIC_ERROR = -8,
> + SCMI_HARDWARE_ERROR = -9,
> + SCMI_PROTOCOL_ERROR = -10,
> +};
> +
> +/*
> + * struct scmi_msg - Context of a SCMI message sent and the response
> +received
> + *
> + * @protocol_id: SCMI protocol ID
> + * @message_id: SCMI message ID for a defined protocol ID
> + * @in_msg: pointer to the message payload sent by the driver
> + * @in_msg_sz: byte size of the message payload sent
> + * @out_msg: pointer to buffer to store response message payload
> + * @out_msg_size: Byte size of the response buffer or payload */
> +struct scmi_msg {
> + unsigned int protocol_id;
> + unsigned int message_id;
> + u8 *in_msg;
> + size_t in_msg_sz;
> + u8 *out_msg;
> + size_t out_msg_sz;
> +};
> +
> +/**
> + * scmi_send_and_process_msg() - send and process a SCMI message
> + *
> + * Send a message to a SCMI server through a target SCMI agent device.
> + * Caller sets scmi_msg::out_msg_sz to the output message buffer size.
> + * On return, scmi_msg::out_msg_sz stores the response payload size.
> + *
> + * @dev: SCMI agent device
> + * @msg: Message structure reference
> + * @return 0 on success, a negative errno otherwise */ int
> +scmi_send_and_process_msg(struct udevice *dev, struct scmi_msg *msg);
> +
> +/**
> + * scmi_to_linux_errno() - Convert an SCMI error code into a Linux
> +errno code
> + *
> + * @scmi_errno: SCMI error code value
> + * @return 0 for successful status and a negative errno otherwise */
> +int scmi_to_linux_errno(s32 scmi_errno);
> +
> +#endif /* SCMI_H */
> --
> 2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/4] dt-bindings: arm: SCMI bindings documentation
[not found] ` <20200717153731.10643-2-etienne.carriere@linaro.org>
@ 2020-07-20 2:01 ` Peng Fan
2020-07-24 9:47 ` Etienne Carriere
0 siblings, 1 reply; 14+ messages in thread
From: Peng Fan @ 2020-07-20 2:01 UTC (permalink / raw)
To: u-boot
> Subject: [PATCH 2/4] dt-bindings: arm: SCMI bindings documentation
Since kernel already has the binding doc, there is no need to add a U-Boot
copy.
Regards,
Peng.
>
> Dump SCMI DT bindings documentation from Linux kernel source tree
> v5.8-rc1.
>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
>
> doc/device-tree-bindings/arm/arm,scmi.txt | 197
> ++++++++++++++++++++++
> 1 file changed, 197 insertions(+)
> create mode 100644 doc/device-tree-bindings/arm/arm,scmi.txt
>
> diff --git a/doc/device-tree-bindings/arm/arm,scmi.txt
> b/doc/device-tree-bindings/arm/arm,scmi.txt
> new file mode 100644
> index 00000000000..1f293ea24cd
> --- /dev/null
> +++ b/doc/device-tree-bindings/arm/arm,scmi.txt
> @@ -0,0 +1,197 @@
> +System Control and Management Interface (SCMI) Message Protocol
> +----------------------------------------------------------
> +
> +The SCMI is intended to allow agents such as OSPM to manage various
> +functions that are provided by the hardware platform it is running on,
> +including power and performance functions.
> +
> +This binding is intended to define the interface the firmware
> +implementing the SCMI as described in ARM document number ARM DEN
> 0056A
> +("ARM System Control and Management Interface Platform Design
> +Document")[0] provide for OSPM in the device tree.
> +
> +Required properties:
> +
> +The scmi node with the following properties shall be under the /firmware/
> node.
> +
> +- compatible : shall be "arm,scmi" or "arm,scmi-smc" for smc/hvc
> +transports
> +- mboxes: List of phandle and mailbox channel specifiers. It should contain
> + exactly one or two mailboxes, one for transmitting messages("tx")
> + and another optional for receiving the notifications("rx") if
> + supported.
> +- shmem : List of phandle pointing to the shared memory(SHM) area as per
> + generic mailbox client binding.
> +- #address-cells : should be '1' if the device has sub-nodes, maps to
> + protocol identifier for a given sub-node.
> +- #size-cells : should be '0' as 'reg' property doesn't have any size
> + associated with it.
> +- arm,smc-id : SMC id required when using smc or hvc transports
> +
> +Optional properties:
> +
> +- mbox-names: shall be "tx" or "rx" depending on mboxes entries.
> +
> +See Documentation/devicetree/bindings/mailbox/mailbox.txt for more
> +details about the generic mailbox controller and client driver bindings.
> +
> +The mailbox is the only permitted method of calling the SCMI firmware.
> +Mailbox doorbell is used as a mechanism to alert the presence of a
> +messages and/or notification.
> +
> +Each protocol supported shall have a sub-node with corresponding
> +compatible as described in the following sections. If the platform
> +supports dedicated communication channel for a particular protocol, the 3
> properties namely:
> +mboxes, mbox-names and shmem shall be present in the sub-node
> +corresponding to that protocol.
> +
> +Clock/Performance bindings for the clocks/OPPs based on SCMI Message
> +Protocol
> +------------------------------------------------------------
> +
> +This binding uses the common clock binding[1].
> +
> +Required properties:
> +- #clock-cells : Should be 1. Contains the Clock ID value used by SCMI
> commands.
> +
> +Power domain bindings for the power domains based on SCMI Message
> +Protocol
> +------------------------------------------------------------
> +
> +This binding for the SCMI power domain providers uses the generic power
> +domain binding[2].
> +
> +Required properties:
> + - #power-domain-cells : Should be 1. Contains the device or the power
> + domain ID value used by SCMI commands.
> +
> +Sensor bindings for the sensors based on SCMI Message Protocol
> +--------------------------------------------------------------
> +SCMI provides an API to access the various sensors on the SoC.
> +
> +Required properties:
> +- #thermal-sensor-cells: should be set to 1. This property follows the
> + thermal device tree bindings[3].
> +
> + Valid cell values are raw identifiers (Sensor ID)
> + as used by the firmware. Refer to platform details
> + for your implementation for the IDs to use.
> +
> +Reset signal bindings for the reset domains based on SCMI Message
> +Protocol
> +------------------------------------------------------------
> +
> +This binding for the SCMI reset domain providers uses the generic reset
> +signal binding[5].
> +
> +Required properties:
> + - #reset-cells : Should be 1. Contains the reset domain ID value used
> + by SCMI commands.
> +
> +SRAM and Shared Memory for SCMI
> +-------------------------------
> +
> +A small area of SRAM is reserved for SCMI communication between
> +application processors and SCP.
> +
> +The properties should follow the generic mmio-sram description found in
> +[4]
> +
> +Each sub-node represents the reserved area for SCMI.
> +
> +Required sub-node properties:
> +- reg : The base offset and size of the reserved area with the SRAM
> +- compatible : should be "arm,scmi-shmem" for Non-secure SRAM based
> + shared memory
> +
> +[0]
> +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Finfoce
> +nter.arm.com%2Fhelp%2Ftopic%2Fcom.arm.doc.den0056a%2Findex.html&
> amp;dat
> +a=02%7C01%7Cpeng.fan%40nxp.com%7C94dcac2bc02346684bec08d82a67
> 76b9%7C686
> +ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637305971145926287&a
> mp;sdata=gS
> +HhNqZ14iDSv0ws76jZox0JOzAL7Yrg%2BV%2ByH1Szkj4%3D&reserved=
> 0
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +[2] Documentation/devicetree/bindings/power/power-domain.yaml
> +[3] Documentation/devicetree/bindings/thermal/thermal.txt
> +[4] Documentation/devicetree/bindings/sram/sram.yaml
> +[5] Documentation/devicetree/bindings/reset/reset.txt
> +
> +Example:
> +
> +sram at 50000000 {
> + compatible = "mmio-sram";
> + reg = <0x0 0x50000000 0x0 0x10000>;
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0x0 0x50000000 0x10000>;
> +
> + cpu_scp_lpri: scp-shmem at 0 {
> + compatible = "arm,scmi-shmem";
> + reg = <0x0 0x200>;
> + };
> +
> + cpu_scp_hpri: scp-shmem at 200 {
> + compatible = "arm,scmi-shmem";
> + reg = <0x200 0x200>;
> + };
> +};
> +
> +mailbox at 40000000 {
> + ....
> + #mbox-cells = <1>;
> + reg = <0x0 0x40000000 0x0 0x10000>;
> +};
> +
> +firmware {
> +
> + ...
> +
> + scmi {
> + compatible = "arm,scmi";
> + mboxes = <&mailbox 0 &mailbox 1>;
> + mbox-names = "tx", "rx";
> + shmem = <&cpu_scp_lpri &cpu_scp_hpri>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + scmi_devpd: protocol at 11 {
> + reg = <0x11>;
> + #power-domain-cells = <1>;
> + };
> +
> + scmi_dvfs: protocol at 13 {
> + reg = <0x13>;
> + #clock-cells = <1>;
> + };
> +
> + scmi_clk: protocol at 14 {
> + reg = <0x14>;
> + #clock-cells = <1>;
> + };
> +
> + scmi_sensors0: protocol at 15 {
> + reg = <0x15>;
> + #thermal-sensor-cells = <1>;
> + };
> +
> + scmi_reset: protocol at 16 {
> + reg = <0x16>;
> + #reset-cells = <1>;
> + };
> + };
> +};
> +
> +cpu at 0 {
> + ...
> + reg = <0 0>;
> + clocks = <&scmi_dvfs 0>;
> +};
> +
> +hdlcd at 7ff60000 {
> + ...
> + reg = <0 0x7ff60000 0 0x1000>;
> + clocks = <&scmi_clk 4>;
> + power-domains = <&scmi_devpd 1>;
> + resets = <&scmi_reset 10>;
> +};
> +
> +thermal-zones {
> + soc_thermal {
> + polling-delay-passive = <100>;
> + polling-delay = <1000>;
> + /* sensor ID */
> + thermal-sensors = <&scmi_sensors0 3>;
> + ...
> + };
> +};
> --
> 2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/4] clk: add clock driver for SCMI agents
[not found] ` <20200717153731.10643-3-etienne.carriere@linaro.org>
@ 2020-07-20 2:06 ` Peng Fan
2020-07-24 9:54 ` Etienne Carriere
0 siblings, 1 reply; 14+ messages in thread
From: Peng Fan @ 2020-07-20 2:06 UTC (permalink / raw)
To: u-boot
> Subject: [PATCH 3/4] clk: add clock driver for SCMI agents
>
> This change introduces a clock driver for SCMI agent devices. When SCMI
> agent and SCMI clock drivers are enabled, SCMI agent binds a clock device for
> each SCMI clock protocol devices enabled in the FDT.
>
> SCMI clock driver is embedded upon CONFIG_CLK_SCMI=y. If enabled,
> CONFIG_SCMI_AGENT is also enabled.
>
> SCMI Clock protocol is defined in the SCMI specification [1].
>
> Links: [1]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdevelo
> per.arm.com%2Farchitectures%2Fsystem-architectures%2Fsoftware-standar
> ds%2Fscmi&data=02%7C01%7Cpeng.fan%40nxp.com%7C6a1c6d1e102
> 94e81e47708d82a6777b9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%
> 7C0%7C637305971176347870&sdata=4pKZGL17y7QUY%2Bi9XRutLOCe
> 5jBs1y9gMW7vNoUIcC0%3D&reserved=0
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
>
> drivers/clk/Kconfig | 8 +++
> drivers/clk/Makefile | 1 +
> drivers/clk/clk_scmi.c | 152
> ++++++++++++++++++++++++++++++++++++++++
> drivers/firmware/scmi.c | 3 +
> 4 files changed, 164 insertions(+)
> create mode 100644 drivers/clk/clk_scmi.c
>
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index
> 82cb1874e19..234d6035202 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -152,6 +152,14 @@ config CLK_CDCE9XX
> Enable the clock synthesizer driver for CDCE913/925/937/949
> series of chips.
>
> +config CLK_SCMI
> + bool "Enable SCMI clock driver"
> + select SCMI_FIRMWARE
> + help
> + Enable this option if you want to support clock devices exposed
> + by a SCMI agent based on SCMI clock protocol communication
> + with a SCMI server.
> +
> source "drivers/clk/analogbits/Kconfig"
> source "drivers/clk/at91/Kconfig"
> source "drivers/clk/exynos/Kconfig"
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index
> d9119545810..76bba77d1f0 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_CLK_K210) += kendryte/
> obj-$(CONFIG_CLK_MPC83XX) += mpc83xx_clk.o
> obj-$(CONFIG_CLK_OWL) += owl/
> obj-$(CONFIG_CLK_RENESAS) += renesas/
> +obj-$(CONFIG_CLK_SCMI) += clk_scmi.o
> obj-$(CONFIG_CLK_SIFIVE) += sifive/
> obj-$(CONFIG_ARCH_SUNXI) += sunxi/
> obj-$(CONFIG_CLK_STM32F) += clk_stm32f.o diff --git
> a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c new file mode 100644 index
> 00000000000..efe64a6a38f
> --- /dev/null
> +++ b/drivers/clk/clk_scmi.c
> @@ -0,0 +1,152 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019-2020 Linaro Limited */ #include <common.h>
> +#include <clk-uclass.h> #include <dm.h> #include <scmi.h> #include
> +<asm/types.h>
> +
> +enum scmi_clock_message_id {
> + SCMI_CLOCK_RATE_SET = 0x5,
> + SCMI_CLOCK_RATE_GET = 0x6,
> + SCMI_CLOCK_CONFIG_SET = 0x7,
> +};
> +
> +#define SCMI_CLK_RATE_ASYNC_NOTIFY BIT(0)
> +#define SCMI_CLK_RATE_ASYNC_NORESP (BIT(0) | BIT(1))
> +#define SCMI_CLK_RATE_ROUND_DOWN 0
> +#define SCMI_CLK_RATE_ROUND_UP BIT(2)
> +#define SCMI_CLK_RATE_ROUND_CLOSEST BIT(3)
> +
> +struct scmi_clk_state_in {
> + u32 clock_id;
> + u32 attributes;
> +};
> +
> +struct scmi_clk_state_out {
> + s32 status;
> +};
> +
> +static int scmi_clk_gate(struct clk *clk, int enable) {
> + struct scmi_clk_state_in in = {
> + .clock_id = clk->id,
> + .attributes = enable,
> + };
> + struct scmi_clk_state_out out;
> + struct scmi_msg scmi_msg = {
> + .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
> + .message_id = SCMI_CLOCK_CONFIG_SET,
> + .in_msg = (u8 *)&in,
> + .in_msg_sz = sizeof(in),
> + .out_msg = (u8 *)&out,
> + .out_msg_sz = sizeof(out),
> + };
> + int rc;
> +
> + rc = scmi_send_and_process_msg(clk->dev->parent, &scmi_msg);
> + if (rc)
> + return rc;
> +
> + return scmi_to_linux_errno(out.status); }
> +
> +static int scmi_clk_enable(struct clk *clk) {
> + return scmi_clk_gate(clk, 1);
> +}
> +
> +static int scmi_clk_disable(struct clk *clk) {
> + return scmi_clk_gate(clk, 0);
> +}
> +
> +struct scmi_clk_rate_get_in {
> + u32 clock_id;
> +};
> +
> +struct scmi_clk_rate_get_out {
> + s32 status;
> + u32 rate_lsb;
> + u32 rate_msb;
> +};
> +
> +static ulong scmi_clk_get_rate(struct clk *clk) {
> + struct scmi_clk_rate_get_in in = {
> + .clock_id = clk->id,
> + };
> + struct scmi_clk_rate_get_out out;
> + struct scmi_msg scmi_msg = {
> + .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
> + .message_id = SCMI_CLOCK_RATE_GET,
> + .in_msg = (u8 *)&in,
> + .in_msg_sz = sizeof(in),
> + .out_msg = (u8 *)&out,
> + .out_msg_sz = sizeof(out),
> + };
> + int rc;
> +
> + rc = scmi_send_and_process_msg(clk->dev->parent, &scmi_msg);
> + if (rc)
> + return 0;
> +
> + rc = scmi_to_linux_errno(out.status);
> + if (rc)
> + return 0;
> +
> + return (ulong)(((u64)out.rate_msb << 32) | out.rate_lsb); }
> +
> +struct scmi_clk_rate_set_in {
> + u32 clock_id;
> + u32 flags;
> + u32 rate_lsb;
> + u32 rate_msb;
> +};
> +
> +struct scmi_clk_rate_set_out {
> + s32 status;
> +};
> +
> +static ulong scmi_clk_set_rate(struct clk *clk, ulong rate) {
> + struct scmi_clk_rate_set_in in = {
> + .clock_id = clk->id,
> + .flags = SCMI_CLK_RATE_ASYNC_NORESP |
This will use async, but how if sync?
Thanks,
Peng.
> + SCMI_CLK_RATE_ROUND_CLOSEST,
> + .rate_lsb = (u32)rate,
> + .rate_msb = (u32)((u64)rate >> 32),
> + };
> + struct scmi_clk_rate_set_out out;
> + struct scmi_msg scmi_msg = {
> + .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
> + .message_id = SCMI_CLOCK_RATE_SET,
> + .in_msg = (u8 *)&in,
> + .in_msg_sz = sizeof(in),
> + .out_msg = (u8 *)&out,
> + .out_msg_sz = sizeof(out),
> + };
> + int rc;
> +
> + rc = scmi_send_and_process_msg(clk->dev->parent, &scmi_msg);
> + if (rc)
> + return 0;
> +
> + return scmi_to_linux_errno(out.status); }
> +
> +static const struct clk_ops scmi_clk_ops = {
> + .enable = scmi_clk_enable,
> + .disable = scmi_clk_disable,
> + .get_rate = scmi_clk_get_rate,
> + .set_rate = scmi_clk_set_rate,
> +};
> +
> +U_BOOT_DRIVER(scmi_clock) = {
> + .name = "scmi_clk",
> + .id = UCLASS_CLK,
> + .ops = &scmi_clk_ops,
> +};
> diff --git a/drivers/firmware/scmi.c b/drivers/firmware/scmi.c index
> fa8a91c3f3d..9f06718df51 100644
> --- a/drivers/firmware/scmi.c
> +++ b/drivers/firmware/scmi.c
> @@ -399,6 +399,9 @@ static int scmi_bind(struct udevice *dev)
> continue;
>
> switch (protocol_id) {
> + case SCMI_PROTOCOL_ID_CLOCK:
> + drv = DM_GET_DRIVER(scmi_clock);
> + break;
> default:
> dev_info(dev, "Ignore unsupported SCMI protocol %u\n",
> protocol_id);
> --
> 2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/4] reset: add reset controller driver for SCMI agents
[not found] ` <20200717153731.10643-4-etienne.carriere@linaro.org>
@ 2020-07-20 2:07 ` Peng Fan
0 siblings, 0 replies; 14+ messages in thread
From: Peng Fan @ 2020-07-20 2:07 UTC (permalink / raw)
To: u-boot
> Subject: [PATCH 4/4] reset: add reset controller driver for SCMI agents
>
> This change introduces a reset controller driver for SCMI agent devices.
> When SCMI agent and SCMI reset domain drivers are enabled, SCMI agent
> binds a reset controller device for each SCMI reset domain protocol devices
> enabled in the FDT.
>
> SCMI reset driver is embedded upon CONFIG_RESET_SCMI=y. If enabled,
> CONFIG_SCMI_AGENT is also enabled.
>
> SCMI Reset Domain protocol is defined in the SCMI specification [1].
>
> Links: [1]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdevelo
> per.arm.com%2Farchitectures%2Fsystem-architectures%2Fsoftware-standar
> ds%2Fscmi&data=02%7C01%7Cpeng.fan%40nxp.com%7C68ec2a92529
> e49d46be608d82a6778a9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%
> 7C0%7C637305971181000613&sdata=NWnJw%2BYBsbC1zBkBdq9Jxh9
> Crepk5JdxJlCxNl0diqA%3D&reserved=0
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
>
> drivers/firmware/scmi.c | 3 ++
> drivers/reset/Kconfig | 8 ++++
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-scmi.c | 86
> ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 98 insertions(+)
> create mode 100644 drivers/reset/reset-scmi.c
>
> diff --git a/drivers/firmware/scmi.c b/drivers/firmware/scmi.c index
> 9f06718df51..9be53a9cf11 100644
> --- a/drivers/firmware/scmi.c
> +++ b/drivers/firmware/scmi.c
> @@ -402,6 +402,9 @@ static int scmi_bind(struct udevice *dev)
> case SCMI_PROTOCOL_ID_CLOCK:
> drv = DM_GET_DRIVER(scmi_clock);
> break;
> + case SCMI_PROTOCOL_ID_RESET_DOMAIN:
> + drv = DM_GET_DRIVER(scmi_reset_domain);
> + break;
> default:
> dev_info(dev, "Ignore unsupported SCMI protocol %u\n",
> protocol_id);
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig index
> 6d535612234..31bd4cd5b45 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -164,4 +164,12 @@ config RESET_RASPBERRYPI
> relevant. This driver provides a reset controller capable of
> interfacing with RPi4's co-processor and model these firmware
> initialization routines as reset lines.
> +
> +config RESET_SCMI
> + bool "Enable SCMI reset domain driver"
> + select SCMI_FIRMWARE
> + help
> + Enable this option if you want to support reset controller
> + devices exposed by a SCMI agent based on SCMI reset domain
> + protocol communication with a SCMI server.
> endmenu
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile index
> 8e0124b8dee..f3c0fbfd8f3 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -25,3 +25,4 @@ obj-$(CONFIG_RESET_HISILICON) += reset-hisilicon.o
> obj-$(CONFIG_RESET_IMX7) += reset-imx7.o
> obj-$(CONFIG_RESET_SYSCON) += reset-syscon.o
> obj-$(CONFIG_RESET_RASPBERRYPI) += reset-raspberrypi.o
> +obj-$(CONFIG_RESET_SCMI) += reset-scmi.o
> diff --git a/drivers/reset/reset-scmi.c b/drivers/reset/reset-scmi.c new file
> mode 100644 index 00000000000..e664d91d865
> --- /dev/null
> +++ b/drivers/reset/reset-scmi.c
> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019-2020 Linaro Limited */ #include <common.h>
> +#include <dm.h> #include <errno.h> #include <reset-uclass.h> #include
> +<scmi.h> #include <asm/types.h>
> +
> +enum scmi_reset_domain_message_id {
> + SCMI_RESET_DOMAIN_RESET = 0x4,
> +};
> +
> +#define SCMI_RD_RESET_FLAG_ASSERT BIT(1)
> +#define SCMI_RD_RESET_FLAG_DEASSERT 0
> +
> +struct scmi_rd_reset_in {
> + u32 domain_id;
> + u32 flags;
> + u32 reset_state;
> +};
> +
> +struct scmi_rd_reset_out {
> + s32 status;
> +};
> +
> +static int scmi_reset_set_state(struct reset_ctl *rst, int
> +assert_not_deassert) {
> + struct scmi_rd_reset_in in = {
> + .domain_id = rst->id,
> + .flags = assert_not_deassert ? SCMI_RD_RESET_FLAG_ASSERT :
> + SCMI_RD_RESET_FLAG_DEASSERT,
> + .reset_state = 0,
> + };
> + struct scmi_rd_reset_out out;
> + struct scmi_msg scmi_msg = {
> + .protocol_id = SCMI_PROTOCOL_ID_RESET_DOMAIN,
> + .message_id = SCMI_RESET_DOMAIN_RESET,
> + .in_msg = (u8 *)&in,
> + .in_msg_sz = sizeof(in),
> + .out_msg = (u8 *)&out,
> + .out_msg_sz = sizeof(out),
> + };
> + int rc;
> +
> + rc = scmi_send_and_process_msg(rst->dev->parent, &scmi_msg);
> + if (rc)
> + return rc;
> +
> + return scmi_to_linux_errno(out.status); }
> +
> +static int scmi_reset_assert(struct reset_ctl *rst) {
> + return scmi_reset_set_state(rst, SCMI_RD_RESET_FLAG_ASSERT); }
> +
> +static int scmi_reset_deassert(struct reset_ctl *rst) {
> + return scmi_reset_set_state(rst, SCMI_RD_RESET_FLAG_DEASSERT); }
> +
> +static int scmi_reset_request(struct reset_ctl *reset_ctl) {
> + return 0;
> +}
> +
> +static int scmi_reset_rfree(struct reset_ctl *reset_ctl) {
> + return 0;
> +}
> +
> +static const struct reset_ops scmi_reset_domain_ops = {
> + .request = scmi_reset_request,
> + .rfree = scmi_reset_rfree,
> + .rst_assert = scmi_reset_assert,
> + .rst_deassert = scmi_reset_deassert,
> +};
> +
> +U_BOOT_DRIVER(scmi_reset_domain) = {
> + .name = "scmi_reset_domain",
> + .id = UCLASS_RESET,
> + .ops = &scmi_reset_domain_ops,
> +};
> --
Regards,
Peng.
> 2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/4] firmware: add new driver for SCMI firmwares
2020-07-20 2:01 ` [PATCH 1/4] firmware: add new driver for SCMI firmwares Peng Fan
@ 2020-07-24 9:46 ` Etienne Carriere
0 siblings, 0 replies; 14+ messages in thread
From: Etienne Carriere @ 2020-07-24 9:46 UTC (permalink / raw)
To: u-boot
Hell Peng,
Thanks for the comments. Please find below few questions for some of
your comments.
On Mon, 20 Jul 2020 at 04:01, Peng Fan <peng.fan@nxp.com> wrote:
>
> Hi Etienne,
>
> +Sudeep
>
> > Subject: [PATCH 1/4] firmware: add new driver for SCMI firmwares
>
> Thanks for posting this out.
>
> >
> > This change introduces SCMI agent driver in U-Boot in the firmware U-class.
> >
> > SCMI agent driver is designed for platforms that embed a SCMI server in a
> > firmware hosted for example by a companion co-processor or the secure
> > world of the executing processor.
> >
> > SCMI protocols allow an SCMI agent to discover and access external
> > resources as clock, reset controllers and many more. SCMI agent and server
> > communicate following the SCMI specification [1]. SCMI agent complies with
> > the DT bindings defined in the Linux kernel source tree regarding SCMI agent
> > description since v5.8-rc1.
> >
> > These bindings describe 2 supported message transport layer: using mailbox
> > uclass devices or using Arm SMC invocation instruction. Both use a piece or
> > shared memory for message data exchange.
> >
> > In the current state, the SCMI agent driver does not bind to any SCMI protocol
> > to a U-Boot device driver. Former changes will implement dedicated driver (i.e.
> > an SCMI clock driver or an SCMI reset controller
> > driver) and add bind supported SCMI protocols in scmi_agent_bind().
> >
> > Links: [1]
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdevelo
> > per.arm.com%2Farchitectures%2Fsystem-architectures%2Fsoftware-standar
> > ds%2Fscmi&data=02%7C01%7Cpeng.fan%40nxp.com%7C39c55064be5
> > 04bf248a708d82a6775cd%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
> > C0%7C637305971142916174&sdata=5kCgz3kzzk4qHy598u79zz3hV17yV
> > zdPxM531sOAnUs%3D&reserved=0
> > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> > ---
> >
> > drivers/firmware/Kconfig | 15 ++
> > drivers/firmware/Makefile | 1 +
> > drivers/firmware/scmi.c | 439
> > ++++++++++++++++++++++++++++++++++++++
> > include/scmi.h | 82 +++++++
> > 4 files changed, 537 insertions(+)
> > create mode 100644 drivers/firmware/scmi.c create mode 100644
> > include/scmi.h
> >
> > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index
> > b70a2063551..f7c7ee7a5aa 100644
> > --- a/drivers/firmware/Kconfig
> > +++ b/drivers/firmware/Kconfig
> > @@ -1,6 +1,21 @@
> > config FIRMWARE
> > bool "Enable Firmware driver support"
> >
> > +config SCMI_FIRMWARE
> > + bool "Enable SCMI support"
> > + select FIRMWARE
> > + select OF_TRANSLATE
> > + depends on DM_MAILBOX || ARM_SMCCC
> > + help
> > + An SCMI agent communicates with a related SCMI server firmware
> > + located in another sub-system, as a companion micro controller
> > + or a companion host in the CPU system.
> > +
> > + Communications between agent (client) and the SCMI server are
> > + based on message exchange. Messages can be exchange over tranport
> > + channels as a mailbox device or an Arm SMCCC service with some
> > + piece of identified shared memory.
> > +
> > config SPL_FIRMWARE
> > bool "Enable Firmware driver support in SPL"
> > depends on FIRMWARE
> > diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile index
> > a0c250a473e..3965838179f 100644
> > --- a/drivers/firmware/Makefile
> > +++ b/drivers/firmware/Makefile
> > @@ -2,4 +2,5 @@ obj-$(CONFIG_FIRMWARE) += firmware-uclass.o
> > obj-$(CONFIG_$(SPL_)ARM_PSCI_FW) += psci.o
> > obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o
> > obj-$(CONFIG_SANDBOX) += firmware-sandbox.o
> > +obj-$(CONFIG_SCMI_FIRMWARE) += scmi.o
> > obj-$(CONFIG_ZYNQMP_FIRMWARE) += firmware-zynqmp.o
> > diff --git a/drivers/firmware/scmi.c b/drivers/firmware/scmi.c new file mode
> > 100644 index 00000000000..fa8a91c3f3d
> > --- /dev/null
> > +++ b/drivers/firmware/scmi.c
> > @@ -0,0 +1,439 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2015-2019, Arm Limited and Contributors. All rights
> > reserved.
> > + * Copyright (C) 2019-2020 Linaro Limited.
> > + */
> > +
> > +#include <common.h>
> > +#include <cpu_func.h>
> > +#include <dm.h>
> > +#include <errno.h>
> > +#include <mailbox.h>
> > +#include <memalign.h>
> > +#include <scmi.h>
> > +#include <asm/system.h>
> > +#include <asm/types.h>
> > +#include <dm/device-internal.h>
> > +#include <dm/devres.h>
> > +#include <dm/lists.h>
> > +#include <dm/ofnode.h>
> > +#include <linux/arm-smccc.h>
> > +#include <linux/compat.h>
> > +#include <linux/errno.h>
> > +#include <linux/io.h>
> > +#include <linux/ioport.h>
> > +
> > +#define TIMEOUT_US_10MS 10000
> > +
> > +struct error_code {
> > + int scmi;
> > + int errno;
> > +};
> > +
> > +static const struct error_code scmi_linux_errmap[] = {
> > + { .scmi = SCMI_NOT_SUPPORTED, .errno = -EOPNOTSUPP, },
> > + { .scmi = SCMI_INVALID_PARAMETERS, .errno = -EINVAL, },
> > + { .scmi = SCMI_DENIED, .errno = -EACCES, },
> > + { .scmi = SCMI_NOT_FOUND, .errno = -ENOENT, },
> > + { .scmi = SCMI_OUT_OF_RANGE, .errno = -ERANGE, },
> > + { .scmi = SCMI_BUSY, .errno = -EBUSY, },
> > + { .scmi = SCMI_COMMS_ERROR, .errno = -ECOMM, },
> > + { .scmi = SCMI_GENERIC_ERROR, .errno = -EIO, },
> > + { .scmi = SCMI_HARDWARE_ERROR, .errno = -EREMOTEIO, },
> > + { .scmi = SCMI_PROTOCOL_ERROR, .errno = -EPROTO, }, };
> > +
> > +int scmi_to_linux_errno(s32 scmi_code)
> > +{
> > + int n;
> > +
> > + if (scmi_code == 0)
>
> !scmi_code
>
> > + return 0;
> > +
> > + for (n = 0; n < ARRAY_SIZE(scmi_linux_errmap); n++)
> > + if (scmi_code == scmi_linux_errmap[n].scmi)
> > + return scmi_linux_errmap[1].errno;
> > +
> > + return -EPROTO;
> > +}
> > +
> > +struct method_ops {
> > + int (*process_msg)(struct udevice *dev, struct scmi_msg *msg);
> > + int (*remove_agent)(struct udevice *dev); };
> > +
> > +struct scmi_agent {
> > + struct method_ops *method_ops;
> > + void *method_priv;
> > +};
> > +
> > +/*
> > + * Shared Memory based Transport (SMT) message buffer management
> > + *
> > + * SMT uses 28 byte header prior message payload to handle the state of
> > + * the communication channel realized by the shared memory area and
> > + * to define SCMI protocol information the payload relates to.
> > + */
> > +struct scmi_smt_header {
> > + __le32 reserved;
> > + __le32 channel_status;
> > +#define SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR BIT(1)
> > +#define SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE BIT(0)
> > + __le32 reserved1[2];
> > + __le32 flags;
> > +#define SCMI_SHMEM_FLAG_INTR_ENABLED BIT(0)
> > + __le32 length;
> > + __le32 msg_header;
> > + u8 msg_payload[0];
> > +};
> > +
> > +#define SMT_HEADER_TOKEN(token) (((token) << 18) & GENMASK(31,
> > 18))
> > +#define SMT_HEADER_PROTOCOL_ID(proto) (((proto) << 10) &
> > GENMASK(17, 10))
> > +#define SMT_HEADER_MESSAGE_TYPE(type) (((type) << 18) & GENMASK(9,
> > 8))
> > +#define SMT_HEADER_MESSAGE_ID(id) ((id) & GENMASK(7, 0))
> > +
> > +struct scmi_shm_buf {
> > + u8 *buf;
> > + size_t size;
> > +};
> > +
> > +static int get_shm_buffer(struct udevice *dev, struct scmi_shm_buf
> > +*shm) {
> > + int rc;
> > + struct ofnode_phandle_args args;
> > + struct resource resource;
> > + fdt32_t faddr;
> > + phys_addr_t paddr;
> > +
> > + rc = dev_read_phandle_with_args(dev, "shmem", NULL, 0, 0, &args);
> > + if (rc)
> > + return rc;
> > +
> > + rc = ofnode_read_resource(args.node, 0, &resource);
> > + if (rc)
> > + return rc;
> > +
> > + faddr = cpu_to_fdt32(resource.start);
> > + paddr = ofnode_translate_address(args.node, &faddr);
> > +
> > + shm->size = resource_size(&resource);
> > + if (shm->size < sizeof(struct scmi_smt_header)) {
> > + dev_err(dev, "Shared memory buffer too small\n");
> > + return -EINVAL;
> > + }
> > +
> > + shm->buf = devm_ioremap(dev, paddr, shm->size);
> > + if (!shm->buf)
> > + return -ENOMEM;
> > +
> > + if (dcache_status())
> > + mmu_set_region_dcache_behaviour((uintptr_t)shm->buf,
> > + shm->size, DCACHE_OFF);
>
>
> Could dev_read_addr be used to replace the upper code block?
I'll check that but i'm not sure it will simplify.
>
> > +
> > + return 0;
> > +}
> > +
> > +static int write_msg_to_smt(struct udevice *dev, struct scmi_shm_buf
> > *shm_buf,
> > + struct scmi_msg *msg)
> > +{
> > + struct scmi_smt_header *hdr = (void *)shm_buf->buf;
> > +
> > + if ((!msg->in_msg && msg->in_msg_sz) ||
> > + (!msg->out_msg && msg->out_msg_sz))
> > + return -EINVAL;
> > +
> > + if (!(hdr->channel_status &
> > SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE)) {
> > + dev_dbg(dev, "Channel busy\n");
> > + return -EBUSY;
> > + }
> > +
> > + if (shm_buf->size < (sizeof(*hdr) + msg->in_msg_sz) ||
> > + shm_buf->size < (sizeof(*hdr) + msg->out_msg_sz)) {
> > + dev_dbg(dev, "Buffer too small\n");
> > + return -ETOOSMALL;
> > + }
> > +
> > + /* Load message in shared memory */
> > + hdr->channel_status &= ~SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE;
> > + hdr->length = msg->in_msg_sz + sizeof(hdr->msg_header);
> > + hdr->msg_header = SMT_HEADER_TOKEN(0) |
> > + SMT_HEADER_MESSAGE_TYPE(0) |
> > + SMT_HEADER_PROTOCOL_ID(msg->protocol_id) |
> > + SMT_HEADER_MESSAGE_ID(msg->message_id);
> > +
> > + memcpy(hdr->msg_payload, msg->in_msg, msg->in_msg_sz);
>
> memcpy_toio?
>
> > +
> > + return 0;
> > +}
> > +
> > +static int read_resp_from_smt(struct udevice *dev, struct scmi_shm_buf
> > *shm_buf,
> > + struct scmi_msg *msg)
> > +{
> > + struct scmi_smt_header *hdr = (void *)shm_buf->buf;
> > +
> > + if (!(hdr->channel_status &
> > SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE)) {
> > + dev_err(dev, "Channel unexpectedly busy\n");
> > + return -EBUSY;
> > + }
> > +
> > + if (hdr->channel_status & SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR)
> > {
> > + dev_err(dev, "Channel error reported, reset channel\n");
> > + return -ECOMM;
> > + }
> > +
> > + if (hdr->length > msg->out_msg_sz + sizeof(hdr->msg_header)) {
> > + dev_err(dev, "Buffer to small\n");
> > + return -ETOOSMALL;
> > + }
> > +
> > + /* Get the data */
> > + msg->out_msg_sz = hdr->length - sizeof(hdr->msg_header);
> > + memcpy(msg->out_msg, hdr->msg_payload, msg->out_msg_sz);
> > +
> > + return 0;
> > +}
> > +
> > +static void clear_smt_channel(struct scmi_shm_buf *shm_buf) {
> > + struct scmi_smt_header *hdr = (void *)shm_buf->buf;
> > +
> > + hdr->channel_status &=
> > ~SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR;
> > +}
> > +
> > +struct scmi_mbox_channel {
> > + struct scmi_shm_buf shm_buf;
> > + struct mbox_chan mbox;
> > + ulong timeout_us;
> > +};
> > +
> > +static int mbox_process_msg(struct udevice *dev, struct scmi_msg *msg)
> > +{
> > + struct scmi_agent *agent = dev_get_priv(dev);
> > + struct scmi_mbox_channel *chan = agent->method_priv;
> > + int rc;
> > +
> > + rc = write_msg_to_smt(dev, &chan->shm_buf, msg);
> > + if (rc)
> > + return rc;
> > +
> > + /* Give shm addr to mbox in case it is meaningful */
> > + rc = mbox_send(&chan->mbox, chan->shm_buf.buf);
> > + if (rc) {
> > + dev_err(dev, "Message send failed: %d\n", rc);
> > + goto out;
> > + }
> > +
> > + /* Receive the response */
> > + rc = mbox_recv(&chan->mbox, chan->shm_buf.buf, chan->timeout_us);
> > + if (rc) {
> > + dev_err(dev, "Response failed: %d, abort\n", rc);
> > + goto out;
> > + }
> > +
> > + rc = read_resp_from_smt(dev, &chan->shm_buf, msg);
> > +
> > +out:
> > + clear_smt_channel(&chan->shm_buf);
> > +
> > + return rc;
> > +}
> > +
> > +struct method_ops mbox_channel_ops = {
> > + .process_msg = mbox_process_msg,
> > +};
> > +
> > +static int probe_mailbox_channel(struct udevice *dev) {
> > + struct scmi_agent *agent = dev_get_priv(dev);
> > + struct scmi_mbox_channel *chan;
> > + int rc;
> > +
> > + chan = devm_kzalloc(dev, sizeof(*chan), GFP_KERNEL);
> > + if (!chan)
> > + return -ENOMEM;
> > +
> > + chan->timeout_us = TIMEOUT_US_10MS;
> > +
> > + rc = mbox_get_by_index(dev, 0, &chan->mbox);
> > + if (rc) {
> > + dev_err(dev, "Failed to find mailbox: %d\n", rc);
> > + goto out;
> > + }
> > +
> > + rc = get_shm_buffer(dev, &chan->shm_buf);
> > + if (rc)
> > + dev_err(dev, "Failed to get shm resources: %d\n", rc);
> > +
> > +out:
> > + if (rc) {
> > + devm_kfree(dev, chan);
> > + } else {
> > + agent->method_ops = &mbox_channel_ops;
> > + agent->method_priv = (void *)chan;
> > + }
> > +
> > + return rc;
> > +}
> > +
> > +struct scmi_arm_smc_channel {
> > + ulong func_id;
> > + struct scmi_shm_buf shm_buf;
> > +};
> > +
> > +#define SMCCC_RET_NOT_SUPPORTED ((unsigned long)-1)
> > +
> > +static int arm_smc_process_msg(struct udevice *dev, struct scmi_msg
> > +*msg) {
> > + struct scmi_agent *agent = dev_get_priv(dev);
> > + struct scmi_arm_smc_channel *chan = agent->method_priv;
> > + struct arm_smccc_res res;
> > + int rc;
> > +
> > + rc = write_msg_to_smt(dev, &chan->shm_buf, msg);
> > + if (rc)
> > + return rc;
> > +
> > + arm_smccc_smc(chan->func_id, 0, 0, 0, 0, 0, 0, 0, &res);
> > + if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
> > + rc = -EINVAL;
> > + else
> > + rc = read_resp_from_smt(dev, &chan->shm_buf, msg);
> > +
> > + clear_smt_channel(&chan->shm_buf);
>
> Should the clear be done in firmware side?
Clear is done by the client once it has read out the status set by the firmware.
>
> > +
> > + return rc;
> > +}
> > +
> > +struct method_ops arm_smc_channel_ops = {
> > + .process_msg = arm_smc_process_msg,
> > +};
> > +
> > +static int probe_arm_smc_channel(struct udevice *dev) {
> > + struct scmi_agent *agent = dev_get_priv(dev);
> > + struct scmi_arm_smc_channel *chan;
> > + ofnode node = dev_ofnode(dev);
> > + u32 func_id;
> > + int rc;
> > +
> > + chan = devm_kzalloc(dev, sizeof(*chan), GFP_KERNEL);
> > + if (!chan)
> > + return -ENOMEM;
> > +
> > + if (ofnode_read_u32(node, "arm,smc-id", &func_id)) {
> > + dev_err(dev, "Missing property func-id\n");
> > + return -EINVAL;
> > + }
> > +
> > + chan->func_id = func_id;
> > +
> > + rc = get_shm_buffer(dev, &chan->shm_buf);
> > + if (rc) {
> > + dev_err(dev, "Failed to get shm resources: %d\n", rc);
> > + return rc;
> > + }
> > +
> > + agent->method_ops = &arm_smc_channel_ops;
> > + agent->method_priv = (void *)chan;
> > +
> > + return rc;
> > +}
> > +
> > +/*
> > + * Exported functions by the SCMI agent */
> > +
> > +int scmi_send_and_process_msg(struct udevice *dev, struct scmi_msg
> > +*msg) {
> > + struct scmi_agent *agent = dev_get_priv(dev);
> > +
> > + return agent->method_ops->process_msg(dev, msg); }
> > +
> > +static int scmi_remove(struct udevice *dev) {
> > + struct scmi_agent *agent = dev_get_priv(dev);
> > +
> > + if (agent->method_ops->remove_agent)
> > + return agent->method_ops->remove_agent(dev);
> > +
> > + return 0;
> > +}
> > +
> > +enum scmi_transport_channel {
> > + SCMI_MAILBOX_TRANSPORT,
> > + SCMI_ARM_SMCCC_TRANSPORT,
> > +};
> > +
> > +static int scmi_probe(struct udevice *dev) {
> > + switch (dev_get_driver_data(dev)) {
> > + case SCMI_MAILBOX_TRANSPORT:
> > + if (IS_ENABLED(CONFIG_DM_MAILBOX))
> > + return probe_mailbox_channel(dev);
> > + break;
> > + case SCMI_ARM_SMCCC_TRANSPORT:
> > + if (IS_ENABLED(CONFIG_ARM_SMCCC))
> > + return probe_arm_smc_channel(dev);
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + return -EINVAL;
> > +}
> > +
> > +static int scmi_bind(struct udevice *dev) {
> > + int rc = 0;
> > + ofnode node;
> > + struct driver *drv;
> > +
> > + dev_for_each_subnode(node, dev) {
> > + u32 protocol_id;
> > +
> > + if (!ofnode_is_available(node))
> > + continue;
> > +
> > + if (ofnode_read_u32(node, "reg", &protocol_id))
> > + continue;
> > +
> > + switch (protocol_id) {
> > + default:
> > + dev_info(dev, "Ignore unsupported SCMI protocol %u\n",
> > + protocol_id);
> > + continue;
> > + }
> > +
> > + rc = device_bind_ofnode(dev, drv, ofnode_get_name(node),
> > + NULL, node, NULL);
> > + if (rc)
> > + break;
> > + }
> > +
> > + if (rc)
> > + device_unbind(dev);
> > +
> > + return rc;
> > +}
> > +
> > +static const struct udevice_id scmi_ids[] = { #ifdef CONFIG_DM_MAILBOX
> > + { .compatible = "arm,scmi", .data = SCMI_MAILBOX_TRANSPORT },
> > #endif
> > +#ifdef CONFIG_ARM_SMCCC
> > + { .compatible = "arm,scmi-smc", .data =
> > SCMI_ARM_SMCCC_TRANSPORT },
> > +#endif
>
> To minimize the code base, how about following Linux kernel to split mailbox
> and smc to different files?
Is it preferred instead of using several #ifdef in this single source file?
If splitting into several source files, it also means each source file
will need to export API function that in the end, are local to SCMI
agent drivers. Since the implementation is quite short , i though a
single source for the whole SCMI agent base + transport layers was
simpler.
Regards,
Etienne
>
> Regards,
> Peng.
>
> > + { }
> > +};
> > +
> > +U_BOOT_DRIVER(scmi) = {
> > + .name = "scmi",
> > + .id = UCLASS_FIRMWARE,
> > + .of_match = scmi_ids,
> > + .priv_auto_alloc_size = sizeof(struct scmi_agent),
> > + .bind = scmi_bind,
> > + .probe = scmi_probe,
> > + .remove = scmi_remove,
> > + .flags = DM_FLAG_OS_PREPARE,
> > +};
> > diff --git a/include/scmi.h b/include/scmi.h new file mode 100644 index
> > 00000000000..e12e322991d
> > --- /dev/null
> > +++ b/include/scmi.h
> > @@ -0,0 +1,82 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause */
> > +/*
> > + * Copyright (c) 2015-2019, Arm Limited and Contributors. All rights
> > reserved.
> > + * Copyright (C) 2019, Linaro Limited
> > + */
> > +#ifndef SCMI_H
> > +#define SCMI_H
> > +
> > +#include <asm/types.h>
> > +
> > +/**
> > + * An SCMI agent represent on communication path from a device driver
> > +to
> > + * the remote SCMI server which driver sends messages to and receives
> > + * response messages from.
> > + */
> > +struct scmi_agent;
> > +
> > +enum scmi_std_protocol {
> > + SCMI_PROTOCOL_ID_BASE = 0x10,
> > + SCMI_PROTOCOL_ID_POWER_DOMAIN = 0x11,
> > + SCMI_PROTOCOL_ID_SYSTEM = 0x12,
> > + SCMI_PROTOCOL_ID_PERF = 0x13,
> > + SCMI_PROTOCOL_ID_CLOCK = 0x14,
> > + SCMI_PROTOCOL_ID_SENSOR = 0x15,
> > + SCMI_PROTOCOL_ID_RESET_DOMAIN = 0x16,
> > +};
> > +
> > +enum scmi_status_code {
> > + SCMI_SUCCESS = 0,
> > + SCMI_NOT_SUPPORTED = -1,
> > + SCMI_INVALID_PARAMETERS = -2,
> > + SCMI_DENIED = -3,
> > + SCMI_NOT_FOUND = -4,
> > + SCMI_OUT_OF_RANGE = -5,
> > + SCMI_BUSY = -6,
> > + SCMI_COMMS_ERROR = -7,
> > + SCMI_GENERIC_ERROR = -8,
> > + SCMI_HARDWARE_ERROR = -9,
> > + SCMI_PROTOCOL_ERROR = -10,
> > +};
> > +
> > +/*
> > + * struct scmi_msg - Context of a SCMI message sent and the response
> > +received
> > + *
> > + * @protocol_id: SCMI protocol ID
> > + * @message_id: SCMI message ID for a defined protocol ID
> > + * @in_msg: pointer to the message payload sent by the driver
> > + * @in_msg_sz: byte size of the message payload sent
> > + * @out_msg: pointer to buffer to store response message payload
> > + * @out_msg_size: Byte size of the response buffer or payload */
> > +struct scmi_msg {
> > + unsigned int protocol_id;
> > + unsigned int message_id;
> > + u8 *in_msg;
> > + size_t in_msg_sz;
> > + u8 *out_msg;
> > + size_t out_msg_sz;
> > +};
> > +
> > +/**
> > + * scmi_send_and_process_msg() - send and process a SCMI message
> > + *
> > + * Send a message to a SCMI server through a target SCMI agent device.
> > + * Caller sets scmi_msg::out_msg_sz to the output message buffer size.
> > + * On return, scmi_msg::out_msg_sz stores the response payload size.
> > + *
> > + * @dev: SCMI agent device
> > + * @msg: Message structure reference
> > + * @return 0 on success, a negative errno otherwise */ int
> > +scmi_send_and_process_msg(struct udevice *dev, struct scmi_msg *msg);
> > +
> > +/**
> > + * scmi_to_linux_errno() - Convert an SCMI error code into a Linux
> > +errno code
> > + *
> > + * @scmi_errno: SCMI error code value
> > + * @return 0 for successful status and a negative errno otherwise */
> > +int scmi_to_linux_errno(s32 scmi_errno);
> > +
> > +#endif /* SCMI_H */
> > --
> > 2.17.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/4] dt-bindings: arm: SCMI bindings documentation
2020-07-20 2:01 ` [PATCH 2/4] dt-bindings: arm: SCMI bindings documentation Peng Fan
@ 2020-07-24 9:47 ` Etienne Carriere
0 siblings, 0 replies; 14+ messages in thread
From: Etienne Carriere @ 2020-07-24 9:47 UTC (permalink / raw)
To: u-boot
Hello Peng,
On Mon, 20 Jul 2020 at 04:01, Peng Fan <peng.fan@nxp.com> wrote:
>
> > Subject: [PATCH 2/4] dt-bindings: arm: SCMI bindings documentation
>
> Since kernel already has the binding doc, there is no need to add a U-Boot
> copy.
>
Ok. I'll remove this patch from the series in my v2.
Thanks,
Etienne
> Regards,
> Peng.
>
> >
> > Dump SCMI DT bindings documentation from Linux kernel source tree
> > v5.8-rc1.
> >
> > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> > ---
> >
> > doc/device-tree-bindings/arm/arm,scmi.txt | 197
> > ++++++++++++++++++++++
> > 1 file changed, 197 insertions(+)
> > create mode 100644 doc/device-tree-bindings/arm/arm,scmi.txt
> >
> > diff --git a/doc/device-tree-bindings/arm/arm,scmi.txt
> > b/doc/device-tree-bindings/arm/arm,scmi.txt
> > new file mode 100644
> > index 00000000000..1f293ea24cd
> > --- /dev/null
> > +++ b/doc/device-tree-bindings/arm/arm,scmi.txt
> > @@ -0,0 +1,197 @@
> > +System Control and Management Interface (SCMI) Message Protocol
> > +----------------------------------------------------------
> > +
> > +The SCMI is intended to allow agents such as OSPM to manage various
> > +functions that are provided by the hardware platform it is running on,
> > +including power and performance functions.
> > +
> > +This binding is intended to define the interface the firmware
> > +implementing the SCMI as described in ARM document number ARM DEN
> > 0056A
> > +("ARM System Control and Management Interface Platform Design
> > +Document")[0] provide for OSPM in the device tree.
> > +
> > +Required properties:
> > +
> > +The scmi node with the following properties shall be under the /firmware/
> > node.
> > +
> > +- compatible : shall be "arm,scmi" or "arm,scmi-smc" for smc/hvc
> > +transports
> > +- mboxes: List of phandle and mailbox channel specifiers. It should contain
> > + exactly one or two mailboxes, one for transmitting messages("tx")
> > + and another optional for receiving the notifications("rx") if
> > + supported.
> > +- shmem : List of phandle pointing to the shared memory(SHM) area as per
> > + generic mailbox client binding.
> > +- #address-cells : should be '1' if the device has sub-nodes, maps to
> > + protocol identifier for a given sub-node.
> > +- #size-cells : should be '0' as 'reg' property doesn't have any size
> > + associated with it.
> > +- arm,smc-id : SMC id required when using smc or hvc transports
> > +
> > +Optional properties:
> > +
> > +- mbox-names: shall be "tx" or "rx" depending on mboxes entries.
> > +
> > +See Documentation/devicetree/bindings/mailbox/mailbox.txt for more
> > +details about the generic mailbox controller and client driver bindings.
> > +
> > +The mailbox is the only permitted method of calling the SCMI firmware.
> > +Mailbox doorbell is used as a mechanism to alert the presence of a
> > +messages and/or notification.
> > +
> > +Each protocol supported shall have a sub-node with corresponding
> > +compatible as described in the following sections. If the platform
> > +supports dedicated communication channel for a particular protocol, the 3
> > properties namely:
> > +mboxes, mbox-names and shmem shall be present in the sub-node
> > +corresponding to that protocol.
> > +
> > +Clock/Performance bindings for the clocks/OPPs based on SCMI Message
> > +Protocol
> > +------------------------------------------------------------
> > +
> > +This binding uses the common clock binding[1].
> > +
> > +Required properties:
> > +- #clock-cells : Should be 1. Contains the Clock ID value used by SCMI
> > commands.
> > +
> > +Power domain bindings for the power domains based on SCMI Message
> > +Protocol
> > +------------------------------------------------------------
> > +
> > +This binding for the SCMI power domain providers uses the generic power
> > +domain binding[2].
> > +
> > +Required properties:
> > + - #power-domain-cells : Should be 1. Contains the device or the power
> > + domain ID value used by SCMI commands.
> > +
> > +Sensor bindings for the sensors based on SCMI Message Protocol
> > +--------------------------------------------------------------
> > +SCMI provides an API to access the various sensors on the SoC.
> > +
> > +Required properties:
> > +- #thermal-sensor-cells: should be set to 1. This property follows the
> > + thermal device tree bindings[3].
> > +
> > + Valid cell values are raw identifiers (Sensor ID)
> > + as used by the firmware. Refer to platform details
> > + for your implementation for the IDs to use.
> > +
> > +Reset signal bindings for the reset domains based on SCMI Message
> > +Protocol
> > +------------------------------------------------------------
> > +
> > +This binding for the SCMI reset domain providers uses the generic reset
> > +signal binding[5].
> > +
> > +Required properties:
> > + - #reset-cells : Should be 1. Contains the reset domain ID value used
> > + by SCMI commands.
> > +
> > +SRAM and Shared Memory for SCMI
> > +-------------------------------
> > +
> > +A small area of SRAM is reserved for SCMI communication between
> > +application processors and SCP.
> > +
> > +The properties should follow the generic mmio-sram description found in
> > +[4]
> > +
> > +Each sub-node represents the reserved area for SCMI.
> > +
> > +Required sub-node properties:
> > +- reg : The base offset and size of the reserved area with the SRAM
> > +- compatible : should be "arm,scmi-shmem" for Non-secure SRAM based
> > + shared memory
> > +
> > +[0]
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Finfoce
> > +nter.arm.com%2Fhelp%2Ftopic%2Fcom.arm.doc.den0056a%2Findex.html&
> > amp;dat
> > +a=02%7C01%7Cpeng.fan%40nxp.com%7C94dcac2bc02346684bec08d82a67
> > 76b9%7C686
> > +ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637305971145926287&a
> > mp;sdata=gS
> > +HhNqZ14iDSv0ws76jZox0JOzAL7Yrg%2BV%2ByH1Szkj4%3D&reserved=
> > 0
> > +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> > +[2] Documentation/devicetree/bindings/power/power-domain.yaml
> > +[3] Documentation/devicetree/bindings/thermal/thermal.txt
> > +[4] Documentation/devicetree/bindings/sram/sram.yaml
> > +[5] Documentation/devicetree/bindings/reset/reset.txt
> > +
> > +Example:
> > +
> > +sram at 50000000 {
> > + compatible = "mmio-sram";
> > + reg = <0x0 0x50000000 0x0 0x10000>;
> > +
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + ranges = <0 0x0 0x50000000 0x10000>;
> > +
> > + cpu_scp_lpri: scp-shmem at 0 {
> > + compatible = "arm,scmi-shmem";
> > + reg = <0x0 0x200>;
> > + };
> > +
> > + cpu_scp_hpri: scp-shmem at 200 {
> > + compatible = "arm,scmi-shmem";
> > + reg = <0x200 0x200>;
> > + };
> > +};
> > +
> > +mailbox at 40000000 {
> > + ....
> > + #mbox-cells = <1>;
> > + reg = <0x0 0x40000000 0x0 0x10000>;
> > +};
> > +
> > +firmware {
> > +
> > + ...
> > +
> > + scmi {
> > + compatible = "arm,scmi";
> > + mboxes = <&mailbox 0 &mailbox 1>;
> > + mbox-names = "tx", "rx";
> > + shmem = <&cpu_scp_lpri &cpu_scp_hpri>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + scmi_devpd: protocol at 11 {
> > + reg = <0x11>;
> > + #power-domain-cells = <1>;
> > + };
> > +
> > + scmi_dvfs: protocol at 13 {
> > + reg = <0x13>;
> > + #clock-cells = <1>;
> > + };
> > +
> > + scmi_clk: protocol at 14 {
> > + reg = <0x14>;
> > + #clock-cells = <1>;
> > + };
> > +
> > + scmi_sensors0: protocol at 15 {
> > + reg = <0x15>;
> > + #thermal-sensor-cells = <1>;
> > + };
> > +
> > + scmi_reset: protocol at 16 {
> > + reg = <0x16>;
> > + #reset-cells = <1>;
> > + };
> > + };
> > +};
> > +
> > +cpu at 0 {
> > + ...
> > + reg = <0 0>;
> > + clocks = <&scmi_dvfs 0>;
> > +};
> > +
> > +hdlcd at 7ff60000 {
> > + ...
> > + reg = <0 0x7ff60000 0 0x1000>;
> > + clocks = <&scmi_clk 4>;
> > + power-domains = <&scmi_devpd 1>;
> > + resets = <&scmi_reset 10>;
> > +};
> > +
> > +thermal-zones {
> > + soc_thermal {
> > + polling-delay-passive = <100>;
> > + polling-delay = <1000>;
> > + /* sensor ID */
> > + thermal-sensors = <&scmi_sensors0 3>;
> > + ...
> > + };
> > +};
> > --
> > 2.17.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/4] clk: add clock driver for SCMI agents
2020-07-20 2:06 ` [PATCH 3/4] clk: add clock driver for SCMI agents Peng Fan
@ 2020-07-24 9:54 ` Etienne Carriere
0 siblings, 0 replies; 14+ messages in thread
From: Etienne Carriere @ 2020-07-24 9:54 UTC (permalink / raw)
To: u-boot
Hi Peng,
On Mon, 20 Jul 2020 at 04:06, Peng Fan <peng.fan@nxp.com> wrote:
>
>
> > Subject: [PATCH 3/4] clk: add clock driver for SCMI agents
> >
> > This change introduces a clock driver for SCMI agent devices. When SCMI
> > agent and SCMI clock drivers are enabled, SCMI agent binds a clock device for
> > each SCMI clock protocol devices enabled in the FDT.
> >
> > (...)
> >
> > +
> > +static ulong scmi_clk_set_rate(struct clk *clk, ulong rate) {
> > + struct scmi_clk_rate_set_in in = {
> > + .clock_id = clk->id,
> > + .flags = SCMI_CLK_RATE_ASYNC_NORESP |
>
> This will use async, but how if sync?
Thanks, you're right, i made it wrong. Considering u-boot sequential
execution, synchronous rate setting is prefered.
I'll remove SCMI_CLK_RATE_ASYNC_NORESP from .flags value.
Etienne
>
> Thanks,
> Peng.
>
> > + SCMI_CLK_RATE_ROUND_CLOSEST,
> > + .rate_lsb = (u32)rate,
> > + .rate_msb = (u32)((u64)rate >> 32),
> > + };
> >
> > (...)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/4] firmware: add new driver for SCMI firmwares
[not found] <20200717153731.10643-1-etienne.carriere@linaro.org>
` (3 preceding siblings ...)
[not found] ` <20200717153731.10643-4-etienne.carriere@linaro.org>
@ 2020-07-26 14:54 ` Simon Glass
2020-08-04 9:33 ` Etienne Carriere
4 siblings, 1 reply; 14+ messages in thread
From: Simon Glass @ 2020-07-26 14:54 UTC (permalink / raw)
To: u-boot
Hi Etienne,
On Fri, 17 Jul 2020 at 09:38, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> This change introduces SCMI agent driver in U-Boot in the firmware
> U-class.
>
> SCMI agent driver is designed for platforms that embed a SCMI server in
> a firmware hosted for example by a companion co-processor or the secure
> world of the executing processor.
>
> SCMI protocols allow an SCMI agent to discover and access external
> resources as clock, reset controllers and many more. SCMI agent and
> server communicate following the SCMI specification [1]. SCMI agent
> complies with the DT bindings defined in the Linux kernel source tree
> regarding SCMI agent description since v5.8-rc1.
>
> These bindings describe 2 supported message transport layer: using
> mailbox uclass devices or using Arm SMC invocation instruction. Both
> use a piece or shared memory for message data exchange.
>
> In the current state, the SCMI agent driver does not bind to any SCMI
> protocol to a U-Boot device driver. Former changes will implement
> dedicated driver (i.e. an SCMI clock driver or an SCMI reset controller
> driver) and add bind supported SCMI protocols in scmi_agent_bind().
>
> Links: [1] https://developer.arm.com/architectures/system-architectures/software-standards/scmi
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
>
> drivers/firmware/Kconfig | 15 ++
> drivers/firmware/Makefile | 1 +
> drivers/firmware/scmi.c | 439 ++++++++++++++++++++++++++++++++++++++
> include/scmi.h | 82 +++++++
> 4 files changed, 537 insertions(+)
> create mode 100644 drivers/firmware/scmi.c
> create mode 100644 include/scmi.h
>
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index b70a2063551..f7c7ee7a5aa 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -1,6 +1,21 @@
> config FIRMWARE
> bool "Enable Firmware driver support"
>
> +config SCMI_FIRMWARE
> + bool "Enable SCMI support"
> + select FIRMWARE
> + select OF_TRANSLATE
> + depends on DM_MAILBOX || ARM_SMCCC
> + help
> + An SCMI agent communicates with a related SCMI server firmware
Please write out SCMI in full somewhere and add a link to the spec.
> + located in another sub-system, as a companion micro controller
> + or a companion host in the CPU system.
> +
> + Communications between agent (client) and the SCMI server are
> + based on message exchange. Messages can be exchange over tranport
> + channels as a mailbox device or an Arm SMCCC service with some
> + piece of identified shared memory.
> +
> config SPL_FIRMWARE
> bool "Enable Firmware driver support in SPL"
> depends on FIRMWARE
> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
> index a0c250a473e..3965838179f 100644
> --- a/drivers/firmware/Makefile
> +++ b/drivers/firmware/Makefile
> @@ -2,4 +2,5 @@ obj-$(CONFIG_FIRMWARE) += firmware-uclass.o
> obj-$(CONFIG_$(SPL_)ARM_PSCI_FW) += psci.o
> obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o
> obj-$(CONFIG_SANDBOX) += firmware-sandbox.o
> +obj-$(CONFIG_SCMI_FIRMWARE) += scmi.o
> obj-$(CONFIG_ZYNQMP_FIRMWARE) += firmware-zynqmp.o
> diff --git a/drivers/firmware/scmi.c b/drivers/firmware/scmi.c
> new file mode 100644
> index 00000000000..fa8a91c3f3d
> --- /dev/null
> +++ b/drivers/firmware/scmi.c
> @@ -0,0 +1,439 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2015-2019, Arm Limited and Contributors. All rights reserved.
> + * Copyright (C) 2019-2020 Linaro Limited.
> + */
> +
> +#include <common.h>
> +#include <cpu_func.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <mailbox.h>
> +#include <memalign.h>
> +#include <scmi.h>
> +#include <asm/system.h>
> +#include <asm/types.h>
> +#include <dm/device-internal.h>
> +#include <dm/devres.h>
> +#include <dm/lists.h>
> +#include <dm/ofnode.h>
> +#include <linux/arm-smccc.h>
> +#include <linux/compat.h>
> +#include <linux/errno.h>
> +#include <linux/io.h>
> +#include <linux/ioport.h>
> +
> +#define TIMEOUT_US_10MS 10000
> +
> +struct error_code {
Function comment
> + int scmi;
> + int errno;
> +};
> +
> +static const struct error_code scmi_linux_errmap[] = {
> + { .scmi = SCMI_NOT_SUPPORTED, .errno = -EOPNOTSUPP, },
> + { .scmi = SCMI_INVALID_PARAMETERS, .errno = -EINVAL, },
> + { .scmi = SCMI_DENIED, .errno = -EACCES, },
> + { .scmi = SCMI_NOT_FOUND, .errno = -ENOENT, },
> + { .scmi = SCMI_OUT_OF_RANGE, .errno = -ERANGE, },
> + { .scmi = SCMI_BUSY, .errno = -EBUSY, },
> + { .scmi = SCMI_COMMS_ERROR, .errno = -ECOMM, },
> + { .scmi = SCMI_GENERIC_ERROR, .errno = -EIO, },
> + { .scmi = SCMI_HARDWARE_ERROR, .errno = -EREMOTEIO, },
> + { .scmi = SCMI_PROTOCOL_ERROR, .errno = -EPROTO, },
> +};
> +
> +int scmi_to_linux_errno(s32 scmi_code)
> +{
> + int n;
> +
> + if (scmi_code == 0)
> + return 0;
> +
> + for (n = 0; n < ARRAY_SIZE(scmi_linux_errmap); n++)
> + if (scmi_code == scmi_linux_errmap[n].scmi)
> + return scmi_linux_errmap[1].errno;
> +
> + return -EPROTO;
> +}
> +
> +struct method_ops {
> + int (*process_msg)(struct udevice *dev, struct scmi_msg *msg);
> + int (*remove_agent)(struct udevice *dev);
Looks like this should be a new uclass and child driver?
Also we should have a sandbox implementation so this code can be tested.
> +};
> +
> +struct scmi_agent {
> + struct method_ops *method_ops;
> + void *method_priv;
> +};
> +
> +/*
> + * Shared Memory based Transport (SMT) message buffer management
> + *
> + * SMT uses 28 byte header prior message payload to handle the state of
> + * the communication channel realized by the shared memory area and
> + * to define SCMI protocol information the payload relates to.
> + */
> +struct scmi_smt_header {
> + __le32 reserved;
> + __le32 channel_status;
> +#define SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR BIT(1)
> +#define SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE BIT(0)
> + __le32 reserved1[2];
> + __le32 flags;
> +#define SCMI_SHMEM_FLAG_INTR_ENABLED BIT(0)
> + __le32 length;
> + __le32 msg_header;
> + u8 msg_payload[0];
> +};
> +
> +#define SMT_HEADER_TOKEN(token) (((token) << 18) & GENMASK(31, 18))
> +#define SMT_HEADER_PROTOCOL_ID(proto) (((proto) << 10) & GENMASK(17, 10))
> +#define SMT_HEADER_MESSAGE_TYPE(type) (((type) << 18) & GENMASK(9, 8))
> +#define SMT_HEADER_MESSAGE_ID(id) ((id) & GENMASK(7, 0))
> +
> +struct scmi_shm_buf {
comment
> + u8 *buf;
> + size_t size;
> +};
> +
> +static int get_shm_buffer(struct udevice *dev, struct scmi_shm_buf *shm)
> +{
> + int rc;
Can you use 'ret' as 'rc' is a bit short for a var, and we mostly use
'ret' with driver model.
> + struct ofnode_phandle_args args;
> + struct resource resource;
> + fdt32_t faddr;
> + phys_addr_t paddr;
> +
> + rc = dev_read_phandle_with_args(dev, "shmem", NULL, 0, 0, &args);
> + if (rc)
> + return rc;
> +
> + rc = ofnode_read_resource(args.node, 0, &resource);
> + if (rc)
> + return rc;
> +
> + faddr = cpu_to_fdt32(resource.start);
> + paddr = ofnode_translate_address(args.node, &faddr);
> +
> + shm->size = resource_size(&resource);
> + if (shm->size < sizeof(struct scmi_smt_header)) {
> + dev_err(dev, "Shared memory buffer too small\n");
> + return -EINVAL;
> + }
> +
> + shm->buf = devm_ioremap(dev, paddr, shm->size);
> + if (!shm->buf)
> + return -ENOMEM;
> +
> + if (dcache_status())
> + mmu_set_region_dcache_behaviour((uintptr_t)shm->buf,
> + shm->size, DCACHE_OFF);
> +
> + return 0;
> +}
> +
> +static int write_msg_to_smt(struct udevice *dev, struct scmi_shm_buf *shm_buf,
> + struct scmi_msg *msg)
function comment with errors
> +{
> + struct scmi_smt_header *hdr = (void *)shm_buf->buf;
> +
> + if ((!msg->in_msg && msg->in_msg_sz) ||
> + (!msg->out_msg && msg->out_msg_sz))
> + return -EINVAL;
> +
> + if (!(hdr->channel_status & SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE)) {
> + dev_dbg(dev, "Channel busy\n");
> + return -EBUSY;
> + }
> +
> + if (shm_buf->size < (sizeof(*hdr) + msg->in_msg_sz) ||
> + shm_buf->size < (sizeof(*hdr) + msg->out_msg_sz)) {
> + dev_dbg(dev, "Buffer too small\n");
> + return -ETOOSMALL;
> + }
> +
> + /* Load message in shared memory */
> + hdr->channel_status &= ~SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE;
> + hdr->length = msg->in_msg_sz + sizeof(hdr->msg_header);
> + hdr->msg_header = SMT_HEADER_TOKEN(0) |
> + SMT_HEADER_MESSAGE_TYPE(0) |
> + SMT_HEADER_PROTOCOL_ID(msg->protocol_id) |
> + SMT_HEADER_MESSAGE_ID(msg->message_id);
> +
> + memcpy(hdr->msg_payload, msg->in_msg, msg->in_msg_sz);
> +
> + return 0;
> +}
> +
> +static int read_resp_from_smt(struct udevice *dev, struct scmi_shm_buf *shm_buf,
> + struct scmi_msg *msg)
> +{
> + struct scmi_smt_header *hdr = (void *)shm_buf->buf;
> +
> + if (!(hdr->channel_status & SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE)) {
> + dev_err(dev, "Channel unexpectedly busy\n");
> + return -EBUSY;
> + }
> +
> + if (hdr->channel_status & SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR) {
> + dev_err(dev, "Channel error reported, reset channel\n");
> + return -ECOMM;
> + }
> +
> + if (hdr->length > msg->out_msg_sz + sizeof(hdr->msg_header)) {
> + dev_err(dev, "Buffer to small\n");
> + return -ETOOSMALL;
> + }
> +
> + /* Get the data */
> + msg->out_msg_sz = hdr->length - sizeof(hdr->msg_header);
> + memcpy(msg->out_msg, hdr->msg_payload, msg->out_msg_sz);
> +
> + return 0;
> +}
> +
> +static void clear_smt_channel(struct scmi_shm_buf *shm_buf)
> +{
> + struct scmi_smt_header *hdr = (void *)shm_buf->buf;
> +
> + hdr->channel_status &= ~SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR;
> +}
> +
> +struct scmi_mbox_channel {
please put structs at top of file
> + struct scmi_shm_buf shm_buf;
> + struct mbox_chan mbox;
> + ulong timeout_us;
> +};
> +
> +static int mbox_process_msg(struct udevice *dev, struct scmi_msg *msg)
> +{
> + struct scmi_agent *agent = dev_get_priv(dev);
> + struct scmi_mbox_channel *chan = agent->method_priv;
> + int rc;
> +
> + rc = write_msg_to_smt(dev, &chan->shm_buf, msg);
> + if (rc)
> + return rc;
> +
> + /* Give shm addr to mbox in case it is meaningful */
> + rc = mbox_send(&chan->mbox, chan->shm_buf.buf);
> + if (rc) {
> + dev_err(dev, "Message send failed: %d\n", rc);
> + goto out;
> + }
> +
> + /* Receive the response */
> + rc = mbox_recv(&chan->mbox, chan->shm_buf.buf, chan->timeout_us);
> + if (rc) {
> + dev_err(dev, "Response failed: %d, abort\n", rc);
> + goto out;
> + }
> +
> + rc = read_resp_from_smt(dev, &chan->shm_buf, msg);
> +
> +out:
> + clear_smt_channel(&chan->shm_buf);
> +
> + return rc;
> +}
> +
> +struct method_ops mbox_channel_ops = {
> + .process_msg = mbox_process_msg,
> +};
> +
> +static int probe_mailbox_channel(struct udevice *dev)
> +{
> + struct scmi_agent *agent = dev_get_priv(dev);
> + struct scmi_mbox_channel *chan;
> + int rc;
> +
> + chan = devm_kzalloc(dev, sizeof(*chan), GFP_KERNEL);
> + if (!chan)
> + return -ENOMEM;
> +
> + chan->timeout_us = TIMEOUT_US_10MS;
> +
> + rc = mbox_get_by_index(dev, 0, &chan->mbox);
> + if (rc) {
> + dev_err(dev, "Failed to find mailbox: %d\n", rc);
> + goto out;
> + }
> +
> + rc = get_shm_buffer(dev, &chan->shm_buf);
> + if (rc)
> + dev_err(dev, "Failed to get shm resources: %d\n", rc);
> +
> +out:
> + if (rc) {
> + devm_kfree(dev, chan);
return rc
> + } else {
> + agent->method_ops = &mbox_channel_ops;
> + agent->method_priv = (void *)chan;
> + }
> +
> + return rc;
return 0
(to keep the normal non-error flow as the main one)
> +}
> +
> +struct scmi_arm_smc_channel {
> + ulong func_id;
> + struct scmi_shm_buf shm_buf;
> +};
> +
> +#define SMCCC_RET_NOT_SUPPORTED ((unsigned long)-1)
Please struct and #define decls to top of file.
> +
> +static int arm_smc_process_msg(struct udevice *dev, struct scmi_msg *msg)
> +{
> + struct scmi_agent *agent = dev_get_priv(dev);
> + struct scmi_arm_smc_channel *chan = agent->method_priv;
> + struct arm_smccc_res res;
> + int rc;
> +
> + rc = write_msg_to_smt(dev, &chan->shm_buf, msg);
> + if (rc)
> + return rc;
> +
> + arm_smccc_smc(chan->func_id, 0, 0, 0, 0, 0, 0, 0, &res);
> + if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
> + rc = -EINVAL;
ENOTSUP?
> + else
> + rc = read_resp_from_smt(dev, &chan->shm_buf, msg);
> +
> + clear_smt_channel(&chan->shm_buf);
> +
> + return rc;
> +}
> +
> +struct method_ops arm_smc_channel_ops = {
> + .process_msg = arm_smc_process_msg,
> +};
> +
> +static int probe_arm_smc_channel(struct udevice *dev)
> +{
> + struct scmi_agent *agent = dev_get_priv(dev);
> + struct scmi_arm_smc_channel *chan;
> + ofnode node = dev_ofnode(dev);
> + u32 func_id;
> + int rc;
> +
> + chan = devm_kzalloc(dev, sizeof(*chan), GFP_KERNEL);
> + if (!chan)
> + return -ENOMEM;
> +
> + if (ofnode_read_u32(node, "arm,smc-id", &func_id)) {
dev_read_u32()
> + dev_err(dev, "Missing property func-id\n");
> + return -EINVAL;
> + }
> +
> + chan->func_id = func_id;
> +
> + rc = get_shm_buffer(dev, &chan->shm_buf);
> + if (rc) {
> + dev_err(dev, "Failed to get shm resources: %d\n", rc);
> + return rc;
> + }
> +
> + agent->method_ops = &arm_smc_channel_ops;
> + agent->method_priv = (void *)chan;
> +
> + return rc;
> +}
> +
> +/*
> + * Exported functions by the SCMI agent
> + */
> +
> +int scmi_send_and_process_msg(struct udevice *dev, struct scmi_msg *msg)
> +{
> + struct scmi_agent *agent = dev_get_priv(dev);
> +
> + return agent->method_ops->process_msg(dev, msg);
> +}
> +
> +static int scmi_remove(struct udevice *dev)
> +{
> + struct scmi_agent *agent = dev_get_priv(dev);
> +
> + if (agent->method_ops->remove_agent)
> + return agent->method_ops->remove_agent(dev);
method_ops should be the uclass operations. I think you need a new
uclass or add something to UCLASS_FIRMWARE.
> +
> + return 0;
> +}
> +
> +enum scmi_transport_channel {
> + SCMI_MAILBOX_TRANSPORT,
> + SCMI_ARM_SMCCC_TRANSPORT,
> +};
> +
> +static int scmi_probe(struct udevice *dev)
> +{
> + switch (dev_get_driver_data(dev)) {
> + case SCMI_MAILBOX_TRANSPORT:
> + if (IS_ENABLED(CONFIG_DM_MAILBOX))
> + return probe_mailbox_channel(dev);
> + break;
> + case SCMI_ARM_SMCCC_TRANSPORT:
> + if (IS_ENABLED(CONFIG_ARM_SMCCC))
> + return probe_arm_smc_channel(dev);
> + break;
> + default:
> + break;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int scmi_bind(struct udevice *dev)
> +{
> + int rc = 0;
> + ofnode node;
> + struct driver *drv;
> +
> + dev_for_each_subnode(node, dev) {
> + u32 protocol_id;
> +
> + if (!ofnode_is_available(node))
> + continue;
> +
> + if (ofnode_read_u32(node, "reg", &protocol_id))
> + continue;
> +
> + switch (protocol_id) {
> + default:
> + dev_info(dev, "Ignore unsupported SCMI protocol %u\n",
> + protocol_id);
> + continue;
> + }
> +
> + rc = device_bind_ofnode(dev, drv, ofnode_get_name(node),
> + NULL, node, NULL);
Can we instead add a compatible string to the device tree so driver
model creates these children automatically?
> + if (rc)
> + break;
> + }
> +
> + if (rc)
> + device_unbind(dev);
You should not unbind the device within its bind() method. Just make
sure it refuses to probe.
> +
> + return rc;
> +}
> +
> +static const struct udevice_id scmi_ids[] = {
> +#ifdef CONFIG_DM_MAILBOX
> + { .compatible = "arm,scmi", .data = SCMI_MAILBOX_TRANSPORT },
> +#endif
> +#ifdef CONFIG_ARM_SMCCC
> + { .compatible = "arm,scmi-smc", .data = SCMI_ARM_SMCCC_TRANSPORT },
> +#endif
Seems like you need a uclass for the transport?
> + { }
> +};
> +
> +U_BOOT_DRIVER(scmi) = {
> + .name = "scmi",
> + .id = UCLASS_FIRMWARE,
> + .of_match = scmi_ids,
> + .priv_auto_alloc_size = sizeof(struct scmi_agent),
> + .bind = scmi_bind,
> + .probe = scmi_probe,
> + .remove = scmi_remove,
> + .flags = DM_FLAG_OS_PREPARE,
> +};
> diff --git a/include/scmi.h b/include/scmi.h
> new file mode 100644
> index 00000000000..e12e322991d
> --- /dev/null
> +++ b/include/scmi.h
> @@ -0,0 +1,82 @@
> +/* SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause */
> +/*
> + * Copyright (c) 2015-2019, Arm Limited and Contributors. All rights reserved.
> + * Copyright (C) 2019, Linaro Limited
> + */
> +#ifndef SCMI_H
> +#define SCMI_H
> +
> +#include <asm/types.h>
> +
> +/**
> + * An SCMI agent represent on communication path from a device driver to
> + * the remote SCMI server which driver sends messages to and receives
> + * response messages from.
> + */
> +struct scmi_agent;
> +
> +enum scmi_std_protocol {
> + SCMI_PROTOCOL_ID_BASE = 0x10,
> + SCMI_PROTOCOL_ID_POWER_DOMAIN = 0x11,
> + SCMI_PROTOCOL_ID_SYSTEM = 0x12,
> + SCMI_PROTOCOL_ID_PERF = 0x13,
> + SCMI_PROTOCOL_ID_CLOCK = 0x14,
> + SCMI_PROTOCOL_ID_SENSOR = 0x15,
> + SCMI_PROTOCOL_ID_RESET_DOMAIN = 0x16,
> +};
> +
> +enum scmi_status_code {
> + SCMI_SUCCESS = 0,
> + SCMI_NOT_SUPPORTED = -1,
> + SCMI_INVALID_PARAMETERS = -2,
> + SCMI_DENIED = -3,
> + SCMI_NOT_FOUND = -4,
> + SCMI_OUT_OF_RANGE = -5,
> + SCMI_BUSY = -6,
> + SCMI_COMMS_ERROR = -7,
> + SCMI_GENERIC_ERROR = -8,
> + SCMI_HARDWARE_ERROR = -9,
> + SCMI_PROTOCOL_ERROR = -10,
> +};
> +
> +/*
> + * struct scmi_msg - Context of a SCMI message sent and the response received
> + *
> + * @protocol_id: SCMI protocol ID
> + * @message_id: SCMI message ID for a defined protocol ID
> + * @in_msg: pointer to the message payload sent by the driver
> + * @in_msg_sz: byte size of the message payload sent
> + * @out_msg: pointer to buffer to store response message payload
> + * @out_msg_size: Byte size of the response buffer or payload
sz
> + */
> +struct scmi_msg {
> + unsigned int protocol_id;
> + unsigned int message_id;
Does this need a particular size, e,g. u32?
> + u8 *in_msg;
> + size_t in_msg_sz;
> + u8 *out_msg;
> + size_t out_msg_sz;
> +};
> +
> +/**
> + * scmi_send_and_process_msg() - send and process a SCMI message
> + *
> + * Send a message to a SCMI server through a target SCMI agent device.
> + * Caller sets scmi_msg::out_msg_sz to the output message buffer size.
> + * On return, scmi_msg::out_msg_sz stores the response payload size.
> + *
> + * @dev: SCMI agent device
> + * @msg: Message structure reference
> + * @return 0 on success, a negative errno otherwise
> + */
> +int scmi_send_and_process_msg(struct udevice *dev, struct scmi_msg *msg);
> +
> +/**
> + * scmi_to_linux_errno() - Convert an SCMI error code into a Linux errno code
> + *
> + * @scmi_errno: SCMI error code value
> + * @return 0 for successful status and a negative errno otherwise
> + */
> +int scmi_to_linux_errno(s32 scmi_errno);
> +
> +#endif /* SCMI_H */
> --
> 2.17.1
>
Regards,
Simon
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/4] firmware: add new driver for SCMI firmwares
2020-07-26 14:54 ` [PATCH 1/4] firmware: add new driver for SCMI firmwares Simon Glass
@ 2020-08-04 9:33 ` Etienne Carriere
2020-08-16 3:39 ` Simon Glass
0 siblings, 1 reply; 14+ messages in thread
From: Etienne Carriere @ 2020-08-04 9:33 UTC (permalink / raw)
To: u-boot
Hello Simon,
Thanks for the feedback, I'll fix the changes in my v2.
and sorry for these delayed answers.
On Sun, 26 Jul 2020 at 16:55, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Etienne,
>
> On Fri, 17 Jul 2020 at 09:38, Etienne Carriere
> <etienne.carriere@linaro.org> wrote:
> >
> > This change introduces SCMI agent driver in U-Boot in the firmware
> > U-class.
> >
> > SCMI agent driver is designed for platforms that embed a SCMI server in
> > a firmware hosted for example by a companion co-processor or the secure
> > world of the executing processor.
> >
> > SCMI protocols allow an SCMI agent to discover and access external
> > resources as clock, reset controllers and many more. SCMI agent and
> > server communicate following the SCMI specification [1]. SCMI agent
> > complies with the DT bindings defined in the Linux kernel source tree
> > regarding SCMI agent description since v5.8-rc1.
> >
> > These bindings describe 2 supported message transport layer: using
> > mailbox uclass devices or using Arm SMC invocation instruction. Both
> > use a piece or shared memory for message data exchange.
> >
> > In the current state, the SCMI agent driver does not bind to any SCMI
> > protocol to a U-Boot device driver. Former changes will implement
> > dedicated driver (i.e. an SCMI clock driver or an SCMI reset controller
> > driver) and add bind supported SCMI protocols in scmi_agent_bind().
> >
> > Links: [1] https://developer.arm.com/architectures/system-architectures/software-standards/scmi
> > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> > ---
> >
> > drivers/firmware/Kconfig | 15 ++
> > drivers/firmware/Makefile | 1 +
> > drivers/firmware/scmi.c | 439 ++++++++++++++++++++++++++++++++++++++
> > include/scmi.h | 82 +++++++
> > 4 files changed, 537 insertions(+)
> > create mode 100644 drivers/firmware/scmi.c
> > create mode 100644 include/scmi.h
> >
> > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> > index b70a2063551..f7c7ee7a5aa 100644
> > --- a/drivers/firmware/Kconfig
> > +++ b/drivers/firmware/Kconfig
> > @@ -1,6 +1,21 @@
> > config FIRMWARE
> > bool "Enable Firmware driver support"
> >
> > +config SCMI_FIRMWARE
> > + bool "Enable SCMI support"
> > + select FIRMWARE
> > + select OF_TRANSLATE
> > + depends on DM_MAILBOX || ARM_SMCCC
> > + help
> > + An SCMI agent communicates with a related SCMI server firmware
>
> Please write out SCMI in full somewhere and add a link to the spec.
>
> > + located in another sub-system, as a companion micro controller
> > + or a companion host in the CPU system.
> > +
> > + Communications between agent (client) and the SCMI server are
> > + based on message exchange. Messages can be exchange over tranport
> > + channels as a mailbox device or an Arm SMCCC service with some
> > + piece of identified shared memory.
> > +
> > config SPL_FIRMWARE
> > bool "Enable Firmware driver support in SPL"
> > depends on FIRMWARE
> > diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
> > index a0c250a473e..3965838179f 100644
> > --- a/drivers/firmware/Makefile
> > +++ b/drivers/firmware/Makefile
> > @@ -2,4 +2,5 @@ obj-$(CONFIG_FIRMWARE) += firmware-uclass.o
> > obj-$(CONFIG_$(SPL_)ARM_PSCI_FW) += psci.o
> > obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o
> > obj-$(CONFIG_SANDBOX) += firmware-sandbox.o
> > +obj-$(CONFIG_SCMI_FIRMWARE) += scmi.o
> > obj-$(CONFIG_ZYNQMP_FIRMWARE) += firmware-zynqmp.o
> > diff --git a/drivers/firmware/scmi.c b/drivers/firmware/scmi.c
> > new file mode 100644
> > index 00000000000..fa8a91c3f3d
> > --- /dev/null
> > +++ b/drivers/firmware/scmi.c
> > @@ -0,0 +1,439 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2015-2019, Arm Limited and Contributors. All rights reserved.
> > + * Copyright (C) 2019-2020 Linaro Limited.
> > + */
> > +
> > +#include <common.h>
> > +#include <cpu_func.h>
> > +#include <dm.h>
> > +#include <errno.h>
> > +#include <mailbox.h>
> > +#include <memalign.h>
> > +#include <scmi.h>
> > +#include <asm/system.h>
> > +#include <asm/types.h>
> > +#include <dm/device-internal.h>
> > +#include <dm/devres.h>
> > +#include <dm/lists.h>
> > +#include <dm/ofnode.h>
> > +#include <linux/arm-smccc.h>
> > +#include <linux/compat.h>
> > +#include <linux/errno.h>
> > +#include <linux/io.h>
> > +#include <linux/ioport.h>
> > +
> > +#define TIMEOUT_US_10MS 10000
> > +
> > +struct error_code {
>
> Function comment
>
> > + int scmi;
> > + int errno;
> > +};
> > +
> > +static const struct error_code scmi_linux_errmap[] = {
> > + { .scmi = SCMI_NOT_SUPPORTED, .errno = -EOPNOTSUPP, },
> > + { .scmi = SCMI_INVALID_PARAMETERS, .errno = -EINVAL, },
> > + { .scmi = SCMI_DENIED, .errno = -EACCES, },
> > + { .scmi = SCMI_NOT_FOUND, .errno = -ENOENT, },
> > + { .scmi = SCMI_OUT_OF_RANGE, .errno = -ERANGE, },
> > + { .scmi = SCMI_BUSY, .errno = -EBUSY, },
> > + { .scmi = SCMI_COMMS_ERROR, .errno = -ECOMM, },
> > + { .scmi = SCMI_GENERIC_ERROR, .errno = -EIO, },
> > + { .scmi = SCMI_HARDWARE_ERROR, .errno = -EREMOTEIO, },
> > + { .scmi = SCMI_PROTOCOL_ERROR, .errno = -EPROTO, },
> > +};
> > +
> > +int scmi_to_linux_errno(s32 scmi_code)
> > +{
> > + int n;
> > +
> > + if (scmi_code == 0)
> > + return 0;
> > +
> > + for (n = 0; n < ARRAY_SIZE(scmi_linux_errmap); n++)
> > + if (scmi_code == scmi_linux_errmap[n].scmi)
> > + return scmi_linux_errmap[1].errno;
> > +
> > + return -EPROTO;
> > +}
> > +
> > +struct method_ops {
> > + int (*process_msg)(struct udevice *dev, struct scmi_msg *msg);
> > + int (*remove_agent)(struct udevice *dev);
>
> Looks like this should be a new uclass and child driver?
I think a uclass is overkilling here.
A scmi agent device is a firmware driver that binds to devices of
other uclasses (clock, reset, ...).
When scmi agent device is probed to get the related clock, reset, ...
devices be probed.
So it does not need any child.
(see also related comment below)
>
> Also we should have a sandbox implementation so this code can be tested.
Ok, I understand the sandbox test stuff.
Should it be part of the v2 this series, can it be done in parallel,
or a later v<N>?
>
> > +};
> > +
> > +struct scmi_agent {
> > + struct method_ops *method_ops;
> > + void *method_priv;
> > +};
> > +
> > +/*
> > + * Shared Memory based Transport (SMT) message buffer management
> > + *
> > + * SMT uses 28 byte header prior message payload to handle the state of
> > + * the communication channel realized by the shared memory area and
> > + * to define SCMI protocol information the payload relates to.
> > + */
> > +struct scmi_smt_header {
> > + __le32 reserved;
> > + __le32 channel_status;
> > +#define SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR BIT(1)
> > +#define SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE BIT(0)
> > + __le32 reserved1[2];
> > + __le32 flags;
> > +#define SCMI_SHMEM_FLAG_INTR_ENABLED BIT(0)
> > + __le32 length;
> > + __le32 msg_header;
> > + u8 msg_payload[0];
> > +};
> > +
> > +#define SMT_HEADER_TOKEN(token) (((token) << 18) & GENMASK(31, 18))
> > +#define SMT_HEADER_PROTOCOL_ID(proto) (((proto) << 10) & GENMASK(17, 10))
> > +#define SMT_HEADER_MESSAGE_TYPE(type) (((type) << 18) & GENMASK(9, 8))
> > +#define SMT_HEADER_MESSAGE_ID(id) ((id) & GENMASK(7, 0))
> > +
> > +struct scmi_shm_buf {
>
> comment
Ok. I will add inline comments for all the defined structs and move
macros/structs to source file top.
>
> > + u8 *buf;
> > + size_t size;
> > +};
> > +
> > +static int get_shm_buffer(struct udevice *dev, struct scmi_shm_buf *shm)
> > +{
> > + int rc;
>
> Can you use 'ret' as 'rc' is a bit short for a var, and we mostly use
> 'ret' with driver model.
Ok, I'll try to remember to adapt my habits :)
>
> > + struct ofnode_phandle_args args;
> > + struct resource resource;
> > + fdt32_t faddr;
> > + phys_addr_t paddr;
> > +
> > + rc = dev_read_phandle_with_args(dev, "shmem", NULL, 0, 0, &args);
> > + if (rc)
> > + return rc;
> > +
> > + rc = ofnode_read_resource(args.node, 0, &resource);
> > + if (rc)
> > + return rc;
> > +
> > + faddr = cpu_to_fdt32(resource.start);
> > + paddr = ofnode_translate_address(args.node, &faddr);
> > +
> > + shm->size = resource_size(&resource);
> > + if (shm->size < sizeof(struct scmi_smt_header)) {
> > + dev_err(dev, "Shared memory buffer too small\n");
> > + return -EINVAL;
> > + }
> > +
> > + shm->buf = devm_ioremap(dev, paddr, shm->size);
> > + if (!shm->buf)
> > + return -ENOMEM;
> > +
> > + if (dcache_status())
> > + mmu_set_region_dcache_behaviour((uintptr_t)shm->buf,
> > + shm->size, DCACHE_OFF);
> > +
> > + return 0;
> > +}
> > +
> > +static int write_msg_to_smt(struct udevice *dev, struct scmi_shm_buf *shm_buf,
> > + struct scmi_msg *msg)
>
> function comment with errors
>
> > +{
> > + struct scmi_smt_header *hdr = (void *)shm_buf->buf;
> > +
> > + if ((!msg->in_msg && msg->in_msg_sz) ||
> > + (!msg->out_msg && msg->out_msg_sz))
> > + return -EINVAL;
> > +
> > + if (!(hdr->channel_status & SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE)) {
> > + dev_dbg(dev, "Channel busy\n");
> > + return -EBUSY;
> > + }
> > +
> > + if (shm_buf->size < (sizeof(*hdr) + msg->in_msg_sz) ||
> > + shm_buf->size < (sizeof(*hdr) + msg->out_msg_sz)) {
> > + dev_dbg(dev, "Buffer too small\n");
> > + return -ETOOSMALL;
> > + }
> > +
> > + /* Load message in shared memory */
> > + hdr->channel_status &= ~SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE;
> > + hdr->length = msg->in_msg_sz + sizeof(hdr->msg_header);
> > + hdr->msg_header = SMT_HEADER_TOKEN(0) |
> > + SMT_HEADER_MESSAGE_TYPE(0) |
> > + SMT_HEADER_PROTOCOL_ID(msg->protocol_id) |
> > + SMT_HEADER_MESSAGE_ID(msg->message_id);
> > +
> > + memcpy(hdr->msg_payload, msg->in_msg, msg->in_msg_sz);
> > +
> > + return 0;
> > +}
> > +
> > +static int read_resp_from_smt(struct udevice *dev, struct scmi_shm_buf *shm_buf,
> > + struct scmi_msg *msg)
> > +{
> > + struct scmi_smt_header *hdr = (void *)shm_buf->buf;
> > +
> > + if (!(hdr->channel_status & SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE)) {
> > + dev_err(dev, "Channel unexpectedly busy\n");
> > + return -EBUSY;
> > + }
> > +
> > + if (hdr->channel_status & SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR) {
> > + dev_err(dev, "Channel error reported, reset channel\n");
> > + return -ECOMM;
> > + }
> > +
> > + if (hdr->length > msg->out_msg_sz + sizeof(hdr->msg_header)) {
> > + dev_err(dev, "Buffer to small\n");
> > + return -ETOOSMALL;
> > + }
> > +
> > + /* Get the data */
> > + msg->out_msg_sz = hdr->length - sizeof(hdr->msg_header);
> > + memcpy(msg->out_msg, hdr->msg_payload, msg->out_msg_sz);
> > +
> > + return 0;
> > +}
> > +
> > +static void clear_smt_channel(struct scmi_shm_buf *shm_buf)
> > +{
> > + struct scmi_smt_header *hdr = (void *)shm_buf->buf;
> > +
> > + hdr->channel_status &= ~SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR;
> > +}
> > +
> > +struct scmi_mbox_channel {
>
> please put structs at top of file
>
> > + struct scmi_shm_buf shm_buf;
> > + struct mbox_chan mbox;
> > + ulong timeout_us;
> > +};
> > +
> > +static int mbox_process_msg(struct udevice *dev, struct scmi_msg *msg)
> > +{
> > + struct scmi_agent *agent = dev_get_priv(dev);
> > + struct scmi_mbox_channel *chan = agent->method_priv;
> > + int rc;
> > +
> > + rc = write_msg_to_smt(dev, &chan->shm_buf, msg);
> > + if (rc)
> > + return rc;
> > +
> > + /* Give shm addr to mbox in case it is meaningful */
> > + rc = mbox_send(&chan->mbox, chan->shm_buf.buf);
> > + if (rc) {
> > + dev_err(dev, "Message send failed: %d\n", rc);
> > + goto out;
> > + }
> > +
> > + /* Receive the response */
> > + rc = mbox_recv(&chan->mbox, chan->shm_buf.buf, chan->timeout_us);
> > + if (rc) {
> > + dev_err(dev, "Response failed: %d, abort\n", rc);
> > + goto out;
> > + }
> > +
> > + rc = read_resp_from_smt(dev, &chan->shm_buf, msg);
> > +
> > +out:
> > + clear_smt_channel(&chan->shm_buf);
> > +
> > + return rc;
> > +}
> > +
> > +struct method_ops mbox_channel_ops = {
> > + .process_msg = mbox_process_msg,
> > +};
> > +
> > +static int probe_mailbox_channel(struct udevice *dev)
> > +{
> > + struct scmi_agent *agent = dev_get_priv(dev);
> > + struct scmi_mbox_channel *chan;
> > + int rc;
> > +
> > + chan = devm_kzalloc(dev, sizeof(*chan), GFP_KERNEL);
> > + if (!chan)
> > + return -ENOMEM;
> > +
> > + chan->timeout_us = TIMEOUT_US_10MS;
> > +
> > + rc = mbox_get_by_index(dev, 0, &chan->mbox);
> > + if (rc) {
> > + dev_err(dev, "Failed to find mailbox: %d\n", rc);
> > + goto out;
> > + }
> > +
> > + rc = get_shm_buffer(dev, &chan->shm_buf);
> > + if (rc)
> > + dev_err(dev, "Failed to get shm resources: %d\n", rc);
> > +
> > +out:
> > + if (rc) {
> > + devm_kfree(dev, chan);
>
> return rc
>
> > + } else {
> > + agent->method_ops = &mbox_channel_ops;
> > + agent->method_priv = (void *)chan;
> > + }
> > +
> > + return rc;
>
> return 0
>
> (to keep the normal non-error flow as the main one)
>
> > +}
> > +
> > +struct scmi_arm_smc_channel {
> > + ulong func_id;
> > + struct scmi_shm_buf shm_buf;
> > +};
> > +
> > +#define SMCCC_RET_NOT_SUPPORTED ((unsigned long)-1)
>
> Please struct and #define decls to top of file.
>
> > +
> > +static int arm_smc_process_msg(struct udevice *dev, struct scmi_msg *msg)
> > +{
> > + struct scmi_agent *agent = dev_get_priv(dev);
> > + struct scmi_arm_smc_channel *chan = agent->method_priv;
> > + struct arm_smccc_res res;
> > + int rc;
> > +
> > + rc = write_msg_to_smt(dev, &chan->shm_buf, msg);
> > + if (rc)
> > + return rc;
> > +
> > + arm_smccc_smc(chan->func_id, 0, 0, 0, 0, 0, 0, 0, &res);
> > + if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
> > + rc = -EINVAL;
>
> ENOTSUP?
ENOTSUP stands for protocols.
Here, it is the communication link that is bugged. Maybe ENXIO?
>
> > + else
> > + rc = read_resp_from_smt(dev, &chan->shm_buf, msg);
> > +
> > + clear_smt_channel(&chan->shm_buf);
> > +
> > + return rc;
> > +}
> > +
> > +struct method_ops arm_smc_channel_ops = {
> > + .process_msg = arm_smc_process_msg,
> > +};
> > +
> > +static int probe_arm_smc_channel(struct udevice *dev)
> > +{
> > + struct scmi_agent *agent = dev_get_priv(dev);
> > + struct scmi_arm_smc_channel *chan;
> > + ofnode node = dev_ofnode(dev);
> > + u32 func_id;
> > + int rc;
> > +
> > + chan = devm_kzalloc(dev, sizeof(*chan), GFP_KERNEL);
> > + if (!chan)
> > + return -ENOMEM;
> > +
> > + if (ofnode_read_u32(node, "arm,smc-id", &func_id)) {
>
> dev_read_u32()
>
> > + dev_err(dev, "Missing property func-id\n");
> > + return -EINVAL;
> > + }
> > +
> > + chan->func_id = func_id;
> > +
> > + rc = get_shm_buffer(dev, &chan->shm_buf);
> > + if (rc) {
> > + dev_err(dev, "Failed to get shm resources: %d\n", rc);
> > + return rc;
> > + }
> > +
> > + agent->method_ops = &arm_smc_channel_ops;
> > + agent->method_priv = (void *)chan;
> > +
> > + return rc;
> > +}
> > +
> > +/*
> > + * Exported functions by the SCMI agent
> > + */
> > +
> > +int scmi_send_and_process_msg(struct udevice *dev, struct scmi_msg *msg)
> > +{
> > + struct scmi_agent *agent = dev_get_priv(dev);
> > +
> > + return agent->method_ops->process_msg(dev, msg);
> > +}
> > +
> > +static int scmi_remove(struct udevice *dev)
> > +{
> > + struct scmi_agent *agent = dev_get_priv(dev);
> > +
> > + if (agent->method_ops->remove_agent)
> > + return agent->method_ops->remove_agent(dev);
>
> method_ops should be the uclass operations. I think you need a new
> uclass or add something to UCLASS_FIRMWARE.
Hmmm, I see you point. I'll consider.
Yet, i think it would be a uclass with a unique type of child: a scmi
agent driver.
Your thoughts?
>
> > +
> > + return 0;
> > +}
> > +
> > +enum scmi_transport_channel {
> > + SCMI_MAILBOX_TRANSPORT,
> > + SCMI_ARM_SMCCC_TRANSPORT,
> > +};
> > +
> > +static int scmi_probe(struct udevice *dev)
> > +{
> > + switch (dev_get_driver_data(dev)) {
> > + case SCMI_MAILBOX_TRANSPORT:
> > + if (IS_ENABLED(CONFIG_DM_MAILBOX))
> > + return probe_mailbox_channel(dev);
> > + break;
> > + case SCMI_ARM_SMCCC_TRANSPORT:
> > + if (IS_ENABLED(CONFIG_ARM_SMCCC))
> > + return probe_arm_smc_channel(dev);
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + return -EINVAL;
> > +}
> > +
> > +static int scmi_bind(struct udevice *dev)
> > +{
> > + int rc = 0;
> > + ofnode node;
> > + struct driver *drv;
> > +
> > + dev_for_each_subnode(node, dev) {
> > + u32 protocol_id;
> > +
> > + if (!ofnode_is_available(node))
> > + continue;
> > +
> > + if (ofnode_read_u32(node, "reg", &protocol_id))
> > + continue;
> > +
> > + switch (protocol_id) {
> > + default:
> > + dev_info(dev, "Ignore unsupported SCMI protocol %u\n",
> > + protocol_id);
> > + continue;
> > + }
> > +
> > + rc = device_bind_ofnode(dev, drv, ofnode_get_name(node),
> > + NULL, node, NULL);
>
> Can we instead add a compatible string to the device tree so driver
> model creates these children automatically?
Here the bound device already belongs to an existing uclass driver, at
least we expect them to.
These are clock, reset, power_domain uclass.
>
> > + if (rc)
> > + break;
> > + }
> > +
> > + if (rc)
> > + device_unbind(dev);
>
> You should not unbind the device within its bind() method. Just make
> sure it refuses to probe.
Ok, thanks.
>
> > +
> > + return rc;
> > +}
> > +
> > +static const struct udevice_id scmi_ids[] = {
> > +#ifdef CONFIG_DM_MAILBOX
> > + { .compatible = "arm,scmi", .data = SCMI_MAILBOX_TRANSPORT },
> > +#endif
> > +#ifdef CONFIG_ARM_SMCCC
> > + { .compatible = "arm,scmi-smc", .data = SCMI_ARM_SMCCC_TRANSPORT },
> > +#endif
>
> Seems like you need a uclass for the transport?
>
> > + { }
> > +};
> > +
> > +U_BOOT_DRIVER(scmi) = {
> > + .name = "scmi",
> > + .id = UCLASS_FIRMWARE,
> > + .of_match = scmi_ids,
> > + .priv_auto_alloc_size = sizeof(struct scmi_agent),
> > + .bind = scmi_bind,
> > + .probe = scmi_probe,
> > + .remove = scmi_remove,
> > + .flags = DM_FLAG_OS_PREPARE,
> > +};
> > diff --git a/include/scmi.h b/include/scmi.h
> > new file mode 100644
> > index 00000000000..e12e322991d
> > --- /dev/null
> > +++ b/include/scmi.h
> > @@ -0,0 +1,82 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause */
> > +/*
> > + * Copyright (c) 2015-2019, Arm Limited and Contributors. All rights reserved.
> > + * Copyright (C) 2019, Linaro Limited
> > + */
> > +#ifndef SCMI_H
> > +#define SCMI_H
> > +
> > +#include <asm/types.h>
> > +
> > +/**
> > + * An SCMI agent represent on communication path from a device driver to
> > + * the remote SCMI server which driver sends messages to and receives
> > + * response messages from.
> > + */
> > +struct scmi_agent;
> > +
> > +enum scmi_std_protocol {
> > + SCMI_PROTOCOL_ID_BASE = 0x10,
> > + SCMI_PROTOCOL_ID_POWER_DOMAIN = 0x11,
> > + SCMI_PROTOCOL_ID_SYSTEM = 0x12,
> > + SCMI_PROTOCOL_ID_PERF = 0x13,
> > + SCMI_PROTOCOL_ID_CLOCK = 0x14,
> > + SCMI_PROTOCOL_ID_SENSOR = 0x15,
> > + SCMI_PROTOCOL_ID_RESET_DOMAIN = 0x16,
> > +};
> > +
> > +enum scmi_status_code {
> > + SCMI_SUCCESS = 0,
> > + SCMI_NOT_SUPPORTED = -1,
> > + SCMI_INVALID_PARAMETERS = -2,
> > + SCMI_DENIED = -3,
> > + SCMI_NOT_FOUND = -4,
> > + SCMI_OUT_OF_RANGE = -5,
> > + SCMI_BUSY = -6,
> > + SCMI_COMMS_ERROR = -7,
> > + SCMI_GENERIC_ERROR = -8,
> > + SCMI_HARDWARE_ERROR = -9,
> > + SCMI_PROTOCOL_ERROR = -10,
> > +};
> > +
> > +/*
> > + * struct scmi_msg - Context of a SCMI message sent and the response received
> > + *
> > + * @protocol_id: SCMI protocol ID
> > + * @message_id: SCMI message ID for a defined protocol ID
> > + * @in_msg: pointer to the message payload sent by the driver
> > + * @in_msg_sz: byte size of the message payload sent
> > + * @out_msg: pointer to buffer to store response message payload
> > + * @out_msg_size: Byte size of the response buffer or payload
>
> sz
>
> > + */
> > +struct scmi_msg {
> > + unsigned int protocol_id;
> > + unsigned int message_id;
>
> Does this need a particular size, e,g. u32?
>
> > + u8 *in_msg;
> > + size_t in_msg_sz;
> > + u8 *out_msg;
> > + size_t out_msg_sz;
> > +};
> > +
> > +/**
> > + * scmi_send_and_process_msg() - send and process a SCMI message
> > + *
> > + * Send a message to a SCMI server through a target SCMI agent device.
> > + * Caller sets scmi_msg::out_msg_sz to the output message buffer size.
> > + * On return, scmi_msg::out_msg_sz stores the response payload size.
> > + *
> > + * @dev: SCMI agent device
> > + * @msg: Message structure reference
> > + * @return 0 on success, a negative errno otherwise
> > + */
> > +int scmi_send_and_process_msg(struct udevice *dev, struct scmi_msg *msg);
> > +
> > +/**
> > + * scmi_to_linux_errno() - Convert an SCMI error code into a Linux errno code
> > + *
> > + * @scmi_errno: SCMI error code value
> > + * @return 0 for successful status and a negative errno otherwise
> > + */
> > +int scmi_to_linux_errno(s32 scmi_errno);
> > +
> > +#endif /* SCMI_H */
> > --
> > 2.17.1
> >
>
> Regards,
> Simon
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/4] firmware: add new driver for SCMI firmwares
2020-08-04 9:33 ` Etienne Carriere
@ 2020-08-16 3:39 ` Simon Glass
0 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2020-08-16 3:39 UTC (permalink / raw)
To: u-boot
Hi Etienne,
On Tue, 4 Aug 2020 at 03:33, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> Hello Simon,
>
> Thanks for the feedback, I'll fix the changes in my v2.
> and sorry for these delayed answers.
>
> On Sun, 26 Jul 2020 at 16:55, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Etienne,
> >
> > On Fri, 17 Jul 2020 at 09:38, Etienne Carriere
> > <etienne.carriere@linaro.org> wrote:
> > >
> > > This change introduces SCMI agent driver in U-Boot in the firmware
> > > U-class.
> > >
> > > SCMI agent driver is designed for platforms that embed a SCMI server in
> > > a firmware hosted for example by a companion co-processor or the secure
> > > world of the executing processor.
> > >
> > > SCMI protocols allow an SCMI agent to discover and access external
> > > resources as clock, reset controllers and many more. SCMI agent and
> > > server communicate following the SCMI specification [1]. SCMI agent
> > > complies with the DT bindings defined in the Linux kernel source tree
> > > regarding SCMI agent description since v5.8-rc1.
> > >
> > > These bindings describe 2 supported message transport layer: using
> > > mailbox uclass devices or using Arm SMC invocation instruction. Both
> > > use a piece or shared memory for message data exchange.
> > >
> > > In the current state, the SCMI agent driver does not bind to any SCMI
> > > protocol to a U-Boot device driver. Former changes will implement
> > > dedicated driver (i.e. an SCMI clock driver or an SCMI reset controller
> > > driver) and add bind supported SCMI protocols in scmi_agent_bind().
> > >
> > > Links: [1] https://developer.arm.com/architectures/system-architectures/software-standards/scmi
> > > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> > > ---
> > >
> > > drivers/firmware/Kconfig | 15 ++
> > > drivers/firmware/Makefile | 1 +
> > > drivers/firmware/scmi.c | 439 ++++++++++++++++++++++++++++++++++++++
> > > include/scmi.h | 82 +++++++
> > > 4 files changed, 537 insertions(+)
> > > create mode 100644 drivers/firmware/scmi.c
> > > create mode 100644 include/scmi.h
> > >
> > > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> > > index b70a2063551..f7c7ee7a5aa 100644
> > > --- a/drivers/firmware/Kconfig
> > > +++ b/drivers/firmware/Kconfig
> > > @@ -1,6 +1,21 @@
> > > config FIRMWARE
> > > bool "Enable Firmware driver support"
> > >
> > > +config SCMI_FIRMWARE
> > > + bool "Enable SCMI support"
> > > + select FIRMWARE
> > > + select OF_TRANSLATE
> > > + depends on DM_MAILBOX || ARM_SMCCC
> > > + help
> > > + An SCMI agent communicates with a related SCMI server firmware
> >
> > Please write out SCMI in full somewhere and add a link to the spec.
> >
> > > + located in another sub-system, as a companion micro controller
> > > + or a companion host in the CPU system.
> > > +
> > > + Communications between agent (client) and the SCMI server are
> > > + based on message exchange. Messages can be exchange over tranport
> > > + channels as a mailbox device or an Arm SMCCC service with some
> > > + piece of identified shared memory.
> > > +
> > > config SPL_FIRMWARE
> > > bool "Enable Firmware driver support in SPL"
> > > depends on FIRMWARE
> > > diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
> > > index a0c250a473e..3965838179f 100644
> > > --- a/drivers/firmware/Makefile
> > > +++ b/drivers/firmware/Makefile
> > > @@ -2,4 +2,5 @@ obj-$(CONFIG_FIRMWARE) += firmware-uclass.o
> > > obj-$(CONFIG_$(SPL_)ARM_PSCI_FW) += psci.o
> > > obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o
> > > obj-$(CONFIG_SANDBOX) += firmware-sandbox.o
> > > +obj-$(CONFIG_SCMI_FIRMWARE) += scmi.o
> > > obj-$(CONFIG_ZYNQMP_FIRMWARE) += firmware-zynqmp.o
> > > diff --git a/drivers/firmware/scmi.c b/drivers/firmware/scmi.c
> > > new file mode 100644
> > > index 00000000000..fa8a91c3f3d
> > > --- /dev/null
> > > +++ b/drivers/firmware/scmi.c
> > > @@ -0,0 +1,439 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright (c) 2015-2019, Arm Limited and Contributors. All rights reserved.
> > > + * Copyright (C) 2019-2020 Linaro Limited.
> > > + */
> > > +
> > > +#include <common.h>
> > > +#include <cpu_func.h>
> > > +#include <dm.h>
> > > +#include <errno.h>
> > > +#include <mailbox.h>
> > > +#include <memalign.h>
> > > +#include <scmi.h>
> > > +#include <asm/system.h>
> > > +#include <asm/types.h>
> > > +#include <dm/device-internal.h>
> > > +#include <dm/devres.h>
> > > +#include <dm/lists.h>
> > > +#include <dm/ofnode.h>
> > > +#include <linux/arm-smccc.h>
> > > +#include <linux/compat.h>
> > > +#include <linux/errno.h>
> > > +#include <linux/io.h>
> > > +#include <linux/ioport.h>
> > > +
> > > +#define TIMEOUT_US_10MS 10000
> > > +
> > > +struct error_code {
> >
> > Function comment
> >
> > > + int scmi;
> > > + int errno;
> > > +};
> > > +
> > > +static const struct error_code scmi_linux_errmap[] = {
> > > + { .scmi = SCMI_NOT_SUPPORTED, .errno = -EOPNOTSUPP, },
> > > + { .scmi = SCMI_INVALID_PARAMETERS, .errno = -EINVAL, },
> > > + { .scmi = SCMI_DENIED, .errno = -EACCES, },
> > > + { .scmi = SCMI_NOT_FOUND, .errno = -ENOENT, },
> > > + { .scmi = SCMI_OUT_OF_RANGE, .errno = -ERANGE, },
> > > + { .scmi = SCMI_BUSY, .errno = -EBUSY, },
> > > + { .scmi = SCMI_COMMS_ERROR, .errno = -ECOMM, },
> > > + { .scmi = SCMI_GENERIC_ERROR, .errno = -EIO, },
> > > + { .scmi = SCMI_HARDWARE_ERROR, .errno = -EREMOTEIO, },
> > > + { .scmi = SCMI_PROTOCOL_ERROR, .errno = -EPROTO, },
> > > +};
> > > +
> > > +int scmi_to_linux_errno(s32 scmi_code)
> > > +{
> > > + int n;
> > > +
> > > + if (scmi_code == 0)
> > > + return 0;
> > > +
> > > + for (n = 0; n < ARRAY_SIZE(scmi_linux_errmap); n++)
> > > + if (scmi_code == scmi_linux_errmap[n].scmi)
> > > + return scmi_linux_errmap[1].errno;
> > > +
> > > + return -EPROTO;
> > > +}
> > > +
> > > +struct method_ops {
> > > + int (*process_msg)(struct udevice *dev, struct scmi_msg *msg);
> > > + int (*remove_agent)(struct udevice *dev);
> >
> > Looks like this should be a new uclass and child driver?
>
> I think a uclass is overkilling here.
> A scmi agent device is a firmware driver that binds to devices of
> other uclasses (clock, reset, ...).
> When scmi agent device is probed to get the related clock, reset, ...
> devices be probed.
> So it does not need any child.
> (see also related comment below)
I'm not sure I agree, but let's see how it turns out.
>
>
> >
> > Also we should have a sandbox implementation so this code can be tested.
>
> Ok, I understand the sandbox test stuff.
> Should it be part of the v2 this series, can it be done in parallel,
> or a later v<N>?
I think it should be part of the series. New code should have tests.
[..]
> > > +
> > > +static int arm_smc_process_msg(struct udevice *dev, struct scmi_msg *msg)
> > > +{
> > > + struct scmi_agent *agent = dev_get_priv(dev);
> > > + struct scmi_arm_smc_channel *chan = agent->method_priv;
> > > + struct arm_smccc_res res;
> > > + int rc;
> > > +
> > > + rc = write_msg_to_smt(dev, &chan->shm_buf, msg);
> > > + if (rc)
> > > + return rc;
> > > +
> > > + arm_smccc_smc(chan->func_id, 0, 0, 0, 0, 0, 0, 0, &res);
> > > + if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
> > > + rc = -EINVAL;
> >
> > ENOTSUP?
>
> ENOTSUP stands for protocols.
> Here, it is the communication link that is bugged. Maybe ENXIO?
OK...just want to avoid -EINVAL as it is so widely used for
device-tree reading; it is too generic.
> > > +static int scmi_remove(struct udevice *dev)
> > > +{
> > > + struct scmi_agent *agent = dev_get_priv(dev);
> > > +
> > > + if (agent->method_ops->remove_agent)
> > > + return agent->method_ops->remove_agent(dev);
> >
> > method_ops should be the uclass operations. I think you need a new
> > uclass or add something to UCLASS_FIRMWARE.
>
> Hmmm, I see you point. I'll consider.
> Yet, i think it would be a uclass with a unique type of child: a scmi
> agent driver.
> Your thoughts?
Yes that's right.
>
>
>
> >
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +enum scmi_transport_channel {
> > > + SCMI_MAILBOX_TRANSPORT,
> > > + SCMI_ARM_SMCCC_TRANSPORT,
> > > +};
> > > +
> > > +static int scmi_probe(struct udevice *dev)
> > > +{
> > > + switch (dev_get_driver_data(dev)) {
> > > + case SCMI_MAILBOX_TRANSPORT:
> > > + if (IS_ENABLED(CONFIG_DM_MAILBOX))
> > > + return probe_mailbox_channel(dev);
> > > + break;
> > > + case SCMI_ARM_SMCCC_TRANSPORT:
> > > + if (IS_ENABLED(CONFIG_ARM_SMCCC))
> > > + return probe_arm_smc_channel(dev);
> > > + break;
> > > + default:
> > > + break;
> > > + }
> > > +
> > > + return -EINVAL;
> > > +}
> > > +
> > > +static int scmi_bind(struct udevice *dev)
> > > +{
> > > + int rc = 0;
> > > + ofnode node;
> > > + struct driver *drv;
> > > +
> > > + dev_for_each_subnode(node, dev) {
> > > + u32 protocol_id;
> > > +
> > > + if (!ofnode_is_available(node))
> > > + continue;
> > > +
> > > + if (ofnode_read_u32(node, "reg", &protocol_id))
> > > + continue;
> > > +
> > > + switch (protocol_id) {
> > > + default:
> > > + dev_info(dev, "Ignore unsupported SCMI protocol %u\n",
> > > + protocol_id);
> > > + continue;
> > > + }
> > > +
> > > + rc = device_bind_ofnode(dev, drv, ofnode_get_name(node),
> > > + NULL, node, NULL);
> >
> > Can we instead add a compatible string to the device tree so driver
> > model creates these children automatically?
>
> Here the bound device already belongs to an existing uclass driver, at
> least we expect them to.
> These are clock, reset, power_domain uclass.
Right, but if the child nodes have compatible strings, driver model
will automatically bind the children and you don't need to write this
code.
[..]
Regards,
Simon
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/4] clk: add clock driver for SCMI agents
2020-08-18 8:46 ` Etienne Carriere
@ 2020-08-18 8:51 ` Sudeep Holla
0 siblings, 0 replies; 14+ messages in thread
From: Sudeep Holla @ 2020-08-18 8:51 UTC (permalink / raw)
To: u-boot
On Tue, Aug 18, 2020 at 10:46:29AM +0200, Etienne Carriere wrote:
> Hello Simon and all,
>
> On Sun, 26 Jul 2020 at 16:54, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Etienne,
> >
> > On Fri, 17 Jul 2020 at 09:43, Etienne Carriere
> > <etienne.carriere@linaro.org> wrote:
> > >
> > > This change introduces a clock driver for SCMI agent devices. When
> > > SCMI agent and SCMI clock drivers are enabled, SCMI agent binds a
> > > clock device for each SCMI clock protocol devices enabled in the FDT.
> > >
> > > SCMI clock driver is embedded upon CONFIG_CLK_SCMI=y. If enabled,
> > > CONFIG_SCMI_AGENT is also enabled.
> > >
> > > SCMI Clock protocol is defined in the SCMI specification [1].
> > >
> > > Links: [1] https://developer.arm.com/architectures/system-architectures/software-standards/scmi
> >
> > That doesn't seem to work for me (404 error)
>
> I think that Arm site was broken when you tried. Works fine and this
> seems to be the official Arm URL for the SCMI specification.
>
Hopefully this works, I have this in one of the kernel doc.
https://developer.arm.com/documentation/den0056/latest
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/4] clk: add clock driver for SCMI agents
2020-07-26 14:54 ` Simon Glass
@ 2020-08-18 8:46 ` Etienne Carriere
2020-08-18 8:51 ` Sudeep Holla
0 siblings, 1 reply; 14+ messages in thread
From: Etienne Carriere @ 2020-08-18 8:46 UTC (permalink / raw)
To: u-boot
Hello Simon and all,
On Sun, 26 Jul 2020 at 16:54, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Etienne,
>
> On Fri, 17 Jul 2020 at 09:43, Etienne Carriere
> <etienne.carriere@linaro.org> wrote:
> >
> > This change introduces a clock driver for SCMI agent devices. When
> > SCMI agent and SCMI clock drivers are enabled, SCMI agent binds a
> > clock device for each SCMI clock protocol devices enabled in the FDT.
> >
> > SCMI clock driver is embedded upon CONFIG_CLK_SCMI=y. If enabled,
> > CONFIG_SCMI_AGENT is also enabled.
> >
> > SCMI Clock protocol is defined in the SCMI specification [1].
> >
> > Links: [1] https://developer.arm.com/architectures/system-architectures/software-standards/scmi
>
> That doesn't seem to work for me (404 error)
I think that Arm site was broken when you tried. Works fine and this
seems to be the official Arm URL for the SCMI specification.
>
> > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> > ---
> >
> > drivers/clk/Kconfig | 8 +++
> > drivers/clk/Makefile | 1 +
> > drivers/clk/clk_scmi.c | 152 ++++++++++++++++++++++++++++++++++++++++
> > drivers/firmware/scmi.c | 3 +
> > 4 files changed, 164 insertions(+)
> > create mode 100644 drivers/clk/clk_scmi.c
> >
> > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> > index 82cb1874e19..234d6035202 100644
> > --- a/drivers/clk/Kconfig
> > +++ b/drivers/clk/Kconfig
> > @@ -152,6 +152,14 @@ config CLK_CDCE9XX
> > Enable the clock synthesizer driver for CDCE913/925/937/949
> > series of chips.
> >
> > +config CLK_SCMI
> > + bool "Enable SCMI clock driver"
> > + select SCMI_FIRMWARE
>
> perhaps 'depends on' would be better?
Ok
>
> > + help
> > + Enable this option if you want to support clock devices exposed
> > + by a SCMI agent based on SCMI clock protocol communication
> > + with a SCMI server.
> > +
> > source "drivers/clk/analogbits/Kconfig"
> > source "drivers/clk/at91/Kconfig"
> > source "drivers/clk/exynos/Kconfig"
> > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> > index d9119545810..76bba77d1f0 100644
> > --- a/drivers/clk/Makefile
> > +++ b/drivers/clk/Makefile
> > @@ -31,6 +31,7 @@ obj-$(CONFIG_CLK_K210) += kendryte/
> > obj-$(CONFIG_CLK_MPC83XX) += mpc83xx_clk.o
> > obj-$(CONFIG_CLK_OWL) += owl/
> > obj-$(CONFIG_CLK_RENESAS) += renesas/
> > +obj-$(CONFIG_CLK_SCMI) += clk_scmi.o
> > obj-$(CONFIG_CLK_SIFIVE) += sifive/
> > obj-$(CONFIG_ARCH_SUNXI) += sunxi/
> > obj-$(CONFIG_CLK_STM32F) += clk_stm32f.o
> > diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
> > new file mode 100644
> > index 00000000000..efe64a6a38f
> > --- /dev/null
> > +++ b/drivers/clk/clk_scmi.c
> > @@ -0,0 +1,152 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2019-2020 Linaro Limited
> > + */
> > +#include <common.h>
> > +#include <clk-uclass.h>
> > +#include <dm.h>
> > +#include <scmi.h>
> > +#include <asm/types.h>
> > +
> > +enum scmi_clock_message_id {
> > + SCMI_CLOCK_RATE_SET = 0x5,
> > + SCMI_CLOCK_RATE_GET = 0x6,
> > + SCMI_CLOCK_CONFIG_SET = 0x7,
> > +};
> > +
> > +#define SCMI_CLK_RATE_ASYNC_NOTIFY BIT(0)
> > +#define SCMI_CLK_RATE_ASYNC_NORESP (BIT(0) | BIT(1))
> > +#define SCMI_CLK_RATE_ROUND_DOWN 0
> > +#define SCMI_CLK_RATE_ROUND_UP BIT(2)
> > +#define SCMI_CLK_RATE_ROUND_CLOSEST BIT(3)
> > +
> > +struct scmi_clk_state_in {
> > + u32 clock_id;
> > + u32 attributes;
> > +};
> > +
> > +struct scmi_clk_state_out {
> > + s32 status;
> > +};
> > +
> > +static int scmi_clk_gate(struct clk *clk, int enable)
> > +{
> > + struct scmi_clk_state_in in = {
> > + .clock_id = clk->id,
> > + .attributes = enable,
> > + };
> > + struct scmi_clk_state_out out;
> > + struct scmi_msg scmi_msg = {
> > + .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
> > + .message_id = SCMI_CLOCK_CONFIG_SET,
> > + .in_msg = (u8 *)&in,
> > + .in_msg_sz = sizeof(in),
> > + .out_msg = (u8 *)&out,
> > + .out_msg_sz = sizeof(out),
> > + };
> > + int rc;
>
> Again please use 'ret'.
Sure, will do.
>
> > +
> > + rc = scmi_send_and_process_msg(clk->dev->parent, &scmi_msg);
> > + if (rc)
> > + return rc;
> > +
> > + return scmi_to_linux_errno(out.status);
> > +}
> > +
> > +static int scmi_clk_enable(struct clk *clk)
> > +{
> > + return scmi_clk_gate(clk, 1);
> > +}
> > +
> > +static int scmi_clk_disable(struct clk *clk)
> > +{
> > + return scmi_clk_gate(clk, 0);
> > +}
> > +
> > +struct scmi_clk_rate_get_in {
> > + u32 clock_id;
> > +};
> > +
> > +struct scmi_clk_rate_get_out {
> > + s32 status;
> > + u32 rate_lsb;
> > + u32 rate_msb;
> > +};
> > +
> > +static ulong scmi_clk_get_rate(struct clk *clk)
> > +{
> > + struct scmi_clk_rate_get_in in = {
> > + .clock_id = clk->id,
> > + };
> > + struct scmi_clk_rate_get_out out;
> > + struct scmi_msg scmi_msg = {
> > + .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
> > + .message_id = SCMI_CLOCK_RATE_GET,
> > + .in_msg = (u8 *)&in,
> > + .in_msg_sz = sizeof(in),
> > + .out_msg = (u8 *)&out,
> > + .out_msg_sz = sizeof(out),
> > + };
> > + int rc;
> > +
> > + rc = scmi_send_and_process_msg(clk->dev->parent, &scmi_msg);
> > + if (rc)
> > + return 0;
>
> Why not rc?
True, thanks.
>
> > +
> > + rc = scmi_to_linux_errno(out.status);
> > + if (rc)
> > + return 0;
> > +
> > + return (ulong)(((u64)out.rate_msb << 32) | out.rate_lsb);
>
> This is odd. Can you use ulong instead of u64 to avoid two casts?
A ulong would fit on a 64bit machine but not on a 32bit as compiler
will likely complain that the '<< 32' operation is meaningless.
>
> > +}
> > +
> > +struct scmi_clk_rate_set_in {
>
> struct comment. I think these need to go in some sort of protocol header.
These structures are used only by this very SCMI clock driver hence I
don't think it deserves exposure in a shared header file.
>
> > + u32 clock_id;
> > + u32 flags;
> > + u32 rate_lsb;
> > + u32 rate_msb;
> > +};
> > +
> > +struct scmi_clk_rate_set_out {
> > + s32 status;
> > +};
> > +
> > +static ulong scmi_clk_set_rate(struct clk *clk, ulong rate)
> > +{
> > + struct scmi_clk_rate_set_in in = {
> > + .clock_id = clk->id,
> > + .flags = SCMI_CLK_RATE_ASYNC_NORESP |
> > + SCMI_CLK_RATE_ROUND_CLOSEST,
> > + .rate_lsb = (u32)rate,
> > + .rate_msb = (u32)((u64)rate >> 32),
> > + };
> > + struct scmi_clk_rate_set_out out;
> > + struct scmi_msg scmi_msg = {
> > + .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
> > + .message_id = SCMI_CLOCK_RATE_SET,
> > + .in_msg = (u8 *)&in,
> > + .in_msg_sz = sizeof(in),
> > + .out_msg = (u8 *)&out,
> > + .out_msg_sz = sizeof(out),
> > + };
> > + int rc;
> > +
> > + rc = scmi_send_and_process_msg(clk->dev->parent, &scmi_msg);
> > + if (rc)
> > + return 0;
> > +
> > + return scmi_to_linux_errno(out.status);
> > +}
> > +
> > +static const struct clk_ops scmi_clk_ops = {
> > + .enable = scmi_clk_enable,
> > + .disable = scmi_clk_disable,
> > + .get_rate = scmi_clk_get_rate,
> > + .set_rate = scmi_clk_set_rate,
> > +};
> > +
> > +U_BOOT_DRIVER(scmi_clock) = {
> > + .name = "scmi_clk",
> > + .id = UCLASS_CLK,
> > + .ops = &scmi_clk_ops,
> > +};
> > diff --git a/drivers/firmware/scmi.c b/drivers/firmware/scmi.c
> > index fa8a91c3f3d..9f06718df51 100644
> > --- a/drivers/firmware/scmi.c
> > +++ b/drivers/firmware/scmi.c
> > @@ -399,6 +399,9 @@ static int scmi_bind(struct udevice *dev)
> > continue;
> >
> > switch (protocol_id) {
> > + case SCMI_PROTOCOL_ID_CLOCK:
> > + drv = DM_GET_DRIVER(scmi_clock);
> > + break;
> > default:
> > dev_info(dev, "Ignore unsupported SCMI protocol %u\n",
> > protocol_id);
> > --
> > 2.17.1
> >
>
> Can you add a sandbox test for this?
Ok I'm looking at that.
>
> Regards,
> Simon
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/4] clk: add clock driver for SCMI agents
2020-07-17 15:42 ` [PATCH 3/4] clk: add clock driver for SCMI agents Etienne Carriere
@ 2020-07-26 14:54 ` Simon Glass
2020-08-18 8:46 ` Etienne Carriere
0 siblings, 1 reply; 14+ messages in thread
From: Simon Glass @ 2020-07-26 14:54 UTC (permalink / raw)
To: u-boot
Hi Etienne,
On Fri, 17 Jul 2020 at 09:43, Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> This change introduces a clock driver for SCMI agent devices. When
> SCMI agent and SCMI clock drivers are enabled, SCMI agent binds a
> clock device for each SCMI clock protocol devices enabled in the FDT.
>
> SCMI clock driver is embedded upon CONFIG_CLK_SCMI=y. If enabled,
> CONFIG_SCMI_AGENT is also enabled.
>
> SCMI Clock protocol is defined in the SCMI specification [1].
>
> Links: [1] https://developer.arm.com/architectures/system-architectures/software-standards/scmi
That doesn't seem to work for me (404 error)
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
>
> drivers/clk/Kconfig | 8 +++
> drivers/clk/Makefile | 1 +
> drivers/clk/clk_scmi.c | 152 ++++++++++++++++++++++++++++++++++++++++
> drivers/firmware/scmi.c | 3 +
> 4 files changed, 164 insertions(+)
> create mode 100644 drivers/clk/clk_scmi.c
>
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 82cb1874e19..234d6035202 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -152,6 +152,14 @@ config CLK_CDCE9XX
> Enable the clock synthesizer driver for CDCE913/925/937/949
> series of chips.
>
> +config CLK_SCMI
> + bool "Enable SCMI clock driver"
> + select SCMI_FIRMWARE
perhaps 'depends on' would be better?
> + help
> + Enable this option if you want to support clock devices exposed
> + by a SCMI agent based on SCMI clock protocol communication
> + with a SCMI server.
> +
> source "drivers/clk/analogbits/Kconfig"
> source "drivers/clk/at91/Kconfig"
> source "drivers/clk/exynos/Kconfig"
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index d9119545810..76bba77d1f0 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_CLK_K210) += kendryte/
> obj-$(CONFIG_CLK_MPC83XX) += mpc83xx_clk.o
> obj-$(CONFIG_CLK_OWL) += owl/
> obj-$(CONFIG_CLK_RENESAS) += renesas/
> +obj-$(CONFIG_CLK_SCMI) += clk_scmi.o
> obj-$(CONFIG_CLK_SIFIVE) += sifive/
> obj-$(CONFIG_ARCH_SUNXI) += sunxi/
> obj-$(CONFIG_CLK_STM32F) += clk_stm32f.o
> diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
> new file mode 100644
> index 00000000000..efe64a6a38f
> --- /dev/null
> +++ b/drivers/clk/clk_scmi.c
> @@ -0,0 +1,152 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019-2020 Linaro Limited
> + */
> +#include <common.h>
> +#include <clk-uclass.h>
> +#include <dm.h>
> +#include <scmi.h>
> +#include <asm/types.h>
> +
> +enum scmi_clock_message_id {
> + SCMI_CLOCK_RATE_SET = 0x5,
> + SCMI_CLOCK_RATE_GET = 0x6,
> + SCMI_CLOCK_CONFIG_SET = 0x7,
> +};
> +
> +#define SCMI_CLK_RATE_ASYNC_NOTIFY BIT(0)
> +#define SCMI_CLK_RATE_ASYNC_NORESP (BIT(0) | BIT(1))
> +#define SCMI_CLK_RATE_ROUND_DOWN 0
> +#define SCMI_CLK_RATE_ROUND_UP BIT(2)
> +#define SCMI_CLK_RATE_ROUND_CLOSEST BIT(3)
> +
> +struct scmi_clk_state_in {
> + u32 clock_id;
> + u32 attributes;
> +};
> +
> +struct scmi_clk_state_out {
> + s32 status;
> +};
> +
> +static int scmi_clk_gate(struct clk *clk, int enable)
> +{
> + struct scmi_clk_state_in in = {
> + .clock_id = clk->id,
> + .attributes = enable,
> + };
> + struct scmi_clk_state_out out;
> + struct scmi_msg scmi_msg = {
> + .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
> + .message_id = SCMI_CLOCK_CONFIG_SET,
> + .in_msg = (u8 *)&in,
> + .in_msg_sz = sizeof(in),
> + .out_msg = (u8 *)&out,
> + .out_msg_sz = sizeof(out),
> + };
> + int rc;
Again please use 'ret'.
> +
> + rc = scmi_send_and_process_msg(clk->dev->parent, &scmi_msg);
> + if (rc)
> + return rc;
> +
> + return scmi_to_linux_errno(out.status);
> +}
> +
> +static int scmi_clk_enable(struct clk *clk)
> +{
> + return scmi_clk_gate(clk, 1);
> +}
> +
> +static int scmi_clk_disable(struct clk *clk)
> +{
> + return scmi_clk_gate(clk, 0);
> +}
> +
> +struct scmi_clk_rate_get_in {
> + u32 clock_id;
> +};
> +
> +struct scmi_clk_rate_get_out {
> + s32 status;
> + u32 rate_lsb;
> + u32 rate_msb;
> +};
> +
> +static ulong scmi_clk_get_rate(struct clk *clk)
> +{
> + struct scmi_clk_rate_get_in in = {
> + .clock_id = clk->id,
> + };
> + struct scmi_clk_rate_get_out out;
> + struct scmi_msg scmi_msg = {
> + .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
> + .message_id = SCMI_CLOCK_RATE_GET,
> + .in_msg = (u8 *)&in,
> + .in_msg_sz = sizeof(in),
> + .out_msg = (u8 *)&out,
> + .out_msg_sz = sizeof(out),
> + };
> + int rc;
> +
> + rc = scmi_send_and_process_msg(clk->dev->parent, &scmi_msg);
> + if (rc)
> + return 0;
Why not rc?
> +
> + rc = scmi_to_linux_errno(out.status);
> + if (rc)
> + return 0;
> +
> + return (ulong)(((u64)out.rate_msb << 32) | out.rate_lsb);
This is odd. Can you use ulong instead of u64 to avoid two casts?
> +}
> +
> +struct scmi_clk_rate_set_in {
struct comment. I think these need to go in some sort of protocol header.
> + u32 clock_id;
> + u32 flags;
> + u32 rate_lsb;
> + u32 rate_msb;
> +};
> +
> +struct scmi_clk_rate_set_out {
> + s32 status;
> +};
> +
> +static ulong scmi_clk_set_rate(struct clk *clk, ulong rate)
> +{
> + struct scmi_clk_rate_set_in in = {
> + .clock_id = clk->id,
> + .flags = SCMI_CLK_RATE_ASYNC_NORESP |
> + SCMI_CLK_RATE_ROUND_CLOSEST,
> + .rate_lsb = (u32)rate,
> + .rate_msb = (u32)((u64)rate >> 32),
> + };
> + struct scmi_clk_rate_set_out out;
> + struct scmi_msg scmi_msg = {
> + .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
> + .message_id = SCMI_CLOCK_RATE_SET,
> + .in_msg = (u8 *)&in,
> + .in_msg_sz = sizeof(in),
> + .out_msg = (u8 *)&out,
> + .out_msg_sz = sizeof(out),
> + };
> + int rc;
> +
> + rc = scmi_send_and_process_msg(clk->dev->parent, &scmi_msg);
> + if (rc)
> + return 0;
> +
> + return scmi_to_linux_errno(out.status);
> +}
> +
> +static const struct clk_ops scmi_clk_ops = {
> + .enable = scmi_clk_enable,
> + .disable = scmi_clk_disable,
> + .get_rate = scmi_clk_get_rate,
> + .set_rate = scmi_clk_set_rate,
> +};
> +
> +U_BOOT_DRIVER(scmi_clock) = {
> + .name = "scmi_clk",
> + .id = UCLASS_CLK,
> + .ops = &scmi_clk_ops,
> +};
> diff --git a/drivers/firmware/scmi.c b/drivers/firmware/scmi.c
> index fa8a91c3f3d..9f06718df51 100644
> --- a/drivers/firmware/scmi.c
> +++ b/drivers/firmware/scmi.c
> @@ -399,6 +399,9 @@ static int scmi_bind(struct udevice *dev)
> continue;
>
> switch (protocol_id) {
> + case SCMI_PROTOCOL_ID_CLOCK:
> + drv = DM_GET_DRIVER(scmi_clock);
> + break;
> default:
> dev_info(dev, "Ignore unsupported SCMI protocol %u\n",
> protocol_id);
> --
> 2.17.1
>
Can you add a sandbox test for this?
Regards,
Simon
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/4] clk: add clock driver for SCMI agents
2020-07-17 15:42 Etienne Carriere
@ 2020-07-17 15:42 ` Etienne Carriere
2020-07-26 14:54 ` Simon Glass
0 siblings, 1 reply; 14+ messages in thread
From: Etienne Carriere @ 2020-07-17 15:42 UTC (permalink / raw)
To: u-boot
This change introduces a clock driver for SCMI agent devices. When
SCMI agent and SCMI clock drivers are enabled, SCMI agent binds a
clock device for each SCMI clock protocol devices enabled in the FDT.
SCMI clock driver is embedded upon CONFIG_CLK_SCMI=y. If enabled,
CONFIG_SCMI_AGENT is also enabled.
SCMI Clock protocol is defined in the SCMI specification [1].
Links: [1] https://developer.arm.com/architectures/system-architectures/software-standards/scmi
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
drivers/clk/Kconfig | 8 +++
drivers/clk/Makefile | 1 +
drivers/clk/clk_scmi.c | 152 ++++++++++++++++++++++++++++++++++++++++
drivers/firmware/scmi.c | 3 +
4 files changed, 164 insertions(+)
create mode 100644 drivers/clk/clk_scmi.c
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 82cb1874e19..234d6035202 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -152,6 +152,14 @@ config CLK_CDCE9XX
Enable the clock synthesizer driver for CDCE913/925/937/949
series of chips.
+config CLK_SCMI
+ bool "Enable SCMI clock driver"
+ select SCMI_FIRMWARE
+ help
+ Enable this option if you want to support clock devices exposed
+ by a SCMI agent based on SCMI clock protocol communication
+ with a SCMI server.
+
source "drivers/clk/analogbits/Kconfig"
source "drivers/clk/at91/Kconfig"
source "drivers/clk/exynos/Kconfig"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index d9119545810..76bba77d1f0 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_CLK_K210) += kendryte/
obj-$(CONFIG_CLK_MPC83XX) += mpc83xx_clk.o
obj-$(CONFIG_CLK_OWL) += owl/
obj-$(CONFIG_CLK_RENESAS) += renesas/
+obj-$(CONFIG_CLK_SCMI) += clk_scmi.o
obj-$(CONFIG_CLK_SIFIVE) += sifive/
obj-$(CONFIG_ARCH_SUNXI) += sunxi/
obj-$(CONFIG_CLK_STM32F) += clk_stm32f.o
diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
new file mode 100644
index 00000000000..efe64a6a38f
--- /dev/null
+++ b/drivers/clk/clk_scmi.c
@@ -0,0 +1,152 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019-2020 Linaro Limited
+ */
+#include <common.h>
+#include <clk-uclass.h>
+#include <dm.h>
+#include <scmi.h>
+#include <asm/types.h>
+
+enum scmi_clock_message_id {
+ SCMI_CLOCK_RATE_SET = 0x5,
+ SCMI_CLOCK_RATE_GET = 0x6,
+ SCMI_CLOCK_CONFIG_SET = 0x7,
+};
+
+#define SCMI_CLK_RATE_ASYNC_NOTIFY BIT(0)
+#define SCMI_CLK_RATE_ASYNC_NORESP (BIT(0) | BIT(1))
+#define SCMI_CLK_RATE_ROUND_DOWN 0
+#define SCMI_CLK_RATE_ROUND_UP BIT(2)
+#define SCMI_CLK_RATE_ROUND_CLOSEST BIT(3)
+
+struct scmi_clk_state_in {
+ u32 clock_id;
+ u32 attributes;
+};
+
+struct scmi_clk_state_out {
+ s32 status;
+};
+
+static int scmi_clk_gate(struct clk *clk, int enable)
+{
+ struct scmi_clk_state_in in = {
+ .clock_id = clk->id,
+ .attributes = enable,
+ };
+ struct scmi_clk_state_out out;
+ struct scmi_msg scmi_msg = {
+ .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
+ .message_id = SCMI_CLOCK_CONFIG_SET,
+ .in_msg = (u8 *)&in,
+ .in_msg_sz = sizeof(in),
+ .out_msg = (u8 *)&out,
+ .out_msg_sz = sizeof(out),
+ };
+ int rc;
+
+ rc = scmi_send_and_process_msg(clk->dev->parent, &scmi_msg);
+ if (rc)
+ return rc;
+
+ return scmi_to_linux_errno(out.status);
+}
+
+static int scmi_clk_enable(struct clk *clk)
+{
+ return scmi_clk_gate(clk, 1);
+}
+
+static int scmi_clk_disable(struct clk *clk)
+{
+ return scmi_clk_gate(clk, 0);
+}
+
+struct scmi_clk_rate_get_in {
+ u32 clock_id;
+};
+
+struct scmi_clk_rate_get_out {
+ s32 status;
+ u32 rate_lsb;
+ u32 rate_msb;
+};
+
+static ulong scmi_clk_get_rate(struct clk *clk)
+{
+ struct scmi_clk_rate_get_in in = {
+ .clock_id = clk->id,
+ };
+ struct scmi_clk_rate_get_out out;
+ struct scmi_msg scmi_msg = {
+ .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
+ .message_id = SCMI_CLOCK_RATE_GET,
+ .in_msg = (u8 *)&in,
+ .in_msg_sz = sizeof(in),
+ .out_msg = (u8 *)&out,
+ .out_msg_sz = sizeof(out),
+ };
+ int rc;
+
+ rc = scmi_send_and_process_msg(clk->dev->parent, &scmi_msg);
+ if (rc)
+ return 0;
+
+ rc = scmi_to_linux_errno(out.status);
+ if (rc)
+ return 0;
+
+ return (ulong)(((u64)out.rate_msb << 32) | out.rate_lsb);
+}
+
+struct scmi_clk_rate_set_in {
+ u32 clock_id;
+ u32 flags;
+ u32 rate_lsb;
+ u32 rate_msb;
+};
+
+struct scmi_clk_rate_set_out {
+ s32 status;
+};
+
+static ulong scmi_clk_set_rate(struct clk *clk, ulong rate)
+{
+ struct scmi_clk_rate_set_in in = {
+ .clock_id = clk->id,
+ .flags = SCMI_CLK_RATE_ASYNC_NORESP |
+ SCMI_CLK_RATE_ROUND_CLOSEST,
+ .rate_lsb = (u32)rate,
+ .rate_msb = (u32)((u64)rate >> 32),
+ };
+ struct scmi_clk_rate_set_out out;
+ struct scmi_msg scmi_msg = {
+ .protocol_id = SCMI_PROTOCOL_ID_CLOCK,
+ .message_id = SCMI_CLOCK_RATE_SET,
+ .in_msg = (u8 *)&in,
+ .in_msg_sz = sizeof(in),
+ .out_msg = (u8 *)&out,
+ .out_msg_sz = sizeof(out),
+ };
+ int rc;
+
+ rc = scmi_send_and_process_msg(clk->dev->parent, &scmi_msg);
+ if (rc)
+ return 0;
+
+ return scmi_to_linux_errno(out.status);
+}
+
+static const struct clk_ops scmi_clk_ops = {
+ .enable = scmi_clk_enable,
+ .disable = scmi_clk_disable,
+ .get_rate = scmi_clk_get_rate,
+ .set_rate = scmi_clk_set_rate,
+};
+
+U_BOOT_DRIVER(scmi_clock) = {
+ .name = "scmi_clk",
+ .id = UCLASS_CLK,
+ .ops = &scmi_clk_ops,
+};
diff --git a/drivers/firmware/scmi.c b/drivers/firmware/scmi.c
index fa8a91c3f3d..9f06718df51 100644
--- a/drivers/firmware/scmi.c
+++ b/drivers/firmware/scmi.c
@@ -399,6 +399,9 @@ static int scmi_bind(struct udevice *dev)
continue;
switch (protocol_id) {
+ case SCMI_PROTOCOL_ID_CLOCK:
+ drv = DM_GET_DRIVER(scmi_clock);
+ break;
default:
dev_info(dev, "Ignore unsupported SCMI protocol %u\n",
protocol_id);
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-08-18 8:51 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200717153731.10643-1-etienne.carriere@linaro.org>
2020-07-20 2:01 ` [PATCH 1/4] firmware: add new driver for SCMI firmwares Peng Fan
2020-07-24 9:46 ` Etienne Carriere
[not found] ` <20200717153731.10643-2-etienne.carriere@linaro.org>
2020-07-20 2:01 ` [PATCH 2/4] dt-bindings: arm: SCMI bindings documentation Peng Fan
2020-07-24 9:47 ` Etienne Carriere
[not found] ` <20200717153731.10643-3-etienne.carriere@linaro.org>
2020-07-20 2:06 ` [PATCH 3/4] clk: add clock driver for SCMI agents Peng Fan
2020-07-24 9:54 ` Etienne Carriere
[not found] ` <20200717153731.10643-4-etienne.carriere@linaro.org>
2020-07-20 2:07 ` [PATCH 4/4] reset: add reset controller " Peng Fan
2020-07-26 14:54 ` [PATCH 1/4] firmware: add new driver for SCMI firmwares Simon Glass
2020-08-04 9:33 ` Etienne Carriere
2020-08-16 3:39 ` Simon Glass
2020-07-17 15:42 Etienne Carriere
2020-07-17 15:42 ` [PATCH 3/4] clk: add clock driver for SCMI agents Etienne Carriere
2020-07-26 14:54 ` Simon Glass
2020-08-18 8:46 ` Etienne Carriere
2020-08-18 8:51 ` Sudeep Holla
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.