| Message ID | 20220616202537.303655-3-daniel.lezcano@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Daniel Lezcano |
| Headers | show |
| Series | [1/3] thermal/drivers/qcom: Remove get_trend function | expand |
Adding Linus who is missing in the recipient list. On 16/06/2022 22:25, Daniel Lezcano wrote: > The get_trend function relies on the interrupt to set the raising or > dropping trend. However the interpolated temperature is already giving > the temperature information to the thermal framework which is able to > deduce the trend. > > Remove the trend code. > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > --- > drivers/thermal/db8500_thermal.c | 26 ++++---------------------- > 1 file changed, 4 insertions(+), 22 deletions(-) > > diff --git a/drivers/thermal/db8500_thermal.c b/drivers/thermal/db8500_thermal.c > index 21d4d6e6409a..ed40cfd9ab7d 100644 > --- a/drivers/thermal/db8500_thermal.c > +++ b/drivers/thermal/db8500_thermal.c > @@ -53,7 +53,6 @@ static const unsigned long db8500_thermal_points[] = { > > struct db8500_thermal_zone { > struct thermal_zone_device *tz; > - enum thermal_trend trend; > unsigned long interpolated_temp; > unsigned int cur_index; > }; > @@ -73,24 +72,12 @@ static int db8500_thermal_get_temp(void *data, int *temp) > return 0; > } > > -/* Callback to get temperature changing trend */ > -static int db8500_thermal_get_trend(void *data, int trip, enum thermal_trend *trend) > -{ > - struct db8500_thermal_zone *th = data; > - > - *trend = th->trend; > - > - return 0; > -} > - > static struct thermal_zone_of_device_ops thdev_ops = { > .get_temp = db8500_thermal_get_temp, > - .get_trend = db8500_thermal_get_trend, > }; > > static void db8500_thermal_update_config(struct db8500_thermal_zone *th, > unsigned int idx, > - enum thermal_trend trend, > unsigned long next_low, > unsigned long next_high) > { > @@ -98,7 +85,6 @@ static void db8500_thermal_update_config(struct db8500_thermal_zone *th, > > th->cur_index = idx; > th->interpolated_temp = (next_low + next_high)/2; > - th->trend = trend; > > /* > * The PRCMU accept absolute temperatures in celsius so divide > @@ -127,8 +113,7 @@ static irqreturn_t prcmu_low_irq_handler(int irq, void *irq_data) > } > idx -= 1; > > - db8500_thermal_update_config(th, idx, THERMAL_TREND_DROPPING, > - next_low, next_high); > + db8500_thermal_update_config(th, idx, next_low, next_high); > dev_dbg(&th->tz->device, > "PRCMU set max %ld, min %ld\n", next_high, next_low); > > @@ -149,8 +134,7 @@ static irqreturn_t prcmu_high_irq_handler(int irq, void *irq_data) > next_low = db8500_thermal_points[idx]; > idx += 1; > > - db8500_thermal_update_config(th, idx, THERMAL_TREND_RAISING, > - next_low, next_high); > + db8500_thermal_update_config(th, idx, next_low, next_high); > > dev_dbg(&th->tz->device, > "PRCMU set max %ld, min %ld\n", next_high, next_low); > @@ -210,8 +194,7 @@ static int db8500_thermal_probe(struct platform_device *pdev) > dev_info(dev, "thermal zone sensor registered\n"); > > /* Start measuring at the lowest point */ > - db8500_thermal_update_config(th, 0, THERMAL_TREND_STABLE, > - PRCMU_DEFAULT_LOW_TEMP, > + db8500_thermal_update_config(th, 0, PRCMU_DEFAULT_LOW_TEMP, > db8500_thermal_points[0]); > > platform_set_drvdata(pdev, th); > @@ -232,8 +215,7 @@ static int db8500_thermal_resume(struct platform_device *pdev) > struct db8500_thermal_zone *th = platform_get_drvdata(pdev); > > /* Resume and start measuring at the lowest point */ > - db8500_thermal_update_config(th, 0, THERMAL_TREND_STABLE, > - PRCMU_DEFAULT_LOW_TEMP, > + db8500_thermal_update_config(th, 0, PRCMU_DEFAULT_LOW_TEMP, > db8500_thermal_points[0]); > > return 0;
On Tue, Jun 28, 2022 at 10:40 AM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > Adding Linus who is missing in the recipient list. > > > On 16/06/2022 22:25, Daniel Lezcano wrote: > > The get_trend function relies on the interrupt to set the raising or > > dropping trend. However the interpolated temperature is already giving > > the temperature information to the thermal framework which is able to > > deduce the trend. > > > > Remove the trend code. > > > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> I certainly trust you with this :) Acked-by: Linus Walleij <linus.walleij@linaro.org> The code was originally written by Hongbo Zhang, but co-developed and tested by Vincent Guittot I think, so paging him as well. Yours, Linus Walleij
On 28/06/2022 14:50, Linus Walleij wrote: > On Tue, Jun 28, 2022 at 10:40 AM Daniel Lezcano > <daniel.lezcano@linaro.org> wrote: > >> Adding Linus who is missing in the recipient list. >> >> >> On 16/06/2022 22:25, Daniel Lezcano wrote: >>> The get_trend function relies on the interrupt to set the raising or >>> dropping trend. However the interpolated temperature is already giving >>> the temperature information to the thermal framework which is able to >>> deduce the trend. >>> >>> Remove the trend code. >>> >>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > > I certainly trust you with this :) > Acked-by: Linus Walleij <linus.walleij@linaro.org> > > The code was originally written by Hongbo Zhang, but co-developed > and tested by Vincent Guittot I think, so paging > him as well. Ok, thanks If Vincent has no concern with this change, I'll queue up the series
On Thu, 30 Jun 2022 at 12:16, Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > On 28/06/2022 14:50, Linus Walleij wrote: > > On Tue, Jun 28, 2022 at 10:40 AM Daniel Lezcano > > <daniel.lezcano@linaro.org> wrote: > > > >> Adding Linus who is missing in the recipient list. > >> > >> > >> On 16/06/2022 22:25, Daniel Lezcano wrote: > >>> The get_trend function relies on the interrupt to set the raising or > >>> dropping trend. However the interpolated temperature is already giving > >>> the temperature information to the thermal framework which is able to > >>> deduce the trend. > >>> > >>> Remove the trend code. > >>> > >>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > > > > I certainly trust you with this :) > > Acked-by: Linus Walleij <linus.walleij@linaro.org> > > > > The code was originally written by Hongbo Zhang, but co-developed > > and tested by Vincent Guittot I think, so paging > > him as well. > > Ok, thanks > > If Vincent has no concern with this change, I'll queue up the series I don't have any particular concerns. I'm just curious, are you planning to remove the get_trend completely from the thermal framework ? > > > > -- > <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs > > Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | > <http://twitter.com/#!/linaroorg> Twitter | > <http://www.linaro.org/linaro-blog/> Blog
Hi Vincent, On 30/06/2022 14:32, Vincent Guittot wrote: > On Thu, 30 Jun 2022 at 12:16, Daniel Lezcano <daniel.lezcano@linaro.org> wrote: >> >> On 28/06/2022 14:50, Linus Walleij wrote: >>> On Tue, Jun 28, 2022 at 10:40 AM Daniel Lezcano >>> <daniel.lezcano@linaro.org> wrote: >>> >>>> Adding Linus who is missing in the recipient list. >>>> >>>> >>>> On 16/06/2022 22:25, Daniel Lezcano wrote: >>>>> The get_trend function relies on the interrupt to set the raising or >>>>> dropping trend. However the interpolated temperature is already giving >>>>> the temperature information to the thermal framework which is able to >>>>> deduce the trend. >>>>> >>>>> Remove the trend code. >>>>> >>>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> >>> >>> I certainly trust you with this :) >>> Acked-by: Linus Walleij <linus.walleij@linaro.org> >>> >>> The code was originally written by Hongbo Zhang, but co-developed >>> and tested by Vincent Guittot I think, so paging >>> him as well. >> >> Ok, thanks >> >> If Vincent has no concern with this change, I'll queue up the series > > I don't have any particular concerns. I'm just curious, are you > planning to remove the get_trend completely from the thermal framework > ? Well, actually the get_trend() ops was added for ACPI and because the ops was there, some drivers provided their own implementation and it appears they are unnecessary. It is this pointless code I want to remove. Only the get_trend() ops will remain for the ACPI. Hopefully we can remove the ops in the future.
diff --git a/drivers/thermal/db8500_thermal.c b/drivers/thermal/db8500_thermal.c index 21d4d6e6409a..ed40cfd9ab7d 100644 --- a/drivers/thermal/db8500_thermal.c +++ b/drivers/thermal/db8500_thermal.c @@ -53,7 +53,6 @@ static const unsigned long db8500_thermal_points[] = { struct db8500_thermal_zone { struct thermal_zone_device *tz; - enum thermal_trend trend; unsigned long interpolated_temp; unsigned int cur_index; }; @@ -73,24 +72,12 @@ static int db8500_thermal_get_temp(void *data, int *temp) return 0; } -/* Callback to get temperature changing trend */ -static int db8500_thermal_get_trend(void *data, int trip, enum thermal_trend *trend) -{ - struct db8500_thermal_zone *th = data; - - *trend = th->trend; - - return 0; -} - static struct thermal_zone_of_device_ops thdev_ops = { .get_temp = db8500_thermal_get_temp, - .get_trend = db8500_thermal_get_trend, }; static void db8500_thermal_update_config(struct db8500_thermal_zone *th, unsigned int idx, - enum thermal_trend trend, unsigned long next_low, unsigned long next_high) { @@ -98,7 +85,6 @@ static void db8500_thermal_update_config(struct db8500_thermal_zone *th, th->cur_index = idx; th->interpolated_temp = (next_low + next_high)/2; - th->trend = trend; /* * The PRCMU accept absolute temperatures in celsius so divide @@ -127,8 +113,7 @@ static irqreturn_t prcmu_low_irq_handler(int irq, void *irq_data) } idx -= 1; - db8500_thermal_update_config(th, idx, THERMAL_TREND_DROPPING, - next_low, next_high); + db8500_thermal_update_config(th, idx, next_low, next_high); dev_dbg(&th->tz->device, "PRCMU set max %ld, min %ld\n", next_high, next_low); @@ -149,8 +134,7 @@ static irqreturn_t prcmu_high_irq_handler(int irq, void *irq_data) next_low = db8500_thermal_points[idx]; idx += 1; - db8500_thermal_update_config(th, idx, THERMAL_TREND_RAISING, - next_low, next_high); + db8500_thermal_update_config(th, idx, next_low, next_high); dev_dbg(&th->tz->device, "PRCMU set max %ld, min %ld\n", next_high, next_low); @@ -210,8 +194,7 @@ static int db8500_thermal_probe(struct platform_device *pdev) dev_info(dev, "thermal zone sensor registered\n"); /* Start measuring at the lowest point */ - db8500_thermal_update_config(th, 0, THERMAL_TREND_STABLE, - PRCMU_DEFAULT_LOW_TEMP, + db8500_thermal_update_config(th, 0, PRCMU_DEFAULT_LOW_TEMP, db8500_thermal_points[0]); platform_set_drvdata(pdev, th); @@ -232,8 +215,7 @@ static int db8500_thermal_resume(struct platform_device *pdev) struct db8500_thermal_zone *th = platform_get_drvdata(pdev); /* Resume and start measuring at the lowest point */ - db8500_thermal_update_config(th, 0, THERMAL_TREND_STABLE, - PRCMU_DEFAULT_LOW_TEMP, + db8500_thermal_update_config(th, 0, PRCMU_DEFAULT_LOW_TEMP, db8500_thermal_points[0]); return 0;
The get_trend function relies on the interrupt to set the raising or dropping trend. However the interpolated temperature is already giving the temperature information to the thermal framework which is able to deduce the trend. Remove the trend code. Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> --- drivers/thermal/db8500_thermal.c | 26 ++++---------------------- 1 file changed, 4 insertions(+), 22 deletions(-)