[PATCH v1 0/5] platform/x86: fujitsu: Bind drivers to platform devices instead of ACPI ones

[PATCH v1 0/5] platform/x86: fujitsu: Bind drivers to platform devices instead of ACPI ones

[Rafael J. Wysocki]

Hi All,

This series is part of a larger effort to switch over all drivers using
the struct acpi_driver interface to the more common struct platform_driver
interface and eliminate the former.  The background is explained in
Documentation/driver-api/acpi/acpi-drivers.rst and in the changelog of
the patch that introduced the above document:

https://lore.kernel.org/all/2396510.ElGaqSPkdT@rafael.j.wysocki/

The bottom line is that the kernel would be better off without struct
acpi_driver and so it is better to get rid of it.

This series carries out driver conversions of the platform x86 drivers
for Fujitsu platforms.

Patch [1/5] converts the fujitsu-tablet to a proper platform one based
on the struct platform_driver interface.

Patch [2/5] rearranges the code in the fujitsu-laptop driver to avoid
introducing forward declarations of some functions in the subsequent
patches.

Patch [3/5] updates the fujitsu-laptop driver to install ACPI notify
handlers by itself instead of using .notify() callbacks from struct
acpi_driver, which is requisite for the driver conversion.

Patches [4-5/5] replaces two instances of the struct acpi_driver
interface in the fujitsu-laptop driver with struct platform_driver.

Thanks!

[PATCH v1 1/5] platform/x86: fujitsu-tablet: Convert ACPI driver to a platform one

[Rafael J. Wysocki]

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

In all cases in which a struct acpi_driver is used for binding a driver
to an ACPI device object, a corresponding platform device is created by
the ACPI core and that device is regarded as a proper representation of
underlying hardware.  Accordingly, a struct platform_driver should be
used by driver code to bind to that device.  There are multiple reasons
why drivers should not bind directly to ACPI device objects [1].

Overall, it is better to bind drivers to platform devices than to their
ACPI companions, so convert the fujitsu-tablet ACPI driver to a platform
one.

After this change, the subordinate input device will be registered under
the platform device used for driver binding instead of its ACPI companion.

While this is not expected to alter functionality, it changes sysfs
layout and so it will be visible to user space.

Link: https://lore.kernel.org/all/2396510.ElGaqSPkdT@rafael.j.wysocki/ [1]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/platform/x86/fujitsu-tablet.c | 30 +++++++++++++--------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-tablet.c b/drivers/platform/x86/fujitsu-tablet.c
index 17f08ce7552d..8319df28e9b8 100644
--- a/drivers/platform/x86/fujitsu-tablet.c
+++ b/drivers/platform/x86/fujitsu-tablet.c
@@ -18,6 +18,7 @@
 #include <linux/input.h>
 #include <linux/delay.h>
 #include <linux/dmi.h>
+#include <linux/platform_device.h>
 
 #define MODULENAME "fujitsu-tablet"
 
@@ -442,14 +443,12 @@ static acpi_status fujitsu_walk_resources(struct acpi_resource *res, void *data)
 	}
 }
 
-static int acpi_fujitsu_add(struct acpi_device *adev)
+static int acpi_fujitsu_probe(struct platform_device *pdev)
 {
+	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
 	acpi_status status;
 	int error;
 
-	if (!adev)
-		return -EINVAL;
-
 	status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
 			fujitsu_walk_resources, NULL);
 	if (ACPI_FAILURE(status) || !fujitsu.irq || !fujitsu.io_base)
@@ -461,7 +460,7 @@ static int acpi_fujitsu_add(struct acpi_device *adev)
 	snprintf(fujitsu.phys, sizeof(fujitsu.phys),
 			"%s/input0", acpi_device_hid(adev));
 
-	error = input_fujitsu_setup(&adev->dev,
+	error = input_fujitsu_setup(&pdev->dev,
 		acpi_device_name(adev), fujitsu.phys);
 	if (error)
 		return error;
@@ -484,7 +483,7 @@ static int acpi_fujitsu_add(struct acpi_device *adev)
 	return 0;
 }
 
-static void acpi_fujitsu_remove(struct acpi_device *adev)
+static void acpi_fujitsu_remove(struct platform_device *pdev)
 {
 	free_irq(fujitsu.irq, fujitsu_interrupt);
 	release_region(fujitsu.io_base, fujitsu.io_length);
@@ -501,15 +500,14 @@ static int acpi_fujitsu_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(acpi_fujitsu_pm, NULL, acpi_fujitsu_resume);
 
-static struct acpi_driver acpi_fujitsu_driver = {
-	.name  = MODULENAME,
-	.class = "hotkey",
-	.ids   = fujitsu_ids,
-	.ops   = {
-		.add    = acpi_fujitsu_add,
-		.remove	= acpi_fujitsu_remove,
+static struct platform_driver acpi_fujitsu_driver = {
+	.probe = acpi_fujitsu_probe,
+	.remove = acpi_fujitsu_remove,
+	.driver = {
+		.name = MODULENAME,
+		.acpi_match_table = fujitsu_ids,
+		.pm = &acpi_fujitsu_pm,
 	},
-	.drv.pm = &acpi_fujitsu_pm,
 };
 
 static int __init fujitsu_module_init(void)
@@ -518,7 +516,7 @@ static int __init fujitsu_module_init(void)
 
 	dmi_check_system(dmi_ids);
 
-	error = acpi_bus_register_driver(&acpi_fujitsu_driver);
+	error = platform_driver_register(&acpi_fujitsu_driver);
 	if (error)
 		return error;
 
@@ -527,7 +525,7 @@ static int __init fujitsu_module_init(void)
 
 static void __exit fujitsu_module_exit(void)
 {
-	acpi_bus_unregister_driver(&acpi_fujitsu_driver);
+	platform_driver_unregister(&acpi_fujitsu_driver);
 }
 
 module_init(fujitsu_module_init);
-- 
2.51.0

[PATCH v1 2/5] platform/x86: fujitsu: Reorder code to avoid forward declarations

[Rafael J. Wysocki]

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

Move the definitions of acpi_fujitsu_bl_notify() and
acpi_fujitsu_laptop_notify() along with some helpers above
the definitions of the functions that will refer to them
after subsequent changes, to avoid having to add forward
declarations of them.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/platform/x86/fujitsu-laptop.c | 216 +++++++++++++-------------
 1 file changed, 108 insertions(+), 108 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 931fbcdd21b8..1adce90ae3e6 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -500,6 +500,36 @@ static int fujitsu_backlight_register(struct acpi_device *device)
 	return 0;
 }
 
+/* Brightness notify */
+
+static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
+{
+	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);
+}
+
 static int acpi_fujitsu_bl_add(struct acpi_device *device)
 {
 	struct fujitsu_bl *priv;
@@ -531,36 +561,6 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device)
 	return fujitsu_backlight_register(device);
 }
 
-/* Brightness notify */
-
-static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
-{
-	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 for hotkey handling */
 
 static const struct key_entry keymap_default[] = {
@@ -908,6 +908,84 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device)
 	return 0;
 }
 
+static void acpi_fujitsu_laptop_press(struct acpi_device *device, int scancode)
+{
+	struct fujitsu_laptop *priv = acpi_driver_data(device);
+	int ret;
+
+	ret = kfifo_in_locked(&priv->fifo, (unsigned char *)&scancode,
+			      sizeof(scancode), &priv->fifo_lock);
+	if (ret != sizeof(scancode)) {
+		dev_info(&priv->input->dev, "Could not push scancode [0x%x]\n",
+			 scancode);
+		return;
+	}
+	sparse_keymap_report_event(priv->input, scancode, 1, false);
+	dev_dbg(&priv->input->dev, "Push scancode into ringbuffer [0x%x]\n",
+		scancode);
+}
+
+static void acpi_fujitsu_laptop_release(struct acpi_device *device)
+{
+	struct fujitsu_laptop *priv = acpi_driver_data(device);
+	int scancode, ret;
+
+	while (true) {
+		ret = kfifo_out_locked(&priv->fifo, (unsigned char *)&scancode,
+				       sizeof(scancode), &priv->fifo_lock);
+		if (ret != sizeof(scancode))
+			return;
+		sparse_keymap_report_event(priv->input, scancode, 0, false);
+		dev_dbg(&priv->input->dev,
+			"Pop scancode from ringbuffer [0x%x]\n", scancode);
+	}
+}
+
+static void acpi_fujitsu_laptop_notify(struct acpi_device *device, u32 event)
+{
+	struct fujitsu_laptop *priv = acpi_driver_data(device);
+	unsigned long flags;
+	int scancode, i = 0;
+	unsigned int irb;
+
+	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;
+	}
+
+	if (priv->flags_supported)
+		priv->flags_state = call_fext_func(device, FUNC_FLAGS, 0x4, 0x0,
+						   0x0);
+
+	while ((irb = call_fext_func(device,
+				     FUNC_BUTTONS, 0x1, 0x0, 0x0)) != 0 &&
+	       i++ < MAX_HOTKEY_RINGBUFFER_SIZE) {
+		scancode = irb & 0x4ff;
+		if (sparse_keymap_entry_from_scancode(priv->input, scancode))
+			acpi_fujitsu_laptop_press(device, scancode);
+		else if (scancode == 0)
+			acpi_fujitsu_laptop_release(device);
+		else
+			acpi_handle_info(device->handle,
+					 "Unknown GIRB result [%x]\n", irb);
+	}
+
+	/*
+	 * First seen on the Skylake-based Lifebook E736/E746/E756), the
+	 * touchpad toggle hotkey (Fn+F4) is handled in software. Other models
+	 * have since added additional "soft keys". These are reported in the
+	 * status flags queried using FUNC_FLAGS.
+	 */
+	if (priv->flags_supported & (FLAG_SOFTKEYS)) {
+		flags = call_fext_func(device, FUNC_FLAGS, 0x1, 0x0, 0x0);
+		flags &= (FLAG_SOFTKEYS);
+		for_each_set_bit(i, &flags, BITS_PER_LONG)
+			sparse_keymap_report_event(priv->input, BIT(i), 1, true);
+	}
+}
+
 static int acpi_fujitsu_laptop_add(struct acpi_device *device)
 {
 	struct fujitsu_laptop *priv;
@@ -1001,84 +1079,6 @@ static void acpi_fujitsu_laptop_remove(struct acpi_device *device)
 	kfifo_free(&priv->fifo);
 }
 
-static void acpi_fujitsu_laptop_press(struct acpi_device *device, int scancode)
-{
-	struct fujitsu_laptop *priv = acpi_driver_data(device);
-	int ret;
-
-	ret = kfifo_in_locked(&priv->fifo, (unsigned char *)&scancode,
-			      sizeof(scancode), &priv->fifo_lock);
-	if (ret != sizeof(scancode)) {
-		dev_info(&priv->input->dev, "Could not push scancode [0x%x]\n",
-			 scancode);
-		return;
-	}
-	sparse_keymap_report_event(priv->input, scancode, 1, false);
-	dev_dbg(&priv->input->dev, "Push scancode into ringbuffer [0x%x]\n",
-		scancode);
-}
-
-static void acpi_fujitsu_laptop_release(struct acpi_device *device)
-{
-	struct fujitsu_laptop *priv = acpi_driver_data(device);
-	int scancode, ret;
-
-	while (true) {
-		ret = kfifo_out_locked(&priv->fifo, (unsigned char *)&scancode,
-				       sizeof(scancode), &priv->fifo_lock);
-		if (ret != sizeof(scancode))
-			return;
-		sparse_keymap_report_event(priv->input, scancode, 0, false);
-		dev_dbg(&priv->input->dev,
-			"Pop scancode from ringbuffer [0x%x]\n", scancode);
-	}
-}
-
-static void acpi_fujitsu_laptop_notify(struct acpi_device *device, u32 event)
-{
-	struct fujitsu_laptop *priv = acpi_driver_data(device);
-	unsigned long flags;
-	int scancode, i = 0;
-	unsigned int irb;
-
-	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;
-	}
-
-	if (priv->flags_supported)
-		priv->flags_state = call_fext_func(device, FUNC_FLAGS, 0x4, 0x0,
-						   0x0);
-
-	while ((irb = call_fext_func(device,
-				     FUNC_BUTTONS, 0x1, 0x0, 0x0)) != 0 &&
-	       i++ < MAX_HOTKEY_RINGBUFFER_SIZE) {
-		scancode = irb & 0x4ff;
-		if (sparse_keymap_entry_from_scancode(priv->input, scancode))
-			acpi_fujitsu_laptop_press(device, scancode);
-		else if (scancode == 0)
-			acpi_fujitsu_laptop_release(device);
-		else
-			acpi_handle_info(device->handle,
-					 "Unknown GIRB result [%x]\n", irb);
-	}
-
-	/*
-	 * First seen on the Skylake-based Lifebook E736/E746/E756), the
-	 * touchpad toggle hotkey (Fn+F4) is handled in software. Other models
-	 * have since added additional "soft keys". These are reported in the
-	 * status flags queried using FUNC_FLAGS.
-	 */
-	if (priv->flags_supported & (FLAG_SOFTKEYS)) {
-		flags = call_fext_func(device, FUNC_FLAGS, 0x1, 0x0, 0x0);
-		flags &= (FLAG_SOFTKEYS);
-		for_each_set_bit(i, &flags, BITS_PER_LONG)
-			sparse_keymap_report_event(priv->input, BIT(i), 1, true);
-	}
-}
-
 /* Initialization */
 
 static const struct acpi_device_id fujitsu_bl_device_ids[] = {
-- 
2.51.0

[PATCH v1 3/5] platform/x86: fujitsu: Register ACPI notify handlers directly

[Rafael J. Wysocki]

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

To facilitate subsequent conversion of the driver to using struct
platform_driver instead of struct acpi_driver, make it install its ACPI
notify handlers directly instead of using struct acpi_driver .notify()
callbacks.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/platform/x86/fujitsu-laptop.c | 30 ++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 1adce90ae3e6..cb13c06b8f35 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -502,8 +502,9 @@ static int fujitsu_backlight_register(struct acpi_device *device)
 
 /* Brightness notify */
 
-static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
+static void acpi_fujitsu_bl_notify(acpi_handle handle, u32 event, void *data)
 {
+	struct acpi_device *device = data;
 	struct fujitsu_bl *priv = acpi_driver_data(device);
 	int oldb, newb;
 
@@ -558,7 +559,18 @@ 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;
+
+	return acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
+					       acpi_fujitsu_bl_notify, device);
+}
+
+static void acpi_fujitsu_bl_remove(struct acpi_device *device)
+{
+	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
+				       acpi_fujitsu_bl_notify);
 }
 
 /* ACPI device for hotkey handling */
@@ -941,8 +953,9 @@ 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 acpi_device *device = data;
 	struct fujitsu_laptop *priv = acpi_driver_data(device);
 	unsigned long flags;
 	int scancode, i = 0;
@@ -1056,6 +1069,11 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
 	if (ret)
 		goto err_free_fifo;
 
+	ret = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
+					      acpi_fujitsu_laptop_notify, device);
+	if (ret)
+		goto err_free_fifo;
+
 	ret = fujitsu_battery_charge_control_add(device);
 	if (ret < 0)
 		pr_warn("Unable to register battery charge control: %d\n", ret);
@@ -1074,6 +1092,9 @@ static void acpi_fujitsu_laptop_remove(struct acpi_device *device)
 
 	fujitsu_battery_charge_control_remove(device);
 
+	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
+				       acpi_fujitsu_laptop_notify);
+
 	fujitsu_laptop_platform_remove(device);
 
 	kfifo_free(&priv->fifo);
@@ -1092,7 +1113,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,
 		},
 };
 
@@ -1108,7 +1129,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.51.0

[PATCH v1 4/5] platform/x86: fujitsu: Convert backlight driver to a platform one

[Rafael J. Wysocki]

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

In all cases in which a struct acpi_driver is used for binding a driver
to an ACPI device object, a corresponding platform device is created by
the ACPI core and that device is regarded as a proper representation of
underlying hardware.  Accordingly, a struct platform_driver should be
used by driver code to bind to that device.  There are multiple reasons
why drivers should not bind directly to ACPI device objects [1].

Overall, it is better to bind drivers to platform devices than to their
ACPI companions, so convert the backlight part of the Fujitsu laptop
driver from an ACPI driver to a platform one.

After this change, the backlight and input subordinate devices will be
registered under the platform device used for driver binding instead of
its ACPI companion.

While this is not expected to alter functionality, it changes sysfs
layout and so it will be visible to user space.

Link: https://lore.kernel.org/all/2396510.ElGaqSPkdT@rafael.j.wysocki/ [1]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/platform/x86/fujitsu-laptop.c | 117 +++++++++++++-------------
 1 file changed, 58 insertions(+), 59 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index cb13c06b8f35..d4550309ac13 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -284,15 +284,16 @@ static void fujitsu_battery_charge_control_remove(struct acpi_device *device)
 
 /* Hardware access for LCD brightness control */
 
-static int set_lcd_level(struct acpi_device *device, int level)
+static int set_lcd_level(struct device *dev, int level)
 {
-	struct fujitsu_bl *priv = acpi_driver_data(device);
+	struct fujitsu_bl *priv = dev_get_drvdata(dev);
+	acpi_handle handle = ACPI_HANDLE(dev);
 	acpi_status status;
 	char *method;
 
 	switch (use_alt_lcd_levels) {
 	case -1:
-		if (acpi_has_method(device->handle, "SBL2"))
+		if (acpi_has_method(handle, "SBL2"))
 			method = "SBL2";
 		else
 			method = "SBLL";
@@ -305,16 +306,14 @@ static int set_lcd_level(struct acpi_device *device, int level)
 		break;
 	}
 
-	acpi_handle_debug(device->handle, "set lcd level via %s [%d]\n", method,
-			  level);
+	acpi_handle_debug(handle, "set lcd level via %s [%d]\n", method, level);
 
 	if (level < 0 || level >= priv->max_brightness)
 		return -EINVAL;
 
-	status = acpi_execute_simple_method(device->handle, method, level);
+	status = acpi_execute_simple_method(handle, method, level);
 	if (ACPI_FAILURE(status)) {
-		acpi_handle_err(device->handle, "Failed to evaluate %s\n",
-				method);
+		acpi_handle_err(handle, "Failed to evaluate %s\n", method);
 		return -ENODEV;
 	}
 
@@ -323,15 +322,16 @@ static int set_lcd_level(struct acpi_device *device, int level)
 	return 0;
 }
 
-static int get_lcd_level(struct acpi_device *device)
+static int get_lcd_level(struct device *dev)
 {
-	struct fujitsu_bl *priv = acpi_driver_data(device);
+	struct fujitsu_bl *priv = dev_get_drvdata(dev);
+	acpi_handle handle = ACPI_HANDLE(dev);
 	unsigned long long state = 0;
 	acpi_status status = AE_OK;
 
-	acpi_handle_debug(device->handle, "get lcd level via GBLL\n");
+	acpi_handle_debug(handle, "get lcd level via GBLL\n");
 
-	status = acpi_evaluate_integer(device->handle, "GBLL", NULL, &state);
+	status = acpi_evaluate_integer(handle, "GBLL", NULL, &state);
 	if (ACPI_FAILURE(status))
 		return 0;
 
@@ -340,15 +340,16 @@ static int get_lcd_level(struct acpi_device *device)
 	return priv->brightness_level;
 }
 
-static int get_max_brightness(struct acpi_device *device)
+static int get_max_brightness(struct device *dev)
 {
-	struct fujitsu_bl *priv = acpi_driver_data(device);
+	struct fujitsu_bl *priv = dev_get_drvdata(dev);
+	acpi_handle handle = ACPI_HANDLE(dev);
 	unsigned long long state = 0;
 	acpi_status status = AE_OK;
 
-	acpi_handle_debug(device->handle, "get max lcd level via RBLL\n");
+	acpi_handle_debug(handle, "get max lcd level via RBLL\n");
 
-	status = acpi_evaluate_integer(device->handle, "RBLL", NULL, &state);
+	status = acpi_evaluate_integer(handle, "RBLL", NULL, &state);
 	if (ACPI_FAILURE(status))
 		return -1;
 
@@ -361,15 +362,13 @@ static int get_max_brightness(struct acpi_device *device)
 
 static int bl_get_brightness(struct backlight_device *b)
 {
-	struct acpi_device *device = bl_get_data(b);
+	struct device *dev = bl_get_data(b);
 
-	return b->props.power == BACKLIGHT_POWER_OFF ? 0 : get_lcd_level(device);
+	return b->props.power == BACKLIGHT_POWER_OFF ? 0 : get_lcd_level(dev);
 }
 
 static int bl_update_status(struct backlight_device *b)
 {
-	struct acpi_device *device = bl_get_data(b);
-
 	if (fext) {
 		if (b->props.power == BACKLIGHT_POWER_OFF)
 			call_fext_func(fext, FUNC_BACKLIGHT, 0x1,
@@ -379,7 +378,7 @@ static int bl_update_status(struct backlight_device *b)
 				       BACKLIGHT_PARAM_POWER, BACKLIGHT_ON);
 	}
 
-	return set_lcd_level(device, b->props.brightness);
+	return set_lcd_level(bl_get_data(b), b->props.brightness);
 }
 
 static const struct backlight_ops fujitsu_bl_ops = {
@@ -455,12 +454,13 @@ static const struct key_entry keymap_backlight[] = {
 	{ KE_END, 0 }
 };
 
-static int acpi_fujitsu_bl_input_setup(struct acpi_device *device)
+static int acpi_fujitsu_bl_input_setup(struct device *dev)
 {
-	struct fujitsu_bl *priv = acpi_driver_data(device);
+	struct fujitsu_bl *priv = dev_get_drvdata(dev);
+	struct acpi_device *device = ACPI_COMPANION(dev);
 	int ret;
 
-	priv->input = devm_input_allocate_device(&device->dev);
+	priv->input = devm_input_allocate_device(dev);
 	if (!priv->input)
 		return -ENOMEM;
 
@@ -479,9 +479,9 @@ static int acpi_fujitsu_bl_input_setup(struct acpi_device *device)
 	return input_register_device(priv->input);
 }
 
-static int fujitsu_backlight_register(struct acpi_device *device)
+static int fujitsu_backlight_register(struct device *dev)
 {
-	struct fujitsu_bl *priv = acpi_driver_data(device);
+	struct fujitsu_bl *priv = dev_get_drvdata(dev);
 	const struct backlight_properties props = {
 		.brightness = priv->brightness_level,
 		.max_brightness = priv->max_brightness - 1,
@@ -489,9 +489,8 @@ static int fujitsu_backlight_register(struct acpi_device *device)
 	};
 	struct backlight_device *bd;
 
-	bd = devm_backlight_device_register(&device->dev, "fujitsu-laptop",
-					    &device->dev, device,
-					    &fujitsu_bl_ops, &props);
+	bd = devm_backlight_device_register(dev, "fujitsu-laptop",
+					    dev, dev, &fujitsu_bl_ops, &props);
 	if (IS_ERR(bd))
 		return PTR_ERR(bd);
 
@@ -504,73 +503,74 @@ static int fujitsu_backlight_register(struct acpi_device *device)
 
 static void acpi_fujitsu_bl_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct acpi_device *device = data;
-	struct fujitsu_bl *priv = acpi_driver_data(device);
+	struct device *dev = data;
+	struct fujitsu_bl *priv = dev_get_drvdata(dev);
 	int oldb, newb;
 
 	if (event != ACPI_FUJITSU_NOTIFY_CODE) {
-		acpi_handle_info(device->handle, "unsupported event [0x%x]\n",
-				 event);
+		acpi_handle_info(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);
+	get_lcd_level(dev);
 	newb = priv->brightness_level;
 
-	acpi_handle_debug(device->handle,
-			  "brightness button event [%i -> %i]\n", oldb, newb);
+	acpi_handle_debug(handle, "brightness button event [%i -> %i]\n",
+			  oldb, newb);
 
 	if (oldb == newb)
 		return;
 
 	if (!disable_brightness_adjust)
-		set_lcd_level(device, newb);
+		set_lcd_level(dev, newb);
 
 	sparse_keymap_report_event(priv->input, oldb < newb, 1, true);
 }
 
-static int acpi_fujitsu_bl_add(struct acpi_device *device)
+static int acpi_fujitsu_bl_probe(struct platform_device *pdev)
 {
+	struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
 	struct fujitsu_bl *priv;
 	int ret;
 
 	if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
 		return -ENODEV;
 
-	priv = devm_kzalloc(&device->dev, sizeof(*priv), GFP_KERNEL);
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
 	fujitsu_bl = priv;
 	strscpy(acpi_device_name(device), ACPI_FUJITSU_BL_DEVICE_NAME);
 	strscpy(acpi_device_class(device), ACPI_FUJITSU_CLASS);
-	device->driver_data = priv;
+
+	platform_set_drvdata(pdev, priv);
 
 	pr_info("ACPI: %s [%s]\n",
 		acpi_device_name(device), acpi_device_bid(device));
 
-	if (get_max_brightness(device) <= 0)
+	if (get_max_brightness(&pdev->dev) <= 0)
 		priv->max_brightness = FUJITSU_LCD_N_LEVELS;
-	get_lcd_level(device);
+	get_lcd_level(&pdev->dev);
 
-	ret = acpi_fujitsu_bl_input_setup(device);
+	ret = acpi_fujitsu_bl_input_setup(&pdev->dev);
 	if (ret)
 		return ret;
 
-	ret = fujitsu_backlight_register(device);
+	ret = fujitsu_backlight_register(&pdev->dev);
 	if (ret)
 		return ret;
 
 	return acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
-					       acpi_fujitsu_bl_notify, device);
+					       acpi_fujitsu_bl_notify, &pdev->dev);
 }
 
-static void acpi_fujitsu_bl_remove(struct acpi_device *device)
+static void acpi_fujitsu_bl_remove(struct platform_device *pdev)
 {
-	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
-				       acpi_fujitsu_bl_notify);
+	acpi_dev_remove_notify_handler(ACPI_COMPANION(&pdev->dev),
+				       ACPI_DEVICE_NOTIFY, acpi_fujitsu_bl_notify);
 }
 
 /* ACPI device for hotkey handling */
@@ -1107,14 +1107,13 @@ static const struct acpi_device_id fujitsu_bl_device_ids[] = {
 	{"", 0},
 };
 
-static struct acpi_driver acpi_fujitsu_bl_driver = {
-	.name = ACPI_FUJITSU_BL_DRIVER_NAME,
-	.class = ACPI_FUJITSU_CLASS,
-	.ids = fujitsu_bl_device_ids,
-	.ops = {
-		.add = acpi_fujitsu_bl_add,
-		.remove = acpi_fujitsu_bl_remove,
-		},
+static struct platform_driver acpi_fujitsu_bl_driver = {
+	.probe = acpi_fujitsu_bl_probe,
+	.remove = acpi_fujitsu_bl_remove,
+	.driver = {
+		.name = ACPI_FUJITSU_BL_DRIVER_NAME,
+		.acpi_match_table = fujitsu_bl_device_ids,
+	},
 };
 
 static const struct acpi_device_id fujitsu_laptop_device_ids[] = {
@@ -1143,7 +1142,7 @@ static int __init fujitsu_init(void)
 {
 	int ret;
 
-	ret = acpi_bus_register_driver(&acpi_fujitsu_bl_driver);
+	ret = platform_driver_register(&acpi_fujitsu_bl_driver);
 	if (ret)
 		return ret;
 
@@ -1166,7 +1165,7 @@ static int __init fujitsu_init(void)
 err_unregister_platform_driver:
 	platform_driver_unregister(&fujitsu_pf_driver);
 err_unregister_acpi:
-	acpi_bus_unregister_driver(&acpi_fujitsu_bl_driver);
+	platform_driver_unregister(&acpi_fujitsu_bl_driver);
 
 	return ret;
 }
@@ -1177,7 +1176,7 @@ static void __exit fujitsu_cleanup(void)
 
 	platform_driver_unregister(&fujitsu_pf_driver);
 
-	acpi_bus_unregister_driver(&acpi_fujitsu_bl_driver);
+	platform_driver_unregister(&acpi_fujitsu_bl_driver);
 
 	pr_info("driver unloaded\n");
 }
-- 
2.51.0

[PATCH v1 5/5] platform/x86: fujitsu: Convert laptop driver to a platform one

[Rafael J. Wysocki]

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

In all cases in which a struct acpi_driver is used for binding a driver
to an ACPI device object, a corresponding platform device is created by
the ACPI core and that device is regarded as a proper representation of
underlying hardware.  Accordingly, a struct platform_driver should be
used by driver code to bind to that device.  There are multiple reasons
why drivers should not bind directly to ACPI device objects [1].

Overall, it is better to bind drivers to platform devices than to their
ACPI companions, so convert the main part of the Fujitsu laptop driver
from an ACPI driver to a platform one.

After this change, the subordinate LED and input devices will be
registered under the platform device used for driver binding instead of
its ACPI companion.

While this is not expected to alter functionality, it changes sysfs
layout and so it will be visible to user space.

Link: https://lore.kernel.org/all/2396510.ElGaqSPkdT@rafael.j.wysocki/ [1]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/platform/x86/fujitsu-laptop.c | 200 +++++++++++++-------------
 1 file changed, 96 insertions(+), 104 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index d4550309ac13..27621e06067b 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -144,11 +144,11 @@ struct fujitsu_laptop {
 	bool charge_control_supported;
 };
 
-static struct acpi_device *fext;
+static struct device *fext;
 
 /* Fujitsu ACPI interface function */
 
-static int call_fext_func(struct acpi_device *device,
+static int call_fext_func(struct device *dev,
 			  int func, int op, int feature, int state)
 {
 	union acpi_object params[4] = {
@@ -158,18 +158,17 @@ static int call_fext_func(struct acpi_device *device,
 		{ .integer.type = ACPI_TYPE_INTEGER, .integer.value = state }
 	};
 	struct acpi_object_list arg_list = { 4, params };
+	acpi_handle handle = ACPI_HANDLE(dev);
 	unsigned long long value;
 	acpi_status status;
 
-	status = acpi_evaluate_integer(device->handle, "FUNC", &arg_list,
-				       &value);
+	status = acpi_evaluate_integer(handle, "FUNC", &arg_list, &value);
 	if (ACPI_FAILURE(status)) {
-		acpi_handle_err(device->handle, "Failed to evaluate FUNC\n");
+		acpi_handle_err(handle, "Failed to evaluate FUNC\n");
 		return -ENODEV;
 	}
 
-	acpi_handle_debug(device->handle,
-			  "FUNC 0x%x (args 0x%x, 0x%x, 0x%x) returned 0x%x\n",
+	acpi_handle_debug(handle, "FUNC 0x%x (args 0x%x, 0x%x, 0x%x) returned 0x%x\n",
 			  func, op, feature, state, (int)value);
 	return value;
 }
@@ -251,9 +250,9 @@ static struct acpi_battery_hook battery_hook = {
  * These functions are intended to be called from acpi_fujitsu_laptop_add and
  * acpi_fujitsu_laptop_remove.
  */
-static int fujitsu_battery_charge_control_add(struct acpi_device *device)
+static int fujitsu_battery_charge_control_add(struct device *dev)
 {
-	struct fujitsu_laptop *priv = acpi_driver_data(device);
+	struct fujitsu_laptop *priv = dev_get_drvdata(dev);
 	int s006_cc_return;
 
 	priv->charge_control_supported = false;
@@ -274,9 +273,9 @@ static int fujitsu_battery_charge_control_add(struct acpi_device *device)
 	return 0;
 }
 
-static void fujitsu_battery_charge_control_remove(struct acpi_device *device)
+static void fujitsu_battery_charge_control_remove(struct device *dev)
 {
-	struct fujitsu_laptop *priv = acpi_driver_data(device);
+	struct fujitsu_laptop *priv = dev_get_drvdata(dev);
 
 	if (priv->charge_control_supported)
 		battery_hook_unregister(&battery_hook);
@@ -665,12 +664,13 @@ static const struct dmi_system_id fujitsu_laptop_dmi_table[] = {
 	{}
 };
 
-static int acpi_fujitsu_laptop_input_setup(struct acpi_device *device)
+static int acpi_fujitsu_laptop_input_setup(struct device *dev)
 {
-	struct fujitsu_laptop *priv = acpi_driver_data(device);
+	struct fujitsu_laptop *priv = dev_get_drvdata(dev);
+	struct acpi_device *device = ACPI_COMPANION(dev);
 	int ret;
 
-	priv->input = devm_input_allocate_device(&device->dev);
+	priv->input = devm_input_allocate_device(dev);
 	if (!priv->input)
 		return -ENOMEM;
 
@@ -689,9 +689,9 @@ static int acpi_fujitsu_laptop_input_setup(struct acpi_device *device)
 	return input_register_device(priv->input);
 }
 
-static int fujitsu_laptop_platform_add(struct acpi_device *device)
+static int fujitsu_laptop_platform_add(struct device *dev)
 {
-	struct fujitsu_laptop *priv = acpi_driver_data(device);
+	struct fujitsu_laptop *priv = dev_get_drvdata(dev);
 	int ret;
 
 	priv->pf_device = platform_device_alloc("fujitsu-laptop", PLATFORM_DEVID_NONE);
@@ -719,9 +719,9 @@ static int fujitsu_laptop_platform_add(struct acpi_device *device)
 	return ret;
 }
 
-static void fujitsu_laptop_platform_remove(struct acpi_device *device)
+static void fujitsu_laptop_platform_remove(struct device *dev)
 {
-	struct fujitsu_laptop *priv = acpi_driver_data(device);
+	struct fujitsu_laptop *priv = dev_get_drvdata(dev);
 
 	sysfs_remove_group(&priv->pf_device->dev.kobj,
 			   &fujitsu_pf_attribute_group);
@@ -731,7 +731,7 @@ static void fujitsu_laptop_platform_remove(struct acpi_device *device)
 static int logolamp_set(struct led_classdev *cdev,
 			enum led_brightness brightness)
 {
-	struct acpi_device *device = to_acpi_device(cdev->dev->parent);
+	struct device *parent = cdev->dev->parent;
 	int poweron = FUNC_LED_ON, always = FUNC_LED_ON;
 	int ret;
 
@@ -741,23 +741,23 @@ static int logolamp_set(struct led_classdev *cdev,
 	if (brightness < LED_FULL)
 		always = FUNC_LED_OFF;
 
-	ret = call_fext_func(device, FUNC_LEDS, 0x1, LOGOLAMP_POWERON, poweron);
+	ret = call_fext_func(parent, FUNC_LEDS, 0x1, LOGOLAMP_POWERON, poweron);
 	if (ret < 0)
 		return ret;
 
-	return call_fext_func(device, FUNC_LEDS, 0x1, LOGOLAMP_ALWAYS, always);
+	return call_fext_func(parent, FUNC_LEDS, 0x1, LOGOLAMP_ALWAYS, always);
 }
 
 static enum led_brightness logolamp_get(struct led_classdev *cdev)
 {
-	struct acpi_device *device = to_acpi_device(cdev->dev->parent);
+	struct device *parent = cdev->dev->parent;
 	int ret;
 
-	ret = call_fext_func(device, FUNC_LEDS, 0x2, LOGOLAMP_ALWAYS, 0x0);
+	ret = call_fext_func(parent, FUNC_LEDS, 0x2, LOGOLAMP_ALWAYS, 0x0);
 	if (ret == FUNC_LED_ON)
 		return LED_FULL;
 
-	ret = call_fext_func(device, FUNC_LEDS, 0x2, LOGOLAMP_POWERON, 0x0);
+	ret = call_fext_func(parent, FUNC_LEDS, 0x2, LOGOLAMP_POWERON, 0x0);
 	if (ret == FUNC_LED_ON)
 		return LED_HALF;
 
@@ -767,22 +767,21 @@ static enum led_brightness logolamp_get(struct led_classdev *cdev)
 static int kblamps_set(struct led_classdev *cdev,
 		       enum led_brightness brightness)
 {
-	struct acpi_device *device = to_acpi_device(cdev->dev->parent);
+	struct device *parent = cdev->dev->parent;
 
 	if (brightness >= LED_FULL)
-		return call_fext_func(device, FUNC_LEDS, 0x1, KEYBOARD_LAMPS,
+		return call_fext_func(parent, FUNC_LEDS, 0x1, KEYBOARD_LAMPS,
 				      FUNC_LED_ON);
 	else
-		return call_fext_func(device, FUNC_LEDS, 0x1, KEYBOARD_LAMPS,
+		return call_fext_func(parent, FUNC_LEDS, 0x1, KEYBOARD_LAMPS,
 				      FUNC_LED_OFF);
 }
 
 static enum led_brightness kblamps_get(struct led_classdev *cdev)
 {
-	struct acpi_device *device = to_acpi_device(cdev->dev->parent);
 	enum led_brightness brightness = LED_OFF;
 
-	if (call_fext_func(device,
+	if (call_fext_func(cdev->dev->parent,
 			   FUNC_LEDS, 0x2, KEYBOARD_LAMPS, 0x0) == FUNC_LED_ON)
 		brightness = LED_FULL;
 
@@ -792,22 +791,22 @@ static enum led_brightness kblamps_get(struct led_classdev *cdev)
 static int radio_led_set(struct led_classdev *cdev,
 			 enum led_brightness brightness)
 {
-	struct acpi_device *device = to_acpi_device(cdev->dev->parent);
+	struct device *parent = cdev->dev->parent;
 
 	if (brightness >= LED_FULL)
-		return call_fext_func(device, FUNC_FLAGS, 0x5, RADIO_LED_ON,
+		return call_fext_func(parent, FUNC_FLAGS, 0x5, RADIO_LED_ON,
 				      RADIO_LED_ON);
 	else
-		return call_fext_func(device, FUNC_FLAGS, 0x5, RADIO_LED_ON,
+		return call_fext_func(parent, FUNC_FLAGS, 0x5, RADIO_LED_ON,
 				      0x0);
 }
 
 static enum led_brightness radio_led_get(struct led_classdev *cdev)
 {
-	struct acpi_device *device = to_acpi_device(cdev->dev->parent);
+	struct device *parent = cdev->dev->parent;
 	enum led_brightness brightness = LED_OFF;
 
-	if (call_fext_func(device, FUNC_FLAGS, 0x4, 0x0, 0x0) & RADIO_LED_ON)
+	if (call_fext_func(parent, FUNC_FLAGS, 0x4, 0x0, 0x0) & RADIO_LED_ON)
 		brightness = LED_FULL;
 
 	return brightness;
@@ -816,60 +815,58 @@ static enum led_brightness radio_led_get(struct led_classdev *cdev)
 static int eco_led_set(struct led_classdev *cdev,
 		       enum led_brightness brightness)
 {
-	struct acpi_device *device = to_acpi_device(cdev->dev->parent);
+	struct device *parent = cdev->dev->parent;
 	int curr;
 
-	curr = call_fext_func(device, FUNC_LEDS, 0x2, ECO_LED, 0x0);
+	curr = call_fext_func(parent, FUNC_LEDS, 0x2, ECO_LED, 0x0);
 	if (brightness >= LED_FULL)
-		return call_fext_func(device, FUNC_LEDS, 0x1, ECO_LED,
+		return call_fext_func(parent, FUNC_LEDS, 0x1, ECO_LED,
 				      curr | ECO_LED_ON);
 	else
-		return call_fext_func(device, FUNC_LEDS, 0x1, ECO_LED,
+		return call_fext_func(parent, FUNC_LEDS, 0x1, ECO_LED,
 				      curr & ~ECO_LED_ON);
 }
 
 static enum led_brightness eco_led_get(struct led_classdev *cdev)
 {
-	struct acpi_device *device = to_acpi_device(cdev->dev->parent);
+	struct device *parent = cdev->dev->parent;
 	enum led_brightness brightness = LED_OFF;
 
-	if (call_fext_func(device, FUNC_LEDS, 0x2, ECO_LED, 0x0) & ECO_LED_ON)
+	if (call_fext_func(parent, FUNC_LEDS, 0x2, ECO_LED, 0x0) & ECO_LED_ON)
 		brightness = LED_FULL;
 
 	return brightness;
 }
 
-static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device)
+static int acpi_fujitsu_laptop_leds_register(struct device *dev)
 {
-	struct fujitsu_laptop *priv = acpi_driver_data(device);
+	struct fujitsu_laptop *priv = dev_get_drvdata(dev);
 	struct led_classdev *led;
 	int ret;
 
-	if (call_fext_func(device,
-			   FUNC_LEDS, 0x0, 0x0, 0x0) & LOGOLAMP_POWERON) {
-		led = devm_kzalloc(&device->dev, sizeof(*led), GFP_KERNEL);
+	if (call_fext_func(dev, FUNC_LEDS, 0x0, 0x0, 0x0) & LOGOLAMP_POWERON) {
+		led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
 		if (!led)
 			return -ENOMEM;
 
 		led->name = "fujitsu::logolamp";
 		led->brightness_set_blocking = logolamp_set;
 		led->brightness_get = logolamp_get;
-		ret = devm_led_classdev_register(&device->dev, led);
+		ret = devm_led_classdev_register(dev, led);
 		if (ret)
 			return ret;
 	}
 
-	if ((call_fext_func(device,
-			    FUNC_LEDS, 0x0, 0x0, 0x0) & KEYBOARD_LAMPS) &&
-	    (call_fext_func(device, FUNC_BUTTONS, 0x0, 0x0, 0x0) == 0x0)) {
-		led = devm_kzalloc(&device->dev, sizeof(*led), GFP_KERNEL);
+	if ((call_fext_func(dev, FUNC_LEDS, 0x0, 0x0, 0x0) & KEYBOARD_LAMPS) &&
+	    (call_fext_func(dev, FUNC_BUTTONS, 0x0, 0x0, 0x0) == 0x0)) {
+		led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
 		if (!led)
 			return -ENOMEM;
 
 		led->name = "fujitsu::kblamps";
 		led->brightness_set_blocking = kblamps_set;
 		led->brightness_get = kblamps_get;
-		ret = devm_led_classdev_register(&device->dev, led);
+		ret = devm_led_classdev_register(dev, led);
 		if (ret)
 			return ret;
 	}
@@ -884,7 +881,7 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device)
 	 * whether given model has a radio toggle button.
 	 */
 	if (priv->flags_supported & BIT(17)) {
-		led = devm_kzalloc(&device->dev, sizeof(*led), GFP_KERNEL);
+		led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
 		if (!led)
 			return -ENOMEM;
 
@@ -892,7 +889,7 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device)
 		led->brightness_set_blocking = radio_led_set;
 		led->brightness_get = radio_led_get;
 		led->default_trigger = "rfkill-any";
-		ret = devm_led_classdev_register(&device->dev, led);
+		ret = devm_led_classdev_register(dev, led);
 		if (ret)
 			return ret;
 	}
@@ -902,17 +899,16 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device)
 	 * bit 14 seems to indicate presence of said led as well.
 	 * Confirm by testing the status.
 	 */
-	if ((call_fext_func(device, FUNC_LEDS, 0x0, 0x0, 0x0) & BIT(14)) &&
-	    (call_fext_func(device,
-			    FUNC_LEDS, 0x2, ECO_LED, 0x0) != UNSUPPORTED_CMD)) {
-		led = devm_kzalloc(&device->dev, sizeof(*led), GFP_KERNEL);
+	if ((call_fext_func(dev, FUNC_LEDS, 0x0, 0x0, 0x0) & BIT(14)) &&
+	    (call_fext_func(dev, FUNC_LEDS, 0x2, ECO_LED, 0x0) != UNSUPPORTED_CMD)) {
+		led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
 		if (!led)
 			return -ENOMEM;
 
 		led->name = "fujitsu::eco_led";
 		led->brightness_set_blocking = eco_led_set;
 		led->brightness_get = eco_led_get;
-		ret = devm_led_classdev_register(&device->dev, led);
+		ret = devm_led_classdev_register(dev, led);
 		if (ret)
 			return ret;
 	}
@@ -920,9 +916,9 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device)
 	return 0;
 }
 
-static void acpi_fujitsu_laptop_press(struct acpi_device *device, int scancode)
+static void acpi_fujitsu_laptop_press(struct device *dev, int scancode)
 {
-	struct fujitsu_laptop *priv = acpi_driver_data(device);
+	struct fujitsu_laptop *priv = dev_get_drvdata(dev);
 	int ret;
 
 	ret = kfifo_in_locked(&priv->fifo, (unsigned char *)&scancode,
@@ -937,9 +933,9 @@ static void acpi_fujitsu_laptop_press(struct acpi_device *device, int scancode)
 		scancode);
 }
 
-static void acpi_fujitsu_laptop_release(struct acpi_device *device)
+static void acpi_fujitsu_laptop_release(struct device *dev)
 {
-	struct fujitsu_laptop *priv = acpi_driver_data(device);
+	struct fujitsu_laptop *priv = dev_get_drvdata(dev);
 	int scancode, ret;
 
 	while (true) {
@@ -955,34 +951,30 @@ static void acpi_fujitsu_laptop_release(struct acpi_device *device)
 
 static void acpi_fujitsu_laptop_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct acpi_device *device = data;
-	struct fujitsu_laptop *priv = acpi_driver_data(device);
+	struct device *dev = data;
+	struct fujitsu_laptop *priv = dev_get_drvdata(dev);
 	unsigned long flags;
 	int scancode, i = 0;
 	unsigned int irb;
 
 	if (event != ACPI_FUJITSU_NOTIFY_CODE) {
-		acpi_handle_info(device->handle, "Unsupported event [0x%x]\n",
-				 event);
+		acpi_handle_info(handle, "Unsupported event [0x%x]\n", event);
 		sparse_keymap_report_event(priv->input, -1, 1, true);
 		return;
 	}
 
 	if (priv->flags_supported)
-		priv->flags_state = call_fext_func(device, FUNC_FLAGS, 0x4, 0x0,
-						   0x0);
+		priv->flags_state = call_fext_func(dev, FUNC_FLAGS, 0x4, 0x0, 0x0);
 
-	while ((irb = call_fext_func(device,
-				     FUNC_BUTTONS, 0x1, 0x0, 0x0)) != 0 &&
+	while ((irb = call_fext_func(dev, FUNC_BUTTONS, 0x1, 0x0, 0x0)) != 0 &&
 	       i++ < MAX_HOTKEY_RINGBUFFER_SIZE) {
 		scancode = irb & 0x4ff;
 		if (sparse_keymap_entry_from_scancode(priv->input, scancode))
-			acpi_fujitsu_laptop_press(device, scancode);
+			acpi_fujitsu_laptop_press(dev, scancode);
 		else if (scancode == 0)
-			acpi_fujitsu_laptop_release(device);
+			acpi_fujitsu_laptop_release(dev);
 		else
-			acpi_handle_info(device->handle,
-					 "Unknown GIRB result [%x]\n", irb);
+			acpi_handle_info(handle, "Unknown GIRB result [%x]\n", irb);
 	}
 
 	/*
@@ -992,28 +984,30 @@ static void acpi_fujitsu_laptop_notify(acpi_handle handle, u32 event, void *data
 	 * status flags queried using FUNC_FLAGS.
 	 */
 	if (priv->flags_supported & (FLAG_SOFTKEYS)) {
-		flags = call_fext_func(device, FUNC_FLAGS, 0x1, 0x0, 0x0);
+		flags = call_fext_func(dev, FUNC_FLAGS, 0x1, 0x0, 0x0);
 		flags &= (FLAG_SOFTKEYS);
 		for_each_set_bit(i, &flags, BITS_PER_LONG)
 			sparse_keymap_report_event(priv->input, BIT(i), 1, true);
 	}
 }
 
-static int acpi_fujitsu_laptop_add(struct acpi_device *device)
+static int acpi_fujitsu_laptop_probe(struct platform_device *pdev)
 {
+	struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
 	struct fujitsu_laptop *priv;
 	int ret, i = 0;
 
-	priv = devm_kzalloc(&device->dev, sizeof(*priv), GFP_KERNEL);
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
 	WARN_ONCE(fext, "More than one FUJ02E3 ACPI device was found.  Driver may not work as intended.");
-	fext = device;
+	fext = &pdev->dev;
 
 	strscpy(acpi_device_name(device), ACPI_FUJITSU_LAPTOP_DEVICE_NAME);
 	strscpy(acpi_device_class(device), ACPI_FUJITSU_CLASS);
-	device->driver_data = priv;
+
+	platform_set_drvdata(pdev, priv);
 
 	/* kfifo */
 	spin_lock_init(&priv->fifo_lock);
@@ -1025,14 +1019,13 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
 	pr_info("ACPI: %s [%s]\n",
 		acpi_device_name(device), acpi_device_bid(device));
 
-	while (call_fext_func(device, FUNC_BUTTONS, 0x1, 0x0, 0x0) != 0 &&
+	while (call_fext_func(fext, FUNC_BUTTONS, 0x1, 0x0, 0x0) != 0 &&
 	       i++ < MAX_HOTKEY_RINGBUFFER_SIZE)
 		; /* No action, result is discarded */
 	acpi_handle_debug(device->handle, "Discarded %i ringbuffer entries\n",
 			  i);
 
-	priv->flags_supported = call_fext_func(device, FUNC_FLAGS, 0x0, 0x0,
-					       0x0);
+	priv->flags_supported = call_fext_func(fext, FUNC_FLAGS, 0x0, 0x0, 0x0);
 
 	/* Make sure our bitmask of supported functions is cleared if the
 	   RFKILL function block is not implemented, like on the S7020. */
@@ -1040,12 +1033,12 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
 		priv->flags_supported = 0;
 
 	if (priv->flags_supported)
-		priv->flags_state = call_fext_func(device, FUNC_FLAGS, 0x4, 0x0,
+		priv->flags_state = call_fext_func(fext, FUNC_FLAGS, 0x4, 0x0,
 						   0x0);
 
 	/* Suspect this is a keymap of the application panel, print it */
 	acpi_handle_info(device->handle, "BTNI: [0x%x]\n",
-			 call_fext_func(device, FUNC_BUTTONS, 0x0, 0x0, 0x0));
+			 call_fext_func(fext, FUNC_BUTTONS, 0x0, 0x0, 0x0));
 
 	/* Sync backlight power status */
 	if (fujitsu_bl && fujitsu_bl->bl_device &&
@@ -1057,24 +1050,24 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
 			fujitsu_bl->bl_device->props.power = BACKLIGHT_POWER_ON;
 	}
 
-	ret = acpi_fujitsu_laptop_input_setup(device);
+	ret = acpi_fujitsu_laptop_input_setup(fext);
 	if (ret)
 		goto err_free_fifo;
 
-	ret = acpi_fujitsu_laptop_leds_register(device);
+	ret = acpi_fujitsu_laptop_leds_register(fext);
 	if (ret)
 		goto err_free_fifo;
 
-	ret = fujitsu_laptop_platform_add(device);
+	ret = fujitsu_laptop_platform_add(fext);
 	if (ret)
 		goto err_free_fifo;
 
 	ret = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
-					      acpi_fujitsu_laptop_notify, device);
+					      acpi_fujitsu_laptop_notify, fext);
 	if (ret)
 		goto err_free_fifo;
 
-	ret = fujitsu_battery_charge_control_add(device);
+	ret = fujitsu_battery_charge_control_add(fext);
 	if (ret < 0)
 		pr_warn("Unable to register battery charge control: %d\n", ret);
 
@@ -1086,16 +1079,16 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
 	return ret;
 }
 
-static void acpi_fujitsu_laptop_remove(struct acpi_device *device)
+static void acpi_fujitsu_laptop_remove(struct platform_device *pdev)
 {
-	struct fujitsu_laptop *priv = acpi_driver_data(device);
+	struct fujitsu_laptop *priv = platform_get_drvdata(pdev);
 
-	fujitsu_battery_charge_control_remove(device);
+	fujitsu_battery_charge_control_remove(&pdev->dev);
 
-	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
+	acpi_dev_remove_notify_handler(ACPI_COMPANION(&pdev->dev), ACPI_DEVICE_NOTIFY,
 				       acpi_fujitsu_laptop_notify);
 
-	fujitsu_laptop_platform_remove(device);
+	fujitsu_laptop_platform_remove(&pdev->dev);
 
 	kfifo_free(&priv->fifo);
 }
@@ -1121,14 +1114,13 @@ static const struct acpi_device_id fujitsu_laptop_device_ids[] = {
 	{"", 0},
 };
 
-static struct acpi_driver acpi_fujitsu_laptop_driver = {
-	.name = ACPI_FUJITSU_LAPTOP_DRIVER_NAME,
-	.class = ACPI_FUJITSU_CLASS,
-	.ids = fujitsu_laptop_device_ids,
-	.ops = {
-		.add = acpi_fujitsu_laptop_add,
-		.remove = acpi_fujitsu_laptop_remove,
-		},
+static struct platform_driver acpi_fujitsu_laptop_driver = {
+	.probe = acpi_fujitsu_laptop_probe,
+	.remove = acpi_fujitsu_laptop_remove,
+	.driver = {
+		.name = ACPI_FUJITSU_LAPTOP_DRIVER_NAME,
+		.acpi_match_table = fujitsu_laptop_device_ids,
+	},
 };
 
 static const struct acpi_device_id fujitsu_ids[] __used = {
@@ -1154,7 +1146,7 @@ static int __init fujitsu_init(void)
 
 	/* Register laptop driver */
 
-	ret = acpi_bus_register_driver(&acpi_fujitsu_laptop_driver);
+	ret = platform_driver_register(&acpi_fujitsu_laptop_driver);
 	if (ret)
 		goto err_unregister_platform_driver;
 
@@ -1172,7 +1164,7 @@ static int __init fujitsu_init(void)
 
 static void __exit fujitsu_cleanup(void)
 {
-	acpi_bus_unregister_driver(&acpi_fujitsu_laptop_driver);
+	platform_driver_unregister(&acpi_fujitsu_laptop_driver);
 
 	platform_driver_unregister(&fujitsu_pf_driver);
 
-- 
2.51.0

Re: [PATCH v1 0/5] platform/x86: fujitsu: Bind drivers to platform devices instead of ACPI ones

[Jonathan Woithe]

Hi Rafael

On Wed, Feb 25, 2026 at 09:41:09PM +0100, Rafael J. Wysocki wrote:
> This series is part of a larger effort to switch over all drivers using
> the struct acpi_driver interface to the more common struct platform_driver
> interface and eliminate the former.  The background is explained in
> Documentation/driver-api/acpi/acpi-drivers.rst and in the changelog of
> the patch that introduced the above document:
> 
> https://lore.kernel.org/all/2396510.ElGaqSPkdT@rafael.j.wysocki/
> 
> The bottom line is that the kernel would be better off without struct
> acpi_driver and so it is better to get rid of it.
> 
> This series carries out driver conversions of the platform x86 drivers
> for Fujitsu platforms.
> 
> Patch [1/5] converts the fujitsu-tablet to a proper platform one based
> on the struct platform_driver interface.
> 
> Patch [2/5] rearranges the code in the fujitsu-laptop driver to avoid
> introducing forward declarations of some functions in the subsequent
> patches.
> 
> Patch [3/5] updates the fujitsu-laptop driver to install ACPI notify
> handlers by itself instead of using .notify() callbacks from struct
> acpi_driver, which is requisite for the driver conversion.
> 
> Patches [4-5/5] replaces two instances of the struct acpi_driver
> interface in the fujitsu-laptop driver with struct platform_driver.

Thanks for preparing this patch series.  It looks reasonable to me.  At this
point I have nothing to add.

Acked-by: Jonathan Woithe <jwoithe@just42.net>

Regards
  jonathan

Re: [PATCH v1 3/5] platform/x86: fujitsu: Register ACPI notify handlers directly

[Ilpo Järvinen]

On Wed, 25 Feb 2026, Rafael J. Wysocki wrote:

> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> To facilitate subsequent conversion of the driver to using struct
> platform_driver instead of struct acpi_driver, make it install its ACPI
> notify handlers directly instead of using struct acpi_driver .notify()
> callbacks.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/platform/x86/fujitsu-laptop.c | 30 ++++++++++++++++++++++-----
>  1 file changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> index 1adce90ae3e6..cb13c06b8f35 100644
> --- a/drivers/platform/x86/fujitsu-laptop.c
> +++ b/drivers/platform/x86/fujitsu-laptop.c
> @@ -502,8 +502,9 @@ static int fujitsu_backlight_register(struct acpi_device *device)
>  
>  /* Brightness notify */
>  
> -static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
> +static void acpi_fujitsu_bl_notify(acpi_handle handle, u32 event, void *data)
>  {
> +	struct acpi_device *device = data;
>  	struct fujitsu_bl *priv = acpi_driver_data(device);
>  	int oldb, newb;
>  
> @@ -558,7 +559,18 @@ 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;
> +
> +	return acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> +					       acpi_fujitsu_bl_notify, device);
> +}
> +
> +static void acpi_fujitsu_bl_remove(struct acpi_device *device)
> +{
> +	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> +				       acpi_fujitsu_bl_notify);
>  }
>  
>  /* ACPI device for hotkey handling */
> @@ -941,8 +953,9 @@ 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 acpi_device *device = data;
>  	struct fujitsu_laptop *priv = acpi_driver_data(device);
>  	unsigned long flags;
>  	int scancode, i = 0;
> @@ -1056,6 +1069,11 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
>  	if (ret)
>  		goto err_free_fifo;
>  
> +	ret = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> +					      acpi_fujitsu_laptop_notify, device);
> +	if (ret)
> +		goto err_free_fifo;

Hi Rafael,

Is the rollback path still correct after adding this here?

-- 
 i.

> +
>  	ret = fujitsu_battery_charge_control_add(device);
>  	if (ret < 0)
>  		pr_warn("Unable to register battery charge control: %d\n", ret);
> @@ -1074,6 +1092,9 @@ static void acpi_fujitsu_laptop_remove(struct acpi_device *device)
>  
>  	fujitsu_battery_charge_control_remove(device);
>  
> +	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> +				       acpi_fujitsu_laptop_notify);
> +
>  	fujitsu_laptop_platform_remove(device);
>  
>  	kfifo_free(&priv->fifo);
> @@ -1092,7 +1113,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,
>  		},
>  };
>  
> @@ -1108,7 +1129,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,
>  		},
>  };
>  
> 

Re: [PATCH v1 3/5] platform/x86: fujitsu: Register ACPI notify handlers directly

[Rafael J. Wysocki]

On Monday, March 9, 2026 2:00:31 PM CET Ilpo Järvinen wrote:
> On Wed, 25 Feb 2026, Rafael J. Wysocki wrote:
> 
> > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > 
> > To facilitate subsequent conversion of the driver to using struct
> > platform_driver instead of struct acpi_driver, make it install its ACPI
> > notify handlers directly instead of using struct acpi_driver .notify()
> > callbacks.
> > 
> > No intentional functional impact.
> > 
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  drivers/platform/x86/fujitsu-laptop.c | 30 ++++++++++++++++++++++-----
> >  1 file changed, 25 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> > index 1adce90ae3e6..cb13c06b8f35 100644
> > --- a/drivers/platform/x86/fujitsu-laptop.c
> > +++ b/drivers/platform/x86/fujitsu-laptop.c
> > @@ -502,8 +502,9 @@ static int fujitsu_backlight_register(struct acpi_device *device)
> >  
> >  /* Brightness notify */
> >  
> > -static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
> > +static void acpi_fujitsu_bl_notify(acpi_handle handle, u32 event, void *data)
> >  {
> > +	struct acpi_device *device = data;
> >  	struct fujitsu_bl *priv = acpi_driver_data(device);
> >  	int oldb, newb;
> >  
> > @@ -558,7 +559,18 @@ 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;
> > +
> > +	return acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > +					       acpi_fujitsu_bl_notify, device);
> > +}
> > +
> > +static void acpi_fujitsu_bl_remove(struct acpi_device *device)
> > +{
> > +	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > +				       acpi_fujitsu_bl_notify);
> >  }
> >  
> >  /* ACPI device for hotkey handling */
> > @@ -941,8 +953,9 @@ 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 acpi_device *device = data;
> >  	struct fujitsu_laptop *priv = acpi_driver_data(device);
> >  	unsigned long flags;
> >  	int scancode, i = 0;
> > @@ -1056,6 +1069,11 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
> >  	if (ret)
> >  		goto err_free_fifo;
> >  
> > +	ret = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > +					      acpi_fujitsu_laptop_notify, device);
> > +	if (ret)
> > +		goto err_free_fifo;
> 
> Hi Rafael,
> 
> Is the rollback path still correct after adding this here?

Why wouldn't it be?  There's only kfifo_free() in it.

> > +
> >  	ret = fujitsu_battery_charge_control_add(device);
> >  	if (ret < 0)
> >  		pr_warn("Unable to register battery charge control: %d\n", ret);
> > @@ -1074,6 +1092,9 @@ static void acpi_fujitsu_laptop_remove(struct acpi_device *device)
> >  
> >  	fujitsu_battery_charge_control_remove(device);
> >  
> > +	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > +				       acpi_fujitsu_laptop_notify);
> > +
> >  	fujitsu_laptop_platform_remove(device);
> >  
> >  	kfifo_free(&priv->fifo);
> > @@ -1092,7 +1113,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,
> >  		},
> >  };
> >  
> > @@ -1108,7 +1129,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,
> >  		},
> >  };
> >  
> > 
> 
> 

Re: [PATCH v1 3/5] platform/x86: fujitsu: Register ACPI notify handlers directly

[Ilpo Järvinen]

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

On Mon, 9 Mar 2026, Rafael J. Wysocki wrote:
> On Monday, March 9, 2026 2:00:31 PM CET Ilpo Järvinen wrote:
> > On Wed, 25 Feb 2026, Rafael J. Wysocki wrote:
> > 
> > > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > > 
> > > To facilitate subsequent conversion of the driver to using struct
> > > platform_driver instead of struct acpi_driver, make it install its ACPI
> > > notify handlers directly instead of using struct acpi_driver .notify()
> > > callbacks.
> > > 
> > > No intentional functional impact.
> > > 
> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > ---
> > >  drivers/platform/x86/fujitsu-laptop.c | 30 ++++++++++++++++++++++-----
> > >  1 file changed, 25 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> > > index 1adce90ae3e6..cb13c06b8f35 100644
> > > --- a/drivers/platform/x86/fujitsu-laptop.c
> > > +++ b/drivers/platform/x86/fujitsu-laptop.c
> > > @@ -502,8 +502,9 @@ static int fujitsu_backlight_register(struct acpi_device *device)
> > >  
> > >  /* Brightness notify */
> > >  
> > > -static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
> > > +static void acpi_fujitsu_bl_notify(acpi_handle handle, u32 event, void *data)
> > >  {
> > > +	struct acpi_device *device = data;
> > >  	struct fujitsu_bl *priv = acpi_driver_data(device);
> > >  	int oldb, newb;
> > >  
> > > @@ -558,7 +559,18 @@ 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;
> > > +
> > > +	return acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > > +					       acpi_fujitsu_bl_notify, device);
> > > +}
> > > +
> > > +static void acpi_fujitsu_bl_remove(struct acpi_device *device)
> > > +{
> > > +	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > > +				       acpi_fujitsu_bl_notify);
> > >  }
> > >  
> > >  /* ACPI device for hotkey handling */
> > > @@ -941,8 +953,9 @@ 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 acpi_device *device = data;
> > >  	struct fujitsu_laptop *priv = acpi_driver_data(device);
> > >  	unsigned long flags;
> > >  	int scancode, i = 0;
> > > @@ -1056,6 +1069,11 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
> > >  	if (ret)
> > >  		goto err_free_fifo;
> > >  
> > > +	ret = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > > +					      acpi_fujitsu_laptop_notify, device);
> > > +	if (ret)
> > > +		goto err_free_fifo;
> > 
> > Hi Rafael,
> > 
> > Is the rollback path still correct after adding this here?
> 
> Why wouldn't it be?  There's only kfifo_free() in it.

So it's intentional to not add fujitsu_laptop_platform_remove() there?

> > > +
> > >  	ret = fujitsu_battery_charge_control_add(device);
> > >  	if (ret < 0)
> > >  		pr_warn("Unable to register battery charge control: %d\n", ret);
> > > @@ -1074,6 +1092,9 @@ static void acpi_fujitsu_laptop_remove(struct acpi_device *device)
> > >  
> > >  	fujitsu_battery_charge_control_remove(device);
> > >  
> > > +	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > > +				       acpi_fujitsu_laptop_notify);
> > > +
> > >  	fujitsu_laptop_platform_remove(device);
> > >  
> > >  	kfifo_free(&priv->fifo);
> > > @@ -1092,7 +1113,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,
> > >  		},
> > >  };
> > >  
> > > @@ -1108,7 +1129,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,
> > >  		},
> > >  };
> > >  
> > > 
> > 
> > 
> 
> 
> 
> 

-- 
 i.

Re: [PATCH v1 3/5] platform/x86: fujitsu: Register ACPI notify handlers directly

[Rafael J. Wysocki]

On Mon, Mar 9, 2026 at 5:17 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Mon, 9 Mar 2026, Rafael J. Wysocki wrote:
> > On Monday, March 9, 2026 2:00:31 PM CET Ilpo Järvinen wrote:
> > > On Wed, 25 Feb 2026, Rafael J. Wysocki wrote:
> > >
> > > > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > > >
> > > > To facilitate subsequent conversion of the driver to using struct
> > > > platform_driver instead of struct acpi_driver, make it install its ACPI
> > > > notify handlers directly instead of using struct acpi_driver .notify()
> > > > callbacks.
> > > >
> > > > No intentional functional impact.
> > > >
> > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > > ---
> > > >  drivers/platform/x86/fujitsu-laptop.c | 30 ++++++++++++++++++++++-----
> > > >  1 file changed, 25 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> > > > index 1adce90ae3e6..cb13c06b8f35 100644
> > > > --- a/drivers/platform/x86/fujitsu-laptop.c
> > > > +++ b/drivers/platform/x86/fujitsu-laptop.c
> > > > @@ -502,8 +502,9 @@ static int fujitsu_backlight_register(struct acpi_device *device)
> > > >
> > > >  /* Brightness notify */
> > > >
> > > > -static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
> > > > +static void acpi_fujitsu_bl_notify(acpi_handle handle, u32 event, void *data)
> > > >  {
> > > > + struct acpi_device *device = data;
> > > >   struct fujitsu_bl *priv = acpi_driver_data(device);
> > > >   int oldb, newb;
> > > >
> > > > @@ -558,7 +559,18 @@ 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;
> > > > +
> > > > + return acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > > > +                                        acpi_fujitsu_bl_notify, device);
> > > > +}
> > > > +
> > > > +static void acpi_fujitsu_bl_remove(struct acpi_device *device)
> > > > +{
> > > > + acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > > > +                                acpi_fujitsu_bl_notify);
> > > >  }
> > > >
> > > >  /* ACPI device for hotkey handling */
> > > > @@ -941,8 +953,9 @@ 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 acpi_device *device = data;
> > > >   struct fujitsu_laptop *priv = acpi_driver_data(device);
> > > >   unsigned long flags;
> > > >   int scancode, i = 0;
> > > > @@ -1056,6 +1069,11 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
> > > >   if (ret)
> > > >           goto err_free_fifo;
> > > >
> > > > + ret = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > > > +                                       acpi_fujitsu_laptop_notify, device);
> > > > + if (ret)
> > > > +         goto err_free_fifo;
> > >
> > > Hi Rafael,
> > >
> > > Is the rollback path still correct after adding this here?
> >
> > Why wouldn't it be?  There's only kfifo_free() in it.
>
> So it's intentional to not add fujitsu_laptop_platform_remove() there?

Ah, yes, it needs to be called when acpi_dev_install_notify_handler() fails.

A fix would be relatively straightforward, but I guess I need to
resubmit the whole series with this fixed.

Will do, thanks!