[PATCH 5/5] thermal/drivers/hisi: Remove costly sensor inspection
From: Rafael David Tinoco
Date: Mon Dec 03 2018 - 08:31:33 EST
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