Hi Benjamin,
On Thu, Aug 11, 2022 at 02:33:13PM -0400, Benjamin Rood wrote:
> From bd53d80e1966320f8d6f2775337043cbac2d1b6e Mon Sep 17 00:00:00 2001
> From: Benjamin Rood <benjaminjrood@xxxxxxxxx>
> Date: Thu, 11 Aug 2022 14:18:16 -0400
> Subject: [PATCH] Input: pixcir_i2c_ts - lengthen reset pulse to touchscreen
> controller
>
> This patch adjust the reset pulse in the pixcir_i2c_ts driver to address
> the following issues:
>
> 1. Not driving the reset signal HIGH for a long enough period, resulting
> in a "shark fin" reset signal.
> 2. Not waiting long enough after assering the reset signal to issue the
> first I2C transaction, which results in a NACK because the touchscreen
> controller was not ready to communicate. This typically results in the
> touchscreen controller not enumerating as an input device.
>
> The changes included in this patch address the above two issues 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.
I guess 1 msec is not that bad...
> 2. Configuring the delay after de-asserting the reset signal to be a
> duration of 1s before issuing the first I2C transaction. This allows
> the touchscreen controller to fully boot which avoids a NACK an a
> missing input device.
but 1 sec is insanely long, especially for driver that is not marked for
async probe. I wonder, since (I assume) the time to get out of reset
state is not specified in the datasheet, if we could not retry I2C xfers
with a small delays between them instead of waiting for whole second.
How long does it actually take for the controller to exit the reset
state on your system?
>
> Kudos also should be given to my colleage Dan MacDonald
> <dmacdonald@xxxxxxxxxxxxxxxxxx> for helping to discover and fix these
> issues.
>
> Signed-off-by: Benjamin Rood <benjaminjrood@xxxxxxxxx>
> ---
> drivers/input/touchscreen/pixcir_i2c_ts.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/touchscreen/pixcir_i2c_ts.c b/drivers/input/touchscreen/pixcir_i2c_ts.c
> index dc148b4bed74..324e41886dea 100644
> --- a/drivers/input/touchscreen/pixcir_i2c_ts.c
> +++ b/drivers/input/touchscreen/pixcir_i2c_ts.c
> @@ -222,10 +222,10 @@ 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);
> + msleep(1000);
Please note that the patch is whitespace-damaged. Did you paste it into
gmail UI?
> }
> }
>
> --
> 2.25.1
Thanks.
--
Dmitry
