Add local reset via VECTRESET support for cortex-m device #114
Conversation
When use setTargetResetState method, apart from using external signals for a global reset through SYSRESETREQ, sometimes we prefer to trigger local reset, which is achieved through VECTRESET.
|
I'm not sure how I would test this. @jreineckearm does this look sensible? |
| * @returns Promise | ||
| */ | ||
| public async setTargetResetState(hardwareReset: boolean = true): Promise<void> { | ||
| public async setTargetResetState(hardwareReset: boolean = true, localReset: boolean = false, timeout: number = 1000): Promise<void> { |
There was a problem hiding this comment.
- Adding new parameters
localResetandtimeouthas IMO potential for confusion. They get skipped ifhardwareResetistrue. Adding a new method with the enhanced functionality (and reset types as enums) might be a better approach. Alternatively, the dependencies between such parameters needs to be better documented in the API description. - Also, local/vector resets are only available for v7-M based CPUs, e.g. Cortex-M3/M4/M7. v6-M and v8-M based CPUs don't have that capability. Writes to the
AIRCR.VECTRESETbit should be ignored by HW. But one could argue that it is the consumer's responsibility to check if a certain operation will succeed. - Adding
timeoutand checking for reset recovery is in general a good idea for all reset types (also the HW reset). But as done here, the changes significantly impact timings and functionality of this function. I am a little worried about the impact on other consumers ofdapjsnot expecting the newly introduced delays forSYSRESETREQ. timeoutisn't really a timeout here but a delay. For a timeout, it should regularly pollDHCSRand return early once the reset completed. Otherwise, this method is now delayed by 1 second by default. When polling, you need to make sure to handle target access errors. Depending on the reset implementation in HW, the DAP/CPU subsystem/CPU debug registers may be temporarily unavailable.- If going the route of checking for reset recovery you may want to indicate this via a return value of the method. This allows consumers handling of a the case where the reset does not recover (in time).
| public async setTargetResetState(hardwareReset: boolean = true): Promise<void> { | ||
| public async setTargetResetState(hardwareReset: boolean = true, localReset: boolean = false, timeout: number = 1000): Promise<void> { | ||
| // Enable Reset Vector Catch | ||
| await this.halt(); |
There was a problem hiding this comment.
I would be careful with halting the CPU here unconditionally. This changes behavior of this method while strictly speaking writing AIRCR.SYSRESETREQ doesn't come with being unpredictable if the CPU is running before reset. On the other hand, the reset vector catch is always enabled in this method as well. So this probably only impacts corner cases.
There was a problem hiding this comment.
Makes sense, maybe only halt when using VECTRESET?
@thegecko , IIRC, the |
|
Here are two points:
|
When use setTargetResetState method, apart from using external signals for a global reset through SYSRESETREQ, sometimes we prefer to trigger local reset, which is achieved through VECTRESET.