[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0e986fbb-e76b-9663-26c2-b84d887b4c98@linaro.org>
Date: Fri, 16 Sep 2022 19:06:42 +0200
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: rafael@...nel.org,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
Cc: linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
Amit Kucheria <amitk@...nel.org>,
Zhang Rui <rui.zhang@...el.com>,
Jiang Jian <jiangjian@...rlc.com>
Subject: Re: [PATCH v3 30/30] thermal/drivers/intel: Use generic
thermal_zone_get_trip() function
Hi Srinivas,
I've Cc'ed you on this patch in case you have any comment
Thanks
-- D.
On 06/09/2022 18:47, Daniel Lezcano wrote:
> The thermal framework gives the possibility to register the trip
> points with the thermal zone. When that is done, no get_trip_* ops are
> needed and they can be removed.
>
> Convert ops content logic into generic trip points and register them with the
> thermal zone.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@...aro.org>
> ---
> drivers/thermal/intel/x86_pkg_temp_thermal.c | 120 ++++++++++---------
> 1 file changed, 66 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/thermal/intel/x86_pkg_temp_thermal.c b/drivers/thermal/intel/x86_pkg_temp_thermal.c
> index a0e234fce71a..e7c3b78d959c 100644
> --- a/drivers/thermal/intel/x86_pkg_temp_thermal.c
> +++ b/drivers/thermal/intel/x86_pkg_temp_thermal.c
> @@ -53,6 +53,7 @@ struct zone_device {
> u32 msr_pkg_therm_high;
> struct delayed_work work;
> struct thermal_zone_device *tzone;
> + struct thermal_trip *trips;
> struct cpumask cpumask;
> };
>
> @@ -138,40 +139,6 @@ static int sys_get_curr_temp(struct thermal_zone_device *tzd, int *temp)
> return -EINVAL;
> }
>
> -static int sys_get_trip_temp(struct thermal_zone_device *tzd,
> - int trip, int *temp)
> -{
> - struct zone_device *zonedev = tzd->devdata;
> - unsigned long thres_reg_value;
> - u32 mask, shift, eax, edx;
> - int ret;
> -
> - if (trip >= MAX_NUMBER_OF_TRIPS)
> - return -EINVAL;
> -
> - if (trip) {
> - mask = THERM_MASK_THRESHOLD1;
> - shift = THERM_SHIFT_THRESHOLD1;
> - } else {
> - mask = THERM_MASK_THRESHOLD0;
> - shift = THERM_SHIFT_THRESHOLD0;
> - }
> -
> - ret = rdmsr_on_cpu(zonedev->cpu, MSR_IA32_PACKAGE_THERM_INTERRUPT,
> - &eax, &edx);
> - if (ret < 0)
> - return ret;
> -
> - thres_reg_value = (eax & mask) >> shift;
> - if (thres_reg_value)
> - *temp = zonedev->tj_max - thres_reg_value * 1000;
> - else
> - *temp = THERMAL_TEMP_INVALID;
> - pr_debug("sys_get_trip_temp %d\n", *temp);
> -
> - return 0;
> -}
> -
> static int
> sys_set_trip_temp(struct thermal_zone_device *tzd, int trip, int temp)
> {
> @@ -212,18 +179,9 @@ sys_set_trip_temp(struct thermal_zone_device *tzd, int trip, int temp)
> l, h);
> }
>
> -static int sys_get_trip_type(struct thermal_zone_device *thermal, int trip,
> - enum thermal_trip_type *type)
> -{
> - *type = THERMAL_TRIP_PASSIVE;
> - return 0;
> -}
> -
> /* Thermal zone callback registry */
> static struct thermal_zone_device_ops tzone_ops = {
> .get_temp = sys_get_curr_temp,
> - .get_trip_temp = sys_get_trip_temp,
> - .get_trip_type = sys_get_trip_type,
> .set_trip_temp = sys_set_trip_temp,
> };
>
> @@ -328,6 +286,48 @@ static int pkg_thermal_notify(u64 msr_val)
> return 0;
> }
>
> +static struct thermal_trip *pkg_temp_thermal_trips_init(int cpu, int tj_max, int num_trips)
> +{
> + struct thermal_trip *trips;
> + unsigned long thres_reg_value;
> + u32 mask, shift, eax, edx;
> + int ret, i;
> +
> + trips = kzalloc(sizeof(*trips) * num_trips, GFP_KERNEL);
> + if (!trips)
> + return ERR_PTR(-ENOMEM);
> +
> + for (i = 0; i < num_trips; i++) {
> +
> + if (i) {
> + mask = THERM_MASK_THRESHOLD1;
> + shift = THERM_SHIFT_THRESHOLD1;
> + } else {
> + mask = THERM_MASK_THRESHOLD0;
> + shift = THERM_SHIFT_THRESHOLD0;
> + }
> +
> + ret = rdmsr_on_cpu(cpu, MSR_IA32_PACKAGE_THERM_INTERRUPT,
> + &eax, &edx);
> + if (ret < 0) {
> + kfree(trips);
> + return ERR_PTR(ret);
> + }
> +
> + thres_reg_value = (eax & mask) >> shift;
> +
> + trips[i].temperature = thres_reg_value ?
> + tj_max - thres_reg_value * 1000 : THERMAL_TEMP_INVALID;
> +
> + trips[i].type = THERMAL_TRIP_PASSIVE;
> +
> + pr_debug("%s: cpu=%d, trip=%d, temp=%d\n",
> + __func__, cpu, i, trips[i].temperature);
> + }
> +
> + return trips;
> +}
> +
> static int pkg_temp_thermal_device_add(unsigned int cpu)
> {
> int id = topology_logical_die_id(cpu);
> @@ -353,24 +353,27 @@ static int pkg_temp_thermal_device_add(unsigned int cpu)
> if (!zonedev)
> return -ENOMEM;
>
> + zonedev->trips = pkg_temp_thermal_trips_init(cpu, tj_max, thres_count);
> + if (IS_ERR(zonedev->trips)) {
> + err = PTR_ERR(zonedev->trips);
> + goto out_kfree_zonedev;
> + }
> +
> INIT_DELAYED_WORK(&zonedev->work, pkg_temp_thermal_threshold_work_fn);
> zonedev->cpu = cpu;
> zonedev->tj_max = tj_max;
> - zonedev->tzone = thermal_zone_device_register("x86_pkg_temp",
> - thres_count,
> + zonedev->tzone = thermal_zone_device_register_with_trips("x86_pkg_temp",
> + zonedev->trips, thres_count,
> (thres_count == MAX_NUMBER_OF_TRIPS) ? 0x03 : 0x01,
> zonedev, &tzone_ops, &pkg_temp_tz_params, 0, 0);
> if (IS_ERR(zonedev->tzone)) {
> err = PTR_ERR(zonedev->tzone);
> - kfree(zonedev);
> - return err;
> + goto out_kfree_trips;
> }
> err = thermal_zone_device_enable(zonedev->tzone);
> - if (err) {
> - thermal_zone_device_unregister(zonedev->tzone);
> - kfree(zonedev);
> - return err;
> - }
> + if (err)
> + goto out_unregister_tz;
> +
> /* Store MSR value for package thermal interrupt, to restore at exit */
> rdmsr(MSR_IA32_PACKAGE_THERM_INTERRUPT, zonedev->msr_pkg_therm_low,
> zonedev->msr_pkg_therm_high);
> @@ -379,7 +382,14 @@ static int pkg_temp_thermal_device_add(unsigned int cpu)
> raw_spin_lock_irq(&pkg_temp_lock);
> zones[id] = zonedev;
> raw_spin_unlock_irq(&pkg_temp_lock);
> - return 0;
> +
> +out_unregister_tz:
> + thermal_zone_device_unregister(zonedev->tzone);
> +out_kfree_trips:
> + kfree(zonedev->trips);
> +out_kfree_zonedev:
> + kfree(zonedev);
> + return err;
> }
>
> static int pkg_thermal_cpu_offline(unsigned int cpu)
> @@ -463,8 +473,10 @@ static int pkg_thermal_cpu_offline(unsigned int cpu)
> raw_spin_unlock_irq(&pkg_temp_lock);
>
> /* Final cleanup if this is the last cpu */
> - if (lastcpu)
> + if (lastcpu) {
> + kfree(zonedev->trips);
> kfree(zonedev);
> + }
> return 0;
> }
>
--
<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
Powered by blists - more mailing lists