[PATCH v1 6/6] platform/chrome: wilco_ec: event: Convert to a platform driver

Rafael J. Wysocki posted 1 patch 1 month ago
drivers/platform/chrome/wilco_ec/event.c | 66 ++++++++++++------------
1 file changed, 34 insertions(+), 32 deletions(-)
[PATCH v1 6/6] platform/chrome: wilco_ec: event: Convert to a platform driver
Posted by Rafael J. Wysocki 1 month ago
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 ChromeOS Wilco Embedded Controller event
ACPI driver to a platform one.

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/chrome/wilco_ec/event.c | 66 ++++++++++++------------
 1 file changed, 34 insertions(+), 32 deletions(-)

diff --git a/drivers/platform/chrome/wilco_ec/event.c b/drivers/platform/chrome/wilco_ec/event.c
index f4500dd05f93..b6e935badc0e 100644
--- a/drivers/platform/chrome/wilco_ec/event.c
+++ b/drivers/platform/chrome/wilco_ec/event.c
@@ -38,6 +38,7 @@
 #include <linux/io.h>
 #include <linux/list.h>
 #include <linux/module.h>
+#include <linux/platform_device.h>
 #include <linux/poll.h>
 #include <linux/spinlock.h>
 #include <linux/uaccess.h>
@@ -198,7 +199,7 @@ struct event_device_data {
 
 /**
  * enqueue_events() - Place EC events in queue to be read by userspace.
- * @adev: Device the events came from.
+ * @dev: Device the events came from.
  * @buf: Buffer of event data.
  * @length: Length of event data buffer.
  *
@@ -209,9 +210,9 @@ struct event_device_data {
  *
  * Return: 0 on success or negative error code on failure.
  */
-static int enqueue_events(struct acpi_device *adev, const u8 *buf, u32 length)
+static int enqueue_events(struct device *dev, const u8 *buf, u32 length)
 {
-	struct event_device_data *dev_data = adev->driver_data;
+	struct event_device_data *dev_data = dev_get_drvdata(dev);
 	struct ec_event *event, *queue_event, *old_event;
 	size_t num_words, event_size;
 	u32 offset = 0;
@@ -222,14 +223,14 @@ static int enqueue_events(struct acpi_device *adev, const u8 *buf, u32 length)
 		num_words = ec_event_num_words(event);
 		event_size = ec_event_size(event);
 		if (num_words > EC_ACPI_MAX_EVENT_WORDS) {
-			dev_err(&adev->dev, "Too many event words: %zu > %d\n",
+			dev_err(dev, "Too many event words: %zu > %d\n",
 				num_words, EC_ACPI_MAX_EVENT_WORDS);
 			return -EOVERFLOW;
 		}
 
 		/* Ensure event does not overflow the available buffer */
 		if ((offset + event_size) > length) {
-			dev_err(&adev->dev, "Event exceeds buffer: %zu > %d\n",
+			dev_err(dev, "Event exceeds buffer: %zu > %d\n",
 				offset + event_size, length);
 			return -EOVERFLOW;
 		}
@@ -262,12 +263,13 @@ static int enqueue_events(struct acpi_device *adev, const u8 *buf, u32 length)
 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;
+	struct device *dev = data;
+	struct acpi_device *adev = ACPI_COMPANION(dev);
 	union acpi_object *obj;
 	acpi_status status;
 
 	if (value != EC_ACPI_NOTIFY_EVENT) {
-		dev_err(&adev->dev, "Invalid event: 0x%08x\n", value);
+		dev_err(dev, "Invalid event: 0x%08x\n", value);
 		return;
 	}
 
@@ -275,31 +277,31 @@ static void event_device_notify(acpi_handle handle, u32 value, void *data)
 	status = acpi_evaluate_object(adev->handle, EC_ACPI_GET_EVENT,
 				      NULL, &event_buffer);
 	if (ACPI_FAILURE(status)) {
-		dev_err(&adev->dev, "Error executing ACPI method %s()\n",
+		dev_err(dev, "Error executing ACPI method %s()\n",
 			EC_ACPI_GET_EVENT);
 		return;
 	}
 
 	obj = (union acpi_object *)event_buffer.pointer;
 	if (!obj) {
-		dev_err(&adev->dev, "Nothing returned from %s()\n",
+		dev_err(dev, "Nothing returned from %s()\n",
 			EC_ACPI_GET_EVENT);
 		return;
 	}
 	if (obj->type != ACPI_TYPE_BUFFER) {
-		dev_err(&adev->dev, "Invalid object returned from %s()\n",
+		dev_err(dev, "Invalid object returned from %s()\n",
 			EC_ACPI_GET_EVENT);
 		kfree(obj);
 		return;
 	}
 	if (obj->buffer.length < sizeof(struct ec_event)) {
-		dev_err(&adev->dev, "Invalid buffer length %d from %s()\n",
+		dev_err(dev, "Invalid buffer length %d from %s()\n",
 			obj->buffer.length, EC_ACPI_GET_EVENT);
 		kfree(obj);
 		return;
 	}
 
-	enqueue_events(adev, obj->buffer.pointer, obj->buffer.length);
+	enqueue_events(dev, obj->buffer.pointer, obj->buffer.length);
 	kfree(obj);
 }
 
@@ -434,8 +436,8 @@ static void hangup_device(struct event_device_data *dev_data)
 }
 
 /**
- * event_device_add() - Callback when creating a new device.
- * @adev: ACPI device that we will be receiving events from.
+ * event_device_probe() - Callback when creating a new device.
+ * @pdev: Platform device that we will be receiving events from.
  *
  * This finds a free minor number for the device, allocates and initializes
  * some device data, and creates a new device and char dev node.
@@ -447,7 +449,7 @@ static void hangup_device(struct event_device_data *dev_data)
  *
  * Return: 0 on success, negative error code on failure.
  */
-static int event_device_add(struct acpi_device *adev)
+static int event_device_probe(struct platform_device *pdev)
 {
 	struct event_device_data *dev_data;
 	int error, minor;
@@ -455,7 +457,7 @@ static int event_device_add(struct acpi_device *adev)
 	minor = ida_alloc_max(&event_ida, EVENT_MAX_DEV-1, GFP_KERNEL);
 	if (minor < 0) {
 		error = minor;
-		dev_err(&adev->dev, "Failed to find minor number: %d\n", error);
+		dev_err(&pdev->dev, "Failed to find minor number: %d\n", error);
 		return error;
 	}
 
@@ -466,7 +468,7 @@ static int event_device_add(struct acpi_device *adev)
 	}
 
 	/* Initialize the device data. */
-	adev->driver_data = dev_data;
+	platform_set_drvdata(pdev, dev_data);
 	dev_data->events = event_queue_new(queue_size);
 	if (!dev_data->events) {
 		kfree(dev_data);
@@ -492,8 +494,9 @@ static int event_device_add(struct acpi_device *adev)
 		goto free_dev_data;
 
 	/* Install an ACPI notify handler. */
-	error = acpi_dev_install_notify_handler(adev, ACPI_DEVICE_NOTIFY,
-						event_device_notify, adev);
+	error = acpi_dev_install_notify_handler(ACPI_COMPANION(&pdev->dev),
+						ACPI_DEVICE_NOTIFY,
+						event_device_notify, &pdev->dev);
 	if (error)
 		goto free_cdev;
 
@@ -508,12 +511,12 @@ static int event_device_add(struct acpi_device *adev)
 	return error;
 }
 
-static void event_device_remove(struct acpi_device *adev)
+static void event_device_remove(struct platform_device *pdev)
 {
-	struct event_device_data *dev_data = adev->driver_data;
+	struct event_device_data *dev_data = platform_get_drvdata(pdev);
 
-	acpi_dev_remove_notify_handler(adev, ACPI_DEVICE_NOTIFY,
-				       event_device_notify);
+	acpi_dev_remove_notify_handler(ACPI_COMPANION(&pdev->dev),
+				       ACPI_DEVICE_NOTIFY, event_device_notify);
 	cdev_device_del(&dev_data->cdev, &dev_data->dev);
 	ida_free(&event_ida, MINOR(dev_data->dev.devt));
 	hangup_device(dev_data);
@@ -525,13 +528,12 @@ static const struct acpi_device_id event_acpi_ids[] = {
 };
 MODULE_DEVICE_TABLE(acpi, event_acpi_ids);
 
-static struct acpi_driver event_driver = {
-	.name = DRV_NAME,
-	.class = DRV_NAME,
-	.ids = event_acpi_ids,
-	.ops = {
-		.add = event_device_add,
-		.remove = event_device_remove,
+static struct platform_driver event_driver = {
+	.probe = event_device_probe,
+	.remove = event_device_remove,
+	.driver = {
+		.name = DRV_NAME,
+		.acpi_match_table = event_acpi_ids,
 	},
 };
 
@@ -554,7 +556,7 @@ static int __init event_module_init(void)
 	}
 	event_major = MAJOR(dev_num);
 
-	ret = acpi_bus_register_driver(&event_driver);
+	ret = platform_driver_register(&event_driver);
 	if (ret < 0) {
 		pr_err(DRV_NAME ": Failed registering driver: %d\n", ret);
 		goto unregister_region;
@@ -572,7 +574,7 @@ static int __init event_module_init(void)
 
 static void __exit event_module_exit(void)
 {
-	acpi_bus_unregister_driver(&event_driver);
+	platform_driver_unregister(&event_driver);
 	unregister_chrdev_region(MKDEV(event_major, 0), EVENT_MAX_DEV);
 	class_unregister(&event_class);
 	ida_destroy(&event_ida);
-- 
2.51.0