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

> +    fflush (stdout);

i dont think we should be touching stdout

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

> +               "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.

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.  

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

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

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

Reply via email to