On Thu, Sep 8, 2011 at 11:49 PM, Mike Frysinger <[email protected]> wrote: > 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. > I'm not sure if my guess is correct, but my guess is if stdout is redirect to some other file, fflush (stdout) should flush that other file too.
>> > "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.". > My patch is simpler, just keep the original help text but add a condition before it. Then append the new case at the end. >> >> >> + "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 :). > Below is another format I tried, which does not have "When ....", but the two types of command syntax are separated. "Usage: %s\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" "Warning: This may be dangerous for some parts (especially if the\n" "part doesn't have TRST signal).\n" "\n" " %s current|INSTRUCTION\n" "Discovery of the DR size of the current or specified instruction.\n" "length of the specified instruction.\n" "\n" >> >> > 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 > We will not need "current" then. We can just spell all instructions on the command line. >> >> > 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. More side effects? I don't think so. The side effect of the original discovery is clobber everything. With my patch it can't clobber more. Regards, Jie ------------------------------------------------------------------------------ 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
