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: <1612785064-3072-1-git-send-email-orsonzhai@gmail.com> <1612785064-3072-2-git-send-email-orsonzhai@gmail.com>
In-Reply-To: <1612785064-3072-2-git-send-email-orsonzhai@gmail.com>
From:   Baolin Wang <baolin.wang7@gmail.com>
Date:   Mon, 8 Feb 2021 22:27:47 +0800
Message-ID: <CADBw62qJRoDGdaY8jB1pppgd8S6JnJ+A5sT+c00uXDO9wXucmg@mail.gmail.com>
Subject: Re: [PATCH 2/3] mailbox: sprd: Add supplementary inbox support
To:     Orson Zhai <orsonzhai@gmail.com>
Cc:     Rob Herring <robh+dt@kernel.org>,
        Jassi Brar <jassisinghbrar@gmail.com>,
        Chunyan Zhang <zhang.lyra@gmail.com>,
        Mark Brown <broonie@kernel.org>,
        Devicetree List <devicetree@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Haidong Yao <haidong.yao@unisoc.com>,
        Orson Zhai <orson.zhai@unisoc.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Feb 8, 2021 at 7:52 PM Orson Zhai <orsonzhai@gmail.com> wrote:
>
> From: Orson Zhai <orson.zhai@unisoc.com>
>
> Some sensors connected to Unisoc mailbox will send data very frequently.
> This makes channel 0 very busy and the messages from other remote cores
> not able to be handled as soon as possible.
>
> Then a supplementary inbox is added to the host core side for transferring
> mass but not emergency messages from the remote cores, such as step
> counting sensor, with an independent FIFO and interrupt.

So this is another part of the mailbox hardware, containing a batch of
hardware channels? I did not see it before, its function is similar
with inbox/outbox?

>
> Signed-off-by: Orson Zhai <orson.zhai@unisoc.com>
> ---
>  drivers/mailbox/sprd-mailbox.c | 93 ++++++++++++++++++++++++++++++++++--------
>  1 file changed, 75 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mailbox/sprd-mailbox.c b/drivers/mailbox/sprd-mailbox.c
> index e606f52..74648db 100644
> --- a/drivers/mailbox/sprd-mailbox.c
> +++ b/drivers/mailbox/sprd-mailbox.c
> @@ -11,6 +11,7 @@
>  #include <linux/io.h>
>  #include <linux/mailbox_controller.h>
>  #include <linux/module.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/clk.h>
>
> @@ -50,13 +51,17 @@
>  #define SPRD_OUTBOX_FIFO_NOT_EMPTY_IRQ         BIT(0)
>  #define SPRD_OUTBOX_FIFO_IRQ_MASK              GENMASK(4, 0)
>
> +#define SPRD_OUTBOX_BASE_SPAN                  0x1000
>  #define SPRD_MBOX_CHAN_MAX                     8
> +#define SPRD_SUPP_INBOX_ID_SC9860              6
>
>  struct sprd_mbox_priv {
>         struct mbox_controller  mbox;
>         struct device           *dev;
>         void __iomem            *inbox_base;
>         void __iomem            *outbox_base;
> +       /*  Base register address for supplementary outbox */
> +       void __iomem            *supp_base;
>         struct clk              *clk;
>         u32                     outbox_fifo_depth;
>
> @@ -96,14 +101,13 @@ static u32 sprd_mbox_get_fifo_len(struct sprd_mbox_priv *priv, u32 fifo_sts)
>         return fifo_len;
>  }
>
> -static irqreturn_t sprd_mbox_outbox_isr(int irq, void *data)
> +static inline irqreturn_t do_outbox_isr(void __iomem *base, struct sprd_mbox_priv *priv)

No need to add an explicit 'inline' tag, the compiler can do the smart
things than us.

>  {
> -       struct sprd_mbox_priv *priv = data;
>         struct mbox_chan *chan;
>         u32 fifo_sts, fifo_len, msg[2];
>         int i, id;
>
> -       fifo_sts = readl(priv->outbox_base + SPRD_MBOX_FIFO_STS);
> +       fifo_sts = readl(base + SPRD_MBOX_FIFO_STS);
>
>         fifo_len = sprd_mbox_get_fifo_len(priv, fifo_sts);
>         if (!fifo_len) {
> @@ -112,23 +116,41 @@ static irqreturn_t sprd_mbox_outbox_isr(int irq, void *data)
>         }
>
>         for (i = 0; i < fifo_len; i++) {
> -               msg[0] = readl(priv->outbox_base + SPRD_MBOX_MSG_LOW);
> -               msg[1] = readl(priv->outbox_base + SPRD_MBOX_MSG_HIGH);
> -               id = readl(priv->outbox_base + SPRD_MBOX_ID);
> +               msg[0] = readl(base + SPRD_MBOX_MSG_LOW);
> +               msg[1] = readl(base + SPRD_MBOX_MSG_HIGH);
> +               id = readl(base + SPRD_MBOX_ID);
>
>                 chan = &priv->chan[id];
> -               mbox_chan_received_data(chan, (void *)msg);
> +               if (chan->cl)
> +                       mbox_chan_received_data(chan, (void *)msg);
> +               else
> +                       dev_warn_ratelimited(priv->dev,
> +                                   "message's been dropped at ch[%d]\n", id);
>
>                 /* Trigger to update outbox FIFO pointer */
> -               writel(0x1, priv->outbox_base + SPRD_MBOX_TRIGGER);
> +               writel(0x1, base + SPRD_MBOX_TRIGGER);
>         }
>
>         /* Clear irq status after reading all message. */
> -       writel(SPRD_MBOX_IRQ_CLR, priv->outbox_base + SPRD_MBOX_IRQ_STS);
> +       writel(SPRD_MBOX_IRQ_CLR, base + SPRD_MBOX_IRQ_STS);
>
>         return IRQ_HANDLED;
>  }
>
> +static irqreturn_t sprd_mbox_outbox_isr(int irq, void *data)
> +{
> +       struct sprd_mbox_priv *priv = data;
> +
> +       return do_outbox_isr(priv->outbox_base, priv);
> +}
> +
> +static irqreturn_t sprd_mbox_supp_isr(int irq, void *data)
> +{
> +       struct sprd_mbox_priv *priv = data;
> +
> +       return do_outbox_isr(priv->supp_base, priv);
> +}
> +
>  static irqreturn_t sprd_mbox_inbox_isr(int irq, void *data)
>  {
>         struct sprd_mbox_priv *priv = data;
> @@ -231,6 +253,14 @@ static int sprd_mbox_startup(struct mbox_chan *chan)
>                 val = readl(priv->outbox_base + SPRD_MBOX_IRQ_MSK);
>                 val &= ~SPRD_OUTBOX_FIFO_NOT_EMPTY_IRQ;
>                 writel(val, priv->outbox_base + SPRD_MBOX_IRQ_MSK);
> +
> +               /* Enable supplementary outbox as the fundamental one */
> +               if (priv->supp_base) {
> +                       writel(0x0, priv->supp_base + SPRD_MBOX_FIFO_RST);
> +                       val = readl(priv->supp_base + SPRD_MBOX_IRQ_MSK);
> +                       val &= ~SPRD_OUTBOX_FIFO_NOT_EMPTY_IRQ;
> +                       writel(val, priv->supp_base + SPRD_MBOX_IRQ_MSK);
> +               }
>         }
>         mutex_unlock(&priv->lock);
>
> @@ -246,6 +276,10 @@ static void sprd_mbox_shutdown(struct mbox_chan *chan)
>                 /* Disable inbox & outbox interrupt */
>                 writel(SPRD_INBOX_FIFO_IRQ_MASK, priv->inbox_base + SPRD_MBOX_IRQ_MSK);
>                 writel(SPRD_OUTBOX_FIFO_IRQ_MASK, priv->outbox_base + SPRD_MBOX_IRQ_MSK);
> +
> +               if (priv->supp_base)
> +                       writel(SPRD_OUTBOX_FIFO_IRQ_MASK,
> +                              priv->supp_base + SPRD_MBOX_IRQ_MSK);
>         }
>         mutex_unlock(&priv->lock);
>  }
> @@ -268,8 +302,8 @@ static int sprd_mbox_probe(struct platform_device *pdev)
>  {
>         struct device *dev = &pdev->dev;
>         struct sprd_mbox_priv *priv;
> -       int ret, inbox_irq, outbox_irq;
> -       unsigned long id;
> +       int ret, inbox_irq, outbox_irq, supp_irq;
> +       unsigned long id, supp;
>
>         priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>         if (!priv)
> @@ -280,11 +314,15 @@ static int sprd_mbox_probe(struct platform_device *pdev)
>         mutex_init(&priv->lock);
>
>         /*
> -        * The Spreadtrum mailbox uses an inbox to send messages to the target
> -        * core, and uses an outbox to receive messages from other cores.
> +        * Unisoc mailbox uses an inbox to send messages to the target
> +        * core, and uses (an) outbox(es) to receive messages from other
> +        * cores.
> +        *
> +        * Thus in general the mailbox controller supplies 2 different
> +        * register addresses and IRQ numbers for inbox and outbox.
>          *
> -        * Thus the mailbox controller supplies 2 different register addresses
> -        * and IRQ numbers for inbox and outbox.
> +        * If necessary, a supplementary inbox could be enabled optionally
> +        * with an independent FIFO and an extra interrupt.
>          */
>         priv->inbox_base = devm_platform_ioremap_resource(pdev, 0);
>         if (IS_ERR(priv->inbox_base))
> @@ -310,7 +348,7 @@ static int sprd_mbox_probe(struct platform_device *pdev)
>                 return ret;
>         }
>
> -       inbox_irq = platform_get_irq(pdev, 0);
> +       inbox_irq = platform_get_irq_byname(pdev, "inbox");

I think you should put the dt changes before this patch.

>         if (inbox_irq < 0)
>                 return inbox_irq;
>
> @@ -321,7 +359,7 @@ static int sprd_mbox_probe(struct platform_device *pdev)
>                 return ret;
>         }
>
> -       outbox_irq = platform_get_irq(pdev, 1);
> +       outbox_irq = platform_get_irq_byname(pdev, "outbox");
>         if (outbox_irq < 0)
>                 return outbox_irq;
>
> @@ -332,6 +370,24 @@ static int sprd_mbox_probe(struct platform_device *pdev)
>                 return ret;
>         }
>
> +       /* Supplementary outbox IRQ is optional */
> +       supp_irq = platform_get_irq_byname(pdev, "supp-outbox");
> +       if (supp_irq > 0) {
> +               ret = devm_request_irq(dev, supp_irq, sprd_mbox_supp_isr,
> +                                      IRQF_NO_SUSPEND, dev_name(dev), priv);
> +               if (ret) {
> +                       dev_err(dev, "failed to request outbox IRQ: %d\n", ret);
> +                       return ret;
> +               }
> +
> +               supp = (unsigned long) of_device_get_match_data(dev);
> +               if (!supp) {
> +                       dev_err(dev, "no supplementary outbox specified\n");
> +                       return -ENODEV;
> +               }
> +               priv->supp_base = priv->outbox_base + (SPRD_OUTBOX_BASE_SPAN * supp);
> +       }
> +
>         /* Get the default outbox FIFO depth */
>         priv->outbox_fifo_depth =
>                 readl(priv->outbox_base + SPRD_MBOX_FIFO_DEPTH) + 1;
> @@ -354,7 +410,8 @@ static int sprd_mbox_probe(struct platform_device *pdev)
>  }
>
>  static const struct of_device_id sprd_mbox_of_match[] = {
> -       { .compatible = "sprd,sc9860-mailbox", },
> +       { .compatible = "sprd,sc9860-mailbox",
> +         .data = (void *)SPRD_SUPP_INBOX_ID_SC9860 },
>         { },
>  };
>  MODULE_DEVICE_TABLE(of, sprd_mbox_of_match);
> --
> 2.7.4
>


-- 
Baolin Wang