Re: [PATCH 5/5] ACPI: Add support for platform bus type

On Wednesday, October 31, 2012 11:34:24 PM Yinghai Lu wrote:

On Wed, Oct 31, 2012 at 2:36 AM, Rafael J. Wysocki [off-list ref] wrote:

quoted

From: Mika Westerberg <mika.westerberg@linux.intel.com>

With ACPI 5 it is now possible to enumerate traditional SoC
peripherals, like serial bus controllers and slave devices behind
them.  These devices are typically based on IP-blocks used in many
existing SoC platforms and platform drivers for them may already
be present in the kernel tree.

To make driver "porting" more straightforward, add ACPI support to
the platform bus type.  Instead of writing ACPI "glue" drivers for
the existing platform drivers, register the platform bus type with
ACPI to create platform device objects for the drivers and bind the
corresponding ACPI handles to those platform devices.

This should allow us to reuse the existing platform drivers for the
devices in question with the minimum amount of modifications.

This changeset is based on Mika Westerberg's and Mathias Nyman's
work.

Signed-off-by: Mathias Nyman <redacted>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <redacted>
---
 drivers/acpi/Makefile        |    1
 drivers/acpi/acpi_platform.c |  285 +++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/internal.h      |    7 +
 drivers/acpi/scan.c          |   16 ++
 drivers/base/platform.c      |    5
 5 files changed, 313 insertions(+), 1 deletion(-)

this patch is too big, and should be split to at least two or more.

quoted

 create mode 100644 drivers/acpi/acpi_platform.c

Index: linux/drivers/acpi/Makefile
===================================================================

--- linux.orig/drivers/acpi/Makefile
+++ linux/drivers/acpi/Makefile

@@ -37,6 +37,7 @@ acpi-y                                += processor_core.o
 acpi-y                         += ec.o
 acpi-$(CONFIG_ACPI_DOCK)       += dock.o
 acpi-y                         += pci_root.o pci_link.o pci_irq.o pci_bind.o
+acpi-y                         += acpi_platform.o
 acpi-y                         += power.o
 acpi-y                         += event.o
 acpi-y                         += sysfs.o

Index: linux/drivers/acpi/acpi_platform.c
===================================================================

--- /dev/null
+++ linux/drivers/acpi/acpi_platform.c

@@ -0,0 +1,285 @@
+/*
+ * ACPI support for platform bus type.
+ *
+ * Copyright (C) 2012, Intel Corporation
+ * Authors: Mika Westerberg <mika.westerberg@linux.intel.com>
+ *          Mathias Nyman <mathias.nyman@linux.intel.com>
+ *          Rafael J. Wysocki <rafael.j.wysocki@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+ACPI_MODULE_NAME("platform");
+
+struct resource_info {
+       struct device *dev;
+       struct resource *res;
+       size_t n, cur;
+};
+
+static acpi_status acpi_platform_count_resources(struct acpi_resource *res,
+                                                void *data)
+{
+       struct acpi_resource_extended_irq *acpi_xirq;
+       struct resource_info *ri = data;
+
+       switch (res->type) {
+       case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
+       case ACPI_RESOURCE_TYPE_IRQ:
+               ri->n++;
+               break;
+       case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
+               acpi_xirq = &res->data.extended_irq;
+               ri->n += acpi_xirq->interrupt_count;
+               break;
+       case ACPI_RESOURCE_TYPE_ADDRESS32:
+               if (res->data.address32.resource_type == ACPI_IO_RANGE)
+                       ri->n++;
+               break;
+       }
+
+       return AE_OK;
+}
+
+static acpi_status acpi_platform_add_resources(struct acpi_resource *res,
+                                              void *data)
+{
+       struct acpi_resource_fixed_memory32 *acpi_mem;
+       struct acpi_resource_address32 *acpi_add32;
+       struct acpi_resource_extended_irq *acpi_xirq;
+       struct acpi_resource_irq *acpi_irq;
+       struct resource_info *ri = data;
+       struct resource *r;
+       int irq, i;
+
+       switch (res->type) {
+       case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
+               acpi_mem = &res->data.fixed_memory32;
+               r = &ri->res[ri->cur++];
+
+               r->start = acpi_mem->address;
+               r->end = r->start + acpi_mem->address_length - 1;
+               r->flags = IORESOURCE_MEM;
+
+               dev_dbg(ri->dev, "Memory32Fixed %pR\n", r);
+               break;
+
+       case ACPI_RESOURCE_TYPE_ADDRESS32:
+               acpi_add32 = &res->data.address32;
+
+               if (acpi_add32->resource_type == ACPI_IO_RANGE) {
+                       r = &ri->res[ri->cur++];
+                       r->start = acpi_add32->minimum;
+                       r->end = r->start + acpi_add32->address_length - 1;
+                       r->flags = IORESOURCE_IO;
+                       dev_dbg(ri->dev, "Address32 %pR\n", r);
+               }
+               break;
+
+       case ACPI_RESOURCE_TYPE_IRQ:
+               acpi_irq = &res->data.irq;
+               r = &ri->res[ri->cur++];
+
+               irq = acpi_register_gsi(ri->dev,
+                                       acpi_irq->interrupts[0],
+                                       acpi_irq->triggering,
+                                       acpi_irq->polarity);
+
+               r->start = r->end = irq;
+               r->flags = IORESOURCE_IRQ;
+
+               dev_dbg(ri->dev, "IRQ %pR\n", r);
+               break;
+
+       case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
+               acpi_xirq = &res->data.extended_irq;
+
+               for (i = 0; i < acpi_xirq->interrupt_count; i++, ri->cur++) {
+                       r = &ri->res[ri->cur];
+                       irq = acpi_register_gsi(ri->dev,
+                                               acpi_xirq->interrupts[i],
+                                               acpi_xirq->triggering,
+                                               acpi_xirq->polarity);
+
+                       r->start = r->end = irq;
+                       r->flags = IORESOURCE_IRQ;
+
+                       dev_dbg(ri->dev, "Interrupt %pR\n", r);
+               }
+               break;
+       }
+
+       return AE_OK;
+}
+

could be shared with pci root or pnp devices?

Maybe.

quoted

+static acpi_status acpi_platform_get_device_uid(struct acpi_device *adev,
+                                               int *uid)
+{
+       struct acpi_device_info *info;
+       acpi_status status;
+
+       status = acpi_get_object_info(adev->handle, &info);
+       if (ACPI_FAILURE(status))
+               return status;
+
+       status = AE_NOT_EXIST;
+       if ((info->valid & ACPI_VALID_UID) &&
+            !kstrtoint(info->unique_id.string, 0, uid))
+               status = AE_OK;
+
+       kfree(info);
+       return status;
+}
+
+/**
+ * acpi_create_platform_device - Create platform device for ACPI device node
+ * @adev: ACPI device node to create a platform device for.
+ *
+ * Check if the given @adev can be represented as a platform device and, if
+ * that's the case, create and register a platform device, populate its common
+ * resources and returns a pointer to it.  Otherwise, return %NULL.
+ *
+ * The platform device's name will be taken from the @adev's _HID and _UID.
+ */
+struct platform_device *acpi_create_platform_device(struct acpi_device *adev)
+{
+       struct platform_device *pdev = NULL;
+       struct acpi_device *acpi_parent;
+       struct device *parent = NULL;
+       struct resource_info ri;
+       acpi_status status;
+       int devid;
+
+       /* If the ACPI node already has a physical device attached, skip it. */
+       if (adev->physical_node_count)
+               return NULL;
+
+       /* Use the UID of the device as the new platform device id if found. */
+       status = acpi_platform_get_device_uid(adev, &devid);
+       if (ACPI_FAILURE(status))
+               devid = -1;
+
+       memset(&ri, 0, sizeof(ri));
+
+       /* First, count the resources. */
+       status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
+                                    acpi_platform_count_resources, &ri);
+       if (ACPI_FAILURE(status) || !ri.n)
+               return NULL;
+
+       /* Next, allocate memory for all the resources and populate it. */
+       ri.dev = &adev->dev;
+       ri.res = kzalloc(ri.n * sizeof(struct resource), GFP_KERNEL);
+       if (!ri.res) {
+               dev_err(&adev->dev,
+                       "failed to allocate memory for resources\n");
+               return NULL;
+       }
+
+       status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
+                                    acpi_platform_add_resources, &ri);
+       if (ACPI_FAILURE(status)) {
+               dev_err(&adev->dev, "failed to walk resources\n");
+               goto out;
+       }
+
+       if (WARN_ON(ri.n != ri.cur))
+               goto out;
+
+       /*
+        * If the ACPI node has a parent and that parent has a physical device
+        * attached to it, that physical device should be the parent of the
+        * platform device we are about to create.
+        */
+       acpi_parent = adev->parent;
+       if (acpi_parent) {
+               struct acpi_device_physical_node *entry;
+               struct list_head *list;
+
+               mutex_lock(&acpi_parent->physical_node_lock);
+               list = &acpi_parent->physical_node_list;
+               if (!list_empty(list)) {
+                       entry = list_first_entry(list,
+                                       struct acpi_device_physical_node,
+                                       node);
+                       parent = entry->dev;
+               }
+               mutex_unlock(&acpi_parent->physical_node_lock);
+       }
+       pdev = platform_device_register_resndata(parent, acpi_device_hid(adev),
+                                                devid, ri.res, ri.n, NULL, 0);
+       if (IS_ERR(pdev)) {
+               dev_err(&adev->dev, "platform device creation failed: %ld\n",
+                       PTR_ERR(pdev));
+               pdev = NULL;
+       } else {
+               dev_dbg(&adev->dev, "created platform device %s\n",
+                       dev_name(&pdev->dev));
+       }
+
+ out:
+       kfree(ri.res);
+       return pdev;
+}
+
+static acpi_status acpi_platform_match(acpi_handle handle, u32 depth,
+                                      void *data, void **return_value)
+{
+       struct platform_device *pdev = data;
+       struct acpi_device *adev;
+       acpi_status status;
+
+       status = acpi_bus_get_device(handle, &adev);
+       if (ACPI_FAILURE(status))
+               return status;
+
+       /* Skip ACPI devices that have physical device attached */
+       if (adev->physical_node_count)
+               return AE_OK;
+
+       if (!strcmp(pdev->name, acpi_device_hid(adev))) {
+               int devid;
+
+               /* Check that both name and UID match if it exists */
+               status = acpi_platform_get_device_uid(adev, &devid);
+               if (ACPI_FAILURE(status))
+                       devid = -1;
+
+               if (pdev->id != devid)
+                       return AE_OK;
+
+               *(acpi_handle *)return_value = handle;
+               return AE_CTRL_TERMINATE;
+       }
+
+       return AE_OK;
+}
+
+static int acpi_platform_find_device(struct device *dev, acpi_handle *handle)
+{
+       struct platform_device *pdev = to_platform_device(dev);
+
+       *handle = NULL;
+       acpi_get_devices(pdev->name, acpi_platform_match, pdev, handle);
+
+       return *handle ? 0 : -ENODEV;
+}
+
+static struct acpi_bus_type acpi_platform_bus = {
+       .bus = &platform_bus_type,
+       .find_device = acpi_platform_find_device,
+};
+
+static int __init acpi_platform_init(void)
+{
+       return register_acpi_bus_type(&acpi_platform_bus);
+}
+arch_initcall(acpi_platform_init);
Index: linux/drivers/acpi/internal.h
===================================================================

--- linux.orig/drivers/acpi/internal.h
+++ linux/drivers/acpi/internal.h

@@ -93,4 +93,11 @@ static inline int suspend_nvs_save(void)
 static inline void suspend_nvs_restore(void) {}
 #endif

+/*--------------------------------------------------------------------------
+                               Platform bus support
+  -------------------------------------------------------------------------- */
+struct platform_device;
+
+struct platform_device *acpi_create_platform_device(struct acpi_device *adev);
+
 #endif /* _ACPI_INTERNAL_H_ */

Index: linux/drivers/acpi/scan.c
===================================================================

--- linux.orig/drivers/acpi/scan.c
+++ linux/drivers/acpi/scan.c

@@ -29,6 +29,15 @@ extern struct acpi_device *acpi_root;

 static const char *dummy_hid = "device";

+/*
+ * The following ACPI IDs are known to be suitable for representing as
+ * platform devices.
+ */
+static const struct acpi_device_id acpi_platform_device_ids[] = {
+

?

Yes, empty list.  We're going to populate it later.

quoted

+       { }
+};
+
 static LIST_HEAD(acpi_device_list);
 static LIST_HEAD(acpi_bus_id_list);
 DEFINE_MUTEX(acpi_device_lock);

@@ -1544,8 +1553,13 @@ static acpi_status acpi_bus_check_add(ac
         */
        device = NULL;
        acpi_bus_get_device(handle, &device);
-       if (ops->acpi_op_add && !device)
+       if (ops->acpi_op_add && !device) {
                acpi_add_single_object(&device, handle, type, sta, ops);
+               /* Is the device a known good platform device? */
+               if (device
+                   && !acpi_match_device_ids(device, acpi_platform_device_ids))
+                       acpi_create_platform_device(device);
+       }

That is ugly! any reason for not using acpi_driver for them.

Yes, a couple of reasons.  First off, there are existing platform drivers for
these things already and there's no reason for creating a "glue" layer between
those drivers and struct acpi_device objects.  Second, we're going to get rid
of acpi_driver things entirely going forward (the existing ones will be replaced
by platform drivers or included into the ACPI core).

there is not reason to treat those platform_device different from pci
root device and other pnp device.

Well, I agree. :-)

So those things you're talking about we'll be platform devices too in the
future.

something like attached patch. .add of acpi_driver ops could call
acpi_create_platform_device.

Been there, decided to do it differently this time.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.