On Mon, May 9, 2022 at 3:37 PM Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> wrote: > > pcie_aspm_pm_state_change() was introduced at the inception of PCIe > ASPM code. > > However, it can cause some issues. For instance, when ASPM config is > changed via sysfs, those changes won't persist across power state change > because pcie_aspm_pm_state_change() overwrites them. > > In addition to that, if the driver is to restore L1ss [1] after system > resume, the restored states will also be overwritten by > pcie_aspm_pm_state_change(). > > So remove pcie_aspm_pm_state_change() for now, if there's any hardware > really needs it to function, a quirk can be used instead. > > [1] https://lore.kernel.org/linux-pci/20220201123536.12962-1-vidyas@xxxxxxxxxx/ > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> A gentle ping... > --- > drivers/pci/pci.c | 3 --- > drivers/pci/pci.h | 2 -- > drivers/pci/pcie/aspm.c | 19 ------------------- > 3 files changed, 24 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 9ecce435fb3f1..d09f7b60ee4dc 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -1181,9 +1181,6 @@ static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state) > if (need_restore) > pci_restore_bars(dev); > > - if (dev->bus->self) > - pcie_aspm_pm_state_change(dev->bus->self); > - > return 0; > } > > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h > index 3d60cabde1a15..86a19f293d4ad 100644 > --- a/drivers/pci/pci.h > +++ b/drivers/pci/pci.h > @@ -560,12 +560,10 @@ bool pcie_wait_for_link(struct pci_dev *pdev, bool active); > #ifdef CONFIG_PCIEASPM > void pcie_aspm_init_link_state(struct pci_dev *pdev); > void pcie_aspm_exit_link_state(struct pci_dev *pdev); > -void pcie_aspm_pm_state_change(struct pci_dev *pdev); > void pcie_aspm_powersave_config_link(struct pci_dev *pdev); > #else > static inline void pcie_aspm_init_link_state(struct pci_dev *pdev) { } > static inline void pcie_aspm_exit_link_state(struct pci_dev *pdev) { } > -static inline void pcie_aspm_pm_state_change(struct pci_dev *pdev) { } > static inline void pcie_aspm_powersave_config_link(struct pci_dev *pdev) { } > #endif > > diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c > index a96b7424c9bc8..7f76a5875feb4 100644 > --- a/drivers/pci/pcie/aspm.c > +++ b/drivers/pci/pcie/aspm.c > @@ -1012,25 +1012,6 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev) > up_read(&pci_bus_sem); > } > > -/* @pdev: the root port or switch downstream port */ > -void pcie_aspm_pm_state_change(struct pci_dev *pdev) > -{ > - struct pcie_link_state *link = pdev->link_state; > - > - if (aspm_disabled || !link) > - return; > - /* > - * Devices changed PM state, we should recheck if latency > - * meets all functions' requirement > - */ > - down_read(&pci_bus_sem); > - mutex_lock(&aspm_lock); > - pcie_update_aspm_capable(link->root); > - pcie_config_aspm_path(link); > - mutex_unlock(&aspm_lock); > - up_read(&pci_bus_sem); > -} > - > void pcie_aspm_powersave_config_link(struct pci_dev *pdev) > { > struct pcie_link_state *link = pdev->link_state; > -- > 2.34.1 >
