| Message ID | 20250404-pcie-reset-slot-v1-2-98952918bf90@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: Add support for resetting the slots in a platform specific way | expand |
On Fri, Apr 04, 2025 at 01:52:22PM +0530, Manivannan Sadhasivam via B4 Relay wrote: > When the PCI error handling requires resetting the slot, reset it using the > host bridge specific 'reset_slot' callback if available before calling the > 'slot_reset' callback of the PCI drivers. > > The 'reset_slot' callback is responsible for resetting the given slot > referenced by the 'pci_dev' pointer in a platform specific way and bring it > back to the working state if possible. If any error occurs during the slot > reset operation, relevant errno should be returned. [...] > --- a/drivers/pci/pcie/err.c > +++ b/drivers/pci/pcie/err.c > @@ -234,11 +234,16 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev, > } > > if (status == PCI_ERS_RESULT_NEED_RESET) { > - /* > - * TODO: Should call platform-specific > - * functions to reset slot before calling > - * drivers' slot_reset callbacks? > - */ > + if (host->reset_slot) { > + ret = host->reset_slot(host, bridge); > + if (ret) { > + pci_err(bridge, "failed to reset slot: %d\n", > + ret); > + status = PCI_ERS_RESULT_DISCONNECT; > + goto failed; > + } > + } > + This feels like something that should be plumbed into pcibios_reset_secondary_bus(), rather than pcie_do_recovery(). Note that in the DPC case, pcie_do_recovery() doesn't issue a reset itself. The reset has already happened, it was automatically done by the hardware and all the kernel needs to do is bring up the link again. Do you really need any special handling for that in the host controller driver? Only in the AER case do you want to issue a reset on the secondary bus and if there's any platform-specific support needed for that, it needs to go into pcibios_reset_secondary_bus(). Thanks, Lukas
diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c index de6381c690f5c21f00021cdc7bde8d93a5c7db52..77ce9354532afee209f658175b86e625bba8a5ee 100644 --- a/drivers/pci/pcie/err.c +++ b/drivers/pci/pcie/err.c @@ -234,11 +234,16 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev, } if (status == PCI_ERS_RESULT_NEED_RESET) { - /* - * TODO: Should call platform-specific - * functions to reset slot before calling - * drivers' slot_reset callbacks? - */ + if (host->reset_slot) { + ret = host->reset_slot(host, bridge); + if (ret) { + pci_err(bridge, "failed to reset slot: %d\n", + ret); + status = PCI_ERS_RESULT_DISCONNECT; + goto failed; + } + } + status = PCI_ERS_RESULT_RECOVERED; pci_dbg(bridge, "broadcast slot_reset message\n"); pci_walk_bridge(bridge, report_slot_reset, &status); diff --git a/include/linux/pci.h b/include/linux/pci.h index 0e8e3fd77e96713054388bdc82f439e51023c1bf..8d7d2a49b76cf64b4218b179cec495e0d69ddf6f 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -599,6 +599,7 @@ struct pci_host_bridge { void (*release_fn)(struct pci_host_bridge *); int (*enable_device)(struct pci_host_bridge *bridge, struct pci_dev *dev); void (*disable_device)(struct pci_host_bridge *bridge, struct pci_dev *dev); + int (*reset_slot)(struct pci_host_bridge *bridge, struct pci_dev *dev); void *release_data; unsigned int ignore_reset_delay:1; /* For entire hierarchy */ unsigned int no_ext_tags:1; /* No Extended Tags */