Re: [PATCH v8 1/5] Input: stmfts - wait for controller ready after reset

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Fri, Sep 25, 2026 at 11:22:19PM +0200, David Heidelberg via B4 Relay wrote:
> From: David Heidelberg <david@ixit.cz>
> 
> After releasing the reset line stmfts_reset() sleeps a fixed 50 ms and
> stmfts_power_on() another 50 ms before the first I2C access.  That is
> enough for a warm reset, but when both supplies were really cut during
> system suspend the controller boots from cold and can need longer.  If
> it does, the first read fails, stmfts_resume() returns an error and the
> touchscreen is left powered off with its interrupt disabled.
> 
> The controller posts a controller ready event once it has booted, and
> the event parser already completes cmd_done on it.  Enable the interrupt
> right after releasing reset and wait for that event instead of sleeping,
> giving the controller 300 ms, the total budget the vendor driver allows.
> Boards without a reset line keep the old timing.
> 
> Usually phones such as Pixel 4a and Xiaomi Mi 8 needs longer delay, so
> without this change touchscreen stops working.
> 
> Fixes: 8a1f9de80e45 ("Input: stmfts - add optional reset GPIO support")
> Cc: stable@vger.kernel.org
> Co-developed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: David Heidelberg <david@ixit.cz>
> ---
>  drivers/input/touchscreen/stmfts.c | 60 ++++++++++++++++++++++++--------------
>  1 file changed, 38 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/stmfts.c b/drivers/input/touchscreen/stmfts.c
> index 972687797f826..1720202d6186b 100644
> --- a/drivers/input/touchscreen/stmfts.c
> +++ b/drivers/input/touchscreen/stmfts.c
> @@ -59,16 +59,18 @@
>  #define STMFTS_MASK_X_MSB			0x0f
>  #define STMFTS_MASK_Y_LSB			0xf0
>  
>  /* key related event masks */
>  #define STMFTS_MASK_KEY_NO_TOUCH		0x00
>  #define STMFTS_MASK_KEY_MENU			0x01
>  #define STMFTS_MASK_KEY_BACK			0x02
>  
> +#define STMFTS_RESET_TIMEOUT_MS	300
> +
>  #define STMFTS_EVENT_SIZE	8
>  #define STMFTS_STACK_DEPTH	32
>  #define STMFTS_DATA_MAX_SIZE	(STMFTS_EVENT_SIZE * STMFTS_STACK_DEPTH)
>  #define STMFTS_MAX_FINGERS	10
>  #define STMFTS_DEV_NAME		"stmfts"
>  
>  static const struct regulator_bulk_data stmfts_supplies[] = {
>  	{ .supply = "vdd" },
> @@ -538,25 +540,16 @@ static int stmfts_read_system_info(struct stmfts_data *sdata)
>  	sdata->chip_ver = reg[0];
>  	sdata->fw_ver = be16_to_cpup((__be16 *)&reg[2]);
>  	sdata->config_id = reg[4];
>  	sdata->config_ver = reg[5];
>  
>  	return 0;
>  }
>  
> -static void stmfts_reset(struct stmfts_data *sdata)
> -{
> -	gpiod_set_value_cansleep(sdata->reset_gpio, 1);
> -	msleep(20);
> -
> -	gpiod_set_value_cansleep(sdata->reset_gpio, 0);
> -	msleep(50);
> -}
> -
>  static int stmfts_configure(struct stmfts_data *sdata)
>  {
>  	int err;
>  
>  	err = stmfts_command(sdata, STMFTS_SYSTEM_RESET);
>  	if (err)
>  		return err;
>  
> @@ -582,53 +575,76 @@ static int stmfts_configure(struct stmfts_data *sdata)
>  
>  	return 0;
>  }
>  
>  static int stmfts_power_on(struct stmfts_data *sdata)
>  {
>  	int err;
>  
> +	if (sdata->reset_gpio) {
> +		gpiod_set_value_cansleep(sdata->reset_gpio, 1);
> +		/* a short delay before powering up */
> +		usleep_range(1000, 1500);
> +	}
> +
>  	err = regulator_bulk_enable(ARRAY_SIZE(stmfts_supplies),
>  				    sdata->supplies);
>  	if (err)
>  		return err;
>  
> -	/*
> -	 * The datasheet does not specify the power on time, but considering
> -	 * that the reset time is < 10ms, I sleep 20ms to be sure
> -	 */
> -	msleep(20);
> +	if (sdata->reset_gpio) {
> +		reinit_completion(&sdata->cmd_done);
>  
> -	if (sdata->reset_gpio)
> -		stmfts_reset(sdata);
> +		/*
> +		 * The datasheet does not specify the power on time, but
> +		 * considering that the reset time is < 10ms, sleep for 20ms
> +		 * to be sure before releasing reset line.
> +		 */
> +		msleep(20);
> +		gpiod_set_value_cansleep(sdata->reset_gpio, 0);
>  
> -	err = stmfts_read_system_info(sdata);
> -	if (err)
> -		goto err_disable_regulators;
> +		enable_irq(sdata->client->irq);
>  
> -	enable_irq(sdata->client->irq);
> +		if (!wait_for_completion_timeout(&sdata->cmd_done,
> +						 msecs_to_jiffies(STMFTS_RESET_TIMEOUT_MS))) {
> +			dev_err(&sdata->client->dev, "controller not ready after reset");
> +			err = -ETIMEDOUT;
> +			goto err_disable_irq;
> +		}
> +	} else {
> +		/*
> +		 * We do not know the real controller state (was it powered
> +		 * off or reset). Let's hope that this is enough time to
> +		 * initialize.
> +		 */
> +		msleep(70);
> +
> +		enable_irq(sdata->client->irq);

Sashiko gave me an idea. What if we do this:

diff --git a/drivers/input/touchscreen/stmfts.c b/drivers/input/touchscreen/stmfts.c
index 1720202d6186..e147a27f57b3 100644
--- a/drivers/input/touchscreen/stmfts.c
+++ b/drivers/input/touchscreen/stmfts.c
@@ -578,6 +578,7 @@ static int stmfts_configure(struct stmfts_data *sdata)
 
 static int stmfts_power_on(struct stmfts_data *sdata)
 {
+	unsigned int timeout;
 	int err;
 
 	if (sdata->reset_gpio) {
@@ -591,9 +592,9 @@ static int stmfts_power_on(struct stmfts_data *sdata)
 	if (err)
 		return err;
 
-	if (sdata->reset_gpio) {
-		reinit_completion(&sdata->cmd_done);
+	reinit_completion(&sdata->cmd_done);
 
+	if (sdata->reset_gpio) {
 		/*
 		 * The datasheet does not specify the power on time, but
 		 * considering that the reset time is < 10ms, sleep for 20ms
@@ -602,23 +603,29 @@ static int stmfts_power_on(struct stmfts_data *sdata)
 		msleep(20);
 		gpiod_set_value_cansleep(sdata->reset_gpio, 0);
 
-		enable_irq(sdata->client->irq);
-
-		if (!wait_for_completion_timeout(&sdata->cmd_done,
-						 msecs_to_jiffies(STMFTS_RESET_TIMEOUT_MS))) {
-			dev_err(&sdata->client->dev, "controller not ready after reset");
-			err = -ETIMEDOUT;
-			goto err_disable_irq;
-		}
+		timeout = STMFTS_RESET_TIMEOUT_MS;
 	} else {
 		/*
 		 * We do not know the real controller state (was it powered
 		 * off or reset). Let's hope that this is enough time to
 		 * initialize.
 		 */
-		msleep(70);
+		timeout = 70;
+	}
 
-		enable_irq(sdata->client->irq);
+	enable_irq(sdata->client->irq);
+
+	if (!wait_for_completion_timeout(&sdata->cmd_done,
+					 msecs_to_jiffies(timeout))) {
+		if (sdata->reset_gpio) {
+			dev_err(&sdata->client->dev, "controller not ready after reset");
+			err = -ETIMEDOUT;
+			goto err_disable_irq;
+		}
+		/*
+		 * If we did not reset the countroller ourselves continue
+		 * even if we did not receive "ready" message.
+		 */
 	}
 
 	err = stmfts_read_system_info(sdata);

No need to resend the series unless there is more feedback on the
bindings.

Thanks.

-- 
Dmitry