Hi David,
On Mon, Sep 07, 2026 at 12:50:19PM +0200, David Heidelberg via B4 Relay wrote:
>
> -static void stmfts_reset(struct stmfts_data *sdata)
> +static int stmfts_reset(struct stmfts_data *sdata)
> {
> gpiod_set_value_cansleep(sdata->reset_gpio, 1);
> msleep(20);
>
> + reinit_completion(&sdata->cmd_done);
> gpiod_set_value_cansleep(sdata->reset_gpio, 0);
> - msleep(50);
> + enable_irq(sdata->client->irq);
> +
> + if (!wait_for_completion_timeout(&sdata->cmd_done,
> +
> msecs_to_jiffies(STMFTS_RESET_TIMEOUT_MS)))
> + return -ETIMEDOUT;
> +
> + return 0;
> }
>
> static int stmfts_configure(struct stmfts_data *sdata)
> {
> int err;
>
> err = stmfts_command(sdata, STMFTS_SYSTEM_RESET);
> if (err)
> @@ -593,42 +602,47 @@ static int stmfts_power_on(struct stmfts_data *sdata)
> 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)
> - stmfts_reset(sdata);
> + if (sdata->reset_gpio) {
> + err = stmfts_reset(sdata);
> + if (err) {
> + dev_err(&sdata->client->dev,
> + "controller not ready after reset: %d\n", err);
> + goto err_disable_irq;
> + }
> + } else {
> + enable_irq(sdata->client->irq);
> + msleep(50);
> + }
>
> err = stmfts_read_system_info(sdata);
> if (err)
> - goto err_disable_regulators;
> -
> - enable_irq(sdata->client->irq);
> -
> - msleep(50);
> + goto err_disable_irq;
I think the logic is becoming quite convoluted here, and factored out
stmfts_reset() does not help. How about we make it look like this:
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;
if (sdata->reset_gpio) {
reinit_completion(&sdata->cmd_done);
/*
* 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);
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);
}
err = sdata->ops->configure(sdata);
if (err)
goto err_disable_irq;
/*
* At this point no one is using the touchscreen
* and I don't really care about the return value
*/
(void)i2c_smbus_write_byte(sdata->client, STMFTS_SLEEP_IN);
return 0;
err_disable_irq:
disable_irq(sdata->client->irq);
regulator_bulk_disable(ARRAY_SIZE(stmfts_supplies), sdata->supplies);
return err;
}
Thanks.
--
Dmitry
