diff mbox

[RFC,2/7] Thermal: Create zone level APIs

Message ID 1353149158-19102-3-git-send-email-durgadoss.r@intel.com (mailing list archive)
State Not Applicable, archived
Delegated to: Zhang Rui
Headers show

Commit Message

durgadoss.r@intel.com Nov. 17, 2012, 10:45 a.m. UTC
This patch adds a new thermal_zone structure to
thermal.h. Also, adds zone level APIs to the thermal
framework.

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/thermal/thermal_sys.c |  206 +++++++++++++++++++++++++++++++++++++++++
 include/linux/thermal.h       |   25 +++++
 2 files changed, 231 insertions(+)

Comments

Hongbo Zhang Dec. 3, 2012, 7:42 a.m. UTC | #1
On 17 November 2012 18:45, Durgadoss R <durgadoss.r@intel.com> wrote:
> This patch adds a new thermal_zone structure to
> thermal.h. Also, adds zone level APIs to the thermal
> framework.
>
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> ---
>  drivers/thermal/thermal_sys.c |  206 +++++++++++++++++++++++++++++++++++++++++
>  include/linux/thermal.h       |   25 +++++
>  2 files changed, 231 insertions(+)
>
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index e726c8b..9d386d7 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -44,19 +44,28 @@ MODULE_DESCRIPTION("Generic thermal management sysfs support");
>  MODULE_LICENSE("GPL");
>
>  static DEFINE_IDR(thermal_tz_idr);
> +static DEFINE_IDR(thermal_zone_idr);
>  static DEFINE_IDR(thermal_cdev_idr);
>  static DEFINE_IDR(thermal_sensor_idr);
>  static DEFINE_MUTEX(thermal_idr_lock);
>
>  static LIST_HEAD(thermal_tz_list);
>  static LIST_HEAD(thermal_sensor_list);
> +static LIST_HEAD(thermal_zone_list);
>  static LIST_HEAD(thermal_cdev_list);
>  static LIST_HEAD(thermal_governor_list);
>
>  static DEFINE_MUTEX(thermal_list_lock);
>  static DEFINE_MUTEX(ts_list_lock);
> +static DEFINE_MUTEX(tz_list_lock);
>  static DEFINE_MUTEX(thermal_governor_lock);
>
> +#define for_each_thermal_sensor(pos) \
> +       list_for_each_entry(pos, &thermal_sensor_list, node)
> +
> +#define for_each_thermal_zone(pos) \
> +       list_for_each_entry(pos, &thermal_zone_list, node)
> +
>  static struct thermal_governor *__find_governor(const char *name)
>  {
>         struct thermal_governor *pos;
> @@ -419,15 +428,62 @@ static void thermal_zone_device_check(struct work_struct *work)
>         thermal_zone_device_update(tz);
>  }
>
> +static int get_sensor_indx(struct thermal_zone *tz, struct thermal_sensor *ts)
> +{
> +       int i, indx = -EINVAL;
> +
> +       if (!tz || !ts)
> +               return -EINVAL;
> +
> +       mutex_lock(&ts_list_lock);
> +       for (i = 0; i < tz->sensor_indx; i++) {
> +               if (tz->sensors[i] == ts) {
> +                       indx = i;
> +                       break;
> +               }
> +       }
> +       mutex_unlock(&ts_list_lock);
> +       return indx;
> +}
> +
> +static void remove_sensor_from_zone(struct thermal_zone *tz,
> +                               struct thermal_sensor *ts)
> +{
> +       int indx, j;
> +
> +       indx = get_sensor_indx(tz, ts);
> +       if (indx < 0)
> +               return;
> +
> +       sysfs_remove_link(&tz->device.kobj, kobject_name(&ts->device.kobj));
> +
> +       /* Shift the entries in the tz->sensors array */
> +       for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++)
> +               tz->sensors[j] = tz->sensors[j + 1];
> +
> +       tz->sensor_indx--;
> +}
> +
>  /* sys I/F for thermal zone */
>
>  #define to_thermal_zone(_dev) \
>         container_of(_dev, struct thermal_zone_device, device)
>
> +#define to_zone(_dev) \
> +       container_of(_dev, struct thermal_zone, device)
> +
>  #define to_thermal_sensor(_dev) \
>         container_of(_dev, struct thermal_sensor, device)
>
>  static ssize_t
> +zone_name_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +       struct thermal_zone *tz = to_zone(dev);
> +
> +       return sprintf(buf, "%s\n", tz->name);
> +}
> +
> +static ssize_t
>  sensor_name_show(struct device *dev, struct device_attribute *attr, char *buf)
>  {
>         struct thermal_sensor *ts = to_thermal_sensor(dev);
> @@ -772,6 +828,8 @@ static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR, policy_show, policy_store);
>  static DEVICE_ATTR(sensor_name, 0444, sensor_name_show, NULL);
>  static DEVICE_ATTR(temp_input, 0444, sensor_temp_show, NULL);
>
> +static DEVICE_ATTR(zone_name, 0444, zone_name_show, NULL);
> +
>  /* sys I/F for cooling device */
>  #define to_cooling_device(_dev)        \
>         container_of(_dev, struct thermal_cooling_device, device)
> @@ -1557,6 +1615,146 @@ static void remove_trip_attrs(struct thermal_zone_device *tz)
>         kfree(tz->trip_hyst_attrs);
>  }
>
> +struct thermal_zone *create_thermal_zone(const char *name, void *devdata)
> +{
> +       struct thermal_zone *tz;
> +       int ret;
> +
> +       if (!name || (name && strlen(name) >= THERMAL_NAME_LENGTH))
> +               return ERR_PTR(-EINVAL);
> +
> +       tz = kzalloc(sizeof(*tz), GFP_KERNEL);
> +       if (!tz)
> +               return ERR_PTR(-ENOMEM);
> +
> +       idr_init(&tz->idr);
> +       ret = get_idr(&thermal_zone_idr, &thermal_idr_lock, &tz->id);
> +       if (ret)
> +               goto exit_free;
> +
> +       strcpy(tz->name, name);
> +       tz->devdata = devdata;
> +       tz->device.class = &thermal_class;
> +
> +       dev_set_name(&tz->device, "zone%d", tz->id);
> +       ret = device_register(&tz->device);
> +       if (ret)
> +               goto exit_idr;
> +
> +       ret = device_create_file(&tz->device, &dev_attr_zone_name);
> +       if (ret)
> +               goto exit_unregister;
> +
> +       /* Add this zone to the global list of thermal zones */
> +       mutex_lock(&tz_list_lock);
> +       list_add_tail(&tz->node, &thermal_zone_list);
> +       mutex_unlock(&tz_list_lock);
> +       return tz;
> +
> +exit_unregister:
> +       device_unregister(&tz->device);
> +exit_idr:
> +       release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
> +exit_free:
> +       kfree(tz);
> +       return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL(create_thermal_zone);
> +
> +void remove_thermal_zone(struct thermal_zone *tz)
> +{
> +       struct thermal_zone *pos, *next;
> +       bool found = false;
> +
> +       if (!tz)
> +               return;
> +
> +       mutex_lock(&tz_list_lock);
> +
> +       list_for_each_entry_safe(pos, next, &thermal_zone_list, node) {
> +               if (pos == tz) {
> +                       list_del(&tz->node);
> +                       found = true;
> +                       break;
> +               }
> +       }
> +
> +       if (!found)
> +               goto exit;
> +
> +       device_remove_file(&tz->device, &dev_attr_zone_name);
> +
> +       release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
> +       idr_destroy(&tz->idr);
> +
> +       device_unregister(&tz->device);
> +       kfree(tz);
> +exit:
> +       mutex_unlock(&tz_list_lock);
> +       return;
> +}
> +EXPORT_SYMBOL(remove_thermal_zone);
> +
> +struct thermal_sensor *get_sensor_by_name(const char *name)
> +{
> +       struct thermal_sensor *pos;
> +       struct thermal_sensor *ts = NULL;
> +
> +       mutex_lock(&ts_list_lock);
> +       for_each_thermal_sensor(pos) {
> +               if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) {
> +                       ts = pos;
> +                       break;
> +               }
> +       }
> +       mutex_unlock(&ts_list_lock);
> +       return ts;
> +}
> +EXPORT_SYMBOL(get_sensor_by_name);
> +
> +int add_sensor_to_zone_by_name(struct thermal_zone *tz, const char *name)
> +{
> +       int ret;
> +       struct thermal_sensor *ts = get_sensor_by_name(name);
> +
> +       if (!tz || !ts)
> +               return -EINVAL;
> +
> +       mutex_lock(&tz_list_lock);
> +
> +       /* Ensure we are not adding the same sensor again!! */
> +       ret = get_sensor_indx(tz, ts);
> +       if (ret >= 0) {
> +               ret = -EEXIST;
> +               goto exit_zone;
> +       }
> +
> +       mutex_lock(&ts_list_lock);
> +
> +       ret = sysfs_create_link(&tz->device.kobj, &ts->device.kobj,
> +                               kobject_name(&ts->device.kobj));
> +       if (ret)
> +               goto exit_sensor;
> +
> +       tz->sensors[tz->sensor_indx++] = ts;
> +
> +exit_sensor:
> +       mutex_unlock(&ts_list_lock);
> +exit_zone:
> +       mutex_unlock(&tz_list_lock);
> +       return ret;
> +}
> +EXPORT_SYMBOL(add_sensor_to_zone_by_name);
> +
> +int add_sensor_to_zone(struct thermal_zone *tz, struct thermal_sensor *ts)
> +{
> +       if (!tz || !ts)
> +               return -EINVAL;
> +
> +       return add_sensor_to_zone_by_name(tz, ts->name);
> +}
> +EXPORT_SYMBOL(add_sensor_to_zone);
> +
>  /**
>   * thermal_sensor_register - register a new thermal sensor
>   * @name:      name of the thermal sensor
> @@ -1624,6 +1822,7 @@ EXPORT_SYMBOL(thermal_sensor_register);
>  void thermal_sensor_unregister(struct thermal_sensor *ts)
>  {
>         int i;
> +       struct thermal_zone *tz;
>         struct thermal_sensor *pos, *next;
>         bool found = false;
>
> @@ -1643,6 +1842,13 @@ void thermal_sensor_unregister(struct thermal_sensor *ts)
>         if (!found)
>                 return;
>
> +       mutex_lock(&tz_list_lock);
> +
> +       for_each_thermal_zone(tz)
> +               remove_sensor_from_zone(tz, ts);
> +
> +       mutex_unlock(&tz_list_lock);
> +
>         for (i = 0; i < ts->thresholds; i++)
>                 device_remove_file(&ts->device, &ts->thresh_attrs[i].attr);
>
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index e2f85ec..38438be 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -49,6 +49,11 @@
>  /* Default Thermal Governor: Does Linear Throttling */
>  #define DEFAULT_THERMAL_GOVERNOR       "step_wise"
>
> +/* Maximum number of sensors, allowed in a thermal zone
> + * We will start with 5, and increase if needed.
> + */
> +#define MAX_SENSORS_PER_ZONE           5
> +
>  struct thermal_sensor;
>  struct thermal_zone_device;
>  struct thermal_cooling_device;
> @@ -191,6 +196,21 @@ struct thermal_zone_device {
>         struct delayed_work poll_queue;
>  };
>
> +struct thermal_zone {
> +       char name[THERMAL_NAME_LENGTH];
> +       int id;
> +       void *devdata;
> +       struct thermal_zone *ops;
What is this, thermal_zone_device_ops? and how to initialize this ops?

> +       struct thermal_governor *governor;
> +       struct idr idr;
> +       struct device device;
> +       struct list_head node;
> +
> +       /* Sensor level information */
> +       int sensor_indx; /* index into 'sensors' array */
> +       struct thermal_sensor *sensors[MAX_SENSORS_PER_ZONE];
> +};
> +
>  /* Structure that holds thermal governor information */
>  struct thermal_governor {
>         char name[THERMAL_NAME_LENGTH];
> @@ -264,6 +284,11 @@ struct thermal_sensor *thermal_sensor_register(const char *,
>  void thermal_sensor_unregister(struct thermal_sensor *);
>  int enable_sensor_thresholds(struct thermal_sensor *, int);
>
> +struct thermal_zone *create_thermal_zone(const char *, void *);
> +void remove_thermal_zone(struct thermal_zone *);
> +int add_sensor_to_zone(struct thermal_zone *, struct thermal_sensor *);
> +int add_sensor_to_zone_by_name(struct thermal_zone *, const char *);
> +
>  #ifdef CONFIG_NET
>  extern int thermal_generate_netlink_event(u32 orig, enum events event);
>  #else
> --
> 1.7.9.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
durgadoss.r@intel.com Dec. 3, 2012, 7:47 a.m. UTC | #2
> -----Original Message-----
> From: Hongbo Zhang [mailto:hongbo.zhang@linaro.org]
> Sent: Monday, December 03, 2012 1:13 PM
> To: R, Durgadoss
> Cc: Zhang, Rui; linux-pm@vger.kernel.org; wni@nvidia.com;
> eduardo.valentin@ti.com; amit.kachhap@linaro.org;
> sachin.kamat@linaro.org
> Subject: Re: [RFC PATCH 2/7] Thermal: Create zone level APIs
> 
> On 17 November 2012 18:45, Durgadoss R <durgadoss.r@intel.com> wrote:
> > This patch adds a new thermal_zone structure to
> > thermal.h. Also, adds zone level APIs to the thermal
> > framework.
> >
> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> > ---
> >  drivers/thermal/thermal_sys.c |  206
> +++++++++++++++++++++++++++++++++++++++++
> >  include/linux/thermal.h       |   25 +++++
> >  2 files changed, 231 insertions(+)
> >
> > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> > index e726c8b..9d386d7 100644
> > --- a/drivers/thermal/thermal_sys.c
> > +++ b/drivers/thermal/thermal_sys.c
> > @@ -44,19 +44,28 @@ MODULE_DESCRIPTION("Generic thermal
> management sysfs support");
> >  MODULE_LICENSE("GPL");
> >
> >  static DEFINE_IDR(thermal_tz_idr);
> > +static DEFINE_IDR(thermal_zone_idr);
> >  static DEFINE_IDR(thermal_cdev_idr);
> >  static DEFINE_IDR(thermal_sensor_idr);
> >  static DEFINE_MUTEX(thermal_idr_lock);
> >
> >  static LIST_HEAD(thermal_tz_list);
> >  static LIST_HEAD(thermal_sensor_list);
> > +static LIST_HEAD(thermal_zone_list);
> >  static LIST_HEAD(thermal_cdev_list);
> >  static LIST_HEAD(thermal_governor_list);
> >
> >  static DEFINE_MUTEX(thermal_list_lock);
> >  static DEFINE_MUTEX(ts_list_lock);
> > +static DEFINE_MUTEX(tz_list_lock);
> >  static DEFINE_MUTEX(thermal_governor_lock);
> >
> > +#define for_each_thermal_sensor(pos) \
> > +       list_for_each_entry(pos, &thermal_sensor_list, node)
> > +
> > +#define for_each_thermal_zone(pos) \
> > +       list_for_each_entry(pos, &thermal_zone_list, node)
> > +
> >  static struct thermal_governor *__find_governor(const char *name)
> >  {
> >         struct thermal_governor *pos;
> > @@ -419,15 +428,62 @@ static void thermal_zone_device_check(struct
> work_struct *work)
> >         thermal_zone_device_update(tz);
> >  }
> >
> > +static int get_sensor_indx(struct thermal_zone *tz, struct thermal_sensor
> *ts)
> > +{
> > +       int i, indx = -EINVAL;
> > +
> > +       if (!tz || !ts)
> > +               return -EINVAL;
> > +
> > +       mutex_lock(&ts_list_lock);
> > +       for (i = 0; i < tz->sensor_indx; i++) {
> > +               if (tz->sensors[i] == ts) {
> > +                       indx = i;
> > +                       break;
> > +               }
> > +       }
> > +       mutex_unlock(&ts_list_lock);
> > +       return indx;
> > +}
> > +
> > +static void remove_sensor_from_zone(struct thermal_zone *tz,
> > +                               struct thermal_sensor *ts)
> > +{
> > +       int indx, j;
> > +
> > +       indx = get_sensor_indx(tz, ts);
> > +       if (indx < 0)
> > +               return;
> > +
> > +       sysfs_remove_link(&tz->device.kobj, kobject_name(&ts-
> >device.kobj));
> > +
> > +       /* Shift the entries in the tz->sensors array */
> > +       for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++)
> > +               tz->sensors[j] = tz->sensors[j + 1];
> > +
> > +       tz->sensor_indx--;
> > +}
> > +
> >  /* sys I/F for thermal zone */
> >
> >  #define to_thermal_zone(_dev) \
> >         container_of(_dev, struct thermal_zone_device, device)
> >
> > +#define to_zone(_dev) \
> > +       container_of(_dev, struct thermal_zone, device)
> > +
> >  #define to_thermal_sensor(_dev) \
> >         container_of(_dev, struct thermal_sensor, device)
> >
> >  static ssize_t
> > +zone_name_show(struct device *dev, struct device_attribute *attr, char
> *buf)
> > +{
> > +       struct thermal_zone *tz = to_zone(dev);
> > +
> > +       return sprintf(buf, "%s\n", tz->name);
> > +}
> > +
> > +static ssize_t
> >  sensor_name_show(struct device *dev, struct device_attribute *attr, char
> *buf)
> >  {
> >         struct thermal_sensor *ts = to_thermal_sensor(dev);
> > @@ -772,6 +828,8 @@ static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR,
> policy_show, policy_store);
> >  static DEVICE_ATTR(sensor_name, 0444, sensor_name_show, NULL);
> >  static DEVICE_ATTR(temp_input, 0444, sensor_temp_show, NULL);
> >
> > +static DEVICE_ATTR(zone_name, 0444, zone_name_show, NULL);
> > +
> >  /* sys I/F for cooling device */
> >  #define to_cooling_device(_dev)        \
> >         container_of(_dev, struct thermal_cooling_device, device)
> > @@ -1557,6 +1615,146 @@ static void remove_trip_attrs(struct
> thermal_zone_device *tz)
> >         kfree(tz->trip_hyst_attrs);
> >  }
> >
> > +struct thermal_zone *create_thermal_zone(const char *name, void
> *devdata)
> > +{
> > +       struct thermal_zone *tz;
> > +       int ret;
> > +
> > +       if (!name || (name && strlen(name) >= THERMAL_NAME_LENGTH))
> > +               return ERR_PTR(-EINVAL);
> > +
> > +       tz = kzalloc(sizeof(*tz), GFP_KERNEL);
> > +       if (!tz)
> > +               return ERR_PTR(-ENOMEM);
> > +
> > +       idr_init(&tz->idr);
> > +       ret = get_idr(&thermal_zone_idr, &thermal_idr_lock, &tz->id);
> > +       if (ret)
> > +               goto exit_free;
> > +
> > +       strcpy(tz->name, name);
> > +       tz->devdata = devdata;
> > +       tz->device.class = &thermal_class;
> > +
> > +       dev_set_name(&tz->device, "zone%d", tz->id);
> > +       ret = device_register(&tz->device);
> > +       if (ret)
> > +               goto exit_idr;
> > +
> > +       ret = device_create_file(&tz->device, &dev_attr_zone_name);
> > +       if (ret)
> > +               goto exit_unregister;
> > +
> > +       /* Add this zone to the global list of thermal zones */
> > +       mutex_lock(&tz_list_lock);
> > +       list_add_tail(&tz->node, &thermal_zone_list);
> > +       mutex_unlock(&tz_list_lock);
> > +       return tz;
> > +
> > +exit_unregister:
> > +       device_unregister(&tz->device);
> > +exit_idr:
> > +       release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
> > +exit_free:
> > +       kfree(tz);
> > +       return ERR_PTR(ret);
> > +}
> > +EXPORT_SYMBOL(create_thermal_zone);
> > +
> > +void remove_thermal_zone(struct thermal_zone *tz)
> > +{
> > +       struct thermal_zone *pos, *next;
> > +       bool found = false;
> > +
> > +       if (!tz)
> > +               return;
> > +
> > +       mutex_lock(&tz_list_lock);
> > +
> > +       list_for_each_entry_safe(pos, next, &thermal_zone_list, node) {
> > +               if (pos == tz) {
> > +                       list_del(&tz->node);
> > +                       found = true;
> > +                       break;
> > +               }
> > +       }
> > +
> > +       if (!found)
> > +               goto exit;
> > +
> > +       device_remove_file(&tz->device, &dev_attr_zone_name);
> > +
> > +       release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
> > +       idr_destroy(&tz->idr);
> > +
> > +       device_unregister(&tz->device);
> > +       kfree(tz);
> > +exit:
> > +       mutex_unlock(&tz_list_lock);
> > +       return;
> > +}
> > +EXPORT_SYMBOL(remove_thermal_zone);
> > +
> > +struct thermal_sensor *get_sensor_by_name(const char *name)
> > +{
> > +       struct thermal_sensor *pos;
> > +       struct thermal_sensor *ts = NULL;
> > +
> > +       mutex_lock(&ts_list_lock);
> > +       for_each_thermal_sensor(pos) {
> > +               if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) {
> > +                       ts = pos;
> > +                       break;
> > +               }
> > +       }
> > +       mutex_unlock(&ts_list_lock);
> > +       return ts;
> > +}
> > +EXPORT_SYMBOL(get_sensor_by_name);
> > +
> > +int add_sensor_to_zone_by_name(struct thermal_zone *tz, const char
> *name)
> > +{
> > +       int ret;
> > +       struct thermal_sensor *ts = get_sensor_by_name(name);
> > +
> > +       if (!tz || !ts)
> > +               return -EINVAL;
> > +
> > +       mutex_lock(&tz_list_lock);
> > +
> > +       /* Ensure we are not adding the same sensor again!! */
> > +       ret = get_sensor_indx(tz, ts);
> > +       if (ret >= 0) {
> > +               ret = -EEXIST;
> > +               goto exit_zone;
> > +       }
> > +
> > +       mutex_lock(&ts_list_lock);
> > +
> > +       ret = sysfs_create_link(&tz->device.kobj, &ts->device.kobj,
> > +                               kobject_name(&ts->device.kobj));
> > +       if (ret)
> > +               goto exit_sensor;
> > +
> > +       tz->sensors[tz->sensor_indx++] = ts;
> > +
> > +exit_sensor:
> > +       mutex_unlock(&ts_list_lock);
> > +exit_zone:
> > +       mutex_unlock(&tz_list_lock);
> > +       return ret;
> > +}
> > +EXPORT_SYMBOL(add_sensor_to_zone_by_name);
> > +
> > +int add_sensor_to_zone(struct thermal_zone *tz, struct thermal_sensor
> *ts)
> > +{
> > +       if (!tz || !ts)
> > +               return -EINVAL;
> > +
> > +       return add_sensor_to_zone_by_name(tz, ts->name);
> > +}
> > +EXPORT_SYMBOL(add_sensor_to_zone);
> > +
> >  /**
> >   * thermal_sensor_register - register a new thermal sensor
> >   * @name:      name of the thermal sensor
> > @@ -1624,6 +1822,7 @@ EXPORT_SYMBOL(thermal_sensor_register);
> >  void thermal_sensor_unregister(struct thermal_sensor *ts)
> >  {
> >         int i;
> > +       struct thermal_zone *tz;
> >         struct thermal_sensor *pos, *next;
> >         bool found = false;
> >
> > @@ -1643,6 +1842,13 @@ void thermal_sensor_unregister(struct
> thermal_sensor *ts)
> >         if (!found)
> >                 return;
> >
> > +       mutex_lock(&tz_list_lock);
> > +
> > +       for_each_thermal_zone(tz)
> > +               remove_sensor_from_zone(tz, ts);
> > +
> > +       mutex_unlock(&tz_list_lock);
> > +
> >         for (i = 0; i < ts->thresholds; i++)
> >                 device_remove_file(&ts->device, &ts->thresh_attrs[i].attr);
> >
> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> > index e2f85ec..38438be 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -49,6 +49,11 @@
> >  /* Default Thermal Governor: Does Linear Throttling */
> >  #define DEFAULT_THERMAL_GOVERNOR       "step_wise"
> >
> > +/* Maximum number of sensors, allowed in a thermal zone
> > + * We will start with 5, and increase if needed.
> > + */
> > +#define MAX_SENSORS_PER_ZONE           5
> > +
> >  struct thermal_sensor;
> >  struct thermal_zone_device;
> >  struct thermal_cooling_device;
> > @@ -191,6 +196,21 @@ struct thermal_zone_device {
> >         struct delayed_work poll_queue;
> >  };
> >
> > +struct thermal_zone {
> > +       char name[THERMAL_NAME_LENGTH];
> > +       int id;
> > +       void *devdata;
> > +       struct thermal_zone *ops;
> What is this, thermal_zone_device_ops? and how to initialize this ops?

oh, Not required, for now. Will remove it..
Good catch :-)

Thanks,
Durga
> 
> > +       struct thermal_governor *governor;
> > +       struct idr idr;
> > +       struct device device;
> > +       struct list_head node;
> > +
> > +       /* Sensor level information */
> > +       int sensor_indx; /* index into 'sensors' array */
> > +       struct thermal_sensor *sensors[MAX_SENSORS_PER_ZONE];
> > +};
> > +
> >  /* Structure that holds thermal governor information */
> >  struct thermal_governor {
> >         char name[THERMAL_NAME_LENGTH];
> > @@ -264,6 +284,11 @@ struct thermal_sensor
> *thermal_sensor_register(const char *,
> >  void thermal_sensor_unregister(struct thermal_sensor *);
> >  int enable_sensor_thresholds(struct thermal_sensor *, int);
> >
> > +struct thermal_zone *create_thermal_zone(const char *, void *);
> > +void remove_thermal_zone(struct thermal_zone *);
> > +int add_sensor_to_zone(struct thermal_zone *, struct thermal_sensor
> *);
> > +int add_sensor_to_zone_by_name(struct thermal_zone *, const char *);
> > +
> >  #ifdef CONFIG_NET
> >  extern int thermal_generate_netlink_event(u32 orig, enum events
> event);
> >  #else
> > --
> > 1.7.9.5
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hongbo Zhang Dec. 3, 2012, 8:21 a.m. UTC | #3
On 3 December 2012 15:47, R, Durgadoss <durgadoss.r@intel.com> wrote:
>
>
>> -----Original Message-----
>> From: Hongbo Zhang [mailto:hongbo.zhang@linaro.org]
>> Sent: Monday, December 03, 2012 1:13 PM
>> To: R, Durgadoss
>> Cc: Zhang, Rui; linux-pm@vger.kernel.org; wni@nvidia.com;
>> eduardo.valentin@ti.com; amit.kachhap@linaro.org;
>> sachin.kamat@linaro.org
>> Subject: Re: [RFC PATCH 2/7] Thermal: Create zone level APIs
>>
>> On 17 November 2012 18:45, Durgadoss R <durgadoss.r@intel.com> wrote:
>> > This patch adds a new thermal_zone structure to
>> > thermal.h. Also, adds zone level APIs to the thermal
>> > framework.
>> >
>> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
>> > ---
>> >  drivers/thermal/thermal_sys.c |  206
>> +++++++++++++++++++++++++++++++++++++++++
>> >  include/linux/thermal.h       |   25 +++++
>> >  2 files changed, 231 insertions(+)
>> >
>> > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
>> > index e726c8b..9d386d7 100644
>> > --- a/drivers/thermal/thermal_sys.c
>> > +++ b/drivers/thermal/thermal_sys.c
>> > @@ -44,19 +44,28 @@ MODULE_DESCRIPTION("Generic thermal
>> management sysfs support");
>> >  MODULE_LICENSE("GPL");
>> >
>> >  static DEFINE_IDR(thermal_tz_idr);
>> > +static DEFINE_IDR(thermal_zone_idr);
>> >  static DEFINE_IDR(thermal_cdev_idr);
>> >  static DEFINE_IDR(thermal_sensor_idr);
>> >  static DEFINE_MUTEX(thermal_idr_lock);
>> >
>> >  static LIST_HEAD(thermal_tz_list);
>> >  static LIST_HEAD(thermal_sensor_list);
>> > +static LIST_HEAD(thermal_zone_list);
>> >  static LIST_HEAD(thermal_cdev_list);
>> >  static LIST_HEAD(thermal_governor_list);
>> >
>> >  static DEFINE_MUTEX(thermal_list_lock);
>> >  static DEFINE_MUTEX(ts_list_lock);
>> > +static DEFINE_MUTEX(tz_list_lock);
>> >  static DEFINE_MUTEX(thermal_governor_lock);
>> >
>> > +#define for_each_thermal_sensor(pos) \
>> > +       list_for_each_entry(pos, &thermal_sensor_list, node)
>> > +
>> > +#define for_each_thermal_zone(pos) \
>> > +       list_for_each_entry(pos, &thermal_zone_list, node)
>> > +
>> >  static struct thermal_governor *__find_governor(const char *name)
>> >  {
>> >         struct thermal_governor *pos;
>> > @@ -419,15 +428,62 @@ static void thermal_zone_device_check(struct
>> work_struct *work)
>> >         thermal_zone_device_update(tz);
>> >  }
>> >
>> > +static int get_sensor_indx(struct thermal_zone *tz, struct thermal_sensor
>> *ts)
>> > +{
>> > +       int i, indx = -EINVAL;
>> > +
>> > +       if (!tz || !ts)
>> > +               return -EINVAL;
>> > +
>> > +       mutex_lock(&ts_list_lock);
>> > +       for (i = 0; i < tz->sensor_indx; i++) {
>> > +               if (tz->sensors[i] == ts) {
>> > +                       indx = i;
>> > +                       break;
>> > +               }
>> > +       }
>> > +       mutex_unlock(&ts_list_lock);
>> > +       return indx;
>> > +}
>> > +
>> > +static void remove_sensor_from_zone(struct thermal_zone *tz,
>> > +                               struct thermal_sensor *ts)
>> > +{
>> > +       int indx, j;
>> > +
>> > +       indx = get_sensor_indx(tz, ts);
>> > +       if (indx < 0)
>> > +               return;
>> > +
>> > +       sysfs_remove_link(&tz->device.kobj, kobject_name(&ts-
>> >device.kobj));
>> > +
>> > +       /* Shift the entries in the tz->sensors array */
>> > +       for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++)
>> > +               tz->sensors[j] = tz->sensors[j + 1];
>> > +
>> > +       tz->sensor_indx--;
>> > +}
>> > +
>> >  /* sys I/F for thermal zone */
>> >
>> >  #define to_thermal_zone(_dev) \
>> >         container_of(_dev, struct thermal_zone_device, device)
>> >
>> > +#define to_zone(_dev) \
>> > +       container_of(_dev, struct thermal_zone, device)
>> > +
>> >  #define to_thermal_sensor(_dev) \
>> >         container_of(_dev, struct thermal_sensor, device)
>> >
>> >  static ssize_t
>> > +zone_name_show(struct device *dev, struct device_attribute *attr, char
>> *buf)
>> > +{
>> > +       struct thermal_zone *tz = to_zone(dev);
>> > +
>> > +       return sprintf(buf, "%s\n", tz->name);
>> > +}
>> > +
>> > +static ssize_t
>> >  sensor_name_show(struct device *dev, struct device_attribute *attr, char
>> *buf)
>> >  {
>> >         struct thermal_sensor *ts = to_thermal_sensor(dev);
>> > @@ -772,6 +828,8 @@ static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR,
>> policy_show, policy_store);
>> >  static DEVICE_ATTR(sensor_name, 0444, sensor_name_show, NULL);
>> >  static DEVICE_ATTR(temp_input, 0444, sensor_temp_show, NULL);
>> >
>> > +static DEVICE_ATTR(zone_name, 0444, zone_name_show, NULL);
>> > +
>> >  /* sys I/F for cooling device */
>> >  #define to_cooling_device(_dev)        \
>> >         container_of(_dev, struct thermal_cooling_device, device)
>> > @@ -1557,6 +1615,146 @@ static void remove_trip_attrs(struct
>> thermal_zone_device *tz)
>> >         kfree(tz->trip_hyst_attrs);
>> >  }
>> >
>> > +struct thermal_zone *create_thermal_zone(const char *name, void
>> *devdata)
>> > +{
>> > +       struct thermal_zone *tz;
>> > +       int ret;
>> > +
>> > +       if (!name || (name && strlen(name) >= THERMAL_NAME_LENGTH))
>> > +               return ERR_PTR(-EINVAL);
>> > +
>> > +       tz = kzalloc(sizeof(*tz), GFP_KERNEL);
>> > +       if (!tz)
>> > +               return ERR_PTR(-ENOMEM);
>> > +
>> > +       idr_init(&tz->idr);
>> > +       ret = get_idr(&thermal_zone_idr, &thermal_idr_lock, &tz->id);
>> > +       if (ret)
>> > +               goto exit_free;
>> > +
>> > +       strcpy(tz->name, name);
>> > +       tz->devdata = devdata;
>> > +       tz->device.class = &thermal_class;
>> > +
>> > +       dev_set_name(&tz->device, "zone%d", tz->id);
>> > +       ret = device_register(&tz->device);
>> > +       if (ret)
>> > +               goto exit_idr;
>> > +
>> > +       ret = device_create_file(&tz->device, &dev_attr_zone_name);
>> > +       if (ret)
>> > +               goto exit_unregister;
>> > +
>> > +       /* Add this zone to the global list of thermal zones */
>> > +       mutex_lock(&tz_list_lock);
>> > +       list_add_tail(&tz->node, &thermal_zone_list);
>> > +       mutex_unlock(&tz_list_lock);
>> > +       return tz;
>> > +
>> > +exit_unregister:
>> > +       device_unregister(&tz->device);
>> > +exit_idr:
>> > +       release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
>> > +exit_free:
>> > +       kfree(tz);
>> > +       return ERR_PTR(ret);
>> > +}
>> > +EXPORT_SYMBOL(create_thermal_zone);
>> > +
>> > +void remove_thermal_zone(struct thermal_zone *tz)
>> > +{
>> > +       struct thermal_zone *pos, *next;
>> > +       bool found = false;
>> > +
>> > +       if (!tz)
>> > +               return;
>> > +
>> > +       mutex_lock(&tz_list_lock);
>> > +
>> > +       list_for_each_entry_safe(pos, next, &thermal_zone_list, node) {
>> > +               if (pos == tz) {
>> > +                       list_del(&tz->node);
>> > +                       found = true;
>> > +                       break;
>> > +               }
>> > +       }
>> > +
>> > +       if (!found)
>> > +               goto exit;
>> > +
>> > +       device_remove_file(&tz->device, &dev_attr_zone_name);
>> > +
>> > +       release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
>> > +       idr_destroy(&tz->idr);
>> > +
>> > +       device_unregister(&tz->device);
>> > +       kfree(tz);
>> > +exit:
>> > +       mutex_unlock(&tz_list_lock);
>> > +       return;
>> > +}
>> > +EXPORT_SYMBOL(remove_thermal_zone);
>> > +
>> > +struct thermal_sensor *get_sensor_by_name(const char *name)
>> > +{
>> > +       struct thermal_sensor *pos;
>> > +       struct thermal_sensor *ts = NULL;
>> > +
>> > +       mutex_lock(&ts_list_lock);
>> > +       for_each_thermal_sensor(pos) {
>> > +               if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) {
>> > +                       ts = pos;
>> > +                       break;
>> > +               }
>> > +       }
>> > +       mutex_unlock(&ts_list_lock);
>> > +       return ts;
>> > +}
>> > +EXPORT_SYMBOL(get_sensor_by_name);
>> > +
>> > +int add_sensor_to_zone_by_name(struct thermal_zone *tz, const char
>> *name)
>> > +{
>> > +       int ret;
>> > +       struct thermal_sensor *ts = get_sensor_by_name(name);
>> > +
>> > +       if (!tz || !ts)
>> > +               return -EINVAL;
>> > +
>> > +       mutex_lock(&tz_list_lock);
>> > +
>> > +       /* Ensure we are not adding the same sensor again!! */
>> > +       ret = get_sensor_indx(tz, ts);
>> > +       if (ret >= 0) {
>> > +               ret = -EEXIST;
>> > +               goto exit_zone;
>> > +       }
>> > +
>> > +       mutex_lock(&ts_list_lock);
>> > +
>> > +       ret = sysfs_create_link(&tz->device.kobj, &ts->device.kobj,
>> > +                               kobject_name(&ts->device.kobj));
>> > +       if (ret)
>> > +               goto exit_sensor;
>> > +
>> > +       tz->sensors[tz->sensor_indx++] = ts;
>> > +
>> > +exit_sensor:
>> > +       mutex_unlock(&ts_list_lock);
>> > +exit_zone:
>> > +       mutex_unlock(&tz_list_lock);
>> > +       return ret;
>> > +}
>> > +EXPORT_SYMBOL(add_sensor_to_zone_by_name);
>> > +
>> > +int add_sensor_to_zone(struct thermal_zone *tz, struct thermal_sensor
>> *ts)
>> > +{
>> > +       if (!tz || !ts)
>> > +               return -EINVAL;
>> > +
>> > +       return add_sensor_to_zone_by_name(tz, ts->name);
>> > +}
>> > +EXPORT_SYMBOL(add_sensor_to_zone);
>> > +
>> >  /**
>> >   * thermal_sensor_register - register a new thermal sensor
>> >   * @name:      name of the thermal sensor
>> > @@ -1624,6 +1822,7 @@ EXPORT_SYMBOL(thermal_sensor_register);
>> >  void thermal_sensor_unregister(struct thermal_sensor *ts)
>> >  {
>> >         int i;
>> > +       struct thermal_zone *tz;
>> >         struct thermal_sensor *pos, *next;
>> >         bool found = false;
>> >
>> > @@ -1643,6 +1842,13 @@ void thermal_sensor_unregister(struct
>> thermal_sensor *ts)
>> >         if (!found)
>> >                 return;
>> >
>> > +       mutex_lock(&tz_list_lock);
>> > +
>> > +       for_each_thermal_zone(tz)
>> > +               remove_sensor_from_zone(tz, ts);
>> > +
>> > +       mutex_unlock(&tz_list_lock);
>> > +
>> >         for (i = 0; i < ts->thresholds; i++)
>> >                 device_remove_file(&ts->device, &ts->thresh_attrs[i].attr);
>> >
>> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> > index e2f85ec..38438be 100644
>> > --- a/include/linux/thermal.h
>> > +++ b/include/linux/thermal.h
>> > @@ -49,6 +49,11 @@
>> >  /* Default Thermal Governor: Does Linear Throttling */
>> >  #define DEFAULT_THERMAL_GOVERNOR       "step_wise"
>> >
>> > +/* Maximum number of sensors, allowed in a thermal zone
>> > + * We will start with 5, and increase if needed.
>> > + */
>> > +#define MAX_SENSORS_PER_ZONE           5
>> > +
>> >  struct thermal_sensor;
>> >  struct thermal_zone_device;
>> >  struct thermal_cooling_device;
>> > @@ -191,6 +196,21 @@ struct thermal_zone_device {
>> >         struct delayed_work poll_queue;
>> >  };
>> >
>> > +struct thermal_zone {
>> > +       char name[THERMAL_NAME_LENGTH];
>> > +       int id;
>> > +       void *devdata;
>> > +       struct thermal_zone *ops;
>> What is this, thermal_zone_device_ops? and how to initialize this ops?
>
> oh, Not required, for now. Will remove it..
> Good catch :-)

I am still confused a bit.
Is this thermal_zone going to take the place of the old thermal_zone_device?
If yes, which functions/callbacks are used to access
thermal_zone->trip[] like the old get_trip_*?

>
> Thanks,
> Durga
>>
>> > +       struct thermal_governor *governor;
>> > +       struct idr idr;
>> > +       struct device device;
>> > +       struct list_head node;
>> > +
>> > +       /* Sensor level information */
>> > +       int sensor_indx; /* index into 'sensors' array */
>> > +       struct thermal_sensor *sensors[MAX_SENSORS_PER_ZONE];
>> > +};
>> > +
>> >  /* Structure that holds thermal governor information */
>> >  struct thermal_governor {
>> >         char name[THERMAL_NAME_LENGTH];
>> > @@ -264,6 +284,11 @@ struct thermal_sensor
>> *thermal_sensor_register(const char *,
>> >  void thermal_sensor_unregister(struct thermal_sensor *);
>> >  int enable_sensor_thresholds(struct thermal_sensor *, int);
>> >
>> > +struct thermal_zone *create_thermal_zone(const char *, void *);
>> > +void remove_thermal_zone(struct thermal_zone *);
>> > +int add_sensor_to_zone(struct thermal_zone *, struct thermal_sensor
>> *);
>> > +int add_sensor_to_zone_by_name(struct thermal_zone *, const char *);
>> > +
>> >  #ifdef CONFIG_NET
>> >  extern int thermal_generate_netlink_event(u32 orig, enum events
>> event);
>> >  #else
>> > --
>> > 1.7.9.5
>> >
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
durgadoss.r@intel.com Dec. 3, 2012, 9:51 a.m. UTC | #4
> -----Original Message-----
> From: Hongbo Zhang [mailto:hongbo.zhang@linaro.org]
> Sent: Monday, December 03, 2012 1:51 PM
> To: R, Durgadoss
> Cc: Zhang, Rui; linux-pm@vger.kernel.org; wni@nvidia.com;
> eduardo.valentin@ti.com; amit.kachhap@linaro.org;
> sachin.kamat@linaro.org
> Subject: Re: [RFC PATCH 2/7] Thermal: Create zone level APIs
> 
> On 3 December 2012 15:47, R, Durgadoss <durgadoss.r@intel.com> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Hongbo Zhang [mailto:hongbo.zhang@linaro.org]
> >> Sent: Monday, December 03, 2012 1:13 PM
> >> To: R, Durgadoss
> >> Cc: Zhang, Rui; linux-pm@vger.kernel.org; wni@nvidia.com;
> >> eduardo.valentin@ti.com; amit.kachhap@linaro.org;
> >> sachin.kamat@linaro.org
> >> Subject: Re: [RFC PATCH 2/7] Thermal: Create zone level APIs
> >>
> >> On 17 November 2012 18:45, Durgadoss R <durgadoss.r@intel.com>
> wrote:
> >> > This patch adds a new thermal_zone structure to
> >> > thermal.h. Also, adds zone level APIs to the thermal
> >> > framework.
> >> >
> >> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> >> > ---
> >> >  drivers/thermal/thermal_sys.c |  206
> >> +++++++++++++++++++++++++++++++++++++++++
> >> >  include/linux/thermal.h       |   25 +++++
> >> >  2 files changed, 231 insertions(+)
> >> >
> >> > diff --git a/drivers/thermal/thermal_sys.c
> b/drivers/thermal/thermal_sys.c
> >> > index e726c8b..9d386d7 100644
> >> > --- a/drivers/thermal/thermal_sys.c
> >> > +++ b/drivers/thermal/thermal_sys.c
> >> > @@ -44,19 +44,28 @@ MODULE_DESCRIPTION("Generic thermal
> >> management sysfs support");
> >> >  MODULE_LICENSE("GPL");
> >> >
> >> >  static DEFINE_IDR(thermal_tz_idr);
> >> > +static DEFINE_IDR(thermal_zone_idr);
> >> >  static DEFINE_IDR(thermal_cdev_idr);
> >> >  static DEFINE_IDR(thermal_sensor_idr);
> >> >  static DEFINE_MUTEX(thermal_idr_lock);
> >> >
> >> >  static LIST_HEAD(thermal_tz_list);
> >> >  static LIST_HEAD(thermal_sensor_list);
> >> > +static LIST_HEAD(thermal_zone_list);
> >> >  static LIST_HEAD(thermal_cdev_list);
> >> >  static LIST_HEAD(thermal_governor_list);
> >> >
> >> >  static DEFINE_MUTEX(thermal_list_lock);
> >> >  static DEFINE_MUTEX(ts_list_lock);
> >> > +static DEFINE_MUTEX(tz_list_lock);
> >> >  static DEFINE_MUTEX(thermal_governor_lock);
> >> >
> >> > +#define for_each_thermal_sensor(pos) \
> >> > +       list_for_each_entry(pos, &thermal_sensor_list, node)
> >> > +
> >> > +#define for_each_thermal_zone(pos) \
> >> > +       list_for_each_entry(pos, &thermal_zone_list, node)
> >> > +
> >> >  static struct thermal_governor *__find_governor(const char *name)
> >> >  {
> >> >         struct thermal_governor *pos;
> >> > @@ -419,15 +428,62 @@ static void thermal_zone_device_check(struct
> >> work_struct *work)
> >> >         thermal_zone_device_update(tz);
> >> >  }
> >> >
> >> > +static int get_sensor_indx(struct thermal_zone *tz, struct
> thermal_sensor
> >> *ts)
> >> > +{
> >> > +       int i, indx = -EINVAL;
> >> > +
> >> > +       if (!tz || !ts)
> >> > +               return -EINVAL;
> >> > +
> >> > +       mutex_lock(&ts_list_lock);
> >> > +       for (i = 0; i < tz->sensor_indx; i++) {
> >> > +               if (tz->sensors[i] == ts) {
> >> > +                       indx = i;
> >> > +                       break;
> >> > +               }
> >> > +       }
> >> > +       mutex_unlock(&ts_list_lock);
> >> > +       return indx;
> >> > +}
> >> > +
> >> > +static void remove_sensor_from_zone(struct thermal_zone *tz,
> >> > +                               struct thermal_sensor *ts)
> >> > +{
> >> > +       int indx, j;
> >> > +
> >> > +       indx = get_sensor_indx(tz, ts);
> >> > +       if (indx < 0)
> >> > +               return;
> >> > +
> >> > +       sysfs_remove_link(&tz->device.kobj, kobject_name(&ts-
> >> >device.kobj));
> >> > +
> >> > +       /* Shift the entries in the tz->sensors array */
> >> > +       for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++)
> >> > +               tz->sensors[j] = tz->sensors[j + 1];
> >> > +
> >> > +       tz->sensor_indx--;
> >> > +}
> >> > +
> >> >  /* sys I/F for thermal zone */
> >> >
> >> >  #define to_thermal_zone(_dev) \
> >> >         container_of(_dev, struct thermal_zone_device, device)
> >> >
> >> > +#define to_zone(_dev) \
> >> > +       container_of(_dev, struct thermal_zone, device)
> >> > +
> >> >  #define to_thermal_sensor(_dev) \
> >> >         container_of(_dev, struct thermal_sensor, device)
> >> >
> >> >  static ssize_t
> >> > +zone_name_show(struct device *dev, struct device_attribute *attr,
> char
> >> *buf)
> >> > +{
> >> > +       struct thermal_zone *tz = to_zone(dev);
> >> > +
> >> > +       return sprintf(buf, "%s\n", tz->name);
> >> > +}
> >> > +
> >> > +static ssize_t
> >> >  sensor_name_show(struct device *dev, struct device_attribute *attr,
> char
> >> *buf)
> >> >  {
> >> >         struct thermal_sensor *ts = to_thermal_sensor(dev);
> >> > @@ -772,6 +828,8 @@ static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR,
> >> policy_show, policy_store);
> >> >  static DEVICE_ATTR(sensor_name, 0444, sensor_name_show, NULL);
> >> >  static DEVICE_ATTR(temp_input, 0444, sensor_temp_show, NULL);
> >> >
> >> > +static DEVICE_ATTR(zone_name, 0444, zone_name_show, NULL);
> >> > +
> >> >  /* sys I/F for cooling device */
> >> >  #define to_cooling_device(_dev)        \
> >> >         container_of(_dev, struct thermal_cooling_device, device)
> >> > @@ -1557,6 +1615,146 @@ static void remove_trip_attrs(struct
> >> thermal_zone_device *tz)
> >> >         kfree(tz->trip_hyst_attrs);
> >> >  }
> >> >
> >> > +struct thermal_zone *create_thermal_zone(const char *name, void
> >> *devdata)
> >> > +{
> >> > +       struct thermal_zone *tz;
> >> > +       int ret;
> >> > +
> >> > +       if (!name || (name && strlen(name) >=
> THERMAL_NAME_LENGTH))
> >> > +               return ERR_PTR(-EINVAL);
> >> > +
> >> > +       tz = kzalloc(sizeof(*tz), GFP_KERNEL);
> >> > +       if (!tz)
> >> > +               return ERR_PTR(-ENOMEM);
> >> > +
> >> > +       idr_init(&tz->idr);
> >> > +       ret = get_idr(&thermal_zone_idr, &thermal_idr_lock, &tz->id);
> >> > +       if (ret)
> >> > +               goto exit_free;
> >> > +
> >> > +       strcpy(tz->name, name);
> >> > +       tz->devdata = devdata;
> >> > +       tz->device.class = &thermal_class;
> >> > +
> >> > +       dev_set_name(&tz->device, "zone%d", tz->id);
> >> > +       ret = device_register(&tz->device);
> >> > +       if (ret)
> >> > +               goto exit_idr;
> >> > +
> >> > +       ret = device_create_file(&tz->device, &dev_attr_zone_name);
> >> > +       if (ret)
> >> > +               goto exit_unregister;
> >> > +
> >> > +       /* Add this zone to the global list of thermal zones */
> >> > +       mutex_lock(&tz_list_lock);
> >> > +       list_add_tail(&tz->node, &thermal_zone_list);
> >> > +       mutex_unlock(&tz_list_lock);
> >> > +       return tz;
> >> > +
> >> > +exit_unregister:
> >> > +       device_unregister(&tz->device);
> >> > +exit_idr:
> >> > +       release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
> >> > +exit_free:
> >> > +       kfree(tz);
> >> > +       return ERR_PTR(ret);
> >> > +}
> >> > +EXPORT_SYMBOL(create_thermal_zone);
> >> > +
> >> > +void remove_thermal_zone(struct thermal_zone *tz)
> >> > +{
> >> > +       struct thermal_zone *pos, *next;
> >> > +       bool found = false;
> >> > +
> >> > +       if (!tz)
> >> > +               return;
> >> > +
> >> > +       mutex_lock(&tz_list_lock);
> >> > +
> >> > +       list_for_each_entry_safe(pos, next, &thermal_zone_list, node) {
> >> > +               if (pos == tz) {
> >> > +                       list_del(&tz->node);
> >> > +                       found = true;
> >> > +                       break;
> >> > +               }
> >> > +       }
> >> > +
> >> > +       if (!found)
> >> > +               goto exit;
> >> > +
> >> > +       device_remove_file(&tz->device, &dev_attr_zone_name);
> >> > +
> >> > +       release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
> >> > +       idr_destroy(&tz->idr);
> >> > +
> >> > +       device_unregister(&tz->device);
> >> > +       kfree(tz);
> >> > +exit:
> >> > +       mutex_unlock(&tz_list_lock);
> >> > +       return;
> >> > +}
> >> > +EXPORT_SYMBOL(remove_thermal_zone);
> >> > +
> >> > +struct thermal_sensor *get_sensor_by_name(const char *name)
> >> > +{
> >> > +       struct thermal_sensor *pos;
> >> > +       struct thermal_sensor *ts = NULL;
> >> > +
> >> > +       mutex_lock(&ts_list_lock);
> >> > +       for_each_thermal_sensor(pos) {
> >> > +               if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) {
> >> > +                       ts = pos;
> >> > +                       break;
> >> > +               }
> >> > +       }
> >> > +       mutex_unlock(&ts_list_lock);
> >> > +       return ts;
> >> > +}
> >> > +EXPORT_SYMBOL(get_sensor_by_name);
> >> > +
> >> > +int add_sensor_to_zone_by_name(struct thermal_zone *tz, const
> char
> >> *name)
> >> > +{
> >> > +       int ret;
> >> > +       struct thermal_sensor *ts = get_sensor_by_name(name);
> >> > +
> >> > +       if (!tz || !ts)
> >> > +               return -EINVAL;
> >> > +
> >> > +       mutex_lock(&tz_list_lock);
> >> > +
> >> > +       /* Ensure we are not adding the same sensor again!! */
> >> > +       ret = get_sensor_indx(tz, ts);
> >> > +       if (ret >= 0) {
> >> > +               ret = -EEXIST;
> >> > +               goto exit_zone;
> >> > +       }
> >> > +
> >> > +       mutex_lock(&ts_list_lock);
> >> > +
> >> > +       ret = sysfs_create_link(&tz->device.kobj, &ts->device.kobj,
> >> > +                               kobject_name(&ts->device.kobj));
> >> > +       if (ret)
> >> > +               goto exit_sensor;
> >> > +
> >> > +       tz->sensors[tz->sensor_indx++] = ts;
> >> > +
> >> > +exit_sensor:
> >> > +       mutex_unlock(&ts_list_lock);
> >> > +exit_zone:
> >> > +       mutex_unlock(&tz_list_lock);
> >> > +       return ret;
> >> > +}
> >> > +EXPORT_SYMBOL(add_sensor_to_zone_by_name);
> >> > +
> >> > +int add_sensor_to_zone(struct thermal_zone *tz, struct
> thermal_sensor
> >> *ts)
> >> > +{
> >> > +       if (!tz || !ts)
> >> > +               return -EINVAL;
> >> > +
> >> > +       return add_sensor_to_zone_by_name(tz, ts->name);
> >> > +}
> >> > +EXPORT_SYMBOL(add_sensor_to_zone);
> >> > +
> >> >  /**
> >> >   * thermal_sensor_register - register a new thermal sensor
> >> >   * @name:      name of the thermal sensor
> >> > @@ -1624,6 +1822,7 @@ EXPORT_SYMBOL(thermal_sensor_register);
> >> >  void thermal_sensor_unregister(struct thermal_sensor *ts)
> >> >  {
> >> >         int i;
> >> > +       struct thermal_zone *tz;
> >> >         struct thermal_sensor *pos, *next;
> >> >         bool found = false;
> >> >
> >> > @@ -1643,6 +1842,13 @@ void thermal_sensor_unregister(struct
> >> thermal_sensor *ts)
> >> >         if (!found)
> >> >                 return;
> >> >
> >> > +       mutex_lock(&tz_list_lock);
> >> > +
> >> > +       for_each_thermal_zone(tz)
> >> > +               remove_sensor_from_zone(tz, ts);
> >> > +
> >> > +       mutex_unlock(&tz_list_lock);
> >> > +
> >> >         for (i = 0; i < ts->thresholds; i++)
> >> >                 device_remove_file(&ts->device, &ts->thresh_attrs[i].attr);
> >> >
> >> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> >> > index e2f85ec..38438be 100644
> >> > --- a/include/linux/thermal.h
> >> > +++ b/include/linux/thermal.h
> >> > @@ -49,6 +49,11 @@
> >> >  /* Default Thermal Governor: Does Linear Throttling */
> >> >  #define DEFAULT_THERMAL_GOVERNOR       "step_wise"
> >> >
> >> > +/* Maximum number of sensors, allowed in a thermal zone
> >> > + * We will start with 5, and increase if needed.
> >> > + */
> >> > +#define MAX_SENSORS_PER_ZONE           5
> >> > +
> >> >  struct thermal_sensor;
> >> >  struct thermal_zone_device;
> >> >  struct thermal_cooling_device;
> >> > @@ -191,6 +196,21 @@ struct thermal_zone_device {
> >> >         struct delayed_work poll_queue;
> >> >  };
> >> >
> >> > +struct thermal_zone {
> >> > +       char name[THERMAL_NAME_LENGTH];
> >> > +       int id;
> >> > +       void *devdata;
> >> > +       struct thermal_zone *ops;
> >> What is this, thermal_zone_device_ops? and how to initialize this ops?
> >
> > oh, Not required, for now. Will remove it..
> > Good catch :-)
> 
> I am still confused a bit.
> Is this thermal_zone going to take the place of the old
> thermal_zone_device?

Yes, it will replace the older one.

> If yes, which functions/callbacks are used to access
> thermal_zone->trip[] like the old get_trip_*?

We will not have call backs associated with 'zone'.
A zone is kind of a virtual entity, which can have one or
more sensors associated with it.

Trip points are attached to 'a sensor' which can be in
any zone. So, thermal_sensor_ops functions will help you
access the trip values.

Thanks,
Durga

> 
> >
> > Thanks,
> > Durga
> >>
> >> > +       struct thermal_governor *governor;
> >> > +       struct idr idr;
> >> > +       struct device device;
> >> > +       struct list_head node;
> >> > +
> >> > +       /* Sensor level information */
> >> > +       int sensor_indx; /* index into 'sensors' array */
> >> > +       struct thermal_sensor *sensors[MAX_SENSORS_PER_ZONE];
> >> > +};
> >> > +
> >> >  /* Structure that holds thermal governor information */
> >> >  struct thermal_governor {
> >> >         char name[THERMAL_NAME_LENGTH];
> >> > @@ -264,6 +284,11 @@ struct thermal_sensor
> >> *thermal_sensor_register(const char *,
> >> >  void thermal_sensor_unregister(struct thermal_sensor *);
> >> >  int enable_sensor_thresholds(struct thermal_sensor *, int);
> >> >
> >> > +struct thermal_zone *create_thermal_zone(const char *, void *);
> >> > +void remove_thermal_zone(struct thermal_zone *);
> >> > +int add_sensor_to_zone(struct thermal_zone *, struct thermal_sensor
> >> *);
> >> > +int add_sensor_to_zone_by_name(struct thermal_zone *, const char
> *);
> >> > +
> >> >  #ifdef CONFIG_NET
> >> >  extern int thermal_generate_netlink_event(u32 orig, enum events
> >> event);
> >> >  #else
> >> > --
> >> > 1.7.9.5
> >> >
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hongbo Zhang Dec. 3, 2012, 11:50 a.m. UTC | #5
On 3 December 2012 17:51, R, Durgadoss <durgadoss.r@intel.com> wrote:
>
>
>> -----Original Message-----
>> From: Hongbo Zhang [mailto:hongbo.zhang@linaro.org]
>> Sent: Monday, December 03, 2012 1:51 PM
>> To: R, Durgadoss
>> Cc: Zhang, Rui; linux-pm@vger.kernel.org; wni@nvidia.com;
>> eduardo.valentin@ti.com; amit.kachhap@linaro.org;
>> sachin.kamat@linaro.org
>> Subject: Re: [RFC PATCH 2/7] Thermal: Create zone level APIs
>>
>> On 3 December 2012 15:47, R, Durgadoss <durgadoss.r@intel.com> wrote:
>> >
>> >
>> >> -----Original Message-----
>> >> From: Hongbo Zhang [mailto:hongbo.zhang@linaro.org]
>> >> Sent: Monday, December 03, 2012 1:13 PM
>> >> To: R, Durgadoss
>> >> Cc: Zhang, Rui; linux-pm@vger.kernel.org; wni@nvidia.com;
>> >> eduardo.valentin@ti.com; amit.kachhap@linaro.org;
>> >> sachin.kamat@linaro.org
>> >> Subject: Re: [RFC PATCH 2/7] Thermal: Create zone level APIs
>> >>
>> >> On 17 November 2012 18:45, Durgadoss R <durgadoss.r@intel.com>
>> wrote:
>> >> > This patch adds a new thermal_zone structure to
>> >> > thermal.h. Also, adds zone level APIs to the thermal
>> >> > framework.
>> >> >
>> >> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
>> >> > ---
>> >> >  drivers/thermal/thermal_sys.c |  206
>> >> +++++++++++++++++++++++++++++++++++++++++
>> >> >  include/linux/thermal.h       |   25 +++++
>> >> >  2 files changed, 231 insertions(+)
>> >> >
>> >> > diff --git a/drivers/thermal/thermal_sys.c
>> b/drivers/thermal/thermal_sys.c
>> >> > index e726c8b..9d386d7 100644
>> >> > --- a/drivers/thermal/thermal_sys.c
>> >> > +++ b/drivers/thermal/thermal_sys.c
>> >> > @@ -44,19 +44,28 @@ MODULE_DESCRIPTION("Generic thermal
>> >> management sysfs support");
>> >> >  MODULE_LICENSE("GPL");
>> >> >
>> >> >  static DEFINE_IDR(thermal_tz_idr);
>> >> > +static DEFINE_IDR(thermal_zone_idr);
>> >> >  static DEFINE_IDR(thermal_cdev_idr);
>> >> >  static DEFINE_IDR(thermal_sensor_idr);
>> >> >  static DEFINE_MUTEX(thermal_idr_lock);
>> >> >
>> >> >  static LIST_HEAD(thermal_tz_list);
>> >> >  static LIST_HEAD(thermal_sensor_list);
>> >> > +static LIST_HEAD(thermal_zone_list);
>> >> >  static LIST_HEAD(thermal_cdev_list);
>> >> >  static LIST_HEAD(thermal_governor_list);
>> >> >
>> >> >  static DEFINE_MUTEX(thermal_list_lock);
>> >> >  static DEFINE_MUTEX(ts_list_lock);
>> >> > +static DEFINE_MUTEX(tz_list_lock);
>> >> >  static DEFINE_MUTEX(thermal_governor_lock);
>> >> >
>> >> > +#define for_each_thermal_sensor(pos) \
>> >> > +       list_for_each_entry(pos, &thermal_sensor_list, node)
>> >> > +
>> >> > +#define for_each_thermal_zone(pos) \
>> >> > +       list_for_each_entry(pos, &thermal_zone_list, node)
>> >> > +
>> >> >  static struct thermal_governor *__find_governor(const char *name)
>> >> >  {
>> >> >         struct thermal_governor *pos;
>> >> > @@ -419,15 +428,62 @@ static void thermal_zone_device_check(struct
>> >> work_struct *work)
>> >> >         thermal_zone_device_update(tz);
>> >> >  }
>> >> >
>> >> > +static int get_sensor_indx(struct thermal_zone *tz, struct
>> thermal_sensor
>> >> *ts)
>> >> > +{
>> >> > +       int i, indx = -EINVAL;
>> >> > +
>> >> > +       if (!tz || !ts)
>> >> > +               return -EINVAL;
>> >> > +
>> >> > +       mutex_lock(&ts_list_lock);
>> >> > +       for (i = 0; i < tz->sensor_indx; i++) {
>> >> > +               if (tz->sensors[i] == ts) {
>> >> > +                       indx = i;
>> >> > +                       break;
>> >> > +               }
>> >> > +       }
>> >> > +       mutex_unlock(&ts_list_lock);
>> >> > +       return indx;
>> >> > +}
>> >> > +
>> >> > +static void remove_sensor_from_zone(struct thermal_zone *tz,
>> >> > +                               struct thermal_sensor *ts)
>> >> > +{
>> >> > +       int indx, j;
>> >> > +
>> >> > +       indx = get_sensor_indx(tz, ts);
>> >> > +       if (indx < 0)
>> >> > +               return;
>> >> > +
>> >> > +       sysfs_remove_link(&tz->device.kobj, kobject_name(&ts-
>> >> >device.kobj));
>> >> > +
>> >> > +       /* Shift the entries in the tz->sensors array */
>> >> > +       for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++)
>> >> > +               tz->sensors[j] = tz->sensors[j + 1];
>> >> > +
>> >> > +       tz->sensor_indx--;
>> >> > +}
>> >> > +
>> >> >  /* sys I/F for thermal zone */
>> >> >
>> >> >  #define to_thermal_zone(_dev) \
>> >> >         container_of(_dev, struct thermal_zone_device, device)
>> >> >
>> >> > +#define to_zone(_dev) \
>> >> > +       container_of(_dev, struct thermal_zone, device)
>> >> > +
>> >> >  #define to_thermal_sensor(_dev) \
>> >> >         container_of(_dev, struct thermal_sensor, device)
>> >> >
>> >> >  static ssize_t
>> >> > +zone_name_show(struct device *dev, struct device_attribute *attr,
>> char
>> >> *buf)
>> >> > +{
>> >> > +       struct thermal_zone *tz = to_zone(dev);
>> >> > +
>> >> > +       return sprintf(buf, "%s\n", tz->name);
>> >> > +}
>> >> > +
>> >> > +static ssize_t
>> >> >  sensor_name_show(struct device *dev, struct device_attribute *attr,
>> char
>> >> *buf)
>> >> >  {
>> >> >         struct thermal_sensor *ts = to_thermal_sensor(dev);
>> >> > @@ -772,6 +828,8 @@ static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR,
>> >> policy_show, policy_store);
>> >> >  static DEVICE_ATTR(sensor_name, 0444, sensor_name_show, NULL);
>> >> >  static DEVICE_ATTR(temp_input, 0444, sensor_temp_show, NULL);
>> >> >
>> >> > +static DEVICE_ATTR(zone_name, 0444, zone_name_show, NULL);
>> >> > +
>> >> >  /* sys I/F for cooling device */
>> >> >  #define to_cooling_device(_dev)        \
>> >> >         container_of(_dev, struct thermal_cooling_device, device)
>> >> > @@ -1557,6 +1615,146 @@ static void remove_trip_attrs(struct
>> >> thermal_zone_device *tz)
>> >> >         kfree(tz->trip_hyst_attrs);
>> >> >  }
>> >> >
>> >> > +struct thermal_zone *create_thermal_zone(const char *name, void
>> >> *devdata)
>> >> > +{
>> >> > +       struct thermal_zone *tz;
>> >> > +       int ret;
>> >> > +
>> >> > +       if (!name || (name && strlen(name) >=
>> THERMAL_NAME_LENGTH))
>> >> > +               return ERR_PTR(-EINVAL);
>> >> > +
>> >> > +       tz = kzalloc(sizeof(*tz), GFP_KERNEL);
>> >> > +       if (!tz)
>> >> > +               return ERR_PTR(-ENOMEM);
>> >> > +
>> >> > +       idr_init(&tz->idr);
>> >> > +       ret = get_idr(&thermal_zone_idr, &thermal_idr_lock, &tz->id);
>> >> > +       if (ret)
>> >> > +               goto exit_free;
>> >> > +
>> >> > +       strcpy(tz->name, name);
>> >> > +       tz->devdata = devdata;
>> >> > +       tz->device.class = &thermal_class;
>> >> > +
>> >> > +       dev_set_name(&tz->device, "zone%d", tz->id);
>> >> > +       ret = device_register(&tz->device);
>> >> > +       if (ret)
>> >> > +               goto exit_idr;
>> >> > +
>> >> > +       ret = device_create_file(&tz->device, &dev_attr_zone_name);
>> >> > +       if (ret)
>> >> > +               goto exit_unregister;
>> >> > +
>> >> > +       /* Add this zone to the global list of thermal zones */
>> >> > +       mutex_lock(&tz_list_lock);
>> >> > +       list_add_tail(&tz->node, &thermal_zone_list);
>> >> > +       mutex_unlock(&tz_list_lock);
>> >> > +       return tz;
>> >> > +
>> >> > +exit_unregister:
>> >> > +       device_unregister(&tz->device);
>> >> > +exit_idr:
>> >> > +       release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
>> >> > +exit_free:
>> >> > +       kfree(tz);
>> >> > +       return ERR_PTR(ret);
>> >> > +}
>> >> > +EXPORT_SYMBOL(create_thermal_zone);
>> >> > +
>> >> > +void remove_thermal_zone(struct thermal_zone *tz)
>> >> > +{
>> >> > +       struct thermal_zone *pos, *next;
>> >> > +       bool found = false;
>> >> > +
>> >> > +       if (!tz)
>> >> > +               return;
>> >> > +
>> >> > +       mutex_lock(&tz_list_lock);
>> >> > +
>> >> > +       list_for_each_entry_safe(pos, next, &thermal_zone_list, node) {
>> >> > +               if (pos == tz) {
>> >> > +                       list_del(&tz->node);
>> >> > +                       found = true;
>> >> > +                       break;
>> >> > +               }
>> >> > +       }
>> >> > +
>> >> > +       if (!found)
>> >> > +               goto exit;
>> >> > +
>> >> > +       device_remove_file(&tz->device, &dev_attr_zone_name);
>> >> > +
>> >> > +       release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
>> >> > +       idr_destroy(&tz->idr);
>> >> > +
>> >> > +       device_unregister(&tz->device);
>> >> > +       kfree(tz);
>> >> > +exit:
>> >> > +       mutex_unlock(&tz_list_lock);
>> >> > +       return;
>> >> > +}
>> >> > +EXPORT_SYMBOL(remove_thermal_zone);
>> >> > +
>> >> > +struct thermal_sensor *get_sensor_by_name(const char *name)
>> >> > +{
>> >> > +       struct thermal_sensor *pos;
>> >> > +       struct thermal_sensor *ts = NULL;
>> >> > +
>> >> > +       mutex_lock(&ts_list_lock);
>> >> > +       for_each_thermal_sensor(pos) {
>> >> > +               if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) {
>> >> > +                       ts = pos;
>> >> > +                       break;
>> >> > +               }
>> >> > +       }
>> >> > +       mutex_unlock(&ts_list_lock);
>> >> > +       return ts;
>> >> > +}
>> >> > +EXPORT_SYMBOL(get_sensor_by_name);
>> >> > +
>> >> > +int add_sensor_to_zone_by_name(struct thermal_zone *tz, const
>> char
>> >> *name)
>> >> > +{
>> >> > +       int ret;
>> >> > +       struct thermal_sensor *ts = get_sensor_by_name(name);
>> >> > +
>> >> > +       if (!tz || !ts)
>> >> > +               return -EINVAL;
>> >> > +
>> >> > +       mutex_lock(&tz_list_lock);
>> >> > +
>> >> > +       /* Ensure we are not adding the same sensor again!! */
>> >> > +       ret = get_sensor_indx(tz, ts);
>> >> > +       if (ret >= 0) {
>> >> > +               ret = -EEXIST;
>> >> > +               goto exit_zone;
>> >> > +       }
>> >> > +
>> >> > +       mutex_lock(&ts_list_lock);
>> >> > +
>> >> > +       ret = sysfs_create_link(&tz->device.kobj, &ts->device.kobj,
>> >> > +                               kobject_name(&ts->device.kobj));
>> >> > +       if (ret)
>> >> > +               goto exit_sensor;
>> >> > +
>> >> > +       tz->sensors[tz->sensor_indx++] = ts;
>> >> > +
>> >> > +exit_sensor:
>> >> > +       mutex_unlock(&ts_list_lock);
>> >> > +exit_zone:
>> >> > +       mutex_unlock(&tz_list_lock);
>> >> > +       return ret;
>> >> > +}
>> >> > +EXPORT_SYMBOL(add_sensor_to_zone_by_name);
>> >> > +
>> >> > +int add_sensor_to_zone(struct thermal_zone *tz, struct
>> thermal_sensor
>> >> *ts)
>> >> > +{
>> >> > +       if (!tz || !ts)
>> >> > +               return -EINVAL;
>> >> > +
>> >> > +       return add_sensor_to_zone_by_name(tz, ts->name);
>> >> > +}
>> >> > +EXPORT_SYMBOL(add_sensor_to_zone);
>> >> > +
>> >> >  /**
>> >> >   * thermal_sensor_register - register a new thermal sensor
>> >> >   * @name:      name of the thermal sensor
>> >> > @@ -1624,6 +1822,7 @@ EXPORT_SYMBOL(thermal_sensor_register);
>> >> >  void thermal_sensor_unregister(struct thermal_sensor *ts)
>> >> >  {
>> >> >         int i;
>> >> > +       struct thermal_zone *tz;
>> >> >         struct thermal_sensor *pos, *next;
>> >> >         bool found = false;
>> >> >
>> >> > @@ -1643,6 +1842,13 @@ void thermal_sensor_unregister(struct
>> >> thermal_sensor *ts)
>> >> >         if (!found)
>> >> >                 return;
>> >> >
>> >> > +       mutex_lock(&tz_list_lock);
>> >> > +
>> >> > +       for_each_thermal_zone(tz)
>> >> > +               remove_sensor_from_zone(tz, ts);
>> >> > +
>> >> > +       mutex_unlock(&tz_list_lock);
>> >> > +
>> >> >         for (i = 0; i < ts->thresholds; i++)
>> >> >                 device_remove_file(&ts->device, &ts->thresh_attrs[i].attr);
>> >> >
>> >> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> >> > index e2f85ec..38438be 100644
>> >> > --- a/include/linux/thermal.h
>> >> > +++ b/include/linux/thermal.h
>> >> > @@ -49,6 +49,11 @@
>> >> >  /* Default Thermal Governor: Does Linear Throttling */
>> >> >  #define DEFAULT_THERMAL_GOVERNOR       "step_wise"
>> >> >
>> >> > +/* Maximum number of sensors, allowed in a thermal zone
>> >> > + * We will start with 5, and increase if needed.
>> >> > + */
>> >> > +#define MAX_SENSORS_PER_ZONE           5
>> >> > +
>> >> >  struct thermal_sensor;
>> >> >  struct thermal_zone_device;
>> >> >  struct thermal_cooling_device;
>> >> > @@ -191,6 +196,21 @@ struct thermal_zone_device {
>> >> >         struct delayed_work poll_queue;
>> >> >  };
>> >> >
>> >> > +struct thermal_zone {
>> >> > +       char name[THERMAL_NAME_LENGTH];
>> >> > +       int id;
>> >> > +       void *devdata;
>> >> > +       struct thermal_zone *ops;
>> >> What is this, thermal_zone_device_ops? and how to initialize this ops?
>> >
>> > oh, Not required, for now. Will remove it..
>> > Good catch :-)
>>
>> I am still confused a bit.
>> Is this thermal_zone going to take the place of the old
>> thermal_zone_device?
>
> Yes, it will replace the older one.
>
>> If yes, which functions/callbacks are used to access
>> thermal_zone->trip[] like the old get_trip_*?
>
> We will not have call backs associated with 'zone'.
> A zone is kind of a virtual entity, which can have one or
> more sensors associated with it.
>
> Trip points are attached to 'a sensor' which can be in
> any zone. So, thermal_sensor_ops functions will help you
> access the trip values.
But I didn't find such functions in the current thermal_sensor_ops for
access trip points, only functions for thresholds now.
Will you add these functions when you update the governors?

>
> Thanks,
> Durga
>
>>
>> >
>> > Thanks,
>> > Durga
>> >>
>> >> > +       struct thermal_governor *governor;
>> >> > +       struct idr idr;
>> >> > +       struct device device;
>> >> > +       struct list_head node;
>> >> > +
>> >> > +       /* Sensor level information */
>> >> > +       int sensor_indx; /* index into 'sensors' array */
>> >> > +       struct thermal_sensor *sensors[MAX_SENSORS_PER_ZONE];
>> >> > +};
>> >> > +
>> >> >  /* Structure that holds thermal governor information */
>> >> >  struct thermal_governor {
>> >> >         char name[THERMAL_NAME_LENGTH];
>> >> > @@ -264,6 +284,11 @@ struct thermal_sensor
>> >> *thermal_sensor_register(const char *,
>> >> >  void thermal_sensor_unregister(struct thermal_sensor *);
>> >> >  int enable_sensor_thresholds(struct thermal_sensor *, int);
>> >> >
>> >> > +struct thermal_zone *create_thermal_zone(const char *, void *);
>> >> > +void remove_thermal_zone(struct thermal_zone *);
>> >> > +int add_sensor_to_zone(struct thermal_zone *, struct thermal_sensor
>> >> *);
>> >> > +int add_sensor_to_zone_by_name(struct thermal_zone *, const char
>> *);
>> >> > +
>> >> >  #ifdef CONFIG_NET
>> >> >  extern int thermal_generate_netlink_event(u32 orig, enum events
>> >> event);
>> >> >  #else
>> >> > --
>> >> > 1.7.9.5
>> >> >
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
durgadoss.r@intel.com Dec. 3, 2012, 1:12 p.m. UTC | #6
Hi,

[big cut.]
> >> >> > +struct thermal_zone {
> >> >> > +       char name[THERMAL_NAME_LENGTH];
> >> >> > +       int id;
> >> >> > +       void *devdata;
> >> >> > +       struct thermal_zone *ops;
> >> >> What is this, thermal_zone_device_ops? and how to initialize this ops?
> >> >
> >> > oh, Not required, for now. Will remove it..
> >> > Good catch :-)
> >>
> >> I am still confused a bit.
> >> Is this thermal_zone going to take the place of the old
> >> thermal_zone_device?
> >
> > Yes, it will replace the older one.
> >
> >> If yes, which functions/callbacks are used to access
> >> thermal_zone->trip[] like the old get_trip_*?
> >
> > We will not have call backs associated with 'zone'.
> > A zone is kind of a virtual entity, which can have one or
> > more sensors associated with it.
> >
> > Trip points are attached to 'a sensor' which can be in
> > any zone. So, thermal_sensor_ops functions will help you
> > access the trip values.
> But I didn't find such functions in the current thermal_sensor_ops for
> access trip points, only functions for thresholds now.
> Will you add these functions when you update the governors?

The idea is the governors need these values to calculate the trend
of the thermal zone. And these are embedded inside thermal zone
structure. So, when required a zone can 'know' about its sensors and
their trip points through these pointers.

Having said that, when we modify governors, if a need arises to
implement these functions, we will implement it.

Thanks,
Durga
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hongbo Zhang Dec. 13, 2012, 6:23 a.m. UTC | #7
On 17 November 2012 18:45, Durgadoss R <durgadoss.r@intel.com> wrote:
> This patch adds a new thermal_zone structure to
> thermal.h. Also, adds zone level APIs to the thermal
> framework.
>
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> ---
>  drivers/thermal/thermal_sys.c |  206 +++++++++++++++++++++++++++++++++++++++++
>  include/linux/thermal.h       |   25 +++++
>  2 files changed, 231 insertions(+)
>
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index e726c8b..9d386d7 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -44,19 +44,28 @@ MODULE_DESCRIPTION("Generic thermal management sysfs support");
>  MODULE_LICENSE("GPL");
>
>  static DEFINE_IDR(thermal_tz_idr);
> +static DEFINE_IDR(thermal_zone_idr);
>  static DEFINE_IDR(thermal_cdev_idr);
>  static DEFINE_IDR(thermal_sensor_idr);
>  static DEFINE_MUTEX(thermal_idr_lock);
>
>  static LIST_HEAD(thermal_tz_list);
>  static LIST_HEAD(thermal_sensor_list);
> +static LIST_HEAD(thermal_zone_list);
>  static LIST_HEAD(thermal_cdev_list);
>  static LIST_HEAD(thermal_governor_list);
>
>  static DEFINE_MUTEX(thermal_list_lock);
>  static DEFINE_MUTEX(ts_list_lock);
> +static DEFINE_MUTEX(tz_list_lock);
>  static DEFINE_MUTEX(thermal_governor_lock);
>
> +#define for_each_thermal_sensor(pos) \
> +       list_for_each_entry(pos, &thermal_sensor_list, node)
> +
> +#define for_each_thermal_zone(pos) \
> +       list_for_each_entry(pos, &thermal_zone_list, node)
> +
>  static struct thermal_governor *__find_governor(const char *name)
>  {
>         struct thermal_governor *pos;
> @@ -419,15 +428,62 @@ static void thermal_zone_device_check(struct work_struct *work)
>         thermal_zone_device_update(tz);
>  }
>
> +static int get_sensor_indx(struct thermal_zone *tz, struct thermal_sensor *ts)
> +{
> +       int i, indx = -EINVAL;
> +
> +       if (!tz || !ts)
> +               return -EINVAL;
> +
> +       mutex_lock(&ts_list_lock);
> +       for (i = 0; i < tz->sensor_indx; i++) {
> +               if (tz->sensors[i] == ts) {
> +                       indx = i;
> +                       break;
> +               }
> +       }
> +       mutex_unlock(&ts_list_lock);
> +       return indx;
> +}
> +
> +static void remove_sensor_from_zone(struct thermal_zone *tz,
> +                               struct thermal_sensor *ts)
> +{
> +       int indx, j;
> +
> +       indx = get_sensor_indx(tz, ts);
> +       if (indx < 0)
> +               return;
> +
> +       sysfs_remove_link(&tz->device.kobj, kobject_name(&ts->device.kobj));
> +
> +       /* Shift the entries in the tz->sensors array */
> +       for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++)
> +               tz->sensors[j] = tz->sensors[j + 1];
> +
> +       tz->sensor_indx--;
> +}
> +
>  /* sys I/F for thermal zone */
>
>  #define to_thermal_zone(_dev) \
>         container_of(_dev, struct thermal_zone_device, device)
>
> +#define to_zone(_dev) \
> +       container_of(_dev, struct thermal_zone, device)
> +
>  #define to_thermal_sensor(_dev) \
>         container_of(_dev, struct thermal_sensor, device)
>
>  static ssize_t
> +zone_name_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +       struct thermal_zone *tz = to_zone(dev);
> +
> +       return sprintf(buf, "%s\n", tz->name);
> +}
> +
> +static ssize_t
>  sensor_name_show(struct device *dev, struct device_attribute *attr, char *buf)
>  {
>         struct thermal_sensor *ts = to_thermal_sensor(dev);
> @@ -772,6 +828,8 @@ static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR, policy_show, policy_store);
>  static DEVICE_ATTR(sensor_name, 0444, sensor_name_show, NULL);
>  static DEVICE_ATTR(temp_input, 0444, sensor_temp_show, NULL);
>
> +static DEVICE_ATTR(zone_name, 0444, zone_name_show, NULL);
> +
>  /* sys I/F for cooling device */
>  #define to_cooling_device(_dev)        \
>         container_of(_dev, struct thermal_cooling_device, device)
> @@ -1557,6 +1615,146 @@ static void remove_trip_attrs(struct thermal_zone_device *tz)
>         kfree(tz->trip_hyst_attrs);
>  }
>
> +struct thermal_zone *create_thermal_zone(const char *name, void *devdata)
Durgaross,
Since more sensors can be added into one thermal zone, think about
this situation that every sensor is in itself file separately:

sensorA_file.c:
if common_zone_for_AB not there;
create common_zone_for_AB;
add sensorA into common_zone_for_AB;

sensorB_file.c:
if common_zone_for_AB not there;
create common_zone_for_AB;
add sensorB into common_zone_for_AB;

But how to check dedicate thermal zone is created or not? we are
lacking of this interface.
Here are two ways to achieve this from my point of view:
a)
add interface get_thermal_zone_byname(const char *name) which will
walk through the thermal_zone_list.
b)
add one more parameter for current interface, like this
create_thermal_zone(const char *name, void *devdata, bool reuse)
if reuse==ture {
if thermal zone already created, return it;
else create thermal zone;
} else {
if thermal zone already created, return error;
else create thermal zone;
}

I prefer a), how do you think about it?

> +{
> +       struct thermal_zone *tz;
> +       int ret;
> +
> +       if (!name || (name && strlen(name) >= THERMAL_NAME_LENGTH))
> +               return ERR_PTR(-EINVAL);
> +
> +       tz = kzalloc(sizeof(*tz), GFP_KERNEL);
> +       if (!tz)
> +               return ERR_PTR(-ENOMEM);
> +
> +       idr_init(&tz->idr);
> +       ret = get_idr(&thermal_zone_idr, &thermal_idr_lock, &tz->id);
> +       if (ret)
> +               goto exit_free;
> +
> +       strcpy(tz->name, name);
> +       tz->devdata = devdata;
> +       tz->device.class = &thermal_class;
> +
> +       dev_set_name(&tz->device, "zone%d", tz->id);
> +       ret = device_register(&tz->device);
> +       if (ret)
> +               goto exit_idr;
> +
> +       ret = device_create_file(&tz->device, &dev_attr_zone_name);
> +       if (ret)
> +               goto exit_unregister;
> +
> +       /* Add this zone to the global list of thermal zones */
> +       mutex_lock(&tz_list_lock);
> +       list_add_tail(&tz->node, &thermal_zone_list);
> +       mutex_unlock(&tz_list_lock);
> +       return tz;
> +
> +exit_unregister:
> +       device_unregister(&tz->device);
> +exit_idr:
> +       release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
> +exit_free:
> +       kfree(tz);
> +       return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL(create_thermal_zone);
> +
> +void remove_thermal_zone(struct thermal_zone *tz)
> +{
> +       struct thermal_zone *pos, *next;
> +       bool found = false;
> +
> +       if (!tz)
> +               return;
> +
> +       mutex_lock(&tz_list_lock);
> +
> +       list_for_each_entry_safe(pos, next, &thermal_zone_list, node) {
> +               if (pos == tz) {
> +                       list_del(&tz->node);
> +                       found = true;
> +                       break;
> +               }
> +       }
> +
> +       if (!found)
> +               goto exit;
> +
> +       device_remove_file(&tz->device, &dev_attr_zone_name);
> +
> +       release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
> +       idr_destroy(&tz->idr);
> +
> +       device_unregister(&tz->device);
> +       kfree(tz);
> +exit:
> +       mutex_unlock(&tz_list_lock);
> +       return;
> +}
> +EXPORT_SYMBOL(remove_thermal_zone);
> +
> +struct thermal_sensor *get_sensor_by_name(const char *name)
> +{
> +       struct thermal_sensor *pos;
> +       struct thermal_sensor *ts = NULL;
> +
> +       mutex_lock(&ts_list_lock);
> +       for_each_thermal_sensor(pos) {
> +               if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) {
> +                       ts = pos;
> +                       break;
> +               }
> +       }
> +       mutex_unlock(&ts_list_lock);
> +       return ts;
> +}
> +EXPORT_SYMBOL(get_sensor_by_name);
> +
> +int add_sensor_to_zone_by_name(struct thermal_zone *tz, const char *name)
> +{
> +       int ret;
> +       struct thermal_sensor *ts = get_sensor_by_name(name);
> +
> +       if (!tz || !ts)
> +               return -EINVAL;
> +
> +       mutex_lock(&tz_list_lock);
> +
> +       /* Ensure we are not adding the same sensor again!! */
> +       ret = get_sensor_indx(tz, ts);
> +       if (ret >= 0) {
> +               ret = -EEXIST;
> +               goto exit_zone;
> +       }
> +
> +       mutex_lock(&ts_list_lock);
> +
> +       ret = sysfs_create_link(&tz->device.kobj, &ts->device.kobj,
> +                               kobject_name(&ts->device.kobj));
> +       if (ret)
> +               goto exit_sensor;
> +
> +       tz->sensors[tz->sensor_indx++] = ts;
> +
> +exit_sensor:
> +       mutex_unlock(&ts_list_lock);
> +exit_zone:
> +       mutex_unlock(&tz_list_lock);
> +       return ret;
> +}
> +EXPORT_SYMBOL(add_sensor_to_zone_by_name);
> +
> +int add_sensor_to_zone(struct thermal_zone *tz, struct thermal_sensor *ts)
> +{
> +       if (!tz || !ts)
> +               return -EINVAL;
> +
> +       return add_sensor_to_zone_by_name(tz, ts->name);
> +}
> +EXPORT_SYMBOL(add_sensor_to_zone);
> +
>  /**
>   * thermal_sensor_register - register a new thermal sensor
>   * @name:      name of the thermal sensor
> @@ -1624,6 +1822,7 @@ EXPORT_SYMBOL(thermal_sensor_register);
>  void thermal_sensor_unregister(struct thermal_sensor *ts)
>  {
>         int i;
> +       struct thermal_zone *tz;
>         struct thermal_sensor *pos, *next;
>         bool found = false;
>
> @@ -1643,6 +1842,13 @@ void thermal_sensor_unregister(struct thermal_sensor *ts)
>         if (!found)
>                 return;
>
> +       mutex_lock(&tz_list_lock);
> +
> +       for_each_thermal_zone(tz)
> +               remove_sensor_from_zone(tz, ts);
> +
> +       mutex_unlock(&tz_list_lock);
> +
>         for (i = 0; i < ts->thresholds; i++)
>                 device_remove_file(&ts->device, &ts->thresh_attrs[i].attr);
>
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index e2f85ec..38438be 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -49,6 +49,11 @@
>  /* Default Thermal Governor: Does Linear Throttling */
>  #define DEFAULT_THERMAL_GOVERNOR       "step_wise"
>
> +/* Maximum number of sensors, allowed in a thermal zone
> + * We will start with 5, and increase if needed.
> + */
> +#define MAX_SENSORS_PER_ZONE           5
> +
>  struct thermal_sensor;
>  struct thermal_zone_device;
>  struct thermal_cooling_device;
> @@ -191,6 +196,21 @@ struct thermal_zone_device {
>         struct delayed_work poll_queue;
>  };
>
> +struct thermal_zone {
> +       char name[THERMAL_NAME_LENGTH];
> +       int id;
> +       void *devdata;
> +       struct thermal_zone *ops;
> +       struct thermal_governor *governor;
> +       struct idr idr;
> +       struct device device;
> +       struct list_head node;
> +
> +       /* Sensor level information */
> +       int sensor_indx; /* index into 'sensors' array */
> +       struct thermal_sensor *sensors[MAX_SENSORS_PER_ZONE];
> +};
> +
>  /* Structure that holds thermal governor information */
>  struct thermal_governor {
>         char name[THERMAL_NAME_LENGTH];
> @@ -264,6 +284,11 @@ struct thermal_sensor *thermal_sensor_register(const char *,
>  void thermal_sensor_unregister(struct thermal_sensor *);
>  int enable_sensor_thresholds(struct thermal_sensor *, int);
>
> +struct thermal_zone *create_thermal_zone(const char *, void *);
> +void remove_thermal_zone(struct thermal_zone *);
> +int add_sensor_to_zone(struct thermal_zone *, struct thermal_sensor *);
> +int add_sensor_to_zone_by_name(struct thermal_zone *, const char *);
> +
>  #ifdef CONFIG_NET
>  extern int thermal_generate_netlink_event(u32 orig, enum events event);
>  #else
> --
> 1.7.9.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
durgadoss.r@intel.com Dec. 13, 2012, 3 p.m. UTC | #8
Hi,


> -----Original Message-----
> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
> owner@vger.kernel.org] On Behalf Of Hongbo Zhang
> Sent: Thursday, December 13, 2012 11:53 AM
> To: R, Durgadoss
> Cc: Zhang, Rui; linux-pm@vger.kernel.org; wni@nvidia.com;
> eduardo.valentin@ti.com; amit.kachhap@linaro.org;
> sachin.kamat@linaro.org
> Subject: Re: [RFC PATCH 2/7] Thermal: Create zone level APIs
> 
> On 17 November 2012 18:45, Durgadoss R <durgadoss.r@intel.com> wrote:
> > This patch adds a new thermal_zone structure to
> > thermal.h. Also, adds zone level APIs to the thermal
> > framework.
> >
> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>

[A big cut.]

> >  #define to_cooling_device(_dev)        \
> >         container_of(_dev, struct thermal_cooling_device, device)
> > @@ -1557,6 +1615,146 @@ static void remove_trip_attrs(struct
> thermal_zone_device *tz)
> >         kfree(tz->trip_hyst_attrs);
> >  }
> >
> > +struct thermal_zone *create_thermal_zone(const char *name, void
> *devdata)
> Durgaross,
> Since more sensors can be added into one thermal zone, think about
> this situation that every sensor is in itself file separately:

Yes, we thought about this scenario.
The way the new framework is designed is like below:

The thermal sensor source file can be generic (can be any sensor driver, in any
subsystem). This driver will use the sensor APIs and register with thermal
framework to participate in platform Thermal management. This does not
(and should not) know about which zone it belongs to, or any other information
about platform thermals. A sensor driver is a standalone piece of code, which
can optionally register with thermal framework.

However, for any platform, there should be a platformX_thermal.c file,
which will know about the platform thermal characteristics (like how many sensors,
zones, cooling devices, etc.. And how they are related to each other i.e the
mapping information). Only in this file, the zone level APIs should be used, in
which case the file will have all information required to attach various sensors
to a particular zone.

This way, we can have one platform level thermal file, which can support
multiple platforms (may be)using the same set of sensors (but)binded in a different way.
This file can get the platform thermal information through Firmware,
ACPI tables, device tree etc.

Unfortunately, today we don't have many drivers that can be clearly differentiated
as 'sensor_file.c' and 'platform_thermal_file.c'. But very soon we will need/have.
The reason I am saying this is because we are seeing a lot of chip drivers,
starting to use thermal framework, and we should keep it really light-weight
for them to do so.

An Example: drivers/hwmon/emc1403.c - a generic thermal chip driver
In one platform this sensor can belong to 'ZoneA' and in another the
same can belong to 'ZoneB'. But, emc1403.c does not really care about
where does it belong. It just reports temperature.
[Okay, I just picked an example, it doesn't register with framework,
as of today :-)]

> 
> sensorA_file.c:
> if common_zone_for_AB not there;
> create common_zone_for_AB;

This create_zone should not even be in sensorA_file.c.
It should be moved to platform level file.

> add sensorA into common_zone_for_AB;
> 
> sensorB_file.c:
> if common_zone_for_AB not there;
> create common_zone_for_AB;
> add sensorB into common_zone_for_AB;
> 

Same here..

> But how to check dedicate thermal zone is created or not? we are
> lacking of this interface.
> Here are two ways to achieve this from my point of view:
> a)
> add interface get_thermal_zone_byname(const char *name) which will
> walk through the thermal_zone_list.

However, (despite this problem we are talking about)
we can introduce this API, if needed.

> b)
> add one more parameter for current interface, like this
> create_thermal_zone(const char *name, void *devdata, bool reuse)
> if reuse==ture {
> if thermal zone already created, return it;
> else create thermal zone;
> } else {
> if thermal zone already created, return error;
> else create thermal zone;
> }
> 
> I prefer a), how do you think about it?

I know I wrote a lengthy explanation. Let me know if it makes sense.
If it does, then probably I will add it to our Documentation file.

Thanks,
Durga
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hongbo Zhang Dec. 14, 2012, 4:10 a.m. UTC | #9
On 13 December 2012 23:00, R, Durgadoss <durgadoss.r@intel.com> wrote:
> Hi,
>
>
>> -----Original Message-----
>> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
>> owner@vger.kernel.org] On Behalf Of Hongbo Zhang
>> Sent: Thursday, December 13, 2012 11:53 AM
>> To: R, Durgadoss
>> Cc: Zhang, Rui; linux-pm@vger.kernel.org; wni@nvidia.com;
>> eduardo.valentin@ti.com; amit.kachhap@linaro.org;
>> sachin.kamat@linaro.org
>> Subject: Re: [RFC PATCH 2/7] Thermal: Create zone level APIs
>>
>> On 17 November 2012 18:45, Durgadoss R <durgadoss.r@intel.com> wrote:
>> > This patch adds a new thermal_zone structure to
>> > thermal.h. Also, adds zone level APIs to the thermal
>> > framework.
>> >
>> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
>
> [A big cut.]
>
>> >  #define to_cooling_device(_dev)        \
>> >         container_of(_dev, struct thermal_cooling_device, device)
>> > @@ -1557,6 +1615,146 @@ static void remove_trip_attrs(struct
>> thermal_zone_device *tz)
>> >         kfree(tz->trip_hyst_attrs);
>> >  }
>> >
>> > +struct thermal_zone *create_thermal_zone(const char *name, void
>> *devdata)
>> Durgaross,
>> Since more sensors can be added into one thermal zone, think about
>> this situation that every sensor is in itself file separately:
>
> Yes, we thought about this scenario.
> The way the new framework is designed is like below:
>
> The thermal sensor source file can be generic (can be any sensor driver, in any
> subsystem). This driver will use the sensor APIs and register with thermal
> framework to participate in platform Thermal management. This does not
> (and should not) know about which zone it belongs to, or any other information
> about platform thermals. A sensor driver is a standalone piece of code, which
> can optionally register with thermal framework.
>
> However, for any platform, there should be a platformX_thermal.c file,
> which will know about the platform thermal characteristics (like how many sensors,
> zones, cooling devices, etc.. And how they are related to each other i.e the
> mapping information). Only in this file, the zone level APIs should be used, in
> which case the file will have all information required to attach various sensors
> to a particular zone.
I guess you will remove the bind callback, I suggest to remove it if
you don't have this plan.
A match callback or/and mapping data can be used to identify the
binding relationship, the current binding process is abundant and
strange a bit.
>
> This way, we can have one platform level thermal file, which can support
> multiple platforms (may be)using the same set of sensors (but)binded in a different way.
> This file can get the platform thermal information through Firmware,
> ACPI tables, device tree etc.
>
> Unfortunately, today we don't have many drivers that can be clearly differentiated
> as 'sensor_file.c' and 'platform_thermal_file.c'. But very soon we will need/have.
> The reason I am saying this is because we are seeing a lot of chip drivers,
> starting to use thermal framework, and we should keep it really light-weight
> for them to do so.
>
> An Example: drivers/hwmon/emc1403.c - a generic thermal chip driver
> In one platform this sensor can belong to 'ZoneA' and in another the
> same can belong to 'ZoneB'. But, emc1403.c does not really care about
> where does it belong. It just reports temperature.
> [Okay, I just picked an example, it doesn't register with framework,
> as of today :-)]
Clear about the above information.
This is a good design, I believe more and more drivers will use this.
But much efforts are needed.
>
>>
>> sensorA_file.c:
>> if common_zone_for_AB not there;
>> create common_zone_for_AB;
>
> This create_zone should not even be in sensorA_file.c.
> It should be moved to platform level file.
>
>> add sensorA into common_zone_for_AB;
>>
>> sensorB_file.c:
>> if common_zone_for_AB not there;
>> create common_zone_for_AB;
>> add sensorB into common_zone_for_AB;
>>
>
> Same here..
>
>> But how to check dedicate thermal zone is created or not? we are
>> lacking of this interface.
>> Here are two ways to achieve this from my point of view:
>> a)
>> add interface get_thermal_zone_byname(const char *name) which will
>> walk through the thermal_zone_list.
>
> However, (despite this problem we are talking about)
> we can introduce this API, if needed.
>
>> b)
>> add one more parameter for current interface, like this
>> create_thermal_zone(const char *name, void *devdata, bool reuse)
>> if reuse==ture {
>> if thermal zone already created, return it;
>> else create thermal zone;
>> } else {
>> if thermal zone already created, return error;
>> else create thermal zone;
>> }
>>
>> I prefer a), how do you think about it?
>
> I know I wrote a lengthy explanation. Let me know if it makes sense.
> If it does, then probably I will add it to our Documentation file.
Yes, It really makes sense.
Thanks.
>
> Thanks,
> Durga
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
durgadoss.r@intel.com Dec. 14, 2012, 5:10 a.m. UTC | #10
Hi,


> -----Original Message-----
> From: Hongbo Zhang [mailto:hongbo.zhang@linaro.org]
> Sent: Friday, December 14, 2012 9:41 AM
> To: R, Durgadoss
> Cc: Zhang, Rui; linux-pm@vger.kernel.org; wni@nvidia.com;
> eduardo.valentin@ti.com; amit.kachhap@linaro.org;
> sachin.kamat@linaro.org
> Subject: Re: [RFC PATCH 2/7] Thermal: Create zone level APIs
> 
> On 13 December 2012 23:00, R, Durgadoss <durgadoss.r@intel.com> wrote:
> > Hi,
> >
> >
> >> -----Original Message-----
> >> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
> >> owner@vger.kernel.org] On Behalf Of Hongbo Zhang
> >> Sent: Thursday, December 13, 2012 11:53 AM
> >> To: R, Durgadoss
> >> Cc: Zhang, Rui; linux-pm@vger.kernel.org; wni@nvidia.com;
> >> eduardo.valentin@ti.com; amit.kachhap@linaro.org;
> >> sachin.kamat@linaro.org
> >> Subject: Re: [RFC PATCH 2/7] Thermal: Create zone level APIs
> >>
> >> On 17 November 2012 18:45, Durgadoss R <durgadoss.r@intel.com>
> wrote:
> >> > This patch adds a new thermal_zone structure to
> >> > thermal.h. Also, adds zone level APIs to the thermal
> >> > framework.
> >> >
> >> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> >
> > [A big cut.]
> >
> >> >  #define to_cooling_device(_dev)        \
> >> >         container_of(_dev, struct thermal_cooling_device, device)
> >> > @@ -1557,6 +1615,146 @@ static void remove_trip_attrs(struct
> >> thermal_zone_device *tz)
> >> >         kfree(tz->trip_hyst_attrs);
> >> >  }
> >> >
> >> > +struct thermal_zone *create_thermal_zone(const char *name, void
> >> *devdata)
> >> Durgaross,
> >> Since more sensors can be added into one thermal zone, think about
> >> this situation that every sensor is in itself file separately:
> >
> > Yes, we thought about this scenario.
> > The way the new framework is designed is like below:
> >
> > The thermal sensor source file can be generic (can be any sensor driver, in
> any
> > subsystem). This driver will use the sensor APIs and register with thermal
> > framework to participate in platform Thermal management. This does not
> > (and should not) know about which zone it belongs to, or any other
> information
> > about platform thermals. A sensor driver is a standalone piece of code,
> which
> > can optionally register with thermal framework.
> >
> > However, for any platform, there should be a platformX_thermal.c file,
> > which will know about the platform thermal characteristics (like how many
> sensors,
> > zones, cooling devices, etc.. And how they are related to each other i.e the
> > mapping information). Only in this file, the zone level APIs should be used,
> in
> > which case the file will have all information required to attach various
> sensors
> > to a particular zone.
> I guess you will remove the bind callback, I suggest to remove it if
> you don't have this plan.

Yes, We will remove this.

> A match callback or/and mapping data can be used to identify the
> binding relationship, the current binding process is abundant and
> strange a bit.

Yes, the mapping data can be used.

> >
> > This way, we can have one platform level thermal file, which can support
> > multiple platforms (may be)using the same set of sensors (but)binded in a
> different way.
> > This file can get the platform thermal information through Firmware,
> > ACPI tables, device tree etc.
> >
> > Unfortunately, today we don't have many drivers that can be clearly
> differentiated
> > as 'sensor_file.c' and 'platform_thermal_file.c'. But very soon we will
> need/have.
> > The reason I am saying this is because we are seeing a lot of chip drivers,
> > starting to use thermal framework, and we should keep it really light-
> weight
> > for them to do so.
> >
> > An Example: drivers/hwmon/emc1403.c - a generic thermal chip driver
> > In one platform this sensor can belong to 'ZoneA' and in another the
> > same can belong to 'ZoneB'. But, emc1403.c does not really care about
> > where does it belong. It just reports temperature.
> > [Okay, I just picked an example, it doesn't register with framework,
> > as of today :-)]

> Clear about the above information.

Thank you :-)

> This is a good design, I believe more and more drivers will use this.
> But much efforts are needed.

Yes, we need quite some code modifications.

> >
> >>
> >> sensorA_file.c:
> >> if common_zone_for_AB not there;
> >> create common_zone_for_AB;
> >
> > This create_zone should not even be in sensorA_file.c.
> > It should be moved to platform level file.
> >
> >> add sensorA into common_zone_for_AB;
> >>
> >> sensorB_file.c:
> >> if common_zone_for_AB not there;
> >> create common_zone_for_AB;
> >> add sensorB into common_zone_for_AB;
> >>
> >
> > Same here..
> >
> >> But how to check dedicate thermal zone is created or not? we are
> >> lacking of this interface.
> >> Here are two ways to achieve this from my point of view:
> >> a)
> >> add interface get_thermal_zone_byname(const char *name) which will
> >> walk through the thermal_zone_list.
> >
> > However, (despite this problem we are talking about)
> > we can introduce this API, if needed.
> >
> >> b)
> >> add one more parameter for current interface, like this
> >> create_thermal_zone(const char *name, void *devdata, bool reuse)
> >> if reuse==ture {
> >> if thermal zone already created, return it;
> >> else create thermal zone;
> >> } else {
> >> if thermal zone already created, return error;
> >> else create thermal zone;
> >> }
> >>
> >> I prefer a), how do you think about it?
> >
> > I know I wrote a lengthy explanation. Let me know if it makes sense.
> > If it does, then probably I will add it to our Documentation file.
> Yes, It really makes sense.
> Thanks.

Okay, Then I am adding this to our Documentation file.

Thanks,
Durga
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index e726c8b..9d386d7 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -44,19 +44,28 @@  MODULE_DESCRIPTION("Generic thermal management sysfs support");
 MODULE_LICENSE("GPL");
 
 static DEFINE_IDR(thermal_tz_idr);
+static DEFINE_IDR(thermal_zone_idr);
 static DEFINE_IDR(thermal_cdev_idr);
 static DEFINE_IDR(thermal_sensor_idr);
 static DEFINE_MUTEX(thermal_idr_lock);
 
 static LIST_HEAD(thermal_tz_list);
 static LIST_HEAD(thermal_sensor_list);
+static LIST_HEAD(thermal_zone_list);
 static LIST_HEAD(thermal_cdev_list);
 static LIST_HEAD(thermal_governor_list);
 
 static DEFINE_MUTEX(thermal_list_lock);
 static DEFINE_MUTEX(ts_list_lock);
+static DEFINE_MUTEX(tz_list_lock);
 static DEFINE_MUTEX(thermal_governor_lock);
 
+#define for_each_thermal_sensor(pos) \
+	list_for_each_entry(pos, &thermal_sensor_list, node)
+
+#define for_each_thermal_zone(pos) \
+	list_for_each_entry(pos, &thermal_zone_list, node)
+
 static struct thermal_governor *__find_governor(const char *name)
 {
 	struct thermal_governor *pos;
@@ -419,15 +428,62 @@  static void thermal_zone_device_check(struct work_struct *work)
 	thermal_zone_device_update(tz);
 }
 
+static int get_sensor_indx(struct thermal_zone *tz, struct thermal_sensor *ts)
+{
+	int i, indx = -EINVAL;
+
+	if (!tz || !ts)
+		return -EINVAL;
+
+	mutex_lock(&ts_list_lock);
+	for (i = 0; i < tz->sensor_indx; i++) {
+		if (tz->sensors[i] == ts) {
+			indx = i;
+			break;
+		}
+	}
+	mutex_unlock(&ts_list_lock);
+	return indx;
+}
+
+static void remove_sensor_from_zone(struct thermal_zone *tz,
+				struct thermal_sensor *ts)
+{
+	int indx, j;
+
+	indx = get_sensor_indx(tz, ts);
+	if (indx < 0)
+		return;
+
+	sysfs_remove_link(&tz->device.kobj, kobject_name(&ts->device.kobj));
+
+	/* Shift the entries in the tz->sensors array */
+	for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++)
+		tz->sensors[j] = tz->sensors[j + 1];
+
+	tz->sensor_indx--;
+}
+
 /* sys I/F for thermal zone */
 
 #define to_thermal_zone(_dev) \
 	container_of(_dev, struct thermal_zone_device, device)
 
+#define to_zone(_dev) \
+	container_of(_dev, struct thermal_zone, device)
+
 #define to_thermal_sensor(_dev) \
 	container_of(_dev, struct thermal_sensor, device)
 
 static ssize_t
+zone_name_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct thermal_zone *tz = to_zone(dev);
+
+	return sprintf(buf, "%s\n", tz->name);
+}
+
+static ssize_t
 sensor_name_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct thermal_sensor *ts = to_thermal_sensor(dev);
@@ -772,6 +828,8 @@  static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR, policy_show, policy_store);
 static DEVICE_ATTR(sensor_name, 0444, sensor_name_show, NULL);
 static DEVICE_ATTR(temp_input, 0444, sensor_temp_show, NULL);
 
+static DEVICE_ATTR(zone_name, 0444, zone_name_show, NULL);
+
 /* sys I/F for cooling device */
 #define to_cooling_device(_dev)	\
 	container_of(_dev, struct thermal_cooling_device, device)
@@ -1557,6 +1615,146 @@  static void remove_trip_attrs(struct thermal_zone_device *tz)
 	kfree(tz->trip_hyst_attrs);
 }
 
+struct thermal_zone *create_thermal_zone(const char *name, void *devdata)
+{
+	struct thermal_zone *tz;
+	int ret;
+
+	if (!name || (name && strlen(name) >= THERMAL_NAME_LENGTH))
+		return ERR_PTR(-EINVAL);
+
+	tz = kzalloc(sizeof(*tz), GFP_KERNEL);
+	if (!tz)
+		return ERR_PTR(-ENOMEM);
+
+	idr_init(&tz->idr);
+	ret = get_idr(&thermal_zone_idr, &thermal_idr_lock, &tz->id);
+	if (ret)
+		goto exit_free;
+
+	strcpy(tz->name, name);
+	tz->devdata = devdata;
+	tz->device.class = &thermal_class;
+
+	dev_set_name(&tz->device, "zone%d", tz->id);
+	ret = device_register(&tz->device);
+	if (ret)
+		goto exit_idr;
+
+	ret = device_create_file(&tz->device, &dev_attr_zone_name);
+	if (ret)
+		goto exit_unregister;
+
+	/* Add this zone to the global list of thermal zones */
+	mutex_lock(&tz_list_lock);
+	list_add_tail(&tz->node, &thermal_zone_list);
+	mutex_unlock(&tz_list_lock);
+	return tz;
+
+exit_unregister:
+	device_unregister(&tz->device);
+exit_idr:
+	release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
+exit_free:
+	kfree(tz);
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(create_thermal_zone);
+
+void remove_thermal_zone(struct thermal_zone *tz)
+{
+	struct thermal_zone *pos, *next;
+	bool found = false;
+
+	if (!tz)
+		return;
+
+	mutex_lock(&tz_list_lock);
+
+	list_for_each_entry_safe(pos, next, &thermal_zone_list, node) {
+		if (pos == tz) {
+			list_del(&tz->node);
+			found = true;
+			break;
+		}
+	}
+
+	if (!found)
+		goto exit;
+
+	device_remove_file(&tz->device, &dev_attr_zone_name);
+
+	release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
+	idr_destroy(&tz->idr);
+
+	device_unregister(&tz->device);
+	kfree(tz);
+exit:
+	mutex_unlock(&tz_list_lock);
+	return;
+}
+EXPORT_SYMBOL(remove_thermal_zone);
+
+struct thermal_sensor *get_sensor_by_name(const char *name)
+{
+	struct thermal_sensor *pos;
+	struct thermal_sensor *ts = NULL;
+
+	mutex_lock(&ts_list_lock);
+	for_each_thermal_sensor(pos) {
+		if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) {
+			ts = pos;
+			break;
+		}
+	}
+	mutex_unlock(&ts_list_lock);
+	return ts;
+}
+EXPORT_SYMBOL(get_sensor_by_name);
+
+int add_sensor_to_zone_by_name(struct thermal_zone *tz, const char *name)
+{
+	int ret;
+	struct thermal_sensor *ts = get_sensor_by_name(name);
+
+	if (!tz || !ts)
+		return -EINVAL;
+
+	mutex_lock(&tz_list_lock);
+
+	/* Ensure we are not adding the same sensor again!! */
+	ret = get_sensor_indx(tz, ts);
+	if (ret >= 0) {
+		ret = -EEXIST;
+		goto exit_zone;
+	}
+
+	mutex_lock(&ts_list_lock);
+
+	ret = sysfs_create_link(&tz->device.kobj, &ts->device.kobj,
+				kobject_name(&ts->device.kobj));
+	if (ret)
+		goto exit_sensor;
+
+	tz->sensors[tz->sensor_indx++] = ts;
+
+exit_sensor:
+	mutex_unlock(&ts_list_lock);
+exit_zone:
+	mutex_unlock(&tz_list_lock);
+	return ret;
+}
+EXPORT_SYMBOL(add_sensor_to_zone_by_name);
+
+int add_sensor_to_zone(struct thermal_zone *tz, struct thermal_sensor *ts)
+{
+	if (!tz || !ts)
+		return -EINVAL;
+
+	return add_sensor_to_zone_by_name(tz, ts->name);
+}
+EXPORT_SYMBOL(add_sensor_to_zone);
+
 /**
  * thermal_sensor_register - register a new thermal sensor
  * @name:	name of the thermal sensor
@@ -1624,6 +1822,7 @@  EXPORT_SYMBOL(thermal_sensor_register);
 void thermal_sensor_unregister(struct thermal_sensor *ts)
 {
 	int i;
+	struct thermal_zone *tz;
 	struct thermal_sensor *pos, *next;
 	bool found = false;
 
@@ -1643,6 +1842,13 @@  void thermal_sensor_unregister(struct thermal_sensor *ts)
 	if (!found)
 		return;
 
+	mutex_lock(&tz_list_lock);
+
+	for_each_thermal_zone(tz)
+		remove_sensor_from_zone(tz, ts);
+
+	mutex_unlock(&tz_list_lock);
+
 	for (i = 0; i < ts->thresholds; i++)
 		device_remove_file(&ts->device, &ts->thresh_attrs[i].attr);
 
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index e2f85ec..38438be 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -49,6 +49,11 @@ 
 /* Default Thermal Governor: Does Linear Throttling */
 #define DEFAULT_THERMAL_GOVERNOR	"step_wise"
 
+/* Maximum number of sensors, allowed in a thermal zone
+ * We will start with 5, and increase if needed.
+ */
+#define MAX_SENSORS_PER_ZONE		5
+
 struct thermal_sensor;
 struct thermal_zone_device;
 struct thermal_cooling_device;
@@ -191,6 +196,21 @@  struct thermal_zone_device {
 	struct delayed_work poll_queue;
 };
 
+struct thermal_zone {
+	char name[THERMAL_NAME_LENGTH];
+	int id;
+	void *devdata;
+	struct thermal_zone *ops;
+	struct thermal_governor *governor;
+	struct idr idr;
+	struct device device;
+	struct list_head node;
+
+	/* Sensor level information */
+	int sensor_indx; /* index into 'sensors' array */
+	struct thermal_sensor *sensors[MAX_SENSORS_PER_ZONE];
+};
+
 /* Structure that holds thermal governor information */
 struct thermal_governor {
 	char name[THERMAL_NAME_LENGTH];
@@ -264,6 +284,11 @@  struct thermal_sensor *thermal_sensor_register(const char *,
 void thermal_sensor_unregister(struct thermal_sensor *);
 int enable_sensor_thresholds(struct thermal_sensor *, int);
 
+struct thermal_zone *create_thermal_zone(const char *, void *);
+void remove_thermal_zone(struct thermal_zone *);
+int add_sensor_to_zone(struct thermal_zone *, struct thermal_sensor *);
+int add_sensor_to_zone_by_name(struct thermal_zone *, const char *);
+
 #ifdef CONFIG_NET
 extern int thermal_generate_netlink_event(u32 orig, enum events event);
 #else