thermal: trip: Rework thermal_zone_set_trip() and its callers

From: Rafael J. Wysocki <[email protected]>

After recent changes in the thermal framework, a trip points array is required for registering a thermal zone that is not tripless, so the tz->trips pointer in thermal_zone_set_trip() is never NULL and the check involving it is redundant. Drop that check.

No functional impact.

Signed-off-by Rafael J. Wysocki <[email protected]>

---

New patch in v2.

---
  drivers/thermal/thermal_trip.c |    3 ---
  1 file changed, 3 deletions(-)

Index: linux-pm/drivers/thermal/thermal_trip.c
===================================================================

From: Rafael J. Wysocki <[email protected]>

Both trip_point_temp_store() and trip_point_hyst_store() use thermal_zone_set_trip() to update a given trip point, but none of them actually needs to change more than one field in struct thermal_trip representing it. However, each of them effectively calls __thermal_zone_get_trip() twice in a row for the same trip index value, once directly and once via thermal_zone_set_trip(), which is not particularly efficient, and the way in which thermal_zone_set_trip() carries out the update is not particularly straightforward.

Moreover, some checks done by them both need not go under the thermal zone lock and code duplication between them can be reduced quite a bit by moving the majority of logic into thermal_zone_set_trip().

Rework all of the above functions to address the above.

No intentional functional impact.

Signed-off-by Rafael J. Wysocki <[email protected]>

---

v1 -> v2:
* Fix 2 typos in the changelog (Lukasz).
* Split one change into the [1/2].

---
  drivers/thermal/thermal_core.h  |    9 ++++++
  drivers/thermal/thermal_sysfs.c |   52 ++++++++--------------------------
  drivers/thermal/thermal_trip.c  |   60 +++++++++++++++++++++++++++-------------
  include/linux/thermal.h         |    3 --
  4 files changed, 62 insertions(+), 62 deletions(-)

Index: linux-pm/drivers/thermal/thermal_trip.c
===================================================================

drivers/thermal/thermal_trip.c +0 -1

@@ -148,39 +148,61 @@ int thermal_zone_get_trip(struct thermal
  EXPORT_SYMBOL_GPL(thermal_zone_get_trip);
 
  int thermal_zone_set_trip(struct thermal_zone_device *tz, int trip_id,
-			  const struct thermal_trip *trip)

drivers/thermal/thermal_trip.c +2 -0

+
+	ret = kstrtoint(buf, 10, &val);
  	if (ret)
  		return ret;
 

Quoted 33 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Both trip_point_temp_store() and trip_point_hyst_store() use thermal_zone_set_trip() to update a given trip point, but none of them actually needs to change more than one field in struct thermal_trip representing it. However, each of them effectively calls __thermal_zone_get_trip() twice in a row for the same trip index value, once directly and once via thermal_zone_set_trip(), which is not particularly efficient, and the way in which thermal_zone_set_trip() carries out the update is not particularly straightforward.

Moreover, some checks done by them both need not go under the thermal zone lock and code duplication between them can be reduced quite a bit by moving the majority of logic into thermal_zone_set_trip().

Rework all of the above functions to address the above.

No intentional functional impact.

Signed-off-by Rafael J. Wysocki <[email protected]>

---

v1 -> v2:
* Fix 2 typos in the changelog (Lukasz).
* Split one change into the [1/2].

---
  drivers/thermal/thermal_core.h  |    9 ++++++
  drivers/thermal/thermal_sysfs.c |   52 ++++++++--------------------------
  drivers/thermal/thermal_trip.c  |   60 +++++++++++++++++++++++++++-------------
  include/linux/thermal.h         |    3 --
  4 files changed, 62 insertions(+), 62 deletions(-)

[snip]

Quoted 16 lines

Index: linux-pm/drivers/thermal/thermal_trip.c
===================================================================

drivers/thermal/thermal_trip.c +3 -3

@@ -148,39 +148,61 @@ int thermal_zone_get_trip(struct thermal
  EXPORT_SYMBOL_GPL(thermal_zone_get_trip);
 
  int thermal_zone_set_trip(struct thermal_zone_device *tz, int trip_id,
-                       const struct thermal_trip *trip)
+                       enum thermal_set_trip_target what, const char *buf)
  {
-     struct thermal_trip t;
-     int ret;
+     struct thermal_trip *trip;
+     int val, ret = 0;
 

drivers/thermal/thermal_trip.c int thermal_zone_get_trip(struct thermal · line 157

Quoted 3 lines

drivers/thermal/thermal_trip.c +2 -1

-     ret = __thermal_zone_get_trip(tz, trip_id, &t);
+     if (trip_id < 0 || trip_id >= tz->num_trips)
+             ret = -EINVAL;

That shouldn't progress forward IMO, but simply 'return -EINVAL;'...

Quoted 9 lines

drivers/thermal/thermal_trip.c +3 -2

+
+     ret = kstrtoint(buf, 10, &val);
      if (ret)
              return ret;
 
-     if (t.type != trip->type)
-             return -EINVAL;
+     mutex_lock(&tz->lock);
 

drivers/thermal/thermal_trip.c int thermal_zone_get_trip(struct thermal · line 165

Quoted 6 lines

drivers/thermal/thermal_trip.c +1 -5

-     if (t.temperature != trip->temperature && tz->ops->set_trip_temp) {
-             ret = tz->ops->set_trip_temp(tz, trip_id, trip->temperature);
-             if (ret)
-                     return ret;
-     }
+     trip = &tz->trips[trip_id];

... because here we might get an issue.