On Wed, Sep 7, 2011 at 12:45 AM, Mike Frysinger <[email protected]> wrote:
> On Friday, September 02, 2011 18:38:35 Jie Zhang wrote:
>> --- src/tap/discovery.c (revision 1994)
>> +++ src/tap/discovery.c (working copy)
>>
>> + for (i = chain->parts->len ; i > 0; i--)
>
> no space before that semi-colon
>
Will fix when commit.
>> + 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.
>> --- src/cmd/cmd_discovery.c (revision 1994)
>> +++ src/cmd/cmd_discovery.c (working copy)
>>
>> urj_log (URJ_LOG_LEVEL_NORMAL,
>> - _("Usage: %s\n"
>> - "Discovery of unknown parts in the JTAG chain.\n"
>> + _("Usage: %s [current|INSTRUCTION]\n"
>> + "When no arguments, discovery of unknown parts in the JTAG
> chain.\n"
>
> this should be with the same text that describes the [current|INSNTRUCTION]
> argument (i.e. at the bottom). also, it should say:
> When no arguments are specified, unknown parts in the JTAG chain are
> discovered
>
I'm willing to use "When no arguments are specified". But I don't
think this should be with the text at the bottom without changing the
rest of the help text. You can try and read it in a whole, you will
find the case of no arguments and the case of having arguments are
then mixed and less clear than my patch.
>> + "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.
> 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".
> 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.
Regards,
Jie
------------------------------------------------------------------------------
Using storage to extend the benefits of virtualization and iSCSI
Virtualization increases hardware utilization and delivers a new level of
agility. Learn what those decisions are and how to modernize your storage
and backup environments for virtualization.
http://www.accelacomm.com/jaw/sfnl/114/51434361/
_______________________________________________
UrJTAG-development mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/urjtag-development
