Hi Mateusz

On Tue, 24 Jun 2025 at 13:28, Mateusz Majewski [off-list ref] wrote:

quoted

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:

Thanks for clarifying this in such detail.

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.

Correct

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.

For Exynos4210, I am skipping this in the next patch.

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.

correct.

quoted

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.

Thanks, I will update this in the next version.

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.

As per the manuals, we have to enable each corresponding bit for RISE and FALL

INTEN          0x0070    Interrupt enable register
INTSTAT      0x0074    Interrupt status register
INTCLEAR   0x0078    Interrupt clear register

When current temperature exceeds a threshold rise temperature, then
it generates corresponding interrupt (INTREQ_RISE[2:0]).
When current temperature goes below a threshold fall temperature, then
it generates corresponding interrupt (INTREQ_FALL[2:0].

If we correctly configure the mapping of the RISE and FALL bits along with
INTEN, INTSTAT, and INTCLEAR, the interrupt behavior should function
as expected.

The following outlines the interrupt flow as described in the user manual

[1} https://usermanual.wiki/Document/Exynos20441220SCPUsers20ManualVer01000Preliminary0.167615881

/* Read the measured data from e-fuse */
Triminfo_25 = TRIMINFO[7:0]
/* Calibrated threshold temperature is written into THRES_TEMP_RISE
and THRES_TEMP_FALL */
/* Refer to 1.6.1 */
THRES_TEMP_RISE0 = 0x40;
THRES_TEMP_RISE1 = 0x50;
THRES_TEMP_RISE2 = 0x60;
THRES_TEMP_RISE3 = 0x70;
THRES_TEMP_FALL0 = 0x3A;
THRES_TEMP_FALL1 = 0x4A;
THRES_TEMP_FALL2 = 0x5A;
/* Parameter for sampling interval is set */
SAMPLING_INTERVAL = 0x1;
/* Interrupt enable */
INTEN[24] =0x1; // for INTEN_FALL2
INTEN[20] =0x1; // for INTEN_FALL1
INTEN[16] =0x1; // for INTEN_FALL0
INTEN[8] =0x1; // for INTEN_RISE2
INTEN[4] =0x1; // for INTEN_RISE1
INTEN[0] =0x1; // for INTEN_RISE0
/* Thermal tripping mode selection */
THERM_TRIP_MODE = 0x4;
/* Thermal tripping enable */
THERM_TRIP_EN = 0x1;
/* Check sensing operation is idle */
tmu_idle = 0;
while(tmu_idle&1) {
    tmu_idle = TMU_STATUS[0];
}
/* Start sensing operation */
TMU_CONTROL |= 1;

ISR_INTREQ_TMU () {
/* Read interrupt status register */
int_status = INTSTAT;
if(int_status[24]) {
    ISR_INT_FALL2();
}
else if(int_status[20]) {
    ISR_INT_FALL1();
}
else if(int_status[16]) {
    ISR_INT_FALL0();
}
Else if(int_status[8]) {
    ISR_INT_RISE2();
}
else if(int_status[4]) {
    ISR_INT_RISE1();
}
else if(int_status[0]) {
    ISR_INT_RISE0();
}
else {
    $display("Some error occurred..!");
}
ISR_INT0 () {
   /* Perform proper task for decrease temperature */
    INTCLEAR[0] = 0x1;
}

Hey, if you’ve got a bit of time, could you take a look at these changes?
[2]  https://lore.kernel.org/all/20250430123306.15072-2-linux.amoon@gmail.com/ (local)

Thank you,
Mateusz Majewski

Thanks
-Anand