[Openocd-development] Outstanding patches = Fix: Correctly exit function: ft2232_init when an error occurred
Hello,
The end goal it's to have a safe state of the used JTAG adapter at the
end of the executable file.
With this patch = Fix: Correctly exit function: ft2232_init when an
error occurred
I wish correct the status at the end of ft2232_init function when an
error occured.
At this moment, in function: ft2232_init the JTAG handle is acquire
with the funtion: ft2232_init_ftd2xx or ft2232_init_libftdi and 131072
bytes are allocate for the variable: ft2232_buffer.
But when an error occured, the JTAG handle isn't free and all allocate
bytes aren't free.
To correct this on the layer ft2232, it's better to clarify the
function: ft2232_init.
And for the end goal, I follow that as speack Laurent:
Open
Init
Deinit
Close
Open it's to acquire the JTAG handle.
Init it's to set all parameters and the pinning of the JTAG adapter.
Deinit it's to be on a safe state with the JTAG adapter.
Close it's to free the JTAG handle.
With this first patch, I have only divided functions and add the part
to close handle and free the bytes of the variable: ft2232_buffer when
an error occured.
So I have take the first part of the function: ft2232_init_ftd2xx and
put to the function: ft2232_open_ftd2xx (from line: 2118 to line: 2230)
And I have remove the unnecessary variables.
The second part of the function: ft2232_init_ftd2xx stay on this
function (from line: 2231 to line: 2278)
I have add the necessary variables.
It's the same work with the function: ft2232_init_libftdi.
So I have take the first part of the function: ft2232_init_libftdi and
put to the function: ft2232_open_libftdi (from line: 2296 to line: 2332)
And I have remove the unnecessary variables.
The second part of the function: ft2232_init_libftdi stay on this
function (from line: 2333 to line: 2366)
I have add the necessary variables.
To not make a lot of change on the function: ft2232_init, I have only
divide the function on two parts and add the part to close handle and
free the bytes of the variable: ft2232_buffer when an error occured.
So the first part of the function: ft2232_init stay on this function
(from line: 2421 to line: 2466)
I have remove the unnecessary variables.
I have change the functions: ft2232_init_ftd2xx and ft2232_init_libftdi
by the functions: ft2232_open_ftd2xx and ft2232_open_libftdi (because
it's the first part of the functions: ft2232_init_ftd2xx and
ft2232_init_libftdi)
I have take the second part of the function: ft2232_init and put to the
function: ft2232_init_sub (from line: 2467 to line: 2508)
I have add the necessary variables.
I have add the call of functions: ft2232_init_ftd2xx and
ft2232_init_libftdi (because it's the second part of these functions)
And to identify the case of an error occured before the JTAG handle be
acquire, I have add:
if (retval != ERROR_OK)
return retval;
(Because if an error occured during the acquire of the JTAG handle, I
think that the JTAG handle stay free.)
To correctly free the JTAG handle and free the bytes of the variable:
ft2232_buffer when an error occured. I have add:
retval = ft2232_init_sub();
if (retval != ERROR_OK)
{
#if BUILD_FT2232_FTD2XX == 1
FT_Close(ftdih);
#elif BUILD_FT2232_LIBFTDI == 1
ftdi_usb_close(ftdic);
ftdi_deinit(ftdic);
#endif
if(ft2232_buffer)
free(ft2232_buffer);
ft2232_buffer = NULL;
return retval;
}
(That is the content of the function: ft2232_quit (from line: 3231 to
line: 3242))
The little modification of the free for the variable: ft2232_buffer,
it's to only free the memory when is necessary (and possible).
--
Regards,
Sébastien Farquet
http://www.amontec.com/
Amontec JTAGkey-2 : High speed USB JTAG interface
openocd-ft2232-fix-ft2232_init-patch-0001.patch
Description: Binary data
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] Outstanding patches = Fix: Correctly exit function: ft2232_init when an error occurred
Hello Sebastien, Please stay to your convention and use open to open functionality and init to init functionality - this is very good idea and used in transport layer (select, then init), but there is select instead of open, so maybe we should change all names to open..? Using init_sub brings confusion which init is it, or maybe open? Please correct me if im wrong :-) Best regards :-) Tomek -- CeDeROM, SQ7MHZ, http://www.tomek.cedro.info ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
[Openocd-development] Added support script for stm32f2xx
Hi, I have made a try to add stm32f2xx and st3220e_eval support in the
scripts, but being my first attempt I expect that there might be
something that could do in a better way. Could anyone have a look and
consider if it can be added?
//Magnus
diff --git a/tcl/board/stm3220e_eval.cfg b/tcl/board/stm3220e_eval.cfg
new file mode 100755
index 000..0298f10
--- /dev/null
+++ b/tcl/board/stm3220e_eval.cfg
@@ -0,0 +1,5 @@
+
+set FLASH_DRV stm32f2xxx
+
+source [find target/stm32.cfg]
+
diff --git a/tcl/target/stm32.cfg b/tcl/target/stm32.cfg
index 9879c04..d9a0c89 100644
--- a/tcl/target/stm32.cfg
+++ b/tcl/target/stm32.cfg
@@ -1,75 +1,91 @@
-# script for stm32
-
-if { [info exists CHIPNAME] } {
- set _CHIPNAME $CHIPNAME
-} else {
- set _CHIPNAME stm32
-}
-
-if { [info exists ENDIAN] } {
- set _ENDIAN $ENDIAN
-} else {
- set _ENDIAN little
-}
-
-# Work-area is a space in RAM used for flash programming
-# By default use 16kB
-if { [info exists WORKAREASIZE] } {
- set _WORKAREASIZE $WORKAREASIZE
-} else {
- set _WORKAREASIZE 0x4000
-}
-
-# JTAG speed should be = F_CPU/6. F_CPU after reset is 8MHz, so use F_JTAG =
1MHz
-adapter_khz 1000
-
-adapter_nsrst_delay 100
-jtag_ntrst_delay 100
-
-#jtag scan chain
-if { [info exists CPUTAPID ] } {
- set _CPUTAPID $CPUTAPID
-} else {
- # See STM Document RM0008
- # Section 26.6.3
- set _CPUTAPID 0x3ba00477
-}
-jtag newtap $_CHIPNAME cpu -irlen 4 -ircapture 0x1 -irmask 0xf -expected-id
$_CPUTAPID
-
-if { [info exists BSTAPID ] } {
- # FIXME this never gets used to override defaults...
- set _BSTAPID $BSTAPID
-} else {
- # See STM Document RM0008
- # Section 29.6.2
- # Low density devices, Rev A
- set _BSTAPID1 0x06412041
- # Medium density devices, Rev A
- set _BSTAPID2 0x06410041
- # Medium density devices, Rev B and Rev Z
- set _BSTAPID3 0x16410041
- set _BSTAPID4 0x06420041
- # High density devices, Rev A
- set _BSTAPID5 0x06414041
- # Connectivity line devices, Rev A and Rev Z
- set _BSTAPID6 0x06418041
- # XL line devices, Rev A
- set _BSTAPID7 0x06430041
-}
-jtag newtap $_CHIPNAME bs -irlen 5 -expected-id $_BSTAPID1 \
- -expected-id $_BSTAPID2 -expected-id $_BSTAPID3 \
- -expected-id $_BSTAPID4 -expected-id $_BSTAPID5 \
- -expected-id $_BSTAPID6 -expected-id $_BSTAPID7
-
-set _TARGETNAME $_CHIPNAME.cpu
-target create $_TARGETNAME cortex_m3 -endian $_ENDIAN -chain-position
$_TARGETNAME
-
-$_TARGETNAME configure -work-area-phys 0x2000 -work-area-size
$_WORKAREASIZE -work-area-backup 0
-
-# flash size will be probed
-set _FLASHNAME $_CHIPNAME.flash
-flash bank $_FLASHNAME stm32x 0x0800 0 0 0 $_TARGETNAME
-
-# if srst is not fitted use SYSRESETREQ to
-# perform a soft reset
-cortex_m3 reset_config sysresetreq
+# script for stm32
+
+if { [info exists CHIPNAME] } {
+ set _CHIPNAME $CHIPNAME
+} else {
+ set _CHIPNAME stm32
+}
+
+if { [info exists ENDIAN] } {
+ set _ENDIAN $ENDIAN
+} else {
+ set _ENDIAN little
+}
+
+# Work-area is a space in RAM used for flash programming
+# By default use 16kB
+if { [info exists WORKAREASIZE] } {
+ set _WORKAREASIZE $WORKAREASIZE
+} else {
+ set _WORKAREASIZE 0x4000
+}
+
+if { [info exists FLASH_DRV ] } {
+#one interesting value to set could be: stm32f2xxx
+set _FLASH_DRV $FLASH_DRV
+} else {
+ set _FLASH_DRV stm32x
+}
+
+
+
+# JTAG speed should be = F_CPU/6. F_CPU after reset is 8MHz, so use F_JTAG =
1MHz
+adapter_khz 1000
+
+adapter_nsrst_delay 100
+jtag_ntrst_delay 100
+
+
+
+
+#STM32F2xx
+set _CPUTAPID_2xx 0x4ba00477
+
+#STM32F1xx
+set _CPUTAPID_1xx 0x3ba00477
+
+jtag newtap $_CHIPNAME cpu -irlen 4 -ircapture 0x1 -irmask 0xf \
+-expected-id $_CPUTAPID_1xx \
+-expected-id $_CPUTAPID_2xx
+
+
+if { [info exists BSTAPID ] } {
+ # FIXME this never gets used to override defaults...
+ set _BSTAPID $BSTAPID
+} else {
+ # See STM Document RM0008
+ # Section 29.6.2
+ # Low density devices, Rev A
+ set _BSTAPID1 0x06412041
+ # Medium density devices, Rev A
+ set _BSTAPID2 0x06410041
+ # Medium density devices, Rev B and Rev Z
+ set _BSTAPID3 0x16410041
+ set _BSTAPID4 0x06420041
+ # High density devices, Rev A
+ set _BSTAPID5 0x06414041
+ # Connectivity line devices, Rev A and Rev Z
+ set _BSTAPID6 0x06418041
+ # XL line devices, Rev A
+ set _BSTAPID7 0x06430041
+ # STM32F2XX
+ set _BSTAPID8 0x06411041
+}
+jtag newtap $_CHIPNAME bs -irlen 5 -expected-id $_BSTAPID1 \
+ -expected-id $_BSTAPID2 -expected-id $_BSTAPID3 \
+ -expected-id $_BSTAPID4 -expected-id $_BSTAPID5 \
+ -expected-id $_BSTAPID6 -expected-id $_BSTAPID7 \
+ -expected-id $_BSTAPID8
+
+set _TARGETNAME $_CHIPNAME.cpu
+target create $_TARGETNAME cortex_m3 -endian $_ENDIAN -chain-position
$_TARGETNAME
+
+$_TARGETNAME configure -work-area-phys 0x2000 -work-area-size
$_WORKAREASIZE -work-area-backup 0
+
+# flash size will be probed
+set
Re: [Openocd-development] [PATCH] mips target
The target endian issue still worries me. How the target memory is represented in host memory? Is it in physical byte order? When using a uint8 array, buf[0] is same value on target and host memory, isn't it? If so, mips32_pracc_read_u32 mips32_pracc_read_mem32 mips32_pracc_read_mem16 etc. return wrong byte order on little endian hosts. Attached you will find a patch as discussion base. I tested all four combinations with BE and LE Host against BE and LE target. Commands tested: mdb, mdh, mdw, mwb, mwh, mww, dump_image and load_image fast_load does not work for me. But it also did not work before, so I is maybe a different issue. Since I have no access to EJTAG DMA capable devices, I could not test it. BR, Stefan 0001-mips-fix-some-more-endian-madness.patch Description: 0001-mips-fix-some-more-endian-madness.patch ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] mips target
On Mon, May 30, 2011 at 4:34 PM, Mahr, Stefan stefan.m...@sphairon.com wrote: fast_load does not work for me. But it also did not work before, so I is maybe a different issue. Did you tried it with my patches applied, or before ? Because these patches fix fastdata transfer. If you inversed byte order before it comes to the code I added, maybe you have to remove my patches now, which flip bytes again. BR, Drasko ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] mips target
On Mon, May 30, 2011 at 4:34 PM, Mahr, Stefan stefan.m...@sphairon.com wrote: fast_load does not work for me. But it also did not work before, so I is maybe a different issue. Since I have no access to EJTAG DMA capable devices, I could not test it. AFAIK, fastscan don't have to do much with EJTAG DMA. You should be calling mips32_pracc_fastdata_xfer() in the case of PrAcc (your case). BR, Drasko ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] mips target
fast_load does not work for me. But it also did not work before, so I is maybe a different issue. Did you tried it with my patches applied, or before ? Because these patches fix fastdata transfer. I work with HEAD of git repository. If you inversed byte order before it comes to the code I added, maybe you have to remove my patches now, which flip bytes again. I modified your endianness swapping code. The order of bytes in host's byte array now depends on target endianness and works equal for both LE and BE hosts. I think it's the way target.c works. I am not very happy with my crappy patch code, but if it is confirmed to work we could find a smarter way. Since I have no access to EJTAG DMA capable devices, I could not test it. AFAIK, fastscan don't have to do much with EJTAG DMA. You should be calling mips32_pracc_fastdata_xfer() in the case of PrAcc (your case). Sorry, I didn't express myself very well. I just meant, that I can't test DMA. Sure, it has nothing to do with pracc_fastdata. ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] mips target
fast_load does not work for me. But it also did not work before, so I is maybe a different issue. Did you tried it with my patches applied, or before ? Because these patches fix fastdata transfer. I work with HEAD of git repository. Sorry, typo in work area configuration. Now fast_load works too. ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] mips target
On Mon, May 30, 2011 at 5:13 PM, Mahr, Stefan stefan.m...@sphairon.com wrote: fast_load does not work for me. But it also did not work before, so I is maybe a different issue. Did you tried it with my patches applied, or before ? Because these patches fix fastdata transfer. I work with HEAD of git repository. Yes, but this changed after Øyvind integrated your first changes. Did the FASTSCAN worked for you before these ? If you inversed byte order before it comes to the code I added, maybe you have to remove my patches now, which flip bytes again. I modified your endianness swapping code. The order of bytes in host's byte array now depends on target endianness and works equal for both LE and BE hosts. I think it's the way target.c works. I did not saw changes to the mips_m4k_write_memory() and mips_m4k_read_memory() (where my byte-flip code lives) in the patches you presented. Maybe I did not follow carefully... I am not very happy with my crappy patch code, but if it is confirmed to work we could find a smarter way. I still did not test it. I can confirm that it worked as I expected before your changes, though. But if these changes would help the design they should be integrated and then the rest of the code corrected, so that the OpenOCD code stays as consistent as possible. I'll take a look soon... BR, Drasko ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] mips target
Correct me if I am wrong, but would not this code :
int mips_ejtag_fastdata_scan(struct mips_ejtag *ejtag_info, int
write_t, uint32_t *data)
{
struct jtag_tap *tap;
+ uint8_t r[4];
+
tap = ejtag_info-tap;
assert(tap != NULL);
@@ -357,10 +367,16 @@ int mips_ejtag_fastdata_scan(struct mips_ejtag
*ejtag_info, int write_t, uint32_
}
else
{
- fields[1].in_value = (uint8_t *) data;
+ fields[1].in_value = r;
}
jtag_add_dr_scan(tap, 2, fields, TAP_IDLE);
+
+ if (!write_t)
+ {
+ *data = buf_get_u32(fields[1].in_value, 0, 32);
+ }
+
keep_alive();
return ERROR_OK;
lead to the NULL pointer dereference in the time of jtag data scan
execution (r is a automatic variable, local to the
mips_ejtag_fastdata_scan() function) ?
BR,
Drasko
On Mon, May 30, 2011 at 4:34 PM, Mahr, Stefan stefan.m...@sphairon.com wrote:
The target endian issue still worries me. How the target memory is
represented in host memory? Is it in physical byte order? When
using a uint8 array, buf[0] is same value on target and host memory,
isn't it?
If so,
mips32_pracc_read_u32
mips32_pracc_read_mem32
mips32_pracc_read_mem16
etc.
return wrong byte order on little endian hosts.
Attached you will find a patch as discussion base. I tested all four
combinations with BE and LE Host against BE and LE target.
Commands tested: mdb, mdh, mdw, mwb, mwh, mww, dump_image and load_image
fast_load does not work for me. But it also did not work before, so I is
maybe a different issue.
Since I have no access to EJTAG DMA capable devices, I could not test it.
BR,
Stefan
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] mips target
On Mon, May 30, 2011 at 5:34 PM, Drasko DRASKOVIC lead to the NULL pointer dereference in the time of jtag data scan execution (r is a automatic variable, local to the mips_ejtag_fastdata_scan() function) ? Correction, not NULL pointer, but some trash value pointer from the no longer valid stack. BR, Drasko ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] mips target
I did not saw changes to the mips_m4k_write_memory() and mips_m4k_read_memory() (where my byte-flip code lives) in the patches you presented. Maybe I did not follow carefully... I talk about the patch I sent a couple of minutes ago. I am not very happy with my crappy patch code, but if it is confirmed to work we could find a smarter way. I still did not test it. I can confirm that it worked as I expected before your changes, though. But if these changes would help the design they should be integrated and then the rest of the code corrected, so that the OpenOCD code stays as consistent as possible. The problem is, that some parts of the code did not work for big endian HOSTs. lead to the NULL pointer dereference in the time of jtag data scan execution (r is a automatic variable, local to the mips_ejtag_fastdata_scan() function) ? Correction, not NULL pointer, but some trash value pointer from the no longer valid stack. No, buf_get_u32 fills r[4]. The initial value does not matter. ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] mips target
On Mon, May 30, 2011 at 7:10 PM, Drasko DRASKOVIC drasko.drasko...@gmail.com wrote: On Mon, May 30, 2011 at 5:47 PM, Mahr, Stefan stefan.m...@sphairon.com wrote: lead to the NULL pointer dereference in the time of jtag data scan execution (r is a automatic variable, local to the mips_ejtag_fastdata_scan() function) ? Correction, not NULL pointer, but some trash value pointer from the no longer valid stack. No, buf_get_u32 fills r[4]. The initial value does not matter. No, I meant about r array. this array is a local variable allocated on a stack. Where is it referenced again ? Outside of this function ? I do not know very well the OpenOCD architecture, I am afraid that this reference might be used during jtag data scan execute function and that at this moment it will not be valid anymore (although I am obviously wrong, since you confirm that it works and you saw no sigfaults). Drasko is probably right here. This will crash and burn. At least sometimes/somewhere. Getting a segfault or even a consistent failure is not guaranteed. However, a more fundamental problem with this code is that it uses the in_field before the jtag queue is likely to have been executed. Most of the times it would just set *data to whatever garbage that was on the stack where r[] got allocated. The correct way to handle host/target endianness swapping without forcing a queue execution is to add a callback to the queue. See for example adi_jtag_dp_scan_u32() in adi_v5_jtag.c. Regards, Andreas ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] mips target
lead to the NULL pointer dereference in the time of jtag data scan execution (r is a automatic variable, local to the mips_ejtag_fastdata_scan() function) ? Correction, not NULL pointer, but some trash value pointer from the no longer valid stack. No, buf_get_u32 fills r[4]. The initial value does not matter. No, I meant about r array. this array is a local variable allocated on a stack. Where is it referenced again ? Outside of this function ? I do not know very well the OpenOCD architecture, I am afraid that this reference might be used during jtag data scan execute function and that at this moment it will not be valid anymore (although I am obviously wrong, since you confirm that it works and you saw no sigfaults). Drasko is probably right here. This will crash and burn. At least sometimes/somewhere. Getting a segfault or even a consistent failure is not guaranteed. However, a more fundamental problem with this code is that it uses the in_field before the jtag queue is likely to have been executed. Most of the times it would just set *data to whatever garbage that was on the stack where r[] got allocated. Yes, you are absolutely right. I overlooked to add jtag_execute_queue(). The correct way to handle host/target endianness swapping without forcing a queue execution is to add a callback to the queue. See for example adi_jtag_dp_scan_u32() in adi_v5_jtag.c. Ok, I will take a look to it later. BR, Stefan ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] mips target
On Mon, May 30, 2011 at 11:32 PM, Mahr, Stefan stefan.m...@sphairon.com wrote: Yes, you are absolutely right. I overlooked to add jtag_execute_queue(). Problematic with jtag_execute_queue() (as per my understanding) is that it adds execution delay and should be delayed as much as possible (i.e. you keep adding things you want to scan, and then you call jtag_execute_queue() just when you need the data to be read because you can not proceed beyond without reading/writing it - for example you need some reg value to condition - check). I bet that they have been deliberatly avoided in these functions because of performance penalities... BR, Drasko ___ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
Re: [Openocd-development] [PATCH] mips target
I bet that they have been deliberatly avoided in these functions
because of performance penalities...
So it should go like this?
--- a/src/target/mips_ejtag.c
+++ b/src/target/mips_ejtag.c
@@ -339,10 +339,15 @@ int mips_ejtag_init(struct mips_ejtag *ejtag_info)
return ERROR_OK;
}
+static __inline__ void mips_le_to_h_u32(jtag_callback_data_t arg)
+{
+ uint8_t *in = (uint8_t *)arg;
+ *((uint32_t *)arg) = le_to_h_u32(in);
+}
+
int mips_ejtag_fastdata_scan(struct mips_ejtag *ejtag_info, int write_t,
uint32_t *data)
{
struct jtag_tap *tap;
- uint8_t r[4];
tap = ejtag_info-tap;
assert(tap != NULL);
@@ -367,15 +372,14 @@ int mips_ejtag_fastdata_scan(struct mips_ejtag
*ejtag_info, int write_t, ui
}
else
{
- fields[1].in_value = r;
+ fields[1].in_value = (void *) data;
}
jtag_add_dr_scan(tap, 2, fields, TAP_IDLE);
- if (!write_t)
- {
- *data = buf_get_u32(fields[1].in_value, 0, 32);
- }
+ if ( (!write_t) (data) )
+ jtag_add_callback(mips_le_to_h_u32,
+ (jtag_callback_data_t) data);
keep_alive();
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
