Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing IRAM_ATTR for esp_timer_is_active() and esp_timer_restart() (IDFGH-9122) #10522

Closed
3 tasks done
AxelLin opened this issue Jan 11, 2023 · 7 comments
Closed
3 tasks done
Assignees
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally

Comments

@AxelLin
Copy link
Contributor

AxelLin commented Jan 11, 2023

Answers checklist.

  • I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
  • I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
  • I have searched the issue tracker for a similar issue and not found a similar issue.

General issue report

  1. esp_timer_is_active() should has IRAM_ATTR.
    It just calls timer_armed() which has IRAM_ATTR.
    Question about the IRAM_ATTR usage with uart ISR esp-nimble#50 (comment)

  2. It's strange that esp_timer_start_periodic() and esp_timer_stop() have IRAM_ATTR but esp_timer_restart() does not has IRAM_ATTR.
    Question about the IRAM_ATTR usage with uart ISR esp-nimble#50 (comment)

@espressif-bot espressif-bot added the Status: Opened Issue is new label Jan 11, 2023
@github-actions github-actions bot changed the title Missing IRAM_ATTR for esp_timer_is_active() and esp_timer_restart() Missing IRAM_ATTR for esp_timer_is_active() and esp_timer_restart() (IDFGH-9122) Jan 11, 2023
@RoshanESP
Copy link
Collaborator

RoshanESP commented Jan 11, 2023

Hi @AxelLin ,

  1. Can you please share the exception traceback log.
  2. Can you please share callgraph output by referring to this comment

@AxelLin
Copy link
Contributor Author

AxelLin commented Jan 11, 2023

For esp_timer_is_active():
I think this code path already proves esp_timer_is_active() needs IRAM_ATTR since it is called by npl_freertos_callout_is_active().
https://github.com/espressif/esp-nimble/blob/56ff5b83337697895997a39bf6593255cdc37268/porting/npl/freertos/src/npl_os_freertos.c#L852-L860

For esp_timer_restart():
I didn't hit the issue because I don't use that function at the moment.
It looks strange that a start/stop functions have IRAM_ATTR but the the new restart function (suppose to be a shortcut of stop/start) does not has IRAM_ATTR.

@AxelLin
Copy link
Contributor Author

AxelLin commented Feb 4, 2023

@RoshanESP @rahult-github

Any update? Can you check if this needs fix?

@KonstantinKondrashov
Copy link
Collaborator

Hi @AxelLin!
Thanks for bringing this issue up. I will help to fix it and backport it to the other versions.

@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Selected for Development Issue is selected for development labels Mar 1, 2023
@KonstantinKondrashov
Copy link
Collaborator

There is a way how to workaround this issue with missed IRAM_ATTR for esp_timer_is_active() and esp_timer_restart(). You need to add a linker.lf into the project and mention there these functions.
https://github.com/espressif/esp-idf/blob/master/examples/peripherals/spi_master/hd_eeprom/components/eeprom/linker.lf#L21-L24.

From our perspective, we do not want to increase IRAM usage if there is a way to customize it on the user side using linker.lf.

[mapping:esp_timer_custom]
archive: libesp_timer.a
entries:
    esp_timer:esp_timer_is_active (noflash)
    esp_timer:esp_timer_restart (noflash) 

But there is another way for us, to add a Kconfig option under esp_timer to do the same.

@AxelLin
Copy link
Contributor Author

AxelLin commented Mar 2, 2023

@KonstantinKondrashov

For https://github.com/espressif/esp-nimble/blob/56ff5b83337697895997a39bf6593255cdc37268/porting/npl/freertos/src/npl_os_freertos.c#L852-L860
npl_freertos_callout_is_active() has IRAM_ATTR, so esp_timer_is_active() must has IRAM_ATTR.
In this case, since the code is not directly called by user's application.
I think linker.lf is not a good option. (You are asking application developers to workaround sdk bug).

If you want to add Kconfig option, you have to make sure you handle the dependency correctly.
in above example,
npl_freertos_callout_is_active() / esp_timer_is_active() / xTimerIsTimerActive() should be consistent.
Either with IRAM_ATTR or without IRAM_ATTR for all of them.

@AxelLin
Copy link
Contributor Author

AxelLin commented Apr 10, 2023

Hi @AxelLin! Thanks for bringing this issue up. I will help to fix it and backport it to the other versions.

Any update for the fix @KonstantinKondrashov ?
Below is the panic I hit, adding IRAM_ATTR to esp_timer_is_active() fixed it.

Guru Meditation Error: Core 0 panic'ed (Store access fault). Exception was unhandled.

Stack dump detected
Core 0 register dump:
MEPC : 0x4038ca40 RA : 0x4038b834 SP : 0x3fca09c0 GP : 0x3fc94400
0x4038ca40: uxListRemove at /home/axel/esp/esp-idf/components/freertos/FreeRTOS-Kernel/list.c:197

0x4038b834: xTaskRemoveFromEventList at /home/axel/esp/esp-idf/components/freertos/FreeRTOS-Kernel/tasks.c:3900

TP : 0x3fc65ef0 T0 : 0x4005890e T1 : 0x0000000f T2 : 0x40382dcc
0x40382dcc: npl_freertos_callout_deinit at /home/axel/esp/esp-idf/components/bt/host/nimble/nimble/porting/npl/freertos/src/npl_os_freertos.c:761

S0/FP : 0x3fc9ce8c S1 : 0x0000007c A0 : 0x3fc9cea4 A1 : 0x3fc9cea4
A2 : 0xffffffff A3 : 0x000a000a A4 : 0x0000000a A5 : 0x3fc9ce60
A6 : 0x3fc9c3ec A7 : 0x00000000 S2 : 0x3fc9cea4 S3 : 0xffffffff
S4 : 0x3fc9ce90 S5 : 0x3c0f9000 S6 : 0x3c0f9000 S7 : 0x00043518
S8 : 0x3c0f9000 S9 : 0x3c0f9000 S10 : 0x00000000 S11 : 0x04a70dd7
T3 : 0x40381fa2 T4 : 0x40382ece T5 : 0x40381f9e T6 : 0x40382ec2
0x40381fa2: npl_freertos_callout_set_arg at /home/axel/esp/esp-idf/components/bt/host/nimble/nimble/porting/npl/freertos/src/npl_os_freertos.c:926

0x40382ece: npl_freertos_callout_remaining_ticks at /home/axel/esp/esp-idf/components/bt/host/nimble/nimble/porting/npl/freertos/src/npl_os_freertos.c:888

0x40381f9e: npl_freertos_callout_get_ticks at /home/axel/esp/esp-idf/components/bt/host/nimble/nimble/porting/npl/freertos/src/npl_os_freertos.c:879

0x40382ec2: npl_freertos_callout_is_active at /home/axel/esp/esp-idf/components/bt/host/nimble/nimble/porting/npl/freertos/src/npl_os_freertos.c:856

MSTATUS : 0x00001881 MTVEC : 0x40380001 MCAUSE : 0x00000007 MTVAL : 0x000a0012
0x40380001: _vector_table at ??:?

MHARTID : 0x00000000

@espressif-bot espressif-bot added Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: In Progress Work is in progress Resolution: NA Issue resolution is unavailable labels Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally
Projects
None yet
Development

No branches or pull requests

4 participants