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

dragonmux commented 2022年01月22日 05:11:06 +01:00

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.

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.

UweBonnes commented 2022年01月22日 14:05:53 +01:00 (Migrated from github.com)

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.

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.

UweBonnes commented 2022年03月14日 13:33:37 +01:00 (Migrated from github.com)

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.

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 commented 2022年03月15日 05:37:38 +01:00 (Migrated from github.com)

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 :)

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 :)