From: Alamy Liu Date: Wed, 12 Aug 2015 00:03:57 +0000 (-0700) Subject: adi_v5: Fix wrong ap value X-Git-Tag: v0.10.0-rc1~350 X-Git-Url: https://git.sur5r.net/?a=commitdiff_plain;h=bfba15a898ae8eb2086f8981fcabf1298ec5c761;p=openocd adi_v5: Fix wrong ap value Problem dap->ap_current is register value, not field value. it restores invalid ap when it calls dap_ap_select(dap, ap_old) later. * assume the current ap is 1, dap->ap_current value would be (1 << 24). ap_old = dap->ap_current; <-- ap_old = 1<<24 = 0x1000000. ... dap_ap_select(dap, ap_old); <-- select 0x1000000, not 1. * All AP registers accessing fail afterwards. One of the reproducible case(s): CORE residents in AP >= 1 dap_lookup_cs_component() being used to find PE(*). In most cases, PE would be found in AP==0, hence the problem is hidden. When AP number is 1, dap->ap_current would have the value of 1<<24. Anyone get the AP value with dap->ap_current and resotre it later would select the wrong AP and all accessing later would fail. The ARM Versatile and/or FPGA would have better chance to provide this kind of environment that PE residents in AP>=1. As they have an 'umbrella' system at AP0, and main system at AP>=1. * PE: Processing Element. AKA Core. See ARM Glossary at http://infocenter.arm.com/help/topic/com.arm.doc.aeg0014g/ABCDEFGH.html Fix Use dap_ap_get_select() to get ap value. a. Retrieve current ap value by calling dap_ap_get_select(); src/flash/nor/kinetis.c src/target/arm_adi_v5.c b. The code is correct (dap->ap_current >> 24), but it's better to use dap_ap_get_select() so everything could be synchronized. src/flash/nor/sim3x.c Change-Id: I97b5a13a3fc5506cf287e299c6c35699374de74f Signed-off-by: Alamy Liu Reviewed-on: http://openocd.zylin.com/2935 Reviewed-by: Andreas Färber Tested-by: jenkins Reviewed-by: Tomas Vanek Reviewed-by: Matthias Welwarsky --- diff --git a/src/flash/nor/kinetis.c b/src/flash/nor/kinetis.c index ce5807ff..17952790 100644 --- a/src/flash/nor/kinetis.c +++ b/src/flash/nor/kinetis.c @@ -316,7 +316,7 @@ COMMAND_HANDLER(kinetis_mdm_mass_erase) } int retval; - const uint8_t original_ap = dap->ap_current; + const uint8_t original_ap = dap_ap_get_select(dap); /* * ... Power on the processor, or if power has already been @@ -420,7 +420,7 @@ COMMAND_HANDLER(kinetis_check_flash_security_status) uint32_t val; int retval; - const uint8_t origninal_ap = dap->ap_current; + const uint8_t origninal_ap = dap_ap_get_select(dap); dap_ap_select(dap, 1); diff --git a/src/flash/nor/sim3x.c b/src/flash/nor/sim3x.c index 2a870028..0a5906c1 100644 --- a/src/flash/nor/sim3x.c +++ b/src/flash/nor/sim3x.c @@ -959,7 +959,7 @@ COMMAND_HANDLER(sim3x_mass_erase) return ERROR_FAIL; } - const uint8_t origninal_ap = dap->ap_current >> 24; + const uint8_t origninal_ap = dap_ap_get_select(dap); dap_ap_select(dap, SIM3X_AP); ret = ap_read_register(dap, SIM3X_AP_ID, &val); @@ -1017,7 +1017,7 @@ COMMAND_HANDLER(sim3x_lock) return ERROR_FAIL; } } else { - const uint8_t origninal_ap = dap->ap_current >> 24; + const uint8_t origninal_ap = dap_ap_get_select(dap); dap_ap_select(dap, SIM3X_AP); /* check SIM3X_AP_ID */ diff --git a/src/target/arm_adi_v5.c b/src/target/arm_adi_v5.c index f7e58d08..c2929ef7 100644 --- a/src/target/arm_adi_v5.c +++ b/src/target/arm_adi_v5.c @@ -844,7 +844,7 @@ int dap_get_debugbase(struct adiv5_dap *dap, int ap, if (ap >= 256) return ERROR_COMMAND_SYNTAX_ERROR; - ap_old = dap->ap_current; + ap_old = dap_ap_get_select(dap); dap_ap_select(dap, ap); retval = dap_queue_ap_read(dap, AP_REG_BASE, dbgbase); @@ -873,7 +873,7 @@ int dap_lookup_cs_component(struct adiv5_dap *dap, int ap, return ERROR_COMMAND_SYNTAX_ERROR; *addr = 0; - ap_old = dap->ap_current; + ap_old = dap_ap_get_select(dap); dap_ap_select(dap, ap); do { @@ -1408,7 +1408,7 @@ static int dap_info_command(struct command_context *cmd_ctx, if (retval != ERROR_OK) return retval; - ap_old = dap->ap_current; + ap_old = dap_ap_get_select(dap); dap_ap_select(dap, ap); /* Now we read ROM table ID registers, ref. ARM IHI 0029B sec */