2013-10-01 13:10:38

by R, Durgadoss

Subject: [PATCHv4 0/9] Thermal Framework Enhancements

This patch set is a v4 of the previous versions submitted here:
[v3]: https://lkml.org/lkml/2013/2/5/228
[v2]: http://lwn.net/Articles/531720/
[v1]: https://lkml.org/lkml/2012/12/18/108
[RFC]:https://patchwork.kernel.org/patch/1758921/

This patch set is based on Rui's -next tree, on top
of commit 'f61d5b4d52e077756ce9dbc47ce737da898ad01d'
This is tested on a Core-i5 and an Atom netbook,
running ubuntu 12.04.

Changes since v3:
* Added a patch to conditionally do kfree(cdev) in
thermal_release function.
* Reworked all sysfs attributes to have one value per file
This includes sensor_trip_* and map_weight* attributes.
* Added 'lock' variable in thermal_zone structure to
protect its members.
* Added Documentation to all functions in thermal_core.c
* Changes all strcpy() to strlcpy()
* Used devm_kzalloc() in places where applicable
* Address some buffer overflow conditions and contentions
in tz->sensors[] and tz->cdevs[].
Changes since v2:
* Reworked the map sysfs attributes in patch [5/8]
* Dropped configuration for maximum sensors and
cooling devices, through Kconfig.
* Added __remove_trip_attr method
* Renamed __clean_map_entry to __remove_map_entry
for consistency in naming.
Changes Since v1:
* Removed kobject creation for thermal_trip and thermal_map
nodes as per Greg-KH's comments.
* Added ABI Documentation under 'testing'.
* Modified the GET_INDEX macro to be more linux-like, thanks
to Joe Perches.
* Added get_[sensor/cdev]_by_name APIs to thermal.h

This series contains 9 patches:
Patch 1/9: Fixes a kfree issue. This is required so that the new sensor
and zone devices are not freed accidently.
We can do two things here:
1) Conditionally free every device
2) Remove this _release function, and free the memory
in corresponding _unregister functions.
I prefer 2) and we can have this as a separate patch
outside this series; but would like to see the opinion
of maintainers'.
Patch 2/9: Creates new sensor level APIs
Patch 3/9: Creates new zone level APIs. The existing tzd structure is
kept as such for clarity and compatibility purposes.
Patch 4/9: Creates functions to add/remove a cdev to/from a zone. The
existing tcd structure need not be modified.
Patch 5/9: Adds sensorX_trip_[active/passive/hot/critical] sysfs nodes,
under /sys/class/thermal/zoneY/. This exposes various trip
points for sensorX present in zoneY.
Patch 6/9: Adds mapY_* sysfs node. These attributes represent
Patch 7/9: Creates Documentation for the new APIs. A new file is
created for clarity. Final goal is to merge with the existing
file or refactor the files, as whatever seems appropriate.
Patch 8/9: Add ABI documentation for sysfs interfaces introduced in this patch.
Patch 9/9: A dummy driver that can be used for testing. This is not for merge.

Sorry for the long delay in resubmitting this patch set.
Thanks to Eduardo, Wei Ni, for their comments on v3.

Durgadoss R (9):
Thermal: Check for validity before doing kfree
Thermal: Create sensor level APIs
Thermal: Create zone level APIs
Thermal: Add APIs to bind cdev to new zone structure
Thermal: Add trip point sysfs nodes for sensor
Thermal: Create Thermal map sysfs attributes for a zone
Thermal: Add Documentation to new APIs
Thermal: Add ABI Documentation for sysfs interfaces
Thermal: Dummy driver used for testing

Documentation/ABI/testing/sysfs-class-thermal | 137 +++
Documentation/thermal/sysfs-api2.txt | 248 +++++
drivers/thermal/Kconfig | 5 +
drivers/thermal/Makefile | 3 +
drivers/thermal/thermal_core.c | 1406 +++++++++++++++++++++++--
drivers/thermal/thermal_test.c | 322 ++++++
include/linux/thermal.h | 127 ++-
7 files changed, 2180 insertions(+), 68 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-class-thermal
create mode 100644 Documentation/thermal/sysfs-api2.txt
create mode 100644 drivers/thermal/thermal_test.c

--
1.7.9.5

2013-10-01 13:10:42

by R, Durgadoss

Subject: [PATCHv4 1/9] Thermal: Check for validity before doing kfree

The thermal_release function is called whenever
any device belonging to 'thermal' class unregisters.
This function performs kfree(cdev) without any check.
In cases where there are more device registrations
other than just 'thermal_zone' and 'cooling_device'
this might accidently free memory allocated them
silently; and cause memory errors.

This patch changes this behavior by doing
kfree(cdev) only when the device pointer belongs
to a real cdev i.e. cooling_device.

Signed-off-by: Durgadoss R <[email protected]>
---
drivers/thermal/thermal_core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 1f02e8e..8c5131d 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1280,7 +1280,8 @@ static void thermal_release(struct device *dev)
sizeof("thermal_zone") - 1)) {
tz = to_thermal_zone(dev);
kfree(tz);
- } else {
+ } else if(!strncmp(dev_name(dev), "cooling_device",
+ sizeof("cooling_device") - 1)){
cdev = to_cooling_device(dev);
kfree(cdev);
}
--
1.7.9.5

2013-10-01 13:10:49

by R, Durgadoss

Subject: [PATCHv4 3/9] Thermal: Create zone level APIs

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 <[email protected]>
---
drivers/thermal/thermal_core.c | 267 +++++++++++++++++++++++++++++++++++++++-
include/linux/thermal.h | 23 ++++
2 files changed, 289 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 8c46567..a053b60 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -44,19 +44,48 @@ MODULE_DESCRIPTION("Generic thermal management sysfs support");
MODULE_LICENSE("GPL v2");

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(&tz->lock); \
+ 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); \
+ mutex_unlock(&tz->lock); \
+ } while (0); \
+ ret; \
+})
+
static struct thermal_governor *__find_governor(const char *name)
{
struct thermal_governor *pos;
@@ -461,15 +490,47 @@ 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));
+
+ mutex_lock(&tz->lock);
+
+ /* 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--;
+ mutex_unlock(&tz->lock);
+}
+
/* 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);
@@ -876,6 +937,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)
@@ -1689,7 +1752,7 @@ static void remove_trip_attrs(struct thermal_zone_device *tz)
kfree(tz->trip_hyst_attrs);
}

- /**
+/**
* enable_sensor_thresholds - create sysfs nodes for thresholdX
* @ts: the thermal sensor
* @count: Number of thresholds supported by sensor hardware
@@ -1746,6 +1809,200 @@ static int enable_sensor_thresholds(struct thermal_sensor *ts, int count)
}

/**
+ * create_thermal_zone - create sysfs nodes for thermal zone
+ * @name: Name of the thermla zone
+ * @devdata: Data supplied by the caller
+ *
+ * On Success returns a thermal zone reference, otherwise:
+ * -EINVAL for invalid parameters,
+ * -ENOMEM for insufficient memory cases,
+ */
+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;
+
+ mutex_init(&tz->lock);
+ strlcpy(tz->name, name, sizeof(tz->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:
+ mutex_destroy(&tz->lock);
+ release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
+exit_free:
+ kfree(tz);
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(create_thermal_zone);
+
+/**
+ * remove_thermal_zone - removes the sysfs nodes for given tz
+ * @tz: Thermal zone to be removed.
+ */
+void remove_thermal_zone(struct thermal_zone *tz)
+{
+ struct thermal_zone *pos, *next;
+ struct thermal_sensor *ts;
+ 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;
+
+ for_each_thermal_sensor(ts)
+ remove_sensor_from_zone(tz, ts);
+
+ device_remove_file(&tz->device, &dev_attr_zone_name);
+
+ mutex_destroy(&tz->lock);
+ 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);
+
+/**
+ * get_sensor_by_name() - search for a sensor and returns its reference
+ * @name: thermal sensor name to fetch the reference
+ *
+ * On success returns a reference to a unique sensor with
+ * name matching that of @name, an ERR_PTR otherwise:
+ * -EINVAL for invalid paramenters
+ * -ENODEV for sensor not found
+ * -EEXIST for multiple matches
+ */
+struct thermal_sensor *get_sensor_by_name(const char *name)
+{
+ int found = 0;
+ struct thermal_sensor *pos;
+ struct thermal_sensor *ts = ERR_PTR(-EINVAL);
+
+ if (!name)
+ return ts;
+
+ mutex_lock(&sensor_list_lock);
+ for_each_thermal_sensor(pos) {
+ if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) {
+ found++;
+ ts = pos;
+ }
+ }
+ mutex_unlock(&sensor_list_lock);
+
+ if (found == 0)
+ ts = ERR_PTR(-ENODEV);
+ else if (found > 1)
+ ts = ERR_PTR(-EEXIST);
+
+ return ts;
+}
+EXPORT_SYMBOL(get_sensor_by_name);
+
+/**
+ * add_sensor_to_zone - Add @ts to thermal zone @tz
+ * @tz: Thermal zone reference
+ * @ts: Thermal sensor reference
+ *
+ * Returns 0 on success, otherwise
+ * -EINVAL for invalid paramenters
+ * -EINVAL when trying to add more zones than MAX_SENSORS_PER_ZONE
+ * -EEXIST when trying add existing thermal sensor again
+ */
+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;
+ }
+
+ /* Protect against 'ts' being unregistered */
+ 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;
+
+ mutex_lock(&tz->lock);
+
+ if (tz->sensor_indx >= MAX_SENSORS_PER_ZONE - 1) {
+ dev_err(&tz->device, "Only %d sensors allowed per zone\n",
+ MAX_SENSORS_PER_ZONE);
+ sysfs_remove_link(&tz->device.kobj,
+ kobject_name(&ts->device.kobj));
+ ret = -EINVAL;
+ goto exit_indx_check;
+ }
+
+ tz->sensors[tz->sensor_indx++] = ts;
+
+exit_indx_check:
+ mutex_unlock(&tz->lock);
+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
* @count: Number of thresholds supported by hardware
@@ -1829,6 +2086,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;

@@ -1848,6 +2106,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 25fc562..2e12321 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -58,6 +58,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;
@@ -207,6 +209,22 @@ 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 mutex lock; /* protect elements of this structure */
+ 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];
@@ -277,6 +295,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);
--
1.7.9.5

2013-10-01 13:10:52

by R, Durgadoss

Subject: [PATCHv4 4/9] Thermal: Add APIs to bind cdev to new zone structure

This patch creates new APIs to add/remove a
cdev to/from a zone. This patch does not change
the old cooling device implementation.

Signed-off-by: Durgadoss R <[email protected]>
---
drivers/thermal/thermal_core.c | 136 ++++++++++++++++++++++++++++++++++++++++
include/linux/thermal.h | 9 +++
2 files changed, 145 insertions(+)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index a053b60..3c4ef62 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -58,6 +58,7 @@ 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(cdev_list_lock);
static DEFINE_MUTEX(thermal_governor_lock);

#define for_each_thermal_sensor(pos) \
@@ -86,6 +87,9 @@ static DEFINE_MUTEX(thermal_governor_lock);
ret; \
})

+#define for_each_cdev(pos) \
+ list_for_each_entry(pos, &thermal_cdev_list, node)
+
static struct thermal_governor *__find_governor(const char *name)
{
struct thermal_governor *pos;
@@ -511,6 +515,27 @@ static void remove_sensor_from_zone(struct thermal_zone *tz,
mutex_unlock(&tz->lock);
}

+static void remove_cdev_from_zone(struct thermal_zone *tz,
+ struct thermal_cooling_device *cdev)
+{
+ int j, indx;
+
+ indx = GET_INDEX(tz, cdev, cdev);
+ if (indx < 0)
+ return;
+
+ sysfs_remove_link(&tz->device.kobj, kobject_name(&cdev->device.kobj));
+
+ mutex_lock(&tz->lock);
+
+ /* Shift the entries in the tz->cdevs array */
+ for (j = indx; j < MAX_CDEVS_PER_ZONE - 1; j++)
+ tz->cdevs[j] = tz->cdevs[j + 1];
+
+ tz->cdev_indx--;
+ mutex_unlock(&tz->lock);
+}
+
/* sys I/F for thermal zone */

#define to_thermal_zone(_dev) \
@@ -1555,6 +1580,7 @@ void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)
int i;
const struct thermal_zone_params *tzp;
struct thermal_zone_device *tz;
+ struct thermal_zone *tmp_tz;
struct thermal_cooling_device *pos = NULL;

if (!cdev)
@@ -1592,6 +1618,13 @@ void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)

mutex_unlock(&thermal_list_lock);

+ mutex_lock(&zone_list_lock);
+
+ for_each_thermal_zone(tmp_tz)
+ remove_cdev_from_zone(tmp_tz, cdev);
+
+ mutex_unlock(&zone_list_lock);
+
if (cdev->type[0])
device_remove_file(&cdev->device, &dev_attr_cdev_type);
device_remove_file(&cdev->device, &dev_attr_max_state);
@@ -1873,6 +1906,7 @@ void remove_thermal_zone(struct thermal_zone *tz)
{
struct thermal_zone *pos, *next;
struct thermal_sensor *ts;
+ struct thermal_cooling_device *cdev;
bool found = false;

if (!tz)
@@ -1894,6 +1928,9 @@ void remove_thermal_zone(struct thermal_zone *tz)
for_each_thermal_sensor(ts)
remove_sensor_from_zone(tz, ts);

+ for_each_cdev(cdev)
+ remove_cdev_from_zone(tz, cdev);
+
device_remove_file(&tz->device, &dev_attr_zone_name);

mutex_destroy(&tz->lock);
@@ -1909,6 +1946,47 @@ exit:
EXPORT_SYMBOL(remove_thermal_zone);

/**
+ * get_cdev_by_name() - search for a cdev and returns its reference
+ * @name: cooling device name to fetch the reference
+ *
+ * On success returns a reference to an unique cdev with
+ * the name matching that of @name, an ERR_PTR otherwise:
+ * -EINVAL for invalid paramenters
+ * -ENODEV for cdev not found
+ * -EEXIST for multiple matches
+ */
+struct thermal_cooling_device *get_cdev_by_name(const char *name)
+{
+ int found = 0;
+ struct thermal_cooling_device *pos;
+ struct thermal_cooling_device *cdev = ERR_PTR(-EINVAL);
+
+ if (!name)
+ return cdev;
+
+ mutex_lock(&cdev_list_lock);
+ for_each_cdev(pos) {
+ if (!strnicmp(pos->type, name, THERMAL_NAME_LENGTH)) {
+ found++;
+ cdev = pos;
+ }
+ }
+ mutex_unlock(&cdev_list_lock);
+
+ /*
+ * Nothing has been found; return an error code for it.
+ * Return success only when an unique cdev is found.
+ */
+ if (found == 0)
+ cdev = ERR_PTR(-ENODEV);
+ else if (found > 1)
+ cdev = ERR_PTR(-EEXIST);
+
+ return cdev;
+}
+EXPORT_SYMBOL(get_cdev_by_name);
+
+/**
* get_sensor_by_name() - search for a sensor and returns its reference
* @name: thermal sensor name to fetch the reference
*
@@ -2003,6 +2081,64 @@ exit_zone:
EXPORT_SYMBOL(add_sensor_to_zone);

/**
+ * add_cdev_to_zone - Add @cdev to thermal zone @tz
+ * @tz: Thermal zone reference
+ * @cdev: Cooling device reference
+ *
+ * Returns 0 on success, otherwise
+ * -EINVAL for invalid paramenters
+ * -EINVAL when trying to add more cdevs than MAX_CDEVS_PER_ZONE
+ * -EEXIST when trying add existing cdev again
+ */
+int add_cdev_to_zone(struct thermal_zone *tz,
+ struct thermal_cooling_device *cdev)
+{
+ int ret;
+
+ if (!tz || !cdev)
+ return -EINVAL;
+
+ mutex_lock(&zone_list_lock);
+
+ /* Ensure we are not adding the same cdev again!! */
+ ret = GET_INDEX(tz, cdev, cdev);
+ if (ret >= 0) {
+ ret = -EEXIST;
+ goto exit_zone;
+ }
+
+ /* Protect against 'cdev' being unregistered */
+ mutex_lock(&cdev_list_lock);
+
+ ret = sysfs_create_link(&tz->device.kobj, &cdev->device.kobj,
+ kobject_name(&cdev->device.kobj));
+ if (ret)
+ goto exit_cdev;
+
+ mutex_lock(&tz->lock);
+
+ if (tz->cdev_indx >= MAX_CDEVS_PER_ZONE - 1) {
+ dev_err(&tz->device, "Only %d cdevs allowed per zone\n",
+ MAX_CDEVS_PER_ZONE);
+ sysfs_remove_link(&tz->device.kobj,
+ kobject_name(&cdev->device.kobj));
+ ret = -EINVAL;
+ goto exit_indx_check;
+ }
+
+ tz->cdevs[tz->cdev_indx++] = cdev;
+
+exit_indx_check:
+ mutex_unlock(&tz->lock);
+exit_cdev:
+ mutex_unlock(&cdev_list_lock);
+exit_zone:
+ mutex_unlock(&zone_list_lock);
+ return ret;
+}
+EXPORT_SYMBOL(add_cdev_to_zone);
+
+/**
* thermal_sensor_register - register a new thermal sensor
* @name: name of the thermal sensor
* @count: Number of thresholds supported by hardware
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 2e12321..da7520c 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -60,6 +60,8 @@

#define MAX_SENSORS_PER_ZONE 5

+#define MAX_CDEVS_PER_ZONE 5
+
struct thermal_sensor;
struct thermal_zone_device;
struct thermal_cooling_device;
@@ -223,6 +225,10 @@ struct thermal_zone {
/* Sensor level information */
int sensor_indx; /* index into 'sensors' array */
struct thermal_sensor *sensors[MAX_SENSORS_PER_ZONE];
+
+ /* cdev level information */
+ int cdev_indx; /* index into 'cdevs' array */
+ struct thermal_cooling_device *cdevs[MAX_CDEVS_PER_ZONE];
};

/* Structure that holds thermal governor information */
@@ -300,6 +306,9 @@ 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 *);

+int add_cdev_to_zone(struct thermal_zone *, struct thermal_cooling_device *);
+struct thermal_cooling_device *get_cdev_by_name(const char *);
+
#ifdef CONFIG_NET
extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,
enum events event);
--
1.7.9.5

2013-10-01 13:11:02

by R, Durgadoss

Subject: [PATCHv4 9/9] Thermal: Dummy driver used for testing

This patch has a dummy driver that can be used for
testing purposes. This patch is not for merge.

Signed-off-by: Durgadoss R <[email protected]>
---
drivers/thermal/Kconfig | 5 +
drivers/thermal/Makefile | 3 +
drivers/thermal/thermal_test.c | 322 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 330 insertions(+)
create mode 100644 drivers/thermal/thermal_test.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index ae18f02..d4b8458 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -192,4 +192,9 @@ depends on PLAT_SAMSUNG
source "drivers/thermal/samsung/Kconfig"
endmenu

+config THERMAL_TEST
+ tristate "test driver"
+ help
+ Enable this to test the thermal framework.
+
endif
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index c19df7a..ccff09e 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -26,3 +26,6 @@ obj-$(CONFIG_DB8500_CPUFREQ_COOLING) += db8500_cpufreq_cooling.o
obj-$(CONFIG_INTEL_POWERCLAMP) += intel_powerclamp.o
obj-$(CONFIG_X86_PKG_TEMP_THERMAL) += x86_pkg_temp_thermal.o
obj-$(CONFIG_TI_SOC_THERMAL) += ti-soc-thermal/
+
+# dummy driver for testing
+obj-$(CONFIG_THERMAL_TEST) += thermal_test.o
diff --git a/drivers/thermal/thermal_test.c b/drivers/thermal/thermal_test.c
new file mode 100644
index 0000000..6d34bca
--- /dev/null
+++ b/drivers/thermal/thermal_test.c
@@ -0,0 +1,322 @@
+/*
+ * thermal_test.c - This driver can be used to test Thermal
+ * Framework changes. Not specific to any
+ * platform. Fills the log buffer generously ;)
+ *
+ * Copyright (C) 2012 Intel Corporation
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * Author: Durgadoss R <[email protected]>
+ */
+
+#define pr_fmt(fmt) "thermal_test: " fmt
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/param.h>
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <linux/pm.h>
+#include <linux/platform_device.h>
+#include <linux/thermal.h>
+
+#define MAX_THERMAL_ZONES 2
+#define MAX_THERMAL_SENSORS 2
+#define MAX_COOLING_DEVS 4
+#define NUM_THRESHOLDS 3
+
+static struct ts_data {
+ int curr_temp;
+ int flag;
+} ts_data;
+
+int active_trips[10] = {100, 90, 80, 70, 60, 50, 40, 30, 20, 10};
+int passive_trips[5] = {100, 90, 60, 50, 40};
+int wts[5] = {50, 50, 50, 50, 40};
+
+static struct platform_device *pdev;
+static unsigned long cur_cdev_state = 2;
+static struct thermal_sensor *ts, *ts1;
+static struct thermal_zone *tz;
+static struct thermal_cooling_device *cdev;
+
+static long thermal_thresholds[NUM_THRESHOLDS] = {30000, 40000, 50000};
+
+static struct thermal_trip_point trip = {
+ .hot = 90,
+ .crit = 100,
+ .num_passive_trips = 5,
+ .passive_trips = passive_trips,
+ .num_active_trips = 10,
+ .active_trips = active_trips,
+};
+
+static struct thermal_trip_point trip1 = {
+ .hot = 95,
+ .crit = 125,
+ .num_passive_trips = 0,
+ .passive_trips = passive_trips,
+ .num_active_trips = 6,
+ .active_trips = active_trips,
+};
+
+static struct thermal_map map = {
+ .trip_type = THERMAL_TRIP_PASSIVE,
+ .sensor_name = "ts",
+ .cdev_name = "cdev",
+ .num_weights = 5,
+ .trip_mask = 0x0F,
+ .weights = wts,
+};
+
+static int read_cur_state(struct thermal_cooling_device *cdev,
+ unsigned long *state)
+{
+ *state = cur_cdev_state;
+ return 0;
+}
+
+static int write_cur_state(struct thermal_cooling_device *cdev,
+ unsigned long state)
+{
+ cur_cdev_state = state;
+ return 0;
+}
+
+static int read_max_state(struct thermal_cooling_device *cdev,
+ unsigned long *state)
+{
+ *state = 5;
+ return 0;
+}
+
+static int read_curr_temp(struct thermal_sensor *ts, long *temp)
+{
+ *temp = ts_data.curr_temp;
+ return 0;
+}
+
+static ssize_t
+flag_show(struct device *dev, struct device_attribute *devattr, char *buf)
+{
+ return sprintf(buf, "%d\n", ts_data.flag);
+}
+
+static ssize_t
+flag_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ long flag;
+
+ if (kstrtol(buf, 10, &flag))
+ return -EINVAL;
+
+ ts_data.flag = flag;
+
+ if (flag == 0) {
+ thermal_sensor_unregister(ts);
+ ts = NULL;
+ pr_err("thermal_sensor_unregister (ts) done\n");
+ } else if (flag == 1) {
+ thermal_sensor_unregister(ts1);
+ ts1 = NULL;
+ pr_err("thermal_sensor_unregister (ts1) done\n");
+ } else if (flag == 2) {
+ thermal_cooling_device_unregister(cdev);
+ cdev = NULL;
+ pr_err("cdev unregister (cdev) done\n");
+ } else if (flag == 3) {
+ if (tz)
+ remove_thermal_zone(tz);
+ tz = NULL;
+ pr_err("removed thermal zone\n");
+ }
+
+ return count;
+}
+
+static ssize_t
+temp_show(struct device *dev, struct device_attribute *devattr, char *buf)
+{
+ return sprintf(buf, "%d\n", ts_data.curr_temp);
+}
+
+static int read_threshold(struct thermal_sensor *ts, int indx, long *val)
+{
+ if (indx < 0 || indx >= NUM_THRESHOLDS)
+ return -EINVAL;
+
+ *val = thermal_thresholds[indx];
+ return 0;
+}
+
+static int write_threshold(struct thermal_sensor *ts, int indx, long val)
+{
+ if (indx < 0 || indx >= NUM_THRESHOLDS)
+ return -EINVAL;
+
+ thermal_thresholds[indx] = val;
+ return 0;
+}
+
+static ssize_t
+temp_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ long temp;
+
+ if (kstrtol(buf, 10, &temp))
+ return -EINVAL;
+
+ ts_data.curr_temp = temp;
+ return count;
+}
+
+static struct thermal_sensor_ops ts_ops = {
+ .get_temp = read_curr_temp,
+ .get_threshold = read_threshold,
+ .set_threshold = write_threshold,
+};
+
+static struct thermal_sensor_ops ts1_ops = {
+ .get_temp = read_curr_temp,
+ .get_threshold = read_threshold,
+ .set_threshold = write_threshold,
+};
+
+static struct thermal_cooling_device_ops cdev_ops = {
+ .get_cur_state = read_cur_state,
+ .set_cur_state = write_cur_state,
+ .get_max_state = read_max_state,
+};
+
+static DEVICE_ATTR(test_temp, S_IRUGO | S_IWUSR, temp_show, temp_store);
+static DEVICE_ATTR(sensor_enable, S_IRUGO | S_IWUSR, flag_show, flag_store);
+
+static int thermal_test_probe(struct platform_device *pdev)
+{
+ int ret;
+
+ ts_data.curr_temp = 30000;
+ ts_data.flag = 1;
+
+ ts = thermal_sensor_register("ts", NUM_THRESHOLDS, &ts_ops, &ts_data);
+ if (!ts) {
+ pr_err("thermal_sensor_register failed:\n");
+ return -EINVAL;
+ }
+
+ ts1 = thermal_sensor_register("ts1", NUM_THRESHOLDS, &ts1_ops, NULL);
+
+ cdev = thermal_cooling_device_register("cdev", NULL, &cdev_ops);
+ if (!cdev) {
+ pr_err("cdev_register failed:\n");
+ return -EINVAL;
+ }
+
+ device_create_file(&pdev->dev, &dev_attr_test_temp);
+ device_create_file(&pdev->dev, &dev_attr_sensor_enable);
+
+ /* Create a zone */
+ tz = create_thermal_zone("myZone", NULL);
+ if (!tz) {
+ pr_err("create_thermal_zone failed:\n");
+ return -EINVAL;
+ }
+
+ pr_err("Zone created successfully..\n");
+
+ ret = add_sensor_to_zone(tz, ts);
+ if (ret) {
+ pr_err("add_sensor_to_zone failed:%d\n", ret);
+ return ret;
+ }
+
+ ret = add_sensor_to_zone(tz, ts1);
+ pr_err("add_sensor (ts1) ret_val: %d\n", ret);
+
+ ret = add_cdev_to_zone(tz, cdev);
+ pr_err("add_cdev_to_zone (cdev) ret_val: %d\n", ret);
+
+ ret = add_sensor_trip_info(tz, ts, &trip);
+ ret = add_sensor_trip_info(tz, ts1, &trip1);
+ pr_err("add_sensor_trip_info (ts) ret_val: %d\n", ret);
+
+ ret = add_map_entry(tz, &map);
+ pr_err("add_map_entry (ts) ret_val: %d\n", ret);
+
+ return 0;
+}
+
+static int thermal_test_remove(struct platform_device *pdev)
+{
+ device_remove_file(&pdev->dev, &dev_attr_test_temp);
+ device_remove_file(&pdev->dev, &dev_attr_sensor_enable);
+
+ return 0;
+}
+
+/*********************************************************************
+ * Driver initialization and finalization
+ *********************************************************************/
+
+#define DRIVER_NAME "thermal_test"
+
+static const struct platform_device_id therm_id_table[] = {
+ { DRIVER_NAME, 1 },
+};
+
+static struct platform_driver thermal_test_driver = {
+ .driver = {
+ .name = DRIVER_NAME,
+ .owner = THIS_MODULE,
+ },
+ .probe = thermal_test_probe,
+ .remove = thermal_test_remove,
+ .id_table = therm_id_table,
+};
+
+static int __init thermal_test_init(void)
+{
+ int ret;
+
+ ret = platform_driver_register(&thermal_test_driver);
+ if (ret) {
+ pr_err("platform driver register failed:%d\n", ret);
+ return ret;
+ }
+
+ pdev = platform_device_register_simple(DRIVER_NAME, -1, NULL, 0);
+ if (IS_ERR(pdev)) {
+ ret = PTR_ERR(pdev);
+ pr_err("platform device register failed:%d\n", ret);
+ platform_driver_unregister(&thermal_test_driver);
+ }
+
+ return ret;
+}
+
+static void __exit thermal_test_exit(void)
+{
+ pr_err("in thermal_test_exit\n");
+ platform_device_unregister(pdev);
+ platform_driver_unregister(&thermal_test_driver);
+}
+
+module_init(thermal_test_init);
+module_exit(thermal_test_exit);
+
+MODULE_AUTHOR("Durgadoss R <[email protected]>");
+MODULE_DESCRIPTION("A dummy driver to test Thermal Framework");
+MODULE_LICENSE("GPL");
--
1.7.9.5

2013-10-01 13:10:59

by R, Durgadoss

Subject: [PATCHv4 5/9] Thermal: Add trip point sysfs nodes for sensor

This patch adds a trip point related sysfs nodes
for each sensor under a zone in /sys/class/thermal/zoneX/.
The nodes will be named, sensorX_trip_activeY,
sensorX_trip_passiveY, sensorX_trip_hot, sensorX_trip_critical
for active, passive, hot and critical trip points
respectively for sensorX.

Signed-off-by: Durgadoss R <[email protected]>
---
drivers/thermal/thermal_core.c | 344 +++++++++++++++++++++++++++++++++++++++-
include/linux/thermal.h | 40 ++++-
2 files changed, 379 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 3c4ef62..d6e29f6 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -494,6 +494,60 @@ static void thermal_zone_device_check(struct work_struct *work)
thermal_zone_device_update(tz);
}

+static int get_sensor_indx_by_kobj(struct thermal_zone *tz, const char *name)
+{
+ int i, indx = -EINVAL;
+
+ /* Protect against tz->sensors[i] being unregistered */
+ mutex_lock(&sensor_list_lock);
+
+ for (i = 0; i < tz->sensor_indx; i++) {
+ if (!strnicmp(name, kobject_name(&tz->sensors[i]->device.kobj),
+ THERMAL_NAME_LENGTH)) {
+ indx = i;
+ break;
+ }
+ }
+
+ mutex_unlock(&sensor_list_lock);
+ return indx;
+}
+
+static void __remove_trip_attr(struct thermal_zone *tz, int indx)
+{
+ int i;
+ struct thermal_trip_attr *attr = tz->trip_attr[indx];
+ struct thermal_trip_point *trip = tz->sensor_trip[indx];
+
+ if (!attr || !trip)
+ return;
+
+ if (trip->crit != THERMAL_TRIPS_NONE)
+ device_remove_file(&tz->device, &attr->crit_attr.attr);
+
+ if (trip->hot != THERMAL_TRIPS_NONE)
+ device_remove_file(&tz->device, &attr->hot_attr.attr);
+
+ if (trip->num_passive_trips > 0) {
+ for (i = 0; i < trip->num_passive_trips; i++) {
+ device_remove_file(&tz->device,
+ &attr->passive_attrs[i].attr);
+ }
+ kfree(attr->passive_attrs);
+ }
+
+ if (trip->num_active_trips > 0) {
+ for (i = 0; i < trip->num_active_trips; i++) {
+ device_remove_file(&tz->device,
+ &attr->active_attrs[i].attr);
+ }
+ kfree(attr->active_attrs);
+ }
+
+ kfree(tz->trip_attr[indx]);
+ tz->trip_attr[indx] = NULL;
+}
+
static void remove_sensor_from_zone(struct thermal_zone *tz,
struct thermal_sensor *ts)
{
@@ -503,13 +557,19 @@ static void remove_sensor_from_zone(struct thermal_zone *tz,
if (indx < 0)
return;

- sysfs_remove_link(&tz->device.kobj, kobject_name(&ts->device.kobj));
-
mutex_lock(&tz->lock);

+ /* Remove trip point attributes associated with this sensor */
+ __remove_trip_attr(tz, indx);
+
+ 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++)
+ for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++) {
tz->sensors[j] = tz->sensors[j + 1];
+ tz->sensor_trip[j] = tz->sensor_trip[j + 1];
+ tz->trip_attr[j] = tz->trip_attr[j + 1];
+ }

tz->sensor_indx--;
mutex_unlock(&tz->lock);
@@ -952,6 +1012,111 @@ emul_temp_store(struct device *dev, struct device_attribute *attr,
static DEVICE_ATTR(emul_temp, S_IWUSR, NULL, emul_temp_store);
#endif/*CONFIG_THERMAL_EMULATION*/

+static ssize_t
+active_trip_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ int i, j, val;
+ char kobj_name[THERMAL_NAME_LENGTH];
+ struct thermal_zone *tz = to_zone(dev);
+
+ if (!sscanf(attr->attr.name, "sensor%d_trip_active%d", &i, &j))
+ return -EINVAL;
+
+ snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", i);
+
+ mutex_lock(&tz->lock);
+
+ i = get_sensor_indx_by_kobj(tz, kobj_name);
+ if (i < 0) {
+ mutex_unlock(&tz->lock);
+ return i;
+ }
+
+ val = tz->sensor_trip[i]->active_trips[j];
+ mutex_unlock(&tz->lock);
+
+ return sprintf(buf, "%d\n", val);
+}
+
+static ssize_t
+passive_trip_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ int i, j, val;
+ char kobj_name[THERMAL_NAME_LENGTH];
+ struct thermal_zone *tz = to_zone(dev);
+
+ if (!sscanf(attr->attr.name, "sensor%d_trip_passive%d", &i, &j))
+ return -EINVAL;
+
+ snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", i);
+
+ mutex_lock(&tz->lock);
+
+ i = get_sensor_indx_by_kobj(tz, kobj_name);
+ if (i < 0) {
+ mutex_unlock(&tz->lock);
+ return i;
+ }
+
+ val = tz->sensor_trip[i]->passive_trips[j];
+ mutex_unlock(&tz->lock);
+
+ return sprintf(buf, "%d\n", val);
+}
+
+static ssize_t
+hot_trip_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ int indx, val;
+ char kobj_name[THERMAL_NAME_LENGTH];
+ struct thermal_zone *tz = to_zone(dev);
+
+ if (!sscanf(attr->attr.name, "sensor%d_trip_hot", &indx))
+ return -EINVAL;
+
+ snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", indx);
+
+ mutex_lock(&tz->lock);
+
+ indx = get_sensor_indx_by_kobj(tz, kobj_name);
+ if (indx < 0) {
+ mutex_unlock(&tz->lock);
+ return indx;
+ }
+
+ val = tz->sensor_trip[indx]->hot;
+ mutex_unlock(&tz->lock);
+
+ return sprintf(buf, "%d\n", val);
+}
+
+static ssize_t
+critical_trip_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int indx, val;
+ char kobj_name[THERMAL_NAME_LENGTH];
+ struct thermal_zone *tz = to_zone(dev);
+
+ if (!sscanf(attr->attr.name, "sensor%d_trip_critical", &indx))
+ return -EINVAL;
+
+ snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", indx);
+
+ mutex_lock(&tz->lock);
+
+ indx = get_sensor_indx_by_kobj(tz, kobj_name);
+ if (indx < 0) {
+ mutex_unlock(&tz->lock);
+ return indx;
+ }
+
+ val = tz->sensor_trip[indx]->crit;
+ mutex_unlock(&tz->lock);
+
+ return sprintf(buf, "%d\n", val);
+}
+
static DEVICE_ATTR(type, 0444, type_show, NULL);
static DEVICE_ATTR(temp, 0444, temp_show, NULL);
static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
@@ -962,7 +1127,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);
+/* Thermal zone attributes */
+static DEVICE_ATTR(zone_name, S_IRUGO, zone_name_show, NULL);

/* sys I/F for cooling device */
#define to_cooling_device(_dev) \
@@ -1841,6 +2007,43 @@ static int enable_sensor_thresholds(struct thermal_sensor *ts, int count)
return 0;
}

+static int create_single_trip_attr(struct thermal_zone *tz,
+ struct thermal_attr *attr,
+ const char *attr_name,
+ ssize_t (*rd_ptr)(struct device *dev,
+ struct device_attribute *devattr, char *buf))
+{
+ snprintf(attr->name, THERMAL_NAME_LENGTH, attr_name);
+ sysfs_attr_init(&attr->attr.attr);
+ attr->attr.attr.name = attr->name;
+ attr->attr.attr.mode = S_IRUGO;
+ attr->attr.show = rd_ptr;
+ return device_create_file(&tz->device, &attr->attr);
+}
+
+static int create_multi_trip_attrs(struct thermal_zone *tz, int size,
+ int indx, struct thermal_attr *attrs,
+ const char *attr_name,
+ ssize_t (*rd_ptr)(struct device *dev,
+ struct device_attribute *devattr, char *buf))
+{
+ char name[THERMAL_NAME_LENGTH];
+ int i, ret;
+
+ for (i = 0; i < size; i++) {
+ snprintf(name, THERMAL_NAME_LENGTH, attr_name, indx, i);
+ ret = create_single_trip_attr(tz, &attrs[i], name, rd_ptr);
+ if (ret)
+ goto exit;
+ }
+ return 0;
+
+exit:
+ while (--i >= 0)
+ device_remove_file(&tz->device, &attrs[i].attr);
+ return ret;
+}
+
/**
* create_thermal_zone - create sysfs nodes for thermal zone
* @name: Name of the thermla zone
@@ -2139,6 +2342,139 @@ exit_zone:
EXPORT_SYMBOL(add_cdev_to_zone);

/**
+ * add_sensor_trip_info - Add trip point information for @ts in @tz
+ * @tz: Thermal zone reference
+ * @ts: Thermal sensor reference
+ * @trip: Trip point structure reference
+ *
+ * Returns 0 on success, otherwise
+ * -EINVAL for invalid paramenters
+ * -EINVAL if @ts is not part of 'this' thermal zone @tz
+ * -ENOMEM on kzalloc failures
+ */
+int add_sensor_trip_info(struct thermal_zone *tz, struct thermal_sensor *ts,
+ struct thermal_trip_point *trip)
+{
+ char name[THERMAL_NAME_LENGTH];
+ int i, indx, kobj_indx, ret, size;
+ struct thermal_trip_attr *attrs;
+
+ if (!tz || !ts || !trip)
+ return -EINVAL;
+
+ if (!sscanf(kobject_name(&ts->device.kobj), "sensor%d", &kobj_indx))
+ return -EINVAL;
+
+ mutex_lock(&zone_list_lock);
+
+ indx = GET_INDEX(tz, ts, sensor);
+ if (indx < 0) {
+ ret = -EINVAL;
+ goto exit;
+ }
+
+ /* Protect against 'ts' being unregistered */
+ mutex_lock(&sensor_list_lock);
+
+ /* Protect tz->trip_attr[] and tz->sensor_trip[] */
+ mutex_lock(&tz->lock);
+
+ tz->trip_attr[indx] = kzalloc(sizeof(struct thermal_trip_attr),
+ GFP_KERNEL);
+ if (!tz->trip_attr[indx]) {
+ ret = -ENOMEM;
+ goto exit_lock;
+ }
+
+ attrs = tz->trip_attr[indx];
+
+ /* Create Critical trip point attribute */
+ if (trip->crit != THERMAL_TRIPS_NONE) {
+ snprintf(name, THERMAL_NAME_LENGTH,
+ "sensor%d_trip_critical", kobj_indx);
+ ret = create_single_trip_attr(tz, &attrs->crit_attr,
+ name, critical_trip_show);
+ if (ret)
+ goto exit_trip;
+ }
+
+ /* Create Hot trip point attribute */
+ if (trip->hot != THERMAL_TRIPS_NONE) {
+ snprintf(name, THERMAL_NAME_LENGTH,
+ "sensor%d_trip_hot", kobj_indx);
+ ret = create_single_trip_attr(tz, &attrs->hot_attr,
+ name, hot_trip_show);
+ if (ret)
+ goto exit_crit_trip;
+ }
+
+ /* Create Passive trip point attributes */
+ if (trip->num_passive_trips > 0) {
+ size = sizeof(struct thermal_attr) * trip->num_passive_trips;
+ attrs->passive_attrs = kzalloc(size, GFP_KERNEL);
+ if (!attrs->passive_attrs) {
+ ret = -ENOMEM;
+ goto exit_hot_trip;
+ }
+
+ ret = create_multi_trip_attrs(tz, trip->num_passive_trips,
+ kobj_indx, attrs->passive_attrs,
+ "sensor%d_trip_passive%d",
+ passive_trip_show);
+ if (ret)
+ goto exit_hot_trip;
+ }
+
+ /* Create Active trip point attributes */
+ if (trip->num_active_trips > 0) {
+ size = sizeof(struct thermal_attr) * trip->num_active_trips;
+ attrs->active_attrs = kzalloc(size, GFP_KERNEL);
+ if (!attrs->active_attrs) {
+ ret = -ENOMEM;
+ goto exit_passive_trips;
+ }
+
+ ret = create_multi_trip_attrs(tz, trip->num_active_trips,
+ kobj_indx, attrs->active_attrs,
+ "sensor%d_trip_active%d",
+ active_trip_show);
+ if (ret)
+ goto exit_passive_trips;
+ }
+
+ tz->sensor_trip[indx] = trip;
+
+ mutex_unlock(&tz->lock);
+ mutex_unlock(&sensor_list_lock);
+ mutex_unlock(&zone_list_lock);
+
+ return 0;
+
+exit_passive_trips:
+ kfree(attrs->active_attrs);
+ i = trip->num_passive_trips;
+ while (--i >= 0)
+ device_remove_file(&tz->device, &attrs->passive_attrs[i].attr);
+exit_hot_trip:
+ kfree(attrs->passive_attrs);
+ if (trip->hot != THERMAL_TRIPS_NONE)
+ device_remove_file(&tz->device, &attrs->hot_attr.attr);
+exit_crit_trip:
+ if (trip->crit != THERMAL_TRIPS_NONE)
+ device_remove_file(&tz->device, &attrs->crit_attr.attr);
+exit_trip:
+ kfree(tz->trip_attr[indx]);
+ tz->trip_attr[indx] = NULL;
+exit_lock:
+ mutex_unlock(&tz->lock);
+ mutex_unlock(&sensor_list_lock);
+exit:
+ mutex_unlock(&zone_list_lock);
+ return ret;
+}
+EXPORT_SYMBOL(add_sensor_trip_info);
+
+/**
* thermal_sensor_register - register a new thermal sensor
* @name: name of the thermal sensor
* @count: Number of thresholds supported by hardware
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index da7520c..f8de86d 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -31,7 +31,7 @@

#define THERMAL_TRIPS_NONE -1
#define THERMAL_MAX_TRIPS 12
-#define THERMAL_NAME_LENGTH 20
+#define THERMAL_NAME_LENGTH 25

/* invalid cooling state */
#define THERMAL_CSTATE_INVALID -1UL
@@ -170,6 +170,37 @@ struct thermal_attr {
char name[THERMAL_NAME_LENGTH];
};

+/*
+ * This structure defines the trip points for a sensor.
+ * The actual values for these trip points come from
+ * platform characterization. The thermal governors
+ * (either kernel or user space) may take appropriate
+ * actions when the sensors reach these trip points.
+ * See Documentation/thermal/sysfs-api2.txt for more details.
+ *
+ * As of now, For a particular sensor, we support:
+ * a) 1 hot trip point
+ * b) 1 critical trip point
+ * c) 'n' passive trip points
+ * d) 'm' active trip points
+ */
+struct thermal_trip_point {
+ int hot;
+ int crit;
+ int num_passive_trips;
+ int *passive_trips;
+ int num_active_trips;
+ int *active_trips;
+ int active_trip_mask;
+};
+
+struct thermal_trip_attr {
+ struct thermal_attr hot_attr;
+ struct thermal_attr crit_attr;
+ struct thermal_attr *active_attrs;
+ struct thermal_attr *passive_attrs;
+};
+
struct thermal_sensor {
char name[THERMAL_NAME_LENGTH];
int id;
@@ -229,6 +260,10 @@ struct thermal_zone {
/* cdev level information */
int cdev_indx; /* index into 'cdevs' array */
struct thermal_cooling_device *cdevs[MAX_CDEVS_PER_ZONE];
+
+ /* Thermal sensors trip information */
+ struct thermal_trip_point *sensor_trip[MAX_SENSORS_PER_ZONE];
+ struct thermal_trip_attr *trip_attr[MAX_SENSORS_PER_ZONE];
};

/* Structure that holds thermal governor information */
@@ -309,6 +344,9 @@ struct thermal_sensor *get_sensor_by_name(const char *);
int add_cdev_to_zone(struct thermal_zone *, struct thermal_cooling_device *);
struct thermal_cooling_device *get_cdev_by_name(const char *);

+int add_sensor_trip_info(struct thermal_zone *, struct thermal_sensor *,
+ struct thermal_trip_point *);
+
#ifdef CONFIG_NET
extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,
enum events event);
--
1.7.9.5

2013-10-01 13:11:33

by R, Durgadoss

Subject: [PATCHv4 8/9] Thermal: Add ABI Documentation for sysfs interfaces

This patch adds Documentation for ABI's introduced
for thermal subsystem (under /sys/class/thermal/).

Signed-off-by: Durgadoss R <[email protected]>
---
Documentation/ABI/testing/sysfs-class-thermal | 137 +++++++++++++++++++++++++
1 file changed, 137 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-class-thermal

diff --git a/Documentation/ABI/testing/sysfs-class-thermal b/Documentation/ABI/testing/sysfs-class-thermal
new file mode 100644
index 0000000..36e23da
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-thermal
@@ -0,0 +1,137 @@
+What: /sys/class/thermal/sensorX/temp
+Date: October 2013
+KernelVersion: 3.12
+Contact: Linux PM Mailing list <[email protected]>
+Description:
+ Exposes 'temperature' of a thermal sensor in mC
+Users: Kernel/User space thermal governors
+
+What: /sys/class/thermal/sensorX/name
+Date: October 2013
+KernelVersion: 3.12
+Contact: Linux PM Mailing list <[email protected]>
+Description:
+ Name of the thermal sensor
+Users: Kernel/User space thermal governors
+
+What: /sys/class/thermal/sensorX/thresholdY
+Date: October 2013
+KernelVersion: 3.12
+Contact: Linux PM Mailing list <[email protected]>
+Description:
+ Programmable threshold (in terms of mC). On reaching
+ this, the thermal governors may take action to control
+ temperature.
+Users: Kernel/User space thermal governors
+
+What: /sys/class/thermal/zoneX/name
+Date: October 2013
+KernelVersion: 3.12
+Contact: Linux PM Mailing list <[email protected]>
+Description:
+ Name of the thermal zone.
+Users: Kernel/User space thermal governors
+
+What: /sys/class/thermal/zoneX/sensorY
+Date: October 2013
+KernelVersion: 3.12
+Contact: Linux PM Mailing list <[email protected]>
+Description:
+ Symlink to a sensor associated with this zone.
+Users: User space thermal governors or applications
+
+What: /sys/class/thermal/zoneX/cooling_deviceY
+Date: October 2013
+KernelVersion: 3.12
+Contact: Linux PM Mailing list <[email protected]>
+Description:
+ Symlink to a cooling device associated with this zone.
+Users: User space thermal governors or applications
+
+What: /sys/class/thermal/zoneX/sensorY_trip_activeM
+Date: October 2013
+KernelVersion: 3.12
+Contact: Linux PM Mailing list <[email protected]>
+Description:
+ Active Trip point temperature in mC for sensorY in
+ thermal zoneX.
+Users: Kernel/User space thermal governors
+
+What: /sys/class/thermal/zoneX/sensorY_trip_passiveN
+Date: October 2013
+KernelVersion: 3.12
+Contact: Linux PM Mailing list <[email protected]>
+Description:
+ Passive Trip point temperature in mC for sensorY in
+ thermal zoneX.
+Users: Kernel/User space thermal governors
+
+What: /sys/class/thermal/zoneX/sensorY_trip_hot
+Date: October 2013
+KernelVersion: 3.12
+Contact: Linux PM Mailing list <[email protected]>
+Description:
+ Hot trip point for 'sensorY' in 'zoneX' (in mC).
+Users: Kernel/User space thermal governors
+
+What: /sys/class/thermal/zoneX/sensorY_trip_critical
+Date: October 2013
+KernelVersion: 3.12
+Contact: Linux PM Mailing list <[email protected]>
+Description:
+ Critical trip point for 'sensorY' in 'zoneX' (in mC).
+Users: Kernel/User space thermal governors
+
+What: /sys/class/thermal/zoneX/mapY_trip_type
+Date: October 2013
+KernelVersion: 3.12
+Contact: Linux PM Mailing list <[email protected]>
+Description:
+ Mapping information between a sensor and a cooling device
+ in 'zoneX'. This interface provides the trip_type for a
+ particular map(Y).
+Users: Kernel/User space thermal governors
+
+What: /sys/class/thermal/zoneX/mapY_sensor_name
+Date: October 2013
+KernelVersion: 3.12
+Contact: Linux PM Mailing list <[email protected]>
+Description:
+ Mapping information between a sensor and a cooling device
+ in 'zoneX'. This interface provides the name of the sensor
+ used in this map(Y).
+Users: Kernel/User space thermal governors
+
+What: /sys/class/thermal/zoneX/mapY_cdev_name
+Date: October 2013
+KernelVersion: 3.12
+Contact: Linux PM Mailing list <[email protected]>
+Description:
+ Mapping information between a sensor and a cooling device
+ in 'zoneX'. This interface provides the name of the cooling
+ device used in this map(Y).
+Users: Kernel/User space thermal governors
+
+What: /sys/class/thermal/zoneX/mapY_trip_mask
+Date: October 2013
+KernelVersion: 3.12
+Contact: Linux PM Mailing list <[email protected]>
+Description:
+ Mapping information between a sensor and a cooling device
+ in 'zoneX'. This interface provides the trip point mask,
+ which defines whether or not to throttle a particular
+ cooling device. See Documentation/thermal/sysfs-api2.txt
+ for more information on this interface.
+Users: Kernel/User space thermal governors
+
+What: /sys/class/thermal/zoneX/mapY_weightN
+Date: October 2013
+KernelVersion: 3.12
+Contact: Linux PM Mailing list <[email protected]>
+Description:
+ Mapping information between a sensor and a cooling device
+ in 'zoneX'. This interface provides the weights that can
+ be used to throttle a cooling device, on thermal violations.
+ See Documentation/thermal/sysfs-api2.txt for more details on
+ this interface.
+Users: Kernel/User space thermal governors
--
1.7.9.5

2013-10-01 13:11:48

by R, Durgadoss

Subject: [PATCHv4 6/9] Thermal: Create Thermal map sysfs attributes for a zone

This patch creates a thermal map sysfs node under
/sys/class/thermal/zoneX/. The thermal map
shows the binding relationship between a sensor
and a cooling device within a particular zone.
This contains entries named mapY_trip_type,
mapY_sensor_name, mapY_cdev_name, mapY_trip_mask,
mapY_weightX.

Signed-off-by: Durgadoss R <[email protected]>
---
drivers/thermal/thermal_core.c | 252 +++++++++++++++++++++++++++++++++++++++-
include/linux/thermal.h | 26 +++++
2 files changed, 277 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index d6e29f6..e1289a4 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -548,6 +548,24 @@ static void __remove_trip_attr(struct thermal_zone *tz, int indx)
tz->trip_attr[indx] = NULL;
}

+static void __remove_map_entry(struct thermal_zone *tz, int indx)
+{
+ int i;
+ struct thermal_map_attr *attr;
+
+ attr = tz->map_attr[indx];
+
+ for (i = 0; i < NUM_MAP_ATTRS; i++)
+ device_remove_file(&tz->device, &attr->attrs[i].attr);
+
+ for (i = 0; i < tz->map[indx]->num_weights; i++)
+ device_remove_file(&tz->device, &attr->weights_attr[i].attr);
+
+ kfree(tz->map_attr[indx]);
+ tz->map_attr[indx] = NULL;
+ tz->map[indx] = NULL;
+}
+
static void remove_sensor_from_zone(struct thermal_zone *tz,
struct thermal_sensor *ts)
{
@@ -572,6 +590,14 @@ static void remove_sensor_from_zone(struct thermal_zone *tz,
}

tz->sensor_indx--;
+
+ /* Remove all mappings associated with this sensor */
+ for (j = 0; j < MAX_MAPS_PER_ZONE; j++) {
+ if (tz->map[j] && !strnicmp(ts->name, tz->map[j]->sensor_name,
+ THERMAL_NAME_LENGTH)) {
+ __remove_map_entry(tz, j);
+ }
+ }
mutex_unlock(&tz->lock);
}

@@ -593,6 +619,14 @@ static void remove_cdev_from_zone(struct thermal_zone *tz,
tz->cdevs[j] = tz->cdevs[j + 1];

tz->cdev_indx--;
+
+ /* Remove all mappings associated with this sensor */
+ for (j = 0; j < MAX_MAPS_PER_ZONE; j++) {
+ if (tz->map[j] && !strnicmp(cdev->type, tz->map[j]->cdev_name,
+ THERMAL_NAME_LENGTH)) {
+ __remove_map_entry(tz, j);
+ }
+ }
mutex_unlock(&tz->lock);
}

@@ -1117,6 +1151,126 @@ critical_trip_show(struct device *dev,
return sprintf(buf, "%d\n", val);
}

+static ssize_t
+map_ttype_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ int indx, ret = -EINVAL;
+ struct thermal_zone *tz = to_zone(dev);
+
+ if (!sscanf(attr->attr.name, "map%d_trip_type", &indx))
+ return -EINVAL;
+
+ if (indx < 0 || indx >= MAX_MAPS_PER_ZONE)
+ return -EINVAL;
+
+ mutex_lock(&tz->lock);
+
+ if (!tz->map[indx])
+ goto exit;
+
+ ret = sprintf(buf, "%s\n",
+ tz->map[indx]->trip_type == THERMAL_TRIP_ACTIVE ?
+ "active" : "passive");
+exit:
+ mutex_unlock(&tz->lock);
+ return ret;
+}
+
+static ssize_t map_ts_name_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int indx, ret = -EINVAL;
+ struct thermal_zone *tz = to_zone(dev);
+
+ if (!sscanf(attr->attr.name, "map%d_sensor_name", &indx))
+ return -EINVAL;
+
+ if (indx < 0 || indx >= MAX_MAPS_PER_ZONE)
+ return -EINVAL;
+
+ mutex_lock(&tz->lock);
+
+ if (!tz->map[indx])
+ goto exit;
+
+ ret = sprintf(buf, "%s\n", tz->map[indx]->sensor_name);
+exit:
+ mutex_unlock(&tz->lock);
+ return ret;
+}
+
+static ssize_t map_cdev_name_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int indx, ret = -EINVAL;
+ struct thermal_zone *tz = to_zone(dev);
+
+ if (!sscanf(attr->attr.name, "map%d_cdev_name", &indx))
+ return -EINVAL;
+
+ if (indx < 0 || indx >= MAX_MAPS_PER_ZONE)
+ return -EINVAL;
+
+ mutex_lock(&tz->lock);
+
+ if (!tz->map[indx])
+ goto exit;
+
+ ret = sprintf(buf, "%s\n", tz->map[indx]->cdev_name);
+exit:
+ mutex_unlock(&tz->lock);
+ return ret;
+}
+
+static ssize_t map_trip_mask_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int indx, ret = -EINVAL;
+ struct thermal_zone *tz = to_zone(dev);
+
+ if (!sscanf(attr->attr.name, "map%d_trip_mask", &indx))
+ return -EINVAL;
+
+ if (indx < 0 || indx >= MAX_MAPS_PER_ZONE)
+ return -EINVAL;
+
+ mutex_lock(&tz->lock);
+
+ if (!tz->map[indx])
+ goto exit;
+
+ ret = sprintf(buf, "0x%x\n", tz->map[indx]->trip_mask);
+exit:
+ mutex_unlock(&tz->lock);
+ return ret;
+}
+
+static ssize_t map_weights_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, j, ret = -EINVAL;
+ struct thermal_zone *tz = to_zone(dev);
+
+ if (!sscanf(attr->attr.name, "map%d_weight%d", &i, &j))
+ return -EINVAL;
+
+ if (i < 0 || i >= MAX_MAPS_PER_ZONE)
+ return -EINVAL;
+
+ mutex_lock(&tz->lock);
+
+ if (!tz->map[i])
+ goto exit;
+
+ if (j < 0 || j >= tz->map[i]->num_weights)
+ goto exit;
+
+ ret = sprintf(buf, "%d\n", tz->map[i]->weights[j]);
+exit:
+ mutex_unlock(&tz->lock);
+ return ret;
+}
+
static DEVICE_ATTR(type, 0444, type_show, NULL);
static DEVICE_ATTR(temp, 0444, temp_show, NULL);
static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
@@ -2044,6 +2198,52 @@ exit:
return ret;
}

+static int create_map_attrs(struct thermal_zone *tz, int indx, int num_weights)
+{
+ int i, ret, size;
+ char name[THERMAL_NAME_LENGTH];
+ struct thermal_map_attr *attr = tz->map_attr[indx];
+
+ static const char *const attr_names[NUM_MAP_ATTRS] = {
+ "map%d_trip_type", "map%d_sensor_name",
+ "map%d_cdev_name", "map%d_trip_mask"};
+
+ static ssize_t (*const rd_ptr[NUM_MAP_ATTRS]) (struct device *dev,
+ struct device_attribute *devattr, char *buf) = {
+ map_ttype_show, map_ts_name_show,
+ map_cdev_name_show, map_trip_mask_show};
+
+ for (i = 0; i < NUM_MAP_ATTRS; i++) {
+ snprintf(name, THERMAL_NAME_LENGTH, attr_names[i], indx);
+ ret = create_single_trip_attr(tz, &attr->attrs[i],
+ name, rd_ptr[i]);
+ if (ret)
+ goto exit_free;
+ }
+
+ size = sizeof(struct thermal_attr) * num_weights;
+ attr->weights_attr = kzalloc(size, GFP_KERNEL);
+ if (!attr->weights_attr) {
+ ret = -ENOMEM;
+ goto exit_free;
+ }
+
+ ret = create_multi_trip_attrs(tz, num_weights, indx,
+ attr->weights_attr,
+ "map%d_weight%d",
+ map_weights_show);
+ if (ret) {
+ kfree(attr->weights_attr);
+ goto exit_free;
+ }
+
+ return 0;
+exit_free:
+ while (--i >= 0)
+ device_remove_file(&tz->device, &attr->attrs[i].attr);
+ return ret;
+}
+
/**
* create_thermal_zone - create sysfs nodes for thermal zone
* @name: Name of the thermla zone
@@ -2135,7 +2335,6 @@ void remove_thermal_zone(struct thermal_zone *tz)
remove_cdev_from_zone(tz, cdev);

device_remove_file(&tz->device, &dev_attr_zone_name);
-
mutex_destroy(&tz->lock);
release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
idr_destroy(&tz->idr);
@@ -2475,6 +2674,57 @@ exit:
EXPORT_SYMBOL(add_sensor_trip_info);

/**
+ * add_map_entry - Add Thermal Map information for @tz
+ * @tz: Thermal zone reference
+ * @map: Thermal map reference
+ *
+ * Returns 0 on success, otherwise
+ * -EINVAL for invalid paramenters
+ * -EINVAL for array out of bounds
+ * -ENOMEM on kzalloc failures
+ */
+int add_map_entry(struct thermal_zone *tz, struct thermal_map *map)
+{
+ int ret, indx;
+
+ if (!tz || !map)
+ return -EINVAL;
+
+ mutex_lock(&zone_list_lock);
+
+ /* Find the first unused index which is NULL */
+ for (indx = 0; indx < MAX_MAPS_PER_ZONE; indx++) {
+ if (tz->map[indx] == NULL)
+ break;
+ }
+
+ if (indx >= MAX_MAPS_PER_ZONE) {
+ ret = -EINVAL;
+ goto exit;
+ }
+
+ tz->map_attr[indx] = kzalloc(sizeof(struct thermal_map_attr),
+ GFP_KERNEL);
+ if (!tz->map_attr[indx]) {
+ ret = -ENOMEM;
+ goto exit;
+ }
+
+ ret = create_map_attrs(tz, indx, map->num_weights);
+ if (ret) {
+ kfree(tz->map_attr[indx]);
+ tz->map_attr[indx] = NULL;
+ goto exit;
+ }
+
+ tz->map[indx] = map;
+exit:
+ mutex_unlock(&zone_list_lock);
+ return ret;
+}
+EXPORT_SYMBOL(add_map_entry);
+
+/**
* thermal_sensor_register - register a new thermal sensor
* @name: name of the thermal sensor
* @count: Number of thresholds supported by hardware
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index f8de86d..724c68b 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -62,6 +62,11 @@

#define MAX_CDEVS_PER_ZONE 5

+#define NUM_MAP_ATTRS 4
+
+/* If we map each sensor with every possible cdev for a zone */
+#define MAX_MAPS_PER_ZONE (MAX_SENSORS_PER_ZONE * MAX_CDEVS_PER_ZONE)
+
struct thermal_sensor;
struct thermal_zone_device;
struct thermal_cooling_device;
@@ -170,6 +175,21 @@ struct thermal_attr {
char name[THERMAL_NAME_LENGTH];
};

+struct thermal_map {
+ enum thermal_trip_type trip_type;
+ char cdev_name[THERMAL_NAME_LENGTH];
+ char sensor_name[THERMAL_NAME_LENGTH];
+
+ int trip_mask;
+ int num_weights;
+ int *weights;
+};
+
+struct thermal_map_attr {
+ struct thermal_attr attrs[NUM_MAP_ATTRS];
+ struct thermal_attr *weights_attr;
+};
+
/*
* This structure defines the trip points for a sensor.
* The actual values for these trip points come from
@@ -264,6 +284,10 @@ struct thermal_zone {
/* Thermal sensors trip information */
struct thermal_trip_point *sensor_trip[MAX_SENSORS_PER_ZONE];
struct thermal_trip_attr *trip_attr[MAX_SENSORS_PER_ZONE];
+
+ /* Thermal map information */
+ struct thermal_map *map[MAX_MAPS_PER_ZONE];
+ struct thermal_map_attr *map_attr[MAX_MAPS_PER_ZONE];
};

/* Structure that holds thermal governor information */
@@ -347,6 +371,8 @@ struct thermal_cooling_device *get_cdev_by_name(const char *);
int add_sensor_trip_info(struct thermal_zone *, struct thermal_sensor *,
struct thermal_trip_point *);

+int add_map_entry(struct thermal_zone *, struct thermal_map *);
+
#ifdef CONFIG_NET
extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,
enum events event);
--
1.7.9.5

2013-10-01 13:11:47

by R, Durgadoss

Subject: [PATCHv4 7/9] Thermal: Add Documentation to new APIs

This patch adds Documentation for the new APIs
introduced in this patch set. The documentation
also has a model sysfs structure for reference.

Signed-off-by: Durgadoss R <[email protected]>
---
Documentation/thermal/sysfs-api2.txt | 248 ++++++++++++++++++++++++++++++++++
1 file changed, 248 insertions(+)
create mode 100644 Documentation/thermal/sysfs-api2.txt

diff --git a/Documentation/thermal/sysfs-api2.txt b/Documentation/thermal/sysfs-api2.txt
new file mode 100644
index 0000000..c456dad
--- /dev/null
+++ b/Documentation/thermal/sysfs-api2.txt
@@ -0,0 +1,248 @@
+Thermal Framework
+-----------------
+
+Written by Durgadoss R <[email protected]>
+Copyright (c) 2013 Intel Corporation
+
+Created on: 25 September 2013
+
+0. Introduction
+---------------
+The Linux thermal framework provides a set of interfaces for thermal
+sensors and thermal cooling devices (fan, processor...) to register
+with the thermal management solution and to be a part of it.
+
+This document focuses on how to enable new thermal sensors and cooling
+devices to participate in thermal management. This solution is intended
+to be 'light-weight' and platform/architecture independent. Any thermal
+sensor/cooling device should be able to use the infrastructure easily.
+
+The goal of thermal framework is to expose the thermal sensor/zone and
+cooling device attributes in a consistent way. This will help the
+thermal governors to make use of the information to manage platform
+thermals efficiently.
+
+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 (e.g 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. We see a lot of chip drivers,
+starting to use thermal framework; we should keep it really
+light-weight for them to do so but at the same time provide all
+the necessary features to participate in platform thermal management.
+
+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.
+
+1. Terminology
+--------------
+This section describes the terminology used in the rest of this
+document as well as the thermal framework code.
+
+thermal_sensor: Hardware that can report temperature of a particular
+ spot in the platform, where it is placed. The temperature
+ reported by the sensor is the 'real' temperature reported
+ by the hardware.
+thermal_zone: A virtual area on the device, that gets heated up. It may
+ have one or more thermal sensors attached to it.
+cooling_device: Any component that can help in reducing the temperature of
+ a 'hot spot' either by reducing its performance (passive
+ cooling) or by other means(Active cooling E.g. Fan)
+
+trip_points: Various temperature levels for each sensor. As of now, we
+ have four levels namely active, passive, hot and critical.
+ Hot and critical trip point support only one value whereas
+ active and passive can have any number of values. These
+ temperature values can come from platform data, and are
+ exposed through sysfs in a consistent manner. Stand-alone
+ thermal sensor drivers are not expected to know these values.
+ These values are RO.
+thresholds: These are programmable temperature limits, on reaching which
+ the thermal sensor generates an interrupt. The framework is
+ notified about this interrupt to take appropriate action.
+ There can be as many number of thresholds as that of the
+ hardware supports. These values are RW.
+
+thermal_map: This provides the mapping (aka binding) information between
+ various sensors and cooling devices in a particular zone.
+ Typically, this also comes from platform data; Stand-alone
+ sensor drivers or cooling device drivers are not expected
+ to know these mapping information.
+
+2. Thermal framework APIs
+-------------------------
+2.1: For Thermal Sensors
+2.1.1 thermal_sensor_register:
+ This function creates a new sensor directory under /sys/class/thermal/
+ as sensor[0-*]. This API is expected to be called by thermal sensor
+ drivers. These drivers may or may not be in thermal subsystem. This
+ function returns a thermal_sensor structure on success and appropriate
+ error on failure.
+
+ name: Name of the sensor
+ count: Number of programmable thresholds associated with this sensor
+ devdata: Device private data
+ ops: Thermal sensor callbacks
+ .get_temp: obtain the current temperature of the sensor
+ .get_trend: obtain the trend of the sensor
+ .get_threshold: get a particular threshold temperature
+ .set_threshold: set a particular threshold temperature
+ .get_hyst: get hysteresis value associated with a threshold
+ .set_hyst: set hysteresis value associated with a threshold
+
+2.1.2 thermal_sensor_unregister:
+ This function deletes the sensor directory under /sys/class/thermal/
+ for the given sensor. Thermal sensor drivers may call this API
+ during the driver's 'exit' routine.
+
+ ts: Thermal sensor that has to be unregistered
+
+2.1.3 enable_sensor_thresholds:
+ This function creates 'threshold[0-*]' attributes under a particular
+ sensorX directory. These values are RW. This function is called by
+ the sensr driver only if the sensor supports interrupt mechanism.
+
+ ts: Thermal sensor for which thresholds have to be enabled
+ num_thresholds: Number of thresholds supported by the sensor
+
+2.2: For Cooling Devices
+2.2.1 thermal_cooling_device_register:
+ This function adds a new thermal cooling device (fan/processor/...)
+ to /sys/class/thermal/ folder as cooling_device[0-*]. This function
+ is expected to be called by cooling device drivers that may be
+ present in other subsystems also.
+
+ name: the cooling device name
+ devdata: device private data
+ ops: thermal cooling devices callbacks
+ .get_max_state: get the Maximum throttle state of the cooling device
+ .get_cur_state: get the Current throttle state of the cooling device
+ .set_cur_state: set the Current throttle state of the cooling device
+
+2.2.2 thermal_cooling_device_unregister:
+ This function deletes the given cdev entry form /sys/class/thermal;
+ and also cleans all the symlinks referred from various zones.
+
+ cdev: Cooling device to be unregistered
+
+2.3: For Thermal Zones
+2.3.1 create_thermal_zone:
+ This function adds a new 'zone' under /sys/class/thermal/
+ directory as zone[0-*]. This zone has at least one thermal
+ sensor and at most MAX_SENSORS_PER_ZONE number of sensors
+ attached to it. Similarly, this zone has at least one cdev
+ and at most MAX_CDEVS_PER_ZONE number of cdevs attached to it.
+ As of now, MAX_*_PER_ZONE values are hard-coded to 5. We can
+ make them configurable, through Kconfig option(during 'menuconfig').
+
+ name: Name of the thermal zone
+ devdata: Device private data
+
+2.3.2 add_sensor_to_zone
+ This function adds a 'sensorX' entry under /sys/class/thermal/
+ zoneY/ directory. This 'sensorX' is a symlink to the actual
+ sensor entry under /sys/class/thermal/. Correspondingly, the
+ method remove_sensor_from_zone deletes the symlink.
+
+ tz: thermal zone structure
+ ts: thermal sensor structure
+
+2.3.3 add_cdev_to_zone
+ This function adds a 'cdevX' entry under /sys/class/thermal/
+ zoneY/ directory. This 'cdevX' is a symlink to the actual
+ cdev entry under /sys/class/thermal/. Correspondingly, the
+ method remove_cdev_from_zone deletes the symlink.
+
+ tz: thermal zone structure
+ cdev: thermal cooling device structure
+
+2.4 For Thermal Trip
+2.4.1 add_sensor_trip_info
+ This function adds trip point information for the given sensor,
+ (under a given zone) under /sys/class/thermal/zoneX/.
+ This API creates sysfs attributes namely:
+ sensorX_trip_activeY, sensorX_trip_passiveY, sensorX_trip_hot,
+ sensorX_trip_critical. Each of these hold one trip point temperature
+ (in mC) values, as provided from platform data. As of now, we
+ support many Active and Passive trip points but only one hot
+ one critical trip point.
+
+ tz: thermal zone structure
+ ts: thermal sensor to which the trip points are attached
+ trip: trip point structure. Usually obtained from platform data
+
+2.5 For Thermal Map
+2.5.1 add_map_entry
+ This function adds a 'map[0-*]' sysfs attribute under
+ /sys/class/thermal/zoneX/mapY_*. Each map attribute helps
+ to describe the binding relationship between a sensor and
+ a cdev in the given zone. The map structure is typically
+ obtained as platform data. For example, through ACPI tables,
+ SFI tables, Device tree etc. The trip mask is a hex value;
+ if 'n' th bit (from LSB) is set, then for trip point 'n' this
+ cdev is throttled with the given weight[n].
+
+ tz: thermal zone to which a 'map' is being added
+ map: thermal_map structure
+
+3. Sysfs attributes structure
+-----------------------------
+Thermal sysfs attributes will be represented under /sys/class/thermal.
+
+3.1: For Thermal Sensors
+ /sys/class/thermal/sensor[0-*]:
+ |---type: Name of the thermal sensor
+ |---temp_input: Current temperature in mC
+ |---threshold[0-*]: Threshold temperature in mC
+ |---threshold[0-*]_hyst:Optional hysteresis value in mC
+
+3.2: For Thermal Cooling Devices
+ /sys/class/thermal/cooling_device[0-*]:
+ |---type: Type of the cooling device
+ |---max_state: Maximum throttle state of the cdev
+ |---cur_state: Current throttle state of the cdev
+
+3.3: For Thermal Zones
+ /sys/class/thermal/zone[0-*]:
+ |---name: Name of the thermal
+ |---sensorX: Symlink to ../sensorX
+ |---cdevY: Symlink to ../cdevY
+ |---sensorX_trip_activeM: Active trip point for sensorX
+ |---sensorX_trip_passiveN: Passive trip point for sensorX
+ |---sensorX_trip_hot: Hot trip point for sensorX
+ |---sensorX_trip_critical: Critical trip point for sensorX
+ |---mapX_sensor_name: Sensor Name
+ |---mapX_cdev_name: Cooling device Name
+ |---mapX_trip_type: Trip point type
+ |---mapX_trip_mask: Trip point mask
+ |---mapX_weightY: Weights (used by governors)
+
+3.5: For Thermal Map
+ Each attribute represents the mapping/binding information between
+ a sensor and a cdev, together with a trip type.
+ /sys/class/thermal/zoneX/:
+ |---mapY_trip_type: active/passive
+ |---mapY_sensor_name: cpu
+ |---mapY_cdev_name: proc
+ |---mapY_trip_mask: 0x03
+ |---mapY_weight0: 50
+ |---mapY_weight1: 30
--
1.7.9.5

2013-10-01 13:10:41

by R, Durgadoss

Subject: [PATCHv4 2/9] Thermal: Create sensor level APIs

This patch creates sensor level APIs, in the
generic thermal framework.

A Thermal sensor is a piece of hardware that can report
temperature of the spot in which it is placed. A thermal
sensor driver reads the temperature from this sensor
and reports it out. This kind of driver can be in
any subsystem. If the sensor needs to participate
in platform thermal management, the corresponding
driver can use the APIs introduced in this patch, to
register(or unregister) with the thermal framework.

Signed-off-by: Durgadoss R <[email protected]>
---
drivers/thermal/thermal_core.c | 284 ++++++++++++++++++++++++++++++++++++++++
include/linux/thermal.h | 29 ++++
2 files changed, 313 insertions(+)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 8c5131d..8c46567 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -45,13 +45,16 @@ MODULE_LICENSE("GPL v2");

static DEFINE_IDR(thermal_tz_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_cdev_list);
static LIST_HEAD(thermal_governor_list);

static DEFINE_MUTEX(thermal_list_lock);
+static DEFINE_MUTEX(sensor_list_lock);
static DEFINE_MUTEX(thermal_governor_lock);

static struct thermal_governor *__find_governor(const char *name)
@@ -463,6 +466,103 @@ static void thermal_zone_device_check(struct work_struct *work)
#define to_thermal_zone(_dev) \
container_of(_dev, struct thermal_zone_device, device)

+#define to_thermal_sensor(_dev) \
+ container_of(_dev, struct thermal_sensor, device)
+
+static ssize_t
+sensor_name_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct thermal_sensor *ts = to_thermal_sensor(dev);
+
+ return sprintf(buf, "%s\n", ts->name);
+}
+
+static ssize_t
+sensor_temp_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ int ret;
+ long val;
+ struct thermal_sensor *ts = to_thermal_sensor(dev);
+
+ ret = ts->ops->get_temp(ts, &val);
+
+ return ret ? ret : sprintf(buf, "%ld\n", val);
+}
+
+static ssize_t
+hyst_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ int indx, ret;
+ long val;
+ struct thermal_sensor *ts = to_thermal_sensor(dev);
+
+ if (!sscanf(attr->attr.name, "threshold%d_hyst", &indx))
+ return -EINVAL;
+
+ ret = ts->ops->get_hyst(ts, indx, &val);
+
+ return ret ? ret : sprintf(buf, "%ld\n", val);
+}
+
+static ssize_t
+hyst_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int indx, ret;
+ long val;
+ struct thermal_sensor *ts = to_thermal_sensor(dev);
+
+ if (!ts->ops->set_hyst)
+ return -EPERM;
+
+ if (!sscanf(attr->attr.name, "threshold%d_hyst", &indx))
+ return -EINVAL;
+
+ if (kstrtol(buf, 10, &val))
+ return -EINVAL;
+
+ ret = ts->ops->set_hyst(ts, indx, val);
+
+ return ret ? ret : count;
+}
+
+static ssize_t
+threshold_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ int indx, ret;
+ long val;
+ struct thermal_sensor *ts = to_thermal_sensor(dev);
+
+ if (!sscanf(attr->attr.name, "threshold%d", &indx))
+ return -EINVAL;
+
+ ret = ts->ops->get_threshold(ts, indx, &val);
+
+ return ret ? ret : sprintf(buf, "%ld\n", val);
+}
+
+static ssize_t
+threshold_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int indx, ret;
+ long val;
+ struct thermal_sensor *ts = to_thermal_sensor(dev);
+
+ if (!ts->ops->set_threshold)
+ return -EPERM;
+
+ if (!sscanf(attr->attr.name, "threshold%d", &indx))
+ return -EINVAL;
+
+ if (kstrtol(buf, 10, &val))
+ return -EINVAL;
+
+ ret = ts->ops->set_threshold(ts, indx, val);
+
+ return ret ? ret : count;
+}
+
static ssize_t
type_show(struct device *dev, struct device_attribute *attr, char *buf)
{
@@ -772,6 +872,10 @@ static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show, passive_store);
static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR, policy_show, policy_store);

+/* Thermal sensor attributes */
+static DEVICE_ATTR(sensor_name, 0444, sensor_name_show, NULL);
+static DEVICE_ATTR(temp_input, 0444, sensor_temp_show, NULL);
+
/* sys I/F for cooling device */
#define to_cooling_device(_dev) \
container_of(_dev, struct thermal_cooling_device, device)
@@ -1585,6 +1689,186 @@ static void remove_trip_attrs(struct thermal_zone_device *tz)
kfree(tz->trip_hyst_attrs);
}

+ /**
+ * enable_sensor_thresholds - create sysfs nodes for thresholdX
+ * @ts: the thermal sensor
+ * @count: Number of thresholds supported by sensor hardware
+ *
+ * 'Thresholds' are temperatures programmed into the sensor hardware,
+ * on crossing which the sensor can generate an interrupt.
+ */
+static int enable_sensor_thresholds(struct thermal_sensor *ts, int count)
+{
+ int i;
+ int size = sizeof(struct thermal_attr) * count;
+
+ ts->thresh_attrs = devm_kzalloc(&ts->device, size, GFP_KERNEL);
+ if (!ts->thresh_attrs)
+ return -ENOMEM;
+
+ if (ts->ops->get_hyst) {
+ ts->hyst_attrs = devm_kzalloc(&ts->device, size, GFP_KERNEL);
+ if (!ts->hyst_attrs)
+ return -ENOMEM;
+ }
+
+ ts->thresholds = count;
+
+ /* Create threshold attributes */
+ for (i = 0; i < count; i++) {
+ snprintf(ts->thresh_attrs[i].name, THERMAL_NAME_LENGTH,
+ "threshold%d", i);
+
+ sysfs_attr_init(&ts->thresh_attrs[i].attr.attr);
+ ts->thresh_attrs[i].attr.attr.name = ts->thresh_attrs[i].name;
+ ts->thresh_attrs[i].attr.attr.mode = S_IRUGO | S_IWUSR;
+ ts->thresh_attrs[i].attr.show = threshold_show;
+ ts->thresh_attrs[i].attr.store = threshold_store;
+
+ device_create_file(&ts->device, &ts->thresh_attrs[i].attr);
+
+ /* Create threshold_hyst attributes */
+ if (!ts->ops->get_hyst)
+ continue;
+
+ snprintf(ts->hyst_attrs[i].name, THERMAL_NAME_LENGTH,
+ "threshold%d_hyst", i);
+
+ sysfs_attr_init(&ts->hyst_attrs[i].attr.attr);
+ ts->hyst_attrs[i].attr.attr.name = ts->hyst_attrs[i].name;
+ ts->hyst_attrs[i].attr.attr.mode = S_IRUGO | S_IWUSR;
+ ts->hyst_attrs[i].attr.show = hyst_show;
+ ts->hyst_attrs[i].attr.store = hyst_store;
+
+ device_create_file(&ts->device, &ts->hyst_attrs[i].attr);
+ }
+ return 0;
+}
+
+/**
+ * thermal_sensor_register - register a new thermal sensor
+ * @name: name of the thermal sensor
+ * @count: Number of thresholds supported by hardware
+ * @ops: standard thermal sensor callbacks
+ * @devdata: private device data
+ *
+ * On Success returns a thermal sensor reference, otherwise:
+ * -EINVAL for invalid parameters,
+ * -ENOMEM for insufficient memory cases,
+*/
+struct thermal_sensor *thermal_sensor_register(const char *name, int count,
+ struct thermal_sensor_ops *ops, void *devdata)
+{
+ struct thermal_sensor *ts;
+ int ret;
+
+ if (!name || (name && strlen(name) >= THERMAL_NAME_LENGTH))
+ return ERR_PTR(-EINVAL);
+
+ if (!ops || !ops->get_temp)
+ return ERR_PTR(-EINVAL);
+
+ ts = kzalloc(sizeof(*ts), GFP_KERNEL);
+ if (!ts)
+ return ERR_PTR(-ENOMEM);
+
+ idr_init(&ts->idr);
+ ret = get_idr(&thermal_sensor_idr, &thermal_idr_lock, &ts->id);
+ if (ret)
+ goto exit_free;
+
+ strlcpy(ts->name, name, sizeof(ts->name));
+ ts->ops = ops;
+ ts->devdata = devdata;
+ ts->device.class = &thermal_class;
+
+ dev_set_name(&ts->device, "sensor%d", ts->id);
+ ret = device_register(&ts->device);
+ if (ret)
+ goto exit_idr;
+
+ ret = device_create_file(&ts->device, &dev_attr_sensor_name);
+ if (ret)
+ goto exit_unregister;
+
+ ret = device_create_file(&ts->device, &dev_attr_temp_input);
+ if (ret)
+ goto exit_name;
+
+ if (count > 0 && ts->ops->get_threshold) {
+ ret = enable_sensor_thresholds(ts, count);
+ if (ret)
+ goto exit_temp;
+ }
+
+ /* Add this sensor to the global list of sensors */
+ mutex_lock(&sensor_list_lock);
+ list_add_tail(&ts->node, &thermal_sensor_list);
+ mutex_unlock(&sensor_list_lock);
+
+ return ts;
+
+exit_temp:
+ device_remove_file(&ts->device, &dev_attr_temp_input);
+exit_name:
+ device_remove_file(&ts->device, &dev_attr_sensor_name);
+exit_unregister:
+ device_unregister(&ts->device);
+exit_idr:
+ release_idr(&thermal_sensor_idr, &thermal_idr_lock, ts->id);
+exit_free:
+ kfree(ts);
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(thermal_sensor_register);
+
+/**
+ * remove_thermal_zone - removes the sysfs nodes for given sensor
+ * @ts: Thermal sensor to be removed.
+ */
+void thermal_sensor_unregister(struct thermal_sensor *ts)
+{
+ int i;
+ struct thermal_sensor *pos, *next;
+ bool found = false;
+
+ if (!ts)
+ return;
+
+ mutex_lock(&sensor_list_lock);
+ list_for_each_entry_safe(pos, next, &thermal_sensor_list, node) {
+ if (pos == ts) {
+ list_del(&ts->node);
+ found = true;
+ break;
+ }
+ }
+ mutex_unlock(&sensor_list_lock);
+
+ if (!found)
+ return;
+
+ for (i = 0; i < ts->thresholds; i++) {
+ device_remove_file(&ts->device, &ts->thresh_attrs[i].attr);
+ if (ts->ops->get_hyst) {
+ device_remove_file(&ts->device,
+ &ts->hyst_attrs[i].attr);
+ }
+ }
+
+ device_remove_file(&ts->device, &dev_attr_sensor_name);
+ device_remove_file(&ts->device, &dev_attr_temp_input);
+
+ release_idr(&thermal_sensor_idr, &thermal_idr_lock, ts->id);
+ idr_destroy(&ts->idr);
+
+ device_unregister(&ts->device);
+
+ kfree(ts);
+ return;
+}
+EXPORT_SYMBOL(thermal_sensor_unregister);
+
/**
* thermal_zone_device_register() - register a new thermal zone device
* @type: the thermal zone device type
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index a386a1c..25fc562 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -58,6 +58,7 @@
#define DEFAULT_THERMAL_GOVERNOR "user_space"
#endif

+struct thermal_sensor;
struct thermal_zone_device;
struct thermal_cooling_device;

@@ -139,6 +140,15 @@ struct thermal_cooling_device_ops {
int (*set_cur_state) (struct thermal_cooling_device *, unsigned long);
};

+struct thermal_sensor_ops {
+ int (*get_temp) (struct thermal_sensor *, long *);
+ int (*get_trend) (struct thermal_sensor *, int, enum thermal_trend *);
+ int (*set_threshold) (struct thermal_sensor *, int, long);
+ int (*get_threshold) (struct thermal_sensor *, int, long *);
+ int (*set_hyst) (struct thermal_sensor *, int, long);
+ int (*get_hyst) (struct thermal_sensor *, int, long *);
+};
+
struct thermal_cooling_device {
int id;
char type[THERMAL_NAME_LENGTH];
@@ -156,6 +166,21 @@ struct thermal_attr {
char name[THERMAL_NAME_LENGTH];
};

+struct thermal_sensor {
+ char name[THERMAL_NAME_LENGTH];
+ int id;
+ int temp;
+ int prev_temp;
+ int thresholds;
+ void *devdata;
+ struct idr idr;
+ struct device device;
+ struct list_head node;
+ struct thermal_sensor_ops *ops;
+ struct thermal_attr *thresh_attrs;
+ struct thermal_attr *hyst_attrs;
+};
+
struct thermal_zone_device {
int id;
char type[THERMAL_NAME_LENGTH];
@@ -248,6 +273,10 @@ struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
void thermal_cdev_update(struct thermal_cooling_device *);
void thermal_notify_framework(struct thermal_zone_device *, int);

+struct thermal_sensor *thermal_sensor_register(const char *, int,
+ struct thermal_sensor_ops *, void *);
+void thermal_sensor_unregister(struct thermal_sensor *);
+
#ifdef CONFIG_NET
extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,
enum events event);
--
1.7.9.5

2013-10-09 15:58:06

by Srinivas Pandruvada

Subject: Internal Only discussion : Re: [PATCHv4 7/9] Thermal: Add Documentation to new APIs

On 10/01/2013 11:38 AM, Durgadoss R wrote:
> This patch adds Documentation for the new APIs
> introduced in this patch set. The documentation
> also has a model sysfs structure for reference.
>
> Signed-off-by: Durgadoss R <[email protected]>
> ---
> Documentation/thermal/sysfs-api2.txt | 248 ++++++++++++++++++++++++++++++++++
> 1 file changed, 248 insertions(+)
> create mode 100644 Documentation/thermal/sysfs-api2.txt
>
> diff --git a/Documentation/thermal/sysfs-api2.txt b/Documentation/thermal/sysfs-api2.txt
> new file mode 100644
> index 0000000..c456dad
> --- /dev/null
> +++ b/Documentation/thermal/sysfs-api2.txt
> @@ -0,0 +1,248 @@
> +Thermal Framework
> +-----------------
> +
> +Written by Durgadoss R <[email protected]>
> +Copyright (c) 2013 Intel Corporation
> +
> +Created on: 25 September 2013
> +
> +0. Introduction
> +---------------
> +The Linux thermal framework provides a set of interfaces for thermal
> +sensors and thermal cooling devices (fan, processor...) to register
> +with the thermal management solution and to be a part of it.
> +
> +This document focuses on how to enable new thermal sensors and cooling
> +devices to participate in thermal management. This solution is intended
> +to be 'light-weight' and platform/architecture independent. Any thermal
> +sensor/cooling device should be able to use the infrastructure easily.
> +
> +The goal of thermal framework is to expose the thermal sensor/zone and
> +cooling device attributes in a consistent way. This will help the
> +thermal governors to make use of the information to manage platform
> +thermals efficiently.
> +
> +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 (e.g 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. We see a lot of chip drivers,
> +starting to use thermal framework; we should keep it really
> +light-weight for them to do so but at the same time provide all
> +the necessary features to participate in platform thermal management.
> +
> +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.
> +
> +1. Terminology
> +--------------
> +This section describes the terminology used in the rest of this
> +document as well as the thermal framework code.
> +
> +thermal_sensor: Hardware that can report temperature of a particular
> + spot in the platform, where it is placed. The temperature
> + reported by the sensor is the 'real' temperature reported
> + by the hardware.
> +thermal_zone: A virtual area on the device, that gets heated up. It may
> + have one or more thermal sensors attached to it.
> +cooling_device: Any component that can help in reducing the temperature of
> + a 'hot spot' either by reducing its performance (passive
> + cooling) or by other means(Active cooling E.g. Fan)
> +
> +trip_points: Various temperature levels for each sensor. As of now, we
> + have four levels namely active, passive, hot and critical.
> + Hot and critical trip point support only one value whereas
> + active and passive can have any number of values. These
> + temperature values can come from platform data, and are
> + exposed through sysfs in a consistent manner. Stand-alone
> + thermal sensor drivers are not expected to know these values.
> + These values are RO.
> +thresholds: These are programmable temperature limits, on reaching which
> + the thermal sensor generates an interrupt. The framework is
> + notified about this interrupt to take appropriate action.
> + There can be as many number of thresholds as that of the
> + hardware supports. These values are RW.
> +
> +thermal_map: This provides the mapping (aka binding) information between
> + various sensors and cooling devices in a particular zone.
> + Typically, this also comes from platform data; Stand-alone
> + sensor drivers or cooling device drivers are not expected
> + to know these mapping information.
> +
> +2. Thermal framework APIs
> +-------------------------
> +2.1: For Thermal Sensors
> +2.1.1 thermal_sensor_register:
> + This function creates a new sensor directory under /sys/class/thermal/
> + as sensor[0-*]. This API is expected to be called by thermal sensor
> + drivers. These drivers may or may not be in thermal subsystem. This
> + function returns a thermal_sensor structure on success and appropriate
> + error on failure.
> +
> + name: Name of the sensor
> + count: Number of programmable thresholds associated with this sensor
> + devdata: Device private data
> + ops: Thermal sensor callbacks
> + .get_temp: obtain the current temperature of the sensor
> + .get_trend: obtain the trend of the sensor
> + .get_threshold: get a particular threshold temperature
> + .set_threshold: set a particular threshold temperature
> + .get_hyst: get hysteresis value associated with a threshold
> + .set_hyst: set hysteresis value associated with a threshold
> +
> +2.1.2 thermal_sensor_unregister:
> + This function deletes the sensor directory under /sys/class/thermal/
> + for the given sensor. Thermal sensor drivers may call this API
> + during the driver's 'exit' routine.
> +
> + ts: Thermal sensor that has to be unregistered
> +
> +2.1.3 enable_sensor_thresholds:
> + This function creates 'threshold[0-*]' attributes under a particular
> + sensorX directory. These values are RW. This function is called by
> + the sensr driver only if the sensor supports interrupt mechanism.
> +
> + ts: Thermal sensor for which thresholds have to be enabled
> + num_thresholds: Number of thresholds supported by the sensor
> +
> +2.2: For Cooling Devices
> +2.2.1 thermal_cooling_device_register:
> + This function adds a new thermal cooling device (fan/processor/...)
> + to /sys/class/thermal/ folder as cooling_device[0-*]. This function
> + is expected to be called by cooling device drivers that may be
> + present in other subsystems also.
> +
> + name: the cooling device name
> + devdata: device private data
> + ops: thermal cooling devices callbacks
> + .get_max_state: get the Maximum throttle state of the cooling device
> + .get_cur_state: get the Current throttle state of the cooling device
> + .set_cur_state: set the Current throttle state of the cooling device
> +
> +2.2.2 thermal_cooling_device_unregister:
> + This function deletes the given cdev entry form /sys/class/thermal;
> + and also cleans all the symlinks referred from various zones.
> +
> + cdev: Cooling device to be unregistered
> +
> +2.3: For Thermal Zones
> +2.3.1 create_thermal_zone:
> + This function adds a new 'zone' under /sys/class/thermal/
> + directory as zone[0-*]. This zone has at least one thermal
> + sensor and at most MAX_SENSORS_PER_ZONE number of sensors
> + attached to it. Similarly, this zone has at least one cdev
> + and at most MAX_CDEVS_PER_ZONE number of cdevs attached to it.
> + As of now, MAX_*_PER_ZONE values are hard-coded to 5. We can
> + make them configurable, through Kconfig option(during 'menuconfig').
> +
> + name: Name of the thermal zone
> + devdata: Device private data
> +
> +2.3.2 add_sensor_to_zone
> + This function adds a 'sensorX' entry under /sys/class/thermal/
> + zoneY/ directory. This 'sensorX' is a symlink to the actual
> + sensor entry under /sys/class/thermal/. Correspondingly, the
> + method remove_sensor_from_zone deletes the symlink.
> +
> + tz: thermal zone structure
> + ts: thermal sensor structure
> +
> +2.3.3 add_cdev_to_zone
> + This function adds a 'cdevX' entry under /sys/class/thermal/
> + zoneY/ directory. This 'cdevX' is a symlink to the actual
> + cdev entry under /sys/class/thermal/. Correspondingly, the
> + method remove_cdev_from_zone deletes the symlink.
> +
> + tz: thermal zone structure
> + cdev: thermal cooling device structure
> +
> +2.4 For Thermal Trip
> +2.4.1 add_sensor_trip_info
> + This function adds trip point information for the given sensor,
> + (under a given zone) under /sys/class/thermal/zoneX/.
> + This API creates sysfs attributes namely:
> + sensorX_trip_activeY, sensorX_trip_passiveY, sensorX_trip_hot,
> + sensorX_trip_critical. Each of these hold one trip point temperature
> + (in mC) values, as provided from platform data. As of now, we
> + support many Active and Passive trip points but only one hot
> + one critical trip point.
> +
> + tz: thermal zone structure
> + ts: thermal sensor to which the trip points are attached
> + trip: trip point structure. Usually obtained from platform data
> +
> +2.5 For Thermal Map
> +2.5.1 add_map_entry
> + This function adds a 'map[0-*]' sysfs attribute under
> + /sys/class/thermal/zoneX/mapY_*. Each map attribute helps
> + to describe the binding relationship between a sensor and
> + a cdev in the given zone. The map structure is typically
> + obtained as platform data. For example, through ACPI tables,
> + SFI tables, Device tree etc. The trip mask is a hex value;
> + if 'n' th bit (from LSB) is set, then for trip point 'n' this
> + cdev is throttled with the given weight[n].
> +
> + tz: thermal zone to which a 'map' is being added
> + map: thermal_map structure
> +
> +3. Sysfs attributes structure
> +-----------------------------
> +Thermal sysfs attributes will be represented under /sys/class/thermal.
> +
> +3.1: For Thermal Sensors
> + /sys/class/thermal/sensor[0-*]:
> + |---type: Name of the thermal sensor
> + |---temp_input: Current temperature in mC
> + |---threshold[0-*]: Threshold temperature in mC
> + |---threshold[0-*]_hyst:Optional hysteresis value in mC
> +
> +3.2: For Thermal Cooling Devices
> + /sys/class/thermal/cooling_device[0-*]:
> + |---type: Type of the cooling device
> + |---max_state: Maximum throttle state of the cdev
> + |---cur_state: Current throttle state of the cdev
> +
-I wish if you can have parent child relationship in this table. For
example cpu package id to a core id relationship (If you set the state
for parent device all children get affected. Currently we can't tie a
cpu cooling device to a physical processor. I am sure there will be
other cases. ).
-Also min_state and step_size is useful. I can configure this thermal
daemon, and someone in the community using these to control level
thermals in a macbook with 10+ sensors.
-I call auto down control in thermal daemon. This helps whether we can
force a device from highest state to min state in one step or you need
to gradually bring down. This helps when you use a trigger based user
space control without polling. Once temperature drops below a limit in
ACPI, you don't get any events. So if you don't poll these devices still
under controlled state because of there are no events. For such systems
once you reach a threshold you can drop state to min state.
> +3.3: For Thermal Zones
> + /sys/class/thermal/zone[0-*]:
> + |---name: Name of the thermal
> + |---sensorX: Symlink to ../sensorX
> + |---cdevY: Symlink to ../cdevY
> + |---sensorX_trip_activeM: Active trip point for sensorX
> + |---sensorX_trip_passiveN: Passive trip point for sensorX
> + |---sensorX_trip_hot: Hot trip point for sensorX
> + |---sensorX_trip_critical: Critical trip point for sensorX
> + |---mapX_sensor_name: Sensor Name
> + |---mapX_cdev_name: Cooling device Name
> + |---mapX_trip_type: Trip point type
> + |---mapX_trip_mask: Trip point mask
> + |---mapX_weightY: Weights (used by governors)
> +
> +3.5: For Thermal Map
> + Each attribute represents the mapping/binding information between
> + a sensor and a cdev, together with a trip type.
> + /sys/class/thermal/zoneX/:
> + |---mapY_trip_type: active/passive
> + |---mapY_sensor_name: cpu
> + |---mapY_cdev_name: proc
> + |---mapY_trip_mask: 0x03
> + |---mapY_weight0: 50
> + |---mapY_weight1: 30
Thanks,
Srinivas

2013-10-14 05:14:47

by Zhang, Rui

Subject: Re: [PATCHv4 1/9] Thermal: Check for validity before doing kfree

On Wed, 2013-10-02 at 00:08 +0530, Durgadoss R wrote:
> The thermal_release function is called whenever
> any device belonging to 'thermal' class unregisters.
> This function performs kfree(cdev) without any check.
> In cases where there are more device registrations
> other than just 'thermal_zone' and 'cooling_device'
> this might accidently free memory allocated them
> silently; and cause memory errors.
>
> This patch changes this behavior by doing
> kfree(cdev) only when the device pointer belongs
> to a real cdev i.e. cooling_device.
>
> Signed-off-by: Durgadoss R <[email protected]>

applied to thermal -next.

thanks,
rui
> ---
> drivers/thermal/thermal_core.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 1f02e8e..8c5131d 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1280,7 +1280,8 @@ static void thermal_release(struct device *dev)
> sizeof("thermal_zone") - 1)) {
> tz = to_thermal_zone(dev);
> kfree(tz);
> - } else {
> + } else if(!strncmp(dev_name(dev), "cooling_device",
> + sizeof("cooling_device") - 1)){
> cdev = to_cooling_device(dev);
> kfree(cdev);
> }

2013-10-14 09:26:17

by Zhang, Rui

Subject: Re: [PATCHv4 2/9] Thermal: Create sensor level APIs

On Wed, 2013-10-02 at 00:08 +0530, Durgadoss R wrote:
> This patch creates sensor level APIs, in the
> generic thermal framework.
>
> A Thermal sensor is a piece of hardware that can report
> temperature of the spot in which it is placed. A thermal
> sensor driver reads the temperature from this sensor
> and reports it out. This kind of driver can be in
> any subsystem. If the sensor needs to participate
> in platform thermal management, the corresponding
> driver can use the APIs introduced in this patch, to
> register(or unregister) with the thermal framework.
>
> Signed-off-by: Durgadoss R <[email protected]>
> ---
> drivers/thermal/thermal_core.c | 284 ++++++++++++++++++++++++++++++++++++++++
> include/linux/thermal.h | 29 ++++
> 2 files changed, 313 insertions(+)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 8c5131d..8c46567 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -45,13 +45,16 @@ MODULE_LICENSE("GPL v2");
>
> static DEFINE_IDR(thermal_tz_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_cdev_list);
> static LIST_HEAD(thermal_governor_list);
>
> static DEFINE_MUTEX(thermal_list_lock);
> +static DEFINE_MUTEX(sensor_list_lock);
> static DEFINE_MUTEX(thermal_governor_lock);
>
> static struct thermal_governor *__find_governor(const char *name)
> @@ -463,6 +466,103 @@ static void thermal_zone_device_check(struct work_struct *work)
> #define to_thermal_zone(_dev) \
> container_of(_dev, struct thermal_zone_device, device)
>
> +#define to_thermal_sensor(_dev) \
> + container_of(_dev, struct thermal_sensor, device)
> +
> +static ssize_t
> +sensor_name_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct thermal_sensor *ts = to_thermal_sensor(dev);
> +
> + return sprintf(buf, "%s\n", ts->name);
> +}
> +
> +static ssize_t
> +sensor_temp_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + int ret;
> + long val;
> + struct thermal_sensor *ts = to_thermal_sensor(dev);
> +
> + ret = ts->ops->get_temp(ts, &val);
> +
> + return ret ? ret : sprintf(buf, "%ld\n", val);
> +}
> +
> +static ssize_t
> +hyst_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + int indx, ret;
> + long val;
> + struct thermal_sensor *ts = to_thermal_sensor(dev);
> +
> + if (!sscanf(attr->attr.name, "threshold%d_hyst", &indx))
> + return -EINVAL;
> +
> + ret = ts->ops->get_hyst(ts, indx, &val);
> +
> + return ret ? ret : sprintf(buf, "%ld\n", val);
> +}
> +
> +static ssize_t
> +hyst_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int indx, ret;
> + long val;
> + struct thermal_sensor *ts = to_thermal_sensor(dev);
> +
> + if (!ts->ops->set_hyst)
> + return -EPERM;
> +
> + if (!sscanf(attr->attr.name, "threshold%d_hyst", &indx))
> + return -EINVAL;
> +
> + if (kstrtol(buf, 10, &val))
> + return -EINVAL;
> +
> + ret = ts->ops->set_hyst(ts, indx, val);
> +
> + return ret ? ret : count;
> +}
> +
> +static ssize_t
> +threshold_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + int indx, ret;
> + long val;
> + struct thermal_sensor *ts = to_thermal_sensor(dev);
> +
> + if (!sscanf(attr->attr.name, "threshold%d", &indx))
> + return -EINVAL;
> +
> + ret = ts->ops->get_threshold(ts, indx, &val);
> +
> + return ret ? ret : sprintf(buf, "%ld\n", val);
> +}
> +
> +static ssize_t
> +threshold_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int indx, ret;
> + long val;
> + struct thermal_sensor *ts = to_thermal_sensor(dev);
> +
> + if (!ts->ops->set_threshold)
> + return -EPERM;
> +
> + if (!sscanf(attr->attr.name, "threshold%d", &indx))
> + return -EINVAL;
> +
> + if (kstrtol(buf, 10, &val))
> + return -EINVAL;
> +
> + ret = ts->ops->set_threshold(ts, indx, val);
> +
> + return ret ? ret : count;
> +}
> +
> static ssize_t
> type_show(struct device *dev, struct device_attribute *attr, char *buf)
> {
> @@ -772,6 +872,10 @@ static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
> static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show, passive_store);
> static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR, policy_show, policy_store);
>
> +/* Thermal sensor attributes */
> +static DEVICE_ATTR(sensor_name, 0444, sensor_name_show, NULL);

this attribute is under sensorX/ directory, thus I think "name" is clear
enough.
what do you think?

> +static DEVICE_ATTR(temp_input, 0444, sensor_temp_show, NULL);
> +
what about "temp"? to be consistent with the current naming.

> /* sys I/F for cooling device */
> #define to_cooling_device(_dev) \
> container_of(_dev, struct thermal_cooling_device, device)
> @@ -1585,6 +1689,186 @@ static void remove_trip_attrs(struct thermal_zone_device *tz)
> kfree(tz->trip_hyst_attrs);
> }
>
> + /**
> + * enable_sensor_thresholds - create sysfs nodes for thresholdX
IMO, this name is confusing. It looks like that you are enabling the
sensor hardware here.
so why not "thermal_sensor_sysfs_add", and you can add the "name" and
"temp" attribute in this function as well.

> + * @ts: the thermal sensor
> + * @count: Number of thresholds supported by sensor hardware
> + *
> + * 'Thresholds' are temperatures programmed into the sensor hardware,
> + * on crossing which the sensor can generate an interrupt.
> + */
> +static int enable_sensor_thresholds(struct thermal_sensor *ts, int count)
> +{
> + int i;
> + int size = sizeof(struct thermal_attr) * count;
> +
> + ts->thresh_attrs = devm_kzalloc(&ts->device, size, GFP_KERNEL);
> + if (!ts->thresh_attrs)
> + return -ENOMEM;
> +
> + if (ts->ops->get_hyst) {
> + ts->hyst_attrs = devm_kzalloc(&ts->device, size, GFP_KERNEL);
> + if (!ts->hyst_attrs)
> + return -ENOMEM;
> + }
> +
I do not think you can use devm_kzalloc here.

> + ts->thresholds = count;
> +
> + /* Create threshold attributes */
> + for (i = 0; i < count; i++) {
> + snprintf(ts->thresh_attrs[i].name, THERMAL_NAME_LENGTH,
> + "threshold%d", i);
> +
> + sysfs_attr_init(&ts->thresh_attrs[i].attr.attr);
> + ts->thresh_attrs[i].attr.attr.name = ts->thresh_attrs[i].name;
> + ts->thresh_attrs[i].attr.attr.mode = S_IRUGO | S_IWUSR;
> + ts->thresh_attrs[i].attr.show = threshold_show;
> + ts->thresh_attrs[i].attr.store = threshold_store;
> +
> + device_create_file(&ts->device, &ts->thresh_attrs[i].attr);
> +
> + /* Create threshold_hyst attributes */
> + if (!ts->ops->get_hyst)
> + continue;
> +
> + snprintf(ts->hyst_attrs[i].name, THERMAL_NAME_LENGTH,
> + "threshold%d_hyst", i);
> +
> + sysfs_attr_init(&ts->hyst_attrs[i].attr.attr);
> + ts->hyst_attrs[i].attr.attr.name = ts->hyst_attrs[i].name;
> + ts->hyst_attrs[i].attr.attr.mode = S_IRUGO | S_IWUSR;
> + ts->hyst_attrs[i].attr.show = hyst_show;
> + ts->hyst_attrs[i].attr.store = hyst_store;
> +
> + device_create_file(&ts->device, &ts->hyst_attrs[i].attr);
> + }
> + return 0;
> +}
> +
> +/**
> + * thermal_sensor_register - register a new thermal sensor
> + * @name: name of the thermal sensor
> + * @count: Number of thresholds supported by hardware
> + * @ops: standard thermal sensor callbacks
> + * @devdata: private device data
> + *
> + * On Success returns a thermal sensor reference, otherwise:
> + * -EINVAL for invalid parameters,
> + * -ENOMEM for insufficient memory cases,
> +*/
> +struct thermal_sensor *thermal_sensor_register(const char *name, int count,
> + struct thermal_sensor_ops *ops, void *devdata)
> +{
> + struct thermal_sensor *ts;
> + int ret;
> +
> + if (!name || (name && strlen(name) >= THERMAL_NAME_LENGTH))
> + return ERR_PTR(-EINVAL);
> +
> + if (!ops || !ops->get_temp)
> + return ERR_PTR(-EINVAL);
> +
> + ts = kzalloc(sizeof(*ts), GFP_KERNEL);
> + if (!ts)
> + return ERR_PTR(-ENOMEM);
> +
> + idr_init(&ts->idr);
> + ret = get_idr(&thermal_sensor_idr, &thermal_idr_lock, &ts->id);
> + if (ret)
> + goto exit_free;
> +
> + strlcpy(ts->name, name, sizeof(ts->name));
> + ts->ops = ops;
> + ts->devdata = devdata;
> + ts->device.class = &thermal_class;
> +
> + dev_set_name(&ts->device, "sensor%d", ts->id);
> + ret = device_register(&ts->device);
> + if (ret)
> + goto exit_idr;
> +
> + ret = device_create_file(&ts->device, &dev_attr_sensor_name);
> + if (ret)
> + goto exit_unregister;
> +
> + ret = device_create_file(&ts->device, &dev_attr_temp_input);
> + if (ret)
> + goto exit_name;
> +
> + if (count > 0 && ts->ops->get_threshold) {
> + ret = enable_sensor_thresholds(ts, count);
> + if (ret)
> + goto exit_temp;
> + }
> +
> + /* Add this sensor to the global list of sensors */
> + mutex_lock(&sensor_list_lock);
> + list_add_tail(&ts->node, &thermal_sensor_list);
> + mutex_unlock(&sensor_list_lock);
> +
> + return ts;
> +
> +exit_temp:
> + device_remove_file(&ts->device, &dev_attr_temp_input);
> +exit_name:
> + device_remove_file(&ts->device, &dev_attr_sensor_name);
> +exit_unregister:
> + device_unregister(&ts->device);
> +exit_idr:
> + release_idr(&thermal_sensor_idr, &thermal_idr_lock, ts->id);
> +exit_free:
> + kfree(ts);
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL(thermal_sensor_register);
> +
> +/**
> + * remove_thermal_zone - removes the sysfs nodes for given sensor

s/remove_thermal_zone/thermal_sensor_unregister

> + * @ts: Thermal sensor to be removed.
> + */
> +void thermal_sensor_unregister(struct thermal_sensor *ts)
> +{
> + int i;
> + struct thermal_sensor *pos, *next;
> + bool found = false;
> +
> + if (!ts)
> + return;
> +
> + mutex_lock(&sensor_list_lock);
> + list_for_each_entry_safe(pos, next, &thermal_sensor_list, node) {
> + if (pos == ts) {
> + list_del(&ts->node);
> + found = true;
> + break;
> + }
> + }
> + mutex_unlock(&sensor_list_lock);
> +
> + if (!found)
> + return;
> +
> + for (i = 0; i < ts->thresholds; i++) {
> + device_remove_file(&ts->device, &ts->thresh_attrs[i].attr);
> + if (ts->ops->get_hyst) {
> + device_remove_file(&ts->device,
> + &ts->hyst_attrs[i].attr);
> + }
> + }
> +
> + device_remove_file(&ts->device, &dev_attr_sensor_name);
> + device_remove_file(&ts->device, &dev_attr_temp_input);
> +
hmmm, I'd prefer you to introduce a separate function for the sysfs
cleanup code.

thanks,
rui
> + release_idr(&thermal_sensor_idr, &thermal_idr_lock, ts->id);
> + idr_destroy(&ts->idr);
> +
> + device_unregister(&ts->device);
> +
> + kfree(ts);
> + return;
> +}
> +EXPORT_SYMBOL(thermal_sensor_unregister);
> +
> /**
> * thermal_zone_device_register() - register a new thermal zone device
> * @type: the thermal zone device type
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index a386a1c..25fc562 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -58,6 +58,7 @@
> #define DEFAULT_THERMAL_GOVERNOR "user_space"
> #endif
>
> +struct thermal_sensor;
> struct thermal_zone_device;
> struct thermal_cooling_device;
>
> @@ -139,6 +140,15 @@ struct thermal_cooling_device_ops {
> int (*set_cur_state) (struct thermal_cooling_device *, unsigned long);
> };
>
> +struct thermal_sensor_ops {
> + int (*get_temp) (struct thermal_sensor *, long *);
> + int (*get_trend) (struct thermal_sensor *, int, enum thermal_trend *);
> + int (*set_threshold) (struct thermal_sensor *, int, long);
> + int (*get_threshold) (struct thermal_sensor *, int, long *);
> + int (*set_hyst) (struct thermal_sensor *, int, long);
> + int (*get_hyst) (struct thermal_sensor *, int, long *);
> +};
> +
> struct thermal_cooling_device {
> int id;
> char type[THERMAL_NAME_LENGTH];
> @@ -156,6 +166,21 @@ struct thermal_attr {
> char name[THERMAL_NAME_LENGTH];
> };
>
> +struct thermal_sensor {
> + char name[THERMAL_NAME_LENGTH];
> + int id;
> + int temp;
> + int prev_temp;
> + int thresholds;
> + void *devdata;
> + struct idr idr;
> + struct device device;
> + struct list_head node;
> + struct thermal_sensor_ops *ops;
> + struct thermal_attr *thresh_attrs;
> + struct thermal_attr *hyst_attrs;
> +};
> +
> struct thermal_zone_device {
> int id;
> char type[THERMAL_NAME_LENGTH];
> @@ -248,6 +273,10 @@ struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
> void thermal_cdev_update(struct thermal_cooling_device *);
> void thermal_notify_framework(struct thermal_zone_device *, int);
>
> +struct thermal_sensor *thermal_sensor_register(const char *, int,
> + struct thermal_sensor_ops *, void *);
> +void thermal_sensor_unregister(struct thermal_sensor *);
> +
> #ifdef CONFIG_NET
> extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,
> enum events event);

2013-10-14 16:21:59

by R, Durgadoss

Subject: RE: [PATCHv4 2/9] Thermal: Create sensor level APIs

Hi Rui,

> -----Original Message-----
> From: [email protected] [mailto:linux-pm-
> [email protected]] On Behalf Of Zhang Rui
> Sent: Monday, October 14, 2013 2:56 PM
> To: R, Durgadoss
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCHv4 2/9] Thermal: Create sensor level APIs
>
> On Wed, 2013-10-02 at 00:08 +0530, Durgadoss R wrote:
> > This patch creates sensor level APIs, in the
> > generic thermal framework.
> >
> > A Thermal sensor is a piece of hardware that can report
> > temperature of the spot in which it is placed. A thermal
> > sensor driver reads the temperature from this sensor
> > and reports it out. This kind of driver can be in
> > any subsystem. If the sensor needs to participate
> > in platform thermal management, the corresponding
> > driver can use the APIs introduced in this patch, to
> > register(or unregister) with the thermal framework.
> >
> > Signed-off-by: Durgadoss R <[email protected]>
> > ---
> > drivers/thermal/thermal_core.c | 284
> ++++++++++++++++++++++++++++++++++++++++
> > include/linux/thermal.h | 29 ++++
> > 2 files changed, 313 insertions(+)
> >
> > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> > index 8c5131d..8c46567 100644
> > --- a/drivers/thermal/thermal_core.c
> > +++ b/drivers/thermal/thermal_core.c
> > @@ -45,13 +45,16 @@ MODULE_LICENSE("GPL v2");
> >
> > static DEFINE_IDR(thermal_tz_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_cdev_list);
> > static LIST_HEAD(thermal_governor_list);
> >
> > static DEFINE_MUTEX(thermal_list_lock);
> > +static DEFINE_MUTEX(sensor_list_lock);
> > static DEFINE_MUTEX(thermal_governor_lock);
> >
> > static struct thermal_governor *__find_governor(const char *name)
> > @@ -463,6 +466,103 @@ static void thermal_zone_device_check(struct
> work_struct *work)
> > #define to_thermal_zone(_dev) \
> > container_of(_dev, struct thermal_zone_device, device)
> >
> > +#define to_thermal_sensor(_dev) \
> > + container_of(_dev, struct thermal_sensor, device)
> > +
> > +static ssize_t
> > +sensor_name_show(struct device *dev, struct device_attribute *attr, char
> *buf)
> > +{
> > + struct thermal_sensor *ts = to_thermal_sensor(dev);
> > +
> > + return sprintf(buf, "%s\n", ts->name);
> > +}
> > +
> > +static ssize_t
> > +sensor_temp_show(struct device *dev, struct device_attribute *attr, char
> *buf)
> > +{
> > + int ret;
> > + long val;
> > + struct thermal_sensor *ts = to_thermal_sensor(dev);
> > +
> > + ret = ts->ops->get_temp(ts, &val);
> > +
> > + return ret ? ret : sprintf(buf, "%ld\n", val);
> > +}
> > +
> > +static ssize_t
> > +hyst_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > + int indx, ret;
> > + long val;
> > + struct thermal_sensor *ts = to_thermal_sensor(dev);
> > +
> > + if (!sscanf(attr->attr.name, "threshold%d_hyst", &indx))
> > + return -EINVAL;
> > +
> > + ret = ts->ops->get_hyst(ts, indx, &val);
> > +
> > + return ret ? ret : sprintf(buf, "%ld\n", val);
> > +}
> > +
> > +static ssize_t
> > +hyst_store(struct device *dev, struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + int indx, ret;
> > + long val;
> > + struct thermal_sensor *ts = to_thermal_sensor(dev);
> > +
> > + if (!ts->ops->set_hyst)
> > + return -EPERM;
> > +
> > + if (!sscanf(attr->attr.name, "threshold%d_hyst", &indx))
> > + return -EINVAL;
> > +
> > + if (kstrtol(buf, 10, &val))
> > + return -EINVAL;
> > +
> > + ret = ts->ops->set_hyst(ts, indx, val);
> > +
> > + return ret ? ret : count;
> > +}
> > +
> > +static ssize_t
> > +threshold_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > + int indx, ret;
> > + long val;
> > + struct thermal_sensor *ts = to_thermal_sensor(dev);
> > +
> > + if (!sscanf(attr->attr.name, "threshold%d", &indx))
> > + return -EINVAL;
> > +
> > + ret = ts->ops->get_threshold(ts, indx, &val);
> > +
> > + return ret ? ret : sprintf(buf, "%ld\n", val);
> > +}
> > +
> > +static ssize_t
> > +threshold_store(struct device *dev, struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + int indx, ret;
> > + long val;
> > + struct thermal_sensor *ts = to_thermal_sensor(dev);
> > +
> > + if (!ts->ops->set_threshold)
> > + return -EPERM;
> > +
> > + if (!sscanf(attr->attr.name, "threshold%d", &indx))
> > + return -EINVAL;
> > +
> > + if (kstrtol(buf, 10, &val))
> > + return -EINVAL;
> > +
> > + ret = ts->ops->set_threshold(ts, indx, val);
> > +
> > + return ret ? ret : count;
> > +}
> > +
> > static ssize_t
> > type_show(struct device *dev, struct device_attribute *attr, char *buf)
> > {
> > @@ -772,6 +872,10 @@ static DEVICE_ATTR(mode, 0644, mode_show,
> mode_store);
> > static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show,
> passive_store);
> > static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR, policy_show, policy_store);
> >
> > +/* Thermal sensor attributes */
> > +static DEVICE_ATTR(sensor_name, 0444, sensor_name_show, NULL);
>
> this attribute is under sensorX/ directory, thus I think "name" is clear
> enough.
> what do you think?

Yes. That makes sense. I will give this a try in next version.

>
> > +static DEVICE_ATTR(temp_input, 0444, sensor_temp_show, NULL);
> > +
> what about "temp"? to be consistent with the current naming.

I think I tried this, but we already had a 'temp' attribute. So, was compelled
to give another name.

I will try this one again and let you know.

>
> > /* sys I/F for cooling device */
> > #define to_cooling_device(_dev) \
> > container_of(_dev, struct thermal_cooling_device, device)
> > @@ -1585,6 +1689,186 @@ static void remove_trip_attrs(struct
> thermal_zone_device *tz)
> > kfree(tz->trip_hyst_attrs);
> > }
> >
> > + /**
> > + * enable_sensor_thresholds - create sysfs nodes for thresholdX
> IMO, this name is confusing. It looks like that you are enabling the
> sensor hardware here.
> so why not "thermal_sensor_sysfs_add", and you can add the "name" and
> "temp" attribute in this function as well.

Okay. Will do.

>
> > + * @ts: the thermal sensor
> > + * @count: Number of thresholds supported by sensor hardware
> > + *
> > + * 'Thresholds' are temperatures programmed into the sensor hardware,
> > + * on crossing which the sensor can generate an interrupt.
> > + */
> > +static int enable_sensor_thresholds(struct thermal_sensor *ts, int count)
> > +{
> > + int i;
> > + int size = sizeof(struct thermal_attr) * count;
> > +
> > + ts->thresh_attrs = devm_kzalloc(&ts->device, size, GFP_KERNEL);
> > + if (!ts->thresh_attrs)
> > + return -ENOMEM;
> > +
> > + if (ts->ops->get_hyst) {
> > + ts->hyst_attrs = devm_kzalloc(&ts->device, size, GFP_KERNEL);
> > + if (!ts->hyst_attrs)
> > + return -ENOMEM;
> > + }
> > +
> I do not think you can use devm_kzalloc here.

Any specific reason ?
I thought this memory will be freed when we do device_unregister on 'ts'.
Please enlighten me if this is not the case.

>
> > + ts->thresholds = count;
> > +
> > + /* Create threshold attributes */
> > + for (i = 0; i < count; i++) {
> > + snprintf(ts->thresh_attrs[i].name, THERMAL_NAME_LENGTH,
> > + "threshold%d", i);
> > +
> > + sysfs_attr_init(&ts->thresh_attrs[i].attr.attr);
> > + ts->thresh_attrs[i].attr.attr.name = ts->thresh_attrs[i].name;
> > + ts->thresh_attrs[i].attr.attr.mode = S_IRUGO | S_IWUSR;
> > + ts->thresh_attrs[i].attr.show = threshold_show;
> > + ts->thresh_attrs[i].attr.store = threshold_store;
> > +
> > + device_create_file(&ts->device, &ts->thresh_attrs[i].attr);
> > +
> > + /* Create threshold_hyst attributes */
> > + if (!ts->ops->get_hyst)
> > + continue;
> > +
> > + snprintf(ts->hyst_attrs[i].name, THERMAL_NAME_LENGTH,
> > + "threshold%d_hyst", i);
> > +
> > + sysfs_attr_init(&ts->hyst_attrs[i].attr.attr);
> > + ts->hyst_attrs[i].attr.attr.name = ts->hyst_attrs[i].name;
> > + ts->hyst_attrs[i].attr.attr.mode = S_IRUGO | S_IWUSR;
> > + ts->hyst_attrs[i].attr.show = hyst_show;
> > + ts->hyst_attrs[i].attr.store = hyst_store;
> > +
> > + device_create_file(&ts->device, &ts->hyst_attrs[i].attr);
> > + }
> > + return 0;
> > +}
> > +
> > +/**
> > + * thermal_sensor_register - register a new thermal sensor
> > + * @name: name of the thermal sensor
> > + * @count: Number of thresholds supported by hardware
> > + * @ops: standard thermal sensor callbacks
> > + * @devdata: private device data
> > + *
> > + * On Success returns a thermal sensor reference, otherwise:
> > + * -EINVAL for invalid parameters,
> > + * -ENOMEM for insufficient memory cases,
> > +*/
> > +struct thermal_sensor *thermal_sensor_register(const char *name, int count,
> > + struct thermal_sensor_ops *ops, void *devdata)
> > +{
> > + struct thermal_sensor *ts;
> > + int ret;
> > +
> > + if (!name || (name && strlen(name) >= THERMAL_NAME_LENGTH))
> > + return ERR_PTR(-EINVAL);
> > +
> > + if (!ops || !ops->get_temp)
> > + return ERR_PTR(-EINVAL);
> > +
> > + ts = kzalloc(sizeof(*ts), GFP_KERNEL);
> > + if (!ts)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + idr_init(&ts->idr);
> > + ret = get_idr(&thermal_sensor_idr, &thermal_idr_lock, &ts->id);
> > + if (ret)
> > + goto exit_free;
> > +
> > + strlcpy(ts->name, name, sizeof(ts->name));
> > + ts->ops = ops;
> > + ts->devdata = devdata;
> > + ts->device.class = &thermal_class;
> > +
> > + dev_set_name(&ts->device, "sensor%d", ts->id);
> > + ret = device_register(&ts->device);
> > + if (ret)
> > + goto exit_idr;
> > +
> > + ret = device_create_file(&ts->device, &dev_attr_sensor_name);
> > + if (ret)
> > + goto exit_unregister;
> > +
> > + ret = device_create_file(&ts->device, &dev_attr_temp_input);
> > + if (ret)
> > + goto exit_name;
> > +
> > + if (count > 0 && ts->ops->get_threshold) {
> > + ret = enable_sensor_thresholds(ts, count);
> > + if (ret)
> > + goto exit_temp;
> > + }
> > +
> > + /* Add this sensor to the global list of sensors */
> > + mutex_lock(&sensor_list_lock);
> > + list_add_tail(&ts->node, &thermal_sensor_list);
> > + mutex_unlock(&sensor_list_lock);
> > +
> > + return ts;
> > +
> > +exit_temp:
> > + device_remove_file(&ts->device, &dev_attr_temp_input);
> > +exit_name:
> > + device_remove_file(&ts->device, &dev_attr_sensor_name);
> > +exit_unregister:
> > + device_unregister(&ts->device);
> > +exit_idr:
> > + release_idr(&thermal_sensor_idr, &thermal_idr_lock, ts->id);
> > +exit_free:
> > + kfree(ts);
> > + return ERR_PTR(ret);
> > +}
> > +EXPORT_SYMBOL(thermal_sensor_register);
> > +
> > +/**
> > + * remove_thermal_zone - removes the sysfs nodes for given sensor
>
> s/remove_thermal_zone/thermal_sensor_unregister
>
> > + * @ts: Thermal sensor to be removed.
> > + */
> > +void thermal_sensor_unregister(struct thermal_sensor *ts)
> > +{
> > + int i;
> > + struct thermal_sensor *pos, *next;
> > + bool found = false;
> > +
> > + if (!ts)
> > + return;
> > +
> > + mutex_lock(&sensor_list_lock);
> > + list_for_each_entry_safe(pos, next, &thermal_sensor_list, node) {
> > + if (pos == ts) {
> > + list_del(&ts->node);
> > + found = true;
> > + break;
> > + }
> > + }
> > + mutex_unlock(&sensor_list_lock);
> > +
> > + if (!found)
> > + return;
> > +
> > + for (i = 0; i < ts->thresholds; i++) {
> > + device_remove_file(&ts->device, &ts->thresh_attrs[i].attr);
> > + if (ts->ops->get_hyst) {
> > + device_remove_file(&ts->device,
> > + &ts->hyst_attrs[i].attr);
> > + }
> > + }
> > +
> > + device_remove_file(&ts->device, &dev_attr_sensor_name);
> > + device_remove_file(&ts->device, &dev_attr_temp_input);
> > +
> hmmm, I'd prefer you to introduce a separate function for the sysfs
> cleanup code.

>From the above comment, we can have a similar
thermal_sensor_sysfs_remove function.

Thanks,
Durga

>
> thanks,
> rui
> > + release_idr(&thermal_sensor_idr, &thermal_idr_lock, ts->id);
> > + idr_destroy(&ts->idr);
> > +
> > + device_unregister(&ts->device);
> > +
> > + kfree(ts);
> > + return;
> > +}
> > +EXPORT_SYMBOL(thermal_sensor_unregister);
> > +
> > /**
> > * thermal_zone_device_register() - register a new thermal zone device
> > * @type: the thermal zone device type
> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> > index a386a1c..25fc562 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -58,6 +58,7 @@
> > #define DEFAULT_THERMAL_GOVERNOR "user_space"
> > #endif
> >
> > +struct thermal_sensor;
> > struct thermal_zone_device;
> > struct thermal_cooling_device;
> >
> > @@ -139,6 +140,15 @@ struct thermal_cooling_device_ops {
> > int (*set_cur_state) (struct thermal_cooling_device *, unsigned long);
> > };
> >
> > +struct thermal_sensor_ops {
> > + int (*get_temp) (struct thermal_sensor *, long *);
> > + int (*get_trend) (struct thermal_sensor *, int, enum thermal_trend *);
> > + int (*set_threshold) (struct thermal_sensor *, int, long);
> > + int (*get_threshold) (struct thermal_sensor *, int, long *);
> > + int (*set_hyst) (struct thermal_sensor *, int, long);
> > + int (*get_hyst) (struct thermal_sensor *, int, long *);
> > +};
> > +
> > struct thermal_cooling_device {
> > int id;
> > char type[THERMAL_NAME_LENGTH];
> > @@ -156,6 +166,21 @@ struct thermal_attr {
> > char name[THERMAL_NAME_LENGTH];
> > };
> >
> > +struct thermal_sensor {
> > + char name[THERMAL_NAME_LENGTH];
> > + int id;
> > + int temp;
> > + int prev_temp;
> > + int thresholds;
> > + void *devdata;
> > + struct idr idr;
> > + struct device device;
> > + struct list_head node;
> > + struct thermal_sensor_ops *ops;
> > + struct thermal_attr *thresh_attrs;
> > + struct thermal_attr *hyst_attrs;
> > +};
> > +
> > struct thermal_zone_device {
> > int id;
> > char type[THERMAL_NAME_LENGTH];
> > @@ -248,6 +273,10 @@ struct thermal_instance *get_thermal_instance(struct
> thermal_zone_device *,
> > void thermal_cdev_update(struct thermal_cooling_device *);
> > void thermal_notify_framework(struct thermal_zone_device *, int);
> >
> > +struct thermal_sensor *thermal_sensor_register(const char *, int,
> > + struct thermal_sensor_ops *, void *);
> > +void thermal_sensor_unregister(struct thermal_sensor *);
> > +
> > #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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2013-10-15 09:45:08

by Zhang, Rui

Subject: RE: [PATCHv4 2/9] Thermal: Create sensor level APIs

On Mon, 2013-10-14 at 10:21 -0600, R, Durgadoss wrote:
> Hi Rui,
>
> > -----Original Message-----
> > From: [email protected] [mailto:linux-pm-
> > [email protected]] On Behalf Of Zhang Rui
> > Sent: Monday, October 14, 2013 2:56 PM
> > To: R, Durgadoss
> > Cc: [email protected]; [email protected]; linux-
> > [email protected]; [email protected]; [email protected]
> > Subject: Re: [PATCHv4 2/9] Thermal: Create sensor level APIs
> >
> > On Wed, 2013-10-02 at 00:08 +0530, Durgadoss R wrote:
> > > This patch creates sensor level APIs, in the
> > > generic thermal framework.
> > >
> > > A Thermal sensor is a piece of hardware that can report
> > > temperature of the spot in which it is placed. A thermal
> > > sensor driver reads the temperature from this sensor
> > > and reports it out. This kind of driver can be in
> > > any subsystem. If the sensor needs to participate
> > > in platform thermal management, the corresponding
> > > driver can use the APIs introduced in this patch, to
> > > register(or unregister) with the thermal framework.
> > >
> > > Signed-off-by: Durgadoss R <[email protected]>
> > > ---
> > > drivers/thermal/thermal_core.c | 284
> > ++++++++++++++++++++++++++++++++++++++++
> > > include/linux/thermal.h | 29 ++++
> > > 2 files changed, 313 insertions(+)
> > >
> > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> > > index 8c5131d..8c46567 100644
> > > --- a/drivers/thermal/thermal_core.c
> > > +++ b/drivers/thermal/thermal_core.c
> > > @@ -45,13 +45,16 @@ MODULE_LICENSE("GPL v2");
> > >
> > > static DEFINE_IDR(thermal_tz_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_cdev_list);
> > > static LIST_HEAD(thermal_governor_list);
> > >
> > > static DEFINE_MUTEX(thermal_list_lock);
> > > +static DEFINE_MUTEX(sensor_list_lock);
> > > static DEFINE_MUTEX(thermal_governor_lock);
> > >
> > > static struct thermal_governor *__find_governor(const char *name)
> > > @@ -463,6 +466,103 @@ static void thermal_zone_device_check(struct
> > work_struct *work)
> > > #define to_thermal_zone(_dev) \
> > > container_of(_dev, struct thermal_zone_device, device)
> > >
> > > +#define to_thermal_sensor(_dev) \
> > > + container_of(_dev, struct thermal_sensor, device)
> > > +
> > > +static ssize_t
> > > +sensor_name_show(struct device *dev, struct device_attribute *attr, char
> > *buf)
> > > +{
> > > + struct thermal_sensor *ts = to_thermal_sensor(dev);
> > > +
> > > + return sprintf(buf, "%s\n", ts->name);
> > > +}
> > > +
> > > +static ssize_t
> > > +sensor_temp_show(struct device *dev, struct device_attribute *attr, char
> > *buf)
> > > +{
> > > + int ret;
> > > + long val;
> > > + struct thermal_sensor *ts = to_thermal_sensor(dev);
> > > +
> > > + ret = ts->ops->get_temp(ts, &val);
> > > +
> > > + return ret ? ret : sprintf(buf, "%ld\n", val);
> > > +}
> > > +
> > > +static ssize_t
> > > +hyst_show(struct device *dev, struct device_attribute *attr, char *buf)
> > > +{
> > > + int indx, ret;
> > > + long val;
> > > + struct thermal_sensor *ts = to_thermal_sensor(dev);
> > > +
> > > + if (!sscanf(attr->attr.name, "threshold%d_hyst", &indx))
> > > + return -EINVAL;
> > > +
> > > + ret = ts->ops->get_hyst(ts, indx, &val);
> > > +
> > > + return ret ? ret : sprintf(buf, "%ld\n", val);
> > > +}
> > > +
> > > +static ssize_t
> > > +hyst_store(struct device *dev, struct device_attribute *attr,
> > > + const char *buf, size_t count)
> > > +{
> > > + int indx, ret;
> > > + long val;
> > > + struct thermal_sensor *ts = to_thermal_sensor(dev);
> > > +
> > > + if (!ts->ops->set_hyst)
> > > + return -EPERM;
> > > +
> > > + if (!sscanf(attr->attr.name, "threshold%d_hyst", &indx))
> > > + return -EINVAL;
> > > +
> > > + if (kstrtol(buf, 10, &val))
> > > + return -EINVAL;
> > > +
> > > + ret = ts->ops->set_hyst(ts, indx, val);
> > > +
> > > + return ret ? ret : count;
> > > +}
> > > +
> > > +static ssize_t
> > > +threshold_show(struct device *dev, struct device_attribute *attr, char *buf)
> > > +{
> > > + int indx, ret;
> > > + long val;
> > > + struct thermal_sensor *ts = to_thermal_sensor(dev);
> > > +
> > > + if (!sscanf(attr->attr.name, "threshold%d", &indx))
> > > + return -EINVAL;
> > > +
> > > + ret = ts->ops->get_threshold(ts, indx, &val);
> > > +
> > > + return ret ? ret : sprintf(buf, "%ld\n", val);
> > > +}
> > > +
> > > +static ssize_t
> > > +threshold_store(struct device *dev, struct device_attribute *attr,
> > > + const char *buf, size_t count)
> > > +{
> > > + int indx, ret;
> > > + long val;
> > > + struct thermal_sensor *ts = to_thermal_sensor(dev);
> > > +
> > > + if (!ts->ops->set_threshold)
> > > + return -EPERM;
> > > +
> > > + if (!sscanf(attr->attr.name, "threshold%d", &indx))
> > > + return -EINVAL;
> > > +
> > > + if (kstrtol(buf, 10, &val))
> > > + return -EINVAL;
> > > +
> > > + ret = ts->ops->set_threshold(ts, indx, val);
> > > +
> > > + return ret ? ret : count;
> > > +}
> > > +
> > > static ssize_t
> > > type_show(struct device *dev, struct device_attribute *attr, char *buf)
> > > {
> > > @@ -772,6 +872,10 @@ static DEVICE_ATTR(mode, 0644, mode_show,
> > mode_store);
> > > static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show,
> > passive_store);
> > > static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR, policy_show, policy_store);
> > >
> > > +/* Thermal sensor attributes */
> > > +static DEVICE_ATTR(sensor_name, 0444, sensor_name_show, NULL);
> >
> > this attribute is under sensorX/ directory, thus I think "name" is clear
> > enough.
> > what do you think?
>
> Yes. That makes sense. I will give this a try in next version.
>
> >
> > > +static DEVICE_ATTR(temp_input, 0444, sensor_temp_show, NULL);
> > > +
> > what about "temp"? to be consistent with the current naming.
>
> I think I tried this, but we already had a 'temp' attribute. So, was compelled
> to give another name.
>
> I will try this one again and let you know.
>
> >
> > > /* sys I/F for cooling device */
> > > #define to_cooling_device(_dev) \
> > > container_of(_dev, struct thermal_cooling_device, device)
> > > @@ -1585,6 +1689,186 @@ static void remove_trip_attrs(struct
> > thermal_zone_device *tz)
> > > kfree(tz->trip_hyst_attrs);
> > > }
> > >
> > > + /**
> > > + * enable_sensor_thresholds - create sysfs nodes for thresholdX
> > IMO, this name is confusing. It looks like that you are enabling the
> > sensor hardware here.
> > so why not "thermal_sensor_sysfs_add", and you can add the "name" and
> > "temp" attribute in this function as well.
>
> Okay. Will do.
>
> >
> > > + * @ts: the thermal sensor
> > > + * @count: Number of thresholds supported by sensor hardware
> > > + *
> > > + * 'Thresholds' are temperatures programmed into the sensor hardware,
> > > + * on crossing which the sensor can generate an interrupt.
> > > + */
> > > +static int enable_sensor_thresholds(struct thermal_sensor *ts, int count)
> > > +{
> > > + int i;
> > > + int size = sizeof(struct thermal_attr) * count;
> > > +
> > > + ts->thresh_attrs = devm_kzalloc(&ts->device, size, GFP_KERNEL);
> > > + if (!ts->thresh_attrs)
> > > + return -ENOMEM;
> > > +
> > > + if (ts->ops->get_hyst) {
> > > + ts->hyst_attrs = devm_kzalloc(&ts->device, size, GFP_KERNEL);
> > > + if (!ts->hyst_attrs)
> > > + return -ENOMEM;
> > > + }
> > > +
> > I do not think you can use devm_kzalloc here.
>
> Any specific reason ?
> I thought this memory will be freed when we do device_unregister on 'ts'.
> Please enlighten me if this is not the case.
>
yes, you're right. It is me misunderstood your code.
Sorry about the noise.

thanks,
rui

2013-10-15 10:22:48

by Zhang, Rui

Subject: Re: [PATCHv4 3/9] Thermal: Create zone level APIs

On Wed, 2013-10-02 at 00:08 +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 <[email protected]>
> ---
> drivers/thermal/thermal_core.c | 267 +++++++++++++++++++++++++++++++++++++++-
> include/linux/thermal.h | 23 ++++
> 2 files changed, 289 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 8c46567..a053b60 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -44,19 +44,48 @@ MODULE_DESCRIPTION("Generic thermal management sysfs support");
> MODULE_LICENSE("GPL v2");
>
> 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(&tz->lock); \
> + 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); \
> + mutex_unlock(&tz->lock); \
> + } while (0); \
> + ret; \
> +})
> +
> static struct thermal_governor *__find_governor(const char *name)
> {
> struct thermal_governor *pos;
> @@ -461,15 +490,47 @@ 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));
> +
> + mutex_lock(&tz->lock);
> +
> + /* 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--;
> + mutex_unlock(&tz->lock);
> +}
> +
> /* 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);
> @@ -876,6 +937,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)
> @@ -1689,7 +1752,7 @@ static void remove_trip_attrs(struct thermal_zone_device *tz)
> kfree(tz->trip_hyst_attrs);
> }
>
> - /**
> +/**
> * enable_sensor_thresholds - create sysfs nodes for thresholdX
> * @ts: the thermal sensor
> * @count: Number of thresholds supported by sensor hardware
> @@ -1746,6 +1809,200 @@ static int enable_sensor_thresholds(struct thermal_sensor *ts, int count)
> }
>
> /**
> + * create_thermal_zone - create sysfs nodes for thermal zone
> + * @name: Name of the thermla zone
> + * @devdata: Data supplied by the caller
> + *
> + * On Success returns a thermal zone reference, otherwise:
> + * -EINVAL for invalid parameters,
> + * -ENOMEM for insufficient memory cases,
> + */
> +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;
> +
> + mutex_init(&tz->lock);
> + strlcpy(tz->name, name, sizeof(tz->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:
> + mutex_destroy(&tz->lock);
> + release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
> +exit_free:
> + kfree(tz);
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL(create_thermal_zone);
> +
as this is a thermal subsystem API, I'd prefer to use the names starting
with thermal_xxx.

> +/**
> + * remove_thermal_zone - removes the sysfs nodes for given tz
> + * @tz: Thermal zone to be removed.
> + */
> +void remove_thermal_zone(struct thermal_zone *tz)
> +{
> + struct thermal_zone *pos, *next;
> + struct thermal_sensor *ts;
> + 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;
> +
> + for_each_thermal_sensor(ts)
> + remove_sensor_from_zone(tz, ts);
> +
> + device_remove_file(&tz->device, &dev_attr_zone_name);
> +
> + mutex_destroy(&tz->lock);
> + 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);
> +
ditto

> +/**
> + * get_sensor_by_name() - search for a sensor and returns its reference
> + * @name: thermal sensor name to fetch the reference
> + *
> + * On success returns a reference to a unique sensor with
> + * name matching that of @name, an ERR_PTR otherwise:
> + * -EINVAL for invalid paramenters
> + * -ENODEV for sensor not found
> + * -EEXIST for multiple matches
> + */
> +struct thermal_sensor *get_sensor_by_name(const char *name)
> +{
> + int found = 0;
> + struct thermal_sensor *pos;
> + struct thermal_sensor *ts = ERR_PTR(-EINVAL);
> +
> + if (!name)
> + return ts;
> +
> + mutex_lock(&sensor_list_lock);
> + for_each_thermal_sensor(pos) {
> + if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) {
> + found++;
> + ts = pos;
> + }
> + }
> + mutex_unlock(&sensor_list_lock);
> +
> + if (found == 0)
> + ts = ERR_PTR(-ENODEV);
> + else if (found > 1)
> + ts = ERR_PTR(-EEXIST);
> +
> + return ts;
> +}
> +EXPORT_SYMBOL(get_sensor_by_name);
> +
ditto

> +/**
> + * add_sensor_to_zone - Add @ts to thermal zone @tz
> + * @tz: Thermal zone reference
> + * @ts: Thermal sensor reference
> + *
> + * Returns 0 on success, otherwise
> + * -EINVAL for invalid paramenters
> + * -EINVAL when trying to add more zones than MAX_SENSORS_PER_ZONE
> + * -EEXIST when trying add existing thermal sensor again
> + */
> +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;
> + }
> +
> + /* Protect against 'ts' being unregistered */
> + 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;
> +
> + mutex_lock(&tz->lock);
> +
> + if (tz->sensor_indx >= MAX_SENSORS_PER_ZONE - 1) {
> + dev_err(&tz->device, "Only %d sensors allowed per zone\n",
> + MAX_SENSORS_PER_ZONE);
> + sysfs_remove_link(&tz->device.kobj,
> + kobject_name(&ts->device.kobj));
> + ret = -EINVAL;
> + goto exit_indx_check;
> + }
> +
> + tz->sensors[tz->sensor_indx++] = ts;
> +
> +exit_indx_check:
> + mutex_unlock(&tz->lock);
> +exit_sensor:
> + mutex_unlock(&sensor_list_lock);
> +exit_zone:
> + mutex_unlock(&zone_list_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL(add_sensor_to_zone);
> +
ditto

> +/**
> * thermal_sensor_register - register a new thermal sensor
> * @name: name of the thermal sensor
> * @count: Number of thresholds supported by hardware
> @@ -1829,6 +2086,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;
>
> @@ -1848,6 +2106,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);
> +
I do not see the code to bind the sensors to zone in this patch, and I
guess we support manually binding only, via explicitly calling () in
platform thermal driver, at the moment, right?

thanks,
rui
> 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 25fc562..2e12321 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -58,6 +58,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;
> @@ -207,6 +209,22 @@ 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 mutex lock; /* protect elements of this structure */
> + 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];
> @@ -277,6 +295,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);

2013-10-15 10:27:57

by Zhang, Rui

Subject: Re: [PATCHv4 4/9] Thermal: Add APIs to bind cdev to new zone structure

On Wed, 2013-10-02 at 00:08 +0530, Durgadoss R wrote:
> This patch creates new APIs to add/remove a
> cdev to/from a zone. This patch does not change
> the old cooling device implementation.
>
> Signed-off-by: Durgadoss R <[email protected]>
I'm okay with this patch except the API naming.

thanks,
rui
> ---
> drivers/thermal/thermal_core.c | 136 ++++++++++++++++++++++++++++++++++++++++
> include/linux/thermal.h | 9 +++
> 2 files changed, 145 insertions(+)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index a053b60..3c4ef62 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -58,6 +58,7 @@ 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(cdev_list_lock);
> static DEFINE_MUTEX(thermal_governor_lock);
>
> #define for_each_thermal_sensor(pos) \
> @@ -86,6 +87,9 @@ static DEFINE_MUTEX(thermal_governor_lock);
> ret; \
> })
>
> +#define for_each_cdev(pos) \
> + list_for_each_entry(pos, &thermal_cdev_list, node)
> +
> static struct thermal_governor *__find_governor(const char *name)
> {
> struct thermal_governor *pos;
> @@ -511,6 +515,27 @@ static void remove_sensor_from_zone(struct thermal_zone *tz,
> mutex_unlock(&tz->lock);
> }
>
> +static void remove_cdev_from_zone(struct thermal_zone *tz,
> + struct thermal_cooling_device *cdev)
> +{
> + int j, indx;
> +
> + indx = GET_INDEX(tz, cdev, cdev);
> + if (indx < 0)
> + return;
> +
> + sysfs_remove_link(&tz->device.kobj, kobject_name(&cdev->device.kobj));
> +
> + mutex_lock(&tz->lock);
> +
> + /* Shift the entries in the tz->cdevs array */
> + for (j = indx; j < MAX_CDEVS_PER_ZONE - 1; j++)
> + tz->cdevs[j] = tz->cdevs[j + 1];
> +
> + tz->cdev_indx--;
> + mutex_unlock(&tz->lock);
> +}
> +
> /* sys I/F for thermal zone */
>
> #define to_thermal_zone(_dev) \
> @@ -1555,6 +1580,7 @@ void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)
> int i;
> const struct thermal_zone_params *tzp;
> struct thermal_zone_device *tz;
> + struct thermal_zone *tmp_tz;
> struct thermal_cooling_device *pos = NULL;
>
> if (!cdev)
> @@ -1592,6 +1618,13 @@ void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)
>
> mutex_unlock(&thermal_list_lock);
>
> + mutex_lock(&zone_list_lock);
> +
> + for_each_thermal_zone(tmp_tz)
> + remove_cdev_from_zone(tmp_tz, cdev);
> +
> + mutex_unlock(&zone_list_lock);
> +
> if (cdev->type[0])
> device_remove_file(&cdev->device, &dev_attr_cdev_type);
> device_remove_file(&cdev->device, &dev_attr_max_state);
> @@ -1873,6 +1906,7 @@ void remove_thermal_zone(struct thermal_zone *tz)
> {
> struct thermal_zone *pos, *next;
> struct thermal_sensor *ts;
> + struct thermal_cooling_device *cdev;
> bool found = false;
>
> if (!tz)
> @@ -1894,6 +1928,9 @@ void remove_thermal_zone(struct thermal_zone *tz)
> for_each_thermal_sensor(ts)
> remove_sensor_from_zone(tz, ts);
>
> + for_each_cdev(cdev)
> + remove_cdev_from_zone(tz, cdev);
> +
> device_remove_file(&tz->device, &dev_attr_zone_name);
>
> mutex_destroy(&tz->lock);
> @@ -1909,6 +1946,47 @@ exit:
> EXPORT_SYMBOL(remove_thermal_zone);
>
> /**
> + * get_cdev_by_name() - search for a cdev and returns its reference
> + * @name: cooling device name to fetch the reference
> + *
> + * On success returns a reference to an unique cdev with
> + * the name matching that of @name, an ERR_PTR otherwise:
> + * -EINVAL for invalid paramenters
> + * -ENODEV for cdev not found
> + * -EEXIST for multiple matches
> + */
> +struct thermal_cooling_device *get_cdev_by_name(const char *name)
> +{
> + int found = 0;
> + struct thermal_cooling_device *pos;
> + struct thermal_cooling_device *cdev = ERR_PTR(-EINVAL);
> +
> + if (!name)
> + return cdev;
> +
> + mutex_lock(&cdev_list_lock);
> + for_each_cdev(pos) {
> + if (!strnicmp(pos->type, name, THERMAL_NAME_LENGTH)) {
> + found++;
> + cdev = pos;
> + }
> + }
> + mutex_unlock(&cdev_list_lock);
> +
> + /*
> + * Nothing has been found; return an error code for it.
> + * Return success only when an unique cdev is found.
> + */
> + if (found == 0)
> + cdev = ERR_PTR(-ENODEV);
> + else if (found > 1)
> + cdev = ERR_PTR(-EEXIST);
> +
> + return cdev;
> +}
> +EXPORT_SYMBOL(get_cdev_by_name);
> +
> +/**
> * get_sensor_by_name() - search for a sensor and returns its reference
> * @name: thermal sensor name to fetch the reference
> *
> @@ -2003,6 +2081,64 @@ exit_zone:
> EXPORT_SYMBOL(add_sensor_to_zone);
>
> /**
> + * add_cdev_to_zone - Add @cdev to thermal zone @tz
> + * @tz: Thermal zone reference
> + * @cdev: Cooling device reference
> + *
> + * Returns 0 on success, otherwise
> + * -EINVAL for invalid paramenters
> + * -EINVAL when trying to add more cdevs than MAX_CDEVS_PER_ZONE
> + * -EEXIST when trying add existing cdev again
> + */
> +int add_cdev_to_zone(struct thermal_zone *tz,
> + struct thermal_cooling_device *cdev)
> +{
> + int ret;
> +
> + if (!tz || !cdev)
> + return -EINVAL;
> +
> + mutex_lock(&zone_list_lock);
> +
> + /* Ensure we are not adding the same cdev again!! */
> + ret = GET_INDEX(tz, cdev, cdev);
> + if (ret >= 0) {
> + ret = -EEXIST;
> + goto exit_zone;
> + }
> +
> + /* Protect against 'cdev' being unregistered */
> + mutex_lock(&cdev_list_lock);
> +
> + ret = sysfs_create_link(&tz->device.kobj, &cdev->device.kobj,
> + kobject_name(&cdev->device.kobj));
> + if (ret)
> + goto exit_cdev;
> +
> + mutex_lock(&tz->lock);
> +
> + if (tz->cdev_indx >= MAX_CDEVS_PER_ZONE - 1) {
> + dev_err(&tz->device, "Only %d cdevs allowed per zone\n",
> + MAX_CDEVS_PER_ZONE);
> + sysfs_remove_link(&tz->device.kobj,
> + kobject_name(&cdev->device.kobj));
> + ret = -EINVAL;
> + goto exit_indx_check;
> + }
> +
> + tz->cdevs[tz->cdev_indx++] = cdev;
> +
> +exit_indx_check:
> + mutex_unlock(&tz->lock);
> +exit_cdev:
> + mutex_unlock(&cdev_list_lock);
> +exit_zone:
> + mutex_unlock(&zone_list_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL(add_cdev_to_zone);
> +
> +/**
> * thermal_sensor_register - register a new thermal sensor
> * @name: name of the thermal sensor
> * @count: Number of thresholds supported by hardware
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 2e12321..da7520c 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -60,6 +60,8 @@
>
> #define MAX_SENSORS_PER_ZONE 5
>
> +#define MAX_CDEVS_PER_ZONE 5
> +
> struct thermal_sensor;
> struct thermal_zone_device;
> struct thermal_cooling_device;
> @@ -223,6 +225,10 @@ struct thermal_zone {
> /* Sensor level information */
> int sensor_indx; /* index into 'sensors' array */
> struct thermal_sensor *sensors[MAX_SENSORS_PER_ZONE];
> +
> + /* cdev level information */
> + int cdev_indx; /* index into 'cdevs' array */
> + struct thermal_cooling_device *cdevs[MAX_CDEVS_PER_ZONE];
> };
>
> /* Structure that holds thermal governor information */
> @@ -300,6 +306,9 @@ 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 *);
>
> +int add_cdev_to_zone(struct thermal_zone *, struct thermal_cooling_device *);
> +struct thermal_cooling_device *get_cdev_by_name(const char *);
> +
> #ifdef CONFIG_NET
> extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,
> enum events event);

2013-10-15 11:01:17

by R, Durgadoss

Subject: RE: [PATCHv4 3/9] Thermal: Create zone level APIs

Hi rui,

> -----Original Message-----
> From: Zhang, Rui
> Sent: Tuesday, October 15, 2013 3:53 PM
> To: R, Durgadoss
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCHv4 3/9] Thermal: Create zone level APIs
>
> On Wed, 2013-10-02 at 00:08 +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 <[email protected]>
> > ---
> > drivers/thermal/thermal_core.c | 267
> +++++++++++++++++++++++++++++++++++++++-
> > include/linux/thermal.h | 23 ++++
> > 2 files changed, 289 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> > index 8c46567..a053b60 100644
> > --- a/drivers/thermal/thermal_core.c
> > +++ b/drivers/thermal/thermal_core.c
> > @@ -44,19 +44,48 @@ MODULE_DESCRIPTION("Generic thermal management
> sysfs support");
> > MODULE_LICENSE("GPL v2");
> >
> > 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(&tz->lock); \
> > + 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); \
> > + mutex_unlock(&tz->lock); \
> > + } while (0); \
> > + ret; \
> > +})
> > +
> > static struct thermal_governor *__find_governor(const char *name)
> > {
> > struct thermal_governor *pos;
> > @@ -461,15 +490,47 @@ 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));
> > +
> > + mutex_lock(&tz->lock);
> > +
> > + /* 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--;
> > + mutex_unlock(&tz->lock);
> > +}
> > +
> > /* 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);
> > @@ -876,6 +937,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)
> > @@ -1689,7 +1752,7 @@ static void remove_trip_attrs(struct
> thermal_zone_device *tz)
> > kfree(tz->trip_hyst_attrs);
> > }
> >
> > - /**
> > +/**
> > * enable_sensor_thresholds - create sysfs nodes for thresholdX
> > * @ts: the thermal sensor
> > * @count: Number of thresholds supported by sensor hardware
> > @@ -1746,6 +1809,200 @@ static int enable_sensor_thresholds(struct
> thermal_sensor *ts, int count)
> > }
> >
> > /**
> > + * create_thermal_zone - create sysfs nodes for thermal zone
> > + * @name: Name of the thermla zone
> > + * @devdata: Data supplied by the caller
> > + *
> > + * On Success returns a thermal zone reference, otherwise:
> > + * -EINVAL for invalid parameters,
> > + * -ENOMEM for insufficient memory cases,
> > + */
> > +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;
> > +
> > + mutex_init(&tz->lock);
> > + strlcpy(tz->name, name, sizeof(tz->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:
> > + mutex_destroy(&tz->lock);
> > + release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
> > +exit_free:
> > + kfree(tz);
> > + return ERR_PTR(ret);
> > +}
> > +EXPORT_SYMBOL(create_thermal_zone);
> > +
> as this is a thermal subsystem API, I'd prefer to use the names starting
> with thermal_xxx.

So, can we name thermal_create_thermal_zone ?
And we will do similar thing for all EXPORT_SYMBOL APIs.

>
> > +/**
> > + * remove_thermal_zone - removes the sysfs nodes for given tz
> > + * @tz: Thermal zone to be removed.
> > + */
> > +void remove_thermal_zone(struct thermal_zone *tz)
> > +{
> > + struct thermal_zone *pos, *next;
> > + struct thermal_sensor *ts;
> > + 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;
> > +
> > + for_each_thermal_sensor(ts)
> > + remove_sensor_from_zone(tz, ts);
> > +
> > + device_remove_file(&tz->device, &dev_attr_zone_name);
> > +
> > + mutex_destroy(&tz->lock);
> > + 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);
> > +
> ditto
>
> > +/**
> > + * get_sensor_by_name() - search for a sensor and returns its reference
> > + * @name: thermal sensor name to fetch the reference
> > + *
> > + * On success returns a reference to a unique sensor with
> > + * name matching that of @name, an ERR_PTR otherwise:
> > + * -EINVAL for invalid paramenters
> > + * -ENODEV for sensor not found
> > + * -EEXIST for multiple matches
> > + */
> > +struct thermal_sensor *get_sensor_by_name(const char *name)
> > +{
> > + int found = 0;
> > + struct thermal_sensor *pos;
> > + struct thermal_sensor *ts = ERR_PTR(-EINVAL);
> > +
> > + if (!name)
> > + return ts;
> > +
> > + mutex_lock(&sensor_list_lock);
> > + for_each_thermal_sensor(pos) {
> > + if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) {
> > + found++;
> > + ts = pos;
> > + }
> > + }
> > + mutex_unlock(&sensor_list_lock);
> > +
> > + if (found == 0)
> > + ts = ERR_PTR(-ENODEV);
> > + else if (found > 1)
> > + ts = ERR_PTR(-EEXIST);
> > +
> > + return ts;
> > +}
> > +EXPORT_SYMBOL(get_sensor_by_name);
> > +
> ditto
>
> > +/**
> > + * add_sensor_to_zone - Add @ts to thermal zone @tz
> > + * @tz: Thermal zone reference
> > + * @ts: Thermal sensor reference
> > + *
> > + * Returns 0 on success, otherwise
> > + * -EINVAL for invalid paramenters
> > + * -EINVAL when trying to add more zones than MAX_SENSORS_PER_ZONE
> > + * -EEXIST when trying add existing thermal sensor again
> > + */
> > +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;
> > + }
> > +
> > + /* Protect against 'ts' being unregistered */
> > + 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;
> > +
> > + mutex_lock(&tz->lock);
> > +
> > + if (tz->sensor_indx >= MAX_SENSORS_PER_ZONE - 1) {
> > + dev_err(&tz->device, "Only %d sensors allowed per zone\n",
> > + MAX_SENSORS_PER_ZONE);
> > + sysfs_remove_link(&tz->device.kobj,
> > + kobject_name(&ts->device.kobj));
> > + ret = -EINVAL;
> > + goto exit_indx_check;
> > + }
> > +
> > + tz->sensors[tz->sensor_indx++] = ts;
> > +
> > +exit_indx_check:
> > + mutex_unlock(&tz->lock);
> > +exit_sensor:
> > + mutex_unlock(&sensor_list_lock);
> > +exit_zone:
> > + mutex_unlock(&zone_list_lock);
> > + return ret;
> > +}
> > +EXPORT_SYMBOL(add_sensor_to_zone);
> > +
> ditto
>
> > +/**
> > * thermal_sensor_register - register a new thermal sensor
> > * @name: name of the thermal sensor
> > * @count: Number of thresholds supported by hardware
> > @@ -1829,6 +2086,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;
> >
> > @@ -1848,6 +2106,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);
> > +
> I do not see the code to bind the sensors to zone in this patch, and I
> guess we support manually binding only, via explicitly calling () in
> platform thermal driver, at the moment, right?

Yes, you are right.

Thanks,
Durga

>
> thanks,
> rui
> > 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 25fc562..2e12321 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -58,6 +58,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;
> > @@ -207,6 +209,22 @@ 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 mutex lock; /* protect elements of this structure */
> > + 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];
> > @@ -277,6 +295,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);
>

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2013-10-15 11:03:34

by Zhang, Rui

Subject: Re: [PATCHv4 5/9] Thermal: Add trip point sysfs nodes for sensor

On Wed, 2013-10-02 at 00:08 +0530, Durgadoss R wrote:
> This patch adds a trip point related sysfs nodes
> for each sensor under a zone in /sys/class/thermal/zoneX/.
> The nodes will be named, sensorX_trip_activeY,
> sensorX_trip_passiveY, sensorX_trip_hot, sensorX_trip_critical
> for active, passive, hot and critical trip points
> respectively for sensorX.
>
> Signed-off-by: Durgadoss R <[email protected]>
> ---
> drivers/thermal/thermal_core.c | 344 +++++++++++++++++++++++++++++++++++++++-
> include/linux/thermal.h | 40 ++++-
> 2 files changed, 379 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 3c4ef62..d6e29f6 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -494,6 +494,60 @@ static void thermal_zone_device_check(struct work_struct *work)
> thermal_zone_device_update(tz);
> }
>
> +static int get_sensor_indx_by_kobj(struct thermal_zone *tz, const char *name)
> +{
> + int i, indx = -EINVAL;
> +
> + /* Protect against tz->sensors[i] being unregistered */
> + mutex_lock(&sensor_list_lock);
> +
> + for (i = 0; i < tz->sensor_indx; i++) {
> + if (!strnicmp(name, kobject_name(&tz->sensors[i]->device.kobj),
> + THERMAL_NAME_LENGTH)) {
> + indx = i;
> + break;
> + }
> + }
> +
> + mutex_unlock(&sensor_list_lock);
> + return indx;
> +}
> +
> +static void __remove_trip_attr(struct thermal_zone *tz, int indx)
> +{
> + int i;
> + struct thermal_trip_attr *attr = tz->trip_attr[indx];
> + struct thermal_trip_point *trip = tz->sensor_trip[indx];
> +
> + if (!attr || !trip)
> + return;
> +
> + if (trip->crit != THERMAL_TRIPS_NONE)
> + device_remove_file(&tz->device, &attr->crit_attr.attr);
> +
> + if (trip->hot != THERMAL_TRIPS_NONE)
> + device_remove_file(&tz->device, &attr->hot_attr.attr);
> +
> + if (trip->num_passive_trips > 0) {
> + for (i = 0; i < trip->num_passive_trips; i++) {
> + device_remove_file(&tz->device,
> + &attr->passive_attrs[i].attr);
> + }
> + kfree(attr->passive_attrs);
> + }
> +
> + if (trip->num_active_trips > 0) {
> + for (i = 0; i < trip->num_active_trips; i++) {
> + device_remove_file(&tz->device,
> + &attr->active_attrs[i].attr);
> + }
> + kfree(attr->active_attrs);
> + }
> +
> + kfree(tz->trip_attr[indx]);
> + tz->trip_attr[indx] = NULL;
> +}
> +
> static void remove_sensor_from_zone(struct thermal_zone *tz,
> struct thermal_sensor *ts)
> {
> @@ -503,13 +557,19 @@ static void remove_sensor_from_zone(struct thermal_zone *tz,
> if (indx < 0)
> return;
>
> - sysfs_remove_link(&tz->device.kobj, kobject_name(&ts->device.kobj));
> -
> mutex_lock(&tz->lock);
>
> + /* Remove trip point attributes associated with this sensor */
> + __remove_trip_attr(tz, indx);
> +
> + 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++)
> + for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++) {
> tz->sensors[j] = tz->sensors[j + 1];
> + tz->sensor_trip[j] = tz->sensor_trip[j + 1];
> + tz->trip_attr[j] = tz->trip_attr[j + 1];
> + }
>
> tz->sensor_indx--;
> mutex_unlock(&tz->lock);
> @@ -952,6 +1012,111 @@ emul_temp_store(struct device *dev, struct device_attribute *attr,
> static DEVICE_ATTR(emul_temp, S_IWUSR, NULL, emul_temp_store);
> #endif/*CONFIG_THERMAL_EMULATION*/
>
> +static ssize_t
> +active_trip_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + int i, j, val;
> + char kobj_name[THERMAL_NAME_LENGTH];
> + struct thermal_zone *tz = to_zone(dev);
> +
> + if (!sscanf(attr->attr.name, "sensor%d_trip_active%d", &i, &j))
> + return -EINVAL;
> +
> + snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", i);
> +
> + mutex_lock(&tz->lock);
> +
> + i = get_sensor_indx_by_kobj(tz, kobj_name);
> + if (i < 0) {
> + mutex_unlock(&tz->lock);
> + return i;
> + }
> +
> + val = tz->sensor_trip[i]->active_trips[j];
> + mutex_unlock(&tz->lock);
> +
> + return sprintf(buf, "%d\n", val);
> +}
> +
> +static ssize_t
> +passive_trip_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + int i, j, val;
> + char kobj_name[THERMAL_NAME_LENGTH];
> + struct thermal_zone *tz = to_zone(dev);
> +
> + if (!sscanf(attr->attr.name, "sensor%d_trip_passive%d", &i, &j))
> + return -EINVAL;
> +
> + snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", i);
> +
> + mutex_lock(&tz->lock);
> +
> + i = get_sensor_indx_by_kobj(tz, kobj_name);
> + if (i < 0) {
> + mutex_unlock(&tz->lock);
> + return i;
> + }
> +
> + val = tz->sensor_trip[i]->passive_trips[j];
> + mutex_unlock(&tz->lock);
> +
> + return sprintf(buf, "%d\n", val);
> +}
> +
> +static ssize_t
> +hot_trip_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + int indx, val;
> + char kobj_name[THERMAL_NAME_LENGTH];
> + struct thermal_zone *tz = to_zone(dev);
> +
> + if (!sscanf(attr->attr.name, "sensor%d_trip_hot", &indx))
> + return -EINVAL;
> +
> + snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", indx);
> +
> + mutex_lock(&tz->lock);
> +
> + indx = get_sensor_indx_by_kobj(tz, kobj_name);
> + if (indx < 0) {
> + mutex_unlock(&tz->lock);
> + return indx;
> + }
> +
> + val = tz->sensor_trip[indx]->hot;
> + mutex_unlock(&tz->lock);
> +
> + return sprintf(buf, "%d\n", val);
> +}
> +
> +static ssize_t
> +critical_trip_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + int indx, val;
> + char kobj_name[THERMAL_NAME_LENGTH];
> + struct thermal_zone *tz = to_zone(dev);
> +
> + if (!sscanf(attr->attr.name, "sensor%d_trip_critical", &indx))
> + return -EINVAL;
> +
> + snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", indx);
> +
> + mutex_lock(&tz->lock);
> +
> + indx = get_sensor_indx_by_kobj(tz, kobj_name);
> + if (indx < 0) {
> + mutex_unlock(&tz->lock);
> + return indx;
> + }
> +
> + val = tz->sensor_trip[indx]->crit;
> + mutex_unlock(&tz->lock);
> +
> + return sprintf(buf, "%d\n", val);
> +}
> +
> static DEVICE_ATTR(type, 0444, type_show, NULL);
> static DEVICE_ATTR(temp, 0444, temp_show, NULL);
> static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
> @@ -962,7 +1127,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);
> +/* Thermal zone attributes */
> +static DEVICE_ATTR(zone_name, S_IRUGO, zone_name_show, NULL);
>
> /* sys I/F for cooling device */
> #define to_cooling_device(_dev) \
> @@ -1841,6 +2007,43 @@ static int enable_sensor_thresholds(struct thermal_sensor *ts, int count)
> return 0;
> }
>
> +static int create_single_trip_attr(struct thermal_zone *tz,
> + struct thermal_attr *attr,
> + const char *attr_name,
> + ssize_t (*rd_ptr)(struct device *dev,
> + struct device_attribute *devattr, char *buf))
> +{
> + snprintf(attr->name, THERMAL_NAME_LENGTH, attr_name);
> + sysfs_attr_init(&attr->attr.attr);
> + attr->attr.attr.name = attr->name;
> + attr->attr.attr.mode = S_IRUGO;
> + attr->attr.show = rd_ptr;
> + return device_create_file(&tz->device, &attr->attr);
> +}
> +
> +static int create_multi_trip_attrs(struct thermal_zone *tz, int size,
> + int indx, struct thermal_attr *attrs,
> + const char *attr_name,
> + ssize_t (*rd_ptr)(struct device *dev,
> + struct device_attribute *devattr, char *buf))
> +{
> + char name[THERMAL_NAME_LENGTH];
> + int i, ret;
> +
> + for (i = 0; i < size; i++) {
> + snprintf(name, THERMAL_NAME_LENGTH, attr_name, indx, i);
> + ret = create_single_trip_attr(tz, &attrs[i], name, rd_ptr);
> + if (ret)
> + goto exit;
> + }
> + return 0;
> +
> +exit:
> + while (--i >= 0)
> + device_remove_file(&tz->device, &attrs[i].attr);
> + return ret;
> +}
> +
> /**
> * create_thermal_zone - create sysfs nodes for thermal zone
> * @name: Name of the thermla zone
> @@ -2139,6 +2342,139 @@ exit_zone:
> EXPORT_SYMBOL(add_cdev_to_zone);
>
> /**
> + * add_sensor_trip_info - Add trip point information for @ts in @tz
> + * @tz: Thermal zone reference
> + * @ts: Thermal sensor reference
> + * @trip: Trip point structure reference
> + *
> + * Returns 0 on success, otherwise
> + * -EINVAL for invalid paramenters
> + * -EINVAL if @ts is not part of 'this' thermal zone @tz
> + * -ENOMEM on kzalloc failures
> + */
> +int add_sensor_trip_info(struct thermal_zone *tz, struct thermal_sensor *ts,
> + struct thermal_trip_point *trip)
> +{
> + char name[THERMAL_NAME_LENGTH];
> + int i, indx, kobj_indx, ret, size;
> + struct thermal_trip_attr *attrs;
> +
> + if (!tz || !ts || !trip)
> + return -EINVAL;
> +
> + if (!sscanf(kobject_name(&ts->device.kobj), "sensor%d", &kobj_indx))
> + return -EINVAL;
> +
> + mutex_lock(&zone_list_lock);
> +
> + indx = GET_INDEX(tz, ts, sensor);
> + if (indx < 0) {
> + ret = -EINVAL;
> + goto exit;
> + }
> +
> + /* Protect against 'ts' being unregistered */
> + mutex_lock(&sensor_list_lock);
> +
> + /* Protect tz->trip_attr[] and tz->sensor_trip[] */
> + mutex_lock(&tz->lock);
> +
> + tz->trip_attr[indx] = kzalloc(sizeof(struct thermal_trip_attr),
> + GFP_KERNEL);
> + if (!tz->trip_attr[indx]) {
> + ret = -ENOMEM;
> + goto exit_lock;
> + }
> +
> + attrs = tz->trip_attr[indx];
> +
> + /* Create Critical trip point attribute */
> + if (trip->crit != THERMAL_TRIPS_NONE) {
> + snprintf(name, THERMAL_NAME_LENGTH,
> + "sensor%d_trip_critical", kobj_indx);
> + ret = create_single_trip_attr(tz, &attrs->crit_attr,
> + name, critical_trip_show);
> + if (ret)
> + goto exit_trip;
> + }
> +
> + /* Create Hot trip point attribute */
> + if (trip->hot != THERMAL_TRIPS_NONE) {
> + snprintf(name, THERMAL_NAME_LENGTH,
> + "sensor%d_trip_hot", kobj_indx);
> + ret = create_single_trip_attr(tz, &attrs->hot_attr,
> + name, hot_trip_show);
> + if (ret)
> + goto exit_crit_trip;
> + }
> +
> + /* Create Passive trip point attributes */
> + if (trip->num_passive_trips > 0) {
> + size = sizeof(struct thermal_attr) * trip->num_passive_trips;
> + attrs->passive_attrs = kzalloc(size, GFP_KERNEL);
> + if (!attrs->passive_attrs) {
> + ret = -ENOMEM;
> + goto exit_hot_trip;
> + }
> +
> + ret = create_multi_trip_attrs(tz, trip->num_passive_trips,
> + kobj_indx, attrs->passive_attrs,
> + "sensor%d_trip_passive%d",

well, I do not think this is a good code style.
I prefer to create the attrs one by one, rather than passing this ugly
format string.
please use create_single_trip_attr() instead if you can not find a clean
way to do this.

thanks,
rui
> + passive_trip_show);
> + if (ret)
> + goto exit_hot_trip;
> + }
> +
> + /* Create Active trip point attributes */
> + if (trip->num_active_trips > 0) {
> + size = sizeof(struct thermal_attr) * trip->num_active_trips;
> + attrs->active_attrs = kzalloc(size, GFP_KERNEL);
> + if (!attrs->active_attrs) {
> + ret = -ENOMEM;
> + goto exit_passive_trips;
> + }
> +
> + ret = create_multi_trip_attrs(tz, trip->num_active_trips,
> + kobj_indx, attrs->active_attrs,
> + "sensor%d_trip_active%d",
> + active_trip_show);
> + if (ret)
> + goto exit_passive_trips;
> + }
> +
> + tz->sensor_trip[indx] = trip;
> +
> + mutex_unlock(&tz->lock);
> + mutex_unlock(&sensor_list_lock);
> + mutex_unlock(&zone_list_lock);
> +
> + return 0;
> +
> +exit_passive_trips:
> + kfree(attrs->active_attrs);
> + i = trip->num_passive_trips;
> + while (--i >= 0)
> + device_remove_file(&tz->device, &attrs->passive_attrs[i].attr);
> +exit_hot_trip:
> + kfree(attrs->passive_attrs);
> + if (trip->hot != THERMAL_TRIPS_NONE)
> + device_remove_file(&tz->device, &attrs->hot_attr.attr);
> +exit_crit_trip:
> + if (trip->crit != THERMAL_TRIPS_NONE)
> + device_remove_file(&tz->device, &attrs->crit_attr.attr);
> +exit_trip:
> + kfree(tz->trip_attr[indx]);
> + tz->trip_attr[indx] = NULL;
> +exit_lock:
> + mutex_unlock(&tz->lock);
> + mutex_unlock(&sensor_list_lock);
> +exit:
> + mutex_unlock(&zone_list_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL(add_sensor_trip_info);
> +
> +/**
> * thermal_sensor_register - register a new thermal sensor
> * @name: name of the thermal sensor
> * @count: Number of thresholds supported by hardware
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index da7520c..f8de86d 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -31,7 +31,7 @@
>
> #define THERMAL_TRIPS_NONE -1
> #define THERMAL_MAX_TRIPS 12
> -#define THERMAL_NAME_LENGTH 20
> +#define THERMAL_NAME_LENGTH 25
>
> /* invalid cooling state */
> #define THERMAL_CSTATE_INVALID -1UL
> @@ -170,6 +170,37 @@ struct thermal_attr {
> char name[THERMAL_NAME_LENGTH];
> };
>
> +/*
> + * This structure defines the trip points for a sensor.
> + * The actual values for these trip points come from
> + * platform characterization. The thermal governors
> + * (either kernel or user space) may take appropriate
> + * actions when the sensors reach these trip points.
> + * See Documentation/thermal/sysfs-api2.txt for more details.
> + *
> + * As of now, For a particular sensor, we support:
> + * a) 1 hot trip point
> + * b) 1 critical trip point
> + * c) 'n' passive trip points
> + * d) 'm' active trip points
> + */
> +struct thermal_trip_point {
> + int hot;
> + int crit;
> + int num_passive_trips;
> + int *passive_trips;
> + int num_active_trips;
> + int *active_trips;
> + int active_trip_mask;
> +};
> +
> +struct thermal_trip_attr {
> + struct thermal_attr hot_attr;
> + struct thermal_attr crit_attr;
> + struct thermal_attr *active_attrs;
> + struct thermal_attr *passive_attrs;
> +};
> +
> struct thermal_sensor {
> char name[THERMAL_NAME_LENGTH];
> int id;
> @@ -229,6 +260,10 @@ struct thermal_zone {
> /* cdev level information */
> int cdev_indx; /* index into 'cdevs' array */
> struct thermal_cooling_device *cdevs[MAX_CDEVS_PER_ZONE];
> +
> + /* Thermal sensors trip information */
> + struct thermal_trip_point *sensor_trip[MAX_SENSORS_PER_ZONE];
> + struct thermal_trip_attr *trip_attr[MAX_SENSORS_PER_ZONE];
> };
>
> /* Structure that holds thermal governor information */
> @@ -309,6 +344,9 @@ struct thermal_sensor *get_sensor_by_name(const char *);
> int add_cdev_to_zone(struct thermal_zone *, struct thermal_cooling_device *);
> struct thermal_cooling_device *get_cdev_by_name(const char *);
>
> +int add_sensor_trip_info(struct thermal_zone *, struct thermal_sensor *,
> + struct thermal_trip_point *);
> +
> #ifdef CONFIG_NET
> extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,
> enum events event);

2013-10-15 11:22:20

by Zhang, Rui

Subject: Re: [PATCHv4 0/9] Thermal Framework Enhancements

Hi, Durga,

On Wed, 2013-10-02 at 00:07 +0530, Durgadoss R wrote:
> This patch set is a v4 of the previous versions submitted here:
> [v3]: https://lkml.org/lkml/2013/2/5/228
> [v2]: http://lwn.net/Articles/531720/
> [v1]: https://lkml.org/lkml/2012/12/18/108
> [RFC]:https://patchwork.kernel.org/patch/1758921/
>
> This patch set is based on Rui's -next tree, on top
> of commit 'f61d5b4d52e077756ce9dbc47ce737da898ad01d'
> This is tested on a Core-i5 and an Atom netbook,
> running ubuntu 12.04.
>
> Changes since v3:
> * Added a patch to conditionally do kfree(cdev) in
> thermal_release function.
> * Reworked all sysfs attributes to have one value per file
> This includes sensor_trip_* and map_weight* attributes.
> * Added 'lock' variable in thermal_zone structure to
> protect its members.
> * Added Documentation to all functions in thermal_core.c
> * Changes all strcpy() to strlcpy()
> * Used devm_kzalloc() in places where applicable
> * Address some buffer overflow conditions and contentions
> in tz->sensors[] and tz->cdevs[].
> Changes since v2:
> * Reworked the map sysfs attributes in patch [5/8]
> * Dropped configuration for maximum sensors and
> cooling devices, through Kconfig.
> * Added __remove_trip_attr method
> * Renamed __clean_map_entry to __remove_map_entry
> for consistency in naming.
> Changes Since v1:
> * Removed kobject creation for thermal_trip and thermal_map
> nodes as per Greg-KH's comments.
> * Added ABI Documentation under 'testing'.
> * Modified the GET_INDEX macro to be more linux-like, thanks
> to Joe Perches.
> * Added get_[sensor/cdev]_by_name APIs to thermal.h
>
> This series contains 9 patches:
> Patch 1/9: Fixes a kfree issue. This is required so that the new sensor
> and zone devices are not freed accidently.
> We can do two things here:
> 1) Conditionally free every device
> 2) Remove this _release function, and free the memory
> in corresponding _unregister functions.
> I prefer 2) and we can have this as a separate patch
> outside this series; but would like to see the opinion
> of maintainers'.
this is a general fix that I will push it in next merge window.

> Patch 2/9: Creates new sensor level APIs
> Patch 3/9: Creates new zone level APIs. The existing tzd structure is
> kept as such for clarity and compatibility purposes.
> Patch 4/9: Creates functions to add/remove a cdev to/from a zone. The
> existing tcd structure need not be modified.
> Patch 5/9: Adds sensorX_trip_[active/passive/hot/critical] sysfs nodes,
> under /sys/class/thermal/zoneY/. This exposes various trip
> points for sensorX present in zoneY.
> Patch 6/9: Adds mapY_* sysfs node. These attributes represent

After reading these patch, here are some questions,
1. this patch set introduces a new thermal sysfs I/F layout, but in
fact, the thermal core can do nothing except exposing those interfaces.
Say, no thermal policy is supported in the new hierarchy, i.e. we can
not take any cooling action from kernel side, right?
2. as we are introducing three new devices, and these devices will not
invoke the existing thermal core functions. Then there are few things
left that can be shared between the current thermal model and your new
one. Thus, can we have a separate .c file for the new code only?

I'm not sure if you have time to do this, if yes, can you please rebase
the patch set and send it out ASAP, if no, I can work on this stuff and
re-base your patches. what do you think?

thanks,
rui
> Patch 7/9: Creates Documentation for the new APIs. A new file is
> created for clarity. Final goal is to merge with the existing
> file or refactor the files, as whatever seems appropriate.
> Patch 8/9: Add ABI documentation for sysfs interfaces introduced in this patch.
> Patch 9/9: A dummy driver that can be used for testing. This is not for merge.
>
> Sorry for the long delay in resubmitting this patch set.
> Thanks to Eduardo, Wei Ni, for their comments on v3.
>
> Durgadoss R (9):
> Thermal: Check for validity before doing kfree
> Thermal: Create sensor level APIs
> Thermal: Create zone level APIs
> Thermal: Add APIs to bind cdev to new zone structure
> Thermal: Add trip point sysfs nodes for sensor
> Thermal: Create Thermal map sysfs attributes for a zone
> Thermal: Add Documentation to new APIs
> Thermal: Add ABI Documentation for sysfs interfaces
> Thermal: Dummy driver used for testing
>
> Documentation/ABI/testing/sysfs-class-thermal | 137 +++
> Documentation/thermal/sysfs-api2.txt | 248 +++++
> drivers/thermal/Kconfig | 5 +
> drivers/thermal/Makefile | 3 +
> drivers/thermal/thermal_core.c | 1406 +++++++++++++++++++++++--
> drivers/thermal/thermal_test.c | 322 ++++++
> include/linux/thermal.h | 127 ++-
> 7 files changed, 2180 insertions(+), 68 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-class-thermal
> create mode 100644 Documentation/thermal/sysfs-api2.txt
> create mode 100644 drivers/thermal/thermal_test.c
>

2013-10-15 13:19:15

by R, Durgadoss

Subject: RE: [PATCHv4 5/9] Thermal: Add trip point sysfs nodes for sensor

> -----Original Message-----
> From: Zhang, Rui
> Sent: Tuesday, October 15, 2013 4:33 PM
> To: R, Durgadoss
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCHv4 5/9] Thermal: Add trip point sysfs nodes for sensor
>
> On Wed, 2013-10-02 at 00:08 +0530, Durgadoss R wrote:
> > This patch adds a trip point related sysfs nodes
> > for each sensor under a zone in /sys/class/thermal/zoneX/.
> > The nodes will be named, sensorX_trip_activeY,
> > sensorX_trip_passiveY, sensorX_trip_hot, sensorX_trip_critical
> > for active, passive, hot and critical trip points
> > respectively for sensorX.
> >
> > Signed-off-by: Durgadoss R <[email protected]>
> > ---
> > drivers/thermal/thermal_core.c | 344
> +++++++++++++++++++++++++++++++++++++++-
> > include/linux/thermal.h | 40 ++++-
> > 2 files changed, 379 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> > index 3c4ef62..d6e29f6 100644
> > --- a/drivers/thermal/thermal_core.c
> > +++ b/drivers/thermal/thermal_core.c
> > @@ -494,6 +494,60 @@ static void thermal_zone_device_check(struct
> work_struct *work)
> > thermal_zone_device_update(tz);
> > }
> >
> > +static int get_sensor_indx_by_kobj(struct thermal_zone *tz, const char
> *name)
> > +{
> > + int i, indx = -EINVAL;
> > +
> > + /* Protect against tz->sensors[i] being unregistered */
> > + mutex_lock(&sensor_list_lock);
> > +
> > + for (i = 0; i < tz->sensor_indx; i++) {
> > + if (!strnicmp(name, kobject_name(&tz->sensors[i]-
> >device.kobj),
> > + THERMAL_NAME_LENGTH)) {
> > + indx = i;
> > + break;
> > + }
> > + }
> > +
> > + mutex_unlock(&sensor_list_lock);
> > + return indx;
> > +}
> > +
> > +static void __remove_trip_attr(struct thermal_zone *tz, int indx)
> > +{
> > + int i;
> > + struct thermal_trip_attr *attr = tz->trip_attr[indx];
> > + struct thermal_trip_point *trip = tz->sensor_trip[indx];
> > +
> > + if (!attr || !trip)
> > + return;
> > +
> > + if (trip->crit != THERMAL_TRIPS_NONE)
> > + device_remove_file(&tz->device, &attr->crit_attr.attr);
> > +
> > + if (trip->hot != THERMAL_TRIPS_NONE)
> > + device_remove_file(&tz->device, &attr->hot_attr.attr);
> > +
> > + if (trip->num_passive_trips > 0) {
> > + for (i = 0; i < trip->num_passive_trips; i++) {
> > + device_remove_file(&tz->device,
> > + &attr->passive_attrs[i].attr);
> > + }
> > + kfree(attr->passive_attrs);
> > + }
> > +
> > + if (trip->num_active_trips > 0) {
> > + for (i = 0; i < trip->num_active_trips; i++) {
> > + device_remove_file(&tz->device,
> > + &attr->active_attrs[i].attr);
> > + }
> > + kfree(attr->active_attrs);
> > + }
> > +
> > + kfree(tz->trip_attr[indx]);
> > + tz->trip_attr[indx] = NULL;
> > +}
> > +
> > static void remove_sensor_from_zone(struct thermal_zone *tz,
> > struct thermal_sensor *ts)
> > {
> > @@ -503,13 +557,19 @@ static void remove_sensor_from_zone(struct
> thermal_zone *tz,
> > if (indx < 0)
> > return;
> >
> > - sysfs_remove_link(&tz->device.kobj, kobject_name(&ts->device.kobj));
> > -
> > mutex_lock(&tz->lock);
> >
> > + /* Remove trip point attributes associated with this sensor */
> > + __remove_trip_attr(tz, indx);
> > +
> > + 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++)
> > + for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++) {
> > tz->sensors[j] = tz->sensors[j + 1];
> > + tz->sensor_trip[j] = tz->sensor_trip[j + 1];
> > + tz->trip_attr[j] = tz->trip_attr[j + 1];
> > + }
> >
> > tz->sensor_indx--;
> > mutex_unlock(&tz->lock);
> > @@ -952,6 +1012,111 @@ emul_temp_store(struct device *dev, struct
> device_attribute *attr,
> > static DEVICE_ATTR(emul_temp, S_IWUSR, NULL, emul_temp_store);
> > #endif/*CONFIG_THERMAL_EMULATION*/
> >
> > +static ssize_t
> > +active_trip_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > + int i, j, val;
> > + char kobj_name[THERMAL_NAME_LENGTH];
> > + struct thermal_zone *tz = to_zone(dev);
> > +
> > + if (!sscanf(attr->attr.name, "sensor%d_trip_active%d", &i, &j))
> > + return -EINVAL;
> > +
> > + snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", i);
> > +
> > + mutex_lock(&tz->lock);
> > +
> > + i = get_sensor_indx_by_kobj(tz, kobj_name);
> > + if (i < 0) {
> > + mutex_unlock(&tz->lock);
> > + return i;
> > + }
> > +
> > + val = tz->sensor_trip[i]->active_trips[j];
> > + mutex_unlock(&tz->lock);
> > +
> > + return sprintf(buf, "%d\n", val);
> > +}
> > +
> > +static ssize_t
> > +passive_trip_show(struct device *dev, struct device_attribute *attr, char
> *buf)
> > +{
> > + int i, j, val;
> > + char kobj_name[THERMAL_NAME_LENGTH];
> > + struct thermal_zone *tz = to_zone(dev);
> > +
> > + if (!sscanf(attr->attr.name, "sensor%d_trip_passive%d", &i, &j))
> > + return -EINVAL;
> > +
> > + snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", i);
> > +
> > + mutex_lock(&tz->lock);
> > +
> > + i = get_sensor_indx_by_kobj(tz, kobj_name);
> > + if (i < 0) {
> > + mutex_unlock(&tz->lock);
> > + return i;
> > + }
> > +
> > + val = tz->sensor_trip[i]->passive_trips[j];
> > + mutex_unlock(&tz->lock);
> > +
> > + return sprintf(buf, "%d\n", val);
> > +}
> > +
> > +static ssize_t
> > +hot_trip_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > + int indx, val;
> > + char kobj_name[THERMAL_NAME_LENGTH];
> > + struct thermal_zone *tz = to_zone(dev);
> > +
> > + if (!sscanf(attr->attr.name, "sensor%d_trip_hot", &indx))
> > + return -EINVAL;
> > +
> > + snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", indx);
> > +
> > + mutex_lock(&tz->lock);
> > +
> > + indx = get_sensor_indx_by_kobj(tz, kobj_name);
> > + if (indx < 0) {
> > + mutex_unlock(&tz->lock);
> > + return indx;
> > + }
> > +
> > + val = tz->sensor_trip[indx]->hot;
> > + mutex_unlock(&tz->lock);
> > +
> > + return sprintf(buf, "%d\n", val);
> > +}
> > +
> > +static ssize_t
> > +critical_trip_show(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + int indx, val;
> > + char kobj_name[THERMAL_NAME_LENGTH];
> > + struct thermal_zone *tz = to_zone(dev);
> > +
> > + if (!sscanf(attr->attr.name, "sensor%d_trip_critical", &indx))
> > + return -EINVAL;
> > +
> > + snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", indx);
> > +
> > + mutex_lock(&tz->lock);
> > +
> > + indx = get_sensor_indx_by_kobj(tz, kobj_name);
> > + if (indx < 0) {
> > + mutex_unlock(&tz->lock);
> > + return indx;
> > + }
> > +
> > + val = tz->sensor_trip[indx]->crit;
> > + mutex_unlock(&tz->lock);
> > +
> > + return sprintf(buf, "%d\n", val);
> > +}
> > +
> > static DEVICE_ATTR(type, 0444, type_show, NULL);
> > static DEVICE_ATTR(temp, 0444, temp_show, NULL);
> > static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
> > @@ -962,7 +1127,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);
> > +/* Thermal zone attributes */
> > +static DEVICE_ATTR(zone_name, S_IRUGO, zone_name_show, NULL);
> >
> > /* sys I/F for cooling device */
> > #define to_cooling_device(_dev) \
> > @@ -1841,6 +2007,43 @@ static int enable_sensor_thresholds(struct
> thermal_sensor *ts, int count)
> > return 0;
> > }
> >
> > +static int create_single_trip_attr(struct thermal_zone *tz,
> > + struct thermal_attr *attr,
> > + const char *attr_name,
> > + ssize_t (*rd_ptr)(struct device *dev,
> > + struct device_attribute *devattr, char *buf))
> > +{
> > + snprintf(attr->name, THERMAL_NAME_LENGTH, attr_name);
> > + sysfs_attr_init(&attr->attr.attr);
> > + attr->attr.attr.name = attr->name;
> > + attr->attr.attr.mode = S_IRUGO;
> > + attr->attr.show = rd_ptr;
> > + return device_create_file(&tz->device, &attr->attr);
> > +}
> > +
> > +static int create_multi_trip_attrs(struct thermal_zone *tz, int size,
> > + int indx, struct thermal_attr *attrs,
> > + const char *attr_name,
> > + ssize_t (*rd_ptr)(struct device *dev,
> > + struct device_attribute *devattr, char *buf))
> > +{
> > + char name[THERMAL_NAME_LENGTH];
> > + int i, ret;
> > +
> > + for (i = 0; i < size; i++) {
> > + snprintf(name, THERMAL_NAME_LENGTH, attr_name, indx, i);
> > + ret = create_single_trip_attr(tz, &attrs[i], name, rd_ptr);
> > + if (ret)
> > + goto exit;
> > + }
> > + return 0;
> > +
> > +exit:
> > + while (--i >= 0)
> > + device_remove_file(&tz->device, &attrs[i].attr);
> > + return ret;
> > +}
> > +
> > /**
> > * create_thermal_zone - create sysfs nodes for thermal zone
> > * @name: Name of the thermla zone
> > @@ -2139,6 +2342,139 @@ exit_zone:
> > EXPORT_SYMBOL(add_cdev_to_zone);
> >
> > /**
> > + * add_sensor_trip_info - Add trip point information for @ts in @tz
> > + * @tz: Thermal zone reference
> > + * @ts: Thermal sensor reference
> > + * @trip: Trip point structure reference
> > + *
> > + * Returns 0 on success, otherwise
> > + * -EINVAL for invalid paramenters
> > + * -EINVAL if @ts is not part of 'this' thermal zone @tz
> > + * -ENOMEM on kzalloc failures
> > + */
> > +int add_sensor_trip_info(struct thermal_zone *tz, struct thermal_sensor *ts,
> > + struct thermal_trip_point *trip)
> > +{
> > + char name[THERMAL_NAME_LENGTH];
> > + int i, indx, kobj_indx, ret, size;
> > + struct thermal_trip_attr *attrs;
> > +
> > + if (!tz || !ts || !trip)
> > + return -EINVAL;
> > +
> > + if (!sscanf(kobject_name(&ts->device.kobj), "sensor%d", &kobj_indx))
> > + return -EINVAL;
> > +
> > + mutex_lock(&zone_list_lock);
> > +
> > + indx = GET_INDEX(tz, ts, sensor);
> > + if (indx < 0) {
> > + ret = -EINVAL;
> > + goto exit;
> > + }
> > +
> > + /* Protect against 'ts' being unregistered */
> > + mutex_lock(&sensor_list_lock);
> > +
> > + /* Protect tz->trip_attr[] and tz->sensor_trip[] */
> > + mutex_lock(&tz->lock);
> > +
> > + tz->trip_attr[indx] = kzalloc(sizeof(struct thermal_trip_attr),
> > + GFP_KERNEL);
> > + if (!tz->trip_attr[indx]) {
> > + ret = -ENOMEM;
> > + goto exit_lock;
> > + }
> > +
> > + attrs = tz->trip_attr[indx];
> > +
> > + /* Create Critical trip point attribute */
> > + if (trip->crit != THERMAL_TRIPS_NONE) {
> > + snprintf(name, THERMAL_NAME_LENGTH,
> > + "sensor%d_trip_critical", kobj_indx);
> > + ret = create_single_trip_attr(tz, &attrs->crit_attr,
> > + name, critical_trip_show);
> > + if (ret)
> > + goto exit_trip;
> > + }
> > +
> > + /* Create Hot trip point attribute */
> > + if (trip->hot != THERMAL_TRIPS_NONE) {
> > + snprintf(name, THERMAL_NAME_LENGTH,
> > + "sensor%d_trip_hot", kobj_indx);
> > + ret = create_single_trip_attr(tz, &attrs->hot_attr,
> > + name, hot_trip_show);
> > + if (ret)
> > + goto exit_crit_trip;
> > + }
> > +
> > + /* Create Passive trip point attributes */
> > + if (trip->num_passive_trips > 0) {
> > + size = sizeof(struct thermal_attr) * trip->num_passive_trips;
> > + attrs->passive_attrs = kzalloc(size, GFP_KERNEL);
> > + if (!attrs->passive_attrs) {
> > + ret = -ENOMEM;
> > + goto exit_hot_trip;
> > + }
> > +
> > + ret = create_multi_trip_attrs(tz, trip->num_passive_trips,
> > + kobj_indx, attrs->passive_attrs,
> > + "sensor%d_trip_passive%d",
>
> well, I do not think this is a good code style.
> I prefer to create the attrs one by one, rather than passing this ugly
> format string.
> please use create_single_trip_attr() instead if you can not find a clean
> way to do this.

Okay, Will change in next revision.

Thanks,
Durga

>
> thanks,
> rui
> > + passive_trip_show);
> > + if (ret)
> > + goto exit_hot_trip;
> > + }
> > +
> > + /* Create Active trip point attributes */
> > + if (trip->num_active_trips > 0) {
> > + size = sizeof(struct thermal_attr) * trip->num_active_trips;
> > + attrs->active_attrs = kzalloc(size, GFP_KERNEL);
> > + if (!attrs->active_attrs) {
> > + ret = -ENOMEM;
> > + goto exit_passive_trips;
> > + }
> > +
> > + ret = create_multi_trip_attrs(tz, trip->num_active_trips,
> > + kobj_indx, attrs->active_attrs,
> > + "sensor%d_trip_active%d",
> > + active_trip_show);
> > + if (ret)
> > + goto exit_passive_trips;
> > + }
> > +
> > + tz->sensor_trip[indx] = trip;
> > +
> > + mutex_unlock(&tz->lock);
> > + mutex_unlock(&sensor_list_lock);
> > + mutex_unlock(&zone_list_lock);
> > +
> > + return 0;
> > +
> > +exit_passive_trips:
> > + kfree(attrs->active_attrs);
> > + i = trip->num_passive_trips;
> > + while (--i >= 0)
> > + device_remove_file(&tz->device, &attrs->passive_attrs[i].attr);
> > +exit_hot_trip:
> > + kfree(attrs->passive_attrs);
> > + if (trip->hot != THERMAL_TRIPS_NONE)
> > + device_remove_file(&tz->device, &attrs->hot_attr.attr);
> > +exit_crit_trip:
> > + if (trip->crit != THERMAL_TRIPS_NONE)
> > + device_remove_file(&tz->device, &attrs->crit_attr.attr);
> > +exit_trip:
> > + kfree(tz->trip_attr[indx]);
> > + tz->trip_attr[indx] = NULL;
> > +exit_lock:
> > + mutex_unlock(&tz->lock);
> > + mutex_unlock(&sensor_list_lock);
> > +exit:
> > + mutex_unlock(&zone_list_lock);
> > + return ret;
> > +}
> > +EXPORT_SYMBOL(add_sensor_trip_info);
> > +
> > +/**
> > * thermal_sensor_register - register a new thermal sensor
> > * @name: name of the thermal sensor
> > * @count: Number of thresholds supported by hardware
> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> > index da7520c..f8de86d 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -31,7 +31,7 @@
> >
> > #define THERMAL_TRIPS_NONE -1
> > #define THERMAL_MAX_TRIPS 12
> > -#define THERMAL_NAME_LENGTH 20
> > +#define THERMAL_NAME_LENGTH 25
> >
> > /* invalid cooling state */
> > #define THERMAL_CSTATE_INVALID -1UL
> > @@ -170,6 +170,37 @@ struct thermal_attr {
> > char name[THERMAL_NAME_LENGTH];
> > };
> >
> > +/*
> > + * This structure defines the trip points for a sensor.
> > + * The actual values for these trip points come from
> > + * platform characterization. The thermal governors
> > + * (either kernel or user space) may take appropriate
> > + * actions when the sensors reach these trip points.
> > + * See Documentation/thermal/sysfs-api2.txt for more details.
> > + *
> > + * As of now, For a particular sensor, we support:
> > + * a) 1 hot trip point
> > + * b) 1 critical trip point
> > + * c) 'n' passive trip points
> > + * d) 'm' active trip points
> > + */
> > +struct thermal_trip_point {
> > + int hot;
> > + int crit;
> > + int num_passive_trips;
> > + int *passive_trips;
> > + int num_active_trips;
> > + int *active_trips;
> > + int active_trip_mask;
> > +};
> > +
> > +struct thermal_trip_attr {
> > + struct thermal_attr hot_attr;
> > + struct thermal_attr crit_attr;
> > + struct thermal_attr *active_attrs;
> > + struct thermal_attr *passive_attrs;
> > +};
> > +
> > struct thermal_sensor {
> > char name[THERMAL_NAME_LENGTH];
> > int id;
> > @@ -229,6 +260,10 @@ struct thermal_zone {
> > /* cdev level information */
> > int cdev_indx; /* index into 'cdevs' array */
> > struct thermal_cooling_device *cdevs[MAX_CDEVS_PER_ZONE];
> > +
> > + /* Thermal sensors trip information */
> > + struct thermal_trip_point *sensor_trip[MAX_SENSORS_PER_ZONE];
> > + struct thermal_trip_attr *trip_attr[MAX_SENSORS_PER_ZONE];
> > };
> >
> > /* Structure that holds thermal governor information */
> > @@ -309,6 +344,9 @@ struct thermal_sensor *get_sensor_by_name(const
> char *);
> > int add_cdev_to_zone(struct thermal_zone *, struct thermal_cooling_device
> *);
> > struct thermal_cooling_device *get_cdev_by_name(const char *);
> >
> > +int add_sensor_trip_info(struct thermal_zone *, struct thermal_sensor *,
> > + struct thermal_trip_point *);
> > +
> > #ifdef CONFIG_NET
> > extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,
> > enum events event);
>

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2013-10-31 01:33:26

by Srinivas Pandruvada

Subject: Re: [PATCHv4 5/9] Thermal: Add trip point sysfs nodes for sensor

You might want to use new DEVICE_ATTR_RO and DEVICE_ATTR_RW in this
series. GKH wanted this changes in my powercap patchset.

Thanks,
Srinivas

On 10/15/2013 06:12 AM, R, Durgadoss wrote:
>> -----Original Message-----
>> From: Zhang, Rui
>> Sent: Tuesday, October 15, 2013 4:33 PM
>> To: R, Durgadoss
>> Cc: [email protected]; [email protected]; linux-
>> [email protected]; [email protected]; [email protected]
>> Subject: Re: [PATCHv4 5/9] Thermal: Add trip point sysfs nodes for sensor
>>
>> On Wed, 2013-10-02 at 00:08 +0530, Durgadoss R wrote:
>>> This patch adds a trip point related sysfs nodes
>>> for each sensor under a zone in /sys/class/thermal/zoneX/.
>>> The nodes will be named, sensorX_trip_activeY,
>>> sensorX_trip_passiveY, sensorX_trip_hot, sensorX_trip_critical
>>> for active, passive, hot and critical trip points
>>> respectively for sensorX.
>>>
>>> Signed-off-by: Durgadoss R <[email protected]>
>>> ---
>>> drivers/thermal/thermal_core.c | 344
>> +++++++++++++++++++++++++++++++++++++++-
>>> include/linux/thermal.h | 40 ++++-
>>> 2 files changed, 379 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>>> index 3c4ef62..d6e29f6 100644
>>> --- a/drivers/thermal/thermal_core.c
>>> +++ b/drivers/thermal/thermal_core.c
>>> @@ -494,6 +494,60 @@ static void thermal_zone_device_check(struct
>> work_struct *work)
>>> thermal_zone_device_update(tz);
>>> }
>>>
>>> +static int get_sensor_indx_by_kobj(struct thermal_zone *tz, const char
>> *name)
>>> +{
>>> + int i, indx = -EINVAL;
>>> +
>>> + /* Protect against tz->sensors[i] being unregistered */
>>> + mutex_lock(&sensor_list_lock);
>>> +
>>> + for (i = 0; i < tz->sensor_indx; i++) {
>>> + if (!strnicmp(name, kobject_name(&tz->sensors[i]-
>>> device.kobj),
>>> + THERMAL_NAME_LENGTH)) {
>>> + indx = i;
>>> + break;
>>> + }
>>> + }
>>> +
>>> + mutex_unlock(&sensor_list_lock);
>>> + return indx;
>>> +}
>>> +
>>> +static void __remove_trip_attr(struct thermal_zone *tz, int indx)
>>> +{
>>> + int i;
>>> + struct thermal_trip_attr *attr = tz->trip_attr[indx];
>>> + struct thermal_trip_point *trip = tz->sensor_trip[indx];
>>> +
>>> + if (!attr || !trip)
>>> + return;
>>> +
>>> + if (trip->crit != THERMAL_TRIPS_NONE)
>>> + device_remove_file(&tz->device, &attr->crit_attr.attr);
>>> +
>>> + if (trip->hot != THERMAL_TRIPS_NONE)
>>> + device_remove_file(&tz->device, &attr->hot_attr.attr);
>>> +
>>> + if (trip->num_passive_trips > 0) {
>>> + for (i = 0; i < trip->num_passive_trips; i++) {
>>> + device_remove_file(&tz->device,
>>> + &attr->passive_attrs[i].attr);
>>> + }
>>> + kfree(attr->passive_attrs);
>>> + }
>>> +
>>> + if (trip->num_active_trips > 0) {
>>> + for (i = 0; i < trip->num_active_trips; i++) {
>>> + device_remove_file(&tz->device,
>>> + &attr->active_attrs[i].attr);
>>> + }
>>> + kfree(attr->active_attrs);
>>> + }
>>> +
>>> + kfree(tz->trip_attr[indx]);
>>> + tz->trip_attr[indx] = NULL;
>>> +}
>>> +
>>> static void remove_sensor_from_zone(struct thermal_zone *tz,
>>> struct thermal_sensor *ts)
>>> {
>>> @@ -503,13 +557,19 @@ static void remove_sensor_from_zone(struct
>> thermal_zone *tz,
>>> if (indx < 0)
>>> return;
>>>
>>> - sysfs_remove_link(&tz->device.kobj, kobject_name(&ts->device.kobj));
>>> -
>>> mutex_lock(&tz->lock);
>>>
>>> + /* Remove trip point attributes associated with this sensor */
>>> + __remove_trip_attr(tz, indx);
>>> +
>>> + 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++)
>>> + for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++) {
>>> tz->sensors[j] = tz->sensors[j + 1];
>>> + tz->sensor_trip[j] = tz->sensor_trip[j + 1];
>>> + tz->trip_attr[j] = tz->trip_attr[j + 1];
>>> + }
>>>
>>> tz->sensor_indx--;
>>> mutex_unlock(&tz->lock);
>>> @@ -952,6 +1012,111 @@ emul_temp_store(struct device *dev, struct
>> device_attribute *attr,
>>> static DEVICE_ATTR(emul_temp, S_IWUSR, NULL, emul_temp_store);
>>> #endif/*CONFIG_THERMAL_EMULATION*/
>>>
>>> +static ssize_t
>>> +active_trip_show(struct device *dev, struct device_attribute *attr, char *buf)
>>> +{
>>> + int i, j, val;
>>> + char kobj_name[THERMAL_NAME_LENGTH];
>>> + struct thermal_zone *tz = to_zone(dev);
>>> +
>>> + if (!sscanf(attr->attr.name, "sensor%d_trip_active%d", &i, &j))
>>> + return -EINVAL;
>>> +
>>> + snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", i);
>>> +
>>> + mutex_lock(&tz->lock);
>>> +
>>> + i = get_sensor_indx_by_kobj(tz, kobj_name);
>>> + if (i < 0) {
>>> + mutex_unlock(&tz->lock);
>>> + return i;
>>> + }
>>> +
>>> + val = tz->sensor_trip[i]->active_trips[j];
>>> + mutex_unlock(&tz->lock);
>>> +
>>> + return sprintf(buf, "%d\n", val);
>>> +}
>>> +
>>> +static ssize_t
>>> +passive_trip_show(struct device *dev, struct device_attribute *attr, char
>> *buf)
>>> +{
>>> + int i, j, val;
>>> + char kobj_name[THERMAL_NAME_LENGTH];
>>> + struct thermal_zone *tz = to_zone(dev);
>>> +
>>> + if (!sscanf(attr->attr.name, "sensor%d_trip_passive%d", &i, &j))
>>> + return -EINVAL;
>>> +
>>> + snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", i);
>>> +
>>> + mutex_lock(&tz->lock);
>>> +
>>> + i = get_sensor_indx_by_kobj(tz, kobj_name);
>>> + if (i < 0) {
>>> + mutex_unlock(&tz->lock);
>>> + return i;
>>> + }
>>> +
>>> + val = tz->sensor_trip[i]->passive_trips[j];
>>> + mutex_unlock(&tz->lock);
>>> +
>>> + return sprintf(buf, "%d\n", val);
>>> +}
>>> +
>>> +static ssize_t
>>> +hot_trip_show(struct device *dev, struct device_attribute *attr, char *buf)
>>> +{
>>> + int indx, val;
>>> + char kobj_name[THERMAL_NAME_LENGTH];
>>> + struct thermal_zone *tz = to_zone(dev);
>>> +
>>> + if (!sscanf(attr->attr.name, "sensor%d_trip_hot", &indx))
>>> + return -EINVAL;
>>> +
>>> + snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", indx);
>>> +
>>> + mutex_lock(&tz->lock);
>>> +
>>> + indx = get_sensor_indx_by_kobj(tz, kobj_name);
>>> + if (indx < 0) {
>>> + mutex_unlock(&tz->lock);
>>> + return indx;
>>> + }
>>> +
>>> + val = tz->sensor_trip[indx]->hot;
>>> + mutex_unlock(&tz->lock);
>>> +
>>> + return sprintf(buf, "%d\n", val);
>>> +}
>>> +
>>> +static ssize_t
>>> +critical_trip_show(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + int indx, val;
>>> + char kobj_name[THERMAL_NAME_LENGTH];
>>> + struct thermal_zone *tz = to_zone(dev);
>>> +
>>> + if (!sscanf(attr->attr.name, "sensor%d_trip_critical", &indx))
>>> + return -EINVAL;
>>> +
>>> + snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", indx);
>>> +
>>> + mutex_lock(&tz->lock);
>>> +
>>> + indx = get_sensor_indx_by_kobj(tz, kobj_name);
>>> + if (indx < 0) {
>>> + mutex_unlock(&tz->lock);
>>> + return indx;
>>> + }
>>> +
>>> + val = tz->sensor_trip[indx]->crit;
>>> + mutex_unlock(&tz->lock);
>>> +
>>> + return sprintf(buf, "%d\n", val);
>>> +}
>>> +
>>> static DEVICE_ATTR(type, 0444, type_show, NULL);
>>> static DEVICE_ATTR(temp, 0444, temp_show, NULL);
>>> static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
>>> @@ -962,7 +1127,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);
>>> +/* Thermal zone attributes */
>>> +static DEVICE_ATTR(zone_name, S_IRUGO, zone_name_show, NULL);
>>>
>>> /* sys I/F for cooling device */
>>> #define to_cooling_device(_dev) \
>>> @@ -1841,6 +2007,43 @@ static int enable_sensor_thresholds(struct
>> thermal_sensor *ts, int count)
>>> return 0;
>>> }
>>>
>>> +static int create_single_trip_attr(struct thermal_zone *tz,
>>> + struct thermal_attr *attr,
>>> + const char *attr_name,
>>> + ssize_t (*rd_ptr)(struct device *dev,
>>> + struct device_attribute *devattr, char *buf))
>>> +{
>>> + snprintf(attr->name, THERMAL_NAME_LENGTH, attr_name);
>>> + sysfs_attr_init(&attr->attr.attr);
>>> + attr->attr.attr.name = attr->name;
>>> + attr->attr.attr.mode = S_IRUGO;
>>> + attr->attr.show = rd_ptr;
>>> + return device_create_file(&tz->device, &attr->attr);
>>> +}
>>> +
>>> +static int create_multi_trip_attrs(struct thermal_zone *tz, int size,
>>> + int indx, struct thermal_attr *attrs,
>>> + const char *attr_name,
>>> + ssize_t (*rd_ptr)(struct device *dev,
>>> + struct device_attribute *devattr, char *buf))
>>> +{
>>> + char name[THERMAL_NAME_LENGTH];
>>> + int i, ret;
>>> +
>>> + for (i = 0; i < size; i++) {
>>> + snprintf(name, THERMAL_NAME_LENGTH, attr_name, indx, i);
>>> + ret = create_single_trip_attr(tz, &attrs[i], name, rd_ptr);
>>> + if (ret)
>>> + goto exit;
>>> + }
>>> + return 0;
>>> +
>>> +exit:
>>> + while (--i >= 0)
>>> + device_remove_file(&tz->device, &attrs[i].attr);
>>> + return ret;
>>> +}
>>> +
>>> /**
>>> * create_thermal_zone - create sysfs nodes for thermal zone
>>> * @name: Name of the thermla zone
>>> @@ -2139,6 +2342,139 @@ exit_zone:
>>> EXPORT_SYMBOL(add_cdev_to_zone);
>>>
>>> /**
>>> + * add_sensor_trip_info - Add trip point information for @ts in @tz
>>> + * @tz: Thermal zone reference
>>> + * @ts: Thermal sensor reference
>>> + * @trip: Trip point structure reference
>>> + *
>>> + * Returns 0 on success, otherwise
>>> + * -EINVAL for invalid paramenters
>>> + * -EINVAL if @ts is not part of 'this' thermal zone @tz
>>> + * -ENOMEM on kzalloc failures
>>> + */
>>> +int add_sensor_trip_info(struct thermal_zone *tz, struct thermal_sensor *ts,
>>> + struct thermal_trip_point *trip)
>>> +{
>>> + char name[THERMAL_NAME_LENGTH];
>>> + int i, indx, kobj_indx, ret, size;
>>> + struct thermal_trip_attr *attrs;
>>> +
>>> + if (!tz || !ts || !trip)
>>> + return -EINVAL;
>>> +
>>> + if (!sscanf(kobject_name(&ts->device.kobj), "sensor%d", &kobj_indx))
>>> + return -EINVAL;
>>> +
>>> + mutex_lock(&zone_list_lock);
>>> +
>>> + indx = GET_INDEX(tz, ts, sensor);
>>> + if (indx < 0) {
>>> + ret = -EINVAL;
>>> + goto exit;
>>> + }
>>> +
>>> + /* Protect against 'ts' being unregistered */
>>> + mutex_lock(&sensor_list_lock);
>>> +
>>> + /* Protect tz->trip_attr[] and tz->sensor_trip[] */
>>> + mutex_lock(&tz->lock);
>>> +
>>> + tz->trip_attr[indx] = kzalloc(sizeof(struct thermal_trip_attr),
>>> + GFP_KERNEL);
>>> + if (!tz->trip_attr[indx]) {
>>> + ret = -ENOMEM;
>>> + goto exit_lock;
>>> + }
>>> +
>>> + attrs = tz->trip_attr[indx];
>>> +
>>> + /* Create Critical trip point attribute */
>>> + if (trip->crit != THERMAL_TRIPS_NONE) {
>>> + snprintf(name, THERMAL_NAME_LENGTH,
>>> + "sensor%d_trip_critical", kobj_indx);
>>> + ret = create_single_trip_attr(tz, &attrs->crit_attr,
>>> + name, critical_trip_show);
>>> + if (ret)
>>> + goto exit_trip;
>>> + }
>>> +
>>> + /* Create Hot trip point attribute */
>>> + if (trip->hot != THERMAL_TRIPS_NONE) {
>>> + snprintf(name, THERMAL_NAME_LENGTH,
>>> + "sensor%d_trip_hot", kobj_indx);
>>> + ret = create_single_trip_attr(tz, &attrs->hot_attr,
>>> + name, hot_trip_show);
>>> + if (ret)
>>> + goto exit_crit_trip;
>>> + }
>>> +
>>> + /* Create Passive trip point attributes */
>>> + if (trip->num_passive_trips > 0) {
>>> + size = sizeof(struct thermal_attr) * trip->num_passive_trips;
>>> + attrs->passive_attrs = kzalloc(size, GFP_KERNEL);
>>> + if (!attrs->passive_attrs) {
>>> + ret = -ENOMEM;
>>> + goto exit_hot_trip;
>>> + }
>>> +
>>> + ret = create_multi_trip_attrs(tz, trip->num_passive_trips,
>>> + kobj_indx, attrs->passive_attrs,
>>> + "sensor%d_trip_passive%d",
>> well, I do not think this is a good code style.
>> I prefer to create the attrs one by one, rather than passing this ugly
>> format string.
>> please use create_single_trip_attr() instead if you can not find a clean
>> way to do this.
> Okay, Will change in next revision.
>
> Thanks,
> Durga
>
>> thanks,
>> rui
>>> + passive_trip_show);
>>> + if (ret)
>>> + goto exit_hot_trip;
>>> + }
>>> +
>>> + /* Create Active trip point attributes */
>>> + if (trip->num_active_trips > 0) {
>>> + size = sizeof(struct thermal_attr) * trip->num_active_trips;
>>> + attrs->active_attrs = kzalloc(size, GFP_KERNEL);
>>> + if (!attrs->active_attrs) {
>>> + ret = -ENOMEM;
>>> + goto exit_passive_trips;
>>> + }
>>> +
>>> + ret = create_multi_trip_attrs(tz, trip->num_active_trips,
>>> + kobj_indx, attrs->active_attrs,
>>> + "sensor%d_trip_active%d",
>>> + active_trip_show);
>>> + if (ret)
>>> + goto exit_passive_trips;
>>> + }
>>> +
>>> + tz->sensor_trip[indx] = trip;
>>> +
>>> + mutex_unlock(&tz->lock);
>>> + mutex_unlock(&sensor_list_lock);
>>> + mutex_unlock(&zone_list_lock);
>>> +
>>> + return 0;
>>> +
>>> +exit_passive_trips:
>>> + kfree(attrs->active_attrs);
>>> + i = trip->num_passive_trips;
>>> + while (--i >= 0)
>>> + device_remove_file(&tz->device, &attrs->passive_attrs[i].attr);
>>> +exit_hot_trip:
>>> + kfree(attrs->passive_attrs);
>>> + if (trip->hot != THERMAL_TRIPS_NONE)
>>> + device_remove_file(&tz->device, &attrs->hot_attr.attr);
>>> +exit_crit_trip:
>>> + if (trip->crit != THERMAL_TRIPS_NONE)
>>> + device_remove_file(&tz->device, &attrs->crit_attr.attr);
>>> +exit_trip:
>>> + kfree(tz->trip_attr[indx]);
>>> + tz->trip_attr[indx] = NULL;
>>> +exit_lock:
>>> + mutex_unlock(&tz->lock);
>>> + mutex_unlock(&sensor_list_lock);
>>> +exit:
>>> + mutex_unlock(&zone_list_lock);
>>> + return ret;
>>> +}
>>> +EXPORT_SYMBOL(add_sensor_trip_info);
>>> +
>>> +/**
>>> * thermal_sensor_register - register a new thermal sensor
>>> * @name: name of the thermal sensor
>>> * @count: Number of thresholds supported by hardware
>>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>>> index da7520c..f8de86d 100644
>>> --- a/include/linux/thermal.h
>>> +++ b/include/linux/thermal.h
>>> @@ -31,7 +31,7 @@
>>>
>>> #define THERMAL_TRIPS_NONE -1
>>> #define THERMAL_MAX_TRIPS 12
>>> -#define THERMAL_NAME_LENGTH 20
>>> +#define THERMAL_NAME_LENGTH 25
>>>
>>> /* invalid cooling state */
>>> #define THERMAL_CSTATE_INVALID -1UL
>>> @@ -170,6 +170,37 @@ struct thermal_attr {
>>> char name[THERMAL_NAME_LENGTH];
>>> };
>>>
>>> +/*
>>> + * This structure defines the trip points for a sensor.
>>> + * The actual values for these trip points come from
>>> + * platform characterization. The thermal governors
>>> + * (either kernel or user space) may take appropriate
>>> + * actions when the sensors reach these trip points.
>>> + * See Documentation/thermal/sysfs-api2.txt for more details.
>>> + *
>>> + * As of now, For a particular sensor, we support:
>>> + * a) 1 hot trip point
>>> + * b) 1 critical trip point
>>> + * c) 'n' passive trip points
>>> + * d) 'm' active trip points
>>> + */
>>> +struct thermal_trip_point {
>>> + int hot;
>>> + int crit;
>>> + int num_passive_trips;
>>> + int *passive_trips;
>>> + int num_active_trips;
>>> + int *active_trips;
>>> + int active_trip_mask;
>>> +};
>>> +
>>> +struct thermal_trip_attr {
>>> + struct thermal_attr hot_attr;
>>> + struct thermal_attr crit_attr;
>>> + struct thermal_attr *active_attrs;
>>> + struct thermal_attr *passive_attrs;
>>> +};
>>> +
>>> struct thermal_sensor {
>>> char name[THERMAL_NAME_LENGTH];
>>> int id;
>>> @@ -229,6 +260,10 @@ struct thermal_zone {
>>> /* cdev level information */
>>> int cdev_indx; /* index into 'cdevs' array */
>>> struct thermal_cooling_device *cdevs[MAX_CDEVS_PER_ZONE];
>>> +
>>> + /* Thermal sensors trip information */
>>> + struct thermal_trip_point *sensor_trip[MAX_SENSORS_PER_ZONE];
>>> + struct thermal_trip_attr *trip_attr[MAX_SENSORS_PER_ZONE];
>>> };
>>>
>>> /* Structure that holds thermal governor information */
>>> @@ -309,6 +344,9 @@ struct thermal_sensor *get_sensor_by_name(const
>> char *);
>>> int add_cdev_to_zone(struct thermal_zone *, struct thermal_cooling_device
>> *);
>>> struct thermal_cooling_device *get_cdev_by_name(const char *);
>>>
>>> +int add_sensor_trip_info(struct thermal_zone *, struct thermal_sensor *,
>>> + struct thermal_trip_point *);
>>> +
>>> #ifdef CONFIG_NET
>>> extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,
>>> enum events event);
> N�����r��y���b�X��ǧv�^�)޺{.n�+����{��h����ܨ}���Ơz�&j:+v�������zZ+��+zf���h���~����i���z��w���?����&�)ߢfl===

2013-10-31 05:25:54

by R, Durgadoss

Subject: RE: [PATCHv4 5/9] Thermal: Add trip point sysfs nodes for sensor

> -----Original Message-----
> From: [email protected] [mailto:linux-pm-
> [email protected]] On Behalf Of Srinivas Pandruvada
> Sent: Thursday, October 31, 2013 7:03 AM
> To: R, Durgadoss; Zhang, Rui
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCHv4 5/9] Thermal: Add trip point sysfs nodes for sensor
>
> You might want to use new DEVICE_ATTR_RO and DEVICE_ATTR_RW in this
> series. GKH wanted this changes in my powercap patchset.

I am working on my next version. Will take care of this in appropriate places.
Thank you for pointing this out.

Thanks,
Durga

>
> Thanks,
> Srinivas
>
> On 10/15/2013 06:12 AM, R, Durgadoss wrote:
> >> -----Original Message-----
> >> From: Zhang, Rui
> >> Sent: Tuesday, October 15, 2013 4:33 PM
> >> To: R, Durgadoss
> >> Cc: [email protected]; [email protected]; linux-
> >> [email protected]; [email protected]; [email protected]
> >> Subject: Re: [PATCHv4 5/9] Thermal: Add trip point sysfs nodes for sensor
> >>
> >> On Wed, 2013-10-02 at 00:08 +0530, Durgadoss R wrote:
> >>> This patch adds a trip point related sysfs nodes
> >>> for each sensor under a zone in /sys/class/thermal/zoneX/.
> >>> The nodes will be named, sensorX_trip_activeY,
> >>> sensorX_trip_passiveY, sensorX_trip_hot, sensorX_trip_critical
> >>> for active, passive, hot and critical trip points
> >>> respectively for sensorX.
> >>>
> >>> Signed-off-by: Durgadoss R <[email protected]>
> >>> ---
> >>> drivers/thermal/thermal_core.c | 344
> >> +++++++++++++++++++++++++++++++++++++++-
> >>> include/linux/thermal.h | 40 ++++-
> >>> 2 files changed, 379 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/thermal/thermal_core.c
> b/drivers/thermal/thermal_core.c
> >>> index 3c4ef62..d6e29f6 100644
> >>> --- a/drivers/thermal/thermal_core.c
> >>> +++ b/drivers/thermal/thermal_core.c
> >>> @@ -494,6 +494,60 @@ static void thermal_zone_device_check(struct
> >> work_struct *work)
> >>> thermal_zone_device_update(tz);
> >>> }
> >>>
> >>> +static int get_sensor_indx_by_kobj(struct thermal_zone *tz, const char
> >> *name)
> >>> +{
> >>> + int i, indx = -EINVAL;
> >>> +
> >>> + /* Protect against tz->sensors[i] being unregistered */
> >>> + mutex_lock(&sensor_list_lock);
> >>> +
> >>> + for (i = 0; i < tz->sensor_indx; i++) {
> >>> + if (!strnicmp(name, kobject_name(&tz->sensors[i]-
> >>> device.kobj),
> >>> + THERMAL_NAME_LENGTH)) {
> >>> + indx = i;
> >>> + break;
> >>> + }
> >>> + }
> >>> +
> >>> + mutex_unlock(&sensor_list_lock);
> >>> + return indx;
> >>> +}
> >>> +
> >>> +static void __remove_trip_attr(struct thermal_zone *tz, int indx)
> >>> +{
> >>> + int i;
> >>> + struct thermal_trip_attr *attr = tz->trip_attr[indx];
> >>> + struct thermal_trip_point *trip = tz->sensor_trip[indx];
> >>> +
> >>> + if (!attr || !trip)
> >>> + return;
> >>> +
> >>> + if (trip->crit != THERMAL_TRIPS_NONE)
> >>> + device_remove_file(&tz->device, &attr->crit_attr.attr);
> >>> +
> >>> + if (trip->hot != THERMAL_TRIPS_NONE)
> >>> + device_remove_file(&tz->device, &attr->hot_attr.attr);
> >>> +
> >>> + if (trip->num_passive_trips > 0) {
> >>> + for (i = 0; i < trip->num_passive_trips; i++) {
> >>> + device_remove_file(&tz->device,
> >>> + &attr->passive_attrs[i].attr);
> >>> + }
> >>> + kfree(attr->passive_attrs);
> >>> + }
> >>> +
> >>> + if (trip->num_active_trips > 0) {
> >>> + for (i = 0; i < trip->num_active_trips; i++) {
> >>> + device_remove_file(&tz->device,
> >>> + &attr->active_attrs[i].attr);
> >>> + }
> >>> + kfree(attr->active_attrs);
> >>> + }
> >>> +
> >>> + kfree(tz->trip_attr[indx]);
> >>> + tz->trip_attr[indx] = NULL;
> >>> +}
> >>> +
> >>> static void remove_sensor_from_zone(struct thermal_zone *tz,
> >>> struct thermal_sensor *ts)
> >>> {
> >>> @@ -503,13 +557,19 @@ static void remove_sensor_from_zone(struct
> >> thermal_zone *tz,
> >>> if (indx < 0)
> >>> return;
> >>>
> >>> - sysfs_remove_link(&tz->device.kobj, kobject_name(&ts->device.kobj));
> >>> -
> >>> mutex_lock(&tz->lock);
> >>>
> >>> + /* Remove trip point attributes associated with this sensor */
> >>> + __remove_trip_attr(tz, indx);
> >>> +
> >>> + 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++)
> >>> + for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++) {
> >>> tz->sensors[j] = tz->sensors[j + 1];
> >>> + tz->sensor_trip[j] = tz->sensor_trip[j + 1];
> >>> + tz->trip_attr[j] = tz->trip_attr[j + 1];
> >>> + }
> >>>
> >>> tz->sensor_indx--;
> >>> mutex_unlock(&tz->lock);
> >>> @@ -952,6 +1012,111 @@ emul_temp_store(struct device *dev, struct
> >> device_attribute *attr,
> >>> static DEVICE_ATTR(emul_temp, S_IWUSR, NULL, emul_temp_store);
> >>> #endif/*CONFIG_THERMAL_EMULATION*/
> >>>
> >>> +static ssize_t
> >>> +active_trip_show(struct device *dev, struct device_attribute *attr, char
> *buf)
> >>> +{
> >>> + int i, j, val;
> >>> + char kobj_name[THERMAL_NAME_LENGTH];
> >>> + struct thermal_zone *tz = to_zone(dev);
> >>> +
> >>> + if (!sscanf(attr->attr.name, "sensor%d_trip_active%d", &i, &j))
> >>> + return -EINVAL;
> >>> +
> >>> + snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", i);
> >>> +
> >>> + mutex_lock(&tz->lock);
> >>> +
> >>> + i = get_sensor_indx_by_kobj(tz, kobj_name);
> >>> + if (i < 0) {
> >>> + mutex_unlock(&tz->lock);
> >>> + return i;
> >>> + }
> >>> +
> >>> + val = tz->sensor_trip[i]->active_trips[j];
> >>> + mutex_unlock(&tz->lock);
> >>> +
> >>> + return sprintf(buf, "%d\n", val);
> >>> +}
> >>> +
> >>> +static ssize_t
> >>> +passive_trip_show(struct device *dev, struct device_attribute *attr, char
> >> *buf)
> >>> +{
> >>> + int i, j, val;
> >>> + char kobj_name[THERMAL_NAME_LENGTH];
> >>> + struct thermal_zone *tz = to_zone(dev);
> >>> +
> >>> + if (!sscanf(attr->attr.name, "sensor%d_trip_passive%d", &i, &j))
> >>> + return -EINVAL;
> >>> +
> >>> + snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", i);
> >>> +
> >>> + mutex_lock(&tz->lock);
> >>> +
> >>> + i = get_sensor_indx_by_kobj(tz, kobj_name);
> >>> + if (i < 0) {
> >>> + mutex_unlock(&tz->lock);
> >>> + return i;
> >>> + }
> >>> +
> >>> + val = tz->sensor_trip[i]->passive_trips[j];
> >>> + mutex_unlock(&tz->lock);
> >>> +
> >>> + return sprintf(buf, "%d\n", val);
> >>> +}
> >>> +
> >>> +static ssize_t
> >>> +hot_trip_show(struct device *dev, struct device_attribute *attr, char *buf)
> >>> +{
> >>> + int indx, val;
> >>> + char kobj_name[THERMAL_NAME_LENGTH];
> >>> + struct thermal_zone *tz = to_zone(dev);
> >>> +
> >>> + if (!sscanf(attr->attr.name, "sensor%d_trip_hot", &indx))
> >>> + return -EINVAL;
> >>> +
> >>> + snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", indx);
> >>> +
> >>> + mutex_lock(&tz->lock);
> >>> +
> >>> + indx = get_sensor_indx_by_kobj(tz, kobj_name);
> >>> + if (indx < 0) {
> >>> + mutex_unlock(&tz->lock);
> >>> + return indx;
> >>> + }
> >>> +
> >>> + val = tz->sensor_trip[indx]->hot;
> >>> + mutex_unlock(&tz->lock);
> >>> +
> >>> + return sprintf(buf, "%d\n", val);
> >>> +}
> >>> +
> >>> +static ssize_t
> >>> +critical_trip_show(struct device *dev,
> >>> + struct device_attribute *attr, char *buf)
> >>> +{
> >>> + int indx, val;
> >>> + char kobj_name[THERMAL_NAME_LENGTH];
> >>> + struct thermal_zone *tz = to_zone(dev);
> >>> +
> >>> + if (!sscanf(attr->attr.name, "sensor%d_trip_critical", &indx))
> >>> + return -EINVAL;
> >>> +
> >>> + snprintf(kobj_name, THERMAL_NAME_LENGTH, "sensor%d", indx);
> >>> +
> >>> + mutex_lock(&tz->lock);
> >>> +
> >>> + indx = get_sensor_indx_by_kobj(tz, kobj_name);
> >>> + if (indx < 0) {
> >>> + mutex_unlock(&tz->lock);
> >>> + return indx;
> >>> + }
> >>> +
> >>> + val = tz->sensor_trip[indx]->crit;
> >>> + mutex_unlock(&tz->lock);
> >>> +
> >>> + return sprintf(buf, "%d\n", val);
> >>> +}
> >>> +
> >>> static DEVICE_ATTR(type, 0444, type_show, NULL);
> >>> static DEVICE_ATTR(temp, 0444, temp_show, NULL);
> >>> static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
> >>> @@ -962,7 +1127,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);
> >>> +/* Thermal zone attributes */
> >>> +static DEVICE_ATTR(zone_name, S_IRUGO, zone_name_show, NULL);
> >>>
> >>> /* sys I/F for cooling device */
> >>> #define to_cooling_device(_dev) \
> >>> @@ -1841,6 +2007,43 @@ static int enable_sensor_thresholds(struct
> >> thermal_sensor *ts, int count)
> >>> return 0;
> >>> }
> >>>
> >>> +static int create_single_trip_attr(struct thermal_zone *tz,
> >>> + struct thermal_attr *attr,
> >>> + const char *attr_name,
> >>> + ssize_t (*rd_ptr)(struct device *dev,
> >>> + struct device_attribute *devattr, char *buf))
> >>> +{
> >>> + snprintf(attr->name, THERMAL_NAME_LENGTH, attr_name);
> >>> + sysfs_attr_init(&attr->attr.attr);
> >>> + attr->attr.attr.name = attr->name;
> >>> + attr->attr.attr.mode = S_IRUGO;
> >>> + attr->attr.show = rd_ptr;
> >>> + return device_create_file(&tz->device, &attr->attr);
> >>> +}
> >>> +
> >>> +static int create_multi_trip_attrs(struct thermal_zone *tz, int size,
> >>> + int indx, struct thermal_attr *attrs,
> >>> + const char *attr_name,
> >>> + ssize_t (*rd_ptr)(struct device *dev,
> >>> + struct device_attribute *devattr, char *buf))
> >>> +{
> >>> + char name[THERMAL_NAME_LENGTH];
> >>> + int i, ret;
> >>> +
> >>> + for (i = 0; i < size; i++) {
> >>> + snprintf(name, THERMAL_NAME_LENGTH, attr_name, indx, i);
> >>> + ret = create_single_trip_attr(tz, &attrs[i], name, rd_ptr);
> >>> + if (ret)
> >>> + goto exit;
> >>> + }
> >>> + return 0;
> >>> +
> >>> +exit:
> >>> + while (--i >= 0)
> >>> + device_remove_file(&tz->device, &attrs[i].attr);
> >>> + return ret;
> >>> +}
> >>> +
> >>> /**
> >>> * create_thermal_zone - create sysfs nodes for thermal zone
> >>> * @name: Name of the thermla zone
> >>> @@ -2139,6 +2342,139 @@ exit_zone:
> >>> EXPORT_SYMBOL(add_cdev_to_zone);
> >>>
> >>> /**
> >>> + * add_sensor_trip_info - Add trip point information for @ts in @tz
> >>> + * @tz: Thermal zone reference
> >>> + * @ts: Thermal sensor reference
> >>> + * @trip: Trip point structure reference
> >>> + *
> >>> + * Returns 0 on success, otherwise
> >>> + * -EINVAL for invalid paramenters
> >>> + * -EINVAL if @ts is not part of 'this' thermal zone @tz
> >>> + * -ENOMEM on kzalloc failures
> >>> + */
> >>> +int add_sensor_trip_info(struct thermal_zone *tz, struct thermal_sensor
> *ts,
> >>> + struct thermal_trip_point *trip)
> >>> +{
> >>> + char name[THERMAL_NAME_LENGTH];
> >>> + int i, indx, kobj_indx, ret, size;
> >>> + struct thermal_trip_attr *attrs;
> >>> +
> >>> + if (!tz || !ts || !trip)
> >>> + return -EINVAL;
> >>> +
> >>> + if (!sscanf(kobject_name(&ts->device.kobj), "sensor%d", &kobj_indx))
> >>> + return -EINVAL;
> >>> +
> >>> + mutex_lock(&zone_list_lock);
> >>> +
> >>> + indx = GET_INDEX(tz, ts, sensor);
> >>> + if (indx < 0) {
> >>> + ret = -EINVAL;
> >>> + goto exit;
> >>> + }
> >>> +
> >>> + /* Protect against 'ts' being unregistered */
> >>> + mutex_lock(&sensor_list_lock);
> >>> +
> >>> + /* Protect tz->trip_attr[] and tz->sensor_trip[] */
> >>> + mutex_lock(&tz->lock);
> >>> +
> >>> + tz->trip_attr[indx] = kzalloc(sizeof(struct thermal_trip_attr),
> >>> + GFP_KERNEL);
> >>> + if (!tz->trip_attr[indx]) {
> >>> + ret = -ENOMEM;
> >>> + goto exit_lock;
> >>> + }
> >>> +
> >>> + attrs = tz->trip_attr[indx];
> >>> +
> >>> + /* Create Critical trip point attribute */
> >>> + if (trip->crit != THERMAL_TRIPS_NONE) {
> >>> + snprintf(name, THERMAL_NAME_LENGTH,
> >>> + "sensor%d_trip_critical", kobj_indx);
> >>> + ret = create_single_trip_attr(tz, &attrs->crit_attr,
> >>> + name, critical_trip_show);
> >>> + if (ret)
> >>> + goto exit_trip;
> >>> + }
> >>> +
> >>> + /* Create Hot trip point attribute */
> >>> + if (trip->hot != THERMAL_TRIPS_NONE) {
> >>> + snprintf(name, THERMAL_NAME_LENGTH,
> >>> + "sensor%d_trip_hot", kobj_indx);
> >>> + ret = create_single_trip_attr(tz, &attrs->hot_attr,
> >>> + name, hot_trip_show);
> >>> + if (ret)
> >>> + goto exit_crit_trip;
> >>> + }
> >>> +
> >>> + /* Create Passive trip point attributes */
> >>> + if (trip->num_passive_trips > 0) {
> >>> + size = sizeof(struct thermal_attr) * trip->num_passive_trips;
> >>> + attrs->passive_attrs = kzalloc(size, GFP_KERNEL);
> >>> + if (!attrs->passive_attrs) {
> >>> + ret = -ENOMEM;
> >>> + goto exit_hot_trip;
> >>> + }
> >>> +
> >>> + ret = create_multi_trip_attrs(tz, trip->num_passive_trips,
> >>> + kobj_indx, attrs->passive_attrs,
> >>> + "sensor%d_trip_passive%d",
> >> well, I do not think this is a good code style.
> >> I prefer to create the attrs one by one, rather than passing this ugly
> >> format string.
> >> please use create_single_trip_attr() instead if you can not find a clean
> >> way to do this.
> > Okay, Will change in next revision.
> >
> > Thanks,
> > Durga
> >
> >> thanks,
> >> rui
> >>> + passive_trip_show);
> >>> + if (ret)
> >>> + goto exit_hot_trip;
> >>> + }
> >>> +
> >>> + /* Create Active trip point attributes */
> >>> + if (trip->num_active_trips > 0) {
> >>> + size = sizeof(struct thermal_attr) * trip->num_active_trips;
> >>> + attrs->active_attrs = kzalloc(size, GFP_KERNEL);
> >>> + if (!attrs->active_attrs) {
> >>> + ret = -ENOMEM;
> >>> + goto exit_passive_trips;
> >>> + }
> >>> +
> >>> + ret = create_multi_trip_attrs(tz, trip->num_active_trips,
> >>> + kobj_indx, attrs->active_attrs,
> >>> + "sensor%d_trip_active%d",
> >>> + active_trip_show);
> >>> + if (ret)
> >>> + goto exit_passive_trips;
> >>> + }
> >>> +
> >>> + tz->sensor_trip[indx] = trip;
> >>> +
> >>> + mutex_unlock(&tz->lock);
> >>> + mutex_unlock(&sensor_list_lock);
> >>> + mutex_unlock(&zone_list_lock);
> >>> +
> >>> + return 0;
> >>> +
> >>> +exit_passive_trips:
> >>> + kfree(attrs->active_attrs);
> >>> + i = trip->num_passive_trips;
> >>> + while (--i >= 0)
> >>> + device_remove_file(&tz->device, &attrs->passive_attrs[i].attr);
> >>> +exit_hot_trip:
> >>> + kfree(attrs->passive_attrs);
> >>> + if (trip->hot != THERMAL_TRIPS_NONE)
> >>> + device_remove_file(&tz->device, &attrs->hot_attr.attr);
> >>> +exit_crit_trip:
> >>> + if (trip->crit != THERMAL_TRIPS_NONE)
> >>> + device_remove_file(&tz->device, &attrs->crit_attr.attr);
> >>> +exit_trip:
> >>> + kfree(tz->trip_attr[indx]);
> >>> + tz->trip_attr[indx] = NULL;
> >>> +exit_lock:
> >>> + mutex_unlock(&tz->lock);
> >>> + mutex_unlock(&sensor_list_lock);
> >>> +exit:
> >>> + mutex_unlock(&zone_list_lock);
> >>> + return ret;
> >>> +}
> >>> +EXPORT_SYMBOL(add_sensor_trip_info);
> >>> +
> >>> +/**
> >>> * thermal_sensor_register - register a new thermal sensor
> >>> * @name: name of the thermal sensor
> >>> * @count: Number of thresholds supported by hardware
> >>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> >>> index da7520c..f8de86d 100644
> >>> --- a/include/linux/thermal.h
> >>> +++ b/include/linux/thermal.h
> >>> @@ -31,7 +31,7 @@
> >>>
> >>> #define THERMAL_TRIPS_NONE -1
> >>> #define THERMAL_MAX_TRIPS 12
> >>> -#define THERMAL_NAME_LENGTH 20
> >>> +#define THERMAL_NAME_LENGTH 25
> >>>
> >>> /* invalid cooling state */
> >>> #define THERMAL_CSTATE_INVALID -1UL
> >>> @@ -170,6 +170,37 @@ struct thermal_attr {
> >>> char name[THERMAL_NAME_LENGTH];
> >>> };
> >>>
> >>> +/*
> >>> + * This structure defines the trip points for a sensor.
> >>> + * The actual values for these trip points come from
> >>> + * platform characterization. The thermal governors
> >>> + * (either kernel or user space) may take appropriate
> >>> + * actions when the sensors reach these trip points.
> >>> + * See Documentation/thermal/sysfs-api2.txt for more details.
> >>> + *
> >>> + * As of now, For a particular sensor, we support:
> >>> + * a) 1 hot trip point
> >>> + * b) 1 critical trip point
> >>> + * c) 'n' passive trip points
> >>> + * d) 'm' active trip points
> >>> + */
> >>> +struct thermal_trip_point {
> >>> + int hot;
> >>> + int crit;
> >>> + int num_passive_trips;
> >>> + int *passive_trips;
> >>> + int num_active_trips;
> >>> + int *active_trips;
> >>> + int active_trip_mask;
> >>> +};
> >>> +
> >>> +struct thermal_trip_attr {
> >>> + struct thermal_attr hot_attr;
> >>> + struct thermal_attr crit_attr;
> >>> + struct thermal_attr *active_attrs;
> >>> + struct thermal_attr *passive_attrs;
> >>> +};
> >>> +
> >>> struct thermal_sensor {
> >>> char name[THERMAL_NAME_LENGTH];
> >>> int id;
> >>> @@ -229,6 +260,10 @@ struct thermal_zone {
> >>> /* cdev level information */
> >>> int cdev_indx; /* index into 'cdevs' array */
> >>> struct thermal_cooling_device *cdevs[MAX_CDEVS_PER_ZONE];
> >>> +
> >>> + /* Thermal sensors trip information */
> >>> + struct thermal_trip_point *sensor_trip[MAX_SENSORS_PER_ZONE];
> >>> + struct thermal_trip_attr *trip_attr[MAX_SENSORS_PER_ZONE];
> >>> };
> >>>
> >>> /* Structure that holds thermal governor information */
> >>> @@ -309,6 +344,9 @@ struct thermal_sensor *get_sensor_by_name(const
> >> char *);
> >>> int add_cdev_to_zone(struct thermal_zone *, struct
> thermal_cooling_device
> >> *);
> >>> struct thermal_cooling_device *get_cdev_by_name(const char *);
> >>>
> >>> +int add_sensor_trip_info(struct thermal_zone *, struct thermal_sensor *,
> >>> + struct thermal_trip_point *);
> >>> +
> >>> #ifdef CONFIG_NET
> >>> extern int thermal_generate_netlink_event(struct thermal_zone_device
> *tz,
> >>> enum events event);
> >
> N�����r��y���b�X��ǧv�^�)޺{.n�+����{��h����ܨ}���Ơz�&j:+v���
����z
> Z+��+zf���h���~����i���z��w���?����&�)ߢfl===
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?