diff mbox series

[v3,14/14] thermal: core: Clean up trip bind/unbind functions

Message ID 1831773.TLkxdtWsSY@rjwysocki.net (mailing list archive)
State Mainlined, archived
Headers show
Series thermal: Rework binding cooling devices to trip points | expand

Commit Message

Rafael J. Wysocki Aug. 19, 2024, 4:33 p.m. UTC
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(-)

Comments

Zhang Rui Aug. 20, 2024, 7:11 a.m. UTC | #1
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);
> 
> 
>
lihuisong (C) Aug. 21, 2024, 9:34 a.m. UTC | #2
在 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>

>
>
>
> .
Daniel Lezcano Aug. 21, 2024, 2:29 p.m. UTC | #3
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>
Rafael J. Wysocki Aug. 21, 2024, 4:21 p.m. UTC | #4
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!
diff mbox series

Patch

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);