From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Use guard pm_runtime_active_try to simplify runtime PM cleanup and
implement runtime resume error handling in multiple places.

Also use guard pm_runtime_noresume to simplify acpi_tad_remove().

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_tad.c |   57 +++++++++++++++++++++---------------------------
 1 file changed, 25 insertions(+), 32 deletions(-)

--- a/drivers/acpi/acpi_tad.c
+++ b/drivers/acpi/acpi_tad.c
@@ -90,12 +90,11 @@ static int acpi_tad_set_real_time(struct
 	args[0].buffer.pointer = (u8 *)rt;
 	args[0].buffer.length = sizeof(*rt);
 
-	pm_runtime_get_sync(dev);
+	ACQUIRE(pm_runtime_active_try, pm)(dev);
+	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
+		return -ENXIO;
 
 	status = acpi_evaluate_integer(handle, "_SRT", &arg_list, &retval);
-
-	pm_runtime_put_sync(dev);
-
 	if (ACPI_FAILURE(status) || retval)
 		return -EIO;
 
@@ -111,12 +110,11 @@ static int acpi_tad_get_real_time(struct
 	acpi_status status;
 	int ret = -EIO;
 
-	pm_runtime_get_sync(dev);
+	ACQUIRE(pm_runtime_active_try, pm)(dev);
+	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
+		return -ENXIO;
 
 	status = acpi_evaluate_object(handle, "_GRT", NULL, &output);
-
-	pm_runtime_put_sync(dev);
-
 	if (ACPI_FAILURE(status))
 		goto out_free;
 
@@ -266,12 +264,11 @@ static int acpi_tad_wake_set(struct devi
 	args[0].integer.value = timer_id;
 	args[1].integer.value = value;
 
-	pm_runtime_get_sync(dev);
+	ACQUIRE(pm_runtime_active_try, pm)(dev);
+	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
+		return -ENXIO;
 
 	status = acpi_evaluate_integer(handle, method, &arg_list, &retval);
-
-	pm_runtime_put_sync(dev);
-
 	if (ACPI_FAILURE(status) || retval)
 		return -EIO;
 
@@ -314,12 +311,11 @@ static ssize_t acpi_tad_wake_read(struct
 
 	args[0].integer.value = timer_id;
 
-	pm_runtime_get_sync(dev);
+	ACQUIRE(pm_runtime_active_try, pm)(dev);
+	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
+		return -ENXIO;
 
 	status = acpi_evaluate_integer(handle, method, &arg_list, &retval);
-
-	pm_runtime_put_sync(dev);
-
 	if (ACPI_FAILURE(status))
 		return -EIO;
 
@@ -370,12 +366,11 @@ static int acpi_tad_clear_status(struct
 
 	args[0].integer.value = timer_id;
 
-	pm_runtime_get_sync(dev);
+	ACQUIRE(pm_runtime_active_try, pm)(dev);
+	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
+		return -ENXIO;
 
 	status = acpi_evaluate_integer(handle, "_CWS", &arg_list, &retval);
-
-	pm_runtime_put_sync(dev);
-
 	if (ACPI_FAILURE(status) || retval)
 		return -EIO;
 
@@ -411,12 +406,11 @@ static ssize_t acpi_tad_status_read(stru
 
 	args[0].integer.value = timer_id;
 
-	pm_runtime_get_sync(dev);
+	ACQUIRE(pm_runtime_active_try, pm)(dev);
+	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
+		return -ENXIO;
 
 	status = acpi_evaluate_integer(handle, "_GWS", &arg_list, &retval);
-
-	pm_runtime_put_sync(dev);
-
 	if (ACPI_FAILURE(status))
 		return -EIO;
 
@@ -571,16 +565,15 @@ static void acpi_tad_remove(struct platf
 
 	sysfs_remove_group(&dev->kobj, &acpi_tad_attr_group);
 
-	pm_runtime_get_noresume(dev);
-
-	acpi_tad_disable_timer(dev, ACPI_TAD_AC_TIMER);
-	acpi_tad_clear_status(dev, ACPI_TAD_AC_TIMER);
-	if (dd->capabilities & ACPI_TAD_DC_WAKE) {
-		acpi_tad_disable_timer(dev, ACPI_TAD_DC_TIMER);
-		acpi_tad_clear_status(dev, ACPI_TAD_DC_TIMER);
+	scoped_guard(pm_runtime_noresume, dev) {
+		acpi_tad_disable_timer(dev, ACPI_TAD_AC_TIMER);
+		acpi_tad_clear_status(dev, ACPI_TAD_AC_TIMER);
+		if (dd->capabilities & ACPI_TAD_DC_WAKE) {
+			acpi_tad_disable_timer(dev, ACPI_TAD_DC_TIMER);
+			acpi_tad_clear_status(dev, ACPI_TAD_DC_TIMER);
+		}
 	}
 
-	pm_runtime_put_noidle(dev);
 	pm_runtime_suspend(dev);
 	pm_runtime_disable(dev);
 	acpi_remove_cmos_rtc_space_handler(handle);

On Sat, 18 Oct 2025 14:24:42 +0200
"Rafael J. Wysocki" <rafael@kernel.org> wrote:

> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Use guard pm_runtime_active_try to simplify runtime PM cleanup and
> implement runtime resume error handling in multiple places.
> 
> Also use guard pm_runtime_noresume to simplify acpi_tad_remove().
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/acpi/acpi_tad.c |   57 +++++++++++++++++++++---------------------------
>  1 file changed, 25 insertions(+), 32 deletions(-)
> 
> --- a/drivers/acpi/acpi_tad.c
> +++ b/drivers/acpi/acpi_tad.c

> @@ -111,12 +110,11 @@ static int acpi_tad_get_real_time(struct
>  	acpi_status status;
>  	int ret = -EIO;
>  
> -	pm_runtime_get_sync(dev);
> +	ACQUIRE(pm_runtime_active_try, pm)(dev);
> +	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
> +		return -ENXIO;
>  
>  	status = acpi_evaluate_object(handle, "_GRT", NULL, &output);
> -
> -	pm_runtime_put_sync(dev);
> -
>  	if (ACPI_FAILURE(status))

Whilst it isn't actually a bug, this does run up against the guidance
in cleanup.h to avoid mixing gotos and cleanup.h usage in a single function.
That's partly a simplification to avoid having to explain the issues with
jumping past inline declarations.

If you want to follow that guidance, either you'd need to add a helper along the
lines of:

DEFINE_FREE(acpi_buffer, void *m ACPI_FREE(_T));
void *acpi_eval_grt(acpi_handle handle, struct acpi_buffer *output)
{
	acpi_status status = acpi_evaluate_object(handle, "_GRT", NULL, &output);

	if (ACPI_FAILURE(status)) {
		ACPI_FREE(output.pointer);
		return ERR_PTR(-EIO); //whatever error makse sense.
	}

	return output.pointer;
}

void *out_obj __free(acpi_buffer) = acpi_eval_grt(handle, &output);

Then can return directly at all error paths.  Not the nicest bit of
code though.

Or, factor out everthing after that allocation down
to the label as helper function and have direct returns in that.

or leave it all as is and hope Linus doesn't get grumpy about mix
and match (which lead to that guidance being so general in the first place!)

The rest look good to me.

Jonathan



>  		goto out_free;
>