On Wed, Mar 29, 2023 at 11:01:44AM -0500, Bjorn Helgaas wrote: > On Sun, Mar 26, 2023 at 04:53:58PM +0300, Ido Schimmel wrote: > > On Thu, Mar 23, 2023 at 11:51:15AM -0500, Bjorn Helgaas wrote: > > > So the first question is why you don't simply use > > > pci_reset_function(), since it is supposed to cause a reset and do all > > > the necessary waiting for the device to be ready. I agree, this driver should use one of the reset helpers provided by the PCI core. Not least because if the Downstream Port is hotplug-capable, fiddling with the Link Disable bit behind the hotplug driver's back will cause unbinding and rebinding of the device. > > __pci_reset_function_locked() is another option, but it assumes the > > device lock was already taken, which is correct during probe, but not > > when reset is performed as part of devlink reload. Just call pci_dev_lock() in the devlink reload code path. > Hmmm, pci_pm_reset() puts the device in D3hot, then back to D0. Spec > says that results in "undefined internal Function state," which > doesn't even sound like a guaranteed reset, but it's what we have, and > in any case, it does not disable the link. Per PCIe r6.0.1 sec 5.8, the device undergoes a Soft Reset when moving from D3hot to D0 (unless the No_Soft_Reset bit is set in the Power Management Control/Status Register, sec 7.5.2.2). The driver can set the PCI_DEV_FLAGS_NO_PM_RESET flag if this reset method doesn't have any effect (via quirk_no_pm_reset()). We could also discuss moving pci_pm_reset after pci_reset_bus_function in pci_reset_fn_methods[] if this reset method has little value on the majority of devices. Alternatively, if Secondary Bus Reset has the intended effect, the driver could invoke that directly via pci_reset_bus(). > Spec (PCIe r6.0, sec 6.6.1) says "Disabling a Link causes Downstream > components to undergo a hot reset." That seems like it *could* be a > general-purpose method of doing a reset, and I don't know why the PCI > core doesn't support it. Added Alex and Lukas in case they know. Sounds reasonable to me. Thanks, Lukas
