[+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 <yinghai@xxxxxxxxxx> wrote:
> >>
> >> 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]
> 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 <peter@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx>
>
> ---
> 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.
> 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.
> }
> -
> - 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.
> *
--
To unsubscribe from this list: send the line "unsubscribe linux-next" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
