Re: [RESEND PATCH v3 2/2] mailbox: sprd: Add Spreadtrum mailbox driver

From: Jassi Brar
Date: Mon Apr 27 2020 - 16:19:51 EST


On Mon, Apr 27, 2020 at 2:20 AM Baolin Wang <baolin.wang7@xxxxxxxxx> wrote:
>
> From: Baolin Wang <baolin.wang@xxxxxxxxxx>
>
> The Spreadtrum mailbox controller supports 8 channels to communicate
> with MCUs, and it contains 2 different parts: inbox and outbox, which
> are used to send and receive messages by IRQ mode.
>
> Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxx>
> Signed-off-by: Baolin Wang <baolin.wang7@xxxxxxxxx>
> ---
> Changes from v2:
> - None.
>
> Changes from v1:
> - None
> ---
> drivers/mailbox/Kconfig | 8 +
> drivers/mailbox/Makefile | 2 +
> drivers/mailbox/sprd-mailbox.c | 350 +++++++++++++++++++++++++++++++++
> 3 files changed, 360 insertions(+)
> create mode 100644 drivers/mailbox/sprd-mailbox.c
>
> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> index 5a577a6734cf..e03f3fb5caed 100644
> --- a/drivers/mailbox/Kconfig
> +++ b/drivers/mailbox/Kconfig
> @@ -236,4 +236,12 @@ config SUN6I_MSGBOX
> various Allwinner SoCs. This mailbox is used for communication
> between the application CPUs and the power management coprocessor.
>
> +config SPRD_MBOX
> + tristate "Spreadtrum Mailbox"
> + depends on ARCH_SPRD || COMPILE_TEST
> + help
> + Mailbox driver implementation for the Spreadtrum platform. It is used
> + to send message between application processors and MCU. Say Y here if
> + you want to build the Spreatrum mailbox controller driver.
> +
> endif
> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> index 2e4364ef5c47..9caf4ede6ce0 100644
> --- a/drivers/mailbox/Makefile
> +++ b/drivers/mailbox/Makefile
> @@ -50,3 +50,5 @@ obj-$(CONFIG_MTK_CMDQ_MBOX) += mtk-cmdq-mailbox.o
> obj-$(CONFIG_ZYNQMP_IPI_MBOX) += zynqmp-ipi-mailbox.o
>
> obj-$(CONFIG_SUN6I_MSGBOX) += sun6i-msgbox.o
> +
> +obj-$(CONFIG_SPRD_MBOX) += sprd-mailbox.o
> diff --git a/drivers/mailbox/sprd-mailbox.c b/drivers/mailbox/sprd-mailbox.c
> new file mode 100644
> index 000000000000..58e5388f190b
> --- /dev/null
> +++ b/drivers/mailbox/sprd-mailbox.c
> @@ -0,0 +1,350 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Spreadtrum mailbox driver
> + *
> + * Copyright (c) 2020 Spreadtrum Communications Inc.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/mailbox_controller.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +
> +#define SPRD_MBOX_ID 0x0
> +#define SPRD_MBOX_MSG_LOW 0x4
> +#define SPRD_MBOX_MSG_HIGH 0x8
> +#define SPRD_MBOX_TRIGGER 0xc
> +#define SPRD_MBOX_FIFO_RST 0x10
> +#define SPRD_MBOX_FIFO_STS 0x14
> +#define SPRD_MBOX_IRQ_STS 0x18
> +#define SPRD_MBOX_IRQ_MSK 0x1c
> +#define SPRD_MBOX_LOCK 0x20
> +#define SPRD_MBOX_FIFO_DEPTH 0x24
> +
> +/* Bit and mask definiation for inbox's SPRD_MBOX_FIFO_STS register */
> +#define SPRD_INBOX_FIFO_DELIVER_MASK GENMASK(23, 16)
> +#define SPRD_INBOX_FIFO_OVERLOW_MASK GENMASK(15, 8)
> +#define SPRD_INBOX_FIFO_DELIVER_SHIFT 16
> +#define SPRD_INBOX_FIFO_BUSY_MASK GENMASK(7, 0)
> +
> +/* Bit and mask definiation for SPRD_MBOX_IRQ_STS register */
> +#define SPRD_MBOX_IRQ_CLR BIT(0)
> +
> +/* Bit and mask definiation for outbox's SPRD_MBOX_FIFO_STS register */
> +#define SPRD_OUTBOX_FIFO_FULL BIT(0)
> +#define SPRD_OUTBOX_FIFO_WR_SHIFT 16
> +#define SPRD_OUTBOX_FIFO_RD_SHIFT 24
> +#define SPRD_OUTBOX_FIFO_POS_MASK GENMASK(7, 0)
> +
> +/* Bit and mask definiation for inbox's SPRD_MBOX_IRQ_MSK register */
> +#define SPRD_INBOX_FIFO_BLOCK_IRQ BIT(0)
> +#define SPRD_INBOX_FIFO_OVERFLOW_IRQ BIT(1)
> +#define SPRD_INBOX_FIFO_DELIVER_IRQ BIT(2)
> +#define SPRD_INBOX_FIFO_IRQ_MASK GENMASK(2, 0)
> +
> +/* Bit and mask definiation for outbox's SPRD_MBOX_IRQ_MSK register */
> +#define SPRD_OUTBOX_FIFO_NOT_EMPTY_IRQ BIT(0)
> +#define SPRD_OUTBOX_FIFO_IRQ_MASK GENMASK(4, 0)
> +
> +#define SPRD_MBOX_CHAN_MAX 8
> +
> +struct sprd_mbox_chan {
> + u8 id;
> + struct mbox_chan *chan;
> +};
If 'id' is all you need, please assign that to mbox_chan.con_priv and
discard the sprd_mbox_chan. That will be much simpler.

> +
> +struct sprd_mbox_priv {
> + struct mbox_controller mbox;
> + struct device *dev;
> + void __iomem *inbox_base;
> + void __iomem *outbox_base;
> + struct clk *clk;
> + u32 outbox_fifo_depth;
> +
> + struct sprd_mbox_chan mchan[SPRD_MBOX_CHAN_MAX];
> + struct mbox_chan chan[SPRD_MBOX_CHAN_MAX];
> +};
> +
.........

> +
> +static irqreturn_t sprd_mbox_inbox_isr(int irq, void *data)
> +{
> + struct sprd_mbox_priv *priv = data;
> + struct sprd_mbox_chan *mchan;
> + u32 fifo_sts, send_sts, id;
> +
> + fifo_sts = readl(priv->inbox_base + SPRD_MBOX_FIFO_STS);
> +
> + /* Get the inbox data delivery status */
> + send_sts = (fifo_sts & SPRD_INBOX_FIFO_DELIVER_MASK) >>
> + SPRD_INBOX_FIFO_DELIVER_SHIFT;
> + if (!send_sts) {
> + dev_warn_ratelimited(priv->dev, "spurious inbox interrupt\n");
> + return IRQ_NONE;
> + }
> +
> + while (send_sts) {
> + id = __ffs(send_sts);
> + send_sts &= (send_sts - 1);
> +
> + mchan = &priv->mchan[id];
> + mbox_chan_txdone(mchan->chan, 0);
> + }
> +
> + /* Clear FIFO delivery and overflow status */
> + writel(fifo_sts &
> + (SPRD_INBOX_FIFO_DELIVER_MASK | SPRD_INBOX_FIFO_OVERLOW_MASK),
> + priv->inbox_base + SPRD_MBOX_FIFO_RST);
> +
> + /* Clear irq status */
> + writel(SPRD_MBOX_IRQ_CLR, priv->inbox_base + SPRD_MBOX_IRQ_STS);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int sprd_mbox_send_data(struct mbox_chan *chan, void *msg)
> +{
> + struct sprd_mbox_priv *priv = to_sprd_mbox_priv(chan->mbox);
> + struct sprd_mbox_chan *mchan = chan->con_priv;
> + u32 *data = msg, busy;
> +
> + /*
> + * Check if current channel is busy or not, and we can not send data
> + * if current channel is busy.
> + */
> + busy = readl(priv->inbox_base + SPRD_MBOX_FIFO_STS) &
> + SPRD_INBOX_FIFO_BUSY_MASK;
> + if (busy & BIT(mchan->id)) {
> + dev_err(priv->dev, "Channel %d is busy\n", mchan->id);
> + return -EBUSY;
> + }
Maybe check this before mbox_chan_txdone(mchan->chan, 0) and avoid
failing in send_data.

Cheers!