diff mbox

[RFC,5/9] Thermal: Support using dt node to get sensor

Message ID 1361187031-3679-6-git-send-email-wni@nvidia.com (mailing list archive)
State RFC, archived
Delegated to: Zhang Rui
Headers show

Commit Message

Wei Ni Feb. 18, 2013, 11:30 a.m. UTC
Add functions to support using dt node with args to get sensor.

Signed-off-by: Wei Ni <wni@nvidia.com>
---
 drivers/thermal/thermal_sys.c |   30 ++++++++++++++++++++++++++++++
 include/linux/thermal.h       |    9 +++++++++
 2 files changed, 39 insertions(+)

Comments

Stephen Warren Feb. 19, 2013, 11:12 p.m. UTC | #1
On 02/18/2013 04:30 AM, Wei Ni wrote:
> Add functions to support using dt node with args to get sensor.

You need to write a device tree binding document to explain this.

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

> +struct thermal_sensor *get_sensor_by_node(struct node_args *np_args)
> +{
> +	struct thermal_sensor *pos;

"pos" isn't a great variable name. Why not use "sensor", or just the
"ts" variable you have right below?

> +	struct thermal_sensor *ts = NULL;
> +	struct node_args *args;
> +
> +	mutex_lock(&sensor_list_lock);
> +	for_each_thermal_sensor(pos) {
> +		args = &pos->np_args;
> +		if (args->np) {
> +			if ((args->np == np_args->np) &&
> +			   (args->index == np_args->index)) {
> +				ts = pos;
> +				break;

Replace those 2 lines with "goto out;".

> +			}
> +		}
> +	}

here, add:

	ts = NULL;
out:

That way, you can use "ts" as the loop iteration variable.

This whole patch rather assumes that all DT nodes can identify their
exposed thermal sensors using an index in a single DT cell. That's not
very flexible. All other DT bindings work like this:

Provider of a service indicates how many DT cells are in the object
(GPIO, IRQ, thermal sensors) specifier:

sensor1: lm90@1c {
	...
	#thermal-sensor-cells = <1>;
};

Each consumer of a service imports it by referencing it:

thermal-zone {
	...
	sensors = <&sensor1 0>;
};

The driver for LM90 provides an "of_xlate" function which receives a
struct of_phandle_args and outputs/returns whatever Linux-internal
identification/representation of the object is required. For example, see:

> include/linux/pwm.h:161:	struct pwm_device *	(*of_xlate)(struct pwm_chip *pc,

This allows each providing object's DT binding to define its own value
of #thermal-sensor-cells, as suited for its own requirements, and allows
each driver to implement the mapping from DT to internal ID in whatever
way is necessary.

Now, many bindings/drivers might just end up using a common simple
implementation. That's why functions such as of_pwm_simple_xlate() or
of_gpio_simple_xlate() exist.
--
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
Wei Ni Feb. 20, 2013, 10:36 a.m. UTC | #2
On 02/20/2013 07:12 AM, Stephen Warren wrote:
> On 02/18/2013 04:30 AM, Wei Ni wrote:
>> Add functions to support using dt node with args to get sensor.
> 
> You need to write a device tree binding document to explain this.
> 
>> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> 
>> +struct thermal_sensor *get_sensor_by_node(struct node_args *np_args)
>> +{
>> +	struct thermal_sensor *pos;
> 
> "pos" isn't a great variable name. Why not use "sensor", or just the
> "ts" variable you have right below?

Oh, yes, it can just use "ts" simply. I didn't consider it, thanks.

> 
>> +	struct thermal_sensor *ts = NULL;
>> +	struct node_args *args;
>> +
>> +	mutex_lock(&sensor_list_lock);
>> +	for_each_thermal_sensor(pos) {
>> +		args = &pos->np_args;
>> +		if (args->np) {
>> +			if ((args->np == np_args->np) &&
>> +			   (args->index == np_args->index)) {
>> +				ts = pos;
>> +				break;
> 
> Replace those 2 lines with "goto out;".
> 
>> +			}
>> +		}
>> +	}
> 
> here, add:
> 
> 	ts = NULL;
> out:
> 
> That way, you can use "ts" as the loop iteration variable.
> 
> This whole patch rather assumes that all DT nodes can identify their
> exposed thermal sensors using an index in a single DT cell. That's not
> very flexible. All other DT bindings work like this:
> 
> Provider of a service indicates how many DT cells are in the object
> (GPIO, IRQ, thermal sensors) specifier:
> 
> sensor1: lm90@1c {
> 	...
> 	#thermal-sensor-cells = <1>;
> };
> 
> Each consumer of a service imports it by referencing it:
> 
> thermal-zone {
> 	...
> 	sensors = <&sensor1 0>;
> };
> 
> The driver for LM90 provides an "of_xlate" function which receives a
> struct of_phandle_args and outputs/returns whatever Linux-internal
> identification/representation of the object is required. For example, see:
> 
>> include/linux/pwm.h:161:	struct pwm_device *	(*of_xlate)(struct pwm_chip *pc,
> 
> This allows each providing object's DT binding to define its own value
> of #thermal-sensor-cells, as suited for its own requirements, and allows
> each driver to implement the mapping from DT to internal ID in whatever
> way is necessary.
> 
> Now, many bindings/drivers might just end up using a common simple
> implementation. That's why functions such as of_pwm_simple_xlate() or
> of_gpio_simple_xlate() exist.

It looks like the "of_xlate" can handle the sensor identification.
I'm not familiar with this function, I will look into it and try to use.
Thanks for your comments.

Wei.

> 

--
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_sys.c b/drivers/thermal/thermal_sys.c
index e284b67..b5bedab 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -2183,6 +2183,28 @@  struct thermal_sensor *get_sensor_by_name(const char *name)
 }
 EXPORT_SYMBOL(get_sensor_by_name);
 
+struct thermal_sensor *get_sensor_by_node(struct node_args *np_args)
+{
+	struct thermal_sensor *pos;
+	struct thermal_sensor *ts = NULL;
+	struct node_args *args;
+
+	mutex_lock(&sensor_list_lock);
+	for_each_thermal_sensor(pos) {
+		args = &pos->np_args;
+		if (args->np) {
+			if ((args->np == np_args->np) &&
+			   (args->index == np_args->index)) {
+				ts = pos;
+				break;
+			}
+		}
+	}
+	mutex_unlock(&sensor_list_lock);
+	return ts;
+}
+EXPORT_SYMBOL(get_sensor_by_node);
+
 static int create_map_attrs(struct thermal_zone *tz, int indx)
 {
 	int ret, i;
@@ -2368,6 +2390,7 @@  EXPORT_SYMBOL(add_sensor_trip_info);
  * @devdata:	private device data
  */
 struct thermal_sensor *thermal_sensor_register(const char *name, int count,
+			struct node_args *np_args,
 			struct thermal_sensor_ops *ops, void *devdata)
 {
 	struct thermal_sensor *ts;
@@ -2412,6 +2435,13 @@  struct thermal_sensor *thermal_sensor_register(const char *name, int count,
 			goto exit_temp;
 	}
 
+	if (np_args) {
+		ts->np_args.np = np_args->np;
+		ts->np_args.index = np_args->index;
+	} else {
+		ts->np_args.np = NULL;
+	}
+
 	/* Add this sensor to the global list of sensors */
 	mutex_lock(&sensor_list_lock);
 	list_add_tail(&ts->node, &thermal_sensor_list);
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 4389599..b560ffa 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -113,6 +113,12 @@  enum {
 };
 #define THERMAL_GENL_CMD_MAX (__THERMAL_GENL_CMD_MAX - 1)
 
+/* support to parse node with args to get sensor or cooling device */
+struct node_args {
+	struct device_node *np;
+	int index;
+};
+
 struct thermal_zone_device_ops {
 	int (*bind) (struct thermal_zone_device *,
 		     struct thermal_cooling_device *);
@@ -223,6 +229,7 @@  struct thermal_sensor {
 	void *devdata;
 	struct idr idr;
 	struct device device;
+	struct node_args np_args;
 	struct list_head node;
 	struct thermal_sensor_ops *ops;
 	struct thermal_attr *thresh_attrs;
@@ -350,6 +357,7 @@  int thermal_register_governor(struct thermal_governor *);
 void thermal_unregister_governor(struct thermal_governor *);
 
 struct thermal_sensor *thermal_sensor_register(const char *, int,
+				struct node_args *,
 				struct thermal_sensor_ops *, void *);
 void thermal_sensor_unregister(struct thermal_sensor *);
 
@@ -357,6 +365,7 @@  struct thermal_zone *create_thermal_zone(const char *, void *);
 void remove_thermal_zone(struct thermal_zone *);
 int add_sensor_to_zone(struct thermal_zone *, struct thermal_sensor *);
 struct thermal_sensor *get_sensor_by_name(const char *);
+struct thermal_sensor *get_sensor_by_node(struct node_args *);
 
 int add_cdev_to_zone(struct thermal_zone *, struct thermal_cooling_device *);
 struct thermal_cooling_device *get_cdev_by_name(const char *);