linux-kernel - [PATCH v2] Input: pixcir_i2c_ts - lengthen reset pulse to touchscreen controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 16 Aug 2022 17:15:18 -0400
From:   Benjamin Rood <benjaminjrood@...il.com>
To:     dmitry.torokhov@...il.com
Cc:     dmacdonald@...bellmedical.com,
        Benjamin Rood <benjaminjrood@...il.com>,
        linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH v2] Input: pixcir_i2c_ts - lengthen reset pulse to touchscreen controller

From: Benjamin Rood <benjaminjrood@...il.com>

This patch adjust the reset pulse in the pixcir_i2c_ts driver to address
the following issue:

1. Not driving the reset signal HIGH for a long enough period, resulting
in a "shark fin" reset signal.  Some microcontrollers do not have "fast"
internal pullups, which may result in a "shark fin" signal if the delay
between asserting and releasing the signal is too short.  Even though as
noted in the driver code the duration of 80ns is the minimum duration to
assert the reset signal, a host microcontroller using an internal
pull-up to assert the reset signal may not drive the signal high enough
with this delay to effectively put the controller into reset.

We encountered this exact scenario with an NXP i.MX8M Plus directly
connected to the RST I/O of the pixcir controller, with a resulting
non-operative touchscreen on a cold-boot sequence.

The change included in this patch addresses the issue by:

1. Configuring the delay after driving the reset signal HIGH to use
usleep_range(500, 1000) to allow the reset signal to reach a full logic
HIGH voltage for a maximum period of 1ms.  This is overkill as the
datasheet specs 80ns as the minimum duration of the reset pulse, so by
overshooting the timing by quite a lot, it gives the driving chip enough
time to drive a logic HIGH to assert the reset.

Kudos also should be given to my colleage Dan MacDonald
<dmacdonald@...bellmedical.com> for helping to discover and fix these
issues.

Signed-off-by: Benjamin Rood <benjaminjrood@...il.com>
---
 drivers/input/touchscreen/pixcir_i2c_ts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/pixcir_i2c_ts.c b/drivers/input/touchscreen/pixcir_i2c_ts.c
index dc148b4bed74..4af4b9016c94 100644
--- a/drivers/input/touchscreen/pixcir_i2c_ts.c
+++ b/drivers/input/touchscreen/pixcir_i2c_ts.c
@@ -222,7 +222,7 @@ static void pixcir_reset(struct pixcir_i2c_ts_data *tsdata)
 {
 	if (!IS_ERR_OR_NULL(tsdata->gpio_reset)) {
 		gpiod_set_value_cansleep(tsdata->gpio_reset, 1);
-		ndelay(100);	/* datasheet section 1.2.3 says 80ns min. */
+		usleep_range(500, 1000);	/* datasheet section 1.2.3 says 80ns min. */
 		gpiod_set_value_cansleep(tsdata->gpio_reset, 0);
 		/* wait for controller ready. 100ms guess. */
 		msleep(100);
-- 
2.25.1