diff mbox

[02/16] Thermal: Add Hysteresis attributes

Message ID 1342679480-5336-3-git-send-email-rui.zhang@intel.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Zhang Rui July 19, 2012, 6:31 a.m. UTC
From: Durgadoss R <dugardoss.r@intel.com>

The Linux Thermal Framework does not support hysteresis
attributes. Most thermal sensors, today, have a
hysteresis value associated with trip points.

This patch adds hysteresis attributes on a per-trip-point
basis, to the Thermal Framework. These attributes are
optionally writable.

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 Documentation/thermal/sysfs-api.txt |    6 +++
 drivers/thermal/thermal_sys.c       |   89 ++++++++++++++++++++++++++++++++---
 include/linux/thermal.h             |    5 ++
 3 files changed, 94 insertions(+), 6 deletions(-)

Comments

durgadoss.r@intel.com July 19, 2012, 10:40 a.m. UTC | #1
Hi Rui,

> -----Original Message-----
> From: Zhang, Rui
> Sent: Thursday, July 19, 2012 12:01 PM
> To: linux-acpi@vger.kernel.org; linux-pm@vger.kernel.org
> Cc: Rafael J. Wysocki; Matthew Garrett; Len Brown; R, Durgadoss; Eduardo
> Valentin; Amit Kachhap; Wei Ni; Zhang, Rui
> Subject: [PATCH 02/16] Thermal: Add Hysteresis attributes
> 
> From: Durgadoss R <dugardoss.r@intel.com>
> 
> The Linux Thermal Framework does not support hysteresis
> attributes. Most thermal sensors, today, have a
> hysteresis value associated with trip points.
> 
> This patch adds hysteresis attributes on a per-trip-point
> basis, to the Thermal Framework. These attributes are
> optionally writable.
> 
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  Documentation/thermal/sysfs-api.txt |    6 +++
>  drivers/thermal/thermal_sys.c       |   89
> ++++++++++++++++++++++++++++++++---
>  include/linux/thermal.h             |    5 ++
>  3 files changed, 94 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/thermal/sysfs-api.txt
> b/Documentation/thermal/sysfs-api.txt
> index 0c7c423..3c8c2f8 100644
> --- a/Documentation/thermal/sysfs-api.txt
> +++ b/Documentation/thermal/sysfs-api.txt
> @@ -121,6 +121,7 @@ if hwmon is compiled in or built as a module.
>      |---mode:			Working mode of the thermal zone
>      |---trip_point_[0-*]_temp:	Trip point temperature
>      |---trip_point_[0-*]_type:	Trip point type
> +    |---trip_point_[0-*]_hyst:	Hysteresis value for this trip point
> 
>  Thermal cooling device sys I/F, created once it's registered:
>  /sys/class/thermal/cooling_device[0-*]:
> @@ -190,6 +191,11 @@ trip_point_[0-*]_type
>  	thermal zone.
>  	RO, Optional
> 
[cut.]

> +		/* create Optional trip hyst attribute */
> +		if (!tz->ops->get_trip_hyst)
> +			continue;
> +		snprintf(tz->trip_hyst_attrs[indx].name,
> THERMAL_NAME_LENGTH,
> +			 "trip_point_%d_hyst", indx);
> +
> +		sysfs_attr_init(&tz->trip_hyst_attrs[indx].attr.attr);
> +		tz->trip_hyst_attrs[indx].attr.attr.name =
> +					tz->trip_hyst_attrs[indx].name;
> +		tz->trip_hyst_attrs[indx].attr.attr.mode = S_IRUGO;
> +		tz->trip_hyst_attrs[indx].attr.show = trip_point_hyst_show;
> +		if (tz->ops->set_trip_hyst) {
> +			tz->trip_hyst_attrs[indx].attr.attr.mode |= S_IWUSR;
> +			tz->trip_hyst_attrs[indx].attr.store =
> +					trip_point_hyst_store;
> +		}
> +
> +		device_create_file(&tz->device,
> +				   &tz->trip_hyst_attrs[indx].attr);
>  	}
>  	return 0;
>  }
> @@ -1151,9 +1225,13 @@ static void remove_trip_attrs(struct
> thermal_zone_device *tz)
>  				   &tz->trip_type_attrs[indx].attr);
>  		device_remove_file(&tz->device,
>  				   &tz->trip_temp_attrs[indx].attr);
> +		if (tz->ops->get_trip_hyst)
> +			device_remove_file(&tz->device,
> +				  &tz->trip_hyst_attrs[indx].attr);
>  	}
>  	kfree(tz->trip_type_attrs);
>  	kfree(tz->trip_temp_attrs);

I believe we should have a check here for 'if (tz->ops->get_trip_hyst)' and then
only free this, if required.

> +	kfree(tz->trip_hyst_attrs);
>  }
> 

Thanks,
Durga
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael Wysocki July 19, 2012, 8 p.m. UTC | #2
On Thursday, July 19, 2012, R, Durgadoss wrote:
> Hi Rui,
> 
> > -----Original Message-----
> > From: Zhang, Rui
> > Sent: Thursday, July 19, 2012 12:01 PM
> > To: linux-acpi@vger.kernel.org; linux-pm@vger.kernel.org
> > Cc: Rafael J. Wysocki; Matthew Garrett; Len Brown; R, Durgadoss; Eduardo
> > Valentin; Amit Kachhap; Wei Ni; Zhang, Rui
> > Subject: [PATCH 02/16] Thermal: Add Hysteresis attributes
> > 
> > From: Durgadoss R <dugardoss.r@intel.com>
> > 
> > The Linux Thermal Framework does not support hysteresis
> > attributes. Most thermal sensors, today, have a
> > hysteresis value associated with trip points.
> > 
> > This patch adds hysteresis attributes on a per-trip-point
> > basis, to the Thermal Framework. These attributes are
> > optionally writable.
> > 
> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > ---
> >  Documentation/thermal/sysfs-api.txt |    6 +++
> >  drivers/thermal/thermal_sys.c       |   89
> > ++++++++++++++++++++++++++++++++---
> >  include/linux/thermal.h             |    5 ++
> >  3 files changed, 94 insertions(+), 6 deletions(-)
> > 
> > diff --git a/Documentation/thermal/sysfs-api.txt
> > b/Documentation/thermal/sysfs-api.txt
> > index 0c7c423..3c8c2f8 100644
> > --- a/Documentation/thermal/sysfs-api.txt
> > +++ b/Documentation/thermal/sysfs-api.txt
> > @@ -121,6 +121,7 @@ if hwmon is compiled in or built as a module.
> >      |---mode:			Working mode of the thermal zone
> >      |---trip_point_[0-*]_temp:	Trip point temperature
> >      |---trip_point_[0-*]_type:	Trip point type
> > +    |---trip_point_[0-*]_hyst:	Hysteresis value for this trip point
> > 
> >  Thermal cooling device sys I/F, created once it's registered:
> >  /sys/class/thermal/cooling_device[0-*]:
> > @@ -190,6 +191,11 @@ trip_point_[0-*]_type
> >  	thermal zone.
> >  	RO, Optional
> > 
> [cut.]
> 
> > +		/* create Optional trip hyst attribute */
> > +		if (!tz->ops->get_trip_hyst)
> > +			continue;
> > +		snprintf(tz->trip_hyst_attrs[indx].name,
> > THERMAL_NAME_LENGTH,
> > +			 "trip_point_%d_hyst", indx);
> > +
> > +		sysfs_attr_init(&tz->trip_hyst_attrs[indx].attr.attr);
> > +		tz->trip_hyst_attrs[indx].attr.attr.name =
> > +					tz->trip_hyst_attrs[indx].name;
> > +		tz->trip_hyst_attrs[indx].attr.attr.mode = S_IRUGO;
> > +		tz->trip_hyst_attrs[indx].attr.show = trip_point_hyst_show;
> > +		if (tz->ops->set_trip_hyst) {
> > +			tz->trip_hyst_attrs[indx].attr.attr.mode |= S_IWUSR;
> > +			tz->trip_hyst_attrs[indx].attr.store =
> > +					trip_point_hyst_store;
> > +		}
> > +
> > +		device_create_file(&tz->device,
> > +				   &tz->trip_hyst_attrs[indx].attr);
> >  	}
> >  	return 0;
> >  }
> > @@ -1151,9 +1225,13 @@ static void remove_trip_attrs(struct
> > thermal_zone_device *tz)
> >  				   &tz->trip_type_attrs[indx].attr);
> >  		device_remove_file(&tz->device,
> >  				   &tz->trip_temp_attrs[indx].attr);
> > +		if (tz->ops->get_trip_hyst)
> > +			device_remove_file(&tz->device,
> > +				  &tz->trip_hyst_attrs[indx].attr);
> >  	}
> >  	kfree(tz->trip_type_attrs);
> >  	kfree(tz->trip_temp_attrs);
> 
> I believe we should have a check here for 'if (tz->ops->get_trip_hyst)' and then
> only free this, if required.
> 
> > +	kfree(tz->trip_hyst_attrs);
> >  }

No, we don't have to, kfree() checks for NULL pointers.

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

Patch

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index 0c7c423..3c8c2f8 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -121,6 +121,7 @@  if hwmon is compiled in or built as a module.
     |---mode:			Working mode of the thermal zone
     |---trip_point_[0-*]_temp:	Trip point temperature
     |---trip_point_[0-*]_type:	Trip point type
+    |---trip_point_[0-*]_hyst:	Hysteresis value for this trip point
 
 Thermal cooling device sys I/F, created once it's registered:
 /sys/class/thermal/cooling_device[0-*]:
@@ -190,6 +191,11 @@  trip_point_[0-*]_type
 	thermal zone.
 	RO, Optional
 
+trip_point_[0-*]_hyst
+	The hysteresis value for a trip point, represented as an integer
+	Unit: Celsius
+	RW, Optional
+
 cdev[0-*]
 	Sysfs link to the thermal cooling device node where the sys I/F
 	for cooling device throttling control represents.
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index a590d57..95e0d63 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -240,6 +240,52 @@  trip_point_temp_show(struct device *dev, struct device_attribute *attr,
 }
 
 static ssize_t
+trip_point_hyst_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_hyst)
+		return -EPERM;
+
+	if (!sscanf(attr->attr.name, "trip_point_%d_hyst", &trip))
+		return -EINVAL;
+
+	if (kstrtoul(buf, 10, &temperature))
+		return -EINVAL;
+
+	/*
+	 * We are not doing any check on the 'temperature' value
+	 * here. The driver implementing 'set_trip_hyst' has to
+	 * take care of this.
+	 */
+	ret = tz->ops->set_trip_hyst(tz, trip, temperature);
+
+	return ret ? ret : count;
+}
+
+static ssize_t
+trip_point_hyst_show(struct device *dev, struct device_attribute *attr,
+			char *buf)
+{
+	struct thermal_zone_device *tz = to_thermal_zone(dev);
+	int trip, ret;
+	unsigned long temperature;
+
+	if (!tz->ops->get_trip_hyst)
+		return -EPERM;
+
+	if (!sscanf(attr->attr.name, "trip_point_%d_hyst", &trip))
+		return -EINVAL;
+
+	ret = tz->ops->get_trip_hyst(tz, trip, &temperature);
+
+	return ret ? ret : sprintf(buf, "%ld\n", temperature);
+}
+
+static ssize_t
 passive_store(struct device *dev, struct device_attribute *attr,
 		    const char *buf, size_t count)
 {
@@ -1092,21 +1138,29 @@  static int create_trip_attrs(struct thermal_zone_device *tz, int flag)
 {
 	int indx;
 	int writeable;
+	int size = sizeof(struct thermal_attr) * tz->trips;
 
-	tz->trip_type_attrs =
-		kzalloc(sizeof(struct thermal_attr) * tz->trips, GFP_KERNEL);
+	tz->trip_type_attrs = kzalloc(size, GFP_KERNEL);
 	if (!tz->trip_type_attrs)
 		return -ENOMEM;
 
-	tz->trip_temp_attrs =
-		kzalloc(sizeof(struct thermal_attr) * tz->trips, GFP_KERNEL);
+	tz->trip_temp_attrs = kzalloc(size, GFP_KERNEL);
 	if (!tz->trip_temp_attrs) {
 		kfree(tz->trip_type_attrs);
 		return -ENOMEM;
 	}
 
-	for (indx = 0; indx < tz->trips; indx++) {
+	if (tz->ops->get_trip_hyst) {
+		tz->trip_hyst_attrs = kzalloc(size, GFP_KERNEL);
+		if (!tz->trip_hyst_attrs) {
+			kfree(tz->trip_type_attrs);
+			kfree(tz->trip_temp_attrs);
+			return -ENOMEM;
+		}
+	}
+
 
+	for (indx = 0; indx < tz->trips; indx++) {
 		/* create trip type attribute */
 		snprintf(tz->trip_type_attrs[indx].name, THERMAL_NAME_LENGTH,
 			 "trip_point_%d_type", indx);
@@ -1138,6 +1192,26 @@  static int create_trip_attrs(struct thermal_zone_device *tz, int flag)
 
 		device_create_file(&tz->device,
 				   &tz->trip_temp_attrs[indx].attr);
+
+		/* create Optional trip hyst attribute */
+		if (!tz->ops->get_trip_hyst)
+			continue;
+		snprintf(tz->trip_hyst_attrs[indx].name, THERMAL_NAME_LENGTH,
+			 "trip_point_%d_hyst", indx);
+
+		sysfs_attr_init(&tz->trip_hyst_attrs[indx].attr.attr);
+		tz->trip_hyst_attrs[indx].attr.attr.name =
+					tz->trip_hyst_attrs[indx].name;
+		tz->trip_hyst_attrs[indx].attr.attr.mode = S_IRUGO;
+		tz->trip_hyst_attrs[indx].attr.show = trip_point_hyst_show;
+		if (tz->ops->set_trip_hyst) {
+			tz->trip_hyst_attrs[indx].attr.attr.mode |= S_IWUSR;
+			tz->trip_hyst_attrs[indx].attr.store =
+					trip_point_hyst_store;
+		}
+
+		device_create_file(&tz->device,
+				   &tz->trip_hyst_attrs[indx].attr);
 	}
 	return 0;
 }
@@ -1151,9 +1225,13 @@  static void remove_trip_attrs(struct thermal_zone_device *tz)
 				   &tz->trip_type_attrs[indx].attr);
 		device_remove_file(&tz->device,
 				   &tz->trip_temp_attrs[indx].attr);
+		if (tz->ops->get_trip_hyst)
+			device_remove_file(&tz->device,
+				  &tz->trip_hyst_attrs[indx].attr);
 	}
 	kfree(tz->trip_type_attrs);
 	kfree(tz->trip_temp_attrs);
+	kfree(tz->trip_hyst_attrs);
 }
 
 /**
@@ -1300,7 +1378,6 @@  void thermal_zone_device_unregister(struct thermal_zone_device *tz)
 {
 	struct thermal_cooling_device *cdev;
 	struct thermal_zone_device *pos = NULL;
-	int count;
 
 	if (!tz)
 		return;
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 6eaf914..cfc8d90 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -60,6 +60,10 @@  struct thermal_zone_device_ops {
 			      unsigned long *);
 	int (*set_trip_temp) (struct thermal_zone_device *, int,
 			      unsigned long);
+	int (*get_trip_hyst) (struct thermal_zone_device *, int,
+			      unsigned long *);
+	int (*set_trip_hyst) (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);
@@ -98,6 +102,7 @@  struct thermal_zone_device {
 	struct device device;
 	struct thermal_attr *trip_temp_attrs;
 	struct thermal_attr *trip_type_attrs;
+	struct thermal_attr *trip_hyst_attrs;
 	void *devdata;
 	int trips;
 	int tc1;