Re: [PATCH v4 1/2] PCI/ERR: Call pci_bus_reset() before calling ->slot_reset() callback

[Kuppuswamy, Sathyanarayanan]

On 10/14/20 1:00 AM, Ethan Zhao wrote:
> Please fix the building issue.
>
> drivers/pci/pcie/err.c:144:25: error: static declaration of
> ‘pcie_do_fatal_recovery’ follows non-static declaration
>   static pci_ers_result_t pcie_do_fatal_recovery(struct pci_dev *dev,
>                           ^~~~~~~~~~~~~~~~~~~~~~
> In file included from drivers/pci/pcie/err.c:21:
> drivers/pci/pcie/../pci.h:560:18: note: previous declaration of
> ‘pcie_do_fatal_recovery’ was here
>   pci_ers_result_t pcie_do_fatal_recovery(struct pci_dev *dev,
>                    ^~~~~~~~~~~~~~~~~~~~~~
> drivers/pci/pcie/err.c:144:25: warning: ‘pcie_do_fatal_recovery’
> defined but not used [-Wunused-function]
>   static pci_ers_result_t pcie_do_fatal_recovery(struct pci_dev *dev,
Fixed in v6. Please use that version.
> Thanks,
> Ethan
>
> On Tue, Oct 13, 2020 at 10:18 PM Kuppuswamy, Sathyanarayanan
> <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx> wrote:
> > On 10/12/20 2:05 PM, Raj, Ashok wrote:
> > > On Sun, Oct 11, 2020 at 10:03:40PM -0700, sathyanarayanan.nkuppuswamy@xxxxxxxxx wrote:
> > > > From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
> > > >
> > > > Currently if report_error_detected() or report_mmio_enabled()
> > > > functions requests PCI_ERS_RESULT_NEED_RESET, current
> > > > pcie_do_recovery() implementation does not do the requested
> > > > explicit device reset, but instead just calls the
> > > > report_slot_reset() on all affected devices. Notifying about the
> > > > reset via report_slot_reset() without doing the actual device
> > > > reset is incorrect. So call pci_bus_reset() before triggering
> > > > ->slot_reset() callback.
> > > >
> > > > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
> > > > ---
> > > >    drivers/pci/pcie/err.c | 6 +-----
> > > >    1 file changed, 1 insertion(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
> > > > index c543f419d8f9..067c58728b88 100644
> > > > --- a/drivers/pci/pcie/err.c
> > > > +++ b/drivers/pci/pcie/err.c
> > > > @@ -181,11 +181,7 @@ 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?
> > > > -             */
> > > > +            pci_reset_bus(dev);
> > >
> > > pci_reset_bus() returns an error, do you need to consult that before
> > > unconditionally setting PCI_ERS_RESULT_RECOVERED?
> > Good point. I will fix this in next version.
> > > >               status = PCI_ERS_RESULT_RECOVERED;
> > > >               pci_dbg(dev, "broadcast slot_reset message\n");
> > > >               pci_walk_bus(bus, report_slot_reset, &status);
> > > > --
> > > > 2.17.1
> >
> > --
> > Sathyanarayanan Kuppuswamy
> > Linux Kernel Developer

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer