Feat: Add cpu clock increase #42

Dzarda7 · 2025-12-16T10:40:35Z

Description

This PR adds CPU frequency increase and fixes the set baudrate function.

Testing

Tested on all targets and measured CPU frequency using Logic analyzer.

Checklist

Before submitting a Pull Request, please ensure the following:

🚨 This PR does not introduce breaking changes.
All CI checks (GH Actions) pass.
Documentation is updated as needed.
Tests are updated or added as necessary.
Code is well-commented, especially in complex areas.
Git history is clean — commits are squashed to the minimum necessary.

github-actions · 2025-12-16T10:41:11Z

	Messages
📖	This PR seems to be quite large (total lines of code: 31909), you might consider splitting it into smaller PRs

👋 Hello Dzarda7, we appreciate your contribution to this project!

Click to see more instructions ...

This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Addressing info messages (📖) is strongly recommended; they're less critical but valuable.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...

We do welcome contributions in the form of bug reports, feature requests and pull requests.

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against 8c0a6a9

Dzarda7 · 2025-12-16T10:45:02Z

@erhankur After this, the rominit from set baudrate function can be basically removed I believe, only issue is the ESP32-C2 where we need to determine crystal frequency from previously initialized baudrate. If you can suggest different way to detect it how you do that, then I can fix this also. Only other way I can think of is calculate RTC clock and compare, which is a bit overkill for the stub.

Copilot

Pull request overview

This PR adds CPU clock frequency increase functionality across all ESP targets and fixes the UART baudrate setting mechanism. The implementation increases CPU speeds to maximize stub performance while ensuring proper UART communication after clock changes.

Key changes:

Introduces clock initialization infrastructure with target-specific implementations
Updates UART baudrate calculation to use correct clock sources (APB frequency instead of crystal/CPU frequency)
Adds delays before/after baudrate changes for stability
Includes volatile casts in register manipulation macros to prevent compiler optimization issues

Reviewed changes

Copilot reviewed 59 out of 64 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/target/*/src/clock.c	Target-specific clock initialization implementations setting CPU frequencies (ESP32: 160MHz, ESP32-S2: 240MHz, ESP32-P4: 320MHz, etc.)
src/target/*/src/uart.c	Updated baudrate calculation to use APB frequency and added stability delays
src/target/base/include/target/clock.h	New internal API header defining clock initialization and frequency getter functions
src/target//ld/.rom.api.ld	Updated ROM API mappings for clock-related functions (esp_rom_get_xtal_freq, removed esp_rom_get_apb_freq where replaced)
include/esp-stub-lib/soc_utils.h	Added volatile casts to register manipulation macros to prevent unwanted compiler optimizations
src/target/*/CMakeLists.txt	Added clock.c source files to build configuration for each target
src/clock.c	Top-level clock wrapper calling target-specific initialization
src/target/common/src/uart.c	Updated weak default UART implementation to use crystal frequency

Copilot · 2025-12-16T10:57:03Z

src/target/esp32c61/ld/esp32c61.rom.api.ld

+PROVIDE ( esp_rom_get_xtal_freq       = ets_clk_get_xtal_freq );
+PROVIDE ( esp_rom_get_cpu_freq        = ets_clk_get_cpu_freq );


Duplicate PROVIDE declaration for esp_rom_get_cpu_freq. Line 29 declares it, and line 31 declares it again. Remove one of the duplicate declarations.

Copilot · 2025-12-16T10:57:03Z

src/target/esp32c3/src/uart.c

+    stub_target_rom_uart_init(uart_num, clock);
+
+    // Set UART clock source to APB, after this, baudrate needs to be set again to recalculate the clock divider
+    SET_PERI_REG_BITS(UART_CLK_CONF_REG(0), UART_SCLK_SEL_V, 1, UART_SCLK_SEL_S);


The comment states "Set UART clock source to APB, after this, baudrate needs to be set again to recalculate the clock divider". However, the code is changing the clock source to APB (value 1) but the baudrate is being set immediately after without any verification that the clock source change has taken effect. Consider adding a brief explanation of why this is safe, or if any additional synchronization is needed after changing the clock source.

Suggested change

SET_PERI_REG_BITS(UART_CLK_CONF_REG(0), UART_SCLK_SEL_V, 1, UART_SCLK_SEL_S);

SET_PERI_REG_BITS(UART_CLK_CONF_REG(0), UART_SCLK_SEL_V, 1, UART_SCLK_SEL_S);

// Ensure the clock source change has taken effect before setting baudrate.

// On ESP32-C3, register writes are synchronous, but a short delay is added for safety and future compatibility.

stub_lib_delay_us(10); // 10us is sufficient for peripheral clock domain sync

Dzarda7 · 2025-12-16T14:19:01Z

BTW to be completely correct, we should set dbias voltage for CPU accordingly to the frequency increase which we are not doing. It is done for ESP32, ESP32-C2 and ESP32-S2 as it is simple there. For newer chips we need to read the calibration value from eFuse, which is quite complicated so it was left out for now as we also did not do that in the old stub and issue was found only foe ESP32-S3 (even thought we might have missed some and considered them as HW issue).

erhankur · 2025-12-17T11:12:14Z

include/esp-stub-lib/clock.h

+
+#pragma once
+
+#include <stdint.h>


No need to include

erhankur · 2025-12-17T11:13:53Z

src/target/base/include/target/clock.h

+#pragma once
+
+#include <stdint.h>
+#include <stdbool.h>


No need to include stdbool.h

erhankur · 2025-12-17T11:17:32Z

src/target/common/src/uart.c

 {
-    uint32_t clock = esp_rom_get_cpu_freq() << 4;
-    uint32_t divisor = clock / baudrate;
+    extern void stub_lib_delay_us(uint32_t us);


#include <esp-stub-lib/rom_wrappers.h> Or without wrapper ets_delay_us. It is ok to call rom functions directly from targets.

erhankur · 2025-12-17T11:22:17Z

src/target/esp32/src/uart.c


 #define RTC_STORE5 (DR_REG_RTCCNTL_BASE + 0xb4)

+#define MHZ 1000000


For the definitions, we can create a new header under base/

erhankur · 2025-12-17T11:26:37Z

src/target/esp32/src/clock.c

+ */
+
+#include <stdint.h>
+#include <stdbool.h>


No need to include. Please check also other files.

erhankur · 2025-12-17T11:33:43Z

src/target/esp32c2/src/clock.c

+    esp_rom_set_cpu_ticks_per_us(s_cpu_freq / MHZ);
+}
+
+uint32_t stub_target_get_cpu_freq(void)


As I see, stub_target_get_cpu_freq same for all targets. You could return s_cpu_freq from targets and process it inside common?

erhankur · 2025-12-17T11:36:53Z

src/target/esp32c3/src/uart.c

-    esp_rom_set_cpu_ticks_per_us(esp_rom_get_xtal_freq() / 1000000);
-    stub_target_rom_uart_init(uart_num, UART_CLK_FREQ_ROM);
+    uint32_t clock = stub_target_get_apb_freq();
+    stub_target_rom_uart_init(uart_num, clock);


Suggested change

stub_target_rom_uart_init(uart_num, clock);

stub_target_rom_uart_init(uart_num, stub_target_get_apb_freq());

erhankur · 2025-12-17T11:42:24Z

src/target/esp32c3/src/clock.c

+    REG_SET_FIELD(SYSTEM_SYSCLK_CONF_REG, SYSTEM_SOC_CLK_SEL, 1);
+    REG_SET_FIELD(SYSTEM_CPU_PER_CONF_REG, SYSTEM_PLL_FREQ_SEL, 1);
+    REG_SET_FIELD(SYSTEM_CPU_PER_CONF_REG, SYSTEM_CPUPERIOD_SEL, 1);
+    s_cpu_freq = 160 * MHZ;


Also for other targets

#define CPU_FREQ_MHZ 160 // on top s_cpu_freq = CPU_FREQ_MHZ * MHZ; esp_rom_set_cpu_ticks_per_us(CPU_FREQ_MHZ);

erhankur · 2025-12-17T11:47:30Z

src/target/esp32c3/src/clock.c

+
+uint32_t stub_target_get_cpu_freq(void)
+{
+    if (s_cpu_freq == 0) {


Do you want to check if stub_target_clock_init is called? If not called, will it be different from the value that set in clock init?

Dzarda7 added 2 commits December 16, 2025 11:38

feat: Add CPU clock increase support

76490a1

fix: Use correct clock for UART baudrate

8c0a6a9

Dzarda7 requested review from erhankur and radimkarnis December 16, 2025 10:41

dobairoland requested a review from Copilot December 16, 2025 10:47

Copilot started reviewing on behalf of dobairoland December 16, 2025 10:50 View session

Copilot AI reviewed Dec 16, 2025

View reviewed changes

erhankur reviewed Dec 17, 2025

View reviewed changes

		PROVIDE ( esp_rom_get_xtal_freq = ets_clk_get_xtal_freq );
		PROVIDE ( esp_rom_get_cpu_freq = ets_clk_get_cpu_freq );

-    SET_PERI_REG_BITS(UART_CLK_CONF_REG(0), UART_SCLK_SEL_V, 1, UART_SCLK_SEL_S);
+    SET_PERI_REG_BITS(UART_CLK_CONF_REG(0), UART_SCLK_SEL_V, 1, UART_SCLK_SEL_S);
+    // Ensure the clock source change has taken effect before setting baudrate.
+    // On ESP32-C3, register writes are synchronous, but a short delay is added for safety and future compatibility.
+    stub_lib_delay_us(10); // 10us is sufficient for peripheral clock domain sync


		#define RTC_STORE5 (DR_REG_RTCCNTL_BASE + 0xb4)

		#define MHZ 1000000

	stub_target_rom_uart_init(uart_num, clock);
	stub_target_rom_uart_init(uart_num, stub_target_get_apb_freq());

Feat: Add cpu clock increase #42

Are you sure you want to change the base?

Feat: Add cpu clock increase #42

Uh oh!

Conversation

Dzarda7 commented Dec 16, 2025

Description

Related

Testing

Checklist

Uh oh!

github-actions bot commented Dec 16, 2025

Uh oh!

Dzarda7 commented Dec 16, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Dzarda7 commented Dec 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants