| Message ID | 20240506051602.1990743-1-kai.heng.feng@canonical.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | f3d049b35b01fff656d720606fcbab0b819f26d1 |
| Headers | show |
| Series | PCI/ASPM: Fix a typo in ASPM restoring logic | expand |
On Mon, 6 May 2024, Kai-Heng Feng wrote: > There's a typo that makes parent device uses child LNKCTL value and vice > versa. This causes Micron NVMe to trigger a reboot upon system resume. > > Correct the typo to fix the issue. > > Fixes: 64dbb2d70744 ("PCI/ASPM: Disable L1 before configuring L1 Substates") > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > --- > drivers/pci/pcie/aspm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c > index 2428d278e015..47761c7ef267 100644 > --- a/drivers/pci/pcie/aspm.c > +++ b/drivers/pci/pcie/aspm.c > @@ -177,8 +177,8 @@ void pci_restore_aspm_l1ss_state(struct pci_dev *pdev) > /* Restore L0s/L1 if they were enabled */ > if (FIELD_GET(PCI_EXP_LNKCTL_ASPMC, clnkctl) || > FIELD_GET(PCI_EXP_LNKCTL_ASPMC, plnkctl)) { > - pcie_capability_write_word(parent, PCI_EXP_LNKCTL, clnkctl); > - pcie_capability_write_word(pdev, PCI_EXP_LNKCTL, plnkctl); > + pcie_capability_write_word(parent, PCI_EXP_LNKCTL, plnkctl); > + pcie_capability_write_word(pdev, PCI_EXP_LNKCTL, clnkctl); Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
[+cc David] On Mon, May 06, 2024 at 01:16:02PM +0800, Kai-Heng Feng wrote: > There's a typo that makes parent device uses child LNKCTL value and vice > versa. This causes Micron NVMe to trigger a reboot upon system resume. > > Correct the typo to fix the issue. > > Fixes: 64dbb2d70744 ("PCI/ASPM: Disable L1 before configuring L1 Substates") > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> This is something David did correctly in his original posting (https://lore.kernel.org/all/20240128233212.1139663-4-david.e.box@linux.intel.com/) and I broke it while reorganizing things (https://lore.kernel.org/all/20240223213733.GA115410@bhelgaas/). Thanks for finding this! Since 64dbb2d70744 was merged for v6.9-rc1, I queued this to for-linus for v6.9 with this subject: PCI/ASPM: Restore parent state to parent, child state to child since it's more than just an innocuous typo. > --- > drivers/pci/pcie/aspm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c > index 2428d278e015..47761c7ef267 100644 > --- a/drivers/pci/pcie/aspm.c > +++ b/drivers/pci/pcie/aspm.c > @@ -177,8 +177,8 @@ void pci_restore_aspm_l1ss_state(struct pci_dev *pdev) > /* Restore L0s/L1 if they were enabled */ > if (FIELD_GET(PCI_EXP_LNKCTL_ASPMC, clnkctl) || > FIELD_GET(PCI_EXP_LNKCTL_ASPMC, plnkctl)) { > - pcie_capability_write_word(parent, PCI_EXP_LNKCTL, clnkctl); > - pcie_capability_write_word(pdev, PCI_EXP_LNKCTL, plnkctl); > + pcie_capability_write_word(parent, PCI_EXP_LNKCTL, plnkctl); > + pcie_capability_write_word(pdev, PCI_EXP_LNKCTL, clnkctl); > } > } > > -- > 2.34.1 >
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index 2428d278e015..47761c7ef267 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -177,8 +177,8 @@ void pci_restore_aspm_l1ss_state(struct pci_dev *pdev) /* Restore L0s/L1 if they were enabled */ if (FIELD_GET(PCI_EXP_LNKCTL_ASPMC, clnkctl) || FIELD_GET(PCI_EXP_LNKCTL_ASPMC, plnkctl)) { - pcie_capability_write_word(parent, PCI_EXP_LNKCTL, clnkctl); - pcie_capability_write_word(pdev, PCI_EXP_LNKCTL, plnkctl); + pcie_capability_write_word(parent, PCI_EXP_LNKCTL, plnkctl); + pcie_capability_write_word(pdev, PCI_EXP_LNKCTL, clnkctl); } }
There's a typo that makes parent device uses child LNKCTL value and vice versa. This causes Micron NVMe to trigger a reboot upon system resume. Correct the typo to fix the issue. Fixes: 64dbb2d70744 ("PCI/ASPM: Disable L1 before configuring L1 Substates") Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> --- drivers/pci/pcie/aspm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)