linux-kernel - Re: [RRC v1 2/3] thermal/drivers/exynos: Handle temperature threshold interrupts and clear corresponding IRQs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250624075815.132207-1-m.majewski2@samsung.com>
Date: Tue, 24 Jun 2025 09:58:15 +0200
From: Mateusz Majewski <m.majewski2@...sung.com>
To: linux.amoon@...il.com
Cc: alim.akhtar@...sung.com, bzolnier@...il.com, daniel.lezcano@...aro.org,
	krzk@...nel.org, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
	linux-samsung-soc@...r.kernel.org, lukasz.luba@....com,
	m.majewski2@...sung.com, rafael@...nel.org, rui.zhang@...el.com
Subject: Re: [RRC v1 2/3] thermal/drivers/exynos: Handle temperature
 threshold interrupts and clear corresponding IRQs

> I tried to configure this, referring to the comment in the driver
>         /*
>          * Clear the interrupts.  Please note that the documentation for
>          * Exynos3250, Exynos4412, Exynos5250 and Exynos5260 incorrectly
>          * states that INTCLEAR register has a different placing of bits
>          * responsible for FALL IRQs than INTSTAT register.  Exynos5420
>          * and Exynos5440 documentation is correct (Exynos4210 doesn't
>          * support FALL IRQs at all).
>          */
>
> By the way, I don't see Exynos5433 and Exynos7 support
> INTSTAT and INTCLEAR registers. We are using TMU_REG_INTPEND
>  to read and update the same register.
>
>         if (data->soc == SOC_ARCH_EXYNOS5260) {
>                 tmu_intstat = EXYNOS5260_TMU_REG_INTSTAT;
>                 tmu_intclear = EXYNOS5260_TMU_REG_INTCLEAR;
>         } else if (data->soc == SOC_ARCH_EXYNOS7) {
>                 tmu_intstat = EXYNOS7_TMU_REG_INTPEND;
>                 tmu_intclear = EXYNOS7_TMU_REG_INTPEND;
>         } else if (data->soc == SOC_ARCH_EXYNOS5433) {
>                 tmu_intstat = EXYNOS5433_TMU_REG_INTPEND;
>                 tmu_intclear = EXYNOS5433_TMU_REG_INTPEND;
>         } else {
>                 tmu_intstat = EXYNOS_TMU_REG_INTSTAT;
>                 tmu_intclear = EXYNOS_TMU_REG_INTCLEAR;
>         }

My understanding of this comment and the situation in general is like
this:

1. On 5420, whenever there is edge interrupt, no matter if rise or fall,
   a bit gets set to 1 inside INTSTAT, and we clear it by setting the
   same bit to 1 inside INTCLEAR. The current code does not rely on the
   concrete bit index, it will just check the temperature after the
   interrupt.
2. On 4210, there is no falling edge interrupts (so
   exynos4210_tmu_set_low_temp is empty, we enable polling in DT etc).
   This is what the "Exynos4210 doesn't support FALL IRQs at all" means.
   However, rising edge interrupts work exactly the same as on 5420:
   a bit gets set to 1 inside INTSTAT, and we clear it by setting the
   same bit to 1 inside INTCLEAR.
3. On 3250, 4412, 5250, 5260, it again works the same way as 5420.
   However, somebody had a copy of documentation that was incorrect: it
   said that bit indices does not match somehow, which is not true.
4. On 5433 and 7, it one more time works the same way as 5420, with a
   single change: a bit gets set to 1 inside INTPEND, and we clear it
   by setting it to 1 inside the same INTPEND.

So, all we need to do to support existing SoCs is to read the 1 bit from
one register, and set the bit with the same index in another register
(which on some SoCs is the same register). We could interpret the index
to see what kind of interrupt is this, but we read the temperature to
get similar information.

So in the end, is it helpful to interpret the INTSTAT bit index, only to
reset the exact same index inside INTCLEAR? I guess it could be valuable
if we also used the information about which interrupt it is and somehow
used it elsewhere (which could actually help with some issues), but that
is another thing to do.

> If you have details on how INTSTAT and INTCLEAR are used
> particularly regarding the update bits, please share them.
> Specifically, I'm interested in how bits [7:0] correspond to rising edge
> interrupts and bits [23:16] to falling edge interrupts
> I feel it's the same as Exynos54222.

Regarding concrete indices on 5433:
- the 0th bit corresponds to RISE0,
- the 1st bit corresponds to RISE1,
- ...
- the 7th bit corresponds to RISE7,
- the 16th bit corresponds to FALL0,
- the 17th bit corresponds to FALL1,
- ...
- the 23th bit corresponds to FALL7.

That is probably because this SoC supports more interrupts than others.
Though do note that currently, we only use part of them (one RISE, one
FALL if supported, and another RISE for critical temperature (one
supporting hardware thermal tripping if possible)). Also note that the
indices in INTSTAT/INTCLEAR/INTPEND match the ones in INTEN, though I
have not checked thoroughly if that is true for all the SoCs.

Thank you,
Mateusz Majewski