linux-kernel - Re: [PATCH v4 06/11] PCI/ASPM: Clear aspm_disable as part of __pci_enable_link

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <qay63njqf7z7mchizt5sm66i67rvxxxicikxmfuvllmmxfy7ek@mulnjvde5q7w>
Date: Sat, 12 Jul 2025 15:05:28 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>, 
	Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>, Bjorn Helgaas <bhelgaas@...gle.com>, 
	Jingoo Han <jingoohan1@...il.com>, Lorenzo Pieralisi <lpieralisi@...nel.org>, 
	Rob Herring <robh@...nel.org>, Jeff Johnson <jjohnson@...nel.org>, 
	Bartosz Golaszewski <brgl@...ev.pl>, Krzysztof Wilczyński <kwilczynski@...nel.org>, 
	linux-pci@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>, 
	linux-arm-msm@...r.kernel.org, mhi@...ts.linux.dev, linux-wireless@...r.kernel.org, 
	ath11k@...ts.infradead.org, qiang.yu@....qualcomm.com, quic_vbadigan@...cinc.com, 
	quic_vpernami@...cinc.com, quic_mrana@...cinc.com, 
	Jeff Johnson <jeff.johnson@....qualcomm.com>
Subject: Re: [PATCH v4 06/11] PCI/ASPM: Clear aspm_disable as part of
 __pci_enable_link_state()

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

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