From 7cc0e8ee6097b1a87f17bbf6cefda70bf250f47f Mon Sep 17 00:00:00 2001 From: Greg Savin <43152568+SiFiveGregS@users.noreply.github.com> Date: Thu, 18 Apr 2019 10:36:46 -0700 Subject: [PATCH] Addressed review comments. --- configure.ac | 1 - src/target/riscv/riscv-011.c | 5 --- src/target/riscv/riscv-013.c | 23 ++++-------- src/target/riscv/riscv.c | 45 ++++++++++++++++------- src/target/riscv/riscv.h | 4 +- src/target/target.c | 22 ----------- src/target/target.h | 3 -- tcl/board/sifive-e31arty-onboard-ftdi.cfg | 3 +- 8 files changed, 43 insertions(+), 63 deletions(-) diff --git a/configure.ac b/configure.ac index eb1189d66..ab05cc656 100644 --- a/configure.ac +++ b/configure.ac @@ -111,7 +111,6 @@ m4_define([ADAPTER_OPT], [m4_translit(ADAPTER_ARG($1), [_], [-])]) m4_define([USB1_ADAPTERS], [[[ftdi], [MPSSE mode of FTDI based devices], [FTDI]], [[ftdi_oscan1], [cJTAG OSCAN1 tunneled thru MPSSE], [FTDI_OSCAN1]], - [[riscv_arty_bscan], [Access to RISCV on Arty board via BSCAN], [RISCV_ARTY_BSCAN]], [[stlink], [ST-Link JTAG Programmer], [HLADAPTER_STLINK]], [[ti_icdi], [TI ICDI JTAG Programmer], [HLADAPTER_ICDI]], [[ulink], [Keil ULINK JTAG Programmer], [ULINK]], diff --git a/src/target/riscv/riscv-011.c b/src/target/riscv/riscv-011.c index fde2a521d..aad5b8bfb 100644 --- a/src/target/riscv/riscv-011.c +++ b/src/target/riscv/riscv-011.c @@ -281,11 +281,6 @@ static uint32_t dtmcontrol_scan(struct target *target, uint32_t out) uint8_t in_value[4]; uint8_t out_value[4]; -#if BUILD_RISCV_ARTY_BSCAN == 1 - if (target->bscan_tunnel_ir_width != 0) - return dtmcontrol_scan_via_bscan(target, out); -#endif - buf_set_u32(out_value, 0, 32, out); jtag_add_ir_scan(target->tap, &select_dtmcontrol, TAP_IDLE); diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 4ce97fd7a..29d03ceb4 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -406,12 +406,10 @@ static void dump_field(int idle, const struct scan_field *field) static void select_dmi(struct target *target) { -#if BUILD_RISCV_ARTY_BSCAN == 1 - if (target->bscan_tunnel_ir_width != 0) { + if (bscan_tunnel_ir_width != 0) { select_dmi_via_bscan(target); return; } -#endif jtag_add_ir_scan(target->tap, &select_dbus, TAP_IDLE); } @@ -421,10 +419,8 @@ static uint32_t dtmcontrol_scan(struct target *target, uint32_t out) uint8_t in_value[4]; uint8_t out_value[4]; -#if BUILD_RISCV_ARTY_BSCAN == 1 - if (target->bscan_tunnel_ir_width != 0) + if (bscan_tunnel_ir_width != 0) return dtmcontrol_scan_via_bscan(target, out); -#endif buf_set_u32(out_value, 0, 32, out); @@ -497,14 +493,13 @@ static dmi_status_t dmi_scan(struct target *target, uint32_t *address_in, buf_set_u32(out, DTM_DMI_DATA_OFFSET, DTM_DMI_DATA_LENGTH, data_out); buf_set_u32(out, DTM_DMI_ADDRESS_OFFSET, info->abits, address_out); -#if BUILD_RISCV_ARTY_BSCAN == 1 /* I wanted to place this code in a different function, but the way JTAG command queueing works in the jtag handling functions, the scan fields either have to be heap allocated, global/static, or else they need to stay on the stack until the jtag_execute_queue() call. Heap or static fields in this case doesn't seem the best fit. Declaring stack based field values in a subsidiary function call wouldn't work. */ - if (target->bscan_tunnel_ir_width != 0) { + if (bscan_tunnel_ir_width != 0) { jtag_add_ir_scan(target->tap, &select_user4, TAP_IDLE); uint8_t tunneled_dr_width[4] = {num_bits}; @@ -535,10 +530,10 @@ static dmi_status_t dmi_scan(struct target *target, uint32_t *address_in, }; jtag_add_dr_scan(target->tap, DIM(tunneled_dr), tunneled_dr, TAP_IDLE); - } else -#endif - /* Assume dbus is already selected. */ - jtag_add_dr_scan(target->tap, 1, &field, TAP_IDLE); + } else { + /* Assume dbus is already selected. */ + jtag_add_dr_scan(target->tap, 1, &field, TAP_IDLE); + } int idle_count = info->dmi_busy_delay; if (exec) @@ -553,12 +548,10 @@ static dmi_status_t dmi_scan(struct target *target, uint32_t *address_in, return DMI_STATUS_FAILED; } -#if BUILD_RISCV_ARTY_BSCAN == 1 - if (target->bscan_tunnel_ir_width != 0) { + if (bscan_tunnel_ir_width != 0) { /* need to right-shift "in" by one bit, because of clock skew between BSCAN TAP and DM TAP */ buffer_shr(in, num_bytes, 1); } -#endif if (data_in) *data_in = buf_get_u32(in, DTM_DMI_DATA_OFFSET, DTM_DMI_DATA_LENGTH); diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 2842a5a47..a241c3061 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -170,8 +170,8 @@ struct scan_field select_idcode = { .out_value = ir_idcode }; -#if BUILD_RISCV_ARTY_BSCAN == 1 +int bscan_tunnel_ir_width; /* if zero, then tunneling is not present/active */ uint8_t bscan_zero[4] = {0}; uint8_t bscan_one[4] = {1}; @@ -207,7 +207,6 @@ struct scan_field _bscan_tunneled_select_dmi[] = { }; struct scan_field *bscan_tunneled_select_dmi = _bscan_tunneled_select_dmi; uint32_t bscan_tunneled_select_dmi_num_fields = DIM(_bscan_tunneled_select_dmi); -#endif struct trigger { uint64_t address; @@ -239,7 +238,6 @@ range_t *expose_csr; range_t *expose_custom; static int riscv_resume_go_all_harts(struct target *target); -#if BUILD_RISCV_ARTY_BSCAN == 1 void select_dmi_via_bscan(struct target *target) { @@ -253,7 +251,7 @@ uint32_t dtmcontrol_scan_via_bscan(struct target *target, uint32_t out) /* jtag_add_ir_scan(target->tap, &select_dtmcontrol, TAP_IDLE); */ /* On BSCAN TAP: Select IR=USER4, issue tunneled IR scan via BSCAN TAP's DR */ - uint8_t tunneled_ir_width[4] = {target->bscan_tunnel_ir_width}; + uint8_t tunneled_ir_width[4] = {bscan_tunnel_ir_width}; uint8_t tunneled_dr_width[4] = {32}; uint8_t out_value[5]; uint8_t in_value[5]; @@ -272,7 +270,7 @@ uint32_t dtmcontrol_scan_via_bscan(struct target *target, uint32_t out) .in_value = NULL, }, { - .num_bits = target->bscan_tunnel_ir_width, + .num_bits = bscan_tunnel_ir_width, .out_value = ir_dtmcontrol, .in_value = NULL, }, @@ -326,7 +324,6 @@ uint32_t dtmcontrol_scan_via_bscan(struct target *target, uint32_t out) return in; } -#endif static uint32_t dtmcontrol_scan(struct target *target, uint32_t out) @@ -335,10 +332,8 @@ static uint32_t dtmcontrol_scan(struct target *target, uint32_t out) uint8_t in_value[4]; uint8_t out_value[4]; -#if BUILD_RISCV_ARTY_BSCAN == 1 - if (target->bscan_tunnel_ir_width != 0) + if (bscan_tunnel_ir_width != 0) return dtmcontrol_scan_via_bscan(target, out); -#endif buf_set_u32(out_value, 0, 32, out); @@ -400,13 +395,11 @@ static int riscv_init_target(struct command_context *cmd_ctx, select_dbus.num_bits = target->tap->ir_length; select_idcode.num_bits = target->tap->ir_length; -#if BUILD_RISCV_ARTY_BSCAN == 1 - if (target->bscan_tunnel_ir_width != 0) { + if (bscan_tunnel_ir_width != 0) { select_user4.num_bits = target->tap->ir_length; - bscan_tunneled_ir_width[0] = target->bscan_tunnel_ir_width; - bscan_tunneled_select_dmi[2].num_bits = target->bscan_tunnel_ir_width; + bscan_tunneled_ir_width[0] = bscan_tunnel_ir_width; + bscan_tunneled_select_dmi[2].num_bits = bscan_tunnel_ir_width; } -#endif riscv_semihosting_init(target); @@ -1969,6 +1962,23 @@ COMMAND_HANDLER(riscv_set_ir) } } +COMMAND_HANDLER(riscv_use_bscan_tunnel) +{ + int irwidth = 0; + + if (CMD_ARGC > 1) { + LOG_ERROR("Command takes at most one argument"); + return ERROR_COMMAND_SYNTAX_ERROR; + } + + if (CMD_ARGC == 1) + COMMAND_PARSE_NUMBER(int, CMD_ARGV[0], irwidth); + + bscan_tunnel_ir_width = irwidth; + return ERROR_OK; +} + + static const struct command_registration riscv_exec_command_handlers[] = { { .name = "test_compliance", @@ -2075,6 +2085,13 @@ static const struct command_registration riscv_exec_command_handlers[] = { .usage = "riscv set_ir_idcode [idcode|dtmcs|dmi] value", .help = "Set IR value for specified JTAG register." }, + { + .name = "use_bscan_tunnel", + .handler = riscv_use_bscan_tunnel, + .mode = COMMAND_ANY, + .usage = "riscv use_bscan_tunnel dm_ir_width", + .help = "Enable or disable use of a BSCAN tunnel to reach DM. Supply value of 0 to disable." + }, COMMAND_REGISTRATION_DONE }; diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index ba742635d..0662efd0c 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -165,16 +165,16 @@ extern uint8_t ir_dbus[4]; extern struct scan_field select_dbus; extern uint8_t ir_idcode[4]; extern struct scan_field select_idcode; -#if BUILD_RISCV_ARTY_BSCAN == 1 + extern struct scan_field select_user4; extern struct scan_field *bscan_tunneled_select_dmi; extern uint32_t bscan_tunneled_select_dmi_num_fields; extern uint8_t bscan_zero[4]; extern uint8_t bscan_one[4]; +extern int bscan_tunnel_ir_width; uint32_t dtmcontrol_scan_via_bscan(struct target *target, uint32_t out); void select_dmi_via_bscan(struct target *target); -#endif /*** OpenOCD Interface */ int riscv_openocd_poll(struct target *target); diff --git a/src/target/target.c b/src/target/target.c index 532ab5c23..036c065d4 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -4603,9 +4603,6 @@ enum target_cfg_param { TCFG_RTOS, TCFG_DEFER_EXAMINE, TCFG_GDB_PORT, -#if BUILD_RISCV_ARTY_BSCAN == 1 - TCFG_BSCAN_TUNNEL_IR_WIDTH, -#endif }; static Jim_Nvp nvp_config_opts[] = { @@ -4622,9 +4619,6 @@ static Jim_Nvp nvp_config_opts[] = { { .name = "-rtos", .value = TCFG_RTOS }, { .name = "-defer-examine", .value = TCFG_DEFER_EXAMINE }, { .name = "-gdb-port", .value = TCFG_GDB_PORT }, -#if BUILD_RISCV_ARTY_BSCAN == 1 - { .name = "-bscan-tunnel-ir-width", .value = TCFG_BSCAN_TUNNEL_IR_WIDTH }, -#endif { .name = NULL, .value = -1 } }; @@ -4926,22 +4920,6 @@ no_params: Jim_SetResultString(goi->interp, target->gdb_port_override ? : "undefined", -1); /* loop for more */ break; - -#if BUILD_RISCV_ARTY_BSCAN == 1 - case TCFG_BSCAN_TUNNEL_IR_WIDTH: - if (goi->isconfigure) { - e = Jim_GetOpt_Wide(goi, &w); - if (e != JIM_OK) - return e; - target->bscan_tunnel_ir_width = (int32_t)w; - } else { - if (goi->argc != 0) - goto no_params; - } - Jim_SetResult(goi->interp, Jim_NewIntObj(goi->interp, target->bscan_tunnel_ir_width)); - /* loop for more */ - break; -#endif } } /* while (goi->argc) */ diff --git a/src/target/target.h b/src/target/target.h index 8826e76c2..7c9db3090 100644 --- a/src/target/target.h +++ b/src/target/target.h @@ -210,9 +210,6 @@ struct target { /* The semihosting information, extracted from the target. */ struct semihosting *semihosting; -#if BUILD_RISCV_ARTY_BSCAN == 1 - int bscan_tunnel_ir_width; /* if zero, then tunneling is not present/active */ -#endif }; struct target_list { diff --git a/tcl/board/sifive-e31arty-onboard-ftdi.cfg b/tcl/board/sifive-e31arty-onboard-ftdi.cfg index db1bfdbe1..3d40cfa04 100644 --- a/tcl/board/sifive-e31arty-onboard-ftdi.cfg +++ b/tcl/board/sifive-e31arty-onboard-ftdi.cfg @@ -9,7 +9,8 @@ jtag newtap $_CHIPNAME cpu -irlen 6; # -expected-id 0x0362d093 set _TARGETNAME $_CHIPNAME.cpu target create $_TARGETNAME.0 riscv -chain-position $_TARGETNAME -$_TARGETNAME.0 configure -work-area-phys 0x80000000 -work-area-size 10000 -work-area-backup 1 -bscan-tunnel-ir-width 5 +$_TARGETNAME.0 configure -work-area-phys 0x80000000 -work-area-size 10000 -work-area-backup 1 +riscv use_bscan_tunnel 5 # Uncomment if hardware has flash # flash bank spi0 fespi 0x40000000 0 0 0 $_TARGETNAME.0 0x20004000