All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/4] mailbox: hisilicon: add Hi6220 mailbox driver
@ 2016-02-01 13:34 ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2016-02-01 13:34 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Catalin Marinas, Will Deacon, Jassi Brar, Leo Yan, Arnd Bergmann,
	Wei Xu, Tyler Baker, Sudeep Holla, Chen Feng, Bintian Wang,
	devicetree, linux-kernel, linux-arm-kernel, Haojian Zhuang

Hi6220 mailbox supports up to 32 channels. Each channel is unidirectional
with a maximum message size of 8 words. I/O is performed using register
access (there is no DMA) and the cell raises an interrupt when messages
are received.

This patch series is to implement Hi6220 mailbox driver. It registers
two channels into framework for communication with MCU, one is tx channel
and another is rx channel. Now mailbox driver is used to send message to
MCU to control dynamic voltage and frequency scaling for CPU, GPU and DDR.

Changes from v4:
* According to Jassi's suggestion, using DT binding to register channels
* Change to use operating-points-v2 to register operating points

Changes from v3:
* The patch series for enabling idle state for Hi6220 has reserved memory
  regions, so this series will not include it anymore
* Refined mailbox driver according to Jassi's suggestion;
  Removed kfifo from mailbox driver;
  Removed spinlock for ipc registers accessing, due every channel has its
  own dedicated bit in ipc register and readl/writel will introduce memory
  barrier, so don't need spinlock to protect ipc registers accessing
* After mailbox driver is ready, can use patch 4 to enable CPU's OPPs and
  stub clock driver; finally can enable CPUFreq driver for CPU frequency
  scaling

Changes from v2:
* Get rid of unused memory regions from memory node in DT, and don't use
  reserved-memory node according to Mark and Leif's suggestion; Haojian also
  has updated UEFI for efi memory info

Changes from v1:
* Correct lock usage for SMP scenario

Changes from RFC:
* According to Jassi's review, totally remove the abstract common driver
  layer and only commit driver dedicated for Hi6220
* According to Paul Bolle's review, fix typo issue for Kconfig and remove
  unnecessary dependency with OF and fix minor for mailbox driver
* Refine a little for dts nodes


Leo Yan (4):
  dt-bindings: mailbox: Document Hi6220 mailbox driver
  mailbox: Hi6220: add mailbox driver
  arm64: dts: add mailbox node for Hi6220
  arm64: dts: add Hi6220's stub clock node

 .../bindings/mailbox/hisilicon,hi6220-mailbox.txt  |  90 +++++
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi          |  67 ++++
 drivers/mailbox/Kconfig                            |   8 +
 drivers/mailbox/Makefile                           |   2 +
 drivers/mailbox/hi6220-mailbox.c                   | 407 +++++++++++++++++++++
 5 files changed, 574 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
 create mode 100644 drivers/mailbox/hi6220-mailbox.c

-- 
1.9.1

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v5 0/4] mailbox: hisilicon: add Hi6220 mailbox driver
@ 2016-02-01 13:34 ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2016-02-01 13:34 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Catalin Marinas, Will Deacon, Jassi Brar, Leo Yan, Arnd Bergmann,
	Wei Xu, Tyler Baker, Sudeep Holla, Chen Feng, Bintian Wang,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Haojian Zhuang

Hi6220 mailbox supports up to 32 channels. Each channel is unidirectional
with a maximum message size of 8 words. I/O is performed using register
access (there is no DMA) and the cell raises an interrupt when messages
are received.

This patch series is to implement Hi6220 mailbox driver. It registers
two channels into framework for communication with MCU, one is tx channel
and another is rx channel. Now mailbox driver is used to send message to
MCU to control dynamic voltage and frequency scaling for CPU, GPU and DDR.

Changes from v4:
* According to Jassi's suggestion, using DT binding to register channels
* Change to use operating-points-v2 to register operating points

Changes from v3:
* The patch series for enabling idle state for Hi6220 has reserved memory
  regions, so this series will not include it anymore
* Refined mailbox driver according to Jassi's suggestion;
  Removed kfifo from mailbox driver;
  Removed spinlock for ipc registers accessing, due every channel has its
  own dedicated bit in ipc register and readl/writel will introduce memory
  barrier, so don't need spinlock to protect ipc registers accessing
* After mailbox driver is ready, can use patch 4 to enable CPU's OPPs and
  stub clock driver; finally can enable CPUFreq driver for CPU frequency
  scaling

Changes from v2:
* Get rid of unused memory regions from memory node in DT, and don't use
  reserved-memory node according to Mark and Leif's suggestion; Haojian also
  has updated UEFI for efi memory info

Changes from v1:
* Correct lock usage for SMP scenario

Changes from RFC:
* According to Jassi's review, totally remove the abstract common driver
  layer and only commit driver dedicated for Hi6220
* According to Paul Bolle's review, fix typo issue for Kconfig and remove
  unnecessary dependency with OF and fix minor for mailbox driver
* Refine a little for dts nodes


Leo Yan (4):
  dt-bindings: mailbox: Document Hi6220 mailbox driver
  mailbox: Hi6220: add mailbox driver
  arm64: dts: add mailbox node for Hi6220
  arm64: dts: add Hi6220's stub clock node

 .../bindings/mailbox/hisilicon,hi6220-mailbox.txt  |  90 +++++
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi          |  67 ++++
 drivers/mailbox/Kconfig                            |   8 +
 drivers/mailbox/Makefile                           |   2 +
 drivers/mailbox/hi6220-mailbox.c                   | 407 +++++++++++++++++++++
 5 files changed, 574 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
 create mode 100644 drivers/mailbox/hi6220-mailbox.c

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v5 0/4] mailbox: hisilicon: add Hi6220 mailbox driver
@ 2016-02-01 13:34 ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2016-02-01 13:34 UTC (permalink / raw)
  To: linux-arm-kernel

Hi6220 mailbox supports up to 32 channels. Each channel is unidirectional
with a maximum message size of 8 words. I/O is performed using register
access (there is no DMA) and the cell raises an interrupt when messages
are received.

This patch series is to implement Hi6220 mailbox driver. It registers
two channels into framework for communication with MCU, one is tx channel
and another is rx channel. Now mailbox driver is used to send message to
MCU to control dynamic voltage and frequency scaling for CPU, GPU and DDR.

Changes from v4:
* According to Jassi's suggestion, using DT binding to register channels
* Change to use operating-points-v2 to register operating points

Changes from v3:
* The patch series for enabling idle state for Hi6220 has reserved memory
  regions, so this series will not include it anymore
* Refined mailbox driver according to Jassi's suggestion;
  Removed kfifo from mailbox driver;
  Removed spinlock for ipc registers accessing, due every channel has its
  own dedicated bit in ipc register and readl/writel will introduce memory
  barrier, so don't need spinlock to protect ipc registers accessing
* After mailbox driver is ready, can use patch 4 to enable CPU's OPPs and
  stub clock driver; finally can enable CPUFreq driver for CPU frequency
  scaling

Changes from v2:
* Get rid of unused memory regions from memory node in DT, and don't use
  reserved-memory node according to Mark and Leif's suggestion; Haojian also
  has updated UEFI for efi memory info

Changes from v1:
* Correct lock usage for SMP scenario

Changes from RFC:
* According to Jassi's review, totally remove the abstract common driver
  layer and only commit driver dedicated for Hi6220
* According to Paul Bolle's review, fix typo issue for Kconfig and remove
  unnecessary dependency with OF and fix minor for mailbox driver
* Refine a little for dts nodes


Leo Yan (4):
  dt-bindings: mailbox: Document Hi6220 mailbox driver
  mailbox: Hi6220: add mailbox driver
  arm64: dts: add mailbox node for Hi6220
  arm64: dts: add Hi6220's stub clock node

 .../bindings/mailbox/hisilicon,hi6220-mailbox.txt  |  90 +++++
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi          |  67 ++++
 drivers/mailbox/Kconfig                            |   8 +
 drivers/mailbox/Makefile                           |   2 +
 drivers/mailbox/hi6220-mailbox.c                   | 407 +++++++++++++++++++++
 5 files changed, 574 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
 create mode 100644 drivers/mailbox/hi6220-mailbox.c

-- 
1.9.1

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v5 1/4] dt-bindings: mailbox: Document Hi6220 mailbox driver
  2016-02-01 13:34 ` Leo Yan
@ 2016-02-01 13:34   ` Leo Yan
  -1 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2016-02-01 13:34 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Catalin Marinas, Will Deacon, Jassi Brar, Leo Yan, Arnd Bergmann,
	Wei Xu, Tyler Baker, Sudeep Holla, Chen Feng, Bintian Wang,
	devicetree, linux-kernel, linux-arm-kernel, Haojian Zhuang

Document DT binding for Hisilicon Hi6220 mailbox driver.

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

diff --git a/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
new file mode 100644
index 0000000..96e6acc
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
@@ -0,0 +1,90 @@
+Hisilicon Hi6220 Mailbox Driver
+===============================
+
+Hisilicon Hi6220 mailbox supports up to 32 channels. Each channel
+is unidirectional with a maximum message size of 8 words. I/O is
+performed using register access (there is no DMA) and the cell
+raises an interrupt when messages are received.
+
+Mailbox Device Node:
+====================
+
+Required properties:
+--------------------
+- compatible:		Shall be "hisilicon,hi6220-mbox"
+- reg:			Contains the mailbox register address range (base
+			address and length); the first item is for IPC
+			registers, the second item is shared buffer for
+			slots.
+- #mbox-cells		Common mailbox binding property to identify the number
+			of cells required for the mailbox specifier. Should be 1.
+- interrupts:		Contains the interrupt information for the mailbox
+			device. The format is dependent on which interrupt
+			controller the SoCs use.
+
+Optional Properties:
+--------------------
+- hi6220,mbox-tx-noirq: Flag to allow the client user of this mailbox driver
+			to send messages without triggering a TX completion
+			interrupt.
+
+Child Nodes:
+============
+A child node is used for representing the actual sub-mailbox device that is
+used for the communication between the host processor and a remote processor.
+Each child node should have a unique node name across all the different
+mailbox device nodes.
+
+Required properties:
+--------------------
+- hi6220,mbox-tx:	sub-mailbox descriptor property defining Tx channel
+- hi6220,mbox-rx:	sub-mailbox descriptor property defining Rx channel
+
+Sub-mailbox Descriptor Data
+---------------------------
+Each of the above hi6220,mbox-tx and hi6220,mbox-rx properties should have 3
+cells of data that represent the following:
+    Cell #1 (slot_id) - mailbox slot id used either for transmitting
+                        (hi6220,mbox-tx) or for receiving (hi6220,mbox-rx)
+    Cell #2 (dst_irq) - irq identifier index number which used by MCU.
+    Cell #3 (ack_irq) - irq identifier index number with generating a tx/rx
+                        interrupt to application processor, mailbox driver
+                        used this id to acknowledge interrupt.
+
+Example:
+--------
+
+	mailbox: mailbox@F7510000 {
+		#mbox-cells = <1>;
+		compatible = "hisilicon,hi6220-mbox";
+		reg = <0x0 0xF7510000 0x0 0x1000>, /* IPC_S */
+		      <0x0 0x06DFF800 0x0 0x0800>; /* Mailbox */
+		interrupt-parent = <&gic>;
+		interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
+		mbox_stub_clock: mbox_stub_clock {
+			hi6220,mbox-rx = <0 1 10>;
+			hi6220,mbox-tx = <1 0 11>;
+		};
+	};
+
+
+Mailbox client
+===============
+
+"mboxes" and the optional "mbox-names" (please see
+Documentation/devicetree/bindings/mailbox/mailbox.txt for details). Each value
+of the mboxes property should contain a phandle to the mailbox controller
+device node and second argument is the channel index. It must be 0 (hardware
+support only one channel). The equivalent "mbox-names" property value can be
+used to give a name to the communication channel to be used by the client user.
+
+Example:
+--------
+
+	stub_clock: stub_clock {
+		compatible = "hisilicon,hi6220-stub-clk";
+		hisilicon,hi6220-clk-sram = <&sram>;
+		#clock-cells = <1>;
+		mbox-names = "mbox-tx";
+		mboxes = <&mailbox 1>;
+	};
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 1/4] dt-bindings: mailbox: Document Hi6220 mailbox driver
@ 2016-02-01 13:34   ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2016-02-01 13:34 UTC (permalink / raw)
  To: linux-arm-kernel

Document DT binding for Hisilicon Hi6220 mailbox driver.

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

diff --git a/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
new file mode 100644
index 0000000..96e6acc
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
@@ -0,0 +1,90 @@
+Hisilicon Hi6220 Mailbox Driver
+===============================
+
+Hisilicon Hi6220 mailbox supports up to 32 channels. Each channel
+is unidirectional with a maximum message size of 8 words. I/O is
+performed using register access (there is no DMA) and the cell
+raises an interrupt when messages are received.
+
+Mailbox Device Node:
+====================
+
+Required properties:
+--------------------
+- compatible:		Shall be "hisilicon,hi6220-mbox"
+- reg:			Contains the mailbox register address range (base
+			address and length); the first item is for IPC
+			registers, the second item is shared buffer for
+			slots.
+- #mbox-cells		Common mailbox binding property to identify the number
+			of cells required for the mailbox specifier. Should be 1.
+- interrupts:		Contains the interrupt information for the mailbox
+			device. The format is dependent on which interrupt
+			controller the SoCs use.
+
+Optional Properties:
+--------------------
+- hi6220,mbox-tx-noirq: Flag to allow the client user of this mailbox driver
+			to send messages without triggering a TX completion
+			interrupt.
+
+Child Nodes:
+============
+A child node is used for representing the actual sub-mailbox device that is
+used for the communication between the host processor and a remote processor.
+Each child node should have a unique node name across all the different
+mailbox device nodes.
+
+Required properties:
+--------------------
+- hi6220,mbox-tx:	sub-mailbox descriptor property defining Tx channel
+- hi6220,mbox-rx:	sub-mailbox descriptor property defining Rx channel
+
+Sub-mailbox Descriptor Data
+---------------------------
+Each of the above hi6220,mbox-tx and hi6220,mbox-rx properties should have 3
+cells of data that represent the following:
+    Cell #1 (slot_id) - mailbox slot id used either for transmitting
+                        (hi6220,mbox-tx) or for receiving (hi6220,mbox-rx)
+    Cell #2 (dst_irq) - irq identifier index number which used by MCU.
+    Cell #3 (ack_irq) - irq identifier index number with generating a tx/rx
+                        interrupt to application processor, mailbox driver
+                        used this id to acknowledge interrupt.
+
+Example:
+--------
+
+	mailbox: mailbox at F7510000 {
+		#mbox-cells = <1>;
+		compatible = "hisilicon,hi6220-mbox";
+		reg = <0x0 0xF7510000 0x0 0x1000>, /* IPC_S */
+		      <0x0 0x06DFF800 0x0 0x0800>; /* Mailbox */
+		interrupt-parent = <&gic>;
+		interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
+		mbox_stub_clock: mbox_stub_clock {
+			hi6220,mbox-rx = <0 1 10>;
+			hi6220,mbox-tx = <1 0 11>;
+		};
+	};
+
+
+Mailbox client
+===============
+
+"mboxes" and the optional "mbox-names" (please see
+Documentation/devicetree/bindings/mailbox/mailbox.txt for details). Each value
+of the mboxes property should contain a phandle to the mailbox controller
+device node and second argument is the channel index. It must be 0 (hardware
+support only one channel). The equivalent "mbox-names" property value can be
+used to give a name to the communication channel to be used by the client user.
+
+Example:
+--------
+
+	stub_clock: stub_clock {
+		compatible = "hisilicon,hi6220-stub-clk";
+		hisilicon,hi6220-clk-sram = <&sram>;
+		#clock-cells = <1>;
+		mbox-names = "mbox-tx";
+		mboxes = <&mailbox 1>;
+	};
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 2/4] mailbox: Hi6220: add mailbox driver
@ 2016-02-01 13:34   ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2016-02-01 13:34 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Catalin Marinas, Will Deacon, Jassi Brar, Leo Yan, Arnd Bergmann,
	Wei Xu, Tyler Baker, Sudeep Holla, Chen Feng, Bintian Wang,
	devicetree, linux-kernel, linux-arm-kernel, Haojian Zhuang

Add driver for Hi6220 mailbox, the mailbox communicates with MCU; for
sending data, it can support two methods for low level implementation:
one is to use interrupt as acknowledge, another is automatic mode which
without any acknowledge. These two methods have been supported in the
driver. For receiving data, it will depend on the interrupt to notify
the channel has incoming message.

Now mailbox driver is used to send message to MCU to control dynamic
voltage and frequency scaling for CPU, GPU and DDR.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 drivers/mailbox/Kconfig          |   8 +
 drivers/mailbox/Makefile         |   2 +
 drivers/mailbox/hi6220-mailbox.c | 407 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 417 insertions(+)
 create mode 100644 drivers/mailbox/hi6220-mailbox.c

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 546d05f..b8bbeb0 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -78,6 +78,14 @@ config STI_MBOX
 	  Mailbox implementation for STMicroelectonics family chips with
 	  hardware for interprocessor communication.
 
+config HI6220_MBOX
+	tristate "Hi6220 Mailbox"
+	depends on ARCH_HISI
+	help
+	  An implementation of the hi6220 mailbox. It is used to send message
+	  between application processors and MCU. Say Y here if you want to
+	  build Hi6220 mailbox controller driver.
+
 config MAILBOX_TEST
 	tristate "Mailbox Test Client"
 	depends on OF
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 92435ef..565adda 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -17,3 +17,5 @@ obj-$(CONFIG_ALTERA_MBOX)	+= mailbox-altera.o
 obj-$(CONFIG_BCM2835_MBOX)	+= bcm2835-mailbox.o
 
 obj-$(CONFIG_STI_MBOX)		+= mailbox-sti.o
+
+obj-$(CONFIG_HI6220_MBOX)	+= hi6220-mailbox.o
diff --git a/drivers/mailbox/hi6220-mailbox.c b/drivers/mailbox/hi6220-mailbox.c
new file mode 100644
index 0000000..42db915
--- /dev/null
+++ b/drivers/mailbox/hi6220-mailbox.c
@@ -0,0 +1,407 @@
+/*
+ * Hisilicon's Hi6220 mailbox driver
+ *
+ * Copyright (c) 2015 Hisilicon Limited.
+ * Copyright (c) 2015 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/device.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kfifo.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#define MBOX_CHAN_MAX			32
+
+#define MBOX_RX				0x0
+#define MBOX_TX				0x1
+
+/* Mailbox message length: 8 words */
+#define MBOX_MSG_LEN			8
+
+/* Mailbox Registers */
+#define MBOX_OFF(m)			(0x40 * (m))
+#define MBOX_MODE_REG(m)		(MBOX_OFF(m) + 0x0)
+#define MBOX_DATA_REG(m)		(MBOX_OFF(m) + 0x4)
+
+#define MBOX_STATE_MASK			(0xF << 4)
+#define MBOX_STATE_IDLE			(0x1 << 4)
+#define MBOX_STATE_TX			(0x2 << 4)
+#define MBOX_STATE_RX			(0x4 << 4)
+#define MBOX_STATE_ACK			(0x8 << 4)
+#define MBOX_ACK_CONFIG_MASK		(0x1 << 0)
+#define MBOX_ACK_AUTOMATIC		(0x1 << 0)
+#define MBOX_ACK_IRQ			(0x0 << 0)
+
+/* IPC registers */
+#define ACK_INT_RAW_REG(i)		((i) + 0x400)
+#define ACK_INT_MSK_REG(i)		((i) + 0x404)
+#define ACK_INT_STAT_REG(i)		((i) + 0x408)
+#define ACK_INT_CLR_REG(i)		((i) + 0x40c)
+#define ACK_INT_ENA_REG(i)		((i) + 0x500)
+#define ACK_INT_DIS_REG(i)		((i) + 0x504)
+#define DST_INT_RAW_REG(i)		((i) + 0x420)
+
+
+struct hi6220_mbox_chan {
+
+	/*
+	 * Description for channel's hardware info:
+	 *  - direction: tx or rx
+	 *  - dst irq: peer core's irq number
+	 *  - ack irq: local irq number
+	 *  - slot number
+	 */
+	unsigned int dir, dst_irq, ack_irq;
+	unsigned int slot;
+
+	struct hi6220_mbox *parent;
+};
+
+struct hi6220_mbox {
+	struct device *dev;
+
+	int irq;
+
+	/* flag of enabling tx's irq mode */
+	bool tx_irq_mode;
+
+	/* region for ipc event */
+	void __iomem *ipc;
+
+	/* region for mailbox */
+	void __iomem *base;
+
+	unsigned int chan_num;
+	struct hi6220_mbox_chan *mchan;
+
+	void *irq_map_chan[MBOX_CHAN_MAX];
+	struct mbox_chan *chan;
+	struct mbox_controller controller;
+};
+
+static void mbox_set_state(struct hi6220_mbox *mbox,
+			   unsigned int slot, u32 val)
+{
+	u32 status;
+
+	status = readl(mbox->base + MBOX_MODE_REG(slot));
+	status = (status & ~MBOX_STATE_MASK) | val;
+	writel(status, mbox->base + MBOX_MODE_REG(slot));
+}
+
+static void mbox_set_mode(struct hi6220_mbox *mbox,
+			  unsigned int slot, u32 val)
+{
+	u32 mode;
+
+	mode = readl(mbox->base + MBOX_MODE_REG(slot));
+	mode = (mode & ~MBOX_ACK_CONFIG_MASK) | val;
+	writel(mode, mbox->base + MBOX_MODE_REG(slot));
+}
+
+static bool hi6220_mbox_last_tx_done(struct mbox_chan *chan)
+{
+	struct hi6220_mbox_chan *mchan = chan->con_priv;
+	struct hi6220_mbox *mbox = mchan->parent;
+	u32 state;
+
+	/* Only set idle state for polling mode */
+	BUG_ON(mbox->tx_irq_mode);
+
+	state = readl(mbox->base + MBOX_MODE_REG(mchan->slot));
+	return ((state & MBOX_STATE_MASK) == MBOX_STATE_IDLE);
+}
+
+static int hi6220_mbox_send_data(struct mbox_chan *chan, void *msg)
+{
+	struct hi6220_mbox_chan *mchan = chan->con_priv;
+	struct hi6220_mbox *mbox = mchan->parent;
+	unsigned int slot = mchan->slot;
+	u32 *buf = msg;
+	int i;
+
+	mbox_set_state(mbox, slot, MBOX_STATE_TX);
+
+	if (mbox->tx_irq_mode)
+		mbox_set_mode(mbox, slot, MBOX_ACK_IRQ);
+	else
+		mbox_set_mode(mbox, slot, MBOX_ACK_AUTOMATIC);
+
+	for (i = 0; i < MBOX_MSG_LEN; i++)
+		writel(buf[i], mbox->base + MBOX_DATA_REG(slot) + i * 4);
+
+	/* trigger remote request */
+	writel(BIT(mchan->dst_irq), DST_INT_RAW_REG(mbox->ipc));
+	return 0;
+}
+
+static irqreturn_t hi6220_mbox_interrupt(int irq, void *p)
+{
+	struct hi6220_mbox *mbox = p;
+	struct hi6220_mbox_chan *mchan;
+	struct mbox_chan *chan;
+	unsigned int state, intr_bit, i;
+	u32 msg[MBOX_MSG_LEN];
+
+	state = readl(ACK_INT_STAT_REG(mbox->ipc));
+	if (!state) {
+		dev_warn(mbox->dev, "%s: spurious interrupt\n",
+			 __func__);
+		return IRQ_HANDLED;
+	}
+
+	while (state) {
+		intr_bit = __ffs(state);
+		state &= (state - 1);
+
+		chan = mbox->irq_map_chan[intr_bit];
+		if (!chan) {
+			dev_warn(mbox->dev, "%s: unexpected irq vector %d\n",
+				 __func__, intr_bit);
+			continue;
+		}
+
+		mchan = chan->con_priv;
+		if (mchan->dir == MBOX_TX)
+			mbox_chan_txdone(chan, 0);
+		else {
+			for (i = 0; i < MBOX_MSG_LEN; i++)
+				msg[i] = readl(mbox->base +
+					MBOX_DATA_REG(mchan->slot) + i * 4);
+
+			mbox_chan_received_data(chan, (void *)msg);
+		}
+
+		/* clear IRQ source */
+		writel(BIT(mchan->ack_irq), ACK_INT_CLR_REG(mbox->ipc));
+		mbox_set_state(mbox, mchan->slot, MBOX_STATE_IDLE);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int hi6220_mbox_startup(struct mbox_chan *chan)
+{
+	struct hi6220_mbox_chan *mchan = chan->con_priv;
+	struct hi6220_mbox *mbox = mchan->parent;
+
+	mbox->irq_map_chan[mchan->ack_irq] = (void *)chan;
+
+	/* enable interrupt */
+	writel(BIT(mchan->ack_irq), ACK_INT_ENA_REG(mbox->ipc));
+	return 0;
+}
+
+static void hi6220_mbox_shutdown(struct mbox_chan *chan)
+{
+	struct hi6220_mbox_chan *mchan = chan->con_priv;
+	struct hi6220_mbox *mbox = mchan->parent;
+
+	/* disable interrupt */
+	writel(BIT(mchan->ack_irq), ACK_INT_DIS_REG(mbox->ipc));
+	mbox->irq_map_chan[mchan->ack_irq] = NULL;
+}
+
+static struct mbox_chan_ops hi6220_mbox_chan_ops = {
+	.send_data    = hi6220_mbox_send_data,
+	.startup      = hi6220_mbox_startup,
+	.shutdown     = hi6220_mbox_shutdown,
+	.last_tx_done = hi6220_mbox_last_tx_done,
+};
+
+static int hi6220_mbox_init_hw(struct hi6220_mbox *mbox,
+			       struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct device_node *child;
+	struct hi6220_mbox_chan *mchan = mbox->mchan;
+	u32 count;
+	u32 tmp[3];
+	int idx, i;
+	int ret;
+
+	for (i = 0; i < mbox->chan_num; i++) {
+		mbox->chan[i].con_priv = &mchan[i];
+		mbox->irq_map_chan[i] = NULL;
+
+		mchan[i].parent = mbox;
+		mchan[i].slot   = i;
+	}
+
+	count = of_get_available_child_count(node);
+	if (!count) {
+		dev_err(&pdev->dev, "no available mbox devices found\n");
+		return -ENODEV;
+	}
+
+	child = NULL;
+	for (i = 0; i < count; i++) {
+		child = of_get_next_available_child(node, child);
+		ret = of_property_read_u32_array(child, "hi6220,mbox-tx",
+						 tmp, ARRAY_SIZE(tmp));
+		if (!ret) {
+			idx = tmp[0];
+			mchan[idx].dir     = MBOX_TX,
+			mchan[idx].dst_irq = tmp[1];
+			mchan[idx].ack_irq = tmp[2];
+		}
+
+		ret = of_property_read_u32_array(child, "hi6220,mbox-rx",
+						 tmp, ARRAY_SIZE(tmp));
+		if (!ret) {
+			idx = tmp[0];
+			mchan[idx].dir     = MBOX_RX,
+			mchan[idx].dst_irq = tmp[1];
+			mchan[idx].ack_irq = tmp[2];
+		}
+	}
+
+	/* mask and clear all interrupt vectors */
+	writel(0x0,  ACK_INT_MSK_REG(mbox->ipc));
+	writel(~0x0, ACK_INT_CLR_REG(mbox->ipc));
+
+	/* use interrupt for tx's ack */
+	if (of_find_property(node, "hi6220,mbox-tx-noirq", NULL))
+		mbox->tx_irq_mode = false;
+	else
+		mbox->tx_irq_mode = true;
+
+	return 0;
+}
+
+static const struct of_device_id hi6220_mbox_of_match[] = {
+	{ .compatible = "hisilicon,hi6220-mbox", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, hi6220_mbox_of_match);
+
+static int hi6220_mbox_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct hi6220_mbox *mbox;
+	struct resource *res;
+	int err;
+
+	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
+	if (!mbox)
+		return -ENOMEM;
+
+	mbox->dev = dev;
+	mbox->chan_num = MBOX_CHAN_MAX;
+	mbox->mchan = devm_kzalloc(dev,
+		mbox->chan_num * sizeof(*mbox->mchan), GFP_KERNEL);
+	if (!mbox->mchan)
+		return -ENOMEM;
+
+	mbox->chan = devm_kzalloc(dev,
+		mbox->chan_num * sizeof(*mbox->chan), GFP_KERNEL);
+	if (!mbox->chan)
+		return -ENOMEM;
+
+	mbox->irq = platform_get_irq(pdev, 0);
+	if (mbox->irq < 0)
+		return mbox->irq;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mbox->ipc = devm_ioremap_resource(dev, res);
+	if (IS_ERR(mbox->ipc)) {
+		dev_err(dev, "ioremap ipc failed\n");
+		return PTR_ERR(mbox->ipc);
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	mbox->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(mbox->base)) {
+		dev_err(dev, "ioremap buffer failed\n");
+		return PTR_ERR(mbox->base);
+	}
+
+	err = devm_request_irq(dev, mbox->irq, hi6220_mbox_interrupt, 0,
+			dev_name(dev), mbox);
+	if (err) {
+		dev_err(dev, "Failed to register a mailbox IRQ handler: %d\n",
+			err);
+		return -ENODEV;
+	}
+
+	/* init hardware parameters */
+	err = hi6220_mbox_init_hw(mbox, pdev);
+	if (err) {
+		dev_err(dev, "Failed to init mbox hardware channels: %d\n",
+			err);
+		return -ENODEV;
+	}
+
+	mbox->controller.dev = dev;
+	mbox->controller.chans = &mbox->chan[0];
+	mbox->controller.num_chans = mbox->chan_num;
+	mbox->controller.ops = &hi6220_mbox_chan_ops;
+
+	if (mbox->tx_irq_mode)
+		mbox->controller.txdone_irq = true;
+	else {
+		mbox->controller.txdone_poll = true;
+		mbox->controller.txpoll_period = 5;
+	}
+
+	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 hi6220_mbox_remove(struct platform_device *pdev)
+{
+	struct hi6220_mbox *mbox = platform_get_drvdata(pdev);
+
+	mbox_controller_unregister(&mbox->controller);
+	return 0;
+}
+
+static struct platform_driver hi6220_mbox_driver = {
+	.driver = {
+		.name = "hi6220-mbox",
+		.owner = THIS_MODULE,
+		.of_match_table = hi6220_mbox_of_match,
+	},
+	.probe	= hi6220_mbox_probe,
+	.remove	= hi6220_mbox_remove,
+};
+
+static int __init hi6220_mbox_init(void)
+{
+	return platform_driver_register(&hi6220_mbox_driver);
+}
+core_initcall(hi6220_mbox_init);
+
+static void __exit hi6220_mbox_exit(void)
+{
+	platform_driver_unregister(&hi6220_mbox_driver);
+}
+module_exit(hi6220_mbox_exit);
+
+MODULE_AUTHOR("Leo Yan <leo.yan@linaro.org>");
+MODULE_DESCRIPTION("Hi6220 mailbox driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 2/4] mailbox: Hi6220: add mailbox driver
@ 2016-02-01 13:34   ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2016-02-01 13:34 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Catalin Marinas, Will Deacon, Jassi Brar, Leo Yan, Arnd Bergmann,
	Wei Xu, Tyler Baker, Sudeep Holla, Chen Feng, Bintian Wang,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Haojian Zhuang

Add driver for Hi6220 mailbox, the mailbox communicates with MCU; for
sending data, it can support two methods for low level implementation:
one is to use interrupt as acknowledge, another is automatic mode which
without any acknowledge. These two methods have been supported in the
driver. For receiving data, it will depend on the interrupt to notify
the channel has incoming message.

Now mailbox driver is used to send message to MCU to control dynamic
voltage and frequency scaling for CPU, GPU and DDR.

Signed-off-by: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/mailbox/Kconfig          |   8 +
 drivers/mailbox/Makefile         |   2 +
 drivers/mailbox/hi6220-mailbox.c | 407 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 417 insertions(+)
 create mode 100644 drivers/mailbox/hi6220-mailbox.c

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 546d05f..b8bbeb0 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -78,6 +78,14 @@ config STI_MBOX
 	  Mailbox implementation for STMicroelectonics family chips with
 	  hardware for interprocessor communication.
 
+config HI6220_MBOX
+	tristate "Hi6220 Mailbox"
+	depends on ARCH_HISI
+	help
+	  An implementation of the hi6220 mailbox. It is used to send message
+	  between application processors and MCU. Say Y here if you want to
+	  build Hi6220 mailbox controller driver.
+
 config MAILBOX_TEST
 	tristate "Mailbox Test Client"
 	depends on OF
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 92435ef..565adda 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -17,3 +17,5 @@ obj-$(CONFIG_ALTERA_MBOX)	+= mailbox-altera.o
 obj-$(CONFIG_BCM2835_MBOX)	+= bcm2835-mailbox.o
 
 obj-$(CONFIG_STI_MBOX)		+= mailbox-sti.o
+
+obj-$(CONFIG_HI6220_MBOX)	+= hi6220-mailbox.o
diff --git a/drivers/mailbox/hi6220-mailbox.c b/drivers/mailbox/hi6220-mailbox.c
new file mode 100644
index 0000000..42db915
--- /dev/null
+++ b/drivers/mailbox/hi6220-mailbox.c
@@ -0,0 +1,407 @@
+/*
+ * Hisilicon's Hi6220 mailbox driver
+ *
+ * Copyright (c) 2015 Hisilicon Limited.
+ * Copyright (c) 2015 Linaro Limited.
+ *
+ * Author: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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/device.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kfifo.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#define MBOX_CHAN_MAX			32
+
+#define MBOX_RX				0x0
+#define MBOX_TX				0x1
+
+/* Mailbox message length: 8 words */
+#define MBOX_MSG_LEN			8
+
+/* Mailbox Registers */
+#define MBOX_OFF(m)			(0x40 * (m))
+#define MBOX_MODE_REG(m)		(MBOX_OFF(m) + 0x0)
+#define MBOX_DATA_REG(m)		(MBOX_OFF(m) + 0x4)
+
+#define MBOX_STATE_MASK			(0xF << 4)
+#define MBOX_STATE_IDLE			(0x1 << 4)
+#define MBOX_STATE_TX			(0x2 << 4)
+#define MBOX_STATE_RX			(0x4 << 4)
+#define MBOX_STATE_ACK			(0x8 << 4)
+#define MBOX_ACK_CONFIG_MASK		(0x1 << 0)
+#define MBOX_ACK_AUTOMATIC		(0x1 << 0)
+#define MBOX_ACK_IRQ			(0x0 << 0)
+
+/* IPC registers */
+#define ACK_INT_RAW_REG(i)		((i) + 0x400)
+#define ACK_INT_MSK_REG(i)		((i) + 0x404)
+#define ACK_INT_STAT_REG(i)		((i) + 0x408)
+#define ACK_INT_CLR_REG(i)		((i) + 0x40c)
+#define ACK_INT_ENA_REG(i)		((i) + 0x500)
+#define ACK_INT_DIS_REG(i)		((i) + 0x504)
+#define DST_INT_RAW_REG(i)		((i) + 0x420)
+
+
+struct hi6220_mbox_chan {
+
+	/*
+	 * Description for channel's hardware info:
+	 *  - direction: tx or rx
+	 *  - dst irq: peer core's irq number
+	 *  - ack irq: local irq number
+	 *  - slot number
+	 */
+	unsigned int dir, dst_irq, ack_irq;
+	unsigned int slot;
+
+	struct hi6220_mbox *parent;
+};
+
+struct hi6220_mbox {
+	struct device *dev;
+
+	int irq;
+
+	/* flag of enabling tx's irq mode */
+	bool tx_irq_mode;
+
+	/* region for ipc event */
+	void __iomem *ipc;
+
+	/* region for mailbox */
+	void __iomem *base;
+
+	unsigned int chan_num;
+	struct hi6220_mbox_chan *mchan;
+
+	void *irq_map_chan[MBOX_CHAN_MAX];
+	struct mbox_chan *chan;
+	struct mbox_controller controller;
+};
+
+static void mbox_set_state(struct hi6220_mbox *mbox,
+			   unsigned int slot, u32 val)
+{
+	u32 status;
+
+	status = readl(mbox->base + MBOX_MODE_REG(slot));
+	status = (status & ~MBOX_STATE_MASK) | val;
+	writel(status, mbox->base + MBOX_MODE_REG(slot));
+}
+
+static void mbox_set_mode(struct hi6220_mbox *mbox,
+			  unsigned int slot, u32 val)
+{
+	u32 mode;
+
+	mode = readl(mbox->base + MBOX_MODE_REG(slot));
+	mode = (mode & ~MBOX_ACK_CONFIG_MASK) | val;
+	writel(mode, mbox->base + MBOX_MODE_REG(slot));
+}
+
+static bool hi6220_mbox_last_tx_done(struct mbox_chan *chan)
+{
+	struct hi6220_mbox_chan *mchan = chan->con_priv;
+	struct hi6220_mbox *mbox = mchan->parent;
+	u32 state;
+
+	/* Only set idle state for polling mode */
+	BUG_ON(mbox->tx_irq_mode);
+
+	state = readl(mbox->base + MBOX_MODE_REG(mchan->slot));
+	return ((state & MBOX_STATE_MASK) == MBOX_STATE_IDLE);
+}
+
+static int hi6220_mbox_send_data(struct mbox_chan *chan, void *msg)
+{
+	struct hi6220_mbox_chan *mchan = chan->con_priv;
+	struct hi6220_mbox *mbox = mchan->parent;
+	unsigned int slot = mchan->slot;
+	u32 *buf = msg;
+	int i;
+
+	mbox_set_state(mbox, slot, MBOX_STATE_TX);
+
+	if (mbox->tx_irq_mode)
+		mbox_set_mode(mbox, slot, MBOX_ACK_IRQ);
+	else
+		mbox_set_mode(mbox, slot, MBOX_ACK_AUTOMATIC);
+
+	for (i = 0; i < MBOX_MSG_LEN; i++)
+		writel(buf[i], mbox->base + MBOX_DATA_REG(slot) + i * 4);
+
+	/* trigger remote request */
+	writel(BIT(mchan->dst_irq), DST_INT_RAW_REG(mbox->ipc));
+	return 0;
+}
+
+static irqreturn_t hi6220_mbox_interrupt(int irq, void *p)
+{
+	struct hi6220_mbox *mbox = p;
+	struct hi6220_mbox_chan *mchan;
+	struct mbox_chan *chan;
+	unsigned int state, intr_bit, i;
+	u32 msg[MBOX_MSG_LEN];
+
+	state = readl(ACK_INT_STAT_REG(mbox->ipc));
+	if (!state) {
+		dev_warn(mbox->dev, "%s: spurious interrupt\n",
+			 __func__);
+		return IRQ_HANDLED;
+	}
+
+	while (state) {
+		intr_bit = __ffs(state);
+		state &= (state - 1);
+
+		chan = mbox->irq_map_chan[intr_bit];
+		if (!chan) {
+			dev_warn(mbox->dev, "%s: unexpected irq vector %d\n",
+				 __func__, intr_bit);
+			continue;
+		}
+
+		mchan = chan->con_priv;
+		if (mchan->dir == MBOX_TX)
+			mbox_chan_txdone(chan, 0);
+		else {
+			for (i = 0; i < MBOX_MSG_LEN; i++)
+				msg[i] = readl(mbox->base +
+					MBOX_DATA_REG(mchan->slot) + i * 4);
+
+			mbox_chan_received_data(chan, (void *)msg);
+		}
+
+		/* clear IRQ source */
+		writel(BIT(mchan->ack_irq), ACK_INT_CLR_REG(mbox->ipc));
+		mbox_set_state(mbox, mchan->slot, MBOX_STATE_IDLE);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int hi6220_mbox_startup(struct mbox_chan *chan)
+{
+	struct hi6220_mbox_chan *mchan = chan->con_priv;
+	struct hi6220_mbox *mbox = mchan->parent;
+
+	mbox->irq_map_chan[mchan->ack_irq] = (void *)chan;
+
+	/* enable interrupt */
+	writel(BIT(mchan->ack_irq), ACK_INT_ENA_REG(mbox->ipc));
+	return 0;
+}
+
+static void hi6220_mbox_shutdown(struct mbox_chan *chan)
+{
+	struct hi6220_mbox_chan *mchan = chan->con_priv;
+	struct hi6220_mbox *mbox = mchan->parent;
+
+	/* disable interrupt */
+	writel(BIT(mchan->ack_irq), ACK_INT_DIS_REG(mbox->ipc));
+	mbox->irq_map_chan[mchan->ack_irq] = NULL;
+}
+
+static struct mbox_chan_ops hi6220_mbox_chan_ops = {
+	.send_data    = hi6220_mbox_send_data,
+	.startup      = hi6220_mbox_startup,
+	.shutdown     = hi6220_mbox_shutdown,
+	.last_tx_done = hi6220_mbox_last_tx_done,
+};
+
+static int hi6220_mbox_init_hw(struct hi6220_mbox *mbox,
+			       struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct device_node *child;
+	struct hi6220_mbox_chan *mchan = mbox->mchan;
+	u32 count;
+	u32 tmp[3];
+	int idx, i;
+	int ret;
+
+	for (i = 0; i < mbox->chan_num; i++) {
+		mbox->chan[i].con_priv = &mchan[i];
+		mbox->irq_map_chan[i] = NULL;
+
+		mchan[i].parent = mbox;
+		mchan[i].slot   = i;
+	}
+
+	count = of_get_available_child_count(node);
+	if (!count) {
+		dev_err(&pdev->dev, "no available mbox devices found\n");
+		return -ENODEV;
+	}
+
+	child = NULL;
+	for (i = 0; i < count; i++) {
+		child = of_get_next_available_child(node, child);
+		ret = of_property_read_u32_array(child, "hi6220,mbox-tx",
+						 tmp, ARRAY_SIZE(tmp));
+		if (!ret) {
+			idx = tmp[0];
+			mchan[idx].dir     = MBOX_TX,
+			mchan[idx].dst_irq = tmp[1];
+			mchan[idx].ack_irq = tmp[2];
+		}
+
+		ret = of_property_read_u32_array(child, "hi6220,mbox-rx",
+						 tmp, ARRAY_SIZE(tmp));
+		if (!ret) {
+			idx = tmp[0];
+			mchan[idx].dir     = MBOX_RX,
+			mchan[idx].dst_irq = tmp[1];
+			mchan[idx].ack_irq = tmp[2];
+		}
+	}
+
+	/* mask and clear all interrupt vectors */
+	writel(0x0,  ACK_INT_MSK_REG(mbox->ipc));
+	writel(~0x0, ACK_INT_CLR_REG(mbox->ipc));
+
+	/* use interrupt for tx's ack */
+	if (of_find_property(node, "hi6220,mbox-tx-noirq", NULL))
+		mbox->tx_irq_mode = false;
+	else
+		mbox->tx_irq_mode = true;
+
+	return 0;
+}
+
+static const struct of_device_id hi6220_mbox_of_match[] = {
+	{ .compatible = "hisilicon,hi6220-mbox", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, hi6220_mbox_of_match);
+
+static int hi6220_mbox_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct hi6220_mbox *mbox;
+	struct resource *res;
+	int err;
+
+	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
+	if (!mbox)
+		return -ENOMEM;
+
+	mbox->dev = dev;
+	mbox->chan_num = MBOX_CHAN_MAX;
+	mbox->mchan = devm_kzalloc(dev,
+		mbox->chan_num * sizeof(*mbox->mchan), GFP_KERNEL);
+	if (!mbox->mchan)
+		return -ENOMEM;
+
+	mbox->chan = devm_kzalloc(dev,
+		mbox->chan_num * sizeof(*mbox->chan), GFP_KERNEL);
+	if (!mbox->chan)
+		return -ENOMEM;
+
+	mbox->irq = platform_get_irq(pdev, 0);
+	if (mbox->irq < 0)
+		return mbox->irq;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mbox->ipc = devm_ioremap_resource(dev, res);
+	if (IS_ERR(mbox->ipc)) {
+		dev_err(dev, "ioremap ipc failed\n");
+		return PTR_ERR(mbox->ipc);
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	mbox->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(mbox->base)) {
+		dev_err(dev, "ioremap buffer failed\n");
+		return PTR_ERR(mbox->base);
+	}
+
+	err = devm_request_irq(dev, mbox->irq, hi6220_mbox_interrupt, 0,
+			dev_name(dev), mbox);
+	if (err) {
+		dev_err(dev, "Failed to register a mailbox IRQ handler: %d\n",
+			err);
+		return -ENODEV;
+	}
+
+	/* init hardware parameters */
+	err = hi6220_mbox_init_hw(mbox, pdev);
+	if (err) {
+		dev_err(dev, "Failed to init mbox hardware channels: %d\n",
+			err);
+		return -ENODEV;
+	}
+
+	mbox->controller.dev = dev;
+	mbox->controller.chans = &mbox->chan[0];
+	mbox->controller.num_chans = mbox->chan_num;
+	mbox->controller.ops = &hi6220_mbox_chan_ops;
+
+	if (mbox->tx_irq_mode)
+		mbox->controller.txdone_irq = true;
+	else {
+		mbox->controller.txdone_poll = true;
+		mbox->controller.txpoll_period = 5;
+	}
+
+	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 hi6220_mbox_remove(struct platform_device *pdev)
+{
+	struct hi6220_mbox *mbox = platform_get_drvdata(pdev);
+
+	mbox_controller_unregister(&mbox->controller);
+	return 0;
+}
+
+static struct platform_driver hi6220_mbox_driver = {
+	.driver = {
+		.name = "hi6220-mbox",
+		.owner = THIS_MODULE,
+		.of_match_table = hi6220_mbox_of_match,
+	},
+	.probe	= hi6220_mbox_probe,
+	.remove	= hi6220_mbox_remove,
+};
+
+static int __init hi6220_mbox_init(void)
+{
+	return platform_driver_register(&hi6220_mbox_driver);
+}
+core_initcall(hi6220_mbox_init);
+
+static void __exit hi6220_mbox_exit(void)
+{
+	platform_driver_unregister(&hi6220_mbox_driver);
+}
+module_exit(hi6220_mbox_exit);
+
+MODULE_AUTHOR("Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>");
+MODULE_DESCRIPTION("Hi6220 mailbox driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 2/4] mailbox: Hi6220: add mailbox driver
@ 2016-02-01 13:34   ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2016-02-01 13:34 UTC (permalink / raw)
  To: linux-arm-kernel

Add driver for Hi6220 mailbox, the mailbox communicates with MCU; for
sending data, it can support two methods for low level implementation:
one is to use interrupt as acknowledge, another is automatic mode which
without any acknowledge. These two methods have been supported in the
driver. For receiving data, it will depend on the interrupt to notify
the channel has incoming message.

Now mailbox driver is used to send message to MCU to control dynamic
voltage and frequency scaling for CPU, GPU and DDR.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 drivers/mailbox/Kconfig          |   8 +
 drivers/mailbox/Makefile         |   2 +
 drivers/mailbox/hi6220-mailbox.c | 407 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 417 insertions(+)
 create mode 100644 drivers/mailbox/hi6220-mailbox.c

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 546d05f..b8bbeb0 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -78,6 +78,14 @@ config STI_MBOX
 	  Mailbox implementation for STMicroelectonics family chips with
 	  hardware for interprocessor communication.
 
+config HI6220_MBOX
+	tristate "Hi6220 Mailbox"
+	depends on ARCH_HISI
+	help
+	  An implementation of the hi6220 mailbox. It is used to send message
+	  between application processors and MCU. Say Y here if you want to
+	  build Hi6220 mailbox controller driver.
+
 config MAILBOX_TEST
 	tristate "Mailbox Test Client"
 	depends on OF
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 92435ef..565adda 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -17,3 +17,5 @@ obj-$(CONFIG_ALTERA_MBOX)	+= mailbox-altera.o
 obj-$(CONFIG_BCM2835_MBOX)	+= bcm2835-mailbox.o
 
 obj-$(CONFIG_STI_MBOX)		+= mailbox-sti.o
+
+obj-$(CONFIG_HI6220_MBOX)	+= hi6220-mailbox.o
diff --git a/drivers/mailbox/hi6220-mailbox.c b/drivers/mailbox/hi6220-mailbox.c
new file mode 100644
index 0000000..42db915
--- /dev/null
+++ b/drivers/mailbox/hi6220-mailbox.c
@@ -0,0 +1,407 @@
+/*
+ * Hisilicon's Hi6220 mailbox driver
+ *
+ * Copyright (c) 2015 Hisilicon Limited.
+ * Copyright (c) 2015 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/device.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kfifo.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#define MBOX_CHAN_MAX			32
+
+#define MBOX_RX				0x0
+#define MBOX_TX				0x1
+
+/* Mailbox message length: 8 words */
+#define MBOX_MSG_LEN			8
+
+/* Mailbox Registers */
+#define MBOX_OFF(m)			(0x40 * (m))
+#define MBOX_MODE_REG(m)		(MBOX_OFF(m) + 0x0)
+#define MBOX_DATA_REG(m)		(MBOX_OFF(m) + 0x4)
+
+#define MBOX_STATE_MASK			(0xF << 4)
+#define MBOX_STATE_IDLE			(0x1 << 4)
+#define MBOX_STATE_TX			(0x2 << 4)
+#define MBOX_STATE_RX			(0x4 << 4)
+#define MBOX_STATE_ACK			(0x8 << 4)
+#define MBOX_ACK_CONFIG_MASK		(0x1 << 0)
+#define MBOX_ACK_AUTOMATIC		(0x1 << 0)
+#define MBOX_ACK_IRQ			(0x0 << 0)
+
+/* IPC registers */
+#define ACK_INT_RAW_REG(i)		((i) + 0x400)
+#define ACK_INT_MSK_REG(i)		((i) + 0x404)
+#define ACK_INT_STAT_REG(i)		((i) + 0x408)
+#define ACK_INT_CLR_REG(i)		((i) + 0x40c)
+#define ACK_INT_ENA_REG(i)		((i) + 0x500)
+#define ACK_INT_DIS_REG(i)		((i) + 0x504)
+#define DST_INT_RAW_REG(i)		((i) + 0x420)
+
+
+struct hi6220_mbox_chan {
+
+	/*
+	 * Description for channel's hardware info:
+	 *  - direction: tx or rx
+	 *  - dst irq: peer core's irq number
+	 *  - ack irq: local irq number
+	 *  - slot number
+	 */
+	unsigned int dir, dst_irq, ack_irq;
+	unsigned int slot;
+
+	struct hi6220_mbox *parent;
+};
+
+struct hi6220_mbox {
+	struct device *dev;
+
+	int irq;
+
+	/* flag of enabling tx's irq mode */
+	bool tx_irq_mode;
+
+	/* region for ipc event */
+	void __iomem *ipc;
+
+	/* region for mailbox */
+	void __iomem *base;
+
+	unsigned int chan_num;
+	struct hi6220_mbox_chan *mchan;
+
+	void *irq_map_chan[MBOX_CHAN_MAX];
+	struct mbox_chan *chan;
+	struct mbox_controller controller;
+};
+
+static void mbox_set_state(struct hi6220_mbox *mbox,
+			   unsigned int slot, u32 val)
+{
+	u32 status;
+
+	status = readl(mbox->base + MBOX_MODE_REG(slot));
+	status = (status & ~MBOX_STATE_MASK) | val;
+	writel(status, mbox->base + MBOX_MODE_REG(slot));
+}
+
+static void mbox_set_mode(struct hi6220_mbox *mbox,
+			  unsigned int slot, u32 val)
+{
+	u32 mode;
+
+	mode = readl(mbox->base + MBOX_MODE_REG(slot));
+	mode = (mode & ~MBOX_ACK_CONFIG_MASK) | val;
+	writel(mode, mbox->base + MBOX_MODE_REG(slot));
+}
+
+static bool hi6220_mbox_last_tx_done(struct mbox_chan *chan)
+{
+	struct hi6220_mbox_chan *mchan = chan->con_priv;
+	struct hi6220_mbox *mbox = mchan->parent;
+	u32 state;
+
+	/* Only set idle state for polling mode */
+	BUG_ON(mbox->tx_irq_mode);
+
+	state = readl(mbox->base + MBOX_MODE_REG(mchan->slot));
+	return ((state & MBOX_STATE_MASK) == MBOX_STATE_IDLE);
+}
+
+static int hi6220_mbox_send_data(struct mbox_chan *chan, void *msg)
+{
+	struct hi6220_mbox_chan *mchan = chan->con_priv;
+	struct hi6220_mbox *mbox = mchan->parent;
+	unsigned int slot = mchan->slot;
+	u32 *buf = msg;
+	int i;
+
+	mbox_set_state(mbox, slot, MBOX_STATE_TX);
+
+	if (mbox->tx_irq_mode)
+		mbox_set_mode(mbox, slot, MBOX_ACK_IRQ);
+	else
+		mbox_set_mode(mbox, slot, MBOX_ACK_AUTOMATIC);
+
+	for (i = 0; i < MBOX_MSG_LEN; i++)
+		writel(buf[i], mbox->base + MBOX_DATA_REG(slot) + i * 4);
+
+	/* trigger remote request */
+	writel(BIT(mchan->dst_irq), DST_INT_RAW_REG(mbox->ipc));
+	return 0;
+}
+
+static irqreturn_t hi6220_mbox_interrupt(int irq, void *p)
+{
+	struct hi6220_mbox *mbox = p;
+	struct hi6220_mbox_chan *mchan;
+	struct mbox_chan *chan;
+	unsigned int state, intr_bit, i;
+	u32 msg[MBOX_MSG_LEN];
+
+	state = readl(ACK_INT_STAT_REG(mbox->ipc));
+	if (!state) {
+		dev_warn(mbox->dev, "%s: spurious interrupt\n",
+			 __func__);
+		return IRQ_HANDLED;
+	}
+
+	while (state) {
+		intr_bit = __ffs(state);
+		state &= (state - 1);
+
+		chan = mbox->irq_map_chan[intr_bit];
+		if (!chan) {
+			dev_warn(mbox->dev, "%s: unexpected irq vector %d\n",
+				 __func__, intr_bit);
+			continue;
+		}
+
+		mchan = chan->con_priv;
+		if (mchan->dir == MBOX_TX)
+			mbox_chan_txdone(chan, 0);
+		else {
+			for (i = 0; i < MBOX_MSG_LEN; i++)
+				msg[i] = readl(mbox->base +
+					MBOX_DATA_REG(mchan->slot) + i * 4);
+
+			mbox_chan_received_data(chan, (void *)msg);
+		}
+
+		/* clear IRQ source */
+		writel(BIT(mchan->ack_irq), ACK_INT_CLR_REG(mbox->ipc));
+		mbox_set_state(mbox, mchan->slot, MBOX_STATE_IDLE);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int hi6220_mbox_startup(struct mbox_chan *chan)
+{
+	struct hi6220_mbox_chan *mchan = chan->con_priv;
+	struct hi6220_mbox *mbox = mchan->parent;
+
+	mbox->irq_map_chan[mchan->ack_irq] = (void *)chan;
+
+	/* enable interrupt */
+	writel(BIT(mchan->ack_irq), ACK_INT_ENA_REG(mbox->ipc));
+	return 0;
+}
+
+static void hi6220_mbox_shutdown(struct mbox_chan *chan)
+{
+	struct hi6220_mbox_chan *mchan = chan->con_priv;
+	struct hi6220_mbox *mbox = mchan->parent;
+
+	/* disable interrupt */
+	writel(BIT(mchan->ack_irq), ACK_INT_DIS_REG(mbox->ipc));
+	mbox->irq_map_chan[mchan->ack_irq] = NULL;
+}
+
+static struct mbox_chan_ops hi6220_mbox_chan_ops = {
+	.send_data    = hi6220_mbox_send_data,
+	.startup      = hi6220_mbox_startup,
+	.shutdown     = hi6220_mbox_shutdown,
+	.last_tx_done = hi6220_mbox_last_tx_done,
+};
+
+static int hi6220_mbox_init_hw(struct hi6220_mbox *mbox,
+			       struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct device_node *child;
+	struct hi6220_mbox_chan *mchan = mbox->mchan;
+	u32 count;
+	u32 tmp[3];
+	int idx, i;
+	int ret;
+
+	for (i = 0; i < mbox->chan_num; i++) {
+		mbox->chan[i].con_priv = &mchan[i];
+		mbox->irq_map_chan[i] = NULL;
+
+		mchan[i].parent = mbox;
+		mchan[i].slot   = i;
+	}
+
+	count = of_get_available_child_count(node);
+	if (!count) {
+		dev_err(&pdev->dev, "no available mbox devices found\n");
+		return -ENODEV;
+	}
+
+	child = NULL;
+	for (i = 0; i < count; i++) {
+		child = of_get_next_available_child(node, child);
+		ret = of_property_read_u32_array(child, "hi6220,mbox-tx",
+						 tmp, ARRAY_SIZE(tmp));
+		if (!ret) {
+			idx = tmp[0];
+			mchan[idx].dir     = MBOX_TX,
+			mchan[idx].dst_irq = tmp[1];
+			mchan[idx].ack_irq = tmp[2];
+		}
+
+		ret = of_property_read_u32_array(child, "hi6220,mbox-rx",
+						 tmp, ARRAY_SIZE(tmp));
+		if (!ret) {
+			idx = tmp[0];
+			mchan[idx].dir     = MBOX_RX,
+			mchan[idx].dst_irq = tmp[1];
+			mchan[idx].ack_irq = tmp[2];
+		}
+	}
+
+	/* mask and clear all interrupt vectors */
+	writel(0x0,  ACK_INT_MSK_REG(mbox->ipc));
+	writel(~0x0, ACK_INT_CLR_REG(mbox->ipc));
+
+	/* use interrupt for tx's ack */
+	if (of_find_property(node, "hi6220,mbox-tx-noirq", NULL))
+		mbox->tx_irq_mode = false;
+	else
+		mbox->tx_irq_mode = true;
+
+	return 0;
+}
+
+static const struct of_device_id hi6220_mbox_of_match[] = {
+	{ .compatible = "hisilicon,hi6220-mbox", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, hi6220_mbox_of_match);
+
+static int hi6220_mbox_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct hi6220_mbox *mbox;
+	struct resource *res;
+	int err;
+
+	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
+	if (!mbox)
+		return -ENOMEM;
+
+	mbox->dev = dev;
+	mbox->chan_num = MBOX_CHAN_MAX;
+	mbox->mchan = devm_kzalloc(dev,
+		mbox->chan_num * sizeof(*mbox->mchan), GFP_KERNEL);
+	if (!mbox->mchan)
+		return -ENOMEM;
+
+	mbox->chan = devm_kzalloc(dev,
+		mbox->chan_num * sizeof(*mbox->chan), GFP_KERNEL);
+	if (!mbox->chan)
+		return -ENOMEM;
+
+	mbox->irq = platform_get_irq(pdev, 0);
+	if (mbox->irq < 0)
+		return mbox->irq;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mbox->ipc = devm_ioremap_resource(dev, res);
+	if (IS_ERR(mbox->ipc)) {
+		dev_err(dev, "ioremap ipc failed\n");
+		return PTR_ERR(mbox->ipc);
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	mbox->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(mbox->base)) {
+		dev_err(dev, "ioremap buffer failed\n");
+		return PTR_ERR(mbox->base);
+	}
+
+	err = devm_request_irq(dev, mbox->irq, hi6220_mbox_interrupt, 0,
+			dev_name(dev), mbox);
+	if (err) {
+		dev_err(dev, "Failed to register a mailbox IRQ handler: %d\n",
+			err);
+		return -ENODEV;
+	}
+
+	/* init hardware parameters */
+	err = hi6220_mbox_init_hw(mbox, pdev);
+	if (err) {
+		dev_err(dev, "Failed to init mbox hardware channels: %d\n",
+			err);
+		return -ENODEV;
+	}
+
+	mbox->controller.dev = dev;
+	mbox->controller.chans = &mbox->chan[0];
+	mbox->controller.num_chans = mbox->chan_num;
+	mbox->controller.ops = &hi6220_mbox_chan_ops;
+
+	if (mbox->tx_irq_mode)
+		mbox->controller.txdone_irq = true;
+	else {
+		mbox->controller.txdone_poll = true;
+		mbox->controller.txpoll_period = 5;
+	}
+
+	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 hi6220_mbox_remove(struct platform_device *pdev)
+{
+	struct hi6220_mbox *mbox = platform_get_drvdata(pdev);
+
+	mbox_controller_unregister(&mbox->controller);
+	return 0;
+}
+
+static struct platform_driver hi6220_mbox_driver = {
+	.driver = {
+		.name = "hi6220-mbox",
+		.owner = THIS_MODULE,
+		.of_match_table = hi6220_mbox_of_match,
+	},
+	.probe	= hi6220_mbox_probe,
+	.remove	= hi6220_mbox_remove,
+};
+
+static int __init hi6220_mbox_init(void)
+{
+	return platform_driver_register(&hi6220_mbox_driver);
+}
+core_initcall(hi6220_mbox_init);
+
+static void __exit hi6220_mbox_exit(void)
+{
+	platform_driver_unregister(&hi6220_mbox_driver);
+}
+module_exit(hi6220_mbox_exit);
+
+MODULE_AUTHOR("Leo Yan <leo.yan@linaro.org>");
+MODULE_DESCRIPTION("Hi6220 mailbox driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 3/4] arm64: dts: add mailbox node for Hi6220
  2016-02-01 13:34 ` Leo Yan
@ 2016-02-01 13:34   ` Leo Yan
  -1 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2016-02-01 13:34 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Catalin Marinas, Will Deacon, Jassi Brar, Leo Yan, Arnd Bergmann,
	Wei Xu, Tyler Baker, Sudeep Holla, Chen Feng, Bintian Wang,
	devicetree, linux-kernel, linux-arm-kernel, Haojian Zhuang

This patch add device mailbox node for Hi6220 in DT.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index 2585236..be3d962 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -249,5 +249,17 @@
 			clocks = <&ao_ctrl 27>;
 			clock-names = "apb_pclk";
 		};
+
+		mailbox: mailbox@f7510000 {
+			#mbox-cells = <1>;
+			compatible = "hisilicon,hi6220-mbox";
+			reg = <0x0 0xf7510000 0x0 0x1000>, /* IPC_S */
+			      <0x0 0x06dff800 0x0 0x0800>; /* Mailbox buffer */
+			interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
+			mbox_stub_clock: mbox_stub_clock {
+				hi6220,mbox-rx = <0 1 10>;
+				hi6220,mbox-tx = <1 0 11>;
+			};
+		};
 	};
 };
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 3/4] arm64: dts: add mailbox node for Hi6220
@ 2016-02-01 13:34   ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2016-02-01 13:34 UTC (permalink / raw)
  To: linux-arm-kernel

This patch add device mailbox node for Hi6220 in DT.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index 2585236..be3d962 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -249,5 +249,17 @@
 			clocks = <&ao_ctrl 27>;
 			clock-names = "apb_pclk";
 		};
+
+		mailbox: mailbox at f7510000 {
+			#mbox-cells = <1>;
+			compatible = "hisilicon,hi6220-mbox";
+			reg = <0x0 0xf7510000 0x0 0x1000>, /* IPC_S */
+			      <0x0 0x06dff800 0x0 0x0800>; /* Mailbox buffer */
+			interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
+			mbox_stub_clock: mbox_stub_clock {
+				hi6220,mbox-rx = <0 1 10>;
+				hi6220,mbox-tx = <1 0 11>;
+			};
+		};
 	};
 };
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 4/4] arm64: dts: add Hi6220's stub clock node
  2016-02-01 13:34 ` Leo Yan
@ 2016-02-01 13:34   ` Leo Yan
  -1 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2016-02-01 13:34 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Catalin Marinas, Will Deacon, Jassi Brar, Leo Yan, Arnd Bergmann,
	Wei Xu, Tyler Baker, Sudeep Holla, Chen Feng, Bintian Wang,
	devicetree, linux-kernel, linux-arm-kernel, Haojian Zhuang

Enable SRAM node and stub clock node for Hi6220, which will use mailbox
channel 0 for CPU's frequency change.

Furthermore, add the CPU clock phandle in CPU's node and using
operating-points-v2 to register operating points. So can be used by
cpufreq-dt driver.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 55 +++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index be3d962..d46820a 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -81,6 +81,11 @@
 			device_type = "cpu";
 			reg = <0x0 0x0>;
 			enable-method = "psci";
+			clocks = <&stub_clock 0>;
+			operating-points-v2 = <&cpu_opp_table>;
+			cooling-min-level = <4>;
+			cooling-max-level = <0>;
+			#cooling-cells = <2>; /* min followed by max */
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
 		};
 
@@ -89,6 +94,7 @@
 			device_type = "cpu";
 			reg = <0x0 0x1>;
 			enable-method = "psci";
+			operating-points-v2 = <&cpu_opp_table>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
 		};
 
@@ -97,6 +103,7 @@
 			device_type = "cpu";
 			reg = <0x0 0x2>;
 			enable-method = "psci";
+			operating-points-v2 = <&cpu_opp_table>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
 		};
 
@@ -105,6 +112,7 @@
 			device_type = "cpu";
 			reg = <0x0 0x3>;
 			enable-method = "psci";
+			operating-points-v2 = <&cpu_opp_table>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
 		};
 
@@ -113,6 +121,7 @@
 			device_type = "cpu";
 			reg = <0x0 0x100>;
 			enable-method = "psci";
+			operating-points-v2 = <&cpu_opp_table>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
 		};
 
@@ -121,6 +130,7 @@
 			device_type = "cpu";
 			reg = <0x0 0x101>;
 			enable-method = "psci";
+			operating-points-v2 = <&cpu_opp_table>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
 		};
 
@@ -129,6 +139,7 @@
 			device_type = "cpu";
 			reg = <0x0 0x102>;
 			enable-method = "psci";
+			operating-points-v2 = <&cpu_opp_table>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
 		};
 
@@ -137,10 +148,42 @@
 			device_type = "cpu";
 			reg = <0x0 0x103>;
 			enable-method = "psci";
+			operating-points-v2 = <&cpu_opp_table>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
 		};
 	};
 
+	cpu_opp_table: cpu_opp_table {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp00 {
+			opp-hz = /bits/ 64 <208000000>;
+			opp-microvolt = <1040000>;
+			clock-latency-ns = <500000>;
+		};
+		opp01 {
+			opp-hz = /bits/ 64 <432000000>;
+			opp-microvolt = <1040000>;
+			clock-latency-ns = <500000>;
+		};
+		opp02 {
+			opp-hz = /bits/ 64 <729000000>;
+			opp-microvolt = <1090000>;
+			clock-latency-ns = <500000>;
+		};
+		opp03 {
+			opp-hz = /bits/ 64 <960000000>;
+			opp-microvolt = <1180000>;
+			clock-latency-ns = <500000>;
+		};
+		opp04 {
+			opp-hz = /bits/ 64 <1200000000>;
+			opp-microvolt = <1330000>;
+			clock-latency-ns = <500000>;
+		};
+	};
+
 	gic: interrupt-controller@f6801000 {
 		compatible = "arm,gic-400";
 		reg = <0x0 0xf6801000 0 0x1000>, /* GICD */
@@ -168,6 +211,11 @@
 		#size-cells = <2>;
 		ranges;
 
+		sram: sram@fff80000 {
+			compatible = "hisilicon,hi6220-sramctrl", "syscon";
+			reg = <0x0 0xfff80000 0x0 0x12000>;
+		};
+
 		ao_ctrl: ao_ctrl@f7800000 {
 			compatible = "hisilicon,hi6220-aoctrl", "syscon";
 			reg = <0x0 0xf7800000 0x0 0x2000>;
@@ -193,6 +241,13 @@
 			#clock-cells = <1>;
 		};
 
+		stub_clock: stub_clock {
+			compatible = "hisilicon,hi6220-stub-clk";
+			hisilicon,hi6220-clk-sram = <&sram>;
+			#clock-cells = <1>;
+			mboxes = <&mailbox 1>;
+		};
+
 		uart0: uart@f8015000 {	/* console */
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x0 0xf8015000 0x0 0x1000>;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 4/4] arm64: dts: add Hi6220's stub clock node
@ 2016-02-01 13:34   ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2016-02-01 13:34 UTC (permalink / raw)
  To: linux-arm-kernel

Enable SRAM node and stub clock node for Hi6220, which will use mailbox
channel 0 for CPU's frequency change.

Furthermore, add the CPU clock phandle in CPU's node and using
operating-points-v2 to register operating points. So can be used by
cpufreq-dt driver.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 55 +++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index be3d962..d46820a 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -81,6 +81,11 @@
 			device_type = "cpu";
 			reg = <0x0 0x0>;
 			enable-method = "psci";
+			clocks = <&stub_clock 0>;
+			operating-points-v2 = <&cpu_opp_table>;
+			cooling-min-level = <4>;
+			cooling-max-level = <0>;
+			#cooling-cells = <2>; /* min followed by max */
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
 		};
 
@@ -89,6 +94,7 @@
 			device_type = "cpu";
 			reg = <0x0 0x1>;
 			enable-method = "psci";
+			operating-points-v2 = <&cpu_opp_table>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
 		};
 
@@ -97,6 +103,7 @@
 			device_type = "cpu";
 			reg = <0x0 0x2>;
 			enable-method = "psci";
+			operating-points-v2 = <&cpu_opp_table>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
 		};
 
@@ -105,6 +112,7 @@
 			device_type = "cpu";
 			reg = <0x0 0x3>;
 			enable-method = "psci";
+			operating-points-v2 = <&cpu_opp_table>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
 		};
 
@@ -113,6 +121,7 @@
 			device_type = "cpu";
 			reg = <0x0 0x100>;
 			enable-method = "psci";
+			operating-points-v2 = <&cpu_opp_table>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
 		};
 
@@ -121,6 +130,7 @@
 			device_type = "cpu";
 			reg = <0x0 0x101>;
 			enable-method = "psci";
+			operating-points-v2 = <&cpu_opp_table>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
 		};
 
@@ -129,6 +139,7 @@
 			device_type = "cpu";
 			reg = <0x0 0x102>;
 			enable-method = "psci";
+			operating-points-v2 = <&cpu_opp_table>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
 		};
 
@@ -137,10 +148,42 @@
 			device_type = "cpu";
 			reg = <0x0 0x103>;
 			enable-method = "psci";
+			operating-points-v2 = <&cpu_opp_table>;
 			cpu-idle-states = <&CPU_SLEEP &CLUSTER_SLEEP>;
 		};
 	};
 
+	cpu_opp_table: cpu_opp_table {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp00 {
+			opp-hz = /bits/ 64 <208000000>;
+			opp-microvolt = <1040000>;
+			clock-latency-ns = <500000>;
+		};
+		opp01 {
+			opp-hz = /bits/ 64 <432000000>;
+			opp-microvolt = <1040000>;
+			clock-latency-ns = <500000>;
+		};
+		opp02 {
+			opp-hz = /bits/ 64 <729000000>;
+			opp-microvolt = <1090000>;
+			clock-latency-ns = <500000>;
+		};
+		opp03 {
+			opp-hz = /bits/ 64 <960000000>;
+			opp-microvolt = <1180000>;
+			clock-latency-ns = <500000>;
+		};
+		opp04 {
+			opp-hz = /bits/ 64 <1200000000>;
+			opp-microvolt = <1330000>;
+			clock-latency-ns = <500000>;
+		};
+	};
+
 	gic: interrupt-controller at f6801000 {
 		compatible = "arm,gic-400";
 		reg = <0x0 0xf6801000 0 0x1000>, /* GICD */
@@ -168,6 +211,11 @@
 		#size-cells = <2>;
 		ranges;
 
+		sram: sram at fff80000 {
+			compatible = "hisilicon,hi6220-sramctrl", "syscon";
+			reg = <0x0 0xfff80000 0x0 0x12000>;
+		};
+
 		ao_ctrl: ao_ctrl at f7800000 {
 			compatible = "hisilicon,hi6220-aoctrl", "syscon";
 			reg = <0x0 0xf7800000 0x0 0x2000>;
@@ -193,6 +241,13 @@
 			#clock-cells = <1>;
 		};
 
+		stub_clock: stub_clock {
+			compatible = "hisilicon,hi6220-stub-clk";
+			hisilicon,hi6220-clk-sram = <&sram>;
+			#clock-cells = <1>;
+			mboxes = <&mailbox 1>;
+		};
+
 		uart0: uart at f8015000 {	/* console */
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x0 0xf8015000 0x0 0x1000>;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 1/4] dt-bindings: mailbox: Document Hi6220 mailbox driver
@ 2016-02-01 14:08     ` Rob Herring
  0 siblings, 0 replies; 23+ messages in thread
From: Rob Herring @ 2016-02-01 14:08 UTC (permalink / raw)
  To: Leo Yan
  Cc: Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Catalin Marinas, Will Deacon, Jassi Brar, Arnd Bergmann, Wei Xu,
	Tyler Baker, Sudeep Holla, Chen Feng, Bintian Wang, devicetree,
	linux-kernel, linux-arm-kernel, Haojian Zhuang

On Mon, Feb 01, 2016 at 09:34:44PM +0800, Leo Yan wrote:
> Document DT binding for Hisilicon Hi6220 mailbox driver.
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  .../bindings/mailbox/hisilicon,hi6220-mailbox.txt  | 90 ++++++++++++++++++++++
>  1 file changed, 90 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
> new file mode 100644
> index 0000000..96e6acc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
> @@ -0,0 +1,90 @@
> +Hisilicon Hi6220 Mailbox Driver
> +===============================
> +
> +Hisilicon Hi6220 mailbox supports up to 32 channels. Each channel
> +is unidirectional with a maximum message size of 8 words. I/O is
> +performed using register access (there is no DMA) and the cell
> +raises an interrupt when messages are received.
> +
> +Mailbox Device Node:
> +====================
> +
> +Required properties:
> +--------------------
> +- compatible:		Shall be "hisilicon,hi6220-mbox"
> +- reg:			Contains the mailbox register address range (base
> +			address and length); the first item is for IPC
> +			registers, the second item is shared buffer for
> +			slots.
> +- #mbox-cells		Common mailbox binding property to identify the number
> +			of cells required for the mailbox specifier. Should be 1.
> +- interrupts:		Contains the interrupt information for the mailbox
> +			device. The format is dependent on which interrupt
> +			controller the SoCs use.
> +
> +Optional Properties:
> +--------------------
> +- hi6220,mbox-tx-noirq: Flag to allow the client user of this mailbox driver
> +			to send messages without triggering a TX completion
> +			interrupt.

I don't think this belongs in DT. This should be a flag the client 
driver sets when it sends messages.

> +
> +Child Nodes:
> +============
> +A child node is used for representing the actual sub-mailbox device that is
> +used for the communication between the host processor and a remote processor.
> +Each child node should have a unique node name across all the different
> +mailbox device nodes.
> +
> +Required properties:
> +--------------------
> +- hi6220,mbox-tx:	sub-mailbox descriptor property defining Tx channel
> +- hi6220,mbox-rx:	sub-mailbox descriptor property defining Rx channel
> +
> +Sub-mailbox Descriptor Data
> +---------------------------
> +Each of the above hi6220,mbox-tx and hi6220,mbox-rx properties should have 3
> +cells of data that represent the following:
> +    Cell #1 (slot_id) - mailbox slot id used either for transmitting
> +                        (hi6220,mbox-tx) or for receiving (hi6220,mbox-rx)
> +    Cell #2 (dst_irq) - irq identifier index number which used by MCU.
> +    Cell #3 (ack_irq) - irq identifier index number with generating a tx/rx
> +                        interrupt to application processor, mailbox driver
> +                        used this id to acknowledge interrupt.
> +
> +Example:
> +--------
> +
> +	mailbox: mailbox@F7510000 {
> +		#mbox-cells = <1>;
> +		compatible = "hisilicon,hi6220-mbox";
> +		reg = <0x0 0xF7510000 0x0 0x1000>, /* IPC_S */
> +		      <0x0 0x06DFF800 0x0 0x0800>; /* Mailbox */
> +		interrupt-parent = <&gic>;
> +		interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
> +		mbox_stub_clock: mbox_stub_clock {
> +			hi6220,mbox-rx = <0 1 10>;
> +			hi6220,mbox-tx = <1 0 11>;
> +		};
> +	};
> +
> +
> +Mailbox client
> +===============
> +
> +"mboxes" and the optional "mbox-names" (please see
> +Documentation/devicetree/bindings/mailbox/mailbox.txt for details). Each value
> +of the mboxes property should contain a phandle to the mailbox controller
> +device node and second argument is the channel index. It must be 0 (hardware

0? But the example has 1.

> +support only one channel). The equivalent "mbox-names" property value can be
> +used to give a name to the communication channel to be used by the client user.
> +
> +Example:
> +--------
> +
> +	stub_clock: stub_clock {
> +		compatible = "hisilicon,hi6220-stub-clk";
> +		hisilicon,hi6220-clk-sram = <&sram>;
> +		#clock-cells = <1>;
> +		mbox-names = "mbox-tx";
> +		mboxes = <&mailbox 1>;
> +	};
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 1/4] dt-bindings: mailbox: Document Hi6220 mailbox driver
@ 2016-02-01 14:08     ` Rob Herring
  0 siblings, 0 replies; 23+ messages in thread
From: Rob Herring @ 2016-02-01 14:08 UTC (permalink / raw)
  To: Leo Yan
  Cc: Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Catalin Marinas, Will Deacon, Jassi Brar, Arnd Bergmann, Wei Xu,
	Tyler Baker, Sudeep Holla, Chen Feng, Bintian Wang,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Haojian Zhuang

On Mon, Feb 01, 2016 at 09:34:44PM +0800, Leo Yan wrote:
> Document DT binding for Hisilicon Hi6220 mailbox driver.
> 
> Signed-off-by: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  .../bindings/mailbox/hisilicon,hi6220-mailbox.txt  | 90 ++++++++++++++++++++++
>  1 file changed, 90 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
> new file mode 100644
> index 0000000..96e6acc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
> @@ -0,0 +1,90 @@
> +Hisilicon Hi6220 Mailbox Driver
> +===============================
> +
> +Hisilicon Hi6220 mailbox supports up to 32 channels. Each channel
> +is unidirectional with a maximum message size of 8 words. I/O is
> +performed using register access (there is no DMA) and the cell
> +raises an interrupt when messages are received.
> +
> +Mailbox Device Node:
> +====================
> +
> +Required properties:
> +--------------------
> +- compatible:		Shall be "hisilicon,hi6220-mbox"
> +- reg:			Contains the mailbox register address range (base
> +			address and length); the first item is for IPC
> +			registers, the second item is shared buffer for
> +			slots.
> +- #mbox-cells		Common mailbox binding property to identify the number
> +			of cells required for the mailbox specifier. Should be 1.
> +- interrupts:		Contains the interrupt information for the mailbox
> +			device. The format is dependent on which interrupt
> +			controller the SoCs use.
> +
> +Optional Properties:
> +--------------------
> +- hi6220,mbox-tx-noirq: Flag to allow the client user of this mailbox driver
> +			to send messages without triggering a TX completion
> +			interrupt.

I don't think this belongs in DT. This should be a flag the client 
driver sets when it sends messages.

> +
> +Child Nodes:
> +============
> +A child node is used for representing the actual sub-mailbox device that is
> +used for the communication between the host processor and a remote processor.
> +Each child node should have a unique node name across all the different
> +mailbox device nodes.
> +
> +Required properties:
> +--------------------
> +- hi6220,mbox-tx:	sub-mailbox descriptor property defining Tx channel
> +- hi6220,mbox-rx:	sub-mailbox descriptor property defining Rx channel
> +
> +Sub-mailbox Descriptor Data
> +---------------------------
> +Each of the above hi6220,mbox-tx and hi6220,mbox-rx properties should have 3
> +cells of data that represent the following:
> +    Cell #1 (slot_id) - mailbox slot id used either for transmitting
> +                        (hi6220,mbox-tx) or for receiving (hi6220,mbox-rx)
> +    Cell #2 (dst_irq) - irq identifier index number which used by MCU.
> +    Cell #3 (ack_irq) - irq identifier index number with generating a tx/rx
> +                        interrupt to application processor, mailbox driver
> +                        used this id to acknowledge interrupt.
> +
> +Example:
> +--------
> +
> +	mailbox: mailbox@F7510000 {
> +		#mbox-cells = <1>;
> +		compatible = "hisilicon,hi6220-mbox";
> +		reg = <0x0 0xF7510000 0x0 0x1000>, /* IPC_S */
> +		      <0x0 0x06DFF800 0x0 0x0800>; /* Mailbox */
> +		interrupt-parent = <&gic>;
> +		interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
> +		mbox_stub_clock: mbox_stub_clock {
> +			hi6220,mbox-rx = <0 1 10>;
> +			hi6220,mbox-tx = <1 0 11>;
> +		};
> +	};
> +
> +
> +Mailbox client
> +===============
> +
> +"mboxes" and the optional "mbox-names" (please see
> +Documentation/devicetree/bindings/mailbox/mailbox.txt for details). Each value
> +of the mboxes property should contain a phandle to the mailbox controller
> +device node and second argument is the channel index. It must be 0 (hardware

0? But the example has 1.

> +support only one channel). The equivalent "mbox-names" property value can be
> +used to give a name to the communication channel to be used by the client user.
> +
> +Example:
> +--------
> +
> +	stub_clock: stub_clock {
> +		compatible = "hisilicon,hi6220-stub-clk";
> +		hisilicon,hi6220-clk-sram = <&sram>;
> +		#clock-cells = <1>;
> +		mbox-names = "mbox-tx";
> +		mboxes = <&mailbox 1>;
> +	};
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v5 1/4] dt-bindings: mailbox: Document Hi6220 mailbox driver
@ 2016-02-01 14:08     ` Rob Herring
  0 siblings, 0 replies; 23+ messages in thread
From: Rob Herring @ 2016-02-01 14:08 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 01, 2016 at 09:34:44PM +0800, Leo Yan wrote:
> Document DT binding for Hisilicon Hi6220 mailbox driver.
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  .../bindings/mailbox/hisilicon,hi6220-mailbox.txt  | 90 ++++++++++++++++++++++
>  1 file changed, 90 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
> new file mode 100644
> index 0000000..96e6acc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
> @@ -0,0 +1,90 @@
> +Hisilicon Hi6220 Mailbox Driver
> +===============================
> +
> +Hisilicon Hi6220 mailbox supports up to 32 channels. Each channel
> +is unidirectional with a maximum message size of 8 words. I/O is
> +performed using register access (there is no DMA) and the cell
> +raises an interrupt when messages are received.
> +
> +Mailbox Device Node:
> +====================
> +
> +Required properties:
> +--------------------
> +- compatible:		Shall be "hisilicon,hi6220-mbox"
> +- reg:			Contains the mailbox register address range (base
> +			address and length); the first item is for IPC
> +			registers, the second item is shared buffer for
> +			slots.
> +- #mbox-cells		Common mailbox binding property to identify the number
> +			of cells required for the mailbox specifier. Should be 1.
> +- interrupts:		Contains the interrupt information for the mailbox
> +			device. The format is dependent on which interrupt
> +			controller the SoCs use.
> +
> +Optional Properties:
> +--------------------
> +- hi6220,mbox-tx-noirq: Flag to allow the client user of this mailbox driver
> +			to send messages without triggering a TX completion
> +			interrupt.

I don't think this belongs in DT. This should be a flag the client 
driver sets when it sends messages.

> +
> +Child Nodes:
> +============
> +A child node is used for representing the actual sub-mailbox device that is
> +used for the communication between the host processor and a remote processor.
> +Each child node should have a unique node name across all the different
> +mailbox device nodes.
> +
> +Required properties:
> +--------------------
> +- hi6220,mbox-tx:	sub-mailbox descriptor property defining Tx channel
> +- hi6220,mbox-rx:	sub-mailbox descriptor property defining Rx channel
> +
> +Sub-mailbox Descriptor Data
> +---------------------------
> +Each of the above hi6220,mbox-tx and hi6220,mbox-rx properties should have 3
> +cells of data that represent the following:
> +    Cell #1 (slot_id) - mailbox slot id used either for transmitting
> +                        (hi6220,mbox-tx) or for receiving (hi6220,mbox-rx)
> +    Cell #2 (dst_irq) - irq identifier index number which used by MCU.
> +    Cell #3 (ack_irq) - irq identifier index number with generating a tx/rx
> +                        interrupt to application processor, mailbox driver
> +                        used this id to acknowledge interrupt.
> +
> +Example:
> +--------
> +
> +	mailbox: mailbox at F7510000 {
> +		#mbox-cells = <1>;
> +		compatible = "hisilicon,hi6220-mbox";
> +		reg = <0x0 0xF7510000 0x0 0x1000>, /* IPC_S */
> +		      <0x0 0x06DFF800 0x0 0x0800>; /* Mailbox */
> +		interrupt-parent = <&gic>;
> +		interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
> +		mbox_stub_clock: mbox_stub_clock {
> +			hi6220,mbox-rx = <0 1 10>;
> +			hi6220,mbox-tx = <1 0 11>;
> +		};
> +	};
> +
> +
> +Mailbox client
> +===============
> +
> +"mboxes" and the optional "mbox-names" (please see
> +Documentation/devicetree/bindings/mailbox/mailbox.txt for details). Each value
> +of the mboxes property should contain a phandle to the mailbox controller
> +device node and second argument is the channel index. It must be 0 (hardware

0? But the example has 1.

> +support only one channel). The equivalent "mbox-names" property value can be
> +used to give a name to the communication channel to be used by the client user.
> +
> +Example:
> +--------
> +
> +	stub_clock: stub_clock {
> +		compatible = "hisilicon,hi6220-stub-clk";
> +		hisilicon,hi6220-clk-sram = <&sram>;
> +		#clock-cells = <1>;
> +		mbox-names = "mbox-tx";
> +		mboxes = <&mailbox 1>;
> +	};
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 1/4] dt-bindings: mailbox: Document Hi6220 mailbox driver
  2016-02-01 14:08     ` Rob Herring
@ 2016-02-01 15:23       ` Leo Yan
  -1 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2016-02-01 15:23 UTC (permalink / raw)
  To: Rob Herring
  Cc: Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Catalin Marinas, Will Deacon, Jassi Brar, Arnd Bergmann, Wei Xu,
	Tyler Baker, Sudeep Holla, Chen Feng, Bintian Wang, devicetree,
	linux-kernel, linux-arm-kernel, Haojian Zhuang

Hi Rob,

Thanks for reviewing, please see below inline comments.

On Mon, Feb 01, 2016 at 08:08:28AM -0600, Rob Herring wrote:
> On Mon, Feb 01, 2016 at 09:34:44PM +0800, Leo Yan wrote:
> > Document DT binding for Hisilicon Hi6220 mailbox driver.
> > 
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> >  .../bindings/mailbox/hisilicon,hi6220-mailbox.txt  | 90 ++++++++++++++++++++++
> >  1 file changed, 90 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
> > new file mode 100644
> > index 0000000..96e6acc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
> > @@ -0,0 +1,90 @@
> > +Hisilicon Hi6220 Mailbox Driver
> > +===============================
> > +
> > +Hisilicon Hi6220 mailbox supports up to 32 channels. Each channel
> > +is unidirectional with a maximum message size of 8 words. I/O is
> > +performed using register access (there is no DMA) and the cell
> > +raises an interrupt when messages are received.
> > +
> > +Mailbox Device Node:
> > +====================
> > +
> > +Required properties:
> > +--------------------
> > +- compatible:		Shall be "hisilicon,hi6220-mbox"
> > +- reg:			Contains the mailbox register address range (base
> > +			address and length); the first item is for IPC
> > +			registers, the second item is shared buffer for
> > +			slots.
> > +- #mbox-cells		Common mailbox binding property to identify the number
> > +			of cells required for the mailbox specifier. Should be 1.
> > +- interrupts:		Contains the interrupt information for the mailbox
> > +			device. The format is dependent on which interrupt
> > +			controller the SoCs use.
> > +
> > +Optional Properties:
> > +--------------------
> > +- hi6220,mbox-tx-noirq: Flag to allow the client user of this mailbox driver
> > +			to send messages without triggering a TX completion
> > +			interrupt.
> 
> I don't think this belongs in DT. This should be a flag the client 
> driver sets when it sends messages.

The client driver can set "tx_block = true" so use this flag indicates
the client thread should be blocked until data is transmitted.

But low level mailbox driver can use two method to support "tx_block"
mode:
- One method is to avoid using interrupt and mailbox framework will
  poll with mailbox's idle flag which is set by remote processor
  automatically;
- Another method is to use interrupt to notify data has been
  transmitted and interrupt handler will call completion function to
  wake up blocked client thread;

So this flag is to distinguish these two different hardware mechanism.
Do you think this is make sense or have other suggestion?

> > +
> > +Child Nodes:
> > +============
> > +A child node is used for representing the actual sub-mailbox device that is
> > +used for the communication between the host processor and a remote processor.
> > +Each child node should have a unique node name across all the different
> > +mailbox device nodes.
> > +
> > +Required properties:
> > +--------------------
> > +- hi6220,mbox-tx:	sub-mailbox descriptor property defining Tx channel
> > +- hi6220,mbox-rx:	sub-mailbox descriptor property defining Rx channel
> > +
> > +Sub-mailbox Descriptor Data
> > +---------------------------
> > +Each of the above hi6220,mbox-tx and hi6220,mbox-rx properties should have 3
> > +cells of data that represent the following:
> > +    Cell #1 (slot_id) - mailbox slot id used either for transmitting
> > +                        (hi6220,mbox-tx) or for receiving (hi6220,mbox-rx)
> > +    Cell #2 (dst_irq) - irq identifier index number which used by MCU.
> > +    Cell #3 (ack_irq) - irq identifier index number with generating a tx/rx
> > +                        interrupt to application processor, mailbox driver
> > +                        used this id to acknowledge interrupt.
> > +
> > +Example:
> > +--------
> > +
> > +	mailbox: mailbox@F7510000 {
> > +		#mbox-cells = <1>;
> > +		compatible = "hisilicon,hi6220-mbox";
> > +		reg = <0x0 0xF7510000 0x0 0x1000>, /* IPC_S */
> > +		      <0x0 0x06DFF800 0x0 0x0800>; /* Mailbox */
> > +		interrupt-parent = <&gic>;
> > +		interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
> > +		mbox_stub_clock: mbox_stub_clock {
> > +			hi6220,mbox-rx = <0 1 10>;
> > +			hi6220,mbox-tx = <1 0 11>;
> > +		};
> > +	};
> > +
> > +
> > +Mailbox client
> > +===============
> > +
> > +"mboxes" and the optional "mbox-names" (please see
> > +Documentation/devicetree/bindings/mailbox/mailbox.txt for details). Each value
> > +of the mboxes property should contain a phandle to the mailbox controller
> > +device node and second argument is the channel index. It must be 0 (hardware
> 
> 0? But the example has 1.

Will fix.

Thanks,
Leo Yan

> > +support only one channel). The equivalent "mbox-names" property value can be
> > +used to give a name to the communication channel to be used by the client user.
> > +
> > +Example:
> > +--------
> > +
> > +	stub_clock: stub_clock {
> > +		compatible = "hisilicon,hi6220-stub-clk";
> > +		hisilicon,hi6220-clk-sram = <&sram>;
> > +		#clock-cells = <1>;
> > +		mbox-names = "mbox-tx";
> > +		mboxes = <&mailbox 1>;
> > +	};
> > -- 
> > 1.9.1
> > 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v5 1/4] dt-bindings: mailbox: Document Hi6220 mailbox driver
@ 2016-02-01 15:23       ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2016-02-01 15:23 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Rob,

Thanks for reviewing, please see below inline comments.

On Mon, Feb 01, 2016 at 08:08:28AM -0600, Rob Herring wrote:
> On Mon, Feb 01, 2016 at 09:34:44PM +0800, Leo Yan wrote:
> > Document DT binding for Hisilicon Hi6220 mailbox driver.
> > 
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> >  .../bindings/mailbox/hisilicon,hi6220-mailbox.txt  | 90 ++++++++++++++++++++++
> >  1 file changed, 90 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
> > new file mode 100644
> > index 0000000..96e6acc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
> > @@ -0,0 +1,90 @@
> > +Hisilicon Hi6220 Mailbox Driver
> > +===============================
> > +
> > +Hisilicon Hi6220 mailbox supports up to 32 channels. Each channel
> > +is unidirectional with a maximum message size of 8 words. I/O is
> > +performed using register access (there is no DMA) and the cell
> > +raises an interrupt when messages are received.
> > +
> > +Mailbox Device Node:
> > +====================
> > +
> > +Required properties:
> > +--------------------
> > +- compatible:		Shall be "hisilicon,hi6220-mbox"
> > +- reg:			Contains the mailbox register address range (base
> > +			address and length); the first item is for IPC
> > +			registers, the second item is shared buffer for
> > +			slots.
> > +- #mbox-cells		Common mailbox binding property to identify the number
> > +			of cells required for the mailbox specifier. Should be 1.
> > +- interrupts:		Contains the interrupt information for the mailbox
> > +			device. The format is dependent on which interrupt
> > +			controller the SoCs use.
> > +
> > +Optional Properties:
> > +--------------------
> > +- hi6220,mbox-tx-noirq: Flag to allow the client user of this mailbox driver
> > +			to send messages without triggering a TX completion
> > +			interrupt.
> 
> I don't think this belongs in DT. This should be a flag the client 
> driver sets when it sends messages.

The client driver can set "tx_block = true" so use this flag indicates
the client thread should be blocked until data is transmitted.

But low level mailbox driver can use two method to support "tx_block"
mode:
- One method is to avoid using interrupt and mailbox framework will
  poll with mailbox's idle flag which is set by remote processor
  automatically;
- Another method is to use interrupt to notify data has been
  transmitted and interrupt handler will call completion function to
  wake up blocked client thread;

So this flag is to distinguish these two different hardware mechanism.
Do you think this is make sense or have other suggestion?

> > +
> > +Child Nodes:
> > +============
> > +A child node is used for representing the actual sub-mailbox device that is
> > +used for the communication between the host processor and a remote processor.
> > +Each child node should have a unique node name across all the different
> > +mailbox device nodes.
> > +
> > +Required properties:
> > +--------------------
> > +- hi6220,mbox-tx:	sub-mailbox descriptor property defining Tx channel
> > +- hi6220,mbox-rx:	sub-mailbox descriptor property defining Rx channel
> > +
> > +Sub-mailbox Descriptor Data
> > +---------------------------
> > +Each of the above hi6220,mbox-tx and hi6220,mbox-rx properties should have 3
> > +cells of data that represent the following:
> > +    Cell #1 (slot_id) - mailbox slot id used either for transmitting
> > +                        (hi6220,mbox-tx) or for receiving (hi6220,mbox-rx)
> > +    Cell #2 (dst_irq) - irq identifier index number which used by MCU.
> > +    Cell #3 (ack_irq) - irq identifier index number with generating a tx/rx
> > +                        interrupt to application processor, mailbox driver
> > +                        used this id to acknowledge interrupt.
> > +
> > +Example:
> > +--------
> > +
> > +	mailbox: mailbox at F7510000 {
> > +		#mbox-cells = <1>;
> > +		compatible = "hisilicon,hi6220-mbox";
> > +		reg = <0x0 0xF7510000 0x0 0x1000>, /* IPC_S */
> > +		      <0x0 0x06DFF800 0x0 0x0800>; /* Mailbox */
> > +		interrupt-parent = <&gic>;
> > +		interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
> > +		mbox_stub_clock: mbox_stub_clock {
> > +			hi6220,mbox-rx = <0 1 10>;
> > +			hi6220,mbox-tx = <1 0 11>;
> > +		};
> > +	};
> > +
> > +
> > +Mailbox client
> > +===============
> > +
> > +"mboxes" and the optional "mbox-names" (please see
> > +Documentation/devicetree/bindings/mailbox/mailbox.txt for details). Each value
> > +of the mboxes property should contain a phandle to the mailbox controller
> > +device node and second argument is the channel index. It must be 0 (hardware
> 
> 0? But the example has 1.

Will fix.

Thanks,
Leo Yan

> > +support only one channel). The equivalent "mbox-names" property value can be
> > +used to give a name to the communication channel to be used by the client user.
> > +
> > +Example:
> > +--------
> > +
> > +	stub_clock: stub_clock {
> > +		compatible = "hisilicon,hi6220-stub-clk";
> > +		hisilicon,hi6220-clk-sram = <&sram>;
> > +		#clock-cells = <1>;
> > +		mbox-names = "mbox-tx";
> > +		mboxes = <&mailbox 1>;
> > +	};
> > -- 
> > 1.9.1
> > 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 1/4] dt-bindings: mailbox: Document Hi6220 mailbox driver
  2016-02-01 15:23       ` Leo Yan
  (?)
@ 2016-02-01 16:16         ` Jassi Brar
  -1 siblings, 0 replies; 23+ messages in thread
From: Jassi Brar @ 2016-02-01 16:16 UTC (permalink / raw)
  To: Leo Yan
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Catalin Marinas, Will Deacon, Arnd Bergmann, Wei Xu, Tyler Baker,
	Sudeep Holla, Chen Feng, Bintian Wang, Devicetree List,
	Linux Kernel Mailing List, linux-arm-kernel, Haojian Zhuang

On Mon, Feb 1, 2016 at 8:53 PM, Leo Yan <leo.yan@linaro.org> wrote:
> Hi Rob,
>
> Thanks for reviewing, please see below inline comments.
>
> On Mon, Feb 01, 2016 at 08:08:28AM -0600, Rob Herring wrote:
>> On Mon, Feb 01, 2016 at 09:34:44PM +0800, Leo Yan wrote:
>> > Document DT binding for Hisilicon Hi6220 mailbox driver.
>> >
>> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
>> > ---
>> >  .../bindings/mailbox/hisilicon,hi6220-mailbox.txt  | 90 ++++++++++++++++++++++
>> >  1 file changed, 90 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
>> > new file mode 100644
>> > index 0000000..96e6acc
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
>> > @@ -0,0 +1,90 @@
>> > +Hisilicon Hi6220 Mailbox Driver
>> > +===============================
>> > +
>> > +Hisilicon Hi6220 mailbox supports up to 32 channels. Each channel
>> > +is unidirectional with a maximum message size of 8 words. I/O is
>> > +performed using register access (there is no DMA) and the cell
>> > +raises an interrupt when messages are received.
>> > +
>> > +Mailbox Device Node:
>> > +====================
>> > +
>> > +Required properties:
>> > +--------------------
>> > +- compatible:              Shall be "hisilicon,hi6220-mbox"
>> > +- reg:                     Contains the mailbox register address range (base
>> > +                   address and length); the first item is for IPC
>> > +                   registers, the second item is shared buffer for
>> > +                   slots.
>> > +- #mbox-cells              Common mailbox binding property to identify the number
>> > +                   of cells required for the mailbox specifier. Should be 1.
>> > +- interrupts:              Contains the interrupt information for the mailbox
>> > +                   device. The format is dependent on which interrupt
>> > +                   controller the SoCs use.
>> > +
>> > +Optional Properties:
>> > +--------------------
>> > +- hi6220,mbox-tx-noirq: Flag to allow the client user of this mailbox driver
>> > +                   to send messages without triggering a TX completion
>> > +                   interrupt.
>>
>> I don't think this belongs in DT. This should be a flag the client
>> driver sets when it sends messages.
>
> The client driver can set "tx_block = true" so use this flag indicates
> the client thread should be blocked until data is transmitted.
>
Yes, but the 'tx_block' feature is provided by the core. The
controller driver should not need to know how the client works.

> But low level mailbox driver can use two method to support "tx_block"
> mode:
>
No, as I said, provider shouldn't care about consumers..

> - One method is to avoid using interrupt and mailbox framework will
>   poll with mailbox's idle flag which is set by remote processor
>   automatically;
> - Another method is to use interrupt to notify data has been
>   transmitted and interrupt handler will call completion function to
>   wake up blocked client thread;
>
If it is possible to have either 'idle flag set' or irq generated (not
both) by the remote, then you may sell the hi6220,mbox-tx-noirq
property as a "f/w feature" ... but still not for the sake of
tx_block.


>> > +
>> > +Child Nodes:
>> > +============
>> > +A child node is used for representing the actual sub-mailbox device that is
>> > +used for the communication between the host processor and a remote processor.
>> > +Each child node should have a unique node name across all the different
>> > +mailbox device nodes.
>> > +
>> > +Required properties:
>> > +--------------------
>> > +- hi6220,mbox-tx:  sub-mailbox descriptor property defining Tx channel
>> > +- hi6220,mbox-rx:  sub-mailbox descriptor property defining Rx channel
>> > +
>> > +Sub-mailbox Descriptor Data
>> > +---------------------------
>> > +Each of the above hi6220,mbox-tx and hi6220,mbox-rx properties should have 3
>> > +cells of data that represent the following:
>> > +    Cell #1 (slot_id) - mailbox slot id used either for transmitting
>> > +                        (hi6220,mbox-tx) or for receiving (hi6220,mbox-rx)
>> > +    Cell #2 (dst_irq) - irq identifier index number which used by MCU.
>> > +    Cell #3 (ack_irq) - irq identifier index number with generating a tx/rx
>> > +                        interrupt to application processor, mailbox driver
>> > +                        used this id to acknowledge interrupt.
>> > +
>> > +Example:
>> > +--------
>> > +
>> > +   mailbox: mailbox@F7510000 {
>> > +           #mbox-cells = <1>;
>> > +           compatible = "hisilicon,hi6220-mbox";
>> > +           reg = <0x0 0xF7510000 0x0 0x1000>, /* IPC_S */
>> > +                 <0x0 0x06DFF800 0x0 0x0800>; /* Mailbox */
>> > +           interrupt-parent = <&gic>;
>> > +           interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
>> > +           mbox_stub_clock: mbox_stub_clock {
>> > +                   hi6220,mbox-rx = <0 1 10>;
>> > +                   hi6220,mbox-tx = <1 0 11>;
>
This looks like meant for the client node...
         mbox-names = "mbox-tx", "mbox-rx";
         mboxes = <&mailbox 1 0 11>, <&mailbox 0 1 10>;

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 1/4] dt-bindings: mailbox: Document Hi6220 mailbox driver
@ 2016-02-01 16:16         ` Jassi Brar
  0 siblings, 0 replies; 23+ messages in thread
From: Jassi Brar @ 2016-02-01 16:16 UTC (permalink / raw)
  To: Leo Yan
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Catalin Marinas, Will Deacon, Arnd Bergmann, Wei Xu, Tyler Baker,
	Sudeep Holla, Chen Feng, Bintian Wang, Devicetree List,
	Linux Kernel Mailing List,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Haojian Zhuang

On Mon, Feb 1, 2016 at 8:53 PM, Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> Hi Rob,
>
> Thanks for reviewing, please see below inline comments.
>
> On Mon, Feb 01, 2016 at 08:08:28AM -0600, Rob Herring wrote:
>> On Mon, Feb 01, 2016 at 09:34:44PM +0800, Leo Yan wrote:
>> > Document DT binding for Hisilicon Hi6220 mailbox driver.
>> >
>> > Signed-off-by: Leo Yan <leo.yan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> > ---
>> >  .../bindings/mailbox/hisilicon,hi6220-mailbox.txt  | 90 ++++++++++++++++++++++
>> >  1 file changed, 90 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
>> > new file mode 100644
>> > index 0000000..96e6acc
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
>> > @@ -0,0 +1,90 @@
>> > +Hisilicon Hi6220 Mailbox Driver
>> > +===============================
>> > +
>> > +Hisilicon Hi6220 mailbox supports up to 32 channels. Each channel
>> > +is unidirectional with a maximum message size of 8 words. I/O is
>> > +performed using register access (there is no DMA) and the cell
>> > +raises an interrupt when messages are received.
>> > +
>> > +Mailbox Device Node:
>> > +====================
>> > +
>> > +Required properties:
>> > +--------------------
>> > +- compatible:              Shall be "hisilicon,hi6220-mbox"
>> > +- reg:                     Contains the mailbox register address range (base
>> > +                   address and length); the first item is for IPC
>> > +                   registers, the second item is shared buffer for
>> > +                   slots.
>> > +- #mbox-cells              Common mailbox binding property to identify the number
>> > +                   of cells required for the mailbox specifier. Should be 1.
>> > +- interrupts:              Contains the interrupt information for the mailbox
>> > +                   device. The format is dependent on which interrupt
>> > +                   controller the SoCs use.
>> > +
>> > +Optional Properties:
>> > +--------------------
>> > +- hi6220,mbox-tx-noirq: Flag to allow the client user of this mailbox driver
>> > +                   to send messages without triggering a TX completion
>> > +                   interrupt.
>>
>> I don't think this belongs in DT. This should be a flag the client
>> driver sets when it sends messages.
>
> The client driver can set "tx_block = true" so use this flag indicates
> the client thread should be blocked until data is transmitted.
>
Yes, but the 'tx_block' feature is provided by the core. The
controller driver should not need to know how the client works.

> But low level mailbox driver can use two method to support "tx_block"
> mode:
>
No, as I said, provider shouldn't care about consumers..

> - One method is to avoid using interrupt and mailbox framework will
>   poll with mailbox's idle flag which is set by remote processor
>   automatically;
> - Another method is to use interrupt to notify data has been
>   transmitted and interrupt handler will call completion function to
>   wake up blocked client thread;
>
If it is possible to have either 'idle flag set' or irq generated (not
both) by the remote, then you may sell the hi6220,mbox-tx-noirq
property as a "f/w feature" ... but still not for the sake of
tx_block.


>> > +
>> > +Child Nodes:
>> > +============
>> > +A child node is used for representing the actual sub-mailbox device that is
>> > +used for the communication between the host processor and a remote processor.
>> > +Each child node should have a unique node name across all the different
>> > +mailbox device nodes.
>> > +
>> > +Required properties:
>> > +--------------------
>> > +- hi6220,mbox-tx:  sub-mailbox descriptor property defining Tx channel
>> > +- hi6220,mbox-rx:  sub-mailbox descriptor property defining Rx channel
>> > +
>> > +Sub-mailbox Descriptor Data
>> > +---------------------------
>> > +Each of the above hi6220,mbox-tx and hi6220,mbox-rx properties should have 3
>> > +cells of data that represent the following:
>> > +    Cell #1 (slot_id) - mailbox slot id used either for transmitting
>> > +                        (hi6220,mbox-tx) or for receiving (hi6220,mbox-rx)
>> > +    Cell #2 (dst_irq) - irq identifier index number which used by MCU.
>> > +    Cell #3 (ack_irq) - irq identifier index number with generating a tx/rx
>> > +                        interrupt to application processor, mailbox driver
>> > +                        used this id to acknowledge interrupt.
>> > +
>> > +Example:
>> > +--------
>> > +
>> > +   mailbox: mailbox@F7510000 {
>> > +           #mbox-cells = <1>;
>> > +           compatible = "hisilicon,hi6220-mbox";
>> > +           reg = <0x0 0xF7510000 0x0 0x1000>, /* IPC_S */
>> > +                 <0x0 0x06DFF800 0x0 0x0800>; /* Mailbox */
>> > +           interrupt-parent = <&gic>;
>> > +           interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
>> > +           mbox_stub_clock: mbox_stub_clock {
>> > +                   hi6220,mbox-rx = <0 1 10>;
>> > +                   hi6220,mbox-tx = <1 0 11>;
>
This looks like meant for the client node...
         mbox-names = "mbox-tx", "mbox-rx";
         mboxes = <&mailbox 1 0 11>, <&mailbox 0 1 10>;
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v5 1/4] dt-bindings: mailbox: Document Hi6220 mailbox driver
@ 2016-02-01 16:16         ` Jassi Brar
  0 siblings, 0 replies; 23+ messages in thread
From: Jassi Brar @ 2016-02-01 16:16 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 1, 2016 at 8:53 PM, Leo Yan <leo.yan@linaro.org> wrote:
> Hi Rob,
>
> Thanks for reviewing, please see below inline comments.
>
> On Mon, Feb 01, 2016 at 08:08:28AM -0600, Rob Herring wrote:
>> On Mon, Feb 01, 2016 at 09:34:44PM +0800, Leo Yan wrote:
>> > Document DT binding for Hisilicon Hi6220 mailbox driver.
>> >
>> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
>> > ---
>> >  .../bindings/mailbox/hisilicon,hi6220-mailbox.txt  | 90 ++++++++++++++++++++++
>> >  1 file changed, 90 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
>> > new file mode 100644
>> > index 0000000..96e6acc
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
>> > @@ -0,0 +1,90 @@
>> > +Hisilicon Hi6220 Mailbox Driver
>> > +===============================
>> > +
>> > +Hisilicon Hi6220 mailbox supports up to 32 channels. Each channel
>> > +is unidirectional with a maximum message size of 8 words. I/O is
>> > +performed using register access (there is no DMA) and the cell
>> > +raises an interrupt when messages are received.
>> > +
>> > +Mailbox Device Node:
>> > +====================
>> > +
>> > +Required properties:
>> > +--------------------
>> > +- compatible:              Shall be "hisilicon,hi6220-mbox"
>> > +- reg:                     Contains the mailbox register address range (base
>> > +                   address and length); the first item is for IPC
>> > +                   registers, the second item is shared buffer for
>> > +                   slots.
>> > +- #mbox-cells              Common mailbox binding property to identify the number
>> > +                   of cells required for the mailbox specifier. Should be 1.
>> > +- interrupts:              Contains the interrupt information for the mailbox
>> > +                   device. The format is dependent on which interrupt
>> > +                   controller the SoCs use.
>> > +
>> > +Optional Properties:
>> > +--------------------
>> > +- hi6220,mbox-tx-noirq: Flag to allow the client user of this mailbox driver
>> > +                   to send messages without triggering a TX completion
>> > +                   interrupt.
>>
>> I don't think this belongs in DT. This should be a flag the client
>> driver sets when it sends messages.
>
> The client driver can set "tx_block = true" so use this flag indicates
> the client thread should be blocked until data is transmitted.
>
Yes, but the 'tx_block' feature is provided by the core. The
controller driver should not need to know how the client works.

> But low level mailbox driver can use two method to support "tx_block"
> mode:
>
No, as I said, provider shouldn't care about consumers..

> - One method is to avoid using interrupt and mailbox framework will
>   poll with mailbox's idle flag which is set by remote processor
>   automatically;
> - Another method is to use interrupt to notify data has been
>   transmitted and interrupt handler will call completion function to
>   wake up blocked client thread;
>
If it is possible to have either 'idle flag set' or irq generated (not
both) by the remote, then you may sell the hi6220,mbox-tx-noirq
property as a "f/w feature" ... but still not for the sake of
tx_block.


>> > +
>> > +Child Nodes:
>> > +============
>> > +A child node is used for representing the actual sub-mailbox device that is
>> > +used for the communication between the host processor and a remote processor.
>> > +Each child node should have a unique node name across all the different
>> > +mailbox device nodes.
>> > +
>> > +Required properties:
>> > +--------------------
>> > +- hi6220,mbox-tx:  sub-mailbox descriptor property defining Tx channel
>> > +- hi6220,mbox-rx:  sub-mailbox descriptor property defining Rx channel
>> > +
>> > +Sub-mailbox Descriptor Data
>> > +---------------------------
>> > +Each of the above hi6220,mbox-tx and hi6220,mbox-rx properties should have 3
>> > +cells of data that represent the following:
>> > +    Cell #1 (slot_id) - mailbox slot id used either for transmitting
>> > +                        (hi6220,mbox-tx) or for receiving (hi6220,mbox-rx)
>> > +    Cell #2 (dst_irq) - irq identifier index number which used by MCU.
>> > +    Cell #3 (ack_irq) - irq identifier index number with generating a tx/rx
>> > +                        interrupt to application processor, mailbox driver
>> > +                        used this id to acknowledge interrupt.
>> > +
>> > +Example:
>> > +--------
>> > +
>> > +   mailbox: mailbox at F7510000 {
>> > +           #mbox-cells = <1>;
>> > +           compatible = "hisilicon,hi6220-mbox";
>> > +           reg = <0x0 0xF7510000 0x0 0x1000>, /* IPC_S */
>> > +                 <0x0 0x06DFF800 0x0 0x0800>; /* Mailbox */
>> > +           interrupt-parent = <&gic>;
>> > +           interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
>> > +           mbox_stub_clock: mbox_stub_clock {
>> > +                   hi6220,mbox-rx = <0 1 10>;
>> > +                   hi6220,mbox-tx = <1 0 11>;
>
This looks like meant for the client node...
         mbox-names = "mbox-tx", "mbox-rx";
         mboxes = <&mailbox 1 0 11>, <&mailbox 0 1 10>;

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 1/4] dt-bindings: mailbox: Document Hi6220 mailbox driver
  2016-02-01 16:16         ` Jassi Brar
  (?)
@ 2016-02-02  9:22           ` Leo Yan
  -1 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2016-02-02  9:22 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Catalin Marinas, Will Deacon, Arnd Bergmann, Wei Xu, Tyler Baker,
	Sudeep Holla, Chen Feng, Bintian Wang, Devicetree List,
	Linux Kernel Mailing List, linux-arm-kernel, Haojian Zhuang

On Mon, Feb 01, 2016 at 09:46:39PM +0530, Jassi Brar wrote:
> On Mon, Feb 1, 2016 at 8:53 PM, Leo Yan <leo.yan@linaro.org> wrote:
> > On Mon, Feb 01, 2016 at 08:08:28AM -0600, Rob Herring wrote:
> >> On Mon, Feb 01, 2016 at 09:34:44PM +0800, Leo Yan wrote:

[...]

> >> > +Optional Properties:
> >> > +--------------------
> >> > +- hi6220,mbox-tx-noirq: Flag to allow the client user of this mailbox driver
> >> > +                   to send messages without triggering a TX completion
> >> > +                   interrupt.
> >>
> >> I don't think this belongs in DT. This should be a flag the client
> >> driver sets when it sends messages.
> >
> > The client driver can set "tx_block = true" so use this flag indicates
> > the client thread should be blocked until data is transmitted.
> >
> Yes, but the 'tx_block' feature is provided by the core. The
> controller driver should not need to know how the client works.
> 
> > But low level mailbox driver can use two method to support "tx_block"
> > mode:
> >
> No, as I said, provider shouldn't care about consumers..
> 
> > - One method is to avoid using interrupt and mailbox framework will
> >   poll with mailbox's idle flag which is set by remote processor
> >   automatically;
> > - Another method is to use interrupt to notify data has been
> >   transmitted and interrupt handler will call completion function to
> >   wake up blocked client thread;
> >
> If it is possible to have either 'idle flag set' or irq generated (not
> both) by the remote, then you may sell the hi6220,mbox-tx-noirq
> property as a "f/w feature" ... but still not for the sake of
> tx_block.

Indeed and totally agree. MCU can support two modes for "automatic idle
flag" or IRQ generated mode, so we can take "hi6220,mbox-tx-noirq" as a
firmware's property.

> >> > +
> >> > +Child Nodes:
> >> > +============
> >> > +A child node is used for representing the actual sub-mailbox device that is
> >> > +used for the communication between the host processor and a remote processor.
> >> > +Each child node should have a unique node name across all the different
> >> > +mailbox device nodes.
> >> > +
> >> > +Required properties:
> >> > +--------------------
> >> > +- hi6220,mbox-tx:  sub-mailbox descriptor property defining Tx channel
> >> > +- hi6220,mbox-rx:  sub-mailbox descriptor property defining Rx channel
> >> > +
> >> > +Sub-mailbox Descriptor Data
> >> > +---------------------------
> >> > +Each of the above hi6220,mbox-tx and hi6220,mbox-rx properties should have 3
> >> > +cells of data that represent the following:
> >> > +    Cell #1 (slot_id) - mailbox slot id used either for transmitting
> >> > +                        (hi6220,mbox-tx) or for receiving (hi6220,mbox-rx)
> >> > +    Cell #2 (dst_irq) - irq identifier index number which used by MCU.
> >> > +    Cell #3 (ack_irq) - irq identifier index number with generating a tx/rx
> >> > +                        interrupt to application processor, mailbox driver
> >> > +                        used this id to acknowledge interrupt.
> >> > +
> >> > +Example:
> >> > +--------
> >> > +
> >> > +   mailbox: mailbox@F7510000 {
> >> > +           #mbox-cells = <1>;
> >> > +           compatible = "hisilicon,hi6220-mbox";
> >> > +           reg = <0x0 0xF7510000 0x0 0x1000>, /* IPC_S */
> >> > +                 <0x0 0x06DFF800 0x0 0x0800>; /* Mailbox */
> >> > +           interrupt-parent = <&gic>;
> >> > +           interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
> >> > +           mbox_stub_clock: mbox_stub_clock {
> >> > +                   hi6220,mbox-rx = <0 1 10>;
> >> > +                   hi6220,mbox-tx = <1 0 11>;
> >
> This looks like meant for the client node...
>          mbox-names = "mbox-tx", "mbox-rx";
>          mboxes = <&mailbox 1 0 11>, <&mailbox 0 1 10>;

Good suggestion. Will refine with this way.

Thanks,
Leo Yan

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 1/4] dt-bindings: mailbox: Document Hi6220 mailbox driver
@ 2016-02-02  9:22           ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2016-02-02  9:22 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Catalin Marinas, Will Deacon, Arnd Bergmann, Wei Xu, Tyler Baker,
	Sudeep Holla, Chen Feng, Bintian Wang, Devicetree List,
	Linux Kernel Mailing List, linux-arm-kernel, Haojian Zhuang

On Mon, Feb 01, 2016 at 09:46:39PM +0530, Jassi Brar wrote:
> On Mon, Feb 1, 2016 at 8:53 PM, Leo Yan <leo.yan@linaro.org> wrote:
> > On Mon, Feb 01, 2016 at 08:08:28AM -0600, Rob Herring wrote:
> >> On Mon, Feb 01, 2016 at 09:34:44PM +0800, Leo Yan wrote:

[...]

> >> > +Optional Properties:
> >> > +--------------------
> >> > +- hi6220,mbox-tx-noirq: Flag to allow the client user of this mailbox driver
> >> > +                   to send messages without triggering a TX completion
> >> > +                   interrupt.
> >>
> >> I don't think this belongs in DT. This should be a flag the client
> >> driver sets when it sends messages.
> >
> > The client driver can set "tx_block = true" so use this flag indicates
> > the client thread should be blocked until data is transmitted.
> >
> Yes, but the 'tx_block' feature is provided by the core. The
> controller driver should not need to know how the client works.
> 
> > But low level mailbox driver can use two method to support "tx_block"
> > mode:
> >
> No, as I said, provider shouldn't care about consumers..
> 
> > - One method is to avoid using interrupt and mailbox framework will
> >   poll with mailbox's idle flag which is set by remote processor
> >   automatically;
> > - Another method is to use interrupt to notify data has been
> >   transmitted and interrupt handler will call completion function to
> >   wake up blocked client thread;
> >
> If it is possible to have either 'idle flag set' or irq generated (not
> both) by the remote, then you may sell the hi6220,mbox-tx-noirq
> property as a "f/w feature" ... but still not for the sake of
> tx_block.

Indeed and totally agree. MCU can support two modes for "automatic idle
flag" or IRQ generated mode, so we can take "hi6220,mbox-tx-noirq" as a
firmware's property.

> >> > +
> >> > +Child Nodes:
> >> > +============
> >> > +A child node is used for representing the actual sub-mailbox device that is
> >> > +used for the communication between the host processor and a remote processor.
> >> > +Each child node should have a unique node name across all the different
> >> > +mailbox device nodes.
> >> > +
> >> > +Required properties:
> >> > +--------------------
> >> > +- hi6220,mbox-tx:  sub-mailbox descriptor property defining Tx channel
> >> > +- hi6220,mbox-rx:  sub-mailbox descriptor property defining Rx channel
> >> > +
> >> > +Sub-mailbox Descriptor Data
> >> > +---------------------------
> >> > +Each of the above hi6220,mbox-tx and hi6220,mbox-rx properties should have 3
> >> > +cells of data that represent the following:
> >> > +    Cell #1 (slot_id) - mailbox slot id used either for transmitting
> >> > +                        (hi6220,mbox-tx) or for receiving (hi6220,mbox-rx)
> >> > +    Cell #2 (dst_irq) - irq identifier index number which used by MCU.
> >> > +    Cell #3 (ack_irq) - irq identifier index number with generating a tx/rx
> >> > +                        interrupt to application processor, mailbox driver
> >> > +                        used this id to acknowledge interrupt.
> >> > +
> >> > +Example:
> >> > +--------
> >> > +
> >> > +   mailbox: mailbox@F7510000 {
> >> > +           #mbox-cells = <1>;
> >> > +           compatible = "hisilicon,hi6220-mbox";
> >> > +           reg = <0x0 0xF7510000 0x0 0x1000>, /* IPC_S */
> >> > +                 <0x0 0x06DFF800 0x0 0x0800>; /* Mailbox */
> >> > +           interrupt-parent = <&gic>;
> >> > +           interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
> >> > +           mbox_stub_clock: mbox_stub_clock {
> >> > +                   hi6220,mbox-rx = <0 1 10>;
> >> > +                   hi6220,mbox-tx = <1 0 11>;
> >
> This looks like meant for the client node...
>          mbox-names = "mbox-tx", "mbox-rx";
>          mboxes = <&mailbox 1 0 11>, <&mailbox 0 1 10>;

Good suggestion. Will refine with this way.

Thanks,
Leo Yan

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v5 1/4] dt-bindings: mailbox: Document Hi6220 mailbox driver
@ 2016-02-02  9:22           ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2016-02-02  9:22 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 01, 2016 at 09:46:39PM +0530, Jassi Brar wrote:
> On Mon, Feb 1, 2016 at 8:53 PM, Leo Yan <leo.yan@linaro.org> wrote:
> > On Mon, Feb 01, 2016 at 08:08:28AM -0600, Rob Herring wrote:
> >> On Mon, Feb 01, 2016 at 09:34:44PM +0800, Leo Yan wrote:

[...]

> >> > +Optional Properties:
> >> > +--------------------
> >> > +- hi6220,mbox-tx-noirq: Flag to allow the client user of this mailbox driver
> >> > +                   to send messages without triggering a TX completion
> >> > +                   interrupt.
> >>
> >> I don't think this belongs in DT. This should be a flag the client
> >> driver sets when it sends messages.
> >
> > The client driver can set "tx_block = true" so use this flag indicates
> > the client thread should be blocked until data is transmitted.
> >
> Yes, but the 'tx_block' feature is provided by the core. The
> controller driver should not need to know how the client works.
> 
> > But low level mailbox driver can use two method to support "tx_block"
> > mode:
> >
> No, as I said, provider shouldn't care about consumers..
> 
> > - One method is to avoid using interrupt and mailbox framework will
> >   poll with mailbox's idle flag which is set by remote processor
> >   automatically;
> > - Another method is to use interrupt to notify data has been
> >   transmitted and interrupt handler will call completion function to
> >   wake up blocked client thread;
> >
> If it is possible to have either 'idle flag set' or irq generated (not
> both) by the remote, then you may sell the hi6220,mbox-tx-noirq
> property as a "f/w feature" ... but still not for the sake of
> tx_block.

Indeed and totally agree. MCU can support two modes for "automatic idle
flag" or IRQ generated mode, so we can take "hi6220,mbox-tx-noirq" as a
firmware's property.

> >> > +
> >> > +Child Nodes:
> >> > +============
> >> > +A child node is used for representing the actual sub-mailbox device that is
> >> > +used for the communication between the host processor and a remote processor.
> >> > +Each child node should have a unique node name across all the different
> >> > +mailbox device nodes.
> >> > +
> >> > +Required properties:
> >> > +--------------------
> >> > +- hi6220,mbox-tx:  sub-mailbox descriptor property defining Tx channel
> >> > +- hi6220,mbox-rx:  sub-mailbox descriptor property defining Rx channel
> >> > +
> >> > +Sub-mailbox Descriptor Data
> >> > +---------------------------
> >> > +Each of the above hi6220,mbox-tx and hi6220,mbox-rx properties should have 3
> >> > +cells of data that represent the following:
> >> > +    Cell #1 (slot_id) - mailbox slot id used either for transmitting
> >> > +                        (hi6220,mbox-tx) or for receiving (hi6220,mbox-rx)
> >> > +    Cell #2 (dst_irq) - irq identifier index number which used by MCU.
> >> > +    Cell #3 (ack_irq) - irq identifier index number with generating a tx/rx
> >> > +                        interrupt to application processor, mailbox driver
> >> > +                        used this id to acknowledge interrupt.
> >> > +
> >> > +Example:
> >> > +--------
> >> > +
> >> > +   mailbox: mailbox at F7510000 {
> >> > +           #mbox-cells = <1>;
> >> > +           compatible = "hisilicon,hi6220-mbox";
> >> > +           reg = <0x0 0xF7510000 0x0 0x1000>, /* IPC_S */
> >> > +                 <0x0 0x06DFF800 0x0 0x0800>; /* Mailbox */
> >> > +           interrupt-parent = <&gic>;
> >> > +           interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
> >> > +           mbox_stub_clock: mbox_stub_clock {
> >> > +                   hi6220,mbox-rx = <0 1 10>;
> >> > +                   hi6220,mbox-tx = <1 0 11>;
> >
> This looks like meant for the client node...
>          mbox-names = "mbox-tx", "mbox-rx";
>          mboxes = <&mailbox 1 0 11>, <&mailbox 0 1 10>;

Good suggestion. Will refine with this way.

Thanks,
Leo Yan

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2016-02-02  9:22 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-01 13:34 [PATCH v5 0/4] mailbox: hisilicon: add Hi6220 mailbox driver Leo Yan
2016-02-01 13:34 ` Leo Yan
2016-02-01 13:34 ` Leo Yan
2016-02-01 13:34 ` [PATCH v5 1/4] dt-bindings: mailbox: Document " Leo Yan
2016-02-01 13:34   ` Leo Yan
2016-02-01 14:08   ` Rob Herring
2016-02-01 14:08     ` Rob Herring
2016-02-01 14:08     ` Rob Herring
2016-02-01 15:23     ` Leo Yan
2016-02-01 15:23       ` Leo Yan
2016-02-01 16:16       ` Jassi Brar
2016-02-01 16:16         ` Jassi Brar
2016-02-01 16:16         ` Jassi Brar
2016-02-02  9:22         ` Leo Yan
2016-02-02  9:22           ` Leo Yan
2016-02-02  9:22           ` Leo Yan
2016-02-01 13:34 ` [PATCH v5 2/4] mailbox: Hi6220: add " Leo Yan
2016-02-01 13:34   ` Leo Yan
2016-02-01 13:34   ` Leo Yan
2016-02-01 13:34 ` [PATCH v5 3/4] arm64: dts: add mailbox node for Hi6220 Leo Yan
2016-02-01 13:34   ` Leo Yan
2016-02-01 13:34 ` [PATCH v5 4/4] arm64: dts: add Hi6220's stub clock node Leo Yan
2016-02-01 13:34   ` Leo Yan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.