[PATCH 0/2] [PATCH] ACPI / button: Change default behavior to avoid lid bug reports

[PATCH 0/2] [PATCH] ACPI / button: Change default behavior to avoid lid bug reports

[Lv Zheng]

There are still many users reporting LID issues to the kernel bugzilla as
there is no change from systemd and we don't have correct default behavior
for now or for the future. It's a headache to teach the reporters to use
button.lid_init_state=open report by report to correctly work the problems
around.

This patchset changes the default behavior of ACPI button driver in order
to reduce such bug reports.

Lv Zheng (2):
  ACPI / button: Change default behavior to lid_init_state=open
  ACPI / button: Remove lid_init_state=method mode

 Documentation/acpi/acpi-lid.txt | 16 ++++------------
 drivers/acpi/button.c           | 11 +----------
 2 files changed, 5 insertions(+), 22 deletions(-)

-- 
2.7.4

[PATCH 1/2] ACPI / button: Change default behavior to lid_init_state=open

[Lv Zheng]

More and more platforms need the button.lid_init_state=open quirk. This
patch sets it the default behavior.

If a platform doesn't send lid open event or lid open event is lost due to
the underlying system problems, then we can compare various combinations:
1. systemd/acpid is used to suspend system or not, systemd has a special
   logic forcing open event after resuming;
2. _LID returns a cached value or not.

The result is as follows:
1. lid_init_state=method
   1. cached
      1. resumed by lid:
         (x) event=close
         (x) systemd=suspends again
         (x) acpid=suspends again
         (x) state=close
      2. resumed by other:
         (o) event=close
         (x) systemd=suspends again
         (x) acpid=suspends again
         (o) state=close
   2. non-cached
      1. resumed by lid:
         (o) event=open
         (o) systemd=resumes
         (o) acpid=resumes
         (o) state=open
      2. resumed by other:
         (o) event=close
         (x) systemd=suspends again
         (x) acpid=suspends again
         (o) state=close
2. lid_init_state=open
   1. cached
      1. resumed by lid:
         (o) event=open
         (o) systemd=resumes
         (o) acpid=resumes
         (x) state=close
      2. resumed by other:
         (x) event=open
         (o) systemd=resumes
         (o) acpid=resumes
         (o) state=close
   2. non-cached
      1. resumed by lid:
         (o) event=open
         (o) systemd=resumes
         (o) acpid=resumes
         (o) state=open
      2. resumed by other:
         (x) event=open
         (o) systemd=resumes
         (o) acpid=resumes
         (o) state=close
3. lid_init_state=ignore
   1. cached
      1. resumed by lid:
         (o) event=none
         (x) systemd=suspends again
         (o) acpid=resumes
         (x) state=close
      2. resumed by other:
         (o) event=none
         (x) systemd=suspends again
         (o) acpid=resumes
         (o) state=close
   2. non-cached
      1. resumed by lid:
         (o) event=none
         (x) systemd=suspends again
         (o) acpid=resumes
         (o) state=open
      2. resumed by other:
         (o) event=none
         (x) systemd=suspends again
         (o) acpid=resumes
         (o) state=close
As a conclusion:
1. With systemd changed, lid_init_state=ignore has only one problem and the
   problem comes from an underlying issue, not userspace and kernel lid
   handling.
2. Without systemd changed, lid_init_state=open can be the default
   behavior as the pass ratio is not much worse than lid_init_state=ignore.
3. lid_init_state=method is buggy, we can have a separate patch to make it
   deprectated.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=187271
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/button.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index e19f530..6d5a8c1 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -113,7 +113,7 @@ struct acpi_button {
 
 static BLOCKING_NOTIFIER_HEAD(acpi_lid_notifier);
 static struct acpi_device *lid_device;
-static u8 lid_init_state = ACPI_BUTTON_LID_INIT_METHOD;
+static u8 lid_init_state = ACPI_BUTTON_LID_INIT_OPEN;
 
 static unsigned long lid_report_interval __read_mostly = 500;
 module_param(lid_report_interval, ulong, 0644);
-- 
2.7.4

[PATCH 2/2] ACPI / button: Remove lid_init_state=method mode

[Lv Zheng]

The mode is buggy, and lid_init__state=open is more useful than this
mode, so this patch makes it deprecated.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 Documentation/acpi/acpi-lid.txt | 16 ++++------------
 drivers/acpi/button.c           |  9 ---------
 2 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/Documentation/acpi/acpi-lid.txt b/Documentation/acpi/acpi-lid.txt
index effe7af..22cb309 100644
--- a/Documentation/acpi/acpi-lid.txt
+++ b/Documentation/acpi/acpi-lid.txt
@@ -59,28 +59,20 @@ button driver uses the following 3 modes in order not to trigger issues.
 If the userspace hasn't been prepared to ignore the unreliable "opened"
 events and the unreliable initial state notification, Linux users can use
 the following kernel parameters to handle the possible issues:
-A. button.lid_init_state=method:
-   When this option is specified, the ACPI button driver reports the
-   initial lid state using the returning value of the _LID control method
-   and whether the "opened"/"closed" events are paired fully relies on the
-   firmware implementation.
-   This option can be used to fix some platforms where the returning value
-   of the _LID control method is reliable but the initial lid state
-   notification is missing.
-   This option is the default behavior during the period the userspace
-   isn't ready to handle the buggy AML tables.
-B. button.lid_init_state=open:
+A. button.lid_init_state=open:
    When this option is specified, the ACPI button driver always reports the
    initial lid state as "opened" and whether the "opened"/"closed" events
    are paired fully relies on the firmware implementation.
    This may fix some platforms where the returning value of the _LID
    control method is not reliable and the initial lid state notification is
    missing.
+   This option is the default behavior during the period the userspace
+   isn't ready to handle the buggy AML tables.
 
 If the userspace has been prepared to ignore the unreliable "opened" events
 and the unreliable initial state notification, Linux users should always
 use the following kernel parameter:
-C. button.lid_init_state=ignore:
+B. button.lid_init_state=ignore:
    When this option is specified, the ACPI button driver never reports the
    initial lid state and there is a compensation mechanism implemented to
    ensure that the reliable "closed" notifications can always be delievered
diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index 6d5a8c1..668137e 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -57,7 +57,6 @@
 
 #define ACPI_BUTTON_LID_INIT_IGNORE	0x00
 #define ACPI_BUTTON_LID_INIT_OPEN	0x01
-#define ACPI_BUTTON_LID_INIT_METHOD	0x02
 
 #define _COMPONENT		ACPI_BUTTON_COMPONENT
 ACPI_MODULE_NAME("button");
@@ -377,9 +376,6 @@ static void acpi_lid_initialize_state(struct acpi_device *device)
 	case ACPI_BUTTON_LID_INIT_OPEN:
 		(void)acpi_lid_notify_state(device, 1);
 		break;
-	case ACPI_BUTTON_LID_INIT_METHOD:
-		(void)acpi_lid_update_state(device);
-		break;
 	case ACPI_BUTTON_LID_INIT_IGNORE:
 	default:
 		break;
@@ -563,9 +559,6 @@ static int param_set_lid_init_state(const char *val, struct kernel_param *kp)
 	if (!strncmp(val, "open", sizeof("open") - 1)) {
 		lid_init_state = ACPI_BUTTON_LID_INIT_OPEN;
 		pr_info("Notify initial lid state as open\n");
-	} else if (!strncmp(val, "method", sizeof("method") - 1)) {
-		lid_init_state = ACPI_BUTTON_LID_INIT_METHOD;
-		pr_info("Notify initial lid state with _LID return value\n");
 	} else if (!strncmp(val, "ignore", sizeof("ignore") - 1)) {
 		lid_init_state = ACPI_BUTTON_LID_INIT_IGNORE;
 		pr_info("Do not notify initial lid state\n");
@@ -579,8 +572,6 @@ static int param_get_lid_init_state(char *buffer, struct kernel_param *kp)
 	switch (lid_init_state) {
 	case ACPI_BUTTON_LID_INIT_OPEN:
 		return sprintf(buffer, "open");
-	case ACPI_BUTTON_LID_INIT_METHOD:
-		return sprintf(buffer, "method");
 	case ACPI_BUTTON_LID_INIT_IGNORE:
 		return sprintf(buffer, "ignore");
 	default:
-- 
2.7.4