From: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> commit 10d7e9a9181f4637640f388d334c6740c1b5d0e8 upstream. The sensor is all setup, bind, resetted, acked, etc... every single second. That was the way to workaround a problem with the interrupt bouncing again and again. With the following changes, we fix all in one: - Do the setup, one time, at probe time - Add the IRQF_ONESHOT, ack the interrupt in the threaded handler - Remove the interrupt handler - Set the correct value for the LAG register - Remove all the irq_enabled stuff in the code as the interruption handling is fixed - Remove the 3ms delay - Reorder the initialization routine to be in the right order It ends up to a nicer code and more efficient, the 3-5ms delay is removed from the get_temp() path. Link: https://bugs.linaro.org/show_bug.cgi?id=4053 (PATCH 5/5) Signed-off-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> Reviewed-by: Leo Yan <leo.yan@xxxxxxxxxx> Tested-by: Leo Yan <leo.yan@xxxxxxxxxx> Signed-off-by: Eduardo Valentin <edubezval@xxxxxxxxx> (cherry picked from commit 10d7e9a9181f4637640f388d334c6740c1b5d0e8) Signed-off-by: Rafael David Tinoco <rafael.tinoco@xxxxxxxxxx> --- drivers/thermal/hisi_thermal.c | 203 +++++++++++++++------------------ 1 file changed, 93 insertions(+), 110 deletions(-) diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c index 31b52ebf426f..7c5d4647cb5e 100644 --- a/drivers/thermal/hisi_thermal.c +++ b/drivers/thermal/hisi_thermal.c @@ -41,6 +41,7 @@ #define HISI_TEMP_BASE (-60000) #define HISI_TEMP_RESET (100000) #define HISI_TEMP_STEP (784) +#define HISI_TEMP_LAG (3500) #define HISI_MAX_SENSORS 4 #define HISI_DEFAULT_SENSOR 2 @@ -60,8 +61,6 @@ struct hisi_thermal_data { struct clk *clk; struct hisi_thermal_sensor sensors; int irq; - bool irq_enabled; - void __iomem *regs; }; @@ -99,9 +98,40 @@ static inline long hisi_thermal_round_temp(int temp) hisi_thermal_temp_to_step(temp)); } +/* + * The lag register contains 5 bits encoding the temperature in steps. + * + * Each time the temperature crosses the threshold boundary, an + * interrupt is raised. It could be when the temperature is going + * above the threshold or below. However, if the temperature is + * fluctuating around this value due to the load, we can receive + * several interrupts which may not desired. + * + * We can setup a temperature representing the delta between the + * threshold and the current temperature when the temperature is + * decreasing. + * + * For instance: the lag register is 5°C, the threshold is 65°C, when + * the temperature reaches 65°C an interrupt is raised and when the + * temperature decrease to 65°C - 5°C another interrupt is raised. + * + * A very short lag can lead to an interrupt storm, a long lag + * increase the latency to react to the temperature changes. In our + * case, that is not really a problem as we are polling the + * temperature. + * + * [0:4] : lag register + * + * The temperature is coded in steps, cf. HISI_TEMP_STEP. + * + * Min : 0x00 : 0.0 °C + * Max : 0x1F : 24.3 °C + * + * The 'value' parameter is in milliCelsius. + */ static inline void hisi_thermal_set_lag(void __iomem *addr, int value) { - writel(value, addr + TEMP0_LAG); + writel((value / HISI_TEMP_STEP) & 0x1F, addr + TEMP0_LAG); } static inline void hisi_thermal_alarm_clear(void __iomem *addr, int value) @@ -171,71 +201,6 @@ static inline void hisi_thermal_hdak_set(void __iomem *addr, int value) (value << 4), addr + TEMP0_CFG); } -static long hisi_thermal_get_sensor_temp(struct hisi_thermal_data *data, - struct hisi_thermal_sensor *sensor) -{ - long val; - - mutex_lock(&data->thermal_lock); - - /* disable interrupt */ - hisi_thermal_alarm_enable(data->regs, 0); - hisi_thermal_alarm_clear(data->regs, 1); - - /* disable module firstly */ - hisi_thermal_enable(data->regs, 0); - - /* select sensor id */ - hisi_thermal_sensor_select(data->regs, sensor->id); - - /* enable module */ - hisi_thermal_enable(data->regs, 1); - - usleep_range(3000, 5000); - - val = hisi_thermal_get_temperature(data->regs); - - mutex_unlock(&data->thermal_lock); - - return val; -} - -static void hisi_thermal_enable_bind_irq_sensor - (struct hisi_thermal_data *data) -{ - struct hisi_thermal_sensor *sensor; - - mutex_lock(&data->thermal_lock); - - sensor = &data->sensors; - - /* setting the hdak time */ - hisi_thermal_hdak_set(data->regs, 0); - - /* disable module firstly */ - hisi_thermal_reset_enable(data->regs, 0); - hisi_thermal_enable(data->regs, 0); - - /* select sensor id */ - hisi_thermal_sensor_select(data->regs, sensor->id); - - /* enable for interrupt */ - hisi_thermal_alarm_set(data->regs, sensor->thres_temp); - - hisi_thermal_reset_set(data->regs, HISI_TEMP_RESET); - - /* enable module */ - hisi_thermal_reset_enable(data->regs, 1); - hisi_thermal_enable(data->regs, 1); - - hisi_thermal_alarm_clear(data->regs, 0); - hisi_thermal_alarm_enable(data->regs, 1); - - usleep_range(3000, 5000); - - mutex_unlock(&data->thermal_lock); -} - static void hisi_thermal_disable_sensor(struct hisi_thermal_data *data) { mutex_lock(&data->thermal_lock); @@ -253,25 +218,10 @@ static int hisi_thermal_get_temp(void *_sensor, int *temp) struct hisi_thermal_sensor *sensor = _sensor; struct hisi_thermal_data *data = sensor->thermal; - *temp = hisi_thermal_get_sensor_temp(data, sensor); - - dev_dbg(&data->pdev->dev, "id=%d, irq=%d, temp=%d, thres=%d\n", - sensor->id, data->irq_enabled, *temp, sensor->thres_temp); - /* - * Bind irq to sensor for two cases: - * Reenable alarm IRQ if temperature below threshold; - * if irq has been enabled, always set it; - */ - if (data->irq_enabled) { - hisi_thermal_enable_bind_irq_sensor(data); - return 0; - } + *temp = hisi_thermal_get_temperature(data->regs); - if (*temp < sensor->thres_temp) { - data->irq_enabled = true; - hisi_thermal_enable_bind_irq_sensor(data); - enable_irq(data->irq); - } + dev_dbg(&data->pdev->dev, "id=%d, temp=%d, thres=%d\n", + sensor->id, *temp, sensor->thres_temp); return 0; } @@ -280,26 +230,27 @@ static const struct thermal_zone_of_device_ops hisi_of_thermal_ops = { .get_temp = hisi_thermal_get_temp, }; -static irqreturn_t hisi_thermal_alarm_irq(int irq, void *dev) +static irqreturn_t hisi_thermal_alarm_irq_thread(int irq, void *dev) { struct hisi_thermal_data *data = dev; + struct hisi_thermal_sensor *sensor = &data->sensors; + int temp; - disable_irq_nosync(irq); - data->irq_enabled = false; + hisi_thermal_alarm_clear(data->regs, 1); - return IRQ_WAKE_THREAD; -} + temp = hisi_thermal_get_temperature(data->regs); -static irqreturn_t hisi_thermal_alarm_irq_thread(int irq, void *dev) -{ - struct hisi_thermal_data *data = dev; - struct hisi_thermal_sensor *sensor = &data->sensors; + if (temp >= sensor->thres_temp) { + dev_crit(&data->pdev->dev, "THERMAL ALARM: %d > %d\n", + temp, sensor->thres_temp); - dev_crit(&data->pdev->dev, "THERMAL ALARM: T > %d\n", - sensor->thres_temp); + thermal_zone_device_update(data->sensors.tzd, + THERMAL_EVENT_UNSPECIFIED); - thermal_zone_device_update(data->sensors.tzd, - THERMAL_EVENT_UNSPECIFIED); + } else if (temp < sensor->thres_temp) { + dev_crit(&data->pdev->dev, "THERMAL ALARM stopped: %d < %d\n", + temp, sensor->thres_temp); + } return IRQ_HANDLED; } @@ -352,6 +303,40 @@ static void hisi_thermal_toggle_sensor(struct hisi_thermal_sensor *sensor, on ? THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED); } +static int hisi_thermal_setup(struct hisi_thermal_data *data) +{ + struct hisi_thermal_sensor *sensor; + + sensor = &data->sensors; + + /* disable module firstly */ + hisi_thermal_reset_enable(data->regs, 0); + hisi_thermal_enable(data->regs, 0); + + /* select sensor id */ + hisi_thermal_sensor_select(data->regs, sensor->id); + + /* setting the hdak time */ + hisi_thermal_hdak_set(data->regs, 0); + + /* setting lag value between current temp and the threshold */ + hisi_thermal_set_lag(data->regs, HISI_TEMP_LAG); + + /* enable for interrupt */ + hisi_thermal_alarm_set(data->regs, sensor->thres_temp); + + hisi_thermal_reset_set(data->regs, HISI_TEMP_RESET); + + /* enable module */ + hisi_thermal_reset_enable(data->regs, 1); + hisi_thermal_enable(data->regs, 1); + + hisi_thermal_alarm_clear(data->regs, 0); + hisi_thermal_alarm_enable(data->regs, 1); + + return 0; +} + static int hisi_thermal_probe(struct platform_device *pdev) { struct hisi_thermal_data *data; @@ -394,9 +379,6 @@ static int hisi_thermal_probe(struct platform_device *pdev) return ret; } - hisi_thermal_enable_bind_irq_sensor(data); - data->irq_enabled = true; - ret = hisi_thermal_register_sensor(pdev, data, &data->sensors, HISI_DEFAULT_SENSOR); @@ -406,10 +388,15 @@ static int hisi_thermal_probe(struct platform_device *pdev) return ret; } - ret = devm_request_threaded_irq(&pdev->dev, data->irq, - hisi_thermal_alarm_irq, + ret = hisi_thermal_setup(data); + if (ret) { + dev_err(&pdev->dev, "Failed to setup the sensor: %d\n", ret); + return ret; + } + + ret = devm_request_threaded_irq(&pdev->dev, data->irq, NULL, hisi_thermal_alarm_irq_thread, - 0, "hisi_thermal", data); + IRQF_ONESHOT, "hisi_thermal", data); if (ret < 0) { dev_err(&pdev->dev, "failed to request alarm irq: %d\n", ret); return ret; @@ -417,8 +404,6 @@ static int hisi_thermal_probe(struct platform_device *pdev) hisi_thermal_toggle_sensor(&data->sensors, true); - enable_irq(data->irq); - return 0; } @@ -440,7 +425,6 @@ static int hisi_thermal_suspend(struct device *dev) struct hisi_thermal_data *data = dev_get_drvdata(dev); hisi_thermal_disable_sensor(data); - data->irq_enabled = false; clk_disable_unprepare(data->clk); @@ -456,8 +440,7 @@ static int hisi_thermal_resume(struct device *dev) if (ret) return ret; - data->irq_enabled = true; - hisi_thermal_enable_bind_irq_sensor(data); + hisi_thermal_setup(data); return 0; } -- 2.20.0.rc1
