[+cc David] On Mon, Jan 08, 2024 at 06:12:48PM +0530, Vidya Sagar wrote: > Many PCIe device drivers save the configuration state of their respective > devices during probe and restore the same when their 'slot_reset' hook > is called through PCIe Error Recovery Handler. > > If the system has a change in ASPM policy after the driver's probe is > called and before error event occurred, 'slot_reset' hook restores the > PCIe configuration state to what it was at the time of probe but not with > what it was just before the occurrence of the error event. > This effectively leads to a mismatch in the ASPM configuration between > the device and its upstream parent device. > > Update the saved configuration state of the device with the latest info > whenever there is a change w.r.t ASPM policy. > > Signed-off-by: Vidya Sagar <vidyas@xxxxxxxxxx> This overlaps with David's patches that are currently queued for v6.9: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=be00f078ad2a Can you rebase this to apply on top of those (this is the pci/aspm branch)? > --- > V3: > * Addressed sathyanarayanan.kuppuswamy's review comments > > V2: > * Rebased on top of the tree code > * Addressed Bjorn's review comments > > drivers/pci/pcie/aspm.c | 24 +++++++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c > index 67b13f26ba7c..1b4f03044ce2 100644 > --- a/drivers/pci/pcie/aspm.c > +++ b/drivers/pci/pcie/aspm.c > @@ -138,16 +138,34 @@ static int policy_to_clkpm_state(struct pcie_link_state *link) > return 0; > } > > +static void pci_save_aspm_state(struct pci_dev *dev) > +{ > + struct pci_cap_saved_state *save_state; > + u16 *cap; > + > + if (!pci_is_pcie(dev)) > + return; > + > + save_state = pci_find_saved_cap(dev, PCI_CAP_ID_EXP); > + if (!save_state) > + return; > + > + cap = (u16 *)&save_state->cap.data[0]; > + pcie_capability_read_word(dev, PCI_EXP_LNKCTL, &cap[1]); > +} > + > static void pcie_set_clkpm_nocheck(struct pcie_link_state *link, int enable) > { > struct pci_dev *child; > struct pci_bus *linkbus = link->pdev->subordinate; > u32 val = enable ? PCI_EXP_LNKCTL_CLKREQ_EN : 0; > > - list_for_each_entry(child, &linkbus->devices, bus_list) > + list_for_each_entry(child, &linkbus->devices, bus_list) { > pcie_capability_clear_and_set_word(child, PCI_EXP_LNKCTL, > PCI_EXP_LNKCTL_CLKREQ_EN, > val); > + pci_save_aspm_state(child); > + } > link->clkpm_enabled = !!enable; > } > > @@ -767,6 +785,10 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state) > pcie_config_aspm_dev(parent, upstream); > > link->aspm_enabled = state; > + > + /* Update latest ASPM configuration in saved context */ > + pci_save_aspm_state(link->downstream); > + pci_save_aspm_state(parent); > } > > static void pcie_config_aspm_path(struct pcie_link_state *link) > -- > 2.25.1 >
