Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1446227 > unrolled thread

[PATCH v4 1/2] ACPI / button: Add KEY_LID_OPEN/KEY_LID_CLOSE for new usage model

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v4 1/2] ACPI / button: Add KEY_LID_OPEN/KEY_LID_CLOSE for new usage model Lv Zheng <lv.zheng@intel.com> - 2016-07-19 10:20 +0200
    Re: [PATCH v4 1/2] ACPI / button: Add KEY_LID_OPEN/KEY_LID_CLOSE for  new usage model Benjamin Tissoires <benjamin.tissoires@gmail.com> - 2016-07-19 10:50 +0200
    Re: [PATCH v4 1/2] ACPI / button: Add KEY_LID_OPEN/KEY_LID_CLOSE for new usage model "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-07-21 15:40 +0200
      Re: [PATCH v4 1/2] ACPI / button: Add KEY_LID_OPEN/KEY_LID_CLOSE for  new usage model Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2016-07-21 22:40 +0200

#1446227 — [PATCH v4 1/2] ACPI / button: Add KEY_LID_OPEN/KEY_LID_CLOSE for new usage model

There are many AML tables reporting wrong initial lid state, and some of
them never report lid open 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 an open event;
3. Userspace should only take action against the close event which is
   reliable, always sent after a real lid close.

OTOH, using an input switch event for the lid device on such platforms can
cause the loss of the close event, but the platforms purposely want to use
these close events to trigger power saving actions.

So we need to introduce a new ABI, which is input key events based, not
input switch events based.

This patch adds a set of new input key events so that the new userspace
programs can use them to handle this usage model correctly. And in the
meanwhile, the old input switch event is kept so that no old programs will
be broken by the ABI change.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Cc: Bastien Nocera: <hadess@hadess.net>
Cc: linux-input@vger.kernel.org
---
 drivers/acpi/button.c                  |   34 +++++++++++++++++++++++++++++++-
 include/uapi/linux/input-event-codes.h |    7 +++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index 148f4e5..dd16879 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -104,6 +104,8 @@ struct acpi_button {
 	struct input_dev *input;
 	char phys[32];			/* for input device */
 	unsigned long pushed;
+	int last_state;
+	unsigned long timestamp;
 	bool suspended;
 };
 
@@ -111,6 +113,10 @@ static BLOCKING_NOTIFIER_HEAD(acpi_lid_notifier);
 static struct acpi_device *lid_device;
 static u8 lid_init_state = ACPI_BUTTON_LID_INIT_METHOD;
 
+static unsigned long lid_report_interval __read_mostly = 500;
+module_param(lid_report_interval, ulong, 0644);
+MODULE_PARM_DESC(lid_report_interval, "Interval (ms) between lid key events");
+
 /* --------------------------------------------------------------------------
                               FS Interface (/proc)
    -------------------------------------------------------------------------- */
@@ -133,12 +139,34 @@ static int acpi_lid_evaluate_state(struct acpi_device *device)
 static int acpi_lid_notify_state(struct acpi_device *device, int state)
 {
 	struct acpi_button *button = acpi_driver_data(device);
+	int keycode;
+	unsigned long timeout;
 	int ret;
 
-	/* input layer checks if event is redundant */
+	/*
+	 * Send the switch event.
+	 * The input layer checks if the event is redundant.
+	 */
 	input_report_switch(button->input, SW_LID, !state);
 	input_sync(button->input);
 
+	/*
+	 * Send the key event.
+	 * The input layer doesn't check if the event is redundant.
+	 */
+	timeout = button->timestamp +
+		  msecs_to_jiffies(lid_report_interval);
+	if (button->last_state != !!state ||
+	    time_after(jiffies, timeout)) {
+		keycode = state ? KEY_LID_OPEN : KEY_LID_CLOSE;
+		input_report_key(button->input, keycode, 1);
+		input_sync(button->input);
+		input_report_key(button->input, keycode, 0);
+		input_sync(button->input);
+		button->last_state = !!state;
+		button->timestamp = jiffies;
+	}
+
 	if (state)
 		pm_wakeup_event(&device->dev, 0);
 
@@ -407,6 +435,8 @@ static int acpi_button_add(struct acpi_device *device)
 		strcpy(name, ACPI_BUTTON_DEVICE_NAME_LID);
 		sprintf(class, "%s/%s",
 			ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_LID);
+		button->last_state = !!acpi_lid_evaluate_state(device);
+		button->timestamp = jiffies;
 	} else {
 		printk(KERN_ERR PREFIX "Unsupported hid [%s]\n", hid);
 		error = -ENODEV;
@@ -436,6 +466,8 @@ 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_KEY, KEY_LID_OPEN);
+		input_set_capability(input, EV_KEY, KEY_LID_CLOSE);
 		break;
 	}
 
diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 737fa32..b062fe1 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -641,6 +641,13 @@
  * e.g. teletext or data broadcast application (MHEG, MHP, HbbTV, etc.)
  */
 #define KEY_DATA			0x275
+/*
+ * Key events sent by the lid drivers.
+ * The drivers may not be able to send paired "open"/"close" events, in
+ * which case, they send KEY_LID_OPEN/KEY_LID_CLOSE instead of SW_LID.
+ */
+#define KEY_LID_OPEN			0x278
+#define KEY_LID_CLOSE			0x279
 
 #define BTN_TRIGGER_HAPPY		0x2c0
 #define BTN_TRIGGER_HAPPY1		0x2c0
-- 
1.7.10

[toc] | [next] | [standalone]

#1446247 — Re: [PATCH v4 1/2] ACPI / button: Add KEY_LID_OPEN/KEY_LID_CLOSE for new usage model

On Tue, Jul 19, 2016 at 10:11 AM, Lv Zheng <lv.zheng@intel.com> wrote:
> There are many AML tables reporting wrong initial lid state, and some of
> them never report lid open 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 an open event;
> 3. Userspace should only take action against the close event which is
>    reliable, always sent after a real lid close.
>
> OTOH, using an input switch event for the lid device on such platforms can
> cause the loss of the close event, but the platforms purposely want to use
> these close events to trigger power saving actions.
>
> So we need to introduce a new ABI, which is input key events based, not
> input switch events based.
>
> This patch adds a set of new input key events so that the new userspace
> programs can use them to handle this usage model correctly. And in the
> meanwhile, the old input switch event is kept so that no old programs will
> be broken by the ABI change.
>
> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Benjamin Tissoires <benjamin.tissoires@gmail.com>
> Cc: Bastien Nocera: <hadess@hadess.net>
> Cc: linux-input@vger.kernel.org
> ---

Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Cheers,
Benjamin

>  drivers/acpi/button.c                  |   34 +++++++++++++++++++++++++++++++-
>  include/uapi/linux/input-event-codes.h |    7 +++++++
>  2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
> index 148f4e5..dd16879 100644
> --- a/drivers/acpi/button.c
> +++ b/drivers/acpi/button.c
> @@ -104,6 +104,8 @@ struct acpi_button {
>         struct input_dev *input;
>         char phys[32];                  /* for input device */
>         unsigned long pushed;
> +       int last_state;
> +       unsigned long timestamp;
>         bool suspended;
>  };
>
> @@ -111,6 +113,10 @@ static BLOCKING_NOTIFIER_HEAD(acpi_lid_notifier);
>  static struct acpi_device *lid_device;
>  static u8 lid_init_state = ACPI_BUTTON_LID_INIT_METHOD;
>
> +static unsigned long lid_report_interval __read_mostly = 500;
> +module_param(lid_report_interval, ulong, 0644);
> +MODULE_PARM_DESC(lid_report_interval, "Interval (ms) between lid key events");
> +
>  /* --------------------------------------------------------------------------
>                                FS Interface (/proc)
>     -------------------------------------------------------------------------- */
> @@ -133,12 +139,34 @@ static int acpi_lid_evaluate_state(struct acpi_device *device)
>  static int acpi_lid_notify_state(struct acpi_device *device, int state)
>  {
>         struct acpi_button *button = acpi_driver_data(device);
> +       int keycode;
> +       unsigned long timeout;
>         int ret;
>
> -       /* input layer checks if event is redundant */
> +       /*
> +        * Send the switch event.
> +        * The input layer checks if the event is redundant.
> +        */
>         input_report_switch(button->input, SW_LID, !state);
>         input_sync(button->input);
>
> +       /*
> +        * Send the key event.
> +        * The input layer doesn't check if the event is redundant.
> +        */
> +       timeout = button->timestamp +
> +                 msecs_to_jiffies(lid_report_interval);
> +       if (button->last_state != !!state ||
> +           time_after(jiffies, timeout)) {
> +               keycode = state ? KEY_LID_OPEN : KEY_LID_CLOSE;
> +               input_report_key(button->input, keycode, 1);
> +               input_sync(button->input);
> +               input_report_key(button->input, keycode, 0);
> +               input_sync(button->input);
> +               button->last_state = !!state;
> +               button->timestamp = jiffies;
> +       }
> +
>         if (state)
>                 pm_wakeup_event(&device->dev, 0);
>
> @@ -407,6 +435,8 @@ static int acpi_button_add(struct acpi_device *device)
>                 strcpy(name, ACPI_BUTTON_DEVICE_NAME_LID);
>                 sprintf(class, "%s/%s",
>                         ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_LID);
> +               button->last_state = !!acpi_lid_evaluate_state(device);
> +               button->timestamp = jiffies;
>         } else {
>                 printk(KERN_ERR PREFIX "Unsupported hid [%s]\n", hid);
>                 error = -ENODEV;
> @@ -436,6 +466,8 @@ 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_KEY, KEY_LID_OPEN);
> +               input_set_capability(input, EV_KEY, KEY_LID_CLOSE);
>                 break;
>         }
>
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 737fa32..b062fe1 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -641,6 +641,13 @@
>   * e.g. teletext or data broadcast application (MHEG, MHP, HbbTV, etc.)
>   */
>  #define KEY_DATA                       0x275
> +/*
> + * Key events sent by the lid drivers.
> + * The drivers may not be able to send paired "open"/"close" events, in
> + * which case, they send KEY_LID_OPEN/KEY_LID_CLOSE instead of SW_LID.
> + */
> +#define KEY_LID_OPEN                   0x278
> +#define KEY_LID_CLOSE                  0x279
>
>  #define BTN_TRIGGER_HAPPY              0x2c0
>  #define BTN_TRIGGER_HAPPY1             0x2c0
> --
> 1.7.10
>

[toc] | [prev] | [next] | [standalone]

#1447889

On Tuesday, July 19, 2016 04:11:02 PM Lv Zheng wrote:
> There are many AML tables reporting wrong initial lid state, and some of
> them never report lid open 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 an open event;
> 3. Userspace should only take action against the close event which is
>    reliable, always sent after a real lid close.
> 
> OTOH, using an input switch event for the lid device on such platforms can
> cause the loss of the close event, but the platforms purposely want to use
> these close events to trigger power saving actions.
> 
> So we need to introduce a new ABI, which is input key events based, not
> input switch events based.
> 
> This patch adds a set of new input key events so that the new userspace
> programs can use them to handle this usage model correctly. And in the
> meanwhile, the old input switch event is kept so that no old programs will
> be broken by the ABI change.
> 
> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Benjamin Tissoires <benjamin.tissoires@gmail.com>
> Cc: Bastien Nocera: <hadess@hadess.net>
> Cc: linux-input@vger.kernel.org

Dmitry, any objections here?

> ---
>  drivers/acpi/button.c                  |   34 +++++++++++++++++++++++++++++++-
>  include/uapi/linux/input-event-codes.h |    7 +++++++
>  2 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
> index 148f4e5..dd16879 100644
> --- a/drivers/acpi/button.c
> +++ b/drivers/acpi/button.c
> @@ -104,6 +104,8 @@ struct acpi_button {
>  	struct input_dev *input;
>  	char phys[32];			/* for input device */
>  	unsigned long pushed;
> +	int last_state;
> +	unsigned long timestamp;
>  	bool suspended;
>  };
>  
> @@ -111,6 +113,10 @@ static BLOCKING_NOTIFIER_HEAD(acpi_lid_notifier);
>  static struct acpi_device *lid_device;
>  static u8 lid_init_state = ACPI_BUTTON_LID_INIT_METHOD;
>  
> +static unsigned long lid_report_interval __read_mostly = 500;
> +module_param(lid_report_interval, ulong, 0644);
> +MODULE_PARM_DESC(lid_report_interval, "Interval (ms) between lid key events");
> +
>  /* --------------------------------------------------------------------------
>                                FS Interface (/proc)
>     -------------------------------------------------------------------------- */
> @@ -133,12 +139,34 @@ static int acpi_lid_evaluate_state(struct acpi_device *device)
>  static int acpi_lid_notify_state(struct acpi_device *device, int state)
>  {
>  	struct acpi_button *button = acpi_driver_data(device);
> +	int keycode;
> +	unsigned long timeout;
>  	int ret;
>  
> -	/* input layer checks if event is redundant */
> +	/*
> +	 * Send the switch event.
> +	 * The input layer checks if the event is redundant.
> +	 */
>  	input_report_switch(button->input, SW_LID, !state);
>  	input_sync(button->input);
>  
> +	/*
> +	 * Send the key event.
> +	 * The input layer doesn't check if the event is redundant.
> +	 */
> +	timeout = button->timestamp +
> +		  msecs_to_jiffies(lid_report_interval);
> +	if (button->last_state != !!state ||
> +	    time_after(jiffies, timeout)) {
> +		keycode = state ? KEY_LID_OPEN : KEY_LID_CLOSE;
> +		input_report_key(button->input, keycode, 1);
> +		input_sync(button->input);
> +		input_report_key(button->input, keycode, 0);
> +		input_sync(button->input);
> +		button->last_state = !!state;
> +		button->timestamp = jiffies;
> +	}
> +
>  	if (state)
>  		pm_wakeup_event(&device->dev, 0);
>  
> @@ -407,6 +435,8 @@ static int acpi_button_add(struct acpi_device *device)
>  		strcpy(name, ACPI_BUTTON_DEVICE_NAME_LID);
>  		sprintf(class, "%s/%s",
>  			ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_LID);
> +		button->last_state = !!acpi_lid_evaluate_state(device);
> +		button->timestamp = jiffies;
>  	} else {
>  		printk(KERN_ERR PREFIX "Unsupported hid [%s]\n", hid);
>  		error = -ENODEV;
> @@ -436,6 +466,8 @@ 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_KEY, KEY_LID_OPEN);
> +		input_set_capability(input, EV_KEY, KEY_LID_CLOSE);
>  		break;
>  	}
>  
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 737fa32..b062fe1 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -641,6 +641,13 @@
>   * e.g. teletext or data broadcast application (MHEG, MHP, HbbTV, etc.)
>   */
>  #define KEY_DATA			0x275
> +/*
> + * Key events sent by the lid drivers.
> + * The drivers may not be able to send paired "open"/"close" events, in
> + * which case, they send KEY_LID_OPEN/KEY_LID_CLOSE instead of SW_LID.
> + */
> +#define KEY_LID_OPEN			0x278
> +#define KEY_LID_CLOSE			0x279
>  
>  #define BTN_TRIGGER_HAPPY		0x2c0
>  #define BTN_TRIGGER_HAPPY1		0x2c0

To this part in particular?

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1448127 — Re: [PATCH v4 1/2] ACPI / button: Add KEY_LID_OPEN/KEY_LID_CLOSE for new usage model

On Thu, Jul 21, 2016 at 03:35:36PM +0200, Rafael J. Wysocki wrote:
> On Tuesday, July 19, 2016 04:11:02 PM Lv Zheng wrote:
> > There are many AML tables reporting wrong initial lid state, and some of
> > them never report lid open 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 an open event;
> > 3. Userspace should only take action against the close event which is
> >    reliable, always sent after a real lid close.
> > 
> > OTOH, using an input switch event for the lid device on such platforms can
> > cause the loss of the close event, but the platforms purposely want to use
> > these close events to trigger power saving actions.
> > 
> > So we need to introduce a new ABI, which is input key events based, not
> > input switch events based.
> > 
> > This patch adds a set of new input key events so that the new userspace
> > programs can use them to handle this usage model correctly. And in the
> > meanwhile, the old input switch event is kept so that no old programs will
> > be broken by the ABI change.
> > 
> > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: Benjamin Tissoires <benjamin.tissoires@gmail.com>
> > Cc: Bastien Nocera: <hadess@hadess.net>
> > Cc: linux-input@vger.kernel.org
> 
> Dmitry, any objections here?

Yes I have (see the other email).

Thanks.

-- 
Dmitry

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web