On Sun, Sep 02, 2018 at 02:34:02AM -0700, Lukas Wunner wrote: > On Fri, Aug 31, 2018 at 03:26:34PM -0600, Keith Busch wrote: > > --- a/drivers/pci/pcie/portdrv_pci.c > > +++ b/drivers/pci/pcie/portdrv_pci.c > > @@ -174,7 +174,9 @@ static int slot_reset_iter(struct device *device, void *data) > > > > static pci_ers_result_t pcie_portdrv_slot_reset(struct pci_dev *dev) > > { > > + pci_restore_state(dev); > > device_for_each_child(&dev->dev, dev, slot_reset_iter); > > + pci_save_state(dev); > > return PCI_ERS_RESULT_RECOVERED; > > } > > Shouldn't this be the other way round, i.e. save, reset, restore? No, this is the correct order. The state is originally saved in pcie_portdrv_probe, and we need to restore to that state in the slot reset callback. Once the slot has been restored through all the child drivers, then we need to save the state again for any future error handling. > Also, the function pcie_portdrv_slot_reset() was introduced in the prior > patch, so it seems this is a fix for that other patch and the two should > be squashed together. I guess they could be squashed, but the problems they're addressing seemed different enough to warrant separate change logs. The previous patch just set up the infrastructure for port error callbacks and chaining to child drivers. This patch is fixing a specific problem to restore the slot to a usable state.