Ali Lown wrote:
> Aah. After failing once by committing too early, I then did it again!!
Look up the interactive rebase feature of git. I use it a lot to
rework commits which are intentionally or unintentionally incomplete.
> The one attached below (usb_blaster.patch) implements those 2 rolled
> into 1,
I would split them up a little, because there are at least two
distinct changes; 1. buffering and 2. LED action. It's a good idea to
document these changes separately by having them be individual
commits, each with a good commit message.
> and then corrects a compile error too.
Third patch it sounds like. Again with a clear commit message
describing the problem and the solution.
> +++ b/src/jtag/drivers/usb_blaster.c
..
> @@ -106,6 +110,9 @@ static uint16_t usb_blaster_pid = 0x6001; /* USB-Blaster
> */
>
> /* last output byte in simple bit banging mode */
> static uint8_t out_value;
> +#define BUF_LEN 64
> +static uint8_t out_buffer[BUF_LEN];
> +static uint16_t out_count = 0;
>
> #if BUILD_USB_BLASTER_FTD2XX == 1
> static FT_HANDLE ftdih;
> @@ -241,10 +248,22 @@ usb_blaster_buf_read(uint8_t *buf, unsigned size,
> uint32_t *bytes_read)
>
> #define READ_TDO (1 << 0)
>
> -static void usb_blaster_write_data(void)
> +static void usb_blaster_write_databuffer(uint8_t* buf, uint16_t len)
> {
> uint32_t bytes_written;
> - usb_blaster_buf_write(&out_value, 1, &bytes_written);
> + usb_blaster_buf_write(buf, len, &bytes_written);
> + out_count = 0;
> +#ifdef _DEBUG_JTAG_IO_
> + LOG_DEBUG("---- WROTE %d",bytes_written);
> +#endif
> +}
Buffering can be a great performance improvement. But do you know if
the hardware and software (openocd) can handle buffering? Ie. there
may be some operations that must be committed immediately, besides
read operations?
Your implementation seems extremely dangerous to me. You add a global
buffer, which will get corrupted if more than one device is being
used. Is there a "contract" from openocd core that there will never
be more than one instance of a hardware driver? If so this method
would be safe, but is still pretty ugly. Is there an instance struct
that could be used instead?
> +static void usb_blaster_addtowritebuffer(uint8_t value, bool forcewrite)
> +{
> + out_buffer[out_count] = value;
> + out_count += 1;
> + if(out_count == BUF_LEN || forcewrite)
> + usb_blaster_write_databuffer(out_buffer, out_count);
> }
There's an extra tab above. Please pay close attention to coding style.
//Peter
_______________________________________________
Openocd-development mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/openocd-development