Re: [Openocd-development] [PATCH] make sure file name case of at91sam3uxx matches what other files include
Patch awaiting review at http://openocd.zylin.com/247 . Next time, please push it directly to gerrit yourself. Read HACKING for instructions. Thanks! ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [OpenOCD-devel] Change in openocd[master]: gdb: add config command to auto send halt cmd
I just want to add that there is a tcl hook that gets called when gdb connects. Depending on how you use openocd/gdb, that one can be more convenient for doing a reset init than with monitor commands from within gdb. As has been said already, openocd (and gdb) should do as little as possible to the target by default. Attaching to a running target for post-mortem analysis is very useful and a by-default reset, or even halt, really hampers this. I'd rather consider it a bug if there are targets that can't be attached to while running (if possible in the debug architecture). I remember fixing a bug of this kind for the STM32 target. SF list is broken, btw. Or is it just me? /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] openocd patch: 96c3cbf cortex_m: add missing error checking
On Tue, Nov 1, 2011 at 11:16 PM, Tomek CEDRO tomek.ce...@gmail.com wrote: Hey does this code is related anyhow to arm_adi_v5 or this is totally alternative solution? Yes, it *uses* arm_adi_v5. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] pointers modification within functions
On Thu, Nov 3, 2011 at 12:41 AM, Tomek CEDRO tomek.ce...@gmail.com wrote: Hey, I have some problems with pointers and need some support plz ;-) I'm not sure I understand what the problem is, but I can give some general hints. int swd_bus_read_ack(swd_ctx_t *swdctx, swd_operation_t operation, char **ack){ swd_cmd_enqueue_miso_ack(swdctx, ack); You discard the return value and one of the parameters, but perhaps this is not the complete function? } int swd_cmd_enqueue_miso_ack(swd_ctx_t *swdctx, char **ack){ if (swdctx==NULL) return SWD_ERROR_NULLCONTEXT; int res; swd_cmd_t *cmd; cmd=(swd_cmd_t *)calloc(1,sizeof(swd_cmd_t)); As a side note, cmd = calloc(1, sizeof(*cmd)); is preferred, in case the type of cmd changes later. if (cmd==NULL) return SWD_ERROR_OUTOFMEM; if (ack!=NULL) *ack=cmd-ack; You probably want to wait with setting *ack until after checking res below. If swd_cmd_enqueue fails, ack will point to free'd memory, but you discard the return value in swd_bus_read_ack so the caller won't be able to tell. Leave *ack unchanged unless the function succeeds. cmd-bits=SWD_ACK_BITLEN; cmd-cmdtype=SWD_CMDTYPE_MISO_ACK; res=swd_cmd_enqueue(swdctx, cmd); //should be 1 on success if (res1) free(cmd); return res; } main(){ int *ack; swd_bus_read_ack(swdctx, operation, ack); This won't even compile. You pass a pointer-to-int, but swd_bus_read_ack expects a pointer-to-pointer-to-char. } The problem is: 1. I need to use double pointers to return back the address of the queue element (*ack=cmd-ack). Correct. 2. If I use single pointer *ack the value of the ack is only changed inside swd_cmd_enqueue_miso_ack() but after its return to swd_bus_read_ack() the ack value is taken back to the state as it was before swd_cmd_enqueue_miso_ack() call. You have already concluded that you need a double pointer in 1, so I don't know why you say this. A single pointer won't work. 3. I have tried to use single pointer *ack and call swd_bus_read_ack(swdctx, operation, ack) but is changes nothing. Ditto. You're reusing the same name (ack) for several things in your description (two function parameters and a variable) so it's hard to understand what you mean. This makes impossible to give back data on queue_dp_read(). There is something wrong with these pointers!!! Help! :-) ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] pointers modification within functions
On Thu, Nov 3, 2011 at 1:43 AM, Tomek CEDRO tomek.ce...@gmail.com wrote: Hello Andreas :-) On Thu, Nov 3, 2011 at 12:36 AM, Andreas Fritiofson andreas.fritiof...@gmail.com wrote: This won't even compile. You pass a pointer-to-int, but swd_bus_read_ack expects a pointer-to-pointer-to-char. naah this is only typo in mind-shortcut, code builds well, but i dont get it why i cannot use single pointer to pass back a memory location to function caller... this is what pointers exist.. Ah, well, but c passes parameters by value, to get out parameters you have to go via a pointer. Consider: void foo(int x) { x++; } int main(void) { int a = 3; foo(a); return 0; } Did you expect the value of a to change after the call to foo? No, didn't think so. You'd have to do this instead: void foo(int *x) { *x++; } int main(void) { int a = 3; foo(a); return 0; } Here a gets the value 4 after the call to foo. Now change the variable type to a pointer instead of an integer: void foo(int **x) { *x++; } int main(void) { int *a = bar; foo(a); return 0; } Here a points to the integer following bar after the call to foo. It's exactly the same concept, just a change of variable type from int to int*. Note that a is now a double pointer, nothing magic here. Do the same variable type change in the first, non-functional, example and convince yourself that that wont work either. ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] scan-build and gerrit rant (Was: Re: openocd patch: 620ba7e clang: fix warning by adding assert that shows that a variable is used)
On Sat, Oct 29, 2011 at 10:56 AM, ger...@openocd.zylin.com wrote: This is an automated email from Gerrit. ?yvind Harboe (oyvindhar...@gmail.com) just uploaded a new patch set to Gerrit, which you can find at http://openocd.zylin.com/134 -- gerrit commit 620ba7e6cd57c951fafa0f1ffab2db102fe2a60f Author: Øyvind Harboe oyvind.har...@zylin.com Date: Sat Oct 29 10:55:02 2011 +0200 clang: fix warning by adding assert that shows that a variable is used Change-Id: I26e0ba9f1ae9d43c9a14c42c4225746782dc4d66 Signed-off-by: Øyvind Harboe oyvind.har...@zylin.com diff --git a/src/jtag/tcl.c b/src/jtag/tcl.c index 468edf5..b634ac0 100644 --- a/src/jtag/tcl.c +++ b/src/jtag/tcl.c @@ -166,6 +166,8 @@ static int Jim_Command_drscan(Jim_Interp *interp, int argc, Jim_Obj *const *args } } /* validate args */ + assert(e == JIM_OK); + tap = jtag_tap_by_jim_obj(interp, args[1]); if (tap == NULL) { return JIM_ERR; I'm not very fond of the idea of merging patches with the sole purpose of fixing scan-build false positives. clang is not perfect and if we start tweaking perfectly good code and adding nonsense checks just to get a clean scan-build output, I think that's a step backwards in terms of code quality. In this case, the warning is probably bogus (I'll have to check the scan-build output but having problems logging in to jenkins). Unfortunately, the fix is, too. There's no point in adding an assert to check for the value of a variable when that value has no possible bearing on the program (the variable is never used after the assert, which, incidentally, was exactly what scan-build complained about). The correct fix would be to reduce the scope of 'e' to inside the loop, which would _increase_ code quality as well as (probably) silence scan-build. My point being that silencing a warning must only ever be a side effect of increasing code quality. The warning _may_ be indicative of a real bug, and forcing the warning to disappear willy-nilly also makes the bug a lot harder to find for someone that uses scan-build to try to pro-actively improve the code base. Of course, I would have added this comment in gerrit, if the patch hadn't already been merged, a measly few hours after upload, by the author himself, without comments from anyone else. No point in having a fancy review system if we're not given the chance to review. Please allow patches to remain in the queue for *days* not *hours*. Gerrit guarantees that no patch will be forgotten, so what's the hurry? I have no possibility to review, or at least comment on, patches during work-hours and sometimes not until the weekends. I'm sure I'm not alone. By then, the patches left in the queue are mostly those that have big red X's or the work-in-progress ones. And to my fellow maintainers; be extra patient with patches you've written yourself and wait for at least some form of feedback before merging. It's not that easy to spot flaws in one's own code so it's always good to get another pair of eyes on it, even if it _should_ be trivial. Regards, Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Remove qP from rtos code?
Sorry for bringing this old thread up (no pun intended). This code got a few minutes of my attention after I browsed through the clang static analysis report someone posted recently. rtos.c was one of the first in the list, and while the bug report probably was a false positive, I noticed some serious problems with this code. To be honest, I can't see how it could work at all. The mode variable is repeatedly bit manipulated in ways that can hardly leave any bits set at all. Further, the reply string is repeatedly written over so the reply will probably be nonsense, or at least not what gdb asked for. If this code actually does something useful, please stop me, otherwise I'll simply purge the qP code from rtos.c. /Andreas On Sat, Aug 27, 2011 at 5:01 PM, Jie Zhang jzhang...@gmail.com wrote: Hi Evan, If qThreadExtraInfo is not implemented, qP will be used. But since qThreadExtraInfo has now been implemented, qP should not be needed any more. GDB added qThreadExtraInfo more than 10 years ago. All GDB releases since 5.0 will not send out qP packet if the stub supports qThreadExtraInfo. So I think it's safe for OpenOCD to remove qP support and only keep qThreadExtraInfo. This will make code clean and reduce maintenance effort. Regards, Jie On Thu, Aug 25, 2011 at 8:50 PM, Evan Hunter e...@ozhiker.com wrote: Backward compatibility is the reason - When I was testing with GDB+eclipse I found that OpenOCD received qP packets sometimes, and I think I implemented it first, before reading that same quotation you mentioned. Then when I implemented qThreadExtraInfo, I figured it was nicer to keep qP compatibility too. Regards, Evan Quoting Jie Zhang jzhang...@gmail.com: Hi Evan, GDB manual says about qP: Don't use this packet; use the `qThreadExtraInfo' query instead (see below). Since qThreadExtraInfo is already supported in rtos.c, why qP is still needed? Regards, Jie ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Remove qP from rtos code?
Hi! On Thu, Oct 20, 2011 at 12:47 AM, Evan Hunter ehun...@broadcom.com wrote: You are right – Looking at it again, the “qP” code looks like I got halfway through implementing it, then found that I should be using qThreadExtraInfo instead. Please feel free to remove it. The only reason I haven’t already is that I’ve not had time. ** Ok, will do. Since you're familiar with the code, could you comment on the return value patch I just posted ( http://openocd.zylin.com/38 ). I'm just guessing here but it seems reasonable enough. Thanks, Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Only every second programming works
On Wed, Oct 19, 2011 at 2:23 PM, freddie_chopin freddie_cho...@op.plwrote: W dniu 2011-10-19 14:14:11 użytkownik Akos Vandra axo...@gmail.com napisał: #lpcfixchecksum takes only binary files, so #make a binary file from the elf, and fix the checksum. arm-eabi-objcopy -O binary $FILE tmp.bin lpcfixchecksum tmp.bin On LPC2xxx you can use elf, hex of bin - no need to convert anything. Actually, that's true for all targets. -c reset run -c mwb 0x400FC040 0x01 How do you expect to write anything to memory when chip is running? 99% of problems in OpenOCD are solved by using reset halt only. LPC17xx is Cortex-M3, right? Then it shouldn't have any problems with writing to memory with the core running. But what is the mwb 0x400FC040 0x01 doing? Perhaps it's better placed in the reset init handler and doing reset init instead. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Clock setup script for STM32F107
On Sun, Oct 9, 2011 at 12:37 PM, Simon Barner bar...@gmx.de wrote: Dear list, I have already posted the attached script some weeks ago. It enables the PLL of the STM32F107 and thus configures the MCU to run at 72 MHz which allows for higher JTAG speeds. Since this should be useful for everybody, I would like to contribute it to OpenOCD. However, I am not sure how to integrate it. One idea is to conditionally call it from scripts/target /stm32f1x.cfg. Comments and alternative suggestions are highly appreciated. First of all, it's better to run off of the HSI instead. Not everyone has a crystal attached, or it could be another frequency. For example, I have this in my config files (for STM32F103, I don't know what differs): $_TARGETNAME configure -event reset-init { mww 0x40022000 0x32 mww 0x40021004 0x3c0400 mww 0x40021000 0x01000883 sleep 10 mww 0x40021004 0x3c0402 sleep 10 adapter_khz 3000 } As Michael said, it's probably best to put in in a tcl procedure that the user can choose to call from the reset-init hook. Don't put the adapter_khz in that procedure. If the settings are the same for all devices in the family, the procedure probably belongs in target/stm32f1x.cfg. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] What's up? Why so slow?
On Sun, Oct 9, 2011 at 10:43 PM, Peter Stuge pe...@stuge.se wrote: Øyvind Harboe wrote: On Sun, Oct 9, 2011 at 3:40 AM, Peter Stuge pe...@stuge.se wrote: Transfer rate: 3 KB/sec, 13848 bytes/write. USB roundtrips? Ḯ'll have a look with usbmon soon. U+0618 LATIN CAPITAL LETTER I WITH DIRT ON TOP? I tried to scrape it off my monitor, dammit... :) It sounds like the LM3S flash algorithm could use a rewrite like the one I merged for STM32F10x yesterday. Flash speed went up from 9KB/s to 29 KB/s. I don't know why before-speed was so low when I tested yesterday, I normally got around 14 KB/s. Maybe the bus/computer was extra heavily loaded. I've never seen it as far down as your figures though. Almost as if the LM3S driver lacks block write completely. Examining the timestamps in the full debug log is a good complement to usbmon when debugging performance problems, btw. Akos Vandra wrote: (At least) in SWD mode with an FTDI programmer openocd is sending data bit by bit. I am planning to send in a patch to fix that, but I am currently unable to test it, because I am waiting for my SWD-capable programmer. LM3S knows JTAG and SWD. ICDI is FT2232 based and knows JTAG and SWD, and it's what the late David Brownell was doing his SWD work on, but my setup is using JTAG. I'm pretty sure that mainline ft2232 uses byte writes whenever possible. Akos: Is SWD even remotely up and running in mainline or are you using a fork? /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] What's up? Why so slow?
On Sun, Oct 9, 2011 at 11:28 PM, Akos Vandra axo...@gmail.com wrote: P.S. Is it just me, or the default reply address is the sender himself, and not the list? The list server doesn't set Reply-To to the list address, for some very good reason which I have forgotten now. Easiest fix is to set reply-to-all as the default reply action in your client. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] arm-jtag-ew fixes: Rebased patches
On Fri, Oct 7, 2011 at 12:03 AM, Andreas Fritiofson andreas.fritiof...@gmail.com wrote: On Thu, Sep 22, 2011 at 9:15 PM, Simon Barner bar...@gmx.de wrote: Dear list, I finally found some time to rebase my patches and to split them into smaller pieces (as suggested by Ųyvind Harboe). Please refer to the commit messages for the purpose of the individual patches. I would really appreciate if you could review them and consider them for inclusion into the OpenOCD main branch. If the patches are not in the expected format, or if you have further suggestions, please don't hesitate to contact me. Best regards, Simon Looks sane to me, but I don't have the relevant hardware. I'm going to merge these if there's no complaint in a day or two. /Andreas Merged w/ slightly reformatted comments, thanks! /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] contrib: fix udev rules for tty based adaptors
On Sat, Oct 8, 2011 at 6:12 PM, Luca BRUNO lu...@debian.org wrote: Luca Bruno scrisse: This patch fix udev rules to work with any listed tty-based adaptor. Speaking of udev, another related patch is currently sitting in the bug tracker at http://sourceforge.net/apps/trac/openocd/ticket/39 Would anyone care to review and commit it? Done, ticket closed. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PULL Request] Flash program speedup through asynchronous algorithms
On Wed, Aug 31, 2011 at 1:21 AM, Andreas Fritiofson andreas.fritiof...@gmail.com wrote: On Tue, Aug 16, 2011 at 8:40 AM, Simon Barner bar...@gmx.de wrote: On 16.08.2011 01:25, Andreas Fritiofson wrote: On Wed, Aug 10, 2011 at 12:01 AM, Spencer Oliver s...@spen-soft.co.uk mailto:s...@spen-soft.co.uk wrote: On 09/08/2011 22:15, Øyvind Harboe wrote: Any objections? I would like to give this a test-run tomorrow. Did you (or anyone else for that matter) get a chance to test it? Yes, I tested it and got a speedup of about 50 percent. I intend to merge these patches in a few days. Objections? Well, s/days/weeks/ but they're merged now. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] load_image trouble
On Fri, Oct 7, 2011 at 7:34 AM, Dmitry E. Oboukhov un...@debian.org wrote: I cant upload an image into internal SRAM (AT91SAM9). it can really write only 20 bytes. load_image into internal flash area works fine. But why it doesn't fail if it writes the other regions? It crashes only if it writes into internal SRAM. Could it be that the load_image command uses working area which overlaps with the address you're trying to write, thus corrupting the algorithm? No, I've tested writing area address:0x30-0x300040 (SRAM start address). So, where is your working area? I don't know which config file you use but, looking through some of the at91*.cfg, they generally set up working area to 0x30 and a few KB onwards. Have you tried moving it a bit to see if it makes a difference. I don't see why load_image should need to use working area so maybe it's a long shot. Worth a try anyway. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Atmel SAM3N with OpenOCD 0.5.0 (and git HEAD too)
On Fri, Oct 7, 2011 at 2:37 PM, Attila Kinali att...@kinali.ch wrote: On Thu, 6 Oct 2011 23:07:44 +0200 Andreas Fritiofson andreas.fritiof...@gmail.com wrote: It obviously hangs during init, which could mean just about anything. It's impossible to tell without a debug log (-d3). I strippped the script down to what is needed. And get the following output, using current git master/HEAD: ---schnipp--- # src/openocd -f /tmp/armusbocd.script Open On-Chip Debugger 0.6.0-dev-00090-gda8ce5f (2011-10-07-14:09) Licensed under GNU GPL v2 For bug reports, read http://openocd.berlios.de/doc/doxygen/bugs.html Info : only one transport option; autoselect 'jtag' 15 kHz trst_only separate trst_push_pull target created Flash bank access DONE Info : clock speed 15 kHz Info : JTAG tap: sam3n.cpu tap/device found: 0x4ba00477 (mfg: 0x23b, part: 0xba00, ver: 0x4) Info : sam3n.cpu: hardware has 6 breakpoints, 4 watchpoints post init ---schnapp--- Well, what's the problem now? It seems to work? Or at least get through init, since post init is printed now, which it wasn't before. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] FT2232: Add additional debug information with libftdi when cable connection fails.
Hi, On Sun, Oct 2, 2011 at 7:34 PM, Karl Kurbjun kkurb...@gmail.com wrote: This additional output was helpful when debugging a FTDI device with a broken EEPROM. I was expecting to connect to a cable with a particular description, but the device was not found. This error message would helps quickly identify the connection failure. A few comments: Avoid unrelated whitespace changes: - layout-name, vid, pid); + layout-name, vid, pid); Don't include unrelated style fixes. Besides, single statement bodies shouldn't have braces according to the kernel coding style, which we have more or less agreed to apply for OpenOCD. if (more) + { LOG_WARNING(unable to open ftdi device (trying more): %s, ftdic.error_str); + } else + { LOG_ERROR(unable to open ftdi device: %s, ftdic.error_str); + } Maybe a little *too* good job of limiting the scope: + /* Limit the scope of these variables */ + int i = 0; + i++; On a more general note, I'm not sure ft2232_init_libftdi is the correct place to dump all found devices, because if I'm not mistaken, it may get called several times if multiple vid/pid combinations are registered. Regards, Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Atmel SAM3N with OpenOCD 0.5.0
Hi, On Wed, Oct 5, 2011 at 3:15 PM, Attila Kinali att...@kinali.ch wrote: Moin, Has anyone got a Atmel SAM3N (N, not U) working with OpenOCD? SAM3U works with OpenOCD 0.5.0, but not with SAM3N. The device is properly detected, but when accessing the flash, OpenOCD hangs indefinitely. See the attached script I use and its output. That log doesn't show that OpenOCD hangs after accessing flash. The Flash bank access DONE part is wrong, the flash bank has only been set up in OpenOCD, there hasn't been any communication with the target yet. It obviously hangs during init, which could mean just about anything. It's impossible to tell without a debug log (-d3). Other than that I'm not of much use, I haven't worked with any of the targets. Regards, Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Atmel SAM3N with OpenOCD 0.5.0
On Thu, Oct 6, 2011 at 11:14 PM, Tomek CEDRO tomek.ce...@gmail.com wrote: On Thu, Oct 6, 2011 at 9:07 PM, Andreas Fritiofson andreas.fritiof...@gmail.com wrote: That log doesn't show that OpenOCD hangs after accessing flash. The Flash bank access DONE part is wrong, the flash bank has only been set up in OpenOCD, there hasn't been any communication with the target yet. It obviously hangs during init, which could mean just about anything. It's impossible to tell without a debug log (-d3). Hey Andreas, you can try to use GDB and retrace issue (if it does not crash then simply hit the Ctrl+C and use backtrace to see how did this got into such situation) :-) Of course I could, if it was me having the problem. Which it's not. :) But it's a good idea, I'm sure Attila can try it if the debug log doesn't show anything useful. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Show of hands for/against gerrit
On Thu, Oct 6, 2011 at 11:01 PM, Øyvind Harboe oyvind.har...@zylin.comwrote: I find gerrit intriguing as a way of managing patches. Can I have a show of hands of contributors for/against/don't care/don't know? Haven't heard of it until now, but sure, a tool like that could be useful. What would the workflow look like? On the other hand I'm a little afraid that adding yet another channel for OpenOCD may make relevant information less accessible. Compare for example mailing list bug reports vs the trac tickets. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] arm-jtag-ew fixes: Rebased patches
On Thu, Sep 22, 2011 at 9:15 PM, Simon Barner bar...@gmx.de wrote: Dear list, I finally found some time to rebase my patches and to split them into smaller pieces (as suggested by Ųyvind Harboe). Please refer to the commit messages for the purpose of the individual patches. I would really appreciate if you could review them and consider them for inclusion into the OpenOCD main branch. If the patches are not in the expected format, or if you have further suggestions, please don't hesitate to contact me. Best regards, Simon Looks sane to me, but I don't have the relevant hardware. I'm going to merge these if there's no complaint in a day or two. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] load_image trouble
Hi, On Sat, Sep 24, 2011 at 2:00 PM, Dmitry E. Oboukhov un...@debian.orgwrote: Dmitry E. Oboukhov wrote: I cant upload an image into internal SRAM (AT91SAM9). it can really write only 20 bytes. load_image into internal flash area works fine. But why it doesn't fail if it writes the other regions? It crashes only if it writes into internal SRAM. Could it be that the load_image command uses working area which overlaps with the address you're trying to write, thus corrupting the algorithm? I used openocd to program internal flash using algorithm like: 1. write 512 bytes (load_image) into flash 2. touch flash registers to start writting 3. repeat 1 with the other 512-block Tedious. Isn't there a flash driver for your target? Maybe write one? /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Possible bug in xscale_receive()
On Mon, Sep 19, 2011 at 4:30 PM, Matt Reimer mrei...@sdgsystems.com wrote: The code in xscale_receive() that tries to skip invalid reads (i.e. reads that don't have the DBG_SR[0] 'valid' bit set) seems to be wrong, as it only looks at the first word's valid flag rather than each word's own valid flag. Am I reading the code correctly? If so, the attached patch should fix it. If this looks correct, I'll generate a proper patch and commit message. After a quick glance, your analysis looks right. That also means the result shifting thingy hasn't gotten much exercise. Might be worth an extra look to see if it's correct. For one thing, wouldn't it be more natural (and efficient) to simply loop over the results, emitting the correct ones directly to the buffer while decrementing the word count, than to look for failures and shift the results for each one found? /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] Fixes and spellchecks for various Buspirate output messages
It's merged. Thanks! /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH]style: typedefs are evil
On Wed, Aug 31, 2011 at 12:27 AM, Martin Schmölzer martin.schmoel...@student.tuwien.ac.at wrote: Hi, On Tue, 2011-08-30 at 15:25 -0400, Eric Wetzel wrote: I don't think I have a way to verify the changes I made to src/flash/ocl/at91sam7x (from the second patch) or the changes I would make to src/jtag/drivers/OpenULINK. I'm the author of the ULINK driver. From a first glance, I think it is possible to fully get rid of all the typedefs in this driver and firmware. Tomorrow I'll take a closer look at the changes and report back. Just checked and the motivation for using shorter types (easier to stay within 80 character maximum line length, from shorttypes.h) is unfounded. Only one or two lines will grow beyond 80 chars because of the longer names, and other lines are far longer anyway. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH]style: typedefs are evil
On Tue, Aug 30, 2011 at 9:25 PM, Eric Wetzel thewet...@gmail.com wrote: On the discussion of style, I mentioned that the Linux coding standard doesn't really like typedefs. Here is an article from Greg K-H that partially addresses the subject: http://www.linuxjournal.com/article/5780?page=0,2 I don't really like typedefs either, so I started writing a script called 'detypedefifier' that will remove typedefs. So far, it only finds them, but I found and removed a good portion of the typedefs. The ones that remain are either function pointer typedefs, have far-reaching consequences, or are generally in places I don't really want to touch right now. Here is a list of the non-function pointer typedefs that remain after my patches: src/flash/mflash.h: Simple typedef: mg_io_uint32 = unsigned long Simple typedef: mg_io_uint16 = unsigned short Simple typedef: mg_io_uint8 = unsigned char These are probably supposed to be roll-your-own fixed-width types. If the mflash code depends on these actually being fixed-width, it is broken as it is. Unless someone who knows the mflash code objects, these should be replaced with standard uintX_t types. src/helper/types.h: Simple typedef: _Bool = int Simple typedef: _Bool = bool Simple typedef: intptr_t = CYG_ADDRWORD Simple typedef: intmax_t = int64_t Simple typedef: uintmax_t = uint64_t These are compatibility typedefs, don't touch them. src/jtag/drivers/OpenULINK/include/shorttypes.h: Simple typedef: s8 = int8_t Simple typedef: s16 = int16_t Simple typedef: s32 = int32_t Simple typedef: u8 = uint8_t Simple typedef: u16 = uint16_t Simple typedef: u32 = uint32_t Ok, the standard types are unnecessarily long, but they're standard. I always disliked the idea of introducing new typedefs even if it saves 4 or 5 characters per declaration. Wouldn't mind getting rid of these even though there's an exception for them in the kernel style guide. src/jtag/jtag.h: Simple typedef: jtag_callback_data_t = intptr_t src/rtos/rtos.h: Simple typedef: threadid_t = int64_t Simple typedef: symbol_address_t = int64_t Possibly these falls under the one of the kernel style guide exceptions, but I haven't checked their usage. Regarding the patches, the ELF types in replacements.h (and their usage in image.c) should be left as is, because they are replacements for a system header. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PULL REQUEST] dsp5680xx fix - halt, flash lock-unlock procedures
Did we decide whether patches should be posted to the list, in addition to the pull request, to ease reviews? Personally I much rather check the patches via a web interface, if one is available for the repo. On the other hand, having the patches in the list archive could be good (if they're not immediately merged) since private forks come and go so web links and git refs could become stale. Regardless, I have zero experience with this target so please speak up if there are objections to merging these. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PULL Request] Flash program speedup through asynchronous algorithms
On Tue, Aug 16, 2011 at 8:40 AM, Simon Barner bar...@gmx.de wrote: On 16.08.2011 01:25, Andreas Fritiofson wrote: On Wed, Aug 10, 2011 at 12:01 AM, Spencer Oliver s...@spen-soft.co.uk mailto:s...@spen-soft.co.uk wrote: On 09/08/2011 22:15, Øyvind Harboe wrote: Any objections? I would like to give this a test-run tomorrow. Did you (or anyone else for that matter) get a chance to test it? Yes, I tested it and got a speedup of about 50 percent. I intend to merge these patches in a few days. Objections? /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] GDB notification of processor reset
On Sat, Aug 27, 2011 at 12:43 AM, Evan Hunter e...@ozhiker.com wrote: Hi All, ** ** I've noticed that if my processor gets reset whilst debugging a program, GDB knows nothing about it. I would have thought GDB should stop, in a similar way as hitting a breakpoint. ** ** Is there currently any way of making this happen? I'm using a Cortex-M3 (STM32f1xx) ** Yes, take a look at the cortex_m3 vector_catch command. Although it has worked automagically for me even without setting a vector catch. Same processor. Haven't tried recently though. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Errors encountered programming an FPGA using 13MB SVF file
On Fri, Aug 26, 2011 at 7:10 PM, Sam Jansen sam.jan...@starleaf.com wrote: ** Hello, In attempting to use openocd with an Amontec JTAG key to program a Xilinx FPGA, I encountered a series of issues I though others might be interested in. I don't have sensible proposed solutions for any of the problems I encountered, but perhaps someone on this mailing list is interested all the same. The SVF file is a few SVF commands, with one very large SDR command in the middle; something like: SDR 27025501 TDI (80008000 ... ) SMASK (1fff ... This was generated by some Xilinx tools I believe. The file was roughly 13MB large, most of which was this one SDR command. The first problem is that openocd failed to read the SVF file; with the error message: buffer is not enough, report to author Not sure which author this is talking about, so I thought I'd inspect the code. And so the saga began... This code here is within an #if 1 clause, the #else looks like it had code to dynamically allocate enough memory for the command. Unfortunately, just changing the #if 1 to an #if 0 was not enough - I tried this once I got things working and ended up with strange TDO check errors. Hence I ended up just making the static buffer size large enough for my needs (16MB). On it's own, this isn't quite enough: the data gets copied into some sort of command object allocated in cmd_queue_alloc(). I patched this code to handle allocating sizes of CMD_QUEUE_PAGE_SIZE; without this I got a segfault in memcpy() as there is an un-checked memcpy() into the memory on this command. At this point, openocd no longer crashes with my SVF file. Unfortunately I started hitting USB/FTDI errors. Using the open source libftdi (version 0.17) driver I get the following: Error: ftdi_write_data: usb bulk write failed Error: couldn't write MPSSE commands to FT2232 Digging into this some more, I found the error was -110 (ETIMEDOUT). Using instead the libftd2xx binary driver, I got: Error: couldn't read enough bytes from FT2232 device (2 3) Error: couldn't read from FT2232 Giving up on that and going back to the libftdi driver, I found the following mailing list thread, which suggested that the buffer size was too large: http://lists.berlios.de/pipermail/openocd-development/2009-February/004695.html Reducing this buffer size to 768 (from 128K!) did not immediately fix the problem at all. Instead I got: openocd: ft2232.c:403: buffer_write: Assertion `(unsigned) ft2232_buffer_size (unsigned) 768' failed. I dug into the ft2232_large_scan() function and found this line which looks like it attempts to do 64K writes at a time: thisrun_bytes = (num_bytes 65537) ? 65536 : (num_bytes - 1); Changing the 64K in that line to instead use 768 made no difference. I tried 512. This worked; no more assertion. I don't have justification for why I needed 512 here The same logic is in ft2232_add_scan so I guess it works in normal cases. At this stage I got the libftdi driver giving the same error message as the libftd2xx driver: trying to read 3 bytes, only got 2 bytes. This seemed like progress so I moved on to diagnosing this error. I guessed it was just an off-by-one error, and found the following lines, which looked suspicious: if (type != SCAN_OUT) thisrun_read += 1; I commented these two lines out, with again no basis other than guess-work. ft2322_large_scan is broken to the point where I've doubt that it's ever used in practice. I could never get it to run using a CPU target, but with a 27 Mb scan from an SVF file I guess it will. Thanks for the idea, I didn't think of that. From your description it seems your SDR is an inout scan? One problem with ft2232_large_scan is that, as far as I can tell, the data read back is completely discarded. Maybe the SVF code doesn't support TDO verification, because I doubt it would work. Also there's a memory leak, buffer's never getting free'd. The ftdi driver is a real mess. I've been trying to clean in up the past days but I'm beginning to think a complete rewrite would be better. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Errors encountered programming an FPGA using 13MB SVF file
On Fri, Aug 26, 2011 at 8:51 PM, Sam Jansen sam.jan...@starleaf.com wrote: ** On 26/08/11 19:18, Andreas Fritiofson wrote: On Fri, Aug 26, 2011 at 7:10 PM, Sam Jansen sam.jan...@starleaf.comwrote: At this stage I got the libftdi driver giving the same error message as the libftd2xx driver: trying to read 3 bytes, only got 2 bytes. This seemed like progress so I moved on to diagnosing this error. I guessed it was just an off-by-one error, and found the following lines, which looked suspicious: if (type != SCAN_OUT) thisrun_read += 1; I commented these two lines out, with again no basis other than guess-work. ft2322_large_scan is broken to the point where I've doubt that it's ever used in practice. I could never get it to run using a CPU target, but with a 27 Mb scan from an SVF file I guess it will. Thanks for the idea, I didn't think of that. From your description it seems your SDR is an inout scan? One problem with ft2232_large_scan is that, as far as I can tell, the data read back is completely discarded. Maybe the SVF code doesn't support TDO verification, because I doubt it would work. Also there's a memory leak, buffer's never getting free'd. Actually, this SDR is out only; purely sending data to program an FPGA and not expecting anything back. Then I guess the SVF interpreter maps it to an inout scan anyway, which is a waste. Otherwise you wouldn't be helped by commenting out the if (type != SCAN_OUT) ... code. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] fix cross compilation: host libsub was used before
On Mon, Aug 22, 2011 at 9:10 PM, Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com wrote: On 19:35 Tue 09 Aug , Jean-Christophe PLAGNIOL-VILLARD wrote: tested in buildroot any comments? Spelling error: + LDFLAGS=$LFFLAGS $LIBUSB_LDFLAGS I'm pretty sure I've seen some hard coded -lusb added to LDFLAGS somewhere. Probably in the Makefile.am in src. Won't that interfere? Also, does this add a dependency on pkg-config? Can you provide some instructions on how to set up a suitable environment with target libusb for cross compiling? /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] A fix (was Re: Git revision string missing from banner)
On Tue, Aug 16, 2011 at 4:19 PM, Jie Zhang jzhang...@gmail.com wrote: On Mon, Aug 15, 2011 at 6:21 PM, Andreas Fritiofson andreas.fritiof...@gmail.com wrote: +if test -f $srcdir/guess-rev.sh ; then Great, but you should probably check if it's executable instead of just a regular file. I considered if we should check this. But I finally decided it would be better to just check if it's a regular file. my reason is: We decide if we are building for release or not by the existence of guess-rev.sh, not by the existence of an executable guess-rev.sh. So 1. guess-rev.sh exists and is executable: checking regular file is same as checking executable file. 2. guess-rev.sh does not exist: checking regular file is same as checking executable file. 3. guess-rev.sh exists but is not executable: there must be something wrong. If we check regular file, user will be given an error when guess-rev.sh is going to be executed on compiler time. If we check executable file, building for release is assumed and user will notice it only in run time. Agreed, failure is probably the better option if the permissions are botched. I'll leave it as is. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] A fix (was Re: Git revision string missing from banner)
On Mon, Aug 15, 2011 at 5:03 PM, Jie Zhang jzhang...@gmail.com wrote: On Mon, Aug 15, 2011 at 9:35 AM, Spencer Oliver s...@spen-soft.co.uk wrote: On 15 August 2011 14:22, Eric Wetzel thewet...@gmail.com wrote: if test $cross_compiling = no; then # guess-rev.sh only exists in the repository, not in the released archives AC_CHECK_FILE($srcdir/guess-rev.sh, has_guess_rev=yes, has_guess_rev=no) We automatically assume that if we are cross-compiling that we are building a release. This section was last modified in July 2009, back in the SVN days, but the commit is here: http://repo.or.cz/w/openocd.git/commitdiff/00fad24996d6642c6a820cc951c197dddef5734a Actually it was introduced earlier http://repo.or.cz/w/openocd.git/commit/64e53c4fd8a5da944de57716b137a7dff5e67b63 This patch should fix it. Please review and merge if it's OK. Thank you! Jie +if test -f $srcdir/guess-rev.sh ; then Great, but you should probably check if it's executable instead of just a regular file. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Please welcome Andreas Fritiofson as a new OpenOCD maintainer!
Thank you all for the encouraging comments! I've been an OpenOCD user and followed the project several years now and watched it getting better and better. I'm excited to join the maintainer crew, taking a closer part in that positive progress. I'll start by getting acquainted with the procedures and then, as time permits, begin to review, comment and eventually merge your fine contributions. I do have both a day job and three kids so, while I check the mailing list more or less daily, there may be significant delays before I can respond, or find the time to dive deeper into matters. Best regards, Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PULL Request] RLink interface speedup and fixes
The following changes since commit 194e3c5bc5e0fbb7d41cfcbe913c4264782cdd5b: Rodrigo L. Rosa (1): fix tapenabler return code are available in the git repository at: http://repo.or.cz/r/openocd/andreasf.git rlink Andreas Fritiofson (7): rlink: fix indentation errors rlink: more indentation fixes rlink: fix reply counter to enable sending full buffers rlink: remove duplicated code rlink: remove redundant text from log messages rlink: simplify and optimize queue fill level checks rlink: read only the expected number of bytes src/jtag/drivers/rlink.c | 930 ++ 1 files changed, 450 insertions(+), 480 deletions(-) --- Please note that Peter Horn has tested these patches and found that rlink: read only the expected number of bytes causes his build to fail during startup on both linux and windows. His windows build fails during init even before the changed code is run so I think it's unrelated. The linux failure however seems related. I've tried to reproduce using both libusb 0.1 and libusb-compat over libusb 1.0. Both using a standalone RLink dongle and the embedded debug interface in the STM32 Primer (they seem identical however). Haven't seen a problem. Hopefully someone else will take a stab at testing before an eventual merge. A review from someone versed in libusb or rlink internals would be good, too. I've rebased the branch so the problematic patch is last and can be easily dropped. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PULL Request] Flash program speedup through asynchronous algorithms
On Wed, Aug 10, 2011 at 12:01 AM, Spencer Oliver s...@spen-soft.co.ukwrote: On 09/08/2011 22:15, Øyvind Harboe wrote: Any objections? I would like to give this a test-run tomorrow. One observation - other targets that do not yet support the new functions will output a LOG_ERROR to the user. Maybe this should be a LOG_DEBUG as the user will have no idea what it means. Cheers Spen + if (!target-type-start_algorithm) { + LOG_ERROR(Target type '%s' does not support %s, + target_type_name(target), __func__); Are you referring to this? That's in line with the corresponding check in target_run_algorithm(), I think. If some code is executing target_xxx_algorithm() when it isn't supported by the target in question, that's clearly an error, right? Well, I can see a scenario where for example an architecture specific checksum_memory function had been (re-)written to make use of target_start/wait_algorithm, and a target x of that architecture points to that checksum_memory function as well as the architecture's start/wait_algorithm functions. If another target of the same architecture uses the same checksum_memory function but hasn't yet populated the start/wait_algorithm pointers, then the user would get an ERROR when checksumming on that target. On the other hand, maybe that's a good way to make someone update the target's pointers. LOG_ERROR or LOG_DEBUG, I don't care much. It shouldn't happen in the current tree anyway, unless I've overlooked something. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Tags
On Fri, Aug 5, 2011 at 8:56 AM, Øyvind Harboe oyvind.har...@zylin.comwrote: When I run git describe now I get v0.4.0-973-g0d7a948 rather than a v0.5.0-rc2-. Is that intentional? I think it's nice that we stick to v0.4.0- until v0.5.0- goes out of the door. I have no particular opinion, except it should be by choice and not by accident :-) As I posted several times already, it's because the release procedure wasn't followed in creating the rc tags and tarballs. The following basic steps should be followed for a proper rc3 release. There are more things to take in consideration, for example making sure the NEWS file is properly updated and so on. Check the release manual that Zach and David wrote. 1. Manually fix, commit and push the skipped version bumps in configure.in: -AC_INIT([openocd], [0.5.0-dev], +AC_INIT([openocd], [0.5.0-rc3-dev], 2. do a fresh git clone of openocd 3. run tools/release.sh --next rc release 4. verify and publish archives/* 5. merge v0.5.0-rc4-dev into master and push it After this, git describe correctly reflects that we're in rc phase: v0.5.0-rc3-1-gbcded6a And version strings should be correct (didn't test): 0.5.0-rc3 if built from tarball, 0.5.0-rc4-dev if built from git HEAD. I think we really need to do this, like, right now, then wait for feedback from people building the released rc3 tarballs (a week tops), then do the final 0.5.0 release. No patches accepted except build and packaging fixes after rc3 is out. Final 0.5.0 release is very similar: 1. do a fresh git clone of openocd 2. run tools/release.sh --next minor --final release 3. verify and publish archives/* 4. merge v0.6.0-dev into master and push it /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Tags
On Fri, Aug 5, 2011 at 11:02 AM, Spencer Oliver s...@spen-soft.co.ukwrote: On 5 August 2011 09:58, Andreas Fritiofson andreas.fritiof...@gmail.com wrote: On Fri, Aug 5, 2011 at 8:56 AM, Øyvind Harboe oyvind.har...@zylin.com wrote: When I run git describe now I get v0.4.0-973-g0d7a948 rather than a v0.5.0-rc2-. Is that intentional? I think it's nice that we stick to v0.4.0- until v0.5.0- goes out of the door. I have no particular opinion, except it should be by choice and not by accident :-) As I posted several times already, it's because the release procedure wasn't followed in creating the rc tags and tarballs. I will agree that the release process has not been followed with regards to tarballs. However this is not the cause of Øyvind query - please see my previous email. Release tags are annotated, and so take priority with git describe. Ok, but if the release script would have been used, the v0.5.0-rc* tags would have been annotated. And they really should be, right? That's what the script does, the 0.4.0 and 0.3.0 rc tags were annotated, and it corresponds with Øyvind's initial expectation of a v0.5.0-rc2- output from git describe. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Tags
On Fri, Aug 5, 2011 at 11:26 AM, Spencer Oliver s...@spen-soft.co.ukwrote: Release tags are annotated, but not rc tags Oh, but they are, or am I completely oblivious of git tags (quite possible)? $ git checkout v0.4.0-rc2~2 $ git describe v0.4.0-rc1-193-g747a607 http://repo.or.cz/w/openocd.git/tag/cd8ad2e961d3476ddfad3353390ce99a4872bdf1 http://repo.or.cz/w/openocd.git/tag/f74fdc5e4afc94b65cba4b4c357f0e67cc9d185a And why shouldn't they be? /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] Automatically generate ChangeLog from git log for release tarball
On Wed, Aug 3, 2011 at 12:28 PM, Luca Bruno lu...@debian.org wrote: Andreas Fritiofson scrisse: make dist should use git2cl to genereate ChangeLog from git history, populating the placeholder file in released tarball. Still not working for srcdir != builddir make[1]: Entering directory `/home/soliver/openocd/openocd-rel' if test -d ../openocd/.git -a \( ! -e openocd-0.5.0-dev/ChangeLog -o -w openocd-0.5.0-dev/ChangeLog \) ; then \ ../openocd/tools/git2cl/git2cl openocd-0.5.0-dev/ChangeLog ; \ fi fatal: Not a git repository (or any of the parent directories): .git Current directory needs to be $(srcdir) before running git2cl. I suspect this will get tricky, as $(distdir) is relative to $(builddir). If that's always the case it's not that tricky: cd $(srcdir); tools/git2cl/git2cl $(builddir)/$(distdir)/ChangeLog If it's not necessarily relative, this should work better: cd $(srcdir); tools/git2cl/git2cl $(abspath $(distdir)/ChangeLog) Although abspath requires GNU Make 3.81. If the pipe method work, fine. However, looking through the git2cl code it apparently assumes git log is run with LC_ALL=C and that wouldn't be the case with the latest incarnation of the patch. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Last call before release
On Wed, Aug 3, 2011 at 3:32 PM, Tomek CEDRO tomek.ce...@gmail.com wrote: Hello Jean, please provide full featured RC package (generated by make distcheck, containing configure scripts) to test it on FreeBSD, as for now bootstrap fails, so the status is no-go. Best regards, Tomek Yes, we haven't had a chance to iron out bugs in the release procedure so I'd say we need a proper RC release before 0.5.0. But there's more to it than make distcheck. At this point I think it would result in wrong version strings in the binary. We have a script, tools/release.sh, that (when used correctly) patches configure.in with the correct version string, builds source packages, creates the proper tags, prepares the repository for the next release cycle and whatnot. Please use that. Some manual steps may be needed too: http://openocd.berlios.de/doc/doxygen/html/releases.html#releasehow Check the git log to see how the 0.4.0 release and preceding RCs were handled. Right now, the repository looks nothing like that. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [OpenOCD][PULL Request][MIPS32] Fixed single byte memory write
On Sat, Jul 30, 2011 at 4:26 AM, Mahr, Stefan stefan.m...@sphairon.comwrote: Hi Drasko. Drasko DRASKOVIC (1): mips32 : Fixed memory byte access Your patch fixes the broken byte access, but not the big endian host issue. Since both problems are tied together, I would prefer a more common solution. Attached patch hopefully fixes both issues. It should also fix alignment warning that Øyvind had reported. Unfortunately I can't test it for the time being. +void target_buffer_get_u32_array(struct target *target, const uint8_t *buffer, uint32_t count, uint32_t *dstbuf); +void target_buffer_get_u16_array(struct target *target, const uint8_t *buffer, uint32_t count, uint16_t *dstbuf); +void target_buffer_set_u32_array(struct target *target, uint8_t *buffer, uint32_t count, uint32_t *srcbuf); +void target_buffer_set_u16_array(struct target *target, uint8_t *buffer, uint32_t count, uint16_t *srcbuf); Great, these helper functions are on my todo list. There are several other places where they are useful. Comments for the get functions are broken, though. /* mips32_..._write_mem with size 4/2 requires uint32_t/uint16_t in host */ /* endianness, but byte array represents target endianness */ /* mips32_pracc_fastdata_xfer requires uint32_t in host endianness, */ /* but byte array represents target endianness */ Those comments suggest to me that the mips32_..._write_mem and mips32_pracc_fastdata_xfer functions are the ones that are broken (by design?). If the byte array already is in target endianness, why on earth do we need to swap it back and forth? Why can't mips32_pracc_write_mem8/16/32 simply copy the already properly formatted memory block to the target, using load/store byte for the -8 version, load/store halfword for the -16 version and load/store word for the -32 version? Looking at the -32 version, the memory block (that with your patch has been swapped to host order) is simply passed on to mips32_pracc_exec(), which I assume perform the actual transfer. Where in the following process is the data swapped *back* to target order? BTW, I'm puzzled by the naming here. It seems mips32_pracc_exec_read is used for transfers host-target and mips32_pracc_exec_write for target-host? It seems to me that the endianness handling in the MIPS code is very much more complex than it would need to be. Maybe the root cause of the complexity is that the mips_ejtag scan functions are handling arrays of uint32_ts when they should be handling arrays of bits (which in OpenOCD is represented by a LSBit-filled-first array of bytes)? That approach forces *host* endianness and alignment considerations far down into the low level code where they don't belong. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] OpenOCD 0.5.0-rc2 release
On Sat, Jul 30, 2011 at 7:10 PM, Tomek CEDRO tomek.ce...@gmail.com wrote: Hello! :-) Trying to bootstrap on FreeBSD 8.2-RELEASE fails with the 0.5.0-rc2 as given at: http://openocd.git.sourceforge.net/git/gitweb.cgi?p=openocd/openocd;a=snapshot;h=d4cd6f032015552f00bf4b5a90f25f5f958e9d9e;sf=tgz This should probably never happen with the real package as configure scripts are already there and no need for bootstrapping..? The released RC-packages are broken because the step-by-step guide for producing releases wasn't followed. There is a script that automates most, if not all of it so I don't understand why this was skipped. Now we have two broken RCs and some manual fixing to do to make the release script get back on track. Also we've missed two chances of catching possible release procedure bugs, due to for example including jimtcl as a submodule. Release manager, please follow http://openocd.berlios.de/doc/doxygen/html/releases.html and use the release script for all releases, including RCs. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH 3/4] cfg: update scripts to use new stm32 driver names
On Thu, Jul 28, 2011 at 1:52 PM, Spencer Oliver s...@spen-soft.co.ukwrote: delete mode 100644 tcl/target/stm32.cfg delete mode 100644 tcl/target/stm32f2xxx.cfg For compatibility with existing user scripts, it would be best to keep these files during the 0.5.x period. Just make them print a deprecation warning and then source the renamed files. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH 3/4] cfg: update scripts to use new stm32 driver names
On Fri, Jul 29, 2011 at 4:09 PM, Spencer Oliver s...@spen-soft.co.ukwrote: On 29 July 2011 14:54, Andreas Fritiofson andreas.fritiof...@gmail.com wrote: On Thu, Jul 28, 2011 at 1:52 PM, Spencer Oliver s...@spen-soft.co.uk wrote: delete mode 100644 tcl/target/stm32.cfg delete mode 100644 tcl/target/stm32f2xxx.cfg see patch 4 Spen That one won't help for all scripts out there that are currently sourcing target/stm32.cfg. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH 3/4] cfg: update scripts to use new stm32 driver names
On Fri, Jul 29, 2011 at 5:38 PM, Spencer Oliver s...@spen-soft.co.ukwrote: On 29 July 2011 15:43, Andreas Fritiofson andreas.fritiof...@gmail.com wrote: That one won't help for all scripts out there that are currently sourcing target/stm32.cfg. /Andreas Good point - how about http://repo.or.cz/w/openocd/ntfreak.git/commit/e4908b71bcac1e3ae01a4f54cffe475b0be6e336 stm32f2xxx.cfg sources itself. Other than that, it should do the trick. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] write_image hangs on lpc2138
On Tue, Jul 26, 2011 at 11:25 PM, Steve Franks bahamasfra...@gmail.comwrote: Every year or so I try openocd, and still no joy. Actually, seems the script and config stuff has all finally matured/stabilized, so I'll assume I can make it work w/o too much pain, after I fix this hang: I have a vanilla lpc2138 on an ARM-USB-OCD, that seems to fire up ok using the built-in configs in /usr/share/openocd. Hangs forever on write_image: steve@dystant:~/projects/clients/zonge/DNT/NT-32/firmware/arm/main$ openocd -f openocd.cfg Open On-Chip Debugger 0.4.0 (2010-10-08-15:42) Licensed under GNU GPL v2 For bug reports, read http://openocd.berlios.de/doc/doxygen/bugs.html RCLK - adaptive jtag_nsrst_delay: 200 jtag_ntrst_delay: 200 srst_only srst_pulls_trst srst_gates_jtag srst_open_drain Info : RCLK (adaptive clock speed) not supported - fallback to 500 kHz Info : JTAG tap: lpc2138.cpu tap/device found: 0x4f1f0f0f (mfg: 0x787, part: 0xf1f0, ver: 0x4) Info : Embedded ICE version 4 Info : lpc2138.cpu: hardware has 2 breakpoint/watchpoint units flash write_image erase unlock Main.elf 0 Two things: 0.4.0 is rather old, you may be seeing a bug that has already been fixed. Grab the latest version from the repo and give it a try. I don't think anyone will bother with a bug report against 0.4.0 at this point. Post a debug log (add -d to the command line) so we can see what's going on under the hood. Regards, Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] write_image hangs on lpc2138
On Wed, Jul 27, 2011 at 12:22 AM, Steve Franks bahamasfra...@gmail.comwrote: On Tue, Jul 26, 2011 at 3:06 PM, Andreas Fritiofson andreas.fritiof...@gmail.com wrote: 0.4.0 is rather old, you may be seeing a bug that has already been fixed. Grab the latest version from the repo and give it a try. I don't think anyone will bother with a bug report against 0.4.0 at this point. Is there a reason ubuntu has a rather old version in their primary repositories? Just not as 'sexy' as libreoffice, so no one bothers to build it? Pretty much so. Plus the fact that 0.4.0 really is the latest release and packagers are unwilling to use snapshots. Long time between releases isn't the best thing for a fairly active project like this. But I hear that's going to change (but again, where's 0.5.0-rc3?). Post a debug log (add -d to the command line) so we can see what's going on under the hood. with -d I guess it's not hung, it's just silently ignoring the command Debug: 427 19156 target.c:968 target_call_event_callbacks(): target event 2 (gdb-halt) flash write_image erase unlock Main.elf 0Debug: 428 19256 target.c:968 target_call_event_callbacks(): target event 2 (gdb-halt) Debug: 429 19357 target.c:968 target_call_event_callbacks(): target event 2 (gdb-halt) Yes, totally ignored. Where are you issuing the flash write_image command? Not to standard input, right? That won't work. Use the command line (-c init -c reset init -c flash write_image erase unlock Main.elf), an extra config file, the telnet server on port for interactive use, or just load from within gdb. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH 0/7] RLink interface speedup, and fixes
On Fri, Jul 22, 2011 at 12:29 AM, Peter Horn peter.h...@bluewin.ch wrote: Am 19.07.2011 22:43, schrieb Peter Horn: Last time I've used OpenOCD with RLink stepping was awfully slow, about 3 seconds or more per step. This was some weeks ago and on Windows / Mingw. On Linux, stepping is considerably faster both with and without your patches, so I have to try the current version again on Windows. Stepping is still slow on Windows. Stepping speed is not affected much by these patches since it depends largely on latency, not throughput. These patches fixes a throughput problem. Cortex-M3 stepping is really dependent on latency because, for some reason, each register is read atomically upon debug entry, plus one atomic restore of a debug register (for DCC purposes) per register. Just reading core registers takes ~0.3 s on my linux system using the primer. I have now also applied the RLink RLink interface speedup and fixes patch series and run into some problems: On Linux with the standalone RLink I get: Open On-Chip Debugger 0.5.0-dev-00959-gd6c42bf-dirty (2011-07-21-23:47) Licensed under GNU GPL v2 For bug reports, read http://openocd.berlios.de/doc/**doxygen/bugs.htmlhttp://openocd.berlios.de/doc/doxygen/bugs.html Warn : Adapter driver 'rlink' did not declare which transports it allows; assuming legacy JTAG-only Info : only one transport option; autoselect 'jtag' 1000 kHz adapter_nsrst_delay: 100 jtag_ntrst_delay: 100 cortex_m3 reset_config sysresetreq Info : clock speed 375 kHz Error: Read of endpoint 2 returned -75, expected 17 Error: dtc_run_download: Value too large for defined data type Patches 1/7 to 4/7 work ok, the problem is caused by patch 5/7 On Windows using the embedded RLink, OpenOCD: Open On-Chip Debugger 0.5.0-dev-snapshot (2011-07-21-07:10) Licensed under GNU GPL v2 For bug reports, read http://openocd.berlios.de/doc/**doxygen/bugs.htmlhttp://openocd.berlios.de/doc/doxygen/bugs.html Warn : Adapter driver 'rlink' did not declare which transports it allows; assuming legacy JTAG-only Info : only one transport option; autoselect 'jtag' 1000 kHz adapter_nsrst_delay: 100 jtag_ntrst_delay: 100 cortex_m3 reset_config sysresetreq Error: USB read error: libusb0-dll:err [_usb_reap_async] timeout error in procedure 'init' Can you post debug logs (add -d on command line)? There seems to be an known issue with libusb-win32: http://permalink.gmane.org/**gmane.comp.lib.libusb.devel.**windows/505http://permalink.gmane.org/gmane.comp.lib.libusb.devel.windows/505 BTW I'm using libusb-win32 1.2.4.0 Peter ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH 0/7] RLink interface speedup, and fixes
On Fri, Jul 22, 2011 at 11:59 AM, Peter Horn peter.h...@bluewin.ch wrote: Patches 1/7 to 4/7 work ok, the problem is caused by patch 5/7 On Windows using the embedded RLink, OpenOCD: Open On-Chip Debugger 0.5.0-dev-snapshot (2011-07-21-07:10) Licensed under GNU GPL v2 For bug reports, read http://openocd.berlios.de/doc/**__doxygen/bugs.htmlhttp://openocd.berlios.de/doc/__doxygen/bugs.html http://openocd.berlios.de/**doc/doxygen/bugs.htmlhttp://openocd.berlios.de/doc/doxygen/bugs.html Warn : Adapter driver 'rlink' did not declare which transports it allows; assuming legacy JTAG-only Info : only one transport option; autoselect 'jtag' 1000 kHz adapter_nsrst_delay: 100 jtag_ntrst_delay: 100 cortex_m3 reset_config sysresetreq Error: USB read error: libusb0-dll:err [_usb_reap_async] timeout error in procedure 'init' At the moment I can provide you the debug log on Windows. Linux log will follow. snip Debug: 188 125 rlink.c:1565 rlink_init(): Opened device, pHDev = 00eb3560 Debug: 189 125 rlink.c:1585 rlink_init(): interface claimed! Error: 190 407 rlink.c:1628 rlink_init(): USB read error: libusb0-dll:err [_usb_reap_async] timeout error Debug: 191 407 command.c:638 run_command(): Command failed with error code -4 User : 192 407 command.c:679 command_run_line(): in procedure 'init' Peter I don't see how this error could be caused by my patches. If I'm not mistaken, none of them have run at this point. It fails during init, and that code is unchanged with the exception of some indentation fixes and AFAICT they're OK. Can you reverify with unpatched tree? /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Problem with OpenOCD 0.5.0-rc1 and STM32 verify_image
On Fri, Jul 22, 2011 at 4:57 PM, Freddie Chopin freddie_cho...@op.plwrote: On 2011-07-21 23:02, Andreas Fritiofson wrote: However I can't explain Freddie's reproduced failures. Maybe post the hex files (matching, slightly different and very different) and I can see if I can take a closer look at it? I attach three .hex files (compressed). From what I see, there's no info about anything in RAM... According to objdump, the completely different hex file is located in RAM: arm-none-eabi-objdump -x verify_image/original.hex verify_image/almost_the_same.hex verify_image/completely_different.hex verify_image/original.hex: file format ihex verify_image/original.hex architecture: UNKNOWN!, flags 0x: start address 0x08000131 Sections: Idx Name Size VMA LMA File off Algn 0 .sec1 74c0 0800 0800 0011 2**0 CONTENTS, ALLOC, LOAD SYMBOL TABLE: no symbols verify_image/almost_the_same.hex: file format ihex verify_image/almost_the_same.hex architecture: UNKNOWN!, flags 0x: start address 0x08000131 Sections: Idx Name Size VMA LMA File off Algn 0 .sec1 74c0 0800 0800 0011 2**0 CONTENTS, ALLOC, LOAD SYMBOL TABLE: no symbols verify_image/completely_different.hex: file format ihex verify_image/completely_different.hex architecture: UNKNOWN!, flags 0x: start address 0x2131 Sections: Idx Name Size VMA LMA File off Algn 0 .sec1 0324 2000 2000 0011 2**0 CONTENTS, ALLOC, LOAD SYMBOL TABLE: no symbols ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH 0/7] RLink interface speedup, and fixes
On Fri, Jul 22, 2011 at 2:55 PM, Peter Horn peter.h...@bluewin.ch wrote: Am 22.07.2011 13:36, schrieb Andreas Fritiofson: I don't see how this error could be caused by my patches. If I'm not mistaken, none of them have run at this point. It fails during init, and that code is unchanged with the exception of some indentation fixes and AFAICT they're OK. Can you reverify with unpatched tree? /Andreas Hi Andreas I did: $ cp openocd-rlink-patched openocd $ cd openocd $ git reset --hard HEAD $ make clean $ make all $ src/openocd.exe -d -s tcl -f interface/rlink.cfg -f board/stm3210c_eval.cfg This yields: Open On-Chip Debugger 0.5.0-dev-snapshot (2011-07-22-12:47) snip Debug: 188 125 rlink.c:1595 rlink_init(): Opened device, pHDev = 00ebf528 Debug: 189 125 rlink.c:1615 rlink_init(): interface claimed! Debug: 190 203 rlink.c:1661 rlink_init(): RLink firmware version: 0.0.3 Your RLink reports the same firmware version as my STM32Primer does. If that means they are identical, I should be able to reproduce, but I can't. Maybe it depends on the version of libusb? Also I don't have a Windows machine to test on. Regardless, I still can't understand how my patches can make it fail as early as during init, though. I see now that the linux log hints that it doesn't fail in the same place. Do you have a debug log for that too? Debug: 279 2250 rlink.c:507 dtc_run_download(): : 55/22 Debug: 280 2359 rlink.c:507 dtc_run_download(): : 55/22 the last few lines continue forever. That's OpenOCD polling the target. /Andreas /*** * Copyright (C) 2005 by Dominic Rath* * dominic.r...@gmx.de * * * * Copyright (C) 2007,2008 Øyvind Harboe * * oyvind.har...@zylin.com * * * * Copyright (C) 2008 Rob Brown, Lou Deluxe * * r...@cobbleware.com, lou.openocd...@fixit.nospammail.net * * * * 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 * * (at your option) any later version. * * * * This program is distributed in the hope that it will be useful, * * but WITHOUT ANY WARRANTY; without even the implied warranty of* * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * * GNU General Public License for more details. * * * * You should have received a copy of the GNU General Public License * * along with this program; if not, write to the * * Free Software Foundation, Inc., * * 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. * ***/ #ifdef HAVE_CONFIG_H #include config.h #endif /* project specific includes */ #include jtag/interface.h #include jtag/commands.h #include rlink.h #include rlink_st7.h #include rlink_ep1_cmd.h #include rlink_dtc_cmd.h #include usb_common.h /* This feature is made useless by running the DTC all the time. When automatic, the LED is on whenever the DTC is running. Otherwise, USB messages are sent to turn it on and off. */ #undef AUTOMATIC_BUSY_LED /* This feature may require derating the speed due to reduced hold time. */ #undef USE_HARDWARE_SHIFTER_FOR_TMS #define INTERFACE_NAME RLink #define USB_IDVENDOR (0x138e) #define USB_IDPRODUCT (0x9000) #define USB_EP1OUT_ADDR (0x01) #define USB_EP1OUT_SIZE (16) #define USB_EP1IN_ADDR (USB_EP1OUT_ADDR | 0x80) #define USB_EP1IN_SIZE (USB_EP1OUT_SIZE) #define USB_EP2OUT_ADDR (0x02) #define USB_EP2OUT_SIZE (64) #define USB_EP2IN_ADDR (USB_EP2OUT_ADDR | 0x80) #define USB_EP2IN_SIZE (USB_EP2OUT_SIZE) #define USB_EP2BANK_SIZE (512) #define USB_TIMEOUT_MS (3 * 1000) #define DTC_STATUS_POLL_BYTE (ST7_USB_BUF_EP0OUT + 0xff) #define ST7_PD_NBUSY_LED ST7_PD0 #define ST7_PD_NRUN_LED ST7_PD1 /* low enables VPP at adapter header, high connects it to GND instead */ #define ST7_PD_VPP_SEL ST7_PD6 /* low: VPP = 12v, high: VPP = 5v */ #define ST7_PD_VPP_SHDN ST7_PD7 /* These pins are connected together */ #define ST7_PE_ADAPTER_SENSE_IN ST7_PE3 #define ST7_PE_ADAPTER_SENSE_OUT ST7_PE4 /* Symbolic mapping between port
Re: [Openocd-development] SRST TRST have to be buffered?
On Fri, Jul 8, 2011 at 1:19 AM, Matthew Lai cyberf...@wecheer.com wrote: Hello! I'm trying to embed a FT2232D based programmer into my board with a STM32 (Cortex-M3 MCU). I want the programmer to be compatible with jtagkey, so I looked at schematics of compaible designs. I noticed that while the JTAG signals (TCK, TDI, TDO, TMS) are only buffered when translation is necessary, the SRST and TRST signals are always tri-state buffered, with OE going into the FTDI chip. Is there a reason for that? Can I omit the buffers? Maybe you've already got the answers you needed, but a clarification may be in order: It's important that SRST is open-drain because it is a bidirectional signal. The STM32 drives it low when it is reset internally. If it's driven high, reset may not work and the chip may possibly even take damage. Also, I heard it's possible to omit the TRST signal and only keep the SRST signal, because system reset will also reset the TAP controller. Is that true? Are there problems with that? TAP or debug logic is not reset by SRST, but you can skip TRST anyway because TAP reset can be achieved by clocking in a pattern on TMS, which OpenOCD does if no TRST is available. I'm not aware of any problems, we have made several STM32 designs without TRST. Even the SRST is optional, it can be asserted via debug logic, but it's good to have because it may be the only way to reset the chip (other than POR) if you're stuck in low power mode (JTAG unresponsive). /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Problem with OpenOCD 0.5.0-rc1 and STM32 verify_image
On Thu, Jul 21, 2011 at 10:44 PM, Magnus S. mankk...@yahoo.se wrote: Freddie Chopin freddie_chopin@... writes: I can partially confirm... Tested with 0.5.0-rc1 Comparing with right image works fine: Comparing with almost right image (compilation date inside has changed) works fine too: However... when checking against something completely wrong (almost completely different code) strange things happen. When checking against bin, everything is as expected, but when checking against hex or elf: If you specify address it is treated as an offset from beginning of RAM... So sometimes OpenOCD starts the check in completely wrong location... 4\/3!! Hi Freddie, Thank you so much for your speedy reply! Indeed, this confirms my suspicions. I gather from your input, that if I switch to verifying using .bin format, based on the .text sections of the ELF file, then this might be an adequate workaround. A quick test I did was able to successfully verify the .bin format. Do I need to forward this problem description to some other mailing list / issue tracking system in order for it to be taken into consideration for future releases? Again, thanks heaps for helping me out in this issue! Best regards, Looking at your log (first one, you shouldn't specify an offset when using elf so the errors in the 2nd are expected) I don't think it's obvious that there's a bug. Rather it seems that you have a section in your elf file located at 0x2000 (ram) and openocd dutifully attempts to compare it and naturally fails. The first section, placed at 0x0800 checksums OK and it doesn't fail until the ram section is checksummed. Can you examine your elf-file and see if and why there is section in ram? Linker script bug? However I can't explain Freddie's reproduced failures. Maybe post the hex files (matching, slightly different and very different) and I can see if I can take a closer look at it? /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] Automatically generate ChangeLog from git log for release tarball
On Mon, Jul 18, 2011 at 3:46 PM, Spencer Oliver s...@spen-soft.co.ukwrote: On 17 July 2011 10:18, Luca Bruno lu...@debian.org wrote: make dist should use git2cl to genereate ChangeLog from git history, populating the placeholder file in released tarball. Still not working for srcdir != builddir make[1]: Entering directory `/home/soliver/openocd/openocd-rel' if test -d ../openocd/.git -a \( ! -e openocd-0.5.0-dev/ChangeLog -o -w openocd-0.5.0-dev/ChangeLog \) ; then \ ../openocd/tools/git2cl/git2cl openocd-0.5.0-dev/ChangeLog ; \ fi fatal: Not a git repository (or any of the parent directories): .git Current directory needs to be $(srcdir) before running git2cl. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH 0/7] RLink interface speedup and fixes
This patch set fixes some general problems in the RLink interface driver. Most importantly it fixes a performance bug that have been causing decreased throughput. Speed test on a STM32 Primer (STM32F103 platform with built in RLink) with the following openocd.cfg --- source [find interface/rlink.cfg] source [find target/stm32.cfg] $_TARGETNAME configure -event reset-init { mww 0x40022000 0x32 mww 0x40021004 0x3c0400 mww 0x40021000 0x01000883 sleep 10 mww 0x40021004 0x3c0402 sleep 10 adapter_khz 3000 } init reset init poll off stm32x mass_erase 0 flash write_image test.bin 0x0800 verify_image test.bin 0x0800 shutdown --- Before: wrote 65536 bytes from file test.bin in 8.316117s (7.696 KiB/s) verified 65536 bytes in 0.748795s (85.471 KiB/s) After: wrote 65536 bytes from file test.bin in 7.171075s (8.925 KiB/s) verified 65536 bytes in 0.748815s (85.468 KiB/s) /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH 1/7] rlink: fix indentation errors
Indentation was inconsistent and some lines not indented at all. Quickfix using Eclipse's auto-indentation. Signed-off-by: Andreas Fritiofson andreas.fritiof...@gmail.com --- src/jtag/drivers/rlink.c | 726 +++--- 1 files changed, 363 insertions(+), 363 deletions(-) diff --git a/src/jtag/drivers/rlink.c b/src/jtag/drivers/rlink.c index 5f53dbc..7111a20 100644 --- a/src/jtag/drivers/rlink.c +++ b/src/jtag/drivers/rlink.c @@ -110,9 +110,9 @@ static usb_dev_handle *pHDev; static int ep1_generic_commandl( - usb_dev_handle *pHDev_param, - size_t length, - ... + usb_dev_handle *pHDev_param, + size_t length, + ... ) { uint8_t usb_buffer[USB_EP1OUT_SIZE]; uint8_t *usb_buffer_p; @@ -132,17 +132,17 @@ ep1_generic_commandl( } memset( - usb_buffer_p, - 0, - sizeof(usb_buffer) - (usb_buffer_p - usb_buffer) -); + usb_buffer_p, + 0, + sizeof(usb_buffer) - (usb_buffer_p - usb_buffer) + ); usb_ret = usb_bulk_write( - pHDev_param, - USB_EP1OUT_ADDR, - (char *)usb_buffer, sizeof(usb_buffer), - USB_TIMEOUT_MS -); + pHDev_param, + USB_EP1OUT_ADDR, + (char *)usb_buffer, sizeof(usb_buffer), + USB_TIMEOUT_MS + ); return(usb_ret); } @@ -153,10 +153,10 @@ ep1_generic_commandl( static ssize_t ep1_memory_read( - usb_dev_handle *pHDev, - uint16_taddr, - size_t length, - uint8_t *buffer + usb_dev_handle *pHDev, + uint16_taddr, + size_t length, + uint8_t *buffer ) { uint8_t usb_buffer[USB_EP1OUT_SIZE]; int usb_ret; @@ -165,10 +165,10 @@ ep1_memory_read( usb_buffer[0] = EP1_CMD_MEMORY_READ; memset( - usb_buffer + 4, - 0, - sizeof(usb_buffer) - 4 -); + usb_buffer + 4, + 0, + sizeof(usb_buffer) - 4 + ); remain = length; count = 0; @@ -184,21 +184,21 @@ ep1_memory_read( usb_buffer[2] = addr; usb_buffer[3] = length; - usb_ret = usb_bulk_write( - pHDev, USB_EP1OUT_ADDR, - usb_buffer, sizeof(usb_buffer), - USB_TIMEOUT_MS -); + usb_ret = usb_bulk_write( + pHDev, USB_EP1OUT_ADDR, + usb_buffer, sizeof(usb_buffer), + USB_TIMEOUT_MS + ); if (usb_ret sizeof(usb_buffer)) { break; } usb_ret = usb_bulk_read( - pHDev, USB_EP1IN_ADDR, - buffer, length, - USB_TIMEOUT_MS -); + pHDev, USB_EP1IN_ADDR, + buffer, length, + USB_TIMEOUT_MS + ); if (usb_ret length) { break; @@ -219,10 +219,10 @@ ep1_memory_read( static ssize_t ep1_memory_write( - usb_dev_handle *pHDev_param, - uint16_taddr, - size_t length, - uint8_t const *buffer + usb_dev_handle *pHDev_param, + uint16_taddr, + size_t length, + uint8_t const *buffer ) { uint8_t usb_buffer[USB_EP1OUT_SIZE]; int usb_ret; @@ -245,21 +245,21 @@ ep1_memory_write( usb_buffer[2] = addr; usb_buffer[3] = length; memcpy( - usb_buffer + 4, - buffer, - length -); + usb_buffer + 4, + buffer, + length + ); memset( - usb_buffer + 4 + length, - 0, - sizeof(usb_buffer) - 4 - length -); + usb_buffer + 4 + length, + 0, + sizeof(usb_buffer) - 4 - length + ); - usb_ret = usb_bulk_write( - pHDev_param, USB_EP1OUT_ADDR, - (char *)usb_buffer, sizeof(usb_buffer), - USB_TIMEOUT_MS -); + usb_ret = usb_bulk_write( + pHDev_param, USB_EP1OUT_ADDR
[Openocd-development] [PATCH 2/7] rlink: more indentation fixes
Remove unnecessary block scopes to reduce indentation level. Signed-off-by: Andreas Fritiofson andreas.fritiof...@gmail.com --- src/jtag/drivers/rlink.c | 203 +++--- 1 files changed, 100 insertions(+), 103 deletions(-) diff --git a/src/jtag/drivers/rlink.c b/src/jtag/drivers/rlink.c index 7111a20..ae3431b 100644 --- a/src/jtag/drivers/rlink.c +++ b/src/jtag/drivers/rlink.c @@ -702,102 +702,101 @@ dtc_queue_run(void) { if (usb_err 0) { LOG_ERROR(dtc_run_download: %s, usb_strerror()); exit(1); - } else { - /* process the reply, which empties the reply queue and frees its entries */ - dtc_p = reply_buffer; + } - /* The rigamarole with the masks and doing it bit-by-bit is due to the fact that the scan buffer is LSb-first and the DTC code is MSb-first for hardware reasons. It was that or craft a function to do the reversal, and that wouldn't work with bit-stuffing (supplying extra bits to use mostly byte operations), or any other scheme which would throw the byte alignment off. */ + /* process the reply, which empties the reply queue and frees its entries */ + dtc_p = reply_buffer; - for ( - rq_p = dtc_queue.rq_head; - rq_p != NULL; - rq_p = rq_next - ) { - tdo_p = rq_p-scan.buffer + (rq_p-scan.offset / 8); - tdo_mask = 1 (rq_p-scan.offset % 8); + /* The rigamarole with the masks and doing it bit-by-bit is due to the fact that the scan buffer is LSb-first and the DTC code is MSb-first for hardware reasons. It was that or craft a function to do the reversal, and that wouldn't work with bit-stuffing (supplying extra bits to use mostly byte operations), or any other scheme which would throw the byte alignment off. */ + for ( + rq_p = dtc_queue.rq_head; + rq_p != NULL; + rq_p = rq_next + ) { + tdo_p = rq_p-scan.buffer + (rq_p-scan.offset / 8); + tdo_mask = 1 (rq_p-scan.offset % 8); - bit_cnt = rq_p-scan.length; - if (bit_cnt = 8) { - /* bytes */ - dtc_mask = 1 (8 - 1); + bit_cnt = rq_p-scan.length; + if (bit_cnt = 8) { + /* bytes */ - for ( - ; - bit_cnt; - bit_cnt-- - ) { - if (*dtc_p dtc_mask) { - *tdo_p |= tdo_mask; - } else { - *tdo_p =~ tdo_mask; - } - - dtc_mask = 1; - if (dtc_mask == 0) { - dtc_p++; - dtc_mask = 1 (8 - 1); - } - - tdo_mask = 1; - if (tdo_mask == 0) { - tdo_p++; - tdo_mask = 1; - } - } - } else { - /* extra bits or last bit */ - - x = *dtc_p++; - if (( - rq_p-scan.type == SCAN_IN - ) ( - rq_p-scan.offset != rq_p-scan.size - 1 - )) { - /* extra bits were sent as a full byte with padding on the end */ - dtc_mask = 1 (8 - 1); + dtc_mask = 1 (8 - 1); + + for ( + ; + bit_cnt
[Openocd-development] [PATCH 3/7] rlink: fix reply counter to enable sending full buffers
dtc_queue.reply_index was wrongly being increased during out scans, causing the queue to be sent before the out buffer was full. This patch increases raw upload speed by 50% or so. Signed-off-by: Andreas Fritiofson andreas.fritiof...@gmail.com --- src/jtag/drivers/rlink.c | 16 +--- 1 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/jtag/drivers/rlink.c b/src/jtag/drivers/rlink.c index ae3431b..d0b18b1 100644 --- a/src/jtag/drivers/rlink.c +++ b/src/jtag/drivers/rlink.c @@ -678,6 +678,10 @@ dtc_queue_run(void) { uint8_t dtc_mask, tdo_mask; uint8_t reply_buffer[USB_EP2IN_SIZE]; + assert((dtc_queue.rq_head != 0) == (dtc_queue.reply_index 0)); + assert(dtc_queue.cmd_index USB_EP2BANK_SIZE); + assert(dtc_queue.reply_index = USB_EP2IN_SIZE); + retval = ERROR_OK; if (dtc_queue.cmd_index 1) return(retval); @@ -807,8 +811,6 @@ dtc_queue_run(void) { return(retval); } - - static int tap_state_queue_init(void) { @@ -1232,6 +1234,7 @@ rlink_scan( LOG_ERROR(enqueuing DTC reply entry: %s, strerror(errno)); exit(1); } + dtc_queue.reply_index += (chunk_bits + 7) / 8; tdi_bit_offset += chunk_bits; } @@ -1264,7 +1267,6 @@ rlink_scan( dtc_mask = 1; if (dtc_mask == 0) { dtc_queue.cmd_buffer[dtc_queue.cmd_index++] = x; - dtc_queue.reply_index++; x = 0; dtc_mask = 1 (8 - 1); } @@ -1298,6 +1300,8 @@ rlink_scan( exit(1); } + dtc_queue.reply_index++; + tdi_bit_offset += extra_bits; if (type == SCAN_IN) { @@ -1327,8 +1331,6 @@ rlink_scan( dtc_queue.cmd_buffer[dtc_queue.cmd_index++] = x; } - - dtc_queue.reply_index++; } /* Schedule the last bit into the DTC command buffer */ @@ -1355,10 +1357,10 @@ rlink_scan( exit(1); } + dtc_queue.reply_index++; + dtc_queue.cmd_buffer[dtc_queue.cmd_index++] = DTC_CMD_SHIFT_TMS_TDI_BIT_PAIR(1, (*tdi_p tdi_mask), 1); - - dtc_queue.reply_index++; } /* Move to pause state */ -- 1.7.4.1 ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH 4/7] rlink: remove duplicated code
After the reply_index handling is fixed, there's no need to special case the out scan. Signed-off-by: Andreas Fritiofson andreas.fritiof...@gmail.com --- src/jtag/drivers/rlink.c | 28 +--- 1 files changed, 9 insertions(+), 19 deletions(-) diff --git a/src/jtag/drivers/rlink.c b/src/jtag/drivers/rlink.c index d0b18b1..4b2e542 100644 --- a/src/jtag/drivers/rlink.c +++ b/src/jtag/drivers/rlink.c @@ -688,26 +688,16 @@ dtc_queue_run(void) { dtc_queue.cmd_buffer[dtc_queue.cmd_index++] = DTC_CMD_STOP; - /* run the cmd */ - if (dtc_queue.rq_head == NULL) { - usb_err = dtc_run_download(pHDev, - dtc_queue.cmd_buffer, dtc_queue.cmd_index, - NULL, 0 - ); - if (usb_err 0) { - LOG_ERROR(dtc_run_download: %s, usb_strerror()); - exit(1); - } - } else { - usb_err = dtc_run_download(pHDev, - dtc_queue.cmd_buffer, dtc_queue.cmd_index, - reply_buffer, dtc_queue.reply_index - ); - if (usb_err 0) { - LOG_ERROR(dtc_run_download: %s, usb_strerror()); - exit(1); - } + usb_err = dtc_run_download(pHDev, + dtc_queue.cmd_buffer, dtc_queue.cmd_index, + reply_buffer, dtc_queue.reply_index + ); + if (usb_err 0) { + LOG_ERROR(dtc_run_download: %s, usb_strerror()); + exit(1); + } + if (dtc_queue.rq_head != NULL) { /* process the reply, which empties the reply queue and frees its entries */ dtc_p = reply_buffer; -- 1.7.4.1 ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH 5/7] rlink: read only the expected number of bytes
After correcting the reply size counter, it should be safe to rely on it for the number of bytes expected in the USB read, instead of reading the endpoint maximum. This doesn't make things go any faster but it's nicer and removes the local buffer. Signed-off-by: Andreas Fritiofson andreas.fritiof...@gmail.com --- src/jtag/drivers/rlink.c | 20 1 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/jtag/drivers/rlink.c b/src/jtag/drivers/rlink.c index 4b2e542..46882a5 100644 --- a/src/jtag/drivers/rlink.c +++ b/src/jtag/drivers/rlink.c @@ -500,7 +500,7 @@ dtc_run_download( uint8_t *reply_buffer, int reply_buffer_size ) { - uint8_t ep2_buffer[USB_EP2IN_SIZE]; + char dtc_status; int usb_err; int i; @@ -530,12 +530,12 @@ dtc_run_download( usb_err = usb_bulk_read( pHDev_param, USB_EP1IN_ADDR, - (char *)ep2_buffer, 1, + dtc_status, 1, USB_TIMEOUT_MS ); if (usb_err 0) return(usb_err); - if (ep2_buffer[0] 0x01) break; + if (dtc_status 0x01) break; if (!--i) { LOG_ERROR(%s, %d: too many retries waiting for DTC status, @@ -546,24 +546,20 @@ dtc_run_download( } - if (!reply_buffer) reply_buffer_size = 0; - if (reply_buffer_size) { + if (reply_buffer reply_buffer_size) { usb_err = usb_bulk_read( pHDev_param, USB_EP2IN_ADDR, - (char *)ep2_buffer, sizeof(ep2_buffer), + (char *)reply_buffer, reply_buffer_size, USB_TIMEOUT_MS ); - if (usb_err (int)sizeof(ep2_buffer)) { - LOG_ERROR(%s, %d: Read of endpoint 2 returned %d, - __FILE__, __LINE__, usb_err + if (usb_err reply_buffer_size) { + LOG_ERROR(%s, %d: Read of endpoint 2 returned %d, expected %d, + __FILE__, __LINE__, usb_err, reply_buffer_size ); return(usb_err); } - - memcpy(reply_buffer, ep2_buffer, reply_buffer_size); - } return(usb_err); -- 1.7.4.1 ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH 6/7] rlink: remove redundant text from log messages
__FILE__ and __LINE__ are already printed using the log macros. Signed-off-by: Andreas Fritiofson andreas.fritiof...@gmail.com --- src/jtag/drivers/rlink.c | 15 +-- 1 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/jtag/drivers/rlink.c b/src/jtag/drivers/rlink.c index 46882a5..c501b56 100644 --- a/src/jtag/drivers/rlink.c +++ b/src/jtag/drivers/rlink.c @@ -504,7 +504,7 @@ dtc_run_download( int usb_err; int i; - LOG_DEBUG(: %d/%d, command_buffer_size, reply_buffer_size); + LOG_DEBUG(%d/%d, command_buffer_size, reply_buffer_size); usb_err = usb_bulk_write( pHDev_param, @@ -538,9 +538,7 @@ dtc_run_download( if (dtc_status 0x01) break; if (!--i) { - LOG_ERROR(%s, %d: too many retries waiting for DTC status, - __FILE__, __LINE__ - ); + LOG_ERROR(too many retries waiting for DTC status); return(-ETIMEDOUT); } } @@ -555,8 +553,8 @@ dtc_run_download( ); if (usb_err reply_buffer_size) { - LOG_ERROR(%s, %d: Read of endpoint 2 returned %d, expected %d, - __FILE__, __LINE__, usb_err, reply_buffer_size + LOG_ERROR(Read of endpoint 2 returned %d, expected %d, + usb_err, reply_buffer_size ); return(usb_err); } @@ -1493,10 +1491,7 @@ int rlink_speed(int speed) } if (dtc_start_download() 0) { - LOG_ERROR(%s, %d: starting DTC: %s, - __FILE__, __LINE__, - usb_strerror() - ); + LOG_ERROR(starting DTC: %s, usb_strerror()); exit(1); } -- 1.7.4.1 ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH 7/7] rlink: simplify and optimize queue fill level checks
Add a helper function for running the queue if it would overflow otherwise. Use it to simplify the queue fill level checks and optimize in a few cases that would previously run the queue prematurely. Signed-off-by: Andreas Fritiofson andreas.fritiof...@gmail.com --- src/jtag/drivers/rlink.c | 78 -- 1 files changed, 34 insertions(+), 44 deletions(-) diff --git a/src/jtag/drivers/rlink.c b/src/jtag/drivers/rlink.c index c501b56..f524b5c 100644 --- a/src/jtag/drivers/rlink.c +++ b/src/jtag/drivers/rlink.c @@ -795,6 +795,24 @@ dtc_queue_run(void) { return(retval); } +/* runs the queue if it cannot take reserved_cmd bytes of command data + * or reserved_reply bytes of reply data */ +static +int +dtc_queue_run_if_full( + int reserved_cmd, + int reserved_reply +) { + /* reserve one additional byte for the STOP cmd appended during run */ + if (dtc_queue.cmd_index + reserved_cmd + 1 USB_EP2BANK_SIZE) + return dtc_queue_run(); + + if (dtc_queue.reply_index + reserved_reply USB_EP2IN_SIZE) + return dtc_queue_run(); + + return ERROR_OK; +} + static int tap_state_queue_init(void) { @@ -825,12 +843,8 @@ tap_state_queue_run(void) { if ((bits = 8) || !i) { byte_param = (8 - bits); - /* make sure there's room for stop, byte op, and one byte */ - if (dtc_queue.cmd_index = (sizeof(dtc_queue.cmd_buffer) - (1 + 1 + 1))) { - dtc_queue.cmd_buffer[dtc_queue.cmd_index++] = - DTC_CMD_STOP; - dtc_queue_run(); - } + /* make sure there's room for two cmd bytes */ + dtc_queue_run_if_full(2, 0); #ifdef USE_HARDWARE_SHIFTER_FOR_TMS if (bits == 8) { @@ -1141,12 +1155,9 @@ rlink_scan( if (extra_bits (type == SCAN_OUT)) { /* Schedule any extra bits into the DTC command buffer, padding as needed */ /* For SCAN_OUT, this comes before the full bytes so the (leading) padding bits will fall off the end */ - /* make sure there's room for stop, byte op, and one byte */ - if ( - (dtc_queue.cmd_index = sizeof(dtc_queue.cmd_buffer) - (1 + 1 + 1)) - ) { - dtc_queue_run(); - } + + /* make sure there's room for two cmd bytes */ + dtc_queue_run_if_full(2, 0); x = 0; dtc_mask = 1 (extra_bits - 1); @@ -1173,22 +1184,9 @@ rlink_scan( /* Loop scheduling full bytes into the DTC command buffer */ while (byte_bits) { - if (type == SCAN_IN) { - /* make sure there's room for stop and byte op */ - x = (dtc_queue.cmd_index = sizeof(dtc_queue.cmd_buffer) - (1 + 1)); - } else { - /* make sure there's room for stop, byte op, and at least one byte */ - x = (dtc_queue.cmd_index = sizeof(dtc_queue.cmd_buffer) - (1 + 1 + 1)); - } - - if (type != SCAN_OUT) { - /* make sure there's room for at least one reply byte */ - x |= (dtc_queue.reply_index = USB_EP2IN_SIZE - (1)); - } - - if (x) { - dtc_queue_run(); - } + /* make sure there's room for one (for in scans) or two cmd bytes and +* at least one reply byte for in or inout scans*/ + dtc_queue_run_if_full(type == SCAN_IN ? 1 : 2, type != SCAN_OUT ? 1 : 0); chunk_bits = byte_bits; /* we can only use up to 16 bytes at a time */ @@ -1266,14 +1264,10 @@ rlink_scan( if (extra_bits (type != SCAN_OUT)) { /* Schedule any extra bits into the DTC command buffer */ - /* make sure there's room for stop, byte op, and one byte */ - if ( - (dtc_queue.cmd_index = sizeof(dtc_queue.cmd_buffer) - (1 + 1 + 1)) - || - (dtc_queue.reply_index = USB_EP2IN_SIZE - (1)) - ) { - dtc_queue_run(); - } + + /* make sure there's room for one (for in scans) or two cmd bytes +* and one reply byte */ + dtc_queue_run_if_full(type == SCAN_IN ? 1 : 2, 1); if (dtc_queue_enqueue_reply( type, buffer, scan_size, tdi_bit_offset, @@ -1318,14 +1312,10 @@ rlink_scan( } /* Schedule the last bit into the DTC command buffer */ - /* make sure there's room for stop, and bit pair
[Openocd-development] [PATCH 0/5] Flash program speedup through asynchronous algorithms
This patch set enables targets to implement asynchronous algorithms that can be run in the background while OpenOCD performs other tasks. It also implements async algorithm support for ARMv7-M and enables it for the Cortex-M3 target. The final two patches rewrites the stm32x flash driver to use an async algorithm for block writes. Speed test on a STM32 Primer (STM32F103 platform with built in RLink) with the following openocd.cfg --- source [find interface/rlink.cfg] source [find target/stm32.cfg] $_TARGETNAME configure -event reset-init { mww 0x40022000 0x32 mww 0x40021004 0x3c0400 mww 0x40021000 0x01000883 sleep 10 mww 0x40021004 0x3c0402 sleep 10 adapter_khz 3000 } init reset init poll off stm32x mass_erase 0 flash write_image test.bin 0x0800 verify_image test.bin 0x0800 shutdown --- Vanilla: wrote 65536 bytes from file test.bin in 8.316117s (7.696 KiB/s) verified 65536 bytes in 0.748795s (85.471 KiB/s) Async patch: wrote 65536 bytes from file test.bin in 4.084972s (15.667 KiB/s) verified 65536 bytes in 0.772365s (82.862 KiB/s) Speedup of 100%! Wait, there's more... the last run was limited by the slow RLink throughput. The previously sent Rlink patchset increases that, but still has the high latency. RLink patch only: wrote 65536 bytes from file test.bin in 7.171075s (8.925 KiB/s) verified 65536 bytes in 0.748815s (85.468 KiB/s) RLink+Async patch wrote 65536 bytes from file test.bin in 3.016943s (21.214 KiB/s) verified 65536 bytes in 0.773821s (82.706 KiB/s) Speedup of 137%! While the RLink patch alone only increases speed by 15%, because the speed is still limited by roundtrip latency, with the async algorithm the speed increase from the RLink patch is 35%, since the algorithm is now bound by throughput, not latency. It would be nice to see some figures using faster interfaces, say some FT2232H design. Unfortunately I don't have one right now. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH 1/5] target: add async algorithm entries to the target type
On supported targets, this may be used to start a long running algorithm in the background so the target may be interacted with during execution and later wait for its completion. The most obvious use case is a double buffered flash algorithm that can upload the next block of data while the algorithm is flashing the current. Signed-off-by: Andreas Fritiofson andreas.fritiof...@gmail.com --- src/target/target.c | 75 ++ src/target/target.h | 22 + src/target/target_type.h |2 + 3 files changed, 99 insertions(+), 0 deletions(-) diff --git a/src/target/target.c b/src/target/target.c index f62915e..8726d55 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -701,6 +701,81 @@ done: return retval; } +/** + * Downloads a target-specific native code algorithm to the target, + * executes and leaves it running. + * + * @param target used to run the algorithm + * @param arch_info target-specific description of the algorithm. + */ +int target_start_algorithm(struct target *target, + int num_mem_params, struct mem_param *mem_params, + int num_reg_params, struct reg_param *reg_params, + uint32_t entry_point, uint32_t exit_point, + void *arch_info) +{ + int retval = ERROR_FAIL; + + if (!target_was_examined(target)) + { + LOG_ERROR(Target not examined yet); + goto done; + } + if (!target-type-start_algorithm) { + LOG_ERROR(Target type '%s' does not support %s, + target_type_name(target), __func__); + goto done; + } + if (target-running_alg) { + LOG_ERROR(Target is already running an algorithm); + goto done; + } + + target-running_alg = true; + retval = target-type-start_algorithm(target, + num_mem_params, mem_params, + num_reg_params, reg_params, + entry_point, exit_point, arch_info); + +done: + return retval; +} + +/** + * Waits for an algorithm started with target_start_algorithm() to complete. + * + * @param target used to run the algorithm + * @param arch_info target-specific description of the algorithm. + */ +int target_wait_algorithm(struct target *target, + int num_mem_params, struct mem_param *mem_params, + int num_reg_params, struct reg_param *reg_params, + uint32_t exit_point, int timeout_ms, + void *arch_info) +{ + int retval = ERROR_FAIL; + + if (!target-type-wait_algorithm) { + LOG_ERROR(Target type '%s' does not support %s, + target_type_name(target), __func__); + goto done; + } + if (!target-running_alg) { + LOG_ERROR(Target is not running an algorithm); + goto done; + } + + retval = target-type-wait_algorithm(target, + num_mem_params, mem_params, + num_reg_params, reg_params, + exit_point, timeout_ms, arch_info); + if (retval != ERROR_TARGET_TIMEOUT) + target-running_alg = false; + +done: + return retval; +} + int target_read_memory(struct target *target, uint32_t address, uint32_t size, uint32_t count, uint8_t *buffer) diff --git a/src/target/target.h b/src/target/target.h index 74f74de..842c836 100644 --- a/src/target/target.h +++ b/src/target/target.h @@ -417,6 +417,28 @@ int target_run_algorithm(struct target *target, int timeout_ms, void *arch_info); /** + * Starts an algorithm in the background on the @a target given. + * + * This routine is a wrapper for target-type-start_algorithm. + */ +int target_start_algorithm(struct target *target, + int num_mem_params, struct mem_param *mem_params, + int num_reg_params, struct reg_param *reg_params, + uint32_t entry_point, uint32_t exit_point, + void *arch_info); + +/** + * Wait for an algorithm on the @a target given. + * + * This routine is a wrapper for target-type-wait_algorithm. + */ +int target_wait_algorithm(struct target *target, + int num_mem_params, struct mem_param *mem_params, + int num_reg_params, struct reg_param *reg_params, + uint32_t exit_point, int timeout_ms, + void *arch_info); + +/** * Read @a count items of @a size bytes from the memory of @a target at * the @a address given. * diff --git a/src/target/target_type.h b/src/target/target_type.h index 6059c40..d44bc56 100644 --- a/src/target/target_type.h +++ b/src/target/target_type.h @@ -169,6 +169,8 @@ struct target_type * use target_run_algorithm() instead. */ int (*run_algorithm)(struct target *target, int num_mem_params, struct mem_param
[Openocd-development] [PATCH 2/5] armv7m: implement async algorithm functions
Split armv7m_run_algorithm into two pieces and use them to reimplement it. The arch_info parameter is used to keep context between the two calls, so both calls must refer to the same armv7m_algorithm struct. Ugly but works for a proof-of-concept. Signed-off-by: Andreas Fritiofson andreas.fritiof...@gmail.com --- src/target/armv7m.c | 110 -- src/target/armv7m.h | 14 ++ 2 files changed, 84 insertions(+), 40 deletions(-) diff --git a/src/target/armv7m.c b/src/target/armv7m.c index fff5dd8..39a89b9 100644 --- a/src/target/armv7m.c +++ b/src/target/armv7m.c @@ -287,53 +287,42 @@ int armv7m_get_gdb_reg_list(struct target *target, struct reg **reg_list[], int return ERROR_OK; } -/* run to exit point. return error if exit point was not reached. */ -static int armv7m_run_and_wait(struct target *target, uint32_t entry_point, int timeout_ms, uint32_t exit_point, struct armv7m_common *armv7m) +/** Runs a Thumb algorithm in the target. */ +int armv7m_run_algorithm(struct target *target, + int num_mem_params, struct mem_param *mem_params, + int num_reg_params, struct reg_param *reg_params, + uint32_t entry_point, uint32_t exit_point, + int timeout_ms, void *arch_info) { - uint32_t pc; int retval; - /* This code relies on the target specific resume() and poll()-debug_entry() -* sequence to write register values to the processor and the read them back */ - if ((retval = target_resume(target, 0, entry_point, 1, 1)) != ERROR_OK) - { - return retval; - } - retval = target_wait_state(target, TARGET_HALTED, timeout_ms); - /* If the target fails to halt due to the breakpoint, force a halt */ - if (retval != ERROR_OK || target-state != TARGET_HALTED) - { - if ((retval = target_halt(target)) != ERROR_OK) - return retval; - if ((retval = target_wait_state(target, TARGET_HALTED, 500)) != ERROR_OK) - { - return retval; - } - return ERROR_TARGET_TIMEOUT; - } + retval = armv7m_start_algorithm(target, + num_mem_params, mem_params, + num_reg_params, reg_params, + entry_point, exit_point, + arch_info); - armv7m-load_core_reg_u32(target, ARMV7M_REGISTER_CORE_GP, 15, pc); - if (exit_point (pc != exit_point)) - { - LOG_DEBUG(failed algorithm halted at 0x% PRIx32 , pc); - return ERROR_TARGET_TIMEOUT; - } + if (retval == ERROR_OK) + retval = armv7m_wait_algorithm(target, + num_mem_params, mem_params, + num_reg_params, reg_params, + exit_point, timeout_ms, + arch_info); - return ERROR_OK; + return retval; } -/** Runs a Thumb algorithm in the target. */ -int armv7m_run_algorithm(struct target *target, +/** Starts a Thumb algorithm in the target. */ +int armv7m_start_algorithm(struct target *target, int num_mem_params, struct mem_param *mem_params, int num_reg_params, struct reg_param *reg_params, uint32_t entry_point, uint32_t exit_point, - int timeout_ms, void *arch_info) + void *arch_info) { struct armv7m_common *armv7m = target_to_armv7m(target); struct armv7m_algorithm *armv7m_algorithm_info = arch_info; enum armv7m_mode core_mode = armv7m-core_mode; int retval = ERROR_OK; - uint32_t context[ARMV7M_NUM_REGS]; /* NOTE: armv7m_run_algorithm requires that each algorithm uses a software breakpoint * at the exit point */ @@ -356,11 +345,12 @@ int armv7m_run_algorithm(struct target *target, { if (!armv7m-core_cache-reg_list[i].valid) armv7m-read_core_reg(target, i); - context[i] = buf_get_u32(armv7m-core_cache-reg_list[i].value, 0, 32); + armv7m_algorithm_info-context[i] = buf_get_u32(armv7m-core_cache-reg_list[i].value, 0, 32); } for (int i = 0; i num_mem_params; i++) { + // TODO: Write only out params if ((retval = target_write_buffer(target, mem_params[i].address, mem_params[i].size, mem_params[i].value)) != ERROR_OK) return retval; } @@ -394,12 +384,52 @@ int armv7m_run_algorithm(struct target *target, armv7m-core_cache-reg_list[ARMV7M_CONTROL].dirty = 1; armv7m-core_cache-reg_list[ARMV7M_CONTROL].valid = 1; } + armv7m_algorithm_info-core_mode = core_mode; - retval = armv7m_run_and_wait(target, entry_point, timeout_ms, exit_point, armv7m); + retval = target_resume(target, 0, entry_point, 1, 1); - if (retval
[Openocd-development] [PATCH 3/5] cortex_m3: use armv7m's async algorithm implementation
Signed-off-by: Andreas Fritiofson andreas.fritiof...@gmail.com --- src/target/cortex_m3.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/target/cortex_m3.c b/src/target/cortex_m3.c index 98a775c..a2f8b78 100644 --- a/src/target/cortex_m3.c +++ b/src/target/cortex_m3.c @@ -2329,6 +2329,8 @@ struct target_type cortexm3_target = .blank_check_memory = armv7m_blank_check_memory, .run_algorithm = armv7m_run_algorithm, + .start_algorithm = armv7m_start_algorithm, + .wait_algorithm = armv7m_wait_algorithm, .add_breakpoint = cortex_m3_add_breakpoint, .remove_breakpoint = cortex_m3_remove_breakpoint, -- 1.7.4.1 ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH 4/5] stm32x: use register base instead of register offset
Access the different flash banks' registers using a bank specific register base and a register specific offset. This is equivalent but feels more natural. Some accesses were discovered that maybe should not be hard coded to bank0 registers. Add a note about that. Signed-off-by: Andreas Fritiofson andreas.fritiof...@gmail.com --- src/flash/nor/stm32x.c | 95 ++-- 1 files changed, 51 insertions(+), 44 deletions(-) diff --git a/src/flash/nor/stm32x.c b/src/flash/nor/stm32x.c index b4300be..ef8c2a8 100644 --- a/src/flash/nor/stm32x.c +++ b/src/flash/nor/stm32x.c @@ -31,14 +31,29 @@ /* stm32x register locations */ -#define STM32_FLASH_ACR0x40022000 -#define STM32_FLASH_KEYR 0x40022004 -#define STM32_FLASH_OPTKEYR0x40022008 -#define STM32_FLASH_SR 0x4002200C -#define STM32_FLASH_CR 0x40022010 -#define STM32_FLASH_AR 0x40022014 -#define STM32_FLASH_OBR0x4002201C -#define STM32_FLASH_WRPR 0x40022020 +#define FLASH_REG_BASE_B0 0x40022000 +#define FLASH_REG_BASE_B1 0x40022040 + +#define STM32_FLASH_ACR 0x00 +#define STM32_FLASH_KEYR0x04 +#define STM32_FLASH_OPTKEYR 0x08 +#define STM32_FLASH_SR 0x0C +#define STM32_FLASH_CR 0x10 +#define STM32_FLASH_AR 0x14 +#define STM32_FLASH_OBR 0x1C +#define STM32_FLASH_WRPR0x20 + +/* TODO: Check if code using these really should be hard coded to bank 0. + * There are valid cases, on dual flash devices the protection of the + * second bank is done on the bank0 reg's. */ +#define STM32_FLASH_ACR_B0 0x40022000 +#define STM32_FLASH_KEYR_B00x40022004 +#define STM32_FLASH_OPTKEYR_B0 0x40022008 +#define STM32_FLASH_SR_B0 0x4002200C +#define STM32_FLASH_CR_B0 0x40022010 +#define STM32_FLASH_AR_B0 0x40022014 +#define STM32_FLASH_OBR_B0 0x4002201C +#define STM32_FLASH_WRPR_B00x40022020 /* option byte location */ @@ -83,12 +98,6 @@ #define KEY1 0x45670123 #define KEY2 0xCDEF89AB -/* we use an offset to access the second bank on dual flash devices - * strangely the protection of the second bank is done on the bank0 reg's */ - -#define FLASH_OFFSET_B00x00 -#define FLASH_OFFSET_B1 0x40 - struct stm32x_options { uint16_t RDP; @@ -104,10 +113,8 @@ struct stm32x_flash_bank int probed; bool has_dual_banks; - /* used to access dual flash bank stm32xl -* 0x00 will address bank 0 flash -* 0x40 will address bank 1 flash */ - int register_offset; + /* used to access dual flash bank stm32xl */ + uint32_t register_base; }; static int stm32x_mass_erase(struct flash_bank *bank); @@ -130,7 +137,7 @@ FLASH_BANK_COMMAND_HANDLER(stm32x_flash_bank_command) stm32x_info-write_algorithm = NULL; stm32x_info-probed = 0; stm32x_info-has_dual_banks = false; - stm32x_info-register_offset = FLASH_OFFSET_B0; + stm32x_info-register_base = FLASH_REG_BASE_B0; return ERROR_OK; } @@ -138,7 +145,7 @@ FLASH_BANK_COMMAND_HANDLER(stm32x_flash_bank_command) static inline int stm32x_get_flash_reg(struct flash_bank *bank, uint32_t reg) { struct stm32x_flash_bank *stm32x_info = bank-driver_priv; - return reg + stm32x_info-register_offset; + return reg + stm32x_info-register_base; } static inline int stm32x_get_flash_status(struct flash_bank *bank, uint32_t *status) @@ -200,7 +207,7 @@ int stm32x_check_operation_supported(struct flash_bank *bank) /* if we have a dual flash bank device then * we need to perform option byte stuff on bank0 only */ - if (stm32x_info-register_offset != FLASH_OFFSET_B0) + if (stm32x_info-register_base != FLASH_REG_BASE_B0) { LOG_ERROR(Option Byte Operation's must use bank0); return ERROR_FLASH_OPERATION_FAILED; @@ -218,7 +225,7 @@ static int stm32x_read_options(struct flash_bank *bank) stm32x_info = bank-driver_priv; /* read current option bytes */ - int retval = target_read_u32(target, STM32_FLASH_OBR, optiondata); + int retval = target_read_u32(target, STM32_FLASH_OBR_B0, optiondata); if (retval != ERROR_OK) return retval; @@ -229,7 +236,7 @@ static int stm32x_read_options(struct flash_bank *bank) LOG_INFO(Device Security Bit Set); /* each bit refers to a 4bank protection */ - retval = target_read_u32(target, STM32_FLASH_WRPR, optiondata); + retval = target_read_u32(target, STM32_FLASH_WRPR_B0, optiondata); if (retval != ERROR_OK) return retval; @@ -252,27 +259,27 @@ static int stm32x_erase_options(struct flash_bank *bank) stm32x_read_options(bank); /* unlock flash registers */ - int retval = target_write_u32(target, STM32_FLASH_KEYR, KEY1); + int retval = target_write_u32(target, STM32_FLASH_KEYR_B0, KEY1
[Openocd-development] [PATCH 5/5] stm32x: use async algorithm in flash programming routine
Let the target algorithm be running in the background and buffer data continuously through a FIFO. This reduces or removes the effect of latency because only a very small number of queue executions needs to be done per buffer fill. Previously, the many repeated target state changes, register accesses (really inefficient) and algorithm uploads caused the flash programming to be latency bound in many cases. Now it should scale better with increased throughput. Signed-off-by: Andreas Fritiofson andreas.fritiof...@gmail.com --- contrib/loaders/flash/stm32x.S | 71 -- src/flash/nor/stm32x.c | 210 ++-- 2 files changed, 201 insertions(+), 80 deletions(-) diff --git a/contrib/loaders/flash/stm32x.S b/contrib/loaders/flash/stm32x.S index 01494b8..125c76a 100644 --- a/contrib/loaders/flash/stm32x.S +++ b/contrib/loaders/flash/stm32x.S @@ -1,6 +1,6 @@ /*** - * Copyright (C) 2010 by Spencer Oliver * - * s...@spen-soft.co.uk * + * Copyright (C) 2011 by Andreas Fritiofson * + * andreas.fritiof...@gmail.com * * * * 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 * @@ -25,34 +25,47 @@ .thumb_func .global write -/* - r0 - source address - r1 - target address - r2 - count (halfword-16bit) - r3 - sector offet in : result out - r4 - flash base -*/ + /* Params: +* r0 - flash base (in), status (out) +* r1 - count (halfword-16bit) +* r2 - workarea start +* r3 - workarea end +* r4 - target address +* Clobbered: +* r5 - rp +* r6 - wp, tmp +*/ -#define STM32_FLASH_CR_OFFSET 0x10/* offset of CR register in FLASH struct */ -#define STM32_FLASH_SR_OFFSET 0x0c/* offset of CR register in FLASH struct */ +#define STM32_FLASH_CR_OFFSET 0x10 /* offset of CR register from flash reg base */ +#define STM32_FLASH_SR_OFFSET 0x0c /* offset of SR register from flash reg base */ -write: - ldr r4, STM32_FLASH_BASE - add r4, r3 /* add offset 0x00 for sector 0 : 0x40 for sector 1 */ -write_half_word: - movsr3, #0x01 - str r3, [r4, #STM32_FLASH_CR_OFFSET]/* PG (bit0) == 1 = flash programming enabled */ - ldrhr3, [r0], #0x02 /* read one half-word from src, increment ptr */ - strhr3, [r1], #0x02 /* write one half-word from src, increment ptr */ +wait_fifo: + ldr r6, [r2, #0]/* read wp */ + cmp r6, #0 /* abort if wp == 0 */ + beq exit + ldr r5, [r2, #4]/* read rp */ + cmp r5, r6 /* wait until rp != wp */ + beq wait_fifo + movsr6, #1 /* set PG flag to enable flash programming */ + str r6, [r0, #STM32_FLASH_CR_OFFSET] + ldrhr6, [r5], #2/* *target_address++ = *rp++ */ + strhr6, [r4], #2 busy: - ldr r3, [r4, #STM32_FLASH_SR_OFFSET] - tst r3, #0x01 /* BSY (bit0) == 1 = operation in progress */ - beq busy /* wait more... */ - tst r3, #0x14 /* PGERR (bit2) == 1 or WRPRTERR (bit4) == 1 = error */ - bne exit /* fail... */ - subsr2, r2, #0x01 /* decrement counter */ - bne write_half_word /* write next half-word if anything left */ + ldr r6, [r0, #STM32_FLASH_SR_OFFSET]/* wait until BSY flag is reset */ + tst r6, #1 + bne busy + tst r6, #0x14 /* check the error bits */ + bne error + cmp r5, r3 /* wrap rp at end of buffer */ + it cs + addcs r5, r2, #8 + str r5, [r2, #4]/* store rp */ + subsr1, r1, #1 /* decrement halfword count */ + cbz r1, exit/* loop if not done */ + b wait_fifo +error: + movsr0, #0 + str r0, [r2, #2]/* set rp = 0 on error */ exit: - bkpt#0x00 - -STM32_FLASH_BASE: .word 0x40022000
Re: [Openocd-development] [PATCH 0/7] RLink interface speedup and fixes
On Mon, Jul 18, 2011 at 4:03 PM, Spencer Oliver s...@spen-soft.co.ukwrote: On 18 July 2011 11:13, Andreas Fritiofson andreas.fritiof...@gmail.com wrote: This patch set fixes some general problems in the RLink interface driver. Most importantly it fixes a performance bug that have been causing decreased throughput. Speed test on a STM32 Primer (STM32F103 platform with built in RLink) with the following openocd.cfg --- great work - someone has been busy :) I have a tendency to leave these patches until after we make a formal 0.5 release - you have some significant changes included. Yes it's best to put these on hold for 0.5.0. It needs some testing on the standalone RLink or other variants, I currently only have access to STM32 Primers. Also more thorough testing in general. Anyone feels like helping? /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH 0/7] RLink interface speedup and fixes
On Mon, Jul 18, 2011 at 2:33 PM, Tomek CEDRO tomek.ce...@gmail.com wrote: Hello Andreas! Do you know what is the difference between JTAG (on Stm32Primer) and SWD (on Stm32Primer2) RLink interface? Would they run on existing driver? Do you have some kind of manual or technical specification? Other than the JTAG/SWD difference, I don't know. I don't have a Primer2 or any spec for the RLinks in general. I don't know if the docs are public or if the protocol was reverse engineered for the OpenOCD driver. I'm guessing that the SWD capable interface has some extra command but the same basic protocol. IIRC the standalone RLink we have at work can handle a number of other protocols too. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH 0/7] RLink interface speedup and fixes
On Mon, Jul 18, 2011 at 4:59 PM, Jean-Claude Repetto jcr...@mxm.eu wrote: Le 18/07/2011 16:25, Andreas Fritiofson a écrit : Yes it's best to put these on hold for 0.5.0. It needs some testing on the standalone RLink or other variants, I currently only have access to STM32 Primers. Also more thorough testing in general. Anyone feels like helping? Hello Andreas, I have a standalone R-Link and a Keil STM32 board, but a very little knowledge of Openocd. What can I do to help the project ? Hi, great! Do you know how to *use* OpenOCD (as opposed to knowing its internals)? Otherwise it could be a good opportunity to learn! :) If you have a setup that's currently working for you, so you can program flash, debug and whatnot, and some knowledge (or willingness to learn) how to clone a git repository and building software from source, you can sure help with testing. Clone the openocd repository, checkout master, build it and start it up to see if it works. Then apply my patches, build it and then verify that you can still do everything that you could before. Try anything that comes to mind. Report any errors. Thanks, Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] MIPS target, big endian host
On Fri, Jul 8, 2011 at 12:52 PM, Øyvind Harboe oyvind.har...@zylin.comwrote: What puzzles me is that there is no warning on x86, even if I the -Wcast-align option is there -Wcast-align Warn whenever a pointer is cast such that the required alignment of the target is increased. For example, warn if a char * is cast to an int * on machines where integers can only be accessed at two- or four-byte boundaries. on machines where integers can only be accessed at two- or four-byte boundaries x86 is not such a machine, it supports unaligned access. Which is why it's good to do a cross-compile to another arch regularly to catch these problems. It would be good if there was an option to emit these warnings regardless. Maybe there is. I looked briefly at the memory read functions in mips32_dmaacc.c and mips32_pracc.c and it looks like the type usage is a bit confused. The difference between the *_read_mem{32,16,8} functions should only be what kind of access is made *on the target*. Host data buffer type should be identical, preferrably void*, with no alignment requirement, and count should be in number of bytes. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] MIPS target, big endian host
On Fri, Jul 8, 2011 at 1:17 PM, Drasko DRASKOVIC drasko.drasko...@gmail.com wrote: I am just wandering, would : t32 = *(uint32_t*)((void *)buffer[i]); quite the compiler ;) Yes probably, but it would still crash on an architecture that doesn't support unaligned accesses. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] MIPS target, big endian host
On Fri, Jul 8, 2011 at 1:38 PM, Mahr, Stefan stefan.m...@sphairon.comwrote: are you sure about this ? It seems to me that buffer[i] is directly filled by target, and I see no reason that it is in the host endianess... Hi Drasko, Yes I'm sure. I tested it on my big endian host platform. I do not understand the code completely, but I think it's caused by the mips ejtag functions. Example: - mips_ejtag_drscan_32 uses uint32 for data - buf_set_u32 and buf_get_u32 make sure that data is in host endianness Where are those functions defined and how do they know what the target endianness is? It sounds a little strange to do the swapping at this low level. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] MIPS target, big endian host
On Fri, Jul 8, 2011 at 2:05 PM, Mahr, Stefan stefan.m...@sphairon.comwrote: Where are those functions defined and how do they know what the target endianness is? They doesn't know the target endianness, but host endianness. It sounds a little strange to do the swapping at this low level. You need swapping when reading and comparing debug registers or send code to MIPS CPU. But there are other times when swapping is not the right thing to do, such as reading/writing arbitrary memory blocks, right? So I'd say the swapping belongs to the higher levels that know if the data is to be interpreted or not. Otherwise we'll just be swapping back and forth needlessly. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] MIPS target, big endian host
On Fri, Jul 8, 2011 at 2:05 PM, Mahr, Stefan stefan.m...@sphairon.comwrote: Where are those functions defined and how do they know what the target endianness is? They doesn't know the target endianness, but host endianness. You can't convert between target and host endianness if you don't know both. I'm getting confused about all this endianness. Main thing we need to do is to define and be consistent with where we have target and host endianess and look over the data types we use. This is probably not restricted to the MIPS code but probably goes up into the target interface. I have to think it through and right now I have a bunch of kids running around my legs so it'll have to wait. :) /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] MIPS target, big endian host
On Fri, Jul 8, 2011 at 1:26 PM, Drasko DRASKOVIC drasko.drasko...@gmail.com wrote: On Fri, Jul 8, 2011 at 1:19 PM, Andreas Fritiofson andreas.fritiof...@gmail.com wrote: I looked briefly at the memory read functions in mips32_dmaacc.c and mips32_pracc.c and it looks like the type usage is a bit confused. The difference between the *_read_mem{32,16,8} functions should only be what kind of access is made *on the target*. How do we determine that ? I thought that it is kept in the size parameter, and mips32_pracc_read_mem is doing exactly this : int mips32_pracc_read_mem(struct mips_ejtag *ejtag_info, uint32_t addr, int size, int count, void *buf) { switch (size) { case 1: return mips32_pracc_read_mem8(ejtag_info, addr, count, (uint8_t*)buf); case 2: return mips32_pracc_read_mem16(ejtag_info, addr, count, (uint16_t*)buf); case 4: if (count == 1) return mips32_pracc_read_u32(ejtag_info, addr, (uint32_t*)buf); else return mips32_pracc_read_mem32(ejtag_info, addr, count, (uint32_t*)buf); } return ERROR_OK; } Host data buffer type should be identical, preferrably void*, with no alignment requirement, and count should be in number of bytes. Problem is not in the mips32_pracc.c, thought, but when you come back to mips_m4k_read_memory(), in which buf is uint8_t*. But already here buf is cast to uint{8,16,32}_t* (from void* so no warning but equally risky) which shouldn't be needed because all read_mem* functions could accept a void* buffer and write the data to arbitrary alignment. Main point is that the alignment requirement (buffer data type) on the host should not be coupled to the access size used on the target. And also the data returned in the host buffer should be identical, regardless of which target access size is chosen (this requirement probably gives the answer to whether endian swapping should be done here or not). Does this make sense? Of course you may not be able to do an access with size 2 or 4 to an unaligned *target address* but that has nothing to do with host buffer alignment. If the memory functions only handle byte addressed generic memory blocks, there are no endian issues (here). They pop up first when higher level code tries to interpret the memory contents as multi-byte entities (instructions, addresses, ...) in which case that code must be aware of the target endianness. Again, I may be confused here. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] MIPS target, big endian host
On Wed, Jul 6, 2011 at 11:35 PM, Øyvind Harboe oyvind.har...@zylin.comwrote: On Wed, Jul 6, 2011 at 11:28 PM, Mahr, Stefan stefan.m...@sphairon.com wrote: mips32_pracc_read_mem casts uint32 to void, so we need to cast it back to uint32. I found no suitable macro in actual sources. Hmm then I think we ought to define one to get this put to bed once and for all... static inline uint32_t uint32_read_unaligned(const void *data) { uint32_t t; // Let's trust the compiler to do something very clever here. memcpy(t, data, sizeof(t)); return t; } Do we really need a memcpy? Trust the compiler: oyvind@fierce:~$ cat test.c #include string.h #include stdint.h uint32_t uint32_read_unaligned(const void *data) { uint32_t t; // Let's trust the compiler to do something very clever here. memcpy(t, data, sizeof(t)); return t; } oyvind@fierce:~$ gcc -O3 -S test.c And with the default optimization level (O2, isn't it)? Could we ever run into an alignment issue when simply cast void *back* to uint32 ? If not, I would prefer the simplifed solution. The below does not work in all cases. It will break on architectures that do not support unaligned access. No, casting a pointer-to-any-type to a pointer-to-void and back will never cause alignment issues. The question is who makes the guarantee that the function is only ever called with uint32-aligned generic pointers. If it just happens to be so that all *current* callers pass a uint32 pointer cast to void pointer, it would be a terribly bad idea to suddenly assume that. If it is guaranteed by design/contract, why is a void pointer used at all? Just use a pointer to uint32 then. I have no experience with the MIPS code or arch at all, but if it was up to me I'd prefer if a *_read_mem function didn't interpret the data and thus it should deal with void pointers. It would by design be totally free from all endian issues and conversions. Moreover, a function accepting a pointer-to-void should be prepared to handle any kind of alignment thrown at it (unless explicitly stated). That excludes direct casts to uint32_t*. That's why GCC is complaining. Sometimes... It should never complain when casting a generic pointer to another pointer, right? You shouldn't even need an explicit cast IIRC. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] eclipse indigo (pipe usage)
On Fri, Jul 1, 2011 at 12:47 PM, Spencer Oliver s...@spen-soft.co.ukwrote: On 1 July 2011 00:41, Andrew Leech coronasen...@gmail.com wrote: One other point, do you know whether you can get openocd to add it's own program directory to the search path for the source [find ...] lines? It'd be great to only need the openocd.cfg file in the eclipse project dir, and have it reference the interface and target files from the common openocd folder. Otherwise I'll likely copy the contents of the files into my openocd.cfg rather than have a copy of target and interface folders in all my project folders. try something like: add_script_search_dir {c:\cygwin\home\soliver\openocd\openocd\tcl} Cheers Spen I wrote a patch to fix this a long time ago, pre 0.4.0 i think. However i didn't test it under cygwin because I don't use it, only msys. It may need some adjustments. Andrew, you didn't say what platform you're on and how you built/installed openocd? The search path should be printed in the debug log, can you look there and see how it differs from where you have the files installed? /Andreas Found the relevant thread(s): http://www.mail-archive.com/openocd-development@lists.berlios.de/msg10145.html http://www.mail-archive.com/openocd-development@lists.berlios.de/msg11678.html ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] compiler warnings with dsp5680xx.c
On Wed, Jun 29, 2011 at 9:17 PM, Rodrigo Rosa rodrigorosa...@gmail.comwrote: On Wed, Jun 29, 2011 at 12:44 AM, Øyvind Harboe oyvind.har...@zylin.com wrote: This is disturbing. Why switch to global variables? +static uint32_t data_read_dummy; static int jtag_data_write(struct target * target, uint32_t instr,int num_bits, uint32_t * data_read){ int retval; - uint32_t data_read_dummy; i did this to have mem to dump the data read at the time the queue is flushed i do not care about this data, but i need somewhere to dump it (otherwise it'll segfault). multiple instances of the target would not cause trouble, since i do not care about this data, so it could be overwritten and it would no break anything. It should be safe to pass null as read pointer if you want to discard the data from the scan. This is also explicitly stated in dsp5680xx_drscan(). However, jtag_data_write does use the data: if(data_read != NULL) *data_read = data_read_dummy; This doesn't seem right, because the queue hasn't been executed yet and so data_read_dummy has a bogus value. Bug? /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] USB Blaster support broken
On Tue, Jun 28, 2011 at 9:19 AM, Domien Nowicki domien.nowi...@gmail.comwrote: ** 1. The USB Blaster clone does not use the original FT245 chip, and so it tries to emulate its behavior. As it turns out, the API call FT_GetLatencyTimer is not properly emulated by the clone, and this makes OpenOCD abort. In reality, this API call is not necessary, so I have removed this call. Are you sure this is not the bug in FTD2XX on Linux that was mentioned on this list recently? In that case, libftdi shouldn't be affected so it could be left as-is. For ft2232 the error was changed to a warning as a workaround (if that patch got merged), perhaps do the same here? 2. The LED blink code that was added in commit (24943498e611649a540d98406288dd6d4889851d) made the JTAG communication unstable, see http://openocd.git.sourceforge.net/git/gitweb.cgi?p=openocd/openocd;a=commitdiff;h=24943498e611649a540d98406288dd6d4889851d. The USB Blaster dongle would properly detect the IDCODE, but would later fail when trying to read/write the DPACC ARM JTAG registers. Not surpringly, this is because the blink code resets the out_value, which keeps track of the state of the JTAG pins. In my tests, disabling or blinking the LED flag made JTAG communication very unstable. This flag needs to be permanently enabled for proper operation. Do you have a clue as to WHY the LED signal must be set for the dongle to function properly? I don't know the internals of the USB Blaster in detail, but it doesn't sound right. For sure, all our USB Blasters blink when used with the Altera tools. No problem there. If out_value keeps state, the blink function is obviously broken, but I don't see why it couldn't be fixed by modifying only the LED bit in out_value. Also, why have you added commented out code? If it doesn't work (it doesn't), don't add it. Of course the better solution would be to make it work. (Hint: fix the bit clear, should be ~LED.) /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Cortex M3: Patch for automatic handling of interrupt mask during stepping
Great, but does it really work? There has been discussions about this on this list in the past. I think the conclusion was that there's no way to robustly know GDB's intentions based on the remote commands. When you do a source-level step in GDB, it may send a step command to OpenOCD. Then we can disable interrupts during the step. But it can also place a breakpoint at the next source line and send a continue command. In that case we must leave interrupts enabled. Wouldn't that mean an unpredictable difference in behaviour depending on how GDB chooses to do the step? Note that I haven't yet looked at the patch so this may not be relevant. Regards, Andreas On Sat, Jun 18, 2011 at 4:26 PM, Peter Horn peter.h...@bluewin.ch wrote: Dear all I'm submitting a change to the Cortex-M3 target which improves behavior of single stepping with interrupts. Here's the description of the patch, hope you like it: This patch extends the cortex_m3 maskisr command by a new option 'auto'. The 'auto' option handles interrupts during stepping in a way they are processed but don't disturb the program flow during debugging. Before one had to choose to either enable or disable interrupts. The former steps into interrupt handlers when they trigger. This disturbs the flow during debugging, making it hard to follow some piece of code when interrupts occur often. When interrupts are disabled, the flow isn't disturbed but code relying on interrupt handlers to be processed will stop working. For example a delay function counting the number of timer interrupts will never complete, RTOS task switching will not occur and output I/O queues of interrupt driven I/O will stall or overflow. Using the 'maskisr' command also typically requires gdb hooks to be supplied by the user to switch interrupts off during the step and to enable them again afterward. The new 'auto' option of the 'maskisr' command solves the above problems. When set, the step command allows pending interrupt handlers to be executed before the step, then the step is taken with interrupts disabled and finally interrupts are enabled again. This way interrupt processing stays in the background without disturbing the flow of debugging. No gdb hooks are required. The 'auto' option is the default, since it's believed that handling interrupts in this way is suitable for most users. The principle used for interrupt handling could probably be used for other targets too. Best Regards Peter Horn ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Cortex M3: Patch for automatic handling of interrupt mask during stepping
Resending, Cc to the list got lost somewhere... On Sat, Jun 18, 2011 at 8:27 PM, Andreas Fritiofson andreas.fritiof...@gmail.com wrote: On Sat, Jun 18, 2011 at 6:38 PM, Peter Horn peter.h...@bluewin.ch wrote: Hi Andreas Am 18.06.2011 17:32, schrieb Andreas Fritiofson: Great, but does it really work? I've tested it with a simple test program containing a main loop and a SysTick_Handler(). I've also tested it with a real application in which UART RX interrupts happen at least once per second (heart beat message of a slave processor coming in). Without the patch, I wasn't able to do stepping, because each step ran into the UART handler. Disabling the interrupts led to blocking because UART output queues overflowed, nothing was received anymore and no RTOS task switching happened. Received data will probably be lost even with your patch, unless you step often enough. With the patch I can do useful work with the debugger again. There has been discussions about this on this list in the past. I think the conclusion was that there's no way to robustly know GDB's intentions based on the remote commands. That's true. You can see what GDB does when you read openocds debug output messages. My comments below are found by observation. When you do a source-level step in GDB, it may send a step command to OpenOCD. Then we can disable interrupts during the step. Step commands are issued by GDB when it doesn't know for sure that the next assembly instruction won't branch. It steps as many times as required until an address is reached which doesn't belong to the current line. Exactly this will happen when a pending interrupt triggers: A branch to the handler and voila, we're on a address that doesn't belong to the line we're stepping, so GDB stops. Only the step command is addressed by my patch: It first lets pending interrupts execute, then disables interrupts, steps over the instruction and finally re-enables the interrupts. I didn't realize you wanted to _execute_ interrupt handlers during step, just not break in them. That's rather brilliant and probably a good default behaviour. But it can also place a breakpoint at the next source line and send a continue command. GDB uses this strategy when it steps over function call, assuming it will return to the next instruction after the call. This is done to speed up things. Instruction stepping takes some milliseconds per step. When GDB uses this method to step, interrupt handlers don't disturb debugging flow because the target isn't stopped before return from the function call. The continue command isn't changed by my patch. In that case we must leave interrupts enabled. Right. Interrupts only get disabled during the step over the next instruction, they are enabled afterward, see above. This is exactly the problem with the GDB hooks to enable and disable the isr masking during stepping. They disable the interrupts in both cases because they act on the GDB step command, not the target step command. My patch acts on the target step command only. The GDB hooks are meant for a different use-case. Sometimes you want to step around and at the same time guarantee that no isr or other code will be run behind your back. Just making sure that the debugger breaks on the expected line may not be enough. That use-case is still available exactly as today, with GDB hooks and maskisr on/off. Which is good. Wouldn't that mean an unpredictable difference in behaviour depending on how GDB chooses to do the step? No. In both cases pending interrupt handlers will be called and in both cases GDB stops on the next line. I see. I assumed you wanted the same behaviour as the GDB hooks use-case. Note that I haven't yet looked at the patch so this may not be relevant. I would be glad if you can try it on a real use case. I've briefly checked the patch now. I won't probably have time to test it anytime soon. But basically it looks good. I guess there's no way around requiring setting a breakpoint at the current pc to force any pending isr to run? Also, will it make stepping significantly slower? ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] transport selection fix
On Thu, Jun 16, 2011 at 8:02 PM, Tomek CEDRO tomek.ce...@gmail.com wrote: On Thu, Jun 16, 2011 at 5:50 PM, Øyvind Harboe oyvind.har...@zylin.com wrote: I take it is just a bugfix? Yes - it changes: retval = allow_transports(CMD_CTX, jtag_interface-transports ? : jtag_only); into: retval = allow_transports(CMD_CTX, jtag_interface-transports ? jtag_interface-transports : jtag_only); Which was obvious mistake or cautious precaution :-) :-) Not true! Your patch doesn't fix a bug, because there is none. a ? : b is equivalent to a ? a : b, unless evaluating a has side-effects or if a is volatile, since it's only evaluated once in the former case and twice in the latter. I don't think jtag_interface-transports is neither volatile nor a macro with side-effects. So, in the name of portability (?: is a GCC extension [1]) and clarity, I'd say this patch is valid, but not for the reasons given in the patch comment. /Andreas [1] http://gcc.gnu.org/onlinedocs/gcc/Conditionals.html ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] ft2232: fix memory leak in ft2232_large_scan()
On Thu, Jun 9, 2011 at 8:40 AM, Øyvind Harboe oyvind.har...@zylin.comwrote: Definitely! I'm holding off on this patch in favor of a patch that deletes cruft instead of decorating it :-) I have a soft spot for patches that delete code that nobody understands IF the function is actually used, the allocation is needed to have somewhere to put the data from ft2232_read(). But since the data is discarded and because I can't see how that could be useful (for an in scan), I'm inclined to believe that this ft2232_large_scan() is non-functional and never used. The only call site has a condition that the scan size must be larger than the ft2232 buffer (128KB). Maybe that's never the case in practice? I could try to dig around to see if there's a point in keeping the function around, otherwise post a patch that removes it. I don't know how much time I can spare though so it could be an idea to fix the obvious memory problem right away. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Outstanding patches = Fix: Correctly exit function: ft2232_init when an error occurred
On Thu, Jun 9, 2011 at 9:06 AM, Øyvind Harboe oyvind.har...@zylin.comwrote: On Thu, Jun 9, 2011 at 7:29 AM, Peter Stuge pe...@stuge.se wrote: Øyvind Harboe wrote: Nit, the construct below is a little bit fancy. I'd prefer splitting it over multiple lines to make it more accessible to the casual reader. That requires adding a lot of brackets. Or just use goto, please, which is clear, simple, well understood, and elegant all at once. :) I think goto is OK sometimes. When it is used to unwind the stack frame and free up resources, in lieu of exceptions and resource tracking it can clean up the code significantly. I'm all for gotos in error handling. In this simple case it wouldn't have made things cleaner and the patch would have grown. Also I didn't want to join the goto-vs-subfunction-flamewar-to-be, so a third option was convenient. :) Main objection is that people are not very used to the comma operator. I don't mind rewriting it using goto if the list feels that would be preferred. The patch uses code duplication and if statements inside the cleanup fn(if (buffer) free(buffer)), which can be avoided using exception handling and resource tracking. In the case of this patch, it looks like the if (ft2232_buffer) free(ft2232_buffer) is a case of conservative programming. I'd much rather have an assert in this case than to have broken code auto-repair and be harder to get right. Duplication? Rather code reuse. Ok, the repeated calls to the cleanup function (or the equivalent gotos, if you want to go that route) would have disappeared using exception handling. But C doesn't have it, so... The if statement was added to make ft2232_quit() a bit more generic so it could be used for all cleanup cases, including the error case where malloc fails. The alternative would have been to add a separate error handling path for that case. And given how often error paths are exercised, it's safer to keep them to a minimum. Note that I'm not holding off committing the patch for the above, as maintainer I'm interested in progress as well as more philosophical coding style discussions. :-) If someone does come forth with an even more cleaner patch, then I'm OK with that. This is now growing into a case-study of how to formulate a patch more than a fix. The openocd community has much to learn about how to formulate patches. As long as there is progress, I don't mind holding off the patch. I'm not in a hurry. There's a lot to do in the ft2232 driver. I have the intention to perform some general cleanup in the near future, will post some thoughts this evening maybe. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH 1/5] ft2232: Clean up infinite loop condition in ft2232_init()
On Thu, Jun 9, 2011 at 12:12 PM, Peter Stuge pe...@stuge.se wrote: - for (int i = 0; 1; i++) + for (int i = 0; ft2232_vid[i] || ft2232_pid[i]; i++) What's the point? This condition can never become false, or the function would have returned in the previous iteration. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH 2/5] ft2232: Refactor ft2232_init_*() into ft2232_initone()
On Thu, Jun 9, 2011 at 12:25 PM, Peter Stuge pe...@stuge.se wrote: + retval = ft2232_initone(ft2232_vid[i], ft2232_pid[i], more, try_more); This is a good start, but what's with the function name?? How about ft2232_init_{device,interface,ftdilib,hardware,handle,whatever} depending on what the function actually initializes. Another ugliness in my point of view is using the try_more parameter to convey information about the type of success/failure. That's what return values are for. And the more parameter apparently only selects whether the diagnostic should be a warning or an error. Seems the message belongs outside the function, making the parameter useless. This function is supposed to initialize hardware and shouldn't care if the caller wants to retry with another vid/pid or not. I'll put removing those two parameters on my todo-list unless you or someone else pitches in or feels strongly against it. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH 3/5] ft2232: Clean up on ft2232_init() errors after a device has been opened
This together with patch 5/5 is equivalent to my patch, only with gotos. Both patches are perfectly fine with me, although I prefer getting rid of the forward declaration as well. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH 2/5] ft2232: Refactor ft2232_init_*() into ft2232_initone()
On Fri, Jun 10, 2011 at 12:26 AM, Peter Stuge pe...@stuge.se wrote: Andreas Fritiofson wrote: + retval = ft2232_initone(ft2232_vid[i], ft2232_pid[i], more, try_more); This is a good start, but what's with the function name?? Short. And one from one device. As opposed to try to init *all* supported devices, which is what ft2232_init does. ft2232_init really refers to init of the openocd driver, while ft2232_init_{libftdi,ftd2xx} refers to init of the ftdi subsystem including hardware (using either of the libraries). Different inits, hence one and all is not the suitable distinction. How about ft2232_init_ftdi that could be interpreted as initializing the ftdi library, the ftdi chip or the ftdi handle, all of which are true? Another ugliness in my point of view is using the try_more parameter Oh for sure. It was like that when I came here. Maybe small steps is a good idea. Please do send more patches to do further cleanup. Seems the message belongs outside the function, making the parameter useless. Note that I know nothing of the code style in OpenOCD. At least the ft2232 driver seems to consistently output error messages at the lowest level in the call stack, as opposed to letting the highest level (user interface) determine how the error should be handled. I found this quite awkward, but I wanted to make changes that were already some improvements over what was there. I am not saying the patches make things perfect, there's certainly still more to do! The last paragraph was not really a comment on this patch, which is good to go if the name is changed, but rather suggestions for further improvements in the related parts of the code. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Outstanding patches = Fix: Correctly exit function: ft2232_init when an error occurred
On Thu, Jun 9, 2011 at 10:17 PM, Tomek CEDRO tomek.ce...@gmail.com wrote: Hello Laurent! :-) On Thu, Jun 9, 2011 at 6:21 AM, Laurent Gauch laurent.ga...@amontec.com wrote: But before to close the handle we should still be able : -1- to tristate signal High-Z via all the out enable signals used in the specific layout, BUT BUT BUT ONLY if we have entered in the MPSSE mode. The whole driver works in MPSSE mode... (If you have a Amotnec JTAGkey-2 you may verifiy this by checking the external yellow LED. This LED should be OFF after any quit/shutdown) As I explained with the TRST and OMAP in previous message ... I have created simple quit() that sets all port pins high and as input (mpsse command 0x80 0x82) but that did not shut down the yellow led :-( Since the suitable setting depends on the mapping of the GPIOs and the surrounding electronics, the quit function would probably need to be layout specific, as Laurent mentioned. Do you have any other ideas on what should be done on ft2232_quit() except rtck, hi-z, div5..? I can prepare a patch, or Peter will as I can see he is already working out the subject :-) What problem is setting pins to a default state on exit supposed to solve? Are we sure this is what we want? If I make openocd pull the reset line and then quit openocd, I'd expect the target to remain in reset. If I quit openocd while the target is running, I'd expect it to keep on running. How would that work if we make this change? /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Outstanding patches = Fix: Correctly exit function: ft2232_init when an error occurred
On Fri, Jun 10, 2011 at 1:40 AM, Tomek CEDRO tomek.ce...@gmail.com wrote: Hello Andreas :-) On Thu, Jun 9, 2011 at 11:30 PM, Andreas Fritiofson andreas.fritiof...@gmail.com wrote: I have created simple quit() that sets all port pins high and as input (mpsse command 0x80 0x82) but that did not shut down the yellow led :-( Since the suitable setting depends on the mapping of the GPIOs and the surrounding electronics, the quit function would probably need to be layout specific, as Laurent mentioned. I think setting all pint as input create Hi-Z for them, so his is the safest choice and _should_ produce situation as if the interface was not connected at all... unless interface use some buggy buffer construction where high impedance would cause output to be active. I thought that was what Laurent mentioned... it sounds sensible, but maybe I did some error somewhere... Setting the FTDI pins to Hi-Z is not necessarily the same as setting the pins in the JTAG connector to Hi-Z. There is arbitrary dongle-specific interface logic between the FTDI chip and the connector. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] ft2232 layout refactoring plans
Hi all! The recent activity surrounding the ft2232 driver makes it obvious that there are lots of bugfixes and structural improvements that need to be done. Unfortunately, trying to understand the code or even just browsing around in it is complicated by the fact that its cluttered with functions related to the different layouts. I plan to break out all the layout specific code to a separate file, making the remaining core ft2232 code easier to work with. The question is, what's a suitable interface between the layout functions and the core ft2232 functions? As a matter of fact I made the separation one and a half years ago, but never got around to completing the patchset. At the time, everything the layout functions required from the core ft2232 code boiled down to appending to the ft2232 command buffer. Pretty clean and probably a suitable interface, assuming things haven't changed too much. The black sheep, now and then, is the signalyzer layout. Its layout functions uses the ftd2xx API directly, requiring access to the ftdi handle and probably a bunch of other data. Many functions are unimplemented for libftdi. It is also an exercise in code duplication with lots of repetitive snippets of code. Trivial refactoring could reduce the number of lines by half or so. Questions to this list: 1) Since this means moving a lot of code around, it's prudent to ask if anyone have ongoing work that could interfere with a change like this. 2) Any thoughts on the interface between the layout functions and the core ft2232 code? 3) What to do with the signalyzer code? I have no idea what this interface is. Is it used at all? Should we adapt 2) to its requirements or let it remain in ft2232.c? Maybe split it out to a completely separate driver, if it's so different? /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Outstanding patches = Fix: Correctly exit function: ft2232_init when an error occurred
On Fri, Jun 10, 2011 at 2:18 AM, Tomek CEDRO tomek.ce...@gmail.com wrote: On Thu, Jun 9, 2011 at 11:46 PM, Andreas Fritiofson andreas.fritiof...@gmail.com wrote: Setting the FTDI pins to Hi-Z is not necessarily the same as setting the pins in the JTAG connector to Hi-Z. There is arbitrary dongle-specific interface logic between the FTDI chip and the connector. Not true. Good example here can be KT-LINK SWD+JTAG buffers [1] configuration designed by Krzyszfot Kajstura - all signals can be electrically disconnected from target and they are in default configuration (after powerup). This is really nice design, also the first open SWD+JTAG for FT2232H chip. Interface can also offer additional signals such as various resets and control that also should be disconnected, especially for other transports. Upcoming SWD framework make use of existing ft2232 code, also the ft2232_quit() function, so there is a default possible behavior common to all ft*232-based devices. Thats not that complicated, isn't it? What are you talking about? Not all ftdi dongles are wired like this = there is no universal default setting suitable for all dongles. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] ft2232 layout refactoring plans
On Fri, Jun 10, 2011 at 2:36 AM, Tomek CEDRO tomek.ce...@gmail.com wrote: Uhm, from what I understand LAYOUT is only used for initialization of specific device based on FT*232 device, maybe other minor stuff, all work is done in fact by queue_flush() and it is common for all devices as it simply translates queued JTAG commands into MPSSE that produces actions on the physical connection between interface and target TAP. Yes they are separate concepts, that's why there's a point in separating them. Having everything in one file is okay for me as there is only one place to look for stuff. Making more files won't increase readability imo its just moving things from one place to another, like women cleanup, then you never know where to look for your stuff ;-P More files won't increase readability, true. Fewer lines of unrelated code will. ft2232.c contains 4434 lines. 2211 of these are layout related, and spread all over the file. 788 of these are for one specific layout, namely signalyzer_h. /Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Outstanding patches = Fix: Correctly exit function: ft2232_init when an error occurred
On Tue, Jun 7, 2011 at 5:19 PM, Andreas Fritiofson andreas.fritiof...@gmail.com wrote: I suggest to drop this patch and merge a simpler fix for the problem adressed by this patch. Only a few lines of code would need to change. Attached is such a patch. The benefit over Tomek's patch is that it frees not only the allocated buffer but also closes the ftdi handle (as was done in the original patch). Plus I'm not very fond of the infinite-loop-that's-not-actually-a-loop-at-all along with the extra indentation. This patch doesn't use gotos either but another (perhaps even more) neglected construct, like it or not. :) /Andreas From c934a6c697c90fd5b5211f744e6607ae9cb7b848 Mon Sep 17 00:00:00 2001 From: Andreas Fritiofson andreas.fritiof...@gmail.com Date: Wed, 8 Jun 2011 23:58:51 +0200 Subject: [PATCH] ft2232: clean up on error in ft2232_init() Check malloc return value. Close handle and free buffer if init fails late. Reuse ft2232_quit() for this so move it up to avoid a forward declaration. Don't set retval if it's not used. Signed-off-by: Andreas Fritiofson andreas.fritiof...@gmail.com --- src/jtag/drivers/ft2232.c | 53 1 files changed, 29 insertions(+), 24 deletions(-) diff --git a/src/jtag/drivers/ft2232.c b/src/jtag/drivers/ft2232.c index 8c2382a..a3b87c3 100644 --- a/src/jtag/drivers/ft2232.c +++ b/src/jtag/drivers/ft2232.c @@ -2418,6 +2418,25 @@ static int ft2232_set_data_bits_high_byte( uint8_t value, uint8_t direction ) return ERROR_OK; } +static int ft2232_quit(void) +{ +#if BUILD_FT2232_FTD2XX == 1 + FT_STATUS status; + + status = FT_Close(ftdih); +#elif BUILD_FT2232_LIBFTDI == 1 + ftdi_usb_close(ftdic); + + ftdi_deinit(ftdic); +#endif + + if (ft2232_buffer) + free(ft2232_buffer); + ft2232_buffer = NULL; + + return ERROR_OK; +} + static int ft2232_init(void) { uint8_t buf[1]; @@ -2467,9 +2486,11 @@ static int ft2232_init(void) ft2232_buffer_size = 0; ft2232_buffer = malloc(FT2232_BUFFER_SIZE); + if (!ft2232_buffer) + return ft2232_quit(), ERROR_JTAG_INIT_FAILED; if (layout-init() != ERROR_OK) - return ERROR_JTAG_INIT_FAILED; + return ft2232_quit(), ERROR_JTAG_INIT_FAILED; if (ft2232_device_is_highspeed()) { @@ -2482,21 +2503,23 @@ static int ft2232_init(void) #endif /* make sure the legacy mode is disabled */ if (ft2232h_ft4232h_clk_divide_by_5(false) != ERROR_OK) - return ERROR_JTAG_INIT_FAILED; + return ft2232_quit(), ERROR_JTAG_INIT_FAILED; } buf[0] = 0x85; /* Disconnect TDI/DO to TDO/DI for Loopback */ - if ((retval = ft2232_write(buf, 1, bytes_written)) != ERROR_OK) + if (ft2232_write(buf, 1, bytes_written) != ERROR_OK) { LOG_ERROR(couldn't write to FT2232 to disable loopback); - return ERROR_JTAG_INIT_FAILED; + return ft2232_quit(), ERROR_JTAG_INIT_FAILED; } #if BUILD_FT2232_FTD2XX == 1 - return ft2232_purge_ftd2xx(); + retval = ft2232_purge_ftd2xx(); #elif BUILD_FT2232_LIBFTDI == 1 - return ft2232_purge_libftdi(); + retval = ft2232_purge_libftdi(); #endif + if (retval != ERROR_OK) + return ft2232_quit(), ERROR_JTAG_INIT_FAILED; return ERROR_OK; } @@ -3220,24 +3243,6 @@ static void flossjtag_blink(void) buffer_write(high_direction); } -static int ft2232_quit(void) -{ -#if BUILD_FT2232_FTD2XX == 1 - FT_STATUS status; - - status = FT_Close(ftdih); -#elif BUILD_FT2232_LIBFTDI == 1 - ftdi_usb_close(ftdic); - - ftdi_deinit(ftdic); -#endif - - free(ft2232_buffer); - ft2232_buffer = NULL; - - return ERROR_OK; -} - COMMAND_HANDLER(ft2232_handle_device_desc_command) { char *cp; -- 1.7.0.4 ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH] ft2232: fix memory leak in ft2232_large_scan()
Free the buffer before return. Also check the malloc return value. Perhaps it's not a good idea to exit() on error, but it's in line with the rest of the function. Strange thing with this function is that the allocated buffer doesn't seem to be used for anything. The data read into it doesn't go anywhere. Maybe the entire function is flawed, or is the data really supposed to be discarded? Signed-off-by: Andreas Fritiofson andreas.fritiof...@gmail.com --- src/jtag/drivers/ft2232.c |8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/src/jtag/drivers/ft2232.c b/src/jtag/drivers/ft2232.c index a3b87c3..9fb4d48 100644 --- a/src/jtag/drivers/ft2232.c +++ b/src/jtag/drivers/ft2232.c @@ -1142,6 +1142,12 @@ static int ft2232_large_scan(struct scan_command* cmd, enum scan_type type, uint int retval; int thisrun_read = 0; + if (!receive_buffer) + { + LOG_ERROR(failed to allocate memory); + exit(-1); + } + if (cmd-ir_scan) { LOG_ERROR(BUG: large IR scans are not supported); @@ -1341,6 +1347,8 @@ static int ft2232_large_scan(struct scan_command* cmd, enum scan_type type, uint receive_pointer += bytes_read; } + free(receive_buffer); + return ERROR_OK; } -- 1.7.0.4 ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development