Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Message-ID: <52e946a9-cfc6-3411-68c5-827bdd762af3@linaro.org>
Date:   Mon, 25 Sep 2023 18:29:40 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH v1 4/9] ACPI: thermal: Create and populate trip points
 table earlier
Content-Language: en-US
To:     "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Linux ACPI <linux-acpi@vger.kernel.org>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Linux PM <linux-pm@vger.kernel.org>,
        Zhang Rui <rui.zhang@intel.com>,
        Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
References: <5708760.DvuYhMxLoT@kreacher> <13346091.uLZWGnKmhe@kreacher>
From:   Daniel Lezcano <daniel.lezcano@linaro.org>
In-Reply-To: <13346091.uLZWGnKmhe@kreacher>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 12/09/2023 20:39, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Create and populate the driver's trip points table in acpi_thermal_add()
> so as to allow the its data structures to be simplified going forward.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

> ---
>   drivers/acpi/thermal.c |  105 ++++++++++++++++++++++++-------------------------
>   1 file changed, 52 insertions(+), 53 deletions(-)
> 
> Index: linux-pm/drivers/acpi/thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/thermal.c
> +++ linux-pm/drivers/acpi/thermal.c
> @@ -688,53 +688,10 @@ static void acpi_thermal_zone_sysfs_remo
>   }
>   
>   static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz,
> -					      unsigned int trip_count)
> +					      unsigned int trip_count,
> +					      int passive_delay)
>   {
> -	struct acpi_thermal_trip *acpi_trip;
> -	struct thermal_trip *trip;
> -	int passive_delay = 0;
>   	int result;
> -	int i;
> -
> -	trip = kcalloc(trip_count, sizeof(*trip), GFP_KERNEL);
> -	if (!trip)
> -		return -ENOMEM;
> -
> -	tz->trip_table = trip;
> -
> -	if (tz->trips.critical.valid) {
> -		trip->type = THERMAL_TRIP_CRITICAL;
> -		trip->temperature = acpi_thermal_temp(tz, tz->trips.critical.temperature);
> -		trip++;
> -	}
> -
> -	if (tz->trips.hot.valid) {
> -		trip->type = THERMAL_TRIP_HOT;
> -		trip->temperature = acpi_thermal_temp(tz, tz->trips.hot.temperature);
> -		trip++;
> -	}
> -
> -	acpi_trip = &tz->trips.passive.trip;
> -	if (acpi_trip->valid) {
> -		passive_delay = tz->trips.passive.tsp * 100;
> -
> -		trip->type = THERMAL_TRIP_PASSIVE;
> -		trip->temperature = acpi_thermal_temp(tz, acpi_trip->temperature);
> -		trip->priv = acpi_trip;
> -		trip++;
> -	}
> -
> -	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
> -		acpi_trip = &tz->trips.active[i].trip;
> -
> -		if (!acpi_trip->valid)
> -			break;
> -
> -		trip->type = THERMAL_TRIP_ACTIVE;
> -		trip->temperature = acpi_thermal_temp(tz, acpi_trip->temperature);
> -		trip->priv = acpi_trip;
> -		trip++;
> -	}
>   
>   	tz->thermal_zone = thermal_zone_device_register_with_trips("acpitz",
>   								   tz->trip_table,
> @@ -744,10 +701,8 @@ static int acpi_thermal_register_thermal
>   								   NULL,
>   								   passive_delay,
>   								   tz->polling_frequency * 100);
> -	if (IS_ERR(tz->thermal_zone)) {
> -		result = PTR_ERR(tz->thermal_zone);
> -		goto free_trip_table;
> -	}
> +	if (IS_ERR(tz->thermal_zone))
> +		return PTR_ERR(tz->thermal_zone);
>   
>   	result = acpi_thermal_zone_sysfs_add(tz);
>   	if (result)
> @@ -766,8 +721,6 @@ remove_links:
>   	acpi_thermal_zone_sysfs_remove(tz);
>   unregister_tzd:
>   	thermal_zone_device_unregister(tz->thermal_zone);
> -free_trip_table:
> -	kfree(tz->trip_table);
>   
>   	return result;
>   }
> @@ -886,9 +839,13 @@ static void acpi_thermal_check_fn(struct
>   
>   static int acpi_thermal_add(struct acpi_device *device)
>   {
> +	struct acpi_thermal_trip *acpi_trip;
> +	struct thermal_trip *trip;
>   	struct acpi_thermal *tz;
>   	unsigned int trip_count;
> +	int passive_delay = 0;
>   	int result;
> +	int i;
>   
>   	if (!device)
>   		return -EINVAL;
> @@ -930,9 +887,49 @@ static int acpi_thermal_add(struct acpi_
>   
>   	acpi_thermal_guess_offset(tz);
>   
> -	result = acpi_thermal_register_thermal_zone(tz, trip_count);
> +	trip = kcalloc(trip_count, sizeof(*trip), GFP_KERNEL);
> +	if (!trip)
> +		return -ENOMEM;
> +
> +	tz->trip_table = trip;
> +
> +	if (tz->trips.critical.valid) {
> +		trip->type = THERMAL_TRIP_CRITICAL;
> +		trip->temperature = acpi_thermal_temp(tz, tz->trips.critical.temperature);
> +		trip++;
> +	}
> +
> +	if (tz->trips.hot.valid) {
> +		trip->type = THERMAL_TRIP_HOT;
> +		trip->temperature = acpi_thermal_temp(tz, tz->trips.hot.temperature);
> +		trip++;
> +	}
> +
> +	acpi_trip = &tz->trips.passive.trip;
> +	if (acpi_trip->valid) {
> +		passive_delay = tz->trips.passive.tsp * 100;
> +
> +		trip->type = THERMAL_TRIP_PASSIVE;
> +		trip->temperature = acpi_thermal_temp(tz, acpi_trip->temperature);
> +		trip->priv = acpi_trip;
> +		trip++;
> +	}
> +
> +	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
> +		acpi_trip =  &tz->trips.active[i].trip;
> +
> +		if (!acpi_trip->valid)
> +			break;
> +
> +		trip->type = THERMAL_TRIP_ACTIVE;
> +		trip->temperature = acpi_thermal_temp(tz, acpi_trip->temperature);
> +		trip->priv = acpi_trip;
> +		trip++;
> +	}
> +
> +	result = acpi_thermal_register_thermal_zone(tz, trip_count, passive_delay);
>   	if (result)
> -		goto free_memory;
> +		goto free_trips;
>   
>   	refcount_set(&tz->thermal_check_count, 3);
>   	mutex_init(&tz->thermal_check_lock);
> @@ -951,6 +948,8 @@ 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:
>   	kfree(tz);
>   
> 
> 
> 

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog