thermal: tegra: add get_trend ops
Subsystems (2)
PATCH 1/1 thermal: tegra: add get_trend ops
Add support for get_trend ops that allows soctherm sensors to be used with the step-wise governor.
--- drivers/thermal/tegra/soctherm.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
drivers/thermal/tegra/soctherm.c +34 -0
@@ -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)
Signature · 1 line
2.7.4
Quoted 38 lines · Wei Ni
Add support for
get_trendops that allows soctherm sensors to be used with the step-wise governor.Signed-off-by Wei Ni <[email protected]>--- drivers/thermal/tegra/soctherm.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)drivers/thermal/tegra/soctherm.c +22
-0@@ -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 ;
drivers/thermal/tegra/soctherm.c static int tegra_thermctl_set_trip_temp(void *data, int trip, int temp) · line 522
Quoted 10 lines
drivers/thermal/tegra/soctherm.c +10
-0+else + *trend =THERMAL_TREND_STABLE ; + }else if (temp < trip_temp) { + *trend =THERMAL_TREND_DROPPING ; + }else { + *trend =THERMAL_TREND_STABLE ; + } + +return 0 ; +}
This looks like a reimplementation of the get_tz_trend() helper. Is seems like that helper already has everything we need. Perhaps this isn't working because of-thermal installs of_thermal_get_trend(), a function that returns -EINVAL if the driver doesn't implement the ->get_trend() callback.
Perhaps a better way would be to do something like this in thermal_zone_of_add_sensor():
if (ops->get_trend) tzd->ops->get_trend = of_thermal_get_trend;
That's similar to how ->set_trips() and ->set_emul_temp() are set up and should make sure that get_tz_trend() will do the right thing for all drivers that don't implement a special ->get_trend().
Thierry
Quoted 47 lines · Thierry Reding
Quoted 46 lines · Wei Ni
Add support for
get_trendops that allows soctherm sensors to be used with the step-wise governor.Signed-off-by Wei Ni <[email protected]>--- drivers/thermal/tegra/soctherm.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)drivers/thermal/tegra/soctherm.c +30
-0@@ -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 ; + } +
Quoted 8 lines
drivers/thermal/tegra/soctherm.c static int tegra_thermctl_set_trip_temp(void *data, int trip, int temp) · line 522
Quoted 2 lines
drivers/thermal/tegra/soctherm.c +2
-0+return 0 ; +}This looks like a reimplementation of the
get_tz_trend()helper. Is seems like that helper already has everything we need. Perhaps this isn't working because of-thermal installsof_thermal_get_trend(), a function that returns-EINVALif the driver doesn't implement the ->get_trend() callback.
1. The get_tz_trend() helper can work, because it has:
if (tz->emul_temperature || !tz->ops->get_trend ||
tz->ops->get_trend(tz, trip, &trend)) {...
}
the tz->ops->get_trend is of_thermal_get_trend(). If without special get_trend(), it will return -EINVAL, so it will implement the if block to get the "trend". If we have the special get_trend(), then the of_thermal_get_trend() will return 0, so this helper will not implement the if block, it will get the "trend" from the special get_trend().
2. There has a little difference between the helper and our special callback. The tegra_thermctl_get_trend() consider the trip_temp, but the get_tz_trend() helper didn't.
Quoted 10 lines
Perhaps a better way would be to do something like this in
thermal_zone_of_add_sensor():if (ops->get_trend) tzd->ops->get_trend = of_thermal_get_trend;That's similar to how ->set_trips() and ->set_emul_temp() are set up and should make sure that
get_tz_trend()will do the right thing for all drivers that don't implement a special ->get_trend().
As above description, I think the of_thermal_get_trend() already can handle this case, doesn't need to change.
Wei.
Quoted 3 lines
Thierry
Hi all,
Does there have any more comments for this patch?
Thanks.
Wei.
Quoted 93 lines · Wei Ni
Quoted 47 lines · Thierry Reding
Quoted 46 lines · Wei Ni
Add support for
get_trendops that allows soctherm sensors to be used with the step-wise governor.Signed-off-by Wei Ni <[email protected]>--- drivers/thermal/tegra/soctherm.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)drivers/thermal/tegra/soctherm.c +30
-0@@ -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 ; + } +Quoted 8 lines
drivers/thermal/tegra/soctherm.c static int tegra_thermctl_set_trip_temp(void *data, int trip, int temp) · line 522
Quoted 2 lines
drivers/thermal/tegra/soctherm.c +2
-0+return 0 ; +}This looks like a reimplementation of the
get_tz_trend()helper. Is seems like that helper already has everything we need. Perhaps this isn't working because of-thermal installsof_thermal_get_trend(), a function that returns-EINVALif the driver doesn't implement the ->get_trend() callback.1. The
get_tz_trend()helper can work, because it has:
if (tz->emul_temperature|| !tz->ops->get_trend ||tz->ops->get_trend(tz, trip, &trend)) {...
}
thetz->ops->get_trendisof_thermal_get_trend(). If without specialget_trend(), it will return-EINVAL, so it will implement the if block to get the "trend". If we have the specialget_trend(), then theof_thermal_get_trend()will return 0, so this helper will not implement the if block, it will get the "trend" from the specialget_trend().2. There has a little difference between the helper and our special callback. The
tegra_thermctl_get_trend()consider thetrip_temp, but theget_tz_trend()helper didn't.Quoted 10 lines
Perhaps a better way would be to do something like this in
thermal_zone_of_add_sensor():if (ops->get_trend) tzd->ops->get_trend = of_thermal_get_trend;That's similar to how ->set_trips() and ->set_emul_temp() are set up and should make sure that
get_tz_trend()will do the right thing for all drivers that don't implement a special ->get_trend().As above description, I think the
of_thermal_get_trend()already can handle this case, doesn't need to change.Wei.
Quoted 3 lines
Thierry
Quoted 61 lines · Wei Ni
Quoted 47 lines · Thierry Reding
Quoted 46 lines · Wei Ni
Add support for
get_trendops that allows soctherm sensors to be used with the step-wise governor.Signed-off-by Wei Ni <[email protected]>--- drivers/thermal/tegra/soctherm.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)drivers/thermal/tegra/soctherm.c +30
-0@@ -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 ; + } +Quoted 8 lines
drivers/thermal/tegra/soctherm.c static int tegra_thermctl_set_trip_temp(void *data, int trip, int temp) · line 522
Quoted 2 lines
drivers/thermal/tegra/soctherm.c +2
-0+return 0 ; +}This looks like a reimplementation of the
get_tz_trend()helper. Is seems like that helper already has everything we need. Perhaps this isn't working because of-thermal installsof_thermal_get_trend(), a function that returns-EINVALif the driver doesn't implement the ->get_trend() callback.1. The
get_tz_trend()helper can work, because it has:
if (tz->emul_temperature|| !tz->ops->get_trend ||
Quoted 8 lines
tz->ops->get_trend(tz, trip, &trend)) {...
}
thetz->ops->get_trendisof_thermal_get_trend(). If without specialget_trend(), it will return-EINVAL, so it will implement the if block to get the "trend". If we have the specialget_trend(), then theof_thermal_get_trend()will return 0, so this helper will not implement the if block, it will get the "trend" from the specialget_trend().
The idea of the helper is to provide a trend in case drivers dont have a specific way of doing so.
Quoted 5 lines
2. There has a little difference between the helper and our special callback. The
tegra_thermctl_get_trend()consider thetrip_temp, but theget_tz_trend()helper didn't.
Yeah, if you are computing trend towards a trip, then yes, that is different and this patch is needed.
Quoted 19 lines
Quoted 10 lines
Perhaps a better way would be to do something like this in
thermal_zone_of_add_sensor():if (ops->get_trend) tzd->ops->get_trend = of_thermal_get_trend;That's similar to how ->set_trips() and ->set_emul_temp() are set up and should make sure that
get_tz_trend()will do the right thing for all drivers that don't implement a special ->get_trend().As above description, I think the
of_thermal_get_trend()already can handle this case, doesn't need to change.Wei.
Quoted 3 lines
Thierry
Quoted 71 lines · Eduardo Valentin
Quoted 61 lines · Wei Ni
Quoted 47 lines · Thierry Reding
Quoted 46 lines · Wei Ni
Add support for
get_trendops that allows soctherm sensors to be used with the step-wise governor.Signed-off-by Wei Ni <[email protected]>--- drivers/thermal/tegra/soctherm.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)drivers/thermal/tegra/soctherm.c +30
-0@@ -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 ; + } +Quoted 8 lines
drivers/thermal/tegra/soctherm.c static int tegra_thermctl_set_trip_temp(void *data, int trip, int temp) · line 522
Quoted 2 lines
drivers/thermal/tegra/soctherm.c +2
-0+return 0 ; +}This looks like a reimplementation of the
get_tz_trend()helper. Is seems like that helper already has everything we need. Perhaps this isn't working because of-thermal installsof_thermal_get_trend(), a function that returns-EINVALif the driver doesn't implement the ->get_trend() callback.1. The
get_tz_trend()helper can work, because it has:
if (tz->emul_temperature|| !tz->ops->get_trend ||Quoted 8 lines
tz->ops->get_trend(tz, trip, &trend)) {...
}
thetz->ops->get_trendisof_thermal_get_trend(). If without specialget_trend(), it will return-EINVAL, so it will implement the if block to get the "trend". If we have the specialget_trend(), then theof_thermal_get_trend()will return 0, so this helper will not implement the if block, it will get the "trend" from the specialget_trend().
Quoted 2 lines
The idea of the helper is to provide a trend in case drivers dont have a specific way of doing so.
Yes, thanks for your explanation.
Quoted 29 lines
Quoted 6 lines
2. There has a little difference between the helper and our special callback. The
tegra_thermctl_get_trend()consider thetrip_temp, but theget_tz_trend()helper didn't.Yeah, if you are computing trend towards a trip, then yes, that is different and this patch is needed.
Quoted 19 lines
Quoted 10 lines
Perhaps a better way would be to do something like this in
thermal_zone_of_add_sensor():if (ops->get_trend) tzd->ops->get_trend = of_thermal_get_trend;That's similar to how ->set_trips() and ->set_emul_temp() are set up and should make sure that
get_tz_trend()will do the right thing for all drivers that don't implement a special ->get_trend().As above description, I think the
of_thermal_get_trend()already can handle this case, doesn't need to change.Wei.
Quoted 3 lines
Thierry
Hi Daniel,
It seems no more comments, could this patch be approved?
Thanks.
Wei.
Quoted 108 lines · Wei Ni
Quoted 71 lines · Eduardo Valentin
Quoted 61 lines · Wei Ni
Quoted 47 lines · Thierry Reding
Quoted 46 lines · Wei Ni
Add support for
get_trendops that allows soctherm sensors to be used with the step-wise governor.Signed-off-by Wei Ni <[email protected]>--- drivers/thermal/tegra/soctherm.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)drivers/thermal/tegra/soctherm.c +30
-0@@ -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 ; + } +Quoted 8 lines
drivers/thermal/tegra/soctherm.c static int tegra_thermctl_set_trip_temp(void *data, int trip, int temp) · line 522
Quoted 2 lines
drivers/thermal/tegra/soctherm.c +2
-0+return 0 ; +}This looks like a reimplementation of the
get_tz_trend()helper. Is seems like that helper already has everything we need. Perhaps this isn't working because of-thermal installsof_thermal_get_trend(), a function that returns-EINVALif the driver doesn't implement the ->get_trend() callback.1. The
get_tz_trend()helper can work, because it has:
if (tz->emul_temperature|| !tz->ops->get_trend ||Quoted 8 lines
tz->ops->get_trend(tz, trip, &trend)) {...
}
thetz->ops->get_trendisof_thermal_get_trend(). If without specialget_trend(), it will return-EINVAL, so it will implement the if block to get the "trend". If we have the specialget_trend(), then theof_thermal_get_trend()will return 0, so this helper will not implement the if block, it will get the "trend" from the specialget_trend().Quoted 2 lines
The idea of the helper is to provide a trend in case drivers dont have a specific way of doing so.
Yes, thanks for your explanation.
Quoted 29 lines
Quoted 6 lines
2. There has a little difference between the helper and our special callback. The
tegra_thermctl_get_trend()consider thetrip_temp, but theget_tz_trend()helper didn't.Yeah, if you are computing trend towards a trip, then yes, that is different and this patch is needed.
Quoted 19 lines
Quoted 10 lines
Perhaps a better way would be to do something like this in
thermal_zone_of_add_sensor():if (ops->get_trend) tzd->ops->get_trend = of_thermal_get_trend;That's similar to how ->set_trips() and ->set_emul_temp() are set up and should make sure that
get_tz_trend()will do the right thing for all drivers that don't implement a special ->get_trend().As above description, I think the
of_thermal_get_trend()already can handle this case, doesn't need to change.Wei.
Quoted 3 lines
Thierry
Hi Rui & Eduardo,
Could you please take this patch?
Thanks.
Wei.
Quoted 115 lines · Wei Ni
Hi Daniel,
It seems no more comments, could this patch be approved?Thanks.
Wei.Quoted 108 lines · Wei Ni
Quoted 71 lines · Eduardo Valentin
Quoted 61 lines · Wei Ni
Quoted 47 lines · Thierry Reding
Quoted 46 lines · Wei Ni
Add support for
get_trendops that allows soctherm sensors to be used with the step-wise governor.Signed-off-by Wei Ni <[email protected]>--- drivers/thermal/tegra/soctherm.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)drivers/thermal/tegra/soctherm.c +30
-0@@ -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 ; + } +Quoted 8 lines
drivers/thermal/tegra/soctherm.c static int tegra_thermctl_set_trip_temp(void *data, int trip, int temp) · line 522
Quoted 2 lines
drivers/thermal/tegra/soctherm.c +2
-0+return 0 ; +}This looks like a reimplementation of the
get_tz_trend()helper. Is seems like that helper already has everything we need. Perhaps this isn't working because of-thermal installsof_thermal_get_trend(), a function that returns-EINVALif the driver doesn't implement the ->get_trend() callback.1. The
get_tz_trend()helper can work, because it has:
if (tz->emul_temperature|| !tz->ops->get_trend ||Quoted 8 lines
tz->ops->get_trend(tz, trip, &trend)) {...
}
thetz->ops->get_trendisof_thermal_get_trend(). If without specialget_trend(), it will return-EINVAL, so it will implement the if block to get the "trend". If we have the specialget_trend(), then theof_thermal_get_trend()will return 0, so this helper will not implement the if block, it will get the "trend" from the specialget_trend().Quoted 2 lines
The idea of the helper is to provide a trend in case drivers dont have a specific way of doing so.
Yes, thanks for your explanation.
Quoted 29 lines
Quoted 6 lines
2. There has a little difference between the helper and our special callback. The
tegra_thermctl_get_trend()consider thetrip_temp, but theget_tz_trend()helper didn't.Yeah, if you are computing trend towards a trip, then yes, that is different and this patch is needed.
Quoted 19 lines
Quoted 10 lines
Perhaps a better way would be to do something like this in
thermal_zone_of_add_sensor():if (ops->get_trend) tzd->ops->get_trend = of_thermal_get_trend;That's similar to how ->set_trips() and ->set_emul_temp() are set up and should make sure that
get_tz_trend()will do the right thing for all drivers that don't implement a special ->get_trend().As above description, I think the
of_thermal_get_trend()already can handle this case, doesn't need to change.Wei.
Quoted 3 lines
Thierry