Re: Please revert 928bea964827d7824b548c1f8e06eccbbc4d0d7d

[Yinghai Lu]

[+ Rafael]

On Fri, Sep 27, 2013 at 4:19 PM, Benjamin Herrenschmidt
<benh@xxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, 2013-09-27 at 15:56 -0700, Yinghai Lu wrote:
>
>> ok, please if you are ok attached one instead. It will print some warning about
>> driver skipping pci_set_master, so we can catch more problem with drivers.
>
> Except that the message is pretty cryptic :-) Especially since the
> driver causing the message to be printed is not the one that did
> the mistake in the first place, it's the next one coming up that
> trips the warning.
>
> In any case, the root cause is indeed the PCIe port driver:
>
> We don't have ACPI, so pcie_port_platform_notify() isn't implemented,
> and pcie_ports_auto is true, so we end up with capabilities set to 0.

in
| commit fe31e69740eddc7316071ed5165fed6703c8cd12
| Author: Rafael J. Wysocki <rjw@xxxxxxx>
| Date:   Sun Dec 19 15:57:16 2010 +0100
|
|    PCI/PCIe: Clear Root PME Status bits early during system resume
|
|    I noticed that PCI Express PMEs don't work on my Toshiba Portege R500
|    after the system has been woken up from a sleep state by a PME
|    (through Wake-on-LAN).  After some investigation it turned out that
|    the BIOS didn't clear the Root PME Status bit in the root port that
|    received the wakeup PME and since the Requester ID was also set in
|    the port's Root Status register, any subsequent PMEs didn't trigger
|    interrupts.
|
|    This problem can be avoided by clearing the Root PME Status bits in
|    all PCI Express root ports during early resume.  For this purpose,
|    add an early resume routine to the PCIe port driver and make this
|    driver be always registered, even if pci_ports_disable is set (in
|    which case the driver's only function is to provide the early
|    resume callback).
|
|
|@@ -349,15 +349,18 @@ int pcie_port_device_register(struct pci_dev *dev)
|        int status, capabilities, i, nr_service;
|        int irqs[PCIE_PORT_DEVICE_MAXSERVICES];
|
|-       /* Get and check PCI Express port services */
|-       capabilities = get_port_device_capability(dev);
|-       if (!capabilities)
|-               return -ENODEV;
|-
|        /* Enable PCI Express port device */
|        status = pci_enable_device(dev);
|        if (status)
|                return status;
|+
|+       /* Get and check PCI Express port services */
|+       capabilities = get_port_device_capability(dev);
|+       if (!capabilities) {
|+               pcie_no_aspm();
|+               return 0;
|+       }
|+
|        pci_set_master(dev);
|        /*
|         * Initialize service irqs. Don't use service devices that

>
> Thus the port driver bails out before calling pci_set_master(). The fix
> is to call pci_set_master() unconditionally. However that lead me to
> find to a few interesting oddities in that port driver code:

can we revert that partially change ? aka we should check get_port....
at first...

like attached.

Thanks

Yinghai

Attachment: fix_pci_set_master_port_pcie.patch
Description: Binary data