[PATCH] Input: tsc2007 - fix resistance calculation when A/D value is saturated

[PATCH] Input: tsc2007 - fix resistance calculation when A/D value is saturated

[Yuki Horii]

The A/D conversion sequence starts with reading the Y position
immediately after the A/D converter is powered on. This makes
the Y position measurement more susceptible to instability due
to noise and mechanical bounce compared to the X position, and
may result in a saturated value (MAX_12BIT = 0xFFF).

While the Y position is not used in equation #1 of the datasheet
for touch resistance calculation, it is required for equation #2.
A saturated Y A/D value, like a saturated X A/D value, indicates
that the touch resistance calculation is invalid.

Only the saturation of the X A/D value was checked, while the
saturation of the Y A/D value was not. Additionally, modifying
tc->x directly had an unintended side effect of corrupting the
caller's data.

The function now returns 0 when either X or Y A/D value is
saturated to indicate that the touch resistance calculation
is invalid.

Signed-off-by: Yuki Horii <horiiyuk@ishida.co.jp>
---
 drivers/input/touchscreen/tsc2007_core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2007_core.c b/drivers/input/touchscreen/tsc2007_core.c
index 524f14eb3da2..ce82163da4ff 100644
--- a/drivers/input/touchscreen/tsc2007_core.c
+++ b/drivers/input/touchscreen/tsc2007_core.c
@@ -71,9 +71,9 @@ u32 tsc2007_calculate_resistance(struct tsc2007 *tsc, struct ts_event *tc)
 {
 	u64 rt = 0;
 
-	/* range filtering */
-	if (tc->x == MAX_12BIT)
-		tc->x = 0;
+	/* invalidate touch resistance if X or Y coordinate is saturated */
+	if (tc->x == MAX_12BIT || tc->y == MAX_12BIT)
+		return 0;
 
 	if (likely(tc->x && tc->z1)) {
 		/* compute touch resistance using equation #1 */
-- 
2.51.0

Re: [PATCH] Input: tsc2007 - fix resistance calculation when A/D value is saturated

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Uninitialized stack variable read in the IIO raw read path.
- [High] Returning 0 to invalidate saturated A/D values causes phantom touches with maximum pressure or premature pen-up events.
--

commit 8a6bcc1904d4e8b8d9d219e6c086925c53bfc4cb
Author: Yuki Horii <yuuki198708@gmail.com>

Input: tsc2007 - fix resistance calculation when A/D value is saturated

This patch modifies the touch resistance calculation to return 0 when either
the X or Y A/D value is saturated. This is intended to indicate that the
touch resistance calculation is invalid.

> diff --git a/drivers/input/touchscreen/tsc2007_core.c b/drivers/input/touchscreen/tsc2007_core.c
> index f9b3d2598933c..7d3bc0c396113 100644
> --- a/drivers/input/touchscreen/tsc2007_core.c
> +++ b/drivers/input/touchscreen/tsc2007_core.c
> @@ -69,9 +69,9 @@ u32 tsc2007_calculate_resistance(struct tsc2007 *tsc, struct ts_event *tc)
>  {
>  	u64 rt = 0;
>  
> -	/* range filtering */
> -	if (tc->x == MAX_12BIT)
> -		tc->x = 0;
> +	/* invalidate touch resistance if X or Y coordinate is saturated */
> +	if (tc->x == MAX_12BIT || tc->y == MAX_12BIT)

Does this code read uninitialized stack memory? The IIO path initializes
tc.x, tc.z1, and tc.z2, but it does not initialize tc.y before passing the
struct to tsc2007_calculate_resistance():

drivers/input/touchscreen/tsc2007_iio.c:tsc2007_read_raw() {
	...
	case 5: {
		struct ts_event tc;

		tc.x = tsc2007_xfer(tsc, READ_X);
		tc.z1 = tsc2007_xfer(tsc, READ_Z1);
		tc.z2 = tsc2007_xfer(tsc, READ_Z2);
		*val = tsc2007_calculate_resistance(tsc, &tc);
		break;
	}
	...
}

> +		return 0;

Is it safe to return 0 to invalidate the sample?

In tsc2007_soft_irq(), if there is no pendown GPIO, !rt evaluates to true
and prematurely breaks the polling loop, causing an early pen-up event.

If there is a pendown GPIO, rt <= ts->max_rt evaluates to true. Then the
driver calculates pressure as ts->max_rt - rt, which maps rt = 0 to the
maximum possible pressure. This results in a valid touch being reported at
the saturated coordinates with maximum pressure, rather than ignoring the
sample.

Should this instead return a value strictly greater than ts->max_rt (such as
U32_MAX) so the debouncing logic safely ignores the invalid sample?

>  
>  	if (likely(tc->x && tc->z1)) {
>  		/* compute touch resistance using equation #1 */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511054003.1205-1-horiiyuk@ishida.co.jp?part=1

Re: [PATCH] Input: tsc2007 - fix resistance calculation when A/D value is saturated

[堀井裕樹]

Thank you for AI review

> Does this code read uninitialized stack memory? The IIO path initializes
> tc.x, tc.z1, and tc.z2, but it does not initialize tc.y before passing the
> struct to tsc2007_calculate_resistance():

I knew this code existed, but I didn't realize it hadn't been initialized.
I'll fix it.

> Is it safe to return 0 to invalidate the sample?
>
> In tsc2007_soft_irq(), if there is no pendown GPIO, !rt evaluates to true
> and prematurely breaks the polling loop, causing an early pen-up event.

I'll fix it.
In the original code, I had set touch input to be disabled when the
value was 0, but after submitting the patch,
I realized that setting the resistance to infinity—rather than
disabling it at 0—would allow the interrupt handling to continue.