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

[David Heidelberg <david@ixit.cz>]

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.
>