Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <8d29eba045ef18c5489e122b3668afc20431f15d.1587894279.git.baolin.wang7@gmail.com>
 <6f8ac1af5a43d6fd487dc21c052624277325e6f1.1587894279.git.baolin.wang7@gmail.com>
In-Reply-To: <6f8ac1af5a43d6fd487dc21c052624277325e6f1.1587894279.git.baolin.wang7@gmail.com>
From:   Jassi Brar <jassisinghbrar@gmail.com>
Date:   Mon, 27 Apr 2020 15:19:37 -0500
Message-ID: <CABb+yY31zAr9xfS3bWC5k=94L921CyhzA7fot3eJP1WYSvNCpw@mail.gmail.com>
Subject: Re: [RESEND PATCH v3 2/2] mailbox: sprd: Add Spreadtrum mailbox driver
To:     Baolin Wang <baolin.wang7@gmail.com>
Cc:     Rob Herring <robh+dt@kernel.org>, orsonzhai@gmail.com,
        zhang.lyra@gmail.com, Devicetree List <devicetree@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Apr 27, 2020 at 2:20 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
>
> From: Baolin Wang <baolin.wang@unisoc.com>
>
> 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@unisoc.com>
> Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
> ---
> 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!