Re: [Openocd-development] Slowly moving from 8 to 32 bit words in jtag_add_xxx API
On Fri, Nov 20, 2009 at 19:05, Øyvind Harboe wrote: >> Just to clarify the whole issue once more, my proposal was actually >> three different things: >> >> 1. Making the use of scan_field safer by providing standard handlers >> for the most common cases. >> >> This not only helps with the readability and reduces trivial >> copy&paste errors. It also makes it much simpler to rewire the >> underlying scan_field in a later step. > > We can relatively easily do away with the scanfields entirely > rather than putting lipstick on it... See the branch I was working on > or jtag_add_dr_out() API for which I'm looking into adding a in_value > as well as the existing "out_value" that it takes today. This still uses an array that needs to be initialized. Your own embeddedice example patch didn't make use of it... And by adding out_value the caller is forced to hold all data in uint32_t. My suggestion was intended to make (almost) all data types first class citizens, not just choosing one that is the most commonly used. >> 2. Eliminating the global variable jtag_command_queue. >> >> The existing jtag_add_... commands would remain similar but would >> operate on a local copy of the queue. jtag_execute_queue then receives >> the pointer to that local copy as parameter instead of using >> jtag_command_queue. The last user then disposes of the command queue. > > This assumes that there is a queue at all. It should be up to the interface > to implement a queue if that is what is needed. > > The current JTAG calls should be invoked on the interface rather > than assuming a single global instance. This is covered by todo.txt > section today as an architectural improvement. These are two things. The global JTAG device instance and the global command sequence. The TODO is specific on the device instances, but not on command queues. >> 3. Break up jtag_add_dr_scan etc. >> >> This works best in tandem with (2). The general idea is not to pass >> one array of scan fields but to pass them in separate function calls >> (which would mimic, but replace the ones in (1)). To output a 7 bit >> field the caller just hands the value to the function and doesn't >> bother about allocating space. To turn jtag_add_?r_scan inside out >> like this requires its states to be kept somewhere so that >> plausibility checks and bypassing can be done. The local copy of the >> jtag_command_queue would be ideal for that (although it would also >> work by adding even more global variables). The caller then does >> something like this: >> >> jtag_queue_t * q = jq_alloc_queue(); > > I'm very much against *forcing* interfaces to implement a queue > in memory. It should be possible to execute the commands > synchronously. The existance of a queue would make the code > *much* less efficient on embedded devices. Where is the bottleneck in this case? Latency, memory consumption, cpu load or something else? > Did you look at jtag_add_dr_out() which exists today? git format-patch d14b6ca0^...d14b6ca0 Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Slowly moving from 8 to 32 bit words in jtag_add_xxx API
On Thu, Nov 19, 2009 at 22:01, David Brownell wrote: > On Wednesday 18 November 2009, Michael Bruck wrote: >> I would actually prefer an API that is tightly linked to an >> independent data structure that that builds up a jtag sequence in the >> target driver and then executes it. All the commands would then work >> on building up that structure and in the end it is handed over >> directly to the jtag interface driver for execution. > > That presumes there *is* a target. We have SVF and XSVF today, > and there can be other tools that work more at the JTAG level. > > Plus, it's a bit awkward to be coupling address space access > (memory read/write) to targets, considering that newer ARMs > decouple them (via ADIv5) and so does, ISTR, Nexus. This is a misunderstanding due to my imprecise choice of words. I used target layer to refer to anything that uses the core.c interface to send JTAG commands. My comment above meant exactly the same thing as the one you quoted at the end, which I think you understood correctly. > I'm not entirely sure what you mean to describe though. I'd > like to see less baroque structures for the JTAG messages, > and one part of that is clearly a need for more efficient > bit vector handling. > > Along those lines, $SUBJECT is the wrong model too. When we > want to work with 64 bit or 128 bit words -- or even just > the 40-bit words common with older ARM stuff -- then we > should be able to just pass those down. Thinking "8" or > "32" focusses on the wrong stuff ... implementation details, > not the concepts that will produce a better interface. Yes I agree. >> The current model does the same but essentially uses a global >> variable. But presumably due to ownership issues the field data is >> cloned for that global variable. If the jtag sequence structure is >> owned by the target this (second) copy operation can be avoided as >> well. > > I think I agree with what you're saying there. JTAG layer > gets delivered a queue, which it processes and modifies > in place (if required). > > For extra credit: some way to package queues with a bit > of intelligence, so they could be downloaded into smarter > controllers. Example, the polling loops which run in > the background ... or between key steps of high level > operations. > Just to clarify the whole issue once more, my proposal was actually three different things: 1. Making the use of scan_field safer by providing standard handlers for the most common cases. This not only helps with the readability and reduces trivial copy&paste errors. It also makes it much simpler to rewire the underlying scan_field in a later step. 2. Eliminating the global variable jtag_command_queue. The existing jtag_add_... commands would remain similar but would operate on a local copy of the queue. jtag_execute_queue then receives the pointer to that local copy as parameter instead of using jtag_command_queue. The last user then disposes of the command queue. The advantage here is a cleaner modular approach. For example this makes scripting complex JTAG sequences possible without worrying about interference from polling. In theory this also allows for the use of multiple JTAG interfaces. With this approach it is also possible to offer an asynchronous jtag execution mode (if someone needs such a nightmare). 3. Break up jtag_add_dr_scan etc. This works best in tandem with (2). The general idea is not to pass one array of scan fields but to pass them in separate function calls (which would mimic, but replace the ones in (1)). To output a 7 bit field the caller just hands the value to the function and doesn't bother about allocating space. To turn jtag_add_?r_scan inside out like this requires its states to be kept somewhere so that plausibility checks and bypassing can be done. The local copy of the jtag_command_queue would be ideal for that (although it would also work by adding even more global variables). The caller then does something like this: jtag_queue_t * q = jq_alloc_queue(); jq_statemove(q, TAP_IDLE); /* This is just a placeholder for a good solution to deal with the fact that the initial TAP state is not known until jq_execute(). */ jq_set_tap(q, my_tap); /* set the tap to be implied by the following functions, until the next jq_set_tap() */ jq_ir_scan_start(q); jq_field_u32_out(q, 5, some_instruction); jq_dr_scan_start(q); jq_field_u32_out(q, 7, my_value); jq_field_u32_outin(q, 3, another_value, &return_value); jq_execute(q); /* callee frees q and associated data */ Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Slowly moving from 8 to 32 bit words in jtag_add_xxx API
On Wed, Nov 18, 2009 at 16:17, Øyvind Harboe wrote: >>> Actually, the minidrivers don't clone today, so that's already taken care >>> of. >> >> Doesn't that apply only to zy1000.c ? > > The USB case needs to delay execution to build a long > scan, so there copy is required. I think you misunderstood what I am proposing. I did not suggest to execute the sequence immediately. I am not only aware of the USB latency, in fact the arm11 code relies on it for the burst mode. >>> The USB drivers have a 1ms roundtrip problem to contend with, the >>> rest is in the noise, essentially. There is plenty of evidence to this >>> effect. >> >> *You* brought up performance as concern... My goal was primarily >> streamlining code. > > I did. We need both streamlined and fast code. Until we have both sorted, > we leave things the way they are. My proposal does not imply any decrease in speed, it can lead to an increase. It still builds up a jtag sequence before executing it, it just modifies the interface for value types. >> Your example seemed to be too much of a workaround rather than >> addressing the problem directly, which IMO is that the uint32_t case >> is so common that there should be a short unambiguous way to deal with >> it. >> >> Of course even a set of standard wrappers to package the set-up of the >> most common field configurations would help a lot, without also >> serving as a shortcut into the minidriver-layer to avoid the copying. > > I've got some thoughts on how to do this, but nothing written up yet. > >>> Also we have to *carefully* consider how we can make *small* steps that >>> can be tested on all the hardware combinations. Otherwise any change >>> is unlikely to pay off. We're getting there but it will and should take >>> time. >> >> It was a suggestion on long-term goals. Small steps are usually most >> effective when they go towards a specific target. > > I'm doing small steps in a branch for now. We'll see how it pans out. > > That branch might end up being a big leap before it is pushed to > the master branch. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Slowly moving from 8 to 32 bit words in jtag_add_xxx API
On Wed, Nov 18, 2009 at 15:35, Øyvind Harboe wrote: > On Wed, Nov 18, 2009 at 3:32 PM, Michael Bruck wrote: >> On Wed, Nov 18, 2009 at 15:12, Øyvind Harboe wrote: >>> On Wed, Nov 18, 2009 at 2:53 PM, Michael Bruck wrote: >>>> I would suggest removing the fields completely from that layer and >>>> replacing them with function calls. For the most common types of data >>>> like uint32_t. >>>> >>>> scan_start_dr(); >>>> >>>> scan_tap(struct jtag_tap); >>>> scan_field_u32_w(size_t bits, uint32_t value); >>>> scan_field_u32_wr(size_t bits, uint32_t value, uint32_t * result); >>>> scan_field_buf_w(size_t bits, const void * buf); >>>> ... >>>> scan_end(); >>>> jtag_execute_queue(); >>>> >>>> etc. >>>> >>>> The layer below takes all these neatly constructed and allocated >>>> fields and copies them anyway. You'd have to switch from an array of >>>> fields to a linked list internally, but overall the code would be >>>> cleaner. >>> >>> I don't see the entire API you are propsing, but it would be very >>> inefficient >>> and completely break with the current model. >>> >>> The current API has a couple of things going for it with being efficient on >>> both low performance cpu low latency and high performance cpu long latency >>> scenarios. >> >> To the contrary, it would be faster. When fully implemented it removes >> the step that clones all the data in driver.c. > > Actually, the minidrivers don't clone today, so that's already taken care of. Doesn't that apply only to zy1000.c ? > The USB drivers have a 1ms roundtrip problem to contend with, the > rest is in the noise, essentially. There is plenty of evidence to this effect. *You* brought up performance as concern... My goal was primarily streamlining code. Your example seemed to be too much of a workaround rather than addressing the problem directly, which IMO is that the uint32_t case is so common that there should be a short unambiguous way to deal with it. Of course even a set of standard wrappers to package the set-up of the most common field configurations would help a lot, without also serving as a shortcut into the minidriver-layer to avoid the copying. > Also we have to *carefully* consider how we can make *small* steps that > can be tested on all the hardware combinations. Otherwise any change > is unlikely to pay off. We're getting there but it will and should take time. It was a suggestion on long-term goals. Small steps are usually most effective when they go towards a specific target. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Slowly moving from 8 to 32 bit words in jtag_add_xxx API
On Wed, Nov 18, 2009 at 15:12, Øyvind Harboe wrote: > On Wed, Nov 18, 2009 at 2:53 PM, Michael Bruck wrote: >> I would suggest removing the fields completely from that layer and >> replacing them with function calls. For the most common types of data >> like uint32_t. >> >> scan_start_dr(); >> >> scan_tap(struct jtag_tap); >> scan_field_u32_w(size_t bits, uint32_t value); >> scan_field_u32_wr(size_t bits, uint32_t value, uint32_t * result); >> scan_field_buf_w(size_t bits, const void * buf); >> ... >> scan_end(); >> jtag_execute_queue(); >> >> etc. >> >> The layer below takes all these neatly constructed and allocated >> fields and copies them anyway. You'd have to switch from an array of >> fields to a linked list internally, but overall the code would be >> cleaner. > > I don't see the entire API you are propsing, but it would be very inefficient > and completely break with the current model. > > The current API has a couple of things going for it with being efficient on > both low performance cpu low latency and high performance cpu long latency > scenarios. To the contrary, it would be faster. When fully implemented it removes the step that clones all the data in driver.c. > But some variant & helper fn's along the lines above would make > a lot of sense. Yes I would actually prefer an API that is tightly linked to an independent data structure that that builds up a jtag sequence in the target driver and then executes it. All the commands would then work on building up that structure and in the end it is handed over directly to the jtag interface driver for execution. The current model does the same but essentially uses a global variable. But presumably due to ownership issues the field data is cloned for that global variable. If the jtag sequence structure is owned by the target this (second) copy operation can be avoided as well. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Slowly moving from 8 to 32 bit words in jtag_add_xxx API
I would suggest removing the fields completely from that layer and replacing them with function calls. For the most common types of data like uint32_t. scan_start_dr(); scan_tap(struct jtag_tap); scan_field_u32_w(size_t bits, uint32_t value); scan_field_u32_wr(size_t bits, uint32_t value, uint32_t * result); scan_field_buf_w(size_t bits, const void * buf); ... scan_end(); jtag_execute_queue(); etc. The layer below takes all these neatly constructed and allocated fields and copies them anyway. You'd have to switch from an array of fields to a linked list internally, but overall the code would be cleaner. Michael On Tue, Nov 17, 2009 at 22:01, Øyvind Harboe wrote: > I'm pondering how we could gently in a series of > non-breaking patches prepare the ground for switching from > 8 to 32 bit words in the jtag_add_xxx API. > > The attached patch gets rid of buf_set_u32() when setting > the value of a byte. > > This achieves two things: the code is less obtuse and it > is more evident how we could introduce a new type > that is *currently* uint8_t and later on could be increased to > uint32_t or wider, for the out_value/in_value bit vectors. > > Comments? Protests? > > > > -- > Øyvind Harboe > http://www.zylin.com/zy1000.html > ARM7 ARM9 ARM11 XScale Cortex > JTAG debugger and flash programmer > > ___ > Openocd-development mailing list > Openocd-development@lists.berlios.de > https://lists.berlios.de/mailman/listinfo/openocd-development > > ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] add new arm11 option for context aware debugging
On Tue, Nov 17, 2009 at 19:28, David Brownell wrote: > On Tuesday 17 November 2009, Michael Bruck wrote: >> >> A good opportunity for the latter would be when >> >> Tcl objects are added and these functions become methods of target >> >> objects. >> > >> > That doesn't seem like such a good opportunity; it's off in the vague >> > future. I'd rather just prevent trivially obvious wrongness from >> > getting merged in the first place. >> >> >> It is functional as it is and can be of use to users. It may be a >> problem in multi-core systems, but that is merely a restriction and >> doesn't make it unusable. Nothing prevents you from providing a patch >> that includes the restructuring that you want. > > Or following standard patch review+merge policies and suggesting > that the original submitter resolve that issue before it's merged. > > Because that *is* a standard process. Post a patch you'd like to > see merged ... accept feedback ... resubmit ... repeat till merged. I questioned your feedback, not the process. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] add new arm11 option for context aware debugging
On Tue, Nov 17, 2009 at 17:43, David Brownell wrote: > On Tuesday 17 November 2009, Michael Bruck wrote: >> > p.s. One thing that needs fixing in this patch: it >> > shares "arm11_asid" between all ARM11 cores. >> > This should be a core-specific value. >> >> >> This is the same issue with vcr and other global variables that should >> be target specific. IMO it makes more sense to add this Context ID >> functionality now and fix the variable scope in a separate patch that >> addresses all cases. > > I don't see that. Why not fix the variable scope before committing > something with such a known problem? It's not difficult to fix it > before merging ... > > >> A good opportunity for the latter would be when >> Tcl objects are added and these functions become methods of target >> objects. > > That doesn't seem like such a good opportunity; it's off in the vague > future. I'd rather just prevent trivially obvious wrongness from > getting merged in the first place. It is functional as it is and can be of use to users. It may be a problem in multi-core systems, but that is merely a restriction and doesn't make it unusable. Nothing prevents you from providing a patch that includes the restructuring that you want. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] add new arm11 option for context aware debugging
On Mon, Nov 16, 2009 at 20:59, David Brownell wrote: > On Monday 16 November 2009, Øyvind Harboe wrote: >> Does anyone thing this should not be committed? Sounds like >> a *neat* feature > > I'm wondering what an interface would look like that allowed > other ARM cores -- not just ARM11 -- to do this. And whether > there shouldn't be a followon patch to make watchpoints use > the same hardware mechanism... > > > ARM11 seems to have this mechanism built into the core, while > (if I didn't miss something important while skimming) older > cores may not have this mechanism at all. > > But starting with ETMv2 (which includes ARM11!) the ETM may > have up to three "context ID" comparators. Combine one of > those with an address comparator and use that to trigger > entry to debug state, and you've just built a breakpoint > that's *also* a "context aware breakpoint". (I think that > with a richly-enough featured ETM, it could implement two > such break-or-watch-points.) > > Can you clarify ... coprocessor #15 register 13 access > can give either the "Fast Context Switch Extension PID" > or a "Context ID". The context you're exposing here is > the low byte of the "Context ID", right? > > Meaning there are *two* ways to implement this specific > breakpoint functionality ... yes? One of which is > specific to ARM11, another of which is more generic. > > > If that's true ... I suggest refactoring this just a bit, > based on some patches I'm about to send: > > - Have it be an "arm asid [N]" command, not "arm11 asid", > and update a field in the "struct arm". Do something > sane for cores that don't support this mechanism, > like a clean failure. > > - The ARM11 breakpoint code would presumably still work > like in your patch, but it'd collect state from a > different place (so the same command could work for > other cores with this functionality). > > Eventually the current notion of breakpoint/watchpoint > may need to evolve, but doing it like I sketched above > seems like it would help us get to that point sooner > by allowing e.g. Cortex-A8, or possibly very modern ARM9 > cores, to implement this same mechanism. > > - Dave > > p.s. One thing that needs fixing in this patch: it > shares "arm11_asid" between all ARM11 cores. > This should be a core-specific value. This is the same issue with vcr and other global variables that should be target specific. IMO it makes more sense to add this Context ID functionality now and fix the variable scope in a separate patch that addresses all cases. A good opportunity for the latter would be when Tcl objects are added and these functions become methods of target objects. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [patch 3/4] ARM11: fixup method table
On Mon, Nov 16, 2009 at 08:34, David Brownell wrote: >> I guess we have very different approaches to looking at new software. >> Since the structures (classes) are the skeleton > > There's a book titled something like "structs + methods = programs". :) > > When I'm analysing code to see how it works inside, I consider it > a flaw if I can't look through the headers and get a basic feel > for the key types and their operations. Those headers are where > the interfaces between components get defined ... or at least, > they should be. On well written and maintainable code, the headers > are very usable for such purposes. (OpenOCD isn't there yet.) > I guess it just goes to show how out of touch with the openocd source code I am that I was still operating under the assumption that the headers are exactly the place where the structs are defined... Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] add new arm11 option for context aware debugging
IMO the help message would be more descriptive if it said: Restrict breakpoints to a Context ID/ASID (default: 0 = unrestricted) Michael On Mon, Nov 16, 2009 at 12:11, Øyvind Harboe wrote: > Could you write some documentation to openocd.texi? > > Does anyone thing this should not be committed? Sounds like > a *neat* feature > > > > -- > Øyvind Harboe > http://www.zylin.com/zy1000.html > ARM7 ARM9 ARM11 XScale Cortex > JTAG debugger and flash programmer > ___ > Openocd-development mailing list > Openocd-development@lists.berlios.de > https://lists.berlios.de/mailman/listinfo/openocd-development > ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [patch 3/4] ARM11: fixup method table
On Sun, Nov 15, 2009 at 23:00, David Brownell wrote:
> [ offtopic threads #2, #3, ... ]
>
>> The reason why I did not plug arm11 into the existing arm
>> infrastructure and why we implemented etm as a tcl script
>
> "We" didn't see any such Tcl scripts posted though.
Yes, they are tailored to our problem at hand and not generic.
> "We" want to see ETM support that doesn't need to be
> rewritten from scratch for each new core, too...
Me too, unfortunately I needed a working solution in a few hours, not
days. I did look at the existing code first and decided that working
with that (and reverse engineering what it actually does) would take
too long.
>> instead of
>> using etm.c is because, beside being mostly undocumented, most of the
>> openocd code in these areas is a programming hazard full of endless
>> repetitive code that obscures the underlying algorithms. Reverse
>> engineering what these functions do just costs too much time.
>
> On the other hand, from a project-wide perspective having
> one part ("arm11") not even *trying* to reuse code is bad.
>
> When there are code quality issues, the solution is to fix
> them ... not to create something off on the side, which is
> primarily just different and not compellingly better.
This is not a hobby project for me, I need predictable working code. I
would have loved to just plug the arm11 extension into the existing
framework.
I provided these two examples as empirical data for you. Your
hypothesis was, that the programming practices that you promote and
that mostly were already in place at the time I wrote the port make it
easy for someone coming new to the project to work with the code.
Unfortunately that is not always the case, leading to undesirable
outcomes like the independent arm11 target code.
> Plus, someone used to an ARM9 should be able to switch to an
> ARM11 platform without needing to learn a whole new approach
> or scratching their head about why something doesn't work
> as fast any more (the memory things) or isn't even present
> (disassembly, show "all" registers, etc).
>
> One of the benefits of having OpenOCD handle multiple
> JTAG targets is supposed to be that generalizing tends to
> improve quality. Going off in a different direction makes
> such shared improvements impossible.
Obviously. Unfortunately the quality of the existing code made that unrealistic.
>> Take for example:
>>
>> fields[0].tap = etm_reg->jtag_info->tap;
>> fields[0].num_bits = 32;
>> fields[0].out_value = reg->value;
>> fields[0].in_value = NULL;
>> fields[0].check_value = NULL;
>> fields[0].check_mask = NULL;
>
> Note that if fields[] were zero-initialized, the size of
> these would be halved: no NULL assignments.
>
>
>> fields[1].tap = etm_reg->jtag_info->tap;
>> fields[1].num_bits = 7;
>> fields[1].out_value = malloc(1);
>> buf_set_u32(fields[1].out_value, 0, 7, reg_addr);
>> fields[1].in_value = NULL;
>> fields[1].check_value = NULL;
>> fields[1].check_mask = NULL;
>>
>> fields[2].tap = etm_reg->jtag_info->tap;
>> fields[2].num_bits = 1;
>> fields[2].out_value = malloc(1);
>> buf_set_u32(fields[2].out_value, 0, 0, 0);
>> fields[2].in_value = NULL;
>> fields[2].check_value = NULL;
>> fields[2].check_mask = NULL;
>>
>> This piece of digital diarrhea
>
> The entire JTAG scan field interface suffers from the same
> malady!!! So I don't think you have grounds to specifically
> criticize *one* of the many modules that suffer from it.
>
> Example, it'd have made sense to have routines packaged which
> read and write registers using that 32 + 7 + 1 bit DR scan
> model, since it's shared by EmbeddedICE and older ETMs. (Plus
> maybe a bit more, but I'm sure of those.) That should be a
> simple bit of re-factoring, yes?
I did not specifically criticize one module, I thought I had not even
mentioned where it came from. The point was that macros can make more
sense in some cases and should not be dismissed out of hand as
"obscurant" for dogmatic reasons.
>> is no doubt "standard and universally
>> understood" C code. Unfortunately it is also an obstacle when someone
>> tries to read the surrounding algorithm. If that block would say:
>>
>> OUT_FIELD(fields[0], 32, reg->value);
>> OUT_FIELD(fields[1], 7, reg_addr);
>> OUT_FIELD(fields[2], 1, 0);
>>
>> it would be macros you might have to look up once when you deal with
>> openocd for the first time, but that is a tiny price to pay for
>> immediately seeing the relevant parts of the action.
>
> Fair enough. I'd have made it a function, possibly an inlined one.
> Note that there's an issue with host vs target bit ordering...
>
> When you have an idea like that, the best approach is to discuss
> it and then provide patches phasing it in over the *WHOLE* tree.
> Not solve it for one little part of the tree, and ignore the rest.
Yes, I did something along these lines in what is now jtag/driver.c.
That took me an hour to write and forever to negotiate via the mailing
list. The arm11 code has been doing that from
Re: [Openocd-development] [patch 3/4] ARM11: fixup method table
On Sun, Nov 15, 2009 at 22:16, David Brownell wrote: > [ Offtopic thread #1: "struct typedefs considered harmful" ] > >> struct foo >> >> and >> >> foo_t >> >> are both universally understood. > > I can't know what a "foo_t" is without looking it up, > so it's not "universal" in any meaningful sense. > > >> "struct foo" however clutters the >> code, especially in long function parameter lists, and distracts from >> the underlying algorithms/parameters. > > When I see "foo_t" param I can't know how to use it. > > When I see "struct foo" I know it's a struct passed by > value; access as NAME.member, and it's safe to mangle. You of course don't know that member until you looked up the definition of struct foo_t. So you are back at where you have to look the type up to do anything meaningful with it. > When I see "struct foo *" I now it's a struct passed by > pointer; access as NAME->member, and unsafe to mangle > (except according to what callers expect). See comment above. > When I see "int" I know it's passed by value, and is > safe to mangle; no substructure access syntax. Well, I have never encountered that state of confusion that you describe with any actual code. So I can't follow you here. >> What you call "standard" is just other people's programming preferences. > > It's standard in the sense that you know what it means > just by looking at those few lines ... no need to look > anywhere else to know the basics. New-to-the-project > programmers have less to learn, which seems healthy. I guess we have very different approaches to looking at new software. Since the structures (classes) are the skeleton around which openocd is structured it seems a bit far-fetched to me that you could go through the code without looking at the structs. > If you call wanting to *avoid* any "look *everything* up > in some type catalog" steps a programming preference ... > I can hardly stop you. Here is an example: void foo(struct bar_t * b); What can you do with this function without looking up bar_t or seeing its function body ? Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [patch 3/4] ARM11: fixup method table
On Sun, Nov 15, 2009 at 19:13, David Brownell wrote:
> On Sunday 15 November 2009, Michael Bruck wrote:
>> >>> The macro is right above the table.
>
> Yep. At least, *now* it's right above.
This is the original commit:
http://openocd.git.sourceforge.net/git/gitweb.cgi?p=openocd/openocd;a=blob;f=src/target/arm11.c;h=11e376a6e6b935e9107f60c1c1d01721d0da924d
>
>> >> The problem with this sort of macros is that
>
> ... it doesn't really add anything. Even though
> it *is* right above that table, you still have to
> think more about it than when you see standard
> initializer syntax (".field = value,"). The lines
> above the top of the table are not necessarily on
> the screen, or in your memory.
It avoids redundant code and focuses the attention on the important parts.
>> >> we have
>> >> a dozen targets and having a dozen different
>> >> macros gets a bit tedious. The arm11 macros
>> >> does show that certain things are tedious in OpenOCD
>> >> speak, but hopefully we can address those on
>> >> in a more general fashion.
>> >
>> > This seems highly unlikely, quite a few contributions here lately
>> > center around enforcing antiquated bloated syntax.
>
> Instead of "antiquated bloated" I'd say "standard and
> universally understood". We're not exactly talking
> about a COBOL level of verbosity here. ;)
struct foo
and
foo_t
are both universally understood. "struct foo" however clutters the
code, especially in long function parameter lists, and distracts from
the underlying algorithms/parameters.
What you call "standard" is just other people's programming preferences.
> Did you ever see the original Bourne shell sources?
> The ones that abused CPP to make the language look
> like Algol-60 instead of C? Now *that* was centered
> around enforcing antiquated syntax ...
Your patch is purely about enforcing arbitrary syntax rules and in the
process adding redundant code that obscures what is going on. For
example consider:
...
ARM11_HANDLER(init),
ARM11_HANDLER(foo),
ARM11_HANDLER(bar),
GENERIC_ARM_HANDLER(dog),
ARM11_HANDLER(moo),
ARM11_HANDLER(close),
...
While not elegant it makes it immediately clear that dog is derived
from the generic arm handler. On the other hand in the macro-free
version it is far easier to overlook:
.init = arm11_init,
.foo = arm11_foo,
.bar = arm11_bar,
.dog = arm_dog,
.moo = arm11_moo,
.init = arm11_close,
>> > The patch does not
>> > improve legibility it merely reflects certain contributer's personal
>> > distaste for macros.
>>
>> This should be contributors', plural is probably warranted here.
>
> It's also a fairly standard policy in most projects:
> avoid syntactic sugar macros. To be used, they need
> first to be learned ... and that's effort that could
> be more usefully applied to other tasks. And to be
> used, they also need to be maintained ... likewise.
The reason why I did not plug arm11 into the existing arm
infrastructure and why we implemented etm as a tcl script instead of
using etm.c is because, beside being mostly undocumented, most of the
openocd code in these areas is a programming hazard full of endless
repetitive code that obscures the underlying algorithms. Reverse
engineering what these functions do just costs too much time.
Take for example:
fields[0].tap = etm_reg->jtag_info->tap;
fields[0].num_bits = 32;
fields[0].out_value = reg->value;
fields[0].in_value = NULL;
fields[0].check_value = NULL;
fields[0].check_mask = NULL;
fields[1].tap = etm_reg->jtag_info->tap;
fields[1].num_bits = 7;
fields[1].out_value = malloc(1);
buf_set_u32(fields[1].out_value, 0, 7, reg_addr);
fields[1].in_value = NULL;
fields[1].check_value = NULL;
fields[1].check_mask = NULL;
fields[2].tap = etm_reg->jtag_info->tap;
fields[2].num_bits = 1;
fields[2].out_value = malloc(1);
buf_set_u32(fields[2].out_value, 0, 0, 0);
fields[2].in_value = NULL;
fields[2].check_value = NULL;
fields[2].check_mask = NULL;
This piece of digital diarrhea is no doubt "standard and universally
understood" C code. Unfortunately it is also an obstacle when someone
tries to read the surrounding algorithm. If that block would say:
OUT_FIELD(fields[0], 32, reg->value);
OUT_FIELD(fields[1], 7, reg_addr);
OUT_FIELD(fields[2], 1, 0);
it would be macros you might have to look up once when you deal with
openocd for the first time, but that is a tiny price to pay for
immediately seeing the relevant parts of the action.
Btw. I added a small typo to the long version above that would not be
detected by the compiler and is difficult to make with the macro
version. Do you think someone would spot that when checking thi
Re: [Openocd-development] [patch 3/4] ARM11: fixup method table
On Sun, Nov 15, 2009 at 16:49, Michael Bruck wrote: > On Sun, Nov 15, 2009 at 16:39, Øyvind Harboe wrote: >> On Sun, Nov 15, 2009 at 4:26 PM, Michael Bruck wrote: >>> On Sun, Nov 15, 2009 at 10:08, Øyvind Harboe >>> wrote: >>>> On Sun, Nov 15, 2009 at 3:48 AM, David Brownell >>>> wrote: >>>>> Three changes: remove ARM11_HANDLER() in favor of normal structure >>>> >>>> I like this cleanup. It makes it possible to read code without >>>> special knowledge about ARM11 helper macros... >>>> >>> >>> The macro is right above the table. >> >> The problem with this sort of macros is that we have >> a dozen targets and having a dozen different >> macros gets a bit tedious. The arm11 macros >> does show that certain things are tedious in OpenOCD >> speak, but hopefully we can address those on >> in a more general fashion. > > This seems highly unlikely, quite a few contributions here lately > center around enforcing antiquated bloated syntax. The patch does not > improve legibility it merely reflects certain contributer's personal > distaste for macros. This should be contributors', plural is probably warranted here. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [patch 3/4] ARM11: fixup method table
On Sun, Nov 15, 2009 at 16:39, Øyvind Harboe wrote: > On Sun, Nov 15, 2009 at 4:26 PM, Michael Bruck wrote: >> On Sun, Nov 15, 2009 at 10:08, Øyvind Harboe wrote: >>> On Sun, Nov 15, 2009 at 3:48 AM, David Brownell wrote: >>>> Three changes: remove ARM11_HANDLER() in favor of normal structure >>> >>> I like this cleanup. It makes it possible to read code without >>> special knowledge about ARM11 helper macros... >>> >> >> The macro is right above the table. > > The problem with this sort of macros is that we have > a dozen targets and having a dozen different > macros gets a bit tedious. The arm11 macros > does show that certain things are tedious in OpenOCD > speak, but hopefully we can address those on > in a more general fashion. This seems highly unlikely, quite a few contributions here lately center around enforcing antiquated bloated syntax. The patch does not improve legibility it merely reflects certain contributer's personal distaste for macros. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [patch 3/4] ARM11: fixup method table
On Sun, Nov 15, 2009 at 10:08, Øyvind Harboe wrote: > On Sun, Nov 15, 2009 at 3:48 AM, David Brownell wrote: >> Three changes: remove ARM11_HANDLER() in favor of normal structure > > I like this cleanup. It makes it possible to read code without > special knowledge about ARM11 helper macros... > The macro is right above the table. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] arm11 etm commands pushed
On Tue, Nov 10, 2009 at 21:38, Øyvind Harboe wrote: > On Tue, Nov 10, 2009 at 9:35 PM, David Brownell wrote: >> On Tuesday 10 November 2009, Øyvind Harboe wrote: >>> On Tue, Nov 10, 2009 at 8:14 PM, David Brownell wrote: >>> > On Tuesday 10 November 2009, Ųyvind Harboe wrote: >>> >> I've pushed the new etm commands from Michael Bruck. They >>> >> are ready for testing now. I suppose they may find another final >>> >> form, but at least we have the lower level code in place... >>> > >>> > I was *really* hoping to have at least a little discussion >>> > on the list *BEFORE* such changes get rushed into the tree. >>> >>> Me as well but I didn't see any replies to Michaels postings. >>> >>> https://lists.berlios.de/pipermail/openocd-development/2009-October/012152.html >> >> I don't believe I even saw that >> >> >>> The patch has the lower level code in place + a few commands >>> to test those lower level fn's. >>> >>> I think it would make perfect sense to toss out the arm11 etm specific >>> commands and hook it up to some more general mechanism >>> when that mechanism surfaces. Meanwhile to be able to test >>> the low level code makes sense to me. >> >> Actually, providing something that doesn't hook up to the >> existing ETM support seems wrong to me. > It is merely an additional feature that does not preclude anyone from integrating arm11 into the openocd etm/etb drivers. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] 0.3.0 release status ...
On Mon, Nov 2, 2009 at 19:35, Øyvind Harboe wrote: > On Mon, Nov 2, 2009 at 7:07 PM, David Brownell wrote: >> On Monday 02 November 2009, Michael Bruck wrote: >>> Hmm, even the correction is wrong again: >>> >>> Parameter 1 is not the target name an ARM11 device. >>> should be: >>> Parameter 1 is not the target name of an ARM11 device. >> >> I'd not object to seeing that fixed. But I'll leave >> such a merge up to Øyvind, he's more on top of ARM11 >> issue ... you didn't quite provide a patch. :) >> >> At some point, feel free to start a separate thread >> for ETM issues (later hunks of that patch). > > I think we should leave out these fixes to arm11 now. My suggestion was only to fix the error message. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] 0.3.0 release status ...
Hmm, even the correction is wrong again: Parameter 1 is not the target name an ARM11 device. should be: Parameter 1 is not the target name of an ARM11 device. Michael On Mon, Nov 2, 2009 at 12:18, Michael Bruck wrote: > You could consider fixing the error message in arm11.c that you can > see in the first hunk of this diff: > > http://repo.or.cz/w/openocd/digenius.git?a=commitdiff;h=15d8c480d204a56d60c1ce565c076473b7319d13 > > Michael > > > On Mon, Nov 2, 2009 at 05:31, David Brownell wrote: >> So: We tagged RC0 about four days ago, and there seems to be >> agreement that getting it out soon is the way to go. >> >> And: all the committers have been pretty good (thanks!) about >> not doing stuff that could destabilize things. Nothing big >> enough to justify an RC1, so far... >> >> We're aiming to release by the END OF THIS WEEK! So: >> >> * If you know of anything that you think *MUST* get >> addressed before then (worth holding up the release) >> please follow up on this thread. Reference patches. >> >> CANDIDATES: [1] affecting writes to CFI flash, and >> possibly related to some FT2232H issues too. >> >> * Likewise if there's something you really *WANT* to >> see adddressed, follow it up here too. Reference >> patches, including "I'm chasing this bug". (The >> later the fix becomes available, the less likely >> it is to merge before release...) >> >> CANDIDATES: -- none (so far) -- >> >> * Same for issues that need documentation, including >> mention in release notes. >> >> CANDIDATE: libftdi 0.16 not working at full speed >> for FTx232H devices, since thinks they always use >> high speed (and 512 byte packets). >> >> In your followups, please be clear about issues which >> are "must" vs. "want". >> >> - Dave >> >> [1] >> https://lists.berlios.de/pipermail/openocd-development/2009-October/011825.html >> ___ >> Openocd-development mailing list >> Openocd-development@lists.berlios.de >> https://lists.berlios.de/mailman/listinfo/openocd-development >> > ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] 0.3.0 release status ...
You could consider fixing the error message in arm11.c that you can see in the first hunk of this diff: http://repo.or.cz/w/openocd/digenius.git?a=commitdiff;h=15d8c480d204a56d60c1ce565c076473b7319d13 Michael On Mon, Nov 2, 2009 at 05:31, David Brownell wrote: > So: We tagged RC0 about four days ago, and there seems to be > agreement that getting it out soon is the way to go. > > And: all the committers have been pretty good (thanks!) about > not doing stuff that could destabilize things. Nothing big > enough to justify an RC1, so far... > > We're aiming to release by the END OF THIS WEEK! So: > > * If you know of anything that you think *MUST* get > addressed before then (worth holding up the release) > please follow up on this thread. Reference patches. > > CANDIDATES: [1] affecting writes to CFI flash, and > possibly related to some FT2232H issues too. > > * Likewise if there's something you really *WANT* to > see adddressed, follow it up here too. Reference > patches, including "I'm chasing this bug". (The > later the fix becomes available, the less likely > it is to merge before release...) > > CANDIDATES: -- none (so far) -- > > * Same for issues that need documentation, including > mention in release notes. > > CANDIDATE: libftdi 0.16 not working at full speed > for FTx232H devices, since thinks they always use > high speed (and 512 byte packets). > > In your followups, please be clear about issues which > are "must" vs. "want". > > - Dave > > [1] > https://lists.berlios.de/pipermail/openocd-development/2009-October/011825.html > ___ > Openocd-development mailing list > Openocd-development@lists.berlios.de > https://lists.berlios.de/mailman/listinfo/openocd-development > ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] mcrmrc plan
- I would leave the check for HALTED at the target level because in
principle it depends on the capabilities of the debug interface if
this check is necessary.
- In target.c
if (CRm>7)
should be
if (CRm > 15)
Michael
On Mon, Oct 26, 2009 at 20:04, Øyvind Harboe wrote:
> On Mon, Oct 26, 2009 at 5:27 PM, Michael Bruck wrote:
>> I haven't tested the patch, but the "retire" part forgets to eliminate
>> arm11_coproc_instruction_limits[].
>>
>> The new code should also check for:
>>
>> if (arm11->target->state != TARGET_HALTED)
>> {
>> LOG_WARNING("target was not halted");
>> return ERROR_TARGET_NOT_HALTED;
>> }
>
> This is fixed in the mrcmcr branch at:
>
> http://repo.or.cz/w/openocd/oharboe.git
>
>
> --
> Øyvind Harboe
> http://www.zylin.com/zy1000.html
> ARM7 ARM9 ARM11 XScale Cortex
> JTAG debugger and flash programmer
>
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] mcrmrc plan
I haven't tested the patch, but the "retire" part forgets to eliminate
arm11_coproc_instruction_limits[].
The new code should also check for:
if (arm11->target->state != TARGET_HALTED)
{
LOG_WARNING("target was not halted");
return ERROR_TARGET_NOT_HALTED;
}
Michael
On Mon, Oct 26, 2009 at 15:00, Øyvind Harboe wrote:
> I'm committing the removal of the obsolete stuff to
> the mrcmcr branch as well. This will demonstrate that we end
> up with *less* code than we started with.
>
> Consider arm11 support for mrc/mcr before & after...
>
> I can't retire the old commands until the new ones are tested
> on real hardware though.
>
> --
> Øyvind Harboe
> http://www.zylin.com/zy1000.html
> ARM7 ARM9 ARM11 XScale Cortex
> JTAG debugger and flash programmer
>
> ___
> Openocd-development mailing list
> Openocd-development@lists.berlios.de
> https://lists.berlios.de/mailman/listinfo/openocd-development
>
>
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] mrc / mcr low level interface
On Mon, Oct 26, 2009 at 00:12, Øyvind Harboe wrote: > On Sun, Oct 25, 2009 at 11:28 PM, Michael Bruck wrote: >> On Fri, Oct 23, 2009 at 10:37, David Brownell wrote: >>> If the arm11 (armv6) stuff isn't layering on top of that, I'm >>> sort of curious why. If the reason is anything more than the >> >> It didn't seem practical at the time because it would have taken >> significantly more time. > > At the end of the day, this may end up pushing us to > define cleaner interfaces. Perhaps not such a bad > thing in the long run It isn't just about the interfaces. I would have had to understand what exactly the armv4_5 etc code does and what it's side-effects are. But the code is hopelessly bloated. Reading a simple local copy of PC is a drama of epic proportions: buf_get_u32(armv4_5->core_cache->reg_list[15].value, 0, 32) Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] mrc / mcr low level interface
On Fri, Oct 23, 2009 at 10:37, David Brownell wrote: > If the arm11 (armv6) stuff isn't layering on top of that, I'm > sort of curious why. If the reason is anything more than the It didn't seem practical at the time because it would have taken significantly more time. > name suggesting it's the wrong approach, then that's something > to fix... Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Summer coding project proposal
On Wed, Jun 24, 2009 at 17:34, Zach Welch wrote: > On Wed, 2009-06-24 at 16:00 +0200, Michael Bruck wrote: >> >> The libusb improvements certainly sound interesting, however no one >> has stepped forward to implement them or to pay someone to implement >> them. They may or may not also require some reverse engineering plus >> extensive profiling that would make them more time-consuming than the >> wrapper. So they don't seem like a near-term solution at the moment. > > Others have stated emphatically that the specifications are open and > available for the developers to take a whack at replacing their library. > Unless it is incomplete or inaccurate, the matter should be > straightforward enough. :) Heh. I have not seen a spec that documents the USB endpoints and how to interact with them. But if someone has a link to that it would be nice if that person could post that. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] openocd, ftd2xx
On Wed, Jun 24, 2009 at 23:30, Duane Ellis wrote: > Zach Welch wrote: >> Hi all, >> >> Here is the full list of GPL-compliant solutions for libftd2xx GPL >> compliance, after further review, consideration, and enumeration: >> >> 1) Documentation: build it yourself! >> 2) Build-Kit: automate the build on users' machines >> 3) XXX: to be revealed, if legal >> 4) libftdi-ftd2xx: ABI compatible with libftdi, wraps ftd2xx >> 5) sockets: separate ftd2xx into their own process space >> 6) libusb+libftdi: The Right Thing To Do ;) >> > You are missing (7) - "WinUSB" - the windows platform type solution that > is simular to libusb. > Sadly, it does not go back to Win2K - but - most (popular) others are > solved. Does WinUSB require any special privileges or signatures for a client that connects to the API? From my understanding libusb requires a kernel mode driver that needs to be installed by an admin first. If that is the case and if WinUSB can be used by the program without signing or admin privileges then WinUSB would seem the more practical approach. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Summer coding project proposal
> Given that you are in the USA you could probably patent it as a > business method and prevent anyone from using it or collecting money > for it. This should have read: "... or you could collect money for it." ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Summer coding project proposal
On Wed, Jun 24, 2009 at 13:29, Zach Welch wrote: > On Wed, 2009-06-24 at 12:23 +0200, Michael Bruck wrote: >> On Wed, Jun 24, 2009 at 11:42, Zach Welch wrote: >> > On Wed, 2009-06-24 at 11:18 +0200, Magnus Lundin wrote: >> >> Simple project for a CS student. >> >> >> >> A wrapper with a libftdi interface calling libftd2xx, as a project using >> >> a LGPL license >> >> >> >> So any user can take their binary copy of OpenOCD linked against libftdi >> >> and simply replace the libftdi dll file, no need to play with system >> >> files or drivers. >> >> >> >> Is such a library illegal ? Who would have standing to complain ? >> > >> > You are doing it to circumvent the GPL. I think that is illegal. >> > >> > You would be contravening this copyright holder's intention, which would >> > make you liable for any possible infringement that I could show. >> >> If a third party develops a libftdi.dll replacement then there is no >> reason a user can not use that replacement. The GPL license that >> applies to the user does not restrict at all what he does with the >> code or binary as long as he does not distribute binaries of it. >> >> Obviously to put the dll wrapper wrapper under a GPL+exceptions >> license it would have to be written from scratch rather than just >> copy&pasting GPLed libftdi header files (although one could ask the >> libfti author to re-license his/her files). > > No matter how you want to call the legality of the situation, I hope > that you will admit there is something clearly unethically about what > you are proposing. I hope the libftdi author(s) would never allow > re-licensing for such dubious efforts. I don't see what is unethical here. The ethical discussion might have more weight if it wasn't directed at a solution that in the eyes of many people has helped the open source software project OpenOCD to become more popular on the Windows platform. I corrected myself in a later mail on the relicensing. No relicensing is necessary because the use of headers is permitted under the LGPL. Even if it were not it would be only a minor obstacle that can be easily circumnavigated legally by recreating them from scratch. > Since it remains a legal gray area, it seems silly to go down this road > when solutions exist that are compliant, without any kind of effective > subterfuge. I don't see a legal gray area there at all. The gray area might be when distributing the replacement DLL *with* the package. >> > Furthermore, I think individuals can be held liable for "inducing" >> > infringement, which is where IANAL becomes useful in some respects. >> > >> > I have been repeatedly warning _against_ infringement and to consult >> > with an attorney on any possible solutions that you intend to distribute >> > in binary form. You should be too. >> >> There is no infringement here, so there is nothing to debate. The >> issue gets a bit murky when the replacement dll is bundled with >> OpenOCD, but that is not really necessary, the user can load it from >> elsewhere. > > The intention to design this library for the purpose of circumventing > the GPL is being documented on this list. Sure, it's murky, but the > discussion only helps to build my case to defend against this option. Your premise is wrong. The user is not restricted in what he can do with the software on his PC. This is crystal clear from the license and the FAQ. You can do all sorts of kinky stuff with the software in the sanctity of your hard drive, if you want to flip all bits or increment all jump addresses by 13 or replace a DLL, that is your right under the GPL. > The problem you will face: can anyone in this community show they were > working on that option for us, before all of these issues came to light? > Lacking a paper trail, how can you explain your motive for doing this? > If there _is_ shown to be infringement, it will be plainly intentional, > and that may mean some cash will be left over after paying the lawyers. This is not necessary. Your copyright on parts of OpenOCD does not preclude a third party or in fact this project from developing a libftdi.dll replacement for whatever purpose and under whatever license. > By all means, show me this evidence and I will permit this option. If > you cannot, then please drop it from further consideration, for the > ethical considerations alone. Or are "ethics" an unrealistic position > from which to make such a plea? Certainly, you _are_ free to ignore my > ethical stance; likewise, do not expect me to help you in those efforts. > I
Re: [Openocd-development] Summer coding project proposal
On Wed, Jun 24, 2009 at 12:23, Michael Bruck wrote: > On Wed, Jun 24, 2009 at 11:42, Zach Welch wrote: >> On Wed, 2009-06-24 at 11:18 +0200, Magnus Lundin wrote: >>> Simple project for a CS student. >>> >>> A wrapper with a libftdi interface calling libftd2xx, as a project using >>> a LGPL license >>> >>> So any user can take their binary copy of OpenOCD linked against libftdi >>> and simply replace the libftdi dll file, no need to play with system >>> files or drivers. >>> >>> Is such a library illegal ? Who would have standing to complain ? >> >> You are doing it to circumvent the GPL. I think that is illegal. >> >> You would be contravening this copyright holder's intention, which would >> make you liable for any possible infringement that I could show. > > If a third party develops a libftdi.dll replacement then there is no > reason a user can not use that replacement. The GPL license that > applies to the user does not restrict at all what he does with the > code or binary as long as he does not distribute binaries of it. > > Obviously to put the dll wrapper wrapper under a GPL+exceptions > license it would have to be written from scratch rather than just > copy&pasting GPLed libftdi header files (although one could ask the > libfti author to re-license his/her files). I missed the part where it's LGPL, then it seems to fall under the definition of "Application" and the "if the incorporated material is not limited to numerical parameters, data structure layouts and accessors, or small macros," clause applies. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Summer coding project proposal
On Wed, Jun 24, 2009 at 11:42, Zach Welch wrote: > On Wed, 2009-06-24 at 11:18 +0200, Magnus Lundin wrote: >> Simple project for a CS student. >> >> A wrapper with a libftdi interface calling libftd2xx, as a project using >> a LGPL license >> >> So any user can take their binary copy of OpenOCD linked against libftdi >> and simply replace the libftdi dll file, no need to play with system >> files or drivers. >> >> Is such a library illegal ? Who would have standing to complain ? > > You are doing it to circumvent the GPL. I think that is illegal. > > You would be contravening this copyright holder's intention, which would > make you liable for any possible infringement that I could show. If a third party develops a libftdi.dll replacement then there is no reason a user can not use that replacement. The GPL license that applies to the user does not restrict at all what he does with the code or binary as long as he does not distribute binaries of it. Obviously to put the dll wrapper wrapper under a GPL+exceptions license it would have to be written from scratch rather than just copy&pasting GPLed libftdi header files (although one could ask the libfti author to re-license his/her files). > Furthermore, I think individuals can be held liable for "inducing" > infringement, which is where IANAL becomes useful in some respects. > > I have been repeatedly warning _against_ infringement and to consult > with an attorney on any possible solutions that you intend to distribute > in binary form. You should be too. There is no infringement here, so there is nothing to debate. The issue gets a bit murky when the replacement dll is bundled with OpenOCD, but that is not really necessary, the user can load it from elsewhere. >> - FTDI ? no their libray and driver is called in accordance with >> their documentation. >> - OpenOCD ? nobody has touched a single line of OpenOCD code >> - Copyright holders of libftdi, Intra2Net AG ? no, libftdi is LGPL and >> the new library would only use the header file in accordance with LGPL >> section 3. >> >> Would it work? with a bit of tweaking I would think so. >> >> Is this a blatant attempt just to work around the problems with OpenOCD, >> GPL and libftd2xx ? What do I know ? Maybe yes, but that does not make >> it illegal. > > It might. This is a very gray area. For the love of all that is sane, > why do you want to keep pressing these fronts, when other options have > been presented that will solve the problems without any objections? TCP/IP will impose a speed penalty. It may be a nice extension for other purposes, but for daily work it is most likely not the best solution. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Summer coding project proposal
On Wed, Jun 24, 2009 at 11:48, Øyvind Harboe wrote: > On Wed, Jun 24, 2009 at 11:46 AM, Michael Bruck wrote: >> On Wed, Jun 24, 2009 at 11:30, Øyvind Harboe wrote: >>> But why should we go for such an inferior and specif solution when a more >>> general one is proposed and worked on? >> >> What are the speed/roundtrip time implications of passing all data >> through the Windows socket interfaces for the TCP/IP solution? > > Possibily neglible as OpenOCD is already highly geared towards > long latency interfaces. It would have to be measured. This may be valid for high-speed data transfers (but on arm11 for example only in the fast memwrite mode that replaces status polling with fixed delays). I doubt that for single-stepping, where the whole processor state is fetched register-by-register, the long-latency-optimizations make much of a difference. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Summer coding project proposal
On Wed, Jun 24, 2009 at 11:30, Øyvind Harboe wrote: > But why should we go for such an inferior and specif solution when a more > general one is proposed and worked on? What are the speed/roundtrip time implications of passing all data through the Windows socket interfaces for the TCP/IP solution? The dll wrappers (there are obviously two approaches which DLL is to be wrapped) seem to have the lower performance penalty. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] OpenOCD, the GPL, and FTD2XX
On Tue, Jun 23, 2009 at 20:49, Zach Welch wrote: > Why do you consider it OK to ignore what I and other contributors are > saying about the license? Why do you feel it necessary to disrespect > our copyright claims? If we interpret it thusly, why do you think that > your interpretation should be considered more valid than ours view? > What if I told you that my interpretation is based in part on having > paid an attorney to counsel me in such nuances? Would that matter? > > You are being disrespectful to contributors that have made this > decision; it is theirs to make and theirs alone. In that way, there is > no doubt that you _are_ creating problems by beating this dead horse. > What will it take to get you and others to drop this issue? During this discussion several suggestions were made how a binary distribution that interacts with ftd2xx could look like. Some of them most likely did not comply with the GPL, other probably do comply with the GPL, even in your certainly very critical view. In these cases your choice of words indicated that these GPL compatible solutions were known to you but you chose not to disclose them to the community. Are there any other solutions that you are aware of but haven't shared in this discussion? Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Will the next release 0.2.0 build on FTD2XX support?
On Sat, Jun 20, 2009 at 8:05 PM, Michael Schwingen wrote: > Zach Welch wrote: >>> BTW: one possible solution for 64-bit windows would be to ship an >>> openocd appliance - ie. a VM image containing a minimal linux system >>> together with openocd & libraries. >>> >>> Users would need to install VMware player or SUN Virtualbox to use that, >>> but would get a clean, 100% legal and working solution without the need >>> to compile/install anything beside the VM. >>> >> >> With OpenOCD linked to the FTD2XX driver inside the image? >> >> That would be a GPL violation too. >> > No, with OpenOCD linked to libusb/libftdi, running on 32-bit linux, only > inside a VM. > I am quite sure that *is* fully legal. Why not just offer a pre-configured linux-VM that cross-compiles for mingw or cygwin on the user's PC? Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [patch] there are no "arm11 memwrite X Y" commands
You are looking at the wrong code. The bug was probably introduced in command.c. Right now I have no time whatsoever to work on this and track down what was changed there. This is copy&paste from a telnet session I just tried on an r517 build that I have lying around here: Open On-Chip Debugger > arm11 memwrite burst 0 Disabled memory write burst mode. > arm11 memwrite burst 1 Enabled memory write burst mode. > RC_TOP marks a command that has further subcommands, not a command that starts at the top level. If the BOOL_WRAPPER handles the argument on a fixed position then this indicates (this was a year ago, so I don't remember and can only guess) that at the time when this was programmed the sub-commands were passed to the function without the top commands. Michael On Mon, Jun 15, 2009 at 6:02 AM, David Brownell wrote: > On Sunday 14 June 2009, Michael Bruck wrote: >> When I programmed this it used to work under arm11. If it does not >> work now, then someone broke the command code. This should be fixed >> rather than messing up the command structure. > > Then you seem to be the best candidate for that ... > > The arm11.c file went (IMO) a bit overboard with macros, but if > I mentally un-wrap them the issues seem to be that: > > (a) arm11_register_commands() uses RC_TOP to define a toplevel > command ... seems to have done so since 27-feb-2008 though, > the original checkin for "memwrite"; > > (b) the BOOL_WRAPPER thing expects enable/disable as the SECOND > param, not the third as it would be with the "arm11". > > I'm not sure I see how what got checked in ever worked with the > prefix though. > > - Dave > > > >> Michael >> >> On Sun, Jun 14, 2009 at 9:00 PM, David Brownell wrote: >> > Fix glitches in ARM11 command doc: the commands >> > are "memwrite ..." not "arm11 memwrite ...". Both >> > the texi and the source code (!) got that wrong. >> > Re-alphabetize. >> > --- >> > doc/openocd.texi | 24 >> > src/target/arm11.c | 2 +- >> > 2 files changed, 13 insertions(+), 13 deletions(-) >> > >> > NOTE: making "arm11 memwrite ..." work would >> > be cleaner, but more work. Plus it would change >> > current behavior. >> > >> > > ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [patch] there are no "arm11 memwrite X Y" commands
When I programmed this it used to work under arm11. If it does not work now, then someone broke the command code. This should be fixed rather than messing up the command structure. Michael On Sun, Jun 14, 2009 at 9:00 PM, David Brownell wrote: > Fix glitches in ARM11 command doc: the commands > are "memwrite ..." not "arm11 memwrite ...". Both > the texi and the source code (!) got that wrong. > Re-alphabetize. > --- > doc/openocd.texi | 24 > src/target/arm11.c | 2 +- > 2 files changed, 13 insertions(+), 13 deletions(-) > > NOTE: making "arm11 memwrite ..." work would > be cleaner, but more work. Plus it would change > current behavior. > > ___ > Openocd-development mailing list > Openocd-development@lists.berlios.de > https://lists.berlios.de/mailman/listinfo/openocd-development > > ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] ir/drscan and target "layering violations"
On Thu, Jun 11, 2009 at 8:23 AM, Øyvind Harboe wrote: > I was thinking that pathmove, irscan and drscan commands should > fail or at least give a warning if they are invoked while background > polling is running. > > But isn't that a "layering violation"? > > Should target tell jtag when it is doing background polling or > should jtag check when target is doing polling? You could add a locking mechanism for the JTAG resource. That wouldn't be a violation then. And it wouldn't be a bad idea in general to have since the number of interfaces to control OpenOCD from outside seems to be growing. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] assert vs. error messages
Errors in user input should detected where user input is parsed. This includes checks that pre-conditions for internal API calls are satisfied. Internal functions should use an assert. Moving user error diagnostics from the user input functions to internal functions is a clear violation of the API layers. To make this more clear, the moment you have a user input handler that calls more than one internal function the user messages you have piggy-backed on the internal APIs will become less and less meaningful to the user. The fact that we have user function like "drscan" that just maps to exactly one internal API call is merely a coincidence and should not be an excuse to let Brownian motion cause the drifting of user error diagnostics into internal API layers. Michael On Tue, Jun 9, 2009 at 9:39 AM, Øyvind Harboe wrote: > I'm satisfied that I've put the performance issue with checking at too > low a level on the radar. > > I'll leave this to Zach's judgement (he's the one who's working on it > currently), but I'm reserving the right to raise an objection later based > on measured performance problems(referring to Rick pointing out > that performance should be dealt with by measurements and > not speculation). > > I very few concerns that it will be fundamentally hard to address any > documented performance issues in this regard, unless something > radically new crops up in terms of doing *extensive* runtime checking. > > > > -- > Øyvind Harboe > Embedded software and hardware consulting services > http://consulting.zylin.com > ___ > Openocd-development mailing list > Openocd-development@lists.berlios.de > https://lists.berlios.de/mailman/listinfo/openocd-development > ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Problems with svn eol line endings and Cygwin when using Windows svn client
On Tue, Jun 9, 2009 at 1:19 PM, Zach Welch wrote:
> On Tue, 2009-06-09 at 12:29 +0200, Michael Bruck wrote:
>> IIRC I had to set
>>
>> SHELLOPTS=igncr
>>
>> globally to force cygwin bash to work with CRLF scripts. On the other
>> hand I suspect that option is what breaks the autotools generated
>> configure file for me. So I "correct" configure manually whenever it
>> is regenerated by replacing '\\r' (see full-text search) with $ac_cr.
>
> Can you provides some more details of your specific process? Basically,
> what macro(s) create the output that you need to fix up? We must be
No. It is a call to gawk, which causes the problem. The whole section
with the edit in it looks like this:
# Set up the scripts for CONFIG_FILES section.
# No need to generate them if there are no CONFIG_FILES.
# This happens for instance with `./config.status config.h'.
if test -n "$CONFIG_FILES"; then
ac_cr='
'
ac_cs_awk_cr=`$AWK 'BEGIN { print "a\rb" }' /dev/null`
if test "$ac_cs_awk_cr" = "a${ac_cr}b"; then
ac_cs_awk_cr=$ac_cr
else
ac_cs_awk_cr=$ac_cr
fi
echo 'BEGIN {' >"$tmp/subs1.awk" &&
_ACEOF
> missing something, because I do not recall this much pain in the past.
The problem has been there since I used cygwin for this (with igncr always set).
Michael
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Problems with svn eol line endings and Cygwin when using Windows svn client
IIRC I had to set SHELLOPTS=igncr globally to force cygwin bash to work with CRLF scripts. On the other hand I suspect that option is what breaks the autotools generated configure file for me. So I "correct" configure manually whenever it is regenerated by replacing '\\r' (see full-text search) with $ac_cr. Michael On Tue, Jun 9, 2009 at 11:58 AM, Øyvind Harboe wrote: > I've had a bit of problems with Cygwin and svn eol native > option. > > I thought I understood what was going on and I went ahead and > did some changes to svn head, but I have since discovered that > I do *not* understand what's going on so svn head is a bit in > limbo... > > 1. "guess-rev.sh" was busted because Cygwin will fail to execute > it when checked out with native. I ran dos2unix and removed the > native option. It now works. > > 2. I ran into problems with Makefile.am and configure.in not having > Unix line endings and Cygwin failing to run "make maintainer-clean" > deleting "ltmain.sh" and various other files. > > I did some tests and it seemed that the problem "make maintainer-clean" > problem was due to the svn native option and Cygwin getting confused. > > "Great! I thought, I know how to fix that!" So I committed a fix to remove > svn eol line endings for Makefile.am and configure.in. > > Only it didn't fix the problem w/"make maintainer-clean" > > So now I have a situation in Cygwin w/Eclipse/Windows svn client > where: > > - I don't know understand why ltmain.sh isn't removed during "make > maintainer-clean" > - I saw "ltmain.sh" being removed by "make maintainer-clean" but I > don't know what I did to make that happen and I can't reproduce > this behaviour under Cygwin. > - Makefile.am and configure.in no longer has the svn native line endings > under in svn head. > - There is clear evidence that it is *wrong* to use svn eol native for > .sh files but it is unclear what to do with other files that Cygwin > reads... > > -- > Øyvind Harboe > Embedded software and hardware consulting services > http://consulting.zylin.com > ___ > Openocd-development mailing list > Openocd-development@lists.berlios.de > https://lists.berlios.de/mailman/listinfo/openocd-development > ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] RFC: struct cleanup and more
On Wed, Jun 3, 2009 at 6:38 AM, Rick Altherr wrote: > I'm speaking in general terms, but, yes, simple accessors can definitely be > a problem depending how the program is structured. I've seen many a game > engine written in C++ with full encapsulation where a statistical profile > shows nearly all of the time in the prolog and epilog. The actual accessor > doesn't cost anything (maybe 1-2 instructions), but the setup for the stack > frame kills it. A stack frame setup forces accesses to memory so even > though the number of instructions is small, the impact on the performance is > significant. Having the instructions sit in the icache does help in terms > of instruction fetch times, but the data operations involve a > read-modify-write pattern which won't sit well in many cache structures. > > To be entirely fair, this problem with accessors mainly shows up when there > are a large number of instances of the object and the accessor is being > called as part of a traversal over the instances. > > The typical solution is to allow direct access to the data member to avoid > the stack frame setup/teardown overhead. Wouldn't you have the accessor as inlined member function in C++ (while keeping the data private)? Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [RFC] Some jtag changes
On Mon, Jun 1, 2009 at 9:41 PM, Magnus Lundin wrote: > Øyvind Harboe wrote: >> On Mon, Jun 1, 2009 at 11:39 AM, Magnus Lundin wrote: >> >>> I propose some changes to the jtag subsystem >>> >>> - Add jtag_add_statmove( endstate ) call, the code is already in >>> jtag_add_tlr() >>> >> >> Could you write the documentation for this fn as a separate proposal? >> >> > This is simply the jtag api wrapper for the JTAG_STATEMOVE command type. > Move from current state to (stable state) endstate using the current > state_table. If we are already in endstate do nothing >> You are here talking about the application code communicating to the >> driver that somehow the JTAG state machine state has changed "behind >> the scenes"...? >> > No, that concernes the use in jtag_tlr(). This is useful after reset > or some communication failure, usually signaled by a failed IR scan > verification check. In my experience we get IR verification errors > when there is some error causing the actual hw tap state and the > drivers idea about current tap state to disagree. >> I don't understand the motivation for allowing access to this fn, but I >> vaguely recall that something was written about this on the list recently. >> >> Post 0.2? >> >> Or are you talking about something along the lines of xsvf_add_statemove() >> in xsvf.c? >> >> > Without knowledge about xsvf_add_statemove, probabky so. >>> - Implement jtag_add_tlr() as >>> jtag_add_statemovet(TAP_RESET) >>> jtag_stableclocks(6) /* I am not 100% sure about this here */ >>> >> >> Why? >> >> > To remove different driver "interpretations" of how this should be done. >>> - Make sure that JTAG_STABLECLOCKS is implemented in all drivers (we >>> will soon fingd out otherwise ) >>> >> >> Don't we already have this in jtag_add_clocks()? >> > Look for JTAG_STABLECLOCKS in the current JLink driver. > > > >> Another thing I'd like first is to see if we can retire >> jtag_add_end_state(), as this >> would effectively make removing jtag_add_runtest() that little bit easier... >> >> > I do not propose to remove jtag_add_runtest. I propose to remove the > JTAG_RUNTEST command type and do the implementation in jtag.c layer. > > This can be done before or after jtag_add_end_state(), there is very > little interation. > > And why am I doing this ? > Testing a lot of reset and connect failures for different targets, > with diffrent reset configurations for both ft2232 and jlink my > conclusion is that the drivers should be smart about efficiently > flipping jtag pins using the interface haredware, but jtag command logic > and reset logic should be handled in jtag.c layer. So this proposal is > part of mw work to make at least ft2232 and jlink drivers behave as > similarily as possible. Or we could say "move common functionality upwards". I don't disagree with that goal. Currently however the lower layers have more intelligence otherwise there would be no pathmove and statemove but rather a shift_tms(data, number_of bits) function. My opinion: We should set the TAP state to TAP_INVALID on startup and resets. All functions that generate activity on the interface except jtag_add_tlr() and the functions that manipulate reset lines should produce fatal errors when called in this state. This way we can weed out many potential errors that can happen when TLR is not enforced after resets. When the device is reset this should *require* an explicit TLR. The condition should not be casually glossed over by an automagic statemove(). This code: > jtag_add_statemove(TAP_RESET) > jtag_stableclocks(6)/* I am not 100% sure about this here */ covers up an error that is produced by the unclear semantics of statemove(). In ft2232 statemove(TAP_RESET) is a NOP under certain circumstances and jtag_stableclocks(6) glosses over that in a rather crude fashion. You can modify statemove() in ft2232 (and in all other drivers that might behave similarly) to work around this, but then you are working against your stated goal to remove intelligence from that layer. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [RFC] Some jtag changes
On Mon, Jun 1, 2009 at 11:39 AM, Magnus Lundin wrote: > I propose some changes to the jtag subsystem > > - Add jtag_add_statmove( endstate ) call, the code is already in > jtag_add_tlr() > > - Remove the JTAG_RUNTEST cmd type. > > - Implement jtag_add_runtest(n) as > jtag_add_statemovet(TAP_IDLE) > jtag_stableclocks(n) /* Should be n-1 if current tap state is not > TAP_IDLE ?? */ > > - Implement jtag_add_tlr() as > jtag_add_statemovet(TAP_RESET) > jtag_stableclocks(6) /* I am not 100% sure about this here */ It might work, but it obscures what is going on. Sometimes statemove would succeed and sometimes not but then stableclocks would do the job, because the JTAG driver uses the then false state information from the upper layer. Why would you add such obscure mechanisms to the code? statemove should be a move between two defined states, not a reset from an unknown state. Especially not as long as the engine does not allow even defining the state to be unknown. jtag_add_tlr() should be implemented as a distinct command in the driver layer and not mixed into statemove. Otherwise you get (exactly as it is now) inconsistent behavior between the lower layer implementations because every driver has its own collection of if's to deal with special cases (like TLR). Michael > - Make sure that JTAG_STABLECLOCKS is implemented in all drivers (we > will soon fingd out otherwise ) > > > Regards > Magnus > ___ > Openocd-development mailing list > Openocd-development@lists.berlios.de > https://lists.berlios.de/mailman/listinfo/openocd-development > ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] RFC: struct cleanup and more
On Mon, Jun 1, 2009 at 6:00 AM, David Brownell wrote: > On Sunday 31 May 2009, Michael Bruck wrote: >> and I imagined how much worse that that would get once we mix in >> struct and enum. > > Heck, I imagined how much *better* it would be, especially > if the whitespace/layout bugs got fixed. ;) Well, I guess then go ahead and do what you have to do. Michael P.S. FYI scan_field_t is a struct, tap_state_t is an enum P.P.S. But hurry up before Zach starts quoting from K&R's section on function declarations. ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] RFC: struct cleanup and more
On Mon, Jun 1, 2009 at 5:31 AM, Zach Welch wrote: > On Mon, 2009-06-01 at 05:16 +0200, Michael Bruck wrote: >> On Mon, Jun 1, 2009 at 4:41 AM, David Brownell wrote: >> > On Sunday 31 May 2009, Michael Bruck wrote: >> >> The 'struct foo_s' syntax is code bloat that obscures the actual >> >> algorithms. 'foo_t' is shorter. >> > >> > Disagree about obscuring. And "shorter" doesn't matter here. >> > >> > If I see "foo_t" I have no idea what kind of thing it is. >> > If I see a "struct foo" there's no such confusion. >> > >> > Ergo, "foo_t" has obscured. >> >> Yes, I read that argument on this list before. By this logic most C++ >> code in existence is doomed. > > Doomed to a different style of code, yes. That's not subjective, it is > a simple fact that derives from the very essence of each language. Sounds quite dramatic. I guess the essence of C is then to put redundant keywords everywhere. Your question was: "How do these changes look in the community's eyes?". My answer was that they obscure the code (in my eyes). As you may have noticed from my absence on the u32 thread I am not really interested in these style debates as they cost too much of my time. I merely commented on this one because I was recently particularly annoyed when editing clusters of impenetrable gibberish like this here: /* This version of jtag_add_dr_scan() uses the check_value/mask fields */ extern void jtag_add_dr_scan_check(int num_fields, scan_field_t* fields, tap_state_t endstate); extern int interface_jtag_add_dr_scan(int num_fields, const scan_field_t* fields, tap_state_t endstate); extern void jtag_add_plain_ir_scan(int num_fields, const scan_field_t* fields, tap_state_t endstate); extern int interface_jtag_add_plain_ir_scan(int num_fields, const scan_field_t* fields, tap_state_t endstate); extern void jtag_add_plain_dr_scan(int num_fields, const scan_field_t* fields, tap_state_t endstate); extern int interface_jtag_add_plain_dr_scan(int num_fields, const scan_field_t* fields, tap_state_t endstate); and I imagined how much worse that that would get once we mix in struct and enum. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] RFC: struct cleanup and more
On Mon, Jun 1, 2009 at 4:41 AM, David Brownell wrote: > On Sunday 31 May 2009, Michael Bruck wrote: >> The 'struct foo_s' syntax is code bloat that obscures the actual >> algorithms. 'foo_t' is shorter. > > Disagree about obscuring. And "shorter" doesn't matter here. > > If I see "foo_t" I have no idea what kind of thing it is. > If I see a "struct foo" there's no such confusion. > > Ergo, "foo_t" has obscured. Yes, I read that argument on this list before. By this logic most C++ code in existence is doomed. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] RFC: struct cleanup and more
On Mon, Jun 1, 2009 at 4:35 AM, Zach Welch wrote:
> On Mon, 2009-06-01 at 04:15 +0200, Michael Bruck wrote:
>> The 'struct foo_s' syntax is code bloat that obscures the actual
>> algorithms. 'foo_t' is shorter.
>
> I assume you prefer u32 over uint32_t, then? ;) Nevermind
Yes I do.
> I think 'struct foo' is much more clear when reading and writing code.
>
>> Doxygen seems to recognize the foo_s and foo_t version as one an the
>> same, I don't see what problem you have there. However it will just as
>> happily accept foo_t only, like in
>>
>> typedef struct {} foo_t;
>
> The problem with the documentation is the fact that there are two types
> that refer to the same thing. The current scheme imposes extra effort
> for new developers to realize that many blah_t types are structures, and
> there is not immediate way to tell them apart without digging into
> further into the documentation or code.
>
> I think the Wikipedia page on typedefs does a good job of explaining the
> reasons to avoid them in this particular case:
>
> http://en.wikipedia.org/wiki/Typedef
>
> Neither of the K&R guidelines for using typedefs are met in this case:
> 1) These types do nothing to aid portability; they add no value here.
> 2) These types do not make a complex declaration easier to understand.
> If anything, they make a simple declaration unnecessarily opaque.
Thanks, I know the purpose of typedef. In this particular case it is a
widely-used way to simulate the C++ behavior where the struct keyword
is unnecessary.
If it was for me the foo_s version would not exist, and there would be
no duplication.
You are pulling out K&R? You can't be serious!
Michael
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] RFC: struct cleanup and more
The 'struct foo_s' syntax is code bloat that obscures the actual
algorithms. 'foo_t' is shorter.
Doxygen seems to recognize the foo_s and foo_t version as one an the
same, I don't see what problem you have there. However it will just as
happily accept foo_t only, like in
typedef struct {} foo_t;
Michael
On Mon, Jun 1, 2009 at 1:37 AM, Zach Welch wrote:
> Hi all,
>
> The following things nagged at me when I did the target_type clean-up:
>
> 1) Remove redundant structure typedefs:
> a) Entails the following steps (for each named struct "type"):
> i) s/^typedef struct type_s/struct type_s/
> ii) s/^} type_t;/};/
> iii) s/type_t/struct type_s/
> iv) Fix any messes that these commands miss or make. ;)
> b) Eliminates what are essentially duplicated symbols:
> - simplifies the Doxygen documentation tremendously.
> - eliminates style ambiguity and forward referencing.
>
> 2) Improve the documentation for the target module files.
> - would be best to wait until other cleanup is done.
>
> 3) More moving and module clean-up:
> - I target.h needs some re-organization.
> - I have a jtag.h doc patch in progress.
>
> How do these changes look in the community's eyes? These were on the
> list of things to be considered for 0.2.0, but no one addressed them.
>
> I will post patches to clean-up and removal of jtag_tap_t in reply to
> this thread, to provide better foundation for concrete discussion about
> issue #1. While I think this would help the code and documentation a
> lot, I would even go further to suggest "s/_s//" from all struct names.
>
> Cheers,
>
> Zach
>
>
> ___
> Openocd-development mailing list
> Openocd-development@lists.berlios.de
> https://lists.berlios.de/mailman/listinfo/openocd-development
>
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] build issue with current svn trunk
On Sat, May 30, 2009 at 10:08 PM, David Brownell wrote: > On Saturday 30 May 2009, David Brownell wrote: >> On Saturday 30 May 2009, Øyvind Harboe wrote: >> > Why do I need to understand what this does? >> >> You don't. You just need to accept that since you're >> working as a "maintainer", you need to enable that mode. >> >> The alternative is ugly: requiring everyone who builds >> from source to have the *right* versions of all the >> autotools things installed. > > ... where, to clarify, "source" doesn't mean "svn" > (or "git-svn") but "source distribution". Even that varies from project to project: http://gcc.gnu.org/viewcvs/trunk/configure Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Dealing with cumulative patches - and patch sets
git has the advantage of having more infrastructure (GUIs for example). But some of the stuff that quilt was designed for is buried in exotic commands in git. 'git help ' brings up the docs, 'git help git' lists all commands. 'git svn clone' creates a local git clone of an svn repo 'git svn rebase' pulls the latest changes from svn; it removes your local commits and re-applies them on top of the newest svn revision 'git stash' lets you stash away changes to the local working dir if you need a clean tree, for example for 'git svn rebase' In git you add all your changes for the next commit to an "index" or "staging area" before you run commit: 'git add' adds changes to the index 'git add -p' is an interactive command allows you to pick individual diff hunks rather than all changes in a file for inclusion in the index 'git commit' turns the changes collected in the index into a commit stuff added to the index doesn't turn up in regular 'git diff' you need 'git diff --cached' (or 'git diff --staged') to show whats in the index 'git reset --mixed' forgets the index while not touching the working dir 'git rebase' allows you to manipulate past commits, re-ordering/merging/splitting (haven't tried the latter) 'git reset --hard ' kills all revisions that were committed after and changes the working dir to (unless you branched) 'gitk' is a gui to browse through the repo and do some things like generating patches 'git-citool' is a gui that supposedly helps you with the whole index/commit thing Michael On Sun, May 31, 2009 at 4:10 AM, David Brownell wrote: > On Saturday 30 May 2009, Peter Denison wrote: >> Quilt. It's exactly the tool to do this. > > Exactly. You're working with a series of patches all > the time, so there's no special step to create one. > > It's also a good way to work with semi-stable snapshots > while you finish stabilizing your work. You develop a > series of "good" modular patches, test, debug, clean up; > updating to a new base snapshot is just reverting all > your patches, making sure the workspace is clean, then > pulling in a new version and re-applying the patches. > (They can need tweaking though.) > > It can also be done with "git" commands, but I've gotten > into the habit of using quilt instead. > > - Dave > ___ > Openocd-development mailing list > Openocd-development@lists.berlios.de > https://lists.berlios.de/mailman/listinfo/openocd-development > ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] build issue with current svn trunk
My understanding is that usually projects provide pre-generated configure files so that users who want to build the package on their system don't need the autotools. A non-maintainer in this context is someone who just wants to compile the package rather than modify any code (which would then potentially require a re-run of the autotools). Michael On Sat, May 30, 2009 at 9:58 AM, Øyvind Harboe wrote: >> You need to pass --enable-maintainer-mode to configure. > > Why isn't enable-maintainer-mode the default? > > I don't know what this option does really, but this should only be disabled > when making releases, right? > > -- > Øyvind Harboe > Embedded software and hardware consulting services > http://consulting.zylin.com > ___ > Openocd-development mailing list > Openocd-development@lists.berlios.de > https://lists.berlios.de/mailman/listinfo/openocd-development > ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] On Resets
On Thu, May 28, 2009 at 5:41 PM, Øyvind Harboe wrote: > There ARE interfaces that do not have TRST and SRST both, but > really one is much better off considering reset_config a configuration > on how to operate the target. If TRST support is *required* by the target, > e.g. XScale, then interfaces that do not support that just will not > work. Is that a documented behavior on XScale or the result of testing with OpenOCD ? Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] On Resets
On Wed, May 27, 2009 at 11:25 PM, Magnus Lundin wrote: > Michael Schwingen wrote: >> Magnus Lundin wrote: >> >>> Michael Bruck wrote: >>> >>> >>>> On Tue, May 26, 2009 at 7:47 PM, Magnus Lundin wrote: >>>> >>>> >>>>> move to TLR works for all current states. It is 7 steps with TMS high, >>>>> that >>>>> takes you to TAP_RESET froma any starting state. >>>>> >>>>> >>>>> >>>> Moving to TLR from an *unknown* state doesn't work because we pretend >>>> that there is no such thing as the TAP being in an unknown state. We >>>> rather initialize the boot state to be TAP_RESET. Which is not only >>>> false but also leads low level drivers to believe that nothing needs >>>> to be done. In particular the ft2232 driver says statemove TAP_RESET >>>> -> TAP_RESET is redundant and does not need to be executed (see >>>> ft2232_execute_statemove() ). >>>> >>>> >>>> >>>> >>>> >>> I have not had time look look closely into this but I think that what >>> must be decided is the exact semantics of state_move(end_state) >>> - Is it, as I think it should be : move to end_state, and if we are >>> already there do nothing ? That would make the implementations unnecessary complex. And you duplicate a lot of logic in each driver. I would even remove the 'if' that causes the TAP_RESET->TAP_RESET to be ignored because such an optimization should be performed uniformly for all JTAG devices by the jtag.c layer. >> I think in case of TAP_RESET, an exception to that rule might be useful. >> Would it make sense to define that a driver has to make sure a requested >> TAP_RESET end state really ends in TAP_RESET state, regardless of the >> previous state, ie. the driver needs to always perform the necessary >> number of clocks with TMS=1 without any clever optimizations? >> >> That way, upper layers would have an easy way to re-synchronize state >> even if openocd and the target do not agree on what the current state is. My preferred solution would be to keep statemove simple (just the table lookup) and add a JTAG_TLR_RESET instruction to the interface which in turn would just clock out the 1's. In the long run we could then add an undefined tap state (for example TAP_INVALID) and enforce that only the jtag_add_tlr command can be called from that. This would help finding reset problems not only after startup but also when a system reset is performed (which would set that as well). > I see your point and I basically agree, the problem is that state_move > uses the state move table(s), that currently do use 7 step reset->reset > transitions, but there is no "contract" for this. I would prefer the contract in the end to be "move within the table with well-defined start and end states" to make it as simple as possible. Whatever can be done in jtag.c should be done there to avoid code duplication and to avoid inconsistent behavior when changing JTAG interfaces. That includes the differentiation between a vanilla statemove and a TLR from undefined state. And it includes turning a statemove into a NOP if start state and end state are equal (and if that is the behavior that the community in fact wants). > I also dislike the idea of having to put exceptions like this in the > interface drivers. I think we also should look into how > jtag_add_runtest is handled because this is a similar situation where we > want to reach a certain endstate and then guarante a certain number of > transitions in this state. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] On Resets
On Tue, May 26, 2009 at 7:47 PM, Magnus Lundin wrote: > Spencer Oliver wrote: If you define your interface with only srst then the >>> >>> software method will be used. Well it did last time i checked - see jtag_add_reset for code. >>> >>> That code makes a move to TLR. This method is fine when you know in which >>> state you are, but not when OpenOCD is started and the previous TAP state is >>> undefined. >>> >>> >> >> I have not dug into jtag.c for a while, i am pretty sure it was not the >> case. >> it was a few years back however that this support was originally added. >> >> Cheers >> Spen >> >> ___ >> Openocd-development mailing list >> Openocd-development@lists.berlios.de >> https://lists.berlios.de/mailman/listinfo/openocd-development >> > > move to TLR works for all current states. It is 7 steps with TMS high, that > takes you to TAP_RESET froma any starting state. Moving to TLR from an *unknown* state doesn't work because we pretend that there is no such thing as the TAP being in an unknown state. We rather initialize the boot state to be TAP_RESET. Which is not only false but also leads low level drivers to believe that nothing needs to be done. In particular the ft2232 driver says statemove TAP_RESET -> TAP_RESET is redundant and does not need to be executed (see ft2232_execute_statemove() ). Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] On Resets
On Tue, May 26, 2009 at 10:30 AM, Spencer Oliver wrote: >> Is there any particular reason why the JTAG layer uses >> hardware resets (TRST)? >> >> I would assume that the most portable way to go to TLR, the >> one that works even if all reset lines are missing or are not >> yet or incorrectly configured would be to shift out five HIGH >> bits which leads to TLR regardless in which state the TAP is. >> > > If you define your interface with only srst then the software method will be > used. > Well it did last time i checked - see jtag_add_reset for code. > That code makes a move to TLR. This method is fine when you know in which state you are, but not when OpenOCD is started and the previous TAP state is undefined. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] On Resets
Is there any particular reason why the JTAG layer uses hardware resets (TRST)? I would assume that the most portable way to go to TLR, the one that works even if all reset lines are missing or are not yet or incorrectly configured would be to shift out five HIGH bits which leads to TLR regardless in which state the TAP is. Currently the JTAG layer doesn't even offer this simple function. So I am wondering if the people who conceived the reset machinery were unaware of this TLR mechanism. Unless there are good reasons (for example that we have cases where going to TLR vs. asserting TRST produce differing results) I think the reset lines should not be used from the JTAG layer at all, they should be available to scripts (and possibly targets, but I think even that is bad design for generic CPU drivers). My suggestion would be to expose the resets as a generic GPIO layer that is available to scripts. An initial system reset (if that is available in a system) could be generated via a configuration command. Asserting TRST is from what I understand redundant, I don't see the use in doing all the extra work that is done currently by the JTAG layer to try to use that. Also I strongly suspect that the many recent reports on the list of varying behavior when starting openocd several times in a row may be a result of failed attempts to reset the TAP state via hardware lines. At least we should consider adding the five 1's mechanism to the JTAG initialization to reduce potential for such errors. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] remove cmd_queue_cur_state
Sorry I take back my earlier comment. I didn't check tap_set_state() and only assumed that it wraps cmd_queue_cur_state. But it doesn't and I just checked the patch on arm11 and it breaks arm11. When arm11 queries cmd_queue_cur_state it wants to know in what state the previously queue command left the queue so that it can correctly queue up the next scan command. Michael On Sun, May 24, 2009 at 10:50 AM, Magnus Lundin wrote: > Dick Hollenbeck skrev: >> Zach, >> >> I think this patch is fundamentally wrong and is a disaster the moment >> it is applied. >> >> The queue sits between the jtag api and the drivers. The api calls >> track "future" state according to the most recent api call submitted >> (and put onto the back end of the queue) via the cmd_queue_cur_state >> variable. >> >> The drivers consume commands in batch mode off the back end of the >> queue. They have their own notion of state, and that is what the "cable >> helper API" is for. That API has that name deliberately so that you >> know these functions are for cable drivers, not for the jtag api layer. >> Rather than worrying about a future state, they are worrying about >> current state of the taps as the driver actually manipulates TMS and the >> clock. >> >> > Right, but the situation where this came up is when the tap state, or > actually the target jtag controller changes state and the drivers must > be informed about this. > > We cannot use cmd_queue_cur_state for this since it only records the > state for sanity checking. > > We can add a " jtag_add_forced_set_state" or someting similar. > > Or we can do a jtag_execute_queue to flush the command queue and then a > tap_set_state(). >> These two concepts are separate, and cannot be merged because of the >> queue. tap_set_state() is for current state of the taps. But realize >> there is can be some delay here also because sometimes their are low >> level commands buffered before being sent out on USB. So >> tap_set_state() only tracks to the best of its ability the current state. >> >> >> Dick >> >> > Best regards > Magnus > > ___ > Openocd-development mailing list > Openocd-development@lists.berlios.de > https://lists.berlios.de/mailman/listinfo/openocd-development > ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] remove cmd_queue_cur_state
Looks good.
Michael
On Sat, May 23, 2009 at 11:08 PM, Zach Welch wrote:
> Hi all,
>
> The attached patch removes the cmd_queue_cur_state global variable,
> replacing all uses with calls to tap_set_state() or tap_get_state().
>
> While this seems completely trivial, I wanted to post it for review.
> Any objections? All in favor?
>
> Cheers,
>
> Zach
>
> ---
> jtag/jtag.c | 13 ++---
> jtag/jtag.h | 3 +--
> jtag/zy1000.c | 4 ++--
> svf/svf.c | 9 ++---
> target/arm11_dbgtap.c | 4 ++--
> xsvf/xsvf.c | 4 ++--
> 6 files changed, 19 insertions(+), 18 deletions(-)
>
> Index: src/jtag/zy1000.c
> ===
> --- src/jtag/zy1000.c (revision 1893)
> +++ src/jtag/zy1000.c (working copy)
> @@ -709,7 +709,7 @@
>
> int interface_jtag_add_clocks(int num_cycles)
> {
> - return zy1000_jtag_add_clocks(num_cycles, cmd_queue_cur_state,
> cmd_queue_end_state);
> + return zy1000_jtag_add_clocks(num_cycles, tap_get_state(),
> cmd_queue_end_state);
> }
>
> int interface_jtag_add_sleep(u32 us)
> @@ -728,7 +728,7 @@
>
> state_count = 0;
>
> - tap_state_t cur_state=cmd_queue_cur_state;
> + tap_state_t cur_state = tap_get_state();
>
> while (num_states)
> {
> Index: src/jtag/jtag.c
> ===
> --- src/jtag/jtag.c (revision 1893)
> +++ src/jtag/jtag.c (working copy)
> @@ -94,7 +94,6 @@
>
> enum reset_types jtag_reset_config = RESET_NONE;
> tap_state_t cmd_queue_end_state = TAP_RESET;
> -tap_state_t cmd_queue_cur_state = TAP_RESET;
>
> int jtag_verify_capture_ir = 1;
> int jtag_verify = 1;
> @@ -565,7 +564,7 @@
> if (state != TAP_INVALID)
> jtag_add_end_state(state);
>
> - cmd_queue_cur_state = cmd_queue_end_state;
> + tap_set_state(cmd_queue_end_state);
> }
>
> void jtag_add_ir_scan_noverify(int in_num_fields, const scan_field_t
> *in_fields, tap_state_t state)
> @@ -1066,7 +1065,7 @@
>
> void jtag_add_pathmove(int num_states, const tap_state_t *path)
> {
> - tap_state_t cur_state = cmd_queue_cur_state;
> + tap_state_t cur_state = tap_get_state();
> int i;
> int retval;
>
> @@ -1097,7 +1096,7 @@
> jtag_prelude1();
>
> retval = interface_jtag_add_pathmove(num_states, path);
> - cmd_queue_cur_state = path[num_states - 1];
> + tap_set_state(path[num_states - 1]);
> if (retval!=ERROR_OK)
> jtag_error=retval;
> }
> @@ -1168,11 +1167,11 @@
> void jtag_add_clocks( int num_cycles )
> {
> int retval;
> -
> - if( !tap_is_state_stable(cmd_queue_cur_state) )
> + tap_state_t cur_state = tap_get_state();
> + if( !tap_is_state_stable(cur_state) )
> {
> LOG_ERROR( "jtag_add_clocks() was called with TAP in
> non-stable state \"%s\"",
> - tap_state_name(cmd_queue_cur_state) );
> + tap_state_name(cur_state) );
> jtag_error = ERROR_JTAG_NOT_STABLE_STATE;
> return;
> }
> Index: src/jtag/jtag.h
> ===
> --- src/jtag/jtag.h (revision 1893)
> +++ src/jtag/jtag.h (working copy)
> @@ -256,7 +256,6 @@
>
>
> extern tap_state_t cmd_queue_end_state; /* finish DR scans in
> dr_end_state */
> -extern tap_state_t cmd_queue_cur_state; /* current TAP state */
>
> typedef void* error_handler_t; /* Later on we can delete error_handler_t,
> but keep it for now to make patches more readable */
>
> @@ -891,7 +890,7 @@
> {
> if (end_state != TAP_INVALID)
> cmd_queue_end_state = end_state;
> - cmd_queue_cur_state = cmd_queue_end_state;
> + tap_set_state(cmd_queue_end_state);
> interface_jtag_add_dr_out(tap, num_fields, num_bits, value,
> cmd_queue_end_state);
> }
>
> Index: src/target/arm11_dbgtap.c
> ===
> --- src/target/arm11_dbgtap.c (revision 1893)
> +++ src/target/arm11_dbgtap.c (working copy)
> @@ -48,7 +48,7 @@
>
> int arm11_add_ir_scan_vc(int num_fields, scan_field_t *fields, tap_state_t
> state)
> {
> - if (cmd_queue_cur_state == TAP_IRPAUSE)
> + if (tap_get_state() == TAP_IRPAUSE)
> jtag_add_pathmove(asizeof(arm11_move_pi_to_si_via_ci),
> arm11_move_pi_to_si_via_ci);
>
> jtag_add_ir_scan(num_fields, fields, state);
> @@ -62,7 +62,7 @@
>
> int arm11_add_dr_scan_vc(int num_fields, scan_field_t *fields, tap_state_t
> state)
> {
> - if (cmd_queue_cur_state == TAP_DRPAUSE)
> + if (tap_get_state() == TAP_DRPAUSE)
> jtag_add_pathmove(asizeof(arm11_move_pd_to_sd_via_cd),
> arm11_move_pd_to_sd_via_cd);
>
> jtag_add_dr_scan(num_fields, fields, state);
> Index: src/xsvf/xsvf
Re: [Openocd-development] Broken r1870 (head)
> If you like you can try it your own: Configure recent trunk (1875) with > --enable-dummy and then > > openocd -s lib/openocd/ -f interface/dummy.cfg -f board/ti_beagleboard.cfg > > This will result in > > -- cut -- > Open On-Chip Debugger 0.2.0-in-development (2009-05-21-19:06) svn:1875 > > BUGS? Read http://svn.berlios.de/svnroot/repos/openocd/trunk/BUGS > > $URL: svn://svn.berlios.de/openocd/trunk/src/openocd.c $ > Warn : JTAG command queued, while TRST is low (TAP in reset) > current endstate: RUN/IDLE > Warn : Tap/Device does not have IDCODE > Error: JTAG tap: omap3.jrc got: 0x (mfg: 0x000, part: > 0x, ver: 0x0) > Error: JTAG tap: omap3.jrc expected 1 of 1: 0x0b7ae02f (mfg: 0x017, part: > 0xb7ae, ver: 0x0) > Error: trying to validate configured JTAG chain anyway... > Error: Could not validate JTAG scan chain, IR mismatch, scan returned 0xFF. > tap=omap3.jrc pos=0 expected 0x1 got 3 > Warn : Could not validate JTAG chain, continuing anyway... > openocd: jtag.c:889: interface_jtag_add_dr_scan: Assertion `field == > out_fields + scan->num_fields' failed. > -- cut -- > From what I can tell this fails because at the point of the call omap3.jrc is enabled but not omap3.cpu. The old code in jtag.c will silently ignore this. If this is indeed the error then this underscores the need to even further strengthen the checks because the error should have been caught in the earlier jtag_add_ir_scan() which also tried to address a TAP that is not enabled. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Broken r1870 (head)
On Thu, May 21, 2009 at 1:39 PM, Dirk Behme wrote: > Michael Bruck wrote: >> >> Dirk, did this happen when you used the drscan/irscan commands? > > Not sure, but possible. Have a look to tap-enable event of > > http://svn.berlios.de/svnroot/repos/openocd/trunk/src/target/target/omap3530.cfg Did you issue any commands before that happened in addition to what is in the config files ? Can you send a log with debug_level 3 ? Does your system produce a .stackdump file during the assert/crash that you can send ? Does the assert happen if you disable the irscan/drscan commands in omap3530.cfg ? >> Given that this is only an assert > > Well, at least OpenOCD stopped here, so it might be more that 'only' an > assert ;) As mentioned, with r1865 it still worked. The assert highlights that superfluous data was passed while older versions ignored that condition. This change was deliberate to track down errors like the one you seem to have. The problem is that the errors can be both in the code and in user commands and for the latter assert() isn't informative enough and irscan/drscan should inspect their input data more closely. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] jtag.c last set of changes [11/11]
I have attached a version without goto. - jtag.c, interface_jtag_add_ir_scan() [2/2] (version without goto): - change 'found' to bool - add comments on loops Michael On Thu, May 21, 2009 at 7:01 AM, Rick Altherr wrote: > > On May 20, 2009, at 8:30 PM, Michael Bruck wrote: > >> - jtag.c, interface_jtag_add_ir_scan() [2/2]: >> - retire variable 'found' and use goto instead >> - add comments on loops >> >> Michael >> < >> commit >> -11-506cef0.patch>___ >> Openocd-development mailing list >> Openocd-development@lists.berlios.de >> https://lists.berlios.de/mailman/listinfo/openocd-development > > > I'm not really keen on the use of goto in this case. The use of found > breaks the code into two clear phases: locate the fields for the tap, and > process the fields for that tap. The goto confuses the flow when reading > the code. > > -- > Rick Altherr > kc8...@kc8apf.net > > "He said he hadn't had a byte in three days. I had a short, so I split it > with him." > -- Unsigned > > > > commit-11b-d5c73bb.patch Description: Binary data ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Broken r1870 (head)
Dirk, did this happen when you used the drscan/irscan commands? Given that this is only an assert I think we'll have to add more checking to these commands to give meaningful errors if someone uses them incorrectly. Michael On Thu, May 21, 2009 at 8:15 AM, Rick Altherr wrote: > > On May 20, 2009, at 11:03 PM, Dirk Behme wrote: > >> >> Just want to let you know that with recent svn head r1870 I get >> >> trying to validate configured JTAG chain anyway... >> openocd: jtag.c:889: interface_jtag_add_dr_scan: Assertion `field == >> out_fields + scan->num_fields' failed. >> >> while at least r1865 seems to work. Found while playing with OMAP3 >> scripts. >> >> Best regards >> >> Dirk >> ___ >> Openocd-development mailing list >> Openocd-development@lists.berlios.de >> https://lists.berlios.de/mailman/listinfo/openocd-development > > > The assert fires when interface_jtag_add_dr_scan() is called with fields for > taps that are in bypass. Technically not a hard error, but this does mean > that unnecessary data is being passed around. Can you run in the debugger > and get a backtrace when this happens? > > -- > Rick Altherr > kc8...@kc8apf.net > > "He said he hadn't had a byte in three days. I had a short, so I split it > with him." > -- Unsigned > > > > > ___ > Openocd-development mailing list > Openocd-development@lists.berlios.de > https://lists.berlios.de/mailman/listinfo/openocd-development > > ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH] jtag.c last set of changes [11/11]
- jtag.c, interface_jtag_add_ir_scan() [2/2]: - retire variable 'found' and use goto instead - add comments on loops Michael commit-11-506cef0.patch Description: Binary data ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH] jtag.c last set of changes [10/11]
- jtag.c, interface_jtag_add_ir_scan() [1/2]: - remove temporary scan_size and use tap->ir_length instead - slight loop restructuring to reduce indentation level Michael commit-10-cb32fa6.patch Description: Binary data ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH] jtag.c last set of changes [9/11]
- jtag.c, interface_jtag_add_dr_out(): - use pointer 'field' instead of scan->fields[field_count] - restructure the main loop to clearly separate the two cases: TAP is not bypassed / TAP is bypassed (this is to keep the function similar to interface_jtag_add_dr_scan()) - fix bug where only the first output field has its tap field set - add asserts to verify that target_tap points to the one not bypassed TAP Michael commit-09-b924d63.patch Description: Binary data ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH] jtag.c last set of changes [8/11]
-jtag.c, interface_jtag_add_dr_scan(): - use pointer 'field' instead of scan->fields[field_count] - restructure the main loop to clearly separate the two cases: TAP is not bypassed / TAP is bypassed - add an assert that each non-bypassed TAP receives at least one field - add an assert that checks that no superfluous input fields were passed Michael commit-08-90a4c4e.patch Description: Binary data ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH] jtag.c last set of changes [7/11]
-jtag.c, interface_jtag_add_ir_scan(): - use pointer 'field' instead of scan->fields[nth_tap] - add assertion to ensure that input data has correct size for TAP's IR Michael commit-07-3525e31.patch Description: Binary data ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH] jtag.c last set of changes [3/11]
- jtag.c: consolidate all memory allocations in scan functions in one block, add out_fields pointer to set stage for further changes Michael commit-03-cfc2397.patch Description: Binary data ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH] jtag.c last set of changes [6/11]
- jtag.c: consolidate output scan field initialization in scan functions - jtag.c: add cmd_queue_scan_field_clone() to handle 1:1 field copies - jtag.c: fix bug where only the first output field in a dr scan has its tap field set Michael commit-06-237a658.patch Description: Binary data ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH] jtag.c last set of changes [5/11]
- jtag.c: remove unused variable 'nth_tap' from DR scan functions Michael commit-05-d633f19.patch Description: Binary data ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH] jtag.c last set of changes [4/11]
- jtag.c: Use single 'for' statement to iterate over list of TAPs in scan functions Michael commit-04-4f198db.patch Description: Binary data ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH] jtag.c last set of changes [2/11]
- add 'const' qualifier to function parameters in jtag.c that are not to be modified or freed by the function Michael commit-02-e7704e6.patch Description: Binary data ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH] jtag.c last set of changes [1/11]
- add doxygen comments to scan commands in jtag.c - move jtag_add_dr_scan next to interface_jtag_add_dr_scan to keep these function pairs together Michael commit-01-53b89a8.patch Description: Binary data ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] remove usage of TAP_INVALID & global variable to track "current" end state
I am all in favor for this change. But I would prefer to eliminate the cases first where jtag_add_end_state() is called immediately before jtag_add_*r_scan(..., TAP_INVALID). On arm11.h: ARM11_TAP_DEFAULT should not be changed. It is used only between the arm11 and the arm11_dbgtap layer. The calls to the jtag.c layer all use explicit end states. Michael On Wed, May 20, 2009 at 5:39 PM, Øyvind Harboe wrote: > This patch is intended to collect feedback on a change to the JTAG API. > > I do *not* intend to commit any of this until after a discussion & 0.2 > release: > > 1. remove TAP_INVALID as a legal argument. The caller *must* provide > an end state. > 2. deprecate jtag_add_end_state(). It is the calling code's responsibility > to figure out what the end state should be. Today this is handled > by a global variable, but the calling code can, in time, be converted to > handle this more cleanly. > > Steps: > > 1. A patch like the attached one. Remove TAP_INVALID as a valid argument. > This should be a pretty safe change and have no effect whatsoever. Commit > it and wait a week or so for any fallout to be reported(inconceivable, but > I'm just being paranoid about this step). I will redo this patch against svn > head when it is time to implement step #1(it took 5 minutes to put together). > > 2. remove all code in jtag.c that assumes that TAP_INVALID can be passed > in as an argument. Commit & wait. > > 3. remove all code in interface drivers that assumes TAP_INVALID can be > passed in. Commit & wait. > > 4. rename jtag_add_end_state() to deprecated_add_end_state(). Commit & wait. > > Done! > > At this point the code is "littered" with usage of global variables. This > usage > should be cleaned up in due course, but on a schedule that allows testing each > commit. > > > Comments? > > -- > Øyvind Harboe > Embedded software and hardware consulting services > http://consulting.zylin.com > > ___ > Openocd-development mailing list > Openocd-development@lists.berlios.de > https://lists.berlios.de/mailman/listinfo/openocd-development > > ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] Further simplifications in jtag.c
On Wed, May 20, 2009 at 12:05 AM, David Brownell wrote:
>> patch 02 renames a local variable 'x' into 'num_taps' to describe what it
>> means
>
> Except I don't like seeing declaration blocks split into
> sections with whitespace:
>
>> {
>> jtag_tap_t *tap;
>> - int x;
>> int nth_tap;
>> int scan_size = 0;
>>
>> + int num_taps = jtag_NumEnabledTaps();
>
> Just put the "int num_taps..." up where "int x" was.
The block above num_taps is gone in the final version.
jtag_NumEnabledTaps() is not a trivial initialization but rather a
step of the algorithm. That's why it is set apart from the variable
declarations.
>> +
>> /* allocate memory for a new list member */
>> jtag_command_t * cmd = cmd_queue_alloc(sizeof(jtag_command_t));
>
> Or what you did in a few other places, "type * ptrvar" should
> be just "type *ptrvar"; those are IMO neither simplifications
The "* ptrvar" was added to enable the modification in #5 that you
seemed to agree with. I personally prefer the syntax with spaces as I
generally add spaces around anything that looks like an operator for
better readability.
> nor cleanups. Ditto using the (IMO annoying) C99 feature where
> declarations don't necessarily live at the beginning of a block.
Why do you need them at the beginning of the block, considering that they
- are not used at that point
- use up an extra statement
- separate initialization from definition thus adding potential for
errors and reducing readability.
>> Again none of these patches change any algorithm.
>
> Making them all coding style fixes, not simplifications.
> And at one point some new comments.
Simple to read and simple to modify. That was the goal here. The point
was that jtag.c contains utterly trivial algorithms that are hidden in
bloated code.
> I liked #5 (other than the "type * ptrvar" issue), that does
> make the code more clear: p->q->r ==> q->r is mostly better.
>
>
> I'd kind of like to see a standard "indent" ruleset agreed on,
> so that at least the things that tool handles well could be
> somewhat automated.
>
>
>
> (By the way, suggestion: only one patch per mail. It's painful
> enough to try reviewing attachments, especially text/plain ones
> that won't get the syntax highlighting that diffs do.)
I don't think flooding the list with 10 mails is a good idea,
especially if people start replying to the mails randomly. What
extension will make your reader highlight the code?
Michael
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH] Further simplifications in jtag.c
This is a series of patches against r1834 that further simplify the
code in jtag.c to make it more readable.
patch 01 is a rather large one that renames the function arguments
'num_fields' and 'fields' into 'in_num_fields' and 'in_fields'
The rationale here is that almost all of these functions take some
input fields and produce from that a new set of output fields. The
patch sets the stage to later be able to use a local variable 'field'
or 'fields' to refer to fields rather then the current endless
repetitions of cmd->cmd.scan->fields[...] while at the same time
making it clear which one is the input argument and which is the
output data.
patch 02 renames a local variable 'x' into 'num_taps' to describe what it means
patches 03-10
add a local variable 'scan_command_t * scan' to the functions to
replace lengthy 'cmd->cmd.scan' with 'scan' to make the code more
readable
and move scan_size and a few loop variables into the local scope where
they are actually used thus reducing clutter and potential for error.
The patch files include a short commit message.
Again none of these patches change any algorithm. They are all tested
here and compile and run on arm11 (although not all of these functions
are used in arm11).
Michael
commit 7130b09dfccb1d93d68ae828e630fda3532f7bc9
Author: Michael Bruck
Date: Sun May 17 13:44:44 2009 +0200
- rename input parameters 'num_fields' and 'fields' to 'in_num_fields' and
'in_fields' in all jtag.c interface functions
diff --git a/src/jtag/jtag.c b/src/jtag/jtag.c
index 4a11658..a4f5d51 100644
--- a/src/jtag/jtag.c
+++ b/src/jtag/jtag.c
@@ -37,8 +37,8 @@
int jtag_flush_queue_count; /* count # of flushes for profiling / debugging
purposes */
-static void jtag_add_scan_check(void (*jtag_add_scan)(int num_fields,
scan_field_t *fields, tap_state_t state),
- int num_fields, scan_field_t *fields, tap_state_t state);
+static void jtag_add_scan_check(void (*jtag_add_scan)(int in_num_fields,
scan_field_t *in_fields, tap_state_t state),
+ int in_num_fields, scan_field_t *in_fields, tap_state_t state);
/* note that this is not marked as static as it must be available from outside
jtag.c for those
that implement the jtag_xxx() minidriver layer
@@ -554,45 +554,44 @@ static void jtag_prelude(tap_state_t state)
cmd_queue_cur_state = cmd_queue_end_state;
}
-void jtag_add_ir_scan_noverify(int num_fields, scan_field_t *fields,
tap_state_t state)
+void jtag_add_ir_scan_noverify(int in_num_fields, scan_field_t *in_fields,
tap_state_t state)
{
int retval;
jtag_prelude(state);
- retval=interface_jtag_add_ir_scan(num_fields, fields,
cmd_queue_end_state);
+ retval=interface_jtag_add_ir_scan(in_num_fields, in_fields,
cmd_queue_end_state);
if (retval!=ERROR_OK)
jtag_error=retval;
}
-void jtag_add_ir_scan(int num_fields, scan_field_t *fields, tap_state_t state)
+void jtag_add_ir_scan(int in_num_fields, scan_field_t *in_fields, tap_state_t
state)
{
if (jtag_verify&&jtag_verify_capture_ir)
{
/* 8 x 32 bit id's is enough for all invoations */
- int j;
- for (j = 0; j < num_fields; j++)
+
+ for (int j = 0; j < in_num_fields; j++)
{
- fields[j].check_value=NULL;
- fields[j].check_mask=NULL;
+ in_fields[j].check_value=NULL;
+ in_fields[j].check_mask=NULL;
/* if we are to run a verification of the ir scan, we
need to get the input back.
* We may have to allocate space if the caller didn't
ask for the input back.
*/
- fields[j].check_value=fields[j].tap->expected;
- fields[j].check_mask=fields[j].tap->expected_mask;
+ in_fields[j].check_value=in_fields[j].tap->expected;
+ in_fields[j].check_mask=in_fields[j].tap->expected_mask;
}
- jtag_add_scan_check(jtag_add_ir_scan_noverify, num_fields,
fields, state);
+ jtag_add_scan_check(jtag_add_ir_scan_noverify, in_num_fields,
in_fields, state);
} else
{
- jtag_add_ir_scan_noverify(num_fields, fields, state);
+ jtag_add_ir_scan_noverify(in_num_fields, in_fields, state);
}
}
-int MINIDRIVER(interface_jtag_add_ir_scan)(int num_fields, scan_field_t
*fields, tap_state_t state)
+int MINIDRIVER(interface_jtag_add_ir_scan)(int in_num_fields, scan_field_t
*in_fields, tap_state_t state)
{
jtag_tap_t *tap;
- int j;
int x;
int nth_tap;
int scan_size = 0;
@@ -632,13 +631,1
Re: [Openocd-development] [PATCH] insulate queue pointer manipulation from general jtag.c code
On Mon, May 18, 2009 at 3:53 PM, Øyvind Harboe wrote: >> I broke it down as small as possible. The change is trivial. I tested >> it here as far as possible. I can't test *all* of the functions as my >> target doesn't use them, but again, the rewrite is absolutely trivial. > > Did Rick want something *else* or something *more*? > > If he wanted something *more* then perhaps it would make sense > to commit this first round of patches? Rick wants more. It would affect the code that I am posting but my code would still be the logical intermediate step. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] insulate queue pointer manipulation from general jtag.c code
On Mon, May 18, 2009 at 3:28 PM, Øyvind Harboe wrote: > If you post patches and vouch for them, then I, or another committer > who beats me to it, will commit them promptly. > > Currently the problem is that there are patches posted, but nobody > has time to say anything as to whether they are ready for testing. > > I think it would be *greatly* helpful if we could break these changes > into a series of harmless patches and then a final patch to flip > the switch... Keeping all patches small & well understood. > > W.r.t. "vouching", I'm not saying that you should guarantee > that they work, but if you can simply state what testing you did > then we have much better understanding of the expected quality > and robustness of these patches. > > The *real* testing only happens after the patches are committed. That's > just how the OpenOCD community works at this point. > I broke it down as small as possible. The change is trivial. I tested it here as far as possible. I can't test *all* of the functions as my target doesn't use them, but again, the rewrite is absolutely trivial. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] insulate queue pointer manipulation from general jtag.c code
> It's even easier than that :-) > > jtag.c needs a few tweaks and a bit of testing and it can be committed > first without further ado. It just needs "someone" to step up to the plate > and take charge. > > https://lists.berlios.de/pipermail/openocd-development/2009-May/006586.html > > I suspect that ft2232 can be refactored in two rounds. The first without > no effect on functionality. I have not really worked on the ft2232 code and > I'd rather leave that to someone else. I looked at the jtag part of that. This does not interfere with my patch so it is safe to apply mine immediately. I would object to hunk #1 in Dick's patch because it masks a potential error. Either the added code is essentially a NOP or it is hiding an earlier error in calculating the number of fields. I don't have the time now to dig into the FT2232 changes so I can't comment on the other hunks. But there is next to zero overlap with my patch. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] insulate queue pointer manipulation from general jtag.c code
On Mon, May 18, 2009 at 1:24 PM, Øyvind Harboe wrote: >> Do we have a time frame for that? Can't Magnus split these changes up >> into small independent pieces and commit them to the trunk regularly? >> This way he can update his working tree from the trunk and get my and >> other's commits without everybody's work stalling until he has >> finished the whole set of changes. > > The problem is testing. > > Once the patches have been brought up to svn head(easy, I did it in > 10 minutes), then only testing is required and then we can commit them. > > I believe it is possible to split the changes into a number of commits > so as to facilitate bisection better. > > We *must* commit them to get them tested, but at least ft2232 should > have a decent round of testing before we commit them to svn head. > Then why not just copy the current trunk to something like branches/merge-ftd2232-vs-r1809 and then commit the change there so that it can be tested? Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] insulate queue pointer manipulation from general jtag.c code
On Mon, May 18, 2009 at 11:25 AM, Øyvind Harboe wrote: > On Mon, May 18, 2009 at 11:12 AM, Michael Bruck wrote: >> On Mon, May 18, 2009 at 10:56 AM, Øyvind Harboe >> wrote: >>>> I agree. I'll get to reviewing the patches in depth tonight and hopefully >>>> get them committed. >>> >>> It would be great to have Magnus Lundin's changes committed too, just >>> to keep things in sync... >>> >> >> Which ones do you mean ? > > jlink.c, ft2232.c and jtag.c had lots of changes from Dick Hollonbeck > (amongst others). Magnus Lundin is currently working off svn 1606 > to concentrate on those particular issues. > > I'd like to have them merged in w/svn head so we can everybody back > up to speed on svn head. > Do we have a time frame for that? Can't Magnus split these changes up into small independent pieces and commit them to the trunk regularly? This way he can update his working tree from the trunk and get my and other's commits without everybody's work stalling until he has finished the whole set of changes. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] insulate queue pointer manipulation from general jtag.c code
On Mon, May 18, 2009 at 10:56 AM, Øyvind Harboe wrote: >> I agree. I'll get to reviewing the patches in depth tonight and hopefully >> get them committed. > > It would be great to have Magnus Lundin's changes committed too, just > to keep things in sync... > Which ones do you mean ? Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] ARM11 cleanup stale dependencies with generic arm code; added comments and whitespace fixes
On Mon, May 18, 2009 at 6:51 AM, Rick Altherr wrote: > > On May 17, 2009, at 2:40 PM, Michael Bruck wrote: > >> - remove stale interdepencies between arm11 and arm7_9_common >> - added comments >> - fixed some indentation >> >> >> Michael >> ___ >> Openocd-development mailing list >> Openocd-development@lists.berlios.de >> https://lists.berlios.de/mailman/listinfo/openocd-development > > > > Is there someone more familiar with the ARM11 code that can comment on this? > From my knowledge, ARM11 is an ARMv6 part and shouldn't have a dependency > on the ARMv4/5 code. Sadly, I don't know enough about OpenOCD's > implementation of the ARM target support to know if separating them is > correct. > The other ARM code is layered so that more advanced cores re-use the code of earlier cores. But that is a huge sparsely documented package that I didn't have time to dig into during the ARM11 port. That is why ARM11 is a clean room port that is not connected to the other ARM layers. The dependencies are there because I used a skeleton from another driver as starting point and never got around analyzing what this bit of code does. I only found it now after the doxygen docs highlighted some unexpected dependencies in the header files. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH] ARM11 cleanup stale dependencies with generic arm code; added comments and whitespace fixes
- remove stale interdepencies between arm11 and arm7_9_common - added comments - fixed some indentation Michael openocd-arm11-small-fixes Description: Binary data ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] insulate queue pointer manipulation from general jtag.c code
On Sun, May 17, 2009 at 11:15 PM, David Brownell wrote: > On Sunday 17 May 2009, Rick Altherr wrote: >> Rather than combine them, I'd like to see jtag_queue_command() enforce >> validation of the command to be enqueued. Then the patterns would be: >> >> cmd = cmd_queue_alloc(); >> >> cmd->type = JTAG_SCAN; >> > > Then how about passing JTAG_* to the allocator? > > Related approach: command-specific allocators, which take > the parameters that will be fed to each command. This is going towards an OO approach. I am no against that, but I would like to get this patch through independantly of this wider discussion. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] insulate queue pointer manipulation from general jtag.c code
On Sun, May 17, 2009 at 10:54 PM, Rick Altherr wrote: > > On May 17, 2009, at 1:43 PM, David Brownell wrote: > >> On Sunday 17 May 2009, Michael Bruck wrote: >>> >>> - jtag_command_t **last_cmd; >>> - last_cmd = jtag_get_last_command_p(); >>> - >>> - *last_cmd = cmd_queue_alloc(sizeof(jtag_command_t)); >>> - (*last_cmd)->next = NULL; >>> - last_comand_pointer = &((*last_cmd)->next); >>> - (*last_cmd)->type = JTAG_SCAN; >>> >>> + jtag_command_t * cmd = cmd_queue_alloc(sizeof(jtag_command_t)); >>> + >>> + jtag_queue_command(cmd); >>> + >>> + cmd->type = JTAG_SCAN; >> >> Seems like a goodly fix ... but, couldn't all of those be wrapped >> up in a single function sort of like >> >> cmd = jtag_alloc_and_queue(JTAG_SCAN); >> >> Or STATEMOVE, PATHMOVE, etc. Agreed that queue_command() logic >> is exactly the error-prone stuff that really *needs* encapsulation; >> it should probably stay separate even with an alloc_and_queue(). I did not primarily want to compact code but separate layers. The function wraps the queue manipulation. The data structure initialization itself is much more code than just the type field so I don't like the idea of tearing it apart. >> ___ >> Openocd-development mailing list >> Openocd-development@lists.berlios.de >> https://lists.berlios.de/mailman/listinfo/openocd-development > > > Rather than combine them, I'd like to see jtag_queue_command() enforce > validation of the command to be enqueued. Then the patterns would be: > > cmd = cmd_queue_alloc(); > > cmd->type = JTAG_SCAN; > > > jtag_queue_command(cmd); > > Otherwise, you place a command in the queue before it is filled out. This > works fine today where the JTAG queue is manually flushed, but if we ever > went to a opportunistic queue draining scheme (device driver pulls next item > off queue automatically when a command finishes), you could get bogus > commands being executed. > While I agree in principle I would like to get this patch through first and discuss this suggestion later. My patches are purely code restructuring that should *not* change the algorithms at all. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] change ir_scan to bool
At least the arm11 port has a macro ZU that is intended to be used when size_t is printed. I recently saw a patch that ignored that, but I didn't comment on this error because it is like fighting windmills. Someone who regularly compiles mingw (or a cross-compile farm) would be in a better position to consistently highlight such issues. Michael On Sun, May 17, 2009 at 1:44 AM, Martin Panter wrote: > On 16/05/2009, Øyvind Harboe wrote: >> I take it GCC is used to build on Windows so we don't have >> to worry, overly much, about Visual C++ oddities w/C. > > Mingw uses GCC to compile, but it uses the VC++ C runtime library. So > I think you do need to be a bit careful sometimes, eg with the C99 > printf format codes. > ___ > Openocd-development mailing list > Openocd-development@lists.berlios.de > https://lists.berlios.de/mailman/listinfo/openocd-development > ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH] insulate queue pointer manipulation from general jtag.c code
The code in jtag.c currently manipulates the command queue pointers
directly in every function. This increases potential for errors and
makes the code less readable by distracting the reader from the core
algorithm contained in every function.
This patch removes the (repetitive) direct command queue manipulation
from the functions in jtag and packs it into a function called
jtag_queue_command().
The main change looks like this:
- jtag_command_t **last_cmd;
- last_cmd = jtag_get_last_command_p();
-
- *last_cmd = cmd_queue_alloc(sizeof(jtag_command_t));
- (*last_cmd)->next = NULL;
- last_comand_pointer = &((*last_cmd)->next);
- (*last_cmd)->type = JTAG_SCAN;
+ jtag_command_t * cmd = cmd_queue_alloc(sizeof(jtag_command_t));
+
+ jtag_queue_command(cmd);
+
+ cmd->type = JTAG_SCAN;
Patch 1 adds jtag_queue_command()
Patches 2-7 rewrite the functions in jtag.c to use jtag_queue_command()
They can be applied in any order. 2-6 are larger functions, 7 includes
all the remaining short functions
Patch 8 rewrites the direct jtag queue manipulation in svf/svf.c to
use jtag_queue_command()
Patch 9 can only be applied after 1-8 were applied. It changes
last_comand_pointer to last_command_pointer.
The patches should not change any algorithms.
Michael
diff --git a/src/jtag/jtag.c b/src/jtag/jtag.c
index dd3cbf2..0777a3a 100644
--- a/src/jtag/jtag.c
+++ b/src/jtag/jtag.c
@@ -439,6 +439,21 @@ jtag_command_t** jtag_get_last_command_p(void)
return last_comand_pointer;
}
+
+void jtag_queue_command(jtag_command_t * cmd)
+{
+ jtag_command_t **last_cmd;
+
+ last_cmd = jtag_get_last_command_p();
+
+ *last_cmd = cmd;
+
+ (*last_cmd)->next = NULL;
+
+ last_comand_pointer = &((*last_cmd)->next);
+}
+
+
void* cmd_queue_alloc(size_t size)
{
cmd_queue_page_t **p_page = &cmd_queue_pages;
diff --git a/src/jtag/jtag.c b/src/jtag/jtag.c
index 0777a3a..0a0153d 100644
--- a/src/jtag/jtag.c
+++ b/src/jtag/jtag.c
@@ -590,28 +590,26 @@ void jtag_add_ir_scan(int num_fields, scan_field_t
*fields, tap_state_t state)
int MINIDRIVER(interface_jtag_add_ir_scan)(int num_fields, scan_field_t
*fields, tap_state_t state)
{
- jtag_command_t **last_cmd;
jtag_tap_t *tap;
int j;
int x;
int nth_tap;
int scan_size = 0;
- last_cmd = jtag_get_last_command_p();
-
/* allocate memory for a new list member */
- *last_cmd = cmd_queue_alloc(sizeof(jtag_command_t));
- (*last_cmd)->next = NULL;
- last_comand_pointer = &((*last_cmd)->next);
- (*last_cmd)->type = JTAG_SCAN;
+ jtag_command_t * cmd = cmd_queue_alloc(sizeof(jtag_command_t));
+
+ jtag_queue_command(cmd);
+
+ cmd->type = JTAG_SCAN;
/* allocate memory for ir scan command */
- (*last_cmd)->cmd.scan = cmd_queue_alloc(sizeof(scan_command_t));
- (*last_cmd)->cmd.scan->ir_scan = true;
+ cmd->cmd.scan = cmd_queue_alloc(sizeof(scan_command_t));
+ cmd->cmd.scan->ir_scan = true;
x = jtag_NumEnabledTaps();
- (*last_cmd)->cmd.scan->num_fields = x; /* one field per device */
- (*last_cmd)->cmd.scan->fields = cmd_queue_alloc(x *
sizeof(scan_field_t));
- (*last_cmd)->cmd.scan->end_state = state;
+ cmd->cmd.scan->num_fields = x; /* one field per device */
+ cmd->cmd.scan->fields = cmd_queue_alloc(x * sizeof(scan_field_t));
+ cmd->cmd.scan->end_state = state;
nth_tap = -1;
tap = NULL;
@@ -628,9 +626,9 @@ int MINIDRIVER(interface_jtag_add_ir_scan)(int num_fields,
scan_field_t *fields,
assert(nth_tap < x );
scan_size = tap->ir_length;
- (*last_cmd)->cmd.scan->fields[nth_tap].tap = tap;
- (*last_cmd)->cmd.scan->fields[nth_tap].num_bits = scan_size;
- (*last_cmd)->cmd.scan->fields[nth_tap].in_value = NULL; /* do
not collect input for tap's in bypass */
+ cmd->cmd.scan->fields[nth_tap].tap = tap;
+ cmd->cmd.scan->fields[nth_tap].num_bits = scan_size;
+ cmd->cmd.scan->fields[nth_tap].in_value = NULL; /* do not
collect input for tap's in bypass */
/* search the list */
for (j = 0; j < num_fields; j++)
@@ -638,8 +636,8 @@ int MINIDRIVER(interface_jtag_add_ir_scan)(int num_fields,
scan_field_t *fields,
if (tap == fields[j].tap)
{
found = 1;
- (*last_cmd)->cmd.scan->fields[nth_tap].in_value
= fields[j].in_value;
-
(*last_cmd)->cmd.scan->fields[nth_tap].out_value = buf_cpy(fields[j].out_value,
cmd_queue_alloc(CEIL(scan_size, 8)), scan_size);
+ cmd->cmd.scan->fields[nth_tap].in_value =
fields[j].in_value;
+ cmd->cmd.scan->fields[nth_tap].
Re: [Openocd-development] [PATCH] change ir_scan to bool
(I thought I replied to this, but it seems the mail got lost.) I would suggest to apply it if no one objects rather than to wait if someone chimes in on this trivial change. Also bool is C99 Michael On Sat, May 16, 2009 at 2:37 PM, Øyvind Harboe wrote: > Hmmm I love true & false as much as the next guy + I'd even like to > see some restrained usage of C++ instead of all the ad hoc constructs > in C that mimic object oriented features... > > I think it would be wrong of me to commit this unless there is somebody > else that speaks up for it too > > -- > Øyvind Harboe > Embedded software and hardware consulting services > http://consulting.zylin.com > ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH] change ir_scan to bool
Changing ir_scan to bool more clearly illustrates its semantics and use: - change 'ir_scan' from 'int' to 'bool' to document its semantics Michael openocd-ir_scan-to-bool-290d4d1 Description: Binary data ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] massive #include reductions
The patches break compile on cygwin. Please apply the attached patch to fix this. The log message is essentially the same as with r1573: Michael Bruck : sys/select.h must preceed windows.h. The patch also changes "windows.h" to similar to how it's used in replacements.h Michael On Mon, May 11, 2009 at 7:54 AM, Zach Welch wrote: > Hi all, > > I have finished committing my outstanding patches that eliminate the > redundancies in OpenOCD header file #include directives, which resulted > in the elimination of about 1000 lines of code and helped decouple > various parts of the system. Work remains to be done on this later. > > Let me know if I managed to break the build; however, I tried to be > careful to ensure that each incremental patch resulted in a working tree > and that little was actually removed. I found that even more header > files may be removed from system.h, but I was not certain that those > changes would be portable. We can experiment at a later time. > > Assuming these changes were safe, I will continue on my quest to > identify and eliminate layering violations in the system. I have > already marked the few that exist in the helper module, so I hope > someone will take the initiative to fix those issues to remove these > unnecessary couplings. I fear that these will be the easy ones > > Cheers, > > Zach > > ___ > Openocd-development mailing list > Openocd-development@lists.berlios.de > https://lists.berlios.de/mailman/listinfo/openocd-development > openocd-cygwin-ftd2232-fix Description: Binary data ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH] add 'const' qualifier to fileio interfaces
- add 'const' qualifier to fileio interfaces - make internal fileio libc wrappers 'static inline' Michael openocd-const-for-fileio-6bedc2b Description: Binary data ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH] add 'const' qualifier to binarybuffer interfaces
- add 'const' qualifier to binarybuffer interfaces - use official C99 'inline' qualifier Michael openocd-const-for-binarybuffer-6c85e09 Description: Binary data ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] [PATCH] ARM11 C99 updates
openocd-arm11-indentation-a07a76e - indentation fix openocd-arm11-for-scoping-c593642 - change 'for' local variable declarations to C99 syntax Michael openocd-arm11-indentation-a07a76e Description: Binary data openocd-arm11-for-scoping-c593642 Description: Binary data ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] ft2232.c major re-work and clock reducing tms_seq support
On Sun, May 3, 2009 at 10:38 PM, Rick Altherr wrote: >> >> In the next few weeks I would like to prepare a roadmap document for where >> I think a project like this one should go. I will make that available to >> this group. That will basically be done to determine who and how many other >> folks would see my vision as an attractive destination. From that reaction >> I will then decide whether to make my fork public or not. But the idea that >> any such development could be done by pouring it through some tiny dinner >> glasses is silly, and economic suicide. >> > > So you appear to be settled on forking. Good luck. I wish you well. > Please do not use this list to discuss anything related to your fork. I want to read Dick's proposal here on this list. I am a bit surprised that people around here would be so close-minded that they don't even want to hear other people's ideas. Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] #if vs. #ifdef, autotools vs. Cmake
On Sun, May 3, 2009 at 1:03 AM, Zach Welch wrote: > On Sun, 2009-05-03 at 00:40 +0200, Michael Schwingen wrote: >> Zach Welch wrote: >> > If the header is required, the configure step should fail when it is not >> > found in the system. The rule is to fail as early as possible. >> > >> > Which headers? >> > >> Not system headers - I was thinking about in-project headers (either >> user-edited, config.h or similar) that define some kind of >> INCLUDE_FEATURE_XXX switch. When multiple source files depend on that >> switch, forgetting to include it in one of them can get "interesting" >> results when using #ifdef. > > These type of features can be handled by --enable-feature-xxx options. > > The autoconf 2.63 info pages are great documentation. Section 4.9.1 > explains the config.h header fairly well, while Section 6.2.1 explains > the differences between #if and #ifdef: > > """ > Both `#if HAVE_STDBOOL_H' and `#ifdef HAVE_STDBOOL_H' will work with > any standard C compiler. Some developers prefer `#if' because it is > easier to read, while others prefer `#ifdef' because it avoids > diagnostics with picky compilers like GCC with the `-Wundef' option. > """ > > As you can imagine, I favor #ifdef for this reason. I can't see your point. The idea of -Wundef is to warn you if you forgot to include a configuration file that defines variables that you use in your source code. And you say you favor #ifdef over #if because with #ifdef you will not be warned about your mistake ?! Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] #if vs. #ifdef, autotools vs. Cmake
On Sat, May 2, 2009 at 12:26 PM, Michael Schwingen wrote: > Zach Welch wrote: >> With autotools, #ifdef should be preferred. For example, HAVE_FOO_H >> will be defined 1 by the AC_CHECK_HEADERS([foo.h],[1],[Comment]) macro; >> configure defines the symbol to the given value if foo.h is present, and >> the symbol is not defined if absent. In both cases, the definition will >> exist in config.h, but it will be commented out when not defined; the >> same would be true for the OpenOCD symbols, except that they are defined >> explicitly. >> > I don't know about autotools, but I do prefer #if, because that way, you > get warnings if the correct headers are not included. When using #ifdef, > a missing header may lead to wrong code without a warning. > > cu > Michael > I prefer #if because it is a clearer syntax: #if IS_WIN32 #if ! IS_WIN32 #if IS_WIN32 || IS_UNIX vs. #ifdef IS_WIN32 #ifndef IS_WIN32 #if defined(IS_WIN32) || defined(IS_UNIX) The ! syntax is more expressive than the 'n' buried somewhere in between. The boolean operations are more focussed on what is going on and they are easier to extend (thus less error prone). Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Build problem with r1588, jim.c
If it is a GNU specific thing then assuming that more often than not this package is used with glibc I would suggest keeping #if !HAVE_UNISTD_H || IS_DARWIN rather than listing all variations that have environ in unistd.h. As an alternative a test in ./configure would be a solution. Michael On Sat, May 2, 2009 at 2:36 PM, Michael Fischer wrote: > Hello, > > the new version r1589 do not solve the problem, > becasue the unistd.h looks like: > > #define getpagesize() PAGE_SIZE > > No information about environ here. > > Regards, > > Michael > ___ > Openocd-development mailing list > Openocd-development@lists.berlios.de > https://lists.berlios.de/mailman/listinfo/openocd-development > ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] #if vs. #ifdef, autotools vs. Cmake
I would like to start another holy war while we are at it: 1. Are false preprocessor variables in OpenOCD specified by not defining a variable or by defining it as 0 ? 2. config.h generated by autotools and cmake use different paradigms for this 3. several files (including my latest patch for jim.c) use #if(n)def HAVE_... which makes no sense with the autotools generated config.h see: svn diff -r1560:1572 src/helper/jim.c 4. Mildly related: what is _WIN32 ? Michael ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
