Re: [PATCH WIP v2 08/11] Input: stmfts - add optional reset GPIO support

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The `stmfts_power_on` error path fails to assert the reset GPIO before disabling regulators, creating a back-powering risk.

Pre-existing issues:
- [High] The `stmfts_power_on` error path fails to disable the device interrupt if a failure occurs after the IRQ is enabled.
--

--- Patch [8]: [PATCH WIP v2 08/11] Input: stmfts - add optional reset GPIO support ---
commit af8956cdddb1b976c94ce0f3a47253f42629d7bf
Author: Petr Hodina <[email protected]>

Input: stmfts - add optional reset GPIO support
    
Add support for an optional "reset-gpios" property. If present, the
driver drives the reset line high at probe time and releases it during
power-on, after the regulators have been enabled.

> diff --git a/drivers/input/touchscreen/stmfts.c b/drivers/input/touchscreen/stmfts.c
> index 4dfaf3a6077d9..2c10256e09c5e 100644
> --- a/drivers/input/touchscreen/stmfts.c
> +++ b/drivers/input/touchscreen/stmfts.c

[ ... ]

> @@ -549,6 +559,9 @@ static int stmfts_power_on(struct stmfts_data *sdata)
>  	if (err)
>  		return err;
>  
> +	if (sdata->reset_gpio)
> +		stmfts_reset(sdata);
> +

[Severity: High]
Does this error path properly unwind the reset GPIO state?

If an error occurs later in stmfts_power_on() (for example, if
stmfts_read_system_info() fails), the code jumps to the power_off label:

power_off:
	regulator_bulk_disable(ARRAY_SIZE(stmfts_supplies),
			       sdata->supplies);
	return err;

Since this disables the regulators unconditionally without first reverting the
reset GPIO to its asserted state (as done in stmfts_power_off()), could this
leave the GPIO driving a voltage into an unpowered touchscreen controller?

This could potentially back-power the chip through its internal ESD protection
diodes, risking power leakage or hardware damage.

[Severity: High]
This is a pre-existing issue, but does the power_off label also fail to unwind
the IRQ state if a failure occurs after the IRQ is enabled?

If stmfts_command(sdata, STMFTS_SYSTEM_RESET) fails:

	enable_irq(sdata->client->irq);

	msleep(50);

	err = stmfts_command(sdata, STMFTS_SYSTEM_RESET);
	if (err)
		goto power_off;

The power_off label disables the regulators but fails to call disable_irq().

Could this leak an enabled IRQ line while powering down the device,
potentially triggering an interrupt storm or an IRQ depth imbalance WARN on
subsequent resumes?

>  	/*
>  	 * The datasheet does not specify the power on time, but considering
>  	 * that the reset time is < 10ms, I sleep 20ms to be sure

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8