| 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: <CAN+gG=EoXd=J13Ky1tBn-bDAgVQWV=4-hQ8eMktBdgPXCu5+TQ@mail.gmail.com>
Date: Fri, 8 Jul 2016 11:27:23 +0200
From: Benjamin Tissoires <benjamin.tissoires@...il.com>
To: Lv Zheng <lv.zheng@...el.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
"Rafael J. Wysocki" <rjw@...ysocki.net>,
Len Brown <len.brown@...el.com>, Lv Zheng <zetalog@...il.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
"Bastien Nocera:" <hadess@...ess.net>,
linux-input <linux-input@...r.kernel.org>,
Dmitry Torokhov <dmitry.torokhov@...il.com>
Subject: Re: [PATCH v2 3/4] ACPI / button: Add SW_ACPI_LID for new usage model
On Thu, Jul 7, 2016 at 9:10 AM, Lv Zheng <lv.zheng@...el.com> wrote:
> There are many AML tables reporting wrong initial lid state, and some of
> them never reports lid state. As a proxy layer acting between, ACPI button
> driver is not able to handle all such cases, but need to re-define the
> usage model of the ACPI lid. That is:
> 1. It's initial state is not reliable;
> 2. There may not be open event;
> 3. Userspace should only take action against the close event which is
> reliable, always sent after a real lid close.
> This patch adds a new input key event so that new userspace programs can
> use it to handle this usage model correctly. And in the meanwhile, no old
> programs will be broken by the userspace changes.
>
> Link: https://lkml.org/2016/3/7/460
> Link: https://github.com/systemd/systemd/issues/2087
> Signed-off-by: Lv Zheng <lv.zheng@...el.com>
> Cc: Bastien Nocera: <hadess@...ess.net>
> Cc: Benjamin Tissoires <benjamin.tissoires@...il.com>
> Cc: linux-input@...r.kernel.org
> ---
> drivers/acpi/button.c | 20 ++++++++++++++------
> include/linux/mod_devicetable.h | 2 +-
> include/uapi/linux/input-event-codes.h | 3 ++-
> 3 files changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
> index 148f4e5..4ef94d2 100644
> --- a/drivers/acpi/button.c
> +++ b/drivers/acpi/button.c
> @@ -130,7 +130,8 @@ static int acpi_lid_evaluate_state(struct acpi_device *device)
> return lid_state ? 1 : 0;
> }
>
> -static int acpi_lid_notify_state(struct acpi_device *device, int state)
> +static int acpi_lid_notify_state(struct acpi_device *device,
> + int state, bool notify_acpi)
> {
> struct acpi_button *button = acpi_driver_data(device);
> int ret;
> @@ -138,6 +139,11 @@ static int acpi_lid_notify_state(struct acpi_device *device, int state)
> /* input layer checks if event is redundant */
> input_report_switch(button->input, SW_LID, !state);
> input_sync(button->input);
> + if (notify_acpi) {
> + input_report_switch(button->input,
> + SW_ACPI_LID, !state);
> + input_sync(button->input);
If you use a switch, you'll never send subsequent open state if you
doesn't close it yourself.
See my comments in 5/5 and please use a KEY event instead.
> + }
>
> if (state)
> pm_wakeup_event(&device->dev, 0);
> @@ -279,7 +285,8 @@ int acpi_lid_open(void)
> }
> EXPORT_SYMBOL(acpi_lid_open);
>
> -static int acpi_lid_update_state(struct acpi_device *device)
> +static int acpi_lid_update_state(struct acpi_device *device,
> + bool notify_acpi)
> {
> int state;
>
> @@ -287,17 +294,17 @@ static int acpi_lid_update_state(struct acpi_device *device)
> if (state < 0)
> return state;
>
> - return acpi_lid_notify_state(device, state);
> + return acpi_lid_notify_state(device, state, notify_acpi);
> }
>
> static void acpi_lid_initialize_state(struct acpi_device *device)
> {
> switch (lid_init_state) {
> case ACPI_BUTTON_LID_INIT_OPEN:
> - (void)acpi_lid_notify_state(device, 1);
> + (void)acpi_lid_notify_state(device, 1, false);
> break;
> case ACPI_BUTTON_LID_INIT_METHOD:
> - (void)acpi_lid_update_state(device);
> + (void)acpi_lid_update_state(device, false);
> break;
> case ACPI_BUTTON_LID_INIT_IGNORE:
> default:
> @@ -317,7 +324,7 @@ static void acpi_button_notify(struct acpi_device *device, u32 event)
> case ACPI_BUTTON_NOTIFY_STATUS:
> input = button->input;
> if (button->type == ACPI_BUTTON_TYPE_LID) {
> - acpi_lid_update_state(device);
> + acpi_lid_update_state(device, true);
> } else {
> int keycode;
>
> @@ -436,6 +443,7 @@ static int acpi_button_add(struct acpi_device *device)
>
> case ACPI_BUTTON_TYPE_LID:
> input_set_capability(input, EV_SW, SW_LID);
> + input_set_capability(input, EV_SW, SW_ACPI_LID);
Can't we export this new event only if the _LID function is not
reliable? This could check for the module parameter lid_init_state and
only enable it for ACPI_BUTTON_LID_INIT_OPEN.
I really hope we will be able to find a reliable way to determine
whether or not the platform support reliable LID state. If not, there
might be a need to have a db of reliable switch platforms. This can be
set in the kernel or with a hwdb entry in userspace.
Cheers,
Benjamin
> break;
> }
>
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 6e4c645..1014968 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -291,7 +291,7 @@ struct pcmcia_device_id {
> #define INPUT_DEVICE_ID_LED_MAX 0x0f
> #define INPUT_DEVICE_ID_SND_MAX 0x07
> #define INPUT_DEVICE_ID_FF_MAX 0x7f
> -#define INPUT_DEVICE_ID_SW_MAX 0x0f
> +#define INPUT_DEVICE_ID_SW_MAX 0x10
>
> #define INPUT_DEVICE_ID_MATCH_BUS 1
> #define INPUT_DEVICE_ID_MATCH_VENDOR 2
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 737fa32..81c344c 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -780,7 +780,8 @@
> #define SW_ROTATE_LOCK 0x0c /* set = rotate locked/disabled */
> #define SW_LINEIN_INSERT 0x0d /* set = inserted */
> #define SW_MUTE_DEVICE 0x0e /* set = device disabled */
> -#define SW_MAX 0x0f
> +#define SW_ACPI_LID 0x0f /* set = lid shut */
> +#define SW_MAX 0x10
> #define SW_CNT (SW_MAX+1)
>
> /*
> --
> 1.7.10
>
Powered by blists - more mailing lists