Re: [Openocd-development] [RFC, PATCH 2/2] ft2232: simplify ft2232_read_scan
On 17/06/2010 09:16, Marc Pignat wrote: --- src/jtag/drivers/ft2232.c | 19 ++- 1 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/jtag/drivers/ft2232.c b/src/jtag/drivers/ft2232.c index bc8463e..9d40b1c 100644 --- a/src/jtag/drivers/ft2232.c +++ b/src/jtag/drivers/ft2232.c @@ -714,23 +714,24 @@ static void ft2232_end_state(tap_state_t state) static void ft2232_read_scan(enum scan_type type, uint8_t* buffer, int scan_size) { - int num_bytes = (scan_size + 7) / 8; - int bits_left = scan_size; - int cur_byte = 0; + int num_bytes = scan_size / 8; + int bits_left = scan_size % 8; + int cur_byte; - while (num_bytes-- 1) + for (cur_byte = 0; cur_byte num_bytes; cur_byte++) { - buffer[cur_byte++] = buffer_read(); - bits_left -= 8; + buffer[cur_byte] = buffer_read(); } - buffer[cur_byte] = 0x0; - - /* There is one more partial byte left from the clock data in/out instructions */ + /* Manage partial byte left from the clock data in/out instructions, if any */ if (bits_left 1) { buffer[cur_byte] = buffer_read() 1; } + else + { + buffer[cur_byte] = 0x0; + } /* This shift depends on the length of the clock data to tms instruction, insterted at end of the scan, now fixed to a two step transition in ft2232_add_scan */ buffer[cur_byte] = (buffer[cur_byte] | (((buffer_read()) 1) 0x80)) (8 - bits_left); } -- 1.7.1 This breaks all my ftdi adapters under win32. Looking closer it actually introduces a overflow segfault when tested under valgrind. After leaving the for loop cur_byte is always out of bounds. Ran out of time attempting to fix, so I propose this patch is reverted any objections. Cheers Spen ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [RFC, PATCH 2/2] ft2232: simplify ft2232_read_scan
I'm going to leave it to you to follow up and make the decision on whether to revert here. I was just committing stuff based upon a consensus in the list that this was the right direction to go, I don't know this code very well... -- Øyvind Harboe US toll free 1-866-980-3434 / International +47 51 63 25 00 http://www.zylin.com/zy1000.html ARM7 ARM9 ARM11 XScale Cortex JTAG debugger and flash programmer ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [RFC, PATCH 2/2] ft2232: simplify ft2232_read_scan
On Friday 02 July 2010 12:15:06 Øyvind Harboe wrote: I'm going to leave it to you to follow up and make the decision on whether to revert here. Hi all! It seems that 2/2 has introduced problems. It seemed trivial to me, but finally it isn't! I think we should revert it. Best regards Marc ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [RFC, PATCH 2/2] ft2232: simplify ft2232_read_scan
Would you mind posting a link to both patches? I'm not really following this interface development, just doing the mechanics of merging patches as approperiate. -- Øyvind Harboe US toll free 1-866-980-3434 / International +47 51 63 25 00 http://www.zylin.com/zy1000.html ARM7 ARM9 ARM11 XScale Cortex JTAG debugger and flash programmer ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [RFC, PATCH 2/2] ft2232: simplify ft2232_read_scan
merged both patches. Thanks! -- Øyvind Harboe US toll free 1-866-980-3434 / International +47 51 63 25 00 http://www.zylin.com/zy1000.html ARM7 ARM9 ARM11 XScale Cortex JTAG debugger and flash programmer ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [RFC, PATCH 2/2] ft2232: simplify ft2232_read_scan
/ These read OK, I thought, but I'm not in a // position right now to review (in detail) or // verify either patch. // // Could someone with an FTDI-based adapter // please try these out and verify them? Then // maybe have a committer commit them, if they // ideed check out? / Works for me on Amontek JTAGKey-Tiny against TI DM355. Works for me with Amontec JTAGkey-2P to custom SAM7 board on Microsoft Windows WIN 7 64 bits. Regards, Laurent Gauch http://www.amontec.com ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [RFC, PATCH 2/2] ft2232: simplify ft2232_read_scan
I'll let this cool off for a few days and then I'll commit it. It has gotten positive testing feedback so far, so anyone with objections should speak now. -- Øyvind Harboe US toll free 1-866-980-3434 / International +47 51 63 25 00 http://www.zylin.com/zy1000.html ARM7 ARM9 ARM11 XScale Cortex JTAG debugger and flash programmer ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [RFC, PATCH 2/2] ft2232: simplify ft2232_read_scan
I'll let this cool off for a few days and then I'll commit it. Great; I wish I could do that but my current patches-from-email situation is malfunctioning. It has gotten positive testing feedback so far, I'd like clarification on that: there were two patches. This #2/2 was more invasive, and is presumably what was tested. But both cleanups should likely get merged. so anyone with objections should speak now. ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [RFC, PATCH 2/2] ft2232: simplify ft2232_read_scan
David Brownell wrote: It has gotten positive testing feedback so far, I'd like clarification on that: there were two patches. This #2/2 was more invasive, and is presumably what was tested. But both cleanups should likely get merged. For my part, I applied both patches and tested. Didn't test only after one patch, though. -- Jon Povey jon.po...@racelogic.co.uk Racelogic is a limited company registered in England. Registered number 2743719 . Registered Office Unit 10, Swan Business Centre, Osier Way, Buckingham, Bucks, MK18 1TB . The information contained in this electronic mail transmission is intended by Racelogic Ltd for the use of the named individual or entity to which it is directed and may contain information that is confidential or privileged. If you have received this electronic mail transmission in error, please delete it from your system without copying or forwarding it, and notify the sender of the error by reply email so that the sender's address records can be corrected. The views expressed by the sender of this communication do not necessarily represent those of Racelogic Ltd. Please note that Racelogic reserves the right to monitor e-mail communications passing through its network ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [RFC, PATCH 2/2] ft2232: simplify ft2232_read_scan
These read OK, I thought, but I'm not in a position right now to review (in detail) or verify either patch. Could someone with an FTDI-based adapter please try these out and verify them? Then maybe have a committer commit them, if they ideed check out? The first one looked pretty obvious (cleaning the write logic, less duplication). ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [RFC, PATCH 2/2] ft2232: simplify ft2232_read_scan
David Brownell wrote: These read OK, I thought, but I'm not in a position right now to review (in detail) or verify either patch. Could someone with an FTDI-based adapter please try these out and verify them? Then maybe have a committer commit them, if they ideed check out? Works for me on Amontek JTAGKey-Tiny against TI DM355. Tested-by: Jon Povey jon.po...@racelogic.co.uk -- Jon Povey jon.po...@racelogic.co.uk Racelogic is a limited company registered in England. Registered number 2743719 . Registered Office Unit 10, Swan Business Centre, Osier Way, Buckingham, Bucks, MK18 1TB . The information contained in this electronic mail transmission is intended by Racelogic Ltd for the use of the named individual or entity to which it is directed and may contain information that is confidential or privileged. If you have received this electronic mail transmission in error, please delete it from your system without copying or forwarding it, and notify the sender of the error by reply email so that the sender's address records can be corrected. The views expressed by the sender of this communication do not necessarily represent those of Racelogic Ltd. Please note that Racelogic reserves the right to monitor e-mail communications passing through its network ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development