[U-Boot] [PATCH] Add a mailbox driver framework/uclass
Simon Glass
sjg at chromium.org
Fri May 13 22:05:34 CEST 2016
Hi Stephen,
On 12 May 2016 at 16:27, Stephen Warren <swarren at wwwdotorg.org> wrote:
> From: Stephen Warren <swarren at nvidia.com>
>
> A mailbox is a hardware mechanism for transferring small message and/or
> notifications between the CPU on which U-Boot runs and some other device
> such as an auxilliary CPU running firmware or a hardware module.
>
> This patch defines a standard API that connects mailbox clients to mailbox
> providers (drivers). Initially, DT is the only supported method for
> connecting the two.
>
> The DT binding specification (mailbox.txt) was taken from Linux kernel
> v4.5's Documentation/devicetree/bindings/mailbox/mailbox.txt.
>
> Signed-off-by: Stephen Warren <swarren at nvidia.com>
> ---
> I'll send an example mailbox implementation for Tegra186 as soon as it's
> passed internal IP review; I'd expect in the next day or so.
>
> That driver will depend on the core Tegra186 support that I posted earlier
> today, although this patch doesn't.
>
> doc/device-tree-bindings/mailbox/mailbox.txt | 32 ++++++
> drivers/Kconfig | 2 +
> drivers/Makefile | 1 +
> drivers/mailbox/Kconfig | 12 +++
> drivers/mailbox/Makefile | 5 +
> drivers/mailbox/mailbox-uclass.c | 147 ++++++++++++++++++++++++++
> include/dm/uclass-id.h | 1 +
> include/mailbox_client.h | 148 +++++++++++++++++++++++++++
> include/mailbox_uclass.h | 83 +++++++++++++++
> 9 files changed, 431 insertions(+)
> create mode 100644 doc/device-tree-bindings/mailbox/mailbox.txt
> create mode 100644 drivers/mailbox/Kconfig
> create mode 100644 drivers/mailbox/Makefile
> create mode 100644 drivers/mailbox/mailbox-uclass.c
> create mode 100644 include/mailbox_client.h
> create mode 100644 include/mailbox_uclass.h
Looks good. A few nits below. Can you please add a sandbox driver and a test?
Also see the remoteproc uclass. I'd just like to make sure that these
should not be combined.
>
> diff --git a/doc/device-tree-bindings/mailbox/mailbox.txt b/doc/device-tree-bindings/mailbox/mailbox.txt
> new file mode 100644
> index 000000000000..be05b9746c69
> --- /dev/null
> +++ b/doc/device-tree-bindings/mailbox/mailbox.txt
> @@ -0,0 +1,32 @@
> +* Generic Mailbox Controller and client driver bindings
> +
> +Generic binding to provide a way for Mailbox controller drivers to
> +assign appropriate mailbox channel to client drivers.
> +
> +* Mailbox Controller
> +
> +Required property:
> +- #mbox-cells: Must be at least 1. Number of cells in a mailbox
> + specifier.
> +
> +Example:
> + mailbox: mailbox {
> + ...
> + #mbox-cells = <1>;
> + };
> +
> +
> +* Mailbox Client
> +
> +Required property:
> +- mboxes: List of phandle and mailbox channel specifiers.
> +
> +Optional property:
> +- mbox-names: List of identifier strings for each mailbox channel.
> +
> +Example:
> + pwr_cntrl: power {
> + ...
> + mbox-names = "pwr-ctrl", "rpc";
> + mboxes = <&mailbox 0 &mailbox 1>;
> + };
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 118b66ed0e14..f2a137ad87fc 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -30,6 +30,8 @@ source "drivers/input/Kconfig"
>
> source "drivers/led/Kconfig"
>
> +source "drivers/mailbox/Kconfig"
> +
> source "drivers/memory/Kconfig"
>
> source "drivers/misc/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 6900097e7998..47fddff084b5 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -62,6 +62,7 @@ obj-y += video/
> obj-y += watchdog/
> obj-$(CONFIG_QE) += qe/
> obj-$(CONFIG_U_QE) += qe/
> +obj-y += mailbox/
> obj-y += memory/
> obj-y += pwm/
> obj-y += input/
> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> new file mode 100644
> index 000000000000..b9721fa16016
> --- /dev/null
> +++ b/drivers/mailbox/Kconfig
> @@ -0,0 +1,12 @@
> +menu "Mailbox Controller Support"
> +
> +config DM_MAILBOX
> + bool "Enable mailbox controllers using Driver Model"
> + depends on DM && OF_CONTROL
> + help
> + Enable support for the mailbox driver class. Mailboxes provide the
> + ability to transfer small messages and/or notifications from one
> + CPU to another CPU, or sometimes to dedicated HW modules. They form
> + the basis of a variety of IPC protocols.
Is this inter-process communication? Can you please write it out in full?
> +
> +endmenu
> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> new file mode 100644
> index 000000000000..a2d96a4e3b98
> --- /dev/null
> +++ b/drivers/mailbox/Makefile
> @@ -0,0 +1,5 @@
> +# Copyright (c) 2016, NVIDIA CORPORATION.
> +#
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_DM_MAILBOX) += mailbox-uclass.o
> diff --git a/drivers/mailbox/mailbox-uclass.c b/drivers/mailbox/mailbox-uclass.c
> new file mode 100644
> index 000000000000..2bec83076a17
> --- /dev/null
> +++ b/drivers/mailbox/mailbox-uclass.c
> @@ -0,0 +1,147 @@
> +/*
> + * Copyright (c) 2016, NVIDIA CORPORATION.
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <fdtdec.h>
> +#include <mailbox_client.h>
> +#include <mailbox_uclass.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static inline struct mbox_ops *mbox_dev_ops(struct udevice *dev)
> +{
> + return (struct mbox_ops *)dev->driver->ops;
> +}
> +
> +static int mbox_of_xlate_default(struct mbox_chan *chan,
> + struct fdtdec_phandle_args *args)
> +{
> + debug("%s(chan=%p)\n", __func__, chan);
> +
> + if (args->args_count != 1) {
> + debug("Invaild args_count: %d\n", args->args_count);
> + return -EINVAL;
> + }
> +
> + chan->id = args->args[0];
> +
> + return 0;
> +}
> +
> +int mbox_get_by_index(struct udevice *dev, int index, struct mbox_chan *chan)
> +{
> + struct fdtdec_phandle_args args;
> + int ret;
> + struct udevice *dev_mbox;
> + struct mbox_ops *ops;
> +
> + debug("%s(dev=%p, index=%d, chan=%p)\n", __func__, dev, index, chan);
> +
> + ret = fdtdec_parse_phandle_with_args(gd->fdt_blob, dev->of_offset,
> + "mboxes", "#mbox-cells", 0,
> + index, &args);
> + if (ret) {
> + debug("%s: fdtdec_parse_phandle_with_args failed: %d\n",
> + __func__, ret);
> + return ret;
> + }
> +
> + ret = uclass_get_device_by_of_offset(UCLASS_MAILBOX, args.node,
> + &dev_mbox);
> + if (ret) {
> + debug("%s: uclass_get_device_by_of_offset failed: %d\n",
> + __func__, ret);
> + return ret;
> + }
> + ops = mbox_dev_ops(dev_mbox);
> +
> + chan->dev = dev_mbox;
> + if (ops->of_xlate)
> + ret = ops->of_xlate(chan, &args);
> + else
> + ret = mbox_of_xlate_default(chan, &args);
> + if (ret) {
> + debug("of_xlate() failed: %d\n", ret);
> + return ret;
> + }
> +
> + ret = ops->request(chan);
> + if (ret) {
> + debug("ops->request() failed: %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +int mbox_get_by_name(struct udevice *dev, const char *name,
> + struct mbox_chan *chan)
> +{
> + int index;
> +
> + debug("%s(dev=%p, name=%s, chan=%p)\n", __func__, dev, name, chan);
> +
> + index = fdt_find_string(gd->fdt_blob, dev->of_offset, "mbox-names",
> + name);
> + if (index < 0) {
> + debug("fdt_find_string() failed: %d\n", index);
> + return index;
> + }
> +
> + return mbox_get_by_index(dev, index, chan);
> +}
> +
> +int mbox_free(struct mbox_chan *chan)
> +{
> + struct mbox_ops *ops = mbox_dev_ops(chan->dev);
> +
> + debug("%s(chan=%p)\n", __func__, chan);
> +
> + return ops->free(chan);
> +}
> +
> +int mbox_send(struct mbox_chan *chan, const void *data)
Is there no length on the data?
> +{
> + struct mbox_ops *ops = mbox_dev_ops(chan->dev);
> +
> + debug("%s(chan=%p, data=%p)\n", __func__, chan, data);
> +
> + return ops->send(chan, data);
> +}
> +
> +int mbox_recv(struct mbox_chan *chan, void *data, ulong timeout_us)
> +{
> + struct mbox_ops *ops = mbox_dev_ops(chan->dev);
> + ulong start_time;
> + ulong timeout;
> + int ret;
> +
> + debug("%s(chan=%p, data=%p, timeout_us=%ld)\n", __func__, chan, data,
> + timeout_us);
> +
> + start_time = get_ticks();
> + /*
> + * Account for partial us ticks, but if timeout_us is 0, ensure we
> + * still don't wait at all.
> + */
> + if (timeout_us)
> + timeout_us++;
This seems a bit picky - it is only a microsecond! Also. you could use
timer_get_us() for this.
> + timeout = usec2ticks(timeout_us);
> +
> + for (;;) {
> + ret = ops->recv(chan, data);
> + if (ret != -ENODATA)
With Ethernet and serial we use -EAGAIN. Actually I think -ENODATA is
better but we should probably change the others.
> + return ret;
> + if ((get_ticks() - start_time) >= timeout)
> + return -ETIMEDOUT;
> + }
> +}
> +
> +UCLASS_DRIVER(mailbox) = {
> + .id = UCLASS_MAILBOX,
> + .name = "mailbox",
> +};
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 45ae9650e3dd..4e252ae210f7 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -44,6 +44,7 @@ enum uclass_id {
> UCLASS_KEYBOARD, /* Keyboard input device */
> UCLASS_LED, /* Light-emitting diode (LED) */
> UCLASS_LPC, /* x86 'low pin count' interface */
> + UCLASS_MAILBOX, /* Mailbox controller */
> UCLASS_MASS_STORAGE, /* Mass storage device */
> UCLASS_MISC, /* Miscellaneous device */
> UCLASS_MMC, /* SD / MMC card or chip */
> diff --git a/include/mailbox_client.h b/include/mailbox_client.h
> new file mode 100644
> index 000000000000..137716072dc3
> --- /dev/null
> +++ b/include/mailbox_client.h
> @@ -0,0 +1,148 @@
> +/*
> + * Copyright (c) 2016, NVIDIA CORPORATION.
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +
> +#ifndef _MAILBOX_CLIENT_H
> +#define _MAILBOX_CLIENT_H
> +
> +/**
> + * A mailbox is a hardware mechanism for transferring small message and/or
> + * notifications between the CPU on which U-Boot runs and some other device
> + * such as an auxilliary CPU running firmware or a hardware module.
auxiliary
> + *
> + * Data transfer is optional; a mailbox may consist solely of a notification
> + * mechanism. When data transfer is implemented, it is via HW registers or
> + * FIFOs, rather than via RAM-based buffers. The mailbox API generall
> + * implements any communication protocol enforced solely by hardware, and
> + * leaves any higher-level protocols to other layers.
> + *
> + * A mailbox channel is a bi-directional mechanism that can send a message or
> + * notification to a single specific remote entity, and receive messages or
> + * notifications from that entity. The content and format of such messages is
> + * defined by the mailbox implementation, or the remote entity with which it
> + * communicates; there is no general standard at this API level.
> + *
> + * A driver that implements UCLASS_MAILBOX is a mailbox provider. A provider
> + * will often implement multiple separate mailbox channels, since the hardware
> + * it manages often has this capability. mailbox_uclass.h describes the
> + * interface which mailbox providers must implement.
> + *
> + * Mailbox consumers/clients generate and send, or receive and process,
> + * messages. This header file describes the API used by clients.
> + */
> +
> +struct udevice;
> +
> +/**
> + * struct mbox_chan - A handle to a single mailbox channel.
> + *
> + * Clients provide storage for channels. The content of the channel structure
> + * is managed solely by the mailbox API and mailbox drivers. A mailbox channel
> + * is initialized by "get"ing the mailbox. The channel struct is passed to all
> + * other mailbox APIs to identify which mailbox to operate upon.
> + *
> + * @dev: The device which implements the mailbox.
> + * @id: The mailbox channel ID within the provider.
> + *
> + * Currently, the mailbox API assumes that a single integer ID is enough to
> + * identify and configure any mailbox channel for any mailbox provider. If this
> + * assumption becomes invalid in the future, the struct could be expanded to
> + * either (a) add more fields to allow mailbox providers to store additional
> + * information, or (b) replace the id field with an opaque pointer, which the
> + * provider would dynamically allocated during its .of_xlate op. This may
> + * require the addition of an extra op to clean up the allocation.
> + */
> +struct mbox_chan {
> + struct udevice *dev;
> + /*
> + * Written by of_xlate. We assume a single id is enough for now. In the
> + * future, we might add more fields here.
> + */
> + unsigned long id;
> +};
> +
> +/**
> + * mbox_get_by_index - Get/request a mailbox by integer index
> + *
> + * This looks up and requests a mailbox channel. The index is relative to the
> + * client device; each device is assumed to have n mailbox channels associated
> + * with it somehow, and this function finds and requests one of them. The
> + * mapping of client device channel indices to provider channels may be via
> + * device-tree properties, board-provided mapping tables, or some other
> + * mechanism.
> + *
> + * @dev: The client device.
> + * @index: The index of the mailbox channel to request, within the
> + * client's list of channels.
> + * @chan A pointer to a channel object to initialize.
> + * @return 0 if OK, or a negative error code.
> + */
> +int mbox_get_by_index(struct udevice *dev, int index, struct mbox_chan *chan);
> +
> +/**
> + * mbox_get_by_name - Get/request a mailbox by name
> + *
> + * This looks up and requests a mailbox channel. The name is relative to the
> + * client device; each device is assumed to have n mailbox channels associated
> + * with it somehow, and this function finds and requests one of them. The
> + * mapping of client device channel names to provider channels may be via
> + * device-tree properties, board-provided mapping tables, or some other
> + * mechanism.
> + *
> + * @dev: The client device.
> + * @name: The name of the mailbox channel to request, within the client's
> + * list of channels.
> + * @chan A pointer to a channel object to initialize.
> + * @return 0 if OK, or a negative error code.
> + */
> +int mbox_get_by_name(struct udevice *dev, const char *name,
> + struct mbox_chan *chan);
> +
> +/**
> + * mbox_free - Free a previously requested mailbox channel.
> + *
> + * @chan: A channel object that was previously successfully requested by
> + * calling mbox_get_by_*().
> + * @return 0 if OK, or a negative error code.
> + */
> +int mbox_free(struct mbox_chan *chan);
> +
> +/**
> + * mbox_send - Send a message over a mailbox channel
> + *
> + * This function will send a message to the remote entity. It may return before
> + * the remote entity has received and/or processed the message.
> + *
> + * @chan: A channel object that was previously successfully requested by
> + * calling mbox_get_by_*().
> + * @data: A pointer to the message to transfer. The format and size of
> + * the memory region pointed at by @data is determined by the
> + * mailbox provider. Providers that solely transfer notifications
> + * will ignore this parameter.
> + * @return 0 if OK, or a negative error code.
> + */
> +int mbox_send(struct mbox_chan *chan, const void *data);
> +
> +/**
> + * mbox_recv - Receive any available message from a mailbox channel
> + *
> + * This function will wait (up to the specified @timeout_us) for a message to
> + * be sent by the remote entity, and write the content of any such message
> + * into a caller-provided buffer.
> + *
> + * @chan: A channel object that was previously successfully requested by
> + * calling mbox_get_by_*().
> + * @data: A pointer to the buffer to receive the message. The format and
> + * size of the memory region pointed at by @data is determined by
> + * the mailbox provider. Providers that solely transfer
> + * notifications will ignore this parameter.
> + * @timeout_us: The maximum time to wait for a message to be available, in
> + * micro-seconds. A value of 0 does not wait at all.
> + * @return 0 if OK, -ENODATA if no message was available, or a negative error
> + * code.
> + */
> +int mbox_recv(struct mbox_chan *chan, void *data, ulong timeout_us);
Great docs, thanks.
> +
> +#endif
> diff --git a/include/mailbox_uclass.h b/include/mailbox_uclass.h
> new file mode 100644
> index 000000000000..6a2994c34cea
> --- /dev/null
> +++ b/include/mailbox_uclass.h
> @@ -0,0 +1,83 @@
> +/*
> + * Copyright (c) 2016, NVIDIA CORPORATION.
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +
> +#ifndef _MAILBOX_UCLASS_H
> +#define _MAILBOX_UCLASS_H
> +
> +/* See mailbox_client.h for background documentation. */
> +
> +#include <mailbox_client.h>
> +
> +struct udevice;
> +
> +/**
> + * struct mbox_ops - The functions that a mailbox driver must implement.
> + */
> +struct mbox_ops {
> + /**
> + * of_xlate - Translate a client's device-tree (OF) mailbox specifier.
> + *
> + * The mailbox core calls this function as the first step in
> + * implementing a client's mbox_get_by_*() call.
> + *
> + * If this function pointer is set to NULL, the mailbox core will use
> + * a default implementation, which assumes #mbox-cells = <1>, and that
> + * the DT cell contains a simple integer channel ID.
> + *
> + * At present, the mailbox API solely supports device-tree. If this
> + * changes, other xxx_xlate() functions may be added to support those
> + * other mechanisms.
> + *
> + * @chan: The channel to hold the translation result.
> + * @args: The mailbox specifier values from device tree.
> + * @return 0 if OK, or a negative error code.
> + */
> + int (*of_xlate)(struct mbox_chan *chan,
> + struct fdtdec_phandle_args *args);
> + /**
> + * request - Request a translated channel.
> + *
> + * The mailbox core calls this function as the second step in
> + * implementing a client's mbox_get_by_*() call, following a successful
> + * xxx_xlate() call.
> + *
> + * @chan: The channel to request; this has been filled in by a
> + * previoux xxx_xlate() function call.
> + * @return 0 if OK, or a negative error code.
> + */
> + int (*request)(struct mbox_chan *chan);
> + /**
> + * free - Free a previously requested channel.
> + *
> + * This is the implementation of the client mbox_free() API.
> + *
> + * @chan: The channel to free.
> + * @return 0 if OK, or a negative error code.
> + */
> + int (*free)(struct mbox_chan *chan);
> + /**
> + * send - Send a message over a mailbox channel
> + *
> + * @chan: The channel to send to the message to.
> + * @data: A pointer to the message to send.
> + * @return 0 if OK, or a negative error code.
> + */
> + int (*send)(struct mbox_chan *chan, const void *data);
> + /**
> + * recv - Receive any available message from the channel.
> + *
> + * This function does not block. If not message is immediately
> + * available, the function should return an error.
> + *
> + * @chan: The channel to receive to the message from.
> + * @data: A pointer to the buffer to hold the received message.
> + * @return 0 if OK, -ENODATA if no message was available, or a negative
> + * error code.
> + */
> + int (*recv)(struct mbox_chan *chan, void *data);
> +};
> +
> +#endif
> --
> 2.8.2
>
Regards,
Simon
More information about the U-Boot
mailing list