linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/2] Renesas IPMMU driver for sh7372
@ 2012-10-15  8:34 Hideki EIRAKU
  2012-10-15  8:34 ` [PATCH v4 1/2] iommu/shmobile: Add iommu driver for Renesas IPMMU modules Hideki EIRAKU
                   ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Hideki EIRAKU @ 2012-10-15  8:34 UTC (permalink / raw)
  To: Paul Mundt, Magnus Damm, Russell King, Simon Horman, Laurent Pinchart
  Cc: linux-sh, linux-arm-kernel, linux-kernel, Marek Szyprowski,
	Katsuya MATSUBARA, Damian Hobson-Garcia, Hideki EIRAKU

This is the Renesas IPMMU driver, IOMMU API implementation and IPMMU
device support for sh7372 (AP4EVB and Mackerel).

The IPMMU module supports the MMU function and the PMB function.  The
MMU function provides address translation by pagetable compatible with
ARMv6.  The PMB function provides address translation including
tile-linear translation.  This is implementation of the MMU function.

v4:
- Rebased on 3.7-rc1.
- Use address space size instead of page table size in config.
- Use Kconfig default value instead of #ifdef-#define-#endif.
v3:
- Rebased on 3.6-rc5.
- Simplify configs.  SHMOBILE_IPMMU is now selected by setting
  SHMOBILE_IOMMU.
- Remove weak symbols.
- Use drvdata to store private driver data.
- Make a function for writing to a register of IPMMU.
- Add a lock to accessing the tlb_enabled member.
- Make unmap work correctly with size larger than map size.
- Free L2 page table when 1MiB page is mapped or unmapped.
- Add VEU devices as IP blocks on the ICB.
v2:
- Rebased on v3.6-rc1.
- Make variable names clear.
- Page table size can now be selected by config.

Hideki EIRAKU (2):
  iommu/shmobile: Add iommu driver for Renesas IPMMU modules
  ARM: mach-shmobile: sh7372: Add IPMMU device

 arch/arm/mach-shmobile/Kconfig              |    6 +
 arch/arm/mach-shmobile/Makefile             |    3 +
 arch/arm/mach-shmobile/board-ap4evb.c       |    5 +
 arch/arm/mach-shmobile/board-mackerel.c     |    5 +
 arch/arm/mach-shmobile/include/mach/ipmmu.h |   16 ++
 arch/arm/mach-shmobile/ipmmu.c              |  150 ++++++++++++
 arch/arm/mach-shmobile/setup-sh7372.c       |   26 ++
 drivers/iommu/Kconfig                       |   56 +++++
 drivers/iommu/Makefile                      |    1 +
 drivers/iommu/shmobile-iommu.c              |  352 +++++++++++++++++++++++++++
 10 files changed, 620 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-shmobile/include/mach/ipmmu.h
 create mode 100644 arch/arm/mach-shmobile/ipmmu.c
 create mode 100644 drivers/iommu/shmobile-iommu.c


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

* [PATCH v4 1/2] iommu/shmobile: Add iommu driver for Renesas IPMMU modules
  2012-10-15  8:34 [PATCH v4 0/2] Renesas IPMMU driver for sh7372 Hideki EIRAKU
@ 2012-10-15  8:34 ` Hideki EIRAKU
  2012-12-10 15:55   ` Laurent Pinchart
  2012-10-15  8:34 ` [PATCH v4 2/2] ARM: mach-shmobile: sh7372: Add IPMMU device Hideki EIRAKU
  2012-12-16 17:25 ` [PATCH/WIP/RFC 00/14] Renesas IPMMU driver work in progress Laurent Pinchart
  2 siblings, 1 reply; 23+ messages in thread
From: Hideki EIRAKU @ 2012-10-15  8:34 UTC (permalink / raw)
  To: Paul Mundt, Magnus Damm, Russell King, Simon Horman, Laurent Pinchart
  Cc: linux-sh, linux-arm-kernel, linux-kernel, Marek Szyprowski,
	Katsuya MATSUBARA, Damian Hobson-Garcia, Hideki EIRAKU

This is the Renesas IPMMU driver and IOMMU API implementation.

The IPMMU module supports the MMU function and the PMB function.  The
MMU function provides address translation by pagetable compatible with
ARMv6.  The PMB function provides address translation including
tile-linear translation.  This patch implements the MMU function.

The iommu driver does not register a platform driver directly because:
- the register space of the MMU function and the PMB function
  have a common register (used for settings flush), so they should ideally
  have a way to appropriately share this register.
- the MMU function uses the IOMMU API while the PMB function does not.
- the two functions may be used independently.

Signed-off-by: Hideki EIRAKU <hdk@igel.co.jp>
---
 arch/arm/mach-shmobile/Kconfig              |    6 +
 arch/arm/mach-shmobile/Makefile             |    3 +
 arch/arm/mach-shmobile/include/mach/ipmmu.h |   16 ++
 arch/arm/mach-shmobile/ipmmu.c              |  150 ++++++++++++
 drivers/iommu/Kconfig                       |   56 +++++
 drivers/iommu/Makefile                      |    1 +
 drivers/iommu/shmobile-iommu.c              |  352 +++++++++++++++++++++++++++
 7 files changed, 584 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-shmobile/include/mach/ipmmu.h
 create mode 100644 arch/arm/mach-shmobile/ipmmu.c
 create mode 100644 drivers/iommu/shmobile-iommu.c

diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
index 8ae100c..de69ab3 100644
--- a/arch/arm/mach-shmobile/Kconfig
+++ b/arch/arm/mach-shmobile/Kconfig
@@ -210,6 +210,12 @@ endmenu
 config SH_CLK_CPG
 	bool
 
+config SHMOBILE_IPMMU
+	bool
+
+config SHMOBILE_IPMMU_TLB
+	bool
+
 source "drivers/sh/Kconfig"
 
 endif
diff --git a/arch/arm/mach-shmobile/Makefile b/arch/arm/mach-shmobile/Makefile
index fe2c97c..4ffba9d 100644
--- a/arch/arm/mach-shmobile/Makefile
+++ b/arch/arm/mach-shmobile/Makefile
@@ -60,3 +60,6 @@ obj-$(CONFIG_MACH_KZM9G)	+= board-kzm9g.o
 # Framework support
 obj-$(CONFIG_SMP)		+= $(smp-y)
 obj-$(CONFIG_GENERIC_GPIO)	+= $(pfc-y)
+
+# IPMMU/IPMMUI
+obj-$(CONFIG_SHMOBILE_IPMMU)	+= ipmmu.o
diff --git a/arch/arm/mach-shmobile/include/mach/ipmmu.h b/arch/arm/mach-shmobile/include/mach/ipmmu.h
new file mode 100644
index 0000000..ede2f0b
--- /dev/null
+++ b/arch/arm/mach-shmobile/include/mach/ipmmu.h
@@ -0,0 +1,16 @@
+#ifdef CONFIG_SHMOBILE_IPMMU_TLB
+void ipmmu_tlb_flush(struct device *ipmmu_dev);
+void ipmmu_tlb_set(struct device *ipmmu_dev, unsigned long phys, int size,
+		   int asid);
+void ipmmu_add_device(struct device *dev);
+int ipmmu_iommu_init(struct device *dev);
+#else
+static inline void ipmmu_add_device(struct device *dev)
+{
+}
+
+static int ipmmu_iommu_init(struct device *dev)
+{
+	return -EINVAL;
+}
+#endif
diff --git a/arch/arm/mach-shmobile/ipmmu.c b/arch/arm/mach-shmobile/ipmmu.c
new file mode 100644
index 0000000..72cacb9
--- /dev/null
+++ b/arch/arm/mach-shmobile/ipmmu.c
@@ -0,0 +1,150 @@
+/*
+ * IPMMU/IPMMUI
+ * Copyright (C) 2012  Hideki EIRAKU
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/slab.h>
+#include <mach/ipmmu.h>
+
+#define IMCTR1 0x000
+#define IMCTR2 0x004
+#define IMASID 0x010
+#define IMTTBR 0x014
+#define IMTTBCR 0x018
+
+#define IMCTR1_TLBEN (1 << 0)
+#define IMCTR1_FLUSH (1 << 1)
+
+struct ipmmu_priv {
+	void __iomem *ipmmu_base;
+	int tlb_enabled;
+	struct mutex flush_lock;
+};
+
+static void ipmmu_reg_write(struct ipmmu_priv *priv, unsigned long reg_off,
+			    unsigned long data)
+{
+	iowrite32(data, priv->ipmmu_base + reg_off);
+}
+
+void ipmmu_tlb_flush(struct device *dev)
+{
+	struct ipmmu_priv *priv;
+
+	if (!dev)
+		return;
+	priv = dev_get_drvdata(dev);
+	mutex_lock(&priv->flush_lock);
+	if (priv->tlb_enabled)
+		ipmmu_reg_write(priv, IMCTR1, IMCTR1_FLUSH | IMCTR1_TLBEN);
+	else
+		ipmmu_reg_write(priv, IMCTR1, IMCTR1_FLUSH);
+	mutex_unlock(&priv->flush_lock);
+}
+
+void ipmmu_tlb_set(struct device *dev, unsigned long phys, int size, int asid)
+{
+	struct ipmmu_priv *priv;
+
+	if (!dev)
+		return;
+	priv = dev_get_drvdata(dev);
+	mutex_lock(&priv->flush_lock);
+	switch (size) {
+	default:
+		priv->tlb_enabled = 0;
+		break;
+	case 0x2000:
+		ipmmu_reg_write(priv, IMTTBCR, 1);
+		priv->tlb_enabled = 1;
+		break;
+	case 0x1000:
+		ipmmu_reg_write(priv, IMTTBCR, 2);
+		priv->tlb_enabled = 1;
+		break;
+	case 0x800:
+		ipmmu_reg_write(priv, IMTTBCR, 3);
+		priv->tlb_enabled = 1;
+		break;
+	case 0x400:
+		ipmmu_reg_write(priv, IMTTBCR, 4);
+		priv->tlb_enabled = 1;
+		break;
+	case 0x200:
+		ipmmu_reg_write(priv, IMTTBCR, 5);
+		priv->tlb_enabled = 1;
+		break;
+	case 0x100:
+		ipmmu_reg_write(priv, IMTTBCR, 6);
+		priv->tlb_enabled = 1;
+		break;
+	case 0x80:
+		ipmmu_reg_write(priv, IMTTBCR, 7);
+		priv->tlb_enabled = 1;
+		break;
+	}
+	ipmmu_reg_write(priv, IMTTBR, phys);
+	ipmmu_reg_write(priv, IMASID, asid);
+	mutex_unlock(&priv->flush_lock);
+}
+
+static int __devinit ipmmu_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	struct ipmmu_priv *priv;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "cannot get platform resources\n");
+		return -ENOENT;
+	}
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		dev_err(&pdev->dev, "cannot allocate device data\n");
+		return -ENOMEM;
+	}
+	mutex_init(&priv->flush_lock);
+	priv->ipmmu_base = ioremap_nocache(res->start, resource_size(res));
+	if (!priv->ipmmu_base) {
+		dev_err(&pdev->dev, "ioremap_nocache failed\n");
+		kfree(priv);
+		return -ENOMEM;
+	}
+	platform_set_drvdata(pdev, priv);
+	ipmmu_reg_write(priv, IMCTR1, 0x0); /* disable TLB */
+	ipmmu_reg_write(priv, IMCTR2, 0x0); /* disable PMB */
+	ipmmu_iommu_init(&pdev->dev);
+	return 0;
+}
+
+static struct platform_driver ipmmu_driver = {
+	.probe = ipmmu_probe,
+	.driver = {
+		.owner = THIS_MODULE,
+		.name = "ipmmu",
+	},
+};
+
+static int __init ipmmu_init(void)
+{
+	return platform_driver_register(&ipmmu_driver);
+}
+subsys_initcall(ipmmu_init);
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index e39f9db..265829f 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -187,4 +187,60 @@ config EXYNOS_IOMMU_DEBUG
 
 	  Say N unless you need kernel log message for IOMMU debugging
 
+config SHMOBILE_IOMMU
+	bool "IOMMU for Renesas IPMMU/IPMMUI"
+	default n
+	select IOMMU_API
+	select ARM_DMA_USE_IOMMU
+	select SHMOBILE_IPMMU
+	select SHMOBILE_IPMMU_TLB
+
+choice
+	prompt "IPMMU/IPMMUI address space size"
+	default SHMOBILE_IOMMU_ADDRSIZE_2048MB
+	depends on SHMOBILE_IOMMU
+	help
+	  This option sets IPMMU/IPMMUI address space size by
+	  adjusting the 1st level page table size. The page table size
+	  is calculated as follows:
+
+	      page table size = number of page table entries * 4 bytes
+	      number of page table entries = address space size / 1 MiB
+
+	  For example, when the address space size is 2048 MiB, the
+	  1st level page table size is 8192 bytes.
+
+	config SHMOBILE_IOMMU_ADDRSIZE_2048MB
+		bool "2 GiB"
+
+	config SHMOBILE_IOMMU_ADDRSIZE_1024MB
+		bool "1 GiB"
+
+	config SHMOBILE_IOMMU_ADDRSIZE_512MB
+		bool "512 MiB"
+
+	config SHMOBILE_IOMMU_ADDRSIZE_256MB
+		bool "256 MiB"
+
+	config SHMOBILE_IOMMU_ADDRSIZE_128MB
+		bool "128 MiB"
+
+	config SHMOBILE_IOMMU_ADDRSIZE_64MB
+		bool "64 MiB"
+
+	config SHMOBILE_IOMMU_ADDRSIZE_32MB
+		bool "32 MiB"
+
+endchoice
+
+config SHMOBILE_IOMMU_L1SIZE
+	int
+	default 8192 if SHMOBILE_IOMMU_ADDRSIZE_2048MB
+	default 4096 if SHMOBILE_IOMMU_ADDRSIZE_1024MB
+	default 2048 if SHMOBILE_IOMMU_ADDRSIZE_512MB
+	default 1024 if SHMOBILE_IOMMU_ADDRSIZE_256MB
+	default 512 if SHMOBILE_IOMMU_ADDRSIZE_128MB
+	default 256 if SHMOBILE_IOMMU_ADDRSIZE_64MB
+	default 128 if SHMOBILE_IOMMU_ADDRSIZE_32MB
+
 endif # IOMMU_SUPPORT
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 14a4d5f..62cf917 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_OMAP_IOMMU_DEBUG) += omap-iommu-debug.o
 obj-$(CONFIG_TEGRA_IOMMU_GART) += tegra-gart.o
 obj-$(CONFIG_TEGRA_IOMMU_SMMU) += tegra-smmu.o
 obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o
+obj-$(CONFIG_SHMOBILE_IOMMU) += shmobile-iommu.o
diff --git a/drivers/iommu/shmobile-iommu.c b/drivers/iommu/shmobile-iommu.c
new file mode 100644
index 0000000..bbbf1bc
--- /dev/null
+++ b/drivers/iommu/shmobile-iommu.c
@@ -0,0 +1,352 @@
+/*
+ * IOMMU for IPMMU/IPMMUI
+ * Copyright (C) 2012  Hideki EIRAKU
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#include <linux/io.h>
+#include <linux/dmapool.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/iommu.h>
+#include <linux/dma-mapping.h>
+#include <mach/ipmmu.h>
+#include <asm/dma-iommu.h>
+
+#define L1_SIZE CONFIG_SHMOBILE_IOMMU_L1SIZE
+#define L1_LEN (L1_SIZE / 4)
+#define L1_ALIGN L1_SIZE
+#define L2_SIZE 0x400
+#define L2_LEN (L2_SIZE / 4)
+#define L2_ALIGN L2_SIZE
+
+struct shmobile_iommu_priv_pgtable {
+	uint32_t *pgtable;
+	dma_addr_t handle;
+};
+
+struct shmobile_iommu_priv {
+	struct shmobile_iommu_priv_pgtable l1, l2[L1_LEN];
+	spinlock_t map_lock;
+	atomic_t active;
+};
+
+static struct dma_iommu_mapping *iommu_mapping;
+static struct device *ipmmu_devices;
+static struct dma_pool *l1pool, *l2pool;
+static spinlock_t lock;
+static DEFINE_SPINLOCK(lock_add);
+static struct shmobile_iommu_priv *attached;
+static int num_attached_devices;
+static struct device *ipmmu_access_device;
+
+static int shmobile_iommu_domain_init(struct iommu_domain *domain)
+{
+	struct shmobile_iommu_priv *priv;
+	int i;
+
+	priv = kmalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	priv->l1.pgtable = dma_pool_alloc(l1pool, GFP_KERNEL,
+					  &priv->l1.handle);
+	if (!priv->l1.pgtable) {
+		kfree(priv);
+		return -ENOMEM;
+	}
+	for (i = 0; i < L1_LEN; i++)
+		priv->l2[i].pgtable = NULL;
+	memset(priv->l1.pgtable, 0, L1_SIZE);
+	spin_lock_init(&priv->map_lock);
+	atomic_set(&priv->active, 0);
+	domain->priv = priv;
+	return 0;
+}
+
+static void shmobile_iommu_domain_destroy(struct iommu_domain *domain)
+{
+	struct shmobile_iommu_priv *priv = domain->priv;
+	int i;
+
+	for (i = 0; i < L1_LEN; i++) {
+		if (priv->l2[i].pgtable)
+			dma_pool_free(l2pool, priv->l2[i].pgtable,
+				      priv->l2[i].handle);
+	}
+	dma_pool_free(l1pool, priv->l1.pgtable, priv->l1.handle);
+	kfree(priv);
+	domain->priv = NULL;
+}
+
+static int shmobile_iommu_attach_device(struct iommu_domain *domain,
+					struct device *dev)
+{
+	struct shmobile_iommu_priv *priv = domain->priv;
+	int ret = -EBUSY;
+
+	spin_lock(&lock);
+	if (attached != priv) {
+		if (attached)
+			goto err;
+		atomic_set(&priv->active, 1);
+		ipmmu_tlb_set(ipmmu_access_device, priv->l1.handle, L1_SIZE,
+			      0);
+		wmb();
+		ipmmu_tlb_flush(ipmmu_access_device);
+		attached = priv;
+		num_attached_devices = 0;
+	}
+	num_attached_devices++;
+	ret = 0;
+err:
+	spin_unlock(&lock);
+	return ret;
+}
+
+static void shmobile_iommu_detach_device(struct iommu_domain *domain,
+					 struct device *dev)
+{
+	struct shmobile_iommu_priv *priv = domain->priv;
+
+	spin_lock(&lock);
+	atomic_set(&priv->active, 0);
+	num_attached_devices--;
+	if (!num_attached_devices) {
+		ipmmu_tlb_set(ipmmu_access_device, 0, 0, 0);
+		ipmmu_tlb_flush(ipmmu_access_device);
+		attached = NULL;
+	}
+	spin_unlock(&lock);
+}
+
+static int
+l2alloc(struct shmobile_iommu_priv *priv, unsigned int l1index)
+{
+	if (!priv->l2[l1index].pgtable) {
+		priv->l2[l1index].pgtable = dma_pool_alloc(l2pool, GFP_KERNEL,
+						&priv->l2[l1index].handle);
+		if (!priv->l2[l1index].pgtable)
+			return -ENOMEM;
+		memset(priv->l2[l1index].pgtable, 0, L2_SIZE);
+	}
+	priv->l1.pgtable[l1index] = priv->l2[l1index].handle | 0x1;
+	return 0;
+}
+
+static void
+l2realfree(struct shmobile_iommu_priv_pgtable *l2)
+{
+	if (l2->pgtable)
+		dma_pool_free(l2pool, l2->pgtable, l2->handle);
+}
+
+static int
+l2free(struct shmobile_iommu_priv *priv, unsigned int l1index,
+	struct shmobile_iommu_priv_pgtable *l2)
+{
+	priv->l1.pgtable[l1index] = 0;
+	if (priv->l2[l1index].pgtable) {
+		*l2 = priv->l2[l1index];
+		priv->l2[l1index].pgtable = NULL;
+	}
+	return 0;
+}
+
+static int shmobile_iommu_map(struct iommu_domain *domain, unsigned long iova,
+			      phys_addr_t paddr, size_t size, int prot)
+{
+	struct shmobile_iommu_priv_pgtable l2 = { .pgtable = NULL };
+	struct shmobile_iommu_priv *priv = domain->priv;
+	unsigned int l1index, l2index, i;
+	int ret;
+
+	l1index = iova >> 20;
+	switch (size) {
+	case 0x1000:
+		l2index = (iova >> 12) & 0xff;
+		spin_lock(&priv->map_lock);
+		ret = l2alloc(priv, l1index);
+		if (!ret)
+			priv->l2[l1index].pgtable[l2index] = paddr | 0xff2;
+		spin_unlock(&priv->map_lock);
+		break;
+	case 0x10000:
+		l2index = (iova >> 12) & 0xf0;
+		spin_lock(&priv->map_lock);
+		ret = l2alloc(priv, l1index);
+		if (!ret) {
+			for (i = 0; i < 0x10; i++)
+				priv->l2[l1index].pgtable[l2index + i] =
+					paddr | 0xff1;
+		}
+		spin_unlock(&priv->map_lock);
+		break;
+	case 0x100000:
+		spin_lock(&priv->map_lock);
+		l2free(priv, l1index, &l2);
+		priv->l1.pgtable[l1index] = paddr | 0xc02;
+		spin_unlock(&priv->map_lock);
+		ret = 0;
+		break;
+	default:
+		ret = -EINVAL;
+	}
+	if (!ret && atomic_read(&priv->active)) {
+		wmb();
+		ipmmu_tlb_flush(ipmmu_access_device);
+		l2realfree(&l2);
+	}
+	return ret;
+}
+
+static size_t shmobile_iommu_unmap(struct iommu_domain *domain,
+				   unsigned long iova, size_t size)
+{
+	struct shmobile_iommu_priv_pgtable l2 = { .pgtable = NULL };
+	struct shmobile_iommu_priv *priv = domain->priv;
+	unsigned int l1index, l2index, i;
+	uint32_t l2entry = 0;
+	size_t ret = 0;
+
+	l1index = iova >> 20;
+	if (!(iova & 0xFFFFF) && size >= 0x100000) {
+		spin_lock(&priv->map_lock);
+		l2free(priv, l1index, &l2);
+		spin_unlock(&priv->map_lock);
+		ret = 0x100000;
+		goto done;
+	}
+	l2index = (iova >> 12) & 0xff;
+	spin_lock(&priv->map_lock);
+	if (priv->l2[l1index].pgtable)
+		l2entry = priv->l2[l1index].pgtable[l2index];
+	switch (l2entry & 3) {
+	case 1:
+		if (l2index & 0xf)
+			break;
+		for (i = 0; i < 0x10; i++)
+			priv->l2[l1index].pgtable[l2index + i] = 0;
+		ret = 0x10000;
+		break;
+	case 2:
+		priv->l2[l1index].pgtable[l2index] = 0;
+		ret = 0x1000;
+		break;
+	}
+	spin_unlock(&priv->map_lock);
+done:
+	if (ret && atomic_read(&priv->active)) {
+		wmb();
+		ipmmu_tlb_flush(ipmmu_access_device);
+		l2realfree(&l2);
+	}
+	return ret;
+}
+
+static phys_addr_t shmobile_iommu_iova_to_phys(struct iommu_domain *domain,
+					       unsigned long iova)
+{
+	struct shmobile_iommu_priv *priv = domain->priv;
+	uint32_t l1entry = 0, l2entry = 0;
+	unsigned int l1index, l2index;
+
+	l1index = iova >> 20;
+	l2index = (iova >> 12) & 0xff;
+	spin_lock(&priv->map_lock);
+	if (priv->l2[l1index].pgtable)
+		l2entry = priv->l2[l1index].pgtable[l2index];
+	else
+		l1entry = priv->l1.pgtable[l1index];
+	spin_unlock(&priv->map_lock);
+	switch (l2entry & 3) {
+	case 1:
+		return (l2entry & ~0xffff) | (iova & 0xffff);
+	case 2:
+		return (l2entry & ~0xfff) | (iova & 0xfff);
+	default:
+		if ((l1entry & 3) == 2)
+			return (l1entry & ~0xfffff) | (iova & 0xfffff);
+		return 0;
+	}
+}
+
+static struct iommu_ops shmobile_iommu_ops = {
+	.domain_init = shmobile_iommu_domain_init,
+	.domain_destroy = shmobile_iommu_domain_destroy,
+	.attach_dev = shmobile_iommu_attach_device,
+	.detach_dev = shmobile_iommu_detach_device,
+	.map = shmobile_iommu_map,
+	.unmap = shmobile_iommu_unmap,
+	.iova_to_phys = shmobile_iommu_iova_to_phys,
+	.pgsize_bitmap = 0x111000,
+};
+
+static int shmobile_iommu_attach_all_devices(void)
+{
+	struct device *dev;
+	int ret = 0;
+
+	spin_lock(&lock_add);
+	iommu_mapping = arm_iommu_create_mapping(&platform_bus_type, 0x0,
+						 L1_LEN << 20, 0);
+	if (IS_ERR_OR_NULL(iommu_mapping)) {
+		ret = PTR_ERR(iommu_mapping);
+		goto err;
+	}
+	for (dev = ipmmu_devices; dev; dev = dev->archdata.iommu) {
+		if (arm_iommu_attach_device(dev, iommu_mapping))
+			pr_err("arm_iommu_attach_device failed\n");
+	}
+err:
+	spin_unlock(&lock_add);
+	return 0;
+}
+
+void ipmmu_add_device(struct device *dev)
+{
+	spin_lock(&lock_add);
+	dev->archdata.iommu = ipmmu_devices;
+	ipmmu_devices = dev;
+	if (!IS_ERR_OR_NULL(iommu_mapping)) {
+		if (arm_iommu_attach_device(dev, iommu_mapping))
+			pr_err("arm_iommu_attach_device failed\n");
+	}
+	spin_unlock(&lock_add);
+}
+
+int ipmmu_iommu_init(struct device *dev)
+{
+	dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
+	l1pool = dma_pool_create("shmobile-iommu-pgtable1", dev,
+				 L1_SIZE, L1_ALIGN, 0);
+	if (!l1pool)
+		goto nomem_pool1;
+	l2pool = dma_pool_create("shmobile-iommu-pgtable2", dev,
+				 L2_SIZE, L2_ALIGN, 0);
+	if (!l2pool)
+		goto nomem_pool2;
+	spin_lock_init(&lock);
+	attached = NULL;
+	ipmmu_access_device = dev;
+	bus_set_iommu(&platform_bus_type, &shmobile_iommu_ops);
+	if (shmobile_iommu_attach_all_devices())
+		pr_err("shmobile_iommu_attach_all_devices failed\n");
+	return 0;
+nomem_pool2:
+	dma_pool_destroy(l1pool);
+nomem_pool1:
+	return -ENOMEM;
+}
-- 
1.7.0.4


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

* [PATCH v4 2/2] ARM: mach-shmobile: sh7372: Add IPMMU device
  2012-10-15  8:34 [PATCH v4 0/2] Renesas IPMMU driver for sh7372 Hideki EIRAKU
  2012-10-15  8:34 ` [PATCH v4 1/2] iommu/shmobile: Add iommu driver for Renesas IPMMU modules Hideki EIRAKU
@ 2012-10-15  8:34 ` Hideki EIRAKU
  2012-12-16 17:25 ` [PATCH/WIP/RFC 00/14] Renesas IPMMU driver work in progress Laurent Pinchart
  2 siblings, 0 replies; 23+ messages in thread
From: Hideki EIRAKU @ 2012-10-15  8:34 UTC (permalink / raw)
  To: Paul Mundt, Magnus Damm, Russell King, Simon Horman, Laurent Pinchart
  Cc: linux-sh, linux-arm-kernel, linux-kernel, Marek Szyprowski,
	Katsuya MATSUBARA, Damian Hobson-Garcia, Hideki EIRAKU

This patch adds an IPMMU device and notifies the IPMMU driver which
devices are connected via the IPMMU module.  All devices connected to the main
memory bus via the IPMMU module MUST be registered when SHMOBILE_IPMMU and
SHMOBILE_IOMMU are enabled because physical address cannot be used
while the IPMMU module's MMU function is enabled.

Signed-off-by: Hideki EIRAKU <hdk@igel.co.jp>
---
 arch/arm/mach-shmobile/board-ap4evb.c   |    5 +++++
 arch/arm/mach-shmobile/board-mackerel.c |    5 +++++
 arch/arm/mach-shmobile/setup-sh7372.c   |   26 ++++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index 790dc68..7006abb 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -61,6 +61,7 @@
 #include <mach/common.h>
 #include <mach/irqs.h>
 #include <mach/sh7372.h>
+#include <mach/ipmmu.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
@@ -1459,6 +1460,10 @@ static void __init ap4evb_init(void)
 
 	sh7372_add_standard_devices();
 
+	ipmmu_add_device(&lcdc_device.dev);
+	ipmmu_add_device(&lcdc1_device.dev);
+	ipmmu_add_device(&ceu_device.dev);
+
 	/* HDMI */
 	gpio_request(GPIO_FN_HDMI_HPD, NULL);
 	gpio_request(GPIO_FN_HDMI_CEC, NULL);
diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index 0c27c81..1c34520 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -60,6 +60,7 @@
 #include <mach/common.h>
 #include <mach/irqs.h>
 #include <mach/sh7372.h>
+#include <mach/ipmmu.h>
 
 #include <asm/mach/arch.h>
 #include <asm/mach-types.h>
@@ -1640,6 +1641,10 @@ static void __init mackerel_init(void)
 
 	sh7372_add_standard_devices();
 
+	ipmmu_add_device(&lcdc_device.dev);
+	ipmmu_add_device(&hdmi_lcdc_device.dev);
+	ipmmu_add_device(&ceu_device.dev);
+
 	platform_add_devices(mackerel_devices, ARRAY_SIZE(mackerel_devices));
 
 	rmobile_add_devices_to_domains(domain_devices,
diff --git a/arch/arm/mach-shmobile/setup-sh7372.c b/arch/arm/mach-shmobile/setup-sh7372.c
index a07954f..aadb769 100644
--- a/arch/arm/mach-shmobile/setup-sh7372.c
+++ b/arch/arm/mach-shmobile/setup-sh7372.c
@@ -38,6 +38,7 @@
 #include <mach/irqs.h>
 #include <mach/sh7372.h>
 #include <mach/common.h>
+#include <mach/ipmmu.h>
 #include <asm/mach/map.h>
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
@@ -968,6 +969,23 @@ static struct platform_device spu1_device = {
 	.num_resources	= ARRAY_SIZE(spu1_resources),
 };
 
+/* IPMMUI (an IPMMU module for ICB/LMB) */
+static struct resource ipmmu_resources[] = {
+	[0] = {
+		.name	= "IPMMUI",
+		.start	= 0xfe951000,
+		.end	= 0xfe9510ff,
+		.flags	= IORESOURCE_MEM,
+	},
+};
+
+static struct platform_device ipmmu_device = {
+	.name           = "ipmmu",
+	.id             = -1,
+	.resource       = ipmmu_resources,
+	.num_resources  = ARRAY_SIZE(ipmmu_resources),
+};
+
 static struct platform_device *sh7372_early_devices[] __initdata = {
 	&scif0_device,
 	&scif1_device,
@@ -979,6 +997,7 @@ static struct platform_device *sh7372_early_devices[] __initdata = {
 	&cmt2_device,
 	&tmu00_device,
 	&tmu01_device,
+	&ipmmu_device,
 };
 
 static struct platform_device *sh7372_late_devices[] __initdata = {
@@ -1033,6 +1052,13 @@ void __init sh7372_add_standard_devices(void)
 	platform_add_devices(sh7372_early_devices,
 			    ARRAY_SIZE(sh7372_early_devices));
 
+	ipmmu_add_device(&vpu_device.dev);
+	ipmmu_add_device(&jpu_device.dev);
+	ipmmu_add_device(&veu0_device.dev);
+	ipmmu_add_device(&veu1_device.dev);
+	ipmmu_add_device(&veu2_device.dev);
+	ipmmu_add_device(&veu3_device.dev);
+
 	platform_add_devices(sh7372_late_devices,
 			    ARRAY_SIZE(sh7372_late_devices));
 
-- 
1.7.0.4


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

* Re: [PATCH v4 1/2] iommu/shmobile: Add iommu driver for Renesas IPMMU modules
  2012-10-15  8:34 ` [PATCH v4 1/2] iommu/shmobile: Add iommu driver for Renesas IPMMU modules Hideki EIRAKU
@ 2012-12-10 15:55   ` Laurent Pinchart
  2012-12-11 10:10     ` Hideki EIRAKU
  0 siblings, 1 reply; 23+ messages in thread
From: Laurent Pinchart @ 2012-12-10 15:55 UTC (permalink / raw)
  To: Hideki EIRAKU
  Cc: Paul Mundt, Magnus Damm, Russell King, Simon Horman, linux-sh,
	linux-arm-kernel, linux-kernel, Marek Szyprowski,
	Katsuya MATSUBARA, Damian Hobson-Garcia

Dear Eiraku-san,

On Monday 15 October 2012 17:34:52 Hideki EIRAKU wrote:
> This is the Renesas IPMMU driver and IOMMU API implementation.
> 
> The IPMMU module supports the MMU function and the PMB function.

That sentence make me believe that both MMU and PMB were supported by the 
driver, as "module" often refers to Linux kernel modules in this context. 
Maybe you could replace "module" by "hardware module".

> The MMU function provides address translation by pagetable compatible with
> ARMv6. The PMB function provides address translation including tile-linear
> translation. This patch implements the MMU function.
>
> The iommu driver does not register a platform driver directly because:
> - the register space of the MMU function and the PMB function
>   have a common register (used for settings flush), so they should ideally
>   have a way to appropriately share this register.
> - the MMU function uses the IOMMU API while the PMB function does not.
> - the two functions may be used independently.
> 
> Signed-off-by: Hideki EIRAKU <hdk@igel.co.jp>
> ---
>  arch/arm/mach-shmobile/Kconfig              |    6 +
>  arch/arm/mach-shmobile/Makefile             |    3 +
>  arch/arm/mach-shmobile/include/mach/ipmmu.h |   16 ++
>  arch/arm/mach-shmobile/ipmmu.c              |  150 ++++++++++++
>  drivers/iommu/Kconfig                       |   56 +++++
>  drivers/iommu/Makefile                      |    1 +
>  drivers/iommu/shmobile-iommu.c              |  352 ++++++++++++++++++++++++
>  7 files changed, 584 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-shmobile/include/mach/ipmmu.h
>  create mode 100644 arch/arm/mach-shmobile/ipmmu.c
>  create mode 100644 drivers/iommu/shmobile-iommu.c

What is the reason for splitting the driver in two files ? Can't you put all 
the code in drivers/iommu/shmobile-iommu.c ? Storing driver code in arch/* is 
discouraged.

> diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
> index 8ae100c..de69ab3 100644
> --- a/arch/arm/mach-shmobile/Kconfig
> +++ b/arch/arm/mach-shmobile/Kconfig
> @@ -210,6 +210,12 @@ endmenu
>  config SH_CLK_CPG
>  	bool
> 
> +config SHMOBILE_IPMMU
> +	bool
> +
> +config SHMOBILE_IPMMU_TLB
> +	bool
> +
>  source "drivers/sh/Kconfig"
> 
>  endif
> diff --git a/arch/arm/mach-shmobile/Makefile
> b/arch/arm/mach-shmobile/Makefile index fe2c97c..4ffba9d 100644
> --- a/arch/arm/mach-shmobile/Makefile
> +++ b/arch/arm/mach-shmobile/Makefile
> @@ -60,3 +60,6 @@ obj-$(CONFIG_MACH_KZM9G)	+= board-kzm9g.o
>  # Framework support
>  obj-$(CONFIG_SMP)		+= $(smp-y)
>  obj-$(CONFIG_GENERIC_GPIO)	+= $(pfc-y)
> +
> +# IPMMU/IPMMUI
> +obj-$(CONFIG_SHMOBILE_IPMMU)	+= ipmmu.o
> diff --git a/arch/arm/mach-shmobile/include/mach/ipmmu.h
> b/arch/arm/mach-shmobile/include/mach/ipmmu.h new file mode 100644
> index 0000000..ede2f0b
> --- /dev/null
> +++ b/arch/arm/mach-shmobile/include/mach/ipmmu.h
> @@ -0,0 +1,16 @@
> +#ifdef CONFIG_SHMOBILE_IPMMU_TLB
> +void ipmmu_tlb_flush(struct device *ipmmu_dev);
> +void ipmmu_tlb_set(struct device *ipmmu_dev, unsigned long phys, int size,
> +		   int asid);
> +void ipmmu_add_device(struct device *dev);
> +int ipmmu_iommu_init(struct device *dev);
> +#else
> +static inline void ipmmu_add_device(struct device *dev)
> +{
> +}
> +
> +static int ipmmu_iommu_init(struct device *dev)
> +{
> +	return -EINVAL;
> +}
> +#endif
> diff --git a/arch/arm/mach-shmobile/ipmmu.c b/arch/arm/mach-shmobile/ipmmu.c
> new file mode 100644
> index 0000000..72cacb9
> --- /dev/null
> +++ b/arch/arm/mach-shmobile/ipmmu.c
> @@ -0,0 +1,150 @@
> +/*
> + * IPMMU/IPMMUI
> + * Copyright (C) 2012  Hideki EIRAKU
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 
> USA

You can remove this last paragraph, we don't want to patch every file in the 
kernel if the FSF moves to a new building :-)

> + *
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/err.h>
> +#include <linux/export.h>
> +#include <linux/slab.h>
> +#include <mach/ipmmu.h>
> +
> +#define IMCTR1 0x000
> +#define IMCTR2 0x004
> +#define IMASID 0x010
> +#define IMTTBR 0x014
> +#define IMTTBCR 0x018
> +
> +#define IMCTR1_TLBEN (1 << 0)
> +#define IMCTR1_FLUSH (1 << 1)
> +
> +struct ipmmu_priv {
> +	void __iomem *ipmmu_base;
> +	int tlb_enabled;
> +	struct mutex flush_lock;
> +};
> +
> +static void ipmmu_reg_write(struct ipmmu_priv *priv, unsigned long reg_off,
> +			    unsigned long data)
> +{
> +	iowrite32(data, priv->ipmmu_base + reg_off);
> +}
> +
> +void ipmmu_tlb_flush(struct device *dev)
> +{
> +	struct ipmmu_priv *priv;
> +
> +	if (!dev)
> +		return;
> +	priv = dev_get_drvdata(dev);
> +	mutex_lock(&priv->flush_lock);
> +	if (priv->tlb_enabled)
> +		ipmmu_reg_write(priv, IMCTR1, IMCTR1_FLUSH | IMCTR1_TLBEN);
> +	else
> +		ipmmu_reg_write(priv, IMCTR1, IMCTR1_FLUSH);
> +	mutex_unlock(&priv->flush_lock);
> +}
> +
> +void ipmmu_tlb_set(struct device *dev, unsigned long phys, int size, int
> asid)
> +{
> +	struct ipmmu_priv *priv;
> +
> +	if (!dev)
> +		return;
> +	priv = dev_get_drvdata(dev);
> +	mutex_lock(&priv->flush_lock);
> +	switch (size) {
> +	default:
> +		priv->tlb_enabled = 0;
> +		break;
> +	case 0x2000:
> +		ipmmu_reg_write(priv, IMTTBCR, 1);
> +		priv->tlb_enabled = 1;
> +		break;
> +	case 0x1000:
> +		ipmmu_reg_write(priv, IMTTBCR, 2);
> +		priv->tlb_enabled = 1;
> +		break;
> +	case 0x800:
> +		ipmmu_reg_write(priv, IMTTBCR, 3);
> +		priv->tlb_enabled = 1;
> +		break;
> +	case 0x400:
> +		ipmmu_reg_write(priv, IMTTBCR, 4);
> +		priv->tlb_enabled = 1;
> +		break;
> +	case 0x200:
> +		ipmmu_reg_write(priv, IMTTBCR, 5);
> +		priv->tlb_enabled = 1;
> +		break;
> +	case 0x100:
> +		ipmmu_reg_write(priv, IMTTBCR, 6);
> +		priv->tlb_enabled = 1;
> +		break;
> +	case 0x80:
> +		ipmmu_reg_write(priv, IMTTBCR, 7);
> +		priv->tlb_enabled = 1;
> +		break;
> +	}
> +	ipmmu_reg_write(priv, IMTTBR, phys);
> +	ipmmu_reg_write(priv, IMASID, asid);
> +	mutex_unlock(&priv->flush_lock);
> +}
> +
> +static int __devinit ipmmu_probe(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	struct ipmmu_priv *priv;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&pdev->dev, "cannot get platform resources\n");
> +		return -ENOENT;
> +	}
> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +	if (!priv) {
> +		dev_err(&pdev->dev, "cannot allocate device data\n");
> +		return -ENOMEM;
> +	}
> +	mutex_init(&priv->flush_lock);
> +	priv->ipmmu_base = ioremap_nocache(res->start, resource_size(res));
> +	if (!priv->ipmmu_base) {
> +		dev_err(&pdev->dev, "ioremap_nocache failed\n");
> +		kfree(priv);
> +		return -ENOMEM;
> +	}
> +	platform_set_drvdata(pdev, priv);
> +	ipmmu_reg_write(priv, IMCTR1, 0x0); /* disable TLB */
> +	ipmmu_reg_write(priv, IMCTR2, 0x0); /* disable PMB */
> +	ipmmu_iommu_init(&pdev->dev);
> +	return 0;
> +}
> +
> +static struct platform_driver ipmmu_driver = {
> +	.probe = ipmmu_probe,
> +	.driver = {
> +		.owner = THIS_MODULE,
> +		.name = "ipmmu",
> +	},
> +};
> +
> +static int __init ipmmu_init(void)
> +{
> +	return platform_driver_register(&ipmmu_driver);
> +}
> +subsys_initcall(ipmmu_init);
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index e39f9db..265829f 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -187,4 +187,60 @@ config EXYNOS_IOMMU_DEBUG
> 
>  	  Say N unless you need kernel log message for IOMMU debugging
> 
> +config SHMOBILE_IOMMU
> +	bool "IOMMU for Renesas IPMMU/IPMMUI"
> +	default n
> +	select IOMMU_API
> +	select ARM_DMA_USE_IOMMU
> +	select SHMOBILE_IPMMU
> +	select SHMOBILE_IPMMU_TLB
> +
> +choice
> +	prompt "IPMMU/IPMMUI address space size"
> +	default SHMOBILE_IOMMU_ADDRSIZE_2048MB
> +	depends on SHMOBILE_IOMMU
> +	help
> +	  This option sets IPMMU/IPMMUI address space size by
> +	  adjusting the 1st level page table size. The page table size
> +	  is calculated as follows:
> +
> +	      page table size = number of page table entries * 4 bytes
> +	      number of page table entries = address space size / 1 MiB
> +
> +	  For example, when the address space size is 2048 MiB, the
> +	  1st level page table size is 8192 bytes.
> +
> +	config SHMOBILE_IOMMU_ADDRSIZE_2048MB
> +		bool "2 GiB"
> +
> +	config SHMOBILE_IOMMU_ADDRSIZE_1024MB
> +		bool "1 GiB"
> +
> +	config SHMOBILE_IOMMU_ADDRSIZE_512MB
> +		bool "512 MiB"
> +
> +	config SHMOBILE_IOMMU_ADDRSIZE_256MB
> +		bool "256 MiB"
> +
> +	config SHMOBILE_IOMMU_ADDRSIZE_128MB
> +		bool "128 MiB"
> +
> +	config SHMOBILE_IOMMU_ADDRSIZE_64MB
> +		bool "64 MiB"
> +
> +	config SHMOBILE_IOMMU_ADDRSIZE_32MB
> +		bool "32 MiB"
> +
> +endchoice
> +
> +config SHMOBILE_IOMMU_L1SIZE
> +	int
> +	default 8192 if SHMOBILE_IOMMU_ADDRSIZE_2048MB
> +	default 4096 if SHMOBILE_IOMMU_ADDRSIZE_1024MB
> +	default 2048 if SHMOBILE_IOMMU_ADDRSIZE_512MB
> +	default 1024 if SHMOBILE_IOMMU_ADDRSIZE_256MB
> +	default 512 if SHMOBILE_IOMMU_ADDRSIZE_128MB
> +	default 256 if SHMOBILE_IOMMU_ADDRSIZE_64MB
> +	default 128 if SHMOBILE_IOMMU_ADDRSIZE_32MB
> +
>  endif # IOMMU_SUPPORT
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index 14a4d5f..62cf917 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_OMAP_IOMMU_DEBUG) += omap-iommu-debug.o
>  obj-$(CONFIG_TEGRA_IOMMU_GART) += tegra-gart.o
>  obj-$(CONFIG_TEGRA_IOMMU_SMMU) += tegra-smmu.o
>  obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o
> +obj-$(CONFIG_SHMOBILE_IOMMU) += shmobile-iommu.o
> diff --git a/drivers/iommu/shmobile-iommu.c b/drivers/iommu/shmobile-iommu.c
> new file mode 100644
> index 0000000..bbbf1bc
> --- /dev/null
> +++ b/drivers/iommu/shmobile-iommu.c
> @@ -0,0 +1,352 @@
> +/*
> + * IOMMU for IPMMU/IPMMUI
> + * Copyright (C) 2012  Hideki EIRAKU
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 
> USA + *
> + */
> +
> +#include <linux/io.h>
> +#include <linux/dmapool.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/iommu.h>
> +#include <linux/dma-mapping.h>
> +#include <mach/ipmmu.h>
> +#include <asm/dma-iommu.h>
> +
> +#define L1_SIZE CONFIG_SHMOBILE_IOMMU_L1SIZE
> +#define L1_LEN (L1_SIZE / 4)
> +#define L1_ALIGN L1_SIZE
> +#define L2_SIZE 0x400
> +#define L2_LEN (L2_SIZE / 4)
> +#define L2_ALIGN L2_SIZE
> +
> +struct shmobile_iommu_priv_pgtable {
> +	uint32_t *pgtable;
> +	dma_addr_t handle;
> +};
> +
> +struct shmobile_iommu_priv {
> +	struct shmobile_iommu_priv_pgtable l1, l2[L1_LEN];
> +	spinlock_t map_lock;
> +	atomic_t active;
> +};
> +
> +static struct dma_iommu_mapping *iommu_mapping;
> +static struct device *ipmmu_devices;
> +static struct dma_pool *l1pool, *l2pool;
> +static spinlock_t lock;
> +static DEFINE_SPINLOCK(lock_add);
> +static struct shmobile_iommu_priv *attached;
> +static int num_attached_devices;
> +static struct device *ipmmu_access_device;

This many global variables is usually a sign that something is wrong, please 
see below.

> +
> +static int shmobile_iommu_domain_init(struct iommu_domain *domain)
> +{
> +	struct shmobile_iommu_priv *priv;
> +	int i;
> +
> +	priv = kmalloc(sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +	priv->l1.pgtable = dma_pool_alloc(l1pool, GFP_KERNEL,
> +					  &priv->l1.handle);
> +	if (!priv->l1.pgtable) {
> +		kfree(priv);
> +		return -ENOMEM;
> +	}
> +	for (i = 0; i < L1_LEN; i++)
> +		priv->l2[i].pgtable = NULL;
> +	memset(priv->l1.pgtable, 0, L1_SIZE);
> +	spin_lock_init(&priv->map_lock);
> +	atomic_set(&priv->active, 0);
> +	domain->priv = priv;
> +	return 0;
> +}
> +
> +static void shmobile_iommu_domain_destroy(struct iommu_domain *domain)
> +{
> +	struct shmobile_iommu_priv *priv = domain->priv;
> +	int i;
> +
> +	for (i = 0; i < L1_LEN; i++) {
> +		if (priv->l2[i].pgtable)
> +			dma_pool_free(l2pool, priv->l2[i].pgtable,
> +				      priv->l2[i].handle);
> +	}
> +	dma_pool_free(l1pool, priv->l1.pgtable, priv->l1.handle);
> +	kfree(priv);
> +	domain->priv = NULL;
> +}
> +
> +static int shmobile_iommu_attach_device(struct iommu_domain *domain,
> +					struct device *dev)
> +{
> +	struct shmobile_iommu_priv *priv = domain->priv;
> +	int ret = -EBUSY;
> +
> +	spin_lock(&lock);
> +	if (attached != priv) {
> +		if (attached)
> +			goto err;
> +		atomic_set(&priv->active, 1);
> +		ipmmu_tlb_set(ipmmu_access_device, priv->l1.handle, L1_SIZE,
> +			      0);
> +		wmb();
> +		ipmmu_tlb_flush(ipmmu_access_device);
> +		attached = priv;
> +		num_attached_devices = 0;
> +	}
> +	num_attached_devices++;
> +	ret = 0;
> +err:
> +	spin_unlock(&lock);
> +	return ret;
> +}
> +
> +static void shmobile_iommu_detach_device(struct iommu_domain *domain,
> +					 struct device *dev)
> +{
> +	struct shmobile_iommu_priv *priv = domain->priv;
> +
> +	spin_lock(&lock);
> +	atomic_set(&priv->active, 0);
> +	num_attached_devices--;
> +	if (!num_attached_devices) {
> +		ipmmu_tlb_set(ipmmu_access_device, 0, 0, 0);
> +		ipmmu_tlb_flush(ipmmu_access_device);
> +		attached = NULL;
> +	}
> +	spin_unlock(&lock);
> +}
> +
> +static int
> +l2alloc(struct shmobile_iommu_priv *priv, unsigned int l1index)
> +{
> +	if (!priv->l2[l1index].pgtable) {
> +		priv->l2[l1index].pgtable = dma_pool_alloc(l2pool, GFP_KERNEL,
> +						&priv->l2[l1index].handle);
> +		if (!priv->l2[l1index].pgtable)
> +			return -ENOMEM;
> +		memset(priv->l2[l1index].pgtable, 0, L2_SIZE);
> +	}
> +	priv->l1.pgtable[l1index] = priv->l2[l1index].handle | 0x1;
> +	return 0;
> +}
> +
> +static void
> +l2realfree(struct shmobile_iommu_priv_pgtable *l2)
> +{
> +	if (l2->pgtable)
> +		dma_pool_free(l2pool, l2->pgtable, l2->handle);
> +}
> +
> +static int
> +l2free(struct shmobile_iommu_priv *priv, unsigned int l1index,
> +	struct shmobile_iommu_priv_pgtable *l2)
> +{
> +	priv->l1.pgtable[l1index] = 0;
> +	if (priv->l2[l1index].pgtable) {
> +		*l2 = priv->l2[l1index];
> +		priv->l2[l1index].pgtable = NULL;
> +	}
> +	return 0;
> +}
> +
> +static int shmobile_iommu_map(struct iommu_domain *domain, unsigned long
> iova, +			      phys_addr_t paddr, size_t size, int prot)
> +{
> +	struct shmobile_iommu_priv_pgtable l2 = { .pgtable = NULL };
> +	struct shmobile_iommu_priv *priv = domain->priv;
> +	unsigned int l1index, l2index, i;
> +	int ret;
> +
> +	l1index = iova >> 20;
> +	switch (size) {
> +	case 0x1000:
> +		l2index = (iova >> 12) & 0xff;
> +		spin_lock(&priv->map_lock);
> +		ret = l2alloc(priv, l1index);
> +		if (!ret)
> +			priv->l2[l1index].pgtable[l2index] = paddr | 0xff2;
> +		spin_unlock(&priv->map_lock);
> +		break;
> +	case 0x10000:
> +		l2index = (iova >> 12) & 0xf0;
> +		spin_lock(&priv->map_lock);
> +		ret = l2alloc(priv, l1index);
> +		if (!ret) {
> +			for (i = 0; i < 0x10; i++)
> +				priv->l2[l1index].pgtable[l2index + i] =
> +					paddr | 0xff1;
> +		}
> +		spin_unlock(&priv->map_lock);
> +		break;
> +	case 0x100000:
> +		spin_lock(&priv->map_lock);
> +		l2free(priv, l1index, &l2);
> +		priv->l1.pgtable[l1index] = paddr | 0xc02;
> +		spin_unlock(&priv->map_lock);
> +		ret = 0;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +	if (!ret && atomic_read(&priv->active)) {
> +		wmb();
> +		ipmmu_tlb_flush(ipmmu_access_device);
> +		l2realfree(&l2);
> +	}
> +	return ret;
> +}
> +
> +static size_t shmobile_iommu_unmap(struct iommu_domain *domain,
> +				   unsigned long iova, size_t size)
> +{
> +	struct shmobile_iommu_priv_pgtable l2 = { .pgtable = NULL };
> +	struct shmobile_iommu_priv *priv = domain->priv;
> +	unsigned int l1index, l2index, i;
> +	uint32_t l2entry = 0;
> +	size_t ret = 0;
> +
> +	l1index = iova >> 20;
> +	if (!(iova & 0xFFFFF) && size >= 0x100000) {
> +		spin_lock(&priv->map_lock);
> +		l2free(priv, l1index, &l2);
> +		spin_unlock(&priv->map_lock);
> +		ret = 0x100000;
> +		goto done;
> +	}
> +	l2index = (iova >> 12) & 0xff;
> +	spin_lock(&priv->map_lock);
> +	if (priv->l2[l1index].pgtable)
> +		l2entry = priv->l2[l1index].pgtable[l2index];
> +	switch (l2entry & 3) {
> +	case 1:
> +		if (l2index & 0xf)
> +			break;
> +		for (i = 0; i < 0x10; i++)
> +			priv->l2[l1index].pgtable[l2index + i] = 0;
> +		ret = 0x10000;
> +		break;
> +	case 2:
> +		priv->l2[l1index].pgtable[l2index] = 0;
> +		ret = 0x1000;
> +		break;
> +	}
> +	spin_unlock(&priv->map_lock);
> +done:
> +	if (ret && atomic_read(&priv->active)) {
> +		wmb();
> +		ipmmu_tlb_flush(ipmmu_access_device);
> +		l2realfree(&l2);
> +	}
> +	return ret;
> +}
> +
> +static phys_addr_t shmobile_iommu_iova_to_phys(struct iommu_domain *domain,
> +					       unsigned long iova)
> +{
> +	struct shmobile_iommu_priv *priv = domain->priv;
> +	uint32_t l1entry = 0, l2entry = 0;
> +	unsigned int l1index, l2index;
> +
> +	l1index = iova >> 20;
> +	l2index = (iova >> 12) & 0xff;
> +	spin_lock(&priv->map_lock);
> +	if (priv->l2[l1index].pgtable)
> +		l2entry = priv->l2[l1index].pgtable[l2index];
> +	else
> +		l1entry = priv->l1.pgtable[l1index];
> +	spin_unlock(&priv->map_lock);
> +	switch (l2entry & 3) {
> +	case 1:
> +		return (l2entry & ~0xffff) | (iova & 0xffff);
> +	case 2:
> +		return (l2entry & ~0xfff) | (iova & 0xfff);
> +	default:
> +		if ((l1entry & 3) == 2)
> +			return (l1entry & ~0xfffff) | (iova & 0xfffff);
> +		return 0;
> +	}
> +}
> +
> +static struct iommu_ops shmobile_iommu_ops = {
> +	.domain_init = shmobile_iommu_domain_init,
> +	.domain_destroy = shmobile_iommu_domain_destroy,
> +	.attach_dev = shmobile_iommu_attach_device,
> +	.detach_dev = shmobile_iommu_detach_device,
> +	.map = shmobile_iommu_map,
> +	.unmap = shmobile_iommu_unmap,
> +	.iova_to_phys = shmobile_iommu_iova_to_phys,
> +	.pgsize_bitmap = 0x111000,
> +};
> +
> +static int shmobile_iommu_attach_all_devices(void)
> +{
> +	struct device *dev;
> +	int ret = 0;
> +
> +	spin_lock(&lock_add);
> +	iommu_mapping = arm_iommu_create_mapping(&platform_bus_type, 0x0,
> +						 L1_LEN << 20, 0);
> +	if (IS_ERR_OR_NULL(iommu_mapping)) {
> +		ret = PTR_ERR(iommu_mapping);
> +		goto err;
> +	}
> +	for (dev = ipmmu_devices; dev; dev = dev->archdata.iommu) {
> +		if (arm_iommu_attach_device(dev, iommu_mapping))
> +			pr_err("arm_iommu_attach_device failed\n");
> +	}
> +err:
> +	spin_unlock(&lock_add);
> +	return 0;
> +}
> +
> +void ipmmu_add_device(struct device *dev)
> +{
> +	spin_lock(&lock_add);
> +	dev->archdata.iommu = ipmmu_devices;
> +	ipmmu_devices = dev;

That looks a bit hackish to me. I'd like to suggest a different approach, that 
would be compatible with supporting multiple IPMMU instances.

dev->archdata.iommu should point to a new sh_ipmmu_arch_data structure that 
would contain an IPMMU name (const char *) and a pointer to a struct 
shmobile_iommu_priv.

ipmmu_add_device() would take a new IPMMU name argument, allocate an 
sh_ipmmu_arch_data instance dynamically and initialize its name field to the 
name passed to the function. The shmobile_iommu_priv pointer would be set to 
NULL. No other operation would be performed (you will likely get rid of the 
global ipmmu_devices and iommu_mapping variables).

Then, the attach_dev operation handler would retrieve the dev->archdata.iommu 
pointer, cast that to an sh_ipmmu_arch_data, and retrieve the IPMMU associated 
with the name (either by walking a driver-global list of IPMMUs, or by using 
driver_find_device()).

This mechanism would get rid of several global variables in the driver 
(several of them would move to the shmobile_ipmmu_priv structure - which I 
would have named shmobile_ipmmu or even sh_ipmmu, but that's up to you) and 
add support for several IPMMU instances (there's 3 of them in the sh7372, even 
if we only need to support one right now it's still a good practice to design 
the driver in a way that multiple instances can be supported).

Could you try to rework the driiver in that direction ? You can have a look at 
the OMAP IOMMU driver if you need sample code, and obviously feel free to 
contact me if you have any question.

> +	if (!IS_ERR_OR_NULL(iommu_mapping)) {
> +		if (arm_iommu_attach_device(dev, iommu_mapping))
> +			pr_err("arm_iommu_attach_device failed\n");
> +	}
> +	spin_unlock(&lock_add);
> +}
> +
> +int ipmmu_iommu_init(struct device *dev)
> +{
> +	dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
> +	l1pool = dma_pool_create("shmobile-iommu-pgtable1", dev,
> +				 L1_SIZE, L1_ALIGN, 0);
> +	if (!l1pool)
> +		goto nomem_pool1;
> +	l2pool = dma_pool_create("shmobile-iommu-pgtable2", dev,
> +				 L2_SIZE, L2_ALIGN, 0);
> +	if (!l2pool)
> +		goto nomem_pool2;
> +	spin_lock_init(&lock);
> +	attached = NULL;
> +	ipmmu_access_device = dev;
> +	bus_set_iommu(&platform_bus_type, &shmobile_iommu_ops);
> +	if (shmobile_iommu_attach_all_devices())
> +		pr_err("shmobile_iommu_attach_all_devices failed\n");
> +	return 0;
> +nomem_pool2:
> +	dma_pool_destroy(l1pool);
> +nomem_pool1:
> +	return -ENOMEM;
> +}

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH v4 1/2] iommu/shmobile: Add iommu driver for Renesas IPMMU modules
  2012-12-10 15:55   ` Laurent Pinchart
@ 2012-12-11 10:10     ` Hideki EIRAKU
  2012-12-11 12:36       ` Laurent Pinchart
  0 siblings, 1 reply; 23+ messages in thread
From: Hideki EIRAKU @ 2012-12-11 10:10 UTC (permalink / raw)
  To: laurent.pinchart
  Cc: lethal, magnus.damm, linux, horms, linux-sh, linux-arm-kernel,
	linux-kernel, m.szyprowski, matsu, dhobsong

Hi Laurent,

Thank you for your comments.

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Subject: Re: [PATCH v4 1/2] iommu/shmobile: Add iommu driver for Renesas IPMMU modules
Date: Mon, 10 Dec 2012 16:55:58 +0100

> On Monday 15 October 2012 17:34:52 Hideki EIRAKU wrote:
>> This is the Renesas IPMMU driver and IOMMU API implementation.
>> 
>> The IPMMU module supports the MMU function and the PMB function.
> 
> That sentence make me believe that both MMU and PMB were supported by the 
> driver, as "module" often refers to Linux kernel modules in this context. 
> Maybe you could replace "module" by "hardware module".

OK,

>> The MMU function provides address translation by pagetable compatible with
>> ARMv6. The PMB function provides address translation including tile-linear
>> translation. This patch implements the MMU function.
>>
>> The iommu driver does not register a platform driver directly because:
>> - the register space of the MMU function and the PMB function
>>   have a common register (used for settings flush), so they should ideally
>>   have a way to appropriately share this register.
>> - the MMU function uses the IOMMU API while the PMB function does not.
>> - the two functions may be used independently.
>> 
>> Signed-off-by: Hideki EIRAKU <hdk@igel.co.jp>
>> ---
>>  arch/arm/mach-shmobile/Kconfig              |    6 +
>>  arch/arm/mach-shmobile/Makefile             |    3 +
>>  arch/arm/mach-shmobile/include/mach/ipmmu.h |   16 ++
>>  arch/arm/mach-shmobile/ipmmu.c              |  150 ++++++++++++
>>  drivers/iommu/Kconfig                       |   56 +++++
>>  drivers/iommu/Makefile                      |    1 +
>>  drivers/iommu/shmobile-iommu.c              |  352 ++++++++++++++++++++++++
>>  7 files changed, 584 insertions(+), 0 deletions(-)
>>  create mode 100644 arch/arm/mach-shmobile/include/mach/ipmmu.h
>>  create mode 100644 arch/arm/mach-shmobile/ipmmu.c
>>  create mode 100644 drivers/iommu/shmobile-iommu.c
> 
> What is the reason for splitting the driver in two files ? Can't you put all 
> the code in drivers/iommu/shmobile-iommu.c ? Storing driver code in arch/* is 
> discouraged.

The reason is that I described in the above text. The PMB function is
completely different from the MMU function but both functions are on
the same IPMMU hardware module and sharing the register space. I think
that a driver using the PMB part which is not yet released should not
depend on the Linux's iommu interface, so I split the driver in two
files: the IPMMU platform driver part (in arch/arm/mach-shmobile/) and
Linux's iommu part (in drivers/iommu/).  For the IPMMU platform driver part,
do you have any suggestions other than arch/* where this should go? It is
a generic platform device.

>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 
>> USA
> 
> You can remove this last paragraph, we don't want to patch every file in the 
> kernel if the FSF moves to a new building :-)

OK, I will remove the paragraph.

>> +	for (dev = ipmmu_devices; dev; dev = dev->archdata.iommu) {
>> +		if (arm_iommu_attach_device(dev, iommu_mapping))
>> +			pr_err("arm_iommu_attach_device failed\n");
>> +	}
>> +err:
>> +	spin_unlock(&lock_add);
>> +	return 0;
>> +}
>> +
>> +void ipmmu_add_device(struct device *dev)
>> +{
>> +	spin_lock(&lock_add);
>> +	dev->archdata.iommu = ipmmu_devices;
>> +	ipmmu_devices = dev;
> 
> That looks a bit hackish to me. I'd like to suggest a different approach, that 
> would be compatible with supporting multiple IPMMU instances.
> 
> dev->archdata.iommu should point to a new sh_ipmmu_arch_data structure that 
> would contain an IPMMU name (const char *) and a pointer to a struct 
> shmobile_iommu_priv.
> 
> ipmmu_add_device() would take a new IPMMU name argument, allocate an 
> sh_ipmmu_arch_data instance dynamically and initialize its name field to the 
> name passed to the function. The shmobile_iommu_priv pointer would be set to 
> NULL. No other operation would be performed (you will likely get rid of the 
> global ipmmu_devices and iommu_mapping variables).
> 
> Then, the attach_dev operation handler would retrieve the dev->archdata.iommu 
> pointer, cast that to an sh_ipmmu_arch_data, and retrieve the IPMMU associated 
> with the name (either by walking a driver-global list of IPMMUs, or by using 
> driver_find_device()).
> 
> This mechanism would get rid of several global variables in the driver 
> (several of them would move to the shmobile_ipmmu_priv structure - which I 
> would have named shmobile_ipmmu or even sh_ipmmu, but that's up to you) and 
> add support for several IPMMU instances (there's 3 of them in the sh7372, even 
> if we only need to support one right now it's still a good practice to design 
> the driver in a way that multiple instances can be supported).
> 
> Could you try to rework the driiver in that direction ? You can have a look at 
> the OMAP IOMMU driver if you need sample code, and obviously feel free to 
> contact me if you have any question.

I agree about this is hackish. I don't mean to make an excuse, but I
could not find good sample code because no other drivers in the
upstream kernel use the arm_iommu_attach_device() API.

But I will try to modify the driver to support for several IPMMU
instances.

-- 
Hideki EIRAKU <hdk@igel.co.jp>

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

* Re: [PATCH v4 1/2] iommu/shmobile: Add iommu driver for Renesas IPMMU modules
  2012-12-11 10:10     ` Hideki EIRAKU
@ 2012-12-11 12:36       ` Laurent Pinchart
  0 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2012-12-11 12:36 UTC (permalink / raw)
  To: Hideki EIRAKU
  Cc: lethal, magnus.damm, linux, horms, linux-sh, linux-arm-kernel,
	linux-kernel, m.szyprowski, matsu, dhobsong

Hi Eiraku-san,

On Tuesday 11 December 2012 19:10:42 Hideki EIRAKU wrote:
> On Mon, 10 Dec 2012 16:55:58 +0100 Laurent Pinchart wrote:
> > On Monday 15 October 2012 17:34:52 Hideki EIRAKU wrote:
> >> This is the Renesas IPMMU driver and IOMMU API implementation.
> >> 
> >> The IPMMU module supports the MMU function and the PMB function.
> > 
> > That sentence make me believe that both MMU and PMB were supported by the
> > driver, as "module" often refers to Linux kernel modules in this context.
> > Maybe you could replace "module" by "hardware module".
> 
> OK,
> 
> >> The MMU function provides address translation by pagetable compatible
> >> with ARMv6. The PMB function provides address translation including
> >> tile-linear translation. This patch implements the MMU function.
> >> 
> >> The iommu driver does not register a platform driver directly because:
> >> - the register space of the MMU function and the PMB function
> >>   have a common register (used for settings flush), so they should
> >>   ideally have a way to appropriately share this register.
> >> - the MMU function uses the IOMMU API while the PMB function does not.
> >> - the two functions may be used independently.
> >> 
> >> Signed-off-by: Hideki EIRAKU <hdk@igel.co.jp>
> >> ---
> >> 
> >>  arch/arm/mach-shmobile/Kconfig              |    6 +
> >>  arch/arm/mach-shmobile/Makefile             |    3 +
> >>  arch/arm/mach-shmobile/include/mach/ipmmu.h |   16 ++
> >>  arch/arm/mach-shmobile/ipmmu.c              |  150 ++++++++++++
> >>  drivers/iommu/Kconfig                       |   56 +++++
> >>  drivers/iommu/Makefile                      |    1 +
> >>  drivers/iommu/shmobile-iommu.c              |  352 +++++++++++++++++++++
> >>  7 files changed, 584 insertions(+), 0 deletions(-)
> >>  create mode 100644 arch/arm/mach-shmobile/include/mach/ipmmu.h
> >>  create mode 100644 arch/arm/mach-shmobile/ipmmu.c
> >>  create mode 100644 drivers/iommu/shmobile-iommu.c
> > 
> > What is the reason for splitting the driver in two files ? Can't you put
> > all the code in drivers/iommu/shmobile-iommu.c ? Storing driver code in
> > arch/* is discouraged.
> 
> The reason is that I described in the above text. The PMB function is
> completely different from the MMU function but both functions are on
> the same IPMMU hardware module and sharing the register space. I think
> that a driver using the PMB part which is not yet released should not
> depend on the Linux's iommu interface, so I split the driver in two
> files: the IPMMU platform driver part (in arch/arm/mach-shmobile/) and
> Linux's iommu part (in drivers/iommu/). For the IPMMU platform driver part,
> do you have any suggestions other than arch/* where this should go? It is a
> generic platform device.

I think both parts should go to drivers/iommu/. You can keep the code split 
across two files, but I think you should register a single platform driver. 
The IPMMU is a single hardware module, so it should be handled by a single 
driver. That driver can expose two different APIs (IOMMU and whatever API will 
be used for PMB), and you can make those APIs selectable as Kconfig options, 
but they should in my opinion be implemented in a single driver.

> >> + * You should have received a copy of the GNU General Public License
> >> + * along with this program; if not, write to the Free Software
> >> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301
> >> USA
> > 
> > You can remove this last paragraph, we don't want to patch every file in
> > the kernel if the FSF moves to a new building :-)
> 
> OK, I will remove the paragraph.
> 
> >> +	for (dev = ipmmu_devices; dev; dev = dev->archdata.iommu) {
> >> +		if (arm_iommu_attach_device(dev, iommu_mapping))
> >> +			pr_err("arm_iommu_attach_device failed\n");
> >> +	}
> >> +err:
> >> +	spin_unlock(&lock_add);
> >> +	return 0;
> >> +}
> >> +
> >> +void ipmmu_add_device(struct device *dev)
> >> +{
> >> +	spin_lock(&lock_add);
> >> +	dev->archdata.iommu = ipmmu_devices;
> >> +	ipmmu_devices = dev;
> > 
> > That looks a bit hackish to me. I'd like to suggest a different approach,
> > that would be compatible with supporting multiple IPMMU instances.
> > 
> > dev->archdata.iommu should point to a new sh_ipmmu_arch_data structure
> > that would contain an IPMMU name (const char *) and a pointer to a struct
> > shmobile_iommu_priv.
> > 
> > ipmmu_add_device() would take a new IPMMU name argument, allocate an
> > sh_ipmmu_arch_data instance dynamically and initialize its name field to
> > the name passed to the function. The shmobile_iommu_priv pointer would be
> > set to NULL. No other operation would be performed (you will likely get
> > rid of the global ipmmu_devices and iommu_mapping variables).
> > 
> > Then, the attach_dev operation handler would retrieve the
> > dev->archdata.iommu pointer, cast that to an sh_ipmmu_arch_data, and
> > retrieve the IPMMU associated with the name (either by walking a
> > driver-global list of IPMMUs, or by using driver_find_device()).
> > 
> > This mechanism would get rid of several global variables in the driver
> > (several of them would move to the shmobile_ipmmu_priv structure - which I
> > would have named shmobile_ipmmu or even sh_ipmmu, but that's up to you)
> > and add support for several IPMMU instances (there's 3 of them in the
> > sh7372, even if we only need to support one right now it's still a good
> > practice to design the driver in a way that multiple instances can be
> > supported).
> > 
> > Could you try to rework the driiver in that direction ? You can have a
> > look at the OMAP IOMMU driver if you need sample code, and obviously feel
> > free to contact me if you have any question.
> 
> I agree about this is hackish. I don't mean to make an excuse,

And I'm not blaming you :-)

> but I could not find good sample code because no other drivers in the
> upstream kernel use the arm_iommu_attach_device() API.

This is all pretty new code, so reference implementations are missing, that's 
true.

> But I will try to modify the driver to support for several IPMMU
> instances.

Thank you. I can also give it a try if you want, just drop me an e-mail.

-- 
Regards,

Laurent Pinchart


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

* [PATCH/WIP/RFC 00/14] Renesas IPMMU driver work in progress
  2012-10-15  8:34 [PATCH v4 0/2] Renesas IPMMU driver for sh7372 Hideki EIRAKU
  2012-10-15  8:34 ` [PATCH v4 1/2] iommu/shmobile: Add iommu driver for Renesas IPMMU modules Hideki EIRAKU
  2012-10-15  8:34 ` [PATCH v4 2/2] ARM: mach-shmobile: sh7372: Add IPMMU device Hideki EIRAKU
@ 2012-12-16 17:25 ` Laurent Pinchart
  2012-12-16 17:25   ` [PATCH/WIP/RFC 01/14] ARM: sh-mobile: Protect ipmmu.h header with ifndef/define Laurent Pinchart
                     ` (13 more replies)
  2 siblings, 14 replies; 23+ messages in thread
From: Laurent Pinchart @ 2012-12-16 17:25 UTC (permalink / raw)
  To: Hideki EIRAKU
  Cc: Paul Mundt, Magnus Damm, Simon Horman, linux-sh,
	linux-arm-kernel, linux-kernel, Marek Szyprowski,
	Katsuya MATSUBARA, Damian Hobson-Garcia

Dear Eiraku-san,

Here's a couple of patches that rework your IPMMU driver in the direction
pointed to by my comments. Feel free to use them as a base, squash them into
your code (my name doesn't need to be kept in commit messages) or even ignore
them completely where they make no sense.

Patch 5/14 has been sent to the linux-arm-kernel mailing list already as it's
an independent fix (but required by the next patches in the series).

After these 14 patches I ran into the issue of matching devices with a
particular IPMMU instance. This turned out to be more complex than I initially
thought. There's currently no driver in mainline that performs that task, and
I'm not sure whether the core infrastructure allows for doing so without
resorting to hacks.

The problem has been discussed very recently on the iommu list, you can find
more information at http://patchwork.ozlabs.org/patch/203717/. No solution has
been agreed on so far, maybe you could reply to the mail thread. Feel free to
CC me.

I'm unsure whether we should use a single ARM IOMMU mapping or one mapping per
device. This needs to be investigated as there's little documentation on the
subject. I would advice contacting Marek Szyprowski (the author of the ARM DMA
IOMMU code) to discuss this matter. Once again feel free to CC me, as well as
the appropriate mailing lists.

In my opinion the shmobile_iommu_attach_all_devices() function should be
removed, and devices should be attached with an IOMMU either as they are added
to the system, or as they are probed. Bus notifiers might be usable to achieve
that, as implemented in the above Tegra SMMU patch. This requires the IOMMU
being registered and fully operational at attachment time, so explicit IOMMU
attachment in driver code might be a better solution for now. That would
likely require one ARM DMA IOMMU mapping per device, device drivers would
explicitly call arm_iommu_create_mapping() and arm_iommu_attach_device().

Laurent Pinchart (14):
  ARM: sh-mobile: Protect ipmmu.h header with ifndef/define
  shmobile-iommu: Move IPMMU driver to drivers/iommu
  shmobile-iommu: Remove __devinit
  shmobile-iommu: Use devm_* managed functions
  ARM: iommu: Include linux/kref.h in asm/dma-iommu.h
  shmobile-iommu: Sort header files alphabetically
  shmobile-iommu: Move header file from arch/ to drivers/iommu/
  shmobile-iommu: Rename shmobile_iommu_priv to shmobile_iommu_domain
  shmobile-ipmmu: Rename ipmmu_priv to shmobile_ipmmu
  shmobile-ipmmu: Pass a struct shmobile_ipmmu to IPMMU functions
  shmobile-ipmmu: Store a struct shmobile_iommu_arch_data in
    archdata.iommu
  shmobile-ipmmu: Store ipmmu pointer in iommu arch data and iommu
    domain
  shmobile-ipmmu: Remove unneeded lock_add spinlock
  shmobile-ipmmu: Store iommu_mapping in struct shmobile_ipmmu

 arch/arm/include/asm/dma-iommu.h                   |    1 +
 arch/arm/mach-shmobile/Kconfig                     |    6 -
 arch/arm/mach-shmobile/Makefile                    |    3 -
 arch/arm/mach-shmobile/board-ap4evb.c              |    2 +-
 arch/arm/mach-shmobile/board-mackerel.c            |    2 +-
 arch/arm/mach-shmobile/include/mach/ipmmu.h        |   16 --
 arch/arm/mach-shmobile/setup-sh7372.c              |    2 +-
 drivers/iommu/Kconfig                              |    6 +
 drivers/iommu/Makefile                             |    1 +
 drivers/iommu/shmobile-iommu.c                     |  259 +++++++++++---------
 .../ipmmu.c => drivers/iommu/shmobile-ipmmu.c      |  107 ++++-----
 drivers/iommu/shmobile-ipmmu.h                     |   27 ++
 include/linux/sh_iommu.h                           |   20 ++
 13 files changed, 248 insertions(+), 204 deletions(-)
 delete mode 100644 arch/arm/mach-shmobile/include/mach/ipmmu.h
 rename arch/arm/mach-shmobile/ipmmu.c => drivers/iommu/shmobile-ipmmu.c (51%)
 create mode 100644 drivers/iommu/shmobile-ipmmu.h
 create mode 100644 include/linux/sh_iommu.h

-- 
Regards,

Laurent Pinchart


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

* [PATCH/WIP/RFC 01/14] ARM: sh-mobile: Protect ipmmu.h header with ifndef/define
  2012-12-16 17:25 ` [PATCH/WIP/RFC 00/14] Renesas IPMMU driver work in progress Laurent Pinchart
@ 2012-12-16 17:25   ` Laurent Pinchart
  2012-12-16 17:25   ` [PATCH/WIP/RFC 02/14] shmobile-iommu: Move IPMMU driver to drivers/iommu Laurent Pinchart
                     ` (12 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2012-12-16 17:25 UTC (permalink / raw)
  To: Hideki EIRAKU
  Cc: Paul Mundt, Magnus Damm, Simon Horman, linux-sh,
	linux-arm-kernel, linux-kernel, Marek Szyprowski,
	Katsuya MATSUBARA, Damian Hobson-Garcia

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm/mach-shmobile/include/mach/ipmmu.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-shmobile/include/mach/ipmmu.h b/arch/arm/mach-shmobile/include/mach/ipmmu.h
index ede2f0b..4b805b1f 100644
--- a/arch/arm/mach-shmobile/include/mach/ipmmu.h
+++ b/arch/arm/mach-shmobile/include/mach/ipmmu.h
@@ -1,3 +1,6 @@
+#ifndef __SHMOBILE_IPMMU_H__
+#define __SHMOBILE_IPMMU_H__
+
 #ifdef CONFIG_SHMOBILE_IPMMU_TLB
 void ipmmu_tlb_flush(struct device *ipmmu_dev);
 void ipmmu_tlb_set(struct device *ipmmu_dev, unsigned long phys, int size,
@@ -14,3 +17,5 @@ static int ipmmu_iommu_init(struct device *dev)
 	return -EINVAL;
 }
 #endif
+
+#endif /* __SHMOBILE_IPMMU_H__ */
-- 
1.7.8.6


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

* [PATCH/WIP/RFC 02/14] shmobile-iommu: Move IPMMU driver to drivers/iommu
  2012-12-16 17:25 ` [PATCH/WIP/RFC 00/14] Renesas IPMMU driver work in progress Laurent Pinchart
  2012-12-16 17:25   ` [PATCH/WIP/RFC 01/14] ARM: sh-mobile: Protect ipmmu.h header with ifndef/define Laurent Pinchart
@ 2012-12-16 17:25   ` Laurent Pinchart
  2012-12-17  3:10     ` Damian Hobson-Garcia
  2012-12-16 17:25   ` [PATCH/WIP/RFC 03/14] shmobile-iommu: Remove __devinit Laurent Pinchart
                     ` (11 subsequent siblings)
  13 siblings, 1 reply; 23+ messages in thread
From: Laurent Pinchart @ 2012-12-16 17:25 UTC (permalink / raw)
  To: Hideki EIRAKU
  Cc: Paul Mundt, Magnus Damm, Simon Horman, linux-sh,
	linux-arm-kernel, linux-kernel, Marek Szyprowski,
	Katsuya MATSUBARA, Damian Hobson-Garcia

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm/mach-shmobile/Kconfig                     |    6 ------
 arch/arm/mach-shmobile/Makefile                    |    3 ---
 drivers/iommu/Kconfig                              |    6 ++++++
 drivers/iommu/Makefile                             |    1 +
 .../ipmmu.c => drivers/iommu/shmobile-ipmmu.c      |    0
 5 files changed, 7 insertions(+), 9 deletions(-)
 rename arch/arm/mach-shmobile/ipmmu.c => drivers/iommu/shmobile-ipmmu.c (100%)

diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
index de69ab3..8ae100c 100644
--- a/arch/arm/mach-shmobile/Kconfig
+++ b/arch/arm/mach-shmobile/Kconfig
@@ -210,12 +210,6 @@ endmenu
 config SH_CLK_CPG
 	bool
 
-config SHMOBILE_IPMMU
-	bool
-
-config SHMOBILE_IPMMU_TLB
-	bool
-
 source "drivers/sh/Kconfig"
 
 endif
diff --git a/arch/arm/mach-shmobile/Makefile b/arch/arm/mach-shmobile/Makefile
index 4ffba9d..fe2c97c 100644
--- a/arch/arm/mach-shmobile/Makefile
+++ b/arch/arm/mach-shmobile/Makefile
@@ -60,6 +60,3 @@ obj-$(CONFIG_MACH_KZM9G)	+= board-kzm9g.o
 # Framework support
 obj-$(CONFIG_SMP)		+= $(smp-y)
 obj-$(CONFIG_GENERIC_GPIO)	+= $(pfc-y)
-
-# IPMMU/IPMMUI
-obj-$(CONFIG_SHMOBILE_IPMMU)	+= ipmmu.o
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 265829f..cb2a3e8 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -187,6 +187,12 @@ config EXYNOS_IOMMU_DEBUG
 
 	  Say N unless you need kernel log message for IOMMU debugging
 
+config SHMOBILE_IPMMU
+	bool
+
+config SHMOBILE_IPMMU_TLB
+	bool
+
 config SHMOBILE_IOMMU
 	bool "IOMMU for Renesas IPMMU/IPMMUI"
 	default n
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 62cf917..e65384f 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_TEGRA_IOMMU_GART) += tegra-gart.o
 obj-$(CONFIG_TEGRA_IOMMU_SMMU) += tegra-smmu.o
 obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o
 obj-$(CONFIG_SHMOBILE_IOMMU) += shmobile-iommu.o
+obj-$(CONFIG_SHMOBILE_IPMMU) += shmobile-ipmmu.o
diff --git a/arch/arm/mach-shmobile/ipmmu.c b/drivers/iommu/shmobile-ipmmu.c
similarity index 100%
rename from arch/arm/mach-shmobile/ipmmu.c
rename to drivers/iommu/shmobile-ipmmu.c
-- 
1.7.8.6


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

* [PATCH/WIP/RFC 03/14] shmobile-iommu: Remove __devinit
  2012-12-16 17:25 ` [PATCH/WIP/RFC 00/14] Renesas IPMMU driver work in progress Laurent Pinchart
  2012-12-16 17:25   ` [PATCH/WIP/RFC 01/14] ARM: sh-mobile: Protect ipmmu.h header with ifndef/define Laurent Pinchart
  2012-12-16 17:25   ` [PATCH/WIP/RFC 02/14] shmobile-iommu: Move IPMMU driver to drivers/iommu Laurent Pinchart
@ 2012-12-16 17:25   ` Laurent Pinchart
  2012-12-16 17:25   ` [PATCH/WIP/RFC 04/14] shmobile-iommu: Use devm_* managed functions Laurent Pinchart
                     ` (10 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2012-12-16 17:25 UTC (permalink / raw)
  To: Hideki EIRAKU
  Cc: Paul Mundt, Magnus Damm, Simon Horman, linux-sh,
	linux-arm-kernel, linux-kernel, Marek Szyprowski,
	Katsuya MATSUBARA, Damian Hobson-Garcia

CONFIG_HOTPLUG is going away as an option, remove __devinit.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/shmobile-ipmmu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/iommu/shmobile-ipmmu.c b/drivers/iommu/shmobile-ipmmu.c
index 72cacb9..2339f91 100644
--- a/drivers/iommu/shmobile-ipmmu.c
+++ b/drivers/iommu/shmobile-ipmmu.c
@@ -106,7 +106,7 @@ void ipmmu_tlb_set(struct device *dev, unsigned long phys, int size, int asid)
 	mutex_unlock(&priv->flush_lock);
 }
 
-static int __devinit ipmmu_probe(struct platform_device *pdev)
+static int ipmmu_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	struct ipmmu_priv *priv;
-- 
1.7.8.6


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

* [PATCH/WIP/RFC 04/14] shmobile-iommu: Use devm_* managed functions
  2012-12-16 17:25 ` [PATCH/WIP/RFC 00/14] Renesas IPMMU driver work in progress Laurent Pinchart
                     ` (2 preceding siblings ...)
  2012-12-16 17:25   ` [PATCH/WIP/RFC 03/14] shmobile-iommu: Remove __devinit Laurent Pinchart
@ 2012-12-16 17:25   ` Laurent Pinchart
  2012-12-16 17:25   ` [PATCH/WIP/RFC 05/14] ARM: iommu: Include linux/kref.h in asm/dma-iommu.h Laurent Pinchart
                     ` (9 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2012-12-16 17:25 UTC (permalink / raw)
  To: Hideki EIRAKU
  Cc: Paul Mundt, Magnus Damm, Simon Horman, linux-sh,
	linux-arm-kernel, linux-kernel, Marek Szyprowski,
	Katsuya MATSUBARA, Damian Hobson-Garcia

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/shmobile-ipmmu.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/shmobile-ipmmu.c b/drivers/iommu/shmobile-ipmmu.c
index 2339f91..d6df0b4 100644
--- a/drivers/iommu/shmobile-ipmmu.c
+++ b/drivers/iommu/shmobile-ipmmu.c
@@ -116,16 +116,16 @@ static int ipmmu_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "cannot get platform resources\n");
 		return -ENOENT;
 	}
-	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv) {
 		dev_err(&pdev->dev, "cannot allocate device data\n");
 		return -ENOMEM;
 	}
 	mutex_init(&priv->flush_lock);
-	priv->ipmmu_base = ioremap_nocache(res->start, resource_size(res));
+	priv->ipmmu_base = devm_ioremap_nocache(&pdev->dev, res->start,
+						resource_size(res));
 	if (!priv->ipmmu_base) {
 		dev_err(&pdev->dev, "ioremap_nocache failed\n");
-		kfree(priv);
 		return -ENOMEM;
 	}
 	platform_set_drvdata(pdev, priv);
-- 
1.7.8.6


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

* [PATCH/WIP/RFC 05/14] ARM: iommu: Include linux/kref.h in asm/dma-iommu.h
  2012-12-16 17:25 ` [PATCH/WIP/RFC 00/14] Renesas IPMMU driver work in progress Laurent Pinchart
                     ` (3 preceding siblings ...)
  2012-12-16 17:25   ` [PATCH/WIP/RFC 04/14] shmobile-iommu: Use devm_* managed functions Laurent Pinchart
@ 2012-12-16 17:25   ` Laurent Pinchart
  2012-12-16 17:25   ` [PATCH/WIP/RFC 06/14] shmobile-iommu: Sort header files alphabetically Laurent Pinchart
                     ` (8 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2012-12-16 17:25 UTC (permalink / raw)
  To: Hideki EIRAKU
  Cc: Paul Mundt, Magnus Damm, Simon Horman, linux-sh,
	linux-arm-kernel, linux-kernel, Marek Szyprowski,
	Katsuya MATSUBARA, Damian Hobson-Garcia

The dma_iommu_mapping structure defined in asm/dma-iommu.h embeds a
struct kref, include the appropriate header file.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 arch/arm/include/asm/dma-iommu.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/dma-iommu.h b/arch/arm/include/asm/dma-iommu.h
index 799b094..666d8a8 100644
--- a/arch/arm/include/asm/dma-iommu.h
+++ b/arch/arm/include/asm/dma-iommu.h
@@ -7,6 +7,7 @@
 #include <linux/scatterlist.h>
 #include <linux/dma-debug.h>
 #include <linux/kmemcheck.h>
+#include <linux/kref.h>
 
 struct dma_iommu_mapping {
 	/* iommu specific data */
-- 
1.7.8.6


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

* [PATCH/WIP/RFC 06/14] shmobile-iommu: Sort header files alphabetically
  2012-12-16 17:25 ` [PATCH/WIP/RFC 00/14] Renesas IPMMU driver work in progress Laurent Pinchart
                     ` (4 preceding siblings ...)
  2012-12-16 17:25   ` [PATCH/WIP/RFC 05/14] ARM: iommu: Include linux/kref.h in asm/dma-iommu.h Laurent Pinchart
@ 2012-12-16 17:25   ` Laurent Pinchart
  2012-12-16 17:25   ` [PATCH/WIP/RFC 07/14] shmobile-iommu: Move header file from arch/ to drivers/iommu/ Laurent Pinchart
                     ` (7 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2012-12-16 17:25 UTC (permalink / raw)
  To: Hideki EIRAKU
  Cc: Paul Mundt, Magnus Damm, Simon Horman, linux-sh,
	linux-arm-kernel, linux-kernel, Marek Szyprowski,
	Katsuya MATSUBARA, Damian Hobson-Garcia

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/shmobile-iommu.c |   10 +++++-----
 drivers/iommu/shmobile-ipmmu.c |    4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/shmobile-iommu.c b/drivers/iommu/shmobile-iommu.c
index bbbf1bc..9f2243e3 100644
--- a/drivers/iommu/shmobile-iommu.c
+++ b/drivers/iommu/shmobile-iommu.c
@@ -17,14 +17,14 @@
  *
  */
 
-#include <linux/io.h>
+#include <asm/dma-iommu.h>
+#include <linux/dma-mapping.h>
 #include <linux/dmapool.h>
-#include <linux/slab.h>
-#include <linux/platform_device.h>
+#include <linux/io.h>
 #include <linux/iommu.h>
-#include <linux/dma-mapping.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
 #include <mach/ipmmu.h>
-#include <asm/dma-iommu.h>
 
 #define L1_SIZE CONFIG_SHMOBILE_IOMMU_L1SIZE
 #define L1_LEN (L1_SIZE / 4)
diff --git a/drivers/iommu/shmobile-ipmmu.c b/drivers/iommu/shmobile-ipmmu.c
index d6df0b4..edec3b8 100644
--- a/drivers/iommu/shmobile-ipmmu.c
+++ b/drivers/iommu/shmobile-ipmmu.c
@@ -17,10 +17,10 @@
  *
  */
 
-#include <linux/platform_device.h>
-#include <linux/io.h>
 #include <linux/err.h>
 #include <linux/export.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <mach/ipmmu.h>
 
-- 
1.7.8.6


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

* [PATCH/WIP/RFC 07/14] shmobile-iommu: Move header file from arch/ to drivers/iommu/
  2012-12-16 17:25 ` [PATCH/WIP/RFC 00/14] Renesas IPMMU driver work in progress Laurent Pinchart
                     ` (5 preceding siblings ...)
  2012-12-16 17:25   ` [PATCH/WIP/RFC 06/14] shmobile-iommu: Sort header files alphabetically Laurent Pinchart
@ 2012-12-16 17:25   ` Laurent Pinchart
  2012-12-16 17:25   ` [PATCH/WIP/RFC 08/14] shmobile-iommu: Rename shmobile_iommu_priv to shmobile_iommu_domain Laurent Pinchart
                     ` (6 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2012-12-16 17:25 UTC (permalink / raw)
  To: Hideki EIRAKU
  Cc: Paul Mundt, Magnus Damm, Simon Horman, linux-sh,
	linux-arm-kernel, linux-kernel, Marek Szyprowski,
	Katsuya MATSUBARA, Damian Hobson-Garcia

And split the function used by board code to its own
include/linux/sh_iommu.h header.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm/mach-shmobile/board-ap4evb.c              |    2 +-
 arch/arm/mach-shmobile/board-mackerel.c            |    2 +-
 arch/arm/mach-shmobile/setup-sh7372.c              |    2 +-
 drivers/iommu/shmobile-iommu.c                     |    4 +++-
 drivers/iommu/shmobile-ipmmu.c                     |    3 ++-
 .../mach/ipmmu.h => drivers/iommu/shmobile-ipmmu.h |    5 -----
 include/linux/sh_iommu.h                           |   12 ++++++++++++
 7 files changed, 20 insertions(+), 10 deletions(-)
 rename arch/arm/mach-shmobile/include/mach/ipmmu.h => drivers/iommu/shmobile-ipmmu.h (78%)
 create mode 100644 include/linux/sh_iommu.h

diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index 7006abb..0c40d72 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -38,6 +38,7 @@
 #include <linux/regulator/machine.h>
 #include <linux/smsc911x.h>
 #include <linux/sh_intc.h>
+#include <linux/sh_iommu.h>
 #include <linux/sh_clk.h>
 #include <linux/gpio.h>
 #include <linux/input.h>
@@ -61,7 +62,6 @@
 #include <mach/common.h>
 #include <mach/irqs.h>
 #include <mach/sh7372.h>
-#include <mach/ipmmu.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
index 1c34520..a9cdfc2 100644
--- a/arch/arm/mach-shmobile/board-mackerel.c
+++ b/arch/arm/mach-shmobile/board-mackerel.c
@@ -45,6 +45,7 @@
 #include <linux/regulator/machine.h>
 #include <linux/smsc911x.h>
 #include <linux/sh_intc.h>
+#include <linux/sh_iommu.h>
 #include <linux/tca6416_keypad.h>
 #include <linux/usb/renesas_usbhs.h>
 #include <linux/dma-mapping.h>
@@ -60,7 +61,6 @@
 #include <mach/common.h>
 #include <mach/irqs.h>
 #include <mach/sh7372.h>
-#include <mach/ipmmu.h>
 
 #include <asm/mach/arch.h>
 #include <asm/mach-types.h>
diff --git a/arch/arm/mach-shmobile/setup-sh7372.c b/arch/arm/mach-shmobile/setup-sh7372.c
index aadb769..5dd3576 100644
--- a/arch/arm/mach-shmobile/setup-sh7372.c
+++ b/arch/arm/mach-shmobile/setup-sh7372.c
@@ -30,6 +30,7 @@
 #include <linux/serial_sci.h>
 #include <linux/sh_dma.h>
 #include <linux/sh_intc.h>
+#include <linux/sh_iommu.h>
 #include <linux/sh_timer.h>
 #include <linux/pm_domain.h>
 #include <linux/dma-mapping.h>
@@ -38,7 +39,6 @@
 #include <mach/irqs.h>
 #include <mach/sh7372.h>
 #include <mach/common.h>
-#include <mach/ipmmu.h>
 #include <asm/mach/map.h>
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
diff --git a/drivers/iommu/shmobile-iommu.c b/drivers/iommu/shmobile-iommu.c
index 9f2243e3..463da32 100644
--- a/drivers/iommu/shmobile-iommu.c
+++ b/drivers/iommu/shmobile-iommu.c
@@ -23,8 +23,10 @@
 #include <linux/io.h>
 #include <linux/iommu.h>
 #include <linux/platform_device.h>
+#include <linux/sh_iommu.h>
 #include <linux/slab.h>
-#include <mach/ipmmu.h>
+
+#include "shmobile-ipmmu.h"
 
 #define L1_SIZE CONFIG_SHMOBILE_IOMMU_L1SIZE
 #define L1_LEN (L1_SIZE / 4)
diff --git a/drivers/iommu/shmobile-ipmmu.c b/drivers/iommu/shmobile-ipmmu.c
index edec3b8..34e3c66 100644
--- a/drivers/iommu/shmobile-ipmmu.c
+++ b/drivers/iommu/shmobile-ipmmu.c
@@ -22,7 +22,8 @@
 #include <linux/io.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
-#include <mach/ipmmu.h>
+
+#include "shmobile-ipmmu.h"
 
 #define IMCTR1 0x000
 #define IMCTR2 0x004
diff --git a/arch/arm/mach-shmobile/include/mach/ipmmu.h b/drivers/iommu/shmobile-ipmmu.h
similarity index 78%
rename from arch/arm/mach-shmobile/include/mach/ipmmu.h
rename to drivers/iommu/shmobile-ipmmu.h
index 4b805b1f..0e0a6a4 100644
--- a/arch/arm/mach-shmobile/include/mach/ipmmu.h
+++ b/drivers/iommu/shmobile-ipmmu.h
@@ -5,13 +5,8 @@
 void ipmmu_tlb_flush(struct device *ipmmu_dev);
 void ipmmu_tlb_set(struct device *ipmmu_dev, unsigned long phys, int size,
 		   int asid);
-void ipmmu_add_device(struct device *dev);
 int ipmmu_iommu_init(struct device *dev);
 #else
-static inline void ipmmu_add_device(struct device *dev)
-{
-}
-
 static int ipmmu_iommu_init(struct device *dev)
 {
 	return -EINVAL;
diff --git a/include/linux/sh_iommu.h b/include/linux/sh_iommu.h
new file mode 100644
index 0000000..cc669a0
--- /dev/null
+++ b/include/linux/sh_iommu.h
@@ -0,0 +1,12 @@
+#ifndef __SH_IOMMU_H__
+#define __SH_IOMMU_H__
+
+#ifdef CONFIG_SHMOBILE_IPMMU_TLB
+void ipmmu_add_device(struct device *dev);
+#else
+static inline void ipmmu_add_device(struct device *dev)
+{
+}
+#endif
+
+#endif /* __SH_IOMMU_H__ */
-- 
1.7.8.6


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

* [PATCH/WIP/RFC 08/14] shmobile-iommu: Rename shmobile_iommu_priv to shmobile_iommu_domain
  2012-12-16 17:25 ` [PATCH/WIP/RFC 00/14] Renesas IPMMU driver work in progress Laurent Pinchart
                     ` (6 preceding siblings ...)
  2012-12-16 17:25   ` [PATCH/WIP/RFC 07/14] shmobile-iommu: Move header file from arch/ to drivers/iommu/ Laurent Pinchart
@ 2012-12-16 17:25   ` Laurent Pinchart
  2012-12-16 17:25   ` [PATCH/WIP/RFC 09/14] shmobile-ipmmu: Rename ipmmu_priv to shmobile_ipmmu Laurent Pinchart
                     ` (5 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2012-12-16 17:25 UTC (permalink / raw)
  To: Hideki EIRAKU
  Cc: Paul Mundt, Magnus Damm, Simon Horman, linux-sh,
	linux-arm-kernel, linux-kernel, Marek Szyprowski,
	Katsuya MATSUBARA, Damian Hobson-Garcia

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/shmobile-iommu.c |  152 ++++++++++++++++++++--------------------
 1 files changed, 76 insertions(+), 76 deletions(-)

diff --git a/drivers/iommu/shmobile-iommu.c b/drivers/iommu/shmobile-iommu.c
index 463da32..1a37be2 100644
--- a/drivers/iommu/shmobile-iommu.c
+++ b/drivers/iommu/shmobile-iommu.c
@@ -35,13 +35,13 @@
 #define L2_LEN (L2_SIZE / 4)
 #define L2_ALIGN L2_SIZE
 
-struct shmobile_iommu_priv_pgtable {
+struct shmobile_iommu_domain_pgtable {
 	uint32_t *pgtable;
 	dma_addr_t handle;
 };
 
-struct shmobile_iommu_priv {
-	struct shmobile_iommu_priv_pgtable l1, l2[L1_LEN];
+struct shmobile_iommu_domain {
+	struct shmobile_iommu_domain_pgtable l1, l2[L1_LEN];
 	spinlock_t map_lock;
 	atomic_t active;
 };
@@ -51,64 +51,64 @@ static struct device *ipmmu_devices;
 static struct dma_pool *l1pool, *l2pool;
 static spinlock_t lock;
 static DEFINE_SPINLOCK(lock_add);
-static struct shmobile_iommu_priv *attached;
+static struct shmobile_iommu_domain *attached;
 static int num_attached_devices;
 static struct device *ipmmu_access_device;
 
 static int shmobile_iommu_domain_init(struct iommu_domain *domain)
 {
-	struct shmobile_iommu_priv *priv;
+	struct shmobile_iommu_domain *sh_domain;
 	int i;
 
-	priv = kmalloc(sizeof(*priv), GFP_KERNEL);
-	if (!priv)
+	sh_domain = kmalloc(sizeof(*sh_domain), GFP_KERNEL);
+	if (!sh_domain)
 		return -ENOMEM;
-	priv->l1.pgtable = dma_pool_alloc(l1pool, GFP_KERNEL,
-					  &priv->l1.handle);
-	if (!priv->l1.pgtable) {
-		kfree(priv);
+	sh_domain->l1.pgtable = dma_pool_alloc(l1pool, GFP_KERNEL,
+					       &sh_domain->l1.handle);
+	if (!sh_domain->l1.pgtable) {
+		kfree(sh_domain);
 		return -ENOMEM;
 	}
 	for (i = 0; i < L1_LEN; i++)
-		priv->l2[i].pgtable = NULL;
-	memset(priv->l1.pgtable, 0, L1_SIZE);
-	spin_lock_init(&priv->map_lock);
-	atomic_set(&priv->active, 0);
-	domain->priv = priv;
+		sh_domain->l2[i].pgtable = NULL;
+	memset(sh_domain->l1.pgtable, 0, L1_SIZE);
+	spin_lock_init(&sh_domain->map_lock);
+	atomic_set(&sh_domain->active, 0);
+	domain->priv = sh_domain;
 	return 0;
 }
 
 static void shmobile_iommu_domain_destroy(struct iommu_domain *domain)
 {
-	struct shmobile_iommu_priv *priv = domain->priv;
+	struct shmobile_iommu_domain *sh_domain = domain->priv;
 	int i;
 
 	for (i = 0; i < L1_LEN; i++) {
-		if (priv->l2[i].pgtable)
-			dma_pool_free(l2pool, priv->l2[i].pgtable,
-				      priv->l2[i].handle);
+		if (sh_domain->l2[i].pgtable)
+			dma_pool_free(l2pool, sh_domain->l2[i].pgtable,
+				      sh_domain->l2[i].handle);
 	}
-	dma_pool_free(l1pool, priv->l1.pgtable, priv->l1.handle);
-	kfree(priv);
+	dma_pool_free(l1pool, sh_domain->l1.pgtable, sh_domain->l1.handle);
+	kfree(sh_domain);
 	domain->priv = NULL;
 }
 
 static int shmobile_iommu_attach_device(struct iommu_domain *domain,
 					struct device *dev)
 {
-	struct shmobile_iommu_priv *priv = domain->priv;
+	struct shmobile_iommu_domain *sh_domain = domain->priv;
 	int ret = -EBUSY;
 
 	spin_lock(&lock);
-	if (attached != priv) {
+	if (attached != sh_domain) {
 		if (attached)
 			goto err;
-		atomic_set(&priv->active, 1);
-		ipmmu_tlb_set(ipmmu_access_device, priv->l1.handle, L1_SIZE,
+		atomic_set(&sh_domain->active, 1);
+		ipmmu_tlb_set(ipmmu_access_device, sh_domain->l1.handle, L1_SIZE,
 			      0);
 		wmb();
 		ipmmu_tlb_flush(ipmmu_access_device);
-		attached = priv;
+		attached = sh_domain;
 		num_attached_devices = 0;
 	}
 	num_attached_devices++;
@@ -121,10 +121,10 @@ err:
 static void shmobile_iommu_detach_device(struct iommu_domain *domain,
 					 struct device *dev)
 {
-	struct shmobile_iommu_priv *priv = domain->priv;
+	struct shmobile_iommu_domain *sh_domain = domain->priv;
 
 	spin_lock(&lock);
-	atomic_set(&priv->active, 0);
+	atomic_set(&sh_domain->active, 0);
 	num_attached_devices--;
 	if (!num_attached_devices) {
 		ipmmu_tlb_set(ipmmu_access_device, 0, 0, 0);
@@ -135,34 +135,34 @@ static void shmobile_iommu_detach_device(struct iommu_domain *domain,
 }
 
 static int
-l2alloc(struct shmobile_iommu_priv *priv, unsigned int l1index)
+l2alloc(struct shmobile_iommu_domain *sh_domain, unsigned int l1index)
 {
-	if (!priv->l2[l1index].pgtable) {
-		priv->l2[l1index].pgtable = dma_pool_alloc(l2pool, GFP_KERNEL,
-						&priv->l2[l1index].handle);
-		if (!priv->l2[l1index].pgtable)
+	if (!sh_domain->l2[l1index].pgtable) {
+		sh_domain->l2[l1index].pgtable = dma_pool_alloc(l2pool, GFP_KERNEL,
+						&sh_domain->l2[l1index].handle);
+		if (!sh_domain->l2[l1index].pgtable)
 			return -ENOMEM;
-		memset(priv->l2[l1index].pgtable, 0, L2_SIZE);
+		memset(sh_domain->l2[l1index].pgtable, 0, L2_SIZE);
 	}
-	priv->l1.pgtable[l1index] = priv->l2[l1index].handle | 0x1;
+	sh_domain->l1.pgtable[l1index] = sh_domain->l2[l1index].handle | 0x1;
 	return 0;
 }
 
 static void
-l2realfree(struct shmobile_iommu_priv_pgtable *l2)
+l2realfree(struct shmobile_iommu_domain_pgtable *l2)
 {
 	if (l2->pgtable)
 		dma_pool_free(l2pool, l2->pgtable, l2->handle);
 }
 
 static int
-l2free(struct shmobile_iommu_priv *priv, unsigned int l1index,
-	struct shmobile_iommu_priv_pgtable *l2)
+l2free(struct shmobile_iommu_domain *sh_domain, unsigned int l1index,
+	struct shmobile_iommu_domain_pgtable *l2)
 {
-	priv->l1.pgtable[l1index] = 0;
-	if (priv->l2[l1index].pgtable) {
-		*l2 = priv->l2[l1index];
-		priv->l2[l1index].pgtable = NULL;
+	sh_domain->l1.pgtable[l1index] = 0;
+	if (sh_domain->l2[l1index].pgtable) {
+		*l2 = sh_domain->l2[l1index];
+		sh_domain->l2[l1index].pgtable = NULL;
 	}
 	return 0;
 }
@@ -170,8 +170,8 @@ l2free(struct shmobile_iommu_priv *priv, unsigned int l1index,
 static int shmobile_iommu_map(struct iommu_domain *domain, unsigned long iova,
 			      phys_addr_t paddr, size_t size, int prot)
 {
-	struct shmobile_iommu_priv_pgtable l2 = { .pgtable = NULL };
-	struct shmobile_iommu_priv *priv = domain->priv;
+	struct shmobile_iommu_domain_pgtable l2 = { .pgtable = NULL };
+	struct shmobile_iommu_domain *sh_domain = domain->priv;
 	unsigned int l1index, l2index, i;
 	int ret;
 
@@ -179,34 +179,34 @@ static int shmobile_iommu_map(struct iommu_domain *domain, unsigned long iova,
 	switch (size) {
 	case 0x1000:
 		l2index = (iova >> 12) & 0xff;
-		spin_lock(&priv->map_lock);
-		ret = l2alloc(priv, l1index);
+		spin_lock(&sh_domain->map_lock);
+		ret = l2alloc(sh_domain, l1index);
 		if (!ret)
-			priv->l2[l1index].pgtable[l2index] = paddr | 0xff2;
-		spin_unlock(&priv->map_lock);
+			sh_domain->l2[l1index].pgtable[l2index] = paddr | 0xff2;
+		spin_unlock(&sh_domain->map_lock);
 		break;
 	case 0x10000:
 		l2index = (iova >> 12) & 0xf0;
-		spin_lock(&priv->map_lock);
-		ret = l2alloc(priv, l1index);
+		spin_lock(&sh_domain->map_lock);
+		ret = l2alloc(sh_domain, l1index);
 		if (!ret) {
 			for (i = 0; i < 0x10; i++)
-				priv->l2[l1index].pgtable[l2index + i] =
+				sh_domain->l2[l1index].pgtable[l2index + i] =
 					paddr | 0xff1;
 		}
-		spin_unlock(&priv->map_lock);
+		spin_unlock(&sh_domain->map_lock);
 		break;
 	case 0x100000:
-		spin_lock(&priv->map_lock);
-		l2free(priv, l1index, &l2);
-		priv->l1.pgtable[l1index] = paddr | 0xc02;
-		spin_unlock(&priv->map_lock);
+		spin_lock(&sh_domain->map_lock);
+		l2free(sh_domain, l1index, &l2);
+		sh_domain->l1.pgtable[l1index] = paddr | 0xc02;
+		spin_unlock(&sh_domain->map_lock);
 		ret = 0;
 		break;
 	default:
 		ret = -EINVAL;
 	}
-	if (!ret && atomic_read(&priv->active)) {
+	if (!ret && atomic_read(&sh_domain->active)) {
 		wmb();
 		ipmmu_tlb_flush(ipmmu_access_device);
 		l2realfree(&l2);
@@ -217,40 +217,40 @@ static int shmobile_iommu_map(struct iommu_domain *domain, unsigned long iova,
 static size_t shmobile_iommu_unmap(struct iommu_domain *domain,
 				   unsigned long iova, size_t size)
 {
-	struct shmobile_iommu_priv_pgtable l2 = { .pgtable = NULL };
-	struct shmobile_iommu_priv *priv = domain->priv;
+	struct shmobile_iommu_domain_pgtable l2 = { .pgtable = NULL };
+	struct shmobile_iommu_domain *sh_domain = domain->priv;
 	unsigned int l1index, l2index, i;
 	uint32_t l2entry = 0;
 	size_t ret = 0;
 
 	l1index = iova >> 20;
 	if (!(iova & 0xFFFFF) && size >= 0x100000) {
-		spin_lock(&priv->map_lock);
-		l2free(priv, l1index, &l2);
-		spin_unlock(&priv->map_lock);
+		spin_lock(&sh_domain->map_lock);
+		l2free(sh_domain, l1index, &l2);
+		spin_unlock(&sh_domain->map_lock);
 		ret = 0x100000;
 		goto done;
 	}
 	l2index = (iova >> 12) & 0xff;
-	spin_lock(&priv->map_lock);
-	if (priv->l2[l1index].pgtable)
-		l2entry = priv->l2[l1index].pgtable[l2index];
+	spin_lock(&sh_domain->map_lock);
+	if (sh_domain->l2[l1index].pgtable)
+		l2entry = sh_domain->l2[l1index].pgtable[l2index];
 	switch (l2entry & 3) {
 	case 1:
 		if (l2index & 0xf)
 			break;
 		for (i = 0; i < 0x10; i++)
-			priv->l2[l1index].pgtable[l2index + i] = 0;
+			sh_domain->l2[l1index].pgtable[l2index + i] = 0;
 		ret = 0x10000;
 		break;
 	case 2:
-		priv->l2[l1index].pgtable[l2index] = 0;
+		sh_domain->l2[l1index].pgtable[l2index] = 0;
 		ret = 0x1000;
 		break;
 	}
-	spin_unlock(&priv->map_lock);
+	spin_unlock(&sh_domain->map_lock);
 done:
-	if (ret && atomic_read(&priv->active)) {
+	if (ret && atomic_read(&sh_domain->active)) {
 		wmb();
 		ipmmu_tlb_flush(ipmmu_access_device);
 		l2realfree(&l2);
@@ -261,18 +261,18 @@ done:
 static phys_addr_t shmobile_iommu_iova_to_phys(struct iommu_domain *domain,
 					       unsigned long iova)
 {
-	struct shmobile_iommu_priv *priv = domain->priv;
+	struct shmobile_iommu_domain *sh_domain = domain->priv;
 	uint32_t l1entry = 0, l2entry = 0;
 	unsigned int l1index, l2index;
 
 	l1index = iova >> 20;
 	l2index = (iova >> 12) & 0xff;
-	spin_lock(&priv->map_lock);
-	if (priv->l2[l1index].pgtable)
-		l2entry = priv->l2[l1index].pgtable[l2index];
+	spin_lock(&sh_domain->map_lock);
+	if (sh_domain->l2[l1index].pgtable)
+		l2entry = sh_domain->l2[l1index].pgtable[l2index];
 	else
-		l1entry = priv->l1.pgtable[l1index];
-	spin_unlock(&priv->map_lock);
+		l1entry = sh_domain->l1.pgtable[l1index];
+	spin_unlock(&sh_domain->map_lock);
 	switch (l2entry & 3) {
 	case 1:
 		return (l2entry & ~0xffff) | (iova & 0xffff);
-- 
1.7.8.6


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

* [PATCH/WIP/RFC 09/14] shmobile-ipmmu: Rename ipmmu_priv to shmobile_ipmmu
  2012-12-16 17:25 ` [PATCH/WIP/RFC 00/14] Renesas IPMMU driver work in progress Laurent Pinchart
                     ` (7 preceding siblings ...)
  2012-12-16 17:25   ` [PATCH/WIP/RFC 08/14] shmobile-iommu: Rename shmobile_iommu_priv to shmobile_iommu_domain Laurent Pinchart
@ 2012-12-16 17:25   ` Laurent Pinchart
  2012-12-16 17:25   ` [PATCH/WIP/RFC 10/14] shmobile-ipmmu: Pass a struct shmobile_ipmmu to IPMMU functions Laurent Pinchart
                     ` (4 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2012-12-16 17:25 UTC (permalink / raw)
  To: Hideki EIRAKU
  Cc: Paul Mundt, Magnus Damm, Simon Horman, linux-sh,
	linux-arm-kernel, linux-kernel, Marek Szyprowski,
	Katsuya MATSUBARA, Damian Hobson-Garcia

And expose the structure to the IOMMU driver.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/shmobile-ipmmu.c |   14 ++++----------
 drivers/iommu/shmobile-ipmmu.h |    6 ++++++
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/iommu/shmobile-ipmmu.c b/drivers/iommu/shmobile-ipmmu.c
index 34e3c66..b308292 100644
--- a/drivers/iommu/shmobile-ipmmu.c
+++ b/drivers/iommu/shmobile-ipmmu.c
@@ -34,13 +34,7 @@
 #define IMCTR1_TLBEN (1 << 0)
 #define IMCTR1_FLUSH (1 << 1)
 
-struct ipmmu_priv {
-	void __iomem *ipmmu_base;
-	int tlb_enabled;
-	struct mutex flush_lock;
-};
-
-static void ipmmu_reg_write(struct ipmmu_priv *priv, unsigned long reg_off,
+static void ipmmu_reg_write(struct shmobile_ipmmu *priv, unsigned long reg_off,
 			    unsigned long data)
 {
 	iowrite32(data, priv->ipmmu_base + reg_off);
@@ -48,7 +42,7 @@ static void ipmmu_reg_write(struct ipmmu_priv *priv, unsigned long reg_off,
 
 void ipmmu_tlb_flush(struct device *dev)
 {
-	struct ipmmu_priv *priv;
+	struct shmobile_ipmmu *priv;
 
 	if (!dev)
 		return;
@@ -63,7 +57,7 @@ void ipmmu_tlb_flush(struct device *dev)
 
 void ipmmu_tlb_set(struct device *dev, unsigned long phys, int size, int asid)
 {
-	struct ipmmu_priv *priv;
+	struct shmobile_ipmmu *priv;
 
 	if (!dev)
 		return;
@@ -110,7 +104,7 @@ void ipmmu_tlb_set(struct device *dev, unsigned long phys, int size, int asid)
 static int ipmmu_probe(struct platform_device *pdev)
 {
 	struct resource *res;
-	struct ipmmu_priv *priv;
+	struct shmobile_ipmmu *priv;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
diff --git a/drivers/iommu/shmobile-ipmmu.h b/drivers/iommu/shmobile-ipmmu.h
index 0e0a6a4..5c17a46 100644
--- a/drivers/iommu/shmobile-ipmmu.h
+++ b/drivers/iommu/shmobile-ipmmu.h
@@ -1,6 +1,12 @@
 #ifndef __SHMOBILE_IPMMU_H__
 #define __SHMOBILE_IPMMU_H__
 
+struct shmobile_ipmmu {
+	void __iomem *ipmmu_base;
+	int tlb_enabled;
+	struct mutex flush_lock;
+};
+
 #ifdef CONFIG_SHMOBILE_IPMMU_TLB
 void ipmmu_tlb_flush(struct device *ipmmu_dev);
 void ipmmu_tlb_set(struct device *ipmmu_dev, unsigned long phys, int size,
-- 
1.7.8.6


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

* [PATCH/WIP/RFC 10/14] shmobile-ipmmu: Pass a struct shmobile_ipmmu to IPMMU functions
  2012-12-16 17:25 ` [PATCH/WIP/RFC 00/14] Renesas IPMMU driver work in progress Laurent Pinchart
                     ` (8 preceding siblings ...)
  2012-12-16 17:25   ` [PATCH/WIP/RFC 09/14] shmobile-ipmmu: Rename ipmmu_priv to shmobile_ipmmu Laurent Pinchart
@ 2012-12-16 17:25   ` Laurent Pinchart
  2012-12-16 17:25   ` [PATCH/WIP/RFC 11/14] shmobile-ipmmu: Store a struct shmobile_iommu_arch_data in archdata.iommu Laurent Pinchart
                     ` (3 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2012-12-16 17:25 UTC (permalink / raw)
  To: Hideki EIRAKU
  Cc: Paul Mundt, Magnus Damm, Simon Horman, linux-sh,
	linux-arm-kernel, linux-kernel, Marek Szyprowski,
	Katsuya MATSUBARA, Damian Hobson-Garcia

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/shmobile-iommu.c |   12 +++---
 drivers/iommu/shmobile-ipmmu.c |   90 +++++++++++++++++++--------------------
 drivers/iommu/shmobile-ipmmu.h |    9 ++--
 3 files changed, 55 insertions(+), 56 deletions(-)

diff --git a/drivers/iommu/shmobile-iommu.c b/drivers/iommu/shmobile-iommu.c
index 1a37be2..423993c 100644
--- a/drivers/iommu/shmobile-iommu.c
+++ b/drivers/iommu/shmobile-iommu.c
@@ -53,7 +53,7 @@ static spinlock_t lock;
 static DEFINE_SPINLOCK(lock_add);
 static struct shmobile_iommu_domain *attached;
 static int num_attached_devices;
-static struct device *ipmmu_access_device;
+static struct shmobile_ipmmu *ipmmu_access_device;
 
 static int shmobile_iommu_domain_init(struct iommu_domain *domain)
 {
@@ -329,20 +329,20 @@ void ipmmu_add_device(struct device *dev)
 	spin_unlock(&lock_add);
 }
 
-int ipmmu_iommu_init(struct device *dev)
+int ipmmu_iommu_init(struct shmobile_ipmmu *ipmmu)
 {
-	dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
-	l1pool = dma_pool_create("shmobile-iommu-pgtable1", dev,
+	dma_set_coherent_mask(ipmmu->dev, DMA_BIT_MASK(32));
+	l1pool = dma_pool_create("shmobile-iommu-pgtable1", ipmmu->dev,
 				 L1_SIZE, L1_ALIGN, 0);
 	if (!l1pool)
 		goto nomem_pool1;
-	l2pool = dma_pool_create("shmobile-iommu-pgtable2", dev,
+	l2pool = dma_pool_create("shmobile-iommu-pgtable2", ipmmu->dev,
 				 L2_SIZE, L2_ALIGN, 0);
 	if (!l2pool)
 		goto nomem_pool2;
 	spin_lock_init(&lock);
 	attached = NULL;
-	ipmmu_access_device = dev;
+	ipmmu_access_device = ipmmu;
 	bus_set_iommu(&platform_bus_type, &shmobile_iommu_ops);
 	if (shmobile_iommu_attach_all_devices())
 		pr_err("shmobile_iommu_attach_all_devices failed\n");
diff --git a/drivers/iommu/shmobile-ipmmu.c b/drivers/iommu/shmobile-ipmmu.c
index b308292..9858c91 100644
--- a/drivers/iommu/shmobile-ipmmu.c
+++ b/drivers/iommu/shmobile-ipmmu.c
@@ -34,99 +34,97 @@
 #define IMCTR1_TLBEN (1 << 0)
 #define IMCTR1_FLUSH (1 << 1)
 
-static void ipmmu_reg_write(struct shmobile_ipmmu *priv, unsigned long reg_off,
+static void ipmmu_reg_write(struct shmobile_ipmmu *ipmmu, unsigned long reg_off,
 			    unsigned long data)
 {
-	iowrite32(data, priv->ipmmu_base + reg_off);
+	iowrite32(data, ipmmu->ipmmu_base + reg_off);
 }
 
-void ipmmu_tlb_flush(struct device *dev)
+void ipmmu_tlb_flush(struct shmobile_ipmmu *ipmmu)
 {
-	struct shmobile_ipmmu *priv;
-
-	if (!dev)
+	if (!ipmmu)
 		return;
-	priv = dev_get_drvdata(dev);
-	mutex_lock(&priv->flush_lock);
-	if (priv->tlb_enabled)
-		ipmmu_reg_write(priv, IMCTR1, IMCTR1_FLUSH | IMCTR1_TLBEN);
+
+	mutex_lock(&ipmmu->flush_lock);
+	if (ipmmu->tlb_enabled)
+		ipmmu_reg_write(ipmmu, IMCTR1, IMCTR1_FLUSH | IMCTR1_TLBEN);
 	else
-		ipmmu_reg_write(priv, IMCTR1, IMCTR1_FLUSH);
-	mutex_unlock(&priv->flush_lock);
+		ipmmu_reg_write(ipmmu, IMCTR1, IMCTR1_FLUSH);
+	mutex_unlock(&ipmmu->flush_lock);
 }
 
-void ipmmu_tlb_set(struct device *dev, unsigned long phys, int size, int asid)
+void ipmmu_tlb_set(struct shmobile_ipmmu *ipmmu, unsigned long phys, int size,
+		   int asid)
 {
-	struct shmobile_ipmmu *priv;
-
-	if (!dev)
+	if (!ipmmu)
 		return;
-	priv = dev_get_drvdata(dev);
-	mutex_lock(&priv->flush_lock);
+
+	mutex_lock(&ipmmu->flush_lock);
 	switch (size) {
 	default:
-		priv->tlb_enabled = 0;
+		ipmmu->tlb_enabled = 0;
 		break;
 	case 0x2000:
-		ipmmu_reg_write(priv, IMTTBCR, 1);
-		priv->tlb_enabled = 1;
+		ipmmu_reg_write(ipmmu, IMTTBCR, 1);
+		ipmmu->tlb_enabled = 1;
 		break;
 	case 0x1000:
-		ipmmu_reg_write(priv, IMTTBCR, 2);
-		priv->tlb_enabled = 1;
+		ipmmu_reg_write(ipmmu, IMTTBCR, 2);
+		ipmmu->tlb_enabled = 1;
 		break;
 	case 0x800:
-		ipmmu_reg_write(priv, IMTTBCR, 3);
-		priv->tlb_enabled = 1;
+		ipmmu_reg_write(ipmmu, IMTTBCR, 3);
+		ipmmu->tlb_enabled = 1;
 		break;
 	case 0x400:
-		ipmmu_reg_write(priv, IMTTBCR, 4);
-		priv->tlb_enabled = 1;
+		ipmmu_reg_write(ipmmu, IMTTBCR, 4);
+		ipmmu->tlb_enabled = 1;
 		break;
 	case 0x200:
-		ipmmu_reg_write(priv, IMTTBCR, 5);
-		priv->tlb_enabled = 1;
+		ipmmu_reg_write(ipmmu, IMTTBCR, 5);
+		ipmmu->tlb_enabled = 1;
 		break;
 	case 0x100:
-		ipmmu_reg_write(priv, IMTTBCR, 6);
-		priv->tlb_enabled = 1;
+		ipmmu_reg_write(ipmmu, IMTTBCR, 6);
+		ipmmu->tlb_enabled = 1;
 		break;
 	case 0x80:
-		ipmmu_reg_write(priv, IMTTBCR, 7);
-		priv->tlb_enabled = 1;
+		ipmmu_reg_write(ipmmu, IMTTBCR, 7);
+		ipmmu->tlb_enabled = 1;
 		break;
 	}
-	ipmmu_reg_write(priv, IMTTBR, phys);
-	ipmmu_reg_write(priv, IMASID, asid);
-	mutex_unlock(&priv->flush_lock);
+	ipmmu_reg_write(ipmmu, IMTTBR, phys);
+	ipmmu_reg_write(ipmmu, IMASID, asid);
+	mutex_unlock(&ipmmu->flush_lock);
 }
 
 static int ipmmu_probe(struct platform_device *pdev)
 {
+	struct shmobile_ipmmu *ipmmu;
 	struct resource *res;
-	struct shmobile_ipmmu *priv;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		dev_err(&pdev->dev, "cannot get platform resources\n");
 		return -ENOENT;
 	}
-	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv) {
+	ipmmu = devm_kzalloc(&pdev->dev, sizeof(*ipmmu), GFP_KERNEL);
+	if (!ipmmu) {
 		dev_err(&pdev->dev, "cannot allocate device data\n");
 		return -ENOMEM;
 	}
-	mutex_init(&priv->flush_lock);
-	priv->ipmmu_base = devm_ioremap_nocache(&pdev->dev, res->start,
+	mutex_init(&ipmmu->flush_lock);
+	ipmmu->dev = &pdev->dev;
+	ipmmu->ipmmu_base = devm_ioremap_nocache(&pdev->dev, res->start,
 						resource_size(res));
-	if (!priv->ipmmu_base) {
+	if (!ipmmu->ipmmu_base) {
 		dev_err(&pdev->dev, "ioremap_nocache failed\n");
 		return -ENOMEM;
 	}
-	platform_set_drvdata(pdev, priv);
-	ipmmu_reg_write(priv, IMCTR1, 0x0); /* disable TLB */
-	ipmmu_reg_write(priv, IMCTR2, 0x0); /* disable PMB */
-	ipmmu_iommu_init(&pdev->dev);
+	platform_set_drvdata(pdev, ipmmu);
+	ipmmu_reg_write(ipmmu, IMCTR1, 0x0); /* disable TLB */
+	ipmmu_reg_write(ipmmu, IMCTR2, 0x0); /* disable PMB */
+	ipmmu_iommu_init(ipmmu);
 	return 0;
 }
 
diff --git a/drivers/iommu/shmobile-ipmmu.h b/drivers/iommu/shmobile-ipmmu.h
index 5c17a46..1458a97 100644
--- a/drivers/iommu/shmobile-ipmmu.h
+++ b/drivers/iommu/shmobile-ipmmu.h
@@ -2,18 +2,19 @@
 #define __SHMOBILE_IPMMU_H__
 
 struct shmobile_ipmmu {
+	struct device *dev;
 	void __iomem *ipmmu_base;
 	int tlb_enabled;
 	struct mutex flush_lock;
 };
 
 #ifdef CONFIG_SHMOBILE_IPMMU_TLB
-void ipmmu_tlb_flush(struct device *ipmmu_dev);
-void ipmmu_tlb_set(struct device *ipmmu_dev, unsigned long phys, int size,
+void ipmmu_tlb_flush(struct shmobile_ipmmu *ipmmu);
+void ipmmu_tlb_set(struct shmobile_ipmmu *ipmmu, unsigned long phys, int size,
 		   int asid);
-int ipmmu_iommu_init(struct device *dev);
+int ipmmu_iommu_init(struct shmobile_ipmmu *ipmmu);
 #else
-static int ipmmu_iommu_init(struct device *dev)
+static int ipmmu_iommu_init(struct shmobile_ipmmu *ipmmu)
 {
 	return -EINVAL;
 }
-- 
1.7.8.6


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

* [PATCH/WIP/RFC 11/14] shmobile-ipmmu: Store a struct shmobile_iommu_arch_data in archdata.iommu
  2012-12-16 17:25 ` [PATCH/WIP/RFC 00/14] Renesas IPMMU driver work in progress Laurent Pinchart
                     ` (9 preceding siblings ...)
  2012-12-16 17:25   ` [PATCH/WIP/RFC 10/14] shmobile-ipmmu: Pass a struct shmobile_ipmmu to IPMMU functions Laurent Pinchart
@ 2012-12-16 17:25   ` Laurent Pinchart
  2012-12-16 17:25   ` [PATCH/WIP/RFC 12/14] shmobile-ipmmu: Store ipmmu pointer in iommu arch data and iommu domain Laurent Pinchart
                     ` (2 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2012-12-16 17:25 UTC (permalink / raw)
  To: Hideki EIRAKU
  Cc: Paul Mundt, Magnus Damm, Simon Horman, linux-sh,
	linux-arm-kernel, linux-kernel, Marek Szyprowski,
	Katsuya MATSUBARA, Damian Hobson-Garcia

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/shmobile-iommu.c |   20 +++++++++++++++++---
 include/linux/sh_iommu.h       |   10 ++++++++--
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/shmobile-iommu.c b/drivers/iommu/shmobile-iommu.c
index 423993c..f27d842 100644
--- a/drivers/iommu/shmobile-iommu.c
+++ b/drivers/iommu/shmobile-iommu.c
@@ -308,25 +308,39 @@ static int shmobile_iommu_attach_all_devices(void)
 		ret = PTR_ERR(iommu_mapping);
 		goto err;
 	}
-	for (dev = ipmmu_devices; dev; dev = dev->archdata.iommu) {
+	for (dev = ipmmu_devices; dev; ) {
+		struct shmobile_iommu_arch_data *data = dev->archdata.iommu;
+
 		if (arm_iommu_attach_device(dev, iommu_mapping))
 			pr_err("arm_iommu_attach_device failed\n");
+
+		dev = data->next;
 	}
 err:
 	spin_unlock(&lock_add);
 	return 0;
 }
 
-void ipmmu_add_device(struct device *dev)
+int ipmmu_add_device(struct device *dev)
 {
+	struct shmobile_iommu_arch_data *data;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (data == NULL)
+		return -ENOMEM;
+
+	dev->archdata.iommu = data;
+
 	spin_lock(&lock_add);
-	dev->archdata.iommu = ipmmu_devices;
+	data->next = ipmmu_devices;
 	ipmmu_devices = dev;
 	if (!IS_ERR_OR_NULL(iommu_mapping)) {
 		if (arm_iommu_attach_device(dev, iommu_mapping))
 			pr_err("arm_iommu_attach_device failed\n");
 	}
 	spin_unlock(&lock_add);
+
+	return 0;
 }
 
 int ipmmu_iommu_init(struct shmobile_ipmmu *ipmmu)
diff --git a/include/linux/sh_iommu.h b/include/linux/sh_iommu.h
index cc669a0..8b7b51d 100644
--- a/include/linux/sh_iommu.h
+++ b/include/linux/sh_iommu.h
@@ -1,10 +1,16 @@
 #ifndef __SH_IOMMU_H__
 #define __SH_IOMMU_H__
 
+struct device;
+
+struct shmobile_iommu_arch_data {
+	struct device *next;
+};
+
 #ifdef CONFIG_SHMOBILE_IPMMU_TLB
-void ipmmu_add_device(struct device *dev);
+int ipmmu_add_device(struct device *dev);
 #else
-static inline void ipmmu_add_device(struct device *dev)
+static inline int ipmmu_add_device(struct device *dev)
 {
 }
 #endif
-- 
1.7.8.6


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

* [PATCH/WIP/RFC 12/14] shmobile-ipmmu: Store ipmmu pointer in iommu arch data and iommu domain
  2012-12-16 17:25 ` [PATCH/WIP/RFC 00/14] Renesas IPMMU driver work in progress Laurent Pinchart
                     ` (10 preceding siblings ...)
  2012-12-16 17:25   ` [PATCH/WIP/RFC 11/14] shmobile-ipmmu: Store a struct shmobile_iommu_arch_data in archdata.iommu Laurent Pinchart
@ 2012-12-16 17:25   ` Laurent Pinchart
  2012-12-16 17:25   ` [PATCH/WIP/RFC 13/14] shmobile-ipmmu: Remove unneeded lock_add spinlock Laurent Pinchart
  2012-12-16 17:26   ` [PATCH/WIP/RFC 14/14] shmobile-ipmmu: Store iommu_mapping in struct shmobile_ipmmu Laurent Pinchart
  13 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2012-12-16 17:25 UTC (permalink / raw)
  To: Hideki EIRAKU
  Cc: Paul Mundt, Magnus Damm, Simon Horman, linux-sh,
	linux-arm-kernel, linux-kernel, Marek Szyprowski,
	Katsuya MATSUBARA, Damian Hobson-Garcia

This gets rid of the global ipmmu_access_device pointer.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/shmobile-iommu.c |   37 ++++++++++++++++++++++++++-----------
 include/linux/sh_iommu.h       |    2 ++
 2 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/iommu/shmobile-iommu.c b/drivers/iommu/shmobile-iommu.c
index f27d842..bf1d0a4 100644
--- a/drivers/iommu/shmobile-iommu.c
+++ b/drivers/iommu/shmobile-iommu.c
@@ -41,6 +41,7 @@ struct shmobile_iommu_domain_pgtable {
 };
 
 struct shmobile_iommu_domain {
+	struct shmobile_ipmmu *ipmmu;
 	struct shmobile_iommu_domain_pgtable l1, l2[L1_LEN];
 	spinlock_t map_lock;
 	atomic_t active;
@@ -53,7 +54,17 @@ static spinlock_t lock;
 static DEFINE_SPINLOCK(lock_add);
 static struct shmobile_iommu_domain *attached;
 static int num_attached_devices;
-static struct shmobile_ipmmu *ipmmu_access_device;
+
+/**
+ * dev_to_ipmmu() - retrieves an shmobile ipmmu object from a user device
+ * @dev: ipmmu client device
+ */
+static inline struct shmobile_ipmmu *dev_to_ipmmu(struct device *dev)
+{
+	struct shmobile_iommu_arch_data *arch_data = dev->archdata.iommu;
+
+	return arch_data->ipmmu;
+}
 
 static int shmobile_iommu_domain_init(struct iommu_domain *domain)
 {
@@ -97,6 +108,7 @@ static int shmobile_iommu_attach_device(struct iommu_domain *domain,
 					struct device *dev)
 {
 	struct shmobile_iommu_domain *sh_domain = domain->priv;
+	struct shmobile_ipmmu *ipmmu = dev_to_ipmmu(dev);
 	int ret = -EBUSY;
 
 	spin_lock(&lock);
@@ -104,12 +116,12 @@ static int shmobile_iommu_attach_device(struct iommu_domain *domain,
 		if (attached)
 			goto err;
 		atomic_set(&sh_domain->active, 1);
-		ipmmu_tlb_set(ipmmu_access_device, sh_domain->l1.handle, L1_SIZE,
-			      0);
+		ipmmu_tlb_set(ipmmu, sh_domain->l1.handle, L1_SIZE, 0);
 		wmb();
-		ipmmu_tlb_flush(ipmmu_access_device);
+		ipmmu_tlb_flush(ipmmu);
 		attached = sh_domain;
 		num_attached_devices = 0;
+		sh_domain->ipmmu = ipmmu;
 	}
 	num_attached_devices++;
 	ret = 0;
@@ -122,14 +134,16 @@ static void shmobile_iommu_detach_device(struct iommu_domain *domain,
 					 struct device *dev)
 {
 	struct shmobile_iommu_domain *sh_domain = domain->priv;
+	struct shmobile_ipmmu *ipmmu = dev_to_ipmmu(dev);
 
 	spin_lock(&lock);
 	atomic_set(&sh_domain->active, 0);
 	num_attached_devices--;
 	if (!num_attached_devices) {
-		ipmmu_tlb_set(ipmmu_access_device, 0, 0, 0);
-		ipmmu_tlb_flush(ipmmu_access_device);
+		ipmmu_tlb_set(ipmmu, 0, 0, 0);
+		ipmmu_tlb_flush(ipmmu);
 		attached = NULL;
+		sh_domain->ipmmu = NULL;
 	}
 	spin_unlock(&lock);
 }
@@ -208,7 +222,7 @@ static int shmobile_iommu_map(struct iommu_domain *domain, unsigned long iova,
 	}
 	if (!ret && atomic_read(&sh_domain->active)) {
 		wmb();
-		ipmmu_tlb_flush(ipmmu_access_device);
+		ipmmu_tlb_flush(sh_domain->ipmmu);
 		l2realfree(&l2);
 	}
 	return ret;
@@ -252,7 +266,7 @@ static size_t shmobile_iommu_unmap(struct iommu_domain *domain,
 done:
 	if (ret && atomic_read(&sh_domain->active)) {
 		wmb();
-		ipmmu_tlb_flush(ipmmu_access_device);
+		ipmmu_tlb_flush(sh_domain->ipmmu);
 		l2realfree(&l2);
 	}
 	return ret;
@@ -296,7 +310,7 @@ static struct iommu_ops shmobile_iommu_ops = {
 	.pgsize_bitmap = 0x111000,
 };
 
-static int shmobile_iommu_attach_all_devices(void)
+static int shmobile_iommu_attach_all_devices(struct shmobile_ipmmu *ipmmu)
 {
 	struct device *dev;
 	int ret = 0;
@@ -311,6 +325,8 @@ static int shmobile_iommu_attach_all_devices(void)
 	for (dev = ipmmu_devices; dev; ) {
 		struct shmobile_iommu_arch_data *data = dev->archdata.iommu;
 
+		data->ipmmu = ipmmu;
+
 		if (arm_iommu_attach_device(dev, iommu_mapping))
 			pr_err("arm_iommu_attach_device failed\n");
 
@@ -356,9 +372,8 @@ int ipmmu_iommu_init(struct shmobile_ipmmu *ipmmu)
 		goto nomem_pool2;
 	spin_lock_init(&lock);
 	attached = NULL;
-	ipmmu_access_device = ipmmu;
 	bus_set_iommu(&platform_bus_type, &shmobile_iommu_ops);
-	if (shmobile_iommu_attach_all_devices())
+	if (shmobile_iommu_attach_all_devices(ipmmu))
 		pr_err("shmobile_iommu_attach_all_devices failed\n");
 	return 0;
 nomem_pool2:
diff --git a/include/linux/sh_iommu.h b/include/linux/sh_iommu.h
index 8b7b51d..76b6e51 100644
--- a/include/linux/sh_iommu.h
+++ b/include/linux/sh_iommu.h
@@ -2,8 +2,10 @@
 #define __SH_IOMMU_H__
 
 struct device;
+struct shmobile_ipmmu;
 
 struct shmobile_iommu_arch_data {
+	struct shmobile_ipmmu *ipmmu;
 	struct device *next;
 };
 
-- 
1.7.8.6


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

* [PATCH/WIP/RFC 13/14] shmobile-ipmmu: Remove unneeded lock_add spinlock
  2012-12-16 17:25 ` [PATCH/WIP/RFC 00/14] Renesas IPMMU driver work in progress Laurent Pinchart
                     ` (11 preceding siblings ...)
  2012-12-16 17:25   ` [PATCH/WIP/RFC 12/14] shmobile-ipmmu: Store ipmmu pointer in iommu arch data and iommu domain Laurent Pinchart
@ 2012-12-16 17:25   ` Laurent Pinchart
  2012-12-16 17:26   ` [PATCH/WIP/RFC 14/14] shmobile-ipmmu: Store iommu_mapping in struct shmobile_ipmmu Laurent Pinchart
  13 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2012-12-16 17:25 UTC (permalink / raw)
  To: Hideki EIRAKU
  Cc: Paul Mundt, Magnus Damm, Simon Horman, linux-sh,
	linux-arm-kernel, linux-kernel, Marek Szyprowski,
	Katsuya MATSUBARA, Damian Hobson-Garcia

ipmmu_add_device() can never race with
shmobile_iommu_attach_all_devices(), called by ipmmu_iommu_init() as the
former is called from an arch initcall and the later from a subsys
initcall.

Remove the unneeded spinlock as well as the arm_iommu_attach_device() in
ipmmu_add_device(), as the condition that guards the call is always
false.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/shmobile-iommu.c |   18 +++---------------
 1 files changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/shmobile-iommu.c b/drivers/iommu/shmobile-iommu.c
index bf1d0a4..2592d25 100644
--- a/drivers/iommu/shmobile-iommu.c
+++ b/drivers/iommu/shmobile-iommu.c
@@ -51,7 +51,6 @@ static struct dma_iommu_mapping *iommu_mapping;
 static struct device *ipmmu_devices;
 static struct dma_pool *l1pool, *l2pool;
 static spinlock_t lock;
-static DEFINE_SPINLOCK(lock_add);
 static struct shmobile_iommu_domain *attached;
 static int num_attached_devices;
 
@@ -313,15 +312,12 @@ static struct iommu_ops shmobile_iommu_ops = {
 static int shmobile_iommu_attach_all_devices(struct shmobile_ipmmu *ipmmu)
 {
 	struct device *dev;
-	int ret = 0;
 
-	spin_lock(&lock_add);
 	iommu_mapping = arm_iommu_create_mapping(&platform_bus_type, 0x0,
 						 L1_LEN << 20, 0);
-	if (IS_ERR_OR_NULL(iommu_mapping)) {
-		ret = PTR_ERR(iommu_mapping);
-		goto err;
-	}
+	if (IS_ERR_OR_NULL(iommu_mapping))
+		return PTR_ERR(iommu_mapping);
+
 	for (dev = ipmmu_devices; dev; ) {
 		struct shmobile_iommu_arch_data *data = dev->archdata.iommu;
 
@@ -332,8 +328,6 @@ static int shmobile_iommu_attach_all_devices(struct shmobile_ipmmu *ipmmu)
 
 		dev = data->next;
 	}
-err:
-	spin_unlock(&lock_add);
 	return 0;
 }
 
@@ -347,14 +341,8 @@ int ipmmu_add_device(struct device *dev)
 
 	dev->archdata.iommu = data;
 
-	spin_lock(&lock_add);
 	data->next = ipmmu_devices;
 	ipmmu_devices = dev;
-	if (!IS_ERR_OR_NULL(iommu_mapping)) {
-		if (arm_iommu_attach_device(dev, iommu_mapping))
-			pr_err("arm_iommu_attach_device failed\n");
-	}
-	spin_unlock(&lock_add);
 
 	return 0;
 }
-- 
1.7.8.6


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

* [PATCH/WIP/RFC 14/14] shmobile-ipmmu: Store iommu_mapping in struct shmobile_ipmmu
  2012-12-16 17:25 ` [PATCH/WIP/RFC 00/14] Renesas IPMMU driver work in progress Laurent Pinchart
                     ` (12 preceding siblings ...)
  2012-12-16 17:25   ` [PATCH/WIP/RFC 13/14] shmobile-ipmmu: Remove unneeded lock_add spinlock Laurent Pinchart
@ 2012-12-16 17:26   ` Laurent Pinchart
  13 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2012-12-16 17:26 UTC (permalink / raw)
  To: Hideki EIRAKU
  Cc: Paul Mundt, Magnus Damm, Simon Horman, linux-sh,
	linux-arm-kernel, linux-kernel, Marek Szyprowski,
	Katsuya MATSUBARA, Damian Hobson-Garcia

And remove the global iommu_mapping variable.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/shmobile-iommu.c |   14 ++++++++------
 drivers/iommu/shmobile-ipmmu.h |    4 ++++
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/shmobile-iommu.c b/drivers/iommu/shmobile-iommu.c
index 2592d25..8cf45df 100644
--- a/drivers/iommu/shmobile-iommu.c
+++ b/drivers/iommu/shmobile-iommu.c
@@ -47,7 +47,6 @@ struct shmobile_iommu_domain {
 	atomic_t active;
 };
 
-static struct dma_iommu_mapping *iommu_mapping;
 static struct device *ipmmu_devices;
 static struct dma_pool *l1pool, *l2pool;
 static spinlock_t lock;
@@ -311,19 +310,22 @@ static struct iommu_ops shmobile_iommu_ops = {
 
 static int shmobile_iommu_attach_all_devices(struct shmobile_ipmmu *ipmmu)
 {
+	struct dma_iommu_mapping *mapping;
 	struct device *dev;
 
-	iommu_mapping = arm_iommu_create_mapping(&platform_bus_type, 0x0,
-						 L1_LEN << 20, 0);
-	if (IS_ERR_OR_NULL(iommu_mapping))
-		return PTR_ERR(iommu_mapping);
+	mapping = arm_iommu_create_mapping(&platform_bus_type, 0,
+					   L1_LEN << 20, 0);
+	if (IS_ERR(mapping))
+		return PTR_ERR(mapping);
+
+	ipmmu->iommu_mapping = mapping;
 
 	for (dev = ipmmu_devices; dev; ) {
 		struct shmobile_iommu_arch_data *data = dev->archdata.iommu;
 
 		data->ipmmu = ipmmu;
 
-		if (arm_iommu_attach_device(dev, iommu_mapping))
+		if (arm_iommu_attach_device(dev, mapping))
 			pr_err("arm_iommu_attach_device failed\n");
 
 		dev = data->next;
diff --git a/drivers/iommu/shmobile-ipmmu.h b/drivers/iommu/shmobile-ipmmu.h
index 1458a97..f8f0f57 100644
--- a/drivers/iommu/shmobile-ipmmu.h
+++ b/drivers/iommu/shmobile-ipmmu.h
@@ -1,11 +1,15 @@
 #ifndef __SHMOBILE_IPMMU_H__
 #define __SHMOBILE_IPMMU_H__
 
+struct dma_iommu_mapping;
+
 struct shmobile_ipmmu {
 	struct device *dev;
 	void __iomem *ipmmu_base;
 	int tlb_enabled;
 	struct mutex flush_lock;
+
+	struct dma_iommu_mapping *iommu_mapping;
 };
 
 #ifdef CONFIG_SHMOBILE_IPMMU_TLB
-- 
1.7.8.6


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

* Re: [PATCH/WIP/RFC 02/14] shmobile-iommu: Move IPMMU driver to drivers/iommu
  2012-12-16 17:25   ` [PATCH/WIP/RFC 02/14] shmobile-iommu: Move IPMMU driver to drivers/iommu Laurent Pinchart
@ 2012-12-17  3:10     ` Damian Hobson-Garcia
  2012-12-17  8:45       ` Laurent Pinchart
  0 siblings, 1 reply; 23+ messages in thread
From: Damian Hobson-Garcia @ 2012-12-17  3:10 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Hideki EIRAKU, Paul Mundt, Magnus Damm, Simon Horman, linux-sh,
	linux-arm-kernel, linux-kernel, Marek Szyprowski,
	Katsuya MATSUBARA

Hi Laurent,

On 2012/12/17 2:25, Laurent Pinchart wrote:
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  arch/arm/mach-shmobile/Kconfig                     |    6 ------
>  arch/arm/mach-shmobile/Makefile                    |    3 ---
>  drivers/iommu/Kconfig                              |    6 ++++++
>  drivers/iommu/Makefile                             |    1 +
>  .../ipmmu.c => drivers/iommu/shmobile-ipmmu.c      |    0
>  5 files changed, 7 insertions(+), 9 deletions(-)
>  rename arch/arm/mach-shmobile/ipmmu.c => drivers/iommu/shmobile-ipmmu.c (100%)

I agree that arch/arm is not a good place, but I'm not completely sure
that ipmmu.c belongs in drivers/iommu.  The reason is because of the PMB
functionality provided by the IPMMU.  The PMB provides a fixed address
remapping capability that is completely unrelated to the IOMMU
functionality.  Since this remapping is done by writing the IPMMU
registers directly, instead of via a page table it doesn't really fit in
well with the IOMMU API (it also supports things like tiled/linear
address translation, which require some other method to set up).  Since
the PMB and the IOMMU functions of the IPPMU share the same register
address space, we would like to have one driver to handle the register
accesses of both of these functions.  That driver is ipmmu.c.  So if
ipmmu.c is in drivers/iommu, the entire IOMMU subsystem must be enabled
in order to use the PMB functionality.
So maybe it might be better to treat the IPMMU like a multifuction
device, with a core driver (ipmmu.c) in one location and the function
implementations in their own respective directories. Does drivers/mfd
sound like a good place for it?

Thanks,
Damian.

 --
Damian Hobson-Garcia
IGEL Co.,Ltd
http://www.igel.co.jp

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

* Re: [PATCH/WIP/RFC 02/14] shmobile-iommu: Move IPMMU driver to drivers/iommu
  2012-12-17  3:10     ` Damian Hobson-Garcia
@ 2012-12-17  8:45       ` Laurent Pinchart
  0 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2012-12-17  8:45 UTC (permalink / raw)
  To: Damian Hobson-Garcia
  Cc: Laurent Pinchart, Hideki EIRAKU, Paul Mundt, Magnus Damm,
	Simon Horman, linux-sh, linux-arm-kernel, linux-kernel,
	Marek Szyprowski, Katsuya MATSUBARA, iommu

Hi Damian,

(CC'ing iommu@lists.linux-foundation.org)

On Monday 17 December 2012 12:10:28 Damian Hobson-Garcia wrote:
> On 2012/12/17 2:25, Laurent Pinchart wrote:
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  arch/arm/mach-shmobile/Kconfig                     |    6 ------
> >  arch/arm/mach-shmobile/Makefile                    |    3 ---
> >  drivers/iommu/Kconfig                              |    6 ++++++
> >  drivers/iommu/Makefile                             |    1 +
> >  .../ipmmu.c => drivers/iommu/shmobile-ipmmu.c      |    0
> >  5 files changed, 7 insertions(+), 9 deletions(-)
> >  rename arch/arm/mach-shmobile/ipmmu.c => drivers/iommu/shmobile-ipmmu.c
> >  (100%)
>
> I agree that arch/arm is not a good place, but I'm not completely sure that
> ipmmu.c belongs in drivers/iommu.  The reason is because of the PMB
> functionality provided by the IPMMU.  The PMB provides a fixed address
> remapping capability that is completely unrelated to the IOMMU
> functionality.  Since this remapping is done by writing the IPMMU registers
> directly, instead of via a page table it doesn't really fit in well with the
> IOMMU API (it also supports things like tiled/linear address translation,
> which require some other method to set up).  Since the PMB and the IOMMU
> functions of the IPPMU share the same register address space, we would like
> to have one driver to handle the register accesses of both of these
> functions.  That driver is ipmmu.c.  So if ipmmu.c is in drivers/iommu, the
> entire IOMMU subsystem must be enabled in order to use the PMB
> functionality. So maybe it might be better to treat the IPMMU like a
> multifuction device, with a core driver (ipmmu.c) in one location and the
> function implementations in their own respective directories. Does
> drivers/mfd sound like a good place for it?

I've thought about this as well. The IPMMU indeed provides two different 
functions, so drivers/mfd/ could be a candidate. This being said, both the 
IOMMU function and the PMB function are related to virtual memory space 
management, so they're not totally unrelated. I agree that the PMB function 
isn't really an IOMMU in the sense that it will likely not be exposed through 
the existing IOMMU API.

However, drivers/iommu/ seems to me like a more natural place to store the 
IPMMU driver compared to drivers/mfd/. Enabling IOMMU support 
(CONFIG_IOMMU_SUPPORT) doesn't mean the IOMMU core (CONFIG_IOMMU_API) will be 
compiled in. There would thus be no extra code compiled in if the IOMMU 
function of the IPMMU is disabled.

-- 
Regards,

Laurent Pinchart


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

end of thread, other threads:[~2012-12-17  8:43 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-15  8:34 [PATCH v4 0/2] Renesas IPMMU driver for sh7372 Hideki EIRAKU
2012-10-15  8:34 ` [PATCH v4 1/2] iommu/shmobile: Add iommu driver for Renesas IPMMU modules Hideki EIRAKU
2012-12-10 15:55   ` Laurent Pinchart
2012-12-11 10:10     ` Hideki EIRAKU
2012-12-11 12:36       ` Laurent Pinchart
2012-10-15  8:34 ` [PATCH v4 2/2] ARM: mach-shmobile: sh7372: Add IPMMU device Hideki EIRAKU
2012-12-16 17:25 ` [PATCH/WIP/RFC 00/14] Renesas IPMMU driver work in progress Laurent Pinchart
2012-12-16 17:25   ` [PATCH/WIP/RFC 01/14] ARM: sh-mobile: Protect ipmmu.h header with ifndef/define Laurent Pinchart
2012-12-16 17:25   ` [PATCH/WIP/RFC 02/14] shmobile-iommu: Move IPMMU driver to drivers/iommu Laurent Pinchart
2012-12-17  3:10     ` Damian Hobson-Garcia
2012-12-17  8:45       ` Laurent Pinchart
2012-12-16 17:25   ` [PATCH/WIP/RFC 03/14] shmobile-iommu: Remove __devinit Laurent Pinchart
2012-12-16 17:25   ` [PATCH/WIP/RFC 04/14] shmobile-iommu: Use devm_* managed functions Laurent Pinchart
2012-12-16 17:25   ` [PATCH/WIP/RFC 05/14] ARM: iommu: Include linux/kref.h in asm/dma-iommu.h Laurent Pinchart
2012-12-16 17:25   ` [PATCH/WIP/RFC 06/14] shmobile-iommu: Sort header files alphabetically Laurent Pinchart
2012-12-16 17:25   ` [PATCH/WIP/RFC 07/14] shmobile-iommu: Move header file from arch/ to drivers/iommu/ Laurent Pinchart
2012-12-16 17:25   ` [PATCH/WIP/RFC 08/14] shmobile-iommu: Rename shmobile_iommu_priv to shmobile_iommu_domain Laurent Pinchart
2012-12-16 17:25   ` [PATCH/WIP/RFC 09/14] shmobile-ipmmu: Rename ipmmu_priv to shmobile_ipmmu Laurent Pinchart
2012-12-16 17:25   ` [PATCH/WIP/RFC 10/14] shmobile-ipmmu: Pass a struct shmobile_ipmmu to IPMMU functions Laurent Pinchart
2012-12-16 17:25   ` [PATCH/WIP/RFC 11/14] shmobile-ipmmu: Store a struct shmobile_iommu_arch_data in archdata.iommu Laurent Pinchart
2012-12-16 17:25   ` [PATCH/WIP/RFC 12/14] shmobile-ipmmu: Store ipmmu pointer in iommu arch data and iommu domain Laurent Pinchart
2012-12-16 17:25   ` [PATCH/WIP/RFC 13/14] shmobile-ipmmu: Remove unneeded lock_add spinlock Laurent Pinchart
2012-12-16 17:26   ` [PATCH/WIP/RFC 14/14] shmobile-ipmmu: Store iommu_mapping in struct shmobile_ipmmu Laurent Pinchart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).