Commit be0a3600 authored by Rafael J. Wysocki's avatar Rafael J. Wysocki
Browse files

thermal: sysfs: Rework the handling of trip point updates 



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, input processing need not be done under the thermal zone lock
in any of these functions.

Rework trip_point_temp_store() and trip_point_hyst_store() to address
the above, move the part of thermal_zone_set_trip() that is still
useful to a new function called thermal_zone_trip_updated() and drop
the rest of it.

While at it, make trip_point_hyst_store() reject negative hysteresis
values.

Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: default avatarDaniel Lezcano <daniel.lezcano@linaro.org>
parent 59730241

drivers/thermal/thermal_core.h

+2 −0
Original line number Diff line number Diff line
@@ -124,6 +124,8 @@ int __thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id, 
			    struct thermal_trip *trip);

int thermal_zone_trip_id(struct thermal_zone_device *tz,

			 const struct thermal_trip *trip);

void thermal_zone_trip_updated(struct thermal_zone_device *tz,

			       const struct thermal_trip *trip);

int __thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);



/* sysfs I/F */

drivers/thermal/thermal_sysfs.c

+36 −16
Original line number Diff line number Diff line
@@ -120,8 +120,13 @@ trip_point_temp_store(struct device *dev, struct device_attribute *attr, 
		      const char *buf, size_t count)

{

	struct thermal_zone_device *tz = to_thermal_zone(dev);

	struct thermal_trip trip;

	struct thermal_trip *trip;

	int trip_id, ret;

	int temp;



	ret = kstrtoint(buf, 10, &temp);

	if (ret)

		return -EINVAL;



	if (sscanf(attr->attr.name, "trip_point_%d_temp", &trip_id) != 1)

		return -EINVAL;
@@ -133,15 +138,20 @@ trip_point_temp_store(struct device *dev, struct device_attribute *attr, 
		goto unlock;

	}



	ret = __thermal_zone_get_trip(tz, trip_id, &trip);

	if (ret)

		goto unlock;

	trip = &tz->trips[trip_id];



	ret = kstrtoint(buf, 10, &trip.temperature);

	if (temp != trip->temperature) {

		if (tz->ops->set_trip_temp) {

			ret = tz->ops->set_trip_temp(tz, trip_id, temp);

			if (ret)

				goto unlock;

		}



		trip->temperature = temp;



		thermal_zone_trip_updated(tz, trip);

	}



	ret = thermal_zone_set_trip(tz, trip_id, &trip);

unlock:

	mutex_unlock(&tz->lock);
@@ -179,8 +189,13 @@ trip_point_hyst_store(struct device *dev, struct device_attribute *attr, 
		      const char *buf, size_t count)

{

	struct thermal_zone_device *tz = to_thermal_zone(dev);

	struct thermal_trip trip;

	struct thermal_trip *trip;

	int trip_id, ret;

	int hyst;



	ret = kstrtoint(buf, 10, &hyst);

	if (ret || hyst < 0)

		return -EINVAL;



	if (sscanf(attr->attr.name, "trip_point_%d_hyst", &trip_id) != 1)

		return -EINVAL;
@@ -192,15 +207,20 @@ trip_point_hyst_store(struct device *dev, struct device_attribute *attr, 
		goto unlock;

	}



	ret = __thermal_zone_get_trip(tz, trip_id, &trip);

	if (ret)

		goto unlock;

	trip = &tz->trips[trip_id];



	ret = kstrtoint(buf, 10, &trip.hysteresis);

	if (hyst != trip->hysteresis) {

		if (tz->ops->set_trip_hyst) {

			ret = tz->ops->set_trip_hyst(tz, trip_id, hyst);

			if (ret)

				goto unlock;

		}



		trip->hysteresis = hyst;



		thermal_zone_trip_updated(tz, trip);

	}



	ret = thermal_zone_set_trip(tz, trip_id, &trip);

unlock:

	mutex_unlock(&tz->lock);

drivers/thermal/thermal_trip.c

+9 −36
Original line number Diff line number Diff line
@@ -147,42 +147,6 @@ int thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id, 
}

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)

{

	struct thermal_trip t;

	int ret;



	ret = __thermal_zone_get_trip(tz, trip_id, &t);

	if (ret)

		return ret;



	if (t.type != trip->type)

		return -EINVAL;



	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;

	}



	if (t.hysteresis != trip->hysteresis && tz->ops->set_trip_hyst) {

		ret = tz->ops->set_trip_hyst(tz, trip_id, trip->hysteresis);

		if (ret)

			return ret;

	}



	if (tz->trips && (t.temperature != trip->temperature || t.hysteresis != trip->hysteresis))

		tz->trips[trip_id] = *trip;



	thermal_notify_tz_trip_change(tz->id, trip_id, trip->type,

				      trip->temperature, trip->hysteresis);



	__thermal_zone_device_update(tz, THERMAL_TRIP_CHANGED);



	return 0;

}



int thermal_zone_trip_id(struct thermal_zone_device *tz,

			 const struct thermal_trip *trip)

{
@@ -192,3 +156,12 @@ int thermal_zone_trip_id(struct thermal_zone_device *tz, 
	 */

	return trip - tz->trips;

}



void thermal_zone_trip_updated(struct thermal_zone_device *tz,

			       const struct thermal_trip *trip)

{

	thermal_notify_tz_trip_change(tz->id, thermal_zone_trip_id(tz, trip),

				      trip->type, trip->temperature,

				      trip->hysteresis);

	__thermal_zone_device_update(tz, THERMAL_TRIP_CHANGED);

}

include/linux/thermal.h

+0 −4
Original line number Diff line number Diff line
@@ -282,10 +282,6 @@ int __thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id, 
			    struct thermal_trip *trip);

int thermal_zone_get_trip(struct thermal_zone_device *tz, int trip_id,

			  struct thermal_trip *trip);



int thermal_zone_set_trip(struct thermal_zone_device *tz, int trip_id,

			  const struct thermal_trip *trip);



int for_each_thermal_trip(struct thermal_zone_device *tz,

			  int (*cb)(struct thermal_trip *, void *),

			  void *data);