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@xxxxxxxxxx> 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
