public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver
@ 2023-06-01 13:17 Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 03/35] acpi/video: " Michal Wilczynski
                   ` (29 more replies)
  0 siblings, 30 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown
  Cc: Michal Wilczynski, linux-acpi, platform-driver-x86

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add callback.
Call acpi_device_remove_event_handler() at the beginning of .remove
callback. Change arguments passed to the notify callback to match with
what's required by acpi_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/acpi/ac.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index 1ace70b831cd..208af5a3106e 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -34,7 +34,7 @@ MODULE_LICENSE("GPL");
 
 static int acpi_ac_add(struct acpi_device *device);
 static void acpi_ac_remove(struct acpi_device *device);
-static void acpi_ac_notify(struct acpi_device *device, u32 event);
+static void acpi_ac_notify(acpi_handle handle, u32 event, void *data);
 
 static const struct acpi_device_id ac_device_ids[] = {
 	{"ACPI0003", 0},
@@ -54,11 +54,9 @@ static struct acpi_driver acpi_ac_driver = {
 	.name = "ac",
 	.class = ACPI_AC_CLASS,
 	.ids = ac_device_ids,
-	.flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS,
 	.ops = {
 		.add = acpi_ac_add,
 		.remove = acpi_ac_remove,
-		.notify = acpi_ac_notify,
 		},
 	.drv.pm = &acpi_ac_pm,
 };
@@ -128,9 +126,12 @@ static enum power_supply_property ac_props[] = {
 };
 
 /* Driver Model */
-static void acpi_ac_notify(struct acpi_device *device, u32 event)
+static void acpi_ac_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct acpi_ac *ac = acpi_driver_data(device);
+	struct acpi_device *device = data;
+	struct acpi_ac *ac;
+
+	ac = acpi_driver_data(device);
 
 	if (!ac)
 		return;
@@ -256,6 +257,8 @@ static int acpi_ac_add(struct acpi_device *device)
 
 	ac->battery_nb.notifier_call = acpi_ac_battery_notify;
 	register_acpi_notifier(&ac->battery_nb);
+
+	result = acpi_device_install_event_handler(device, ACPI_ALL_NOTIFY, acpi_ac_notify);
 end:
 	if (result)
 		kfree(ac);
@@ -297,6 +300,7 @@ static void acpi_ac_remove(struct acpi_device *device)
 
 	ac = acpi_driver_data(device);
 
+	acpi_device_remove_event_handler(device, ACPI_ALL_NOTIFY, acpi_ac_notify);
 	power_supply_unregister(ac->charger);
 	unregister_acpi_notifier(&ac->battery_nb);
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 03/35] acpi/video: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 04/35] acpi/battery: " Michal Wilczynski
                   ` (28 subsequent siblings)
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown
  Cc: Michal Wilczynski, linux-acpi, platform-driver-x86

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/acpi/acpi_video.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 62f4364e4460..f1623fcc3973 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -77,7 +77,7 @@ static DEFINE_MUTEX(video_list_lock);
 static LIST_HEAD(video_bus_head);
 static int acpi_video_bus_add(struct acpi_device *device);
 static void acpi_video_bus_remove(struct acpi_device *device);
-static void acpi_video_bus_notify(struct acpi_device *device, u32 event);
+static void acpi_video_bus_notify(acpi_handle handle, u32 event, void *data);
 
 /*
  * Indices in the _BCL method response: the first two items are special,
@@ -104,7 +104,6 @@ static struct acpi_driver acpi_video_bus = {
 	.ops = {
 		.add = acpi_video_bus_add,
 		.remove = acpi_video_bus_remove,
-		.notify = acpi_video_bus_notify,
 		},
 };
 
@@ -1527,12 +1526,15 @@ static int acpi_video_bus_stop_devices(struct acpi_video_bus *video)
 				  acpi_osi_is_win8() ? 0 : 1);
 }
 
-static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
+static void acpi_video_bus_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct acpi_video_bus *video = acpi_driver_data(device);
+	struct acpi_device *device = data;
+	struct acpi_video_bus *video;
 	struct input_dev *input;
 	int keycode = 0;
 
+	video = acpi_driver_data(device);
+
 	if (!video || !video->input)
 		return;
 
@@ -2053,6 +2055,11 @@ static int acpi_video_bus_add(struct acpi_device *device)
 
 	acpi_video_bus_add_notify_handler(video);
 
+	error =  acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY,
+						   acpi_video_bus_notify);
+	if (error)
+		goto err_put_video;
+
 	return 0;
 
 err_put_video:
@@ -2075,6 +2082,7 @@ static void acpi_video_bus_remove(struct acpi_device *device)
 
 	video = acpi_driver_data(device);
 
+	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_video_bus_notify);
 	mutex_lock(&video_list_lock);
 	list_del(&video->entry);
 	mutex_unlock(&video_list_lock);
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 04/35] acpi/battery: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 03/35] acpi/video: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 05/35] acpi/button: " Michal Wilczynski
                   ` (27 subsequent siblings)
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown
  Cc: Michal Wilczynski, linux-acpi, platform-driver-x86

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/acpi/battery.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 9c67ed02d797..37449c33771b 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -1034,11 +1034,14 @@ static void acpi_battery_refresh(struct acpi_battery *battery)
 }
 
 /* Driver Interface */
-static void acpi_battery_notify(struct acpi_device *device, u32 event)
+static void acpi_battery_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct acpi_battery *battery = acpi_driver_data(device);
+	struct acpi_device *device = data;
+	struct acpi_battery *battery;
 	struct power_supply *old;
 
+	battery = acpi_driver_data(device);
+
 	if (!battery)
 		return;
 	old = battery->bat;
@@ -1212,7 +1215,11 @@ static int acpi_battery_add(struct acpi_device *device)
 
 	device_init_wakeup(&device->dev, 1);
 
-	return result;
+	result = acpi_device_install_event_handler(device, ACPI_ALL_NOTIFY, acpi_battery_notify);
+	if (result)
+		goto fail;
+
+	return 0;
 
 fail:
 	sysfs_remove_battery(battery);
@@ -1228,6 +1235,7 @@ static void acpi_battery_remove(struct acpi_device *device)
 
 	if (!device || !acpi_driver_data(device))
 		return;
+	acpi_device_remove_event_handler(device, ACPI_ALL_NOTIFY, acpi_battery_notify);
 	device_init_wakeup(&device->dev, 0);
 	battery = acpi_driver_data(device);
 	unregister_pm_notifier(&battery->pm_nb);
@@ -1264,11 +1272,9 @@ static struct acpi_driver acpi_battery_driver = {
 	.name = "battery",
 	.class = ACPI_BATTERY_CLASS,
 	.ids = battery_device_ids,
-	.flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS,
 	.ops = {
 		.add = acpi_battery_add,
 		.remove = acpi_battery_remove,
-		.notify = acpi_battery_notify,
 		},
 	.drv.pm = &acpi_battery_pm,
 };
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 05/35] acpi/button: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 03/35] acpi/video: " Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 04/35] acpi/battery: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 06/35] acpi/hed: " Michal Wilczynski
                   ` (26 subsequent siblings)
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown
  Cc: Michal Wilczynski, linux-acpi, platform-driver-x86

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/acpi/button.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index ef77c14c72a9..96ed1877a61a 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -135,7 +135,7 @@ static const struct dmi_system_id dmi_lid_quirks[] = {
 
 static int acpi_button_add(struct acpi_device *device);
 static void acpi_button_remove(struct acpi_device *device);
-static void acpi_button_notify(struct acpi_device *device, u32 event);
+static void acpi_button_notify(acpi_handle handle, u32 event, void *data);
 
 #ifdef CONFIG_PM_SLEEP
 static int acpi_button_suspend(struct device *dev);
@@ -153,7 +153,6 @@ static struct acpi_driver acpi_button_driver = {
 	.ops = {
 		.add = acpi_button_add,
 		.remove = acpi_button_remove,
-		.notify = acpi_button_notify,
 	},
 	.drv.pm = &acpi_button_pm,
 };
@@ -409,11 +408,14 @@ static void acpi_lid_initialize_state(struct acpi_device *device)
 	button->lid_state_initialized = true;
 }
 
-static void acpi_button_notify(struct acpi_device *device, u32 event)
+static void acpi_button_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct acpi_button *button = acpi_driver_data(device);
+	struct acpi_device *device = data;
+	struct acpi_button *button;
 	struct input_dev *input;
 
+	button = acpi_driver_data(device);
+
 	switch (event) {
 	case ACPI_FIXED_HARDWARE_EVENT:
 		event = ACPI_BUTTON_NOTIFY_STATUS;
@@ -578,6 +580,11 @@ static int acpi_button_add(struct acpi_device *device)
 
 	device_init_wakeup(&device->dev, true);
 	pr_info("%s [%s]\n", name, acpi_device_bid(device));
+
+	error =  acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_button_notify);
+	if (error)
+		goto err_remove_fs;
+
 	return 0;
 
  err_remove_fs:
@@ -593,6 +600,7 @@ static void acpi_button_remove(struct acpi_device *device)
 {
 	struct acpi_button *button = acpi_driver_data(device);
 
+	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_button_notify);
 	acpi_button_remove_fs(device);
 	input_unregister_device(button->input);
 	kfree(button);
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 06/35] acpi/hed: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (2 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 05/35] acpi/button: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 07/35] acpi/nfit: " Michal Wilczynski
                   ` (25 subsequent siblings)
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown
  Cc: Michal Wilczynski, linux-acpi, platform-driver-x86

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/acpi/hed.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/hed.c b/drivers/acpi/hed.c
index 78d44e3fe129..5217b7082b29 100644
--- a/drivers/acpi/hed.c
+++ b/drivers/acpi/hed.c
@@ -42,7 +42,7 @@ EXPORT_SYMBOL_GPL(unregister_acpi_hed_notifier);
  * it is used by HEST Generic Hardware Error Source with notify type
  * SCI.
  */
-static void acpi_hed_notify(struct acpi_device *device, u32 event)
+static void acpi_hed_notify(acpi_handle handle, u32 event, void *data)
 {
 	blocking_notifier_call_chain(&acpi_hed_notify_list, 0, NULL);
 }
@@ -53,11 +53,13 @@ static int acpi_hed_add(struct acpi_device *device)
 	if (hed_handle)
 		return -EINVAL;
 	hed_handle = device->handle;
-	return 0;
+
+	return acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_hed_notify);
 }
 
 static void acpi_hed_remove(struct acpi_device *device)
 {
+	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_hed_notify);
 	hed_handle = NULL;
 }
 
@@ -68,7 +70,6 @@ static struct acpi_driver acpi_hed_driver = {
 	.ops = {
 		.add = acpi_hed_add,
 		.remove = acpi_hed_remove,
-		.notify = acpi_hed_notify,
 	},
 };
 module_acpi_driver(acpi_hed_driver);
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 07/35] acpi/nfit: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (3 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 06/35] acpi/hed: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 08/35] acpi/thermal: " Michal Wilczynski
                   ` (24 subsequent siblings)
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Dan Williams, Vishal Verma, Dave Jiang, Ira Weiny,
	Rafael J. Wysocki, Len Brown
  Cc: Michal Wilczynski, nvdimm, linux-acpi, platform-driver-x86

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/acpi/nfit/core.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 07204d482968..633c34513071 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -3312,6 +3312,15 @@ void acpi_nfit_shutdown(void *data)
 }
 EXPORT_SYMBOL_GPL(acpi_nfit_shutdown);
 
+static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data)
+{
+	struct acpi_device *device = data;
+
+	device_lock(&device->dev);
+	__acpi_nfit_notify(&device->dev, handle, event);
+	device_unlock(&device->dev);
+}
+
 static int acpi_nfit_add(struct acpi_device *adev)
 {
 	struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
@@ -3368,12 +3377,18 @@ static int acpi_nfit_add(struct acpi_device *adev)
 
 	if (rc)
 		return rc;
-	return devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc);
+
+	rc = devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc);
+	if (rc)
+		return rc;
+
+	return acpi_device_install_event_handler(adev, ACPI_DEVICE_NOTIFY, acpi_nfit_notify);
 }
 
 static void acpi_nfit_remove(struct acpi_device *adev)
 {
 	/* see acpi_nfit_unregister */
+	acpi_device_remove_event_handler(adev, ACPI_DEVICE_NOTIFY, acpi_nfit_notify);
 }
 
 static void acpi_nfit_update_notify(struct device *dev, acpi_handle handle)
@@ -3446,13 +3461,6 @@ void __acpi_nfit_notify(struct device *dev, acpi_handle handle, u32 event)
 }
 EXPORT_SYMBOL_GPL(__acpi_nfit_notify);
 
-static void acpi_nfit_notify(struct acpi_device *adev, u32 event)
-{
-	device_lock(&adev->dev);
-	__acpi_nfit_notify(&adev->dev, adev->handle, event);
-	device_unlock(&adev->dev);
-}
-
 static const struct acpi_device_id acpi_nfit_ids[] = {
 	{ "ACPI0012", 0 },
 	{ "", 0 },
@@ -3465,7 +3473,6 @@ static struct acpi_driver acpi_nfit_driver = {
 	.ops = {
 		.add = acpi_nfit_add,
 		.remove = acpi_nfit_remove,
-		.notify = acpi_nfit_notify,
 	},
 };
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 08/35] acpi/thermal: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (4 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 07/35] acpi/nfit: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 09/35] acpi/tiny-power-button: " Michal Wilczynski
                   ` (23 subsequent siblings)
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, Zhang Rui, Len Brown
  Cc: Michal Wilczynski, linux-acpi, platform-driver-x86

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/acpi/thermal.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 4720a3649a61..66156c5998b7 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -75,7 +75,7 @@ static struct workqueue_struct *acpi_thermal_pm_queue;
 
 static int acpi_thermal_add(struct acpi_device *device);
 static void acpi_thermal_remove(struct acpi_device *device);
-static void acpi_thermal_notify(struct acpi_device *device, u32 event);
+static void acpi_thermal_notify(acpi_handle handle, u32 event, void *data);
 
 static const struct acpi_device_id  thermal_device_ids[] = {
 	{ACPI_THERMAL_HID, 0},
@@ -99,7 +99,6 @@ static struct acpi_driver acpi_thermal_driver = {
 	.ops = {
 		.add = acpi_thermal_add,
 		.remove = acpi_thermal_remove,
-		.notify = acpi_thermal_notify,
 		},
 	.drv.pm = &acpi_thermal_pm,
 };
@@ -894,9 +893,12 @@ static void acpi_queue_thermal_check(struct acpi_thermal *tz)
 		queue_work(acpi_thermal_pm_queue, &tz->thermal_check_work);
 }
 
-static void acpi_thermal_notify(struct acpi_device *device, u32 event)
+static void acpi_thermal_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct acpi_thermal *tz = acpi_driver_data(device);
+	struct acpi_device *device = data;
+	struct acpi_thermal *tz;
+
+	tz = acpi_driver_data(device);
 
 	if (!tz)
 		return;
@@ -1067,11 +1069,16 @@ static int acpi_thermal_add(struct acpi_device *device)
 
 	pr_info("%s [%s] (%ld C)\n", acpi_device_name(device),
 		acpi_device_bid(device), deci_kelvin_to_celsius(tz->temperature));
-	goto end;
+
+	result = acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY,
+						   acpi_thermal_notify);
+	if (result)
+		goto free_memory;
+
+	return 0;
 
 free_memory:
 	kfree(tz);
-end:
 	return result;
 }
 
@@ -1082,6 +1089,7 @@ static void acpi_thermal_remove(struct acpi_device *device)
 	if (!device || !acpi_driver_data(device))
 		return;
 
+	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_thermal_notify);
 	flush_workqueue(acpi_thermal_pm_queue);
 	tz = acpi_driver_data(device);
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 09/35] acpi/tiny-power-button: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (5 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 08/35] acpi/thermal: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 10/35] hwmon/acpi_power_meter: " Michal Wilczynski
                   ` (22 subsequent siblings)
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown
  Cc: Michal Wilczynski, linux-acpi, platform-driver-x86

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/acpi/tiny-power-button.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/tiny-power-button.c b/drivers/acpi/tiny-power-button.c
index 598f548b21f3..9c8e8b8cc19f 100644
--- a/drivers/acpi/tiny-power-button.c
+++ b/drivers/acpi/tiny-power-button.c
@@ -19,18 +19,21 @@ static const struct acpi_device_id tiny_power_button_device_ids[] = {
 };
 MODULE_DEVICE_TABLE(acpi, tiny_power_button_device_ids);
 
-static int acpi_noop_add(struct acpi_device *device)
+static void acpi_tiny_power_button_notify(acpi_handle handle, u32 event, void *data)
 {
-	return 0;
+	kill_cad_pid(power_signal, 1);
 }
 
-static void acpi_noop_remove(struct acpi_device *device)
+static int acpi_tiny_power_button_add(struct acpi_device *device)
 {
+	return acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY,
+						 acpi_tiny_power_button_notify);
 }
 
-static void acpi_tiny_power_button_notify(struct acpi_device *device, u32 event)
+static void acpi_tiny_power_button_remove(struct acpi_device *device)
 {
-	kill_cad_pid(power_signal, 1);
+	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY,
+					 acpi_tiny_power_button_notify);
 }
 
 static struct acpi_driver acpi_tiny_power_button_driver = {
@@ -38,9 +41,8 @@ static struct acpi_driver acpi_tiny_power_button_driver = {
 	.class = "tiny-power-button",
 	.ids = tiny_power_button_device_ids,
 	.ops = {
-		.add = acpi_noop_add,
-		.remove = acpi_noop_remove,
-		.notify = acpi_tiny_power_button_notify,
+		.add = acpi_tiny_power_button_add,
+		.remove = acpi_tiny_power_button_remove,
 	},
 };
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 10/35] hwmon/acpi_power_meter: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (6 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 09/35] acpi/tiny-power-button: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:51   ` Guenter Roeck
  2023-06-01 13:17 ` [PATCH v4 11/35] iio/acpi-als: " Michal Wilczynski
                   ` (21 subsequent siblings)
  29 siblings, 1 reply; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck
  Cc: Michal Wilczynski, linux-hwmon, linux-acpi, platform-driver-x86,
	rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/hwmon/acpi_power_meter.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
index fa28d447f0df..7410ee8693ba 100644
--- a/drivers/hwmon/acpi_power_meter.c
+++ b/drivers/hwmon/acpi_power_meter.c
@@ -817,9 +817,10 @@ static int read_capabilities(struct acpi_power_meter_resource *resource)
 }
 
 /* Handle ACPI event notifications */
-static void acpi_power_meter_notify(struct acpi_device *device, u32 event)
+static void acpi_power_meter_notify(acpi_handle handle, u32 event, void *data)
 {
 	struct acpi_power_meter_resource *resource;
+	struct acpi_device *device = data;
 	int res;
 
 	if (!device || !acpi_driver_data(device))
@@ -897,8 +898,12 @@ static int acpi_power_meter_add(struct acpi_device *device)
 		goto exit_remove;
 	}
 
-	res = 0;
-	goto exit;
+	res = acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY,
+						acpi_power_meter_notify);
+	if (res)
+		goto exit_remove;
+
+	return 0;
 
 exit_remove:
 	remove_attrs(resource);
@@ -906,7 +911,7 @@ static int acpi_power_meter_add(struct acpi_device *device)
 	free_capabilities(resource);
 exit_free:
 	kfree(resource);
-exit:
+
 	return res;
 }
 
@@ -917,6 +922,7 @@ static void acpi_power_meter_remove(struct acpi_device *device)
 	if (!device || !acpi_driver_data(device))
 		return;
 
+	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_power_meter_notify);
 	resource = acpi_driver_data(device);
 	hwmon_device_unregister(resource->hwmon_dev);
 
@@ -953,7 +959,6 @@ static struct acpi_driver acpi_power_meter_driver = {
 	.ops = {
 		.add = acpi_power_meter_add,
 		.remove = acpi_power_meter_remove,
-		.notify = acpi_power_meter_notify,
 		},
 	.drv.pm = pm_sleep_ptr(&acpi_power_meter_pm),
 };
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 11/35] iio/acpi-als: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (7 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 10/35] hwmon/acpi_power_meter: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-04 10:53   ` Jonathan Cameron
  2023-06-01 13:17 ` [PATCH v4 12/35] platform/chromeos_tbmc: " Michal Wilczynski
                   ` (20 subsequent siblings)
  29 siblings, 1 reply; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Jonathan Cameron, Lars-Peter Clausen
  Cc: Michal Wilczynski, linux-iio, linux-acpi, platform-driver-x86,
	rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/iio/light/acpi-als.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/light/acpi-als.c b/drivers/iio/light/acpi-als.c
index 2d91caf24dd0..5e200c6d91bc 100644
--- a/drivers/iio/light/acpi-als.c
+++ b/drivers/iio/light/acpi-als.c
@@ -100,10 +100,14 @@ static int acpi_als_read_value(struct acpi_als *als, char *prop, s32 *val)
 	return 0;
 }
 
-static void acpi_als_notify(struct acpi_device *device, u32 event)
+static void acpi_als_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct iio_dev *indio_dev = acpi_driver_data(device);
-	struct acpi_als *als = iio_priv(indio_dev);
+	struct acpi_device *device = data;
+	struct iio_dev *indio_dev;
+	struct acpi_als *als;
+
+	indio_dev = acpi_driver_data(device);
+	als = iio_priv(indio_dev);
 
 	if (iio_buffer_enabled(indio_dev) && iio_trigger_using_own(indio_dev)) {
 		switch (event) {
@@ -225,7 +229,16 @@ static int acpi_als_add(struct acpi_device *device)
 	if (ret)
 		return ret;
 
-	return devm_iio_device_register(dev, indio_dev);
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret)
+		return ret;
+
+	return acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_als_notify);
+}
+
+static void acpi_als_remove(struct acpi_device *device)
+{
+	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_als_notify);
 }
 
 static const struct acpi_device_id acpi_als_device_ids[] = {
@@ -241,7 +254,7 @@ static struct acpi_driver acpi_als_driver = {
 	.ids	= acpi_als_device_ids,
 	.ops = {
 		.add	= acpi_als_add,
-		.notify	= acpi_als_notify,
+		.remove = acpi_als_remove,
 	},
 };
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 12/35] platform/chromeos_tbmc: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (8 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 11/35] iio/acpi-als: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 13/35] platform/wilco_ec: " Michal Wilczynski
                   ` (19 subsequent siblings)
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Benson Leung
  Cc: Michal Wilczynski, chrome-platform, linux-acpi,
	platform-driver-x86, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/platform/chrome/chromeos_tbmc.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/chrome/chromeos_tbmc.c b/drivers/platform/chrome/chromeos_tbmc.c
index d1cf8f3463ce..ac0f43fc530a 100644
--- a/drivers/platform/chrome/chromeos_tbmc.c
+++ b/drivers/platform/chrome/chromeos_tbmc.c
@@ -45,8 +45,10 @@ static __maybe_unused int chromeos_tbmc_resume(struct device *dev)
 	return chromeos_tbmc_query_switch(adev, adev->driver_data);
 }
 
-static void chromeos_tbmc_notify(struct acpi_device *adev, u32 event)
+static void chromeos_tbmc_notify(acpi_handle handle, u32 event, void *data)
 {
+	struct acpi_device *adev = data;
+
 	acpi_pm_wakeup_event(&adev->dev);
 	switch (event) {
 	case 0x80:
@@ -92,7 +94,13 @@ static int chromeos_tbmc_add(struct acpi_device *adev)
 		return ret;
 	}
 	device_init_wakeup(dev, true);
-	return 0;
+
+	return acpi_device_install_event_handler(adev, ACPI_DEVICE_NOTIFY, chromeos_tbmc_notify);
+}
+
+static void chromeos_tbmc_remove(struct acpi_device *adev)
+{
+	acpi_device_remove_event_handler(adev, ACPI_DEVICE_NOTIFY, chromeos_tbmc_notify);
 }
 
 static const struct acpi_device_id chromeos_tbmc_acpi_device_ids[] = {
@@ -110,7 +118,7 @@ static struct acpi_driver chromeos_tbmc_driver = {
 	.ids = chromeos_tbmc_acpi_device_ids,
 	.ops = {
 		.add = chromeos_tbmc_add,
-		.notify = chromeos_tbmc_notify,
+		.remove = chromeos_tbmc_remove,
 	},
 	.drv.pm = &chromeos_tbmc_pm_ops,
 };
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 13/35] platform/wilco_ec: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (9 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 12/35] platform/chromeos_tbmc: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 14/35] platform/surface/button: " Michal Wilczynski
                   ` (18 subsequent siblings)
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Benson Leung
  Cc: Michal Wilczynski, chrome-platform, linux-acpi,
	platform-driver-x86, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/platform/chrome/wilco_ec/event.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/chrome/wilco_ec/event.c b/drivers/platform/chrome/wilco_ec/event.c
index a40f60bcefb6..23fd6de623f4 100644
--- a/drivers/platform/chrome/wilco_ec/event.c
+++ b/drivers/platform/chrome/wilco_ec/event.c
@@ -251,16 +251,10 @@ static int enqueue_events(struct acpi_device *adev, const u8 *buf, u32 length)
 	return 0;
 }
 
-/**
- * event_device_notify() - Callback when EC generates an event over ACPI.
- * @adev: The device that the event is coming from.
- * @value: Value passed to Notify() in ACPI.
- *
- * This function will read the events from the device and enqueue them.
- */
-static void event_device_notify(struct acpi_device *adev, u32 value)
+static void event_device_notify(acpi_handle handle, u32 value, void *data)
 {
 	struct acpi_buffer event_buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	struct acpi_device *adev = data;
 	union acpi_object *obj;
 	acpi_status status;
 
@@ -490,6 +484,11 @@ static int event_device_add(struct acpi_device *adev)
 	if (error)
 		goto free_dev_data;
 
+	error =  acpi_device_install_event_handler(adev, ACPI_DEVICE_NOTIFY,
+						   event_device_notify);
+	if (error)
+		goto free_dev_data;
+
 	return 0;
 
 free_dev_data:
@@ -503,6 +502,7 @@ static void event_device_remove(struct acpi_device *adev)
 {
 	struct event_device_data *dev_data = adev->driver_data;
 
+	acpi_device_remove_event_handler(adev, ACPI_DEVICE_NOTIFY, event_device_notify);
 	cdev_device_del(&dev_data->cdev, &dev_data->dev);
 	ida_simple_remove(&event_ida, MINOR(dev_data->dev.devt));
 	hangup_device(dev_data);
@@ -520,7 +520,6 @@ static struct acpi_driver event_driver = {
 	.ids = event_acpi_ids,
 	.ops = {
 		.add = event_device_add,
-		.notify = event_device_notify,
 		.remove = event_device_remove,
 	},
 	.owner = THIS_MODULE,
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 14/35] platform/surface/button: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (10 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 13/35] platform/wilco_ec: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 15/35] platform/x86/acer-wireless: " Michal Wilczynski
                   ` (17 subsequent siblings)
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Chen Yu, Hans de Goede, Mark Gross, Maximilian Luz
  Cc: Michal Wilczynski, platform-driver-x86, linux-acpi, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/platform/surface/surfacepro3_button.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/surface/surfacepro3_button.c b/drivers/platform/surface/surfacepro3_button.c
index 2755601f979c..fa5f1e4c4abc 100644
--- a/drivers/platform/surface/surfacepro3_button.c
+++ b/drivers/platform/surface/surfacepro3_button.c
@@ -71,13 +71,16 @@ struct surface_button {
 	bool suspended;
 };
 
-static void surface_button_notify(struct acpi_device *device, u32 event)
+static void surface_button_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct surface_button *button = acpi_driver_data(device);
-	struct input_dev *input;
+	struct acpi_device *device = data;
+	struct surface_button *button;
 	int key_code = KEY_RESERVED;
+	struct input_dev *input;
 	bool pressed = false;
 
+	button = acpi_driver_data(device);
+
 	switch (event) {
 	/* Power button press,release handle */
 	case SURFACE_BUTTON_NOTIFY_PRESS_POWER:
@@ -230,6 +233,12 @@ static int surface_button_add(struct acpi_device *device)
 	device_init_wakeup(&device->dev, true);
 	dev_info(&device->dev,
 			"%s [%s]\n", name, acpi_device_bid(device));
+
+	error = acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY,
+						  surface_button_notify);
+	if (error)
+		goto err_free_input;
+
 	return 0;
 
  err_free_input:
@@ -243,6 +252,7 @@ static void surface_button_remove(struct acpi_device *device)
 {
 	struct surface_button *button = acpi_driver_data(device);
 
+	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, surface_button_notify);
 	input_unregister_device(button->input);
 	kfree(button);
 }
@@ -257,7 +267,6 @@ static struct acpi_driver surface_button_driver = {
 	.ops = {
 		.add = surface_button_add,
 		.remove = surface_button_remove,
-		.notify = surface_button_notify,
 	},
 	.drv.pm = &surface_button_pm,
 };
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 15/35] platform/x86/acer-wireless: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (11 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 14/35] platform/surface/button: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 16/35] platform/x86/asus-laptop: " Michal Wilczynski
                   ` (16 subsequent siblings)
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Hans de Goede, Mark Gross
  Cc: Michal Wilczynski, platform-driver-x86, linux-acpi, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/platform/x86/acer-wireless.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/acer-wireless.c b/drivers/platform/x86/acer-wireless.c
index 1b5d935d085a..af7aef27cdff 100644
--- a/drivers/platform/x86/acer-wireless.c
+++ b/drivers/platform/x86/acer-wireless.c
@@ -18,9 +18,12 @@ static const struct acpi_device_id acer_wireless_acpi_ids[] = {
 };
 MODULE_DEVICE_TABLE(acpi, acer_wireless_acpi_ids);
 
-static void acer_wireless_notify(struct acpi_device *adev, u32 event)
+static void acer_wireless_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct input_dev *idev = acpi_driver_data(adev);
+	struct acpi_device *adev = data;
+	struct input_dev *idev;
+
+	idev = acpi_driver_data(adev);
 
 	dev_dbg(&adev->dev, "event=%#x\n", event);
 	if (event != 0x80) {
@@ -36,6 +39,7 @@ static void acer_wireless_notify(struct acpi_device *adev, u32 event)
 static int acer_wireless_add(struct acpi_device *adev)
 {
 	struct input_dev *idev;
+	int ret;
 
 	idev = devm_input_allocate_device(&adev->dev);
 	if (!idev)
@@ -50,7 +54,17 @@ static int acer_wireless_add(struct acpi_device *adev)
 	set_bit(EV_KEY, idev->evbit);
 	set_bit(KEY_RFKILL, idev->keybit);
 
-	return input_register_device(idev);
+	ret = input_register_device(idev);
+
+	if (ret)
+		return ret;
+
+	return acpi_device_install_event_handler(adev, ACPI_DEVICE_NOTIFY, acer_wireless_notify);
+}
+
+static void acer_wireless_remove(struct acpi_device *adev)
+{
+	acpi_device_remove_event_handler(adev, ACPI_DEVICE_NOTIFY, acer_wireless_notify);
 }
 
 static struct acpi_driver acer_wireless_driver = {
@@ -59,7 +73,7 @@ static struct acpi_driver acer_wireless_driver = {
 	.ids = acer_wireless_acpi_ids,
 	.ops = {
 		.add = acer_wireless_add,
-		.notify = acer_wireless_notify,
+		.remove = acer_wireless_remove,
 	},
 };
 module_acpi_driver(acer_wireless_driver);
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 16/35] platform/x86/asus-laptop: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (12 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 15/35] platform/x86/acer-wireless: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 17/35] platform/x86/asus-wireless: " Michal Wilczynski
                   ` (15 subsequent siblings)
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Corentin Chary, Hans de Goede, Mark Gross
  Cc: Michal Wilczynski, acpi4asus-user, platform-driver-x86,
	linux-acpi, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/platform/x86/asus-laptop.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c
index 761029f39314..6d2f5767bf70 100644
--- a/drivers/platform/x86/asus-laptop.c
+++ b/drivers/platform/x86/asus-laptop.c
@@ -1515,11 +1515,14 @@ static void asus_input_exit(struct asus_laptop *asus)
 /*
  * ACPI driver
  */
-static void asus_acpi_notify(struct acpi_device *device, u32 event)
+static void asus_acpi_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct asus_laptop *asus = acpi_driver_data(device);
+	struct acpi_device *device = data;
+	struct asus_laptop *asus;
 	u16 count;
 
+	asus = acpi_driver_data(device);
+
 	/* TODO Find a better way to handle events count. */
 	count = asus->event_count[event % 128]++;
 	acpi_bus_generate_netlink_event(asus->device->pnp.device_class,
@@ -1880,6 +1883,10 @@ static int asus_acpi_add(struct acpi_device *device)
 	if (result && result != -ENODEV)
 		goto fail_pega_rfkill;
 
+	result =  acpi_device_install_event_handler(device, ACPI_ALL_NOTIFY, asus_acpi_notify);
+	if (result)
+		goto fail_pega_rfkill;
+
 	asus_device_present = true;
 	return 0;
 
@@ -1905,6 +1912,7 @@ static void asus_acpi_remove(struct acpi_device *device)
 {
 	struct asus_laptop *asus = acpi_driver_data(device);
 
+	acpi_device_remove_event_handler(device, ACPI_ALL_NOTIFY, asus_acpi_notify);
 	asus_backlight_exit(asus);
 	asus_rfkill_exit(asus);
 	asus_led_exit(asus);
@@ -1928,11 +1936,9 @@ static struct acpi_driver asus_acpi_driver = {
 	.class = ASUS_LAPTOP_CLASS,
 	.owner = THIS_MODULE,
 	.ids = asus_device_ids,
-	.flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS,
 	.ops = {
 		.add = asus_acpi_add,
 		.remove = asus_acpi_remove,
-		.notify = asus_acpi_notify,
 		},
 };
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 17/35] platform/x86/asus-wireless: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (13 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 16/35] platform/x86/asus-laptop: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-02 13:09   ` Ilpo Järvinen
  2023-06-01 13:17 ` [PATCH v4 18/35] platform/x86/classmate-laptop: " Michal Wilczynski
                   ` (14 subsequent siblings)
  29 siblings, 1 reply; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Corentin Chary, João Paulo Rechi Vita, Hans de Goede,
	Mark Gross
  Cc: Michal Wilczynski, platform-driver-x86, acpi4asus-user,
	linux-acpi, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/platform/x86/asus-wireless.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c
index abf01e00b799..6544e3419ae4 100644
--- a/drivers/platform/x86/asus-wireless.c
+++ b/drivers/platform/x86/asus-wireless.c
@@ -108,19 +108,22 @@ static void led_state_set(struct led_classdev *led, enum led_brightness value)
 	queue_work(data->wq, &data->led_work);
 }
 
-static void asus_wireless_notify(struct acpi_device *adev, u32 event)
+static void asus_wireless_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct asus_wireless_data *data = acpi_driver_data(adev);
+	struct asus_wireless_data *w_data;
+	struct acpi_device *adev = data;
+
+	w_data = acpi_driver_data(adev);
 
 	dev_dbg(&adev->dev, "event=%#x\n", event);
 	if (event != 0x88) {
 		dev_notice(&adev->dev, "Unknown ASHS event: %#x\n", event);
 		return;
 	}
-	input_report_key(data->idev, KEY_RFKILL, 1);
-	input_sync(data->idev);
-	input_report_key(data->idev, KEY_RFKILL, 0);
-	input_sync(data->idev);
+	input_report_key(w_data->idev, KEY_RFKILL, 1);
+	input_sync(w_data->idev);
+	input_report_key(w_data->idev, KEY_RFKILL, 0);
+	input_sync(w_data->idev);
 }
 
 static int asus_wireless_add(struct acpi_device *adev)
@@ -169,16 +172,20 @@ static int asus_wireless_add(struct acpi_device *adev)
 	data->led.max_brightness = 1;
 	data->led.default_trigger = "rfkill-none";
 	err = devm_led_classdev_register(&adev->dev, &data->led);
-	if (err)
+	if (err) {
 		destroy_workqueue(data->wq);
+		return err;
+	}
 
-	return err;
+	return acpi_device_install_event_handler(adev, ACPI_DEVICE_NOTIFY, asus_wireless_notify);
 }
 
 static void asus_wireless_remove(struct acpi_device *adev)
 {
 	struct asus_wireless_data *data = acpi_driver_data(adev);
 
+	acpi_device_remove_event_handler(adev, ACPI_DEVICE_NOTIFY, asus_wireless_notify);
+
 	if (data->wq) {
 		devm_led_classdev_unregister(&adev->dev, &data->led);
 		destroy_workqueue(data->wq);
@@ -192,7 +199,6 @@ static struct acpi_driver asus_wireless_driver = {
 	.ops = {
 		.add = asus_wireless_add,
 		.remove = asus_wireless_remove,
-		.notify = asus_wireless_notify,
 	},
 };
 module_acpi_driver(asus_wireless_driver);
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 18/35] platform/x86/classmate-laptop: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (14 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 17/35] platform/x86/asus-wireless: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-02 10:29   ` Thadeu Lima de Souza Cascardo
  2023-06-01 13:17 ` [PATCH v4 19/35] platform/x86/dell/dell-rbtn: " Michal Wilczynski
                   ` (13 subsequent siblings)
  29 siblings, 1 reply; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Thadeu Lima de Souza Cascardo, Daniel Oliveira Nascimento,
	Hans de Goede, Mark Gross
  Cc: Michal Wilczynski, platform-driver-x86, linux-acpi, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/platform/x86/classmate-laptop.c | 53 +++++++++++++++++++------
 1 file changed, 41 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/x86/classmate-laptop.c b/drivers/platform/x86/classmate-laptop.c
index 2edaea2492df..2d36abf5ecfe 100644
--- a/drivers/platform/x86/classmate-laptop.c
+++ b/drivers/platform/x86/classmate-laptop.c
@@ -180,8 +180,9 @@ static acpi_status cmpc_get_accel_v4(acpi_handle handle,
 	return status;
 }
 
-static void cmpc_accel_handler_v4(struct acpi_device *dev, u32 event)
+static void cmpc_accel_handler_v4(acpi_handle handle, u32 event, void *data)
 {
+	struct acpi_device *dev = data;
 	if (event == 0x81) {
 		int16_t x, y, z;
 		acpi_status status;
@@ -407,6 +408,11 @@ static int cmpc_accel_add_v4(struct acpi_device *acpi)
 	inputdev = dev_get_drvdata(&acpi->dev);
 	dev_set_drvdata(&inputdev->dev, accel);
 
+	error = acpi_device_install_event_handler(acpi, ACPI_DEVICE_NOTIFY,
+						  cmpc_accel_handler_v4);
+	if (error)
+		goto failed_input;
+
 	return 0;
 
 failed_input:
@@ -420,6 +426,7 @@ static int cmpc_accel_add_v4(struct acpi_device *acpi)
 
 static void cmpc_accel_remove_v4(struct acpi_device *acpi)
 {
+	acpi_device_remove_event_handler(acpi, ACPI_DEVICE_NOTIFY, cmpc_accel_handler_v4);
 	device_remove_file(&acpi->dev, &cmpc_accel_sensitivity_attr_v4);
 	device_remove_file(&acpi->dev, &cmpc_accel_g_select_attr_v4);
 	cmpc_remove_acpi_notify_device(acpi);
@@ -441,7 +448,6 @@ static struct acpi_driver cmpc_accel_acpi_driver_v4 = {
 	.ops = {
 		.add = cmpc_accel_add_v4,
 		.remove = cmpc_accel_remove_v4,
-		.notify = cmpc_accel_handler_v4,
 	},
 	.drv.pm = &cmpc_accel_pm,
 };
@@ -523,8 +529,10 @@ static acpi_status cmpc_get_accel(acpi_handle handle,
 	return status;
 }
 
-static void cmpc_accel_handler(struct acpi_device *dev, u32 event)
+static void cmpc_accel_handler(acpi_handle handle, u32 event, void *data)
 {
+	struct acpi_device *dev = data;
+
 	if (event == 0x81) {
 		unsigned char x, y, z;
 		acpi_status status;
@@ -639,6 +647,11 @@ static int cmpc_accel_add(struct acpi_device *acpi)
 	inputdev = dev_get_drvdata(&acpi->dev);
 	dev_set_drvdata(&inputdev->dev, accel);
 
+	error = acpi_device_install_event_handler(acpi, ACPI_DEVICE_NOTIFY,
+						  cmpc_accel_handler);
+	if (error)
+		goto failed_input;
+
 	return 0;
 
 failed_input:
@@ -650,6 +663,7 @@ static int cmpc_accel_add(struct acpi_device *acpi)
 
 static void cmpc_accel_remove(struct acpi_device *acpi)
 {
+	acpi_device_remove_event_handler(acpi, ACPI_DEVICE_NOTIFY, cmpc_accel_handler);
 	device_remove_file(&acpi->dev, &cmpc_accel_sensitivity_attr);
 	cmpc_remove_acpi_notify_device(acpi);
 }
@@ -667,7 +681,6 @@ static struct acpi_driver cmpc_accel_acpi_driver = {
 	.ops = {
 		.add = cmpc_accel_add,
 		.remove = cmpc_accel_remove,
-		.notify = cmpc_accel_handler,
 	}
 };
 
@@ -693,8 +706,9 @@ static acpi_status cmpc_get_tablet(acpi_handle handle,
 	return status;
 }
 
-static void cmpc_tablet_handler(struct acpi_device *dev, u32 event)
+static void cmpc_tablet_handler(acpi_handle handle, u32 event, void *data)
 {
+	struct acpi_device *dev = data;
 	unsigned long long val = 0;
 	struct input_dev *inputdev = dev_get_drvdata(&dev->dev);
 
@@ -723,12 +737,20 @@ static void cmpc_tablet_idev_init(struct input_dev *inputdev)
 
 static int cmpc_tablet_add(struct acpi_device *acpi)
 {
-	return cmpc_add_acpi_notify_device(acpi, "cmpc_tablet",
-					   cmpc_tablet_idev_init);
+	int ret;
+
+	ret = cmpc_add_acpi_notify_device(acpi, "cmpc_tablet",
+					  cmpc_tablet_idev_init);
+	if (ret)
+		return ret;
+
+	return acpi_device_install_event_handler(acpi, ACPI_DEVICE_NOTIFY,
+						 cmpc_tablet_handler);
 }
 
 static void cmpc_tablet_remove(struct acpi_device *acpi)
 {
+	acpi_device_remove_event_handler(acpi, ACPI_DEVICE_NOTIFY, cmpc_tablet_handler);
 	cmpc_remove_acpi_notify_device(acpi);
 }
 
@@ -761,7 +783,6 @@ static struct acpi_driver cmpc_tablet_acpi_driver = {
 	.ops = {
 		.add = cmpc_tablet_add,
 		.remove = cmpc_tablet_remove,
-		.notify = cmpc_tablet_handler,
 	},
 	.drv.pm = &cmpc_tablet_pm,
 };
@@ -1026,8 +1047,9 @@ static int cmpc_keys_codes[] = {
 	KEY_MAX
 };
 
-static void cmpc_keys_handler(struct acpi_device *dev, u32 event)
+static void cmpc_keys_handler(acpi_handle handle, u32 event, void *data)
 {
+	struct acpi_device *dev = data;
 	struct input_dev *inputdev;
 	int code = KEY_MAX;
 
@@ -1049,12 +1071,20 @@ static void cmpc_keys_idev_init(struct input_dev *inputdev)
 
 static int cmpc_keys_add(struct acpi_device *acpi)
 {
-	return cmpc_add_acpi_notify_device(acpi, "cmpc_keys",
-					   cmpc_keys_idev_init);
+	int error;
+
+	error = cmpc_add_acpi_notify_device(acpi, "cmpc_keys",
+					    cmpc_keys_idev_init);
+	if (error)
+		return error;
+
+	return acpi_device_install_event_handler(acpi, ACPI_DEVICE_NOTIFY,
+						 cmpc_keys_handler);
 }
 
 static void cmpc_keys_remove(struct acpi_device *acpi)
 {
+	acpi_device_remove_event_handler(acpi, ACPI_DEVICE_NOTIFY, cmpc_keys_handler);
 	cmpc_remove_acpi_notify_device(acpi);
 }
 
@@ -1071,7 +1101,6 @@ static struct acpi_driver cmpc_keys_acpi_driver = {
 	.ops = {
 		.add = cmpc_keys_add,
 		.remove = cmpc_keys_remove,
-		.notify = cmpc_keys_handler,
 	}
 };
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 19/35] platform/x86/dell/dell-rbtn: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (15 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 18/35] platform/x86/classmate-laptop: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-02 13:20   ` Ilpo Järvinen
  2023-06-01 13:17 ` [PATCH v4 20/35] platform/x86/eeepc-laptop: " Michal Wilczynski
                   ` (12 subsequent siblings)
  29 siblings, 1 reply; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Pali Rohár, Hans de Goede, Mark Gross
  Cc: Michal Wilczynski, platform-driver-x86, linux-acpi, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/platform/x86/dell/dell-rbtn.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-rbtn.c b/drivers/platform/x86/dell/dell-rbtn.c
index aa0e6c907494..4dcad59eb035 100644
--- a/drivers/platform/x86/dell/dell-rbtn.c
+++ b/drivers/platform/x86/dell/dell-rbtn.c
@@ -207,7 +207,7 @@ static void rbtn_input_event(struct rbtn_data *rbtn_data)
 
 static int rbtn_add(struct acpi_device *device);
 static void rbtn_remove(struct acpi_device *device);
-static void rbtn_notify(struct acpi_device *device, u32 event);
+static void rbtn_notify(acpi_handle handle, u32 event, void *data);
 
 static const struct acpi_device_id rbtn_ids[] = {
 	{ "DELRBTN", 0 },
@@ -293,7 +293,6 @@ static struct acpi_driver rbtn_driver = {
 	.ops = {
 		.add = rbtn_add,
 		.remove = rbtn_remove,
-		.notify = rbtn_notify,
 	},
 	.owner = THIS_MODULE,
 };
@@ -422,7 +421,10 @@ static int rbtn_add(struct acpi_device *device)
 		ret = -EINVAL;
 	}
 
-	return ret;
+	if (ret)
+		return ret;
+
+	return acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY, rbtn_notify);
 
 }
 
@@ -430,6 +432,8 @@ static void rbtn_remove(struct acpi_device *device)
 {
 	struct rbtn_data *rbtn_data = device->driver_data;
 
+	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, rbtn_notify);
+
 	switch (rbtn_data->type) {
 	case RBTN_TOGGLE:
 		rbtn_input_exit(rbtn_data);
@@ -445,9 +449,12 @@ static void rbtn_remove(struct acpi_device *device)
 	device->driver_data = NULL;
 }
 
-static void rbtn_notify(struct acpi_device *device, u32 event)
+static void rbtn_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct rbtn_data *rbtn_data = device->driver_data;
+	struct acpi_device *device = data;
+	struct rbtn_data *rbtn_data;
+
+	rbtn_data = device->driver_data;
 
 	/*
 	 * Some BIOSes send a notification at resume.
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 20/35] platform/x86/eeepc-laptop: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (16 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 19/35] platform/x86/dell/dell-rbtn: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-02 13:24   ` Ilpo Järvinen
  2023-06-01 13:17 ` [PATCH v4 21/35] platform/x86/fujitsu-laptop: " Michal Wilczynski
                   ` (11 subsequent siblings)
  29 siblings, 1 reply; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Corentin Chary, Hans de Goede, Mark Gross
  Cc: Michal Wilczynski, acpi4asus-user, platform-driver-x86,
	linux-acpi, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/platform/x86/eeepc-laptop.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c
index 62b71e8e3567..bd6ada963d88 100644
--- a/drivers/platform/x86/eeepc-laptop.c
+++ b/drivers/platform/x86/eeepc-laptop.c
@@ -1204,12 +1204,15 @@ static void eeepc_input_notify(struct eeepc_laptop *eeepc, int event)
 		pr_info("Unknown key %x pressed\n", event);
 }
 
-static void eeepc_acpi_notify(struct acpi_device *device, u32 event)
+static void eeepc_acpi_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct eeepc_laptop *eeepc = acpi_driver_data(device);
 	int old_brightness, new_brightness;
+	struct acpi_device *device = data;
+	struct eeepc_laptop *eeepc;
 	u16 count;
 
+	eeepc = acpi_driver_data(device);
+
 	if (event > ACPI_MAX_SYS_NOTIFY)
 		return;
 	count = eeepc->event_count[event % 128]++;
@@ -1423,6 +1426,11 @@ static int eeepc_acpi_add(struct acpi_device *device)
 		goto fail_rfkill;
 
 	eeepc_device_present = true;
+	result = acpi_device_install_event_handler(device, ACPI_ALL_NOTIFY,
+						   eeepc_acpi_notify);
+	if (result)
+		goto fail_rfkill;
+
 	return 0;
 
 fail_rfkill:
@@ -1444,6 +1452,8 @@ static void eeepc_acpi_remove(struct acpi_device *device)
 {
 	struct eeepc_laptop *eeepc = acpi_driver_data(device);
 
+	acpi_device_remove_event_handler(device, ACPI_ALL_NOTIFY,
+					 eeepc_acpi_notify);
 	eeepc_backlight_exit(eeepc);
 	eeepc_rfkill_exit(eeepc);
 	eeepc_input_exit(eeepc);
@@ -1465,11 +1475,9 @@ static struct acpi_driver eeepc_acpi_driver = {
 	.class = EEEPC_ACPI_CLASS,
 	.owner = THIS_MODULE,
 	.ids = eeepc_device_ids,
-	.flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS,
 	.ops = {
 		.add = eeepc_acpi_add,
 		.remove = eeepc_acpi_remove,
-		.notify = eeepc_acpi_notify,
 	},
 };
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 21/35] platform/x86/fujitsu-laptop: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (17 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 20/35] platform/x86/eeepc-laptop: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-02 13:30   ` Ilpo Järvinen
  2023-06-01 13:17 ` [PATCH v4 22/35] platform/x86/lg-laptop: " Michal Wilczynski
                   ` (10 subsequent siblings)
  29 siblings, 1 reply; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Jonathan Woithe, Hans de Goede, Mark Gross
  Cc: Michal Wilczynski, platform-driver-x86, linux-acpi, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/platform/x86/fujitsu-laptop.c | 86 +++++++++++++++++----------
 1 file changed, 54 insertions(+), 32 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 085e044e888e..001333edba9f 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -136,6 +136,8 @@ struct fujitsu_laptop {
 
 static struct acpi_device *fext;
 
+static void acpi_fujitsu_laptop_notify(acpi_handle handle, u32 event, void *data);
+
 /* Fujitsu ACPI interface function */
 
 static int call_fext_func(struct acpi_device *device,
@@ -382,6 +384,37 @@ static int fujitsu_backlight_register(struct acpi_device *device)
 	return 0;
 }
 
+static void acpi_fujitsu_bl_notify(acpi_handle handle, u32 event, void *data)
+{
+	struct acpi_device *device = data;
+	struct fujitsu_bl *priv;
+	int oldb, newb;
+
+	priv = acpi_driver_data(device);
+
+	if (event != ACPI_FUJITSU_NOTIFY_CODE) {
+		acpi_handle_info(device->handle, "unsupported event [0x%x]\n",
+				 event);
+		sparse_keymap_report_event(priv->input, -1, 1, true);
+		return;
+	}
+
+	oldb = priv->brightness_level;
+	get_lcd_level(device);
+	newb = priv->brightness_level;
+
+	acpi_handle_debug(device->handle,
+			  "brightness button event [%i -> %i]\n", oldb, newb);
+
+	if (oldb == newb)
+		return;
+
+	if (!disable_brightness_adjust)
+		set_lcd_level(device, newb);
+
+	sparse_keymap_report_event(priv->input, oldb < newb, 1, true);
+}
+
 static int acpi_fujitsu_bl_add(struct acpi_device *device)
 {
 	struct fujitsu_bl *priv;
@@ -410,37 +443,17 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device)
 	if (ret)
 		return ret;
 
-	return fujitsu_backlight_register(device);
-}
+	ret = fujitsu_backlight_register(device);
+	if (ret)
+		return ret;
 
-/* Brightness notify */
+	return acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY,
+						 acpi_fujitsu_bl_notify);
+}
 
-static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
+static void acpi_fujitsu_bl_remove(struct acpi_device *device)
 {
-	struct fujitsu_bl *priv = acpi_driver_data(device);
-	int oldb, newb;
-
-	if (event != ACPI_FUJITSU_NOTIFY_CODE) {
-		acpi_handle_info(device->handle, "unsupported event [0x%x]\n",
-				 event);
-		sparse_keymap_report_event(priv->input, -1, 1, true);
-		return;
-	}
-
-	oldb = priv->brightness_level;
-	get_lcd_level(device);
-	newb = priv->brightness_level;
-
-	acpi_handle_debug(device->handle,
-			  "brightness button event [%i -> %i]\n", oldb, newb);
-
-	if (oldb == newb)
-		return;
-
-	if (!disable_brightness_adjust)
-		set_lcd_level(device, newb);
-
-	sparse_keymap_report_event(priv->input, oldb < newb, 1, true);
+	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_fujitsu_bl_notify);
 }
 
 /* ACPI device for hotkey handling */
@@ -839,6 +852,11 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
 	if (ret)
 		goto err_free_fifo;
 
+	ret = acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY,
+						acpi_fujitsu_laptop_notify);
+	if (ret)
+		goto err_free_fifo;
+
 	return 0;
 
 err_free_fifo:
@@ -851,6 +869,8 @@ static void acpi_fujitsu_laptop_remove(struct acpi_device *device)
 {
 	struct fujitsu_laptop *priv = acpi_driver_data(device);
 
+	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_fujitsu_laptop_notify);
+
 	fujitsu_laptop_platform_remove(device);
 
 	kfifo_free(&priv->fifo);
@@ -889,13 +909,16 @@ static void acpi_fujitsu_laptop_release(struct acpi_device *device)
 	}
 }
 
-static void acpi_fujitsu_laptop_notify(struct acpi_device *device, u32 event)
+static void acpi_fujitsu_laptop_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct fujitsu_laptop *priv = acpi_driver_data(device);
+	struct acpi_device *device = data;
+	struct fujitsu_laptop *priv;
 	unsigned long flags;
 	int scancode, i = 0;
 	unsigned int irb;
 
+	priv = acpi_driver_data(device);
+
 	if (event != ACPI_FUJITSU_NOTIFY_CODE) {
 		acpi_handle_info(device->handle, "Unsupported event [0x%x]\n",
 				 event);
@@ -947,7 +970,7 @@ static struct acpi_driver acpi_fujitsu_bl_driver = {
 	.ids = fujitsu_bl_device_ids,
 	.ops = {
 		.add = acpi_fujitsu_bl_add,
-		.notify = acpi_fujitsu_bl_notify,
+		.remove = acpi_fujitsu_bl_remove,
 		},
 };
 
@@ -963,7 +986,6 @@ static struct acpi_driver acpi_fujitsu_laptop_driver = {
 	.ops = {
 		.add = acpi_fujitsu_laptop_add,
 		.remove = acpi_fujitsu_laptop_remove,
-		.notify = acpi_fujitsu_laptop_notify,
 		},
 };
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 22/35] platform/x86/lg-laptop: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (18 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 21/35] platform/x86/fujitsu-laptop: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 23/35] platform/x86/panasonic-laptop: " Michal Wilczynski
                   ` (9 subsequent siblings)
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Matan Ziv-Av, Hans de Goede, Mark Gross
  Cc: Michal Wilczynski, platform-driver-x86, linux-acpi, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/platform/x86/lg-laptop.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/lg-laptop.c b/drivers/platform/x86/lg-laptop.c
index ad3c39e9e9f5..7469354c84d6 100644
--- a/drivers/platform/x86/lg-laptop.c
+++ b/drivers/platform/x86/lg-laptop.c
@@ -270,8 +270,9 @@ static void wmi_input_setup(void)
 	}
 }
 
-static void acpi_notify(struct acpi_device *device, u32 event)
+static void acpi_notify(acpi_handle handle, u32 event, void *data)
 {
+	struct acpi_device *device = data;
 	struct key_entry *key;
 
 	acpi_handle_debug(device->handle, "notify: %d\n", event);
@@ -752,6 +753,10 @@ static int acpi_add(struct acpi_device *device)
 	wmi_input_setup();
 	battery_hook_register(&battery_hook);
 
+	ret = acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_notify);
+	if (ret)
+		goto out_platform_device;
+
 	return 0;
 
 out_platform_device:
@@ -763,6 +768,8 @@ static int acpi_add(struct acpi_device *device)
 
 static void acpi_remove(struct acpi_device *device)
 {
+	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_notify);
+
 	sysfs_remove_group(&pf_device->dev.kobj, &dev_attribute_group);
 
 	led_classdev_unregister(&tpad_led);
@@ -788,7 +795,6 @@ static struct acpi_driver acpi_driver = {
 	.ops = {
 		.add = acpi_add,
 		.remove = acpi_remove,
-		.notify = acpi_notify,
 		},
 	.owner = THIS_MODULE,
 };
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 23/35] platform/x86/panasonic-laptop: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (19 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 22/35] platform/x86/lg-laptop: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 24/35] platform/x86/system76_acpi: " Michal Wilczynski
                   ` (8 subsequent siblings)
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Kenneth Chan, Hans de Goede, Mark Gross
  Cc: Michal Wilczynski, platform-driver-x86, linux-acpi, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/platform/x86/panasonic-laptop.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
index cf845ee1c7b1..723a898738d0 100644
--- a/drivers/platform/x86/panasonic-laptop.c
+++ b/drivers/platform/x86/panasonic-laptop.c
@@ -184,7 +184,7 @@ enum SINF_BITS { SINF_NUM_BATTERIES = 0,
 
 static int acpi_pcc_hotkey_add(struct acpi_device *device);
 static void acpi_pcc_hotkey_remove(struct acpi_device *device);
-static void acpi_pcc_hotkey_notify(struct acpi_device *device, u32 event);
+static void acpi_pcc_hotkey_notify(acpi_handle handle, u32 event, void *data);
 
 static const struct acpi_device_id pcc_device_ids[] = {
 	{ "MAT0012", 0},
@@ -207,7 +207,6 @@ static struct acpi_driver acpi_pcc_driver = {
 	.ops =		{
 				.add =		acpi_pcc_hotkey_add,
 				.remove =	acpi_pcc_hotkey_remove,
-				.notify =	acpi_pcc_hotkey_notify,
 			},
 	.drv.pm =	&acpi_pcc_hotkey_pm,
 };
@@ -833,9 +832,12 @@ static void acpi_pcc_generate_keyinput(struct pcc_acpi *pcc)
 		pr_err("Unknown hotkey event: 0x%04llx\n", result);
 }
 
-static void acpi_pcc_hotkey_notify(struct acpi_device *device, u32 event)
+static void acpi_pcc_hotkey_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct pcc_acpi *pcc = acpi_driver_data(device);
+	struct acpi_device *device = data;
+	struct pcc_acpi *pcc;
+
+	pcc = acpi_driver_data(device);
 
 	switch (event) {
 	case HKEY_NOTIFY:
@@ -1049,6 +1051,12 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
 	}
 
 	i8042_install_filter(panasonic_i8042_filter);
+
+	result = acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY,
+						   acpi_pcc_hotkey_notify);
+	if (result)
+		goto out_platform;
+
 	return 0;
 
 out_platform:
@@ -1072,6 +1080,8 @@ static void acpi_pcc_hotkey_remove(struct acpi_device *device)
 	if (!device || !pcc)
 		return;
 
+	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_pcc_hotkey_notify);
+
 	i8042_remove_filter(panasonic_i8042_filter);
 
 	if (pcc->platform) {
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 24/35] platform/x86/system76_acpi: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (20 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 23/35] platform/x86/panasonic-laptop: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 25/35] platform/x86/topstar-laptop: " Michal Wilczynski
                   ` (7 subsequent siblings)
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Jeremy Soller, System76 Product Development, Hans de Goede,
	Mark Gross
  Cc: Michal Wilczynski, platform-driver-x86, linux-acpi, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/platform/x86/system76_acpi.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/system76_acpi.c b/drivers/platform/x86/system76_acpi.c
index 97f5a8255b91..b08411b60993 100644
--- a/drivers/platform/x86/system76_acpi.c
+++ b/drivers/platform/x86/system76_acpi.c
@@ -627,29 +627,30 @@ static void input_key(struct system76_data *data, unsigned int code)
 }
 
 // Handle ACPI notification
-static void system76_notify(struct acpi_device *acpi_dev, u32 event)
+static void system76_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct system76_data *data;
+	struct acpi_device *acpi_dev = data;
+	struct system76_data *s_data;
 
-	data = acpi_driver_data(acpi_dev);
+	s_data = acpi_driver_data(acpi_dev);
 	switch (event) {
 	case 0x80:
-		kb_led_hotkey_hardware(data);
+		kb_led_hotkey_hardware(s_data);
 		break;
 	case 0x81:
-		kb_led_hotkey_toggle(data);
+		kb_led_hotkey_toggle(s_data);
 		break;
 	case 0x82:
-		kb_led_hotkey_down(data);
+		kb_led_hotkey_down(s_data);
 		break;
 	case 0x83:
-		kb_led_hotkey_up(data);
+		kb_led_hotkey_up(s_data);
 		break;
 	case 0x84:
-		kb_led_hotkey_color(data);
+		kb_led_hotkey_color(s_data);
 		break;
 	case 0x85:
-		input_key(data, KEY_SCREENLOCK);
+		input_key(s_data, KEY_SCREENLOCK);
 		break;
 	}
 }
@@ -733,6 +734,10 @@ static int system76_add(struct acpi_device *acpi_dev)
 		system76_battery_init();
 	}
 
+	err = acpi_device_install_event_handler(acpi_dev, ACPI_DEVICE_NOTIFY, system76_notify);
+	if (err)
+		goto error;
+
 	return 0;
 
 error:
@@ -750,6 +755,8 @@ static void system76_remove(struct acpi_device *acpi_dev)
 
 	data = acpi_driver_data(acpi_dev);
 
+	acpi_device_remove_event_handler(acpi_dev, ACPI_DEVICE_NOTIFY, system76_notify);
+
 	if (data->has_open_ec) {
 		system76_battery_exit();
 		kfree(data->nfan);
@@ -769,7 +776,6 @@ static struct acpi_driver system76_driver = {
 	.ops = {
 		.add = system76_add,
 		.remove = system76_remove,
-		.notify = system76_notify,
 	},
 };
 module_acpi_driver(system76_driver);
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 25/35] platform/x86/topstar-laptop: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (21 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 24/35] platform/x86/system76_acpi: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 26/35] platform/x86/toshiba_acpi: " Michal Wilczynski
                   ` (6 subsequent siblings)
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Herton Ronaldo Krzesinski, Hans de Goede, Mark Gross
  Cc: Michal Wilczynski, platform-driver-x86, linux-acpi, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/platform/x86/topstar-laptop.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/topstar-laptop.c b/drivers/platform/x86/topstar-laptop.c
index 20df1ebefc30..cfe2a92ef1f5 100644
--- a/drivers/platform/x86/topstar-laptop.c
+++ b/drivers/platform/x86/topstar-laptop.c
@@ -232,12 +232,15 @@ static int topstar_acpi_fncx_switch(struct acpi_device *device, bool state)
 	return 0;
 }
 
-static void topstar_acpi_notify(struct acpi_device *device, u32 event)
+static void topstar_acpi_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct topstar_laptop *topstar = acpi_driver_data(device);
+	struct acpi_device *device = data;
+	struct topstar_laptop *topstar;
 	static bool dup_evnt[2];
 	bool *dup;
 
+	topstar = acpi_driver_data(device);
+
 	/* 0x83 and 0x84 key events comes duplicated... */
 	if (event == 0x83 || event == 0x84) {
 		dup = &dup_evnt[event - 0x83];
@@ -319,6 +322,10 @@ static int topstar_acpi_add(struct acpi_device *device)
 			goto err_input_exit;
 	}
 
+	err = acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY, topstar_acpi_notify);
+	if (err)
+		goto err_input_exit;
+
 	return 0;
 
 err_input_exit:
@@ -336,6 +343,8 @@ static void topstar_acpi_remove(struct acpi_device *device)
 {
 	struct topstar_laptop *topstar = acpi_driver_data(device);
 
+	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, topstar_acpi_notify);
+
 	if (led_workaround)
 		topstar_led_exit(topstar);
 
@@ -360,7 +369,6 @@ static struct acpi_driver topstar_acpi_driver = {
 	.ops = {
 		.add = topstar_acpi_add,
 		.remove = topstar_acpi_remove,
-		.notify = topstar_acpi_notify,
 	},
 };
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 26/35] platform/x86/toshiba_acpi: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (22 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 25/35] platform/x86/topstar-laptop: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 27/35] platform/x86/toshiba_bluetooth: " Michal Wilczynski
                   ` (5 subsequent siblings)
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Azael Avalos, Hans de Goede, Mark Gross
  Cc: Michal Wilczynski, platform-driver-x86, linux-acpi, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/platform/x86/toshiba_acpi.c | 129 +++++++++++++++-------------
 1 file changed, 68 insertions(+), 61 deletions(-)

diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index b34984bbee33..28d78b018547 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -3186,10 +3186,74 @@ static void print_supported_features(struct toshiba_acpi_dev *dev)
 	pr_cont("\n");
 }
 
+static void toshiba_acpi_notify(acpi_handle handle, u32 event, void *data)
+{
+	struct acpi_device *acpi_dev = data;
+	struct toshiba_acpi_dev *dev;
+
+	dev = acpi_driver_data(acpi_dev);
+
+	switch (event) {
+	case 0x80: /* Hotkeys and some system events */
+		/*
+		 * Machines with this WMI GUID aren't supported due to bugs in
+		 * their AML.
+		 *
+		 * Return silently to avoid triggering a netlink event.
+		 */
+		if (wmi_has_guid(TOSHIBA_WMI_EVENT_GUID))
+			return;
+		toshiba_acpi_process_hotkeys(dev);
+		break;
+	case 0x81: /* Dock events */
+	case 0x82:
+	case 0x83:
+		pr_info("Dock event received %x\n", event);
+		break;
+	case 0x88: /* Thermal events */
+		pr_info("Thermal event received\n");
+		break;
+	case 0x8f: /* LID closed */
+	case 0x90: /* LID is closed and Dock has been ejected */
+		break;
+	case 0x8c: /* SATA power events */
+	case 0x8b:
+		pr_info("SATA power event received %x\n", event);
+		break;
+	case 0x92: /* Keyboard backlight mode changed */
+		dev->kbd_event_generated = true;
+		/* Update sysfs entries */
+		if (sysfs_update_group(&acpi_dev->dev.kobj,
+				       &toshiba_attr_group))
+			pr_err("Unable to update sysfs entries\n");
+		/* Notify LED subsystem about keyboard backlight change */
+		if (dev->kbd_type == 2 && dev->kbd_mode != SCI_KBD_MODE_AUTO)
+			led_classdev_notify_brightness_hw_changed(&dev->kbd_led,
+					(dev->kbd_mode == SCI_KBD_MODE_ON) ?
+					LED_FULL : LED_OFF);
+		break;
+	case 0x85: /* Unknown */
+	case 0x8d: /* Unknown */
+	case 0x8e: /* Unknown */
+	case 0x94: /* Unknown */
+	case 0x95: /* Unknown */
+	default:
+		pr_info("Unknown event received %x\n", event);
+		break;
+	}
+
+	acpi_bus_generate_netlink_event(acpi_dev->pnp.device_class,
+					dev_name(&acpi_dev->dev),
+					event, (event == 0x80) ?
+					dev->last_key_event : 0);
+}
+
 static void toshiba_acpi_remove(struct acpi_device *acpi_dev)
 {
 	struct toshiba_acpi_dev *dev = acpi_driver_data(acpi_dev);
 
+	acpi_device_remove_event_handler(acpi_dev, ACPI_ALL_NOTIFY, toshiba_acpi_notify);
+
 	misc_deregister(&dev->miscdev);
 
 	remove_toshiba_proc_entries(dev);
@@ -3473,6 +3537,10 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
 	if (dev->battery_charge_mode_supported)
 		battery_hook_register(&battery_hook);
 
+	ret = acpi_device_install_event_handler(acpi_dev, ACPI_ALL_NOTIFY, toshiba_acpi_notify);
+	if (ret)
+		goto error;
+
 	return 0;
 
 error:
@@ -3480,65 +3548,6 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
 	return ret;
 }
 
-static void toshiba_acpi_notify(struct acpi_device *acpi_dev, u32 event)
-{
-	struct toshiba_acpi_dev *dev = acpi_driver_data(acpi_dev);
-
-	switch (event) {
-	case 0x80: /* Hotkeys and some system events */
-		/*
-		 * Machines with this WMI GUID aren't supported due to bugs in
-		 * their AML.
-		 *
-		 * Return silently to avoid triggering a netlink event.
-		 */
-		if (wmi_has_guid(TOSHIBA_WMI_EVENT_GUID))
-			return;
-		toshiba_acpi_process_hotkeys(dev);
-		break;
-	case 0x81: /* Dock events */
-	case 0x82:
-	case 0x83:
-		pr_info("Dock event received %x\n", event);
-		break;
-	case 0x88: /* Thermal events */
-		pr_info("Thermal event received\n");
-		break;
-	case 0x8f: /* LID closed */
-	case 0x90: /* LID is closed and Dock has been ejected */
-		break;
-	case 0x8c: /* SATA power events */
-	case 0x8b:
-		pr_info("SATA power event received %x\n", event);
-		break;
-	case 0x92: /* Keyboard backlight mode changed */
-		dev->kbd_event_generated = true;
-		/* Update sysfs entries */
-		if (sysfs_update_group(&acpi_dev->dev.kobj,
-				       &toshiba_attr_group))
-			pr_err("Unable to update sysfs entries\n");
-		/* Notify LED subsystem about keyboard backlight change */
-		if (dev->kbd_type == 2 && dev->kbd_mode != SCI_KBD_MODE_AUTO)
-			led_classdev_notify_brightness_hw_changed(&dev->kbd_led,
-					(dev->kbd_mode == SCI_KBD_MODE_ON) ?
-					LED_FULL : LED_OFF);
-		break;
-	case 0x85: /* Unknown */
-	case 0x8d: /* Unknown */
-	case 0x8e: /* Unknown */
-	case 0x94: /* Unknown */
-	case 0x95: /* Unknown */
-	default:
-		pr_info("Unknown event received %x\n", event);
-		break;
-	}
-
-	acpi_bus_generate_netlink_event(acpi_dev->pnp.device_class,
-					dev_name(&acpi_dev->dev),
-					event, (event == 0x80) ?
-					dev->last_key_event : 0);
-}
-
 #ifdef CONFIG_PM_SLEEP
 static int toshiba_acpi_suspend(struct device *device)
 {
@@ -3583,11 +3592,9 @@ static struct acpi_driver toshiba_acpi_driver = {
 	.name	= "Toshiba ACPI driver",
 	.owner	= THIS_MODULE,
 	.ids	= toshiba_device_ids,
-	.flags	= ACPI_DRIVER_ALL_NOTIFY_EVENTS,
 	.ops	= {
 		.add		= toshiba_acpi_add,
 		.remove		= toshiba_acpi_remove,
-		.notify		= toshiba_acpi_notify,
 	},
 	.drv.pm	= &toshiba_acpi_pm,
 };
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 27/35] platform/x86/toshiba_bluetooth: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (23 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 26/35] platform/x86/toshiba_acpi: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 28/35] platform/x86/toshiba_haps: " Michal Wilczynski
                   ` (4 subsequent siblings)
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Azael Avalos, Hans de Goede, Mark Gross
  Cc: Michal Wilczynski, platform-driver-x86, linux-acpi, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/platform/x86/toshiba_bluetooth.c | 30 +++++++++++++++++-------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/platform/x86/toshiba_bluetooth.c b/drivers/platform/x86/toshiba_bluetooth.c
index d8f81962a240..a717bd5fcc88 100644
--- a/drivers/platform/x86/toshiba_bluetooth.c
+++ b/drivers/platform/x86/toshiba_bluetooth.c
@@ -37,7 +37,7 @@ struct toshiba_bluetooth_dev {
 
 static int toshiba_bt_rfkill_add(struct acpi_device *device);
 static void toshiba_bt_rfkill_remove(struct acpi_device *device);
-static void toshiba_bt_rfkill_notify(struct acpi_device *device, u32 event);
+static void toshiba_bt_rfkill_notify(acpi_handle handle, u32 event, void *data);
 
 static const struct acpi_device_id bt_device_ids[] = {
 	{ "TOS6205", 0},
@@ -57,7 +57,6 @@ static struct acpi_driver toshiba_bt_rfkill_driver = {
 	.ops =		{
 				.add =		toshiba_bt_rfkill_add,
 				.remove =	toshiba_bt_rfkill_remove,
-				.notify =	toshiba_bt_rfkill_notify,
 			},
 	.owner = 	THIS_MODULE,
 	.drv.pm =	&toshiba_bt_pm,
@@ -204,9 +203,12 @@ static const struct rfkill_ops rfk_ops = {
 };
 
 /* ACPI driver functions */
-static void toshiba_bt_rfkill_notify(struct acpi_device *device, u32 event)
+static void toshiba_bt_rfkill_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct toshiba_bluetooth_dev *bt_dev = acpi_driver_data(device);
+	struct toshiba_bluetooth_dev *bt_dev;
+	struct acpi_device *device = data;
+
+	bt_dev = acpi_driver_data(device);
 
 	if (toshiba_bluetooth_sync_status(bt_dev))
 		return;
@@ -263,8 +265,8 @@ static int toshiba_bt_rfkill_add(struct acpi_device *device)
 				   bt_dev);
 	if (!bt_dev->rfk) {
 		pr_err("Unable to allocate rfkill device\n");
-		kfree(bt_dev);
-		return -ENOMEM;
+		result = -ENOMEM;
+		goto fail_allocate;
 	}
 
 	rfkill_set_hw_state(bt_dev->rfk, !bt_dev->killswitch);
@@ -272,10 +274,20 @@ static int toshiba_bt_rfkill_add(struct acpi_device *device)
 	result = rfkill_register(bt_dev->rfk);
 	if (result) {
 		pr_err("Unable to register rfkill device\n");
-		rfkill_destroy(bt_dev->rfk);
-		kfree(bt_dev);
+		goto fail_register;
 	}
 
+	result = acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY,
+						   toshiba_bt_rfkill_notify);
+	if (result)
+		goto fail_register;
+
+	return 0;
+
+fail_register:
+	rfkill_destroy(bt_dev->rfk);
+fail_allocate:
+	kfree(bt_dev);
 	return result;
 }
 
@@ -283,6 +295,8 @@ static void toshiba_bt_rfkill_remove(struct acpi_device *device)
 {
 	struct toshiba_bluetooth_dev *bt_dev = acpi_driver_data(device);
 
+	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, toshiba_bt_rfkill_notify);
+
 	/* clean up */
 	if (bt_dev->rfk) {
 		rfkill_unregister(bt_dev->rfk);
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 28/35] platform/x86/toshiba_haps: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (24 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 27/35] platform/x86/toshiba_bluetooth: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 29/35] platform/x86/wireless-hotkey: " Michal Wilczynski
                   ` (3 subsequent siblings)
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Azael Avalos, Hans de Goede, Mark Gross
  Cc: Michal Wilczynski, platform-driver-x86, linux-acpi, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/platform/x86/toshiba_haps.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/toshiba_haps.c b/drivers/platform/x86/toshiba_haps.c
index 8c9f76286b08..39e4c49847a8 100644
--- a/drivers/platform/x86/toshiba_haps.c
+++ b/drivers/platform/x86/toshiba_haps.c
@@ -129,8 +129,10 @@ static const struct attribute_group haps_attr_group = {
 /*
  * ACPI stuff
  */
-static void toshiba_haps_notify(struct acpi_device *device, u32 event)
+static void toshiba_haps_notify(acpi_handle handle, u32 event, void *data)
 {
+	struct acpi_device *device = data;
+
 	pr_debug("Received event: 0x%x\n", event);
 
 	acpi_bus_generate_netlink_event(device->pnp.device_class,
@@ -140,6 +142,7 @@ static void toshiba_haps_notify(struct acpi_device *device, u32 event)
 
 static void toshiba_haps_remove(struct acpi_device *device)
 {
+	acpi_device_remove_event_handler(device, ACPI_ALL_NOTIFY, toshiba_haps_notify);
 	sysfs_remove_group(&device->dev.kobj, &haps_attr_group);
 
 	if (toshiba_haps)
@@ -203,7 +206,7 @@ static int toshiba_haps_add(struct acpi_device *acpi_dev)
 
 	toshiba_haps = haps;
 
-	return 0;
+	return acpi_device_install_event_handler(acpi_dev, ACPI_ALL_NOTIFY, toshiba_haps_notify);
 }
 
 #ifdef CONFIG_PM_SLEEP
@@ -253,11 +256,9 @@ static struct acpi_driver toshiba_haps_driver = {
 	.name = "Toshiba HAPS",
 	.owner = THIS_MODULE,
 	.ids = haps_device_ids,
-	.flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS,
 	.ops = {
 		.add =		toshiba_haps_add,
 		.remove =	toshiba_haps_remove,
-		.notify =	toshiba_haps_notify,
 	},
 	.drv.pm = &toshiba_haps_pm,
 };
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 29/35] platform/x86/wireless-hotkey: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (25 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 28/35] platform/x86/toshiba_haps: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 30/35] platform/x86/xo15-ebook: " Michal Wilczynski
                   ` (2 subsequent siblings)
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Hans de Goede, Mark Gross
  Cc: Michal Wilczynski, platform-driver-x86, linux-acpi, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/platform/x86/wireless-hotkey.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/platform/x86/wireless-hotkey.c b/drivers/platform/x86/wireless-hotkey.c
index 4422863f47bb..6554ef2e9c70 100644
--- a/drivers/platform/x86/wireless-hotkey.c
+++ b/drivers/platform/x86/wireless-hotkey.c
@@ -68,9 +68,12 @@ static void wireless_input_destroy(struct acpi_device *device)
 	kfree(button);
 }
 
-static void wl_notify(struct acpi_device *acpi_dev, u32 event)
+static void wl_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct wl_button *button = acpi_driver_data(acpi_dev);
+	struct acpi_device *acpi_dev = data;
+	struct wl_button *button;
+
+	button = acpi_driver_data(acpi_dev);
 
 	if (event != 0x80) {
 		pr_info("Received unknown event (0x%x)\n", event);
@@ -95,16 +98,24 @@ static int wl_add(struct acpi_device *device)
 	device->driver_data = button;
 
 	err = wireless_input_setup(device);
-	if (err) {
-		pr_err("Failed to setup wireless hotkeys\n");
-		kfree(button);
-	}
+	if (err)
+		goto fail;
 
+	err = acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY, wl_notify);
+	if (err)
+		goto fail;
+
+	return 0;
+
+fail:
+	pr_err("Failed to setup wireless hotkeys\n");
+	kfree(button);
 	return err;
 }
 
 static void wl_remove(struct acpi_device *device)
 {
+	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, wl_notify);
 	wireless_input_destroy(device);
 }
 
@@ -115,7 +126,6 @@ static struct acpi_driver wl_driver = {
 	.ops	= {
 		.add	= wl_add,
 		.remove	= wl_remove,
-		.notify	= wl_notify,
 	},
 };
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 30/35] platform/x86/xo15-ebook: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (26 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 29/35] platform/x86/wireless-hotkey: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 31/35] platform/x86/sony-laptop: " Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 32/35] virt/vmgenid: " Michal Wilczynski
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Hans de Goede, Mark Gross
  Cc: Michal Wilczynski, platform-driver-x86, linux-acpi, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/platform/x86/xo15-ebook.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/xo15-ebook.c b/drivers/platform/x86/xo15-ebook.c
index 391f7ea4431e..57d143e06d65 100644
--- a/drivers/platform/x86/xo15-ebook.c
+++ b/drivers/platform/x86/xo15-ebook.c
@@ -56,8 +56,9 @@ static int ebook_send_state(struct acpi_device *device)
 	return 0;
 }
 
-static void ebook_switch_notify(struct acpi_device *device, u32 event)
+static void ebook_switch_notify(acpi_handle handle, u32 event, void *data)
 {
+	struct acpi_device *device = data;
 	switch (event) {
 	case ACPI_FIXED_HARDWARE_EVENT:
 	case XO15_EBOOK_NOTIFY_STATUS:
@@ -134,6 +135,10 @@ static int ebook_switch_add(struct acpi_device *device)
 		device_set_wakeup_enable(&device->dev, true);
 	}
 
+	error = acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY, ebook_switch_notify);
+	if (error)
+		goto err_free_input;
+
 	return 0;
 
  err_free_input:
@@ -147,6 +152,7 @@ static void ebook_switch_remove(struct acpi_device *device)
 {
 	struct ebook_switch *button = acpi_driver_data(device);
 
+	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, ebook_switch_notify);
 	input_unregister_device(button->input);
 	kfree(button);
 }
@@ -158,7 +164,6 @@ static struct acpi_driver xo15_ebook_driver = {
 	.ops = {
 		.add = ebook_switch_add,
 		.remove = ebook_switch_remove,
-		.notify = ebook_switch_notify,
 	},
 	.drv.pm = &ebook_switch_pm,
 };
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 31/35] platform/x86/sony-laptop: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (27 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 30/35] platform/x86/xo15-ebook: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  2023-06-01 13:17 ` [PATCH v4 32/35] virt/vmgenid: " Michal Wilczynski
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Mattia Dongili, Hans de Goede, Mark Gross
  Cc: Michal Wilczynski, platform-driver-x86, linux-acpi, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/platform/x86/sony-laptop.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
index 9569f11dec8c..93e630d937a8 100644
--- a/drivers/platform/x86/sony-laptop.c
+++ b/drivers/platform/x86/sony-laptop.c
@@ -1178,7 +1178,7 @@ enum event_types {
 	KILLSWITCH,
 	GFX_SWITCH
 };
-static void sony_nc_notify(struct acpi_device *device, u32 event)
+static void sony_nc_notify(acpi_handle handle, u32 event, void *data)
 {
 	u32 real_ev = event;
 	u8 ev_type = 0;
@@ -3246,6 +3246,11 @@ static int sony_nc_add(struct acpi_device *device)
 		}
 	}
 
+	result = acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY,
+						   sony_nc_notify);
+	if (result)
+		goto out_sysfs;
+
 	pr_info("SNC setup done.\n");
 	return 0;
 
@@ -3272,6 +3277,7 @@ static void sony_nc_remove(struct acpi_device *device)
 {
 	struct sony_nc_value *item;
 
+	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, sony_nc_notify);
 	sony_nc_backlight_cleanup();
 
 	sony_nc_acpi_device = NULL;
@@ -3307,7 +3313,6 @@ static struct acpi_driver sony_nc_driver = {
 	.ops = {
 		.add = sony_nc_add,
 		.remove = sony_nc_remove,
-		.notify = sony_nc_notify,
 		},
 	.drv.pm = &sony_nc_pm,
 };
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v4 32/35] virt/vmgenid: Move handler installing logic to driver
  2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
                   ` (28 preceding siblings ...)
  2023-06-01 13:17 ` [PATCH v4 31/35] platform/x86/sony-laptop: " Michal Wilczynski
@ 2023-06-01 13:17 ` Michal Wilczynski
  29 siblings, 0 replies; 44+ messages in thread
From: Michal Wilczynski @ 2023-06-01 13:17 UTC (permalink / raw)
  To: Theodore Ts'o, Jason A. Donenfeld
  Cc: Michal Wilczynski, linux-acpi, platform-driver-x86, rafael

Currently logic for installing notifications from ACPI devices is
implemented using notify callback in struct acpi_driver. Preparations
are being made to replace acpi_driver with more generic struct
platform_driver, which doesn't contain notify callback. Furthermore
as of now handlers are being called indirectly through
acpi_notify_device(), which decreases performance.

Call acpi_device_install_event_handler() at the end of .add() callback.
Call acpi_device_remove_event_handler() at the beginning of .remove()
callback. Change arguments passed to the notify callback to match with
what's required by acpi_device_install_event_handler().

Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
---
 drivers/virt/vmgenid.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/virt/vmgenid.c b/drivers/virt/vmgenid.c
index a1c467a0e9f7..50e7f4a82f99 100644
--- a/drivers/virt/vmgenid.c
+++ b/drivers/virt/vmgenid.c
@@ -21,6 +21,21 @@ struct vmgenid_state {
 	u8 this_id[VMGENID_SIZE];
 };
 
+static void vmgenid_notify(acpi_handle handle, u32 event, void *data)
+{
+	struct acpi_device *device = data;
+	struct vmgenid_state *state;
+	u8 old_id[VMGENID_SIZE];
+
+	state = acpi_driver_data(device);
+
+	memcpy(old_id, state->this_id, sizeof(old_id));
+	memcpy(state->this_id, state->next_id, sizeof(state->this_id));
+	if (!memcmp(old_id, state->this_id, sizeof(old_id)))
+		return;
+	add_vmfork_randomness(state->this_id, sizeof(state->this_id));
+}
+
 static int vmgenid_add(struct acpi_device *device)
 {
 	struct acpi_buffer parsed = { ACPI_ALLOCATE_BUFFER };
@@ -60,21 +75,16 @@ static int vmgenid_add(struct acpi_device *device)
 
 	device->driver_data = state;
 
+	ret = acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY, vmgenid_notify);
+
 out:
 	ACPI_FREE(parsed.pointer);
 	return ret;
 }
 
-static void vmgenid_notify(struct acpi_device *device, u32 event)
+static void vmgenid_remove(struct acpi_device *device)
 {
-	struct vmgenid_state *state = acpi_driver_data(device);
-	u8 old_id[VMGENID_SIZE];
-
-	memcpy(old_id, state->this_id, sizeof(old_id));
-	memcpy(state->this_id, state->next_id, sizeof(state->this_id));
-	if (!memcmp(old_id, state->this_id, sizeof(old_id)))
-		return;
-	add_vmfork_randomness(state->this_id, sizeof(state->this_id));
+	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, vmgenid_notify);
 }
 
 static const struct acpi_device_id vmgenid_ids[] = {
@@ -89,7 +99,7 @@ static struct acpi_driver vmgenid_driver = {
 	.owner = THIS_MODULE,
 	.ops = {
 		.add = vmgenid_add,
-		.notify = vmgenid_notify
+		.remove = vmgenid_remove,
 	}
 };
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* Re: [PATCH v4 10/35] hwmon/acpi_power_meter: Move handler installing logic to driver
  2023-06-01 13:17 ` [PATCH v4 10/35] hwmon/acpi_power_meter: " Michal Wilczynski
@ 2023-06-01 13:51   ` Guenter Roeck
  0 siblings, 0 replies; 44+ messages in thread
From: Guenter Roeck @ 2023-06-01 13:51 UTC (permalink / raw)
  To: Michal Wilczynski, Jean Delvare
  Cc: linux-hwmon, linux-acpi, platform-driver-x86, rafael

On 6/1/23 06:17, Michal Wilczynski wrote:
> Currently logic for installing notifications from ACPI devices is
> implemented using notify callback in struct acpi_driver. Preparations
> are being made to replace acpi_driver with more generic struct
> platform_driver, which doesn't contain notify callback. Furthermore
> as of now handlers are being called indirectly through
> acpi_notify_device(), which decreases performance.
> 
> Call acpi_device_install_event_handler() at the end of .add() callback.
> Call acpi_device_remove_event_handler() at the beginning of .remove()
> callback. Change arguments passed to the notify callback to match with
> what's required by acpi_device_install_event_handler().
> 
> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>

Acked-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/hwmon/acpi_power_meter.c | 15 ++++++++++-----
>   1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
> index fa28d447f0df..7410ee8693ba 100644
> --- a/drivers/hwmon/acpi_power_meter.c
> +++ b/drivers/hwmon/acpi_power_meter.c
> @@ -817,9 +817,10 @@ static int read_capabilities(struct acpi_power_meter_resource *resource)
>   }
>   
>   /* Handle ACPI event notifications */
> -static void acpi_power_meter_notify(struct acpi_device *device, u32 event)
> +static void acpi_power_meter_notify(acpi_handle handle, u32 event, void *data)
>   {
>   	struct acpi_power_meter_resource *resource;
> +	struct acpi_device *device = data;
>   	int res;
>   
>   	if (!device || !acpi_driver_data(device))
> @@ -897,8 +898,12 @@ static int acpi_power_meter_add(struct acpi_device *device)
>   		goto exit_remove;
>   	}
>   
> -	res = 0;
> -	goto exit;
> +	res = acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY,
> +						acpi_power_meter_notify);
> +	if (res)
> +		goto exit_remove;
> +
> +	return 0;
>   
>   exit_remove:
>   	remove_attrs(resource);
> @@ -906,7 +911,7 @@ static int acpi_power_meter_add(struct acpi_device *device)
>   	free_capabilities(resource);
>   exit_free:
>   	kfree(resource);
> -exit:
> +
>   	return res;
>   }
>   
> @@ -917,6 +922,7 @@ static void acpi_power_meter_remove(struct acpi_device *device)
>   	if (!device || !acpi_driver_data(device))
>   		return;
>   
> +	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_power_meter_notify);
>   	resource = acpi_driver_data(device);
>   	hwmon_device_unregister(resource->hwmon_dev);
>   
> @@ -953,7 +959,6 @@ static struct acpi_driver acpi_power_meter_driver = {
>   	.ops = {
>   		.add = acpi_power_meter_add,
>   		.remove = acpi_power_meter_remove,
> -		.notify = acpi_power_meter_notify,
>   		},
>   	.drv.pm = pm_sleep_ptr(&acpi_power_meter_pm),
>   };


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v4 18/35] platform/x86/classmate-laptop: Move handler installing logic to driver
  2023-06-01 13:17 ` [PATCH v4 18/35] platform/x86/classmate-laptop: " Michal Wilczynski
@ 2023-06-02 10:29   ` Thadeu Lima de Souza Cascardo
  2023-06-02 13:25     ` Wilczynski, Michal
  0 siblings, 1 reply; 44+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2023-06-02 10:29 UTC (permalink / raw)
  To: Michal Wilczynski
  Cc: Daniel Oliveira Nascimento, Hans de Goede, Mark Gross,
	platform-driver-x86, linux-acpi, rafael

On Thu, Jun 01, 2023 at 03:17:21PM +0200, Michal Wilczynski wrote:
> Currently logic for installing notifications from ACPI devices is
> implemented using notify callback in struct acpi_driver. Preparations
> are being made to replace acpi_driver with more generic struct
> platform_driver, which doesn't contain notify callback. Furthermore
> as of now handlers are being called indirectly through
> acpi_notify_device(), which decreases performance.
> 
> Call acpi_device_install_event_handler() at the end of .add() callback.
> Call acpi_device_remove_event_handler() at the beginning of .remove()
> callback. Change arguments passed to the notify callback to match with
> what's required by acpi_device_install_event_handler().
> 
> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
> ---
>  drivers/platform/x86/classmate-laptop.c | 53 +++++++++++++++++++------
>  1 file changed, 41 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/platform/x86/classmate-laptop.c b/drivers/platform/x86/classmate-laptop.c
> index 2edaea2492df..2d36abf5ecfe 100644
> --- a/drivers/platform/x86/classmate-laptop.c
> +++ b/drivers/platform/x86/classmate-laptop.c
> @@ -180,8 +180,9 @@ static acpi_status cmpc_get_accel_v4(acpi_handle handle,
>  	return status;
>  }
>  
> -static void cmpc_accel_handler_v4(struct acpi_device *dev, u32 event)
> +static void cmpc_accel_handler_v4(acpi_handle handle, u32 event, void *data)
>  {
> +	struct acpi_device *dev = data;
>  	if (event == 0x81) {
>  		int16_t x, y, z;
>  		acpi_status status;
> @@ -407,6 +408,11 @@ static int cmpc_accel_add_v4(struct acpi_device *acpi)
>  	inputdev = dev_get_drvdata(&acpi->dev);
>  	dev_set_drvdata(&inputdev->dev, accel);
>  
> +	error = acpi_device_install_event_handler(acpi, ACPI_DEVICE_NOTIFY,
> +						  cmpc_accel_handler_v4);
> +	if (error)
> +		goto failed_input;
> +

In all cases, acpi_device_install_event_handler is being called after
cmpc_add_acpi_notify_device, which allocates and registers an input
device.

You should cleanup in case acpi_device_install_event_handler fails and
call cmpc_remove_acpi_notify_device.

Cascardo.

>  	return 0;
>  
>  failed_input:
> @@ -420,6 +426,7 @@ static int cmpc_accel_add_v4(struct acpi_device *acpi)
>  
>  static void cmpc_accel_remove_v4(struct acpi_device *acpi)
>  {
> +	acpi_device_remove_event_handler(acpi, ACPI_DEVICE_NOTIFY, cmpc_accel_handler_v4);
>  	device_remove_file(&acpi->dev, &cmpc_accel_sensitivity_attr_v4);
>  	device_remove_file(&acpi->dev, &cmpc_accel_g_select_attr_v4);
>  	cmpc_remove_acpi_notify_device(acpi);
> @@ -441,7 +448,6 @@ static struct acpi_driver cmpc_accel_acpi_driver_v4 = {
>  	.ops = {
>  		.add = cmpc_accel_add_v4,
>  		.remove = cmpc_accel_remove_v4,
> -		.notify = cmpc_accel_handler_v4,
>  	},
>  	.drv.pm = &cmpc_accel_pm,
>  };
> @@ -523,8 +529,10 @@ static acpi_status cmpc_get_accel(acpi_handle handle,
>  	return status;
>  }
>  
> -static void cmpc_accel_handler(struct acpi_device *dev, u32 event)
> +static void cmpc_accel_handler(acpi_handle handle, u32 event, void *data)
>  {
> +	struct acpi_device *dev = data;
> +
>  	if (event == 0x81) {
>  		unsigned char x, y, z;
>  		acpi_status status;
> @@ -639,6 +647,11 @@ static int cmpc_accel_add(struct acpi_device *acpi)
>  	inputdev = dev_get_drvdata(&acpi->dev);
>  	dev_set_drvdata(&inputdev->dev, accel);
>  
> +	error = acpi_device_install_event_handler(acpi, ACPI_DEVICE_NOTIFY,
> +						  cmpc_accel_handler);
> +	if (error)
> +		goto failed_input;
> +
>  	return 0;
>  
>  failed_input:
> @@ -650,6 +663,7 @@ static int cmpc_accel_add(struct acpi_device *acpi)
>  
>  static void cmpc_accel_remove(struct acpi_device *acpi)
>  {
> +	acpi_device_remove_event_handler(acpi, ACPI_DEVICE_NOTIFY, cmpc_accel_handler);
>  	device_remove_file(&acpi->dev, &cmpc_accel_sensitivity_attr);
>  	cmpc_remove_acpi_notify_device(acpi);
>  }
> @@ -667,7 +681,6 @@ static struct acpi_driver cmpc_accel_acpi_driver = {
>  	.ops = {
>  		.add = cmpc_accel_add,
>  		.remove = cmpc_accel_remove,
> -		.notify = cmpc_accel_handler,
>  	}
>  };
>  
> @@ -693,8 +706,9 @@ static acpi_status cmpc_get_tablet(acpi_handle handle,
>  	return status;
>  }
>  
> -static void cmpc_tablet_handler(struct acpi_device *dev, u32 event)
> +static void cmpc_tablet_handler(acpi_handle handle, u32 event, void *data)
>  {
> +	struct acpi_device *dev = data;
>  	unsigned long long val = 0;
>  	struct input_dev *inputdev = dev_get_drvdata(&dev->dev);
>  
> @@ -723,12 +737,20 @@ static void cmpc_tablet_idev_init(struct input_dev *inputdev)
>  
>  static int cmpc_tablet_add(struct acpi_device *acpi)
>  {
> -	return cmpc_add_acpi_notify_device(acpi, "cmpc_tablet",
> -					   cmpc_tablet_idev_init);
> +	int ret;
> +
> +	ret = cmpc_add_acpi_notify_device(acpi, "cmpc_tablet",
> +					  cmpc_tablet_idev_init);
> +	if (ret)
> +		return ret;
> +
> +	return acpi_device_install_event_handler(acpi, ACPI_DEVICE_NOTIFY,
> +						 cmpc_tablet_handler);
>  }
>  
>  static void cmpc_tablet_remove(struct acpi_device *acpi)
>  {
> +	acpi_device_remove_event_handler(acpi, ACPI_DEVICE_NOTIFY, cmpc_tablet_handler);
>  	cmpc_remove_acpi_notify_device(acpi);
>  }
>  
> @@ -761,7 +783,6 @@ static struct acpi_driver cmpc_tablet_acpi_driver = {
>  	.ops = {
>  		.add = cmpc_tablet_add,
>  		.remove = cmpc_tablet_remove,
> -		.notify = cmpc_tablet_handler,
>  	},
>  	.drv.pm = &cmpc_tablet_pm,
>  };
> @@ -1026,8 +1047,9 @@ static int cmpc_keys_codes[] = {
>  	KEY_MAX
>  };
>  
> -static void cmpc_keys_handler(struct acpi_device *dev, u32 event)
> +static void cmpc_keys_handler(acpi_handle handle, u32 event, void *data)
>  {
> +	struct acpi_device *dev = data;
>  	struct input_dev *inputdev;
>  	int code = KEY_MAX;
>  
> @@ -1049,12 +1071,20 @@ static void cmpc_keys_idev_init(struct input_dev *inputdev)
>  
>  static int cmpc_keys_add(struct acpi_device *acpi)
>  {
> -	return cmpc_add_acpi_notify_device(acpi, "cmpc_keys",
> -					   cmpc_keys_idev_init);
> +	int error;
> +
> +	error = cmpc_add_acpi_notify_device(acpi, "cmpc_keys",
> +					    cmpc_keys_idev_init);
> +	if (error)
> +		return error;
> +
> +	return acpi_device_install_event_handler(acpi, ACPI_DEVICE_NOTIFY,
> +						 cmpc_keys_handler);
>  }
>  
>  static void cmpc_keys_remove(struct acpi_device *acpi)
>  {
> +	acpi_device_remove_event_handler(acpi, ACPI_DEVICE_NOTIFY, cmpc_keys_handler);
>  	cmpc_remove_acpi_notify_device(acpi);
>  }
>  
> @@ -1071,7 +1101,6 @@ static struct acpi_driver cmpc_keys_acpi_driver = {
>  	.ops = {
>  		.add = cmpc_keys_add,
>  		.remove = cmpc_keys_remove,
> -		.notify = cmpc_keys_handler,
>  	}
>  };
>  
> -- 
> 2.40.1
> 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v4 17/35] platform/x86/asus-wireless: Move handler installing logic to driver
  2023-06-01 13:17 ` [PATCH v4 17/35] platform/x86/asus-wireless: " Michal Wilczynski
@ 2023-06-02 13:09   ` Ilpo Järvinen
  2023-06-02 13:16     ` Wilczynski, Michal
  0 siblings, 1 reply; 44+ messages in thread
From: Ilpo Järvinen @ 2023-06-02 13:09 UTC (permalink / raw)
  To: Michal Wilczynski
  Cc: Corentin Chary, João Paulo Rechi Vita, Hans de Goede,
	Mark Gross, platform-driver-x86, acpi4asus-user, linux-acpi,
	rafael

On Thu, 1 Jun 2023, Michal Wilczynski wrote:

> Currently logic for installing notifications from ACPI devices is
> implemented using notify callback in struct acpi_driver. Preparations
> are being made to replace acpi_driver with more generic struct
> platform_driver, which doesn't contain notify callback. Furthermore
> as of now handlers are being called indirectly through
> acpi_notify_device(), which decreases performance.
> 
> Call acpi_device_install_event_handler() at the end of .add() callback.
> Call acpi_device_remove_event_handler() at the beginning of .remove()
> callback. Change arguments passed to the notify callback to match with
> what's required by acpi_device_install_event_handler().
> 
> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
> ---
>  drivers/platform/x86/asus-wireless.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c
> index abf01e00b799..6544e3419ae4 100644
> --- a/drivers/platform/x86/asus-wireless.c
> +++ b/drivers/platform/x86/asus-wireless.c
> @@ -108,19 +108,22 @@ static void led_state_set(struct led_classdev *led, enum led_brightness value)
>  	queue_work(data->wq, &data->led_work);
>  }
>  
> -static void asus_wireless_notify(struct acpi_device *adev, u32 event)
> +static void asus_wireless_notify(acpi_handle handle, u32 event, void *data)
>  {
> -	struct asus_wireless_data *data = acpi_driver_data(adev);
> +	struct asus_wireless_data *w_data;
> +	struct acpi_device *adev = data;
> +
> +	w_data = acpi_driver_data(adev);
>  
>  	dev_dbg(&adev->dev, "event=%#x\n", event);
>  	if (event != 0x88) {
>  		dev_notice(&adev->dev, "Unknown ASHS event: %#x\n", event);
>  		return;
>  	}
> -	input_report_key(data->idev, KEY_RFKILL, 1);
> -	input_sync(data->idev);
> -	input_report_key(data->idev, KEY_RFKILL, 0);
> -	input_sync(data->idev);
> +	input_report_key(w_data->idev, KEY_RFKILL, 1);
> +	input_sync(w_data->idev);
> +	input_report_key(w_data->idev, KEY_RFKILL, 0);
> +	input_sync(w_data->idev);
>  }
>  
>  static int asus_wireless_add(struct acpi_device *adev)
> @@ -169,16 +172,20 @@ static int asus_wireless_add(struct acpi_device *adev)
>  	data->led.max_brightness = 1;
>  	data->led.default_trigger = "rfkill-none";
>  	err = devm_led_classdev_register(&adev->dev, &data->led);
> -	if (err)
> +	if (err) {
>  		destroy_workqueue(data->wq);
> +		return err;
> +	}
>  
> -	return err;
> +	return acpi_device_install_event_handler(adev, ACPI_DEVICE_NOTIFY, asus_wireless_notify);

So if acpi_device_install_event_handler() returns error, should you 
rollback something here like the error branch above? If that's the case, 
use goto to rollback as you'll have two places calling destroy_workqueue() 
already.

-- 
 i.


>  }
>  
>  static void asus_wireless_remove(struct acpi_device *adev)
>  {
>  	struct asus_wireless_data *data = acpi_driver_data(adev);
>  
> +	acpi_device_remove_event_handler(adev, ACPI_DEVICE_NOTIFY, asus_wireless_notify);
> +
>  	if (data->wq) {
>  		devm_led_classdev_unregister(&adev->dev, &data->led);
>  		destroy_workqueue(data->wq);
> @@ -192,7 +199,6 @@ static struct acpi_driver asus_wireless_driver = {
>  	.ops = {
>  		.add = asus_wireless_add,
>  		.remove = asus_wireless_remove,
> -		.notify = asus_wireless_notify,
>  	},
>  };
>  module_acpi_driver(asus_wireless_driver);
> 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v4 17/35] platform/x86/asus-wireless: Move handler installing logic to driver
  2023-06-02 13:09   ` Ilpo Järvinen
@ 2023-06-02 13:16     ` Wilczynski, Michal
  0 siblings, 0 replies; 44+ messages in thread
From: Wilczynski, Michal @ 2023-06-02 13:16 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: Corentin Chary, João Paulo Rechi Vita, Hans de Goede,
	Mark Gross, platform-driver-x86, acpi4asus-user, linux-acpi,
	rafael



On 6/2/2023 3:09 PM, Ilpo Järvinen wrote:
> On Thu, 1 Jun 2023, Michal Wilczynski wrote:
>
>> Currently logic for installing notifications from ACPI devices is
>> implemented using notify callback in struct acpi_driver. Preparations
>> are being made to replace acpi_driver with more generic struct
>> platform_driver, which doesn't contain notify callback. Furthermore
>> as of now handlers are being called indirectly through
>> acpi_notify_device(), which decreases performance.
>>
>> Call acpi_device_install_event_handler() at the end of .add() callback.
>> Call acpi_device_remove_event_handler() at the beginning of .remove()
>> callback. Change arguments passed to the notify callback to match with
>> what's required by acpi_device_install_event_handler().
>>
>> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
>> ---
>>  drivers/platform/x86/asus-wireless.c | 24 +++++++++++++++---------
>>  1 file changed, 15 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c
>> index abf01e00b799..6544e3419ae4 100644
>> --- a/drivers/platform/x86/asus-wireless.c
>> +++ b/drivers/platform/x86/asus-wireless.c
>> @@ -108,19 +108,22 @@ static void led_state_set(struct led_classdev *led, enum led_brightness value)
>>  	queue_work(data->wq, &data->led_work);
>>  }
>>  
>> -static void asus_wireless_notify(struct acpi_device *adev, u32 event)
>> +static void asus_wireless_notify(acpi_handle handle, u32 event, void *data)
>>  {
>> -	struct asus_wireless_data *data = acpi_driver_data(adev);
>> +	struct asus_wireless_data *w_data;
>> +	struct acpi_device *adev = data;
>> +
>> +	w_data = acpi_driver_data(adev);
>>  
>>  	dev_dbg(&adev->dev, "event=%#x\n", event);
>>  	if (event != 0x88) {
>>  		dev_notice(&adev->dev, "Unknown ASHS event: %#x\n", event);
>>  		return;
>>  	}
>> -	input_report_key(data->idev, KEY_RFKILL, 1);
>> -	input_sync(data->idev);
>> -	input_report_key(data->idev, KEY_RFKILL, 0);
>> -	input_sync(data->idev);
>> +	input_report_key(w_data->idev, KEY_RFKILL, 1);
>> +	input_sync(w_data->idev);
>> +	input_report_key(w_data->idev, KEY_RFKILL, 0);
>> +	input_sync(w_data->idev);
>>  }
>>  
>>  static int asus_wireless_add(struct acpi_device *adev)
>> @@ -169,16 +172,20 @@ static int asus_wireless_add(struct acpi_device *adev)
>>  	data->led.max_brightness = 1;
>>  	data->led.default_trigger = "rfkill-none";
>>  	err = devm_led_classdev_register(&adev->dev, &data->led);
>> -	if (err)
>> +	if (err) {
>>  		destroy_workqueue(data->wq);
>> +		return err;
>> +	}
>>  
>> -	return err;
>> +	return acpi_device_install_event_handler(adev, ACPI_DEVICE_NOTIFY, asus_wireless_notify);
> So if acpi_device_install_event_handler() returns error, should you 
> rollback something here like the error branch above? If that's the case, 
> use goto to rollback as you'll have two places calling destroy_workqueue() 
> already.

Oh yeah, this error path is leaking workqueue now, will fix as suggested,

Thanks !
Michał

>


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v4 19/35] platform/x86/dell/dell-rbtn: Move handler installing logic to driver
  2023-06-01 13:17 ` [PATCH v4 19/35] platform/x86/dell/dell-rbtn: " Michal Wilczynski
@ 2023-06-02 13:20   ` Ilpo Järvinen
  2023-06-02 13:41     ` Wilczynski, Michal
  0 siblings, 1 reply; 44+ messages in thread
From: Ilpo Järvinen @ 2023-06-02 13:20 UTC (permalink / raw)
  To: Michal Wilczynski
  Cc: Pali Rohár, Hans de Goede, Mark Gross, platform-driver-x86,
	linux-acpi, rafael

On Thu, 1 Jun 2023, Michal Wilczynski wrote:

> Currently logic for installing notifications from ACPI devices is
> implemented using notify callback in struct acpi_driver. Preparations
> are being made to replace acpi_driver with more generic struct
> platform_driver, which doesn't contain notify callback. Furthermore
> as of now handlers are being called indirectly through
> acpi_notify_device(), which decreases performance.
> 
> Call acpi_device_install_event_handler() at the end of .add() callback.
> Call acpi_device_remove_event_handler() at the beginning of .remove()
> callback. Change arguments passed to the notify callback to match with
> what's required by acpi_device_install_event_handler().
> 
> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
> ---
>  drivers/platform/x86/dell/dell-rbtn.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/dell-rbtn.c b/drivers/platform/x86/dell/dell-rbtn.c
> index aa0e6c907494..4dcad59eb035 100644
> --- a/drivers/platform/x86/dell/dell-rbtn.c
> +++ b/drivers/platform/x86/dell/dell-rbtn.c
> @@ -207,7 +207,7 @@ static void rbtn_input_event(struct rbtn_data *rbtn_data)
>  
>  static int rbtn_add(struct acpi_device *device);
>  static void rbtn_remove(struct acpi_device *device);
> -static void rbtn_notify(struct acpi_device *device, u32 event);
> +static void rbtn_notify(acpi_handle handle, u32 event, void *data);
>  
>  static const struct acpi_device_id rbtn_ids[] = {
>  	{ "DELRBTN", 0 },
> @@ -293,7 +293,6 @@ static struct acpi_driver rbtn_driver = {
>  	.ops = {
>  		.add = rbtn_add,
>  		.remove = rbtn_remove,
> -		.notify = rbtn_notify,
>  	},
>  	.owner = THIS_MODULE,
>  };
> @@ -422,7 +421,10 @@ static int rbtn_add(struct acpi_device *device)
>  		ret = -EINVAL;
>  	}
>  
> -	return ret;
> +	if (ret)
> +		return ret;
> +
> +	return acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY, rbtn_notify);

What about the other things that are done in rbtn_remove(), should you 
rollback more?

I suspect there's a pre-existing lack of rbtn_acquire(device, false); 
here to begin with.

-- 
 i.


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v4 20/35] platform/x86/eeepc-laptop: Move handler installing logic to driver
  2023-06-01 13:17 ` [PATCH v4 20/35] platform/x86/eeepc-laptop: " Michal Wilczynski
@ 2023-06-02 13:24   ` Ilpo Järvinen
  0 siblings, 0 replies; 44+ messages in thread
From: Ilpo Järvinen @ 2023-06-02 13:24 UTC (permalink / raw)
  To: Michal Wilczynski
  Cc: Corentin Chary, Hans de Goede, Mark Gross, acpi4asus-user,
	platform-driver-x86, linux-acpi, rafael

On Thu, 1 Jun 2023, Michal Wilczynski wrote:

> Currently logic for installing notifications from ACPI devices is
> implemented using notify callback in struct acpi_driver. Preparations
> are being made to replace acpi_driver with more generic struct
> platform_driver, which doesn't contain notify callback. Furthermore
> as of now handlers are being called indirectly through
> acpi_notify_device(), which decreases performance.
> 
> Call acpi_device_install_event_handler() at the end of .add() callback.
> Call acpi_device_remove_event_handler() at the beginning of .remove()
> callback. Change arguments passed to the notify callback to match with
> what's required by acpi_device_install_event_handler().
> 
> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
> ---
>  drivers/platform/x86/eeepc-laptop.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c
> index 62b71e8e3567..bd6ada963d88 100644
> --- a/drivers/platform/x86/eeepc-laptop.c
> +++ b/drivers/platform/x86/eeepc-laptop.c
> @@ -1204,12 +1204,15 @@ static void eeepc_input_notify(struct eeepc_laptop *eeepc, int event)
>  		pr_info("Unknown key %x pressed\n", event);
>  }
>  
> -static void eeepc_acpi_notify(struct acpi_device *device, u32 event)
> +static void eeepc_acpi_notify(acpi_handle handle, u32 event, void *data)
>  {
> -	struct eeepc_laptop *eeepc = acpi_driver_data(device);
>  	int old_brightness, new_brightness;
> +	struct acpi_device *device = data;
> +	struct eeepc_laptop *eeepc;
>  	u16 count;
>  
> +	eeepc = acpi_driver_data(device);
> +
>  	if (event > ACPI_MAX_SYS_NOTIFY)
>  		return;
>  	count = eeepc->event_count[event % 128]++;
> @@ -1423,6 +1426,11 @@ static int eeepc_acpi_add(struct acpi_device *device)
>  		goto fail_rfkill;
>  
>  	eeepc_device_present = true;
> +	result = acpi_device_install_event_handler(device, ACPI_ALL_NOTIFY,
> +						   eeepc_acpi_notify);
> +	if (result)
> +		goto fail_rfkill;

This too is incorrectly rolled back. You shouldn't just copy the previous 
goto label like this but think what _more_ should be rolled back if that 
preceeding call succeeded. Usually, the remove function gives you good 
hint on what additional things should be rolled back. In here it looks 
like eeepc_rfkill_exit() would be needed too.

> +
>  	return 0;
>  
>  fail_rfkill:
> @@ -1444,6 +1452,8 @@ static void eeepc_acpi_remove(struct acpi_device *device)
>  {
>  	struct eeepc_laptop *eeepc = acpi_driver_data(device);
>  
> +	acpi_device_remove_event_handler(device, ACPI_ALL_NOTIFY,
> +					 eeepc_acpi_notify);
>  	eeepc_backlight_exit(eeepc);
>  	eeepc_rfkill_exit(eeepc);
>  	eeepc_input_exit(eeepc);


-- 
 i.


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v4 18/35] platform/x86/classmate-laptop: Move handler installing logic to driver
  2023-06-02 10:29   ` Thadeu Lima de Souza Cascardo
@ 2023-06-02 13:25     ` Wilczynski, Michal
  0 siblings, 0 replies; 44+ messages in thread
From: Wilczynski, Michal @ 2023-06-02 13:25 UTC (permalink / raw)
  To: Thadeu Lima de Souza Cascardo
  Cc: Daniel Oliveira Nascimento, Hans de Goede, Mark Gross,
	platform-driver-x86, linux-acpi, rafael



On 6/2/2023 12:29 PM, Thadeu Lima de Souza Cascardo wrote:
> On Thu, Jun 01, 2023 at 03:17:21PM +0200, Michal Wilczynski wrote:
>> Currently logic for installing notifications from ACPI devices is
>> implemented using notify callback in struct acpi_driver. Preparations
>> are being made to replace acpi_driver with more generic struct
>> platform_driver, which doesn't contain notify callback. Furthermore
>> as of now handlers are being called indirectly through
>> acpi_notify_device(), which decreases performance.
>>
>> Call acpi_device_install_event_handler() at the end of .add() callback.
>> Call acpi_device_remove_event_handler() at the beginning of .remove()
>> callback. Change arguments passed to the notify callback to match with
>> what's required by acpi_device_install_event_handler().
>>
>> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
>> ---
>>  drivers/platform/x86/classmate-laptop.c | 53 +++++++++++++++++++------
>>  1 file changed, 41 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/platform/x86/classmate-laptop.c b/drivers/platform/x86/classmate-laptop.c
>> index 2edaea2492df..2d36abf5ecfe 100644
>> --- a/drivers/platform/x86/classmate-laptop.c
>> +++ b/drivers/platform/x86/classmate-laptop.c
>> @@ -180,8 +180,9 @@ static acpi_status cmpc_get_accel_v4(acpi_handle handle,
>>  	return status;
>>  }
>>  
>> -static void cmpc_accel_handler_v4(struct acpi_device *dev, u32 event)
>> +static void cmpc_accel_handler_v4(acpi_handle handle, u32 event, void *data)
>>  {
>> +	struct acpi_device *dev = data;
>>  	if (event == 0x81) {
>>  		int16_t x, y, z;
>>  		acpi_status status;
>> @@ -407,6 +408,11 @@ static int cmpc_accel_add_v4(struct acpi_device *acpi)
>>  	inputdev = dev_get_drvdata(&acpi->dev);
>>  	dev_set_drvdata(&inputdev->dev, accel);
>>  
>> +	error = acpi_device_install_event_handler(acpi, ACPI_DEVICE_NOTIFY,
>> +						  cmpc_accel_handler_v4);
>> +	if (error)
>> +		goto failed_input;
>> +
> In all cases, acpi_device_install_event_handler is being called after
> cmpc_add_acpi_notify_device, which allocates and registers an input
> device.
>
> You should cleanup in case acpi_device_install_event_handler fails and
> call cmpc_remove_acpi_notify_device.
>
> Cascardo.

Hi Cascardo

Yeah I see this now, I'm not freeing the resource in the error path properly,
will add another label for example 'failed_notify_install' that would call
cmpc_remove_acpi_notify_device,

Thanks !
Michał

>
>>  	return 0;
>>  
>>  failed_input:
>> @@ -420,6 +426,7 @@ static int cmpc_accel_add_v4(struct acpi_device *acpi)
>>  
>>  static void cmpc_accel_remove_v4(struct acpi_device *acpi)
>>  {
>> +	acpi_device_remove_event_handler(acpi, ACPI_DEVICE_NOTIFY, cmpc_accel_handler_v4);
>>  	device_remove_file(&acpi->dev, &cmpc_accel_sensitivity_attr_v4);
>>  	device_remove_file(&acpi->dev, &cmpc_accel_g_select_attr_v4);
>>  	cmpc_remove_acpi_notify_device(acpi);
>> @@ -441,7 +448,6 @@ static struct acpi_driver cmpc_accel_acpi_driver_v4 = {
>>  	.ops = {
>>  		.add = cmpc_accel_add_v4,
>>  		.remove = cmpc_accel_remove_v4,
>> -		.notify = cmpc_accel_handler_v4,
>>  	},
>>  	.drv.pm = &cmpc_accel_pm,
>>  };
>> @@ -523,8 +529,10 @@ static acpi_status cmpc_get_accel(acpi_handle handle,
>>  	return status;
>>  }
>>  
>> -static void cmpc_accel_handler(struct acpi_device *dev, u32 event)
>> +static void cmpc_accel_handler(acpi_handle handle, u32 event, void *data)
>>  {
>> +	struct acpi_device *dev = data;
>> +
>>  	if (event == 0x81) {
>>  		unsigned char x, y, z;
>>  		acpi_status status;
>> @@ -639,6 +647,11 @@ static int cmpc_accel_add(struct acpi_device *acpi)
>>  	inputdev = dev_get_drvdata(&acpi->dev);
>>  	dev_set_drvdata(&inputdev->dev, accel);
>>  
>> +	error = acpi_device_install_event_handler(acpi, ACPI_DEVICE_NOTIFY,
>> +						  cmpc_accel_handler);
>> +	if (error)
>> +		goto failed_input;
>> +
>>  	return 0;
>>  
>>  failed_input:
>> @@ -650,6 +663,7 @@ static int cmpc_accel_add(struct acpi_device *acpi)
>>  
>>  static void cmpc_accel_remove(struct acpi_device *acpi)
>>  {
>> +	acpi_device_remove_event_handler(acpi, ACPI_DEVICE_NOTIFY, cmpc_accel_handler);
>>  	device_remove_file(&acpi->dev, &cmpc_accel_sensitivity_attr);
>>  	cmpc_remove_acpi_notify_device(acpi);
>>  }
>> @@ -667,7 +681,6 @@ static struct acpi_driver cmpc_accel_acpi_driver = {
>>  	.ops = {
>>  		.add = cmpc_accel_add,
>>  		.remove = cmpc_accel_remove,
>> -		.notify = cmpc_accel_handler,
>>  	}
>>  };
>>  
>> @@ -693,8 +706,9 @@ static acpi_status cmpc_get_tablet(acpi_handle handle,
>>  	return status;
>>  }
>>  
>> -static void cmpc_tablet_handler(struct acpi_device *dev, u32 event)
>> +static void cmpc_tablet_handler(acpi_handle handle, u32 event, void *data)
>>  {
>> +	struct acpi_device *dev = data;
>>  	unsigned long long val = 0;
>>  	struct input_dev *inputdev = dev_get_drvdata(&dev->dev);
>>  
>> @@ -723,12 +737,20 @@ static void cmpc_tablet_idev_init(struct input_dev *inputdev)
>>  
>>  static int cmpc_tablet_add(struct acpi_device *acpi)
>>  {
>> -	return cmpc_add_acpi_notify_device(acpi, "cmpc_tablet",
>> -					   cmpc_tablet_idev_init);
>> +	int ret;
>> +
>> +	ret = cmpc_add_acpi_notify_device(acpi, "cmpc_tablet",
>> +					  cmpc_tablet_idev_init);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return acpi_device_install_event_handler(acpi, ACPI_DEVICE_NOTIFY,
>> +						 cmpc_tablet_handler);
>>  }
>>  
>>  static void cmpc_tablet_remove(struct acpi_device *acpi)
>>  {
>> +	acpi_device_remove_event_handler(acpi, ACPI_DEVICE_NOTIFY, cmpc_tablet_handler);
>>  	cmpc_remove_acpi_notify_device(acpi);
>>  }
>>  
>> @@ -761,7 +783,6 @@ static struct acpi_driver cmpc_tablet_acpi_driver = {
>>  	.ops = {
>>  		.add = cmpc_tablet_add,
>>  		.remove = cmpc_tablet_remove,
>> -		.notify = cmpc_tablet_handler,
>>  	},
>>  	.drv.pm = &cmpc_tablet_pm,
>>  };
>> @@ -1026,8 +1047,9 @@ static int cmpc_keys_codes[] = {
>>  	KEY_MAX
>>  };
>>  
>> -static void cmpc_keys_handler(struct acpi_device *dev, u32 event)
>> +static void cmpc_keys_handler(acpi_handle handle, u32 event, void *data)
>>  {
>> +	struct acpi_device *dev = data;
>>  	struct input_dev *inputdev;
>>  	int code = KEY_MAX;
>>  
>> @@ -1049,12 +1071,20 @@ static void cmpc_keys_idev_init(struct input_dev *inputdev)
>>  
>>  static int cmpc_keys_add(struct acpi_device *acpi)
>>  {
>> -	return cmpc_add_acpi_notify_device(acpi, "cmpc_keys",
>> -					   cmpc_keys_idev_init);
>> +	int error;
>> +
>> +	error = cmpc_add_acpi_notify_device(acpi, "cmpc_keys",
>> +					    cmpc_keys_idev_init);
>> +	if (error)
>> +		return error;
>> +
>> +	return acpi_device_install_event_handler(acpi, ACPI_DEVICE_NOTIFY,
>> +						 cmpc_keys_handler);
>>  }
>>  
>>  static void cmpc_keys_remove(struct acpi_device *acpi)
>>  {
>> +	acpi_device_remove_event_handler(acpi, ACPI_DEVICE_NOTIFY, cmpc_keys_handler);
>>  	cmpc_remove_acpi_notify_device(acpi);
>>  }
>>  
>> @@ -1071,7 +1101,6 @@ static struct acpi_driver cmpc_keys_acpi_driver = {
>>  	.ops = {
>>  		.add = cmpc_keys_add,
>>  		.remove = cmpc_keys_remove,
>> -		.notify = cmpc_keys_handler,
>>  	}
>>  };
>>  
>> -- 
>> 2.40.1
>>


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v4 21/35] platform/x86/fujitsu-laptop: Move handler installing logic to driver
  2023-06-01 13:17 ` [PATCH v4 21/35] platform/x86/fujitsu-laptop: " Michal Wilczynski
@ 2023-06-02 13:30   ` Ilpo Järvinen
  2023-06-02 13:49     ` Wilczynski, Michal
  0 siblings, 1 reply; 44+ messages in thread
From: Ilpo Järvinen @ 2023-06-02 13:30 UTC (permalink / raw)
  To: Michal Wilczynski
  Cc: Jonathan Woithe, Hans de Goede, Mark Gross, platform-driver-x86,
	linux-acpi, rafael

On Thu, 1 Jun 2023, Michal Wilczynski wrote:

> Currently logic for installing notifications from ACPI devices is
> implemented using notify callback in struct acpi_driver. Preparations
> are being made to replace acpi_driver with more generic struct
> platform_driver, which doesn't contain notify callback. Furthermore
> as of now handlers are being called indirectly through
> acpi_notify_device(), which decreases performance.
> 
> Call acpi_device_install_event_handler() at the end of .add() callback.
> Call acpi_device_remove_event_handler() at the beginning of .remove()
> callback. Change arguments passed to the notify callback to match with
> what's required by acpi_device_install_event_handler().
> 
> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
> ---
>  drivers/platform/x86/fujitsu-laptop.c | 86 +++++++++++++++++----------
>  1 file changed, 54 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> index 085e044e888e..001333edba9f 100644
> --- a/drivers/platform/x86/fujitsu-laptop.c
> +++ b/drivers/platform/x86/fujitsu-laptop.c
> @@ -136,6 +136,8 @@ struct fujitsu_laptop {
>  
>  static struct acpi_device *fext;
>  
> +static void acpi_fujitsu_laptop_notify(acpi_handle handle, u32 event, void *data);
> +
>  /* Fujitsu ACPI interface function */
>  
>  static int call_fext_func(struct acpi_device *device,
> @@ -382,6 +384,37 @@ static int fujitsu_backlight_register(struct acpi_device *device)
>  	return 0;
>  }
>  
> +static void acpi_fujitsu_bl_notify(acpi_handle handle, u32 event, void *data)
> +{
> +	struct acpi_device *device = data;
> +	struct fujitsu_bl *priv;
> +	int oldb, newb;
> +
> +	priv = acpi_driver_data(device);
> +
> +	if (event != ACPI_FUJITSU_NOTIFY_CODE) {
> +		acpi_handle_info(device->handle, "unsupported event [0x%x]\n",
> +				 event);
> +		sparse_keymap_report_event(priv->input, -1, 1, true);
> +		return;
> +	}
> +
> +	oldb = priv->brightness_level;
> +	get_lcd_level(device);
> +	newb = priv->brightness_level;
> +
> +	acpi_handle_debug(device->handle,
> +			  "brightness button event [%i -> %i]\n", oldb, newb);
> +
> +	if (oldb == newb)
> +		return;
> +
> +	if (!disable_brightness_adjust)
> +		set_lcd_level(device, newb);
> +
> +	sparse_keymap_report_event(priv->input, oldb < newb, 1, true);
> +}
> +
>  static int acpi_fujitsu_bl_add(struct acpi_device *device)
>  {
>  	struct fujitsu_bl *priv;
> @@ -410,37 +443,17 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device)
>  	if (ret)
>  		return ret;
>  
> -	return fujitsu_backlight_register(device);
> -}
> +	ret = fujitsu_backlight_register(device);
> +	if (ret)
> +		return ret;
>  
> -/* Brightness notify */
> +	return acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY,
> +						 acpi_fujitsu_bl_notify);
> +}
>  
> -static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
> +static void acpi_fujitsu_bl_remove(struct acpi_device *device)
>  {
> -	struct fujitsu_bl *priv = acpi_driver_data(device);
> -	int oldb, newb;
> -
> -	if (event != ACPI_FUJITSU_NOTIFY_CODE) {
> -		acpi_handle_info(device->handle, "unsupported event [0x%x]\n",
> -				 event);
> -		sparse_keymap_report_event(priv->input, -1, 1, true);
> -		return;
> -	}
> -
> -	oldb = priv->brightness_level;
> -	get_lcd_level(device);
> -	newb = priv->brightness_level;
> -
> -	acpi_handle_debug(device->handle,
> -			  "brightness button event [%i -> %i]\n", oldb, newb);
> -
> -	if (oldb == newb)
> -		return;
> -
> -	if (!disable_brightness_adjust)
> -		set_lcd_level(device, newb);
> -
> -	sparse_keymap_report_event(priv->input, oldb < newb, 1, true);
> +	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_fujitsu_bl_notify);

Please move the function in a separate (no function changes intended) 
patch to keep this patch on point.

>  }
>  
>  /* ACPI device for hotkey handling */
> @@ -839,6 +852,11 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
>  	if (ret)
>  		goto err_free_fifo;
>  
> +	ret = acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY,
> +						acpi_fujitsu_laptop_notify);
> +	if (ret)
> +		goto err_free_fifo;

Here too, fujitsu_laptop_platform_remove() is necessary, goto + put it 
into the rollback path.

Please go through your patches with these rollback corrections in mind, 
I'll skip looking through the rest of platform/x86 patches until you've 
done that.

> +
>  	return 0;
>  
>  err_free_fifo:
> @@ -851,6 +869,8 @@ static void acpi_fujitsu_laptop_remove(struct acpi_device *device)
>  {
>  	struct fujitsu_laptop *priv = acpi_driver_data(device);
>  
> +	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_fujitsu_laptop_notify);
> +
>  	fujitsu_laptop_platform_remove(device);
>  
>  	kfifo_free(&priv->fifo);


-- 
 i.


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v4 19/35] platform/x86/dell/dell-rbtn: Move handler installing logic to driver
  2023-06-02 13:20   ` Ilpo Järvinen
@ 2023-06-02 13:41     ` Wilczynski, Michal
  2023-06-02 14:01       ` Ilpo Järvinen
  0 siblings, 1 reply; 44+ messages in thread
From: Wilczynski, Michal @ 2023-06-02 13:41 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: Pali Rohár, Hans de Goede, Mark Gross, platform-driver-x86,
	linux-acpi, rafael



On 6/2/2023 3:20 PM, Ilpo Järvinen wrote:
> On Thu, 1 Jun 2023, Michal Wilczynski wrote:
>
>> Currently logic for installing notifications from ACPI devices is
>> implemented using notify callback in struct acpi_driver. Preparations
>> are being made to replace acpi_driver with more generic struct
>> platform_driver, which doesn't contain notify callback. Furthermore
>> as of now handlers are being called indirectly through
>> acpi_notify_device(), which decreases performance.
>>
>> Call acpi_device_install_event_handler() at the end of .add() callback.
>> Call acpi_device_remove_event_handler() at the beginning of .remove()
>> callback. Change arguments passed to the notify callback to match with
>> what's required by acpi_device_install_event_handler().
>>
>> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
>> ---
>>  drivers/platform/x86/dell/dell-rbtn.c | 17 ++++++++++++-----
>>  1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/platform/x86/dell/dell-rbtn.c b/drivers/platform/x86/dell/dell-rbtn.c
>> index aa0e6c907494..4dcad59eb035 100644
>> --- a/drivers/platform/x86/dell/dell-rbtn.c
>> +++ b/drivers/platform/x86/dell/dell-rbtn.c
>> @@ -207,7 +207,7 @@ static void rbtn_input_event(struct rbtn_data *rbtn_data)
>>  
>>  static int rbtn_add(struct acpi_device *device);
>>  static void rbtn_remove(struct acpi_device *device);
>> -static void rbtn_notify(struct acpi_device *device, u32 event);
>> +static void rbtn_notify(acpi_handle handle, u32 event, void *data);
>>  
>>  static const struct acpi_device_id rbtn_ids[] = {
>>  	{ "DELRBTN", 0 },
>> @@ -293,7 +293,6 @@ static struct acpi_driver rbtn_driver = {
>>  	.ops = {
>>  		.add = rbtn_add,
>>  		.remove = rbtn_remove,
>> -		.notify = rbtn_notify,
>>  	},
>>  	.owner = THIS_MODULE,
>>  };
>> @@ -422,7 +421,10 @@ static int rbtn_add(struct acpi_device *device)
>>  		ret = -EINVAL;
>>  	}
>>  
>> -	return ret;
>> +	if (ret)
>> +		return ret;
>> +
>> +	return acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY, rbtn_notify);
> What about the other things that are done in rbtn_remove(), should you 
> rollback more?

Yeah you're right, but the total lack of rollback in .add() here seems to be an issue on
it's own i.e even before this patchset .add() was leaking resources in case of failure.
I wonder whether to add missing rollback in separate commit ?

>
> I suspect there's a pre-existing lack of rbtn_acquire(device, false); 
> here to begin with.
>


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v4 21/35] platform/x86/fujitsu-laptop: Move handler installing logic to driver
  2023-06-02 13:30   ` Ilpo Järvinen
@ 2023-06-02 13:49     ` Wilczynski, Michal
  2023-06-02 13:55       ` Ilpo Järvinen
  0 siblings, 1 reply; 44+ messages in thread
From: Wilczynski, Michal @ 2023-06-02 13:49 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: Jonathan Woithe, Hans de Goede, Mark Gross, platform-driver-x86,
	linux-acpi, rafael



On 6/2/2023 3:30 PM, Ilpo Järvinen wrote:
> On Thu, 1 Jun 2023, Michal Wilczynski wrote:
>
>> Currently logic for installing notifications from ACPI devices is
>> implemented using notify callback in struct acpi_driver. Preparations
>> are being made to replace acpi_driver with more generic struct
>> platform_driver, which doesn't contain notify callback. Furthermore
>> as of now handlers are being called indirectly through
>> acpi_notify_device(), which decreases performance.
>>
>> Call acpi_device_install_event_handler() at the end of .add() callback.
>> Call acpi_device_remove_event_handler() at the beginning of .remove()
>> callback. Change arguments passed to the notify callback to match with
>> what's required by acpi_device_install_event_handler().
>>
>> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
>> ---
>>  drivers/platform/x86/fujitsu-laptop.c | 86 +++++++++++++++++----------
>>  1 file changed, 54 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
>> index 085e044e888e..001333edba9f 100644
>> --- a/drivers/platform/x86/fujitsu-laptop.c
>> +++ b/drivers/platform/x86/fujitsu-laptop.c
>> @@ -136,6 +136,8 @@ struct fujitsu_laptop {
>>  
>>  static struct acpi_device *fext;
>>  
>> +static void acpi_fujitsu_laptop_notify(acpi_handle handle, u32 event, void *data);
>> +
>>  /* Fujitsu ACPI interface function */
>>  
>>  static int call_fext_func(struct acpi_device *device,
>> @@ -382,6 +384,37 @@ static int fujitsu_backlight_register(struct acpi_device *device)
>>  	return 0;
>>  }
>>  
>> +static void acpi_fujitsu_bl_notify(acpi_handle handle, u32 event, void *data)
>> +{
>> +	struct acpi_device *device = data;
>> +	struct fujitsu_bl *priv;
>> +	int oldb, newb;
>> +
>> +	priv = acpi_driver_data(device);
>> +
>> +	if (event != ACPI_FUJITSU_NOTIFY_CODE) {
>> +		acpi_handle_info(device->handle, "unsupported event [0x%x]\n",
>> +				 event);
>> +		sparse_keymap_report_event(priv->input, -1, 1, true);
>> +		return;
>> +	}
>> +
>> +	oldb = priv->brightness_level;
>> +	get_lcd_level(device);
>> +	newb = priv->brightness_level;
>> +
>> +	acpi_handle_debug(device->handle,
>> +			  "brightness button event [%i -> %i]\n", oldb, newb);
>> +
>> +	if (oldb == newb)
>> +		return;
>> +
>> +	if (!disable_brightness_adjust)
>> +		set_lcd_level(device, newb);
>> +
>> +	sparse_keymap_report_event(priv->input, oldb < newb, 1, true);
>> +}
>> +
>>  static int acpi_fujitsu_bl_add(struct acpi_device *device)
>>  {
>>  	struct fujitsu_bl *priv;
>> @@ -410,37 +443,17 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device)
>>  	if (ret)
>>  		return ret;
>>  
>> -	return fujitsu_backlight_register(device);
>> -}
>> +	ret = fujitsu_backlight_register(device);
>> +	if (ret)
>> +		return ret;
>>  
>> -/* Brightness notify */
>> +	return acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY,
>> +						 acpi_fujitsu_bl_notify);
>> +}
>>  
>> -static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
>> +static void acpi_fujitsu_bl_remove(struct acpi_device *device)
>>  {
>> -	struct fujitsu_bl *priv = acpi_driver_data(device);
>> -	int oldb, newb;
>> -
>> -	if (event != ACPI_FUJITSU_NOTIFY_CODE) {
>> -		acpi_handle_info(device->handle, "unsupported event [0x%x]\n",
>> -				 event);
>> -		sparse_keymap_report_event(priv->input, -1, 1, true);
>> -		return;
>> -	}
>> -
>> -	oldb = priv->brightness_level;
>> -	get_lcd_level(device);
>> -	newb = priv->brightness_level;
>> -
>> -	acpi_handle_debug(device->handle,
>> -			  "brightness button event [%i -> %i]\n", oldb, newb);
>> -
>> -	if (oldb == newb)
>> -		return;
>> -
>> -	if (!disable_brightness_adjust)
>> -		set_lcd_level(device, newb);
>> -
>> -	sparse_keymap_report_event(priv->input, oldb < newb, 1, true);
>> +	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_fujitsu_bl_notify);
> Please move the function in a separate (no function changes intended) 
> patch to keep this patch on point.

Sure I can do that, however this moving around make sense in context of this patch as
it's necessary to compile with the new event installation method.

>
>>  }
>>  
>>  /* ACPI device for hotkey handling */
>> @@ -839,6 +852,11 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
>>  	if (ret)
>>  		goto err_free_fifo;
>>  
>> +	ret = acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY,
>> +						acpi_fujitsu_laptop_notify);
>> +	if (ret)
>> +		goto err_free_fifo;
> Here too, fujitsu_laptop_platform_remove() is necessary, goto + put it 
> into the rollback path.
>
> Please go through your patches with these rollback corrections in mind, 
> I'll skip looking through the rest of platform/x86 patches until you've 
> done that.

Will do that,
Thank you for your time !


>
>> +
>>  	return 0;
>>  
>>  err_free_fifo:
>> @@ -851,6 +869,8 @@ static void acpi_fujitsu_laptop_remove(struct acpi_device *device)
>>  {
>>  	struct fujitsu_laptop *priv = acpi_driver_data(device);
>>  
>> +	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_fujitsu_laptop_notify);
>> +
>>  	fujitsu_laptop_platform_remove(device);
>>  
>>  	kfifo_free(&priv->fifo);
>


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v4 21/35] platform/x86/fujitsu-laptop: Move handler installing logic to driver
  2023-06-02 13:49     ` Wilczynski, Michal
@ 2023-06-02 13:55       ` Ilpo Järvinen
  0 siblings, 0 replies; 44+ messages in thread
From: Ilpo Järvinen @ 2023-06-02 13:55 UTC (permalink / raw)
  To: Wilczynski, Michal
  Cc: Jonathan Woithe, Hans de Goede, Mark Gross, platform-driver-x86,
	linux-acpi, rafael

[-- Attachment #1: Type: text/plain, Size: 4789 bytes --]

On Fri, 2 Jun 2023, Wilczynski, Michal wrote:
> On 6/2/2023 3:30 PM, Ilpo Järvinen wrote:
> > On Thu, 1 Jun 2023, Michal Wilczynski wrote:
> >
> >> Currently logic for installing notifications from ACPI devices is
> >> implemented using notify callback in struct acpi_driver. Preparations
> >> are being made to replace acpi_driver with more generic struct
> >> platform_driver, which doesn't contain notify callback. Furthermore
> >> as of now handlers are being called indirectly through
> >> acpi_notify_device(), which decreases performance.
> >>
> >> Call acpi_device_install_event_handler() at the end of .add() callback.
> >> Call acpi_device_remove_event_handler() at the beginning of .remove()
> >> callback. Change arguments passed to the notify callback to match with
> >> what's required by acpi_device_install_event_handler().
> >>
> >> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
> >> ---
> >>  drivers/platform/x86/fujitsu-laptop.c | 86 +++++++++++++++++----------
> >>  1 file changed, 54 insertions(+), 32 deletions(-)
> >>
> >> diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> >> index 085e044e888e..001333edba9f 100644
> >> --- a/drivers/platform/x86/fujitsu-laptop.c
> >> +++ b/drivers/platform/x86/fujitsu-laptop.c
> >> @@ -136,6 +136,8 @@ struct fujitsu_laptop {
> >>  
> >>  static struct acpi_device *fext;
> >>  
> >> +static void acpi_fujitsu_laptop_notify(acpi_handle handle, u32 event, void *data);
> >> +
> >>  /* Fujitsu ACPI interface function */
> >>  
> >>  static int call_fext_func(struct acpi_device *device,
> >> @@ -382,6 +384,37 @@ static int fujitsu_backlight_register(struct acpi_device *device)
> >>  	return 0;
> >>  }
> >>  
> >> +static void acpi_fujitsu_bl_notify(acpi_handle handle, u32 event, void *data)
> >> +{
> >> +	struct acpi_device *device = data;
> >> +	struct fujitsu_bl *priv;
> >> +	int oldb, newb;
> >> +
> >> +	priv = acpi_driver_data(device);
> >> +
> >> +	if (event != ACPI_FUJITSU_NOTIFY_CODE) {
> >> +		acpi_handle_info(device->handle, "unsupported event [0x%x]\n",
> >> +				 event);
> >> +		sparse_keymap_report_event(priv->input, -1, 1, true);
> >> +		return;
> >> +	}
> >> +
> >> +	oldb = priv->brightness_level;
> >> +	get_lcd_level(device);
> >> +	newb = priv->brightness_level;
> >> +
> >> +	acpi_handle_debug(device->handle,
> >> +			  "brightness button event [%i -> %i]\n", oldb, newb);
> >> +
> >> +	if (oldb == newb)
> >> +		return;
> >> +
> >> +	if (!disable_brightness_adjust)
> >> +		set_lcd_level(device, newb);
> >> +
> >> +	sparse_keymap_report_event(priv->input, oldb < newb, 1, true);
> >> +}
> >> +
> >>  static int acpi_fujitsu_bl_add(struct acpi_device *device)
> >>  {
> >>  	struct fujitsu_bl *priv;
> >> @@ -410,37 +443,17 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device)
> >>  	if (ret)
> >>  		return ret;
> >>  
> >> -	return fujitsu_backlight_register(device);
> >> -}
> >> +	ret = fujitsu_backlight_register(device);
> >> +	if (ret)
> >> +		return ret;
> >>  
> >> -/* Brightness notify */
> >> +	return acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY,
> >> +						 acpi_fujitsu_bl_notify);
> >> +}
> >>  
> >> -static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
> >> +static void acpi_fujitsu_bl_remove(struct acpi_device *device)
> >>  {
> >> -	struct fujitsu_bl *priv = acpi_driver_data(device);
> >> -	int oldb, newb;
> >> -
> >> -	if (event != ACPI_FUJITSU_NOTIFY_CODE) {
> >> -		acpi_handle_info(device->handle, "unsupported event [0x%x]\n",
> >> -				 event);
> >> -		sparse_keymap_report_event(priv->input, -1, 1, true);
> >> -		return;
> >> -	}
> >> -
> >> -	oldb = priv->brightness_level;
> >> -	get_lcd_level(device);
> >> -	newb = priv->brightness_level;
> >> -
> >> -	acpi_handle_debug(device->handle,
> >> -			  "brightness button event [%i -> %i]\n", oldb, newb);
> >> -
> >> -	if (oldb == newb)
> >> -		return;
> >> -
> >> -	if (!disable_brightness_adjust)
> >> -		set_lcd_level(device, newb);
> >> -
> >> -	sparse_keymap_report_event(priv->input, oldb < newb, 1, true);
> >> +	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_fujitsu_bl_notify);
> > Please move the function in a separate (no function changes intended) 
> > patch to keep this patch on point.
> 
> Sure I can do that, however this moving around make sense in context of this patch as
> it's necessary to compile with the new event installation method.

You move the function in one patch without changing anything else. And 
then in another change that is this notify change you make the necessary 
adjustments to the moved function.

It makes both patches easier to review because one is a trivial copy and 
notify related changes are no longer mixed up with the copy.

-- 
 i.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v4 19/35] platform/x86/dell/dell-rbtn: Move handler installing logic to driver
  2023-06-02 13:41     ` Wilczynski, Michal
@ 2023-06-02 14:01       ` Ilpo Järvinen
  0 siblings, 0 replies; 44+ messages in thread
From: Ilpo Järvinen @ 2023-06-02 14:01 UTC (permalink / raw)
  To: Wilczynski, Michal
  Cc: Pali Rohár, Hans de Goede, Mark Gross, platform-driver-x86,
	linux-acpi, rafael

[-- Attachment #1: Type: text/plain, Size: 2764 bytes --]

On Fri, 2 Jun 2023, Wilczynski, Michal wrote:
> On 6/2/2023 3:20 PM, Ilpo Järvinen wrote:
> > On Thu, 1 Jun 2023, Michal Wilczynski wrote:
> >
> >> Currently logic for installing notifications from ACPI devices is
> >> implemented using notify callback in struct acpi_driver. Preparations
> >> are being made to replace acpi_driver with more generic struct
> >> platform_driver, which doesn't contain notify callback. Furthermore
> >> as of now handlers are being called indirectly through
> >> acpi_notify_device(), which decreases performance.
> >>
> >> Call acpi_device_install_event_handler() at the end of .add() callback.
> >> Call acpi_device_remove_event_handler() at the beginning of .remove()
> >> callback. Change arguments passed to the notify callback to match with
> >> what's required by acpi_device_install_event_handler().
> >>
> >> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
> >> ---
> >>  drivers/platform/x86/dell/dell-rbtn.c | 17 ++++++++++++-----
> >>  1 file changed, 12 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/platform/x86/dell/dell-rbtn.c b/drivers/platform/x86/dell/dell-rbtn.c
> >> index aa0e6c907494..4dcad59eb035 100644
> >> --- a/drivers/platform/x86/dell/dell-rbtn.c
> >> +++ b/drivers/platform/x86/dell/dell-rbtn.c
> >> @@ -207,7 +207,7 @@ static void rbtn_input_event(struct rbtn_data *rbtn_data)
> >>  
> >>  static int rbtn_add(struct acpi_device *device);
> >>  static void rbtn_remove(struct acpi_device *device);
> >> -static void rbtn_notify(struct acpi_device *device, u32 event);
> >> +static void rbtn_notify(acpi_handle handle, u32 event, void *data);
> >>  
> >>  static const struct acpi_device_id rbtn_ids[] = {
> >>  	{ "DELRBTN", 0 },
> >> @@ -293,7 +293,6 @@ static struct acpi_driver rbtn_driver = {
> >>  	.ops = {
> >>  		.add = rbtn_add,
> >>  		.remove = rbtn_remove,
> >> -		.notify = rbtn_notify,
> >>  	},
> >>  	.owner = THIS_MODULE,
> >>  };
> >> @@ -422,7 +421,10 @@ static int rbtn_add(struct acpi_device *device)
> >>  		ret = -EINVAL;
> >>  	}
> >>  
> >> -	return ret;
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	return acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY, rbtn_notify);
> > What about the other things that are done in rbtn_remove(), should you 
> > rollback more?
> 
> Yeah you're right, but the total lack of rollback in .add() here seems 
> to be an issue on it's own i.e even before this patchset .add() was 
> leaking resources in case of failure.
> I wonder whether to add missing rollback in separate commit ?

Yes, make separate patch out of it and mark it with Fixes tag. You can 
send it separately.

> > I suspect there's a pre-existing lack of rbtn_acquire(device, false); 
> > here to begin with.
> >
> 

-- 
 i.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v4 11/35] iio/acpi-als: Move handler installing logic to driver
  2023-06-01 13:17 ` [PATCH v4 11/35] iio/acpi-als: " Michal Wilczynski
@ 2023-06-04 10:53   ` Jonathan Cameron
  0 siblings, 0 replies; 44+ messages in thread
From: Jonathan Cameron @ 2023-06-04 10:53 UTC (permalink / raw)
  To: Michal Wilczynski
  Cc: Lars-Peter Clausen, linux-iio, linux-acpi, platform-driver-x86,
	rafael

On Thu,  1 Jun 2023 15:17:14 +0200
Michal Wilczynski <michal.wilczynski@intel.com> wrote:

> Currently logic for installing notifications from ACPI devices is
> implemented using notify callback in struct acpi_driver. Preparations
> are being made to replace acpi_driver with more generic struct
> platform_driver, which doesn't contain notify callback. Furthermore
> as of now handlers are being called indirectly through
> acpi_notify_device(), which decreases performance.
> 
> Call acpi_device_install_event_handler() at the end of .add() callback.
> Call acpi_device_remove_event_handler() at the beginning of .remove()
> callback. Change arguments passed to the notify callback to match with
> what's required by acpi_device_install_event_handler().
> 
> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com>
Hi Michal,

Comments inline.

> ---
>  drivers/iio/light/acpi-als.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/light/acpi-als.c b/drivers/iio/light/acpi-als.c
> index 2d91caf24dd0..5e200c6d91bc 100644
> --- a/drivers/iio/light/acpi-als.c
> +++ b/drivers/iio/light/acpi-als.c
> @@ -100,10 +100,14 @@ static int acpi_als_read_value(struct acpi_als *als, char *prop, s32 *val)
>  	return 0;
>  }
>  
> -static void acpi_als_notify(struct acpi_device *device, u32 event)
> +static void acpi_als_notify(acpi_handle handle, u32 event, void *data)
>  {
> -	struct iio_dev *indio_dev = acpi_driver_data(device);
> -	struct acpi_als *als = iio_priv(indio_dev);
> +	struct acpi_device *device = data;
> +	struct iio_dev *indio_dev;
> +	struct acpi_als *als;
> +
> +	indio_dev = acpi_driver_data(device);
> +	als = iio_priv(indio_dev);

Not particularly important, but I'd have kept to existing style

	struct acpi_device *device = data;
	struct iio_dev *indio_dev = acpi_driver_data(device);
	struct acpi_als *als = iio_priv(indio_dev);

Less churn that way.

>  
>  	if (iio_buffer_enabled(indio_dev) && iio_trigger_using_own(indio_dev)) {
>  		switch (event) {
> @@ -225,7 +229,16 @@ static int acpi_als_add(struct acpi_device *device)
>  	if (ret)
>  		return ret;
>  
> -	return devm_iio_device_register(dev, indio_dev);
> +	ret = devm_iio_device_register(dev, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	return acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_als_notify);

Prefer to keep to a fully devm managed flow for removal

So use a devm_add_action_or_reset() to unwind this rather than adding a remove()
callback.

Obviously ordering is the same currently but that may change if this driver
is modified in future and it's a lot easier to get that right if fully devm
(or fully not).

Jonathan

> +}
> +
> +static void acpi_als_remove(struct acpi_device *device)
> +{
> +	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_als_notify);
>  }
>  
>  static const struct acpi_device_id acpi_als_device_ids[] = {
> @@ -241,7 +254,7 @@ static struct acpi_driver acpi_als_driver = {
>  	.ids	= acpi_als_device_ids,
>  	.ops = {
>  		.add	= acpi_als_add,
> -		.notify	= acpi_als_notify,
> +		.remove = acpi_als_remove,
>  	},
>  };
>  


^ permalink raw reply	[flat|nested] 44+ messages in thread

end of thread, other threads:[~2023-06-04 10:53 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-01 13:17 [PATCH v4 02/35] acpi/ac: Move handler installing logic to driver Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 03/35] acpi/video: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 04/35] acpi/battery: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 05/35] acpi/button: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 06/35] acpi/hed: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 07/35] acpi/nfit: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 08/35] acpi/thermal: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 09/35] acpi/tiny-power-button: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 10/35] hwmon/acpi_power_meter: " Michal Wilczynski
2023-06-01 13:51   ` Guenter Roeck
2023-06-01 13:17 ` [PATCH v4 11/35] iio/acpi-als: " Michal Wilczynski
2023-06-04 10:53   ` Jonathan Cameron
2023-06-01 13:17 ` [PATCH v4 12/35] platform/chromeos_tbmc: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 13/35] platform/wilco_ec: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 14/35] platform/surface/button: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 15/35] platform/x86/acer-wireless: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 16/35] platform/x86/asus-laptop: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 17/35] platform/x86/asus-wireless: " Michal Wilczynski
2023-06-02 13:09   ` Ilpo Järvinen
2023-06-02 13:16     ` Wilczynski, Michal
2023-06-01 13:17 ` [PATCH v4 18/35] platform/x86/classmate-laptop: " Michal Wilczynski
2023-06-02 10:29   ` Thadeu Lima de Souza Cascardo
2023-06-02 13:25     ` Wilczynski, Michal
2023-06-01 13:17 ` [PATCH v4 19/35] platform/x86/dell/dell-rbtn: " Michal Wilczynski
2023-06-02 13:20   ` Ilpo Järvinen
2023-06-02 13:41     ` Wilczynski, Michal
2023-06-02 14:01       ` Ilpo Järvinen
2023-06-01 13:17 ` [PATCH v4 20/35] platform/x86/eeepc-laptop: " Michal Wilczynski
2023-06-02 13:24   ` Ilpo Järvinen
2023-06-01 13:17 ` [PATCH v4 21/35] platform/x86/fujitsu-laptop: " Michal Wilczynski
2023-06-02 13:30   ` Ilpo Järvinen
2023-06-02 13:49     ` Wilczynski, Michal
2023-06-02 13:55       ` Ilpo Järvinen
2023-06-01 13:17 ` [PATCH v4 22/35] platform/x86/lg-laptop: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 23/35] platform/x86/panasonic-laptop: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 24/35] platform/x86/system76_acpi: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 25/35] platform/x86/topstar-laptop: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 26/35] platform/x86/toshiba_acpi: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 27/35] platform/x86/toshiba_bluetooth: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 28/35] platform/x86/toshiba_haps: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 29/35] platform/x86/wireless-hotkey: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 30/35] platform/x86/xo15-ebook: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 31/35] platform/x86/sony-laptop: " Michal Wilczynski
2023-06-01 13:17 ` [PATCH v4 32/35] virt/vmgenid: " Michal Wilczynski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox