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

Reversions, cleanup, and fixes to the scanchain systems #978

Merged
merged 12 commits into from
Mar 15, 2022

Conversation

dragonmux
Copy link
Member

This set of commits seeks to address issues in the current JTAG scanchain detection and setup code.

The first problem this addresses is the way in which the post-scanchain-detection handler init is done as the table of ID Codes to handlers is needed for a future PR to provide support for JTAG-PDI, and passing the entire jtag_devs slot structure to the handler is horridly wasteful for information that can be gleaned a different way without this layering violation.

The second problem is in how chain scanning is being performed - the old code works a very particular way and this is necessary for correctly detecting things like ATXMega devices.

The third is the conflation of SWD and JTAG scan that is occurring, which is only partially addressed by this PR as SWD being able to defer on failure to JTAG at all is a violation of the boundy set by having specific jtag_scan and swd_scan monitor commands - a future PR will be submitted that entirely removes this deferral and introduces an auto_scan command that allows the BlackMagic Probe Firmware to slew through a pile of scan chain possibilities and try and auto-detect what is present.

@dragonmux dragonmux requested a review from esden January 22, 2022 04:11
@UweBonnes
Copy link
Contributor

Please break up in several PRs. b502a43, 1b677c5, d693647 are mostly syntactical fixes. Removal of the "auto-scan" is user interface change . For reverting the JTAG detection scheme, I think an issue to discuss is worth the effort. I have to access the impact on the RiscV and AVR8 branch. If jtag_scan again needs the irlens argument to scan through unknown chains, I think this would be a regression.

Copy link
Member

@esden esden left a comment

Choose a reason for hiding this comment

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

Overall it looks good to me. I am happy to merge this after the other questions are answered or amended.

src/target/adiv5.h Show resolved Hide resolved
src/target/adiv5.h Show resolved Hide resolved
src/target/jtag_devs.c Outdated Show resolved Hide resolved
@UweBonnes
Copy link
Contributor

I still urge to break up this PR in smaller, unrelated ones. This mixes up changes to the scan system with other things like syntactical changes and user interface changes.
The scan system targeted to revert is used in the present way in the riscv branch #924 and https://github.com/UweBonnes/blackmagic/tree/avr (able to flash JTAG-AVRs and some Xilinx parts). Does the present way really not find the XMega? The present way tries more auto-detection on unknown chains, while the ability to provide scan length number is still given.

Of course auto/swd/jtag scan is a good idea, but still worth it's own PR.

@esden
Copy link
Member

esden commented Mar 15, 2022

There is a bunch of patches in the pipeline that will improve things regarding device detection and supporting new platforms. They depend on this cleanup patch. I agree that we should strive to make pull requests transparent and focused on one task. But after talking to @DX-MON I think we will have to make an exception in this particular case. So I am merging this.

Thanks for the hard work @DX-MON :)

@esden esden merged commit e271c16 into master Mar 15, 2022
@dragonmux dragonmux deleted the scanchain-cleanup branch March 15, 2022 12:15
dragonmux added a commit that referenced this pull request Mar 15, 2022
dragonmux added a commit that referenced this pull request Mar 16, 2022
dragonmux added a commit that referenced this pull request Mar 16, 2022
dragonmux added a commit that referenced this pull request Mar 31, 2022
dragonmux added a commit that referenced this pull request May 17, 2022
dragonmux added a commit that referenced this pull request May 17, 2022
dragonmux added a commit that referenced this pull request Jun 25, 2022
dragonmux added a commit that referenced this pull request Jun 27, 2022
dragonmux added a commit that referenced this pull request Jul 20, 2022
dragonmux added a commit that referenced this pull request Jul 20, 2022
dragonmux added a commit that referenced this pull request Jul 24, 2022
dragonmux added a commit that referenced this pull request Jul 24, 2022
dragonmux added a commit that referenced this pull request Jul 24, 2022
dragonmux added a commit that referenced this pull request Jul 24, 2022
dragonmux added a commit that referenced this pull request Jul 24, 2022
dragonmux added a commit that referenced this pull request Jul 24, 2022
dragonmux added a commit that referenced this pull request Jul 24, 2022
dragonmux added a commit that referenced this pull request Jul 24, 2022
dragonmux added a commit that referenced this pull request Jul 24, 2022
dragonmux added a commit that referenced this pull request Jul 24, 2022
dragonmux added a commit that referenced this pull request Jul 24, 2022
dragonmux added a commit that referenced this pull request Jul 24, 2022
dragonmux added a commit that referenced this pull request Jul 24, 2022
dragonmux added a commit that referenced this pull request Jul 24, 2022
dragonmux added a commit that referenced this pull request Jul 24, 2022
dragonmux added a commit that referenced this pull request Aug 8, 2022
dragonmux added a commit that referenced this pull request Aug 8, 2022
dragonmux added a commit that referenced this pull request Aug 14, 2022
dragonmux added a commit that referenced this pull request Aug 14, 2022
dragonmux added a commit that referenced this pull request Aug 14, 2022
dragonmux added a commit that referenced this pull request Aug 14, 2022
dragonmux added a commit that referenced this pull request Aug 14, 2022
dragonmux added a commit that referenced this pull request Sep 23, 2022
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