All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] Initial support for omap iommu driver
@ 2009-05-05 12:46 Hiroshi DOYU
  2009-05-05 12:46 ` [PATCH 1/6] omap iommu: tlb and pagetable primitives Hiroshi DOYU
                   ` (5 more replies)
  0 siblings, 6 replies; 53+ messages in thread
From: Hiroshi DOYU @ 2009-05-05 12:46 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-omap, h-kanigeri2, omar.ramirez, sakari.ailus, tony

Some of TI OMAP series have the peripheral devices with their own
Memory Management Unit(IOMMU), which is composed of its own TLB and
optional H/W pagetable (TWL). These MMUs doesn't depend on MPU(ARM)
MMU at all, but their algorithms are quite similar and they share the
same physical address space. This patch provides with common in-kernel
iommu APIs such OMAP peripheral devices(Camera ISP, IVA1, IVA2, DSP
and the equivalent ones in the latest OMAP successors) to handle
peripheral device IOMMUs in the same manner.

"tlb and pagetable primitives" has been updated with Russell's
comments on:

      http://marc.info/?l=linux-omap&m=122087083712670&w=2

Generalizing this omap iommu code independently from each device
drivers, "Camera(ISP)", "TI bridge(IVA/DSP)", "dspgateway(DSP)" and
the future OMAP equivalent device would be more robust and reduce the
maintenance cost since keeping this critical code at one place can
avoid some risks, like wrong MMU settings, which may cause critical
damages on the system.

---
The following changes since commit 091438dd5668396328a3419abcbc6591159eb8d1:
  Linus Torvalds (1):
        Linux 2.6.30-rc4

are available in the git repository at:

  http://git.gitorious.org/lk/mainline.git iommu

Hiroshi DOYU (6):
      omap iommu: tlb and pagetable primitives
      omap iommu: omap2 architecture specific functions
      omap iommu: omap3 iommu device registration
      omap iommu: simple virtual address space management
      omap iommu: entries for Kconfig and Makefile
      omap2 iommu: entries for Kconfig and Makefile

 arch/arm/include/asm/io.h                |    6 +
 arch/arm/mach-omap2/Makefile             |    5 +
 arch/arm/mach-omap2/iommu2.c             |  323 ++++++++++
 arch/arm/mach-omap2/omap3-iommu.c        |  105 ++++
 arch/arm/mm/ioremap.c                    |   11 +
 arch/arm/plat-omap/Kconfig               |    8 +
 arch/arm/plat-omap/Makefile              |    1 +
 arch/arm/plat-omap/include/mach/iommu.h  |  168 +++++
 arch/arm/plat-omap/include/mach/iommu2.h |   96 +++
 arch/arm/plat-omap/include/mach/iovmm.h  |   94 +++
 arch/arm/plat-omap/iommu.c               |  996 ++++++++++++++++++++++++++++++
 arch/arm/plat-omap/iopgtable.h           |   72 +++
 arch/arm/plat-omap/iovmm.c               |  890 ++++++++++++++++++++++++++
 13 files changed, 2775 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-omap2/iommu2.c
 create mode 100644 arch/arm/mach-omap2/omap3-iommu.c
 create mode 100644 arch/arm/plat-omap/include/mach/iommu.h
 create mode 100644 arch/arm/plat-omap/include/mach/iommu2.h
 create mode 100644 arch/arm/plat-omap/include/mach/iovmm.h
 create mode 100644 arch/arm/plat-omap/iommu.c
 create mode 100644 arch/arm/plat-omap/iopgtable.h
 create mode 100644 arch/arm/plat-omap/iovmm.c

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

* [PATCH 1/6] omap iommu: tlb and pagetable primitives
  2009-05-05 12:46 [PATCH 0/6] Initial support for omap iommu driver Hiroshi DOYU
@ 2009-05-05 12:46 ` Hiroshi DOYU
  2009-05-05 20:19   ` Kanigeri, Hari
  2009-05-05 12:46 ` [PATCH 2/6] omap iommu: omap2 architecture specific functions Hiroshi DOYU
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 53+ messages in thread
From: Hiroshi DOYU @ 2009-05-05 12:46 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-omap, h-kanigeri2, omar.ramirez, sakari.ailus, tony

This patch provides:

- iotlb_*()     : iommu tlb operations
- iopgtable_*() : iommu pagetable(twl) operations
- iommu_*()     : the other generic operations

and the entry points to register and acquire iommu object.

Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---

 arch/arm/plat-omap/include/mach/iommu.h |  168 +++++
 arch/arm/plat-omap/iommu.c              |  996 +++++++++++++++++++++++++++++++
 arch/arm/plat-omap/iopgtable.h          |   72 ++
 3 files changed, 1236 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/plat-omap/include/mach/iommu.h
 create mode 100644 arch/arm/plat-omap/iommu.c
 create mode 100644 arch/arm/plat-omap/iopgtable.h

diff --git a/arch/arm/plat-omap/include/mach/iommu.h b/arch/arm/plat-omap/include/mach/iommu.h
new file mode 100644
index 0000000..769b00b
--- /dev/null
+++ b/arch/arm/plat-omap/include/mach/iommu.h
@@ -0,0 +1,168 @@
+/*
+ * omap iommu: main structures
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation
+ *
+ * Written by Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __MACH_IOMMU_H
+#define __MACH_IOMMU_H
+
+struct iotlb_entry {
+	u32 da;
+	u32 pa;
+	u32 pgsz, prsvd, valid;
+	union {
+		u16 ap;
+		struct {
+			u32 endian, elsz, mixed;
+		};
+	};
+};
+
+struct iommu {
+	const char	*name;
+	struct module	*owner;
+	struct clk	*clk;
+	void __iomem	*regbase;
+	struct device	*dev;
+
+	unsigned int	refcount;
+	struct mutex	iommu_lock;	/* global for this whole object */
+
+	/*
+	 * We don't change iopgd for a situation like pgd for a task,
+	 * but share it globally for each iommu.
+	 */
+	u32		*iopgd;
+	spinlock_t	page_table_lock; /* protect iopgd */
+
+	int		nr_tlb_entries;
+
+	struct list_head	mmap;
+	struct mutex		mmap_lock; /* protect mmap */
+
+	int (*isr)(struct iommu *obj);
+
+	void *ctx; /* iommu context: registres saved area */
+};
+
+struct cr_regs {
+	union {
+		struct {
+			u16 cam_l;
+			u16 cam_h;
+		};
+		u32 cam;
+	};
+	union {
+		struct {
+			u16 ram_l;
+			u16 ram_h;
+		};
+		u32 ram;
+	};
+};
+
+struct iotlb_lock {
+	short base;
+	short vict;
+};
+
+/* architecture specific functions */
+struct iommu_functions {
+	unsigned long	version;
+
+	int (*enable)(struct iommu *obj);
+	void (*disable)(struct iommu *obj);
+	u32 (*fault_isr)(struct iommu *obj, u32 *ra);
+
+	void (*tlb_read_cr)(struct iommu *obj, struct cr_regs *cr);
+	void (*tlb_load_cr)(struct iommu *obj, struct cr_regs *cr);
+
+	struct cr_regs *(*alloc_cr)(struct iommu *obj, struct iotlb_entry *e);
+	int (*cr_valid)(struct cr_regs *cr);
+	u32 (*cr_to_virt)(struct cr_regs *cr);
+	void (*cr_to_e)(struct cr_regs *cr, struct iotlb_entry *e);
+	ssize_t (*dump_cr)(struct iommu *obj, struct cr_regs *cr, char *buf);
+
+	u32 (*get_pte_attr)(struct iotlb_entry *e);
+
+	void (*save_ctx)(struct iommu *obj);
+	void (*restore_ctx)(struct iommu *obj);
+	ssize_t (*dump_ctx)(struct iommu *obj, char *buf);
+};
+
+struct iommu_platform_data {
+	const char *name;
+	const char *clk_name;
+	const int nr_tlb_entries;
+};
+
+#if defined(CONFIG_ARCH_OMAP1)
+#error "iommu for this processor not implemented yet"
+#else
+#include <mach/iommu2.h>
+#endif
+
+/*
+ * utilities for super page(16MB, 1MB, 64KB and 4KB)
+ */
+
+#define iopgsz_max(bytes)			\
+	(((bytes) >= SZ_16M) ? SZ_16M :		\
+	 ((bytes) >= SZ_1M)  ? SZ_1M  :		\
+	 ((bytes) >= SZ_64K) ? SZ_64K :		\
+	 ((bytes) >= SZ_4K)  ? SZ_4K  :	0)
+
+#define bytes_to_iopgsz(bytes)				\
+	(((bytes) == SZ_16M) ? MMU_CAM_PGSZ_16M :	\
+	 ((bytes) == SZ_1M)  ? MMU_CAM_PGSZ_1M  :	\
+	 ((bytes) == SZ_64K) ? MMU_CAM_PGSZ_64K :	\
+	 ((bytes) == SZ_4K)  ? MMU_CAM_PGSZ_4K  : -1)
+
+#define iopgsz_to_bytes(iopgsz)				\
+	(((iopgsz) == MMU_CAM_PGSZ_16M)	? SZ_16M :	\
+	 ((iopgsz) == MMU_CAM_PGSZ_1M)	? SZ_1M  :	\
+	 ((iopgsz) == MMU_CAM_PGSZ_64K)	? SZ_64K :	\
+	 ((iopgsz) == MMU_CAM_PGSZ_4K)	? SZ_4K  : 0)
+
+#define iopgsz_ok(bytes) (bytes_to_iopgsz(bytes) >= 0)
+
+/*
+ * global functions
+ */
+extern u32 iommu_arch_version(void);
+
+extern void iotlb_cr_to_e(struct cr_regs *cr, struct iotlb_entry *e);
+extern u32 iotlb_cr_to_virt(struct cr_regs *cr);
+
+extern int load_iotlb_entry(struct iommu *obj, struct iotlb_entry *e);
+extern void flush_iotlb_page(struct iommu *obj, u32 da);
+extern void flush_iotlb_range(struct iommu *obj, u32 start, u32 end);
+extern void flush_iotlb_all(struct iommu *obj);
+
+extern int iopgtable_store_entry(struct iommu *obj, struct iotlb_entry *e);
+extern size_t iopgtable_clear_entry(struct iommu *obj, u32 iova);
+
+extern struct iommu *iommu_get(const char *name);
+extern void iommu_put(struct iommu *obj);
+
+extern void iommu_save_ctx(struct iommu *obj);
+extern void iommu_restore_ctx(struct iommu *obj);
+
+extern int install_iommu_arch(const struct iommu_functions *ops);
+extern void uninstall_iommu_arch(const struct iommu_functions *ops);
+
+extern int foreach_iommu_device(void *data,
+				int (*fn)(struct device *, void *));
+
+extern ssize_t iommu_dump_ctx(struct iommu *obj, char *buf);
+extern size_t dump_tlb_entries(struct iommu *obj, char *buf);
+
+#endif /* __MACH_IOMMU_H */
diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
new file mode 100644
index 0000000..4cf449f
--- /dev/null
+++ b/arch/arm/plat-omap/iommu.c
@@ -0,0 +1,996 @@
+/*
+ * omap iommu: tlb and pagetable primitives
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation
+ *
+ * Written by Hiroshi DOYU <Hiroshi.DOYU@nokia.com>,
+ *		Paul Mundt and Toshihiro Kobayashi
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/ioport.h>
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+
+#include <asm/cacheflush.h>
+
+#include <mach/iommu.h>
+
+#include "iopgtable.h"
+
+/* accommodate the difference between omap1 and omap2/3 */
+static const struct iommu_functions *arch_iommu;
+
+static struct platform_driver omap_iommu_driver;
+static struct kmem_cache *iopte_cachep;
+
+/**
+ * install_iommu_arch - Install archtecure specific iommu functions
+ * @ops:	a pointer to architecture specific iommu functions
+ *
+ * There are several kind of iommu algorithm(tlb, pagetable) among
+ * omap series. This interface installs such an iommu algorighm.
+ **/
+int install_iommu_arch(const struct iommu_functions *ops)
+{
+	if (arch_iommu)
+		return -EBUSY;
+
+	arch_iommu = ops;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(install_iommu_arch);
+
+/**
+ * uninstall_iommu_arch - Uninstall archtecure specific iommu functions
+ * @ops:	a pointer to architecture specific iommu functions
+ *
+ * This interface uninstalls the iommu algorighm installed previously.
+ **/
+void uninstall_iommu_arch(const struct iommu_functions *ops)
+{
+	if (arch_iommu != ops)
+		pr_err("%s: not your arch\n", __func__);
+
+	arch_iommu = NULL;
+}
+EXPORT_SYMBOL_GPL(uninstall_iommu_arch);
+
+/**
+ * iommu_save_ctx - Save registers for pm off-mode support
+ * @obj:	target iommu
+ **/
+void iommu_save_ctx(struct iommu *obj)
+{
+	arch_iommu->save_ctx(obj);
+}
+EXPORT_SYMBOL_GPL(iommu_save_ctx);
+
+/**
+ * iommu_restore_ctx - Restore registers for pm off-mode support
+ * @obj:	target iommu
+ **/
+void iommu_restore_ctx(struct iommu *obj)
+{
+	arch_iommu->restore_ctx(obj);
+}
+EXPORT_SYMBOL_GPL(iommu_restore_ctx);
+
+/**
+ * iommu_arch_version - Return running iommu arch version
+ **/
+u32 iommu_arch_version(void)
+{
+	return arch_iommu->version;
+}
+EXPORT_SYMBOL_GPL(iommu_arch_version);
+
+static int iommu_enable(struct iommu *obj)
+{
+	int err;
+
+	if (!obj)
+		return -EINVAL;
+
+	clk_enable(obj->clk);
+
+	err = arch_iommu->enable(obj);
+
+	clk_disable(obj->clk);
+	return err;
+}
+
+static void iommu_disable(struct iommu *obj)
+{
+	if (!obj)
+		return;
+
+	clk_enable(obj->clk);
+
+	arch_iommu->disable(obj);
+
+	clk_disable(obj->clk);
+}
+
+/*
+ *	TLB operations
+ */
+void iotlb_cr_to_e(struct cr_regs *cr, struct iotlb_entry *e)
+{
+	BUG_ON(!cr || !e);
+
+	arch_iommu->cr_to_e(cr, e);
+}
+EXPORT_SYMBOL_GPL(iotlb_cr_to_e);
+
+static inline int iotlb_cr_valid(struct cr_regs *cr)
+{
+	if (!cr)
+		return -EINVAL;
+
+	return arch_iommu->cr_valid(cr);
+}
+
+static inline struct cr_regs *iotlb_alloc_cr(struct iommu *obj,
+					     struct iotlb_entry *e)
+{
+	if (!e)
+		return NULL;
+
+	return arch_iommu->alloc_cr(obj, e);
+}
+
+u32 iotlb_cr_to_virt(struct cr_regs *cr)
+{
+	return arch_iommu->cr_to_virt(cr);
+}
+EXPORT_SYMBOL_GPL(iotlb_cr_to_virt);
+
+static u32 get_iopte_attr(struct iotlb_entry *e)
+{
+	return arch_iommu->get_pte_attr(e);
+}
+
+static u32 iommu_report_fault(struct iommu *obj, u32 *da)
+{
+	return arch_iommu->fault_isr(obj, da);
+}
+
+static void iotlb_lock_get(struct iommu *obj, struct iotlb_lock *l)
+{
+	u32 val;
+
+	val = iommu_read_reg(obj, MMU_LOCK);
+
+	l->base = MMU_LOCK_BASE(val);
+	l->vict = MMU_LOCK_VICT(val);
+
+	BUG_ON(l->base != 0); /* Currently no preservation is used */
+}
+
+static void iotlb_lock_set(struct iommu *obj, struct iotlb_lock *l)
+{
+	u32 val;
+
+	BUG_ON(l->base != 0); /* Currently no preservation is used */
+
+	val = (l->base << MMU_LOCK_BASE_SHIFT);
+	val |= (l->vict << MMU_LOCK_VICT_SHIFT);
+
+	iommu_write_reg(obj, val, MMU_LOCK);
+}
+
+static void iotlb_read_cr(struct iommu *obj, struct cr_regs *cr)
+{
+	arch_iommu->tlb_read_cr(obj, cr);
+}
+
+static void iotlb_load_cr(struct iommu *obj, struct cr_regs *cr)
+{
+	arch_iommu->tlb_load_cr(obj, cr);
+
+	iommu_write_reg(obj, 1, MMU_FLUSH_ENTRY);
+	iommu_write_reg(obj, 1, MMU_LD_TLB);
+}
+
+/**
+ * iotlb_dump_cr - Dump an iommu tlb entry into buf
+ * @obj:	target iommu
+ * @cr:		contents of cam and ram register
+ * @buf:	output buffer
+ **/
+static inline ssize_t iotlb_dump_cr(struct iommu *obj, struct cr_regs *cr,
+				    char *buf)
+{
+	BUG_ON(!cr || !buf);
+
+	return arch_iommu->dump_cr(obj, cr, buf);
+}
+
+/**
+ * load_iotlb_entry - Set an iommu tlb entry
+ * @obj:	target iommu
+ * @e:		an iommu tlb entry info
+ **/
+int load_iotlb_entry(struct iommu *obj, struct iotlb_entry *e)
+{
+	int i;
+	int err = 0;
+	struct iotlb_lock l;
+	struct cr_regs *cr;
+
+	if (!obj || !obj->nr_tlb_entries || !e)
+		return -EINVAL;
+
+	clk_enable(obj->clk);
+
+	for (i = 0; i < obj->nr_tlb_entries; i++) {
+		struct cr_regs tmp;
+
+		iotlb_lock_get(obj, &l);
+		l.vict = i;
+		iotlb_lock_set(obj, &l);
+		iotlb_read_cr(obj, &tmp);
+		if (!iotlb_cr_valid(&tmp))
+			break;
+	}
+
+	if (i == obj->nr_tlb_entries) {
+		dev_dbg(obj->dev, "%s: full: no entry\n", __func__);
+		err = -EBUSY;
+		goto out;
+	}
+
+	cr = iotlb_alloc_cr(obj, e);
+	if (IS_ERR(cr)) {
+		clk_disable(obj->clk);
+		return PTR_ERR(cr);
+	}
+
+	iotlb_load_cr(obj, cr);
+	kfree(cr);
+
+	/* increment victim for next tlb load */
+	if (++l.vict == obj->nr_tlb_entries)
+		l.vict = 0;
+	iotlb_lock_set(obj, &l);
+out:
+	clk_disable(obj->clk);
+	return err;
+}
+EXPORT_SYMBOL_GPL(load_iotlb_entry);
+
+/**
+ * flush_iotlb_page - Clear an iommu tlb entry
+ * @obj:	target iommu
+ * @da:		iommu device virtual address
+ *
+ * Clear an iommu tlb entry which includes 'da' address.
+ **/
+void flush_iotlb_page(struct iommu *obj, u32 da)
+{
+	struct iotlb_lock l;
+	int i;
+
+	clk_enable(obj->clk);
+
+	for (i = 0; i < obj->nr_tlb_entries; i++) {
+		struct cr_regs cr;
+		u32 start;
+		size_t bytes;
+
+		iotlb_lock_get(obj, &l);
+		l.vict = i;
+		iotlb_lock_set(obj, &l);
+		iotlb_read_cr(obj, &cr);
+		if (!iotlb_cr_valid(&cr))
+			continue;
+
+		start = iotlb_cr_to_virt(&cr);
+		bytes = iopgsz_to_bytes(cr.cam & 3);
+
+		if ((start <= da) && (da < start + bytes)) {
+			dev_dbg(obj->dev, "%s: %08x<=%08x(%x)\n",
+				__func__, start, da, bytes);
+
+			iommu_write_reg(obj, 1, MMU_FLUSH_ENTRY);
+		}
+	}
+	clk_disable(obj->clk);
+
+	if (i == obj->nr_tlb_entries)
+		dev_dbg(obj->dev, "%s: no page for %08x\n", __func__, da);
+}
+EXPORT_SYMBOL_GPL(flush_iotlb_page);
+
+/**
+ * flush_iotlb_range - Clear an iommu tlb entries
+ * @obj:	target iommu
+ * @start:	iommu device virtual address(start)
+ * @end:	iommu device virtual address(end)
+ *
+ * Clear an iommu tlb entry which includes 'da' address.
+ **/
+void flush_iotlb_range(struct iommu *obj, u32 start, u32 end)
+{
+	u32 da = start;
+
+	while (da < end) {
+		flush_iotlb_page(obj, da);
+		/* FIXME: Optimize for multiple page size */
+		da += IOPTE_SIZE;
+	}
+}
+EXPORT_SYMBOL_GPL(flush_iotlb_range);
+
+/**
+ * flush_iotlb_all - Clear all iommu tlb entries
+ * @obj:	target iommu
+ **/
+void flush_iotlb_all(struct iommu *obj)
+{
+	struct iotlb_lock l;
+
+	clk_enable(obj->clk);
+
+	l.base = 0;
+	l.vict = 0;
+	iotlb_lock_set(obj, &l);
+
+	iommu_write_reg(obj, 1, MMU_GFLUSH);
+
+	clk_disable(obj->clk);
+}
+EXPORT_SYMBOL_GPL(flush_iotlb_all);
+
+#if defined(CONFIG_OMAP_IOMMU_DEBUG_MODULE)
+
+ssize_t iommu_dump_ctx(struct iommu *obj, char *buf)
+{
+	ssize_t bytes;
+
+	if (!obj || !buf)
+		return -EINVAL;
+
+	clk_enable(obj->clk);
+
+	bytes = arch_iommu->dump_ctx(obj, buf);
+
+	clk_disable(obj->clk);
+
+	return bytes;
+}
+EXPORT_SYMBOL_GPL(iommu_dump_ctx);
+
+static int __dump_tlb_entries(struct iommu *obj, struct cr_regs *crs)
+{
+	int i;
+	struct iotlb_lock saved, l;
+	struct cr_regs *p = crs;
+
+	clk_enable(obj->clk);
+
+	iotlb_lock_get(obj, &saved);
+	memcpy(&l, &saved, sizeof(saved));
+
+	for (i = 0; i < obj->nr_tlb_entries; i++) {
+		struct cr_regs tmp;
+
+		iotlb_lock_get(obj, &l);
+		l.vict = i;
+		iotlb_lock_set(obj, &l);
+		iotlb_read_cr(obj, &tmp);
+		if (!iotlb_cr_valid(&tmp))
+			continue;
+
+		*p++ = tmp;
+	}
+	iotlb_lock_set(obj, &saved);
+	clk_disable(obj->clk);
+
+	return  p - crs;
+}
+
+/**
+ * dump_tlb_entries - dump cr arrays to given buffer
+ * @obj:	target iommu
+ * @buf:	output buffer
+ **/
+size_t dump_tlb_entries(struct iommu *obj, char *buf)
+{
+	int i, n;
+	struct cr_regs *cr;
+	char *p = buf;
+
+	cr = kcalloc(obj->nr_tlb_entries, sizeof(*cr), GFP_KERNEL);
+	if (!cr)
+		return 0;
+
+	n = __dump_tlb_entries(obj, cr);
+	for (i = 0; i < n; i++)
+		p += iotlb_dump_cr(obj, cr + i, p);
+	kfree(cr);
+
+	return p - buf;
+}
+EXPORT_SYMBOL_GPL(dump_tlb_entries);
+
+int foreach_iommu_device(void *data, int (*fn)(struct device *, void *))
+{
+	return driver_for_each_device(&omap_iommu_driver.driver,
+				      NULL, data, fn);
+}
+EXPORT_SYMBOL_GPL(foreach_iommu_device);
+
+#endif /* CONFIG_OMAP_IOMMU_DEBUG_MODULE */
+
+/*
+ *	H/W pagetable operations
+ */
+static void flush_iopgd_range(u32 *first, u32 *last)
+{
+	/* FIXME: L2 cache should be taken care of if it exists */
+	do {
+		asm("mcr	p15, 0, %0, c7, c10, 1 @ flush_pgd"
+		    : : "r" (first));
+		first += L1_CACHE_BYTES / sizeof(*first);
+	} while (first <= last);
+}
+
+static void flush_iopte_range(u32 *first, u32 *last)
+{
+	/* FIXME: L2 cache should be taken care of if it exists */
+	do {
+		asm("mcr	p15, 0, %0, c7, c10, 1 @ flush_pte"
+		    : : "r" (first));
+		first += L1_CACHE_BYTES / sizeof(*first);
+	} while (first <= last);
+}
+
+static void iopte_free(u32 *iopte)
+{
+	/* Note: freed iopte's must be clean ready for re-use */
+	kmem_cache_free(iopte_cachep, iopte);
+}
+
+static u32 *iopte_alloc(struct iommu *obj, u32 *iopgd, u32 da)
+{
+	u32 *iopte;
+
+	/* a table has already existed */
+	if (*iopgd)
+		goto pte_ready;
+
+	/*
+	 * do the allocation outside the page table lock
+	 */
+	spin_unlock(&obj->page_table_lock);
+	iopte = kmem_cache_zalloc(iopte_cachep, GFP_KERNEL);
+	spin_lock(&obj->page_table_lock);
+
+	if (!*iopgd) {
+		if (!iopte)
+			return ERR_PTR(-ENOMEM);
+
+		*iopgd = virt_to_phys(iopte) | IOPGD_TABLE;
+		flush_iopgd_range(iopgd, iopgd);
+
+		dev_vdbg(obj->dev, "%s: a new pte:%p\n", __func__, iopte);
+	} else {
+		/* We raced, free the reduniovant table */
+		iopte_free(iopte);
+	}
+
+pte_ready:
+	iopte = iopte_offset(iopgd, da);
+
+	dev_vdbg(obj->dev,
+		 "%s: da:%08x pgd:%p *pgd:%08x pte:%p *pte:%08x\n",
+		 __func__, da, iopgd, *iopgd, iopte, *iopte);
+
+	return iopte;
+}
+
+static int iopgd_alloc_section(struct iommu *obj, u32 da, u32 pa, u32 prot)
+{
+	u32 *iopgd = iopgd_offset(obj, da);
+
+	*iopgd = (pa & IOSECTION_MASK) | prot | IOPGD_SECTION;
+	flush_iopgd_range(iopgd, iopgd);
+	return 0;
+}
+
+static int iopgd_alloc_super(struct iommu *obj, u32 da, u32 pa, u32 prot)
+{
+	u32 *iopgd = iopgd_offset(obj, da);
+	int i;
+
+	for (i = 0; i < 16; i++)
+		*(iopgd + i) = (pa & IOSUPER_MASK) | prot | IOPGD_SUPER;
+	flush_iopgd_range(iopgd, iopgd + 15);
+	return 0;
+}
+
+static int iopte_alloc_page(struct iommu *obj, u32 da, u32 pa, u32 prot)
+{
+	u32 *iopgd = iopgd_offset(obj, da);
+	u32 *iopte = iopte_alloc(obj, iopgd, da);
+
+	if (IS_ERR(iopte))
+		return PTR_ERR(iopte);
+
+	*iopte = (pa & IOPAGE_MASK) | prot | IOPTE_SMALL;
+	flush_iopte_range(iopte, iopte);
+
+	dev_vdbg(obj->dev, "%s: da:%08x pa:%08x pte:%p *pte:%08x\n",
+		 __func__, da, pa, iopte, *iopte);
+
+	return 0;
+}
+
+static int iopte_alloc_large(struct iommu *obj, u32 da, u32 pa, u32 prot)
+{
+	u32 *iopgd = iopgd_offset(obj, da);
+	u32 *iopte = iopte_alloc(obj, iopgd, da);
+	int i;
+
+	if (IS_ERR(iopte))
+		return PTR_ERR(iopte);
+
+	for (i = 0; i < 16; i++)
+		*(iopte + i) = (pa & IOLARGE_MASK) | prot | IOPTE_LARGE;
+	flush_iopte_range(iopte, iopte + 15);
+	return 0;
+}
+
+static int iopgtable_store_entry_core(struct iommu *obj, struct iotlb_entry *e)
+{
+	int (*fn)(struct iommu *, u32, u32, u32);
+	u32 prot;
+	int err;
+
+	if (!obj || !e)
+		return -EINVAL;
+
+	switch (e->pgsz) {
+	case MMU_CAM_PGSZ_16M:
+		fn = iopgd_alloc_super;
+		break;
+	case MMU_CAM_PGSZ_1M:
+		fn = iopgd_alloc_section;
+		break;
+	case MMU_CAM_PGSZ_64K:
+		fn = iopte_alloc_large;
+		break;
+	case MMU_CAM_PGSZ_4K:
+		fn = iopte_alloc_page;
+		break;
+	default:
+		fn = NULL;
+		BUG();
+		break;
+	}
+
+	prot = get_iopte_attr(e);
+
+	spin_lock(&obj->page_table_lock);
+	err = fn(obj, e->da, e->pa, prot);
+	spin_unlock(&obj->page_table_lock);
+
+	return err;
+}
+
+/**
+ * iopgtable_store_entry - Make an iommu pte entry
+ * @obj:	target iommu
+ * @e:		an iommu tlb entry info
+ **/
+int iopgtable_store_entry(struct iommu *obj, struct iotlb_entry *e)
+{
+	int err;
+
+	flush_iotlb_page(obj, e->da);
+	err = iopgtable_store_entry_core(obj, e);
+#ifdef PREFETCH_IOTLB
+	if (!err)
+		load_iotlb_entry(obj, e);
+#endif
+	return err;
+}
+EXPORT_SYMBOL_GPL(iopgtable_store_entry);
+
+/**
+ * iopgtable_lookup_entry - Lookup an iommu pte entry
+ * @obj:	target iommu
+ * @da:		iommu device virtual address
+ * @ppgd:	iommu pgd entry pointer to be returned
+ * @ppte:	iommu pte entry pointer to be returned
+ **/
+void iopgtable_lookup_entry(struct iommu *obj, u32 da, u32 **ppgd, u32 **ppte)
+{
+	u32 *iopgd, *iopte = NULL;
+
+	iopgd = iopgd_offset(obj, da);
+	if (!*iopgd)
+		goto out;
+
+	if (*iopgd & IOPGD_TABLE)
+		iopte = iopte_offset(iopgd, da);
+out:
+	*ppgd = iopgd;
+	*ppte = iopte;
+}
+EXPORT_SYMBOL_GPL(iopgtable_lookup_entry);
+
+static size_t iopgtable_clear_entry_core(struct iommu *obj, u32 da)
+{
+	size_t bytes;
+	u32 *iopgd = iopgd_offset(obj, da);
+	int nent = 1;
+
+	if (!*iopgd)
+		return 0;
+
+	if (*iopgd & IOPGD_TABLE) {
+		int i;
+		u32 *iopte = iopte_offset(iopgd, da);
+
+		bytes = IOPTE_SIZE;
+		if (*iopte & IOPTE_LARGE) {
+			nent *= 16;
+			/* rewind to the 1st entry */
+			iopte = (u32 *)((u32)iopte & IOLARGE_MASK);
+		}
+		bytes *= nent;
+		memset(iopte, 0, nent * sizeof(*iopte));
+		flush_iopte_range(iopte, iopte + (nent - 1) * sizeof(*iopte));
+
+		/*
+		 * do table walk to check if this table is necessary or not
+		 */
+		iopte = iopte_offset(iopgd, 0);
+		for (i = 0; i < PTRS_PER_IOPTE; i++)
+			if (iopte[i])
+				goto out;
+
+		iopte_free(iopte);
+		nent = 1; /* for the next L1 entry */
+	} else {
+		bytes = IOPGD_SIZE;
+		if (*iopgd & IOPGD_SUPER) {
+			nent *= 16;
+			/* rewind to the 1st entry */
+			iopgd = (u32 *)((u32)iopgd & IOSUPER_MASK);
+		}
+		bytes *= nent;
+	}
+	memset(iopgd, 0, nent * sizeof(*iopgd));
+	flush_iopgd_range(iopgd, iopgd + (nent - 1) * sizeof(*iopgd));
+out:
+	return bytes;
+}
+
+/**
+ * iopgtable_clear_entry - Remove an iommu pte entry
+ * @obj:	target iommu
+ * @da:		iommu device virtual address
+ **/
+size_t iopgtable_clear_entry(struct iommu *obj, u32 da)
+{
+	size_t bytes;
+
+	spin_lock(&obj->page_table_lock);
+
+	bytes = iopgtable_clear_entry_core(obj, da);
+	flush_iotlb_page(obj, da);
+
+	spin_unlock(&obj->page_table_lock);
+
+	return bytes;
+}
+EXPORT_SYMBOL_GPL(iopgtable_clear_entry);
+
+static void iopgtable_clear_entry_all(struct iommu *obj)
+{
+	int i;
+
+	spin_lock(&obj->page_table_lock);
+
+	for (i = 0; i < PTRS_PER_IOPGD; i++) {
+		u32 da;
+		u32 *iopgd;
+
+		da = i << IOPGD_SHIFT;
+		iopgd = iopgd_offset(obj, da);
+
+		if (!*iopgd)
+			continue;
+
+		if (*iopgd & IOPGD_TABLE)
+			iopte_free(iopte_offset(iopgd, 0));
+
+		*iopgd = 0;
+		flush_iopgd_range(iopgd, iopgd);
+	}
+
+	flush_iotlb_all(obj);
+
+	spin_unlock(&obj->page_table_lock);
+}
+
+/*
+ *	Device IOMMU generic operations
+ */
+static irqreturn_t iommu_fault_handler(int irq, void *data)
+{
+	u32 stat, da;
+	u32 *iopgd, *iopte;
+	int err = -EIO;
+	struct iommu *obj = data;
+
+	if (!obj->refcount)
+		return IRQ_NONE;
+
+	/* Dynamic loading TLB or PTE */
+	if (obj->isr)
+		err = obj->isr(obj);
+
+	if (!err)
+		return IRQ_HANDLED;
+
+	clk_enable(obj->clk);
+	stat = iommu_report_fault(obj, &da);
+	clk_disable(obj->clk);
+	if (!stat)
+		return IRQ_HANDLED;
+
+	iopgd = iopgd_offset(obj, da);
+
+	if (!(*iopgd & IOPGD_TABLE)) {
+		dev_err(obj->dev, "%s: da:%08x pgd:%p *pgd:%08x\n", __func__,
+			da, iopgd, *iopgd);
+		return IRQ_NONE;
+	}
+
+	iopte = iopte_offset(iopgd, da);
+
+	dev_err(obj->dev, "%s: da:%08x pgd:%p *pgd:%08x pte:%p *pte:%08x\n",
+		__func__, da, iopgd, *iopgd, iopte, *iopte);
+
+	return IRQ_NONE;
+}
+
+static int device_match_by_alias(struct device *dev, void *data)
+{
+	struct iommu *obj = to_iommu(dev);
+	const char *name = data;
+
+	pr_debug("%s: %s %s\n", __func__, obj->name, name);
+
+	return strcmp(obj->name, name) == 0;
+}
+
+/**
+ * iommu_get - Get iommu handler
+ * @name:	target iommu name
+ **/
+struct iommu *iommu_get(const char *name)
+{
+	int err = -ENOMEM;
+	struct device *dev;
+	struct iommu *obj;
+
+	dev = driver_find_device(&omap_iommu_driver.driver, NULL, (void *)name,
+				 device_match_by_alias);
+	if (!dev)
+		return ERR_PTR(-ENODEV);
+
+	obj = to_iommu(dev);
+
+	mutex_lock(&obj->iommu_lock);
+
+	if (obj->refcount++ == 0) {
+		err = iommu_enable(obj);
+		if (err)
+			goto err_enable;
+		flush_iotlb_all(obj);
+	}
+
+	if (!try_module_get(obj->owner))
+		goto err_module;
+
+	mutex_unlock(&obj->iommu_lock);
+
+	dev_dbg(obj->dev, "%s: %s\n", __func__, obj->name);
+	return obj;
+
+err_module:
+	if (obj->refcount == 1)
+		iommu_disable(obj);
+err_enable:
+	obj->refcount--;
+	mutex_unlock(&obj->iommu_lock);
+	return ERR_PTR(err);
+}
+EXPORT_SYMBOL_GPL(iommu_get);
+
+/**
+ * iommu_put - Put back iommu handler
+ * @obj:	target iommu
+ **/
+void iommu_put(struct iommu *obj)
+{
+	if (!obj && IS_ERR(obj))
+		return;
+
+	mutex_lock(&obj->iommu_lock);
+
+	if (--obj->refcount == 0)
+		iommu_disable(obj);
+
+	module_put(obj->owner);
+
+	mutex_unlock(&obj->iommu_lock);
+
+	dev_dbg(obj->dev, "%s: %s\n", __func__, obj->name);
+}
+EXPORT_SYMBOL_GPL(iommu_put);
+
+/*
+ *	OMAP Device MMU(IOMMU) detection
+ */
+static int __devinit omap_iommu_probe(struct platform_device *pdev)
+{
+	int err = -ENODEV;
+	void *p;
+	int irq;
+	struct iommu *obj;
+	struct resource *res;
+	struct iommu_platform_data *pdata = pdev->dev.platform_data;
+
+	if (pdev->num_resources != 2)
+		return -EINVAL;
+
+	obj = kzalloc(sizeof(*obj) + MMU_REG_SIZE, GFP_KERNEL);
+	if (!obj)
+		return -ENOMEM;
+
+	obj->clk = clk_get(&pdev->dev, pdata->clk_name);
+	if (IS_ERR(obj->clk))
+		goto err_clk;
+
+	obj->nr_tlb_entries = pdata->nr_tlb_entries;
+	obj->name = pdata->name;
+	obj->dev = &pdev->dev;
+	obj->ctx = (void *)obj + sizeof(*obj);
+
+	mutex_init(&obj->iommu_lock);
+	mutex_init(&obj->mmap_lock);
+	spin_lock_init(&obj->page_table_lock);
+	INIT_LIST_HEAD(&obj->mmap);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		err = -ENODEV;
+		goto err_mem;
+	}
+	obj->regbase = ioremap(res->start, resource_size(res));
+	if (!obj->regbase) {
+		err = -ENOMEM;
+		goto err_mem;
+	}
+
+	res = request_mem_region(res->start, resource_size(res),
+				 dev_name(&pdev->dev));
+	if (!res) {
+		err = -EIO;
+		goto err_mem;
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		err = -ENODEV;
+		goto err_irq;
+	}
+	err = request_irq(irq, iommu_fault_handler, IRQF_SHARED,
+			  dev_name(&pdev->dev), obj);
+	if (err < 0)
+		goto err_irq;
+	platform_set_drvdata(pdev, obj);
+
+	p = (void *)__get_free_pages(GFP_KERNEL, get_order(IOPGD_TABLE_SIZE));
+	if (!p) {
+		err = -ENOMEM;
+		goto err_pgd;
+	}
+	memset(p, 0, IOPGD_TABLE_SIZE);
+	clean_dcache_area(p, IOPGD_TABLE_SIZE);
+	obj->iopgd = p;
+
+	BUG_ON(!IS_ALIGNED((unsigned long)obj->iopgd, IOPGD_TABLE_SIZE));
+
+	dev_info(&pdev->dev, "%s registered\n", obj->name);
+	return 0;
+
+err_pgd:
+	free_irq(irq, obj);
+err_irq:
+	release_mem_region(res->start, resource_size(res));
+	iounmap(obj->regbase);
+err_mem:
+	clk_put(obj->clk);
+err_clk:
+	kfree(obj);
+	return err;
+}
+
+static int __devexit omap_iommu_remove(struct platform_device *pdev)
+{
+	int irq;
+	struct resource *res;
+	struct iommu *obj = platform_get_drvdata(pdev);
+
+	platform_set_drvdata(pdev, NULL);
+
+	iopgtable_clear_entry_all(obj);
+	free_pages((unsigned long)obj->iopgd, get_order(IOPGD_TABLE_SIZE));
+
+	irq = platform_get_irq(pdev, 0);
+	free_irq(irq, obj);
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	release_mem_region(res->start, resource_size(res));
+	iounmap(obj->regbase);
+
+	clk_put(obj->clk);
+	dev_info(&pdev->dev, "%s removed\n", obj->name);
+	kfree(obj);
+	return 0;
+}
+
+static struct platform_driver omap_iommu_driver = {
+	.probe	= omap_iommu_probe,
+	.remove	= __devexit_p(omap_iommu_remove),
+	.driver	= {
+		.name	= "omap-iommu",
+	},
+};
+
+static void iopte_cachep_ctor(void *iopte)
+{
+	clean_dcache_area(iopte, IOPTE_TABLE_SIZE);
+}
+
+static int __init omap_iommu_init(void)
+{
+	struct kmem_cache *p;
+	const unsigned long flags = SLAB_HWCACHE_ALIGN;
+	size_t align = 1 << 10; /* L2 pagetable alignement */
+
+	p = kmem_cache_create("iopte_cache", IOPTE_TABLE_SIZE, align, flags,
+			      iopte_cachep_ctor);
+	if (!p)
+		return -ENOMEM;
+	iopte_cachep = p;
+
+	return platform_driver_register(&omap_iommu_driver);
+}
+module_init(omap_iommu_init);
+
+static void __exit omap_iommu_exit(void)
+{
+	kmem_cache_destroy(iopte_cachep);
+
+	platform_driver_unregister(&omap_iommu_driver);
+}
+module_exit(omap_iommu_exit);
+
+MODULE_DESCRIPTION("omap iommu: tlb and pagetable primitives");
+MODULE_ALIAS("platform:omap-iommu");
+MODULE_AUTHOR("Hiroshi DOYU, Paul Mundt and Toshihiro Kobayashi");
+MODULE_LICENSE("GPL v2");
diff --git a/arch/arm/plat-omap/iopgtable.h b/arch/arm/plat-omap/iopgtable.h
new file mode 100644
index 0000000..37dac43
--- /dev/null
+++ b/arch/arm/plat-omap/iopgtable.h
@@ -0,0 +1,72 @@
+/*
+ * omap iommu: pagetable definitions
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation
+ *
+ * Written by Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __PLAT_OMAP_IOMMU_H
+#define __PLAT_OMAP_IOMMU_H
+
+#define IOPGD_SHIFT		20
+#define IOPGD_SIZE		(1 << IOPGD_SHIFT)
+#define IOPGD_MASK		(~(IOPGD_SIZE - 1))
+#define IOSECTION_MASK		IOPGD_MASK
+#define PTRS_PER_IOPGD		(1 << (32 - IOPGD_SHIFT))
+#define IOPGD_TABLE_SIZE	(PTRS_PER_IOPGD * sizeof(u32))
+
+#define IOSUPER_SIZE		(IOPGD_SIZE << 4)
+#define IOSUPER_MASK		(~(IOSUPER_SIZE - 1))
+
+#define IOPTE_SHIFT		12
+#define IOPTE_SIZE		(1 << IOPTE_SHIFT)
+#define IOPTE_MASK		(~(IOPTE_SIZE - 1))
+#define IOPAGE_MASK		IOPTE_MASK
+#define PTRS_PER_IOPTE		(1 << (IOPGD_SHIFT - IOPTE_SHIFT))
+#define IOPTE_TABLE_SIZE	(PTRS_PER_IOPTE * sizeof(u32))
+
+#define IOLARGE_SIZE		(IOPTE_SIZE << 4)
+#define IOLARGE_MASK		(~(IOLARGE_SIZE - 1))
+
+#define IOPGD_TABLE		(1 << 0)
+#define IOPGD_SECTION		(2 << 0)
+#define IOPGD_SUPER		(1 << 18 | 2 << 0)
+
+#define IOPTE_SMALL		(2 << 0)
+#define IOPTE_LARGE		(1 << 0)
+
+#define iopgd_index(da)		(((da) >> IOPGD_SHIFT) & (PTRS_PER_IOPGD - 1))
+#define iopgd_offset(obj, da)	((obj)->iopgd + iopgd_index(da))
+
+#define iopte_paddr(iopgd)	(*iopgd & ~((1 << 10) - 1))
+#define iopte_vaddr(iopgd)	((u32 *)phys_to_virt(iopte_paddr(iopgd)))
+
+#define iopte_index(da)		(((da) >> IOPTE_SHIFT) & (PTRS_PER_IOPTE - 1))
+#define iopte_offset(iopgd, da)	(iopte_vaddr(iopgd) + iopte_index(da))
+
+static inline u32 iotlb_init_entry(struct iotlb_entry *e, u32 da, u32 pa,
+				   u32 flags)
+{
+	memset(e, 0, sizeof(*e));
+
+	e->da		= da;
+	e->pa		= pa;
+	e->valid	= 1;
+	/* FIXME: add OMAP1 support */
+	e->pgsz		= flags & MMU_CAM_PGSZ_MASK;
+	e->endian	= flags & MMU_RAM_ENDIAN_MASK;
+	e->elsz		= flags & MMU_RAM_ELSZ_MASK;
+	e->mixed	= flags & MMU_RAM_MIXED_MASK;
+
+	return iopgsz_to_bytes(e->pgsz);
+}
+
+#define to_iommu(dev)							\
+	(struct iommu *)platform_get_drvdata(to_platform_device(dev))
+
+#endif /* __PLAT_OMAP_IOMMU_H */


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

* [PATCH 2/6] omap iommu: omap2 architecture specific functions
  2009-05-05 12:46 [PATCH 0/6] Initial support for omap iommu driver Hiroshi DOYU
  2009-05-05 12:46 ` [PATCH 1/6] omap iommu: tlb and pagetable primitives Hiroshi DOYU
@ 2009-05-05 12:46 ` Hiroshi DOYU
  2009-05-06 14:31   ` Kanigeri, Hari
  2009-05-05 12:47 ` [PATCH 3/6] omap iommu: omap3 iommu device registration Hiroshi DOYU
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 53+ messages in thread
From: Hiroshi DOYU @ 2009-05-05 12:46 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-omap, h-kanigeri2, omar.ramirez, sakari.ailus, tony

The structure 'arch_mmu' accommodates the difference between omap1 and
omap2/3.

This patch provides omap2/3 specific functions

Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---

 arch/arm/mach-omap2/iommu2.c             |  323 ++++++++++++++++++++++++++++++
 arch/arm/plat-omap/include/mach/iommu2.h |   96 +++++++++
 2 files changed, 419 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-omap2/iommu2.c
 create mode 100644 arch/arm/plat-omap/include/mach/iommu2.h

diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
new file mode 100644
index 0000000..015f22a
--- /dev/null
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -0,0 +1,323 @@
+/*
+ * omap iommu: omap2/3 architecture specific functions
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation
+ *
+ * Written by Hiroshi DOYU <Hiroshi.DOYU@nokia.com>,
+ *		Paul Mundt and Toshihiro Kobayashi
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/err.h>
+#include <linux/device.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/stringify.h>
+
+#include <mach/iommu.h>
+
+/*
+ * omap2 architecture specific register bit definitions
+ */
+#define IOMMU_ARCH_VERSION	0x00000011
+
+/* SYSCONF */
+#define MMU_SYS_IDLE_SHIFT	3
+#define MMU_SYS_IDLE_FORCE	(0 << MMU_SYS_IDLE_SHIFT)
+#define MMU_SYS_IDLE_NONE	(1 << MMU_SYS_IDLE_SHIFT)
+#define MMU_SYS_IDLE_SMART	(2 << MMU_SYS_IDLE_SHIFT)
+#define MMU_SYS_IDLE_MASK	(3 << MMU_SYS_IDLE_SHIFT)
+
+#define MMU_SYS_SOFTRESET	(1 << 1)
+#define MMU_SYS_AUTOIDLE	1
+
+/* SYSSTATUS */
+#define MMU_SYS_RESETDONE	1
+
+/* IRQSTATUS & IRQENABLE */
+#define MMU_IRQ_MULTIHITFAULT	(1 << 4)
+#define MMU_IRQ_TABLEWALKFAULT	(1 << 3)
+#define MMU_IRQ_EMUMISS		(1 << 2)
+#define MMU_IRQ_TRANSLATIONFAULT	(1 << 1)
+#define MMU_IRQ_TLBMISS		(1 << 0)
+#define MMU_IRQ_MASK	\
+	(MMU_IRQ_MULTIHITFAULT | MMU_IRQ_TABLEWALKFAULT | MMU_IRQ_EMUMISS | \
+	 MMU_IRQ_TRANSLATIONFAULT)
+
+/* MMU_CNTL */
+#define MMU_CNTL_SHIFT		1
+#define MMU_CNTL_MASK		(7 << MMU_CNTL_SHIFT)
+#define MMU_CNTL_EML_TLB	(1 << 3)
+#define MMU_CNTL_TWL_EN		(1 << 2)
+#define MMU_CNTL_MMU_EN		(1 << 1)
+
+#define get_cam_va_mask(pgsz)				\
+	(((pgsz) == MMU_CAM_PGSZ_16M) ? 0xff000000 :	\
+	 ((pgsz) == MMU_CAM_PGSZ_1M)  ? 0xfff00000 :	\
+	 ((pgsz) == MMU_CAM_PGSZ_64K) ? 0xffff0000 :	\
+	 ((pgsz) == MMU_CAM_PGSZ_4K)  ? 0xfffff000 : 0)
+
+static int omap2_iommu_enable(struct iommu *obj)
+{
+	u32 l, pa;
+	unsigned long timeout;
+
+	if (!obj->iopgd || !IS_ALIGNED((u32)obj->iopgd,  SZ_16K))
+		return -EINVAL;
+
+	pa = virt_to_phys(obj->iopgd);
+	if (!IS_ALIGNED(pa, SZ_16K))
+		return -EINVAL;
+
+	iommu_write_reg(obj, MMU_SYS_SOFTRESET, MMU_SYSCONFIG);
+
+	timeout = jiffies + msecs_to_jiffies(20);
+	do {
+		l = iommu_read_reg(obj, MMU_SYSSTATUS);
+		if (l & MMU_SYS_RESETDONE)
+			break;
+	} while (time_after(jiffies, timeout));
+
+	if (!(l & MMU_SYS_RESETDONE)) {
+		dev_err(obj->dev, "can't take mmu out of reset\n");
+		return -ENODEV;
+	}
+
+	l = iommu_read_reg(obj, MMU_REVISION);
+	dev_info(obj->dev, "%s: version %d.%d\n", obj->name,
+		 (l >> 4) & 0xf, l & 0xf);
+
+	l = iommu_read_reg(obj, MMU_SYSCONFIG);
+	l &= ~MMU_SYS_IDLE_MASK;
+	l |= (MMU_SYS_IDLE_SMART | MMU_SYS_AUTOIDLE);
+	iommu_write_reg(obj, l, MMU_SYSCONFIG);
+
+	iommu_write_reg(obj, MMU_IRQ_MASK, MMU_IRQENABLE);
+	iommu_write_reg(obj, pa, MMU_TTB);
+
+	l = iommu_read_reg(obj, MMU_CNTL);
+	l &= ~MMU_CNTL_MASK;
+	l |= (MMU_CNTL_MMU_EN | MMU_CNTL_TWL_EN);
+	iommu_write_reg(obj, l, MMU_CNTL);
+
+	return 0;
+}
+
+static void omap2_iommu_disable(struct iommu *obj)
+{
+	u32 l = iommu_read_reg(obj, MMU_CNTL);
+
+	l &= ~MMU_CNTL_MASK;
+	iommu_write_reg(obj, l, MMU_CNTL);
+	iommu_write_reg(obj, MMU_SYS_IDLE_FORCE, MMU_SYSCONFIG);
+
+	dev_dbg(obj->dev, "%s is shutting down\n", obj->name);
+}
+
+static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra)
+{
+	int i;
+	u32 stat, da;
+	const char *err_msg[] =	{
+		"tlb miss",
+		"translation fault",
+		"emulation miss",
+		"table walk fault",
+		"multi hit fault",
+	};
+
+	stat = iommu_read_reg(obj, MMU_IRQSTATUS);
+	stat &= MMU_IRQ_MASK;
+	if (!stat)
+		return 0;
+
+	da = iommu_read_reg(obj, MMU_FAULT_AD);
+	*ra = da;
+
+	dev_err(obj->dev, "%s:\tda:%08x ", __func__, da);
+
+	for (i = 0; i < ARRAY_SIZE(err_msg); i++) {
+		if (stat & (1 << i))
+			printk("%s ", err_msg[i]);
+	}
+	printk("\n");
+
+	iommu_write_reg(obj, stat, MMU_IRQSTATUS);
+	return stat;
+}
+
+static void omap2_tlb_read_cr(struct iommu *obj, struct cr_regs *cr)
+{
+	cr->cam = iommu_read_reg(obj, MMU_READ_CAM);
+	cr->ram = iommu_read_reg(obj, MMU_READ_RAM);
+}
+
+static void omap2_tlb_load_cr(struct iommu *obj, struct cr_regs *cr)
+{
+	iommu_write_reg(obj, cr->cam | MMU_CAM_V, MMU_CAM);
+	iommu_write_reg(obj, cr->ram, MMU_RAM);
+}
+
+static u32 omap2_cr_to_virt(struct cr_regs *cr)
+{
+	u32 page_size = cr->cam & MMU_CAM_PGSZ_MASK;
+	u32 mask = get_cam_va_mask(cr->cam & page_size);
+
+	return cr->cam & mask;
+}
+
+static struct cr_regs *omap2_alloc_cr(struct iommu *obj, struct iotlb_entry *e)
+{
+	struct cr_regs *cr;
+
+	if (e->da & ~(get_cam_va_mask(e->pgsz))) {
+		dev_err(obj->dev, "%s:\twrong alignment: %08x\n", __func__,
+			e->da);
+		return ERR_PTR(-EINVAL);
+	}
+
+	cr = kmalloc(sizeof(*cr), GFP_KERNEL);
+	if (!cr)
+		return ERR_PTR(-ENOMEM);
+
+	cr->cam = (e->da & MMU_CAM_VATAG_MASK) | e->prsvd | e->pgsz;
+	cr->ram = e->pa | e->endian | e->elsz | e->mixed;
+
+	return cr;
+}
+
+static inline int omap2_cr_valid(struct cr_regs *cr)
+{
+	return cr->cam & MMU_CAM_V;
+}
+
+static u32 omap2_get_pte_attr(struct iotlb_entry *e)
+{
+	u32 attr;
+
+	attr = e->mixed << 5;
+	attr |= e->endian;
+	attr |= e->elsz >> 3;
+	attr <<= ((e->pgsz & MMU_CAM_PGSZ_4K) ? 0 : 6);
+
+	return attr;
+}
+
+static ssize_t omap2_dump_cr(struct iommu *obj, struct cr_regs *cr, char *buf)
+{
+	char *p = buf;
+
+	/* FIXME: Need more detail analysis of cam/ram */
+	p += sprintf(p, "%08x %08x\n", cr->cam, cr->ram);
+
+	return p - buf;
+}
+
+#define pr_reg(name)							\
+	p += sprintf(p, "%20s: %08x\n",					\
+		     __stringify(name), iommu_read_reg(obj, MMU_##name));
+
+static ssize_t omap2_iommu_dump_ctx(struct iommu *obj, char *buf)
+{
+	char *p = buf;
+
+	pr_reg(REVISION);
+	pr_reg(SYSCONFIG);
+	pr_reg(SYSSTATUS);
+	pr_reg(IRQSTATUS);
+	pr_reg(IRQENABLE);
+	pr_reg(WALKING_ST);
+	pr_reg(CNTL);
+	pr_reg(FAULT_AD);
+	pr_reg(TTB);
+	pr_reg(LOCK);
+	pr_reg(LD_TLB);
+	pr_reg(CAM);
+	pr_reg(RAM);
+	pr_reg(GFLUSH);
+	pr_reg(FLUSH_ENTRY);
+	pr_reg(READ_CAM);
+	pr_reg(READ_RAM);
+	pr_reg(EMU_FAULT_AD);
+
+	return p - buf;
+}
+
+static void omap2_iommu_save_ctx(struct iommu *obj)
+{
+	int i;
+	u32 *p = obj->ctx;
+
+	for (i = 0; i < (MMU_REG_SIZE / sizeof(u32)); i++) {
+		p[i] = iommu_read_reg(obj, i * sizeof(u32));
+		dev_dbg(obj->dev, "%s\t[%02d] %08x\n", __func__, i, p[i]);
+	}
+
+	BUG_ON(p[0] != IOMMU_ARCH_VERSION);
+}
+
+static void omap2_iommu_restore_ctx(struct iommu *obj)
+{
+	int i;
+	u32 *p = obj->ctx;
+
+	for (i = 0; i < (MMU_REG_SIZE / sizeof(u32)); i++) {
+		iommu_write_reg(obj, p[i], i * sizeof(u32));
+		dev_dbg(obj->dev, "%s\t[%02d] %08x\n", __func__, i, p[i]);
+	}
+
+	BUG_ON(p[0] != IOMMU_ARCH_VERSION);
+}
+
+static void omap2_cr_to_e(struct cr_regs *cr, struct iotlb_entry *e)
+{
+	e->da		= cr->cam & MMU_CAM_VATAG_MASK;
+	e->pa		= cr->ram & MMU_RAM_PADDR_MASK;
+	e->valid	= cr->cam & MMU_CAM_V;
+	e->pgsz		= cr->cam & MMU_CAM_PGSZ_MASK;
+	e->endian	= cr->ram & MMU_RAM_ENDIAN_MASK;
+	e->elsz		= cr->ram & MMU_RAM_ELSZ_MASK;
+	e->mixed	= cr->ram & MMU_RAM_MIXED;
+}
+
+static const struct iommu_functions omap2_iommu_ops = {
+	.version	= IOMMU_ARCH_VERSION,
+
+	.enable		= omap2_iommu_enable,
+	.disable	= omap2_iommu_disable,
+	.fault_isr	= omap2_iommu_fault_isr,
+
+	.tlb_read_cr	= omap2_tlb_read_cr,
+	.tlb_load_cr	= omap2_tlb_load_cr,
+
+	.cr_to_e	= omap2_cr_to_e,
+	.cr_to_virt	= omap2_cr_to_virt,
+	.alloc_cr	= omap2_alloc_cr,
+	.cr_valid	= omap2_cr_valid,
+	.dump_cr	= omap2_dump_cr,
+
+	.get_pte_attr	= omap2_get_pte_attr,
+
+	.save_ctx	= omap2_iommu_save_ctx,
+	.restore_ctx	= omap2_iommu_restore_ctx,
+	.dump_ctx	= omap2_iommu_dump_ctx,
+};
+
+static int __init omap2_iommu_init(void)
+{
+	return install_iommu_arch(&omap2_iommu_ops);
+}
+module_init(omap2_iommu_init);
+
+static void __exit omap2_iommu_exit(void)
+{
+	uninstall_iommu_arch(&omap2_iommu_ops);
+}
+module_exit(omap2_iommu_exit);
+
+MODULE_AUTHOR("Hiroshi DOYU, Paul Mundt and Toshihiro Kobayashi");
+MODULE_DESCRIPTION("omap iommu: omap2/3 architecture specific functions");
+MODULE_LICENSE("GPL v2");
diff --git a/arch/arm/plat-omap/include/mach/iommu2.h b/arch/arm/plat-omap/include/mach/iommu2.h
new file mode 100644
index 0000000..10ad05f
--- /dev/null
+++ b/arch/arm/plat-omap/include/mach/iommu2.h
@@ -0,0 +1,96 @@
+/*
+ * omap iommu: omap2 architecture specific definitions
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation
+ *
+ * Written by Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __MACH_IOMMU2_H
+#define __MACH_IOMMU2_H
+
+#include <linux/io.h>
+
+/*
+ * MMU Register offsets
+ */
+#define MMU_REVISION		0x00
+#define MMU_SYSCONFIG		0x10
+#define MMU_SYSSTATUS		0x14
+#define MMU_IRQSTATUS		0x18
+#define MMU_IRQENABLE		0x1c
+#define MMU_WALKING_ST		0x40
+#define MMU_CNTL		0x44
+#define MMU_FAULT_AD		0x48
+#define MMU_TTB			0x4c
+#define MMU_LOCK		0x50
+#define MMU_LD_TLB		0x54
+#define MMU_CAM			0x58
+#define MMU_RAM			0x5c
+#define MMU_GFLUSH		0x60
+#define MMU_FLUSH_ENTRY		0x64
+#define MMU_READ_CAM		0x68
+#define MMU_READ_RAM		0x6c
+#define MMU_EMU_FAULT_AD	0x70
+
+#define MMU_REG_SIZE		256
+
+/*
+ * MMU Register bit definitions
+ */
+#define MMU_LOCK_BASE_SHIFT	10
+#define MMU_LOCK_BASE_MASK	(0x1f << MMU_LOCK_BASE_SHIFT)
+#define MMU_LOCK_BASE(x)	\
+	((x & MMU_LOCK_BASE_MASK) >> MMU_LOCK_BASE_SHIFT)
+
+#define MMU_LOCK_VICT_SHIFT	4
+#define MMU_LOCK_VICT_MASK	(0x1f << MMU_LOCK_VICT_SHIFT)
+#define MMU_LOCK_VICT(x)	\
+	((x & MMU_LOCK_VICT_MASK) >> MMU_LOCK_VICT_SHIFT)
+
+#define MMU_CAM_VATAG_SHIFT	12
+#define MMU_CAM_VATAG_MASK \
+	((~0UL >> MMU_CAM_VATAG_SHIFT) << MMU_CAM_VATAG_SHIFT)
+#define MMU_CAM_P		(1 << 3)
+#define MMU_CAM_V		(1 << 2)
+#define MMU_CAM_PGSZ_MASK	3
+#define MMU_CAM_PGSZ_1M		(0 << 0)
+#define MMU_CAM_PGSZ_64K	(1 << 0)
+#define MMU_CAM_PGSZ_4K		(2 << 0)
+#define MMU_CAM_PGSZ_16M	(3 << 0)
+
+#define MMU_RAM_PADDR_SHIFT	12
+#define MMU_RAM_PADDR_MASK \
+	((~0UL >> MMU_RAM_PADDR_SHIFT) << MMU_RAM_PADDR_SHIFT)
+#define MMU_RAM_ENDIAN_SHIFT	9
+#define MMU_RAM_ENDIAN_MASK	(1 << MMU_RAM_ENDIAN_SHIFT)
+#define MMU_RAM_ENDIAN_BIG	(1 << MMU_RAM_ENDIAN_SHIFT)
+#define MMU_RAM_ENDIAN_LITTLE	(0 << MMU_RAM_ENDIAN_SHIFT)
+#define MMU_RAM_ELSZ_SHIFT	7
+#define MMU_RAM_ELSZ_MASK	(3 << MMU_RAM_ELSZ_SHIFT)
+#define MMU_RAM_ELSZ_8		(0 << MMU_RAM_ELSZ_SHIFT)
+#define MMU_RAM_ELSZ_16		(1 << MMU_RAM_ELSZ_SHIFT)
+#define MMU_RAM_ELSZ_32		(2 << MMU_RAM_ELSZ_SHIFT)
+#define MMU_RAM_ELSZ_NONE	(3 << MMU_RAM_ELSZ_SHIFT)
+#define MMU_RAM_MIXED_SHIFT	6
+#define MMU_RAM_MIXED_MASK	(1 << MMU_RAM_MIXED_SHIFT)
+#define MMU_RAM_MIXED		MMU_RAM_MIXED_MASK
+
+/*
+ * register accessors
+ */
+static inline u32 iommu_read_reg(struct iommu *obj, size_t offs)
+{
+	return __raw_readl(obj->regbase + offs);
+}
+
+static inline void iommu_write_reg(struct iommu *obj, u32 val, size_t offs)
+{
+	__raw_writel(val, obj->regbase + offs);
+}
+
+#endif /* __MACH_IOMMU2_H */


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

* [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-05-05 12:46 [PATCH 0/6] Initial support for omap iommu driver Hiroshi DOYU
  2009-05-05 12:46 ` [PATCH 1/6] omap iommu: tlb and pagetable primitives Hiroshi DOYU
  2009-05-05 12:46 ` [PATCH 2/6] omap iommu: omap2 architecture specific functions Hiroshi DOYU
@ 2009-05-05 12:47 ` Hiroshi DOYU
  2009-05-05 19:32   ` Felipe Contreras
                     ` (2 more replies)
  2009-05-05 12:47 ` [PATCH 4/6] omap iommu: simple virtual address space management Hiroshi DOYU
                   ` (2 subsequent siblings)
  5 siblings, 3 replies; 53+ messages in thread
From: Hiroshi DOYU @ 2009-05-05 12:47 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-omap, h-kanigeri2, omar.ramirez, sakari.ailus, tony

Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---

 arch/arm/mach-omap2/omap3-iommu.c |  105 +++++++++++++++++++++++++++++++++++++
 1 files changed, 105 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-omap2/omap3-iommu.c

diff --git a/arch/arm/mach-omap2/omap3-iommu.c b/arch/arm/mach-omap2/omap3-iommu.c
new file mode 100644
index 0000000..367f36a
--- /dev/null
+++ b/arch/arm/mach-omap2/omap3-iommu.c
@@ -0,0 +1,105 @@
+/*
+ * omap iommu: omap3 device registration
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation
+ *
+ * Written by Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/platform_device.h>
+
+#include <mach/iommu.h>
+
+#define OMAP3_MMU1_BASE	0x480bd400
+#define OMAP3_MMU2_BASE	0x5d000000
+#define OMAP3_MMU1_IRQ	24
+#define OMAP3_MMU2_IRQ	28
+
+static struct resource omap3_iommu_res[] = {
+	{ /* Camera ISP MMU */
+		.start		= OMAP3_MMU1_BASE,
+		.end		= OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
+		.flags		= IORESOURCE_MEM,
+	},
+	{
+		.start		= OMAP3_MMU1_IRQ,
+		.flags		= IORESOURCE_IRQ,
+	},
+	{ /* IVA2.2 MMU */
+		.start		= OMAP3_MMU2_BASE,
+		.end		= OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
+		.flags		= IORESOURCE_MEM,
+	},
+	{
+		.start		= OMAP3_MMU2_IRQ,
+		.flags		= IORESOURCE_IRQ,
+	},
+};
+#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
+
+static const struct iommu_platform_data omap3_iommu_pdata[] __initconst = {
+	{
+		.name = "isp",
+		.nr_tlb_entries = 8,
+		.clk_name = "cam_ick",
+	},
+	{
+		.name = "iva2",
+		.nr_tlb_entries = 32,
+		.clk_name = "iva2_ck",
+	},
+};
+#define NR_IOMMU_DEVICES ARRAY_SIZE(omap3_iommu_pdata)
+
+static struct platform_device *omap3_iommu_pdev[NR_IOMMU_DEVICES];
+
+static int __init omap3_iommu_init(void)
+{
+	int i, err;
+
+	for (i = 0; i < NR_IOMMU_DEVICES; i++) {
+		struct platform_device *pdev;
+
+		pdev = platform_device_alloc("omap-iommu", i + 1);
+		if (!pdev) {
+			err = -ENOMEM;
+			goto err_out;
+		}
+		err = platform_device_add_resources(pdev,
+				    &omap3_iommu_res[2 * i], NR_IOMMU_RES);
+		if (err)
+			goto err_out;
+		err = platform_device_add_data(pdev, &omap3_iommu_pdata[i],
+					       sizeof(omap3_iommu_pdata[0]));
+		if (err)
+			goto err_out;
+		err = platform_device_add(pdev);
+		if (err)
+			goto err_out;
+		omap3_iommu_pdev[i] = pdev;
+	}
+	return 0;
+
+err_out:
+	while (i--)
+		platform_device_put(omap3_iommu_pdev[i]);
+	return err;
+}
+module_init(omap3_iommu_init);
+
+static void __exit omap3_iommu_exit(void)
+{
+	int i;
+
+	for (i = 0; i < NR_IOMMU_DEVICES; i++)
+		platform_device_unregister(omap3_iommu_pdev[i]);
+}
+module_exit(omap3_iommu_exit);
+
+MODULE_AUTHOR("Hiroshi DOYU");
+MODULE_DESCRIPTION("omap iommu: omap3 device registration");
+MODULE_LICENSE("GPL v2");


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

* [PATCH 4/6] omap iommu: simple virtual address space management
  2009-05-05 12:46 [PATCH 0/6] Initial support for omap iommu driver Hiroshi DOYU
                   ` (2 preceding siblings ...)
  2009-05-05 12:47 ` [PATCH 3/6] omap iommu: omap3 iommu device registration Hiroshi DOYU
@ 2009-05-05 12:47 ` Hiroshi DOYU
  2009-05-16  9:22   ` Russell King - ARM Linux
  2009-05-05 12:47 ` [PATCH 5/6] omap iommu: entries for Kconfig and Makefile Hiroshi DOYU
  2009-05-05 12:47 ` [PATCH 6/6] omap2 " Hiroshi DOYU
  5 siblings, 1 reply; 53+ messages in thread
From: Hiroshi DOYU @ 2009-05-05 12:47 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-omap, h-kanigeri2, omar.ramirez, sakari.ailus, tony

This patch provides a device drivers, which has a omap iommu, with
address mapping APIs between device virtual address(iommu), physical
address and MPU virtual address.

There are 4 possible patterns for iommu virtual address(iova/da) mapping.

    |iova/			  mapping		iommu_		page
    | da	pa	va	(d)-(p)-(v)		function	type
  ---------------------------------------------------------------------------
  1 | c		c	c	 1 - 1 - 1	  _kmap() / _kunmap()	s
  2 | c		c,a	c	 1 - 1 - 1	_kmalloc()/ _kfree()	s
  3 | c		d	c	 1 - n - 1	  _vmap() / _vunmap()	s
  4 | c		d,a	c	 1 - n - 1	_vmalloc()/ _vfree()	n*

    'iova':	device iommu virtual address
    'da':	alias of 'iova'
    'pa':	physical address
    'va':	mpu virtual address

    'c':	contiguous memory area
    'd':	dicontiguous memory area
    'a':	anonymous memory allocation
    '()':	optional feature

    'n':	a normal page(4KB) size is used.
    's':	multiple iommu superpage(16MB, 1MB, 64KB, 4KB) size is used.

    '*':	not yet, but feasible.

Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---

 arch/arm/include/asm/io.h               |    6 
 arch/arm/mm/ioremap.c                   |   11 
 arch/arm/plat-omap/include/mach/iovmm.h |   94 +++
 arch/arm/plat-omap/iovmm.c              |  890 +++++++++++++++++++++++++++++++
 4 files changed, 1001 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/plat-omap/include/mach/iovmm.h
 create mode 100644 arch/arm/plat-omap/iovmm.c

diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index d2a59cf..cbdadfe 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -75,6 +75,12 @@ extern void __iomem * __arm_ioremap(unsigned long, size_t, unsigned int);
 extern void __iounmap(volatile void __iomem *addr);
 
 /*
+ * external interface to remap single page with appropriate type
+ */
+extern int ioremap_page(unsigned long virt, unsigned long phys,
+			unsigned int mtype);
+
+/*
  * Bad read/write accesses...
  */
 extern void __readwrite_bug(const char *fn);
diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index 9f88dd3..8441351 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -110,6 +110,17 @@ static int remap_area_pages(unsigned long start, unsigned long pfn,
 	return err;
 }
 
+int ioremap_page(unsigned long virt, unsigned long phys, unsigned int mtype)
+{
+	const struct mem_type *type;
+
+	type = get_mem_type(mtype);
+	if (!type)
+		return -EINVAL;
+
+	return remap_area_pages(virt, __phys_to_pfn(phys), PAGE_SIZE, type);
+}
+EXPORT_SYMBOL(ioremap_page);
 
 void __check_kvm_seq(struct mm_struct *mm)
 {
diff --git a/arch/arm/plat-omap/include/mach/iovmm.h b/arch/arm/plat-omap/include/mach/iovmm.h
new file mode 100644
index 0000000..bdc7ce5
--- /dev/null
+++ b/arch/arm/plat-omap/include/mach/iovmm.h
@@ -0,0 +1,94 @@
+/*
+ * omap iommu: simple virtual address space management
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation
+ *
+ * Written by Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __IOMMU_MMAP_H
+#define __IOMMU_MMAP_H
+
+struct iovm_struct {
+	struct iommu		*iommu;	/* iommu object which this belongs to */
+	u32			da_start; /* area definition */
+	u32			da_end;
+	u32			flags; /* IOVMF_: see below */
+	struct list_head	list; /* linked in ascending order */
+	const struct sg_table	*sgt; /* keep 'page' <-> 'da' mapping */
+	void			*va; /* mpu side mapped address */
+};
+
+/*
+ * IOVMF_FLAGS: attribute for iommu virtual memory area(iovma)
+ *
+ * lower 16 bit is used for h/w and upper 16 bit is for s/w.
+ */
+#define IOVMF_SW_SHIFT		16
+#define IOVMF_HW_SIZE		(1 << IOVMF_SW_SHIFT)
+#define IOVMF_HW_MASK		(IOVMF_HW_SIZE - 1)
+#define IOVMF_SW_MASK		(~IOVMF_HW_MASK)UL
+
+/*
+ * iovma: h/w flags derived from cam and ram attribute
+ */
+#define IOVMF_CAM_MASK		(~((1 << 10) - 1))
+#define IOVMF_RAM_MASK		(~IOVMF_CAM_MASK)
+
+#define IOVMF_PGSZ_MASK		(3 << 0)
+#define IOVMF_PGSZ_1M		MMU_CAM_PGSZ_1M
+#define IOVMF_PGSZ_64K		MMU_CAM_PGSZ_64K
+#define IOVMF_PGSZ_4K		MMU_CAM_PGSZ_4K
+#define IOVMF_PGSZ_16M		MMU_CAM_PGSZ_16M
+
+#define IOVMF_ENDIAN_MASK	(1 << 9)
+#define IOVMF_ENDIAN_BIG	MMU_RAM_ENDIAN_BIG
+#define IOVMF_ENDIAN_LITTLE	MMU_RAM_ENDIAN_LITTLE
+
+#define IOVMF_ELSZ_MASK		(3 << 7)
+#define IOVMF_ELSZ_8		MMU_RAM_ELSZ_8
+#define IOVMF_ELSZ_16		MMU_RAM_ELSZ_16
+#define IOVMF_ELSZ_32		MMU_RAM_ELSZ_32
+#define IOVMF_ELSZ_NONE		MMU_RAM_ELSZ_NONE
+
+#define IOVMF_MIXED_MASK	(1 << 6)
+#define IOVMF_MIXED		MMU_RAM_MIXED
+
+/*
+ * iovma: s/w flags, used for mapping and umapping internally.
+ */
+#define IOVMF_MMIO		(1 << IOVMF_SW_SHIFT)
+#define IOVMF_ALLOC		(2 << IOVMF_SW_SHIFT)
+#define IOVMF_ALLOC_MASK	(3 << IOVMF_SW_SHIFT)
+
+/* "superpages" is supported just with physically linear pages */
+#define IOVMF_DISCONT		(1 << (2 + IOVMF_SW_SHIFT))
+#define IOVMF_LINEAR		(2 << (2 + IOVMF_SW_SHIFT))
+#define IOVMF_LINEAR_MASK	(3 << (2 + IOVMF_SW_SHIFT))
+
+#define IOVMF_DA_FIXED		(1 << (4 + IOVMF_SW_SHIFT))
+#define IOVMF_DA_ANON		(2 << (4 + IOVMF_SW_SHIFT))
+#define IOVMF_DA_MASK		(3 << (4 + IOVMF_SW_SHIFT))
+
+
+extern struct iovm_struct *find_iovm_area(struct iommu *obj, u32 da);
+extern u32 iommu_vmap(struct iommu *obj, u32 da,
+			const struct sg_table *sgt, u32 flags);
+extern struct sg_table *iommu_vunmap(struct iommu *obj, u32 da);
+extern u32 iommu_vmalloc(struct iommu *obj, u32 da, size_t bytes,
+			   u32 flags);
+extern void iommu_vfree(struct iommu *obj, const u32 da);
+extern u32 iommu_kmap(struct iommu *obj, u32 da, u32 pa, size_t bytes,
+			u32 flags);
+extern void iommu_kunmap(struct iommu *obj, u32 da);
+extern u32 iommu_kmalloc(struct iommu *obj, u32 da, size_t bytes,
+			   u32 flags);
+extern void iommu_kfree(struct iommu *obj, u32 da);
+
+extern void *da_to_va(struct iommu *obj, u32 da);
+
+#endif /* __IOMMU_MMAP_H */
diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c
new file mode 100644
index 0000000..5ce7965
--- /dev/null
+++ b/arch/arm/plat-omap/iovmm.c
@@ -0,0 +1,890 @@
+/*
+ * omap iommu: simple virtual address space management
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation
+ *
+ * Written by Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/err.h>
+#include <linux/vmalloc.h>
+#include <linux/device.h>
+#include <linux/scatterlist.h>
+
+#include <asm/cacheflush.h>
+
+#include <mach/iommu.h>
+#include <mach/iovmm.h>
+
+#include "iopgtable.h"
+
+/*
+ * A device driver needs to create address mappings between:
+ *
+ * - iommu/device address
+ * - physical address
+ * - mpu virtual address
+ *
+ * There are 4 possible patterns for them:
+ *
+ *    |iova/			  mapping		iommu_		page
+ *    | da	pa	va	(d)-(p)-(v)		function	type
+ *  ---------------------------------------------------------------------------
+ *  1 | c	c	c	 1 - 1 - 1	  _kmap() / _kunmap()	s
+ *  2 | c	c,a	c	 1 - 1 - 1	_kmalloc()/ _kfree()	s
+ *  3 | c	d	c	 1 - n - 1	  _vmap() / _vunmap()	s
+ *  4 | c	d,a	c	 1 - n - 1	_vmalloc()/ _vfree()	n*
+ *
+ *
+ *	'iova':	device iommu virtual address
+ *	'da':	alias of 'iova'
+ *	'pa':	physical address
+ *	'va':	mpu virtual address
+ *
+ *	'c':	contiguous memory area
+ *	'd':	dicontiguous memory area
+ *	'a':	anonymous memory allocation
+ *	'()':	optional feature
+ *
+ *	'n':	a normal page(4KB) size is used.
+ *	's':	multiple iommu superpage(16MB, 1MB, 64KB, 4KB) size is used.
+ *
+ *	'*':	not yet, but feasible.
+ */
+
+static struct kmem_cache *iovm_area_cachep;
+
+/* return total bytes of sg buffers */
+static size_t sgtable_len(const struct sg_table *sgt)
+{
+	unsigned int i, total = 0;
+	struct scatterlist *sg;
+
+	if (!sgt)
+		return 0;
+
+	for_each_sg(sgt->sgl, sg, sgt->nents, i) {
+		size_t bytes;
+
+		bytes = sg_dma_len(sg);
+
+		if (!iopgsz_ok(bytes)) {
+			pr_err("%s: sg[%d] not iommu pagesize(%x)\n",
+			       __func__, i, bytes);
+			return 0;
+		}
+
+		total += bytes;
+	}
+
+	return total;
+}
+#define sgtable_ok(x)	(!!sgtable_len(x))
+
+/*
+ * calculate the optimal number sg elements from total bytes based on
+ * iommu superpages
+ */
+static unsigned int sgtable_nents(size_t bytes)
+{
+	int i;
+	unsigned int nr_entries;
+	const unsigned long pagesize[] = { SZ_16M, SZ_1M, SZ_64K, SZ_4K, };
+
+	if (!IS_ALIGNED(bytes, PAGE_SIZE)) {
+		pr_err("%s: wrong size %08x\n", __func__, bytes);
+		return 0;
+	}
+
+	nr_entries = 0;
+	for (i = 0; i < ARRAY_SIZE(pagesize); i++) {
+		if (bytes >= pagesize[i]) {
+			nr_entries += (bytes / pagesize[i]);
+			bytes %= pagesize[i];
+		}
+	}
+	BUG_ON(bytes);
+
+	return nr_entries;
+}
+
+/* allocate and initialize sg_table header(a kind of 'superblock') */
+static struct sg_table *sgtable_alloc(const size_t bytes, u32 flags)
+{
+	unsigned int nr_entries;
+	int err;
+	struct sg_table *sgt;
+
+	if (!bytes)
+		return ERR_PTR(-EINVAL);
+
+	if (!IS_ALIGNED(bytes, PAGE_SIZE))
+		return ERR_PTR(-EINVAL);
+
+	/* FIXME: IOVMF_DA_FIXED should support 'superpages' */
+	if ((flags & IOVMF_LINEAR) && (flags & IOVMF_DA_ANON)) {
+		nr_entries = sgtable_nents(bytes);
+		if (!nr_entries)
+			return ERR_PTR(-EINVAL);
+	} else
+		nr_entries =  bytes / PAGE_SIZE;
+
+	sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
+	if (!sgt)
+		return ERR_PTR(-ENOMEM);
+
+	err = sg_alloc_table(sgt, nr_entries, GFP_KERNEL);
+	if (err)
+		return ERR_PTR(err);
+
+	pr_debug("%s: sgt:%p(%d entries)\n", __func__, sgt, nr_entries);
+
+	return sgt;
+}
+
+/* free sg_table header(a kind of superblock) */
+static void sgtable_free(struct sg_table *sgt)
+{
+	if (!sgt)
+		return;
+
+	sg_free_table(sgt);
+	kfree(sgt);
+
+	pr_debug("%s: sgt:%p\n", __func__, sgt);
+}
+
+/* map 'sglist' to a contiguous mpu virtual area and return 'va' */
+static void *vmap_sg(const struct sg_table *sgt)
+{
+	u32 va;
+	size_t total;
+	unsigned int i;
+	struct scatterlist *sg;
+	struct vm_struct *new;
+
+	total = sgtable_len(sgt);
+	if (!total)
+		return ERR_PTR(-EINVAL);
+
+	new = __get_vm_area(total, VM_IOREMAP, VMALLOC_START, VMALLOC_END);
+	if (!new)
+		return ERR_PTR(-ENOMEM);
+	va = (u32)new->addr;
+
+	for_each_sg(sgt->sgl, sg, sgt->nents, i) {
+		size_t bytes;
+		u32 pa;
+		int err;
+
+		pa = sg_phys(sg);
+		bytes = sg_dma_len(sg);
+
+		BUG_ON(bytes != PAGE_SIZE);
+
+		err = ioremap_page(va,  pa, MT_DEVICE);
+		if (err)
+			goto err_out;
+
+		va += bytes;
+	}
+
+	flush_cache_vmap(new->addr, total);
+	return new->addr;
+
+err_out:
+	WARN_ON(1); /* FIXME: cleanup some mpu mappings */
+	vunmap(new->addr);
+	return ERR_PTR(-EAGAIN);
+}
+
+static inline void vunmap_sg(const void *va)
+{
+	vunmap(va);
+}
+
+static struct iovm_struct *__find_iovm_area(struct iommu *obj, const u32 da)
+{
+	struct iovm_struct *tmp;
+
+	list_for_each_entry(tmp, &obj->mmap, list) {
+		if ((da >= tmp->da_start) && (da < tmp->da_end)) {
+			size_t len;
+
+			len = tmp->da_end - tmp->da_start;
+
+			dev_dbg(obj->dev, "%s: %08x-%08x-%08x(%x) %08x\n",
+				__func__, tmp->da_start, da, tmp->da_end, len,
+				tmp->flags);
+
+			return tmp;
+		}
+	}
+
+	return NULL;
+}
+
+/**
+ * find_iovm_area  -  find iovma which includes @da
+ * @da:		iommu device virtual address
+ *
+ * Find the existing iovma starting at @da
+ */
+struct iovm_struct *find_iovm_area(struct iommu *obj, u32 da)
+{
+	struct iovm_struct *area;
+
+	mutex_lock(&obj->mmap_lock);
+	area = __find_iovm_area(obj, da);
+	mutex_unlock(&obj->mmap_lock);
+
+	return area;
+}
+EXPORT_SYMBOL_GPL(find_iovm_area);
+
+/*
+ * This finds the hole(area) which fits the requested address and len
+ * in iovmas mmap, and returns the new allocated iovma.
+ */
+static struct iovm_struct *alloc_iovm_area(struct iommu *obj, u32 da,
+					   size_t bytes, u32 flags)
+{
+	struct iovm_struct *new, *tmp;
+	u32 start, prev_end, alignement;
+
+	if (!obj || !bytes)
+		return ERR_PTR(-EINVAL);
+
+	start = da;
+	alignement = PAGE_SIZE;
+
+	if (flags & IOVMF_DA_ANON) {
+		/*
+		 * Reserve the first page for NULL
+		 */
+		start = PAGE_SIZE;
+		if (flags & IOVMF_LINEAR)
+			alignement = iopgsz_max(bytes);
+		start = roundup(start, alignement);
+	}
+
+	tmp = NULL;
+	if (list_empty(&obj->mmap))
+		goto found;
+
+	prev_end = 0;
+	list_for_each_entry(tmp, &obj->mmap, list) {
+
+		if ((prev_end <= start) && (start + bytes < tmp->da_start))
+			goto found;
+
+		if (flags & IOVMF_DA_ANON)
+			start = roundup(tmp->da_end, alignement);
+
+		prev_end = tmp->da_end;
+	}
+
+	if ((start >= prev_end) && (ULONG_MAX - start >= bytes))
+		goto found;
+
+	dev_dbg(obj->dev, "%s: no space to fit %08x(%x) flags: %08x\n",
+		__func__, da, bytes, flags);
+
+	return ERR_PTR(-EINVAL);
+
+found:
+	new = kmem_cache_zalloc(iovm_area_cachep, GFP_KERNEL);
+	if (!new)
+		return ERR_PTR(-ENOMEM);
+
+	new->iommu = obj;
+	new->da_start = start;
+	new->da_end = start + bytes;
+	new->flags = flags;
+
+	/*
+	 * keep ascending order of iovmas
+	 */
+	if (tmp)
+		list_add_tail(&new->list, &tmp->list);
+	else
+		list_add(&new->list, &obj->mmap);
+
+	dev_dbg(obj->dev, "%s: found %08x-%08x-%08x(%x) %08x\n",
+		__func__, new->da_start, start, new->da_end, bytes, flags);
+
+	return new;
+}
+
+static void free_iovm_area(struct iommu *obj, struct iovm_struct *area)
+{
+	size_t bytes;
+
+	BUG_ON(!obj || !area);
+
+	bytes = area->da_end - area->da_start;
+
+	dev_dbg(obj->dev, "%s: %08x-%08x(%x) %08x\n",
+		__func__, area->da_start, area->da_end, bytes, area->flags);
+
+	list_del(&area->list);
+	kmem_cache_free(iovm_area_cachep, area);
+}
+
+/**
+ * da_to_va - convert (d) to (v)
+ * @obj:	objective iommu
+ * @da:		iommu device virtual address
+ * @va:		mpu virtual address
+ *
+ * Returns mpu virtual addr which corresponds to a given device virtual addr
+ */
+void *da_to_va(struct iommu *obj, u32 da)
+{
+	void *va = NULL;
+	struct iovm_struct *area;
+
+	mutex_lock(&obj->mmap_lock);
+
+	area = __find_iovm_area(obj, da);
+	if (!area) {
+		dev_dbg(obj->dev, "%s: no da area(%08x)\n", __func__, da);
+		goto out;
+	}
+	va = area->va;
+	mutex_unlock(&obj->mmap_lock);
+out:
+	return va;
+}
+EXPORT_SYMBOL_GPL(da_to_va);
+
+static void sgtable_fill_vmalloc(struct sg_table *sgt, void *_va)
+{
+	unsigned int i;
+	struct scatterlist *sg;
+	void *va = _va;
+	void *va_end;
+
+	for_each_sg(sgt->sgl, sg, sgt->nents, i) {
+		struct page *pg;
+		const size_t bytes = PAGE_SIZE;
+
+		/*
+		 * iommu 'superpage' isn't supported with 'iommu_vmalloc()'
+		 */
+		pg = vmalloc_to_page(va);
+		BUG_ON(!pg);
+		sg_set_page(sg, pg, bytes, 0);
+
+		va += bytes;
+	}
+
+	va_end = _va + PAGE_SIZE * i;
+	flush_cache_vmap(_va, va_end);
+}
+
+static inline void sgtable_drain_vmalloc(struct sg_table *sgt)
+{
+	/*
+	 * Actually this is not necessary at all, just exists for
+	 * consistency of the code readibility.
+	 */
+	BUG_ON(!sgt);
+}
+
+static void sgtable_fill_kmalloc(struct sg_table *sgt, u32 pa, size_t len)
+{
+	unsigned int i;
+	struct scatterlist *sg;
+	void *va;
+
+	va = phys_to_virt(pa);
+
+	for_each_sg(sgt->sgl, sg, sgt->nents, i) {
+		size_t bytes;
+
+		bytes = iopgsz_max(len);
+
+		BUG_ON(!iopgsz_ok(bytes));
+
+		sg_set_buf(sg, phys_to_virt(pa), bytes);
+		/*
+		 * 'pa' is cotinuous(linear).
+		 */
+		pa += bytes;
+		len -= bytes;
+	}
+	BUG_ON(len);
+
+	clean_dcache_area(va, len);
+}
+
+static inline void sgtable_drain_kmalloc(struct sg_table *sgt)
+{
+	/*
+	 * Actually this is not necessary at all, just exists for
+	 * consistency of the code readibility
+	 */
+	BUG_ON(!sgt);
+}
+
+/* create 'da' <-> 'pa' mapping from 'sgt' */
+static int map_iovm_area(struct iommu *obj, struct iovm_struct *new,
+			 const struct sg_table *sgt, u32 flags)
+{
+	int err;
+	unsigned int i, j;
+	struct scatterlist *sg;
+	u32 da = new->da_start;
+
+	if (!obj || !new || !sgt)
+		return -EINVAL;
+
+	BUG_ON(!sgtable_ok(sgt));
+
+	for_each_sg(sgt->sgl, sg, sgt->nents, i) {
+		u32 pa;
+		int pgsz;
+		size_t bytes;
+		struct iotlb_entry e;
+
+		pa = sg_phys(sg);
+		bytes = sg_dma_len(sg);
+
+		flags &= ~IOVMF_PGSZ_MASK;
+		pgsz = bytes_to_iopgsz(bytes);
+		if (pgsz < 0)
+			goto err_out;
+		flags |= pgsz;
+
+		pr_debug("%s: [%d] %08x %08x(%x)\n", __func__,
+			 i, da, pa, bytes);
+
+		iotlb_init_entry(&e, da, pa, flags);
+		err = iopgtable_store_entry(obj, &e);
+		if (err)
+			goto err_out;
+
+		da += bytes;
+	}
+	return 0;
+
+err_out:
+	da = new->da_start;
+
+	for_each_sg(sgt->sgl, sg, i, j) {
+		size_t bytes;
+
+		bytes = iopgtable_clear_entry(obj, da);
+
+		BUG_ON(!iopgsz_ok(bytes));
+
+		da += bytes;
+	}
+	return err;
+}
+
+/* release 'da' <-> 'pa' mapping */
+static void unmap_iovm_area(struct iommu *obj, struct iovm_struct *area)
+{
+	u32 start;
+	size_t total = area->da_end - area->da_start;
+
+	BUG_ON((!total) || !IS_ALIGNED(total, PAGE_SIZE));
+
+	start = area->da_start;
+	while (total > 0) {
+		size_t bytes;
+
+		bytes = iopgtable_clear_entry(obj, start);
+		if (bytes == 0)
+			bytes = PAGE_SIZE;
+		else
+			dev_dbg(obj->dev, "%s: unmap %08x(%x) %08x\n",
+				__func__, start, bytes, area->flags);
+
+		BUG_ON(!IS_ALIGNED(bytes, PAGE_SIZE));
+
+		total -= bytes;
+		start += bytes;
+	}
+	BUG_ON(total);
+}
+
+/* template function for all unmapping */
+static struct sg_table *unmap_vm_area(struct iommu *obj, const u32 da,
+				      void (*fn)(const void *), u32 flags)
+{
+	struct sg_table *sgt = NULL;
+	struct iovm_struct *area;
+
+	if (!IS_ALIGNED(da, PAGE_SIZE)) {
+		dev_err(obj->dev, "%s: alignment err(%08x)\n", __func__, da);
+		return NULL;
+	}
+
+	mutex_lock(&obj->mmap_lock);
+
+	area = __find_iovm_area(obj, da);
+	if (!area) {
+		dev_dbg(obj->dev, "%s: no da area(%08x)\n", __func__, da);
+		goto out;
+	}
+
+	if ((area->flags & flags) != flags) {
+		dev_err(obj->dev, "%s: wrong flags(%08x)\n", __func__,
+			area->flags);
+		goto out;
+	}
+	sgt = (struct sg_table *)area->sgt;
+
+	unmap_iovm_area(obj, area);
+
+	fn(area->va);
+
+	dev_dbg(obj->dev, "%s: %08x-%08x-%08x(%x) %08x\n", __func__,
+		area->da_start, da, area->da_end,
+		area->da_end - area->da_start, area->flags);
+
+	free_iovm_area(obj, area);
+out:
+	mutex_unlock(&obj->mmap_lock);
+
+	return sgt;
+}
+
+static u32 map_iommu_region(struct iommu *obj, u32 da,
+	      const struct sg_table *sgt, void *va, size_t bytes, u32 flags)
+{
+	int err = -ENOMEM;
+	struct iovm_struct *new;
+
+	mutex_lock(&obj->mmap_lock);
+
+	new = alloc_iovm_area(obj, da, bytes, flags);
+	if (IS_ERR(new)) {
+		err = PTR_ERR(new);
+		goto err_alloc_iovma;
+	}
+	new->va = va;
+	new->sgt = sgt;
+
+	if (map_iovm_area(obj, new, sgt, new->flags))
+		goto err_map;
+
+	mutex_unlock(&obj->mmap_lock);
+
+	dev_dbg(obj->dev, "%s: da:%08x(%x) flags:%08x va:%p\n",
+		__func__, new->da_start, bytes, new->flags, va);
+
+	return new->da_start;
+
+err_map:
+	free_iovm_area(obj, new);
+err_alloc_iovma:
+	mutex_unlock(&obj->mmap_lock);
+	return err;
+}
+
+static inline u32 __iommu_vmap(struct iommu *obj, u32 da,
+		 const struct sg_table *sgt, void *va, size_t bytes, u32 flags)
+{
+	return map_iommu_region(obj, da, sgt, va, bytes, flags);
+}
+
+/**
+ * iommu_vmap  -  (d)-(p)-(v) address mapper
+ * @obj:	objective iommu
+ * @sgt:	address of scatter gather table
+ * @flags:	iovma and page property
+ *
+ * Creates 1-n-1 mapping with given @sgt and returns @da.
+ * All @sgt element must be io page size aligned.
+ */
+u32 iommu_vmap(struct iommu *obj, u32 da, const struct sg_table *sgt,
+		 u32 flags)
+{
+	size_t bytes;
+	void *va;
+
+	if (!obj || !obj->dev || !sgt)
+		return -EINVAL;
+
+	bytes = sgtable_len(sgt);
+	if (!bytes)
+		return -EINVAL;
+	bytes = PAGE_ALIGN(bytes);
+
+	va = vmap_sg(sgt);
+	if (IS_ERR(va))
+		return PTR_ERR(va);
+
+	flags &= IOVMF_HW_MASK;
+	flags |= IOVMF_DISCONT;
+	flags |= IOVMF_MMIO;
+	flags |= (da ? IOVMF_DA_FIXED : IOVMF_DA_ANON);
+
+	da = __iommu_vmap(obj, da, sgt, va, bytes, flags);
+	if (IS_ERR_VALUE(da))
+		vunmap_sg(va);
+
+	return da;
+}
+EXPORT_SYMBOL_GPL(iommu_vmap);
+
+/**
+ * iommu_vunmap  -  release virtual mapping obtained by 'iommu_vmap()'
+ * @obj:	objective iommu
+ * @da:		iommu device virtual address
+ *
+ * Free the iommu virtually contiguous memory area starting at
+ * @da, which was returned by 'iommu_vmap()'.
+ */
+struct sg_table *iommu_vunmap(struct iommu *obj, u32 da)
+{
+	struct sg_table *sgt;
+	/*
+	 * 'sgt' is allocated before 'iommu_vmalloc()' is called.
+	 * Just returns 'sgt' to the caller to free
+	 */
+	sgt = unmap_vm_area(obj, da, vunmap_sg, IOVMF_DISCONT | IOVMF_MMIO);
+	if (!sgt)
+		dev_dbg(obj->dev, "%s: No sgt\n", __func__);
+	return sgt;
+}
+EXPORT_SYMBOL_GPL(iommu_vunmap);
+
+/**
+ * iommu_vmalloc  -  (d)-(p)-(v) address allocator and mapper
+ * @obj:	objective iommu
+ * @da:		contiguous iommu virtual memory
+ * @bytes:	allocation size
+ * @flags:	iovma and page property
+ *
+ * Allocate @bytes linearly and creates 1-n-1 mapping and returns
+ * @da again, which might be adjusted if 'IOVMF_DA_ANON' is set.
+ */
+u32 iommu_vmalloc(struct iommu *obj, u32 da, size_t bytes, u32 flags)
+{
+	void *va;
+	struct sg_table *sgt;
+
+	if (!obj || !obj->dev || !bytes)
+		return -EINVAL;
+
+	bytes = PAGE_ALIGN(bytes);
+
+	va = vmalloc(bytes);
+	if (!va)
+		return -ENOMEM;
+
+	sgt = sgtable_alloc(bytes, flags);
+	if (IS_ERR(sgt)) {
+		da = PTR_ERR(sgt);
+		goto err_sgt_alloc;
+	}
+	sgtable_fill_vmalloc(sgt, va);
+
+	flags &= IOVMF_HW_MASK;
+	flags |= IOVMF_DISCONT;
+	flags |= IOVMF_ALLOC;
+	flags |= (da ? IOVMF_DA_FIXED : IOVMF_DA_ANON);
+
+	da = __iommu_vmap(obj, da, sgt, va, bytes, flags);
+	if (IS_ERR_VALUE(da))
+		goto err_iommu_vmap;
+
+	return da;
+
+err_iommu_vmap:
+	sgtable_drain_vmalloc(sgt);
+	sgtable_free(sgt);
+err_sgt_alloc:
+	vfree(va);
+	return da;
+}
+EXPORT_SYMBOL_GPL(iommu_vmalloc);
+
+/**
+ * iommu_vfree  -  release memory allocated by 'iommu_vmalloc()'
+ * @obj:	objective iommu
+ * @da:		iommu device virtual address
+ *
+ * Frees the iommu virtually continuous memory area starting at
+ * @da, as obtained from 'iommu_vmalloc()'.
+ */
+void iommu_vfree(struct iommu *obj, const u32 da)
+{
+	struct sg_table *sgt;
+
+	sgt = unmap_vm_area(obj, da, vfree, IOVMF_DISCONT | IOVMF_ALLOC);
+	if (!sgt)
+		dev_dbg(obj->dev, "%s: No sgt\n", __func__);
+	sgtable_free(sgt);
+}
+EXPORT_SYMBOL_GPL(iommu_vfree);
+
+static u32 __iommu_kmap(struct iommu *obj, u32 da, u32 pa, void *va,
+			  size_t bytes, u32 flags)
+{
+	struct sg_table *sgt;
+
+	sgt = sgtable_alloc(bytes, flags);
+	if (IS_ERR(sgt))
+		return PTR_ERR(sgt);
+
+	sgtable_fill_kmalloc(sgt, pa, bytes);
+
+	da = map_iommu_region(obj, da, sgt, va, bytes, flags);
+	if (IS_ERR_VALUE(da)) {
+		sgtable_drain_kmalloc(sgt);
+		sgtable_free(sgt);
+	}
+
+	return da;
+}
+
+/**
+ * iommu_kmap  -  (d)-(p)-(v) address mapper
+ * @obj:	objective iommu
+ * @da:		contiguous iommu virtual memory
+ * @pa:		contiguous physical memory
+ * @flags:	iovma and page property
+ *
+ * Creates 1-1-1 mapping and returns @da again, which can be
+ * adjusted if 'IOVMF_DA_ANON' is set.
+ */
+u32 iommu_kmap(struct iommu *obj, u32 da, u32 pa, size_t bytes,
+		 u32 flags)
+{
+	void *va;
+
+	if (!obj || !obj->dev || !bytes)
+		return -EINVAL;
+
+	bytes = PAGE_ALIGN(bytes);
+
+	va = ioremap(pa, bytes);
+	if (!va)
+		return -ENOMEM;
+
+	flags &= IOVMF_HW_MASK;
+	flags |= IOVMF_LINEAR;
+	flags |= IOVMF_MMIO;
+	flags |= (da ? IOVMF_DA_FIXED : IOVMF_DA_ANON);
+
+	da = __iommu_kmap(obj, da, pa, va, bytes, flags);
+	if (IS_ERR_VALUE(da))
+		iounmap(va);
+
+	return da;
+}
+EXPORT_SYMBOL_GPL(iommu_kmap);
+
+/**
+ * iommu_kunmap  -  release virtual mapping obtained by 'iommu_kmap()'
+ * @obj:	objective iommu
+ * @da:		iommu device virtual address
+ *
+ * Frees the iommu virtually contiguous memory area starting at
+ * @da, which was passed to and was returned by'iommu_kmap()'.
+ */
+void iommu_kunmap(struct iommu *obj, u32 da)
+{
+	struct sg_table *sgt;
+	typedef void (*func_t)(const void *);
+
+	sgt = unmap_vm_area(obj, da, (func_t)__iounmap,
+			    IOVMF_LINEAR | IOVMF_MMIO);
+	if (!sgt)
+		dev_dbg(obj->dev, "%s: No sgt\n", __func__);
+	sgtable_free(sgt);
+}
+EXPORT_SYMBOL_GPL(iommu_kunmap);
+
+/**
+ * iommu_kmalloc  -  (d)-(p)-(v) address allocator and mapper
+ * @obj:	objective iommu
+ * @da:		contiguous iommu virtual memory
+ * @bytes:	bytes for allocation
+ * @flags:	iovma and page property
+ *
+ * Allocate @bytes linearly and creates 1-1-1 mapping and returns
+ * @da again, which might be adjusted if 'IOVMF_DA_ANON' is set.
+ */
+u32 iommu_kmalloc(struct iommu *obj, u32 da, size_t bytes, u32 flags)
+{
+	void *va;
+	u32 pa;
+
+	if (!obj || !obj->dev || !bytes)
+		return -EINVAL;
+
+	bytes = PAGE_ALIGN(bytes);
+
+	va = kmalloc(bytes, GFP_KERNEL | GFP_DMA);
+	if (!va)
+		return -ENOMEM;
+	pa = virt_to_phys(va);
+
+	flags &= IOVMF_HW_MASK;
+	flags |= IOVMF_LINEAR;
+	flags |= IOVMF_ALLOC;
+	flags |= (da ? IOVMF_DA_FIXED : IOVMF_DA_ANON);
+
+	da = __iommu_kmap(obj, da, pa, va, bytes, flags);
+	if (IS_ERR_VALUE(da))
+		kfree(va);
+
+	return da;
+}
+EXPORT_SYMBOL_GPL(iommu_kmalloc);
+
+/**
+ * iommu_kfree  -  release virtual mapping obtained by 'iommu_kmalloc()'
+ * @obj:	objective iommu
+ * @da:		iommu device virtual address
+ *
+ * Frees the iommu virtually contiguous memory area starting at
+ * @da, which was passed to and was returned by'iommu_kmalloc()'.
+ */
+void iommu_kfree(struct iommu *obj, u32 da)
+{
+	struct sg_table *sgt;
+
+	sgt = unmap_vm_area(obj, da, kfree, IOVMF_LINEAR | IOVMF_ALLOC);
+	if (!sgt)
+		dev_dbg(obj->dev, "%s: No sgt\n", __func__);
+	sgtable_free(sgt);
+}
+EXPORT_SYMBOL_GPL(iommu_kfree);
+
+
+static int __init iovmm_init(void)
+{
+	const unsigned long flags = SLAB_HWCACHE_ALIGN;
+	struct kmem_cache *p;
+
+	p = kmem_cache_create("iovm_area_cache", sizeof(struct iovm_struct), 0,
+			      flags, NULL);
+	if (!p)
+		return -ENOMEM;
+	iovm_area_cachep = p;
+
+	return 0;
+}
+module_init(iovmm_init);
+
+static void __exit iovmm_exit(void)
+{
+	kmem_cache_destroy(iovm_area_cachep);
+}
+module_exit(iovmm_exit);
+
+MODULE_DESCRIPTION("omap iommu: simple virtual address space management");
+MODULE_AUTHOR("Hiroshi DOYU <Hiroshi.DOYU@nokia.com>");
+MODULE_LICENSE("GPL v2");


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

* [PATCH 5/6] omap iommu: entries for Kconfig and Makefile
  2009-05-05 12:46 [PATCH 0/6] Initial support for omap iommu driver Hiroshi DOYU
                   ` (3 preceding siblings ...)
  2009-05-05 12:47 ` [PATCH 4/6] omap iommu: simple virtual address space management Hiroshi DOYU
@ 2009-05-05 12:47 ` Hiroshi DOYU
  2009-05-18 13:36   ` Russell King - ARM Linux
  2009-05-05 12:47 ` [PATCH 6/6] omap2 " Hiroshi DOYU
  5 siblings, 1 reply; 53+ messages in thread
From: Hiroshi DOYU @ 2009-05-05 12:47 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-omap, h-kanigeri2, omar.ramirez, sakari.ailus, tony

Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---

 arch/arm/plat-omap/Kconfig  |    8 ++++++++
 arch/arm/plat-omap/Makefile |    1 +
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
index 9dd68fa..505e6ab 100644
--- a/arch/arm/plat-omap/Kconfig
+++ b/arch/arm/plat-omap/Kconfig
@@ -115,6 +115,14 @@ config OMAP_MBOX_FWK
 	  Say Y here if you want to use OMAP Mailbox framework support for
 	  DSP, IVA1.0 and IVA2 in OMAP1/2/3.
 
+config OMAP_IOMMU
+	tristate "IOMMU support"
+	depends on ARCH_OMAP
+	default n
+	help
+	  Say Y here if you want to use OMAP IOMMU support for IVA2 and
+	  Camera in OMAP3.
+
 choice
         prompt "System timer"
 	default OMAP_MPU_TIMER
diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
index 04a100c..a832795 100644
--- a/arch/arm/plat-omap/Makefile
+++ b/arch/arm/plat-omap/Makefile
@@ -13,6 +13,7 @@ obj-  :=
 obj-$(CONFIG_ARCH_OMAP16XX) += ocpi.o
 
 obj-$(CONFIG_OMAP_MCBSP) += mcbsp.o
+obj-$(CONFIG_OMAP_IOMMU) += iommu.o iovmm.o
 
 obj-$(CONFIG_CPU_FREQ) += cpu-omap.o
 obj-$(CONFIG_OMAP_DM_TIMER) += dmtimer.o


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

* [PATCH 6/6] omap2 iommu: entries for Kconfig and Makefile
  2009-05-05 12:46 [PATCH 0/6] Initial support for omap iommu driver Hiroshi DOYU
                   ` (4 preceding siblings ...)
  2009-05-05 12:47 ` [PATCH 5/6] omap iommu: entries for Kconfig and Makefile Hiroshi DOYU
@ 2009-05-05 12:47 ` Hiroshi DOYU
  5 siblings, 0 replies; 53+ messages in thread
From: Hiroshi DOYU @ 2009-05-05 12:47 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-omap, h-kanigeri2, omar.ramirez, sakari.ailus, tony

Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---

 arch/arm/mach-omap2/Makefile |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index c49d9bf..88629a7 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -28,6 +28,11 @@ endif
 obj-$(CONFIG_ARCH_OMAP2)		+= clock24xx.o
 obj-$(CONFIG_ARCH_OMAP3)		+= clock34xx.o
 
+iommu-y					+= iommu2.o
+iommu-$(CONFIG_ARCH_OMAP3)		+= omap3-iommu.o
+
+obj-$(CONFIG_OMAP_IOMMU)		+= $(iommu-y)
+
 # Specific board support
 obj-$(CONFIG_MACH_OMAP_GENERIC)		+= board-generic.o
 obj-$(CONFIG_MACH_OMAP_H4)		+= board-h4.o


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

* Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-05-05 12:47 ` [PATCH 3/6] omap iommu: omap3 iommu device registration Hiroshi DOYU
@ 2009-05-05 19:32   ` Felipe Contreras
  2009-05-06  6:00     ` Hiroshi DOYU
  2009-05-07 20:05   ` [PATCH 3/6] omap iommu: omap3 iommu device registration Felipe Contreras
  2009-05-16  9:20   ` Russell King - ARM Linux
  2 siblings, 1 reply; 53+ messages in thread
From: Felipe Contreras @ 2009-05-05 19:32 UTC (permalink / raw)
  To: Hiroshi DOYU
  Cc: linux-arm-kernel, linux-omap, h-kanigeri2, omar.ramirez,
	sakari.ailus, tony

On Tue, May 5, 2009 at 3:47 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> ---
>
>  arch/arm/mach-omap2/omap3-iommu.c |  105 +++++++++++++++++++++++++++++++++++++
>  1 files changed, 105 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-omap2/omap3-iommu.c
>
> diff --git a/arch/arm/mach-omap2/omap3-iommu.c b/arch/arm/mach-omap2/omap3-iommu.c
> new file mode 100644
> index 0000000..367f36a
> --- /dev/null
> +++ b/arch/arm/mach-omap2/omap3-iommu.c
> @@ -0,0 +1,105 @@
> +/*
> + * omap iommu: omap3 device registration
> + *
> + * Copyright (C) 2008-2009 Nokia Corporation
> + *
> + * Written by Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/platform_device.h>
> +
> +#include <mach/iommu.h>
> +
> +#define OMAP3_MMU1_BASE        0x480bd400
> +#define OMAP3_MMU2_BASE        0x5d000000
> +#define OMAP3_MMU1_IRQ 24
> +#define OMAP3_MMU2_IRQ 28
> +
> +static struct resource omap3_iommu_res[] = {
> +       { /* Camera ISP MMU */
> +               .start          = OMAP3_MMU1_BASE,
> +               .end            = OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
> +               .flags          = IORESOURCE_MEM,
> +       },
> +       {
> +               .start          = OMAP3_MMU1_IRQ,
> +               .flags          = IORESOURCE_IRQ,
> +       },
> +       { /* IVA2.2 MMU */
> +               .start          = OMAP3_MMU2_BASE,
> +               .end            = OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
> +               .flags          = IORESOURCE_MEM,
> +       },
> +       {
> +               .start          = OMAP3_MMU2_IRQ,
> +               .flags          = IORESOURCE_IRQ,
> +       },
> +};

This will break TI's bridgedriver, right?

Can camera ISP and IVA request the registration of their respective
IOMMU's from their codebase?

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* RE: [PATCH 1/6] omap iommu: tlb and pagetable primitives
  2009-05-05 12:46 ` [PATCH 1/6] omap iommu: tlb and pagetable primitives Hiroshi DOYU
@ 2009-05-05 20:19   ` Kanigeri, Hari
  2009-05-06  5:56     ` Hiroshi DOYU
  0 siblings, 1 reply; 53+ messages in thread
From: Kanigeri, Hari @ 2009-05-05 20:19 UTC (permalink / raw)
  To: Hiroshi DOYU, linux-arm-kernel
  Cc: linux-omap, Ramirez Luna, Omar, sakari.ailus, tony, Pasam, Vijay

Hi Doyu-san,

+static void iotlb_lock_set(struct iommu *obj, struct iotlb_lock *l) {
+	u32 val;
+
+	BUG_ON(l->base != 0); /* Currently no preservation is used */

-- Did you notice any issues in the IOMMU driver by enabling the preservation? 

Bridge driver is locking the memory segment that is used for IPC in the TLB and by not having the preservation we might see a hit in the performance.

Thank you,
Best regards,
Hari

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

* Re: [PATCH 1/6] omap iommu: tlb and pagetable primitives
  2009-05-05 20:19   ` Kanigeri, Hari
@ 2009-05-06  5:56     ` Hiroshi DOYU
  0 siblings, 0 replies; 53+ messages in thread
From: Hiroshi DOYU @ 2009-05-06  5:56 UTC (permalink / raw)
  To: h-kanigeri2
  Cc: linux-arm-kernel, linux-omap, omar.ramirez, sakari.ailus, tony, vpasam

Hi Hari,

From: "ext Kanigeri, Hari" <h-kanigeri2@ti.com>
Subject: RE: [PATCH 1/6] omap iommu: tlb and pagetable primitives
Date: Tue, 5 May 2009 22:19:55 +0200

> Hi Doyu-san,
> 
> +static void iotlb_lock_set(struct iommu *obj, struct iotlb_lock *l) {
> +	u32 val;
> +
> +	BUG_ON(l->base != 0); /* Currently no preservation is used */
> 
> -- Did you notice any issues in the IOMMU driver by enabling the preservation? 
> 
> Bridge driver is locking the memory segment that is used for IPC in
> the TLB and by not having the preservation we might see a hit in the
> performance.

The current user of this driver is "camera isp" and no need for tlb
preservation for it, but it's on todo list for other clients.



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

* Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-05-05 19:32   ` Felipe Contreras
@ 2009-05-06  6:00     ` Hiroshi DOYU
  2009-05-07 20:02       ` Felipe Contreras
  0 siblings, 1 reply; 53+ messages in thread
From: Hiroshi DOYU @ 2009-05-06  6:00 UTC (permalink / raw)
  To: felipe.contreras
  Cc: linux-arm-kernel, linux-omap, h-kanigeri2, omar.ramirez,
	sakari.ailus, tony

From: ext Felipe Contreras <felipe.contreras@gmail.com>
Subject: Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
Date: Tue, 5 May 2009 21:32:34 +0200

> On Tue, May 5, 2009 at 3:47 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> > Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> > ---
> >
> >  arch/arm/mach-omap2/omap3-iommu.c |  105 +++++++++++++++++++++++++++++++++++++
> >  1 files changed, 105 insertions(+), 0 deletions(-)
> >  create mode 100644 arch/arm/mach-omap2/omap3-iommu.c
> >
> > diff --git a/arch/arm/mach-omap2/omap3-iommu.c b/arch/arm/mach-omap2/omap3-iommu.c
> > new file mode 100644
> > index 0000000..367f36a
> > --- /dev/null
> > +++ b/arch/arm/mach-omap2/omap3-iommu.c
> > @@ -0,0 +1,105 @@
> > +/*
> > + * omap iommu: omap3 device registration
> > + *
> > + * Copyright (C) 2008-2009 Nokia Corporation
> > + *
> > + * Written by Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/platform_device.h>
> > +
> > +#include <mach/iommu.h>
> > +
> > +#define OMAP3_MMU1_BASE        0x480bd400
> > +#define OMAP3_MMU2_BASE        0x5d000000
> > +#define OMAP3_MMU1_IRQ 24
> > +#define OMAP3_MMU2_IRQ 28
> > +
> > +static struct resource omap3_iommu_res[] = {
> > +       { /* Camera ISP MMU */
> > +               .start          = OMAP3_MMU1_BASE,
> > +               .end            = OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
> > +               .flags          = IORESOURCE_MEM,
> > +       },
> > +       {
> > +               .start          = OMAP3_MMU1_IRQ,
> > +               .flags          = IORESOURCE_IRQ,
> > +       },
> > +       { /* IVA2.2 MMU */
> > +               .start          = OMAP3_MMU2_BASE,
> > +               .end            = OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
> > +               .flags          = IORESOURCE_MEM,
> > +       },
> > +       {
> > +               .start          = OMAP3_MMU2_IRQ,
> > +               .flags          = IORESOURCE_IRQ,
> > +       },
> > +};
> 
> This will break TI's bridgedriver, right?

Working around is easy, but let's go forward, correcting TI's bridgedriver;-)

http://marc.info/?l=linux-omap&m=124148814309478&w=2


> 
> Can camera ISP and IVA request the registration of their respective
> IOMMU's from their codebase?
> 
> -- 
> Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* RE: [PATCH 2/6] omap iommu: omap2 architecture specific functions
  2009-05-05 12:46 ` [PATCH 2/6] omap iommu: omap2 architecture specific functions Hiroshi DOYU
@ 2009-05-06 14:31   ` Kanigeri, Hari
  2009-05-07  5:12     ` Hiroshi DOYU
  0 siblings, 1 reply; 53+ messages in thread
From: Kanigeri, Hari @ 2009-05-06 14:31 UTC (permalink / raw)
  To: Hiroshi DOYU, linux-arm-kernel
  Cc: linux-omap, Ramirez Luna, Omar, sakari.ailus, tony, Pasam, Vijay

Hi,

> +static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra)
> +{
> +	int i;
> +	u32 stat, da;
> +	const char *err_msg[] =	{
> +		"tlb miss",
> +		"translation fault",
> +		"emulation miss",
> +		"table walk fault",
> +		"multi hit fault",
> +	};
> +
> +	stat = iommu_read_reg(obj, MMU_IRQSTATUS);
> +	stat &= MMU_IRQ_MASK;
> +	if (!stat)
> +		return 0;
> +
> +	da = iommu_read_reg(obj, MMU_FAULT_AD);
> +	*ra = da;
> +
> +	dev_err(obj->dev, "%s:\tda:%08x ", __func__, da);
> +
> +	for (i = 0; i < ARRAY_SIZE(err_msg); i++) {
> +		if (stat & (1 << i))
> +			printk("%s ", err_msg[i]);
> +	}
> +	printk("\n");
> +
> +	iommu_write_reg(obj, stat, MMU_IRQSTATUS);
> +	return stat;
> +}
> +

-- I see you are acking the MMU fault in the ISR, but I don't think this will be enough to stop the further generation of MMU faults as the device will again try to access the same fault address. 

In the mean time before the callback mechanism is implemented, we should consider disabling the MMU for the device that caused the MMU fault to stop further generation of MMU faults.

+	printk("\n");
+
+	iommu_write_reg(obj, stat, MMU_IRQSTATUS);
+	omap2_iommu_disable(obj) -----------------------> [HK]
+	return stat;
+}

Thank you,
Best regards,
Hari

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

* Re: [PATCH 2/6] omap iommu: omap2 architecture specific functions
  2009-05-06 14:31   ` Kanigeri, Hari
@ 2009-05-07  5:12     ` Hiroshi DOYU
  2009-05-07 14:40       ` Kanigeri, Hari
  0 siblings, 1 reply; 53+ messages in thread
From: Hiroshi DOYU @ 2009-05-07  5:12 UTC (permalink / raw)
  To: h-kanigeri2
  Cc: linux-arm-kernel, linux-omap, omar.ramirez, sakari.ailus, tony, vpasam

From: "ext Kanigeri, Hari" <h-kanigeri2@ti.com>
Subject: RE: [PATCH 2/6] omap iommu: omap2 architecture specific functions
Date: Wed, 6 May 2009 16:31:37 +0200

> Hi,
> 
> > +static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra)
> > +{
> > +	int i;
> > +	u32 stat, da;
> > +	const char *err_msg[] =	{
> > +		"tlb miss",
> > +		"translation fault",
> > +		"emulation miss",
> > +		"table walk fault",
> > +		"multi hit fault",
> > +	};
> > +
> > +	stat = iommu_read_reg(obj, MMU_IRQSTATUS);
> > +	stat &= MMU_IRQ_MASK;
> > +	if (!stat)
> > +		return 0;
> > +
> > +	da = iommu_read_reg(obj, MMU_FAULT_AD);
> > +	*ra = da;
> > +
> > +	dev_err(obj->dev, "%s:\tda:%08x ", __func__, da);
> > +
> > +	for (i = 0; i < ARRAY_SIZE(err_msg); i++) {
> > +		if (stat & (1 << i))
> > +			printk("%s ", err_msg[i]);
> > +	}
> > +	printk("\n");
> > +
> > +	iommu_write_reg(obj, stat, MMU_IRQSTATUS);
> > +	return stat;
> > +}
> > +
> 
> -- I see you are acking the MMU fault in the ISR, but I don't think
> this will be enough to stop the further generation of MMU faults as
> the device will again try to access the same fault address. 

My idea is that, an iommu fault should be handled by a client
specific/provided callback function enough flexibly that it can handle
more complecated use cases like implementing on-demand dynamic memory
mapping at getting an iommu fault.

linux/arch/arm/plat-omap/iommu.c:

+static irqreturn_t iommu_fault_handler(int irq, void *data)
+{
+       u32 stat, da;
+       u32 *iopgd, *iopte;
+       int err = -EIO;
+       struct iommu *obj = data;
+
+       if (!obj->refcount)
+               return IRQ_NONE;
+
+       /* Dynamic loading TLB or PTE */
+       if (obj->isr)
+               err = obj->isr(obj);
		      ^^^^^^^^^^^^^^

If the above "isr" function sets a new "tlb" or "pte" entry, then
further iommu fault won't happen anymore.

> 
> In the mean time before the callback mechanism is implemented, we
> should consider disabling the MMU for the device that caused the MMU
> fault to stop further generation of MMU faults.
> 
> +	printk("\n");
> +
> +	iommu_write_reg(obj, stat, MMU_IRQSTATUS);
> +	omap2_iommu_disable(obj) -----------------------> [HK]
> +	return stat;
> +}

It's not bad idea at all to disable iommu temporary as a default
behavior, but maybe it should be done in the latter half of
"iommu_fault_handler()".

> 
> Thank you,
> Best regards,
> Hari

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

* RE: [PATCH 2/6] omap iommu: omap2 architecture specific functions
  2009-05-07  5:12     ` Hiroshi DOYU
@ 2009-05-07 14:40       ` Kanigeri, Hari
  0 siblings, 0 replies; 53+ messages in thread
From: Kanigeri, Hari @ 2009-05-07 14:40 UTC (permalink / raw)
  To: Hiroshi DOYU
  Cc: linux-arm-kernel, linux-omap, Ramirez Luna, Omar, sakari.ailus,
	tony, Pasam, Vijay

Hi,

> +static irqreturn_t iommu_fault_handler(int irq, void *data)
> +{
> +       u32 stat, da;
> +       u32 *iopgd, *iopte;
> +       int err = -EIO;
> +       struct iommu *obj = data;
> +
> +       if (!obj->refcount)
> +               return IRQ_NONE;
> +
> +       /* Dynamic loading TLB or PTE */
> +       if (obj->isr)
> +               err = obj->isr(obj);
> 		      ^^^^^^^^^^^^^^
> 
> If the above "isr" function sets a new "tlb" or "pte" entry, then
> further iommu fault won't happen anymore.
>

-- I think adding the new tlb entry or any other error recovery mechanism should be deferred to the registered callback function. 

Thank you,
Best regards,
Hari

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

* Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-05-06  6:00     ` Hiroshi DOYU
@ 2009-05-07 20:02       ` Felipe Contreras
  2009-05-07 20:11         ` [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration Felipe Contreras
  0 siblings, 1 reply; 53+ messages in thread
From: Felipe Contreras @ 2009-05-07 20:02 UTC (permalink / raw)
  To: Hiroshi DOYU
  Cc: linux-arm-kernel, linux-omap, h-kanigeri2, omar.ramirez,
	sakari.ailus, tony

On Wed, May 6, 2009 at 9:00 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> From: ext Felipe Contreras <felipe.contreras@gmail.com>
> Subject: Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
> Date: Tue, 5 May 2009 21:32:34 +0200
>
>> On Tue, May 5, 2009 at 3:47 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
>> > Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
>> > ---
>> >
>> >  arch/arm/mach-omap2/omap3-iommu.c |  105 +++++++++++++++++++++++++++++++++++++
>> >  1 files changed, 105 insertions(+), 0 deletions(-)
>> >  create mode 100644 arch/arm/mach-omap2/omap3-iommu.c
>> >
>> > diff --git a/arch/arm/mach-omap2/omap3-iommu.c b/arch/arm/mach-omap2/omap3-iommu.c
>> > new file mode 100644
>> > index 0000000..367f36a
>> > --- /dev/null
>> > +++ b/arch/arm/mach-omap2/omap3-iommu.c
>> > @@ -0,0 +1,105 @@
>> > +/*
>> > + * omap iommu: omap3 device registration
>> > + *
>> > + * Copyright (C) 2008-2009 Nokia Corporation
>> > + *
>> > + * Written by Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
>> > + *
>> > + * This program is free software; you can redistribute it and/or modify
>> > + * it under the terms of the GNU General Public License version 2 as
>> > + * published by the Free Software Foundation.
>> > + */
>> > +
>> > +#include <linux/platform_device.h>
>> > +
>> > +#include <mach/iommu.h>
>> > +
>> > +#define OMAP3_MMU1_BASE        0x480bd400
>> > +#define OMAP3_MMU2_BASE        0x5d000000
>> > +#define OMAP3_MMU1_IRQ 24
>> > +#define OMAP3_MMU2_IRQ 28
>> > +
>> > +static struct resource omap3_iommu_res[] = {
>> > +       { /* Camera ISP MMU */
>> > +               .start          = OMAP3_MMU1_BASE,
>> > +               .end            = OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
>> > +               .flags          = IORESOURCE_MEM,
>> > +       },
>> > +       {
>> > +               .start          = OMAP3_MMU1_IRQ,
>> > +               .flags          = IORESOURCE_IRQ,
>> > +       },
>> > +       { /* IVA2.2 MMU */
>> > +               .start          = OMAP3_MMU2_BASE,
>> > +               .end            = OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
>> > +               .flags          = IORESOURCE_MEM,
>> > +       },
>> > +       {
>> > +               .start          = OMAP3_MMU2_IRQ,
>> > +               .flags          = IORESOURCE_IRQ,
>> > +       },
>> > +};
>>
>> This will break TI's bridgedriver, right?
>
> Working around is easy, but let's go forward, correcting TI's bridgedriver;-)
>
> http://marc.info/?l=linux-omap&m=124148814309478&w=2

*sigh*

I can't recall how many times I've asked for this, I'm sending a patch
series (RFC) that allows exactly what I want.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-05-05 12:47 ` [PATCH 3/6] omap iommu: omap3 iommu device registration Hiroshi DOYU
  2009-05-05 19:32   ` Felipe Contreras
@ 2009-05-07 20:05   ` Felipe Contreras
  2009-05-08  4:10     ` Hiroshi DOYU
  2009-05-16  9:20   ` Russell King - ARM Linux
  2 siblings, 1 reply; 53+ messages in thread
From: Felipe Contreras @ 2009-05-07 20:05 UTC (permalink / raw)
  To: Hiroshi DOYU
  Cc: linux-arm-kernel, linux-omap, h-kanigeri2, omar.ramirez,
	sakari.ailus, tony

On Tue, May 5, 2009 at 3:47 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> ---
>
>  arch/arm/mach-omap2/omap3-iommu.c |  105 +++++++++++++++++++++++++++++++++++++
>  1 files changed, 105 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-omap2/omap3-iommu.c
>
> diff --git a/arch/arm/mach-omap2/omap3-iommu.c b/arch/arm/mach-omap2/omap3-iommu.c
> new file mode 100644
> index 0000000..367f36a
> --- /dev/null
> +++ b/arch/arm/mach-omap2/omap3-iommu.c
> @@ -0,0 +1,105 @@
> +/*
> + * omap iommu: omap3 device registration
> + *
> + * Copyright (C) 2008-2009 Nokia Corporation
> + *
> + * Written by Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/platform_device.h>
> +
> +#include <mach/iommu.h>
> +
> +#define OMAP3_MMU1_BASE        0x480bd400
> +#define OMAP3_MMU2_BASE        0x5d000000
> +#define OMAP3_MMU1_IRQ 24
> +#define OMAP3_MMU2_IRQ 28
> +
> +static struct resource omap3_iommu_res[] = {
> +       { /* Camera ISP MMU */
> +               .start          = OMAP3_MMU1_BASE,
> +               .end            = OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
> +               .flags          = IORESOURCE_MEM,
> +       },
> +       {
> +               .start          = OMAP3_MMU1_IRQ,
> +               .flags          = IORESOURCE_IRQ,
> +       },
> +       { /* IVA2.2 MMU */
> +               .start          = OMAP3_MMU2_BASE,
> +               .end            = OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
> +               .flags          = IORESOURCE_MEM,
> +       },
> +       {
> +               .start          = OMAP3_MMU2_IRQ,
> +               .flags          = IORESOURCE_IRQ,
> +       },
> +};
> +#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)

<snip/>

> +               err = platform_device_add_resources(pdev,
> +                                   &omap3_iommu_res[2 * i], NR_IOMMU_RES);

This is wrong. In this particular case NR_IOMMU_RES is 2, but not
because there's 2 resources: MEM and IRQ, but because there's 2 iommu
devices: isp and iva2.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration
  2009-05-07 20:02       ` Felipe Contreras
@ 2009-05-07 20:11         ` Felipe Contreras
  2009-05-07 20:11           ` [RFC/PATCH 1/3] omap3-iommu: reorganize Felipe Contreras
                             ` (2 more replies)
  0 siblings, 3 replies; 53+ messages in thread
From: Felipe Contreras @ 2009-05-07 20:11 UTC (permalink / raw)
  To: linux-omap
  Cc: Hiroshi DOYU, Felipe Contreras, Tony Lindgren, Felipe Contreras

This patch series cleanups up a bit the opap3-iommu device registration and
then allows registration from other parts of the code.

Currently the iva2 code (tidspbridge) is not using iommu, therefore it can't be
used as-is with the current omap iommu. By allowing devies to be registered
externaly (either isp, or iva2) this problem goes away.

Felipe Contreras (3):
  omap3-iommu: reorganize
  omap3-iommu: split init function into omap_iommu_add
  omap3-iommu: remote registration

 arch/arm/mach-omap2/omap3-iommu.c       |  130 ++++++++++++++-----------------
 arch/arm/plat-omap/include/mach/iommu.h |    2 +
 2 files changed, 62 insertions(+), 70 deletions(-)


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

* [RFC/PATCH 1/3] omap3-iommu: reorganize
  2009-05-07 20:11         ` [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration Felipe Contreras
@ 2009-05-07 20:11           ` Felipe Contreras
  2009-05-07 20:11             ` [RFC/PATCH 2/3] omap3-iommu: split init function into omap_iommu_add Felipe Contreras
  2009-05-07 21:14             ` [RFC/PATCH 1/3] omap3-iommu: reorganize Felipe Balbi
  2009-05-07 20:28           ` [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration Kanigeri, Hari
  2009-05-08  4:14           ` Hiroshi DOYU
  2 siblings, 2 replies; 53+ messages in thread
From: Felipe Contreras @ 2009-05-07 20:11 UTC (permalink / raw)
  To: linux-omap
  Cc: Hiroshi DOYU, Felipe Contreras, Tony Lindgren, Felipe Contreras

No functional changes.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/mach-omap2/omap3-iommu.c |   72 ++++++++++++++++++------------------
 1 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/arch/arm/mach-omap2/omap3-iommu.c b/arch/arm/mach-omap2/omap3-iommu.c
index 367f36a..f4232ec 100644
--- a/arch/arm/mach-omap2/omap3-iommu.c
+++ b/arch/arm/mach-omap2/omap3-iommu.c
@@ -14,43 +14,31 @@
 
 #include <mach/iommu.h>
 
-#define OMAP3_MMU1_BASE	0x480bd400
-#define OMAP3_MMU2_BASE	0x5d000000
-#define OMAP3_MMU1_IRQ	24
-#define OMAP3_MMU2_IRQ	28
-
-static struct resource omap3_iommu_res[] = {
-	{ /* Camera ISP MMU */
-		.start		= OMAP3_MMU1_BASE,
-		.end		= OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
-		.flags		= IORESOURCE_MEM,
-	},
-	{
-		.start		= OMAP3_MMU1_IRQ,
-		.flags		= IORESOURCE_IRQ,
-	},
-	{ /* IVA2.2 MMU */
-		.start		= OMAP3_MMU2_BASE,
-		.end		= OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
-		.flags		= IORESOURCE_MEM,
-	},
-	{
-		.start		= OMAP3_MMU2_IRQ,
-		.flags		= IORESOURCE_IRQ,
-	},
+struct iommu_device {
+	resource_size_t base;
+	resource_size_t irq;
+	struct iommu_platform_data pdata;
+	struct resource res[2];
 };
-#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
 
-static const struct iommu_platform_data omap3_iommu_pdata[] __initconst = {
+static struct iommu_device devices[] = {
 	{
-		.name = "isp",
-		.nr_tlb_entries = 8,
-		.clk_name = "cam_ick",
+		.base = 0x480bd400,
+		.irq = 24,
+		.pdata = {
+			.name = "isp",
+			.nr_tlb_entries = 8,
+			.clk_name = "cam_ick",
+		},
 	},
 	{
-		.name = "iva2",
-		.nr_tlb_entries = 32,
-		.clk_name = "iva2_ck",
+		.base = 0x5d000000,
+		.irq = 28,
+		.pdata = {
+			.name = "iva2",
+			.nr_tlb_entries = 32,
+			.clk_name = "iva2_ck",
+		},
 	},
 };
 #define NR_IOMMU_DEVICES ARRAY_SIZE(omap3_iommu_pdata)
@@ -63,23 +51,35 @@ static int __init omap3_iommu_init(void)
 
 	for (i = 0; i < NR_IOMMU_DEVICES; i++) {
 		struct platform_device *pdev;
+		const struct iommu_device *d = &devices[i];
+		struct resource res[] = {
+			{ .flags = IORESOURCE_MEM },
+			{ .flags = IORESOURCE_IRQ },
+		};
 
 		pdev = platform_device_alloc("omap-iommu", i + 1);
 		if (!pdev) {
 			err = -ENOMEM;
 			goto err_out;
 		}
-		err = platform_device_add_resources(pdev,
-				    &omap3_iommu_res[2 * i], NR_IOMMU_RES);
+
+		res[0].start = d->base;
+		res[0].end = d->base + MMU_REG_SIZE - 1;
+
+		res[1].start = d->irq;
+
+		err = platform_device_add_resources(pdev, res, ARRAY_SIZE(res));
 		if (err)
 			goto err_out;
-		err = platform_device_add_data(pdev, &omap3_iommu_pdata[i],
-					       sizeof(omap3_iommu_pdata[0]));
+
+		err = platform_device_add_data(pdev, &d->pdata, sizeof(d->pdata));
 		if (err)
 			goto err_out;
+
 		err = platform_device_add(pdev);
 		if (err)
 			goto err_out;
+
 		omap3_iommu_pdev[i] = pdev;
 	}
 	return 0;
-- 
1.6.3.GIT


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

* [RFC/PATCH 2/3] omap3-iommu: split init function into omap_iommu_add
  2009-05-07 20:11           ` [RFC/PATCH 1/3] omap3-iommu: reorganize Felipe Contreras
@ 2009-05-07 20:11             ` Felipe Contreras
  2009-05-07 20:11               ` [RFC/PATCH 3/3] omap3-iommu: remote registration Felipe Contreras
  2009-05-07 21:14             ` [RFC/PATCH 1/3] omap3-iommu: reorganize Felipe Balbi
  1 sibling, 1 reply; 53+ messages in thread
From: Felipe Contreras @ 2009-05-07 20:11 UTC (permalink / raw)
  To: linux-omap
  Cc: Hiroshi DOYU, Felipe Contreras, Tony Lindgren, Felipe Contreras

In preparation for external registration.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/mach-omap2/omap3-iommu.c |   76 ++++++++++++++++++++++++-------------
 1 files changed, 49 insertions(+), 27 deletions(-)

diff --git a/arch/arm/mach-omap2/omap3-iommu.c b/arch/arm/mach-omap2/omap3-iommu.c
index f4232ec..149c624 100644
--- a/arch/arm/mach-omap2/omap3-iommu.c
+++ b/arch/arm/mach-omap2/omap3-iommu.c
@@ -45,43 +45,65 @@ static struct iommu_device devices[] = {
 
 static struct platform_device *omap3_iommu_pdev[NR_IOMMU_DEVICES];
 
-static int __init omap3_iommu_init(void)
+static struct platform_device *omap_iommu_add(const char *name)
 {
-	int i, err;
-
-	for (i = 0; i < NR_IOMMU_DEVICES; i++) {
-		struct platform_device *pdev;
-		const struct iommu_device *d = &devices[i];
-		struct resource res[] = {
-			{ .flags = IORESOURCE_MEM },
-			{ .flags = IORESOURCE_IRQ },
-		};
-
-		pdev = platform_device_alloc("omap-iommu", i + 1);
-		if (!pdev) {
-			err = -ENOMEM;
-			goto err_out;
+	struct platform_device *pdev;
+	const struct iommu_device *d = NULL;
+	struct resource res[] = {
+		{ .flags = IORESOURCE_MEM },
+		{ .flags = IORESOURCE_IRQ },
+	};
+	int err, i;
+
+	for (i = 0; i < ARRAY_SIZE(devices); i++)
+		if (strcmp(devices[i].pdata.name, name) == 0) {
+			d = &devices[i];
+			break;
 		}
 
-		res[0].start = d->base;
-		res[0].end = d->base + MMU_REG_SIZE - 1;
+	if (!d)
+		return NULL;
 
-		res[1].start = d->irq;
+	pdev = platform_device_alloc("omap-iommu", i + 1);
+	if (!pdev)
+		return NULL;
 
-		err = platform_device_add_resources(pdev, res, ARRAY_SIZE(res));
-		if (err)
-			goto err_out;
+	res[0].start = d->base;
+	res[0].end = d->base + MMU_REG_SIZE - 1;
 
-		err = platform_device_add_data(pdev, &d->pdata, sizeof(d->pdata));
-		if (err)
-			goto err_out;
+	res[1].start = d->irq;
 
-		err = platform_device_add(pdev);
-		if (err)
-			goto err_out;
+	err = platform_device_add_resources(pdev, res, ARRAY_SIZE(res));
+	if (err)
+		goto err_out;
+
+	err = platform_device_add_data(pdev, &d->pdata, sizeof(d->pdata));
+	if (err)
+		goto err_out;
+
+	err = platform_device_add(pdev);
+	if (err)
+		goto err_out;
+
+	return pdev;
 
+err_out:
+	platform_device_put(pdev);
+	return NULL;
+}
+
+static int __init omap3_iommu_init(void)
+{
+	struct platform_device *pdev;
+	int i, err;
+
+	for (i = 0; i < ARRAY_SIZE(devices); i++) {
+		pdev = omap_iommu_add(devices[i].pdata.name);
+		if (!pdev)
+			goto err_out;
 		omap3_iommu_pdev[i] = pdev;
 	}
+
 	return 0;
 
 err_out:
-- 
1.6.3.GIT


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

* [RFC/PATCH 3/3] omap3-iommu: remote registration
  2009-05-07 20:11             ` [RFC/PATCH 2/3] omap3-iommu: split init function into omap_iommu_add Felipe Contreras
@ 2009-05-07 20:11               ` Felipe Contreras
  0 siblings, 0 replies; 53+ messages in thread
From: Felipe Contreras @ 2009-05-07 20:11 UTC (permalink / raw)
  To: linux-omap
  Cc: Hiroshi DOYU, Felipe Contreras, Tony Lindgren, Felipe Contreras

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/mach-omap2/omap3-iommu.c       |   36 +-----------------------------
 arch/arm/plat-omap/include/mach/iommu.h |    2 +
 2 files changed, 4 insertions(+), 34 deletions(-)

diff --git a/arch/arm/mach-omap2/omap3-iommu.c b/arch/arm/mach-omap2/omap3-iommu.c
index 149c624..8380cd5 100644
--- a/arch/arm/mach-omap2/omap3-iommu.c
+++ b/arch/arm/mach-omap2/omap3-iommu.c
@@ -41,11 +41,8 @@ static struct iommu_device devices[] = {
 		},
 	},
 };
-#define NR_IOMMU_DEVICES ARRAY_SIZE(omap3_iommu_pdata)
 
-static struct platform_device *omap3_iommu_pdev[NR_IOMMU_DEVICES];
-
-static struct platform_device *omap_iommu_add(const char *name)
+struct platform_device *omap_iommu_add(const char *name)
 {
 	struct platform_device *pdev;
 	const struct iommu_device *d = NULL;
@@ -91,36 +88,7 @@ err_out:
 	platform_device_put(pdev);
 	return NULL;
 }
-
-static int __init omap3_iommu_init(void)
-{
-	struct platform_device *pdev;
-	int i, err;
-
-	for (i = 0; i < ARRAY_SIZE(devices); i++) {
-		pdev = omap_iommu_add(devices[i].pdata.name);
-		if (!pdev)
-			goto err_out;
-		omap3_iommu_pdev[i] = pdev;
-	}
-
-	return 0;
-
-err_out:
-	while (i--)
-		platform_device_put(omap3_iommu_pdev[i]);
-	return err;
-}
-module_init(omap3_iommu_init);
-
-static void __exit omap3_iommu_exit(void)
-{
-	int i;
-
-	for (i = 0; i < NR_IOMMU_DEVICES; i++)
-		platform_device_unregister(omap3_iommu_pdev[i]);
-}
-module_exit(omap3_iommu_exit);
+EXPORT_SYMBOL_GPL(omap_iommu_add);
 
 MODULE_AUTHOR("Hiroshi DOYU");
 MODULE_DESCRIPTION("omap iommu: omap3 device registration");
diff --git a/arch/arm/plat-omap/include/mach/iommu.h b/arch/arm/plat-omap/include/mach/iommu.h
index 769b00b..e22a4a4 100644
--- a/arch/arm/plat-omap/include/mach/iommu.h
+++ b/arch/arm/plat-omap/include/mach/iommu.h
@@ -165,4 +165,6 @@ extern int foreach_iommu_device(void *data,
 extern ssize_t iommu_dump_ctx(struct iommu *obj, char *buf);
 extern size_t dump_tlb_entries(struct iommu *obj, char *buf);
 
+struct platform_device *omap_iommu_add(const char *name);
+
 #endif /* __MACH_IOMMU_H */
-- 
1.6.3.GIT


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

* RE: [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration
  2009-05-07 20:11         ` [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration Felipe Contreras
  2009-05-07 20:11           ` [RFC/PATCH 1/3] omap3-iommu: reorganize Felipe Contreras
@ 2009-05-07 20:28           ` Kanigeri, Hari
  2009-05-07 20:39             ` Felipe Contreras
  2009-05-08  4:14           ` Hiroshi DOYU
  2 siblings, 1 reply; 53+ messages in thread
From: Kanigeri, Hari @ 2009-05-07 20:28 UTC (permalink / raw)
  To: Felipe Contreras, linux-omap
  Cc: Hiroshi DOYU, Felipe Contreras, Tony Lindgren

> Currently the iva2 code (tidspbridge) is not using iommu, therefore it
> can't be
> used as-is with the current omap iommu. 

Bridge can use IOMMU module that Hiroshi has sent by calling below 2 lines of code. 

struct iommu *iva_iommu_ptr;
iva_iommu_ptr = iommu_get("iva2");


Thank you,
Best regards,
Hari

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Felipe Contreras
> Sent: Thursday, May 07, 2009 3:11 PM
> To: linux-omap@vger.kernel.org
> Cc: Hiroshi DOYU; Felipe Contreras; Tony Lindgren; Felipe Contreras
> Subject: [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration
> 
> This patch series cleanups up a bit the opap3-iommu device registration
> and
> then allows registration from other parts of the code.
> 
> Currently the iva2 code (tidspbridge) is not using iommu, therefore it
> can't be
> used as-is with the current omap iommu. By allowing devies to be
> registered
> externaly (either isp, or iva2) this problem goes away.
> 
> Felipe Contreras (3):
>   omap3-iommu: reorganize
>   omap3-iommu: split init function into omap_iommu_add
>   omap3-iommu: remote registration
> 
>  arch/arm/mach-omap2/omap3-iommu.c       |  130 ++++++++++++++------------
> -----
>  arch/arm/plat-omap/include/mach/iommu.h |    2 +
>  2 files changed, 62 insertions(+), 70 deletions(-)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

* Re: [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration
  2009-05-07 20:28           ` [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration Kanigeri, Hari
@ 2009-05-07 20:39             ` Felipe Contreras
  2009-05-07 20:54               ` Kanigeri, Hari
  0 siblings, 1 reply; 53+ messages in thread
From: Felipe Contreras @ 2009-05-07 20:39 UTC (permalink / raw)
  To: Kanigeri, Hari; +Cc: linux-omap, Hiroshi DOYU, Felipe Contreras, Tony Lindgren

On Thu, May 7, 2009 at 11:28 PM, Kanigeri, Hari <h-kanigeri2@ti.com> wrote:
>> Currently the iva2 code (tidspbridge) is not using iommu, therefore it
>> can't be
>> used as-is with the current omap iommu.
>
> Bridge can use IOMMU module that Hiroshi has sent by calling below 2 lines of code.
>
> struct iommu *iva_iommu_ptr;
> iva_iommu_ptr = iommu_get("iva2");

>From where? Care to send an actual patch?

-- 
Felipe Contreras

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

* RE: [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration
  2009-05-07 20:39             ` Felipe Contreras
@ 2009-05-07 20:54               ` Kanigeri, Hari
  0 siblings, 0 replies; 53+ messages in thread
From: Kanigeri, Hari @ 2009-05-07 20:54 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: linux-omap, Hiroshi DOYU, Felipe Contreras, Tony Lindgren

> -----Original Message-----
> From: Felipe Contreras [mailto:felipe.contreras@gmail.com]
> Sent: Thursday, May 07, 2009 3:40 PM
> To: Kanigeri, Hari
> Cc: linux-omap@vger.kernel.org; Hiroshi DOYU; Felipe Contreras; Tony
> Lindgren
> Subject: Re: [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration
> 
> On Thu, May 7, 2009 at 11:28 PM, Kanigeri, Hari <h-kanigeri2@ti.com>
> wrote:
> >> Currently the iva2 code (tidspbridge) is not using iommu, therefore it
> >> can't be
> >> used as-is with the current omap iommu.
> >
> > Bridge can use IOMMU module that Hiroshi has sent by calling below 2
> lines of code.
> >
> > struct iommu *iva_iommu_ptr;
> > iva_iommu_ptr = iommu_get("iva2");
> 
> From where? Care to send an actual patch?
> 

I think it is in the initial set of patches that Hiroshi has sent some time ago.

He provided the link to it in this first email [patch 0/6].
http://marc.info/?l=linux-omap&m=122087083712670&w=2


Thank you,
Best regards,
Hari



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

* Re: [RFC/PATCH 1/3] omap3-iommu: reorganize
  2009-05-07 20:11           ` [RFC/PATCH 1/3] omap3-iommu: reorganize Felipe Contreras
  2009-05-07 20:11             ` [RFC/PATCH 2/3] omap3-iommu: split init function into omap_iommu_add Felipe Contreras
@ 2009-05-07 21:14             ` Felipe Balbi
  2009-05-07 22:16               ` Felipe Contreras
  2009-05-08  4:15               ` Hiroshi DOYU
  1 sibling, 2 replies; 53+ messages in thread
From: Felipe Balbi @ 2009-05-07 21:14 UTC (permalink / raw)
  To: ext Felipe Contreras
  Cc: linux-omap, Doyu Hiroshi (Nokia-D/Helsinki),
	Contreras Felipe (Nokia-D/Helsinki),
	Tony Lindgren

On Thu, May 07, 2009 at 10:11:07PM +0200, ext Felipe Contreras wrote:
> -static struct resource omap3_iommu_res[] = {
> -	{ /* Camera ISP MMU */
> -		.start		= OMAP3_MMU1_BASE,
> -		.end		= OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
> -		.flags		= IORESOURCE_MEM,
> -	},
> -	{
> -		.start		= OMAP3_MMU1_IRQ,
> -		.flags		= IORESOURCE_IRQ,
> -	},
> -	{ /* IVA2.2 MMU */
> -		.start		= OMAP3_MMU2_BASE,
> -		.end		= OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
> -		.flags		= IORESOURCE_MEM,
> -	},
> -	{
> -		.start		= OMAP3_MMU2_IRQ,
> -		.flags		= IORESOURCE_IRQ,
> -	},

i find this one much more readable :-s

> +struct iommu_device {
> +	resource_size_t base;
> +	resource_size_t irq;
> +	struct iommu_platform_data pdata;

generally, platform_data is a void *. Don't know if it matters here.

> +	struct resource res[2];
>  };
> -#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
>  
> -static const struct iommu_platform_data omap3_iommu_pdata[] __initconst = {
> +static struct iommu_device devices[] = {
>  	{
> -		.name = "isp",
> -		.nr_tlb_entries = 8,
> -		.clk_name = "cam_ick",
> +		.base = 0x480bd400,
> +		.irq = 24,
> +		.pdata = {
> +			.name = "isp",
> +			.nr_tlb_entries = 8,
> +			.clk_name = "cam_ick",

still passing clk names ? what about clkdev ?

-- 
balbi

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

* Re: [RFC/PATCH 1/3] omap3-iommu: reorganize
  2009-05-07 21:14             ` [RFC/PATCH 1/3] omap3-iommu: reorganize Felipe Balbi
@ 2009-05-07 22:16               ` Felipe Contreras
  2009-05-08  4:15               ` Hiroshi DOYU
  1 sibling, 0 replies; 53+ messages in thread
From: Felipe Contreras @ 2009-05-07 22:16 UTC (permalink / raw)
  To: felipe.balbi
  Cc: linux-omap, Doyu Hiroshi (Nokia-D/Helsinki),
	Contreras Felipe (Nokia-D/Helsinki),
	Tony Lindgren

On Fri, May 8, 2009 at 12:14 AM, Felipe Balbi <felipe.balbi@nokia.com> wrote:
> On Thu, May 07, 2009 at 10:11:07PM +0200, ext Felipe Contreras wrote:
>> -static struct resource omap3_iommu_res[] = {
>> -     { /* Camera ISP MMU */
>> -             .start          = OMAP3_MMU1_BASE,
>> -             .end            = OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
>> -             .flags          = IORESOURCE_MEM,
>> -     },
>> -     {
>> -             .start          = OMAP3_MMU1_IRQ,
>> -             .flags          = IORESOURCE_IRQ,
>> -     },
>> -     { /* IVA2.2 MMU */
>> -             .start          = OMAP3_MMU2_BASE,
>> -             .end            = OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
>> -             .flags          = IORESOURCE_MEM,
>> -     },
>> -     {
>> -             .start          = OMAP3_MMU2_IRQ,
>> -             .flags          = IORESOURCE_IRQ,
>> -     },
>
> i find this one much more readable :-s

You find an array of 'struct resource' more readable than this?

+               .base = 0x480bd400,
+               .irq = 24,

+               .base = 0x5d000000,
+               .irq = 28,

+               res[0].start = d->base;
+               res[0].end = d->base + MMU_REG_SIZE - 1;
+
+               res[1].start = d->irq;

How would you calculate the number of resources per iommu device?

How about these?

-               err = platform_device_add_resources(pdev,
-                                   &omap3_iommu_res[2 * i], NR_IOMMU_RES);
+               err = platform_device_add_resources(pdev, res, ARRAY_SIZE(res));
               if (err)
                       goto err_out;
-               err = platform_device_add_data(pdev, &omap3_iommu_pdata[i],
-                                              sizeof(omap3_iommu_pdata[0]));
+               err = platform_device_add_data(pdev, &d->pdata,
sizeof(d->pdata));
               if (err)
                       goto err_out;

Do you find the original version easier to read?

>> +struct iommu_device {
>> +     resource_size_t base;
>> +     resource_size_t irq;
>> +     struct iommu_platform_data pdata;
>
> generally, platform_data is a void *. Don't know if it matters here.

Yes but we need to set the actual contents for platform_device_add_data.

>> +     struct resource res[2];
>>  };
>> -#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
>>
>> -static const struct iommu_platform_data omap3_iommu_pdata[] __initconst = {
>> +static struct iommu_device devices[] = {
>>       {
>> -             .name = "isp",
>> -             .nr_tlb_entries = 8,
>> -             .clk_name = "cam_ick",
>> +             .base = 0x480bd400,
>> +             .irq = 24,
>> +             .pdata = {
>> +                     .name = "isp",
>> +                     .nr_tlb_entries = 8,
>> +                     .clk_name = "cam_ick",
>
> still passing clk names ? what about clkdev ?

That's for iommu to fix. I'm simply interested on decoupling iommu and
a hypothetical tidspbridge that doesn't exists yet.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-05-07 20:05   ` [PATCH 3/6] omap iommu: omap3 iommu device registration Felipe Contreras
@ 2009-05-08  4:10     ` Hiroshi DOYU
  2009-05-08  7:32       ` Felipe Contreras
  0 siblings, 1 reply; 53+ messages in thread
From: Hiroshi DOYU @ 2009-05-08  4:10 UTC (permalink / raw)
  To: felipe.contreras
  Cc: linux-arm-kernel, linux-omap, h-kanigeri2, omar.ramirez,
	sakari.ailus, tony

Hi Felipe,

From: ext Felipe Contreras <felipe.contreras@gmail.com>
Subject: Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
Date: Thu, 7 May 2009 22:05:00 +0200

> On Tue, May 5, 2009 at 3:47 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> > Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> > ---
> >
> >  arch/arm/mach-omap2/omap3-iommu.c |  105 +++++++++++++++++++++++++++++++++++++
> >  1 files changed, 105 insertions(+), 0 deletions(-)
> >  create mode 100644 arch/arm/mach-omap2/omap3-iommu.c
> >
> > diff --git a/arch/arm/mach-omap2/omap3-iommu.c b/arch/arm/mach-omap2/omap3-iommu.c
> > new file mode 100644
> > index 0000000..367f36a
> > --- /dev/null
> > +++ b/arch/arm/mach-omap2/omap3-iommu.c
> > @@ -0,0 +1,105 @@
> > +/*
> > + * omap iommu: omap3 device registration
> > + *
> > + * Copyright (C) 2008-2009 Nokia Corporation
> > + *
> > + * Written by Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/platform_device.h>
> > +
> > +#include <mach/iommu.h>
> > +
> > +#define OMAP3_MMU1_BASE        0x480bd400
> > +#define OMAP3_MMU2_BASE        0x5d000000
> > +#define OMAP3_MMU1_IRQ 24
> > +#define OMAP3_MMU2_IRQ 28
> > +
> > +static struct resource omap3_iommu_res[] = {
> > +       { /* Camera ISP MMU */
> > +               .start          = OMAP3_MMU1_BASE,
> > +               .end            = OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
> > +               .flags          = IORESOURCE_MEM,
> > +       },
> > +       {
> > +               .start          = OMAP3_MMU1_IRQ,
> > +               .flags          = IORESOURCE_IRQ,
> > +       },
> > +       { /* IVA2.2 MMU */
> > +               .start          = OMAP3_MMU2_BASE,
> > +               .end            = OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
> > +               .flags          = IORESOURCE_MEM,
> > +       },
> > +       {
> > +               .start          = OMAP3_MMU2_IRQ,
> > +               .flags          = IORESOURCE_IRQ,
> > +       },
> > +};
> > +#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
> 
> <snip/>
> 
> > +               err = platform_device_add_resources(pdev,
> > +                                   &omap3_iommu_res[2 * i], NR_IOMMU_RES);
> 
> This is wrong. In this particular case NR_IOMMU_RES is 2, but not
> because there's 2 resources: MEM and IRQ, but because there's 2 iommu
> devices: isp and iva2.

Does the following make sense?

+#define NR_IOMMU_RES 2

....

+               err = platform_device_add_resources(pdev,
+                                   omap3_iommu_res +  i * NR_IOMMU_RES, NR_IOMMU_RES);
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration
  2009-05-07 20:11         ` [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration Felipe Contreras
  2009-05-07 20:11           ` [RFC/PATCH 1/3] omap3-iommu: reorganize Felipe Contreras
  2009-05-07 20:28           ` [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration Kanigeri, Hari
@ 2009-05-08  4:14           ` Hiroshi DOYU
  2009-05-08  8:04             ` Felipe Contreras
  2 siblings, 1 reply; 53+ messages in thread
From: Hiroshi DOYU @ 2009-05-08  4:14 UTC (permalink / raw)
  To: felipe.contreras; +Cc: linux-omap, felipe.contreras, tony, h-kanigeri2

Hi Felipe,

From: ext Felipe Contreras <felipe.contreras@gmail.com>
Subject: [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration
Date: Thu, 7 May 2009 22:11:06 +0200

> This patch series cleanups up a bit the opap3-iommu device registration and
> then allows registration from other parts of the code.

I think that this part of code is just a simple conventional
platform_device registration and there may be no need to add more
complexicity by introducing a new special structure/function just for
a workaround of a client module("tidspbridge"). Also having these
device registrations here and there doesn't look so nice, looking at
other ones around kernel.

> Currently the iva2 code (tidspbridge) is not using iommu, therefore it can't be
> used as-is with the current omap iommu. By allowing devies to be registered
> externaly (either isp, or iva2) this problem goes away.

Fixing tidspbridge itself is the way to go, and it seems that Hari has
already verified this iommu with tidspbridge and he's pointed out some
of missing features(*1). I hope that tidspbridge will start to use
iommu soon.

*1: http://marc.info/?l=linux-omap&m=124148814309478&w=2





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

* Re: [RFC/PATCH 1/3] omap3-iommu: reorganize
  2009-05-07 21:14             ` [RFC/PATCH 1/3] omap3-iommu: reorganize Felipe Balbi
  2009-05-07 22:16               ` Felipe Contreras
@ 2009-05-08  4:15               ` Hiroshi DOYU
  1 sibling, 0 replies; 53+ messages in thread
From: Hiroshi DOYU @ 2009-05-08  4:15 UTC (permalink / raw)
  To: felipe.balbi; +Cc: felipe.contreras, linux-omap, felipe.contreras, tony

From: "Balbi Felipe (Nokia-D/Helsinki)" <felipe.balbi@nokia.com>
Subject: Re: [RFC/PATCH 1/3] omap3-iommu: reorganize
Date: Thu, 7 May 2009 23:14:47 +0200

> On Thu, May 07, 2009 at 10:11:07PM +0200, ext Felipe Contreras wrote:
> > -static struct resource omap3_iommu_res[] = {
> > -	{ /* Camera ISP MMU */
> > -		.start		= OMAP3_MMU1_BASE,
> > -		.end		= OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
> > -		.flags		= IORESOURCE_MEM,
> > -	},
> > -	{
> > -		.start		= OMAP3_MMU1_IRQ,
> > -		.flags		= IORESOURCE_IRQ,
> > -	},
> > -	{ /* IVA2.2 MMU */
> > -		.start		= OMAP3_MMU2_BASE,
> > -		.end		= OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
> > -		.flags		= IORESOURCE_MEM,
> > -	},
> > -	{
> > -		.start		= OMAP3_MMU2_IRQ,
> > -		.flags		= IORESOURCE_IRQ,
> > -	},
> 
> i find this one much more readable :-s
> 
> > +struct iommu_device {
> > +	resource_size_t base;
> > +	resource_size_t irq;
> > +	struct iommu_platform_data pdata;
> 
> generally, platform_data is a void *. Don't know if it matters here.
> 
> > +	struct resource res[2];
> >  };
> > -#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
> >  
> > -static const struct iommu_platform_data omap3_iommu_pdata[] __initconst = {
> > +static struct iommu_device devices[] = {
> >  	{
> > -		.name = "isp",
> > -		.nr_tlb_entries = 8,
> > -		.clk_name = "cam_ick",
> > +		.base = 0x480bd400,
> > +		.irq = 24,
> > +		.pdata = {
> > +			.name = "isp",
> > +			.nr_tlb_entries = 8,
> > +			.clk_name = "cam_ick",
> 
> still passing clk names ? what about clkdev ?

Useful comment, I'll update mine;) thanks

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

* Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-05-08  4:10     ` Hiroshi DOYU
@ 2009-05-08  7:32       ` Felipe Contreras
  2009-05-08 17:21         ` Aguirre Rodriguez, Sergio Alberto
  0 siblings, 1 reply; 53+ messages in thread
From: Felipe Contreras @ 2009-05-08  7:32 UTC (permalink / raw)
  To: Hiroshi DOYU
  Cc: linux-arm-kernel, linux-omap, h-kanigeri2, omar.ramirez,
	sakari.ailus, tony

On Fri, May 8, 2009 at 7:10 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> Hi Felipe,
>
> From: ext Felipe Contreras <felipe.contreras@gmail.com>
> Subject: Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
> Date: Thu, 7 May 2009 22:05:00 +0200
>
>> On Tue, May 5, 2009 at 3:47 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
>> > Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
>> > ---
>> >
>> >  arch/arm/mach-omap2/omap3-iommu.c |  105 +++++++++++++++++++++++++++++++++++++
>> >  1 files changed, 105 insertions(+), 0 deletions(-)
>> >  create mode 100644 arch/arm/mach-omap2/omap3-iommu.c
>> >
>> > diff --git a/arch/arm/mach-omap2/omap3-iommu.c b/arch/arm/mach-omap2/omap3-iommu.c
>> > new file mode 100644
>> > index 0000000..367f36a
>> > --- /dev/null
>> > +++ b/arch/arm/mach-omap2/omap3-iommu.c
>> > @@ -0,0 +1,105 @@
>> > +/*
>> > + * omap iommu: omap3 device registration
>> > + *
>> > + * Copyright (C) 2008-2009 Nokia Corporation
>> > + *
>> > + * Written by Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
>> > + *
>> > + * This program is free software; you can redistribute it and/or modify
>> > + * it under the terms of the GNU General Public License version 2 as
>> > + * published by the Free Software Foundation.
>> > + */
>> > +
>> > +#include <linux/platform_device.h>
>> > +
>> > +#include <mach/iommu.h>
>> > +
>> > +#define OMAP3_MMU1_BASE        0x480bd400
>> > +#define OMAP3_MMU2_BASE        0x5d000000
>> > +#define OMAP3_MMU1_IRQ 24
>> > +#define OMAP3_MMU2_IRQ 28
>> > +
>> > +static struct resource omap3_iommu_res[] = {
>> > +       { /* Camera ISP MMU */
>> > +               .start          = OMAP3_MMU1_BASE,
>> > +               .end            = OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
>> > +               .flags          = IORESOURCE_MEM,
>> > +       },
>> > +       {
>> > +               .start          = OMAP3_MMU1_IRQ,
>> > +               .flags          = IORESOURCE_IRQ,
>> > +       },
>> > +       { /* IVA2.2 MMU */
>> > +               .start          = OMAP3_MMU2_BASE,
>> > +               .end            = OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
>> > +               .flags          = IORESOURCE_MEM,
>> > +       },
>> > +       {
>> > +               .start          = OMAP3_MMU2_IRQ,
>> > +               .flags          = IORESOURCE_IRQ,
>> > +       },
>> > +};
>> > +#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
>>
>> <snip/>
>>
>> > +               err = platform_device_add_resources(pdev,
>> > +                                   &omap3_iommu_res[2 * i], NR_IOMMU_RES);
>>
>> This is wrong. In this particular case NR_IOMMU_RES is 2, but not
>> because there's 2 resources: MEM and IRQ, but because there's 2 iommu
>> devices: isp and iva2.
>
> Does the following make sense?
>
> +#define NR_IOMMU_RES 2
>
> ....
>
> +               err = platform_device_add_resources(pdev,
> +                                   omap3_iommu_res +  i * NR_IOMMU_RES, NR_IOMMU_RES);

Yeap, also:

> +               err = platform_device_add_resources(pdev, omap3_iommu_res +  i * 2, 2);

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration
  2009-05-08  4:14           ` Hiroshi DOYU
@ 2009-05-08  8:04             ` Felipe Contreras
  0 siblings, 0 replies; 53+ messages in thread
From: Felipe Contreras @ 2009-05-08  8:04 UTC (permalink / raw)
  To: Hiroshi DOYU; +Cc: linux-omap, felipe.contreras, tony, h-kanigeri2

On Fri, May 8, 2009 at 7:14 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> Hi Felipe,
>
> From: ext Felipe Contreras <felipe.contreras@gmail.com>
> Subject: [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration
> Date: Thu, 7 May 2009 22:11:06 +0200
>
>> This patch series cleanups up a bit the opap3-iommu device registration and
>> then allows registration from other parts of the code.
>
> I think that this part of code is just a simple conventional
> platform_device registration and there may be no need to add more
> complexicity by introducing a new special structure/function just for
> a workaround of a client module("tidspbridge"). Also having these
> device registrations here and there doesn't look so nice, looking at
> other ones around kernel.

First of all, it's not a workaround. Fake dependencies are bad. iommu
should not depend on isp or iva2 (or how they are implemented), it's
the other way around.

Let's say you disable isp, do you want iommu to add the isp iommu
device (same for iva2)? What happens if you don't want either isp or
iva2 in the kernel, what is the point of having iommu?

Second, it's not introducing any new structure, the structure is
internal, you can remove it if you want and implement the same with a
select or any way you want, I just thought an internal structure was
easier to follow.

And third, this kind of device registration is used all over the place, look at:
omap_mmc_add
pxa2xx_set_spi_info
at32_add_device_psif
at32_add_device_twi
at32_add_device_mci
at32_add_device_pwm
at32_add_device_usba
at32_add_device_ide
at32_add_device_cf
at32_add_device_nand
at32_add_device_ac97c
at32_add_device_abdac
txx9_ethaddr_init
txx9_physmap_flash_init
txx9_iocled_init
mv64x60_mpsc_device_setup
mv64x60_eth_device_setup
mv64x60_i2c_device_setup
mv64x60_wdt_device_setup
ipmi_bmc_register
aem_init_aem1_inst
aem_init_aem2_inst
w1_ds2760_add_slave
of_snd_soc_register_device

>> Currently the iva2 code (tidspbridge) is not using iommu, therefore it can't be
>> used as-is with the current omap iommu. By allowing devies to be registered
>> externaly (either isp, or iva2) this problem goes away.
>
> Fixing tidspbridge itself is the way to go, and it seems that Hari has
> already verified this iommu with tidspbridge and he's pointed out some
> of missing features(*1). I hope that tidspbridge will start to use
> iommu soon.

Yes, definitely, but that's not an excuse not to make iommu more
extensible. With my patch the iommu can be merged as is and would not
*require* any change in tidspbridge. When the tidspbridge is ready, it
would be a matter of adding one line.

Also, it's better to have independent branches. It would be ugly to
modify iommu code from the tidspbridge branch depending on whether or
not the migration to iommu have happened.

And finally, suppose you load the bridgedriver on-demand, with my
patch the iva2 iommu device will be created *only* when the
bridgedriver is loaded, and when the bridgedriver itself is unloaded
it can remove the iva2 iommu device. So essentially we'll have only
the iommu devices that we really need.

Now I ask the question the other way around, what do we loose if we
apply these patches?

-- 
Felipe Contreras

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

* RE: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-05-08  7:32       ` Felipe Contreras
@ 2009-05-08 17:21         ` Aguirre Rodriguez, Sergio Alberto
  2009-05-08 22:27           ` Felipe Contreras
  0 siblings, 1 reply; 53+ messages in thread
From: Aguirre Rodriguez, Sergio Alberto @ 2009-05-08 17:21 UTC (permalink / raw)
  To: Felipe Contreras, Hiroshi DOYU
  Cc: linux-arm-kernel, linux-omap, Kanigeri, Hari, Ramirez Luna, Omar,
	sakari.ailus, tony

Hi,

Just one comment below:

> > Does the following make sense?
> >
> > +#define NR_IOMMU_RES 2
> >
> > ....
> >
> > +               err = platform_device_add_resources(pdev,
> > +                                   omap3_iommu_res +  i * NR_IOMMU_RES,
> NR_IOMMU_RES);
> 
> Yeap, also:
> 
> > +               err = platform_device_add_resources(pdev,
> omap3_iommu_res +  i * 2, 2);

IMHO, I don't think it's a good idea to add magical numbers to any code in the kernel. Why is it better to NOT use a define?

Regards,
Sergio
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-05-08 17:21         ` Aguirre Rodriguez, Sergio Alberto
@ 2009-05-08 22:27           ` Felipe Contreras
  0 siblings, 0 replies; 53+ messages in thread
From: Felipe Contreras @ 2009-05-08 22:27 UTC (permalink / raw)
  To: Aguirre Rodriguez, Sergio Alberto
  Cc: Hiroshi DOYU, linux-arm-kernel, linux-omap, Kanigeri, Hari,
	Ramirez Luna, Omar, sakari.ailus, tony

On Fri, May 8, 2009 at 8:21 PM, Aguirre Rodriguez, Sergio Alberto
<saaguirre@ti.com> wrote:
> Hi,
>
> Just one comment below:
>
>> > Does the following make sense?
>> >
>> > +#define NR_IOMMU_RES 2
>> >
>> > ....
>> >
>> > +               err = platform_device_add_resources(pdev,
>> > +                                   omap3_iommu_res +  i * NR_IOMMU_RES,
>> NR_IOMMU_RES);
>>
>> Yeap, also:
>>
>> > +               err = platform_device_add_resources(pdev,
>> omap3_iommu_res +  i * 2, 2);
>
> IMHO, I don't think it's a good idea to add magical numbers to any code in the kernel. Why is it better to NOT use a define?

I agree, but even a define is not good enough. For example you can
update the array without updating the define.

A better solution is to do what I did on my follow up patch series:

+               struct resource res[] = {
+                       { .flags = IORESOURCE_MEM },
+                       { .flags = IORESOURCE_IRQ },
+               };

+               err = platform_device_add_resources(pdev, res, ARRAY_SIZE(res));

No need for a define, and as soon as the array is updated so will the
second argument sent to platform_device_add_resources.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-05-05 12:47 ` [PATCH 3/6] omap iommu: omap3 iommu device registration Hiroshi DOYU
  2009-05-05 19:32   ` Felipe Contreras
  2009-05-07 20:05   ` [PATCH 3/6] omap iommu: omap3 iommu device registration Felipe Contreras
@ 2009-05-16  9:20   ` Russell King - ARM Linux
  2009-05-16  9:38     ` Felipe Contreras
                       ` (2 more replies)
  2 siblings, 3 replies; 53+ messages in thread
From: Russell King - ARM Linux @ 2009-05-16  9:20 UTC (permalink / raw)
  To: Hiroshi DOYU
  Cc: linux-arm-kernel, linux-omap, h-kanigeri2, omar.ramirez,
	sakari.ailus, tony

On Tue, May 05, 2009 at 03:47:00PM +0300, Hiroshi DOYU wrote:
> +static struct resource omap3_iommu_res[] = {
> +	{ /* Camera ISP MMU */
> +		.start		= OMAP3_MMU1_BASE,
> +		.end		= OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
> +		.flags		= IORESOURCE_MEM,
> +	},
> +	{
> +		.start		= OMAP3_MMU1_IRQ,
> +		.flags		= IORESOURCE_IRQ,
> +	},
> +	{ /* IVA2.2 MMU */
> +		.start		= OMAP3_MMU2_BASE,
> +		.end		= OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
> +		.flags		= IORESOURCE_MEM,
> +	},
> +	{
> +		.start		= OMAP3_MMU2_IRQ,
> +		.flags		= IORESOURCE_IRQ,
> +	},
> +};
> +#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)

This looks all very convoluted.  Why not do something like:

static unsigned long iommu_base[] = {
	OMAP3_MMU1_BASE,
	OMAP3_MMU2_BASE,
};

static int iommu_irq[] = {
	OMAP3_MMU1_IRQ,
	OMAP3_MMU2_IRQ,
};

> +static struct platform_device *omap3_iommu_pdev[NR_IOMMU_DEVICES];
> +
> +static int __init omap3_iommu_init(void)
> +{
> +	int i, err;
> +
> +	for (i = 0; i < NR_IOMMU_DEVICES; i++) {
> +		struct platform_device *pdev;
> +
> +		pdev = platform_device_alloc("omap-iommu", i + 1);

Why number them 1 and up when zero is perfectly fine?

> +		if (!pdev) {
> +			err = -ENOMEM;
> +			goto err_out;
> +		}
> +		err = platform_device_add_resources(pdev,
> +				    &omap3_iommu_res[2 * i], NR_IOMMU_RES);

		struct resource res[2];

		res[0].start = iommu_base[i];
		res[0].end = iommu_base[i] + MMU_REG_SIZE - 1;
		res[0].flags = IORESOURCE_MEM;
		res[1].start = res[1].end = iommu_irq[i];
		res[1].flags = IORESOURCE_IRQ;

		err = platform_device_add_resources(pdev, &res, ARRAY_SIZE(res));

There's no need to keep 'res' around since it's copied by add_resources.

> +		if (err)
> +			goto err_out;
> +		err = platform_device_add_data(pdev, &omap3_iommu_pdata[i],
> +					       sizeof(omap3_iommu_pdata[0]));
> +		if (err)
> +			goto err_out;
> +		err = platform_device_add(pdev);
> +		if (err)
> +			goto err_out;
> +		omap3_iommu_pdev[i] = pdev;
> +	}
> +	return 0;
> +
> +err_out:
> +	while (i--)
> +		platform_device_put(omap3_iommu_pdev[i]);
> +	return err;
> +}
> +module_init(omap3_iommu_init);
> +
> +static void __exit omap3_iommu_exit(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < NR_IOMMU_DEVICES; i++)
> +		platform_device_unregister(omap3_iommu_pdev[i]);
> +}
> +module_exit(omap3_iommu_exit);
> +
> +MODULE_AUTHOR("Hiroshi DOYU");
> +MODULE_DESCRIPTION("omap iommu: omap3 device registration");
> +MODULE_LICENSE("GPL v2");
> 
> 
> -------------------------------------------------------------------
> List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
> FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
> Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php

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

* Re: [PATCH 4/6] omap iommu: simple virtual address space management
  2009-05-05 12:47 ` [PATCH 4/6] omap iommu: simple virtual address space management Hiroshi DOYU
@ 2009-05-16  9:22   ` Russell King - ARM Linux
  2009-05-18  6:31     ` Hiroshi DOYU
  0 siblings, 1 reply; 53+ messages in thread
From: Russell King - ARM Linux @ 2009-05-16  9:22 UTC (permalink / raw)
  To: Hiroshi DOYU
  Cc: linux-arm-kernel, linux-omap, h-kanigeri2, omar.ramirez,
	sakari.ailus, tony

On Tue, May 05, 2009 at 03:47:06PM +0300, Hiroshi DOYU wrote:
> +int ioremap_page(unsigned long virt, unsigned long phys, unsigned int mtype)
> +{
> +	const struct mem_type *type;
> +
> +	type = get_mem_type(mtype);
> +	if (!type)
> +		return -EINVAL;

I think it would make more sense to move this lookup into the caller
for this - you're going to be making quite a lot of calls into
ioremap_page() so you really don't want to be doing the same lookup
every time.

> +/* map 'sglist' to a contiguous mpu virtual area and return 'va' */
> +static void *vmap_sg(const struct sg_table *sgt)
> +{
> +	u32 va;
> +	size_t total;
> +	unsigned int i;
> +	struct scatterlist *sg;
> +	struct vm_struct *new;
> +
> +	total = sgtable_len(sgt);
> +	if (!total)
> +		return ERR_PTR(-EINVAL);
> +
> +	new = __get_vm_area(total, VM_IOREMAP, VMALLOC_START, VMALLOC_END);
> +	if (!new)
> +		return ERR_PTR(-ENOMEM);
> +	va = (u32)new->addr;

In other words, move it here.

> +
> +	for_each_sg(sgt->sgl, sg, sgt->nents, i) {
> +		size_t bytes;
> +		u32 pa;
> +		int err;
> +
> +		pa = sg_phys(sg);
> +		bytes = sg_dma_len(sg);
> +
> +		BUG_ON(bytes != PAGE_SIZE);
> +
> +		err = ioremap_page(va,  pa, MT_DEVICE);
> +		if (err)
> +			goto err_out;
> +
> +		va += bytes;
> +	}
> +
> +	flush_cache_vmap(new->addr, total);
> +	return new->addr;
> +
> +err_out:
> +	WARN_ON(1); /* FIXME: cleanup some mpu mappings */
> +	vunmap(new->addr);
> +	return ERR_PTR(-EAGAIN);
> +}

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

* Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-05-16  9:20   ` Russell King - ARM Linux
@ 2009-05-16  9:38     ` Felipe Contreras
  2009-05-16  9:54       ` Russell King - ARM Linux
  2009-05-16  9:55     ` Russell King - ARM Linux
  2009-05-18  5:22     ` [PATCH 3/6] omap iommu: omap3 iommu device registration Hiroshi DOYU
  2 siblings, 1 reply; 53+ messages in thread
From: Felipe Contreras @ 2009-05-16  9:38 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Hiroshi DOYU, linux-arm-kernel, linux-omap, h-kanigeri2,
	omar.ramirez, sakari.ailus, tony

On Sat, May 16, 2009 at 12:20 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, May 05, 2009 at 03:47:00PM +0300, Hiroshi DOYU wrote:
>> +static struct resource omap3_iommu_res[] = {
>> +     { /* Camera ISP MMU */
>> +             .start          = OMAP3_MMU1_BASE,
>> +             .end            = OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
>> +             .flags          = IORESOURCE_MEM,
>> +     },
>> +     {
>> +             .start          = OMAP3_MMU1_IRQ,
>> +             .flags          = IORESOURCE_IRQ,
>> +     },
>> +     { /* IVA2.2 MMU */
>> +             .start          = OMAP3_MMU2_BASE,
>> +             .end            = OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
>> +             .flags          = IORESOURCE_MEM,
>> +     },
>> +     {
>> +             .start          = OMAP3_MMU2_IRQ,
>> +             .flags          = IORESOURCE_IRQ,
>> +     },
>> +};
>> +#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
>
> This looks all very convoluted.  Why not do something like:
>
> static unsigned long iommu_base[] = {
>        OMAP3_MMU1_BASE,
>        OMAP3_MMU2_BASE,
> };
>
> static int iommu_irq[] = {
>        OMAP3_MMU1_IRQ,
>        OMAP3_MMU2_IRQ,
> };

All your comments are pretty similar to my reorganize patch:
http://marc.info/?l=linux-omap&m=124172711303076&w=2

-- 
Felipe Contreras

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

* Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-05-16  9:38     ` Felipe Contreras
@ 2009-05-16  9:54       ` Russell King - ARM Linux
  2009-05-16  9:56         ` Felipe Contreras
  0 siblings, 1 reply; 53+ messages in thread
From: Russell King - ARM Linux @ 2009-05-16  9:54 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Hiroshi DOYU, linux-arm-kernel, linux-omap, h-kanigeri2,
	omar.ramirez, sakari.ailus, tony

On Sat, May 16, 2009 at 12:38:23PM +0300, Felipe Contreras wrote:
> On Sat, May 16, 2009 at 12:20 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Tue, May 05, 2009 at 03:47:00PM +0300, Hiroshi DOYU wrote:
> >> +static struct resource omap3_iommu_res[] = {
> >> +     { /* Camera ISP MMU */
> >> +             .start          = OMAP3_MMU1_BASE,
> >> +             .end            = OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
> >> +             .flags          = IORESOURCE_MEM,
> >> +     },
> >> +     {
> >> +             .start          = OMAP3_MMU1_IRQ,
> >> +             .flags          = IORESOURCE_IRQ,
> >> +     },
> >> +     { /* IVA2.2 MMU */
> >> +             .start          = OMAP3_MMU2_BASE,
> >> +             .end            = OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
> >> +             .flags          = IORESOURCE_MEM,
> >> +     },
> >> +     {
> >> +             .start          = OMAP3_MMU2_IRQ,
> >> +             .flags          = IORESOURCE_IRQ,
> >> +     },
> >> +};
> >> +#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
> >
> > This looks all very convoluted.  Why not do something like:
> >
> > static unsigned long iommu_base[] = {
> >        OMAP3_MMU1_BASE,
> >        OMAP3_MMU2_BASE,
> > };
> >
> > static int iommu_irq[] = {
> >        OMAP3_MMU1_IRQ,
> >        OMAP3_MMU2_IRQ,
> > };
> 
> All your comments are pretty similar to my reorganize patch:
> http://marc.info/?l=linux-omap&m=124172711303076&w=2

... which hasn't been posted to linux-arm-kernel, and so I won't see it.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-05-16  9:20   ` Russell King - ARM Linux
  2009-05-16  9:38     ` Felipe Contreras
@ 2009-05-16  9:55     ` Russell King - ARM Linux
  2009-05-18  5:36       ` Hiroshi DOYU
                         ` (2 more replies)
  2009-05-18  5:22     ` [PATCH 3/6] omap iommu: omap3 iommu device registration Hiroshi DOYU
  2 siblings, 3 replies; 53+ messages in thread
From: Russell King - ARM Linux @ 2009-05-16  9:55 UTC (permalink / raw)
  To: Hiroshi DOYU
  Cc: linux-arm-kernel, linux-omap, h-kanigeri2, omar.ramirez,
	sakari.ailus, tony

On Sat, May 16, 2009 at 10:20:36AM +0100, Russell King - ARM Linux wrote:
> This looks all very convoluted.  Why not do something like:
> 
> static unsigned long iommu_base[] = {
> 	OMAP3_MMU1_BASE,
> 	OMAP3_MMU2_BASE,
> };
> 
> static int iommu_irq[] = {
> 	OMAP3_MMU1_IRQ,
> 	OMAP3_MMU2_IRQ,
> };

BTW, both of these can be __initdata.

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

* Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-05-16  9:54       ` Russell King - ARM Linux
@ 2009-05-16  9:56         ` Felipe Contreras
  0 siblings, 0 replies; 53+ messages in thread
From: Felipe Contreras @ 2009-05-16  9:56 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Hiroshi DOYU, linux-arm-kernel, linux-omap, h-kanigeri2,
	omar.ramirez, sakari.ailus, tony

On Sat, May 16, 2009 at 12:54 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sat, May 16, 2009 at 12:38:23PM +0300, Felipe Contreras wrote:
>> On Sat, May 16, 2009 at 12:20 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Tue, May 05, 2009 at 03:47:00PM +0300, Hiroshi DOYU wrote:
>> >> +static struct resource omap3_iommu_res[] = {
>> >> +     { /* Camera ISP MMU */
>> >> +             .start          = OMAP3_MMU1_BASE,
>> >> +             .end            = OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
>> >> +             .flags          = IORESOURCE_MEM,
>> >> +     },
>> >> +     {
>> >> +             .start          = OMAP3_MMU1_IRQ,
>> >> +             .flags          = IORESOURCE_IRQ,
>> >> +     },
>> >> +     { /* IVA2.2 MMU */
>> >> +             .start          = OMAP3_MMU2_BASE,
>> >> +             .end            = OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
>> >> +             .flags          = IORESOURCE_MEM,
>> >> +     },
>> >> +     {
>> >> +             .start          = OMAP3_MMU2_IRQ,
>> >> +             .flags          = IORESOURCE_IRQ,
>> >> +     },
>> >> +};
>> >> +#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
>> >
>> > This looks all very convoluted.  Why not do something like:
>> >
>> > static unsigned long iommu_base[] = {
>> >        OMAP3_MMU1_BASE,
>> >        OMAP3_MMU2_BASE,
>> > };
>> >
>> > static int iommu_irq[] = {
>> >        OMAP3_MMU1_IRQ,
>> >        OMAP3_MMU2_IRQ,
>> > };
>>
>> All your comments are pretty similar to my reorganize patch:
>> http://marc.info/?l=linux-omap&m=124172711303076&w=2
>
> ... which hasn't been posted to linux-arm-kernel, and so I won't see it.

You are not subscribed to linux-omap? I wasn't subscribed to
linux-arm-kernel at that point, I'll resend to there.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-05-16  9:20   ` Russell King - ARM Linux
  2009-05-16  9:38     ` Felipe Contreras
  2009-05-16  9:55     ` Russell King - ARM Linux
@ 2009-05-18  5:22     ` Hiroshi DOYU
  2 siblings, 0 replies; 53+ messages in thread
From: Hiroshi DOYU @ 2009-05-18  5:22 UTC (permalink / raw)
  To: linux
  Cc: linux-arm-kernel, linux-omap, h-kanigeri2, omar.ramirez,
	sakari.ailus, tony

Hi Russell,

From: ext Russell King - ARM Linux <linux@arm.linux.org.uk>
Subject: Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
Date: Sat, 16 May 2009 11:20:36 +0200

> On Tue, May 05, 2009 at 03:47:00PM +0300, Hiroshi DOYU wrote:
> > +static struct resource omap3_iommu_res[] = {
> > +	{ /* Camera ISP MMU */
> > +		.start		= OMAP3_MMU1_BASE,
> > +		.end		= OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
> > +		.flags		= IORESOURCE_MEM,
> > +	},
> > +	{
> > +		.start		= OMAP3_MMU1_IRQ,
> > +		.flags		= IORESOURCE_IRQ,
> > +	},
> > +	{ /* IVA2.2 MMU */
> > +		.start		= OMAP3_MMU2_BASE,
> > +		.end		= OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
> > +		.flags		= IORESOURCE_MEM,
> > +	},
> > +	{
> > +		.start		= OMAP3_MMU2_IRQ,
> > +		.flags		= IORESOURCE_IRQ,
> > +	},
> > +};
> > +#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
> 
> This looks all very convoluted.  Why not do something like:
> 
> static unsigned long iommu_base[] = {
> 	OMAP3_MMU1_BASE,
> 	OMAP3_MMU2_BASE,
> };
> 
> static int iommu_irq[] = {
> 	OMAP3_MMU1_IRQ,
> 	OMAP3_MMU2_IRQ,
> };
> 
> > +static struct platform_device *omap3_iommu_pdev[NR_IOMMU_DEVICES];
> > +
> > +static int __init omap3_iommu_init(void)
> > +{
> > +	int i, err;
> > +
> > +	for (i = 0; i < NR_IOMMU_DEVICES; i++) {
> > +		struct platform_device *pdev;
> > +
> > +		pdev = platform_device_alloc("omap-iommu", i + 1);
> 
> Why number them 1 and up when zero is perfectly fine?

I just wanted to use the same expression in TRM. Actually no need to
start from 1.

> 
> > +		if (!pdev) {
> > +			err = -ENOMEM;
> > +			goto err_out;
> > +		}
> > +		err = platform_device_add_resources(pdev,
> > +				    &omap3_iommu_res[2 * i], NR_IOMMU_RES);
> 
> 		struct resource res[2];
> 
> 		res[0].start = iommu_base[i];
> 		res[0].end = iommu_base[i] + MMU_REG_SIZE - 1;
> 		res[0].flags = IORESOURCE_MEM;
> 		res[1].start = res[1].end = iommu_irq[i];
> 		res[1].flags = IORESOURCE_IRQ;
> 
> 		err = platform_device_add_resources(pdev, &res, ARRAY_SIZE(res));
> 
> There's no need to keep 'res' around since it's copied by
> add_resources.

OK.

> 
> > +		if (err)
> > +			goto err_out;
> > +		err = platform_device_add_data(pdev, &omap3_iommu_pdata[i],
> > +					       sizeof(omap3_iommu_pdata[0]));
> > +		if (err)
> > +			goto err_out;
> > +		err = platform_device_add(pdev);
> > +		if (err)
> > +			goto err_out;
> > +		omap3_iommu_pdev[i] = pdev;
> > +	}
> > +	return 0;
> > +
> > +err_out:
> > +	while (i--)
> > +		platform_device_put(omap3_iommu_pdev[i]);
> > +	return err;
> > +}
> > +module_init(omap3_iommu_init);
> > +
> > +static void __exit omap3_iommu_exit(void)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < NR_IOMMU_DEVICES; i++)
> > +		platform_device_unregister(omap3_iommu_pdev[i]);
> > +}
> > +module_exit(omap3_iommu_exit);
> > +
> > +MODULE_AUTHOR("Hiroshi DOYU");
> > +MODULE_DESCRIPTION("omap iommu: omap3 device registration");
> > +MODULE_LICENSE("GPL v2");
> > 
> > 
> > -------------------------------------------------------------------
> > List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
> > FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
> > Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php

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

* Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-05-16  9:55     ` Russell King - ARM Linux
@ 2009-05-18  5:36       ` Hiroshi DOYU
  2009-05-18  9:39       ` Hiroshi DOYU
  2009-05-18 14:26       ` Hiroshi DOYU
  2 siblings, 0 replies; 53+ messages in thread
From: Hiroshi DOYU @ 2009-05-18  5:36 UTC (permalink / raw)
  To: linux
  Cc: linux-arm-kernel, linux-omap, h-kanigeri2, omar.ramirez,
	sakari.ailus, tony

From: ext Russell King - ARM Linux <linux@arm.linux.org.uk>
Subject: Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
Date: Sat, 16 May 2009 11:55:14 +0200

> On Sat, May 16, 2009 at 10:20:36AM +0100, Russell King - ARM Linux wrote:
> > This looks all very convoluted.  Why not do something like:
> > 
> > static unsigned long iommu_base[] = {
> > 	OMAP3_MMU1_BASE,
> > 	OMAP3_MMU2_BASE,
> > };
> > 
> > static int iommu_irq[] = {
> > 	OMAP3_MMU1_IRQ,
> > 	OMAP3_MMU2_IRQ,
> > };
> 
> BTW, both of these can be __initdata.

OK, I'll send the update ones. Thank you for reviewing them.

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

* Re: [PATCH 4/6] omap iommu: simple virtual address space management
  2009-05-16  9:22   ` Russell King - ARM Linux
@ 2009-05-18  6:31     ` Hiroshi DOYU
  2009-05-18 13:10       ` Russell King - ARM Linux
  0 siblings, 1 reply; 53+ messages in thread
From: Hiroshi DOYU @ 2009-05-18  6:31 UTC (permalink / raw)
  To: linux
  Cc: linux-arm-kernel, linux-omap, h-kanigeri2, omar.ramirez,
	sakari.ailus, tony

Hi Russell,

From: ext Russell King - ARM Linux <linux@arm.linux.org.uk>
Subject: Re: [PATCH 4/6] omap iommu: simple virtual address space management
Date: Sat, 16 May 2009 11:22:48 +0200

> On Tue, May 05, 2009 at 03:47:06PM +0300, Hiroshi DOYU wrote:
> > +int ioremap_page(unsigned long virt, unsigned long phys, unsigned int mtype)
> > +{
> > +	const struct mem_type *type;
> > +
> > +	type = get_mem_type(mtype);
> > +	if (!type)
> > +		return -EINVAL;
> 
> I think it would make more sense to move this lookup into the caller
> for this - you're going to be making quite a lot of calls into
> ioremap_page() so you really don't want to be doing the same lookup
> every time.
> 
> > +/* map 'sglist' to a contiguous mpu virtual area and return 'va' */
> > +static void *vmap_sg(const struct sg_table *sgt)
> > +{
> > +	u32 va;
> > +	size_t total;
> > +	unsigned int i;
> > +	struct scatterlist *sg;
> > +	struct vm_struct *new;
> > +
> > +	total = sgtable_len(sgt);
> > +	if (!total)
> > +		return ERR_PTR(-EINVAL);
> > +
> > +	new = __get_vm_area(total, VM_IOREMAP, VMALLOC_START, VMALLOC_END);
> > +	if (!new)
> > +		return ERR_PTR(-ENOMEM);
> > +	va = (u32)new->addr;
> 
> In other words, move it here.

Right. In this change for better performance, I have to expose "struct
mem_type" and "get_mem_type()" a bit more as below. Is this change
acceptable, or do you have a better idea?

diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index 85a49e2..ad1cbc4 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -77,8 +77,17 @@ extern void __iounmap(volatile void __iomem *addr);
 /*
  * external interface to remap single page with appropriate type
  */
+struct mem_type {
+	unsigned int prot_pte;
+	unsigned int prot_l1;
+	unsigned int prot_sect;
+	unsigned int domain;
+};
+
+const struct mem_type *get_mem_type(unsigned int type);
+
 extern int ioremap_page(unsigned long virt, unsigned long phys,
-			unsigned int mtype);
+			const struct mem_type *mtype);
 
 /*
  * Bad read/write accesses...
diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index 8441351..0ab75c6 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -110,15 +110,10 @@ static int remap_area_pages(unsigned long start, unsigned long pfn,
 	return err;
 }
 
-int ioremap_page(unsigned long virt, unsigned long phys, unsigned int mtype)
+int ioremap_page(unsigned long virt, unsigned long phys,
+		 const struct mem_type *mtype)
 {
-	const struct mem_type *type;
-
-	type = get_mem_type(mtype);
-	if (!type)
-		return -EINVAL;
-
-	return remap_area_pages(virt, __phys_to_pfn(phys), PAGE_SIZE, type);
+	return remap_area_pages(virt, __phys_to_pfn(phys), PAGE_SIZE, mtype);
 }
 EXPORT_SYMBOL(ioremap_page);
 
diff --git a/arch/arm/mm/mm.h b/arch/arm/mm/mm.h
index 5d9f539..57f10aa 100644
--- a/arch/arm/mm/mm.h
+++ b/arch/arm/mm/mm.h
@@ -16,15 +16,6 @@ static inline pmd_t *pmd_off_k(unsigned long virt)
 	return pmd_off(pgd_offset_k(virt), virt);
 }
 
-struct mem_type {
-	unsigned int prot_pte;
-	unsigned int prot_l1;
-	unsigned int prot_sect;
-	unsigned int domain;
-};
-
-const struct mem_type *get_mem_type(unsigned int type);
-
 #endif
 
 struct map_desc;
diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c
index a539051..020f5af 100644
--- a/arch/arm/plat-omap/iovmm.c
+++ b/arch/arm/plat-omap/iovmm.c
@@ -167,6 +167,11 @@ static void *vmap_sg(const struct sg_table *sgt)
 	unsigned int i;
 	struct scatterlist *sg;
 	struct vm_struct *new;
+	const struct mem_type *mtype;
+
+	mtype = get_mem_type(MT_DEVICE);
+	if (!type)
+		return -EINVAL;
 
 	total = sgtable_len(sgt);
 	if (!total)
@@ -187,7 +192,7 @@ static void *vmap_sg(const struct sg_table *sgt)
 
 		BUG_ON(bytes != PAGE_SIZE);
 
-		err = ioremap_page(va,  pa, MT_DEVICE);
+		err = ioremap_page(va,  pa, mtype);
 		if (err)
 			goto err_out;
 

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

* Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-05-16  9:55     ` Russell King - ARM Linux
  2009-05-18  5:36       ` Hiroshi DOYU
@ 2009-05-18  9:39       ` Hiroshi DOYU
  2009-05-18 14:26       ` Hiroshi DOYU
  2 siblings, 0 replies; 53+ messages in thread
From: Hiroshi DOYU @ 2009-05-18  9:39 UTC (permalink / raw)
  To: linux
  Cc: linux-arm-kernel, linux-omap, h-kanigeri2, omar.ramirez,
	sakari.ailus, tony

[-- Attachment #1: Type: Text/Plain, Size: 561 bytes --]

From: ext Russell King - ARM Linux <linux@arm.linux.org.uk>
Subject: Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
Date: Sat, 16 May 2009 11:55:14 +0200

> On Sat, May 16, 2009 at 10:20:36AM +0100, Russell King - ARM Linux wrote:
> > This looks all very convoluted.  Why not do something like:
> > 
> > static unsigned long iommu_base[] = {
> > 	OMAP3_MMU1_BASE,
> > 	OMAP3_MMU2_BASE,
> > };
> > 
> > static int iommu_irq[] = {
> > 	OMAP3_MMU1_IRQ,
> > 	OMAP3_MMU2_IRQ,
> > };
> 
> BTW, both of these can be __initdata.

Attached the updated one.

[-- Attachment #2: 0003-omap-iommu-omap3-iommu-device-registration.patch --]
[-- Type: Text/Plain, Size: 3061 bytes --]

>From 2bfdb4130433708dc04e1c30aa5099d48de408e0 Mon Sep 17 00:00:00 2001
From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
Date: Wed, 28 Jan 2009 21:32:04 +0200
Subject: [PATCH 3/6] omap iommu: omap3 iommu device registration

Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---
 arch/arm/mach-omap2/omap3-iommu.c |  103 +++++++++++++++++++++++++++++++++++++
 1 files changed, 103 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-omap2/omap3-iommu.c

diff --git a/arch/arm/mach-omap2/omap3-iommu.c b/arch/arm/mach-omap2/omap3-iommu.c
new file mode 100644
index 0000000..91ee38a
--- /dev/null
+++ b/arch/arm/mach-omap2/omap3-iommu.c
@@ -0,0 +1,103 @@
+/*
+ * omap iommu: omap3 device registration
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation
+ *
+ * Written by Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/platform_device.h>
+
+#include <mach/iommu.h>
+
+#define OMAP3_MMU1_BASE	0x480bd400
+#define OMAP3_MMU2_BASE	0x5d000000
+#define OMAP3_MMU1_IRQ	24
+#define OMAP3_MMU2_IRQ	28
+
+
+static unsigned long iommu_base[] __initdata = {
+	OMAP3_MMU1_BASE,
+	OMAP3_MMU2_BASE,
+};
+
+static int iommu_irq[] __initdata = {
+	OMAP3_MMU1_IRQ,
+	OMAP3_MMU2_IRQ,
+};
+
+static const struct iommu_platform_data omap3_iommu_pdata[] __initconst = {
+	{
+		.name = "isp",
+		.nr_tlb_entries = 8,
+		.clk_name = "cam_ick",
+	},
+	{
+		.name = "iva2",
+		.nr_tlb_entries = 32,
+		.clk_name = "iva2_ck",
+	},
+};
+#define NR_IOMMU_DEVICES ARRAY_SIZE(omap3_iommu_pdata)
+
+static struct platform_device *omap3_iommu_pdev[NR_IOMMU_DEVICES];
+
+static int __init omap3_iommu_init(void)
+{
+	int i, err;
+
+	for (i = 0; i < NR_IOMMU_DEVICES; i++) {
+		struct platform_device *pdev;
+		struct resource res[2];
+
+		pdev = platform_device_alloc("omap-iommu", i);
+		if (!pdev) {
+			err = -ENOMEM;
+			goto err_out;
+		}
+
+		memset(res, 0,  sizeof(res));
+		res[0].start = iommu_base[i];
+		res[0].end = iommu_base[i] + MMU_REG_SIZE - 1;
+		res[0].flags = IORESOURCE_MEM;
+		res[1].start = res[1].end = iommu_irq[i];
+		res[1].flags = IORESOURCE_IRQ;
+
+		err = platform_device_add_resources(pdev, res,
+						    ARRAY_SIZE(res));
+		if (err)
+			goto err_out;
+		err = platform_device_add_data(pdev, &omap3_iommu_pdata[i],
+					       sizeof(omap3_iommu_pdata[0]));
+		if (err)
+			goto err_out;
+		err = platform_device_add(pdev);
+		if (err)
+			goto err_out;
+		omap3_iommu_pdev[i] = pdev;
+	}
+	return 0;
+
+err_out:
+	while (i--)
+		platform_device_put(omap3_iommu_pdev[i]);
+	return err;
+}
+module_init(omap3_iommu_init);
+
+static void __exit omap3_iommu_exit(void)
+{
+	int i;
+
+	for (i = 0; i < NR_IOMMU_DEVICES; i++)
+		platform_device_unregister(omap3_iommu_pdev[i]);
+}
+module_exit(omap3_iommu_exit);
+
+MODULE_AUTHOR("Hiroshi DOYU");
+MODULE_DESCRIPTION("omap iommu: omap3 device registration");
+MODULE_LICENSE("GPL v2");
-- 
1.6.0.4


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

* Re: [PATCH 4/6] omap iommu: simple virtual address space management
  2009-05-18  6:31     ` Hiroshi DOYU
@ 2009-05-18 13:10       ` Russell King - ARM Linux
  0 siblings, 0 replies; 53+ messages in thread
From: Russell King - ARM Linux @ 2009-05-18 13:10 UTC (permalink / raw)
  To: Hiroshi DOYU
  Cc: linux-arm-kernel, linux-omap, h-kanigeri2, omar.ramirez,
	sakari.ailus, tony

On Mon, May 18, 2009 at 09:31:26AM +0300, Hiroshi DOYU wrote:
> Right. In this change for better performance, I have to expose "struct
> mem_type" and "get_mem_type()" a bit more as below. Is this change
> acceptable, or do you have a better idea?

You don't need to expose struct mem_type at all.  You only need to
tell the compiler that it exists and is valid by:

struct mem_type;

Then you can use it in function prototypes without complaint.  I'd also
prefer to hide ioremap_page() and get_mem_type() outside of the normal
Linux headers - maybe in asm/mach/map.h.

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

* Re: [PATCH 5/6] omap iommu: entries for Kconfig and Makefile
  2009-05-05 12:47 ` [PATCH 5/6] omap iommu: entries for Kconfig and Makefile Hiroshi DOYU
@ 2009-05-18 13:36   ` Russell King - ARM Linux
  0 siblings, 0 replies; 53+ messages in thread
From: Russell King - ARM Linux @ 2009-05-18 13:36 UTC (permalink / raw)
  To: Hiroshi DOYU
  Cc: linux-arm-kernel, linux-omap, h-kanigeri2, omar.ramirez,
	sakari.ailus, tony

On Tue, May 05, 2009 at 03:47:11PM +0300, Hiroshi DOYU wrote:
> +config OMAP_IOMMU
> +	tristate "IOMMU support"
> +	depends on ARCH_OMAP
> +	default n

default n is the default whatever.  There's no need for the depends line
either - the whole file is protected by an "if ARCH_OMAP".."endif"
encapsulation.

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

* Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-05-16  9:55     ` Russell King - ARM Linux
  2009-05-18  5:36       ` Hiroshi DOYU
  2009-05-18  9:39       ` Hiroshi DOYU
@ 2009-05-18 14:26       ` Hiroshi DOYU
  2009-05-18 16:43         ` [PATCH] omap3-iommu: reorganize Felipe Contreras
  2 siblings, 1 reply; 53+ messages in thread
From: Hiroshi DOYU @ 2009-05-18 14:26 UTC (permalink / raw)
  To: linux
  Cc: linux-arm-kernel, linux-omap, h-kanigeri2, omar.ramirez,
	sakari.ailus, tony

[-- Attachment #1: Type: Text/Plain, Size: 561 bytes --]

From: ext Russell King - ARM Linux <linux@arm.linux.org.uk>
Subject: Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
Date: Sat, 16 May 2009 11:55:14 +0200

> On Sat, May 16, 2009 at 10:20:36AM +0100, Russell King - ARM Linux wrote:
> > This looks all very convoluted.  Why not do something like:
> > 
> > static unsigned long iommu_base[] = {
> > 	OMAP3_MMU1_BASE,
> > 	OMAP3_MMU2_BASE,
> > };
> > 
> > static int iommu_irq[] = {
> > 	OMAP3_MMU1_IRQ,
> > 	OMAP3_MMU2_IRQ,
> > };
> 
> BTW, both of these can be __initdata.

Attached the updated one.

[-- Attachment #2: 0003-omap-iommu-omap3-iommu-device-registration.patch --]
[-- Type: Text/Plain, Size: 3061 bytes --]

>From 2bfdb4130433708dc04e1c30aa5099d48de408e0 Mon Sep 17 00:00:00 2001
From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
Date: Wed, 28 Jan 2009 21:32:04 +0200
Subject: [PATCH 3/6] omap iommu: omap3 iommu device registration

Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---
 arch/arm/mach-omap2/omap3-iommu.c |  103 +++++++++++++++++++++++++++++++++++++
 1 files changed, 103 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-omap2/omap3-iommu.c

diff --git a/arch/arm/mach-omap2/omap3-iommu.c b/arch/arm/mach-omap2/omap3-iommu.c
new file mode 100644
index 0000000..91ee38a
--- /dev/null
+++ b/arch/arm/mach-omap2/omap3-iommu.c
@@ -0,0 +1,103 @@
+/*
+ * omap iommu: omap3 device registration
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation
+ *
+ * Written by Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/platform_device.h>
+
+#include <mach/iommu.h>
+
+#define OMAP3_MMU1_BASE	0x480bd400
+#define OMAP3_MMU2_BASE	0x5d000000
+#define OMAP3_MMU1_IRQ	24
+#define OMAP3_MMU2_IRQ	28
+
+
+static unsigned long iommu_base[] __initdata = {
+	OMAP3_MMU1_BASE,
+	OMAP3_MMU2_BASE,
+};
+
+static int iommu_irq[] __initdata = {
+	OMAP3_MMU1_IRQ,
+	OMAP3_MMU2_IRQ,
+};
+
+static const struct iommu_platform_data omap3_iommu_pdata[] __initconst = {
+	{
+		.name = "isp",
+		.nr_tlb_entries = 8,
+		.clk_name = "cam_ick",
+	},
+	{
+		.name = "iva2",
+		.nr_tlb_entries = 32,
+		.clk_name = "iva2_ck",
+	},
+};
+#define NR_IOMMU_DEVICES ARRAY_SIZE(omap3_iommu_pdata)
+
+static struct platform_device *omap3_iommu_pdev[NR_IOMMU_DEVICES];
+
+static int __init omap3_iommu_init(void)
+{
+	int i, err;
+
+	for (i = 0; i < NR_IOMMU_DEVICES; i++) {
+		struct platform_device *pdev;
+		struct resource res[2];
+
+		pdev = platform_device_alloc("omap-iommu", i);
+		if (!pdev) {
+			err = -ENOMEM;
+			goto err_out;
+		}
+
+		memset(res, 0,  sizeof(res));
+		res[0].start = iommu_base[i];
+		res[0].end = iommu_base[i] + MMU_REG_SIZE - 1;
+		res[0].flags = IORESOURCE_MEM;
+		res[1].start = res[1].end = iommu_irq[i];
+		res[1].flags = IORESOURCE_IRQ;
+
+		err = platform_device_add_resources(pdev, res,
+						    ARRAY_SIZE(res));
+		if (err)
+			goto err_out;
+		err = platform_device_add_data(pdev, &omap3_iommu_pdata[i],
+					       sizeof(omap3_iommu_pdata[0]));
+		if (err)
+			goto err_out;
+		err = platform_device_add(pdev);
+		if (err)
+			goto err_out;
+		omap3_iommu_pdev[i] = pdev;
+	}
+	return 0;
+
+err_out:
+	while (i--)
+		platform_device_put(omap3_iommu_pdev[i]);
+	return err;
+}
+module_init(omap3_iommu_init);
+
+static void __exit omap3_iommu_exit(void)
+{
+	int i;
+
+	for (i = 0; i < NR_IOMMU_DEVICES; i++)
+		platform_device_unregister(omap3_iommu_pdev[i]);
+}
+module_exit(omap3_iommu_exit);
+
+MODULE_AUTHOR("Hiroshi DOYU");
+MODULE_DESCRIPTION("omap iommu: omap3 device registration");
+MODULE_LICENSE("GPL v2");
-- 
1.6.0.4


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

* [PATCH] omap3-iommu: reorganize
  2009-05-18 14:26       ` Hiroshi DOYU
@ 2009-05-18 16:43         ` Felipe Contreras
  2009-05-19  6:03           ` Hiroshi DOYU
  0 siblings, 1 reply; 53+ messages in thread
From: Felipe Contreras @ 2009-05-18 16:43 UTC (permalink / raw)
  To: linux-omap
  Cc: Hiroshi DOYU, Hari Kanigeri, linux-arm-kernel, Felipe Contreras

No functional changes.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

How about this?

 arch/arm/mach-omap2/omap3-iommu.c |   53 ++++++++++++++++++-------------------
 1 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/arch/arm/mach-omap2/omap3-iommu.c b/arch/arm/mach-omap2/omap3-iommu.c
index 91ee38a..50bd445 100644
--- a/arch/arm/mach-omap2/omap3-iommu.c
+++ b/arch/arm/mach-omap2/omap3-iommu.c
@@ -14,32 +14,30 @@
 
 #include <mach/iommu.h>
 
-#define OMAP3_MMU1_BASE	0x480bd400
-#define OMAP3_MMU2_BASE	0x5d000000
-#define OMAP3_MMU1_IRQ	24
-#define OMAP3_MMU2_IRQ	28
-
-
-static unsigned long iommu_base[] __initdata = {
-	OMAP3_MMU1_BASE,
-	OMAP3_MMU2_BASE,
-};
-
-static int iommu_irq[] __initdata = {
-	OMAP3_MMU1_IRQ,
-	OMAP3_MMU2_IRQ,
+struct iommu_device {
+	resource_size_t base;
+	int irq;
+	struct iommu_platform_data pdata;
 };
 
-static const struct iommu_platform_data omap3_iommu_pdata[] __initconst = {
+static struct iommu_device devices[] __initdata = {
 	{
-		.name = "isp",
-		.nr_tlb_entries = 8,
-		.clk_name = "cam_ick",
+		.base = 0x480bd400,
+		.irq = 24,
+		.pdata = {
+			.name = "isp",
+			.nr_tlb_entries = 8,
+			.clk_name = "cam_ick",
+		},
 	},
 	{
-		.name = "iva2",
-		.nr_tlb_entries = 32,
-		.clk_name = "iva2_ck",
+		.base = 0x5d000000,
+		.irq = 28,
+		.pdata = {
+			.name = "iva2",
+			.nr_tlb_entries = 32,
+			.clk_name = "iva2_ck",
+		},
 	},
 };
 #define NR_IOMMU_DEVICES ARRAY_SIZE(omap3_iommu_pdata)
@@ -52,6 +50,7 @@ static int __init omap3_iommu_init(void)
 
 	for (i = 0; i < NR_IOMMU_DEVICES; i++) {
 		struct platform_device *pdev;
+		const struct iommu_device *d = &devices[i];
 		struct resource res[2];
 
 		pdev = platform_device_alloc("omap-iommu", i);
@@ -60,19 +59,19 @@ static int __init omap3_iommu_init(void)
 			goto err_out;
 		}
 
-		memset(res, 0,  sizeof(res));
-		res[0].start = iommu_base[i];
-		res[0].end = iommu_base[i] + MMU_REG_SIZE - 1;
+		memset(res, 0, sizeof(res));
+		res[0].start = d->base;
+		res[0].end = d->base + MMU_REG_SIZE - 1;
 		res[0].flags = IORESOURCE_MEM;
-		res[1].start = res[1].end = iommu_irq[i];
+		res[1].start = res[1].end = d->irq;
 		res[1].flags = IORESOURCE_IRQ;
 
 		err = platform_device_add_resources(pdev, res,
 						    ARRAY_SIZE(res));
 		if (err)
 			goto err_out;
-		err = platform_device_add_data(pdev, &omap3_iommu_pdata[i],
-					       sizeof(omap3_iommu_pdata[0]));
+		err = platform_device_add_data(pdev, &d->pdata,
+					       sizeof(d->pdata));
 		if (err)
 			goto err_out;
 		err = platform_device_add(pdev);
-- 
1.6.3.1


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

* Re: [PATCH] omap3-iommu: reorganize
  2009-05-18 16:43         ` [PATCH] omap3-iommu: reorganize Felipe Contreras
@ 2009-05-19  6:03           ` Hiroshi DOYU
  2009-05-19  9:43             ` Felipe Contreras
  0 siblings, 1 reply; 53+ messages in thread
From: Hiroshi DOYU @ 2009-05-19  6:03 UTC (permalink / raw)
  To: felipe.contreras; +Cc: linux-omap, h-kanigeri2, linux-arm-kernel

From: ext Felipe Contreras <felipe.contreras@gmail.com>
Subject: [PATCH] omap3-iommu: reorganize
Date: Mon, 18 May 2009 18:43:00 +0200

> No functional changes.
> 
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> 
> How about this?

What's the advantage of introducing a new structure here?

> 
>  arch/arm/mach-omap2/omap3-iommu.c |   53 ++++++++++++++++++-------------------
>  1 files changed, 26 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap3-iommu.c b/arch/arm/mach-omap2/omap3-iommu.c
> index 91ee38a..50bd445 100644
> --- a/arch/arm/mach-omap2/omap3-iommu.c
> +++ b/arch/arm/mach-omap2/omap3-iommu.c
> @@ -14,32 +14,30 @@
>  
>  #include <mach/iommu.h>
>  
> -#define OMAP3_MMU1_BASE	0x480bd400
> -#define OMAP3_MMU2_BASE	0x5d000000
> -#define OMAP3_MMU1_IRQ	24
> -#define OMAP3_MMU2_IRQ	28
> -
> -
> -static unsigned long iommu_base[] __initdata = {
> -	OMAP3_MMU1_BASE,
> -	OMAP3_MMU2_BASE,
> -};
> -
> -static int iommu_irq[] __initdata = {
> -	OMAP3_MMU1_IRQ,
> -	OMAP3_MMU2_IRQ,
> +struct iommu_device {
> +	resource_size_t base;
> +	int irq;
> +	struct iommu_platform_data pdata;
>  };
>  
> -static const struct iommu_platform_data omap3_iommu_pdata[] __initconst = {
> +static struct iommu_device devices[] __initdata = {
>  	{
> -		.name = "isp",
> -		.nr_tlb_entries = 8,
> -		.clk_name = "cam_ick",
> +		.base = 0x480bd400,
> +		.irq = 24,
> +		.pdata = {
> +			.name = "isp",
> +			.nr_tlb_entries = 8,
> +			.clk_name = "cam_ick",
> +		},
>  	},
>  	{
> -		.name = "iva2",
> -		.nr_tlb_entries = 32,
> -		.clk_name = "iva2_ck",
> +		.base = 0x5d000000,
> +		.irq = 28,
> +		.pdata = {
> +			.name = "iva2",
> +			.nr_tlb_entries = 32,
> +			.clk_name = "iva2_ck",
> +		},
>  	},
>  };
>  #define NR_IOMMU_DEVICES ARRAY_SIZE(omap3_iommu_pdata)
> @@ -52,6 +50,7 @@ static int __init omap3_iommu_init(void)
>  
>  	for (i = 0; i < NR_IOMMU_DEVICES; i++) {
>  		struct platform_device *pdev;
> +		const struct iommu_device *d = &devices[i];
>  		struct resource res[2];
>  
>  		pdev = platform_device_alloc("omap-iommu", i);
> @@ -60,19 +59,19 @@ static int __init omap3_iommu_init(void)
>  			goto err_out;
>  		}
>  
> -		memset(res, 0,  sizeof(res));
> -		res[0].start = iommu_base[i];
> -		res[0].end = iommu_base[i] + MMU_REG_SIZE - 1;
> +		memset(res, 0, sizeof(res));
> +		res[0].start = d->base;
> +		res[0].end = d->base + MMU_REG_SIZE - 1;
>  		res[0].flags = IORESOURCE_MEM;
> -		res[1].start = res[1].end = iommu_irq[i];
> +		res[1].start = res[1].end = d->irq;
>  		res[1].flags = IORESOURCE_IRQ;
>  
>  		err = platform_device_add_resources(pdev, res,
>  						    ARRAY_SIZE(res));
>  		if (err)
>  			goto err_out;
> -		err = platform_device_add_data(pdev, &omap3_iommu_pdata[i],
> -					       sizeof(omap3_iommu_pdata[0]));
> +		err = platform_device_add_data(pdev, &d->pdata,
> +					       sizeof(d->pdata));
>  		if (err)
>  			goto err_out;
>  		err = platform_device_add(pdev);
> -- 
> 1.6.3.1
> 

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

* Re: [PATCH] omap3-iommu: reorganize
  2009-05-19  6:03           ` Hiroshi DOYU
@ 2009-05-19  9:43             ` Felipe Contreras
  0 siblings, 0 replies; 53+ messages in thread
From: Felipe Contreras @ 2009-05-19  9:43 UTC (permalink / raw)
  To: Hiroshi DOYU; +Cc: linux-omap, h-kanigeri2, linux-arm-kernel

On Tue, May 19, 2009 at 9:03 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> From: ext Felipe Contreras <felipe.contreras@gmail.com>
> Subject: [PATCH] omap3-iommu: reorganize
> Date: Mon, 18 May 2009 18:43:00 +0200
>
>> No functional changes.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>>
>> How about this?
>
> What's the advantage of introducing a new structure here?

That instead of having 3 arrays with potentially different sizes:

iommu_base
iommu_irq
omap3_iommu_pdata

You have one:

devices

Also, think about the hypothetical situation where you need 2 more
iommu devices (maybe OMAP4?). What would you need to do? You'll need
to add OMAP3_MMU3_BASE and OMAP3_MMU4_BASE add them to iommu_base,
then add OMAP3_MMU3_IRQ and OMAP3_MMU4_IRQ and put them to iommu_irq,
finally add the device data to omap3_iommu_pdata.

In the process a simple mistake would be easy to overlook:

static unsigned long iommu_base[] __initdata = {
       OMAP3_MMU1_BASE,
       OMAP3_MMU2_BASE,
+       OMAP3_MMU3_BASE,
+       OMAP3_MMU2_BASE,
};

With the 'devices' array you just need to add two more elements and
that's it. It leaves less room for mistakes.

However 'struct iommu_device' is a local structure, it could very well
be called __foobar__, nobody outside omap3-iommu.c will see it anyway.

-- 
Felipe Contreras

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

* Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-01-28 10:41       ` Russell King - ARM Linux
@ 2009-01-28 11:37         ` Hiroshi DOYU
  0 siblings, 0 replies; 53+ messages in thread
From: Hiroshi DOYU @ 2009-01-28 11:37 UTC (permalink / raw)
  To: linux; +Cc: linux-kernel, linux-arm-kernel, linux-omap

From: ext Russell King - ARM Linux <linux@arm.linux.org.uk>
Subject: Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
Date: Wed, 28 Jan 2009 11:41:57 +0100

> On Tue, Jan 27, 2009 at 11:29:35PM +0200, Hiroshi DOYU wrote:
> > I attached the update one.
> 
> Thanks.  I want to hold off on taking these just a little bit longer.

OK. agreed.

> > +static const struct iommu_platform_data omap3_iommu_pdata[] __initconst = {
> > +     {
> > +             .name = "isp",
> > +             .nr_tlb_entries = 8,
> > +             .clk_name = "cam_ick",
> > +     },
> > +     {
> > +             .name = "iva2",
> > +             .nr_tlb_entries = 32,
> > +             .clk_name = "iva2_ck",
> 
> With the omap clk implementation in a state of flux, hopefully moving
> towards something which better reflects the intentions of the clk API,
> passing clk names around becomes entirely wasteful and unnecessary.
> 
> That's not to say that what you currently have won't work with the patches
> as they currently stand - it will work as is.  I'd just rather avoid
> having to have a separate patch to convert this code as well.

OK. I'll update later, then.

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

* Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-01-27 21:29     ` Hiroshi DOYU
@ 2009-01-28 10:41       ` Russell King - ARM Linux
  2009-01-28 11:37         ` Hiroshi DOYU
  0 siblings, 1 reply; 53+ messages in thread
From: Russell King - ARM Linux @ 2009-01-28 10:41 UTC (permalink / raw)
  To: Hiroshi DOYU; +Cc: linux-kernel, linux-arm-kernel, linux-omap

On Tue, Jan 27, 2009 at 11:29:35PM +0200, Hiroshi DOYU wrote:
> I attached the update one.

Thanks.  I want to hold off on taking these just a little bit longer.

> +static const struct iommu_platform_data omap3_iommu_pdata[] __initconst = {
> +	{
> +		.name = "isp",
> +		.nr_tlb_entries = 8,
> +		.clk_name = "cam_ick",
> +	},
> +	{
> +		.name = "iva2",
> +		.nr_tlb_entries = 32,
> +		.clk_name = "iva2_ck",

With the omap clk implementation in a state of flux, hopefully moving
towards something which better reflects the intentions of the clk API,
passing clk names around becomes entirely wasteful and unnecessary.

That's not to say that what you currently have won't work with the patches
as they currently stand - it will work as is.  I'd just rather avoid
having to have a separate patch to convert this code as well.

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

* Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-01-17 16:21   ` Russell King - ARM Linux
@ 2009-01-27 21:29     ` Hiroshi DOYU
  2009-01-28 10:41       ` Russell King - ARM Linux
  0 siblings, 1 reply; 53+ messages in thread
From: Hiroshi DOYU @ 2009-01-27 21:29 UTC (permalink / raw)
  To: linux; +Cc: linux-kernel, linux-arm-kernel, linux-omap

[-- Attachment #1: Type: Text/Plain, Size: 2413 bytes --]

Hi Russell,

I attached the update one.

From: ext Russell King - ARM Linux <linux@arm.linux.org.uk>
Subject: Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
Date: Sat, 17 Jan 2009 17:21:39 +0100

> On Fri, Jan 16, 2009 at 10:37:20AM +0200, Hiroshi DOYU wrote:
> > +#include <linux/io.h>
> 
> Is linux/io.h needed, or will a more specific include be better?
> 
> > +#include <linux/platform_device.h>
> > +
> > +#include <mach/iommu.h>
> > +
> > +#define DEVNAME "omap-iommu"
> 
> I'm not sure this DEVNAME definition really helps anything.
> 
> > +static void omap3_iommu_release(struct device *dev)
> > +{
> > +}
> 
> Err, no.  Never ever ever provide a NULL release function.  Providing
> such a function is a screaming message that what you're doing is buggy.
> 
> And if you get a warning through not providing such a function, it's
> telling you that what your overall approach with the driver API is
> buggy (and you haven't understood the implications of refcounted
> object management.)
> 
> > +
> > +static struct platform_device omap3_iommu_pdev[] = {
> > +	{
> > +		.name		= DEVNAME,
> > +		.id		= 1,
> > +		.num_resources	= ARRAY_SIZE(iommu1_res),
> > +		.resource	= iommu1_res,
> > +		.dev		= {
> > +			.release = omap3_iommu_release,
> > +			.platform_data = &omap3_iommu_pdata[0],
> > +		},
> > +	},
> > +	{
> > +		.name		= DEVNAME,
> > +		.id		= 2,
> > +		.num_resources	= ARRAY_SIZE(iommu2_res),
> > +		.resource	= iommu2_res,
> > +		.dev		= {
> > +			.release = omap3_iommu_release,
> > +			.platform_data = &omap3_iommu_pdata[1],
> > +		},
> > +	},
> > +};
> > +
> > +static int __init omap3_iommu_init(void)
> > +{
> > +	int i;
> > +	for (i = 0; i < ARRAY_SIZE(omap3_iommu_pdev); i++)
> > +		platform_device_register(&omap3_iommu_pdev[i]);
> > +	return 0;
> > +}
> > +module_init(omap3_iommu_init);
> > +
> > +static void __exit omap3_iommu_exit(void)
> > +{
> > +	int i;
> > +	for (i = 0; i < ARRAY_SIZE(omap3_iommu_pdev); i++)
> > +		platform_device_unregister(&omap3_iommu_pdev[i]);
> 
> So... this can never be bug free - you can _never_ unregister statically
> allocated devices.  Not even if you provide an empty release function.
> 
> If you want to register and unregister device structures, it must be
> done using the correct APIs, and in the case of platform devices, that's
> the platform_device_alloc(), platform_device_add() and
> platform_device_unregister() APIs.

[-- Attachment #2: 0003-omap-iommu-omap3-iommu-device-registration.patch --]
[-- Type: Text/Plain, Size: 3148 bytes --]

>From 5b61d40f516d7df3a597df5bd798cb06df01fb28 Mon Sep 17 00:00:00 2001
From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
Date: Tue, 27 Jan 2009 22:46:36 +0200
Subject: [PATCH 3/6] omap iommu: omap3 iommu device registration

Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---
 arch/arm/mach-omap2/omap3-iommu.c |  103 +++++++++++++++++++++++++++++++++++++
 1 files changed, 103 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-omap2/omap3-iommu.c

diff --git a/arch/arm/mach-omap2/omap3-iommu.c b/arch/arm/mach-omap2/omap3-iommu.c
new file mode 100644
index 0000000..4f51c18
--- /dev/null
+++ b/arch/arm/mach-omap2/omap3-iommu.c
@@ -0,0 +1,103 @@
+/*
+ * omap iommu: omap3 device registration
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation
+ *
+ * Written by Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/platform_device.h>
+#include <asm/io.h>
+#include <mach/iommu.h>
+
+#define OMAP3_MMU1_BASE	0x480bd400
+#define OMAP3_MMU2_BASE	0x5d000000
+#define OMAP3_MMU1_IRQ	24
+#define OMAP3_MMU2_IRQ	28
+
+static struct resource omap3_iommu_res[] = {
+	{ /* Camera ISP MMU */
+		.start		= OMAP3_MMU1_BASE,
+		.end		= OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
+		.flags		= IORESOURCE_MEM,
+	},
+	{
+		.start		= OMAP3_MMU1_IRQ,
+		.flags		= IORESOURCE_IRQ,
+	},
+	{ /* IVA2.2 MMU */
+		.start		= OMAP3_MMU2_BASE,
+		.end		= OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
+		.flags		= IORESOURCE_MEM,
+	},
+	{
+		.start		= OMAP3_MMU2_IRQ,
+		.flags		= IORESOURCE_IRQ,
+	},
+};
+#define NR_IOMMU_RES (ARRAY_SIZE(omap3_iommu_res) / 2)
+
+static const struct iommu_platform_data omap3_iommu_pdata[] __initconst = {
+	{
+		.name = "isp",
+		.nr_tlb_entries = 8,
+		.clk_name = "cam_ick",
+	},
+	{
+		.name = "iva2",
+		.nr_tlb_entries = 32,
+		.clk_name = "iva2_ck",
+	},
+};
+#define NR_IOMMU_DEVICES ARRAY_SIZE(omap3_iommu_pdata)
+
+static struct platform_device *omap3_iommu_pdev[NR_IOMMU_DEVICES];
+
+static int __init omap3_iommu_init(void)
+{
+	int i, err;
+
+	for (i = 0; i < NR_IOMMU_DEVICES; i++) {
+		struct platform_device *pdev;
+
+		pdev = platform_device_alloc("omap-iommu", i + 1);
+		if (!pdev)
+			goto err_out;
+		err = platform_device_add_resources(pdev,
+				    &omap3_iommu_res[2 * i], NR_IOMMU_RES);
+		if (err)
+			goto err_out;
+		err = platform_device_add_data(pdev, &omap3_iommu_pdata[i],
+					       sizeof(omap3_iommu_pdata[0]));
+		if (err)
+			goto err_out;
+		err = platform_device_add(pdev);
+		if (err)
+			goto err_out;
+		omap3_iommu_pdev[i] = pdev;
+	}
+	return 0;
+
+err_out:
+	while (i--)
+		platform_device_put(omap3_iommu_pdev[i]);
+	return err;
+}
+module_init(omap3_iommu_init);
+
+static void __exit omap3_iommu_exit(void)
+{
+	int i;
+
+	for (i = 0; i < NR_IOMMU_DEVICES; i++)
+		platform_device_unregister(omap3_iommu_pdev[i]);
+}
+module_exit(omap3_iommu_exit);
+
+MODULE_AUTHOR("Hiroshi DOYU");
+MODULE_DESCRIPTION("omap iommu: omap3 device registration");
+MODULE_LICENSE("GPL v2");
-- 
1.6.1.rc4


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

* Re: [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-01-16  8:37 ` [PATCH 3/6] omap iommu: omap3 iommu device registration Hiroshi DOYU
@ 2009-01-17 16:21   ` Russell King - ARM Linux
  2009-01-27 21:29     ` Hiroshi DOYU
  0 siblings, 1 reply; 53+ messages in thread
From: Russell King - ARM Linux @ 2009-01-17 16:21 UTC (permalink / raw)
  To: Hiroshi DOYU; +Cc: linux-kernel, linux-arm-kernel, linux-omap

On Fri, Jan 16, 2009 at 10:37:20AM +0200, Hiroshi DOYU wrote:
> +#include <linux/io.h>

Is linux/io.h needed, or will a more specific include be better?

> +#include <linux/platform_device.h>
> +
> +#include <mach/iommu.h>
> +
> +#define DEVNAME "omap-iommu"

I'm not sure this DEVNAME definition really helps anything.

> +static void omap3_iommu_release(struct device *dev)
> +{
> +}

Err, no.  Never ever ever provide a NULL release function.  Providing
such a function is a screaming message that what you're doing is buggy.

And if you get a warning through not providing such a function, it's
telling you that what your overall approach with the driver API is
buggy (and you haven't understood the implications of refcounted
object management.)

> +
> +static struct platform_device omap3_iommu_pdev[] = {
> +	{
> +		.name		= DEVNAME,
> +		.id		= 1,
> +		.num_resources	= ARRAY_SIZE(iommu1_res),
> +		.resource	= iommu1_res,
> +		.dev		= {
> +			.release = omap3_iommu_release,
> +			.platform_data = &omap3_iommu_pdata[0],
> +		},
> +	},
> +	{
> +		.name		= DEVNAME,
> +		.id		= 2,
> +		.num_resources	= ARRAY_SIZE(iommu2_res),
> +		.resource	= iommu2_res,
> +		.dev		= {
> +			.release = omap3_iommu_release,
> +			.platform_data = &omap3_iommu_pdata[1],
> +		},
> +	},
> +};
> +
> +static int __init omap3_iommu_init(void)
> +{
> +	int i;
> +	for (i = 0; i < ARRAY_SIZE(omap3_iommu_pdev); i++)
> +		platform_device_register(&omap3_iommu_pdev[i]);
> +	return 0;
> +}
> +module_init(omap3_iommu_init);
> +
> +static void __exit omap3_iommu_exit(void)
> +{
> +	int i;
> +	for (i = 0; i < ARRAY_SIZE(omap3_iommu_pdev); i++)
> +		platform_device_unregister(&omap3_iommu_pdev[i]);

So... this can never be bug free - you can _never_ unregister statically
allocated devices.  Not even if you provide an empty release function.

If you want to register and unregister device structures, it must be
done using the correct APIs, and in the case of platform devices, that's
the platform_device_alloc(), platform_device_add() and
platform_device_unregister() APIs.

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

* [PATCH 3/6] omap iommu: omap3 iommu device registration
  2009-01-16  8:37 [PATCH 0/6] arm: omap iommu: add initial support Hiroshi DOYU
@ 2009-01-16  8:37 ` Hiroshi DOYU
  2009-01-17 16:21   ` Russell King - ARM Linux
  0 siblings, 1 reply; 53+ messages in thread
From: Hiroshi DOYU @ 2009-01-16  8:37 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-arm-kernel, linux-omap

Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---

 arch/arm/mach-omap2/omap3-iommu.c |  111 +++++++++++++++++++++++++++++++++++++
 1 files changed, 111 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-omap2/omap3-iommu.c

diff --git a/arch/arm/mach-omap2/omap3-iommu.c b/arch/arm/mach-omap2/omap3-iommu.c
new file mode 100644
index 0000000..52d0e56
--- /dev/null
+++ b/arch/arm/mach-omap2/omap3-iommu.c
@@ -0,0 +1,111 @@
+/*
+ * omap iommu: omap3 device registration
+ *
+ * Copyright (C) 2008 Nokia Corporation
+ *
+ * Written by Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/io.h>
+#include <linux/platform_device.h>
+
+#include <mach/iommu.h>
+
+#define DEVNAME "omap-iommu"
+
+/* Camera ISP MMU */
+#define OMAP3_MMU1_BASE		0x480bd400
+#define OMAP3_MMU1_IRQ		24
+
+/* IVA2.2 MMU */
+#define OMAP3_MMU2_BASE		0x5d000000
+#define OMAP3_MMU2_IRQ		28
+
+static struct resource iommu1_res[] = { /* Camera ISP MMU */
+	{
+		.start		= OMAP3_MMU1_BASE,
+		.end		= OMAP3_MMU1_BASE + MMU_REG_SIZE - 1,
+		.flags		= IORESOURCE_MEM,
+	},
+	{
+		.start		= OMAP3_MMU1_IRQ,
+		.flags		= IORESOURCE_IRQ,
+	},
+};
+
+static struct resource iommu2_res[] = { /* IVA2.2 MMU */
+	{
+		.start		= OMAP3_MMU2_BASE,
+		.end		= OMAP3_MMU2_BASE + MMU_REG_SIZE - 1,
+		.flags		= IORESOURCE_MEM,
+	},
+	{
+		.start		= OMAP3_MMU2_IRQ,
+		.flags		= IORESOURCE_IRQ,
+	},
+};
+
+static struct iommu_platform_data omap3_iommu_pdata[] = {
+	{
+		.name = "isp",
+		.nr_tlb_entries = 8,
+		.clk_name = "cam_ick",
+	},
+	{
+		.name = "iva2",
+		.nr_tlb_entries = 32,
+		.clk_name = "iva2_ck",
+	},
+};
+
+static void omap3_iommu_release(struct device *dev)
+{
+}
+
+static struct platform_device omap3_iommu_pdev[] = {
+	{
+		.name		= DEVNAME,
+		.id		= 1,
+		.num_resources	= ARRAY_SIZE(iommu1_res),
+		.resource	= iommu1_res,
+		.dev		= {
+			.release = omap3_iommu_release,
+			.platform_data = &omap3_iommu_pdata[0],
+		},
+	},
+	{
+		.name		= DEVNAME,
+		.id		= 2,
+		.num_resources	= ARRAY_SIZE(iommu2_res),
+		.resource	= iommu2_res,
+		.dev		= {
+			.release = omap3_iommu_release,
+			.platform_data = &omap3_iommu_pdata[1],
+		},
+	},
+};
+
+static int __init omap3_iommu_init(void)
+{
+	int i;
+	for (i = 0; i < ARRAY_SIZE(omap3_iommu_pdev); i++)
+		platform_device_register(&omap3_iommu_pdev[i]);
+	return 0;
+}
+module_init(omap3_iommu_init);
+
+static void __exit omap3_iommu_exit(void)
+{
+	int i;
+	for (i = 0; i < ARRAY_SIZE(omap3_iommu_pdev); i++)
+		platform_device_unregister(&omap3_iommu_pdev[i]);
+}
+module_exit(omap3_iommu_exit);
+
+MODULE_AUTHOR("Hiroshi DOYU");
+MODULE_DESCRIPTION("omap iommu: omap3 device registration");
+MODULE_LICENSE("GPL v2");


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

end of thread, other threads:[~2009-05-19  9:43 UTC | newest]

Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-05-05 12:46 [PATCH 0/6] Initial support for omap iommu driver Hiroshi DOYU
2009-05-05 12:46 ` [PATCH 1/6] omap iommu: tlb and pagetable primitives Hiroshi DOYU
2009-05-05 20:19   ` Kanigeri, Hari
2009-05-06  5:56     ` Hiroshi DOYU
2009-05-05 12:46 ` [PATCH 2/6] omap iommu: omap2 architecture specific functions Hiroshi DOYU
2009-05-06 14:31   ` Kanigeri, Hari
2009-05-07  5:12     ` Hiroshi DOYU
2009-05-07 14:40       ` Kanigeri, Hari
2009-05-05 12:47 ` [PATCH 3/6] omap iommu: omap3 iommu device registration Hiroshi DOYU
2009-05-05 19:32   ` Felipe Contreras
2009-05-06  6:00     ` Hiroshi DOYU
2009-05-07 20:02       ` Felipe Contreras
2009-05-07 20:11         ` [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration Felipe Contreras
2009-05-07 20:11           ` [RFC/PATCH 1/3] omap3-iommu: reorganize Felipe Contreras
2009-05-07 20:11             ` [RFC/PATCH 2/3] omap3-iommu: split init function into omap_iommu_add Felipe Contreras
2009-05-07 20:11               ` [RFC/PATCH 3/3] omap3-iommu: remote registration Felipe Contreras
2009-05-07 21:14             ` [RFC/PATCH 1/3] omap3-iommu: reorganize Felipe Balbi
2009-05-07 22:16               ` Felipe Contreras
2009-05-08  4:15               ` Hiroshi DOYU
2009-05-07 20:28           ` [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration Kanigeri, Hari
2009-05-07 20:39             ` Felipe Contreras
2009-05-07 20:54               ` Kanigeri, Hari
2009-05-08  4:14           ` Hiroshi DOYU
2009-05-08  8:04             ` Felipe Contreras
2009-05-07 20:05   ` [PATCH 3/6] omap iommu: omap3 iommu device registration Felipe Contreras
2009-05-08  4:10     ` Hiroshi DOYU
2009-05-08  7:32       ` Felipe Contreras
2009-05-08 17:21         ` Aguirre Rodriguez, Sergio Alberto
2009-05-08 22:27           ` Felipe Contreras
2009-05-16  9:20   ` Russell King - ARM Linux
2009-05-16  9:38     ` Felipe Contreras
2009-05-16  9:54       ` Russell King - ARM Linux
2009-05-16  9:56         ` Felipe Contreras
2009-05-16  9:55     ` Russell King - ARM Linux
2009-05-18  5:36       ` Hiroshi DOYU
2009-05-18  9:39       ` Hiroshi DOYU
2009-05-18 14:26       ` Hiroshi DOYU
2009-05-18 16:43         ` [PATCH] omap3-iommu: reorganize Felipe Contreras
2009-05-19  6:03           ` Hiroshi DOYU
2009-05-19  9:43             ` Felipe Contreras
2009-05-18  5:22     ` [PATCH 3/6] omap iommu: omap3 iommu device registration Hiroshi DOYU
2009-05-05 12:47 ` [PATCH 4/6] omap iommu: simple virtual address space management Hiroshi DOYU
2009-05-16  9:22   ` Russell King - ARM Linux
2009-05-18  6:31     ` Hiroshi DOYU
2009-05-18 13:10       ` Russell King - ARM Linux
2009-05-05 12:47 ` [PATCH 5/6] omap iommu: entries for Kconfig and Makefile Hiroshi DOYU
2009-05-18 13:36   ` Russell King - ARM Linux
2009-05-05 12:47 ` [PATCH 6/6] omap2 " Hiroshi DOYU
  -- strict thread matches above, loose matches on Subject: below --
2009-01-16  8:37 [PATCH 0/6] arm: omap iommu: add initial support Hiroshi DOYU
2009-01-16  8:37 ` [PATCH 3/6] omap iommu: omap3 iommu device registration Hiroshi DOYU
2009-01-17 16:21   ` Russell King - ARM Linux
2009-01-27 21:29     ` Hiroshi DOYU
2009-01-28 10:41       ` Russell King - ARM Linux
2009-01-28 11:37         ` Hiroshi DOYU

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