Re: [PATCH] Input: elan_i2c - Fix regulator enable count imbalance after suspend/resume

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

Hi Hans,

On Wed, Dec 22, 2021 at 11:06:41PM +0100, Hans de Goede wrote:
> Before these changes elan_suspend() would only call elan_disable_power()
> when device_may_wakeup() returns false; whereas elan_resume() would
> unconditionally call elan_enable_power(), leading to an enable count
> imbalance when device_may_wakeup() returns true.
> 
> This triggers the "WARN_ON(regulator->enable_count)" in regulator_put()
> when the elan_i2c driver gets unbound, this happens e.g. with the
> hot-plugable dock with Elan I2C touchpad for the Asus TF103C 2-in-1.
> 
> Fix this by making the elan_enable_power() call also be conditional
> on device_may_wakeup() returning false.

elan_enable_power() not only tries to toggle the regulator, but also
tries to issue command to the controller to power/wake it up. I need
confirmation from Jingle Wu that skipping this is OK for all Elan
touchpad controllers, or if we need to add call to either power control
or sleep control method to make sure the controller it fully resumed.

Jingle, what can you tell us here?

> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/input/mouse/elan_i2c_core.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> index 47af62c12267..cdb36d35ffa4 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -1412,17 +1412,17 @@ static int __maybe_unused elan_resume(struct device *dev)
>  	struct elan_tp_data *data = i2c_get_clientdata(client);
>  	int error;
>  
> -	if (device_may_wakeup(dev) && data->irq_wake) {
> +	if (!device_may_wakeup(dev)) {
> +		error = elan_enable_power(data);
> +		if (error) {
> +			dev_err(dev, "power up when resuming failed: %d\n", error);
> +			goto err;
> +		}
> +	} else if (data->irq_wake) {
>  		disable_irq_wake(client->irq);
>  		data->irq_wake = false;
>  	}
>  
> -	error = elan_enable_power(data);
> -	if (error) {
> -		dev_err(dev, "power up when resuming failed: %d\n", error);
> -		goto err;
> -	}
> -
>  	error = elan_initialize(data, data->quirks & ETP_QUIRK_QUICK_WAKEUP);
>  	if (error)
>  		dev_err(dev, "initialize when resuming failed: %d\n", error);
> -- 
> 2.33.1
> 

Thanks.

-- 
Dmitry