Re: [Openocd-development] [PATCH] Do not try to examine targets with disabled taps
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
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
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
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
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
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
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
[ 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
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
>>> [ 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
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
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
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
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
> > 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
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
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
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