Hi,

On Fri, Jan 22, 2021 at 03:03:11PM +0800, Mingchuang Qiao wrote:

On Thu, 2021-01-21 at 16:31 -0600, Bjorn Helgaas wrote:

quoted

[+cc Alex and Mingchuang et al from
https://lore.kernel.org/r/20210112072739.31624-1-mingchuang.qiao@mediatek.com (local)]

On Tue, Jan 19, 2021 at 04:14:10PM +0300, Mika Westerberg wrote:

quoted

PCIe r5.0, sec 7.5.3.16 says that the downstream ports must reset the
LTR enable bit if the link goes down (port goes DL_Down status). Now, if
we had LTR previously enabled and the PCIe endpoint gets hot-removed and
then hot-added back the ->ltr_path of the downstream port is still set
but the port now does not have the LTR enable bit set anymore.

For this reason check if the bridge upstream had LTR enabled previously
and re-enable it before enabling LTR for the endpoint.

Reported-by: Utkarsh H Patel <redacted>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

I think this and Mingchuang's patch, which is essentially identical,
are right and solves the problem for hot-remove/hot-add.  In that
scenario we call pci_configure_ltr() on the hot-added device, and
with this patch, we'll re-enable LTR on the bridge leading to the new
device before enabling LTR on the new device itself.

But don't we have a similar problem if we simply do a Fundamental
Reset on a device?  I think the reset path will restore the device's
state, including PCI_EXP_DEVCTL2, but it doesn't do anything with the
upstream bridge, does it?

Yes. I think the same problem exists under such scenario, and that’s the
issue my patch intends to resolve.
I also prepared a v2 patch for review(update the patch description).
Shall I submit the v2 patch for review?

I looked at your patch and indeed it is essentially doing the same as
this one. So let's forget this patch and go forward with yours :)

Would you like to expand your patch to handle the reset case too that
Bjorn desribes below?

quoted

So if a bridge and a device below it both have LTR enabled, can't we
have the following:

  - bridge LTR enabled
  - device LTR enabled
  - reset device, e.g., via Secondary Bus Reset
  - link goes down, bridge disables LTR
  - link comes back up, LTR disabled in both bridge and device
  - restore device state, including LTR enable
  - device sends LTR message
  - bridge reports Unsupported Request

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel