* [PATCH] acpi: trigger wakeup key event from power button
@ 2023-09-07 7:43 Ken Xue
2023-09-07 18:51 ` kernel test robot
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Ken Xue @ 2023-09-07 7:43 UTC (permalink / raw)
To: linux-acpi, rafael, andriy.shevchenko; +Cc: Ken.Xue
Andorid can wakeup from various wakeup sources,
but only several wakeup sources can wake up screen
with right events(POWER, WAKEUP) from input device.
Regarding pressing acpi power button, it can resume system and
ACPI_BITMASK_WAKE_STATUS and ACPI_BITMASK_POWER_BUTTON_STATUS
are set in pm1a_sts, but kernel does not report any key
event to user space during resume by default.
So, trigger wakeup key event to user space during resume
from power button.
Signed-off-by: Ken Xue <Ken.Xue@amd.com>
---
drivers/acpi/button.c | 18 ++++++++++++++++++
drivers/acpi/sleep.c | 2 ++
include/acpi/button.h | 5 +++++
3 files changed, 25 insertions(+)
diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index 1e76a64cce0a..22f41526731e 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -363,6 +363,23 @@ static int acpi_button_remove_fs(struct acpi_device *device)
return 0;
}
+void acpi_power_button_wakeup(struct acpi_device *device)
+{
+ struct acpi_button *button = acpi_driver_data(device);
+ struct input_dev *input;
+
+ if (button->type == ACPI_BUTTON_TYPE_POWER) {
+ input = button->input;
+ input_report_key(input, KEY_WAKEUP, 1);
+ input_sync(input);
+ input_report_key(input, KEY_WAKEUP, 0);
+ input_sync(input);
+ }
+
+ return;
+}
+EXPORT_SYMBOL(acpi_power_button_wakeup);
+
/* Driver Interface */
int acpi_lid_open(void)
{
@@ -579,6 +596,7 @@ static int acpi_button_add(struct acpi_device *device)
switch (button->type) {
case ACPI_BUTTON_TYPE_POWER:
input_set_capability(input, EV_KEY, KEY_POWER);
+ input_set_capability(input, EV_KEY, KEY_WAKEUP);
break;
case ACPI_BUTTON_TYPE_SLEEP:
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 808484d11209..dcd5d0237eeb 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -22,6 +22,7 @@
#include <linux/syscore_ops.h>
#include <asm/io.h>
#include <trace/events/power.h>
+#include <acpi/button.h>
#include "internal.h"
#include "sleep.h"
@@ -507,6 +508,7 @@ static void acpi_pm_finish(void)
pwr_btn_adev = acpi_dev_get_first_match_dev(ACPI_BUTTON_HID_POWERF,
NULL, -1);
if (pwr_btn_adev) {
+ acpi_power_button_wakeup(pwr_btn_adev);
pm_wakeup_event(&pwr_btn_adev->dev, 0);
acpi_dev_put(pwr_btn_adev);
}
diff --git a/include/acpi/button.h b/include/acpi/button.h
index af2fce5d2ee3..67e2dab27401 100644
--- a/include/acpi/button.h
+++ b/include/acpi/button.h
@@ -8,11 +8,16 @@
#if IS_ENABLED(CONFIG_ACPI_BUTTON)
extern int acpi_lid_open(void);
+extern void acpi_power_button_wakeup(struct acpi_device *device);
#else
static inline int acpi_lid_open(void)
{
return 1;
}
+static inline void acpi_power_button_wakeup(struct acpi_device *device)
+{
+ return;
+}
#endif /* IS_ENABLED(CONFIG_ACPI_BUTTON) */
#endif /* ACPI_BUTTON_H */
--
2.35.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] acpi: trigger wakeup key event from power button
2023-09-07 7:43 [PATCH] acpi: trigger wakeup key event from power button Ken Xue
@ 2023-09-07 18:51 ` kernel test robot
2023-09-07 19:34 ` kernel test robot
2023-09-07 20:06 ` kernel test robot
2 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2023-09-07 18:51 UTC (permalink / raw)
To: Ken Xue, linux-acpi, rafael, andriy.shevchenko; +Cc: oe-kbuild-all, Ken.Xue
Hi Ken,
kernel test robot noticed the following build warnings:
[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on linus/master v6.5 next-20230907]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ken-Xue/acpi-trigger-wakeup-key-event-from-power-button/20230907-232828
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link: https://lore.kernel.org/r/20230907074342.7722-1-Ken.Xue%40amd.com
patch subject: [PATCH] acpi: trigger wakeup key event from power button
config: riscv-defconfig (https://download.01.org/0day-ci/archive/20230908/202309080239.IiC7uLpW-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230908/202309080239.IiC7uLpW-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309080239.IiC7uLpW-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/gpu/drm/nouveau/nouveau_connector.c:27:
>> include/acpi/button.h:11:45: warning: 'struct acpi_device' declared inside parameter list will not be visible outside of this definition or declaration
11 | extern void acpi_power_button_wakeup(struct acpi_device *device);
| ^~~~~~~~~~~
vim +11 include/acpi/button.h
8
9 #if IS_ENABLED(CONFIG_ACPI_BUTTON)
10 extern int acpi_lid_open(void);
> 11 extern void acpi_power_button_wakeup(struct acpi_device *device);
12 #else
13 static inline int acpi_lid_open(void)
14 {
15 return 1;
16 }
17 static inline void acpi_power_button_wakeup(struct acpi_device *device)
18 {
19 return;
20 }
21 #endif /* IS_ENABLED(CONFIG_ACPI_BUTTON) */
22
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] acpi: trigger wakeup key event from power button
2023-09-07 7:43 [PATCH] acpi: trigger wakeup key event from power button Ken Xue
2023-09-07 18:51 ` kernel test robot
@ 2023-09-07 19:34 ` kernel test robot
2023-09-07 20:06 ` kernel test robot
2 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2023-09-07 19:34 UTC (permalink / raw)
To: Ken Xue, linux-acpi, rafael, andriy.shevchenko; +Cc: oe-kbuild-all, Ken.Xue
Hi Ken,
kernel test robot noticed the following build errors:
[auto build test ERROR on rafael-pm/linux-next]
[also build test ERROR on linus/master v6.5 next-20230907]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ken-Xue/acpi-trigger-wakeup-key-event-from-power-button/20230907-232828
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link: https://lore.kernel.org/r/20230907074342.7722-1-Ken.Xue%40amd.com
patch subject: [PATCH] acpi: trigger wakeup key event from power button
config: i386-randconfig-r032-20230908 (https://download.01.org/0day-ci/archive/20230908/202309080315.txQUEyHQ-lkp@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230908/202309080315.txQUEyHQ-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309080315.txQUEyHQ-lkp@intel.com/
All errors (new ones prefixed by >>):
ld: drivers/acpi/sleep.o: in function `acpi_pm_finish':
>> drivers/acpi/sleep.c:511: undefined reference to `acpi_power_button_wakeup'
vim +511 drivers/acpi/sleep.c
469
470 /**
471 * acpi_pm_finish - Instruct the platform to leave a sleep state.
472 *
473 * This is called after we wake back up (or if entering the sleep state
474 * failed).
475 */
476 static void acpi_pm_finish(void)
477 {
478 struct acpi_device *pwr_btn_adev;
479 u32 acpi_state = acpi_target_sleep_state;
480
481 acpi_ec_unblock_transactions();
482 suspend_nvs_free();
483
484 if (acpi_state == ACPI_STATE_S0)
485 return;
486
487 pr_info("Waking up from system sleep state S%d\n", acpi_state);
488 acpi_disable_wakeup_devices(acpi_state);
489 acpi_leave_sleep_state(acpi_state);
490
491 /* reset firmware waking vector */
492 acpi_set_waking_vector(0);
493
494 acpi_target_sleep_state = ACPI_STATE_S0;
495
496 acpi_resume_power_resources();
497
498 /* If we were woken with the fixed power button, provide a small
499 * hint to userspace in the form of a wakeup event on the fixed power
500 * button device (if it can be found).
501 *
502 * We delay the event generation til now, as the PM layer requires
503 * timekeeping to be running before we generate events. */
504 if (!pwr_btn_event_pending)
505 return;
506
507 pwr_btn_event_pending = false;
508 pwr_btn_adev = acpi_dev_get_first_match_dev(ACPI_BUTTON_HID_POWERF,
509 NULL, -1);
510 if (pwr_btn_adev) {
> 511 acpi_power_button_wakeup(pwr_btn_adev);
512 pm_wakeup_event(&pwr_btn_adev->dev, 0);
513 acpi_dev_put(pwr_btn_adev);
514 }
515 }
516
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] acpi: trigger wakeup key event from power button
2023-09-07 7:43 [PATCH] acpi: trigger wakeup key event from power button Ken Xue
2023-09-07 18:51 ` kernel test robot
2023-09-07 19:34 ` kernel test robot
@ 2023-09-07 20:06 ` kernel test robot
2 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2023-09-07 20:06 UTC (permalink / raw)
To: Ken Xue, linux-acpi, rafael, andriy.shevchenko
Cc: llvm, oe-kbuild-all, Ken.Xue
Hi Ken,
kernel test robot noticed the following build warnings:
[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on linus/master next-20230907]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ken-Xue/acpi-trigger-wakeup-key-event-from-power-button/20230907-232828
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link: https://lore.kernel.org/r/20230907074342.7722-1-Ken.Xue%40amd.com
patch subject: [PATCH] acpi: trigger wakeup key event from power button
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20230908/202309080351.xHt2qhP2-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230908/202309080351.xHt2qhP2-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309080351.xHt2qhP2-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/gpu/drm/i915/display/intel_lvds.c:30:
>> include/acpi/button.h:11:45: warning: declaration of 'struct acpi_device' will not be visible outside of this function [-Wvisibility]
extern void acpi_power_button_wakeup(struct acpi_device *device);
^
1 warning generated.
vim +11 include/acpi/button.h
8
9 #if IS_ENABLED(CONFIG_ACPI_BUTTON)
10 extern int acpi_lid_open(void);
> 11 extern void acpi_power_button_wakeup(struct acpi_device *device);
12 #else
13 static inline int acpi_lid_open(void)
14 {
15 return 1;
16 }
17 static inline void acpi_power_button_wakeup(struct acpi_device *device)
18 {
19 return;
20 }
21 #endif /* IS_ENABLED(CONFIG_ACPI_BUTTON) */
22
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] acpi: trigger wakeup key event from power button
@ 2023-09-08 9:57 Ken Xue
2023-09-11 9:42 ` Andy Shevchenko
0 siblings, 1 reply; 9+ messages in thread
From: Ken Xue @ 2023-09-08 9:57 UTC (permalink / raw)
To: linux-acpi
Cc: Ken.Xue, andriy.shevchenko, oe-kbuild-all, rafael,
kernel test robot
Andorid can wakeup from various wakeup sources,
but only several wakeup sources can wake up screen
with right events(POWER, WAKEUP) from input device.
Regarding pressing acpi power button, it can resume system and
ACPI_BITMASK_WAKE_STATUS and ACPI_BITMASK_POWER_BUTTON_STATUS
are set in pm1a_sts, but kernel does not report any key
event to user space during resume by default.
So, trigger wakeup key event to user space during resume
from power button.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202309080315.txQUEyHQ-lkp@intel.com/
Closes: https://lore.kernel.org/oe-kbuild-all/202309080239.IiC7uLpW-lkp@intel.com/
Closes: https://lore.kernel.org/oe-kbuild-all/202309080351.xHt2qhP2-lkp@intel.com/
Signed-off-by: Ken Xue <Ken.Xue@amd.com>
---
drivers/acpi/button.c | 16 ++++++++++++++++
drivers/acpi/sleep.c | 2 ++
include/acpi/button.h | 6 ++++++
3 files changed, 24 insertions(+)
diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index 1e76a64cce0a..0e0f30286c22 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -363,6 +363,21 @@ static int acpi_button_remove_fs(struct acpi_device *device)
return 0;
}
+void acpi_power_button_wakeup(struct acpi_device *device)
+{
+ struct acpi_button *button = acpi_driver_data(device);
+ struct input_dev *input;
+
+ if (button->type == ACPI_BUTTON_TYPE_POWER) {
+ input = button->input;
+ input_report_key(input, KEY_WAKEUP, 1);
+ input_sync(input);
+ input_report_key(input, KEY_WAKEUP, 0);
+ input_sync(input);
+ }
+}
+EXPORT_SYMBOL(acpi_power_button_wakeup);
+
/* Driver Interface */
int acpi_lid_open(void)
{
@@ -579,6 +594,7 @@ static int acpi_button_add(struct acpi_device *device)
switch (button->type) {
case ACPI_BUTTON_TYPE_POWER:
input_set_capability(input, EV_KEY, KEY_POWER);
+ input_set_capability(input, EV_KEY, KEY_WAKEUP);
break;
case ACPI_BUTTON_TYPE_SLEEP:
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 808484d11209..dcd5d0237eeb 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -22,6 +22,7 @@
#include <linux/syscore_ops.h>
#include <asm/io.h>
#include <trace/events/power.h>
+#include <acpi/button.h>
#include "internal.h"
#include "sleep.h"
@@ -507,6 +508,7 @@ static void acpi_pm_finish(void)
pwr_btn_adev = acpi_dev_get_first_match_dev(ACPI_BUTTON_HID_POWERF,
NULL, -1);
if (pwr_btn_adev) {
+ acpi_power_button_wakeup(pwr_btn_adev);
pm_wakeup_event(&pwr_btn_adev->dev, 0);
acpi_dev_put(pwr_btn_adev);
}
diff --git a/include/acpi/button.h b/include/acpi/button.h
index af2fce5d2ee3..fa0fb170cfb5 100644
--- a/include/acpi/button.h
+++ b/include/acpi/button.h
@@ -2,17 +2,23 @@
#ifndef ACPI_BUTTON_H
#define ACPI_BUTTON_H
+#include <linux/acpi.h>
+
#define ACPI_BUTTON_HID_POWER "PNP0C0C"
#define ACPI_BUTTON_HID_LID "PNP0C0D"
#define ACPI_BUTTON_HID_SLEEP "PNP0C0E"
#if IS_ENABLED(CONFIG_ACPI_BUTTON)
extern int acpi_lid_open(void);
+extern void acpi_power_button_wakeup(struct acpi_device *device);
#else
static inline int acpi_lid_open(void)
{
return 1;
}
+static inline void acpi_power_button_wakeup(struct acpi_device *device)
+{
+}
#endif /* IS_ENABLED(CONFIG_ACPI_BUTTON) */
#endif /* ACPI_BUTTON_H */
base-commit: b483d3b8a54a544ab8854ca6dbb8d99c423b3ba4
--
2.35.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] acpi: trigger wakeup key event from power button
2023-09-08 9:57 Ken Xue
@ 2023-09-11 9:42 ` Andy Shevchenko
2023-09-12 5:32 ` Ken Xue
0 siblings, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2023-09-11 9:42 UTC (permalink / raw)
To: Ken Xue; +Cc: linux-acpi, oe-kbuild-all, rafael, kernel test robot
On Fri, Sep 08, 2023 at 05:57:49PM +0800, Ken Xue wrote:
> Andorid can wakeup from various wakeup sources,
> but only several wakeup sources can wake up screen
> with right events(POWER, WAKEUP) from input device.
>
> Regarding pressing acpi power button, it can resume system and
> ACPI_BITMASK_WAKE_STATUS and ACPI_BITMASK_POWER_BUTTON_STATUS
> are set in pm1a_sts, but kernel does not report any key
> event to user space during resume by default.
>
> So, trigger wakeup key event to user space during resume
> from power button.
> Reported-by: kernel test robot <lkp@intel.com>
Are you sure?
> Closes: https://lore.kernel.org/oe-kbuild-all/202309080315.txQUEyHQ-lkp@intel.com/
> Closes: https://lore.kernel.org/oe-kbuild-all/202309080239.IiC7uLpW-lkp@intel.com/
> Closes: https://lore.kernel.org/oe-kbuild-all/202309080351.xHt2qhP2-lkp@intel.com/
Are you sure?
>
>
Blank lines are not allowed in the tag block.
> Signed-off-by: Ken Xue <Ken.Xue@amd.com>
> ---
How this change is different to the previous patch you sent?
Do you forgot versioning?
Do you forgot changelog?
Please, read Submitting Patches documentation before trying again.
It will help to make your contribution nice and understandable.
...
> + if (button->type == ACPI_BUTTON_TYPE_POWER) {
if (... != )
return;
?
> + input = button->input;
> + input_report_key(input, KEY_WAKEUP, 1);
> + input_sync(input);
> + input_report_key(input, KEY_WAKEUP, 0);
> + input_sync(input);
> + }
...
> +#include <linux/acpi.h>
There is no users of this header.
Check how forward declaration can be used (as it's done in many other headers).
> +extern void acpi_power_button_wakeup(struct acpi_device *device);
...
> +static inline void acpi_power_button_wakeup(struct acpi_device *device)
> +{
> +}
This can be done on a single line.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] acpi: trigger wakeup key event from power button
2023-09-11 9:42 ` Andy Shevchenko
@ 2023-09-12 5:32 ` Ken Xue
2023-09-12 9:30 ` Andy Shevchenko
0 siblings, 1 reply; 9+ messages in thread
From: Ken Xue @ 2023-09-12 5:32 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-acpi, oe-kbuild-all, rafael, kernel test robot, cwhuang
On 2023/9/11 17:42, Andy Shevchenko wrote:
> On Fri, Sep 08, 2023 at 05:57:49PM +0800, Ken Xue wrote:
>> Andorid can wakeup from various wakeup sources,
>> but only several wakeup sources can wake up screen
>> with right events(POWER, WAKEUP) from input device.
>>
>> Regarding pressing acpi power button, it can resume system and
>> ACPI_BITMASK_WAKE_STATUS and ACPI_BITMASK_POWER_BUTTON_STATUS
>> are set in pm1a_sts, but kernel does not report any key
>> event to user space during resume by default.
>>
>> So, trigger wakeup key event to user space during resume
>> from power button.
>> Reported-by: kernel test robot <lkp@intel.com>
> Are you sure?
Thanks for review. Sorry for confusion.
1)The patch is trying to fix the android wakeup from power button issue
I can see Android can resume from S3 and wake up screen from USB keyboard.
but Android only can resume from S3 and fail to wake up screen from
power button.
opensource android-x86 tried to fix this issue with a patch
(https://git.osdn.net/view?p=android-x86/system-hardware-interfaces.git;a=commit;h=78688b149314ec16cb2d90507a5908e5f2ba0fda)
in upper layer system_suspend service as a WA. It emulates wakeup key
during resume in a user space thread. And it is still not part of
upstream Android.
I believe the root cause is no wakeup key event being reported in kernel
during the resume from acpi power button.
And I have verified this patch on linux also, it seems no side effect
for Linux resume with this patch.
2) test robot reported some compile warnings and errors detected by test
robot which is fixed in V2.
>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202309080315.txQUEyHQ-lkp@intel.com/
>> Closes: https://lore.kernel.org/oe-kbuild-all/202309080239.IiC7uLpW-lkp@intel.com/
>> Closes: https://lore.kernel.org/oe-kbuild-all/202309080351.xHt2qhP2-lkp@intel.com/
> Are you sure?
Just some errors/warnings from the v1 patch.
>
>>
> Blank lines are not allowed in the tag block.
>
>> Signed-off-by: Ken Xue <Ken.Xue@amd.com>
>> ---
> How this change is different to the previous patch you sent?
> Do you forgot versioning?
> Do you forgot changelog?
>
> Please, read Submitting Patches documentation before trying again.
> It will help to make your contribution nice and understandable.
>
> ...
sorry.
I will add V3 and change log later.
>
>> + if (button->type == ACPI_BUTTON_TYPE_POWER) {
> if (... != )
> return;
>
> ?
>
>> + input = button->input;
>> + input_report_key(input, KEY_WAKEUP, 1);
>> + input_sync(input);
>> + input_report_key(input, KEY_WAKEUP, 0);
>> + input_sync(input);
>> + }
> ...
>
>> +#include <linux/acpi.h>
> There is no users of this header.
>
> Check how forward declaration can be used (as it's done in many other headers).
>
Yes, "struct acpi_device" is defined in "include/acpi/acpi_bus.h", but include acpi_bus.h alone
will lead to more compile issues.
Regarding "forward declaration", how about
typedef struct acpi_device *acpi_device;
>> +extern void acpi_power_button_wakeup(struct acpi_device *device);
> ...
>
>> +static inline void acpi_power_button_wakeup(struct acpi_device *device)
>> +{
>> +}
> This can be done on a single line.
>
Ack.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] acpi: trigger wakeup key event from power button
2023-09-12 5:32 ` Ken Xue
@ 2023-09-12 9:30 ` Andy Shevchenko
2023-09-12 12:53 ` Ken Xue
0 siblings, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2023-09-12 9:30 UTC (permalink / raw)
To: Ken Xue; +Cc: linux-acpi, oe-kbuild-all, rafael, kernel test robot, cwhuang
On Tue, Sep 12, 2023 at 01:32:02PM +0800, Ken Xue wrote:
> On 2023/9/11 17:42, Andy Shevchenko wrote:
> > On Fri, Sep 08, 2023 at 05:57:49PM +0800, Ken Xue wrote:
...
> > > Reported-by: kernel test robot <lkp@intel.com>
> > Are you sure?
>
> Thanks for review. Sorry for confusion.
> 2) test robot reported some compile warnings and errors detected by test
> robot which is fixed in V2.
Yes and that's what I'm asking about. You are not supposed to add it as the
initial problem, the patch is trying to solve, has _not_ been reported by LKP,
hasn't it?
...
> > > Closes: https://lore.kernel.org/oe-kbuild-all/202309080315.txQUEyHQ-lkp@intel.com/
> > > Closes: https://lore.kernel.org/oe-kbuild-all/202309080239.IiC7uLpW-lkp@intel.com/
> > > Closes: https://lore.kernel.org/oe-kbuild-all/202309080351.xHt2qhP2-lkp@intel.com/
> > Are you sure?
>
> Just some errors/warnings from the v1 patch.
Same as above.
...
> > > +#include <linux/acpi.h>
> > There are no users of this header.
> >
> > Check how forward declaration can be used (as it's done in many other headers).
> >
> Yes, "struct acpi_device" is defined in "include/acpi/acpi_bus.h", but
> include acpi_bus.h alone will lead to more compile issues.
>
> Regarding "forward declaration", how about
>
> typedef struct acpi_device *acpi_device;
Is it a forward declaration?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] acpi: trigger wakeup key event from power button
2023-09-12 9:30 ` Andy Shevchenko
@ 2023-09-12 12:53 ` Ken Xue
0 siblings, 0 replies; 9+ messages in thread
From: Ken Xue @ 2023-09-12 12:53 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-acpi, oe-kbuild-all, rafael, kernel test robot, cwhuang
On 2023/9/12 17:30, Andy Shevchenko wrote:
> On Tue, Sep 12, 2023 at 01:32:02PM +0800, Ken Xue wrote:
>> On 2023/9/11 17:42, Andy Shevchenko wrote:
>>> On Fri, Sep 08, 2023 at 05:57:49PM +0800, Ken Xue wrote:
...
>> 2) test robot reported some compile warnings and errors detected by test
>> robot which is fixed in V2.
> Yes and that's what I'm asking about. You are not supposed to add it as the
> initial problem, the patch is trying to solve, has _not_ been reported by LKP,
> hasn't it?
>
> ...
>
>>>> Closes: https://lore.kernel.org/oe-kbuild-all/202309080315.txQUEyHQ-lkp@intel.com/
>>>> Closes: https://lore.kernel.org/oe-kbuild-all/202309080239.IiC7uLpW-lkp@intel.com/
>>>> Closes: https://lore.kernel.org/oe-kbuild-all/202309080351.xHt2qhP2-lkp@intel.com/
>>> Are you sure?
>> Just some errors/warnings from the v1 patch.
> Same as above.
Get it. Those info will not be included in commit message.
>
> ...
>
>>>> +#include <linux/acpi.h>
>>> There are no users of this header.
>>>
>>> Check how forward declaration can be used (as it's done in many other headers).
>>>
>> Yes, "struct acpi_device" is defined in "include/acpi/acpi_bus.h", but
>> include acpi_bus.h alone will lead to more compile issues.
>>
>> Regarding "forward declaration", how about
>>
>> typedef struct acpi_device *acpi_device;
> Is it a forward declaration?
Ok, I will use "struct acpi_device;"
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-09-12 12:54 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-07 7:43 [PATCH] acpi: trigger wakeup key event from power button Ken Xue
2023-09-07 18:51 ` kernel test robot
2023-09-07 19:34 ` kernel test robot
2023-09-07 20:06 ` kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2023-09-08 9:57 Ken Xue
2023-09-11 9:42 ` Andy Shevchenko
2023-09-12 5:32 ` Ken Xue
2023-09-12 9:30 ` Andy Shevchenko
2023-09-12 12:53 ` Ken Xue
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox