| Message ID | 20220616202537.303655-1-daniel.lezcano@linaro.org |
|---|---|
| State | Accepted |
| Commit | 198fa45252d84baea593456cfda01deeb9dff13f |
| Headers | show |
| Series | [1/3] thermal/drivers/qcom: Remove get_trend function | expand |
On Fri, Jun 17, 2022 at 1:56 AM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > There is a get_trend function which is a wrapper to call a private > get_trend function. However, this private get_trend function is not > assigned anywhere. > > Remove this dead code. > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> Acked-by: Amit Kucheria <amitk@kernel.org> > --- > drivers/thermal/qcom/tsens.c | 12 ------------ > drivers/thermal/qcom/tsens.h | 2 -- > 2 files changed, 14 deletions(-) > > diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c > index 7963ee33bf75..e49f58e83513 100644 > --- a/drivers/thermal/qcom/tsens.c > +++ b/drivers/thermal/qcom/tsens.c > @@ -933,17 +933,6 @@ static int tsens_get_temp(void *data, int *temp) > return priv->ops->get_temp(s, temp); > } > > -static int tsens_get_trend(void *data, int trip, enum thermal_trend *trend) > -{ > - struct tsens_sensor *s = data; > - struct tsens_priv *priv = s->priv; > - > - if (priv->ops->get_trend) > - return priv->ops->get_trend(s, trend); > - > - return -ENOTSUPP; > -} > - > static int __maybe_unused tsens_suspend(struct device *dev) > { > struct tsens_priv *priv = dev_get_drvdata(dev); > @@ -1004,7 +993,6 @@ MODULE_DEVICE_TABLE(of, tsens_table); > > static const struct thermal_zone_of_device_ops tsens_of_ops = { > .get_temp = tsens_get_temp, > - .get_trend = tsens_get_trend, > .set_trips = tsens_set_trips, > }; > > diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h > index 1471a2c00f15..ba05c8233356 100644 > --- a/drivers/thermal/qcom/tsens.h > +++ b/drivers/thermal/qcom/tsens.h > @@ -65,7 +65,6 @@ struct tsens_sensor { > * @disable: Function to disable the tsens device > * @suspend: Function to suspend the tsens device > * @resume: Function to resume the tsens device > - * @get_trend: Function to get the thermal/temp trend > */ > struct tsens_ops { > /* mandatory callbacks */ > @@ -77,7 +76,6 @@ struct tsens_ops { > void (*disable)(struct tsens_priv *priv); > int (*suspend)(struct tsens_priv *priv); > int (*resume)(struct tsens_priv *priv); > - int (*get_trend)(struct tsens_sensor *s, enum thermal_trend *trend); > }; > > #define REG_FIELD_FOR_EACH_SENSOR11(_name, _offset, _startbit, _stopbit) \ > -- > 2.25.1 >
16.06.2022 23:25, Daniel Lezcano пишет: > The get_trend function does already what the generic framework does. > > Remove it. > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > --- > drivers/thermal/tegra/soctherm.c | 32 -------------------------------- > 1 file changed, 32 deletions(-) > > diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c > index 210325f92559..825eab526619 100644 > --- a/drivers/thermal/tegra/soctherm.c > +++ b/drivers/thermal/tegra/soctherm.c > @@ -633,37 +633,6 @@ 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; > - > - temp = READ_ONCE(tz->temperature); > - last_temp = READ_ONCE(tz->last_temperature); > - > - 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 void thermal_irq_enable(struct tegra_thermctl_zone *zn) > { > u32 r; > @@ -716,7 +685,6 @@ static int tegra_thermctl_set_trips(void *data, int lo, int hi) > 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, > .set_trips = tegra_thermctl_set_trips, > }; > The framework doesn't use the trip temperature, is it really the same? Previously, if temperature was above the trip and was dropping, then it was THERMAL_TREND_STABLE instead of THERMAL_TREND_DROPPING.
On 18/06/2022 14:44, Dmitry Osipenko wrote: > 16.06.2022 23:25, Daniel Lezcano пишет: >> The get_trend function does already what the generic framework does. >> >> Remove it. >> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> >> --- [ ... ] >> static void thermal_irq_enable(struct tegra_thermctl_zone *zn) >> { >> u32 r; >> @@ -716,7 +685,6 @@ static int tegra_thermctl_set_trips(void *data, int lo, int hi) >> 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, >> .set_trips = tegra_thermctl_set_trips, >> }; >> > > The framework doesn't use the trip temperature, is it really the same? > Previously, if temperature was above the trip and was dropping, then it > was THERMAL_TREND_STABLE instead of THERMAL_TREND_DROPPING. Actually, the only difference is the temp > trip and the temperature < last_temperature. It results in the STABLE trend and the governor does nothing. With the core trend function for the same inputs, temperature < last_temperature results in a DROPPING but as temp > trip the 'throttle' boolean is true in the governor, and get_next_state() with DROPPING + throttle=true, results in nothing because the action happens when throttle=false. All the combinations result at end at the same action from the governor.
diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c index 7963ee33bf75..e49f58e83513 100644 --- a/drivers/thermal/qcom/tsens.c +++ b/drivers/thermal/qcom/tsens.c @@ -933,17 +933,6 @@ static int tsens_get_temp(void *data, int *temp) return priv->ops->get_temp(s, temp); } -static int tsens_get_trend(void *data, int trip, enum thermal_trend *trend) -{ - struct tsens_sensor *s = data; - struct tsens_priv *priv = s->priv; - - if (priv->ops->get_trend) - return priv->ops->get_trend(s, trend); - - return -ENOTSUPP; -} - static int __maybe_unused tsens_suspend(struct device *dev) { struct tsens_priv *priv = dev_get_drvdata(dev); @@ -1004,7 +993,6 @@ MODULE_DEVICE_TABLE(of, tsens_table); static const struct thermal_zone_of_device_ops tsens_of_ops = { .get_temp = tsens_get_temp, - .get_trend = tsens_get_trend, .set_trips = tsens_set_trips, }; diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h index 1471a2c00f15..ba05c8233356 100644 --- a/drivers/thermal/qcom/tsens.h +++ b/drivers/thermal/qcom/tsens.h @@ -65,7 +65,6 @@ struct tsens_sensor { * @disable: Function to disable the tsens device * @suspend: Function to suspend the tsens device * @resume: Function to resume the tsens device - * @get_trend: Function to get the thermal/temp trend */ struct tsens_ops { /* mandatory callbacks */ @@ -77,7 +76,6 @@ struct tsens_ops { void (*disable)(struct tsens_priv *priv); int (*suspend)(struct tsens_priv *priv); int (*resume)(struct tsens_priv *priv); - int (*get_trend)(struct tsens_sensor *s, enum thermal_trend *trend); }; #define REG_FIELD_FOR_EACH_SENSOR11(_name, _offset, _startbit, _stopbit) \
There is a get_trend function which is a wrapper to call a private get_trend function. However, this private get_trend function is not assigned anywhere. Remove this dead code. Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> --- drivers/thermal/qcom/tsens.c | 12 ------------ drivers/thermal/qcom/tsens.h | 2 -- 2 files changed, 14 deletions(-)