- 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

diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c
index 1fac46972a9e4f6c9c3bae36313f396012272421..f01d638fddd2cac05e5c5b9818e66c2e463b1ded 100644 (file)
--- 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 9c3d1f28d6062d1b1e3031807ccc590a21d1600a..80189ffa2161fa9d45367dc5f104d6ceab2891be 100644 (file)
--- 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 9a3f561727c61436f25627b58c4fe38f5dcdf2c8..962ad1bd9e8872b84d255a48809ee4f7e2a98360 100644 (file)
--- 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 e31b0151168287e0cd122f804208dfdef2aea395..a7d756a32f962b9061e73cf683a20443654f267b 100644 (file)
--- 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 43d48152dd2efab12ee0cec0a4a3755a194d12fa..c1521993b8c434c2dd0a09f9b9c1a88b36a59482 100644 (file)
--- 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 ff73e1a12ac6f5d0f953d9c9f9a9439ebe7f2375..1b3a17b0a976d0141d32908aebc0e7cd0e4ae42c 100644 (file)
--- 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 3bd50cd6e4a631a86c541bde0f2582916752769d..82e65c78906d4a781e06242fb60d5b54e0bed6ac 100644 (file)
--- 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 6a8119a02ddb4fdd8c9b10b1aa6fc51a113658cc..6ee5903ac8074628374163226e802032f5d88fa0 100644 (file)
--- 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 964c6b8c397687645f734f97bdab062bb0d0a5fd..51ec52e0fa651152fb0288a9f02653b7bfe3b98b 100644 (file)
--- 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 baf57565cc9988e06dec1ec84aba9689e36c0e8d..dc69e994438f4e821bf85bbf80fb66d70c2ec34a 100644 (file)
--- 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 dae5f19e717ae7084787e764f14107239c6b7d3c..8d70e77ebc9f3ac1cb2263dd742676804e74bfca 100644 (file)
--- 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 4e62c8a16ac006efa25a87277811d4407728d9f8..7e362ff5fac8e05ffc8865b5eaea3ec05fab81af 100644 (file)
--- 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)
        {