[RFC/RFT v2 0/4] PRCM interrupt handler updates/fixes/cleanups

[RFC/RFT v2 0/4] PRCM interrupt handler updates/fixes/cleanups

[Kevin Hilman]

This series incoporates various changes/updates/fixes to the PRCM
interrupt handler.  Applies to current PM branch.

These could use some extra review and especially some testing.

I did some basic testing on 3430SDP (es3.0) and Zoom2 (es3.1)

Updates from previous series: I added a reworked version of
Vikram's USBHOST fix.

Kevin


Jon Hunter (1):
  OMAP3: PM: Prevent hang in prcm_interrupt_handler

Paul Walmsley (2):
  OMAP3: PM: PRCM interrupt: check MPUGRPSEL register
  OMAP3: PM: PRCM interrupt: only handle selected PRCM interrupts

Vikram Pandita (1):
  OMAP3: PM: USBHOST: clear wakeup events on both hosts

 arch/arm/mach-omap2/pm34xx.c |  177 +++++++++++++++++++++++-------------------
 1 files changed, 96 insertions(+), 81 deletions(-)

[RFC/RFT 1/4] OMAP3: PM: Prevent hang in prcm_interrupt_handler

[Kevin Hilman]

From: Jon Hunter <jon-hunter@ti.com>

There are two scenarios where a race condition could result in a hang
in the prcm_interrupt handler. These are:

1). Waiting for PRM_IRQSTATUS_MPU register to clear.
Bit 0 of the PRM_IRQSTATUS_MPU register indicates that a wake-up event
is pending for the MPU. This bit can only be cleared if the all the
wake-up events latched in the various PM_WKST_x registers have been
cleared. If a wake-up event occurred during the processing of the prcm
interrupt handler, after the corresponding PM_WKST_x register was
checked but before the PRM_IRQSTATUS_MPU was cleared, then the CPU
would be stuck forever waiting for bit 0 in PRM_IRQSTATUS_MPU to be
cleared.

2). Waiting for the PM_WKST_x register to clear.
Some power domains have more than one wake-up source. The PM_WKST_x
registers indicate the source of a wake-up event and need to be cleared
after a wake-up event occurs. When the PM_WKST_x registers are read and
before they are cleared, it is possible that another wake-up event
could occur causing another bit to be set in one of the PM_WKST_x
registers. If this did occur after reading a PM_WKST_x register then
the CPU would miss this event and get stuck forever in a loop waiting
for that PM_WKST_x register to clear.

This patch address the above race conditions that would result in a
hang.

Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 arch/arm/mach-omap2/pm34xx.c |  143 ++++++++++++++++++------------------------
 1 files changed, 60 insertions(+), 83 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 14f10bc..f6cc71a 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -194,97 +194,74 @@ static void omap3_save_secure_ram_context(u32 target_mpu_state)
 	}
 }
 
-/* PRCM Interrupt Handler for wakeups */
-static irqreturn_t prcm_interrupt_handler (int irq, void *dev_id)
+/*
+ * PRCM Interrupt Handler Helper Function
+ *
+ * The purpose of this function is to clear any wake-up events latched
+ * in the PRCM PM_WKST_x registers. It is possible that a wake-up event
+ * may occur whilst attempting to clear a PM_WKST_x register and thus
+ * set another bit in this register. A while loop is used to ensure
+ * that any peripheral wake-up events occurring while attempting to
+ * clear the PM_WKST_x are detected and cleared.
+ */
+static void prcm_clear_mod_irqs(s16 module, u8 regs)
 {
-	u32 wkst, irqstatus_mpu;
-	u32 fclk, iclk;
-
-	/* WKUP */
-	wkst = prm_read_mod_reg(WKUP_MOD, PM_WKST);
-	if (wkst) {
-		iclk = cm_read_mod_reg(WKUP_MOD, CM_ICLKEN);
-		fclk = cm_read_mod_reg(WKUP_MOD, CM_FCLKEN);
-		cm_set_mod_reg_bits(wkst, WKUP_MOD, CM_ICLKEN);
-		cm_set_mod_reg_bits(wkst, WKUP_MOD, CM_FCLKEN);
-		prm_write_mod_reg(wkst, WKUP_MOD, PM_WKST);
-		while (prm_read_mod_reg(WKUP_MOD, PM_WKST))
-			cpu_relax();
-		cm_write_mod_reg(iclk, WKUP_MOD, CM_ICLKEN);
-		cm_write_mod_reg(fclk, WKUP_MOD, CM_FCLKEN);
-	}
+	u32 wkst, fclk, iclk;
+	u16 wkst_off = (regs == 3) ? OMAP3430ES2_PM_WKST3 : PM_WKST1;
+	u16 fclk_off = (regs == 3) ? OMAP3430ES2_CM_FCLKEN3 : CM_FCLKEN1;
+	u16 iclk_off = (regs == 3) ? CM_ICLKEN3 : CM_ICLKEN1;
 
-	/* CORE */
-	wkst = prm_read_mod_reg(CORE_MOD, PM_WKST1);
-	if (wkst) {
-		iclk = cm_read_mod_reg(CORE_MOD, CM_ICLKEN1);
-		fclk = cm_read_mod_reg(CORE_MOD, CM_FCLKEN1);
-		cm_set_mod_reg_bits(wkst, CORE_MOD, CM_ICLKEN1);
-		cm_set_mod_reg_bits(wkst, CORE_MOD, CM_FCLKEN1);
-		prm_write_mod_reg(wkst, CORE_MOD, PM_WKST1);
-		while (prm_read_mod_reg(CORE_MOD, PM_WKST1))
-			cpu_relax();
-		cm_write_mod_reg(iclk, CORE_MOD, CM_ICLKEN1);
-		cm_write_mod_reg(fclk, CORE_MOD, CM_FCLKEN1);
-	}
-	wkst = prm_read_mod_reg(CORE_MOD, OMAP3430ES2_PM_WKST3);
+	wkst = prm_read_mod_reg(module, wkst_off);
 	if (wkst) {
-		iclk = cm_read_mod_reg(CORE_MOD, CM_ICLKEN3);
-		fclk = cm_read_mod_reg(CORE_MOD, OMAP3430ES2_CM_FCLKEN3);
-		cm_set_mod_reg_bits(wkst, CORE_MOD, CM_ICLKEN3);
-		cm_set_mod_reg_bits(wkst, CORE_MOD, OMAP3430ES2_CM_FCLKEN3);
-		prm_write_mod_reg(wkst, CORE_MOD, OMAP3430ES2_PM_WKST3);
-		while (prm_read_mod_reg(CORE_MOD, OMAP3430ES2_PM_WKST3))
-			cpu_relax();
-		cm_write_mod_reg(iclk, CORE_MOD, CM_ICLKEN3);
-		cm_write_mod_reg(fclk, CORE_MOD, OMAP3430ES2_CM_FCLKEN3);
-	}
-
-	/* PER */
-	wkst = prm_read_mod_reg(OMAP3430_PER_MOD, PM_WKST);
-	if (wkst) {
-		iclk = cm_read_mod_reg(OMAP3430_PER_MOD, CM_ICLKEN);
-		fclk = cm_read_mod_reg(OMAP3430_PER_MOD, CM_FCLKEN);
-		cm_set_mod_reg_bits(wkst, OMAP3430_PER_MOD, CM_ICLKEN);
-		cm_set_mod_reg_bits(wkst, OMAP3430_PER_MOD, CM_FCLKEN);
-		prm_write_mod_reg(wkst, OMAP3430_PER_MOD, PM_WKST);
-		while (prm_read_mod_reg(OMAP3430_PER_MOD, PM_WKST))
-			cpu_relax();
-		cm_write_mod_reg(iclk, OMAP3430_PER_MOD, CM_ICLKEN);
-		cm_write_mod_reg(fclk, OMAP3430_PER_MOD, CM_FCLKEN);
+		iclk = cm_read_mod_reg(module, iclk_off);
+		fclk = cm_read_mod_reg(module, fclk_off);
+		while (wkst) {
+			cm_set_mod_reg_bits(wkst, module, iclk_off);
+			cm_set_mod_reg_bits(wkst, module, fclk_off);
+			prm_write_mod_reg(wkst, module, wkst_off);
+			wkst = prm_read_mod_reg(module, wkst_off);
+		}
+		cm_write_mod_reg(iclk, module, iclk_off);
+		cm_write_mod_reg(fclk, module, fclk_off);
 	}
+}
 
-	if (omap_rev() > OMAP3430_REV_ES1_0) {
-		/* USBHOST */
-		wkst = prm_read_mod_reg(OMAP3430ES2_USBHOST_MOD, PM_WKST);
-		if (wkst) {
-			iclk = cm_read_mod_reg(OMAP3430ES2_USBHOST_MOD,
-					       CM_ICLKEN);
-			fclk = cm_read_mod_reg(OMAP3430ES2_USBHOST_MOD,
-					       CM_FCLKEN);
-			cm_set_mod_reg_bits(wkst, OMAP3430ES2_USBHOST_MOD,
-					    CM_ICLKEN);
-			cm_set_mod_reg_bits(wkst, OMAP3430ES2_USBHOST_MOD,
-					    CM_FCLKEN);
-			prm_write_mod_reg(wkst, OMAP3430ES2_USBHOST_MOD,
-					  PM_WKST);
-			while (prm_read_mod_reg(OMAP3430ES2_USBHOST_MOD,
-						PM_WKST))
-				cpu_relax();
-			cm_write_mod_reg(iclk, OMAP3430ES2_USBHOST_MOD,
-					 CM_ICLKEN);
-			cm_write_mod_reg(fclk, OMAP3430ES2_USBHOST_MOD,
-					 CM_FCLKEN);
+/*
+ * PRCM Interrupt Handler
+ *
+ * The PRM_IRQSTATUS_MPU register indicates if there are any pending
+ * interrupts from the PRCM for the MPU. These bits must be cleared in
+ * order to clear the PRCM interrupt. The PRCM interrupt handler is
+ * implemented to simply clear the PRM_IRQSTATUS_MPU in order to clear
+ * the PRCM interrupt. Please note that bit 0 of the PRM_IRQSTATUS_MPU
+ * register indicates that a wake-up event is pending for the MPU and
+ * this bit can only be cleared if the all the wake-up events latched
+ * in the various PM_WKST_x registers have been cleared. The interrupt
+ * handler is implemented using a do-while loop so that if a wake-up
+ * event occurred during the processing of the prcm interrupt handler
+ * (setting a bit in the corresponding PM_WKST_x register and thus
+ * preventing us from clearing bit 0 of the PRM_IRQSTATUS_MPU register)
+ * this would be handled.
+ */
+static irqreturn_t prcm_interrupt_handler (int irq, void *dev_id)
+{
+	u32 irqstatus_mpu;
+
+	do {
+		prcm_clear_mod_irqs(WKUP_MOD, 1);
+		prcm_clear_mod_irqs(CORE_MOD, 1);
+		prcm_clear_mod_irqs(OMAP3430_PER_MOD, 1);
+		if (omap_rev() > OMAP3430_REV_ES1_0) {
+			prcm_clear_mod_irqs(CORE_MOD, 3);
+			prcm_clear_mod_irqs(OMAP3430ES2_USBHOST_MOD, 1);
 		}
-	}
 
-	irqstatus_mpu = prm_read_mod_reg(OCP_MOD,
-					 OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
-	prm_write_mod_reg(irqstatus_mpu, OCP_MOD,
-			  OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
+		irqstatus_mpu = prm_read_mod_reg(OCP_MOD,
+					OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
+		prm_write_mod_reg(irqstatus_mpu, OCP_MOD,
+					OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
 
-	while (prm_read_mod_reg(OCP_MOD, OMAP3_PRM_IRQSTATUS_MPU_OFFSET))
-		cpu_relax();
+	} while (prm_read_mod_reg(OCP_MOD, OMAP3_PRM_IRQSTATUS_MPU_OFFSET));
 
 	return IRQ_HANDLED;
 }
-- 
1.6.3.3

[RFC/RFT 2/4] OMAP3: PM: PRCM interrupt: check MPUGRPSEL register

[Kevin Hilman]

From: Paul Walmsley <paul@pwsan.com>

PM_WKST register contents should be ANDed with the contents of the
MPUGRPSEL registers.  Otherwise the MPU PRCM interrupt handler could
wind up clearing wakeup events meant for the IVA PRCM interrupt
handler.  For a production version of this patch, we should not read
MPUGRPSEL from the PRM, since those reads are very slow; rather, we
should use a cached version from struct powerdomain (not yet
implemented)
---
 arch/arm/mach-omap2/pm34xx.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index f6cc71a..25372b7 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -210,8 +210,11 @@ static void prcm_clear_mod_irqs(s16 module, u8 regs)
 	u16 wkst_off = (regs == 3) ? OMAP3430ES2_PM_WKST3 : PM_WKST1;
 	u16 fclk_off = (regs == 3) ? OMAP3430ES2_CM_FCLKEN3 : CM_FCLKEN1;
 	u16 iclk_off = (regs == 3) ? CM_ICLKEN3 : CM_ICLKEN1;
+	u16 grpsel_off = (regs == 3) ?
+		OMAP3430ES2_PM_MPUGRPSEL3 : OMAP3430_PM_MPUGRPSEL;
 
 	wkst = prm_read_mod_reg(module, wkst_off);
+	wkst &= prm_read_mod_reg(module, grpsel_off);
 	if (wkst) {
 		iclk = cm_read_mod_reg(module, iclk_off);
 		fclk = cm_read_mod_reg(module, fclk_off);
-- 
1.6.3.3

[RFC/RFT 3/4] OMAP3: PM: PRCM interrupt: only handle selected PRCM interrupts

[Kevin Hilman]

From: Paul Walmsley <paul@pwsan.com>

Clearing wakeup sources is now only done when the PRM indicates a
wakeup source interrupt.  Since we don't handle any other types of
PRCM interrupts right now, warn if we get any other type of PRCM
interrupt.  Either code needs to be added to the PRCM interrupt
handler to react to these, or these other interrupts should be masked
off at init.
---
 arch/arm/mach-omap2/pm34xx.c |   46 +++++++++++++++++++++++++++++++++--------
 1 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 25372b7..348a683 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -204,7 +204,7 @@ static void omap3_save_secure_ram_context(u32 target_mpu_state)
  * that any peripheral wake-up events occurring while attempting to
  * clear the PM_WKST_x are detected and cleared.
  */
-static void prcm_clear_mod_irqs(s16 module, u8 regs)
+static int prcm_clear_mod_irqs(s16 module, u8 regs)
 {
 	u32 wkst, fclk, iclk;
 	u16 wkst_off = (regs == 3) ? OMAP3430ES2_PM_WKST3 : PM_WKST1;
@@ -212,6 +212,7 @@ static void prcm_clear_mod_irqs(s16 module, u8 regs)
 	u16 iclk_off = (regs == 3) ? CM_ICLKEN3 : CM_ICLKEN1;
 	u16 grpsel_off = (regs == 3) ?
 		OMAP3430ES2_PM_MPUGRPSEL3 : OMAP3430_PM_MPUGRPSEL;
+	int c = 0;
 
 	wkst = prm_read_mod_reg(module, wkst_off);
 	wkst &= prm_read_mod_reg(module, grpsel_off);
@@ -223,10 +224,28 @@ static void prcm_clear_mod_irqs(s16 module, u8 regs)
 			cm_set_mod_reg_bits(wkst, module, fclk_off);
 			prm_write_mod_reg(wkst, module, wkst_off);
 			wkst = prm_read_mod_reg(module, wkst_off);
+			c++;
 		}
 		cm_write_mod_reg(iclk, module, iclk_off);
 		cm_write_mod_reg(fclk, module, fclk_off);
 	}
+
+	return c;
+}
+
+static int _prcm_int_handle_wakeup(void)
+{
+	int c;
+
+	c = prcm_clear_mod_irqs(WKUP_MOD, 1);
+	c += prcm_clear_mod_irqs(CORE_MOD, 1);
+	c += prcm_clear_mod_irqs(OMAP3430_PER_MOD, 1);
+	if (omap_rev() > OMAP3430_REV_ES1_0) {
+		c += prcm_clear_mod_irqs(CORE_MOD, 3);
+		c += prcm_clear_mod_irqs(OMAP3430ES2_USBHOST_MOD, 1);
+	}
+
+	return c;
 }
 
 /*
@@ -249,18 +268,27 @@ static void prcm_clear_mod_irqs(s16 module, u8 regs)
 static irqreturn_t prcm_interrupt_handler (int irq, void *dev_id)
 {
 	u32 irqstatus_mpu;
+	int c = 0;
 
 	do {
-		prcm_clear_mod_irqs(WKUP_MOD, 1);
-		prcm_clear_mod_irqs(CORE_MOD, 1);
-		prcm_clear_mod_irqs(OMAP3430_PER_MOD, 1);
-		if (omap_rev() > OMAP3430_REV_ES1_0) {
-			prcm_clear_mod_irqs(CORE_MOD, 3);
-			prcm_clear_mod_irqs(OMAP3430ES2_USBHOST_MOD, 1);
-		}
-
 		irqstatus_mpu = prm_read_mod_reg(OCP_MOD,
 					OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
+
+		if (irqstatus_mpu & (OMAP3430_WKUP_ST | OMAP3430_IO_ST)) {
+			c = _prcm_int_handle_wakeup();
+
+			/*
+			 * Is the MPU PRCM interrupt handler racing with the
+			 * IVA2 PRCM interrupt handler ?
+			 */
+			WARN(c == 0, "prcm: WARNING: PRCM indicated MPU wakeup "
+			     "but no wakeup sources are marked\n");
+		} else {
+			/* XXX we need to expand our PRCM interrupt handler */
+			WARN(1, "prcm: WARNING: PRCM interrupt received, but "
+			     "no code to handle it (%08x)\n", irqstatus_mpu);
+		}
+
 		prm_write_mod_reg(irqstatus_mpu, OCP_MOD,
 					OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
 
-- 
1.6.3.3

[RFC/RFT 4/4] OMAP3: PM: USBHOST: clear wakeup events on both hosts

[Kevin Hilman]

From: Vikram Pandita <vikram.pandita@ti.com>

USBHOST module has 2 fclocks (for HOST1 and HOST2), only one iclock
and only a single bit in the WKST register to indicate a wakeup event.

Because of the single WKST bit, we cannot know whether a wakeup event
was on HOST1 or HOST2, so enable both fclocks before clearing the
wakeup event to ensure both hosts can properly clear the event.

Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
---
 arch/arm/mach-omap2/pm34xx.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 348a683..9ce651a 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -206,7 +206,7 @@ static void omap3_save_secure_ram_context(u32 target_mpu_state)
  */
 static int prcm_clear_mod_irqs(s16 module, u8 regs)
 {
-	u32 wkst, fclk, iclk;
+	u32 wkst, fclk, iclk, clken;
 	u16 wkst_off = (regs == 3) ? OMAP3430ES2_PM_WKST3 : PM_WKST1;
 	u16 fclk_off = (regs == 3) ? OMAP3430ES2_CM_FCLKEN3 : CM_FCLKEN1;
 	u16 iclk_off = (regs == 3) ? CM_ICLKEN3 : CM_ICLKEN1;
@@ -220,8 +220,15 @@ static int prcm_clear_mod_irqs(s16 module, u8 regs)
 		iclk = cm_read_mod_reg(module, iclk_off);
 		fclk = cm_read_mod_reg(module, fclk_off);
 		while (wkst) {
-			cm_set_mod_reg_bits(wkst, module, iclk_off);
-			cm_set_mod_reg_bits(wkst, module, fclk_off);
+			clken = wkst;
+			cm_set_mod_reg_bits(clken, module, iclk_off);
+			/*
+			 * For USBHOST, we don't know whether HOST1 or
+			 * HOST2 woke us up, so enable both f-clocks
+			 */
+			if (module == OMAP3430ES2_USBHOST_MOD)
+				clken |= 1 << OMAP3430ES2_EN_USBHOST2_SHIFT;
+			cm_set_mod_reg_bits(clken, module, fclk_off);
 			prm_write_mod_reg(wkst, module, wkst_off);
 			wkst = prm_read_mod_reg(module, wkst_off);
 			c++;
-- 
1.6.3.3

Question about tput constraint on zoom2 camera

[Curran, Dominic]

Hi Kevin

I have been testing the zoom2 camera streaming while using different OPP's.
Following table provides summary of what OPP's caused to happen:

  Streaming      Vdd1(OPP)        Vdd2(OPP)           P/F
 VGA @ 30fps           1                2             Pass
 8MP @ 7.5fps          1                2             Fails (stop streaming)
 8MP @ 7.5fps          1                3             Pass

So table shows that locking Vdd2 to OPP=3 when streaming 8MPixel works, but at OPP=2 then streaming fails (stops).

So I thought the tput constraint made the most sense for camera.
The Zoom2 camera sensor has a max tput of:

    3280 x 2464 x 2bpp x 7.5fps = 121228800 bytes/s
                                = 118387 KB/s

However, this calculated value doesn't constrain Vdd2 to OPP3 (DVFS enabled).

Experimentation shows that a tput value of 350000 KB/s is required to constrain Vdd2 to OPP=3.

Can you explain why the practical tput constraint is so much greater than the theoretical value ?

Thanks
dom

NOTE: Following patch was created against 2.6.29 APG  kernel.




From: Dominic Curran <dcurran@ti.com>
Subject: ZOOM2: Add OMAP3 power constraints for Sony camera.

Adds power constraints for the Zoom2 camera sensor (Sony IMX046).
If no constraint was applied then ISP stopped working if Vdd2 went
to OPP2 when streaming 8MPixel at 7.5fps.

Thus Vdd2 needs to be kept at OPP3 while streaming with this sensor.

In theory the following throughput constraint should be sufficient:

  3280 x 2464 x 2bpp x 7.5fps = 118387 KByte/sec

However, in practice it was found that a tput value of 350000 KB/s
was required in order to constrain Vdd2 to OPP3.

This constraint still allowed Vdd1 to goto OPP1 while streaming
without causing problems.

Signed-off-by: Dominic Curran <dcurran@ti.com>
---
 arch/arm/mach-omap2/board-zoom2-camera.c |    9 ++++++++-
 drivers/media/video/imx046.c             |    2 +-
 include/media/imx046.h                   |    2 +-
 3 files changed, 10 insertions(+), 3 deletions(-)

Index: google/arch/arm/mach-omap2/board-zoom2-camera.c
===================================================================
--- google.orig/arch/arm/mach-omap2/board-zoom2-camera.c
+++ google/arch/arm/mach-omap2/board-zoom2-camera.c
@@ -24,6 +24,7 @@
 #include <asm/io.h>

 #include <mach/gpio.h>
+#include <mach/omap-pm.h>

 static int cam_inited;
 #include <media/v4l2-int-device.h>
@@ -165,7 +166,7 @@ static struct isp_interface_config imx04
 };


-static int imx046_sensor_power_set(enum v4l2_power power)
+static int imx046_sensor_power_set(struct device *dev, enum v4l2_power power)
 {
        struct isp_csi2_lanes_cfg lanecfg;
        struct isp_csi2_phy_cfg phyconfig;
@@ -176,6 +177,9 @@ static int imx046_sensor_power_set(enum
        case V4L2_POWER_ON:
                /* Power Up Sequence */
                printk(KERN_DEBUG "imx046_sensor_power_set(ON)\n");
+
+               omap_pm_set_min_bus_tput(dev, OCP_INITIATOR_AGENT, 350000);
+
                isp_csi2_reset();

                lanecfg.clk.pol = IMX046_CSI2_CLOCK_POLARITY;
@@ -245,10 +249,13 @@ static int imx046_sensor_power_set(enum
                twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER,
                                VAUX_DEV_GRP_NONE, TWL4030_VAUX2_DEV_GRP);
                gpio_free(IMX046_RESET_GPIO);
+
+               omap_pm_set_min_bus_tput(dev, OCP_INITIATOR_AGENT, 0);
                break;
        case V4L2_POWER_STANDBY:
                printk(KERN_DEBUG "imx046_sensor_power_set(STANDBY)\n");
                /*TODO*/
+               omap_pm_set_min_bus_tput(dev, OCP_INITIATOR_AGENT, 0);
                break;
        }

Index: google/drivers/media/video/imx046.c
===================================================================
--- google.orig/drivers/media/video/imx046.c
+++ google/drivers/media/video/imx046.c
@@ -1439,7 +1439,7 @@ static int ioctl_s_power(struct v4l2_int
        else
                sensor->pdata->set_xclk(xclk_current, hw_config.u.sensor.xclk);

-       rval = sensor->pdata->power_set(on);
+       rval = sensor->pdata->power_set(&client->dev, on);
        if (rval < 0) {
                v4l_err(client, "Unable to set the power state: "
                        IMX046_DRIVER_NAME " sensor\n");
Index: google/include/media/imx046.h
===================================================================
--- google.orig/include/media/imx046.h
+++ google/include/media/imx046.h
@@ -28,7 +28,7 @@
  * @priv_data_set: device private data (pointer) access function
  */
 struct imx046_platform_data {
-       int (*power_set)(enum v4l2_power power);
+       int (*power_set)(struct device*, enum v4l2_power power);
        int (*ifparm)(struct v4l2_ifparm *p);
        int (*priv_data_set)(void *);
        u32 (*set_xclk)(u32 xclk, u8 xclksel);

Re: Question about tput constraint on zoom2 camera

[Paul Walmsley]

Hello Dominic,

On Fri, 31 Jul 2009, Curran, Dominic wrote:

> I have been testing the zoom2 camera streaming while using different OPP's.
> Following table provides summary of what OPP's caused to happen:
> 
>   Streaming      Vdd1(OPP)        Vdd2(OPP)           P/F
>  VGA @ 30fps           1                2             Pass
>  8MP @ 7.5fps          1                2             Fails (stop streaming)
>  8MP @ 7.5fps          1                3             Pass
> 
> So table shows that locking Vdd2 to OPP=3 when streaming 8MPixel works, but at OPP=2 then streaming fails (stops).
> 
> So I thought the tput constraint made the most sense for camera.
> The Zoom2 camera sensor has a max tput of:
> 
>     3280 x 2464 x 2bpp x 7.5fps = 121228800 bytes/s
>                                 = 118387 KB/s
> 
> However, this calculated value doesn't constrain Vdd2 to OPP3 (DVFS enabled).
> 
> Experimentation shows that a tput value of 350000 KB/s is required to constrain Vdd2 to OPP=3.
>
> Can you explain why the practical tput constraint is so much greater than the theoretical value ?

Probably it is mostly due to two reasons: 

1. most other L3 initiator drivers (eg., for DSS, SDMA, USB, etc) don't 
currently set bus throughput constraints, so we aren't currently adding in 
their interconnect usage; and

2. the interconnect throughput model in omap-pm-srf.c is optimistic.

A couple of questions for you: (please forgive my ignorance of the camera 
subsystem):

A. What other L3 initiators are active during the test?  Presumably DSS, 
MPU?  IVA2?

B. I am assuming you are using the CCP2.  What do you have CCP2_CTRL.BURST 
set to?  This could impact interconnect utilization.


- Paul

RE: Question about tput constraint on zoom2 camera

[Curran, Dominic]

________________________________________
From: Paul Walmsley [paul@pwsan.com]
Sent: Saturday, August 01, 2009 9:57 PM
To: Curran, Dominic
Cc: Kevin Hilman; linux-omap@vger.kernel.org; Subramaniam, Muthu
Subject: Re: Question about tput constraint on zoom2 camera

Hello Dominic,

On Fri, 31 Jul 2009, Curran, Dominic wrote:

> I have been testing the zoom2 camera streaming while using different OPP's.
> Following table provides summary of what OPP's caused to happen:
>
>   Streaming      Vdd1(OPP)        Vdd2(OPP)           P/F
>  VGA @ 30fps           1                2             Pass
>  8MP @ 7.5fps          1                2             Fails (stop streaming)
>  8MP @ 7.5fps          1                3             Pass
>
> So table shows that locking Vdd2 to OPP=3 when streaming 8MPixel works, but at OPP=2 then streaming fails (stops).
>
> So I thought the tput constraint made the most sense for camera.
> The Zoom2 camera sensor has a max tput of:
>
>     3280 x 2464 x 2bpp x 7.5fps = 121228800 bytes/s
>                                 = 118387 KB/s
>
> However, this calculated value doesn't constrain Vdd2 to OPP3 (DVFS enabled).
>
> Experimentation shows that a tput value of 350000 KB/s is required to constrain Vdd2 to OPP=3.
>
> Can you explain why the practical tput constraint is so much greater than the theoretical value ?

Probably it is mostly due to two reasons:

1. most other L3 initiator drivers (eg., for DSS, SDMA, USB, etc) don't
currently set bus throughput constraints, so we aren't currently adding in
their interconnect usage; and

2. the interconnect throughput model in omap-pm-srf.c is optimistic.

A couple of questions for you: (please forgive my ignorance of the camera
subsystem):

A. What other L3 initiators are active during the test?  Presumably DSS,
MPU?  IVA2?

B. I am assuming you are using the CCP2.  What do you have CCP2_CTRL.BURST
set to?  This could impact interconnect utilization.

- Paul



Hi Paul
No DSS (i'm just printing a '.' when i dequeue a frame).
No IVA2.
No per pixel processing by the ARM.

I was trying to keep me testing as simple as possible.

HOWEVER, your questions have made me think of something else which i think _may_ explain everything.

The camera pipe should look like this:

Sensor --> CSI2 Receiver --> CCDC --> PREVIEWER --> RESIZER --> MEM

But because of a hardware bug, data has to be written to memory by Previewer and then read by Resizer.
Thus a 'workaround' buffer is allocated for this purpose.  
Its not pretty but its the only way we can have Preview & Resizer in the pipe at the same time.
So the pipeline actually looks like this:

Sensor --> CSI2 Receiver --> CCDC --> PREVIEWER --> Workaround MEM --> RESIZER --> MEM

Thus in order to get a single pixel through the pipe there has to be three L3 operations:
1) Write to workaround mem
2) Read from workaround mem
3) Write to final memory  

This seems to me like it actually increases the tput by 3x.
  118387 KB/s  x  3  =  355161 KB/s

Which looks like it is very close to the number I found in practice (350000).

Does this seem like a reasonable explanation to you ?

Thanks
dom

RE: Question about tput constraint on zoom2 camera

[Paul Walmsley]

Hello Dominic,

On Sun, 2 Aug 2009, Curran, Dominic wrote:

> From: Paul Walmsley [paul@pwsan.com]
> Sent: Saturday, August 01, 2009 9:57 PM
> 
> On Fri, 31 Jul 2009, Curran, Dominic wrote:
> 
> > I have been testing the zoom2 camera streaming while using different OPP's.
> > Following table provides summary of what OPP's caused to happen:
> >
> >   Streaming      Vdd1(OPP)        Vdd2(OPP)           P/F
> >  VGA @ 30fps           1                2             Pass
> >  8MP @ 7.5fps          1                2             Fails (stop streaming)
> >  8MP @ 7.5fps          1                3             Pass
> >
> > So table shows that locking Vdd2 to OPP=3 when streaming 8MPixel works, but at OPP=2 then streaming fails (stops).
> >
> > So I thought the tput constraint made the most sense for camera.
> > The Zoom2 camera sensor has a max tput of:
> >
> >     3280 x 2464 x 2bpp x 7.5fps = 121228800 bytes/s
> >                                 = 118387 KB/s
> >
> > However, this calculated value doesn't constrain Vdd2 to OPP3 (DVFS enabled).
> >
> > Experimentation shows that a tput value of 350000 KB/s is required to constrain Vdd2 to OPP=3.
> >
> > Can you explain why the practical tput constraint is so much greater than the theoretical value ?
> 
> Probably it is mostly due to two reasons:
> 
> 1. most other L3 initiator drivers (eg., for DSS, SDMA, USB, etc) don't
> currently set bus throughput constraints, so we aren't currently adding in
> their interconnect usage; and
> 
> 2. the interconnect throughput model in omap-pm-srf.c is optimistic.
> 
> A couple of questions for you: (please forgive my ignorance of the camera
> subsystem):
> 
> A. What other L3 initiators are active during the test?  Presumably DSS,
> MPU?  IVA2?
> 
> B. I am assuming you are using the CCP2.  What do you have CCP2_CTRL.BURST
> set to?  This could impact interconnect utilization.
> 
> - Paul
> 
> 
> 
> Hi Paul
> No DSS (i'm just printing a '.' when i dequeue a frame).
> No IVA2.
> No per pixel processing by the ARM.
> 
> I was trying to keep me testing as simple as possible.
> 
> HOWEVER, your questions have made me think of something else which i think _may_ explain everything.
> 
> The camera pipe should look like this:
> 
> Sensor --> CSI2 Receiver --> CCDC --> PREVIEWER --> RESIZER --> MEM
> 
> But because of a hardware bug, data has to be written to memory by Previewer and then read by Resizer.
> Thus a 'workaround' buffer is allocated for this purpose.  
> Its not pretty but its the only way we can have Preview & Resizer in the pipe at the same time.
> So the pipeline actually looks like this:
> 
> Sensor --> CSI2 Receiver --> CCDC --> PREVIEWER --> Workaround MEM --> RESIZER --> MEM
> 
> Thus in order to get a single pixel through the pipe there has to be three L3 operations:
> 1) Write to workaround mem
> 2) Read from workaround mem
> 3) Write to final memory  
> 
> This seems to me like it actually increases the tput by 3x.
>   118387 KB/s  x  3  =  355161 KB/s
> 
> Which looks like it is very close to the number I found in practice (350000).
> 
> Does this seem like a reasonable explanation to you ?

It does indeed.

Thanks for the explanation of the ISP pipelines.


- Paul

Re: [RFC/RFT 1/4] OMAP3: PM: Prevent hang in prcm_interrupt_handler

[Paul Walmsley]

On Fri, 24 Jul 2009, Kevin Hilman wrote:

> From: Jon Hunter <jon-hunter@ti.com>
> 
> There are two scenarios where a race condition could result in a hang
> in the prcm_interrupt handler. These are:
> 
> 1). Waiting for PRM_IRQSTATUS_MPU register to clear.
> Bit 0 of the PRM_IRQSTATUS_MPU register indicates that a wake-up event
> is pending for the MPU. This bit can only be cleared if the all the
> wake-up events latched in the various PM_WKST_x registers have been
> cleared. If a wake-up event occurred during the processing of the prcm
> interrupt handler, after the corresponding PM_WKST_x register was
> checked but before the PRM_IRQSTATUS_MPU was cleared, then the CPU
> would be stuck forever waiting for bit 0 in PRM_IRQSTATUS_MPU to be
> cleared.
> 
> 2). Waiting for the PM_WKST_x register to clear.
> Some power domains have more than one wake-up source. The PM_WKST_x
> registers indicate the source of a wake-up event and need to be cleared
> after a wake-up event occurs. When the PM_WKST_x registers are read and
> before they are cleared, it is possible that another wake-up event
> could occur causing another bit to be set in one of the PM_WKST_x
> registers. If this did occur after reading a PM_WKST_x register then
> the CPU would miss this event and get stuck forever in a loop waiting
> for that PM_WKST_x register to clear.
> 
> This patch address the above race conditions that would result in a
> hang.
> 
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>

Reviewed-by: Paul Walmsley <paul@pwsan.com>

Nice comments on this patch, Jon, they are definitely appreciated.

> ---
>  arch/arm/mach-omap2/pm34xx.c |  143 ++++++++++++++++++------------------------
>  1 files changed, 60 insertions(+), 83 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 14f10bc..f6cc71a 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -194,97 +194,74 @@ static void omap3_save_secure_ram_context(u32 target_mpu_state)
>  	}
>  }
>  
> -/* PRCM Interrupt Handler for wakeups */
> -static irqreturn_t prcm_interrupt_handler (int irq, void *dev_id)
> +/*
> + * PRCM Interrupt Handler Helper Function
> + *
> + * The purpose of this function is to clear any wake-up events latched
> + * in the PRCM PM_WKST_x registers. It is possible that a wake-up event
> + * may occur whilst attempting to clear a PM_WKST_x register and thus
> + * set another bit in this register. A while loop is used to ensure
> + * that any peripheral wake-up events occurring while attempting to
> + * clear the PM_WKST_x are detected and cleared.
> + */
> +static void prcm_clear_mod_irqs(s16 module, u8 regs)
>  {
> -	u32 wkst, irqstatus_mpu;
> -	u32 fclk, iclk;
> -
> -	/* WKUP */
> -	wkst = prm_read_mod_reg(WKUP_MOD, PM_WKST);
> -	if (wkst) {
> -		iclk = cm_read_mod_reg(WKUP_MOD, CM_ICLKEN);
> -		fclk = cm_read_mod_reg(WKUP_MOD, CM_FCLKEN);
> -		cm_set_mod_reg_bits(wkst, WKUP_MOD, CM_ICLKEN);
> -		cm_set_mod_reg_bits(wkst, WKUP_MOD, CM_FCLKEN);
> -		prm_write_mod_reg(wkst, WKUP_MOD, PM_WKST);
> -		while (prm_read_mod_reg(WKUP_MOD, PM_WKST))
> -			cpu_relax();
> -		cm_write_mod_reg(iclk, WKUP_MOD, CM_ICLKEN);
> -		cm_write_mod_reg(fclk, WKUP_MOD, CM_FCLKEN);
> -	}
> +	u32 wkst, fclk, iclk;
> +	u16 wkst_off = (regs == 3) ? OMAP3430ES2_PM_WKST3 : PM_WKST1;
> +	u16 fclk_off = (regs == 3) ? OMAP3430ES2_CM_FCLKEN3 : CM_FCLKEN1;
> +	u16 iclk_off = (regs == 3) ? CM_ICLKEN3 : CM_ICLKEN1;
>  
> -	/* CORE */
> -	wkst = prm_read_mod_reg(CORE_MOD, PM_WKST1);
> -	if (wkst) {
> -		iclk = cm_read_mod_reg(CORE_MOD, CM_ICLKEN1);
> -		fclk = cm_read_mod_reg(CORE_MOD, CM_FCLKEN1);
> -		cm_set_mod_reg_bits(wkst, CORE_MOD, CM_ICLKEN1);
> -		cm_set_mod_reg_bits(wkst, CORE_MOD, CM_FCLKEN1);
> -		prm_write_mod_reg(wkst, CORE_MOD, PM_WKST1);
> -		while (prm_read_mod_reg(CORE_MOD, PM_WKST1))
> -			cpu_relax();
> -		cm_write_mod_reg(iclk, CORE_MOD, CM_ICLKEN1);
> -		cm_write_mod_reg(fclk, CORE_MOD, CM_FCLKEN1);
> -	}
> -	wkst = prm_read_mod_reg(CORE_MOD, OMAP3430ES2_PM_WKST3);
> +	wkst = prm_read_mod_reg(module, wkst_off);
>  	if (wkst) {
> -		iclk = cm_read_mod_reg(CORE_MOD, CM_ICLKEN3);
> -		fclk = cm_read_mod_reg(CORE_MOD, OMAP3430ES2_CM_FCLKEN3);
> -		cm_set_mod_reg_bits(wkst, CORE_MOD, CM_ICLKEN3);
> -		cm_set_mod_reg_bits(wkst, CORE_MOD, OMAP3430ES2_CM_FCLKEN3);
> -		prm_write_mod_reg(wkst, CORE_MOD, OMAP3430ES2_PM_WKST3);
> -		while (prm_read_mod_reg(CORE_MOD, OMAP3430ES2_PM_WKST3))
> -			cpu_relax();
> -		cm_write_mod_reg(iclk, CORE_MOD, CM_ICLKEN3);
> -		cm_write_mod_reg(fclk, CORE_MOD, OMAP3430ES2_CM_FCLKEN3);
> -	}
> -
> -	/* PER */
> -	wkst = prm_read_mod_reg(OMAP3430_PER_MOD, PM_WKST);
> -	if (wkst) {
> -		iclk = cm_read_mod_reg(OMAP3430_PER_MOD, CM_ICLKEN);
> -		fclk = cm_read_mod_reg(OMAP3430_PER_MOD, CM_FCLKEN);
> -		cm_set_mod_reg_bits(wkst, OMAP3430_PER_MOD, CM_ICLKEN);
> -		cm_set_mod_reg_bits(wkst, OMAP3430_PER_MOD, CM_FCLKEN);
> -		prm_write_mod_reg(wkst, OMAP3430_PER_MOD, PM_WKST);
> -		while (prm_read_mod_reg(OMAP3430_PER_MOD, PM_WKST))
> -			cpu_relax();
> -		cm_write_mod_reg(iclk, OMAP3430_PER_MOD, CM_ICLKEN);
> -		cm_write_mod_reg(fclk, OMAP3430_PER_MOD, CM_FCLKEN);
> +		iclk = cm_read_mod_reg(module, iclk_off);
> +		fclk = cm_read_mod_reg(module, fclk_off);
> +		while (wkst) {
> +			cm_set_mod_reg_bits(wkst, module, iclk_off);
> +			cm_set_mod_reg_bits(wkst, module, fclk_off);
> +			prm_write_mod_reg(wkst, module, wkst_off);
> +			wkst = prm_read_mod_reg(module, wkst_off);
> +		}
> +		cm_write_mod_reg(iclk, module, iclk_off);
> +		cm_write_mod_reg(fclk, module, fclk_off);
>  	}
> +}
>  
> -	if (omap_rev() > OMAP3430_REV_ES1_0) {
> -		/* USBHOST */
> -		wkst = prm_read_mod_reg(OMAP3430ES2_USBHOST_MOD, PM_WKST);
> -		if (wkst) {
> -			iclk = cm_read_mod_reg(OMAP3430ES2_USBHOST_MOD,
> -					       CM_ICLKEN);
> -			fclk = cm_read_mod_reg(OMAP3430ES2_USBHOST_MOD,
> -					       CM_FCLKEN);
> -			cm_set_mod_reg_bits(wkst, OMAP3430ES2_USBHOST_MOD,
> -					    CM_ICLKEN);
> -			cm_set_mod_reg_bits(wkst, OMAP3430ES2_USBHOST_MOD,
> -					    CM_FCLKEN);
> -			prm_write_mod_reg(wkst, OMAP3430ES2_USBHOST_MOD,
> -					  PM_WKST);
> -			while (prm_read_mod_reg(OMAP3430ES2_USBHOST_MOD,
> -						PM_WKST))
> -				cpu_relax();
> -			cm_write_mod_reg(iclk, OMAP3430ES2_USBHOST_MOD,
> -					 CM_ICLKEN);
> -			cm_write_mod_reg(fclk, OMAP3430ES2_USBHOST_MOD,
> -					 CM_FCLKEN);
> +/*
> + * PRCM Interrupt Handler
> + *
> + * The PRM_IRQSTATUS_MPU register indicates if there are any pending
> + * interrupts from the PRCM for the MPU. These bits must be cleared in
> + * order to clear the PRCM interrupt. The PRCM interrupt handler is
> + * implemented to simply clear the PRM_IRQSTATUS_MPU in order to clear
> + * the PRCM interrupt. Please note that bit 0 of the PRM_IRQSTATUS_MPU
> + * register indicates that a wake-up event is pending for the MPU and
> + * this bit can only be cleared if the all the wake-up events latched
> + * in the various PM_WKST_x registers have been cleared. The interrupt
> + * handler is implemented using a do-while loop so that if a wake-up
> + * event occurred during the processing of the prcm interrupt handler
> + * (setting a bit in the corresponding PM_WKST_x register and thus
> + * preventing us from clearing bit 0 of the PRM_IRQSTATUS_MPU register)
> + * this would be handled.
> + */
> +static irqreturn_t prcm_interrupt_handler (int irq, void *dev_id)
> +{
> +	u32 irqstatus_mpu;
> +
> +	do {
> +		prcm_clear_mod_irqs(WKUP_MOD, 1);
> +		prcm_clear_mod_irqs(CORE_MOD, 1);
> +		prcm_clear_mod_irqs(OMAP3430_PER_MOD, 1);
> +		if (omap_rev() > OMAP3430_REV_ES1_0) {
> +			prcm_clear_mod_irqs(CORE_MOD, 3);
> +			prcm_clear_mod_irqs(OMAP3430ES2_USBHOST_MOD, 1);
>  		}
> -	}
>  
> -	irqstatus_mpu = prm_read_mod_reg(OCP_MOD,
> -					 OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
> -	prm_write_mod_reg(irqstatus_mpu, OCP_MOD,
> -			  OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
> +		irqstatus_mpu = prm_read_mod_reg(OCP_MOD,
> +					OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
> +		prm_write_mod_reg(irqstatus_mpu, OCP_MOD,
> +					OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
>  
> -	while (prm_read_mod_reg(OCP_MOD, OMAP3_PRM_IRQSTATUS_MPU_OFFSET))
> -		cpu_relax();
> +	} while (prm_read_mod_reg(OCP_MOD, OMAP3_PRM_IRQSTATUS_MPU_OFFSET));
>  
>  	return IRQ_HANDLED;
>  }
> -- 
> 1.6.3.3
> 
> --
> 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
> 


- Paul

Re: [RFC/RFT 2/4] OMAP3: PM: PRCM interrupt: check MPUGRPSEL register

[Paul Walmsley]

On Fri, 24 Jul 2009, Kevin Hilman wrote:

> From: Paul Walmsley <paul@pwsan.com>
> 
> PM_WKST register contents should be ANDed with the contents of the
> MPUGRPSEL registers.  Otherwise the MPU PRCM interrupt handler could
> wind up clearing wakeup events meant for the IVA PRCM interrupt
> handler.  For a production version of this patch, we should not read
> MPUGRPSEL from the PRM, since those reads are very slow; rather, we
> should use a cached version from struct powerdomain (not yet
> implemented)

This patch is fine to go in.  Maybe alter the last sentence of the commit 
message to read something like "A future revision to this code should be 
to read a cached version of MPUGRPSEL from the powerdomain code, since 
PRM reads are relatively slow."

Then it would be:

Signed-off-by: Paul Walmsley <paul@pwsan.com>


> ---
>  arch/arm/mach-omap2/pm34xx.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index f6cc71a..25372b7 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -210,8 +210,11 @@ static void prcm_clear_mod_irqs(s16 module, u8 regs)
>  	u16 wkst_off = (regs == 3) ? OMAP3430ES2_PM_WKST3 : PM_WKST1;
>  	u16 fclk_off = (regs == 3) ? OMAP3430ES2_CM_FCLKEN3 : CM_FCLKEN1;
>  	u16 iclk_off = (regs == 3) ? CM_ICLKEN3 : CM_ICLKEN1;
> +	u16 grpsel_off = (regs == 3) ?
> +		OMAP3430ES2_PM_MPUGRPSEL3 : OMAP3430_PM_MPUGRPSEL;
>  
>  	wkst = prm_read_mod_reg(module, wkst_off);
> +	wkst &= prm_read_mod_reg(module, grpsel_off);
>  	if (wkst) {
>  		iclk = cm_read_mod_reg(module, iclk_off);
>  		fclk = cm_read_mod_reg(module, fclk_off);
> -- 
> 1.6.3.3
> 
> --
> 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
> 


- Paul

Re: [RFC/RFT 3/4] OMAP3: PM: PRCM interrupt: only handle selected PRCM interrupts

[Paul Walmsley]

On Fri, 24 Jul 2009, Kevin Hilman wrote:

> From: Paul Walmsley <paul@pwsan.com>
> 
> Clearing wakeup sources is now only done when the PRM indicates a
> wakeup source interrupt.  Since we don't handle any other types of
> PRCM interrupts right now, warn if we get any other type of PRCM
> interrupt.  Either code needs to be added to the PRCM interrupt
> handler to react to these, or these other interrupts should be masked
> off at init.

Looks good to me, Kevin -


Signed-off-by: Paul Walmsley <paul@pwsan.com>

Thanks for doing the heavy lifting to update this and the previous patch 
after Jon's work.



> ---
>  arch/arm/mach-omap2/pm34xx.c |   46 +++++++++++++++++++++++++++++++++--------
>  1 files changed, 37 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 25372b7..348a683 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -204,7 +204,7 @@ static void omap3_save_secure_ram_context(u32 target_mpu_state)
>   * that any peripheral wake-up events occurring while attempting to
>   * clear the PM_WKST_x are detected and cleared.
>   */
> -static void prcm_clear_mod_irqs(s16 module, u8 regs)
> +static int prcm_clear_mod_irqs(s16 module, u8 regs)
>  {
>  	u32 wkst, fclk, iclk;
>  	u16 wkst_off = (regs == 3) ? OMAP3430ES2_PM_WKST3 : PM_WKST1;
> @@ -212,6 +212,7 @@ static void prcm_clear_mod_irqs(s16 module, u8 regs)
>  	u16 iclk_off = (regs == 3) ? CM_ICLKEN3 : CM_ICLKEN1;
>  	u16 grpsel_off = (regs == 3) ?
>  		OMAP3430ES2_PM_MPUGRPSEL3 : OMAP3430_PM_MPUGRPSEL;
> +	int c = 0;
>  
>  	wkst = prm_read_mod_reg(module, wkst_off);
>  	wkst &= prm_read_mod_reg(module, grpsel_off);
> @@ -223,10 +224,28 @@ static void prcm_clear_mod_irqs(s16 module, u8 regs)
>  			cm_set_mod_reg_bits(wkst, module, fclk_off);
>  			prm_write_mod_reg(wkst, module, wkst_off);
>  			wkst = prm_read_mod_reg(module, wkst_off);
> +			c++;
>  		}
>  		cm_write_mod_reg(iclk, module, iclk_off);
>  		cm_write_mod_reg(fclk, module, fclk_off);
>  	}
> +
> +	return c;
> +}
> +
> +static int _prcm_int_handle_wakeup(void)
> +{
> +	int c;
> +
> +	c = prcm_clear_mod_irqs(WKUP_MOD, 1);
> +	c += prcm_clear_mod_irqs(CORE_MOD, 1);
> +	c += prcm_clear_mod_irqs(OMAP3430_PER_MOD, 1);
> +	if (omap_rev() > OMAP3430_REV_ES1_0) {
> +		c += prcm_clear_mod_irqs(CORE_MOD, 3);
> +		c += prcm_clear_mod_irqs(OMAP3430ES2_USBHOST_MOD, 1);
> +	}
> +
> +	return c;
>  }
>  
>  /*
> @@ -249,18 +268,27 @@ static void prcm_clear_mod_irqs(s16 module, u8 regs)
>  static irqreturn_t prcm_interrupt_handler (int irq, void *dev_id)
>  {
>  	u32 irqstatus_mpu;
> +	int c = 0;
>  
>  	do {
> -		prcm_clear_mod_irqs(WKUP_MOD, 1);
> -		prcm_clear_mod_irqs(CORE_MOD, 1);
> -		prcm_clear_mod_irqs(OMAP3430_PER_MOD, 1);
> -		if (omap_rev() > OMAP3430_REV_ES1_0) {
> -			prcm_clear_mod_irqs(CORE_MOD, 3);
> -			prcm_clear_mod_irqs(OMAP3430ES2_USBHOST_MOD, 1);
> -		}
> -
>  		irqstatus_mpu = prm_read_mod_reg(OCP_MOD,
>  					OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
> +
> +		if (irqstatus_mpu & (OMAP3430_WKUP_ST | OMAP3430_IO_ST)) {
> +			c = _prcm_int_handle_wakeup();
> +
> +			/*
> +			 * Is the MPU PRCM interrupt handler racing with the
> +			 * IVA2 PRCM interrupt handler ?
> +			 */
> +			WARN(c == 0, "prcm: WARNING: PRCM indicated MPU wakeup "
> +			     "but no wakeup sources are marked\n");
> +		} else {
> +			/* XXX we need to expand our PRCM interrupt handler */
> +			WARN(1, "prcm: WARNING: PRCM interrupt received, but "
> +			     "no code to handle it (%08x)\n", irqstatus_mpu);
> +		}
> +
>  		prm_write_mod_reg(irqstatus_mpu, OCP_MOD,
>  					OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
>  
> -- 
> 1.6.3.3
> 
> --
> 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
> 


- Paul

Re: Question about tput constraint on zoom2 camera

[Kevin Hilman]

Paul Walmsley wrote:
> Hello Dominic,
> 
> On Sun, 2 Aug 2009, Curran, Dominic wrote:
> 
>> From: Paul Walmsley [paul@pwsan.com]
>> Sent: Saturday, August 01, 2009 9:57 PM
>>
>> On Fri, 31 Jul 2009, Curran, Dominic wrote:
>>
>>> I have been testing the zoom2 camera streaming while using different OPP's.
>>> Following table provides summary of what OPP's caused to happen:
>>>
>>>   Streaming      Vdd1(OPP)        Vdd2(OPP)           P/F
>>>  VGA @ 30fps           1                2             Pass
>>>  8MP @ 7.5fps          1                2             Fails (stop streaming)
>>>  8MP @ 7.5fps          1                3             Pass
>>>
>>> So table shows that locking Vdd2 to OPP=3 when streaming 8MPixel works, but at OPP=2 then streaming fails (stops).
>>>
>>> So I thought the tput constraint made the most sense for camera.
>>> The Zoom2 camera sensor has a max tput of:
>>>
>>>     3280 x 2464 x 2bpp x 7.5fps = 121228800 bytes/s
>>>                                 = 118387 KB/s
>>>
>>> However, this calculated value doesn't constrain Vdd2 to OPP3 (DVFS enabled).
>>>
>>> Experimentation shows that a tput value of 350000 KB/s is required to constrain Vdd2 to OPP=3.
>>>
>>> Can you explain why the practical tput constraint is so much greater than the theoretical value ?
>> Probably it is mostly due to two reasons:
>>
>> 1. most other L3 initiator drivers (eg., for DSS, SDMA, USB, etc) don't
>> currently set bus throughput constraints, so we aren't currently adding in
>> their interconnect usage; and
>>
>> 2. the interconnect throughput model in omap-pm-srf.c is optimistic.
>>
>> A couple of questions for you: (please forgive my ignorance of the camera
>> subsystem):
>>
>> A. What other L3 initiators are active during the test?  Presumably DSS,
>> MPU?  IVA2?
>>
>> B. I am assuming you are using the CCP2.  What do you have CCP2_CTRL.BURST
>> set to?  This could impact interconnect utilization.
>>
>> - Paul
>>
>>
>>
>> Hi Paul
>> No DSS (i'm just printing a '.' when i dequeue a frame).
>> No IVA2.
>> No per pixel processing by the ARM.
>>
>> I was trying to keep me testing as simple as possible.
>>
>> HOWEVER, your questions have made me think of something else which i think _may_ explain everything.
>>
>> The camera pipe should look like this:
>>
>> Sensor --> CSI2 Receiver --> CCDC --> PREVIEWER --> RESIZER --> MEM
>>
>> But because of a hardware bug, data has to be written to memory by Previewer and then read by Resizer.
>> Thus a 'workaround' buffer is allocated for this purpose.  
>> Its not pretty but its the only way we can have Preview & Resizer in the pipe at the same time.
>> So the pipeline actually looks like this:
>>
>> Sensor --> CSI2 Receiver --> CCDC --> PREVIEWER --> Workaround MEM --> RESIZER --> MEM
>>
>> Thus in order to get a single pixel through the pipe there has to be three L3 operations:
>> 1) Write to workaround mem
>> 2) Read from workaround mem
>> 3) Write to final memory  
>>
>> This seems to me like it actually increases the tput by 3x.
>>   118387 KB/s  x  3  =  355161 KB/s
>>
>> Which looks like it is very close to the number I found in practice (350000).
>>
>> Does this seem like a reasonable explanation to you ?
> 
> It does indeed.
> 
> Thanks for the explanation of the ISP pipelines.

Dominic,

Please be sure to update the changelog in your patch to describe the 
pipeline and workaround mem which leads to your tput number.

Thanks,

Kevin

Re: [RFC/RFT 1/4] OMAP3: PM: Prevent hang in prcm_interrupt_handler

[Kevin Hilman]

Paul Walmsley <paul@pwsan.com> writes:

> On Fri, 24 Jul 2009, Kevin Hilman wrote:
>
>> From: Jon Hunter <jon-hunter@ti.com>
>> 
>> There are two scenarios where a race condition could result in a hang
>> in the prcm_interrupt handler. These are:
>> 
>> 1). Waiting for PRM_IRQSTATUS_MPU register to clear.
>> Bit 0 of the PRM_IRQSTATUS_MPU register indicates that a wake-up event
>> is pending for the MPU. This bit can only be cleared if the all the
>> wake-up events latched in the various PM_WKST_x registers have been
>> cleared. If a wake-up event occurred during the processing of the prcm
>> interrupt handler, after the corresponding PM_WKST_x register was
>> checked but before the PRM_IRQSTATUS_MPU was cleared, then the CPU
>> would be stuck forever waiting for bit 0 in PRM_IRQSTATUS_MPU to be
>> cleared.
>> 
>> 2). Waiting for the PM_WKST_x register to clear.
>> Some power domains have more than one wake-up source. The PM_WKST_x
>> registers indicate the source of a wake-up event and need to be cleared
>> after a wake-up event occurs. When the PM_WKST_x registers are read and
>> before they are cleared, it is possible that another wake-up event
>> could occur causing another bit to be set in one of the PM_WKST_x
>> registers. If this did occur after reading a PM_WKST_x register then
>> the CPU would miss this event and get stuck forever in a loop waiting
>> for that PM_WKST_x register to clear.
>> 
>> This patch address the above race conditions that would result in a
>> hang.
>> 
>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>
> Reviewed-by: Paul Walmsley <paul@pwsan.com>

Pusing to PM branch.

Kevin

> Nice comments on this patch, Jon, they are definitely appreciated.
>
>> ---
>>  arch/arm/mach-omap2/pm34xx.c |  143 ++++++++++++++++++------------------------
>>  1 files changed, 60 insertions(+), 83 deletions(-)
>> 
>> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
>> index 14f10bc..f6cc71a 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -194,97 +194,74 @@ static void omap3_save_secure_ram_context(u32 target_mpu_state)
>>  	}
>>  }
>>  
>> -/* PRCM Interrupt Handler for wakeups */
>> -static irqreturn_t prcm_interrupt_handler (int irq, void *dev_id)
>> +/*
>> + * PRCM Interrupt Handler Helper Function
>> + *
>> + * The purpose of this function is to clear any wake-up events latched
>> + * in the PRCM PM_WKST_x registers. It is possible that a wake-up event
>> + * may occur whilst attempting to clear a PM_WKST_x register and thus
>> + * set another bit in this register. A while loop is used to ensure
>> + * that any peripheral wake-up events occurring while attempting to
>> + * clear the PM_WKST_x are detected and cleared.
>> + */
>> +static void prcm_clear_mod_irqs(s16 module, u8 regs)
>>  {
>> -	u32 wkst, irqstatus_mpu;
>> -	u32 fclk, iclk;
>> -
>> -	/* WKUP */
>> -	wkst = prm_read_mod_reg(WKUP_MOD, PM_WKST);
>> -	if (wkst) {
>> -		iclk = cm_read_mod_reg(WKUP_MOD, CM_ICLKEN);
>> -		fclk = cm_read_mod_reg(WKUP_MOD, CM_FCLKEN);
>> -		cm_set_mod_reg_bits(wkst, WKUP_MOD, CM_ICLKEN);
>> -		cm_set_mod_reg_bits(wkst, WKUP_MOD, CM_FCLKEN);
>> -		prm_write_mod_reg(wkst, WKUP_MOD, PM_WKST);
>> -		while (prm_read_mod_reg(WKUP_MOD, PM_WKST))
>> -			cpu_relax();
>> -		cm_write_mod_reg(iclk, WKUP_MOD, CM_ICLKEN);
>> -		cm_write_mod_reg(fclk, WKUP_MOD, CM_FCLKEN);
>> -	}
>> +	u32 wkst, fclk, iclk;
>> +	u16 wkst_off = (regs == 3) ? OMAP3430ES2_PM_WKST3 : PM_WKST1;
>> +	u16 fclk_off = (regs == 3) ? OMAP3430ES2_CM_FCLKEN3 : CM_FCLKEN1;
>> +	u16 iclk_off = (regs == 3) ? CM_ICLKEN3 : CM_ICLKEN1;
>>  
>> -	/* CORE */
>> -	wkst = prm_read_mod_reg(CORE_MOD, PM_WKST1);
>> -	if (wkst) {
>> -		iclk = cm_read_mod_reg(CORE_MOD, CM_ICLKEN1);
>> -		fclk = cm_read_mod_reg(CORE_MOD, CM_FCLKEN1);
>> -		cm_set_mod_reg_bits(wkst, CORE_MOD, CM_ICLKEN1);
>> -		cm_set_mod_reg_bits(wkst, CORE_MOD, CM_FCLKEN1);
>> -		prm_write_mod_reg(wkst, CORE_MOD, PM_WKST1);
>> -		while (prm_read_mod_reg(CORE_MOD, PM_WKST1))
>> -			cpu_relax();
>> -		cm_write_mod_reg(iclk, CORE_MOD, CM_ICLKEN1);
>> -		cm_write_mod_reg(fclk, CORE_MOD, CM_FCLKEN1);
>> -	}
>> -	wkst = prm_read_mod_reg(CORE_MOD, OMAP3430ES2_PM_WKST3);
>> +	wkst = prm_read_mod_reg(module, wkst_off);
>>  	if (wkst) {
>> -		iclk = cm_read_mod_reg(CORE_MOD, CM_ICLKEN3);
>> -		fclk = cm_read_mod_reg(CORE_MOD, OMAP3430ES2_CM_FCLKEN3);
>> -		cm_set_mod_reg_bits(wkst, CORE_MOD, CM_ICLKEN3);
>> -		cm_set_mod_reg_bits(wkst, CORE_MOD, OMAP3430ES2_CM_FCLKEN3);
>> -		prm_write_mod_reg(wkst, CORE_MOD, OMAP3430ES2_PM_WKST3);
>> -		while (prm_read_mod_reg(CORE_MOD, OMAP3430ES2_PM_WKST3))
>> -			cpu_relax();
>> -		cm_write_mod_reg(iclk, CORE_MOD, CM_ICLKEN3);
>> -		cm_write_mod_reg(fclk, CORE_MOD, OMAP3430ES2_CM_FCLKEN3);
>> -	}
>> -
>> -	/* PER */
>> -	wkst = prm_read_mod_reg(OMAP3430_PER_MOD, PM_WKST);
>> -	if (wkst) {
>> -		iclk = cm_read_mod_reg(OMAP3430_PER_MOD, CM_ICLKEN);
>> -		fclk = cm_read_mod_reg(OMAP3430_PER_MOD, CM_FCLKEN);
>> -		cm_set_mod_reg_bits(wkst, OMAP3430_PER_MOD, CM_ICLKEN);
>> -		cm_set_mod_reg_bits(wkst, OMAP3430_PER_MOD, CM_FCLKEN);
>> -		prm_write_mod_reg(wkst, OMAP3430_PER_MOD, PM_WKST);
>> -		while (prm_read_mod_reg(OMAP3430_PER_MOD, PM_WKST))
>> -			cpu_relax();
>> -		cm_write_mod_reg(iclk, OMAP3430_PER_MOD, CM_ICLKEN);
>> -		cm_write_mod_reg(fclk, OMAP3430_PER_MOD, CM_FCLKEN);
>> +		iclk = cm_read_mod_reg(module, iclk_off);
>> +		fclk = cm_read_mod_reg(module, fclk_off);
>> +		while (wkst) {
>> +			cm_set_mod_reg_bits(wkst, module, iclk_off);
>> +			cm_set_mod_reg_bits(wkst, module, fclk_off);
>> +			prm_write_mod_reg(wkst, module, wkst_off);
>> +			wkst = prm_read_mod_reg(module, wkst_off);
>> +		}
>> +		cm_write_mod_reg(iclk, module, iclk_off);
>> +		cm_write_mod_reg(fclk, module, fclk_off);
>>  	}
>> +}
>>  
>> -	if (omap_rev() > OMAP3430_REV_ES1_0) {
>> -		/* USBHOST */
>> -		wkst = prm_read_mod_reg(OMAP3430ES2_USBHOST_MOD, PM_WKST);
>> -		if (wkst) {
>> -			iclk = cm_read_mod_reg(OMAP3430ES2_USBHOST_MOD,
>> -					       CM_ICLKEN);
>> -			fclk = cm_read_mod_reg(OMAP3430ES2_USBHOST_MOD,
>> -					       CM_FCLKEN);
>> -			cm_set_mod_reg_bits(wkst, OMAP3430ES2_USBHOST_MOD,
>> -					    CM_ICLKEN);
>> -			cm_set_mod_reg_bits(wkst, OMAP3430ES2_USBHOST_MOD,
>> -					    CM_FCLKEN);
>> -			prm_write_mod_reg(wkst, OMAP3430ES2_USBHOST_MOD,
>> -					  PM_WKST);
>> -			while (prm_read_mod_reg(OMAP3430ES2_USBHOST_MOD,
>> -						PM_WKST))
>> -				cpu_relax();
>> -			cm_write_mod_reg(iclk, OMAP3430ES2_USBHOST_MOD,
>> -					 CM_ICLKEN);
>> -			cm_write_mod_reg(fclk, OMAP3430ES2_USBHOST_MOD,
>> -					 CM_FCLKEN);
>> +/*
>> + * PRCM Interrupt Handler
>> + *
>> + * The PRM_IRQSTATUS_MPU register indicates if there are any pending
>> + * interrupts from the PRCM for the MPU. These bits must be cleared in
>> + * order to clear the PRCM interrupt. The PRCM interrupt handler is
>> + * implemented to simply clear the PRM_IRQSTATUS_MPU in order to clear
>> + * the PRCM interrupt. Please note that bit 0 of the PRM_IRQSTATUS_MPU
>> + * register indicates that a wake-up event is pending for the MPU and
>> + * this bit can only be cleared if the all the wake-up events latched
>> + * in the various PM_WKST_x registers have been cleared. The interrupt
>> + * handler is implemented using a do-while loop so that if a wake-up
>> + * event occurred during the processing of the prcm interrupt handler
>> + * (setting a bit in the corresponding PM_WKST_x register and thus
>> + * preventing us from clearing bit 0 of the PRM_IRQSTATUS_MPU register)
>> + * this would be handled.
>> + */
>> +static irqreturn_t prcm_interrupt_handler (int irq, void *dev_id)
>> +{
>> +	u32 irqstatus_mpu;
>> +
>> +	do {
>> +		prcm_clear_mod_irqs(WKUP_MOD, 1);
>> +		prcm_clear_mod_irqs(CORE_MOD, 1);
>> +		prcm_clear_mod_irqs(OMAP3430_PER_MOD, 1);
>> +		if (omap_rev() > OMAP3430_REV_ES1_0) {
>> +			prcm_clear_mod_irqs(CORE_MOD, 3);
>> +			prcm_clear_mod_irqs(OMAP3430ES2_USBHOST_MOD, 1);
>>  		}
>> -	}
>>  
>> -	irqstatus_mpu = prm_read_mod_reg(OCP_MOD,
>> -					 OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
>> -	prm_write_mod_reg(irqstatus_mpu, OCP_MOD,
>> -			  OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
>> +		irqstatus_mpu = prm_read_mod_reg(OCP_MOD,
>> +					OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
>> +		prm_write_mod_reg(irqstatus_mpu, OCP_MOD,
>> +					OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
>>  
>> -	while (prm_read_mod_reg(OCP_MOD, OMAP3_PRM_IRQSTATUS_MPU_OFFSET))
>> -		cpu_relax();
>> +	} while (prm_read_mod_reg(OCP_MOD, OMAP3_PRM_IRQSTATUS_MPU_OFFSET));
>>  
>>  	return IRQ_HANDLED;
>>  }
>> -- 
>> 1.6.3.3
>> 
>> --
>> 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
>> 
>
>
> - Paul

Re: [RFC/RFT 2/4] OMAP3: PM: PRCM interrupt: check MPUGRPSEL register

[Kevin Hilman]

Paul Walmsley <paul@pwsan.com> writes:

> On Fri, 24 Jul 2009, Kevin Hilman wrote:
>
>> From: Paul Walmsley <paul@pwsan.com>
>> 
>> PM_WKST register contents should be ANDed with the contents of the
>> MPUGRPSEL registers.  Otherwise the MPU PRCM interrupt handler could
>> wind up clearing wakeup events meant for the IVA PRCM interrupt
>> handler.  For a production version of this patch, we should not read
>> MPUGRPSEL from the PRM, since those reads are very slow; rather, we
>> should use a cached version from struct powerdomain (not yet
>> implemented)
>
> This patch is fine to go in.  Maybe alter the last sentence of the commit 
> message to read something like "A future revision to this code should be 
> to read a cached version of MPUGRPSEL from the powerdomain code, since 
> PRM reads are relatively slow."
>
> Then it would be:
>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>

Updated, and pushing to PM branch.

Kevin

>
>> ---
>>  arch/arm/mach-omap2/pm34xx.c |    3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
>> 
>> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
>> index f6cc71a..25372b7 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -210,8 +210,11 @@ static void prcm_clear_mod_irqs(s16 module, u8 regs)
>>  	u16 wkst_off = (regs == 3) ? OMAP3430ES2_PM_WKST3 : PM_WKST1;
>>  	u16 fclk_off = (regs == 3) ? OMAP3430ES2_CM_FCLKEN3 : CM_FCLKEN1;
>>  	u16 iclk_off = (regs == 3) ? CM_ICLKEN3 : CM_ICLKEN1;
>> +	u16 grpsel_off = (regs == 3) ?
>> +		OMAP3430ES2_PM_MPUGRPSEL3 : OMAP3430_PM_MPUGRPSEL;
>>  
>>  	wkst = prm_read_mod_reg(module, wkst_off);
>> +	wkst &= prm_read_mod_reg(module, grpsel_off);
>>  	if (wkst) {
>>  		iclk = cm_read_mod_reg(module, iclk_off);
>>  		fclk = cm_read_mod_reg(module, fclk_off);
>> -- 
>> 1.6.3.3
>> 
>> --
>> 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
>> 
>
>
> - Paul

Re: [RFC/RFT 3/4] OMAP3: PM: PRCM interrupt: only handle selected PRCM interrupts

[Kevin Hilman]

Paul Walmsley <paul@pwsan.com> writes:

> On Fri, 24 Jul 2009, Kevin Hilman wrote:
>
>> From: Paul Walmsley <paul@pwsan.com>
>> 
>> Clearing wakeup sources is now only done when the PRM indicates a
>> wakeup source interrupt.  Since we don't handle any other types of
>> PRCM interrupts right now, warn if we get any other type of PRCM
>> interrupt.  Either code needs to be added to the PRCM interrupt
>> handler to react to these, or these other interrupts should be masked
>> off at init.
>
> Looks good to me, Kevin -
>
>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
>
> Thanks for doing the heavy lifting to update this and the previous patch 
> after Jon's work.

Pushing to PM branch.

Kevin

>
>
>
>> ---
>>  arch/arm/mach-omap2/pm34xx.c |   46 +++++++++++++++++++++++++++++++++--------
>>  1 files changed, 37 insertions(+), 9 deletions(-)
>> 
>> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
>> index 25372b7..348a683 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -204,7 +204,7 @@ static void omap3_save_secure_ram_context(u32 target_mpu_state)
>>   * that any peripheral wake-up events occurring while attempting to
>>   * clear the PM_WKST_x are detected and cleared.
>>   */
>> -static void prcm_clear_mod_irqs(s16 module, u8 regs)
>> +static int prcm_clear_mod_irqs(s16 module, u8 regs)
>>  {
>>  	u32 wkst, fclk, iclk;
>>  	u16 wkst_off = (regs == 3) ? OMAP3430ES2_PM_WKST3 : PM_WKST1;
>> @@ -212,6 +212,7 @@ static void prcm_clear_mod_irqs(s16 module, u8 regs)
>>  	u16 iclk_off = (regs == 3) ? CM_ICLKEN3 : CM_ICLKEN1;
>>  	u16 grpsel_off = (regs == 3) ?
>>  		OMAP3430ES2_PM_MPUGRPSEL3 : OMAP3430_PM_MPUGRPSEL;
>> +	int c = 0;
>>  
>>  	wkst = prm_read_mod_reg(module, wkst_off);
>>  	wkst &= prm_read_mod_reg(module, grpsel_off);
>> @@ -223,10 +224,28 @@ static void prcm_clear_mod_irqs(s16 module, u8 regs)
>>  			cm_set_mod_reg_bits(wkst, module, fclk_off);
>>  			prm_write_mod_reg(wkst, module, wkst_off);
>>  			wkst = prm_read_mod_reg(module, wkst_off);
>> +			c++;
>>  		}
>>  		cm_write_mod_reg(iclk, module, iclk_off);
>>  		cm_write_mod_reg(fclk, module, fclk_off);
>>  	}
>> +
>> +	return c;
>> +}
>> +
>> +static int _prcm_int_handle_wakeup(void)
>> +{
>> +	int c;
>> +
>> +	c = prcm_clear_mod_irqs(WKUP_MOD, 1);
>> +	c += prcm_clear_mod_irqs(CORE_MOD, 1);
>> +	c += prcm_clear_mod_irqs(OMAP3430_PER_MOD, 1);
>> +	if (omap_rev() > OMAP3430_REV_ES1_0) {
>> +		c += prcm_clear_mod_irqs(CORE_MOD, 3);
>> +		c += prcm_clear_mod_irqs(OMAP3430ES2_USBHOST_MOD, 1);
>> +	}
>> +
>> +	return c;
>>  }
>>  
>>  /*
>> @@ -249,18 +268,27 @@ static void prcm_clear_mod_irqs(s16 module, u8 regs)
>>  static irqreturn_t prcm_interrupt_handler (int irq, void *dev_id)
>>  {
>>  	u32 irqstatus_mpu;
>> +	int c = 0;
>>  
>>  	do {
>> -		prcm_clear_mod_irqs(WKUP_MOD, 1);
>> -		prcm_clear_mod_irqs(CORE_MOD, 1);
>> -		prcm_clear_mod_irqs(OMAP3430_PER_MOD, 1);
>> -		if (omap_rev() > OMAP3430_REV_ES1_0) {
>> -			prcm_clear_mod_irqs(CORE_MOD, 3);
>> -			prcm_clear_mod_irqs(OMAP3430ES2_USBHOST_MOD, 1);
>> -		}
>> -
>>  		irqstatus_mpu = prm_read_mod_reg(OCP_MOD,
>>  					OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
>> +
>> +		if (irqstatus_mpu & (OMAP3430_WKUP_ST | OMAP3430_IO_ST)) {
>> +			c = _prcm_int_handle_wakeup();
>> +
>> +			/*
>> +			 * Is the MPU PRCM interrupt handler racing with the
>> +			 * IVA2 PRCM interrupt handler ?
>> +			 */
>> +			WARN(c == 0, "prcm: WARNING: PRCM indicated MPU wakeup "
>> +			     "but no wakeup sources are marked\n");
>> +		} else {
>> +			/* XXX we need to expand our PRCM interrupt handler */
>> +			WARN(1, "prcm: WARNING: PRCM interrupt received, but "
>> +			     "no code to handle it (%08x)\n", irqstatus_mpu);
>> +		}
>> +
>>  		prm_write_mod_reg(irqstatus_mpu, OCP_MOD,
>>  					OMAP3_PRM_IRQSTATUS_MPU_OFFSET);
>>  
>> -- 
>> 1.6.3.3
>> 
>> --
>> 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
>> 
>
>
> - Paul