Prior to this patch, the bitmask ends up being 0x3, as opposed to 0x1
which likely was the intention. The erroneous bit results in the driver
reporting 2 different button activations in designs with 2 or more
buttons.
To detect which button has been pressed, cyttsp5_btn_attention() uses a
for loop to iterate through the input buffer, while shifting and
applying a bitmask to determine the state for each button.
Unfortunately, when the bitmask is 0x3 and there are multiple buttons,
this procedure falls apart.
Consider a design with 3 buttons. Pressing the third button will result
in a call to cyttsp5_btn_attention() with the input buffer containing
0x4 (binary 0100). In the first iteration of the for loop cur_btn_state
will be:
(0x4 >> 0 * 1) & 0x3 = 0x4 & 0x3 = 0x0
This is correct. However, in the next iteration this happens:
(0x4 >> 1 * 1) & 0x3 = 0x2 & 0x3 = 0x2
Which means that a key event for key 1 is generated, even though it's
not really active. In the third iteration, the loop detects the button
that was actually pressed:
(0x4 >> 2 * 1) & 0x3 = 0x1 & 0x3 = 0x1
This key event is the only one that should have been detected, but it is
accompanied by the preceding key. Ensuring the applied mask is 0x1
solves this problem.
Signed-off-by: Richard Kjerstadius <[email protected]>
---
drivers/input/touchscreen/cyttsp5.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
index 4a23d6231382..16caffa35dd9 100644
--- a/drivers/input/touchscreen/cyttsp5.c
+++ b/drivers/input/touchscreen/cyttsp5.c
@@ -29,7 +29,7 @@
#define CY_MAX_INPUT 512
#define CYTTSP5_PREALLOCATED_CMD_BUFFER 32
#define CY_BITS_PER_BTN 1
-#define CY_NUM_BTN_EVENT_ID GENMASK(CY_BITS_PER_BTN, 0)
+#define CY_NUM_BTN_EVENT_ID GENMASK(CY_BITS_PER_BTN - 1, 0)
#define MAX_AREA 255
#define HID_OUTPUT_BL_SOP 0x1
--
2.39.0
On Fri, 27 Jan 2023, at 8:29 PM, Richard Kjerstadius wrote:
> Prior to this patch, the bitmask ends up being 0x3, as opposed to 0x1
> which likely was the intention. The erroneous bit results in the driver
> reporting 2 different button activations in designs with 2 or more
> buttons.
>
> To detect which button has been pressed, cyttsp5_btn_attention() uses a
> for loop to iterate through the input buffer, while shifting and
> applying a bitmask to determine the state for each button.
> Unfortunately, when the bitmask is 0x3 and there are multiple buttons,
> this procedure falls apart.
>
> Consider a design with 3 buttons. Pressing the third button will result
> in a call to cyttsp5_btn_attention() with the input buffer containing
> 0x4 (binary 0100). In the first iteration of the for loop cur_btn_state
> will be:
>
> (0x4 >> 0 * 1) & 0x3 = 0x4 & 0x3 = 0x0
>
> This is correct. However, in the next iteration this happens:
>
> (0x4 >> 1 * 1) & 0x3 = 0x2 & 0x3 = 0x2
>
> Which means that a key event for key 1 is generated, even though it's
> not really active. In the third iteration, the loop detects the button
> that was actually pressed:
>
> (0x4 >> 2 * 1) & 0x3 = 0x1 & 0x3 = 0x1
>
> This key event is the only one that should have been detected, but it is
> accompanied by the preceding key. Ensuring the applied mask is 0x1
> solves this problem.
>
> Signed-off-by: Richard Kjerstadius <[email protected]>
Reviewed-by: Alistair Francis <[email protected]>
Alistair
> ---
> drivers/input/touchscreen/cyttsp5.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
> index 4a23d6231382..16caffa35dd9 100644
> --- a/drivers/input/touchscreen/cyttsp5.c
> +++ b/drivers/input/touchscreen/cyttsp5.c
> @@ -29,7 +29,7 @@
> #define CY_MAX_INPUT 512
> #define CYTTSP5_PREALLOCATED_CMD_BUFFER 32
> #define CY_BITS_PER_BTN 1
> -#define CY_NUM_BTN_EVENT_ID GENMASK(CY_BITS_PER_BTN, 0)
> +#define CY_NUM_BTN_EVENT_ID GENMASK(CY_BITS_PER_BTN - 1, 0)
>
> #define MAX_AREA 255
> #define HID_OUTPUT_BL_SOP 0x1
> --
> 2.39.0
>
>
On Fri, Jan 27, 2023 at 11:29:03AM +0100, Richard Kjerstadius wrote:
> Prior to this patch, the bitmask ends up being 0x3, as opposed to 0x1
> which likely was the intention. The erroneous bit results in the driver
> reporting 2 different button activations in designs with 2 or more
> buttons.
>
> To detect which button has been pressed, cyttsp5_btn_attention() uses a
> for loop to iterate through the input buffer, while shifting and
> applying a bitmask to determine the state for each button.
> Unfortunately, when the bitmask is 0x3 and there are multiple buttons,
> this procedure falls apart.
>
> Consider a design with 3 buttons. Pressing the third button will result
> in a call to cyttsp5_btn_attention() with the input buffer containing
> 0x4 (binary 0100). In the first iteration of the for loop cur_btn_state
> will be:
>
> (0x4 >> 0 * 1) & 0x3 = 0x4 & 0x3 = 0x0
>
> This is correct. However, in the next iteration this happens:
>
> (0x4 >> 1 * 1) & 0x3 = 0x2 & 0x3 = 0x2
>
> Which means that a key event for key 1 is generated, even though it's
> not really active. In the third iteration, the loop detects the button
> that was actually pressed:
>
> (0x4 >> 2 * 1) & 0x3 = 0x1 & 0x3 = 0x1
>
> This key event is the only one that should have been detected, but it is
> accompanied by the preceding key. Ensuring the applied mask is 0x1
> solves this problem.
>
> Signed-off-by: Richard Kjerstadius <[email protected]>
Applied, thank you.
--
Dmitry