On Thursday, September 08, 2011 23:26:04 Jie Zhang wrote:
> On Thu, Sep 8, 2011 at 9:04 PM, Mike Frysinger <[email protected]> wrote:
> > On Wednesday, September 07, 2011 09:16:35 Jie Zhang wrote:
> >> On Wed, Sep 7, 2011 at 12:45 AM, Mike Frysinger wrote:
> >> > On Friday, September 02, 2011 18:38:35 Jie Zhang wrote:
> >> >> +    fflush (stdout);
> >> > 
> >> > i dont think we should be touching stdout
> >> 
> >> urj_tap_discovery does fflush. I just follow the existing code. I
> >> think the fflush is used to ensure the log information is displayed
> >> before detecting.
> > 
> > we should fix that :).  the library should not be fflushing stdout
> > anywhere
> 
> Why the library should not be fflushing stdout anywhere?

who is to say where stdout is going ?  or that our log output is going to 
stdout at all ?  apps can redirect stdout wherever they want.

> > "Usage: %s [current|INSTRUCTION]\n"
> > "Discovery of unknown parts in the JTAG chain.\n"
> > "\n"
> > "When no argument is specified, '%s' attempts to detect these\n"
> > "parameters of an unknown JTAG chain:\n"
> > " 1. IR (instruction register) length\n"
> > " 2. DR (data register) length for all possible instructions\n"
> > "\n"
> > "When an argument is specified, '%s' attempts to detect the data\n"
> > "length of the specified instruction.\n"
> > "\n"
> > "Warning: This may be dangerous for some parts (especially if the\n"
> > "part doesn't have TRST signal).\n"
> 
> When an argument is specified, the command is not used to discover the
> unknown parts, it's only for detecting the DR length of the specified
> INSTRUCTION. So the first sentence should not be a summary description
> of the command.

so just generalize it.  something like "Discovery of unknown aspects of the 
JTAG chain.".

> >> >> +               "When there is an argument, discovery of the DR size
> >> >> of the current or\n"
> >> >> +               "specified instruction.\n"), "discovery",
> >> > 
> >> > use: When an argument is specified, the DR size of the specified
> >> > instruction is discovered.
> >> 
> >> As above, I'm willing to change it to "When an argument is specified".
> >> But I think the last half of the sentence is good, especially it
> >> contains the command name "discovery". Your sentence is more like an
> >> English sentence but the original sentence is also OK with English
> >> syntax.
> > 
> > the original has slightly broken grammar, or is at least confusing when
> > it uses the straight command name "discovery" in place of proper
> > structure.
> 
> When I first read the help of this command several years ago,
> "discovery of" was also strange to me. But now I'm used to it. Now I
> also think it has some goodness. So I don't see a strong reason to
> change it.

the existing grammar is correct.  your slight tweaks breaks it :).

> >> > what if someone names an instruction "current" ?  seems like this
> >> > would break that.  also, you should add a signal completion handler
> >> > ;). there's a helper to do this for you already ... see some of the
> >> > existing commands that take signals.
> >> 
> >> The INSTRUCTION could only be in two forms: 0xnnnn or 01 strings. It
> >> could not be "current".
> > 
> > reading the help made me think i could do:
> >        discovery BYPASS
> > so the help text should explicitly mention the format it expects.
> 
> I also considered this. But I could not find a concise way to describe
> it. So I just hope we can make it accept INSTRUCTION name in future.

if we move to that, then we'd have to make "current" work in a diff way

> >> > also, it seems like "discovery FOO" will implicitly change the active
> >> > insn to "FOO".  should we be saving/restoring the active insn ?  or am
> >> > i just reading your patch wrong ?
> >> 
> >> Yes, my patch will clobber the existing instruction. But the existing
> >> command discovery clobbers even much more. It resets the TAP.
> > 
> > yes, i missed that part of the recent refactoring of the discovery
> > command. we should document these side-effects in tap.h above the func
> > prototype.
> 
> Not in the scope of my patch.

it is because you're adding more side effects ;).  just have to add one line 
to the existing comment :P.
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.

------------------------------------------------------------------------------
Why Cloud-Based Security and Archiving Make Sense
Osterman Research conducted this study that outlines how and why cloud
computing security and archiving is rapidly being adopted across the IT 
space for its ease of implementation, lower cost, and increased 
reliability. Learn more. http://www.accelacomm.com/jaw/sfnl/114/51425301/
_______________________________________________
UrJTAG-development mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/urjtag-development

Reply via email to