All of lore.kernel.org
 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: 33+ 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
2026-10-16 22:09     ` Bjorn Helgaas

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.