Linux Power Management development
 help / color / mirror / Atom feed
* [RFC PATCH 2/7] Thermal: Create zone level APIs
From: Durgadoss R @ 2012-11-17 10:45 UTC (permalink / raw)
  To: rui.zhang, linux-pm
  Cc: wni, eduardo.valentin, amit.kachhap, hongbo.zhang, sachin.kamat,
	Durgadoss R
In-Reply-To: <1353149158-19102-1-git-send-email-durgadoss.r@intel.com>

This patch adds a new thermal_zone structure to
thermal.h. Also, adds zone level APIs to the thermal
framework.

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

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


^ permalink raw reply related

* [RFC PATCH 3/7] Thermal: Add APIs to bind cdev to new zone structure
From: Durgadoss R @ 2012-11-17 10:45 UTC (permalink / raw)
  To: rui.zhang, linux-pm
  Cc: wni, eduardo.valentin, amit.kachhap, hongbo.zhang, sachin.kamat,
	Durgadoss R
In-Reply-To: <1353149158-19102-1-git-send-email-durgadoss.r@intel.com>

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 <durgadoss.r@intel.com>
---
 drivers/thermal/thermal_sys.c |  109 +++++++++++++++++++++++++++++++++++++++++
 include/linux/thermal.h       |    9 ++++
 2 files changed, 118 insertions(+)

diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 9d386d7..70e5f5a 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -58,6 +58,7 @@ static LIST_HEAD(thermal_governor_list);
 static DEFINE_MUTEX(thermal_list_lock);
 static DEFINE_MUTEX(ts_list_lock);
 static DEFINE_MUTEX(tz_list_lock);
+static DEFINE_MUTEX(cdev_list_lock);
 static DEFINE_MUTEX(thermal_governor_lock);
 
 #define for_each_thermal_sensor(pos) \
@@ -66,6 +67,9 @@ static DEFINE_MUTEX(thermal_governor_lock);
 #define for_each_thermal_zone(pos) \
 	list_for_each_entry(pos, &thermal_zone_list, node)
 
+#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;
@@ -428,6 +432,25 @@ static void thermal_zone_device_check(struct work_struct *work)
 	thermal_zone_device_update(tz);
 }
 
+static int get_cdev_indx(struct thermal_zone *tz,
+			struct thermal_cooling_device *cdev)
+{
+	int i, indx = -EINVAL;
+
+	if (!tz || !cdev)
+		return -EINVAL;
+
+	mutex_lock(&cdev_list_lock);
+	for (i = 0; i < tz->cdev_indx; i++) {
+		if (tz->cdevs[i] == cdev) {
+			indx = i;
+			break;
+		}
+	}
+	mutex_unlock(&cdev_list_lock);
+	return indx;
+}
+
 static int get_sensor_indx(struct thermal_zone *tz, struct thermal_sensor *ts)
 {
 	int i, indx = -EINVAL;
@@ -464,6 +487,24 @@ static void remove_sensor_from_zone(struct thermal_zone *tz,
 	tz->sensor_indx--;
 }
 
+static void remove_cdev_from_zone(struct thermal_zone *tz,
+				struct thermal_cooling_device *cdev)
+{
+	int indx, j;
+
+	indx = get_cdev_indx(tz, cdev);
+	if (indx < 0)
+		return;
+
+	sysfs_remove_link(&tz->device.kobj, kobject_name(&cdev->device.kobj));
+
+	/* 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--;
+}
+
 /* sys I/F for thermal zone */
 
 #define to_thermal_zone(_dev) \
@@ -1423,6 +1464,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)
@@ -1460,6 +1502,13 @@ void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)
 
 	mutex_unlock(&thermal_list_lock);
 
+	mutex_lock(&tz_list_lock);
+
+	for_each_thermal_zone(tmp_tz)
+		remove_cdev_from_zone(tmp_tz, cdev);
+
+	mutex_unlock(&tz_list_lock);
+
 	if (cdev->type[0])
 		device_remove_file(&cdev->device, &dev_attr_cdev_type);
 	device_remove_file(&cdev->device, &dev_attr_max_state);
@@ -1695,6 +1744,23 @@ exit:
 }
 EXPORT_SYMBOL(remove_thermal_zone);
 
+struct thermal_cooling_device *get_cdev_by_name(const char *name)
+{
+	struct thermal_cooling_device *pos;
+	struct thermal_cooling_device *cdev = NULL;
+
+	mutex_lock(&cdev_list_lock);
+	for_each_cdev(pos) {
+		if (!strnicmp(pos->type, name, THERMAL_NAME_LENGTH)) {
+			cdev = pos;
+			break;
+		}
+	}
+	mutex_unlock(&cdev_list_lock);
+	return cdev;
+}
+EXPORT_SYMBOL(get_cdev_by_name);
+
 struct thermal_sensor *get_sensor_by_name(const char *name)
 {
 	struct thermal_sensor *pos;
@@ -1755,6 +1821,49 @@ int add_sensor_to_zone(struct thermal_zone *tz, struct thermal_sensor *ts)
 }
 EXPORT_SYMBOL(add_sensor_to_zone);
 
+int add_cdev_to_zone_by_name(struct thermal_zone *tz, const char *name)
+{
+	int ret;
+	struct thermal_cooling_device *cdev = get_cdev_by_name(name);
+
+	if (!tz || !cdev)
+		return -EINVAL;
+
+	mutex_lock(&tz_list_lock);
+
+	/* Ensure we are not adding the same cdev again!! */
+	ret = get_cdev_indx(tz, cdev);
+	if (ret >= 0) {
+		ret = -EEXIST;
+		goto exit_zone;
+	}
+
+	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;
+
+	tz->cdevs[tz->cdev_indx++] = cdev;
+
+exit_cdev:
+	mutex_unlock(&cdev_list_lock);
+exit_zone:
+	mutex_unlock(&tz_list_lock);
+	return ret;
+}
+EXPORT_SYMBOL(add_cdev_to_zone_by_name);
+
+int add_cdev_to_zone(struct thermal_zone *tz,
+			struct thermal_cooling_device *cdev)
+{
+	if (!tz || !cdev)
+		return -EINVAL;
+
+	return add_cdev_to_zone_by_name(tz, cdev->type);
+}
+EXPORT_SYMBOL(add_cdev_to_zone);
+
 /**
  * thermal_sensor_register - register a new thermal sensor
  * @name:	name of the thermal sensor
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 38438be..2913910 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -54,6 +54,8 @@
  */
 #define MAX_SENSORS_PER_ZONE		5
 
+#define MAX_CDEVS_PER_ZONE		5
+
 struct thermal_sensor;
 struct thermal_zone_device;
 struct thermal_cooling_device;
@@ -209,6 +211,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 */
@@ -289,6 +295,9 @@ void remove_thermal_zone(struct thermal_zone *);
 int add_sensor_to_zone(struct thermal_zone *, struct thermal_sensor *);
 int add_sensor_to_zone_by_name(struct thermal_zone *, const char *);
 
+int add_cdev_to_zone(struct thermal_zone *, struct thermal_cooling_device *);
+int add_cdev_to_zone_by_name(struct thermal_zone *, const char *);
+
 #ifdef CONFIG_NET
 extern int thermal_generate_netlink_event(u32 orig, enum events event);
 #else
-- 
1.7.9.5


^ permalink raw reply related

* [RFC PATCH 4/7] Thermal: Add Thermal_trip sysfs node
From: Durgadoss R @ 2012-11-17 10:45 UTC (permalink / raw)
  To: rui.zhang, linux-pm
  Cc: wni, eduardo.valentin, amit.kachhap, hongbo.zhang, sachin.kamat,
	Durgadoss R
In-Reply-To: <1353149158-19102-1-git-send-email-durgadoss.r@intel.com>

This patch adds a thermal_trip directory under
/sys/class/thermal/zoneX. This directory contains
the trip point values for sensors bound to this
zone.

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

diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 70e5f5a..011588a 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -480,9 +480,15 @@ static void remove_sensor_from_zone(struct thermal_zone *tz,
 
 	sysfs_remove_link(&tz->device.kobj, kobject_name(&ts->device.kobj));
 
+	/* Delete this sensor's trip Kobject */
+	kobject_del(tz->kobj_trip[indx]);
+
 	/* 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->trip[j] = tz->trip[j + 1];
+		tz->kobj_trip[j] = tz->kobj_trip[j + 1];
+	}
 
 	tz->sensor_indx--;
 }
@@ -505,6 +511,41 @@ static void remove_cdev_from_zone(struct thermal_zone *tz,
 	tz->cdev_indx--;
 }
 
+static struct thermal_zone *get_zone_by_trip_kobj(struct kobject *kobj)
+{
+	struct thermal_zone *pos;
+	struct thermal_zone *tz = NULL;
+
+	if (!kobj)
+		return NULL;
+
+	mutex_lock(&tz_list_lock);
+	for_each_thermal_zone(pos) {
+		if (pos->kobj_thermal_trip == kobj) {
+			tz = pos;
+			break;
+		}
+	}
+	mutex_unlock(&tz_list_lock);
+	return tz;
+}
+
+static struct thermal_sensor *get_sensor_by_kobj_name(const char *name)
+{
+	struct thermal_sensor *pos;
+	struct thermal_sensor *ts = NULL;
+
+	mutex_lock(&ts_list_lock);
+	for_each_thermal_sensor(pos) {
+		if (!strcmp(kobject_name(&pos->device.kobj), name)) {
+			ts = pos;
+			break;
+		}
+	}
+	mutex_unlock(&ts_list_lock);
+	return ts;
+}
+
 /* sys I/F for thermal zone */
 
 #define to_thermal_zone(_dev) \
@@ -859,6 +900,113 @@ policy_show(struct device *dev, struct device_attribute *devattr, char *buf)
 	return sprintf(buf, "%s\n", tz->governor->name);
 }
 
+static ssize_t
+active_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	int i, indx, ret = 0;
+	struct thermal_sensor *ts;
+	struct thermal_zone *tz;
+
+	ts = get_sensor_by_kobj_name(kobject_name(kobj));
+	if (!ts)
+		return -EINVAL;
+
+	tz = get_zone_by_trip_kobj(kobj->parent);
+	if (!tz)
+		return -EINVAL;
+
+	indx = get_sensor_indx(tz, ts);
+	if (indx < 0)
+		return indx;
+
+	if (tz->trip[indx]->num_active_trips <= 0)
+		return sprintf(buf, "<Not available>\n");
+
+	ret += sprintf(buf, "0x%x", tz->trip[indx]->active_trip_mask);
+	for (i = 0; i < tz->trip[indx]->num_active_trips; i++) {
+		ret += sprintf(buf + ret, " %d",
+			tz->trip[indx]->active_trips[i]);
+	}
+
+	ret += sprintf(buf + ret, "\n");
+	return ret;
+}
+
+static ssize_t
+ptrip_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	int i, indx, ret = 0;
+	struct thermal_sensor *ts;
+	struct thermal_zone *tz;
+
+	ts = get_sensor_by_kobj_name(kobject_name(kobj));
+	if (!ts)
+		return -EINVAL;
+
+	tz = get_zone_by_trip_kobj(kobj->parent);
+	if (!tz)
+		return -EINVAL;
+
+	indx = get_sensor_indx(tz, ts);
+	if (indx < 0)
+		return indx;
+
+	if (tz->trip[indx]->num_passive_trips <= 0)
+		return sprintf(buf, "<Not available>\n");
+
+	for (i = 0; i < tz->trip[indx]->num_passive_trips; i++) {
+		ret += sprintf(buf + ret, "%d ",
+			tz->trip[indx]->passive_trips[i]);
+	}
+
+	ret += sprintf(buf + ret, "\n");
+	return ret;
+}
+
+static ssize_t
+hot_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	int indx;
+	struct thermal_sensor *ts;
+	struct thermal_zone *tz;
+
+	ts = get_sensor_by_kobj_name(kobject_name(kobj));
+	if (!ts)
+		return -EINVAL;
+
+	tz = get_zone_by_trip_kobj(kobj->parent);
+	if (!tz)
+		return -EINVAL;
+
+	indx = get_sensor_indx(tz, ts);
+	if (indx < 0)
+		return indx;
+
+	return sprintf(buf, "%d\n", tz->trip[indx]->hot);
+}
+
+static ssize_t
+critical_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	int indx;
+	struct thermal_sensor *ts;
+	struct thermal_zone *tz;
+
+	ts = get_sensor_by_kobj_name(kobject_name(kobj));
+	if (!ts)
+		return -EINVAL;
+
+	tz = get_zone_by_trip_kobj(kobj->parent);
+	if (!tz)
+		return -EINVAL;
+
+	indx = get_sensor_indx(tz, ts);
+	if (indx < 0)
+		return indx;
+
+	return sprintf(buf, "%d\n", tz->trip[indx]->crit);
+}
+
 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);
@@ -869,7 +1017,26 @@ 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);
+
+/* Thermal trip attributes */
+static struct kobj_attribute active_attr = __ATTR_RO(active);
+static struct kobj_attribute passive_attr = __ATTR_RO(ptrip);
+static struct kobj_attribute hot_attr = __ATTR_RO(hot);
+static struct kobj_attribute crit_attr = __ATTR_RO(critical);
+
+static struct attribute *trip_attrs[] = {
+			&active_attr.attr,
+			&passive_attr.attr,
+			&hot_attr.attr,
+			&crit_attr.attr,
+			NULL,
+			};
+
+static struct attribute_group trip_attr_group = {
+			.attrs = trip_attrs,
+			};
 
 /* sys I/F for cooling device */
 #define to_cooling_device(_dev)	\
@@ -1694,12 +1861,19 @@ struct thermal_zone *create_thermal_zone(const char *name, void *devdata)
 	if (ret)
 		goto exit_unregister;
 
+	tz->kobj_thermal_trip = kobject_create_and_add("thermal_trip",
+					&tz->device.kobj);
+	if (!tz->kobj_thermal_trip)
+		goto exit_name;
+
 	/* Add this zone to the global list of thermal zones */
 	mutex_lock(&tz_list_lock);
 	list_add_tail(&tz->node, &thermal_zone_list);
 	mutex_unlock(&tz_list_lock);
 	return tz;
 
+exit_name:
+	device_remove_file(&tz->device, &dev_attr_zone_name);
 exit_unregister:
 	device_unregister(&tz->device);
 exit_idr:
@@ -1713,6 +1887,7 @@ EXPORT_SYMBOL(create_thermal_zone);
 void remove_thermal_zone(struct thermal_zone *tz)
 {
 	struct thermal_zone *pos, *next;
+	int i;
 	bool found = false;
 
 	if (!tz)
@@ -1733,6 +1908,29 @@ void remove_thermal_zone(struct thermal_zone *tz)
 
 	device_remove_file(&tz->device, &dev_attr_zone_name);
 
+	/* Just for ease of usage */
+	i = tz->sensor_indx;
+
+	while (--i >= 0) {
+		sysfs_remove_link(&tz->device.kobj,
+				kobject_name(&tz->sensors[i]->device.kobj));
+		if (tz->kobj_trip[i])
+			kobject_del(tz->kobj_trip[i]);
+	}
+
+	if (tz->kobj_thermal_trip)
+		kobject_del(tz->kobj_thermal_trip);
+
+	/* Release the cdevs attached to this zone */
+	i = tz->cdev_indx;
+
+	while (--i >= 0) {
+		sysfs_remove_link(&tz->device.kobj,
+				kobject_name(&tz->cdevs[i]->device.kobj));
+	}
+
+	tz->sensor_indx = tz->cdev_indx = 0;
+
 	release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
 	idr_destroy(&tz->idr);
 
@@ -1864,6 +2062,48 @@ int add_cdev_to_zone(struct thermal_zone *tz,
 }
 EXPORT_SYMBOL(add_cdev_to_zone);
 
+int add_sensor_trip_info(struct thermal_zone *tz, const char *sensor_name,
+			struct thermal_trip_point *trip)
+{
+	int indx, ret = -EINVAL;
+	struct thermal_sensor *ts = get_sensor_by_name(sensor_name);
+
+	if (!tz || !ts || !trip)
+		return ret;
+
+	mutex_lock(&tz_list_lock);
+
+	indx = get_sensor_indx(tz, ts);
+	if (indx < 0)
+		goto exit_indx;
+
+	tz->kobj_trip[indx] = kobject_create_and_add(
+					kobject_name(&ts->device.kobj),
+					tz->kobj_thermal_trip);
+	if (!tz->kobj_trip[indx]) {
+		ret = -ENOMEM;
+		goto exit_indx;
+	}
+
+	ret = sysfs_create_group(tz->kobj_trip[indx], &trip_attr_group);
+	if (ret) {
+		dev_err(&tz->device, "sysfs_create_group failed:%d\n", ret);
+		goto exit_kobj;
+	}
+
+	tz->trip[indx] = trip;
+	mutex_unlock(&tz_list_lock);
+	return 0;
+
+exit_kobj:
+	kobject_del(tz->kobj_trip[indx]);
+	tz->kobj_trip[indx] = NULL;
+exit_indx:
+	mutex_unlock(&tz_list_lock);
+	return ret;
+}
+EXPORT_SYMBOL(add_sensor_trip_info);
+
 /**
  * thermal_sensor_register - register a new thermal sensor
  * @name:	name of the thermal sensor
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 2913910..7b2fe35 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -159,6 +159,30 @@ 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_sensor {
 	char name[THERMAL_NAME_LENGTH];
 	int id;
@@ -215,6 +239,11 @@ struct thermal_zone {
 	/* cdev level information */
 	int cdev_indx; /* index into 'cdevs' array */
 	struct thermal_cooling_device *cdevs[MAX_CDEVS_PER_ZONE];
+
+	/* Thermal trip information */
+	struct thermal_trip_point *trip[MAX_SENSORS_PER_ZONE];
+	struct kobject *kobj_trip[MAX_SENSORS_PER_ZONE];
+	struct kobject *kobj_thermal_trip;
 };
 
 /* Structure that holds thermal governor information */
@@ -298,6 +327,9 @@ int add_sensor_to_zone_by_name(struct thermal_zone *, const char *);
 int add_cdev_to_zone(struct thermal_zone *, struct thermal_cooling_device *);
 int add_cdev_to_zone_by_name(struct thermal_zone *, const char *);
 
+int add_sensor_trip_info(struct thermal_zone *, const char *,
+			struct thermal_trip_point *);
+
 #ifdef CONFIG_NET
 extern int thermal_generate_netlink_event(u32 orig, enum events event);
 #else
-- 
1.7.9.5


^ permalink raw reply related

* [RFC PATCH 5/7] Thermal: Add 'thermal_map' sysfs node
From: Durgadoss R @ 2012-11-17 10:45 UTC (permalink / raw)
  To: rui.zhang, linux-pm
  Cc: wni, eduardo.valentin, amit.kachhap, hongbo.zhang, sachin.kamat,
	Durgadoss R
In-Reply-To: <1353149158-19102-1-git-send-email-durgadoss.r@intel.com>

This patch creates a thermal map sysfs node under
/sys/class/thermal/thermal_zoneX/. This contains
entries named map0, map1 .. mapN. Each map has the
following space separated values:
trip_type sensor_name cdev_name trip_mask weights

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

diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 011588a..af52a16 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -530,6 +530,25 @@ static struct thermal_zone *get_zone_by_trip_kobj(struct kobject *kobj)
 	return tz;
 }
 
+static struct thermal_zone *get_zone_by_map_kobj(struct kobject *kobj)
+{
+	struct thermal_zone *pos;
+	struct thermal_zone *tz = NULL;
+
+	if (!kobj)
+		return NULL;
+
+	mutex_lock(&tz_list_lock);
+	for_each_thermal_zone(pos) {
+		if (pos->kobj_thermal_map == kobj) {
+			tz = pos;
+			break;
+		}
+	}
+	mutex_unlock(&tz_list_lock);
+	return tz;
+}
+
 static struct thermal_sensor *get_sensor_by_kobj_name(const char *name)
 {
 	struct thermal_sensor *pos;
@@ -546,6 +565,41 @@ static struct thermal_sensor *get_sensor_by_kobj_name(const char *name)
 	return ts;
 }
 
+static void __clean_map_entry(struct thermal_zone *tz, int i)
+{
+	tz->map[i] = NULL;
+	sysfs_remove_file(tz->kobj_thermal_map, &tz->map_attr[i]->attr.attr);
+	/* Free map attributes */
+	kfree(tz->map_attr[i]);
+	tz->map_attr[i] = NULL;
+}
+
+static void remove_sensor_map_entry(struct thermal_zone *tz,
+				struct thermal_sensor *ts)
+{
+	int i;
+
+	for (i = 0; i < MAX_MAPS_PER_ZONE; i++) {
+		if (tz->map[i] && !strnicmp(ts->name, tz->map[i]->sensor_name,
+						THERMAL_NAME_LENGTH)) {
+			__clean_map_entry(tz, i);
+		}
+	}
+}
+
+static void remove_cdev_map_entry(struct thermal_zone *tz,
+				struct thermal_cooling_device *cdev)
+{
+	int i;
+
+	for (i = 0; i < MAX_MAPS_PER_ZONE; i++) {
+		if (tz->map[i] && !strnicmp(cdev->type, tz->map[i]->cdev_name,
+						THERMAL_NAME_LENGTH)) {
+			__clean_map_entry(tz, i);
+		}
+	}
+}
+
 /* sys I/F for thermal zone */
 
 #define to_thermal_zone(_dev) \
@@ -901,6 +955,42 @@ policy_show(struct device *dev, struct device_attribute *devattr, char *buf)
 }
 
 static ssize_t
+map_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	int i, indx, ret = 0;
+	struct thermal_zone *tz;
+	struct thermal_map *map;
+	char *trip;
+
+	tz = get_zone_by_map_kobj(kobj);
+	if (!tz)
+		return -EINVAL;
+
+	sscanf(attr->attr.name, "map%d", &indx);
+
+	if (indx < 0 || indx >= MAX_MAPS_PER_ZONE)
+		return -EINVAL;
+
+	if (!tz->map[indx])
+		return sprintf(buf, "<Unavailable>\n");
+
+	map = tz->map[indx];
+
+	trip = (map->trip_type == THERMAL_TRIP_ACTIVE) ?
+					"active" : "passive";
+	ret += sprintf(buf, "%s", trip);
+	ret += sprintf(buf + ret, " %s", map->sensor_name);
+	ret += sprintf(buf + ret, " %s", map->cdev_name);
+	ret += sprintf(buf + ret, " 0x%x", map->trip_mask);
+
+	for (i = 0; i < map->num_weights; i++)
+		ret += sprintf(buf + ret, " %d", map->weights[i]);
+
+	ret += sprintf(buf + ret, "\n");
+	return ret;
+}
+
+static ssize_t
 active_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
 {
 	int i, indx, ret = 0;
@@ -1671,8 +1761,10 @@ void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)
 
 	mutex_lock(&tz_list_lock);
 
-	for_each_thermal_zone(tmp_tz)
+	for_each_thermal_zone(tmp_tz) {
 		remove_cdev_from_zone(tmp_tz, cdev);
+		remove_cdev_map_entry(tmp_tz, cdev);
+	}
 
 	mutex_unlock(&tz_list_lock);
 
@@ -1866,12 +1958,19 @@ struct thermal_zone *create_thermal_zone(const char *name, void *devdata)
 	if (!tz->kobj_thermal_trip)
 		goto exit_name;
 
+	tz->kobj_thermal_map = kobject_create_and_add("thermal_map",
+					&tz->device.kobj);
+	if (!tz->kobj_thermal_map)
+		goto exit_trip;
+
 	/* Add this zone to the global list of thermal zones */
 	mutex_lock(&tz_list_lock);
 	list_add_tail(&tz->node, &thermal_zone_list);
 	mutex_unlock(&tz_list_lock);
 	return tz;
 
+exit_trip:
+	kobject_del(tz->kobj_thermal_trip);
 exit_name:
 	device_remove_file(&tz->device, &dev_attr_zone_name);
 exit_unregister:
@@ -1918,9 +2017,6 @@ void remove_thermal_zone(struct thermal_zone *tz)
 			kobject_del(tz->kobj_trip[i]);
 	}
 
-	if (tz->kobj_thermal_trip)
-		kobject_del(tz->kobj_thermal_trip);
-
 	/* Release the cdevs attached to this zone */
 	i = tz->cdev_indx;
 
@@ -1931,6 +2027,9 @@ void remove_thermal_zone(struct thermal_zone *tz)
 
 	tz->sensor_indx = tz->cdev_indx = 0;
 
+	kobject_del(tz->kobj_thermal_trip);
+	kobject_del(tz->kobj_thermal_map);
+
 	release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
 	idr_destroy(&tz->idr);
 
@@ -1976,6 +2075,53 @@ struct thermal_sensor *get_sensor_by_name(const char *name)
 }
 EXPORT_SYMBOL(get_sensor_by_name);
 
+int add_map_entry(struct thermal_zone *tz, struct thermal_map *map)
+{
+	int ret, indx;
+
+	if (!tz || !map)
+		return -EINVAL;
+
+	mutex_lock(&tz_list_lock);
+
+	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;
+	}
+
+	sprintf(tz->map_attr[indx]->name, "map%d", indx);
+
+	tz->map_attr[indx]->attr.attr.name = tz->map_attr[indx]->name;
+	tz->map_attr[indx]->attr.attr.mode = S_IRUGO;
+	tz->map_attr[indx]->attr.show = map_show;
+
+	sysfs_attr_init(&tz->map_attr[indx]->attr.attr);
+	ret = sysfs_create_file(tz->kobj_thermal_map,
+				&tz->map_attr[indx]->attr.attr);
+	if (ret) {
+		kfree(tz->map_attr[indx]);
+		goto exit;
+	}
+
+	tz->map[indx] = map;
+exit:
+	mutex_unlock(&tz_list_lock);
+	return ret;
+}
+EXPORT_SYMBOL(add_map_entry);
+
 int add_sensor_to_zone_by_name(struct thermal_zone *tz, const char *name)
 {
 	int ret;
@@ -2193,8 +2339,10 @@ void thermal_sensor_unregister(struct thermal_sensor *ts)
 
 	mutex_lock(&tz_list_lock);
 
-	for_each_thermal_zone(tz)
+	for_each_thermal_zone(tz) {
 		remove_sensor_from_zone(tz, ts);
+		remove_sensor_map_entry(tz, ts);
+	}
 
 	mutex_unlock(&tz_list_lock);
 
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 7b2fe35..84763b5 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -56,6 +56,9 @@
 
 #define MAX_CDEVS_PER_ZONE		5
 
+/* 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;
@@ -159,6 +162,21 @@ struct thermal_attr {
 	char name[THERMAL_NAME_LENGTH];
 };
 
+struct thermal_map_attr {
+	struct kobj_attribute 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;
+};
+
 /*
  * This structure defines the trip points for a sensor.
  * The actual values for these trip points come from
@@ -244,6 +262,11 @@ struct thermal_zone {
 	struct thermal_trip_point *trip[MAX_SENSORS_PER_ZONE];
 	struct kobject *kobj_trip[MAX_SENSORS_PER_ZONE];
 	struct kobject *kobj_thermal_trip;
+
+	/* Thermal map information */
+	struct thermal_map *map[MAX_MAPS_PER_ZONE];
+	struct thermal_map_attr *map_attr[MAX_MAPS_PER_ZONE];
+	struct kobject *kobj_thermal_map;
 };
 
 /* Structure that holds thermal governor information */
@@ -330,6 +353,8 @@ int add_cdev_to_zone_by_name(struct thermal_zone *, const char *);
 int add_sensor_trip_info(struct thermal_zone *, const char *,
 			struct thermal_trip_point *);
 
+int add_map_entry(struct thermal_zone *, struct thermal_map *);
+
 #ifdef CONFIG_NET
 extern int thermal_generate_netlink_event(u32 orig, enum events event);
 #else
-- 
1.7.9.5


^ permalink raw reply related

* [RFC PATCH 6/7] Thermal: Add Documentation to new APIs
From: Durgadoss R @ 2012-11-17 10:45 UTC (permalink / raw)
  To: rui.zhang, linux-pm
  Cc: wni, eduardo.valentin, amit.kachhap, hongbo.zhang, sachin.kamat,
	Durgadoss R
In-Reply-To: <1353149158-19102-1-git-send-email-durgadoss.r@intel.com>

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 <durgadoss.r@intel.com>
---
 Documentation/thermal/sysfs-api2.txt |  213 ++++++++++++++++++++++++++++++++++
 1 file changed, 213 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..be67125
--- /dev/null
+++ b/Documentation/thermal/sysfs-api2.txt
@@ -0,0 +1,213 @@
+Thermal Framework
+-----------------
+
+Written by Durgadoss R <durgadoss.r@intel.com>
+Copyright (c) 2012 Intel Corporation
+
+Created on: 4 November 2012
+
+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.
+
+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
+	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
+
+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.
+	Both the MAX_*_PER_ZONE values are 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/thermal_trip/
+	This API creates 4 sysfs attributes namely active, passive, hot,
+	and critical. Each of these hold one or more trip point temperature
+	values, as provided from platform data.
+
+	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/thermal_map/. Each map holds a space
+	separated list of values, that specify 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.
+
+	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/thermal_zone[0-*]:
+		|---type:		Name of the thermal sensor
+		|---temp_input:		Current temperature in mC
+		|---threshold[0-*]:	Threshold temperature
+
+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
+		|---thermal_trip:	trip point values for sensors
+		|---thermal_map:	mapping info between sensors and cdevs
+
+3.4: For Thermal Trip
+	This attribute represents the trip point values for all sensors
+	present in the thermal zone. All values are in mC.
+	/sys/class/thermal/zoneX/thermal_trip/sensorY:
+		|---hot:		hot trip point value
+		|---critical:		critical trip point value
+		|---passive:		list of passive trip point values
+		|---active:		list of active trip point values
+
+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/thermal_map/:
+		|---mapX:		mapping information
+	A typical map entry is like below:
+
+	trip_type  sensor  cdev  trip_mask  weight(s)
+	passive    cpu     proc  0x03       50 30
+	active     cpu     fan0  0x03       50 70
+
+	The trip mask is a bit string; if 'n' th bit is set, then for
+	trip point 'n' this cdev is throttled with the given weight[n].
-- 
1.7.9.5


^ permalink raw reply related

* [RFC PATCH 7/7] Thermal: Dummy driver used for testing
From: Durgadoss R @ 2012-11-17 10:45 UTC (permalink / raw)
  To: rui.zhang, linux-pm
  Cc: wni, eduardo.valentin, amit.kachhap, hongbo.zhang, sachin.kamat,
	Durgadoss R
In-Reply-To: <1353149158-19102-1-git-send-email-durgadoss.r@intel.com>

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

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/thermal/Kconfig        |    5 +
 drivers/thermal/Makefile       |    3 +
 drivers/thermal/thermal_test.c |  321 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 329 insertions(+)
 create mode 100644 drivers/thermal/thermal_test.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index d96da07..9a083ae 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -122,4 +122,9 @@ config DB8500_CPUFREQ_COOLING
 	  bound cpufreq cooling device turns active to set CPU frequency low to
 	  cool down the CPU.
 
+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 d8da683..02c3edb 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -18,3 +18,6 @@ obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
 obj-$(CONFIG_EXYNOS_THERMAL)	+= exynos_thermal.o
 obj-$(CONFIG_DB8500_THERMAL)	+= db8500_thermal.o
 obj-$(CONFIG_DB8500_CPUFREQ_COOLING)	+= db8500_cpufreq_cooling.o
+
+# 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..c1940b9
--- /dev/null
+++ b/drivers/thermal/thermal_test.c
@@ -0,0 +1,321 @@
+/*
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * Author: Durgadoss R <durgadoss.r@intel.com>
+ */
+
+#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};
+
+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,
+	.active_trip_mask = 0xCFF,
+};
+
+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,
+	.active_trip_mask = 0xFF,
+};
+
+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", &ts_ops, &ts_data);
+	if (!ts) {
+		pr_err("thermal_sensor_register failed:\n");
+		return -EINVAL;
+	}
+
+	ts1 = thermal_sensor_register("ts1", &ts1_ops, NULL);
+
+	cdev = thermal_cooling_device_register("cdev", NULL, &cdev_ops);
+	if (!cdev) {
+		pr_err("cdev_register failed:\n");
+		return -EINVAL;
+	}
+
+	ret = enable_sensor_thresholds(ts, NUM_THRESHOLDS);
+	if (ret) {
+		pr_err("enable_sensor_thresholds failed:%d\n", ret);
+		return ret;
+	}
+
+	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 ret;
+	}
+
+	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);
+	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 = __devexit_p(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 <durgadoss.r@intel.com>");
+MODULE_DESCRIPTION("A dummy driver to test Thermal Framework");
+MODULE_LICENSE("GPL");
-- 
1.7.9.5


^ permalink raw reply related

* [PATCHv9 0/3] Runtime Interpreted Power Sequences
From: Alexandre Courbot @ 2012-11-17 10:55 UTC (permalink / raw)
  To: Anton Vorontsov, Stephen Warren, Thierry Reding, Mark Zhang,
	Grant Likely, Rob Herring, Mark Brown, David Woodhouse,
	Arnd Bergmann
  Cc: linux-tegra, linux-arm-kernel, linux-kernel, linux-fbdev,
	devicetree-discuss, linux-pm, Alexandre Courbot,
	Alexandre Courbot

Apologies for sending two patchsets in two days - the main purpose
of this new revision is to add the linux-arm-kernel list to the
audience. A few suggestions from v8 have also been added.

Changelog from v8:
- Compile resource support into different compilation units
- Check that resource support is compiled in when resolving sequences
- Now compilable as a module
- Renamed source files to avoid repeated power_seq in their path
- Add linux-arm-kernel list to audience as suggested by Mark Rutland

Alexandre Courbot (3):
  Runtime Interpreted Power Sequences
  pwm_backlight: use power sequences
  Take maintainership of power sequences

 .../devicetree/bindings/power/power_seq.txt        | 121 +++++++
 .../bindings/video/backlight/pwm-backlight.txt     |  63 +++-
 Documentation/power/power_seq.txt                  | 253 ++++++++++++++
 MAINTAINERS                                        |  10 +
 drivers/power/Kconfig                              |   1 +
 drivers/power/Makefile                             |   1 +
 drivers/power/power_seq/Kconfig                    |   2 +
 drivers/power/power_seq/Makefile                   |   2 +
 drivers/power/power_seq/core.c                     | 362 +++++++++++++++++++++
 drivers/power/power_seq/delay.c                    |  66 ++++
 drivers/power/power_seq/gpio.c                     |  95 ++++++
 drivers/power/power_seq/power_seq_priv.h           |  56 ++++
 drivers/power/power_seq/pwm.c                      |  85 +++++
 drivers/power/power_seq/regulator.c                |  87 +++++
 drivers/video/backlight/Kconfig                    |   1 +
 drivers/video/backlight/pwm_bl.c                   | 160 +++++++--
 include/linux/power_seq.h                          | 203 ++++++++++++
 include/linux/pwm_backlight.h                      |  18 +-
 18 files changed, 1546 insertions(+), 40 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/power_seq.txt
 create mode 100644 Documentation/power/power_seq.txt
 create mode 100644 drivers/power/power_seq/Kconfig
 create mode 100644 drivers/power/power_seq/Makefile
 create mode 100644 drivers/power/power_seq/core.c
 create mode 100644 drivers/power/power_seq/delay.c
 create mode 100644 drivers/power/power_seq/gpio.c
 create mode 100644 drivers/power/power_seq/power_seq_priv.h
 create mode 100644 drivers/power/power_seq/pwm.c
 create mode 100644 drivers/power/power_seq/regulator.c
 create mode 100644 include/linux/power_seq.h

-- 
1.8.0

^ permalink raw reply

* [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Alexandre Courbot @ 2012-11-17 10:55 UTC (permalink / raw)
  To: Anton Vorontsov, Stephen Warren, Thierry Reding, Mark Zhang,
	Grant Likely, Rob Herring, Mark Brown, David Woodhouse,
	Arnd Bergmann
  Cc: Alexandre Courbot, linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1353149747-31871-1-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Some device drivers (e.g. panel or backlights) need to follow precise
sequences for powering on and off, involving GPIOs, regulators, PWMs
and other power-related resources, with a defined powering order and
delays to respect between steps. These sequences are device-specific,
and do not belong to a particular driver - therefore they have been
performed by board-specific hook functions so far.

With the advent of the device tree and of ARM kernels that are not
board-tied, we cannot rely on these board-specific hooks anymore but
need a way to implement these sequences in a portable manner. This patch
introduces a simple interpreter that can execute such power sequences
encoded either as platform data or within the device tree. It also
introduces first support for regulator, GPIO and PWM resources.

Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
Reviewed-by: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
Tested-by: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
Tested-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Acked-by: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
---
 .../devicetree/bindings/power/power_seq.txt        | 121 +++++++
 Documentation/power/power_seq.txt                  | 253 ++++++++++++++
 drivers/power/Kconfig                              |   1 +
 drivers/power/Makefile                             |   1 +
 drivers/power/power_seq/Kconfig                    |   2 +
 drivers/power/power_seq/Makefile                   |   2 +
 drivers/power/power_seq/core.c                     | 362 +++++++++++++++++++++
 drivers/power/power_seq/delay.c                    |  66 ++++
 drivers/power/power_seq/gpio.c                     |  95 ++++++
 drivers/power/power_seq/power_seq_priv.h           |  56 ++++
 drivers/power/power_seq/pwm.c                      |  85 +++++
 drivers/power/power_seq/regulator.c                |  87 +++++
 include/linux/power_seq.h                          | 203 ++++++++++++
 13 files changed, 1334 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/power_seq.txt
 create mode 100644 Documentation/power/power_seq.txt
 create mode 100644 drivers/power/power_seq/Kconfig
 create mode 100644 drivers/power/power_seq/Makefile
 create mode 100644 drivers/power/power_seq/core.c
 create mode 100644 drivers/power/power_seq/delay.c
 create mode 100644 drivers/power/power_seq/gpio.c
 create mode 100644 drivers/power/power_seq/power_seq_priv.h
 create mode 100644 drivers/power/power_seq/pwm.c
 create mode 100644 drivers/power/power_seq/regulator.c
 create mode 100644 include/linux/power_seq.h

diff --git a/Documentation/devicetree/bindings/power/power_seq.txt b/Documentation/devicetree/bindings/power/power_seq.txt
new file mode 100644
index 0000000..7880a6c
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/power_seq.txt
@@ -0,0 +1,121 @@
+Runtime Interpreted Power Sequences
+===================================
+
+Power sequences are sequential descriptions of actions to be performed on
+power-related resources. Having these descriptions in a well-defined data format
+allows us to take much of the board- or device- specific power control code out
+of the kernel and place it into the device tree instead, making kernels less
+board-dependant.
+
+A device typically makes use of multiple power sequences, for different purposes
+such as powering on and off. All the power sequences of a given device are
+grouped into a set. In the device tree, this set is a sub-node of the device
+node named "power-sequences".
+
+Power Sequences Structure
+-------------------------
+Every device that makes use of power sequences must have a "power-sequences"
+node into which individual power sequences are declared as sub-nodes. The name
+of the node becomes the name of the sequence within the power sequences
+framework.
+
+Similarly, each power sequence declares its steps as sub-nodes of itself. Steps
+must be named sequentially, with the first step named step0, the second step1,
+etc. Failure to follow this rule will result in a parsing error.
+
+Power Sequences Steps
+---------------------
+Steps of a sequence describe an action to be performed on a resource. They
+always include a "type" property which indicates what kind of resource this
+step works on. Depending on the resource type, additional properties are defined
+to control the action to be performed.
+
+"delay" type required properties:
+  - delay: delay to wait (in microseconds)
+
+"regulator" type required properties:
+  - id: name of the regulator to use.
+  - enable / disable: one of these two empty properties must be present to
+                      enable or disable the resource
+
+"pwm" type required properties:
+  - id: name of the PWM to use.
+  - enable / disable: one of these two empty properties must be present to
+                      enable or disable the resource
+
+"gpio" type required properties:
+  - gpio: phandle of the GPIO to use.
+  - value: value this GPIO should take. Must be 0 or 1.
+
+Example
+-------
+Here are example sequences declared within a backlight device that use all the
+supported resources types:
+
+	backlight {
+		compatible = "pwm-backlight";
+		...
+
+		/* resources used by the power sequences */
+		pwms = <&pwm 2 5000000>;
+		pwm-names = "backlight";
+		power-supply = <&backlight_reg>;
+
+		power-sequences {
+			power-on {
+				step0 {
+					type = "regulator";
+					id = "power";
+					enable;
+				};
+				step1 {
+					type = "delay";
+					delay = <10000>;
+				};
+				step2 {
+					type = "pwm";
+					id = "backlight";
+					enable;
+				};
+				step3 {
+					type = "gpio";
+					gpio = <&gpio 28 0>;
+					value = <1>;
+				};
+			};
+
+			power-off {
+				step0 {
+					type = "gpio";
+					gpio = <&gpio 28 0>;
+					value = <0>;
+				};
+				step1 {
+					type = "pwm";
+					id = "backlight";
+					disable;
+				};
+				step2 {
+					type = "delay";
+					delay = <10000>;
+				};
+				step3 {
+					type = "regulator";
+					id = "power";
+					disable;
+				};
+			};
+		};
+	};
+
+The first part lists the PWM and regulator resources used by the sequences.
+These resources will be requested on behalf of the backlight device when the
+sequences are built and are declared according to their own bindings (for
+instance, regulators and pwms are resolved by name - note though that name
+declaration is done differently by the two frameworks).
+
+After the resources declaration, two sequences follow for powering the backlight
+on and off. Their names are specified by the pwm-backlight device bindings. Once
+the sequences are built by calling devm_of_parse_power_seq_set() on the
+backlight device, they can be added to a set using
+power_seq_set_add_sequences().
diff --git a/Documentation/power/power_seq.txt b/Documentation/power/power_seq.txt
new file mode 100644
index 0000000..8be0570
--- /dev/null
+++ b/Documentation/power/power_seq.txt
@@ -0,0 +1,253 @@
+Runtime Interpreted Power Sequences
+===================================
+
+Problem
+-------
+Very commonly, boards need the help of out-of-driver code to turn some of their
+devices on and off. For instance, SoC boards might use a GPIO (abstracted to a
+regulator or not) to control the power supply of a backlight. The GPIO that
+should be used, however, as well as the exact power sequence that may also
+involve other resources, is board-dependent and thus unknown to the driver.
+
+This was previously addressed by having hooks in the device's platform data that
+are called whenever the state of the device might need a power status change.
+This approach, however, introduces board-dependant code into the kernel and is
+not compatible with the device tree.
+
+The Runtime Interpreted Power Sequences (or power sequences for short) aim at
+turning this code into platform data or device tree nodes. Power sequences are
+described using a simple format and run by a lightweight interpreter whenever
+needed. This allows device drivers to work without power callbacks and makes the
+kernel less board-dependant.
+
+What are Power Sequences?
+-------------------------
+A power sequence is an array of sequential steps describing an action to be
+performed on a resource. The supported resources and actions operations are:
+- delay (just wait for a given number of microseconds)
+- GPIO (set to 0 or 1)
+- regulator (enable or disable)
+- PWM (enable or disable)
+
+When a power sequence is run, its steps is executed one after the other until
+one step fails or the end of the sequence is reached.
+
+Power sequences are named, and grouped into "sets" which contain all the
+sequences of a device as well as the resources they use.
+
+Power sequences can be declared as platform data or in the device tree.
+
+Platform Data Format
+--------------------
+All relevant data structures for declaring power sequences are located in
+include/linux/power_seq.h.
+
+The platform data for a device may include an instance of platform_power_seq_set
+which references all the power sequences used for a device. The power sequences
+reference resources in their steps, and setup the union member that corresponds
+to the resource's type. Resources, similarly, have a union which relevant member
+depends on their type.
+
+Note that the only "platform data" per se here is platform_power_seq_set. Other
+structures (power_seq and power_seq_resource) will be used at runtime and thus
+*must* survive initialization, so do not declare them with the __initdata
+attribute.
+
+The following example should make it clear how the platform data for power
+sequences is defined. It declares two power sequences named "power-on" and
+"power-off" for a backlight device. The "power-on" sequence enables the "power"
+regulator of the device, waits for 10ms, and then enables PWM "backlight" and
+set GPIO 28 to 1. "power-off" does the opposite.
+
+struct power_seq_resource reg_res = {
+	.type = POWER_SEQ_REGULATOR,
+	.regulator.id = "power",
+};
+
+struct power_seq_resource gpio_res = {
+	.type = POWER_SEQ_GPIO,
+	.gpio.gpio = 28,
+};
+
+struct power_seq_resource pwm_res = {
+	.type = POWER_SEQ_PWM,
+	.pwm.id = "backlight",
+};
+
+struct power_seq_resource delay_res = {
+	.type = POWER_SEQ_DELAY,
+};
+
+struct power_seq power_on_seq = {
+	.id = "power-on",
+	.num_steps = 4,
+	.steps = {
+		{
+			.resource = &reg_res,
+			.regulator.enable = true,
+		}, {
+			.resource = &delay_res,
+			.delay.delay = 10000,
+		}, {
+			.resource = &pwm_res,
+			.pwm.enable = true,
+		}, {
+			.resource = &gpio_res,
+			.gpio.value = 1,
+		},
+	},
+};
+
+struct power_seq power_off_seq = {
+	.id = "power-off",
+	.num_steps = 4,
+	.steps = {
+		{
+			.resource = &gpio_res,
+			.gpio.value = 0,
+		}, {
+			.resource = &pwm_res,
+			.pwm.enable = false,
+		}, {
+			.resource = &delay_res,
+			.delay.delay = 10000,
+		}, {
+			.resource = &reg_res,
+			.regulator.enable = false,
+		},
+	},
+};
+
+struct platform_power_seq_set backlight_power_seqs __initdata = {
+	.num_seqs = 2,
+	.seqs = {
+		&power_on_seq,
+		&power_off_seq,
+	},
+};
+
+"backlight_power_seqs" can then be passed to power_seq_set_add_sequences() in
+order to add the sequences to a set and allocate all the necessary resources.
+More on this later in this document.
+
+Device Tree
+-----------
+Power sequences can also be encoded as device tree nodes. The following
+properties and nodes are equivalent to the platform data defined previously:
+
+pwms = <&pwm 2 5000000>;
+pwm-names = "backlight";
+power-supply = <&vdd_bl_reg>;
+
+power-sequences {
+	power-on {
+		step0 {
+			type = "regulator";
+			id = "power";
+			enable;
+		};
+		step1 {
+			type = "delay";
+			delay = <10000>;
+		};
+		step2 {
+			type = "pwm";
+			id = "backlight";
+			enable;
+		};
+		step3 {
+			type = "gpio";
+			gpio = <&gpio 28 0>;
+			value = <1>;
+		};
+	};
+
+	power-off {
+		step0 {
+			type = "gpio";
+			gpio = <&gpio 28 0>;
+			value = <0>;
+		};
+		step1 {
+			type = "pwm";
+			id = "backlight";
+			disable;
+		};
+		step2 {
+			type = "delay";
+			delay = <10000>;
+		};
+		step3 {
+			type = "regulator";
+			id = "power";
+			disable;
+		};
+	};
+};
+
+See Documentation/devicetree/bindings/power/power_seq.txt for the complete
+syntax of the DT bindings.
+
+Use by Drivers and Resources Management
+---------------------------------------
+Power sequences make use of resources that must be properly allocated and
+managed. The power_seq_set structure manages the sequences and resources for a
+particular device. A driver willing to use power sequences will thus declare one
+instance of power_seq_set per device and initialize it at probe time:
+
+struct my_device_data {
+	struct device *dev;
+	...
+	struct power_set_set power_seqs;
+	...
+};
+
+power_seq_set_init(&my_device->power_seqs, my_device->dev);
+
+The power_seq_set_add_sequence() and power_seq_set_add_sequences() functions are
+then used to add one or several sequences to a set. These functions will also
+allocate all the resources used by the sequence(s) and make sure they are ready
+to be run. All resources are allocated through devm and will thus be freed when
+the set's device is removed.
+
+  int power_seq_set_add_sequence(struct power_seq_set *set,
+			         struct power_seq *seq);
+  int power_seq_set_add_sequences(struct power_seq_set *set,
+				  struct platform_power_seq_set *seqs);
+
+Power sequences added to a set can then be resolved by their name using
+power_seq_lookup():
+
+  struct power_seq *power_seq_lookup(struct power_seq_set *seqs,
+				     const char *id);
+
+power_seq_lookup() returns a ready-to-run pointer to the power sequence which
+name matches the id parameter.
+
+A retrieved power sequence can then be executed by power_seq_run:
+
+  int power_seq_run(struct power_seq *seq);
+
+It returns 0 if the sequence has successfully been run, or an error code if a
+problem occurred.
+
+Sometimes, you may want to browse the list of resources allocated for the
+sequences of a device, for instance to ensure that a resource of a given type is
+present. The power_seq_for_each_resource() macro does this:
+
+  power_seq_for_each_resource(pos, seqs)
+
+Here "pos" will be a pointer to a struct power_seq_resource. This structure
+contains the type of the resource, the information used for identifying it, and
+the resolved resource itself.
+
+Finally, users of the device tree can obtain a platform_power_seq_set structure
+built from the device's node using devm_of_parse_power_seq_set:
+
+  struct platform_power_seq_set *devm_of_parse_power_seq_set(struct device *dev);
+
+The power sequences must be declared under a "power-sequences" node directly
+declared under the device's node. Detailed syntax contained in Documentation/devicetree/bindings/power/power_seq.txt. As the function name
+states, all memory is allocated through devm. The returned
+platform_power_seq_set can be freed after being added to a set, but the
+sequences themselves must be preserved until they are freed by devm.
\ No newline at end of file
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 49a8939..f20d449 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -338,3 +338,4 @@ config AB8500_BATTERY_THERM_ON_BATCTRL
 endif # POWER_SUPPLY
 
 source "drivers/power/avs/Kconfig"
+source "drivers/power/power_seq/Kconfig"
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index b949cf8..883ad4d 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -49,3 +49,4 @@ obj-$(CONFIG_CHARGER_MAX8997)	+= max8997_charger.o
 obj-$(CONFIG_CHARGER_MAX8998)	+= max8998_charger.o
 obj-$(CONFIG_POWER_AVS)		+= avs/
 obj-$(CONFIG_CHARGER_SMB347)	+= smb347-charger.o
+obj-$(CONFIG_POWER_SEQ)		+= power_seq/
diff --git a/drivers/power/power_seq/Kconfig b/drivers/power/power_seq/Kconfig
new file mode 100644
index 0000000..0ece819
--- /dev/null
+++ b/drivers/power/power_seq/Kconfig
@@ -0,0 +1,2 @@
+config POWER_SEQ
+	tristate
diff --git a/drivers/power/power_seq/Makefile b/drivers/power/power_seq/Makefile
new file mode 100644
index 0000000..964b91e
--- /dev/null
+++ b/drivers/power/power_seq/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_POWER_SEQ)		+= power_seq.o
+power_seq-y			:= core.o delay.o regulator.o gpio.o pwm.o
diff --git a/drivers/power/power_seq/core.c b/drivers/power/power_seq/core.c
new file mode 100644
index 0000000..d51b9b8
--- /dev/null
+++ b/drivers/power/power_seq/core.c
@@ -0,0 +1,362 @@
+/*
+ * core.c - power sequence interpreter for platform devices and device tree
+ *
+ * Author: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
+ *
+ * Copyright (c) 2012 NVIDIA 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.
+ *
+ */
+
+#include <linux/power_seq.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/device.h>
+#include <linux/of.h>
+
+#include "power_seq_priv.h"
+
+static const struct power_seq_res_ops *power_seq_ops[POWER_SEQ_NUM_TYPES];
+
+#ifdef CONFIG_OF
+int of_power_seq_parse_enable_properties(struct device_node *node,
+					 struct power_seq *seq,
+					 unsigned int step_nbr, bool *enable)
+{
+	if (of_find_property(node, "enable", NULL)) {
+		*enable = true;
+	} else if (of_find_property(node, "disable", NULL)) {
+		*enable = false;
+	} else {
+		power_seq_err(seq, step_nbr,
+			      "missing enable or disable property\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int of_power_seq_parse_step(struct device *dev,
+				   struct device_node *node,
+				   struct power_seq *seq,
+				   unsigned int step_nbr,
+				   struct list_head *resources)
+{
+	struct power_seq_step *step = &seq->steps[step_nbr];
+	struct power_seq_resource res, *res2;
+	const char *type;
+	unsigned int i;
+	int err;
+
+	err = of_property_read_string(node, "type", &type);
+	if (err < 0) {
+		power_seq_err(seq, step_nbr, "cannot read type property\n");
+		return err;
+	}
+	for (i = 0; i < POWER_SEQ_NUM_TYPES; i++) {
+		if (power_seq_ops[i]->name == NULL)
+			continue;
+		if (!strcmp(type, power_seq_ops[i]->name))
+			break;
+	}
+	if (i >= POWER_SEQ_NUM_TYPES) {
+		power_seq_err(seq, step_nbr, "unknown type %s\n", type);
+		return -EINVAL;
+	}
+	memset(&res, 0, sizeof(res));
+	res.type = i;
+	err = power_seq_ops[res.type]->of_parse(node, seq, step_nbr, &res);
+	if (err < 0)
+		return err;
+
+	/* Use the same instance of the resource if met before */
+	list_for_each_entry(res2, resources, list) {
+		if (res.type == res2->type &&
+		    power_seq_ops[res.type]->res_compare(&res, res2))
+			break;
+	}
+	/* Resource never met before, create it */
+	if (&res2->list == resources) {
+		res2 = devm_kzalloc(dev, sizeof(*res2), GFP_KERNEL);
+		if (!res2)
+			return -ENOMEM;
+		memcpy(res2, &res, sizeof(res));
+		list_add_tail(&res2->list, resources);
+	}
+	step->resource = res2;
+
+	return 0;
+}
+
+static struct power_seq *of_parse_power_seq(struct device *dev,
+					    struct device_node *node,
+					    struct list_head *resources)
+{
+	struct device_node *child = NULL;
+	struct power_seq *pseq;
+	unsigned int sz;
+	int num_steps;
+	int err;
+
+	if (!node)
+		return ERR_PTR(-EINVAL);
+
+	num_steps = of_get_child_count(node);
+	sz = sizeof(*pseq) + sizeof(pseq->steps[0]) * num_steps;
+	pseq = devm_kzalloc(dev, sz, GFP_KERNEL);
+	if (!pseq)
+		return ERR_PTR(-ENOMEM);
+	pseq->id = node->name;
+	pseq->num_steps = num_steps;
+
+	for_each_child_of_node(node, child) {
+		unsigned int pos;
+
+		/* Check that the name's format is correct and within bounds */
+		if (strncmp("step", child->name, 4)) {
+			err = -EINVAL;
+			goto parse_error;
+		}
+
+		err = kstrtouint(child->name + 4, 10, &pos);
+		if (err < 0)
+			goto parse_error;
+
+		/* Invalid step index or step already parsed? */
+		if (pos >= num_steps || pseq->steps[pos].resource != NULL) {
+			err = -EINVAL;
+			goto parse_error;
+		}
+
+		err = of_power_seq_parse_step(dev, child, pseq, pos, resources);
+		if (err)
+			return ERR_PTR(err);
+	}
+
+	return pseq;
+
+parse_error:
+	dev_err(dev, "%s: invalid power step name %s!\n", pseq->id,
+		child->name);
+	return ERR_PTR(err);
+}
+
+/**
+ * devm_of_parse_power_seq_set - build a power_seq_set from the device tree
+ * @dev:	Device to parse the power sequences of
+ *
+ * Sequences must be contained into a subnode named "power-sequences" of the
+ * device root node.
+ *
+ * Memory for the sequence is allocated using devm_kzalloc on dev. The returned
+ * platform_power_seq_set can be freed by devm_kfree after the sequences have
+ * been added, but the sequences themselves must be preserved.
+ *
+ * Returns the built set on success, or an error code in case of failure.
+ */
+struct platform_power_seq_set *devm_of_parse_power_seq_set(struct device *dev)
+{
+	struct platform_power_seq_set *set;
+	struct device_node *root = dev->of_node;
+	struct device_node *seq;
+	struct list_head resources;
+	unsigned int sz;
+	int n;
+
+	if (!root)
+		return NULL;
+
+	root = of_find_node_by_name(root, "power-sequences");
+	if (!root)
+		return NULL;
+
+	n = of_get_child_count(root);
+	sz = sizeof(*set) + sizeof(struct power_seq *) * n;
+	set = devm_kzalloc(dev, sz, GFP_KERNEL);
+	if (!set)
+		return ERR_PTR(-ENOMEM);
+	set->num_seqs = n;
+
+	n = 0;
+	INIT_LIST_HEAD(&resources);
+	for_each_child_of_node(root, seq) {
+		struct power_seq *pseq;
+
+		pseq = of_parse_power_seq(dev, seq, &resources);
+		if (IS_ERR(pseq))
+			return (void *)pseq;
+
+		set->seqs[n++] = pseq;
+	}
+
+	return set;
+}
+EXPORT_SYMBOL_GPL(devm_of_parse_power_seq_set);
+#endif /* CONFIG_OF */
+
+/**
+ * power_seq_set_init - initialize a power_seq_set
+ * @set:	Set to initialize
+ * @dev:	Device this set is going to belong to
+ */
+void power_seq_set_init(struct power_seq_set *set, struct device *dev)
+{
+	set->dev = dev;
+	INIT_LIST_HEAD(&set->resources);
+	INIT_LIST_HEAD(&set->seqs);
+}
+EXPORT_SYMBOL_GPL(power_seq_set_init);
+
+/**
+ * power_seq_add_sequence - add a power sequence to a set
+ * @set:	Set to add the sequence to
+ * @seq:	Sequence to add
+ *
+ * This step will check that all the resources used by the sequence are
+ * allocated. If they are not, an attempt to allocate them is made. This
+ * operation can fail and and return an error code.
+ *
+ * Returns 0 on success, error code if a resource initialization failed.
+ */
+int power_seq_add_sequence(struct power_seq_set *set, struct power_seq *seq)
+{
+	struct power_seq_resource *res;
+	unsigned int i;
+	int err;
+
+	for (i = 0; i < seq->num_steps; i++) {
+		struct power_seq_step *step = &seq->steps[i];
+		struct power_seq_resource *step_res = step->resource;
+
+		/* support for resource type not compiled in? */
+		if (power_seq_ops[step_res->type]->name == NULL) {
+			power_seq_err(seq, i, "unsupported resource type\n");
+			return -EINVAL;
+		}
+
+		/* resource already allocated from a previous step ? */
+		list_for_each_entry(res, &set->resources, list) {
+			if (res == step_res)
+				break;
+		}
+		/* resource not allocated yet, allocate and add it */
+		if (&res->list == &set->resources) {
+			err = power_seq_ops[step_res->type]->res_alloc(set->dev,
+								      step_res);
+			if (err)
+				return err;
+			list_add_tail(&step->resource->list, &set->resources);
+		}
+	}
+
+	list_add_tail(&seq->list, &set->seqs);
+	seq->set = set;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(power_seq_add_sequence);
+
+/**
+ * power_seq_add_sequences - add power sequences defined as platform data
+ * @set:	Set to add the sequences to
+ * @seqs:	Sequences to add
+ *
+ * See power_seq_add_sequence for more details.
+ *
+ * Returns 0 on success, error code if a resource initialization failed.
+ */
+int power_seq_set_add_sequences(struct power_seq_set *set,
+				struct platform_power_seq_set *seqs)
+{
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < seqs->num_seqs; i++) {
+		ret = power_seq_add_sequence(set, seqs->seqs[i]);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(power_seq_set_add_sequences);
+
+/**
+ * power_seq_lookup - Lookup a power sequence by name from a set
+ * @seqs:	The set to look in
+ * @id:		Name to look after
+ *
+ * Returns a matching power sequence if it exists, NULL if it does not.
+ */
+struct power_seq *power_seq_lookup(struct power_seq_set *set, const char *id)
+{
+	struct power_seq *seq;
+
+	list_for_each_entry(seq, &set->seqs, list) {
+		if (!strcmp(seq->id, id))
+			return seq;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(power_seq_lookup);
+
+/**
+ * power_seq_run() - run a power sequence
+ * @seq:	The power sequence to run
+ *
+ * Returns 0 on success, error code in case of failure.
+ */
+int power_seq_run(struct power_seq *seq)
+{
+	unsigned int i;
+	int err;
+
+	if (!seq)
+		return 0;
+
+	if (!seq->set) {
+		pr_err("cannot run a sequence not added to a set");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < seq->num_steps; i++) {
+		unsigned int type = seq->steps[i].resource->type;
+
+		err = power_seq_ops[type]->step_run(&seq->steps[i]);
+		if (err) {
+			power_seq_err(seq, i,
+				"error %d while running power sequence step\n",
+				err);
+			return err;
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(power_seq_run);
+
+/* defined in power_seq_*.c files */
+extern const struct power_seq_res_ops power_seq_delay_ops;
+extern const struct power_seq_res_ops power_seq_gpio_ops;
+extern const struct power_seq_res_ops power_seq_regulator_ops;
+extern const struct power_seq_res_ops power_seq_pwm_ops;
+
+static const struct power_seq_res_ops *power_seq_ops[POWER_SEQ_NUM_TYPES] = {
+	[POWER_SEQ_DELAY] = &power_seq_delay_ops,
+	[POWER_SEQ_REGULATOR] = &power_seq_regulator_ops,
+	[POWER_SEQ_PWM] = &power_seq_gpio_ops,
+	[POWER_SEQ_GPIO] = &power_seq_pwm_ops,
+};
+
+MODULE_AUTHOR("Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>");
+MODULE_DESCRIPTION("Runtime Interpreted Power Sequences");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/power/power_seq/delay.c b/drivers/power/power_seq/delay.c
new file mode 100644
index 0000000..de6810b
--- /dev/null
+++ b/drivers/power/power_seq/delay.c
@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) 2012 NVIDIA 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.
+ *
+ */
+
+#include "power_seq_priv.h"
+#include <linux/delay.h>
+
+#ifdef CONFIG_OF
+static int of_power_seq_parse_delay(struct device_node *node,
+				    struct power_seq *seq,
+				    unsigned int step_nbr,
+				    struct power_seq_resource *res)
+{
+	struct power_seq_step *step = &seq->steps[step_nbr];
+	int err;
+
+	err = of_property_read_u32(node, "delay",
+				   &step->delay.delay);
+	if (err < 0)
+		power_seq_err(seq, step_nbr, "error reading delay property\n");
+
+	return err;
+}
+#else
+#define of_power_seq_parse_delay NULL
+#endif
+
+static bool power_seq_res_compare_delay(struct power_seq_resource *res,
+					struct power_seq_resource *res2)
+{
+	/* Delay resources are just here to hold the type of steps, so they are
+	 * all equivalent. */
+	return true;
+}
+
+static int power_seq_res_alloc_delay(struct device *dev,
+				     struct power_seq_resource *res)
+{
+	return 0;
+}
+
+static int power_seq_step_run_delay(struct power_seq_step *step)
+{
+	usleep_range(step->delay.delay,
+		     step->delay.delay + 1000);
+
+	return 0;
+}
+
+const struct power_seq_res_ops power_seq_delay_ops = {
+	.name = "delay",
+	.of_parse = of_power_seq_parse_delay,
+	.step_run = power_seq_step_run_delay,
+	.res_compare = power_seq_res_compare_delay,
+	.res_alloc = power_seq_res_alloc_delay,
+};
diff --git a/drivers/power/power_seq/gpio.c b/drivers/power/power_seq/gpio.c
new file mode 100644
index 0000000..0cd143a
--- /dev/null
+++ b/drivers/power/power_seq/gpio.c
@@ -0,0 +1,95 @@
+/*
+ * Copyright (c) 2012 NVIDIA 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.
+ *
+ */
+
+#include "power_seq_priv.h"
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+
+#ifdef CONFIG_OF
+static int of_power_seq_parse_gpio(struct device_node *node,
+				   struct power_seq *seq,
+				   unsigned int step_nbr,
+				   struct power_seq_resource *res)
+{
+	struct power_seq_step *step = &seq->steps[step_nbr];
+	int gpio;
+	int err;
+
+	gpio = of_get_named_gpio(node, "gpio", 0);
+	if (gpio < 0) {
+		power_seq_err(seq, step_nbr, "error reading gpio property\n");
+		return gpio;
+	}
+	res->gpio.gpio = gpio;
+
+	err = of_property_read_u32(node, "value", &step->gpio.value);
+	if (err < 0) {
+		power_seq_err(seq, step_nbr, "error reading value property\n");
+	} else if (step->gpio.value < 0 || step->gpio.value > 1) {
+		power_seq_err(seq, step_nbr,
+			      "value out of range (must be 0 or 1)\n");
+		err = -EINVAL;
+	}
+
+	return err;
+}
+#else
+#define of_power_seq_parse_gpio NULL
+#endif
+
+static bool power_seq_res_compare_gpio(struct power_seq_resource *res,
+				       struct power_seq_resource *res2)
+{
+	return res->gpio.gpio == res2->gpio.gpio;
+}
+
+static int power_seq_res_alloc_gpio(struct device *dev,
+				    struct power_seq_resource *res)
+{
+	int err;
+
+	err = devm_gpio_request(dev, res->gpio.gpio, dev_name(dev));
+	if (err) {
+		dev_err(dev, "cannot get gpio %d\n", res->gpio.gpio);
+		return err;
+	}
+
+	return 0;
+}
+
+static int power_seq_step_run_gpio(struct power_seq_step *step)
+{
+	struct power_seq_resource *res = step->resource;
+
+	/* set the GPIO direction at first use */
+	if (!res->gpio.is_set) {
+		int err = gpio_direction_output(res->gpio.gpio,
+						step->gpio.value);
+		if (err)
+			return err;
+		res->gpio.is_set = true;
+	} else {
+		gpio_set_value_cansleep(res->gpio.gpio, step->gpio.value);
+	}
+
+	return 0;
+}
+
+const struct power_seq_res_ops power_seq_gpio_ops = {
+	.name = "gpio",
+	.of_parse = of_power_seq_parse_gpio,
+	.step_run = power_seq_step_run_gpio,
+	.res_compare = power_seq_res_compare_gpio,
+	.res_alloc = power_seq_res_alloc_gpio,
+};
diff --git a/drivers/power/power_seq/power_seq_priv.h b/drivers/power/power_seq/power_seq_priv.h
new file mode 100644
index 0000000..fa56e21
--- /dev/null
+++ b/drivers/power/power_seq/power_seq_priv.h
@@ -0,0 +1,56 @@
+/*
+ * Copyright (c) 2012 NVIDIA 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.
+ *
+ */
+
+#ifndef POWER_SEQ_PRIV_H
+#define POWER_SEQ_PRIV_H
+
+#include <linux/device.h>
+#include <linux/power_seq.h>
+#include <linux/of.h>
+
+#define power_seq_err(seq, step_nbr, format, ...)			\
+	dev_err(seq->set->dev, "%s[%d]: " format, seq->id, step_nbr,	\
+	##__VA_ARGS__);
+
+#ifdef CONFIG_OF
+int of_power_seq_parse_enable_properties(struct device_node *node,
+					 struct power_seq *seq,
+					 unsigned int step_nbr, bool *enable);
+#endif
+
+/**
+ * struct power_seq_res_ops - operators for power sequences resources
+ * @name:		Name of the resource type. Set to null when a resource
+ *			type support is not compiled in
+ * @of_parse:		Parse a step for this kind of resource from a device
+ *			tree node. The result of parsing must be written into
+ *			step step_nbr of seq
+ * @step_run:		Run a step for this kind of resource
+ * @res_compare:	Return true if the resource used by the resource is the
+ *			same as the one referenced by the step, false otherwise.
+ * @res_alloc:		Resolve and allocate a resource. Return error code if
+ *			the resource cannot be allocated, 0 otherwise
+ */
+struct power_seq_res_ops {
+	const char *name;
+	int (*of_parse)(struct device_node *node, struct power_seq *seq,
+			unsigned int step_nbr, struct power_seq_resource *res);
+	int (*step_run)(struct power_seq_step *step);
+	bool (*res_compare)(struct power_seq_resource *res,
+			    struct power_seq_resource *res2);
+	int (*res_alloc)(struct device *dev,
+			 struct power_seq_resource *res);
+};
+
+#endif
diff --git a/drivers/power/power_seq/pwm.c b/drivers/power/power_seq/pwm.c
new file mode 100644
index 0000000..a74b768
--- /dev/null
+++ b/drivers/power/power_seq/pwm.c
@@ -0,0 +1,85 @@
+/*
+ * Copyright (c) 2012 NVIDIA 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.
+ *
+ */
+
+#include "power_seq_priv.h"
+
+#ifdef CONFIG_PWM
+
+#include <linux/pwm.h>
+
+#ifdef CONFIG_OF
+static int of_power_seq_parse_pwm(struct device_node *node,
+				  struct power_seq *seq,
+				  unsigned int step_nbr,
+				  struct power_seq_resource *res)
+{
+	struct power_seq_step *step = &seq->steps[step_nbr];
+	int err;
+
+	err = of_property_read_string(node, "id", &res->pwm.id);
+	if (err) {
+		power_seq_err(seq, step_nbr, "error reading id property\n");
+		return err;
+	}
+
+	err = of_power_seq_parse_enable_properties(node, seq, step_nbr,
+						   &step->pwm.enable);
+	return err;
+}
+#else
+#define of_power_seq_parse_pwm NULL
+#endif
+
+static bool power_seq_res_compare_pwm(struct power_seq_resource *res,
+				      struct power_seq_resource *res2)
+{
+	return !strcmp(res->pwm.id, res2->pwm.id);
+}
+
+static int power_seq_res_alloc_pwm(struct device *dev,
+				   struct power_seq_resource *res)
+{
+	res->pwm.pwm = devm_pwm_get(dev, res->pwm.id);
+	if (IS_ERR(res->pwm.pwm)) {
+		dev_err(dev, "cannot get pwm \"%s\"\n", res->pwm.id);
+		return PTR_ERR(res->pwm.pwm);
+	}
+
+	return 0;
+}
+
+static int power_seq_step_run_pwm(struct power_seq_step *step)
+{
+	if (step->pwm.enable) {
+		return pwm_enable(step->resource->pwm.pwm);
+	} else {
+		pwm_disable(step->resource->pwm.pwm);
+		return 0;
+	}
+}
+
+const struct power_seq_res_ops power_seq_pwm_ops = {
+	.name = "pwm",
+	.of_parse = of_power_seq_parse_pwm,
+	.step_run = power_seq_step_run_pwm,
+	.res_compare = power_seq_res_compare_pwm,
+	.res_alloc = power_seq_res_alloc_pwm,
+};
+
+#else
+
+const struct power_seq_res_ops power_seq_pwm_ops = {
+};
+
+#endif
diff --git a/drivers/power/power_seq/regulator.c b/drivers/power/power_seq/regulator.c
new file mode 100644
index 0000000..4663dbc
--- /dev/null
+++ b/drivers/power/power_seq/regulator.c
@@ -0,0 +1,87 @@
+/*
+ * Copyright (c) 2012 NVIDIA 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.
+ *
+ */
+
+#include "power_seq_priv.h"
+
+#ifdef CONFIG_REGULATOR
+
+#include <linux/err.h>
+#include <linux/regulator/consumer.h>
+
+#ifdef CONFIG_OF
+static int of_power_seq_parse_regulator(struct device_node *node,
+					struct power_seq *seq,
+					unsigned int step_nbr,
+					struct power_seq_resource *res)
+{
+	struct power_seq_step *step = &seq->steps[step_nbr];
+	int err;
+
+	err = of_property_read_string(node, "id",
+				      &res->regulator.id);
+	if (err) {
+		power_seq_err(seq, step_nbr, "error reading id property\n");
+		return err;
+	}
+
+	err = of_power_seq_parse_enable_properties(node, seq, step_nbr,
+						   &step->regulator.enable);
+	return err;
+}
+#else
+#define of_power_seq_parse_regulator NULL
+#endif
+
+static bool
+power_seq_res_compare_regulator(struct power_seq_resource *res,
+				struct power_seq_resource *res2)
+{
+	return !strcmp(res->regulator.id, res2->regulator.id);
+}
+
+static int power_seq_res_alloc_regulator(struct device *dev,
+					 struct power_seq_resource *res)
+{
+	res->regulator.regulator = devm_regulator_get(dev, res->regulator.id);
+	if (IS_ERR(res->regulator.regulator)) {
+		dev_err(dev, "cannot get regulator \"%s\"\n",
+			res->regulator.id);
+		return PTR_ERR(res->regulator.regulator);
+	}
+
+	return 0;
+}
+
+static int power_seq_step_run_regulator(struct power_seq_step *step)
+{
+	if (step->regulator.enable)
+		return regulator_enable(step->resource->regulator.regulator);
+	else
+		return regulator_disable(step->resource->regulator.regulator);
+}
+
+const struct power_seq_res_ops power_seq_regulator_ops = {
+	.name = "regulator",
+	.of_parse = of_power_seq_parse_regulator,
+	.step_run = power_seq_step_run_regulator,
+	.res_compare = power_seq_res_compare_regulator,
+	.res_alloc = power_seq_res_alloc_regulator,
+};
+
+#else
+
+const struct power_seq_res_ops power_seq_regulator_ops = {
+};
+
+#endif
diff --git a/include/linux/power_seq.h b/include/linux/power_seq.h
new file mode 100644
index 0000000..21b95b6
--- /dev/null
+++ b/include/linux/power_seq.h
@@ -0,0 +1,203 @@
+/*
+ * power_seq.h
+ *
+ * Simple interpreter for power sequences defined as platform data or device
+ * tree properties.
+ *
+ * Power sequences are designed to replace the callbacks typically used in
+ * board-specific files that implement board- or device- specific power
+ * sequences (such as those of backlights). A power sequence is an array of
+ * steps referencing resources (regulators, GPIOs, PWMs, ...) with an action to
+ * perform on them. By having the power sequences interpreted, it becomes
+ * possible to describe them in the device tree and thus to remove
+ * board-specific files from the kernel.
+ *
+ * See Documentation/power/power_seqs.txt for detailed information.
+ *
+ * Author: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
+ *
+ * Copyright (c) 2012 NVIDIA 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.
+ *
+ */
+
+#ifndef __LINUX_POWER_SEQ_H
+#define __LINUX_POWER_SEQ_H
+
+#include <linux/types.h>
+#include <linux/list.h>
+
+struct device;
+struct regulator;
+struct pwm_device;
+
+/**
+ * The different kinds of resources that can be controlled by the sequences
+ */
+enum power_seq_res_type {
+	POWER_SEQ_DELAY,
+	POWER_SEQ_REGULATOR,
+	POWER_SEQ_PWM,
+	POWER_SEQ_GPIO,
+	POWER_SEQ_NUM_TYPES,
+};
+
+/**
+ * struct power_seq_regulator_resource
+ * @id:		name of the regulator
+ * @regulator:	resolved regulator. Written during resource resolution.
+ */
+struct power_seq_regulator_resource {
+	const char *id;
+	struct regulator *regulator;
+};
+
+/**
+ * struct power_seq_pwm_resource
+ * @id:		name of the PWM
+ * @regulator:	resolved PWM. Written during resource resolution.
+ */
+struct power_seq_pwm_resource {
+	const char *id;
+	struct pwm_device *pwm;
+};
+
+/**
+ * struct power_seq_gpio_resource
+ * @gpio:	number of the GPIO
+ * @is_set:	track GPIO state to set its direction at first use
+ */
+struct power_seq_gpio_resource {
+	int gpio;
+	bool is_set;
+};
+
+/**
+ * struct power_seq_resource - resource used by power sequences
+ * @type:	type of the resource. This decides which member of the union is
+ *		used for this resource
+ * @list:	link resources together in power_seq_set
+ * @regulator:	used if @type == POWER_SEQ_REGULATOR
+ * @pwm:	used if @type == POWER_SEQ_PWM
+ * @gpio:	used if @type == POWER_SEQ_GPIO
+ */
+struct power_seq_resource {
+	enum power_seq_res_type type;
+	struct list_head list;
+	union {
+		struct power_seq_regulator_resource regulator;
+		struct power_seq_pwm_resource pwm;
+		struct power_seq_gpio_resource gpio;
+	};
+};
+#define power_seq_for_each_resource(pos, set)			\
+	list_for_each_entry(pos, &(set)->resources, list)
+
+/**
+ * struct power_seq_delay_step - action data for delay steps
+ * @delay:	amount of time to wait, in microseconds
+ */
+struct power_seq_delay_step {
+	unsigned int delay;
+};
+
+/**
+ * struct power_seq_regulator_step - platform data for regulator steps
+ * @enable:	whether to enable or disable the regulator during this step
+ */
+struct power_seq_regulator_step {
+	bool enable;
+};
+
+/**
+ * struct power_seq_pwm_step - action data for PWM steps
+ * @enable:	whether to enable or disable the PWM during this step
+ */
+struct power_seq_pwm_step {
+	bool enable;
+};
+
+/**
+ * struct power_seq_gpio_step - action data for GPIO steps
+ * @enable:	whether to enable or disable the GPIO during this step
+ */
+struct power_seq_gpio_step {
+	int value;
+};
+
+/**
+ * struct power_seq_step - data for power sequences steps
+ * @resource:	resource used by this step
+ * @delay:	used if resource->type == POWER_SEQ_DELAY
+ * @regulator:	used if resource->type == POWER_SEQ_REGULATOR
+ * @pwm:	used if resource->type == POWER_SEQ_PWN
+ * @gpio:	used if resource->type == POWER_SEQ_GPIO
+ */
+struct power_seq_step {
+	struct power_seq_resource *resource;
+	union {
+		struct power_seq_delay_step delay;
+		struct power_seq_regulator_step regulator;
+		struct power_seq_pwm_step pwm;
+		struct power_seq_gpio_step gpio;
+	};
+};
+
+struct power_seq_set;
+
+/**
+ * struct power_seq - single power sequence
+ * @id:		name of this sequence
+ * @list:	link sequences together in power_seq_set. Leave as-is
+ * @set:	set this sequence belongs to. Written when added to a set
+ * @num_steps:	number of steps in the sequence
+ * @steps:	array of steps that make the sequence
+ */
+struct power_seq {
+	const char *id;
+	struct list_head list;
+	struct power_seq_set *set;
+	unsigned int num_steps;
+	struct power_seq_step steps[];
+};
+
+/**
+ * struct power_seq_set - power sequences and resources used by a device
+ * @dev:	device this set belongs to
+ * @resources:	list of resources used by power sequences
+ * @seqs:	list of power sequences
+ */
+struct power_seq_set {
+	struct device *dev;
+	struct list_head resources;
+	struct list_head seqs;
+};
+
+/**
+ * struct platform_power_seq_set - define power sequences as platform data
+ * @num_seqs:	number of sequences defined
+ * @seqs:	array of num_seqs power sequences
+ */
+struct platform_power_seq_set {
+	unsigned int num_seqs;
+	struct power_seq *seqs[];
+};
+
+struct platform_power_seq_set *devm_of_parse_power_seq_set(struct device *dev);
+void power_seq_set_init(struct power_seq_set *set, struct device *dev);
+int power_seq_set_add_sequence(struct power_seq_set *set,
+			       struct power_seq *seq);
+int power_seq_set_add_sequences(struct power_seq_set *set,
+				struct platform_power_seq_set *seqs);
+struct power_seq *power_seq_lookup(struct power_seq_set *seqs, const char *id);
+int power_seq_run(struct power_seq *seq);
+
+#endif
-- 
1.8.0

^ permalink raw reply related

* [PATCHv9 2/3] pwm_backlight: use power sequences
From: Alexandre Courbot @ 2012-11-17 10:55 UTC (permalink / raw)
  To: Anton Vorontsov, Stephen Warren, Thierry Reding, Mark Zhang,
	Grant Likely, Rob Herring, Mark Brown, David Woodhouse,
	Arnd Bergmann
  Cc: Alexandre Courbot, linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1353149747-31871-1-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Make use of the power sequences specified in the device tree or platform
data to control how the backlight is powered on and off.

Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
Tested-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Tested-by: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
Acked-by: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
---
 .../bindings/video/backlight/pwm-backlight.txt     |  63 +++++++-
 drivers/video/backlight/Kconfig                    |   1 +
 drivers/video/backlight/pwm_bl.c                   | 160 ++++++++++++++++-----
 include/linux/pwm_backlight.h                      |  18 ++-
 4 files changed, 202 insertions(+), 40 deletions(-)

diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
index 1e4fc72..b20e98e 100644
--- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
+++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
@@ -13,16 +13,73 @@ Required properties:
 
 Optional properties:
   - pwm-names: a list of names for the PWM devices specified in the
-               "pwms" property (see PWM binding[0])
+      "pwms" property (see PWM binding[0]).
+  - power-sequences: Power sequences (see Power sequences[1]) used to bring the
+      backlight on and off. If this property is present, then two power
+      sequences named "power-on" and "power-off" must be defined to control how
+      the backlight is to be powered on and off. These sequences must reference
+      the PWM specified in the pwms property by its name, and can also reference
+      other resources supported by the power sequences mechanism
 
 [0]: Documentation/devicetree/bindings/pwm/pwm.txt
+[1]: Documentation/devicetree/bindings/power/power_seq.txt
 
 Example:
 
 	backlight {
 		compatible = "pwm-backlight";
-		pwms = <&pwm 0 5000000>;
-
 		brightness-levels = <0 4 8 16 32 64 128 255>;
 		default-brightness-level = <6>;
+		low-threshold-brightness = <50>;
+
+		/* resources used by the power sequences */
+		pwms = <&pwm 0 5000000>;
+		pwm-names = "backlight";
+		power-supply = <&backlight_reg>;
+
+		power-sequences {
+			power-on {
+				step0 {
+					type = "regulator";
+					id = "power";
+					enable;
+				};
+				step1 {
+					type = "delay";
+					delay = <10000>;
+				};
+				step2 {
+					type = "pwm";
+					id = "backlight";
+					enable;
+				};
+				step3 {
+					type = "gpio";
+					gpio = <&gpio 28 0>;
+					value = <1>;
+				};
+			};
+
+			power-off {
+				step0 {
+					type = "gpio";
+					gpio = <&gpio 28 0>;
+					value = <0>;
+				};
+				step1 {
+					type = "pwm";
+					id = "backlight";
+					disable;
+				};
+				step2 {
+					type = "delay";
+					delay = <10000>;
+				};
+				step3 {
+					type = "regulator";
+					id = "power";
+					disable;
+				};
+			};
+		};
 	};
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 765a945..a6b0640 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -240,6 +240,7 @@ config BACKLIGHT_CARILLO_RANCH
 config BACKLIGHT_PWM
 	tristate "Generic PWM based Backlight Driver"
 	depends on PWM
+	select POWER_SEQ
 	help
 	  If you have a LCD backlight adjustable by PWM, say Y to enable
 	  this driver.
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 069983c..cfc0780 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -27,6 +27,12 @@ struct pwm_bl_data {
 	unsigned int		period;
 	unsigned int		lth_brightness;
 	unsigned int		*levels;
+	bool			enabled;
+	struct power_seq_set	power_seqs;
+	struct power_seq	*power_on_seq;
+	struct power_seq	*power_off_seq;
+
+	/* Legacy callbacks */
 	int			(*notify)(struct device *,
 					  int brightness);
 	void			(*notify_after)(struct device *,
@@ -35,6 +41,51 @@ struct pwm_bl_data {
 	void			(*exit)(struct device *);
 };
 
+static void pwm_backlight_on(struct backlight_device *bl)
+{
+	struct pwm_bl_data *pb = dev_get_drvdata(&bl->dev);
+	int ret;
+
+	if (pb->enabled)
+		return;
+
+	if (pb->power_on_seq) {
+		ret = power_seq_run(pb->power_on_seq);
+		if (ret < 0) {
+			dev_err(&bl->dev, "cannot run power on sequence\n");
+			return;
+		}
+	} else {
+		/* legacy framework */
+		pwm_enable(pb->pwm);
+	}
+
+	pb->enabled = true;
+}
+
+static void pwm_backlight_off(struct backlight_device *bl)
+{
+	struct pwm_bl_data *pb = dev_get_drvdata(&bl->dev);
+	int ret;
+
+	if (!pb->enabled)
+		return;
+
+	if (pb->power_off_seq) {
+		ret = power_seq_run(pb->power_off_seq);
+		if (ret < 0) {
+			dev_err(&bl->dev, "cannot run power off sequence\n");
+			return;
+		}
+	} else {
+		/* legacy framework */
+		pwm_config(pb->pwm, 0, pb->period);
+		pwm_disable(pb->pwm);
+	}
+
+	pb->enabled = false;
+}
+
 static int pwm_backlight_update_status(struct backlight_device *bl)
 {
 	struct pwm_bl_data *pb = dev_get_drvdata(&bl->dev);
@@ -51,8 +102,7 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
 		brightness = pb->notify(pb->dev, brightness);
 
 	if (brightness == 0) {
-		pwm_config(pb->pwm, 0, pb->period);
-		pwm_disable(pb->pwm);
+		pwm_backlight_off(bl);
 	} else {
 		int duty_cycle;
 
@@ -66,7 +116,7 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
 		duty_cycle = pb->lth_brightness +
 		     (duty_cycle * (pb->period - pb->lth_brightness) / max);
 		pwm_config(pb->pwm, duty_cycle, pb->period);
-		pwm_enable(pb->pwm);
+		pwm_backlight_on(bl);
 	}
 
 	if (pb->notify_after)
@@ -145,11 +195,10 @@ static int pwm_backlight_parse_dt(struct device *dev,
 		data->max_brightness--;
 	}
 
-	/*
-	 * TODO: Most users of this driver use a number of GPIOs to control
-	 *       backlight power. Support for specifying these needs to be
-	 *       added.
-	 */
+	/* read power sequences */
+	data->power_seqs = devm_of_parse_power_seq_set(dev);
+	if (IS_ERR(data->power_seqs))
+		return PTR_ERR(data->power_seqs);
 
 	return 0;
 }
@@ -172,6 +221,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 {
 	struct platform_pwm_backlight_data *data = pdev->dev.platform_data;
 	struct platform_pwm_backlight_data defdata;
+	struct power_seq_resource *res;
 	struct backlight_properties props;
 	struct backlight_device *bl;
 	struct pwm_bl_data *pb;
@@ -180,7 +230,9 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 
 	if (!data) {
 		ret = pwm_backlight_parse_dt(&pdev->dev, &defdata);
-		if (ret < 0) {
+		if (ret == -EPROBE_DEFER) {
+			return ret;
+		} else if (ret < 0) {
 			dev_err(&pdev->dev, "failed to find platform data\n");
 			return ret;
 		}
@@ -201,6 +253,68 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 		goto err_alloc;
 	}
 
+	if (data->power_seqs) {
+		/* use power sequences */
+		struct power_seq_set *seqs = &pb->power_seqs;
+
+		power_seq_set_init(seqs, &pdev->dev);
+		power_seq_set_add_sequences(seqs, data->power_seqs);
+
+		/* Check that the required sequences are here */
+		pb->power_on_seq = power_seq_lookup(seqs, "power-on");
+		if (!pb->power_on_seq) {
+			dev_err(&pdev->dev, "missing power-on sequence\n");
+			return -EINVAL;
+		}
+		pb->power_off_seq = power_seq_lookup(seqs, "power-off");
+		if (!pb->power_off_seq) {
+			dev_err(&pdev->dev, "missing power-off sequence\n");
+			return -EINVAL;
+		}
+
+		/* we must have exactly one PWM resource for this driver */
+		power_seq_for_each_resource(res, seqs) {
+			if (res->type != POWER_SEQ_PWM)
+				continue;
+			if (pb->pwm) {
+				dev_err(&pdev->dev, "more than one PWM used\n");
+				return -EINVAL;
+			}
+			/* keep the pwm at hand */
+			pb->pwm = res->pwm.pwm;
+		}
+		/* from here we should have a PWM */
+		if (!pb->pwm) {
+			dev_err(&pdev->dev, "no PWM defined!\n");
+			return -EINVAL;
+		}
+	} else {
+		/* use legacy interface */
+		pb->pwm = devm_pwm_get(&pdev->dev, NULL);
+		if (IS_ERR(pb->pwm)) {
+			dev_err(&pdev->dev,
+				"unable to request PWM, trying legacy API\n");
+
+			pb->pwm = pwm_request(data->pwm_id, "pwm-backlight");
+			if (IS_ERR(pb->pwm)) {
+				dev_err(&pdev->dev,
+					"unable to request legacy PWM\n");
+				ret = PTR_ERR(pb->pwm);
+				goto err_alloc;
+			}
+		}
+
+		dev_dbg(&pdev->dev, "got pwm for backlight\n");
+
+		/*
+		* The DT case will set the pwm_period_ns field to 0 and store
+		* the period, parsed from the DT, in the PWM device. For the
+		* non-DT case, set the period from platform data.
+		*/
+		if (data->pwm_period_ns > 0)
+			pwm_set_period(pb->pwm, data->pwm_period_ns);
+	}
+
 	if (data->levels) {
 		max = data->levels[data->max_brightness];
 		pb->levels = data->levels;
@@ -213,28 +327,6 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 	pb->exit = data->exit;
 	pb->dev = &pdev->dev;
 
-	pb->pwm = devm_pwm_get(&pdev->dev, NULL);
-	if (IS_ERR(pb->pwm)) {
-		dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
-
-		pb->pwm = pwm_request(data->pwm_id, "pwm-backlight");
-		if (IS_ERR(pb->pwm)) {
-			dev_err(&pdev->dev, "unable to request legacy PWM\n");
-			ret = PTR_ERR(pb->pwm);
-			goto err_alloc;
-		}
-	}
-
-	dev_dbg(&pdev->dev, "got pwm for backlight\n");
-
-	/*
-	 * The DT case will set the pwm_period_ns field to 0 and store the
-	 * period, parsed from the DT, in the PWM device. For the non-DT case,
-	 * set the period from platform data.
-	 */
-	if (data->pwm_period_ns > 0)
-		pwm_set_period(pb->pwm, data->pwm_period_ns);
-
 	pb->period = pwm_get_period(pb->pwm);
 	pb->lth_brightness = data->lth_brightness * (pb->period / max);
 
@@ -267,8 +359,7 @@ static int pwm_backlight_remove(struct platform_device *pdev)
 	struct pwm_bl_data *pb = dev_get_drvdata(&bl->dev);
 
 	backlight_device_unregister(bl);
-	pwm_config(pb->pwm, 0, pb->period);
-	pwm_disable(pb->pwm);
+	pwm_backlight_off(bl);
 	if (pb->exit)
 		pb->exit(&pdev->dev);
 	return 0;
@@ -282,8 +373,7 @@ static int pwm_backlight_suspend(struct device *dev)
 
 	if (pb->notify)
 		pb->notify(pb->dev, 0);
-	pwm_config(pb->pwm, 0, pb->period);
-	pwm_disable(pb->pwm);
+	pwm_backlight_off(bl);
 	if (pb->notify_after)
 		pb->notify_after(pb->dev, 0);
 	return 0;
diff --git a/include/linux/pwm_backlight.h b/include/linux/pwm_backlight.h
index 56f4a86..0dcec1d 100644
--- a/include/linux/pwm_backlight.h
+++ b/include/linux/pwm_backlight.h
@@ -5,14 +5,28 @@
 #define __LINUX_PWM_BACKLIGHT_H
 
 #include <linux/backlight.h>
+#include <linux/power_seq.h>
 
 struct platform_pwm_backlight_data {
-	int pwm_id;
 	unsigned int max_brightness;
 	unsigned int dft_brightness;
 	unsigned int lth_brightness;
-	unsigned int pwm_period_ns;
 	unsigned int *levels;
+	/*
+	 * New interface using power sequences. Must include exactly
+	 * two power sequences named 'power-on' and 'power-off'. If NULL,
+	 * the legacy interface is used.
+	 */
+	struct platform_power_seq_set *power_seqs;
+
+	/*
+	 * Legacy interface - use power sequences instead!
+	 *
+	 * pwm_id and pwm_period_ns need only be specified
+	 * if get_pwm(dev, NULL) would return NULL.
+	 */
+	int pwm_id;
+	unsigned int pwm_period_ns;
 	int (*init)(struct device *dev);
 	int (*notify)(struct device *dev, int brightness);
 	void (*notify_after)(struct device *dev, int brightness);
-- 
1.8.0

^ permalink raw reply related

* [PATCHv9 3/3] Take maintainership of power sequences
From: Alexandre Courbot @ 2012-11-17 10:55 UTC (permalink / raw)
  To: Anton Vorontsov, Stephen Warren, Thierry Reding, Mark Zhang,
	Grant Likely, Rob Herring, Mark Brown, David Woodhouse,
	Arnd Bergmann
  Cc: linux-tegra, linux-arm-kernel, linux-kernel, linux-fbdev,
	devicetree-discuss, linux-pm, Alexandre Courbot,
	Alexandre Courbot
In-Reply-To: <1353149747-31871-1-git-send-email-acourbot@nvidia.com>

Add entry for power sequences into MAINTAINERS with all the needed
contact and SCM info.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Acked-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
---
 MAINTAINERS | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index bb0b27d..e9098f5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5745,6 +5745,16 @@ F:	fs/timerfd.c
 F:	include/linux/timer*
 F:	kernel/*timer*
 
+POWER SEQUENCES
+M:	Alexandre Courbot <acourbot@nvidia.com>
+S:	Supported
+W:	https://github.com/Gnurou/linux
+T:	git git://github.com/Gnurou/linux.git power-seq/for-next
+F:	Documentation/devicetree/bindings/power/power_seq.txt
+F:	Documentation/power/power_seq.txt
+F:	include/linux/power_seq.h
+F:	drivers/power/power_seq/
+
 POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS
 M:	Anton Vorontsov <cbou@mail.ru>
 M:	David Woodhouse <dwmw2@infradead.org>
-- 
1.8.0


^ permalink raw reply related

* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Anton Vorontsov @ 2012-11-17 11:38 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: Stephen Warren, Thierry Reding, Mark Zhang, Grant Likely,
	Rob Herring, Mark Brown, David Woodhouse, Arnd Bergmann,
	linux-tegra, linux-arm-kernel, linux-kernel, linux-fbdev,
	devicetree-discuss, linux-pm, Alexandre Courbot
In-Reply-To: <1353149747-31871-2-git-send-email-acourbot@nvidia.com>

On Sat, Nov 17, 2012 at 07:55:45PM +0900, Alexandre Courbot wrote:
> Some device drivers (e.g. panel or backlights) need to follow precise
> sequences for powering on and off, involving GPIOs, regulators, PWMs
> and other power-related resources, with a defined powering order and
> delays to respect between steps. These sequences are device-specific,
> and do not belong to a particular driver - therefore they have been
> performed by board-specific hook functions so far.
> 
> With the advent of the device tree and of ARM kernels that are not
> board-tied, we cannot rely on these board-specific hooks anymore but
> need a way to implement these sequences in a portable manner. This patch
> introduces a simple interpreter that can execute such power sequences
> encoded either as platform data or within the device tree. It also
> introduces first support for regulator, GPIO and PWM resources.
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> Reviewed-by: Stephen Warren <swarren@wwwdotorg.org>
> Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Stephen Warren <swarren@nvidia.com>
> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
> ---

This looks almost perfect!

Just a few final notes, again mostly about the build system bits.

[...]
> diff --git a/drivers/power/power_seq/Kconfig b/drivers/power/power_seq/Kconfig
> new file mode 100644
> index 0000000..0ece819
> --- /dev/null
> +++ b/drivers/power/power_seq/Kconfig
> @@ -0,0 +1,2 @@
> +config POWER_SEQ
> +	tristate

This really needs a proper Kconfig description and a help text, shortly
describing the purpose of the subsystem.

> diff --git a/drivers/power/power_seq/Makefile b/drivers/power/power_seq/Makefile
> new file mode 100644
> index 0000000..964b91e
> --- /dev/null
> +++ b/drivers/power/power_seq/Makefile
> @@ -0,0 +1,2 @@
> +obj-$(CONFIG_POWER_SEQ)		+= power_seq.o
> +power_seq-y			:= core.o delay.o regulator.o gpio.o pwm.o
> diff --git a/drivers/power/power_seq/core.c b/drivers/power/power_seq/core.c
> new file mode 100644
> index 0000000..d51b9b8
> --- /dev/null
> +++ b/drivers/power/power_seq/core.c
> @@ -0,0 +1,362 @@
> +/*
> + * core.c - power sequence interpreter for platform devices and device tree

We usually don't write file names in the comments.

> + *
> + * Author: Alexandre Courbot <acourbot@nvidia.com>
> + *
[...]
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(power_seq_run);
> +
> +/* defined in power_seq_*.c files */

Why not place the decls into the _priv.h file?.. I understand that this
might be a temporary stuff until we have something better for ops
registration, but still, I believe it belongs to the header file.

> +extern const struct power_seq_res_ops power_seq_delay_ops;
> +extern const struct power_seq_res_ops power_seq_gpio_ops;
> +extern const struct power_seq_res_ops power_seq_regulator_ops;
> +extern const struct power_seq_res_ops power_seq_pwm_ops;
> +
> +static const struct power_seq_res_ops *power_seq_ops[POWER_SEQ_NUM_TYPES] = {
> +	[POWER_SEQ_DELAY] = &power_seq_delay_ops,
> +	[POWER_SEQ_REGULATOR] = &power_seq_regulator_ops,
> +	[POWER_SEQ_PWM] = &power_seq_gpio_ops,
> +	[POWER_SEQ_GPIO] = &power_seq_pwm_ops,
> +};
> +
> +MODULE_AUTHOR("Alexandre Courbot <acourbot@nvidia.com>");
> +MODULE_DESCRIPTION("Runtime Interpreted Power Sequences");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/power/power_seq/delay.c b/drivers/power/power_seq/delay.c
> new file mode 100644
> index 0000000..de6810b
> --- /dev/null
> +++ b/drivers/power/power_seq/delay.c
> @@ -0,0 +1,66 @@
> +/*
> + * Copyright (c) 2012 NVIDIA 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.
> + *
> + */
> +
> +#include "power_seq_priv.h"
> +#include <linux/delay.h>

Things should not depend on _priv.h's includes. I.e. I see this file uses
of_ routines, so it should include <linux/of.h>. <linux/delay.h> for
udelay_range(), etc.

Also, usually we place "private" headers at the very end, not at the
beginning.

> +
> +#ifdef CONFIG_OF
> +static int of_power_seq_parse_delay(struct device_node *node,
> +				    struct power_seq *seq,
> +				    unsigned int step_nbr,
> +				    struct power_seq_resource *res)
> +{
> +	struct power_seq_step *step = &seq->steps[step_nbr];
> +	int err;
> +
> +	err = of_property_read_u32(node, "delay",
> +				   &step->delay.delay);
> +	if (err < 0)
> +		power_seq_err(seq, step_nbr, "error reading delay property\n");
> +
> +	return err;
> +}
> +#else
> +#define of_power_seq_parse_delay NULL
> +#endif

[...]
> +#define power_seq_err(seq, step_nbr, format, ...)			\
> +	dev_err(seq->set->dev, "%s[%d]: " format, seq->id, step_nbr,	\
> +	##__VA_ARGS__);
> +
> +#ifdef CONFIG_OF
> +int of_power_seq_parse_enable_properties(struct device_node *node,
> +					 struct power_seq *seq,
> +					 unsigned int step_nbr, bool *enable);

Um, I believe you don't need CONFIG_OF here. (If it's about 'struct
device_node', then as I see it in of.h, the header always declares it,
even for the !OF case.)

> +#endif
> +
> +/**
[...]
> +++ b/drivers/power/power_seq/regulator.c
> @@ -0,0 +1,87 @@
> +/*
> + * Copyright (c) 2012 NVIDIA 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.
> + *
> + */
> +
> +#include "power_seq_priv.h"
> +
> +#ifdef CONFIG_REGULATOR

Would be really great if you could get rid of the #ifdefs in the .c files.

To do so, in the makefile you could write something like this:

power_seq-$(CONFIG_REGULATOR) += regulator.o

And in the header file (as explained above), you'd have

#ifdef CONFIG_REGULATOR
#define ...REGULATOR_OPS &power_seq_regulator_ops
#else
#define ...REGULATOR_OPS NULL
#endif

Or something along these lines...

Thanks,
Anton.

^ permalink raw reply

* Re: [PATCH 3/6 v4] cpufreq: tolerate inexact values when collecting stats
From: Borislav Petkov @ 2012-11-17 14:50 UTC (permalink / raw)
  To: Mark Langsdorf
  Cc: linux-kernel@vger.kernel.org, cpufreq@vger.kernel.org,
	linux-pm@vger.kernel.org, MyungJoo Ham
In-Reply-To: <21672683C5A3814BB4DB938EBE482DE426EA988080@IAD2MBX09.mex02.mlsrvr.com>

On Tue, Nov 13, 2012 at 02:13:38PM -0500, Mark Langsdorf wrote:
> Although cpufreq_driver has a flag field, no part of cpufreq_driver
> is directly passed to the cpufreq_stat code. Only cpufreq_policy
> is. It's cleaner to do passes of the while loop than to copy the
> cpufreq_driver.flag field into cpufreq_policy and then store it again
> in cpufreq_stats.

That maybe so but this newly added loop which is only Calxeda-relevant
is called in cpufreq_stat_notifier_trans, which is the frequency change
notifier call, AFAICT.

So each cpufreq driver will be paying that small and needless penalty
now for nothing and on each frequency change. Which adds to the
kernel-wide bloat and we absolutely don't want that.

So you probably need to find a slick way of detecting calxeda hw
somewhere along the init path of cpufreq_stats_init and set a
hw-specific flag instead of adding that cost to each driver.

Thanks.

-- 
Regards/Gruss,
Boris.

^ permalink raw reply

* Re: [RFC] cpuidle - remove the power_specified field in the driver
From: Francesco Lavra @ 2012-11-18  8:40 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: linux-pm, khilman, deepthi, g.trinabh, linaro-dev, len.brown,
	linux-kernel, rjw, jwerner, akpm, snanda
In-Reply-To: <1352752016-3136-1-git-send-email-daniel.lezcano@linaro.org>

Hi,

On 11/12/2012 09:26 PM, Daniel Lezcano wrote:
> This patch follows the discussion about reinitializing the power usage
> when a C-state is added/removed.
> 
>  https://lkml.org/lkml/2012/10/16/518
> 
> We realized the power usage field is never filled and when it is
> filled for tegra, the power_specified flag is not set making all these
> values to be resetted when the driver is initialized with the set_power_state
> function.
> 
> Julius and I feel this is over-engineered and the power_specified
> flag could be simply removed and continue assuming the states are
> backward sorted.
> 
> The menu governor select function is simplified as the power is ordered.
> Actually the condition is always true with the current code.
> 
> The cpuidle_play_dead function is also simplified by doing a reverse lookup
> on the array.
> 
> The set_power_states function is removed as it does no make sense anymore.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/cpuidle/cpuidle.c        |   17 ++++-------------
>  drivers/cpuidle/driver.c         |   25 -------------------------
>  drivers/cpuidle/governors/menu.c |    8 ++------
>  include/linux/cpuidle.h          |    2 +-
>  4 files changed, 7 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 711dd83..f983262 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -69,24 +69,15 @@ int cpuidle_play_dead(void)
>  {
>  	struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
>  	struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
> -	int i, dead_state = -1;
> -	int power_usage = -1;
> +	int i;
>  
>  	if (!drv)
>  		return -ENODEV;
>  
>  	/* Find lowest-power state that supports long-term idle */
> -	for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) {
> -		struct cpuidle_state *s = &drv->states[i];
> -
> -		if (s->power_usage < power_usage && s->enter_dead) {
> -			power_usage = s->power_usage;
> -			dead_state = i;
> -		}
> -	}
> -
> -	if (dead_state != -1)
> -		return drv->states[dead_state].enter_dead(dev, dead_state);
> +	for (i = drv->state_count; i >= CPUIDLE_DRIVER_STATE_START; i--)
> +		if (drv->states[i].play_dead)

I guess you meant drv->states[i].enter_dead

> +			return drv->states[i].enter_dead(dev, i);

--
Francesco

^ permalink raw reply

* Re: [RFC] cpuidle - remove the power_specified field in the driver
From: Daniel Lezcano @ 2012-11-18  9:17 UTC (permalink / raw)
  To: Francesco Lavra
  Cc: linux-pm, khilman, deepthi, g.trinabh, linaro-dev, len.brown,
	linux-kernel, rjw, jwerner, akpm, snanda
In-Reply-To: <50A89F08.7020307@gmail.com>

On 11/18/2012 09:40 AM, Francesco Lavra wrote:
> Hi,
> 
> On 11/12/2012 09:26 PM, Daniel Lezcano wrote:
>> This patch follows the discussion about reinitializing the power usage
>> when a C-state is added/removed.
>>
>>  https://lkml.org/lkml/2012/10/16/518
>>
>> We realized the power usage field is never filled and when it is
>> filled for tegra, the power_specified flag is not set making all these
>> values to be resetted when the driver is initialized with the set_power_state
>> function.
>>
>> Julius and I feel this is over-engineered and the power_specified
>> flag could be simply removed and continue assuming the states are
>> backward sorted.
>>
>> The menu governor select function is simplified as the power is ordered.
>> Actually the condition is always true with the current code.
>>
>> The cpuidle_play_dead function is also simplified by doing a reverse lookup
>> on the array.
>>
>> The set_power_states function is removed as it does no make sense anymore.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>>  drivers/cpuidle/cpuidle.c        |   17 ++++-------------
>>  drivers/cpuidle/driver.c         |   25 -------------------------
>>  drivers/cpuidle/governors/menu.c |    8 ++------
>>  include/linux/cpuidle.h          |    2 +-
>>  4 files changed, 7 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>> index 711dd83..f983262 100644
>> --- a/drivers/cpuidle/cpuidle.c
>> +++ b/drivers/cpuidle/cpuidle.c
>> @@ -69,24 +69,15 @@ int cpuidle_play_dead(void)
>>  {
>>  	struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
>>  	struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
>> -	int i, dead_state = -1;
>> -	int power_usage = -1;
>> +	int i;
>>  
>>  	if (!drv)
>>  		return -ENODEV;
>>  
>>  	/* Find lowest-power state that supports long-term idle */
>> -	for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) {
>> -		struct cpuidle_state *s = &drv->states[i];
>> -
>> -		if (s->power_usage < power_usage && s->enter_dead) {
>> -			power_usage = s->power_usage;
>> -			dead_state = i;
>> -		}
>> -	}
>> -
>> -	if (dead_state != -1)
>> -		return drv->states[dead_state].enter_dead(dev, dead_state);
>> +	for (i = drv->state_count; i >= CPUIDLE_DRIVER_STATE_START; i--)
>> +		if (drv->states[i].play_dead)
> 
> I guess you meant drv->states[i].enter_dead

Yep :)



-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply

* Re: [PATCH V6 1/2] Thermal: Add ST-Ericsson DB8500 thermal driver.
From: Francesco Lavra @ 2012-11-18 14:29 UTC (permalink / raw)
  To: Zhang Rui
  Cc: linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
	linaro-dev-cunTk1MwBs8s++Sfvej+rw,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, patches-QSEj5FYQhm4dnm+yROfE0A,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	STEricsson_nomadik_linux-nkJGhpqTU55BDgjK7y7TUQ,
	kernel-vMlcbD5RyM6HZuj8yyL1ah2eb7JE58TQ, hongbo.zhang
In-Reply-To: <1352983878.2080.25.camel@rzhang1-mobl4>

On 11/15/2012 01:51 PM, Zhang Rui wrote:
> On Thu, 2012-11-15 at 18:56 +0800, hongbo.zhang wrote:
>> From: "hongbo.zhang" <hongbo.zhang-68IGFXMjmZ7QT0dZR+AlfA@public.gmane.org>
>>
>> This driver is based on the thermal management framework in thermal_sys.c. A
>> thermal zone device is created with the trip points to which cooling devices
>> can be bound, the current cooling device is cpufreq, e.g. CPU frequency is
>> clipped down to cool the CPU, and other cooling devices can be added and bound
>> to the trip points dynamically.  The platform specific PRCMU interrupts are
>> used to active thermal update when trip points are reached.
>>
>> Signed-off-by: hongbo.zhang <hongbo.zhang-68IGFXMjmZ7QT0dZR+AlfA@public.gmane.org>
>> Reviewed-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Reviewed-by: Francesco Lavra <francescolavra.fl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> Patch is refreshed and applied to thermal next.
> refreshed patch attached.
[...]
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 99b6587..d96da07 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -101,5 +101,25 @@ config EXYNOS_THERMAL
>  	  If you say yes here you get support for TMU (Thermal Managment
>  	  Unit) on SAMSUNG EXYNOS series of SoC.
>  
> +config DB8500_THERMAL
> +	bool "DB8500 thermal management"
> +	depends on ARCH_U8500

Shouldn't it depend on THERMAL as well, as in Hongbo's original patch?

> +	default y
> +	help
> +	  Adds DB8500 thermal management implementation according to the thermal
> +	  management framework. A thermal zone with several trip points will be
> +	  created. Cooling devices can be bound to the trip points to cool this
> +	  thermal zone if trip points reached.
> +
> +config DB8500_CPUFREQ_COOLING
> +	tristate "DB8500 cpufreq cooling"
> +	depends on ARCH_U8500
> +	depends on CPU_THERMAL
> +	default y
> +	help
> +	  Adds DB8500 cpufreq cooling devices, and these cooling devices can be
> +	  bound to thermal zone trip points. When a trip point reached, the
> +	  bound cpufreq cooling device turns active to set CPU frequency low to
> +	  cool down the CPU.
>  
>  endif

--
Francesco

^ permalink raw reply

* Re: [PATCH v9 03/10] ata: zpodd: identify and init ZPODD devices
From: Tejun Heo @ 2012-11-18 14:38 UTC (permalink / raw)
  To: Aaron Lu
  Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
	Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <50A2F4AB.6060903@intel.com>

Hello, Aaron.

On Wed, Nov 14, 2012 at 09:32:27AM +0800, Aaron Lu wrote:
> > I don't think you're supposed to use dev->private_data from libata
> > core layer.  Just add a new field.  Nobody cares about adding 8 more
> > bytes to struct ata_device and spending 8 more bytes is way better
> > than muddying the ownership of ->private_data.
> 
> OK.
> And just out of curiosity, who's supposed to use device's private_data?
> I didn't find any user for ata_device's private_data in libata.

All the ->private_data fields are to be used by low level drivers
(ahci, ata_piix, pata_via...).  Given the twisted nature of ATA
devices, it's a bit surprising that no driver yet found a need for
ata_dev->private_data.  For most SATA controllers, port to device is
one to one so maybe ap->private_data is enough.

> > And this gets completely wrong.  What if the device supports DA and
> > low level driver makes use of ->private_data?
> 
> I didn't find any user of ata_device's private_data, so I used it for
> ZPODD. But if this is dangerous, I'll use a new field.

As there currently is no other user, it won't break anything but yeah
please add a properly typed and named field.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH v9 06/10] ata: zpodd: check zero power ready status
From: Tejun Heo @ 2012-11-18 15:00 UTC (permalink / raw)
  To: Aaron Lu
  Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
	Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <50A2FF6F.3080603@intel.com>

Hello, Aaron.

On Wed, Nov 14, 2012 at 10:18:23AM +0800, Aaron Lu wrote:
> On 11/13/2012 03:13 AM, Tejun Heo wrote:
> > Hello,
> > 
> > On Fri, Nov 09, 2012 at 02:51:58PM +0800, Aaron Lu wrote:
> >> +#define POWEROFF_DELAY  (30 * 1000)     /* 30 seconds for power off delay */
> >> +
> >>  struct zpodd {
> >>  	bool slot:1;
> >>  	bool drawer:1;
> >>  	bool from_notify:1;	/* resumed as a result of acpi notification */
> >> +	bool status_ready:1;	/* ready status derived from media event poll,
> >> +				   it is not accurate, but serves as a hint */
> >> +	bool zp_ready:1;	/* zero power ready state */
> >> +
> >> +	unsigned long last_ready; /* last zero power ready timestamp */
> >>  
> >>  	struct ata_device *dev;
> >>  };
> > 
> > How are accesses to the bit fields synchronized?
> 
> They are synchronized by PM core.
> PM core ensures that no two suspend or resume callback run concurrently.
> And when ODD is executing a command, it is in active state, no PM
> callback will run.

Care to add short comment for that?  Flag and bitfield updates aren't
atomic to each other, so I find it usually helpful to clearly state
the synchronization rules for them.  More so if locking is inherited
from upprer layer and not immediately obvious.

> > Hmmm... so, the "full" check only happens when autopm kicks in, right?
> > Is it really worth avoiding an extra TUR on autopm events?  That's not
> > really a hot path.  It seems a bit over-engineered to me.
> 
> A little more information about this:
> When there is disc inside and no program mounted the drive, the ODD will
> be runtime suspended/resumed every 2 seconds along with the event poll.

Is that a desirable behavior?  I haven't been following autopm and am
a bit fuzzy about how autopm works and what it does.

If there isn't any user of the device autopm kicks in.  If zpodd is
enabled and there's no media, the device goes off power.  If the user
initiates an event which may change media status, the driver is
notified via acpi and autopm backs out restoring power to the device.
Am I understanding it correctly?

What I'm confused about is what autopm does for devices w/o zpodd.
What happens then?  Is it gonna leave power on for the device and,
say, go on to suspend the controller?  But, how would that work for,
say, future devices with async notification for media events?

Also, if autopm is enabled, an optical device would go in and out of
suspend every two seconds?

> I'm not sure if the above situation can happen often. Normal desktop
> environment should automatically mount the ODD once they got uevent, and
> for console users, they will probably manually mount the drive after
> they have inserted a disc. The way I did it this way is to deal with the
> worst possible case. But if this is deemed as not necessary, I think I
> can remove the snoop hint thing and use TUR directly.

The problem with issuing TUR regularly is that some ODDs lock up after
getting hit by frequent TURs.  That's the reason why sr event check
routine is being careful with TUR and only issue
GET_EVENT_STATUS_NOTIFICATION.  Windows does about the same thing and
some vendors somehow screwed up TUR.

That said, I can't say the snooping is pretty.  It's a rather nasty
thing to do.  So, libata now wants information from the event polling
in block layer, but reaching for block_device from ata_devices is
nasty too.  Hmmm... but aren't you already doing that to block polling
on a powered down device?

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH v9 05/10] libata: separate ATAPI code
From: Tejun Heo @ 2012-11-18 15:01 UTC (permalink / raw)
  To: Aaron Lu
  Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
	Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20121113124923.GA11672@aaronlu.sh.intel.com>

On Tue, Nov 13, 2012 at 08:49:24PM +0800, Aaron Lu wrote:
> On Mon, Nov 12, 2012 at 10:57:10AM -0800, Tejun Heo wrote:
> > On Fri, Nov 09, 2012 at 02:51:57PM +0800, Aaron Lu wrote:
> > > The atapi_eh_tur and atapi_eh_request_sense can be reused by ZPODD
> > > code, so separate them out to a file named libata-atapi.c, and the
> > > Makefile is modified accordingly. No functional changes should result
> > > from this commit.
> > 
> > Why is this change necessary?  This doesn't seem to help anything to
> > me.
> 
> Function zpready introduced in patch 6 used these 2 functions to see if
> an ODD is in a zero power ready state.

And why zpready can't use the functions if they're in a different file?

-- 
tejun

^ permalink raw reply

* Re: [PATCH v9 07/10] block: add a new interface to block events
From: Tejun Heo @ 2012-11-18 15:05 UTC (permalink / raw)
  To: Alan Stern
  Cc: Aaron Lu, Jeff Garzik, James Bottomley, Rafael J. Wysocki,
	Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <Pine.LNX.4.44L0.1211121432050.31816-100000@netrider.rowland.org>

Hello, Alan.

On Mon, Nov 12, 2012 at 02:34:01PM -0500, Alan Stern wrote:
> It wouldn't do anything for the device, but it would allow the device's
> parent to go to low power in between polls.  That's why Aaron at one
> point suggested lengthening the polling interval.

Ah, okay.  Entering and leaving suspend even every two seconds may
save noticeable power for some controllers.  Does that, tho, mean that
the port would get reset every two seconds?  If so (I can't see how
else it would behave), it may not really work.  I mean, you would be
spending about ten seconds reinitiaizing everything and then enter
sleep and do it all over again two seconds later, most likely wasting
power in the process.  Sounds weird to me.  What am I missing?

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH v9 07/10] block: add a new interface to block events
From: Alan Stern @ 2012-11-18 17:41 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Aaron Lu, Jeff Garzik, James Bottomley, Rafael J. Wysocki,
	Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20121118150555.GL7306@mtj.dyndns.org>

On Sun, 18 Nov 2012, Tejun Heo wrote:

> Hello, Alan.

Hi.

> On Mon, Nov 12, 2012 at 02:34:01PM -0500, Alan Stern wrote:
> > It wouldn't do anything for the device, but it would allow the device's
> > parent to go to low power in between polls.  That's why Aaron at one
> > point suggested lengthening the polling interval.
> 
> Ah, okay.  Entering and leaving suspend even every two seconds may
> save noticeable power for some controllers.  Does that, tho, mean that
> the port would get reset every two seconds?

I don't know how suspend works for (S)ATA ports.  For USB mass storage, 
suspend/resume does not involve any resets unless something goes wrong.

>  If so (I can't see how
> else it would behave), it may not really work.  I mean, you would be
> spending about ten seconds reinitiaizing everything and then enter
> sleep and do it all over again two seconds later, most likely wasting
> power in the process.  Sounds weird to me.  What am I missing?

Does it really take 10 seconds to recover from an ATA suspend?  That 
sounds awfully long.

The two second value is merely the default interval for media polling.  
Devices with no async notification mechanism require some sort of
polling if anybody wants to know when media is inserted or removed,
and the polling can't be carried out if the device's port is suspended.

You asked about autopm.  "Autopm" is a rather vague and ugly term I
made up; it means that the kernel automatically suspends the device
after some specified idle time.  In the case of optical drives, "idle"  
currently means the device file is not open (which implies the drive
doesn't contain a mounted filesystem).  As far as I know, the kernel
has no direct way to affect the power level of an optical drive that
isn't ZP.  For these devices "suspended" is merely a logical state,
meaning that the device isn't in use and therefore its parent can be
put into a low-power mode.

Regardless, an optical drive can't remain suspended when polling
occurs.  So unless there's some other mechanism for getting the
information about media changes, suspended drives (and their parents)  
will have to be resumed periodically for polling.

Hence there's a tradeoff.  How can we use the minimum amount of energy
while still polling the drive acceptably often?  In general, the kernel
doesn't know.  That's why these things can be controlled from
userspace.  And the answer may be different for ATA drives vs.  
USB-connected drives.

Does this answer your questions?

Alan Stern


^ permalink raw reply

* Re: [PATCH v9 07/10] block: add a new interface to block events
From: Tejun Heo @ 2012-11-18 21:56 UTC (permalink / raw)
  To: Alan Stern
  Cc: Aaron Lu, Jeff Garzik, James Bottomley, Rafael J. Wysocki,
	Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <Pine.LNX.4.44L0.1211181221350.4991-100000@netrider.rowland.org>

Hello, Alan.

On Sun, Nov 18, 2012 at 12:41:06PM -0500, Alan Stern wrote:
> >  If so (I can't see how
> > else it would behave), it may not really work.  I mean, you would be
> > spending about ten seconds reinitiaizing everything and then enter
> > sleep and do it all over again two seconds later, most likely wasting
> > power in the process.  Sounds weird to me.  What am I missing?
> 
> Does it really take 10 seconds to recover from an ATA suspend?  That 
> sounds awfully long.

If it's using the same ->suspend ops as the regular system suspend,
10secs isn't a crazy number.  If the controller goes offline, the PHY
would go offline too and the only way to come back from that is
performing full probing sequence.  From SATA procotol POV, it isn't
too bad.  It's just link initiazliation followed by IDENTIFY and some
config commands.

The problem is that SATA devices aren't really designed to be used
like that.  If you reset a ODD w/ a media in it, it'll probably spin
it up and try to re-establish media state during probe sequence.  It
isn't designed that way and has never been used like that.  SATA has
its own dynamic link power management (DIPM/HIPM) tho.

So, this whole autopm thing doesn't sound like a good idea to me.

> The two second value is merely the default interval for media polling.  
> Devices with no async notification mechanism require some sort of
> polling if anybody wants to know when media is inserted or removed,
> and the polling can't be carried out if the device's port is suspended.

Yeah, I know.  I wrote that thing. :)

> You asked about autopm.  "Autopm" is a rather vague and ugly term I
> made up; it means that the kernel automatically suspends the device
> after some specified idle time.  In the case of optical drives, "idle"  
> currently means the device file is not open (which implies the drive
> doesn't contain a mounted filesystem).  As far as I know, the kernel
> has no direct way to affect the power level of an optical drive that
> isn't ZP.  For these devices "suspended" is merely a logical state,
> meaning that the device isn't in use and therefore its parent can be
> put into a low-power mode.
>
> Regardless, an optical drive can't remain suspended when polling
> occurs.  So unless there's some other mechanism for getting the
> information about media changes, suspended drives (and their parents)  
> will have to be resumed periodically for polling.
> 
> Hence there's a tradeoff.  How can we use the minimum amount of energy
> while still polling the drive acceptably often?  In general, the kernel
> doesn't know.  That's why these things can be controlled from
> userspace.  And the answer may be different for ATA drives vs.  
> USB-connected drives.
> 
> Does this answer your questions?

I think the only reason autopm doesn't blow up for SATA devices is
that userland usually automatically mounts them thus effectively
disabling autopm.  I *think* the only sane thing we can do is doing
autopm iff zpodd is available && no media.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH v9 07/10] block: add a new interface to block events
From: Tejun Heo @ 2012-11-18 21:58 UTC (permalink / raw)
  To: Alan Stern
  Cc: Aaron Lu, Jeff Garzik, James Bottomley, Rafael J. Wysocki,
	Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20121118215657.GA25790@mtj.dyndns.org>

On Sun, Nov 18, 2012 at 01:56:57PM -0800, Tejun Heo wrote:
> I think the only reason autopm doesn't blow up for SATA devices is
> that userland usually automatically mounts them thus effectively
> disabling autopm.  I *think* the only sane thing we can do is doing
> autopm iff zpodd is available && no media.

That is, at least for devices which get polled.  If devices are to
leave unused for some time, suspending is fine but auto-suspending
inbetween event polling wouldn't work.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH v9 07/10] block: add a new interface to block events
From: Alan Stern @ 2012-11-18 23:28 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Aaron Lu, Jeff Garzik, James Bottomley, Rafael J. Wysocki,
	Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20121118215657.GA25790@mtj.dyndns.org>

On Sun, 18 Nov 2012, Tejun Heo wrote:

> > Does it really take 10 seconds to recover from an ATA suspend?  That 
> > sounds awfully long.
> 
> If it's using the same ->suspend ops as the regular system suspend,
> 10secs isn't a crazy number.  If the controller goes offline, the PHY
> would go offline too and the only way to come back from that is
> performing full probing sequence.  From SATA procotol POV, it isn't
> too bad.  It's just link initiazliation followed by IDENTIFY and some
> config commands.
> 
> The problem is that SATA devices aren't really designed to be used
> like that.  If you reset a ODD w/ a media in it, it'll probably spin
> it up and try to re-establish media state during probe sequence.  It
> isn't designed that way and has never been used like that.  SATA has
> its own dynamic link power management (DIPM/HIPM) tho.

Is it possible to use those to implement runtime suspend?  Or are they 
handled autonomously by the hardware?

> So, this whole autopm thing doesn't sound like a good idea to me.

No doubt it's better suited to some devices than to others.

> > Hence there's a tradeoff.  How can we use the minimum amount of energy
> > while still polling the drive acceptably often?  In general, the kernel
> > doesn't know.  That's why these things can be controlled from
> > userspace.  And the answer may be different for ATA drives vs.  
> > USB-connected drives.
> > 
> > Does this answer your questions?
> 
> I think the only reason autopm doesn't blow up for SATA devices is
> that userland usually automatically mounts them thus effectively
> disabling autopm.

Either that or else because it hasn't been fully implemented yet.  :-)

>  I *think* the only sane thing we can do is doing
> autopm iff zpodd is available && no media.

That may be true for SATA.  For USB optical drives, it does make sense
to power-down the host controller when the drive isn't in use.  USB
suspend/resume takes on the order of 50-100 ms or so.

Alan Stern


^ permalink raw reply

* Re: [PATCH v9 07/10] block: add a new interface to block events
From: Tejun Heo @ 2012-11-18 23:35 UTC (permalink / raw)
  To: Alan Stern
  Cc: Aaron Lu, Jeff Garzik, James Bottomley, Rafael J. Wysocki,
	Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <Pine.LNX.4.44L0.1211181822320.9225-100000@netrider.rowland.org>

Hey, Alan.

On Sun, Nov 18, 2012 at 06:28:24PM -0500, Alan Stern wrote:
> > The problem is that SATA devices aren't really designed to be used
> > like that.  If you reset a ODD w/ a media in it, it'll probably spin
> > it up and try to re-establish media state during probe sequence.  It
> > isn't designed that way and has never been used like that.  SATA has
> > its own dynamic link power management (DIPM/HIPM) tho.
> 
> Is it possible to use those to implement runtime suspend?  Or are they 
> handled autonomously by the hardware?

It's controlled by /sys/class/scsi_host/hostX/link_power_management.
Once enabled, the power saving is completedly handled by hardware.  If
link stays idle longer than certain duration, the hardware initiates
low power state and leaves it when something needs to happen.

> > So, this whole autopm thing doesn't sound like a good idea to me.
> 
> No doubt it's better suited to some devices than to others.

Yeah, SATA seems to need a different approach than USB.

> > I think the only reason autopm doesn't blow up for SATA devices is
> > that userland usually automatically mounts them thus effectively
> > disabling autopm.
> 
> Either that or else because it hasn't been fully implemented yet.  :-)

Ah.. that makes sense. :)

> >  I *think* the only sane thing we can do is doing
> > autopm iff zpodd is available && no media.
> 
> That may be true for SATA.  For USB optical drives, it does make sense
> to power-down the host controller when the drive isn't in use.  USB
> suspend/resume takes on the order of 50-100 ms or so.

I see.  For SATA too, the controller / link bring up doesn't take too
long.  The crappy part is what the attached, especially ATAPI, devices
would do after such events as those events will be visible to them
basically as random link resets.

So, at least for SATA, I think what autopm can do is...

* Trigger zpodd if possible.

* Trigger suspend iff polling isn't happening on the device.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH V6 1/2] Thermal: Add ST-Ericsson DB8500 thermal driver.
From: Zhang Rui @ 2012-11-19  0:42 UTC (permalink / raw)
  To: Francesco Lavra
  Cc: hongbo.zhang, linaro-kernel, linaro-dev, patches, linux-pm,
	linux-kernel, amit.kachhap, STEricsson_nomadik_linux, kernel,
	hongbo.zhang
In-Reply-To: <50A8F0DF.8040509@gmail.com>

On Sun, 2012-11-18 at 15:29 +0100, Francesco Lavra wrote:
> On 11/15/2012 01:51 PM, Zhang Rui wrote:
> > On Thu, 2012-11-15 at 18:56 +0800, hongbo.zhang wrote:
> >> From: "hongbo.zhang" <hongbo.zhang@linaro.com>
> >>
> >> This driver is based on the thermal management framework in thermal_sys.c. A
> >> thermal zone device is created with the trip points to which cooling devices
> >> can be bound, the current cooling device is cpufreq, e.g. CPU frequency is
> >> clipped down to cool the CPU, and other cooling devices can be added and bound
> >> to the trip points dynamically.  The platform specific PRCMU interrupts are
> >> used to active thermal update when trip points are reached.
> >>
> >> Signed-off-by: hongbo.zhang <hongbo.zhang@linaro.com>
> >> Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
> >> Reviewed-by: Francesco Lavra <francescolavra.fl@gmail.com>
> > 
> > Patch is refreshed and applied to thermal next.
> > refreshed patch attached.
> [...]
> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > index 99b6587..d96da07 100644
> > --- a/drivers/thermal/Kconfig
> > +++ b/drivers/thermal/Kconfig
> > @@ -101,5 +101,25 @@ config EXYNOS_THERMAL
> >  	  If you say yes here you get support for TMU (Thermal Managment
> >  	  Unit) on SAMSUNG EXYNOS series of SoC.
> >  
> > +config DB8500_THERMAL
> > +	bool "DB8500 thermal management"
> > +	depends on ARCH_U8500
> 
> Shouldn't it depend on THERMAL as well, as in Hongbo's original patch?
> 
all of these options are available only if CONFIG_THERMAL is set.
please refer to commit 72e198978223f2020f7f59a6e2520f2b7d005e72 in the
thermal next tree.

thanks,
rui
> > +	default y
> > +	help
> > +	  Adds DB8500 thermal management implementation according to the thermal
> > +	  management framework. A thermal zone with several trip points will be
> > +	  created. Cooling devices can be bound to the trip points to cool this
> > +	  thermal zone if trip points reached.
> > +
> > +config DB8500_CPUFREQ_COOLING
> > +	tristate "DB8500 cpufreq cooling"
> > +	depends on ARCH_U8500
> > +	depends on CPU_THERMAL
> > +	default y
> > +	help
> > +	  Adds DB8500 cpufreq cooling devices, and these cooling devices can be
> > +	  bound to thermal zone trip points. When a trip point reached, the
> > +	  bound cpufreq cooling device turns active to set CPU frequency low to
> > +	  cool down the CPU.
> >  
> >  endif
> 
> --
> Francesco

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox