On Thu, Dec 31, 2020 at 10:27:01AM +0100, Loic Poulain wrote:
> Hi Mani,
>
> On Thu, 31 Dec 2020 at 08:18, Manivannan Sadhasivam
> <manivannan.sadhasivam@xxxxxxxxxx> wrote:
> >
> > On Tue, Dec 29, 2020 at 09:43:48AM +0100, Loic Poulain wrote:
> > > In AER capable root complex, errors are reported to the host which
> > > can then act accordingly and perform PCI recovering procedure.
> > >
> > > This patch enables error reporting and implements error_detected,
> > > slot_reset and resume callbacks.
> > >
> > > Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxx>
> > > Reviewed-by Hemant Kumar <hemantk@xxxxxxxxxxxxxx>
> > > ---
> > > drivers/bus/mhi/pci_generic.c | 50 +++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 50 insertions(+)
>
> > > +static pci_ers_result_t mhi_pci_slot_reset(struct pci_dev *pdev)
> > > +{
> > > + if (pci_enable_device(pdev)) {
> > > + dev_err(&pdev->dev, "Cannot re-enable PCI device after reset.\n");
> > > + return PCI_ERS_RESULT_DISCONNECT;
> > > + }
> > > +
> >
> > This callback will be called after PCI slot reset, so we should also be resetting
> > the device after enabling it.
>
> Yes, but that is done in mhi_pci_io_resume.
>
> From the PCI error recovery documentation, "drivers should not restart
> normal I/O processing operations at this point (in slot_reset) If all
> device drivers report success on this callback, the platform will call
> resume() to complete the sequence, and let the driver restart normal
> I/O processing."
> The actual MHI PCI/recovery is then done in resume (mhi_pci_io_resume).
>
If you read one paragraph above,
"This call gives drivers the chance to re-initialize the hardware
(re-download firmware, etc.). At this point, the driver may assume
that the card is in a fresh state and is fully functional. The slot
is unfrozen and the driver has full access to PCI config space,
memory mapped I/O space and DMA."
So at the end of this call, the device is assumed to be functional (then only
PCI_ERS_RESULT_RECOVERED makes sense).
IMO, you should call mhi_pci_reset_prepare() in this callback and
mhi_pci_reset_done() in resume(). No need to schedule the recovery work.
Thanks,
Mani
> Regards,
> Loic
