[PATCH 0/2] Support for samsung mailbox controller

[PATCH 0/2] Support for samsung mailbox controller

[Girish K S]

This patch series is based on the mailbox framework prepared by jassi
and others.  

Girish K S (2):
  mailbox: samsung: added support for samsung mailbox
  arm64: dts: exynos: added mailbox node

 .../bindings/mailbox/samsung-mailbox.txt           |   24 ++
 arch/arm64/boot/dts/samsung-gh7.dtsi               |   66 ++++
 arch/arm64/boot/dts/samsung-ssdk-gh7.dts           |    3 +
 drivers/mailbox/Kconfig                            |    8 +
 drivers/mailbox/Makefile                           |    2 +
 drivers/mailbox/mailbox-samsung.c                  |  354 ++++++++++++++++++++
 include/linux/mailbox-samsung.h                    |  112 +++++++
 7 files changed, 569 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/samsung-mailbox.txt
 create mode 100644 drivers/mailbox/mailbox-samsung.c
 create mode 100644 include/linux/mailbox-samsung.h

-- 
1.7.10.4

[PATCH 1/2] mailbox: samsung: added support for samsung mailbox

[Girish K S]

This patch adds support to the samsung mailbox driver.

Signed-off-by: Girish K S <ks.giri@samsung.com>
---
 drivers/mailbox/Kconfig           |    8 +
 drivers/mailbox/Makefile          |    2 +
 drivers/mailbox/mailbox-samsung.c |  354 +++++++++++++++++++++++++++++++++++++
 include/linux/mailbox-samsung.h   |  112 ++++++++++++
 4 files changed, 476 insertions(+)
 create mode 100644 drivers/mailbox/mailbox-samsung.c
 create mode 100644 include/linux/mailbox-samsung.h

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index c8b5c13..bce60ba 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -23,6 +23,14 @@ config OMAP_MBOX
 	  driver such as CONFIG_OMAP1_MBOX or CONFIG_OMAP2PLUS_MBOX. This
 	  enables the common OMAP mailbox framework code.
 
+config SAMSUNG_MBOX
+	tristate "Samsung Mailbox Controller"
+	help
+	  An implementation of the Samsung Interprocessor Communication
+	  Mailbox (IPCM). It is used to send short messages between A57 cores
+	  and the System Control Processor or Network Coprocessor firmware.
+	  Say Y here if you want to use the Samsung IPCM support.
+
 config OMAP1_MBOX
 	tristate "OMAP1 Mailbox framework support"
 	depends on ARCH_OMAP1
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 2fa343a..7e83a7d 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -9,3 +9,5 @@ obj-$(CONFIG_OMAP1_MBOX)	+= mailbox_omap1.o
 mailbox_omap1-objs		:= mailbox-omap1.o
 obj-$(CONFIG_OMAP2PLUS_MBOX)	+= mailbox_omap2.o
 mailbox_omap2-objs		:= mailbox-omap2.o
+obj-$(CONFIG_SAMSUNG_MBOX)	+= mailbox_samsung.o
+mailbox_samsung-objs		:= mailbox-samsung.o
diff --git a/drivers/mailbox/mailbox-samsung.c b/drivers/mailbox/mailbox-samsung.c
new file mode 100644
index 0000000..903e88a
--- /dev/null
+++ b/drivers/mailbox/mailbox-samsung.c
@@ -0,0 +1,354 @@
+/*
+ * Copyright 2013 Samsung Electronics Co. Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/jiffies.h>
+#include <linux/mailbox_controller.h>
+#include <linux/mailbox-samsung.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+
+/*
+   Mailbox Register offsets
+ */
+
+/* Offset of tx mailbox data registers */
+#define MBOX_REG_RXDATA0	0
+#define MBOX_REG_RXDATA1	4
+
+/* offset of tx mailbox lock registers */
+#define MBOX_REG_RXLOCK		8
+#define MBOX_REG_RXUNLOCK	12
+
+/* offset of tx mailbox interrupt registers */
+#define MBOX_REG_RXINT		16
+
+/* Offset of rx mailbox data registers */
+#define MBOX_REG_TXDATA0	32
+#define MBOX_REG_TXDATA1	36
+
+/* offset of rx mailbox lock registers */
+#define MBOX_REG_TXLOCK		40
+#define MBOX_REG_TXUNLOCK	44
+
+/* offset of rx mailbox interrupt registers */
+#define MBOX_REG_TXINT		48
+
+#define MBOX_RX_TX_FULL		1
+#define MBOX_INT_ENABLE		1
+
+#define MBOX_LOCK_RESET		(-1)
+#define MBOX_WAIT_TIMEOUT	(1)
+
+#define RXBUF_LEN	32
+#define MAX_LINKS	1
+
+struct samsung_mlink {
+	const char *name;
+	struct ipc_link link;
+	struct samsung_mbox *smc;
+	int irq;
+	void *data;
+};
+
+struct samsung_mbox {
+	struct device *dev;
+	void __iomem *ipc_base;
+	struct ipc_controller ipc_con;
+	struct mutex lock;
+	struct samsung_mlink samsung_link[MAX_LINKS];
+};
+
+static inline struct samsung_mlink *to_samsung_mlink(struct ipc_link *plink)
+{
+	if (!plink)
+		return NULL;
+
+	return container_of(plink, struct samsung_mlink, link);
+}
+
+static inline struct samsung_mbox *to_samsung_mbox(struct ipc_link *plink)
+{
+	if (!plink)
+		return NULL;
+
+	return to_samsung_mlink(plink)->smc;
+}
+
+static void samsung_mbox_read(struct samsung_mlink *link)
+{
+	u64 data;
+	void __iomem *ipc_base = link->smc->ipc_base;
+
+	/* Read both words which have information specific to client */
+	data = readq(ipc_base + MBOX_REG_RXDATA0);
+	link->data = phys_to_virt((phys_addr_t)data);
+}
+
+static irqreturn_t samsung_ipc_handler(int irq, void *p)
+{
+	struct samsung_mlink *plink = p;
+	void __iomem *ipc_base = plink->smc->ipc_base;
+
+	samsung_mbox_read(plink);
+	ipc_link_received_data(&plink->link, plink->data);
+
+	/* Acknowledge the interrupt by clearing the interrupt register */
+	writel(~MBOX_INT_ENABLE, ipc_base + MBOX_REG_RXINT);
+	writel(readl(ipc_base + MBOX_REG_RXLOCK),
+	       ipc_base + MBOX_REG_RXUNLOCK);
+
+	return IRQ_HANDLED;
+}
+
+static bool samsung_mbox_is_ready(struct ipc_link *link)
+{
+	struct samsung_mbox *samsung_mbox = to_samsung_mbox(link);
+	void __iomem *ipc_base = samsung_mbox->ipc_base;
+	int ret = true;
+
+	mutex_lock(&samsung_mbox->lock);
+
+	/* If the RX interrupt register is clear then mbox is empty */
+	if (readl(ipc_base + MBOX_REG_TXLOCK) &&
+	    readl(ipc_base + MBOX_REG_TXINT))
+		ret = false;
+
+	mutex_unlock(&samsung_mbox->lock);
+	return ret;
+}
+
+static int samsung_mbox_send_data(struct ipc_link *link, void *msg)
+{
+	u32 val;
+	phys_addr_t addr;
+	unsigned long timeout;
+	struct samsung_mbox *samsung_mbox = to_samsung_mbox(link);
+	void __iomem *ipc_base = samsung_mbox->ipc_base;
+	int ret = 0;
+
+	mutex_lock(&samsung_mbox->lock);
+
+	timeout = jiffies + msecs_to_jiffies(MBOX_WAIT_TIMEOUT);
+
+	/* Check if the mailbox is busy transmitting data */
+	if (readl(ipc_base + MBOX_REG_TXLOCK)) {
+		ret = -EBUSY;
+		goto unlock_mutex;
+	}
+
+	addr = virt_to_phys(msg);
+	val = samsung_get_client_id(msg);
+
+	/* Lock the mailbox for this transaction with the client id */
+	writel(val, ipc_base + MBOX_REG_TXLOCK);
+	do {
+		if (time_after(jiffies, timeout)) {
+			pr_err("cannot aquire the lock\n");
+			ret = -EAGAIN;
+			goto unlock_mutex;
+		}
+	} while (readl(ipc_base + MBOX_REG_TXLOCK) != val);
+
+	writeq(addr, ipc_base + MBOX_REG_TXDATA0);
+	val = readl(ipc_base + MBOX_REG_TXINT);
+	val |= MBOX_INT_ENABLE;
+	writel(val, ipc_base + MBOX_REG_TXINT);
+
+unlock_mutex:
+	mutex_unlock(&samsung_mbox->lock);
+	return ret;
+}
+
+static int samsung_mbox_startup(struct ipc_link *link, void *ignored)
+{
+	int ret;
+	struct samsung_mlink *samsung_link = to_samsung_mlink(link);
+	struct samsung_mbox *samsung_mbox = to_samsung_mbox(link);
+	void __iomem *ipc_base = samsung_mbox->ipc_base;
+	struct device *dev = samsung_mbox->dev;
+
+	mutex_lock(&samsung_mbox->lock);
+
+	ret = devm_request_irq(dev, samsung_link->irq, samsung_ipc_handler,
+					IRQF_SHARED, link->link_name,
+					samsung_link);
+	if (unlikely(ret)) {
+		pr_err("failed to register mailbox interrupt:%d\n", ret);
+		mutex_unlock(&samsung_mbox->lock);
+		return ret;
+	}
+
+	/*
+	 * On shutdown irrespective of what is the previous lock id
+	 * clear it. Dont have to do the same for RX lock. Remote
+	 * processor shall handle the RX lock clear on shutdown.
+	 */
+	writel(~MBOX_INT_ENABLE, ipc_base + MBOX_REG_TXINT);
+	writel(MBOX_LOCK_RESET, ipc_base + MBOX_REG_TXUNLOCK);
+
+	mutex_unlock(&samsung_mbox->lock);
+	return 0;
+}
+
+static void samsung_mbox_shutdown(struct ipc_link *link)
+{
+	struct samsung_mbox *samsung_mbox = to_samsung_mbox(link);
+	struct samsung_mlink *samsung_link = to_samsung_mlink(link);
+	void __iomem *ipc_base = samsung_mbox->ipc_base;
+	struct device *dev = samsung_mbox->dev;
+
+	mutex_lock(&samsung_mbox->lock);
+
+	/*
+	 * On shutdown irrespective of what is the previous lock id
+	 * clear it. Dont have to do the same for RX lock. Remote
+	 * processor shall handle the RX lock clear on shutdown.
+	 */
+	writel(~MBOX_INT_ENABLE, ipc_base + MBOX_REG_TXINT);
+	writel(MBOX_LOCK_RESET, ipc_base + MBOX_REG_TXUNLOCK);
+
+	devm_free_irq(dev, samsung_link->irq, samsung_link);
+
+	mutex_unlock(&samsung_mbox->lock);
+}
+
+static struct ipc_link_ops samsung_mbox_ops = {
+	.is_ready = samsung_mbox_is_ready,
+	.send_data = samsung_mbox_send_data,
+	.startup = samsung_mbox_startup,
+	.shutdown = samsung_mbox_shutdown,
+};
+
+static int samsung_mbox_probe(struct platform_device *pdev)
+{
+	struct samsung_mbox *samsung_mbox;
+	struct samsung_mlink *mbox_link;
+	struct resource res;
+	struct ipc_link **link;
+	int loop, count, ret = 0;
+	struct device_node *node = pdev->dev.of_node;
+
+	if (!node) {
+		dev_err(&pdev->dev, "driver doesnt support"
+				"non-dt devices\n");
+		return -ENODEV;
+	}
+
+	count = of_property_count_strings(node,
+				"samsung,mbox-names");
+	if (count <= 0) {
+		dev_err(&pdev->dev, "no mbox devices found\n");
+		return -ENODEV;
+	}
+
+	samsung_mbox = devm_kzalloc(&pdev->dev,
+			sizeof(struct samsung_mbox), GFP_KERNEL);
+	if (IS_ERR(samsung_mbox))
+		return PTR_ERR(samsung_mbox);
+
+	link = devm_kzalloc(&pdev->dev, (count + 1) * sizeof(*link),
+			GFP_KERNEL);
+	if (IS_ERR(link))
+		return PTR_ERR(link);
+
+	samsung_mbox->dev = &pdev->dev;
+
+	samsung_mbox->ipc_base = devm_ioremap_resource(&pdev->dev, &res);
+	if (IS_ERR(samsung_mbox->ipc_base))
+		return PTR_ERR(samsung_mbox->ipc_base);
+
+	for (loop = 0; loop < count; loop++) {
+		mbox_link = &samsung_mbox->samsung_link[loop];
+
+		ret = of_address_to_resource(node, 0, &res);
+		if (ret < 0)
+			return ret;
+
+		mbox_link->irq = irq_of_parse_and_map(node, loop);
+		if (!mbox_link->irq)
+			return -ENODEV;
+
+		mbox_link->smc = samsung_mbox;
+		link[loop] = &mbox_link->link;
+
+		if (of_property_read_string_index(node, "samsung,mbox-names",
+						  loop, &mbox_link->name)) {
+			dev_err(&pdev->dev,
+				"mbox_name [%d] read failed\n", loop);
+			return -ENODEV;
+		}
+
+		snprintf(link[loop]->link_name, 16, mbox_link->name);
+	}
+
+	link[loop] = NULL; /* Terminating link */
+
+	mutex_init(&samsung_mbox->lock);
+	samsung_mbox->ipc_con.links = link;
+	samsung_mbox->ipc_con.txdone_irq = false;
+	samsung_mbox->ipc_con.txdone_poll = true;
+	samsung_mbox->ipc_con.txpoll_period = 1;
+	samsung_mbox->ipc_con.ops = &samsung_mbox_ops;
+	snprintf(samsung_mbox->ipc_con.controller_name, 16, "samsung_mbox");
+
+	ret = ipc_links_register(&samsung_mbox->ipc_con);
+	if (ret) {
+		pr_err("%s: IPC Link register failed\n", __func__);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, samsung_mbox);
+
+	return ret;
+}
+
+static int samsung_mbox_remove(struct platform_device *pdev)
+{
+	struct samsung_mbox *samsung_mbox = platform_get_drvdata(pdev);
+
+	ipc_links_unregister(&samsung_mbox->ipc_con);
+
+	return 0;
+}
+
+static const struct of_device_id mailbox_smc_match[] = {
+	{ .compatible = "samsung,gh7-mailbox" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mailbox_smc_match);
+
+static struct platform_driver samsung_mbox_driver = {
+	.probe	= samsung_mbox_probe,
+	.remove	= samsung_mbox_remove,
+	.driver	= {
+		.name = "gh7-mailbox",
+		.of_match_table	= mailbox_smc_match,
+	},
+};
+
+module_platform_driver(samsung_mbox_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Samsung Mailbox Driver");
+MODULE_AUTHOR("Girish K S <ks.giri@samsung.com>");
+MODULE_ALIAS("platform:gh7-mailbox");
diff --git a/include/linux/mailbox-samsung.h b/include/linux/mailbox-samsung.h
new file mode 100644
index 0000000..c3f2d2c
--- /dev/null
+++ b/include/linux/mailbox-samsung.h
@@ -0,0 +1,112 @@
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _MAILBOX_SAMSUNG_H_
+#define _MAILBOX_SAMSUNG_H_
+
+#include <linux/slab.h>
+
+#define SAMSUNG_MB_RESP_LEN	(4096)
+#define SAMSUNG_MB_DRV_ID	(0xa5)
+#define SAMSUNG_MB_DRV_LEN	(0x08)
+#define REQUEST_PAGE_ORDER	(16)
+
+#define IRQ_TX			(1)
+#define IRQ_RX			(2)
+
+struct samsung_packet {
+	u8 version;
+	u8 client_id;
+	u8 remote_id;
+	u16 req_len;
+	u64 resp_addr;
+	u16 resp_len;
+	u8 reserved[16];
+	u8 checksum;
+};
+
+static inline unsigned long get_alligned_buffer(void)
+{
+	return __get_free_pages(GFP_KERNEL, REQUEST_PAGE_ORDER);
+}
+
+static inline void put_alligned_buffer(unsigned long addr)
+{
+	free_pages(addr, REQUEST_PAGE_ORDER);
+}
+
+static inline void samsung_put_version(void *buf, u8 version)
+{
+	struct samsung_packet *data = buf;
+	data->version = version;
+}
+
+static inline void samsung_put_client_id(void *buf, u8 id)
+{
+	struct samsung_packet *data = buf;
+	data->client_id = (SAMSUNG_MB_DRV_ID << SAMSUNG_MB_DRV_LEN) | id;
+}
+
+static inline void samsung_put_remote_id(void *buf, u8 id)
+{
+	struct samsung_packet *data = buf;
+	data->remote_id = id;
+}
+
+static inline void samsung_put_req_len(void *buf, u16 len)
+{
+	struct samsung_packet *data = buf;
+	data->req_len = len;
+}
+
+static inline void samsung_put_resp_addr(void *buf, u64 addr)
+{
+	struct samsung_packet *data = buf;
+	data->resp_addr = addr;
+}
+
+static inline void samsung_put_checksum(void *buf, u8 checksum)
+{
+	struct samsung_packet *data = buf;
+	data->checksum = checksum;
+}
+
+static inline int samsung_get_resp_size(void)
+{
+	/* The response buffer length can be max upto 4kb */
+	return SAMSUNG_MB_RESP_LEN;
+}
+
+static inline int samsung_get_resp_len(void *buf)
+{
+	struct samsung_packet *data = buf;
+
+	/* The actual lenght of response buffer is got from remote packet */
+	return data->resp_len;
+}
+
+static inline u64 samsung_get_resp_addr(void *buf)
+{
+	struct samsung_packet *data = buf;
+	return data->resp_addr;
+}
+
+static inline int samsung_get_client_id(const void *buf)
+{
+	const struct samsung_packet *data = buf;
+	return data->client_id;
+}
+
+#endif
-- 
1.7.10.4

[PATCH 2/2] arm64: dts: exynos: added mailbox node

[Girish K S]

This patch adds the dt node for the mailbox IP

Signed-off-by: Girish K S <ks.giri@samsung.com>

Change-Id: I35e45e9a62592887a84a909aee54f259a2f731fa
---
 .../bindings/mailbox/samsung-mailbox.txt           |   24 +++++++
 arch/arm64/boot/dts/samsung-gh7.dtsi               |   66 ++++++++++++++++++++
 arch/arm64/boot/dts/samsung-ssdk-gh7.dts           |    3 +
 3 files changed, 93 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/samsung-mailbox.txt

diff --git a/Documentation/devicetree/bindings/mailbox/samsung-mailbox.txt b/Documentation/devicetree/bindings/mailbox/samsung-mailbox.txt
new file mode 100644
index 0000000..1908d71
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/samsung-mailbox.txt
@@ -0,0 +1,24 @@
+
+Samsung Mailbox Driver
+
+Required properties:
+- compatible:		Should be one of the following,
+			    "samsung,gh7-mailbox" for
+				Samsung GH7 SoC series
+			    "samsung,exynos-mailbox" for
+				exynosx SoC series
+- reg:			Contains the mailbox register address range (base address
+			and length)
+- interrupts: 		Contains the interrupt information for the mailbox
+			device.
+- samsung,mbox-names:	Array of the names of the mailboxes
+
+Example:
+
+/* Samsung GH7 SoC */
+mailbox@100a0000 {
+	compatible = "samsung,gh7-mailbox";
+	reg = <0x0 0x100a0000 0x0 0x1000>;
+	interrupts = <0 310 0>;
+	samsung,mbox-names = "a7q-scp";
+};
diff --git a/arch/arm64/boot/dts/samsung-gh7.dtsi b/arch/arm64/boot/dts/samsung-gh7.dtsi
index c3610bd..be4cce9 100644
--- a/arch/arm64/boot/dts/samsung-gh7.dtsi
+++ b/arch/arm64/boot/dts/samsung-gh7.dtsi
@@ -107,5 +107,71 @@
 			interrupts = <0 420 0>;
 			arm,primecell-periphid = <0x341011>; /* HACK */
 		};
+
+	};
+
+	mailbox@100a0000 {
+		compatible = "samsung,gh7-mailbox";
+		reg = <0x0 0x100a0000 0x0 0x1000>;
+		interrupts = <0 310 0>;
+		samsung,mbox-names = "a7q-scp";
+		status = "disabled";
+	};
+
+	mailbox@24100000 {
+		compatible = "samsung,exynos-mailbox";
+		reg = 	< 0x0 0x24100000 0x0 0x10000>;
+		interrupts = <0 251 0>;
+		samsung,mbox-names = "ncp-a7q-0";
+		status = "disabled";
+	};
+	mailbox@24110000 {
+		compatible = "samsung,exynos-mailbox";
+		reg = 	<0x0 0x24110000 0x0 0x10000>;
+		interrupts = <0 252 0>;
+		samsung,mbox-names = "ncp-a7q-1";
+		status = "disabled";
+	};
+	mailbox@24120000 {
+		compatible = "samsung,exynos-mailbox";
+		reg = 	<0x0 0x24120000 0x0 0x10000>;
+		interrupts = <0 253 0>;
+		samsung,mbox-names = "ncp-a7q-2";
+		status = "disabled";
+	};
+	mailbox@24130000 {
+		compatible = "samsung,exynos-mailbox";
+		reg = 	<0x0 0x24130000 0x0 0x10000>;
+		interrupts = <0 254 0>;
+		samsung,mbox-names = "ncp-a7q-3";
+		status = "disabled";
+	};
+	mailbox@24140000 {
+		compatible = "samsung,exynos-mailbox";
+		reg = 	<0x0 0x24140000 0x0 0x10000>;
+		interrupts = <0 255 0>;
+		samsung,mbox-names = "ncp-a7q-4";
+		status = "disabled";
+	};
+	mailbox@24150000 {
+		compatible = "samsung,exynos-mailbox";
+		reg = 	<0x0 0x24150000 0x0 0x10000>;
+		interrupts = <0 256 0>;
+		samsung,mbox-names = "ncp-a7q-5";
+		status = "disabled";
+	};
+	mailbox@24160000 {
+		compatible = "samsung,exynos-mailbox";
+		reg = 	<0x0 0x24160000 0x0 0x10000>;
+		interrupts = <0 257 0>;
+		samsung,mbox-names = "ncp-a7q-6";
+		status = "disabled";
+	};
+	mailbox@24170000 {
+		compatible = "samsung,exynos-mailbox";
+		reg = 	<0x0 0x24170000 0x0 0x10000>;
+		interrupts = <0 258 0>;
+		samsung,mbox-names = "ncp-a7q-7";
+		status = "disabled";
 	};
 };
diff --git a/arch/arm64/boot/dts/samsung-ssdk-gh7.dts b/arch/arm64/boot/dts/samsung-ssdk-gh7.dts
index 4ce7d67..bfe5455 100644
--- a/arch/arm64/boot/dts/samsung-ssdk-gh7.dts
+++ b/arch/arm64/boot/dts/samsung-ssdk-gh7.dts
@@ -23,4 +23,7 @@
 		device_type = "memory";
 		reg = <0x00000000 0x80000000 0 0x20000000>;
 	};
+	mailbox@100a0000 {
+		status = "okay";
+	};
 };
-- 
1.7.10.4

Re: [PATCH 2/2] arm64: dts: exynos: added mailbox node

[Girish KS]

On Mon, Mar 17, 2014 at 5:33 PM, Girish K S <ks.giri@samsung.com> wrote:
> This patch adds the dt node for the mailbox IP
>
> Signed-off-by: Girish K S <ks.giri@samsung.com>
>
> Change-Id: I35e45e9a62592887a84a909aee54f259a2f731fa

Sorry for this. will modify in the v2 with other comments

> ---
>  .../bindings/mailbox/samsung-mailbox.txt           |   24 +++++++
>  arch/arm64/boot/dts/samsung-gh7.dtsi               |   66 ++++++++++++++++++++
>  arch/arm64/boot/dts/samsung-ssdk-gh7.dts           |    3 +
>  3 files changed, 93 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/samsung-mailbox.txt
>
> diff --git a/Documentation/devicetree/bindings/mailbox/samsung-mailbox.txt b/Documentation/devicetree/bindings/mailbox/samsung-mailbox.txt
> new file mode 100644
> index 0000000..1908d71
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/samsung-mailbox.txt
> @@ -0,0 +1,24 @@
> +
> +Samsung Mailbox Driver
> +
> +Required properties:
> +- compatible:          Should be one of the following,
> +                           "samsung,gh7-mailbox" for
> +                               Samsung GH7 SoC series
> +                           "samsung,exynos-mailbox" for
> +                               exynosx SoC series
> +- reg:                 Contains the mailbox register address range (base address
> +                       and length)
> +- interrupts:          Contains the interrupt information for the mailbox
> +                       device.
> +- samsung,mbox-names:  Array of the names of the mailboxes
> +
> +Example:
> +
> +/* Samsung GH7 SoC */
> +mailbox@100a0000 {
> +       compatible = "samsung,gh7-mailbox";
> +       reg = <0x0 0x100a0000 0x0 0x1000>;
> +       interrupts = <0 310 0>;
> +       samsung,mbox-names = "a7q-scp";
> +};
> diff --git a/arch/arm64/boot/dts/samsung-gh7.dtsi b/arch/arm64/boot/dts/samsung-gh7.dtsi
> index c3610bd..be4cce9 100644
> --- a/arch/arm64/boot/dts/samsung-gh7.dtsi
> +++ b/arch/arm64/boot/dts/samsung-gh7.dtsi
> @@ -107,5 +107,71 @@
>                         interrupts = <0 420 0>;
>                         arm,primecell-periphid = <0x341011>; /* HACK */
>                 };
> +
> +       };
> +
> +       mailbox@100a0000 {
> +               compatible = "samsung,gh7-mailbox";
> +               reg = <0x0 0x100a0000 0x0 0x1000>;
> +               interrupts = <0 310 0>;
> +               samsung,mbox-names = "a7q-scp";
> +               status = "disabled";
> +       };
> +
> +       mailbox@24100000 {
> +               compatible = "samsung,exynos-mailbox";
> +               reg =   < 0x0 0x24100000 0x0 0x10000>;
> +               interrupts = <0 251 0>;
> +               samsung,mbox-names = "ncp-a7q-0";
> +               status = "disabled";
> +       };
> +       mailbox@24110000 {
> +               compatible = "samsung,exynos-mailbox";
> +               reg =   <0x0 0x24110000 0x0 0x10000>;
> +               interrupts = <0 252 0>;
> +               samsung,mbox-names = "ncp-a7q-1";
> +               status = "disabled";
> +       };
> +       mailbox@24120000 {
> +               compatible = "samsung,exynos-mailbox";
> +               reg =   <0x0 0x24120000 0x0 0x10000>;
> +               interrupts = <0 253 0>;
> +               samsung,mbox-names = "ncp-a7q-2";
> +               status = "disabled";
> +       };
> +       mailbox@24130000 {
> +               compatible = "samsung,exynos-mailbox";
> +               reg =   <0x0 0x24130000 0x0 0x10000>;
> +               interrupts = <0 254 0>;
> +               samsung,mbox-names = "ncp-a7q-3";
> +               status = "disabled";
> +       };
> +       mailbox@24140000 {
> +               compatible = "samsung,exynos-mailbox";
> +               reg =   <0x0 0x24140000 0x0 0x10000>;
> +               interrupts = <0 255 0>;
> +               samsung,mbox-names = "ncp-a7q-4";
> +               status = "disabled";
> +       };
> +       mailbox@24150000 {
> +               compatible = "samsung,exynos-mailbox";
> +               reg =   <0x0 0x24150000 0x0 0x10000>;
> +               interrupts = <0 256 0>;
> +               samsung,mbox-names = "ncp-a7q-5";
> +               status = "disabled";
> +       };
> +       mailbox@24160000 {
> +               compatible = "samsung,exynos-mailbox";
> +               reg =   <0x0 0x24160000 0x0 0x10000>;
> +               interrupts = <0 257 0>;
> +               samsung,mbox-names = "ncp-a7q-6";
> +               status = "disabled";
> +       };
> +       mailbox@24170000 {
> +               compatible = "samsung,exynos-mailbox";
> +               reg =   <0x0 0x24170000 0x0 0x10000>;
> +               interrupts = <0 258 0>;
> +               samsung,mbox-names = "ncp-a7q-7";
> +               status = "disabled";
>         };
>  };
> diff --git a/arch/arm64/boot/dts/samsung-ssdk-gh7.dts b/arch/arm64/boot/dts/samsung-ssdk-gh7.dts
> index 4ce7d67..bfe5455 100644
> --- a/arch/arm64/boot/dts/samsung-ssdk-gh7.dts
> +++ b/arch/arm64/boot/dts/samsung-ssdk-gh7.dts
> @@ -23,4 +23,7 @@
>                 device_type = "memory";
>                 reg = <0x00000000 0x80000000 0 0x20000000>;
>         };
> +       mailbox@100a0000 {
> +               status = "okay";
> +       };
>  };
> --
> 1.7.10.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] arm64: dts: exynos: added mailbox node

[Arnd Bergmann]

On Monday 17 March 2014 17:33:59 Girish K S wrote:
> +Samsung Mailbox Driver
> +
> +Required properties:
> +- compatible:          Should be one of the following,
> +                           "samsung,gh7-mailbox" for
> +                               Samsung GH7 SoC series
> +                           "samsung,exynos-mailbox" for
> +                               exynosx SoC series
> +- reg:                 Contains the mailbox register address range (base address
> +                       and length)
> +- interrupts:          Contains the interrupt information for the mailbox
> +                       device.
> +- samsung,mbox-names:  Array of the names of the mailboxes
> 

I think we should not allow new mailbox drivers that don't conform to
the framework that is currently under discussion. In particular, this
means don't do a "samsung,mbox-names" property. The current consensus
seems to be to have a #mbox-cells" property that allows to pass extra
parameters from the client driver, and uses an "mboxes" property
to reference the mailbox provided.

It would be good if you follow up for the subsystem discussion and
ensure it gets merged in time, and supports all the use cases you are
interested in. The interface is not entirely nailed down yet, so
it's a good time to contribute. However, I can already promise that
it won't use matching by strings.

	Arnd

Re: [PATCH 2/2] arm64: dts: exynos: added mailbox node

[Mark Rutland]

On Mon, Mar 17, 2014 at 12:03:59PM +0000, Girish K S wrote:
> This patch adds the dt node for the mailbox IP
> 
> Signed-off-by: Girish K S <ks.giri@samsung.com>
> 
> Change-Id: I35e45e9a62592887a84a909aee54f259a2f731fa
> ---
>  .../bindings/mailbox/samsung-mailbox.txt           |   24 +++++++
>  arch/arm64/boot/dts/samsung-gh7.dtsi               |   66 ++++++++++++++++++++
>  arch/arm64/boot/dts/samsung-ssdk-gh7.dts           |    3 +
>  3 files changed, 93 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/samsung-mailbox.txt
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/samsung-mailbox.txt b/Documentation/devicetree/bindings/mailbox/samsung-mailbox.txt
> new file mode 100644
> index 0000000..1908d71
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/samsung-mailbox.txt
> @@ -0,0 +1,24 @@
> +
> +Samsung Mailbox Driver
> +
> +Required properties:
> +- compatible:		Should be one of the following,
> +			    "samsung,gh7-mailbox" for
> +				Samsung GH7 SoC series
> +			    "samsung,exynos-mailbox" for
> +				exynosx SoC series
> +- reg:			Contains the mailbox register address range (base address
> +			and length)
> +- interrupts: 		Contains the interrupt information for the mailbox
> +			device.

How many interrupts?

What are they for?

> +- samsung,mbox-names:	Array of the names of the mailboxes

Juding by the code there is one name per reg entry, but the description
above implies a single entry (as all the dt fragments have).

What values are expected? What is the consumer of these values? What are
they used for?

> +
> +Example:
> +
> +/* Samsung GH7 SoC */
> +mailbox@100a0000 {
> +	compatible = "samsung,gh7-mailbox";
> +	reg = <0x0 0x100a0000 0x0 0x1000>;
> +	interrupts = <0 310 0>;
> +	samsung,mbox-names = "a7q-scp";
> +};
> diff --git a/arch/arm64/boot/dts/samsung-gh7.dtsi b/arch/arm64/boot/dts/samsung-gh7.dtsi
> index c3610bd..be4cce9 100644
> --- a/arch/arm64/boot/dts/samsung-gh7.dtsi
> +++ b/arch/arm64/boot/dts/samsung-gh7.dtsi
> @@ -107,5 +107,71 @@
>  			interrupts = <0 420 0>;
>  			arm,primecell-periphid = <0x341011>; /* HACK */

This looks odd.

What tree is this against?

Cheers,
Mark.

Re: [PATCH 2/2] arm64: dts: exynos: added mailbox node

[Jassi Brar]

On Mon, Mar 17, 2014 at 5:45 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 17 March 2014 17:33:59 Girish K S wrote:
>> +Samsung Mailbox Driver
>> +
>> +Required properties:
>> +- compatible:          Should be one of the following,
>> +                           "samsung,gh7-mailbox" for
>> +                               Samsung GH7 SoC series
>> +                           "samsung,exynos-mailbox" for
>> +                               exynosx SoC series
>> +- reg:                 Contains the mailbox register address range (base address
>> +                       and length)
>> +- interrupts:          Contains the interrupt information for the mailbox
>> +                       device.
>> +- samsung,mbox-names:  Array of the names of the mailboxes
>>
>
> I think we should not allow new mailbox drivers that don't conform to
> the framework that is currently under discussion. In particular, this
> means don't do a "samsung,mbox-names" property. The current consensus
> seems to be to have a #mbox-cells" property that allows to pass extra
> parameters from the client driver, and uses an "mboxes" property
> to reference the mailbox provided.
>
> It would be good if you follow up for the subsystem discussion and
> ensure it gets merged in time, and supports all the use cases you are
> interested in. The interface is not entirely nailed down yet, so
> it's a good time to contribute. However, I can already promise that
> it won't use matching by strings.
>
I was just about to submit next revision of framework that this
Samsung driver conforms to. Now I think I need some expert opinion ...
(sorry if following is repetition)

  As Kumar Gala pointed out in the other thread, the mailbox/ipc chain
is going to be _very_ platform specific so much so that one rethinks
if a common api is even warranted : because the client driver will be
different even if just the remote api(which is going to be
proprietary) changes, keeping everything else same. For example, a
client driver for Highbank is highly unlikely to be reusable on
Exynos, even if both used the same mailbox controller. By my limited
foresight, mailbox assignment via DT doesn't bring us much benefit but
only ritual code.

  Perhaps the mailbox controller driver should name its links as it
wants. By how the remote works with the mailbox links, the client
driver asks for a specific mailbox link (which maybe a hardcoded
string in the driver or be gotten alongside other data via client's
DT) ?

  IOW we can't have a generic API/DT-bindings that could get us
reusable client drivers. But only common framework/code that would
otherwise be duplicated by every platform.

Thanks
Jassi

Re: [PATCH 1/2] mailbox: samsung: added support for samsung mailbox

[Girish KS]

On Mon, Mar 17, 2014 at 5:33 PM, Girish K S <ks.giri@samsung.com> wrote:
> This patch adds support to the samsung mailbox driver.
>
> Signed-off-by: Girish K S <ks.giri@samsung.com>
> ---
>  drivers/mailbox/Kconfig           |    8 +
>  drivers/mailbox/Makefile          |    2 +
>  drivers/mailbox/mailbox-samsung.c |  354 +++++++++++++++++++++++++++++++++++++
>  include/linux/mailbox-samsung.h   |  112 ++++++++++++
>  4 files changed, 476 insertions(+)
>  create mode 100644 drivers/mailbox/mailbox-samsung.c
>  create mode 100644 include/linux/mailbox-samsung.h
>
> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> index c8b5c13..bce60ba 100644
> --- a/drivers/mailbox/Kconfig
> +++ b/drivers/mailbox/Kconfig
> @@ -23,6 +23,14 @@ config OMAP_MBOX
>           driver such as CONFIG_OMAP1_MBOX or CONFIG_OMAP2PLUS_MBOX. This
>           enables the common OMAP mailbox framework code.
>
> +config SAMSUNG_MBOX
> +       tristate "Samsung Mailbox Controller"
> +       help
> +         An implementation of the Samsung Interprocessor Communication
> +         Mailbox (IPCM). It is used to send short messages between A57 cores
> +         and the System Control Processor or Network Coprocessor firmware.
> +         Say Y here if you want to use the Samsung IPCM support.
> +
>  config OMAP1_MBOX
>         tristate "OMAP1 Mailbox framework support"
>         depends on ARCH_OMAP1
> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> index 2fa343a..7e83a7d 100644
> --- a/drivers/mailbox/Makefile
> +++ b/drivers/mailbox/Makefile
> @@ -9,3 +9,5 @@ obj-$(CONFIG_OMAP1_MBOX)        += mailbox_omap1.o
>  mailbox_omap1-objs             := mailbox-omap1.o
>  obj-$(CONFIG_OMAP2PLUS_MBOX)   += mailbox_omap2.o
>  mailbox_omap2-objs             := mailbox-omap2.o
> +obj-$(CONFIG_SAMSUNG_MBOX)     += mailbox_samsung.o
> +mailbox_samsung-objs           := mailbox-samsung.o
> diff --git a/drivers/mailbox/mailbox-samsung.c b/drivers/mailbox/mailbox-samsung.c
> new file mode 100644
> index 0000000..903e88a
> --- /dev/null
> +++ b/drivers/mailbox/mailbox-samsung.c
> @@ -0,0 +1,354 @@
> +/*
> + * Copyright 2013 Samsung Electronics Co. Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/jiffies.h>
> +#include <linux/mailbox_controller.h>
> +#include <linux/mailbox-samsung.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +
> +/*
> +   Mailbox Register offsets
> + */
> +
> +/* Offset of tx mailbox data registers */
> +#define MBOX_REG_RXDATA0       0
> +#define MBOX_REG_RXDATA1       4
> +
> +/* offset of tx mailbox lock registers */
> +#define MBOX_REG_RXLOCK                8
> +#define MBOX_REG_RXUNLOCK      12
> +
> +/* offset of tx mailbox interrupt registers */
> +#define MBOX_REG_RXINT         16
> +
> +/* Offset of rx mailbox data registers */
> +#define MBOX_REG_TXDATA0       32
> +#define MBOX_REG_TXDATA1       36
> +
> +/* offset of rx mailbox lock registers */
> +#define MBOX_REG_TXLOCK                40
> +#define MBOX_REG_TXUNLOCK      44
> +
> +/* offset of rx mailbox interrupt registers */
> +#define MBOX_REG_TXINT         48
> +
> +#define MBOX_RX_TX_FULL                1
> +#define MBOX_INT_ENABLE                1
> +
> +#define MBOX_LOCK_RESET                (-1)
> +#define MBOX_WAIT_TIMEOUT      (1)
> +
> +#define RXBUF_LEN      32
> +#define MAX_LINKS      1
> +
> +struct samsung_mlink {
> +       const char *name;
> +       struct ipc_link link;
> +       struct samsung_mbox *smc;
> +       int irq;
> +       void *data;
> +};
> +
> +struct samsung_mbox {
> +       struct device *dev;
> +       void __iomem *ipc_base;
> +       struct ipc_controller ipc_con;
> +       struct mutex lock;
> +       struct samsung_mlink samsung_link[MAX_LINKS];
> +};
> +
> +static inline struct samsung_mlink *to_samsung_mlink(struct ipc_link *plink)
> +{
> +       if (!plink)
> +               return NULL;
> +
> +       return container_of(plink, struct samsung_mlink, link);
> +}
> +
> +static inline struct samsung_mbox *to_samsung_mbox(struct ipc_link *plink)
> +{
> +       if (!plink)
> +               return NULL;
> +
> +       return to_samsung_mlink(plink)->smc;
> +}
> +
> +static void samsung_mbox_read(struct samsung_mlink *link)
> +{
> +       u64 data;
> +       void __iomem *ipc_base = link->smc->ipc_base;
> +
> +       /* Read both words which have information specific to client */
> +       data = readq(ipc_base + MBOX_REG_RXDATA0);
> +       link->data = phys_to_virt((phys_addr_t)data);
> +}
> +
> +static irqreturn_t samsung_ipc_handler(int irq, void *p)
> +{
> +       struct samsung_mlink *plink = p;
> +       void __iomem *ipc_base = plink->smc->ipc_base;
> +
> +       samsung_mbox_read(plink);
> +       ipc_link_received_data(&plink->link, plink->data);
> +
> +       /* Acknowledge the interrupt by clearing the interrupt register */
> +       writel(~MBOX_INT_ENABLE, ipc_base + MBOX_REG_RXINT);
> +       writel(readl(ipc_base + MBOX_REG_RXLOCK),
> +              ipc_base + MBOX_REG_RXUNLOCK);
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static bool samsung_mbox_is_ready(struct ipc_link *link)
> +{
> +       struct samsung_mbox *samsung_mbox = to_samsung_mbox(link);
> +       void __iomem *ipc_base = samsung_mbox->ipc_base;
> +       int ret = true;
> +
> +       mutex_lock(&samsung_mbox->lock);
> +
> +       /* If the RX interrupt register is clear then mbox is empty */
> +       if (readl(ipc_base + MBOX_REG_TXLOCK) &&
> +           readl(ipc_base + MBOX_REG_TXINT))
> +               ret = false;
> +
> +       mutex_unlock(&samsung_mbox->lock);
> +       return ret;
> +}
> +
> +static int samsung_mbox_send_data(struct ipc_link *link, void *msg)
> +{
> +       u32 val;
> +       phys_addr_t addr;
> +       unsigned long timeout;
> +       struct samsung_mbox *samsung_mbox = to_samsung_mbox(link);
> +       void __iomem *ipc_base = samsung_mbox->ipc_base;
> +       int ret = 0;
> +
> +       mutex_lock(&samsung_mbox->lock);
> +
> +       timeout = jiffies + msecs_to_jiffies(MBOX_WAIT_TIMEOUT);
> +
> +       /* Check if the mailbox is busy transmitting data */
> +       if (readl(ipc_base + MBOX_REG_TXLOCK)) {
> +               ret = -EBUSY;
> +               goto unlock_mutex;
> +       }
> +
> +       addr = virt_to_phys(msg);
> +       val = samsung_get_client_id(msg);
> +
> +       /* Lock the mailbox for this transaction with the client id */
> +       writel(val, ipc_base + MBOX_REG_TXLOCK);
> +       do {
> +               if (time_after(jiffies, timeout)) {
> +                       pr_err("cannot aquire the lock\n");
> +                       ret = -EAGAIN;
> +                       goto unlock_mutex;
> +               }
> +       } while (readl(ipc_base + MBOX_REG_TXLOCK) != val);
> +
> +       writeq(addr, ipc_base + MBOX_REG_TXDATA0);
> +       val = readl(ipc_base + MBOX_REG_TXINT);
> +       val |= MBOX_INT_ENABLE;
> +       writel(val, ipc_base + MBOX_REG_TXINT);
> +
> +unlock_mutex:
> +       mutex_unlock(&samsung_mbox->lock);
> +       return ret;
> +}
> +
> +static int samsung_mbox_startup(struct ipc_link *link, void *ignored)
> +{
> +       int ret;
> +       struct samsung_mlink *samsung_link = to_samsung_mlink(link);
> +       struct samsung_mbox *samsung_mbox = to_samsung_mbox(link);
> +       void __iomem *ipc_base = samsung_mbox->ipc_base;
> +       struct device *dev = samsung_mbox->dev;
> +
> +       mutex_lock(&samsung_mbox->lock);
> +
> +       ret = devm_request_irq(dev, samsung_link->irq, samsung_ipc_handler,
> +                                       IRQF_SHARED, link->link_name,
> +                                       samsung_link);
> +       if (unlikely(ret)) {
> +               pr_err("failed to register mailbox interrupt:%d\n", ret);
> +               mutex_unlock(&samsung_mbox->lock);
> +               return ret;
> +       }
> +
> +       /*
> +        * On shutdown irrespective of what is the previous lock id
> +        * clear it. Dont have to do the same for RX lock. Remote
> +        * processor shall handle the RX lock clear on shutdown.
> +        */
> +       writel(~MBOX_INT_ENABLE, ipc_base + MBOX_REG_TXINT);
> +       writel(MBOX_LOCK_RESET, ipc_base + MBOX_REG_TXUNLOCK);
> +
> +       mutex_unlock(&samsung_mbox->lock);
> +       return 0;
> +}
> +
> +static void samsung_mbox_shutdown(struct ipc_link *link)
> +{
> +       struct samsung_mbox *samsung_mbox = to_samsung_mbox(link);
> +       struct samsung_mlink *samsung_link = to_samsung_mlink(link);
> +       void __iomem *ipc_base = samsung_mbox->ipc_base;
> +       struct device *dev = samsung_mbox->dev;
> +
> +       mutex_lock(&samsung_mbox->lock);
> +
> +       /*
> +        * On shutdown irrespective of what is the previous lock id
> +        * clear it. Dont have to do the same for RX lock. Remote
> +        * processor shall handle the RX lock clear on shutdown.
> +        */
> +       writel(~MBOX_INT_ENABLE, ipc_base + MBOX_REG_TXINT);
> +       writel(MBOX_LOCK_RESET, ipc_base + MBOX_REG_TXUNLOCK);
> +
> +       devm_free_irq(dev, samsung_link->irq, samsung_link);
> +
> +       mutex_unlock(&samsung_mbox->lock);
> +}
> +
> +static struct ipc_link_ops samsung_mbox_ops = {
> +       .is_ready = samsung_mbox_is_ready,
> +       .send_data = samsung_mbox_send_data,
> +       .startup = samsung_mbox_startup,
> +       .shutdown = samsung_mbox_shutdown,
> +};
> +
> +static int samsung_mbox_probe(struct platform_device *pdev)
> +{
> +       struct samsung_mbox *samsung_mbox;
> +       struct samsung_mlink *mbox_link;
> +       struct resource res;
> +       struct ipc_link **link;
> +       int loop, count, ret = 0;
> +       struct device_node *node = pdev->dev.of_node;
> +
> +       if (!node) {
> +               dev_err(&pdev->dev, "driver doesnt support"
> +                               "non-dt devices\n");
> +               return -ENODEV;
> +       }
> +
> +       count = of_property_count_strings(node,
> +                               "samsung,mbox-names");
> +       if (count <= 0) {
> +               dev_err(&pdev->dev, "no mbox devices found\n");
> +               return -ENODEV;
> +       }
> +
> +       samsung_mbox = devm_kzalloc(&pdev->dev,
> +                       sizeof(struct samsung_mbox), GFP_KERNEL);
> +       if (IS_ERR(samsung_mbox))
> +               return PTR_ERR(samsung_mbox);
> +
> +       link = devm_kzalloc(&pdev->dev, (count + 1) * sizeof(*link),
> +                       GFP_KERNEL);
> +       if (IS_ERR(link))
> +               return PTR_ERR(link);
> +
> +       samsung_mbox->dev = &pdev->dev;
> +
> +       samsung_mbox->ipc_base = devm_ioremap_resource(&pdev->dev, &res);
> +       if (IS_ERR(samsung_mbox->ipc_base))
> +               return PTR_ERR(samsung_mbox->ipc_base);
> +
> +       for (loop = 0; loop < count; loop++) {
> +               mbox_link = &samsung_mbox->samsung_link[loop];
> +
> +               ret = of_address_to_resource(node, 0, &res);
> +               if (ret < 0)
> +                       return ret;

Sorry posted a wrong version. Above code should be moved before ioremap

> +
> +               mbox_link->irq = irq_of_parse_and_map(node, loop);
> +               if (!mbox_link->irq)
> +                       return -ENODEV;
> +
> +               mbox_link->smc = samsung_mbox;
> +               link[loop] = &mbox_link->link;
> +
> +               if (of_property_read_string_index(node, "samsung,mbox-names",
> +                                                 loop, &mbox_link->name)) {
> +                       dev_err(&pdev->dev,
> +                               "mbox_name [%d] read failed\n", loop);
> +                       return -ENODEV;
> +               }
> +
> +               snprintf(link[loop]->link_name, 16, mbox_link->name);
> +       }
> +
> +       link[loop] = NULL; /* Terminating link */
> +
> +       mutex_init(&samsung_mbox->lock);
> +       samsung_mbox->ipc_con.links = link;
> +       samsung_mbox->ipc_con.txdone_irq = false;
> +       samsung_mbox->ipc_con.txdone_poll = true;
> +       samsung_mbox->ipc_con.txpoll_period = 1;
> +       samsung_mbox->ipc_con.ops = &samsung_mbox_ops;
> +       snprintf(samsung_mbox->ipc_con.controller_name, 16, "samsung_mbox");
> +
> +       ret = ipc_links_register(&samsung_mbox->ipc_con);
> +       if (ret) {
> +               pr_err("%s: IPC Link register failed\n", __func__);
> +               return ret;
> +       }
> +
> +       platform_set_drvdata(pdev, samsung_mbox);
> +
> +       return ret;
> +}
> +
> +static int samsung_mbox_remove(struct platform_device *pdev)
> +{
> +       struct samsung_mbox *samsung_mbox = platform_get_drvdata(pdev);
> +
> +       ipc_links_unregister(&samsung_mbox->ipc_con);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id mailbox_smc_match[] = {
> +       { .compatible = "samsung,gh7-mailbox" },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, mailbox_smc_match);
> +
> +static struct platform_driver samsung_mbox_driver = {
> +       .probe  = samsung_mbox_probe,
> +       .remove = samsung_mbox_remove,
> +       .driver = {
> +               .name = "gh7-mailbox",
> +               .of_match_table = mailbox_smc_match,
> +       },
> +};
> +
> +module_platform_driver(samsung_mbox_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Samsung Mailbox Driver");
> +MODULE_AUTHOR("Girish K S <ks.giri@samsung.com>");
> +MODULE_ALIAS("platform:gh7-mailbox");
> diff --git a/include/linux/mailbox-samsung.h b/include/linux/mailbox-samsung.h
> new file mode 100644
> index 0000000..c3f2d2c
> --- /dev/null
> +++ b/include/linux/mailbox-samsung.h
> @@ -0,0 +1,112 @@
> +
> +/*
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef _MAILBOX_SAMSUNG_H_
> +#define _MAILBOX_SAMSUNG_H_
> +
> +#include <linux/slab.h>
> +
> +#define SAMSUNG_MB_RESP_LEN    (4096)
> +#define SAMSUNG_MB_DRV_ID      (0xa5)
> +#define SAMSUNG_MB_DRV_LEN     (0x08)
> +#define REQUEST_PAGE_ORDER     (16)
> +
> +#define IRQ_TX                 (1)
> +#define IRQ_RX                 (2)
> +
> +struct samsung_packet {
> +       u8 version;
> +       u8 client_id;
> +       u8 remote_id;
> +       u16 req_len;
> +       u64 resp_addr;
> +       u16 resp_len;
> +       u8 reserved[16];
> +       u8 checksum;
> +};
> +
> +static inline unsigned long get_alligned_buffer(void)
> +{
> +       return __get_free_pages(GFP_KERNEL, REQUEST_PAGE_ORDER);
> +}
> +
> +static inline void put_alligned_buffer(unsigned long addr)
> +{
> +       free_pages(addr, REQUEST_PAGE_ORDER);
> +}
> +
> +static inline void samsung_put_version(void *buf, u8 version)
> +{
> +       struct samsung_packet *data = buf;
> +       data->version = version;
> +}
> +
> +static inline void samsung_put_client_id(void *buf, u8 id)
> +{
> +       struct samsung_packet *data = buf;
> +       data->client_id = (SAMSUNG_MB_DRV_ID << SAMSUNG_MB_DRV_LEN) | id;
> +}
> +
> +static inline void samsung_put_remote_id(void *buf, u8 id)
> +{
> +       struct samsung_packet *data = buf;
> +       data->remote_id = id;
> +}
> +
> +static inline void samsung_put_req_len(void *buf, u16 len)
> +{
> +       struct samsung_packet *data = buf;
> +       data->req_len = len;
> +}
> +
> +static inline void samsung_put_resp_addr(void *buf, u64 addr)
> +{
> +       struct samsung_packet *data = buf;
> +       data->resp_addr = addr;
> +}
> +
> +static inline void samsung_put_checksum(void *buf, u8 checksum)
> +{
> +       struct samsung_packet *data = buf;
> +       data->checksum = checksum;
> +}
> +
> +static inline int samsung_get_resp_size(void)
> +{
> +       /* The response buffer length can be max upto 4kb */
> +       return SAMSUNG_MB_RESP_LEN;
> +}
> +
> +static inline int samsung_get_resp_len(void *buf)
> +{
> +       struct samsung_packet *data = buf;
> +
> +       /* The actual lenght of response buffer is got from remote packet */
> +       return data->resp_len;
> +}
> +
> +static inline u64 samsung_get_resp_addr(void *buf)
> +{
> +       struct samsung_packet *data = buf;
> +       return data->resp_addr;
> +}
> +
> +static inline int samsung_get_client_id(const void *buf)
> +{
> +       const struct samsung_packet *data = buf;
> +       return data->client_id;
> +}
> +
> +#endif
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/2] mailbox: samsung: added support for samsung mailbox

[Joe Perches]

On Tue, 2014-03-18 at 15:52 +0530, Girish KS wrote:
> On Mon, Mar 17, 2014 at 5:33 PM, Girish K S <ks.giri@samsung.com> wrote:

Hi, just some trivial notes about the patch

> > diff --git a/drivers/mailbox/mailbox-samsung.c b/drivers/mailbox/mailbox-samsung.c
[]
> > @@ -0,0 +1,354 @@
> > +/*
> > + * Copyright 2013 Samsung Electronics Co. Ltd.
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope 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.
> > + *
> > + * You should have received a copy of the GNU General Public License along with
> > + * this program.  If not, see <http://www.gnu.org/licenses/>.
> > + */

Please add

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

before any #include so that error messages are
prefixed in the output with "samsung-mailbox: "
and the reader of the log has an idea what subsystem
is emitting messages.

> > +#include <linux/err.h>
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
[]
> > +static int samsung_mbox_send_data(struct ipc_link *link, void *msg)
> > +{
[]
> > +       /* Lock the mailbox for this transaction with the client id */
> > +       writel(val, ipc_base + MBOX_REG_TXLOCK);
> > +       do {
> > +               if (time_after(jiffies, timeout)) {
> > +                       pr_err("cannot aquire the lock\n");

This would be prefixed appropriately via pr_fmt.
Also, there's a typo of aquire here: acquire

> > +static int samsung_mbox_startup(struct ipc_link *link, void *ignored)
> > +{
[]
> > +       ret = devm_request_irq(dev, samsung_link->irq, samsung_ipc_handler,
> > +                                       IRQF_SHARED, link->link_name,
> > +                                       samsung_link);
> > +       if (unlikely(ret)) {
> > +               pr_err("failed to register mailbox interrupt:%d\n", ret);

Here too it would be prefixed

> > +static int samsung_mbox_probe(struct platform_device *pdev)
> > +{
> > +       struct samsung_mbox *samsung_mbox;
> > +       struct samsung_mlink *mbox_link;
> > +       struct resource res;
> > +       struct ipc_link **link;
> > +       int loop, count, ret = 0;
> > +       struct device_node *node = pdev->dev.of_node;
> > +
> > +       if (!node) {
> > +               dev_err(&pdev->dev, "driver doesnt support"
> > +                               "non-dt devices\n");

Please coalesce the format fragments.

You would notice the missing space between
"support" and "non-dt"

		dev_err(&pdev->dev, "driver does not support non-dt devices\n");

It's OK to exceed 80 columns for these format strings.

> > diff --git a/include/linux/mailbox-samsung.h b/include/linux/mailbox-samsung.h
[]
> > +static inline unsigned long get_alligned_buffer(void)

Typo: There's one l in aligned

> > +{
> > +       return __get_free_pages(GFP_KERNEL, REQUEST_PAGE_ORDER);
> > +}
> > +
> > +static inline void put_alligned_buffer(unsigned long addr)
> > +{

here too

> > +       free_pages(addr, REQUEST_PAGE_ORDER);
> > +}
> > +

Re: [PATCH 1/2] mailbox: samsung: added support for samsung mailbox

[Girish KS]

On Tue, Mar 18, 2014 at 4:26 PM, Joe Perches <joe@perches.com> wrote:
> On Tue, 2014-03-18 at 15:52 +0530, Girish KS wrote:
>> On Mon, Mar 17, 2014 at 5:33 PM, Girish K S <ks.giri@samsung.com> wrote:
>
> Hi, just some trivial notes about the patch
>
>> > diff --git a/drivers/mailbox/mailbox-samsung.c b/drivers/mailbox/mailbox-samsung.c
> []
>> > @@ -0,0 +1,354 @@
>> > +/*
>> > + * Copyright 2013 Samsung Electronics Co. Ltd.
>> > + *
>> > + * This program is free software; you can redistribute it and/or modify it
>> > + * under the terms and conditions of the GNU General Public License,
>> > + * version 2, as published by the Free Software Foundation.
>> > + *
>> > + * This program is distributed in the hope 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.
>> > + *
>> > + * You should have received a copy of the GNU General Public License along with
>> > + * this program.  If not, see <http://www.gnu.org/licenses/>.
>> > + */
>
> Please add
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> before any #include so that error messages are
> prefixed in the output with "samsung-mailbox: "
> and the reader of the log has an idea what subsystem
> is emitting messages.
>
Sure will do it

>> > +#include <linux/err.h>
>> > +#include <linux/module.h>
>> > +#include <linux/slab.h>
> []
>> > +static int samsung_mbox_send_data(struct ipc_link *link, void *msg)
>> > +{
> []
>> > +       /* Lock the mailbox for this transaction with the client id */
>> > +       writel(val, ipc_base + MBOX_REG_TXLOCK);
>> > +       do {
>> > +               if (time_after(jiffies, timeout)) {
>> > +                       pr_err("cannot aquire the lock\n");
>
> This would be prefixed appropriately via pr_fmt.
> Also, there's a typo of aquire here: acquire

OK

>
>> > +static int samsung_mbox_startup(struct ipc_link *link, void *ignored)
>> > +{
> []
>> > +       ret = devm_request_irq(dev, samsung_link->irq, samsung_ipc_handler,
>> > +                                       IRQF_SHARED, link->link_name,
>> > +                                       samsung_link);
>> > +       if (unlikely(ret)) {
>> > +               pr_err("failed to register mailbox interrupt:%d\n", ret);
>
> Here too it would be prefixed

ok

>
>> > +static int samsung_mbox_probe(struct platform_device *pdev)
>> > +{
>> > +       struct samsung_mbox *samsung_mbox;
>> > +       struct samsung_mlink *mbox_link;
>> > +       struct resource res;
>> > +       struct ipc_link **link;
>> > +       int loop, count, ret = 0;
>> > +       struct device_node *node = pdev->dev.of_node;
>> > +
>> > +       if (!node) {
>> > +               dev_err(&pdev->dev, "driver doesnt support"
>> > +                               "non-dt devices\n");
>
> Please coalesce the format fragments.
>
> You would notice the missing space between
> "support" and "non-dt"

ok

>
>                 dev_err(&pdev->dev, "driver does not support non-dt devices\n");
>
> It's OK to exceed 80 columns for these format strings.
>
>> > diff --git a/include/linux/mailbox-samsung.h b/include/linux/mailbox-samsung.h
> []
>> > +static inline unsigned long get_alligned_buffer(void)
>
> Typo: There's one l in aligned

will do it

>
>> > +{
>> > +       return __get_free_pages(GFP_KERNEL, REQUEST_PAGE_ORDER);
>> > +}
>> > +
>> > +static inline void put_alligned_buffer(unsigned long addr)
>> > +{
>
> here too
>
>> > +       free_pages(addr, REQUEST_PAGE_ORDER);
>> > +}
>> > +
>
>

Re: [PATCH 2/2] arm64: dts: exynos: added mailbox node

[Girish KS]

On Mon, Mar 17, 2014 at 7:36 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Mon, Mar 17, 2014 at 12:03:59PM +0000, Girish K S wrote:
>> This patch adds the dt node for the mailbox IP
>>
>> Signed-off-by: Girish K S <ks.giri@samsung.com>
>>
>> Change-Id: I35e45e9a62592887a84a909aee54f259a2f731fa
>> ---
>>  .../bindings/mailbox/samsung-mailbox.txt           |   24 +++++++
>>  arch/arm64/boot/dts/samsung-gh7.dtsi               |   66 ++++++++++++++++++++
>>  arch/arm64/boot/dts/samsung-ssdk-gh7.dts           |    3 +
>>  3 files changed, 93 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mailbox/samsung-mailbox.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mailbox/samsung-mailbox.txt b/Documentation/devicetree/bindings/mailbox/samsung-mailbox.txt
>> new file mode 100644
>> index 0000000..1908d71
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mailbox/samsung-mailbox.txt
>> @@ -0,0 +1,24 @@
>> +
>> +Samsung Mailbox Driver
>> +
>> +Required properties:
>> +- compatible:                Should be one of the following,
>> +                         "samsung,gh7-mailbox" for
>> +                             Samsung GH7 SoC series
>> +                         "samsung,exynos-mailbox" for
>> +                             exynosx SoC series
>> +- reg:                       Contains the mailbox register address range (base address
>> +                     and length)
>> +- interrupts:                Contains the interrupt information for the mailbox
>> +                     device.
>
> How many interrupts?
just one receive interrupt
I will update this detail in binding doc
>
> What are they for?
It is only for receive from remote processor
>
>> +- samsung,mbox-names:        Array of the names of the mailboxes
>
> Juding by the code there is one name per reg entry, but the description
> above implies a single entry (as all the dt fragments have).
>
> What values are expected? What is the consumer of these values? What are
> they used for?
These names are used to mention the links in the platform. tx and rx
link via mailbox.
i can mention of 2 consumers now. the ipmi driver and the cpu freq
driver. They are used
to exchange information with the remote processor
>
>> +
>> +Example:
>> +
>> +/* Samsung GH7 SoC */
>> +mailbox@100a0000 {
>> +     compatible = "samsung,gh7-mailbox";
>> +     reg = <0x0 0x100a0000 0x0 0x1000>;
>> +     interrupts = <0 310 0>;
>> +     samsung,mbox-names = "a7q-scp";
>> +};
>> diff --git a/arch/arm64/boot/dts/samsung-gh7.dtsi b/arch/arm64/boot/dts/samsung-gh7.dtsi
>> index c3610bd..be4cce9 100644
>> --- a/arch/arm64/boot/dts/samsung-gh7.dtsi
>> +++ b/arch/arm64/boot/dts/samsung-gh7.dtsi
>> @@ -107,5 +107,71 @@
>>                       interrupts = <0 420 0>;
>>                       arm,primecell-periphid = <0x341011>; /* HACK */
>
> This looks odd.
>
> What tree is this against?

Sorry this was based on a internal tree. Will base it on Kukjin's gh7 base patch

>
> Cheers,
> Mark.
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] arm64: dts: exynos: added mailbox node

[Arnd Bergmann]

On Monday 17 March 2014, Jassi Brar wrote:
> On Mon, Mar 17, 2014 at 5:45 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Monday 17 March 2014 17:33:59 Girish K S wrote:
> >> +Samsung Mailbox Driver
> >> +
> >> +Required properties:
> >> +- compatible:          Should be one of the following,
> >> +                           "samsung,gh7-mailbox" for
> >> +                               Samsung GH7 SoC series
> >> +                           "samsung,exynos-mailbox" for
> >> +                               exynosx SoC series
> >> +- reg:                 Contains the mailbox register address range (base address
> >> +                       and length)
> >> +- interrupts:          Contains the interrupt information for the mailbox
> >> +                       device.
> >> +- samsung,mbox-names:  Array of the names of the mailboxes
> >>
> >
> > I think we should not allow new mailbox drivers that don't conform to
> > the framework that is currently under discussion. In particular, this
> > means don't do a "samsung,mbox-names" property. The current consensus
> > seems to be to have a #mbox-cells" property that allows to pass extra
> > parameters from the client driver, and uses an "mboxes" property
> > to reference the mailbox provided.
> >
> > It would be good if you follow up for the subsystem discussion and
> > ensure it gets merged in time, and supports all the use cases you are
> > interested in. The interface is not entirely nailed down yet, so
> > it's a good time to contribute. However, I can already promise that
> > it won't use matching by strings.
> >
> I was just about to submit next revision of framework that this
> Samsung driver conforms to. Now I think I need some expert opinion ...
> (sorry if following is repetition)
> 
>   As Kumar Gala pointed out in the other thread, the mailbox/ipc chain
> is going to be _very_ platform specific so much so that one rethinks
> if a common api is even warranted : because the client driver will be
> different even if just the remote api(which is going to be
> proprietary) changes, keeping everything else same. For example, a
> client driver for Highbank is highly unlikely to be reusable on
> Exynos, even if both used the same mailbox controller. By my limited
> foresight, mailbox assignment via DT doesn't bring us much benefit but
> only ritual code.

I don't see what that would change. Doing a cross-device binding in DT
is hard, as we see by lots of people (e.g. the above "samsung,mbox-names"
crap) getting it wrong. If we do it properly once, everyone can use the
same binding, and common code to parse the connection between the
mailbox driver and the user.

>   Perhaps the mailbox controller driver should name its links as it
> wants. By how the remote works with the mailbox links, the client
> driver asks for a specific mailbox link (which maybe a hardcoded
> string in the driver or be gotten alongside other data via client's
> DT) ?

I don't see why we should do it any different from the other bindings.
Let's just stick with mboxes/mbox-names or mailboxes/mailbox-names
if you prefer.

>   IOW we can't have a generic API/DT-bindings that could get us
> reusable client drivers. But only common framework/code that would
> otherwise be duplicated by every platform.

That is a major benefit though.
Also even if most drivers won't work across multiple platforms, there
is still a reasonable chance that /some/ drivers will.

	Arnd

Re: [PATCH 2/2] arm64: dts: exynos: added mailbox node

[Jassi Brar]

On Wed, Mar 19, 2014 at 11:16 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 17 March 2014, Jassi Brar wrote:

>>   Perhaps the mailbox controller driver should name its links as it
>> wants. By how the remote works with the mailbox links, the client
>> driver asks for a specific mailbox link (which maybe a hardcoded
>> string in the driver or be gotten alongside other data via client's
>> DT) ?
>
> I don't see why we should do it any different from the other bindings.
> Let's just stick with mboxes/mbox-names or mailboxes/mailbox-names
> if you prefer.
>
>>   IOW we can't have a generic API/DT-bindings that could get us
>> reusable client drivers. But only common framework/code that would
>> otherwise be duplicated by every platform.
>
> That is a major benefit though.
> Also even if most drivers won't work across multiple platforms, there
> is still a reasonable chance that /some/ drivers will.
>
It seems those /some/ drivers will have to work with everything same
but the channel name (which the client could get from its DT node when
the second platform appears).

Anyways, I accept your opinion and I will specify DT binding for
mailbox<->client in next revision.

Also I just read the new requirement of 'peek_message' for QMTM
driver, which isn't supported atm but should be easy.

Thanks
-Jassi

Re: [PATCH 2/2] arm64: dts: exynos: added mailbox node

[Arnd Bergmann]

On Thursday 20 March 2014 21:39:56 Jassi Brar wrote:
> On Wed, Mar 19, 2014 at 11:16 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Monday 17 March 2014, Jassi Brar wrote:
> 
> >>   Perhaps the mailbox controller driver should name its links as it
> >> wants. By how the remote works with the mailbox links, the client
> >> driver asks for a specific mailbox link (which maybe a hardcoded
> >> string in the driver or be gotten alongside other data via client's
> >> DT) ?
> >
> > I don't see why we should do it any different from the other bindings.
> > Let's just stick with mboxes/mbox-names or mailboxes/mailbox-names
> > if you prefer.
> >
> >>   IOW we can't have a generic API/DT-bindings that could get us
> >> reusable client drivers. But only common framework/code that would
> >> otherwise be duplicated by every platform.
> >
> > That is a major benefit though.
> > Also even if most drivers won't work across multiple platforms, there
> > is still a reasonable chance that /some/ drivers will.
> >
> It seems those /some/ drivers will have to work with everything same
> but the channel name (which the client could get from its DT node when
> the second platform appears).

Why would you ever have varying channel names? I would expect that
the name is always fixed in the binding of the client driver, like
we do for clocks or interrupts for instance.

	Arnd

Re: [PATCH 2/2] arm64: dts: exynos: added mailbox node

[Jassi Brar]

On Thu, Mar 20, 2014 at 9:42 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 20 March 2014 21:39:56 Jassi Brar wrote:
>> On Wed, Mar 19, 2014 at 11:16 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Monday 17 March 2014, Jassi Brar wrote:
>>
>> >>   Perhaps the mailbox controller driver should name its links as it
>> >> wants. By how the remote works with the mailbox links, the client
>> >> driver asks for a specific mailbox link (which maybe a hardcoded
>> >> string in the driver or be gotten alongside other data via client's
>> >> DT) ?
>> >
>> > I don't see why we should do it any different from the other bindings.
>> > Let's just stick with mboxes/mbox-names or mailboxes/mailbox-names
>> > if you prefer.
>> >
>> >>   IOW we can't have a generic API/DT-bindings that could get us
>> >> reusable client drivers. But only common framework/code that would
>> >> otherwise be duplicated by every platform.
>> >
>> > That is a major benefit though.
>> > Also even if most drivers won't work across multiple platforms, there
>> > is still a reasonable chance that /some/ drivers will.
>> >
>> It seems those /some/ drivers will have to work with everything same
>> but the channel name (which the client could get from its DT node when
>> the second platform appears).
>
> Why would you ever have varying channel names? I would expect that
> the name is always fixed in the binding of the client driver, like
> we do for clocks or interrupts for instance.
>
I meant across platforms and without generic DT bindings for
mailboxes. Not during runtime.

-jassi