This series fixes interrupt handling in the LVTS thermal driver. It not
only solves the interrupt storms currently happening, but also allows
the temperature monitoring interrupts to trigger when the thermal trip
temperature is reached.
This series was tested together with the MT8192 support series [1] on
the Spherion Chromebook (mt8192-asurada-spherion).
These are standalone fixes and don't depend on anything else.
[1] https://lore.kernel.org/all/[email protected]
Thanks,
Nícolas
Changes in v3:
- Reworded cover letter
- Split bitmaps for immediate and filtered modes into separate arrays
(patch 2)
- Removed duplicate code for setting MINIMUM_THRESHOLD (patch 5)
Changes in v2:
- Added commits 3, 5, 6 to get working interrupts when crossing thermal
trip points
- Updated commit 4 with interrupt flags for the offset
Nícolas F. R. A. Prado (6):
thermal/drivers/mediatek/lvts_thermal: Handle IRQ on all controllers
thermal/drivers/mediatek/lvts_thermal: Honor sensors in immediate mode
thermal/drivers/mediatek/lvts_thermal: Use offset threshold for IRQ
thermal/drivers/mediatek/lvts_thermal: Disable undesired interrupts
thermal/drivers/mediatek/lvts_thermal: Don't leave threshold zeroed
thermal/drivers/mediatek/lvts_thermal: Manage threshold between
sensors
drivers/thermal/mediatek/lvts_thermal.c | 152 ++++++++++++++++++------
1 file changed, 115 insertions(+), 37 deletions(-)
--
2.41.0
There are two kinds of temperature monitoring interrupts available:
* High Offset, Low Offset
* Hot, Hot to normal, Cold
The code currently uses the hot/h2n/cold interrupts, however in a way
that doesn't work: the cold threshold is left uninitialized, which
prevents the other thresholds from ever triggering, and the h2n
interrupt is used as the lower threshold, which prevents the hot
interrupt from triggering again after the thresholds are updated by the
thermal framework, since a hot interrupt can only trigger again after
the hot to normal interrupt has been triggered.
But better yet than addressing those issues, is to use the high/low
offset interrupts instead. This way only two thresholds need to be
managed, which have a simpler state machine, making them a better match
to the thermal framework's high and low thresholds.
Fixes: f5f633b18234 ("thermal/drivers/mediatek: Add the Low Voltage Thermal Sensor driver")
Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
---
(no changes since v2)
Changes in v2:
- Added this commit
drivers/thermal/mediatek/lvts_thermal.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index 8082195f53ae..e7cbfe0426b5 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -298,9 +298,9 @@ static int lvts_set_trips(struct thermal_zone_device *tz, int low, int high)
u32 raw_high = lvts_temp_to_raw(high);
/*
- * Hot to normal temperature threshold
+ * Low offset temperature threshold
*
- * LVTS_H2NTHRE
+ * LVTS_OFFSETL
*
* Bits:
*
@@ -309,13 +309,13 @@ static int lvts_set_trips(struct thermal_zone_device *tz, int low, int high)
if (low != -INT_MAX) {
pr_debug("%s: Setting low limit temperature interrupt: %d\n",
thermal_zone_device_type(tz), low);
- writel(raw_low, LVTS_H2NTHRE(base));
+ writel(raw_low, LVTS_OFFSETL(base));
}
/*
- * Hot temperature threshold
+ * High offset temperature threshold
*
- * LVTS_HTHRE
+ * LVTS_OFFSETH
*
* Bits:
*
@@ -323,7 +323,7 @@ static int lvts_set_trips(struct thermal_zone_device *tz, int low, int high)
*/
pr_debug("%s: Setting high limit temperature interrupt: %d\n",
thermal_zone_device_type(tz), high);
- writel(raw_high, LVTS_HTHRE(base));
+ writel(raw_high, LVTS_OFFSETH(base));
return 0;
}
--
2.41.0
Out of the many interrupts supported by the hardware, the only ones of
interest to the driver currently are:
* The temperature went over the high offset threshold, for any of the
sensors
* The temperature went below the low offset threshold, for any of the
sensors
* The temperature went over the stage3 threshold
These are the only thresholds configured by the driver through the
OFFSETH, OFFSETL, and PROTTC registers, respectively.
The current interrupt mask in LVTS_MONINT_CONF, enables many more
interrupts, including data ready on sensors for both filtered and
immediate mode. These are not only not handled by the driver, but they
are also triggered too often, causing unneeded overhead. Disable these
unnecessary interrupts.
The meaning of each bit can be seen in the comment describing
LVTS_MONINTST in the IRQ handler.
Fixes: f5f633b18234 ("thermal/drivers/mediatek: Add the Low Voltage Thermal Sensor driver")
Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
---
(no changes since v2)
Changes in v2:
- Reworded commit and changed flag to use offset interrupts instead
drivers/thermal/mediatek/lvts_thermal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index e7cbfe0426b5..e6dd4d120e54 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -65,7 +65,7 @@
#define LVTS_HW_FILTER 0x2
#define LVTS_TSSEL_CONF 0x13121110
#define LVTS_CALSCALE_CONF 0x300
-#define LVTS_MONINT_CONF 0x9FBF7BDE
+#define LVTS_MONINT_CONF 0x8300318C
#define LVTS_INT_SENSOR0 0x0009001F
#define LVTS_INT_SENSOR1 0x001203E0
--
2.41.0
Each LVTS thermal controller can have up to four sensors, each capable
of triggering its own interrupt when its measured temperature crosses
the configured threshold. The threshold for each sensor is handled
separately by the thermal framework, since each one is registered with
its own thermal zone and trips. However, the temperature thresholds are
configured on the controller, and therefore are shared between all
sensors on that controller.
When the temperature measured by the sensors is different enough to
cause the thermal framework to configure different thresholds for each
one, interrupts start triggering on sensors outside the last threshold
configured.
To address the issue, track the thresholds required by each sensor and
only actually set the highest one in the hardware, and disable
interrupts for all sensors outside the current configured range.
Fixes: f5f633b18234 ("thermal/drivers/mediatek: Add the Low Voltage Thermal Sensor driver")
Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
---
(no changes since v2)
Changes in v2:
- Added this commit
drivers/thermal/mediatek/lvts_thermal.c | 69 +++++++++++++++++++++++++
1 file changed, 69 insertions(+)
diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index 7552e1d59dc9..a2dc33697371 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -67,6 +67,11 @@
#define LVTS_CALSCALE_CONF 0x300
#define LVTS_MONINT_CONF 0x8300318C
+#define LVTS_MONINT_OFFSET_SENSOR0 0xC
+#define LVTS_MONINT_OFFSET_SENSOR1 0x180
+#define LVTS_MONINT_OFFSET_SENSOR2 0x3000
+#define LVTS_MONINT_OFFSET_SENSOR3 0x3000000
+
#define LVTS_INT_SENSOR0 0x0009001F
#define LVTS_INT_SENSOR1 0x001203E0
#define LVTS_INT_SENSOR2 0x00247C00
@@ -112,6 +117,8 @@ struct lvts_sensor {
void __iomem *base;
int id;
int dt_id;
+ int low_thresh;
+ int high_thresh;
};
struct lvts_ctrl {
@@ -121,6 +128,8 @@ struct lvts_ctrl {
int num_lvts_sensor;
int mode;
void __iomem *base;
+ int low_thresh;
+ int high_thresh;
};
struct lvts_domain {
@@ -292,12 +301,66 @@ static int lvts_get_temp(struct thermal_zone_device *tz, int *temp)
return 0;
}
+static void lvts_update_irq_mask(struct lvts_ctrl *lvts_ctrl)
+{
+ u32 masks[] = {
+ LVTS_MONINT_OFFSET_SENSOR0,
+ LVTS_MONINT_OFFSET_SENSOR1,
+ LVTS_MONINT_OFFSET_SENSOR2,
+ LVTS_MONINT_OFFSET_SENSOR3,
+ };
+ u32 value = 0;
+ int i;
+
+ value = readl(LVTS_MONINT(lvts_ctrl->base));
+
+ for (i = 0; i < ARRAY_SIZE(masks); i++) {
+ if (lvts_ctrl->sensors[i].high_thresh == lvts_ctrl->high_thresh
+ && lvts_ctrl->sensors[i].low_thresh == lvts_ctrl->low_thresh)
+ value |= masks[i];
+ else
+ value &= ~masks[i];
+ }
+
+ writel(value, LVTS_MONINT(lvts_ctrl->base));
+}
+
+static bool lvts_should_update_thresh(struct lvts_ctrl *lvts_ctrl, int high)
+{
+ int i;
+
+ if (high > lvts_ctrl->high_thresh)
+ return true;
+
+ for (i = 0; i < lvts_ctrl->num_lvts_sensor; i++)
+ if (lvts_ctrl->sensors[i].high_thresh == lvts_ctrl->high_thresh
+ && lvts_ctrl->sensors[i].low_thresh == lvts_ctrl->low_thresh)
+ return false;
+
+ return true;
+}
+
static int lvts_set_trips(struct thermal_zone_device *tz, int low, int high)
{
struct lvts_sensor *lvts_sensor = thermal_zone_device_priv(tz);
+ struct lvts_ctrl *lvts_ctrl = container_of(lvts_sensor, struct lvts_ctrl, sensors[lvts_sensor->id]);
void __iomem *base = lvts_sensor->base;
u32 raw_low = lvts_temp_to_raw(low != -INT_MAX ? low : LVTS_MINIMUM_THRESHOLD);
u32 raw_high = lvts_temp_to_raw(high);
+ bool should_update_thresh;
+
+ lvts_sensor->low_thresh = low;
+ lvts_sensor->high_thresh = high;
+
+ should_update_thresh = lvts_should_update_thresh(lvts_ctrl, high);
+ if (should_update_thresh) {
+ lvts_ctrl->high_thresh = high;
+ lvts_ctrl->low_thresh = low;
+ }
+ lvts_update_irq_mask(lvts_ctrl);
+
+ if (!should_update_thresh)
+ return 0;
/*
* Low offset temperature threshold
@@ -521,6 +584,9 @@ static int lvts_sensor_init(struct device *dev, struct lvts_ctrl *lvts_ctrl,
*/
lvts_sensor[i].msr = lvts_ctrl_data->mode == LVTS_MSR_IMMEDIATE_MODE ?
imm_regs[i] : msr_regs[i];
+
+ lvts_sensor[i].low_thresh = INT_MIN;
+ lvts_sensor[i].high_thresh = INT_MIN;
};
lvts_ctrl->num_lvts_sensor = lvts_ctrl_data->num_lvts_sensor;
@@ -688,6 +754,9 @@ static int lvts_ctrl_init(struct device *dev, struct lvts_domain *lvts_td,
*/
lvts_ctrl[i].hw_tshut_raw_temp =
lvts_temp_to_raw(lvts_data->lvts_ctrl[i].hw_tshut_temp);
+
+ lvts_ctrl[i].low_thresh = INT_MIN;
+ lvts_ctrl[i].high_thresh = INT_MIN;
}
/*
--
2.41.0
There is a single IRQ handler for each LVTS thermal domain, and it is
supposed to check each of its underlying controllers for the origin of
the interrupt and clear its status. However due to a typo, only the
first controller was ever being handled, which resulted in the interrupt
never being cleared when it happened on the other controllers. Add the
missing index so interrupts are handled for all controllers.
Fixes: f5f633b18234 ("thermal/drivers/mediatek: Add the Low Voltage Thermal Sensor driver")
Reviewed-by: Matthias Brugger <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Tested-by: Chen-Yu Tsai <[email protected]>
Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
---
(no changes since v1)
drivers/thermal/mediatek/lvts_thermal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index 1e11defe4f35..ba8f86ee12b6 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -451,7 +451,7 @@ static irqreturn_t lvts_irq_handler(int irq, void *data)
for (i = 0; i < lvts_td->num_lvts_ctrl; i++) {
- aux = lvts_ctrl_irq_handler(lvts_td->lvts_ctrl);
+ aux = lvts_ctrl_irq_handler(&lvts_td->lvts_ctrl[i]);
if (aux != IRQ_HANDLED)
continue;
--
2.41.0
Il 06/07/23 17:37, Nícolas F. R. A. Prado ha scritto:
> Each LVTS thermal controller can have up to four sensors, each capable
> of triggering its own interrupt when its measured temperature crosses
> the configured threshold. The threshold for each sensor is handled
> separately by the thermal framework, since each one is registered with
> its own thermal zone and trips. However, the temperature thresholds are
> configured on the controller, and therefore are shared between all
> sensors on that controller.
>
> When the temperature measured by the sensors is different enough to
> cause the thermal framework to configure different thresholds for each
> one, interrupts start triggering on sensors outside the last threshold
> configured.
>
> To address the issue, track the thresholds required by each sensor and
> only actually set the highest one in the hardware, and disable
> interrupts for all sensors outside the current configured range.
>
> Fixes: f5f633b18234 ("thermal/drivers/mediatek: Add the Low Voltage Thermal Sensor driver")
> Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Il 06/07/23 17:37, Nícolas F. R. A. Prado ha scritto:
> Out of the many interrupts supported by the hardware, the only ones of
> interest to the driver currently are:
> * The temperature went over the high offset threshold, for any of the
> sensors
> * The temperature went below the low offset threshold, for any of the
> sensors
> * The temperature went over the stage3 threshold
>
> These are the only thresholds configured by the driver through the
> OFFSETH, OFFSETL, and PROTTC registers, respectively.
>
> The current interrupt mask in LVTS_MONINT_CONF, enables many more
> interrupts, including data ready on sensors for both filtered and
> immediate mode. These are not only not handled by the driver, but they
> are also triggered too often, causing unneeded overhead. Disable these
> unnecessary interrupts.
>
> The meaning of each bit can be seen in the comment describing
> LVTS_MONINTST in the IRQ handler.
>
> Fixes: f5f633b18234 ("thermal/drivers/mediatek: Add the Low Voltage Thermal Sensor driver")
> Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Il 06/07/23 17:37, Nícolas F. R. A. Prado ha scritto:
> There are two kinds of temperature monitoring interrupts available:
> * High Offset, Low Offset
> * Hot, Hot to normal, Cold
>
> The code currently uses the hot/h2n/cold interrupts, however in a way
> that doesn't work: the cold threshold is left uninitialized, which
> prevents the other thresholds from ever triggering, and the h2n
> interrupt is used as the lower threshold, which prevents the hot
> interrupt from triggering again after the thresholds are updated by the
> thermal framework, since a hot interrupt can only trigger again after
> the hot to normal interrupt has been triggered.
>
> But better yet than addressing those issues, is to use the high/low
> offset interrupts instead. This way only two thresholds need to be
> managed, which have a simpler state machine, making them a better match
> to the thermal framework's high and low thresholds.
>
> Fixes: f5f633b18234 ("thermal/drivers/mediatek: Add the Low Voltage Thermal Sensor driver")
> Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
On 06/07/2023 17:37, Nícolas F. R. A. Prado wrote:
> There is a single IRQ handler for each LVTS thermal domain, and it is
> supposed to check each of its underlying controllers for the origin of
> the interrupt and clear its status. However due to a typo, only the
> first controller was ever being handled, which resulted in the interrupt
> never being cleared when it happened on the other controllers. Add the
> missing index so interrupts are handled for all controllers.
Reviewed-by: Alexandre Mergnat <[email protected]>
--
Regards,
Alexandre
On 06/07/2023 17:37, Nícolas F. R. A. Prado wrote:
> There are two kinds of temperature monitoring interrupts available:
> * High Offset, Low Offset
> * Hot, Hot to normal, Cold
>
> The code currently uses the hot/h2n/cold interrupts, however in a way
> that doesn't work: the cold threshold is left uninitialized, which
> prevents the other thresholds from ever triggering, and the h2n
> interrupt is used as the lower threshold, which prevents the hot
> interrupt from triggering again after the thresholds are updated by the
> thermal framework, since a hot interrupt can only trigger again after
> the hot to normal interrupt has been triggered.
>
> But better yet than addressing those issues, is to use the high/low
> offset interrupts instead. This way only two thresholds need to be
> managed, which have a simpler state machine, making them a better match
> to the thermal framework's high and low thresholds.
Reviewed-by: Alexandre Mergnat <[email protected]>
--
Regards,
Alexandre
On 06/07/2023 17:37, Nícolas F. R. A. Prado wrote:
> Out of the many interrupts supported by the hardware, the only ones of
> interest to the driver currently are:
> * The temperature went over the high offset threshold, for any of the
> sensors
> * The temperature went below the low offset threshold, for any of the
> sensors
> * The temperature went over the stage3 threshold
>
> These are the only thresholds configured by the driver through the
> OFFSETH, OFFSETL, and PROTTC registers, respectively.
>
> The current interrupt mask in LVTS_MONINT_CONF, enables many more
> interrupts, including data ready on sensors for both filtered and
> immediate mode. These are not only not handled by the driver, but they
> are also triggered too often, causing unneeded overhead. Disable these
> unnecessary interrupts.
>
> The meaning of each bit can be seen in the comment describing
> LVTS_MONINTST in the IRQ handler.
Reviewed-by: Alexandre Mergnat <[email protected]>
--
Regards,
Alexandre
On 06/07/2023 17:37, Nícolas F. R. A. Prado wrote:
> Each LVTS thermal controller can have up to four sensors, each capable
> of triggering its own interrupt when its measured temperature crosses
> the configured threshold. The threshold for each sensor is handled
> separately by the thermal framework, since each one is registered with
> its own thermal zone and trips. However, the temperature thresholds are
> configured on the controller, and therefore are shared between all
> sensors on that controller.
>
> When the temperature measured by the sensors is different enough to
> cause the thermal framework to configure different thresholds for each
> one, interrupts start triggering on sensors outside the last threshold
> configured.
>
> To address the issue, track the thresholds required by each sensor and
> only actually set the highest one in the hardware, and disable
> interrupts for all sensors outside the current configured range.
Reviewed-by: Alexandre Mergnat <[email protected]>
--
Regards,
Alexandre
On 06/07/2023 17:37, Nícolas F. R. A. Prado wrote:
>
> This series fixes interrupt handling in the LVTS thermal driver. It not
> only solves the interrupt storms currently happening, but also allows
> the temperature monitoring interrupts to trigger when the thermal trip
> temperature is reached.
>
> This series was tested together with the MT8192 support series [1] on
> the Spherion Chromebook (mt8192-asurada-spherion).
>
> These are standalone fixes and don't depend on anything else.
>
> [1] https://lore.kernel.org/all/[email protected]
Applied, thanks
--
<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