2013-05-18 09:51:08

by Jonghwa Lee

[permalink] [raw]
Subject: [PATCH 2/3] Thermal: core: Modify temp_crit_show() to use proper callback function.

This patch modifies temp_crit_show() which is used to create hwmon's sysfs
node to use .get_crit_temp callback function of thermal zone device rather
than .get_trip_temp.

Signed-off-by: Jonghwa Lee <[email protected]>
Signed-off-by: MyungJoo Ham <[email protected]>
---
drivers/thermal/thermal_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index f753f48..ce4384a 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -924,7 +924,7 @@ temp_crit_show(struct device *dev, struct device_attribute *attr,
long temperature;
int ret;

- ret = tz->ops->get_trip_temp(tz, 0, &temperature);
+ ret = tz->ops->get_crit_temp(tz, &temperature);
if (ret)
return ret;

--
1.7.9.5


2013-05-20 15:58:11

by Zhang, Rui

[permalink] [raw]
Subject: RE: [PATCH 2/3] Thermal: core: Modify temp_crit_show() to use proper callback function.



> -----Original Message-----
> From: Jonghwa Lee [mailto:[email protected]]
> Sent: Saturday, May 18, 2013 5:51 PM
> To: [email protected]
> Cc: [email protected]; Zhang, Rui; Eduardo Valentin; Amit
> Dinel Kachhap; Jonghwa Lee; MyungJoo Ham
> Subject: [PATCH 2/3] Thermal: core: Modify temp_crit_show() to use
> proper callback function.
> Importance: High
>
> This patch modifies temp_crit_show() which is used to create hwmon's
> sysfs node to use .get_crit_temp callback function of thermal zone
> device rather than .get_trip_temp.
>
> Signed-off-by: Jonghwa Lee <[email protected]>
> Signed-off-by: MyungJoo Ham <[email protected]>

The problem is that .get_crit_temp is optional for thermal drivers
that has a critical trip point.
At least we do not have such kind of check in thermal core.
Take rcar_thermal driver for example,
It supports critical trip point but it does not have .get_crit_type.

So I'd like to see a fix in rcar thermal driver and thermal core
for this issue, together with this patch.

Thanks,
rui
> ---
> drivers/thermal/thermal_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/thermal_core.c
> b/drivers/thermal/thermal_core.c index f753f48..ce4384a 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -924,7 +924,7 @@ temp_crit_show(struct device *dev, struct
> device_attribute *attr,
> long temperature;
> int ret;
>
> - ret = tz->ops->get_trip_temp(tz, 0, &temperature);
> + ret = tz->ops->get_crit_temp(tz, &temperature);
> if (ret)
> return ret;
>
> --
> 1.7.9.5

2013-05-21 01:32:01

by Jonghwa Lee

[permalink] [raw]
Subject: Re: [PATCH 2/3] Thermal: core: Modify temp_crit_show() to use proper callback function.

On 2013년 05월 21일 00:57, Zhang, Rui wrote:

>
>
>> -----Original Message-----
>> From: Jonghwa Lee [mailto:[email protected]]
>> Sent: Saturday, May 18, 2013 5:51 PM
>> To: [email protected]
>> Cc: [email protected]; Zhang, Rui; Eduardo Valentin; Amit
>> Dinel Kachhap; Jonghwa Lee; MyungJoo Ham
>> Subject: [PATCH 2/3] Thermal: core: Modify temp_crit_show() to use
>> proper callback function.
>> Importance: High
>>
>> This patch modifies temp_crit_show() which is used to create hwmon's
>> sysfs node to use .get_crit_temp callback function of thermal zone
>> device rather than .get_trip_temp.
>>
>> Signed-off-by: Jonghwa Lee <[email protected]>
>> Signed-off-by: MyungJoo Ham <[email protected]>
>
> The problem is that .get_crit_temp is optional for thermal drivers
> that has a critical trip point.


Actually, this is a interface for hwmon not generic thermal framework. And when
we create this node through the thermal_add_hwmon_sysfs(), we've already checked
whether call back function is available. So the problem won't happen.

Thanks
Jonghwa

> At least we do not have such kind of check in thermal core.
> Take rcar_thermal driver for example,
> It supports critical trip point but it does not have .get_crit_type.
>
> So I'd like to see a fix in rcar thermal driver and thermal core
> for this issue, together with this patch.
>
> Thanks,
> rui
>> ---
>> drivers/thermal/thermal_core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/thermal/thermal_core.c
>> b/drivers/thermal/thermal_core.c index f753f48..ce4384a 100644
>> --- a/drivers/thermal/thermal_core.c
>> +++ b/drivers/thermal/thermal_core.c
>> @@ -924,7 +924,7 @@ temp_crit_show(struct device *dev, struct
>> device_attribute *attr,
>> long temperature;
>> int ret;
>>
>> - ret = tz->ops->get_trip_temp(tz, 0, &temperature);
>> + ret = tz->ops->get_crit_temp(tz, &temperature);
>> if (ret)
>> return ret;
>>
>> --
>> 1.7.9.5
>
>

2013-05-23 02:19:49

by Zhang, Rui

[permalink] [raw]
Subject: Re: [PATCH 2/3] Thermal: core: Modify temp_crit_show() to use proper callback function.

On Tue, 2013-05-21 at 10:31 +0900, [email protected] wrote:
> On 2013년 05월 21일 00:57, Zhang, Rui wrote:
>
> >
> >
> >> -----Original Message-----
> >> From: Jonghwa Lee [mailto:[email protected]]
> >> Sent: Saturday, May 18, 2013 5:51 PM
> >> To: [email protected]
> >> Cc: [email protected]; Zhang, Rui; Eduardo Valentin; Amit
> >> Dinel Kachhap; Jonghwa Lee; MyungJoo Ham
> >> Subject: [PATCH 2/3] Thermal: core: Modify temp_crit_show() to use
> >> proper callback function.
> >> Importance: High
> >>
> >> This patch modifies temp_crit_show() which is used to create hwmon's
> >> sysfs node to use .get_crit_temp callback function of thermal zone
> >> device rather than .get_trip_temp.
> >>
> >> Signed-off-by: Jonghwa Lee <[email protected]>
> >> Signed-off-by: MyungJoo Ham <[email protected]>
> >
> > The problem is that .get_crit_temp is optional for thermal drivers
> > that has a critical trip point.
>
>
> Actually, this is a interface for hwmon not generic thermal framework. And when
> we create this node through the thermal_add_hwmon_sysfs(), we've already checked
> whether call back function is available. So the problem won't happen.
>
Oh, I see.

But back to my question, still take rcar driver for example,
it has a critical trip point but this is not shown in hwmon, which IMO
is still a problem.
So we should either make .get_crit_temp() mandatory for thermal drivers
with critical trip point or remove this callback and register hwmon
crit_temp interface via checking .get_trip_type().

thanks,
rui

> Thanks
> Jonghwa
>
> > At least we do not have such kind of check in thermal core.
> > Take rcar_thermal driver for example,
> > It supports critical trip point but it does not have .get_crit_type.
> >
> > So I'd like to see a fix in rcar thermal driver and thermal core
> > for this issue, together with this patch.
> >
> > Thanks,
> > rui
> >> ---
> >> drivers/thermal/thermal_core.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/thermal/thermal_core.c
> >> b/drivers/thermal/thermal_core.c index f753f48..ce4384a 100644
> >> --- a/drivers/thermal/thermal_core.c
> >> +++ b/drivers/thermal/thermal_core.c
> >> @@ -924,7 +924,7 @@ temp_crit_show(struct device *dev, struct
> >> device_attribute *attr,
> >> long temperature;
> >> int ret;
> >>
> >> - ret = tz->ops->get_trip_temp(tz, 0, &temperature);
> >> + ret = tz->ops->get_crit_temp(tz, &temperature);
> >> if (ret)
> >> return ret;
> >>
> >> --
> >> 1.7.9.5
> >
> >
>
>