lkml.org 
[lkml]   [2012]   [Jan]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [linux-pm] [RFC PATCH 1/2] thermal: Add a new trip type to use cooling device instance number
    From
    Hey Amit/Vincent,

    It appears that with this implementation the STATE_ACTIVE trip number
    used will also be the index of the cool_freq_tab used. If that is
    true, then perhaps a common structure would be beneficial that links
    each STATE_ACTIVE trip point with its corresponding cooling data.

    BR,
    Rob

    On Tue, Dec 20, 2011 at 11:11 PM, Amit Kachhap <amit.kachhap@linaro.org> wrote:
    >  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 <vincent.guittot@linaro.org> wrote:
    >>>
    >>> 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 <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-*] 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
    >>
    >>
    > --
    > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    > the body of a message to majordomo@vger.kernel.org
    > More majordomo info at  http://vger.kernel.org/majordomo-info.html
    > Please read the FAQ at  http://www.tux.org/lkml/
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2012-01-11 09:15    [W:0.039 / U:16.316 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site