[PATCH v3 0/3] Add support for Hi3660 mailbox driver

[PATCH v3 0/3] Add support for Hi3660 mailbox driver

[Xu YiPing]

From: Leo Yan <leo.yan@linaro.org>

Hi3660 mailbox controller is used to send message within multiple
processors, MCU, HIFI, etc. This patch series is to implement an
initial version for Hi3660 mailbox driver with "automatic
acknowledge" mode.

The patch set have been verified with Hi3660 stub clock driver, so
we can send message to MCU to execute CPU frequency scaling. This is
tested on 96boards Hikey960.

Changes from v2:
* According to Mark Rutland suggestions, removed sev()/wfe() from
  driver, the system has no two masters sharing the same channel for
  data transferring so we don't need these instructions;
* Refined DT binding and doc according to Rob suggestions;
* Refined driver according to Julien suggestions;

Changes from v1:
* Added cover letter to track the changelog;
* Added document for DT binding;
* Refactored and simplized mailbox driver with "automatic ack" mode;
* Refined commit logs for patches;


Kaihua Zhong (2):
  mailbox: Add support for Hi3660 mailbox
  dts: arm64: Add mailbox binding for hi3660

Leo Yan (1):
  dt-bindings: mailbox: Introduce Hi3660 controller binding

 .../bindings/mailbox/hisilicon,hi3660-mailbox.txt  |  51 ++++
 arch/arm64/boot/dts/hisilicon/hi3660.dtsi          |   8 +
 drivers/mailbox/Kconfig                            |   8 +
 drivers/mailbox/Makefile                           |   2 +
 drivers/mailbox/hi3660-mailbox.c                   | 322 +++++++++++++++++++++
 5 files changed, 391 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/hisilicon,hi3660-mailbox.txt
 create mode 100644 drivers/mailbox/hi3660-mailbox.c

-- 
1.9.1

[PATCH v3 1/3] dt-bindings: mailbox: Introduce Hi3660 controller binding

[Xu YiPing]

From: Leo Yan <leo.yan@linaro.org>

Introduce a binding for the Hi3660 mailbox controller, the mailbox is
used within application processor (AP), communication processor (CP),
HIFI and MCU, etc.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 .../bindings/mailbox/hisilicon,hi3660-mailbox.txt  | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/hisilicon,hi3660-mailbox.txt

diff --git a/Documentation/devicetree/bindings/mailbox/hisilicon,hi3660-mailbox.txt b/Documentation/devicetree/bindings/mailbox/hisilicon,hi3660-mailbox.txt
new file mode 100644
index 0000000..3e5b453
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/hisilicon,hi3660-mailbox.txt
@@ -0,0 +1,51 @@
+Hisilicon Hi3660 Mailbox Controller
+
+Hisilicon Hi3660 mailbox controller supports up to 32 channels.  Messages
+are passed between processors, including application & communication
+processors, MCU, HIFI, etc.  Each channel is unidirectional and accessed
+by using MMIO registers; it supports maximum to 8 words message.
+
+Controller
+----------
+
+Required properties:
+- compatible:		: Shall be "hisilicon,hi3660-mbox"
+- reg:			: Offset and length of the device's register set
+- #mbox-cells:		: Must be 3
+			  <&phandle channel dst_irq ack_irq>
+			    phandle	: Label name of controller
+			    channel	: Channel number
+			    dst_irq	: Remote interrupt vector
+			    ack_irq	: Local interrupt vector
+
+- interrupts:		: Contains the two IRQ lines for mailbox.
+
+Example:
+
+mailbox: mailbox@e896b000 {
+	compatible = "hisilicon,hi3660-mbox";
+	reg = <0x0 0xe896b000 0x0 0x1000>;
+	interrupts = <0x0 0xc0 0x4>,
+		     <0x0 0xc1 0x4>;
+	#mbox-cells = <3>;
+};
+
+Client
+------
+
+Required properties:
+- compatible		: See the client docs
+- mboxes		: Standard property to specify a Mailbox (See ./mailbox.txt)
+			  Cells must match 'mbox-cells' (See Controller docs above)
+
+Optional properties
+- mbox-names		: Name given to channels seen in the 'mboxes' property.
+
+Example:
+
+stub_clock: stub_clock@e896b500 {
+	compatible = "hisilicon,hi3660-stub-clk";
+	reg = <0x0 0xe896b500 0x0 0x0100>;
+	#clock-cells = <1>;
+	mboxes = <&mailbox 13 3 0>;
+};
-- 
1.9.1

[PATCH v3 2/3] mailbox: Add support for Hi3660 mailbox

[Xu YiPing]

From: Kaihua Zhong <zhongkaihua@huawei.com>

Hi3660 mailbox controller is used to send message within multiple
processors, MCU, HIFI, etc.  It supports 32 mailbox channels and every
channel can only be used for single transferring direction.  Once the
channel is enabled, it needs to specify the destination interrupt and
acknowledge interrupt, these two interrupt vectors are used to create
the connection between the mailbox and interrupt controllers.

The data transferring supports two modes, one is named as "automatic
acknowledge" mode so after send message the kernel doesn't need to wait
for acknowledge from remote and directly return; there have another mode
is to rely on handling interrupt for acknowledge.

This commit is for initial version driver, which only supports
"automatic acknowledge" mode to support CPU clock, which is the only
one consumer to use mailbox and has been verified.  Later may enhance
this driver for interrupt mode (e.g. for supporting HIFI).

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Signed-off-by: Ruyi Wang <wangruyi@huawei.com>
Signed-off-by: Kaihua Zhong <zhongkaihua@huawei.com>
---
 drivers/mailbox/Kconfig          |   8 +
 drivers/mailbox/Makefile         |   2 +
 drivers/mailbox/hi3660-mailbox.c | 322 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 332 insertions(+)
 create mode 100644 drivers/mailbox/hi3660-mailbox.c

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index c5731e5..4b5d6e9 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -108,6 +108,14 @@ config TI_MESSAGE_MANAGER
 	  multiple processors within the SoC. Select this driver if your
 	  platform has support for the hardware block.
 
+config HI3660_MBOX
+	tristate "Hi3660 Mailbox"
+	depends on ARCH_HISI && OF
+	help
+	  An implementation of the hi3660 mailbox. It is used to send message
+	  between application processors and other processors/MCU/DSP. Select
+	  Y here if you want to use Hi3660 mailbox controller.
+
 config HI6220_MBOX
 	tristate "Hi6220 Mailbox"
 	depends on ARCH_HISI
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index d54e412..7d1bd51 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -26,6 +26,8 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o
 
 obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o
 
+obj-$(CONFIG_HI3660_MBOX)	+= hi3660-mailbox.o
+
 obj-$(CONFIG_HI6220_MBOX)	+= hi6220-mailbox.o
 
 obj-$(CONFIG_BCM_PDC_MBOX)	+= bcm-pdc-mailbox.o
diff --git a/drivers/mailbox/hi3660-mailbox.c b/drivers/mailbox/hi3660-mailbox.c
new file mode 100644
index 0000000..ba80834
--- /dev/null
+++ b/drivers/mailbox/hi3660-mailbox.c
@@ -0,0 +1,322 @@
+/*
+ * Hisilicon's Hi3660 mailbox controller driver
+ *
+ * Copyright (c) 2017 Hisilicon Limited.
+ * Copyright (c) 2017 Linaro Limited.
+ *
+ * Author: Leo Yan <leo.yan@linaro.org>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include "mailbox.h"
+
+#define MBOX_CHAN_MAX			32
+
+#define MBOX_RX				(0x0)
+#define MBOX_TX				(0x1)
+
+#define MBOX_BASE(mbox, ch)		((mbox)->base + ((ch) * 0x40))
+#define MBOX_SRC_REG			(0x00)
+#define MBOX_DST_REG			(0x04)
+#define MBOX_DCLR_REG			(0x08)
+#define MBOX_DSTAT_REG			(0x0c)
+#define MBOX_MODE_REG			(0x10)
+#define MBOX_IMASK_REG			(0x14)
+#define MBOX_ICLR_REG			(0x18)
+#define MBOX_SEND_REG			(0x1c)
+#define MBOX_DATA_REG			(0x20)
+
+#define MBOX_IPC_LOCK_REG		(0xa00)
+#define MBOX_IPC_UNLOCK			(0x1acce551)
+
+#define MBOX_AUTOMATIC_ACK		(1)
+
+#define MBOX_STATE_IDLE			BIT(4)
+#define MBOX_STATE_ACK			BIT(7)
+
+#define MBOX_MSG_LEN			8
+
+/**
+ * Hi3660 mailbox channel device data
+ *
+ * A channel can be used for TX or RX, it can trigger remote
+ * processor interrupt to notify remote processor and can receive
+ * interrupt if has incoming message.
+ *
+ * @dst_irq:	Interrupt vector for remote processor
+ * @ack_irq:	Interrupt vector for local processor
+ */
+struct hi3660_mbox_dev {
+	unsigned int dst_irq;
+	unsigned int ack_irq;
+};
+
+/**
+ * Hi3660 mailbox controller data
+ *
+ * Mailbox controller includes 32 channels and can allocate
+ * channel for message transferring.
+ *
+ * @dev:	Device to which it is attached
+ * @base:	Base address of the register mapping region
+ * @chan:	Representation of channels in mailbox controller
+ * @mdev:	Representation of channel device data
+ * @controller:	Representation of a communication channel controller
+ */
+struct hi3660_mbox {
+	struct device *dev;
+	void __iomem *base;
+	struct mbox_chan chan[MBOX_CHAN_MAX];
+	struct hi3660_mbox_dev mdev[MBOX_CHAN_MAX];
+	struct mbox_controller controller;
+};
+
+static inline struct hi3660_mbox *to_hi3660_mbox(struct mbox_controller *mbox)
+{
+	return container_of(mbox, struct hi3660_mbox, controller);
+}
+
+static int hi3660_mbox_check_state(struct mbox_chan *chan)
+{
+	unsigned long ch = (unsigned long)chan->con_priv;
+	struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
+	struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
+	void __iomem *base = MBOX_BASE(mbox, ch);
+	unsigned long val;
+	unsigned int state, ret;
+
+	/* Mailbox is idle so directly bail out */
+	state = readl_relaxed(base + MBOX_MODE_REG);
+	if (state & MBOX_STATE_IDLE)
+		return 0;
+
+	/* Wait for acknowledge from remote */
+	ret = readx_poll_timeout_atomic(readl_relaxed, base + MBOX_MODE_REG,
+			val, (val & MBOX_STATE_ACK), 1000, 300000);
+	if (ret) {
+		dev_err(mbox->dev, "%s: timeout for receiving ack\n", __func__);
+		return ret;
+	}
+
+	/* Ensure channel is released */
+	writel_relaxed(0xffffffff, base + MBOX_IMASK_REG);
+	writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SRC_REG);
+
+	return 0;
+}
+
+static int hi3660_mbox_unlock(struct mbox_chan *chan)
+{
+	struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
+	unsigned int val, retry = 3;
+
+	do {
+		writel_relaxed(MBOX_IPC_UNLOCK, mbox->base + MBOX_IPC_LOCK_REG);
+
+		val = readl_relaxed(mbox->base + MBOX_IPC_LOCK_REG);
+		if (!val)
+			break;
+
+		udelay(10);
+	} while (retry--);
+
+	return (!val) ? 0 : -ETIMEDOUT;
+}
+
+static int hi3660_mbox_acquire_channel(struct mbox_chan *chan)
+{
+	unsigned long ch = (unsigned long)chan->con_priv;
+	struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
+	struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
+	void __iomem *base = MBOX_BASE(mbox, ch);
+	unsigned int val, retry;
+
+	for (retry = 10; retry; retry--) {
+		/* Check if channel has been acquired */
+		if (readl_relaxed(base + MBOX_MODE_REG) & MBOX_STATE_IDLE) {
+			writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SRC_REG);
+			val = readl_relaxed(base + MBOX_SRC_REG);
+			if (val & BIT(mdev->ack_irq))
+				break;
+		}
+	}
+
+	return retry ? 0 : -ETIMEDOUT;
+}
+
+static int hi3660_mbox_send(struct mbox_chan *chan, u32 *msg)
+{
+	unsigned long ch = (unsigned long)chan->con_priv;
+	struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
+	struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
+	void __iomem *base = MBOX_BASE(mbox, ch);
+	unsigned int i;
+
+	/* Clear mask for destination interrupt */
+	writel_relaxed(~BIT(mdev->dst_irq), base + MBOX_IMASK_REG);
+
+	/* Config destination for interrupt vector */
+	writel_relaxed(BIT(mdev->dst_irq), base + MBOX_DST_REG);
+
+	/* Automatic acknowledge mode */
+	writel_relaxed(MBOX_AUTOMATIC_ACK, base + MBOX_MODE_REG);
+
+	/* Fill message data */
+	for (i = 0; i < MBOX_MSG_LEN; i++)
+		writel_relaxed(msg[i], base + MBOX_DATA_REG + i * 4);
+
+	/* Trigger data transferring */
+	writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SEND_REG);
+	return 0;
+}
+
+static int hi3660_mbox_send_data(struct mbox_chan *chan, void *msg)
+{
+	struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
+	int err;
+
+	err = hi3660_mbox_check_state(chan);
+	if (err) {
+		dev_err(mbox->dev, "checking state failed\n");
+		return err;
+	}
+
+	err = hi3660_mbox_unlock(chan);
+	if (err) {
+		dev_err(mbox->dev, "unlocking mailbox failed\n");
+		return err;
+	}
+
+	err = hi3660_mbox_acquire_channel(chan);
+	if (err) {
+		dev_err(mbox->dev, "acquiring channel failed\n");
+		return err;
+	}
+
+	return hi3660_mbox_send(chan, msg);
+}
+
+static struct mbox_chan_ops hi3660_mbox_ops = {
+	.send_data    = hi3660_mbox_send_data,
+};
+
+static struct mbox_chan *hi3660_mbox_xlate(struct mbox_controller *controller,
+					   const struct of_phandle_args *spec)
+{
+	struct hi3660_mbox *mbox = to_hi3660_mbox(controller);
+	struct hi3660_mbox_dev *mdev;
+	unsigned int ch = spec->args[0];
+
+	if (ch >= MBOX_CHAN_MAX) {
+		dev_err(mbox->dev, "Invalid channel idx %d\n", ch);
+		return ERR_PTR(-EINVAL);
+	}
+
+	mdev = &mbox->mdev[ch];
+	mdev->dst_irq = spec->args[1];
+	mdev->ack_irq = spec->args[2];
+
+	return &mbox->chan[ch];
+}
+
+static const struct of_device_id hi3660_mbox_of_match[] = {
+	{ .compatible = "hisilicon,hi3660-mbox", },
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, hi3660_mbox_of_match);
+
+static int hi3660_mbox_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct hi3660_mbox *mbox;
+	struct mbox_chan *chan;
+	struct resource *res;
+	unsigned long ch;
+	int err;
+
+	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
+	if (!mbox)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mbox->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(mbox->base))
+		return PTR_ERR(mbox->base);
+
+	mbox->dev = dev;
+	mbox->controller.dev = dev;
+	mbox->controller.chans = mbox->chan;
+	mbox->controller.num_chans = MBOX_CHAN_MAX;
+	mbox->controller.ops = &hi3660_mbox_ops;
+	mbox->controller.of_xlate = hi3660_mbox_xlate;
+
+	/* Initialize mailbox channel data */
+	chan = mbox->chan;
+	for (ch = 0; ch < MBOX_CHAN_MAX; ch++)
+		chan[ch].con_priv = (void *)ch;
+
+	err = mbox_controller_register(&mbox->controller);
+	if (err) {
+		dev_err(dev, "Failed to register mailbox %d\n", err);
+		return err;
+	}
+
+	platform_set_drvdata(pdev, mbox);
+	dev_info(dev, "Mailbox enabled\n");
+	return 0;
+}
+
+static int hi3660_mbox_remove(struct platform_device *pdev)
+{
+	struct hi3660_mbox *mbox = platform_get_drvdata(pdev);
+
+	mbox_controller_unregister(&mbox->controller);
+	return 0;
+}
+
+static struct platform_driver hi3660_mbox_driver = {
+	.probe  = hi3660_mbox_probe,
+	.remove = hi3660_mbox_remove,
+	.driver = {
+		.name = "hi3660-mbox",
+		.of_match_table = hi3660_mbox_of_match,
+	},
+};
+
+static int __init hi3660_mbox_init(void)
+{
+	return platform_driver_register(&hi3660_mbox_driver);
+}
+core_initcall(hi3660_mbox_init);
+
+static void __exit hi3660_mbox_exit(void)
+{
+	platform_driver_unregister(&hi3660_mbox_driver);
+}
+module_exit(hi3660_mbox_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Hisilicon Hi3660 Mailbox Controller");
+MODULE_AUTHOR("Leo Yan <leo.yan@linaro.org>");
-- 
1.9.1

[PATCH v3 3/3] dts: arm64: Add mailbox binding for hi3660

[Xu YiPing]

From: Kaihua Zhong <zhongkaihua@huawei.com>

Add DT binding for mailbox driver.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Signed-off-by: Ruyi Wang <wangruyi@huawei.com>
Signed-off-by: Kaihua Zhong <zhongkaihua@huawei.com>
---
 arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
index 610990f..451b6bf 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
@@ -285,6 +285,14 @@
 			#reset-cells = <2>;
 		};
 
+		mailbox: mailbox@e896b000 {
+			compatible = "hisilicon,hi3660-mbox";
+			reg = <0x0 0xe896b000 0x0 0x1000>;
+			interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>;
+			#mbox-cells = <3>;
+		};
+
 		dual_timer0: timer@fff14000 {
 			compatible = "arm,sp804", "arm,primecell";
 			reg = <0x0 0xfff14000 0x0 0x1000>;
-- 
1.9.1

Re: [PATCH v3 1/3] dt-bindings: mailbox: Introduce Hi3660 controller binding

[Rob Herring]

On Fri, Nov 17, 2017 at 05:24:30PM +0800, Xu YiPing wrote:
> From: Leo Yan <leo.yan@linaro.org>
> 
> Introduce a binding for the Hi3660 mailbox controller, the mailbox is
> used within application processor (AP), communication processor (CP),
> HIFI and MCU, etc.
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  .../bindings/mailbox/hisilicon,hi3660-mailbox.txt  | 51 ++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/hisilicon,hi3660-mailbox.txt

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 2/3] mailbox: Add support for Hi3660 mailbox

[Leo Yan]

On Fri, Nov 17, 2017 at 05:24:31PM +0800, Xu YiPing wrote:
> From: Kaihua Zhong <zhongkaihua@huawei.com>
> 
> Hi3660 mailbox controller is used to send message within multiple
> processors, MCU, HIFI, etc.  It supports 32 mailbox channels and every
> channel can only be used for single transferring direction.  Once the
> channel is enabled, it needs to specify the destination interrupt and
> acknowledge interrupt, these two interrupt vectors are used to create
> the connection between the mailbox and interrupt controllers.
> 
> The data transferring supports two modes, one is named as "automatic
> acknowledge" mode so after send message the kernel doesn't need to wait
> for acknowledge from remote and directly return; there have another mode
> is to rely on handling interrupt for acknowledge.
> 
> This commit is for initial version driver, which only supports
> "automatic acknowledge" mode to support CPU clock, which is the only
> one consumer to use mailbox and has been verified.  Later may enhance
> this driver for interrupt mode (e.g. for supporting HIFI).

Gentle ping ...

> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> Signed-off-by: Ruyi Wang <wangruyi@huawei.com>
> Signed-off-by: Kaihua Zhong <zhongkaihua@huawei.com>
> ---
>  drivers/mailbox/Kconfig          |   8 +
>  drivers/mailbox/Makefile         |   2 +
>  drivers/mailbox/hi3660-mailbox.c | 322 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 332 insertions(+)
>  create mode 100644 drivers/mailbox/hi3660-mailbox.c
> 
> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> index c5731e5..4b5d6e9 100644
> --- a/drivers/mailbox/Kconfig
> +++ b/drivers/mailbox/Kconfig
> @@ -108,6 +108,14 @@ config TI_MESSAGE_MANAGER
>  	  multiple processors within the SoC. Select this driver if your
>  	  platform has support for the hardware block.
>  
> +config HI3660_MBOX
> +	tristate "Hi3660 Mailbox"
> +	depends on ARCH_HISI && OF
> +	help
> +	  An implementation of the hi3660 mailbox. It is used to send message
> +	  between application processors and other processors/MCU/DSP. Select
> +	  Y here if you want to use Hi3660 mailbox controller.
> +
>  config HI6220_MBOX
>  	tristate "Hi6220 Mailbox"
>  	depends on ARCH_HISI
> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> index d54e412..7d1bd51 100644
> --- a/drivers/mailbox/Makefile
> +++ b/drivers/mailbox/Makefile
> @@ -26,6 +26,8 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o
>  
>  obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o
>  
> +obj-$(CONFIG_HI3660_MBOX)	+= hi3660-mailbox.o
> +
>  obj-$(CONFIG_HI6220_MBOX)	+= hi6220-mailbox.o
>  
>  obj-$(CONFIG_BCM_PDC_MBOX)	+= bcm-pdc-mailbox.o
> diff --git a/drivers/mailbox/hi3660-mailbox.c b/drivers/mailbox/hi3660-mailbox.c
> new file mode 100644
> index 0000000..ba80834
> --- /dev/null
> +++ b/drivers/mailbox/hi3660-mailbox.c
> @@ -0,0 +1,322 @@
> +/*
> + * Hisilicon's Hi3660 mailbox controller driver
> + *
> + * Copyright (c) 2017 Hisilicon Limited.
> + * Copyright (c) 2017 Linaro Limited.
> + *
> + * Author: Leo Yan <leo.yan@linaro.org>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/mailbox_controller.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include "mailbox.h"
> +
> +#define MBOX_CHAN_MAX			32
> +
> +#define MBOX_RX				(0x0)
> +#define MBOX_TX				(0x1)
> +
> +#define MBOX_BASE(mbox, ch)		((mbox)->base + ((ch) * 0x40))
> +#define MBOX_SRC_REG			(0x00)
> +#define MBOX_DST_REG			(0x04)
> +#define MBOX_DCLR_REG			(0x08)
> +#define MBOX_DSTAT_REG			(0x0c)
> +#define MBOX_MODE_REG			(0x10)
> +#define MBOX_IMASK_REG			(0x14)
> +#define MBOX_ICLR_REG			(0x18)
> +#define MBOX_SEND_REG			(0x1c)
> +#define MBOX_DATA_REG			(0x20)
> +
> +#define MBOX_IPC_LOCK_REG		(0xa00)
> +#define MBOX_IPC_UNLOCK			(0x1acce551)
> +
> +#define MBOX_AUTOMATIC_ACK		(1)
> +
> +#define MBOX_STATE_IDLE			BIT(4)
> +#define MBOX_STATE_ACK			BIT(7)
> +
> +#define MBOX_MSG_LEN			8
> +
> +/**
> + * Hi3660 mailbox channel device data
> + *
> + * A channel can be used for TX or RX, it can trigger remote
> + * processor interrupt to notify remote processor and can receive
> + * interrupt if has incoming message.
> + *
> + * @dst_irq:	Interrupt vector for remote processor
> + * @ack_irq:	Interrupt vector for local processor
> + */
> +struct hi3660_mbox_dev {
> +	unsigned int dst_irq;
> +	unsigned int ack_irq;
> +};
> +
> +/**
> + * Hi3660 mailbox controller data
> + *
> + * Mailbox controller includes 32 channels and can allocate
> + * channel for message transferring.
> + *
> + * @dev:	Device to which it is attached
> + * @base:	Base address of the register mapping region
> + * @chan:	Representation of channels in mailbox controller
> + * @mdev:	Representation of channel device data
> + * @controller:	Representation of a communication channel controller
> + */
> +struct hi3660_mbox {
> +	struct device *dev;
> +	void __iomem *base;
> +	struct mbox_chan chan[MBOX_CHAN_MAX];
> +	struct hi3660_mbox_dev mdev[MBOX_CHAN_MAX];
> +	struct mbox_controller controller;
> +};
> +
> +static inline struct hi3660_mbox *to_hi3660_mbox(struct mbox_controller *mbox)
> +{
> +	return container_of(mbox, struct hi3660_mbox, controller);
> +}
> +
> +static int hi3660_mbox_check_state(struct mbox_chan *chan)
> +{
> +	unsigned long ch = (unsigned long)chan->con_priv;
> +	struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> +	struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> +	void __iomem *base = MBOX_BASE(mbox, ch);
> +	unsigned long val;
> +	unsigned int state, ret;
> +
> +	/* Mailbox is idle so directly bail out */
> +	state = readl_relaxed(base + MBOX_MODE_REG);
> +	if (state & MBOX_STATE_IDLE)
> +		return 0;
> +
> +	/* Wait for acknowledge from remote */
> +	ret = readx_poll_timeout_atomic(readl_relaxed, base + MBOX_MODE_REG,
> +			val, (val & MBOX_STATE_ACK), 1000, 300000);
> +	if (ret) {
> +		dev_err(mbox->dev, "%s: timeout for receiving ack\n", __func__);
> +		return ret;
> +	}
> +
> +	/* Ensure channel is released */
> +	writel_relaxed(0xffffffff, base + MBOX_IMASK_REG);
> +	writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SRC_REG);
> +
> +	return 0;
> +}
> +
> +static int hi3660_mbox_unlock(struct mbox_chan *chan)
> +{
> +	struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> +	unsigned int val, retry = 3;
> +
> +	do {
> +		writel_relaxed(MBOX_IPC_UNLOCK, mbox->base + MBOX_IPC_LOCK_REG);
> +
> +		val = readl_relaxed(mbox->base + MBOX_IPC_LOCK_REG);
> +		if (!val)
> +			break;
> +
> +		udelay(10);
> +	} while (retry--);
> +
> +	return (!val) ? 0 : -ETIMEDOUT;
> +}
> +
> +static int hi3660_mbox_acquire_channel(struct mbox_chan *chan)
> +{
> +	unsigned long ch = (unsigned long)chan->con_priv;
> +	struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> +	struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> +	void __iomem *base = MBOX_BASE(mbox, ch);
> +	unsigned int val, retry;
> +
> +	for (retry = 10; retry; retry--) {
> +		/* Check if channel has been acquired */
> +		if (readl_relaxed(base + MBOX_MODE_REG) & MBOX_STATE_IDLE) {
> +			writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SRC_REG);
> +			val = readl_relaxed(base + MBOX_SRC_REG);
> +			if (val & BIT(mdev->ack_irq))
> +				break;
> +		}
> +	}
> +
> +	return retry ? 0 : -ETIMEDOUT;
> +}
> +
> +static int hi3660_mbox_send(struct mbox_chan *chan, u32 *msg)
> +{
> +	unsigned long ch = (unsigned long)chan->con_priv;
> +	struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> +	struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> +	void __iomem *base = MBOX_BASE(mbox, ch);
> +	unsigned int i;
> +
> +	/* Clear mask for destination interrupt */
> +	writel_relaxed(~BIT(mdev->dst_irq), base + MBOX_IMASK_REG);
> +
> +	/* Config destination for interrupt vector */
> +	writel_relaxed(BIT(mdev->dst_irq), base + MBOX_DST_REG);
> +
> +	/* Automatic acknowledge mode */
> +	writel_relaxed(MBOX_AUTOMATIC_ACK, base + MBOX_MODE_REG);
> +
> +	/* Fill message data */
> +	for (i = 0; i < MBOX_MSG_LEN; i++)
> +		writel_relaxed(msg[i], base + MBOX_DATA_REG + i * 4);
> +
> +	/* Trigger data transferring */
> +	writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SEND_REG);
> +	return 0;
> +}
> +
> +static int hi3660_mbox_send_data(struct mbox_chan *chan, void *msg)
> +{
> +	struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> +	int err;
> +
> +	err = hi3660_mbox_check_state(chan);
> +	if (err) {
> +		dev_err(mbox->dev, "checking state failed\n");
> +		return err;
> +	}
> +
> +	err = hi3660_mbox_unlock(chan);
> +	if (err) {
> +		dev_err(mbox->dev, "unlocking mailbox failed\n");
> +		return err;
> +	}
> +
> +	err = hi3660_mbox_acquire_channel(chan);
> +	if (err) {
> +		dev_err(mbox->dev, "acquiring channel failed\n");
> +		return err;
> +	}
> +
> +	return hi3660_mbox_send(chan, msg);
> +}
> +
> +static struct mbox_chan_ops hi3660_mbox_ops = {
> +	.send_data    = hi3660_mbox_send_data,
> +};
> +
> +static struct mbox_chan *hi3660_mbox_xlate(struct mbox_controller *controller,
> +					   const struct of_phandle_args *spec)
> +{
> +	struct hi3660_mbox *mbox = to_hi3660_mbox(controller);
> +	struct hi3660_mbox_dev *mdev;
> +	unsigned int ch = spec->args[0];
> +
> +	if (ch >= MBOX_CHAN_MAX) {
> +		dev_err(mbox->dev, "Invalid channel idx %d\n", ch);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	mdev = &mbox->mdev[ch];
> +	mdev->dst_irq = spec->args[1];
> +	mdev->ack_irq = spec->args[2];
> +
> +	return &mbox->chan[ch];
> +}
> +
> +static const struct of_device_id hi3660_mbox_of_match[] = {
> +	{ .compatible = "hisilicon,hi3660-mbox", },
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(of, hi3660_mbox_of_match);
> +
> +static int hi3660_mbox_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct hi3660_mbox *mbox;
> +	struct mbox_chan *chan;
> +	struct resource *res;
> +	unsigned long ch;
> +	int err;
> +
> +	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> +	if (!mbox)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	mbox->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(mbox->base))
> +		return PTR_ERR(mbox->base);
> +
> +	mbox->dev = dev;
> +	mbox->controller.dev = dev;
> +	mbox->controller.chans = mbox->chan;
> +	mbox->controller.num_chans = MBOX_CHAN_MAX;
> +	mbox->controller.ops = &hi3660_mbox_ops;
> +	mbox->controller.of_xlate = hi3660_mbox_xlate;
> +
> +	/* Initialize mailbox channel data */
> +	chan = mbox->chan;
> +	for (ch = 0; ch < MBOX_CHAN_MAX; ch++)
> +		chan[ch].con_priv = (void *)ch;
> +
> +	err = mbox_controller_register(&mbox->controller);
> +	if (err) {
> +		dev_err(dev, "Failed to register mailbox %d\n", err);
> +		return err;
> +	}
> +
> +	platform_set_drvdata(pdev, mbox);
> +	dev_info(dev, "Mailbox enabled\n");
> +	return 0;
> +}
> +
> +static int hi3660_mbox_remove(struct platform_device *pdev)
> +{
> +	struct hi3660_mbox *mbox = platform_get_drvdata(pdev);
> +
> +	mbox_controller_unregister(&mbox->controller);
> +	return 0;
> +}
> +
> +static struct platform_driver hi3660_mbox_driver = {
> +	.probe  = hi3660_mbox_probe,
> +	.remove = hi3660_mbox_remove,
> +	.driver = {
> +		.name = "hi3660-mbox",
> +		.of_match_table = hi3660_mbox_of_match,
> +	},
> +};
> +
> +static int __init hi3660_mbox_init(void)
> +{
> +	return platform_driver_register(&hi3660_mbox_driver);
> +}
> +core_initcall(hi3660_mbox_init);
> +
> +static void __exit hi3660_mbox_exit(void)
> +{
> +	platform_driver_unregister(&hi3660_mbox_driver);
> +}
> +module_exit(hi3660_mbox_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Hisilicon Hi3660 Mailbox Controller");
> +MODULE_AUTHOR("Leo Yan <leo.yan@linaro.org>");
> -- 
> 1.9.1
> 

Re: [PATCH v3 2/3] mailbox: Add support for Hi3660 mailbox

[Jassi Brar]

On Fri, Nov 17, 2017 at 2:54 PM, Xu YiPing <xuyiping@hisilicon.com> wrote:
> From: Kaihua Zhong <zhongkaihua@huawei.com>
>
> Hi3660 mailbox controller is used to send message within multiple
> processors, MCU, HIFI, etc.  It supports 32 mailbox channels and every
> channel can only be used for single transferring direction.  Once the
> channel is enabled, it needs to specify the destination interrupt and
> acknowledge interrupt, these two interrupt vectors are used to create
> the connection between the mailbox and interrupt controllers.
>
> The data transferring supports two modes, one is named as "automatic
> acknowledge" mode so after send message the kernel doesn't need to wait
> for acknowledge from remote and directly return; there have another mode
> is to rely on handling interrupt for acknowledge.
>
> This commit is for initial version driver, which only supports
> "automatic acknowledge" mode to support CPU clock, which is the only
> one consumer to use mailbox and has been verified.  Later may enhance
> this driver for interrupt mode (e.g. for supporting HIFI).
>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> Signed-off-by: Ruyi Wang <wangruyi@huawei.com>
> Signed-off-by: Kaihua Zhong <zhongkaihua@huawei.com>
> ---
>  drivers/mailbox/Kconfig          |   8 +
>  drivers/mailbox/Makefile         |   2 +
>  drivers/mailbox/hi3660-mailbox.c | 322 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 332 insertions(+)
>  create mode 100644 drivers/mailbox/hi3660-mailbox.c
>
> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> index c5731e5..4b5d6e9 100644
> --- a/drivers/mailbox/Kconfig
> +++ b/drivers/mailbox/Kconfig
> @@ -108,6 +108,14 @@ config TI_MESSAGE_MANAGER
>           multiple processors within the SoC. Select this driver if your
>           platform has support for the hardware block.
>
> +config HI3660_MBOX
> +       tristate "Hi3660 Mailbox"
> +       depends on ARCH_HISI && OF
> +       help
> +         An implementation of the hi3660 mailbox. It is used to send message
> +         between application processors and other processors/MCU/DSP. Select
> +         Y here if you want to use Hi3660 mailbox controller.
> +
>  config HI6220_MBOX
>         tristate "Hi6220 Mailbox"
>         depends on ARCH_HISI
> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> index d54e412..7d1bd51 100644
> --- a/drivers/mailbox/Makefile
> +++ b/drivers/mailbox/Makefile
> @@ -26,6 +26,8 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o
>
>  obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o
>
> +obj-$(CONFIG_HI3660_MBOX)      += hi3660-mailbox.o
> +
>  obj-$(CONFIG_HI6220_MBOX)      += hi6220-mailbox.o
>
>  obj-$(CONFIG_BCM_PDC_MBOX)     += bcm-pdc-mailbox.o
> diff --git a/drivers/mailbox/hi3660-mailbox.c b/drivers/mailbox/hi3660-mailbox.c
> new file mode 100644
> index 0000000..ba80834
> --- /dev/null
> +++ b/drivers/mailbox/hi3660-mailbox.c
> @@ -0,0 +1,322 @@
> +/*
> + * Hisilicon's Hi3660 mailbox controller driver
> + *
> + * Copyright (c) 2017 Hisilicon Limited.
> + * Copyright (c) 2017 Linaro Limited.
> + *
> + * Author: Leo Yan <leo.yan@linaro.org>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
>
We now use SPDX licence

> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/mailbox_controller.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include "mailbox.h"
> +
> +#define MBOX_CHAN_MAX                  32
> +
> +#define MBOX_RX                                (0x0)
> +#define MBOX_TX                                (0x1)
> +
> +#define MBOX_BASE(mbox, ch)            ((mbox)->base + ((ch) * 0x40))
> +#define MBOX_SRC_REG                   (0x00)
> +#define MBOX_DST_REG                   (0x04)
> +#define MBOX_DCLR_REG                  (0x08)
> +#define MBOX_DSTAT_REG                 (0x0c)
> +#define MBOX_MODE_REG                  (0x10)
> +#define MBOX_IMASK_REG                 (0x14)
> +#define MBOX_ICLR_REG                  (0x18)
> +#define MBOX_SEND_REG                  (0x1c)
> +#define MBOX_DATA_REG                  (0x20)
> +
> +#define MBOX_IPC_LOCK_REG              (0xa00)
> +#define MBOX_IPC_UNLOCK                        (0x1acce551)
> +
> +#define MBOX_AUTOMATIC_ACK             (1)
> +
Please, no brackets around numbers.

> +#define MBOX_STATE_IDLE                        BIT(4)
> +#define MBOX_STATE_ACK                 BIT(7)
> +
> +#define MBOX_MSG_LEN                   8
> +
> +/**
> + * Hi3660 mailbox channel device data
> + *
> + * A channel can be used for TX or RX, it can trigger remote
> + * processor interrupt to notify remote processor and can receive
> + * interrupt if has incoming message.
> + *
> + * @dst_irq:   Interrupt vector for remote processor
> + * @ack_irq:   Interrupt vector for local processor
> + */
> +struct hi3660_mbox_dev {
>
Better than dev, maybe hi3660_chan_info ?

> +       unsigned int dst_irq;
> +       unsigned int ack_irq;
> +};
> +
> +/**
> + * Hi3660 mailbox controller data
> + *
> + * Mailbox controller includes 32 channels and can allocate
> + * channel for message transferring.
> + *
> + * @dev:       Device to which it is attached
> + * @base:      Base address of the register mapping region
> + * @chan:      Representation of channels in mailbox controller
> + * @mdev:      Representation of channel device data
> + * @controller:        Representation of a communication channel controller
> + */
> +struct hi3660_mbox {
> +       struct device *dev;
> +       void __iomem *base;
> +       struct mbox_chan chan[MBOX_CHAN_MAX];
> +       struct hi3660_mbox_dev mdev[MBOX_CHAN_MAX];
>
Maybe mchan, instead of mdev.

> +       struct mbox_controller controller;
> +};
> +
> +static inline struct hi3660_mbox *to_hi3660_mbox(struct mbox_controller *mbox)
> +{
> +       return container_of(mbox, struct hi3660_mbox, controller);
> +}
> +
> +static int hi3660_mbox_check_state(struct mbox_chan *chan)
> +{
> +       unsigned long ch = (unsigned long)chan->con_priv;
> +       struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> +       struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> +       void __iomem *base = MBOX_BASE(mbox, ch);
> +       unsigned long val;
> +       unsigned int state, ret;
> +
> +       /* Mailbox is idle so directly bail out */
> +       state = readl_relaxed(base + MBOX_MODE_REG);
> +       if (state & MBOX_STATE_IDLE)
> +               return 0;
> +
> +       /* Wait for acknowledge from remote */
> +       ret = readx_poll_timeout_atomic(readl_relaxed, base + MBOX_MODE_REG,
> +                       val, (val & MBOX_STATE_ACK), 1000, 300000);
> +       if (ret) {
> +               dev_err(mbox->dev, "%s: timeout for receiving ack\n", __func__);
> +               return ret;
> +       }
> +
> +       /* Ensure channel is released */
> +       writel_relaxed(0xffffffff, base + MBOX_IMASK_REG);
> +       writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SRC_REG);
> +
> +       return 0;
> +}
> +
> +static int hi3660_mbox_unlock(struct mbox_chan *chan)
> +{
> +       struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> +       unsigned int val, retry = 3;
> +
> +       do {
> +               writel_relaxed(MBOX_IPC_UNLOCK, mbox->base + MBOX_IPC_LOCK_REG);
> +
> +               val = readl_relaxed(mbox->base + MBOX_IPC_LOCK_REG);
> +               if (!val)
> +                       break;
> +
> +               udelay(10);
> +       } while (retry--);
> +
> +       return (!val) ? 0 : -ETIMEDOUT;
> +}
> +
> +static int hi3660_mbox_acquire_channel(struct mbox_chan *chan)
> +{
> +       unsigned long ch = (unsigned long)chan->con_priv;
> +       struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> +       struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> +       void __iomem *base = MBOX_BASE(mbox, ch);
> +       unsigned int val, retry;
> +
> +       for (retry = 10; retry; retry--) {
> +               /* Check if channel has been acquired */
> +               if (readl_relaxed(base + MBOX_MODE_REG) & MBOX_STATE_IDLE) {
> +                       writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SRC_REG);
> +                       val = readl_relaxed(base + MBOX_SRC_REG);
> +                       if (val & BIT(mdev->ack_irq))
> +                               break;
> +               }
> +       }
> +
> +       return retry ? 0 : -ETIMEDOUT;
> +}
> +
> +static int hi3660_mbox_send(struct mbox_chan *chan, u32 *msg)
> +{
> +       unsigned long ch = (unsigned long)chan->con_priv;
> +       struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> +       struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> +       void __iomem *base = MBOX_BASE(mbox, ch);
> +       unsigned int i;
> +
> +       /* Clear mask for destination interrupt */
> +       writel_relaxed(~BIT(mdev->dst_irq), base + MBOX_IMASK_REG);
> +
> +       /* Config destination for interrupt vector */
> +       writel_relaxed(BIT(mdev->dst_irq), base + MBOX_DST_REG);
> +
> +       /* Automatic acknowledge mode */
> +       writel_relaxed(MBOX_AUTOMATIC_ACK, base + MBOX_MODE_REG);
> +
> +       /* Fill message data */
> +       for (i = 0; i < MBOX_MSG_LEN; i++)
> +               writel_relaxed(msg[i], base + MBOX_DATA_REG + i * 4);
> +
> +       /* Trigger data transferring */
> +       writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SEND_REG);
> +       return 0;
> +}
> +
> +static int hi3660_mbox_send_data(struct mbox_chan *chan, void *msg)
> +{
> +       struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> +       int err;
> +
> +       err = hi3660_mbox_check_state(chan);
> +       if (err) {
> +               dev_err(mbox->dev, "checking state failed\n");
> +               return err;
> +       }
> +
> +       err = hi3660_mbox_unlock(chan);
> +       if (err) {
> +               dev_err(mbox->dev, "unlocking mailbox failed\n");
> +               return err;
> +       }
> +
> +       err = hi3660_mbox_acquire_channel(chan);
> +       if (err) {
> +               dev_err(mbox->dev, "acquiring channel failed\n");
> +               return err;
> +       }
> +
Please remember .send_data() is called with irqs disabled spinlock
All these polling functions hurt. Please move the above three checks
in last_tx_done()

Thanks

Re: [PATCH v3 2/3] mailbox: Add support for Hi3660 mailbox

[Leo Yan]

On Tue, Dec 05, 2017 at 08:58:05AM +0530, Jassi Brar wrote:
> On Fri, Nov 17, 2017 at 2:54 PM, Xu YiPing <xuyiping@hisilicon.com> wrote:
> > From: Kaihua Zhong <zhongkaihua@huawei.com>
> >
> > Hi3660 mailbox controller is used to send message within multiple
> > processors, MCU, HIFI, etc.  It supports 32 mailbox channels and every
> > channel can only be used for single transferring direction.  Once the
> > channel is enabled, it needs to specify the destination interrupt and
> > acknowledge interrupt, these two interrupt vectors are used to create
> > the connection between the mailbox and interrupt controllers.
> >
> > The data transferring supports two modes, one is named as "automatic
> > acknowledge" mode so after send message the kernel doesn't need to wait
> > for acknowledge from remote and directly return; there have another mode
> > is to rely on handling interrupt for acknowledge.
> >
> > This commit is for initial version driver, which only supports
> > "automatic acknowledge" mode to support CPU clock, which is the only
> > one consumer to use mailbox and has been verified.  Later may enhance
> > this driver for interrupt mode (e.g. for supporting HIFI).
> >
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > Signed-off-by: Ruyi Wang <wangruyi@huawei.com>
> > Signed-off-by: Kaihua Zhong <zhongkaihua@huawei.com>
> > ---
> >  drivers/mailbox/Kconfig          |   8 +
> >  drivers/mailbox/Makefile         |   2 +
> >  drivers/mailbox/hi3660-mailbox.c | 322 +++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 332 insertions(+)
> >  create mode 100644 drivers/mailbox/hi3660-mailbox.c
> >
> > diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> > index c5731e5..4b5d6e9 100644
> > --- a/drivers/mailbox/Kconfig
> > +++ b/drivers/mailbox/Kconfig
> > @@ -108,6 +108,14 @@ config TI_MESSAGE_MANAGER
> >           multiple processors within the SoC. Select this driver if your
> >           platform has support for the hardware block.
> >
> > +config HI3660_MBOX
> > +       tristate "Hi3660 Mailbox"
> > +       depends on ARCH_HISI && OF
> > +       help
> > +         An implementation of the hi3660 mailbox. It is used to send message
> > +         between application processors and other processors/MCU/DSP. Select
> > +         Y here if you want to use Hi3660 mailbox controller.
> > +
> >  config HI6220_MBOX
> >         tristate "Hi6220 Mailbox"
> >         depends on ARCH_HISI
> > diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> > index d54e412..7d1bd51 100644
> > --- a/drivers/mailbox/Makefile
> > +++ b/drivers/mailbox/Makefile
> > @@ -26,6 +26,8 @@ obj-$(CONFIG_TI_MESSAGE_MANAGER) += ti-msgmgr.o
> >
> >  obj-$(CONFIG_XGENE_SLIMPRO_MBOX) += mailbox-xgene-slimpro.o
> >
> > +obj-$(CONFIG_HI3660_MBOX)      += hi3660-mailbox.o
> > +
> >  obj-$(CONFIG_HI6220_MBOX)      += hi6220-mailbox.o
> >
> >  obj-$(CONFIG_BCM_PDC_MBOX)     += bcm-pdc-mailbox.o
> > diff --git a/drivers/mailbox/hi3660-mailbox.c b/drivers/mailbox/hi3660-mailbox.c
> > new file mode 100644
> > index 0000000..ba80834
> > --- /dev/null
> > +++ b/drivers/mailbox/hi3660-mailbox.c
> > @@ -0,0 +1,322 @@
> > +/*
> > + * Hisilicon's Hi3660 mailbox controller driver
> > + *
> > + * Copyright (c) 2017 Hisilicon Limited.
> > + * Copyright (c) 2017 Linaro Limited.
> > + *
> > + * Author: Leo Yan <leo.yan@linaro.org>
> > + *
> > + * This program is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation, version 2 of the License.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> >
> We now use SPDX licence

Thanks for quick response, Jassi.

Will change to SPDX licence fashion.

> > + */
> > +
> > +#include <linux/bitops.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/mailbox_controller.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +
> > +#include "mailbox.h"
> > +
> > +#define MBOX_CHAN_MAX                  32
> > +
> > +#define MBOX_RX                                (0x0)
> > +#define MBOX_TX                                (0x1)
> > +
> > +#define MBOX_BASE(mbox, ch)            ((mbox)->base + ((ch) * 0x40))
> > +#define MBOX_SRC_REG                   (0x00)
> > +#define MBOX_DST_REG                   (0x04)
> > +#define MBOX_DCLR_REG                  (0x08)
> > +#define MBOX_DSTAT_REG                 (0x0c)
> > +#define MBOX_MODE_REG                  (0x10)
> > +#define MBOX_IMASK_REG                 (0x14)
> > +#define MBOX_ICLR_REG                  (0x18)
> > +#define MBOX_SEND_REG                  (0x1c)
> > +#define MBOX_DATA_REG                  (0x20)
> > +
> > +#define MBOX_IPC_LOCK_REG              (0xa00)
> > +#define MBOX_IPC_UNLOCK                        (0x1acce551)
> > +
> > +#define MBOX_AUTOMATIC_ACK             (1)
> > +
> Please, no brackets around numbers.

Sure, will remove brackets.

> > +#define MBOX_STATE_IDLE                        BIT(4)
> > +#define MBOX_STATE_ACK                 BIT(7)
> > +
> > +#define MBOX_MSG_LEN                   8
> > +
> > +/**
> > + * Hi3660 mailbox channel device data
> > + *
> > + * A channel can be used for TX or RX, it can trigger remote
> > + * processor interrupt to notify remote processor and can receive
> > + * interrupt if has incoming message.
> > + *
> > + * @dst_irq:   Interrupt vector for remote processor
> > + * @ack_irq:   Interrupt vector for local processor
> > + */
> > +struct hi3660_mbox_dev {
> >
> Better than dev, maybe hi3660_chan_info ?
> 
> > +       unsigned int dst_irq;
> > +       unsigned int ack_irq;
> > +};
> > +
> > +/**
> > + * Hi3660 mailbox controller data
> > + *
> > + * Mailbox controller includes 32 channels and can allocate
> > + * channel for message transferring.
> > + *
> > + * @dev:       Device to which it is attached
> > + * @base:      Base address of the register mapping region
> > + * @chan:      Representation of channels in mailbox controller
> > + * @mdev:      Representation of channel device data
> > + * @controller:        Representation of a communication channel controller
> > + */
> > +struct hi3660_mbox {
> > +       struct device *dev;
> > +       void __iomem *base;
> > +       struct mbox_chan chan[MBOX_CHAN_MAX];
> > +       struct hi3660_mbox_dev mdev[MBOX_CHAN_MAX];
> >
> Maybe mchan, instead of mdev.

Will refactor structure name.

> > +       struct mbox_controller controller;
> > +};
> > +
> > +static inline struct hi3660_mbox *to_hi3660_mbox(struct mbox_controller *mbox)
> > +{
> > +       return container_of(mbox, struct hi3660_mbox, controller);
> > +}
> > +
> > +static int hi3660_mbox_check_state(struct mbox_chan *chan)
> > +{
> > +       unsigned long ch = (unsigned long)chan->con_priv;
> > +       struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> > +       struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> > +       void __iomem *base = MBOX_BASE(mbox, ch);
> > +       unsigned long val;
> > +       unsigned int state, ret;
> > +
> > +       /* Mailbox is idle so directly bail out */
> > +       state = readl_relaxed(base + MBOX_MODE_REG);
> > +       if (state & MBOX_STATE_IDLE)
> > +               return 0;
> > +
> > +       /* Wait for acknowledge from remote */
> > +       ret = readx_poll_timeout_atomic(readl_relaxed, base + MBOX_MODE_REG,
> > +                       val, (val & MBOX_STATE_ACK), 1000, 300000);
> > +       if (ret) {
> > +               dev_err(mbox->dev, "%s: timeout for receiving ack\n", __func__);
> > +               return ret;
> > +       }
> > +
> > +       /* Ensure channel is released */
> > +       writel_relaxed(0xffffffff, base + MBOX_IMASK_REG);
> > +       writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SRC_REG);
> > +
> > +       return 0;
> > +}
> > +
> > +static int hi3660_mbox_unlock(struct mbox_chan *chan)
> > +{
> > +       struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> > +       unsigned int val, retry = 3;
> > +
> > +       do {
> > +               writel_relaxed(MBOX_IPC_UNLOCK, mbox->base + MBOX_IPC_LOCK_REG);
> > +
> > +               val = readl_relaxed(mbox->base + MBOX_IPC_LOCK_REG);
> > +               if (!val)
> > +                       break;
> > +
> > +               udelay(10);
> > +       } while (retry--);
> > +
> > +       return (!val) ? 0 : -ETIMEDOUT;
> > +}
> > +
> > +static int hi3660_mbox_acquire_channel(struct mbox_chan *chan)
> > +{
> > +       unsigned long ch = (unsigned long)chan->con_priv;
> > +       struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> > +       struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> > +       void __iomem *base = MBOX_BASE(mbox, ch);
> > +       unsigned int val, retry;
> > +
> > +       for (retry = 10; retry; retry--) {
> > +               /* Check if channel has been acquired */
> > +               if (readl_relaxed(base + MBOX_MODE_REG) & MBOX_STATE_IDLE) {
> > +                       writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SRC_REG);
> > +                       val = readl_relaxed(base + MBOX_SRC_REG);
> > +                       if (val & BIT(mdev->ack_irq))
> > +                               break;
> > +               }
> > +       }
> > +
> > +       return retry ? 0 : -ETIMEDOUT;
> > +}
> > +
> > +static int hi3660_mbox_send(struct mbox_chan *chan, u32 *msg)
> > +{
> > +       unsigned long ch = (unsigned long)chan->con_priv;
> > +       struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> > +       struct hi3660_mbox_dev *mdev = &mbox->mdev[ch];
> > +       void __iomem *base = MBOX_BASE(mbox, ch);
> > +       unsigned int i;
> > +
> > +       /* Clear mask for destination interrupt */
> > +       writel_relaxed(~BIT(mdev->dst_irq), base + MBOX_IMASK_REG);
> > +
> > +       /* Config destination for interrupt vector */
> > +       writel_relaxed(BIT(mdev->dst_irq), base + MBOX_DST_REG);
> > +
> > +       /* Automatic acknowledge mode */
> > +       writel_relaxed(MBOX_AUTOMATIC_ACK, base + MBOX_MODE_REG);
> > +
> > +       /* Fill message data */
> > +       for (i = 0; i < MBOX_MSG_LEN; i++)
> > +               writel_relaxed(msg[i], base + MBOX_DATA_REG + i * 4);
> > +
> > +       /* Trigger data transferring */
> > +       writel_relaxed(BIT(mdev->ack_irq), base + MBOX_SEND_REG);
> > +       return 0;
> > +}
> > +
> > +static int hi3660_mbox_send_data(struct mbox_chan *chan, void *msg)
> > +{
> > +       struct hi3660_mbox *mbox = to_hi3660_mbox(chan->mbox);
> > +       int err;
> > +
> > +       err = hi3660_mbox_check_state(chan);
> > +       if (err) {
> > +               dev_err(mbox->dev, "checking state failed\n");
> > +               return err;
> > +       }
> > +
> > +       err = hi3660_mbox_unlock(chan);
> > +       if (err) {
> > +               dev_err(mbox->dev, "unlocking mailbox failed\n");
> > +               return err;
> > +       }
> > +
> > +       err = hi3660_mbox_acquire_channel(chan);
> > +       if (err) {
> > +               dev_err(mbox->dev, "acquiring channel failed\n");
> > +               return err;
> > +       }
> > +
> Please remember .send_data() is called with irqs disabled spinlock
> All these polling functions hurt. Please move the above three checks
> in last_tx_done()

I have tried to move these three checks in last_tx_done(), since these
three functions tightly couple with mailbox state machine, so I failed
to move them to last_tx_done().

Anyway, I will try to move them to last_tx_done() for next version
patch.

Thanks,
Leo Yan