RE: [PATCH v2] OMAP3: PM: PRCM interrupt: Fix warning "MPU wakeup but no wakeup sources"
Hi Thomas / Kevin,
I did verify Thomas Petazzoni's patch - [PATCH] omap: prcm: switch to a
chained IRQ handler mechanism, and I have below questions or comments.
1. I see for each WKUP_ST or IO_ST interrupt the _prcm_int_handle_wakeup
handler is getting called 2 times which impacts on performance.
printk("irq:%d,%d\n",irq,c); just before returning from the handler shows.
[ 221.966308] irq wkst:377,2
[ 221.968597] irq wkst:377,0
I see, the code checking the below warning is removed, won't it be good to
retain this check ?
WARN(c == 0, "prcm: WARNING: PRCM indicated "
"MPU wakeup but no wakeup sources "
"are marked\n");
Also need to address the corner case issue, for which I submitted the patch
fix.
[ 222.002563] irq wkst:368,3
[ 222.004913] irq iost:377,0
Regards
Gowda
From: ext Kevin Hilman [khil...@deeprootsystems.com]
Sent: Friday, November 19, 2010 6:36 PM
To: Gowda Madhusudhan.1 (EXT-Elektrobit/Helsinki)
Cc: linux-omap@vger.kernel.org; p...@pwsan.com
Subject: Re: [PATCH v2] OMAP3: PM: PRCM interrupt: Fix warning "MPU wakeup but
no wakeup sources"
Madhusudhan Gowda writes:
> A corner case where prcm_interrupt handler is handling the WKST_WKUP and
> before acknowledging the wakeup sources if an IO Pad wakeup ST_IO is
> indicated then hits the below warning since the wakeup sources are already
> cleared.
> WARN(c == 0, "prcm: WARNING: PRCM indicated "
> "MPU wakeup but no wakeup sources "
> "are marked\n");
>
> Since the above warning condition is only valid if the prcm_interrupt
> handler is called but no wakeup sources are marked in first iteration.
>
> The patch fixes this corner case.
>
> Updated after Paul Walmsley's "only handle selected PRCM interrupts" patch.
Can you have a look at the recent work by Thomas Petazzoni:
[PATCH] omap: prcm: switch to a chained IRQ handler mechanism
where the PRCM IRQ handler is broken up to see if this problem still
exists? I suspect the problem is gone as each type of interrupt is
separated out, but should be verified.
Kevin
>
> Signed-off-by: Madhusudhan Gowda
> ---
> arch/arm/mach-omap2/pm34xx.c |7 +--
> 1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 75c0cd1..2ed3662 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -266,6 +266,7 @@ static irqreturn_t prcm_interrupt_handler (int irq, void
> *dev_id)
> {
> u32 irqenable_mpu, irqstatus_mpu;
> int c = 0;
> + int ct = 0;
>
> irqenable_mpu = prm_read_mod_reg(OCP_MOD,
>OMAP3_PRM_IRQENABLE_MPU_OFFSET);
> @@ -277,13 +278,15 @@ static irqreturn_t prcm_interrupt_handler (int irq,
> void *dev_id)
> if (irqstatus_mpu & (OMAP3430_WKUP_ST_MASK |
>OMAP3430_IO_ST_MASK)) {
> c = _prcm_int_handle_wakeup();
> + ct++;
>
> /*
>* 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");
> + WARN(!c && (ct == 1), "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 "
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] OMAP3: PM: PRCM interrupt: Fix warning "MPU wakeup but no wakeup sources"
Yes Balbi. I will incorporate your comments.
Regards
Gowda
From: linux-omap-ow...@vger.kernel.org [linux-omap-ow...@vger.kernel.org] On
Behalf Of ext Felipe Balbi [ba...@ti.com]
Sent: Friday, November 19, 2010 1:47 PM
To: Gowda Madhusudhan.1 (EXT-Elektrobit/Helsinki)
Cc: linux-omap@vger.kernel.org; p...@pwsan.com
Subject: Re: [PATCH] OMAP3: PM: PRCM interrupt: Fix warning "MPU wakeup but no
wakeup sources"
Hi,
On Fri, Nov 19, 2010 at 01:26:33PM +0200, Madhusudhan Gowda wrote:
>@@ -277,13 +278,16 @@ static irqreturn_t prcm_interrupt_handler (int irq, void
>*dev_id)
> if (irqstatus_mpu & (OMAP3430_WKUP_ST_MASK |
>OMAP3430_IO_ST_MASK)) {
> c = _prcm_int_handle_wakeup();
>+ ct++;
>
> /*
>* 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");
>+ if (ct == 1)
>+ WARN(c == 0, "prcm: WARNING: PRCM indicated "
>+ "MPU wakeup but no wakeup sources "
>+ "are marked\n");
WARN(!c && (ct == 1), "prcm: WARNING: PRCM indicated "
"MPU wakeup but no wakeup sources "
"are marked\n");
would do it? Then you don't need the extra if ().
--
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 5/6] save and restore etm state across core OFF modes
Hi,
> > + bne debug_restore /* loop till done */
> > + str r5, [r6, #ETMMR_OSSRR] /* clear lock */
I had informed Alexander about the missing OSLAR to clear the lock and also
the do_etm_save value does not retain across coreoff since sram size may vary
across coreoffs. We need to save this do_etm_save value to sdram along with the
jtag etm debug context and restore it to do_etm_save.
Thanks
Gowda
From: Eduardo Valentin [eduardo.valen...@nokia.com]
Sent: Wednesday, October 06, 2010 2:22 PM
To: Valentin Eduardo (Nokia-MS/Helsinki)
Cc: ext virtu...@slind.org; t...@atomide.com; linux-omap@vger.kernel.org;
khil...@deeprootsystems.com; r-woodru...@ti.com; Gowda Madhusudhan.1
(EXT-Elektrobit/Helsinki)
Subject: Re: [PATCH 5/6] save and restore etm state across core OFF modes
Hey,
I think Gowda had also some thoughts about this patch. Cc'ing him.
BR,
On Wed, Oct 06, 2010 at 10:35:09AM +0200, Valentin Eduardo (Nokia-D/Helsinki)
wrote:
> Hello Alexander,
>
> Few points as follows,
>
> On Sat, May 01, 2010 at 07:38:20PM +0200, ext virtu...@slind.org wrote:
> > From: Alexander Shishkin
> >
> > This prevents ETM stalls whenever core enters OFF mode. Original patch
> > author is Richard Woodruff .
> >
> > This version of the patch makes use of the ETM OS save/restore mechanism,
> > which takes about 55 words in omap3_arm_context[] instead of 128. Also,
> > saving ETM context can be switched on/off at runtime.
> >
> > Signed-off-by: Alexander Shishkin
> > CC: Richard Woodruff
> > ---
> > arch/arm/mach-omap2/Kconfig |9 ++
> > arch/arm/mach-omap2/control.c |2 +-
> > arch/arm/mach-omap2/sleep34xx.S | 135
> > +
> > arch/arm/plat-omap/include/plat/control.h |2 +-
> > 4 files changed, 146 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> > index 2455dcc..5460bfe 100644
> > --- a/arch/arm/mach-omap2/Kconfig
> > +++ b/arch/arm/mach-omap2/Kconfig
> > @@ -150,6 +150,15 @@ config MACH_OMAP_4430SDP
> > bool "OMAP 4430 SDP board"
> > depends on ARCH_OMAP4
> >
> > +config ENABLE_OFF_MODE_JTAG_ETM_DEBUG
> > + bool "Enable hardware emulation context save and restore"
> > + depends on ARCH_OMAP3
> > + default y
> > + help
> > + This option enables JTAG & ETM debugging across power states.
> > + With out this option emulation features are reset across OFF
> > + mode state changes.
> > +
> > config OMAP3_EMU
> > bool "OMAP3 debugging peripherals"
> > depends on ARCH_OMAP3
> > diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
> > index 43f8a33..70b1674 100644
> > --- a/arch/arm/mach-omap2/control.c
> > +++ b/arch/arm/mach-omap2/control.c
> > @@ -93,7 +93,7 @@ void *omap3_secure_ram_storage;
> > * The address is stored in scratchpad, so that it can be used
> > * during the restore path.
> > */
> > -u32 omap3_arm_context[128];
> > +u32 omap3_arm_context[256];
> >
> > struct omap3_control_regs {
> > u32 sysconfig;
> > diff --git a/arch/arm/mach-omap2/sleep34xx.S
> > b/arch/arm/mach-omap2/sleep34xx.S
> > index d522cd7..cd6a1d4 100644
> > --- a/arch/arm/mach-omap2/sleep34xx.S
> > +++ b/arch/arm/mach-omap2/sleep34xx.S
> > @@ -28,6 +28,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #include "cm.h"
> > #include "prm.h"
> > @@ -226,6 +227,18 @@ loop:
> > nop
> > bl wait_sdrc_ok
> >
> > +#ifdef CONFIG_ENABLE_OFF_MODE_JTAG_ETM_DEBUG
> > + /*
> > +* Restore Coresight debug registers
> > +*/
> > + ldr r6, debug_vbase /* base Vaddr of CortexA8-Debug */
> > + ldr r4, debug_xlar_key /* get lock key for OSLAR */
> > + bl unlock_debug/* remove global lock if set */
> > + ldr r6, etm_vbase /* base Vaddr of ETM */
> > + bl unlock_debug/* remove global lock if set */
> > + str r6, [r6, #ETMMR_OSLAR] /* clear OSLAR lock using non-key */
> > +#endif
> > +
> > ldmfd sp!, {r0-r12, pc} @ restore regs and return
> > restore_es3:
> > /*b restore_es3*/ @ Enable to debug restore code
> > @@ -385,6 +398,44 @@ logic_l1_restore:
> > /*normal memory remap register */
> > MCR p15, 0, r5, c10, c2, 1
> >
> > +#ifdef CONFIG_ENABLE_OFF_MODE_JTAG_ETM_DEBUG
> > + /*
> > +* Restore Coresight debug registers
> > +*/
> > + ldr r6, debug_pbase /* base paddr of CortexA8-Debug */
> > + ldr r4, debug_xlar_key /* get lock key for OSLAR */
> > + bl unlock_debug/* remove global lock if set */
> > + str r4, [r6, #ETMMR_OSLAR] /* reset-pointer (already locked) */
> > + ldr r4, [r6, #ETMMR_OSSRR] /* dummy read */
> > + ldr r4, [r3], #4/* load save size */
> > + cmp r4, #0 /* check for zero */
> > +debug_restore:
> > +
