Re: [Openocd-development] [PATCH] Do not try to examine targets with disabled taps

2009-06-07 Thread David Brownell
On Sunday 07 June 2009, Duane Ellis wrote:
> But - today I think what you mean is this:
> 
> Today, all target reset handlers are done like this:  
>     $_targetname  configure -event reset-init  { CURLYBRACE }

They aren't "all" like that, though.  So the solution you list
below can't always work ... though the seeds of a workaround
are found there.


> TCL variable parsing rules mean that things in { CURLYBRACE } are not 
> not expanded right now.
> 
> The *could* be done like this:  
>     $_targetname  configure -event reset-init  "double-quoted-string"
> 
> In the double-quoted case, the "double-quoted" method would cause the 
> embeded $_TARGETNAME to be expanded prior to invoking the "configure" 
> sub command.


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


Re: [Openocd-development] [PATCH] Do not try to examine targets with disabled taps

2009-06-07 Thread Duane Ellis
duane> $TARGETNAME mdw

david> Though "mdw" is really impractical for scripting.

yea, it's probably a very bad example, one probably needs to use 
'$targetname mem2array" or "$targetname array2mem", or we can create a 
helper sub-command

david>  A third option: too painful to use. How exactly is $_TARGETNAME
david>  going to get *into* the event handler since nothing binds the right
david>  value to that name??

david> I'll tell you how. By hard-wiring a particular target name.

I presume you mean the $_TARGETNAME  will not be valid when the event is 
invoked. TK  handles events with things like %w for the window path, 
treating the event handler body sort of like an "sprintf()" format 
string.  Sadly, JIM is not that complicated, I thought about adding a %T 
for the current target name. That would be a good future enhancement.

But - today I think what you mean is this:

Today, all target reset handlers are done like this:  
$_targetname  configure -event reset-init  { CURLYBRACE }

TCL variable parsing rules mean that things in { CURLYBRACE } are not 
not expanded right now.

The *could* be done like this:  
$_targetname  configure -event reset-init  "double-quoted-string"

In the double-quoted case, the "double-quoted" method would cause the 
embeded $_TARGETNAME to be expanded prior to invoking the "configure" 
sub command.

david>  Some of that's a general event handler issue: the event handlers 
have no invocation-specific context.

There is no doubt in my mind there are giant dragons we do not know about.

david>  [snip] But "reset halt"means I get a bare CPU, in need of 
serious re-init, and using a slow JTAG clock...)

Good example of that is a complex multi-target board.  However, that 
senario presents an N-way way combination that we cannot today make 
reusable code for. We can, however document each thing seperately - so 
that the user/victim can figure out how to weave the two reset processes 
they need together.

-Duane.


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


Re: [Openocd-development] [PATCH] Do not try to examine targets with disabled taps

2009-06-07 Thread David Brownell
On Saturday 06 June 2009, Rick Altherr wrote:
> > Again, we don't have such cases today.  We can speculate all kinds
> > of things, but in the absence of real hardware I don't think the
> > results of speculation are compelling.  Plus, "$target_name curstate"
> > has a very limited range of state outputs, and it would need to grow
> > to accomodate such states.
> 
> Actually, many Cortex-A8 designs include power/clock gating for the  
> processor core.

And some earlier ARMs do strange things in idle states too.

ISTR that "wait for interrupt" (WFI) instructions may need
special treatment with emulators in use; haven't had much
need to look at it myself, before now.


> Such devices may have the TAP enabled, but the target   
> isn't enabled because the power/clock is turned off to it.  I'm not  
> sure that a "$target_name curstate" is the right idea either since as  
> you pointed out, it would need to grow to handle these cases and we  
> won't necessarily know what all of them are.

I think those are somewhat orthogonal issues though.

Power gating is through a separate control framework,
which one hopes is accessible without the CPU ... that
seems to have been an 1149.7 goal (class T1 has power
states), also something of a Nexus goal (in the sense
that there seem to be bus access primitives that don't
go through the CPU, I recall it as a separate TAP).

Clock gating for ARM cores is coupled to stuff like
the WFI instruction, but once the core is in a real
debug mode at least some of them use that trick of
having TCK drive the pipeline at "debug speed".

At any rate ... we agree that "curstate" is a trifle
weak as models go.


> > The IEEE 1149.7 stuff seems to incorporate a "tap state" model of
> > sorts ... parts relevant to OpenOCD would be the power states, and
> > whether it's routed into the scan chain or not.  I'd say those use
> > cases are relatively "near future".  Not so with "target state".



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


Re: [Openocd-development] [PATCH] Do not try to examine targets with disabled taps

2009-06-06 Thread David Brownell
On Saturday 06 June 2009, Duane Ellis wrote:
> duane>  $TARGETNAME mdw

Though "mdw" is really impractical for scripting.  The "memread32"
thing would be better ... but notice that *it* ignores $TARGETNAME
too, for much the same reason other scripts can't use it.


> david>  You'll notice most of the reset-init event handlers can't use that.
> 
> CAN'T - or "just happen to not use that" - Big difference.

A third option:  too painful to use.  How exactly is $_TARGETNAME
going to get *into* the event handler since nothing binds the right
value to that name??

I'll tell you how.  By hard-wiring a particular target name.  Which
means non-reusable code.  But ... the entire point of switching to
that new "$target_name mww addr value" is reusability, right?  By
decoupling from the "current target" notion.  Else there's no reason
not to continue using un-adorned "mww"...


Some of that's a general event handler issue:  the event handlers
have no invocation-specific context.  They have to be coded to know
they're working on a specific target and event.  They can't know if
the "reset-init" code is going to run later, or if the reset mode
is "run".

(Example:  "reset run" normally means the first stage loader will
run and set up clocks, so jtag_khz can be upped from pokey-slow to
something non-painful.  Likewise with "reset init".  But "reset halt"
means I get a bare CPU, in need of serious re-init, and using a slow
JTAG clock...)


> There's a subtle thing here that I did when I created these new 
> commands, I quietly enforced inline documentation of parameters in the 
> script files.
> 
> How?  I *COULD* have made the parameters positional, ie: 'the 5th 
> parameter is the tap name' - but i did not on purpose. By *REQUIRING* a 
> prefix it serves as *EXPLICIT* documentation of the parameters purpose, 
> one does not need a comment line above the script command explaining the 
> positional parameters :-) 
> 
> Look at the "flash bank" command as a *nasty* example.
> (tcl/target/sam7x256.cfg - line 48)

Also known as "require an armload of zeroes before the only
useful parameter".  What's the "preferred" number of zeroes
lately ...  a dozen?  Sooo easy to lose count there.  :(


> It's not thrash if it becomes *in*line* documentation,

I'm in favor of non-positional paramters, but that's unrelated
to my point:  that the changes you suggest touch a *LOT* of code,
forcing a flag day ... but for relatively minor benefit.

One could for example adopt the coding convention incrementally,
withoug the "flag day" provision of your "part #1".  No code
thrash at all.


> think +2 years  
> from now, with +10 more targets, and +10 more boards, they will be 
> better documented, that is the bigger win.

If only ten more targets and boards appear in two years, this
project won't have lived up to a fraction of its potential.
Heck, I've got at least five more ARM boards I could hook up,
and a few non-ARM ones.

Expect at least ten more boards by the 0.3 release, and that
should be a more realistic model.  Maybe they won't all have
nontrivial configs -- as in, DRAM initialized, and OpenOCD
can replace their bootloaders -- but I think there must be
a lot of "low hanging fruit" lurking.

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


Re: [Openocd-development] [PATCH] Do not try to examine targets with disabled taps

2009-06-06 Thread Duane Ellis
duane>  $TARGETNAME mdw

david>  You'll notice most of the reset-init event handlers can't use that.

CAN'T - or "just happen to not use that" - Big difference.

By design, they should be able to do exactly that, see:

 src/target/target.c - lines 3559 ... 3731

By design, it should work, that is why for example how [$TARGETNAME cget 
-chain-position] works.

==

duane>  (B)  Hidden in C code... effectively enforced, like above, see 
 target.c
duane>  - line 3444..3456

david>  That is, the "-chain-position" logic should change?

Yes, exactly. What is happening here is the "tap_by_jim_obj()" is passed 
the argument to the -chain-position parameter.

I suggest instead, doing a simple "sprintf()" and add ".tap" as a suffix 
here
That can be done *IN*CODE* - or in the CFG file...

One could argue (2)(A) or (2)(B) - either way... it's a toss up.

=

david>  If one were to take your (1) forcing the ".tap" suffix, then IMO
david> the correct route is to take your (2A) and change all scripts,
david> plus (1a) update the TAP naming convention to tell folk not to
david> use "tap" themselves, and (1b) when creating targets, reject
david> any target name with a ".tap" suffix.

I do like your approach  - it *enforces* a specific naming scheme that 
is unambiguously clear that "foo.tap" is the *TAP* and not the thing the 
tap is controlling, be that "boundary scan" or "a cpu", or a "embedded 
flash".

david>  This seems to me like it'd be much thrash for not much benefit.

In consumer electronics, the most important thing is: "Can the customer 
understand/install the product without using the manual".   In our case, 
that will be impossible, but reducing manual lookups is a good thing.

There's a subtle thing here that I did when I created these new 
commands, I quietly enforced inline documentation of parameters in the 
script files.

How?  I *COULD* have made the parameters positional, ie: 'the 5th 
parameter is the tap name' - but i did not on purpose. By *REQUIRING* a 
prefix it serves as *EXPLICIT* documentation of the parameters purpose, 
one does not need a comment line above the script command explaining the 
positional parameters :-) 

Look at the "flash bank" command as a *nasty* example.
(tcl/target/sam7x256.cfg - line 48)

Sure, it is extra typing - but - it is *far*less* end user and developer 
confusion, after a few parameters I can't keep track of the order of things.

It's not thrash if it becomes *in*line* documentation, think +2 years 
from now, with +10 more targets, and +10 more boards, they will be 
better documented, that is the bigger win.

-Duane.

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


Re: [Openocd-development] [PATCH] Do not try to examine targets with disabled taps

2009-06-06 Thread Rick Altherr




On Jun 6, 2009, at 4:27 PM, David Brownell wrote:


[ second part of reply, focussed on before-0.2.0 ]

On Saturday 06 June 2009, Rick Altherr wrote:

On Jun 6, 2009, at 1:20 PM, David Brownell wrote:


Which just
points out another conceptual problem ... either (a) "target  
create"

should force target and TAP names to be the same (so "-chain-
position"
becomes redundant), *or* (b) we need a "list enabled taps"
primitive.


Minor nit:  I should have written "mechanism" not primitive.

And part of why I think that (a) is preferable is that it would let
us avoid creating parallel "$target_name tapisenabled" predicates
to match the already-working "jtag tapisenabled $tap_name" one.



Such a redundancy wouldn't be great, but relying on target_name ==
tap_name isn't good for long-term extensibility.  Chip designers like
to lots of strange things.  Imagine a single TAP that has the debug
registers for two separate target types.  You can't have 2 targets
with the same name even though they use the same TAP.


I had imagined that, but realized that since we don't have any such
monstrosity today, we could defer that issue until later if we wanted
to go the simpler "require $TAPNAME == $TARGETNAME" route.  :)



It doesn't make the code any simpler and doesn't change the typical  
case for the end user either.  It just limits the flexibility and  
forces a code change if we ever run into such a device.  $TAPNAME ==  
$TARGETNAME can definitely be the typical case, but we should support  
$TAPNAME != $TARGETNAME since the cost to do so is minimal.





 Of course, the
'target $target_name isenabled' isn't always a direct translation to
checking if the TAP is enabled.  There could easily be some other set
of items within the target's registers that determine if it is
enabled.  In the simple case, it can be implemented as just looking  
up

the tap name and asking the JTAG layer if the tap is enabled.  A more
complex case would likely always check if the TAP is enabled and then
perform other checks.  I can see value in a general "target is
enabled" command.


Again, we don't have such cases today.  We can speculate all kinds
of things, but in the absence of real hardware I don't think the
results of speculation are compelling.  Plus, "$target_name curstate"
has a very limited range of state outputs, and it would need to grow
to accomodate such states.



Actually, many Cortex-A8 designs include power/clock gating for the  
processor core.  Such devices may have the TAP enabled, but the target  
isn't enabled because the power/clock is turned off to it.  I'm not  
sure that a "$target_name curstate" is the right idea either since as  
you pointed out, it would need to grow to handle these cases and we  
won't necessarily know what all of them are.



The IEEE 1149.7 stuff seems to incorporate a "tap state" model of
sorts ... parts relevant to OpenOCD would be the power states, and
whether it's routed into the scan chain or not.  I'd say those use
cases are relatively "near future".  Not so with "target state".



Another is to avoid the nastiness inherent in the current capability
of running "target configure -chain-position foo.bar" *after* the
target has been set up.  I can't ever see goodness coming from  
that...




Yup.  We definitely should partition things into init-time settings
and run-time settings.  There are a number of things that really
should only be set on target (or tap) creation.  Imagine someone
trying to change the width of the TAP IR at runtime.


Part of why that's not allowed.  :)

It's the target stuff that's goofy that way, not TAP stuff.



I'm aware.  I was pointing out the parallel.




That is, it would help keep the layer distinction:  there's a TAP
layer, and a target layer.  TAP operations don't work on targets,
and vice versa.

(Some TAPs are isomorphic to targets too, and in the simplest  
systems

there may be only one tap == a target.)





Right.  I don't think our command structure today does a very good  
job

of keeping them separate or distinguishing which layer a command is
part of.  If we group the TAP commands under the 'jtag' command and
target commands under the 'target' command, it is _much_ clearer.


So to get back to the real "it's *BROKEN* and 0.2.0 needs a fix"
issue ... sounds like you would be happier with

$target_name tapname

command to return the TAP, so the startup.tcl loops would look
something like

if { [jtag tapisenabled [$t tapname]] == 0 } {
continue
}

or something similar, instead of assuming the target is enabled.

Yes?


Either that or add '$target_name isenabled' that has an implementation  
that does '[jtag tapisenabled [$target_name tapname]]'.  The latter  
means we won't need to revisit this when we do the full '$target_name  
isenabled' handling.





And fixing "$target_name configure ..." so it disallows changing the
target po

Re: [Openocd-development] [PATCH] Do not try to examine targets with disabled taps

2009-06-06 Thread David Brownell
On Saturday 06 June 2009, David Brownell wrote:
> if { [jtag tapisenabled [$t tapname]] == 0 } {
> continue
> }

Turns out I can already:

if {[jtag tapisenabled [$t cget -chain-position]] == 0} {
continue
}

so can make $SUBJECT work with no new primitives.

There are plenty of other messes in the vicinity though.  ;)

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


Re: [Openocd-development] [PATCH] Do not try to examine targets with disabled taps

2009-06-06 Thread David Brownell
[ second part of reply, focussed on before-0.2.0 ]

On Saturday 06 June 2009, Rick Altherr wrote:
> On Jun 6, 2009, at 1:20 PM, David Brownell wrote:
> 
> >>>   Which just
> >>> points out another conceptual problem ... either (a) "target create"
> >>> should force target and TAP names to be the same (so "-chain- 
> >>> position"
> >>> becomes redundant), *or* (b) we need a "list enabled taps"  
> >>> primitive.
> >
> > Minor nit:  I should have written "mechanism" not primitive.
> >
> > And part of why I think that (a) is preferable is that it would let
> > us avoid creating parallel "$target_name tapisenabled" predicates
> > to match the already-working "jtag tapisenabled $tap_name" one.
> >
> 
> Such a redundancy wouldn't be great, but relying on target_name ==  
> tap_name isn't good for long-term extensibility.  Chip designers like  
> to lots of strange things.  Imagine a single TAP that has the debug  
> registers for two separate target types.  You can't have 2 targets   
> with the same name even though they use the same TAP.  

I had imagined that, but realized that since we don't have any such
monstrosity today, we could defer that issue until later if we wanted
to go the simpler "require $TAPNAME == $TARGETNAME" route.  :)


>Of course, the  
> 'target $target_name isenabled' isn't always a direct translation to  
> checking if the TAP is enabled.  There could easily be some other set  
> of items within the target's registers that determine if it is  
> enabled.  In the simple case, it can be implemented as just looking up  
> the tap name and asking the JTAG layer if the tap is enabled.  A more  
> complex case would likely always check if the TAP is enabled and then  
> perform other checks.  I can see value in a general "target is  
> enabled" command.

Again, we don't have such cases today.  We can speculate all kinds
of things, but in the absence of real hardware I don't think the
results of speculation are compelling.  Plus, "$target_name curstate"
has a very limited range of state outputs, and it would need to grow
to accomodate such states.

The IEEE 1149.7 stuff seems to incorporate a "tap state" model of
sorts ... parts relevant to OpenOCD would be the power states, and
whether it's routed into the scan chain or not.  I'd say those use
cases are relatively "near future".  Not so with "target state".


> > Another is to avoid the nastiness inherent in the current capability
> > of running "target configure -chain-position foo.bar" *after* the
> > target has been set up.  I can't ever see goodness coming from that...
> >
> 
> Yup.  We definitely should partition things into init-time settings  
> and run-time settings.  There are a number of things that really  
> should only be set on target (or tap) creation.  Imagine someone  
> trying to change the width of the TAP IR at runtime.

Part of why that's not allowed.  :)

It's the target stuff that's goofy that way, not TAP stuff.

 
> > That is, it would help keep the layer distinction:  there's a TAP
> > layer, and a target layer.  TAP operations don't work on targets,
> > and vice versa.
> >
> > (Some TAPs are isomorphic to targets too, and in the simplest systems
> > there may be only one tap == a target.)
> >
> >
> 
> 
> Right.  I don't think our command structure today does a very good job  
> of keeping them separate or distinguishing which layer a command is  
> part of.  If we group the TAP commands under the 'jtag' command and  
> target commands under the 'target' command, it is _much_ clearer.

So to get back to the real "it's *BROKEN* and 0.2.0 needs a fix"
issue ... sounds like you would be happier with

$target_name tapname

command to return the TAP, so the startup.tcl loops would look
something like

if { [jtag tapisenabled [$t tapname]] == 0 } {
continue
}

or something similar, instead of assuming the target is enabled.

Yes?

And fixing "$target_name configure ..." so it disallows changing the
target position or endianness after target setup.

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


Re: [Openocd-development] [PATCH] Do not try to examine targets with disabled taps

2009-06-06 Thread David Brownell
Splitting my response here in two parts ... this first one
seems more in the "after 0.2.0 ships" territory.


On Saturday 06 June 2009, Rick Altherr wrote:
> On Jun 6, 2009, at 1:20 PM, David Brownell wrote:

> Sorry, I thought you were recommending naming things such as  
> "omap3530" for both the target and TAP.  As long as we stick to  
> descriptive names such as "omap3530.cpu" for both, I'm OK with it.

But Duane suggests e.g. omap3530.cpu (target) vs omap3530.cpu.tap ..


> The confusion can partially be resolved by changing the layout of the  
> commands.  Right now, the TAP-related commands are spread out (some  
> are under JTAG, some are top-level (irscan, drscan), etc).  The target  
> commands are less so, but still are to an extent (targets are created  
> with 'target create', but every other operation is done on the target  
> name rather than 'target ').  If we straightened out the  
> commands such that it is obvious which namespace we are working in,  
> then the confusion of overlapping namespaces would be reduced  
> considerably.

I could see "jtag irscan" etc (better:  "jtag execute $arrayname").

But remember that "irscan" and "drscan" apply to the entire
scan chain, not individual TAPs; there aren't many commands to
address individual TAPs.  That'd seem post-0.2.0 no matter how
it all shakes out.


> I'd still prefer to keep the -chain-pos portion of the   
> target command since I'd not a fan of automagic operations.  We could  
> make it optional and have the default be the target name, but I'd like  
> to keep the option of being explicit and to allow for non-standard  
> naming.

That's where I started.  Easier to explain that way too: it
works , unless .

Then I noticed that there *is* no such wierdstuff today, and
so I entertained the slightly subversive thought that we should
try to do without the wierdstuff for as long as possible, and
at least until we have real use cases for it.



> As long as the command structure and documentation make this obvious,  
> it's fine.  As it stands today, it's not obvious that the reason  
> 'halt' failed it because it is trying to use the current target name  
> as a command, but that target doesn't exist.  This gets worse if you  
> have more than one target.

However, nothing you've said offers a *fix* for that bug...


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


Re: [Openocd-development] [PATCH] Do not try to examine targets with disabled taps

2009-06-06 Thread Duane Ellis
 >>> [ targetname & tapnames are the same, and is confusing]

Yea, ugh, that is my fault, I did all that last year, I set the 
example.  What I did not consider well was the TAP names when I setup my 
examples after creating the "tcl-target-as-an-object-command.

FYI - The original idea was to support multi-core targets with different 
names, for example:

 mdw  - works on the current target, what ever that is...
$TARGETNAME  mdw

That said, I think there is an easy fix - by means of 'enforcement' of 
naming convention.

step 1: a 1 line change @  'jtag.c' -line 1360
===
FROM:
sprintf( cp, "%s.%s", pTap->chip, pTap->tapname );
TO:
sprintf( cp, "%s.%s.tap", pTap->chip, pTap->tapname );

This *forces* all tapnames to have the suffix ".tap".


Step 2, there are 2 options, if (1) is done, I think (B) should be done.

(A) a purely mechanical change on all "-chain-position" options in the 
"CFG" files, adding the ".tap" suffix.

(B)  Hidden in C code... effectively enforced, like above, see  target.c 
- line 3444..3456
 It's important to do that *THERE* - not inside: "jtag_tap_by_jim_obj()"
 Reason is we want to add ".tap" when we *configure* a target.

 
Step 3. is cleanup, I think only applies to the OMAP3530 (beagle board)
   ie:  "opmap3530.cfg" - the various "irscan" and "drscan" commands 
would need tweaks.
ie: From:   "irscan omap3.jrc ..."  To:  "irscan omap3.jrc.tap "

In total, I think (1) and (2)(B) - make the most amount of sense, ie: 
"taps" are named with ".tap" at the end, there is no other way to do so, 
and makes it very clear to the the user/victim,

ie: If it *ENDS* with *TAP* - it is the *TAP* ...

-Duane.



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


Re: [Openocd-development] [PATCH] Do not try to examine targets with disabled taps

2009-06-06 Thread Rick Altherr

On Jun 6, 2009, at 1:20 PM, David Brownell wrote:


On Saturday 06 June 2009, Rick Altherr wrote:

Having the target and tap names be the same is _not_  preferable.  It
makes the relationship between those two layers very confusing.


Hmm, having them be the same is the convention that's encouraged
already, as well as being the one used in every target/*.cfg
script I've looked at.  And I just grepped ... all the current
"target create" commands use $_TARGETNAME for both the target and
TAP names.

Notice how the "targets" command shows two names, normally the same.
If it's confusing (and I wouldn't agree that it is), it's not a new
type of confusion.  And I would argue that having more than one
namespace is itself very confusing... particularly since the
current policy more or less hides one of the namespaces.



Sorry, I thought you were recommending naming things such as  
"omap3530" for both the target and TAP.  As long as we stick to  
descriptive names such as "omap3530.cpu" for both, I'm OK with it.   
The confusion can partially be resolved by changing the layout of the  
commands.  Right now, the TAP-related commands are spread out (some  
are under JTAG, some are top-level (irscan, drscan), etc).  The target  
commands are less so, but still are to an extent (targets are created  
with 'target create', but every other operation is done on the target  
name rather than 'target ').  If we straightened out the  
commands such that it is obvious which namespace we are working in,  
then the confusion of overlapping namespaces would be reduced  
considerably.  I'd still prefer to keep the -chain-pos portion of the  
target command since I'd not a fan of automagic operations.  We could  
make it optional and have the default be the target name, but I'd like  
to keep the option of being explicit and to allow for non-standard  
naming.





If you make
the names the same, people get confused when the $(target name)
command isn't available but the TAP has been created.


People are always free to be foolish.  If "targets" doesn't list
the name, there is no target.  TAP != target except on very simple
systems.



As long as the command structure and documentation make this obvious,  
it's fine.  As it stands today, it's not obvious that the reason  
'halt' failed it because it is trying to use the current target name  
as a command, but that target doesn't exist.  This gets worse if you  
have more than one target.



Although ... this reminds me how the "scan_chain" command is
buried so deeply in the user's guide.  I'll send a patch to
present it when scan chains are presented ... which will help
better distinguish the "list of TAPs" and "list of targets"
concepts, by properly presenting both of them.  (Though it'd
be nice to have tap-list primitives like we have target-list
primitives...)



There is a definite learning curve when dealing with OpenOCD.  We  
don't do a good job of hiding the details of JTAG from the user.   
Fixing that is a long-term project, but we can at least make it better  
in the short term by describing the JTAG layer early on and  
partitioning commands into logical layers.





Which just
points out another conceptual problem ... either (a) "target create"
should force target and TAP names to be the same (so "-chain- 
position"
becomes redundant), *or* (b) we need a "list enabled taps"  
primitive.


Minor nit:  I should have written "mechanism" not primitive.

And part of why I think that (a) is preferable is that it would let
us avoid creating parallel "$target_name tapisenabled" predicates
to match the already-working "jtag tapisenabled $tap_name" one.



Such a redundancy wouldn't be great, but relying on target_name ==  
tap_name isn't good for long-term extensibility.  Chip designers like  
to lots of strange things.  Imagine a single TAP that has the debug  
registers for two separate target types.  You can't have 2 targets  
with the same name even though they use the same TAP.  Of course, the  
'target $target_name isenabled' isn't always a direct translation to  
checking if the TAP is enabled.  There could easily be some other set  
of items within the target's registers that determine if it is  
enabled.  In the simple case, it can be implemented as just looking up  
the tap name and asking the JTAG layer if the tap is enabled.  A more  
complex case would likely always check if the TAP is enabled and then  
perform other checks.  I can see value in a general "target is  
enabled" command.



Another is to avoid the nastiness inherent in the current capability
of running "target configure -chain-position foo.bar" *after* the
target has been set up.  I can't ever see goodness coming from that...



Yup.  We definitely should partition things into init-time settings  
and run-time settings.  There are a number of things that really  
should only be set on target (or tap) cr

Re: [Openocd-development] [PATCH] Do not try to examine targets with disabled taps

2009-06-06 Thread David Brownell
On Saturday 06 June 2009, Rick Altherr wrote:
> Having the target and tap names be the same is _not_  preferable.  It  
> makes the relationship between those two layers very confusing.

Hmm, having them be the same is the convention that's encouraged
already, as well as being the one used in every target/*.cfg
script I've looked at.  And I just grepped ... all the current
"target create" commands use $_TARGETNAME for both the target and
TAP names.

Notice how the "targets" command shows two names, normally the same.
If it's confusing (and I wouldn't agree that it is), it's not a new
type of confusion.  And I would argue that having more than one
namespace is itself very confusing... particularly since the
current policy more or less hides one of the namespaces.


>   If you make  
> the names the same, people get confused when the $(target name)  
> command isn't available but the TAP has been created.

People are always free to be foolish.  If "targets" doesn't list
the name, there is no target.  TAP != target except on very simple
systems.

Although ... this reminds me how the "scan_chain" command is
buried so deeply in the user's guide.  I'll send a patch to
present it when scan chains are presented ... which will help
better distinguish the "list of TAPs" and "list of targets"
concepts, by properly presenting both of them.  (Though it'd
be nice to have tap-list primitives like we have target-list
primitives...)


> > Which just
> > points out another conceptual problem ... either (a) "target create"
> > should force target and TAP names to be the same (so "-chain-position"
> > becomes redundant), *or* (b) we need a "list enabled taps" primitive.

Minor nit:  I should have written "mechanism" not primitive.

And part of why I think that (a) is preferable is that it would let
us avoid creating parallel "$target_name tapisenabled" predicates
to match the already-working "jtag tapisenabled $tap_name" one.

Another is to avoid the nastiness inherent in the current capability
of running "target configure -chain-position foo.bar" *after* the
target has been set up.  I can't ever see goodness coming from that...

That is, it would help keep the layer distinction:  there's a TAP
layer, and a target layer.  TAP operations don't work on targets,
and vice versa.

(Some TAPs are isomorphic to targets too, and in the simplest systems
there may be only one tap == a target.)

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


Re: [Openocd-development] [PATCH] Do not try to examine targets with disabled taps

2009-06-06 Thread Rick Altherr
Having the target and tap names be the same is _not_  preferable.  It  
makes the relationship between those two layers very confusing.  For  
example, when a target is created, it introduces a new command names  
for the target.  The same does _not_ happen for a TAP.  If you make  
the names the same, people get confused when the $(target name)  
command isn't available but the TAP has been created.

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



On Jun 6, 2009, at 12:15 PM, David Brownell wrote:


On Thursday 04 June 2009, David Brownell wrote:



... although this touches on some other glitches in the
vicinity of tap enable/disable logic.  The "tapenable"
code paths don't seem to have an obvious way to fail and
report that the tap was not enabled.


Still true, but not directly related to the notion that
targets on disabled taps should "just work".



Glitch in the patch I sent, nyet sorted out:  "reset" makes
the server abort:

lt-openocd: jtag_driver.c:243: interface_jtag_add_dr_scan:  
Assertion `field == out_fields + scan->num_fields' failed.

Aborted

I seem to have missed something.


There are two problems I've turned up so far, through
the startup.tcl code:

- The $target_name arp_* methods should not try to talk
  to targets that are disabled.

- Conceptual problem:  all the startup.tcl loops go
foreach t [target names] {
... doit ...
}
   but the loop should be "for each *ENABLED* target.

Re that conceptual problem, my first thought was just
growing the loops:

if { 0 == [jtag tapisenabled $t] } {
continue
}

But for "target create MyTarget cortex_m3 -chain-position mychip.foo"
that loses, because there is no JTAG tap named MyTarget.  Which just
points out another conceptual problem ... either (a) "target create"
should force target and TAP names to be the same (so "-chain-position"
becomes redundant), *or* (b) we need a "list enabled taps" primitive.

I tend to think that (a) is preferable.

Comments?


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




smime.p7s
Description: S/MIME cryptographic signature
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development


Re: [Openocd-development] [PATCH] Do not try to examine targets with disabled taps

2009-06-06 Thread David Brownell
On Thursday 04 June 2009, David Brownell wrote:
> 
> > ... although this touches on some other glitches in the
> > vicinity of tap enable/disable logic.  The "tapenable"
> > code paths don't seem to have an obvious way to fail and
> > report that the tap was not enabled.

Still true, but not directly related to the notion that
targets on disabled taps should "just work".


> Glitch in the patch I sent, nyet sorted out:  "reset" makes
> the server abort:
> 
> lt-openocd: jtag_driver.c:243: interface_jtag_add_dr_scan: Assertion `field 
> == out_fields + scan->num_fields' failed.
> Aborted
> 
> I seem to have missed something.

There are two problems I've turned up so far, through
the startup.tcl code:

 - The $target_name arp_* methods should not try to talk
   to targets that are disabled.

 - Conceptual problem:  all the startup.tcl loops go
foreach t [target names] {
... doit ...
}
but the loop should be "for each *ENABLED* target.

Re that conceptual problem, my first thought was just
growing the loops:

if { 0 == [jtag tapisenabled $t] } {
continue
}

But for "target create MyTarget cortex_m3 -chain-position mychip.foo"
that loses, because there is no JTAG tap named MyTarget.  Which just
points out another conceptual problem ... either (a) "target create"
should force target and TAP names to be the same (so "-chain-position"
becomes redundant), *or* (b) we need a "list enabled taps" primitive.

I tend to think that (a) is preferable.

Comments?


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


Re: [Openocd-development] [PATCH] Do not try to examine targets with disabled taps

2009-06-04 Thread David Brownell

> > How about fixing this in the calling code?
> 
> ... although this touches on some other glitches in the
> vicinity of tap enable/disable logic.  The "tapenable"
> code paths don't seem to have an obvious way to fail and
> report that the tap was not enabled.

Glitch in the patch I sent, nyet sorted out:  "reset" makes
the server abort:

lt-openocd: jtag_driver.c:243: interface_jtag_add_dr_scan: Assertion `field == 
out_fields + scan->num_fields' failed.
Aborted

I seem to have missed something.
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development


Re: [Openocd-development] [PATCH] Do not try to examine targets with disabled taps

2009-06-04 Thread David Brownell
On Thursday 04 June 2009, Øyvind Harboe wrote:
> > It won't work,  but is is not an error. Return ERROR_OK so other targets in
> > the chain can be examined.
> 
> How about fixing this in the calling code?

... although this touches on some other glitches in the
vicinity of tap enable/disable logic.  The "tapenable"
code paths don't seem to have an obvious way to fail and
report that the tap was not enabled.

--- a/src/target/target.c
+++ b/src/target/target.c
@@ -481,12 +481,14 @@ int target_examine_one(struct target_s *target)
 int target_examine(void)
 {
 	int retval = ERROR_OK;
-	target_t *target = all_targets;
-	while (target)
+	target_t *target;
+
+	for (target = all_targets; target; target = target->next)
 	{
+		if (!target->tap->enabled)
+			continue;
 		if ((retval = target_examine_one(target)) != ERROR_OK)
 			return retval;
-		target = target->next;
 	}
 	return retval;
 }
@@ -1478,6 +1480,14 @@ DumpTargets:
 	command_print(cmd_ctx, "--  -- -- -- --- - --");
 	while (target)
 	{
+		const char *state;
+
+		if (target->tap->enabled)
+			state = Jim_Nvp_value2name_simple(nvp_target_state,
+	target->state)->name;
+		else
+			state = "tap-disabled";
+
 		/* XX: abcdefghij abcdefghij abcdefghij abcdefghij */
 		command_print(cmd_ctx, "%2d: %-10s %-10s %-10s %10d %14s %s",
 	  target->target_number,
@@ -1486,7 +1496,7 @@ DumpTargets:
 	  Jim_Nvp_value2name_simple( nvp_target_endian, target->endianness )->name,
 	  target->tap->abs_chain_position,
 	  target->tap->dotted_name,
-	  Jim_Nvp_value2name_simple( nvp_target_state, target->state )->name );
+	  state);
 		target = target->next;
 	}
 
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development


Re: [Openocd-development] [PATCH] Do not try to examine targets with disabled taps

2009-06-04 Thread Øyvind Harboe
On Thu, Jun 4, 2009 at 5:54 PM, Magnus Lundin  wrote:
> It won't work,  but is is not an error. Return ERROR_OK so other targets in
> the chain can be examined.

How about fixing this in the calling code?

It *did* fail to examine the  target, so isn't returning ERROR_OK best?

What if some other code wants to handle the failing case for disabled taps
(or other interim errors) differently?


-- 
Ø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] Do not try to examine targets with disabled taps

2009-06-04 Thread Magnus Lundin
It won't work,  but is is not an error. Return ERROR_OK so other targets 
in the chain can be examined.


Reagrds
Magnus

Index: src/target/target.c
===
--- src/target/target.c	(revision 2051)
+++ src/target/target.c	(working copy)
@@ -470,6 +470,11 @@
 
 int target_examine_one(struct target_s *target)
 {
+	if (! target->tap->enabled)
+	{
+		LOG_WARNING("Target %s with disabled tap cannot be examined, use arp_examine after init",target_get_name(target));
+		return ERROR_OK;
+	}
 	return target->type->examine(target);
 }
 

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