GPIO debounce problems on 3.2

GPIO debounce problems on 3.2

[Grazvydas Ignotas]

Hi,

On 3.2 (I think some earlier versions too), with CONFIG_CPU_IDLE
enabled GPIO based buttons are not working properly on OMAP3 pandora,
button presses are almost never registered. The buttons are connected
GPIO bank4 and have hardware debounce feature enabled.

Doing either of the following solves (or hides) the problem:
- disabling CPU_IDLE in kernel config
- disabling debounce for the buttons
- running a program spinning a loop on the CPU

From what I can see in the code debounce clock is disabled when
entering idle, can those GPIOs work without debounce clock?


-- 
Gražvydas
--
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

Re: GPIO debounce problems on 3.2

[Kevin Hilman]

Grazvydas Ignotas <notasas@gmail.com> writes:

> Hi,
>
> On 3.2 (I think some earlier versions too), with CONFIG_CPU_IDLE
> enabled GPIO based buttons are not working properly on OMAP3 pandora,
> button presses are almost never registered. The buttons are connected
> GPIO bank4 and have hardware debounce feature enabled.
>
> Doing either of the following solves (or hides) the problem:
> - disabling CPU_IDLE in kernel config
> - disabling debounce for the buttons
> - running a program spinning a loop on the CPU
>
> From what I can see in the code debounce clock is disabled when
> entering idle, can those GPIOs work without debounce clock?

Yes, the clock is only for the debounce feature, but the GPIOs are
capable of wakeups and interrupts with the debounce clock disabled.

Kevin

Re: GPIO debounce problems on 3.2

[Grazvydas Ignotas]

On Mon, Jan 30, 2012 at 9:36 PM, Kevin Hilman <khilman@ti.com> wrote:
> Grazvydas Ignotas <notasas@gmail.com> writes:
>
>> Hi,
>>
>> On 3.2 (I think some earlier versions too), with CONFIG_CPU_IDLE
>> enabled GPIO based buttons are not working properly on OMAP3 pandora,
>> button presses are almost never registered. The buttons are connected
>> GPIO bank4 and have hardware debounce feature enabled.
>>
>> Doing either of the following solves (or hides) the problem:
>> - disabling CPU_IDLE in kernel config
>> - disabling debounce for the buttons
>> - running a program spinning a loop on the CPU
>>
>> From what I can see in the code debounce clock is disabled when
>> entering idle, can those GPIOs work without debounce clock?
>
> Yes, the clock is only for the debounce feature, but the GPIOs are
> capable of wakeups and interrupts with the debounce clock disabled.

Hmm but it doesn't work here (OMAP3530 ES2.1), /proc/interrupts
doesn't increase when I hit buttons unless something else is happening
at the same time. I wonder if I'm missing something here, does it all
work for you with debounce on? It can be easily tested if you add
.debounce_interval to gpio_keys_button of gpio_keys_platform_data.


-- 
Gražvydas
--
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

Re: GPIO debounce problems on 3.2

[Kevin Hilman]

Grazvydas Ignotas <notasas@gmail.com> writes:

> On Mon, Jan 30, 2012 at 9:36 PM, Kevin Hilman <khilman@ti.com> wrote:
>> Grazvydas Ignotas <notasas@gmail.com> writes:
>>
>>> Hi,
>>>
>>> On 3.2 (I think some earlier versions too), with CONFIG_CPU_IDLE
>>> enabled GPIO based buttons are not working properly on OMAP3 pandora,
>>> button presses are almost never registered. The buttons are connected
>>> GPIO bank4 and have hardware debounce feature enabled.
>>>
>>> Doing either of the following solves (or hides) the problem:
>>> - disabling CPU_IDLE in kernel config
>>> - disabling debounce for the buttons
>>> - running a program spinning a loop on the CPU
>>>
>>> From what I can see in the code debounce clock is disabled when
>>> entering idle, can those GPIOs work without debounce clock?
>>
>> Yes, the clock is only for the debounce feature, but the GPIOs are
>> capable of wakeups and interrupts with the debounce clock disabled.
>
> Hmm but it doesn't work here (OMAP3530 ES2.1), /proc/interrupts
> doesn't increase when I hit buttons unless something else is happening
> at the same time. I wonder if I'm missing something here, does it all
> work for you with debounce on? 

Yes.  

Specifically, I tested on a n900 which has several GPIOs in the board
file (board-rx51.c) with debounce enabled.

When I push the button (or slide the switch in this case), I see
/proc/interrupts incrementing on an idle system with CPUidle enabled.

The same GPIOs can also bring the system out of suspend (debounce clocks
are disabled on suspend also.)

Kevin

Re: GPIO debounce problems on 3.2

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2663 bytes --]

On Mon, 30 Jan 2012, Kevin Hilman wrote:

> Grazvydas Ignotas <notasas@gmail.com> writes:
> 
> > On Mon, Jan 30, 2012 at 9:36 PM, Kevin Hilman <khilman@ti.com> wrote:
> >> Grazvydas Ignotas <notasas@gmail.com> writes:
> >>
> >>> On 3.2 (I think some earlier versions too), with CONFIG_CPU_IDLE
> >>> enabled GPIO based buttons are not working properly on OMAP3 pandora,
> >>> button presses are almost never registered. The buttons are connected
> >>> GPIO bank4 and have hardware debounce feature enabled.
> >>>
> >>> Doing either of the following solves (or hides) the problem:
> >>> - disabling CPU_IDLE in kernel config
> >>> - disabling debounce for the buttons
> >>> - running a program spinning a loop on the CPU
> >>>
> >>> From what I can see in the code debounce clock is disabled when
> >>> entering idle, can those GPIOs work without debounce clock?
> >>
> >> Yes, the clock is only for the debounce feature, but the GPIOs are
> >> capable of wakeups and interrupts with the debounce clock disabled.
> >
> > Hmm but it doesn't work here (OMAP3530 ES2.1), /proc/interrupts
> > doesn't increase when I hit buttons unless something else is happening
> > at the same time. I wonder if I'm missing something here, does it all
> > work for you with debounce on? 
> 
> Yes.  
> 
> Specifically, I tested on a n900 which has several GPIOs in the board
> file (board-rx51.c) with debounce enabled.
> 
> When I push the button (or slide the switch in this case), I see
> /proc/interrupts incrementing on an idle system with CPUidle enabled.
> 
> The same GPIOs can also bring the system out of suspend (debounce clocks
> are disabled on suspend also.)

That's probably I/O ring/pad wakeups happening there, not GPIO wakeups.  
Gražvydas, you are referring to the case where the CORE powerdomain is on, 
but the GPIO IP block is idle?

Supposedly when the GPIO blocks enter idle with the debounce clock off, 
they enter a state that basically links the input pad to the GPIO block's 
SWAKEUP line.  Pure combinational logic, no clocks.  The TRM references, 
in the 34xx TRM Rev. ZT [SWPU223T], are Figure 25-7 "Asynchronous Path", 
Figure 25-9 "Wake-Up Request Generation", and Section 25.4.1.2 
"Asynchronous Path: Wake-Up Request Generation".

So apparently, looking at these references, the following registers should 
be configured:

1. at least one of GPIO_LEVELDETECT{0,1} or GPIO_{RISING,FALLING}DETECT

2. GPIO_WAKEUPENABLE

3. GPIO_IRQENABLE1

It seems highly suspicious to me that nothing in 
set_24xx_gpio_triggering() in our GPIO code seems to touch the 
WAKEUPENABLE register.


- Paul

Re: GPIO debounce problems on 3.2

[Kevin Hilman]

Paul Walmsley <paul@pwsan.com> writes:

> On Mon, 30 Jan 2012, Kevin Hilman wrote:
>
>> Grazvydas Ignotas <notasas@gmail.com> writes:
>> 
>> > On Mon, Jan 30, 2012 at 9:36 PM, Kevin Hilman <khilman@ti.com> wrote:
>> >> Grazvydas Ignotas <notasas@gmail.com> writes:
>> >>
>> >>> On 3.2 (I think some earlier versions too), with CONFIG_CPU_IDLE
>> >>> enabled GPIO based buttons are not working properly on OMAP3 pandora,
>> >>> button presses are almost never registered. The buttons are connected
>> >>> GPIO bank4 and have hardware debounce feature enabled.
>> >>>
>> >>> Doing either of the following solves (or hides) the problem:
>> >>> - disabling CPU_IDLE in kernel config
>> >>> - disabling debounce for the buttons
>> >>> - running a program spinning a loop on the CPU
>> >>>
>> >>> From what I can see in the code debounce clock is disabled when
>> >>> entering idle, can those GPIOs work without debounce clock?
>> >>
>> >> Yes, the clock is only for the debounce feature, but the GPIOs are
>> >> capable of wakeups and interrupts with the debounce clock disabled.
>> >
>> > Hmm but it doesn't work here (OMAP3530 ES2.1), /proc/interrupts
>> > doesn't increase when I hit buttons unless something else is happening
>> > at the same time. I wonder if I'm missing something here, does it all
>> > work for you with debounce on? 
>> 
>> Yes.  
>> 
>> Specifically, I tested on a n900 which has several GPIOs in the board
>> file (board-rx51.c) with debounce enabled.
>> 
>> When I push the button (or slide the switch in this case), I see
>> /proc/interrupts incrementing on an idle system with CPUidle enabled.
>> 
>> The same GPIOs can also bring the system out of suspend (debounce clocks
>> are disabled on suspend also.)
>
> That's probably I/O ring/pad wakeups happening there, not GPIO wakeups.  
> Gražvydas, you are referring to the case where the CORE powerdomain is on, 
> but the GPIO IP block is idle?

Right good point.

If CORE is powerdomain is staying on, you might also have a look at the
"GPIO module-level wakeups not always working" item in the "Known
problems" section of the OMAP PM wiki:

          http://elinux.org/OMAP_Power_Management#Known_Problems

It might be the case that haven't done a request_irq() +
enable_irq_wake() for those GPIOs.

Kevin
--
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

Re: GPIO debounce problems on 3.2

[Grazvydas Ignotas]

On Tue, Jan 31, 2012 at 2:22 AM, Kevin Hilman <khilman@ti.com> wrote:
> Paul Walmsley <paul@pwsan.com> writes:
>
>> That's probably I/O ring/pad wakeups happening there, not GPIO wakeups.
>> Gražvydas, you are referring to the case where the CORE powerdomain is on,
>> but the GPIO IP block is idle?
>
> Right good point.
>
> If CORE is powerdomain is staying on, you might also have a look at the
> "GPIO module-level wakeups not always working" item in the "Known
> problems" section of the OMAP PM wiki:
>
>          http://elinux.org/OMAP_Power_Management#Known_Problems
>
> It might be the case that haven't done a request_irq() +
> enable_irq_wake() for those GPIOs.

Well I'm just using standard gpio_keys driver, which does
request_threaded_irq(), I guess that's good enough? I've also tried
sticking enable_irq_wake() just after it but that didn't help.


-- 
Gražvydas
--
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

Re: GPIO debounce problems on 3.2

[Grazvydas Ignotas]

On Tue, Jan 31, 2012 at 1:34 AM, Paul Walmsley <paul@pwsan.com> wrote:
> On Mon, 30 Jan 2012, Kevin Hilman wrote:
>
>> Grazvydas Ignotas <notasas@gmail.com> writes:
>>
>> > Hmm but it doesn't work here (OMAP3530 ES2.1), /proc/interrupts
>> > doesn't increase when I hit buttons unless something else is happening
>> > at the same time. I wonder if I'm missing something here, does it all
>> > work for you with debounce on?
>>
>> Yes.
>>
>> Specifically, I tested on a n900 which has several GPIOs in the board
>> file (board-rx51.c) with debounce enabled.

Thanks for testing. Which kernel was that on, and what .config?

>> When I push the button (or slide the switch in this case), I see
>> /proc/interrupts incrementing on an idle system with CPUidle enabled.
>>
>> The same GPIOs can also bring the system out of suspend (debounce clocks
>> are disabled on suspend also.)
>
> That's probably I/O ring/pad wakeups happening there, not GPIO wakeups.
> Gražvydas, you are referring to the case where the CORE powerdomain is on,
> but the GPIO IP block is idle?

I think so, it's just after boot with cpuidle enabled.
/debug/pm_debug/count seems to confirm it.

> Supposedly when the GPIO blocks enter idle with the debounce clock off,
> they enter a state that basically links the input pad to the GPIO block's
> SWAKEUP line.  Pure combinational logic, no clocks.  The TRM references,
> in the 34xx TRM Rev. ZT [SWPU223T], are Figure 25-7 "Asynchronous Path",
> Figure 25-9 "Wake-Up Request Generation", and Section 25.4.1.2
> "Asynchronous Path: Wake-Up Request Generation".

ok..

> So apparently, looking at these references, the following registers should
> be configured:
>
> 1. at least one of GPIO_LEVELDETECT{0,1} or GPIO_{RISING,FALLING}DETECT
>
> 2. GPIO_WAKEUPENABLE
>
> 3. GPIO_IRQENABLE1

According to some /dev/mem hacking, all these have corresponding bits
set.. And as soon as I clear corresponding bit in GPIO_DEBOUNCENABLE
(using the same /dev/mem hacking method), it starts working. When it's
set, even GPIO_DATAIN
isn't updating in realtime.


-- 
Gražvydas
--
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

Re: GPIO debounce problems on 3.2

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1804 bytes --]

Hi Gražvydas

On Tue, 31 Jan 2012, Grazvydas Ignotas wrote:

> On Tue, Jan 31, 2012 at 1:34 AM, Paul Walmsley <paul@pwsan.com> wrote:
> 
> > So apparently, looking at these references, the following registers should
> > be configured:
> >
> > 1. at least one of GPIO_LEVELDETECT{0,1} or GPIO_{RISING,FALLING}DETECT
> >
> > 2. GPIO_WAKEUPENABLE
> >
> > 3. GPIO_IRQENABLE1
> 
> According to some /dev/mem hacking, all these have corresponding bits 
> set.. And as soon as I clear corresponding bit in GPIO_DEBOUNCENABLE 
> (using the same /dev/mem hacking method), it starts working. When it's 
> set, even GPIO_DATAIN isn't updating in realtime.

Hmm so I may have misunderstood the problem... 

If the DEBOUNCENABLE bit is set and the debounce clock is off and the GPIO 
IP block is idle, then yeah, I'd assume that GPIO wakeups would not 
work.

But it sounds like they are working for you when DEBOUNCENABLE is set and 
the debounce clock is on and the GPIO IP block is idle and CORE & PER are 
ON?

And when DEBOUNCENABLE is clear and the debounce clock is off and the GPIO 
IP block is idle and CORE & PER are still ON, then they work?

If the answer to those two latter questions are true, then it sounds like 
the hardware is working more or less as expected...

Of course our software support probably still isn't right.  We probably 
should keep the debounce clock enabled as long as some GPIO has debounce 
support enabled.  And probably provide some way for GPIO users to indicate 
whether they are okay with debounce support being disabled when the 
rest of the CORE/PER can enter a low-power state.  The motivating 
principle being that PM shouldn't violate a requirement from the rest of 
the system...

Anyway, just thinking out loud.


- Paul

Re: GPIO debounce problems on 3.2

[Grazvydas Ignotas]

On Tue, Jan 31, 2012 at 3:48 AM, Paul Walmsley <paul@pwsan.com> wrote:
>
> If the DEBOUNCENABLE bit is set and the debounce clock is off and the GPIO
> IP block is idle, then yeah, I'd assume that GPIO wakeups would not
> work.
>
> But it sounds like they are working for you when DEBOUNCENABLE is set and
> the debounce clock is on and the GPIO IP block is idle and CORE & PER are
> ON?

I think so. It works with DEBOUNCENABLE set when cpuidle is disabled
or CPU is spinning. CORE & PER are ON according to pm_debug/count, I
think debounce clock is enabled too but I don't know if GPIO is idle
then (how do I check?).

> And when DEBOUNCENABLE is clear and the debounce clock is off and the GPIO
> IP block is idle and CORE & PER are still ON, then they work?

Yes.

> If the answer to those two latter questions are true, then it sounds like
> the hardware is working more or less as expected...

So when does all wakeup stuff come into effect, when CORE+PER go to
low power state? I'm using 3.2 and CORE seems to always be ON
(regardless of cpuidle option) except when I suspend I guess. I can
get PER state to change if I set serial timeouts, but that doesn't
help the GPIO problem.

> Of course our software support probably still isn't right.  We probably
> should keep the debounce clock enabled as long as some GPIO has debounce
> support enabled.  And probably provide some way for GPIO users to indicate
> whether they are okay with debounce support being disabled when the
> rest of the CORE/PER can enter a low-power state.  The motivating
> principle being that PM shouldn't violate a requirement from the rest of
> the system...
>
> Anyway, just thinking out loud.
>
>
> - Paul

-- 
Gražvydas
--
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

Re: GPIO debounce problems on 3.2

[Grazvydas Ignotas]

On Tue, Jan 31, 2012 at 1:39 PM, Grazvydas Ignotas <notasas@gmail.com> wrote:
> So when does all wakeup stuff come into effect, when CORE+PER go to
> low power state? I'm using 3.2 and CORE seems to always be ON
> (regardless of cpuidle option) except when I suspend I guess. I can
> get PER state to change if I set serial timeouts, but that doesn't
> help the GPIO problem.

There is a strange thing in pm34xx.c, omap_sram_idle() function:
I've added logging of per_next_state, and it's always 1
(PWRDM_POWER_RET) for me, however if I check /debug/pm_debug/count ,
core_pwrdm and per_pwrdm only has 1 for ON and zeros for all other
states. That function calls gpio debounce clock disable if
per_next_state < PWRDM_POWER_ON.

Does that mean this code is preparing for RET but never enters it?
Maybe GPIO module ends up active but with debounce clock disabled?


-- 
Gražvydas
--
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

Re: GPIO debounce problems on 3.2

[Paul Walmsley]

On Tue, 31 Jan 2012, Grazvydas Ignotas wrote:

> On Tue, Jan 31, 2012 at 1:39 PM, Grazvydas Ignotas <notasas@gmail.com> wrote:
> > So when does all wakeup stuff come into effect, when CORE+PER go to
> > low power state? I'm using 3.2 and CORE seems to always be ON
> > (regardless of cpuidle option) except when I suspend I guess. I can
> > get PER state to change if I set serial timeouts, but that doesn't
> > help the GPIO problem.
> 
> There is a strange thing in pm34xx.c, omap_sram_idle() function:
> I've added logging of per_next_state, and it's always 1
> (PWRDM_POWER_RET) for me, however if I check /debug/pm_debug/count ,
> core_pwrdm and per_pwrdm only has 1 for ON and zeros for all other
> states. That function calls gpio debounce clock disable if
> per_next_state < PWRDM_POWER_ON.
> 
> Does that mean this code is preparing for RET but never enters it? 

Probably.  I think what's going on here is that you're hitting the 
hardwired sleep dependency between PER and CORE_L3.   This is documented 
briefly in the 34xx TRM Rev. ZT in Section 4.8.5.1 "Sleep Dependencies".
The PER clockdomain isn't allowed to go to sleep until the CORE_L3 
clockdomain goes to sleep.

Sounds like we may not be handling this correctly in the pm34xx.c code.  
As an aside, I've often thought that we should program the hardcoded sleep 
dependencies into the clockdomain data...

> Maybe GPIO module ends up active but with debounce clock disabled?

Well, whether it's in or out of idle, I'll bet the DEBOUNCENABLE 
bits are still set to 1 while the debounce clock is off :-(  


- Paul

Re: GPIO debounce problems on 3.2

[Paul Walmsley]

On Tue, 31 Jan 2012, Paul Walmsley wrote:

> Well, whether it's in or out of idle, I'll bet the DEBOUNCENABLE 
> bits are still set to 1 while the debounce clock is off :-(  

Along these lines, you might try patching omap2_gpio_prepare_for_idle() to 
turn off the DEBOUNCENABLE bits?

- Paul

Re: GPIO debounce problems on 3.2

[Grazvydas Ignotas]

On Wed, Feb 1, 2012 at 8:06 AM, Paul Walmsley <paul@pwsan.com> wrote:
> On Tue, 31 Jan 2012, Paul Walmsley wrote:
>
>> Well, whether it's in or out of idle, I'll bet the DEBOUNCENABLE
>> bits are still set to 1 while the debounce clock is off :-(
>
> Along these lines, you might try patching omap2_gpio_prepare_for_idle() to
> turn off the DEBOUNCENABLE bits?

Yes that helps. I guess I'll carry this in pandora tree, at least
we'll have debounce while more demanding programs/games are running.
Don't know how it's all working for Kevin though..


-- 
Gražvydas
--
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

Re: GPIO debounce problems on 3.2

[Kevin Hilman]

Grazvydas Ignotas <notasas@gmail.com> writes:

> On Wed, Feb 1, 2012 at 8:06 AM, Paul Walmsley <paul@pwsan.com> wrote:
>> On Tue, 31 Jan 2012, Paul Walmsley wrote:
>>
>>> Well, whether it's in or out of idle, I'll bet the DEBOUNCENABLE
>>> bits are still set to 1 while the debounce clock is off :-(
>>
>> Along these lines, you might try patching omap2_gpio_prepare_for_idle() to
>> turn off the DEBOUNCENABLE bits?
>
> Yes that helps. I guess I'll carry this in pandora tree, at least
> we'll have debounce while more demanding programs/games are running.

Care to share the patch?  It might be worth merging until we have a full
solution.

> Don't know how it's all working for Kevin though..

Now I'm not so sure either...

At first I thought it was because I was getting IO ring wakeups, but
using vanilla 3.2, MPU, CORE & PER are staying on due to the (broken)
UART constraint, so I'm getting module-level wakeups, but haven't dug
into why.

Kevin

Re: GPIO debounce problems on 3.2

[Grazvydas Ignotas]

[-- Attachment #1: Type: text/plain, Size: 1014 bytes --]

On Wed, Feb 1, 2012 at 5:30 PM, Kevin Hilman <khilman@ti.com> wrote:
> Grazvydas Ignotas <notasas@gmail.com> writes:
>
>> On Wed, Feb 1, 2012 at 8:06 AM, Paul Walmsley <paul@pwsan.com> wrote:
>>>
>>> Along these lines, you might try patching omap2_gpio_prepare_for_idle() to
>>> turn off the DEBOUNCENABLE bits?
>>
>> Yes that helps. I guess I'll carry this in pandora tree, at least
>> we'll have debounce while more demanding programs/games are running.
>
> Care to share the patch?  It might be worth merging until we have a full
> solution.

Attached.

>> Don't know how it's all working for Kevin though..
>
> Now I'm not so sure either...
>
> At first I thought it was because I was getting IO ring wakeups, but
> using vanilla 3.2, MPU, CORE & PER are staying on due to the (broken)
> UART constraint, so I'm getting module-level wakeups, but haven't dug
> into why.

Maybe the CPU is sometimes busy and debounce clock is enabled long
enough for it to work.


-- 
Gražvydas

[-- Attachment #2: 0001-gpio-omap-disable-debounce-on-idle.patch --]
[-- Type: text/x-diff, Size: 1865 bytes --]

From f384b2b48970db36a751f454d1ef10910775b5d2 Mon Sep 17 00:00:00 2001
From: Grazvydas Ignotas <notasas@gmail.com>
Date: Wed, 1 Feb 2012 15:10:52 +0200
Subject: [PATCH] gpio/omap: disable debounce on idle

After omap2_gpio_prepare_for_idle call it's not guaranteed that given
GPIO module will hit idle, so it might end up in active state but with
debounce clock disabled, which will break input functionality.

Disable debounce feature on idle to avoid this problem.

Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
---
 drivers/gpio/gpio-omap.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 0b05629..4f8e8ba 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1347,9 +1347,19 @@ void omap2_gpio_prepare_for_idle(int off_mode)
 
 	for (i = min; i < gpio_bank_count; i++) {
 		struct gpio_bank *bank = &gpio_bank[i];
+		void __iomem *reg;
 		u32 l1 = 0, l2 = 0;
 		int j;
 
+		/*
+		 * Disable debounce since clock disable below will cause
+		 * problems if GPIO module doesn't go idle for some reason.
+		 */
+		if (bank->dbck_enable_mask != 0) {
+			reg = bank->base + bank->regs->debounce_en;
+			__raw_writel(0, reg);
+		}
+
 		for (j = 0; j < hweight_long(bank->dbck_enable_mask); j++)
 			clk_disable(bank->dbck);
 
@@ -1415,12 +1425,18 @@ void omap2_gpio_resume_after_idle(void)
 		min = 1;
 	for (i = min; i < gpio_bank_count; i++) {
 		struct gpio_bank *bank = &gpio_bank[i];
+		void __iomem *reg;
 		u32 l = 0, gen, gen0, gen1;
 		int j;
 
 		for (j = 0; j < hweight_long(bank->dbck_enable_mask); j++)
 			clk_enable(bank->dbck);
 
+		if (bank->dbck_enable_mask != 0) {
+			reg = bank->base + bank->regs->debounce_en;
+			__raw_writel(bank->dbck_enable_mask, reg);
+		}
+
 		if (!workaround_enabled)
 			continue;
 
-- 
1.7.0.4

Re: GPIO debounce problems on 3.2

[Paul Walmsley]

On Wed, 1 Feb 2012, Grazvydas Ignotas wrote:

> On Wed, Feb 1, 2012 at 8:06 AM, Paul Walmsley <paul@pwsan.com> wrote:
> > On Tue, 31 Jan 2012, Paul Walmsley wrote:
> >
> >> Well, whether it's in or out of idle, I'll bet the DEBOUNCENABLE
> >> bits are still set to 1 while the debounce clock is off :-(
> >
> > Along these lines, you might try patching omap2_gpio_prepare_for_idle() to
> > turn off the DEBOUNCENABLE bits?
> 
> Yes that helps. I guess I'll carry this in pandora tree, at least
> we'll have debounce while more demanding programs/games are running.

Yep definitely.  When only I/O wakeups can occur on those GPIO lines (when 
CORE/PER is in a low-power state), they are intrinsically debounced :-)  
So it should be fine.  You are re-enabling the DEBOUNCENABLE bits in 
omap2_gpio_resume_after_idle() I assume?  If so, that should be a good 
workaround for mainline too.


- Paul

Re: GPIO debounce problems on 3.2

[Grazvydas Ignotas]

On Wed, Feb 1, 2012 at 8:41 PM, Paul Walmsley <paul@pwsan.com> wrote:
> On Wed, 1 Feb 2012, Grazvydas Ignotas wrote:
>
>> On Wed, Feb 1, 2012 at 8:06 AM, Paul Walmsley <paul@pwsan.com> wrote:
>> > On Tue, 31 Jan 2012, Paul Walmsley wrote:
>> >
>> >> Well, whether it's in or out of idle, I'll bet the DEBOUNCENABLE
>> >> bits are still set to 1 while the debounce clock is off :-(
>> >
>> > Along these lines, you might try patching omap2_gpio_prepare_for_idle() to
>> > turn off the DEBOUNCENABLE bits?
>>
>> Yes that helps. I guess I'll carry this in pandora tree, at least
>> we'll have debounce while more demanding programs/games are running.
>
> Yep definitely.  When only I/O wakeups can occur on those GPIO lines (when
> CORE/PER is in a low-power state), they are intrinsically debounced :-)
> So it should be fine.  You are re-enabling the DEBOUNCENABLE bits in
> omap2_gpio_resume_after_idle() I assume?  If so, that should be a good
> workaround for mainline too.

To follow up on this, it appears clearing DEBOUNCENABLE in
omap2_gpio_prepare_for_idle (or omap_gpio_runtime_suspend in 3.4) and
reenabling on resume causes some sort of irq storm, I get ~50 irqs on
key press/release for given GPIO, which causes CPU usage spikes, but
no duplicate input events, curiously. Obviously this is not good for
UIs and games, so I have nothing left but to disable hardware debounce
now :(



-- 
Gražvydas
--
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

Re: GPIO debounce problems on 3.2

[Paul Walmsley]

On Tue, 31 Jan 2012, Grazvydas Ignotas wrote:

> On Tue, Jan 31, 2012 at 3:48 AM, Paul Walmsley <paul@pwsan.com> wrote:
> >
> > If the DEBOUNCENABLE bit is set and the debounce clock is off and the GPIO
> > IP block is idle, then yeah, I'd assume that GPIO wakeups would not
> > work.
> >
> > But it sounds like they are working for you when DEBOUNCENABLE is set and
> > the debounce clock is on and the GPIO IP block is idle and CORE & PER are
> > ON?
> 
> I think so. It works with DEBOUNCENABLE set when cpuidle is disabled
> or CPU is spinning. CORE & PER are ON according to pm_debug/count, I
> think debounce clock is enabled too but I don't know if GPIO is idle
> then (how do I check?).

Maybe try reading the appropriate CM_IDLEST bit?  I can't remember at the 
moment if this represents the PRCM's SIdleReq signal or the module's 
SIdleAck, but it's worth a quick look.

> > And when DEBOUNCENABLE is clear and the debounce clock is off and the GPIO
> > IP block is idle and CORE & PER are still ON, then they work?
> 
> Yes.
> 
> > If the answer to those two latter questions are true, then it sounds like
> > the hardware is working more or less as expected...
> 
> So when does all wakeup stuff come into effect, when CORE+PER go to
> low power state?

>From the perspective of the GPIO IP block, it comes into effect when the 
GPIO module has IdleAcked ("is idle").  At that point the module needs to 
assert its SWAKEUP signal when it wants to have its clocks re-enabled, 
wake up other parts of the system, etc.  Maybe try searching the 34xx TRM 
for SWAKEUP; there are a few bits and pieces scattered around that might 
help build a larger mosaic :-)  As far as CORE & PER powerstates go, the 
CORE & PER still might be ON while the module is idle; that's a separate 
thing.

Let me also answer the question from the MPU's perspective.  Suppose the 
MPU powerdomain has entered a low power state.  That means that the MPU 
INTC -- part of the MPU powerdomain -- is also in a low power state.  So 
neither the MPU nor the MPU INTC are working: clocks are disabled, the 
voltage may be scaled down, etc.  So even if an IP block elsewhere on the 
chip asserts an MPU interrupt, the MPU INTC won't notice it; it's 
non-functional at this point.  So for the MPU to notice the interrupt, it 
has to first come out of its low-power state.  That happens when some IP 
block asserts that SWAKEUP signal to the PRCM, which, if it's programmed 
correctly, will then bring the MPU powerdomain out of its low-power state, 
re-enable clocks, etc.  At that point, the MPU INTC should notice the 
interrupt, and the kernel should take it from there.


- Paul

Re: GPIO debounce problems on 3.2

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 1490 bytes --]

On Tue, 31 Jan 2012 22:47:32 -0700 (MST) Paul Walmsley <paul@pwsan.com> wrote:

> Let me also answer the question from the MPU's perspective.  Suppose the 
> MPU powerdomain has entered a low power state.  That means that the MPU 
> INTC -- part of the MPU powerdomain -- is also in a low power state. 

My TRM says - in section 12.3.1.3 Power Management

    The MPU subsystem INTC belongs to the CORE power domain.

This is:

      AM/DM37x Multimedia Device
      Silicon Revision 1.x
      Version N

Is it wrong, are you wrong, or am I confused?

Thanks,
NeilBrown


 So 
> neither the MPU nor the MPU INTC are working: clocks are disabled, the 
> voltage may be scaled down, etc.  So even if an IP block elsewhere on the 
> chip asserts an MPU interrupt, the MPU INTC won't notice it; it's 
> non-functional at this point.  So for the MPU to notice the interrupt, it 
> has to first come out of its low-power state.  That happens when some IP 
> block asserts that SWAKEUP signal to the PRCM, which, if it's programmed 
> correctly, will then bring the MPU powerdomain out of its low-power state, 
> re-enable clocks, etc.  At that point, the MPU INTC should notice the 
> interrupt, and the kernel should take it from there.
> 
> 
> - Paul
> --
> 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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: GPIO debounce problems on 3.2

[Paul Walmsley]

On Wed, 1 Feb 2012, NeilBrown wrote:

> On Tue, 31 Jan 2012 22:47:32 -0700 (MST) Paul Walmsley <paul@pwsan.com> wrote:
> 
> > Let me also answer the question from the MPU's perspective.  Suppose the 
> > MPU powerdomain has entered a low power state.  That means that the MPU 
> > INTC -- part of the MPU powerdomain -- is also in a low power state. 
> 
> My TRM says - in section 12.3.1.3 Power Management
> 
>     The MPU subsystem INTC belongs to the CORE power domain.
> 
> This is:
> 
>       AM/DM37x Multimedia Device
>       Silicon Revision 1.x
>       Version N
> 
> Is it wrong, are you wrong, or am I confused?

Hmm.  Since section 3.3.2.1 "Power Domains" mentions this too, and refers 
to CORE_RST, I suspect you and the TRM are right.  

So, for the moment, let's strike that second paragraph that I wrote about 
MPU wakeups.  I need to doublecheck with some other sources about exactly 
where the MPU INTC's clock comes from when the MPU clockdomain is idle.


- Paul

Re: GPIO debounce problems on 3.2

[Paul Walmsley]

On Tue, 31 Jan 2012, Grazvydas Ignotas wrote:

> On Tue, Jan 31, 2012 at 1:34 AM, Paul Walmsley <paul@pwsan.com> wrote:
> 
> > So apparently, looking at these references, the following registers should
> > be configured:
> >
> > 1. at least one of GPIO_LEVELDETECT{0,1} or GPIO_{RISING,FALLING}DETECT
> >
> > 2. GPIO_WAKEUPENABLE
> >
> > 3. GPIO_IRQENABLE1
> 
> According to some /dev/mem hacking, all these have corresponding bits
> set.

OK so that's good, something in the GPIO driver is setting those 
WAKEUPENABLE bits then :-)


- Paul

Re: GPIO debounce problems on 3.2

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 609 bytes --]

On Mon, 30 Jan 2012, Paul Walmsley wrote:

> It seems highly suspicious to me that nothing in 
> set_24xx_gpio_triggering() in our GPIO code seems to touch the 
> WAKEUPENABLE register.

Heh.  Just wanted to follow up on this for anyone who cares.  Gražvydas 
noticed that the bit was indeed being set, so was curious to find out why.  
It turns out that, like some other GPIO regs, there is another pair of 
registers that can be used to modify the wakeup state without a 
read-modify-write cycle, and the GPIO driver uses these other two regs - 
GPIO_SETWKUENA and GPIO_CLEARWKUENA.

 
 - Paul