Re: [Openocd-development] [PATCH] make sure file name case of at91sam3uxx matches what other files include

2011-12-01 Thread Andreas Fritiofson
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

2011-11-25 Thread Andreas Fritiofson
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

2011-11-02 Thread Andreas Fritiofson
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

2011-11-02 Thread Andreas Fritiofson
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

2011-11-02 Thread Andreas Fritiofson
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)

2011-10-29 Thread Andreas Fritiofson
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?

2011-10-19 Thread Andreas Fritiofson
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?

2011-10-19 Thread Andreas Fritiofson
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

2011-10-19 Thread Andreas Fritiofson
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

2011-10-09 Thread Andreas Fritiofson
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?

2011-10-09 Thread Andreas Fritiofson
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?

2011-10-09 Thread Andreas Fritiofson
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

2011-10-08 Thread Andreas Fritiofson
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

2011-10-08 Thread Andreas Fritiofson
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

2011-10-08 Thread Andreas Fritiofson
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

2011-10-07 Thread Andreas Fritiofson
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)

2011-10-07 Thread Andreas Fritiofson
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.

2011-10-06 Thread Andreas Fritiofson
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

2011-10-06 Thread Andreas Fritiofson
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

2011-10-06 Thread Andreas Fritiofson
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

2011-10-06 Thread Andreas Fritiofson
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

2011-10-06 Thread Andreas Fritiofson
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

2011-10-06 Thread Andreas Fritiofson
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()

2011-10-06 Thread Andreas Fritiofson
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

2011-09-10 Thread Andreas Fritiofson
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

2011-08-30 Thread Andreas Fritiofson
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

2011-08-30 Thread Andreas Fritiofson
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

2011-08-30 Thread Andreas Fritiofson
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

2011-08-30 Thread Andreas Fritiofson
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

2011-08-30 Thread Andreas Fritiofson
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

2011-08-26 Thread Andreas Fritiofson
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

2011-08-26 Thread Andreas Fritiofson
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

2011-08-23 Thread Andreas Fritiofson
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)

2011-08-16 Thread Andreas Fritiofson
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)

2011-08-15 Thread Andreas Fritiofson
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!

2011-08-12 Thread Andreas Fritiofson
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

2011-08-09 Thread Andreas Fritiofson
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

2011-08-09 Thread Andreas Fritiofson
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

2011-08-05 Thread Andreas Fritiofson
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

2011-08-05 Thread Andreas Fritiofson
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

2011-08-05 Thread Andreas Fritiofson
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

2011-08-03 Thread Andreas Fritiofson
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

2011-08-03 Thread Andreas Fritiofson
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

2011-07-30 Thread Andreas Fritiofson
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

2011-07-30 Thread Andreas Fritiofson
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

2011-07-29 Thread Andreas Fritiofson
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

2011-07-29 Thread Andreas Fritiofson
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

2011-07-29 Thread Andreas Fritiofson
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

2011-07-26 Thread Andreas Fritiofson
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

2011-07-26 Thread Andreas Fritiofson
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

2011-07-22 Thread Andreas Fritiofson
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

2011-07-22 Thread Andreas Fritiofson
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

2011-07-22 Thread Andreas Fritiofson
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

2011-07-22 Thread Andreas Fritiofson
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?

2011-07-22 Thread Andreas Fritiofson
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

2011-07-21 Thread Andreas Fritiofson
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

2011-07-19 Thread Andreas Fritiofson
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

2011-07-18 Thread Andreas Fritiofson
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

2011-07-18 Thread Andreas Fritiofson
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

2011-07-18 Thread Andreas Fritiofson
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

2011-07-18 Thread Andreas Fritiofson
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

2011-07-18 Thread Andreas Fritiofson
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

2011-07-18 Thread Andreas Fritiofson
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

2011-07-18 Thread Andreas Fritiofson
__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

2011-07-18 Thread Andreas Fritiofson
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

2011-07-18 Thread Andreas Fritiofson
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

2011-07-18 Thread Andreas Fritiofson
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

2011-07-18 Thread Andreas Fritiofson
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

2011-07-18 Thread Andreas Fritiofson
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

2011-07-18 Thread Andreas Fritiofson
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

2011-07-18 Thread Andreas Fritiofson
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

2011-07-18 Thread Andreas Fritiofson
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

2011-07-18 Thread Andreas Fritiofson
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

2011-07-18 Thread Andreas Fritiofson
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

2011-07-08 Thread Andreas Fritiofson
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

2011-07-08 Thread Andreas Fritiofson
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

2011-07-08 Thread Andreas Fritiofson
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

2011-07-08 Thread Andreas Fritiofson
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

2011-07-08 Thread Andreas Fritiofson
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

2011-07-08 Thread Andreas Fritiofson
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

2011-07-06 Thread Andreas Fritiofson
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)

2011-07-06 Thread Andreas Fritiofson
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

2011-06-29 Thread Andreas Fritiofson
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

2011-06-28 Thread Andreas Fritiofson
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

2011-06-18 Thread Andreas Fritiofson
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

2011-06-18 Thread Andreas Fritiofson
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

2011-06-16 Thread Andreas Fritiofson
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()

2011-06-09 Thread Andreas Fritiofson
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

2011-06-09 Thread Andreas Fritiofson
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()

2011-06-09 Thread Andreas Fritiofson
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()

2011-06-09 Thread Andreas Fritiofson
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

2011-06-09 Thread Andreas Fritiofson
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()

2011-06-09 Thread Andreas Fritiofson
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

2011-06-09 Thread Andreas Fritiofson
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

2011-06-09 Thread Andreas Fritiofson
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

2011-06-09 Thread Andreas Fritiofson
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

2011-06-09 Thread Andreas Fritiofson
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

2011-06-09 Thread Andreas Fritiofson
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

2011-06-08 Thread Andreas Fritiofson
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()

2011-06-08 Thread Andreas Fritiofson
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


  1   2   >