2013-04-03 22:13:46

by Eduardo Valentin

[permalink] [raw]
Subject: [PATCHv2 0/3] thermal: lookup temperature

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


2013-04-03 22:14:09

by Eduardo Valentin

[permalink] [raw]
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 <[email protected]>
---
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

2013-04-03 22:14:32

by Eduardo Valentin

[permalink] [raw]
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 <[email protected]>
---
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

2013-04-03 22:14:56

by Eduardo Valentin

[permalink] [raw]
Subject: [PATCHv2 3/3] staging: ti-soc-thermal: remove external heat while extrapolating hotspot

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 <[email protected]>
---
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

2013-04-04 17:12:34

by R, Durgadoss

[permalink] [raw]
Subject: RE: [PATCHv2 1/3] thermal: introduce thermal_zone_get_zone_by_name helper function

> -----Original Message-----
> From: Eduardo Valentin [mailto:[email protected]]
> Sent: Thursday, April 04, 2013 3:43 AM
> To: Zhang, Rui
> Cc: [email protected]; [email protected]; 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 <[email protected]>
> ---
> 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

2013-04-04 17:14:05

by R, Durgadoss

[permalink] [raw]
Subject: RE: [PATCHv2 2/3] thermal: expose thermal_zone_get_temp API

> -----Original Message-----
> From: [email protected] [mailto:linux-pm-
> [email protected]] On Behalf Of Eduardo Valentin
> Sent: Thursday, April 04, 2013 3:43 AM
> To: Zhang, Rui
> Cc: [email protected]; [email protected]; 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 <[email protected]>

Looks fine,
Acked-By: Durgadoss R <[email protected]>

> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2013-04-04 20:22:32

by Eduardo Valentin

[permalink] [raw]
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:[email protected]]
>> Sent: Thursday, April 04, 2013 3:43 AM
>> To: Zhang, Rui
>> Cc: [email protected]; [email protected]; 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 <[email protected]>
>> ---
>> 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
>
>
>

2013-04-05 04:57:57

by R, Durgadoss

[permalink] [raw]
Subject: RE: [PATCHv2 1/3] thermal: introduce thermal_zone_get_zone_by_name helper function

> -----Original Message-----
> From: Eduardo Valentin [mailto:[email protected]]
> Sent: Friday, April 05, 2013 1:52 AM
> To: R, Durgadoss
> Cc: Eduardo Valentin; Zhang, Rui; [email protected]; linux-
> [email protected]
> 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:[email protected]]
> >> Sent: Thursday, April 04, 2013 3:43 AM
> >> To: Zhang, Rui
> >> Cc: [email protected]; [email protected]; 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 <[email protected]>
> >> ---
> >> 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
> >
> >
> >

2013-04-22 11:06:51

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCHv2 0/3] thermal: lookup temperature

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

2013-04-22 13:11:28

by Eduardo Valentin

[permalink] [raw]
Subject: Re: [PATCHv2 0/3] thermal: lookup temperature

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.