Re: [linux-pm] "i8042: Can't reactivate AUX port" after s2ram on 3.4-rc2

[Mikko Vinni <mmvinni@yahoo.com>]

From: Rafael J. Wysocki

>>  OK, I know.  There simply are fewer BARs for bridges (and PCIe ports)
>>  and here we're attempting to overwrite the secondary status register.  
> Sigh.
>> 
>>  Well, I guess we should only retry the writes to BARs for Type 0 headers.
> 
> Which is done by the appended updated patch.  Can you please give it a try?


Ok, I tested v4 and also v3, which is in 3.4-rc3. On this laptop both versions
work equally: touchpad works after resume (no help for the card reader
problem, though).

Here is part of the resume log with version 4:

      1 ACPI: Waking up from system sleep state S3
      1 pcieport 0000:00:02.0: restoring config space at offset 0x1c (was 0x20005151, writing 0x5151)
      1 pcieport 0000:00:02.0: restoring config space at offset 0x4 (was 0x100007, writing 0x100407)
      1 pcieport 0000:00:04.0: restoring config space at offset 0x4 (was 0x100007, writing 0x100407)
      1 pcieport 0000:00:05.0: restoring config space at offset 0x4 (was 0x100007, writing 0x100407)
      1 pcieport 0000:00:06.0: restoring config space at offset 0x1c (was 0x20002121, writing 0x2121)
      1 pcieport 0000:00:06.0: restoring config space at offset 0x4 (was 0x100007, writing 0x100407)
      1 pcieport 0000:00:0a.0: restoring config space at offset 0x4 (was 0x100007, writing 0x100407)
      1 ahci 0000:00:11.0: restoring config space at offset 0x4 (was 0x2300003, writing 0x2300007)
      1 ohci_hcd 0000:00:12.0: restoring config space at offset 0x4 (was 0x2a00002, writing 0x2a00013)
      1 ehci_hcd 0000:00:12.2: BAR 0: set to [mem 0xd2408500-0xd24085ff] (PCI address [0xd2408500-0xd24085ff])
      1 ehci_hcd 0000:00:12.2: restoring config space at offset 0x4 (was 0x2b00000, writing 0x2b00013)
      1 ehci_hcd 0000:00:12.2: PME# disabled
      1 ohci_hcd 0000:00:13.0: restoring config space at offset 0x4 (was 0x2a00002, writing 0x2a00013)
      1 ehci_hcd 0000:00:13.2: BAR 0: set to [mem 0xd2408400-0xd24084ff] (PCI address [0xd2408400-0xd24084ff])
      1 ehci_hcd 0000:00:13.2: restoring config space at offset 0x4 (was 0x2b00000, writing 0x2b00013)
      1 ehci_hcd 0000:00:13.2: PME# disabled
      1 pata_atiixp 0000:00:14.1: restoring config space at offset 0x4 (was 0x2300011, writing 0x2300015)
      1 radeon 0000:01:00.0: restoring config space at offset 0x4 (was 0x100003, writing 0x100407)
      1 pci 0000:09:00.0: restoring config space at offset 0x4 (was 0x100007, writing 0x100003)
      1 firewire_ohci 0000:0a:00.0: Refused to change power state, currently in D3
      1 firewire_ohci 0000:0a:00.0: restoring config space at offset 0x3c (was 0xffffffff, writing 0x10a)
      1 firewire_ohci 0000:0a:00.0: restoring config space at offset 0x38 (was 0xffffffff, writing 0x0)
      1 firewire_ohci 0000:0a:00.0: restoring config space at offset 0x34 (was 0xffffffff, writing 0x44)
      1 firewire_ohci 0000:0a:00.0: restoring config space at offset 0x30 (was 0xffffffff, writing 0x0)
      1 firewire_ohci 0000:0a:00.0: restoring config space at offset 0x2c (was 0xffffffff, writing 0x3600103c)
      1 firewire_ohci 0000:0a:00.0: restoring config space at offset 0x28 (was 0xffffffff, writing 0x0)
     11 firewire_ohci 0000:0a:00.0: restoring config space at offset 0x24 (was 0xffffffff, writing 0xd1100c00)
     11 firewire_ohci 0000:0a:00.0: restoring config space at offset 0x20 (was 0xffffffff, writing 0xd1100c80)
     11 firewire_ohci 0000:0a:00.0: restoring config space at offset 0x1c (was 0xffffffff, writing 0x0)
     11 firewire_ohci 0000:0a:00.0: restoring config space at offset 0x18 (was 0xffffffff, writing 0x0)
     11 firewire_ohci 0000:0a:00.0: restoring config space at offset 0x14 (was 0xffffffff, writing 0xd1100d00)
     11 firewire_ohci 0000:0a:00.0: restoring config space at offset 0x10 (was 0xffffffff, writing 0xd1100000)
      1 firewire_ohci 0000:0a:00.0: restoring config space at offset 0xc (was 0xffffffff, writing 0x800000)
      1 firewire_ohci 0000:0a:00.0: restoring config space at offset 0x8 (was 0xffffffff, writing 0xc001000)
      1 firewire_ohci 0000:0a:00.0: restoring config space at offset 0x4 (was 0xffffffff, writing 0x100007)
      1 firewire_ohci 0000:0a:00.0: restoring config space at offset 0x0 (was 0xffffffff, writing 0x2380197b)
      1 sdhci-pci 0000:0a:00.1: Refused to change power state, currently in D3
      1 sdhci-pci 0000:0a:00.1: restoring config space at offset 0x3c (was 0xffffffff, writing 0x10a)
      1 sdhci-pci 0000:0a:00.1: restoring config space at offset 0x38 (was 0xffffffff, writing 0x0)
      1 sdhci-pci 0000:0a:00.1: restoring config space at offset 0x34 (was 0xffffffff, writing 0xa4)
      1 sdhci-pci 0000:0a:00.1: restoring config space at offset 0x30 (was 0xffffffff, writing 0x0)
      1 sdhci-pci 0000:0a:00.1: restoring config space at offset 0x2c (was 0xffffffff, writing 0x3600103c)
      1 sdhci-pci 0000:0a:00.1: restoring config space at offset 0x28 (was 0xffffffff, writing 0x0)
     11 sdhci-pci 0000:0a:00.1: restoring config space at offset 0x24 (was 0xffffffff, writing 0x0)
     11 sdhci-pci 0000:0a:00.1: restoring config space at offset 0x20 (was 0xffffffff, writing 0x0)
     11 sdhci-pci 0000:0a:00.1: restoring config space at offset 0x1c (was 0xffffffff, writing 0x0)
     11 sdhci-pci 0000:0a:00.1: restoring config space at offset 0x18 (was 0xffffffff, writing 0x0)
     11 sdhci-pci 0000:0a:00.1: restoring config space at offset 0x14 (was 0xffffffff, writing 0x0)
     11 sdhci-pci 0000:0a:00.1: restoring config space at offset 0x10 (was 0xffffffff, writing 0xd1100b00)
      1 sdhci-pci 0000:0a:00.1: restoring config space at offset 0xc (was 0xffffffff, writing 0x800000)
      1 sdhci-pci 0000:0a:00.1: restoring config space at offset 0x8 (was 0xffffffff, writing 0x8800000)
      1 sdhci-pci 0000:0a:00.1: restoring config space at offset 0x4 (was 0xffffffff, writing 0x100007)
      1 sdhci-pci 0000:0a:00.1: restoring config space at offset 0x0 (was 0xffffffff, writing 0x2382197b)
      1 pci 0000:0a:00.2: Refused to change power state, currently in D3
      1 pci 0000:0a:00.2: restoring config space at offset 0x3c (was 0xffffffff, writing 0x10a)
      1 pci 0000:0a:00.2: restoring config space at offset 0x38 (was 0xffffffff, writing 0x0)
      1 pci 0000:0a:00.2: restoring config space at offset 0x34 (was 0xffffffff, writing 0xa4)
      1 pci 0000:0a:00.2: restoring config space at offset 0x30 (was 0xffffffff, writing 0x0)
      1 pci 0000:0a:00.2: restoring config space at offset 0x2c (was 0xffffffff, writing 0x3600103c)
      1 pci 0000:0a:00.2: restoring config space at offset 0x28 (was 0xffffffff, writing 0x0)
     11 pci 0000:0a:00.2: restoring config space at offset 0x24 (was 0xffffffff, writing 0x0)
     11 pci 0000:0a:00.2: restoring config space at offset 0x20 (was 0xffffffff, writing 0x0)
     11 pci 0000:0a:00.2: restoring config space at offset 0x1c (was 0xffffffff, writing 0x0)
     11 pci 0000:0a:00.2: restoring config space at offset 0x18 (was 0xffffffff, writing 0x0)
     11 pci 0000:0a:00.2: restoring config space at offset 0x14 (was 0xffffffff, writing 0x0)
     11 pci 0000:0a:00.2: restoring config space at offset 0x10 (was 0xffffffff, writing 0xd1100a00)
      1 pci 0000:0a:00.2: restoring config space at offset 0xc (was 0xffffffff, writing 0x800000)
      1 pci 0000:0a:00.2: restoring config space at offset 0x8 (was 0xffffffff, writing 0x8050100)
      1 pci 0000:0a:00.2: restoring config space at offset 0x4 (was 0xffffffff, writing 0x100003)
      1 pci 0000:0a:00.2: restoring config space at offset 0x0 (was 0xffffffff, writing 0x2381197b)
      1 jmb38x_ms 0000:0a:00.3: Refused to change power state, currently in D3
      1 jmb38x_ms 0000:0a:00.3: restoring config space at offset 0x3c (was 0xffffffff, writing 0x10a)
      1 jmb38x_ms 0000:0a:00.3: restoring config space at offset 0x38 (was 0xffffffff, writing 0x0)
      1 jmb38x_ms 0000:0a:00.3: restoring config space at offset 0x34 (was 0xffffffff, writing 0xa4)
      1 jmb38x_ms 0000:0a:00.3: restoring config space at offset 0x30 (was 0xffffffff, writing 0x0)
      1 jmb38x_ms 0000:0a:00.3: restoring config space at offset 0x2c (was 0xffffffff, writing 0x3600103c)
      1 jmb38x_ms 0000:0a:00.3: restoring config space at offset 0x28 (was 0xffffffff, writing 0x0)
     11 jmb38x_ms 0000:0a:00.3: restoring config space at offset 0x24 (was 0xffffffff, writing 0x0)
     11 jmb38x_ms 0000:0a:00.3: restoring config space at offset 0x20 (was 0xffffffff, writing 0x0)
     11 jmb38x_ms 0000:0a:00.3: restoring config space at offset 0x1c (was 0xffffffff, writing 0x0)
     11 jmb38x_ms 0000:0a:00.3: restoring config space at offset 0x18 (was 0xffffffff, writing 0x0)
     11 jmb38x_ms 0000:0a:00.3: restoring config space at offset 0x14 (was 0xffffffff, writing 0x0)
     11 jmb38x_ms 0000:0a:00.3: restoring config space at offset 0x10 (was 0xffffffff, writing 0xd1100900)
      1 jmb38x_ms 0000:0a:00.3: restoring config space at offset 0xc (was 0xffffffff, writing 0x800000)
      1 jmb38x_ms 0000:0a:00.3: restoring config space at offset 0x8 (was 0xffffffff, writing 0x8800000)
      1 jmb38x_ms 0000:0a:00.3: restoring config space at offset 0x4 (was 0xffffffff, writing 0x100007)
      1 jmb38x_ms 0000:0a:00.3: restoring config space at offset 0x0 (was 0xffffffff, writing 0x2383197b)
      1 pci 0000:0a:00.4: Refused to change power state, currently in D3
      1 pci 0000:0a:00.4: restoring config space at offset 0x3c (was 0xffffffff, writing 0x10a)
      1 pci 0000:0a:00.4: restoring config space at offset 0x38 (was 0xffffffff, writing 0x0)
      1 pci 0000:0a:00.4: restoring config space at offset 0x34 (was 0xffffffff, writing 0xa4)
      1 pci 0000:0a:00.4: restoring config space at offset 0x30 (was 0xffffffff, writing 0x0)
      1 pci 0000:0a:00.4: restoring config space at offset 0x2c (was 0xffffffff, writing 0x3600103c)
      1 pci 0000:0a:00.4: restoring config space at offset 0x28 (was 0xffffffff, writing 0x0)
     11 pci 0000:0a:00.4: restoring config space at offset 0x24 (was 0xffffffff, writing 0x0)
     11 pci 0000:0a:00.4: restoring config space at offset 0x20 (was 0xffffffff, writing 0x0)
     11 pci 0000:0a:00.4: restoring config space at offset 0x1c (was 0xffffffff, writing 0x0)
     11 pci 0000:0a:00.4: restoring config space at offset 0x18 (was 0xffffffff, writing 0x0)
     11 pci 0000:0a:00.4: restoring config space at offset 0x14 (was 0xffffffff, writing 0x0)
     11 pci 0000:0a:00.4: restoring config space at offset 0x10 (was 0xffffffff, writing 0xd1100800)
      1 pci 0000:0a:00.4: restoring config space at offset 0xc (was 0xffffffff, writing 0x800000)
      1 pci 0000:0a:00.4: restoring config space at offset 0x8 (was 0xffffffff, writing 0x8800000)
      1 pci 0000:0a:00.4: restoring config space at offset 0x4 (was 0xffffffff, writing 0x100007)
      1 pci 0000:0a:00.4: restoring config space at offset 0x0 (was 0xffffffff, writing 0x2384197b)
      1 PM: noirq resume of devices complete after 440.260 msecs
      1 PM: early resume of devices complete after 0.152 msecs
      1 ohci_hcd 0000:00:12.0: enabling bus mastering
      1 ohci_hcd 0000:00:12.1: enabling bus mastering


Difference from v3:
--- /tmp/v3-uniq        2012-04-16 09:57:42.880521878 +0300
+++ /tmp/v4-uniq        2012-04-16 09:58:03.880545386 +0300
@@ -1019,11 +1018,11 @@
       1 Booting Node 0 Processor 1 APIC 0x1
       1 CPU1 is up
       1 ACPI: Waking up from system sleep state S3
-     11 pcieport 0000:00:02.0: restoring config space at offset 0x1c (was 0x20005151, writing 0x5151)
+      1 pcieport 0000:00:02.0: restoring config space at offset 0x1c (was 0x20005151, writing 0x5151)
       1 pcieport 0000:00:02.0: restoring config space at offset 0x4 (was 0x100007, writing 0x100407)
       1 pcieport 0000:00:04.0: restoring config space at offset 0x4 (was 0x100007, writing 0x100407)
       1 pcieport 0000:00:05.0: restoring config space at offset 0x4 (was 0x100007, writing 0x100407)
-     11 pcieport 0000:00:06.0: restoring config space at offset 0x1c (was 0x20002121, writing 0x2121)
+      1 pcieport 0000:00:06.0: restoring config space at offset 0x1c (was 0x20002121, writing 0x2121)
       1 pcieport 0000:00:06.0: restoring config space at offset 0x4 (was 0x100007, writing 0x100407)
       1 pcieport 0000:00:0a.0: restoring config space at offset 0x4 (was 0x100007, writing 0x100407)
       1 ahci 0000:00:11.0: restoring config space at offset 0x4 (was 0x2300003, writing 0x2300007)
@@ -1123,8 +1122,8 @@
       1 pci 0000:0a:00.4: restoring config space at offset 0x8 (was 0xffffffff, writing 0x8800000)
       1 pci 0000:0a:00.4: restoring config space at offset 0x4 (was 0xffffffff, writing 0x100007)
       1 pci 0000:0a:00.4: restoring config space at offset 0x0 (was 0xffffffff, writing 0x2384197b)
-      1 PM: noirq resume of devices complete after 460.315 msecs
-      1 PM: early resume of devices complete after 0.168 msecs
+      1 PM: noirq resume of devices complete after 440.260 msecs
+      1 PM: early resume of devices complete after 0.152 msecs
       1 ohci_hcd 0000:00:12.0: enabling bus mastering
       1 ohci_hcd 0000:00:12.1: enabling bus mastering
       1 ehci_hcd 0000:00:12.2: enabling bus mastering


(...if only radeon didn't add 10 seconds to resume time, that improvement of 20 msecs
would be very nice; that is another issue, of course, and not a new problem for this
kernel version:

[drm:atom_op_jump] *ERROR* atombios stuck in loop for more than 5secs aborting
[drm:atom_execute_table_locked] *ERROR* atombios stuck executing F8B6 (len 485, WS 0, PS 4) @ 0xF8F7
[drm:atom_op_jump] *ERROR* atombios stuck in loop for more than 5secs aborting
[drm:atom_execute_table_locked] *ERROR* atombios stuck executing F8B6 (len 485, WS 0, PS 4) @ 0xF8F7
PM: resume of devices complete after 12741.575 msecs
)


Thanks,

Mikko


> 
> Rafael
> 
> ---
> From: Rafael J. Wysocki <rjw@sisk.pl>
> Subject: PCI: Fix regression in pci_restore_state(), v4
> 
> Commit 26f41062f28de65e11d3cf353e52d0be73442be1
> 
>     PCI: check for pci bar restore completion and retry
> 
> attempted to address problems with PCI BAR restoration on systems
> where FLR had not been completed before pci_restore_state() was
> called, but it did that in an utterly wrong way.
> 
> First off, instead of retrying the writes for the BAR registers
> only, it did that for all of the PCI config space of the device,
> including the status registers (whose values after the write quite
> obviously need not be the same as the written ones).  Second, it
> added arbitrary delay to pci_restore_state() even for systems
> where the PCI config space restoration was successful at first
> attempt.  Finally, the mdelay(10) it added to every iteration of the
> writing loop was way too much of a delay for any reasonable device.
> 
> All of this actually caused resume failures for some devices on
> the Mikko's system.
> 
> To fix the regression, make pci_restore_state() only retry the
> writes to BAR registers and only for devices with Type 0 headers.
> Moreover, make it wait only if the next first read from the given
> register doesn't return the value written to it.  Finally, make it
> wait for 1 ms, instead of 10 ms, after every failing attempt to write
> into the device's config space.
> 
> Reported-by: Mikko Vinni <mmvinni@yahoo.com>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
> drivers/pci/pci.c |   67 +++++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 46 insertions(+), 21 deletions(-)
> 
> Index: linux/drivers/pci/pci.c
> ===================================================================
> --- linux.orig/drivers/pci/pci.c
> +++ linux/drivers/pci/pci.c
> @@ -967,16 +967,47 @@ pci_save_state(struct pci_dev *dev)
>     return 0;
> }
> 
> +static void pci_restore_config_dword(struct pci_dev *pdev, int offset,
> +                     u32 saved_val, int retry)
> +{
> +    u32 val;
> +
> +    pci_read_config_dword(pdev, offset, &val);
> +    if (val == saved_val)
> +        return;
> +
> +    for (;;) {
> +        dev_dbg(&pdev->dev, "restoring config space at offset 
> "
> +            "%#x (was %#x, writing %#x)\n", offset, val, 
> saved_val);
> +        pci_write_config_dword(pdev, offset, saved_val);
> +        if (retry-- <= 0)
> +            return;
> +
> +        pci_read_config_dword(pdev, offset, &val);
> +        if (val == saved_val)
> +            return;
> +
> +        mdelay(1);
> +    }
> +}
> +
> +static void pci_restore_config_space(struct pci_dev *pdev, int start, int end,
> +                     int retry)
> +{
> +    int index;
> +
> +    for (index = end; index >= start; index--)
> +        pci_restore_config_dword(pdev, 4 * index,
> +                     pdev->saved_config_space[index],
> +                     retry);
> +}
> +
> /** 
>   * pci_restore_state - Restore the saved state of a PCI device
>   * @dev: - PCI device that we're dealing with
>   */
> void pci_restore_state(struct pci_dev *dev)
> {
> -    int i;
> -    u32 val;
> -    int tries;
> -
>     if (!dev->state_saved)
>         return;
> 
> @@ -984,24 +1015,18 @@ void pci_restore_state(struct pci_dev *d
>     pci_restore_pcie_state(dev);
>     pci_restore_ats_state(dev);
> 
> -    /*
> -     * The Base Address register should be programmed before the command
> -     * register(s)
> -     */
> -    for (i = 15; i >= 0; i--) {
> -        pci_read_config_dword(dev, i * 4, &val);
> -        tries = 10;        
> -        while (tries && val != dev->saved_config_space[i]) {
> -            dev_dbg(&dev->dev, "restoring config "
> -                "space at offset %#x (was %#x, writing %#x)\n",
> -                i, val, (int)dev->saved_config_space[i]);
> -            pci_write_config_dword(dev,i * 4,
> -                dev->saved_config_space[i]);
> -            pci_read_config_dword(dev, i * 4, &val);
> -            mdelay(10);
> -            tries--;
> -        }
> +    if (dev->hdr_type == PCI_HEADER_TYPE_NORMAL) {
> +        pci_restore_config_space(dev, 10, 15, 0);
> +        /*
> +         * The Base Address register should be programmed before the
> +         * command register(s)
> +         */
> +        pci_restore_config_space(dev, 4, 9, 10);
> +        pci_restore_config_space(dev, 0, 3, 0);
> +    } else {
> +        pci_restore_config_space(dev, 0, 15, 0);
>     }
> +
>     pci_restore_pcix_state(dev);
>     pci_restore_msi_state(dev);
>     pci_restore_iov_state(dev);
>