Message ID | 1322685811-14060-1-git-send-email-riyer@nvidia.com |
---|---|
State | Superseded, archived |
Headers | show |
Rakesh Iyer wrote at Wednesday, November 30, 2011 1:44 PM: > Tegra kbc cannot detect exact keypress causing wakeup in interrupt mode. > Allow wakeup keypress to be reported for certain platforms. > > Signed-off-by: Rakesh Iyer <riyer@nvidia.com> Acked-by: Stephen Warren <swarren@nvidia.com>
Hi Rakesh, On Wed, Nov 30, 2011 at 12:43:31PM -0800, riyer@nvidia.com wrote: > From: Rakesh Iyer <riyer@nvidia.com> > > Tegra kbc cannot detect exact keypress causing wakeup in interrupt mode. > Allow wakeup keypress to be reported for certain platforms. > > Signed-off-by: Rakesh Iyer <riyer@nvidia.com> > --- > arch/arm/mach-tegra/include/mach/kbc.h | 1 + > drivers/input/keyboard/tegra-kbc.c | 20 ++++++++++++++++---- > 2 files changed, 17 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/mach-tegra/include/mach/kbc.h b/arch/arm/mach-tegra/include/mach/kbc.h > index 4f3572a..20bb054 100644 > --- a/arch/arm/mach-tegra/include/mach/kbc.h > +++ b/arch/arm/mach-tegra/include/mach/kbc.h > @@ -53,6 +53,7 @@ struct tegra_kbc_platform_data { > struct tegra_kbc_pin_cfg pin_cfg[KBC_MAX_GPIO]; > const struct matrix_keymap_data *keymap_data; > > + u32 wakeup_key; > bool wakeup; > bool use_fn_map; > bool use_ghost_filter; > diff --git a/drivers/input/keyboard/tegra-kbc.c b/drivers/input/keyboard/tegra-kbc.c > index cf3228b..ceb1185 100644 > --- a/drivers/input/keyboard/tegra-kbc.c > +++ b/drivers/input/keyboard/tegra-kbc.c > @@ -52,6 +52,7 @@ > /* KBC Interrupt Register */ > #define KBC_INT_0 0x4 > #define KBC_INT_FIFO_CNT_INT_STATUS (1 << 2) > +#define KBC_INT_KEYPRESS_INT_STATUS (1 << 0) > > #define KBC_ROW_CFG0_0 0x8 > #define KBC_COL_CFG0_0 0x18 > @@ -74,10 +75,12 @@ struct tegra_kbc { > unsigned int cp_to_wkup_dly; > bool use_fn_map; > bool use_ghost_filter; > + bool keypress_caused_wake; > const struct tegra_kbc_platform_data *pdata; > unsigned short keycode[KBC_MAX_KEY * 2]; > unsigned short current_keys[KBC_MAX_KPENT]; > unsigned int num_pressed_keys; > + u32 wakeup_key; > struct timer_list timer; > struct clk *clk; > }; > @@ -409,6 +412,9 @@ static irqreturn_t tegra_kbc_isr(int irq, void *args) > */ > tegra_kbc_set_fifo_interrupt(kbc, false); > mod_timer(&kbc->timer, jiffies + kbc->cp_dly_jiffies); > + } else if (val & KBC_INT_KEYPRESS_INT_STATUS) { > + /* We can be here only through system resume path */ > + kbc->keypress_caused_wake = true; > } > > spin_unlock_irqrestore(&kbc->lock, flags); > @@ -674,9 +680,10 @@ static int __devinit tegra_kbc_probe(struct platform_device *pdev) > keymap_data = pdata->keymap_data ?: &tegra_kbc_default_keymap_data; > matrix_keypad_build_keymap(keymap_data, KBC_ROW_SHIFT, > input_dev->keycode, input_dev->keybit); > + kbc->wakeup_key = pdata->wakeup_key; > > - err = request_irq(kbc->irq, tegra_kbc_isr, IRQF_TRIGGER_HIGH, > - pdev->name, kbc); > + err = request_irq(kbc->irq, tegra_kbc_isr, > + IRQF_NO_SUSPEND | IRQF_TRIGGER_HIGH, pdev->name, kbc); > if (err) { > dev_err(&pdev->dev, "failed to request keyboard IRQ\n"); > goto err_put_clk; > @@ -738,7 +745,6 @@ static int tegra_kbc_suspend(struct device *dev) > > mutex_lock(&kbc->idev->mutex); > if (device_may_wakeup(&pdev->dev)) { > - disable_irq(kbc->irq); > del_timer_sync(&kbc->timer); > tegra_kbc_set_fifo_interrupt(kbc, false); This disturbs locking rules and allows timer to run past this point. Instead of keeping interrupt enabled can't you simply read controller state in tegra_kbc_resume (before enabling interrupt) and emit the keycode if you detect KBC_INT_KEYPRESS_INT_STATUS condition? Thanks.
Thanks Dmitry. Since there can be multiple wake causes I wanted to isolate wake key generation to the case where keyboard actually generated the wake interrupt. If the system was woken by some other source and keypress occurs before resume is invoked we will pass on an unintended wake key. Leaving the disable_irq as is, can I can enable the interrupts at the end of the suspend routine? Note with the FIFO interrupt disabled, there will be no interrupt after that point from the device unless it's a Wake interrupt. > -----Original Message----- > From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com] > Sent: Wednesday, November 30, 2011 1:20 PM > To: Rakesh Iyer > Cc: rydberg@euromail.se; Stephen Warren; Laxman Dewangan; linux- > kernel@vger.kernel.org; linux-input@vger.kernel.org; linux-tegra@vger.kernel.org > Subject: Re: [PATCH v1] Input: tegra-kbc - report wakeup key for some platforms. > > Hi Rakesh, > > On Wed, Nov 30, 2011 at 12:43:31PM -0800, riyer@nvidia.com wrote: > > From: Rakesh Iyer <riyer@nvidia.com> > > > > Tegra kbc cannot detect exact keypress causing wakeup in interrupt mode. > > Allow wakeup keypress to be reported for certain platforms. > > > > Signed-off-by: Rakesh Iyer <riyer@nvidia.com> > > --- > > arch/arm/mach-tegra/include/mach/kbc.h | 1 + > > drivers/input/keyboard/tegra-kbc.c | 20 ++++++++++++++++---- > > 2 files changed, 17 insertions(+), 4 deletions(-) > > > > diff --git a/arch/arm/mach-tegra/include/mach/kbc.h b/arch/arm/mach- > tegra/include/mach/kbc.h > > index 4f3572a..20bb054 100644 > > --- a/arch/arm/mach-tegra/include/mach/kbc.h > > +++ b/arch/arm/mach-tegra/include/mach/kbc.h > > @@ -53,6 +53,7 @@ struct tegra_kbc_platform_data { > > struct tegra_kbc_pin_cfg pin_cfg[KBC_MAX_GPIO]; > > const struct matrix_keymap_data *keymap_data; > > > > + u32 wakeup_key; > > bool wakeup; > > bool use_fn_map; > > bool use_ghost_filter; > > diff --git a/drivers/input/keyboard/tegra-kbc.c b/drivers/input/keyboard/tegra-kbc.c > > index cf3228b..ceb1185 100644 > > --- a/drivers/input/keyboard/tegra-kbc.c > > +++ b/drivers/input/keyboard/tegra-kbc.c > > @@ -52,6 +52,7 @@ > > /* KBC Interrupt Register */ > > #define KBC_INT_0 0x4 > > #define KBC_INT_FIFO_CNT_INT_STATUS (1 << 2) > > +#define KBC_INT_KEYPRESS_INT_STATUS (1 << 0) > > > > #define KBC_ROW_CFG0_0 0x8 > > #define KBC_COL_CFG0_0 0x18 > > @@ -74,10 +75,12 @@ struct tegra_kbc { > > unsigned int cp_to_wkup_dly; > > bool use_fn_map; > > bool use_ghost_filter; > > + bool keypress_caused_wake; > > const struct tegra_kbc_platform_data *pdata; > > unsigned short keycode[KBC_MAX_KEY * 2]; > > unsigned short current_keys[KBC_MAX_KPENT]; > > unsigned int num_pressed_keys; > > + u32 wakeup_key; > > struct timer_list timer; > > struct clk *clk; > > }; > > @@ -409,6 +412,9 @@ static irqreturn_t tegra_kbc_isr(int irq, void *args) > > */ > > tegra_kbc_set_fifo_interrupt(kbc, false); > > mod_timer(&kbc->timer, jiffies + kbc->cp_dly_jiffies); > > + } else if (val & KBC_INT_KEYPRESS_INT_STATUS) { > > + /* We can be here only through system resume path */ > > + kbc->keypress_caused_wake = true; > > } > > > > spin_unlock_irqrestore(&kbc->lock, flags); > > @@ -674,9 +680,10 @@ static int __devinit tegra_kbc_probe(struct platform_device > *pdev) > > keymap_data = pdata->keymap_data ?: &tegra_kbc_default_keymap_data; > > matrix_keypad_build_keymap(keymap_data, KBC_ROW_SHIFT, > > input_dev->keycode, input_dev->keybit); > > + kbc->wakeup_key = pdata->wakeup_key; > > > > - err = request_irq(kbc->irq, tegra_kbc_isr, IRQF_TRIGGER_HIGH, > > - pdev->name, kbc); > > + err = request_irq(kbc->irq, tegra_kbc_isr, > > + IRQF_NO_SUSPEND | IRQF_TRIGGER_HIGH, pdev->name, > kbc); > > if (err) { > > dev_err(&pdev->dev, "failed to request keyboard IRQ\n"); > > goto err_put_clk; > > @@ -738,7 +745,6 @@ static int tegra_kbc_suspend(struct device *dev) > > > > mutex_lock(&kbc->idev->mutex); > > if (device_may_wakeup(&pdev->dev)) { > > - disable_irq(kbc->irq); > > del_timer_sync(&kbc->timer); > > tegra_kbc_set_fifo_interrupt(kbc, false); > > This disturbs locking rules and allows timer to run past this point. > Instead of keeping interrupt enabled can't you simply read controller > state in tegra_kbc_resume (before enabling interrupt) and emit the > keycode if you detect KBC_INT_KEYPRESS_INT_STATUS condition? > > Thanks. > > -- > Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Nov 30, 2011 at 02:14:03PM -0800, Rakesh Iyer wrote: > Thanks Dmitry. > > Since there can be multiple wake causes I wanted to isolate wake key > generation to the case where keyboard actually generated the wake > interrupt. I do not think you can guarantee this though because if user touches keyboard "too early", before your resume method had a chance to disable kbc interrupt as a wakeup source, you are still going to get that interrupt and deliver KEY_POWER even though KBC is not the actual wakeup source. So don't over-complicate it. If hardware can't detect actual key pressed just emit KEY_POWER if a key was at any time between calls to tegra_kbc_suspend() and tegra_kbc_resume(). BTW, could you please have your MUA wrap long lines around 75 column or so? Thanks.
Hello Dmitry. Sorry for the wrap issue, my Outlook does not seem to obey the settings. I wanted to explain the tegra system resume path implementation so I can justify why I am doing this complicated fix and why I feel it will guarantee the resume is due to keypress. The tegra wake resume code is registered as a syscore ops. When the system is resumed due to a wake event, the suspend_enter (after wakeup) routine will invoke the tegra syscoreops_resume method and that routine will propagate the wake event to the individual ISR's through genirq. If kbc was wake source, kbc_isr will be invoked in this execution path. If system is resumed due to other reason, the tegra_syscoreops_resume code will not find the event. In the kbc we ignore all keypresses until kbc_resume re-enables the fifo interrupt. So the only way to generate this key would be if the tegra_syscoreops_resume finds that kbc was wake source Maybe my understanding is wrong. Please feel free to correct me. Regards Rakesh > -----Original Message----- > From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com] > Sent: Wednesday, November 30, 2011 11:27 PM > To: Rakesh Iyer > Cc: rydberg@euromail.se; Stephen Warren; Laxman Dewangan; linux- > kernel@vger.kernel.org; linux-input@vger.kernel.org; linux-tegra@vger.kernel.org > Subject: Re: [PATCH v1] Input: tegra-kbc - report wakeup key for some platforms. > > On Wed, Nov 30, 2011 at 02:14:03PM -0800, Rakesh Iyer wrote: > > Thanks Dmitry. > > > > Since there can be multiple wake causes I wanted to isolate wake key > > generation to the case where keyboard actually generated the wake > > interrupt. > > I do not think you can guarantee this though because if user touches > keyboard "too early", before your resume method had a chance to disable > kbc interrupt as a wakeup source, you are still going to get that > interrupt and deliver KEY_POWER even though KBC is not the actual wakeup > source. > > So don't over-complicate it. If hardware can't detect actual key pressed > just emit KEY_POWER if a key was at any time between calls to > tegra_kbc_suspend() and tegra_kbc_resume(). > > BTW, could you please have your MUA wrap long lines around 75 column or so? > > Thanks. > > -- > Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rakesh, On Thu, Dec 01, 2011 at 01:09:59PM -0800, Rakesh Iyer wrote: > Hello Dmitry. > > Sorry for the wrap issue, my Outlook does not seem to obey the settings. > > I wanted to explain the tegra system resume path implementation so I can justify > why I am doing this complicated fix and why I feel it will guarantee the resume > is due to keypress. > > The tegra wake resume code is registered as a syscore ops. > When the system is resumed due to a wake event, the suspend_enter (after wakeup) > routine will invoke the tegra syscoreops_resume method and that routine will propagate > the wake event to the individual ISR's through genirq. > If kbc was wake source, kbc_isr will be invoked in this execution path. > > If system is resumed due to other reason, the tegra_syscoreops_resume code will not > find the event. Consider the following sequence: 1. Something other than keyboard generates wakeup event 2. It's IRQ fires up and gets serviced 3. System starts resuming devices 4. User presses a key on the keypad while it is still suspended _and_ registered as a wakeup source 5. Keypad's ISR runs as well and you decide that KEY_POWER should be reported even though keypad wasn't the real reason the system woke up. Is this scenario not possible? Thanks.
Hello Dmitry. Please find replies inline. On Sun, 2011-12-04 at 00:50 -0800, Dmitry Torokhov wrote: > Hi Rakesh, > > On Thu, Dec 01, 2011 at 01:09:59PM -0800, Rakesh Iyer wrote: > > Hello Dmitry. > > > > Sorry for the wrap issue, my Outlook does not seem to obey the settings. > > > > I wanted to explain the tegra system resume path implementation so I can justify > > why I am doing this complicated fix and why I feel it will guarantee the resume > > is due to keypress. > > > > The tegra wake resume code is registered as a syscore ops. > > When the system is resumed due to a wake event, the suspend_enter (after wakeup) > > routine will invoke the tegra syscoreops_resume method and that routine will propagate > > the wake event to the individual ISR's through genirq. > > If kbc was wake source, kbc_isr will be invoked in this execution path. > > > > If system is resumed due to other reason, the tegra_syscoreops_resume code will not > > find the event. > > Consider the following sequence: > > 1. Something other than keyboard generates wakeup event > 2. It's IRQ fires up and gets serviced At this point syscoreops_resume has finished all its wakeup processing. > 3. System starts resuming devices > 4. User presses a key on the keypad while it is still suspended _and_ > registered as a wakeup source This will have no impact on the system and keypad ISR will not be invoked. > 5. Keypad's ISR runs as well and you decide that KEY_POWER should be > reported even though keypad wasn't the real reason the system > woke up. The interrupt line we use to detect wakeup processing is the keypress interrupt which is disabled and will never cause the ISR invocation from a device interrupt(i.e. the PIC). In other words KBC isr gets invoked for 2 reasons a) FIFO interrupt is generated, which will not happen as long as scanning logic is disabled until kbc_resume executes. b) The syscoreops_resume codepath calls into the ISR after finding KBC was a wakesource. If this happens it will happen only when the kernel resumes from its suspend code path. > > Is this scenario not possible? > > Thanks. > With that being the case do you think the fix makes sense? Regards Rakesh -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Dec 04, 2011 at 05:18:17PM -0800, riyer wrote: > Hello Dmitry. > Please find replies inline. > > On Sun, 2011-12-04 at 00:50 -0800, Dmitry Torokhov wrote: > > Hi Rakesh, > > > > On Thu, Dec 01, 2011 at 01:09:59PM -0800, Rakesh Iyer wrote: > > > Hello Dmitry. > > > > > > Sorry for the wrap issue, my Outlook does not seem to obey the settings. > > > > > > I wanted to explain the tegra system resume path implementation so I can justify > > > why I am doing this complicated fix and why I feel it will guarantee the resume > > > is due to keypress. > > > > > > The tegra wake resume code is registered as a syscore ops. > > > When the system is resumed due to a wake event, the suspend_enter (after wakeup) > > > routine will invoke the tegra syscoreops_resume method and that routine will propagate > > > the wake event to the individual ISR's through genirq. > > > If kbc was wake source, kbc_isr will be invoked in this execution path. > > > > > > If system is resumed due to other reason, the tegra_syscoreops_resume code will not > > > find the event. > > > > Consider the following sequence: > > > > 1. Something other than keyboard generates wakeup event > > 2. It's IRQ fires up and gets serviced > At this point syscoreops_resume has finished all its wakeup processing. > > > 3. System starts resuming devices > > 4. User presses a key on the keypad while it is still suspended _and_ > > registered as a wakeup source > This will have no impact on the system and keypad ISR will not be > invoked. > > 5. Keypad's ISR runs as well and you decide that KEY_POWER should be > > reported even though keypad wasn't the real reason the system > > woke up. > The interrupt line we use to detect wakeup processing is the keypress > interrupt which is disabled and will never cause the ISR invocation from > a device interrupt(i.e. the PIC). > > In other words KBC isr gets invoked for 2 reasons > a) FIFO interrupt is generated, which will not happen as long as > scanning logic is disabled until kbc_resume executes. > b) The syscoreops_resume codepath calls into the ISR after finding KBC > was a wakesource. If this happens it will happen only when the kernel > resumes from its suspend code path. Is this code already in mainline or still somewhere else? > > > > Is this scenario not possible? > > > > Thanks. > > > With that being the case do you think the fix makes sense? OK, yes. I do not think it is that important if we occasionally report KEY_POWER even if KBC was not a true wakeup source but that's fine. Thanks.
Hi Rakesh, On Wed, Nov 30, 2011 at 12:43:31PM -0800, riyer@nvidia.com wrote: > From: Rakesh Iyer <riyer@nvidia.com> > > Tegra kbc cannot detect exact keypress causing wakeup in interrupt mode. > Allow wakeup keypress to be reported for certain platforms. > > Signed-off-by: Rakesh Iyer <riyer@nvidia.com> > --- > arch/arm/mach-tegra/include/mach/kbc.h | 1 + > drivers/input/keyboard/tegra-kbc.c | 20 ++++++++++++++++---- > 2 files changed, 17 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/mach-tegra/include/mach/kbc.h b/arch/arm/mach-tegra/include/mach/kbc.h > index 4f3572a..20bb054 100644 > --- a/arch/arm/mach-tegra/include/mach/kbc.h > +++ b/arch/arm/mach-tegra/include/mach/kbc.h > @@ -53,6 +53,7 @@ struct tegra_kbc_platform_data { > struct tegra_kbc_pin_cfg pin_cfg[KBC_MAX_GPIO]; > const struct matrix_keymap_data *keymap_data; > > + u32 wakeup_key; > bool wakeup; > bool use_fn_map; > bool use_ghost_filter; > diff --git a/drivers/input/keyboard/tegra-kbc.c b/drivers/input/keyboard/tegra-kbc.c > index cf3228b..ceb1185 100644 > --- a/drivers/input/keyboard/tegra-kbc.c > +++ b/drivers/input/keyboard/tegra-kbc.c > @@ -52,6 +52,7 @@ > /* KBC Interrupt Register */ > #define KBC_INT_0 0x4 > #define KBC_INT_FIFO_CNT_INT_STATUS (1 << 2) > +#define KBC_INT_KEYPRESS_INT_STATUS (1 << 0) > > #define KBC_ROW_CFG0_0 0x8 > #define KBC_COL_CFG0_0 0x18 > @@ -74,10 +75,12 @@ struct tegra_kbc { > unsigned int cp_to_wkup_dly; > bool use_fn_map; > bool use_ghost_filter; > + bool keypress_caused_wake; > const struct tegra_kbc_platform_data *pdata; > unsigned short keycode[KBC_MAX_KEY * 2]; > unsigned short current_keys[KBC_MAX_KPENT]; > unsigned int num_pressed_keys; > + u32 wakeup_key; > struct timer_list timer; > struct clk *clk; > }; > @@ -409,6 +412,9 @@ static irqreturn_t tegra_kbc_isr(int irq, void *args) > */ > tegra_kbc_set_fifo_interrupt(kbc, false); > mod_timer(&kbc->timer, jiffies + kbc->cp_dly_jiffies); > + } else if (val & KBC_INT_KEYPRESS_INT_STATUS) { > + /* We can be here only through system resume path */ > + kbc->keypress_caused_wake = true; > } > > spin_unlock_irqrestore(&kbc->lock, flags); > @@ -674,9 +680,10 @@ static int __devinit tegra_kbc_probe(struct platform_device *pdev) > keymap_data = pdata->keymap_data ?: &tegra_kbc_default_keymap_data; > matrix_keypad_build_keymap(keymap_data, KBC_ROW_SHIFT, > input_dev->keycode, input_dev->keybit); > + kbc->wakeup_key = pdata->wakeup_key; > > - err = request_irq(kbc->irq, tegra_kbc_isr, IRQF_TRIGGER_HIGH, > - pdev->name, kbc); > + err = request_irq(kbc->irq, tegra_kbc_isr, > + IRQF_NO_SUSPEND | IRQF_TRIGGER_HIGH, pdev->name, kbc); > if (err) { > dev_err(&pdev->dev, "failed to request keyboard IRQ\n"); > goto err_put_clk; > @@ -738,7 +745,6 @@ static int tegra_kbc_suspend(struct device *dev) > > mutex_lock(&kbc->idev->mutex); > if (device_may_wakeup(&pdev->dev)) { > - disable_irq(kbc->irq); > del_timer_sync(&kbc->timer); > tegra_kbc_set_fifo_interrupt(kbc, false); > > @@ -754,6 +760,7 @@ static int tegra_kbc_suspend(struct device *dev) > tegra_kbc_setup_wakekeys(kbc, true); > msleep(30); > > + kbc->keypress_caused_wake = false; > enable_irq_wake(kbc->irq); > } else { > if (kbc->idev->users) > @@ -780,7 +787,12 @@ static int tegra_kbc_resume(struct device *dev) > > tegra_kbc_set_fifo_interrupt(kbc, true); > > - enable_irq(kbc->irq); > + if (kbc->keypress_caused_wake && kbc->wakeup_key) { > + input_report_key(kbc->idev, kbc->wakeup_key, 1); > + input_sync(kbc->idev); > + input_report_key(kbc->idev, kbc->wakeup_key, 0); > + input_sync(kbc->idev); Would you mind moving this block into the ISR itself? If we leave ISR running then there is no reason to postpone event generation, is there? If not then we can get rid of kbc->keypress_caused_wake. Thanks.
diff --git a/arch/arm/mach-tegra/include/mach/kbc.h b/arch/arm/mach-tegra/include/mach/kbc.h index 4f3572a..20bb054 100644 --- a/arch/arm/mach-tegra/include/mach/kbc.h +++ b/arch/arm/mach-tegra/include/mach/kbc.h @@ -53,6 +53,7 @@ struct tegra_kbc_platform_data { struct tegra_kbc_pin_cfg pin_cfg[KBC_MAX_GPIO]; const struct matrix_keymap_data *keymap_data; + u32 wakeup_key; bool wakeup; bool use_fn_map; bool use_ghost_filter; diff --git a/drivers/input/keyboard/tegra-kbc.c b/drivers/input/keyboard/tegra-kbc.c index cf3228b..ceb1185 100644 --- a/drivers/input/keyboard/tegra-kbc.c +++ b/drivers/input/keyboard/tegra-kbc.c @@ -52,6 +52,7 @@ /* KBC Interrupt Register */ #define KBC_INT_0 0x4 #define KBC_INT_FIFO_CNT_INT_STATUS (1 << 2) +#define KBC_INT_KEYPRESS_INT_STATUS (1 << 0) #define KBC_ROW_CFG0_0 0x8 #define KBC_COL_CFG0_0 0x18 @@ -74,10 +75,12 @@ struct tegra_kbc { unsigned int cp_to_wkup_dly; bool use_fn_map; bool use_ghost_filter; + bool keypress_caused_wake; const struct tegra_kbc_platform_data *pdata; unsigned short keycode[KBC_MAX_KEY * 2]; unsigned short current_keys[KBC_MAX_KPENT]; unsigned int num_pressed_keys; + u32 wakeup_key; struct timer_list timer; struct clk *clk; }; @@ -409,6 +412,9 @@ static irqreturn_t tegra_kbc_isr(int irq, void *args) */ tegra_kbc_set_fifo_interrupt(kbc, false); mod_timer(&kbc->timer, jiffies + kbc->cp_dly_jiffies); + } else if (val & KBC_INT_KEYPRESS_INT_STATUS) { + /* We can be here only through system resume path */ + kbc->keypress_caused_wake = true; } spin_unlock_irqrestore(&kbc->lock, flags); @@ -674,9 +680,10 @@ static int __devinit tegra_kbc_probe(struct platform_device *pdev) keymap_data = pdata->keymap_data ?: &tegra_kbc_default_keymap_data; matrix_keypad_build_keymap(keymap_data, KBC_ROW_SHIFT, input_dev->keycode, input_dev->keybit); + kbc->wakeup_key = pdata->wakeup_key; - err = request_irq(kbc->irq, tegra_kbc_isr, IRQF_TRIGGER_HIGH, - pdev->name, kbc); + err = request_irq(kbc->irq, tegra_kbc_isr, + IRQF_NO_SUSPEND | IRQF_TRIGGER_HIGH, pdev->name, kbc); if (err) { dev_err(&pdev->dev, "failed to request keyboard IRQ\n"); goto err_put_clk; @@ -738,7 +745,6 @@ static int tegra_kbc_suspend(struct device *dev) mutex_lock(&kbc->idev->mutex); if (device_may_wakeup(&pdev->dev)) { - disable_irq(kbc->irq); del_timer_sync(&kbc->timer); tegra_kbc_set_fifo_interrupt(kbc, false); @@ -754,6 +760,7 @@ static int tegra_kbc_suspend(struct device *dev) tegra_kbc_setup_wakekeys(kbc, true); msleep(30); + kbc->keypress_caused_wake = false; enable_irq_wake(kbc->irq); } else { if (kbc->idev->users) @@ -780,7 +787,12 @@ static int tegra_kbc_resume(struct device *dev) tegra_kbc_set_fifo_interrupt(kbc, true); - enable_irq(kbc->irq); + if (kbc->keypress_caused_wake && kbc->wakeup_key) { + input_report_key(kbc->idev, kbc->wakeup_key, 1); + input_sync(kbc->idev); + input_report_key(kbc->idev, kbc->wakeup_key, 0); + input_sync(kbc->idev); + } } else { if (kbc->idev->users) err = tegra_kbc_start(kbc);