diff mbox

[01/16] Thermal: Make Thermal trip points writeable

Message ID 1342679480-5336-2-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>

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>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 Documentation/thermal/sysfs-api.txt      |    4 +-
 drivers/acpi/thermal.c                   |    4 +-
 drivers/platform/x86/acerhdf.c           |    2 +-
 drivers/platform/x86/intel_mid_thermal.c |    2 +-
 drivers/thermal/spear_thermal.c          |    2 +-
 drivers/thermal/thermal_sys.c            |  147 +++++++++++++++++++++---------
 include/linux/thermal.h                  |   15 ++-
 7 files changed, 125 insertions(+), 51 deletions(-)

Comments

durgadoss.r@intel.com July 19, 2012, 10:35 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 01/16] Thermal: Make Thermal trip points writeable
> 
> From: Durgadoss R <dugardoss.r@intel.com>

[cut.]

> 
>  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
>  trip_point_temp_show(struct device *dev, struct device_attribute *attr,
>  		     char *buf)
>  {
> @@ -283,33 +305,6 @@ static DEVICE_ATTR(temp, 0444, temp_show, NULL);
>  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),
> -};
> -
>  /* sys I/F for cooling device */
>  #define to_cooling_device(_dev)	\
>  	container_of(_dev, struct thermal_cooling_device, device)
> @@ -1089,9 +1084,83 @@ void thermal_zone_device_update(struct
> thermal_zone_device *tz)
>  EXPORT_SYMBOL(thermal_zone_device_update);
> 
>  /**
> + * create_trip_attrs - create attributes for trip points
> + * @tz:		the thermal zone device
> + * @flag:	Writeable trip point bitmap.
> + */
> +static int create_trip_attrs(struct thermal_zone_device *tz, int flag)
> +{
> +	int indx;
> +	int writeable;
> +
> +	tz->trip_type_attrs =
> +		kzalloc(sizeof(struct thermal_attr) * tz->trips, GFP_KERNEL);
> +	if (!tz->trip_type_attrs)
> +		return -ENOMEM;
> +
> +	tz->trip_temp_attrs =
> +		kzalloc(sizeof(struct thermal_attr) * tz->trips, GFP_KERNEL);
> +	if (!tz->trip_temp_attrs) {
> +		kfree(tz->trip_type_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);
> +
> +		sysfs_attr_init(&tz->trip_type_attrs[count].attr.attr);

Shouldn't the 'count' be 'indx' ?

> +		tz->trip_type_attrs[indx].attr.attr.name =

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, 7:38 p.m. UTC | #2
On Thursday, July 19, 2012, Zhang Rui wrote:
> From: Durgadoss R <dugardoss.r@intel.com>
> 
> 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>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  Documentation/thermal/sysfs-api.txt      |    4 +-
>  drivers/acpi/thermal.c                   |    4 +-
>  drivers/platform/x86/acerhdf.c           |    2 +-
>  drivers/platform/x86/intel_mid_thermal.c |    2 +-
>  drivers/thermal/spear_thermal.c          |    2 +-
>  drivers/thermal/thermal_sys.c            |  147 +++++++++++++++++++++---------
>  include/linux/thermal.h                  |   15 ++-
>  7 files changed, 125 insertions(+), 51 deletions(-)
> 
> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> index 1733ab9..0c7c423 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.

I wonder.  What is the maximum number of trip points currently passed to this
function in one call?

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
Rafael Wysocki July 19, 2012, 8:27 p.m. UTC | #3
On Thursday, July 19, 2012, Zhang Rui wrote:
> From: Durgadoss R <dugardoss.r@intel.com>
> 
> 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>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  Documentation/thermal/sysfs-api.txt      |    4 +-
>  drivers/acpi/thermal.c                   |    4 +-
>  drivers/platform/x86/acerhdf.c           |    2 +-
>  drivers/platform/x86/intel_mid_thermal.c |    2 +-
>  drivers/thermal/spear_thermal.c          |    2 +-
>  drivers/thermal/thermal_sys.c            |  147 +++++++++++++++++++++---------
>  include/linux/thermal.h                  |   15 ++-
>  7 files changed, 125 insertions(+), 51 deletions(-)
> 
> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> index 1733ab9..0c7c423 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,

The 'flags' argument should rather be called 'mask', or even 'writable_mask'.

> +		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 7dbebea..2107d1b 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/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
> index 2fd9d36..39abb15 100644
> --- a/drivers/platform/x86/acerhdf.c
> +++ b/drivers/platform/x86/acerhdf.c
> @@ -660,7 +660,7 @@ static int acerhdf_register_thermal(void)
>  	if (IS_ERR(cl_dev))
>  		return -EINVAL;
>  
> -	thz_dev = thermal_zone_device_register("acerhdf", 1, NULL,
> +	thz_dev = thermal_zone_device_register("acerhdf", 1, 0, NULL,
>  					      &acerhdf_dev_ops, 0, 0, 0,
>  					      (kernelmode) ? interval*1000 : 0);
>  	if (IS_ERR(thz_dev))
> diff --git a/drivers/platform/x86/intel_mid_thermal.c b/drivers/platform/x86/intel_mid_thermal.c
> index 5ae9cd9..2b2c212 100644
> --- a/drivers/platform/x86/intel_mid_thermal.c
> +++ b/drivers/platform/x86/intel_mid_thermal.c
> @@ -499,7 +499,7 @@ static int mid_thermal_probe(struct platform_device *pdev)
>  			goto err;
>  		}
>  		pinfo->tzd[i] = thermal_zone_device_register(name[i],
> -				0, td_info, &tzd_ops, 0, 0, 0, 0);
> +				0, 0, td_info, &tzd_ops, 0, 0, 0, 0);
>  		if (IS_ERR(pinfo->tzd[i])) {
>  			kfree(td_info);
>  			ret = PTR_ERR(pinfo->tzd[i]);
> diff --git a/drivers/thermal/spear_thermal.c b/drivers/thermal/spear_thermal.c
> index c2e32df..69a55d4 100644
> --- a/drivers/thermal/spear_thermal.c
> +++ b/drivers/thermal/spear_thermal.c
> @@ -147,7 +147,7 @@ static int spear_thermal_probe(struct platform_device *pdev)
>  	stdev->flags = pdata->thermal_flags;
>  	writel_relaxed(stdev->flags, stdev->thermal_base);
>  
> -	spear_thermal = thermal_zone_device_register("spear_thermal", 0,
> +	spear_thermal = thermal_zone_device_register("spear_thermal", 0, 0,
>  				stdev, &ops, 0, 0, 0, 0);
>  	if (IS_ERR(spear_thermal)) {
>  		dev_err(&pdev->dev, "thermal zone device is NULL\n");
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index 022bacb..a590d57 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -196,6 +196,28 @@ trip_point_type_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
>  trip_point_temp_show(struct device *dev, struct device_attribute *attr,
>  		     char *buf)
>  {
> @@ -283,33 +305,6 @@ static DEVICE_ATTR(temp, 0444, temp_show, NULL);
>  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),
> -};
> -
>  /* sys I/F for cooling device */
>  #define to_cooling_device(_dev)	\
>  	container_of(_dev, struct thermal_cooling_device, device)
> @@ -1089,9 +1084,83 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
>  EXPORT_SYMBOL(thermal_zone_device_update);
>  
>  /**
> + * create_trip_attrs - create attributes for trip points
> + * @tz:		the thermal zone device
> + * @flag:	Writeable trip point bitmap.
> + */
> +static int create_trip_attrs(struct thermal_zone_device *tz, int flag)
> +{
> +	int indx;
> +	int writeable;
> +
> +	tz->trip_type_attrs =
> +		kzalloc(sizeof(struct thermal_attr) * tz->trips, GFP_KERNEL);
> +	if (!tz->trip_type_attrs)
> +		return -ENOMEM;
> +
> +	tz->trip_temp_attrs =
> +		kzalloc(sizeof(struct thermal_attr) * tz->trips, GFP_KERNEL);
> +	if (!tz->trip_temp_attrs) {
> +		kfree(tz->trip_type_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);

If this really truncates the name, we'll run into problems in _show() and _store().
Is there any solution to that?

> +
> +		sysfs_attr_init(&tz->trip_type_attrs[count].attr.attr);
> +		tz->trip_type_attrs[indx].attr.attr.name =
> +						tz->trip_type_attrs[indx].name;
> +		tz->trip_type_attrs[indx].attr.attr.mode = S_IRUGO;
> +		tz->trip_type_attrs[indx].attr.show = trip_point_type_show;
> +
> +		device_create_file(&tz->device,
> +				   &tz->trip_type_attrs[indx].attr);
> +
> +		/* create trip temp attribute */
> +		writeable = flag & (1 << indx);
> +		snprintf(tz->trip_temp_attrs[indx].name, THERMAL_NAME_LENGTH,
> +			 "trip_point_%d_temp", indx);
> +
> +		sysfs_attr_init(&tz->trip_temp_attrs[indx].attr.attr);
> +		tz->trip_temp_attrs[indx].attr.attr.name =
> +						tz->trip_temp_attrs[indx].name;
> +		tz->trip_temp_attrs[indx].attr.attr.mode = S_IRUGO;
> +		tz->trip_temp_attrs[indx].attr.show = trip_point_temp_show;
> +		if (writeable) {

Why don't you put the "flag & (1 << indx)" here directly instead of 'writable'?

> +			tz->trip_temp_attrs[indx].attr.attr.mode |= S_IWUSR;
> +			tz->trip_temp_attrs[indx].attr.store =
> +							trip_point_temp_store;
> +		}
> +
> +		device_create_file(&tz->device,
> +				   &tz->trip_temp_attrs[indx].attr);
> +	}
> +	return 0;
> +}
> +
> +static void remove_trip_attrs(struct thermal_zone_device *tz)
> +{
> +	int indx;
> +
> +	for (indx = 0; indx < tz->trips; indx++) {
> +		device_remove_file(&tz->device,
> +				   &tz->trip_type_attrs[indx].attr);
> +		device_remove_file(&tz->device,
> +				   &tz->trip_temp_attrs[indx].attr);
> +	}
> +	kfree(tz->trip_type_attrs);
> +	kfree(tz->trip_temp_attrs);
> +}
> +
> +/**
>   * 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
> @@ -1107,7 +1176,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)
>  {
> @@ -1124,6 +1193,9 @@ struct thermal_zone_device *thermal_zone_device_register(char *type,
>  	if (trips > THERMAL_MAX_TRIPS || trips < 0)
>  		return ERR_PTR(-EINVAL);
>  
> +	if (flag >> trips)
> +		return ERR_PTR(-EINVAL);

I'm not sure if this check is necessary.  Does it actually matter is someone
passes ones in the unused bit positions?

> +
>  	if (!ops || !ops->get_temp)
>  		return ERR_PTR(-EINVAL);
>  
> @@ -1175,15 +1247,11 @@ struct thermal_zone_device *thermal_zone_device_register(char *type,
>  			goto unregister;
>  	}
>  
> +	result = create_trip_attrs(tz, flag);
> +	if (result)
> +		goto unregister;
> +
>  	for (count = 0; count < trips; count++) {
> -		result = device_create_file(&tz->device,
> -					    &trip_point_attrs[count * 2]);
> -		if (result)
> -			break;
> -		result = device_create_file(&tz->device,
> -					    &trip_point_attrs[count * 2 + 1]);
> -		if (result)
> -			goto unregister;
>  		tz->ops->get_trip_type(tz, count, &trip_type);
>  		if (trip_type == THERMAL_TRIP_PASSIVE)
>  			passive = 1;
> @@ -1259,13 +1327,8 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
>  	device_remove_file(&tz->device, &dev_attr_temp);
>  	if (tz->ops->get_mode)
>  		device_remove_file(&tz->device, &dev_attr_mode);
> +	remove_trip_attrs(tz);
>  
> -	for (count = 0; count < tz->trips; count++) {
> -		device_remove_file(&tz->device,
> -				   &trip_point_attrs[count * 2]);
> -		device_remove_file(&tz->device,
> -				   &trip_point_attrs[count * 2 + 1]);
> -	}
>  	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 796f1ff..6eaf914 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);
> @@ -85,10 +87,17 @@ struct thermal_cooling_device {
>  				((long)t-2732+5)/10 : ((long)t-2732-5)/10)
>  #define CELSIUS_TO_KELVIN(t)	((t)*10+2732)
>  
> +struct thermal_attr {
> +	struct device_attribute attr;
> +	char name[THERMAL_NAME_LENGTH];
> +};
> +
>  struct thermal_zone_device {
>  	int id;
>  	char type[THERMAL_NAME_LENGTH];
>  	struct device device;
> +	struct thermal_attr *trip_temp_attrs;
> +	struct thermal_attr *trip_type_attrs;
>  	void *devdata;
>  	int trips;
>  	int tc1;
> @@ -137,9 +146,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,
> 

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
Zhang Rui July 23, 2012, 8:11 a.m. UTC | #4
Hi, Rafael,

thanks for reviewing this patch set.

On ?, 2012-07-19 at 21:38 +0200, Rafael J. Wysocki wrote:
> On Thursday, July 19, 2012, Zhang Rui wrote:
> > From: Durgadoss R <dugardoss.r@intel.com>
> > 
> > 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>
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > ---
> >  Documentation/thermal/sysfs-api.txt      |    4 +-
> >  drivers/acpi/thermal.c                   |    4 +-
> >  drivers/platform/x86/acerhdf.c           |    2 +-
> >  drivers/platform/x86/intel_mid_thermal.c |    2 +-
> >  drivers/thermal/spear_thermal.c          |    2 +-
> >  drivers/thermal/thermal_sys.c            |  147 +++++++++++++++++++++---------
> >  include/linux/thermal.h                  |   15 ++-
> >  7 files changed, 125 insertions(+), 51 deletions(-)
> > 
> > diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> > index 1733ab9..0c7c423 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.
> 
> I wonder.  What is the maximum number of trip points currently passed to this
> function in one call?
> 
in include/linux/thermal.h
#define THERMAL_MAX_TRIPS 12

thanks,
rui

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhang Rui July 23, 2012, 8:22 a.m. UTC | #5
On ?, 2012-07-19 at 22:27 +0200, Rafael J. Wysocki wrote:
> On Thursday, July 19, 2012, Zhang Rui wrote:
> > From: Durgadoss R <dugardoss.r@intel.com>
> > 
> > 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>
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > ---
> >  Documentation/thermal/sysfs-api.txt      |    4 +-
> >  drivers/acpi/thermal.c                   |    4 +-
> >  drivers/platform/x86/acerhdf.c           |    2 +-
> >  drivers/platform/x86/intel_mid_thermal.c |    2 +-
> >  drivers/thermal/spear_thermal.c          |    2 +-
> >  drivers/thermal/thermal_sys.c            |  147 +++++++++++++++++++++---------
> >  include/linux/thermal.h                  |   15 ++-
> >  7 files changed, 125 insertions(+), 51 deletions(-)
> > 
> > diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> > index 1733ab9..0c7c423 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,
> 
> The 'flags' argument should rather be called 'mask', or even 'writable_mask'.
> 
Agreed.

> >  #define to_cooling_device(_dev)	\
> >  	container_of(_dev, struct thermal_cooling_device, device)
> > @@ -1089,9 +1084,83 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
> >  EXPORT_SYMBOL(thermal_zone_device_update);
> >  >  /**
> > + * create_trip_attrs - create attributes for trip points
> > + * @tz:		the thermal zone device
> > + * @flag:	Writeable trip point bitmap.
> > + */
> > +static int create_trip_attrs(struct thermal_zone_device *tz, int flag)
> > +{
> > +	int indx;
> > +	int writeable;
> > +
> > +	tz->trip_type_attrs =
> > +		kzalloc(sizeof(struct thermal_attr) * tz->trips, GFP_KERNEL);
> > +	if (!tz->trip_type_attrs)
> > +		return -ENOMEM;
> > +
> > +	tz->trip_temp_attrs =
> > +		kzalloc(sizeof(struct thermal_attr) * tz->trips, GFP_KERNEL);
> > +	if (!tz->trip_temp_attrs) {
> > +		kfree(tz->trip_type_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);
> 
> If this really truncates the name, we'll run into problems in _show() and _store().
> Is there any solution to that?
> 
as THERMAL_MAX_TRIP is smaller than 100, so the answer is no.

> > +
> > +		sysfs_attr_init(&tz->trip_type_attrs[count].attr.attr);
> > +		tz->trip_type_attrs[indx].attr.attr.name =
> > +						tz->trip_type_attrs[indx].name;
> > +		tz->trip_type_attrs[indx].attr.attr.mode = S_IRUGO;
> > +		tz->trip_type_attrs[indx].attr.show = trip_point_type_show;
> > +
> > +		device_create_file(&tz->device,
> > +				   &tz->trip_type_attrs[indx].attr);
> > +
> > +		/* create trip temp attribute */
> > +		writeable = flag & (1 << indx);
> > +		snprintf(tz->trip_temp_attrs[indx].name, THERMAL_NAME_LENGTH,
> > +			 "trip_point_%d_temp", indx);
> > +
> > +		sysfs_attr_init(&tz->trip_temp_attrs[indx].attr.attr);
> > +		tz->trip_temp_attrs[indx].attr.attr.name =
> > +						tz->trip_temp_attrs[indx].name;
> > +		tz->trip_temp_attrs[indx].attr.attr.mode = S_IRUGO;
> > +		tz->trip_temp_attrs[indx].attr.show = trip_point_temp_show;
> > +		if (writeable) {
> 
> Why don't you put the "flag & (1 << indx)" here directly instead of 'writable'?
> 
agreed.

> > +			tz->trip_temp_attrs[indx].attr.attr.mode |= S_IWUSR;
> > +			tz->trip_temp_attrs[indx].attr.store =
> > +							trip_point_temp_store;
> > +		}
> > +
> > +		device_create_file(&tz->device,
> > +				   &tz->trip_temp_attrs[indx].attr);
> > +	}
> > +	return 0;
> > +}
> > +
> > +static void remove_trip_attrs(struct thermal_zone_device *tz)
> > +{
> > +	int indx;
> > +
> > +	for (indx = 0; indx < tz->trips; indx++) {
> > +		device_remove_file(&tz->device,
> > +				   &tz->trip_type_attrs[indx].attr);
> > +		device_remove_file(&tz->device,
> > +				   &tz->trip_temp_attrs[indx].attr);
> > +	}
> > +	kfree(tz->trip_type_attrs);
> > +	kfree(tz->trip_temp_attrs);
> > +}
> > +
> > +/**
> >   * 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
> > @@ -1107,7 +1176,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)
> >  {
> > @@ -1124,6 +1193,9 @@ struct thermal_zone_device *thermal_zone_device_register(char *type,
> >  	if (trips > THERMAL_MAX_TRIPS || trips < 0)
> >  		return ERR_PTR(-EINVAL);
> >  
> > +	if (flag >> trips)
> > +		return ERR_PTR(-EINVAL);
> 
> I'm not sure if this check is necessary.  Does it actually matter is someone
> passes ones in the unused bit positions?
> 
I do not think we can trust this value if the caller even wants to set
writable mask for a non-exist trip point.

thanks,
rui

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael Wysocki July 23, 2012, 10:25 a.m. UTC | #6
On Monday, July 23, 2012, Zhang Rui wrote:
> On ?, 2012-07-19 at 22:27 +0200, Rafael J. Wysocki wrote:
> > On Thursday, July 19, 2012, Zhang Rui wrote:
> > > From: Durgadoss R <dugardoss.r@intel.com>
[...]
> > >  	if (trips > THERMAL_MAX_TRIPS || trips < 0)
> > >  		return ERR_PTR(-EINVAL);
> > >  
> > > +	if (flag >> trips)
> > > +		return ERR_PTR(-EINVAL);
> > 
> > I'm not sure if this check is necessary.  Does it actually matter is someone
> > passes ones in the unused bit positions?
> > 
> I do not think we can trust this value if the caller even wants to set
> writable mask for a non-exist trip point.

OK, so perhaps we can merge the two conditionals into one?

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 1733ab9..0c7c423 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 7dbebea..2107d1b 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/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
index 2fd9d36..39abb15 100644
--- a/drivers/platform/x86/acerhdf.c
+++ b/drivers/platform/x86/acerhdf.c
@@ -660,7 +660,7 @@  static int acerhdf_register_thermal(void)
 	if (IS_ERR(cl_dev))
 		return -EINVAL;
 
-	thz_dev = thermal_zone_device_register("acerhdf", 1, NULL,
+	thz_dev = thermal_zone_device_register("acerhdf", 1, 0, NULL,
 					      &acerhdf_dev_ops, 0, 0, 0,
 					      (kernelmode) ? interval*1000 : 0);
 	if (IS_ERR(thz_dev))
diff --git a/drivers/platform/x86/intel_mid_thermal.c b/drivers/platform/x86/intel_mid_thermal.c
index 5ae9cd9..2b2c212 100644
--- a/drivers/platform/x86/intel_mid_thermal.c
+++ b/drivers/platform/x86/intel_mid_thermal.c
@@ -499,7 +499,7 @@  static int mid_thermal_probe(struct platform_device *pdev)
 			goto err;
 		}
 		pinfo->tzd[i] = thermal_zone_device_register(name[i],
-				0, td_info, &tzd_ops, 0, 0, 0, 0);
+				0, 0, td_info, &tzd_ops, 0, 0, 0, 0);
 		if (IS_ERR(pinfo->tzd[i])) {
 			kfree(td_info);
 			ret = PTR_ERR(pinfo->tzd[i]);
diff --git a/drivers/thermal/spear_thermal.c b/drivers/thermal/spear_thermal.c
index c2e32df..69a55d4 100644
--- a/drivers/thermal/spear_thermal.c
+++ b/drivers/thermal/spear_thermal.c
@@ -147,7 +147,7 @@  static int spear_thermal_probe(struct platform_device *pdev)
 	stdev->flags = pdata->thermal_flags;
 	writel_relaxed(stdev->flags, stdev->thermal_base);
 
-	spear_thermal = thermal_zone_device_register("spear_thermal", 0,
+	spear_thermal = thermal_zone_device_register("spear_thermal", 0, 0,
 				stdev, &ops, 0, 0, 0, 0);
 	if (IS_ERR(spear_thermal)) {
 		dev_err(&pdev->dev, "thermal zone device is NULL\n");
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 022bacb..a590d57 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -196,6 +196,28 @@  trip_point_type_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
 trip_point_temp_show(struct device *dev, struct device_attribute *attr,
 		     char *buf)
 {
@@ -283,33 +305,6 @@  static DEVICE_ATTR(temp, 0444, temp_show, NULL);
 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),
-};
-
 /* sys I/F for cooling device */
 #define to_cooling_device(_dev)	\
 	container_of(_dev, struct thermal_cooling_device, device)
@@ -1089,9 +1084,83 @@  void thermal_zone_device_update(struct thermal_zone_device *tz)
 EXPORT_SYMBOL(thermal_zone_device_update);
 
 /**
+ * create_trip_attrs - create attributes for trip points
+ * @tz:		the thermal zone device
+ * @flag:	Writeable trip point bitmap.
+ */
+static int create_trip_attrs(struct thermal_zone_device *tz, int flag)
+{
+	int indx;
+	int writeable;
+
+	tz->trip_type_attrs =
+		kzalloc(sizeof(struct thermal_attr) * tz->trips, GFP_KERNEL);
+	if (!tz->trip_type_attrs)
+		return -ENOMEM;
+
+	tz->trip_temp_attrs =
+		kzalloc(sizeof(struct thermal_attr) * tz->trips, GFP_KERNEL);
+	if (!tz->trip_temp_attrs) {
+		kfree(tz->trip_type_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);
+
+		sysfs_attr_init(&tz->trip_type_attrs[count].attr.attr);
+		tz->trip_type_attrs[indx].attr.attr.name =
+						tz->trip_type_attrs[indx].name;
+		tz->trip_type_attrs[indx].attr.attr.mode = S_IRUGO;
+		tz->trip_type_attrs[indx].attr.show = trip_point_type_show;
+
+		device_create_file(&tz->device,
+				   &tz->trip_type_attrs[indx].attr);
+
+		/* create trip temp attribute */
+		writeable = flag & (1 << indx);
+		snprintf(tz->trip_temp_attrs[indx].name, THERMAL_NAME_LENGTH,
+			 "trip_point_%d_temp", indx);
+
+		sysfs_attr_init(&tz->trip_temp_attrs[indx].attr.attr);
+		tz->trip_temp_attrs[indx].attr.attr.name =
+						tz->trip_temp_attrs[indx].name;
+		tz->trip_temp_attrs[indx].attr.attr.mode = S_IRUGO;
+		tz->trip_temp_attrs[indx].attr.show = trip_point_temp_show;
+		if (writeable) {
+			tz->trip_temp_attrs[indx].attr.attr.mode |= S_IWUSR;
+			tz->trip_temp_attrs[indx].attr.store =
+							trip_point_temp_store;
+		}
+
+		device_create_file(&tz->device,
+				   &tz->trip_temp_attrs[indx].attr);
+	}
+	return 0;
+}
+
+static void remove_trip_attrs(struct thermal_zone_device *tz)
+{
+	int indx;
+
+	for (indx = 0; indx < tz->trips; indx++) {
+		device_remove_file(&tz->device,
+				   &tz->trip_type_attrs[indx].attr);
+		device_remove_file(&tz->device,
+				   &tz->trip_temp_attrs[indx].attr);
+	}
+	kfree(tz->trip_type_attrs);
+	kfree(tz->trip_temp_attrs);
+}
+
+/**
  * 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
@@ -1107,7 +1176,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)
 {
@@ -1124,6 +1193,9 @@  struct thermal_zone_device *thermal_zone_device_register(char *type,
 	if (trips > THERMAL_MAX_TRIPS || trips < 0)
 		return ERR_PTR(-EINVAL);
 
+	if (flag >> trips)
+		return ERR_PTR(-EINVAL);
+
 	if (!ops || !ops->get_temp)
 		return ERR_PTR(-EINVAL);
 
@@ -1175,15 +1247,11 @@  struct thermal_zone_device *thermal_zone_device_register(char *type,
 			goto unregister;
 	}
 
+	result = create_trip_attrs(tz, flag);
+	if (result)
+		goto unregister;
+
 	for (count = 0; count < trips; count++) {
-		result = device_create_file(&tz->device,
-					    &trip_point_attrs[count * 2]);
-		if (result)
-			break;
-		result = device_create_file(&tz->device,
-					    &trip_point_attrs[count * 2 + 1]);
-		if (result)
-			goto unregister;
 		tz->ops->get_trip_type(tz, count, &trip_type);
 		if (trip_type == THERMAL_TRIP_PASSIVE)
 			passive = 1;
@@ -1259,13 +1327,8 @@  void thermal_zone_device_unregister(struct thermal_zone_device *tz)
 	device_remove_file(&tz->device, &dev_attr_temp);
 	if (tz->ops->get_mode)
 		device_remove_file(&tz->device, &dev_attr_mode);
+	remove_trip_attrs(tz);
 
-	for (count = 0; count < tz->trips; count++) {
-		device_remove_file(&tz->device,
-				   &trip_point_attrs[count * 2]);
-		device_remove_file(&tz->device,
-				   &trip_point_attrs[count * 2 + 1]);
-	}
 	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 796f1ff..6eaf914 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);
@@ -85,10 +87,17 @@  struct thermal_cooling_device {
 				((long)t-2732+5)/10 : ((long)t-2732-5)/10)
 #define CELSIUS_TO_KELVIN(t)	((t)*10+2732)
 
+struct thermal_attr {
+	struct device_attribute attr;
+	char name[THERMAL_NAME_LENGTH];
+};
+
 struct thermal_zone_device {
 	int id;
 	char type[THERMAL_NAME_LENGTH];
 	struct device device;
+	struct thermal_attr *trip_temp_attrs;
+	struct thermal_attr *trip_type_attrs;
 	void *devdata;
 	int trips;
 	int tc1;
@@ -137,9 +146,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,