linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] thermal/drivers/mediatek/lvts_thermal: Fixes to the interrupt handling
@ 2023-04-28 19:53 Nícolas F. R. A. Prado
  2023-04-28 19:53 ` [PATCH 1/3] thermal/drivers/mediatek/lvts_thermal: Handle IRQ on all controllers Nícolas F. R. A. Prado
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Nícolas F. R. A. Prado @ 2023-04-28 19:53 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Alexandre Bailon, kernel, Chen-Yu Tsai, Balsam CHIHI,
	AngeloGioacchino Del Regno, Alexandre Mergnat,
	Nícolas F. R. A. Prado, Amit Kucheria, Matthias Brugger,
	Rafael J. Wysocki, Zhang Rui, linux-arm-kernel, linux-kernel,
	linux-mediatek, linux-pm


Fixes in the interrupt handling of the LVTS thermal driver noticed while
testing it on the Spherion Chromebook (mt8192-asurada-spherion) with the
MT8192 support series [1].

These are standalone fixes and don't depend on anything else.

[1] https://lore.kernel.org/all/20230307163413.143334-1-bchihi@baylibre.com/

Thanks,
Nícolas


Nícolas F. R. A. Prado (3):
  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: Disable undesired interrupts

 drivers/thermal/mediatek/lvts_thermal.c | 54 +++++++++++++------------
 1 file changed, 28 insertions(+), 26 deletions(-)

-- 
2.40.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/3] thermal/drivers/mediatek/lvts_thermal: Handle IRQ on all controllers
  2023-04-28 19:53 [PATCH 0/3] thermal/drivers/mediatek/lvts_thermal: Fixes to the interrupt handling Nícolas F. R. A. Prado
@ 2023-04-28 19:53 ` Nícolas F. R. A. Prado
  2023-05-02 10:00   ` Matthias Brugger
  2023-05-03 15:08   ` AngeloGioacchino Del Regno
  2023-04-28 19:53 ` [PATCH 2/3] thermal/drivers/mediatek/lvts_thermal: Honor sensors in immediate mode Nícolas F. R. A. Prado
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 13+ messages in thread
From: Nícolas F. R. A. Prado @ 2023-04-28 19:53 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Alexandre Bailon, kernel, Chen-Yu Tsai, Balsam CHIHI,
	AngeloGioacchino Del Regno, Alexandre Mergnat,
	Nícolas F. R. A. Prado, Amit Kucheria, Matthias Brugger,
	Rafael J. Wysocki, Zhang Rui, linux-arm-kernel, linux-kernel,
	linux-mediatek, linux-pm

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")
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

---

 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 d0a3f95b7884..56b24c5b645f 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -449,7 +449,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.40.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/3] thermal/drivers/mediatek/lvts_thermal: Honor sensors in immediate mode
  2023-04-28 19:53 [PATCH 0/3] thermal/drivers/mediatek/lvts_thermal: Fixes to the interrupt handling Nícolas F. R. A. Prado
  2023-04-28 19:53 ` [PATCH 1/3] thermal/drivers/mediatek/lvts_thermal: Handle IRQ on all controllers Nícolas F. R. A. Prado
@ 2023-04-28 19:53 ` Nícolas F. R. A. Prado
  2023-05-03 15:08   ` AngeloGioacchino Del Regno
  2023-04-28 19:53 ` [PATCH 3/3] thermal/drivers/mediatek/lvts_thermal: Disable undesired interrupts Nícolas F. R. A. Prado
  2023-05-02 10:33 ` [PATCH 0/3] thermal/drivers/mediatek/lvts_thermal: Fixes to the interrupt handling Chen-Yu Tsai
  3 siblings, 1 reply; 13+ messages in thread
From: Nícolas F. R. A. Prado @ 2023-04-28 19:53 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Alexandre Bailon, kernel, Chen-Yu Tsai, Balsam CHIHI,
	AngeloGioacchino Del Regno, Alexandre Mergnat,
	Nícolas F. R. A. Prado, Amit Kucheria, Matthias Brugger,
	Rafael J. Wysocki, Zhang Rui, linux-arm-kernel, linux-kernel,
	linux-mediatek, linux-pm

Each controller can be configured to operate on immediate or filtered
mode. On filtered mode, the sensors are enabled by setting the
corresponding bits in MONCTL0, while on immediate mode, by setting
MSRCTL1.

Previously, the code would set MSRCTL1 for all four sensors when
configured to immediate mode, but given that the controller might not
have all four sensors connected, this would cause interrupts to trigger
for non-existent sensors. Fix this by handling the MSRCTL1 register
analogously to the MONCTL0: only enable the sensors that were declared.

Fixes: f5f633b18234 ("thermal/drivers/mediatek: Add the Low Voltage Thermal Sensor driver")
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---

 drivers/thermal/mediatek/lvts_thermal.c | 50 +++++++++++++------------
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index 56b24c5b645f..29d10eaa8dfc 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -894,24 +894,6 @@ static int lvts_ctrl_configure(struct device *dev, struct lvts_ctrl *lvts_ctrl)
 			LVTS_HW_FILTER << 3 | LVTS_HW_FILTER;
 	writel(value, LVTS_MSRCTL0(lvts_ctrl->base));
 
-	/*
-	 * LVTS_MSRCTL1 : Measurement control
-	 *
-	 * Bits:
-	 *
-	 * 9: Ignore MSRCTL0 config and do immediate measurement on sensor3
-	 * 6: Ignore MSRCTL0 config and do immediate measurement on sensor2
-	 * 5: Ignore MSRCTL0 config and do immediate measurement on sensor1
-	 * 4: Ignore MSRCTL0 config and do immediate measurement on sensor0
-	 *
-	 * That configuration will ignore the filtering and the delays
-	 * introduced below in MONCTL1 and MONCTL2
-	 */
-	if (lvts_ctrl->mode == LVTS_MSR_IMMEDIATE_MODE) {
-		value = BIT(9) | BIT(6) | BIT(5) | BIT(4);
-		writel(value, LVTS_MSRCTL1(lvts_ctrl->base));
-	}
-
 	/*
 	 * LVTS_MONCTL1 : Period unit and group interval configuration
 	 *
@@ -976,6 +958,8 @@ static int lvts_ctrl_start(struct device *dev, struct lvts_ctrl *lvts_ctrl)
 	struct lvts_sensor *lvts_sensors = lvts_ctrl->sensors;
 	struct thermal_zone_device *tz;
 	u32 sensor_map = 0;
+	u32 sensor_map_bits[][4] = {{BIT(4), BIT(5), BIT(6), BIT(9)},
+				    {BIT(0), BIT(1), BIT(2), BIT(3)}};
 	int i;
 
 	for (i = 0; i < lvts_ctrl->num_lvts_sensor; i++) {
@@ -1012,20 +996,38 @@ static int lvts_ctrl_start(struct device *dev, struct lvts_ctrl *lvts_ctrl)
 		 * map, so we can enable the temperature monitoring in
 		 * the hardware thermal controller.
 		 */
-		sensor_map |= BIT(i);
+		sensor_map |= sensor_map_bits[lvts_ctrl->mode][i];
 	}
 
 	/*
-	 * Bits:
-	 *      9: Single point access flow
-	 *    0-3: Enable sensing point 0-3
-	 *
 	 * The initialization of the thermal zones give us
 	 * which sensor point to enable. If any thermal zone
 	 * was not described in the device tree, it won't be
 	 * enabled here in the sensor map.
 	 */
-	writel(sensor_map | BIT(9), LVTS_MONCTL0(lvts_ctrl->base));
+	if (lvts_ctrl->mode == LVTS_MSR_IMMEDIATE_MODE) {
+		/*
+		 * LVTS_MSRCTL1 : Measurement control
+		 *
+		 * Bits:
+		 *
+		 * 9: Ignore MSRCTL0 config and do immediate measurement on sensor3
+		 * 6: Ignore MSRCTL0 config and do immediate measurement on sensor2
+		 * 5: Ignore MSRCTL0 config and do immediate measurement on sensor1
+		 * 4: Ignore MSRCTL0 config and do immediate measurement on sensor0
+		 *
+		 * That configuration will ignore the filtering and the delays
+		 * introduced in MONCTL1 and MONCTL2
+		 */
+		writel(sensor_map, LVTS_MSRCTL1(lvts_ctrl->base));
+	} else {
+		/*
+		 * Bits:
+		 *      9: Single point access flow
+		 *    0-3: Enable sensing point 0-3
+		 */
+		writel(sensor_map | BIT(9), LVTS_MONCTL0(lvts_ctrl->base));
+	}
 
 	return 0;
 }
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/3] thermal/drivers/mediatek/lvts_thermal: Disable undesired interrupts
  2023-04-28 19:53 [PATCH 0/3] thermal/drivers/mediatek/lvts_thermal: Fixes to the interrupt handling Nícolas F. R. A. Prado
  2023-04-28 19:53 ` [PATCH 1/3] thermal/drivers/mediatek/lvts_thermal: Handle IRQ on all controllers Nícolas F. R. A. Prado
  2023-04-28 19:53 ` [PATCH 2/3] thermal/drivers/mediatek/lvts_thermal: Honor sensors in immediate mode Nícolas F. R. A. Prado
@ 2023-04-28 19:53 ` Nícolas F. R. A. Prado
  2023-05-03 15:12   ` AngeloGioacchino Del Regno
  2023-05-02 10:33 ` [PATCH 0/3] thermal/drivers/mediatek/lvts_thermal: Fixes to the interrupt handling Chen-Yu Tsai
  3 siblings, 1 reply; 13+ messages in thread
From: Nícolas F. R. A. Prado @ 2023-04-28 19:53 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Alexandre Bailon, kernel, Chen-Yu Tsai, Balsam CHIHI,
	AngeloGioacchino Del Regno, Alexandre Mergnat,
	Nícolas F. R. A. Prado, Amit Kucheria, Matthias Brugger,
	Rafael J. Wysocki, Zhang Rui, linux-arm-kernel, linux-kernel,
	linux-mediatek, linux-pm

Out of the many interrupts supported by the hardware, the only ones of
interest to the driver currently are:
* The temperature went over the hot threshold, for any of the sensors
* The temperature went over the hot to normal threshold, for any of the
  sensors
* The temperature went over the stage3 threshold

These are the only thresholds configured by the driver through the
HTHRE, H2NTHRE, and PROTTC registers, respectively.

The current interrupt mask in LVTS_MONINT_CONF, enables many more
interrupts, including offset detection and 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 <nfraprado@collabora.com>
---

 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 29d10eaa8dfc..300ce22a7471 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -63,7 +63,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			0x84804A52
 
 #define LVTS_INT_SENSOR0			0x0009001F
 #define LVTS_INT_SENSOR1			0x001203E0
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] thermal/drivers/mediatek/lvts_thermal: Handle IRQ on all controllers
  2023-04-28 19:53 ` [PATCH 1/3] thermal/drivers/mediatek/lvts_thermal: Handle IRQ on all controllers Nícolas F. R. A. Prado
@ 2023-05-02 10:00   ` Matthias Brugger
  2023-05-03 15:08   ` AngeloGioacchino Del Regno
  1 sibling, 0 replies; 13+ messages in thread
From: Matthias Brugger @ 2023-05-02 10:00 UTC (permalink / raw)
  To: Nícolas F. R. A. Prado, Daniel Lezcano
  Cc: Alexandre Bailon, kernel, Chen-Yu Tsai, Balsam CHIHI,
	AngeloGioacchino Del Regno, Alexandre Mergnat, Amit Kucheria,
	Rafael J. Wysocki, Zhang Rui, linux-arm-kernel, linux-kernel,
	linux-mediatek, linux-pm



On 28/04/2023 21:53, 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.
> 
> Fixes: f5f633b18234 ("thermal/drivers/mediatek: Add the Low Voltage Thermal Sensor driver")
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> 

Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>

> ---
> 
>   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 d0a3f95b7884..56b24c5b645f 100644
> --- a/drivers/thermal/mediatek/lvts_thermal.c
> +++ b/drivers/thermal/mediatek/lvts_thermal.c
> @@ -449,7 +449,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;
>   

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] thermal/drivers/mediatek/lvts_thermal: Fixes to the interrupt handling
  2023-04-28 19:53 [PATCH 0/3] thermal/drivers/mediatek/lvts_thermal: Fixes to the interrupt handling Nícolas F. R. A. Prado
                   ` (2 preceding siblings ...)
  2023-04-28 19:53 ` [PATCH 3/3] thermal/drivers/mediatek/lvts_thermal: Disable undesired interrupts Nícolas F. R. A. Prado
@ 2023-05-02 10:33 ` Chen-Yu Tsai
  2023-05-30 12:27   ` Daniel Lezcano
  3 siblings, 1 reply; 13+ messages in thread
From: Chen-Yu Tsai @ 2023-05-02 10:33 UTC (permalink / raw)
  To: Nícolas F. R. A. Prado
  Cc: Daniel Lezcano, Alexandre Bailon, kernel, Balsam CHIHI,
	AngeloGioacchino Del Regno, Alexandre Mergnat, Amit Kucheria,
	Matthias Brugger, Rafael J. Wysocki, Zhang Rui, linux-arm-kernel,
	linux-kernel, linux-mediatek, linux-pm

On Fri, Apr 28, 2023 at 03:53:44PM -0400, Nícolas F. R. A. Prado wrote:
> 
> Fixes in the interrupt handling of the LVTS thermal driver noticed while
> testing it on the Spherion Chromebook (mt8192-asurada-spherion) with the
> MT8192 support series [1].
> 
> These are standalone fixes and don't depend on anything else.
> 
> [1] https://lore.kernel.org/all/20230307163413.143334-1-bchihi@baylibre.com/
> 
> Thanks,
> Nícolas
> 
> 
> Nícolas F. R. A. Prado (3):
>   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: Disable undesired interrupts

This series seems to have solved all interrupt storm issue I ran into, so

Tested-by: Chen-Yu Tsai <wenst@chromium.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] thermal/drivers/mediatek/lvts_thermal: Handle IRQ on all controllers
  2023-04-28 19:53 ` [PATCH 1/3] thermal/drivers/mediatek/lvts_thermal: Handle IRQ on all controllers Nícolas F. R. A. Prado
  2023-05-02 10:00   ` Matthias Brugger
@ 2023-05-03 15:08   ` AngeloGioacchino Del Regno
  1 sibling, 0 replies; 13+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-05-03 15:08 UTC (permalink / raw)
  To: Nícolas F. R. A. Prado, Daniel Lezcano
  Cc: Alexandre Bailon, kernel, Chen-Yu Tsai, Balsam CHIHI,
	Alexandre Mergnat, Amit Kucheria, Matthias Brugger,
	Rafael J. Wysocki, Zhang Rui, linux-arm-kernel, linux-kernel,
	linux-mediatek, linux-pm

Il 28/04/23 21:53, Nícolas F. R. A. Prado ha scritto:
> 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")
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] thermal/drivers/mediatek/lvts_thermal: Honor sensors in immediate mode
  2023-04-28 19:53 ` [PATCH 2/3] thermal/drivers/mediatek/lvts_thermal: Honor sensors in immediate mode Nícolas F. R. A. Prado
@ 2023-05-03 15:08   ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 13+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-05-03 15:08 UTC (permalink / raw)
  To: Nícolas F. R. A. Prado, Daniel Lezcano
  Cc: Alexandre Bailon, kernel, Chen-Yu Tsai, Balsam CHIHI,
	Alexandre Mergnat, Amit Kucheria, Matthias Brugger,
	Rafael J. Wysocki, Zhang Rui, linux-arm-kernel, linux-kernel,
	linux-mediatek, linux-pm

Il 28/04/23 21:53, Nícolas F. R. A. Prado ha scritto:
> Each controller can be configured to operate on immediate or filtered
> mode. On filtered mode, the sensors are enabled by setting the
> corresponding bits in MONCTL0, while on immediate mode, by setting
> MSRCTL1.
> 
> Previously, the code would set MSRCTL1 for all four sensors when
> configured to immediate mode, but given that the controller might not
> have all four sensors connected, this would cause interrupts to trigger
> for non-existent sensors. Fix this by handling the MSRCTL1 register
> analogously to the MONCTL0: only enable the sensors that were declared.
> 
> Fixes: f5f633b18234 ("thermal/drivers/mediatek: Add the Low Voltage Thermal Sensor driver")
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Awesome!!!!!

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] thermal/drivers/mediatek/lvts_thermal: Disable undesired interrupts
  2023-04-28 19:53 ` [PATCH 3/3] thermal/drivers/mediatek/lvts_thermal: Disable undesired interrupts Nícolas F. R. A. Prado
@ 2023-05-03 15:12   ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 13+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-05-03 15:12 UTC (permalink / raw)
  To: Nícolas F. R. A. Prado, Daniel Lezcano
  Cc: Alexandre Bailon, kernel, Chen-Yu Tsai, Balsam CHIHI,
	Alexandre Mergnat, Amit Kucheria, Matthias Brugger,
	Rafael J. Wysocki, Zhang Rui, linux-arm-kernel, linux-kernel,
	linux-mediatek, linux-pm

Il 28/04/23 21:53, 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 hot threshold, for any of the sensors
> * The temperature went over the hot to normal threshold, for any of the
>    sensors
> * The temperature went over the stage3 threshold
> 
> These are the only thresholds configured by the driver through the
> HTHRE, H2NTHRE, and PROTTC registers, respectively.
> 
> The current interrupt mask in LVTS_MONINT_CONF, enables many more
> interrupts, including offset detection and 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 <nfraprado@collabora.com>


Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] thermal/drivers/mediatek/lvts_thermal: Fixes to the interrupt handling
  2023-05-02 10:33 ` [PATCH 0/3] thermal/drivers/mediatek/lvts_thermal: Fixes to the interrupt handling Chen-Yu Tsai
@ 2023-05-30 12:27   ` Daniel Lezcano
  2023-05-30 19:46     ` Nícolas F. R. A. Prado
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel Lezcano @ 2023-05-30 12:27 UTC (permalink / raw)
  To: Chen-Yu Tsai, Nícolas F. R. A. Prado
  Cc: Alexandre Bailon, kernel, Balsam CHIHI,
	AngeloGioacchino Del Regno, Alexandre Mergnat, Amit Kucheria,
	Matthias Brugger, Rafael J. Wysocki, Zhang Rui, linux-arm-kernel,
	linux-kernel, linux-mediatek, linux-pm


Hi,

On 02/05/2023 12:33, Chen-Yu Tsai wrote:
> On Fri, Apr 28, 2023 at 03:53:44PM -0400, Nícolas F. R. A. Prado wrote:
>>
>> Fixes in the interrupt handling of the LVTS thermal driver noticed while
>> testing it on the Spherion Chromebook (mt8192-asurada-spherion) with the
>> MT8192 support series [1].
>>
>> These are standalone fixes and don't depend on anything else.
>>
>> [1] https://lore.kernel.org/all/20230307163413.143334-1-bchihi@baylibre.com/
>>
>> Thanks,
>> Nícolas
>>
>>
>> Nícolas F. R. A. Prado (3):
>>    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: Disable undesired interrupts
> 
> This series seems to have solved all interrupt storm issue I ran into, so
> 
> Tested-by: Chen-Yu Tsai <wenst@chromium.org>

I gave a try on a mt8195 board and I don't see any interrupt firing when 
crossing the temperature thresholds.

Did I miss something ?

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] thermal/drivers/mediatek/lvts_thermal: Fixes to the interrupt handling
  2023-05-30 12:27   ` Daniel Lezcano
@ 2023-05-30 19:46     ` Nícolas F. R. A. Prado
  2023-06-02  8:07       ` Daniel Lezcano
  0 siblings, 1 reply; 13+ messages in thread
From: Nícolas F. R. A. Prado @ 2023-05-30 19:46 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Chen-Yu Tsai, Alexandre Bailon, kernel, Balsam CHIHI,
	AngeloGioacchino Del Regno, Alexandre Mergnat, Amit Kucheria,
	Matthias Brugger, Rafael J. Wysocki, Zhang Rui, linux-arm-kernel,
	linux-kernel, linux-mediatek, linux-pm

On Tue, May 30, 2023 at 02:27:36PM +0200, Daniel Lezcano wrote:
> 
> Hi,
> 
> On 02/05/2023 12:33, Chen-Yu Tsai wrote:
> > On Fri, Apr 28, 2023 at 03:53:44PM -0400, Nícolas F. R. A. Prado wrote:
> > > 
> > > Fixes in the interrupt handling of the LVTS thermal driver noticed while
> > > testing it on the Spherion Chromebook (mt8192-asurada-spherion) with the
> > > MT8192 support series [1].
> > > 
> > > These are standalone fixes and don't depend on anything else.
> > > 
> > > [1] https://lore.kernel.org/all/20230307163413.143334-1-bchihi@baylibre.com/
> > > 
> > > Thanks,
> > > Nícolas
> > > 
> > > 
> > > Nícolas F. R. A. Prado (3):
> > >    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: Disable undesired interrupts
> > 
> > This series seems to have solved all interrupt storm issue I ran into, so
> > 
> > Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> 
> I gave a try on a mt8195 board and I don't see any interrupt firing when
> crossing the temperature thresholds.
> 
> Did I miss something ?

No, indeed interrupts seem to be completely disabled on mt8195, even after
setting the controllers to filtered mode (a requirement to get interrupts). I
haven't investigated that further yet. This series was validated on mt8192,
which did have working interrupts, but they were being triggered too often.

Also note that I've sent a v2 with even more fixes:
https://lore.kernel.org/all/20230504004852.627049-1-nfraprado@collabora.com/

Thanks,
Nícolas

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] thermal/drivers/mediatek/lvts_thermal: Fixes to the interrupt handling
  2023-05-30 19:46     ` Nícolas F. R. A. Prado
@ 2023-06-02  8:07       ` Daniel Lezcano
  2023-06-02 13:22         ` Nícolas F. R. A. Prado
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel Lezcano @ 2023-06-02  8:07 UTC (permalink / raw)
  To: Nícolas F. R. A. Prado
  Cc: Chen-Yu Tsai, Alexandre Bailon, kernel, Balsam CHIHI,
	AngeloGioacchino Del Regno, Alexandre Mergnat, Amit Kucheria,
	Matthias Brugger, Rafael J. Wysocki, Zhang Rui, linux-arm-kernel,
	linux-kernel, linux-mediatek, linux-pm

On 30/05/2023 21:46, Nícolas F. R. A. Prado wrote:
> On Tue, May 30, 2023 at 02:27:36PM +0200, Daniel Lezcano wrote:
>>
>> Hi,
>>
>> On 02/05/2023 12:33, Chen-Yu Tsai wrote:
>>> On Fri, Apr 28, 2023 at 03:53:44PM -0400, Nícolas F. R. A. Prado wrote:
>>>>
>>>> Fixes in the interrupt handling of the LVTS thermal driver noticed while
>>>> testing it on the Spherion Chromebook (mt8192-asurada-spherion) with the
>>>> MT8192 support series [1].
>>>>
>>>> These are standalone fixes and don't depend on anything else.
>>>>
>>>> [1] https://lore.kernel.org/all/20230307163413.143334-1-bchihi@baylibre.com/
>>>>
>>>> Thanks,
>>>> Nícolas
>>>>
>>>>
>>>> Nícolas F. R. A. Prado (3):
>>>>     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: Disable undesired interrupts
>>>
>>> This series seems to have solved all interrupt storm issue I ran into, so
>>>
>>> Tested-by: Chen-Yu Tsai <wenst@chromium.org>
>>
>> I gave a try on a mt8195 board and I don't see any interrupt firing when
>> crossing the temperature thresholds.
>>
>> Did I miss something ?
> 
> No, indeed interrupts seem to be completely disabled on mt8195, even after
> setting the controllers to filtered mode (a requirement to get interrupts).

Really? interrupts work only on filtered mode? That sounds strange

What board are you using for testing?

> I
> haven't investigated that further yet. This series was validated on mt8192,
> which did have working interrupts, but they were being triggered too often.

Ok.

> Also note that I've sent a v2 with even more fixes:
> https://lore.kernel.org/all/20230504004852.627049-1-nfraprado@collabora.com/

Yes, I'm reviewing it closely

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] thermal/drivers/mediatek/lvts_thermal: Fixes to the interrupt handling
  2023-06-02  8:07       ` Daniel Lezcano
@ 2023-06-02 13:22         ` Nícolas F. R. A. Prado
  0 siblings, 0 replies; 13+ messages in thread
From: Nícolas F. R. A. Prado @ 2023-06-02 13:22 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Chen-Yu Tsai, Alexandre Bailon, kernel, Balsam CHIHI,
	AngeloGioacchino Del Regno, Alexandre Mergnat, Amit Kucheria,
	Matthias Brugger, Rafael J. Wysocki, Zhang Rui, linux-arm-kernel,
	linux-kernel, linux-mediatek, linux-pm

On Fri, Jun 02, 2023 at 10:07:29AM +0200, Daniel Lezcano wrote:
> On 30/05/2023 21:46, Nícolas F. R. A. Prado wrote:
> > On Tue, May 30, 2023 at 02:27:36PM +0200, Daniel Lezcano wrote:
> > > 
> > > Hi,
> > > 
> > > On 02/05/2023 12:33, Chen-Yu Tsai wrote:
> > > > On Fri, Apr 28, 2023 at 03:53:44PM -0400, Nícolas F. R. A. Prado wrote:
> > > > > 
> > > > > Fixes in the interrupt handling of the LVTS thermal driver noticed while
> > > > > testing it on the Spherion Chromebook (mt8192-asurada-spherion) with the
> > > > > MT8192 support series [1].
> > > > > 
> > > > > These are standalone fixes and don't depend on anything else.
> > > > > 
> > > > > [1] https://lore.kernel.org/all/20230307163413.143334-1-bchihi@baylibre.com/
> > > > > 
> > > > > Thanks,
> > > > > Nícolas
> > > > > 
> > > > > 
> > > > > Nícolas F. R. A. Prado (3):
> > > > >     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: Disable undesired interrupts
> > > > 
> > > > This series seems to have solved all interrupt storm issue I ran into, so
> > > > 
> > > > Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> > > 
> > > I gave a try on a mt8195 board and I don't see any interrupt firing when
> > > crossing the temperature thresholds.
> > > 
> > > Did I miss something ?
> > 
> > No, indeed interrupts seem to be completely disabled on mt8195, even after
> > setting the controllers to filtered mode (a requirement to get interrupts).
> 
> Really? interrupts work only on filtered mode? That sounds strange

Sorry my reply was confusing, let me clarify. What I meant to say is that
the threshold interrupts (cold, hot2normal, hot, low offset, high offset) only
trigger in filtered mode. AFAICT that's by design, since immediate mode is meant
only for one-off temperature readings, and filtered mode is the one meant to be
used for temperature monitoring. But in immediate mode you could still get the
data ready for immediate mode (bits 16, 17, 18, 27) interrupts triggering.
Though note that I have disabled those in my series, since they are triggered
constantly.

> 
> What board are you using for testing?

I'm testing on the Acer Chromebook 514 (mt8192-asurada-spherion-r0). And noticed
the interrupts aren't triggered on Acer Chromebook Spin 513
(mt8195-cherry-tomato-r2).

> 
> > I
> > haven't investigated that further yet. This series was validated on mt8192,
> > which did have working interrupts, but they were being triggered too often.
> 
> Ok.
> 
> > Also note that I've sent a v2 with even more fixes:
> > https://lore.kernel.org/all/20230504004852.627049-1-nfraprado@collabora.com/
> 
> Yes, I'm reviewing it closely

Thanks!

Nícolas

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-06-02 13:22 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-28 19:53 [PATCH 0/3] thermal/drivers/mediatek/lvts_thermal: Fixes to the interrupt handling Nícolas F. R. A. Prado
2023-04-28 19:53 ` [PATCH 1/3] thermal/drivers/mediatek/lvts_thermal: Handle IRQ on all controllers Nícolas F. R. A. Prado
2023-05-02 10:00   ` Matthias Brugger
2023-05-03 15:08   ` AngeloGioacchino Del Regno
2023-04-28 19:53 ` [PATCH 2/3] thermal/drivers/mediatek/lvts_thermal: Honor sensors in immediate mode Nícolas F. R. A. Prado
2023-05-03 15:08   ` AngeloGioacchino Del Regno
2023-04-28 19:53 ` [PATCH 3/3] thermal/drivers/mediatek/lvts_thermal: Disable undesired interrupts Nícolas F. R. A. Prado
2023-05-03 15:12   ` AngeloGioacchino Del Regno
2023-05-02 10:33 ` [PATCH 0/3] thermal/drivers/mediatek/lvts_thermal: Fixes to the interrupt handling Chen-Yu Tsai
2023-05-30 12:27   ` Daniel Lezcano
2023-05-30 19:46     ` Nícolas F. R. A. Prado
2023-06-02  8:07       ` Daniel Lezcano
2023-06-02 13:22         ` Nícolas F. R. A. Prado

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).