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

From: Baolin Wang
Date: Mon Apr 27 2020 - 23:12:59 EST

Hi Jassi,

On Tue, Apr 28, 2020 at 4:19 AM Jassi Brar <jassisinghbrar@xxxxxxxxx> wrote:
>
> 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.

Yes, will do in next version.

> > +
> > +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.

This is used to check if the target core have fetched the message from
the FIFO or not, if not, we can not send any more message, otherwise
the message will be overflowed.

Thanks for your comments.

--
Baolin Wang