| Message ID | 1360061183-14137-3-git-send-email-durgadoss.r@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Zhang Rui |
| Headers | show |
On Tue, 2013-02-05 at 16:16 +0530, Durgadoss R wrote: > This patch adds a new thermal_zone structure to > thermal.h. Also, adds zone level APIs to the thermal > framework. > > A thermal zone is a hot spot on the platform, which > can have one or more sensors and cooling devices attached > to it. These sensors can be mapped to a set of cooling > devices, which when throttled, can help to bring down > the temperature of the hot spot. > > Signed-off-by: Durgadoss R <durgadoss.r@intel.com> > --- > drivers/thermal/thermal_sys.c | 196 +++++++++++++++++++++++++++++++++++++++++ > include/linux/thermal.h | 22 +++++ > 2 files changed, 218 insertions(+) > > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c > index cb94497..838d4fb 100644 > --- a/drivers/thermal/thermal_sys.c > +++ b/drivers/thermal/thermal_sys.c > @@ -43,19 +43,46 @@ 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(sensor_list_lock); > +static DEFINE_MUTEX(zone_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) > + > +#define GET_INDEX(tz, ptr, type) \ > +({ \ > + int i, ret = -EINVAL; \ > + do { \ > + if (!tz || !ptr) \ > + break; \ > + mutex_lock(&type##_list_lock); \ > + for (i = 0; i < tz->type##_indx; i++) { \ > + if (tz->type##s[i] == ptr) { \ > + ret = i; \ > + break; \ > + } \ > + } \ > + mutex_unlock(&type##_list_lock); \ > + } while (0); \ > + ret; \ > +}) > + > static struct thermal_governor *__find_governor(const char *name) > { > struct thermal_governor *pos; > @@ -421,15 +448,44 @@ static void thermal_zone_device_check(struct work_struct *work) > thermal_zone_device_update(tz); > } > > +static void remove_sensor_from_zone(struct thermal_zone *tz, > + struct thermal_sensor *ts) > +{ > + int j, indx; > + > + indx = GET_INDEX(tz, ts, sensor); > + 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); > @@ -811,6 +867,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) > @@ -1656,6 +1714,136 @@ static int enable_sensor_thresholds(struct thermal_sensor *ts, int count) > return 0; > } > > +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(&zone_list_lock); > + list_add_tail(&tz->node, &thermal_zone_list); > + mutex_unlock(&zone_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(&zone_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(&zone_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(&sensor_list_lock); > + for_each_thermal_sensor(pos) { > + if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) { > + ts = pos; > + break; this function depends on the assumption that all the sensor names are unique. thus I prefer to go through all the list and return -EINVAL if duplicate names found, because in this case, the pointer returned may be not the sensor we want to get. thanks, rui > + } > + } > + mutex_unlock(&sensor_list_lock); > + return ts; > +} > +EXPORT_SYMBOL(get_sensor_by_name); > + > +int add_sensor_to_zone(struct thermal_zone *tz, struct thermal_sensor *ts) > +{ > + int ret; > + > + if (!tz || !ts) > + return -EINVAL; > + > + mutex_lock(&zone_list_lock); > + > + /* Ensure we are not adding the same sensor again!! */ > + ret = GET_INDEX(tz, ts, sensor); > + if (ret >= 0) { > + ret = -EEXIST; > + goto exit_zone; > + } > + > + mutex_lock(&sensor_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(&sensor_list_lock); > +exit_zone: > + mutex_unlock(&zone_list_lock); > + return ret; > +} > +EXPORT_SYMBOL(add_sensor_to_zone); > + > /** > * thermal_sensor_register - register a new thermal sensor > * @name: name of the thermal sensor > @@ -1732,6 +1920,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; > > @@ -1751,6 +1940,13 @@ void thermal_sensor_unregister(struct thermal_sensor *ts) > if (!found) > return; > > + mutex_lock(&zone_list_lock); > + > + for_each_thermal_zone(tz) > + remove_sensor_from_zone(tz, ts); > + > + mutex_unlock(&zone_list_lock); > + > for (i = 0; i < ts->thresholds; i++) { > device_remove_file(&ts->device, &ts->thresh_attrs[i].attr); > if (ts->ops->get_hyst) { > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index 5470dae..2194519 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -55,6 +55,8 @@ > #define DEFAULT_THERMAL_GOVERNOR "user_space" > #endif > > +#define MAX_SENSORS_PER_ZONE 5 > + > struct thermal_sensor; > struct thermal_zone_device; > struct thermal_cooling_device; > @@ -202,6 +204,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]; > @@ -274,6 +291,11 @@ struct thermal_sensor *thermal_sensor_register(const char *, int, > struct thermal_sensor_ops *, void *); > void thermal_sensor_unregister(struct thermal_sensor *); > > +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 *); > +struct thermal_sensor *get_sensor_by_name(const char *); > + > #ifdef CONFIG_NET > extern int thermal_generate_netlink_event(struct thermal_zone_device *tz, > enum events event); -- 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
Hi Rui, > -----Original Message----- > From: Zhang, Rui > Sent: Friday, February 08, 2013 1:42 PM > To: R, Durgadoss > Cc: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org; > eduardo.valentin@ti.com; hongbo.zhang@linaro.org; wni@nvidia.com > Subject: Re: [PATCH 2/8] Thermal: Create zone level APIs > > On Tue, 2013-02-05 at 16:16 +0530, Durgadoss R wrote: > > This patch adds a new thermal_zone structure to > > thermal.h. Also, adds zone level APIs to the thermal > > framework. > > [snip.] > > + > > +struct thermal_sensor *get_sensor_by_name(const char *name) > > +{ > > + struct thermal_sensor *pos; > > + struct thermal_sensor *ts = NULL; > > + > > + mutex_lock(&sensor_list_lock); > > + for_each_thermal_sensor(pos) { > > + if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) > { > > + ts = pos; > > + break; > > this function depends on the assumption that all the sensor names are > unique. > thus I prefer to go through all the list and return -EINVAL if duplicate > names found, because in this case, the pointer returned may be not the > sensor we want to get. Yes, I agree with you. But I prefer having this check in the register API itself, which then will not allow duplicates. The reason being, we use this get* API (comparatively) a lot more than the register APIs. And putting this check in the register APIs means doing this check only once. Let me know what you think. And the same for cooling devices too. Thanks, Durga
On Fri, 2013-02-08 at 01:54 -0700, R, Durgadoss wrote: > Hi Rui, > > > -----Original Message----- > > From: Zhang, Rui > > Sent: Friday, February 08, 2013 1:42 PM > > To: R, Durgadoss > > Cc: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org; > > eduardo.valentin@ti.com; hongbo.zhang@linaro.org; wni@nvidia.com > > Subject: Re: [PATCH 2/8] Thermal: Create zone level APIs > > > > On Tue, 2013-02-05 at 16:16 +0530, Durgadoss R wrote: > > > This patch adds a new thermal_zone structure to > > > thermal.h. Also, adds zone level APIs to the thermal > > > framework. > > > > > [snip.] > > > > + > > > +struct thermal_sensor *get_sensor_by_name(const char *name) > > > +{ > > > + struct thermal_sensor *pos; > > > + struct thermal_sensor *ts = NULL; > > > + > > > + mutex_lock(&sensor_list_lock); > > > + for_each_thermal_sensor(pos) { > > > + if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) > > { > > > + ts = pos; > > > + break; > > > > this function depends on the assumption that all the sensor names are > > unique. > > thus I prefer to go through all the list and return -EINVAL if duplicate > > names found, because in this case, the pointer returned may be not the > > sensor we want to get. > > Yes, I agree with you. But I prefer having this check in the register API > itself, which then will not allow duplicates. > No, I do not think so. Unique cdev/sensor name is not a hard rule for generic thermal layer, and will not be. Because any cooling device driver does not have the technology that if its name is platform unique or not. Say, your platform thermal driver wants to use FAN cooling device, what if another FAN cooling device has been registered before the FAN your platform thermal driver wants to use get registered? If the platform thermal driver wants to use get_cdev/sensor_by_name(), it has already made the assumption that all the cooling devices have unique names. Thus duplicate names are a big issue, we should abort the platform thermal driver, rather than aborting the cooling device driver with duplicate names. > The reason being, we use this get* API (comparatively) a lot more than > the register APIs. why? why can not invoke get_sensor/cdev_by_name once and cache the pointer in the platform thermal driver? thanks, rui -- 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
> -----Original Message----- > From: Zhang, Rui > Sent: Friday, February 08, 2013 3:24 PM > To: R, Durgadoss > Cc: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org; > eduardo.valentin@ti.com; hongbo.zhang@linaro.org; wni@nvidia.com > Subject: RE: [PATCH 2/8] Thermal: Create zone level APIs > > On Fri, 2013-02-08 at 01:54 -0700, R, Durgadoss wrote: > > Hi Rui, > > > > > -----Original Message----- > > > From: Zhang, Rui > > > Sent: Friday, February 08, 2013 1:42 PM > > > To: R, Durgadoss > > > Cc: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org; > > > eduardo.valentin@ti.com; hongbo.zhang@linaro.org; wni@nvidia.com > > > Subject: Re: [PATCH 2/8] Thermal: Create zone level APIs > > > > > > On Tue, 2013-02-05 at 16:16 +0530, Durgadoss R wrote: > > > > This patch adds a new thermal_zone structure to > > > > thermal.h. Also, adds zone level APIs to the thermal > > > > framework. > > > > > > > > [snip.] > > > > > > + > > > > +struct thermal_sensor *get_sensor_by_name(const char *name) > > > > +{ > > > > + struct thermal_sensor *pos; > > > > + struct thermal_sensor *ts = NULL; > > > > + > > > > + mutex_lock(&sensor_list_lock); > > > > + for_each_thermal_sensor(pos) { > > > > + if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) > > > { > > > > + ts = pos; > > > > + break; > > > > > > this function depends on the assumption that all the sensor names are > > > unique. > > > thus I prefer to go through all the list and return -EINVAL if duplicate > > > names found, because in this case, the pointer returned may be not the > > > sensor we want to get. > > > > Yes, I agree with you. But I prefer having this check in the register API > > itself, which then will not allow duplicates. > > > No, I do not think so. > > Unique cdev/sensor name is not a hard rule for generic thermal layer, > and will not be. > Because any cooling device driver does not have the technology that if > its name is platform unique or not. > > Say, your platform thermal driver wants to use FAN cooling device, what > if another FAN cooling device has been registered before the FAN your > platform thermal driver wants to use get registered? > If the platform thermal driver wants to use get_cdev/sensor_by_name(), > it has already made the assumption that all the cooling devices have > unique names. Thus duplicate names are a big issue, we should abort the > platform thermal driver, rather than aborting the cooling device driver > with duplicate names. > > > The reason being, we use this get* API (comparatively) a lot more than > > the register APIs. > > why? > why can not invoke get_sensor/cdev_by_name once and cache the pointer > in > the platform thermal driver? Okay, I did not think of this caching part .. Then, I am fine with this change. Will fix it in next version. Thanks, Durga
On 05-02-2013 06:46, Durgadoss R wrote: > This patch adds a new thermal_zone structure to > thermal.h. Also, adds zone level APIs to the thermal > framework. > > A thermal zone is a hot spot on the platform, which > can have one or more sensors and cooling devices attached > to it. These sensors can be mapped to a set of cooling > devices, which when throttled, can help to bring down > the temperature of the hot spot. > > Signed-off-by: Durgadoss R <durgadoss.r@intel.com> > --- > drivers/thermal/thermal_sys.c | 196 +++++++++++++++++++++++++++++++++++++++++ > include/linux/thermal.h | 22 +++++ > 2 files changed, 218 insertions(+) > > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c > index cb94497..838d4fb 100644 > --- a/drivers/thermal/thermal_sys.c > +++ b/drivers/thermal/thermal_sys.c > @@ -43,19 +43,46 @@ 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(sensor_list_lock); > +static DEFINE_MUTEX(zone_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) > + > +#define GET_INDEX(tz, ptr, type) \ Why do you need type? You seam to be using it only for sensors. It becomes a bit cryptic :-) > +({ \ > + int i, ret = -EINVAL; \ > + do { \ > + if (!tz || !ptr) \ > + break; \ > + mutex_lock(&type##_list_lock); \ > + for (i = 0; i < tz->type##_indx; i++) { \ > + if (tz->type##s[i] == ptr) { \ > + ret = i; \ > + break; \ > + } \ > + } \ > + mutex_unlock(&type##_list_lock); \ > + } while (0); \ > + ret; \ > +}) > + > static struct thermal_governor *__find_governor(const char *name) > { > struct thermal_governor *pos; > @@ -421,15 +448,44 @@ static void thermal_zone_device_check(struct work_struct *work) > thermal_zone_device_update(tz); > } > > +static void remove_sensor_from_zone(struct thermal_zone *tz, > + struct thermal_sensor *ts) > +{ > + int j, indx; > + > + indx = GET_INDEX(tz, ts, sensor); > + 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); snprintf > +} > + > +static ssize_t > sensor_name_show(struct device *dev, struct device_attribute *attr, char *buf) > { > struct thermal_sensor *ts = to_thermal_sensor(dev); > @@ -811,6 +867,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) > @@ -1656,6 +1714,136 @@ static int enable_sensor_thresholds(struct thermal_sensor *ts, int count) > return 0; > } > > +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); devm_ helpers > + 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); strlcpy > + 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(&zone_list_lock); > + list_add_tail(&tz->node, &thermal_zone_list); > + mutex_unlock(&zone_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(&zone_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(&zone_list_lock); > + return; > +} > +EXPORT_SYMBOL(remove_thermal_zone); Do we need to impose removal ordering here? Meaning, what happens if the above is called with sensors registered? > + > +struct thermal_sensor *get_sensor_by_name(const char *name) Why do you need this function? does not seam to be used anywhere in this patch. Besides it is unrelated to what this patch proposes itself to do. > +{ > + struct thermal_sensor *pos; > + struct thermal_sensor *ts = NULL; > + > + mutex_lock(&sensor_list_lock); > + for_each_thermal_sensor(pos) { > + if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) { > + ts = pos; > + break; > + } > + } > + mutex_unlock(&sensor_list_lock); > + return ts; > +} > +EXPORT_SYMBOL(get_sensor_by_name); > + > +int add_sensor_to_zone(struct thermal_zone *tz, struct thermal_sensor *ts) > +{ > + int ret; > + > + if (!tz || !ts) > + return -EINVAL; > + > + mutex_lock(&zone_list_lock); > + > + /* Ensure we are not adding the same sensor again!! */ > + ret = GET_INDEX(tz, ts, sensor); > + if (ret >= 0) { > + ret = -EEXIST; > + goto exit_zone; > + } > + > + mutex_lock(&sensor_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; you may overflow your sensors buffer with the above implementation. you may have a contention on sensors/sensor_indx. Consider using linked lists. > + > +exit_sensor: > + mutex_unlock(&sensor_list_lock); > +exit_zone: > + mutex_unlock(&zone_list_lock); > + return ret; > +} > +EXPORT_SYMBOL(add_sensor_to_zone); > + > /** > * thermal_sensor_register - register a new thermal sensor > * @name: name of the thermal sensor > @@ -1732,6 +1920,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; > > @@ -1751,6 +1940,13 @@ void thermal_sensor_unregister(struct thermal_sensor *ts) > if (!found) > return; > > + mutex_lock(&zone_list_lock); > + > + for_each_thermal_zone(tz) > + remove_sensor_from_zone(tz, ts); > + > + mutex_unlock(&zone_list_lock); > + > for (i = 0; i < ts->thresholds; i++) { > device_remove_file(&ts->device, &ts->thresh_attrs[i].attr); > if (ts->ops->get_hyst) { > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index 5470dae..2194519 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -55,6 +55,8 @@ > #define DEFAULT_THERMAL_GOVERNOR "user_space" > #endif > > +#define MAX_SENSORS_PER_ZONE 5 Why not making it a linked list? Why does it need to be static? Just trying to avoid to maintain this number sane, as we cannot predict what happens in future :-) > + > struct thermal_sensor; > struct thermal_zone_device; > struct thermal_cooling_device; > @@ -202,6 +204,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]; > +}; Could you please add documentation for the above structure? Why it does not need locking? what is *ops? > + > /* Structure that holds thermal governor information */ > struct thermal_governor { > char name[THERMAL_NAME_LENGTH]; > @@ -274,6 +291,11 @@ struct thermal_sensor *thermal_sensor_register(const char *, int, > struct thermal_sensor_ops *, void *); > void thermal_sensor_unregister(struct thermal_sensor *); > > +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 *); > +struct thermal_sensor *get_sensor_by_name(const char *); > + I believe we need a better naming for this API. First suggestion is to use same prefix on all of them. Probably this comment applies not only to this specific patch. Besides, for all functions above, you may want to add comments documenting them and their parameters? > #ifdef CONFIG_NET > extern int thermal_generate_netlink_event(struct thermal_zone_device *tz, > enum events event); > -- 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
> -----Original Message----- > From: linux-pm-owner@vger.kernel.org [mailto:linux-pm- > owner@vger.kernel.org] On Behalf Of Eduardo Valentin > Sent: Friday, March 01, 2013 1:00 AM > To: R, Durgadoss > Cc: Zhang, Rui; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org; > hongbo.zhang@linaro.org; wni@nvidia.com > Subject: Re: [PATCH 2/8] Thermal: Create zone level APIs > > On 05-02-2013 06:46, Durgadoss R wrote: > > This patch adds a new thermal_zone structure to > > thermal.h. Also, adds zone level APIs to the thermal > > framework. > > > > A thermal zone is a hot spot on the platform, which > > can have one or more sensors and cooling devices attached > > to it. These sensors can be mapped to a set of cooling > > devices, which when throttled, can help to bring down > > the temperature of the hot spot. > > > > Signed-off-by: Durgadoss R <durgadoss.r@intel.com> > > --- > > drivers/thermal/thermal_sys.c | 196 > +++++++++++++++++++++++++++++++++++++++++ > > include/linux/thermal.h | 22 +++++ > > 2 files changed, 218 insertions(+) > > > > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c > > index cb94497..838d4fb 100644 > > --- a/drivers/thermal/thermal_sys.c > > +++ b/drivers/thermal/thermal_sys.c > > @@ -43,19 +43,46 @@ 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(sensor_list_lock); > > +static DEFINE_MUTEX(zone_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) > > + > > +#define GET_INDEX(tz, ptr, type) \ > > Why do you need type? You seam to be using it only for sensors. It > becomes a bit cryptic :-) In the next patch, we use it for cooling devices also. > > > +({ \ > > + int i, ret = -EINVAL; \ > > + do { \ > > + if (!tz || !ptr) \ > > + break; \ > > + mutex_lock(&type##_list_lock); \ > > + for (i = 0; i < tz->type##_indx; i++) { \ > > + if (tz->type##s[i] == ptr) { \ > > + ret = i; \ > > + break; \ > > + } \ > > + } \ > > + mutex_unlock(&type##_list_lock); \ > > + } while (0); \ > > + ret; \ > > +}) > > + > > static struct thermal_governor *__find_governor(const char *name) > > { > > struct thermal_governor *pos; > > @@ -421,15 +448,44 @@ static void thermal_zone_device_check(struct > work_struct *work) > > thermal_zone_device_update(tz); > > } > > > > +static void remove_sensor_from_zone(struct thermal_zone *tz, > > + struct thermal_sensor *ts) > > +{ > > + int j, indx; > > + > > + indx = GET_INDEX(tz, ts, sensor); > > + 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); > snprintf > > > +} > > + > > +static ssize_t > > sensor_name_show(struct device *dev, struct device_attribute *attr, > char *buf) > > { > > struct thermal_sensor *ts = to_thermal_sensor(dev); > > @@ -811,6 +867,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) > > @@ -1656,6 +1714,136 @@ static int enable_sensor_thresholds(struct > thermal_sensor *ts, int count) > > return 0; > > } > > > > +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); > > devm_ helpers > > > + 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); > > strlcpy > > > + 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(&zone_list_lock); > > + list_add_tail(&tz->node, &thermal_zone_list); > > + mutex_unlock(&zone_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(&zone_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(&zone_list_lock); > > + return; > > +} > > +EXPORT_SYMBOL(remove_thermal_zone); > > Do we need to impose removal ordering here? Meaning, what happens if > the > above is called with sensors registered? A zone only contains a symbolic link to a sensor. So, when this is called with sensors registered, all the symbolic links will go away i.e /sys/../zone/sensorX will go but /sys/../sensorX will remain as such. > > > + > > +struct thermal_sensor *get_sensor_by_name(const char *name) > Why do you need this function? does not seam to be used anywhere in this > patch. Besides it is unrelated to what this patch proposes itself to do. > > > +{ > > + struct thermal_sensor *pos; > > + struct thermal_sensor *ts = NULL; > > + > > + mutex_lock(&sensor_list_lock); > > + for_each_thermal_sensor(pos) { > > + if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) > { > > + ts = pos; > > + break; > > + } > > + } > > + mutex_unlock(&sensor_list_lock); > > + return ts; > > +} > > +EXPORT_SYMBOL(get_sensor_by_name); > > + > > +int add_sensor_to_zone(struct thermal_zone *tz, struct thermal_sensor > *ts) > > +{ > > + int ret; > > + > > + if (!tz || !ts) > > + return -EINVAL; > > + > > + mutex_lock(&zone_list_lock); > > + > > + /* Ensure we are not adding the same sensor again!! */ > > + ret = GET_INDEX(tz, ts, sensor); > > + if (ret >= 0) { > > + ret = -EEXIST; > > + goto exit_zone; > > + } > > + > > + mutex_lock(&sensor_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; > > you may overflow your sensors buffer with the above implementation. Will add a check before the increment. > > you may have a contention on sensors/sensor_indx. > > Consider using linked lists. > > > + > > +exit_sensor: > > + mutex_unlock(&sensor_list_lock); > > +exit_zone: > > + mutex_unlock(&zone_list_lock); > > + return ret; > > +} > > +EXPORT_SYMBOL(add_sensor_to_zone); > > + > > /** > > * thermal_sensor_register - register a new thermal sensor > > * @name: name of the thermal sensor > > @@ -1732,6 +1920,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; > > > > @@ -1751,6 +1940,13 @@ void thermal_sensor_unregister(struct > thermal_sensor *ts) > > if (!found) > > return; > > > > + mutex_lock(&zone_list_lock); > > + > > + for_each_thermal_zone(tz) > > + remove_sensor_from_zone(tz, ts); > > + > > + mutex_unlock(&zone_list_lock); > > + > > for (i = 0; i < ts->thresholds; i++) { > > device_remove_file(&ts->device, &ts->thresh_attrs[i].attr); > > if (ts->ops->get_hyst) { > > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > > index 5470dae..2194519 100644 > > --- a/include/linux/thermal.h > > +++ b/include/linux/thermal.h > > @@ -55,6 +55,8 @@ > > #define DEFAULT_THERMAL_GOVERNOR "user_space" > > #endif > > > > +#define MAX_SENSORS_PER_ZONE 5 > > > Why not making it a linked list? Why does it need to be static? Just > trying to avoid to maintain this number sane, as we cannot predict what > happens in future :-) As I said earlier, making it a list makes things even more complex. But I wanted this to be configurable through Kconfig, for which I submitted a patch in my v2. But we had some comments against it. So, we removed it, and need to revisit this later. Thanks, Durga > > > + > > struct thermal_sensor; > > struct thermal_zone_device; > > struct thermal_cooling_device; > > @@ -202,6 +204,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]; > > +}; > > > Could you please add documentation for the above structure? Why it does > not need locking? what is *ops? > > + > > /* Structure that holds thermal governor information */ > > struct thermal_governor { > > char name[THERMAL_NAME_LENGTH]; > > @@ -274,6 +291,11 @@ struct thermal_sensor > *thermal_sensor_register(const char *, int, > > struct thermal_sensor_ops *, void *); > > void thermal_sensor_unregister(struct thermal_sensor *); > > > > +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 > *); > > +struct thermal_sensor *get_sensor_by_name(const char *); > > + > > I believe we need a better naming for this API. First suggestion is to > use same prefix on all of them. Probably this comment applies not only > to this specific patch. > > Besides, for all functions above, you may want to add comments > documenting them and their parameters? > > > #ifdef CONFIG_NET > > extern int thermal_generate_netlink_event(struct thermal_zone_device > *tz, > > enum events event); > > > > -- > 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 -- 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 --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c index cb94497..838d4fb 100644 --- a/drivers/thermal/thermal_sys.c +++ b/drivers/thermal/thermal_sys.c @@ -43,19 +43,46 @@ 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(sensor_list_lock); +static DEFINE_MUTEX(zone_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) + +#define GET_INDEX(tz, ptr, type) \ +({ \ + int i, ret = -EINVAL; \ + do { \ + if (!tz || !ptr) \ + break; \ + mutex_lock(&type##_list_lock); \ + for (i = 0; i < tz->type##_indx; i++) { \ + if (tz->type##s[i] == ptr) { \ + ret = i; \ + break; \ + } \ + } \ + mutex_unlock(&type##_list_lock); \ + } while (0); \ + ret; \ +}) + static struct thermal_governor *__find_governor(const char *name) { struct thermal_governor *pos; @@ -421,15 +448,44 @@ static void thermal_zone_device_check(struct work_struct *work) thermal_zone_device_update(tz); } +static void remove_sensor_from_zone(struct thermal_zone *tz, + struct thermal_sensor *ts) +{ + int j, indx; + + indx = GET_INDEX(tz, ts, sensor); + 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); @@ -811,6 +867,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) @@ -1656,6 +1714,136 @@ static int enable_sensor_thresholds(struct thermal_sensor *ts, int count) return 0; } +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(&zone_list_lock); + list_add_tail(&tz->node, &thermal_zone_list); + mutex_unlock(&zone_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(&zone_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(&zone_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(&sensor_list_lock); + for_each_thermal_sensor(pos) { + if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) { + ts = pos; + break; + } + } + mutex_unlock(&sensor_list_lock); + return ts; +} +EXPORT_SYMBOL(get_sensor_by_name); + +int add_sensor_to_zone(struct thermal_zone *tz, struct thermal_sensor *ts) +{ + int ret; + + if (!tz || !ts) + return -EINVAL; + + mutex_lock(&zone_list_lock); + + /* Ensure we are not adding the same sensor again!! */ + ret = GET_INDEX(tz, ts, sensor); + if (ret >= 0) { + ret = -EEXIST; + goto exit_zone; + } + + mutex_lock(&sensor_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(&sensor_list_lock); +exit_zone: + mutex_unlock(&zone_list_lock); + return ret; +} +EXPORT_SYMBOL(add_sensor_to_zone); + /** * thermal_sensor_register - register a new thermal sensor * @name: name of the thermal sensor @@ -1732,6 +1920,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; @@ -1751,6 +1940,13 @@ void thermal_sensor_unregister(struct thermal_sensor *ts) if (!found) return; + mutex_lock(&zone_list_lock); + + for_each_thermal_zone(tz) + remove_sensor_from_zone(tz, ts); + + mutex_unlock(&zone_list_lock); + for (i = 0; i < ts->thresholds; i++) { device_remove_file(&ts->device, &ts->thresh_attrs[i].attr); if (ts->ops->get_hyst) { diff --git a/include/linux/thermal.h b/include/linux/thermal.h index 5470dae..2194519 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -55,6 +55,8 @@ #define DEFAULT_THERMAL_GOVERNOR "user_space" #endif +#define MAX_SENSORS_PER_ZONE 5 + struct thermal_sensor; struct thermal_zone_device; struct thermal_cooling_device; @@ -202,6 +204,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]; @@ -274,6 +291,11 @@ struct thermal_sensor *thermal_sensor_register(const char *, int, struct thermal_sensor_ops *, void *); void thermal_sensor_unregister(struct thermal_sensor *); +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 *); +struct thermal_sensor *get_sensor_by_name(const char *); + #ifdef CONFIG_NET extern int thermal_generate_netlink_event(struct thermal_zone_device *tz, enum events event);
This patch adds a new thermal_zone structure to thermal.h. Also, adds zone level APIs to the thermal framework. A thermal zone is a hot spot on the platform, which can have one or more sensors and cooling devices attached to it. These sensors can be mapped to a set of cooling devices, which when throttled, can help to bring down the temperature of the hot spot. Signed-off-by: Durgadoss R <durgadoss.r@intel.com> --- drivers/thermal/thermal_sys.c | 196 +++++++++++++++++++++++++++++++++++++++++ include/linux/thermal.h | 22 +++++ 2 files changed, 218 insertions(+)