adi_v5: Fix wrong ap value 35/2935/6
authorAlamy Liu <alamy.liu@gmail.com>
Wed, 12 Aug 2015 00:03:57 +0000 (17:03 -0700)
committerPaul Fertser <fercerpav@gmail.com>
Fri, 27 Nov 2015 10:38:22 +0000 (10:38 +0000)
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 <alamy.liu@gmail.com>
Reviewed-on: http://openocd.zylin.com/2935
Reviewed-by: Andreas Färber <afaerber@suse.de>
Tested-by: jenkins
Reviewed-by: Tomas Vanek <vanekt@fbl.cz>
Reviewed-by: Matthias Welwarsky <matthias@welwarsky.de>
src/flash/nor/kinetis.c
src/flash/nor/sim3x.c
src/target/arm_adi_v5.c

index ce5807ffedf4bb449de39bf45b45086cb589dc20..179527905db56b35be2ebb866f993ccdfc5a7fe3 100644 (file)
@@ -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);
 
index 2a870028240da770683df2ea53888c348dc5781b..0a5906c12b8641ce03d7cd5e776cd2d9c8516fb3 100644 (file)
@@ -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 */
index f7e58d08b8fbc561fe4c8ed47c1e05d6059418a6..c2929ef7fe466679bcfc558fc90ed2f9c1ea458f 100644 (file)
@@ -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  */

Linking to existing account procedure

If you already have an account and want to add another login method you MUST first sign in with your existing account and then change URL to read https://review.openocd.org/login/?link to get to this page again but this time it'll work for linking. Thank you.

SSH host keys fingerprints

1024 SHA256:YKx8b7u5ZWdcbp7/4AeXNaqElP49m6QrwfXaqQGJAOk gerrit-code-review@openocd.zylin.com (DSA)
384 SHA256:jHIbSQa4REvwCFG4cq5LBlBLxmxSqelQPem/EXIrxjk gerrit-code-review@openocd.org (ECDSA)
521 SHA256:UAOPYkU9Fjtcao0Ul/Rrlnj/OsQvt+pgdYSZ4jOYdgs gerrit-code-review@openocd.org (ECDSA)
256 SHA256:A13M5QlnozFOvTllybRZH6vm7iSt0XLxbA48yfc2yfY gerrit-code-review@openocd.org (ECDSA)
256 SHA256:spYMBqEYoAOtK7yZBrcwE8ZpYt6b68Cfh9yEVetvbXg gerrit-code-review@openocd.org (ED25519)
+--[ED25519 256]--+
|=..              |
|+o..   .         |
|*.o   . .        |
|+B . . .         |
|Bo. = o S        |
|Oo.+ + =         |
|oB=.* = . o      |
| =+=.+   + E     |
|. .=o   . o      |
+----[SHA256]-----+
2048 SHA256:0Onrb7/PHjpo6iVZ7xQX2riKN83FJ3KGU0TvI0TaFG4 gerrit-code-review@openocd.zylin.com (RSA)