The PCI link, when down, needs to be recovered to bring it back. But that
cannot be done in a generic way as link recovery procedure is specific to
host bridges. So add a new API pci_host_handle_link_down() that could be
called by the host bridge drivers when the link goes down.
The API will iterate through all the slots and calls the pcie_do_recovery()
function with 'pci_channel_io_frozen' as the state. This will result in the
execution of the AER Fatal error handling code. Since the link down
recovery is pretty much the same as AER Fatal error handling,
pcie_do_recovery() helper is reused here. First the AER error_detected
callback will be triggered for the bridge and the downstream devices. Then,
pci_host_reset_slot() will be called for the slot, which will reset the
slot using 'reset_slot' callback to recover the link. Once that's done,
resume message will be broadcasted to the bridge and the downstream devices
indicating successful link recovery.
In case if the AER support is not enabled in the kernel, only
pci_bus_error_reset() will be called for each slots as there is no way we
could inform the drivers about link recovery.
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
drivers/pci/controller/pci-host-common.c | 58 ++++++++++++++++++++++++++++++++
drivers/pci/controller/pci-host-common.h | 1 +
drivers/pci/pci.c | 1 +
drivers/pci/pcie/err.c | 1 +
4 files changed, 61 insertions(+)
diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
index f93bc7034e697250711833a5151f7ef177cd62a0..f916f0a874a61ddfbfd99f96975c00fb66dd224c 100644
--- a/drivers/pci/controller/pci-host-common.c
+++ b/drivers/pci/controller/pci-host-common.c
@@ -12,9 +12,11 @@
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_pci.h>
+#include <linux/pci.h>
#include <linux/pci-ecam.h>
#include <linux/platform_device.h>
+#include "../pci.h"
#include "pci-host-common.h"
static void gen_pci_unmap_cfg(void *ptr)
@@ -96,5 +98,61 @@ void pci_host_common_remove(struct platform_device *pdev)
}
EXPORT_SYMBOL_GPL(pci_host_common_remove);
+#if IS_ENABLED(CONFIG_PCIEAER)
+static pci_ers_result_t pci_host_reset_slot(struct pci_dev *dev)
+{
+ int ret;
+
+ ret = pci_bus_error_reset(dev);
+ if (ret) {
+ pci_err(dev, "Failed to reset slot: %d\n", ret);
+ return PCI_ERS_RESULT_DISCONNECT;
+ }
+
+ pci_info(dev, "Slot has been reset\n");
+
+ return PCI_ERS_RESULT_RECOVERED;
+}
+
+static void pci_host_recover_slots(struct pci_host_bridge *host)
+{
+ struct pci_bus *bus = host->bus;
+ struct pci_dev *dev;
+
+ for_each_pci_bridge(dev, bus) {
+ if (!pci_is_root_bus(bus))
+ continue;
+
+ pcie_do_recovery(dev, pci_channel_io_frozen,
+ pci_host_reset_slot);
+ }
+}
+#else
+static void pci_host_recover_slots(struct pci_host_bridge *host)
+{
+ struct pci_bus *bus = host->bus;
+ struct pci_dev *dev;
+ int ret;
+
+ for_each_pci_bridge(dev, bus) {
+ if (!pci_is_root_bus(bus))
+ continue;
+
+ ret = pci_bus_error_reset(dev);
+ if (ret)
+ pci_err(dev, "Failed to reset slot: %d\n", ret);
+ else
+ pci_info(dev, "Slot has been reset\n");
+ }
+}
+#endif
+
+void pci_host_handle_link_down(struct pci_host_bridge *bridge)
+{
+ dev_info(&bridge->dev, "Recovering slots due to Link Down\n");
+ pci_host_recover_slots(bridge);
+}
+EXPORT_SYMBOL_GPL(pci_host_handle_link_down);
+
MODULE_DESCRIPTION("Common library for PCI host controller drivers");
MODULE_LICENSE("GPL v2");
diff --git a/drivers/pci/controller/pci-host-common.h b/drivers/pci/controller/pci-host-common.h
index d8be024ca68d43afb147fd9104d632b907277144..904698c1a2695888a0fc9c2fac360e456116eb1d 100644
--- a/drivers/pci/controller/pci-host-common.h
+++ b/drivers/pci/controller/pci-host-common.h
@@ -12,5 +12,6 @@
int pci_host_common_probe(struct platform_device *pdev);
void pci_host_common_remove(struct platform_device *pdev);
+void pci_host_handle_link_down(struct pci_host_bridge *bridge);
#endif
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 13709bb898a967968540826a2b7ee8ade6b7e082..4d396bbab4a8f33cae0ffe8982da120a9f1d92c9 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5781,6 +5781,7 @@ int pci_bus_error_reset(struct pci_dev *bridge)
mutex_unlock(&pci_slot_mutex);
return pci_bus_reset(bridge->subordinate, PCI_RESET_DO_RESET);
}
+EXPORT_SYMBOL_GPL(pci_bus_error_reset);
/**
* pci_probe_reset_bus - probe whether a PCI bus can be reset
diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index b834fc0d705938540d3d7d3d8739770c09fe7cf1..3e3084bb7cb7fa06b526e6fab60e77927aba0ad0 100644
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -270,3 +270,4 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
return status;
}
+EXPORT_SYMBOL_GPL(pcie_do_recovery);
--
2.43.0On Thu, May 08, 2025 at 12:40:33PM +0530, Manivannan Sadhasivam wrote: > The PCI link, when down, needs to be recovered to bring it back. But that > cannot be done in a generic way as link recovery procedure is specific to > host bridges. So add a new API pci_host_handle_link_down() that could be > called by the host bridge drivers when the link goes down. IIUC you plumbed this into the reset path so the standard entries (pci_reset_function() and the sysfs "reset" files) work can now work for Root Ports on DT systems just like they do for ACPI systems (assuming the ACPI systems supply an _RST method for the ports). That all sounds good. > The API will iterate through all the slots and calls the pcie_do_recovery() > function with 'pci_channel_io_frozen' as the state. This will result in the > execution of the AER Fatal error handling code. Since the link down > recovery is pretty much the same as AER Fatal error handling, > pcie_do_recovery() helper is reused here. First the AER error_detected > callback will be triggered for the bridge and the downstream devices. Then, > pci_host_reset_slot() will be called for the slot, which will reset the > slot using 'reset_slot' callback to recover the link. Once that's done, > resume message will be broadcasted to the bridge and the downstream devices > indicating successful link recovery. We have standard PCIe mechanisms to learn about "link down" events, e.g., AER Surprise Down error reporting and the Data Link Layer State Changed events for hot-plug capable ports. How does this controller-specific "link down" notification relate to those? Is this for controllers that don't support those AER or hotplug mechanisms? Or is this a different scenario that wouldn't be covered by them? If AER is enabled, do we get both the AER interrupt and the controller "link down" interrupt? > In case if the AER support is not enabled in the kernel, only > pci_bus_error_reset() will be called for each slots as there is no way we > could inform the drivers about link recovery.
On Thu, May 08, 2025 at 12:40:33PM +0530, Manivannan Sadhasivam wrote: > The PCI link, when down, needs to be recovered to bring it back. But that > cannot be done in a generic way as link recovery procedure is specific to > host bridges. So add a new API pci_host_handle_link_down() that could be > called by the host bridge drivers when the link goes down. > > The API will iterate through all the slots and calls the pcie_do_recovery() > function with 'pci_channel_io_frozen' as the state. This will result in the > execution of the AER Fatal error handling code. Since the link down > recovery is pretty much the same as AER Fatal error handling, > pcie_do_recovery() helper is reused here. First the AER error_detected > callback will be triggered for the bridge and the downstream devices. Then, > pci_host_reset_slot() will be called for the slot, which will reset the > slot using 'reset_slot' callback to recover the link. Once that's done, > resume message will be broadcasted to the bridge and the downstream devices > indicating successful link recovery. Link down is an event for a single Root Port. Why would we iterate through all the Root Ports if the link went down for one of them?
On Wed, May 28, 2025 at 05:35:00PM -0500, Bjorn Helgaas wrote: > On Thu, May 08, 2025 at 12:40:33PM +0530, Manivannan Sadhasivam wrote: > > The PCI link, when down, needs to be recovered to bring it back. But that > > cannot be done in a generic way as link recovery procedure is specific to > > host bridges. So add a new API pci_host_handle_link_down() that could be > > called by the host bridge drivers when the link goes down. > > > > The API will iterate through all the slots and calls the pcie_do_recovery() > > function with 'pci_channel_io_frozen' as the state. This will result in the > > execution of the AER Fatal error handling code. Since the link down > > recovery is pretty much the same as AER Fatal error handling, > > pcie_do_recovery() helper is reused here. First the AER error_detected > > callback will be triggered for the bridge and the downstream devices. Then, > > pci_host_reset_slot() will be called for the slot, which will reset the > > slot using 'reset_slot' callback to recover the link. Once that's done, > > resume message will be broadcasted to the bridge and the downstream devices > > indicating successful link recovery. > > Link down is an event for a single Root Port. Why would we iterate > through all the Root Ports if the link went down for one of them? Because on the reference platform (Qcom), link down notification is not per-port, but per controller. So that's why we are iterating through all ports. The callback is supposed to identify the ports that triggered the link down event and recover them. - Mani -- மணிவண்ணன் சதாசிவம்
On Fri, May 30, 2025 at 09:16:59AM +0530, Manivannan Sadhasivam wrote: > On Wed, May 28, 2025 at 05:35:00PM -0500, Bjorn Helgaas wrote: > > On Thu, May 08, 2025 at 12:40:33PM +0530, Manivannan Sadhasivam wrote: > > > The PCI link, when down, needs to be recovered to bring it back. But that > > > cannot be done in a generic way as link recovery procedure is specific to > > > host bridges. So add a new API pci_host_handle_link_down() that could be > > > called by the host bridge drivers when the link goes down. > > > > > > The API will iterate through all the slots and calls the pcie_do_recovery() > > > function with 'pci_channel_io_frozen' as the state. This will result in the > > > execution of the AER Fatal error handling code. Since the link down > > > recovery is pretty much the same as AER Fatal error handling, > > > pcie_do_recovery() helper is reused here. First the AER error_detected > > > callback will be triggered for the bridge and the downstream devices. Then, > > > pci_host_reset_slot() will be called for the slot, which will reset the > > > slot using 'reset_slot' callback to recover the link. Once that's done, > > > resume message will be broadcasted to the bridge and the downstream devices > > > indicating successful link recovery. > > > > Link down is an event for a single Root Port. Why would we iterate > > through all the Root Ports if the link went down for one of them? > > Because on the reference platform (Qcom), link down notification is > not per-port, but per controller. So that's why we are iterating > through all ports. The callback is supposed to identify the ports > that triggered the link down event and recover them. Maybe I'm missing something. Which callback identifies the port(s) that triggered the link down event? I see that pci_host_handle_link_down() is called by rockchip_pcie_rc_sys_irq_thread() and qcom_pcie_global_irq_thread(), but I don't see the logic that identifies a particular Root Port. Per-controller notification of per-port events is a controller deficiency, not something inherent to PCIe. I don't think we should build common infrastructure that resets all the Root Ports just because one of them had an issue. I think pci_host_handle_link_down() should take a Root Port, not a host bridge, and the controller driver should figure out which port needs to be recovered, or the controller driver can have its own loop to recover all of them if it can't figure out which one needs it. Bjorn
On Fri, May 30, 2025 at 06:34:04AM -0500, Bjorn Helgaas wrote: > On Fri, May 30, 2025 at 09:16:59AM +0530, Manivannan Sadhasivam wrote: > > On Wed, May 28, 2025 at 05:35:00PM -0500, Bjorn Helgaas wrote: > > > On Thu, May 08, 2025 at 12:40:33PM +0530, Manivannan Sadhasivam wrote: > > > > The PCI link, when down, needs to be recovered to bring it back. But that > > > > cannot be done in a generic way as link recovery procedure is specific to > > > > host bridges. So add a new API pci_host_handle_link_down() that could be > > > > called by the host bridge drivers when the link goes down. > > > > > > > > The API will iterate through all the slots and calls the pcie_do_recovery() > > > > function with 'pci_channel_io_frozen' as the state. This will result in the > > > > execution of the AER Fatal error handling code. Since the link down > > > > recovery is pretty much the same as AER Fatal error handling, > > > > pcie_do_recovery() helper is reused here. First the AER error_detected > > > > callback will be triggered for the bridge and the downstream devices. Then, > > > > pci_host_reset_slot() will be called for the slot, which will reset the > > > > slot using 'reset_slot' callback to recover the link. Once that's done, > > > > resume message will be broadcasted to the bridge and the downstream devices > > > > indicating successful link recovery. > > > > > > Link down is an event for a single Root Port. Why would we iterate > > > through all the Root Ports if the link went down for one of them? > > > > Because on the reference platform (Qcom), link down notification is > > not per-port, but per controller. So that's why we are iterating > > through all ports. The callback is supposed to identify the ports > > that triggered the link down event and recover them. > > Maybe I'm missing something. Which callback identifies the port(s) > that triggered the link down event? I was referring to the host_bridge::reset_root_port() callback that resets the root ports. > I see that > pci_host_handle_link_down() is called by > rockchip_pcie_rc_sys_irq_thread() and qcom_pcie_global_irq_thread(), > but I don't see the logic that identifies a particular Root Port. > > Per-controller notification of per-port events is a controller > deficiency, not something inherent to PCIe. I don't think we should > build common infrastructure that resets all the Root Ports just > because one of them had an issue. > Hmm, fair enough. > I think pci_host_handle_link_down() should take a Root Port, not a > host bridge, and the controller driver should figure out which port > needs to be recovered, or the controller driver can have its own loop > to recover all of them if it can't figure out which one needs it. > This should also work. Feel free to drop the relevant commits for v6.16, I can resubmit them (including dw-rockchip after -rc1). - Mani -- மணிவண்ணன் சதாசிவம்
Hello Mani, On Fri, May 30, 2025 at 09:39:28PM +0530, Manivannan Sadhasivam wrote: > On Fri, May 30, 2025 at 06:34:04AM -0500, Bjorn Helgaas wrote: > > > I think pci_host_handle_link_down() should take a Root Port, not a > > host bridge, and the controller driver should figure out which port > > needs to be recovered, or the controller driver can have its own loop > > to recover all of them if it can't figure out which one needs it. > > > > This should also work. Feel free to drop the relevant commits for v6.16, I can > resubmit them (including dw-rockchip after -rc1). What is the current status of this? I assume that there is not much time left before 6.17 cut-off. Kind regards, Niklas
+ Mani's kernel.org email. On Mon, Jul 07, 2025 at 01:05:39PM +0200, Niklas Cassel wrote: > Hello Mani, > > On Fri, May 30, 2025 at 09:39:28PM +0530, Manivannan Sadhasivam wrote: > > On Fri, May 30, 2025 at 06:34:04AM -0500, Bjorn Helgaas wrote: > > > > > I think pci_host_handle_link_down() should take a Root Port, not a > > > host bridge, and the controller driver should figure out which port > > > needs to be recovered, or the controller driver can have its own loop > > > to recover all of them if it can't figure out which one needs it. > > > > > > > This should also work. Feel free to drop the relevant commits for v6.16, I can > > resubmit them (including dw-rockchip after -rc1). > > What is the current status of this? > > I assume that there is not much time left before 6.17 cut-off. > > > Kind regards, > Niklas
On Mon, Jul 07, 2025 at 01:29:22PM GMT, Niklas Cassel wrote: > + Mani's kernel.org email. > > On Mon, Jul 07, 2025 at 01:05:39PM +0200, Niklas Cassel wrote: > > Hello Mani, > > > > On Fri, May 30, 2025 at 09:39:28PM +0530, Manivannan Sadhasivam wrote: > > > On Fri, May 30, 2025 at 06:34:04AM -0500, Bjorn Helgaas wrote: > > > > > > > I think pci_host_handle_link_down() should take a Root Port, not a > > > > host bridge, and the controller driver should figure out which port > > > > needs to be recovered, or the controller driver can have its own loop > > > > to recover all of them if it can't figure out which one needs it. > > > > > > > > > > This should also work. Feel free to drop the relevant commits for v6.16, I can > > > resubmit them (including dw-rockchip after -rc1). > > > > What is the current status of this? > > Thanks for the nudge! I couldn't respin the series as I lost access to the hardware I was testing on due to job change. I'll figure out a way to test it and respin it asap. > > I assume that there is not much time left before 6.17 cut-off. Mid of -rc5 is not that bad ;) Let's see how long it takes. - Mani -- மணிவண்ணன் சதாசிவம்
On Fri, May 30, 2025 at 09:39:28PM +0530, Manivannan Sadhasivam wrote: > On Fri, May 30, 2025 at 06:34:04AM -0500, Bjorn Helgaas wrote: > > On Fri, May 30, 2025 at 09:16:59AM +0530, Manivannan Sadhasivam wrote: > > > On Wed, May 28, 2025 at 05:35:00PM -0500, Bjorn Helgaas wrote: > > > > On Thu, May 08, 2025 at 12:40:33PM +0530, Manivannan Sadhasivam wrote: > > > > > The PCI link, when down, needs to be recovered to bring it back. But that > > > > > cannot be done in a generic way as link recovery procedure is specific to > > > > > host bridges. So add a new API pci_host_handle_link_down() that could be > > > > > called by the host bridge drivers when the link goes down. > > > > > > > > > > The API will iterate through all the slots and calls the pcie_do_recovery() > > > > > function with 'pci_channel_io_frozen' as the state. This will result in the > > > > > execution of the AER Fatal error handling code. Since the link down > > > > > recovery is pretty much the same as AER Fatal error handling, > > > > > pcie_do_recovery() helper is reused here. First the AER error_detected > > > > > callback will be triggered for the bridge and the downstream devices. Then, > > > > > pci_host_reset_slot() will be called for the slot, which will reset the > > > > > slot using 'reset_slot' callback to recover the link. Once that's done, > > > > > resume message will be broadcasted to the bridge and the downstream devices > > > > > indicating successful link recovery. > > > > > > > > Link down is an event for a single Root Port. Why would we iterate > > > > through all the Root Ports if the link went down for one of them? > > > > > > Because on the reference platform (Qcom), link down notification is > > > not per-port, but per controller. So that's why we are iterating > > > through all ports. The callback is supposed to identify the ports > > > that triggered the link down event and recover them. > > > > Maybe I'm missing something. Which callback identifies the port(s) > > that triggered the link down event? > > I was referring to the host_bridge::reset_root_port() callback that resets the > root ports. > > > I see that > > pci_host_handle_link_down() is called by > > rockchip_pcie_rc_sys_irq_thread() and qcom_pcie_global_irq_thread(), > > but I don't see the logic that identifies a particular Root Port. > > > > Per-controller notification of per-port events is a controller > > deficiency, not something inherent to PCIe. I don't think we should > > build common infrastructure that resets all the Root Ports just > > because one of them had an issue. > > Hmm, fair enough. > > > I think pci_host_handle_link_down() should take a Root Port, not a > > host bridge, and the controller driver should figure out which port > > needs to be recovered, or the controller driver can have its own loop > > to recover all of them if it can't figure out which one needs it. > > This should also work. Feel free to drop the relevant commits for > v6.16, I can resubmit them (including dw-rockchip after -rc1). OK, I kept "PCI: host-common: Make the driver as a common library for host controller drivers" (renamed to "PCI: host-common: Convert to library for host controller drivers") on pci/controller/dw-rockchip for v6.16 and deferred the rest until later.
On 5/8/2025 12:40 PM, Manivannan Sadhasivam wrote:
> The PCI link, when down, needs to be recovered to bring it back. But that
> cannot be done in a generic way as link recovery procedure is specific to
> host bridges. So add a new API pci_host_handle_link_down() that could be
> called by the host bridge drivers when the link goes down.
>
> The API will iterate through all the slots and calls the pcie_do_recovery()
> function with 'pci_channel_io_frozen' as the state. This will result in the
> execution of the AER Fatal error handling code. Since the link down
> recovery is pretty much the same as AER Fatal error handling,
> pcie_do_recovery() helper is reused here. First the AER error_detected
> callback will be triggered for the bridge and the downstream devices. Then,
> pci_host_reset_slot() will be called for the slot, which will reset the
> slot using 'reset_slot' callback to recover the link. Once that's done,
> resume message will be broadcasted to the bridge and the downstream devices
> indicating successful link recovery.
>
> In case if the AER support is not enabled in the kernel, only
> pci_bus_error_reset() will be called for each slots as there is no way we
> could inform the drivers about link recovery.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
> drivers/pci/controller/pci-host-common.c | 58 ++++++++++++++++++++++++++++++++
> drivers/pci/controller/pci-host-common.h | 1 +
> drivers/pci/pci.c | 1 +
> drivers/pci/pcie/err.c | 1 +
> 4 files changed, 61 insertions(+)
>
> diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> index f93bc7034e697250711833a5151f7ef177cd62a0..f916f0a874a61ddfbfd99f96975c00fb66dd224c 100644
> --- a/drivers/pci/controller/pci-host-common.c
> +++ b/drivers/pci/controller/pci-host-common.c
> @@ -12,9 +12,11 @@
> #include <linux/of.h>
> #include <linux/of_address.h>
> #include <linux/of_pci.h>
> +#include <linux/pci.h>
> #include <linux/pci-ecam.h>
> #include <linux/platform_device.h>
>
> +#include "../pci.h"
> #include "pci-host-common.h"
>
> static void gen_pci_unmap_cfg(void *ptr)
> @@ -96,5 +98,61 @@ void pci_host_common_remove(struct platform_device *pdev)
> }
> EXPORT_SYMBOL_GPL(pci_host_common_remove);
>
> +#if IS_ENABLED(CONFIG_PCIEAER)
> +static pci_ers_result_t pci_host_reset_slot(struct pci_dev *dev)
> +{
> + int ret;
> +
> + ret = pci_bus_error_reset(dev);
> + if (ret) {
> + pci_err(dev, "Failed to reset slot: %d\n", ret);
> + return PCI_ERS_RESULT_DISCONNECT;
> + }
> +
> + pci_info(dev, "Slot has been reset\n");
> +
> + return PCI_ERS_RESULT_RECOVERED;
> +}
> +
> +static void pci_host_recover_slots(struct pci_host_bridge *host)
> +{
> + struct pci_bus *bus = host->bus;
> + struct pci_dev *dev;
> +
> + for_each_pci_bridge(dev, bus) {
> + if (!pci_is_root_bus(bus))
bus here is always constant here, we may need to have check
for dev here like if (!pci_is_root_bus(dev->bus))
> + continue;
> +
> + pcie_do_recovery(dev, pci_channel_io_frozen,
> + pci_host_reset_slot);
> + }
> +}
> +#else
> +static void pci_host_recover_slots(struct pci_host_bridge *host)
> +{
> + struct pci_bus *bus = host->bus;
> + struct pci_dev *dev;
> + int ret;
> +
> + for_each_pci_bridge(dev, bus) {
> + if (!pci_is_root_bus(bus))Same comment as above.
- Krishna Chaitanya.
> + continue;
> +
> + ret = pci_bus_error_reset(dev);
> + if (ret)
> + pci_err(dev, "Failed to reset slot: %d\n", ret);
> + else
> + pci_info(dev, "Slot has been reset\n");
> + }
> +}
> +#endif
> +
> +void pci_host_handle_link_down(struct pci_host_bridge *bridge)
> +{
> + dev_info(&bridge->dev, "Recovering slots due to Link Down\n");
> + pci_host_recover_slots(bridge);
> +}
> +EXPORT_SYMBOL_GPL(pci_host_handle_link_down);
> +
> MODULE_DESCRIPTION("Common library for PCI host controller drivers");
> MODULE_LICENSE("GPL v2");
> diff --git a/drivers/pci/controller/pci-host-common.h b/drivers/pci/controller/pci-host-common.h
> index d8be024ca68d43afb147fd9104d632b907277144..904698c1a2695888a0fc9c2fac360e456116eb1d 100644
> --- a/drivers/pci/controller/pci-host-common.h
> +++ b/drivers/pci/controller/pci-host-common.h
> @@ -12,5 +12,6 @@
>
> int pci_host_common_probe(struct platform_device *pdev);
> void pci_host_common_remove(struct platform_device *pdev);
> +void pci_host_handle_link_down(struct pci_host_bridge *bridge);
>
> #endif
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 13709bb898a967968540826a2b7ee8ade6b7e082..4d396bbab4a8f33cae0ffe8982da120a9f1d92c9 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5781,6 +5781,7 @@ int pci_bus_error_reset(struct pci_dev *bridge)
> mutex_unlock(&pci_slot_mutex);
> return pci_bus_reset(bridge->subordinate, PCI_RESET_DO_RESET);
> }
> +EXPORT_SYMBOL_GPL(pci_bus_error_reset);
>
> /**
> * pci_probe_reset_bus - probe whether a PCI bus can be reset
> diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
> index b834fc0d705938540d3d7d3d8739770c09fe7cf1..3e3084bb7cb7fa06b526e6fab60e77927aba0ad0 100644
> --- a/drivers/pci/pcie/err.c
> +++ b/drivers/pci/pcie/err.c
> @@ -270,3 +270,4 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
>
> return status;
> }
> +EXPORT_SYMBOL_GPL(pcie_do_recovery);
>
On Wed, May 14, 2025 at 12:00:11PM +0530, Krishna Chaitanya Chundru wrote:
>
>
> On 5/8/2025 12:40 PM, Manivannan Sadhasivam wrote:
> > The PCI link, when down, needs to be recovered to bring it back. But that
> > cannot be done in a generic way as link recovery procedure is specific to
> > host bridges. So add a new API pci_host_handle_link_down() that could be
> > called by the host bridge drivers when the link goes down.
> >
> > The API will iterate through all the slots and calls the pcie_do_recovery()
> > function with 'pci_channel_io_frozen' as the state. This will result in the
> > execution of the AER Fatal error handling code. Since the link down
> > recovery is pretty much the same as AER Fatal error handling,
> > pcie_do_recovery() helper is reused here. First the AER error_detected
> > callback will be triggered for the bridge and the downstream devices. Then,
> > pci_host_reset_slot() will be called for the slot, which will reset the
> > slot using 'reset_slot' callback to recover the link. Once that's done,
> > resume message will be broadcasted to the bridge and the downstream devices
> > indicating successful link recovery.
> >
> > In case if the AER support is not enabled in the kernel, only
> > pci_bus_error_reset() will be called for each slots as there is no way we
> > could inform the drivers about link recovery.
> >
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> > drivers/pci/controller/pci-host-common.c | 58 ++++++++++++++++++++++++++++++++
> > drivers/pci/controller/pci-host-common.h | 1 +
> > drivers/pci/pci.c | 1 +
> > drivers/pci/pcie/err.c | 1 +
> > 4 files changed, 61 insertions(+)
> >
> > diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> > index f93bc7034e697250711833a5151f7ef177cd62a0..f916f0a874a61ddfbfd99f96975c00fb66dd224c 100644
> > --- a/drivers/pci/controller/pci-host-common.c
> > +++ b/drivers/pci/controller/pci-host-common.c
> > @@ -12,9 +12,11 @@
> > #include <linux/of.h>
> > #include <linux/of_address.h>
> > #include <linux/of_pci.h>
> > +#include <linux/pci.h>
> > #include <linux/pci-ecam.h>
> > #include <linux/platform_device.h>
> > +#include "../pci.h"
> > #include "pci-host-common.h"
> > static void gen_pci_unmap_cfg(void *ptr)
> > @@ -96,5 +98,61 @@ void pci_host_common_remove(struct platform_device *pdev)
> > }
> > EXPORT_SYMBOL_GPL(pci_host_common_remove);
> > +#if IS_ENABLED(CONFIG_PCIEAER)
> > +static pci_ers_result_t pci_host_reset_slot(struct pci_dev *dev)
> > +{
> > + int ret;
> > +
> > + ret = pci_bus_error_reset(dev);
> > + if (ret) {
> > + pci_err(dev, "Failed to reset slot: %d\n", ret);
> > + return PCI_ERS_RESULT_DISCONNECT;
> > + }
> > +
> > + pci_info(dev, "Slot has been reset\n");
> > +
> > + return PCI_ERS_RESULT_RECOVERED;
> > +}
> > +
> > +static void pci_host_recover_slots(struct pci_host_bridge *host)
> > +{
> > + struct pci_bus *bus = host->bus;
> > + struct pci_dev *dev;
> > +
> > + for_each_pci_bridge(dev, bus) {
> > + if (!pci_is_root_bus(bus))
> bus here is always constant here, we may need to have check
> for dev here like if (!pci_is_root_bus(dev->bus))
Good catch! Ammended it while applying.
- Mani
--
மணிவண்ணன் சதாசிவம்
© 2016 - 2025 Red Hat, Inc.