diff mbox series

[v1,2/6] thermal: ACPI: Discard trip table after zone registration

Message ID 4905992.31r3eYUQgx@kreacher (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series thermal: Store trips table and ops in thermal_zone_device | expand

Commit Message

Rafael J. Wysocki Feb. 5, 2024, 9:15 p.m. UTC 
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Because the thermal core creates and uses its own copy of the trips
table passed to thermal_zone_device_register_with_trips(), it is not
necessary to hold on to a local copy of it any more after the given
thermal zone has been registered.

Accordingly, modify the ACPI thermal driver to store the trips table
passed to thermal_zone_device_register_with_trips() in a local variable
which is automatically discarded after the zone registration.

Also make some additional code simplifications unlocked by the above
change.

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

Comments

Stanislaw Gruszka Feb. 8, 2024, 8:22 a.m. UTC | #1 
On Mon, Feb 05, 2024 at 10:15:50PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Because the thermal core creates and uses its own copy of the trips
> table passed to thermal_zone_device_register_with_trips(), it is not
> necessary to hold on to a local copy of it any more after the given
> thermal zone has been registered.
> 
> Accordingly, modify the ACPI thermal driver to store the trips table
> passed to thermal_zone_device_register_with_trips() in a local variable
> which is automatically discarded after the zone registration.
> 
> Also make some additional code simplifications unlocked by the above
> change.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>

> ---
>  drivers/acpi/thermal.c |   57 +++++++++++++++++--------------------------------
>  1 file changed, 20 insertions(+), 37 deletions(-)
> 
> Index: linux-pm/drivers/acpi/thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/thermal.c
> +++ linux-pm/drivers/acpi/thermal.c
> @@ -47,6 +47,8 @@
>  
>  #define ACPI_THERMAL_TRIP_PASSIVE	(-1)
>  
> +#define ACPI_THERMAL_MAX_NR_TRIPS	(ACPI_THERMAL_MAX_ACTIVE + 3)
> +
>  /*
>   * This exception is thrown out in two cases:
>   * 1.An invalid trip point becomes invalid or a valid trip point becomes invalid
> @@ -112,7 +114,6 @@ struct acpi_thermal {
>  	unsigned long polling_frequency;
>  	volatile u8 zombie;
>  	struct acpi_thermal_trips trips;
> -	struct thermal_trip *trip_table;
>  	struct thermal_zone_device *thermal_zone;
>  	int kelvin_offset;	/* in millidegrees */
>  	struct work_struct thermal_check_work;
> @@ -451,26 +452,19 @@ fail:
>  	return false;
>  }
>  
> -static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
> +static void acpi_thermal_get_trip_points(struct acpi_thermal *tz)
>  {
> -	unsigned int count = 0;
>  	int i;
>  
> -	if (acpi_thermal_init_trip(tz, ACPI_THERMAL_TRIP_PASSIVE))
> -		count++;
> +	acpi_thermal_init_trip(tz, ACPI_THERMAL_TRIP_PASSIVE);
>  
>  	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
> -		if (acpi_thermal_init_trip(tz, i))
> -			count++;
> -		else
> +		if (!acpi_thermal_init_trip(tz, i))
>  			break;
> -
>  	}
>  
>  	while (++i < ACPI_THERMAL_MAX_ACTIVE)
>  		tz->trips.active[i].trip.temp_dk = THERMAL_TEMP_INVALID;
> -
> -	return count;
>  }
>  
>  /* sys I/F for generic thermal sysfs support */
> @@ -662,13 +656,14 @@ static void acpi_thermal_zone_sysfs_remo
>  }
>  
>  static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz,
> +					      const struct thermal_trip *trip_table,
>  					      unsigned int trip_count,
>  					      int passive_delay)
>  {
>  	int result;
>  
>  	tz->thermal_zone = thermal_zone_device_register_with_trips("acpitz",
> -								   tz->trip_table,
> +								   trip_table,
>  								   trip_count,
>  								   0, tz,
>  								   &acpi_thermal_zone_ops,
> @@ -823,10 +818,10 @@ static void acpi_thermal_free_thermal_zo
>  
>  static int acpi_thermal_add(struct acpi_device *device)
>  {
> +	struct thermal_trip trip_table[ACPI_THERMAL_MAX_NR_TRIPS] = { 0 };
>  	struct acpi_thermal_trip *acpi_trip;
>  	struct thermal_trip *trip;
>  	struct acpi_thermal *tz;
> -	unsigned int trip_count;
>  	int crit_temp, hot_temp;
>  	int passive_delay = 0;
>  	int result;
> @@ -848,21 +843,10 @@ static int acpi_thermal_add(struct acpi_
>  	acpi_thermal_aml_dependency_fix(tz);
>  
>  	/* Get trip points [_CRT, _PSV, etc.] (required). */
> -	trip_count = acpi_thermal_get_trip_points(tz);
> +	acpi_thermal_get_trip_points(tz);
>  
>  	crit_temp = acpi_thermal_get_critical_trip(tz);
> -	if (crit_temp != THERMAL_TEMP_INVALID)
> -		trip_count++;
> -
>  	hot_temp = acpi_thermal_get_hot_trip(tz);
> -	if (hot_temp != THERMAL_TEMP_INVALID)
> -		trip_count++;
> -
> -	if (!trip_count) {
> -		pr_warn(FW_BUG "No valid trip points!\n");
> -		result = -ENODEV;
> -		goto free_memory;
> -	}
>  
>  	/* Get temperature [_TMP] (required). */
>  	result = acpi_thermal_get_temperature(tz);
> @@ -881,13 +865,7 @@ static int acpi_thermal_add(struct acpi_
>  
>  	acpi_thermal_guess_offset(tz, crit_temp);
>  
> -	trip = kcalloc(trip_count, sizeof(*trip), GFP_KERNEL);
> -	if (!trip) {
> -		result = -ENOMEM;
> -		goto free_memory;
> -	}
> -
> -	tz->trip_table = trip;
> +	trip = trip_table;
>  
>  	if (crit_temp != THERMAL_TEMP_INVALID) {
>  		trip->type = THERMAL_TRIP_CRITICAL;
> @@ -923,9 +901,17 @@ static int acpi_thermal_add(struct acpi_
>  		trip++;
>  	}
>  
> -	result = acpi_thermal_register_thermal_zone(tz, trip_count, passive_delay);
> +	if (trip == trip_table) {
> +		pr_warn(FW_BUG "No valid trip points!\n");
> +		result = -ENODEV;
> +		goto free_memory;
> +	}
> +
> +	result = acpi_thermal_register_thermal_zone(tz, trip_table,
> +						    trip - trip_table,
> +						    passive_delay);
>  	if (result)
> -		goto free_trips;
> +		goto free_memory;
>  
>  	refcount_set(&tz->thermal_check_count, 3);
>  	mutex_init(&tz->thermal_check_lock);
> @@ -944,8 +930,6 @@ static int acpi_thermal_add(struct acpi_
>  flush_wq:
>  	flush_workqueue(acpi_thermal_pm_queue);
>  	acpi_thermal_unregister_thermal_zone(tz);
> -free_trips:
> -	kfree(tz->trip_table);
>  free_memory:
>  	acpi_thermal_free_thermal_zone(tz);
>  
> @@ -966,7 +950,6 @@ static void acpi_thermal_remove(struct a
>  
>  	flush_workqueue(acpi_thermal_pm_queue);
>  	acpi_thermal_unregister_thermal_zone(tz);
> -	kfree(tz->trip_table);
>  	acpi_thermal_free_thermal_zone(tz);
>  }
>  
> 
> 
>
diff mbox series

Patch

Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -47,6 +47,8 @@ 
 
 #define ACPI_THERMAL_TRIP_PASSIVE	(-1)
 
+#define ACPI_THERMAL_MAX_NR_TRIPS	(ACPI_THERMAL_MAX_ACTIVE + 3)
+
 /*
  * This exception is thrown out in two cases:
  * 1.An invalid trip point becomes invalid or a valid trip point becomes invalid
@@ -112,7 +114,6 @@  struct acpi_thermal {
 	unsigned long polling_frequency;
 	volatile u8 zombie;
 	struct acpi_thermal_trips trips;
-	struct thermal_trip *trip_table;
 	struct thermal_zone_device *thermal_zone;
 	int kelvin_offset;	/* in millidegrees */
 	struct work_struct thermal_check_work;
@@ -451,26 +452,19 @@  fail:
 	return false;
 }
 
-static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
+static void acpi_thermal_get_trip_points(struct acpi_thermal *tz)
 {
-	unsigned int count = 0;
 	int i;
 
-	if (acpi_thermal_init_trip(tz, ACPI_THERMAL_TRIP_PASSIVE))
-		count++;
+	acpi_thermal_init_trip(tz, ACPI_THERMAL_TRIP_PASSIVE);
 
 	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
-		if (acpi_thermal_init_trip(tz, i))
-			count++;
-		else
+		if (!acpi_thermal_init_trip(tz, i))
 			break;
-
 	}
 
 	while (++i < ACPI_THERMAL_MAX_ACTIVE)
 		tz->trips.active[i].trip.temp_dk = THERMAL_TEMP_INVALID;
-
-	return count;
 }
 
 /* sys I/F for generic thermal sysfs support */
@@ -662,13 +656,14 @@  static void acpi_thermal_zone_sysfs_remo
 }
 
 static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz,
+					      const struct thermal_trip *trip_table,
 					      unsigned int trip_count,
 					      int passive_delay)
 {
 	int result;
 
 	tz->thermal_zone = thermal_zone_device_register_with_trips("acpitz",
-								   tz->trip_table,
+								   trip_table,
 								   trip_count,
 								   0, tz,
 								   &acpi_thermal_zone_ops,
@@ -823,10 +818,10 @@  static void acpi_thermal_free_thermal_zo
 
 static int acpi_thermal_add(struct acpi_device *device)
 {
+	struct thermal_trip trip_table[ACPI_THERMAL_MAX_NR_TRIPS] = { 0 };
 	struct acpi_thermal_trip *acpi_trip;
 	struct thermal_trip *trip;
 	struct acpi_thermal *tz;
-	unsigned int trip_count;
 	int crit_temp, hot_temp;
 	int passive_delay = 0;
 	int result;
@@ -848,21 +843,10 @@  static int acpi_thermal_add(struct acpi_
 	acpi_thermal_aml_dependency_fix(tz);
 
 	/* Get trip points [_CRT, _PSV, etc.] (required). */
-	trip_count = acpi_thermal_get_trip_points(tz);
+	acpi_thermal_get_trip_points(tz);
 
 	crit_temp = acpi_thermal_get_critical_trip(tz);
-	if (crit_temp != THERMAL_TEMP_INVALID)
-		trip_count++;
-
 	hot_temp = acpi_thermal_get_hot_trip(tz);
-	if (hot_temp != THERMAL_TEMP_INVALID)
-		trip_count++;
-
-	if (!trip_count) {
-		pr_warn(FW_BUG "No valid trip points!\n");
-		result = -ENODEV;
-		goto free_memory;
-	}
 
 	/* Get temperature [_TMP] (required). */
 	result = acpi_thermal_get_temperature(tz);
@@ -881,13 +865,7 @@  static int acpi_thermal_add(struct acpi_
 
 	acpi_thermal_guess_offset(tz, crit_temp);
 
-	trip = kcalloc(trip_count, sizeof(*trip), GFP_KERNEL);
-	if (!trip) {
-		result = -ENOMEM;
-		goto free_memory;
-	}
-
-	tz->trip_table = trip;
+	trip = trip_table;
 
 	if (crit_temp != THERMAL_TEMP_INVALID) {
 		trip->type = THERMAL_TRIP_CRITICAL;
@@ -923,9 +901,17 @@  static int acpi_thermal_add(struct acpi_
 		trip++;
 	}
 
-	result = acpi_thermal_register_thermal_zone(tz, trip_count, passive_delay);
+	if (trip == trip_table) {
+		pr_warn(FW_BUG "No valid trip points!\n");
+		result = -ENODEV;
+		goto free_memory;
+	}
+
+	result = acpi_thermal_register_thermal_zone(tz, trip_table,
+						    trip - trip_table,
+						    passive_delay);
 	if (result)
-		goto free_trips;
+		goto free_memory;
 
 	refcount_set(&tz->thermal_check_count, 3);
 	mutex_init(&tz->thermal_check_lock);
@@ -944,8 +930,6 @@  static int acpi_thermal_add(struct acpi_
 flush_wq:
 	flush_workqueue(acpi_thermal_pm_queue);
 	acpi_thermal_unregister_thermal_zone(tz);
-free_trips:
-	kfree(tz->trip_table);
 free_memory:
 	acpi_thermal_free_thermal_zone(tz);
 
@@ -966,7 +950,6 @@  static void acpi_thermal_remove(struct a
 
 	flush_workqueue(acpi_thermal_pm_queue);
 	acpi_thermal_unregister_thermal_zone(tz);
-	kfree(tz->trip_table);
 	acpi_thermal_free_thermal_zone(tz);
 }