Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1723121 > unrolled thread
| Started by | Daniel Lezcano <daniel.lezcano@linaro.org> |
|---|---|
| First post | 2017-08-30 10:50 +0200 |
| Last post | 2017-09-01 10:40 +0200 |
| Articles | 2 on this page of 22 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH 01/13] thermal/drivers/hisi: Fix missing interrupt enablement Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 10:50 +0200
[PATCH 02/13] thermal/drivers/hisi: Remove the multiple sensors support Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 10:50 +0200
Re: [PATCH 02/13] thermal/drivers/hisi: Remove the multiple sensors support Leo Yan <leo.yan@linaro.org> - 2017-09-01 16:10 +0200
Re: [PATCH 02/13] thermal/drivers/hisi: Remove the multiple sensors support Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-09-01 22:50 +0200
[PATCH 06/13] thermal/drivers/hisi: Remove pointless lock Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 11:00 +0200
Re: [PATCH 06/13] thermal/drivers/hisi: Remove pointless lock Leo Yan <leo.yan@linaro.org> - 2017-09-01 16:50 +0200
[PATCH 03/13] thermal/drivers/hisi: Fix kernel panic on alarm interrupt Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 11:00 +0200
Re: [PATCH 03/13] thermal/drivers/hisi: Fix kernel panic on alarm interrupt Leo Yan <leo.yan@linaro.org> - 2017-09-01 16:20 +0200
[PATCH 08/13] thermal/drivers/hisi: Fix configuration register setting Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 11:00 +0200
Re: [PATCH 08/13] thermal/drivers/hisi: Fix configuration register setting Leo Yan <leo.yan@linaro.org> - 2017-09-02 05:00 +0200
Re: [PATCH 08/13] thermal/drivers/hisi: Fix configuration register setting Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-09-02 10:40 +0200
Re: [PATCH 08/13] thermal/drivers/hisi: Fix configuration register setting Leo Yan <leo.yan@linaro.org> - 2017-09-04 03:00 +0200
Re: [PATCH 08/13] thermal/drivers/hisi: Fix configuration register setting Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-09-04 11:20 +0200
[PATCH 09/13] thermal/drivers/hisi: Remove costly sensor inspection Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 11:00 +0200
Re: [PATCH 09/13] thermal/drivers/hisi: Remove costly sensor inspection Leo Yan <leo.yan@linaro.org> - 2017-09-02 05:30 +0200
Re: [PATCH 09/13] thermal/drivers/hisi: Remove costly sensor inspection Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-09-02 15:20 +0200
Re: [PATCH 09/13] thermal/drivers/hisi: Remove costly sensor inspection Leo Yan <leo.yan@linaro.org> - 2017-09-04 03:00 +0200
Re: [PATCH 09/13] thermal/drivers/hisi: Remove costly sensor inspection Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-09-04 13:30 +0200
Re: [PATCH 09/13] thermal/drivers/hisi: Remove costly sensor inspection Leo Yan <leo.yan@linaro.org> - 2017-09-04 16:40 +0200
[PATCH 05/13] thermal/drivers/hisi: Fix multiple alarm interrupts firing Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-08-30 11:00 +0200
Re: [PATCH 05/13] thermal/drivers/hisi: Fix multiple alarm interrupts firing Leo Yan <leo.yan@linaro.org> - 2017-09-01 16:50 +0200
Re: [PATCH 01/13] thermal/drivers/hisi: Fix missing interrupt enablement Leo Yan <leo.yan@linaro.org> - 2017-09-01 10:40 +0200
Page 2 of 2 — ← Prev page 1 [2]
| From | Leo Yan <leo.yan@linaro.org> |
|---|---|
| Date | 2017-09-01 16:50 +0200 |
| Subject | Re: [PATCH 05/13] thermal/drivers/hisi: Fix multiple alarm interrupts firing |
| Message-ID | <ukVqG-6pN-21@gated-at.bofh.it> |
| In reply to | #1723134 |
On Wed, Aug 30, 2017 at 10:47:29AM +0200, Daniel Lezcano wrote:
> The DT specifies a threshold of 65000, we setup the register with a value in
> the temperature resolution for the controller, 64656.
>
> When we reach 64656, the interrupt fires, the interrupt is disabled. Then the
> irq thread runs and calls thermal_zone_device_update() which will call in turn
> hisi_thermal_get_temp().
>
> The function will look if the temperature decreased, assuming it was more than
> 65000, but that is not the case because the current temperature is 64656
> (because of the rounding when setting the threshold). This condition being
> true, we re-enable the interrupt which fires immediately after exiting the irq
> thread. That happens again and again until the temperature goes to more than
> 65000.
>
> Potentially, there is here an interrupt storm if the temperature stabilizes at
> this temperature. A very unlikely case but possible.
>
> In any case, it does not make sense to handle dozens of alarm interrupt for
> nothing.
>
> Fix this by rounding the threshold value to the controller resolution so the
> check against the threshold is consistent with the one set in the controller.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
This is a good fixing. I do see when the temperature over the tipping
point, if without this patch it's possible to generate interrupt for
2~3 times; after applied this patch it always generate single
interrupt.
Reviewed-by: Leo Yan <leo.yan@linaro.org>
Tested-by: Leo Yan <leo.yan@linaro.org>
> ---
> drivers/thermal/hisi_thermal.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
> index b58ad40..524310d 100644
> --- a/drivers/thermal/hisi_thermal.c
> +++ b/drivers/thermal/hisi_thermal.c
> @@ -90,6 +90,12 @@ static inline long hisi_thermal_temp_to_step(long temp)
> return (temp - HISI_TEMP_BASE) / HISI_TEMP_STEP;
> }
>
> +static inline long hisi_thermal_round_temp(int temp)
> +{
> + return hisi_thermal_step_to_temp(
> + hisi_thermal_temp_to_step(temp));
> +}
> +
> static long hisi_thermal_get_sensor_temp(struct hisi_thermal_data *data,
> struct hisi_thermal_sensor *sensor)
> {
> @@ -221,7 +227,7 @@ static irqreturn_t hisi_thermal_alarm_irq_thread(int irq, void *dev)
> sensor = &data->sensors;
>
> dev_crit(&data->pdev->dev, "THERMAL ALARM: T > %d\n",
> - sensor->thres_temp / 1000);
> + sensor->thres_temp);
> mutex_unlock(&data->thermal_lock);
>
> thermal_zone_device_update(data->sensors.tzd,
> @@ -255,7 +261,7 @@ static int hisi_thermal_register_sensor(struct platform_device *pdev,
>
> for (i = 0; i < of_thermal_get_ntrips(sensor->tzd); i++) {
> if (trip[i].type == THERMAL_TRIP_PASSIVE) {
> - sensor->thres_temp = trip[i].temperature;
> + sensor->thres_temp = hisi_thermal_round_temp(trip[i].temperature);
> break;
> }
> }
> --
> 2.7.4
>
[toc] | [prev] | [next] | [standalone]
| From | Leo Yan <leo.yan@linaro.org> |
|---|---|
| Date | 2017-09-01 10:40 +0200 |
| Subject | Re: [PATCH 01/13] thermal/drivers/hisi: Fix missing interrupt enablement |
| Message-ID | <ukPEC-1pS-5@gated-at.bofh.it> |
| In reply to | #1723121 |
On Wed, Aug 30, 2017 at 10:47:25AM +0200, Daniel Lezcano wrote:
> The interrupt for the temperature threshold is not enabled at the end of the
> probe function, enable it after the setup is complete.
>
> On the other side, the irq_enabled is not correctly set as we are checking if
> the interrupt is masked where 'yes' means irq_enabled=false.
>
> irq_get_irqchip_state(data->irq, IRQCHIP_STATE_MASKED,
> &data->irq_enabled);
>
> As we are always enabling the interrupt, it is pointless to check if
> the interrupt is masked or not, just set irq_enabled to 'true'.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Thanks for patch series, I will verify them one by one. I verified the
interrupt can work well with this patch:
Reviewed-by: Leo Yan <leo.yan@linaro.org>
Tested-by: Leo Yan <leo.yan@linaro.org>
> ---
> drivers/thermal/hisi_thermal.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
> index bd3572c..8381696 100644
> --- a/drivers/thermal/hisi_thermal.c
> +++ b/drivers/thermal/hisi_thermal.c
> @@ -345,8 +345,7 @@ static int hisi_thermal_probe(struct platform_device *pdev)
> }
>
> hisi_thermal_enable_bind_irq_sensor(data);
> - irq_get_irqchip_state(data->irq, IRQCHIP_STATE_MASKED,
> - &data->irq_enabled);
> + data->irq_enabled = true;
>
> for (i = 0; i < HISI_MAX_SENSORS; ++i) {
> ret = hisi_thermal_register_sensor(pdev, data,
> @@ -358,6 +357,8 @@ static int hisi_thermal_probe(struct platform_device *pdev)
> hisi_thermal_toggle_sensor(&data->sensors[i], true);
> }
>
> + enable_irq(data->irq);
> +
> return 0;
> }
>
> --
> 2.7.4
>
[toc] | [prev] | [standalone]
Page 2 of 2 — ← Prev page 1 [2]
Back to top | Article view | linux.kernel
csiph-web