fix SWD turnaround issues

[tlyu]

Detailed description

There were multiple problems with SWD turnaround. In the write-to-read direction, a short clock pulse could be produced (#1706). When terminating a read transaction, the clock wasn't driven low. If a subsequent clock float was requested, as for a detach, the clock would float low, then be driven high when re-enabled, causing an extra rising clock edge.

Additionally, idle cycles weren't being inserted as required after a read transaction.

There's a small chance of a regression with targets that pull the SWD clock high.

Your checklist for this pull request

• I've read the Code of Conduct
• I've read the guidelines for contributing to this repository
• It builds for hardware native (make PROBE_HOST=native)
• It builds as BMDA (make PROBE_HOST=hosted)
• I've tested it to the best of my ability
• My commit messages provide a useful short description of what the commits do

Closing issues

Fixes #1706.

Details

These are debug logs from BMDA.

Log excerpt from unpatched, attaching after detaching. Note the non-response error after re-enabling the clock.

!SI20#
       K2000497C
remote_v0_swd_seq_in_parity 32 clock_cycles: 2000497c OK
Read RDBUFF: 0x2000497c
!GE0#
       K0
gdb_putpacket: OK
gdb_getpacket: vAttach;1
!GE1#
       K0
remote_v0_swd_seq_out 8 clock_cycles: 0000008d
!So088d#
       K0
!Si03#
       K7
remote_v0_swd_seq_in 3 clock_cycles: 00000007
!SI20#
       PFFFFFFFF
remote_v0_swd_seq_in_parity 32 clock_cycles: ffffffff ERR
Read STATUS: 0x00000000
DP Error 0x00000000
ap_mem_write_sized @ e000edf0 len 4, align 4: 03 00 5f a0
Write AP 0 CSW: 0xa3000052
remote_v0_swd_seq_out 8 clock_cycles: 000000b1
!So08b1#
       K0
!Si03#
       K7
remote_v0_swd_seq_in 3 clock_cycles: 00000007
SWD access resulted in no response
!SI20#
       PFFFFFFFF
remote_v0_swd_seq_in_parity 32 clock_cycles: ffffffff ERR
Recovering and re-trying access

Full log from unpatched code:
unpatched.txt

Log excerpt from patched, with only the swdptap_turnaround fix. Note the lack of non-response error after re-enabling the clock.

!SI20#
       K1
remote_v0_swd_seq_in_parity 32 clock_cycles: 00000001 OK
Read RDBUFF: 0x00000001
!GE0#
       K0
gdb_putpacket: OK
gdb_getpacket: vAttach;1
!GE1#
       K0
remote_v0_swd_seq_out 8 clock_cycles: 0000008d
!So088d#
       K0
!Si03#
       K1
remote_v0_swd_seq_in 3 clock_cycles: 00000001
!SI20#
       KF0000040
remote_v0_swd_seq_in_parity 32 clock_cycles: f0000040 OK
Read STATUS: 0xf0000040
DP Error 0x00000000

Full log from patch with only swdptap_turnaround fix:
ta-no-8idle.txt

Full log with both commits:
ta-with-8idle.txt

[tlyu]

Target is a GD32F303 on a Keyboardio Model 100 keyboard. Probe is BMP native, hardware v2.3b. There are 10k pull up resistors on all JTAG/SWD lines, except JTCLK/SWDCLK, which has a 10k pull down. (I misread the schematic when reporting this to Discord earlier.) In any case, the user manual for the chip says at reset, JTDI, JTMS/SWDIO, NJTRST are pulled up, JTCK/SWDCLK is pulled down, and JTDO is floating.

[dragonmux]

This is looking good - there are a couple of stylistic review notes we've got, but functionally this all looks good - we'll get this merged once the review items have been addressed as we can see it fixes the bug and we're satisfied that it's all functioning properly.

Particularly appreciate this gives an explanation for the attach bug we'd hit near the release of v1.9 and couldn't explain! We knew at the time that protocol recovery was just papering over the real problem, but this explains exactly what the problem actually was and fixes its root cause which is awesome.

[tlyu]

Thanks for the review! I addressed your style comments.

[dragonmux]

All looks good to us now - good stuff, merging once we've briefly tested this with a couple of targets for good measure. Thank you for the contribution!

[tlyu]

I created the patch for the missing turnaround delay before seeing that @stansotn had already made a very similar one. I can add credit to the commit message, if that's desired.

[dragonmux]

Yeah, probably a kindness to add a Co-Authored-By: line for them, good thinking!

[dragonmux]

Well, we can confirm this all works as intended on our RP2040 and a STM32F411 and the attach protocol recovery trigger is now gone 👍🏼

[tlyu]

@stansotn do you consent to be listed as co-author on this commit, and if so, should I use the author name and email address that you used in the commit you linked to #1706? Thanks!

[stansotn]

@tlyu

do you consent to be listed as co-author on this commit, and if so, should I use the author name and email address that you used in the commit you linked to #1706? Thanks!

I just changed the authorship of the commit to my personal email (gmail). I consent to be included as co-author. Thanks for asking!

stansotn@aef2a64

[dragonmux]

Merging now the authorship bit is solved, thank you all for your efforts and work on this!