lkml.org 
[lkml]   [2012]   [Nov]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 5/5] ACPI: Add support for platform bus type
    Date
    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.


    \
     
     \ /
      Last update: 2012-11-01 15:41    [W:4.287 / U:0.392 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site