On Thu, Apr 10, 2025 at 07:34:41PM -0700, Sathyanarayanan Kuppuswamy wrote:
> On 4/10/25 8:27 AM, Lukas Wunner wrote:
> > Commit a97396c6eb13 ("PCI: pciehp: Ignore Link Down/Up caused by DPC")
> > amended PCIe hotplug to not bring down the slot upon Data Link Layer State
> > Changed events caused by Downstream Port Containment.
> >
> > However Keith reports off-list that if the slot uses in-band presence
> > detect (i.e. Presence Detect State is derived from Data Link Layer Link
> > Active), DPC also causes a spurious Presence Detect Changed event.
> >
> > This needs to be ignored as well.
> >
> > Unfortunately there's no register indicating that in-band presence detect
> > is used. PCIe r5.0 sec 7.5.3.10 introduced the In-Band PD Disable bit in
> > the Slot Control Register. The PCIe hotplug driver sets this bit on
> > ports supporting it. But older ports may still use in-band presence
> > detect.
> >
> > If in-band presence detect can be disabled, Presence Detect Changed events
>
> It should be "in-band presence detect is disabled", right?
Well, for all practical purposes it's the same because pciehp disables
in-band PD if it can be disabled.
> > occurring during DPC must not be ignored because they signal device
> > replacement. On all other ports, device replacement cannot be detected
> > reliably because the Presence Detect Changed event could be a side effect
> > of DPC. On those (older) ports, perform a best-effort device replacement
> > check by comparing the Vendor ID, Device ID and other data in Config Space
> > with the values cached in struct pci_dev. Use the existing helper
> > pciehp_device_replaced() to accomplish this. It is currently #ifdef'ed to
> > CONFIG_PM_SLEEP in pciehp_core.c, so move it to pciehp_hpc.c where most
> > other functions accessing config space reside.
>
> Code looks fine to me
>
> Reviewed-by: Kuppuswamy Sathyanarayanan
> <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
Thanks for taking a look!
Lukas
