From: David B. <da...@pa...> - 2009-12-01 03:59:18
|
Now that we have this DPM component, it can (and IMO should!) handle more of the cases where the ARMv6 and ARMv7 code have needlessly unique implementations. This patch addresses coprocessor stuff: - stop using read_cp15/write_cp15 methods in * armv7a.c * cortex_a8.c - add and use DPM support for coprocessors * dpm.c ... makes other implementations become dead * arm11.c ... remove * cortex_a8.c ... remove It's basically cleanup (see diffstat) but also there's a bit of speedup since the DPM ops let code be removed: instead of N * { prepare, access, finish } it's now prepare, N * access, finish which clearly wins whenever more than one coprocessor register needs to be read or written (like cacheflush). - Dave src/target/arm11.c | 68 -------------------- src/target/arm_dpm.c | 55 ++++++++++++++++ src/target/armv7a.c | 39 ++++++++++- src/target/armv7a.h | 7 -- src/target/cortex_a8.c | 166 +++++++++++++++---------------------------------- 5 files changed, 144 insertions(+), 191 deletions(-) |
From: David B. <da...@pa...> - 2009-12-01 03:59:21
|
We don't need this code, now that the DPM code handles it. --- src/target/arm11.c | 68 --------------------------------------------------- 1 file changed, 68 deletions(-) --- a/src/target/arm11.c +++ b/src/target/arm11.c @@ -1510,71 +1510,6 @@ COMMAND_HANDLER(arm11_handle_vcr) return ERROR_OK; } -static const uint32_t arm11_coproc_instruction_limits[] = -{ - 15, /* coprocessor */ - 7, /* opcode 1 */ - 15, /* CRn */ - 15, /* CRm */ - 7, /* opcode 2 */ - 0xFFFFFFFF, /* value */ -}; - -static int arm11_mrc_inner(struct target *target, int cpnum, - uint32_t op1, uint32_t op2, uint32_t CRn, uint32_t CRm, - uint32_t *value, bool read) -{ - int retval; - struct arm11_common *arm11 = target_to_arm11(target); - - if (target->state != TARGET_HALTED) - { - LOG_ERROR("Target not halted"); - return ERROR_FAIL; - } - - uint32_t instr = 0xEE000010 | - (cpnum << 8) | - (op1 << 21) | - (CRn << 16) | - (CRm << 0) | - (op2 << 5); - - if (read) - instr |= 0x00100000; - - retval = arm11_run_instr_data_prepare(arm11); - if (retval != ERROR_OK) - return retval; - - if (read) - { - retval = arm11_run_instr_data_from_core_via_r0(arm11, instr, value); - if (retval != ERROR_OK) - return retval; - } - else - { - retval = arm11_run_instr_data_to_core_via_r0(arm11, instr, *value); - if (retval != ERROR_OK) - return retval; - } - - return arm11_run_instr_data_finish(arm11); -} - -static int arm11_mrc(struct target *target, int cpnum, - uint32_t op1, uint32_t op2, uint32_t CRn, uint32_t CRm, uint32_t *value) -{ - return arm11_mrc_inner(target, cpnum, op1, op2, CRn, CRm, value, true); -} - -static int arm11_mcr(struct target *target, int cpnum, - uint32_t op1, uint32_t op2, uint32_t CRn, uint32_t CRm, uint32_t value) -{ - return arm11_mrc_inner(target, cpnum, op1, op2, CRn, CRm, &value, false); -} - static const struct command_registration arm11_mw_command_handlers[] = { { .name = "burst", @@ -1679,7 +1614,4 @@ struct target_type arm11_target = { .target_create = arm11_target_create, .init_target = arm11_init_target, .examine = arm11_examine, - - .mrc = arm11_mrc, - .mcr = arm11_mcr, }; |
From: David B. <da...@pa...> - 2009-12-01 03:59:21
|
Instead of having separate ARM11 and Cortex-A8 implementations of this code, have one shared implementation which just builds on the existing "run instruction via R0" support. This enables followup patches to remove that now-unused code from those two drivers. (Patches to move the "mrc" and "mcr" code into "struct arm" are due too ... MIPS and other cores do not support those ARM-specific concepts.) --- src/target/arm_dpm.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) --- a/src/target/arm_dpm.c +++ b/src/target/arm_dpm.c @@ -34,6 +34,57 @@ * implementation differences between cores like ARM1136 and Cortex-A8. */ +/* + * Coprocessor support + */ + +/* Read coprocessor */ +static int dpm_mrc(struct target *target, int cpnum, + uint32_t op1, uint32_t op2, uint32_t CRn, uint32_t CRm, + uint32_t *value) +{ + struct arm *arm = target_to_arm(target); + struct arm_dpm *dpm = arm->dpm; + int retval; + + retval = dpm->prepare(dpm); + if (retval != ERROR_OK) + return retval; + + /* read coprocessor register into R0; return via DCC */ + retval = dpm->instr_read_data_r0(dpm, + ARMV4_5_MRC(cpnum, op1, 0, CRn, CRm, op2), + value); + + /* (void) */ dpm->finish(dpm); + return retval; +} + +static int dpm_mcr(struct target *target, int cpnum, + uint32_t op1, uint32_t op2, uint32_t CRn, uint32_t CRm, + uint32_t value) +{ + struct arm *arm = target_to_arm(target); + struct arm_dpm *dpm = arm->dpm; + int retval; + + retval = dpm->prepare(dpm); + if (retval != ERROR_OK) + return retval; + + /* read DCC into r0; then write coprocessor register from R0 */ + retval = dpm->instr_write_data_r0(dpm, + ARMV4_5_MCR(cpnum, op1, 0, CRn, CRm, op2), + value); + + /* (void) */ dpm->finish(dpm); + return retval; +} + +/* + * Register access utilities + */ + /* Toggles between recorded core mode (USR, SVC, etc) and a temporary one. * Routines *must* restore the original mode before returning!! */ @@ -510,6 +561,10 @@ int arm_dpm_setup(struct arm_dpm *dpm) return ERROR_FAIL; *register_get_last_cache_p(&target->reg_cache) = cache; + + target->type->mrc = dpm_mrc; + target->type->mcr = dpm_mcr; + return ERROR_OK; } |
From: David B. <da...@pa...> - 2009-12-01 03:59:25
|
The ARMv7-A code uses read_cp15() to access fault registers. Instead, use DPM operations directly, passing in the relevant MRC instructions. This eliminates some per-operation overhead (though it'll be hard to observe, this is uncommon) and helps eliminate read_cp15(). --- src/target/armv7a.c | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) --- a/src/target/armv7a.c +++ b/src/target/armv7a.c @@ -38,17 +38,48 @@ static void armv7a_show_fault_registers( { uint32_t dfsr, ifsr, dfar, ifar; struct armv7a_common *armv7a = target_to_armv7a(target); + struct arm_dpm *dpm = armv7a->armv4_5_common.dpm; + int retval; - armv7a->read_cp15(target, 0, 0, 5, 0, &dfsr); - armv7a->read_cp15(target, 0, 1, 5, 0, &ifsr); - armv7a->read_cp15(target, 0, 0, 6, 0, &dfar); - armv7a->read_cp15(target, 0, 2, 6, 0, &ifar); + retval = dpm->prepare(dpm); + if (retval != ERROR_OK) + return; + + /* ARMV4_5_MRC(cpnum, op1, r0, CRn, CRm, op2) */ + + /* c5/c0 - {data, instruction} fault status registers */ + retval = dpm->instr_read_data_r0(dpm, + ARMV4_5_MRC(15, 0, 0, 5, 0, 0), + &dfsr); + if (retval != ERROR_OK) + goto done; + + retval = dpm->instr_read_data_r0(dpm, + ARMV4_5_MRC(15, 0, 0, 5, 0, 1), + &ifsr); + if (retval != ERROR_OK) + goto done; + + /* c6/c0 - {data, instruction} fault address registers */ + retval = dpm->instr_read_data_r0(dpm, + ARMV4_5_MRC(15, 0, 0, 6, 0, 0), + &dfar); + if (retval != ERROR_OK) + goto done; + + retval = dpm->instr_read_data_r0(dpm, + ARMV4_5_MRC(15, 0, 0, 6, 0, 2), + &ifar); + if (retval != ERROR_OK) + goto done; LOG_USER("Data fault registers DFSR: %8.8" PRIx32 ", DFAR: %8.8" PRIx32, dfsr, dfar); LOG_USER("Instruction fault registers IFSR: %8.8" PRIx32 ", IFAR: %8.8" PRIx32, ifsr, ifar); +done: + /* (void) */ dpm->finish(dpm); } int armv7a_arch_state(struct target *target) |
From: David B. <da...@pa...> - 2009-12-01 03:59:33
|
We don't need this code, now that the DPM code handles it. Neither do we need the ARMv7-A CP15 operations; remove their remnants too. And disable a mostly-needless diagnostic. --- src/target/armv7a.h | 7 --- src/target/cortex_a8.c | 97 ----------------------------------------------- 2 files changed, 1 insertion(+), 103 deletions(-) --- a/src/target/armv7a.h +++ b/src/target/armv7a.h @@ -62,13 +62,6 @@ struct armv7a_common /* Cache and Memory Management Unit */ struct armv4_5_mmu_common armv4_5_mmu; - int (*read_cp15)(struct target *target, - uint32_t op1, uint32_t op2, - uint32_t CRn, uint32_t CRm, uint32_t *value); - int (*write_cp15)(struct target *target, - uint32_t op1, uint32_t op2, - uint32_t CRn, uint32_t CRm, uint32_t value); - int (*examine_debug_reason)(struct target *target); void (*post_debug_entry)(struct target *target); --- a/src/target/cortex_a8.c +++ b/src/target/cortex_a8.c @@ -159,97 +159,6 @@ static int cortex_a8_read_regs_through_m return retval; } -static int cortex_a8_read_cp(struct target *target, uint32_t *value, uint8_t CP, - uint8_t op1, uint8_t CRn, uint8_t CRm, uint8_t op2) -{ - int retval; - struct armv7a_common *armv7a = target_to_armv7a(target); - struct swjdp_common *swjdp = &armv7a->swjdp_info; - uint32_t dscr = 0; - - /* MRC(...) to read coprocessor register into r0 */ - cortex_a8_exec_opcode(target, ARMV4_5_MRC(CP, op1, 0, CRn, CRm, op2), - &dscr); - - /* Move R0 to DTRTX */ - cortex_a8_exec_opcode(target, ARMV4_5_MCR(14, 0, 0, 0, 5, 0), - &dscr); - - /* Read DCCTX */ - retval = mem_ap_read_atomic_u32(swjdp, - armv7a->debug_base + CPUDBG_DTRTX, value); - - return retval; -} - -static int cortex_a8_write_cp(struct target *target, uint32_t value, - uint8_t CP, uint8_t op1, uint8_t CRn, uint8_t CRm, uint8_t op2) -{ - int retval; - uint32_t dscr; - struct armv7a_common *armv7a = target_to_armv7a(target); - struct swjdp_common *swjdp = &armv7a->swjdp_info; - - LOG_DEBUG("CP%i, CRn %i, value 0x%08" PRIx32, CP, CRn, value); - - /* Check that DCCRX is not full */ - retval = mem_ap_read_atomic_u32(swjdp, - armv7a->debug_base + CPUDBG_DSCR, &dscr); - if (dscr & (1 << DSCR_DTR_RX_FULL)) - { - LOG_ERROR("DSCR_DTR_RX_FULL, dscr 0x%08" PRIx32, dscr); - /* Clear DCCRX with MCR(p14, 0, Rd, c0, c5, 0), opcode 0xEE000E15 */ - cortex_a8_exec_opcode(target, ARMV4_5_MRC(14, 0, 0, 0, 5, 0), - &dscr); - } - - /* Write DTRRX ... sets DSCR.DTRRXfull but exec_opcode() won't care */ - retval = mem_ap_write_u32(swjdp, - armv7a->debug_base + CPUDBG_DTRRX, value); - - /* Move DTRRX to r0 */ - cortex_a8_exec_opcode(target, ARMV4_5_MRC(14, 0, 0, 0, 5, 0), &dscr); - - /* MCR(...) to write r0 to coprocessor */ - return cortex_a8_exec_opcode(target, - ARMV4_5_MCR(CP, op1, 0, CRn, CRm, op2), - &dscr); -} - -static int cortex_a8_read_cp15(struct target *target, uint32_t op1, uint32_t op2, - uint32_t CRn, uint32_t CRm, uint32_t *value) -{ - return cortex_a8_read_cp(target, value, 15, op1, CRn, CRm, op2); -} - -static int cortex_a8_write_cp15(struct target *target, uint32_t op1, uint32_t op2, - uint32_t CRn, uint32_t CRm, uint32_t value) -{ - return cortex_a8_write_cp(target, value, 15, op1, CRn, CRm, op2); -} - -static int cortex_a8_mrc(struct target *target, int cpnum, uint32_t op1, uint32_t op2, uint32_t CRn, uint32_t CRm, uint32_t *value) -{ - if (cpnum!=15) - { - LOG_ERROR("Only cp15 is supported"); - return ERROR_FAIL; - } - return cortex_a8_read_cp15(target, op1, op2, CRn, CRm, value); -} - -static int cortex_a8_mcr(struct target *target, int cpnum, uint32_t op1, uint32_t op2, uint32_t CRn, uint32_t CRm, uint32_t value) -{ - if (cpnum!=15) - { - LOG_ERROR("Only cp15 is supported"); - return ERROR_FAIL; - } - return cortex_a8_write_cp15(target, op1, op2, CRn, CRm, value); -} - - - static int cortex_a8_dap_read_coreregister_u32(struct target *target, uint32_t *value, int regnum) { @@ -421,7 +330,7 @@ static int cortex_a8_read_dcc(struct cor retval = mem_ap_read_atomic_u32(swjdp, a8->armv7a_common.debug_base + CPUDBG_DTRTX, data); - LOG_DEBUG("read DCC 0x%08" PRIx32, *data); + //LOG_DEBUG("read DCC 0x%08" PRIx32, *data); if (dscr_p) *dscr_p = dscr; @@ -1642,8 +1551,6 @@ static int cortex_a8_init_arch_info(stru // armv7a->armv4_5_mmu.enable_mmu_caches = armv7a_enable_mmu_caches; armv7a->armv4_5_mmu.has_tiny_pages = 1; armv7a->armv4_5_mmu.mmu_enabled = 0; - armv7a->read_cp15 = cortex_a8_read_cp15; - armv7a->write_cp15 = cortex_a8_write_cp15; // arm7_9->handle_target_request = cortex_a8_handle_target_request; @@ -1752,6 +1659,4 @@ struct target_type cortexa8_target = { .target_create = cortex_a8_target_create, .init_target = cortex_a8_init_target, .examine = cortex_a8_examine, - .mrc = cortex_a8_mrc, - .mcr = cortex_a8_mcr, }; |
From: David B. <da...@pa...> - 2009-12-01 03:59:41
|
There were two chunks of Cortex-A8 code which called the ARMv7-A CP15 operations; get rid of them, helping prepare to remove those methods completely: - post_debug_entry() can use the mrc() method to read its two registers. - write_memory() can use dpm->instr_write_data_r0() to flush the ICache and DCache ... doing it this way is actually faster since it reduces per-write overhead. Note that the mrc() method parameters are re-ordered with respect to the ARM instruction documentation, so that part can be confusing. Cleaned up the layout and comments in those areas a bit. --- src/target/cortex_a8.c | 69 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 53 insertions(+), 16 deletions(-) --- a/src/target/cortex_a8.c +++ b/src/target/cortex_a8.c @@ -933,19 +933,26 @@ static void cortex_a8_post_debug_entry(s { struct cortex_a8_common *cortex_a8 = target_to_cortex_a8(target); struct armv7a_common *armv7a = &cortex_a8->armv7a_common; + int retval; -// cortex_a8_read_cp(target, &cp15_control_register, 15, 0, 1, 0, 0); - /* examine cp15 control reg */ - armv7a->read_cp15(target, 0, 0, 1, 0, &cortex_a8->cp15_control_reg); - jtag_execute_queue(); + /* MRC p15,0,<Rt>,c1,c0,0 ; Read CP15 System Control Register */ + retval = target->type->mrc(target, 15, + 0, 0, /* op1, op2 */ + 1, 0, /* CRn, CRm */ + &cortex_a8->cp15_control_reg); LOG_DEBUG("cp15_control_reg: %8.8" PRIx32, cortex_a8->cp15_control_reg); if (armv7a->armv4_5_mmu.armv4_5_cache.ctype == -1) { uint32_t cache_type_reg; - /* identify caches */ - armv7a->read_cp15(target, 0, 1, 0, 0, &cache_type_reg); - jtag_execute_queue(); + + /* MRC p15,0,<Rt>,c0,c0,1 ; Read CP15 Cache Type Register */ + retval = target->type->mrc(target, 15, + 0, 1, /* op1, op2 */ + 0, 0, /* CRn, CRm */ + &cache_type_reg); + LOG_DEBUG("cp15 cache type: %8.8x", (unsigned) cache_type_reg); + /* FIXME the armv4_4 cache info DOES NOT APPLY to Cortex-A8 */ armv4_5_identify_cache(cache_type_reg, &armv7a->armv4_5_mmu.armv4_5_cache); @@ -1350,25 +1357,55 @@ static int cortex_a8_write_memory(struct } } + /* REVISIT this op is generic ARMv7-A/R stuff */ if (retval == ERROR_OK && target->state == TARGET_HALTED) { - /* The Cache handling will NOT work with MMU active, the wrong addresses will be invalidated */ + struct arm_dpm *dpm = armv7a->armv4_5_common.dpm; + + retval = dpm->prepare(dpm); + if (retval != ERROR_OK) + return retval; + + /* The Cache handling will NOT work with MMU active, the + * wrong addresses will be invalidated! + * + * For both ICache and DCache, walk all cache lines in the + * address range. Cortex-A8 has fixed 64 byte line length. + */ + /* invalidate I-Cache */ if (armv7a->armv4_5_mmu.armv4_5_cache.i_cache_enabled) { - /* Invalidate ICache single entry with MVA, repeat this for all cache - lines in the address range, Cortex-A8 has fixed 64 byte line length */ - /* Invalidate Cache single entry with MVA to PoU */ - for (uint32_t cacheline=address; cacheline<address+size*count; cacheline+=64) - armv7a->write_cp15(target, 0, 1, 7, 5, cacheline); /* I-Cache to PoU */ + /* ICIMVAU - Invalidate Cache single entry + * with MVA to PoU + * MCR p15, 0, r0, c7, c5, 1 + */ + for (uint32_t cacheline = address; + cacheline < address + size * count; + cacheline += 64) { + retval = dpm->instr_write_data_r0(dpm, + ARMV4_5_MCR(15, 0, 0, 7, 5, 1), + cacheline); + } } + /* invalidate D-Cache */ if (armv7a->armv4_5_mmu.armv4_5_cache.d_u_cache_enabled) { - /* Invalidate Cache single entry with MVA to PoC */ - for (uint32_t cacheline=address; cacheline<address+size*count; cacheline+=64) - armv7a->write_cp15(target, 0, 1, 7, 6, cacheline); /* U/D cache to PoC */ + /* DCIMVAC - Invalidate data Cache line + * with MVA to PoC + * MCR p15, 0, r0, c7, c6, 1 + */ + for (uint32_t cacheline = address; + cacheline < address + size * count; + cacheline += 64) { + retval = dpm->instr_write_data_r0(dpm, + ARMV4_5_MCR(15, 0, 0, 7, 6, 1), + cacheline); + } } + + /* (void) */ dpm->finish(dpm); } return retval; |
From: Øyvind H. <oyv...@zy...> - 2009-12-01 07:55:34
|
On Tue, Dec 1, 2009 at 3:59 AM, David Brownell <da...@pa...> wrote: > We don't need this code, now that the DPM code handles it. > Neither do we need the ARMv7-A CP15 operations; remove their > remnants too. And disable a mostly-needless diagnostic. What about reset init scripts. Will they *never* need the mrc/mcr commands? -- Øyvind Harboe US toll free 1-866-980-3434 / International +47 51 63 25 00 http://www.zylin.com/zy1000.html ARM7 ARM9 ARM11 XScale Cortex JTAG debugger and flash programmer |
From: Øyvind H. <oyv...@zy...> - 2009-12-01 07:57:31
|
On Tue, Dec 1, 2009 at 7:55 AM, Øyvind Harboe <oyv...@zy...> wrote: > On Tue, Dec 1, 2009 at 3:59 AM, David Brownell <da...@pa...> wrote: >> We don't need this code, now that the DPM code handles it. >> Neither do we need the ARMv7-A CP15 operations; remove their >> remnants too. And disable a mostly-needless diagnostic. > > What about reset init scripts. Will they *never* need the mrc/mcr commands? Right... better read the entire series... I see that you added something via a much more general mechanism to cover this case. You're obviously way ahead of me on this one. -- Øyvind Harboe US toll free 1-866-980-3434 / International +47 51 63 25 00 http://www.zylin.com/zy1000.html ARM7 ARM9 ARM11 XScale Cortex JTAG debugger and flash programmer |
From: David B. <da...@pa...> - 2009-12-01 08:42:31
|
On Monday 30 November 2009, David Brownell wrote: > (Patches to move the "mrc" and "mcr" code into > "struct arm" are due too ... MIPS and other cores do not support > those ARM-specific concepts.) Needed for testing, even ... here it is. Goes before slightly tweaked versions of the last few patches. These behave in light testing on Cortex-A8; didn't make time to do so with ARM1136. I'm going to merge the series anyway. ========= CUT HERE From: David Brownell <dbr...@us...> Subject: target: "mcr" and "mrc" are ARM-specific Switch "mrc" and "mcr" commands to be toplevel ARM operations, as they should initially have been. Correct the usage message for both commands: it matches ARM documentation (as one wants!) instead of reordering them to match the funky mrc() and mcr() method usage (sigh). For Cortex-A8: restore a line that got accidentally dropped, so the secure monitor mode shadow registers will show again. --- src/target/arm11.c | 13 ++ src/target/arm720t.c | 15 ++- src/target/arm920t.c | 15 ++- src/target/arm926ejs.c | 5 - src/target/armv4_5.c | 168 +++++++++++++++++++++++++++++++++++++ src/target/armv4_5.h | 15 +++ src/target/cortex_a8.c | 11 +- src/target/target.c | 199 --------------------------------------------- src/target/target_type.h | 5 - 9 files changed, 228 insertions(+), 218 deletions(-) --- a/src/target/arm11.c +++ b/src/target/arm11.c @@ -1219,6 +1219,13 @@ static int arm11_remove_watchpoint(struc return ERROR_FAIL; } +static int arm11_mrc(struct target *target, int cpnum, + uint32_t op1, uint32_t op2, + uint32_t CRn, uint32_t CRm, uint32_t *value); +static int arm11_mcr(struct target *target, int cpnum, + uint32_t op1, uint32_t op2, uint32_t CRn, + uint32_t CRm, uint32_t value); + static int arm11_target_create(struct target *target, Jim_Interp *interp) { struct arm11_common *arm11; @@ -1238,6 +1245,9 @@ static int arm11_target_create(struct ta armv4_5_init_arch_info(target, &arm11->arm); + arm11->arm.mrc = arm11_mrc; + arm11->arm.mcr = arm11_mcr; + arm11->target = target; arm11->jtag_info.tap = target->tap; @@ -1679,7 +1689,4 @@ struct target_type arm11_target = { .target_create = arm11_target_create, .init_target = arm11_init_target, .examine = arm11_examine, - - .mrc = arm11_mrc, - .mcr = arm11_mcr, }; --- a/src/target/arm720t.c +++ b/src/target/arm720t.c @@ -378,11 +378,24 @@ static int arm720t_init_target(struct co return arm7tdmi_init_target(cmd_ctx, target); } +/* FIXME remove forward decls */ +static int arm720t_mrc(struct target *target, int cpnum, + uint32_t op1, uint32_t op2, + uint32_t CRn, uint32_t CRm, + uint32_t *value); +static int arm720t_mcr(struct target *target, int cpnum, + uint32_t op1, uint32_t op2, + uint32_t CRn, uint32_t CRm, + uint32_t value); + static int arm720t_init_arch_info(struct target *target, struct arm720t_common *arm720t, struct jtag_tap *tap) { struct arm7_9_common *arm7_9 = &arm720t->arm7_9_common; + arm7_9->armv4_5_common.mrc = arm720t_mrc; + arm7_9->armv4_5_common.mcr = arm720t_mcr; + arm7tdmi_init_arch_info(target, arm7_9, tap); arm720t->common_magic = ARM720T_COMMON_MAGIC; @@ -556,6 +569,4 @@ struct target_type arm720t_target = .target_create = arm720t_target_create, .init_target = arm720t_init_target, .examine = arm7_9_examine, - .mrc = arm720t_mrc, - .mcr = arm720t_mcr, }; --- a/src/target/arm920t.c +++ b/src/target/arm920t.c @@ -624,10 +624,23 @@ int arm920t_soft_reset_halt(struct targe return ERROR_OK; } +/* FIXME remove forward decls */ +static int arm920t_mrc(struct target *target, int cpnum, + uint32_t op1, uint32_t op2, + uint32_t CRn, uint32_t CRm, + uint32_t *value); +static int arm920t_mcr(struct target *target, int cpnum, + uint32_t op1, uint32_t op2, + uint32_t CRn, uint32_t CRm, + uint32_t value); + int arm920t_init_arch_info(struct target *target, struct arm920t_common *arm920t, struct jtag_tap *tap) { struct arm7_9_common *arm7_9 = &arm920t->arm7_9_common; + arm7_9->armv4_5_common.mrc = arm920t_mrc; + arm7_9->armv4_5_common.mcr = arm920t_mcr; + /* initialize arm7/arm9 specific info (including armv4_5) */ arm9tdmi_init_arch_info(target, arm7_9, tap); @@ -1452,6 +1465,4 @@ struct target_type arm920t_target = .target_create = arm920t_target_create, .init_target = arm9tdmi_init_target, .examine = arm7_9_examine, - .mrc = arm920t_mrc, - .mcr = arm920t_mcr, }; --- a/src/target/arm926ejs.c +++ b/src/target/arm926ejs.c @@ -673,6 +673,9 @@ int arm926ejs_init_arch_info(struct targ { struct arm7_9_common *arm7_9 = &arm926ejs->arm7_9_common; + arm7_9->armv4_5_common.mrc = arm926ejs_mrc; + arm7_9->armv4_5_common.mcr = arm926ejs_mcr; + /* initialize arm7/arm9 specific info (including armv4_5) */ arm9tdmi_init_arch_info(target, arm7_9, tap); @@ -822,6 +825,4 @@ struct target_type arm926ejs_target = .read_phys_memory = arm926ejs_read_phys_memory, .write_phys_memory = arm926ejs_write_phys_memory, - .mrc = arm926ejs_mrc, - .mcr = arm926ejs_mcr, }; --- a/src/target/armv4_5.c +++ b/src/target/armv4_5.c @@ -790,6 +790,137 @@ usage: return retval; } +static int jim_mcrmrc(Jim_Interp *interp, int argc, Jim_Obj *const *argv) +{ + struct command_context *context; + struct target *target; + struct arm *arm; + int retval; + + context = Jim_GetAssocData(interp, "context"); + if (context == NULL) { + LOG_ERROR("%s: no command context", __func__); + return JIM_ERR; + } + target = get_current_target(context); + if (target == NULL) { + LOG_ERROR("%s: no current target", __func__); + return JIM_ERR; + } + if (!target_was_examined(target)) { + LOG_ERROR("%s: not yet examined", target_name(target)); + return JIM_ERR; + } + arm = target_to_arm(target); + if (!is_arm(arm)) { + LOG_ERROR("%s: not an ARM", target_name(target)); + return JIM_ERR; + } + + if ((argc < 6) || (argc > 7)) { + /* FIXME use the command name to verify # params... */ + LOG_ERROR("%s: wrong number of arguments", __func__); + return JIM_ERR; + } + + int cpnum; + uint32_t op1; + uint32_t op2; + uint32_t CRn; + uint32_t CRm; + uint32_t value; + long l; + + /* NOTE: parameter sequence matches ARM instruction set usage: + * MCR pNUM, op1, rX, CRn, CRm, op2 ; write CP from rX + * MRC pNUM, op1, rX, CRn, CRm, op2 ; read CP into rX + * The "rX" is necessarily omitted; it uses Tcl mechanisms. + */ + retval = Jim_GetLong(interp, argv[1], &l); + if (retval != JIM_OK) + return retval; + if (l & ~0xf) { + LOG_ERROR("%s: %s %d out of range", __func__, + "coprocessor", (int) l); + return JIM_ERR; + } + cpnum = l; + + retval = Jim_GetLong(interp, argv[2], &l); + if (retval != JIM_OK) + return retval; + if (l & ~0x7) { + LOG_ERROR("%s: %s %d out of range", __func__, + "op1", (int) l); + return JIM_ERR; + } + op1 = l; + + retval = Jim_GetLong(interp, argv[3], &l); + if (retval != JIM_OK) + return retval; + if (l & ~0xf) { + LOG_ERROR("%s: %s %d out of range", __func__, + "CRn", (int) l); + return JIM_ERR; + } + CRn = l; + + retval = Jim_GetLong(interp, argv[4], &l); + if (retval != JIM_OK) + return retval; + if (l & ~0xf) { + LOG_ERROR("%s: %s %d out of range", __func__, + "CRm", (int) l); + return JIM_ERR; + } + CRm = l; + + retval = Jim_GetLong(interp, argv[5], &l); + if (retval != JIM_OK) + return retval; + if (l & ~0x7) { + LOG_ERROR("%s: %s %d out of range", __func__, + "op2", (int) l); + return JIM_ERR; + } + op2 = l; + + value = 0; + + /* FIXME don't assume "mrc" vs "mcr" from the number of params; + * that could easily be a typo! Check both... + * + * FIXME change the call syntax here ... simplest to just pass + * the MRC() or MCR() instruction to be executed. That will also + * let us support the "mrc2" and "mcr2" opcodes (toggling one bit) + * if that's ever needed. + */ + if (argc == 7) { + retval = Jim_GetLong(interp, argv[6], &l); + if (retval != JIM_OK) { + return retval; + } + value = l; + + /* NOTE: parameters reordered! */ + // ARMV4_5_MCR(cpnum, op1, 0, CRn, CRm, op2) + retval = arm->mcr(target, cpnum, op1, op2, CRn, CRm, value); + if (retval != ERROR_OK) + return JIM_ERR; + } else { + /* NOTE: parameters reordered! */ + // ARMV4_5_MRC(cpnum, op1, 0, CRn, CRm, op2) + retval = arm->mrc(target, cpnum, op1, op2, CRn, CRm, &value); + if (retval != ERROR_OK) + return JIM_ERR; + + Jim_SetResult(interp, Jim_NewIntObj(interp, value)); + } + + return JIM_OK; +} + static const struct command_registration arm_exec_command_handlers[] = { { .name = "reg", @@ -811,6 +942,20 @@ static const struct command_registration .usage = "<address> [<count> ['thumb']]", .help = "disassemble instructions ", }, + { + .name = "mcr", + .mode = COMMAND_EXEC, + .jim_handler = &jim_mcrmrc, + .help = "write coprocessor register", + .usage = "cpnum op1 CRn op2 CRm value", + }, + { + .name = "mrc", + .jim_handler = &jim_mcrmrc, + .help = "read coprocessor register", + .usage = "cpnum op1 CRn op2 CRm", + }, + COMMAND_REGISTRATION_DONE }; const struct command_registration arm_command_handlers[] = { @@ -1252,6 +1397,24 @@ static int arm_full_context(struct targe return retval; } +static int arm_default_mrc(struct target *target, int cpnum, + uint32_t op1, uint32_t op2, + uint32_t CRn, uint32_t CRm, + uint32_t *value) +{ + LOG_ERROR("%s doesn't implement MRC", target_type_name(target)); + return ERROR_FAIL; +} + +static int arm_default_mcr(struct target *target, int cpnum, + uint32_t op1, uint32_t op2, + uint32_t CRn, uint32_t CRm, + uint32_t value) +{ + LOG_ERROR("%s doesn't implement MCR", target_type_name(target)); + return ERROR_FAIL; +} + int armv4_5_init_arch_info(struct target *target, struct arm *armv4_5) { target->arch_info = armv4_5; @@ -1267,5 +1430,10 @@ int armv4_5_init_arch_info(struct target if (!armv4_5->full_context && armv4_5->read_core_reg) armv4_5->full_context = arm_full_context; + if (!armv4_5->mrc) + armv4_5->mrc = arm_default_mrc; + if (!armv4_5->mcr) + armv4_5->mcr = arm_default_mcr; + return ERROR_OK; } --- a/src/target/armv4_5.h +++ b/src/target/armv4_5.h @@ -112,11 +112,26 @@ struct arm /** Handle for the Embedded Trace Module, if one is present. */ struct etm_context *etm; + /* FIXME all these methods should take "struct arm *" not target */ + int (*full_context)(struct target *target); int (*read_core_reg)(struct target *target, struct reg *reg, int num, enum armv4_5_mode mode); int (*write_core_reg)(struct target *target, struct reg *reg, int num, enum armv4_5_mode mode, uint32_t value); + + /** Read coprocessor register. */ + int (*mrc)(struct target *target, int cpnum, + uint32_t op1, uint32_t op2, + uint32_t CRn, uint32_t CRm, + uint32_t *value); + + /* Write coprocessor register. */ + int (*mcr)(struct target *target, int cpnum, + uint32_t op1, uint32_t op2, + uint32_t CRn, uint32_t CRm, + uint32_t value); + void *arch_info; }; --- a/src/target/cortex_a8.c +++ b/src/target/cortex_a8.c @@ -936,7 +936,7 @@ static void cortex_a8_post_debug_entry(s int retval; /* MRC p15,0,<Rt>,c1,c0,0 ; Read CP15 System Control Register */ - retval = target->type->mrc(target, 15, + retval = armv7a->armv4_5_common.mrc(target, 15, 0, 0, /* op1, op2 */ 1, 0, /* CRn, CRm */ &cortex_a8->cp15_control_reg); @@ -947,7 +947,7 @@ static void cortex_a8_post_debug_entry(s uint32_t cache_type_reg; /* MRC p15,0,<Rt>,c0,c0,1 ; Read CP15 Cache Type Register */ - retval = target->type->mrc(target, 15, + retval = armv7a->armv4_5_common.mrc(target, 15, 0, 1, /* op1, op2 */ 0, 0, /* CRn, CRm */ &cache_type_reg); @@ -1535,6 +1535,7 @@ static int cortex_a8_examine_first(struc LOG_DEBUG("ttypr = 0x%08" PRIx32, ttypr); LOG_DEBUG("didr = 0x%08" PRIx32, didr); + armv7a->armv4_5_common.core_type = ARM_MODE_MON; cortex_a8_dpm_setup(cortex_a8, didr); /* Setup Breakpoint Register Pairs */ @@ -1611,6 +1612,9 @@ static int cortex_a8_init_arch_info(stru cortex_a8->common_magic = CORTEX_A8_COMMON_MAGIC; armv4_5->arch_info = armv7a; + armv4_5->mrc = cortex_a8_mrc, + armv4_5->mcr = cortex_a8_mcr, + /* prepare JTAG information for the new target */ cortex_a8->jtag_info.tap = tap; cortex_a8->jtag_info.scann_size = 4; @@ -1626,7 +1630,6 @@ static int cortex_a8_init_arch_info(stru cortex_a8->fast_reg_read = 0; - /* register arch-specific functions */ armv7a->examine_debug_reason = NULL; @@ -1752,6 +1755,4 @@ struct target_type cortexa8_target = { .target_create = cortex_a8_target_create, .init_target = cortex_a8_init_target, .examine = cortex_a8_examine, - .mrc = cortex_a8_mrc, - .mcr = cortex_a8_mcr, }; --- a/src/target/target.c +++ b/src/target/target.c @@ -44,8 +44,6 @@ #include "jtag.h" -static int jim_mcrmrc(Jim_Interp *interp, int argc, Jim_Obj *const *argv); - static int target_array2mem(Jim_Interp *interp, struct target *target, int argc, Jim_Obj *const *argv); static int target_mem2array(Jim_Interp *interp, struct target *target, int argc, Jim_Obj *const *argv); @@ -665,84 +663,6 @@ static void target_reset_examined(struct target->examined = false; } - - -static int default_mrc(struct target *target, int cpnum, uint32_t op1, uint32_t op2, uint32_t CRn, uint32_t CRm, uint32_t *value) -{ - LOG_ERROR("Not implemented: %s", __func__); - return ERROR_FAIL; -} - -static int default_mcr(struct target *target, int cpnum, uint32_t op1, uint32_t op2, uint32_t CRn, uint32_t CRm, uint32_t value) -{ - LOG_ERROR("Not implemented: %s", __func__); - return ERROR_FAIL; -} - -static int arm_cp_check(struct target *target, int cpnum, uint32_t op1, uint32_t op2, uint32_t CRn, uint32_t CRm) -{ - /* basic check */ - if (!target_was_examined(target)) - { - LOG_ERROR("Target not examined yet"); - return ERROR_FAIL; - } - - if ((cpnum <0) || (cpnum > 15)) - { - LOG_ERROR("Illegal co-processor %d", cpnum); - return ERROR_FAIL; - } - - if (op1 > 7) - { - LOG_ERROR("Illegal op1"); - return ERROR_FAIL; - } - - if (op2 > 7) - { - LOG_ERROR("Illegal op2"); - return ERROR_FAIL; - } - - if (CRn > 15) - { - LOG_ERROR("Illegal CRn"); - return ERROR_FAIL; - } - - if (CRm > 15) - { - LOG_ERROR("Illegal CRm"); - return ERROR_FAIL; - } - - return ERROR_OK; -} - -int target_mrc(struct target *target, int cpnum, uint32_t op1, uint32_t op2, uint32_t CRn, uint32_t CRm, uint32_t *value) -{ - int retval; - - retval = arm_cp_check(target, cpnum, op1, op2, CRn, CRm); - if (retval != ERROR_OK) - return retval; - - return target->type->mrc(target, cpnum, op1, op2, CRn, CRm, value); -} - -int target_mcr(struct target *target, int cpnum, uint32_t op1, uint32_t op2, uint32_t CRn, uint32_t CRm, uint32_t value) -{ - int retval; - - retval = arm_cp_check(target, cpnum, op1, op2, CRn, CRm); - if (retval != ERROR_OK) - return retval; - - return target->type->mcr(target, cpnum, op1, op2, CRn, CRm, value); -} - static int err_read_phys_memory(struct target *target, uint32_t address, uint32_t size, uint32_t count, uint8_t *buffer) @@ -781,39 +701,6 @@ int target_init(struct command_context * return retval; } - /** - * @todo MCR/MRC are ARM-specific; don't require them in - * all targets, or for ARMs without coprocessors. - */ - if (target->type->mcr == NULL) - { - target->type->mcr = default_mcr; - } else - { - const struct command_registration mcr_cmd = { - .name = "mcr", - .mode = COMMAND_EXEC, - .jim_handler = &jim_mcrmrc, - .help = "write coprocessor", - .usage = "<cpnum> <op1> <op2> <CRn> <CRm> <value>", - }; - register_command(cmd_ctx, NULL, &mcr_cmd); - } - - if (target->type->mrc == NULL) - { - target->type->mrc = default_mrc; - } else - { - const struct command_registration mrc_cmd = { - .name = "mrc", - .jim_handler = &jim_mcrmrc, - .help = "read coprocessor", - .usage = "<cpnum> <op1> <op2> <CRn> <CRm>", - }; - register_command(cmd_ctx, NULL, &mrc_cmd); - } - /** * @todo get rid of those *memory_imp() methods, now that all @@ -4883,92 +4770,6 @@ COMMAND_HANDLER(handle_fast_load_command return retval; } -static int jim_mcrmrc(Jim_Interp *interp, int argc, Jim_Obj *const *argv) -{ - struct command_context *context; - struct target *target; - int retval; - - context = Jim_GetAssocData(interp, "context"); - if (context == NULL) { - LOG_ERROR("array2mem: no command context"); - return JIM_ERR; - } - target = get_current_target(context); - if (target == NULL) { - LOG_ERROR("array2mem: no current target"); - return JIM_ERR; - } - - if ((argc < 6) || (argc > 7)) - { - return JIM_ERR; - } - - int cpnum; - uint32_t op1; - uint32_t op2; - uint32_t CRn; - uint32_t CRm; - uint32_t value; - - int e; - long l; - e = Jim_GetLong(interp, argv[1], &l); - if (e != JIM_OK) { - return e; - } - cpnum = l; - - e = Jim_GetLong(interp, argv[2], &l); - if (e != JIM_OK) { - return e; - } - op1 = l; - - e = Jim_GetLong(interp, argv[3], &l); - if (e != JIM_OK) { - return e; - } - CRn = l; - - e = Jim_GetLong(interp, argv[4], &l); - if (e != JIM_OK) { - return e; - } - CRm = l; - - e = Jim_GetLong(interp, argv[5], &l); - if (e != JIM_OK) { - return e; - } - op2 = l; - - value = 0; - - if (argc == 7) - { - e = Jim_GetLong(interp, argv[6], &l); - if (e != JIM_OK) { - return e; - } - value = l; - - retval = target_mcr(target, cpnum, op1, op2, CRn, CRm, value); - if (retval != ERROR_OK) - return JIM_ERR; - } else - { - retval = target_mrc(target, cpnum, op1, op2, CRn, CRm, &value); - if (retval != ERROR_OK) - return JIM_ERR; - - Jim_SetResult(interp, Jim_NewIntObj(interp, value)); - } - - return JIM_OK; -} - static const struct command_registration target_command_handlers[] = { { .name = "targets", --- a/src/target/target_type.h +++ b/src/target/target_type.h @@ -213,11 +213,6 @@ struct target_type int (*mmu)(struct target *target, int *enabled); - /* Read coprocessor - arm specific. Default implementation returns error. */ - int (*mrc)(struct target *target, int cpnum, uint32_t op1, uint32_t op2, uint32_t CRn, uint32_t CRm, uint32_t *value); - - /* Write coprocessor. Default implementation returns error. */ - int (*mcr)(struct target *target, int cpnum, uint32_t op1, uint32_t op2, uint32_t CRn, uint32_t CRm, uint32_t value); }; #endif // TARGET_TYPE_H |
From: Zach W. <zw...@su...> - 2009-12-01 08:57:19
|
On Mon, 2009-11-30 at 23:42 -0800, David Brownell wrote: > On Monday 30 November 2009, David Brownell wrote: > > (Patches to move the "mrc" and "mcr" code into > > "struct arm" are due too ... MIPS and other cores do not support > > those ARM-specific concepts.) > > Needed for testing, even ... here it is. Goes before slightly > tweaked versions of the last few patches. Sweet! I am very glad to see this moved. > These behave in light testing on Cortex-A8; didn't make time to > do so with ARM1136. I'm going to merge the series anyway. I'll trust you to fix any reported regressions. ;) --Z |