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

ARM:Add Jtag to Dormant sequence in case the target has both jtag and swd #1959

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mean00
Copy link

@mean00 mean00 commented Oct 13, 2024

Detailed description

  • The current Dormant to SWD code manages correctly the case where SWD is default/the only interface
  • If the chip has both SWD & Jtag and default is Jtag, it lacks the Jtag->dormant sequence and does not work
  • The proposed patch adds that sequence

For what i could see, it still works with RP2040 and older Arm chips i have (STM32F1/GD32F3)
Tested on a chip with both Jtag & SWD/Default jtag + Debug Protocol v3, it works with the change, doesnt without

NB: Not completely sure the 8 trailer '1' bits are really needed, it works fine with, was there in the demo code

Your checklist for this pull request

Closing issues

@mean00 mean00 force-pushed the mean00/add_jtag_to_dormant_sequence branch from ee984ad to b79fc11 Compare October 13, 2024 07:31
Copy link
Member

@dragonmux dragonmux left a comment

Choose a reason for hiding this comment

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

Having picked through the spec to check this, this all looks good. With our only review notes (stylistic) fixed, please rebase this on main and we'll get it merged.

We checked the specification where it talks about how to step through the various state machines, and per the spec, those 8 high cycles are required:

image

/* Send at least 8 SWCLKTCK cycles with SWDIOTMS HIGH */
swd_line_reset_sequence(false);

/* If the target is both Jtag and SWD with Jtag as default, switch Jtag->DS first.
Copy link
Member

Choose a reason for hiding this comment

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

Please put the opening /* on its own line like in the comment blocks above this.

/* If the target is both Jtag and SWD with Jtag as default, switch Jtag->DS first.
* See B5.3.2
*/
DEBUG_INFO("Switching from jtag do dormant \n");
Copy link
Member

Choose a reason for hiding this comment

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

Please remove the extra space between the end of the sentence and the \n, and capitalise JTAG.

@dragonmux dragonmux added this to the v2.0 release milestone Oct 23, 2024
@dragonmux dragonmux added Bug Confirmed bug New Target New debug target labels Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Confirmed bug New Target New debug target
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants