diff mbox series

[v2,1/3] mailbox: sprd: Introduce refcnt when clients requests/free channels

Message ID 20210213122143.19240-2-orsonzhai@gmail.com
State Superseded
Headers show
Series [v2,1/3] mailbox: sprd: Introduce refcnt when clients requests/free channels | expand

Commit Message

Orson Zhai Feb. 13, 2021, 12:21 p.m. UTC
From: Orson Zhai <orson.zhai@unisoc.com>

Unisoc mailbox has no way to be enabled/disabled for any single channel.
They can only be set to startup or shutdown as a whole device at same time.

Add a variable to count references to avoid mailbox FIFO being reset
unexpectedly when clients are requesting or freeing channels.

Also add a lock to dismiss possible conflicts from register r/w in
different startup or shutdown threads. And fix the crash problem when early
interrupts come from channel which has not been requested by client yet.

Fixes: ca27fc26cd22 ("mailbox: sprd: Add Spreadtrum mailbox driver")
Signed-off-by: Orson Zhai <orson.zhai@unisoc.com>
---
 drivers/mailbox/sprd-mailbox.c | 43 +++++++++++++++++++++++-----------
 1 file changed, 29 insertions(+), 14 deletions(-)

Comments

Baolin Wang March 7, 2021, 3:23 p.m. UTC | #1
On Sat, Feb 13, 2021 at 8:22 PM Orson Zhai <orsonzhai@gmail.com> wrote:
>

> From: Orson Zhai <orson.zhai@unisoc.com>

>

> Unisoc mailbox has no way to be enabled/disabled for any single channel.

> They can only be set to startup or shutdown as a whole device at same time.

>

> Add a variable to count references to avoid mailbox FIFO being reset

> unexpectedly when clients are requesting or freeing channels.

>

> Also add a lock to dismiss possible conflicts from register r/w in

> different startup or shutdown threads. And fix the crash problem when early

> interrupts come from channel which has not been requested by client yet.

>

> Fixes: ca27fc26cd22 ("mailbox: sprd: Add Spreadtrum mailbox driver")

> Signed-off-by: Orson Zhai <orson.zhai@unisoc.com>


Sorry for the late reply. LGTM.
Reviewed-by: Baolin Wang <baolin.wang7@gmail.com>


> ---

>  drivers/mailbox/sprd-mailbox.c | 43 +++++++++++++++++++++++-----------

>  1 file changed, 29 insertions(+), 14 deletions(-)

>

> diff --git a/drivers/mailbox/sprd-mailbox.c b/drivers/mailbox/sprd-mailbox.c

> index f6fab24ae8a9..920de7b9dce1 100644

> --- a/drivers/mailbox/sprd-mailbox.c

> +++ b/drivers/mailbox/sprd-mailbox.c

> @@ -60,6 +60,8 @@ struct sprd_mbox_priv {

>         struct clk              *clk;

>         u32                     outbox_fifo_depth;

>

> +       struct mutex            lock;

> +       u32                     refcnt;

>         struct mbox_chan        chan[SPRD_MBOX_CHAN_MAX];

>  };

>

> @@ -115,7 +117,11 @@ static irqreturn_t sprd_mbox_outbox_isr(int irq, void *data)

>                 id = readl(priv->outbox_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);

> @@ -215,18 +221,22 @@ static int sprd_mbox_startup(struct mbox_chan *chan)

>         struct sprd_mbox_priv *priv = to_sprd_mbox_priv(chan->mbox);

>         u32 val;

>

> -       /* Select outbox FIFO mode and reset the outbox FIFO status */

> -       writel(0x0, priv->outbox_base + SPRD_MBOX_FIFO_RST);

> +       mutex_lock(&priv->lock);

> +       if (priv->refcnt++ == 0) {

> +               /* Select outbox FIFO mode and reset the outbox FIFO status */

> +               writel(0x0, priv->outbox_base + SPRD_MBOX_FIFO_RST);

>

> -       /* Enable inbox FIFO overflow and delivery interrupt */

> -       val = readl(priv->inbox_base + SPRD_MBOX_IRQ_MSK);

> -       val &= ~(SPRD_INBOX_FIFO_OVERFLOW_IRQ | SPRD_INBOX_FIFO_DELIVER_IRQ);

> -       writel(val, priv->inbox_base + SPRD_MBOX_IRQ_MSK);

> +               /* Enable inbox FIFO overflow and delivery interrupt */

> +               val = readl(priv->inbox_base + SPRD_MBOX_IRQ_MSK);

> +               val &= ~(SPRD_INBOX_FIFO_OVERFLOW_IRQ | SPRD_INBOX_FIFO_DELIVER_IRQ);

> +               writel(val, priv->inbox_base + SPRD_MBOX_IRQ_MSK);

>

> -       /* Enable outbox FIFO not empty interrupt */

> -       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 outbox FIFO not empty interrupt */

> +               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);

> +       }

> +       mutex_unlock(&priv->lock);

>

>         return 0;

>  }

> @@ -235,9 +245,13 @@ static void sprd_mbox_shutdown(struct mbox_chan *chan)

>  {

>         struct sprd_mbox_priv *priv = to_sprd_mbox_priv(chan->mbox);

>

> -       /* 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);

> +       mutex_lock(&priv->lock);

> +       if (--priv->refcnt == 0) {

> +               /* 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);

> +       }

> +       mutex_unlock(&priv->lock);

>  }

>

>  static const struct mbox_chan_ops sprd_mbox_ops = {

> @@ -266,6 +280,7 @@ static int sprd_mbox_probe(struct platform_device *pdev)

>                 return -ENOMEM;

>

>         priv->dev = dev;

> +       mutex_init(&priv->lock);

>

>         /*

>          * The Spreadtrum mailbox uses an inbox to send messages to the target

> --

> 2.17.1

>



-- 
Baolin Wang
diff mbox series

Patch

diff --git a/drivers/mailbox/sprd-mailbox.c b/drivers/mailbox/sprd-mailbox.c
index f6fab24ae8a9..920de7b9dce1 100644
--- a/drivers/mailbox/sprd-mailbox.c
+++ b/drivers/mailbox/sprd-mailbox.c
@@ -60,6 +60,8 @@  struct sprd_mbox_priv {
 	struct clk		*clk;
 	u32			outbox_fifo_depth;
 
+	struct mutex		lock;
+	u32			refcnt;
 	struct mbox_chan	chan[SPRD_MBOX_CHAN_MAX];
 };
 
@@ -115,7 +117,11 @@  static irqreturn_t sprd_mbox_outbox_isr(int irq, void *data)
 		id = readl(priv->outbox_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);
@@ -215,18 +221,22 @@  static int sprd_mbox_startup(struct mbox_chan *chan)
 	struct sprd_mbox_priv *priv = to_sprd_mbox_priv(chan->mbox);
 	u32 val;
 
-	/* Select outbox FIFO mode and reset the outbox FIFO status */
-	writel(0x0, priv->outbox_base + SPRD_MBOX_FIFO_RST);
+	mutex_lock(&priv->lock);
+	if (priv->refcnt++ == 0) {
+		/* Select outbox FIFO mode and reset the outbox FIFO status */
+		writel(0x0, priv->outbox_base + SPRD_MBOX_FIFO_RST);
 
-	/* Enable inbox FIFO overflow and delivery interrupt */
-	val = readl(priv->inbox_base + SPRD_MBOX_IRQ_MSK);
-	val &= ~(SPRD_INBOX_FIFO_OVERFLOW_IRQ | SPRD_INBOX_FIFO_DELIVER_IRQ);
-	writel(val, priv->inbox_base + SPRD_MBOX_IRQ_MSK);
+		/* Enable inbox FIFO overflow and delivery interrupt */
+		val = readl(priv->inbox_base + SPRD_MBOX_IRQ_MSK);
+		val &= ~(SPRD_INBOX_FIFO_OVERFLOW_IRQ | SPRD_INBOX_FIFO_DELIVER_IRQ);
+		writel(val, priv->inbox_base + SPRD_MBOX_IRQ_MSK);
 
-	/* Enable outbox FIFO not empty interrupt */
-	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 outbox FIFO not empty interrupt */
+		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);
+	}
+	mutex_unlock(&priv->lock);
 
 	return 0;
 }
@@ -235,9 +245,13 @@  static void sprd_mbox_shutdown(struct mbox_chan *chan)
 {
 	struct sprd_mbox_priv *priv = to_sprd_mbox_priv(chan->mbox);
 
-	/* 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);
+	mutex_lock(&priv->lock);
+	if (--priv->refcnt == 0) {
+		/* 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);
+	}
+	mutex_unlock(&priv->lock);
 }
 
 static const struct mbox_chan_ops sprd_mbox_ops = {
@@ -266,6 +280,7 @@  static int sprd_mbox_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	priv->dev = dev;
+	mutex_init(&priv->lock);
 
 	/*
 	 * The Spreadtrum mailbox uses an inbox to send messages to the target