Re: [RFC/RFT PATCH] PM: sleep: Ignore device driver suspend() callback return values

[Ulf Hansson]

On Thu, 5 Dec 2024 at 16:09, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Thu, Dec 05, 2024 at 12:55:08PM +0100, Rafael J. Wysocki wrote:
> > Expanded CC list.
> >
> > On Thu, Nov 14, 2024 at 4:23 AM Len Brown <lenb@xxxxxxxxxx> wrote:
> > >
> > > From: Len Brown <len.brown@xxxxxxxxx>
> > >
> > > Drivers commonly return non-zero values from their suspend
> > > callbacks due to transient errors, not realizing that doing so
> > > aborts system-wide suspend.
> > >
> > > Log, but do not abort system suspend on non-zero return values
> > > from driver's .suspend/.suspend_noirq/.suspend_late callbacks.
> > >
> > > Both before and after this patch, the correct method for a
> > > device driver to abort system-wide suspend is to invoke
> > > pm_system_wakeup() during the suspend flow.
> > >
> > > Legacy behaviour can be restored by adding this line to your .config:
> > > CONFIG_PM_SLEEP_LEGACY_CALLBACK_ABORT=y
> > >
> > > Signed-off-by: Len Brown <len.brown@xxxxxxxxx>
> > > ---
>
> >
> > I'm wondering if there are any opinions on this.
> >
> > IMV, drivers returning errors from their suspend callbacks without a
> > sufficiently serious reason are kind of a problem.
>
> There is a least one driver whose suspend callback returns an error if
> the device is enabled for wakeup and a wakeup event occurs during the
> suspend procedure.  We don't want to ignore those races.
>
> Alan Stern

Right. I also think this looks a bit risky as the current behaviour
has really been there for a long time. Who knows what depends on this.

A way forward could be to implement the change as an opt-in thing,
rather than an opt-out. That would allow us to test it and see how it
plays to potentially change the default behaviour down the road.

Kind regards
Uffe