Re: [PATCH] PCI: Mark NXP LS1088 to avoid bus reset bus

[Bjorn Helgaas <helgaas@kernel.org>]

[+cc David, Jan, Alex, Maik, Chris]

On Tue, Nov 27, 2018 at 08:46:33AM +0000, Bharat Bhushan wrote:
> NXP (Freescale Vendor ID) LS1088 chips do not behave correctly after
> bus reset with e1000e. Link state of device does not comes UP and so
> config space never accessible again.

Previous similar commits:

  822155100e58 ("PCI: Mark Cavium CN8xxx to avoid bus reset")
  8e2e03179923 ("PCI: Mark Atheros AR9580 to avoid bus reset")
  9ac0108c2bac ("PCI: Mark Atheros AR9485 and QCA9882 to avoid bus reset")
  c3e59ee4e766 ("PCI: Mark Atheros AR93xx to avoid bus reset")

1) Please make your subject match (remove the spurious "bus" at the
end)

2) This should probably be marked for stable (v3.14 and later, since
the quirk itself appeared in v3.19 and marked for v3.14 and later
stable kernels).  Maybe even mark it as "Fixes: c3e59ee4e766..." to
connect it.

3) The 1957:80c0 PCI ID doesn't appear in https://pci-ids.ucw.cz/; can
you add it?

4) Is there a hardware erratum for this?  If so, please include the
URL here.

5) Can you reproduce the problem using the same endpoint (e1000e) on a
different system with a different bridge?

6) Have you looked at this with a PCIe analyzer?  It would be very
interesting to compare the boot-time or system reboot path with the
individual bus reset path you're fixing.

Since there are several similar reports and they sometimes involve the
same devices (both your patch and 822155100e58 mention e1000e), I'm a
little suspicious that we're doing something wrong in the bus reset
path.

I think bus reset uses Secondary Bus Reset in the Bridge Control
register.  That's a generic mechanism that I would expect to be pretty
well-tested.  I suspect the BIOS probably uses it in the reboot path,
and the device probably works after that.

So I wonder if the Linux delay isn't quite long enough, or our first
access to the device isn't quite right, e.g., maybe there's some issue
with the bus/device number capture (PCIe r4.0, sec 2.2.6.2).

> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
> ---
>  drivers/pci/quirks.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 4700d24e5d55..b9ae4e9f101a 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3391,6 +3391,13 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATHEROS, 0x0033, quirk_no_bus_reset);
>   */
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CAVIUM, 0xa100, quirk_no_bus_reset);
>  
> +/*
> + * NXP (Freescale Vendor ID) LS1088 chips do not behave correctly after
> + * bus reset. Link state of device does not comes UP and so config space
> + * never accessible again.
> + */
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, 0x80c0, quirk_no_bus_reset);
> +
>  static void quirk_no_pm_reset(struct pci_dev *dev)
>  {
>  	/*
> -- 
> 2.19.1
>