linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] input: keyboard-matrix: add a function to reset input states
@ 2021-09-26 15:18 Angelo Dureghello
  2021-10-06 20:03 ` Dmitry Torokhov
  0 siblings, 1 reply; 3+ messages in thread
From: Angelo Dureghello @ 2021-09-26 15:18 UTC (permalink / raw)
  To: linux-input, dmitry.torokhov; +Cc: Angelo Dureghello

In some cases, at open(), when matrix_keypad_start() schedules
matrix_keypad_scan(), some events may be fired, but they can't be
grabbed from the event queue by evtest later on, since this
condition causes a continuous event triggered.

Add a function to get (and align) button states at open without
firing events at open stage.

Signed-off-by: Angelo Dureghello <angelo.dureghello@timesys.com>
---
 drivers/input/keyboard/matrix_keypad.c | 37 ++++++++++++++++----------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
index 30924b57058f..ce8cb09ee333 100644
--- a/drivers/input/keyboard/matrix_keypad.c
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -108,6 +108,24 @@ static void disable_row_irqs(struct matrix_keypad *keypad)
 	}
 }
 
+static inline void matrix_get_keys_state(struct matrix_keypad *keypad,
+					 uint32_t *states)
+{
+	int row, col;
+	const struct matrix_keypad_platform_data *pdata = keypad->pdata;
+
+	states = states ?: keypad->last_key_state;
+
+	/* assert each column and read the row status out */
+	for (col = 0; col < pdata->num_col_gpios; col++) {
+		activate_col(pdata, col, true);
+		for (row = 0; row < pdata->num_row_gpios; row++)
+			states[col] |=
+				row_asserted(pdata, row) ? (1 << row) : 0;
+		activate_col(pdata, col, false);
+	}
+}
+
 /*
  * This gets the keys from keyboard and reports it to input subsystem
  */
@@ -126,17 +144,7 @@ static void matrix_keypad_scan(struct work_struct *work)
 
 	memset(new_state, 0, sizeof(new_state));
 
-	/* assert each column and read the row status out */
-	for (col = 0; col < pdata->num_col_gpios; col++) {
-
-		activate_col(pdata, col, true);
-
-		for (row = 0; row < pdata->num_row_gpios; row++)
-			new_state[col] |=
-				row_asserted(pdata, row) ? (1 << row) : 0;
-
-		activate_col(pdata, col, false);
-	}
+	matrix_get_keys_state(keypad, new_state);
 
 	for (col = 0; col < pdata->num_col_gpios; col++) {
 		uint32_t bits_changed;
@@ -202,10 +210,11 @@ static int matrix_keypad_start(struct input_dev *dev)
 	mb();
 
 	/*
-	 * Schedule an immediate key scan to capture current key state;
-	 * columns will be activated and IRQs be enabled after the scan.
+	 * Reset current key state and activate columns and IRQs.
 	 */
-	schedule_delayed_work(&keypad->work, 0);
+	matrix_get_keys_state(keypad, NULL);
+	activate_all_cols(keypad->pdata, true);
+	enable_row_irqs(keypad);
 
 	return 0;
 }
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] input: keyboard-matrix: add a function to reset input states
  2021-09-26 15:18 [PATCH] input: keyboard-matrix: add a function to reset input states Angelo Dureghello
@ 2021-10-06 20:03 ` Dmitry Torokhov
  2021-10-07 12:17   ` Angelo Dureghello
  0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Torokhov @ 2021-10-06 20:03 UTC (permalink / raw)
  To: Angelo Dureghello; +Cc: linux-input

Hi Angelo,

On Sun, Sep 26, 2021 at 05:18:47PM +0200, Angelo Dureghello wrote:
> In some cases, at open(), when matrix_keypad_start() schedules
> matrix_keypad_scan(), some events may be fired, but they can't be
> grabbed from the event queue by evtest later on, since this
> condition causes a continuous event triggered.

I am not quite sure what you mean by "continuous event triggered". Could
you please explain in more detail?

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] input: keyboard-matrix: add a function to reset input states
  2021-10-06 20:03 ` Dmitry Torokhov
@ 2021-10-07 12:17   ` Angelo Dureghello
  0 siblings, 0 replies; 3+ messages in thread
From: Angelo Dureghello @ 2021-10-07 12:17 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input

Hi Dimitry,

On Wed, Oct 6, 2021 at 10:03 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hi Angelo,
>
> On Sun, Sep 26, 2021 at 05:18:47PM +0200, Angelo Dureghello wrote:
> > In some cases, at open(), when matrix_keypad_start() schedules
> > matrix_keypad_scan(), some events may be fired, but they can't be
> > grabbed from the event queue by evtest later on, since this
> > condition causes a continuous event triggered.
>
> I am not quite sure what you mean by "continuous event triggered". Could
> you please explain in more detail?
>

sure, mainly, without this fix, by using evtest, i get
continuously the same key event (last key of the keypad),
even if evtest is supposed to remove the event from the queue,
the event is always there. Then, pressing a key of the matrix
keypad, things get fixed, i get the key press event, then events
stop, queue is now empty, no other events are detected.

Honestly couldn't go much deeper, but the behavior described above
is originated from calling:
matrix_keypad_start() ->
  scheduled_delayed_work() ->
     matrix_keypad_scan(), that detects some initial key pressed state
        and fires an event, and interrupts are not enabled still.

Cleaning key states before enabling them solved the issue (i am on
a sun8i H2+).

>
> Thanks.
>

YW, available for any test in case.

Regards,
angelo


>
> --
> Dmitry



-- 
Angelo Dureghello
Timesys
e. angelo.dureghello@timesys.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-10-07 12:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-26 15:18 [PATCH] input: keyboard-matrix: add a function to reset input states Angelo Dureghello
2021-10-06 20:03 ` Dmitry Torokhov
2021-10-07 12:17   ` Angelo Dureghello

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).