| Submitter | Thomas Renninger |
|---|---|
| Date | Dec. 8, 2008, 3:17 p.m. |
| Message ID | <200812081617.52047.trenn@suse.de> |
| Download | mbox | patch |
| Permalink | /patch/12784/ |
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
Comments
On Mon, 2008-12-08 at 23:17 +0800, Thomas Renninger wrote: > On Monday 08 December 2008 16:09:19 Matthew Garrett wrote: > > On Mon, Dec 08, 2008 at 04:04:09PM +0100, Thomas Renninger wrote: > > > - pci_read_config_word(pdev, pos + PCI_EXP_LNKCTL, ®16); > > > + parent_reg = pci_read_config_word(pdev, pos + PCI_EXP_LNKCTL, ®16); > > > > I don't think that does what you think it does :) > > Hehe, thanks for the quick and detailed review! > > This one should be better: > > PCIe: ASPM: Break out of endless loop waiting for PCI config bits to switch > > Makes a Compaq 6735s boot reliably again which hang in the loop > on some boots. > > Signed-off-by: Thomas Renninger <trenn@suse.de> > > --- > drivers/pci/pcie/aspm.c | 28 +++++++++++++++++++++++++--- > 1 file changed, 25 insertions(+), 3 deletions(-) > > Index: linux-2.6.27/drivers/pci/pcie/aspm.c > =================================================================== > --- linux-2.6.27.orig/drivers/pci/pcie/aspm.c > +++ linux-2.6.27/drivers/pci/pcie/aspm.c > @@ -16,6 +16,7 @@ > #include <linux/pm.h> > #include <linux/init.h> > #include <linux/slab.h> > +#include <linux/jiffies.h> > #include <linux/pci-aspm.h> > #include "../pci.h" > > @@ -161,11 +162,12 @@ static void pcie_check_clock_pm(struct p > */ > static void pcie_aspm_configure_common_clock(struct pci_dev *pdev) > { > - int pos, child_pos; > + int pos, child_pos, i = 0; > u16 reg16 = 0; > struct pci_dev *child_dev; > int same_clock = 1; > - > + unsigned long start_jiffies = jiffies; > + u16 child_regs[256], parent_reg; child_regs[8] should be enough. There should be just one pcie slot under the port. > /* > * all functions of a slot should have the same Slot Clock > * Configuration, so just check one function > @@ -191,16 +193,19 @@ static void pcie_aspm_configure_common_c > child_pos = pci_find_capability(child_dev, PCI_CAP_ID_EXP); > pci_read_config_word(child_dev, child_pos + PCI_EXP_LNKCTL, > ®16); > + child_regs[i] = reg16; > if (same_clock) > reg16 |= PCI_EXP_LNKCTL_CCC; > else > reg16 &= ~PCI_EXP_LNKCTL_CCC; > pci_write_config_word(child_dev, child_pos + PCI_EXP_LNKCTL, > reg16); > + i++; > } > > /* Configure upstream component */ > pci_read_config_word(pdev, pos + PCI_EXP_LNKCTL, ®16); > + parent_reg = reg16; > if (same_clock) > reg16 |= PCI_EXP_LNKCTL_CCC; > else > @@ -212,12 +217,29 @@ static void pcie_aspm_configure_common_c > pci_write_config_word(pdev, pos + PCI_EXP_LNKCTL, reg16); > > /* Wait for link training end */ > - while (1) { > + /* break out after waiting for 1 second */ should we set start_jiffies here? Otherwise, it's ok to me. Thanks, Shaohua -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Patch
Index: linux-2.6.27/drivers/pci/pcie/aspm.c =================================================================== --- linux-2.6.27.orig/drivers/pci/pcie/aspm.c +++ linux-2.6.27/drivers/pci/pcie/aspm.c @@ -16,6 +16,7 @@ #include <linux/pm.h> #include <linux/init.h> #include <linux/slab.h> +#include <linux/jiffies.h> #include <linux/pci-aspm.h> #include "../pci.h" @@ -161,11 +162,12 @@ static void pcie_check_clock_pm(struct p */ static void pcie_aspm_configure_common_clock(struct pci_dev *pdev) { - int pos, child_pos; + int pos, child_pos, i = 0; u16 reg16 = 0; struct pci_dev *child_dev; int same_clock = 1; - + unsigned long start_jiffies = jiffies; + u16 child_regs[256], parent_reg; /* * all functions of a slot should have the same Slot Clock * Configuration, so just check one function @@ -191,16 +193,19 @@ static void pcie_aspm_configure_common_c child_pos = pci_find_capability(child_dev, PCI_CAP_ID_EXP); pci_read_config_word(child_dev, child_pos + PCI_EXP_LNKCTL, ®16); + child_regs[i] = reg16; if (same_clock) reg16 |= PCI_EXP_LNKCTL_CCC; else reg16 &= ~PCI_EXP_LNKCTL_CCC; pci_write_config_word(child_dev, child_pos + PCI_EXP_LNKCTL, reg16); + i++; } /* Configure upstream component */ pci_read_config_word(pdev, pos + PCI_EXP_LNKCTL, ®16); + parent_reg = reg16; if (same_clock) reg16 |= PCI_EXP_LNKCTL_CCC; else @@ -212,12 +217,29 @@ static void pcie_aspm_configure_common_c pci_write_config_word(pdev, pos + PCI_EXP_LNKCTL, reg16); /* Wait for link training end */ - while (1) { + /* break out after waiting for 1 second */ + while ((jiffies - start_jiffies) < HZ) { pci_read_config_word(pdev, pos + PCI_EXP_LNKSTA, ®16); if (!(reg16 & PCI_EXP_LNKSTA_LT)) break; cpu_relax(); } + /* training failed -> recover */ + if ((jiffies - start_jiffies) >= HZ) { + dev_printk (KERN_ERR, &pdev->dev, "ASPM: Could not configure" + " common clock\n"); + i = 0; + list_for_each_entry(child_dev, &pdev->subordinate->devices, + bus_list) { + child_pos = pci_find_capability(child_dev, + PCI_CAP_ID_EXP); + pci_write_config_word(child_dev, + child_pos + PCI_EXP_LNKCTL, + child_regs[i]); + i++; + } + pci_write_config_word(pdev, pos + PCI_EXP_LNKCTL, parent_reg); + } } /*