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

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

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. 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?

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?