[PATCH v1 00/15] ACPI: button: Cleanups and conversion to using devm

[PATCH v1 00/15] ACPI: button: Cleanups and conversion to using devm

[Rafael J. Wysocki]

Hi All,

This series makes a number of changes, that are minor fixes and cleanups, to
the ACPI button driver and concludes in switching it over to devres-based
resource management.

Please see individual patch changelogs for details.

Thanks!

[PATCH v1 01/15] ACPI: button: Fix lid_device value leak past driver removal

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Static variable lid_device is set when the ACPI button driver probes
the last lid device (under the assumptions that there will be only
one lid device in the system) and never cleared, but in principle it
should be reset when the driver unbinds from the lid device pointed
to by it.

Address that and add locking that is needed to clear and set that
variable safely.

Fixes: 7e12715ecc47 ("ACPI button: provide lid status functions")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/button.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index d80276368b81..5df470eea754 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -182,7 +182,6 @@ struct acpi_button {
 	bool gpe_enabled;
 };
 
-static struct acpi_device *lid_device;
 static long lid_init_state = -1;
 
 static unsigned long lid_report_interval __read_mostly = 500;
@@ -378,9 +377,29 @@ static int acpi_button_remove_fs(struct acpi_button *button)
 	return 0;
 }
 
+static struct acpi_device *lid_device;
+static DEFINE_MUTEX(acpi_lid_lock);
+
+static void acpi_lid_save(struct acpi_device *adev)
+{
+	guard(mutex)(&acpi_lid_lock);
+
+	lid_device = adev;
+}
+
+static void acpi_lid_forget(struct acpi_device *adev)
+{
+	guard(mutex)(&acpi_lid_lock);
+
+	if (lid_device == adev)
+		lid_device = NULL;
+}
+
 /* Driver Interface */
 int acpi_lid_open(void)
 {
+	guard(mutex)(&acpi_lid_lock);
+
 	if (!lid_device)
 		return -ENODEV;
 
@@ -674,7 +693,7 @@ static int acpi_button_probe(struct platform_device *pdev)
 		 * This assumes there's only one lid device, or if there are
 		 * more we only care about the last one...
 		 */
-		lid_device = device;
+		acpi_lid_save(device);
 	}
 
 	pr_info("%s [%s]\n", name, acpi_device_bid(device));
@@ -696,6 +715,9 @@ static void acpi_button_remove(struct platform_device *pdev)
 	struct acpi_button *button = platform_get_drvdata(pdev);
 	struct acpi_device *adev = button->adev;
 
+	if (button->type == ACPI_BUTTON_TYPE_LID)
+		acpi_lid_forget(adev);
+
 	switch (adev->device_type) {
 	case ACPI_BUS_TYPE_POWER_BUTTON:
 		acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON,
-- 
2.51.0

[PATCH v1 02/15] ACPI: button: Pass ACPI handle to acpi_lid_evaluate_state()

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Make it clear that acpi_lid_evaluate_state() only uses the ACPI handle
of the lid by changing its argument to acpi_handle and adjust its
callers accordingly.

Also save the ACPI handle of the lid, that later may be passed to
acpi_lid_evaluate_state(), in a static variable instead of saving a
pointer to the ACPI device object containing that handle.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/button.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index 5df470eea754..0b96db762bea 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -192,12 +192,12 @@ MODULE_PARM_DESC(lid_report_interval, "Interval (ms) between lid key events");
 static struct proc_dir_entry *acpi_button_dir;
 static struct proc_dir_entry *acpi_lid_dir;
 
-static int acpi_lid_evaluate_state(struct acpi_device *device)
+static int acpi_lid_evaluate_state(acpi_handle lid_handle)
 {
 	unsigned long long lid_state;
 	acpi_status status;
 
-	status = acpi_evaluate_integer(device->handle, "_LID", NULL, &lid_state);
+	status = acpi_evaluate_integer(lid_handle, "_LID", NULL, &lid_state);
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
@@ -292,7 +292,7 @@ static int __maybe_unused acpi_button_state_seq_show(struct seq_file *seq,
 	struct acpi_button *button = seq->private;
 	int state;
 
-	state = acpi_lid_evaluate_state(button->adev);
+	state = acpi_lid_evaluate_state(button->adev->handle);
 	seq_printf(seq, "state:      %s\n",
 		   state < 0 ? "unsupported" : (state ? "open" : "closed"));
 	return 0;
@@ -377,22 +377,22 @@ static int acpi_button_remove_fs(struct acpi_button *button)
 	return 0;
 }
 
-static struct acpi_device *lid_device;
+static acpi_handle saved_lid_handle;
 static DEFINE_MUTEX(acpi_lid_lock);
 
 static void acpi_lid_save(struct acpi_device *adev)
 {
 	guard(mutex)(&acpi_lid_lock);
 
-	lid_device = adev;
+	saved_lid_handle = adev->handle;
 }
 
 static void acpi_lid_forget(struct acpi_device *adev)
 {
 	guard(mutex)(&acpi_lid_lock);
 
-	if (lid_device == adev)
-		lid_device = NULL;
+	if (saved_lid_handle == adev->handle)
+		saved_lid_handle = NULL;
 }
 
 /* Driver Interface */
@@ -400,20 +400,19 @@ int acpi_lid_open(void)
 {
 	guard(mutex)(&acpi_lid_lock);
 
-	if (!lid_device)
+	if (!saved_lid_handle)
 		return -ENODEV;
 
-	return acpi_lid_evaluate_state(lid_device);
+	return acpi_lid_evaluate_state(saved_lid_handle);
 }
 EXPORT_SYMBOL(acpi_lid_open);
 
 static int acpi_lid_update_state(struct acpi_button *button,
 				 bool signal_wakeup)
 {
-	struct acpi_device *device = button->adev;
 	int state;
 
-	state = acpi_lid_evaluate_state(device);
+	state = acpi_lid_evaluate_state(button->adev->handle);
 	if (state < 0)
 		return state;
 
@@ -516,12 +515,11 @@ static int acpi_button_suspend(struct device *dev)
 static int acpi_button_resume(struct device *dev)
 {
 	struct acpi_button *button = dev_get_drvdata(dev);
-	struct acpi_device *device = ACPI_COMPANION(dev);
 	struct input_dev *input;
 
 	button->suspended = false;
 	if (button->type == ACPI_BUTTON_TYPE_LID) {
-		button->last_state = !!acpi_lid_evaluate_state(device);
+		button->last_state = !!acpi_lid_evaluate_state(ACPI_HANDLE(dev));
 		button->last_time = ktime_get();
 		acpi_lid_initialize_state(button);
 	}
@@ -540,9 +538,8 @@ static int acpi_button_resume(struct device *dev)
 static int acpi_lid_input_open(struct input_dev *input)
 {
 	struct acpi_button *button = input_get_drvdata(input);
-	struct acpi_device *device = button->adev;
 
-	button->last_state = !!acpi_lid_evaluate_state(device);
+	button->last_state = !!acpi_lid_evaluate_state(button->adev->handle);
 	button->last_time = ktime_get();
 	acpi_lid_initialize_state(button);
 
-- 
2.51.0

[PATCH v1 03/15] ACPI: button: Improve warning message regarding lid state

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

The warning message regarding an unexpected lid state printed by
acpi_lid_notify_state() is quite cryptic and there is no information
in it to indicate that it is about a platform firmware defect.  In
fact, it can only be understood after reading the comment below the
statement printing it.

For this reason, replace it with a more direct one including FW_BUG so
its connection to a firmware issue is clearer.

While at it, fix up a comment preceding the statement printing the
message in question.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/button.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index 0b96db762bea..d2c2b8105639 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -227,8 +227,8 @@ static int acpi_lid_notify_state(struct acpi_button *button, int state)
 				ms_to_ktime(lid_report_interval));
 	if (button->last_state == !!state &&
 	    ktime_after(ktime_get(), next_report)) {
-		/* Complain the buggy firmware */
-		pr_warn_once("The lid device is not compliant to SW_LID.\n");
+		/* Complain about the buggy firmware. */
+		pr_warn_once(FW_BUG "Unexpected lid state reported by firmware\n");
 
 		/*
 		 * Send the unreliable complement switch event:
-- 
2.51.0

[PATCH v1 04/15] ACPI: button: Use bool for representing boolean values

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Change the data type of the last_state field in struct acpi_button and
the data type of the acpi_lid_notify_state() second argument to bool
because they both are used for storing boolean values.

Update the callers of acpi_lid_notify_state() accordingly and
while at it, remove the unnecessary (void) cast from the
acpi_lid_update_state() call in acpi_lid_initialize_state() for
consistency.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/button.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index d2c2b8105639..21a10da8b60b 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -175,7 +175,7 @@ struct acpi_button {
 	struct input_dev *input;
 	char phys[32];			/* for input device */
 	unsigned long pushed;
-	int last_state;
+	bool last_state;
 	ktime_t last_time;
 	bool suspended;
 	bool lid_state_initialized;
@@ -204,7 +204,7 @@ static int acpi_lid_evaluate_state(acpi_handle lid_handle)
 	return lid_state ? 1 : 0;
 }
 
-static int acpi_lid_notify_state(struct acpi_button *button, int state)
+static int acpi_lid_notify_state(struct acpi_button *button, bool state)
 {
 	struct acpi_device *device = button->adev;
 	ktime_t next_report;
@@ -218,14 +218,14 @@ static int acpi_lid_notify_state(struct acpi_button *button, int state)
 	 * switch.
 	 */
 	if (lid_init_state != ACPI_BUTTON_LID_INIT_IGNORE ||
-	    button->last_state != !!state)
+	    button->last_state != state)
 		do_update = true;
 	else
 		do_update = false;
 
 	next_report = ktime_add(button->last_time,
 				ms_to_ktime(lid_report_interval));
-	if (button->last_state == !!state &&
+	if (button->last_state == state &&
 	    ktime_after(ktime_get(), next_report)) {
 		/* Complain about the buggy firmware. */
 		pr_warn_once(FW_BUG "Unexpected lid state reported by firmware\n");
@@ -279,7 +279,7 @@ static int acpi_lid_notify_state(struct acpi_button *button, int state)
 				  state ? "open" : "closed");
 		input_report_switch(button->input, SW_LID, !state);
 		input_sync(button->input);
-		button->last_state = !!state;
+		button->last_state = state;
 		button->last_time = ktime_get();
 	}
 
@@ -426,10 +426,10 @@ static void acpi_lid_initialize_state(struct acpi_button *button)
 {
 	switch (lid_init_state) {
 	case ACPI_BUTTON_LID_INIT_OPEN:
-		(void)acpi_lid_notify_state(button, 1);
+		acpi_lid_notify_state(button, true);
 		break;
 	case ACPI_BUTTON_LID_INIT_METHOD:
-		(void)acpi_lid_update_state(button, false);
+		acpi_lid_update_state(button, false);
 		break;
 	case ACPI_BUTTON_LID_INIT_IGNORE:
 	default:
-- 
2.51.0

[PATCH v1 05/15] ACPI: button: Eliminate ternary operator from acpi_lid_evaluate_state()

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

The ternary operator in acpi_lid_evaluate_state() is not actually needed
because the same result can be achieved by applying the !! operator to
the lid_state value, so update the code accordingly.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@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 21a10da8b60b..ae97c83bae2c 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -201,7 +201,7 @@ static int acpi_lid_evaluate_state(acpi_handle lid_handle)
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
-	return lid_state ? 1 : 0;
+	return !!lid_state;
 }
 
 static int acpi_lid_notify_state(struct acpi_button *button, bool state)
-- 
2.51.0

[PATCH v1 06/15] ACPI: button: Change return type of two functions to void

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

The return value of acpi_lid_notify_state() is always 0, so change
its return type to void.

Moreover, the return value of the only caller of that function,
acpi_lid_update_state(), is never used, so change its return type
to void either.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/button.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index ae97c83bae2c..fcb2eed823c1 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -204,7 +204,7 @@ static int acpi_lid_evaluate_state(acpi_handle lid_handle)
 	return !!lid_state;
 }
 
-static int acpi_lid_notify_state(struct acpi_button *button, bool state)
+static void acpi_lid_notify_state(struct acpi_button *button, bool state)
 {
 	struct acpi_device *device = button->adev;
 	ktime_t next_report;
@@ -282,8 +282,6 @@ static int acpi_lid_notify_state(struct acpi_button *button, bool state)
 		button->last_state = state;
 		button->last_time = ktime_get();
 	}
-
-	return 0;
 }
 
 static int __maybe_unused acpi_button_state_seq_show(struct seq_file *seq,
@@ -407,19 +405,18 @@ int acpi_lid_open(void)
 }
 EXPORT_SYMBOL(acpi_lid_open);
 
-static int acpi_lid_update_state(struct acpi_button *button,
-				 bool signal_wakeup)
+static void acpi_lid_update_state(struct acpi_button *button, bool signal_wakeup)
 {
 	int state;
 
 	state = acpi_lid_evaluate_state(button->adev->handle);
 	if (state < 0)
-		return state;
+		return;
 
 	if (state && signal_wakeup)
 		acpi_pm_wakeup_event(button->dev);
 
-	return acpi_lid_notify_state(button, state);
+	acpi_lid_notify_state(button, state);
 }
 
 static void acpi_lid_initialize_state(struct acpi_button *button)
-- 
2.51.0

[PATCH v1 07/15] ACPI: button: Eliminate redundant conditional statement

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Simplify do_update initialization in acpi_lid_notify_state() by
assigning the value of the condition it depends on directly to it.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/button.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index fcb2eed823c1..236eb025bb0d 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -217,12 +217,8 @@ static void acpi_lid_notify_state(struct acpi_button *button, bool state)
 	 * So "last_time" is only updated after a timeout or an actual
 	 * switch.
 	 */
-	if (lid_init_state != ACPI_BUTTON_LID_INIT_IGNORE ||
-	    button->last_state != state)
-		do_update = true;
-	else
-		do_update = false;
-
+	do_update = lid_init_state != ACPI_BUTTON_LID_INIT_IGNORE ||
+			button->last_state != state;
 	next_report = ktime_add(button->last_time,
 				ms_to_ktime(lid_report_interval));
 	if (button->last_state == state &&
-- 
2.51.0

[PATCH v1 08/15] ACPI: button: Use local pointer to platform device dev field in probe

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

To avoid dereferencing pdev to get to the target platform device's
dev field in multiple places in acpi_button_probe(), use a local pointer
to that field.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/button.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index 236eb025bb0d..9fe8d212b606 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -541,6 +541,7 @@ static int acpi_lid_input_open(struct input_dev *input)
 
 static int acpi_button_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	acpi_notify_handler handler;
 	struct acpi_device *device;
 	struct acpi_button *button;
@@ -550,7 +551,7 @@ static int acpi_button_probe(struct platform_device *pdev)
 	const char *hid;
 	int error = 0;
 
-	device = ACPI_COMPANION(&pdev->dev);
+	device = ACPI_COMPANION(dev);
 	if (!device)
 		return -ENODEV;
 
@@ -565,7 +566,7 @@ static int acpi_button_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, button);
 
-	button->dev = &pdev->dev;
+	button->dev = dev;
 	button->adev = device;
 	button->input = input = input_allocate_device();
 	if (!input) {
@@ -615,7 +616,7 @@ static int acpi_button_probe(struct platform_device *pdev)
 	input->phys = button->phys;
 	input->id.bustype = BUS_HOST;
 	input->id.product = button->type;
-	input->dev.parent = &pdev->dev;
+	input->dev.parent = dev;
 
 	switch (button->type) {
 	case ACPI_BUTTON_TYPE_POWER:
-- 
2.51.0

[PATCH v1 09/15] ACPI: button: Rework device verification during probe

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Instead of manually comparing the primary ID of the device (retuned
by _HID) with each of the device IDs supported by the driver, use
acpi_match_acpi_device() (which includes the ACPI companion device
pointer check against NULL) and store the ACPI button type as
driver_data in button_device_ids[], which allows a multi-branch
conditional statement to be replaced with a switch () one.  However,
to continue preventing successful probing of devices that only have
one of the supported device IDs in their _CID lists, compare the
matched device ID with the primary ID of the device and return an
error if they don't match.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/button.c | 77 ++++++++++++++++++++++---------------------
 1 file changed, 40 insertions(+), 37 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index 9fe8d212b606..dbaf87417428 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -59,11 +59,11 @@ MODULE_DESCRIPTION("ACPI Button Driver");
 MODULE_LICENSE("GPL");
 
 static const struct acpi_device_id button_device_ids[] = {
-	{ACPI_BUTTON_HID_LID,    0},
-	{ACPI_BUTTON_HID_SLEEP,  0},
-	{ACPI_BUTTON_HID_SLEEPF, 0},
-	{ACPI_BUTTON_HID_POWER,  0},
-	{ACPI_BUTTON_HID_POWERF, 0},
+	{ACPI_BUTTON_HID_LID, ACPI_BUTTON_TYPE_LID},
+	{ACPI_BUTTON_HID_SLEEP, ACPI_BUTTON_TYPE_SLEEP},
+	{ACPI_BUTTON_HID_SLEEPF, ACPI_BUTTON_TYPE_SLEEP},
+	{ACPI_BUTTON_HID_POWER, ACPI_BUTTON_TYPE_POWER},
+	{ACPI_BUTTON_HID_POWERF, ACPI_BUTTON_TYPE_POWER},
 	{"", 0},
 };
 MODULE_DEVICE_TABLE(acpi, button_device_ids);
@@ -542,22 +542,23 @@ static int acpi_lid_input_open(struct input_dev *input)
 static int acpi_button_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct acpi_device *device = ACPI_COMPANION(dev);
+	const struct acpi_device_id *id;
 	acpi_notify_handler handler;
-	struct acpi_device *device;
 	struct acpi_button *button;
 	struct input_dev *input;
 	acpi_status status;
 	char *name, *class;
-	const char *hid;
+	u8 button_type;
 	int error = 0;
 
-	device = ACPI_COMPANION(dev);
-	if (!device)
-		return -ENODEV;
+	id = acpi_match_acpi_device(button_device_ids, device);
+	if (!id || strcmp(acpi_device_hid(device), id->id))
+		return dev_err_probe(dev, -ENODEV, "Unsupported device\n");
 
-	hid = acpi_device_hid(device);
-	if (!strcmp(hid, ACPI_BUTTON_HID_LID) &&
-	     lid_init_state == ACPI_BUTTON_LID_INIT_DISABLED)
+	button_type = id->driver_data;
+	if (button_type == ACPI_BUTTON_TYPE_LID &&
+	    lid_init_state == ACPI_BUTTON_LID_INIT_DISABLED)
 		return -ENODEV;
 
 	button = kzalloc_obj(struct acpi_button);
@@ -568,57 +569,59 @@ static int acpi_button_probe(struct platform_device *pdev)
 
 	button->dev = dev;
 	button->adev = device;
-	button->input = input = input_allocate_device();
+	input = input_allocate_device();
 	if (!input) {
 		error = -ENOMEM;
 		goto err_free_button;
 	}
+	button->input = input;
+	button->type = button_type;
 
 	class = acpi_device_class(device);
 
-	if (!strcmp(hid, ACPI_BUTTON_HID_POWER) ||
-	    !strcmp(hid, ACPI_BUTTON_HID_POWERF)) {
-		button->type = ACPI_BUTTON_TYPE_POWER;
+	switch (button_type) {
+	case ACPI_BUTTON_TYPE_LID:
+		handler = acpi_lid_notify;
+		name = ACPI_BUTTON_DEVICE_NAME_LID;
+		sprintf(class, "%s/%s",
+			ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_LID);
+		input->open = acpi_lid_input_open;
+		break;
+
+	case ACPI_BUTTON_TYPE_POWER:
 		handler = acpi_button_notify;
 		name = ACPI_BUTTON_DEVICE_NAME_POWER;
 		sprintf(class, "%s/%s",
 			ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_POWER);
-	} else if (!strcmp(hid, ACPI_BUTTON_HID_SLEEP) ||
-		   !strcmp(hid, ACPI_BUTTON_HID_SLEEPF)) {
-		button->type = ACPI_BUTTON_TYPE_SLEEP;
+		break;
+
+	case ACPI_BUTTON_TYPE_SLEEP:
 		handler = acpi_button_notify;
 		name = ACPI_BUTTON_DEVICE_NAME_SLEEP;
 		sprintf(class, "%s/%s",
 			ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_SLEEP);
-	} else if (!strcmp(hid, ACPI_BUTTON_HID_LID)) {
-		button->type = ACPI_BUTTON_TYPE_LID;
-		handler = acpi_lid_notify;
-		name = ACPI_BUTTON_DEVICE_NAME_LID;
-		sprintf(class, "%s/%s",
-			ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_LID);
-		input->open = acpi_lid_input_open;
-	} else {
-		pr_info("Unsupported hid [%s]\n", hid);
-		error = -ENODEV;
-	}
+		break;
 
-	if (!error)
-		error = acpi_button_add_fs(button);
+	default:
+		input_free_device(input);
+		return dev_err_probe(dev, -ENODEV, "Unrecognized button type\n");
+	}
 
+	error = acpi_button_add_fs(button);
 	if (error) {
 		input_free_device(input);
 		goto err_free_button;
 	}
 
-	snprintf(button->phys, sizeof(button->phys), "%s/button/input0", hid);
+	snprintf(button->phys, sizeof(button->phys), "%s/button/input0", id->id);
 
 	input->name = name;
 	input->phys = button->phys;
 	input->id.bustype = BUS_HOST;
-	input->id.product = button->type;
+	input->id.product = button_type;
 	input->dev.parent = dev;
 
-	switch (button->type) {
+	switch (button_type) {
 	case ACPI_BUTTON_TYPE_POWER:
 		input_set_capability(input, EV_KEY, KEY_POWER);
 		input_set_capability(input, EV_KEY, KEY_WAKEUP);
@@ -679,7 +682,7 @@ static int acpi_button_probe(struct platform_device *pdev)
 		goto err_input_unregister;
 	}
 
-	if (button->type == ACPI_BUTTON_TYPE_LID) {
+	if (button_type == ACPI_BUTTON_TYPE_LID) {
 		/*
 		 * This assumes there's only one lid device, or if there are
 		 * more we only care about the last one...
-- 
2.51.0

[PATCH v1 10/15] ACPI: button: Drop redundant variable from acpi_button_probe()

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Local char pointer called "name" in acpi_button_probe() is redundant
because its value can be assigned directly to input->name and the
latter can be used in the only other place where "name" is read, so
get rid of it.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/button.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index dbaf87417428..f0d29870b926 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -548,7 +548,7 @@ static int acpi_button_probe(struct platform_device *pdev)
 	struct acpi_button *button;
 	struct input_dev *input;
 	acpi_status status;
-	char *name, *class;
+	char *class;
 	u8 button_type;
 	int error = 0;
 
@@ -581,23 +581,23 @@ static int acpi_button_probe(struct platform_device *pdev)
 
 	switch (button_type) {
 	case ACPI_BUTTON_TYPE_LID:
+		input->name = ACPI_BUTTON_DEVICE_NAME_LID;
 		handler = acpi_lid_notify;
-		name = ACPI_BUTTON_DEVICE_NAME_LID;
 		sprintf(class, "%s/%s",
 			ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_LID);
 		input->open = acpi_lid_input_open;
 		break;
 
 	case ACPI_BUTTON_TYPE_POWER:
+		input->name = ACPI_BUTTON_DEVICE_NAME_POWER;
 		handler = acpi_button_notify;
-		name = ACPI_BUTTON_DEVICE_NAME_POWER;
 		sprintf(class, "%s/%s",
 			ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_POWER);
 		break;
 
 	case ACPI_BUTTON_TYPE_SLEEP:
+		input->name = ACPI_BUTTON_DEVICE_NAME_SLEEP;
 		handler = acpi_button_notify;
-		name = ACPI_BUTTON_DEVICE_NAME_SLEEP;
 		sprintf(class, "%s/%s",
 			ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_SLEEP);
 		break;
@@ -615,7 +615,6 @@ static int acpi_button_probe(struct platform_device *pdev)
 
 	snprintf(button->phys, sizeof(button->phys), "%s/button/input0", id->id);
 
-	input->name = name;
 	input->phys = button->phys;
 	input->id.bustype = BUS_HOST;
 	input->id.product = button_type;
@@ -690,7 +689,7 @@ static int acpi_button_probe(struct platform_device *pdev)
 		acpi_lid_save(device);
 	}
 
-	pr_info("%s [%s]\n", name, acpi_device_bid(device));
+	pr_info("%s [%s]\n", input->name, acpi_device_bid(device));
 	return 0;
 
 err_input_unregister:
-- 
2.51.0

[PATCH v1 11/15] ACPI: button: Merge two switch () statements in acpi_button_probe()

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Two switch () statements in acpi_button_probe() operate on the same
value and the statements between them can be reordered with respect
to the second one, so merge them.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/button.c | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index f0d29870b926..c035741e5abf 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -582,14 +582,19 @@ static int acpi_button_probe(struct platform_device *pdev)
 	switch (button_type) {
 	case ACPI_BUTTON_TYPE_LID:
 		input->name = ACPI_BUTTON_DEVICE_NAME_LID;
+		input_set_capability(input, EV_SW, SW_LID);
+		input->open = acpi_lid_input_open;
+
 		handler = acpi_lid_notify;
 		sprintf(class, "%s/%s",
 			ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_LID);
-		input->open = acpi_lid_input_open;
 		break;
 
 	case ACPI_BUTTON_TYPE_POWER:
 		input->name = ACPI_BUTTON_DEVICE_NAME_POWER;
+		input_set_capability(input, EV_KEY, KEY_POWER);
+		input_set_capability(input, EV_KEY, KEY_WAKEUP);
+
 		handler = acpi_button_notify;
 		sprintf(class, "%s/%s",
 			ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_POWER);
@@ -597,6 +602,8 @@ static int acpi_button_probe(struct platform_device *pdev)
 
 	case ACPI_BUTTON_TYPE_SLEEP:
 		input->name = ACPI_BUTTON_DEVICE_NAME_SLEEP;
+		input_set_capability(input, EV_KEY, KEY_SLEEP);
+
 		handler = acpi_button_notify;
 		sprintf(class, "%s/%s",
 			ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_SLEEP);
@@ -613,28 +620,14 @@ static int acpi_button_probe(struct platform_device *pdev)
 		goto err_free_button;
 	}
 
-	snprintf(button->phys, sizeof(button->phys), "%s/button/input0", id->id);
+	snprintf(button->phys, sizeof(button->phys), "%s/button/input0",
+		 acpi_device_hid(device));
 
 	input->phys = button->phys;
 	input->id.bustype = BUS_HOST;
 	input->id.product = button_type;
 	input->dev.parent = dev;
 
-	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:
-		input_set_capability(input, EV_KEY, KEY_SLEEP);
-		break;
-
-	case ACPI_BUTTON_TYPE_LID:
-		input_set_capability(input, EV_SW, SW_LID);
-		break;
-	}
-
 	input_set_drvdata(input, button);
 	error = input_register_device(input);
 	if (error) {
-- 
2.51.0

[PATCH v1 12/15] ACPI: button: Clean up adding and removing lid procfs interface

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

The procfs interface is only used with lid devices which only becomes
clear after looking into the function bodies of acpi_button_add_fs()
and acpi_button_remove_fs().  Moreover, the only error code returned
by the former of these functions is -ENODEV, so the ret local variable
in it is redundant, and the return type of the latter one can be changed
to void.

Accordingly, rename these functions to acpi_button_add_fs() and
acpi_button_remove_fs(), respectively, move the button->type checks
against ACPI_BUTTON_TYPE_LID from them to their callers, and make
code simplifications as per the above.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/button.c | 54 ++++++++++++++++---------------------------
 1 file changed, 20 insertions(+), 34 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index c035741e5abf..3327ee8132ad 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -292,15 +292,10 @@ static int __maybe_unused acpi_button_state_seq_show(struct seq_file *seq,
 	return 0;
 }
 
-static int acpi_button_add_fs(struct acpi_button *button)
+static int acpi_lid_add_fs(struct acpi_button *button)
 {
 	struct acpi_device *device = button->adev;
 	struct proc_dir_entry *entry = NULL;
-	int ret = 0;
-
-	/* procfs I/F for ACPI lid device only */
-	if (button->type != ACPI_BUTTON_TYPE_LID)
-		return 0;
 
 	if (acpi_button_dir || acpi_lid_dir) {
 		pr_info("More than one Lid device found!\n");
@@ -314,33 +309,25 @@ static int acpi_button_add_fs(struct acpi_button *button)
 
 	/* create /proc/acpi/button/lid */
 	acpi_lid_dir = proc_mkdir(ACPI_BUTTON_SUBCLASS_LID, acpi_button_dir);
-	if (!acpi_lid_dir) {
-		ret = -ENODEV;
+	if (!acpi_lid_dir)
 		goto remove_button_dir;
-	}
 
 	/* create /proc/acpi/button/lid/LID/ */
 	acpi_device_dir(device) = proc_mkdir(acpi_device_bid(device), acpi_lid_dir);
-	if (!acpi_device_dir(device)) {
-		ret = -ENODEV;
+	if (!acpi_device_dir(device))
 		goto remove_lid_dir;
-	}
 
 	/* create /proc/acpi/button/lid/LID/state */
 	entry = proc_create_single_data(ACPI_BUTTON_FILE_STATE, S_IRUGO,
 			acpi_device_dir(device), acpi_button_state_seq_show,
 			button);
-	if (!entry) {
-		ret = -ENODEV;
+	if (!entry)
 		goto remove_dev_dir;
-	}
 
-done:
-	return ret;
+	return 0;
 
 remove_dev_dir:
-	remove_proc_entry(acpi_device_bid(device),
-			  acpi_lid_dir);
+	remove_proc_entry(acpi_device_bid(device), acpi_lid_dir);
 	acpi_device_dir(device) = NULL;
 remove_lid_dir:
 	remove_proc_entry(ACPI_BUTTON_SUBCLASS_LID, acpi_button_dir);
@@ -348,16 +335,13 @@ static int acpi_button_add_fs(struct acpi_button *button)
 remove_button_dir:
 	remove_proc_entry(ACPI_BUTTON_CLASS, acpi_root_dir);
 	acpi_button_dir = NULL;
-	goto done;
+	return -ENODEV;
 }
 
-static int acpi_button_remove_fs(struct acpi_button *button)
+static void acpi_lid_remove_fs(struct acpi_button *button)
 {
 	struct acpi_device *device = button->adev;
 
-	if (button->type != ACPI_BUTTON_TYPE_LID)
-		return 0;
-
 	remove_proc_entry(ACPI_BUTTON_FILE_STATE,
 			  acpi_device_dir(device));
 	remove_proc_entry(acpi_device_bid(device),
@@ -367,8 +351,6 @@ static int acpi_button_remove_fs(struct acpi_button *button)
 	acpi_lid_dir = NULL;
 	remove_proc_entry(ACPI_BUTTON_CLASS, acpi_root_dir);
 	acpi_button_dir = NULL;
-
-	return 0;
 }
 
 static acpi_handle saved_lid_handle;
@@ -588,6 +570,12 @@ static int acpi_button_probe(struct platform_device *pdev)
 		handler = acpi_lid_notify;
 		sprintf(class, "%s/%s",
 			ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_LID);
+
+		error = acpi_lid_add_fs(button);
+		if (error) {
+			input_free_device(input);
+			goto err_free_button;
+		}
 		break;
 
 	case ACPI_BUTTON_TYPE_POWER:
@@ -614,12 +602,6 @@ static int acpi_button_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, -ENODEV, "Unrecognized button type\n");
 	}
 
-	error = acpi_button_add_fs(button);
-	if (error) {
-		input_free_device(input);
-		goto err_free_button;
-	}
-
 	snprintf(button->phys, sizeof(button->phys), "%s/button/input0",
 		 acpi_device_hid(device));
 
@@ -689,7 +671,9 @@ static int acpi_button_probe(struct platform_device *pdev)
 	device_init_wakeup(button->dev, false);
 	input_unregister_device(input);
 err_remove_fs:
-	acpi_button_remove_fs(button);
+	if (button_type == ACPI_BUTTON_TYPE_LID)
+		acpi_lid_remove_fs(button);
+
 err_free_button:
 	kfree(button);
 	memset(acpi_device_class(device), 0, sizeof(acpi_device_class));
@@ -730,8 +714,10 @@ static void acpi_button_remove(struct platform_device *pdev)
 
 	device_init_wakeup(button->dev, false);
 
-	acpi_button_remove_fs(button);
 	input_unregister_device(button->input);
+	if (button->type == ACPI_BUTTON_TYPE_LID)
+		acpi_lid_remove_fs(button);
+
 	kfree(button);
 
 	memset(acpi_device_class(adev), 0, sizeof(acpi_device_class));
-- 
2.51.0

[PATCH v1 13/15] ACPI: button: Use string literals for generating netlink messages

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Instead of storing strings that never change later under
acpi_device_class(device) and using them for generating netlink
messages, use pointers to string literals with the same content.

This also allows the clearing of the acpi_device_class(device)
area during driver removal and in the probe rollback path to be
dropped.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/button.c | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index 3327ee8132ad..122c1dd8a68d 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -28,14 +28,15 @@
 #define ACPI_BUTTON_NOTIFY_WAKE		0x02
 #define ACPI_BUTTON_NOTIFY_STATUS	0x80
 
-#define ACPI_BUTTON_SUBCLASS_POWER	"power"
+#define ACPI_BUTTON_CLASS_POWER		"button/power"
 #define ACPI_BUTTON_DEVICE_NAME_POWER	"Power Button"
 #define ACPI_BUTTON_TYPE_POWER		0x01
 
-#define ACPI_BUTTON_SUBCLASS_SLEEP	"sleep"
+#define ACPI_BUTTON_CLASS_SLEEP		"button/sleep"
 #define ACPI_BUTTON_DEVICE_NAME_SLEEP	"Sleep Button"
 #define ACPI_BUTTON_TYPE_SLEEP		0x03
 
+#define ACPI_BUTTON_CLASS_LID		"button/lid"
 #define ACPI_BUTTON_SUBCLASS_LID	"lid"
 #define ACPI_BUTTON_DEVICE_NAME_LID	"Lid Switch"
 #define ACPI_BUTTON_TYPE_LID		0x05
@@ -173,6 +174,7 @@ struct acpi_button {
 	struct device *dev;		/* physical button device */
 	unsigned int type;
 	struct input_dev *input;
+	const char *class;		/* for netlink messages */
 	char phys[32];			/* for input device */
 	unsigned long pushed;
 	bool last_state;
@@ -462,8 +464,7 @@ static void acpi_button_notify(acpi_handle handle, u32 event, void *data)
 	input_report_key(input, keycode, 0);
 	input_sync(input);
 
-	acpi_bus_generate_netlink_event(acpi_device_class(device),
-					dev_name(&device->dev),
+	acpi_bus_generate_netlink_event(button->class, dev_name(&device->dev),
 					event, ++button->pushed);
 }
 
@@ -530,7 +531,6 @@ static int acpi_button_probe(struct platform_device *pdev)
 	struct acpi_button *button;
 	struct input_dev *input;
 	acpi_status status;
-	char *class;
 	u8 button_type;
 	int error = 0;
 
@@ -559,17 +559,15 @@ static int acpi_button_probe(struct platform_device *pdev)
 	button->input = input;
 	button->type = button_type;
 
-	class = acpi_device_class(device);
-
 	switch (button_type) {
 	case ACPI_BUTTON_TYPE_LID:
+		button->class = ACPI_BUTTON_CLASS_LID;
+
 		input->name = ACPI_BUTTON_DEVICE_NAME_LID;
 		input_set_capability(input, EV_SW, SW_LID);
 		input->open = acpi_lid_input_open;
 
 		handler = acpi_lid_notify;
-		sprintf(class, "%s/%s",
-			ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_LID);
 
 		error = acpi_lid_add_fs(button);
 		if (error) {
@@ -579,22 +577,22 @@ static int acpi_button_probe(struct platform_device *pdev)
 		break;
 
 	case ACPI_BUTTON_TYPE_POWER:
+		button->class = ACPI_BUTTON_CLASS_POWER;
+
 		input->name = ACPI_BUTTON_DEVICE_NAME_POWER;
 		input_set_capability(input, EV_KEY, KEY_POWER);
 		input_set_capability(input, EV_KEY, KEY_WAKEUP);
 
 		handler = acpi_button_notify;
-		sprintf(class, "%s/%s",
-			ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_POWER);
 		break;
 
 	case ACPI_BUTTON_TYPE_SLEEP:
+		button->class = ACPI_BUTTON_CLASS_SLEEP;
+
 		input->name = ACPI_BUTTON_DEVICE_NAME_SLEEP;
 		input_set_capability(input, EV_KEY, KEY_SLEEP);
 
 		handler = acpi_button_notify;
-		sprintf(class, "%s/%s",
-			ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_SLEEP);
 		break;
 
 	default:
@@ -676,7 +674,6 @@ static int acpi_button_probe(struct platform_device *pdev)
 
 err_free_button:
 	kfree(button);
-	memset(acpi_device_class(device), 0, sizeof(acpi_device_class));
 	return error;
 }
 
@@ -719,8 +716,6 @@ static void acpi_button_remove(struct platform_device *pdev)
 		acpi_lid_remove_fs(button);
 
 	kfree(button);
-
-	memset(acpi_device_class(adev), 0, sizeof(acpi_device_class));
 }
 
 static int param_set_lid_init_state(const char *val,
-- 
2.51.0

[PATCH v1 14/15] ACPI: button: Reorganize installing and removing event handlers

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

To facilitate subsequent changes, move the code installing and
removing button event handlers into two separate functions called
acpi_button_add_event_handler() and acpi_button_remove_event_handler(),
respectively, and rearrange it to reduce code duplication.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/button.c | 155 ++++++++++++++++++++++++------------------
 1 file changed, 89 insertions(+), 66 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index 122c1dd8a68d..8a56780e13da 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -522,15 +522,99 @@ static int acpi_lid_input_open(struct input_dev *input)
 	return 0;
 }
 
+static acpi_notify_handler acpi_button_notify_handler(struct acpi_button *button)
+{
+	if (button->type == ACPI_BUTTON_TYPE_LID)
+		return acpi_lid_notify;
+
+	return acpi_button_notify;
+}
+
+static void acpi_button_remove_event_handler(struct acpi_button *button)
+{
+	struct acpi_device *adev = button->adev;
+
+	switch (adev->device_type) {
+	case ACPI_BUS_TYPE_POWER_BUTTON:
+		acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON,
+						acpi_button_event);
+		break;
+
+	case ACPI_BUS_TYPE_SLEEP_BUTTON:
+		acpi_remove_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON,
+						acpi_button_event);
+		break;
+
+	default:
+		if (button->gpe_enabled) {
+			dev_dbg(button->dev, "Disabling ACPI GPE%02llx\n",
+				adev->wakeup.gpe_number);
+			acpi_disable_gpe(adev->wakeup.gpe_device,
+					 adev->wakeup.gpe_number);
+		}
+		acpi_remove_notify_handler(adev->handle, ACPI_ALL_NOTIFY,
+					   acpi_button_notify_handler(button));
+		break;
+	}
+	acpi_os_wait_events_complete();
+}
+
+static int acpi_button_add_fixed_event_handler(u32 event,
+					       struct acpi_button *button)
+{
+	acpi_status status;
+
+	status = acpi_install_fixed_event_handler(event, acpi_button_event, button);
+	if (ACPI_FAILURE(status))
+		return -ENODEV;
+
+	return 0;
+}
+
+static int acpi_button_add_event_handler(struct acpi_button *button)
+{
+	struct acpi_device *adev = button->adev;
+	acpi_status status;
+
+	if (adev->device_type == ACPI_BUS_TYPE_POWER_BUTTON)
+		return acpi_button_add_fixed_event_handler(ACPI_EVENT_POWER_BUTTON,
+							   button);
+
+	if (adev->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON)
+		return acpi_button_add_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON,
+							   button);
+
+	status = acpi_install_notify_handler(adev->handle, ACPI_ALL_NOTIFY,
+					     acpi_button_notify_handler(button),
+					     button);
+	if (ACPI_FAILURE(status))
+		return -ENODEV;
+
+	if (!adev->wakeup.flags.valid)
+		return 0;
+
+	/*
+	 * If the wakeup GPE has a handler method, enable it in case it is also
+	 * used for signaling runtime events.
+	 */
+	status = acpi_enable_gpe_cond(adev->wakeup.gpe_device,
+				      adev->wakeup.gpe_number,
+				      ACPI_GPE_DISPATCH_METHOD);
+	button->gpe_enabled = ACPI_SUCCESS(status);
+	if (button->gpe_enabled)
+		dev_dbg(button->dev, "Enabled ACPI GPE%02llx\n",
+			adev->wakeup.gpe_number);
+
+	return 0;
+}
+
 static int acpi_button_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct acpi_device *device = ACPI_COMPANION(dev);
 	const struct acpi_device_id *id;
-	acpi_notify_handler handler;
 	struct acpi_button *button;
 	struct input_dev *input;
-	acpi_status status;
 	u8 button_type;
 	int error = 0;
 
@@ -567,8 +649,6 @@ static int acpi_button_probe(struct platform_device *pdev)
 		input_set_capability(input, EV_SW, SW_LID);
 		input->open = acpi_lid_input_open;
 
-		handler = acpi_lid_notify;
-
 		error = acpi_lid_add_fs(button);
 		if (error) {
 			input_free_device(input);
@@ -582,8 +662,6 @@ static int acpi_button_probe(struct platform_device *pdev)
 		input->name = ACPI_BUTTON_DEVICE_NAME_POWER;
 		input_set_capability(input, EV_KEY, KEY_POWER);
 		input_set_capability(input, EV_KEY, KEY_WAKEUP);
-
-		handler = acpi_button_notify;
 		break;
 
 	case ACPI_BUTTON_TYPE_SLEEP:
@@ -591,8 +669,6 @@ static int acpi_button_probe(struct platform_device *pdev)
 
 		input->name = ACPI_BUTTON_DEVICE_NAME_SLEEP;
 		input_set_capability(input, EV_KEY, KEY_SLEEP);
-
-		handler = acpi_button_notify;
 		break;
 
 	default:
@@ -617,42 +693,9 @@ static int acpi_button_probe(struct platform_device *pdev)
 
 	device_init_wakeup(button->dev, true);
 
-	switch (device->device_type) {
-	case ACPI_BUS_TYPE_POWER_BUTTON:
-		status = acpi_install_fixed_event_handler(ACPI_EVENT_POWER_BUTTON,
-							  acpi_button_event,
-							  button);
-		break;
-	case ACPI_BUS_TYPE_SLEEP_BUTTON:
-		status = acpi_install_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON,
-							  acpi_button_event,
-							  button);
-		break;
-	default:
-		status = acpi_install_notify_handler(device->handle,
-						     ACPI_ALL_NOTIFY, handler,
-						     button);
-		if (ACPI_SUCCESS(status) && device->wakeup.flags.valid) {
-			acpi_status st;
-
-			/*
-			 * If the wakeup GPE has a handler method, enable it in
-			 * case it is also used for signaling runtime events.
-			 */
-			st = acpi_enable_gpe_cond(device->wakeup.gpe_device,
-						   device->wakeup.gpe_number,
-						   ACPI_GPE_DISPATCH_METHOD);
-			button->gpe_enabled = ACPI_SUCCESS(st);
-			if (button->gpe_enabled)
-				dev_dbg(button->dev, "Enabled ACPI GPE%02llx\n",
-					device->wakeup.gpe_number);
-		}
-		break;
-	}
-	if (ACPI_FAILURE(status)) {
-		error = -ENODEV;
+	error = acpi_button_add_event_handler(button);
+	if (error)
 		goto err_input_unregister;
-	}
 
 	if (button_type == ACPI_BUTTON_TYPE_LID) {
 		/*
@@ -685,29 +728,7 @@ static void acpi_button_remove(struct platform_device *pdev)
 	if (button->type == ACPI_BUTTON_TYPE_LID)
 		acpi_lid_forget(adev);
 
-	switch (adev->device_type) {
-	case ACPI_BUS_TYPE_POWER_BUTTON:
-		acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON,
-						acpi_button_event);
-		break;
-	case ACPI_BUS_TYPE_SLEEP_BUTTON:
-		acpi_remove_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON,
-						acpi_button_event);
-		break;
-	default:
-		if (button->gpe_enabled) {
-			dev_dbg(button->dev, "Disabling ACPI GPE%02llx\n",
-				adev->wakeup.gpe_number);
-			acpi_disable_gpe(adev->wakeup.gpe_device,
-					 adev->wakeup.gpe_number);
-		}
-		acpi_remove_notify_handler(adev->handle, ACPI_ALL_NOTIFY,
-					   button->type == ACPI_BUTTON_TYPE_LID ?
-						acpi_lid_notify :
-						acpi_button_notify);
-		break;
-	}
-	acpi_os_wait_events_complete();
+	acpi_button_remove_event_handler(button);
 
 	device_init_wakeup(button->dev, false);
 
-- 
2.51.0

[PATCH v1 15/15] ACPI: button: Switch over to devres-based resource management

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Switch over the ACPI button driver to devres-based resource management
by making the following changes:

 * Use devm_kzalloc() for allocating button object memory.

 * Use devm_input_allocate_device() for allocating the input class
   device object.

 * Turn acpi_lid_remove_fs() into a devm cleanup action added
   by devm_acpi_lid_add_fs() which is a new wrapper around
   acpi_lid_add_fs().

 * Add devm_acpi_button_init_wakeup() for initializing the wakeup source
   and make it add a custom devm action that will automatically remove
   the wakeup source registered by it.

 * Turn acpi_button_remove_event_handler() into a devm cleanup action
   added by devm_acpi_button_add_event_handler() which is a new wrapper
   around acpi_button_add_event_handler().

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/button.c | 101 ++++++++++++++++++++++++------------------
 1 file changed, 57 insertions(+), 44 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index 8a56780e13da..154b17a8ea25 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -340,8 +340,9 @@ static int acpi_lid_add_fs(struct acpi_button *button)
 	return -ENODEV;
 }
 
-static void acpi_lid_remove_fs(struct acpi_button *button)
+static void acpi_lid_remove_fs(void *data)
 {
+	struct acpi_button *button = data;
 	struct acpi_device *device = button->adev;
 
 	remove_proc_entry(ACPI_BUTTON_FILE_STATE,
@@ -355,6 +356,17 @@ static void acpi_lid_remove_fs(struct acpi_button *button)
 	acpi_button_dir = NULL;
 }
 
+static int devm_acpi_lid_add_fs(struct device *dev, struct acpi_button *button)
+{
+	int ret;
+
+	ret = acpi_lid_add_fs(button);
+	if (ret)
+		return ret;
+
+	return devm_add_action_or_reset(dev, acpi_lid_remove_fs, button);
+}
+
 static acpi_handle saved_lid_handle;
 static DEFINE_MUTEX(acpi_lid_lock);
 
@@ -530,8 +542,20 @@ static acpi_notify_handler acpi_button_notify_handler(struct acpi_button *button
 	return acpi_button_notify;
 }
 
-static void acpi_button_remove_event_handler(struct acpi_button *button)
+static void acpi_button_wakeup_cleanup(void *data)
+{
+	device_init_wakeup(data, false);
+}
+
+static int devm_acpi_button_init_wakeup(struct device *dev)
 {
+	device_init_wakeup(dev, true);
+	return devm_add_action_or_reset(dev, acpi_button_wakeup_cleanup, dev);
+}
+
+static void acpi_button_remove_event_handler(void *data)
+{
+	struct acpi_button *button = data;
 	struct acpi_device *adev = button->adev;
 
 	switch (adev->device_type) {
@@ -606,6 +630,19 @@ static int acpi_button_add_event_handler(struct acpi_button *button)
 	return 0;
 }
 
+static int devm_acpi_button_add_event_handler(struct device *dev,
+					      struct acpi_button *button)
+{
+	int ret;
+
+	ret = acpi_button_add_event_handler(button);
+	if (ret)
+		return ret;
+
+	return devm_add_action_or_reset(dev, acpi_button_remove_event_handler,
+					button);
+}
+
 static int acpi_button_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -625,7 +662,7 @@ static int acpi_button_probe(struct platform_device *pdev)
 	    lid_init_state == ACPI_BUTTON_LID_INIT_DISABLED)
 		return -ENODEV;
 
-	button = kzalloc_obj(struct acpi_button);
+	button = devm_kzalloc(dev, sizeof(*button), GFP_KERNEL);
 	if (!button)
 		return -ENOMEM;
 
@@ -633,11 +670,10 @@ static int acpi_button_probe(struct platform_device *pdev)
 
 	button->dev = dev;
 	button->adev = device;
-	input = input_allocate_device();
-	if (!input) {
-		error = -ENOMEM;
-		goto err_free_button;
-	}
+	input = devm_input_allocate_device(dev);
+	if (!input)
+		return -ENOMEM;
+
 	button->input = input;
 	button->type = button_type;
 
@@ -649,11 +685,10 @@ static int acpi_button_probe(struct platform_device *pdev)
 		input_set_capability(input, EV_SW, SW_LID);
 		input->open = acpi_lid_input_open;
 
-		error = acpi_lid_add_fs(button);
-		if (error) {
-			input_free_device(input);
-			goto err_free_button;
-		}
+		error = devm_acpi_lid_add_fs(dev, button);
+		if (error)
+			return error;
+
 		break;
 
 	case ACPI_BUTTON_TYPE_POWER:
@@ -672,7 +707,6 @@ static int acpi_button_probe(struct platform_device *pdev)
 		break;
 
 	default:
-		input_free_device(input);
 		return dev_err_probe(dev, -ENODEV, "Unrecognized button type\n");
 	}
 
@@ -686,16 +720,16 @@ static int acpi_button_probe(struct platform_device *pdev)
 
 	input_set_drvdata(input, button);
 	error = input_register_device(input);
-	if (error) {
-		input_free_device(input);
-		goto err_remove_fs;
-	}
+	if (error)
+		return error;
 
-	device_init_wakeup(button->dev, true);
+	error = devm_acpi_button_init_wakeup(dev);
+	if (error)
+		return error;
 
-	error = acpi_button_add_event_handler(button);
+	error = devm_acpi_button_add_event_handler(dev, button);
 	if (error)
-		goto err_input_unregister;
+		return error;
 
 	if (button_type == ACPI_BUTTON_TYPE_LID) {
 		/*
@@ -706,37 +740,16 @@ static int acpi_button_probe(struct platform_device *pdev)
 	}
 
 	pr_info("%s [%s]\n", input->name, acpi_device_bid(device));
-	return 0;
 
-err_input_unregister:
-	device_init_wakeup(button->dev, false);
-	input_unregister_device(input);
-err_remove_fs:
-	if (button_type == ACPI_BUTTON_TYPE_LID)
-		acpi_lid_remove_fs(button);
-
-err_free_button:
-	kfree(button);
-	return error;
+	return 0;
 }
 
 static void acpi_button_remove(struct platform_device *pdev)
 {
 	struct acpi_button *button = platform_get_drvdata(pdev);
-	struct acpi_device *adev = button->adev;
-
-	if (button->type == ACPI_BUTTON_TYPE_LID)
-		acpi_lid_forget(adev);
-
-	acpi_button_remove_event_handler(button);
 
-	device_init_wakeup(button->dev, false);
-
-	input_unregister_device(button->input);
 	if (button->type == ACPI_BUTTON_TYPE_LID)
-		acpi_lid_remove_fs(button);
-
-	kfree(button);
+		acpi_lid_forget(button->adev);
 }
 
 static int param_set_lid_init_state(const char *val,
-- 
2.51.0