linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@sisk.pl>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: Mikko Vinni <mmvinni@yahoo.com>,
	"linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Allen Kay <allen.m.kay@intel.com>,
	Jesse Barnes <jbarnes@virtuousgeek.org>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: [PATCH] PCI: Fix regression in pci_restore_state()
Date: Sun, 15 Apr 2012 20:32:36 +0200	[thread overview]
Message-ID: <201204152032.36770.rjw@sisk.pl> (raw)
In-Reply-To: <1334486385.82898.YahooMailNeo@web161803.mail.bf1.yahoo.com>

From: Rafael J. Wysocki <rjw@sisk.pl>

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 register (whose value after the write quite
obviously need not be the same as the written one).  Second, it
added arbitrary delay to pci_restore_state() even for systems
where the PCI config space restoration was successful at first
attempt.  Finally, it used mdelay() for that, although there's
no rule stating that pci_restore_state() could not sleep (at least
I'm not aware of such a rule).

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 for BAR registers and only wait if the first read from
the register doesn't return the written value.  Additionally, make
it use msleep() instead of mdelay() for waiting.

Reported-and-tested-by: Mikko Vinni <mmvinni@yahoo.com>
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/pci/pci.c |   50 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 18 deletions(-)

Index: linux/drivers/pci/pci.c
===================================================================
--- linux.orig/drivers/pci/pci.c
+++ linux/drivers/pci/pci.c
@@ -967,16 +967,40 @@ pci_save_state(struct pci_dev *dev)
 	return 0;
 }
 
+static void pci_restore_config_space(struct pci_dev *pdev, int start, int end,
+				     int retry)
+{
+	int index;
+
+	for (index = end; index >= start; index--) {
+		u32 val, saved;
+		int offset = 4 * index;
+		int count = retry;
+
+		saved = pdev->saved_config_space[index];
+		pci_read_config_dword(pdev, offset, &val);
+		while (saved != val) {
+			dev_dbg(&pdev->dev, "restoring config space at offset "
+				"%#x (was %#x, writing %#x)\n",
+				offset, val, saved);
+			pci_write_config_dword(pdev, offset, saved);
+			if (count-- > 0) {
+				pci_read_config_dword(pdev, offset, &val);
+				if (saved != val)
+					msleep(10);
+			} else {
+				break;
+			}
+		}
+	}
+}
+
 /** 
  * 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 +1008,14 @@ void pci_restore_state(struct pci_dev *d
 	pci_restore_pcie_state(dev);
 	pci_restore_ats_state(dev);
 
+	pci_restore_config_space(dev, 10, 15, 0);
 	/*
 	 * 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--;
-		}
-	}
+	pci_restore_config_space(dev, 4, 9, 10);
+	pci_restore_config_space(dev, 0, 3, 0);
+
 	pci_restore_pcix_state(dev);
 	pci_restore_msi_state(dev);
 	pci_restore_iov_state(dev);

  parent reply	other threads:[~2012-04-15 18:28 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1334310754.17013.YahooMailNeo@web161804.mail.bf1.yahoo.com>
2012-04-13 19:18 ` [linux-pm] "i8042: Can't reactivate AUX port" after s2ram on 3.4-rc2 Rafael J. Wysocki
2012-04-13 19:49   ` Mikko Vinni
2012-04-13 20:19     ` Rafael J. Wysocki
2012-04-13 20:47       ` Mikko Vinni
2012-04-14 22:11         ` Rafael J. Wysocki
2012-04-15 10:39           ` Mikko Vinni
2012-04-15 18:30             ` Rafael J. Wysocki
2012-04-15 19:52               ` Rafael J. Wysocki
2012-04-15 20:56                 ` Rafael J. Wysocki
2012-04-16  7:23                   ` Mikko Vinni
2012-04-16 16:17                     ` Rafael J. Wysocki
2012-04-16 18:57                       ` Mikko Vinni
2012-04-16 19:59                         ` Rafael J. Wysocki
2012-04-16 20:35                         ` [PATCH] PCI: Retry BARs restoration for Type 0 headers only Rafael J. Wysocki
2012-04-16 20:35                           ` Linus Torvalds
2012-04-16 21:07                             ` Rafael J. Wysocki
2012-04-16  5:15               ` [linux-pm] "i8042: Can't reactivate AUX port" after s2ram on 3.4-rc2 Mikko Vinni
2012-04-16 16:14                 ` Rafael J. Wysocki
2012-04-15 18:32             ` Rafael J. Wysocki [this message]
2012-04-15 18:36               ` [PATCH] PCI: Fix regression in pci_restore_state() Linus Torvalds
2012-04-15 18:47                 ` Rafael J. Wysocki
2012-04-15 18:59                   ` Linus Torvalds
2012-04-15 19:40                     ` Rafael J. Wysocki
2012-04-23 17:03                       ` Bjorn Helgaas
2012-04-23 19:53                         ` Rafael J. Wysocki
2012-04-23 20:07                           ` Don Dutile
2012-04-23 22:33                             ` Bjorn Helgaas
2012-04-24 16:03                               ` Don Dutile
2012-04-24 17:01                                 ` Bjorn Helgaas
2012-04-24 17:35                                   ` Don Dutile
2012-04-27 22:20                                     ` Bjorn Helgaas
2012-04-15  8:12         ` [linux-pm] "i8042: Can't reactivate AUX port" after s2ram on 3.4-rc2 James Courtier-Dutton
2012-04-15 10:47           ` Mikko Vinni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201204152032.36770.rjw@sisk.pl \
    --to=rjw@sisk.pl \
    --cc=allen.m.kay@intel.com \
    --cc=bhelgaas@google.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=jbarnes@virtuousgeek.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mmvinni@yahoo.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).