[PATCH 0/5] ACPI-based enumeration of devices that are not discoverable natively

[PATCH 0/5] ACPI-based enumeration of devices that are not discoverable natively

[Rafael J. Wysocki]

Hi All,

The following patch series adds support for ACPI-based enumeration of devices
that aren't discoverable natively to the driver core and ACPI subsystem.  It
covers the generic part and the platform bus type support.

Since in this case the role of ACPI is analogous to the role of Device Trees,
the idea is to follow what Device Trees did previously.

[1/5] Move ACPI handle to struct device and add device IDs table to
      struct device_driver.

[2/5] Generic functions for matching ACPI device nodes.

[3/5] Export GSI registration/unregistration functions on x86 (needed by [5/5]).

[4/5] Export GSI registration/unregistration functions on ia64 (needed by [5/5]).

[5/5] ACPI-based device enumeration support for platform devices.

The patches are on top of the linux-next branch of the linux-pm.git tree
with the "ACPI / PM: ACPI power management callback routines for subsystems"
patch series applied (although that series is not strictly necessary for them
to work).  They have been tested by Mika.

Thanks,
Rafael


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

[PATCH 2/5] ACPI: Provide generic functions for matching ACPI device nodes

[Rafael J. Wysocki]

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

Introduce function acpi_match_device() allowing callers to match
struct device objects with populated acpi_handle fields against
arrays of ACPI device IDs.  Also introduce function
acpi_driver_match_device() using acpi_match_device() internally and
allowing callers to match a struct device object against an array of
ACPI device IDs provided by a device driver.

Additionally, introduce macro ACPI_PTR() that may be used by device
drivers to escape pointers to data structures whose definitions
depend on CONFIG_ACPI.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/scan.c  |   40 +++++++++++++++++++++++++++++++++++-----
 include/linux/acpi.h |   28 ++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 5 deletions(-)

Index: linux/drivers/acpi/scan.c
===================================================================
--- linux.orig/drivers/acpi/scan.c
+++ linux/drivers/acpi/scan.c
@@ -372,8 +372,8 @@ static void acpi_device_remove_files(str
 			ACPI Bus operations
    -------------------------------------------------------------------------- */
 
-int acpi_match_device_ids(struct acpi_device *device,
-			  const struct acpi_device_id *ids)
+static const struct acpi_device_id *__acpi_match_device(
+	struct acpi_device *device, const struct acpi_device_id *ids)
 {
 	const struct acpi_device_id *id;
 	struct acpi_hardware_id *hwid;
@@ -383,14 +383,44 @@ int acpi_match_device_ids(struct acpi_de
 	 * driver for it.
 	 */
 	if (!device->status.present)
-		return -ENODEV;
+		return NULL;
 
 	for (id = ids; id->id[0]; id++)
 		list_for_each_entry(hwid, &device->pnp.ids, list)
 			if (!strcmp((char *) id->id, hwid->id))
-				return 0;
+				return id;
+
+	return NULL;
+}
+
+/**
+ * acpi_match_device - Match a struct device against a given list of ACPI IDs
+ * @ids: Array of struct acpi_device_id object to match against.
+ * @dev: The device structure to match.
+ *
+ * Check if @dev has a valid ACPI handle and if there is a struct acpi_device
+ * object for that handle and use that object to match against a given list of
+ * device IDs.
+ *
+ * Return a pointer to the first matching ID on success or %NULL on failure.
+ */
+const struct acpi_device_id *acpi_match_device(const struct acpi_device_id *ids,
+					       const struct device *dev)
+{
+	struct acpi_device *adev;
 
-	return -ENOENT;
+	if (!ids || !dev->acpi_handle
+	    || ACPI_FAILURE(acpi_bus_get_device(dev->acpi_handle, &adev)))
+		return NULL;
+
+	return __acpi_match_device(adev, ids);
+}
+EXPORT_SYMBOL_GPL(acpi_match_device);
+
+int acpi_match_device_ids(struct acpi_device *device,
+			  const struct acpi_device_id *ids)
+{
+	return __acpi_match_device(device, ids) ? 0 : -ENOENT;
 }
 EXPORT_SYMBOL(acpi_match_device_ids);
 
Index: linux/include/linux/acpi.h
===================================================================
--- linux.orig/include/linux/acpi.h
+++ linux/include/linux/acpi.h
@@ -26,6 +26,7 @@
 #define _LINUX_ACPI_H
 
 #include <linux/ioport.h>	/* for struct resource */
+#include <linux/device.h>
 
 #ifdef	CONFIG_ACPI
 
@@ -368,6 +369,17 @@ extern int acpi_nvs_register(__u64 start
 extern int acpi_nvs_for_each_region(int (*func)(__u64, __u64, void *),
 				    void *data);
 
+const struct acpi_device_id *acpi_match_device(const struct acpi_device_id *ids,
+					       const struct device *dev);
+
+static inline bool acpi_driver_match_device(struct device *dev,
+					    const struct device_driver *drv)
+{
+	return !!acpi_match_device(drv->acpi_match_table, dev);
+}
+
+#define ACPI_PTR(_ptr)	(_ptr)
+
 #else	/* !CONFIG_ACPI */
 
 #define acpi_disabled 1
@@ -422,6 +434,22 @@ static inline int acpi_nvs_for_each_regi
 	return 0;
 }
 
+struct acpi_device_id;
+
+static inline const struct acpi_device_id *acpi_match_device(
+	const struct acpi_device_id *ids, const struct device *dev)
+{
+	return NULL;
+}
+
+static inline bool acpi_driver_match_device(struct device *dev,
+					    const struct device_driver *drv)
+{
+	return false;
+}
+
+#define ACPI_PTR(_ptr)	(NULL)
+
 #endif	/* !CONFIG_ACPI */
 
 #ifdef CONFIG_ACPI

[PATCH 3/5] ACPI / x86: Export acpi_[un]register_gsi()

[Rafael J. Wysocki]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

These functions might be called from modules as well so make sure
they are exported.

In addition, implement empty version of acpi_unregister_gsi() and
remove the one from pci_irq.c.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 arch/x86/kernel/acpi/boot.c |    6 ++++++
 drivers/acpi/pci_irq.c      |    5 -----
 2 files changed, 6 insertions(+), 5 deletions(-)

Index: linux/arch/x86/kernel/acpi/boot.c
===================================================================
--- linux.orig/arch/x86/kernel/acpi/boot.c
+++ linux/arch/x86/kernel/acpi/boot.c
@@ -574,6 +574,12 @@ int acpi_register_gsi(struct device *dev
 
 	return irq;
 }
+EXPORT_SYMBOL_GPL(acpi_register_gsi);
+
+void acpi_unregister_gsi(u32 gsi)
+{
+}
+EXPORT_SYMBOL_GPL(acpi_unregister_gsi);
 
 void __init acpi_set_irq_model_pic(void)
 {
Index: linux/drivers/acpi/pci_irq.c
===================================================================
--- linux.orig/drivers/acpi/pci_irq.c
+++ linux/drivers/acpi/pci_irq.c
@@ -495,11 +495,6 @@ int acpi_pci_irq_enable(struct pci_dev *
 	return 0;
 }
 
-/* FIXME: implement x86/x86_64 version */
-void __attribute__ ((weak)) acpi_unregister_gsi(u32 i)
-{
-}
-
 void acpi_pci_irq_disable(struct pci_dev *dev)
 {
 	struct acpi_prt_entry *entry;

[PATCH 4/5] ACPI / ia64: Export acpi_[un]register_gsi()

[Rafael J. Wysocki]

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

These functions might be called from modules as well so make sure
they are exported.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 arch/ia64/kernel/acpi.c |    2 ++
 1 file changed, 2 insertions(+)

Index: linux/arch/ia64/kernel/acpi.c
===================================================================
--- linux.orig/arch/ia64/kernel/acpi.c
+++ linux/arch/ia64/kernel/acpi.c
@@ -633,6 +633,7 @@ int acpi_register_gsi(struct device *dev
 				      ACPI_EDGE_SENSITIVE) ? IOSAPIC_EDGE :
 				     IOSAPIC_LEVEL);
 }
+EXPORT_SYMBOL_GPL(acpi_register_gsi);
 
 void acpi_unregister_gsi(u32 gsi)
 {
@@ -644,6 +645,7 @@ void acpi_unregister_gsi(u32 gsi)
 
 	iosapic_unregister_intr(gsi);
 }
+EXPORT_SYMBOL_GPL(acpi_unregister_gsi);
 
 static int __init acpi_parse_fadt(struct acpi_table_header *table)
 {

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

[Rafael J. Wysocki]

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 <mathias.nyman@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 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(-)
 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;
+}
+
+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[] = {
+
+	{ }
+};
+
 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);
+	}
 
 	if (!device)
 		return AE_CTRL_DEPTH;
Index: linux/drivers/base/platform.c
===================================================================
--- linux.orig/drivers/base/platform.c
+++ linux/drivers/base/platform.c
@@ -21,6 +21,7 @@
 #include <linux/slab.h>
 #include <linux/pm_runtime.h>
 #include <linux/idr.h>
+#include <linux/acpi.h>
 
 #include "base.h"
 #include "power/power.h"
@@ -702,6 +703,10 @@ static int platform_match(struct device
 	if (of_driver_match_device(dev, drv))
 		return 1;
 
+	/* Then try ACPI style match */
+	if (acpi_driver_match_device(dev, drv))
+		return 1;
+
 	/* Then try to match against the id table */
 	if (pdrv->id_table)
 		return platform_match_id(pdrv->id_table, pdev) != NULL;

[PATCH 1/5] driver core / ACPI: Move ACPI support to core device and driver types

[Rafael J. Wysocki]

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

With ACPI 5 we are starting to see devices that don't natively support
discovery but can be enumerated with the help of the ACPI namespace.
Typically, these devices can be represented in the Linux device driver
model as platform devices or some serial bus devices, like SPI or I2C
devices.

Since we want to re-use existing drivers for those devices, we need a
way for drivers to specify the ACPI IDs of supported devices, so that
they can be matched against device nodes in the ACPI namespace.  To
this end, it is sufficient to add a pointer to an array of supported
ACPI device IDs, that can be provided by the driver, to struct device.

Moreover, things like ACPI power management need to have access to
the ACPI handle of each supported device, because that handle is used
to invoke AML methods associated with the corresponding ACPI device
node.  The ACPI handles of devices are now stored in the archdata
member structure of struct device whose definition depends on the
architecture and includes the ACPI handle only on x86 and ia64. Since
the pointer to an array of supported ACPI IDs is added to struct
device_driver in an architecture-independent way, it is logical to
move the ACPI handle from archdata to struct device itself at the same
time.  This also makes code more straightforward in some places and
follows the example of Device Trees that have a poiter to struct
device_node in there too.

This changeset is based on Mika Westerberg's work.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 arch/ia64/include/asm/device.h |    3 ---
 arch/x86/include/asm/device.h  |    3 ---
 drivers/acpi/glue.c            |   14 ++++++--------
 include/acpi/acpi_bus.h        |    2 +-
 include/linux/device.h         |    4 ++++
 5 files changed, 11 insertions(+), 15 deletions(-)

Index: linux/drivers/acpi/glue.c
===================================================================
--- linux.orig/drivers/acpi/glue.c
+++ linux/drivers/acpi/glue.c
@@ -134,7 +134,7 @@ static int acpi_bind_one(struct device *
 	char physical_node_name[sizeof(PHYSICAL_NODE_STRING) + 2];
 	int retval = -EINVAL;
 
-	if (dev->archdata.acpi_handle) {
+	if (dev->acpi_handle) {
 		dev_warn(dev, "Drivers changed 'acpi_handle'\n");
 		return -EINVAL;
 	}
@@ -169,7 +169,7 @@ static int acpi_bind_one(struct device *
 	acpi_dev->physical_node_count++;
 	mutex_unlock(&acpi_dev->physical_node_lock);
 
-	dev->archdata.acpi_handle = handle;
+	dev->acpi_handle = handle;
 
 	if (!physical_node->node_id)
 		strcpy(physical_node_name, PHYSICAL_NODE_STRING);
@@ -198,11 +198,10 @@ static int acpi_unbind_one(struct device
 	acpi_status status;
 	struct list_head *node, *next;
 
-	if (!dev->archdata.acpi_handle)
+	if (!dev->acpi_handle)
 		return 0;
 
-	status = acpi_bus_get_device(dev->archdata.acpi_handle,
-		&acpi_dev);
+	status = acpi_bus_get_device(dev->acpi_handle, &acpi_dev);
 	if (ACPI_FAILURE(status))
 		goto err;
 
@@ -228,7 +227,7 @@ static int acpi_unbind_one(struct device
 
 		sysfs_remove_link(&acpi_dev->dev.kobj, physical_node_name);
 		sysfs_remove_link(&dev->kobj, "firmware_node");
-		dev->archdata.acpi_handle = NULL;
+		dev->acpi_handle = NULL;
 		/* acpi_bind_one increase refcnt by one */
 		put_device(dev);
 		kfree(entry);
@@ -269,8 +268,7 @@ static int acpi_platform_notify(struct d
 	if (!ret) {
 		struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 
-		acpi_get_name(dev->archdata.acpi_handle,
-			      ACPI_FULL_PATHNAME, &buffer);
+		acpi_get_name(dev->acpi_handle, ACPI_FULL_PATHNAME, &buffer);
 		DBG("Device %s -> %s\n", dev_name(dev), (char *)buffer.pointer);
 		kfree(buffer.pointer);
 	} else
Index: linux/include/acpi/acpi_bus.h
===================================================================
--- linux.orig/include/acpi/acpi_bus.h
+++ linux/include/acpi/acpi_bus.h
@@ -410,7 +410,7 @@ acpi_handle acpi_get_child(acpi_handle,
 int acpi_is_root_bridge(acpi_handle);
 acpi_handle acpi_get_pci_rootbridge_handle(unsigned int, unsigned int);
 struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle);
-#define DEVICE_ACPI_HANDLE(dev) ((acpi_handle)((dev)->archdata.acpi_handle))
+#define DEVICE_ACPI_HANDLE(dev) ((acpi_handle)((dev)->acpi_handle))
 
 int acpi_enable_wakeup_device_power(struct acpi_device *dev, int state);
 int acpi_disable_wakeup_device_power(struct acpi_device *dev);
Index: linux/include/linux/device.h
===================================================================
--- linux.orig/include/linux/device.h
+++ linux/include/linux/device.h
@@ -190,6 +190,7 @@ extern struct klist *bus_get_device_klis
  * @mod_name:	Used for built-in modules.
  * @suppress_bind_attrs: Disables bind/unbind via sysfs.
  * @of_match_table: The open firmware table.
+ * @acpi_match_table: The ACPI match table.
  * @probe:	Called to query the existence of a specific device,
  *		whether this driver can work with it, and bind the driver
  *		to a specific device.
@@ -223,6 +224,7 @@ struct device_driver {
 	bool suppress_bind_attrs;	/* disables bind/unbind via sysfs */
 
 	const struct of_device_id	*of_match_table;
+	const struct acpi_device_id	*acpi_match_table;
 
 	int (*probe) (struct device *dev);
 	int (*remove) (struct device *dev);
@@ -616,6 +618,7 @@ struct device_dma_parameters {
  * @dma_mem:	Internal for coherent mem override.
  * @archdata:	For arch-specific additions.
  * @of_node:	Associated device tree node.
+ * @acpi_handle: Associated ACPI device node's namespace handle.
  * @devt:	For creating the sysfs "dev".
  * @id:		device instance
  * @devres_lock: Spinlock to protect the resource of the device.
@@ -680,6 +683,7 @@ struct device {
 	struct dev_archdata	archdata;
 
 	struct device_node	*of_node; /* associated device tree node */
+	void			*acpi_handle; /* associated ACPI device node */
 
 	dev_t			devt;	/* dev_t, creates the sysfs "dev" */
 	u32			id;	/* device instance */
Index: linux/arch/x86/include/asm/device.h
===================================================================
--- linux.orig/arch/x86/include/asm/device.h
+++ linux/arch/x86/include/asm/device.h
@@ -2,9 +2,6 @@
 #define _ASM_X86_DEVICE_H
 
 struct dev_archdata {
-#ifdef CONFIG_ACPI
-	void	*acpi_handle;
-#endif
 #ifdef CONFIG_X86_DEV_DMA_OPS
 	struct dma_map_ops *dma_ops;
 #endif
Index: linux/arch/ia64/include/asm/device.h
===================================================================
--- linux.orig/arch/ia64/include/asm/device.h
+++ linux/arch/ia64/include/asm/device.h
@@ -7,9 +7,6 @@
 #define _ASM_IA64_DEVICE_H
 
 struct dev_archdata {
-#ifdef CONFIG_ACPI
-	void	*acpi_handle;
-#endif
 #ifdef CONFIG_INTEL_IOMMU
 	void *iommu; /* hook for IOMMU specific extension */
 #endif

Re: [PATCH 1/5] driver core / ACPI: Move ACPI support to core device and driver types

[Greg Kroah-Hartman]

On Wed, Oct 31, 2012 at 10:38:29AM +0100, Rafael J. Wysocki wrote:
> From: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> With ACPI 5 we are starting to see devices that don't natively support
> discovery but can be enumerated with the help of the ACPI namespace.
> Typically, these devices can be represented in the Linux device driver
> model as platform devices or some serial bus devices, like SPI or I2C
> devices.
> 
> Since we want to re-use existing drivers for those devices, we need a
> way for drivers to specify the ACPI IDs of supported devices, so that
> they can be matched against device nodes in the ACPI namespace.  To
> this end, it is sufficient to add a pointer to an array of supported
> ACPI device IDs, that can be provided by the driver, to struct device.
> 
> Moreover, things like ACPI power management need to have access to
> the ACPI handle of each supported device, because that handle is used
> to invoke AML methods associated with the corresponding ACPI device
> node.  The ACPI handles of devices are now stored in the archdata
> member structure of struct device whose definition depends on the
> architecture and includes the ACPI handle only on x86 and ia64. Since
> the pointer to an array of supported ACPI IDs is added to struct
> device_driver in an architecture-independent way, it is logical to
> move the ACPI handle from archdata to struct device itself at the same
> time.  This also makes code more straightforward in some places and
> follows the example of Device Trees that have a poiter to struct
> device_node in there too.
> 
> This changeset is based on Mika Westerberg's work.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  arch/ia64/include/asm/device.h |    3 ---
>  arch/x86/include/asm/device.h  |    3 ---
>  drivers/acpi/glue.c            |   14 ++++++--------
>  include/acpi/acpi_bus.h        |    2 +-
>  include/linux/device.h         |    4 ++++
>  5 files changed, 11 insertions(+), 15 deletions(-)

The driver core pieces look fine to me:

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 1/5] driver core / ACPI: Move ACPI support to core device and driver types

[Rafael J. Wysocki]

On Wednesday, October 31, 2012 09:24:07 AM Greg Kroah-Hartman wrote:
> On Wed, Oct 31, 2012 at 10:38:29AM +0100, Rafael J. Wysocki wrote:
> > From: Mika Westerberg <mika.westerberg@linux.intel.com>
> > 
> > With ACPI 5 we are starting to see devices that don't natively support
> > discovery but can be enumerated with the help of the ACPI namespace.
> > Typically, these devices can be represented in the Linux device driver
> > model as platform devices or some serial bus devices, like SPI or I2C
> > devices.
> > 
> > Since we want to re-use existing drivers for those devices, we need a
> > way for drivers to specify the ACPI IDs of supported devices, so that
> > they can be matched against device nodes in the ACPI namespace.  To
> > this end, it is sufficient to add a pointer to an array of supported
> > ACPI device IDs, that can be provided by the driver, to struct device.
> > 
> > Moreover, things like ACPI power management need to have access to
> > the ACPI handle of each supported device, because that handle is used
> > to invoke AML methods associated with the corresponding ACPI device
> > node.  The ACPI handles of devices are now stored in the archdata
> > member structure of struct device whose definition depends on the
> > architecture and includes the ACPI handle only on x86 and ia64. Since
> > the pointer to an array of supported ACPI IDs is added to struct
> > device_driver in an architecture-independent way, it is logical to
> > move the ACPI handle from archdata to struct device itself at the same
> > time.  This also makes code more straightforward in some places and
> > follows the example of Device Trees that have a poiter to struct
> > device_node in there too.
> > 
> > This changeset is based on Mika Westerberg's work.
> > 
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  arch/ia64/include/asm/device.h |    3 ---
> >  arch/x86/include/asm/device.h  |    3 ---
> >  drivers/acpi/glue.c            |   14 ++++++--------
> >  include/acpi/acpi_bus.h        |    2 +-
> >  include/linux/device.h         |    4 ++++
> >  5 files changed, 11 insertions(+), 15 deletions(-)
> 
> The driver core pieces look fine to me:
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Great, thanks!

I wonder if the x86 and/or ia64 maintainers have any reservations?

Rafael


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

Re: [PATCH 1/5] driver core / ACPI: Move ACPI support to core device and driver types

[H. Peter Anvin]

On 10/31/2012 11:42 AM, Rafael J. Wysocki wrote:
>
> Great, thanks!
>
> I wonder if the x86 and/or ia64 maintainers have any reservations?
>

None here.

Acked-by: H. Peter Anvin <hpa@zytor.com>

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH 1/5] driver core / ACPI: Move ACPI support to core device and driver types

[Rafael J. Wysocki]

On Wednesday, October 31, 2012 11:44:45 AM H. Peter Anvin wrote:
> On 10/31/2012 11:42 AM, Rafael J. Wysocki wrote:
> >
> > Great, thanks!
> >
> > I wonder if the x86 and/or ia64 maintainers have any reservations?
> >
> 
> None here.
> 
> Acked-by: H. Peter Anvin <hpa@zytor.com>

Thanks!


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

RE: [PATCH 1/5] driver core / ACPI: Move ACPI support to core device and driver types

[Luck, Tony]

On 10/31/2012 11:42 AM, Rafael J. Wysocki wrote:
> I wonder if the x86 and/or ia64 maintainers have any reservations?

Can you elaborate on the "tested by mika" that you put into the 0/5
message. Especially w.r.t. ia64. Compile tested? Boot tested? Ran with
some new device that uses the ACPI enumeration provided by this series?

Nothing in the concept or code scares me ... but I'd like to know that it
actually works :-)

-Tony

RE: [PATCH 1/5] driver core / ACPI: Move ACPI support to core device and driver types

[Luck, Tony]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 591 bytes --]

On 10/31/2012 11:42 AM, Rafael J. Wysocki wrote:
> I wonder if the x86 and/or ia64 maintainers have any reservations?

Can you elaborate on the "tested by mika" that you put into the 0/5
message. Especially w.r.t. ia64. Compile tested? Boot tested? Ran with
some new device that uses the ACPI enumeration provided by this series?

Nothing in the concept or code scares me ... but I'd like to know that it
actually works :-)

-Tony
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 1/5] driver core / ACPI: Move ACPI support to core device and driver types

[Rafael J. Wysocki]

On Wednesday, October 31, 2012 08:03:34 PM Luck, Tony wrote:
> On 10/31/2012 11:42 AM, Rafael J. Wysocki wrote:
> > I wonder if the x86 and/or ia64 maintainers have any reservations?
> 
> Can you elaborate on the "tested by mika" that you put into the 0/5
> message. Especially w.r.t. ia64. Compile tested? Boot tested? Ran with
> some new device that uses the ACPI enumeration provided by this series?

By "tested" I mean "run with some new devices that use the ACPI enumeration
provided here, on x86".  Sorry for being too vague.

> Nothing in the concept or code scares me ... but I'd like to know that it
> actually works :-)

Sure.

Thanks,
Rafael


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

RE: [PATCH 1/5] driver core / ACPI: Move ACPI support to core device and driver types

[Luck, Tony]

> By "tested" I mean "run with some new devices that use the ACPI enumeration
> provided here, on x86".  Sorry for being too vague.

Do you or Mika have access to an ia64 box to test.  If not, can you suggest
some way that I could exercise this code w/o the new devices. Or at least
reassure myself that all is benign in a system full of old devices.

-Tony

RE: [PATCH 1/5] driver core / ACPI: Move ACPI support to core device and driver types

[Luck, Tony]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 518 bytes --]

> By "tested" I mean "run with some new devices that use the ACPI enumeration
> provided here, on x86".  Sorry for being too vague.

Do you or Mika have access to an ia64 box to test.  If not, can you suggest
some way that I could exercise this code w/o the new devices. Or at least
reassure myself that all is benign in a system full of old devices.

-Tony
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 1/5] driver core / ACPI: Move ACPI support to core device and driver types

[Rafael J. Wysocki]

On Wednesday, October 31, 2012 08:33:53 PM Luck, Tony wrote:
> > By "tested" I mean "run with some new devices that use the ACPI enumeration
> > provided here, on x86".  Sorry for being too vague.
> 
> Do you or Mika have access to an ia64 box to test.

I don't.  I'm not sure about Mika.

> If not, can you suggest some way that I could exercise this code w/o the new
> devices. Or at least reassure myself that all is benign in a system full of
> old devices.

There are two parts of the new code, one that's always executed, regardless of
whether or not there are devices using the support provided by this series, and
the other that's executed only for those devices.

All depends on what's there in acpi_platform_device_ids[] (added by patch [5/5]),
which is empty for now (Mika has a separate patch adding some IDs in there).
The second part of the new code will only be run and platform device objects
will only be created by it if there is at least one entry in
acpi_platform_device_ids[] matching a device ID of an ACPI node in the BIOS.

The BIOSes of currently available ia64 systems don't contain ACPI nodes whose
IDs will match the IDs of the new devices (ie. the ones that are going to be
added to acpi_platform_device_ids[]), so for ia64 it should be sufficient to
test that code as is (ie. without any new devices in the system).

Thanks,
Rafael


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

Re: [PATCH 1/5] driver core / ACPI: Move ACPI support to core device and driver types

[Mika Westerberg]

On Wed, Oct 31, 2012 at 10:06:00PM +0100, Rafael J. Wysocki wrote:
> On Wednesday, October 31, 2012 08:33:53 PM Luck, Tony wrote:
> > > By "tested" I mean "run with some new devices that use the ACPI enumeration
> > > provided here, on x86".  Sorry for being too vague.
> > 
> > Do you or Mika have access to an ia64 box to test.
> 
> I don't.  I'm not sure about Mika.

I also don't have access to any ia64 machine.

As Rafael said, I tested this on x86 machine only.

RE: [PATCH 1/5] driver core / ACPI: Move ACPI support to core device and driver types

[Luck, Tony]

> The BIOSes of currently available ia64 systems don't contain ACPI nodes whose
> IDs will match the IDs of the new devices (ie. the ones that are going to be
> added to acpi_platform_device_ids[]), so for ia64 it should be sufficient to
> test that code as is (ie. without any new devices in the system).

Ok - built cleanly on ia64.  Boots too. Just one new console message:

ACPI: bus type platform registered

that seems pretty harmless.

Acked-by: Tony Luck <tony.luck@intel.com>

RE: [PATCH 1/5] driver core / ACPI: Move ACPI support to core device and driver types

[Luck, Tony]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 645 bytes --]

> The BIOSes of currently available ia64 systems don't contain ACPI nodes whose
> IDs will match the IDs of the new devices (ie. the ones that are going to be
> added to acpi_platform_device_ids[]), so for ia64 it should be sufficient to
> test that code as is (ie. without any new devices in the system).

Ok - built cleanly on ia64.  Boots too. Just one new console message:

ACPI: bus type platform registered

that seems pretty harmless.

Acked-by: Tony Luck <tony.luck@intel.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 1/5] driver core / ACPI: Move ACPI support to core device and driver types

[Rafael J. Wysocki]

On Wednesday, October 31, 2012 09:36:36 PM Luck, Tony wrote:
> > The BIOSes of currently available ia64 systems don't contain ACPI nodes whose
> > IDs will match the IDs of the new devices (ie. the ones that are going to be
> > added to acpi_platform_device_ids[]), so for ia64 it should be sufficient to
> > test that code as is (ie. without any new devices in the system).
> 
> Ok - built cleanly on ia64.  Boots too. Just one new console message:
> 
> ACPI: bus type platform registered
> 
> that seems pretty harmless.
> 
> Acked-by: Tony Luck <tony.luck@intel.com>

Thanks a lot!


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

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

[Yinghai Lu]

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

On Wed, Oct 31, 2012 at 2:36 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> 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 <mathias.nyman@linux.intel.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  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.

>  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?

> +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[] = {
> +

?

> +       { }
> +};
> +
>  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.

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

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

Yinghai

[-- Attachment #2: acpi_platform_driver.patch --]
[-- Type: application/octet-stream, Size: 2369 bytes --]

diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
index a5a2346..4ed97a7 100644
--- a/drivers/acpi/acpi_platform.c
+++ b/drivers/acpi/acpi_platform.c
@@ -230,6 +230,44 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev)
 	return pdev;
 }
 
+/*
+ * The following ACPI IDs are known to be suitable for representing as
+ * platform devices.
+ */
+static const struct acpi_device_id acpi_platform_device_ids[] = {
+
+	{ }
+};
+
+#define ACPI_PLATFORM_CLASS		"acpi_platform"
+static int __devinit acpi_platform_device_add(struct acpi_device *device)
+{
+	acpi_create_platform_device(device);
+}
+static int acpi_platform_device_remove(struct acpi_device *device, int type)
+{
+	/* TODO: remove that platfrom_device and resource */
+}
+
+static struct acpi_driver acpi_platform_driver = {
+	.name = "acpi_platform",
+	.class = ACPI_PLATFORM_CLASS,
+	.ids = acpi_platform_device_ids,
+	.ops = {
+		.add = acpi_platform_device_add,
+		.remove = acpi_platform_device_remove,
+		},
+};
+
+static int __init acpi_platform_init(void)
+{
+	if (acpi_bus_register_driver(&acpi_platform_driver) < 0)
+		return -ENODEV;
+
+	return 0;
+}
+subsys_initcall(acpi_platform_init);
+
 static acpi_status acpi_platform_match(acpi_handle handle, u32 depth,
 				       void *data, void **return_value)
 {
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index e7afba1..73bfa3d 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -29,15 +29,6 @@ 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[] = {
-
-	{ }
-};
-
 static LIST_HEAD(acpi_device_list);
 static LIST_HEAD(acpi_bus_id_list);
 DEFINE_MUTEX(acpi_device_lock);
@@ -1553,13 +1544,8 @@ static acpi_status acpi_bus_check_add(acpi_handle handle, u32 lvl,
 	 */
 	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);
-	}
 
 	if (!device)
 		return AE_CTRL_DEPTH;

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

[Rafael J. Wysocki]

On Wednesday, October 31, 2012 11:34:24 PM Yinghai Lu wrote:
> On Wed, Oct 31, 2012 at 2:36 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > 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 <mathias.nyman@linux.intel.com>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  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.
> 
> >  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.

> > +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.

> > +       { }
> > +};
> > +
> >  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.

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

[Yinghai Lu]

On Thu, Nov 1, 2012 at 7:35 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> > @@ -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).

that should be glue between acpi_device and platform_device.

how are you going to handle removing path ? aka when acpi_device get
trimed, how those platform get deleted?

>
>> 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.
>

So you are going to replace acpi_device/acpi_driver with
platform_device/platform_driver ?

Did you ever try to start to the work to see if it is doable? aka do
you have draft version to do that?

Yinghai

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

[Rafael J. Wysocki]

On Thursday, November 01, 2012 09:19:59 AM Yinghai Lu wrote:
> On Thu, Nov 1, 2012 at 7:35 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> > @@ -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).
> 
> that should be glue between acpi_device and platform_device.
> 
> how are you going to handle removing path ? aka when acpi_device get
> trimed, how those platform get deleted?

This is a valid point.

Roughly, the idea is to walk the dev's list of physical nodes and call
.remove() for each of them in acpi_bus_remove().  Or to do something
equivalent to that.

However, we're not going to add any IDs of removable devices to
acpi_platform_device_ids[] for now, so we simply don't need to modify that
code path just yet (we'll modify it when we're about to add such devices to
that table).

> >> 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.
> >
> 
> So you are going to replace acpi_device/acpi_driver with
> platform_device/platform_driver ?

Not exactly.  Let me start from the big picture, though. :-)

First off, we need to unify the handling of devices by the ACPI subsystem
regardless of whether they are on a specific bus, like PCI, or they are
busless "platform" devices.

Currently, if a device is on a specific bus *and* there is a device node in the
ACPI namespace corresponding to it, there will be two objects based on
struct device for it eventually, the "physical node", like struct pci_dev, and
the "ACPI node" represented by struct acpi_device.  They are associated with
each other through the code in drivers/acpi/glue.c.  In turn, if the device is
busless and not discoverable natively, we only create the "ACPI node" struct
acpi_device thing for it.  Those ACPI nodes are then *sometimes* bind to
drivers (represented by struct acpi_driver).

The fact that the busless devices are *sometimes* handled by binding drivers
directly to struct acpi_device while the other devices are handled through
glue.c confuses things substantially and causes problems to happen right now
(for example, acpi_driver drivers sometimes attempt to bind to things that have
other native drivers and should really be handled by them).
Furthermore, the situation will only get worse over time if we don't do
anything about that, because we're going to see more and more devices that
won't be discoverable natively and will have corresponding nodes in the ACPI
namespace and we're going to see more buses whose devices will have such
nodes.

Moreover, for many of those devices there are native drivers present in
the kernel tree already, because they will be based on IP blocks used in
the current hardware (for example, we may see ARM-based systems based on
exactly the same hardware with ACPI BIOSes and without them).  That applies
to busless devices as well as to devices on specific buses.

Now, the problem is how the unification is going to be done and I honestly
don't think we have much *choice* here.  Namely, for PCI (and other devices
discoverable natively) we pretty much have to do the glue.c thing (or something
equivalent), because we need to match what we've discovered natively against
the information from the ACPI tables in the BIOS.  This means that for busless
devices we need to create "physical" nodes as well, so that all of them are
handled by drivers binding to the "physical" node rather than to struct
acpi_device.  This also will allow us to reuse the existing drivers with
minimum modifications (well, hopefully).

Pretty much every ACPI developer I have discussed that with so far, including
people like Matthew Garrett and Zhang Rui who have been working on ACPI for
years, seems to agree that this is the way to go.

Thus, in the long run, acpi_driver drivers will be replaced by something else
(platform drivers seem to be a natural choice in many cases), but struct
acpi_device objects won't go away, at least not entirely.  My long haul plan
is to drop the "dev" component of struct acpi_device and rename it to something
like struct acpi_dev_node, to clarify its meaning.

I'm quite confident that this is doable.  The part I'm most concerned about is
the interactions with user space, because we need to pay attention not to break
the existing user space interfaces (that are actually used).  That said, I'm
not expecting this to be a one-shot transition.  Rather, this is going to take
time, like several kernel releases, and I'm sure there are details we need to
be very careful about.

That's one of the reasons why acpi_platform_device_ids[] is there: so that we
can carefully carry out the unification step by step.

> Did you ever try to start to the work to see if it is doable? aka do
> you have draft version to do that?

Unfortunately, I don't have anything I could share with you at the moment.

Thanks,
Rafael


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

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

[Yinghai Lu]

On Thu, Nov 1, 2012 at 2:21 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> So you are going to replace acpi_device/acpi_driver with
>> platform_device/platform_driver ?
>
> Not exactly.  Let me start from the big picture, though. :-)
>
> First off, we need to unify the handling of devices by the ACPI subsystem
> regardless of whether they are on a specific bus, like PCI, or they are
> busless "platform" devices.
>
> Currently, if a device is on a specific bus *and* there is a device node in the
> ACPI namespace corresponding to it, there will be two objects based on
> struct device for it eventually, the "physical node", like struct pci_dev, and
> the "ACPI node" represented by struct acpi_device.  They are associated with
> each other through the code in drivers/acpi/glue.c.  In turn, if the device is
> busless and not discoverable natively, we only create the "ACPI node" struct
> acpi_device thing for it.  Those ACPI nodes are then *sometimes* bind to
> drivers (represented by struct acpi_driver).
>
> The fact that the busless devices are *sometimes* handled by binding drivers
> directly to struct acpi_device while the other devices are handled through
> glue.c confuses things substantially and causes problems to happen right now
> (for example, acpi_driver drivers sometimes attempt to bind to things that have
> other native drivers and should really be handled by them).
> Furthermore, the situation will only get worse over time if we don't do
> anything about that, because we're going to see more and more devices that
> won't be discoverable natively and will have corresponding nodes in the ACPI
> namespace and we're going to see more buses whose devices will have such
> nodes.
>
> Moreover, for many of those devices there are native drivers present in
> the kernel tree already, because they will be based on IP blocks used in
> the current hardware (for example, we may see ARM-based systems based on
> exactly the same hardware with ACPI BIOSes and without them).  That applies
> to busless devices as well as to devices on specific buses.
>
> Now, the problem is how the unification is going to be done and I honestly
> don't think we have much *choice* here.  Namely, for PCI (and other devices
> discoverable natively) we pretty much have to do the glue.c thing (or something
> equivalent), because we need to match what we've discovered natively against
> the information from the ACPI tables in the BIOS.  This means that for busless
> devices we need to create "physical" nodes as well, so that all of them are
> handled by drivers binding to the "physical" node rather than to struct
> acpi_device.  This also will allow us to reuse the existing drivers with
> minimum modifications (well, hopefully).

ok, acpi_driver will be killed at first.

acpi_pci_root_driver will be converted to platform driver or
add acpi_pci_host_bridge to work with pci_host_bridge.

BTW,  the problem for hotadd pci root bus,
the acpi_driver ops.add can pci root bus and create pci dev before all
acpi device get
created still there.
    https://lkml.org/lkml/2012/10/5/569

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

[Rafael J. Wysocki]

On Thursday, November 01, 2012 03:38:19 PM Yinghai Lu wrote:
> On Thu, Nov 1, 2012 at 2:21 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> So you are going to replace acpi_device/acpi_driver with
> >> platform_device/platform_driver ?
> >
> > Not exactly.  Let me start from the big picture, though. :-)
> >
> > First off, we need to unify the handling of devices by the ACPI subsystem
> > regardless of whether they are on a specific bus, like PCI, or they are
> > busless "platform" devices.
> >
> > Currently, if a device is on a specific bus *and* there is a device node in the
> > ACPI namespace corresponding to it, there will be two objects based on
> > struct device for it eventually, the "physical node", like struct pci_dev, and
> > the "ACPI node" represented by struct acpi_device.  They are associated with
> > each other through the code in drivers/acpi/glue.c.  In turn, if the device is
> > busless and not discoverable natively, we only create the "ACPI node" struct
> > acpi_device thing for it.  Those ACPI nodes are then *sometimes* bind to
> > drivers (represented by struct acpi_driver).
> >
> > The fact that the busless devices are *sometimes* handled by binding drivers
> > directly to struct acpi_device while the other devices are handled through
> > glue.c confuses things substantially and causes problems to happen right now
> > (for example, acpi_driver drivers sometimes attempt to bind to things that have
> > other native drivers and should really be handled by them).
> > Furthermore, the situation will only get worse over time if we don't do
> > anything about that, because we're going to see more and more devices that
> > won't be discoverable natively and will have corresponding nodes in the ACPI
> > namespace and we're going to see more buses whose devices will have such
> > nodes.
> >
> > Moreover, for many of those devices there are native drivers present in
> > the kernel tree already, because they will be based on IP blocks used in
> > the current hardware (for example, we may see ARM-based systems based on
> > exactly the same hardware with ACPI BIOSes and without them).  That applies
> > to busless devices as well as to devices on specific buses.
> >
> > Now, the problem is how the unification is going to be done and I honestly
> > don't think we have much *choice* here.  Namely, for PCI (and other devices
> > discoverable natively) we pretty much have to do the glue.c thing (or something
> > equivalent), because we need to match what we've discovered natively against
> > the information from the ACPI tables in the BIOS.  This means that for busless
> > devices we need to create "physical" nodes as well, so that all of them are
> > handled by drivers binding to the "physical" node rather than to struct
> > acpi_device.  This also will allow us to reuse the existing drivers with
> > minimum modifications (well, hopefully).
> 
> ok, acpi_driver will be killed at first.
> 
> acpi_pci_root_driver will be converted to platform driver or
> add acpi_pci_host_bridge to work with pci_host_bridge.

Yup.

> BTW,  the problem for hotadd pci root bus,
> the acpi_driver ops.add can pci root bus and create pci dev before all
> acpi device get
> created still there.
>     https://lkml.org/lkml/2012/10/5/569

Yes, I'm aware of that, it's on my todo list FWIW. :-)

Thanks,
Rafael


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