Hi Vincent,
Thanks for the review. Well actually your are correct that current temperature and last temperature can be used to increase or decrease the cufreq. But this has to be done again in cooling devices so to make the cooling devices generic and to avoid the temperature comparision again this new trip type passes the cooling device instance id. Also about your queries that this may add dependency between trip index and cooling state. This is actually needed and this dependency is created when the cooling device is binded with trip points(For cpufreq type cooling device just the instance of cooling device is associated with trip points). More over the existing PASSIVE cooling trip type does the same thing and iterates across all the cooling state.
Thanks, Amit Daniel
On 20 December 2011 18:07, Vincent Guittot vincent.guittot@linaro.orgwrote:
Hi Amit,
I'm not sure that using the trip index for setting the state of a cooling device is a generic solution because you are adding a dependency between the trip index and the cooling device state that might be difficult to handle. This dependency implies that a cooling device like cpufreq_cooling_device must be registered in the 1st trips of a thermal_zone which is not possible when we want to register 2 cpufreq_cooling_devices in the same thermal_zone. You should only rely on the current and last temperatures to detect if a trip_temp has been crossed and you should increase or decrease the current state of the cooling device accordingly.
something like below should work with cpufreq_cooling_device and will not add any constraint on the trip index. The state of a cooling device is increased/decreased once for each trip
case THERMAL_TRIP_STATE_ACTIVE:list_for_each_entry(instance, &tz->cooling_devices,node) {if (instance->trip != count)continue;cdev = instance->cdev;if ((temp >= trip_temp)&& (trip_temp >tz->last_temperature)) {
cdev->ops->get_max_state(cdev,&max_state);cdev->ops->get_cur_state(cdev,¤t_state);if (++current_state <= max_state)cdev->ops->set_cur_state(cdev,
current_state);
}else if ((temp < trip_temp)&& (trip_temp <=tz->last_temperature)) {
cdev->ops->get_cur_state(cdev,¤t_state);if (current_state > 0)cdev->ops->set_cur_state(cdev,
--current_state);
}break;Regards, Vincent
On 13 December 2011 16:13, Amit Daniel Kachhap amit.kachhap@linaro.org wrote:
This patch adds a new trip type THERMAL_TRIP_STATE_ACTIVE. This trip behaves same as THERMAL_TRIP_ACTIVE but also passes the cooling device instance number. This helps the cooling device registered as different instances to perform appropriate cooling action decision in the set_cur_state call back function.
Also since the trip temperature's are in ascending order so some logic is put in place to skip the un-necessary checks.
Signed-off-by: Amit Daniel Kachhap amit.kachhap@linaro.org
Documentation/thermal/sysfs-api.txt | 4 ++-- drivers/thermal/thermal_sys.c | 27 ++++++++++++++++++++++++++- include/linux/thermal.h | 1 + 3 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/Documentation/thermal/sysfs-api.txt
b/Documentation/thermal/sysfs-api.txt
index b61e46f..5c1d44e 100644 --- a/Documentation/thermal/sysfs-api.txt +++ b/Documentation/thermal/sysfs-api.txt @@ -184,8 +184,8 @@ trip_point_[0-*]_temp
trip_point_[0-*]_type Strings which indicate the type of the trip point.
E.g. it can be one of critical, hot, passive, active[0-*] forACPI
thermal zone.
E.g. it can be one of critical, hot, passive, active[0-1],state-active[0-*] for ACPI thermal zone. RO, Optionalcdev[0-*] diff --git a/drivers/thermal/thermal_sys.c
b/drivers/thermal/thermal_sys.c
index dd9a574..72b1ab3 100644 --- a/drivers/thermal/thermal_sys.c +++ b/drivers/thermal/thermal_sys.c @@ -192,6 +192,8 @@ trip_point_type_show(struct device *dev, struct
device_attribute *attr,
return sprintf(buf, "passive\n"); case THERMAL_TRIP_ACTIVE: return sprintf(buf, "active\n");
case THERMAL_TRIP_STATE_ACTIVE:return sprintf(buf, "state-active\n"); default: return sprintf(buf, "unknown\n"); }@@ -1035,7 +1037,7 @@ EXPORT_SYMBOL(thermal_cooling_device_unregister); void thermal_zone_device_update(struct thermal_zone_device *tz) { int count, ret = 0;
long temp, trip_temp;
long temp, trip_temp, max_state, last_trip_change = 0; enum thermal_trip_type trip_type; struct thermal_cooling_device_instance *instance; struct thermal_cooling_device *cdev;@@ -1086,6 +1088,29 @@ void thermal_zone_device_update(struct
thermal_zone_device *tz)
cdev->ops->set_cur_state(cdev, 0); } break;
case THERMAL_TRIP_STATE_ACTIVE:list_for_each_entry(instance,&tz->cooling_devices,
node) {if (instance->trip != count)continue;if (temp <= last_trip_change)continue;cdev = instance->cdev;cdev->ops->get_max_state(cdev,&max_state);
if ((temp >= trip_temp) &&((count + 1) <=max_state))
cdev->ops->set_cur_state(cdev,count +1);
else if ((temp < trip_temp) &&(count <=max_state))
cdev->ops->set_cur_state(cdev,count);
last_trip_change = trip_temp;}break; case THERMAL_TRIP_PASSIVE: if (temp >= trip_temp || tz->passive) thermal_zone_device_passive(tz, temp,diff --git a/include/linux/thermal.h b/include/linux/thermal.h index 47b4a27..d7d0a27 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -42,6 +42,7 @@ enum thermal_trip_type { THERMAL_TRIP_PASSIVE, THERMAL_TRIP_HOT, THERMAL_TRIP_CRITICAL,
THERMAL_TRIP_STATE_ACTIVE,};
struct thermal_zone_device_ops {
1.7.1
linux-pm mailing list linux-pm@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-pm