| Message ID | 20220906164720.330701-31-daniel.lezcano@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3,01/30] thermal/core: Add a generic thermal_zone_get_trip() function | expand |
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@linaro.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; > } >
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@linaro.org> Reviewed-by: Srinivas Pandruvada <srinivas.pandruvada@linux.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; > > } > > > >
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; }
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@linaro.org> --- drivers/thermal/intel/x86_pkg_temp_thermal.c | 120 ++++++++++--------- 1 file changed, 66 insertions(+), 54 deletions(-)