On Sun, Nov 15, 2009 at 23:00, David Brownell <davi...@pacbell.net> 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 the start, obviously.
Looks like the idea hasn't caught on since then.
>> 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 this in as
>> a patch?
>
> Depends on the level of checking. But on the other hand, when you
> submit a buggy patch, that's basically your problem. One that's
> addressable by testing. And fixable by a followup patch. (Fixed
> before the bug gets released, one hopes...)
"Shit happens" is not a strategy.
>> And if it is not a typo but an obscure special case for a
>> given CPU, would you, when inheriting that code for another CPU,
>> notice a small difference that is buried under that pile of repetitive
>> code that you skim over at best?
>
> This is why I tell developers to provide comments. Specifically,
> when something works around hardware bugs, reference those docs
> (where available, else at least comment) to reduce such problems.
>
> I'm sure we've all had problems with flakey code someone else
> wrote. There's not a lot we can do about it, other than try
> to minimize such problems in code we hand to someone else.
I was addressing specifically the side-effects of code that contains
endless duplications that bury important details. Such code can in the
context of archaic programming languages like C benefit from macros.
Michael
_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
