linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv2 0/3] thermal: lookup temperature
@ 2013-04-03 22:13 Eduardo Valentin
  2013-04-03 22:13 ` [PATCHv2 1/3] thermal: introduce thermal_zone_get_zone_by_name helper function Eduardo Valentin
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Eduardo Valentin @ 2013-04-03 22:13 UTC (permalink / raw)
  To: rui.zhang; +Cc: linux-pm, linux-kernel, durgadoss.r, Eduardo Valentin

Hello Rui,

Here is V2 of temperature lookup helper function. This has been
split into two API as suggested on V1.

The usage of it is exemplified on patch 03.


Eduardo Valentin (3):
  thermal: introduce thermal_zone_get_zone_by_name helper function
  thermal: expose thermal_zone_get_temp API
  staging: ti-soc-thermal: remove external heat while extrapolating
    hotspot

 drivers/staging/ti-soc-thermal/ti-thermal-common.c |   30 +++++++----
 drivers/thermal/thermal_sys.c                      |   54 ++++++++++++++++++-
 include/linux/thermal.h                            |    2 +
 3 files changed, 73 insertions(+), 13 deletions(-)

-- 
1.7.7.1.488.ge8e1c


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

* [PATCHv2 1/3] thermal: introduce thermal_zone_get_zone_by_name helper function
  2013-04-03 22:13 [PATCHv2 0/3] thermal: lookup temperature Eduardo Valentin
@ 2013-04-03 22:13 ` Eduardo Valentin
  2013-04-04 17:12   ` R, Durgadoss
  2013-04-03 22:13 ` [PATCHv2 2/3] thermal: expose thermal_zone_get_temp API Eduardo Valentin
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Eduardo Valentin @ 2013-04-03 22:13 UTC (permalink / raw)
  To: rui.zhang; +Cc: linux-pm, linux-kernel, durgadoss.r, Eduardo Valentin

This patch adds a helper function to get a reference of
a thermal zone, based on the zone type name.

It will perform a zone name lookup and return a reference
to a thermal zone device that matches the name requested.
In case the zone is not found or when several zones match
same name or if the required parameters are invalid, it will return
the corresponding error code (ERR_PTR).

Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
---
 drivers/thermal/thermal_sys.c |   34 ++++++++++++++++++++++++++++++++++
 include/linux/thermal.h       |    1 +
 2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 5bd95d4..6e1da0a 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -1790,6 +1790,40 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
 }
 EXPORT_SYMBOL_GPL(thermal_zone_device_unregister);
 
+/**
+ * thermal_zone_get_zone_by_name() - search for a zone and returns its ref
+ * @name: thermal zone name to fetch the temperature
+ *
+ * When only one zone is found with the passed name, returns a reference to it.
+ *
+ * Return: On success returns a reference to an unique thermal zone with
+ * matching name equals to @name, a ERR_PTR otherwise.
+ */
+struct thermal_zone_device *thermal_zone_get_zone_by_name(const char *name)
+{
+	struct thermal_zone_device *pos = NULL, *ref = ERR_PTR(-EINVAL);
+	int found = 0;
+
+	if (!name)
+		goto exit;
+
+	mutex_lock(&thermal_list_lock);
+	list_for_each_entry(pos, &thermal_tz_list, node)
+		if (!strnicmp(name, pos->type, THERMAL_NAME_LENGTH)) {
+			found++;
+			ref = pos;
+		}
+	mutex_unlock(&thermal_list_lock);
+
+	/* Success only when an unique zone is found */
+	if (found != 1)
+		ref = ERR_PTR(-ENODEV);
+
+exit:
+	return ref;
+}
+EXPORT_SYMBOL_GPL(thermal_zone_get_zone_by_name);
+
 #ifdef CONFIG_NET
 static struct genl_family thermal_event_genl_family = {
 	.id = GENL_ID_GENERATE,
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 542a39c..0cf9eb5 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -237,6 +237,7 @@ void thermal_zone_device_update(struct thermal_zone_device *);
 struct thermal_cooling_device *thermal_cooling_device_register(char *, void *,
 		const struct thermal_cooling_device_ops *);
 void thermal_cooling_device_unregister(struct thermal_cooling_device *);
+struct thermal_zone_device *thermal_zone_get_zone_by_name(const char *name);
 
 int thermal_zone_trend_get(struct thermal_zone_device *, int);
 struct thermal_instance *thermal_instance_get(struct thermal_zone_device *,
-- 
1.7.7.1.488.ge8e1c


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

* [PATCHv2 2/3] thermal: expose thermal_zone_get_temp API
  2013-04-03 22:13 [PATCHv2 0/3] thermal: lookup temperature Eduardo Valentin
  2013-04-03 22:13 ` [PATCHv2 1/3] thermal: introduce thermal_zone_get_zone_by_name helper function Eduardo Valentin
@ 2013-04-03 22:13 ` Eduardo Valentin
  2013-04-04 17:13   ` R, Durgadoss
  2013-04-03 22:13 ` [PATCHv2 3/3] staging: ti-soc-thermal: remove external heat while extrapolating hotspot Eduardo Valentin
  2013-04-22 11:06 ` [PATCHv2 0/3] thermal: lookup temperature Pavel Machek
  3 siblings, 1 reply; 10+ messages in thread
From: Eduardo Valentin @ 2013-04-03 22:13 UTC (permalink / raw)
  To: rui.zhang; +Cc: linux-pm, linux-kernel, durgadoss.r, Eduardo Valentin

This patch exports the thermal_zone_get_temp API so that driver
writers can fetch temperature of thermal zones managed by other
drivers.

Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
---
 drivers/thermal/thermal_sys.c |   20 +++++++++++++++++---
 include/linux/thermal.h       |    1 +
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 6e1da0a..e8afb7f 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -371,16 +371,28 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
 	monitor_thermal_zone(tz);
 }
 
-static int thermal_zone_get_temp(struct thermal_zone_device *tz,
-				unsigned long *temp)
+/**
+ * thermal_zone_get_temp() - returns its the temperature of thermal zone
+ * @tz: a valid pointer to a struct thermal_zone_device
+ * @temp: a valid pointer to where to store the resulting temperature.
+ *
+ * When a valid thermal zone reference is passed, it will fetch its
+ * temperature and fill @temp.
+ *
+ * Return: On success returns 0, an error code otherwise
+ */
+int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp)
 {
-	int ret = 0;
+	int ret = -EINVAL;
 #ifdef CONFIG_THERMAL_EMULATION
 	int count;
 	unsigned long crit_temp = -1UL;
 	enum thermal_trip_type type;
 #endif
 
+	if (IS_ERR_OR_NULL(tz))
+		goto exit;
+
 	mutex_lock(&tz->lock);
 
 	ret = tz->ops->get_temp(tz, temp);
@@ -404,8 +416,10 @@ static int thermal_zone_get_temp(struct thermal_zone_device *tz,
 skip_emul:
 #endif
 	mutex_unlock(&tz->lock);
+exit:
 	return ret;
 }
+EXPORT_SYMBOL_GPL(thermal_zone_get_temp);
 
 static void update_temperature(struct thermal_zone_device *tz)
 {
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 0cf9eb5..8eea86c 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -238,6 +238,7 @@ struct thermal_cooling_device *thermal_cooling_device_register(char *, void *,
 		const struct thermal_cooling_device_ops *);
 void thermal_cooling_device_unregister(struct thermal_cooling_device *);
 struct thermal_zone_device *thermal_zone_get_zone_by_name(const char *name);
+int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned long *temp);
 
 int thermal_zone_trend_get(struct thermal_zone_device *, int);
 struct thermal_instance *thermal_instance_get(struct thermal_zone_device *,
-- 
1.7.7.1.488.ge8e1c


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

* [PATCHv2 3/3] staging: ti-soc-thermal: remove external heat while extrapolating hotspot
  2013-04-03 22:13 [PATCHv2 0/3] thermal: lookup temperature Eduardo Valentin
  2013-04-03 22:13 ` [PATCHv2 1/3] thermal: introduce thermal_zone_get_zone_by_name helper function Eduardo Valentin
  2013-04-03 22:13 ` [PATCHv2 2/3] thermal: expose thermal_zone_get_temp API Eduardo Valentin
@ 2013-04-03 22:13 ` Eduardo Valentin
  2013-04-22 11:06 ` [PATCHv2 0/3] thermal: lookup temperature Pavel Machek
  3 siblings, 0 replies; 10+ messages in thread
From: Eduardo Valentin @ 2013-04-03 22:13 UTC (permalink / raw)
  To: rui.zhang; +Cc: linux-pm, linux-kernel, durgadoss.r, Eduardo Valentin

For boards that provide a PCB sensor close to SoC junction
temperature, it is possible to remove the cumulative heat
reported by the SoC temperature sensor.

This patch changes the extrapolation computation to consider
an external sensor in the extrapolation equations.

Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
---
 drivers/staging/ti-soc-thermal/ti-thermal-common.c |   30 +++++++++++++------
 1 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/ti-soc-thermal/ti-thermal-common.c b/drivers/staging/ti-soc-thermal/ti-thermal-common.c
index 231c549..780368b 100644
--- a/drivers/staging/ti-soc-thermal/ti-thermal-common.c
+++ b/drivers/staging/ti-soc-thermal/ti-thermal-common.c
@@ -38,6 +38,7 @@
 /* common data structures */
 struct ti_thermal_data {
 	struct thermal_zone_device *ti_thermal;
+	struct thermal_zone_device *pcb_tz;
 	struct thermal_cooling_device *cool_dev;
 	struct ti_bandgap *bgp;
 	enum thermal_device_mode mode;
@@ -77,10 +78,12 @@ static inline int ti_thermal_hotspot_temperature(int t, int s, int c)
 static inline int ti_thermal_get_temp(struct thermal_zone_device *thermal,
 				      unsigned long *temp)
 {
+	struct thermal_zone_device *pcb_tz = NULL;
 	struct ti_thermal_data *data = thermal->devdata;
 	struct ti_bandgap *bgp;
 	const struct ti_temp_sensor *s;
-	int ret, tmp, pcb_temp, slope, constant;
+	int ret, tmp, slope, constant;
+	unsigned long pcb_temp;
 
 	if (!data)
 		return 0;
@@ -92,16 +95,22 @@ static inline int ti_thermal_get_temp(struct thermal_zone_device *thermal,
 	if (ret)
 		return ret;
 
-	pcb_temp = 0;
-	/* TODO: Introduce pcb temperature lookup */
+	/* Default constants */
+	slope = s->slope;
+	constant = s->constant;
+
+	pcb_tz = data->pcb_tz;
 	/* In case pcb zone is available, use the extrapolation rule with it */
-	if (pcb_temp) {
-		tmp -= pcb_temp;
-		slope = s->slope_pcb;
-		constant = s->constant_pcb;
-	} else {
-		slope = s->slope;
-		constant = s->constant;
+	if (!IS_ERR_OR_NULL(pcb_tz)) {
+		ret = thermal_zone_get_temp(pcb_tz, &pcb_temp);
+		if (!ret) {
+			tmp -= pcb_temp; /* got a valid PCB temp */
+			slope = s->slope_pcb;
+			constant = s->constant_pcb;
+		} else {
+			dev_err(bgp->dev,
+				"Failed to read PCB state. Using defaults\n");
+		}
 	}
 	*temp = ti_thermal_hotspot_temperature(tmp, slope, constant);
 
@@ -248,6 +257,7 @@ static struct ti_thermal_data
 	data->sensor_id = id;
 	data->bgp = bgp;
 	data->mode = THERMAL_DEVICE_ENABLED;
+	data->pcb_tz = thermal_zone_get_zone_by_name("pcb");
 	INIT_WORK(&data->thermal_wq, ti_thermal_work);
 
 	return data;
-- 
1.7.7.1.488.ge8e1c


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

* RE: [PATCHv2 1/3] thermal: introduce thermal_zone_get_zone_by_name helper function
  2013-04-03 22:13 ` [PATCHv2 1/3] thermal: introduce thermal_zone_get_zone_by_name helper function Eduardo Valentin
@ 2013-04-04 17:12   ` R, Durgadoss
  2013-04-04 20:22     ` Eduardo Valentin
  0 siblings, 1 reply; 10+ messages in thread
From: R, Durgadoss @ 2013-04-04 17:12 UTC (permalink / raw)
  To: Eduardo Valentin, Zhang, Rui; +Cc: linux-pm, linux-kernel

> -----Original Message-----
> From: Eduardo Valentin [mailto:eduardo.valentin@ti.com]
> Sent: Thursday, April 04, 2013 3:43 AM
> To: Zhang, Rui
> Cc: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org; R, Durgadoss;
> Eduardo Valentin
> Subject: [PATCHv2 1/3] thermal: introduce
> thermal_zone_get_zone_by_name helper function
> 
> This patch adds a helper function to get a reference of
> a thermal zone, based on the zone type name.
> 
> It will perform a zone name lookup and return a reference
> to a thermal zone device that matches the name requested.
> In case the zone is not found or when several zones match
> same name or if the required parameters are invalid, it will return
> the corresponding error code (ERR_PTR).
> 
> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> ---
>  drivers/thermal/thermal_sys.c |   34
> ++++++++++++++++++++++++++++++++++
>  include/linux/thermal.h       |    1 +
>  2 files changed, 35 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index 5bd95d4..6e1da0a 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -1790,6 +1790,40 @@ void thermal_zone_device_unregister(struct
> thermal_zone_device *tz)
>  }
>  EXPORT_SYMBOL_GPL(thermal_zone_device_unregister);
> 
> +/**
> + * thermal_zone_get_zone_by_name() - search for a zone and returns its
> ref
> + * @name: thermal zone name to fetch the temperature
> + *
> + * When only one zone is found with the passed name, returns a reference
> to it.
> + *
> + * Return: On success returns a reference to an unique thermal zone with
> + * matching name equals to @name, a ERR_PTR otherwise.
> + */
> +struct thermal_zone_device *thermal_zone_get_zone_by_name(const
> char *name)
> +{
> +	struct thermal_zone_device *pos = NULL, *ref = ERR_PTR(-EINVAL);
> +	int found = 0;
> +
> +	if (!name)
> +		goto exit;
> +
> +	mutex_lock(&thermal_list_lock);
> +	list_for_each_entry(pos, &thermal_tz_list, node)
> +		if (!strnicmp(name, pos->type, THERMAL_NAME_LENGTH)) {
> +			found++;
> +			ref = pos;
> +		}
> +	mutex_unlock(&thermal_list_lock);
> +
> +	/* Success only when an unique zone is found */
> +	if (found != 1)
> +		ref = ERR_PTR(-ENODEV);

I think we should differentiate between the two cases:
1. The zone does not exist at all
	return NULL in this case
2. There are multiple zones
	return ERR_PTR(-EEXIST) in this case

This way the calling function can figure out the exact reason
for failure.

Thanks,
Durga

> +
> +exit:
> +	return ref;
> +}
> +EXPORT_SYMBOL_GPL(thermal_zone_get_zone_by_name);
> +
>  #ifdef CONFIG_NET
>  static struct genl_family thermal_event_genl_family = {
>  	.id = GENL_ID_GENERATE,
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 542a39c..0cf9eb5 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -237,6 +237,7 @@ void thermal_zone_device_update(struct
> thermal_zone_device *);
>  struct thermal_cooling_device *thermal_cooling_device_register(char *,
> void *,
>  		const struct thermal_cooling_device_ops *);
>  void thermal_cooling_device_unregister(struct thermal_cooling_device *);
> +struct thermal_zone_device *thermal_zone_get_zone_by_name(const
> char *name);
> 
>  int thermal_zone_trend_get(struct thermal_zone_device *, int);
>  struct thermal_instance *thermal_instance_get(struct
> thermal_zone_device *,
> --
> 1.7.7.1.488.ge8e1c


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

* RE: [PATCHv2 2/3] thermal: expose thermal_zone_get_temp API
  2013-04-03 22:13 ` [PATCHv2 2/3] thermal: expose thermal_zone_get_temp API Eduardo Valentin
@ 2013-04-04 17:13   ` R, Durgadoss
  0 siblings, 0 replies; 10+ messages in thread
From: R, Durgadoss @ 2013-04-04 17:13 UTC (permalink / raw)
  To: Eduardo Valentin, Zhang, Rui; +Cc: linux-pm, linux-kernel

> -----Original Message-----
> From: linux-pm-owner@vger.kernel.org [mailto:linux-pm-
> owner@vger.kernel.org] On Behalf Of Eduardo Valentin
> Sent: Thursday, April 04, 2013 3:43 AM
> To: Zhang, Rui
> Cc: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org; R, Durgadoss;
> Eduardo Valentin
> Subject: [PATCHv2 2/3] thermal: expose thermal_zone_get_temp API
> 
> This patch exports the thermal_zone_get_temp API so that driver
> writers can fetch temperature of thermal zones managed by other
> drivers.
> 
> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>

Looks fine,
Acked-By: Durgadoss R <durgadoss.r@intel.com>

> ---
>  drivers/thermal/thermal_sys.c |   20 +++++++++++++++++---
>  include/linux/thermal.h       |    1 +
>  2 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index 6e1da0a..e8afb7f 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -371,16 +371,28 @@ static void handle_thermal_trip(struct
> thermal_zone_device *tz, int trip)
>  	monitor_thermal_zone(tz);
>  }
> 
> -static int thermal_zone_get_temp(struct thermal_zone_device *tz,
> -				unsigned long *temp)
> +/**
> + * thermal_zone_get_temp() - returns its the temperature of thermal zone
> + * @tz: a valid pointer to a struct thermal_zone_device
> + * @temp: a valid pointer to where to store the resulting temperature.
> + *
> + * When a valid thermal zone reference is passed, it will fetch its
> + * temperature and fill @temp.
> + *
> + * Return: On success returns 0, an error code otherwise
> + */
> +int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned
> long *temp)
>  {
> -	int ret = 0;
> +	int ret = -EINVAL;
>  #ifdef CONFIG_THERMAL_EMULATION
>  	int count;
>  	unsigned long crit_temp = -1UL;
>  	enum thermal_trip_type type;
>  #endif
> 
> +	if (IS_ERR_OR_NULL(tz))
> +		goto exit;
> +
>  	mutex_lock(&tz->lock);
> 
>  	ret = tz->ops->get_temp(tz, temp);
> @@ -404,8 +416,10 @@ static int thermal_zone_get_temp(struct
> thermal_zone_device *tz,
>  skip_emul:
>  #endif
>  	mutex_unlock(&tz->lock);
> +exit:
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(thermal_zone_get_temp);
> 
>  static void update_temperature(struct thermal_zone_device *tz)
>  {
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 0cf9eb5..8eea86c 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -238,6 +238,7 @@ struct thermal_cooling_device
> *thermal_cooling_device_register(char *, void *,
>  		const struct thermal_cooling_device_ops *);
>  void thermal_cooling_device_unregister(struct thermal_cooling_device *);
>  struct thermal_zone_device *thermal_zone_get_zone_by_name(const
> char *name);
> +int thermal_zone_get_temp(struct thermal_zone_device *tz, unsigned
> long *temp);
> 
>  int thermal_zone_trend_get(struct thermal_zone_device *, int);
>  struct thermal_instance *thermal_instance_get(struct
> thermal_zone_device *,
> --
> 1.7.7.1.488.ge8e1c
> 
> --
> 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

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

* Re: [PATCHv2 1/3] thermal: introduce thermal_zone_get_zone_by_name helper function
  2013-04-04 17:12   ` R, Durgadoss
@ 2013-04-04 20:22     ` Eduardo Valentin
  2013-04-05  4:57       ` R, Durgadoss
  0 siblings, 1 reply; 10+ messages in thread
From: Eduardo Valentin @ 2013-04-04 20:22 UTC (permalink / raw)
  To: R, Durgadoss; +Cc: Eduardo Valentin, Zhang, Rui, linux-pm, linux-kernel

On 04-04-2013 13:12, R, Durgadoss wrote:
>> -----Original Message-----
>> From: Eduardo Valentin [mailto:eduardo.valentin@ti.com]
>> Sent: Thursday, April 04, 2013 3:43 AM
>> To: Zhang, Rui
>> Cc: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org; R, Durgadoss;
>> Eduardo Valentin
>> Subject: [PATCHv2 1/3] thermal: introduce
>> thermal_zone_get_zone_by_name helper function
>>
>> This patch adds a helper function to get a reference of
>> a thermal zone, based on the zone type name.
>>
>> It will perform a zone name lookup and return a reference
>> to a thermal zone device that matches the name requested.
>> In case the zone is not found or when several zones match
>> same name or if the required parameters are invalid, it will return
>> the corresponding error code (ERR_PTR).
>>
>> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
>> ---
>>   drivers/thermal/thermal_sys.c |   34
>> ++++++++++++++++++++++++++++++++++
>>   include/linux/thermal.h       |    1 +
>>   2 files changed, 35 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
>> index 5bd95d4..6e1da0a 100644
>> --- a/drivers/thermal/thermal_sys.c
>> +++ b/drivers/thermal/thermal_sys.c
>> @@ -1790,6 +1790,40 @@ void thermal_zone_device_unregister(struct
>> thermal_zone_device *tz)
>>   }
>>   EXPORT_SYMBOL_GPL(thermal_zone_device_unregister);
>>
>> +/**
>> + * thermal_zone_get_zone_by_name() - search for a zone and returns its
>> ref
>> + * @name: thermal zone name to fetch the temperature
>> + *
>> + * When only one zone is found with the passed name, returns a reference
>> to it.
>> + *
>> + * Return: On success returns a reference to an unique thermal zone with
>> + * matching name equals to @name, a ERR_PTR otherwise.
>> + */
>> +struct thermal_zone_device *thermal_zone_get_zone_by_name(const
>> char *name)
>> +{
>> +	struct thermal_zone_device *pos = NULL, *ref = ERR_PTR(-EINVAL);
>> +	int found = 0;
>> +
>> +	if (!name)
>> +		goto exit;
>> +
>> +	mutex_lock(&thermal_list_lock);
>> +	list_for_each_entry(pos, &thermal_tz_list, node)
>> +		if (!strnicmp(name, pos->type, THERMAL_NAME_LENGTH)) {
>> +			found++;
>> +			ref = pos;
>> +		}
>> +	mutex_unlock(&thermal_list_lock);
>> +
>> +	/* Success only when an unique zone is found */
>> +	if (found != 1)
>> +		ref = ERR_PTR(-ENODEV);
>
> I think we should differentiate between the two cases:
> 1. The zone does not exist at all
> 	return NULL in this case
> 2. There are multiple zones
> 	return ERR_PTR(-EEXIST) in this case
>
> This way the calling function can figure out the exact reason
> for failure.

I think the code documentation is already clear enough to say that this 
is for unique matches. But in case you want to differentiate these 
cases, I can resend it. Do you have a usage for this?

Besides, I would prefer to return ERR_PTR(-ENODEV) in the first case. 
The way it is in the original patch.

>
> Thanks,
> Durga
>
>> +
>> +exit:
>> +	return ref;
>> +}
>> +EXPORT_SYMBOL_GPL(thermal_zone_get_zone_by_name);
>> +
>>   #ifdef CONFIG_NET
>>   static struct genl_family thermal_event_genl_family = {
>>   	.id = GENL_ID_GENERATE,
>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> index 542a39c..0cf9eb5 100644
>> --- a/include/linux/thermal.h
>> +++ b/include/linux/thermal.h
>> @@ -237,6 +237,7 @@ void thermal_zone_device_update(struct
>> thermal_zone_device *);
>>   struct thermal_cooling_device *thermal_cooling_device_register(char *,
>> void *,
>>   		const struct thermal_cooling_device_ops *);
>>   void thermal_cooling_device_unregister(struct thermal_cooling_device *);
>> +struct thermal_zone_device *thermal_zone_get_zone_by_name(const
>> char *name);
>>
>>   int thermal_zone_trend_get(struct thermal_zone_device *, int);
>>   struct thermal_instance *thermal_instance_get(struct
>> thermal_zone_device *,
>> --
>> 1.7.7.1.488.ge8e1c
>
>
>


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

* RE: [PATCHv2 1/3] thermal: introduce thermal_zone_get_zone_by_name helper function
  2013-04-04 20:22     ` Eduardo Valentin
@ 2013-04-05  4:57       ` R, Durgadoss
  0 siblings, 0 replies; 10+ messages in thread
From: R, Durgadoss @ 2013-04-05  4:57 UTC (permalink / raw)
  To: Eduardo Valentin; +Cc: Zhang, Rui, linux-pm, linux-kernel

> -----Original Message-----
> From: Eduardo Valentin [mailto:eduardo.valentin@ti.com]
> Sent: Friday, April 05, 2013 1:52 AM
> To: R, Durgadoss
> Cc: Eduardo Valentin; Zhang, Rui; linux-pm@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCHv2 1/3] thermal: introduce
> thermal_zone_get_zone_by_name helper function
> 
> On 04-04-2013 13:12, R, Durgadoss wrote:
> >> -----Original Message-----
> >> From: Eduardo Valentin [mailto:eduardo.valentin@ti.com]
> >> Sent: Thursday, April 04, 2013 3:43 AM
> >> To: Zhang, Rui
> >> Cc: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org; R,
> Durgadoss;
> >> Eduardo Valentin
> >> Subject: [PATCHv2 1/3] thermal: introduce
> >> thermal_zone_get_zone_by_name helper function
> >>
> >> This patch adds a helper function to get a reference of
> >> a thermal zone, based on the zone type name.
> >>
> >> It will perform a zone name lookup and return a reference
> >> to a thermal zone device that matches the name requested.
> >> In case the zone is not found or when several zones match
> >> same name or if the required parameters are invalid, it will return
> >> the corresponding error code (ERR_PTR).
> >>
> >> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> >> ---
> >>   drivers/thermal/thermal_sys.c |   34
> >> ++++++++++++++++++++++++++++++++++
> >>   include/linux/thermal.h       |    1 +
> >>   2 files changed, 35 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/drivers/thermal/thermal_sys.c
> b/drivers/thermal/thermal_sys.c
> >> index 5bd95d4..6e1da0a 100644
> >> --- a/drivers/thermal/thermal_sys.c
> >> +++ b/drivers/thermal/thermal_sys.c
> >> @@ -1790,6 +1790,40 @@ void thermal_zone_device_unregister(struct
> >> thermal_zone_device *tz)
> >>   }
> >>   EXPORT_SYMBOL_GPL(thermal_zone_device_unregister);
> >>
> >> +/**
> >> + * thermal_zone_get_zone_by_name() - search for a zone and returns
> its
> >> ref
> >> + * @name: thermal zone name to fetch the temperature
> >> + *
> >> + * When only one zone is found with the passed name, returns a
> reference
> >> to it.
> >> + *
> >> + * Return: On success returns a reference to an unique thermal zone
> with
> >> + * matching name equals to @name, a ERR_PTR otherwise.
> >> + */
> >> +struct thermal_zone_device *thermal_zone_get_zone_by_name(const
> >> char *name)
> >> +{
> >> +	struct thermal_zone_device *pos = NULL, *ref = ERR_PTR(-EINVAL);
> >> +	int found = 0;
> >> +
> >> +	if (!name)
> >> +		goto exit;
> >> +
> >> +	mutex_lock(&thermal_list_lock);
> >> +	list_for_each_entry(pos, &thermal_tz_list, node)
> >> +		if (!strnicmp(name, pos->type, THERMAL_NAME_LENGTH)) {
> >> +			found++;
> >> +			ref = pos;
> >> +		}
> >> +	mutex_unlock(&thermal_list_lock);
> >> +
> >> +	/* Success only when an unique zone is found */
> >> +	if (found != 1)
> >> +		ref = ERR_PTR(-ENODEV);
> >
> > I think we should differentiate between the two cases:
> > 1. The zone does not exist at all
> > 	return NULL in this case
> > 2. There are multiple zones
> > 	return ERR_PTR(-EEXIST) in this case
> >
> > This way the calling function can figure out the exact reason
> > for failure.
> 
> I think the code documentation is already clear enough to say that this
> is for unique matches. But in case you want to differentiate these
> cases, I can resend it. Do you have a usage for this?

Yes, the calling function may want to re-try (or even unregister a zone,
to proceed further..)

> 
> Besides, I would prefer to return ERR_PTR(-ENODEV) in the first case.
> The way it is in the original patch.

Okay, I am fine with that too :-)

> 
> >
> > Thanks,
> > Durga
> >
> >> +
> >> +exit:
> >> +	return ref;
> >> +}
> >> +EXPORT_SYMBOL_GPL(thermal_zone_get_zone_by_name);
> >> +
> >>   #ifdef CONFIG_NET
> >>   static struct genl_family thermal_event_genl_family = {
> >>   	.id = GENL_ID_GENERATE,
> >> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> >> index 542a39c..0cf9eb5 100644
> >> --- a/include/linux/thermal.h
> >> +++ b/include/linux/thermal.h
> >> @@ -237,6 +237,7 @@ void thermal_zone_device_update(struct
> >> thermal_zone_device *);
> >>   struct thermal_cooling_device *thermal_cooling_device_register(char *,
> >> void *,
> >>   		const struct thermal_cooling_device_ops *);
> >>   void thermal_cooling_device_unregister(struct thermal_cooling_device
> *);
> >> +struct thermal_zone_device *thermal_zone_get_zone_by_name(const
> >> char *name);
> >>
> >>   int thermal_zone_trend_get(struct thermal_zone_device *, int);
> >>   struct thermal_instance *thermal_instance_get(struct
> >> thermal_zone_device *,
> >> --
> >> 1.7.7.1.488.ge8e1c
> >
> >
> >


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

* Re: [PATCHv2 0/3] thermal: lookup temperature
  2013-04-03 22:13 [PATCHv2 0/3] thermal: lookup temperature Eduardo Valentin
                   ` (2 preceding siblings ...)
  2013-04-03 22:13 ` [PATCHv2 3/3] staging: ti-soc-thermal: remove external heat while extrapolating hotspot Eduardo Valentin
@ 2013-04-22 11:06 ` Pavel Machek
  2013-04-22 13:11   ` Eduardo Valentin
  3 siblings, 1 reply; 10+ messages in thread
From: Pavel Machek @ 2013-04-22 11:06 UTC (permalink / raw)
  To: Eduardo Valentin; +Cc: rui.zhang, linux-pm, linux-kernel, durgadoss.r

Hi!

> Here is V2 of temperature lookup helper function. This has been
> split into two API as suggested on V1.
> 
> The usage of it is exemplified on patch 03.
> 
> 
> Eduardo Valentin (3):
>   thermal: introduce thermal_zone_get_zone_by_name helper function

Is the function name maybe a bit long? Perhaps using tz_ prefix
instead of "thermal_zone_" would help? or thermal_get_by_name? tz_get_by_name?

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

* Re: [PATCHv2 0/3] thermal: lookup temperature
  2013-04-22 11:06 ` [PATCHv2 0/3] thermal: lookup temperature Pavel Machek
@ 2013-04-22 13:11   ` Eduardo Valentin
  0 siblings, 0 replies; 10+ messages in thread
From: Eduardo Valentin @ 2013-04-22 13:11 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Eduardo Valentin, rui.zhang, linux-pm, linux-kernel, durgadoss.r

Hello Pavel,

On 22-04-2013 07:06, Pavel Machek wrote:
> Hi!
>
>> Here is V2 of temperature lookup helper function. This has been
>> split into two API as suggested on V1.
>>
>> The usage of it is exemplified on patch 03.
>>
>>
>> Eduardo Valentin (3):
>>    thermal: introduce thermal_zone_get_zone_by_name helper function
>
> Is the function name maybe a bit long? Perhaps using tz_ prefix
> instead of "thermal_zone_" would help? or thermal_get_by_name? tz_get_by_name?
>
> 									Pavel
>
This patch follows the naming convention on this file. If we are  going 
to use different prefixes, then I suggest sending a patch that changes 
the functions that are exported by this file. Besides the names you 
suggested do not reflect exactly the purpose of the introduced API.

Thanks for your suggestion.

All best,

Thanks.

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

end of thread, other threads:[~2013-04-22 13:11 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-03 22:13 [PATCHv2 0/3] thermal: lookup temperature Eduardo Valentin
2013-04-03 22:13 ` [PATCHv2 1/3] thermal: introduce thermal_zone_get_zone_by_name helper function Eduardo Valentin
2013-04-04 17:12   ` R, Durgadoss
2013-04-04 20:22     ` Eduardo Valentin
2013-04-05  4:57       ` R, Durgadoss
2013-04-03 22:13 ` [PATCHv2 2/3] thermal: expose thermal_zone_get_temp API Eduardo Valentin
2013-04-04 17:13   ` R, Durgadoss
2013-04-03 22:13 ` [PATCHv2 3/3] staging: ti-soc-thermal: remove external heat while extrapolating hotspot Eduardo Valentin
2013-04-22 11:06 ` [PATCHv2 0/3] thermal: lookup temperature Pavel Machek
2013-04-22 13:11   ` Eduardo Valentin

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