mbox series

[0/4] Add LVTS support for mt8192

Message ID 20230307163413.143334-1-bchihi@baylibre.com
Headers show
Series Add LVTS support for mt8192 | expand

Message

Balsam CHIHI March 7, 2023, 4:34 p.m. UTC
From: Balsam CHIHI <bchihi@baylibre.com>

Add full LVTS support (MCU thermal domain + AP thermal domain) to MediaTek MT8192 SoC.

This series is a continuation of the previous series "Add LVTS Thermal Architecture" v14 :
    https://patchwork.kernel.org/project/linux-pm/cover/20230209105628.50294-1-bchihi@baylibre.com/
and "Add LVTS's AP thermal domain support for mt8195" :
    https://patchwork.kernel.org/project/linux-pm/cover/20230307154524.118541-1-bchihi@baylibre.com/

Based on top of thermal/linux-next :
    base-commit: 6828e402d06f7c574430b61c05db784cd847b19f

Depends on these patches as they are not yet applyied to thermal/linux-next branch :
    [1/4] dt-bindings: thermal: mediatek: Add AP domain to LVTS thermal controllers for mt8195
    https://patchwork.kernel.org/project/linux-pm/patch/20230307154524.118541-2-bchihi@baylibre.com/
    [2/4] thermal/drivers/mediatek/lvts_thermal: Add AP domain for mt8195
    https://patchwork.kernel.org/project/linux-pm/patch/20230307154524.118541-3-bchihi@baylibre.com/

Balsam CHIHI (4):
  dt-bindings: thermal: mediatek: Add LVTS thermal controller definition
    for mt8192
  thermal/drivers/mediatek/lvts_thermal: Add mt8192 support
  arm64: dts: mediatek: mt8192: Add thermal zones and thermal nodes
  arm64: dts: mediatek: mt8192: Add temperature mitigation threshold

 arch/arm64/boot/dts/mediatek/mt8192.dtsi      | 454 ++++++++++++++++++
 drivers/thermal/mediatek/lvts_thermal.c       | 106 +++-
 .../thermal/mediatek,lvts-thermal.h           |  19 +
 3 files changed, 577 insertions(+), 2 deletions(-)


base-commit: 6828e402d06f7c574430b61c05db784cd847b19f
prerequisite-patch-id: 73be949bd16979769e5b94905b244dcee4a8f687
prerequisite-patch-id: 9076e9b3bd3cc411b7b80344211364db5f0cca17
prerequisite-patch-id: e220d6ae26786f524c249588433f02e5f5f906ad
prerequisite-patch-id: 58e295ae36ad4784f3eb3830412f35dad31bb8b6
prerequisite-patch-id: d23d83a946e5b876ef01a717fd51b07df1fa08dd
prerequisite-patch-id: d67f2455eef1c4a9ecc460dbf3c2e3ad47d213ec
prerequisite-patch-id: b407d2998e57678952128b3a4bac92a379132b09
prerequisite-patch-id: fbb9212ce8c3530da17d213f56fa334ce4fa1b2b
prerequisite-patch-id: 5db9eed2659028cf4419f2de3d093af7df6c2dad
prerequisite-patch-id: a83c00c628605d1c8fbe1d97074f9f28efb1bcfc

Comments

Chen-Yu Tsai March 25, 2023, 4:33 a.m. UTC | #1
On Wed, Mar 22, 2023 at 8:48 PM Balsam CHIHI <bchihi@baylibre.com> wrote:
>
> Hi Chen-Yu,
>
> I suspect the bug comes from incorrect calibration data offsets for AP
> Domain because you confirm that MCU Domain probe runs without issues.
> Is it possible to test something for us to confirm this theory (i
> don't have an mt8192 board on hand now), when you have the time of
> course?
> We would like to test AP Domain's calibration data offsets with a
> working one, for example :
>
>  static const struct lvts_ctrl_data mt8192_lvts_ap_data_ctrl[] = {
>                 {
> -               .cal_offset = { 0x25, 0x28 },
> +               .cal_offset = { 0x04, 0x04 },
>                 .lvts_sensor = {
>                         { .dt_id = MT8192_AP_VPU0 },
>                         { .dt_id = MT8192_AP_VPU1 }
> @@ -1336,7 +1336,7 @@ static const struct lvts_ctrl_data
> mt8192_lvts_ap_data_ctrl[] = {
>                 .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
>         },
>         {
> -               .cal_offset = { 0x2e, 0x31 },
> +               .cal_offset = { 0x04, 0x04 },
>                 .lvts_sensor = {
>                         { .dt_id = MT8192_AP_GPU0 },
>                         { .dt_id = MT8192_AP_GPU1 }
> @@ -1346,7 +1346,7 @@ static const struct lvts_ctrl_data
> mt8192_lvts_ap_data_ctrl[] = {
>                 .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
>         },
>         {
> -               .cal_offset = { 0x37, 0x3a },
> +               .cal_offset = { 0x04, 0x04 },
>                 .lvts_sensor = {
>                         { .dt_id = MT8192_AP_INFRA },
>                         { .dt_id = MT8192_AP_CAM },
> @@ -1356,7 +1356,7 @@ static const struct lvts_ctrl_data
> mt8192_lvts_ap_data_ctrl[] = {
>                 .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
>         },
>         {
> -               .cal_offset = { 0x40, 0x43, 0x46 },
> +               .cal_offset = { 0x04, 0x04, 0x04 },
>                 .lvts_sensor = {
>                         { .dt_id = MT8192_AP_MD0 },
>                         { .dt_id = MT8192_AP_MD1 },
>
> This example is tested and works for mt8195,
> (all sensors use the same calibration data offset for testing purposes).
>
> Thank you in advance for your help.

The MCU ones are still tripping though. If I change all of them to 0x04,
then nothing trips. There's also a bug in the interrupt handling code
that needs to be dealt with.

AFAICT the calibration data is stored differently. If you look at ChromeOS's
downstream v5.10 driver, you'll see mt6873_efuse_to_cal_data() for MT8192,
and mt8195_efuse_to_cal_data() for MT8195. The difference sums up to:
MT8195 has all data sequentially stored, while MT8192 has most data stored
in lower 24 bits of each 32-bit word, and the highest 8 bits are then used
to pack data for the remaining sensors.

Regards
ChenYu
Nícolas F. R. A. Prado April 24, 2023, 10:21 p.m. UTC | #2
On Tue, Mar 28, 2023 at 02:20:24AM +0200, Balsam CHIHI wrote:
> On Sat, Mar 25, 2023 at 5:33 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> >
> > On Wed, Mar 22, 2023 at 8:48 PM Balsam CHIHI <bchihi@baylibre.com> wrote:
> > >
> > > Hi Chen-Yu,
> > >
> > > I suspect the bug comes from incorrect calibration data offsets for AP
> > > Domain because you confirm that MCU Domain probe runs without issues.
> > > Is it possible to test something for us to confirm this theory (i
> > > don't have an mt8192 board on hand now), when you have the time of
> > > course?
> > > We would like to test AP Domain's calibration data offsets with a
> > > working one, for example :
> > >
> > >  static const struct lvts_ctrl_data mt8192_lvts_ap_data_ctrl[] = {
> > >                 {
> > > -               .cal_offset = { 0x25, 0x28 },
> > > +               .cal_offset = { 0x04, 0x04 },
> > >                 .lvts_sensor = {
> > >                         { .dt_id = MT8192_AP_VPU0 },
> > >                         { .dt_id = MT8192_AP_VPU1 }
> > > @@ -1336,7 +1336,7 @@ static const struct lvts_ctrl_data
[..]
> > >
> > > This example is tested and works for mt8195,
> > > (all sensors use the same calibration data offset for testing purposes).
> > >
> > > Thank you in advance for your help.
> >
> > The MCU ones are still tripping though. If I change all of them to 0x04,
> > then nothing trips. There's also a bug in the interrupt handling code
> > that needs to be dealt with.
> >
> > AFAICT the calibration data is stored differently. If you look at ChromeOS's
> > downstream v5.10 driver, you'll see mt6873_efuse_to_cal_data() for MT8192,
> > and mt8195_efuse_to_cal_data() for MT8195. The difference sums up to:
> > MT8195 has all data sequentially stored, while MT8192 has most data stored
> > in lower 24 bits of each 32-bit word, and the highest 8 bits are then used
> > to pack data for the remaining sensors.
> >
> > Regards
> > ChenYu
> 
> Hi Chen-Yu Tsai,
> 
> Thank you very much for helping me testing this suggestion.
> 
> Indeed, calibration data is stored differently in the mt8192 compared to mt8195.
> So, the mt8192's support will be delayed for now, to allow further debugging.
> 
> In the mean time, we will only continue to upstream the remaining
> mt8195's source code, so it will get full LVTS support.
> A new series will be submitted soon.

Hi Balsam,

like Chen-Yu mentioned, the calibration data is stored with 4 byte alignment for
MT8192, but the data that is split between non-contiguous bytes is for the
thermal controllers (called Resistor-Capacitor Calibration downstream) not the
sensors. The controller calibration isn't currently handled in this driver (and
downstream it also isn't used, since a current value is read from the controller
instead), so we can just ignore those.

The patch below adjusts the addresseses for the sensors and gives me reasonable
reads, so the machine no longer reboots. Can you integrate it into your series?

Thanks,
Nícolas
Chen-Yu Tsai April 25, 2023, 9:59 a.m. UTC | #3
On Tue, Apr 25, 2023 at 6:21 AM Nícolas F. R. A. Prado
<nfraprado@collabora.com> wrote:
>
> On Tue, Mar 28, 2023 at 02:20:24AM +0200, Balsam CHIHI wrote:
> > On Sat, Mar 25, 2023 at 5:33 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> > >
> > > On Wed, Mar 22, 2023 at 8:48 PM Balsam CHIHI <bchihi@baylibre.com> wrote:
> > > >
> > > > Hi Chen-Yu,
> > > >
> > > > I suspect the bug comes from incorrect calibration data offsets for AP
> > > > Domain because you confirm that MCU Domain probe runs without issues.
> > > > Is it possible to test something for us to confirm this theory (i
> > > > don't have an mt8192 board on hand now), when you have the time of
> > > > course?
> > > > We would like to test AP Domain's calibration data offsets with a
> > > > working one, for example :
> > > >
> > > >  static const struct lvts_ctrl_data mt8192_lvts_ap_data_ctrl[] = {
> > > >                 {
> > > > -               .cal_offset = { 0x25, 0x28 },
> > > > +               .cal_offset = { 0x04, 0x04 },
> > > >                 .lvts_sensor = {
> > > >                         { .dt_id = MT8192_AP_VPU0 },
> > > >                         { .dt_id = MT8192_AP_VPU1 }
> > > > @@ -1336,7 +1336,7 @@ static const struct lvts_ctrl_data
> [..]
> > > >
> > > > This example is tested and works for mt8195,
> > > > (all sensors use the same calibration data offset for testing purposes).
> > > >
> > > > Thank you in advance for your help.
> > >
> > > The MCU ones are still tripping though. If I change all of them to 0x04,
> > > then nothing trips. There's also a bug in the interrupt handling code
> > > that needs to be dealt with.
> > >
> > > AFAICT the calibration data is stored differently. If you look at ChromeOS's
> > > downstream v5.10 driver, you'll see mt6873_efuse_to_cal_data() for MT8192,
> > > and mt8195_efuse_to_cal_data() for MT8195. The difference sums up to:
> > > MT8195 has all data sequentially stored, while MT8192 has most data stored
> > > in lower 24 bits of each 32-bit word, and the highest 8 bits are then used
> > > to pack data for the remaining sensors.
> > >
> > > Regards
> > > ChenYu
> >
> > Hi Chen-Yu Tsai,
> >
> > Thank you very much for helping me testing this suggestion.
> >
> > Indeed, calibration data is stored differently in the mt8192 compared to mt8195.
> > So, the mt8192's support will be delayed for now, to allow further debugging.
> >
> > In the mean time, we will only continue to upstream the remaining
> > mt8195's source code, so it will get full LVTS support.
> > A new series will be submitted soon.
>
> Hi Balsam,
>
> like Chen-Yu mentioned, the calibration data is stored with 4 byte alignment for
> MT8192, but the data that is split between non-contiguous bytes is for the
> thermal controllers (called Resistor-Capacitor Calibration downstream) not the
> sensors. The controller calibration isn't currently handled in this driver (and
> downstream it also isn't used, since a current value is read from the controller
> instead), so we can just ignore those.
>
> The patch below adjusts the addresseses for the sensors and gives me reasonable
> reads, so the machine no longer reboots. Can you integrate it into your series?

Not sure what I got wrong, but on my machine the VPU0 and VPU1 zone interrupts
are still tripping excessively. The readings seem normal though. Specifically,
it's bits 16 and 17 that are tripping.

> Thanks,
> Nícolas
>
> From 4506f03b806f3eeb89887bac2c1c86d61da97281 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?N=C3=ADcolas=20F=2E=20R=2E=20A=2E=20Prado?=
>  <nfraprado@collabora.com>
> Date: Mon, 24 Apr 2023 17:42:42 -0400
> Subject: [PATCH] thermal/drivers/mediatek/lvts_thermal: Fix calibration
>  offsets for MT8192
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
>  drivers/thermal/mediatek/lvts_thermal.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
> index b6956c89d557..f8afbc2ac190 100644
> --- a/drivers/thermal/mediatek/lvts_thermal.c
> +++ b/drivers/thermal/mediatek/lvts_thermal.c
> @@ -1261,7 +1261,7 @@ static const struct lvts_ctrl_data mt8195_lvts_ap_data_ctrl[] = {
>
>  static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
>         {
> -               .cal_offset = { 0x04, 0x07 },
> +               .cal_offset = { 0x04, 0x08 },
>                 .lvts_sensor = {
>                         { .dt_id = MT8192_MCU_BIG_CPU0 },
>                         { .dt_id = MT8192_MCU_BIG_CPU1 }
> @@ -1271,7 +1271,7 @@ static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
>                 .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
>         },
>         {
> -               .cal_offset = { 0x0d, 0x10 },
> +               .cal_offset = { 0x0c, 0x10 },
>                 .lvts_sensor = {
>                         { .dt_id = MT8192_MCU_BIG_CPU2 },
>                         { .dt_id = MT8192_MCU_BIG_CPU3 }
> @@ -1281,7 +1281,7 @@ static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
>                 .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
>         },
>         {
> -               .cal_offset = { 0x16, 0x19, 0x1c, 0x1f },
> +               .cal_offset = { 0x14, 0x18, 0x1c, 0x20 },
>                 .lvts_sensor = {
>                         { .dt_id = MT8192_MCU_LITTLE_CPU0 },
>                         { .dt_id = MT8192_MCU_LITTLE_CPU1 },
> @@ -1296,7 +1296,7 @@ static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
>
>  static const struct lvts_ctrl_data mt8192_lvts_ap_data_ctrl[] = {
>                 {
> -               .cal_offset = { 0x25, 0x28 },
> +               .cal_offset = { 0x24, 0x28 },
>                 .lvts_sensor = {
>                         { .dt_id = MT8192_AP_VPU0 },
>                         { .dt_id = MT8192_AP_VPU1 }
> @@ -1306,7 +1306,7 @@ static const struct lvts_ctrl_data mt8192_lvts_ap_data_ctrl[] = {
>                 .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
>         },
>         {
> -               .cal_offset = { 0x2e, 0x31 },
> +               .cal_offset = { 0x2c, 0x30 },
>                 .lvts_sensor = {
>                         { .dt_id = MT8192_AP_GPU0 },
>                         { .dt_id = MT8192_AP_GPU1 }
> @@ -1316,7 +1316,7 @@ static const struct lvts_ctrl_data mt8192_lvts_ap_data_ctrl[] = {
>                 .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
>         },
>         {
> -               .cal_offset = { 0x37, 0x3a },
> +               .cal_offset = { 0x34, 0x38 },
>                 .lvts_sensor = {
>                         { .dt_id = MT8192_AP_INFRA },
>                         { .dt_id = MT8192_AP_CAM },
> @@ -1326,7 +1326,7 @@ static const struct lvts_ctrl_data mt8192_lvts_ap_data_ctrl[] = {
>                 .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
>         },
>         {
> -               .cal_offset = { 0x40, 0x43, 0x46 },
> +               .cal_offset = { 0x3c, 0x40, 0x44 },
>                 .lvts_sensor = {
>                         { .dt_id = MT8192_AP_MD0 },
>                         { .dt_id = MT8192_AP_MD1 },
> --
> 2.40.0
Balsam CHIHI April 25, 2023, 11:28 a.m. UTC | #4
On Tue, Apr 25, 2023 at 12:00 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> On Tue, Apr 25, 2023 at 6:21 AM Nícolas F. R. A. Prado
> <nfraprado@collabora.com> wrote:
> >
> > On Tue, Mar 28, 2023 at 02:20:24AM +0200, Balsam CHIHI wrote:
> > > On Sat, Mar 25, 2023 at 5:33 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> > > >
> > > > On Wed, Mar 22, 2023 at 8:48 PM Balsam CHIHI <bchihi@baylibre.com> wrote:
> > > > >
> > > > > Hi Chen-Yu,
> > > > >
> > > > > I suspect the bug comes from incorrect calibration data offsets for AP
> > > > > Domain because you confirm that MCU Domain probe runs without issues.
> > > > > Is it possible to test something for us to confirm this theory (i
> > > > > don't have an mt8192 board on hand now), when you have the time of
> > > > > course?
> > > > > We would like to test AP Domain's calibration data offsets with a
> > > > > working one, for example :
> > > > >
> > > > >  static const struct lvts_ctrl_data mt8192_lvts_ap_data_ctrl[] = {
> > > > >                 {
> > > > > -               .cal_offset = { 0x25, 0x28 },
> > > > > +               .cal_offset = { 0x04, 0x04 },
> > > > >                 .lvts_sensor = {
> > > > >                         { .dt_id = MT8192_AP_VPU0 },
> > > > >                         { .dt_id = MT8192_AP_VPU1 }
> > > > > @@ -1336,7 +1336,7 @@ static const struct lvts_ctrl_data
> > [..]
> > > > >
> > > > > This example is tested and works for mt8195,
> > > > > (all sensors use the same calibration data offset for testing purposes).
> > > > >
> > > > > Thank you in advance for your help.
> > > >
> > > > The MCU ones are still tripping though. If I change all of them to 0x04,
> > > > then nothing trips. There's also a bug in the interrupt handling code
> > > > that needs to be dealt with.
> > > >
> > > > AFAICT the calibration data is stored differently. If you look at ChromeOS's
> > > > downstream v5.10 driver, you'll see mt6873_efuse_to_cal_data() for MT8192,
> > > > and mt8195_efuse_to_cal_data() for MT8195. The difference sums up to:
> > > > MT8195 has all data sequentially stored, while MT8192 has most data stored
> > > > in lower 24 bits of each 32-bit word, and the highest 8 bits are then used
> > > > to pack data for the remaining sensors.
> > > >
> > > > Regards
> > > > ChenYu
> > >
> > > Hi Chen-Yu Tsai,
> > >
> > > Thank you very much for helping me testing this suggestion.
> > >
> > > Indeed, calibration data is stored differently in the mt8192 compared to mt8195.
> > > So, the mt8192's support will be delayed for now, to allow further debugging.
> > >
> > > In the mean time, we will only continue to upstream the remaining
> > > mt8195's source code, so it will get full LVTS support.
> > > A new series will be submitted soon.
> >
> > Hi Balsam,
> >
> > like Chen-Yu mentioned, the calibration data is stored with 4 byte alignment for
> > MT8192, but the data that is split between non-contiguous bytes is for the
> > thermal controllers (called Resistor-Capacitor Calibration downstream) not the
> > sensors. The controller calibration isn't currently handled in this driver (and
> > downstream it also isn't used, since a current value is read from the controller
> > instead), so we can just ignore those.
> >
> > The patch below adjusts the addresseses for the sensors and gives me reasonable
> > reads, so the machine no longer reboots. Can you integrate it into your series?
>
> Not sure what I got wrong, but on my machine the VPU0 and VPU1 zone interrupts
> are still tripping excessively. The readings seem normal though. Specifically,
> it's bits 16 and 17 that are tripping.
>

Hi Chen-Yu,

Thank you for testing!

As the readings are normal that proves that calibration data offsets
are correct.
would you like that I send the v2 of series to add mt8192 support?
Then we could deal with the interrupts later in a separate fix,
because the interrupt code in common for both SoC (mt8192 and mt8195)?

Does Nícolas also have tripping interrupts?
On my side, I've got no interrupts tripping on mt8195.

Any other suggestions (a question for everyone)?

Best regards,
Balsam

> > Thanks,
> > Nícolas
> >
> > From 4506f03b806f3eeb89887bac2c1c86d61da97281 Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?N=C3=ADcolas=20F=2E=20R=2E=20A=2E=20Prado?=
> >  <nfraprado@collabora.com>
> > Date: Mon, 24 Apr 2023 17:42:42 -0400
> > Subject: [PATCH] thermal/drivers/mediatek/lvts_thermal: Fix calibration
> >  offsets for MT8192
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> >
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > ---
> >  drivers/thermal/mediatek/lvts_thermal.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
> > index b6956c89d557..f8afbc2ac190 100644
> > --- a/drivers/thermal/mediatek/lvts_thermal.c
> > +++ b/drivers/thermal/mediatek/lvts_thermal.c
> > @@ -1261,7 +1261,7 @@ static const struct lvts_ctrl_data mt8195_lvts_ap_data_ctrl[] = {
> >
> >  static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
> >         {
> > -               .cal_offset = { 0x04, 0x07 },
> > +               .cal_offset = { 0x04, 0x08 },
> >                 .lvts_sensor = {
> >                         { .dt_id = MT8192_MCU_BIG_CPU0 },
> >                         { .dt_id = MT8192_MCU_BIG_CPU1 }
> > @@ -1271,7 +1271,7 @@ static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
> >                 .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
> >         },
> >         {
> > -               .cal_offset = { 0x0d, 0x10 },
> > +               .cal_offset = { 0x0c, 0x10 },
> >                 .lvts_sensor = {
> >                         { .dt_id = MT8192_MCU_BIG_CPU2 },
> >                         { .dt_id = MT8192_MCU_BIG_CPU3 }
> > @@ -1281,7 +1281,7 @@ static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
> >                 .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
> >         },
> >         {
> > -               .cal_offset = { 0x16, 0x19, 0x1c, 0x1f },
> > +               .cal_offset = { 0x14, 0x18, 0x1c, 0x20 },
> >                 .lvts_sensor = {
> >                         { .dt_id = MT8192_MCU_LITTLE_CPU0 },
> >                         { .dt_id = MT8192_MCU_LITTLE_CPU1 },
> > @@ -1296,7 +1296,7 @@ static const struct lvts_ctrl_data mt8192_lvts_mcu_data_ctrl[] = {
> >
> >  static const struct lvts_ctrl_data mt8192_lvts_ap_data_ctrl[] = {
> >                 {
> > -               .cal_offset = { 0x25, 0x28 },
> > +               .cal_offset = { 0x24, 0x28 },
> >                 .lvts_sensor = {
> >                         { .dt_id = MT8192_AP_VPU0 },
> >                         { .dt_id = MT8192_AP_VPU1 }
> > @@ -1306,7 +1306,7 @@ static const struct lvts_ctrl_data mt8192_lvts_ap_data_ctrl[] = {
> >                 .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
> >         },
> >         {
> > -               .cal_offset = { 0x2e, 0x31 },
> > +               .cal_offset = { 0x2c, 0x30 },
> >                 .lvts_sensor = {
> >                         { .dt_id = MT8192_AP_GPU0 },
> >                         { .dt_id = MT8192_AP_GPU1 }
> > @@ -1316,7 +1316,7 @@ static const struct lvts_ctrl_data mt8192_lvts_ap_data_ctrl[] = {
> >                 .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
> >         },
> >         {
> > -               .cal_offset = { 0x37, 0x3a },
> > +               .cal_offset = { 0x34, 0x38 },
> >                 .lvts_sensor = {
> >                         { .dt_id = MT8192_AP_INFRA },
> >                         { .dt_id = MT8192_AP_CAM },
> > @@ -1326,7 +1326,7 @@ static const struct lvts_ctrl_data mt8192_lvts_ap_data_ctrl[] = {
> >                 .hw_tshut_temp = LVTS_HW_SHUTDOWN_MT8192,
> >         },
> >         {
> > -               .cal_offset = { 0x40, 0x43, 0x46 },
> > +               .cal_offset = { 0x3c, 0x40, 0x44 },
> >                 .lvts_sensor = {
> >                         { .dt_id = MT8192_AP_MD0 },
> >                         { .dt_id = MT8192_AP_MD1 },
> > --
> > 2.40.0
Nícolas F. R. A. Prado April 26, 2023, 11:20 p.m. UTC | #5
On Tue, Apr 25, 2023 at 01:28:39PM +0200, Balsam CHIHI wrote:
> On Tue, Apr 25, 2023 at 12:00 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
> >
> > On Tue, Apr 25, 2023 at 6:21 AM Nícolas F. R. A. Prado
> > <nfraprado@collabora.com> wrote:
> > >
> > > On Tue, Mar 28, 2023 at 02:20:24AM +0200, Balsam CHIHI wrote:
> > > > On Sat, Mar 25, 2023 at 5:33 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> > > > >
> > > > > On Wed, Mar 22, 2023 at 8:48 PM Balsam CHIHI <bchihi@baylibre.com> wrote:
[..]
> > > >
> > > > Hi Chen-Yu Tsai,
> > > >
> > > > Thank you very much for helping me testing this suggestion.
> > > >
> > > > Indeed, calibration data is stored differently in the mt8192 compared to mt8195.
> > > > So, the mt8192's support will be delayed for now, to allow further debugging.
> > > >
> > > > In the mean time, we will only continue to upstream the remaining
> > > > mt8195's source code, so it will get full LVTS support.
> > > > A new series will be submitted soon.
> > >
> > > Hi Balsam,
> > >
> > > like Chen-Yu mentioned, the calibration data is stored with 4 byte alignment for
> > > MT8192, but the data that is split between non-contiguous bytes is for the
> > > thermal controllers (called Resistor-Capacitor Calibration downstream) not the
> > > sensors. The controller calibration isn't currently handled in this driver (and
> > > downstream it also isn't used, since a current value is read from the controller
> > > instead), so we can just ignore those.
> > >
> > > The patch below adjusts the addresseses for the sensors and gives me reasonable
> > > reads, so the machine no longer reboots. Can you integrate it into your series?
> >
> > Not sure what I got wrong, but on my machine the VPU0 and VPU1 zone interrupts
> > are still tripping excessively. The readings seem normal though. Specifically,
> > it's bits 16 and 17 that are tripping.
> >
> 
> Hi Chen-Yu,
> 
> Thank you for testing!
> 
> As the readings are normal that proves that calibration data offsets
> are correct.
> would you like that I send the v2 of series to add mt8192 support?
> Then we could deal with the interrupts later in a separate fix,
> because the interrupt code in common for both SoC (mt8192 and mt8195)?
> 
> Does Nícolas also have tripping interrupts?
> On my side, I've got no interrupts tripping on mt8195.
> 
> Any other suggestions (a question for everyone)?

Hi,

sorry for the delay.

Indeed the interrupts are constantly tripping on mt8192 here as well.

I do not see the same bits as Chen-Yu mentioned however, I see

LVTS_MONINTSTS = 0x08070000

which corresponds to

	Hot threshold on sense point 3
	high to normal offset on sense point 2
	high offset on sense point 2
	low offset on sense point 2

and it's the same on all controllers and domains here, which is weird. I noticed
we have offset interrupts enabled even though we don't configure the values for
those, but even after disabling them and clearing the status register, the
interrupts keep triggering and the status is the same, so for some reason
LVTS_MONINT doesn't seem to be honored.

I also tried using the filtered mode instead of immediate for the sensors, and
that together with disabling the extra interrupts, got me a zeroed
LVTS_MONINTSTS. However no interrupts seem to be triggered at all (nor
LVTS_MONINTSTS updated) when the temperature goes over the configured one in
LVTS_HTHRE.

I tried the driver on mt8195 (Tomato chromebook) as well, and it has the same
LVTS_MONINTSTS = 0x08070000
even though the interrupts aren't being triggered, but in fact I don't see them
triggering over the threshold either, so I suspect the irq number might be
incorrectly described in the DT there.

Do either of you have it working correctly on mt8195?

Anyway, I'll keep digging and reply here when I find a solution.

Thanks,
Nícolas