diff mbox series

[v7,3/3] thermal: tegra: add get_trend ops

Message ID 1546510369-28491-4-git-send-email-wni@nvidia.com (mailing list archive)
State Not Applicable, archived
Delegated to: Zhang Rui
Headers show
Series Fixes for Tegra soctherm | expand

Commit Message

Wei Ni Jan. 3, 2019, 10:12 a.m. UTC
Add support for get_trend ops that allows soctherm
sensors to be used with the step-wise governor.

Signed-off-by: Wei Ni <wni@nvidia.com>
---
 drivers/thermal/tegra/soctherm.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

Comments

Daniel Lezcano Feb. 18, 2019, 10:27 a.m. UTC | #1
On 03/01/2019 11:12, Wei Ni wrote:
> Add support for get_trend ops that allows soctherm
> sensors to be used with the step-wise governor.
> 
> Signed-off-by: Wei Ni <wni@nvidia.com>
> ---
>  drivers/thermal/tegra/soctherm.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c
> index fd2703c0cfc5..864205af104b 100644
> --- a/drivers/thermal/tegra/soctherm.c
> +++ b/drivers/thermal/tegra/soctherm.c
> @@ -488,9 +488,43 @@ static int tegra_thermctl_set_trip_temp(void *data, int trip, int temp)
>  	return 0;
>  }
>  
> +static int tegra_thermctl_get_trend(void *data, int trip,
> +				    enum thermal_trend *trend)
> +{
> +	struct tegra_thermctl_zone *zone = data;
> +	struct thermal_zone_device *tz = zone->tz;
> +	int trip_temp, temp, last_temp, ret;
> +
> +	if (!tz)
> +		return -EINVAL;
> +
> +	ret = tz->ops->get_trip_temp(zone->tz, trip, &trip_temp);
> +	if (ret)
> +		return ret;
> +
> +	mutex_lock(&tz->lock);

No need to use the mutex here.

Why not ?

temp = READ_ONCE(tz->temperature);
last_temp = READ_ONCE(tz->last_temperature);

> +	temp = tz->temperature;
> +	last_temp = tz->last_temperature;
> +	mutex_unlock(&tz->lock);
> +
> +	if (temp > trip_temp) {
> +		if (temp >= last_temp)
> +			*trend = THERMAL_TREND_RAISING;
> +		else
> +			*trend = THERMAL_TREND_STABLE;
> +	} else if (temp < trip_temp) {
> +		*trend = THERMAL_TREND_DROPPING;
> +	} else {
> +		*trend = THERMAL_TREND_STABLE;
> +	}
> +
> +	return 0;
> +}
> +
>  static const struct thermal_zone_of_device_ops tegra_of_thermal_ops = {
>  	.get_temp = tegra_thermctl_get_temp,
>  	.set_trip_temp = tegra_thermctl_set_trip_temp,
> +	.get_trend = tegra_thermctl_get_trend,
>  };
>  
>  static int get_hot_temp(struct thermal_zone_device *tz, int *trip, int *temp)
>
Wei Ni Feb. 19, 2019, 2:15 a.m. UTC | #2
On 18/2/2019 6:27 PM, Daniel Lezcano wrote:
> On 03/01/2019 11:12, Wei Ni wrote:
>> Add support for get_trend ops that allows soctherm
>> sensors to be used with the step-wise governor.
>>
>> Signed-off-by: Wei Ni <wni@nvidia.com>
>> ---
>>  drivers/thermal/tegra/soctherm.c | 34 ++++++++++++++++++++++++++++++++++
>>  1 file changed, 34 insertions(+)
>>
>> diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c
>> index fd2703c0cfc5..864205af104b 100644
>> --- a/drivers/thermal/tegra/soctherm.c
>> +++ b/drivers/thermal/tegra/soctherm.c
>> @@ -488,9 +488,43 @@ static int tegra_thermctl_set_trip_temp(void *data, int trip, int temp)
>>  	return 0;
>>  }
>>  
>> +static int tegra_thermctl_get_trend(void *data, int trip,
>> +				    enum thermal_trend *trend)
>> +{
>> +	struct tegra_thermctl_zone *zone = data;
>> +	struct thermal_zone_device *tz = zone->tz;
>> +	int trip_temp, temp, last_temp, ret;
>> +
>> +	if (!tz)
>> +		return -EINVAL;
>> +
>> +	ret = tz->ops->get_trip_temp(zone->tz, trip, &trip_temp);
>> +	if (ret)
>> +		return ret;
>> +
>> +	mutex_lock(&tz->lock);
> 
> No need to use the mutex here.
> 
> Why not ?
> 
> temp = READ_ONCE(tz->temperature);
> last_temp = READ_ONCE(tz->last_temperature);

Yes, you are right, will change it in next version.

Wei.

> 
>> +	temp = tz->temperature;
>> +	last_temp = tz->last_temperature;
>> +	mutex_unlock(&tz->lock);
>> +
>> +	if (temp > trip_temp) {
>> +		if (temp >= last_temp)
>> +			*trend = THERMAL_TREND_RAISING;
>> +		else
>> +			*trend = THERMAL_TREND_STABLE;
>> +	} else if (temp < trip_temp) {
>> +		*trend = THERMAL_TREND_DROPPING;
>> +	} else {
>> +		*trend = THERMAL_TREND_STABLE;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static const struct thermal_zone_of_device_ops tegra_of_thermal_ops = {
>>  	.get_temp = tegra_thermctl_get_temp,
>>  	.set_trip_temp = tegra_thermctl_set_trip_temp,
>> +	.get_trend = tegra_thermctl_get_trend,
>>  };
>>  
>>  static int get_hot_temp(struct thermal_zone_device *tz, int *trip, int *temp)
>>
> 
>
Daniel Lezcano March 30, 2020, 5:23 p.m. UTC | #3
Hi Wei,

On 19/02/2019 03:15, Wei Ni wrote:

>>> +static int tegra_thermctl_get_trend(void *data, int trip, +
>>> enum thermal_trend *trend)

[ ... ]

>>> +	temp = tz->temperature; +	last_temp = tz->last_temperature; +
>>> mutex_unlock(&tz->lock); + +	if (temp > trip_temp) { +		if
>>> (temp >= last_temp) +			*trend = THERMAL_TREND_RAISING; +
>>> else +			*trend = THERMAL_TREND_STABLE; +	} else if (temp <
>>> trip_temp) { +		*trend = THERMAL_TREND_DROPPING; +	} else { +
>>> *trend = THERMAL_TREND_STABLE; +	} + +	return 0; +} + static
>>> const struct thermal_zone_of_device_ops tegra_of_thermal_ops =
>>> { .get_temp = tegra_thermctl_get_temp, .set_trip_temp =
>>> tegra_thermctl_set_trip_temp, +	.get_trend =
>>> tegra_thermctl_get_trend, };

It has been awhile since this patch was submitted and merged by
Eduardo. I replace him to co-maintain the thermal framework with Rui.

While figuring out the internals for code cleanup, I ended up in this
function above.

Why do you have to use this routine instead of the generic one in
get_tz_trend()?
diff mbox series

Patch

diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c
index fd2703c0cfc5..864205af104b 100644
--- a/drivers/thermal/tegra/soctherm.c
+++ b/drivers/thermal/tegra/soctherm.c
@@ -488,9 +488,43 @@  static int tegra_thermctl_set_trip_temp(void *data, int trip, int temp)
 	return 0;
 }
 
+static int tegra_thermctl_get_trend(void *data, int trip,
+				    enum thermal_trend *trend)
+{
+	struct tegra_thermctl_zone *zone = data;
+	struct thermal_zone_device *tz = zone->tz;
+	int trip_temp, temp, last_temp, ret;
+
+	if (!tz)
+		return -EINVAL;
+
+	ret = tz->ops->get_trip_temp(zone->tz, trip, &trip_temp);
+	if (ret)
+		return ret;
+
+	mutex_lock(&tz->lock);
+	temp = tz->temperature;
+	last_temp = tz->last_temperature;
+	mutex_unlock(&tz->lock);
+
+	if (temp > trip_temp) {
+		if (temp >= last_temp)
+			*trend = THERMAL_TREND_RAISING;
+		else
+			*trend = THERMAL_TREND_STABLE;
+	} else if (temp < trip_temp) {
+		*trend = THERMAL_TREND_DROPPING;
+	} else {
+		*trend = THERMAL_TREND_STABLE;
+	}
+
+	return 0;
+}
+
 static const struct thermal_zone_of_device_ops tegra_of_thermal_ops = {
 	.get_temp = tegra_thermctl_get_temp,
 	.set_trip_temp = tegra_thermctl_set_trip_temp,
+	.get_trend = tegra_thermctl_get_trend,
 };
 
 static int get_hot_temp(struct thermal_zone_device *tz, int *trip, int *temp)