| Message ID | 20200924142443.260861-2-refactormyself@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI/ASPM: Move some ASPM info to struct pci_dev | expand |
On Thu, Sep 24, 2020 at 04:24:37PM +0200, Saheed O. Bolarinwa wrote: > pcie_get_aspm_reg() reads LNKCAP to learn whether the device supports > ASPM L0s and/or L1 and L1 substates. I'm working from this v2 series. But it's always nice if you include a cover letter (as you did for the series you posted yesterday) and if the cover letter includes a note about what changed from the previous versions. Bjorn
On Thu, Sep 24, 2020 at 04:24:37PM +0200, Saheed O. Bolarinwa wrote: > pcie_get_aspm_reg() reads LNKCAP to learn whether the device supports > ASPM L0s and/or L1 and L1 substates. > > If we cache the entire LNKCAP word early enough, we may be able to > use it in other places that read LNKCAP, e.g. pcie_get_speed_cap(), > pcie_get_width_cap(), pcie_init(), etc. > > - Add struct pci_dev.lnkcap (u32) > - Read PCI_EXP_LNKCAP in set_pcie_port_type() and save it > in pci_dev.lnkcap > - Use pdev->lnkcap instead of reading PCI_EXP_LNKCAP I think we might as well go ahead and use the cached copy in these other places in this patch, i.e., pcie_init pcie_get_speed_cap pcie_get_width_cap pcie_link_bandwidth_notification_supported
On Thu, Sep 24, 2020 at 04:24:37PM +0200, Saheed O. Bolarinwa wrote: > pcie_get_aspm_reg() reads LNKCAP to learn whether the device supports > ASPM L0s and/or L1 and L1 substates. > > If we cache the entire LNKCAP word early enough, we may be able to > use it in other places that read LNKCAP, e.g. pcie_get_speed_cap(), > pcie_get_width_cap(), pcie_init(), etc. > > - Add struct pci_dev.lnkcap (u32) > - Read PCI_EXP_LNKCAP in set_pcie_port_type() and save it > in pci_dev.lnkcap > - Use pdev->lnkcap instead of reading PCI_EXP_LNKCAP I think we need to be a little careful here because there's a note in the spec (PCIe r5.0, sec 7.5.3.6): Note that exit latencies may be influenced by PCI Express reference clock configuration depending upon whether a component uses a common or separate reference clock. So if we change the common clock configuration, e.g., in pcie_aspm_configure_common_clock() or anything else that writes PCI_EXP_LNKCTL_CCC, I think we will need to update pdev->lnkcap.
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index 253c30cc1967..d7e69b3595a0 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -177,15 +177,13 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable) static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist) { int capable = 1, enabled = 1; - u32 reg32; u16 reg16; struct pci_dev *child; struct pci_bus *linkbus = link->pdev->subordinate; /* All functions should have the same cap and state, take the worst */ list_for_each_entry(child, &linkbus->devices, bus_list) { - pcie_capability_read_dword(child, PCI_EXP_LNKCAP, ®32); - if (!(reg32 & PCI_EXP_LNKCAP_CLKPM)) { + if (!(child->lnkcap & PCI_EXP_LNKCAP_CLKPM)) { capable = 0; enabled = 0; break; @@ -397,9 +395,8 @@ static void pcie_get_aspm_reg(struct pci_dev *pdev, struct aspm_register_info *info) { u16 reg16; - u32 reg32; + u32 reg32 = pdev->lnkcap; - pcie_capability_read_dword(pdev, PCI_EXP_LNKCAP, ®32); info->support = (reg32 & PCI_EXP_LNKCAP_ASPMS) >> 10; info->latency_encoding_l0s = (reg32 & PCI_EXP_LNKCAP_L0SEL) >> 12; info->latency_encoding_l1 = (reg32 & PCI_EXP_LNKCAP_L1EL) >> 15; diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 03d37128a24f..2d5898f05f89 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -1486,6 +1486,7 @@ void set_pcie_port_type(struct pci_dev *pdev) pdev->pcie_flags_reg = reg16; pci_read_config_word(pdev, pos + PCI_EXP_DEVCAP, ®16); pdev->pcie_mpss = reg16 & PCI_EXP_DEVCAP_PAYLOAD; + pcie_capability_read_dword(pdev, PCI_EXP_LNKCAP, &pdev->lnkcap); parent = pci_upstream_bridge(pdev); if (!parent) diff --git a/include/linux/pci.h b/include/linux/pci.h index 835530605c0d..5b305cfeb1dc 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -375,6 +375,7 @@ struct pci_dev { bit manually */ unsigned int d3_delay; /* D3->D0 transition time in ms */ unsigned int d3cold_delay; /* D3cold->D0 transition time in ms */ + u32 lnkcap; /* Link Capabilities */ #ifdef CONFIG_PCIEASPM struct pcie_link_state *link_state; /* ASPM link state */
pcie_get_aspm_reg() reads LNKCAP to learn whether the device supports ASPM L0s and/or L1 and L1 substates. If we cache the entire LNKCAP word early enough, we may be able to use it in other places that read LNKCAP, e.g. pcie_get_speed_cap(), pcie_get_width_cap(), pcie_init(), etc. - Add struct pci_dev.lnkcap (u32) - Read PCI_EXP_LNKCAP in set_pcie_port_type() and save it in pci_dev.lnkcap - Use pdev->lnkcap instead of reading PCI_EXP_LNKCAP Signed-off-by: Saheed O. Bolarinwa <refactormyself@gmail.com> --- drivers/pci/pcie/aspm.c | 7 ++----- drivers/pci/probe.c | 1 + include/linux/pci.h | 1 + 3 files changed, 4 insertions(+), 5 deletions(-)