OMAP: iommu: hwmod, reset handling and runtime PM

PATCH Superseded Omar Ramirez Luna 3 participants 22 messages Oct 12, 2012 – Oct 19, 2012

Subsystems (6)

Device tree bindings Arm (32-bit) Arm sub-architectures OMAP2+ support +2 more subsystems

lore

Share & tools

lore

Omar Ramirez Luna

author

2012-10-12 01:06 UTC #0 lore raw

v3 cover OMAP: iommu: hwmod, reset handling and runtime PM

These patches are needed for remoteproc to work on OMAP4.

Introduced iommu hwmod support for OMAP3 (iva, isp) and OMAP4 (ipu, dsp), along with the corresponding runtime PM and routines to deassert reset lines, enable/disable clocks and configure sysc registers.

Although IOMMU hwmod patches were already submitted in the past, this series adds few more changes, like:
- New reset handling.
- Save and restore context code rework.
- Device tree bindings for OMAP3 and OMAP4.

For this series I just dropped the patches already included in mainline.

Previous work can be found at:
[v2]
http://www.mail-archive.com/linux-omap at vger.kernel.org/msg75701.html
[v1]
http://www.mail-archive.com/linux-omap at vger.kernel.org/msg70447.html

[old iteration without reset, save/restore and device tree]
http://www.mail-archive.com/linux-omap at vger.kernel.org/msg60133.html

Omar Ramirez Luna (6):
ARM: OMAP3/4: iommu: migrate to hwmod framework
ARM: OMAP3/4: iommu: adapt to runtime pm
ARM: OMAP: iommu: pm runtime save and restore context
ARM: OMAP: iommu: optimize save and restore routines
ARM: OMAP: iommu: add device tree support arm/dts: OMAP3/4: Add iommu nodes

 .../devicetree/bindings/arm/omap/iommu.txt         |   10 ++
 arch/arm/boot/dts/omap3.dtsi                       |   12 +-
 arch/arm/boot/dts/omap4.dtsi                       |   17 +-
 arch/arm/mach-omap2/devices.c                      |    2 +-
 arch/arm/mach-omap2/iommu2.c                       |   74 ++------
 arch/arm/mach-omap2/omap-iommu.c                   |  176 +++++---------------
 arch/arm/plat-omap/include/plat/iommu.h            |   20 ++-
 arch/arm/plat-omap/include/plat/iommu2.h           |    4 -
 drivers/iommu/omap-iommu.c                         |  163 ++++++++++++++----
 9 files changed, 245 insertions(+), 233 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/omap/iommu.txt

Signature · 1 line

1.7.9.5

Omar Ramirez Luna

author

2012-10-12 01:06 UTC #1 lore raw

PATCH v3 1/6 ARM: OMAP3/4: iommu: migrate to hwmod framework

Use hwmod data and device attributes to build and register an omap device for iommu driver.

- Update the naming convention in isp module.
- Remove unneeded check for number of resources, as this is now handled by omap_device and prevents driver from loading.
- Now unused, remove platform device and resource data, handling of sysconfig register for softreset purposes, use default latency structure.
- Use hwmod API for reset handling.

Signed-off-by Omar Ramirez Luna <[email protected]>

---
 arch/arm/mach-omap2/devices.c           |    2 +-
 arch/arm/mach-omap2/iommu2.c            |   19 ----
 arch/arm/mach-omap2/omap-iommu.c        |  165 +++++++------------------------
 arch/arm/plat-omap/include/plat/iommu.h |    8 +-
 drivers/iommu/omap-iommu.c              |   23 ++++-
 5 files changed, 64 insertions(+), 153 deletions(-)

arch/arm/mach-omap2/devices.c +1 -1

index c8c2117..e084b94 100644
@@ -213,7 +213,7 @@ static struct platform_device omap3isp_device = {
 };
 
 static struct omap_iommu_arch_data omap3_isp_iommu = {
-	.name = "isp",
+	.name = "mmu_isp",
 };
 
 int omap3_init_camera(struct isp_platform_data *pdata)

arch/arm/mach-omap2/iommu2.c +0 ~~-19~~

index eefc379..35cab47 100644
@@ -32,12 +32,8 @@
 #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)

@@ -88,7 +84,6 @@ static void __iommu_set_twl(struct omap_iommu *obj, bool on)
 static int omap2_iommu_enable(struct omap_iommu *obj)
 {
 	u32 l, pa;
-	unsigned long timeout;
 
 	if (!obj->iopgd || !IS_ALIGNED((u32)obj->iopgd,  SZ_16K))
 		return -EINVAL;

@@ -97,20 +92,6 @@ static int omap2_iommu_enable(struct omap_iommu *obj)
 	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);

arch/arm/mach-omap2/omap-iommu.c +38 ~~-127~~

191 lines not on this page · raw message

arch/arm/plat-omap/include/plat/iommu.h +7 -1

index 68b5f03..0fa913d 100644
@@ -13,6 +13,8 @@
 #ifndef __MACH_IOMMU_H
 #define __MACH_IOMMU_H
 
+#include <linux/platform_device.h>
+
 struct iotlb_entry {
 	u32 da;
 	u32 pa;

@@ -119,9 +121,13 @@ struct omap_mmu_dev_attr {
 struct iommu_platform_data {
 	const char *name;
 	const char *clk_name;
-	const int nr_tlb_entries;
+	const char *reset_name;
+	int nr_tlb_entries;
 	u32 da_start;
 	u32 da_end;
+
+	int (*assert_reset)(struct platform_device *pdev, const char *name);
+	int (*deassert_reset)(struct platform_device *pdev, const char *name);
 };
 
 /**

drivers/iommu/omap-iommu.c +18 -5

index d0b1234..c1caee4 100644
@@ -125,13 +125,23 @@ EXPORT_SYMBOL_GPL(omap_iommu_arch_version);
 static int iommu_enable(struct omap_iommu *obj)
 {
 	int err;
+	struct platform_device *pdev = to_platform_device(obj->dev);
+	struct iommu_platform_data *pdata = pdev->dev.platform_data;
 
-	if (!obj)
+	if (!obj || !pdata)
 		return -EINVAL;
 
 	if (!arch_iommu)
 		return -ENODEV;
 
+	if (pdata->deassert_reset) {
+		err = pdata->deassert_reset(pdev, pdata->reset_name);
+		if (err) {
+			dev_err(obj->dev, "deassert_reset failed: %d\n", err);
+			return err;
+		}
+	}
+
 	clk_enable(obj->clk);
 
 	err = arch_iommu->enable(obj);

@@ -142,7 +152,10 @@ static int iommu_enable(struct omap_iommu *obj)
 
 static void iommu_disable(struct omap_iommu *obj)
 {
-	if (!obj)
+	struct platform_device *pdev = to_platform_device(obj->dev);
+	struct iommu_platform_data *pdata = pdev->dev.platform_data;
+
+	if (!obj || !pdata)
 		return;
 
 	clk_enable(obj->clk);

@@ -150,6 +163,9 @@ static void iommu_disable(struct omap_iommu *obj)
 	arch_iommu->disable(obj);
 
 	clk_disable(obj->clk);
+
+	if (pdata->assert_reset)
+		pdata->assert_reset(pdev, pdata->reset_name);
 }
 
 /*

@@ -913,9 +929,6 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev)
 	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;

Signature · 1 line

1.7.9.5

Omar Ramirez Luna

author

2012-10-12 01:06 UTC #2 lore raw

PATCH v3 1/6 ARM: OMAP3/4: iommu: migrate to hwmod framework

In reply to Omar Ramirez Luna

Use hwmod data and device attributes to build and register an omap device for iommu driver.

Signed-off-by Omar Ramirez Luna <[email protected]>

---
 arch/arm/mach-omap2/devices.c           |    2 +-
 arch/arm/mach-omap2/iommu2.c            |   19 ----
 arch/arm/mach-omap2/omap-iommu.c        |  165 +++++++------------------------
 arch/arm/plat-omap/include/plat/iommu.h |    8 +-
 drivers/iommu/omap-iommu.c              |   23 ++++-
 5 files changed, 64 insertions(+), 153 deletions(-)

arch/arm/mach-omap2/devices.c +1 -1

index c8c2117..e084b94 100644
@@ -213,7 +213,7 @@ static struct platform_device omap3isp_device = {
 };
 
 static struct omap_iommu_arch_data omap3_isp_iommu = {
-	.name = "isp",
+	.name = "mmu_isp",
 };
 
 int omap3_init_camera(struct isp_platform_data *pdata)

arch/arm/mach-omap2/iommu2.c +0 ~~-19~~

index eefc379..35cab47 100644
@@ -32,12 +32,8 @@
 #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)

@@ -88,7 +84,6 @@ static void __iommu_set_twl(struct omap_iommu *obj, bool on)
 static int omap2_iommu_enable(struct omap_iommu *obj)
 {
 	u32 l, pa;
-	unsigned long timeout;
 
 	if (!obj->iopgd || !IS_ALIGNED((u32)obj->iopgd,  SZ_16K))
 		return -EINVAL;

@@ -97,20 +92,6 @@ static int omap2_iommu_enable(struct omap_iommu *obj)
 	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);

arch/arm/mach-omap2/omap-iommu.c +38 ~~-127~~

191 lines not on this page · raw message

arch/arm/plat-omap/include/plat/iommu.h +7 -1

index 68b5f03..0fa913d 100644
@@ -13,6 +13,8 @@
 #ifndef __MACH_IOMMU_H
 #define __MACH_IOMMU_H
 
+#include <linux/platform_device.h>
+
 struct iotlb_entry {
 	u32 da;
 	u32 pa;

@@ -119,9 +121,13 @@ struct omap_mmu_dev_attr {
 struct iommu_platform_data {
 	const char *name;
 	const char *clk_name;
-	const int nr_tlb_entries;
+	const char *reset_name;
+	int nr_tlb_entries;
 	u32 da_start;
 	u32 da_end;
+
+	int (*assert_reset)(struct platform_device *pdev, const char *name);
+	int (*deassert_reset)(struct platform_device *pdev, const char *name);
 };
 
 /**

drivers/iommu/omap-iommu.c +18 -5

index d0b1234..c1caee4 100644
@@ -125,13 +125,23 @@ EXPORT_SYMBOL_GPL(omap_iommu_arch_version);
 static int iommu_enable(struct omap_iommu *obj)
 {
 	int err;
+	struct platform_device *pdev = to_platform_device(obj->dev);
+	struct iommu_platform_data *pdata = pdev->dev.platform_data;
 
-	if (!obj)
+	if (!obj || !pdata)
 		return -EINVAL;
 
 	if (!arch_iommu)
 		return -ENODEV;
 
+	if (pdata->deassert_reset) {
+		err = pdata->deassert_reset(pdev, pdata->reset_name);
+		if (err) {
+			dev_err(obj->dev, "deassert_reset failed: %d\n", err);
+			return err;
+		}
+	}
+
 	clk_enable(obj->clk);
 
 	err = arch_iommu->enable(obj);

@@ -142,7 +152,10 @@ static int iommu_enable(struct omap_iommu *obj)
 
 static void iommu_disable(struct omap_iommu *obj)
 {
-	if (!obj)
+	struct platform_device *pdev = to_platform_device(obj->dev);
+	struct iommu_platform_data *pdata = pdev->dev.platform_data;
+
+	if (!obj || !pdata)
 		return;
 
 	clk_enable(obj->clk);

@@ -150,6 +163,9 @@ static void iommu_disable(struct omap_iommu *obj)
 	arch_iommu->disable(obj);
 
 	clk_disable(obj->clk);
+
+	if (pdata->assert_reset)
+		pdata->assert_reset(pdev, pdata->reset_name);
 }
 
 /*

@@ -913,9 +929,6 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev)
 	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;

Signature · 1 line

1.7.9.5

Omar Ramirez Luna

author

2012-10-12 01:06 UTC #3 lore raw

PATCH v3 2/6 ARM: OMAP3/4: iommu: adapt to runtime pm

In reply to Omar Ramirez Luna

Use runtime PM functionality interfaced with hwmod enable/idle functions, to replace direct clock operations and sysconfig handling.

Dues to reset sequence, pm_runtime_put_sync must be used, to avoid possible operations with the module under reset.

Signed-off-by Omar Ramirez Luna <[email protected]>

---
 arch/arm/mach-omap2/iommu2.c             |   17 -----------
 arch/arm/mach-omap2/omap-iommu.c         |    1 -
 arch/arm/plat-omap/include/plat/iommu.h  |    2 --
 arch/arm/plat-omap/include/plat/iommu2.h |    2 --
 drivers/iommu/omap-iommu.c               |   45 +++++++++++++-----------------
 5 files changed, 19 insertions(+), 48 deletions(-)

arch/arm/mach-omap2/iommu2.c +0 ~~-17~~

index 35cab47..3e47786 100644
@@ -25,15 +25,6 @@
  */
 #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_AUTOIDLE	1
-
 /* IRQSTATUS & IRQENABLE */
 #define MMU_IRQ_MULTIHITFAULT	(1 << 4)
 #define MMU_IRQ_TABLEWALKFAULT	(1 << 3)

@@ -96,11 +87,6 @@ static int omap2_iommu_enable(struct omap_iommu *obj)
 	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, pa, MMU_TTB);
 
 	__iommu_set_twl(obj, true);

@@ -114,7 +100,6 @@ static void omap2_iommu_disable(struct omap_iommu *obj)
 
 	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);
 }

@@ -243,8 +228,6 @@ omap2_iommu_dump_ctx(struct omap_iommu *obj, char *buf, ssize_t len)
 	char *p = buf;
 
 	pr_reg(REVISION);
-	pr_reg(SYSCONFIG);
-	pr_reg(SYSSTATUS);
 	pr_reg(IRQSTATUS);
 	pr_reg(IRQENABLE);
 	pr_reg(WALKING_ST);

arch/arm/mach-omap2/omap-iommu.c +0 -1

index e400845..82a422a 100644
@@ -34,7 +34,6 @@ static int __init omap_iommu_dev_init(struct omap_hwmod *oh, void *unused)
 		return -ENOMEM;
 
 	pdata->name = oh->name;
-	pdata->clk_name = oh->main_clk;
 	pdata->nr_tlb_entries = a->nr_tlb_entries;
 	pdata->da_start = a->da_start;
 	pdata->da_end = a->da_end;

arch/arm/plat-omap/include/plat/iommu.h +0 -2

index 0fa913d..902d193 100644
@@ -30,7 +30,6 @@ struct iotlb_entry {
 struct omap_iommu {
 	const char	*name;
 	struct module	*owner;
-	struct clk	*clk;
 	void __iomem	*regbase;
 	struct device	*dev;
 	void		*isr_priv;

@@ -120,7 +119,6 @@ struct omap_mmu_dev_attr {
 
 struct iommu_platform_data {
 	const char *name;
-	const char *clk_name;
 	const char *reset_name;
 	int nr_tlb_entries;
 	u32 da_start;

arch/arm/plat-omap/include/plat/iommu2.h +0 -2

index d4116b5..1579694 100644
@@ -19,8 +19,6 @@
  * 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

drivers/iommu/omap-iommu.c +19 ~~-26~~

166 lines not on this page · raw message

Signature · 1 line

1.7.9.5

Omar Ramirez Luna

author

2012-10-12 01:06 UTC #4 lore raw

PATCH v3 2/6 ARM: OMAP3/4: iommu: adapt to runtime pm

In reply to Omar Ramirez Luna

Use runtime PM functionality interfaced with hwmod enable/idle functions, to replace direct clock operations and sysconfig handling.

Dues to reset sequence, pm_runtime_put_sync must be used, to avoid possible operations with the module under reset.

Signed-off-by Omar Ramirez Luna <[email protected]>

---
 arch/arm/mach-omap2/iommu2.c             |   17 -----------
 arch/arm/mach-omap2/omap-iommu.c         |    1 -
 arch/arm/plat-omap/include/plat/iommu.h  |    2 --
 arch/arm/plat-omap/include/plat/iommu2.h |    2 --
 drivers/iommu/omap-iommu.c               |   45 +++++++++++++-----------------
 5 files changed, 19 insertions(+), 48 deletions(-)

arch/arm/mach-omap2/iommu2.c +0 ~~-17~~

index 35cab47..3e47786 100644
@@ -25,15 +25,6 @@
  */
 #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_AUTOIDLE	1
-
 /* IRQSTATUS & IRQENABLE */
 #define MMU_IRQ_MULTIHITFAULT	(1 << 4)
 #define MMU_IRQ_TABLEWALKFAULT	(1 << 3)

@@ -96,11 +87,6 @@ static int omap2_iommu_enable(struct omap_iommu *obj)
 	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, pa, MMU_TTB);
 
 	__iommu_set_twl(obj, true);

@@ -114,7 +100,6 @@ static void omap2_iommu_disable(struct omap_iommu *obj)
 
 	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);
 }

@@ -243,8 +228,6 @@ omap2_iommu_dump_ctx(struct omap_iommu *obj, char *buf, ssize_t len)
 	char *p = buf;
 
 	pr_reg(REVISION);
-	pr_reg(SYSCONFIG);
-	pr_reg(SYSSTATUS);
 	pr_reg(IRQSTATUS);
 	pr_reg(IRQENABLE);
 	pr_reg(WALKING_ST);

arch/arm/mach-omap2/omap-iommu.c +0 -1

index e400845..82a422a 100644
@@ -34,7 +34,6 @@ static int __init omap_iommu_dev_init(struct omap_hwmod *oh, void *unused)
 		return -ENOMEM;
 
 	pdata->name = oh->name;
-	pdata->clk_name = oh->main_clk;
 	pdata->nr_tlb_entries = a->nr_tlb_entries;
 	pdata->da_start = a->da_start;
 	pdata->da_end = a->da_end;

arch/arm/plat-omap/include/plat/iommu.h +0 -2

index 0fa913d..902d193 100644
@@ -30,7 +30,6 @@ struct iotlb_entry {
 struct omap_iommu {
 	const char	*name;
 	struct module	*owner;
-	struct clk	*clk;
 	void __iomem	*regbase;
 	struct device	*dev;
 	void		*isr_priv;

@@ -120,7 +119,6 @@ struct omap_mmu_dev_attr {
 
 struct iommu_platform_data {
 	const char *name;
-	const char *clk_name;
 	const char *reset_name;
 	int nr_tlb_entries;
 	u32 da_start;

arch/arm/plat-omap/include/plat/iommu2.h +0 -2

index d4116b5..1579694 100644
@@ -19,8 +19,6 @@
  * 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

drivers/iommu/omap-iommu.c +19 ~~-26~~

166 lines not on this page · raw message

Signature · 1 line

1.7.9.5

Felipe Contreras

2012-10-12 07:48 UTC #5 lore raw

PATCH 2/6 ARM: OMAP3/4: iommu: adapt to runtime pm

Quoted 6 lines · Omar Ramirez Luna

Use runtime PM functionality interfaced with hwmod enable/idle functions, to replace direct clock operations and sysconfig handling.
Dues to reset sequence, pm_runtime_put_sync must be used, to avoid possible operations with the module under reset.

I already made most of these comments, but here they go again.

Review static int iommu_enable(struct omap_iommu *obj) · line 149

Quoted 10 lines

(quoted diff) +1 -2

@@ -142,11 +142,10 @@ static int iommu_enable(struct omap_iommu *obj)
                }
        }
 
-       clk_enable(obj->clk);
+       pm_runtime_get_sync(obj->dev);
 
        err = arch_iommu->enable(obj);
 
-       clk_disable(obj->clk);

The device will never go to sleep, until iommu_disable is called. clk_enable -> pm_runtime_get_sync, clk_disable pm_runtime_put.

Quoted 10 lines

(quoted diff) +1 -1

@@ -288,7 +285,7 @@ static int load_iotlb_entry(struct omap_iommu *obj, struct iotlb_entry *e)
        if (!obj || !obj->nr_tlb_entries || !e)
                return -EINVAL;
 
-       clk_enable(obj->clk);
+       pm_runtime_get_sync(obj->dev);
 
        iotlb_lock_get(obj, &l);
        if (l.base == obj->nr_tlb_entries) {

Review static int load_iotlb_entry(struct omap_iommu *obj, struct iotlb_entry *e) · line 320

Quoted 7 lines

(quoted diff) +1 -1

@@ -318,7 +315,7 @@ static int load_iotlb_entry(struct omap_iommu *obj, struct iotlb_entry *e)
 
        cr = iotlb_alloc_cr(obj, e);
        if (IS_ERR(cr)) {
-               clk_disable(obj->clk);
+               pm_runtime_put_sync(obj->dev);
                return PTR_ERR(cr);
        }

If I'm correct, the above pm_runtime_get/put are redundant, because the count can't possibly reach 0 because of the reason I explained before.

The same for all the cases below.

Review static int __devexit omap_iommu_remove(struct platform_device *pdev) · line 1004

Quoted 6 lines

(quoted diff) +1 -1

@@ -1009,7 +1001,8 @@ static int __devexit omap_iommu_remove(struct platform_device *pdev)
        release_mem_region(res->start, resource_size(res));
        iounmap(obj->regbase);
 
-       clk_put(obj->clk);
+       pm_runtime_disable(obj->dev);

This will turn on the device unnecessarily, wasting power, and there's no need for that, kfree will take care of that without resuming.

Review static int __devexit omap_iommu_remove(struct platform_device *pdev) · line 1007

Quoted 3 lines

(quoted diff) +0 -0

        dev_info(&pdev->dev, "%s removed\n", obj->name);
        kfree(obj);
        return 0;

Also, I still think that something like this is needed:

arch/arm/mach-omap2/clock3xxx_data.c +11 -1

@@ -2222,8 +2222,17 @@ static struct clk cam_mclk = {
        .recalc         = &followparent_recalc,
 };
 
+static struct clk cam_fck = {
+       .name           = "cam_fck",
+       .ops            = &clkops_omap2_iclk_dflt,
+       .parent         = &l3_ick,
+       .enable_reg     = OMAP_CM_REGADDR(OMAP3430_CAM_MOD, CM_ICLKEN),
+       .enable_bit     = OMAP3430_EN_CAM_SHIFT,
+       .clkdm_name     = "cam_clkdm",
+       .recalc         = &followparent_recalc,
+};
+
 static struct clk cam_ick = {
-       /* Handles both L3 and L4 clocks */
        .name           = "cam_ick",
        .ops            = &clkops_omap2_iclk_dflt,
        .parent         = &l4_ick,

@@ -3394,6 +3403,7 @@ static struct omap_clk omap3xxx_clks[] = {
        CLK("omapdss_dss",      "ick",          &dss_ick_3430es2,
 CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
        CLK(NULL,       "dss_ick",              &dss_ick_3430es2,
 CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
        CLK(NULL,       "cam_mclk",     &cam_mclk,      CK_34XX | CK_36XX),
+       CLK(NULL,       "cam_fck",      &cam_fck,       CK_34XX | CK_36XX),
        CLK(NULL,       "cam_ick",      &cam_ick,       CK_34XX | CK_36XX),

        CLK(NULL,       "csi2_96m_fck", &csi2_96m_fck,  CK_34XX | CK_36XX),
        CLK(NULL,       "usbhost_120m_fck", &usbhost_120m_fck,

CK_3430ES2PLUS | CK_AM35XX | CK_36XX),

Cheers.

Signature · 1 line

Felipe Contreras

Omar Ramirez Luna

author

2012-10-12 17:46 UTC #6 lore raw

PATCH v3 2/6 ARM: OMAP3/4: iommu: adapt to runtime pm

Quoted 1 line · Felipe Contreras

I already made most of these comments, but here they go again.

I replied to all, but here it goes again:

Quoted 7 lines

(quoted diff) +1 -1

@@ -142,11 +142,10 @@ static int iommu_enable(struct omap_iommu *obj)
                }
        }
 
-       clk_enable(obj->clk);
+       pm_runtime_get_sync(obj->dev);

Quoted 6 lines

Review static int iommu_enable(struct omap_iommu *obj) · line 149
Quoted 3 lines
(quoted diff) +0 -1
        err = arch_iommu->enable(obj);
 
-       clk_disable(obj->clk);
The device will never go to sleep, until iommu_disable is called. clk_enable -> pm_runtime_get_sync, clk_disable pm_runtime_put.

Which is what you want... why would you want your iommu to be disabled if the client of that iommu could request a translation?

Remember that these iommus, sit along of other processors not on the main processor side. So, this code should enable it for the other processor to use, and there is no point where the processor can say "I'm not using it, shut it down" or "I'm using it, turn it on" in the middle of execution, other than suspend/resume and if supported, autosuspend.

Quoted 22 lines

Quoted 10 lines

(quoted diff) +1 -1

@@ -288,7 +285,7 @@ static int load_iotlb_entry(struct omap_iommu *obj, struct iotlb_entry *e)
        if (!obj || !obj->nr_tlb_entries || !e)
                return -EINVAL;
 
-       clk_enable(obj->clk);
+       pm_runtime_get_sync(obj->dev);
 
        iotlb_lock_get(obj, &l);
        if (l.base == obj->nr_tlb_entries) {

Review static int load_iotlb_entry(struct omap_iommu *obj, struct iotlb_entry *e) · line 320

Quoted 7 lines

(quoted diff) +1 -1

@@ -318,7 +315,7 @@ static int load_iotlb_entry(struct omap_iommu *obj, struct iotlb_entry *e)
 
        cr = iotlb_alloc_cr(obj, e);
        if (IS_ERR(cr)) {
-               clk_disable(obj->clk);
+               pm_runtime_put_sync(obj->dev);
                return PTR_ERR(cr);
        }

If I'm correct, the above pm_runtime_get/put are redundant, because the count can't possibly reach 0 because of the reason I explained before.

Quoted 1 line

The same for all the cases below.

You can access this paths through debugfs, some of them doesn't work if the module is not enabled first, but in future if you just want to idle the iommu withouth freeing, these are needed to debug.

BTW, the next patch in the series: ARM: OMAP: iommu: pm runtime save and restore context, let's you do a pm_runtime_[enable|put] through save/restore ctx functions, which is just for compatibility on how isp code uses the save and restore code.

Quoted 9 lines

Review static int __devexit omap_iommu_remove(struct platform_device *pdev) · line 1004
Quoted 6 lines
(quoted diff) +1 -1
@@ -1009,7 +1001,8 @@ static int __devexit omap_iommu_remove(struct platform_device *pdev)
        release_mem_region(res->start, resource_size(res));
        iounmap(obj->regbase);
 
-       clk_put(obj->clk);
+       pm_runtime_disable(obj->dev);
This will turn on the device unnecessarily, wasting power, and there's no need for that, kfree will take care of that without resuming.

Left aside the aesthetics of having balanced calls, the device will be enabled if there was a pending resume to be executed, otherwise it won't, kfree won't increment the disable_depth counter and I don't think that freeing the pointer is enough reason to ignore pm_runtime_disable.

Quoted 1 line

Also, I still think that something like this is needed:

...

arch/arm/mach-omap2/clock3xxx_data.c static struct clk cam_mclk = { · line 2229

Quoted 5 lines

arch/arm/mach-omap2/clock3xxx_data.c +5 -0

+static struct clk cam_fck = {
+       .name           = "cam_fck",
+       .ops            = &clkops_omap2_iclk_dflt,
+       .parent         = &l3_ick,
+       .enable_reg     = OMAP_CM_REGADDR(OMAP3430_CAM_MOD, CM_ICLKEN),

a cam_fck name to enable the ick?

Cheers,

Omar

Felipe Contreras

2012-10-12 21:25 UTC #7 lore raw

PATCH 2/6 ARM: OMAP3/4: iommu: adapt to runtime pm

Quoted 4 lines · Omar Ramirez Luna

Quoted 1 line · Felipe Contreras
I already made most of these comments, but here they go again.
I replied to all, but here it goes again:

Mostly, but not all :)

Quoted 9 lines

(quoted diff) +1 -1

@@ -142,11 +142,10 @@ static int iommu_enable(struct omap_iommu *obj)
                }
        }
 
-       clk_enable(obj->clk);
+       pm_runtime_get_sync(obj->dev);
 
        err = arch_iommu->enable(obj);

Quoted 7 lines

Quoted 4 lines
Review static int iommu_enable(struct omap_iommu *obj) · line 149
Quoted 1 line
(quoted diff) +0 -1
-       clk_disable(obj->clk);
The device will never go to sleep, until iommu_disable is called. clk_enable -> pm_runtime_get_sync, clk_disable pm_runtime_put.
Which is what you want... why would you want your iommu to be disabled if the client of that iommu could request a translation?

That's the whole point of *dynamic* pm; _when_ the client wants to request a translation, _then_ the device is waken up, which is what I believe the code currently does.

After your patch, even if I don't use the camera, or the DSP, the iommu devices will be enabled, and will be consuming energy *all the time*. Which I don't think is what we want.

I'm not saying I have a solution, I'm simply saying that's what's going to happen if I'm correct.

Quoted 6 lines

Remember that these iommus, sit along of other processors not on the main processor side. So, this code should enable it for the other processor to use, and there is no point where the processor can say "I'm not using it, shut it down" or "I'm using it, turn it on" in the middle of execution, other than suspend/resume and if supported, autosuspend.

I understand, but perhaps there should be?

Quoted 24 lines

Quoted 22 lines

Quoted 10 lines

(quoted diff) +1 -1

@@ -288,7 +285,7 @@ static int load_iotlb_entry(struct omap_iommu *obj, struct iotlb_entry *e)
        if (!obj || !obj->nr_tlb_entries || !e)
                return -EINVAL;
 
-       clk_enable(obj->clk);
+       pm_runtime_get_sync(obj->dev);
 
        iotlb_lock_get(obj, &l);
        if (l.base == obj->nr_tlb_entries) {

Review static int load_iotlb_entry(struct omap_iommu *obj, struct iotlb_entry *e) · line 320

Quoted 7 lines

(quoted diff) +1 -1

@@ -318,7 +315,7 @@ static int load_iotlb_entry(struct omap_iommu *obj, struct iotlb_entry *e)
 
        cr = iotlb_alloc_cr(obj, e);
        if (IS_ERR(cr)) {
-               clk_disable(obj->clk);
+               pm_runtime_put_sync(obj->dev);
                return PTR_ERR(cr);
        }

If I'm correct, the above pm_runtime_get/put are redundant, because the count can't possibly reach 0 because of the reason I explained before.

Quoted 1 line

The same for all the cases below.

Quoted 8 lines

You can access this paths through debugfs, some of them doesn't work if the module is not enabled first, but in future if you just want to idle the iommu withouth freeing, these are needed to debug.
BTW, the next patch in the series: ARM: OMAP: iommu: pm runtime save and restore context, let's you do a pm_runtime_[enable|put] through save/restore ctx functions, which is just for compatibility on how isp code uses the save and restore code.

All right, it has been some time since I've touched pm code, so if you say so.

Quoted 10 lines

Quoted 9 lines
Review static int __devexit omap_iommu_remove(struct platform_device *pdev) · line 1004
Quoted 6 lines
(quoted diff) +1 -1
@@ -1009,7 +1001,8 @@ static int __devexit omap_iommu_remove(struct platform_device *pdev)
        release_mem_region(res->start, resource_size(res));
        iounmap(obj->regbase);
 
-       clk_put(obj->clk);
+       pm_runtime_disable(obj->dev);
This will turn on the device unnecessarily, wasting power, and there's no need for that, kfree will take care of that without resuming.

Quoted 5 lines

Left aside the aesthetics of having balanced calls, the device will be enabled if there was a pending resume to be executed, otherwise it won't, kfree won't increment the disable_depth counter and I don't think that freeing the pointer is enough reason to ignore pm_runtime_disable.

You are doing __pm_runtime_disable(dev, true), kfree will do __pm_runtime_disable(dev, false), which is what we want. Both will decrement the disable_depth.

But at least you agree that there's a chance that the device will be waken up.

Quoted 9 lines

Quoted 1 line
Also, I still think that something like this is needed:
...
arch/arm/mach-omap2/clock3xxx_data.c static struct clk cam_mclk = { · line 2229
Quoted 5 lines
arch/arm/mach-omap2/clock3xxx_data.c +5 -0
+static struct clk cam_fck = {
+       .name           = "cam_fck",
+       .ops            = &clkops_omap2_iclk_dflt,
+       .parent         = &l3_ick,
+       .enable_reg     = OMAP_CM_REGADDR(OMAP3430_CAM_MOD, CM_ICLKEN),
a cam_fck name to enable the ick?

Yeap, according to the TRM. Take a look at 12.3 Camera ISP Integration Fig 12-50.

Cheers.

Signature · 1 line

Felipe Contreras

Omar Ramirez Luna

author

2012-10-16 01:29 UTC #8 lore raw

PATCH v3 2/6 ARM: OMAP3/4: iommu: adapt to runtime pm

Hi Felipe,

Quoted 14 lines · Felipe Contreras

Quoted 14 lines
Quoted 13 lines
Review static int iommu_enable(struct omap_iommu *obj) · line 149
Quoted 10 lines
(quoted diff) +1 -2
@@ -142,11 +142,10 @@ static int iommu_enable(struct omap_iommu *obj)
                }
        }
 
-       clk_enable(obj->clk);
+       pm_runtime_get_sync(obj->dev);
 
        err = arch_iommu->enable(obj);
 
-       clk_disable(obj->clk);
The device will never go to sleep, until iommu_disable is called. clk_enable -> pm_runtime_get_sync, clk_disable pm_runtime_put.

Quoted 6 lines

Quoted 2 lines
Which is what you want... why would you want your iommu to be disabled if the client of that iommu could request a translation?
That's the whole point of *dynamic* pm; _when_ the client wants to request a translation, _then_ the device is waken up, which is what I believe the code currently does.

No it doesn't, current code is working because the processor and the iommu share the same clock, so enabling the processor is implicitly guaranteeing that the iommu will be enabled. IMHO, there shouldn't be such assumption that you can control both with the same clock.

So, once the remote processor is enabled, any "dynamic pm" from iommu with current code has no effect because the clock was already enabled for the processor.

Quoted 3 lines

After your patch, even if I don't use the camera, or the DSP, the iommu devices will be enabled, and will be consuming energy *all the time*. Which I don't think is what we want.

Wrong, the iommu device will be enabled by pm_runtime_get_sync once you decide to attach with iommu_attach_device, if you do not use camera or the dsp, you won't turn ON the iommus.

On probe this patch does pm_runtime_enable, however this doesn't mean the device is turned ON or resumed or kept ON all the time.

Quoted 2 lines

I'm not saying I have a solution, I'm simply saying that's what's going to happen if I'm correct.

Ok, but that is not what happens here.

Quoted 8 lines

Quoted 6 lines
Remember that these iommus, sit along of other processors not on the main processor side. So, this code should enable it for the other processor to use, and there is no point where the processor can say "I'm not using it, shut it down" or "I'm using it, turn it on" in the middle of execution, other than suspend/resume and if supported, autosuspend.
I understand, but perhaps there should be?

Autosuspend is a feature missing and should handle the scenario where the remote processor can sleep dynamically, this scenario should turn off the iommu and the remote processor itself when there is no workload but it depends on the remote processor activity not the iommu activity.

Quoted 16 lines

Quoted 10 lines
Quoted 9 lines
Review static int __devexit omap_iommu_remove(struct platform_device *pdev) · line 1004
Quoted 6 lines
(quoted diff) +1 -1
@@ -1009,7 +1001,8 @@ static int __devexit omap_iommu_remove(struct platform_device *pdev)
        release_mem_region(res->start, resource_size(res));
        iounmap(obj->regbase);
 
-       clk_put(obj->clk);
+       pm_runtime_disable(obj->dev);
This will turn on the device unnecessarily, wasting power, and there's no need for that, kfree will take care of that without resuming.
Quoted 5 lines
Left aside the aesthetics of having balanced calls, the device will be enabled if there was a pending resume to be executed, otherwise it won't, kfree won't increment the disable_depth counter and I don't think that freeing the pointer is enough reason to ignore pm_runtime_disable.

Quoted 3 lines

You are doing __pm_runtime_disable(dev, true), kfree will do __pm_runtime_disable(dev, false), which is what we want. Both will decrement the disable_depth.

I'm quite confused here, could you please point me to the kfree snip that does __pm_runtime_disable(dev, false)?

Quoted 1 line

But at least you agree that there's a chance that the device will be waken up.

Of course, if there is a pending resume to be executed, it must honor that resume request and then turn off the device before removing the iommu, IMHO.

Quoted 8 lines

Quoted 8 lines
Quoted 1 line
Also, I still think that something like this is needed:
...
Quoted 5 lines
arch/arm/mach-omap2/clock3xxx_data.c +5 -0
+static struct clk cam_fck = {
+       .name           = "cam_fck",
+       .ops            = &clkops_omap2_iclk_dflt,
+       .parent         = &l3_ick,
+       .enable_reg     = OMAP_CM_REGADDR(OMAP3430_CAM_MOD, CM_ICLKEN),

Quoted 4 lines

Quoted 1 line
a cam_fck name to enable the ick?
Yeap, according to the TRM. Take a look at 12.3 Camera ISP Integration Fig 12-50.

What I meant is that, you are using the CM_ICLKEN to enable a clock named "cam_fck" which has l3_ick as a parent. And that is not consistent with what that register is meant to do, which is:

4.14.1.10 CAM_CM Registers

CM_ICKLEN_CAM
0x0: CAM_L3_ICK and CAM_L4_ICLK are disabled
0x1: CAM_L3_ICK and CAM_L4_ICLK are enabled

So, I'm complaining about the name "cam_fck", for an interface clock with parent l3_ick. However I don't know why on section 12.3 they refer to CAM_FCK to a l3_ick child clock.

Cheers,

Omar

Felipe Contreras

2012-10-16 03:23 UTC #9 lore raw

PATCH 2/6 ARM: OMAP3/4: iommu: adapt to runtime pm

Hi,

Quoted 7 lines · Omar Ramirez Luna

Quoted 3 lines
After your patch, even if I don't use the camera, or the DSP, the iommu devices will be enabled, and will be consuming energy *all the time*. Which I don't think is what we want.
Wrong, the iommu device will be enabled by pm_runtime_get_sync once you decide to attach with iommu_attach_device, if you do not use camera or the dsp, you won't turn ON the iommus.

I see, somehow I conflated the two functions.

Quoted 2 lines

On probe this patch does pm_runtime_enable, however this doesn't mean the device is turned ON or resumed or kept ON all the time.

In fact it's the other way around; pm_runtime_enable turns off the power (if it's ON).

Quoted 16 lines

Quoted 16 lines
Quoted 15 lines
Quoted 9 lines
Review static int __devexit omap_iommu_remove(struct platform_device *pdev) · line 1004
Quoted 6 lines
(quoted diff) +1 -1
@@ -1009,7 +1001,8 @@ static int __devexit omap_iommu_remove(struct platform_device *pdev)
        release_mem_region(res->start, resource_size(res));
        iounmap(obj->regbase);
 
-       clk_put(obj->clk);
+       pm_runtime_disable(obj->dev);
This will turn on the device unnecessarily, wasting power, and there's no need for that, kfree will take care of that without resuming.
Left aside the aesthetics of having balanced calls, the device will be enabled if there was a pending resume to be executed, otherwise it won't, kfree won't increment the disable_depth counter and I don't think that freeing the pointer is enough reason to ignore pm_runtime_disable.

Quoted 6 lines

Quoted 3 lines
You are doing __pm_runtime_disable(dev, true), kfree will do __pm_runtime_disable(dev, false), which is what we want. Both will decrement the disable_depth.
I'm quite confused here, could you please point me to the kfree snip that does __pm_runtime_disable(dev, false)?

Sorry, not kfree, but the device removal process:

device_del
device_pm_remove
pm_runtime_remove
__pm_runtime_disable <- HERE
bus_remove_device
device_release_driver
__device_release_driver
.remove => platform_drv_remove
.remove => omap_iommu_remove

Actually, it seems the pm is disabled _before_ omap_iommu_remove is even called, so it's a no-op.

Quoted 5 lines

Quoted 1 line
But at least you agree that there's a chance that the device will be waken up.
Of course, if there is a pending resume to be executed, it must honor that resume request and then turn off the device before removing the iommu, IMHO.

Who will turn off the device afterwards?

Quoted 19 lines

Quoted 8 lines
Quoted 8 lines
Quoted 1 line
Also, I still think that something like this is needed:
...
Quoted 5 lines
arch/arm/mach-omap2/clock3xxx_data.c +5 -0
+static struct clk cam_fck = {
+       .name           = "cam_fck",
+       .ops            = &clkops_omap2_iclk_dflt,
+       .parent         = &l3_ick,
+       .enable_reg     = OMAP_CM_REGADDR(OMAP3430_CAM_MOD, CM_ICLKEN),
Quoted 4 lines
Quoted 1 line
a cam_fck name to enable the ick?
Yeap, according to the TRM. Take a look at 12.3 Camera ISP Integration Fig 12-50.
What I meant is that, you are using the CM_ICLKEN to enable a clock named "cam_fck" which has l3_ick as a parent. And that is not consistent with what that register is meant to do, which is:
4.14.1.10 CAM_CM Registers

Quoted 7 lines

CM_ICKLEN_CAM
0x0: CAM_L3_ICK and CAM_L4_ICLK are disabled
0x1: CAM_L3_ICK and CAM_L4_ICLK are enabled
So, I'm complaining about the name "cam_fck", for an interface clock with parent l3_ick. However I don't know why on section 12.3 they refer to CAM_FCK to a l3_ick child clock.

Because it's also used as a functional clock. Anyway, I don't care much about the name of the clock, what is clear is that there's a link missing to the l3_ick.

Cheers.

Signature · 1 line

Felipe Contreras

Omar Ramirez Luna

author

2012-10-17 23:51 UTC #10 lore raw

PATCH v3 2/6 ARM: OMAP3/4: iommu: adapt to runtime pm

Quoted 5 lines · Felipe Contreras

Quoted 2 lines
On probe this patch does pm_runtime_enable, however this doesn't mean the device is turned ON or resumed or kept ON all the time.
In fact it's the other way around; pm_runtime_enable turns off the power (if it's ON).

pm_runtime_enable just decrements the disable_depth counter. Doesn't turn off anything by itself.

Quoted 23 lines

Quoted 16 lines
Quoted 16 lines
Quoted 15 lines
Quoted 9 lines
Review static int __devexit omap_iommu_remove(struct platform_device *pdev) · line 1004
Quoted 6 lines
(quoted diff) +1 -1
@@ -1009,7 +1001,8 @@ static int __devexit omap_iommu_remove(struct platform_device *pdev)
        release_mem_region(res->start, resource_size(res));
        iounmap(obj->regbase);
 
-       clk_put(obj->clk);
+       pm_runtime_disable(obj->dev);
This will turn on the device unnecessarily, wasting power, and there's no need for that, kfree will take care of that without resuming.
Left aside the aesthetics of having balanced calls, the device will be enabled if there was a pending resume to be executed, otherwise it won't, kfree won't increment the disable_depth counter and I don't think that freeing the pointer is enough reason to ignore pm_runtime_disable.
Quoted 6 lines
Quoted 3 lines
You are doing __pm_runtime_disable(dev, true), kfree will do __pm_runtime_disable(dev, false), which is what we want. Both will decrement the disable_depth.
I'm quite confused here, could you please point me to the kfree snip that does __pm_runtime_disable(dev, false)?

Quoted 6 lines

Sorry, not kfree, but the device removal process:
device_del
device_pm_remove
pm_runtime_remove
__pm_runtime_disable <- HERE

I'm not entirely convinced _iommu_ follows this path, it doesn't create any devices nor register them... whereas mailbox does create devices and mailbox does follow this path for the devices created which are child devices.

So this path won't take care of the omap-iommu device pm_runtime_disable.

Quoted 7 lines

Quoted 5 lines
Quoted 1 line
But at least you agree that there's a chance that the device will be waken up.
Of course, if there is a pending resume to be executed, it must honor that resume request and then turn off the device before removing the iommu, IMHO.
Who will turn off the device afterwards?

What I meant is that omap_iommu_detach should turn off the device before removing the iommu.

Cheers,

Omar

Felipe Contreras

2012-10-18 04:03 UTC #11 lore raw

PATCH 2/6 ARM: OMAP3/4: iommu: adapt to runtime pm

Quoted 9 lines · Omar Ramirez Luna

Quoted 5 lines · Felipe Contreras
Quoted 2 lines
On probe this patch does pm_runtime_enable, however this doesn't mean the device is turned ON or resumed or kept ON all the time.
In fact it's the other way around; pm_runtime_enable turns off the power (if it's ON).
pm_runtime_enable just decrements the disable_depth counter. Doesn't turn off anything by itself.

pm_runtime_enable turns on power management, without it the device remains in whatever state it booted.

Quoted 11 lines

Quoted 4 lines
device_del
device_pm_remove
pm_runtime_remove
__pm_runtime_disable <- HERE
I'm not entirely convinced _iommu_ follows this path, it doesn't create any devices nor register them... whereas mailbox does create devices and mailbox does follow this path for the devices created which are child devices.
So this path won't take care of the omap-iommu device pm_runtime_disable.

Are you sure? What code-path calls omap_iommu_remove then?

Anyway, it was just an observation. I've seen other code do this.

Signature · 1 line

Felipe Contreras

Omar Ramirez Luna

author

2012-10-12 01:06 UTC #12 lore raw

PATCH v3 3/6 ARM: OMAP: iommu: pm runtime save and restore context

In reply to Omar Ramirez Luna

Save and restore context during pm runtime transitions.

For now, the previous API for this purpose will trigger pm runtime functions, and will be left as exported symbol for compatibility with it's only user.

Signed-off-by Omar Ramirez Luna <[email protected]>

---
 drivers/iommu/omap-iommu.c |   29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

drivers/iommu/omap-iommu.c +27 -2

index 37644c4..875e894 100644
@@ -97,7 +97,7 @@ void omap_iommu_save_ctx(struct device *dev)
 {
 	struct omap_iommu *obj = dev_to_omap_iommu(dev);
 
-	arch_iommu->save_ctx(obj);
+	pm_runtime_put_sync(obj->dev);
 }
 EXPORT_SYMBOL_GPL(omap_iommu_save_ctx);

@@ -109,7 +109,7 @@ void omap_iommu_restore_ctx(struct device *dev)
 {
 	struct omap_iommu *obj = dev_to_omap_iommu(dev);
 
-	arch_iommu->restore_ctx(obj);
+	pm_runtime_get_sync(obj->dev);
 }
 EXPORT_SYMBOL_GPL(omap_iommu_restore_ctx);

@@ -1008,11 +1008,36 @@ static int __devexit omap_iommu_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int omap_iommu_runtime_suspend(struct device *dev)
+{
+	struct omap_iommu *obj = to_iommu(dev);
+
+	arch_iommu->save_ctx(obj);
+
+	return 0;
+}
+
+static int omap_iommu_runtime_resume(struct device *dev)
+{
+	struct omap_iommu *obj = to_iommu(dev);
+
+	arch_iommu->restore_ctx(obj);
+
+	return 0;
+}
+
+static const struct dev_pm_ops iommu_pm_ops = {
+	SET_RUNTIME_PM_OPS(omap_iommu_runtime_suspend,
+			   omap_iommu_runtime_resume,
+			   NULL)
+};
+
 static struct platform_driver omap_iommu_driver = {
 	.probe	= omap_iommu_probe,
 	.remove	= __devexit_p(omap_iommu_remove),
 	.driver	= {
 		.name	= "omap-iommu",
+		.pm	= &iommu_pm_ops,
 	},
 };

Signature · 1 line

1.7.9.5

Omar Ramirez Luna

author

2012-10-12 01:06 UTC #13 lore raw

PATCH v3 3/6 ARM: OMAP: iommu: pm runtime save and restore context

In reply to Omar Ramirez Luna

Save and restore context during pm runtime transitions.

For now, the previous API for this purpose will trigger pm runtime functions, and will be left as exported symbol for compatibility with it's only user.

Signed-off-by Omar Ramirez Luna <[email protected]>

---
 drivers/iommu/omap-iommu.c |   29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

drivers/iommu/omap-iommu.c +27 -2

index 37644c4..875e894 100644
@@ -97,7 +97,7 @@ void omap_iommu_save_ctx(struct device *dev)
 {
 	struct omap_iommu *obj = dev_to_omap_iommu(dev);
 
-	arch_iommu->save_ctx(obj);
+	pm_runtime_put_sync(obj->dev);
 }
 EXPORT_SYMBOL_GPL(omap_iommu_save_ctx);

@@ -109,7 +109,7 @@ void omap_iommu_restore_ctx(struct device *dev)
 {
 	struct omap_iommu *obj = dev_to_omap_iommu(dev);
 
-	arch_iommu->restore_ctx(obj);
+	pm_runtime_get_sync(obj->dev);
 }
 EXPORT_SYMBOL_GPL(omap_iommu_restore_ctx);

@@ -1008,11 +1008,36 @@ static int __devexit omap_iommu_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int omap_iommu_runtime_suspend(struct device *dev)
+{
+	struct omap_iommu *obj = to_iommu(dev);
+
+	arch_iommu->save_ctx(obj);
+
+	return 0;
+}
+
+static int omap_iommu_runtime_resume(struct device *dev)
+{
+	struct omap_iommu *obj = to_iommu(dev);
+
+	arch_iommu->restore_ctx(obj);
+
+	return 0;
+}
+
+static const struct dev_pm_ops iommu_pm_ops = {
+	SET_RUNTIME_PM_OPS(omap_iommu_runtime_suspend,
+			   omap_iommu_runtime_resume,
+			   NULL)
+};
+
 static struct platform_driver omap_iommu_driver = {
 	.probe	= omap_iommu_probe,
 	.remove	= __devexit_p(omap_iommu_remove),
 	.driver	= {
 		.name	= "omap-iommu",
+		.pm	= &iommu_pm_ops,
 	},
 };

Signature · 1 line

1.7.9.5

Omar Ramirez Luna

author

2012-10-12 01:06 UTC #14 lore raw

PATCH v3 4/6 ARM: OMAP: iommu: optimize save and restore routines

In reply to Omar Ramirez Luna

These functions save and restore registers irrespectively of their read or write permissions, this ends up in registers being saved that can't be restored because of read only attributes. OTOH, so far only 3 of them need to be saved.

In future GP_REG (which is present only on OMAP4 ipu) needs to be saved but right now there is no API that can alter its value. Also, protected TLB entries must be saved but this can be in a separate patch as the original code didn't implement the loop to traverse protected TLB entries.

Signed-off-by Omar Ramirez Luna <[email protected]>

---
 arch/arm/mach-omap2/iommu2.c             |   38 ++++++++++++++----------------
 arch/arm/plat-omap/include/plat/iommu.h  |   10 +++++++-
 arch/arm/plat-omap/include/plat/iommu2.h |    2 --
 drivers/iommu/omap-iommu.c               |    3 +--
 4 files changed, 28 insertions(+), 25 deletions(-)

arch/arm/mach-omap2/iommu2.c +18 ~~-20~~

index 3e47786..cd77abb 100644
@@ -19,6 +19,7 @@
 #include <linux/stringify.h>
 
 #include <plat/iommu.h>
+#include <plat/omap-pm.h>
 
 /*
  * omap2 architecture specific register bit definitions

@@ -55,20 +56,26 @@
 
 static void __iommu_set_twl(struct omap_iommu *obj, bool on)
 {
-	u32 l = iommu_read_reg(obj, MMU_CNTL);
+	u32 l;
 
 	if (on)
-		iommu_write_reg(obj, MMU_IRQ_TWL_MASK, MMU_IRQENABLE);
+		l = MMU_IRQ_TWL_MASK;
 	else
-		iommu_write_reg(obj, MMU_IRQ_TLB_MISS_MASK, MMU_IRQENABLE);
+		l = MMU_IRQ_TLB_MISS_MASK;
+
+	iommu_write_reg(obj, l, MMU_IRQENABLE);
+	obj->context.irqen = l;
 
+	l = iommu_read_reg(obj, MMU_CNTL);
 	l &= ~MMU_CNTL_MASK;
+
 	if (on)
 		l |= (MMU_CNTL_MMU_EN | MMU_CNTL_TWL_EN);
 	else
 		l |= (MMU_CNTL_MMU_EN);
 
 	iommu_write_reg(obj, l, MMU_CNTL);
+	obj->context.cntl = l;
 }

@@ -88,6 +95,7 @@ static int omap2_iommu_enable(struct omap_iommu *obj)
 		 (l >> 4) & 0xf, l & 0xf);
 
 	iommu_write_reg(obj, pa, MMU_TTB);
+	obj->context.ttb = pa;
 
 	__iommu_set_twl(obj, true);

@@ -100,6 +108,7 @@ static void omap2_iommu_disable(struct omap_iommu *obj)
 
 	l &= ~MMU_CNTL_MASK;
 	iommu_write_reg(obj, l, MMU_CNTL);
+	obj->context.cntl = l;
 
 	dev_dbg(obj->dev, "%s is shutting down\n", obj->name);
 }

@@ -249,28 +258,17 @@ out:
 
 static void omap2_iommu_save_ctx(struct omap_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);
+	obj->ctx_loss_cnt = omap_pm_get_dev_context_loss_count(obj->dev);
 }
 
 static void omap2_iommu_restore_ctx(struct omap_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]);
-	}
+	if (omap_pm_get_dev_context_loss_count(obj->dev) == obj->ctx_loss_cnt)
+		return;
 
-	BUG_ON(p[0] != IOMMU_ARCH_VERSION);
+	iommu_write_reg(obj, obj->context.ttb, MMU_TTB);
+	iommu_write_reg(obj, obj->context.irqen, MMU_IRQENABLE);
+	iommu_write_reg(obj, obj->context.cntl, MMU_CNTL);
 }
 
 static void omap2_cr_to_e(struct cr_regs *cr, struct iotlb_entry *e)

arch/arm/plat-omap/include/plat/iommu.h +9 -1

index 902d193..af14486 100644
@@ -27,6 +27,13 @@ struct iotlb_entry {
 	};
 };
 
+/* context registers */
+struct iommu_regs {
+	u32 irqen;
+	u32 cntl;
+	u32 ttb;
+};
+
 struct omap_iommu {
 	const char	*name;
 	struct module	*owner;

@@ -50,7 +57,8 @@ struct omap_iommu {
 	struct list_head	mmap;
 	struct mutex		mmap_lock; /* protect mmap */
 
-	void *ctx; /* iommu context: registres saved area */
+	struct iommu_regs context;
+	int ctx_loss_cnt;
 	u32 da_start;
 	u32 da_end;
 };

arch/arm/plat-omap/include/plat/iommu2.h +0 -2

index 1579694..bc43d41 100644
@@ -35,8 +35,6 @@
 #define MMU_READ_RAM		0x6c
 #define MMU_EMU_FAULT_AD	0x70
 
-#define MMU_REG_SIZE		256
-
 /*
  * MMU Register bit definitions
  */

drivers/iommu/omap-iommu.c +1 -2

index 875e894..e266ad7 100644
@@ -924,14 +924,13 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct iommu_platform_data *pdata = pdev->dev.platform_data;
 
-	obj = kzalloc(sizeof(*obj) + MMU_REG_SIZE, GFP_KERNEL);
+	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
 	if (!obj)
 		return -ENOMEM;
 
 	obj->nr_tlb_entries = pdata->nr_tlb_entries;
 	obj->name = pdata->name;
 	obj->dev = &pdev->dev;
-	obj->ctx = (void *)obj + sizeof(*obj);
 	obj->da_start = pdata->da_start;
 	obj->da_end = pdata->da_end;

Signature · 1 line

1.7.9.5

Omar Ramirez Luna

author

2012-10-12 01:06 UTC #15 lore raw

PATCH v3 4/6 ARM: OMAP: iommu: optimize save and restore routines

In reply to Omar Ramirez Luna

Signed-off-by Omar Ramirez Luna <[email protected]>

---
 arch/arm/mach-omap2/iommu2.c             |   38 ++++++++++++++----------------
 arch/arm/plat-omap/include/plat/iommu.h  |   10 +++++++-
 arch/arm/plat-omap/include/plat/iommu2.h |    2 --
 drivers/iommu/omap-iommu.c               |    3 +--
 4 files changed, 28 insertions(+), 25 deletions(-)

arch/arm/mach-omap2/iommu2.c +18 ~~-20~~

index 3e47786..cd77abb 100644
@@ -19,6 +19,7 @@
 #include <linux/stringify.h>
 
 #include <plat/iommu.h>
+#include <plat/omap-pm.h>
 
 /*
  * omap2 architecture specific register bit definitions

@@ -55,20 +56,26 @@
 
 static void __iommu_set_twl(struct omap_iommu *obj, bool on)
 {
-	u32 l = iommu_read_reg(obj, MMU_CNTL);
+	u32 l;
 
 	if (on)
-		iommu_write_reg(obj, MMU_IRQ_TWL_MASK, MMU_IRQENABLE);
+		l = MMU_IRQ_TWL_MASK;
 	else
-		iommu_write_reg(obj, MMU_IRQ_TLB_MISS_MASK, MMU_IRQENABLE);
+		l = MMU_IRQ_TLB_MISS_MASK;
+
+	iommu_write_reg(obj, l, MMU_IRQENABLE);
+	obj->context.irqen = l;
 
+	l = iommu_read_reg(obj, MMU_CNTL);
 	l &= ~MMU_CNTL_MASK;
+
 	if (on)
 		l |= (MMU_CNTL_MMU_EN | MMU_CNTL_TWL_EN);
 	else
 		l |= (MMU_CNTL_MMU_EN);
 
 	iommu_write_reg(obj, l, MMU_CNTL);
+	obj->context.cntl = l;
 }

@@ -88,6 +95,7 @@ static int omap2_iommu_enable(struct omap_iommu *obj)
 		 (l >> 4) & 0xf, l & 0xf);
 
 	iommu_write_reg(obj, pa, MMU_TTB);
+	obj->context.ttb = pa;
 
 	__iommu_set_twl(obj, true);

@@ -100,6 +108,7 @@ static void omap2_iommu_disable(struct omap_iommu *obj)
 
 	l &= ~MMU_CNTL_MASK;
 	iommu_write_reg(obj, l, MMU_CNTL);
+	obj->context.cntl = l;
 
 	dev_dbg(obj->dev, "%s is shutting down\n", obj->name);
 }

@@ -249,28 +258,17 @@ out:
 
 static void omap2_iommu_save_ctx(struct omap_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);
+	obj->ctx_loss_cnt = omap_pm_get_dev_context_loss_count(obj->dev);
 }
 
 static void omap2_iommu_restore_ctx(struct omap_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]);
-	}
+	if (omap_pm_get_dev_context_loss_count(obj->dev) == obj->ctx_loss_cnt)
+		return;
 
-	BUG_ON(p[0] != IOMMU_ARCH_VERSION);
+	iommu_write_reg(obj, obj->context.ttb, MMU_TTB);
+	iommu_write_reg(obj, obj->context.irqen, MMU_IRQENABLE);
+	iommu_write_reg(obj, obj->context.cntl, MMU_CNTL);
 }
 
 static void omap2_cr_to_e(struct cr_regs *cr, struct iotlb_entry *e)

arch/arm/plat-omap/include/plat/iommu.h +9 -1

index 902d193..af14486 100644
@@ -27,6 +27,13 @@ struct iotlb_entry {
 	};
 };
 
+/* context registers */
+struct iommu_regs {
+	u32 irqen;
+	u32 cntl;
+	u32 ttb;
+};
+
 struct omap_iommu {
 	const char	*name;
 	struct module	*owner;

@@ -50,7 +57,8 @@ struct omap_iommu {
 	struct list_head	mmap;
 	struct mutex		mmap_lock; /* protect mmap */
 
-	void *ctx; /* iommu context: registres saved area */
+	struct iommu_regs context;
+	int ctx_loss_cnt;
 	u32 da_start;
 	u32 da_end;
 };

arch/arm/plat-omap/include/plat/iommu2.h +0 -2

index 1579694..bc43d41 100644
@@ -35,8 +35,6 @@
 #define MMU_READ_RAM		0x6c
 #define MMU_EMU_FAULT_AD	0x70
 
-#define MMU_REG_SIZE		256
-
 /*
  * MMU Register bit definitions
  */

drivers/iommu/omap-iommu.c +1 -2

index 875e894..e266ad7 100644
@@ -924,14 +924,13 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct iommu_platform_data *pdata = pdev->dev.platform_data;
 
-	obj = kzalloc(sizeof(*obj) + MMU_REG_SIZE, GFP_KERNEL);
+	obj = kzalloc(sizeof(*obj), GFP_KERNEL);
 	if (!obj)
 		return -ENOMEM;
 
 	obj->nr_tlb_entries = pdata->nr_tlb_entries;
 	obj->name = pdata->name;
 	obj->dev = &pdev->dev;
-	obj->ctx = (void *)obj + sizeof(*obj);
 	obj->da_start = pdata->da_start;
 	obj->da_end = pdata->da_end;

Signature · 1 line

1.7.9.5

Omar Ramirez Luna

author

2012-10-12 01:06 UTC #16 lore raw

PATCH v3 5/6 ARM: OMAP: iommu: add device tree support

In reply to Omar Ramirez Luna

Adapt driver to use DT if provided.

Signed-off-by Omar Ramirez Luna <[email protected]>

---
 .../devicetree/bindings/arm/omap/iommu.txt         |   10 +++
 arch/arm/mach-omap2/omap-iommu.c                   |    4 ++
 drivers/iommu/omap-iommu.c                         |   65 +++++++++++++++++++-
 3 files changed, 78 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/arm/omap/iommu.txt

Documentation/devicetree/bindings/arm/omap/iommu.txt new file +10 -0

new file mode 100644
index 0000000..2bb780f
@@ -0,0 +1,10 @@
+* MMU (Memory Management Unit)
+
+MMU present in OMAP subsystems.
+
+Required properties:
+ compatible : should be "ti,omap3-iommu" for OMAP3 MMUs
+ compatible : should be "ti,omap4-iommu" for OMAP4 MMUs
+
+Optional properties:
+ None

arch/arm/mach-omap2/omap-iommu.c +4 -0

index 82a422a..4d6145d 100644
@@ -11,6 +11,7 @@
  */
 
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/err.h>
 #include <linux/slab.h>

@@ -29,6 +30,9 @@ static int __init omap_iommu_dev_init(struct omap_hwmod *oh, void *unused)
 	struct omap_mmu_dev_attr *a = (struct omap_mmu_dev_attr *)oh->dev_attr;
 	static int i;
 
+	if (of_have_populated_dt())
+		return 0;
+
 	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
 	if (!pdata)
 		return -ENOMEM;

drivers/iommu/omap-iommu.c +64 -1

101 lines not on this page · raw message

Signature · 1 line

1.7.9.5

Omar Ramirez Luna

author

2012-10-12 01:06 UTC #17 lore raw

PATCH v3 6/6 arm/dts: OMAP3/4: Add iommu nodes

In reply to Omar Ramirez Luna

Add nodes for iommu DT, to interface with hwmods.

Cc Grant Likely <[email protected]>

Cc Rob Herring <[email protected]>

Cc Benoit Cousson <[email protected]>

Signed-off-by Omar Ramirez Luna <[email protected]>

---
 arch/arm/boot/dts/omap3.dtsi |   12 +++++++++++-
 arch/arm/boot/dts/omap4.dtsi |   17 ++++++++++++++++-
 2 files changed, 27 insertions(+), 2 deletions(-)

arch/arm/boot/dts/omap3.dtsi +11 -1

index f38ea87..c76872e 100644
@@ -37,12 +37,17 @@
 		};
 
 		iva {
-			compatible = "ti,iva2.2";
+			compatible = "ti,iva2.2", "simple-bus";
 			ti,hwmods = "iva";
 
 			dsp {
 				compatible = "ti,omap3-c64";
 			};
+
+			mmu_iva: mmu_iva at 5d000000 {
+				compatible = "ti,omap3-iommu";
+				ti,hwmods = "mmu_iva";
+			};
 		};
 	};

@@ -227,6 +232,11 @@
 			ti,hwmods = "mmc3";
 		};
 
+		mmu_isp: mmu_isp at 480bd400 {
+			compatible = "ti,omap3-iommu";
+			ti,hwmods = "mmu_isp";
+		};
+
 		wdt2: wdt at 48314000 {
 			compatible = "ti,omap3-wdt";
 			ti,hwmods = "wd_timer2";

arch/arm/boot/dts/omap4.dtsi +16 -1

index 3883f94..f084418 100644
@@ -71,8 +71,23 @@
 		};
 
 		dsp {
-			compatible = "ti,omap3-c64";
+			compatible = "ti,omap3-c64", "simple-bus";
 			ti,hwmods = "dsp";
+
+			mmu_dsp: mmu_dsp at 4a066000 {
+				compatible = "ti,omap4-iommu";
+				ti,hwmods = "mmu_dsp";
+			};
+		};
+
+		ipu {
+			compatible = "ti,omap4-ipu", "simple-bus";
+			ti,hwmods = "ipu";
+
+			mmu_ipu: mmu_ipu at 55082000 {
+				compatible = "ti,omap4-iommu";
+				ti,hwmods = "mmu_ipu";
+			};
 		};
 
 		iva {

Signature · 1 line

1.7.9.5

Tony Lindgren

maintainer

2012-10-16 17:22 UTC #18 lore raw

v3 cover OMAP: iommu: hwmod, reset handling and runtime PM

In reply to Omar Ramirez Luna

Quoted 7 lines · Omar Ramirez Luna

These patches are needed for remoteproc to work on OMAP4.
Introduced iommu hwmod support for OMAP3 (iva, isp) and OMAP4 (ipu, dsp), along with the corresponding runtime PM and routines to deassert reset lines, enable/disable clocks and configure sysc registers.

Quoted 8 lines

Although IOMMU hwmod patches were already submitted in the past, this series adds few more changes, like:
- New reset handling.
- Save and restore context code rework.
- Device tree bindings for OMAP3 and OMAP4.
For this series I just dropped the patches already included in mainline.

These will need to be rebased on omap-for-v3.8/cleanup-headers-iommu when I have that pushed out as that removes plat/*iommu*.h files.

Regards,

Tony

Quoted 32 lines

Previous work can be found at:
[v2]
http://www.mail-archive.com/linux-omap at vger.kernel.org/msg75701.html
[v1]
http://www.mail-archive.com/linux-omap at vger.kernel.org/msg70447.html
[old iteration without reset, save/restore and device tree]
http://www.mail-archive.com/linux-omap at vger.kernel.org/msg60133.html
Omar Ramirez Luna (6):
ARM: OMAP3/4: iommu: migrate to hwmod framework
ARM: OMAP3/4: iommu: adapt to runtime pm
ARM: OMAP: iommu: pm runtime save and restore context
ARM: OMAP: iommu: optimize save and restore routines
ARM: OMAP: iommu: add device tree support arm/dts: OMAP3/4: Add iommu nodes
 .../devicetree/bindings/arm/omap/iommu.txt         |   10 ++
 arch/arm/boot/dts/omap3.dtsi                       |   12 +-
 arch/arm/boot/dts/omap4.dtsi                       |   17 +-
 arch/arm/mach-omap2/devices.c                      |    2 +-
 arch/arm/mach-omap2/iommu2.c                       |   74 ++------
 arch/arm/mach-omap2/omap-iommu.c                   |  176 +++++---------------
 arch/arm/plat-omap/include/plat/iommu.h            |   20 ++-
 arch/arm/plat-omap/include/plat/iommu2.h           |    4 -
 drivers/iommu/omap-iommu.c                         |  163 ++++++++++++++----
 9 files changed, 245 insertions(+), 233 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/omap/iommu.txt
Signature · 1 line
1.7.9.5

Omar Ramirez Luna

author

2012-10-17 23:52 UTC #19 lore raw

v3 cover OMAP: iommu: hwmod, reset handling and runtime PM

Quoted 14 lines · Tony Lindgren

Quoted 13 lines · Omar Ramirez Luna
These patches are needed for remoteproc to work on OMAP4.
Introduced iommu hwmod support for OMAP3 (iva, isp) and OMAP4 (ipu, dsp), along with the corresponding runtime PM and routines to deassert reset lines, enable/disable clocks and configure sysc registers.
Although IOMMU hwmod patches were already submitted in the past, this series adds few more changes, like:
- New reset handling.
- Save and restore context code rework.
- Device tree bindings for OMAP3 and OMAP4.

Quoted 5 lines

Quoted 2 lines
For this series I just dropped the patches already included in mainline.
These will need to be rebased on omap-for-v3.8/cleanup-headers-iommu when I have that pushed out as that removes plat/*iommu*.h files.

Ok, will wait and rebase on top of it.

Thanks,

Omar

Tony Lindgren

maintainer

2012-10-18 23:52 UTC #20 lore raw

v3 cover OMAP: iommu: hwmod, reset handling and runtime PM

Omar,

Quoted 6 lines · Omar Ramirez Luna

Quoted 2 lines · Tony Lindgren
These will need to be rebased on omap-for-v3.8/cleanup-headers-iommu when I have that pushed out as that removes plat/*iommu*.h files.
Ok, will wait and rebase on top of it.

Thanks, the related patches are now posted in thread "[PATCH v3 0/6] omap iommu changes to remove plat includes".

Also, can you please take a look at the "Updated status of the removal of plat headers" thread?

I've tagged you to remove the omap plat/mailbox.h :)

Regards,

Tony

Omar Ramirez Luna

author

2012-10-19 16:37 UTC #21 lore raw

v3 cover OMAP: iommu: hwmod, reset handling and runtime PM

Tony,

Quoted 2 lines · Tony Lindgren

Thanks, the related patches are now posted in thread "[PATCH v3 0/6] omap iommu changes to remove plat includes".

Ok.

Quoted 4 lines

Also, can you please take a look at the "Updated status of the removal of plat headers" thread?
I've tagged you to remove the omap plat/mailbox.h :)

Yes, got the request from Benoit too, but just now had the time to look at it.

Cheers,

Omar