On 25/09/2026 21:27, Dmitry Torokhov wrote:
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, it seems to be better (thou would be nicer if we didn't have two
branches, but... what we can do).
I added you as Co-developed-by :)
David
Thanks.
