timer_spinlock_take/give IRAM_ATTR (IDFGH-5038)

[boborjan2]

Environment

• Module or chip used: [ESP32-WROOM-32E]
• IDF version (run git describe --tags to find it): v4.3-dev-3175-g9a2d25191
• Build System: [Make]
• Compiler version (run xtensa-esp32-elf-gcc --version to find it): (crosstool-NG esp-2020r3) 8.4.0
• Operating System: [Linux]
• Using an IDE?: [No]
• Power Supply: [external]

Problem Description

timer_spinlock_take has no IRAM_ATTR attribute but according to docs, it should be able to be called from an isr that may be placed to IRAM.

Expected Behavior

My understanding is that these two functions shall be in iram.

Thanks
Viktor

[AxelLin]

The v4.3 branch also needs this fix.

[Alvin1Zhang]

Thanks for pointing out, we will back port this fix to release/4.3, thanks.

[boborjan2]

It should go into 4.2 as well imho. Thx.

[ESP-Marius]

Fix will be backported up to and including v4.1

[KaeLL]

@ESP-Marius ETA on the publishing of these fixes?
Also, since we're on the subject, doesn't deprecating the timer spinlock APIs also deprecate timer_isr_register? And by the way, why deprecate the timer spinlock APIs in the first place? timer_isr_default is overly verbose and in my use case, re-enabling the alarm by itself is undesirable. Seems ironic to constrain what you call General Purpose Timers in the docs in such a way.

[suda-morris]

Hi @KaeLL we deprecated because nowhere is using that spinlock_get/set in esp-idf code base. Exposing spinlock_get/set is not a good design, at least we don't want the user space to touch a spinlock used by the driver itself directly. So we have introduced another callback way.

re-enabling the alarm by itself is undesirable.

Thanks for this information, yes this is an issue and we're fixing now, will be backported to release/v4.1. The issue is, we shound't reenable the alarm interrupt in timer_isr_register if user has configured the "auto-reload".