Message ID | 20240623033940.1806616-1-chenhuacai@loongson.cn (mailing list archive) |
---|---|
State | Not Applicable |
Delegated to: | Bjorn Helgaas |
Headers | show |
Series | PCI: PM: Fix PCIe MRRS restoring for Loongson | expand |
On Sun, Jun 23, 2024 at 11:39:40AM +0800, Huacai Chen wrote: > Don't limit MRRS during resume, so that saved value can be restored, > otherwise the MRRS will become the minimum value after resume. > > Cc: <stable@vger.kernel.org> > Fixes: 8b3517f88ff2983f ("PCI: loongson: Prevent LS7A MRRS increases") > Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn> > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> > --- > drivers/pci/pci.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 35fb1f17a589..bfc806d9e9bd 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -31,6 +31,7 @@ > #include <asm/dma.h> > #include <linux/aer.h> > #include <linux/bitfield.h> > +#include <linux/suspend.h> > #include "pci.h" > > DEFINE_MUTEX(pci_slot_mutex); > @@ -5945,7 +5946,7 @@ int pcie_set_readrq(struct pci_dev *dev, int rq) > > v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8); > > - if (bridge->no_inc_mrrs) { > + if (bridge->no_inc_mrrs && (pm_suspend_target_state == PM_SUSPEND_ON)) { I don't think we can check pm_suspend_target_state here. That seems like a layering/encapsulation problem. Are we failing to save this state at suspend? Seems like something we should address more explicitly higher up in the suspend/resume path where we save/restore config space. > int max_mrrs = pcie_get_readrq(dev); > > if (rq > max_mrrs) { > -- > 2.43.0 >
Hi, Bjorn, On Wed, Jun 26, 2024 at 11:08 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Sun, Jun 23, 2024 at 11:39:40AM +0800, Huacai Chen wrote: > > Don't limit MRRS during resume, so that saved value can be restored, > > otherwise the MRRS will become the minimum value after resume. > > > > Cc: <stable@vger.kernel.org> > > Fixes: 8b3517f88ff2983f ("PCI: loongson: Prevent LS7A MRRS increases") > > Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn> > > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> > > --- > > drivers/pci/pci.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > index 35fb1f17a589..bfc806d9e9bd 100644 > > --- a/drivers/pci/pci.c > > +++ b/drivers/pci/pci.c > > @@ -31,6 +31,7 @@ > > #include <asm/dma.h> > > #include <linux/aer.h> > > #include <linux/bitfield.h> > > +#include <linux/suspend.h> > > #include "pci.h" > > > > DEFINE_MUTEX(pci_slot_mutex); > > @@ -5945,7 +5946,7 @@ int pcie_set_readrq(struct pci_dev *dev, int rq) > > > > v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8); > > > > - if (bridge->no_inc_mrrs) { > > + if (bridge->no_inc_mrrs && (pm_suspend_target_state == PM_SUSPEND_ON)) { > > I don't think we can check pm_suspend_target_state here. That seems > like a layering/encapsulation problem. Are we failing to save this > state at suspend? Seems like something we should address more > explicitly higher up in the suspend/resume path where we save/restore > config space. I'm sorry, after some deep analysis, we found this patch is unnecessary, please ignore this. Huacai > > > int max_mrrs = pcie_get_readrq(dev); > > > > if (rq > max_mrrs) { > > -- > > 2.43.0 > >
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 35fb1f17a589..bfc806d9e9bd 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -31,6 +31,7 @@ #include <asm/dma.h> #include <linux/aer.h> #include <linux/bitfield.h> +#include <linux/suspend.h> #include "pci.h" DEFINE_MUTEX(pci_slot_mutex); @@ -5945,7 +5946,7 @@ int pcie_set_readrq(struct pci_dev *dev, int rq) v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8); - if (bridge->no_inc_mrrs) { + if (bridge->no_inc_mrrs && (pm_suspend_target_state == PM_SUSPEND_ON)) { int max_mrrs = pcie_get_readrq(dev); if (rq > max_mrrs) {