On Fri, Sep 25, 2026 at 11:22:19PM +0200, David Heidelberg via B4 Relay wrote:
> From: David Heidelberg <[email protected]>
> 
> 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: [email protected]
> Co-developed-by: Dmitry Torokhov <[email protected]>
> Signed-off-by: David Heidelberg <[email protected]>
> ---
>  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

Reply via email to