On Wednesday 04 November 2009 12:03:21 pm Jesse Barnes wrote:
> [Cc'ing linux-pci@xxxxxxxxxxxxxxx too]
>
> On Tue, 3 Nov 2009 16:38:20 -0500
> RDH <rdh@xxxxxxxxxxxx> wrote:
>
> >
> > This patch avoids unnecessary PCIe link retraining sequences within
> > the PCIe ASPM common clock setup code by issuing a link retrain
> > command only if we are actually changing the PCIe clock configuration.
> >
> > Signed-off-by: Robert D. Houk <rdh@xxxxxxxxxxxx>
> > ---
> > aspm.c | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> > --- a/drivers/pci/pcie/aspm.c 2009-10-15 20:41:50.000000000
> > -0400 +++ b/drivers/pci/pcie/aspm.c 2009-11-02
> > 14:29:35.000000000 -0500 @@ -183,6 +183,7 @@ static void
> > pcie_aspm_configure_common_c {
> > int ppos, cpos, same_clock = 1;
> > u16 reg16, parent_reg, child_reg[8];
> > + u16 lnkctl_ccc_or, lnkctl_ccc_and;
> > unsigned long start_jiffies;
> > struct pci_dev *child, *parent = link->pdev;
> > struct pci_bus *linkbus = parent->subordinate;
> > @@ -205,6 +206,25 @@ static void pcie_aspm_configure_common_c
> > if (!(reg16 & PCI_EXP_LNKSTA_SLC))
> > same_clock = 0;
> >
> > + /* Check upstream component for Common Clock Config */
> > + pci_read_config_word(parent, ppos + PCI_EXP_LNKCTL, ®16);
> > + lnkctl_ccc_and = lnkctl_ccc_or = (reg16 &
> > PCI_EXP_LNKCTL_CCC); +
> > + /* Scan downstream component for CCC, all functions */
> > + list_for_each_entry(child, &linkbus->devices, bus_list) {
> > + cpos = pci_find_capability(child, PCI_CAP_ID_EXP);
Some other places in pcie/aspm (e.g., pcie_set_clkpm_nocheck() and
pcie_clkpm_cap_init()) check cpos for NULL. Your code looks
superficially similar, so maybe you should check it, too, although
I do see many other place in aspm where we *don't* check it.
We look up this capability so often, I wonder if we should save it
in the struct pci_dev or struct pcie_link or something in such a
way that if we have a struct pcie_link, we are guaranteed that there
is a PCIe capability, and we don't have to search for it again.
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
