[PATCH v4 06/11] PCI/ASPM: Clear aspm_disable as part of __pci_enable_link_state()

Manivannan Sadhasivam mani at kernel.org
Sat Jul 12 02:35:28 PDT 2025


On Fri, Jul 11, 2025 at 06:00:13PM GMT, Bjorn Helgaas wrote:
> On Fri, Jul 11, 2025 at 04:38:48PM +0300, Ilpo Järvinen wrote:
> 
> > +++ b/include/linux/pci.h
> > @@ -1826,8 +1826,8 @@ static inline int pcie_set_target_speed(struct pci_dev *port,
> >  #ifdef CONFIG_PCIEASPM
> >  int pci_disable_link_state(struct pci_dev *pdev, int state);
> >  int pci_disable_link_state_locked(struct pci_dev *pdev, int state);
> > -int pci_enable_link_state(struct pci_dev *pdev, int state);
> 
> AFAICT there's no caller of this at all.  Why do we keep it?
> 

I'm just working on a series to convert the ath{10/11/12}k drivers to use this
API instead of modifying LNKCTL register directly:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/wireless/ath/ath12k/pci.c#n961

> > -int pci_enable_link_state_locked(struct pci_dev *pdev, int state);
> 
> We only have two callers of this (pcie-qcom.c and vmd.c, both in
> drivers/pci/), so it's not clear to me that it needs to be in
> include/linux/pci.h.
> 
> I'm a little dubious about it in the first place since I don't think
> drivers should be enabling ASPM states on their own, but pcie-qcom.c
> and vmd.c are PCIe controller drivers, not PCI device drivers, so I
> guess we can live with them for now.
> 
> IMO the "someday" goal should be that we get rid of aspm_policy and
> enable all the available power saving states by default.  We have
> sysfs knobs that administrators can use if necessary, and drivers or
> quirks can disable states if they need to work around hardware
> defects.
> 

Yeah, I think the default should be powersave and let the users disable it for
performance if they want.

- Mani

-- 
மணிவண்ணன் சதாசிவம்



More information about the ath11k mailing list