| Message ID | 1546510369-28491-4-git-send-email-wni@nvidia.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | Fixes for Tegra soctherm | expand |
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) >
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) >> > >
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 --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)
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(+)