On Sunday 20 December 2009, Catalin Patulea wrote:
> +
> +/* project specific includes */
> +#include <jtag/interface.h>
> +#include <jtag/commands.h>
> +#include <helper/time_support.h>
> +#include "bitbang.h"
> +
> +/* system includes */
> +#include <string.h>
> +#include <stdlib.h>
> +#include <unistd.h>

Minor nit-picking:  put the "local.h" files after every
one of the <nonlocal.h> includes.  There's supposed to
be layering, where the nonlocal stuff isn't allowed to
have dependencies on local stuff.

That's minor since a lot of headers already get that
wrong (subject to cleanup patches) ... but please fix
this in the next iteration of this patch, so we won't
need to fix it separately.


You should add a NEWS entry for this new driver.
Also, update openocd.texi to list this driver and
the commands it defines.  I'd prefer one patch to
address everything, but splitting docs into their
own patch is OK too.


> +int usb_blaster_execute_queue(void);
> +
> +void usb_blaster_blink(void);
> +int usb_blaster_speed(int speed);
> +int usb_blaster_init(void);
> +int usb_blaster_quit(void);

You don't need most of those forward declarations;
does usb_blaster_blink() even exist?

Also, those routines should be declared "static"
since they're only exported as driver methods.  I
think "nm -g --defined-only usb_blaster.o" should
probably only show one symbol...


> +/* The following code doesn't fully utilize the possibilities of the 
> USB-Blaster. It
> + * writes one byte per JTAG pin state change at a time; it doesn't even try 
> to buffer
> + * data up to the maximum packet size of 64 bytes.

Please reformat to fit things into 80 character lines.


There seem to be tcl/interface/*... files missing
for the USB-JTAG and Altera adapters.  Again, those
could be in separate patches.

- Dave
_______________________________________________
Openocd-development mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/openocd-development

Reply via email to