diff mbox

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

Message ID 1380652688-5787-3-git-send-email-durgadoss.r@intel.com (mailing list archive)
State Not Applicable, archived
Delegated to: Zhang Rui
Headers show

Commit Message

durgadoss.r@intel.com Oct. 1, 2013, 6:38 p.m. UTC
This patch creates sensor level APIs, in the
generic thermal framework.

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

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/thermal/thermal_core.c |  284 ++++++++++++++++++++++++++++++++++++++++
 include/linux/thermal.h        |   29 ++++
 2 files changed, 313 insertions(+)

Comments

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

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

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

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

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

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

s/remove_thermal_zone/thermal_sensor_unregister

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

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


--
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
durgadoss.r@intel.com Oct. 14, 2013, 4:21 p.m. UTC | #2
Hi Rui,

> -----Original Message-----

> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-

> owner@vger.kernel.org] On Behalf Of Zhang Rui

> Sent: Monday, October 14, 2013 2:56 PM

> To: R, Durgadoss

> Cc: eduardo.valentin@ti.com; linux-pm@vger.kernel.org; linux-

> kernel@vger.kernel.org; hongbo.zhang@freescale.com; wni@nvidia.com

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

> 

> On Wed, 2013-10-02 at 00:08 +0530, Durgadoss R wrote:

> > This patch creates sensor level APIs, in the

> > generic thermal framework.

> >

> > A Thermal sensor is a piece of hardware that can report

> > temperature of the spot in which it is placed. A thermal

> > sensor driver reads the temperature from this sensor

> > and reports it out. This kind of driver can be in

> > any subsystem. If the sensor needs to participate

> > in platform thermal management, the corresponding

> > driver can use the APIs introduced in this patch, to

> > register(or unregister) with the thermal framework.

> >

> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>

> > ---

> >  drivers/thermal/thermal_core.c |  284

> ++++++++++++++++++++++++++++++++++++++++

> >  include/linux/thermal.h        |   29 ++++

> >  2 files changed, 313 insertions(+)

> >

> > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c

> > index 8c5131d..8c46567 100644

> > --- a/drivers/thermal/thermal_core.c

> > +++ b/drivers/thermal/thermal_core.c

> > @@ -45,13 +45,16 @@ MODULE_LICENSE("GPL v2");

> >

> >  static DEFINE_IDR(thermal_tz_idr);

> >  static DEFINE_IDR(thermal_cdev_idr);

> > +static DEFINE_IDR(thermal_sensor_idr);

> >  static DEFINE_MUTEX(thermal_idr_lock);

> >

> >  static LIST_HEAD(thermal_tz_list);

> > +static LIST_HEAD(thermal_sensor_list);

> >  static LIST_HEAD(thermal_cdev_list);

> >  static LIST_HEAD(thermal_governor_list);

> >

> >  static DEFINE_MUTEX(thermal_list_lock);

> > +static DEFINE_MUTEX(sensor_list_lock);

> >  static DEFINE_MUTEX(thermal_governor_lock);

> >

> >  static struct thermal_governor *__find_governor(const char *name)

> > @@ -463,6 +466,103 @@ static void thermal_zone_device_check(struct

> work_struct *work)

> >  #define to_thermal_zone(_dev) \

> >  	container_of(_dev, struct thermal_zone_device, device)

> >

> > +#define to_thermal_sensor(_dev) \

> > +	container_of(_dev, struct thermal_sensor, device)

> > +

> > +static ssize_t

> > +sensor_name_show(struct device *dev, struct device_attribute *attr, char

> *buf)

> > +{

> > +	struct thermal_sensor *ts = to_thermal_sensor(dev);

> > +

> > +	return sprintf(buf, "%s\n", ts->name);

> > +}

> > +

> > +static ssize_t

> > +sensor_temp_show(struct device *dev, struct device_attribute *attr, char

> *buf)

> > +{

> > +	int ret;

> > +	long val;

> > +	struct thermal_sensor *ts = to_thermal_sensor(dev);

> > +

> > +	ret = ts->ops->get_temp(ts, &val);

> > +

> > +	return ret ? ret : sprintf(buf, "%ld\n", val);

> > +}

> > +

> > +static ssize_t

> > +hyst_show(struct device *dev, struct device_attribute *attr, char *buf)

> > +{

> > +	int indx, ret;

> > +	long val;

> > +	struct thermal_sensor *ts = to_thermal_sensor(dev);

> > +

> > +	if (!sscanf(attr->attr.name, "threshold%d_hyst", &indx))

> > +		return -EINVAL;

> > +

> > +	ret = ts->ops->get_hyst(ts, indx, &val);

> > +

> > +	return ret ? ret : sprintf(buf, "%ld\n", val);

> > +}

> > +

> > +static ssize_t

> > +hyst_store(struct device *dev, struct device_attribute *attr,

> > +				   const char *buf, size_t count)

> > +{

> > +	int indx, ret;

> > +	long val;

> > +	struct thermal_sensor *ts = to_thermal_sensor(dev);

> > +

> > +	if (!ts->ops->set_hyst)

> > +		return -EPERM;

> > +

> > +	if (!sscanf(attr->attr.name, "threshold%d_hyst", &indx))

> > +		return -EINVAL;

> > +

> > +	if (kstrtol(buf, 10, &val))

> > +		return -EINVAL;

> > +

> > +	ret = ts->ops->set_hyst(ts, indx, val);

> > +

> > +	return ret ? ret : count;

> > +}

> > +

> > +static ssize_t

> > +threshold_show(struct device *dev, struct device_attribute *attr, char *buf)

> > +{

> > +	int indx, ret;

> > +	long val;

> > +	struct thermal_sensor *ts = to_thermal_sensor(dev);

> > +

> > +	if (!sscanf(attr->attr.name, "threshold%d", &indx))

> > +		return -EINVAL;

> > +

> > +	ret = ts->ops->get_threshold(ts, indx, &val);

> > +

> > +	return ret ? ret : sprintf(buf, "%ld\n", val);

> > +}

> > +

> > +static ssize_t

> > +threshold_store(struct device *dev, struct device_attribute *attr,

> > +				   const char *buf, size_t count)

> > +{

> > +	int indx, ret;

> > +	long val;

> > +	struct thermal_sensor *ts = to_thermal_sensor(dev);

> > +

> > +	if (!ts->ops->set_threshold)

> > +		return -EPERM;

> > +

> > +	if (!sscanf(attr->attr.name, "threshold%d", &indx))

> > +		return -EINVAL;

> > +

> > +	if (kstrtol(buf, 10, &val))

> > +		return -EINVAL;

> > +

> > +	ret = ts->ops->set_threshold(ts, indx, val);

> > +

> > +	return ret ? ret : count;

> > +}

> > +

> >  static ssize_t

> >  type_show(struct device *dev, struct device_attribute *attr, char *buf)

> >  {

> > @@ -772,6 +872,10 @@ static DEVICE_ATTR(mode, 0644, mode_show,

> mode_store);

> >  static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show,

> passive_store);

> >  static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR, policy_show, policy_store);

> >

> > +/* Thermal sensor attributes */

> > +static DEVICE_ATTR(sensor_name, 0444, sensor_name_show, NULL);

> 

> this attribute is under sensorX/ directory, thus I think "name" is clear

> enough.

> what do you think?


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

> 

> > +static DEVICE_ATTR(temp_input, 0444, sensor_temp_show, NULL);

> > +

> what about "temp"? to be consistent with the current naming.


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

I will try this one again and let you know.

> 

> >  /* sys I/F for cooling device */

> >  #define to_cooling_device(_dev)	\

> >  	container_of(_dev, struct thermal_cooling_device, device)

> > @@ -1585,6 +1689,186 @@ static void remove_trip_attrs(struct

> thermal_zone_device *tz)

> >  	kfree(tz->trip_hyst_attrs);

> >  }

> >

> > + /**

> > + * enable_sensor_thresholds - create sysfs nodes for thresholdX

> IMO, this name is confusing. It looks like that you are enabling the

> sensor hardware here.

> so why not "thermal_sensor_sysfs_add", and you can add the "name" and

> "temp" attribute in this function as well.


Okay. Will do.

> 

> > + * @ts:		the thermal sensor

> > + * @count:	Number of thresholds supported by sensor hardware

> > + *

> > + * 'Thresholds' are temperatures programmed into the sensor hardware,

> > + * on crossing which the sensor can generate an interrupt.

> > + */

> > +static int enable_sensor_thresholds(struct thermal_sensor *ts, int count)

> > +{

> > +	int i;

> > +	int size = sizeof(struct thermal_attr) * count;

> > +

> > +	ts->thresh_attrs = devm_kzalloc(&ts->device, size, GFP_KERNEL);

> > +	if (!ts->thresh_attrs)

> > +		return -ENOMEM;

> > +

> > +	if (ts->ops->get_hyst) {

> > +		ts->hyst_attrs = devm_kzalloc(&ts->device, size, GFP_KERNEL);

> > +		if (!ts->hyst_attrs)

> > +			return -ENOMEM;

> > +	}

> > +

> I do not think you can use devm_kzalloc here.


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

> 

> > +	ts->thresholds = count;

> > +

> > +	/* Create threshold attributes */

> > +	for (i = 0; i < count; i++) {

> > +		snprintf(ts->thresh_attrs[i].name, THERMAL_NAME_LENGTH,

> > +						 "threshold%d", i);

> > +

> > +		sysfs_attr_init(&ts->thresh_attrs[i].attr.attr);

> > +		ts->thresh_attrs[i].attr.attr.name = ts->thresh_attrs[i].name;

> > +		ts->thresh_attrs[i].attr.attr.mode = S_IRUGO | S_IWUSR;

> > +		ts->thresh_attrs[i].attr.show = threshold_show;

> > +		ts->thresh_attrs[i].attr.store = threshold_store;

> > +

> > +		device_create_file(&ts->device, &ts->thresh_attrs[i].attr);

> > +

> > +		/* Create threshold_hyst attributes */

> > +		if (!ts->ops->get_hyst)

> > +			continue;

> > +

> > +		snprintf(ts->hyst_attrs[i].name, THERMAL_NAME_LENGTH,

> > +						 "threshold%d_hyst", i);

> > +

> > +		sysfs_attr_init(&ts->hyst_attrs[i].attr.attr);

> > +		ts->hyst_attrs[i].attr.attr.name = ts->hyst_attrs[i].name;

> > +		ts->hyst_attrs[i].attr.attr.mode = S_IRUGO | S_IWUSR;

> > +		ts->hyst_attrs[i].attr.show = hyst_show;

> > +		ts->hyst_attrs[i].attr.store = hyst_store;

> > +

> > +		device_create_file(&ts->device, &ts->hyst_attrs[i].attr);

> > +	}

> > +	return 0;

> > +}

> > +

> > +/**

> > + * thermal_sensor_register - register a new thermal sensor

> > + * @name:	name of the thermal sensor

> > + * @count:	Number of thresholds supported by hardware

> > + * @ops:	standard thermal sensor callbacks

> > + * @devdata:	private device data

> > + *

> > + * On Success returns a thermal sensor reference, otherwise:

> > + * -EINVAL for invalid parameters,

> > + * -ENOMEM for insufficient memory cases,

> > +*/

> > +struct thermal_sensor *thermal_sensor_register(const char *name, int count,

> > +			struct thermal_sensor_ops *ops, void *devdata)

> > +{

> > +	struct thermal_sensor *ts;

> > +	int ret;

> > +

> > +	if (!name || (name && strlen(name) >= THERMAL_NAME_LENGTH))

> > +		return ERR_PTR(-EINVAL);

> > +

> > +	if (!ops || !ops->get_temp)

> > +		return ERR_PTR(-EINVAL);

> > +

> > +	ts = kzalloc(sizeof(*ts), GFP_KERNEL);

> > +	if (!ts)

> > +		return ERR_PTR(-ENOMEM);

> > +

> > +	idr_init(&ts->idr);

> > +	ret = get_idr(&thermal_sensor_idr, &thermal_idr_lock, &ts->id);

> > +	if (ret)

> > +		goto exit_free;

> > +

> > +	strlcpy(ts->name, name, sizeof(ts->name));

> > +	ts->ops = ops;

> > +	ts->devdata = devdata;

> > +	ts->device.class = &thermal_class;

> > +

> > +	dev_set_name(&ts->device, "sensor%d", ts->id);

> > +	ret = device_register(&ts->device);

> > +	if (ret)

> > +		goto exit_idr;

> > +

> > +	ret = device_create_file(&ts->device, &dev_attr_sensor_name);

> > +	if (ret)

> > +		goto exit_unregister;

> > +

> > +	ret = device_create_file(&ts->device, &dev_attr_temp_input);

> > +	if (ret)

> > +		goto exit_name;

> > +

> > +	if (count > 0 && ts->ops->get_threshold) {

> > +		ret = enable_sensor_thresholds(ts, count);

> > +		if (ret)

> > +			goto exit_temp;

> > +	}

> > +

> > +	/* Add this sensor to the global list of sensors */

> > +	mutex_lock(&sensor_list_lock);

> > +	list_add_tail(&ts->node, &thermal_sensor_list);

> > +	mutex_unlock(&sensor_list_lock);

> > +

> > +	return ts;

> > +

> > +exit_temp:

> > +	device_remove_file(&ts->device, &dev_attr_temp_input);

> > +exit_name:

> > +	device_remove_file(&ts->device, &dev_attr_sensor_name);

> > +exit_unregister:

> > +	device_unregister(&ts->device);

> > +exit_idr:

> > +	release_idr(&thermal_sensor_idr, &thermal_idr_lock, ts->id);

> > +exit_free:

> > +	kfree(ts);

> > +	return ERR_PTR(ret);

> > +}

> > +EXPORT_SYMBOL(thermal_sensor_register);

> > +

> > +/**

> > + * remove_thermal_zone - removes the sysfs nodes for given sensor

> 

> s/remove_thermal_zone/thermal_sensor_unregister

> 

> > + * @ts:	Thermal sensor to be removed.

> > + */

> > +void thermal_sensor_unregister(struct thermal_sensor *ts)

> > +{

> > +	int i;

> > +	struct thermal_sensor *pos, *next;

> > +	bool found = false;

> > +

> > +	if (!ts)

> > +		return;

> > +

> > +	mutex_lock(&sensor_list_lock);

> > +	list_for_each_entry_safe(pos, next, &thermal_sensor_list, node) {

> > +		if (pos == ts) {

> > +			list_del(&ts->node);

> > +			found = true;

> > +			break;

> > +		}

> > +	}

> > +	mutex_unlock(&sensor_list_lock);

> > +

> > +	if (!found)

> > +		return;

> > +

> > +	for (i = 0; i < ts->thresholds; i++) {

> > +		device_remove_file(&ts->device, &ts->thresh_attrs[i].attr);

> > +		if (ts->ops->get_hyst) {

> > +			device_remove_file(&ts->device,

> > +					&ts->hyst_attrs[i].attr);

> > +		}

> > +	}

> > +

> > +	device_remove_file(&ts->device, &dev_attr_sensor_name);

> > +	device_remove_file(&ts->device, &dev_attr_temp_input);

> > +

> hmmm, I'd prefer you to introduce a separate function for the sysfs

> cleanup code.


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

Thanks,
Durga

> 

> thanks,

> rui

> > +	release_idr(&thermal_sensor_idr, &thermal_idr_lock, ts->id);

> > +	idr_destroy(&ts->idr);

> > +

> > +	device_unregister(&ts->device);

> > +

> > +	kfree(ts);

> > +	return;

> > +}

> > +EXPORT_SYMBOL(thermal_sensor_unregister);

> > +

> >  /**

> >   * thermal_zone_device_register() - register a new thermal zone device

> >   * @type:	the thermal zone device type

> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h

> > index a386a1c..25fc562 100644

> > --- a/include/linux/thermal.h

> > +++ b/include/linux/thermal.h

> > @@ -58,6 +58,7 @@

> >  #define DEFAULT_THERMAL_GOVERNOR       "user_space"

> >  #endif

> >

> > +struct thermal_sensor;

> >  struct thermal_zone_device;

> >  struct thermal_cooling_device;

> >

> > @@ -139,6 +140,15 @@ struct thermal_cooling_device_ops {

> >  	int (*set_cur_state) (struct thermal_cooling_device *, unsigned long);

> >  };

> >

> > +struct thermal_sensor_ops {

> > +	int (*get_temp) (struct thermal_sensor *, long *);

> > +	int (*get_trend) (struct thermal_sensor *, int, enum thermal_trend *);

> > +	int (*set_threshold) (struct thermal_sensor *, int, long);

> > +	int (*get_threshold) (struct thermal_sensor *, int, long *);

> > +	int (*set_hyst) (struct thermal_sensor *, int, long);

> > +	int (*get_hyst) (struct thermal_sensor *, int, long *);

> > +};

> > +

> >  struct thermal_cooling_device {

> >  	int id;

> >  	char type[THERMAL_NAME_LENGTH];

> > @@ -156,6 +166,21 @@ struct thermal_attr {

> >  	char name[THERMAL_NAME_LENGTH];

> >  };

> >

> > +struct thermal_sensor {

> > +	char name[THERMAL_NAME_LENGTH];

> > +	int id;

> > +	int temp;

> > +	int prev_temp;

> > +	int thresholds;

> > +	void *devdata;

> > +	struct idr idr;

> > +	struct device device;

> > +	struct list_head node;

> > +	struct thermal_sensor_ops *ops;

> > +	struct thermal_attr *thresh_attrs;

> > +	struct thermal_attr *hyst_attrs;

> > +};

> > +

> >  struct thermal_zone_device {

> >  	int id;

> >  	char type[THERMAL_NAME_LENGTH];

> > @@ -248,6 +273,10 @@ struct thermal_instance *get_thermal_instance(struct

> thermal_zone_device *,

> >  void thermal_cdev_update(struct thermal_cooling_device *);

> >  void thermal_notify_framework(struct thermal_zone_device *, int);

> >

> > +struct thermal_sensor *thermal_sensor_register(const char *, int,

> > +				struct thermal_sensor_ops *, void *);

> > +void thermal_sensor_unregister(struct thermal_sensor *);

> > +

> >  #ifdef CONFIG_NET

> >  extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,

> >  						enum events event);

> 

> 

> --

> To unsubscribe from this list: send the line "unsubscribe linux-pm" in

> the body of a message to majordomo@vger.kernel.org

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

thanks,
rui



--
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/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 8c5131d..8c46567 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -45,13 +45,16 @@  MODULE_LICENSE("GPL v2");
 
 static DEFINE_IDR(thermal_tz_idr);
 static DEFINE_IDR(thermal_cdev_idr);
+static DEFINE_IDR(thermal_sensor_idr);
 static DEFINE_MUTEX(thermal_idr_lock);
 
 static LIST_HEAD(thermal_tz_list);
+static LIST_HEAD(thermal_sensor_list);
 static LIST_HEAD(thermal_cdev_list);
 static LIST_HEAD(thermal_governor_list);
 
 static DEFINE_MUTEX(thermal_list_lock);
+static DEFINE_MUTEX(sensor_list_lock);
 static DEFINE_MUTEX(thermal_governor_lock);
 
 static struct thermal_governor *__find_governor(const char *name)
@@ -463,6 +466,103 @@  static void thermal_zone_device_check(struct work_struct *work)
 #define to_thermal_zone(_dev) \
 	container_of(_dev, struct thermal_zone_device, device)
 
+#define to_thermal_sensor(_dev) \
+	container_of(_dev, struct thermal_sensor, device)
+
+static ssize_t
+sensor_name_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct thermal_sensor *ts = to_thermal_sensor(dev);
+
+	return sprintf(buf, "%s\n", ts->name);
+}
+
+static ssize_t
+sensor_temp_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	int ret;
+	long val;
+	struct thermal_sensor *ts = to_thermal_sensor(dev);
+
+	ret = ts->ops->get_temp(ts, &val);
+
+	return ret ? ret : sprintf(buf, "%ld\n", val);
+}
+
+static ssize_t
+hyst_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	int indx, ret;
+	long val;
+	struct thermal_sensor *ts = to_thermal_sensor(dev);
+
+	if (!sscanf(attr->attr.name, "threshold%d_hyst", &indx))
+		return -EINVAL;
+
+	ret = ts->ops->get_hyst(ts, indx, &val);
+
+	return ret ? ret : sprintf(buf, "%ld\n", val);
+}
+
+static ssize_t
+hyst_store(struct device *dev, struct device_attribute *attr,
+				   const char *buf, size_t count)
+{
+	int indx, ret;
+	long val;
+	struct thermal_sensor *ts = to_thermal_sensor(dev);
+
+	if (!ts->ops->set_hyst)
+		return -EPERM;
+
+	if (!sscanf(attr->attr.name, "threshold%d_hyst", &indx))
+		return -EINVAL;
+
+	if (kstrtol(buf, 10, &val))
+		return -EINVAL;
+
+	ret = ts->ops->set_hyst(ts, indx, val);
+
+	return ret ? ret : count;
+}
+
+static ssize_t
+threshold_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	int indx, ret;
+	long val;
+	struct thermal_sensor *ts = to_thermal_sensor(dev);
+
+	if (!sscanf(attr->attr.name, "threshold%d", &indx))
+		return -EINVAL;
+
+	ret = ts->ops->get_threshold(ts, indx, &val);
+
+	return ret ? ret : sprintf(buf, "%ld\n", val);
+}
+
+static ssize_t
+threshold_store(struct device *dev, struct device_attribute *attr,
+				   const char *buf, size_t count)
+{
+	int indx, ret;
+	long val;
+	struct thermal_sensor *ts = to_thermal_sensor(dev);
+
+	if (!ts->ops->set_threshold)
+		return -EPERM;
+
+	if (!sscanf(attr->attr.name, "threshold%d", &indx))
+		return -EINVAL;
+
+	if (kstrtol(buf, 10, &val))
+		return -EINVAL;
+
+	ret = ts->ops->set_threshold(ts, indx, val);
+
+	return ret ? ret : count;
+}
+
 static ssize_t
 type_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
@@ -772,6 +872,10 @@  static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
 static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show, passive_store);
 static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR, policy_show, policy_store);
 
+/* Thermal sensor attributes */
+static DEVICE_ATTR(sensor_name, 0444, sensor_name_show, NULL);
+static DEVICE_ATTR(temp_input, 0444, sensor_temp_show, NULL);
+
 /* sys I/F for cooling device */
 #define to_cooling_device(_dev)	\
 	container_of(_dev, struct thermal_cooling_device, device)
@@ -1585,6 +1689,186 @@  static void remove_trip_attrs(struct thermal_zone_device *tz)
 	kfree(tz->trip_hyst_attrs);
 }
 
+ /**
+ * enable_sensor_thresholds - create sysfs nodes for thresholdX
+ * @ts:		the thermal sensor
+ * @count:	Number of thresholds supported by sensor hardware
+ *
+ * 'Thresholds' are temperatures programmed into the sensor hardware,
+ * on crossing which the sensor can generate an interrupt.
+ */
+static int enable_sensor_thresholds(struct thermal_sensor *ts, int count)
+{
+	int i;
+	int size = sizeof(struct thermal_attr) * count;
+
+	ts->thresh_attrs = devm_kzalloc(&ts->device, size, GFP_KERNEL);
+	if (!ts->thresh_attrs)
+		return -ENOMEM;
+
+	if (ts->ops->get_hyst) {
+		ts->hyst_attrs = devm_kzalloc(&ts->device, size, GFP_KERNEL);
+		if (!ts->hyst_attrs)
+			return -ENOMEM;
+	}
+
+	ts->thresholds = count;
+
+	/* Create threshold attributes */
+	for (i = 0; i < count; i++) {
+		snprintf(ts->thresh_attrs[i].name, THERMAL_NAME_LENGTH,
+						 "threshold%d", i);
+
+		sysfs_attr_init(&ts->thresh_attrs[i].attr.attr);
+		ts->thresh_attrs[i].attr.attr.name = ts->thresh_attrs[i].name;
+		ts->thresh_attrs[i].attr.attr.mode = S_IRUGO | S_IWUSR;
+		ts->thresh_attrs[i].attr.show = threshold_show;
+		ts->thresh_attrs[i].attr.store = threshold_store;
+
+		device_create_file(&ts->device, &ts->thresh_attrs[i].attr);
+
+		/* Create threshold_hyst attributes */
+		if (!ts->ops->get_hyst)
+			continue;
+
+		snprintf(ts->hyst_attrs[i].name, THERMAL_NAME_LENGTH,
+						 "threshold%d_hyst", i);
+
+		sysfs_attr_init(&ts->hyst_attrs[i].attr.attr);
+		ts->hyst_attrs[i].attr.attr.name = ts->hyst_attrs[i].name;
+		ts->hyst_attrs[i].attr.attr.mode = S_IRUGO | S_IWUSR;
+		ts->hyst_attrs[i].attr.show = hyst_show;
+		ts->hyst_attrs[i].attr.store = hyst_store;
+
+		device_create_file(&ts->device, &ts->hyst_attrs[i].attr);
+	}
+	return 0;
+}
+
+/**
+ * thermal_sensor_register - register a new thermal sensor
+ * @name:	name of the thermal sensor
+ * @count:	Number of thresholds supported by hardware
+ * @ops:	standard thermal sensor callbacks
+ * @devdata:	private device data
+ *
+ * On Success returns a thermal sensor reference, otherwise:
+ * -EINVAL for invalid parameters,
+ * -ENOMEM for insufficient memory cases,
+*/
+struct thermal_sensor *thermal_sensor_register(const char *name, int count,
+			struct thermal_sensor_ops *ops, void *devdata)
+{
+	struct thermal_sensor *ts;
+	int ret;
+
+	if (!name || (name && strlen(name) >= THERMAL_NAME_LENGTH))
+		return ERR_PTR(-EINVAL);
+
+	if (!ops || !ops->get_temp)
+		return ERR_PTR(-EINVAL);
+
+	ts = kzalloc(sizeof(*ts), GFP_KERNEL);
+	if (!ts)
+		return ERR_PTR(-ENOMEM);
+
+	idr_init(&ts->idr);
+	ret = get_idr(&thermal_sensor_idr, &thermal_idr_lock, &ts->id);
+	if (ret)
+		goto exit_free;
+
+	strlcpy(ts->name, name, sizeof(ts->name));
+	ts->ops = ops;
+	ts->devdata = devdata;
+	ts->device.class = &thermal_class;
+
+	dev_set_name(&ts->device, "sensor%d", ts->id);
+	ret = device_register(&ts->device);
+	if (ret)
+		goto exit_idr;
+
+	ret = device_create_file(&ts->device, &dev_attr_sensor_name);
+	if (ret)
+		goto exit_unregister;
+
+	ret = device_create_file(&ts->device, &dev_attr_temp_input);
+	if (ret)
+		goto exit_name;
+
+	if (count > 0 && ts->ops->get_threshold) {
+		ret = enable_sensor_thresholds(ts, count);
+		if (ret)
+			goto exit_temp;
+	}
+
+	/* Add this sensor to the global list of sensors */
+	mutex_lock(&sensor_list_lock);
+	list_add_tail(&ts->node, &thermal_sensor_list);
+	mutex_unlock(&sensor_list_lock);
+
+	return ts;
+
+exit_temp:
+	device_remove_file(&ts->device, &dev_attr_temp_input);
+exit_name:
+	device_remove_file(&ts->device, &dev_attr_sensor_name);
+exit_unregister:
+	device_unregister(&ts->device);
+exit_idr:
+	release_idr(&thermal_sensor_idr, &thermal_idr_lock, ts->id);
+exit_free:
+	kfree(ts);
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(thermal_sensor_register);
+
+/**
+ * remove_thermal_zone - removes the sysfs nodes for given sensor
+ * @ts:	Thermal sensor to be removed.
+ */
+void thermal_sensor_unregister(struct thermal_sensor *ts)
+{
+	int i;
+	struct thermal_sensor *pos, *next;
+	bool found = false;
+
+	if (!ts)
+		return;
+
+	mutex_lock(&sensor_list_lock);
+	list_for_each_entry_safe(pos, next, &thermal_sensor_list, node) {
+		if (pos == ts) {
+			list_del(&ts->node);
+			found = true;
+			break;
+		}
+	}
+	mutex_unlock(&sensor_list_lock);
+
+	if (!found)
+		return;
+
+	for (i = 0; i < ts->thresholds; i++) {
+		device_remove_file(&ts->device, &ts->thresh_attrs[i].attr);
+		if (ts->ops->get_hyst) {
+			device_remove_file(&ts->device,
+					&ts->hyst_attrs[i].attr);
+		}
+	}
+
+	device_remove_file(&ts->device, &dev_attr_sensor_name);
+	device_remove_file(&ts->device, &dev_attr_temp_input);
+
+	release_idr(&thermal_sensor_idr, &thermal_idr_lock, ts->id);
+	idr_destroy(&ts->idr);
+
+	device_unregister(&ts->device);
+
+	kfree(ts);
+	return;
+}
+EXPORT_SYMBOL(thermal_sensor_unregister);
+
 /**
  * thermal_zone_device_register() - register a new thermal zone device
  * @type:	the thermal zone device type
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index a386a1c..25fc562 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -58,6 +58,7 @@ 
 #define DEFAULT_THERMAL_GOVERNOR       "user_space"
 #endif
 
+struct thermal_sensor;
 struct thermal_zone_device;
 struct thermal_cooling_device;
 
@@ -139,6 +140,15 @@  struct thermal_cooling_device_ops {
 	int (*set_cur_state) (struct thermal_cooling_device *, unsigned long);
 };
 
+struct thermal_sensor_ops {
+	int (*get_temp) (struct thermal_sensor *, long *);
+	int (*get_trend) (struct thermal_sensor *, int, enum thermal_trend *);
+	int (*set_threshold) (struct thermal_sensor *, int, long);
+	int (*get_threshold) (struct thermal_sensor *, int, long *);
+	int (*set_hyst) (struct thermal_sensor *, int, long);
+	int (*get_hyst) (struct thermal_sensor *, int, long *);
+};
+
 struct thermal_cooling_device {
 	int id;
 	char type[THERMAL_NAME_LENGTH];
@@ -156,6 +166,21 @@  struct thermal_attr {
 	char name[THERMAL_NAME_LENGTH];
 };
 
+struct thermal_sensor {
+	char name[THERMAL_NAME_LENGTH];
+	int id;
+	int temp;
+	int prev_temp;
+	int thresholds;
+	void *devdata;
+	struct idr idr;
+	struct device device;
+	struct list_head node;
+	struct thermal_sensor_ops *ops;
+	struct thermal_attr *thresh_attrs;
+	struct thermal_attr *hyst_attrs;
+};
+
 struct thermal_zone_device {
 	int id;
 	char type[THERMAL_NAME_LENGTH];
@@ -248,6 +273,10 @@  struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
 void thermal_cdev_update(struct thermal_cooling_device *);
 void thermal_notify_framework(struct thermal_zone_device *, int);
 
+struct thermal_sensor *thermal_sensor_register(const char *, int,
+				struct thermal_sensor_ops *, void *);
+void thermal_sensor_unregister(struct thermal_sensor *);
+
 #ifdef CONFIG_NET
 extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,
 						enum events event);