Re: [PATCH v3 1/8] PCI/ASPM: Always disable ASPM when driver requests it

[Manivannan Sadhasivam]

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@xxxxxxxxxxxxxxx>
> > 
> > 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@xxxxxxxxxxxxxx/
> > Link: https://lore.kernel.org/all/20230511131441.45704-1-ilpo.jarvinen@xxxxxxxxxxxxxxx/
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> > [mani: commit message fixup]
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
> 
> 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@xxxxxxxxxxxxxxx/,
> 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@xxxxxxxxxxxxxxx/

> 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@xxxxxxxxxxxxxxx/
> and added the CONFIG_PCIEASPM dependency to the commit log.  The code
> change is the same.
> 

This looks good to me.

- Mani

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