Zach,
I think this patch is fundamentally wrong and is a disaster the moment
it is applied.
The queue sits between the jtag api and the drivers. The api calls
track "future" state according to the most recent api call submitted
(and put onto the back end of the queue) via the cmd_queue_cur_state
variable.
The drivers consume commands in batch mode off the back end of the
queue. They have their own notion of state, and that is what the "cable
helper API" is for. That API has that name deliberately so that you
know these functions are for cable drivers, not for the jtag api layer.
Rather than worrying about a future state, they are worrying about
current state of the taps as the driver actually manipulates TMS and the
clock.
These two concepts are separate, and cannot be merged because of the
queue. tap_set_state() is for current state of the taps. But realize
there is can be some delay here also because sometimes their are low
level commands buffered before being sent out on USB. So
tap_set_state() only tracks to the best of its ability the current state.
Dick
> Hi all,
>
> The attached patch removes the cmd_queue_cur_state global variable,
> replacing all uses with calls to tap_set_state() or tap_get_state().
>
> While this seems completely trivial, I wanted to post it for review.
> Any objections? All in favor?
>
> Cheers,
>
> Zach
>
> ---
> jtag/jtag.c | 13 ++++++-------
> jtag/jtag.h | 3 +--
> jtag/zy1000.c | 4 ++--
> svf/svf.c | 9 ++++++---
> target/arm11_dbgtap.c | 4 ++--
> xsvf/xsvf.c | 4 ++--
> 6 files changed, 19 insertions(+), 18 deletions(-)
>
> Index: src/jtag/zy1000.c
> ===================================================================
> --- src/jtag/zy1000.c (revision 1893)
> +++ src/jtag/zy1000.c (working copy)
> @@ -709,7 +709,7 @@
>
> int interface_jtag_add_clocks(int num_cycles)
> {
> - return zy1000_jtag_add_clocks(num_cycles, cmd_queue_cur_state,
> cmd_queue_end_state);
> + return zy1000_jtag_add_clocks(num_cycles, tap_get_state(),
> cmd_queue_end_state);
> }
>
> int interface_jtag_add_sleep(u32 us)
> @@ -728,7 +728,7 @@
>
> state_count = 0;
>
> - tap_state_t cur_state=cmd_queue_cur_state;
> + tap_state_t cur_state = tap_get_state();
>
> while (num_states)
> {
> Index: src/jtag/jtag.c
> ===================================================================
> --- src/jtag/jtag.c (revision 1893)
> +++ src/jtag/jtag.c (working copy)
> @@ -94,7 +94,6 @@
>
> enum reset_types jtag_reset_config = RESET_NONE;
> tap_state_t cmd_queue_end_state = TAP_RESET;
> -tap_state_t cmd_queue_cur_state = TAP_RESET;
>
> int jtag_verify_capture_ir = 1;
> int jtag_verify = 1;
> @@ -565,7 +564,7 @@
> if (state != TAP_INVALID)
> jtag_add_end_state(state);
>
> - cmd_queue_cur_state = cmd_queue_end_state;
> + tap_set_state(cmd_queue_end_state);
> }
>
> void jtag_add_ir_scan_noverify(int in_num_fields, const scan_field_t
> *in_fields, tap_state_t state)
> @@ -1066,7 +1065,7 @@
>
> void jtag_add_pathmove(int num_states, const tap_state_t *path)
> {
> - tap_state_t cur_state = cmd_queue_cur_state;
> + tap_state_t cur_state = tap_get_state();
> int i;
> int retval;
>
> @@ -1097,7 +1096,7 @@
> jtag_prelude1();
>
> retval = interface_jtag_add_pathmove(num_states, path);
> - cmd_queue_cur_state = path[num_states - 1];
> + tap_set_state(path[num_states - 1]);
> if (retval!=ERROR_OK)
> jtag_error=retval;
> }
> @@ -1168,11 +1167,11 @@
> void jtag_add_clocks( int num_cycles )
> {
> int retval;
> -
> - if( !tap_is_state_stable(cmd_queue_cur_state) )
> + tap_state_t cur_state = tap_get_state();
> + if( !tap_is_state_stable(cur_state) )
> {
> LOG_ERROR( "jtag_add_clocks() was called with TAP in
> non-stable state \"%s\"",
> - tap_state_name(cmd_queue_cur_state) );
> + tap_state_name(cur_state) );
> jtag_error = ERROR_JTAG_NOT_STABLE_STATE;
> return;
> }
> Index: src/jtag/jtag.h
> ===================================================================
> --- src/jtag/jtag.h (revision 1893)
> +++ src/jtag/jtag.h (working copy)
> @@ -256,7 +256,6 @@
>
>
> extern tap_state_t cmd_queue_end_state; /* finish DR scans in
> dr_end_state */
> -extern tap_state_t cmd_queue_cur_state; /* current TAP state */
>
> typedef void* error_handler_t; /* Later on we can delete error_handler_t,
> but keep it for now to make patches more readable */
>
> @@ -891,7 +890,7 @@
> {
> if (end_state != TAP_INVALID)
> cmd_queue_end_state = end_state;
> - cmd_queue_cur_state = cmd_queue_end_state;
> + tap_set_state(cmd_queue_end_state);
> interface_jtag_add_dr_out(tap, num_fields, num_bits, value,
> cmd_queue_end_state);
> }
>
> Index: src/target/arm11_dbgtap.c
> ===================================================================
> --- src/target/arm11_dbgtap.c (revision 1893)
> +++ src/target/arm11_dbgtap.c (working copy)
> @@ -48,7 +48,7 @@
>
> int arm11_add_ir_scan_vc(int num_fields, scan_field_t *fields, tap_state_t
> state)
> {
> - if (cmd_queue_cur_state == TAP_IRPAUSE)
> + if (tap_get_state() == TAP_IRPAUSE)
> jtag_add_pathmove(asizeof(arm11_move_pi_to_si_via_ci),
> arm11_move_pi_to_si_via_ci);
>
> jtag_add_ir_scan(num_fields, fields, state);
> @@ -62,7 +62,7 @@
>
> int arm11_add_dr_scan_vc(int num_fields, scan_field_t *fields, tap_state_t
> state)
> {
> - if (cmd_queue_cur_state == TAP_DRPAUSE)
> + if (tap_get_state() == TAP_DRPAUSE)
> jtag_add_pathmove(asizeof(arm11_move_pd_to_sd_via_cd),
> arm11_move_pd_to_sd_via_cd);
>
> jtag_add_dr_scan(num_fields, fields, state);
> Index: src/xsvf/xsvf.c
> ===================================================================
> --- src/xsvf/xsvf.c (revision 1893)
> +++ src/xsvf/xsvf.c (working copy)
> @@ -171,7 +171,7 @@
> int retval = ERROR_OK;
>
> tap_state_t moves[8];
> - tap_state_t cur_state = cmd_queue_cur_state;
> + tap_state_t cur_state = tap_get_state();
> int i;
> int tms_bits;
> int tms_count;
> @@ -608,7 +608,7 @@
>
> LOG_ERROR("XSTATE %s is not reachable
> from current state %s in one clock cycle",
> tap_state_name(mystate),
> -
> tap_state_name(cmd_queue_cur_state)
> + tap_state_name(tap_get_state())
> );
> }
> }
> Index: src/svf/svf.c
> ===================================================================
> --- src/svf/svf.c (revision 1893)
> +++ src/svf/svf.c (working copy)
> @@ -1184,7 +1184,8 @@
> cmd->cmd.statemove =
> cmd_queue_alloc(sizeof(statemove_command_t));
> cmd->cmd.statemove->end_state =
> svf_para.runtest_run_state;
>
> - cmd_queue_end_state =
> cmd_queue_cur_state = cmd->cmd.statemove->end_state;
> +
> tap_set_state(cmd->cmd.statemove->end_state);
> + cmd_queue_end_state =
> cmd->cmd.statemove->end_state;
> }
>
> // call jtag_add_clocks
> @@ -1200,7 +1201,8 @@
> cmd->cmd.statemove =
> cmd_queue_alloc(sizeof(statemove_command_t));
> cmd->cmd.statemove->end_state =
> svf_para.runtest_end_state;
>
> - cmd_queue_end_state =
> cmd_queue_cur_state = cmd->cmd.statemove->end_state;
> +
> tap_set_state(cmd->cmd.statemove->end_state);
> + cmd_queue_end_state =
> cmd->cmd.statemove->end_state;
> }
> last_state = svf_para.runtest_end_state;
> #else
> @@ -1297,7 +1299,8 @@
> cmd->cmd.statemove =
> cmd_queue_alloc(sizeof(statemove_command_t));
> cmd->cmd.statemove->end_state = state;
>
> - cmd_queue_end_state = cmd_queue_cur_state =
> cmd->cmd.statemove->end_state;
> + tap_set_state(state);
> + cmd_queue_end_state = state;
> last_state = state;
>
> LOG_DEBUG("\tmove to %s by state_move",
> svf_tap_state_name[state]);
>
> _______________________________________________
> Openocd-development mailing list
> Openocd-development@lists.berlios.de
> https://lists.berlios.de/mailman/listinfo/openocd-development
>
>
_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
