Re: [RFC PATCH] Input: elan_i2c_smbus - resend ENABLE_TP on wake from sleep (NOT TESTED, NEEDS TESTER!)

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] input: redundant ETP_SMBUS_ENABLE_TP command during initialization
- [Low] input: explicit failure path violation in elan_smbus_sleep_control()
--

commit a7e8e038348366c26f0b3d1a3a7f9e778cbce51a
Author: R. Larocque <rlarocque@disroot.org>

Input: elan_i2c_smbus - resend ENABLE_TP on wake from sleep (NOT TESTED, NEEDS TESTER!)

This commit updates the Elan SMBus sleep control to resend the enable command
when waking up the touchpad. This mirrors the initialization path to ensure
touch reporting resumes.

> diff --git a/drivers/input/mouse/elan_i2c_smbus.c b/drivers/input/mouse/elan_i2c_smbus.c
> index 0287441cda46d..dd86bd1739c55 100644
> --- a/drivers/input/mouse/elan_i2c_smbus.c
> +++ b/drivers/input/mouse/elan_i2c_smbus.c
> @@ -93,10 +93,25 @@ static int elan_smbus_set_mode(struct i2c_client *client, u8 mode)
>  
>  static int elan_smbus_sleep_control(struct i2c_client *client, bool sleep)
>  {
> +	int error;
> +
>  	if (sleep)
>  		return i2c_smbus_write_byte(client, ETP_SMBUS_SLEEP_CMD);

[Severity: Low]
Does this direct return bypass the preferred explicit failure path style?

The input subsystem maintainer style guidelines suggest expanding direct
returns into explicit error checks when a function has multiple failure
points. Should this be updated to assign the return value to error and return
explicitly?

> -	else
> -		return 0; /* XXX should we send ETP_SMBUS_ENABLE_TP here? */
> +
> +	/*
> +	 * The controller stops touch reporting while asleep; resend
> +	 * ETP_SMBUS_ENABLE_TP on wake, mirroring what is done during
> +	 * elan_smbus_initialize(), so that reporting actually resumes.
> +	 */
> +	error = i2c_smbus_write_byte(client, ETP_SMBUS_ENABLE_TP);

[Severity: Medium]
Will this cause a redundant enable command during initialization that
might confuse the hardware?

During device probe, the sequence calls the initialization routine and then
the sleep control routine to wake it up:

__elan_initialize()
   data->ops->initialize(client)
      -> elan_smbus_initialize() sends ETP_SMBUS_ENABLE_TP
   data->ops->sleep_control(client, false)
      -> elan_smbus_sleep_control() now sends ETP_SMBUS_ENABLE_TP again

Since this patch is explicitly untested on real hardware, is there a risk
that this double-command could cause unexpected states or fail the probe
sequence?

> +	if (error) {
> +		dev_err(&client->dev,
> +			"failed to re-enable touchpad after wake: %d\n",
> +			error);
> +		return error;
> +	}
> +
> +	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922232707.8949-1-rlarocque@disroot.org?part=1