[PATCH v1 0/3] platform/surface: surfacepro3_button: Use platform device for driver binding

[PATCH v1 0/3] platform/surface: surfacepro3_button: Use platform device for driver binding

[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 conversion of the Surface Pro3 button
driver.

Patch [1/3] Fixes a wakeup source leak on driver removal.

Patch [2/3] updates the driver to install an ACPI notify handler by itself
instead of using the .notify() callback from struct acpi_driver, which is
requisite for the driver conversion.

Patch [3/3] converts the driver to using struct platform_driver for device
binding.

Thanks!

[PATCH v1 1/3] platform/surface: surfacepro3_button: Drop wakeup source on remove

[Rafael J. Wysocki]

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

The wakeup source added by device_init_wakeup() in surface_button_add()
needs to be dropped during driver removal, so update the driver to do
that.

Fixes: 19351f340765 ("platform/x86: surfacepro3: Support for wakeup from suspend-to-idle")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/platform/surface/surfacepro3_button.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/surface/surfacepro3_button.c b/drivers/platform/surface/surfacepro3_button.c
index 9bd39f09c7db..a6c9d4d370be 100644
--- a/drivers/platform/surface/surfacepro3_button.c
+++ b/drivers/platform/surface/surfacepro3_button.c
@@ -242,6 +242,7 @@ static void surface_button_remove(struct acpi_device *device)
 {
 	struct surface_button *button = acpi_driver_data(device);
 
+	device_init_wakeup(&device->dev, false);
 	input_unregister_device(button->input);
 	kfree(button);
 }
-- 
2.51.0

[PATCH v1 2/3] platform/surface: surfacepro3_button: Register ACPI notify handler

[Rafael J. Wysocki]

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

To facilitate subsequent conversion of the driver to a platform one,
make it install an ACPI notify handler directly instead of using
a .notify() callback in struct acpi_driver.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/platform/surface/surfacepro3_button.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/surface/surfacepro3_button.c b/drivers/platform/surface/surfacepro3_button.c
index a6c9d4d370be..6d394daf5bc4 100644
--- a/drivers/platform/surface/surfacepro3_button.c
+++ b/drivers/platform/surface/surfacepro3_button.c
@@ -72,8 +72,9 @@ 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 acpi_device *device = data;
 	struct surface_button *button = acpi_driver_data(device);
 	struct input_dev *input;
 	int key_code = KEY_RESERVED;
@@ -227,6 +228,15 @@ static int surface_button_add(struct acpi_device *device)
 		goto err_free_input;
 
 	device_init_wakeup(&device->dev, true);
+
+	error = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
+						surface_button_notify, device);
+	if (error) {
+		device_init_wakeup(&device->dev, false);
+		input_unregister_device(input);
+		goto err_free_button;
+	}
+
 	dev_info(&device->dev, "%s [%s]\n", acpi_device_name(device),
 		 acpi_device_bid(device));
 	return 0;
@@ -242,6 +252,8 @@ static void surface_button_remove(struct acpi_device *device)
 {
 	struct surface_button *button = acpi_driver_data(device);
 
+	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
+				       surface_button_notify);
 	device_init_wakeup(&device->dev, false);
 	input_unregister_device(button->input);
 	kfree(button);
@@ -257,7 +269,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.51.0

Re: [PATCH v1 2/3] platform/surface: surfacepro3_button: Register ACPI notify handler

[Ilpo Järvinen]

On Wed, 4 Mar 2026, Rafael J. Wysocki wrote:

> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> To facilitate subsequent conversion of the driver to a platform one,
> make it install an ACPI notify handler directly instead of using
> a .notify() callback in struct acpi_driver.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/platform/surface/surfacepro3_button.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/surface/surfacepro3_button.c b/drivers/platform/surface/surfacepro3_button.c
> index a6c9d4d370be..6d394daf5bc4 100644
> --- a/drivers/platform/surface/surfacepro3_button.c
> +++ b/drivers/platform/surface/surfacepro3_button.c
> @@ -72,8 +72,9 @@ 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 acpi_device *device = data;
>  	struct surface_button *button = acpi_driver_data(device);
>  	struct input_dev *input;
>  	int key_code = KEY_RESERVED;
> @@ -227,6 +228,15 @@ static int surface_button_add(struct acpi_device *device)
>  		goto err_free_input;
>  
>  	device_init_wakeup(&device->dev, true);
> +
> +	error = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> +						surface_button_notify, device);
> +	if (error) {
> +		device_init_wakeup(&device->dev, false);
> +		input_unregister_device(input);

Add a new label to rollback path instead.

> +		goto err_free_button;
> +	}
> +
>  	dev_info(&device->dev, "%s [%s]\n", acpi_device_name(device),
>  		 acpi_device_bid(device));
>  	return 0;
> @@ -242,6 +252,8 @@ static void surface_button_remove(struct acpi_device *device)
>  {
>  	struct surface_button *button = acpi_driver_data(device);
>  
> +	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> +				       surface_button_notify);
>  	device_init_wakeup(&device->dev, false);
>  	input_unregister_device(button->input);
>  	kfree(button);
> @@ -257,7 +269,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,
>  };
> 

-- 
 i.

Re: [PATCH v1 2/3] platform/surface: surfacepro3_button: Register ACPI notify handler

[Ilpo Järvinen]

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

On Mon, 9 Mar 2026, Ilpo Järvinen wrote:

> On Wed, 4 Mar 2026, Rafael J. Wysocki wrote:
> 
> > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > 
> > To facilitate subsequent conversion of the driver to a platform one,
> > make it install an ACPI notify handler directly instead of using
> > a .notify() callback in struct acpi_driver.
> > 
> > No intentional functional impact.
> > 
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  drivers/platform/surface/surfacepro3_button.c | 15 +++++++++++++--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/platform/surface/surfacepro3_button.c b/drivers/platform/surface/surfacepro3_button.c
> > index a6c9d4d370be..6d394daf5bc4 100644
> > --- a/drivers/platform/surface/surfacepro3_button.c
> > +++ b/drivers/platform/surface/surfacepro3_button.c
> > @@ -72,8 +72,9 @@ 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 acpi_device *device = data;
> >  	struct surface_button *button = acpi_driver_data(device);
> >  	struct input_dev *input;
> >  	int key_code = KEY_RESERVED;
> > @@ -227,6 +228,15 @@ static int surface_button_add(struct acpi_device *device)
> >  		goto err_free_input;
> >  
> >  	device_init_wakeup(&device->dev, true);
> > +
> > +	error = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > +						surface_button_notify, device);
> > +	if (error) {
> > +		device_init_wakeup(&device->dev, false);
> > +		input_unregister_device(input);
> 
> Add a new label to rollback path instead.

Nevermind.

-- 
 i.

> > +		goto err_free_button;
> > +	}
> > +
> >  	dev_info(&device->dev, "%s [%s]\n", acpi_device_name(device),
> >  		 acpi_device_bid(device));
> >  	return 0;
> > @@ -242,6 +252,8 @@ static void surface_button_remove(struct acpi_device *device)
> >  {
> >  	struct surface_button *button = acpi_driver_data(device);
> >  
> > +	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > +				       surface_button_notify);
> >  	device_init_wakeup(&device->dev, false);
> >  	input_unregister_device(button->input);
> >  	kfree(button);
> > @@ -257,7 +269,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,
> >  };
> > 
> 
> 

[PATCH v1 3/3] platform/surface: surfacepro3_button: Convert to a platform driver

[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 Surface Pro 3 button ACPI driver to a
platform one.

After this change, the subordinate input device and wakeup source class
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/surface/surfacepro3_button.c | 67 +++++++++----------
 1 file changed, 32 insertions(+), 35 deletions(-)

diff --git a/drivers/platform/surface/surfacepro3_button.c b/drivers/platform/surface/surfacepro3_button.c
index 6d394daf5bc4..0293bc517b54 100644
--- a/drivers/platform/surface/surfacepro3_button.c
+++ b/drivers/platform/surface/surfacepro3_button.c
@@ -14,6 +14,7 @@
 #include <linux/types.h>
 #include <linux/input.h>
 #include <linux/acpi.h>
+#include <linux/platform_device.h>
 #include <acpi/button.h>
 
 #define SURFACE_PRO3_BUTTON_HID		"MSHW0028"
@@ -74,8 +75,8 @@ struct surface_button {
 
 static void surface_button_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct acpi_device *device = data;
-	struct surface_button *button = acpi_driver_data(device);
+	struct device *dev = data;
+	struct surface_button *button = dev_get_drvdata(dev);
 	struct input_dev *input;
 	int key_code = KEY_RESERVED;
 	bool pressed = false;
@@ -110,18 +111,17 @@ static void surface_button_notify(acpi_handle handle, u32 event, void *data)
 		key_code = KEY_VOLUMEDOWN;
 		break;
 	case SURFACE_BUTTON_NOTIFY_TABLET_MODE:
-		dev_warn_once(&device->dev, "Tablet mode is not supported\n");
+		dev_warn_once(dev, "Tablet mode is not supported\n");
 		break;
 	default:
-		dev_info_ratelimited(&device->dev,
-				     "Unsupported event [0x%x]\n", event);
+		dev_info_ratelimited(dev, "Unsupported event [0x%x]\n", event);
 		break;
 	}
 	input = button->input;
 	if (key_code == KEY_RESERVED)
 		return;
 	if (pressed)
-		pm_wakeup_dev_event(&device->dev, 0, button->suspended);
+		pm_wakeup_dev_event(dev, 0, button->suspended);
 	if (button->suspended)
 		return;
 	input_report_key(input, key_code, pressed?1:0);
@@ -131,8 +131,7 @@ static void surface_button_notify(acpi_handle handle, u32 event, void *data)
 #ifdef CONFIG_PM_SLEEP
 static int surface_button_suspend(struct device *dev)
 {
-	struct acpi_device *device = to_acpi_device(dev);
-	struct surface_button *button = acpi_driver_data(device);
+	struct surface_button *button = dev_get_drvdata(dev);
 
 	button->suspended = true;
 	return 0;
@@ -140,8 +139,7 @@ static int surface_button_suspend(struct device *dev)
 
 static int surface_button_resume(struct device *dev)
 {
-	struct acpi_device *device = to_acpi_device(dev);
-	struct surface_button *button = acpi_driver_data(device);
+	struct surface_button *button = dev_get_drvdata(dev);
 
 	button->suspended = false;
 	return 0;
@@ -156,9 +154,8 @@ static int surface_button_resume(struct device *dev)
  * Returns true if the driver should bind to this device, i.e. the device is
  * either MSWH0028 (Pro 3) or MSHW0040 on a Pro 4 or Book 1.
  */
-static bool surface_button_check_MSHW0040(struct acpi_device *dev)
+static bool surface_button_check_MSHW0040(struct device *dev, acpi_handle handle)
 {
-	acpi_handle handle = dev->handle;
 	union acpi_object *result;
 	u64 oem_platform_rev = 0;	// valid revisions are nonzero
 
@@ -180,14 +177,15 @@ static bool surface_button_check_MSHW0040(struct acpi_device *dev)
 		ACPI_FREE(result);
 	}
 
-	dev_dbg(&dev->dev, "OEM Platform Revision %llu\n", oem_platform_rev);
+	dev_dbg(dev, "OEM Platform Revision %llu\n", oem_platform_rev);
 
 	return oem_platform_rev == 0;
 }
 
 
-static int surface_button_add(struct acpi_device *device)
+static int surface_button_probe(struct platform_device *pdev)
 {
+	struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
 	struct surface_button *button;
 	struct input_dev *input;
 	const char *hid = acpi_device_hid(device);
@@ -197,14 +195,14 @@ static int surface_button_add(struct acpi_device *device)
 	    strlen(SURFACE_BUTTON_OBJ_NAME)))
 		return -ENODEV;
 
-	if (!surface_button_check_MSHW0040(device))
+	if (!surface_button_check_MSHW0040(&pdev->dev, device->handle))
 		return -ENODEV;
 
 	button = kzalloc_obj(struct surface_button);
 	if (!button)
 		return -ENOMEM;
 
-	device->driver_data = button;
+	platform_set_drvdata(pdev, button);
 	button->input = input = input_allocate_device();
 	if (!input) {
 		error = -ENOMEM;
@@ -217,7 +215,7 @@ static int surface_button_add(struct acpi_device *device)
 	input->name = acpi_device_name(device);
 	input->phys = button->phys;
 	input->id.bustype = BUS_HOST;
-	input->dev.parent = &device->dev;
+	input->dev.parent = &pdev->dev;
 	input_set_capability(input, EV_KEY, KEY_POWER);
 	input_set_capability(input, EV_KEY, KEY_LEFTMETA);
 	input_set_capability(input, EV_KEY, KEY_VOLUMEUP);
@@ -227,17 +225,17 @@ static int surface_button_add(struct acpi_device *device)
 	if (error)
 		goto err_free_input;
 
-	device_init_wakeup(&device->dev, true);
+	device_init_wakeup(&pdev->dev, true);
 
 	error = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
-						surface_button_notify, device);
+						surface_button_notify, &pdev->dev);
 	if (error) {
-		device_init_wakeup(&device->dev, false);
+		device_init_wakeup(&pdev->dev, false);
 		input_unregister_device(input);
 		goto err_free_button;
 	}
 
-	dev_info(&device->dev, "%s [%s]\n", acpi_device_name(device),
+	dev_info(&pdev->dev, "%s [%s]\n", acpi_device_name(device),
 		 acpi_device_bid(device));
 	return 0;
 
@@ -248,13 +246,13 @@ static int surface_button_add(struct acpi_device *device)
 	return error;
 }
 
-static void surface_button_remove(struct acpi_device *device)
+static void surface_button_remove(struct platform_device *pdev)
 {
-	struct surface_button *button = acpi_driver_data(device);
+	struct surface_button *button = platform_get_drvdata(pdev);
 
-	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
-				       surface_button_notify);
-	device_init_wakeup(&device->dev, false);
+	acpi_dev_remove_notify_handler(ACPI_COMPANION(&pdev->dev),
+				       ACPI_DEVICE_NOTIFY, surface_button_notify);
+	device_init_wakeup(&pdev->dev, false);
 	input_unregister_device(button->input);
 	kfree(button);
 }
@@ -262,15 +260,14 @@ static void surface_button_remove(struct acpi_device *device)
 static SIMPLE_DEV_PM_OPS(surface_button_pm,
 		surface_button_suspend, surface_button_resume);
 
-static struct acpi_driver surface_button_driver = {
-	.name = "surface_pro3_button",
-	.class = "SurfacePro3",
-	.ids = surface_button_device_ids,
-	.ops = {
-		.add = surface_button_add,
-		.remove = surface_button_remove,
+static struct platform_driver surface_button_driver = {
+	.probe = surface_button_probe,
+	.remove = surface_button_remove,
+	.driver = {
+		.name = "surface_pro3_button",
+		.acpi_match_table = surface_button_device_ids,
+		.pm = &surface_button_pm,
 	},
-	.drv.pm = &surface_button_pm,
 };
 
-module_acpi_driver(surface_button_driver);
+module_platform_driver(surface_button_driver);
-- 
2.51.0

Re: [PATCH v1 0/3] platform/surface: surfacepro3_button: Use platform device for driver binding

[Ilpo Järvinen]

On Wed, 04 Mar 2026 19:53:24 +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/
> 
> [...]


Thank you for your contribution, it has been applied to my local
review-ilpo-next branch. Note it will show up in the public
platform-drivers-x86/review-ilpo-next branch only once I've pushed my
local branch there, which might take a while.

The list of commits applied:
[1/3] platform/surface: surfacepro3_button: Drop wakeup source on remove
      commit: 1410a228ab2d36fe2b383415a632ae12048d4f3a
[2/3] platform/surface: surfacepro3_button: Register ACPI notify handler
      commit: 639d8c601c7a9aab44803245a22f6e3c365b08be
[3/3] platform/surface: surfacepro3_button: Convert to a platform driver
      commit: d913a5a12b4036e4219b02777d0a9c70e37a6620

--
 i.