deassert_reset should not wait for a target to come out of reset and should not update target->state

[en-sc]

Currently there is a loop in deassert_reset() handler that waits for the hart to come out of reset.

riscv-openocd/src/target/riscv/riscv-013.c

Lines 2930 to 2953 in e870c5f

	LOG_TARGET_DEBUG(target, "Waiting for hart to come out of reset.");
	do {
	result = dmstatus_read(target, &dmstatus, true);
	if (result != ERROR_OK)
	return result;
	if (time(NULL) - start > riscv_get_command_timeout_sec()) {
	LOG_TARGET_ERROR(target, "Hart didn't leave reset in %ds; "
	"dmstatus=0x%x (allunavail=%s, allhavereset=%s); "
	"Increase the timeout with riscv set_command_timeout_sec.",
	riscv_get_command_timeout_sec(), dmstatus,
	get_field(dmstatus, DM_DMSTATUS_ALLUNAVAIL) ? "true" : "false",
	get_field(dmstatus, DM_DMSTATUS_ALLHAVERESET) ? "true" : "false");
	return ERROR_TIMEOUT_REACHED;
	}
	/* Certain debug modules, like the one in GD32VF103
	* MCUs, violate the specification's requirement that
	* each hart is in "exactly one of four states" and,
	* during reset, report harts as both unavailable and
	* halted/running. To work around this, we check for
	* the absence of the unavailable state rather than
	* the presence of any other state. */
	} while (get_field(dmstatus, DM_DMSTATUS_ALLUNAVAIL) &&
	!get_field(dmstatus, DM_DMSTATUS_ALLHAVERESET));

This is not necessary -- there is a call to arp_waitstate:

riscv-openocd/src/target/startup.tcl

Line 140 in e870c5f

catch { $t arp_waitstate halted 1000 }

The task is to move handling of a target coming out of reset from deassert_reset() to poll(). This will also allow to drop the awkward handling of an unexpected reset in riscv013_get_hart_state():

riscv-openocd/src/target/riscv/riscv-013.c

Lines 2716 to 2737 in e870c5f

	if (get_field(dmstatus, DM_DMSTATUS_ANYHAVERESET)) {
	LOG_TARGET_INFO(target, "Hart unexpectedly reset!");
	info->dcsr_ebreak_is_set = false;
	/* TODO: Can we make this more obvious to eg. a gdb user? */
	uint32_t dmcontrol = DM_DMCONTROL_DMACTIVE |
	DM_DMCONTROL_ACKHAVERESET;
	dmcontrol = set_dmcontrol_hartsel(dmcontrol, info->index);
	/* If we had been halted when we reset, request another halt. If we
	* ended up running out of reset, then the user will (hopefully) get a
	* message that a reset happened, that the target is running, and then
	* that it is halted again once the request goes through.
	*/
	if (target->state == TARGET_HALTED) {
	dmcontrol |= DM_DMCONTROL_HALTREQ;
	/* `haltreq` should not be issued if `abstractcs.busy`
	* is set. */
	int result = wait_for_idle_if_needed(target);
	if (result != ERROR_OK)
	return result;
	}
	dm_write(target, DM_DMCONTROL, dmcontrol);
	}

[en-sc]

Huge thanks to @tom-van for pointing this out. Please see #1214 (comment) for the original comment.

[tom-van]

Well at least Cortex-Ms deassert_reset() leaves TARGET_RESET state until poll() detects that CPU runs or is halted.
Unlike this Cortex-A calls poll() in deassert_reset() to update the state immediately.

RISC-V has much better designed req - ack structure of control/status and we are able to detect that reset is done.
Yes, I can imagine deassert_reset() just clears NMDRESET and the rest of processing is done in the following poll().

Anyway the reset assert/deassert implementation in OpenOCD has serious shortcomings for multi-cores and needs significant rewrite. I tried once, but don't have enough power to uphold it.