Spen, et. al.,
As promised, here's a revised patch that preserves the DCB_DCRDR
across halts.
It also zeros the DCRDR just before the initial resume after any
reset.
The version requires no target initialization for funky delays
and seems to step through code without side effects on the DCC
data transfer.
I noticed that the cortex-m version saves and restores the
DCB_DCRDR across each ocd register access.
The attached patch for the ST-Link only preserves the DCRDR once
on entering the halted state and restores it (once) just before
resuming. This is obviously more efficient.
But, I strongly suspect that you had a good reason for doing
preserving the DCRDR repeatedly, on each register access.
What corner case am I missing here?
Also, note that, because I didn't want to modify other files, I
(mis)used target->dbgbase to store the DCRDR while the target is
halted. Can you or anyone else recommend a better place to save
this value?
I wasn't able to find a private data area associated with targets.
Should it be associated with the interface? If so, how best to
code that?
Just point me to something similar, done right.
Unfortunately, I won't have time to do much more with this for
at least the next couple months.
I'll look into merging this patch then (via gerrit), if you
don't beat me to it.
- brent
Spencer Oliver wrote:
On 08/01/13 07:52, Brent Roman wrote:
Spen,
Yes, I could see where you would have liked to have a target
specific
method for reading a word from the DCC.
That would have avoided some code duplication.
Note 1:
I don't see any code that lets the the ST-Link interface
write 16-bit
words atomically. This requires that it one write just the
low byte
when resetting the "busy" LSB of the DCC. If you just try
overwriting
the whole 16-bit word, the target usually slips in its next
write before
the relatively slow JTAG host zeros out the high byte.
There probably should be a comment about this added to
stm32_stlink_dcc_read()
It is currently not possible todo real 16 bit writes at all on
the stlink. The stlink firmware will probably do two 8 bit
writes, so it may be worth just writing a single 8bit to clear
the busy bit.
You should not need to modify the target dcc lib.
Note 2:
After further testing, I discovered that DCC commands output
at the top
of the 'C' main() would appear to be missed by the host.
Adding a 150ms
delay in the target "fixed" this, but I wasn't pleased.
The real problem is that the DCC busy bit was coming up set,
so the host
would misinterpret the zero in the high byte as a TRACEMSG
command,
which would be followed by the trace number. That would get
the protocol
out of sync.
If you look at the std cortexm implementation you will see it
clears/saves/restores the DCB_DCRDR register so you do not get
this problem.
The stlink version would have todo something similar.
A better fix is to have the target clear the DDC-link busy
bit a the top
of its 'C' main().
Theoretically, this is still a race. Can one assume the
target will get
to its 'C' main() before the host reads its DCC?
If not, it would seem that the "correct" approach might be
for the host
to also clear the DCC busy bit on target reset.
What would be the best way to do that?
Despite all the above, it now seems to work well in practice.
I'd be very pleased if you'd merge this patch. Thank you.
Can you upload to gerrit for review?
- brent
P.S. I've attached an optimized version of dcc_stio I call
dccput (as
it does not do any input)
Do with it what you will. Mainly, I just hoisted invariants
out of
inner loops.
P.P.S. What does hla stand for?
High Level Adapter
Spen
--
Brent Roman MBARI
Software Engineer Tel: (831) 775-1808
425 Clinton Street, Santa Cruz, CA 95062
mailto:[email protected] http://www.mbari.org/~brent
--- code/src/target/original/stm32_stlink.c 2013-01-06 16:47:39.499308560 -0800
+++ code/src/target/stm32_stlink.c 2013-01-08 22:08:59.305373887 -0800
@@ -5,6 +5,8 @@
* Copyright (C) 2011 by Spencer Oliver *
* [email protected] *
* *
+ * revised: 1/8/13 by [email protected] [DCC target request support] *
+ * *
* This program is free software; you can redistribute it and/or modify *
* it under the terms of the GNU General Public License as published by *
* the Free Software Foundation; either version 2 of the License, or *
@@ -37,9 +39,12 @@
#include "armv7m.h"
#include "cortex_m.h"
#include "arm_semihosting.h"
+#include "target_request.h"
+
+#define savedDCRDR dbgbase //FIXME: using target->dbgbase to preserve DCRDR
-#define ARMV7M_SCS_DCRSR 0xe000edf4
-#define ARMV7M_SCS_DCRDR 0xe000edf8
+#define ARMV7M_SCS_DCRSR DCB_DCRSR
+#define ARMV7M_SCS_DCRDR DCB_DCRDR
static inline struct stlink_interface_s *target_to_stlink(struct target *target)
{
@@ -265,6 +270,80 @@
return ERROR_OK;
}
+
+static int stm32_stlink_dcc_read(struct stlink_interface_s *stlink_if, uint8_t *value, uint8_t *ctrl)
+{
+ uint16_t dcrdr;
+ int retval = stlink_if->layout->api->read_mem8(stlink_if->fd,
+ DCB_DCRDR, sizeof(dcrdr), (uint8_t *)&dcrdr);
+ if (retval == ERROR_OK) {
+ *ctrl = (uint8_t)dcrdr;
+ *value = (uint8_t)(dcrdr >> 8);
+
+ LOG_DEBUG("data 0x%x ctrl 0x%x", *value, *ctrl);
+
+ if (dcrdr & 1) {
+ /* write ack back to software dcc register
+ * to signify we have read data */
+ //atomically clear just the byte containing the busy bit
+ static const uint8_t zero = 0;
+ retval = stlink_if->layout->api->write_mem8(stlink_if->fd, DCB_DCRDR, 1, &zero);
+ }
+ }
+ return retval;
+}
+
+static int stm32_stlink_target_request_data(struct target *target,
+ uint32_t size, uint8_t *buffer)
+{
+ struct stlink_interface_s *stlink_if = target_to_stlink(target);
+ uint8_t data;
+ uint8_t ctrl;
+ uint32_t i;
+
+ for (i = 0; i < (size * 4); i++) {
+ stm32_stlink_dcc_read(stlink_if, &data, &ctrl);
+ buffer[i] = data;
+ }
+
+ return ERROR_OK;
+}
+
+static int stm32_stlink_handle_target_request(void *priv)
+{
+ struct target *target = priv;
+ if (!target_was_examined(target))
+ return ERROR_OK;
+ struct stlink_interface_s *stlink_if = target_to_stlink(target);
+
+ if (!target->dbg_msg_enabled)
+ return ERROR_OK;
+
+ if (target->state == TARGET_RUNNING) {
+ uint8_t data;
+ uint8_t ctrl;
+
+ stm32_stlink_dcc_read(stlink_if, &data, &ctrl);
+
+ /* check if we have data */
+ if (ctrl & (1 << 0)) {
+ uint32_t request;
+
+ /* we assume target is quick enough */
+ request = data;
+ stm32_stlink_dcc_read(stlink_if, &data, &ctrl);
+ request |= (data << 8);
+ stm32_stlink_dcc_read(stlink_if, &data, &ctrl);
+ request |= (data << 16);
+ stm32_stlink_dcc_read(stlink_if, &data, &ctrl);
+ request |= (data << 24);
+ target_request(target, request);
+ }
+ }
+
+ return ERROR_OK;
+}
+
static int stm32_stlink_init_arch_info(struct target *target,
struct cortex_m3_common *cortex_m3,
struct jtag_tap *tap)
@@ -282,6 +361,8 @@
armv7m->examine_debug_reason = stm32_stlink_examine_debug_reason;
armv7m->stlink = true;
+ target_register_timer_callback(stm32_stlink_handle_target_request, 1, 1, target);
+
return ERROR_OK;
}
@@ -332,6 +413,11 @@
uint32_t xPSR;
int retval;
+ /* preserve the DCRDR across halts */
+ retval = target_read_u32(target, DCB_DCRDR, &target->savedDCRDR);
+ if (retval != ERROR_OK)
+ return retval;
+
retval = armv7m->examine_debug_reason(target);
if (retval != ERROR_OK)
return retval;
@@ -476,7 +562,6 @@
static int stm32_stlink_deassert_reset(struct target *target)
{
- int res;
struct stlink_interface_s *stlink_if = target_to_stlink(target);
enum reset_types jtag_reset_config = jtag_get_reset_config();
@@ -491,14 +576,9 @@
*/
jtag_add_reset(0, 0);
- if (!target->reset_halt) {
- res = target_resume(target, 1, 0, 0, 0);
+ target->savedDCRDR = 0; //clear both DCC busy bits on initial resume
- if (res != ERROR_OK)
- return res;
- }
-
- return ERROR_OK;
+ return target->reset_halt ? ERROR_OK : target_resume(target, 1, 0, 0, 0);
}
static int stm32_stlink_soft_reset_halt(struct target *target)
@@ -567,6 +647,11 @@
armv7m_restore_context(target);
+ /* restore savedDCRDR */
+ res = target_write_u32(target, DCB_DCRDR, target->savedDCRDR);
+ if (res != ERROR_OK)
+ return res;
+
/* registers are now invalid */
register_cache_invalidate(armv7m->core_cache);
@@ -640,6 +725,11 @@
armv7m_restore_context(target);
+ /* restore savedDCRDR */
+ res = target_write_u32(target, DCB_DCRDR, target->savedDCRDR);
+ if (res != ERROR_OK)
+ return res;
+
target_call_event_callbacks(target, TARGET_EVENT_RESUMED);
res = stlink_if->layout->api->step(stlink_if->fd);
@@ -782,6 +872,8 @@
.poll = stm32_stlink_poll,
.arch_state = armv7m_arch_state,
+ .target_request_data = stm32_stlink_target_request_data,
+
.assert_reset = stm32_stlink_assert_reset,
.deassert_reset = stm32_stlink_deassert_reset,
.soft_reset_halt = stm32_stlink_soft_reset_halt,
------------------------------------------------------------------------------
Master Java SE, Java EE, Eclipse, Spring, Hibernate, JavaScript, jQuery
and much more. Keep your Java skills current with LearnJavaNow -
200+ hours of step-by-step video tutorials by Java experts.
SALE $49.99 this month only -- learn more at:
http://p.sf.net/sfu/learnmore_122612
_______________________________________________
OpenOCD-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openocd-devel
