linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ACPI-Thermal: Make Thermal trip points writeable
@ 2012-01-27 16:28 Durgadoss R
  2012-03-30 10:05 ` Len Brown
  2012-04-05  5:52 ` Amit Daniel Kachhap
  0 siblings, 2 replies; 6+ messages in thread
From: Durgadoss R @ 2012-01-27 16:28 UTC (permalink / raw)
  To: linux-acpi; +Cc: lenb, rui.zhang, Durgadoss R

Some of the thermal drivers using the Generic Thermal Framework
require (all/some) trip points to be writeable. This patch makes
the trip point temperatures writeable on a per-trip point basis,
and modifies the required function call in thermal.c. This patch
also updates the Documentation to reflect the new change.

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
v1
* patch1/2: Code for making trip points writeable
* patch2/2: Change the callee in thermal.c
v2
* Make both the changes in a single patch
* Update Documentation/thermal/sysfs-api.txt
---
 Documentation/thermal/sysfs-api.txt |    4 +-
 drivers/acpi/thermal.c              |    4 +-
 drivers/thermal/thermal_sys.c       |  127 ++++++++++++++++++++++-------------
 include/linux/thermal.h             |   10 ++-
 4 files changed, 91 insertions(+), 54 deletions(-)

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index b61e46f..cd1476c 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -32,7 +32,8 @@ temperature) and throttle appropriate devices.
 
 1.1 thermal zone device interface
 1.1.1 struct thermal_zone_device *thermal_zone_device_register(char *name,
-		int trips, void *devdata, struct thermal_zone_device_ops *ops)
+		int trips, int flag, void *devdata,
+		struct thermal_zone_device_ops *ops)
 
     This interface function adds a new thermal zone device (sensor) to
     /sys/class/thermal folder as thermal_zone[0-*]. It tries to bind all the
@@ -40,6 +41,7 @@ temperature) and throttle appropriate devices.
 
     name: the thermal zone name.
     trips: the total number of trip points this thermal zone supports.
+    flag: Bit string: If 'n'th bit is set, then trip point 'n' is writeable.
     devdata: device private data
     ops: thermal zone device call-backs.
 	.bind: bind the thermal zone device with a thermal cooling device.
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 48fbc64..1c3133e 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -845,7 +845,7 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
 
 	if (tz->trips.passive.flags.valid)
 		tz->thermal_zone =
-			thermal_zone_device_register("acpitz", trips, tz,
+			thermal_zone_device_register("acpitz", trips, 0, tz,
 						     &acpi_thermal_zone_ops,
 						     tz->trips.passive.tc1,
 						     tz->trips.passive.tc2,
@@ -853,7 +853,7 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
 						     tz->polling_frequency*100);
 	else
 		tz->thermal_zone =
-			thermal_zone_device_register("acpitz", trips, tz,
+			thermal_zone_device_register("acpitz", trips, 0, tz,
 						     &acpi_thermal_zone_ops,
 						     0, 0, 0,
 						     tz->polling_frequency*100);
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index dd9a574..3a4341e 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -220,6 +220,28 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr,
 }
 
 static ssize_t
+trip_point_temp_store(struct device *dev, struct device_attribute *attr,
+		     const char *buf, size_t count)
+{
+	struct thermal_zone_device *tz = to_thermal_zone(dev);
+	int trip, ret;
+	unsigned long temperature;
+
+	if (!tz->ops->set_trip_temp)
+		return -EPERM;
+
+	if (!sscanf(attr->attr.name, "trip_point_%d_temp", &trip))
+		return -EINVAL;
+
+	if (kstrtoul(buf, 10, &temperature))
+		return -EINVAL;
+
+	ret = tz->ops->set_trip_temp(tz, trip, temperature);
+
+	return ret ? ret : count;
+}
+
+static ssize_t
 passive_store(struct device *dev, struct device_attribute *attr,
 		    const char *buf, size_t count)
 {
@@ -286,49 +308,6 @@ static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
 static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show, \
 		   passive_store);
 
-static struct device_attribute trip_point_attrs[] = {
-	__ATTR(trip_point_0_type, 0444, trip_point_type_show, NULL),
-	__ATTR(trip_point_0_temp, 0444, trip_point_temp_show, NULL),
-	__ATTR(trip_point_1_type, 0444, trip_point_type_show, NULL),
-	__ATTR(trip_point_1_temp, 0444, trip_point_temp_show, NULL),
-	__ATTR(trip_point_2_type, 0444, trip_point_type_show, NULL),
-	__ATTR(trip_point_2_temp, 0444, trip_point_temp_show, NULL),
-	__ATTR(trip_point_3_type, 0444, trip_point_type_show, NULL),
-	__ATTR(trip_point_3_temp, 0444, trip_point_temp_show, NULL),
-	__ATTR(trip_point_4_type, 0444, trip_point_type_show, NULL),
-	__ATTR(trip_point_4_temp, 0444, trip_point_temp_show, NULL),
-	__ATTR(trip_point_5_type, 0444, trip_point_type_show, NULL),
-	__ATTR(trip_point_5_temp, 0444, trip_point_temp_show, NULL),
-	__ATTR(trip_point_6_type, 0444, trip_point_type_show, NULL),
-	__ATTR(trip_point_6_temp, 0444, trip_point_temp_show, NULL),
-	__ATTR(trip_point_7_type, 0444, trip_point_type_show, NULL),
-	__ATTR(trip_point_7_temp, 0444, trip_point_temp_show, NULL),
-	__ATTR(trip_point_8_type, 0444, trip_point_type_show, NULL),
-	__ATTR(trip_point_8_temp, 0444, trip_point_temp_show, NULL),
-	__ATTR(trip_point_9_type, 0444, trip_point_type_show, NULL),
-	__ATTR(trip_point_9_temp, 0444, trip_point_temp_show, NULL),
-	__ATTR(trip_point_10_type, 0444, trip_point_type_show, NULL),
-	__ATTR(trip_point_10_temp, 0444, trip_point_temp_show, NULL),
-	__ATTR(trip_point_11_type, 0444, trip_point_type_show, NULL),
-	__ATTR(trip_point_11_temp, 0444, trip_point_temp_show, NULL),
-};
-
-#define TRIP_POINT_ATTR_ADD(_dev, _index, result)     \
-do {    \
-	result = device_create_file(_dev,	\
-				&trip_point_attrs[_index * 2]);	\
-	if (result)	\
-		break;	\
-	result = device_create_file(_dev,	\
-			&trip_point_attrs[_index * 2 + 1]);	\
-} while (0)
-
-#define TRIP_POINT_ATTR_REMOVE(_dev, _index)	\
-do {	\
-	device_remove_file(_dev, &trip_point_attrs[_index * 2]);	\
-	device_remove_file(_dev, &trip_point_attrs[_index * 2 + 1]);	\
-} while (0)
-
 /* sys I/F for cooling device */
 #define to_cooling_device(_dev)	\
 	container_of(_dev, struct thermal_cooling_device, device)
@@ -1112,9 +1091,54 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
 EXPORT_SYMBOL(thermal_zone_device_update);
 
 /**
+ * create_trip_type_attr - creates a trip point type attribute
+ * @tz:		the thermal zone device
+ * @indx:	index into the trip_type_attrs array
+ */
+static int create_trip_type_attr(struct thermal_zone_device *tz, int indx)
+{
+	char attr_name[THERMAL_NAME_LENGTH];
+
+	snprintf(attr_name, THERMAL_NAME_LENGTH, "trip_point_%d_type", indx);
+
+	sysfs_attr_init(&tz->trip_type_attrs[indx].attr);
+	tz->trip_type_attrs[indx].attr.name = attr_name;
+	tz->trip_type_attrs[indx].attr.mode = S_IRUGO;
+	tz->trip_type_attrs[indx].show = trip_point_type_show;
+
+	return device_create_file(&tz->device, &tz->trip_type_attrs[indx]);
+}
+
+/**
+ * create_trip_temp_attr - creates a trip point temp attribute
+ * @tz:		the thermal zone device
+ * @indx:	index into the trip_type_attrs array
+ * @writeable:	A flag: If 1, 'this' trip point is writeable; otherwise not
+ */
+static int create_trip_temp_attr(struct thermal_zone_device *tz,
+				int indx, int writeable)
+{
+	char attr_name[THERMAL_NAME_LENGTH];
+
+	snprintf(attr_name, THERMAL_NAME_LENGTH, "trip_point_%d_temp", indx);
+
+	sysfs_attr_init(&tz->trip_temp_attrs[indx].attr);
+	tz->trip_temp_attrs[indx].attr.name = attr_name;
+	tz->trip_temp_attrs[indx].attr.mode = S_IRUGO;
+	tz->trip_temp_attrs[indx].show = trip_point_temp_show;
+	if (writeable) {
+		tz->trip_temp_attrs[indx].attr.mode |= S_IWUSR;
+		tz->trip_temp_attrs[indx].store = trip_point_temp_store;
+	}
+
+	return device_create_file(&tz->device, &tz->trip_temp_attrs[indx]);
+}
+
+/**
  * thermal_zone_device_register - register a new thermal zone device
  * @type:	the thermal zone device type
  * @trips:	the number of trip points the thermal zone support
+ * @flag:	a bit string indicating the writeablility of trip points
  * @devdata:	private device data
  * @ops:	standard thermal zone device callbacks
  * @tc1:	thermal coefficient 1 for passive calculations
@@ -1130,7 +1154,7 @@ EXPORT_SYMBOL(thermal_zone_device_update);
  * section 11.1.5.1 of the ACPI specification 3.0.
  */
 struct thermal_zone_device *thermal_zone_device_register(char *type,
-	int trips, void *devdata,
+	int trips, int flag, void *devdata,
 	const struct thermal_zone_device_ops *ops,
 	int tc1, int tc2, int passive_delay, int polling_delay)
 {
@@ -1199,9 +1223,15 @@ struct thermal_zone_device *thermal_zone_device_register(char *type,
 	}
 
 	for (count = 0; count < trips; count++) {
-		TRIP_POINT_ATTR_ADD(&tz->device, count, result);
+		result = create_trip_type_attr(tz, count);
 		if (result)
 			goto unregister;
+
+		result = create_trip_temp_attr(tz, count,
+						(flag & (1 << count)));
+		if (result)
+			goto unregister;
+
 		tz->ops->get_trip_type(tz, count, &trip_type);
 		if (trip_type == THERMAL_TRIP_PASSIVE)
 			passive = 1;
@@ -1279,9 +1309,10 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
 	if (tz->ops->get_mode)
 		device_remove_file(&tz->device, &dev_attr_mode);
 
-	for (count = 0; count < tz->trips; count++)
-		TRIP_POINT_ATTR_REMOVE(&tz->device, count);
-
+	for (count = 0; count < tz->trips; count++) {
+		device_remove_file(&tz->device, &tz->trip_type_attrs[count]);
+		device_remove_file(&tz->device, &tz->trip_temp_attrs[count]);
+	}
 	thermal_remove_hwmon_sysfs(tz);
 	release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
 	idr_destroy(&tz->idr);
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 47b4a27..4aa5a45 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -58,6 +58,8 @@ struct thermal_zone_device_ops {
 		enum thermal_trip_type *);
 	int (*get_trip_temp) (struct thermal_zone_device *, int,
 			      unsigned long *);
+	int (*set_trip_temp) (struct thermal_zone_device *, int,
+			      unsigned long);
 	int (*get_crit_temp) (struct thermal_zone_device *, unsigned long *);
 	int (*notify) (struct thermal_zone_device *, int,
 		       enum thermal_trip_type);
@@ -89,6 +91,8 @@ struct thermal_zone_device {
 	int id;
 	char type[THERMAL_NAME_LENGTH];
 	struct device device;
+	struct device_attribute trip_temp_attrs[THERMAL_MAX_TRIPS];
+	struct device_attribute trip_type_attrs[THERMAL_MAX_TRIPS];
 	void *devdata;
 	int trips;
 	int tc1;
@@ -137,9 +141,9 @@ enum {
 };
 #define THERMAL_GENL_CMD_MAX (__THERMAL_GENL_CMD_MAX - 1)
 
-struct thermal_zone_device *thermal_zone_device_register(char *, int, void *,
-		const struct thermal_zone_device_ops *, int tc1, int tc2,
-		int passive_freq, int polling_freq);
+struct thermal_zone_device *thermal_zone_device_register(char *, int, int,
+		void *, const struct thermal_zone_device_ops *, int tc1,
+		int tc2, int passive_freq, int polling_freq);
 void thermal_zone_device_unregister(struct thermal_zone_device *);
 
 int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ACPI-Thermal: Make Thermal trip points writeable
  2012-01-27 16:28 [PATCH] ACPI-Thermal: Make Thermal trip points writeable Durgadoss R
@ 2012-03-30 10:05 ` Len Brown
  2012-04-05  5:52 ` Amit Daniel Kachhap
  1 sibling, 0 replies; 6+ messages in thread
From: Len Brown @ 2012-03-30 10:05 UTC (permalink / raw)
  To: Durgadoss R; +Cc: linux-acpi, rui.zhang

Durga,
I failed to merge this properly with the other thermal patches
and had to drop it.  If you can refresh for me on top of my "next"
branch, that would be great.

thanks,
-Len Brown, Intel Open Source Technology Center



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ACPI-Thermal: Make Thermal trip points writeable
  2012-01-27 16:28 [PATCH] ACPI-Thermal: Make Thermal trip points writeable Durgadoss R
  2012-03-30 10:05 ` Len Brown
@ 2012-04-05  5:52 ` Amit Daniel Kachhap
  2012-04-05  7:43   ` R, Durgadoss
  2012-04-06 13:01   ` Rob Lee
  1 sibling, 2 replies; 6+ messages in thread
From: Amit Daniel Kachhap @ 2012-04-05  5:52 UTC (permalink / raw)
  To: durgadoss.r; +Cc: linux-acpi, lenb, rui.zhang, amit.kachhap, linaro-dev

Hi Durgadoss,

Instead of making all the trip-points of a thermal zone writeable we should
only allow non-critical trip points to be writeable.

Thanks,
Amit Daniel

> -----Original Message-----
> From: Durgadoss R <durgadoss.r@intel.com>
> Sent: Friday, January 27, 2012 9:58 PM
> To: linux-acpi@vger.kernel.org
> Cc: lenb@kernel.org; rui.zhang@intel.com; durgadoss.r@intel.com
> Subject: [PATCH] ACPI-Thermal: Make Thermal trip points writeable

>Some of the thermal drivers using the Generic Thermal Framework
>require (all/some) trip points to be writeable. This patch makes
>the trip point temperatures writeable on a per-trip point basis,
>and modifies the required function call in thermal.c. This patch
>also updates the Documentation to reflect the new change.
>
>Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
>---
>v1
>* patch1/2: Code for making trip points writeable
>* patch2/2: Change the callee in thermal.c
>v2
>* Make both the changes in a single patch
>* Update Documentation/thermal/sysfs-api.txt
>---
> Documentation/thermal/sysfs-api.txt |    4 +-
> drivers/acpi/thermal.c              |    4 +-
> drivers/thermal/thermal_sys.c       |  127 ++++++++++++++++++++++-------------
> include/linux/thermal.h             |   10 ++-
> 4 files changed, 91 insertions(+), 54 deletions(-)
>
>diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
>index b61e46f..cd1476c 100644
>--- a/Documentation/thermal/sysfs-api.txt
>+++ b/Documentation/thermal/sysfs-api.txt
>@@ -32,7 +32,8 @@ temperature) and throttle appropriate devices.
> 
> 1.1 thermal zone device interface
> 1.1.1 struct thermal_zone_device *thermal_zone_device_register(char *name,
>-		int trips, void *devdata, struct thermal_zone_device_ops *ops)
>+		int trips, int flag, void *devdata,
>+		struct thermal_zone_device_ops *ops)
> 
>     This interface function adds a new thermal zone device (sensor) to
>     /sys/class/thermal folder as thermal_zone[0-*]. It tries to bind all the
>@@ -40,6 +41,7 @@ temperature) and throttle appropriate devices.
> 
>     name: the thermal zone name.
>     trips: the total number of trip points this thermal zone supports.
>+    flag: Bit string: If 'n'th bit is set, then trip point 'n' is writeable.
>     devdata: device private data
>     ops: thermal zone device call-backs.
> 	.bind: bind the thermal zone device with a thermal cooling device.
>diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
>index 48fbc64..1c3133e 100644
>--- a/drivers/acpi/thermal.c
>+++ b/drivers/acpi/thermal.c
>@@ -845,7 +845,7 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
> 
> 	if (tz->trips.passive.flags.valid)
> 		tz->thermal_zone =
>-			thermal_zone_device_register("acpitz", trips, tz,
>+			thermal_zone_device_register("acpitz", trips, 0, tz,
> 						     &acpi_thermal_zone_ops,
> 						     tz->trips.passive.tc1,
> 						     tz->trips.passive.tc2,
>@@ -853,7 +853,7 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
> 						     tz->polling_frequency*100);
> 	else
> 		tz->thermal_zone =
>-			thermal_zone_device_register("acpitz", trips, tz,
>+			thermal_zone_device_register("acpitz", trips, 0, tz,
> 						     &acpi_thermal_zone_ops,
> 						     0, 0, 0,
> 						     tz->polling_frequency*100);
>diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
>index dd9a574..3a4341e 100644
>--- a/drivers/thermal/thermal_sys.c
>+++ b/drivers/thermal/thermal_sys.c
>@@ -220,6 +220,28 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr,
> }
> 
> static ssize_t
>+trip_point_temp_store(struct device *dev, struct device_attribute *attr,
>+		     const char *buf, size_t count)
>+{
>+	struct thermal_zone_device *tz = to_thermal_zone(dev);
>+	int trip, ret;
>+	unsigned long temperature;
>+
>+	if (!tz->ops->set_trip_temp)
>+		return -EPERM;
>+
>+	if (!sscanf(attr->attr.name, "trip_point_%d_temp", &trip))
>+		return -EINVAL;
>+
>+	if (kstrtoul(buf, 10, &temperature))
>+		return -EINVAL;
>+
>+	ret = tz->ops->set_trip_temp(tz, trip, temperature);
>+
>+	return ret ? ret : count;
>+}
>+
>+static ssize_t
> passive_store(struct device *dev, struct device_attribute *attr,
> 		    const char *buf, size_t count)
> {
>@@ -286,49 +308,6 @@ static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
> static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show, \
> 		   passive_store);
> 
>-static struct device_attribute trip_point_attrs[] = {
>-	__ATTR(trip_point_0_type, 0444, trip_point_type_show, NULL),
>-	__ATTR(trip_point_0_temp, 0444, trip_point_temp_show, NULL),
>-	__ATTR(trip_point_1_type, 0444, trip_point_type_show, NULL),
>-	__ATTR(trip_point_1_temp, 0444, trip_point_temp_show, NULL),
>-	__ATTR(trip_point_2_type, 0444, trip_point_type_show, NULL),
>-	__ATTR(trip_point_2_temp, 0444, trip_point_temp_show, NULL),
>-	__ATTR(trip_point_3_type, 0444, trip_point_type_show, NULL),
>-	__ATTR(trip_point_3_temp, 0444, trip_point_temp_show, NULL),
>-	__ATTR(trip_point_4_type, 0444, trip_point_type_show, NULL),
>-	__ATTR(trip_point_4_temp, 0444, trip_point_temp_show, NULL),
>-	__ATTR(trip_point_5_type, 0444, trip_point_type_show, NULL),
>-	__ATTR(trip_point_5_temp, 0444, trip_point_temp_show, NULL),
>-	__ATTR(trip_point_6_type, 0444, trip_point_type_show, NULL),
>-	__ATTR(trip_point_6_temp, 0444, trip_point_temp_show, NULL),
>-	__ATTR(trip_point_7_type, 0444, trip_point_type_show, NULL),
>-	__ATTR(trip_point_7_temp, 0444, trip_point_temp_show, NULL),
>-	__ATTR(trip_point_8_type, 0444, trip_point_type_show, NULL),
>-	__ATTR(trip_point_8_temp, 0444, trip_point_temp_show, NULL),
>-	__ATTR(trip_point_9_type, 0444, trip_point_type_show, NULL),
>-	__ATTR(trip_point_9_temp, 0444, trip_point_temp_show, NULL),
>-	__ATTR(trip_point_10_type, 0444, trip_point_type_show, NULL),
>-	__ATTR(trip_point_10_temp, 0444, trip_point_temp_show, NULL),
>-	__ATTR(trip_point_11_type, 0444, trip_point_type_show, NULL),
>-	__ATTR(trip_point_11_temp, 0444, trip_point_temp_show, NULL),
>-};
>-
>-#define TRIP_POINT_ATTR_ADD(_dev, _index, result)     \
>-do {    \
>-	result = device_create_file(_dev,	\
>-				&trip_point_attrs[_index * 2]);	\
>-	if (result)	\
>-		break;	\
>-	result = device_create_file(_dev,	\
>-			&trip_point_attrs[_index * 2 + 1]);	\
>-} while (0)
>-
>-#define TRIP_POINT_ATTR_REMOVE(_dev, _index)	\
>-do {	\
>-	device_remove_file(_dev, &trip_point_attrs[_index * 2]);	\
>-	device_remove_file(_dev, &trip_point_attrs[_index * 2 + 1]);	\
>-} while (0)
>-
> /* sys I/F for cooling device */
> #define to_cooling_device(_dev)	\
> 	container_of(_dev, struct thermal_cooling_device, device)
>@@ -1112,9 +1091,54 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
> EXPORT_SYMBOL(thermal_zone_device_update);
> 
> /**
>+ * create_trip_type_attr - creates a trip point type attribute
>+ * @tz:		the thermal zone device
>+ * @indx:	index into the trip_type_attrs array
>+ */
>+static int create_trip_type_attr(struct thermal_zone_device *tz, int indx)
>+{
>+	char attr_name[THERMAL_NAME_LENGTH];
>+
>+	snprintf(attr_name, THERMAL_NAME_LENGTH, "trip_point_%d_type", indx);
>+
>+	sysfs_attr_init(&tz->trip_type_attrs[indx].attr);
>+	tz->trip_type_attrs[indx].attr.name = attr_name;
>+	tz->trip_type_attrs[indx].attr.mode = S_IRUGO;
>+	tz->trip_type_attrs[indx].show = trip_point_type_show;
>+
>+	return device_create_file(&tz->device, &tz->trip_type_attrs[indx]);
>+}
>+
>+/**
>+ * create_trip_temp_attr - creates a trip point temp attribute
>+ * @tz:		the thermal zone device
>+ * @indx:	index into the trip_type_attrs array
>+ * @writeable:	A flag: If 1, 'this' trip point is writeable; otherwise not
>+ */
>+static int create_trip_temp_attr(struct thermal_zone_device *tz,
>+				int indx, int writeable)
>+{
>+	char attr_name[THERMAL_NAME_LENGTH];
>+
>+	snprintf(attr_name, THERMAL_NAME_LENGTH, "trip_point_%d_temp", indx);
>+
>+	sysfs_attr_init(&tz->trip_temp_attrs[indx].attr);
>+	tz->trip_temp_attrs[indx].attr.name = attr_name;
>+	tz->trip_temp_attrs[indx].attr.mode = S_IRUGO;
>+	tz->trip_temp_attrs[indx].show = trip_point_temp_show;
>+	if (writeable) {
>+		tz->trip_temp_attrs[indx].attr.mode |= S_IWUSR;
>+		tz->trip_temp_attrs[indx].store = trip_point_temp_store;
>+	}
>+
>+	return device_create_file(&tz->device, &tz->trip_temp_attrs[indx]);
>+}
>+
>+/**
>  * thermal_zone_device_register - register a new thermal zone device
>  * @type:	the thermal zone device type
>  * @trips:	the number of trip points the thermal zone support
>+ * @flag:	a bit string indicating the writeablility of trip points
>  * @devdata:	private device data
>  * @ops:	standard thermal zone device callbacks
>  * @tc1:	thermal coefficient 1 for passive calculations
>@@ -1130,7 +1154,7 @@ EXPORT_SYMBOL(thermal_zone_device_update);
>  * section 11.1.5.1 of the ACPI specification 3.0.
>  */
> struct thermal_zone_device *thermal_zone_device_register(char *type,
>-	int trips, void *devdata,
>+	int trips, int flag, void *devdata,
> 	const struct thermal_zone_device_ops *ops,
> 	int tc1, int tc2, int passive_delay, int polling_delay)
> {
>@@ -1199,9 +1223,15 @@ struct thermal_zone_device *thermal_zone_device_register(char *type,
> 	}
> 
> 	for (count = 0; count < trips; count++) {
>-		TRIP_POINT_ATTR_ADD(&tz->device, count, result);
>+		result = create_trip_type_attr(tz, count);
> 		if (result)
> 			goto unregister;
>+
>+		result = create_trip_temp_attr(tz, count,
>+						(flag & (1 << count)));
>+		if (result)
>+			goto unregister;
>+
> 		tz->ops->get_trip_type(tz, count, &trip_type);
> 		if (trip_type == THERMAL_TRIP_PASSIVE)
> 			passive = 1;
>@@ -1279,9 +1309,10 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
> 	if (tz->ops->get_mode)
> 		device_remove_file(&tz->device, &dev_attr_mode);
> 
>-	for (count = 0; count < tz->trips; count++)
>-		TRIP_POINT_ATTR_REMOVE(&tz->device, count);
>-
>+	for (count = 0; count < tz->trips; count++) {
>+		device_remove_file(&tz->device, &tz->trip_type_attrs[count]);
>+		device_remove_file(&tz->device, &tz->trip_temp_attrs[count]);
>+	}
> 	thermal_remove_hwmon_sysfs(tz);
> 	release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
> 	idr_destroy(&tz->idr);
>diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>index 47b4a27..4aa5a45 100644
>--- a/include/linux/thermal.h
>+++ b/include/linux/thermal.h
>@@ -58,6 +58,8 @@ struct thermal_zone_device_ops {
> 		enum thermal_trip_type *);
> 	int (*get_trip_temp) (struct thermal_zone_device *, int,
> 			      unsigned long *);
>+	int (*set_trip_temp) (struct thermal_zone_device *, int,
>+			      unsigned long);
> 	int (*get_crit_temp) (struct thermal_zone_device *, unsigned long *);
> 	int (*notify) (struct thermal_zone_device *, int,
> 		       enum thermal_trip_type);
>@@ -89,6 +91,8 @@ struct thermal_zone_device {
> 	int id;
> 	char type[THERMAL_NAME_LENGTH];
> 	struct device device;
>+	struct device_attribute trip_temp_attrs[THERMAL_MAX_TRIPS];
>+	struct device_attribute trip_type_attrs[THERMAL_MAX_TRIPS];
> 	void *devdata;
> 	int trips;
> 	int tc1;
>@@ -137,9 +141,9 @@ enum {
> };
> #define THERMAL_GENL_CMD_MAX (__THERMAL_GENL_CMD_MAX - 1)
> 
>-struct thermal_zone_device *thermal_zone_device_register(char *, int, void *,
>-		const struct thermal_zone_device_ops *, int tc1, int tc2,
>-		int passive_freq, int polling_freq);
>+struct thermal_zone_device *thermal_zone_device_register(char *, int, int,
>+		void *, const struct thermal_zone_device_ops *, int tc1,
>+		int tc2, int passive_freq, int polling_freq);
> void thermal_zone_device_unregister(struct thermal_zone_device *);
> 
> int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
>-- 
>1.7.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] ACPI-Thermal: Make Thermal trip points writeable
  2012-04-05  5:52 ` Amit Daniel Kachhap
@ 2012-04-05  7:43   ` R, Durgadoss
  2012-04-06 13:01   ` Rob Lee
  1 sibling, 0 replies; 6+ messages in thread
From: R, Durgadoss @ 2012-04-05  7:43 UTC (permalink / raw)
  To: Amit Daniel Kachhap
  Cc: linux-acpi@vger.kernel.org, lenb@kernel.org, Zhang, Rui,
	linaro-dev@lists.linaro.org

Hi Amit,

> -----Original Message-----
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> owner@vger.kernel.org] On Behalf Of Amit Daniel Kachhap
> Sent: Thursday, April 05, 2012 11:22 AM
> To: R, Durgadoss
> Cc: linux-acpi@vger.kernel.org; lenb@kernel.org; Zhang, Rui;
> amit.kachhap@linaro.org; linaro-dev@lists.linaro.org
> Subject: Re: [PATCH] ACPI-Thermal: Make Thermal trip points writeable
> 
> Hi Durgadoss,
> 
> Instead of making all the trip-points of a thermal zone writeable we should
> only allow non-critical trip points to be writeable.

We are not making all trip points writeable. It is on a per-trip point basis.
The 'flag' variable determines whether a particular trip point is writeable or not.

Thanks,
Durga

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ACPI-Thermal: Make Thermal trip points writeable
  2012-04-05  5:52 ` Amit Daniel Kachhap
  2012-04-05  7:43   ` R, Durgadoss
@ 2012-04-06 13:01   ` Rob Lee
  2012-04-06 13:18     ` Rob Lee
  1 sibling, 1 reply; 6+ messages in thread
From: Rob Lee @ 2012-04-06 13:01 UTC (permalink / raw)
  To: Amit Daniel Kachhap; +Cc: durgadoss.r, linux-acpi, rui.zhang, linaro-dev, lenb

On Thu, Apr 5, 2012 at 12:52 AM, Amit Daniel Kachhap
<amit.kachhap@linaro.org> wrote:
> Hi Durgadoss,
>
> Instead of making all the trip-points of a thermal zone writeable we should
> only allow non-critical trip points to be writeable.
>
> Thanks,
> Amit Daniel
>

Agree, or even better, could the writeable attribute be made an
optional for any trip point?

Many SoCs now have built in thermal sensing and for cost reasons some
device makers may want to rely on the kernel thermal handling
capabilities as their primary thermal protection.  Allowing userspace
to modify the trip points may increase their risk/exposure to lawsuits
or other undesirable results in some cases.

Best Regards,
Rob

>> -----Original Message-----
>> From: Durgadoss R <durgadoss.r@intel.com>
>> Sent: Friday, January 27, 2012 9:58 PM
>> To: linux-acpi@vger.kernel.org
>> Cc: lenb@kernel.org; rui.zhang@intel.com; durgadoss.r@intel.com
>> Subject: [PATCH] ACPI-Thermal: Make Thermal trip points writeable
>
>>Some of the thermal drivers using the Generic Thermal Framework
>>require (all/some) trip points to be writeable. This patch makes
>>the trip point temperatures writeable on a per-trip point basis,
>>and modifies the required function call in thermal.c. This patch
>>also updates the Documentation to reflect the new change.
>>
>>Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
>>---
>>v1
>>* patch1/2: Code for making trip points writeable
>>* patch2/2: Change the callee in thermal.c
>>v2
>>* Make both the changes in a single patch
>>* Update Documentation/thermal/sysfs-api.txt
>>---
>> Documentation/thermal/sysfs-api.txt |    4 +-
>> drivers/acpi/thermal.c              |    4 +-
>> drivers/thermal/thermal_sys.c       |  127 ++++++++++++++++++++++-------------
>> include/linux/thermal.h             |   10 ++-
>> 4 files changed, 91 insertions(+), 54 deletions(-)
>>
>>diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
>>index b61e46f..cd1476c 100644
>>--- a/Documentation/thermal/sysfs-api.txt
>>+++ b/Documentation/thermal/sysfs-api.txt
>>@@ -32,7 +32,8 @@ temperature) and throttle appropriate devices.
>>
>> 1.1 thermal zone device interface
>> 1.1.1 struct thermal_zone_device *thermal_zone_device_register(char *name,
>>-              int trips, void *devdata, struct thermal_zone_device_ops *ops)
>>+              int trips, int flag, void *devdata,
>>+              struct thermal_zone_device_ops *ops)
>>
>>     This interface function adds a new thermal zone device (sensor) to
>>     /sys/class/thermal folder as thermal_zone[0-*]. It tries to bind all the
>>@@ -40,6 +41,7 @@ temperature) and throttle appropriate devices.
>>
>>     name: the thermal zone name.
>>     trips: the total number of trip points this thermal zone supports.
>>+    flag: Bit string: If 'n'th bit is set, then trip point 'n' is writeable.
>>     devdata: device private data
>>     ops: thermal zone device call-backs.
>>       .bind: bind the thermal zone device with a thermal cooling device.
>>diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
>>index 48fbc64..1c3133e 100644
>>--- a/drivers/acpi/thermal.c
>>+++ b/drivers/acpi/thermal.c
>>@@ -845,7 +845,7 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
>>
>>       if (tz->trips.passive.flags.valid)
>>               tz->thermal_zone =
>>-                      thermal_zone_device_register("acpitz", trips, tz,
>>+                      thermal_zone_device_register("acpitz", trips, 0, tz,
>>                                                    &acpi_thermal_zone_ops,
>>                                                    tz->trips.passive.tc1,
>>                                                    tz->trips.passive.tc2,
>>@@ -853,7 +853,7 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
>>                                                    tz->polling_frequency*100);
>>       else
>>               tz->thermal_zone =
>>-                      thermal_zone_device_register("acpitz", trips, tz,
>>+                      thermal_zone_device_register("acpitz", trips, 0, tz,
>>                                                    &acpi_thermal_zone_ops,
>>                                                    0, 0, 0,
>>                                                    tz->polling_frequency*100);
>>diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
>>index dd9a574..3a4341e 100644
>>--- a/drivers/thermal/thermal_sys.c
>>+++ b/drivers/thermal/thermal_sys.c
>>@@ -220,6 +220,28 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr,
>> }
>>
>> static ssize_t
>>+trip_point_temp_store(struct device *dev, struct device_attribute *attr,
>>+                   const char *buf, size_t count)
>>+{
>>+      struct thermal_zone_device *tz = to_thermal_zone(dev);
>>+      int trip, ret;
>>+      unsigned long temperature;
>>+
>>+      if (!tz->ops->set_trip_temp)
>>+              return -EPERM;
>>+
>>+      if (!sscanf(attr->attr.name, "trip_point_%d_temp", &trip))
>>+              return -EINVAL;
>>+
>>+      if (kstrtoul(buf, 10, &temperature))
>>+              return -EINVAL;
>>+
>>+      ret = tz->ops->set_trip_temp(tz, trip, temperature);
>>+
>>+      return ret ? ret : count;
>>+}
>>+
>>+static ssize_t
>> passive_store(struct device *dev, struct device_attribute *attr,
>>                   const char *buf, size_t count)
>> {
>>@@ -286,49 +308,6 @@ static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
>> static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show, \
>>                  passive_store);
>>
>>-static struct device_attribute trip_point_attrs[] = {
>>-      __ATTR(trip_point_0_type, 0444, trip_point_type_show, NULL),
>>-      __ATTR(trip_point_0_temp, 0444, trip_point_temp_show, NULL),
>>-      __ATTR(trip_point_1_type, 0444, trip_point_type_show, NULL),
>>-      __ATTR(trip_point_1_temp, 0444, trip_point_temp_show, NULL),
>>-      __ATTR(trip_point_2_type, 0444, trip_point_type_show, NULL),
>>-      __ATTR(trip_point_2_temp, 0444, trip_point_temp_show, NULL),
>>-      __ATTR(trip_point_3_type, 0444, trip_point_type_show, NULL),
>>-      __ATTR(trip_point_3_temp, 0444, trip_point_temp_show, NULL),
>>-      __ATTR(trip_point_4_type, 0444, trip_point_type_show, NULL),
>>-      __ATTR(trip_point_4_temp, 0444, trip_point_temp_show, NULL),
>>-      __ATTR(trip_point_5_type, 0444, trip_point_type_show, NULL),
>>-      __ATTR(trip_point_5_temp, 0444, trip_point_temp_show, NULL),
>>-      __ATTR(trip_point_6_type, 0444, trip_point_type_show, NULL),
>>-      __ATTR(trip_point_6_temp, 0444, trip_point_temp_show, NULL),
>>-      __ATTR(trip_point_7_type, 0444, trip_point_type_show, NULL),
>>-      __ATTR(trip_point_7_temp, 0444, trip_point_temp_show, NULL),
>>-      __ATTR(trip_point_8_type, 0444, trip_point_type_show, NULL),
>>-      __ATTR(trip_point_8_temp, 0444, trip_point_temp_show, NULL),
>>-      __ATTR(trip_point_9_type, 0444, trip_point_type_show, NULL),
>>-      __ATTR(trip_point_9_temp, 0444, trip_point_temp_show, NULL),
>>-      __ATTR(trip_point_10_type, 0444, trip_point_type_show, NULL),
>>-      __ATTR(trip_point_10_temp, 0444, trip_point_temp_show, NULL),
>>-      __ATTR(trip_point_11_type, 0444, trip_point_type_show, NULL),
>>-      __ATTR(trip_point_11_temp, 0444, trip_point_temp_show, NULL),
>>-};
>>-
>>-#define TRIP_POINT_ATTR_ADD(_dev, _index, result)     \
>>-do {    \
>>-      result = device_create_file(_dev,       \
>>-                              &trip_point_attrs[_index * 2]); \
>>-      if (result)     \
>>-              break;  \
>>-      result = device_create_file(_dev,       \
>>-                      &trip_point_attrs[_index * 2 + 1]);     \
>>-} while (0)
>>-
>>-#define TRIP_POINT_ATTR_REMOVE(_dev, _index)  \
>>-do {  \
>>-      device_remove_file(_dev, &trip_point_attrs[_index * 2]);        \
>>-      device_remove_file(_dev, &trip_point_attrs[_index * 2 + 1]);    \
>>-} while (0)
>>-
>> /* sys I/F for cooling device */
>> #define to_cooling_device(_dev)       \
>>       container_of(_dev, struct thermal_cooling_device, device)
>>@@ -1112,9 +1091,54 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
>> EXPORT_SYMBOL(thermal_zone_device_update);
>>
>> /**
>>+ * create_trip_type_attr - creates a trip point type attribute
>>+ * @tz:               the thermal zone device
>>+ * @indx:     index into the trip_type_attrs array
>>+ */
>>+static int create_trip_type_attr(struct thermal_zone_device *tz, int indx)
>>+{
>>+      char attr_name[THERMAL_NAME_LENGTH];
>>+
>>+      snprintf(attr_name, THERMAL_NAME_LENGTH, "trip_point_%d_type", indx);
>>+
>>+      sysfs_attr_init(&tz->trip_type_attrs[indx].attr);
>>+      tz->trip_type_attrs[indx].attr.name = attr_name;
>>+      tz->trip_type_attrs[indx].attr.mode = S_IRUGO;
>>+      tz->trip_type_attrs[indx].show = trip_point_type_show;
>>+
>>+      return device_create_file(&tz->device, &tz->trip_type_attrs[indx]);
>>+}
>>+
>>+/**
>>+ * create_trip_temp_attr - creates a trip point temp attribute
>>+ * @tz:               the thermal zone device
>>+ * @indx:     index into the trip_type_attrs array
>>+ * @writeable:        A flag: If 1, 'this' trip point is writeable; otherwise not
>>+ */
>>+static int create_trip_temp_attr(struct thermal_zone_device *tz,
>>+                              int indx, int writeable)
>>+{
>>+      char attr_name[THERMAL_NAME_LENGTH];
>>+
>>+      snprintf(attr_name, THERMAL_NAME_LENGTH, "trip_point_%d_temp", indx);
>>+
>>+      sysfs_attr_init(&tz->trip_temp_attrs[indx].attr);
>>+      tz->trip_temp_attrs[indx].attr.name = attr_name;
>>+      tz->trip_temp_attrs[indx].attr.mode = S_IRUGO;
>>+      tz->trip_temp_attrs[indx].show = trip_point_temp_show;
>>+      if (writeable) {
>>+              tz->trip_temp_attrs[indx].attr.mode |= S_IWUSR;
>>+              tz->trip_temp_attrs[indx].store = trip_point_temp_store;
>>+      }
>>+
>>+      return device_create_file(&tz->device, &tz->trip_temp_attrs[indx]);
>>+}
>>+
>>+/**
>>  * thermal_zone_device_register - register a new thermal zone device
>>  * @type:     the thermal zone device type
>>  * @trips:    the number of trip points the thermal zone support
>>+ * @flag:     a bit string indicating the writeablility of trip points
>>  * @devdata:  private device data
>>  * @ops:      standard thermal zone device callbacks
>>  * @tc1:      thermal coefficient 1 for passive calculations
>>@@ -1130,7 +1154,7 @@ EXPORT_SYMBOL(thermal_zone_device_update);
>>  * section 11.1.5.1 of the ACPI specification 3.0.
>>  */
>> struct thermal_zone_device *thermal_zone_device_register(char *type,
>>-      int trips, void *devdata,
>>+      int trips, int flag, void *devdata,
>>       const struct thermal_zone_device_ops *ops,
>>       int tc1, int tc2, int passive_delay, int polling_delay)
>> {
>>@@ -1199,9 +1223,15 @@ struct thermal_zone_device *thermal_zone_device_register(char *type,
>>       }
>>
>>       for (count = 0; count < trips; count++) {
>>-              TRIP_POINT_ATTR_ADD(&tz->device, count, result);
>>+              result = create_trip_type_attr(tz, count);
>>               if (result)
>>                       goto unregister;
>>+
>>+              result = create_trip_temp_attr(tz, count,
>>+                                              (flag & (1 << count)));
>>+              if (result)
>>+                      goto unregister;
>>+
>>               tz->ops->get_trip_type(tz, count, &trip_type);
>>               if (trip_type == THERMAL_TRIP_PASSIVE)
>>                       passive = 1;
>>@@ -1279,9 +1309,10 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
>>       if (tz->ops->get_mode)
>>               device_remove_file(&tz->device, &dev_attr_mode);
>>
>>-      for (count = 0; count < tz->trips; count++)
>>-              TRIP_POINT_ATTR_REMOVE(&tz->device, count);
>>-
>>+      for (count = 0; count < tz->trips; count++) {
>>+              device_remove_file(&tz->device, &tz->trip_type_attrs[count]);
>>+              device_remove_file(&tz->device, &tz->trip_temp_attrs[count]);
>>+      }
>>       thermal_remove_hwmon_sysfs(tz);
>>       release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
>>       idr_destroy(&tz->idr);
>>diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>>index 47b4a27..4aa5a45 100644
>>--- a/include/linux/thermal.h
>>+++ b/include/linux/thermal.h
>>@@ -58,6 +58,8 @@ struct thermal_zone_device_ops {
>>               enum thermal_trip_type *);
>>       int (*get_trip_temp) (struct thermal_zone_device *, int,
>>                             unsigned long *);
>>+      int (*set_trip_temp) (struct thermal_zone_device *, int,
>>+                            unsigned long);
>>       int (*get_crit_temp) (struct thermal_zone_device *, unsigned long *);
>>       int (*notify) (struct thermal_zone_device *, int,
>>                      enum thermal_trip_type);
>>@@ -89,6 +91,8 @@ struct thermal_zone_device {
>>       int id;
>>       char type[THERMAL_NAME_LENGTH];
>>       struct device device;
>>+      struct device_attribute trip_temp_attrs[THERMAL_MAX_TRIPS];
>>+      struct device_attribute trip_type_attrs[THERMAL_MAX_TRIPS];
>>       void *devdata;
>>       int trips;
>>       int tc1;
>>@@ -137,9 +141,9 @@ enum {
>> };
>> #define THERMAL_GENL_CMD_MAX (__THERMAL_GENL_CMD_MAX - 1)
>>
>>-struct thermal_zone_device *thermal_zone_device_register(char *, int, void *,
>>-              const struct thermal_zone_device_ops *, int tc1, int tc2,
>>-              int passive_freq, int polling_freq);
>>+struct thermal_zone_device *thermal_zone_device_register(char *, int, int,
>>+              void *, const struct thermal_zone_device_ops *, int tc1,
>>+              int tc2, int passive_freq, int polling_freq);
>> void thermal_zone_device_unregister(struct thermal_zone_device *);
>>
>> int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
>>--
>>1.7.1
>>
>
> _______________________________________________
> linaro-dev mailing list
> linaro-dev@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ACPI-Thermal: Make Thermal trip points writeable
  2012-04-06 13:01   ` Rob Lee
@ 2012-04-06 13:18     ` Rob Lee
  0 siblings, 0 replies; 6+ messages in thread
From: Rob Lee @ 2012-04-06 13:18 UTC (permalink / raw)
  To: Amit Daniel Kachhap; +Cc: durgadoss.r, linux-acpi, rui.zhang, linaro-dev, lenb

Sorry, I just read Durgadoss last comment.  Please ignore mine.

On Fri, Apr 6, 2012 at 8:01 AM, Rob Lee <rob.lee@linaro.org> wrote:
> On Thu, Apr 5, 2012 at 12:52 AM, Amit Daniel Kachhap
> <amit.kachhap@linaro.org> wrote:
>> Hi Durgadoss,
>>
>> Instead of making all the trip-points of a thermal zone writeable we should
>> only allow non-critical trip points to be writeable.
>>
>> Thanks,
>> Amit Daniel
>>
>
> Agree, or even better, could the writeable attribute be made an
> optional for any trip point?
>
> Many SoCs now have built in thermal sensing and for cost reasons some
> device makers may want to rely on the kernel thermal handling
> capabilities as their primary thermal protection.  Allowing userspace
> to modify the trip points may increase their risk/exposure to lawsuits
> or other undesirable results in some cases.
>
> Best Regards,
> Rob
>
>>> -----Original Message-----
>>> From: Durgadoss R <durgadoss.r@intel.com>
>>> Sent: Friday, January 27, 2012 9:58 PM
>>> To: linux-acpi@vger.kernel.org
>>> Cc: lenb@kernel.org; rui.zhang@intel.com; durgadoss.r@intel.com
>>> Subject: [PATCH] ACPI-Thermal: Make Thermal trip points writeable
>>
>>>Some of the thermal drivers using the Generic Thermal Framework
>>>require (all/some) trip points to be writeable. This patch makes
>>>the trip point temperatures writeable on a per-trip point basis,
>>>and modifies the required function call in thermal.c. This patch
>>>also updates the Documentation to reflect the new change.
>>>
>>>Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
>>>---
>>>v1
>>>* patch1/2: Code for making trip points writeable
>>>* patch2/2: Change the callee in thermal.c
>>>v2
>>>* Make both the changes in a single patch
>>>* Update Documentation/thermal/sysfs-api.txt
>>>---
>>> Documentation/thermal/sysfs-api.txt |    4 +-
>>> drivers/acpi/thermal.c              |    4 +-
>>> drivers/thermal/thermal_sys.c       |  127 ++++++++++++++++++++++-------------
>>> include/linux/thermal.h             |   10 ++-
>>> 4 files changed, 91 insertions(+), 54 deletions(-)
>>>
>>>diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
>>>index b61e46f..cd1476c 100644
>>>--- a/Documentation/thermal/sysfs-api.txt
>>>+++ b/Documentation/thermal/sysfs-api.txt
>>>@@ -32,7 +32,8 @@ temperature) and throttle appropriate devices.
>>>
>>> 1.1 thermal zone device interface
>>> 1.1.1 struct thermal_zone_device *thermal_zone_device_register(char *name,
>>>-              int trips, void *devdata, struct thermal_zone_device_ops *ops)
>>>+              int trips, int flag, void *devdata,
>>>+              struct thermal_zone_device_ops *ops)
>>>
>>>     This interface function adds a new thermal zone device (sensor) to
>>>     /sys/class/thermal folder as thermal_zone[0-*]. It tries to bind all the
>>>@@ -40,6 +41,7 @@ temperature) and throttle appropriate devices.
>>>
>>>     name: the thermal zone name.
>>>     trips: the total number of trip points this thermal zone supports.
>>>+    flag: Bit string: If 'n'th bit is set, then trip point 'n' is writeable.
>>>     devdata: device private data
>>>     ops: thermal zone device call-backs.
>>>       .bind: bind the thermal zone device with a thermal cooling device.
>>>diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
>>>index 48fbc64..1c3133e 100644
>>>--- a/drivers/acpi/thermal.c
>>>+++ b/drivers/acpi/thermal.c
>>>@@ -845,7 +845,7 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
>>>
>>>       if (tz->trips.passive.flags.valid)
>>>               tz->thermal_zone =
>>>-                      thermal_zone_device_register("acpitz", trips, tz,
>>>+                      thermal_zone_device_register("acpitz", trips, 0, tz,
>>>                                                    &acpi_thermal_zone_ops,
>>>                                                    tz->trips.passive.tc1,
>>>                                                    tz->trips.passive.tc2,
>>>@@ -853,7 +853,7 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
>>>                                                    tz->polling_frequency*100);
>>>       else
>>>               tz->thermal_zone =
>>>-                      thermal_zone_device_register("acpitz", trips, tz,
>>>+                      thermal_zone_device_register("acpitz", trips, 0, tz,
>>>                                                    &acpi_thermal_zone_ops,
>>>                                                    0, 0, 0,
>>>                                                    tz->polling_frequency*100);
>>>diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
>>>index dd9a574..3a4341e 100644
>>>--- a/drivers/thermal/thermal_sys.c
>>>+++ b/drivers/thermal/thermal_sys.c
>>>@@ -220,6 +220,28 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr,
>>> }
>>>
>>> static ssize_t
>>>+trip_point_temp_store(struct device *dev, struct device_attribute *attr,
>>>+                   const char *buf, size_t count)
>>>+{
>>>+      struct thermal_zone_device *tz = to_thermal_zone(dev);
>>>+      int trip, ret;
>>>+      unsigned long temperature;
>>>+
>>>+      if (!tz->ops->set_trip_temp)
>>>+              return -EPERM;
>>>+
>>>+      if (!sscanf(attr->attr.name, "trip_point_%d_temp", &trip))
>>>+              return -EINVAL;
>>>+
>>>+      if (kstrtoul(buf, 10, &temperature))
>>>+              return -EINVAL;
>>>+
>>>+      ret = tz->ops->set_trip_temp(tz, trip, temperature);
>>>+
>>>+      return ret ? ret : count;
>>>+}
>>>+
>>>+static ssize_t
>>> passive_store(struct device *dev, struct device_attribute *attr,
>>>                   const char *buf, size_t count)
>>> {
>>>@@ -286,49 +308,6 @@ static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
>>> static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show, \
>>>                  passive_store);
>>>
>>>-static struct device_attribute trip_point_attrs[] = {
>>>-      __ATTR(trip_point_0_type, 0444, trip_point_type_show, NULL),
>>>-      __ATTR(trip_point_0_temp, 0444, trip_point_temp_show, NULL),
>>>-      __ATTR(trip_point_1_type, 0444, trip_point_type_show, NULL),
>>>-      __ATTR(trip_point_1_temp, 0444, trip_point_temp_show, NULL),
>>>-      __ATTR(trip_point_2_type, 0444, trip_point_type_show, NULL),
>>>-      __ATTR(trip_point_2_temp, 0444, trip_point_temp_show, NULL),
>>>-      __ATTR(trip_point_3_type, 0444, trip_point_type_show, NULL),
>>>-      __ATTR(trip_point_3_temp, 0444, trip_point_temp_show, NULL),
>>>-      __ATTR(trip_point_4_type, 0444, trip_point_type_show, NULL),
>>>-      __ATTR(trip_point_4_temp, 0444, trip_point_temp_show, NULL),
>>>-      __ATTR(trip_point_5_type, 0444, trip_point_type_show, NULL),
>>>-      __ATTR(trip_point_5_temp, 0444, trip_point_temp_show, NULL),
>>>-      __ATTR(trip_point_6_type, 0444, trip_point_type_show, NULL),
>>>-      __ATTR(trip_point_6_temp, 0444, trip_point_temp_show, NULL),
>>>-      __ATTR(trip_point_7_type, 0444, trip_point_type_show, NULL),
>>>-      __ATTR(trip_point_7_temp, 0444, trip_point_temp_show, NULL),
>>>-      __ATTR(trip_point_8_type, 0444, trip_point_type_show, NULL),
>>>-      __ATTR(trip_point_8_temp, 0444, trip_point_temp_show, NULL),
>>>-      __ATTR(trip_point_9_type, 0444, trip_point_type_show, NULL),
>>>-      __ATTR(trip_point_9_temp, 0444, trip_point_temp_show, NULL),
>>>-      __ATTR(trip_point_10_type, 0444, trip_point_type_show, NULL),
>>>-      __ATTR(trip_point_10_temp, 0444, trip_point_temp_show, NULL),
>>>-      __ATTR(trip_point_11_type, 0444, trip_point_type_show, NULL),
>>>-      __ATTR(trip_point_11_temp, 0444, trip_point_temp_show, NULL),
>>>-};
>>>-
>>>-#define TRIP_POINT_ATTR_ADD(_dev, _index, result)     \
>>>-do {    \
>>>-      result = device_create_file(_dev,       \
>>>-                              &trip_point_attrs[_index * 2]); \
>>>-      if (result)     \
>>>-              break;  \
>>>-      result = device_create_file(_dev,       \
>>>-                      &trip_point_attrs[_index * 2 + 1]);     \
>>>-} while (0)
>>>-
>>>-#define TRIP_POINT_ATTR_REMOVE(_dev, _index)  \
>>>-do {  \
>>>-      device_remove_file(_dev, &trip_point_attrs[_index * 2]);        \
>>>-      device_remove_file(_dev, &trip_point_attrs[_index * 2 + 1]);    \
>>>-} while (0)
>>>-
>>> /* sys I/F for cooling device */
>>> #define to_cooling_device(_dev)       \
>>>       container_of(_dev, struct thermal_cooling_device, device)
>>>@@ -1112,9 +1091,54 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
>>> EXPORT_SYMBOL(thermal_zone_device_update);
>>>
>>> /**
>>>+ * create_trip_type_attr - creates a trip point type attribute
>>>+ * @tz:               the thermal zone device
>>>+ * @indx:     index into the trip_type_attrs array
>>>+ */
>>>+static int create_trip_type_attr(struct thermal_zone_device *tz, int indx)
>>>+{
>>>+      char attr_name[THERMAL_NAME_LENGTH];
>>>+
>>>+      snprintf(attr_name, THERMAL_NAME_LENGTH, "trip_point_%d_type", indx);
>>>+
>>>+      sysfs_attr_init(&tz->trip_type_attrs[indx].attr);
>>>+      tz->trip_type_attrs[indx].attr.name = attr_name;
>>>+      tz->trip_type_attrs[indx].attr.mode = S_IRUGO;
>>>+      tz->trip_type_attrs[indx].show = trip_point_type_show;
>>>+
>>>+      return device_create_file(&tz->device, &tz->trip_type_attrs[indx]);
>>>+}
>>>+
>>>+/**
>>>+ * create_trip_temp_attr - creates a trip point temp attribute
>>>+ * @tz:               the thermal zone device
>>>+ * @indx:     index into the trip_type_attrs array
>>>+ * @writeable:        A flag: If 1, 'this' trip point is writeable; otherwise not
>>>+ */
>>>+static int create_trip_temp_attr(struct thermal_zone_device *tz,
>>>+                              int indx, int writeable)
>>>+{
>>>+      char attr_name[THERMAL_NAME_LENGTH];
>>>+
>>>+      snprintf(attr_name, THERMAL_NAME_LENGTH, "trip_point_%d_temp", indx);
>>>+
>>>+      sysfs_attr_init(&tz->trip_temp_attrs[indx].attr);
>>>+      tz->trip_temp_attrs[indx].attr.name = attr_name;
>>>+      tz->trip_temp_attrs[indx].attr.mode = S_IRUGO;
>>>+      tz->trip_temp_attrs[indx].show = trip_point_temp_show;
>>>+      if (writeable) {
>>>+              tz->trip_temp_attrs[indx].attr.mode |= S_IWUSR;
>>>+              tz->trip_temp_attrs[indx].store = trip_point_temp_store;
>>>+      }
>>>+
>>>+      return device_create_file(&tz->device, &tz->trip_temp_attrs[indx]);
>>>+}
>>>+
>>>+/**
>>>  * thermal_zone_device_register - register a new thermal zone device
>>>  * @type:     the thermal zone device type
>>>  * @trips:    the number of trip points the thermal zone support
>>>+ * @flag:     a bit string indicating the writeablility of trip points
>>>  * @devdata:  private device data
>>>  * @ops:      standard thermal zone device callbacks
>>>  * @tc1:      thermal coefficient 1 for passive calculations
>>>@@ -1130,7 +1154,7 @@ EXPORT_SYMBOL(thermal_zone_device_update);
>>>  * section 11.1.5.1 of the ACPI specification 3.0.
>>>  */
>>> struct thermal_zone_device *thermal_zone_device_register(char *type,
>>>-      int trips, void *devdata,
>>>+      int trips, int flag, void *devdata,
>>>       const struct thermal_zone_device_ops *ops,
>>>       int tc1, int tc2, int passive_delay, int polling_delay)
>>> {
>>>@@ -1199,9 +1223,15 @@ struct thermal_zone_device *thermal_zone_device_register(char *type,
>>>       }
>>>
>>>       for (count = 0; count < trips; count++) {
>>>-              TRIP_POINT_ATTR_ADD(&tz->device, count, result);
>>>+              result = create_trip_type_attr(tz, count);
>>>               if (result)
>>>                       goto unregister;
>>>+
>>>+              result = create_trip_temp_attr(tz, count,
>>>+                                              (flag & (1 << count)));
>>>+              if (result)
>>>+                      goto unregister;
>>>+
>>>               tz->ops->get_trip_type(tz, count, &trip_type);
>>>               if (trip_type == THERMAL_TRIP_PASSIVE)
>>>                       passive = 1;
>>>@@ -1279,9 +1309,10 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
>>>       if (tz->ops->get_mode)
>>>               device_remove_file(&tz->device, &dev_attr_mode);
>>>
>>>-      for (count = 0; count < tz->trips; count++)
>>>-              TRIP_POINT_ATTR_REMOVE(&tz->device, count);
>>>-
>>>+      for (count = 0; count < tz->trips; count++) {
>>>+              device_remove_file(&tz->device, &tz->trip_type_attrs[count]);
>>>+              device_remove_file(&tz->device, &tz->trip_temp_attrs[count]);
>>>+      }
>>>       thermal_remove_hwmon_sysfs(tz);
>>>       release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id);
>>>       idr_destroy(&tz->idr);
>>>diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>>>index 47b4a27..4aa5a45 100644
>>>--- a/include/linux/thermal.h
>>>+++ b/include/linux/thermal.h
>>>@@ -58,6 +58,8 @@ struct thermal_zone_device_ops {
>>>               enum thermal_trip_type *);
>>>       int (*get_trip_temp) (struct thermal_zone_device *, int,
>>>                             unsigned long *);
>>>+      int (*set_trip_temp) (struct thermal_zone_device *, int,
>>>+                            unsigned long);
>>>       int (*get_crit_temp) (struct thermal_zone_device *, unsigned long *);
>>>       int (*notify) (struct thermal_zone_device *, int,
>>>                      enum thermal_trip_type);
>>>@@ -89,6 +91,8 @@ struct thermal_zone_device {
>>>       int id;
>>>       char type[THERMAL_NAME_LENGTH];
>>>       struct device device;
>>>+      struct device_attribute trip_temp_attrs[THERMAL_MAX_TRIPS];
>>>+      struct device_attribute trip_type_attrs[THERMAL_MAX_TRIPS];
>>>       void *devdata;
>>>       int trips;
>>>       int tc1;
>>>@@ -137,9 +141,9 @@ enum {
>>> };
>>> #define THERMAL_GENL_CMD_MAX (__THERMAL_GENL_CMD_MAX - 1)
>>>
>>>-struct thermal_zone_device *thermal_zone_device_register(char *, int, void *,
>>>-              const struct thermal_zone_device_ops *, int tc1, int tc2,
>>>-              int passive_freq, int polling_freq);
>>>+struct thermal_zone_device *thermal_zone_device_register(char *, int, int,
>>>+              void *, const struct thermal_zone_device_ops *, int tc1,
>>>+              int tc2, int passive_freq, int polling_freq);
>>> void thermal_zone_device_unregister(struct thermal_zone_device *);
>>>
>>> int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
>>>--
>>>1.7.1
>>>
>>
>> _______________________________________________
>> linaro-dev mailing list
>> linaro-dev@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/linaro-dev
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-04-06 13:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-27 16:28 [PATCH] ACPI-Thermal: Make Thermal trip points writeable Durgadoss R
2012-03-30 10:05 ` Len Brown
2012-04-05  5:52 ` Amit Daniel Kachhap
2012-04-05  7:43   ` R, Durgadoss
2012-04-06 13:01   ` Rob Lee
2012-04-06 13:18     ` Rob Lee

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).