Hi Etienne,
On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
<etienne.carri...@linaro.org> wrote:
>
> This change implements a mailbox transport using SMT format for SCMI
> exchanges. This implementation follows the Linux kernel and
> SCP-firmware [1] as references implementation for SCMI message
> processing using SMT format for communication channel meta-data.
>
> Use of mailboxes in SCMI FDT bindings are defined in the Linux kernel
> DT bindings since v4.17.
>
> Links: [1] https://github.com/ARM-software/SCP-firmware
> Signed-off-by: Etienne Carriere <etienne.carri...@linaro.org>
> Cc: Simon Glass <s...@chromium.org>
> Cc: Peng Fan <peng....@nxp.com>
> Cc: Sudeep Holla <sudeep.ho...@arm.com>
> ---
>
> Changes in v3:
> - This is a followup of the SCMI agent patches posted in
> https://patchwork.ozlabs.org/project/uboot/list/?series=196253
> The v3 splits commits and introduces a new uclass as requested.
> - This patch implements the same mailbox SCMI agent proposed in v2
> but split over few source files.
> ---
> drivers/firmware/scmi/Kconfig | 6 +-
> drivers/firmware/scmi/Makefile | 2 +
> drivers/firmware/scmi/mailbox_agent.c | 107 ++++++++++++++++++++
> drivers/firmware/scmi/smt.c | 139 ++++++++++++++++++++++++++
> drivers/firmware/scmi/smt.h | 86 ++++++++++++++++
> 5 files changed, 338 insertions(+), 2 deletions(-)
> create mode 100644 drivers/firmware/scmi/mailbox_agent.c
> create mode 100644 drivers/firmware/scmi/smt.c
> create mode 100644 drivers/firmware/scmi/smt.h
Reviewed-by: Simon Glass <s...@chromium.org>
>
> diff --git a/drivers/firmware/scmi/Kconfig b/drivers/firmware/scmi/Kconfig
> index 57e2ebbe42..c501bf4943 100644
> --- a/drivers/firmware/scmi/Kconfig
> +++ b/drivers/firmware/scmi/Kconfig
> @@ -2,7 +2,7 @@ config SCMI_FIRMWARE
> bool "Enable SCMI support"
> select FIRMWARE
> select OF_TRANSLATE
> - depends on SANDBOX
> + depends on SANDBOX || DM_MAILBOX
> help
> System Control and Management Interface (SCMI) is a communication
> protocol that defines standard interfaces for power, performance
> @@ -14,4 +14,6 @@ config SCMI_FIRMWARE
> or a companion host in the CPU system.
>
> Communications between agent (client) and the SCMI server are
> - based on message exchange.
> + based on message exchange. Messages can be exchange over tranport
> + channels as a mailbox device with some piece of identified shared
> + memory.
> diff --git a/drivers/firmware/scmi/Makefile b/drivers/firmware/scmi/Makefile
> index 336ea1f2a3..d22f53efe7 100644
> --- a/drivers/firmware/scmi/Makefile
> +++ b/drivers/firmware/scmi/Makefile
> @@ -1,2 +1,4 @@
> obj-y += scmi_agent-uclass.o
> +obj-y += smt.o
> +obj-$(CONFIG_DM_MAILBOX) += mailbox_agent.o
> obj-$(CONFIG_SANDBOX) += sandbox-scmi_agent.o
> diff --git a/drivers/firmware/scmi/mailbox_agent.c
> b/drivers/firmware/scmi/mailbox_agent.c
> new file mode 100644
> index 0000000000..9a7b0a5858
> --- /dev/null
> +++ b/drivers/firmware/scmi/mailbox_agent.c
> @@ -0,0 +1,107 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2020 Linaro Limited.
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <mailbox.h>
> +#include <scmi_agent.h>
> +#include <scmi_agent-uclass.h>
> +#include <dm/devres.h>
> +#include <linux/compat.h>
> +
> +#include "smt.h"
> +
> +#define TIMEOUT_US_10MS 10000
> +
> +/**
> + * struct scmi_mbox_channel - Description of an SCMI mailbox transport
> + * @smt: Shared memory buffer
> + * @mbox: Mailbox channel description
> + * @timeout_us: Timeout in microseconds for the mailbox transfer
> + */
> +struct scmi_mbox_channel {
> + struct scmi_smt smt;
> + struct mbox_chan mbox;
> + ulong timeout_us;
> +};
> +
> +static struct scmi_mbox_channel *scmi_mbox_get_priv(struct udevice *dev)
> +{
> + return (struct scmi_mbox_channel *)dev_get_priv(dev);
> +}
> +
> +static int scmi_mbox_process_msg(struct udevice *dev, struct scmi_msg *msg)
> +{
> + struct scmi_mbox_channel *chan = scmi_mbox_get_priv(dev);
> + int ret;
> +
> + ret = scmi_write_msg_to_smt(dev, &chan->smt, msg);
> + if (ret)
> + return ret;
> +
> + /* Give shm addr to mbox in case it is meaningful */
> + ret = mbox_send(&chan->mbox, chan->smt.buf);
> + if (ret) {
> + dev_err(dev, "Message send failed: %d\n", ret);
> + goto out;
> + }
> +
> + /* Receive the response */
> + ret = mbox_recv(&chan->mbox, chan->smt.buf, chan->timeout_us);
> + if (ret) {
> + dev_err(dev, "Response failed: %d, abort\n", ret);
> + goto out;
> + }
> +
> + ret = scmi_read_resp_from_smt(dev, &chan->smt, msg);
> +
> +out:
> + scmi_clear_smt_channel(&chan->smt);
> +
> + return ret;
> +}
> +
> +int scmi_mbox_probe(struct udevice *dev)
> +{
> + struct scmi_mbox_channel *chan = scmi_mbox_get_priv(dev);
> + int ret;
> +
> + chan->timeout_us = TIMEOUT_US_10MS;
> +
> + ret = mbox_get_by_index(dev, 0, &chan->mbox);
> + if (ret) {
> + dev_err(dev, "Failed to find mailbox: %d\n", ret);
> + goto out;
> + }
> +
> + ret = scmi_dt_get_smt_buffer(dev, &chan->smt);
> + if (ret)
> + dev_err(dev, "Failed to get shm resources: %d\n", ret);
> +
> +out:
> + if (ret)
> + devm_kfree(dev, chan);
> +
> + return ret;
> +}
> +
> +static const struct udevice_id scmi_mbox_ids[] = {
> + { .compatible = "arm,scmi" },
> + { }
> +};
> +
> +static const struct scmi_agent_ops scmi_mbox_ops = {
> + .process_msg = scmi_mbox_process_msg,
> +};
> +
> +U_BOOT_DRIVER(scmi_mbox) = {
> + .name = "scmi-over-mailbox",
> + .id = UCLASS_SCMI_AGENT,
> + .of_match = scmi_mbox_ids,
> + .priv_auto_alloc_size = sizeof(struct scmi_mbox_channel),
> + .probe = scmi_mbox_probe,
> + .ops = &scmi_mbox_ops,
> +};
> diff --git a/drivers/firmware/scmi/smt.c b/drivers/firmware/scmi/smt.c
> new file mode 100644
> index 0000000000..afe95a4736
> --- /dev/null
> +++ b/drivers/firmware/scmi/smt.c
> @@ -0,0 +1,139 @@
> +// 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 <scmi_agent.h>
> +#include <asm/cache.h>
> +#include <asm/system.h>
> +#include <dm/ofnode.h>
> +#include <linux/compat.h>
> +#include <linux/io.h>
> +#include <linux/ioport.h>
> +
> +#include "smt.h"
> +
> +/**
> + * Get shared memory configuration defined by the referred DT phandle
> + * Return with a errno compliant value.
> + */
> +int scmi_dt_get_smt_buffer(struct udevice *dev, struct scmi_smt *smt)
> +{
> + int ret;
> + struct ofnode_phandle_args args;
> + struct resource resource;
> + fdt32_t faddr;
> + phys_addr_t paddr;
> +
> + ret = dev_read_phandle_with_args(dev, "shmem", NULL, 0, 0, &args);
> + if (ret)
> + return ret;
> +
> + ret = ofnode_read_resource(args.node, 0, &resource);
> + if (ret)
> + return ret;
> +
> + faddr = cpu_to_fdt32(resource.start);
> + paddr = ofnode_translate_address(args.node, &faddr);
> +
> + smt->size = resource_size(&resource);
> + if (smt->size < sizeof(struct scmi_smt_header)) {
> + dev_err(dev, "Shared memory buffer too small\n");
> + return -EINVAL;
> + }
> +
> + smt->buf = devm_ioremap(dev, paddr, smt->size);
> + if (!smt->buf)
> + return -ENOMEM;
> +
> +#ifdef __arm__
Should that be CONFIG_ARM ?
> + if (dcache_status())
> + mmu_set_region_dcache_behaviour((uintptr_t)smt->buf,
> + smt->size, DCACHE_OFF);
> +#endif
> +
> + return 0;
> +}
> +
[..]
Regards,
Simon
