[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <45a483ff097220ca4bc534ada3c7c5d5fca5b19e.camel@linux.intel.com>
Date: Sun, 18 Sep 2022 23:54:55 -0700
From: srinivas pandruvada <srinivas.pandruvada@...ux.intel.com>
To: Daniel Lezcano <daniel.lezcano@...aro.org>, rafael@...nel.org
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
On Fri, 2022-09-16 at 19:06 +0200, Daniel Lezcano wrote:
>
> 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>
Reviewed-by: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
> > ---
> > 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;
> > }
> >
>
>
Powered by blists - more mailing lists