Re: [Bisected] [-next-20130204] usb/hcd: irq 18: nobody cared
From: Bjorn Helgaas <bhelgaas@google.com>
Date: 2013-02-11 22:41:48
Also in:
linux-acpi, linux-pci, lkml
[+cc linux-acpi, linux-pci] On Mon, Feb 11, 2013 at 01:06:27PM -0800, Yinghai Lu wrote:
On Mon, Feb 11, 2013 at 11:57 AM, Yinghai Lu [off-list ref] wrote:quoted
quoted
Bjorn, Rafael, acpi_pci_irq_add_prt need to be called after pci bridge get scanned, so we can not call it from pci_acpi_setup, after we move dev_register for pci_dev early. The attached debug patch move down that calling into pci_bus_add_devices and that will make prt works again. Can acpi provide another hook after bridge get scanned?Rafael, Bjorn, Can you check attached patch?
[Yinghai's patch included below for ease of review]
quoted hunk ↗ jump to hunk
Subject: [PATCH] ACPI, PCI: add acpi_platform_notify_scan() Peter Hurley found irq nobody cared, and dmesg has [ 8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A [ 8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5 bisect to | commit 4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4 | PCI: Put pci_dev in device tree as early as possible It turns out we need to call acpi_pci_irq_add_prt() after the pci bridges are scanned. Add acpi_platform_notify_scan() to call acpi_pci_irq_add_prt later. Reported-and-tested-by: Peter Hurley <redacted> Signed-off-by: Yinghai Lu <yinghai@kernel.org> --- drivers/acpi/glue.c | 12 ++++++++++++ drivers/base/core.c | 1 + drivers/pci/bus.c | 4 ++++ drivers/pci/pci-acpi.c | 27 +++++++++++++++++---------- include/acpi/acpi_bus.h | 1 + include/linux/device.h | 3 +-- 6 files changed, 36 insertions(+), 12 deletions(-) Index: linux-2.6/drivers/acpi/glue.c ===================================================================--- linux-2.6.orig/drivers/acpi/glue.c +++ linux-2.6/drivers/acpi/glue.c@@ -312,6 +312,17 @@ static int acpi_platform_notify(struct d return ret; } +static int acpi_platform_notify_scan(struct device *dev) +{ + struct acpi_bus_type *type; + + type = acpi_get_bus_type(dev->bus); + if (type && type->setup_scan) + type->setup_scan(dev); + + return 0; +} + static int acpi_platform_notify_remove(struct device *dev) { struct acpi_bus_type *type;@@ -331,6 +342,7 @@ int __init init_acpi_device_notify(void) return 0; } platform_notify = acpi_platform_notify; + platform_notify_scan = acpi_platform_notify_scan; platform_notify_remove = acpi_platform_notify_remove; return 0; }Index: linux-2.6/drivers/base/core.c ===================================================================--- linux-2.6.orig/drivers/base/core.c +++ linux-2.6/drivers/base/core.c@@ -44,6 +44,7 @@ early_param("sysfs.deprecated", sysfs_de #endif int (*platform_notify)(struct device *dev) = NULL; +int (*platform_notify_scan)(struct device *dev) = NULL;
I don't like the idea of adding another global function pointer just for this. That seems like overkill for a small, local, problem.
quoted hunk ↗ jump to hunk
int (*platform_notify_remove)(struct device *dev) = NULL; static struct kobject *dev_kobj; struct kobject *sysfs_dev_char_kobj; Index: linux-2.6/drivers/pci/bus.c ===================================================================--- linux-2.6.orig/drivers/pci/bus.c +++ linux-2.6/drivers/pci/bus.c@@ -170,6 +170,10 @@ int pci_bus_add_device(struct pci_dev *d { int retval; + /* need to be called after bridge is scanned */ + if (platform_notify_scan) + platform_notify_scan(&dev->dev); + /* * Can not put in pci_device_add yet because resources * are not assigned yet for some devices.Index: linux-2.6/drivers/pci/pci-acpi.c ===================================================================--- linux-2.6.orig/drivers/pci/pci-acpi.c +++ linux-2.6/drivers/pci/pci-acpi.c@@ -307,6 +307,22 @@ static void pci_acpi_setup(struct device struct pci_dev *pci_dev = to_pci_dev(dev); acpi_handle handle = ACPI_HANDLE(dev); struct acpi_device *adev; + + if (acpi_bus_get_device(handle, &adev) || !adev->wakeup.flags.valid) + return; + + device_set_wakeup_capable(dev, true); + acpi_pci_sleep_wake(pci_dev, false); + + pci_acpi_add_pm_notifier(adev, pci_dev); + if (adev->wakeup.flags.run_wake) + device_set_run_wake(dev, true); +} + +static void pci_acpi_setup_scan(struct device *dev) +{ + struct pci_dev *pci_dev = to_pci_dev(dev); + acpi_handle handle = ACPI_HANDLE(dev); acpi_status status; acpi_handle dummy;@@ -326,16 +342,6 @@ static void pci_acpi_setup(struct device pci_dev->subordinate->number : pci_dev->bus->number; acpi_pci_irq_add_prt(handle, pci_domain_nr(pci_dev->bus), bus);
The _PRT describes motherboard interrupt wiring, which has nothing to do with PCI bus numbers. Our current drivers/acpi/pci_irq.c caches the PCI bus number along with the _PRT, and I think that's a mistake. The bus number binding means acpi_pci_irq_add_prt() has to happen after enumerating everything below a bridge, and it will prevent us from doing any bus number reassignment for hotplug. I think we should remove the bus numbers from the cached _PRT (or maybe even remove the _PRT caching completely). When we enable a PCI device's IRQ, we should search up the PCI device tree looking for a _PRT associated with each node, and applying normal PCI bridge swizzling when we don't find a _PRT. I think this can be done without using PCI bus numbers at all.
quoted hunk ↗ jump to hunk
} - - if (acpi_bus_get_device(handle, &adev) || !adev->wakeup.flags.valid) - return; - - device_set_wakeup_capable(dev, true); - acpi_pci_sleep_wake(pci_dev, false); - - pci_acpi_add_pm_notifier(adev, pci_dev); - if (adev->wakeup.flags.run_wake) - device_set_run_wake(dev, true); } static void pci_acpi_cleanup(struct device *dev)@@ -359,6 +365,7 @@ static struct acpi_bus_type acpi_pci_bus .bus = &pci_bus_type, .find_device = acpi_pci_find_device, .setup = pci_acpi_setup, + .setup_scan = pci_acpi_setup_scan, .cleanup = pci_acpi_cleanup, };Index: linux-2.6/include/acpi/acpi_bus.h ===================================================================--- linux-2.6.orig/include/acpi/acpi_bus.h +++ linux-2.6/include/acpi/acpi_bus.h@@ -440,6 +440,7 @@ struct acpi_bus_type { /* For bridges, such as PCI root bridge, IDE controller */ int (*find_bridge) (struct device *, acpi_handle *); void (*setup)(struct device *); + void (*setup_scan)(struct device *); void (*cleanup)(struct device *); }; int register_acpi_bus_type(struct acpi_bus_type *);Index: linux-2.6/include/linux/device.h ===================================================================--- linux-2.6.orig/include/linux/device.h +++ linux-2.6/include/linux/device.h@@ -897,10 +897,9 @@ extern void device_destroy(struct class */ /* Notify platform of device discovery */ extern int (*platform_notify)(struct device *dev); - +extern int (*platform_notify_scan)(struct device *dev); extern int (*platform_notify_remove)(struct device *dev); - /* * get_device - atomically increment the reference count for the device. *