Dear Dmitry, Greetings!! Could you please help to review and update on this? Thanks and Regards, Abhishek Kumar Singh Sr. Chief Engineer, Samsung Electronics, Noida-India --------- Original Message --------- Sender : Abhishek Kumar Singh <abhi1.singh@xxxxxxxxxxx>System Power Part /SRI-Noida/Samsung Electronics Date : 2023-11-07 13:59 (GMT+5:30) Title : 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@xxxxxxxxx <dmitry.torokhov@xxxxxxxxx> Date : 2023-10-29 07:42 (GMT+5:30) Title : Re: [PATCH] input: gpio-keys - optimize wakeup sequence. To : Abhishek Kumar Singh<abhi1.singh@xxxxxxxxxxx> CC : robh@xxxxxxxxxx<robh@xxxxxxxxxx>, linux-input@xxxxxxxxxxxxxxx<linux-input@xxxxxxxxxxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx<linux-kernel@xxxxxxxxxxxxxxx>, SRI-N IT Security<sri-n.itsec@xxxxxxxxxxx> 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, Thanks and Regards, Thanks and Regards,
