From: Manivannan Sadhasivam <mani@kernel.org>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: "Bjorn Helgaas" <bhelgaas@google.com>,
"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
"Rob Herring" <robh@kernel.org>,
"Nirmal Patel" <nirmal.patel@linux.intel.com>,
"Jonathan Derrick" <jonathan.derrick@linux.dev>,
"Jeff Johnson" <jjohnson@kernel.org>,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-msm@vger.kernel.org, linux-wireless@vger.kernel.org,
ath12k@lists.infradead.org, ath11k@lists.infradead.org,
ath10k@lists.infradead.org,
"Krishna Chaitanya Chundru" <krishna.chundru@oss.qualcomm.com>,
"Qiang Yu" <qiang.yu@oss.qualcomm.com>,
"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Subject: Re: [PATCH v3 1/8] PCI/ASPM: Always disable ASPM when driver requests it
Date: Fri, 16 Oct 2026 11:08:29 +0530 [thread overview]
Message-ID: <atG4Gaq7GLiq2H7t@debian> (raw)
In-Reply-To: <20261015230714.GA1802070@bhelgaas>
On Thu, Oct 15, 2026 at 06:07:14PM -0500, Bjorn Helgaas wrote:
> On Wed, Jul 08, 2026 at 04:30:15PM +0200, Manivannan Sadhasivam wrote:
> > From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> >
> > PCI core/ASPM service driver allows controlling ASPM state through
> > pci_disable_link_state() API. It was decided earlier (see the Link
> > below), to not allow ASPM changes when OS does not have control over it
> > but only log a warning about the problem 'commit 2add0ec14c25 ("PCI/ASPM:
> > Warn when driver asks to disable ASPM, but we can't do it")'.
> >
> > A number of drivers have added workarounds to force ASPM off with own
> > writes into the Link Control Register (some even with comments
> > explaining why PCI core does not disable it under some circumstances).
> > According to the comments, some drivers require ASPM to be off for
> > reliable operation.
> >
> > Having custom ASPM handling in drivers is problematic because the state
> > kept in the ASPM service driver is not updated by the changes made
> > outside the link state management API.
> >
> > As the first step to address this issue, make pci_disable_link_state()
> > to unconditionally disable ASPM so the motivation for drivers to come
> > up with custom ASPM handling code is eliminated.
> >
> > To fully take advantage of the ASPM handling core provides, the drivers
> > that need to quirk ASPM have to be altered depend on PCIEASPM and the
> > custom ASPM code is removed. This is to be done separately. As PCIEASPM
> > is already behind EXPERT, it should be no problem to limit disabling it
> > for configurations that do not require touching ASPM.
> >
> > Make pci_disable_link_state() function comment to comply kerneldoc
> > formatting while changing the description.
> >
> > Link: https://lore.kernel.org/all/CANUX_P3F5YhbZX3WGU-j1AGpbXb_T9Bis2ErhvKkFMtDvzatVQ@mail.gmail.com/
> > Link: https://lore.kernel.org/all/20230511131441.45704-1-ilpo.jarvinen@linux.intel.com/
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > [mani: commit message fixup]
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>
> I was a little confused about the provenance of this patch. It
> *looks* like it should have been posted somewhere by Ilpo and
> something tweaked by Mani when applying, but none of the links above
> are a posting of this patch. I dug around and found
> https://lore.kernel.org/linux-pci/20230918131103.24119-4-ilpo.jarvinen@linux.intel.com/,
> and this patch looks like basically the aspm.c part of it, so I assume
> that should probably be the https://patch.msgid.link/?
>
Ilpo shared this patch during the review of v1 of this series:
https://lore.kernel.org/linux-pci/52baf40b-41ed-2588-7817-4d8cd859e0d1@linux.intel.com/
> To fix the provenance and the "Drivers can controlling" typo mentioned
> by Ilpo, I replaced this with just the aspm.c changes from
> https://lore.kernel.org/linux-pci/20230918131103.24119-4-ilpo.jarvinen@linux.intel.com/
> and added the CONFIG_PCIEASPM dependency to the commit log. The code
> change is the same.
>
This looks good to me.
- Mani
--
மணிவண்ணன் சதாசிவம்
next prev parent reply other threads:[~2026-10-16 5:38 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-08 14:30 [PATCH v3 0/8] PCI/ASPM: Add pci_force_enable_link_state() API and convert Atheros drivers Manivannan Sadhasivam
2026-07-08 14:30 ` [PATCH v3 1/8] PCI/ASPM: Always disable ASPM when driver requests it Manivannan Sadhasivam
2026-10-15 23:07 ` Bjorn Helgaas
2026-10-16 5:38 ` Manivannan Sadhasivam [this message]
2026-07-08 14:30 ` [PATCH v3 2/8] PCI/ASPM: Add pci_force_enable_link_state() API Manivannan Sadhasivam
2026-10-12 22:58 ` Bjorn Helgaas
2026-10-13 11:41 ` Ilpo Järvinen
2026-10-13 16:31 ` Bjorn Helgaas
2026-10-13 17:42 ` Ilpo Järvinen
2026-10-16 3:03 ` Val Packett
2026-07-08 14:30 ` [PATCH v3 3/8] PCI/ASPM: Transition the device to D0 (if required) when enabling ASPM link states Manivannan Sadhasivam
2026-10-02 22:55 ` Bjorn Helgaas
2026-07-08 14:30 ` [PATCH v3 4/8] PCI/ASPM: Improve the kernel-doc for pci_{enable,disable}_link_state*() APIs Manivannan Sadhasivam
2026-07-08 14:30 ` [PATCH v3 5/8] PCI/ASPM: Return enabled ASPM states from pcie_aspm_enabled() API Manivannan Sadhasivam
2026-07-08 14:30 ` [PATCH v3 6/8] wifi: ath12k: Use pci_{enable/disable}_link_state() APIs to enable/disable ASPM states Manivannan Sadhasivam
2026-07-10 20:25 ` Bjorn Helgaas
2026-07-31 5:34 ` Manivannan Sadhasivam
2026-08-03 17:08 ` Ilpo Järvinen
2026-07-08 14:30 ` [PATCH v3 7/8] wifi: ath11k: " Manivannan Sadhasivam
2026-07-08 14:30 ` [PATCH v3 8/8] wifi: ath10k: " Manivannan Sadhasivam
2026-10-14 20:51 ` [PATCH v3 0/8] PCI/ASPM: Add pci_force_enable_link_state() API and convert Atheros drivers Bjorn Helgaas
2026-10-15 8:35 ` Ilpo Järvinen
2026-10-16 5:58 ` Manivannan Sadhasivam
2026-10-16 10:17 ` Ilpo Järvinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=atG4Gaq7GLiq2H7t@debian \
--to=mani@kernel.org \
--cc=ath10k@lists.infradead.org \
--cc=ath11k@lists.infradead.org \
--cc=ath12k@lists.infradead.org \
--cc=bhelgaas@google.com \
--cc=helgaas@kernel.org \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=jjohnson@kernel.org \
--cc=jonathan.derrick@linux.dev \
--cc=krishna.chundru@oss.qualcomm.com \
--cc=kwilczynski@kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=lpieralisi@kernel.org \
--cc=nirmal.patel@linux.intel.com \
--cc=qiang.yu@oss.qualcomm.com \
--cc=robh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox