| Message ID | 20241108-lvts-v1-1-eee339c6ca20@chromium.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | thermal/drivers/mediatek/lvts_thermal: Switch to IMMEDIATE_MODE | expand |
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> > >
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
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,
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, > >
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, } };
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(-) --- base-commit: 906bd684e4b1e517dd424a354744c5b0aebef8af change-id: 20241108-lvts-f7beb36efc59 Best regards,