Re: [Openocd-development] git gui/cola doesn't push to review

[Peter Stuge]

Mathias K. wrote:
> If i try to push i can choose the review as target but it will
> always pushed into the master and i got a error message.
> The console command "git push review" works. Any hints?

Can you choose which local branch to push, and which branch on the
remote that it should be pushed into?

The key is to push into: refs/for/master

One canonical command line for this would e.g. be:

git push review HEAD:refs/for/master


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Gerrit mail subject

[Peter Stuge]

Mathias K. wrote:
> >> can you remove the magic incomplete number too?
> >>
> >> In this case the "1533d9d". This number is useless in the subject:
> > 
> > I disagree. This is an abbreviated commit hash, which identifies the
> > commit that was pushed. There's also Gerrit's identifiers, but losing
> > the commit hash would be impractical.
> 
> Okay, is it possible to change the commit message on a commit with the
> same commit hash?

No, besides contents the hash also depends on commit time and commit
message. That's exactly why the hash is useful, to identify the
particular commit that is refered to.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] scan-build and gerrit rant

[Peter Stuge]

Andreas Fritiofson wrote:
> >clang: fix warning by adding assert that shows that a variable is used
..
> >
> > +   assert(e == JIM_OK);
> > +
..

> I'm not very fond of the idea of merging patches with the sole
> purpose of fixing scan-build false positives.

+1


> 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.

Yes, I agree. I'm not at all fond of throwing assert() at the clang
warnings.


Thanks

//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] openocd patch: d171eb8 target config files: Fix whitespace issues.

[Peter Stuge]

ger...@openocd.zylin.com wrote:
> +++ b/tcl/target/atmega128.cfg
> @@ -1,12 +1,12 @@
>  # for avr
>  
> -   set  _CHIPNAME avr
> -   set  _ENDIAN little
> +   set _CHIPNAME avr
> +   set _ENDIAN little

Why keep indent here?

>  # jtag speed
>  adapter_khz 4500
>  
> -reset_config  srst_only
> +reset_config srst_only
>  adapter_nsrst_delay 100
>  
>  #jtag scan chain

..when the rest of the file seems unindented?

The same might exist in other files too.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Gerrit mail subject

[Peter Stuge]

Mathias K. wrote:
> can you remove the magic incomplete number too?
> 
> In this case the "1533d9d". This number is useless in the subject:

I disagree. This is an abbreviated commit hash, which identifies the
commit that was pushed. There's also Gerrit's identifiers, but losing
the commit hash would be impractical.


> [Openocd-development] openocd patch: 1533d9d cfi: unsupported code paths are 
> now covered by asserts
> 
> Now we have 2 times openocd as static text left. Do you think we
> need it?

My idea is obviously that we'll remove the first one.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Gerrit mail subject

[Peter Stuge]

Jon Povey wrote:
> > is there any chance to reduce the gerrit subject line to a
> > normal length with more information at
> > the beginning? Currently the subject consist of 50% static text
> > at the beginning.
> >
> > [Openocd-development] New patch to review for openocd: XXX bugfixes:
> 
> I agree. maybe one or two characters of the commit-id are all I see
> before the subject gets chopped off on my display.

I just changed it to New openocd patch: xxx


> In fact I would get rid of [Openocd-development] too.

+1 but since this is a bit more established maybe give people time to
change their filters? Or just change it and people will change after?


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] New patch to review for openocd: e043913 bugfixes: numerous bugs in error propagation found by clang

[Peter Stuge]

Øyvind Harboe wrote:
> > I don't like the change so much. It ends up declaring the exact same
> > variable twice, and other odd noise, to silence a warning. Is there
> > no better way?
> 
> Reusing the 'target' variable name here is bad.
..
> The code is now more easily re-factored as multiple functions

Ok, can we please do that then, instead of working around static
analysis? Also, I'm not sure the command handler should really
return ERROR_OK if the target is unknown. (Not a new bug, but I
think it's a bug just the same.)


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] New patch to review for openocd: e043913 bugfixes: numerous bugs in error propagation found by clang

[Peter Stuge]

Øyvind Harboe wrote:
> 2011/10/28 Peter Stuge :
> > ger...@openocd.zylin.com wrote:
> >> -DumpTargets:
> >> +DumpTargets:;
> >
> > Hm?
> 
> Syntactically a declaration can't follow a label, so add an empty
> statement.

I don't like the change so much. It ends up declaring the exact same
variable twice, and other odd noise, to silence a warning. Is there
no better way?


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] New patch to review for openocd: e043913 bugfixes: numerous bugs in error propagation found by clang

[Peter Stuge]

ger...@openocd.zylin.com wrote:
> -DumpTargets:
> +DumpTargets:;

Hm?


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Can't push...

[Peter Stuge]

jim norris wrote:
> Pushing to ssh://jnor...@openocd.zylinx.com:29418/openocd.git

Typo, there's an extra x in the domain name. Run:

git config remote.review.url ssh://jnor...@openocd.zylin.com:29418/openocd.git

And then try git push review again.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] git gui

[Peter Stuge]

Pete Batard wrote:
> Git is just a tool, feel free to use it as you see fit, GUI or not.

Yes and no. Git is a tool, and of course there are good and bad GUIs,
but the point is that since it's common to work together with others,
and since this is also the setting where git really shines, it's a
good idea to make sure that one's use of git is a) useful for others
and b) the most efficient for self.

It's not a good idea to use git as one sees fit if that ends up being
unfit for others, and it's of course not a good idea to use anything
in an inefficient way.


> But please remember that, like all tools, it has its advantages and 
> limitations.

The only real limitation that I've come across after a few years is
that git really sucks at dealing with a large number of refs
(branches+tags).

I think it's difficult to find a setting where git doesn't offer some
significant advantages over other common choices. The flip side is
that because of those advantages git can also be very different from
said other choices, and that can in itself be enough of a problem to
cancel out all advantages. It depends on the people involved.


> Also don't try impose your views on others with regards to git 
> usage, unless you really have something to say about the quality of
> the patches they submit to mainline.

To clarify for those not following libusb, and to take a concrete
example: Pete's public libusb repo isn't rebased on the main libusb
repo merge queue, but is rather a fork of the main repo from when
Pete started working on libusb.

Pete applies commits from the merge queue and also generates patches
against the merge queue, no problem, except that the patches Pete
generates aren't in his repo. The repository gap is big, which is
a problem. It means that a) Pete has to do some work twice (first
for own repo, then for main repo) and b) everyone else can't really
relate Pete's ongoing work to the main repo using git, only using
patches or in terms of git diff output between files or blobs; no fun.

As with Gerrit, or when using git send-email (like Linux kernel
development) or with git push to say github, where work is based on
other work by someone else, git can be an integral part of the
workflow. In those cases it *is* important how git is being used
locally, because the output from local work is used as input by
others, and a common view is not only reasonable and accurate but
in fact neccessary.

Normal rebase is fundamental for interacting with other efforts.
Interactive rebase is fundamental for reworking patches based on
feedback. Both tasks are common, so I agree with Øyvind that these
git features need to be understood and practised for any sort of fun
and profit with git, and the git UI of choice has to support them.

We can blame git for bad situations that arise when people use it -
Pete's public libusb repo is just one example, the same situation
exists e.g. with Linux kernel ARM trees and within many corporations
with organizational problems - but git doesn't create gaps and forks;
people create gaps and forks, and it typically happens when people
use git as they see fit instead of according to the view of others.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Another git question....

[Peter Stuge]

Øyvind Harboe wrote:
> On Wed, Oct 26, 2011 at 3:09 PM, jim norris  wrote:
> >
> >
> >  Assuming I've done my commits. How do I see what the
> >  patch will look like before I do the 'git push review'?
> 
> git format-patch origin/master
> 
> => produces patch files

I would recommend using git log rather than generating patch files,
since the output can be coloured to very clearly mark whitespace
issues and since no files need to be created.

git log -p origin/master..


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] git gui

[Peter Stuge]

Øyvind Harboe wrote:
> my main problem has been that that I've found that those that use Tortoise
> have no patience for the complexities and necessity of interactive rebase,
> rather than Tortoise lack of support for this feature.

This might fit Pete. In another project I've learned that he prefers
not to use many of the features offered by git, among others indeed
interactive rebase and the quick and easy way to deal with branches -
and in that case TortoiseGit can of course work fine.

I've evaluated Tortoise together with multiple developers who use
Windows and it never ended up as the prefered tool, simply because
it didn't have wanted features. If it has all features *you* want
then of course it's as good a pick as any other, but at the time
we looked at it, it wasn't really useful for taking advantage of git,
and everyone picked Git Extensions or git bash instead. Some of the
guys especially like that Git Extensions integrates at all with
Visual Studio.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] git gui

[Peter Stuge]

jim norris wrote:
> For those using a git gui, what are you using?

On which system?

For Windows, there is Git Extensions and TortoiseGit. The Git
Extensions looked less slick than TortoiseGit last time I looked, but
TortoisGit on the other hand lacked fundamental functionality.

For Mac, there is GitX which is kinda nice, but also did not have
enough functionality last time I looked. There was some payware which
looked nice, but I didn't try it and don't recall the name.

I find that using the CLI is by far the easiest, and all features are
available.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] [PATCH] ft2232: Set PWR_RST and LOOPBACK for xds100v2

[Peter Stuge]

Mathias K. wrote:
> 2.) create a ssl certificate
> and set it in your profile at gerrit and make it available to your local ssh 
> client

SSH private key - not an SSL certificate.

See also
http://openocd.zylin.com/gitweb/?p=openocd.git;a=blob;f=HACKING


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] jimtcl compile error

[Peter Stuge]

billium wrote:
> I am trying to update openocd but it fails to build:
> 
> In function `Jim_Nvp_name2value_obj':
> jim-nvp.c:(.text+0xc7): undefined reference to `Jim_String'
> 
> This is repeated four times for other instances of Jim_String.
> 
> I am using amd64 on Gentoo, with the 999 ebuild which pulls from git.
> Anybody come across this or know what I am doing wrong?

I recall something about the nvp extension moving from jimtcl into
OpenOCD, which I think makes it impossible to package the two
packages separately. This is no good.

Basically you can't build - until at least the ebuild but
possibly also upstream has been fixed. Of course you can submit a bug
report on bugs.gentoo.org for jimtcl. Please include the full build
log, as always when reporting problems.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Amontec - Out of business?

[Peter Stuge]

Hi Ananda,

Ananda Regmi wrote:
> Does anybody here know if Amontec - makers of JTAG Tiny - company
> went out of business or something?

I guess it's possible, but company owner Laurent posted to this very
mailing list a week or so ago.


> Here's the reason I asked. I bought a JTAG tiny from them like 3-4 weeks
> ago. The payment went through and I haven't heard back from them. I have
> tried to send multiple emails and I don't get any response back. If they
> are indeed out of business, I will have to buy another JTAG dongle. But,
> if they are still in business, I guess I don't have any other option
> than to warn people to not to deal with them.

Thanks for the info. Laurent and a guy that he hired to do some
development have participated on this mailing list before, so I
think they should also receive this, and will hopefully get back to
you soon.

Personally I really like the Versaloon dongle, since it is fairly
intelligent, and also open hardware, with open source firmware.

http://www.versaloon.com/

It works great with OpenOCD for JTAG, SWD I (still :/ ) haven't
tested yet.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] commit a62d8f22 causes double evaluation of $filename in src/helper/startup.tcl proc script

[Peter Stuge]

Freddie Chopin wrote:
>> But I disagree with reverting the commit. Fix the problem instead.
>
> What?
>
> So you don't know how to fix that, I don't know how to fix that, but
> we cannot revert a one-line change that introduced A BUG just
> because it (supposedly - I did not check) adds some functionality
> that was not actually discussed in detail EVER?

It is obvious that the commit is a significant improvement for life
in Tcl files, that needs no discussion. Thus, we really want it.

We can also assume that Steve did not pick this way to do the
improvement because he likes to bully Windows users, but because
it is the better way to reach the goal.

We have discovered a side effect of uplevel, so we should find out
how we can deal with that side effect, not abandon the improvement.


> Why can't we revert a change, fixing a bug and wait for a real 
> jimtcl-expert (Steve) to discuss other alternatives

Maybe you, like me, roughly know the capabilities of Tcl, even if you
are not an expert? It is a pretty basic language, so it is fairly
safe to assume that there is no alternative to uplevel.


> while in the meantime OpenOCD would work as expected for Windows
> users?

Of course I agree that it would be great to fix the problem quickly.
But abandoning the improvement is not the answer.


> "The problem" is that Windows paths now cannot be used normally and
> OpenOCD cannot be used as before.

It is trivial to work around this problem. Open src/startup.tcl and
remove uplevel #0 from line 58, and you are done. Since
src/startup.tcl is generated during build, git will not care that it
has been changed. Of course the workaround needs to be re-applied in
case one of the .tcl files that make up startup.tcl changes, by your
hand or by a commit you fetch.


> Global variables thing which this patch (supposedly) improves is
> just an improvement, because you could use globals before anyway,
> just had to add "global NAME" before.

I think it's a very desirable improvement, and even though it does
not directly affect what you and I have so far focused on in OpenOCD
(ie. not .tcl files) I guess you agree that it can make a difference
for those who do work with .tcl files.

More importantly, every user is exposed to the .tcl files, so
anything that allows to simplify or clarify them is real important.


> Please revert to fix a real problem (removing backslashes from
> Windows paths).

This is not the problem, this is just how double evaluation manifests
itself. Since when is treating the symptom any good?


> We can deal with improvement while at the same time OpenOCD works
> as it should.

Please do deal with the improvement right away. I don't expect it
will have to be very complicated. Meanwhile, the workaround is quite
trivial.


I'm more interested in knowing if not using uplevel also changes
which directory relative paths are relative to?


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] clang static analyzer

[Peter Stuge]

Øyvind Harboe wrote:
> I upgraded to Ubuntu 11.10 from Ubuntu 11.04 and now it works :-)
> 
> Looks like this is bleeding edge stuff...

Not really, it is more likely just not something that Canonical cares
so much about.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Command-line paths problem in Windows

[Peter Stuge]

Freddie Chopin wrote:
> Just as Peter suspected the problem is caused by commit 
> a62d8f2271312ba955e839509590f5a5975b1b49. I've verified that with 
> bisecting. I've also build version with that single commit reverted
> which worked as expected and as before.

Thanks for confirming that!


> I've pushed revert-patch for Gerrit review.

Nak.


> I believe it should be reverted for now but the general idea it
> introduced (easing use of global variables) is worth implementing
> but in some other way.

Not so easy. I disagree with reverting the patch without having a
clear plan for the real fix.

Not easy because it is not neccessarily possible to implement the
same functionality in any other way.

The proper solution requries Jim expertise which I have none, or
possibly Tcl expertise which I have very very little. If pressed for
a solution, I would look into a workaround for the double evaluation,
such as a way to escape any \ in $filename.

But I disagree with reverting the commit. Fix the problem instead.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Command-line paths problem in Windows

[Peter Stuge]

Xiaofan Chen wrote:
> The following does work now.
> C:\work\openocd\binary>openocd -f board\\ek-lm3s1968.cfg

-f has not changed in almost 5 years, and directly calls the script
procedure, which was changed by Steve's commit.

So maybe both things are caused by the commit after all.

There's actually an easier way to test further, without creating
branches, since the commit only changes interpreted code nothing
needs to be recompiled.

Edit src/startup.tcl (note! not the file that the commit changes, the
commit changes src/helper/startup.tcl, if that file is changed then
rebuild is neccessary) and remove uplevel #0 from line 58. The line
should only read:

source [find $filename]

This is the functional equivalent of reverting Steve's commit. I am
not proposing that we do this, but it helps understanding this code
better.

FWIW I like very much that -f in current git is using paths relative
to the directory where openocd is being run.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Command-line paths problem in Windows

[Peter Stuge]

Peter Stuge wrote:
> > C:\work\openocd\binary\bin>openocd.exe -f board//ek-lm3s1968.cfg
> > Open On-Chip Debugger 0.6.0-dev-snapshot (2011-10-21-21:26)
> > Licensed under GNU GPL v2
> > For bug reports, read
> > http://openocd.sourceforge.net/doc/doxygen/bugs.html
> > Runtime Error: embedded:startup.tcl:58: couldn't read file 
> > "C:workopenocinarin..
> > /board//ek-lm3s1968.cfg": No such file or directory
> > in procedure 'script'
> > at file "embedded:startup.tcl", line 58
> 
> So the commit that broke this is probably
> 
> commit a62d8f2271312ba955e839509590f5a5975b1b49

By the way, here is how I found this:

$ git grep -n proc.script|cat
src/helper/startup.tcl:57:proc script {filename} {
$ git blame -L 57,59 src/helper/startup.tcl|cat
3287b866 src/startup.tcl(oharboe 2008-07-16 20:20:15 + 57) 
proc script {filename} {
a62d8f22 src/helper/startup.tcl (Steve Bennett   2011-08-11 12:10:54 +1000 58) 
uplevel #0 source [find $filename]
3287b866 src/startup.tcl(oharboe 2008-07-16 20:20:15 + 59) }

And I confirmed that the commit was not in 0.5.0:

$ git tag --contains a62d8f22
$ 

Of course it is still possible that some other commit is the real
problem! But the uplevel name and the perfect commit message makes
it easy to guess that this change is responsible for the extra
evaluation.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Command-line paths problem in Windows

[Peter Stuge]

Xiaofan Chen wrote:
> For version 0.5.0, the following two commands work and I use only
> either of them.
> openocd.exe -f board\ek-lm3s1968.cfg
> openocd.exe -f board/ek-lm3s1968.cfg
> 
> For latest git, none of the following commands work.
> openocd.exe -f board\ek-lm3s1968.cfg
> openocd.exe -f board/ek-lm3s1968.cfg

The first is understandable, but the second not working may be a real
problem.


> Example running log.

When looking for string parsing errors it's critical that every byte
of log output is unchanged. I think some lines were wrapped in the
email, most likely by some email software. This makes debugging
impossible. Perhaps you can direct output to a text file and attach
it to an email?


> C:\work\openocd\binary\bin>openocd.exe -f board//ek-lm3s1968.cfg
> Open On-Chip Debugger 0.6.0-dev-snapshot (2011-10-21-21:26)
> Licensed under GNU GPL v2
> For bug reports, read
> http://openocd.sourceforge.net/doc/doxygen/bugs.html
> Runtime Error: embedded:startup.tcl:58: couldn't read file 
> "C:workopenocinarin..
> /board//ek-lm3s1968.cfg": No such file or directory
> in procedure 'script'
> at file "embedded:startup.tcl", line 58

So the commit that broke this is probably

commit a62d8f2271312ba955e839509590f5a5975b1b49
Author: Steve Bennett 
Date:   Thu Aug 11 12:10:54 2011 +1000

Evaluate 'script' in the global scope

Scripts sourced via 'script' should evaluate in the global
scope to make it easy to set and reference global variables.

Signed-off-by: Steve Bennett 


You could test by running:

git checkout -b beforesuspect a62d8f22^

In the git dir. This creates a branch called beforesuspect which
points to the commit preceding Steve's. Then build as usual. (make
distclean or so.)

If you then want to test that this commit created the problem then
you can run:

git checkout -b suspect a62d8f22

Which creates a branch called suspect, that points to Steve's commit.

Clean and build again.

When done, go back to the master branch, and clean up the temporary
ones:

git checkout master
git branch -d beforesuspect
git branch -d suspect


> > Double backslash seems to fix it.
> 
> That helps. The following commands work.
> openocd.exe -f ..\\board\\ek-lm3s1968.cfg

I guess the uplevel makes the parameter be evaluated by the
interpreter, meaning that all \ is unescaped.


> openocd.exe -f ..//board//ek-lm3s1968.cfg
> 
> The following command also works.
> openocd.exe -f ../board/ek-lm3s1968.cfg

So there are two differences in behavior versus 0.5:

1. \ in -f parameters get evaluated one time extra by Jim tcl
2. paths in -f parameters are now relative to a different directory,
   one level down from previously

2. is most likely caused by another commit.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Gerrit background

[Peter Stuge]

Xiaofan Chen wrote:
> >> "+1 Looks good to me, but someone else must approve
> >> 0 No score
> >> -1 I would prefer that you didn't submit this"
> >>
> >> If the contributor see a -1 and refer to the above,
> >> I do not think it is that encouraging. :-).

Do you already consider that "submit" is the Gerrit term for adding a
change into the public repository; in the above it does *not* refer
to the contributor sending the change into Gerrit?

The text in the labels is what a reviewer is saying to the project,
ie. maintainers. What the reviewer is saying to the contributor is
the text written as comments both inline within the change and in the
general comment field during review.

I think the wording makes sense when keeping in mind the terminology
and who the receiver of the different pieces of information is, but I
do agree that the text labels could be made more clear.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Change in openocd[master]: jlink libusb1 driver with libusb1 common driver interface.

[Peter Stuge]

Mauro Gamba wrote:
> I think to proceed in this way:
> 
> - Add to the configuration step the possibility to choose which usb
>   library to use.

Use pkg-config to check for all libraries that can work, and use
with the first one that is found. Check for libusb-1.0 first. You
will need to study more autoconf than you needed before. You can of
course also ask on the list if you are uncertain about something.


> - Create an abstraction layer to the usb library with all the
>   necessary functions for the drivers.

Yes and no. "all the neccessary functions" should not be created at
once. Start with an abstraction layer that works for one driver but
still allows upward compatibility with the other drivers. This may be
more difficult than you think. You will have to analyze each driver
more in depth, and I guess it will take a bit of time.

The above two items shall be in the first commit. Please do not wait
for the complete driver analysis before you make the commit, please
start with creating an abstraction layer that is sufficient for only
one driver, push that to Gerrit, and see what the feedback is. This
is OK to do even if you want to do more work on the commit, but it is
important to get review feedback early, so that you do not have to do
double work.

Of course, remember to pay close attention to simple details like
checking all possible errors and code style.


> - Modify the drivers to use the abstraction layer instead directly
>   the library.
> 
> Do you think it's reasonable?

Yes, but it is more work and more complicated, so it is even more
important than before to only start small and then gather feedback.


Thanks again for working on this! I'm really looking forward to your
commits!


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Gerrit background

[Peter Stuge]

Xiaofan Chen wrote:
> >> I think some changes are necessary.
> >
> > This is what you say with -1. 
..
> Then please change the wording of "-1"
> 
> "+1 Looks good to me, but someone else must approve
> 0 No score
> -1 I would prefer that you didn't submit this"
> 
> If the contributor see a -1 and refer to the above,
> I do not think it is that encouraging. :-).

I think it is very important to always write some explanations for
why a change should not be submitted (which is the Gerrit term for
adding the change into the public repository), so that the
contributor knows why the change is not yet acceptable.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Gerrit background

[Peter Stuge]

Xiaofan Chen wrote:
> "-1" is too harsh in the case of the J-Link libusb-1.0 patch.

I think it fits, because:

> I think some changes are necessary.

This is what you say with -1. You don't think this should be added
exactly as-is. It can sometimes be nice to use the comment field in
addition to inline comments, to expand a little in general on the
chosen scoring and what needs further improvement, if that isn't
clear from inline comments already.

-1 isn't "Never ever" but rather "Not yet". The "what is missing" has
to be explained manually, which I think is a very important step.
(Ie. writing concrete comments, as you did.)

I mean, there is no point to try to create a multi-choice selection
that will have one option for every case, because every change will
most likely need different fixes.

It's important for both contributors and reviewers to remember what
+1 and -1 each means; +1 says that the change should be included
exactly as it is, and -1 says that you want something to be improved
first.

Also remember that the score is only for the patch set, and not for
the entire change. After the commit has been changed locally, and a
new version of the commit has been pushed, it will be shown as a new
patch set, and that patch set will receive a new score.


//Peter
___
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

[Peter Stuge]

Akos Vandra wrote:
> I know what TRST is, but not SRST. Would it be the "normal" chip
> reset?

Right, system reset.


//Peter
___
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

[Peter Stuge]

Akos Vandra wrote:
> 1.  (in reply to freddie choppin)
> As far as I understand, reset halt is not supported on my target
> because srst pulls trst.

Note that this was set incorrectly for the lpc17xx chip. Double check
if the setting in openocd really actually matches your hardware. You
need to check both chip and the board schematic.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] [PATCH] Unused variables

[Peter Stuge]

Øyvind Harboe wrote:
> Some graphing I've been thinking about:
> 
> - plot a number that shows the redundancy in maintainers. I.e. are we
> relying on a single maintainer or are there multiple maintainers

I've so far taken infinitely many more commits from Gerrit into
openocd.git than before. Much more effort to deal with patch files.

Committed LOC is a function of review, but not completely accurate,
since review unfortunately sometimes also leads to nothing.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] [PATCH] Unused variables

[Peter Stuge]

Øyvind Harboe wrote:
> I'm thinking a bit about how I can draw up some graphs on this...

Graphing number of commits is easy, but graphing the review is
difficult.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] [PATCH] Unused variables

[Peter Stuge]

Jon Povey wrote:
> this additional barrier to contributing does put me and others off
> contributing in future.

Using Git is also a barrier for some, perhaps even for many. Gerrit
is new, so sure there will be resistance. Maybe sometime it will be
just as common as Git itself.

I think the learning curve for Git is much steeper than for Gerrit.

Please look at the updated HACKING file, or my previous email, for an
overview of the quick and simple steps to use Gerrit:

You need an OpenID from somewhere (let me know if you want one from me)
You need to register on a web page and pick a username
You need to set an HTTP password or upload a public SSH key

The above takes not two minutes.

Once that has been done, git push on your command line or in your GUI
sends commits to Gerrit.

Setting up Gerrit is a one-time thing, and it allows much quicker
workflow for you, other contributors, the reviewers and the
maintainers.

An interesting fact is that in the project where I've seen Gerrit
introduced there were several new contributors sending commits,
who had previously never been seen on the mailing list. I was
surprised, but in a good way!


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] arm-jtag-ew + swd

[Peter Stuge]

Michael Ashton wrote:
> > It might be really easy to make the device speak SWD.
> 
> Might be, but I needed to get something working quickly and
> reliably -- it's a commercial project. --mpa

I guess you already know that SWD support in OpenOCD is not so
complete.

If you still want to try to use OpenOCD with SWD then there are
basically three approaches you can try:

1. Continue what David Brownell worked on for FT2232-based adapters
2. Try Tomek's libswd, which I think works so far primarily with
   (only some?) FT2232-based adapters
3. Use a Versaloon (e.g. Versaloon Mini, see http://www.versaloon.com/ )
   and Simon's patches to OpenOCD

I've only done very brief testing with the Versaloon, but what I've
tested works perfectly (SWD programming using vsprog, not using
OpenOCD - and JTAG using OpenOCD). I'm unfortunately unsure exactly
of the current libswd+OpenOCD status.


//Peter
___
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

[Peter Stuge]

Andreas Fritiofson wrote:
> But what is the mwb 0x400FC040 0x01 doing?

MEMMAP
Bit 0  MAP  Reset value 0
0 Boot mode. A portion of the Boot ROM is mapped to address 0.
1 User mode. The on-chip Flash memory is mapped to address 0.

6. Debug memory re-mapping
-
Following chip reset, a portion of the Boot ROM is mapped to address
0 so that it will be automatically executed. The Boot ROM switches
the map to point to Flash memory prior to user code being executed.
In this way a user normally does not need to know that this
re-mapping occurs.

However, when a debugger halts CPU execution immediately following
reset, the Boot ROM is still mapped to address 0 and can cause
confusion. Ideally, the debugger should correct the mapping
automatically in this case, so that a user does not need to deal with
it.


So yes indeed this should be taken care of in openocd. And it should
be the default.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] [PATCH] Unused variables

[Peter Stuge]

Freddie Chopin wrote:
>> Please read HACKING and post to Gerrit.

I second this.


> Sorry, I don't have time to do so because:

Sorry, noone has time to deal with your patch.


> 1. The patch is trivial

It might seem so, but actually it's not. It has multiple logical
changes combined in a single commit, which is no good. Please
separate the return error into it's own commit, with it's own clear
commit message about how and why.


> 2. I use git in Linux hosted in VitualBox, where emailing patches
> kinda does not work

Øyvind could have written "push" instead of "post", since commits
go to Gerrit using git push, and not email. It's clear that you
misunderstood this, and it's clear that you didn't even look into
how commits would be sent to Gerrit, before arguing that you do not
have time to do so because it is too complicated.

Your VM needs network access. I guess it has that.

On another note, it's absolutely not neccessary for you to work with
OpenOCD source code in any particular operating system. If your
prefered environment is Windows then Git can perform line ending
translation for you, allowing you to use any Windows tools to work
with any project codebase.


> 3. My knowledge of Linux and Git is minimal

That's not a problem. If your willingness to gain knowledge of Git is
also minimal, *that* is a big problem, since you can notwork
efficiently within the project then.

Perhaps you realize that learning a little about Git makes a huge
difference in productivity. It's of course impossible to discuss this
with someone if they have already decided that they hate Git (you
might be surprised how common this is) or that they otherwise don't
want to learn the tool.

I would see OpenOCD as an opportunity to learn more about Git, and I
would be happy that I took the opportunity, since I can benefit from
it also outside of OpenOCD. If you run into trouble then please do
ask - there are several on the mailing list who are happy to help.


> So basically I can say that Gerrit posting would NOT work in my
> environment without some serious effort,

The effort in all it's seriousness is documented in HACKING. But here
are the steps again, summarized for your convenience:

* acquiring any OpenID account (let me know if you can't get one)
* registering on the Gerrit website
* configuring a username on Gerrit website
* (optional) uploading a public ssh key on Gerrit website
* adding the commit hook in your local repo
* re-committing your change (to get the Change-Id)
* git config remote.origin.url with ssh or http URL to Gerrit
* git push

The next time you've made a commit you run git push again, to send
commit(s) to Gerrit. I am confident that this is quite significantly
simpler than whatever workflow you currently use with patch files and
email software.


> while you can just accept that trivial patch or post it there in
> two seconds.

Except that you can not get feedback then. Since it is your patch it
is you who needs to push it to Gerrit.


> I have got a lot on my head now and no time to spare.

Yeah, it takes time to learn tools, but Gerrit is really not your
enemy.


> Make Gerrit accept patches via web interface (I see no reason why it 
> shouldn't allow to do so as it's web based)

IMO that would be stupid. You already have the commit in Git, so it
is really impossible to have a simpler interface than git push.


> and I'll be happy to post anything there - now it's just like a
> stone wall for me with milions of steps required

See above, and HACKING, for the million steps that are required.


> to send a patch once a year.

Next year you do it with one command. Since your VM can not send
email I know that you are not using git send-email, which would be
similarly simple as git push to Gerrit; you must be creating patch
files, and sending them manually in email. This is not efficient.

There's really no reason to be inefficient about something, even if
you only do it once a year, especially when the knowledge that allows
you to work more efficiently can benefit you for a whole year in
between.


> For me - a user who sometimes patches simple stuff or adds config
> files - Gerrit is an obstacle, nothing more.

I think you should look at how Gerrit actually works, and re-evaluate
your position.


Of course, being required to even think about a new tool is a slight
inconvenience for a new contributor, but hopefully you can help
document the process, or suggest simplifications, instead of only
complaining about how difficult it is before you have any experience.

On the flip side, the value of Gerrit is significant. Commits can
very quickly and easily come into Gerrit and go out of Gerrit into
the public openocd.git repo. (You may have noticed the command line
interface via SSH that Gerrit offers.) It's also very easy to make
detailed comments on commits. Øyvind mentioned that Gerrit also
brings a bit of quality assurance, for free, without human
interaction.

The slight added inconvenience is instantl

Re: [Openocd-development] arm-jtag-ew + swd

[Peter Stuge]

Michael Ashton wrote:
> On Wed, Oct 19, 2011 at 8:56 AM, Laurent Gauch wrote:
> > certainly olime waits on Segger JLINK SWD implementation in
> > openocd to let say their arm jtag ew has swd
> > certainly olime waits on Amontec JTAGkey-3 SWD implementation in
> > openocd to let say their 2232 has swd ...
> 
> Ha .. I see .. cheeky of them .. Well, I'm sending it back. --mpa

Don't listen too much to Laurent's complaints. Of course he will be
unhappy with competition.

It might be really easy to make the device speak SWD.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Change in openocd[master]: jlink libusb1 driver with libusb1 common driver interface.

[Peter Stuge]

Spencer Oliver wrote:
> http://openocd.zylin.com/33 adds libusb-1.0 support to the jlink.
> 
> Just wondering if anyone had any input on this ?

Yes, plenty.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Building OpenOCD for Windows

[Peter Stuge]

Freddie Chopin wrote:
> I guess it's right time for me to provide a new development version
> on my website...

Maybe you can also describe how you build them?


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Building OpenOCD for Windows

[Peter Stuge]

Attila Kinali wrote:
> Could someone give me a pointer on how to build current git HEAD on windows?
> None of the howtos on the net i've tried worked. Each and everyone
> failed at some point with errors that are out of my league (linking
> errors, libtool errors etc).

Post the actual errors and you may be able to get help.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] uploading patches without ssh to Gerrit

[Peter Stuge]

Øyvind Harboe wrote:
> Is there a way to upload patches to Gerrit w/o ssh?

You can also use HTTP once you have configured a password at
http://openocd.zylin.com/#settings,http-password

See notes on http://www.coreboot.org/Git#Accessing_the_repository


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] OpenOCD Gerrit Review

[Peter Stuge]

Jason wrote:
> 1.) Threading versions of a patch series together.

I think Gerrit already does this. Did you try it?


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] gerrit and OpenID

[Peter Stuge]

jim norris wrote:
> Is the setup of gerrit limited to only using OpenID's
> from google and yahoo at the present time?

Not at all, you can use any OpenID, but gerrit is picky and you have
to include http:// at the start of the OpenID URL.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] OpenOCD Gerrit Review

[Peter Stuge]

Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > how about using patchwoek simply as done for the kernel ML?
> > 
> > One significant benefit of Gerrit is that it integrates really
> > well with git. Gerrit takes input from git, and output from
> > Gerrit goes to git.
..
> I prefer to manage git by myself patchwork manage to patch workflow

Hm, how is using Gerrit different from "manage git by myself" ? I
mean, what do you do that Gerrit can not?


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] OpenOCD Gerrit Review

[Peter Stuge]

Jean-Christophe PLAGNIOL-VILLARD wrote:
> how about using patchwoek simply as done for the kernel ML?

One significant benefit of Gerrit is that it integrates really well
with git. Gerrit takes input from git, and output from Gerrit goes to
git. This helps everyone who is working with the code.

Patchwork is imo nice for documenting review, but it's mostly a
passive listener in the patch lifetime.

Since Gerrit integrates with git, it can even automate some things
that must be done manually with Patchwork.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] OpenOCD Gerrit Review

[Peter Stuge]

Jason wrote:
> Is there any intent to integrate gerrit back into the ML?

Sending email is easy. Receiving and parsing email is not as easy.
Thus anything actually worth calling integration takes a fair bit of
work.


> See [1] for an existing gripe.

I'm the Peter refered to there. I also like to do patch review in the
mailer rather than in the browser, but it is difficult to integrate
that with Gerrit. Instead of asking about intent, please participate
in actually doing this integration.


> The main difference I see between gerrit and a mailinglist is, for
> most users, a mailinglist is passive and consolidated.  A web
> interface requires a lot more interaction just to keep up.

Alain Mouette wrote:
> I simply don have the time to browse through Forums seekng out what
> might have been going on :) :)

Seriously guys. Gerrit has a web interface, but it sure as hell is
not a forum, nor like one. Please spend five minutes to learn what
the software actually does and how it works - if you find no good
resources or descriptions then please ask instead of making
assumptions.

You have probably already found out that Gerrit has an SSH interface
besides the web interface. The SSH interface is of course much better
suited for integration, but so far I don't think it's possible to
create inline comments through it, which is the main glitch for me
between what I can do in my mailer and what I could do with Gerrit
via SSH. On the other hand the data is better structured in Gerrit
which is worth much more for everyone who consumes my review.


Jason wrote:
> If I am out for several days, my openocd imap folder has everything that
> happened.  I browse the subjects, read the threads of interest, and move
> on.  In the gerrit lurkflow, there's quite a bit more clicking (how long
> ago was it I last checked the pending patch list?) to get the same
> information. I also have to remember to visit the site.

Again: Look at what the tool does and how it works! It is not a web
site. It is a code review tool which receives git commits.

I'm not at all against Gerrit sending lots of email to the list, the
only bug is that review happening on the list will miss out on the
point with Gerrit. But of course ideas about how to connect the
glitch are very welcome.


> A new user has to work significantly harder to maintain a prolonged
> interest in the project with gerrit (imnsho).

See other email about Gerrit sending email.


> On a personal note, I've found it much easier to contribute to u-boot
> and the kernel vice cyanogenmod.  The main reason for this is the lack
> of a mailinglist for CM (they use gerrit).

It is IMO not possible to replace a mailing list with a code review
tool. They serve different purposes and complement each other.


> Maybe I'm odd and a one-off, but I thought it was worth mentioning

Please give yourself more freedom in shaping how the project works.
If you like patches to still come to the mailing list then you just
have to say so, don't apologize. :) And FWIW I don't think anyone is
against it.


> I'd hate to see that stream of patches and comments dissappear. :-(

I'd really like a way to do inline comments via SSH. Can you think of
any ideas for how we could do this? Gerrit needs to know which line
in which file.

The quickest thing I can come up with would be to add a prefix on
each patch line sent in email, like 1:14 meaning file 1 this patch
touches, line 14 of either the patch or the file. It would cost a few
characters per line, but it would have the data needed for an inline
comment. Combine with a PGP signature (or send via SSH) and I think
it could work. Of course someone has to develop this software too.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] how to push patch series to gerrit?

[Peter Stuge]

Øyvind Harboe wrote:
> How do I push a patch series to gerrit?

Put the commits in a branch rooted in the main openocd.git repo
master branch, then push that branch into refs/for/master in gerrit.
(With Spencer's default config you will push it if it is currently
checked out and you run git push review)


> Does gerrit have a concept of patch series or are all patches
> islands?

If you push a series of commits to gerrit as per above then gerrit
will record dependency between the commits.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] OpenOCD Gerrit Review

[Peter Stuge]

Laurent Gauch wrote:
> Why using a commercial server and a commercial url for an open
> source project?

What is "a commercial server"?

A company has donated resources to the project for running Gerrit.

I don't think the URL that is used really matters. Do you? Really?

Perhaps you can focus on the actual service instead. Please ask on
the list if you encounter problems using the service, we'll try to
help you out.

The domain name is quite possibly temporary, but I find that since a
company has donated resources it's fine that their name is visible.


Please feel free to use the IP address instead, if you prefer:

http://68.169.55.149/


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] OpenOCD Gerrit Review

[Peter Stuge]

Spencer Oliver wrote:
> git remote add review ssh://usern...@openocd.zylin.com:29418/openocd.git
> git config remote.review.push HEAD:refs/for/master

When pushing, run:

git push review

Thanks to the remote.review.push setting this will push whatever is
currently checked out.

As always, use git log -p to review your changes before you push. You
can trivially find and fix simple stuff that nevertheless *will* hold
up your patch if you expect only others to do review.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] OpenOCD Gerrit Review

[Peter Stuge]

Spencer Oliver wrote:
> It is advised to keep each patch in its own branch.

I'll try to explain this a little.

If multiple commits are pushed, ie. if the branch being pushed has
more than one commit which is not in the main openocd.git repo, then
Gerrit will record that these commits form a chain of dependencies.

If this is not accurate, ie. if one or more commits are independent,
then they should be pushed in a separate branch, also based on what
is currently openocd.git master.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] OpenOCD Gerrit Review

[Peter Stuge]

Spencer Oliver wrote:
> One addition i forgot to mention.
> Gerrit uses a Change-Id to track the change, thsi is generated client
> side using a hook.
> 
> You will need to install this hook, we will look into a better solution
> scp -p -P 29418 usern...@openocd.zylin.com:hooks/commit-msg .git/hooks/

It is very important to use this hook in the local repo.

After commit and push a patch gets reviewed, and perhaps some changes
will be made, the commit will be amended or rebased, and then pushed
again.

If a commit already has a change-id, the hook will not add a new one.
Since the change-id remains identical, Gerrit is able to detect that
a new patch was pushed for the same logical change. This allows very
easy-to-follow review iterations over changes.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Fwd: [PATCH] Start porting of usb jtag drivers to libusb-1.0 instead of libusb.0.1

[Peter Stuge]

Xiaofan Chen wrote:
> > Please use pkg-config.
> 
> Please also consider where pkg_config is usually not available,
> like in the case of MinGW under Windows.

There's pkg-config also for MinGW, although maybe that is perhaps
more within a full MSYS environment. The question is whether OpenOCD
already has a standalone MinGW Makefile or not. If not (only
autoconf) then I think requiring pkg-config is fine as a build
dependency.


> >>  #if IS_WIN32 == 0
> >>
> >> -     usb_reset(dev);
> >> +     libusb_reset_device(devh);
> >
> > Unsure. libusb_reset_device() shouldn't really need to be used in the
> > common case..
> 
> I remember this was necessary last time some of us were
> testing OpenOCD with J-Link.

Ok.


> On the other hand, this may well be not necessary now
> but the best is to look at Segger J-Link utility and see
> if it issues a reset in the beginning or not.

This is a good idea. Also, usb_reset() may not be exactly the same as
libusb_reset_device().


> The following codes are obviously hacky but you can see that
> any changes in these codes deserve great scrutiny.
> 
>   /* BE ***VERY CAREFUL*** ABOUT MAKING CHANGES IN THIS
>* AREA!!!  The behavior of libusb is not completely
>* consistent across Windows, Linux, and Mac OS X platforms.
>* The actions taken in the following compiler conditionals may
>* not agree with published documentation for libusb, but were
>* found to be necessary through trials and tribulations.

I think this may be largely due to the fact that libusb-0.1 was
implemented in different projects for different platforms.

It is really important to me that we make sure to have libusb-1.0
behave the same across platforms, because it is the only way we can
claim to actually have a cross-platform library.

With the exception of the still young Windows code I think we are so
far doing a fairly good job. I believe the difference between Linux
and Mac OS X were larger in libusb-0.1, so I think this code in
OpenOCD can be simplified. Let's see.


> > Or does the driver support so many jlink devices with so many different
> > USB interface variants that there's no way to get around it?
> 
> That is right, there are different version of J-Link with different
> endpoints configuration.

With the same vidpid? Meh. :(


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Fwd: [PATCH] Start porting of usb jtag drivers to libusb-1.0 instead of libusb.0.1

[Peter Stuge]

Hi!

Mauro Gamba wrote:
> I start the porting of the usb jtag drivers to the libusb-1.0 library.

Awesome. Thank you!


> +++ b/configure.ac
> @@ -994,7 +994,7 @@ fi
>  if test $build_ft2232_libftdi = yes ; then
># We assume: the package is preinstalled in the proper place
># these present as 2 libraries..
> -  LIBS="$LIBS -lftdi -lusb"
> +  LIBS="$LIBS -lftdi -lusb-1.0"
>#
># Try to build a small program.
>AC_MSG_CHECKING([Build & Link with libftdi...])
> @@ -1055,8 +1055,8 @@ build_usb=no
>  if test $build_jlink = yes -o $build_vsllink = yes -o $build_usbprog = yes 
> -o \
>$build_rlink = yes -o $build_ulink = yes -o $build_armjtagew = yes
>  then
> -  AC_CHECK_HEADERS([usb.h],[],
> -  [AC_MSG_ERROR([usb.h is required to build some OpenOCD driver(s)])])
> +  AC_CHECK_HEADERS([libusb-1.0/libusb.h],[],
> +  [AC_MSG_ERROR([libusb.h is required to build some OpenOCD driver(s)])])

Please use pkg-config.

pkg-config(1) http://linux.die.net/man/1/pkg-config mentions the
autoconf macro to use:

PKG_CHECK_MODULES(VARIABLE-PREFIX,MODULES[,ACTION-IF-FOUND,[ACTION-IF-NOT-FOUND]])

as well as the added variables that will be available. Suggest use:

PKG_CHECK_MODULES([LIBUSB1], [libusb-1.0 >= 1.0.8])


> +++ b/src/Makefile.am
> @@ -82,13 +82,13 @@ endif
>  endif
>  
>  if USBPROG
> -LIBUSB = -lusb
> +LIBUSB = -lusb-1.0
>  else
>  if JLINK
> -LIBUSB = -lusb
> +LIBUSB = -lusb-1.0
>  else
>  if RLINK
> -LIBUSB = -lusb
> +LIBUSB = -lusb-1.0
>  else

These would use LIBUSB1_LIBS instead. You may have to AC_SUBST
LIBUSB1_CFLAGS and LIBUSB1_LIBS explicitly, I'm not sure if they will
travel over from autoconf to automake automagically.


> @@ -1426,7 +1425,7 @@ static struct jlink* jlink_usb_open()
>  
>  #if IS_WIN32 == 0
>  
> - usb_reset(dev);
> + libusb_reset_device(devh);

Unsure. libusb_reset_device() shouldn't really need to be used in the
common case..

(Also, libusb-1.0 works to some degree on Windows.)


> @@ -1448,10 +1447,10 @@ static struct jlink* jlink_usb_open()
>  
>  #endif
>  
> - /* usb_set_configuration required under win32 */
> - struct usb_device *udev = usb_device(dev);
> - usb_set_configuration(dev, udev->config[0].bConfigurationValue);
> - usb_claim_interface(dev, 0);
> +//   /* usb_set_configuration required under win32 */
> +//   struct usb_device *udev = usb_device(dev);
> +//   usb_set_configuration(dev, udev->config[0].bConfigurationValue);
> +//   usb_claim_interface(dev, 0);

The interface must always be claimed, but maybe this is done in the
common open function now?


> @@ -1460,27 +1459,46 @@ static struct jlink* jlink_usb_open()
..
> + dev = libusb_get_device(devh);
> + struct libusb_config_descriptor *config;
> + libusb_get_config_descriptor(dev, 0, &config);
> + 
> + const struct libusb_interface *inter;
> + const struct libusb_interface_descriptor *interdesc;
> + const struct libusb_endpoint_descriptor *epdesc;
> + 
> + for(int i=0; i<(int)config->bNumInterfaces; i++) 
> + for(int j=0; jnum_altsetting; j++) 
> + for(int k=0; k<(int)interdesc->bNumEndpoints; k++) 

Is this neccessary? Aren't the endpoint addresses always the same? Or
does the driver support so many jlink devices with so many different
USB interface variants that there's no way to get around it?


> +static int wrap_usb_bulk_write(libusb_device_handle *dev, int ep,
>  char *buff, int size, int timeout)
>  {
> + int transferred;
>   /* usb_bulk_write() takes const char *buff */
> - return usb_bulk_write(dev, ep, buff, size, timeout);
> + libusb_bulk_transfer(dev, ep, (unsigned char *)buff, size, 
> &transferred, timeout);
> + return transferred;
>  }

This should do error checking of the return value of
libusb_bulk_transfer(). Better add it right from the beginning, so
that it doesn't get overlooked anywhere.


> +++ b/src/jtag/drivers/rlink.c
..
> @@ -137,13 +138,15 @@ ep1_generic_commandl(
>   sizeof(usb_buffer) - (usb_buffer_p - usb_buffer)
>   );
>  
> - usb_ret = usb_bulk_write(
> + usb_ret = libusb_bulk_transfer(
>   pHDev_param,
>   USB_EP1OUT_ADDR,
> - (char *)usb_buffer, sizeof(usb_buffer),
> + (unsigned char *)usb_buffer, sizeof(usb_buffer), 
> &transferred,
>   USB_TIMEOUT_MS
>   );
>  
> + /* If command succesfull, return the nr of byte transferred */
> + if (usb_ret == LIBUSB_SUCCESS) usb_ret = transferred;
>   return(usb_ret);
>  }

Note that libusb-0.1 returns errno.h errors, while libusb-1.0 returns
libusb.h errors. They are not compatible, so depending on who calls
ep1_generic_commandl() and how they check the return value, you may
have to adjust that code too.


> @@ -194,9 +197,9 @@ ep1_memory_read(
>   break;
>   }
>  
> - usb_ret = usb_bulk_rea

Re: [Openocd-development] What's up? Why so slow?

[Peter Stuge]

Akos Vandra wrote:
> default reply address is the sender himself, and not the list?

Please see
http://woozle.org/~neale/papers/reply-to-still-harmful.html


Mailing lists are a third form of email communication pattern. (The
first two are person-to-person, and person-to-many-persons.) Using an
email software which does not have explicit support for mailing lists
makes life with lists very inconvenient. :\


//Peter
___
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?

[Peter Stuge]

Øyvind Harboe wrote:
> On Sun, Oct 9, 2011 at 3:40 AM, Peter Stuge  wrote:
> > Transfer rate: 3 KB/sec, 13848 bytes/write.
> 
> USB roundtrips?

Ḯ'll have a look with usbmon soon.


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.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

[Openocd-development] What's up? Why so slow?

[Peter Stuge]

EK-LM3S9B90 ICDI JTAG openocd git jtag_khz 6000

(gdb) load
Loading section .text, size 0x1e5f8 lma 0x0
Loading section .data, size 0xe0 lma 0x1e5f8
Start address 0xc63d, load size 124632
Transfer rate: 3 KB/sec, 13848 bytes/write.


Seriously, in 2011?


//Peter
___
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

[Peter Stuge]

Øyvind Harboe wrote:
> Can I have a show of hands of contributors for/against/don't care
> /don't know?

I'm also +1.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Fwd: BerliOS will be closed on 31.12.2011

[Peter Stuge]

Michel Catudal wrote:
>> I am personally very strongly against depending on companies based in
>> the US for open source project infrastructure. I find there are quite
>> significant drawbacks, due to stupid US law and/or due to the service
>> being provided by a business with a clear agenda, which makes it an
>> impossible choice for any self-respecting open source project.
>> Already the very very first condition of github terms and many others
>> completely rule it out IMO:
>
> The anti-American stand is pure stupidity. Find better arguments to
> discredit any site.

I think you misunderstood. It does not matter that a service provider
is American, it matters that the service provider is bound by US law,
because that means that the service can only be made available under
silly terms. I tried to make this clear with the example.


> As for assuming that an American company is likely to be anti
> opensource, it is ridiculous.

The purpose of every company in every country is to generate profit
for the company, which is in some cases perfectly compatible with the
aim of a given open source project, and in other cases not so much.
In any case, how the company purpose expresses itself can change
quickly, while self hosting has none of these problems and great
technical advantages in addition.


> Some people should stick to facts and stop taking their information
> out of their behind.

Yes, calling someone stupid and ridiculous is much better.


> Having worked on project for Sourceforge for many years I think
> that Sourceforge is one of the best site for Open Source

It's unclear what you mean here, but I think you mean that you worked
with development of SF infrastructure.

As I already wrote, large operators benefit from economy of scale.
For the case of SF mailing lists and the FRS (SF download hosting) I
think it is a great move to allow our project to take advantage of
this. But I think that the SF services that require a user account
are not nearly open enough for a project that wants to call itself
open.

Sorry if you are offended by this opinion.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Typo in src/flash/nor/lpc2000.c

[Peter Stuge]

Akos Vandra wrote:
> Shouldn't the line in src/flash/nor/lpc2000.c:333 be LOG_ERROR("BUG:
> unknown lpc2000_info->variant encountered"); ?
> I suspect copy pasting from the switch above it.

Copypaste errors are plenty. Could you send a patch to fix it?


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] software breakpoint with multicore systems

[Peter Stuge]

Hi,

Michel JAOUEN wrote:
> Here is a patch correcting this issue.

Content-Description: 0001-breakpoint-smp-software-breakpoint-correction.patch
> From 5fc6a3e930bdfde679b50f00cee98ba1273b8ee9 Mon Sep 17 00:00:00 2001
> From: Michel Jaouen 
> Date: Fri, 30 Sep 2011 18:42:52 +0200
> Subject: [PATCH] breakpoint : smp software breakpoint correction
> 
> ---
>  src/target/breakpoints.c |   14 +++---
>  src/target/target.c  |7 +++
>  2 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/src/target/breakpoints.c b/src/target/breakpoints.c
> index 5a0fc40..d7acf00 100644
> --- a/src/target/breakpoints.c
> +++ b/src/target/breakpoints.c
> @@ -228,6 +228,8 @@ int retval = ERROR_OK;
>   struct target_list *head;
>   struct target *curr;
>   head = target->head;
> + if (type == BKPT_SOFT)  breakpoint_add_internal(head->target, 
> address,length, type);
> + else
>   while(head != (struct target_list*)NULL)
>   {

Sorry, but this is unacceptable. You are adding a new level of
conditionals without changing indentation. Come on.

I understand and very much appreciate the desire to touch as few
lines of codes as possible, but that objective can not take
precedence over correct indentation, which is absolutely fundamental
for quality source code, which I guess we all want to work with.

A suggestion is to correctly use goto, in order to avoid changing a
very large block of code, if this makes sense. You might not need to
use goto, but might be able to use return instead. I haven't looked
into this function so I can't say for sure, but the above is in any
case not OK.


> @@ -330,15 +332,19 @@ void breakpoint_remove_internal(struct target *target, 
> uint32_t address)
>   if (breakpoint)
>   {
>   breakpoint_free(target, breakpoint);
> + return 1;
>   }
>   else
>   {
> + if (!target->smp)
>   LOG_ERROR("no breakpoint at address 0x%8.8" PRIx32 " found", 
> address);
> +return 0;
>   }

Same here about indentation. The added return line is also using
the wrong indentation character, please check your editor settings
and adjust it to the code style already used by the file. (I really
like the feature of some editors to autodetect the indent style used
in a file, but of course not all editors support this.)


>  void breakpoint_remove(struct target *target, uint32_t address)
>  {
> -if ((target->smp))
> + int found = 0;
> + if ((target->smp))
>   {

Please avoid mixing whitespace changes with other changes. Send one
patch which fixes all whitespace issues at once.


> --- a/src/target/target.c
> +++ b/src/target/target.c
> @@ -3023,6 +3023,13 @@ COMMAND_HANDLER(handle_bp_command)
>   {
>   case 0:
>   return handle_bp_command_list(CMD_CTX);
> + case 2:
> + {
> + asid = 0;
> + COMMAND_PARSE_NUMBER(u32, CMD_ARGV[0], addr);
> + COMMAND_PARSE_NUMBER(u32, CMD_ARGV[1], length);
> + return handle_bp_command_set(CMD_CTX, addr, 
> asid, length, hw);
> + }

Why are these {} added, and why have the variable when it isn't used?


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] [PATCH] updated Fujitsu FM3 Flash driver

[Peter Stuge]

openOCD.fseu wrote:
> please find attached the updated Fujistu CORTEX M3 NOR Flash driver.
> It includes:
> - Improvement of error handling of "retval = target_read/write_xxx();"

Great to get better error handling! All the repeated retval=..if()
return look absolutely horrible but that's indeed not your fault.

We should maybe have some handy macros for this in shared code, to
make drivers prettier and simpler.


> - No #if statements anymore
> - Added "Flash type 2" support

Great.


Content-Description: 
0001-Updated-fm3.c-added-Flash-type-2-support-error-handl.patch
> --- a/src/flash/nor/fm3.c
> +++ b/src/flash/nor/fm3.c
> @@ -1,23 +1,28 @@
> -/***
> - *   Copyright (C) 2011 by Marc Willam, Holger Wech*
> - *   m.wil...@gmx.eu   *
> - *   Copyright (C) 2011 Ronny Strutz   *
> - * *
> - *   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. *
> - ***/
> +/*
> + *   Copyright (C) 2011 by Marc Willam, Holger Wech  
> *
> + *   openOCD.fseu(AT)de.fujitsu.com  
> *
> + *   Copyright (C) 2011 Ronny Strutz 
> *
> + *   
> *
> + *   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.   
> *
> + 
> */

Please avoid all whitespace-only changes. Perhaps especially so in
the license text for a file.


> + 
> +// History:
> +// 2011-07-13 MWi First version
> +// 2011-09-27 MWi Flash type 2 added, algorithm start address now 
> relocateable
> +

As Øyvind pointed out this is redundant, since git tracks history
already, and with good detail.


>  enum fm3_variant
>  {
> - mb9bfxx1,
> + mb9bfxx1,   /* Flash Type '1' */
>   mb9bfxx2,
>   mb9bfxx3,
>   mb9bfxx4,
>   mb9bfxx5,
> - mb9bfxx6
> + mb9bfxx6,
> + mb9afxx1,   /* Flash Type '2' */
> + mb9afxx2,
> + mb9afxx3,
> + mb9afxx4,
> + mb9afxx5,
> + mb9afxx6
> +};

It would help to have a separate patch that adds the comma to the
last entry in the enum, followed by another patch that adds the new
variants, making sure to include a comma after the last new entry,
and putting the two comments on separate lines instead of together
with actual values, so that in the future no lines will need to
change unless there is an ac

Re: [Openocd-development] Fwd: BerliOS will be closed on 31.12.2011

[Peter Stuge]

Anders Montonen wrote:
> > Øyvind Harboe wrote:
> >> My first instinct is to move the mailing list wholesale to sourceforge.
> > I agree strongly with this.
> ...
> > I am personally very strongly against depending on companies based in
> > the US for open source project infrastructure.
> 
> lol wut?
> 
> From :
> "SourceForge.net is owned and operated by Geeknet, Inc., a publicly
> traded US-based company."

I expected this. :)

Mailing lists are an exception, because participation does not
require creating an SF user account.

Another less important factor is that open lists require a lot of
work to operate (as everyone having operated a mail server knows)
and a large operator benefits tremendously from economy of scale,
thus can do a very good job at keeping mailing lists efficient and
pleasant.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Fwd: BerliOS will be closed on 31.12.2011

[Peter Stuge]

Øyvind Harboe wrote:
> My first instinct is to move the mailing list wholesale to sourceforge.

I agree strongly with this.


Spencer Oliver wrote:
> I agree - I will look into it, website aswell.

As for website and repositories, a lot of nice integration can be
accomplished when self hosting, as I am doing for several projects
already. I would be happy to host also the openocd repository(ies)
and set up a Trac, which includes a wiki, a ticket system, a source
code browser, and repository history view, all nicely integrated.


Jim Larson wrote:
> Any consideration of using github (github.com)? I've been pretty
> happy with them and they have a huge user base.

I am personally very strongly against depending on companies based in
the US for open source project infrastructure. I find there are quite
significant drawbacks, due to stupid US law and/or due to the service
being provided by a business with a clear agenda, which makes it an
impossible choice for any self-respecting open source project.
Already the very very first condition of github terms and many others
completely rule it out IMO:

--8<-- http://help.github.com/terms-of-service/
A. Account Terms

1. You must be 13 years or older to use this Service.
-->8--

This is not significant because OpenOCD has so many contributors
under the age of 13, but because it is unacceptable IMO that it would
be *ILLEGAL* to welcome a young contributor into the project. It is
just absurd. Of course the terms continue with more great stuff.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Fwd: ft2232_transfer why bit-by-bit?

[Peter Stuge]

Hi Sven,

Sven Krauss wrote:
> A while ago I sent a patch solving this problem. Have a look at the
> mailing from Sep, 1.

I didn't have email at the time so I didn't see this before. Thanks
for the hint!


> https://lists.berlios.de/pipermail/openocd-development/2011-September/020865.html

Can you talk a little about the changes that the patch makes?


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Fwd: ft2232_transfer why bit-by-bit?

[Peter Stuge]

Tomek CEDRO wrote:
> >> Does Versaloon fully support ARM target, memory access and debug
> >> already?
> >
> > I haven't tried it yet, but I believe so. Simon has patches. I think
> > this may be the most up to date source:
> >
> > http://www.versaloon.com/bbs/viewtopic.php?f=2&t=2808
> 
> Hmm, looks interesting, mem_ap_read_buf_u32() in the openocd HEAD tree
> use hardcoded jtag for access. I need to take look at the patches :-)

http://www.versaloon.com/bbs/download/file.php?id=118
(openocd_0.5.0_patch.zip) is the one to look inside, for
openocd.patch. It abstracts this hardcode to instead call
dap_queue_dp_scan(). On the other hand there is a not very pretty
swd_mode global variable introduced. Hopefully that's easy to solve
somehow though.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Fwd: ft2232_transfer why bit-by-bit?

[Peter Stuge]

Tomek CEDRO wrote:
> On Mon, Sep 26, 2011 at 10:29 PM, Peter Stuge  wrote:
> > I really like the Versaloon project. It supports SWD, and the main
> > author in the project Simon is also quick to send OpenOCD patches for
> > the device.
> 
> Does Versaloon fully support ARM target, memory access and debug
> already?

I haven't tried it yet, but I believe so. Simon has patches. I think
this may be the most up to date source:

http://www.versaloon.com/bbs/viewtopic.php?f=2&t=2808


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Fwd: ft2232_transfer why bit-by-bit?

[Peter Stuge]

Akos Vandra wrote:
> Turns out I don't have an SWD compatible programmer yet.
..
> Does anyone by chance have designs for such a programmer, that I
> could build myself?

I really like the Versaloon project. It supports SWD, and the main
author in the project Simon is also quick to send OpenOCD patches for
the device.

http://www.versaloon.com/


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Fwd: SWD?

[Peter Stuge]

Akos Vandra wrote:
> Thank you for the advice, I will do my best to live up to them :)

Cool.


> I'm new to git as well, I tried making the patch you requested:

Some feedback. First, please see if you can send patches as
text/plain attachments (optionally with a Content-Disposition: inline
MIME header) because when they are included in the body of the
message usually they will be destroyed somehow by either the sending
or the receiving email software. :\


> From 88a430100be4d9685863aa2912173ddc5b83e92f Mon Sep 17 00:00:00 2001
> From: Akos 

Please fix this to look nice by running:

git config --global user.name 'Akos Vandra'
git config --global user.email 'y...@email.addr'

And then please fix the authorship information for this commit. You
will now already jump into one of the most powerful features of git;
rewriting history. Recommend reading Pro Git on this topic:
http://progit.org/book/ch6-4.html

So if this is the last commit on the branch that you are working with
then run git commit --amend --author='Akos Vandra '
and save the commit message. NB I think you need a not too old
version of git for --author to work. It's possible that what is in
Ubuntu 10.10 is too old. :\ In that case you can still do it, but it
gets a bit more complicated.

You need to say --author because git tracks authorship separate from
who committed. Run git log --pretty=fuller to see both fields. The
user.(name|email) settings will only be used for Commit information
when doing git commit --amend, because even though you are making a
new commit it does not mean that you are now suddenly the author of
this change, so you have to manually set a new Author on the command
line.


> Date: Sun, 25 Sep 2011 23:55:36 +0200
> Subject: [PATCH] Fixed two warnings for uninitialized variables
> generating build errors
> 
> Two retvals in the functions oocd_swd_transport_init and
> mem_ap_read_buf_u32 have not had their values initialized
> and generated a warning, which cause the build to fail.
> Fixed by initializing these variables to ERROR_OK

Why was ERROR_OK in particular chosen?

Is it the common pattern in other functions? Or did you check these
two functions in detail to discover that the unused variable will be
used in the success case?

It is a good idea to not neccessarily focus on *what* is changed
(when this can trivially be learned from the patch itself) but
more importantly discuss *why* the change was made in the particular
way it was.


> +++ b/src/target/arm_adi_v5.c
> @@ -696,7 +696,7 @@ int mem_ap_read_buf_u32(struct adiv5_dap *dap,
> uint8_t *buffer,
>  //   uint32_t invalue, adr = address;
>  //   uint8_t* pBuffer = buffer;
> 
> - int i, retval;
> + int i, retval = ERROR_OK;
..

> +++ b/src/transport/swd_core.c
> @@ -125,7 +125,7 @@ int oocd_swd_run(struct adiv5_dap *dap){
>  // TODO: We are operating on global interface pointer, change it into
> function parameter asap.
>  int oocd_swd_transport_init(struct command_context *ctx){
>   LOG_DEBUG("entering function...");
> - int retval, *idcode;
> + int *idcode, retval = ERROR_OK;

Ideally try to always change as few bytes as possible in every patch.
Some patches will be large anyway, but it is a good idea to not
change "extra" things, such as the reordering of variable
declarations here. It will work also to initialize retval if it
remains first on the line, so this is preferable. Some diff
algorithms are smart enough to visualize this change as only being
new characters added after retval, so swapping the order adds noise.

This change is probably perfectly fine, but in this case the patch
itself is not enough for a reviewer to judge, since the only thing we
see is your commit message saying "I changed it to return ERROR_OK
where there was a warning for uninitialized variable" and we see this
one more time in the patch, but we don't see the code that is
affected by this change, so it's impossible to say if your fix is
right or wrong. :\ The easiest way is to have more *why* and less
*what* in the commit message.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Fwd: SWD?

[Peter Stuge]

Akos Vandra wrote:
> I suspect it is because I have an x64 system where pointers are 64bits in 
> size.
> The lines listed below try to cast a pointer to an int (signed 32bit),
> for producing log messages.
> I don't know why the address pointed is useful log information, and
> don't really know how to make this piece of code portable between
> 32bit and 64bit systems, so I'll leave it up to you.

I think %p should work.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Fwd: SWD?

[Peter Stuge]

Hi Akos,

Akos Vandra wrote:
> Bingo, I am working on Ubuntu 10.10, x64, gcc 4.4.5 (shipped with ubuntu).

10.10 is soon one year old, which can be a long time in open source.

Also keep in mind that pretty much every major distribution is
applying significant amounts of patches in their toolchain packages
and possibly also system libraries.

They intend this to make things better of course, but it can backfire
and make the toolchain unusable sometimes. Keep this in mind if you
encounter a weird build issue of any package.

>From many years of use of gcc and binutils I would recommend to use
a vanilla toolchain, or at the very least very aggressively pulling
toolchain updates from your distribution.


> I have been getting some warnings (treated as errors) due to some
> retvals didn't have initial value set, but I set them to ERROR_OK,
> and now it passes those points.

Please send patch(es) for these to the mailing list. Thanks!


> It seems like uint32_t is not defined, even though it should be, as
> stdint.h is included.

stdint.h is included only #ifdef HAVE_STDINT_H. Did you verify with
gcc -E or at least in config.h that HAVE_STDINT_H is defined?


> Funny thing, if I set the first occurance of uint32_t to unsigned
> int, the other ones build fine, which is kinda wierd.

It is not funny at all. I find it typical of a toolchain issue. It is
bullshit that wastes precious developer time. :\


> Also, should we spamming the developer list,

Certainly yes. The purpose of the developer list is to carry
developer discussion.


> or rather continue this discussion personally?

Absolutely not.


> I'm not familiar with opensource development *at all*,

Welcome to open source! It can be a wonderful environment of
cooperation and synergy when things align well. It can also be a
challenging environment of conflicts and complaints when they don't.

Overall I personally find it to be more than worth the trouble, and
I think it leads to better software.


> so I'm not familiar with list policies either.

Again, this is a developer list whose sole purpose is for developer
communication. Direct communication should be an exception.
(Sometimes it's the right thing of course, just usually it is not.)

Please study git and work on becoming fluent with it if you are not
already. It is a great tool that supports development in very many
ways. Beyond simply keeping track of what happened in the code git
when used well allows incredibly efficient review and movement of
code between developers in a project. I can recommend the
http://progit.org/ book, but start out by just playing with it a
little. Make some repositories with some commits and get the basic
workflow going. Learning the rest is usually easy. I also recommend
the #git IRC channel on freenode, for getting live help.


One use for developer mailing lists is to distribute proposed
patches, so that they can be reviewed and then included into the
public source tree.

To make the review step quick and thus make it somewhat likely to
actually happen, it is *critical* that the patch is what I call
"clean". Maybe all this is obvious to you already, in that case
please skip to bottom. The individual points of a clean patch are:


* Write a top quality commit message, technically and logically

In git repositories it is useful to follow a certain commit message
format: (Line numbers added to the left)

1. Description of change, prefer <=60 chars, absmax 74 or even 72 chars
2. 
3. Third line and onward hold longer description of change
4. each line absmax 74 chars

The first line is used in list views, where one commit is listed per
line. This makes it important to have a short and sweet description
of the change in the first line. The second line is always left
blank. The third line and any number of following lines can and
should include background on this part of the code and discuss why
this change was made the way it was done instead of some other more
obvious way. Ie. document the research that went into this particular
change of the code. Including links to web pages with relevant
information is helpful, as is links to mailing list messages e.g. on
http://marc.info/ which has nice short message links. (Only the m=
URL parameter is required for links to a message.)

The line length restrictions are in part due to use of 80 char wide
terminals, but more importantly they are due to how patches are sent
and refered to in email. I.e. even if in an open source project no
active developer is using 80 char wide terminals there may be
participants on the mailing list who benefit from the shorter lines,
in which case it makes sense to keep them short. Also, web based
repository viewers tend to not want or be able to present very long
first line descriptions.

As for the logical quality, it is important to write the first line
description of the change so that it makes sense for someone who
knows nothing at all about the code, since this is used in list
views, and since the ba

Re: [Openocd-development] load_image trouble

[Peter Stuge]

Hi Dmitry,

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.

Are you using latest openocd git code, if not which version?

Perhaps this is a problem of clocking, OpenOCD may need to set that
up depending on what the chip expects.

Which JTAG speed are you using? It should say in the output from
running openocd.

General suggestions would be to disable DCC and to try lower JTAG
speed, but at best those are workarounds. The real solution is
probably more complicated.


//Peter


pgppHRjZR3QKM.pgp
Description: PGP signature
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] rebase vs. merge

[Peter Stuge]

Jean-Christophe PLAGNIOL-VILLARD wrote:
> When you rebase you do not known against which commit the current
> pull request was tested

I think there may be a little bit of confusion.

As I understood it, Øyvind asked which of the following two flows we
should use:

a. "rebase"
 1. developer makes a branch locally
 2. git commit locally
 3. does git pull --rebase to ensure the local changes apply cleanly
on top of public master
 4. git send-email
 5. maintainer does git pull, which will always be a fast-forward
merge, no matter if git pull is used with or without --rebase is
used, because the developer has already done a rebase in (3) onto
the latest commit

b. "merge"
 identical to above, except that (3) is skipped.

Flow a. will generate a single linear history in the main repo. Flow
b. will create a merge commit whenever a merge is not fast-forward,
ie. when commits have been made in the public repo after the branch
was created in (1).

Flow a. loses information about where the branch was created, but
results in a simple linear history.

Flow b. keeps information about where the branch was created, but
results in merge commits.


> If a pul conflit we try to resolve it but if it's too complex at that
> time we ask the one that send the pull request to rebase it's patches
> against the current tree

I think it's very reasonable to require any pull request to apply
cleanly to current tree.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Noise from gmane..

[Peter Stuge]

Sorry about the noise. I also found 500 emails in my inbox. The
problem is solved, so hopefully no more noise now.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] OpenOCD libftdi and ftd2xx benchmark

[Peter Stuge]

Xiaofan Chen wrote:
> > Actually the result is pretty close for the LPC-P2148 based test.
> > jtag_khz = 1500 KHz, 38.927 KiB/s (ftd2xx) versus 38.754 KiB/s.
> 
> The above is for Amontec JtagKey2 which is high speed USB.
> 
> The J-Link under OpenOCD (full speed USB but with intelligence
> in it) seems much slower than Amontec JtagKey2. The speed
> is 18.813 KiB/s at the same 1500KHz jtag_khz for J-Link (V6, IAR OEM).

I'll test the Versaloon with an LPC-P2148 tomorrow.


Xiaofan Chen wrote:
> > So in conclusion there is almost no difference in performance between
> > OpenOCD using libftdi-0.19 and ftd2xx, or did I overlook something?
> 
> Yes ftd2xx is only slightly faster based on my limited tests.

Please don't be shy. You did rather extensive testing in an excellent
structured fashion which makes the results clear and easy to
understand for anyone. That is incredibly valuable. :)


> Freddie, Laurant and others may have more inputs

Even more test results are of course interesting!


> > If not, I would suggest to only use libftdi in OpenOCD.
> 
> I think it is still good to have both options.

No doubt a convenience for users, but will it be big enough to
warrant the extra work (and really ugly code) needed while rewriting
the ft2232 driver? I tend to say no.

I think the majority of ftd2xx users were on Windows, and I think
they are equally if not more happy with an OpenOCD binary package
that is easily downloaded and has kernel drivers (WinUSB) that are
even installed automatically (go libwdi!) just by running openocd.

That is the open source one-click embedded debug (well ok gdb too)
that NXP has accomplished for some of their targets, but which we can
actually do for *every* target. :) Just like Pete this is exactly
what I would like OpenOCD to do, and it seems not so far away.


> > The benefit for OpenOCD would be that it can use libusb-1 on Windows
> > without first having to rewrite the ft2232 driver.
> 
> That is right. It is actually not difficult to switch to libftdi-1.0
> since the current 1.0 API is compatible with libftdi-0.1x.

Awesome.


> > That rewrite is
> > absolutely desirable, but it seems that everyone agrees that the
> > driver deserves and needs a thorough job, so would be nice to be able
> > to punt on that and still be able to start using WinUSB and libusb-1.
> 
> I did some tests for libftdi-1.0 last time and it did not
> offer any speed improvement for OpenOCD since OpenOCD
> has not taken the advantage of the libftdi-1.0 async API.

Yes, I also don't expect speed advantages without rewrite, but rather
usability advantages basically thanks to libwdi and libusb-1/WinUSB.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] OpenOCD libftdi and ftd2xx benchmark

[Peter Stuge]

Many thanks for making these tests! Awesome!

Xiaofan Chen wrote:
> Actually the result is pretty close for the LPC-P2148 based test.
> jtag_khz = 1500 KHz, 38.927 KiB/s (ftd2xx) versus 38.754 KiB/s.

So in conclusion there is almost no difference in performance between
OpenOCD using libftdi-0.19 and ftd2xx, or did I overlook something?

If not, I would suggest to only use libftdi in OpenOCD.

Uwe, do you see a possibility to port libftdi-0 over to use the
libusb-1 API? If only synchronous is used it should be straight
forward. I'm not saying that *you* must do it, but am rather asking
how much effort it might be.

The benefit for OpenOCD would be that it can use libusb-1 on Windows
without first having to rewrite the ft2232 driver. That rewrite is
absolutely desirable, but it seems that everyone agrees that the
driver deserves and needs a thorough job, so would be nice to be able
to punt on that and still be able to start using WinUSB and libusb-1.


//Peter
___
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

[Peter Stuge]

Andrew Leech wrote:
> Again just a release tarball issue, but the tarball from above doesn't work 
> properly, it needs bootstrap run to get jimtcl etc

In particular for a release it's really really important that the two
separate packages (OpenOCD and jimtcl) are *two separate packages*.

The tricks that autobuild jimtcl are convenient for random perusal
but does not very nice packaging make.


> specify the full mingw gcc name as per below or CC="gcc-3 -mno-cygwin"
> (which I haven't tested recently):
> Using: $ ./configure  --enable-maintainer-mode --enable-ft2232_ftd2xx 
> --with-ftd2xx-win32-zipdir=../ftdi CC="i686-pc-mingw32-gcc"

CC should basically *never* be used. --host=i686-pc-mingw32 should be
given to configure in this case, with no environment variables.


> I also had problems with jimtcl and dos line ends, the configure
> scripts and autosetup files didn't work

Nod. For unknown reason at least configure absolutely must have LF
only line endings.


> Could bootstrap have it's git commands for pulling jimtcl changed
> to ensure it doesn't save in dos mode?

Rather difficult. Unsure how that would be done if at all. What could
be done is to add eol=lf in .gitattributes for the files that need it.


> Debugging a lpc3131 (ARM926EJ-S) with a ft2232 jtagkey compatible
> dongle then works fine!

Thanks for writing up the build issues and testing!


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] [PATCH 2/2] ft2232: Failure to get latency should not be fatal

[Peter Stuge]

Spencer Oliver wrote:
>> Why are we duplicating effort on two different libraries that
>> accomplish exactly the same thing?
>
> Main reason is that ftd2xx works better/faster on windoze.

Is it known how much, and why?


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] [PATCH 2/2] ft2232: Failure to get latency should not be fatal

[Peter Stuge]

Spencer Oliver wrote:
> ftd2xx

Why are we duplicating effort on two different libraries that
accomplish exactly the same thing?

Wouldn't it be nicer to simply cut ftd2xx and only use libftdi?

What are respective advantages of the two libraries?


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Versaloon driver update fix, and about jtag_usb_open

[Peter Stuge]

simon qian wrote:
> Attachment is the versaloon driver update, please commit if no problem.

Thanks!

I encountered a large number of lines with trailing whitespace:

$ git am 0001-versaloon-driver-update.patch 
Applying: versaloon driver update
/tmp/openocd/.git/rebase-apply/patch:92: trailing whitespace.
RESULT usbtogpio_config(uint8_t interface_index, uint32_t mask, 
/tmp/openocd/.git/rebase-apply/patch:93: trailing whitespace.
uint32_t dir_mask, uint32_t 
pull_en_mask, 
/tmp/openocd/.git/rebase-apply/patch:97: trailing whitespace.

/tmp/openocd/.git/rebase-apply/patch:105: trailing whitespace.

/tmp/openocd/.git/rebase-apply/patch:111: trailing whitespace.

warning: squelched 273 whitespace errors
warning: 278 lines add whitespace errors.

$ 

These should be fixed.

Once applied, I successfully tested my Versaloon Mini with OpenOCD
communicating with a LM3S9B90 CPU. This CPU supports both JTAG and
SWD, but I believe only JTAG was used.

I have verified that my Versaloon also does SWD using an LPC1343 CPU,
but this CPU isn't supported by OpenOCD, so I could so far only test
SWD using vsprog.

Can SWD also be tested using Versaloon and OpenOCD? This question of
course also relates to Tomek's work on SWD in OpenOCD. Have you
communicated with each other about how to join your efforts, if
needed?


Kind regards

//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] General SWD Support in OpenOCD

[Peter Stuge]

Tomek CEDRO wrote:
> Im afraid there is not much sense in that interactive rebasing for
> such big change - it took me so many hours to split patches and in
> result now there are even more patches some new patches patched by old
> patches ;] This is far from perfect.

You know that interactive rebase can also be used to reorder commits?

So you would move the commits that belong together after each other,
have the first be pick, and then use squash or fixup for the others.

To be able to use this feature it is an absolute requirement that
each commit in the repo only changes one logical thing, or there will
be massive complications. This is only one of the many reasons why it
is so important to have each commit be only a single logical change.


> I can see git wants to keep the commit history at all cost,

For a public repository it is the prefered way, but it's not a hard
requirement, especially not if you announce in advance that there
will be history changes in the repository, or maybe only in certain
branches.


> so the cructial thing here is producting good commits.

This is important. Not because git wants it, but rather because it is
the only way that allows to take full advantage of features in git,
which makes further work in the repo really easy.


> how to produce "good patches" from git diff (i.e. something as
> git format-patch)... or the git diff is enough?

I'd be happy to help you with this but I think email is not so
convenient. I would prefer IRC or jabber, and probably some
pastebinning. I just joined #openocd on irc.freenode.net.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] General SWD Support in OpenOCD

[Peter Stuge]

Tomek CEDRO wrote:
> Now when I do "git format-patch openocd-master file" I get bunch of
> patch files based on my local commits. This is nasty as brings
> unnecessary commits. I would rather produce one patch that contains
> only important changes.. or you prefer the way format-patch produces?

A mix is best. Please turn it into as many patches as make sense. One
logical change per patch is the best. Maybe it will indeed only be a
single commit, but maybe there are some preparatory changes that are
not related to SWD itself, then they should go in a commit each.


> How to produce "good" patch with description etc from "git diff"
> result? :-)

Have a look at how to use interactive rebase. (rebase -i)

When you have a branch off master with a couple of commits you can
run:

git rebase -i origin/master

to start an interactive rebase of all commits since the last commit
you have from the remote repository.

Check out the man page git rebase --help and/or the
http://progit.org/ book. Please ask if you have some questions.


//Peter
___
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

[Peter Stuge]

Tomek CEDRO wrote:
> >> If you switch output into Hi-Z the pull-up or pull-down resistor will
> >> take care of the on the pin state.
> >
> > I think Michael's point is that *which* state it is is not known (and
> > can not be known) by generic ft2232 code in OpenOCD.
> >
> > And that just because the code does not have information, it should
> > not make guesses.
> 
> How it cannot have this information? Who does then? When?

Well, it depends on what is between the FT2232 and the CPU, so
information from both interface and target board is needed. It could
certainly be added to OpenOCD, but I'm not sure if it is actually
available yet?


> The goal of OpenOCD is to interfere with target to obtain expected
> results, after the job is done no interaction is necessary, or we
> simply not quit the program to enforce some specific behavior...

Yes, a good point. Michael, is the point to leave a known debug state
driven, or just to leave with a known pin state?


//Peter
___
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

[Peter Stuge]

Tomek CEDRO wrote:
> > Changing signals to High-Z is *not* safe. It changes the JTAG signals to
> > a board-specific state that depends on the pullup/pulldown resistors
..
> If you switch output into Hi-Z the pull-up or pull-down resistor will
> take care of the on the pin state.

I think Michael's point is that *which* state it is is not known (and
can not be known) by generic ft2232 code in OpenOCD.

And that just because the code does not have information, it should
not make guesses.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] New development version of OpenOCD available at last!

[Peter Stuge]

Øyvind Harboe wrote:
> Figure out what outstanding problems there are,

I personally have only one problem; that unplugging an ft2232 dongle
while openocd is running requires that I kill the program and start
it (and telnet clients) again. Would be very nice to recover
gracefully from that.

Some work with LPC1768 suggested that there may be reliability issues
there also. Completely possible those problems are all in tcl.

Finally, I would be fine with a release only after Tomek's work on
SWD is done.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Peter Stuge is now an OpenOCD maintainer

[Peter Stuge]

Pete Batard wrote:
> Peter's involvement as a maintainer has mostly been limited to
> rebuilding his personal (i.e. non official) git branch every 3
> months or so.

If this is really how things look that is bad.

I like to rebase my libusb repo, which makes it difficult to see if
the last change was a small commit message fix or ten hours of
analysis and code rework. Suggestions to make work more visible are
most welcome.

I've always communicated that my libusb repo is the proposed merge
queue for libusb.git. Keeping it there was probably a mistake, it
will become a testing branch in libusb.git instead, so it's more
visible.

I still offer to add libusb repos for anyone and everyone who wants
one, since even before I was maintainer, sadly without many takers. :\


> action needs to be taken by the maintainer...

I've found that our views often differ fundamentally. I think this
may be one more instance of that. The perception of maintainer is
curious.

I haven't contributed in great numbers to OpenOCD, yet I was still
asked to be a maintainer.

I explained that I don't have much time and that I will not likely
be able to increase my level of contribution to OpenOCD, but Øyvind
and Spencer were still interested.

This reinforces my belief that the *what* is more important than the
*how much*, ie. level of involvement.

I am of course honored by Øyvind and Spencer having considered me,
and I will continue to do what I have done so far; try to help when
I can.


> condemning the libusb project to a slow death...

> Peter being able to afford the level of involvement that is
> expected from a project maintainer has really been at the crux

Maybe the expectations are also a factor.

If noone works on code then a project is inactive. At least you, I,
Sean and Hans have worked on the libusb code over the last month, so
I think libusb is still active, actually maybe more widely active
than ever.


To finally come back on topic :) I also don't think that OpenOCD is
anywheree near dying. There's a lot of active development going on,
and I'm excited to continue to be a part of it!


Many thanks, and kind regards

//Peter
___
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()

[Peter Stuge]

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.


> 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!


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] Versaloon driver update

[Peter Stuge]

simon qian wrote:
> Is this patch acceptable?

Sorry, still need to test and look closer. :\ Maybe tonight, maybe
tomorrow night.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Re: [Openocd-development] [PATCH 5/5] ft2232: Handle command buffer malloc() error in ft2232_init()

[Peter Stuge]

Sébastien Farquet wrote:
> Is it possible to have the git patches so to test it and compare it
> with what I did ?

You can pipe each email to git am in order to apply the commits to a
branch in your repo.

For greater convenience I've also pushed the commits to the repo
shown in gitweb here: http://git.stuge.se/?p=openocd.git


//Peter
___
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()

[Peter Stuge]

Øyvind Harboe wrote:
> Can I commit?

Sorry - I messed up the sending a little, I'm resending the full
series right now. The patch is the same though.


> Surely this one is straightforward!!!??? :-)

Yes. Let's see what people think of the other ones. (There be goto.)


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

[Openocd-development] [PATCH 4/5] ft2232: Refactor ft2232_purge_*() into ft2232_purge()

[Peter Stuge]

Like with ft2232_init_ftd2xx() and ft2232_init_libftdi() it helps to
keep abstraction between ftd2xx and libftdi in only one place in the
source code; this time in a ft2232_purge() function.
---
 src/jtag/drivers/ft2232.c |   50 ++--
 1 files changed, 21 insertions(+), 29 deletions(-)

diff --git a/src/jtag/drivers/ft2232.c b/src/jtag/drivers/ft2232.c
index 219d41f..9315eff 100644
--- a/src/jtag/drivers/ft2232.c
+++ b/src/jtag/drivers/ft2232.c
@@ -2280,19 +2280,6 @@ static int ft2232_initone(uint16_t vid, uint16_t pid, 
int more, int* try_more)
return ERROR_OK;
 }
 
-static int ft2232_purge_ftd2xx(void)
-{
-   FT_STATUS status;
-
-   if ((status = FT_Purge(ftdih, FT_PURGE_RX | FT_PURGE_TX)) != FT_OK)
-   {
-   LOG_ERROR("error purging ftd2xx device: %lu", status);
-   return ERROR_JTAG_INIT_FAILED;
-   }
-
-   return ERROR_OK;
-}
-
 #elif BUILD_FT2232_LIBFTDI == 1
 
 static int ft2232_initone(uint16_t vid, uint16_t pid, int more, int* try_more)
@@ -2367,17 +2354,6 @@ static int ft2232_initone(uint16_t vid, uint16_t pid, 
int more, int* try_more)
return ERROR_OK;
 }
 
-static int ft2232_purge_libftdi(void)
-{
-   if (ftdi_usb_purge_buffers(&ftdic) < 0)
-   {
-   LOG_ERROR("ftdi_purge_buffers: %s", ftdic.error_str);
-   return ERROR_JTAG_INIT_FAILED;
-   }
-
-   return ERROR_OK;
-}
-
 #endif
 
 static int ft2232_set_data_bits_low_byte( uint8_t value, uint8_t direction )
@@ -2420,6 +2396,26 @@ static int ft2232_set_data_bits_high_byte( uint8_t 
value, uint8_t direction )
return ERROR_OK;
 }
 
+static int ft2232_purge(void)
+{
+#if BUILD_FT2232_FTD2XX == 1
+   FT_STATUS status;
+
+   if ((status = FT_Purge(ftdih, FT_PURGE_RX | FT_PURGE_TX)) != FT_OK)
+   {
+   LOG_ERROR("error purging ftd2xx device: %lu", status);
+   return ERROR_JTAG_INIT_FAILED;
+   }
+#elif BUILD_FT2232_LIBFTDI == 1
+   if (ftdi_usb_purge_buffers(&ftdic) < 0)
+   {
+   LOG_ERROR("ftdi_purge_buffers: %s", ftdic.error_str);
+   return ERROR_JTAG_INIT_FAILED;
+   }
+#endif
+   return ERROR_OK;
+}
+
 static int ft2232_init(void)
 {
uint8_t  buf[1];
@@ -2488,11 +2484,7 @@ static int ft2232_init(void)
goto quit;
}
 
-#if BUILD_FT2232_FTD2XX == 1
-   if (ft2232_purge_ftd2xx() != ERROR_OK)
-#elif BUILD_FT2232_LIBFTDI == 1
-   if (ft2232_purge_libftdi() != ERROR_OK)
-#endif
+   if (ft2232_purge() != ERROR_OK)
goto quit;
 
return ERROR_OK;
-- 
1.7.4.1.343.ga91df.dirty

___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

[Openocd-development] [PATCH 3/5] ft2232: Clean up on ft2232_init() errors after a device has been opened

[Peter Stuge]

ft2232_quit() is called on errors and should clean up after any device
initialization that has been done by ft2232_init().
---
 src/jtag/drivers/ft2232.c |   17 -
 1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/src/jtag/drivers/ft2232.c b/src/jtag/drivers/ft2232.c
index cb50c50..219d41f 100644
--- a/src/jtag/drivers/ft2232.c
+++ b/src/jtag/drivers/ft2232.c
@@ -217,6 +217,8 @@ static void ktlink_blink(void);
 static void lisa_l_blink(void);
 static void flossjtag_blink(void);
 
+static int ft2232_quit(void);
+
 /* common transport support options */
 
 //static const char *jtag_and_swd[] = { "jtag", "swd", NULL };
@@ -2463,7 +2465,7 @@ static int ft2232_init(void)
ft2232_buffer = malloc(FT2232_BUFFER_SIZE);
 
if (layout->init() != ERROR_OK)
-   return ERROR_JTAG_INIT_FAILED;
+   goto quit;
 
if (ft2232_device_is_highspeed())
{
@@ -2476,23 +2478,28 @@ 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;
+   goto quit;
}
 
buf[0] = 0x85; /* Disconnect TDI/DO to TDO/DI for Loopback */
if ((retval = ft2232_write(buf, 1, &bytes_written)) != ERROR_OK)
{
LOG_ERROR("couldn't write to FT2232 to disable loopback");
-   return ERROR_JTAG_INIT_FAILED;
+   goto quit;
}
 
 #if BUILD_FT2232_FTD2XX == 1
-   return ft2232_purge_ftd2xx();
+   if (ft2232_purge_ftd2xx() != ERROR_OK)
 #elif BUILD_FT2232_LIBFTDI == 1
-   return ft2232_purge_libftdi();
+   if (ft2232_purge_libftdi() != ERROR_OK)
 #endif
+   goto quit;
 
return ERROR_OK;
+
+quit:
+   ft2232_quit();
+   return ERROR_JTAG_INIT_FAILED;
 }
 
 /** Updates defaults for DBUS signals:  the four JTAG signals
-- 
1.7.4.1.343.ga91df.dirty

___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

[Openocd-development] [PATCH 5/5] ft2232: Handle command buffer malloc() error in ft2232_init()

[Peter Stuge]

ft2232_init() fails if the command buffer can not be allocated, but
ft2232_quit() will be called to clean up, so the command buffer must
be free():d only if it has been successfully allocated.
---
 src/jtag/drivers/ft2232.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/src/jtag/drivers/ft2232.c b/src/jtag/drivers/ft2232.c
index 9315eff..aba7f14 100644
--- a/src/jtag/drivers/ft2232.c
+++ b/src/jtag/drivers/ft2232.c
@@ -2459,6 +2459,8 @@ static int ft2232_init(void)
 
ft2232_buffer_size = 0;
ft2232_buffer = malloc(FT2232_BUFFER_SIZE);
+   if (!ft2232_buffer)
+   goto quit;
 
if (layout->init() != ERROR_OK)
goto quit;
@@ -3225,7 +3227,8 @@ static int ft2232_quit(void)
ftdi_deinit(&ftdic);
 #endif
 
-   free(ft2232_buffer);
+   if (ft2232_buffer)
+   free(ft2232_buffer);
ft2232_buffer = NULL;
 
return ERROR_OK;
-- 
1.7.4.1.343.ga91df.dirty

___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

[Openocd-development] [PATCH 2/5] ft2232: Refactor ft2232_init_*() into ft2232_initone()

[Peter Stuge]

ft2232.c is compiled either for ftd2xx or for libftdi. The two init
functions had slightly different prototypes, making it neccessary to
distinguish between them not only at declaration, but also when they
were being called.

Since the code is only compiled for one library type we can homogenize
the function parameters and contain the abstraction within a single
ft2232_initone() function, which allows removing the CPP conditional
in ft2232_init() that determined which function to call.

ft2232_init_libftdi() took an extra channel parameter which came from
the layout struct. layout is treated as a global variable however,
already accessed directly both in ft2232_initone() and ft2232_init(),
so the channel parameter is unneccessary, and can be removed without
risk for new trouble or limitations.

A context parameter that references the layout could be added if and when
this code sees more significant cleanup in the future.
---
 src/jtag/drivers/ft2232.c |   24 +---
 1 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/src/jtag/drivers/ft2232.c b/src/jtag/drivers/ft2232.c
index 1f0269a..cb50c50 100644
--- a/src/jtag/drivers/ft2232.c
+++ b/src/jtag/drivers/ft2232.c
@@ -2115,7 +2115,8 @@ static int ft2232_execute_queue(void)
 }
 
 #if BUILD_FT2232_FTD2XX == 1
-static int ft2232_init_ftd2xx(uint16_t vid, uint16_t pid, int more, int* 
try_more)
+
+static int ft2232_initone(uint16_t vid, uint16_t pid, int more, int* try_more)
 {
FT_STATUS   status;
DWORD   deviceID;
@@ -2290,10 +2291,9 @@ static int ft2232_purge_ftd2xx(void)
return ERROR_OK;
 }
 
-#endif /* BUILD_FT2232_FTD2XX == 1 */
+#elif BUILD_FT2232_LIBFTDI == 1
 
-#if BUILD_FT2232_LIBFTDI == 1
-static int ft2232_init_libftdi(uint16_t vid, uint16_t pid, int more, int* 
try_more, int channel)
+static int ft2232_initone(uint16_t vid, uint16_t pid, int more, int* try_more)
 {
uint8_t latency_timer;
 
@@ -2309,9 +2309,9 @@ static int ft2232_init_libftdi(uint16_t vid, uint16_t 
pid, int more, int* try_mo
return ERROR_JTAG_INIT_FAILED;
 
/* default to INTERFACE_A */
-   if(channel == INTERFACE_ANY) { channel = INTERFACE_A; }
+   if(layout->channel == INTERFACE_ANY) { layout->channel = INTERFACE_A; }
 
-   if (ftdi_set_interface(&ftdic, channel) < 0)
+   if (ftdi_set_interface(&ftdic, layout->channel) < 0)
{
LOG_ERROR("unable to select FT2232 channel A: %s", 
ftdic.error_str);
return ERROR_JTAG_INIT_FAILED;
@@ -2376,7 +2376,7 @@ static int ft2232_purge_libftdi(void)
return ERROR_OK;
 }
 
-#endif /* BUILD_FT2232_LIBFTDI == 1 */
+#endif
 
 static int ft2232_set_data_bits_low_byte( uint8_t value, uint8_t direction )
 {
@@ -2452,14 +2452,8 @@ static int ft2232_init(void)
int more = ft2232_vid[i + 1] || ft2232_pid[i + 1];
int try_more = 0;
 
-#if BUILD_FT2232_FTD2XX == 1
-   retval = ft2232_init_ftd2xx(ft2232_vid[i], ft2232_pid[i],
-   more, &try_more);
-#elif BUILD_FT2232_LIBFTDI == 1
-   retval = ft2232_init_libftdi(ft2232_vid[i], ft2232_pid[i],
-more, &try_more, layout->channel);
-#endif
-   if (retval >= 0)
+   retval = ft2232_initone(ft2232_vid[i], ft2232_pid[i], more, 
&try_more);
+   if (ERROR_OK == retval)
break;
if (!more || !try_more)
return retval;
-- 
1.7.4.1.343.ga91df.dirty

___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

[Openocd-development] [PATCH 1/5] ft2232: Clean up infinite loop condition in ft2232_init()

[Peter Stuge]

---
 src/jtag/drivers/ft2232.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/jtag/drivers/ft2232.c b/src/jtag/drivers/ft2232.c
index 8c2382a..1f0269a 100644
--- a/src/jtag/drivers/ft2232.c
+++ b/src/jtag/drivers/ft2232.c
@@ -2438,7 +2438,7 @@ static int ft2232_init(void)
return ERROR_JTAG_INIT_FAILED;
}
 
-   for (int i = 0; 1; i++)
+   for (int i = 0; ft2232_vid[i] || ft2232_pid[i]; i++)
{
/*
 * "more indicates that there are more IDs to try, so we should
-- 
1.7.4.1.343.ga91df.dirty

___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

[Openocd-development] [PATCH 1/5] ft2232: Clean up infinite loop condition in ft2232_init()

[Peter Stuge]

---
 src/jtag/drivers/ft2232.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/jtag/drivers/ft2232.c b/src/jtag/drivers/ft2232.c
index 8c2382a..1f0269a 100644
--- a/src/jtag/drivers/ft2232.c
+++ b/src/jtag/drivers/ft2232.c
@@ -2438,7 +2438,7 @@ static int ft2232_init(void)
return ERROR_JTAG_INIT_FAILED;
}
 
-   for (int i = 0; 1; i++)
+   for (int i = 0; ft2232_vid[i] || ft2232_pid[i]; i++)
{
/*
 * "more indicates that there are more IDs to try, so we should
-- 
1.7.4.1.343.ga91df.dirty

___
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

[Peter Stuge]

Øyvind Harboe wrote:
> > 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.

Right, that's the point here.


> If someone does come forth with an even more cleaner patch, then
> I'm OK with that.

Trying, but what a can of worms this file is.


//Peter
___
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()

[Peter Stuge]

Andreas Fritiofson wrote:
> 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?

If noone can answer this then I would vote for removing the buffer
completely instead of doing allocation which isn't used anyway.


//Peter
___
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

[Peter Stuge]

Ø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.

Or just use goto, please, which is clear, simple, well understood,
and elegant all at once. :)


//Peter
___
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

[Peter Stuge]

Andreas Fritiofson wrote:
> I'm not very fond of the "infinite-loop-that's-not-actually-a-loop-at-all"
> along with the extra indentation.

+1


> This patch doesn't use gotos either but another (perhaps even more)
> neglected construct, like it or not. :)

Hah. I wouldn't have done it this way, but it's your patch.


> From c934a6c697c90fd5b5211f744e6607ae9cb7b848 Mon Sep 17 00:00:00 2001
> From: Andreas Fritiofson 
> 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 

I'm inclined to say

Acked-by: Peter Stuge 


But I'd like a separate commit for either moving _quit() or simply
adding a prototype and leaving it in place.


//Peter
___
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

[Peter Stuge]

Laurent Gauch wrote:
> First, thank you for giving objective comments on the patch.
> Much appreciated from you.
> (Much better than the "Student code galore" from Peter)

I imagine that Tomek spent a fair bit of time writing up his
comments in such a clear way.

I did not spend as much time writing my comment, because I don't have
that much time to explain why bad code is bad.

Let's try to get to the *actual* reason for your patch:

> The objective is to provide at middle term a better code and more
> comprehensible code for the FT2232.c.
..
> Actually the deinit is not implemented and it should be done quickly.

You claim to have the patch ready - what you should have done is to
have sent the patch already. By sending only a single patch out of a
patch set with several related patches you are being extremely
unhelpful, because you are refusing anyone else than you to see the
full picture.


> The deinit should first deinitialize the specific layout (as a
> jtagkey_deinit), then disable the MPSSE / bitbang mode for any USB
> JTAG based ft2232. Objective is to let see the ft2232 device as if
> it was just plugged (after a power-off -> power-on).

This is absolutely obvious to anyone with a slight understanding. You
really do not need to spend time writing about it. Please just send
(small, smart) patches that fix it, ideally while also making the
codebase simpler, more elegant and uniform.


> Also, you can see the importance of decoupling the :
> open / close of the FT2232 handle
> init / deinit of the FT2232 device

No.


> But when we will come with new SWD transport support, the interface
> layer ( ft2232.c ) will have to be modified to accept new hardware
> as the Amontec JTAGkey-3 coming with the SWD support.

Maybe OpenOCD is not the right software for your product. Dunno.

In any case, yes, it is also absolutely obvious that SWD is not a
great fit for OpenOCD. The correct way to deal with this would be for
you to work together with the other (at least two) stakeholders in
OpenOCD SWD (Tomek and Simon, sorry if I forgot anyone!) and see how
you can contribute to improving OpenOCD infrastructure so that it
will be ready for the future.

The incorrect way is to try to microoptimize the ft2232 driver for
your future products. Maybe that approach is successful for you
elsewhere, but I'm not surprised that the OpenOCD community is
calling bullshit on the change.


> the patch corrects an important memory trouble in the FT2232.c,

Which has been shown possible also with another, much simpler, solution.


> and prepare the FT2232.c for a better open -> init -> deinit -> close !
> The patch is not only a rewrite, it is a correction.

Since you are trying to advocate a largeish change you really must
show everyone what your complete plan is. Here's how to successfully
and quickly get large changes through:

1. Present high level overview of the idea in clear terms, while
doing a bit of education on areas where you know more than most readers
2. Acquire buy-in from stakeholders near same area (hint: not by force)
3. Try to get third parties excited about your idea
4. Present implementation overview
5. Acquire buy-in on implementation
6. Implement, and send patches
7. Watch patches get committed

Commit depends heavily on having passed 1-5. You skipped those parts,
and only provide limited information when pressed for it by review.
It must be the other way around. Trouble with 7 can come as no surprise.


//Peter
___
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

[Peter Stuge]

Laurent Gauch wrote:
> out - of - topic.
>
> Sorry, but please comment the patch instead to talk about coding
> style, or please start a new thread.

It is not off topic just because you think so.

This discussion is a direct result of the patch that was sent, and it
should be clear from the comments that the patch needs some more
work.


//Peter
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development