On Thu, Oct 15, 2020 at 05:11:07PM -0700, Sean V Kelley wrote: > From: Sean V Kelley <sean.v.kelley@xxxxxxxxx> > > Consolidate subordinate bus checks with pci_walk_bus() into > pci_walk_bridge() for walking below potentially AER affected bridges. > > [bhelgaas: fix kerneldoc] > Suggested-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > Link: https://lore.kernel.org/r/20201002184735.1229220-7-seanvk.dev@xxxxxxxxxxxxxxxx > Signed-off-by: Sean V Kelley <sean.v.kelley@xxxxxxxxx> > Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > --- > drivers/pci/pcie/err.c | 30 +++++++++++++++++++++++------- > 1 file changed, 23 insertions(+), 7 deletions(-) > > diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c > index 931e75f2549d..8b53aecdb43d 100644 > --- a/drivers/pci/pcie/err.c > +++ b/drivers/pci/pcie/err.c > @@ -146,13 +146,30 @@ static int report_resume(struct pci_dev *dev, void *data) > return 0; > } > > +/** > + * pci_walk_bridge - walk bridges potentially AER affected > + * @bridge: bridge which may be a Port > + * @cb: callback to be called for each device found > + * @userdata: arbitrary pointer to be passed to callback > + * > + * If the device provided is a bridge, walk the subordinate bus, including > + * any bridged devices on buses under this bus. Call the provided callback > + * on each device found. > + */ > +static void pci_walk_bridge(struct pci_dev *bridge, > + int (*cb)(struct pci_dev *, void *), > + void *userdata) > +{ > + if (bridge->subordinate) Remind me why we add this bridge->subordinate test? I see that we're going to need it later, but I think we should add the test in the same patch that adds the case where "bridge->subordinate == NULL" becomes possible. Or else a note in this commit log about what's happening. AFAICT, this test is literally the only possible functional change in this patch, so the commit log should mention it. > + pci_walk_bus(bridge->subordinate, cb, userdata); > +} > + > pci_ers_result_t pcie_do_recovery(struct pci_dev *dev, > pci_channel_state_t state, > pci_ers_result_t (*reset_subordinates)(struct pci_dev *pdev)) > { > int type = pci_pcie_type(dev); > struct pci_dev *bridge; > - struct pci_bus *bus; > pci_ers_result_t status = PCI_ERS_RESULT_CAN_RECOVER; > > /* > @@ -165,23 +182,22 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev, > else > bridge = pci_upstream_bridge(dev); > > - bus = bridge->subordinate; > pci_dbg(bridge, "broadcast error_detected message\n"); > if (state == pci_channel_io_frozen) { > - pci_walk_bus(bus, report_frozen_detected, &status); > + pci_walk_bridge(bridge, report_frozen_detected, &status); > status = reset_subordinates(bridge); > if (status != PCI_ERS_RESULT_RECOVERED) { > pci_warn(bridge, "subordinate device reset failed\n"); > goto failed; > } > } else { > - pci_walk_bus(bus, report_normal_detected, &status); > + pci_walk_bridge(bridge, report_normal_detected, &status); > } > > if (status == PCI_ERS_RESULT_CAN_RECOVER) { > status = PCI_ERS_RESULT_RECOVERED; > pci_dbg(bridge, "broadcast mmio_enabled message\n"); > - pci_walk_bus(bus, report_mmio_enabled, &status); > + pci_walk_bridge(bridge, report_mmio_enabled, &status); > } > > if (status == PCI_ERS_RESULT_NEED_RESET) { > @@ -192,14 +208,14 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev, > */ > status = PCI_ERS_RESULT_RECOVERED; > pci_dbg(bridge, "broadcast slot_reset message\n"); > - pci_walk_bus(bus, report_slot_reset, &status); > + pci_walk_bridge(bridge, report_slot_reset, &status); > } > > if (status != PCI_ERS_RESULT_RECOVERED) > goto failed; > > pci_dbg(bridge, "broadcast resume message\n"); > - pci_walk_bus(bus, report_resume, &status); > + pci_walk_bridge(bridge, report_resume, &status); > > if (pcie_aer_is_native(bridge)) > pcie_clear_device_status(bridge); > -- > 2.28.0 >
