linux-kernel - Re: [PATCH] Input: elan_i2c - Reduce the resume time for new devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YDx8M4Rhdi8hW4EO@google.com>
Date:   Sun, 28 Feb 2021 21:31:31 -0800
From:   Dmitry Torokhov <dmitry.torokhov@...il.com>
To:     "jingle.wu" <jingle.wu@....com.tw>
Cc:     linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
        phoenix@....com.tw, dave.wang@....com.tw, josh.chen@....com.tw
Subject: Re: [PATCH] Input: elan_i2c - Reduce the resume time for new devices

Hi Jingle,

On Fri, Feb 26, 2021 at 03:35:37PM +0800, jingle.wu wrote:
> @@ -273,10 +318,12 @@ static int __elan_initialize(struct elan_tp_data *data)
>  	bool woken_up = false;
>  	int error;
>  
> -	error = data->ops->initialize(client);
> -	if (error) {
> -		dev_err(&client->dev, "device initialize failed: %d\n", error);
> -		return error;
> +	if (!(data->quirks & ETP_QUIRK_SET_QUICK_WAKEUP_DEV)) {
> +		error = data->ops->initialize(client);
> +		if (error) {
> +			dev_err(&client->dev, "device initialize failed: %d\n", error);
> +			return error;
> +		}

So data->ops->initialize(client) essentially performs reset of the
controller (we may want to rename it even) and as far as I understand
you would want to avoid resetting the controller on newer devices,
right?

My question is how behavior of older devices differ from the new ones
(are they stay in "undefined" state at power up) and whether it is
possible to determine if controller is in operating mode. For example,
what would happen on older devices if we call elan_query_product() below
without resetting the controller?

I also think that while I can see us skipping reset in resume paths we
probably want to keep it in probe as we really do not know the state of
the device (was it powered up properly earlier, etc).

>  	}
>  
>  	error = elan_query_product(data);
> @@ -366,6 +413,8 @@ static int elan_query_device_info(struct elan_tp_data *data)
>  	if (error)
>  		return error;
>  
> +	data->quirks = elan_i2c_lookup_quirk(data->ic_type, data->product_id);
> +
>  	error = elan_get_fwinfo(data->ic_type, data->iap_version,
>  				&data->fw_validpage_count,
>  				&data->fw_signature_address,
> -- 
> 2.17.1
> 

Thanks.

-- 
Dmitry