[PATCH] thermal/drivers/mediatek/lvts_thermal: Switch to IMMEDIATE_MODE

Hsin-Te Yuan posted 1 patch 1 year, 1 month ago
drivers/thermal/mediatek/lvts_thermal.c | 3 ---
1 file changed, 3 deletions(-)
[PATCH] thermal/drivers/mediatek/lvts_thermal: Switch to IMMEDIATE_MODE
Posted by Hsin-Te Yuan 1 year, 1 month ago
Currently, MT8192 cannot suspend with FILTERED_MODE. Switch to
IMMEDIATE_MODE will fix this.

Signed-off-by: Hsin-Te Yuan <yuanhsinte@chromium.org>
---
 drivers/thermal/mediatek/lvts_thermal.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index 1997e91bb3be94a3059db619238aa5787edc7675..daad52f14fc03d0c4131f2ffdf3eb6b49a4a43d0 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -1541,7 +1541,6 @@ static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
 		},
 		VALID_SENSOR_MAP(1, 1, 0, 0),
 		.offset = 0x0,
-		.mode = LVTS_MSR_FILTERED_MODE,
 	},
 	{
 		.lvts_sensor = {
@@ -1552,7 +1551,6 @@ static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
 		},
 		VALID_SENSOR_MAP(1, 1, 0, 0),
 		.offset = 0x100,
-		.mode = LVTS_MSR_FILTERED_MODE,
 	},
 	{
 		.lvts_sensor = {
@@ -1567,7 +1565,6 @@ static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
 		},
 		VALID_SENSOR_MAP(1, 1, 1, 1),
 		.offset = 0x200,
-		.mode = LVTS_MSR_FILTERED_MODE,
 	}
 };
 

---
base-commit: 906bd684e4b1e517dd424a354744c5b0aebef8af
change-id: 20241108-lvts-f7beb36efc59

Best regards,
-- 
Hsin-Te Yuan <yuanhsinte@chromium.org>
Re: [PATCH] thermal/drivers/mediatek/lvts_thermal: Switch to IMMEDIATE_MODE
Posted by Daniel Lezcano 1 year, 1 month ago
On 08/11/2024 07:46, Hsin-Te Yuan wrote:
> Currently, MT8192 cannot suspend with FILTERED_MODE. Switch to
> IMMEDIATE_MODE will fix this.

Given the comments and the different changes related to the filtered and 
immediate modes, I won't apply the patch until this is clearly sorted out.

> Signed-off-by: Hsin-Te Yuan <yuanhsinte@chromium.org>
> ---
>   drivers/thermal/mediatek/lvts_thermal.c | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
> index 1997e91bb3be94a3059db619238aa5787edc7675..daad52f14fc03d0c4131f2ffdf3eb6b49a4a43d0 100644
> --- a/drivers/thermal/mediatek/lvts_thermal.c
> +++ b/drivers/thermal/mediatek/lvts_thermal.c
> @@ -1541,7 +1541,6 @@ static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
>   		},
>   		VALID_SENSOR_MAP(1, 1, 0, 0),
>   		.offset = 0x0,
> -		.mode = LVTS_MSR_FILTERED_MODE,
>   	},
>   	{
>   		.lvts_sensor = {
> @@ -1552,7 +1551,6 @@ static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
>   		},
>   		VALID_SENSOR_MAP(1, 1, 0, 0),
>   		.offset = 0x100,
> -		.mode = LVTS_MSR_FILTERED_MODE,
>   	},
>   	{
>   		.lvts_sensor = {
> @@ -1567,7 +1565,6 @@ static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
>   		},
>   		VALID_SENSOR_MAP(1, 1, 1, 1),
>   		.offset = 0x200,
> -		.mode = LVTS_MSR_FILTERED_MODE,
>   	}
>   };
>   
> 
> ---
> base-commit: 906bd684e4b1e517dd424a354744c5b0aebef8af
> change-id: 20241108-lvts-f7beb36efc59
> 
> Best regards,


-- 
<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
Re: [PATCH] thermal/drivers/mediatek/lvts_thermal: Switch to IMMEDIATE_MODE
Posted by AngeloGioacchino Del Regno 1 year, 1 month ago
Il 14/11/24 09:13, Daniel Lezcano ha scritto:
> On 08/11/2024 07:46, Hsin-Te Yuan wrote:
>> Currently, MT8192 cannot suspend with FILTERED_MODE. Switch to
>> IMMEDIATE_MODE will fix this.
> 
> Given the comments and the different changes related to the filtered and immediate 
> modes, I won't apply the patch until this is clearly sorted out.
> 

Thanks - indeed do not apply this for now.

If this has been broken for a year, one or two more weeks ain't gonna be a
show stopper, as long as we can come to a proper fix during that time, instead
of yet another compromise.

Anyway....

We discussed about that internally and we do suspect that there might be some
interrupt that is getting erroneously enabled, never handled, and constantly
firing because.. well, it was never handled, so... :-)

Nicolas is looking into this issue, and I really hope that we can come to a
conclusion that will make us switch back all of the SoCs to filtered mode
instead, as there indeed are some advantages in using that.

Cheers,
Angelo

>> Signed-off-by: Hsin-Te Yuan <yuanhsinte@chromium.org>
>> ---
>>   drivers/thermal/mediatek/lvts_thermal.c | 3 ---
>>   1 file changed, 3 deletions(-)
>>
>> diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/ 
>> lvts_thermal.c
>> index 
>> 1997e91bb3be94a3059db619238aa5787edc7675..daad52f14fc03d0c4131f2ffdf3eb6b49a4a43d0 100644
>> --- a/drivers/thermal/mediatek/lvts_thermal.c
>> +++ b/drivers/thermal/mediatek/lvts_thermal.c
>> @@ -1541,7 +1541,6 @@ static const struct lvts_ctrl_data 
>> mt8192_lvts_mcu_data_ctrl[] = {
>>           },
>>           VALID_SENSOR_MAP(1, 1, 0, 0),
>>           .offset = 0x0,
>> -        .mode = LVTS_MSR_FILTERED_MODE,
>>       },
>>       {
>>           .lvts_sensor = {
>> @@ -1552,7 +1551,6 @@ static const struct lvts_ctrl_data 
>> mt8192_lvts_mcu_data_ctrl[] = {
>>           },
>>           VALID_SENSOR_MAP(1, 1, 0, 0),
>>           .offset = 0x100,
>> -        .mode = LVTS_MSR_FILTERED_MODE,
>>       },
>>       {
>>           .lvts_sensor = {
>> @@ -1567,7 +1565,6 @@ static const struct lvts_ctrl_data 
>> mt8192_lvts_mcu_data_ctrl[] = {
>>           },
>>           VALID_SENSOR_MAP(1, 1, 1, 1),
>>           .offset = 0x200,
>> -        .mode = LVTS_MSR_FILTERED_MODE,
>>       }
>>   };
>>
>> ---
>> base-commit: 906bd684e4b1e517dd424a354744c5b0aebef8af
>> change-id: 20241108-lvts-f7beb36efc59
>>
>> Best regards,
> 
> 


Re: [PATCH] thermal/drivers/mediatek/lvts_thermal: Switch to IMMEDIATE_MODE
Posted by Chen-Yu Tsai 1 year, 1 month ago
On Fri, Nov 8, 2024 at 2:51 PM Hsin-Te Yuan <yuanhsinte@chromium.org> wrote:
>
> Currently, MT8192 cannot suspend with FILTERED_MODE. Switch to
> IMMEDIATE_MODE will fix this.
>

Probably should have a Fixes tag.

Also, Nicolas previously reported that the threshold interrupts don't
work with the immediate mode [1], which is why filtered mode was used
in the final version.


ChenYu

[1] https://lore.kernel.org/linux-mediatek/37680c5e-e61c-410b-b48d-829914200e4a@notapiano/

> Signed-off-by: Hsin-Te Yuan <yuanhsinte@chromium.org>
> ---
>  drivers/thermal/mediatek/lvts_thermal.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
> index 1997e91bb3be94a3059db619238aa5787edc7675..daad52f14fc03d0c4131f2ffdf3eb6b49a4a43d0 100644
> --- a/drivers/thermal/mediatek/lvts_thermal.c
> +++ b/drivers/thermal/mediatek/lvts_thermal.c
> @@ -1541,7 +1541,6 @@ static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
>                 },
>                 VALID_SENSOR_MAP(1, 1, 0, 0),
>                 .offset = 0x0,
> -               .mode = LVTS_MSR_FILTERED_MODE,
>         },
>         {
>                 .lvts_sensor = {
> @@ -1552,7 +1551,6 @@ static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
>                 },
>                 VALID_SENSOR_MAP(1, 1, 0, 0),
>                 .offset = 0x100,
> -               .mode = LVTS_MSR_FILTERED_MODE,
>         },
>         {
>                 .lvts_sensor = {
> @@ -1567,7 +1565,6 @@ static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
>                 },
>                 VALID_SENSOR_MAP(1, 1, 1, 1),
>                 .offset = 0x200,
> -               .mode = LVTS_MSR_FILTERED_MODE,
>         }
>  };
>
>
> ---
> base-commit: 906bd684e4b1e517dd424a354744c5b0aebef8af
> change-id: 20241108-lvts-f7beb36efc59
>
> Best regards,
> --
> Hsin-Te Yuan <yuanhsinte@chromium.org>
>
>
Re: [PATCH] thermal/drivers/mediatek/lvts_thermal: Switch to IMMEDIATE_MODE
Posted by Nícolas F. R. A. Prado 1 year, 1 month ago
On Fri, Nov 08, 2024 at 04:21:07PM +0800, Chen-Yu Tsai wrote:
> On Fri, Nov 8, 2024 at 2:51 PM Hsin-Te Yuan <yuanhsinte@chromium.org> wrote:
> >
> > Currently, MT8192 cannot suspend with FILTERED_MODE. Switch to
> > IMMEDIATE_MODE will fix this.

Do you mean that the whole MT8192 SoC is not able to enter system suspend? I
just tested that on a fairly recent linux-next and it was working without a
problem. We need more context here.

> >
> 
> Probably should have a Fixes tag.
> 
> Also, Nicolas previously reported that the threshold interrupts don't
> work with the immediate mode [1], which is why filtered mode was used
> in the final version.

Indeed. I suppose it would be possible to configure immediate mode only during
suspend if this is really the problem.

Thanks,
Nícolas