| Message ID | 1831773.TLkxdtWsSY@rjwysocki.net (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | thermal: Rework binding cooling devices to trip points | expand |
On Mon, 2024-08-19 at 18:33 +0200, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Make thermal_bind_cdev_to_trip() take a struct cooling_spec pointer > to reduce the number of its arguments, change the return type of > thermal_unbind_cdev_from_trip() to void and rearrange the code in > thermal_zone_cdev_binding() to reduce the indentation level. > > No intentional functional impact. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Reviewed-by: Zhang Rui <rui.zhang@intel.com> thanks, rui > --- > > v2 -> v3: Subject fix > > v1-> v2: No changes > > --- > drivers/thermal/thermal_core.c | 54 +++++++++++++++--------------- > ----------- > 1 file changed, 21 insertions(+), 33 deletions(-) > > Index: linux-pm/drivers/thermal/thermal_core.c > =================================================================== > --- linux-pm.orig/drivers/thermal/thermal_core.c > +++ linux-pm/drivers/thermal/thermal_core.c > @@ -757,15 +757,7 @@ struct thermal_zone_device *thermal_zone > * @tz: pointer to struct thermal_zone_device > * @trip: trip point the cooling devices is associated with in > this zone. > * @cdev: pointer to struct thermal_cooling_device > - * @upper: the Maximum cooling state for this trip point. > - * THERMAL_NO_LIMIT means no upper limit, > - * and the cooling device can be in max_state. > - * @lower: the Minimum cooling state can be used for this trip > point. > - * THERMAL_NO_LIMIT means no lower limit, > - * and the cooling device can be in cooling state 0. > - * @weight: The weight of the cooling device to be bound to the > - * thermal zone. Use THERMAL_WEIGHT_DEFAULT for the > - * default value > + * @c: cooling specification for @trip and @cdev > * > * This interface function bind a thermal cooling device to the > certain trip > * point of a thermal zone device. > @@ -776,8 +768,7 @@ struct thermal_zone_device *thermal_zone > static int thermal_bind_cdev_to_trip(struct thermal_zone_device *tz, > const struct thermal_trip *trip, > struct thermal_cooling_device > *cdev, > - unsigned long upper, unsigned > long lower, > - unsigned int weight) > + struct cooling_spec *c) > { > struct thermal_instance *dev; > struct thermal_instance *pos; > @@ -791,17 +782,17 @@ static int thermal_bind_cdev_to_trip(str > return -EINVAL; > > /* lower default 0, upper default max_state */ > - if (lower == THERMAL_NO_LIMIT) > - lower = 0; > + if (c->lower == THERMAL_NO_LIMIT) > + c->lower = 0; > > - if (upper == THERMAL_NO_LIMIT) { > - upper = cdev->max_state; > + if (c->upper == THERMAL_NO_LIMIT) { > + c->upper = cdev->max_state; > upper_no_limit = true; > } else { > upper_no_limit = false; > } > > - if (lower > upper || upper > cdev->max_state) > + if (c->lower > c->upper || c->upper > cdev->max_state) > return -EINVAL; > > dev = kzalloc(sizeof(*dev), GFP_KERNEL); > @@ -810,11 +801,11 @@ static int thermal_bind_cdev_to_trip(str > dev->tz = tz; > dev->cdev = cdev; > dev->trip = trip; > - dev->upper = upper; > + dev->upper = c->upper; > dev->upper_no_limit = upper_no_limit; > - dev->lower = lower; > + dev->lower = c->lower; > dev->target = THERMAL_NO_TARGET; > - dev->weight = weight; > + dev->weight = c->weight; > > result = ida_alloc(&tz->ida, GFP_KERNEL); > if (result < 0) > @@ -887,12 +878,10 @@ free_mem: > * This interface function unbind a thermal cooling device from the > certain > * trip point of a thermal zone device. > * This function is usually called in the thermal zone device > .unbind callback. > - * > - * Return: 0 on success, the proper error value otherwise. > */ > -static int thermal_unbind_cdev_from_trip(struct thermal_zone_device > *tz, > - const struct thermal_trip > *trip, > - struct > thermal_cooling_device *cdev) > +static void thermal_unbind_cdev_from_trip(struct thermal_zone_device > *tz, > + const struct thermal_trip > *trip, > + struct > thermal_cooling_device *cdev) > { > struct thermal_instance *pos, *next; > > @@ -912,7 +901,7 @@ static int thermal_unbind_cdev_from_trip > } > mutex_unlock(&cdev->lock); > > - return -ENODEV; > + return; > > unbind: > device_remove_file(&tz->device, &pos->weight_attr); > @@ -920,7 +909,6 @@ unbind: > sysfs_remove_link(&tz->device.kobj, pos->name); > ida_free(&tz->ida, pos->id); > kfree(pos); > - return 0; > } > > static void thermal_release(struct device *dev) > @@ -959,7 +947,6 @@ static void thermal_zone_cdev_binding(st > struct thermal_cooling_device > *cdev) > { > struct thermal_trip_desc *td; > - int ret; > > if (!tz->ops.should_bind) > return; > @@ -973,13 +960,14 @@ static void thermal_zone_cdev_binding(st > .lower = THERMAL_NO_LIMIT, > .weight = THERMAL_WEIGHT_DEFAULT > }; > + int ret; > > - if (tz->ops.should_bind(tz, trip, cdev, &c)) { > - ret = thermal_bind_cdev_to_trip(tz, trip, > cdev, c.upper, > - c.lower, > c.weight); > - if (ret) > - print_bind_err_msg(tz, trip, cdev, > ret); > - } > + if (!tz->ops.should_bind(tz, trip, cdev, &c)) > + continue; > + > + ret = thermal_bind_cdev_to_trip(tz, trip, cdev, &c); > + if (ret) > + print_bind_err_msg(tz, trip, cdev, ret); > } > > mutex_unlock(&tz->lock); > > >
在 2024/8/20 0:33, Rafael J. Wysocki 写道: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Make thermal_bind_cdev_to_trip() take a struct cooling_spec pointer > to reduce the number of its arguments, change the return type of > thermal_unbind_cdev_from_trip() to void and rearrange the code in > thermal_zone_cdev_binding() to reduce the indentation level. > > No intentional functional impact. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- Acked-by: Huisong Li <lihuisong@huawei.com> > > > > .
On 19/08/2024 18:33, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Make thermal_bind_cdev_to_trip() take a struct cooling_spec pointer > to reduce the number of its arguments, change the return type of > thermal_unbind_cdev_from_trip() to void and rearrange the code in > thermal_zone_cdev_binding() to reduce the indentation level. > > No intentional functional impact. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > > v2 -> v3: Subject fix > > v1-> v2: No changes > > --- > drivers/thermal/thermal_core.c | 54 +++++++++++++++-------------------------- > 1 file changed, 21 insertions(+), 33 deletions(-) > > Index: linux-pm/drivers/thermal/thermal_core.c > =================================================================== > --- linux-pm.orig/drivers/thermal/thermal_core.c > +++ linux-pm/drivers/thermal/thermal_core.c > @@ -757,15 +757,7 @@ struct thermal_zone_device *thermal_zone > * @tz: pointer to struct thermal_zone_device > * @trip: trip point the cooling devices is associated with in this zone. > * @cdev: pointer to struct thermal_cooling_device > - * @upper: the Maximum cooling state for this trip point. > - * THERMAL_NO_LIMIT means no upper limit, > - * and the cooling device can be in max_state. > - * @lower: the Minimum cooling state can be used for this trip point. > - * THERMAL_NO_LIMIT means no lower limit, > - * and the cooling device can be in cooling state 0. > - * @weight: The weight of the cooling device to be bound to the > - * thermal zone. Use THERMAL_WEIGHT_DEFAULT for the > - * default value > + * @c: cooling specification for @trip and @cdev s/c/cspec/ at least :) Other than that Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
On Wed, Aug 21, 2024 at 4:29 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > On 19/08/2024 18:33, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > Make thermal_bind_cdev_to_trip() take a struct cooling_spec pointer > > to reduce the number of its arguments, change the return type of > > thermal_unbind_cdev_from_trip() to void and rearrange the code in > > thermal_zone_cdev_binding() to reduce the indentation level. > > > > No intentional functional impact. > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > --- > > > > v2 -> v3: Subject fix > > > > v1-> v2: No changes > > > > --- > > drivers/thermal/thermal_core.c | 54 +++++++++++++++-------------------------- > > 1 file changed, 21 insertions(+), 33 deletions(-) > > > > Index: linux-pm/drivers/thermal/thermal_core.c > > =================================================================== > > --- linux-pm.orig/drivers/thermal/thermal_core.c > > +++ linux-pm/drivers/thermal/thermal_core.c > > @@ -757,15 +757,7 @@ struct thermal_zone_device *thermal_zone > > * @tz: pointer to struct thermal_zone_device > > * @trip: trip point the cooling devices is associated with in this zone. > > * @cdev: pointer to struct thermal_cooling_device > > - * @upper: the Maximum cooling state for this trip point. > > - * THERMAL_NO_LIMIT means no upper limit, > > - * and the cooling device can be in max_state. > > - * @lower: the Minimum cooling state can be used for this trip point. > > - * THERMAL_NO_LIMIT means no lower limit, > > - * and the cooling device can be in cooling state 0. > > - * @weight: The weight of the cooling device to be bound to the > > - * thermal zone. Use THERMAL_WEIGHT_DEFAULT for the > > - * default value > > + * @c: cooling specification for @trip and @cdev > > s/c/cspec/ at least :) I have settled on cool_spec here. > Other than that > > Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org> Thank you for all of the reviews!
Index: linux-pm/drivers/thermal/thermal_core.c =================================================================== --- linux-pm.orig/drivers/thermal/thermal_core.c +++ linux-pm/drivers/thermal/thermal_core.c @@ -757,15 +757,7 @@ struct thermal_zone_device *thermal_zone * @tz: pointer to struct thermal_zone_device * @trip: trip point the cooling devices is associated with in this zone. * @cdev: pointer to struct thermal_cooling_device - * @upper: the Maximum cooling state for this trip point. - * THERMAL_NO_LIMIT means no upper limit, - * and the cooling device can be in max_state. - * @lower: the Minimum cooling state can be used for this trip point. - * THERMAL_NO_LIMIT means no lower limit, - * and the cooling device can be in cooling state 0. - * @weight: The weight of the cooling device to be bound to the - * thermal zone. Use THERMAL_WEIGHT_DEFAULT for the - * default value + * @c: cooling specification for @trip and @cdev * * This interface function bind a thermal cooling device to the certain trip * point of a thermal zone device. @@ -776,8 +768,7 @@ struct thermal_zone_device *thermal_zone static int thermal_bind_cdev_to_trip(struct thermal_zone_device *tz, const struct thermal_trip *trip, struct thermal_cooling_device *cdev, - unsigned long upper, unsigned long lower, - unsigned int weight) + struct cooling_spec *c) { struct thermal_instance *dev; struct thermal_instance *pos; @@ -791,17 +782,17 @@ static int thermal_bind_cdev_to_trip(str return -EINVAL; /* lower default 0, upper default max_state */ - if (lower == THERMAL_NO_LIMIT) - lower = 0; + if (c->lower == THERMAL_NO_LIMIT) + c->lower = 0; - if (upper == THERMAL_NO_LIMIT) { - upper = cdev->max_state; + if (c->upper == THERMAL_NO_LIMIT) { + c->upper = cdev->max_state; upper_no_limit = true; } else { upper_no_limit = false; } - if (lower > upper || upper > cdev->max_state) + if (c->lower > c->upper || c->upper > cdev->max_state) return -EINVAL; dev = kzalloc(sizeof(*dev), GFP_KERNEL); @@ -810,11 +801,11 @@ static int thermal_bind_cdev_to_trip(str dev->tz = tz; dev->cdev = cdev; dev->trip = trip; - dev->upper = upper; + dev->upper = c->upper; dev->upper_no_limit = upper_no_limit; - dev->lower = lower; + dev->lower = c->lower; dev->target = THERMAL_NO_TARGET; - dev->weight = weight; + dev->weight = c->weight; result = ida_alloc(&tz->ida, GFP_KERNEL); if (result < 0) @@ -887,12 +878,10 @@ free_mem: * This interface function unbind a thermal cooling device from the certain * trip point of a thermal zone device. * This function is usually called in the thermal zone device .unbind callback. - * - * Return: 0 on success, the proper error value otherwise. */ -static int thermal_unbind_cdev_from_trip(struct thermal_zone_device *tz, - const struct thermal_trip *trip, - struct thermal_cooling_device *cdev) +static void thermal_unbind_cdev_from_trip(struct thermal_zone_device *tz, + const struct thermal_trip *trip, + struct thermal_cooling_device *cdev) { struct thermal_instance *pos, *next; @@ -912,7 +901,7 @@ static int thermal_unbind_cdev_from_trip } mutex_unlock(&cdev->lock); - return -ENODEV; + return; unbind: device_remove_file(&tz->device, &pos->weight_attr); @@ -920,7 +909,6 @@ unbind: sysfs_remove_link(&tz->device.kobj, pos->name); ida_free(&tz->ida, pos->id); kfree(pos); - return 0; } static void thermal_release(struct device *dev) @@ -959,7 +947,6 @@ static void thermal_zone_cdev_binding(st struct thermal_cooling_device *cdev) { struct thermal_trip_desc *td; - int ret; if (!tz->ops.should_bind) return; @@ -973,13 +960,14 @@ static void thermal_zone_cdev_binding(st .lower = THERMAL_NO_LIMIT, .weight = THERMAL_WEIGHT_DEFAULT }; + int ret; - if (tz->ops.should_bind(tz, trip, cdev, &c)) { - ret = thermal_bind_cdev_to_trip(tz, trip, cdev, c.upper, - c.lower, c.weight); - if (ret) - print_bind_err_msg(tz, trip, cdev, ret); - } + if (!tz->ops.should_bind(tz, trip, cdev, &c)) + continue; + + ret = thermal_bind_cdev_to_trip(tz, trip, cdev, &c); + if (ret) + print_bind_err_msg(tz, trip, cdev, ret); } mutex_unlock(&tz->lock);