| 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
| ||
|
Message-ID: <20231107082901epcms5p80c830aecfe50bfb12b661defad701149@epcms5p8>
Date: Tue, 07 Nov 2023 13:59:01 +0530
From: Abhishek Kumar Singh <abhi1.singh@...sung.com>
To: "dmitry.torokhov@...il.com" <dmitry.torokhov@...il.com>
CC: "robh@...nel.org" <robh@...nel.org>,
"linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
SRI-N IT Security <sri-n.itsec@...sung.com>
Subject: Re[2]: [PATCH] input: gpio-keys - optimize wakeup sequence.
Dear Dmitry,
Greetings!!!
Thank you so much for your response.
I checked in detailed again and observed the below points, please help to review
and approve it.
There is ISR "gpio_keys_gpio_isr" which is called when the key state is 1 i.e.
key pressed.
Therefore modified code will not impact on the existing driver code.
//For key pressed event:
<3>[ 549.180072] I[0: swapper/0: 0] gpio_keys_gpio_isr
<3>[ 549.196126] [1: kworker/1:1: 78] gpio_keys_gpio_work_func
<3>[ 549.196198] [1: kworker/1:1: 78] gpio-keys soc:gpio_keys: gpio_keys_gpio_report_event key = 115, value = 1
Performance:
I have calculated the differece between entry & exit time
with modified and without modified code and observed that
0.3ms extra computation time in current scenario in each entry/exit time.
Because below APIs will not be called in every resume functions:
1. static void gpio_keys_report_state(struct gpio_keys_drvdata *ddata)
2. static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
3. gpiod_get_value_cansleep(bdata->gpiod);
4. input_event(input, type, *bdata->code, state);
5. input_sync(input)
So we can save 0.3ms computation time, resume operations will faster and save battery as well.
With changes:
Line 311960: 07-18 16:50:09.359 root 0 0 I [0: Binder:699_1:20972] PM: gpio_keys_report_state exit 2023-07-18 11:20:37.573207725 UTC
Line 312626: 07-18 16:50:42.123 root 0 0 I [0: Binder:699_1:20972] PM: gpio_keys_report_state enrty 2023-07-18 11:22:20.503579404 UTC
Line 312627: 07-18 16:50:42.123 root 0 0 I [0: Binder:699_1:20972] PM: gpio_keys_report_state exit 2023-07-18 11:22:20.503656644 UTC
Line 313301: 07-18 16:52:24.182 root 0 0 I [0: Binder:699_1:20972] PM: gpio_keys_report_state enrty 2023-07-18 11:22:33.865626325 UTC
Line 313302: 07-18 16:52:24.182 root 0 0 I [0: Binder:699_1:20972] PM: gpio_keys_report_state exit 2023-07-18 11:22:33.865724502 UTC
Line 313572: 07-18 16:52:35.111 root 0 0 I [1: Binder:699_1:20972] PM: gpio_keys_report_state enrty 2023-07-18 11:22:37.678468979 UTC
Line 313573: 07-18 16:52:35.111 root 0 0 I [1: Binder:699_1:20972] PM: gpio_keys_report_state exit 2023-07-18 11:22:37.678566167 UTC
Line 314209: 07-18 16:52:43.598 root 0 0 I [0: Binder:699_1:20972] PM: gpio_keys_report_state enrty 2023-07-18 11:23:05.925340634 UTC
Line 314210: 07-18 16:52:43.598 root 0 0 I [0: Binder:699_1:20972] PM: gpio_keys_report_state exit 2023-07-18 11:23:05.925439384 UTC
Without changes:
Line 372095: 07-18 16:10:24.250 root 0 0 I [1: Binder:702_2:18137] PM: gpio_keys_report_state exit 2023-07-18 10:43:38.592548979 UTC
Line 372344: 07-18 16:13:45.439 root 0 0 I [0: Binder:702_2:18137] PM: gpio_keys_report_state enrty 2023-07-18 10:44:11.589164226 UTC
Line 372346: 07-18 16:13:45.439 root 0 0 I [0: Binder:702_2:18137] PM: gpio_keys_report_state exit 2023-07-18 10:44:11.589514955 UTC
Line 372573: 07-18 16:14:13.414 root 0 0 I [0: Binder:702_2:18137] PM: gpio_keys_report_state enrty 2023-07-18 10:44:22.606227138 UTC
Line 372575: 07-18 16:14:13.414 root 0 0 I [0: Binder:702_2:18137] PM: gpio_keys_report_state exit 2023-07-18 10:44:22.606490107 UTC
Line 372944: 07-18 16:14:26.732 root 0 0 I [1: Binder:702_2:18137] PM: gpio_keys_report_state enrty 2023-07-18 10:44:29.024121927 UTC
Line 372946: 07-18 16:14:26.732 root 0 0 I [1: Binder:702_2:18137] PM: gpio_keys_report_state exit 2023-07-18 10:44:29.024528958 UTC
Line 373181: 07-18 16:14:30.790 root 0 0 I [0: Binder:702_2:18137] PM: gpio_keys_report_state enrty 2023-07-18 10:44:30.904866770 UTC
Line 373183: 07-18 16:14:30.790 root 0 0 I [0: Binder:702_2:18137] PM: gpio_keys_report_state exit 2023-07-18 10:44:30.905126353 UTC
Thanks and Regards,
Abhishek Kumar Singh
Sr. Chief Engineer, Samsung Electronics, Noida-India
--------- Original Message ---------
Sender : dmitry.torokhov@...il.com <dmitry.torokhov@...il.com>
Date : 2023-10-29 07:42 (GMT+5:30)
Title : Re: [PATCH] input: gpio-keys - optimize wakeup sequence.
To : Abhishek Kumar Singh<abhi1.singh@...sung.com>
CC : robh@...nel.org<robh@...nel.org>, linux-input@...r.kernel.org<linux-input@...r.kernel.org>, linux-kernel@...r.kernel.org<linux-kernel@...r.kernel.org>, SRI-N IT Security<sri-n.itsec@...sung.com>
Hi Abhishek,
On Thu, Oct 26, 2023 at 11:23:20AM +0530, Abhishek Kumar Singh wrote:
> Dear Mr. Dmitry,
> Greetings!
>
>
> The patch removes unused many APIs call chain for every suspend/resume of the device
> if no key press event triggered.
>
>
> There is a call back function gpio_keys_resume() called for
> every suspend/resume of the device. and whenever this function called, it is
> reading the status of the key. And gpio_keys_resume() API further calls the
> below chain of API irrespective of key press event
>
>
> APIs call chain:
> static void gpio_keys_report_state(struct gpio_keys_drvdata *ddata)
> static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
> gpiod_get_value_cansleep(bdata->gpiod);
> input_event(input, type, *bdata->code, state);
> input_sync(input);
>
>
> The patch avoid the above APIs call chain if there is no key press event triggered.
> It will save the device computational resources, power resources and optimize the suspend/resume time
Unfortunately it also breaks the driver as button->value does not hold
the current state of the GPIO but rather set one via device tree so that
the driver can use that value when sending EV_ABS events. So with
typical GPIO-backed keys or buttons you change results in no events
reported on resume.
I also wonder what kind of measurements you did on improvements to
suspend/resume time with your change.
Thanks.
--
Dmitry
Thanks and Regards,
Powered by blists - more mailing lists