JTAG IR scans are a bit wierd ... you can't read instructions,
you get some bitstring with 01 as two LSBs.  Or so (I'm told)
says the IEEE spec.  (Note that if all higher bits were zero
too, it would be possible to autoconfigure IR length...)

Change the handling of the "-ircapture" and "-irmask" parameters
to be slightly more sensible, given that expected behavior:

 - First, provide and use default values that satisfy the IEEE spec.
   Existing TAP configs will override the defaults, but those parms
   are no longer required.

 - Second, warn if any TAP gets set up to violate the JTAG spec.
   It's likely a bug, but maybe not; else this should be an error.
   Improve the related diagnostics to say which TAP is affected.

And minor fixes:  get rid of a memory leak, shrink a line'o'code,
fix dangerous macro, add note re allocating extra bytes.
---
REQUEST:  someone with the IEEE JTAG spec, please verify that
entry to IR_Capture state is required to load the shift register
with xxxxx01 bits (as I'm told it does).

Regardless, I suspect I'll commit this soonish unless someone
explains why I shouldn't...

 doc/openocd.texi          |   22 ++++++++++++----------
 src/helper/binarybuffer.h |    2 +-
 src/jtag/core.c           |    5 ++++-
 src/jtag/tcl.c            |   34 ++++++++++++++++++++++------------
 4 files changed, 39 insertions(+), 24 deletions(-)

--- a/doc/openocd.texi
+++ b/doc/openocd.texi
@@ -2309,19 +2309,9 @@ a JTAG TAP; that TAP should be named @co
 Every TAP requires at least the following @var{configparams}:
 
 @itemize @bullet
-...@item @code{-ircapture} @var{NUMBER}
-...@*the bit pattern loaded by the TAP into the JTAG shift register
-on entry to the @sc{ircapture} state, such as 0x01.
-JTAG requires the two LSBs of this value to be 01.
-The value is used to verify that instruction scans work correctly.
 @item @code{-irlen} @var{NUMBER}
 @*The length in bits of the
 instruction register, such as 4 or 5 bits.
-...@item @code{-irmask} @var{NUMBER}
-...@*a mask for the IR register.
-For some devices, there are bits in the IR that aren't used.
-This lets OpenOCD mask them off when doing IDCODE comparisons.
-In general, this should just be all ones for the size of the IR.
 @end itemize
 
 A TAP may also provide optional @var{configparams}:
@@ -2340,6 +2330,18 @@ found when the JTAG chain is examined.
 These codes are not required by all JTAG devices.
 @emph{Repeat the option} as many times as required if more than one
 ID code could appear (for example, multiple versions).
+...@item @code{-ircapture} @var{NUMBER}
+...@*the bit pattern loaded by the TAP into the JTAG shift register
+on entry to the @sc{ircapture} state, such as 0x01.
+JTAG requires the two LSBs of this value to be 01.
+By default, @code{-ircapture} and @code{-irmask} are set
+up to verify that two-bit value; but you may provide
+additional bits, if you know them.
+...@item @code{-irmask} @var{NUMBER}
+...@*a mask used with @code{-ircapture}
+to verify that instruction scans work correctly.
+Such scans are not used by OpenOCD except to verify that
+there seems to be no problems with JTAG scan chain operations.
 @end itemize
 @end deffn
 
--- a/src/helper/binarybuffer.h
+++ b/src/helper/binarybuffer.h
@@ -86,7 +86,7 @@ extern char* buf_to_str(const uint8_t *b
 struct scan_field_s;
 extern int buf_to_u32_handler(uint8_t *in_buf, void *priv, struct scan_field_s 
*field);
 
-#define CEIL(m, n)     ((m + n - 1) / n)
+#define CEIL(m, n)     (((m) + (n) - 1) / (n))
 
 /* read a uint32_t from a buffer in target memory endianness */
 static inline uint32_t fast_target_buffer_get_u32(const uint8_t *buffer, int 
little)
--- a/src/jtag/core.c
+++ b/src/jtag/core.c
@@ -1115,6 +1115,7 @@ void jtag_tap_init(jtag_tap_t *tap)
 {
        assert(0 != tap->ir_length);
 
+       /// @todo fix, this allocates one byte per bit for all three fields!
        tap->expected = malloc(tap->ir_length);
        tap->expected_mask = malloc(tap->ir_length);
        tap->cur_instr = malloc(tap->ir_length);
@@ -1132,7 +1133,8 @@ void jtag_tap_init(jtag_tap_t *tap)
        LOG_DEBUG("Created Tap: %s @ abs position %d, "
                        "irlen %d, capture: 0x%x mask: 0x%x", tap->dotted_name,
                                tap->abs_chain_position, tap->ir_length,
-                         (unsigned int)(tap->ir_capture_value), (unsigned 
int)(tap->ir_capture_mask));
+                               (unsigned) tap->ir_capture_value,
+                               (unsigned) tap->ir_capture_mask);
        jtag_tap_add(tap);
 }
 
@@ -1141,6 +1143,7 @@ void jtag_tap_free(jtag_tap_t *tap)
        jtag_unregister_event_callback(&jtag_reset_callback, tap);
 
        /// @todo is anything missing? no memory leaks please
+       free((void *)tap->expected);
        free((void *)tap->expected_ids);
        free((void *)tap->chip);
        free((void *)tap->tapname);
--- a/src/jtag/tcl.c
+++ b/src/jtag/tcl.c
@@ -242,13 +242,15 @@ static int jim_newtap_cmd(Jim_GetOptInfo
        LOG_DEBUG("Creating New Tap, Chip: %s, Tap: %s, Dotted: %s, %d params",
                          pTap->chip, pTap->tapname, pTap->dotted_name, 
goi->argc);
 
-       /* deal with options */
-#define NTREQ_IRLEN      1
-#define NTREQ_IRCAPTURE  2
-#define NTREQ_IRMASK     4
+       /* IEEE specifies that the two LSBs of an IR scan are 01, so make
+        * that the default.  The "-irlen" and "-irmask" options are only
+        * needed to cope with nonstandard TAPs, or to specify more bits.
+        */
+       pTap->ir_capture_mask = 0x03;
+       pTap->ir_capture_value = 0x01;
 
-       /* clear them as we find them */
-       reqbits = (NTREQ_IRLEN | NTREQ_IRCAPTURE | NTREQ_IRMASK);
+       /* clear flags for "required options" them as we find them */
+       reqbits = 1;
 
        while (goi->argc) {
                e = Jim_GetOpt_Nvp(goi, opts, &n);
@@ -308,31 +310,39 @@ static int jim_newtap_cmd(Jim_GetOptInfo
                        switch (n->value) {
                        case NTAP_OPT_IRLEN:
                                if (w > (jim_wide) (8 * 
sizeof(pTap->ir_capture_value)))
-                                       LOG_WARNING("huge IR length %d", (int) 
w);
+                                       LOG_WARNING("%s: huge IR length %d",
+                                                       pTap->dotted_name,
+                                                       (int) w);
                                pTap->ir_length = w;
-                               reqbits &= (~(NTREQ_IRLEN));
+                               reqbits = 0;
                                break;
                        case NTAP_OPT_IRMASK:
                                if (is_bad_irval(pTap->ir_length, w)) {
-                                       LOG_ERROR("IR mask %x too big",
+                                       LOG_ERROR("%s: IR mask %x too big",
+                                                       pTap->dotted_name,
                                                        (int) w);
                                        free((void *)pTap->dotted_name);
                                        free(pTap);
                                        return ERROR_FAIL;
                                }
+                               if ((w & 3) != 3)
+                                       LOG_WARNING("%s: nonstandard IR mask",
+                                                       pTap->dotted_name);
                                pTap->ir_capture_mask = w;
-                               reqbits &= (~(NTREQ_IRMASK));
                                break;
                        case NTAP_OPT_IRCAPTURE:
                                if (is_bad_irval(pTap->ir_length, w)) {
-                                       LOG_ERROR("IR capture %x too big",
+                                       LOG_ERROR("%s: IR capture %x too big",
+                                                       pTap->dotted_name,
                                                        (int) w);
                                        free((void *)pTap->dotted_name);
                                        free(pTap);
                                        return ERROR_FAIL;
                                }
+                               if ((w & 3) != 1)
+                                       LOG_WARNING("%s: nonstandard IR value",
+                                                       pTap->dotted_name);
                                pTap->ir_capture_value = w;
-                               reqbits &= (~(NTREQ_IRCAPTURE));
                                break;
                        }
                } /* switch (n->value) */
_______________________________________________
Openocd-development mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/openocd-development

Reply via email to