Re: [PATCH v1 2/4] mpi3mr: Support PCIe Error Recovery callback handlers

[Bjorn Helgaas <helgaas@kernel.org>]

On Thu, Dec 07, 2023 at 09:55:09AM -0700, Sathya Prakash Veerichetty wrote:
> On Wed, Dec 6, 2023 at 9:56 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Wed, Dec 06, 2023 at 08:55:11PM +0530, Ranjan Kumar wrote:
> ...

> > >  static int __maybe_unused
> > >  mpi3mr_suspend(struct device *dev)
> > >  {
> > >       struct pci_dev *pdev = to_pci_dev(dev);
> > > -     struct Scsi_Host *shost = pci_get_drvdata(pdev);
> > > +     struct Scsi_Host *shost;
> > >       struct mpi3mr_ioc *mrioc;
> > >
> > > -     if (!shost)
> > > -             return 0;
> > > +     if (mpi3mr_get_shost_and_mrioc(pdev, &shost, &mrioc))
> > > +             return -1;
> >
> > Is -1 really the best return value here?  It seems like usually a
> > negative errno is returned.
>
> The __suspend_report_result just prints the return value, so i thought
> -1 is fine as we already print a error message. Do you recommend to
> use another negative (non -1) number for differentiation between
> generic error and this?

In addition to being printed by __suspend_report_result() (if the
return value is non-zero), the return value is returned up the chain
by legacy_suspend(), pci_pm_suspend(), etc.

I would probably make mpi3mr_get_shost_and_mrioc() return an errno
like -ENODEV and then have mpi3mr_suspend() capture that and return
it.

These function pointers go in struct dev_pm_ops, and these functions
are documented as returning "error codes", which I interpret as errno
values:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/pm.h?id=v6.6#n258

Bjorn