2019-11-04 13:31:59

by Pascal Paillet

Subject: [PATCH v2 1/5] thermal: stm32: remove hardware irq handler

Remove hardware irq handler because it is not needed to disable the
interrupt before the threaded handler. The goal is to simplify
the code.

Change-Id: Ida967e8543c8dafc6a24508000f64f6405add31d
---
drivers/thermal/st/stm_thermal.c | 25 +------------------------
1 file changed, 1 insertion(+), 24 deletions(-)

diff --git a/drivers/thermal/st/stm_thermal.c b/drivers/thermal/st/stm_thermal.c
index cf9ddc52f30e..31aa63fb3db1 100644
--- a/drivers/thermal/st/stm_thermal.c
+++ b/drivers/thermal/st/stm_thermal.c
@@ -98,21 +98,10 @@ struct stm_thermal_sensor {
unsigned int low_temp_enabled;
int num_trips;
int irq;
- unsigned int irq_enabled;
void __iomem *base;
int t0, fmt0, ramp_coeff;
};

-static irqreturn_t stm_thermal_alarm_irq(int irq, void *sdata)
-{
- struct stm_thermal_sensor *sensor = sdata;
-
- disable_irq_nosync(irq);
- sensor->irq_enabled = false;
-
- return IRQ_WAKE_THREAD;
-}
-
static irqreturn_t stm_thermal_alarm_irq_thread(int irq, void *sdata)
{
u32 value;
@@ -464,16 +453,6 @@ static int stm_thermal_get_temp(void *data, int *temp)
if (ret)
return ret;
}
-
- /*
- * Re-enable alarm IRQ if temperature below critical
- * temperature
- */
- if (!sensor->irq_enabled &&
- (celsius(*temp) < sensor->temp_critical)) {
- sensor->irq_enabled = true;
- enable_irq(sensor->irq);
- }
}

return 0;
@@ -493,7 +472,7 @@ static int stm_register_irq(struct stm_thermal_sensor *sensor)
}

ret = devm_request_threaded_irq(dev, sensor->irq,
- stm_thermal_alarm_irq,
+ NULL,
stm_thermal_alarm_irq_thread,
IRQF_ONESHOT,
dev->driver->name, sensor);
@@ -503,8 +482,6 @@ static int stm_register_irq(struct stm_thermal_sensor *sensor)
return ret;
}

- sensor->irq_enabled = true;
-
dev_dbg(dev, "%s: thermal IRQ registered", __func__);

return 0;
--
2.17.1

2019-12-04 21:08:33

by Daniel Lezcano

Subject: Re: [PATCH v2 1/5] thermal: stm32: remove hardware irq handler

On 04/11/2019 14:30, Pascal Paillet wrote:
> Remove hardware irq handler because it is not needed to disable the
> interrupt before the threaded handler. The goal is to simplify
> the code.

Please elaborate the explanation here. I guess all the code removed is
because of:

/* read IT reason in SR and clear flags */
value = readl_relaxed(sensor->base + DTS_SR_OFFSET);

Right?

> Change-Id: Ida967e8543c8dafc6a24508000f64f6405add31d

Remove Change-Id.

Missing SoB.

> ---
> drivers/thermal/st/stm_thermal.c | 25 +------------------------
> 1 file changed, 1 insertion(+), 24 deletions(-)
>
> diff --git a/drivers/thermal/st/stm_thermal.c b/drivers/thermal/st/stm_thermal.c
> index cf9ddc52f30e..31aa63fb3db1 100644
> --- a/drivers/thermal/st/stm_thermal.c
> +++ b/drivers/thermal/st/stm_thermal.c
> @@ -98,21 +98,10 @@ struct stm_thermal_sensor {
> unsigned int low_temp_enabled;
> int num_trips;
> int irq;
> - unsigned int irq_enabled;
> void __iomem *base;
> int t0, fmt0, ramp_coeff;
> };
>
> -static irqreturn_t stm_thermal_alarm_irq(int irq, void *sdata)
> -{
> - struct stm_thermal_sensor *sensor = sdata;
> -
> - disable_irq_nosync(irq);
> - sensor->irq_enabled = false;
> -
> - return IRQ_WAKE_THREAD;
> -}
> -
> static irqreturn_t stm_thermal_alarm_irq_thread(int irq, void *sdata)
> {
> u32 value;
> @@ -464,16 +453,6 @@ static int stm_thermal_get_temp(void *data, int *temp)
> if (ret)
> return ret;
> }
> -
> - /*
> - * Re-enable alarm IRQ if temperature below critical
> - * temperature
> - */
> - if (!sensor->irq_enabled &&
> - (celsius(*temp) < sensor->temp_critical)) {
> - sensor->irq_enabled = true;
> - enable_irq(sensor->irq);
> - }
> }
>
> return 0;
> @@ -493,7 +472,7 @@ static int stm_register_irq(struct stm_thermal_sensor *sensor)
> }
>
> ret = devm_request_threaded_irq(dev, sensor->irq,
> - stm_thermal_alarm_irq,
> + NULL,
> stm_thermal_alarm_irq_thread,
> IRQF_ONESHOT,
> dev->driver->name, sensor);
> @@ -503,8 +482,6 @@ static int stm_register_irq(struct stm_thermal_sensor *sensor)
> return ret;
> }
>
> - sensor->irq_enabled = true;
> -
> dev_dbg(dev, "%s: thermal IRQ registered", __func__);
>
> return 0;
>


--
<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