Linux PCI subsystem development
 help / color / mirror / Atom feed
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

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

  reply	other threads:[~2026-10-16  5:38 UTC|newest]

Thread overview: 32+ 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-07-08 14:44   ` sashiko-bot
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-07-08 14:44   ` sashiko-bot
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-07-08 14:50   ` sashiko-bot
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:37   ` sashiko-bot
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:36   ` sashiko-bot
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-08 14:49   ` sashiko-bot
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:47   ` sashiko-bot
2026-07-08 14:30 ` [PATCH v3 8/8] wifi: ath10k: " Manivannan Sadhasivam
2026-07-08 14:51   ` sashiko-bot
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