On Tue, 2021-01-12 at 15:36 -0600, Bjorn Helgaas wrote: > Note subject line tips at https://lore.kernel.org/r/20171026223701.GA25649@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx > > On Tue, Jan 12, 2021 at 03:27:39PM +0800, mingchuang.qiao@xxxxxxxxxxxx wrote: > > From: Mingchuang Qiao <mingchuang.qiao@xxxxxxxxxxxx> > > > > In pci bus scan flow, the LTR mechanism enable bit of DEVCTL2 register > > is configured in pci_configure_ltr(). If device and it's bridge both > > support LTR mechanism, LTR mechanism of device and it's bridge will > > be enabled in DEVCTL2 register. And the flag pci_dev->ltr_path will > > be set as 1. > > s/it's/its/ twice above. > It's == It is. > Its == belonging to 'it'. > Weird, I know, but that's English for you :) > > > For some pcie products, pcie link becomes down when device reset. And then > > the LTR mechanism enable bit of bridge will become 0 based on description > > in PCIE r4.0, sec 7.8.16. However, the pci_dev->ltr_path value of bridge > > is still 1. Remove and rescan flow could be triggered to recover after > > device reset. In the bus rescan flow, the LTR mechanism of device will be > > enabled in pci_configure_ltr() due to ltr_path of its bridge is still 1. > > s/pcie/PCIe/ twice above. > s/PCIE/PCIe/; also reference r5.0 instead of r4.0 if possible. > Sorry for the misspelling and inconvenience. Thanks for the correction and I will follow the suggestion in later patch. > This sounds like a general problem of most device config bits being > cleared by reset. Usually these are restored by pci_restore_state(). > Is that function missing a restore for PCI_EXP_DEVCTL2? I'd rather > have a general-purpose way of restoring all the config state than > little pieces scattered all over. > Actually it’s not PCI_EXP_DEVCTL2 restore missing issue. The PCI_EXP_DEVCTL2 is correctly saved/restored during bridge runtime suspend/resume. It’s the issue that ltr_path value of the bridge mismatches the actual value in bridge’s PCI_EXP_DEVCTL2. Here is the scenario for the issue: 1.PCI bus scan done -For both PCIe Device and bridge: -"LTR Mechanism Enable" bit in PCI_EXP_DEVCTL2 is 1 - ltr_path value is 1 2.Device resets and PCIe link is down, then "LTR Mechanism Enable" bit of bridge changes to 0 according to PCIe r5.0, sec 7.5.3.16. -The ltr_path value of bridge is still 1 but the "LTR Mechanism Enable" bit within PCI_EXP_DEVCTL2 changed to 0. 3.Trigger device removal and bridge enters runtime suspend flow. -"LTR Mechanism Enable" bit of bridge is 0 and saved by pci_save_state(). 4.Trigger bus rescan and bridge enters runtime resume flow. -"LTR Mechanism Enable" bit of bridge is restored by pci_restore_state() and the value is 0. 5.Scan device and configure device's LTR in pci_configure_ltr(). -"LTR Mechanism Enable" bit of device is configured as 1 due to bridge's ltr_path value is 1. 6.The "LTR Mechanism Enable" bit of device and bridge is different now. -When device sends LTR Message, bridge will treat the Message as Unsupported Request according to PCIe r5.0, sec 6.18. This patch is used to make bridge's ltr_path value match "LTR Mechanism Enable" bit within DEVCTL2 register and avoid the Unsupported Request. > > When device's LTR mechanism is enabled, device will send LTR message to > > bridge. Bridge receives the device's LTR message and found bridge's LTR > > mechanism is disabled. Then the bridge will generate unsupported request > > and other error handling flow will be triggered such as AER Non-Fatal > > error handling. > > > > This patch is used to avoid this unsupported request and make the bridge's > > ltr_path value is aligned with DEVCTL2 register value. Check bridge > > register value if aligned with ltr_path in pci_configure_ltr(). If > > register value is disable and the ltr_path is 1, we need configure > > the register value as enable. > > > > Signed-off-by: Mingchuang Qiao <mingchuang.qiao@xxxxxxxxxxxx> > > --- > > drivers/pci/probe.c | 18 +++++++++++++++--- > > 1 file changed, 15 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > > index 953f15abc850..49355cf4af54 100644 > > --- a/drivers/pci/probe.c > > +++ b/drivers/pci/probe.c > > @@ -2132,9 +2132,21 @@ static void pci_configure_ltr(struct pci_dev *dev) > > * Complex and all intermediate Switches indicate support for LTR. > > * PCIe r4.0, sec 6.18. > > */ > > - if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT || > > - ((bridge = pci_upstream_bridge(dev)) && > > - bridge->ltr_path)) { > > + if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) { > > + pcie_capability_set_word(dev, PCI_EXP_DEVCTL2, > > + PCI_EXP_DEVCTL2_LTR_EN); > > + dev->ltr_path = 1; > > + return; > > + } > > + > > + bridge = pci_upstream_bridge(dev); > > + if (bridge && bridge->ltr_path) { > > + pcie_capability_read_dword(bridge, PCI_EXP_DEVCTL2, &ctl); > > + if (!(ctl & PCI_EXP_DEVCTL2_LTR_EN)) { > > + pcie_capability_set_word(bridge, PCI_EXP_DEVCTL2, > > + PCI_EXP_DEVCTL2_LTR_EN); > > + } > > + > > pcie_capability_set_word(dev, PCI_EXP_DEVCTL2, > > PCI_EXP_DEVCTL2_LTR_EN); > > dev->ltr_path = 1; > > -- > > 2.18.0 > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
