All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/4] OMAP: iommu: hwmod support and runtime PM
@ 2011-12-15  4:18 ` Omar Ramirez Luna
  0 siblings, 0 replies; 48+ messages in thread
From: Omar Ramirez Luna @ 2011-12-15  4:18 UTC (permalink / raw)
  To: Tony Lindgren, Benoit Cousson
  Cc: Russell King, Kevin Hilman, Ohad Ben-Cohen, Rafael J. Wysocki,
	Joerg Roedel, Laurent Pinchart, MyungJoo Ham, linux-omap,
	linux-arm-kernel, iommu, Linux PM, Omar Ramirez Luna

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

v4:
- Rebased to 3.2-rc5 LO.
- Added missing clkdm field for OMAP4 hwmod data.
- Moved pm runtime enable/disable to probe/remove functions as
  suggested by MyungJoo Ham.
- Removed HWMOD_INIT_NO_RESET as it is not needed and because of
  Kevin Hilman's comments, the failure I was seeing without
  HWMOD_INIT_NO_RESET might have been related to local changes as
  it couldn't be reproduced anymore.
- Removed iommu latency as it is the same as the default omap latency.
- Tested on OMAP3 (zoom3 + tidspbridge) and OMAP4 (Panda + rpmsg).

v3:
- Rebased to 3.1-rc10 lo rebuilt, added structure terminators, and
removed .omap_chip field.

v2:
- Added oh reset info to assert/deassert mmu reset lines.
- Addressed previous comments on v1
http://www.spinics.net/lists/arm-kernel/msg103271.html

Due to compatibility an ifdef needs to be propagated (previously on
iommu resource info) to hwmod data in OMAP3, so users of iommu and
tidspbridge can avoid issues of two modules managing mmu data/irqs/resets;
this until tidspbridge can be safely migrated to iommu framework.

Omar Ramirez Luna (4):
  OMAP3: hwmod data: add mmu data for iva and isp
  OMAP4: hwmod data: add mmu hwmod for ipu and dsp
  OMAP3/4: iommu: migrate to hwmod framework
  OMAP3/4: iommu: adapt to runtime pm

 arch/arm/mach-omap2/iommu2.c               |   36 -------
 arch/arm/mach-omap2/omap-iommu.c           |  156 +++++-----------------------
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |  135 ++++++++++++++++++++++++
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  154 +++++++++++++++++++++++++--
 arch/arm/plat-omap/include/plat/iommu.h    |   17 +++-
 arch/arm/plat-omap/include/plat/iommu2.h   |    2 -
 drivers/iommu/omap-iommu.c                 |   47 +++-----
 7 files changed, 334 insertions(+), 213 deletions(-)

-- 
1.7.4.1


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

* [PATCH v4 0/4] OMAP: iommu: hwmod support and runtime PM
@ 2011-12-15  4:18 ` Omar Ramirez Luna
  0 siblings, 0 replies; 48+ messages in thread
From: Omar Ramirez Luna @ 2011-12-15  4:18 UTC (permalink / raw)
  To: linux-arm-kernel

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

v4:
- Rebased to 3.2-rc5 LO.
- Added missing clkdm field for OMAP4 hwmod data.
- Moved pm runtime enable/disable to probe/remove functions as
  suggested by MyungJoo Ham.
- Removed HWMOD_INIT_NO_RESET as it is not needed and because of
  Kevin Hilman's comments, the failure I was seeing without
  HWMOD_INIT_NO_RESET might have been related to local changes as
  it couldn't be reproduced anymore.
- Removed iommu latency as it is the same as the default omap latency.
- Tested on OMAP3 (zoom3 + tidspbridge) and OMAP4 (Panda + rpmsg).

v3:
- Rebased to 3.1-rc10 lo rebuilt, added structure terminators, and
removed .omap_chip field.

v2:
- Added oh reset info to assert/deassert mmu reset lines.
- Addressed previous comments on v1
http://www.spinics.net/lists/arm-kernel/msg103271.html

Due to compatibility an ifdef needs to be propagated (previously on
iommu resource info) to hwmod data in OMAP3, so users of iommu and
tidspbridge can avoid issues of two modules managing mmu data/irqs/resets;
this until tidspbridge can be safely migrated to iommu framework.

Omar Ramirez Luna (4):
  OMAP3: hwmod data: add mmu data for iva and isp
  OMAP4: hwmod data: add mmu hwmod for ipu and dsp
  OMAP3/4: iommu: migrate to hwmod framework
  OMAP3/4: iommu: adapt to runtime pm

 arch/arm/mach-omap2/iommu2.c               |   36 -------
 arch/arm/mach-omap2/omap-iommu.c           |  156 +++++-----------------------
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |  135 ++++++++++++++++++++++++
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  154 +++++++++++++++++++++++++--
 arch/arm/plat-omap/include/plat/iommu.h    |   17 +++-
 arch/arm/plat-omap/include/plat/iommu2.h   |    2 -
 drivers/iommu/omap-iommu.c                 |   47 +++-----
 7 files changed, 334 insertions(+), 213 deletions(-)

-- 
1.7.4.1

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

* [PATCH v4 1/4] OMAP3: hwmod data: add mmu data for iva and isp
  2011-12-15  4:18 ` Omar Ramirez Luna
@ 2011-12-15  4:18   ` Omar Ramirez Luna
  -1 siblings, 0 replies; 48+ messages in thread
From: Omar Ramirez Luna @ 2011-12-15  4:18 UTC (permalink / raw)
  To: Tony Lindgren, Benoit Cousson
  Cc: Russell King, Kevin Hilman, Ohad Ben-Cohen, Rafael J. Wysocki,
	Joerg Roedel, Laurent Pinchart, MyungJoo Ham, linux-omap,
	linux-arm-kernel, iommu, Linux PM, Omar Ramirez Luna

Add mmu hwmod data for iva and isp.

Due to compatibility an ifdef CONFIG_OMAP_IOMMU_IVA2 needs to be
propagated (previously on iommu resource info) to hwmod data in OMAP3,
so users of iommu and tidspbridge can avoid issues of two modules
managing mmu data/irqs/resets; this until tidspbridge can be migrated
to iommu framework.

Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |  135 ++++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/iommu.h    |   13 +++
 2 files changed, 148 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 7f8915a..6afd925 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -26,6 +26,7 @@
 #include <plat/mcbsp.h>
 #include <plat/mcspi.h>
 #include <plat/dmtimer.h>
+#include <plat/iommu.h>
 
 #include "omap_hwmod_common_data.h"
 
@@ -2947,6 +2948,132 @@ static struct omap_hwmod omap34xx_mcspi4 = {
 };
 
 /*
+ * 'mmu' class
+ * The memory management unit performs virtual to physical address translation
+ * for its requestors.
+ */
+
+static struct omap_hwmod_class_sysconfig mmu_sysc = {
+	.rev_offs	= 0x000,
+	.sysc_offs	= 0x010,
+	.syss_offs	= 0x014,
+	.sysc_flags	= (SYSC_HAS_CLOCKACTIVITY | SYSC_HAS_SIDLEMODE |
+			   SYSC_HAS_SOFTRESET | SYSC_HAS_AUTOIDLE),
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
+	.sysc_fields	= &omap_hwmod_sysc_type1,
+};
+
+static struct omap_hwmod_class omap3xxx_mmu_hwmod_class = {
+	.name = "mmu",
+	.sysc = &mmu_sysc,
+};
+
+/* isp mmu */
+
+static struct omap_mmu_dev_attr isp_mmu_dev_attr = {
+	.da_start = 0x0,
+	.da_end = 0xfffff000,
+	.nr_tlb_entries = 8,
+};
+
+static struct omap_hwmod omap3xxx_isp_mmu_hwmod;
+static struct omap_hwmod_irq_info omap3xxx_isp_mmu_irqs[] = {
+	{ .irq = 24 },
+	{ .irq = -1 }
+};
+
+static struct omap_hwmod_addr_space omap3xxx_isp_mmu_addrs[] = {
+	{
+		.pa_start	= 0x480bd400,
+		.pa_end		= 0x480bd47f,
+		.flags		= ADDR_TYPE_RT,
+	},
+	{ }
+};
+
+/* l4_core -> isp mmu */
+static struct omap_hwmod_ocp_if omap3xxx_l4_core__isp_mmu = {
+	.master		= &omap3xxx_l4_core_hwmod,
+	.slave		= &omap3xxx_isp_mmu_hwmod,
+	.addr		= omap3xxx_isp_mmu_addrs,
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+/* isp mmu slave ports */
+static struct omap_hwmod_ocp_if *omap3xxx_isp_mmu_slaves[] = {
+	&omap3xxx_l4_core__isp_mmu,
+};
+
+static struct omap_hwmod omap3xxx_isp_mmu_hwmod = {
+	.name		= "isp_mmu",
+	.class		= &omap3xxx_mmu_hwmod_class,
+	.mpu_irqs	= omap3xxx_isp_mmu_irqs,
+	.main_clk	= "cam_ick",
+	.dev_attr	= &isp_mmu_dev_attr,
+	.slaves		= omap3xxx_isp_mmu_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap3xxx_isp_mmu_slaves),
+	.flags		= HWMOD_NO_IDLEST,
+};
+
+/* iva mmu */
+
+static struct omap_mmu_dev_attr iva_mmu_dev_attr = {
+	.da_start = 0x11000000,
+	.da_end = 0xfffff000,
+	.nr_tlb_entries = 32,
+};
+
+static struct omap_hwmod omap3xxx_iva_mmu_hwmod;
+static struct omap_hwmod_irq_info omap3xxx_iva_mmu_irqs[] = {
+	{ .irq = 28 },
+	{ .irq = -1 }
+};
+
+static struct omap_hwmod_rst_info omap3xxx_iva_mmu_resets[] = {
+	{ .name = "mmu", .rst_shift = 1, .st_shift = 9 },
+};
+
+static struct omap_hwmod_addr_space omap3xxx_iva_mmu_addrs[] = {
+	{
+		.pa_start	= 0x5d000000,
+		.pa_end		= 0x5d00007f,
+		.flags		= ADDR_TYPE_RT,
+	},
+	{ }
+};
+
+/* l3_main -> iva mmu */
+static struct omap_hwmod_ocp_if omap3xxx_l3_main__iva_mmu = {
+	.master		= &omap3xxx_l3_main_hwmod,
+	.slave		= &omap3xxx_iva_mmu_hwmod,
+	.addr		= omap3xxx_iva_mmu_addrs,
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+/* iva mmu slave ports */
+static struct omap_hwmod_ocp_if *omap3xxx_iva_mmu_slaves[] = {
+	&omap3xxx_l3_main__iva_mmu,
+};
+
+static struct omap_hwmod omap3xxx_iva_mmu_hwmod = {
+	.name		= "iva_mmu",
+	.class		= &omap3xxx_mmu_hwmod_class,
+	.mpu_irqs	= omap3xxx_iva_mmu_irqs,
+	.rst_lines	= omap3xxx_iva_mmu_resets,
+	.rst_lines_cnt	= ARRAY_SIZE(omap3xxx_iva_mmu_resets),
+	.main_clk	= "iva2_ck",
+	.prcm = {
+		.omap2 = {
+			.module_offs = OMAP3430_IVA2_MOD,
+		},
+	},
+	.dev_attr	= &iva_mmu_dev_attr,
+	.slaves		= omap3xxx_iva_mmu_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap3xxx_iva_mmu_slaves),
+	.flags		= HWMOD_NO_IDLEST,
+};
+
+/*
  * usbhsotg
  */
 static struct omap_hwmod_class_sysconfig omap3xxx_usbhsotg_sysc = {
@@ -3264,7 +3391,11 @@ static __initdata struct omap_hwmod *omap3430es2plus_hwmods[] = {
 
 /* 34xx-only hwmods (all ES revisions) */
 static __initdata struct omap_hwmod *omap34xx_hwmods[] = {
+	&omap3xxx_isp_mmu_hwmod,
 	&omap3xxx_iva_hwmod,
+#ifdef CONFIG_OMAP_IOMMU_IVA2
+	&omap3xxx_iva_mmu_hwmod,
+#endif
 	&omap34xx_sr1_hwmod,
 	&omap34xx_sr2_hwmod,
 	&omap3xxx_mailbox_hwmod,
@@ -3273,7 +3404,11 @@ static __initdata struct omap_hwmod *omap34xx_hwmods[] = {
 
 /* 36xx-only hwmods (all ES revisions) */
 static __initdata struct omap_hwmod *omap36xx_hwmods[] = {
+	&omap3xxx_isp_mmu_hwmod,
 	&omap3xxx_iva_hwmod,
+#ifdef CONFIG_OMAP_IOMMU_IVA2
+	&omap3xxx_iva_mmu_hwmod,
+#endif
 	&omap3xxx_uart4_hwmod,
 	&omap3xxx_dss_core_hwmod,
 	&omap36xx_sr1_hwmod,
diff --git a/arch/arm/plat-omap/include/plat/iommu.h b/arch/arm/plat-omap/include/plat/iommu.h
index a1d79ee..cfab835 100644
--- a/arch/arm/plat-omap/include/plat/iommu.h
+++ b/arch/arm/plat-omap/include/plat/iommu.h
@@ -103,6 +103,19 @@ struct iommu_functions {
 	ssize_t (*dump_ctx)(struct omap_iommu *obj, char *buf, ssize_t len);
 };
 
+/**
+ * omap_mmu_dev_attr - OMAP mmu device attributes for omap_hwmod
+ * @da_start:		device address where the va space starts.
+ * @da_end:		device address where the va space ends.
+ * @nr_tlb_entries:	number of entries supported by the translation
+ *			look-aside buffer (TLB).
+ */
+struct omap_mmu_dev_attr {
+	u32 da_start;
+	u32 da_end;
+	int nr_tlb_entries;
+};
+
 struct iommu_platform_data {
 	const char *name;
 	const char *clk_name;
-- 
1.7.4.1


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

* [PATCH v4 1/4] OMAP3: hwmod data: add mmu data for iva and isp
@ 2011-12-15  4:18   ` Omar Ramirez Luna
  0 siblings, 0 replies; 48+ messages in thread
From: Omar Ramirez Luna @ 2011-12-15  4:18 UTC (permalink / raw)
  To: linux-arm-kernel

Add mmu hwmod data for iva and isp.

Due to compatibility an ifdef CONFIG_OMAP_IOMMU_IVA2 needs to be
propagated (previously on iommu resource info) to hwmod data in OMAP3,
so users of iommu and tidspbridge can avoid issues of two modules
managing mmu data/irqs/resets; this until tidspbridge can be migrated
to iommu framework.

Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |  135 ++++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/iommu.h    |   13 +++
 2 files changed, 148 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 7f8915a..6afd925 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -26,6 +26,7 @@
 #include <plat/mcbsp.h>
 #include <plat/mcspi.h>
 #include <plat/dmtimer.h>
+#include <plat/iommu.h>
 
 #include "omap_hwmod_common_data.h"
 
@@ -2947,6 +2948,132 @@ static struct omap_hwmod omap34xx_mcspi4 = {
 };
 
 /*
+ * 'mmu' class
+ * The memory management unit performs virtual to physical address translation
+ * for its requestors.
+ */
+
+static struct omap_hwmod_class_sysconfig mmu_sysc = {
+	.rev_offs	= 0x000,
+	.sysc_offs	= 0x010,
+	.syss_offs	= 0x014,
+	.sysc_flags	= (SYSC_HAS_CLOCKACTIVITY | SYSC_HAS_SIDLEMODE |
+			   SYSC_HAS_SOFTRESET | SYSC_HAS_AUTOIDLE),
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
+	.sysc_fields	= &omap_hwmod_sysc_type1,
+};
+
+static struct omap_hwmod_class omap3xxx_mmu_hwmod_class = {
+	.name = "mmu",
+	.sysc = &mmu_sysc,
+};
+
+/* isp mmu */
+
+static struct omap_mmu_dev_attr isp_mmu_dev_attr = {
+	.da_start = 0x0,
+	.da_end = 0xfffff000,
+	.nr_tlb_entries = 8,
+};
+
+static struct omap_hwmod omap3xxx_isp_mmu_hwmod;
+static struct omap_hwmod_irq_info omap3xxx_isp_mmu_irqs[] = {
+	{ .irq = 24 },
+	{ .irq = -1 }
+};
+
+static struct omap_hwmod_addr_space omap3xxx_isp_mmu_addrs[] = {
+	{
+		.pa_start	= 0x480bd400,
+		.pa_end		= 0x480bd47f,
+		.flags		= ADDR_TYPE_RT,
+	},
+	{ }
+};
+
+/* l4_core -> isp mmu */
+static struct omap_hwmod_ocp_if omap3xxx_l4_core__isp_mmu = {
+	.master		= &omap3xxx_l4_core_hwmod,
+	.slave		= &omap3xxx_isp_mmu_hwmod,
+	.addr		= omap3xxx_isp_mmu_addrs,
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+/* isp mmu slave ports */
+static struct omap_hwmod_ocp_if *omap3xxx_isp_mmu_slaves[] = {
+	&omap3xxx_l4_core__isp_mmu,
+};
+
+static struct omap_hwmod omap3xxx_isp_mmu_hwmod = {
+	.name		= "isp_mmu",
+	.class		= &omap3xxx_mmu_hwmod_class,
+	.mpu_irqs	= omap3xxx_isp_mmu_irqs,
+	.main_clk	= "cam_ick",
+	.dev_attr	= &isp_mmu_dev_attr,
+	.slaves		= omap3xxx_isp_mmu_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap3xxx_isp_mmu_slaves),
+	.flags		= HWMOD_NO_IDLEST,
+};
+
+/* iva mmu */
+
+static struct omap_mmu_dev_attr iva_mmu_dev_attr = {
+	.da_start = 0x11000000,
+	.da_end = 0xfffff000,
+	.nr_tlb_entries = 32,
+};
+
+static struct omap_hwmod omap3xxx_iva_mmu_hwmod;
+static struct omap_hwmod_irq_info omap3xxx_iva_mmu_irqs[] = {
+	{ .irq = 28 },
+	{ .irq = -1 }
+};
+
+static struct omap_hwmod_rst_info omap3xxx_iva_mmu_resets[] = {
+	{ .name = "mmu", .rst_shift = 1, .st_shift = 9 },
+};
+
+static struct omap_hwmod_addr_space omap3xxx_iva_mmu_addrs[] = {
+	{
+		.pa_start	= 0x5d000000,
+		.pa_end		= 0x5d00007f,
+		.flags		= ADDR_TYPE_RT,
+	},
+	{ }
+};
+
+/* l3_main -> iva mmu */
+static struct omap_hwmod_ocp_if omap3xxx_l3_main__iva_mmu = {
+	.master		= &omap3xxx_l3_main_hwmod,
+	.slave		= &omap3xxx_iva_mmu_hwmod,
+	.addr		= omap3xxx_iva_mmu_addrs,
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+/* iva mmu slave ports */
+static struct omap_hwmod_ocp_if *omap3xxx_iva_mmu_slaves[] = {
+	&omap3xxx_l3_main__iva_mmu,
+};
+
+static struct omap_hwmod omap3xxx_iva_mmu_hwmod = {
+	.name		= "iva_mmu",
+	.class		= &omap3xxx_mmu_hwmod_class,
+	.mpu_irqs	= omap3xxx_iva_mmu_irqs,
+	.rst_lines	= omap3xxx_iva_mmu_resets,
+	.rst_lines_cnt	= ARRAY_SIZE(omap3xxx_iva_mmu_resets),
+	.main_clk	= "iva2_ck",
+	.prcm = {
+		.omap2 = {
+			.module_offs = OMAP3430_IVA2_MOD,
+		},
+	},
+	.dev_attr	= &iva_mmu_dev_attr,
+	.slaves		= omap3xxx_iva_mmu_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap3xxx_iva_mmu_slaves),
+	.flags		= HWMOD_NO_IDLEST,
+};
+
+/*
  * usbhsotg
  */
 static struct omap_hwmod_class_sysconfig omap3xxx_usbhsotg_sysc = {
@@ -3264,7 +3391,11 @@ static __initdata struct omap_hwmod *omap3430es2plus_hwmods[] = {
 
 /* 34xx-only hwmods (all ES revisions) */
 static __initdata struct omap_hwmod *omap34xx_hwmods[] = {
+	&omap3xxx_isp_mmu_hwmod,
 	&omap3xxx_iva_hwmod,
+#ifdef CONFIG_OMAP_IOMMU_IVA2
+	&omap3xxx_iva_mmu_hwmod,
+#endif
 	&omap34xx_sr1_hwmod,
 	&omap34xx_sr2_hwmod,
 	&omap3xxx_mailbox_hwmod,
@@ -3273,7 +3404,11 @@ static __initdata struct omap_hwmod *omap34xx_hwmods[] = {
 
 /* 36xx-only hwmods (all ES revisions) */
 static __initdata struct omap_hwmod *omap36xx_hwmods[] = {
+	&omap3xxx_isp_mmu_hwmod,
 	&omap3xxx_iva_hwmod,
+#ifdef CONFIG_OMAP_IOMMU_IVA2
+	&omap3xxx_iva_mmu_hwmod,
+#endif
 	&omap3xxx_uart4_hwmod,
 	&omap3xxx_dss_core_hwmod,
 	&omap36xx_sr1_hwmod,
diff --git a/arch/arm/plat-omap/include/plat/iommu.h b/arch/arm/plat-omap/include/plat/iommu.h
index a1d79ee..cfab835 100644
--- a/arch/arm/plat-omap/include/plat/iommu.h
+++ b/arch/arm/plat-omap/include/plat/iommu.h
@@ -103,6 +103,19 @@ struct iommu_functions {
 	ssize_t (*dump_ctx)(struct omap_iommu *obj, char *buf, ssize_t len);
 };
 
+/**
+ * omap_mmu_dev_attr - OMAP mmu device attributes for omap_hwmod
+ * @da_start:		device address where the va space starts.
+ * @da_end:		device address where the va space ends.
+ * @nr_tlb_entries:	number of entries supported by the translation
+ *			look-aside buffer (TLB).
+ */
+struct omap_mmu_dev_attr {
+	u32 da_start;
+	u32 da_end;
+	int nr_tlb_entries;
+};
+
 struct iommu_platform_data {
 	const char *name;
 	const char *clk_name;
-- 
1.7.4.1

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

* [PATCH v4 2/4] OMAP4: hwmod data: add mmu hwmod for ipu and dsp
  2011-12-15  4:18 ` Omar Ramirez Luna
@ 2011-12-15  4:18   ` Omar Ramirez Luna
  -1 siblings, 0 replies; 48+ messages in thread
From: Omar Ramirez Luna @ 2011-12-15  4:18 UTC (permalink / raw)
  To: Tony Lindgren, Benoit Cousson
  Cc: Russell King, Kevin Hilman, Ohad Ben-Cohen, Rafael J. Wysocki,
	Joerg Roedel, Laurent Pinchart, MyungJoo Ham, linux-omap,
	linux-arm-kernel, iommu, Linux PM, Omar Ramirez Luna

Add mmu hwmod data for ipu and dsp.

Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  154 +++++++++++++++++++++++++--
 1 files changed, 142 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index daaf165..3a29407 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -31,6 +31,7 @@
 #include <plat/i2c.h>
 #include <plat/dmtimer.h>
 #include <plat/common.h>
+#include <plat/iommu.h>
 
 #include "omap_hwmod_common_data.h"
 
@@ -1102,10 +1103,6 @@ static struct omap_hwmod_irq_info omap44xx_dsp_irqs[] = {
 	{ .irq = -1 }
 };
 
-static struct omap_hwmod_rst_info omap44xx_dsp_resets[] = {
-	{ .name = "mmu_cache", .rst_shift = 1 },
-};
-
 static struct omap_hwmod_rst_info omap44xx_dsp_c0_resets[] = {
 	{ .name = "dsp", .rst_shift = 0 },
 };
@@ -1157,8 +1154,6 @@ static struct omap_hwmod omap44xx_dsp_hwmod = {
 	.class		= &omap44xx_dsp_hwmod_class,
 	.clkdm_name	= "tesla_clkdm",
 	.mpu_irqs	= omap44xx_dsp_irqs,
-	.rst_lines	= omap44xx_dsp_resets,
-	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_dsp_resets),
 	.main_clk	= "dsp_fck",
 	.prcm = {
 		.omap4 = {
@@ -2507,10 +2502,6 @@ static struct omap_hwmod_rst_info omap44xx_ipu_c1_resets[] = {
 	{ .name = "cpu1", .rst_shift = 1 },
 };
 
-static struct omap_hwmod_rst_info omap44xx_ipu_resets[] = {
-	{ .name = "mmu_cache", .rst_shift = 2 },
-};
-
 /* ipu master ports */
 static struct omap_hwmod_ocp_if *omap44xx_ipu_masters[] = {
 	&omap44xx_ipu__l3_main_2,
@@ -2564,8 +2555,6 @@ static struct omap_hwmod omap44xx_ipu_hwmod = {
 	.class		= &omap44xx_ipu_hwmod_class,
 	.clkdm_name	= "ducati_clkdm",
 	.mpu_irqs	= omap44xx_ipu_irqs,
-	.rst_lines	= omap44xx_ipu_resets,
-	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_ipu_resets),
 	.main_clk	= "ipu_fck",
 	.prcm = {
 		.omap4 = {
@@ -3932,6 +3921,143 @@ static struct omap_hwmod omap44xx_mpu_hwmod = {
 };
 
 /*
+ * 'mmu' class
+ * The memory management unit performs virtual to physical address translation
+ * for its requestors.
+ */
+
+static struct omap_hwmod_class_sysconfig mmu_sysc = {
+	.rev_offs	= 0x000,
+	.sysc_offs	= 0x010,
+	.syss_offs	= 0x014,
+	.sysc_flags	= (SYSC_HAS_CLOCKACTIVITY | SYSC_HAS_SIDLEMODE |
+			   SYSC_HAS_SOFTRESET | SYSC_HAS_AUTOIDLE),
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
+	.sysc_fields	= &omap_hwmod_sysc_type1,
+};
+
+static struct omap_hwmod_class omap44xx_mmu_hwmod_class = {
+	.name = "mmu",
+	.sysc = &mmu_sysc,
+};
+
+/* ipu mmu */
+
+static struct omap_mmu_dev_attr ipu_mmu_dev_attr = {
+	.da_start = 0x0,
+	.da_end = 0xfffff000,
+	.nr_tlb_entries = 32,
+};
+
+static struct omap_hwmod omap44xx_ipu_mmu_hwmod;
+static struct omap_hwmod_irq_info omap44xx_ipu_mmu_irqs[] = {
+	{ .irq = 100 + OMAP44XX_IRQ_GIC_START, },
+	{ .irq = -1 }
+};
+
+static struct omap_hwmod_rst_info omap44xx_ipu_mmu_resets[] = {
+	{ .name = "mmu_cache", .rst_shift = 2 },
+};
+
+static struct omap_hwmod_addr_space omap44xx_ipu_mmu_addrs[] = {
+	{
+		.pa_start	= 0x55082000,
+		.pa_end		= 0x550820ff,
+		.flags		= ADDR_TYPE_RT,
+	},
+	{ }
+};
+
+/* l3_main_1 -> ipu mmu */
+static struct omap_hwmod_ocp_if omap44xx_l3_main_1__ipu_mmu = {
+	.master		= &omap44xx_l3_main_1_hwmod,
+	.slave		= &omap44xx_ipu_mmu_hwmod,
+	.addr		= omap44xx_ipu_mmu_addrs,
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+/* ipu mmu slave ports */
+static struct omap_hwmod_ocp_if *omap44xx_ipu_mmu_slaves[] = {
+	&omap44xx_l3_main_1__ipu_mmu,
+};
+
+static struct omap_hwmod omap44xx_ipu_mmu_hwmod = {
+	.name		= "ipu_mmu",
+	.class		= &omap44xx_mmu_hwmod_class,
+	.clkdm_name	= "ducati_clkdm",
+	.mpu_irqs	= omap44xx_ipu_mmu_irqs,
+	.rst_lines	= omap44xx_ipu_mmu_resets,
+	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_ipu_mmu_resets),
+	.main_clk	= "ipu_fck",
+	.prcm = {
+		.omap4 = {
+			.rstctrl_offs = OMAP4_RM_DUCATI_RSTCTRL_OFFSET,
+		},
+	},
+	.dev_attr	= &ipu_mmu_dev_attr,
+	.slaves		= omap44xx_ipu_mmu_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap44xx_ipu_mmu_slaves),
+};
+
+/* dsp mmu */
+
+static struct omap_mmu_dev_attr dsp_mmu_dev_attr = {
+	.da_start = 0x0,
+	.da_end = 0xfffff000,
+	.nr_tlb_entries = 32,
+};
+
+static struct omap_hwmod omap44xx_dsp_mmu_hwmod;
+static struct omap_hwmod_irq_info omap44xx_dsp_mmu_irqs[] = {
+	{ .irq = 28 + OMAP44XX_IRQ_GIC_START },
+	{ .irq = -1 }
+};
+
+static struct omap_hwmod_rst_info omap44xx_dsp_mmu_resets[] = {
+	{ .name = "mmu_cache", .rst_shift = 1 },
+};
+
+static struct omap_hwmod_addr_space omap44xx_dsp_mmu_addrs[] = {
+	{
+		.pa_start	= 0x4a066000,
+		.pa_end		= 0x4a0660ff,
+		.flags		= ADDR_TYPE_RT,
+	},
+	{ }
+};
+
+/* l3_main_1 -> dsp mmu */
+static struct omap_hwmod_ocp_if omap44xx_l3_main_1__dsp_mmu = {
+	.master		= &omap44xx_l3_main_1_hwmod,
+	.slave		= &omap44xx_dsp_mmu_hwmod,
+	.addr		= omap44xx_dsp_mmu_addrs,
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+/* dsp mmu slave ports */
+static struct omap_hwmod_ocp_if *omap44xx_dsp_mmu_slaves[] = {
+	&omap44xx_l3_main_1__dsp_mmu,
+};
+
+static struct omap_hwmod omap44xx_dsp_mmu_hwmod = {
+	.name		= "dsp_mmu",
+	.class		= &omap44xx_mmu_hwmod_class,
+	.clkdm_name	= "tesla_clkdm",
+	.mpu_irqs	= omap44xx_dsp_mmu_irqs,
+	.rst_lines	= omap44xx_dsp_mmu_resets,
+	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_dsp_mmu_resets),
+	.main_clk	= "dsp_fck",
+	.prcm = {
+		.omap4 = {
+			.rstctrl_offs = OMAP4_RM_TESLA_RSTCTRL_OFFSET,
+		},
+	},
+	.dev_attr	= &dsp_mmu_dev_attr,
+	.slaves		= omap44xx_dsp_mmu_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap44xx_dsp_mmu_slaves),
+};
+
+/*
  * 'smartreflex' class
  * smartreflex module (monitor silicon performance and outputs a measure of
  * performance error)
@@ -5388,6 +5514,10 @@ static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
 	/* mpu class */
 	&omap44xx_mpu_hwmod,
 
+	/* mmu class */
+	&omap44xx_ipu_mmu_hwmod,
+	&omap44xx_dsp_mmu_hwmod,
+
 	/* smartreflex class */
 	&omap44xx_smartreflex_core_hwmod,
 	&omap44xx_smartreflex_iva_hwmod,
-- 
1.7.4.1


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

* [PATCH v4 2/4] OMAP4: hwmod data: add mmu hwmod for ipu and dsp
@ 2011-12-15  4:18   ` Omar Ramirez Luna
  0 siblings, 0 replies; 48+ messages in thread
From: Omar Ramirez Luna @ 2011-12-15  4:18 UTC (permalink / raw)
  To: linux-arm-kernel

Add mmu hwmod data for ipu and dsp.

Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  154 +++++++++++++++++++++++++--
 1 files changed, 142 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index daaf165..3a29407 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -31,6 +31,7 @@
 #include <plat/i2c.h>
 #include <plat/dmtimer.h>
 #include <plat/common.h>
+#include <plat/iommu.h>
 
 #include "omap_hwmod_common_data.h"
 
@@ -1102,10 +1103,6 @@ static struct omap_hwmod_irq_info omap44xx_dsp_irqs[] = {
 	{ .irq = -1 }
 };
 
-static struct omap_hwmod_rst_info omap44xx_dsp_resets[] = {
-	{ .name = "mmu_cache", .rst_shift = 1 },
-};
-
 static struct omap_hwmod_rst_info omap44xx_dsp_c0_resets[] = {
 	{ .name = "dsp", .rst_shift = 0 },
 };
@@ -1157,8 +1154,6 @@ static struct omap_hwmod omap44xx_dsp_hwmod = {
 	.class		= &omap44xx_dsp_hwmod_class,
 	.clkdm_name	= "tesla_clkdm",
 	.mpu_irqs	= omap44xx_dsp_irqs,
-	.rst_lines	= omap44xx_dsp_resets,
-	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_dsp_resets),
 	.main_clk	= "dsp_fck",
 	.prcm = {
 		.omap4 = {
@@ -2507,10 +2502,6 @@ static struct omap_hwmod_rst_info omap44xx_ipu_c1_resets[] = {
 	{ .name = "cpu1", .rst_shift = 1 },
 };
 
-static struct omap_hwmod_rst_info omap44xx_ipu_resets[] = {
-	{ .name = "mmu_cache", .rst_shift = 2 },
-};
-
 /* ipu master ports */
 static struct omap_hwmod_ocp_if *omap44xx_ipu_masters[] = {
 	&omap44xx_ipu__l3_main_2,
@@ -2564,8 +2555,6 @@ static struct omap_hwmod omap44xx_ipu_hwmod = {
 	.class		= &omap44xx_ipu_hwmod_class,
 	.clkdm_name	= "ducati_clkdm",
 	.mpu_irqs	= omap44xx_ipu_irqs,
-	.rst_lines	= omap44xx_ipu_resets,
-	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_ipu_resets),
 	.main_clk	= "ipu_fck",
 	.prcm = {
 		.omap4 = {
@@ -3932,6 +3921,143 @@ static struct omap_hwmod omap44xx_mpu_hwmod = {
 };
 
 /*
+ * 'mmu' class
+ * The memory management unit performs virtual to physical address translation
+ * for its requestors.
+ */
+
+static struct omap_hwmod_class_sysconfig mmu_sysc = {
+	.rev_offs	= 0x000,
+	.sysc_offs	= 0x010,
+	.syss_offs	= 0x014,
+	.sysc_flags	= (SYSC_HAS_CLOCKACTIVITY | SYSC_HAS_SIDLEMODE |
+			   SYSC_HAS_SOFTRESET | SYSC_HAS_AUTOIDLE),
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
+	.sysc_fields	= &omap_hwmod_sysc_type1,
+};
+
+static struct omap_hwmod_class omap44xx_mmu_hwmod_class = {
+	.name = "mmu",
+	.sysc = &mmu_sysc,
+};
+
+/* ipu mmu */
+
+static struct omap_mmu_dev_attr ipu_mmu_dev_attr = {
+	.da_start = 0x0,
+	.da_end = 0xfffff000,
+	.nr_tlb_entries = 32,
+};
+
+static struct omap_hwmod omap44xx_ipu_mmu_hwmod;
+static struct omap_hwmod_irq_info omap44xx_ipu_mmu_irqs[] = {
+	{ .irq = 100 + OMAP44XX_IRQ_GIC_START, },
+	{ .irq = -1 }
+};
+
+static struct omap_hwmod_rst_info omap44xx_ipu_mmu_resets[] = {
+	{ .name = "mmu_cache", .rst_shift = 2 },
+};
+
+static struct omap_hwmod_addr_space omap44xx_ipu_mmu_addrs[] = {
+	{
+		.pa_start	= 0x55082000,
+		.pa_end		= 0x550820ff,
+		.flags		= ADDR_TYPE_RT,
+	},
+	{ }
+};
+
+/* l3_main_1 -> ipu mmu */
+static struct omap_hwmod_ocp_if omap44xx_l3_main_1__ipu_mmu = {
+	.master		= &omap44xx_l3_main_1_hwmod,
+	.slave		= &omap44xx_ipu_mmu_hwmod,
+	.addr		= omap44xx_ipu_mmu_addrs,
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+/* ipu mmu slave ports */
+static struct omap_hwmod_ocp_if *omap44xx_ipu_mmu_slaves[] = {
+	&omap44xx_l3_main_1__ipu_mmu,
+};
+
+static struct omap_hwmod omap44xx_ipu_mmu_hwmod = {
+	.name		= "ipu_mmu",
+	.class		= &omap44xx_mmu_hwmod_class,
+	.clkdm_name	= "ducati_clkdm",
+	.mpu_irqs	= omap44xx_ipu_mmu_irqs,
+	.rst_lines	= omap44xx_ipu_mmu_resets,
+	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_ipu_mmu_resets),
+	.main_clk	= "ipu_fck",
+	.prcm = {
+		.omap4 = {
+			.rstctrl_offs = OMAP4_RM_DUCATI_RSTCTRL_OFFSET,
+		},
+	},
+	.dev_attr	= &ipu_mmu_dev_attr,
+	.slaves		= omap44xx_ipu_mmu_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap44xx_ipu_mmu_slaves),
+};
+
+/* dsp mmu */
+
+static struct omap_mmu_dev_attr dsp_mmu_dev_attr = {
+	.da_start = 0x0,
+	.da_end = 0xfffff000,
+	.nr_tlb_entries = 32,
+};
+
+static struct omap_hwmod omap44xx_dsp_mmu_hwmod;
+static struct omap_hwmod_irq_info omap44xx_dsp_mmu_irqs[] = {
+	{ .irq = 28 + OMAP44XX_IRQ_GIC_START },
+	{ .irq = -1 }
+};
+
+static struct omap_hwmod_rst_info omap44xx_dsp_mmu_resets[] = {
+	{ .name = "mmu_cache", .rst_shift = 1 },
+};
+
+static struct omap_hwmod_addr_space omap44xx_dsp_mmu_addrs[] = {
+	{
+		.pa_start	= 0x4a066000,
+		.pa_end		= 0x4a0660ff,
+		.flags		= ADDR_TYPE_RT,
+	},
+	{ }
+};
+
+/* l3_main_1 -> dsp mmu */
+static struct omap_hwmod_ocp_if omap44xx_l3_main_1__dsp_mmu = {
+	.master		= &omap44xx_l3_main_1_hwmod,
+	.slave		= &omap44xx_dsp_mmu_hwmod,
+	.addr		= omap44xx_dsp_mmu_addrs,
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+/* dsp mmu slave ports */
+static struct omap_hwmod_ocp_if *omap44xx_dsp_mmu_slaves[] = {
+	&omap44xx_l3_main_1__dsp_mmu,
+};
+
+static struct omap_hwmod omap44xx_dsp_mmu_hwmod = {
+	.name		= "dsp_mmu",
+	.class		= &omap44xx_mmu_hwmod_class,
+	.clkdm_name	= "tesla_clkdm",
+	.mpu_irqs	= omap44xx_dsp_mmu_irqs,
+	.rst_lines	= omap44xx_dsp_mmu_resets,
+	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_dsp_mmu_resets),
+	.main_clk	= "dsp_fck",
+	.prcm = {
+		.omap4 = {
+			.rstctrl_offs = OMAP4_RM_TESLA_RSTCTRL_OFFSET,
+		},
+	},
+	.dev_attr	= &dsp_mmu_dev_attr,
+	.slaves		= omap44xx_dsp_mmu_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap44xx_dsp_mmu_slaves),
+};
+
+/*
  * 'smartreflex' class
  * smartreflex module (monitor silicon performance and outputs a measure of
  * performance error)
@@ -5388,6 +5514,10 @@ static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
 	/* mpu class */
 	&omap44xx_mpu_hwmod,
 
+	/* mmu class */
+	&omap44xx_ipu_mmu_hwmod,
+	&omap44xx_dsp_mmu_hwmod,
+
 	/* smartreflex class */
 	&omap44xx_smartreflex_core_hwmod,
 	&omap44xx_smartreflex_iva_hwmod,
-- 
1.7.4.1

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

* [PATCH v4 3/4] OMAP3/4: iommu: migrate to hwmod framework
  2011-12-15  4:18 ` Omar Ramirez Luna
@ 2011-12-15  4:18   ` Omar Ramirez Luna
  -1 siblings, 0 replies; 48+ messages in thread
From: Omar Ramirez Luna @ 2011-12-15  4:18 UTC (permalink / raw)
  To: Tony Lindgren, Benoit Cousson
  Cc: Russell King, Kevin Hilman, Ohad Ben-Cohen, Rafael J. Wysocki,
	Joerg Roedel, Laurent Pinchart, MyungJoo Ham, linux-omap,
	linux-arm-kernel, iommu, Linux PM, Omar Ramirez Luna

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.

Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 arch/arm/mach-omap2/iommu2.c            |   19 ----
 arch/arm/mach-omap2/omap-iommu.c        |  157 +++++--------------------------
 arch/arm/plat-omap/include/plat/iommu.h |    2 +-
 drivers/iommu/omap-iommu.c              |    3 -
 4 files changed, 27 insertions(+), 154 deletions(-)

diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index eefc379..35cab47 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -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);
diff --git a/arch/arm/mach-omap2/omap-iommu.c b/arch/arm/mach-omap2/omap-iommu.c
index b882204..6da05e1 100644
--- a/arch/arm/mach-omap2/omap-iommu.c
+++ b/arch/arm/mach-omap2/omap-iommu.c
@@ -12,152 +12,47 @@
 
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/err.h>
 
 #include <plat/iommu.h>
 #include <plat/irqs.h>
+#include <plat/omap_hwmod.h>
+#include <plat/omap_device.h>
 
-struct iommu_device {
-	resource_size_t base;
-	int irq;
+static int __init omap_iommu_dev_init(struct omap_hwmod *oh, void *unused)
+{
+	struct omap_device *od;
 	struct iommu_platform_data pdata;
-	struct resource res[2];
-};
-static struct iommu_device *devices;
-static int num_iommu_devices;
-
-#ifdef CONFIG_ARCH_OMAP3
-static struct iommu_device omap3_devices[] = {
-	{
-		.base = 0x480bd400,
-		.irq = 24,
-		.pdata = {
-			.name = "isp",
-			.nr_tlb_entries = 8,
-			.clk_name = "cam_ick",
-			.da_start = 0x0,
-			.da_end = 0xFFFFF000,
-		},
-	},
-#if defined(CONFIG_OMAP_IOMMU_IVA2)
-	{
-		.base = 0x5d000000,
-		.irq = 28,
-		.pdata = {
-			.name = "iva2",
-			.nr_tlb_entries = 32,
-			.clk_name = "iva2_ck",
-			.da_start = 0x11000000,
-			.da_end = 0xFFFFF000,
-		},
-	},
-#endif
-};
-#define NR_OMAP3_IOMMU_DEVICES ARRAY_SIZE(omap3_devices)
-static struct platform_device *omap3_iommu_pdev[NR_OMAP3_IOMMU_DEVICES];
-#else
-#define omap3_devices		NULL
-#define NR_OMAP3_IOMMU_DEVICES	0
-#define omap3_iommu_pdev	NULL
-#endif
+	struct omap_mmu_dev_attr *a = (struct omap_mmu_dev_attr *)oh->dev_attr;
+	static int i;
+
+	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;
+
+	od = omap_device_build("omap-iommu", i, oh, &pdata, sizeof(pdata),
+				NULL, 0, 0);
+	if (IS_ERR(od)) {
+		pr_err("%s: device build error: %ld\n", __func__, PTR_ERR(od));
+		return PTR_ERR(od);
+	}
 
-#ifdef CONFIG_ARCH_OMAP4
-static struct iommu_device omap4_devices[] = {
-	{
-		.base = OMAP4_MMU1_BASE,
-		.irq = OMAP44XX_IRQ_DUCATI_MMU,
-		.pdata = {
-			.name = "ducati",
-			.nr_tlb_entries = 32,
-			.clk_name = "ipu_fck",
-			.da_start = 0x0,
-			.da_end = 0xFFFFF000,
-		},
-	},
-#if defined(CONFIG_MPU_TESLA_IOMMU)
-	{
-		.base = OMAP4_MMU2_BASE,
-		.irq = INT_44XX_DSP_MMU,
-		.pdata = {
-			.name = "tesla",
-			.nr_tlb_entries = 32,
-			.clk_name = "tesla_ick",
-			.da_start = 0x0,
-			.da_end = 0xFFFFF000,
-		},
-	},
-#endif
-};
-#define NR_OMAP4_IOMMU_DEVICES ARRAY_SIZE(omap4_devices)
-static struct platform_device *omap4_iommu_pdev[NR_OMAP4_IOMMU_DEVICES];
-#else
-#define omap4_devices		NULL
-#define NR_OMAP4_IOMMU_DEVICES	0
-#define omap4_iommu_pdev	NULL
-#endif
+	i++;
 
-static struct platform_device **omap_iommu_pdev;
+	return 0;
+}
 
 static int __init omap_iommu_init(void)
 {
-	int i, err;
-	struct resource res[] = {
-		{ .flags = IORESOURCE_MEM },
-		{ .flags = IORESOURCE_IRQ },
-	};
-
-	if (cpu_is_omap34xx()) {
-		devices = omap3_devices;
-		omap_iommu_pdev = omap3_iommu_pdev;
-		num_iommu_devices = NR_OMAP3_IOMMU_DEVICES;
-	} else if (cpu_is_omap44xx()) {
-		devices = omap4_devices;
-		omap_iommu_pdev = omap4_iommu_pdev;
-		num_iommu_devices = NR_OMAP4_IOMMU_DEVICES;
-	} else
-		return -ENODEV;
-
-	for (i = 0; i < num_iommu_devices; i++) {
-		struct platform_device *pdev;
-		const struct iommu_device *d = &devices[i];
-
-		pdev = platform_device_alloc("omap-iommu", i);
-		if (!pdev) {
-			err = -ENOMEM;
-			goto err_out;
-		}
-
-		res[0].start = d->base;
-		res[0].end = d->base + MMU_REG_SIZE - 1;
-		res[1].start = res[1].end = d->irq;
-
-		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;
-		omap_iommu_pdev[i] = pdev;
-	}
-	return 0;
-
-err_out:
-	while (i--)
-		platform_device_put(omap_iommu_pdev[i]);
-	return err;
+	return omap_hwmod_for_each_by_class("mmu", omap_iommu_dev_init, NULL);
 }
 module_init(omap_iommu_init);
 
 static void __exit omap_iommu_exit(void)
 {
-	int i;
-
-	for (i = 0; i < num_iommu_devices; i++)
-		platform_device_unregister(omap_iommu_pdev[i]);
+	/* Do nothing */
 }
 module_exit(omap_iommu_exit);
 
diff --git a/arch/arm/plat-omap/include/plat/iommu.h b/arch/arm/plat-omap/include/plat/iommu.h
index cfab835..c5b23b8 100644
--- a/arch/arm/plat-omap/include/plat/iommu.h
+++ b/arch/arm/plat-omap/include/plat/iommu.h
@@ -119,7 +119,7 @@ struct omap_mmu_dev_attr {
 struct iommu_platform_data {
 	const char *name;
 	const char *clk_name;
-	const int nr_tlb_entries;
+	int nr_tlb_entries;
 	u32 da_start;
 	u32 da_end;
 };
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 8f32b2b..78e62b1 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -916,9 +916,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;
-- 
1.7.4.1


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

* [PATCH v4 3/4] OMAP3/4: iommu: migrate to hwmod framework
@ 2011-12-15  4:18   ` Omar Ramirez Luna
  0 siblings, 0 replies; 48+ messages in thread
From: Omar Ramirez Luna @ 2011-12-15  4:18 UTC (permalink / raw)
  To: linux-arm-kernel

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.

Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 arch/arm/mach-omap2/iommu2.c            |   19 ----
 arch/arm/mach-omap2/omap-iommu.c        |  157 +++++--------------------------
 arch/arm/plat-omap/include/plat/iommu.h |    2 +-
 drivers/iommu/omap-iommu.c              |    3 -
 4 files changed, 27 insertions(+), 154 deletions(-)

diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index eefc379..35cab47 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -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);
diff --git a/arch/arm/mach-omap2/omap-iommu.c b/arch/arm/mach-omap2/omap-iommu.c
index b882204..6da05e1 100644
--- a/arch/arm/mach-omap2/omap-iommu.c
+++ b/arch/arm/mach-omap2/omap-iommu.c
@@ -12,152 +12,47 @@
 
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/err.h>
 
 #include <plat/iommu.h>
 #include <plat/irqs.h>
+#include <plat/omap_hwmod.h>
+#include <plat/omap_device.h>
 
-struct iommu_device {
-	resource_size_t base;
-	int irq;
+static int __init omap_iommu_dev_init(struct omap_hwmod *oh, void *unused)
+{
+	struct omap_device *od;
 	struct iommu_platform_data pdata;
-	struct resource res[2];
-};
-static struct iommu_device *devices;
-static int num_iommu_devices;
-
-#ifdef CONFIG_ARCH_OMAP3
-static struct iommu_device omap3_devices[] = {
-	{
-		.base = 0x480bd400,
-		.irq = 24,
-		.pdata = {
-			.name = "isp",
-			.nr_tlb_entries = 8,
-			.clk_name = "cam_ick",
-			.da_start = 0x0,
-			.da_end = 0xFFFFF000,
-		},
-	},
-#if defined(CONFIG_OMAP_IOMMU_IVA2)
-	{
-		.base = 0x5d000000,
-		.irq = 28,
-		.pdata = {
-			.name = "iva2",
-			.nr_tlb_entries = 32,
-			.clk_name = "iva2_ck",
-			.da_start = 0x11000000,
-			.da_end = 0xFFFFF000,
-		},
-	},
-#endif
-};
-#define NR_OMAP3_IOMMU_DEVICES ARRAY_SIZE(omap3_devices)
-static struct platform_device *omap3_iommu_pdev[NR_OMAP3_IOMMU_DEVICES];
-#else
-#define omap3_devices		NULL
-#define NR_OMAP3_IOMMU_DEVICES	0
-#define omap3_iommu_pdev	NULL
-#endif
+	struct omap_mmu_dev_attr *a = (struct omap_mmu_dev_attr *)oh->dev_attr;
+	static int i;
+
+	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;
+
+	od = omap_device_build("omap-iommu", i, oh, &pdata, sizeof(pdata),
+				NULL, 0, 0);
+	if (IS_ERR(od)) {
+		pr_err("%s: device build error: %ld\n", __func__, PTR_ERR(od));
+		return PTR_ERR(od);
+	}
 
-#ifdef CONFIG_ARCH_OMAP4
-static struct iommu_device omap4_devices[] = {
-	{
-		.base = OMAP4_MMU1_BASE,
-		.irq = OMAP44XX_IRQ_DUCATI_MMU,
-		.pdata = {
-			.name = "ducati",
-			.nr_tlb_entries = 32,
-			.clk_name = "ipu_fck",
-			.da_start = 0x0,
-			.da_end = 0xFFFFF000,
-		},
-	},
-#if defined(CONFIG_MPU_TESLA_IOMMU)
-	{
-		.base = OMAP4_MMU2_BASE,
-		.irq = INT_44XX_DSP_MMU,
-		.pdata = {
-			.name = "tesla",
-			.nr_tlb_entries = 32,
-			.clk_name = "tesla_ick",
-			.da_start = 0x0,
-			.da_end = 0xFFFFF000,
-		},
-	},
-#endif
-};
-#define NR_OMAP4_IOMMU_DEVICES ARRAY_SIZE(omap4_devices)
-static struct platform_device *omap4_iommu_pdev[NR_OMAP4_IOMMU_DEVICES];
-#else
-#define omap4_devices		NULL
-#define NR_OMAP4_IOMMU_DEVICES	0
-#define omap4_iommu_pdev	NULL
-#endif
+	i++;
 
-static struct platform_device **omap_iommu_pdev;
+	return 0;
+}
 
 static int __init omap_iommu_init(void)
 {
-	int i, err;
-	struct resource res[] = {
-		{ .flags = IORESOURCE_MEM },
-		{ .flags = IORESOURCE_IRQ },
-	};
-
-	if (cpu_is_omap34xx()) {
-		devices = omap3_devices;
-		omap_iommu_pdev = omap3_iommu_pdev;
-		num_iommu_devices = NR_OMAP3_IOMMU_DEVICES;
-	} else if (cpu_is_omap44xx()) {
-		devices = omap4_devices;
-		omap_iommu_pdev = omap4_iommu_pdev;
-		num_iommu_devices = NR_OMAP4_IOMMU_DEVICES;
-	} else
-		return -ENODEV;
-
-	for (i = 0; i < num_iommu_devices; i++) {
-		struct platform_device *pdev;
-		const struct iommu_device *d = &devices[i];
-
-		pdev = platform_device_alloc("omap-iommu", i);
-		if (!pdev) {
-			err = -ENOMEM;
-			goto err_out;
-		}
-
-		res[0].start = d->base;
-		res[0].end = d->base + MMU_REG_SIZE - 1;
-		res[1].start = res[1].end = d->irq;
-
-		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;
-		omap_iommu_pdev[i] = pdev;
-	}
-	return 0;
-
-err_out:
-	while (i--)
-		platform_device_put(omap_iommu_pdev[i]);
-	return err;
+	return omap_hwmod_for_each_by_class("mmu", omap_iommu_dev_init, NULL);
 }
 module_init(omap_iommu_init);
 
 static void __exit omap_iommu_exit(void)
 {
-	int i;
-
-	for (i = 0; i < num_iommu_devices; i++)
-		platform_device_unregister(omap_iommu_pdev[i]);
+	/* Do nothing */
 }
 module_exit(omap_iommu_exit);
 
diff --git a/arch/arm/plat-omap/include/plat/iommu.h b/arch/arm/plat-omap/include/plat/iommu.h
index cfab835..c5b23b8 100644
--- a/arch/arm/plat-omap/include/plat/iommu.h
+++ b/arch/arm/plat-omap/include/plat/iommu.h
@@ -119,7 +119,7 @@ struct omap_mmu_dev_attr {
 struct iommu_platform_data {
 	const char *name;
 	const char *clk_name;
-	const int nr_tlb_entries;
+	int nr_tlb_entries;
 	u32 da_start;
 	u32 da_end;
 };
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 8f32b2b..78e62b1 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -916,9 +916,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;
-- 
1.7.4.1

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

* [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
  2011-12-15  4:18 ` Omar Ramirez Luna
@ 2011-12-15  4:18   ` Omar Ramirez Luna
  -1 siblings, 0 replies; 48+ messages in thread
From: Omar Ramirez Luna @ 2011-12-15  4:18 UTC (permalink / raw)
  To: Tony Lindgren, Benoit Cousson
  Cc: Russell King, Kevin Hilman, Ohad Ben-Cohen, Rafael J. Wysocki,
	Joerg Roedel, Laurent Pinchart, MyungJoo Ham, linux-omap,
	linux-arm-kernel, iommu, Linux PM, Omar Ramirez Luna

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

Removed clk handling during interrupt, given that in order to receive one,
the device should be powered on in advance. Now doing pm_runtime_get/put
on iommu_enable/disable so it doesn't rely on others to keep the clocks on.

Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 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               |   44 ++++++++++++-----------------
 5 files changed, 18 insertions(+), 48 deletions(-)

diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index 35cab47..3e47786 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -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);
diff --git a/arch/arm/mach-omap2/omap-iommu.c b/arch/arm/mach-omap2/omap-iommu.c
index 6da05e1..99ed583 100644
--- a/arch/arm/mach-omap2/omap-iommu.c
+++ b/arch/arm/mach-omap2/omap-iommu.c
@@ -27,7 +27,6 @@ static int __init omap_iommu_dev_init(struct omap_hwmod *oh, void *unused)
 	static int i;
 
 	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;
diff --git a/arch/arm/plat-omap/include/plat/iommu.h b/arch/arm/plat-omap/include/plat/iommu.h
index c5b23b8..46d2b9a 100644
--- a/arch/arm/plat-omap/include/plat/iommu.h
+++ b/arch/arm/plat-omap/include/plat/iommu.h
@@ -28,7 +28,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;
@@ -118,7 +117,6 @@ struct omap_mmu_dev_attr {
 
 struct iommu_platform_data {
 	const char *name;
-	const char *clk_name;
 	int nr_tlb_entries;
 	u32 da_start;
 	u32 da_end;
diff --git a/arch/arm/plat-omap/include/plat/iommu2.h b/arch/arm/plat-omap/include/plat/iommu2.h
index d4116b5..1579694 100644
--- a/arch/arm/plat-omap/include/plat/iommu2.h
+++ b/arch/arm/plat-omap/include/plat/iommu2.h
@@ -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
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 78e62b1..e8ae36c 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -16,11 +16,11 @@
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/ioport.h>
-#include <linux/clk.h>
 #include <linux/platform_device.h>
 #include <linux/iommu.h>
 #include <linux/mutex.h>
 #include <linux/spinlock.h>
+#include <linux/pm_runtime.h>
 
 #include <asm/cacheflush.h>
 
@@ -123,11 +123,10 @@ static int iommu_enable(struct omap_iommu *obj)
 	if (!arch_iommu)
 		return -ENODEV;
 
-	clk_enable(obj->clk);
+	pm_runtime_get_sync(obj->dev);
 
 	err = arch_iommu->enable(obj);
 
-	clk_disable(obj->clk);
 	return err;
 }
 
@@ -136,11 +135,9 @@ static void iommu_disable(struct omap_iommu *obj)
 	if (!obj)
 		return;
 
-	clk_enable(obj->clk);
-
 	arch_iommu->disable(obj);
 
-	clk_disable(obj->clk);
+	pm_runtime_put(obj->dev);
 }
 
 /*
@@ -263,7 +260,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) {
@@ -293,7 +290,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(obj->dev);
 		return PTR_ERR(cr);
 	}
 
@@ -307,7 +304,7 @@ static int load_iotlb_entry(struct omap_iommu *obj, struct iotlb_entry *e)
 		l.vict = l.base;
 	iotlb_lock_set(obj, &l);
 out:
-	clk_disable(obj->clk);
+	pm_runtime_put(obj->dev);
 	return err;
 }
 
@@ -337,7 +334,7 @@ static void flush_iotlb_page(struct omap_iommu *obj, u32 da)
 	int i;
 	struct cr_regs cr;
 
-	clk_enable(obj->clk);
+	pm_runtime_get_sync(obj->dev);
 
 	for_each_iotlb_cr(obj, obj->nr_tlb_entries, i, cr) {
 		u32 start;
@@ -356,7 +353,7 @@ static void flush_iotlb_page(struct omap_iommu *obj, u32 da)
 			iommu_write_reg(obj, 1, MMU_FLUSH_ENTRY);
 		}
 	}
-	clk_disable(obj->clk);
+	pm_runtime_put(obj->dev);
 
 	if (i == obj->nr_tlb_entries)
 		dev_dbg(obj->dev, "%s: no page for %08x\n", __func__, da);
@@ -370,7 +367,7 @@ static void flush_iotlb_all(struct omap_iommu *obj)
 {
 	struct iotlb_lock l;
 
-	clk_enable(obj->clk);
+	pm_runtime_get_sync(obj->dev);
 
 	l.base = 0;
 	l.vict = 0;
@@ -378,7 +375,7 @@ static void flush_iotlb_all(struct omap_iommu *obj)
 
 	iommu_write_reg(obj, 1, MMU_GFLUSH);
 
-	clk_disable(obj->clk);
+	pm_runtime_put(obj->dev);
 }
 
 #if defined(CONFIG_OMAP_IOMMU_DEBUG) || defined(CONFIG_OMAP_IOMMU_DEBUG_MODULE)
@@ -388,11 +385,11 @@ ssize_t omap_iommu_dump_ctx(struct omap_iommu *obj, char *buf, ssize_t bytes)
 	if (!obj || !buf)
 		return -EINVAL;
 
-	clk_enable(obj->clk);
+	pm_runtime_get_sync(obj->dev);
 
 	bytes = arch_iommu->dump_ctx(obj, buf, bytes);
 
-	clk_disable(obj->clk);
+	pm_runtime_put(obj->dev);
 
 	return bytes;
 }
@@ -406,7 +403,7 @@ __dump_tlb_entries(struct omap_iommu *obj, struct cr_regs *crs, int num)
 	struct cr_regs tmp;
 	struct cr_regs *p = crs;
 
-	clk_enable(obj->clk);
+	pm_runtime_get_sync(obj->dev);
 	iotlb_lock_get(obj, &saved);
 
 	for_each_iotlb_cr(obj, num, i, tmp) {
@@ -416,7 +413,7 @@ __dump_tlb_entries(struct omap_iommu *obj, struct cr_regs *crs, int num)
 	}
 
 	iotlb_lock_set(obj, &saved);
-	clk_disable(obj->clk);
+	pm_runtime_put(obj->dev);
 
 	return  p - crs;
 }
@@ -780,9 +777,7 @@ static irqreturn_t iommu_fault_handler(int irq, void *data)
 	if (!obj->refcount)
 		return IRQ_NONE;
 
-	clk_enable(obj->clk);
 	errs = iommu_report_fault(obj, &da);
-	clk_disable(obj->clk);
 	if (errs == 0)
 		return IRQ_HANDLED;
 
@@ -920,10 +915,6 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev)
 	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;
@@ -966,6 +957,8 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev)
 		goto err_irq;
 	platform_set_drvdata(pdev, obj);
 
+	pm_runtime_enable(obj->dev);
+
 	dev_info(&pdev->dev, "%s registered\n", obj->name);
 	return 0;
 
@@ -974,8 +967,6 @@ err_irq:
 err_ioremap:
 	release_mem_region(res->start, resource_size(res));
 err_mem:
-	clk_put(obj->clk);
-err_clk:
 	kfree(obj);
 	return err;
 }
@@ -996,7 +987,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);
+
 	dev_info(&pdev->dev, "%s removed\n", obj->name);
 	kfree(obj);
 	return 0;
-- 
1.7.4.1


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

* [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
@ 2011-12-15  4:18   ` Omar Ramirez Luna
  0 siblings, 0 replies; 48+ messages in thread
From: Omar Ramirez Luna @ 2011-12-15  4:18 UTC (permalink / raw)
  To: linux-arm-kernel

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

Removed clk handling during interrupt, given that in order to receive one,
the device should be powered on in advance. Now doing pm_runtime_get/put
on iommu_enable/disable so it doesn't rely on others to keep the clocks on.

Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 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               |   44 ++++++++++++-----------------
 5 files changed, 18 insertions(+), 48 deletions(-)

diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index 35cab47..3e47786 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -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);
diff --git a/arch/arm/mach-omap2/omap-iommu.c b/arch/arm/mach-omap2/omap-iommu.c
index 6da05e1..99ed583 100644
--- a/arch/arm/mach-omap2/omap-iommu.c
+++ b/arch/arm/mach-omap2/omap-iommu.c
@@ -27,7 +27,6 @@ static int __init omap_iommu_dev_init(struct omap_hwmod *oh, void *unused)
 	static int i;
 
 	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;
diff --git a/arch/arm/plat-omap/include/plat/iommu.h b/arch/arm/plat-omap/include/plat/iommu.h
index c5b23b8..46d2b9a 100644
--- a/arch/arm/plat-omap/include/plat/iommu.h
+++ b/arch/arm/plat-omap/include/plat/iommu.h
@@ -28,7 +28,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;
@@ -118,7 +117,6 @@ struct omap_mmu_dev_attr {
 
 struct iommu_platform_data {
 	const char *name;
-	const char *clk_name;
 	int nr_tlb_entries;
 	u32 da_start;
 	u32 da_end;
diff --git a/arch/arm/plat-omap/include/plat/iommu2.h b/arch/arm/plat-omap/include/plat/iommu2.h
index d4116b5..1579694 100644
--- a/arch/arm/plat-omap/include/plat/iommu2.h
+++ b/arch/arm/plat-omap/include/plat/iommu2.h
@@ -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
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 78e62b1..e8ae36c 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -16,11 +16,11 @@
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/ioport.h>
-#include <linux/clk.h>
 #include <linux/platform_device.h>
 #include <linux/iommu.h>
 #include <linux/mutex.h>
 #include <linux/spinlock.h>
+#include <linux/pm_runtime.h>
 
 #include <asm/cacheflush.h>
 
@@ -123,11 +123,10 @@ static int iommu_enable(struct omap_iommu *obj)
 	if (!arch_iommu)
 		return -ENODEV;
 
-	clk_enable(obj->clk);
+	pm_runtime_get_sync(obj->dev);
 
 	err = arch_iommu->enable(obj);
 
-	clk_disable(obj->clk);
 	return err;
 }
 
@@ -136,11 +135,9 @@ static void iommu_disable(struct omap_iommu *obj)
 	if (!obj)
 		return;
 
-	clk_enable(obj->clk);
-
 	arch_iommu->disable(obj);
 
-	clk_disable(obj->clk);
+	pm_runtime_put(obj->dev);
 }
 
 /*
@@ -263,7 +260,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) {
@@ -293,7 +290,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(obj->dev);
 		return PTR_ERR(cr);
 	}
 
@@ -307,7 +304,7 @@ static int load_iotlb_entry(struct omap_iommu *obj, struct iotlb_entry *e)
 		l.vict = l.base;
 	iotlb_lock_set(obj, &l);
 out:
-	clk_disable(obj->clk);
+	pm_runtime_put(obj->dev);
 	return err;
 }
 
@@ -337,7 +334,7 @@ static void flush_iotlb_page(struct omap_iommu *obj, u32 da)
 	int i;
 	struct cr_regs cr;
 
-	clk_enable(obj->clk);
+	pm_runtime_get_sync(obj->dev);
 
 	for_each_iotlb_cr(obj, obj->nr_tlb_entries, i, cr) {
 		u32 start;
@@ -356,7 +353,7 @@ static void flush_iotlb_page(struct omap_iommu *obj, u32 da)
 			iommu_write_reg(obj, 1, MMU_FLUSH_ENTRY);
 		}
 	}
-	clk_disable(obj->clk);
+	pm_runtime_put(obj->dev);
 
 	if (i == obj->nr_tlb_entries)
 		dev_dbg(obj->dev, "%s: no page for %08x\n", __func__, da);
@@ -370,7 +367,7 @@ static void flush_iotlb_all(struct omap_iommu *obj)
 {
 	struct iotlb_lock l;
 
-	clk_enable(obj->clk);
+	pm_runtime_get_sync(obj->dev);
 
 	l.base = 0;
 	l.vict = 0;
@@ -378,7 +375,7 @@ static void flush_iotlb_all(struct omap_iommu *obj)
 
 	iommu_write_reg(obj, 1, MMU_GFLUSH);
 
-	clk_disable(obj->clk);
+	pm_runtime_put(obj->dev);
 }
 
 #if defined(CONFIG_OMAP_IOMMU_DEBUG) || defined(CONFIG_OMAP_IOMMU_DEBUG_MODULE)
@@ -388,11 +385,11 @@ ssize_t omap_iommu_dump_ctx(struct omap_iommu *obj, char *buf, ssize_t bytes)
 	if (!obj || !buf)
 		return -EINVAL;
 
-	clk_enable(obj->clk);
+	pm_runtime_get_sync(obj->dev);
 
 	bytes = arch_iommu->dump_ctx(obj, buf, bytes);
 
-	clk_disable(obj->clk);
+	pm_runtime_put(obj->dev);
 
 	return bytes;
 }
@@ -406,7 +403,7 @@ __dump_tlb_entries(struct omap_iommu *obj, struct cr_regs *crs, int num)
 	struct cr_regs tmp;
 	struct cr_regs *p = crs;
 
-	clk_enable(obj->clk);
+	pm_runtime_get_sync(obj->dev);
 	iotlb_lock_get(obj, &saved);
 
 	for_each_iotlb_cr(obj, num, i, tmp) {
@@ -416,7 +413,7 @@ __dump_tlb_entries(struct omap_iommu *obj, struct cr_regs *crs, int num)
 	}
 
 	iotlb_lock_set(obj, &saved);
-	clk_disable(obj->clk);
+	pm_runtime_put(obj->dev);
 
 	return  p - crs;
 }
@@ -780,9 +777,7 @@ static irqreturn_t iommu_fault_handler(int irq, void *data)
 	if (!obj->refcount)
 		return IRQ_NONE;
 
-	clk_enable(obj->clk);
 	errs = iommu_report_fault(obj, &da);
-	clk_disable(obj->clk);
 	if (errs == 0)
 		return IRQ_HANDLED;
 
@@ -920,10 +915,6 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev)
 	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;
@@ -966,6 +957,8 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev)
 		goto err_irq;
 	platform_set_drvdata(pdev, obj);
 
+	pm_runtime_enable(obj->dev);
+
 	dev_info(&pdev->dev, "%s registered\n", obj->name);
 	return 0;
 
@@ -974,8 +967,6 @@ err_irq:
 err_ioremap:
 	release_mem_region(res->start, resource_size(res));
 err_mem:
-	clk_put(obj->clk);
-err_clk:
 	kfree(obj);
 	return err;
 }
@@ -996,7 +987,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);
+
 	dev_info(&pdev->dev, "%s removed\n", obj->name);
 	kfree(obj);
 	return 0;
-- 
1.7.4.1

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

* Re: [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
  2011-12-15  4:18   ` Omar Ramirez Luna
@ 2011-12-16  0:33     ` Felipe Contreras
  -1 siblings, 0 replies; 48+ messages in thread
From: Felipe Contreras @ 2011-12-16  0:33 UTC (permalink / raw)
  To: Omar Ramirez Luna
  Cc: Tony Lindgren, Benoit Cousson, Kevin Hilman, Ohad Ben-Cohen,
	Russell King, Linux PM, Rafael J. Wysocki, iommu, MyungJoo Ham,
	Laurent Pinchart, linux-omap, linux-arm-kernel

On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
> @@ -123,11 +123,10 @@ static int iommu_enable(struct omap_iommu *obj)
>        if (!arch_iommu)
>                return -ENODEV;
>
> -       clk_enable(obj->clk);
> +       pm_runtime_get_sync(obj->dev);
>
>        err = arch_iommu->enable(obj);
>
> -       clk_disable(obj->clk);
>        return err;
>  }

Hmm, this is called on omap_iommu_attach, and iommu_disable is not
called until omap_iommu_detach, so this means the device will never
sleep. Why don't you call pm_runtime_put() instead of clk_disable()?

All the rest of the calls to pm_runtime_get/put after this are
basically no-ops, because the count will never go below 1.

You mention some irq issues, but could it be due to some bad clocks in
the hwmod data?

> @@ -136,11 +135,9 @@ static void iommu_disable(struct omap_iommu *obj)
>        if (!obj)
>                return;
>
> -       clk_enable(obj->clk);
> -
>        arch_iommu->disable(obj);
>
> -       clk_disable(obj->clk);
> +       pm_runtime_put(obj->dev);
>  }
>
>  /*
> @@ -263,7 +260,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) {
> @@ -293,7 +290,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(obj->dev);
>                return PTR_ERR(cr);
>        }
>
> @@ -307,7 +304,7 @@ static int load_iotlb_entry(struct omap_iommu *obj, struct iotlb_entry *e)
>                l.vict = l.base;
>        iotlb_lock_set(obj, &l);
>  out:
> -       clk_disable(obj->clk);
> +       pm_runtime_put(obj->dev);
>        return err;
>  }
>
> @@ -337,7 +334,7 @@ static void flush_iotlb_page(struct omap_iommu *obj, u32 da)
>        int i;
>        struct cr_regs cr;
>
> -       clk_enable(obj->clk);
> +       pm_runtime_get_sync(obj->dev);
>
>        for_each_iotlb_cr(obj, obj->nr_tlb_entries, i, cr) {
>                u32 start;
> @@ -356,7 +353,7 @@ static void flush_iotlb_page(struct omap_iommu *obj, u32 da)
>                        iommu_write_reg(obj, 1, MMU_FLUSH_ENTRY);
>                }
>        }
> -       clk_disable(obj->clk);
> +       pm_runtime_put(obj->dev);
>
>        if (i == obj->nr_tlb_entries)
>                dev_dbg(obj->dev, "%s: no page for %08x\n", __func__, da);
> @@ -370,7 +367,7 @@ static void flush_iotlb_all(struct omap_iommu *obj)
>  {
>        struct iotlb_lock l;
>
> -       clk_enable(obj->clk);
> +       pm_runtime_get_sync(obj->dev);
>
>        l.base = 0;
>        l.vict = 0;
> @@ -378,7 +375,7 @@ static void flush_iotlb_all(struct omap_iommu *obj)
>
>        iommu_write_reg(obj, 1, MMU_GFLUSH);
>
> -       clk_disable(obj->clk);
> +       pm_runtime_put(obj->dev);
>  }
>
>  #if defined(CONFIG_OMAP_IOMMU_DEBUG) || defined(CONFIG_OMAP_IOMMU_DEBUG_MODULE)
> @@ -388,11 +385,11 @@ ssize_t omap_iommu_dump_ctx(struct omap_iommu *obj, char *buf, ssize_t bytes)
>        if (!obj || !buf)
>                return -EINVAL;
>
> -       clk_enable(obj->clk);
> +       pm_runtime_get_sync(obj->dev);
>
>        bytes = arch_iommu->dump_ctx(obj, buf, bytes);
>
> -       clk_disable(obj->clk);
> +       pm_runtime_put(obj->dev);
>
>        return bytes;
>  }
> @@ -406,7 +403,7 @@ __dump_tlb_entries(struct omap_iommu *obj, struct cr_regs *crs, int num)
>        struct cr_regs tmp;
>        struct cr_regs *p = crs;
>
> -       clk_enable(obj->clk);
> +       pm_runtime_get_sync(obj->dev);
>        iotlb_lock_get(obj, &saved);
>
>        for_each_iotlb_cr(obj, num, i, tmp) {
> @@ -416,7 +413,7 @@ __dump_tlb_entries(struct omap_iommu *obj, struct cr_regs *crs, int num)
>        }
>
>        iotlb_lock_set(obj, &saved);
> -       clk_disable(obj->clk);
> +       pm_runtime_put(obj->dev);
>
>        return  p - crs;
>  }
> @@ -780,9 +777,7 @@ static irqreturn_t iommu_fault_handler(int irq, void *data)
>        if (!obj->refcount)
>                return IRQ_NONE;
>
> -       clk_enable(obj->clk);
>        errs = iommu_report_fault(obj, &da);
> -       clk_disable(obj->clk);

Why?

>        if (errs == 0)
>                return IRQ_HANDLED;
>
> @@ -920,10 +915,6 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev)
>        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;
> @@ -966,6 +957,8 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev)
>                goto err_irq;
>        platform_set_drvdata(pdev, obj);
>
> +       pm_runtime_enable(obj->dev);
> +
>        dev_info(&pdev->dev, "%s registered\n", obj->name);
>        return 0;
>
> @@ -974,8 +967,6 @@ err_irq:
>  err_ioremap:
>        release_mem_region(res->start, resource_size(res));
>  err_mem:
> -       clk_put(obj->clk);
> -err_clk:
>        kfree(obj);
>        return err;
>  }
> @@ -996,7 +987,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);

I'm not sure if this is needed; you want to resume the driver? AFAICS
kfree will take care of that _without_ resuming.

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

Cheers.

-- 
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] 48+ messages in thread

* [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
@ 2011-12-16  0:33     ` Felipe Contreras
  0 siblings, 0 replies; 48+ messages in thread
From: Felipe Contreras @ 2011-12-16  0:33 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
> @@ -123,11 +123,10 @@ static int iommu_enable(struct omap_iommu *obj)
> ? ? ? ?if (!arch_iommu)
> ? ? ? ? ? ? ? ?return -ENODEV;
>
> - ? ? ? clk_enable(obj->clk);
> + ? ? ? pm_runtime_get_sync(obj->dev);
>
> ? ? ? ?err = arch_iommu->enable(obj);
>
> - ? ? ? clk_disable(obj->clk);
> ? ? ? ?return err;
> ?}

Hmm, this is called on omap_iommu_attach, and iommu_disable is not
called until omap_iommu_detach, so this means the device will never
sleep. Why don't you call pm_runtime_put() instead of clk_disable()?

All the rest of the calls to pm_runtime_get/put after this are
basically no-ops, because the count will never go below 1.

You mention some irq issues, but could it be due to some bad clocks in
the hwmod data?

> @@ -136,11 +135,9 @@ static void iommu_disable(struct omap_iommu *obj)
> ? ? ? ?if (!obj)
> ? ? ? ? ? ? ? ?return;
>
> - ? ? ? clk_enable(obj->clk);
> -
> ? ? ? ?arch_iommu->disable(obj);
>
> - ? ? ? clk_disable(obj->clk);
> + ? ? ? pm_runtime_put(obj->dev);
> ?}
>
> ?/*
> @@ -263,7 +260,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) {
> @@ -293,7 +290,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(obj->dev);
> ? ? ? ? ? ? ? ?return PTR_ERR(cr);
> ? ? ? ?}
>
> @@ -307,7 +304,7 @@ static int load_iotlb_entry(struct omap_iommu *obj, struct iotlb_entry *e)
> ? ? ? ? ? ? ? ?l.vict = l.base;
> ? ? ? ?iotlb_lock_set(obj, &l);
> ?out:
> - ? ? ? clk_disable(obj->clk);
> + ? ? ? pm_runtime_put(obj->dev);
> ? ? ? ?return err;
> ?}
>
> @@ -337,7 +334,7 @@ static void flush_iotlb_page(struct omap_iommu *obj, u32 da)
> ? ? ? ?int i;
> ? ? ? ?struct cr_regs cr;
>
> - ? ? ? clk_enable(obj->clk);
> + ? ? ? pm_runtime_get_sync(obj->dev);
>
> ? ? ? ?for_each_iotlb_cr(obj, obj->nr_tlb_entries, i, cr) {
> ? ? ? ? ? ? ? ?u32 start;
> @@ -356,7 +353,7 @@ static void flush_iotlb_page(struct omap_iommu *obj, u32 da)
> ? ? ? ? ? ? ? ? ? ? ? ?iommu_write_reg(obj, 1, MMU_FLUSH_ENTRY);
> ? ? ? ? ? ? ? ?}
> ? ? ? ?}
> - ? ? ? clk_disable(obj->clk);
> + ? ? ? pm_runtime_put(obj->dev);
>
> ? ? ? ?if (i == obj->nr_tlb_entries)
> ? ? ? ? ? ? ? ?dev_dbg(obj->dev, "%s: no page for %08x\n", __func__, da);
> @@ -370,7 +367,7 @@ static void flush_iotlb_all(struct omap_iommu *obj)
> ?{
> ? ? ? ?struct iotlb_lock l;
>
> - ? ? ? clk_enable(obj->clk);
> + ? ? ? pm_runtime_get_sync(obj->dev);
>
> ? ? ? ?l.base = 0;
> ? ? ? ?l.vict = 0;
> @@ -378,7 +375,7 @@ static void flush_iotlb_all(struct omap_iommu *obj)
>
> ? ? ? ?iommu_write_reg(obj, 1, MMU_GFLUSH);
>
> - ? ? ? clk_disable(obj->clk);
> + ? ? ? pm_runtime_put(obj->dev);
> ?}
>
> ?#if defined(CONFIG_OMAP_IOMMU_DEBUG) || defined(CONFIG_OMAP_IOMMU_DEBUG_MODULE)
> @@ -388,11 +385,11 @@ ssize_t omap_iommu_dump_ctx(struct omap_iommu *obj, char *buf, ssize_t bytes)
> ? ? ? ?if (!obj || !buf)
> ? ? ? ? ? ? ? ?return -EINVAL;
>
> - ? ? ? clk_enable(obj->clk);
> + ? ? ? pm_runtime_get_sync(obj->dev);
>
> ? ? ? ?bytes = arch_iommu->dump_ctx(obj, buf, bytes);
>
> - ? ? ? clk_disable(obj->clk);
> + ? ? ? pm_runtime_put(obj->dev);
>
> ? ? ? ?return bytes;
> ?}
> @@ -406,7 +403,7 @@ __dump_tlb_entries(struct omap_iommu *obj, struct cr_regs *crs, int num)
> ? ? ? ?struct cr_regs tmp;
> ? ? ? ?struct cr_regs *p = crs;
>
> - ? ? ? clk_enable(obj->clk);
> + ? ? ? pm_runtime_get_sync(obj->dev);
> ? ? ? ?iotlb_lock_get(obj, &saved);
>
> ? ? ? ?for_each_iotlb_cr(obj, num, i, tmp) {
> @@ -416,7 +413,7 @@ __dump_tlb_entries(struct omap_iommu *obj, struct cr_regs *crs, int num)
> ? ? ? ?}
>
> ? ? ? ?iotlb_lock_set(obj, &saved);
> - ? ? ? clk_disable(obj->clk);
> + ? ? ? pm_runtime_put(obj->dev);
>
> ? ? ? ?return ?p - crs;
> ?}
> @@ -780,9 +777,7 @@ static irqreturn_t iommu_fault_handler(int irq, void *data)
> ? ? ? ?if (!obj->refcount)
> ? ? ? ? ? ? ? ?return IRQ_NONE;
>
> - ? ? ? clk_enable(obj->clk);
> ? ? ? ?errs = iommu_report_fault(obj, &da);
> - ? ? ? clk_disable(obj->clk);

Why?

> ? ? ? ?if (errs == 0)
> ? ? ? ? ? ? ? ?return IRQ_HANDLED;
>
> @@ -920,10 +915,6 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev)
> ? ? ? ?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;
> @@ -966,6 +957,8 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev)
> ? ? ? ? ? ? ? ?goto err_irq;
> ? ? ? ?platform_set_drvdata(pdev, obj);
>
> + ? ? ? pm_runtime_enable(obj->dev);
> +
> ? ? ? ?dev_info(&pdev->dev, "%s registered\n", obj->name);
> ? ? ? ?return 0;
>
> @@ -974,8 +967,6 @@ err_irq:
> ?err_ioremap:
> ? ? ? ?release_mem_region(res->start, resource_size(res));
> ?err_mem:
> - ? ? ? clk_put(obj->clk);
> -err_clk:
> ? ? ? ?kfree(obj);
> ? ? ? ?return err;
> ?}
> @@ -996,7 +987,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);

I'm not sure if this is needed; you want to resume the driver? AFAICS
kfree will take care of that _without_ resuming.

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

Cheers.

-- 
Felipe Contreras

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

* Re: [PATCH v4 1/4] OMAP3: hwmod data: add mmu data for iva and isp
  2011-12-15  4:18   ` Omar Ramirez Luna
@ 2011-12-16  0:39     ` Felipe Contreras
  -1 siblings, 0 replies; 48+ messages in thread
From: Felipe Contreras @ 2011-12-16  0:39 UTC (permalink / raw)
  To: Omar Ramirez Luna
  Cc: Tony Lindgren, Benoit Cousson, Kevin Hilman, Ohad Ben-Cohen,
	Russell King, Linux PM, Rafael J. Wysocki, iommu, MyungJoo Ham,
	Laurent Pinchart, linux-omap, linux-arm-kernel

On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
> +/* l4_core -> isp mmu */
> +static struct omap_hwmod_ocp_if omap3xxx_l4_core__isp_mmu = {
> +       .master         = &omap3xxx_l4_core_hwmod,
> +       .slave          = &omap3xxx_isp_mmu_hwmod,
> +       .addr           = omap3xxx_isp_mmu_addrs,
> +       .user           = OCP_USER_MPU | OCP_USER_SDMA,
> +};

Are you sure you are not missing something like:

  .clk = "cam_ick",

> +/* isp mmu slave ports */
> +static struct omap_hwmod_ocp_if *omap3xxx_isp_mmu_slaves[] = {
> +       &omap3xxx_l4_core__isp_mmu,
> +};
> +
> +static struct omap_hwmod omap3xxx_isp_mmu_hwmod = {
> +       .name           = "isp_mmu",
> +       .class          = &omap3xxx_mmu_hwmod_class,
> +       .mpu_irqs       = omap3xxx_isp_mmu_irqs,
> +       .main_clk       = "cam_ick",

It's not "cam_fck"?

> +       .dev_attr       = &isp_mmu_dev_attr,
> +       .slaves         = omap3xxx_isp_mmu_slaves,
> +       .slaves_cnt     = ARRAY_SIZE(omap3xxx_isp_mmu_slaves),
> +       .flags          = HWMOD_NO_IDLEST,
> +};

Most of the stuff I see the hwmods is .main_lock = "foo_fck", slave
.clk = "foo_ick". Maybe that explains the irq issues you get.

Cheers.

-- 
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] 48+ messages in thread

* [PATCH v4 1/4] OMAP3: hwmod data: add mmu data for iva and isp
@ 2011-12-16  0:39     ` Felipe Contreras
  0 siblings, 0 replies; 48+ messages in thread
From: Felipe Contreras @ 2011-12-16  0:39 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
> +/* l4_core -> isp mmu */
> +static struct omap_hwmod_ocp_if omap3xxx_l4_core__isp_mmu = {
> + ? ? ? .master ? ? ? ? = &omap3xxx_l4_core_hwmod,
> + ? ? ? .slave ? ? ? ? ?= &omap3xxx_isp_mmu_hwmod,
> + ? ? ? .addr ? ? ? ? ? = omap3xxx_isp_mmu_addrs,
> + ? ? ? .user ? ? ? ? ? = OCP_USER_MPU | OCP_USER_SDMA,
> +};

Are you sure you are not missing something like:

  .clk = "cam_ick",

> +/* isp mmu slave ports */
> +static struct omap_hwmod_ocp_if *omap3xxx_isp_mmu_slaves[] = {
> + ? ? ? &omap3xxx_l4_core__isp_mmu,
> +};
> +
> +static struct omap_hwmod omap3xxx_isp_mmu_hwmod = {
> + ? ? ? .name ? ? ? ? ? = "isp_mmu",
> + ? ? ? .class ? ? ? ? ?= &omap3xxx_mmu_hwmod_class,
> + ? ? ? .mpu_irqs ? ? ? = omap3xxx_isp_mmu_irqs,
> + ? ? ? .main_clk ? ? ? = "cam_ick",

It's not "cam_fck"?

> + ? ? ? .dev_attr ? ? ? = &isp_mmu_dev_attr,
> + ? ? ? .slaves ? ? ? ? = omap3xxx_isp_mmu_slaves,
> + ? ? ? .slaves_cnt ? ? = ARRAY_SIZE(omap3xxx_isp_mmu_slaves),
> + ? ? ? .flags ? ? ? ? ?= HWMOD_NO_IDLEST,
> +};

Most of the stuff I see the hwmods is .main_lock = "foo_fck", slave
.clk = "foo_ick". Maybe that explains the irq issues you get.

Cheers.

-- 
Felipe Contreras

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

* Re: [PATCH v4 3/4] OMAP3/4: iommu: migrate to hwmod framework
  2011-12-15  4:18   ` Omar Ramirez Luna
@ 2011-12-16  0:47     ` Felipe Contreras
  -1 siblings, 0 replies; 48+ messages in thread
From: Felipe Contreras @ 2011-12-16  0:47 UTC (permalink / raw)
  To: Omar Ramirez Luna
  Cc: Tony Lindgren, Benoit Cousson, Kevin Hilman, Ohad Ben-Cohen,
	Russell King, Linux PM, Rafael J. Wysocki, iommu, MyungJoo Ham,
	Laurent Pinchart, linux-omap, linux-arm-kernel

On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
> 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.

Looks good to me.

-- 
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] 48+ messages in thread

* [PATCH v4 3/4] OMAP3/4: iommu: migrate to hwmod framework
@ 2011-12-16  0:47     ` Felipe Contreras
  0 siblings, 0 replies; 48+ messages in thread
From: Felipe Contreras @ 2011-12-16  0:47 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
> 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.

Looks good to me.

-- 
Felipe Contreras

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

* Re: [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
  2011-12-15  4:18   ` Omar Ramirez Luna
@ 2011-12-16  0:53     ` Felipe Contreras
  -1 siblings, 0 replies; 48+ messages in thread
From: Felipe Contreras @ 2011-12-16  0:53 UTC (permalink / raw)
  To: Omar Ramirez Luna
  Cc: Tony Lindgren, Benoit Cousson, Kevin Hilman, Ohad Ben-Cohen,
	Russell King, Linux PM, Rafael J. Wysocki, iommu, MyungJoo Ham,
	Laurent Pinchart, linux-omap, linux-arm-kernel

On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
> Use runtime PM functionality interfaced with hwmod enable/idle
> functions, to replace direct clock operations, reset and sysconfig
> handling.
>
> Removed clk handling during interrupt, given that in order to receive one,
> the device should be powered on in advance. Now doing pm_runtime_get/put
> on iommu_enable/disable so it doesn't rely on others to keep the clocks on.
>
> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
> ---
>  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               |   44 ++++++++++++-----------------
>  5 files changed, 18 insertions(+), 48 deletions(-)

Shouldn't pm_runtime_enable() be called in omap_iommu_init(), and
omap_iommu_probe() call pm_runtime_get_sync()/put() on the sections
where the device should be active?

Cheers.

-- 
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] 48+ messages in thread

* [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
@ 2011-12-16  0:53     ` Felipe Contreras
  0 siblings, 0 replies; 48+ messages in thread
From: Felipe Contreras @ 2011-12-16  0:53 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
> Use runtime PM functionality interfaced with hwmod enable/idle
> functions, to replace direct clock operations, reset and sysconfig
> handling.
>
> Removed clk handling during interrupt, given that in order to receive one,
> the device should be powered on in advance. Now doing pm_runtime_get/put
> on iommu_enable/disable so it doesn't rely on others to keep the clocks on.
>
> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
> ---
> ?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 ? ? ? ? ? ? ? | ? 44 ++++++++++++-----------------
> ?5 files changed, 18 insertions(+), 48 deletions(-)

Shouldn't pm_runtime_enable() be called in omap_iommu_init(), and
omap_iommu_probe() call pm_runtime_get_sync()/put() on the sections
where the device should be active?

Cheers.

-- 
Felipe Contreras

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

* Re: [PATCH v4 1/4] OMAP3: hwmod data: add mmu data for iva and isp
  2011-12-16  0:39     ` Felipe Contreras
@ 2011-12-16  2:01       ` Ramirez Luna, Omar
  -1 siblings, 0 replies; 48+ messages in thread
From: Ramirez Luna, Omar @ 2011-12-16  2:01 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Tony Lindgren, Benoit Cousson, Kevin Hilman, Ohad Ben-Cohen,
	Russell King, Linux PM, Rafael J. Wysocki, iommu, MyungJoo Ham,
	Laurent Pinchart, linux-omap, linux-arm-kernel

On Thu, Dec 15, 2011 at 6:39 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
>> +/* l4_core -> isp mmu */
>> +static struct omap_hwmod_ocp_if omap3xxx_l4_core__isp_mmu = {
>> +       .master         = &omap3xxx_l4_core_hwmod,
>> +       .slave          = &omap3xxx_isp_mmu_hwmod,
>> +       .addr           = omap3xxx_isp_mmu_addrs,
>> +       .user           = OCP_USER_MPU | OCP_USER_SDMA,
>> +};
>
> Are you sure you are not missing something like:
>
>  .clk = "cam_ick",

I believe in this case cam_ick is used as the main clock as it
supplies both functional and interface.

>
>> +/* isp mmu slave ports */
>> +static struct omap_hwmod_ocp_if *omap3xxx_isp_mmu_slaves[] = {
>> +       &omap3xxx_l4_core__isp_mmu,
>> +};
>> +
>> +static struct omap_hwmod omap3xxx_isp_mmu_hwmod = {
>> +       .name           = "isp_mmu",
>> +       .class          = &omap3xxx_mmu_hwmod_class,
>> +       .mpu_irqs       = omap3xxx_isp_mmu_irqs,
>> +       .main_clk       = "cam_ick",
>
> It's not "cam_fck"?

AFAIK cam_fck doesn't exist in the code, and CAM_L3_ICK is used as
both ick/fck according to TRM.

>
>> +       .dev_attr       = &isp_mmu_dev_attr,
>> +       .slaves         = omap3xxx_isp_mmu_slaves,
>> +       .slaves_cnt     = ARRAY_SIZE(omap3xxx_isp_mmu_slaves),
>> +       .flags          = HWMOD_NO_IDLEST,
>> +};
>
> Most of the stuff I see the hwmods is .main_lock = "foo_fck", slave
> .clk = "foo_ick". Maybe that explains the irq issues you get.

I see irq issues with iva hwmod because tidspbridge doesn't use iommu
API yet, so if you enable both the mmu hwmod and tidspbridge own mmu
implementation there will be some conflicts.

I didn't see isp issues though, but I didn't went more than
booting/enabling with isp mmu.

Regards,

Omar
--
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] 48+ messages in thread

* [PATCH v4 1/4] OMAP3: hwmod data: add mmu data for iva and isp
@ 2011-12-16  2:01       ` Ramirez Luna, Omar
  0 siblings, 0 replies; 48+ messages in thread
From: Ramirez Luna, Omar @ 2011-12-16  2:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Dec 15, 2011 at 6:39 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
>> +/* l4_core -> isp mmu */
>> +static struct omap_hwmod_ocp_if omap3xxx_l4_core__isp_mmu = {
>> + ? ? ? .master ? ? ? ? = &omap3xxx_l4_core_hwmod,
>> + ? ? ? .slave ? ? ? ? ?= &omap3xxx_isp_mmu_hwmod,
>> + ? ? ? .addr ? ? ? ? ? = omap3xxx_isp_mmu_addrs,
>> + ? ? ? .user ? ? ? ? ? = OCP_USER_MPU | OCP_USER_SDMA,
>> +};
>
> Are you sure you are not missing something like:
>
> ?.clk = "cam_ick",

I believe in this case cam_ick is used as the main clock as it
supplies both functional and interface.

>
>> +/* isp mmu slave ports */
>> +static struct omap_hwmod_ocp_if *omap3xxx_isp_mmu_slaves[] = {
>> + ? ? ? &omap3xxx_l4_core__isp_mmu,
>> +};
>> +
>> +static struct omap_hwmod omap3xxx_isp_mmu_hwmod = {
>> + ? ? ? .name ? ? ? ? ? = "isp_mmu",
>> + ? ? ? .class ? ? ? ? ?= &omap3xxx_mmu_hwmod_class,
>> + ? ? ? .mpu_irqs ? ? ? = omap3xxx_isp_mmu_irqs,
>> + ? ? ? .main_clk ? ? ? = "cam_ick",
>
> It's not "cam_fck"?

AFAIK cam_fck doesn't exist in the code, and CAM_L3_ICK is used as
both ick/fck according to TRM.

>
>> + ? ? ? .dev_attr ? ? ? = &isp_mmu_dev_attr,
>> + ? ? ? .slaves ? ? ? ? = omap3xxx_isp_mmu_slaves,
>> + ? ? ? .slaves_cnt ? ? = ARRAY_SIZE(omap3xxx_isp_mmu_slaves),
>> + ? ? ? .flags ? ? ? ? ?= HWMOD_NO_IDLEST,
>> +};
>
> Most of the stuff I see the hwmods is .main_lock = "foo_fck", slave
> .clk = "foo_ick". Maybe that explains the irq issues you get.

I see irq issues with iva hwmod because tidspbridge doesn't use iommu
API yet, so if you enable both the mmu hwmod and tidspbridge own mmu
implementation there will be some conflicts.

I didn't see isp issues though, but I didn't went more than
booting/enabling with isp mmu.

Regards,

Omar

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

* Re: [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
  2011-12-16  0:33     ` Felipe Contreras
@ 2011-12-16  2:59       ` Ramirez Luna, Omar
  -1 siblings, 0 replies; 48+ messages in thread
From: Ramirez Luna, Omar @ 2011-12-16  2:59 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Tony Lindgren, Benoit Cousson, Kevin Hilman, Ohad Ben-Cohen,
	Russell King, Linux PM, Rafael J. Wysocki, iommu, MyungJoo Ham,
	Laurent Pinchart, linux-omap, linux-arm-kernel

On Thu, Dec 15, 2011 at 6:33 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
>> @@ -123,11 +123,10 @@ static int iommu_enable(struct omap_iommu *obj)
>>        if (!arch_iommu)
>>                return -ENODEV;
>>
>> -       clk_enable(obj->clk);
>> +       pm_runtime_get_sync(obj->dev);
>>
>>        err = arch_iommu->enable(obj);
>>
>> -       clk_disable(obj->clk);
>>        return err;
>>  }
>
> Hmm, this is called on omap_iommu_attach, and iommu_disable is not
> called until omap_iommu_detach, so this means the device will never
> sleep. Why don't you call pm_runtime_put() instead of clk_disable()?

Here once you call pm_runtime_put, the hwmod should turn off iva2_ck
leaving device unpowered, not usable if it is part of an independent
module and that module is still awake, but since the module clock
feeds both the dsp and the mmu this doesn't occur. However might be
possible if there is a specific clock to feed the mmu and another for
the mmu user.

In theory iommu should be independent on handling its clock resources,
take tidspbridge, it powers iva2_clk which indirectly powers iva_mmu,
so as long as the dsp is powered and using the mmu (omap_iommu_attach)
the iommu should be ON, once you omap_iommu_detach means that the
device is no longer using the iommu and can be put to sleep.

This could be helpful if your main processor can go to sleep
independently of other processors, say you attach the iommu and leave
the dsp doing calculations and memory accesses through mmu while the
main processor decides to enter sleep.

> All the rest of the calls to pm_runtime_get/put after this are
> basically no-ops, because the count will never go below 1.

I didn't try but without calling iommu_attach the other functions that
use pm_runtime_get/put can still be called as they are exported
symbols. I believe omap-iommu-debug.c does something like this to dump
stuff. This should be cleaned up/audited, IMHO, but it doesn't seem to
belong to this conversion series.

> You mention some irq issues, but could it be due to some bad clocks in
> the hwmod data?

There are no "issues" with irqs, just a weird case, see below...

...
>> @@ -780,9 +777,7 @@ static irqreturn_t iommu_fault_handler(int irq, void *data)
>>        if (!obj->refcount)
>>                return IRQ_NONE;
>>
>> -       clk_enable(obj->clk);
>>        errs = iommu_report_fault(obj, &da);
>> -       clk_disable(obj->clk);
>
> Why?

In order to get an irq from the mmu, the mmu should be functional and
have a clock, but iommu_enable used to enable and disable the clock.

In the end you are able to get an irq because someone else
(tidspbridge) has the mmu indirectly powered (since they share the
same clock), I felt this shouldn't be and the iommu should handle its
own clocks even if they are shared with other modules.

Hence iommu_enable does pm_runtime_get for the life time of the user of the mmu.

...
>> @@ -996,7 +987,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);
>
> I'm not sure if this is needed; you want to resume the driver? AFAICS
> kfree will take care of that _without_ resuming.

No, the driver should resume only if there are outstanding wake up
requests, right? I don't seem to get this question.

Thanks,

Omar
--
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] 48+ messages in thread

* [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
@ 2011-12-16  2:59       ` Ramirez Luna, Omar
  0 siblings, 0 replies; 48+ messages in thread
From: Ramirez Luna, Omar @ 2011-12-16  2:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Dec 15, 2011 at 6:33 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
>> @@ -123,11 +123,10 @@ static int iommu_enable(struct omap_iommu *obj)
>> ? ? ? ?if (!arch_iommu)
>> ? ? ? ? ? ? ? ?return -ENODEV;
>>
>> - ? ? ? clk_enable(obj->clk);
>> + ? ? ? pm_runtime_get_sync(obj->dev);
>>
>> ? ? ? ?err = arch_iommu->enable(obj);
>>
>> - ? ? ? clk_disable(obj->clk);
>> ? ? ? ?return err;
>> ?}
>
> Hmm, this is called on omap_iommu_attach, and iommu_disable is not
> called until omap_iommu_detach, so this means the device will never
> sleep. Why don't you call pm_runtime_put() instead of clk_disable()?

Here once you call pm_runtime_put, the hwmod should turn off iva2_ck
leaving device unpowered, not usable if it is part of an independent
module and that module is still awake, but since the module clock
feeds both the dsp and the mmu this doesn't occur. However might be
possible if there is a specific clock to feed the mmu and another for
the mmu user.

In theory iommu should be independent on handling its clock resources,
take tidspbridge, it powers iva2_clk which indirectly powers iva_mmu,
so as long as the dsp is powered and using the mmu (omap_iommu_attach)
the iommu should be ON, once you omap_iommu_detach means that the
device is no longer using the iommu and can be put to sleep.

This could be helpful if your main processor can go to sleep
independently of other processors, say you attach the iommu and leave
the dsp doing calculations and memory accesses through mmu while the
main processor decides to enter sleep.

> All the rest of the calls to pm_runtime_get/put after this are
> basically no-ops, because the count will never go below 1.

I didn't try but without calling iommu_attach the other functions that
use pm_runtime_get/put can still be called as they are exported
symbols. I believe omap-iommu-debug.c does something like this to dump
stuff. This should be cleaned up/audited, IMHO, but it doesn't seem to
belong to this conversion series.

> You mention some irq issues, but could it be due to some bad clocks in
> the hwmod data?

There are no "issues" with irqs, just a weird case, see below...

...
>> @@ -780,9 +777,7 @@ static irqreturn_t iommu_fault_handler(int irq, void *data)
>> ? ? ? ?if (!obj->refcount)
>> ? ? ? ? ? ? ? ?return IRQ_NONE;
>>
>> - ? ? ? clk_enable(obj->clk);
>> ? ? ? ?errs = iommu_report_fault(obj, &da);
>> - ? ? ? clk_disable(obj->clk);
>
> Why?

In order to get an irq from the mmu, the mmu should be functional and
have a clock, but iommu_enable used to enable and disable the clock.

In the end you are able to get an irq because someone else
(tidspbridge) has the mmu indirectly powered (since they share the
same clock), I felt this shouldn't be and the iommu should handle its
own clocks even if they are shared with other modules.

Hence iommu_enable does pm_runtime_get for the life time of the user of the mmu.

...
>> @@ -996,7 +987,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);
>
> I'm not sure if this is needed; you want to resume the driver? AFAICS
> kfree will take care of that _without_ resuming.

No, the driver should resume only if there are outstanding wake up
requests, right? I don't seem to get this question.

Thanks,

Omar

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

* Re: [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
  2011-12-16  0:53     ` Felipe Contreras
@ 2011-12-16  3:18       ` Ramirez Luna, Omar
  -1 siblings, 0 replies; 48+ messages in thread
From: Ramirez Luna, Omar @ 2011-12-16  3:18 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Tony Lindgren, Benoit Cousson, Kevin Hilman, Ohad Ben-Cohen,
	Russell King, Linux PM, Rafael J. Wysocki, iommu, MyungJoo Ham,
	Laurent Pinchart, linux-omap, linux-arm-kernel

On Thu, Dec 15, 2011 at 6:53 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
>> Use runtime PM functionality interfaced with hwmod enable/idle
>> functions, to replace direct clock operations, reset and sysconfig
>> handling.
>>
>> Removed clk handling during interrupt, given that in order to receive one,
>> the device should be powered on in advance. Now doing pm_runtime_get/put
>> on iommu_enable/disable so it doesn't rely on others to keep the clocks on.
>>
>> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
>> ---
>>  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               |   44 ++++++++++++-----------------
>>  5 files changed, 18 insertions(+), 48 deletions(-)
>
> Shouldn't pm_runtime_enable() be called in omap_iommu_init(), and
> omap_iommu_probe() call pm_runtime_get_sync()/put() on the sections
> where the device should be active?

omap_iommu_init is called on module init however omap_iommu_probe is
called by driver instance (isp, iva), on probe pm_runtime_enable
activates runtime for both isp and iva devices, one at a time.

Regards,

Omar
--
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] 48+ messages in thread

* [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
@ 2011-12-16  3:18       ` Ramirez Luna, Omar
  0 siblings, 0 replies; 48+ messages in thread
From: Ramirez Luna, Omar @ 2011-12-16  3:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Dec 15, 2011 at 6:53 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
>> Use runtime PM functionality interfaced with hwmod enable/idle
>> functions, to replace direct clock operations, reset and sysconfig
>> handling.
>>
>> Removed clk handling during interrupt, given that in order to receive one,
>> the device should be powered on in advance. Now doing pm_runtime_get/put
>> on iommu_enable/disable so it doesn't rely on others to keep the clocks on.
>>
>> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
>> ---
>> ?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 ? ? ? ? ? ? ? | ? 44 ++++++++++++-----------------
>> ?5 files changed, 18 insertions(+), 48 deletions(-)
>
> Shouldn't pm_runtime_enable() be called in omap_iommu_init(), and
> omap_iommu_probe() call pm_runtime_get_sync()/put() on the sections
> where the device should be active?

omap_iommu_init is called on module init however omap_iommu_probe is
called by driver instance (isp, iva), on probe pm_runtime_enable
activates runtime for both isp and iva devices, one at a time.

Regards,

Omar

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

* Re: [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
  2011-12-15  4:18   ` Omar Ramirez Luna
@ 2011-12-17  1:39     ` Tony Lindgren
  -1 siblings, 0 replies; 48+ messages in thread
From: Tony Lindgren @ 2011-12-17  1:39 UTC (permalink / raw)
  To: Omar Ramirez Luna
  Cc: Benoit Cousson, Russell King, Kevin Hilman, Ohad Ben-Cohen,
	Rafael J. Wysocki, Joerg Roedel, Laurent Pinchart, MyungJoo Ham,
	linux-omap, linux-arm-kernel, iommu, Linux PM

* Omar Ramirez Luna <omar.ramirez@ti.com> [111214 19:47]:
> Use runtime PM functionality interfaced with hwmod enable/idle
> functions, to replace direct clock operations, reset and sysconfig
> handling.
> 
> Removed clk handling during interrupt, given that in order to receive one,
> the device should be powered on in advance. Now doing pm_runtime_get/put
> on iommu_enable/disable so it doesn't rely on others to keep the clocks on.
> 
> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>

The omap parts look OK to me for Ohad to queue:

Acked-by: Tony Lindgren <tony@atomide.com>

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

* [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
@ 2011-12-17  1:39     ` Tony Lindgren
  0 siblings, 0 replies; 48+ messages in thread
From: Tony Lindgren @ 2011-12-17  1:39 UTC (permalink / raw)
  To: linux-arm-kernel

* Omar Ramirez Luna <omar.ramirez@ti.com> [111214 19:47]:
> Use runtime PM functionality interfaced with hwmod enable/idle
> functions, to replace direct clock operations, reset and sysconfig
> handling.
> 
> Removed clk handling during interrupt, given that in order to receive one,
> the device should be powered on in advance. Now doing pm_runtime_get/put
> on iommu_enable/disable so it doesn't rely on others to keep the clocks on.
> 
> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>

The omap parts look OK to me for Ohad to queue:

Acked-by: Tony Lindgren <tony@atomide.com>

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

* Re: [PATCH v4 1/4] OMAP3: hwmod data: add mmu data for iva and isp
  2011-12-16  2:01       ` Ramirez Luna, Omar
@ 2011-12-19 16:11         ` Felipe Contreras
  -1 siblings, 0 replies; 48+ messages in thread
From: Felipe Contreras @ 2011-12-19 16:11 UTC (permalink / raw)
  To: Ramirez Luna, Omar
  Cc: Tony Lindgren, Benoit Cousson, Kevin Hilman, Ohad Ben-Cohen,
	Russell King, Linux PM, Rafael J. Wysocki, iommu, MyungJoo Ham,
	Laurent Pinchart, linux-omap, linux-arm-kernel

On Fri, Dec 16, 2011 at 4:01 AM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
> On Thu, Dec 15, 2011 at 6:39 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> Are you sure you are not missing something like:
>>
>>  .clk = "cam_ick",
>
> I believe in this case cam_ick is used as the main clock as it
> supplies both functional and interface.

Are you sure?

>>> +/* isp mmu slave ports */
>>> +static struct omap_hwmod_ocp_if *omap3xxx_isp_mmu_slaves[] = {
>>> +       &omap3xxx_l4_core__isp_mmu,
>>> +};
>>> +
>>> +static struct omap_hwmod omap3xxx_isp_mmu_hwmod = {
>>> +       .name           = "isp_mmu",
>>> +       .class          = &omap3xxx_mmu_hwmod_class,
>>> +       .mpu_irqs       = omap3xxx_isp_mmu_irqs,
>>> +       .main_clk       = "cam_ick",
>>
>> It's not "cam_fck"?
>
> AFAIK cam_fck doesn't exist in the code, and CAM_L3_ICK is used as
> both ick/fck according to TRM.

Then maybe the code is wrong.

Look at the OMAP34xx documentation, it says that:

CAM_L3_ICLK -> CAM_FCLK
CAM_L4_ICLK -> CAM_ICLK
CAM_MCLK -> CAM_MCLK
CSI2_96M_FCLK -> CSI2_96M_FCLK

CAM_FCLK
Functional clock (L3 interconnect clock domain)
Functional clock domain.

CAM_ICLK
Interface clock (L4 interconnect clock domain)
Interface clock domain.

Maybe something like this:

--- a/arch/arm/mach-omap2/clock3xxx_data.c
+++ b/arch/arm/mach-omap2/clock3xxx_data.c
@@ -2225,8 +2225,17 @@ static struct clk cam_mclk = {
        .recalc         = &followparent_recalc,
 };

+static struct clk cam_fck = {
+       .name           = "cam_fck",
+       .ops            = &clkops_omap2_dflt,
+       .parent         = &l3_ick,
+       .enable_reg     = OMAP_CM_REGADDR(OMAP3430_CAM_MOD, CM_FCLKEN),
+       .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,
@@ -3364,6 +3373,7 @@ static struct omap_clk omap3xxx_clks[] = {
        CLK("omapdss_dss",      "ick",          &dss_ick_3430es1,
 CK_3430ES1),
        CLK("omapdss_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),

Also, I find this chunk peculiar in drivers/media/video/omap3isp/isp.c:

static const char *isp_clocks[] = {
	"cam_ick",
	"cam_mclk",
	"dpll4_m5_ck",
	"csi2_96m_fck",
	"l3_ick",
};

Looks like the driver is manually calling clk_get() and clk_put() for
the "i3_ick", where I guess the clock framework is supposed to do
that... It's almost as if somebody forgot a dependency somewhere.

>>> +       .dev_attr       = &isp_mmu_dev_attr,
>>> +       .slaves         = omap3xxx_isp_mmu_slaves,
>>> +       .slaves_cnt     = ARRAY_SIZE(omap3xxx_isp_mmu_slaves),
>>> +       .flags          = HWMOD_NO_IDLEST,
>>> +};
>>
>> Most of the stuff I see the hwmods is .main_lock = "foo_fck", slave
>> .clk = "foo_ick". Maybe that explains the irq issues you get.
>
> I see irq issues with iva hwmod because tidspbridge doesn't use iommu
> API yet, so if you enable both the mmu hwmod and tidspbridge own mmu
> implementation there will be some conflicts.
>
> I didn't see isp issues though, but I didn't went more than
> booting/enabling with isp mmu.

This is what you said:
Removed clk handling during interrupt, given that in order to receive one,
the device should be powered on in advance.

I'm not sure how this clock stuff works, but I'm guessing the device
is supposed to go to sleep at some points in time, and with your patch
"OMAP3/4: iommu: adapt to runtime pm" it won't, as long as the module
is loaded, unless I'm missing something.

Cheers.

-- 
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] 48+ messages in thread

* [PATCH v4 1/4] OMAP3: hwmod data: add mmu data for iva and isp
@ 2011-12-19 16:11         ` Felipe Contreras
  0 siblings, 0 replies; 48+ messages in thread
From: Felipe Contreras @ 2011-12-19 16:11 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Dec 16, 2011 at 4:01 AM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
> On Thu, Dec 15, 2011 at 6:39 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> Are you sure you are not missing something like:
>>
>> ?.clk = "cam_ick",
>
> I believe in this case cam_ick is used as the main clock as it
> supplies both functional and interface.

Are you sure?

>>> +/* isp mmu slave ports */
>>> +static struct omap_hwmod_ocp_if *omap3xxx_isp_mmu_slaves[] = {
>>> + ? ? ? &omap3xxx_l4_core__isp_mmu,
>>> +};
>>> +
>>> +static struct omap_hwmod omap3xxx_isp_mmu_hwmod = {
>>> + ? ? ? .name ? ? ? ? ? = "isp_mmu",
>>> + ? ? ? .class ? ? ? ? ?= &omap3xxx_mmu_hwmod_class,
>>> + ? ? ? .mpu_irqs ? ? ? = omap3xxx_isp_mmu_irqs,
>>> + ? ? ? .main_clk ? ? ? = "cam_ick",
>>
>> It's not "cam_fck"?
>
> AFAIK cam_fck doesn't exist in the code, and CAM_L3_ICK is used as
> both ick/fck according to TRM.

Then maybe the code is wrong.

Look at the OMAP34xx documentation, it says that:

CAM_L3_ICLK -> CAM_FCLK
CAM_L4_ICLK -> CAM_ICLK
CAM_MCLK -> CAM_MCLK
CSI2_96M_FCLK -> CSI2_96M_FCLK

CAM_FCLK
Functional clock (L3 interconnect clock domain)
Functional clock domain.

CAM_ICLK
Interface clock (L4 interconnect clock domain)
Interface clock domain.

Maybe something like this:

--- a/arch/arm/mach-omap2/clock3xxx_data.c
+++ b/arch/arm/mach-omap2/clock3xxx_data.c
@@ -2225,8 +2225,17 @@ static struct clk cam_mclk = {
        .recalc         = &followparent_recalc,
 };

+static struct clk cam_fck = {
+       .name           = "cam_fck",
+       .ops            = &clkops_omap2_dflt,
+       .parent         = &l3_ick,
+       .enable_reg     = OMAP_CM_REGADDR(OMAP3430_CAM_MOD, CM_FCLKEN),
+       .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,
@@ -3364,6 +3373,7 @@ static struct omap_clk omap3xxx_clks[] = {
        CLK("omapdss_dss",      "ick",          &dss_ick_3430es1,
 CK_3430ES1),
        CLK("omapdss_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),

Also, I find this chunk peculiar in drivers/media/video/omap3isp/isp.c:

static const char *isp_clocks[] = {
	"cam_ick",
	"cam_mclk",
	"dpll4_m5_ck",
	"csi2_96m_fck",
	"l3_ick",
};

Looks like the driver is manually calling clk_get() and clk_put() for
the "i3_ick", where I guess the clock framework is supposed to do
that... It's almost as if somebody forgot a dependency somewhere.

>>> + ? ? ? .dev_attr ? ? ? = &isp_mmu_dev_attr,
>>> + ? ? ? .slaves ? ? ? ? = omap3xxx_isp_mmu_slaves,
>>> + ? ? ? .slaves_cnt ? ? = ARRAY_SIZE(omap3xxx_isp_mmu_slaves),
>>> + ? ? ? .flags ? ? ? ? ?= HWMOD_NO_IDLEST,
>>> +};
>>
>> Most of the stuff I see the hwmods is .main_lock = "foo_fck", slave
>> .clk = "foo_ick". Maybe that explains the irq issues you get.
>
> I see irq issues with iva hwmod because tidspbridge doesn't use iommu
> API yet, so if you enable both the mmu hwmod and tidspbridge own mmu
> implementation there will be some conflicts.
>
> I didn't see isp issues though, but I didn't went more than
> booting/enabling with isp mmu.

This is what you said:
Removed clk handling during interrupt, given that in order to receive one,
the device should be powered on in advance.

I'm not sure how this clock stuff works, but I'm guessing the device
is supposed to go to sleep at some points in time, and with your patch
"OMAP3/4: iommu: adapt to runtime pm" it won't, as long as the module
is loaded, unless I'm missing something.

Cheers.

-- 
Felipe Contreras

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

* Re: [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
  2011-12-16  3:18       ` Ramirez Luna, Omar
@ 2011-12-19 16:27         ` Felipe Contreras
  -1 siblings, 0 replies; 48+ messages in thread
From: Felipe Contreras @ 2011-12-19 16:27 UTC (permalink / raw)
  To: Ramirez Luna, Omar
  Cc: Tony Lindgren, Benoit Cousson, Kevin Hilman, Ohad Ben-Cohen,
	Russell King, Linux PM, Rafael J. Wysocki, iommu, MyungJoo Ham,
	Laurent Pinchart, linux-omap, linux-arm-kernel, Paul Walmsley

On Fri, Dec 16, 2011 at 5:18 AM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
> On Thu, Dec 15, 2011 at 6:53 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
>>> Use runtime PM functionality interfaced with hwmod enable/idle
>>> functions, to replace direct clock operations, reset and sysconfig
>>> handling.
>>>
>>> Removed clk handling during interrupt, given that in order to receive one,
>>> the device should be powered on in advance. Now doing pm_runtime_get/put
>>> on iommu_enable/disable so it doesn't rely on others to keep the clocks on.
>>>
>>> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
>>> ---
>>>  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               |   44 ++++++++++++-----------------
>>>  5 files changed, 18 insertions(+), 48 deletions(-)
>>
>> Shouldn't pm_runtime_enable() be called in omap_iommu_init(), and
>> omap_iommu_probe() call pm_runtime_get_sync()/put() on the sections
>> where the device should be active?
>
> omap_iommu_init is called on module init however omap_iommu_probe is
> called by driver instance (isp, iva), on probe pm_runtime_enable
> activates runtime for both isp and iva devices, one at a time.

Yes, but omap_iommu_init() will *always* be called at boot time and
will register the data for all the devices. If the 'iommu' module is
never loaded, then the devices will remain active the whole time.

Maybe Paul or somebody with better knowledge of the pm runtime
framework can clarify this.

Cheers.

-- 
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] 48+ messages in thread

* [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
@ 2011-12-19 16:27         ` Felipe Contreras
  0 siblings, 0 replies; 48+ messages in thread
From: Felipe Contreras @ 2011-12-19 16:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Dec 16, 2011 at 5:18 AM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
> On Thu, Dec 15, 2011 at 6:53 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
>>> Use runtime PM functionality interfaced with hwmod enable/idle
>>> functions, to replace direct clock operations, reset and sysconfig
>>> handling.
>>>
>>> Removed clk handling during interrupt, given that in order to receive one,
>>> the device should be powered on in advance. Now doing pm_runtime_get/put
>>> on iommu_enable/disable so it doesn't rely on others to keep the clocks on.
>>>
>>> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
>>> ---
>>> ?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 ? ? ? ? ? ? ? | ? 44 ++++++++++++-----------------
>>> ?5 files changed, 18 insertions(+), 48 deletions(-)
>>
>> Shouldn't pm_runtime_enable() be called in omap_iommu_init(), and
>> omap_iommu_probe() call pm_runtime_get_sync()/put() on the sections
>> where the device should be active?
>
> omap_iommu_init is called on module init however omap_iommu_probe is
> called by driver instance (isp, iva), on probe pm_runtime_enable
> activates runtime for both isp and iva devices, one at a time.

Yes, but omap_iommu_init() will *always* be called at boot time and
will register the data for all the devices. If the 'iommu' module is
never loaded, then the devices will remain active the whole time.

Maybe Paul or somebody with better knowledge of the pm runtime
framework can clarify this.

Cheers.

-- 
Felipe Contreras

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

* Re: [PATCH v4 1/4] OMAP3: hwmod data: add mmu data for iva and isp
  2011-12-19 16:11         ` Felipe Contreras
@ 2011-12-23 15:53           ` Ramirez Luna, Omar
  -1 siblings, 0 replies; 48+ messages in thread
From: Ramirez Luna, Omar @ 2011-12-23 15:53 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Tony Lindgren, Benoit Cousson, Kevin Hilman, Ohad Ben-Cohen,
	Russell King, Linux PM, Rafael J. Wysocki, iommu, MyungJoo Ham,
	Laurent Pinchart, linux-omap, linux-arm-kernel

On Mon, Dec 19, 2011 at 10:11 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Fri, Dec 16, 2011 at 4:01 AM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
>> On Thu, Dec 15, 2011 at 6:39 PM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>>> Are you sure you are not missing something like:
>>>
>>>  .clk = "cam_ick",
>>
>> I believe in this case cam_ick is used as the main clock as it
>> supplies both functional and interface.
>
> Are you sure?

As sure as 4.7.4.1.7 CAM Power Domain ;), if someone else could
clarify would be great to avoid the "are you sure" discussion.

>>>> +/* isp mmu slave ports */
>>>> +static struct omap_hwmod_ocp_if *omap3xxx_isp_mmu_slaves[] = {
>>>> +       &omap3xxx_l4_core__isp_mmu,
>>>> +};
>>>> +
>>>> +static struct omap_hwmod omap3xxx_isp_mmu_hwmod = {
>>>> +       .name           = "isp_mmu",
>>>> +       .class          = &omap3xxx_mmu_hwmod_class,
>>>> +       .mpu_irqs       = omap3xxx_isp_mmu_irqs,
>>>> +       .main_clk       = "cam_ick",
>>>
>>> It's not "cam_fck"?
>>
>> AFAIK cam_fck doesn't exist in the code, and CAM_L3_ICK is used as
>> both ick/fck according to TRM.
>
> Then maybe the code is wrong.
>
> Look at the OMAP34xx documentation, it says that:
>
> CAM_L3_ICLK -> CAM_FCLK
> CAM_L4_ICLK -> CAM_ICLK
> CAM_MCLK -> CAM_MCLK
> CSI2_96M_FCLK -> CSI2_96M_FCLK
>
> CAM_FCLK
> Functional clock (L3 interconnect clock domain)
> Functional clock domain.
>
> CAM_ICLK
> Interface clock (L4 interconnect clock domain)
> Interface clock domain.

"The camera subsystem interface is clocked with the L3 and L4 clocks
(CAM_L3_ICLK and CAM_L4_ICLK, respectively). CAM_L3_ICLK is also used
as the main functional clock. The functional clock (CAM_MCLK) is
provided by DPLL4 to supply the external sensor."

Either CAM subsystem section or PRCM - CAM PWRDM is ambiguous or wrong.

...
>
> Looks like the driver is manually calling clk_get() and clk_put() for
> the "i3_ick", where I guess the clock framework is supposed to do
> that... It's almost as if somebody forgot a dependency somewhere.

Would be good to clarify the intentions to keep the code as it is.

>>>> +       .dev_attr       = &isp_mmu_dev_attr,
>>>> +       .slaves         = omap3xxx_isp_mmu_slaves,
>>>> +       .slaves_cnt     = ARRAY_SIZE(omap3xxx_isp_mmu_slaves),
>>>> +       .flags          = HWMOD_NO_IDLEST,
>>>> +};
>>>
>>> Most of the stuff I see the hwmods is .main_lock = "foo_fck", slave
>>> .clk = "foo_ick". Maybe that explains the irq issues you get.
>>
>> I see irq issues with iva hwmod because tidspbridge doesn't use iommu
>> API yet, so if you enable both the mmu hwmod and tidspbridge own mmu
>> implementation there will be some conflicts.
>>
>> I didn't see isp issues though, but I didn't went more than
>> booting/enabling with isp mmu.
>
> This is what you said:
> Removed clk handling during interrupt, given that in order to receive one,
> the device should be powered on in advance.

Yes, you should receive an interrupt if the clock is enabled and the
iommu is being used, hence the part inside in the ISR to enable the
clocks was removed.

> I'm not sure how this clock stuff works, but I'm guessing the device
> is supposed to go to sleep at some points in time, and with your patch
> "OMAP3/4: iommu: adapt to runtime pm" it won't, as long as the module
> is loaded, unless I'm missing something.

The device should be able to be put to sleep at anytime when it is NOT
being used. AFAIK there is no mechanism for the main processor (the
one running the kernel) to know when the other iommus are not being
used, given that they are in independent processors/subsystems, at
least for the ones in the DSP or M3 processors. Once the user releases
its iommu resource it means it is no longer using it, at that point
the device can be put to sleep.

Regards,

Omar
--
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] 48+ messages in thread

* [PATCH v4 1/4] OMAP3: hwmod data: add mmu data for iva and isp
@ 2011-12-23 15:53           ` Ramirez Luna, Omar
  0 siblings, 0 replies; 48+ messages in thread
From: Ramirez Luna, Omar @ 2011-12-23 15:53 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Dec 19, 2011 at 10:11 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Fri, Dec 16, 2011 at 4:01 AM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
>> On Thu, Dec 15, 2011 at 6:39 PM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>>> Are you sure you are not missing something like:
>>>
>>> ?.clk = "cam_ick",
>>
>> I believe in this case cam_ick is used as the main clock as it
>> supplies both functional and interface.
>
> Are you sure?

As sure as 4.7.4.1.7 CAM Power Domain ;), if someone else could
clarify would be great to avoid the "are you sure" discussion.

>>>> +/* isp mmu slave ports */
>>>> +static struct omap_hwmod_ocp_if *omap3xxx_isp_mmu_slaves[] = {
>>>> + ? ? ? &omap3xxx_l4_core__isp_mmu,
>>>> +};
>>>> +
>>>> +static struct omap_hwmod omap3xxx_isp_mmu_hwmod = {
>>>> + ? ? ? .name ? ? ? ? ? = "isp_mmu",
>>>> + ? ? ? .class ? ? ? ? ?= &omap3xxx_mmu_hwmod_class,
>>>> + ? ? ? .mpu_irqs ? ? ? = omap3xxx_isp_mmu_irqs,
>>>> + ? ? ? .main_clk ? ? ? = "cam_ick",
>>>
>>> It's not "cam_fck"?
>>
>> AFAIK cam_fck doesn't exist in the code, and CAM_L3_ICK is used as
>> both ick/fck according to TRM.
>
> Then maybe the code is wrong.
>
> Look at the OMAP34xx documentation, it says that:
>
> CAM_L3_ICLK -> CAM_FCLK
> CAM_L4_ICLK -> CAM_ICLK
> CAM_MCLK -> CAM_MCLK
> CSI2_96M_FCLK -> CSI2_96M_FCLK
>
> CAM_FCLK
> Functional clock (L3 interconnect clock domain)
> Functional clock domain.
>
> CAM_ICLK
> Interface clock (L4 interconnect clock domain)
> Interface clock domain.

"The camera subsystem interface is clocked with the L3 and L4 clocks
(CAM_L3_ICLK and CAM_L4_ICLK, respectively). CAM_L3_ICLK is also used
as the main functional clock. The functional clock (CAM_MCLK) is
provided by DPLL4 to supply the external sensor."

Either CAM subsystem section or PRCM - CAM PWRDM is ambiguous or wrong.

...
>
> Looks like the driver is manually calling clk_get() and clk_put() for
> the "i3_ick", where I guess the clock framework is supposed to do
> that... It's almost as if somebody forgot a dependency somewhere.

Would be good to clarify the intentions to keep the code as it is.

>>>> + ? ? ? .dev_attr ? ? ? = &isp_mmu_dev_attr,
>>>> + ? ? ? .slaves ? ? ? ? = omap3xxx_isp_mmu_slaves,
>>>> + ? ? ? .slaves_cnt ? ? = ARRAY_SIZE(omap3xxx_isp_mmu_slaves),
>>>> + ? ? ? .flags ? ? ? ? ?= HWMOD_NO_IDLEST,
>>>> +};
>>>
>>> Most of the stuff I see the hwmods is .main_lock = "foo_fck", slave
>>> .clk = "foo_ick". Maybe that explains the irq issues you get.
>>
>> I see irq issues with iva hwmod because tidspbridge doesn't use iommu
>> API yet, so if you enable both the mmu hwmod and tidspbridge own mmu
>> implementation there will be some conflicts.
>>
>> I didn't see isp issues though, but I didn't went more than
>> booting/enabling with isp mmu.
>
> This is what you said:
> Removed clk handling during interrupt, given that in order to receive one,
> the device should be powered on in advance.

Yes, you should receive an interrupt if the clock is enabled and the
iommu is being used, hence the part inside in the ISR to enable the
clocks was removed.

> I'm not sure how this clock stuff works, but I'm guessing the device
> is supposed to go to sleep at some points in time, and with your patch
> "OMAP3/4: iommu: adapt to runtime pm" it won't, as long as the module
> is loaded, unless I'm missing something.

The device should be able to be put to sleep at anytime when it is NOT
being used. AFAIK there is no mechanism for the main processor (the
one running the kernel) to know when the other iommus are not being
used, given that they are in independent processors/subsystems, at
least for the ones in the DSP or M3 processors. Once the user releases
its iommu resource it means it is no longer using it, at that point
the device can be put to sleep.

Regards,

Omar

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

* Re: [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
  2011-12-19 16:27         ` Felipe Contreras
@ 2011-12-23 16:30           ` Ramirez Luna, Omar
  -1 siblings, 0 replies; 48+ messages in thread
From: Ramirez Luna, Omar @ 2011-12-23 16:30 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Tony Lindgren, Benoit Cousson, Kevin Hilman, Ohad Ben-Cohen,
	Russell King, Linux PM, Rafael J. Wysocki, iommu, MyungJoo Ham,
	Laurent Pinchart, linux-omap, linux-arm-kernel, Paul Walmsley

On Mon, Dec 19, 2011 at 10:27 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Fri, Dec 16, 2011 at 5:18 AM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
>> On Thu, Dec 15, 2011 at 6:53 PM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>>> On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
>>>> Use runtime PM functionality interfaced with hwmod enable/idle
>>>> functions, to replace direct clock operations, reset and sysconfig
>>>> handling.
>>>>
>>>> Removed clk handling during interrupt, given that in order to receive one,
>>>> the device should be powered on in advance. Now doing pm_runtime_get/put
>>>> on iommu_enable/disable so it doesn't rely on others to keep the clocks on.
>>>>
>>>> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
>>>> ---
>>>>  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               |   44 ++++++++++++-----------------
>>>>  5 files changed, 18 insertions(+), 48 deletions(-)
>>>
>>> Shouldn't pm_runtime_enable() be called in omap_iommu_init(), and
>>> omap_iommu_probe() call pm_runtime_get_sync()/put() on the sections
>>> where the device should be active?
>>
>> omap_iommu_init is called on module init however omap_iommu_probe is
>> called by driver instance (isp, iva), on probe pm_runtime_enable
>> activates runtime for both isp and iva devices, one at a time.
>
> Yes, but omap_iommu_init() will *always* be called at boot time and
> will register the data for all the devices.

There are 2 omap_iommu_init :/ Thought you were talking about the one
in drivers/iommu/omap-iommu.c.

> If the 'iommu' module is
> never loaded, then the devices will remain active the whole time.

oma-iommu is meant to be built-in as part of the kernel, there is no
option for module anymore.

Regards,

Omar
--
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] 48+ messages in thread

* [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
@ 2011-12-23 16:30           ` Ramirez Luna, Omar
  0 siblings, 0 replies; 48+ messages in thread
From: Ramirez Luna, Omar @ 2011-12-23 16:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Dec 19, 2011 at 10:27 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Fri, Dec 16, 2011 at 5:18 AM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
>> On Thu, Dec 15, 2011 at 6:53 PM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>>> On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
>>>> Use runtime PM functionality interfaced with hwmod enable/idle
>>>> functions, to replace direct clock operations, reset and sysconfig
>>>> handling.
>>>>
>>>> Removed clk handling during interrupt, given that in order to receive one,
>>>> the device should be powered on in advance. Now doing pm_runtime_get/put
>>>> on iommu_enable/disable so it doesn't rely on others to keep the clocks on.
>>>>
>>>> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
>>>> ---
>>>> ?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 ? ? ? ? ? ? ? | ? 44 ++++++++++++-----------------
>>>> ?5 files changed, 18 insertions(+), 48 deletions(-)
>>>
>>> Shouldn't pm_runtime_enable() be called in omap_iommu_init(), and
>>> omap_iommu_probe() call pm_runtime_get_sync()/put() on the sections
>>> where the device should be active?
>>
>> omap_iommu_init is called on module init however omap_iommu_probe is
>> called by driver instance (isp, iva), on probe pm_runtime_enable
>> activates runtime for both isp and iva devices, one at a time.
>
> Yes, but omap_iommu_init() will *always* be called at boot time and
> will register the data for all the devices.

There are 2 omap_iommu_init :/ Thought you were talking about the one
in drivers/iommu/omap-iommu.c.

> If the 'iommu' module is
> never loaded, then the devices will remain active the whole time.

oma-iommu is meant to be built-in as part of the kernel, there is no
option for module anymore.

Regards,

Omar

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

* Re: [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
  2011-12-23 16:30           ` Ramirez Luna, Omar
@ 2011-12-23 17:04             ` Felipe Contreras
  -1 siblings, 0 replies; 48+ messages in thread
From: Felipe Contreras @ 2011-12-23 17:04 UTC (permalink / raw)
  To: Ramirez Luna, Omar
  Cc: Tony Lindgren, Benoit Cousson, Kevin Hilman, Ohad Ben-Cohen,
	Russell King, Linux PM, Rafael J. Wysocki, iommu, MyungJoo Ham,
	Laurent Pinchart, linux-omap, linux-arm-kernel, Paul Walmsley

On Fri, Dec 23, 2011 at 6:30 PM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
> On Mon, Dec 19, 2011 at 10:27 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Fri, Dec 16, 2011 at 5:18 AM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
>>> On Thu, Dec 15, 2011 at 6:53 PM, Felipe Contreras
>>> <felipe.contreras@gmail.com> wrote:
>>>> On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
>>>>> Use runtime PM functionality interfaced with hwmod enable/idle
>>>>> functions, to replace direct clock operations, reset and sysconfig
>>>>> handling.
>>>>>
>>>>> Removed clk handling during interrupt, given that in order to receive one,
>>>>> the device should be powered on in advance. Now doing pm_runtime_get/put
>>>>> on iommu_enable/disable so it doesn't rely on others to keep the clocks on.
>>>>>
>>>>> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
>>>>> ---
>>>>>  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               |   44 ++++++++++++-----------------
>>>>>  5 files changed, 18 insertions(+), 48 deletions(-)
>>>>
>>>> Shouldn't pm_runtime_enable() be called in omap_iommu_init(), and
>>>> omap_iommu_probe() call pm_runtime_get_sync()/put() on the sections
>>>> where the device should be active?
>>>
>>> omap_iommu_init is called on module init however omap_iommu_probe is
>>> called by driver instance (isp, iva), on probe pm_runtime_enable
>>> activates runtime for both isp and iva devices, one at a time.
>>
>> Yes, but omap_iommu_init() will *always* be called at boot time and
>> will register the data for all the devices.
>
> There are 2 omap_iommu_init :/ Thought you were talking about the one
> in drivers/iommu/omap-iommu.c.
>
>> If the 'iommu' module is
>> never loaded, then the devices will remain active the whole time.
>
> oma-iommu is meant to be built-in as part of the kernel, there is no
> option for module anymore.

Which omap-iommu? The platform driver, or the device stuff? The device
stuff is always built-in, but not the platform driver
(drivers/iommu/omap-iommu.c), that can be a module.

-- 
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] 48+ messages in thread

* [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
@ 2011-12-23 17:04             ` Felipe Contreras
  0 siblings, 0 replies; 48+ messages in thread
From: Felipe Contreras @ 2011-12-23 17:04 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Dec 23, 2011 at 6:30 PM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
> On Mon, Dec 19, 2011 at 10:27 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Fri, Dec 16, 2011 at 5:18 AM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
>>> On Thu, Dec 15, 2011 at 6:53 PM, Felipe Contreras
>>> <felipe.contreras@gmail.com> wrote:
>>>> On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
>>>>> Use runtime PM functionality interfaced with hwmod enable/idle
>>>>> functions, to replace direct clock operations, reset and sysconfig
>>>>> handling.
>>>>>
>>>>> Removed clk handling during interrupt, given that in order to receive one,
>>>>> the device should be powered on in advance. Now doing pm_runtime_get/put
>>>>> on iommu_enable/disable so it doesn't rely on others to keep the clocks on.
>>>>>
>>>>> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
>>>>> ---
>>>>> ?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 ? ? ? ? ? ? ? | ? 44 ++++++++++++-----------------
>>>>> ?5 files changed, 18 insertions(+), 48 deletions(-)
>>>>
>>>> Shouldn't pm_runtime_enable() be called in omap_iommu_init(), and
>>>> omap_iommu_probe() call pm_runtime_get_sync()/put() on the sections
>>>> where the device should be active?
>>>
>>> omap_iommu_init is called on module init however omap_iommu_probe is
>>> called by driver instance (isp, iva), on probe pm_runtime_enable
>>> activates runtime for both isp and iva devices, one at a time.
>>
>> Yes, but omap_iommu_init() will *always* be called at boot time and
>> will register the data for all the devices.
>
> There are 2 omap_iommu_init :/ Thought you were talking about the one
> in drivers/iommu/omap-iommu.c.
>
>> If the 'iommu' module is
>> never loaded, then the devices will remain active the whole time.
>
> oma-iommu is meant to be built-in as part of the kernel, there is no
> option for module anymore.

Which omap-iommu? The platform driver, or the device stuff? The device
stuff is always built-in, but not the platform driver
(drivers/iommu/omap-iommu.c), that can be a module.

-- 
Felipe Contreras

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

* Re: [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
  2011-12-23 17:04             ` Felipe Contreras
@ 2011-12-25  0:03               ` Ramirez Luna, Omar
  -1 siblings, 0 replies; 48+ messages in thread
From: Ramirez Luna, Omar @ 2011-12-25  0:03 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Tony Lindgren, Benoit Cousson, Kevin Hilman, Ohad Ben-Cohen,
	Russell King, Linux PM, Rafael J. Wysocki, iommu, MyungJoo Ham,
	Laurent Pinchart, linux-omap, linux-arm-kernel, Paul Walmsley

On Fri, Dec 23, 2011 at 11:04 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> Which omap-iommu? The platform driver, or the device stuff? The device
> stuff is always built-in, but not the platform driver
> (drivers/iommu/omap-iommu.c), that can be a module.

Both, I can't recall exactly when it changed (prior to being moved to
drivers/iommu it could be built as a module), but now
CONFIG_OMAP_IOMMU is boolean type.

Regards,

Omar

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

* [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
@ 2011-12-25  0:03               ` Ramirez Luna, Omar
  0 siblings, 0 replies; 48+ messages in thread
From: Ramirez Luna, Omar @ 2011-12-25  0:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Dec 23, 2011 at 11:04 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> Which omap-iommu? The platform driver, or the device stuff? The device
> stuff is always built-in, but not the platform driver
> (drivers/iommu/omap-iommu.c), that can be a module.

Both, I can't recall exactly when it changed (prior to being moved to
drivers/iommu it could be built as a module), but now
CONFIG_OMAP_IOMMU is boolean type.

Regards,

Omar

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

* Re: [PATCH v4 1/4] OMAP3: hwmod data: add mmu data for iva and isp
  2011-12-23 15:53           ` Ramirez Luna, Omar
@ 2011-12-25 21:08             ` Laurent Pinchart
  -1 siblings, 0 replies; 48+ messages in thread
From: Laurent Pinchart @ 2011-12-25 21:08 UTC (permalink / raw)
  To: Ramirez Luna, Omar
  Cc: Felipe Contreras, Tony Lindgren, Benoit Cousson, Kevin Hilman,
	Ohad Ben-Cohen, Russell King, Linux PM, Rafael J. Wysocki, iommu,
	MyungJoo Ham, linux-omap, linux-arm-kernel

Hi Omar,

On Friday 23 December 2011 16:53:58 Ramirez Luna, Omar wrote:
> On Mon, Dec 19, 2011 at 10:11 AM, Felipe Contreras wrote:
> > On Fri, Dec 16, 2011 at 4:01 AM, Ramirez Luna, Omar wrote:
> >> On Thu, Dec 15, 2011 at 6:39 PM, Felipe Contreras wrote:
> >>> Are you sure you are not missing something like:
> >>> 
> >>>  .clk = "cam_ick",
> >> 
> >> I believe in this case cam_ick is used as the main clock as it
> >> supplies both functional and interface.
> > 
> > Are you sure?
> 
> As sure as 4.7.4.1.7 CAM Power Domain ;), if someone else could
> clarify would be great to avoid the "are you sure" discussion.

I don't know where the ISP IOMMU fonctional and interface clocks come from, 
sorry. Shouldn't it be possible to find someone inside TI who can provide that 
information ? :-)

> >>>> +/* isp mmu slave ports */
> >>>> +static struct omap_hwmod_ocp_if *omap3xxx_isp_mmu_slaves[] = {
> >>>> +       &omap3xxx_l4_core__isp_mmu,
> >>>> +};
> >>>> +
> >>>> +static struct omap_hwmod omap3xxx_isp_mmu_hwmod = {
> >>>> +       .name           = "isp_mmu",
> >>>> +       .class          = &omap3xxx_mmu_hwmod_class,
> >>>> +       .mpu_irqs       = omap3xxx_isp_mmu_irqs,
> >>>> +       .main_clk       = "cam_ick",
> >>> 
> >>> It's not "cam_fck"?
> >> 
> >> AFAIK cam_fck doesn't exist in the code, and CAM_L3_ICK is used as
> >> both ick/fck according to TRM.
> > 
> > Then maybe the code is wrong.
> > 
> > Look at the OMAP34xx documentation, it says that:
> > 
> > CAM_L3_ICLK -> CAM_FCLK
> > CAM_L4_ICLK -> CAM_ICLK
> > CAM_MCLK -> CAM_MCLK
> > CSI2_96M_FCLK -> CSI2_96M_FCLK
> > 
> > CAM_FCLK
> > Functional clock (L3 interconnect clock domain)
> > Functional clock domain.
> > 
> > CAM_ICLK
> > Interface clock (L4 interconnect clock domain)
> > Interface clock domain.
> 
> "The camera subsystem interface is clocked with the L3 and L4 clocks
> (CAM_L3_ICLK and CAM_L4_ICLK, respectively). CAM_L3_ICLK is also used
> as the main functional clock. The functional clock (CAM_MCLK) is
> provided by DPLL4 to supply the external sensor."
> 
> Either CAM subsystem section or PRCM - CAM PWRDM is ambiguous or wrong.

I haven't checked how the clocks are used in details (I started working on the 
OMAP3 ISP driver after the clock handling code was in place), but I can at 
least confirm that CAM_L3_ICLK is the main functional clock. I would be a bit 
surprised if CAM_L3_ICLK was used as the interface clock as well.

Once again, you should be able to find clarification on this subject inside 
TI.

> ...
> 
> > Looks like the driver is manually calling clk_get() and clk_put() for
> > the "i3_ick", where I guess the clock framework is supposed to do
> > that... It's almost as if somebody forgot a dependency somewhere.
> 
> Would be good to clarify the intentions to keep the code as it is.

Once proper clock dependencies will be in place I'll be glad to remove the 
direct l3_ick if needed.

> >>>> +       .dev_attr       = &isp_mmu_dev_attr,
> >>>> +       .slaves         = omap3xxx_isp_mmu_slaves,
> >>>> +       .slaves_cnt     = ARRAY_SIZE(omap3xxx_isp_mmu_slaves),
> >>>> +       .flags          = HWMOD_NO_IDLEST,
> >>>> +};
> >>> 
> >>> Most of the stuff I see the hwmods is .main_lock = "foo_fck", slave
> >>> .clk = "foo_ick". Maybe that explains the irq issues you get.
> >> 
> >> I see irq issues with iva hwmod because tidspbridge doesn't use iommu
> >> API yet, so if you enable both the mmu hwmod and tidspbridge own mmu
> >> implementation there will be some conflicts.
> >> 
> >> I didn't see isp issues though, but I didn't went more than
> >> booting/enabling with isp mmu.
> > 
> > This is what you said:
> > Removed clk handling during interrupt, given that in order to receive
> > one, the device should be powered on in advance.
> 
> Yes, you should receive an interrupt if the clock is enabled and the
> iommu is being used, hence the part inside in the ISR to enable the
> clocks was removed.
> 
> > I'm not sure how this clock stuff works, but I'm guessing the device
> > is supposed to go to sleep at some points in time, and with your patch
> > "OMAP3/4: iommu: adapt to runtime pm" it won't, as long as the module
> > is loaded, unless I'm missing something.
> 
> The device should be able to be put to sleep at anytime when it is NOT
> being used. AFAIK there is no mechanism for the main processor (the
> one running the kernel) to know when the other iommus are not being
> used, given that they are in independent processors/subsystems, at
> least for the ones in the DSP or M3 processors. Once the user releases
> its iommu resource it means it is no longer using it, at that point
> the device can be put to sleep.

How should the OMAP3 ISP driver proceed to make sure that its IOMMU is clocked 
off when it doesn't need it ?

-- 
Regards,

Laurent Pinchart

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

* [PATCH v4 1/4] OMAP3: hwmod data: add mmu data for iva and isp
@ 2011-12-25 21:08             ` Laurent Pinchart
  0 siblings, 0 replies; 48+ messages in thread
From: Laurent Pinchart @ 2011-12-25 21:08 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Omar,

On Friday 23 December 2011 16:53:58 Ramirez Luna, Omar wrote:
> On Mon, Dec 19, 2011 at 10:11 AM, Felipe Contreras wrote:
> > On Fri, Dec 16, 2011 at 4:01 AM, Ramirez Luna, Omar wrote:
> >> On Thu, Dec 15, 2011 at 6:39 PM, Felipe Contreras wrote:
> >>> Are you sure you are not missing something like:
> >>> 
> >>>  .clk = "cam_ick",
> >> 
> >> I believe in this case cam_ick is used as the main clock as it
> >> supplies both functional and interface.
> > 
> > Are you sure?
> 
> As sure as 4.7.4.1.7 CAM Power Domain ;), if someone else could
> clarify would be great to avoid the "are you sure" discussion.

I don't know where the ISP IOMMU fonctional and interface clocks come from, 
sorry. Shouldn't it be possible to find someone inside TI who can provide that 
information ? :-)

> >>>> +/* isp mmu slave ports */
> >>>> +static struct omap_hwmod_ocp_if *omap3xxx_isp_mmu_slaves[] = {
> >>>> +       &omap3xxx_l4_core__isp_mmu,
> >>>> +};
> >>>> +
> >>>> +static struct omap_hwmod omap3xxx_isp_mmu_hwmod = {
> >>>> +       .name           = "isp_mmu",
> >>>> +       .class          = &omap3xxx_mmu_hwmod_class,
> >>>> +       .mpu_irqs       = omap3xxx_isp_mmu_irqs,
> >>>> +       .main_clk       = "cam_ick",
> >>> 
> >>> It's not "cam_fck"?
> >> 
> >> AFAIK cam_fck doesn't exist in the code, and CAM_L3_ICK is used as
> >> both ick/fck according to TRM.
> > 
> > Then maybe the code is wrong.
> > 
> > Look at the OMAP34xx documentation, it says that:
> > 
> > CAM_L3_ICLK -> CAM_FCLK
> > CAM_L4_ICLK -> CAM_ICLK
> > CAM_MCLK -> CAM_MCLK
> > CSI2_96M_FCLK -> CSI2_96M_FCLK
> > 
> > CAM_FCLK
> > Functional clock (L3 interconnect clock domain)
> > Functional clock domain.
> > 
> > CAM_ICLK
> > Interface clock (L4 interconnect clock domain)
> > Interface clock domain.
> 
> "The camera subsystem interface is clocked with the L3 and L4 clocks
> (CAM_L3_ICLK and CAM_L4_ICLK, respectively). CAM_L3_ICLK is also used
> as the main functional clock. The functional clock (CAM_MCLK) is
> provided by DPLL4 to supply the external sensor."
> 
> Either CAM subsystem section or PRCM - CAM PWRDM is ambiguous or wrong.

I haven't checked how the clocks are used in details (I started working on the 
OMAP3 ISP driver after the clock handling code was in place), but I can at 
least confirm that CAM_L3_ICLK is the main functional clock. I would be a bit 
surprised if CAM_L3_ICLK was used as the interface clock as well.

Once again, you should be able to find clarification on this subject inside 
TI.

> ...
> 
> > Looks like the driver is manually calling clk_get() and clk_put() for
> > the "i3_ick", where I guess the clock framework is supposed to do
> > that... It's almost as if somebody forgot a dependency somewhere.
> 
> Would be good to clarify the intentions to keep the code as it is.

Once proper clock dependencies will be in place I'll be glad to remove the 
direct l3_ick if needed.

> >>>> +       .dev_attr       = &isp_mmu_dev_attr,
> >>>> +       .slaves         = omap3xxx_isp_mmu_slaves,
> >>>> +       .slaves_cnt     = ARRAY_SIZE(omap3xxx_isp_mmu_slaves),
> >>>> +       .flags          = HWMOD_NO_IDLEST,
> >>>> +};
> >>> 
> >>> Most of the stuff I see the hwmods is .main_lock = "foo_fck", slave
> >>> .clk = "foo_ick". Maybe that explains the irq issues you get.
> >> 
> >> I see irq issues with iva hwmod because tidspbridge doesn't use iommu
> >> API yet, so if you enable both the mmu hwmod and tidspbridge own mmu
> >> implementation there will be some conflicts.
> >> 
> >> I didn't see isp issues though, but I didn't went more than
> >> booting/enabling with isp mmu.
> > 
> > This is what you said:
> > Removed clk handling during interrupt, given that in order to receive
> > one, the device should be powered on in advance.
> 
> Yes, you should receive an interrupt if the clock is enabled and the
> iommu is being used, hence the part inside in the ISR to enable the
> clocks was removed.
> 
> > I'm not sure how this clock stuff works, but I'm guessing the device
> > is supposed to go to sleep at some points in time, and with your patch
> > "OMAP3/4: iommu: adapt to runtime pm" it won't, as long as the module
> > is loaded, unless I'm missing something.
> 
> The device should be able to be put to sleep at anytime when it is NOT
> being used. AFAIK there is no mechanism for the main processor (the
> one running the kernel) to know when the other iommus are not being
> used, given that they are in independent processors/subsystems, at
> least for the ones in the DSP or M3 processors. Once the user releases
> its iommu resource it means it is no longer using it, at that point
> the device can be put to sleep.

How should the OMAP3 ISP driver proceed to make sure that its IOMMU is clocked 
off when it doesn't need it ?

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
  2011-12-25  0:03               ` Ramirez Luna, Omar
@ 2011-12-27  9:41                 ` Felipe Contreras
  -1 siblings, 0 replies; 48+ messages in thread
From: Felipe Contreras @ 2011-12-27  9:41 UTC (permalink / raw)
  To: Ramirez Luna, Omar
  Cc: Tony Lindgren, Benoit Cousson, Kevin Hilman, Ohad Ben-Cohen,
	Russell King, Linux PM, Rafael J. Wysocki, iommu, MyungJoo Ham,
	Laurent Pinchart, linux-omap, linux-arm-kernel, Paul Walmsley

On Sun, Dec 25, 2011 at 2:03 AM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
> On Fri, Dec 23, 2011 at 11:04 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> Which omap-iommu? The platform driver, or the device stuff? The device
>> stuff is always built-in, but not the platform driver
>> (drivers/iommu/omap-iommu.c), that can be a module.
>
> Both, I can't recall exactly when it changed (prior to being moved to
> drivers/iommu it could be built as a module), but now
> CONFIG_OMAP_IOMMU is boolean type.

I see. Still, it looks like the proper way to use the API is to call
_enable() on the platform driver.

-- 
Felipe Contreras

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

* [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
@ 2011-12-27  9:41                 ` Felipe Contreras
  0 siblings, 0 replies; 48+ messages in thread
From: Felipe Contreras @ 2011-12-27  9:41 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, Dec 25, 2011 at 2:03 AM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
> On Fri, Dec 23, 2011 at 11:04 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> Which omap-iommu? The platform driver, or the device stuff? The device
>> stuff is always built-in, but not the platform driver
>> (drivers/iommu/omap-iommu.c), that can be a module.
>
> Both, I can't recall exactly when it changed (prior to being moved to
> drivers/iommu it could be built as a module), but now
> CONFIG_OMAP_IOMMU is boolean type.

I see. Still, it looks like the proper way to use the API is to call
_enable() on the platform driver.

-- 
Felipe Contreras

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

* Re: [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
  2011-12-27  9:41                 ` Felipe Contreras
@ 2012-01-05 18:26                   ` Ramirez Luna, Omar
  -1 siblings, 0 replies; 48+ messages in thread
From: Ramirez Luna, Omar @ 2012-01-05 18:26 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Tony Lindgren, Benoit Cousson, Kevin Hilman, Ohad Ben-Cohen,
	Russell King, Linux PM, Rafael J. Wysocki, iommu, MyungJoo Ham,
	Laurent Pinchart, linux-omap, linux-arm-kernel, Paul Walmsley

On Tue, Dec 27, 2011 at 3:41 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Sun, Dec 25, 2011 at 2:03 AM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
>> On Fri, Dec 23, 2011 at 11:04 AM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>>> Which omap-iommu? The platform driver, or the device stuff? The device
>>> stuff is always built-in, but not the platform driver
>>> (drivers/iommu/omap-iommu.c), that can be a module.
>>
>> Both, I can't recall exactly when it changed (prior to being moved to
>> drivers/iommu it could be built as a module), but now
>> CONFIG_OMAP_IOMMU is boolean type.
>
> I see. Still, it looks like the proper way to use the API is to call
> _enable() on the platform driver.

I'm sorry, I don't seem to follow... you want _enable in platform
driver, and you call drivers/iommu/omap-iommu.c the platform driver...

And that is precisely where pm_runtime_enable() is, no?

Regards,

Omar

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

* [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
@ 2012-01-05 18:26                   ` Ramirez Luna, Omar
  0 siblings, 0 replies; 48+ messages in thread
From: Ramirez Luna, Omar @ 2012-01-05 18:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Dec 27, 2011 at 3:41 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Sun, Dec 25, 2011 at 2:03 AM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
>> On Fri, Dec 23, 2011 at 11:04 AM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>>> Which omap-iommu? The platform driver, or the device stuff? The device
>>> stuff is always built-in, but not the platform driver
>>> (drivers/iommu/omap-iommu.c), that can be a module.
>>
>> Both, I can't recall exactly when it changed (prior to being moved to
>> drivers/iommu it could be built as a module), but now
>> CONFIG_OMAP_IOMMU is boolean type.
>
> I see. Still, it looks like the proper way to use the API is to call
> _enable() on the platform driver.

I'm sorry, I don't seem to follow... you want _enable in platform
driver, and you call drivers/iommu/omap-iommu.c the platform driver...

And that is precisely where pm_runtime_enable() is, no?

Regards,

Omar

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

* Re: [PATCH v4 1/4] OMAP3: hwmod data: add mmu data for iva and isp
  2011-12-25 21:08             ` Laurent Pinchart
@ 2012-01-05 19:24               ` Ramirez Luna, Omar
  -1 siblings, 0 replies; 48+ messages in thread
From: Ramirez Luna, Omar @ 2012-01-05 19:24 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Felipe Contreras, Tony Lindgren, Benoit Cousson, Kevin Hilman,
	Ohad Ben-Cohen, Russell King, Linux PM, Rafael J. Wysocki, iommu,
	MyungJoo Ham, linux-omap, linux-arm-kernel

Hi Laurent

On Sun, Dec 25, 2011 at 3:08 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> > I'm not sure how this clock stuff works, but I'm guessing the device
>> > is supposed to go to sleep at some points in time, and with your patch
>> > "OMAP3/4: iommu: adapt to runtime pm" it won't, as long as the module
>> > is loaded, unless I'm missing something.
>>
>> The device should be able to be put to sleep at anytime when it is NOT
>> being used. AFAIK there is no mechanism for the main processor (the
>> one running the kernel) to know when the other iommus are not being
>> used, given that they are in independent processors/subsystems, at
>> least for the ones in the DSP or M3 processors. Once the user releases
>> its iommu resource it means it is no longer using it, at that point
>> the device can be put to sleep.
>
> How should the OMAP3 ISP driver proceed to make sure that its IOMMU is clocked
> off when it doesn't need it ?

If there is an specific scenario where the iommu should be disabled,
iommu_detach_device can be called to release the iommu resource. On
suspend/resume scenarios runtime pm callbacks should still be able to
put the device in idle.

Regards,

Omar

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

* [PATCH v4 1/4] OMAP3: hwmod data: add mmu data for iva and isp
@ 2012-01-05 19:24               ` Ramirez Luna, Omar
  0 siblings, 0 replies; 48+ messages in thread
From: Ramirez Luna, Omar @ 2012-01-05 19:24 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Laurent

On Sun, Dec 25, 2011 at 3:08 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> > I'm not sure how this clock stuff works, but I'm guessing the device
>> > is supposed to go to sleep at some points in time, and with your patch
>> > "OMAP3/4: iommu: adapt to runtime pm" it won't, as long as the module
>> > is loaded, unless I'm missing something.
>>
>> The device should be able to be put to sleep at anytime when it is NOT
>> being used. AFAIK there is no mechanism for the main processor (the
>> one running the kernel) to know when the other iommus are not being
>> used, given that they are in independent processors/subsystems, at
>> least for the ones in the DSP or M3 processors. Once the user releases
>> its iommu resource it means it is no longer using it, at that point
>> the device can be put to sleep.
>
> How should the OMAP3 ISP driver proceed to make sure that its IOMMU is clocked
> off when it doesn't need it ?

If there is an specific scenario where the iommu should be disabled,
iommu_detach_device can be called to release the iommu resource. On
suspend/resume scenarios runtime pm callbacks should still be able to
put the device in idle.

Regards,

Omar

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

* Re: [PATCH v4 1/4] OMAP3: hwmod data: add mmu data for iva and isp
  2012-01-05 19:24               ` Ramirez Luna, Omar
@ 2012-01-07 16:12                 ` Laurent Pinchart
  -1 siblings, 0 replies; 48+ messages in thread
From: Laurent Pinchart @ 2012-01-07 16:12 UTC (permalink / raw)
  To: Ramirez Luna, Omar
  Cc: Felipe Contreras, Tony Lindgren, Benoit Cousson, Kevin Hilman,
	Ohad Ben-Cohen, Russell King, Linux PM, Rafael J. Wysocki, iommu,
	MyungJoo Ham, linux-omap, linux-arm-kernel

Hi Omar,

On Thursday 05 January 2012 20:24:25 Ramirez Luna, Omar wrote:
> On Sun, Dec 25, 2011 at 3:08 PM, Laurent Pinchart wrote:
> >> > I'm not sure how this clock stuff works, but I'm guessing the device
> >> > is supposed to go to sleep at some points in time, and with your patch
> >> > "OMAP3/4: iommu: adapt to runtime pm" it won't, as long as the module
> >> > is loaded, unless I'm missing something.
> >> 
> >> The device should be able to be put to sleep at anytime when it is NOT
> >> being used. AFAIK there is no mechanism for the main processor (the
> >> one running the kernel) to know when the other iommus are not being
> >> used, given that they are in independent processors/subsystems, at
> >> least for the ones in the DSP or M3 processors. Once the user releases
> >> its iommu resource it means it is no longer using it, at that point
> >> the device can be put to sleep.
> > 
> > How should the OMAP3 ISP driver proceed to make sure that its IOMMU is
> > clocked off when it doesn't need it ?
> 
> If there is an specific scenario where the iommu should be disabled,
> iommu_detach_device can be called to release the iommu resource. On
> suspend/resume scenarios runtime pm callbacks should still be able to
> put the device in idle.

Runtime PM might indeed be a better option. The OMAP3 ISP doesn't need to 
IOMMU until video streams are started, so we should keep it powered down in 
that case.

-- 
Regards,

Laurent Pinchart

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

* [PATCH v4 1/4] OMAP3: hwmod data: add mmu data for iva and isp
@ 2012-01-07 16:12                 ` Laurent Pinchart
  0 siblings, 0 replies; 48+ messages in thread
From: Laurent Pinchart @ 2012-01-07 16:12 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Omar,

On Thursday 05 January 2012 20:24:25 Ramirez Luna, Omar wrote:
> On Sun, Dec 25, 2011 at 3:08 PM, Laurent Pinchart wrote:
> >> > I'm not sure how this clock stuff works, but I'm guessing the device
> >> > is supposed to go to sleep at some points in time, and with your patch
> >> > "OMAP3/4: iommu: adapt to runtime pm" it won't, as long as the module
> >> > is loaded, unless I'm missing something.
> >> 
> >> The device should be able to be put to sleep at anytime when it is NOT
> >> being used. AFAIK there is no mechanism for the main processor (the
> >> one running the kernel) to know when the other iommus are not being
> >> used, given that they are in independent processors/subsystems, at
> >> least for the ones in the DSP or M3 processors. Once the user releases
> >> its iommu resource it means it is no longer using it, at that point
> >> the device can be put to sleep.
> > 
> > How should the OMAP3 ISP driver proceed to make sure that its IOMMU is
> > clocked off when it doesn't need it ?
> 
> If there is an specific scenario where the iommu should be disabled,
> iommu_detach_device can be called to release the iommu resource. On
> suspend/resume scenarios runtime pm callbacks should still be able to
> put the device in idle.

Runtime PM might indeed be a better option. The OMAP3 ISP doesn't need to 
IOMMU until video streams are started, so we should keep it powered down in 
that case.

-- 
Regards,

Laurent Pinchart

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

end of thread, other threads:[~2012-01-07 16:12 UTC | newest]

Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-15  4:18 [PATCH v4 0/4] OMAP: iommu: hwmod support and runtime PM Omar Ramirez Luna
2011-12-15  4:18 ` Omar Ramirez Luna
2011-12-15  4:18 ` [PATCH v4 1/4] OMAP3: hwmod data: add mmu data for iva and isp Omar Ramirez Luna
2011-12-15  4:18   ` Omar Ramirez Luna
2011-12-16  0:39   ` Felipe Contreras
2011-12-16  0:39     ` Felipe Contreras
2011-12-16  2:01     ` Ramirez Luna, Omar
2011-12-16  2:01       ` Ramirez Luna, Omar
2011-12-19 16:11       ` Felipe Contreras
2011-12-19 16:11         ` Felipe Contreras
2011-12-23 15:53         ` Ramirez Luna, Omar
2011-12-23 15:53           ` Ramirez Luna, Omar
2011-12-25 21:08           ` Laurent Pinchart
2011-12-25 21:08             ` Laurent Pinchart
2012-01-05 19:24             ` Ramirez Luna, Omar
2012-01-05 19:24               ` Ramirez Luna, Omar
2012-01-07 16:12               ` Laurent Pinchart
2012-01-07 16:12                 ` Laurent Pinchart
2011-12-15  4:18 ` [PATCH v4 2/4] OMAP4: hwmod data: add mmu hwmod for ipu and dsp Omar Ramirez Luna
2011-12-15  4:18   ` Omar Ramirez Luna
2011-12-15  4:18 ` [PATCH v4 3/4] OMAP3/4: iommu: migrate to hwmod framework Omar Ramirez Luna
2011-12-15  4:18   ` Omar Ramirez Luna
2011-12-16  0:47   ` Felipe Contreras
2011-12-16  0:47     ` Felipe Contreras
2011-12-15  4:18 ` [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm Omar Ramirez Luna
2011-12-15  4:18   ` Omar Ramirez Luna
2011-12-16  0:33   ` Felipe Contreras
2011-12-16  0:33     ` Felipe Contreras
2011-12-16  2:59     ` Ramirez Luna, Omar
2011-12-16  2:59       ` Ramirez Luna, Omar
2011-12-16  0:53   ` Felipe Contreras
2011-12-16  0:53     ` Felipe Contreras
2011-12-16  3:18     ` Ramirez Luna, Omar
2011-12-16  3:18       ` Ramirez Luna, Omar
2011-12-19 16:27       ` Felipe Contreras
2011-12-19 16:27         ` Felipe Contreras
2011-12-23 16:30         ` Ramirez Luna, Omar
2011-12-23 16:30           ` Ramirez Luna, Omar
2011-12-23 17:04           ` Felipe Contreras
2011-12-23 17:04             ` Felipe Contreras
2011-12-25  0:03             ` Ramirez Luna, Omar
2011-12-25  0:03               ` Ramirez Luna, Omar
2011-12-27  9:41               ` Felipe Contreras
2011-12-27  9:41                 ` Felipe Contreras
2012-01-05 18:26                 ` Ramirez Luna, Omar
2012-01-05 18:26                   ` Ramirez Luna, Omar
2011-12-17  1:39   ` Tony Lindgren
2011-12-17  1:39     ` Tony Lindgren

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.