[PATCH] Input: tca8418_keypad - enable overflow mode per datasheet (SCPS215G)

[PATCH] Input: tca8418_keypad - enable overflow mode per datasheet (SCPS215G)

[Zhian Liang]

The driver currently sets only the overflow interrupt enable bit
(OVR_FLOW_IEN) in the configuration register, leaving the overflow
mode bit (OVR_FLOW_M) at its default value of 0.

According to the TCA8418 datasheet (SCPS215G, Section 8.6.4.1
"Overflow Errata - Description"), both OVR_FLOW_M (Bit_5) and
OVR_FLOW_IEN (Bit_3) must be set high for the overflow interrupt
to be generated. If only OVR_FLOW_IEN is set, FIFO overflow events
are silently lost without notifying the host.

Fix this by setting OVR_FLOW_M alongside OVR_FLOW_IEN in the
configuration register.

Note: I do not have access to hardware to test this change.
Testing by generating 11+ key events without reading the FIFO
would be appreciated. Full handling of overflow events in the
interrupt handler is left for future improvement.

Signed-off-by: Zhian Liang <liangzhan5dev@gmail.com>
---
 drivers/input/keyboard/tca8418_keypad.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/tca8418_keypad.c b/drivers/input/keyboard/tca8418_keypad.c
index 68c0afafee7b..b124e576feca 100644
--- a/drivers/input/keyboard/tca8418_keypad.c
+++ b/drivers/input/keyboard/tca8418_keypad.c
@@ -254,7 +254,7 @@ static int tca8418_configure(struct tca8418_keypad *keypad_data,
 		return error;
 
 	error = tca8418_write_byte(keypad_data, REG_CFG,
-				CFG_INT_CFG | CFG_OVR_FLOW_IEN | CFG_KE_IEN);
+				CFG_INT_CFG | CFG_OVR_FLOW_IEN | CFG_OVR_FLOW_M | CFG_KE_IEN);
 
 	return error;
 }
-- 
2.34.1

Re: [PATCH] Input: tca8418_keypad - enable overflow mode per datasheet (SCPS215G)

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver does not sufficiently validate the hardware keycode, allowing a malformed I2C read to cause an out-of-bounds array access.
- [High] The keypad FIFO read loop lacks an upper iteration bound, risking an infinite loop if the I2C bus fails silently.
--

commit c17e8cb1a67b8fa0dfa772dc4d63c35917e63ccb
Author: Zhian Liang <liangzhan5dev@gmail.com>

Input: tca8418_keypad - enable overflow mode per datasheet (SCPS215G)

This commit sets the OVR_FLOW_M bit alongside OVR_FLOW_IEN in the
configuration register. This ensures the overflow interrupt is properly
generated, preventing FIFO overflow events from being silently lost.

> diff --git a/drivers/input/keyboard/tca8418_keypad.c b/drivers/input/keyboard/tca8418_keypad.c
> index 68c0afafee7bf..b124e576feca4 100644
> --- a/drivers/input/keyboard/tca8418_keypad.c
> +++ b/drivers/input/keyboard/tca8418_keypad.c
> @@ -254,7 +254,7 @@ static int tca8418_configure(struct tca8418_keypad *keypad_data,
>  		return error;
>  
>  	error = tca8418_write_byte(keypad_data, REG_CFG,
> -				CFG_INT_CFG | CFG_OVR_FLOW_IEN | CFG_KE_IEN);
> +				CFG_INT_CFG | CFG_OVR_FLOW_IEN | CFG_OVR_FLOW_M | CFG_KE_IEN);
>  
>  	return error;
>  }

While reviewing this, I noticed a couple of potential issues in the same
file.

drivers/input/keyboard/tca8418_keypad.c:tca8418_read_keypad() {
	[ ... ]
	do {
		error = tca8418_read_byte(keypad_data, REG_KEY_EVENT_A, &reg);
		if (error < 0) {
			dev_err(&keypad_data->client->dev,
				"unable to read REG_KEY_EVENT_A\n");
			break;
		}

		/* Assume that key code 0 signifies empty FIFO */
		if (reg <= 0)
			break;
	[ ... ]

[Severity: High]
This is a pre-existing issue, but does this loop risk running infinitely if
the I2C bus fails silently?

If the I2C bus gets stuck high, it may return 0xFF. Because reg is an
unsigned u8, the condition reg <= 0 evaluates to false for 0xFF (255).

The loop would never terminate, stalling the threaded IRQ handler
indefinitely.

	[ ... ]
		row = code / TCA8418_MAX_COLS;
		col = code % TCA8418_MAX_COLS;

		row = (col) ? row : row - 1;
		col = (col) ? col - 1 : TCA8418_MAX_COLS - 1;

		code = MATRIX_SCAN_CODE(row, col, keypad_data->row_shift);
		input_event(input, EV_MSC, MSC_SCAN, code);
		input_report_key(input, keymap[code], state);
	[ ... ]
}

[Severity: High]
This isn't a bug introduced by this patch, but does the driver sufficiently
validate the hardware keycode to prevent an out-of-bounds array access?

If the I2C bus returns 0xFF, code is calculated as 127. The index
calculations yield row = 12 and col = 6, resulting in code = 198.

Since the keymap array allocated for this device has a maximum of 128
elements, this results in an out-of-bounds memory read on keymap[198].

Similarly, a malformed value of 0x80 results in row = -1, col = 9, and
code = 249, causing the same out-of-bounds access.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529013900.43854-1-liangzhan5dev@gmail.com?part=1