[+cc Lukas since you mentioned pciehp]
On Wed, May 01, 2024 at 07:51:18AM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@xxxxxxxxxx>
>
> After a link reset, the Broadcom / LSI PEX890xx PCIe Gen 5 Switch in synth
> mode will temporarily insert a fake place-holder device, 1000 02b2, before
> the link is actually active for the expected downstream device. Confirm
> the device's identifier matches what we expect before moving forward.
> Otherwise, the pciehp driver may unmask hotplug notifications before
> the link is actually active, which triggers an undesired device removal.
Is this a Switch that doesn't conform to the PCIe spec, or is there
something wrong with the way we're looking for a CRS completion?
In the absence of a device defect, I would not expect to need a
Broacom-specific comment in this code.
> Cc: Suganath Prabu S <suganath-prabu.subramani@xxxxxxxxxxxx>
> Cc: Peter Delevoryas <pdel@xxxxxxxx>
> Signed-off-by: Keith Busch <kbusch@xxxxxxxxxx>
> ---
> drivers/pci/pci.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index e5f243dd42884..4dc00f7411a94 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -1255,6 +1255,7 @@ static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout)
> int delay = 1;
> bool retrain = false;
> struct pci_dev *bridge;
> + u32 vid = dev->vendor | dev->device << 16;
>
> if (pci_is_pcie(dev)) {
> bridge = pci_upstream_bridge(dev);
> @@ -1268,17 +1269,22 @@ static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout)
> * responding to them with CRS completions. The Root Port will
> * generally synthesize ~0 (PCI_ERROR_RESPONSE) data to complete
> * the read (except when CRS SV is enabled and the read was for the
> - * Vendor ID; in that case it synthesizes 0x0001 data).
> + * Vendor ID; in that case it synthesizes 0x0001 data, or if the device
> + * is downstream a Broadcom switch, which syntesizes a fake device)
s/syntesizes/synthesizes/
> * Wait for the device to return a non-CRS completion. Read the
> - * Command register instead of Vendor ID so we don't have to
> - * contend with the CRS SV value.
> + * Command register instead of Vendor ID so we don't have to contend
> + * with the CRS SV value. But, also read the Vendor and Device ID's
> + * to defeat Broadcom switch's placeholder device.
s/ID's/IDs/
> */
> for (;;) {
> - u32 id;
> + u32 id, l;
>
> + pci_read_config_dword(dev, PCI_VENDOR_ID, &l);
> pci_read_config_dword(dev, PCI_COMMAND, &id);
> - if (!PCI_POSSIBLE_ERROR(id))
> +
> + if (!PCI_POSSIBLE_ERROR(id) && !PCI_POSSIBLE_ERROR(l) &&
> + l == vid)
> break;
>
> if (delay > timeout) {
> --
> 2.43.0
>
