[v1] Input: tegra-kbc - modify keypress wakeup logic.

[v1] Input: tegra-kbc - modify keypress wakeup logic.

[riyer]

From: Rakesh Iyer <riyer@nvidia.com>

Modify wakeup logic so either all keypresses wake the system or none do.

Signed-off-by: Rakesh Iyer <riyer@nvidia.com>
---
v1: Modify wakeup logic so either all keypresses wake the system or none do.

 arch/arm/mach-tegra/include/mach/kbc.h |    4 +---
 drivers/input/keyboard/tegra-kbc.c     |   28 ++++------------------------
 2 files changed, 5 insertions(+), 27 deletions(-)

diff --git a/arch/arm/mach-tegra/include/mach/kbc.h b/arch/arm/mach-tegra/include/mach/kbc.h
index bd99744..6b8e934 100644
--- a/arch/arm/mach-tegra/include/mach/kbc.h
+++ b/arch/arm/mach-tegra/include/mach/kbc.h
@@ -50,14 +50,12 @@ struct tegra_kbc_platform_data {
 	unsigned int debounce_cnt;
 	unsigned int repeat_cnt;
 
-	unsigned int wake_cnt; /* 0:wake on any key >1:wake on wake_cfg */
-	const struct tegra_kbc_wake_key *wake_cfg;
-
 	struct tegra_kbc_pin_cfg pin_cfg[KBC_MAX_GPIO];
 	const struct matrix_keymap_data *keymap_data;
 
 	bool wakeup;
 	bool use_fn_map;
 	bool use_ghost_filter;
+	bool wake_on_keypress;
 };
 #endif
diff --git a/drivers/input/keyboard/tegra-kbc.c b/drivers/input/keyboard/tegra-kbc.c
index 5fb4b97..cdbc55b 100644
--- a/drivers/input/keyboard/tegra-kbc.c
+++ b/drivers/input/keyboard/tegra-kbc.c
@@ -66,8 +66,6 @@ struct tegra_kbc {
 	void __iomem *mmio;
 	struct input_dev *idev;
 	unsigned int irq;
-	unsigned int wake_enable_rows;
-	unsigned int wake_enable_cols;
 	spinlock_t lock;
 	unsigned int repoll_dly;
 	unsigned long cp_dly_jiffies;
@@ -291,7 +289,7 @@ static void tegra_kbc_report_keys(struct tegra_kbc *kbc)
 	 * Matrix keyboard designs are prone to keyboard ghosting.
 	 * Ghosting occurs if there are 3 keys such that -
 	 * any 2 of the 3 keys share a row, and any 2 of them share a column.
-	 * If so ignore the key presses for this iteration.
+	 * If so ignore the keypresses for this iteration.
 	 */
 	if ((kbc->use_ghost_filter) && (num_down >= 3)) {
 		for (i = 0; i < num_down; i++) {
@@ -328,7 +326,7 @@ static void tegra_kbc_report_keys(struct tegra_kbc *kbc)
 
 	spin_unlock_irqrestore(&kbc->lock, flags);
 
-	/* Ignore the key presses for this iteration? */
+	/* Ignore the keypresses for this iteration? */
 	if (key_in_same_col && key_in_same_row)
 		return;
 
@@ -418,21 +416,11 @@ static void tegra_kbc_setup_wakekeys(struct tegra_kbc *kbc, bool filter)
 	int i;
 	unsigned int rst_val;
 
-	BUG_ON(pdata->wake_cnt > KBC_MAX_KEY);
-	rst_val = (filter && pdata->wake_cnt) ? ~0 : 0;
+	/* Either mask all keys or none. */
+	rst_val = (filter && !pdata->wake_on_keypress) ? ~0 : 0;
 
 	for (i = 0; i < KBC_MAX_ROW; i++)
 		writel(rst_val, kbc->mmio + KBC_ROW0_MASK_0 + i * 4);
-
-	if (filter) {
-		for (i = 0; i < pdata->wake_cnt; i++) {
-			u32 val, addr;
-			addr = pdata->wake_cfg[i].row * 4 + KBC_ROW0_MASK_0;
-			val = readl(kbc->mmio + addr);
-			val &= ~(1 << pdata->wake_cfg[i].col);
-			writel(val, kbc->mmio + addr);
-		}
-	}
 }
 
 static void tegra_kbc_config_pins(struct tegra_kbc *kbc)
@@ -594,7 +582,6 @@ static int __devinit tegra_kbc_probe(struct platform_device *pdev)
 	struct resource *res;
 	int irq;
 	int err;
-	int i;
 	int num_rows = 0;
 	unsigned int debounce_cnt;
 	unsigned int scan_time_rows;
@@ -651,13 +638,6 @@ static int __devinit tegra_kbc_probe(struct platform_device *pdev)
 		goto err_iounmap;
 	}
 
-	kbc->wake_enable_rows = 0;
-	kbc->wake_enable_cols = 0;
-	for (i = 0; i < pdata->wake_cnt; i++) {
-		kbc->wake_enable_rows |= (1 << pdata->wake_cfg[i].row);
-		kbc->wake_enable_cols |= (1 << pdata->wake_cfg[i].col);
-	}
-
 	/*
 	 * The time delay between two consecutive reads of the FIFO is
 	 * the sum of the repeat time and the time taken for scanning
-- 
1.7.2.3

Re: [v1] Input: tegra-kbc - modify keypress wakeup logic.

[Dmitry Torokhov]

Hi Rakesh,

On Mon, May 2, 2011 at 5:23 PM,  <riyer@nvidia.com> wrote:
> From: Rakesh Iyer <riyer@nvidia.com>
>
> Modify wakeup logic so either all keypresses wake the system or none do.

You need to provide justification for this change (i.e. why the new
way is better).

Thanks.

-- 
Dmitry

RE: [v1] Input: tegra-kbc - modify keypress wakeup logic.

[Rakesh Iyer]

Hello Dmitry

Our hardware design cannot reliably support an arbitrary set of keys waking up the system.

In contrast we can reliably accommodate the scenario where all keys can wake the system.

Our vendors agree to having all keys wake the system up rather than the unreliable solution.

Please let me know if more information is needed.

Thanks and Regards
Rakesh

> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: Monday, May 02, 2011 5:49 PM
> To: Rakesh Iyer
> Cc: rydberg@euromail.se; Allen Martin; olof@lixom.net; linux-kernel@vger.kernel.org;
> linux-input@vger.kernel.org
> Subject: Re: [v1] Input: tegra-kbc - modify keypress wakeup logic.
> 
> Hi Rakesh,
> 
> On Mon, May 2, 2011 at 5:23 PM,  <riyer@nvidia.com> wrote:
> > From: Rakesh Iyer <riyer@nvidia.com>
> >
> > Modify wakeup logic so either all keypresses wake the system or none do.
> 
> You need to provide justification for this change (i.e. why the new
> way is better).
> 
> Thanks.
> 
> --
> Dmitry

Re: [v1] Input: tegra-kbc - modify keypress wakeup logic.

[Dmitry Torokhov]

Hi Rakesh,

On Mon, May 02, 2011 at 05:23:08PM -0700, riyer@nvidia.com wrote:
> From: Rakesh Iyer <riyer@nvidia.com>
> 
> Modify wakeup logic so either all keypresses wake the system or none do.
> 
> Signed-off-by: Rakesh Iyer <riyer@nvidia.com>
> ---
> v1: Modify wakeup logic so either all keypresses wake the system or none do.
> 
>  arch/arm/mach-tegra/include/mach/kbc.h |    4 +---
>  drivers/input/keyboard/tegra-kbc.c     |   28 ++++------------------------
>  2 files changed, 5 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/arm/mach-tegra/include/mach/kbc.h b/arch/arm/mach-tegra/include/mach/kbc.h
> index bd99744..6b8e934 100644
> --- a/arch/arm/mach-tegra/include/mach/kbc.h
> +++ b/arch/arm/mach-tegra/include/mach/kbc.h
> @@ -50,14 +50,12 @@ struct tegra_kbc_platform_data {
>  	unsigned int debounce_cnt;
>  	unsigned int repeat_cnt;
>  
> -	unsigned int wake_cnt; /* 0:wake on any key >1:wake on wake_cfg */
> -	const struct tegra_kbc_wake_key *wake_cfg;
> -
>  	struct tegra_kbc_pin_cfg pin_cfg[KBC_MAX_GPIO];
>  	const struct matrix_keymap_data *keymap_data;
>  
>  	bool wakeup;
>  	bool use_fn_map;
>  	bool use_ghost_filter;
> +	bool wake_on_keypress;

Why do we need new wake_on_keypress flag if we already have 'wakeup'?
What besides a keypress can be used as a wakeup source with this device?

Thanks.

-- 
Dmitry