arm_adi_v5: do not deactivate power domains while trying to clear sticky error

At OpenOCD start-up the operation of clearing the sticky error in
CTRL/STAT register ignores the current value of the power domains
bits CDBGPWRUPREQ and CSYSPWRUPREQ in the same register and
incorrectly set them to zero.
This abrupt disable does not follow the requirement in IHI0031 to
wait for the acknowledgment of power disabled before continuing.
The power domains are then re-enabled immediately after; it is
possible that such short disable period has passed undetected or
has been tested only on devices that do not implement the power
domains.
Anyway, this sequence is incorrect and can generate unexpected
and hard-to-debug issues while OpenOCD attaches to a running
target that implements power domains.

Anticipate the initialization of dap->dp_ctrl_stat and use it
while clearing the sticky bit. This has the additional effect of
avoiding a power disable in the error recovery part of the
function dap_dp_read_atomic().
Keep the same sequence of read/write in dap_dp_init() to avoid
breaking the initialization of some problematic target.
Add comments to document these choices.

Change-Id: I8d6da788f2dd11909792b5d6b69bc90fbe4df25d
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: http://openocd.zylin.com/4677
Tested-by: jenkins
Reviewed-by: Tomas Vanek <vanekt@fbl.cz>
Reviewed-by: Matthias Welwarsky <matthias@welwarsky.de>

diff --git a/src/target/arm_adi_v5.c b/src/target/arm_adi_v5.c
index e62ac07577acce79c56af03dcd8b04764a5b0ffe..03e642bfeaec701b228a40195208cb652dc41507 100644 (file)
--- a/src/target/arm_adi_v5.c
+++ b/src/target/arm_adi_v5.c
@@ -652,6 +652,15 @@ int dap_dp_init(struct adiv5_dap *dap)
 
        dap_invalidate_cache(dap);
 
 
        dap_invalidate_cache(dap);
 

+       /*
+        * Early initialize dap->dp_ctrl_stat.
+        * In jtag mode only, if the following atomic reads fail and set the
+        * sticky error, it will trigger the clearing of the sticky. Without this
+        * initialization system and debug power would be disabled while clearing
+        * the sticky error bit.
+        */
+       dap->dp_ctrl_stat = CDBGPWRUPREQ | CSYSPWRUPREQ;
+

        for (size_t i = 0; i < 30; i++) {
                /* DP initialization */
 
        for (size_t i = 0; i < 30; i++) {
                /* DP initialization */
 


@@ -660,7 +669,18 @@ int dap_dp_init(struct adiv5_dap *dap)
                        break;
        }
 
                        break;
        }
 

-       retval = dap_queue_dp_write(dap, DP_CTRL_STAT, SSTICKYERR);
+       /*
+        * This write operation clears the sticky error bit in jtag mode only and
+        * is ignored in swd mode. It also powers-up system and debug domains in
+        * both jtag and swd modes, if not done before.
+        * Actually we do not need to clear the sticky error here because it has
+        * been already cleared (if it was set) in the previous atomic read. This
+        * write could be removed, but this initial part of dap_dp_init() is the
+        * result of years of fine tuning and there are strong concerns about any
+        * unnecessary code change. It doesn't harm, so let's keep it here and
+        * preserve the historical sequence of read/write operations!
+        */
+       retval = dap_queue_dp_write(dap, DP_CTRL_STAT, dap->dp_ctrl_stat | SSTICKYERR);

        if (retval != ERROR_OK)
                return retval;
 
        if (retval != ERROR_OK)
                return retval;
 


@@ -668,7 +688,6 @@ int dap_dp_init(struct adiv5_dap *dap)
        if (retval != ERROR_OK)
                return retval;
 
        if (retval != ERROR_OK)
                return retval;
 

-       dap->dp_ctrl_stat = CDBGPWRUPREQ | CSYSPWRUPREQ;

        retval = dap_queue_dp_write(dap, DP_CTRL_STAT, dap->dp_ctrl_stat);
        if (retval != ERROR_OK)
                return retval;
        retval = dap_queue_dp_write(dap, DP_CTRL_STAT, dap->dp_ctrl_stat);
        if (retval != ERROR_OK)
                return retval;