Skip to contentSkip to conversation

omap3: pm: Downgrade WARN for no wakeup source

PATCH New Sanjeev Premi 2 participants 5 messages Jun 16, 2011

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 omap3: pm: Downgrade WARN for no wakeup source

When multiple wakeup sources are defined in a system, there is a small window, when more than one source can trigger wakeup interrupt.

In the current implementation, the do-while() loop can handle all wakeup sources that are recorded when probing the status register in prcm_interrupt_handler().

When the ISR executes due to next queued wakeup, it there is nothing to be handled and value of "c" is 0.

The comment in the code, attributing this behavior to race with IVA2 along with WARN (and associated text) makes this condition appear as an error condition - while it isn't.

Though the problem was found in kernel version 2.6.32 running on AM3703 (no IVA2), it is still relevant.

This patch also fixes the comment that attributes current behavior to race with interrupt handler on IVA2.

Quoting specific instances, on entry into the ISR, the PM_WKST_WKUP read either 0x00010101 or 0x00010001

Where,
Bit 0 : ST_GPT1 - GPTIMER wakeup occurred
Bit 8 : ST_IO_EN - IO pad wakeup occurred
Bit 16 : ST_IO_CHAIN - The I/O wake-up scheme is enabled

This time value of "c" is 2 (or 3), but then on next and immediate entry, value of "c" is 0 leading to WARN().

Another approach I considered was to keep track of the number of wakeup sources handled in previous execution of ISR, but it fails when status register indicates 3 wakeup sources.

Downgrading the WARN seemed to be the simplest solution.

---
History:
v1: Original RFC posted:

     http://marc.info/?l=linux-omap&m=130754890807333&w=2
 arch/arm/mach-omap2/pm34xx.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)
arch/arm/mach-omap2/pm34xx.c +5 -4
index c155c9d..d24942b 100644
@@ -289,11 +289,12 @@ static irqreturn_t prcm_interrupt_handler (int irq, void *dev_id)
 			c = _prcm_int_handle_wakeup();
 
 			/*
-			 * Is the MPU PRCM interrupt handler racing with the
-			 * IVA2 PRCM interrupt handler ?
+			 * If multiple wakeup events get handled in certain
+			 * execution through this do-while, value of "c" will
+			 * be returned as 0.
 			 */
-			WARN(c == 0, "prcm: WARNING: PRCM indicated MPU wakeup "
-			     "but no wakeup sources are marked\n");
+			if (c == 0)
+				pr_debug ("prcm: no more wakeup event to handle") ;
 		} else {
 			/* XXX we need to expand our PRCM interrupt handler */
 			WARN(1, "prcm: WARNING: PRCM interrupt received, but "
Signature · 1 line
1.7.2.2
#1 lore raw

PATCH 1/1 omap3: pm: Downgrade WARN for no wakeup source

Hi Sanjeev,

Quoted 10 lines · Sanjeev Premi

When multiple wakeup sources are defined in a system, there is a small window, when more than one source can trigger wakeup interrupt.

In the current implementation, the do-while() loop can handle all wakeup sources that are recorded when probing the status register in prcm_interrupt_handler().

When the ISR executes due to next queued wakeup, it there is nothing to be handled and value of "c" is 0.

Thanks for tracking this one down.

However, It's still not clear to me what is happening here.

Why is the IRQ firing if there is nothing to be handled? That suggests to me that the IRQ status is not properly being cleared.

Kevin

#2 lore raw

PATCH 1/1 omap3: pm: Downgrade WARN for no wakeup source

Quoted 22 lines

-----Original Message-----
From: Hilman, Kevin
Sent: Thursday, June 16, 2011 8:52 PM
To: Premi, Sanjeev
Cc: linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] omap3: pm: Downgrade WARN for no wakeup source

Hi Sanjeev,

Quoted 10 lines · Sanjeev Premi

When multiple wakeup sources are defined in a system, there is a small window, when more than one source can trigger wakeup interrupt.

In the current implementation, the do-while() loop can handle all wakeup sources that are recorded when probing the status register in prcm_interrupt_handler().

When the ISR executes due to next queued wakeup, it there is nothing to be handled and value of "c" is 0.

Quoted 7 lines

Thanks for tracking this one down.

However, It's still not clear to me what is happening here.

Why is the IRQ firing if there is nothing to be handled? That suggests
to me that the IRQ status is not properly being cleared.

[SP] On the contrary the IRQ status is actually getting cleared,
but there are more than "1" PRCM interrupts queued.

The do-while() clears all the interrupt sources that are flagged in the status register.

When the next wakeup interrupt is getting processed, the status register doesn't contain any "source" to handle. and value of "c" is returned as 0.... leading to the warning.

~sanjeev

Quoted 3 lines

Kevin

#3 lore raw

PATCH 1/1 omap3: pm: Downgrade WARN for no wakeup source

Quoted 30 lines · Sanjeev
Quoted 22 lines

-----Original Message-----
From: Hilman, Kevin
Sent: Thursday, June 16, 2011 8:52 PM
To: Premi, Sanjeev
Cc: linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] omap3: pm: Downgrade WARN for no wakeup source

Hi Sanjeev,

Quoted 10 lines · Sanjeev Premi

When multiple wakeup sources are defined in a system, there is a small window, when more than one source can trigger wakeup interrupt.

In the current implementation, the do-while() loop can handle all wakeup sources that are recorded when probing the status register in prcm_interrupt_handler().

When the ISR executes due to next queued wakeup, it there is nothing to be handled and value of "c" is 0.

Quoted 7 lines

Thanks for tracking this one down.

However, It's still not clear to me what is happening here.

Why is the IRQ firing if there is nothing to be handled? That suggests
to me that the IRQ status is not properly being cleared.

Quoted 8 lines

[SP] On the contrary the IRQ status is actually getting cleared,
but there are more than "1" PRCM interrupts queued.

The do-while() clears all the interrupt sources that are flagged in the status register.

When the next wakeup interrupt is getting processed, the status register doesn't contain any "source" to handle.

Still confused.

If a wakeup interrupt is fired, it was caused by something.

IOW, if "the status register doesn't contain any source", how is

		if (irqstatus_mpu & (OMAP3430_WKUP_ST_MASK |
				     OMAP3430_IO_ST_MASK)) 

ever true?

Kevin

#4 lore raw

PATCH 1/1 omap3: pm: Downgrade WARN for no wakeup source

Quoted 51 lines

-----Original Message-----
From: Hilman, Kevin
Sent: Thursday, June 16, 2011 11:55 PM
To: Premi, Sanjeev
Cc: linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] omap3: pm: Downgrade WARN for no wakeup source

Quoted 5 lines · Sanjeev
Quoted 5 lines

-----Original Message-----
From: Hilman, Kevin
Sent: Thursday, June 16, 2011 8:52 PM
To: Premi, Sanjeev
Cc: linux-omap at vger.kernel.org;

linux-arm-kernel at lists.infradead.org

Quoted 25 lines
Quoted 17 lines

Subject: Re: [PATCH] omap3: pm: Downgrade WARN for no wakeup source

Hi Sanjeev,

Quoted 10 lines · Sanjeev Premi

When multiple wakeup sources are defined in a system, there is a small window, when more than one source can trigger wakeup interrupt.

In the current implementation, the do-while() loop can handle all wakeup sources that are recorded when probing the status register in prcm_interrupt_handler().

When the ISR executes due to next queued wakeup, it there is nothing to be handled and value of "c" is 0.

Quoted 7 lines

Thanks for tracking this one down.

However, It's still not clear to me what is happening here.

Why is the IRQ firing if there is nothing to be handled? That suggests
to me that the IRQ status is not properly being cleared.

Quoted 8 lines

[SP] On the contrary the IRQ status is actually getting cleared,
but there are more than "1" PRCM interrupts queued.

The do-while() clears all the interrupt sources that are flagged in the status register.

When the next wakeup interrupt is getting processed, the status register doesn't contain any "source" to handle.

Still confused.

Quoted 8 lines

If a wakeup interrupt is fired, it was caused by something.

IOW, if "the status register doesn't contain any source", how is

		if (irqstatus_mpu & (OMAP3430_WKUP_ST_MASK |
				     OMAP3430_IO_ST_MASK)) 

ever true?

[sp] I had been hesitant to say "nesting" because the wakeup
interrupts seem to be independent of each other. But now I think - from SW perspective, it should/could be considered as nesting.

~sanjeev

Quoted 3 lines

Kevin