cpu/esp32/periph: fix esp32c6 timer clk#22456
Conversation
875c47f to
7768314
Compare
| /* TIMER_CLK_FREQ corresponds to APB_CLK_FREQ for ESP32, ESP32-S2, ESP32-S3, | ||
| * ESP32-C2 and ESP32-C3, which is a fixed frequency of 80 MHz. However, | ||
| * this only applies to CPU clock frequencies of 80 MHz and above. | ||
| * For lower CPU clock frequencies, the APB clock corresponds to the CPU clock | ||
| * frequency. Therefore, we need to determine the actual TIMER clock frequency | ||
| * from the actual APB clock frequency. */ |
There was a problem hiding this comment.
| /* TIMER_CLK_FREQ corresponds to APB_CLK_FREQ for ESP32, ESP32-S2, ESP32-S3, | |
| * ESP32-C2 and ESP32-C3, which is a fixed frequency of 80 MHz. However, | |
| * this only applies to CPU clock frequencies of 80 MHz and above. | |
| * For lower CPU clock frequencies, the APB clock corresponds to the CPU clock | |
| * frequency. Therefore, we need to determine the actual TIMER clock frequency | |
| * from the actual APB clock frequency. */ | |
| /* For ESP32, ESP32-S2, ESP32-S3, ESP32-C2 and ESP32-C3, the TIMER_CLK_FREQ | |
| * is based on APB_CLK_FREQ. This is fixed at 80MHz for CPU clock frequencies | |
| * >=80MHz and equal to the CPU clock frequency for CPU clock frequencies <80Mhz. */ |
Although this part is copy&pasted from the UART code, I think we can optimize the wording a bit nevertheless.
|
Can you please give some more details about how you tested the changes? The hello world application shouldn't print anything about timers 🤔 Also, please add the AI statement back to your original PR description. |
7768314 to
c3da5f2
Compare
|
Without the fix: With the fix: Also very useful for testing this is Current With the fix: Out of curiosity, I added two It is quite obvious why the timer was running 2x too quickly, it assumed it ran with 40MHz while it actually ran with 80MHz. |
| @@ -60,6 +61,19 @@ | |||
|
|
|||
| /* hardware timer modules used */ | |||
|
|
|||
There was a problem hiding this comment.
| /* The ESP32 SDK (incorrectly) defines TIMER_CLK_FREQ, but does not use it | |
| * itself. The definition has been removed in Version 6.0 too. */ | |
| #ifdef TIMER_CLK_FREQ | |
| # undef TIMER_CLK_FREQ | |
| #endif | |
I think you'll have to add this too. Usually #undef is not a good style, but TIMER_CLK_FREQ is not used in the ESP32 SDK and the definitions seem to be incorrect (and even one comment is incorrect lol):
cbuec@W11nMate:~/RIOTstuff/riot-vanillaice/RIOT/build/pkg/esp32_sdk$ grep -RnwI TIMER_CLK_FREQ
components/soc/esp32c3/include/soc/soc.h:146:#define TIMER_CLK_FREQ (80000000>>4) //80MHz divided by 16
components/soc/esp32s3/include/soc/soc.h:162:#define TIMER_CLK_FREQ (80000000>>4) //80MHz divided by 16
components/soc/esp32s2/include/soc/soc.h:152:#define TIMER_CLK_FREQ (80000000>>4) //80MHz divided by 16
components/soc/esp32/include/soc/soc.h:166:#define TIMER_CLK_FREQ (80000000>>4) //80MHz divided by 16
components/soc/esp32c2/include/soc/soc.h:152:#define TIMER_CLK_FREQ (80000000>>4) //80MHz divided by 4
There was a problem hiding this comment.
We can also just use a different name, this is just an internal define after all.
crasbe
left a comment
There was a problem hiding this comment.
Would you mind "backporting" the changes to the UART code too? That way we have the same code and docs in both drivers.
| # define TIMER_CLK_FREQ (CLK_LL_PLL_80M_FREQ_MHZ * MHZ) /* PLL_F80M_CLK is used */ | ||
| #elif CPU_FAM_ESP32H2 | ||
| # define TIMER_CLK_FREQ (CLK_LL_PLL_48M_FREQ_MHZ * MHZ) /* PLL_F48M_CLK is used */ |
There was a problem hiding this comment.
| # define TIMER_CLK_FREQ (CLK_LL_PLL_80M_FREQ_MHZ * MHZ) /* PLL_F80M_CLK is used */ | |
| #elif CPU_FAM_ESP32H2 | |
| # define TIMER_CLK_FREQ (CLK_LL_PLL_48M_FREQ_MHZ * MHZ) /* PLL_F48M_CLK is used */ | |
| # define TIMER_CLK_FREQ ((uint32_t)CLK_LL_PLL_80M_FREQ_MHZ * MHZ) /* PLL_F80M_CLK is used */ | |
| #elif CPU_FAM_ESP32H2 | |
| # define TIMER_CLK_FREQ ((uint32_t)CLK_LL_PLL_48M_FREQ_MHZ * MHZ) /* PLL_F48M_CLK is used */ |
The return value of rtc_clk_apb_freq_get() is an uint32_t, so we want to have the same data type for all variants.
|
This will also fix the timer setting on the ESP32H2: Behavior on With this PR: |
c3da5f2 to
b70a616
Compare
Contribution description
This PR fixes the timer clock source used on the ESP32-C6 in the GPTimer peripheral.
The existing implementation always used rtc_clk_apb_freq_get() to calculate the timer clock divider. While this is correct for ESP32, ESP32-S2, ESP32-S3 and ESP32-C3, the ESP32-C6 GPTimer is clocked from the fixed 80 MHz PLL (PLL_F80M_CLK) instead of the APB clock.
Testing procedure
hello world application on esp32c6 board:
Test application: examples/hello-world on an ESP32-C6 board
I used the hello-world example because it is a minimal application that helps me isolate issues and eliminates the influence of other modules or application logic.
I used it the app to check the timer.
Makefile:
main.c
without the fix:
2026-07-10 10:21:10,997 # Hello World!
2026-07-10 10:21:10,998 # You are running RIOT on a(n) index_wr_tracker board.
2026-07-10 10:21:10,999 # This board features a(n) esp32 CPU.
2026-07-10 10:21:10,999 # Available timers: 1
2026-07-10 10:21:10,999 # timer set for 1 second, waiting for callback...
2026-07-10 10:21:11,499 # timer fired
with the fix:
2026-07-10 10:22:42,063 # Hello World!
2026-07-10 10:22:42,064 # You are running RIOT on a(n) index_wr_tracker board.
2026-07-10 10:22:42,064 # This board features a(n) esp32 CPU.
2026-07-10 10:22:42,064 # Available timers: 1
2026-07-10 10:22:42,065 # timer set for 1 second, waiting for callback...
2026-07-10 10:22:43,064 # timer fired
Issues/PRs references
None.
Declaration of AI-Tools / LLMs usage:
AI-Tools / LLMs that were used are: