Skip to content

Commit ebfc5b8

Browse files
rjwysockitorvalds
authored andcommitted
PCI: Fix regression in pci_restore_state(), v3
Commit 26f4106 ("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, 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 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. Additionaly, make it wait for 1 ms, instead of 10 ms, after every failing attempt to write into config space. Reported-by: Mikko Vinni <[email protected]> Signed-off-by: Rafael J. Wysocki <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent 6c23b8e commit ebfc5b8

File tree

1 file changed

+39
-18
lines changed

1 file changed

+39
-18
lines changed

drivers/pci/pci.c

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -967,41 +967,62 @@ pci_save_state(struct pci_dev *dev)
967967
return 0;
968968
}
969969

970+
static void pci_restore_config_dword(struct pci_dev *pdev, int offset,
971+
u32 saved_val, int retry)
972+
{
973+
u32 val;
974+
975+
pci_read_config_dword(pdev, offset, &val);
976+
if (val == saved_val)
977+
return;
978+
979+
for (;;) {
980+
dev_dbg(&pdev->dev, "restoring config space at offset "
981+
"%#x (was %#x, writing %#x)\n", offset, val, saved_val);
982+
pci_write_config_dword(pdev, offset, saved_val);
983+
if (retry-- <= 0)
984+
return;
985+
986+
pci_read_config_dword(pdev, offset, &val);
987+
if (val == saved_val)
988+
return;
989+
990+
mdelay(1);
991+
}
992+
}
993+
994+
static void pci_restore_config_space(struct pci_dev *pdev, int start, int end,
995+
int retry)
996+
{
997+
int index;
998+
999+
for (index = end; index >= start; index--)
1000+
pci_restore_config_dword(pdev, 4 * index,
1001+
pdev->saved_config_space[index],
1002+
retry);
1003+
}
1004+
9701005
/**
9711006
* pci_restore_state - Restore the saved state of a PCI device
9721007
* @dev: - PCI device that we're dealing with
9731008
*/
9741009
void pci_restore_state(struct pci_dev *dev)
9751010
{
976-
int i;
977-
u32 val;
978-
int tries;
979-
9801011
if (!dev->state_saved)
9811012
return;
9821013

9831014
/* PCI Express register must be restored first */
9841015
pci_restore_pcie_state(dev);
9851016
pci_restore_ats_state(dev);
9861017

1018+
pci_restore_config_space(dev, 10, 15, 0);
9871019
/*
9881020
* The Base Address register should be programmed before the command
9891021
* register(s)
9901022
*/
991-
for (i = 15; i >= 0; i--) {
992-
pci_read_config_dword(dev, i * 4, &val);
993-
tries = 10;
994-
while (tries && val != dev->saved_config_space[i]) {
995-
dev_dbg(&dev->dev, "restoring config "
996-
"space at offset %#x (was %#x, writing %#x)\n",
997-
i, val, (int)dev->saved_config_space[i]);
998-
pci_write_config_dword(dev,i * 4,
999-
dev->saved_config_space[i]);
1000-
pci_read_config_dword(dev, i * 4, &val);
1001-
mdelay(10);
1002-
tries--;
1003-
}
1004-
}
1023+
pci_restore_config_space(dev, 4, 9, 10);
1024+
pci_restore_config_space(dev, 0, 3, 0);
1025+
10051026
pci_restore_pcix_state(dev);
10061027
pci_restore_msi_state(dev);
10071028
pci_restore_iov_state(dev);

0 commit comments

Comments
 (0)