[PATCH v4 1/2] dt-bindings: mailbox: Add the Spreadtrum mailbox documentation

[PATCH v4 1/2] dt-bindings: mailbox: Add the Spreadtrum mailbox documentation

[Baolin Wang]

From: Baolin Wang <baolin.wang@unisoc.com>

Add the Spreadtrum mailbox documentation.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Baolin Wang <baolin.wang@unisoc.com>
Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
---
Changes from v3:
 - None.

Changes from v2:
 - Add reviewed tag from Rob.
 - Remove redundant 'minItems'.

Changes from v1:
 - Add 'additionalProperties'.
 - Split description for each entry.
---
 .../bindings/mailbox/sprd-mailbox.yaml        | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/sprd-mailbox.yaml

diff --git a/Documentation/devicetree/bindings/mailbox/sprd-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/sprd-mailbox.yaml
new file mode 100644
index 000000000000..0f7451b42d7e
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/sprd-mailbox.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/mailbox/sprd-mailbox.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Spreadtrum mailbox controller bindings
+
+maintainers:
+  - Orson Zhai <orsonzhai@gmail.com>
+  - Baolin Wang <baolin.wang7@gmail.com>
+  - Chunyan Zhang <zhang.lyra@gmail.com>
+
+properties:
+  compatible:
+    enum:
+      - sprd,sc9860-mailbox
+
+  reg:
+    items:
+      - description: inbox registers' base address
+      - description: outbox registers' base address
+
+  interrupts:
+    items:
+      - description: inbox interrupt
+      - description: outbox interrupt
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: enable
+
+  "#mbox-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - "#mbox-cells"
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    mailbox: mailbox@400a0000 {
+      compatible = "sprd,sc9860-mailbox";
+      reg = <0 0x400a0000 0 0x8000>, <0 0x400a8000 0 0x8000>;
+      #mbox-cells = <1>;
+      clock-names = "enable";
+      clocks = <&aon_gate 53>;
+      interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
+    };
+...
-- 
2.17.1

[PATCH v4 2/2] mailbox: sprd: Add Spreadtrum mailbox driver

[Baolin Wang]

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 v3:
 - Save the id in mbox_chan.con_priv and remove the 'sprd_mbox_chan'

Changes from v2:
 - None.

Changes from v1:
 - None
---
 drivers/mailbox/Kconfig        |   8 +
 drivers/mailbox/Makefile       |   2 +
 drivers/mailbox/sprd-mailbox.c | 341 +++++++++++++++++++++++++++++++++
 3 files changed, 351 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..f2c04984858a
--- /dev/null
+++ b/drivers/mailbox/sprd-mailbox.c
@@ -0,0 +1,341 @@
+// 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_priv {
+	struct mbox_controller	mbox;
+	struct device		*dev;
+	void __iomem		*inbox_base;
+	void __iomem		*outbox_base;
+	struct clk		*clk;
+	u32			outbox_fifo_depth;
+
+	struct mbox_chan	chan[SPRD_MBOX_CHAN_MAX];
+};
+
+static struct sprd_mbox_priv *to_sprd_mbox_priv(struct mbox_controller *mbox)
+{
+	return container_of(mbox, struct sprd_mbox_priv, mbox);
+}
+
+static u32 sprd_mbox_get_fifo_len(struct sprd_mbox_priv *priv, u32 fifo_sts)
+{
+	u32 wr_pos = (fifo_sts >> SPRD_OUTBOX_FIFO_WR_SHIFT) &
+		SPRD_OUTBOX_FIFO_POS_MASK;
+	u32 rd_pos = (fifo_sts >> SPRD_OUTBOX_FIFO_RD_SHIFT) &
+		SPRD_OUTBOX_FIFO_POS_MASK;
+	u32 fifo_len;
+
+	/*
+	 * If the read pointer is equal with write pointer, which means the fifo
+	 * is full or empty.
+	 */
+	if (wr_pos == rd_pos) {
+		if (fifo_sts & SPRD_OUTBOX_FIFO_FULL)
+			fifo_len = priv->outbox_fifo_depth;
+		else
+			fifo_len = 0;
+	} else if (wr_pos > rd_pos) {
+		fifo_len = wr_pos - rd_pos;
+	} else {
+		fifo_len = priv->outbox_fifo_depth - rd_pos + wr_pos;
+	}
+
+	return fifo_len;
+}
+
+static irqreturn_t sprd_mbox_outbox_isr(int irq, void *data)
+{
+	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_len = sprd_mbox_get_fifo_len(priv, fifo_sts);
+	if (!fifo_len) {
+		dev_warn_ratelimited(priv->dev, "spurious outbox interrupt\n");
+		return IRQ_NONE;
+	}
+
+	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);
+
+		chan = &priv->chan[id];
+		mbox_chan_received_data(chan, (void *)msg);
+
+		/* Trigger to update outbox FIFO pointer */
+		writel(0x1, priv->outbox_base + SPRD_MBOX_TRIGGER);
+	}
+
+	/* Clear irq status after reading all message. */
+	writel(SPRD_MBOX_IRQ_CLR, priv->outbox_base + SPRD_MBOX_IRQ_STS);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t sprd_mbox_inbox_isr(int irq, void *data)
+{
+	struct sprd_mbox_priv *priv = data;
+	struct mbox_chan *chan;
+	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);
+
+		chan = &priv->chan[id];
+		mbox_chan_txdone(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);
+	unsigned long id = (unsigned long)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(id)) {
+		dev_err(priv->dev, "Channel %ld is busy\n", id);
+		return -EBUSY;
+	}
+
+	/* Write data into inbox FIFO, and only support 8 bytes every time */
+	writel(data[0], priv->inbox_base + SPRD_MBOX_MSG_LOW);
+	writel(data[1], priv->inbox_base + SPRD_MBOX_MSG_HIGH);
+
+	/* Set target core id */
+	writel(id, priv->inbox_base + SPRD_MBOX_ID);
+
+	/* Trigger remote request */
+	writel(0x1, priv->inbox_base + SPRD_MBOX_TRIGGER);
+
+	return 0;
+}
+
+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);
+
+	/* 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);
+
+	return 0;
+}
+
+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);
+}
+
+static const struct mbox_chan_ops sprd_mbox_ops = {
+	.send_data    = sprd_mbox_send_data,
+	.startup      = sprd_mbox_startup,
+	.shutdown     = sprd_mbox_shutdown,
+};
+
+static void sprd_mbox_disable(void *data)
+{
+	struct sprd_mbox_priv *priv = data;
+
+	clk_disable_unprepare(priv->clk);
+}
+
+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;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+
+	/*
+	 * The Spreadtrum mailbox uses an inbox to send messages to the target
+	 * core, and uses an outbox to receive messages from other cores.
+	 *
+	 * Thus the mailbox controller supplies 2 different register addresses
+	 * and IRQ numbers for inbox and outbox.
+	 */
+	priv->inbox_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->inbox_base))
+		return PTR_ERR(priv->inbox_base);
+
+	priv->outbox_base = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(priv->outbox_base))
+		return PTR_ERR(priv->outbox_base);
+
+	priv->clk = devm_clk_get(dev, "enable");
+	if (IS_ERR(priv->clk)) {
+		dev_err(dev, "failed to get mailbox clock\n");
+		return PTR_ERR(priv->clk);
+	}
+
+	ret = clk_prepare_enable(priv->clk);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(dev, sprd_mbox_disable, priv);
+	if (ret) {
+		dev_err(dev, "failed to add mailbox disable action\n");
+		return ret;
+	}
+
+	inbox_irq = platform_get_irq(pdev, 0);
+	if (inbox_irq < 0)
+		return inbox_irq;
+
+	ret = devm_request_irq(dev, inbox_irq, sprd_mbox_inbox_isr,
+			       IRQF_NO_SUSPEND, dev_name(dev), priv);
+	if (ret) {
+		dev_err(dev, "failed to request inbox IRQ: %d\n", ret);
+		return ret;
+	}
+
+	outbox_irq = platform_get_irq(pdev, 1);
+	if (outbox_irq < 0)
+		return outbox_irq;
+
+	ret = devm_request_irq(dev, outbox_irq, sprd_mbox_outbox_isr,
+			       IRQF_NO_SUSPEND, dev_name(dev), priv);
+	if (ret) {
+		dev_err(dev, "failed to request outbox IRQ: %d\n", ret);
+		return ret;
+	}
+
+	/* Get the default outbox FIFO depth */
+	priv->outbox_fifo_depth =
+		readl(priv->outbox_base + SPRD_MBOX_FIFO_DEPTH) + 1;
+	priv->mbox.dev = dev;
+	priv->mbox.chans = &priv->chan[0];
+	priv->mbox.num_chans = SPRD_MBOX_CHAN_MAX;
+	priv->mbox.ops = &sprd_mbox_ops;
+	priv->mbox.txdone_irq = true;
+
+	for (id = 0; id < SPRD_MBOX_CHAN_MAX; id++)
+		priv->chan[id].con_priv = (void *)id;
+
+	ret = devm_mbox_controller_register(dev, &priv->mbox);
+	if (ret) {
+		dev_err(dev, "failed to register mailbox: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id sprd_mbox_of_match[] = {
+	{ .compatible = "sprd,sc9860-mailbox", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, sprd_mbox_of_match);
+
+static struct platform_driver sprd_mbox_driver = {
+	.driver = {
+		.name = "sprd-mailbox",
+		.of_match_table = sprd_mbox_of_match,
+	},
+	.probe	= sprd_mbox_probe,
+};
+module_platform_driver(sprd_mbox_driver);
+
+MODULE_AUTHOR("Baolin Wang <baolin.wang@unisoc.com>");
+MODULE_DESCRIPTION("Spreadtrum mailbox driver");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1

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

[Randy Dunlap]

On 4/27/20 8:10 PM, Baolin Wang wrote:
> 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.

	                        ^^typo^^

> +
>  endif


-- 
~Randy

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

[Baolin Wang]

On Tue, Apr 28, 2020 at 11:16 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 4/27/20 8:10 PM, Baolin Wang wrote:
> > 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.
>
>                                 ^^typo^^

Ah, should be 'Spreadtrum', thanks for pointing it out.
Let's wait for Jassi's comments for this patch.

-- 
Baolin Wang

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

[Baolin Wang]

Hi Jassi,

On Tue, Apr 28, 2020 at 11:10 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 v3:
>  - Save the id in mbox_chan.con_priv and remove the 'sprd_mbox_chan'
>
> Changes from v2:
>  - None.
>
> Changes from v1:
>  - None

Gentle ping, do you have any other comments? Thanks.

> ---
>  drivers/mailbox/Kconfig        |   8 +
>  drivers/mailbox/Makefile       |   2 +
>  drivers/mailbox/sprd-mailbox.c | 341 +++++++++++++++++++++++++++++++++
>  3 files changed, 351 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..f2c04984858a
> --- /dev/null
> +++ b/drivers/mailbox/sprd-mailbox.c
> @@ -0,0 +1,341 @@
> +// 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_priv {
> +       struct mbox_controller  mbox;
> +       struct device           *dev;
> +       void __iomem            *inbox_base;
> +       void __iomem            *outbox_base;
> +       struct clk              *clk;
> +       u32                     outbox_fifo_depth;
> +
> +       struct mbox_chan        chan[SPRD_MBOX_CHAN_MAX];
> +};
> +
> +static struct sprd_mbox_priv *to_sprd_mbox_priv(struct mbox_controller *mbox)
> +{
> +       return container_of(mbox, struct sprd_mbox_priv, mbox);
> +}
> +
> +static u32 sprd_mbox_get_fifo_len(struct sprd_mbox_priv *priv, u32 fifo_sts)
> +{
> +       u32 wr_pos = (fifo_sts >> SPRD_OUTBOX_FIFO_WR_SHIFT) &
> +               SPRD_OUTBOX_FIFO_POS_MASK;
> +       u32 rd_pos = (fifo_sts >> SPRD_OUTBOX_FIFO_RD_SHIFT) &
> +               SPRD_OUTBOX_FIFO_POS_MASK;
> +       u32 fifo_len;
> +
> +       /*
> +        * If the read pointer is equal with write pointer, which means the fifo
> +        * is full or empty.
> +        */
> +       if (wr_pos == rd_pos) {
> +               if (fifo_sts & SPRD_OUTBOX_FIFO_FULL)
> +                       fifo_len = priv->outbox_fifo_depth;
> +               else
> +                       fifo_len = 0;
> +       } else if (wr_pos > rd_pos) {
> +               fifo_len = wr_pos - rd_pos;
> +       } else {
> +               fifo_len = priv->outbox_fifo_depth - rd_pos + wr_pos;
> +       }
> +
> +       return fifo_len;
> +}
> +
> +static irqreturn_t sprd_mbox_outbox_isr(int irq, void *data)
> +{
> +       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_len = sprd_mbox_get_fifo_len(priv, fifo_sts);
> +       if (!fifo_len) {
> +               dev_warn_ratelimited(priv->dev, "spurious outbox interrupt\n");
> +               return IRQ_NONE;
> +       }
> +
> +       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);
> +
> +               chan = &priv->chan[id];
> +               mbox_chan_received_data(chan, (void *)msg);
> +
> +               /* Trigger to update outbox FIFO pointer */
> +               writel(0x1, priv->outbox_base + SPRD_MBOX_TRIGGER);
> +       }
> +
> +       /* Clear irq status after reading all message. */
> +       writel(SPRD_MBOX_IRQ_CLR, priv->outbox_base + SPRD_MBOX_IRQ_STS);
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t sprd_mbox_inbox_isr(int irq, void *data)
> +{
> +       struct sprd_mbox_priv *priv = data;
> +       struct mbox_chan *chan;
> +       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);
> +
> +               chan = &priv->chan[id];
> +               mbox_chan_txdone(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);
> +       unsigned long id = (unsigned long)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(id)) {
> +               dev_err(priv->dev, "Channel %ld is busy\n", id);
> +               return -EBUSY;
> +       }
> +
> +       /* Write data into inbox FIFO, and only support 8 bytes every time */
> +       writel(data[0], priv->inbox_base + SPRD_MBOX_MSG_LOW);
> +       writel(data[1], priv->inbox_base + SPRD_MBOX_MSG_HIGH);
> +
> +       /* Set target core id */
> +       writel(id, priv->inbox_base + SPRD_MBOX_ID);
> +
> +       /* Trigger remote request */
> +       writel(0x1, priv->inbox_base + SPRD_MBOX_TRIGGER);
> +
> +       return 0;
> +}
> +
> +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);
> +
> +       /* 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);
> +
> +       return 0;
> +}
> +
> +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);
> +}
> +
> +static const struct mbox_chan_ops sprd_mbox_ops = {
> +       .send_data    = sprd_mbox_send_data,
> +       .startup      = sprd_mbox_startup,
> +       .shutdown     = sprd_mbox_shutdown,
> +};
> +
> +static void sprd_mbox_disable(void *data)
> +{
> +       struct sprd_mbox_priv *priv = data;
> +
> +       clk_disable_unprepare(priv->clk);
> +}
> +
> +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;
> +
> +       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +       if (!priv)
> +               return -ENOMEM;
> +
> +       priv->dev = dev;
> +
> +       /*
> +        * The Spreadtrum mailbox uses an inbox to send messages to the target
> +        * core, and uses an outbox to receive messages from other cores.
> +        *
> +        * Thus the mailbox controller supplies 2 different register addresses
> +        * and IRQ numbers for inbox and outbox.
> +        */
> +       priv->inbox_base = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(priv->inbox_base))
> +               return PTR_ERR(priv->inbox_base);
> +
> +       priv->outbox_base = devm_platform_ioremap_resource(pdev, 1);
> +       if (IS_ERR(priv->outbox_base))
> +               return PTR_ERR(priv->outbox_base);
> +
> +       priv->clk = devm_clk_get(dev, "enable");
> +       if (IS_ERR(priv->clk)) {
> +               dev_err(dev, "failed to get mailbox clock\n");
> +               return PTR_ERR(priv->clk);
> +       }
> +
> +       ret = clk_prepare_enable(priv->clk);
> +       if (ret)
> +               return ret;
> +
> +       ret = devm_add_action_or_reset(dev, sprd_mbox_disable, priv);
> +       if (ret) {
> +               dev_err(dev, "failed to add mailbox disable action\n");
> +               return ret;
> +       }
> +
> +       inbox_irq = platform_get_irq(pdev, 0);
> +       if (inbox_irq < 0)
> +               return inbox_irq;
> +
> +       ret = devm_request_irq(dev, inbox_irq, sprd_mbox_inbox_isr,
> +                              IRQF_NO_SUSPEND, dev_name(dev), priv);
> +       if (ret) {
> +               dev_err(dev, "failed to request inbox IRQ: %d\n", ret);
> +               return ret;
> +       }
> +
> +       outbox_irq = platform_get_irq(pdev, 1);
> +       if (outbox_irq < 0)
> +               return outbox_irq;
> +
> +       ret = devm_request_irq(dev, outbox_irq, sprd_mbox_outbox_isr,
> +                              IRQF_NO_SUSPEND, dev_name(dev), priv);
> +       if (ret) {
> +               dev_err(dev, "failed to request outbox IRQ: %d\n", ret);
> +               return ret;
> +       }
> +
> +       /* Get the default outbox FIFO depth */
> +       priv->outbox_fifo_depth =
> +               readl(priv->outbox_base + SPRD_MBOX_FIFO_DEPTH) + 1;
> +       priv->mbox.dev = dev;
> +       priv->mbox.chans = &priv->chan[0];
> +       priv->mbox.num_chans = SPRD_MBOX_CHAN_MAX;
> +       priv->mbox.ops = &sprd_mbox_ops;
> +       priv->mbox.txdone_irq = true;
> +
> +       for (id = 0; id < SPRD_MBOX_CHAN_MAX; id++)
> +               priv->chan[id].con_priv = (void *)id;
> +
> +       ret = devm_mbox_controller_register(dev, &priv->mbox);
> +       if (ret) {
> +               dev_err(dev, "failed to register mailbox: %d\n", ret);
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id sprd_mbox_of_match[] = {
> +       { .compatible = "sprd,sc9860-mailbox", },
> +       { },
> +};
> +MODULE_DEVICE_TABLE(of, sprd_mbox_of_match);
> +
> +static struct platform_driver sprd_mbox_driver = {
> +       .driver = {
> +               .name = "sprd-mailbox",
> +               .of_match_table = sprd_mbox_of_match,
> +       },
> +       .probe  = sprd_mbox_probe,
> +};
> +module_platform_driver(sprd_mbox_driver);
> +
> +MODULE_AUTHOR("Baolin Wang <baolin.wang@unisoc.com>");
> +MODULE_DESCRIPTION("Spreadtrum mailbox driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.17.1
>


-- 
Baolin Wang

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

[Jassi Brar]

On Wed, May 6, 2020 at 8:29 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
>
> Hi Jassi,
>
> On Tue, Apr 28, 2020 at 11:10 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 v3:
> >  - Save the id in mbox_chan.con_priv and remove the 'sprd_mbox_chan'
> >
> > Changes from v2:
> >  - None.
> >
> > Changes from v1:
> >  - None
>
> Gentle ping, do you have any other comments? Thanks.
>
Yea, I am still not sure about the error returned in send_data().  It
will either never hit or there will be no easy recovery from it. The
api expects the driver to tell it the last-tx was done only when it
can send the next message. (There may be case like sending depend on
remote, which can't be ensured before hand).

thnx

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

[Baolin Wang]

Hi Jassi,

On Thu, May 7, 2020 at 7:25 AM Jassi Brar <jassisinghbrar@gmail.com> wrote:
>
> On Wed, May 6, 2020 at 8:29 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
> >
> > Hi Jassi,
> >
> > On Tue, Apr 28, 2020 at 11:10 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 v3:
> > >  - Save the id in mbox_chan.con_priv and remove the 'sprd_mbox_chan'
> > >
> > > Changes from v2:
> > >  - None.
> > >
> > > Changes from v1:
> > >  - None
> >
> > Gentle ping, do you have any other comments? Thanks.
> >
> Yea, I am still not sure about the error returned in send_data().  It
> will either never hit or there will be no easy recovery from it. The
> api expects the driver to tell it the last-tx was done only when it
> can send the next message. (There may be case like sending depend on
> remote, which can't be ensured before hand).

Actually this is an unusual case, suppose the remote target did not
fetch the message as soon as possile, which will cause the FIFO
overflow, so in this case we  can not send messages to the remote
target any more, otherwise messages will be lost. Thus we can return
errors to users to indicate that something wrong with the remote
target need to be checked.

So this validation in send_data() is mostly for debugging for this
abnormal case and we will not trigger this issue if the remote target
works well. So I think it is useful to keep this validation in
send_data(). Thanks.

-- 
Baolin Wang

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

[Baolin Wang]

Hi Jassi,

On Thu, May 7, 2020 at 11:23 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
>
> Hi Jassi,
>
> On Thu, May 7, 2020 at 7:25 AM Jassi Brar <jassisinghbrar@gmail.com> wrote:
> >
> > On Wed, May 6, 2020 at 8:29 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
> > >
> > > Hi Jassi,
> > >
> > > On Tue, Apr 28, 2020 at 11:10 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 v3:
> > > >  - Save the id in mbox_chan.con_priv and remove the 'sprd_mbox_chan'
> > > >
> > > > Changes from v2:
> > > >  - None.
> > > >
> > > > Changes from v1:
> > > >  - None
> > >
> > > Gentle ping, do you have any other comments? Thanks.
> > >
> > Yea, I am still not sure about the error returned in send_data().  It
> > will either never hit or there will be no easy recovery from it. The
> > api expects the driver to tell it the last-tx was done only when it
> > can send the next message. (There may be case like sending depend on
> > remote, which can't be ensured before hand).
>
> Actually this is an unusual case, suppose the remote target did not
> fetch the message as soon as possile, which will cause the FIFO
> overflow, so in this case we  can not send messages to the remote
> target any more, otherwise messages will be lost. Thus we can return
> errors to users to indicate that something wrong with the remote
> target need to be checked.
>
> So this validation in send_data() is mostly for debugging for this
> abnormal case and we will not trigger this issue if the remote target
> works well. So I think it is useful to keep this validation in
> send_data(). Thanks.

Any comments? Thanks.

-- 
Baolin Wang

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

[Jassi Brar]

On Tue, May 12, 2020 at 11:14 PM Baolin Wang <baolin.wang7@gmail.com> wrote:
>
> Hi Jassi,
>
> On Thu, May 7, 2020 at 11:23 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
> >
> > Hi Jassi,
> >
> > On Thu, May 7, 2020 at 7:25 AM Jassi Brar <jassisinghbrar@gmail.com> wrote:
> > >
> > > On Wed, May 6, 2020 at 8:29 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
> > > >
> > > > Hi Jassi,
> > > >
> > > > On Tue, Apr 28, 2020 at 11:10 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 v3:
> > > > >  - Save the id in mbox_chan.con_priv and remove the 'sprd_mbox_chan'
> > > > >
> > > > > Changes from v2:
> > > > >  - None.
> > > > >
> > > > > Changes from v1:
> > > > >  - None
> > > >
> > > > Gentle ping, do you have any other comments? Thanks.
> > > >
> > > Yea, I am still not sure about the error returned in send_data().  It
> > > will either never hit or there will be no easy recovery from it. The
> > > api expects the driver to tell it the last-tx was done only when it
> > > can send the next message. (There may be case like sending depend on
> > > remote, which can't be ensured before hand).
> >
> > Actually this is an unusual case, suppose the remote target did not
> > fetch the message as soon as possile, which will cause the FIFO
> > overflow, so in this case we  can not send messages to the remote
> > target any more, otherwise messages will be lost. Thus we can return
> > errors to users to indicate that something wrong with the remote
> > target need to be checked.
> >
> > So this validation in send_data() is mostly for debugging for this
> > abnormal case and we will not trigger this issue if the remote target
> > works well. So I think it is useful to keep this validation in
> > send_data(). Thanks.
>
> Any comments? Thanks.
>
Same as my last post.

thnx

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

[Baolin Wang]

On Wed, May 13, 2020 at 2:05 PM Jassi Brar <jassisinghbrar@gmail.com> wrote:
>
> On Tue, May 12, 2020 at 11:14 PM Baolin Wang <baolin.wang7@gmail.com> wrote:
> >
> > Hi Jassi,
> >
> > On Thu, May 7, 2020 at 11:23 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
> > >
> > > Hi Jassi,
> > >
> > > On Thu, May 7, 2020 at 7:25 AM Jassi Brar <jassisinghbrar@gmail.com> wrote:
> > > >
> > > > On Wed, May 6, 2020 at 8:29 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
> > > > >
> > > > > Hi Jassi,
> > > > >
> > > > > On Tue, Apr 28, 2020 at 11:10 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 v3:
> > > > > >  - Save the id in mbox_chan.con_priv and remove the 'sprd_mbox_chan'
> > > > > >
> > > > > > Changes from v2:
> > > > > >  - None.
> > > > > >
> > > > > > Changes from v1:
> > > > > >  - None
> > > > >
> > > > > Gentle ping, do you have any other comments? Thanks.
> > > > >
> > > > Yea, I am still not sure about the error returned in send_data().  It
> > > > will either never hit or there will be no easy recovery from it. The
> > > > api expects the driver to tell it the last-tx was done only when it
> > > > can send the next message. (There may be case like sending depend on
> > > > remote, which can't be ensured before hand).
> > >
> > > Actually this is an unusual case, suppose the remote target did not
> > > fetch the message as soon as possile, which will cause the FIFO
> > > overflow, so in this case we  can not send messages to the remote
> > > target any more, otherwise messages will be lost. Thus we can return
> > > errors to users to indicate that something wrong with the remote
> > > target need to be checked.
> > >
> > > So this validation in send_data() is mostly for debugging for this
> > > abnormal case and we will not trigger this issue if the remote target
> > > works well. So I think it is useful to keep this validation in
> > > send_data(). Thanks.
> >
> > Any comments? Thanks.
> >
> Same as my last post.

I think I've explained the reason why we need add this validation in
my previous email, I am not sure how do you think? You still want to
remove this validation?

-- 
Baolin Wang

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

[Baolin Wang]

Hi Jassi,

On Wed, May 13, 2020 at 2:32 PM Baolin Wang <baolin.wang7@gmail.com> wrote:
>
> On Wed, May 13, 2020 at 2:05 PM Jassi Brar <jassisinghbrar@gmail.com> wrote:
> >
> > On Tue, May 12, 2020 at 11:14 PM Baolin Wang <baolin.wang7@gmail.com> wrote:
> > >
> > > Hi Jassi,
> > >
> > > On Thu, May 7, 2020 at 11:23 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
> > > >
> > > > Hi Jassi,
> > > >
> > > > On Thu, May 7, 2020 at 7:25 AM Jassi Brar <jassisinghbrar@gmail.com> wrote:
> > > > >
> > > > > On Wed, May 6, 2020 at 8:29 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
> > > > > >
> > > > > > Hi Jassi,
> > > > > >
> > > > > > On Tue, Apr 28, 2020 at 11:10 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 v3:
> > > > > > >  - Save the id in mbox_chan.con_priv and remove the 'sprd_mbox_chan'
> > > > > > >
> > > > > > > Changes from v2:
> > > > > > >  - None.
> > > > > > >
> > > > > > > Changes from v1:
> > > > > > >  - None
> > > > > >
> > > > > > Gentle ping, do you have any other comments? Thanks.
> > > > > >
> > > > > Yea, I am still not sure about the error returned in send_data().  It
> > > > > will either never hit or there will be no easy recovery from it. The
> > > > > api expects the driver to tell it the last-tx was done only when it
> > > > > can send the next message. (There may be case like sending depend on
> > > > > remote, which can't be ensured before hand).
> > > >
> > > > Actually this is an unusual case, suppose the remote target did not
> > > > fetch the message as soon as possile, which will cause the FIFO
> > > > overflow, so in this case we  can not send messages to the remote
> > > > target any more, otherwise messages will be lost. Thus we can return
> > > > errors to users to indicate that something wrong with the remote
> > > > target need to be checked.
> > > >
> > > > So this validation in send_data() is mostly for debugging for this
> > > > abnormal case and we will not trigger this issue if the remote target
> > > > works well. So I think it is useful to keep this validation in
> > > > send_data(). Thanks.
> > >
> > > Any comments? Thanks.
> > >
> > Same as my last post.
>
> I think I've explained the reason why we need add this validation in
> my previous email, I am not sure how do you think? You still want to
> remove this validation?

Gentle ping.

As I explained in previous email, this validation is for an unusual
case, suppose the remote target did not fetch the message as soon as
possile, which will cause the FIFO overflow, so in this case we  can
not send messages to the remote
target any more, otherwise messages will be lost. Thus we can return
errors to users to indicate that something wrong with the remote
target need to be checked.

So this validation in send_data() is mostly for debugging for this
abnormal case and we will not trigger this issue if the remote target
works well. So I think it is useful to keep this validation in
send_data(). What do you think? Thanks.

-- 
Baolin Wang

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

[Jassi Brar]

On Thu, May 21, 2020 at 7:24 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
>
> Hi Jassi,
>
> On Wed, May 13, 2020 at 2:32 PM Baolin Wang <baolin.wang7@gmail.com> wrote:
> >
> > On Wed, May 13, 2020 at 2:05 PM Jassi Brar <jassisinghbrar@gmail.com> wrote:
> > >
> > > On Tue, May 12, 2020 at 11:14 PM Baolin Wang <baolin.wang7@gmail.com> wrote:
> > > >
> > > > Hi Jassi,
> > > >
> > > > On Thu, May 7, 2020 at 11:23 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
> > > > >
> > > > > Hi Jassi,
> > > > >
> > > > > On Thu, May 7, 2020 at 7:25 AM Jassi Brar <jassisinghbrar@gmail.com> wrote:
> > > > > >
> > > > > > On Wed, May 6, 2020 at 8:29 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
> > > > > > >
> > > > > > > Hi Jassi,
> > > > > > >
> > > > > > > On Tue, Apr 28, 2020 at 11:10 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 v3:
> > > > > > > >  - Save the id in mbox_chan.con_priv and remove the 'sprd_mbox_chan'
> > > > > > > >
> > > > > > > > Changes from v2:
> > > > > > > >  - None.
> > > > > > > >
> > > > > > > > Changes from v1:
> > > > > > > >  - None
> > > > > > >
> > > > > > > Gentle ping, do you have any other comments? Thanks.
> > > > > > >
> > > > > > Yea, I am still not sure about the error returned in send_data().  It
> > > > > > will either never hit or there will be no easy recovery from it. The
> > > > > > api expects the driver to tell it the last-tx was done only when it
> > > > > > can send the next message. (There may be case like sending depend on
> > > > > > remote, which can't be ensured before hand).
> > > > >
> > > > > Actually this is an unusual case, suppose the remote target did not
> > > > > fetch the message as soon as possile, which will cause the FIFO
> > > > > overflow, so in this case we  can not send messages to the remote
> > > > > target any more, otherwise messages will be lost. Thus we can return
> > > > > errors to users to indicate that something wrong with the remote
> > > > > target need to be checked.
> > > > >
> > > > > So this validation in send_data() is mostly for debugging for this
> > > > > abnormal case and we will not trigger this issue if the remote target
> > > > > works well. So I think it is useful to keep this validation in
> > > > > send_data(). Thanks.
> > > >
> > > > Any comments? Thanks.
> > > >
> > > Same as my last post.
> >
> > I think I've explained the reason why we need add this validation in
> > my previous email, I am not sure how do you think? You still want to
> > remove this validation?
>
> Gentle ping.
>
> As I explained in previous email, this validation is for an unusual
> case, suppose the remote target did not fetch the message as soon as
> possile, which will cause the FIFO overflow, so in this case we  can
> not send messages to the remote
> target any more, otherwise messages will be lost. Thus we can return
> errors to users to indicate that something wrong with the remote
> target need to be checked.
>
> So this validation in send_data() is mostly for debugging for this
> abnormal case and we will not trigger this issue if the remote target
> works well. So I think it is useful to keep this validation in
> send_data(). What do you think? Thanks.
>
I still think the same as before.
You should do this check before you call mbox_chan_txdone() and wait
if busy ... which is exactly the purpose of txdone().
It seems harmless to be paranoid and place a block of code in
practically "if 0", but that sets bad precedence for other drivers. So
please move the check before txdone().

thanks.

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

[Baolin Wang]

On Fri, May 22, 2020 at 11:48 AM Jassi Brar <jassisinghbrar@gmail.com> wrote:
>
> On Thu, May 21, 2020 at 7:24 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
> >
> > Hi Jassi,
> >
> > On Wed, May 13, 2020 at 2:32 PM Baolin Wang <baolin.wang7@gmail.com> wrote:
> > >
> > > On Wed, May 13, 2020 at 2:05 PM Jassi Brar <jassisinghbrar@gmail.com> wrote:
> > > >
> > > > On Tue, May 12, 2020 at 11:14 PM Baolin Wang <baolin.wang7@gmail.com> wrote:
> > > > >
> > > > > Hi Jassi,
> > > > >
> > > > > On Thu, May 7, 2020 at 11:23 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
> > > > > >
> > > > > > Hi Jassi,
> > > > > >
> > > > > > On Thu, May 7, 2020 at 7:25 AM Jassi Brar <jassisinghbrar@gmail.com> wrote:
> > > > > > >
> > > > > > > On Wed, May 6, 2020 at 8:29 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
> > > > > > > >
> > > > > > > > Hi Jassi,
> > > > > > > >
> > > > > > > > On Tue, Apr 28, 2020 at 11:10 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 v3:
> > > > > > > > >  - Save the id in mbox_chan.con_priv and remove the 'sprd_mbox_chan'
> > > > > > > > >
> > > > > > > > > Changes from v2:
> > > > > > > > >  - None.
> > > > > > > > >
> > > > > > > > > Changes from v1:
> > > > > > > > >  - None
> > > > > > > >
> > > > > > > > Gentle ping, do you have any other comments? Thanks.
> > > > > > > >
> > > > > > > Yea, I am still not sure about the error returned in send_data().  It
> > > > > > > will either never hit or there will be no easy recovery from it. The
> > > > > > > api expects the driver to tell it the last-tx was done only when it
> > > > > > > can send the next message. (There may be case like sending depend on
> > > > > > > remote, which can't be ensured before hand).
> > > > > >
> > > > > > Actually this is an unusual case, suppose the remote target did not
> > > > > > fetch the message as soon as possile, which will cause the FIFO
> > > > > > overflow, so in this case we  can not send messages to the remote
> > > > > > target any more, otherwise messages will be lost. Thus we can return
> > > > > > errors to users to indicate that something wrong with the remote
> > > > > > target need to be checked.
> > > > > >
> > > > > > So this validation in send_data() is mostly for debugging for this
> > > > > > abnormal case and we will not trigger this issue if the remote target
> > > > > > works well. So I think it is useful to keep this validation in
> > > > > > send_data(). Thanks.
> > > > >
> > > > > Any comments? Thanks.
> > > > >
> > > > Same as my last post.
> > >
> > > I think I've explained the reason why we need add this validation in
> > > my previous email, I am not sure how do you think? You still want to
> > > remove this validation?
> >
> > Gentle ping.
> >
> > As I explained in previous email, this validation is for an unusual
> > case, suppose the remote target did not fetch the message as soon as
> > possile, which will cause the FIFO overflow, so in this case we  can
> > not send messages to the remote
> > target any more, otherwise messages will be lost. Thus we can return
> > errors to users to indicate that something wrong with the remote
> > target need to be checked.
> >
> > So this validation in send_data() is mostly for debugging for this
> > abnormal case and we will not trigger this issue if the remote target
> > works well. So I think it is useful to keep this validation in
> > send_data(). What do you think? Thanks.
> >
> I still think the same as before.
> You should do this check before you call mbox_chan_txdone() and wait
> if busy ... which is exactly the purpose of txdone().
> It seems harmless to be paranoid and place a block of code in
> practically "if 0", but that sets bad precedence for other drivers. So
> please move the check before txdone().

OK. I realized I can implement the flush() to make sure the
transmission has been completed. Thanks.

--
Baolin Wang