arm: retry WAIT errors at a lower level. Fixes nrf51 flashing. #2119
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
the issue is this logic here https://github.com/probe-rs/probe-rs/blob/master/probe-rs/src/probe/arm_debug_interface.rs#L448-L449
Currently we assume for AP writes the result comes back in the next transfer for all errors. However this is not the case for WAIT errors. This causes WAIT errors to be incorrectly returned 1 transfer ahead (ie if transfer 5 failed with WAIT, it's returned on transfer 4), which causes the caller to retry transfer 4 onwards, duplicating transfer 4. This causes the word to be duplicated in the memory write.
Instead, retry below the
perform_transfers
logic that handles the "delayed faults". This way WAITs are naturally not subject to that logic. It also makes the code a bit nicer, there's now 1 copy of the retry loop instead of 4.Also, this PR removes retry on other errors (not WAIT or FAULT), for simplicity. https://github.com/probe-rs/probe-rs/blob/master/probe-rs/src/probe/arm_debug_interface.rs#L1269-L1275
TODO:
perform_raw_transfers_retry
doesn't know the original purpose of the transfers so it can't use the right field from SwdSettings. Do we need this, or can we simplify SwdSettings a bit?Tested working with nrf51422 and nrf52840.