Re: [PATCH v2 2/3] spi / ACPI: add ACPI enumeration support
From: Mika Westerberg <mika.westerberg@linux.intel.com>
Date: 2012-11-17 10:15:52
Also in:
lkml
On Sat, Nov 17, 2012 at 11:11:59AM +0100, Rafael J. Wysocki wrote:
quoted
+static acpi_status acpi_spi_add_device(acpi_handle handle, u32 level, + void *data, void **return_value) +{ + struct spi_master *master = data; + struct resource_list_entry *rentry; + struct list_head resource_list; + struct acpi_device *adev; + struct spi_device *spi; + int ret; + + if (acpi_bus_get_device(handle, &adev)) + return AE_OK; + if (acpi_bus_get_status(adev) || !adev->status.present) + return AE_OK; + + spi = spi_alloc_device(master); + if (!spi) { + dev_err(&master->dev, "failed to allocate SPI device for %s\n", + dev_name(&adev->dev)); + return AE_NO_MEMORY; + } + + INIT_LIST_HEAD(&resource_list); + ret = acpi_dev_get_resources(adev, &resource_list, + acpi_spi_add_resource, spi); + if (ret < 0) + goto fail_put_dev; + + list_for_each_entry(rentry, &resource_list, node) { + struct resource *r = &rentry->res; + + if (resource_type(r) == IORESOURCE_IRQ) { + spi->irq = r->start; + break; + } + } + + acpi_dev_free_resource_list(&resource_list);A potential problem is lurking here, or rather two of them. Suppose there are multiple IRQ resources for this device. The code above will cause GSIs to be registered for all of them, but we'll use only one eventually. Moreover, we'll use the last one, but perhaps we should use the first one instead?
Indeed we should use the first one.
Maybe a better approach would be to make acpi_spi_add_resource() call acpi_dev_resource_interrupt(ares, 0, &r) directly and then skip all of the remaining IRQ resources if it finds one? For example set spi->irq to -1 initially, assign it if an IRQ resource is found and skip all of the remaining IRQ resources if it is non-negative?
OK, will do. Thanks.