From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Add and use a special guard for cooling devices.

This allows quite a few error code paths to be simplified among
other things and brings in code size reduction for a good measure.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This is a new iteration of

https://lore.kernel.org/linux-pm/1890654.atdPhlSkOF@rjwysocki.net/

v1 -> v2: Rearrange cur_state_store()

---
 drivers/thermal/gov_power_allocator.c |   21 +++++++--------
 drivers/thermal/gov_step_wise.c       |    6 ++--
 drivers/thermal/thermal_core.c        |   17 +++---------
 drivers/thermal/thermal_debugfs.c     |   25 +++++++++++-------
 drivers/thermal/thermal_helpers.c     |   19 +++-----------
 drivers/thermal/thermal_sysfs.c       |   45 ++++++++++++----------------------
 include/linux/thermal.h               |    3 ++
 7 files changed, 57 insertions(+), 79 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -756,12 +756,10 @@ static int thermal_instance_add(struct t
 
 	list_add_tail(&new_instance->trip_node, &td->thermal_instances);
 
-	mutex_lock(&cdev->lock);
+	guard(cooling_dev)(cdev);
 
 	list_add_tail(&new_instance->cdev_node, &cdev->thermal_instances);
 
-	mutex_unlock(&cdev->lock);
-
 	return 0;
 }
 
@@ -870,11 +868,9 @@ static void thermal_instance_delete(stru
 {
 	list_del(&instance->trip_node);
 
-	mutex_lock(&instance->cdev->lock);
+	guard(cooling_dev)(instance->cdev);
 
 	list_del(&instance->cdev_node);
-
-	mutex_unlock(&instance->cdev->lock);
 }
 
 /**
@@ -1237,10 +1233,10 @@ void thermal_cooling_device_update(struc
 	 * Update under the cdev lock to prevent the state from being set beyond
 	 * the new limit concurrently.
 	 */
-	mutex_lock(&cdev->lock);
+	guard(cooling_dev)(cdev);
 
 	if (cdev->ops->get_max_state(cdev, &cdev->max_state))
-		goto unlock;
+		return;
 
 	thermal_cooling_device_stats_reinit(cdev);
 
@@ -1267,12 +1263,9 @@ void thermal_cooling_device_update(struc
 	}
 
 	if (cdev->ops->get_cur_state(cdev, &state) || state > cdev->max_state)
-		goto unlock;
+		return;
 
 	thermal_cooling_device_stats_update(cdev, state);
-
-unlock:
-	mutex_unlock(&cdev->lock);
 }
 EXPORT_SYMBOL_GPL(thermal_cooling_device_update);
 
Index: linux-pm/include/linux/thermal.h
===================================================================
--- linux-pm.orig/include/linux/thermal.h
+++ linux-pm/include/linux/thermal.h
@@ -137,6 +137,9 @@ struct thermal_cooling_device {
 #endif
 };
 
+DEFINE_GUARD(cooling_dev, struct thermal_cooling_device *, mutex_lock(&_T->lock),
+	     mutex_unlock(&_T->lock))
+
 /* Structure to define Thermal Zone parameters */
 struct thermal_zone_params {
 	const char *governor_name;
Index: linux-pm/drivers/thermal/thermal_sysfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_sysfs.c
+++ linux-pm/drivers/thermal/thermal_sysfs.c
@@ -544,14 +544,15 @@ cur_state_store(struct device *dev, stru
 	if (state > cdev->max_state)
 		return -EINVAL;
 
-	mutex_lock(&cdev->lock);
+	guard(cooling_dev)(cdev);
 
 	result = cdev->ops->set_cur_state(cdev, state);
-	if (!result)
-		thermal_cooling_device_stats_update(cdev, state);
+	if (result)
+		return result;
 
-	mutex_unlock(&cdev->lock);
-	return result ? result : count;
+	thermal_cooling_device_stats_update(cdev, state);
+
+	return count;
 }
 
 static struct device_attribute
@@ -625,21 +626,18 @@ static ssize_t total_trans_show(struct d
 {
 	struct thermal_cooling_device *cdev = to_cooling_device(dev);
 	struct cooling_dev_stats *stats;
-	int ret = 0;
+	int ret;
 
-	mutex_lock(&cdev->lock);
+	guard(cooling_dev)(cdev);
 
 	stats = cdev->stats;
 	if (!stats)
-		goto unlock;
+		return 0;
 
 	spin_lock(&stats->lock);
 	ret = sprintf(buf, "%u\n", stats->total_trans);
 	spin_unlock(&stats->lock);
 
-unlock:
-	mutex_unlock(&cdev->lock);
-
 	return ret;
 }
 
@@ -652,11 +650,11 @@ time_in_state_ms_show(struct device *dev
 	ssize_t len = 0;
 	int i;
 
-	mutex_lock(&cdev->lock);
+	guard(cooling_dev)(cdev);
 
 	stats = cdev->stats;
 	if (!stats)
-		goto unlock;
+		return 0;
 
 	spin_lock(&stats->lock);
 
@@ -668,9 +666,6 @@ time_in_state_ms_show(struct device *dev
 	}
 	spin_unlock(&stats->lock);
 
-unlock:
-	mutex_unlock(&cdev->lock);
-
 	return len;
 }
 
@@ -682,11 +677,11 @@ reset_store(struct device *dev, struct d
 	struct cooling_dev_stats *stats;
 	int i, states;
 
-	mutex_lock(&cdev->lock);
+	guard(cooling_dev)(cdev);
 
 	stats = cdev->stats;
 	if (!stats)
-		goto unlock;
+		return count;
 
 	states = cdev->max_state + 1;
 
@@ -702,9 +697,6 @@ reset_store(struct device *dev, struct d
 
 	spin_unlock(&stats->lock);
 
-unlock:
-	mutex_unlock(&cdev->lock);
-
 	return count;
 }
 
@@ -716,13 +708,11 @@ static ssize_t trans_table_show(struct d
 	ssize_t len = 0;
 	int i, j;
 
-	mutex_lock(&cdev->lock);
+	guard(cooling_dev)(cdev);
 
 	stats = cdev->stats;
-	if (!stats) {
-		len = -ENODATA;
-		goto unlock;
-	}
+	if (!stats)
+		return -ENODATA;
 
 	len += snprintf(buf + len, PAGE_SIZE - len, " From  :    To\n");
 	len += snprintf(buf + len, PAGE_SIZE - len, "       : ");
@@ -760,9 +750,6 @@ static ssize_t trans_table_show(struct d
 		len = -EFBIG;
 	}
 
-unlock:
-	mutex_unlock(&cdev->lock);
-
 	return len;
 }
 
Index: linux-pm/drivers/thermal/thermal_helpers.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_helpers.c
+++ linux-pm/drivers/thermal/thermal_helpers.c
@@ -58,17 +58,10 @@ bool thermal_trip_is_bound_to_cdev(struc
 				   const struct thermal_trip *trip,
 				   struct thermal_cooling_device *cdev)
 {
-	bool ret;
-
 	guard(thermal_zone)(tz);
+	guard(cooling_dev)(cdev);
 
-	mutex_lock(&cdev->lock);
-
-	ret = thermal_instance_present(tz, cdev, trip);
-
-	mutex_unlock(&cdev->lock);
-
-	return ret;
+	return thermal_instance_present(tz, cdev, trip);
 }
 EXPORT_SYMBOL_GPL(thermal_trip_is_bound_to_cdev);
 
@@ -197,12 +190,12 @@ void __thermal_cdev_update(struct therma
  */
 void thermal_cdev_update(struct thermal_cooling_device *cdev)
 {
-	mutex_lock(&cdev->lock);
+	guard(cooling_dev)(cdev);
+
 	if (!cdev->updated) {
 		__thermal_cdev_update(cdev);
 		cdev->updated = true;
 	}
-	mutex_unlock(&cdev->lock);
 }
 
 /**
@@ -211,11 +204,9 @@ void thermal_cdev_update(struct thermal_
  */
 void thermal_cdev_update_nocheck(struct thermal_cooling_device *cdev)
 {
-	mutex_lock(&cdev->lock);
+	guard(cooling_dev)(cdev);
 
 	__thermal_cdev_update(cdev);
-
-	mutex_unlock(&cdev->lock);
 }
 
 /**
Index: linux-pm/drivers/thermal/thermal_debugfs.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_debugfs.c
+++ linux-pm/drivers/thermal/thermal_debugfs.c
@@ -516,6 +516,19 @@ void thermal_debug_cdev_add(struct therm
 	cdev->debugfs = thermal_dbg;
 }
 
+static struct thermal_debugfs *thermal_debug_cdev_clear(struct thermal_cooling_device *cdev)
+{
+	struct thermal_debugfs *thermal_dbg;
+
+	guard(cooling_dev)(cdev);
+
+	thermal_dbg = cdev->debugfs;
+	if (thermal_dbg)
+		cdev->debugfs = NULL;
+
+	return thermal_dbg;
+}
+
 /**
  * thermal_debug_cdev_remove - Remove a cooling device debugfs entry
  *
@@ -527,17 +540,9 @@ void thermal_debug_cdev_remove(struct th
 {
 	struct thermal_debugfs *thermal_dbg;
 
-	mutex_lock(&cdev->lock);
-
-	thermal_dbg = cdev->debugfs;
-	if (!thermal_dbg) {
-		mutex_unlock(&cdev->lock);
+	thermal_dbg = thermal_debug_cdev_clear(cdev);
+	if (!thermal_dbg)
 		return;
-	}
-
-	cdev->debugfs = NULL;
-
-	mutex_unlock(&cdev->lock);
 
 	mutex_lock(&thermal_dbg->lock);
 
Index: linux-pm/drivers/thermal/gov_power_allocator.c
===================================================================
--- linux-pm.orig/drivers/thermal/gov_power_allocator.c
+++ linux-pm/drivers/thermal/gov_power_allocator.c
@@ -549,18 +549,17 @@ static void allow_maximum_power(struct t
 		cdev = instance->cdev;
 
 		instance->target = 0;
-		mutex_lock(&cdev->lock);
-		/*
-		 * Call for updating the cooling devices local stats and avoid
-		 * periods of dozen of seconds when those have not been
-		 * maintained.
-		 */
-		cdev->ops->get_requested_power(cdev, &req_power);
+		scoped_guard(cooling_dev, cdev) {
+			/*
+			 * Call for updating the cooling devices local stats and
+			 * avoid periods of dozen of seconds when those have not
+			 * been maintained.
+			 */
+			cdev->ops->get_requested_power(cdev, &req_power);
 
-		if (params->update_cdevs)
-			__thermal_cdev_update(cdev);
-
-		mutex_unlock(&cdev->lock);
+			if (params->update_cdevs)
+				__thermal_cdev_update(cdev);
+		}
 	}
 }
 
Index: linux-pm/drivers/thermal/gov_step_wise.c
===================================================================
--- linux-pm.orig/drivers/thermal/gov_step_wise.c
+++ linux-pm/drivers/thermal/gov_step_wise.c
@@ -97,9 +97,9 @@ static void thermal_zone_trip_update(str
 
 		instance->initialized = true;
 
-		mutex_lock(&instance->cdev->lock);
-		instance->cdev->updated = false; /* cdev needs update */
-		mutex_unlock(&instance->cdev->lock);
+		scoped_guard(cooling_dev, instance->cdev) {
+			instance->cdev->updated = false; /* cdev needs update */
+		}
 	}
 }

On Fri, Oct 11, 2024 at 12:22 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Add and use a special guard for cooling devices.
>
> This allows quite a few error code paths to be simplified among
> other things and brings in code size reduction for a good measure.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>
> This is a new iteration of
>
> https://lore.kernel.org/linux-pm/1890654.atdPhlSkOF@rjwysocki.net/
>
> v1 -> v2: Rearrange cur_state_store()
>
> ---
>  drivers/thermal/gov_power_allocator.c |   21 +++++++--------
>  drivers/thermal/gov_step_wise.c       |    6 ++--
>  drivers/thermal/thermal_core.c        |   17 +++---------
>  drivers/thermal/thermal_debugfs.c     |   25 +++++++++++-------
>  drivers/thermal/thermal_helpers.c     |   19 +++-----------
>  drivers/thermal/thermal_sysfs.c       |   45 ++++++++++++----------------------
>  include/linux/thermal.h               |    3 ++
>  7 files changed, 57 insertions(+), 79 deletions(-)
>
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -756,12 +756,10 @@ static int thermal_instance_add(struct t
>
>         list_add_tail(&new_instance->trip_node, &td->thermal_instances);
>
> -       mutex_lock(&cdev->lock);
> +       guard(cooling_dev)(cdev);
>
>         list_add_tail(&new_instance->cdev_node, &cdev->thermal_instances);
>
> -       mutex_unlock(&cdev->lock);
> -
>         return 0;
>  }
>
> @@ -870,11 +868,9 @@ static void thermal_instance_delete(stru
>  {
>         list_del(&instance->trip_node);
>
> -       mutex_lock(&instance->cdev->lock);
> +       guard(cooling_dev)(instance->cdev);
>
>         list_del(&instance->cdev_node);
> -
> -       mutex_unlock(&instance->cdev->lock);
>  }
>
>  /**
> @@ -1237,10 +1233,10 @@ void thermal_cooling_device_update(struc
>          * Update under the cdev lock to prevent the state from being set beyond
>          * the new limit concurrently.
>          */
> -       mutex_lock(&cdev->lock);
> +       guard(cooling_dev)(cdev);
>
>         if (cdev->ops->get_max_state(cdev, &cdev->max_state))
> -               goto unlock;
> +               return;
>
>         thermal_cooling_device_stats_reinit(cdev);
>
> @@ -1267,12 +1263,9 @@ void thermal_cooling_device_update(struc
>         }
>
>         if (cdev->ops->get_cur_state(cdev, &state) || state > cdev->max_state)
> -               goto unlock;
> +               return;
>
>         thermal_cooling_device_stats_update(cdev, state);
> -
> -unlock:
> -       mutex_unlock(&cdev->lock);
>  }
>  EXPORT_SYMBOL_GPL(thermal_cooling_device_update);
>
> Index: linux-pm/include/linux/thermal.h
> ===================================================================
> --- linux-pm.orig/include/linux/thermal.h
> +++ linux-pm/include/linux/thermal.h
> @@ -137,6 +137,9 @@ struct thermal_cooling_device {
>  #endif
>  };
>
> +DEFINE_GUARD(cooling_dev, struct thermal_cooling_device *, mutex_lock(&_T->lock),
> +            mutex_unlock(&_T->lock))
> +
>  /* Structure to define Thermal Zone parameters */
>  struct thermal_zone_params {
>         const char *governor_name;
> Index: linux-pm/drivers/thermal/thermal_sysfs.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_sysfs.c
> +++ linux-pm/drivers/thermal/thermal_sysfs.c
> @@ -544,14 +544,15 @@ cur_state_store(struct device *dev, stru
>         if (state > cdev->max_state)
>                 return -EINVAL;
>
> -       mutex_lock(&cdev->lock);
> +       guard(cooling_dev)(cdev);
>
>         result = cdev->ops->set_cur_state(cdev, state);
> -       if (!result)
> -               thermal_cooling_device_stats_update(cdev, state);
> +       if (result)
> +               return result;
>
> -       mutex_unlock(&cdev->lock);
> -       return result ? result : count;
> +       thermal_cooling_device_stats_update(cdev, state);
> +
> +       return count;
>  }
>
>  static struct device_attribute
> @@ -625,21 +626,18 @@ static ssize_t total_trans_show(struct d
>  {
>         struct thermal_cooling_device *cdev = to_cooling_device(dev);
>         struct cooling_dev_stats *stats;
> -       int ret = 0;
> +       int ret;
>
> -       mutex_lock(&cdev->lock);
> +       guard(cooling_dev)(cdev);
>
>         stats = cdev->stats;
>         if (!stats)
> -               goto unlock;
> +               return 0;
>
>         spin_lock(&stats->lock);
>         ret = sprintf(buf, "%u\n", stats->total_trans);
>         spin_unlock(&stats->lock);
>
> -unlock:
> -       mutex_unlock(&cdev->lock);
> -
>         return ret;
>  }
>
> @@ -652,11 +650,11 @@ time_in_state_ms_show(struct device *dev
>         ssize_t len = 0;
>         int i;
>
> -       mutex_lock(&cdev->lock);
> +       guard(cooling_dev)(cdev);
>
>         stats = cdev->stats;
>         if (!stats)
> -               goto unlock;
> +               return 0;
>
>         spin_lock(&stats->lock);
>
> @@ -668,9 +666,6 @@ time_in_state_ms_show(struct device *dev
>         }
>         spin_unlock(&stats->lock);
>
> -unlock:
> -       mutex_unlock(&cdev->lock);
> -
>         return len;
>  }
>
> @@ -682,11 +677,11 @@ reset_store(struct device *dev, struct d
>         struct cooling_dev_stats *stats;
>         int i, states;
>
> -       mutex_lock(&cdev->lock);
> +       guard(cooling_dev)(cdev);
>
>         stats = cdev->stats;
>         if (!stats)
> -               goto unlock;
> +               return count;
>
>         states = cdev->max_state + 1;
>
> @@ -702,9 +697,6 @@ reset_store(struct device *dev, struct d
>
>         spin_unlock(&stats->lock);
>
> -unlock:
> -       mutex_unlock(&cdev->lock);
> -
>         return count;
>  }
>
> @@ -716,13 +708,11 @@ static ssize_t trans_table_show(struct d
>         ssize_t len = 0;
>         int i, j;
>
> -       mutex_lock(&cdev->lock);
> +       guard(cooling_dev)(cdev);
>
>         stats = cdev->stats;
> -       if (!stats) {
> -               len = -ENODATA;
> -               goto unlock;
> -       }
> +       if (!stats)
> +               return -ENODATA;
>
>         len += snprintf(buf + len, PAGE_SIZE - len, " From  :    To\n");
>         len += snprintf(buf + len, PAGE_SIZE - len, "       : ");

There is one more "goto unlock" statement in this function that needs
to be replaced with "return".

I'll send an update of it shortly.

> @@ -760,9 +750,6 @@ static ssize_t trans_table_show(struct d
>                 len = -EFBIG;
>         }
>
> -unlock:
> -       mutex_unlock(&cdev->lock);
> -
>         return len;
>  }
>
> Index: linux-pm/drivers/thermal/thermal_helpers.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_helpers.c
> +++ linux-pm/drivers/thermal/thermal_helpers.c
> @@ -58,17 +58,10 @@ bool thermal_trip_is_bound_to_cdev(struc
>                                    const struct thermal_trip *trip,
>                                    struct thermal_cooling_device *cdev)
>  {
> -       bool ret;
> -
>         guard(thermal_zone)(tz);
> +       guard(cooling_dev)(cdev);
>
> -       mutex_lock(&cdev->lock);
> -
> -       ret = thermal_instance_present(tz, cdev, trip);
> -
> -       mutex_unlock(&cdev->lock);
> -
> -       return ret;
> +       return thermal_instance_present(tz, cdev, trip);
>  }
>  EXPORT_SYMBOL_GPL(thermal_trip_is_bound_to_cdev);
>
> @@ -197,12 +190,12 @@ void __thermal_cdev_update(struct therma
>   */
>  void thermal_cdev_update(struct thermal_cooling_device *cdev)
>  {
> -       mutex_lock(&cdev->lock);
> +       guard(cooling_dev)(cdev);
> +
>         if (!cdev->updated) {
>                 __thermal_cdev_update(cdev);
>                 cdev->updated = true;
>         }
> -       mutex_unlock(&cdev->lock);
>  }
>
>  /**
> @@ -211,11 +204,9 @@ void thermal_cdev_update(struct thermal_
>   */
>  void thermal_cdev_update_nocheck(struct thermal_cooling_device *cdev)
>  {
> -       mutex_lock(&cdev->lock);
> +       guard(cooling_dev)(cdev);
>
>         __thermal_cdev_update(cdev);
> -
> -       mutex_unlock(&cdev->lock);
>  }
>
>  /**
> Index: linux-pm/drivers/thermal/thermal_debugfs.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_debugfs.c
> +++ linux-pm/drivers/thermal/thermal_debugfs.c
> @@ -516,6 +516,19 @@ void thermal_debug_cdev_add(struct therm
>         cdev->debugfs = thermal_dbg;
>  }
>
> +static struct thermal_debugfs *thermal_debug_cdev_clear(struct thermal_cooling_device *cdev)
> +{
> +       struct thermal_debugfs *thermal_dbg;
> +
> +       guard(cooling_dev)(cdev);
> +
> +       thermal_dbg = cdev->debugfs;
> +       if (thermal_dbg)
> +               cdev->debugfs = NULL;
> +
> +       return thermal_dbg;
> +}
> +
>  /**
>   * thermal_debug_cdev_remove - Remove a cooling device debugfs entry
>   *
> @@ -527,17 +540,9 @@ void thermal_debug_cdev_remove(struct th
>  {
>         struct thermal_debugfs *thermal_dbg;
>
> -       mutex_lock(&cdev->lock);
> -
> -       thermal_dbg = cdev->debugfs;
> -       if (!thermal_dbg) {
> -               mutex_unlock(&cdev->lock);
> +       thermal_dbg = thermal_debug_cdev_clear(cdev);
> +       if (!thermal_dbg)
>                 return;
> -       }
> -
> -       cdev->debugfs = NULL;
> -
> -       mutex_unlock(&cdev->lock);
>
>         mutex_lock(&thermal_dbg->lock);
>
> Index: linux-pm/drivers/thermal/gov_power_allocator.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/gov_power_allocator.c
> +++ linux-pm/drivers/thermal/gov_power_allocator.c
> @@ -549,18 +549,17 @@ static void allow_maximum_power(struct t
>                 cdev = instance->cdev;
>
>                 instance->target = 0;
> -               mutex_lock(&cdev->lock);
> -               /*
> -                * Call for updating the cooling devices local stats and avoid
> -                * periods of dozen of seconds when those have not been
> -                * maintained.
> -                */
> -               cdev->ops->get_requested_power(cdev, &req_power);
> +               scoped_guard(cooling_dev, cdev) {
> +                       /*
> +                        * Call for updating the cooling devices local stats and
> +                        * avoid periods of dozen of seconds when those have not
> +                        * been maintained.
> +                        */
> +                       cdev->ops->get_requested_power(cdev, &req_power);
>
> -               if (params->update_cdevs)
> -                       __thermal_cdev_update(cdev);
> -
> -               mutex_unlock(&cdev->lock);
> +                       if (params->update_cdevs)
> +                               __thermal_cdev_update(cdev);
> +               }
>         }
>  }
>
> Index: linux-pm/drivers/thermal/gov_step_wise.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/gov_step_wise.c
> +++ linux-pm/drivers/thermal/gov_step_wise.c
> @@ -97,9 +97,9 @@ static void thermal_zone_trip_update(str
>
>                 instance->initialized = true;
>
> -               mutex_lock(&instance->cdev->lock);
> -               instance->cdev->updated = false; /* cdev needs update */
> -               mutex_unlock(&instance->cdev->lock);
> +               scoped_guard(cooling_dev, instance->cdev) {
> +                       instance->cdev->updated = false; /* cdev needs update */
> +               }
>         }
>  }
>
>
>
>
>

On 10/14/24 13:26, Rafael J. Wysocki wrote:
> On Fri, Oct 11, 2024 at 12:22 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>>
>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>> Add and use a special guard for cooling devices.
>>
>> This allows quite a few error code paths to be simplified among
>> other things and brings in code size reduction for a good measure.
>>
>> No intentional functional impact.
>>
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> ---
>>
>> This is a new iteration of
>>
>> https://lore.kernel.org/linux-pm/1890654.atdPhlSkOF@rjwysocki.net/
>>
>> v1 -> v2: Rearrange cur_state_store()
>>
>> ---
>>   drivers/thermal/gov_power_allocator.c |   21 +++++++--------
>>   drivers/thermal/gov_step_wise.c       |    6 ++--
>>   drivers/thermal/thermal_core.c        |   17 +++---------
>>   drivers/thermal/thermal_debugfs.c     |   25 +++++++++++-------
>>   drivers/thermal/thermal_helpers.c     |   19 +++-----------
>>   drivers/thermal/thermal_sysfs.c       |   45 ++++++++++++----------------------
>>   include/linux/thermal.h               |    3 ++
>>   7 files changed, 57 insertions(+), 79 deletions(-)
>>

[snip]

>>
>>          stats = cdev->stats;
>> -       if (!stats) {
>> -               len = -ENODATA;
>> -               goto unlock;
>> -       }
>> +       if (!stats)
>> +               return -ENODATA;
>>
>>          len += snprintf(buf + len, PAGE_SIZE - len, " From  :    To\n");
>>          len += snprintf(buf + len, PAGE_SIZE - len, "       : ");
> 
> There is one more "goto unlock" statement in this function that needs
> to be replaced with "return".
> 
> I'll send an update of it shortly.
> 

OK, I will review that when it's ready.

On Wed, Oct 23, 2024 at 12:24 AM Lukasz Luba <lukasz.luba@arm.com> wrote:
>
>
>
> On 10/14/24 13:26, Rafael J. Wysocki wrote:
> > On Fri, Oct 11, 2024 at 12:22 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >>
> >> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >>
> >> Add and use a special guard for cooling devices.
> >>
> >> This allows quite a few error code paths to be simplified among
> >> other things and brings in code size reduction for a good measure.
> >>
> >> No intentional functional impact.
> >>
> >> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >> ---
> >>
> >> This is a new iteration of
> >>
> >> https://lore.kernel.org/linux-pm/1890654.atdPhlSkOF@rjwysocki.net/
> >>
> >> v1 -> v2: Rearrange cur_state_store()
> >>
> >> ---
> >>   drivers/thermal/gov_power_allocator.c |   21 +++++++--------
> >>   drivers/thermal/gov_step_wise.c       |    6 ++--
> >>   drivers/thermal/thermal_core.c        |   17 +++---------
> >>   drivers/thermal/thermal_debugfs.c     |   25 +++++++++++-------
> >>   drivers/thermal/thermal_helpers.c     |   19 +++-----------
> >>   drivers/thermal/thermal_sysfs.c       |   45 ++++++++++++----------------------
> >>   include/linux/thermal.h               |    3 ++
> >>   7 files changed, 57 insertions(+), 79 deletions(-)
> >>
>
> [snip]
>
> >>
> >>          stats = cdev->stats;
> >> -       if (!stats) {
> >> -               len = -ENODATA;
> >> -               goto unlock;
> >> -       }
> >> +       if (!stats)
> >> +               return -ENODATA;
> >>
> >>          len += snprintf(buf + len, PAGE_SIZE - len, " From  :    To\n");
> >>          len += snprintf(buf + len, PAGE_SIZE - len, "       : ");
> >
> > There is one more "goto unlock" statement in this function that needs
> > to be replaced with "return".
> >
> > I'll send an update of it shortly.
> >
>
> OK, I will review that when it's ready.

It's been sent already:

https://lore.kernel.org/linux-pm/5837621.DvuYhMxLoT@rjwysocki.net/