From c78b4fe426c35f638e5575b59ec89d0e794263af Mon Sep 17 00:00:00 2001 From: oharboe Date: Fri, 7 Mar 2008 21:49:16 +0000 Subject: [PATCH] - Improves error handling upon GDB connect - switch to synchronous halt during connect. This fixes the bug where poll() was not invoked between halt() and servicing the 'g' register packet - halt() no longer returns error code when target is already halted, just logs a warning. Only the halt() implementation can say anything meaningful about why a halt() failed, so error messages are pushed up to halt() - fixed soft_reset_halt infinite loop bug in arm7_9_common.c. The rest of the implementations are still busted. - by using USER() instead of command_print() the log gets the source + line #. Nice. - no longer invoke exit() if soft_reset_halt fails. A reset can often fix the problem. git-svn-id: svn://svn.berlios.de/openocd/trunk@475 b42882b7-edfa-0310-969c-e2dbd0fdcd60 --- src/server/gdb_server.c | 11 +++++++++-- src/target/arm11.c | 7 ++++--- src/target/arm720t.c | 5 +---- src/target/arm7_9_common.c | 24 +++++++++++++++++------- src/target/arm920t.c | 5 +---- src/target/arm926ejs.c | 5 +---- src/target/armv4_5.c | 1 + src/target/armv7m.c | 1 + src/target/cortex_m3.c | 2 +- src/target/target.c | 18 +++--------------- src/target/target.h | 3 +-- src/target/xscale.c | 2 +- 12 files changed, 41 insertions(+), 43 deletions(-) diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c index 1fac46972..f01d638fd 100644 --- a/src/server/gdb_server.c +++ b/src/server/gdb_server.c @@ -684,13 +684,19 @@ int gdb_new_connection(connection_t *connection) * GDB connection will fail if e.g. register read packets fail, * otherwise resetting/halting the target could have been left to GDB init * scripts + * + * DANGER!!!! + * We need a synchronous halt, lest connect will fail. + * Also there is no guarantee that poll() will be invoked + * between here and serving the first packet, so the halt() + * statement above is *NOT* sufficient */ - if (((retval = gdb_service->target->type->halt(gdb_service->target)) != ERROR_OK) && - (retval != ERROR_TARGET_ALREADY_HALTED)) + if ((retval = gdb_service->target->type->halt(gdb_service->target)) != ERROR_OK) { ERROR("error(%d) when trying to halt target, falling back to \"reset\"", retval); command_run_line(connection->cmd_ctx, "reset"); } + command_run_line(connection->cmd_ctx, "halt"); /* remove the initial ACK from the incoming buffer */ if ((retval = gdb_get_char(connection, &initial_ack)) != ERROR_OK) @@ -1462,6 +1468,7 @@ int gdb_query_packet(connection_t *connection, target_t *target, char *packet, i log_add_callback(gdb_log_callback, connection); target_call_timer_callbacks(); command_run_line(cmd_ctx, cmd); + target_call_timer_callbacks(); log_remove_callback(gdb_log_callback, connection); free(cmd); } diff --git a/src/target/arm11.c b/src/target/arm11.c index 9c3d1f28d..80189ffa2 100644 --- a/src/target/arm11.c +++ b/src/target/arm11.c @@ -735,8 +735,8 @@ int arm11_halt(struct target_s *target) if (target->state == TARGET_HALTED) { - WARNING("target was already halted"); - return ERROR_TARGET_ALREADY_HALTED; + WARNING("target was already halted"); + return ERROR_OK; } if (arm11->trst_active) @@ -1044,7 +1044,8 @@ int arm11_get_gdb_reg_list(struct target_s *target, struct reg_s **reg_list[], i if (target->state != TARGET_HALTED) { - return ERROR_TARGET_NOT_HALTED; + ERROR("Target not halted"); + return ERROR_TARGET_NOT_HALTED; } *reg_list_size = ARM11_GDB_REGISTER_COUNT; diff --git a/src/target/arm720t.c b/src/target/arm720t.c index 9a3f56172..962ad1bd9 100644 --- a/src/target/arm720t.c +++ b/src/target/arm720t.c @@ -367,10 +367,7 @@ int arm720t_soft_reset_halt(struct target_s *target) arm720t_common_t *arm720t = arm7tdmi->arch_info; reg_t *dbg_stat = &arm7_9->eice_cache->reg_list[EICE_DBG_STAT]; - if (target->state == TARGET_RUNNING) - { - target->type->halt(target); - } + target->type->halt(target); while (buf_get_u32(dbg_stat->value, EICE_DBG_STATUS_DBGACK, 1) == 0) { diff --git a/src/target/arm7_9_common.c b/src/target/arm7_9_common.c index e31b01511..a7d756a32 100644 --- a/src/target/arm7_9_common.c +++ b/src/target/arm7_9_common.c @@ -860,16 +860,26 @@ int arm7_9_soft_reset_halt(struct target_s *target) reg_t *dbg_stat = &arm7_9->eice_cache->reg_list[EICE_DBG_STAT]; reg_t *dbg_ctrl = &arm7_9->eice_cache->reg_list[EICE_DBG_CTRL]; int i; + int retval; - if (target->state == TARGET_RUNNING) - { - target->type->halt(target); - } + if ((retval=target->type->halt(target))!=ERROR_OK) + return retval; - while (buf_get_u32(dbg_stat->value, EICE_DBG_STATUS_DBGACK, 1) == 0) + for (i=0; i<10; i++) { + if (buf_get_u32(dbg_stat->value, EICE_DBG_STATUS_DBGACK, 1) != 0) + break; embeddedice_read_reg(dbg_stat); - jtag_execute_queue(); + if ((retval=jtag_execute_queue())!=ERROR_OK) + return retval; + /* do not eat all CPU, time out after 1 se*/ + usleep(100*1000); + + } + if (i==10) + { + ERROR("Failed to halt CPU after 1 sec"); + return ERROR_TARGET_TIMEOUT; } target->state = TARGET_HALTED; @@ -962,7 +972,7 @@ int arm7_9_halt(target_t *target) if (target->state == TARGET_HALTED) { WARNING("target was already halted"); - return ERROR_TARGET_ALREADY_HALTED; + return ERROR_OK; } if (target->state == TARGET_UNKNOWN) diff --git a/src/target/arm920t.c b/src/target/arm920t.c index 43d48152d..c1521993b 100644 --- a/src/target/arm920t.c +++ b/src/target/arm920t.c @@ -623,10 +623,7 @@ int arm920t_soft_reset_halt(struct target_s *target) arm920t_common_t *arm920t = arm9tdmi->arch_info; reg_t *dbg_stat = &arm7_9->eice_cache->reg_list[EICE_DBG_STAT]; - if (target->state == TARGET_RUNNING) - { - target->type->halt(target); - } + target->type->halt(target); while (buf_get_u32(dbg_stat->value, EICE_DBG_STATUS_DBGACK, 1) == 0) { diff --git a/src/target/arm926ejs.c b/src/target/arm926ejs.c index ff73e1a12..1b3a17b0a 100644 --- a/src/target/arm926ejs.c +++ b/src/target/arm926ejs.c @@ -578,10 +578,7 @@ int arm926ejs_soft_reset_halt(struct target_s *target) arm926ejs_common_t *arm926ejs = arm9tdmi->arch_info; reg_t *dbg_stat = &arm7_9->eice_cache->reg_list[EICE_DBG_STAT]; - if (target->state == TARGET_RUNNING) - { - target->type->halt(target); - } + target->type->halt(target); while (buf_get_u32(dbg_stat->value, EICE_DBG_STATUS_DBGACK, 1) == 0) { diff --git a/src/target/armv4_5.c b/src/target/armv4_5.c index 3bd50cd6e..82e65c789 100644 --- a/src/target/armv4_5.c +++ b/src/target/armv4_5.c @@ -476,6 +476,7 @@ int armv4_5_get_gdb_reg_list(target_t *target, reg_t **reg_list[], int *reg_list if (target->state != TARGET_HALTED) { + ERROR("Target not halted"); return ERROR_TARGET_NOT_HALTED; } diff --git a/src/target/armv7m.c b/src/target/armv7m.c index 6a8119a02..6ee5903ac 100644 --- a/src/target/armv7m.c +++ b/src/target/armv7m.c @@ -323,6 +323,7 @@ int armv7m_get_gdb_reg_list(target_t *target, reg_t **reg_list[], int *reg_list_ if (target->state != TARGET_HALTED) { + ERROR("Target not halted"); return ERROR_TARGET_NOT_HALTED; } diff --git a/src/target/cortex_m3.c b/src/target/cortex_m3.c index 964c6b8c3..51ec52e0f 100644 --- a/src/target/cortex_m3.c +++ b/src/target/cortex_m3.c @@ -432,7 +432,7 @@ int cortex_m3_halt(target_t *target) if (target->state == TARGET_HALTED) { WARNING("target was already halted"); - return ERROR_TARGET_ALREADY_HALTED; + return ERROR_OK; } if (target->state == TARGET_UNKNOWN) diff --git a/src/target/target.c b/src/target/target.c index baf57565c..dc69e9944 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -359,7 +359,7 @@ int target_process_reset(struct command_context_s *cmd_ctx) { if ((now.tv_sec > timeout.tv_sec) || ((now.tv_sec == timeout.tv_sec) && (now.tv_usec >= timeout.tv_usec))) { - command_print(cmd_ctx, "Timed out waiting for reset"); + USER("Timed out waiting for reset"); goto done; } /* this will send alive messages on e.g. GDB remote protocol. */ @@ -1614,22 +1614,10 @@ int handle_daemon_startup_command(struct command_context_s *cmd_ctx, char *cmd, int handle_soft_reset_halt_command(struct command_context_s *cmd_ctx, char *cmd, char **args, int argc) { target_t *target = get_current_target(cmd_ctx); - int retval; - command_print(cmd_ctx, "requesting target halt and executing a soft reset"); + USER("requesting target halt and executing a soft reset"); - if ((retval = target->type->soft_reset_halt(target)) != ERROR_OK) - { - switch (retval) - { - case ERROR_TARGET_TIMEOUT: - command_print(cmd_ctx, "target timed out... shutting down"); - exit(-1); - default: - command_print(cmd_ctx, "unknown error... shutting down"); - exit(-1); - } - } + target->type->soft_reset_halt(target); return ERROR_OK; } diff --git a/src/target/target.h b/src/target/target.h index dae5f19e7..8d70e77eb 100644 --- a/src/target/target.h +++ b/src/target/target.h @@ -116,7 +116,7 @@ typedef struct target_type_s /* target request support */ int (*target_request_data)(struct target_s *target, u32 size, u8 *buffer); - /* target execution control */ + /* halt will log a warning, but return ERROR_OK if the target is already halted. */ int (*halt)(struct target_s *target); int (*resume)(struct target_s *target, int current, u32 address, int handle_breakpoints, int debug_execution); int (*step)(struct target_s *target, int current, u32 address, int handle_breakpoints); @@ -270,7 +270,6 @@ int target_arch_state(struct target_s *target); #define ERROR_TARGET_INVALID (-300) #define ERROR_TARGET_INIT_FAILED (-301) #define ERROR_TARGET_TIMEOUT (-302) -#define ERROR_TARGET_ALREADY_HALTED (-303) #define ERROR_TARGET_NOT_HALTED (-304) #define ERROR_TARGET_FAILURE (-305) #define ERROR_TARGET_UNALIGNED_ACCESS (-306) diff --git a/src/target/xscale.c b/src/target/xscale.c index 4e62c8a16..7e362ff5f 100644 --- a/src/target/xscale.c +++ b/src/target/xscale.c @@ -1274,7 +1274,7 @@ int xscale_halt(target_t *target) if (target->state == TARGET_HALTED) { WARNING("target was already halted"); - return ERROR_TARGET_ALREADY_HALTED; + return ERROR_OK; } else if (target->state == TARGET_UNKNOWN) {