fix software reset by writing in SYSRESETQ register when a reset is requested

[kerms]

Hi, since software reset is never triggered, and hardware reset is triggered for both soft/hard.

I'm thinking why not add soft reset before hard reset for any reset request, that will make soft reset working when nRESET is not connected.

I added a write to SYSRESETQ register to trigger a software reset manually.

That cause software + hardware reset in anycase instead of hard reset only.

I have tested on esp32s3-WROOM-1-N16R8.

Can you test it on your side to confirm that is working ?

[windowsair]

Thanks for your contribution.

Your workaround seems to be addressing the issue of cmsis-dap not working in keil?

I have seen similar code. Most of them are strongly related to debug protocols (swd), or target architectures (cortex-m). Could you add more comments to clarify the code?

[kerms]

Your workaround seems to be addressing the issue of cmsis-dap not working in keil?

Yes, when using SWD and setting SYSRESETQ on keil for SMT32F103C8T6.

I have seen similar code. Most of them are strongly related to debug protocols (swd), or target architectures (cortex-m). Could you add more comments to clarify the code?

This code uses SWD protocol to write to the AIRCR core register, and specifically target cortex-m0, m1X and cortex-m2X.
Normally its up to Keil to write to the specific reset register though :/

That have some compatibility problems:

target specific compatibility: You are right that above cortex-m3, there exists AIRCR_PRIGROUP(3 bits) that should not be overwritten with 0b000 with this code. To make things not broken, a read on AIRCR register should be performed to get AIRCR_PRIGROUP bits, then OR these bits to the write. Also this code does not take account the change of reserved AIRCR bits in the future.

Single shoot reset compability: The added codes work only after a flash operation, as the SWD line is already initialized after flashing. To trigger a single software reset without flashing, the SWD line should be reinitialized. But if reinitialize the SWD line, this will broke the current debugging context(e.g. debug port SW-DP/SW-AP changed).

potential fix:

• save JTAG/SWD mode when the debug port is initialized
• if SWD reset, read the current debug port context through SWD, get the target chip info, then do soft reset only on known target (arm cortex-m1 to m7 + sc300)

[windowsair]

Your approach can be applied to legacy Cortex-M devices, including M0, M0+, M3, M4, M7, M23, M33. That's great, but there are still some limitations. For example, we need to assume that there is only one CPU on the system, or the currently selected CPU is the one we want to reset. In other words, this may not apply to some simple SoC.

Regarding SWD and JTAG, I think it might relatively simple. It's reading and writing memory for upper level applications, and for the DAP itself it's just some access to AP and DP resources. By the way, we always know what protocol we are currently selected, because ResetTarget will be called after DAP_Connect, which configured to use SWD or JTAG.

Since we don't know as much information about the debugging device as the upper level application does, I'd rather leave this as an optional feature that is turned off by default, like we do in main/dap_configuration.h. Everything else is fine, and I think this code is ready to be merged :)

Also, on the basis of your pr, I have a lot of thing want to do. I think the first step might be to add SWD soft reset support, the second step to add JTAG soft reset support, and the third step to extend general memory read/write support. But I'm not sure I'll have the time to do them.

Once again, great job. Feel free to make any modifications, and please remind me to merge these code when appropriate.

[kerms]

Since we don't know as much information about the debugging device as the upper level application does

You are right, there is only IDCODE about the part number, which depends on the manufacturer -> need a board database(not exist) to know what debugging device is connected.

This is my PoC code note for single shoot reset in case you are interested. Mostly from https://github.com/adafruit/Adafruit_DAP and https://github.com/ataradov/edbg

code

#include "cmsis-dap/include/DAP.h"

#include <stdio.h>

#define AIRCR_REG_ADDR 0xE000ED0C
#define AIRCR_RESET_VAL ((0x5FA << 16) | (1 << 2))

enum {
	SWD_AP_CSW = 0x00 | DAP_TRANSFER_APnDP,
	SWD_AP_TAR = 0x04 | DAP_TRANSFER_APnDP,
	SWD_AP_DRW = 0x0c | DAP_TRANSFER_APnDP,

	SWD_AP_DB0 = 0x00 | DAP_TRANSFER_APnDP, // 0x10
	SWD_AP_DB1 = 0x04 | DAP_TRANSFER_APnDP, // 0x14
	SWD_AP_DB2 = 0x08 | DAP_TRANSFER_APnDP, // 0x18
	SWD_AP_DB3 = 0x0c | DAP_TRANSFER_APnDP, // 0x1c

	SWD_AP_CFG = 0x04 | DAP_TRANSFER_APnDP,  // 0xf4
	SWD_AP_BASE = 0x08 | DAP_TRANSFER_APnDP, // 0xf8
	SWD_AP_IDR = 0x0c | DAP_TRANSFER_APnDP,  // 0xfc
};

enum {
	SWD_DP_R_IDCODE = 0x00,
	SWD_DP_W_ABORT = 0x00,
	SWD_DP_R_CTRL_STAT = 0x04,
	SWD_DP_W_CTRL_STAT = 0x04, // When CTRLSEL == 0
	SWD_DP_W_WCR = 0x04,       // When CTRLSEL == 1
	SWD_DP_R_RESEND = 0x08,
	SWD_DP_W_SELECT = 0x08,
	SWD_DP_R_RDBUFF = 0x0c,
};

uint8_t dap_write_reg(uint8_t reg, uint32_t data) {
	uint8_t buf[8];
	uint8_t response[4];
	uint32_t status;

	buf[0] = ID_DAP_Transfer;
	buf[1] = 0x00; // DAP index
	buf[2] = 0x01; // Request size
	buf[3] = reg;
	buf[4] = data & 0xff;
	buf[5] = (data >> 8) & 0xff;
	buf[6] = (data >> 16) & 0xff;
	buf[7] = (data >> 24) & 0xff;
	status = DAP_ExecuteCommand(buf, response);
	printf("status :%lx, 0: %x, 1: %x, 2: %x, 3: %x\n", status, response[0], response[1], response[2], response[3]);

	return true;
}

uint32_t dap_read_reg(uint8_t reg) {
	uint8_t buf[8];
	uint8_t response[8];
	uint32_t status;

	buf[0] = ID_DAP_Transfer;
	buf[1] = 0x00; // DAP index
	buf[2] = 0x01; // Request size
	buf[3] = reg | DAP_TRANSFER_RnW;
	status = DAP_ExecuteCommand(buf, response);

	printf("status :%lx, %x %x %x %x\n", status, response[0], response[1], response[2], response[3]);
	printf("status :%lx, %x %x %x %x\n", status, response[4], response[5], response[6], response[7]);


	return ((uint32_t)buf[5] << 24) | ((uint32_t)buf[4] << 16) |
	       ((uint32_t)buf[3] << 8) | (uint32_t)buf[2];
}

uint8_t dap_write_word(uint32_t addr, uint32_t data) {
	dap_write_reg(SWD_AP_TAR, addr);
	dap_write_reg(SWD_AP_DRW, data);
	return true;
}

uint8_t dap_target_prepare(void) {
	dap_write_reg(SWD_DP_W_ABORT, 0x00000016); // DP_ABORT_STKCMPCLR  = 2| DP_ABORT_STKERRCLR = 4 | DP_ABORT_ORUNERRCLR = 0x10
	dap_write_reg(SWD_DP_W_SELECT, 0x00000000); // DP_SELECT_APBANKSEL(0) = 0 | DP_SELECT_APSEL(0) = 0
	dap_write_reg(SWD_DP_W_CTRL_STAT, 0x50000f00); // DP_CST_CDBGPWRUPREQ = 0x10000000 | DP_CST_CSYSPWRUPREQ = 0x40000000| DP_CST_MASKLANE(0xf) = 0x0F00
	dap_write_reg(SWD_AP_CSW, 0x23000052); // AP_CSW_ADDRINC_SINGLE = 0x10 | AP_CSW_DEVICEEN = 0x40 | AP_CSW_PROT(0x23) = 0x23000000 | AP_CSW_SIZE_WORD = 0x02
	return true;
}


uint8_t dap_trigger_reset()
{
	uint8_t response[10];
	uint32_t status;

	// Disconnect the device
	uint8_t disconenctRequest[] = { ID_DAP_Disconnect, 0x00 };
	status = DAP_ExecuteCommand(disconenctRequest, response);
	printf("status :%lx, 0: %x, 1: %x, 2: %x, 3: %x\n", status, response[0], response[1], response[2], response[3]);

	// Connect to the device
	uint8_t connectRequest[] = { ID_DAP_Connect, DAP_PORT_SWD };
	status = DAP_ExecuteCommand(connectRequest, response);
	printf("status :%lx, 0: %x, 1: %x, 2: %x, 3: %x\n", status, response[0], response[1], response[2], response[3]);
	if (DAP_PORT_SWD != response[1]) {
		printf((char *)"DAP_CONNECT failed\n");
	}

	/* Configure Transfer */
	uint8_t transferConfigRequest[] = { ID_DAP_TransferConfigure, 0x00, 0x40, 0x00, 0x40, 0x00 };
	status = DAP_ExecuteCommand(transferConfigRequest, response);
	printf("status :%lx, 0: %x, 1: %x, 2: %x, 3: %x\n", status, response[0], response[1], response[2], response[3]);

	uint8_t swdConfigRequest[] = { ID_DAP_SWD_Configure, 0x00};
	status = DAP_ExecuteCommand(swdConfigRequest, response);
	printf("status :%lx, 0: %x, 1: %x, 2: %x, 3: %x\n", status, response[0], response[1], response[2], response[3]);

	/* Configure clock */
	uint32_t clock = 1000000;
	uint8_t swjClockRequest[] = {ID_DAP_SWJ_Clock,
	                             clock & 0xff,
	                             (clock >> 8) & 0xff,
	                             (clock >> 16) & 0xff,
	                             (clock >> 24) & 0xff,
	};
	status = DAP_ExecuteCommand(swjClockRequest, response);
	printf("status :%lx, 0: %x, 1: %x, 2: %x, 3: %x\n", status, response[0], response[1], response[2], response[3]);

	uint8_t buf[128];

	//------------- JTAG TO SWD sequence
	buf[0] = ID_DAP_SWJ_Sequence;
	buf[1] = (7 + 2 + 7 + 1) * 8;
	buf[2] = 0xff;
	buf[3] = 0xff;
	buf[4] = 0xff;
	buf[5] = 0xff;
	buf[6] = 0xff;
	buf[7] = 0xff;
	buf[8] = 0xff;
	buf[9] = 0x9e;
	buf[10] = 0xe7;
	buf[11] = 0xff;
	buf[12] = 0xff;
	buf[13] = 0xff;
	buf[14] = 0xff;
	buf[15] = 0xff;
	buf[16] = 0xff;
	buf[17] = 0xff;
	buf[18] = 0x00;
	status = DAP_ExecuteCommand(buf, response);
	printf("status :%lx, 0: %x, 1: %x, 2: %x, 3: %x\n", status, response[0], response[1], response[2], response[3]);

	vTaskDelay(1);

	/* READ IDCODE to unlock SWD interface */
	buf[0] = ID_DAP_Transfer;
	buf[1] = 0; // DAP index
	buf[2] = 1; // Request size
	buf[3] = DP_IDCODE | DAP_TRANSFER_RnW;
	status = DAP_ExecuteCommand(buf, response);
	printf("status :%lx, 0: %x, 1: %x, 2: %x, 3: %x\n", status, response[0], response[1], response[2], response[3]);

	dap_target_prepare();

	/* Unknown reason: Need send twice to make a reset */
	buf[0] = ID_DAP_ResetTarget;
	status = DAP_ExecuteCommand(buf, response);
	printf("status :%lx, 0: %x, 1: %x, 2: %x, 3: %x\n", status, response[0], response[1], response[2], response[3]);
	
	vTaskDelay(1);

	buf[0] = ID_DAP_ResetTarget;
	status = DAP_ExecuteCommand(buf, response);
	printf("status :%lx, 0: %x, 1: %x, 2: %x, 3: %x\n", status, response[0], response[1], response[2], response[3]);
	
}

</details>

[windowsair]

Out of curiosity, I have a couple more questions here:

1/ Should we make sure that the DAP is in the right state, e.g. no sticky error, the DP.SELECT register is selecting the right AP, the configured write word length is what we need. This ensures that we can reset correctly even in some edge cases. ADI requires the debugger to do this when doing a line reset, but as far as I can remember, Keil's CMSIS-DAP doesn't seem to do that, or maybe I'm remembering wrong :)

2/ Should we introduce a delay after a reset because reset doesn't guarantee immediate effect ?

[kerms]

ADI requires the debugger to do this when doing a line reset, but as far as I can remember, Keil's CMSIS-DAP doesn't seem to do that

I don't know much about what does Keil's CMSIS-DAP do. In my last trace for Keil reset after flash process (writing & verify). The upper stream does some initialization just before reset request: like write a05f0000 -> e000edf0 and 0x0000000X -> e000edfc. Can we assume that Keil already done things correctly before reset ?

Should we make sure that the DAP is in the right state, e.g. no sticky error, the DP.SELECT register is selecting the right AP, the configured write word length is what we need.

No idea. For a single reset without Keil flash&reset, maybe just check and clear sticky error ? Since it is persistent and may ignore write to SYSRESETQ(not sure), thus can't resting target. Other configurations are reinitialized anyway before each actual write to SYSRESETQ.

2/ Should we introduce a delay after a reset because reset doesn't guarantee immediate effect ?

I think delay is not really needed, the upper stream seems spam read(req=0x5) after reset request on 0xe000edf0(DHCSR), this register contains the status of the processor.

[windowsair]

Great. It looks like we still have a lot of work to do. Let's start by merging this.