Skip to content

Conversation

@Dzarda7
Copy link
Collaborator

@Dzarda7 Dzarda7 commented Dec 16, 2025

Description

This PR simplifies USB-Serial-JTAG logic a bit by removing common file and one function per target that supports USB-Serial-JTAG. This also adds USB-Serial-JTAG into the build test.

This mainly adds watchdog disable, because watchdog needs to be disabled in order to preven reset in the middle of a communication. By default watchdog is disabled after chip reset, but second stage bootloader may enable it again and if application does not disable it, it will reset the device, because USB-Serial/JTAG does only software reset.

Related

Testing


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.

Dzarda7 and others added 3 commits December 16, 2025 15:50
This commit removes necessitty to define support function per target
and removes common usb_serial_jtag c file. This also adds usb-Serial-JTAG
functions to the build test.
Watchdog needs to be disabled in order to preven reset in the middle of
a communication. By default watchdog is disabled after chip reset,
but second stage bootloader may enable it again and if application does
not disable it, it will reset the device, because USB-Serial/JTAG does
only software reset.
@github-actions
Copy link

github-actions bot commented Dec 16, 2025

Warnings
⚠️

Some issues found for the commit messages in this PR:

  • the commit message "Apply automatic fixes from pre-commit hooks":
    • summary looks empty
    • type/action looks empty

Please fix these commit messages - here are some basic tips:

  • follow Conventional Commits style
  • correct format of commit message should be: <type/action>(<scope/component>): <summary>, for example fix(esp32): Fixed startup timeout issue
  • allowed types are: change,ci,docs,feat,fix,refactor,remove,revert,test
  • sufficiently descriptive message summary should be between 20 to 72 characters and start with upper case letter
  • avoid Jira references in commit messages (unavailable/irrelevant for our customers)

TIP: Install pre-commit hooks and run this check when committing (uses the Conventional Precommit Linter).

Messages
📖 You might consider squashing your 3 commits (simplifying branch history).
📖 This PR seems to be quite large (total lines of code: 2700), 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.
- Resolve all warnings (⚠️ ) before requesting a review from human reviewers - they will appreciate it.
- 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 a65e20d

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds watchdog disable functionality for USB-Serial-JTAG to prevent device resets during communication. The changes remove the common implementation file and the per-target stub_target_usb_serial_jtag_is_supported() function, replacing it with target-specific stub_target_usb_serial_jtag_disable_watchdogs() implementations.

Key changes include:

  • New public API function stub_lib_usb_serial_jtag_disable_watchdogs()
  • Target-specific implementations for disabling RWDT and configuring SWD to autofeed
  • New LP_WDT register definition headers for multiple targets (ESP32C5/C6/C61, ESP32H2/H4/H21, ESP32P4)

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
include/esp-stub-lib/usb_serial_jtag.h Adds public API declaration for watchdog disable function
src/usb_serial_jtag.c Implements wrapper with conditional compilation for watchdog disable
src/target/base/include/target/usb_serial_jtag.h Updates internal API signature from is_supported to disable_watchdogs
src/target/esp32s3/src/usb_serial_jtag.c Implements RTC watchdog disable using RTC_CNTL registers
src/target/esp32p4/src/usb_serial_jtag.c Implements LP watchdog disable for ESP32P4
src/target/esp32h4/src/usb_serial_jtag.c Implements LP watchdog disable for ESP32H4
src/target/esp32h21/src/usb_serial_jtag.c Implements LP watchdog disable for ESP32H21
src/target/esp32h2/src/usb_serial_jtag.c Implements LP watchdog disable for ESP32H2
src/target/esp32c61/src/usb_serial_jtag.c Implements LP watchdog disable for ESP32C61
src/target/esp32c6/src/usb_serial_jtag.c Implements LP watchdog disable for ESP32C6
src/target/esp32c5/src/usb_serial_jtag.c Implements LP watchdog disable for ESP32C5
src/target/esp32c3/src/usb_serial_jtag.c Implements RTC watchdog disable using RTC_CNTL registers
src/target/*/include/soc/lp_wdt_reg.h New register definition headers for LP watchdog peripherals
src/target/common/CMakeLists.txt Removes common usb_serial_jtag.c from build
example/stub_main.c Adds test function demonstrating watchdog disable usage

{
return true;
// Disable RWDT (RTC Watchdog)
WRITE_PERI_REG(RTC_CNTL_WDTWPROTECT_REG, RTC_CNTL_WDT_WKEY);
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The watchdog write protection key macro name uses "WKEY" but should use "KEY" to match the actual macro definition pattern. Line 27 writes RTC_CNTL_WDT_WKEY but the macro is defined as RTC_CNTL_WDT_KEY on line 21. This will cause a compilation error.

Copilot uses AI. Check for mistakes.
WRITE_PERI_REG(RTC_CNTL_WDTWPROTECT_REG, 0x0);

// Configure SWD (Super Watchdog) to autofeed
WRITE_PERI_REG(RTC_CNTL_SWD_WPROTECT_REG, RTC_CNTL_SWD_WKEY);
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The super watchdog write protection key macro name uses "WKEY" but should use "KEY" to match the actual macro definition pattern. Line 32 writes RTC_CNTL_SWD_WKEY but the macro is defined as RTC_CNTL_SWD_KEY on line 22. This will cause a compilation error.

Copilot uses AI. Check for mistakes.
{
return true;
// Disable RWDT (RTC Watchdog)
WRITE_PERI_REG(RTC_CNTL_WDTWPROTECT_REG, RTC_CNTL_WDT_WKEY);
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The watchdog write protection key macro name uses "WKEY" but should use "KEY" to match the actual macro definition pattern. Line 27 writes RTC_CNTL_WDT_WKEY but the macro is defined as RTC_CNTL_WDT_KEY on line 21. This will cause a compilation error.

Copilot uses AI. Check for mistakes.
WRITE_PERI_REG(RTC_CNTL_WDTWPROTECT_REG, 0x0);

// Configure SWD (Super Watchdog) to autofeed
WRITE_PERI_REG(RTC_CNTL_SWD_WPROTECT_REG, RTC_CNTL_SWD_WKEY);
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The super watchdog write protection key macro name uses "WKEY" but should use "KEY" to match the actual macro definition pattern. Line 32 writes RTC_CNTL_SWD_WKEY but the macro is defined as RTC_CNTL_SWD_KEY on line 22. This will cause a compilation error.

Copilot uses AI. Check for mistakes.
WRITE_PERI_REG(RTC_CNTL_WDTCONFIG0_REG, 0x0);
WRITE_PERI_REG(RTC_CNTL_WDTWPROTECT_REG, 0x0);

// Configure SWD (Super Watchdog) to autofeed
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in comment - "preven" should be "prevent"

Copilot uses AI. Check for mistakes.
Comment on lines +22 to +24
* This function disables the watchdogs for USB-Serial/JTAG to prevent the device from resetting.
* By default watchdog is disabled after chip reset, but second stage bootloader may enable it again,
* and if application does not disable it, it will reset the device, because USB-Serial/JTAG does only software reset.
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in comment - "preven" should be "prevent"

Copilot uses AI. Check for mistakes.
Comment on lines +15 to +17
* This function disables the watchdogs for USB-Serial/JTAG to prevent the device from resetting.
* By default watchdog is disabled after chip reset, but second stage bootloader may enable it again,
* and if application does not disable it, it will reset the device, because USB-Serial/JTAG does only software reset.
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in comment - "preven" should be "prevent"

Copilot uses AI. Check for mistakes.
@radimkarnis
Copy link
Member

I know this is intended to solve an issue in the USB-Serial/JTAG mode, but it is not related to the peripheral in any way. Should this be moved to another place? e.g. clock.c introduced in the other PR. This (clk.c) is also where most of the RTC WDT logic happens in IDF.

stub_target_usb_serial_jtag_disable_watchdogs -> stub_target_rtc_wdt_disable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants