Re: [PATCH v1 1/3] PM: runtime: Introduce PM_RUNTIME_ACQUIRE_OR_FAIL() macro

[<dan.j.williams@intel.com>]

Rafael J. Wysocki wrote:
> On Thu, Oct 16, 2025 at 9:45 PM <dan.j.williams@intel.com> wrote:
> >
> > Rafael J. Wysocki wrote:
> > [..]
> > > > > [1]: https://lore.kernel.org/all/CAHk-=whn07tnDosPfn+UcAtWHBcLg=KqA16SHVv0GV4t8P1fHw@mail.gmail.com/
> > > >
> > > > Yeah, I myself also find it suboptimal, hence it wasn't really
> > > > proposed...  It's a limit of macro, unfortunately.
> > >
> > > The macro from the $subject patch can be split along the lines of the appended
> > > patch to avoid the "disgusting syntax" issue, although it then becomes less
> > > attractive as far as I'm concerned.  It still allows the details unrelated to
> > > the rest of the code to be hidden though.
> > >
> > > ---
> > >  drivers/acpi/acpi_tad.c |   10 ++++++++--
> > >  1 file changed, 8 insertions(+), 2 deletions(-)
> > >
> > > --- a/drivers/acpi/acpi_tad.c
> > > +++ b/drivers/acpi/acpi_tad.c
> > > @@ -31,6 +31,12 @@ MODULE_DESCRIPTION("ACPI Time and Alarm
> > >  MODULE_LICENSE("GPL v2");
> > >  MODULE_AUTHOR("Rafael J. Wysocki");
> > >
> > > +#define PM_RUNTIME_ACQUIRE_ACTIVE(dev)       \
> > > +     ACQUIRE(pm_runtime_active_try, pm_runtime_active_guard_var)(dev)
> > > +
> > > +#define PM_RUNTIME_ACQUIRE_ACTIVE_ERR        \
> > > +     ACQUIRE_ERR(pm_runtime_active_try, &pm_runtime_active_guard_var)
> > > +
> > >  /* ACPI TAD capability flags (ACPI 6.2, Section 9.18.2) */
> > >  #define ACPI_TAD_AC_WAKE     BIT(0)
> > >  #define ACPI_TAD_DC_WAKE     BIT(1)
> > > @@ -264,8 +270,8 @@ static int acpi_tad_wake_set(struct devi
> > >       args[0].integer.value = timer_id;
> > >       args[1].integer.value = value;
> > >
> > > -     ACQUIRE(pm_runtime_active_try, pm)(dev);
> > > -     if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
> > > +     PM_RUNTIME_ACQUIRE_ACTIVE(dev);
> > > +     if (PM_RUNTIME_ACQUIRE_ACTIVE_ERR)
> > >               return -ENXIO;
> >
> > This defeats one of the other motivations for ACQUIRE() vs
> > scoped_cond_guard() in that it drops the error code from
> > pm_runtime_active_try.
> 
> No, it doesn't.  PM_RUNTIME_ACQUIRE_ACTIVE_ERR is that error code.  Or
> did I misunderstand what you said?

Oh, what I am saying is that pm_runtime_get_active() returns a distinct
error code like -EACCES or -EINPROGRESS etc. The
PM_RUNTIME_ACQUIRE_ACTIVE_ERR proposal ignores that value and open codes
returning -ENXIO.

> > Maybe it is the case that failure is always -ENXIO, but from a
> > future code evolution standpoint do you want to commit to always
> > translating _try errors to a local error code?
> 
> No, I don't.
> 
> > Btw, was acpi_tad_wake_set() buggy previously for ignoring
> > pm_runtime_get_sync() errors, or is it a regression risk now for
> > honoring errors?
> 
> You may call it buggy strictly speaking, but it just assumed that if
> the runtime resume failed, the subsequent operation would just fail
> either, so -EIO would be returned to the caller.
> 
> This change allows distinguishing resume errors from I/O errors.

Ah, ok, makes sense.