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 cpu frequency. But
this has to be done again in cooling devices so to make the cooling
devices generic and to avoid the temperature comparison 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 [off-list ref] wrote:

quoted

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,
+                                                       &current_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,
+                                                       &current_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 [off-list ref] wrote:

quoted

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 <redacted>
---
 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-*] for ACPI
-       thermal zone.
+       E.g. it can be one of critical, hot, passive, active[0-1],
+       state-active[0-*] for ACPI thermal zone.
       RO, Optional

 cdev[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