-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
STM32L496xx: MCU restarts after running st-util, loading image #980
Comments
Honestly I don't really understand this issue report. |
@Nightwalker-87 |
There may be no response from the author any more. I still can't see the error yet. |
I'm still responsive, just not able to respond immediately. I'll dig in further to see how I can describe the issue better and/or isolate it to a specific change between 1.6.0 and 1.6.1. |
I bisected between v1.6.0 and v1.6.1 and identified the regressing commit:
Unfortunately, that's rather a large commit, and I'm not very familiar with this codebase (nor STM32 programming more generally), so it isn't immediately obvious to me where the problem might be nor how to debug it. I can run my build of st-util in a debugger, however, and I'm happy to do so, if you have some ideas about where the issue might be that you'd like me to investigate. FWIW, I use a .gdbinit file that looks like this:
(I know that last line is redundant in st-link 1.6.1, but my team is still pinned to 1.6.0 because of this issue.) Here's what gdb output looks like when I start gdb with a build of st-util from before the regressing commit:
Then my program starts running on the target. When I start gdb with a build of st-util on/after the regressing commit, however, I see this:
In other words: the program fails to start, even on the second try. And subsequent tries continue to fail. Note that the program sometimes fails to start on the first try even before the regressing commit. But when that happens, it always starts the second time. After the regressing commit, however, sometimes it successfully starts on the first try; but when it doesn't, then it never succeeds. |
@mykmelez Thanks for the update and the details. |
Here is the graphical view of the mentioned commit: 09ea99a I may assume the relevant change is in |
Obviously also related to #692. |
I tried reverting the change in gdb-server.c on top of the regressing commit, but that didn't have any effect. Then I started reverting additional chunks by hand, one at a time, and I discovered that reverting the changes to just two functions in usb.c resolved the issues. Here's the set of changes I made to restore the original behavior: diff --git a/src/usb.c b/src/usb.c
index 12945d6..3a67749 100644
--- a/src/usb.c
+++ b/src/usb.c
@@ -505,10 +505,10 @@ int _stlink_usb_reset(stlink_t * sl) {
int i = fill_command(sl, SG_DXFER_FROM_DEV, rep_len);
cmd[i++] = STLINK_DEBUG_COMMAND;
- if (sl->version.jtag_api == STLINK_JTAG_API_V1)
+ // if (sl->version.jtag_api == STLINK_JTAG_API_V1)
cmd[i++] = STLINK_DEBUG_APIV1_RESETSYS;
- else
- cmd[i++] = STLINK_DEBUG_APIV2_RESETSYS;
+ // else
+ // cmd[i++] = STLINK_DEBUG_APIV2_RESETSYS;
size = send_recv(slu, 1, cmd, slu->cmd_len, data, rep_len);
if (size == -1) {
@@ -718,14 +718,15 @@ int _stlink_usb_read_all_regs(stlink_t *sl, struct stlink_reg *regp) {
unsigned char* const cmd = sl->c_buf;
unsigned char* const data = sl->q_buf;
ssize_t size;
- uint32_t rep_len = sl->version.jtag_api == STLINK_JTAG_API_V1 ? 84 : 88;
+ // uint32_t rep_len = sl->version.jtag_api == STLINK_JTAG_API_V1 ? 84 : 88;
+ uint32_t rep_len = 84;
int i = fill_command(sl, SG_DXFER_FROM_DEV, rep_len);
cmd[i++] = STLINK_DEBUG_COMMAND;
- if (sl->version.jtag_api == STLINK_JTAG_API_V1)
+ // if (sl->version.jtag_api == STLINK_JTAG_API_V1)
cmd[i++] = STLINK_DEBUG_APIV1_READALLREGS;
- else
- cmd[i++] = STLINK_DEBUG_APIV2_READALLREGS;
+ // else
+ // cmd[i++] = STLINK_DEBUG_APIV2_READALLREGS;
size = send_recv(slu, 1, cmd, slu->cmd_len, data, rep_len);
if (size == -1) {
printf("[!] send_recv STLINK_DEBUG_READALLREGS\n");
@@ -734,7 +735,8 @@ int _stlink_usb_read_all_regs(stlink_t *sl, struct stlink_reg *regp) {
/* V1: regs data from offset 0 */
/* V2: status at offset 0, regs data from offset 4 */
- int reg_offset = sl->version.jtag_api == STLINK_JTAG_API_V1 ? 0 : 4;
+ // int reg_offset = sl->version.jtag_api == STLINK_JTAG_API_V1 ? 0 : 4;
+ int reg_offset = 0;
sl->q_len = (int) size;
stlink_print_data(sl);
for(i=reg_offset; i<16; i++) The changes in _stlink_usb_reset() fixed the issue that the MPU restarts the program rather than halting when I invoke st-util; the changes in _stlink_usb_read_all_regs() fixed the issue that starting the program in GDB fails repeatedly. Looking at those changes, my first thought was that perhaps my ST-LINK/V2 actually supports STLINK_JTAG_API_V1 rather than STLINK_JTAG_API_V2, so I can resolve these problems (and others) by simply setting slv->jtag_api to STLINK_JTAG_API_V1 in __parse_version()_. But that introduced new problems:
So I guess it isn't that simple. I also considered the possibility that I'm running older firmware on the ST-LINK/V2, and newer firmware would resolve the problem. So I updated the device from V2J29S7 to V2J37S7 (the newest version available on ST's website). But that doesn't seem to have made a difference. I'm not sure what to try next. Perhaps my device really does support STLINK_JTAG_API_V2, but the code in usb.c is using the V2 API incorrectly in some cases? If so, then it wouldn't be correct to treat my device as supporting STLINK_JTAG_API_V1. Rather, it would be necessary to identify the specific cases of incorrect V2 API usage and correct them. I'm not sure if that's really the right approach, though. (Of course I also know that any changeset would have to be developed against the tip of the develop branch, not on top of the regressing commit. I only started there in the hopes that it would help to narrow the regression.) |
After thinking about it a bit more, I realized that maybe the issue is that some of the code that was introduced to support STLINK_JTAG_API_V3 is specific to that JTAG API version, and it doesn't apply to V2. In that case, using the V1 version of the code for both V1 and V2 might be the solution, i.e. something like this change on top of the tip of the develop branch: diff --git a/src/stlink-lib/usb.c b/src/stlink-lib/usb.c
index ce0d672..a8cd0fa 100644
--- a/src/stlink-lib/usb.c
+++ b/src/stlink-lib/usb.c
@@ -523,10 +523,10 @@ int _stlink_usb_reset(stlink_t * sl) {
i = fill_command(sl, SG_DXFER_FROM_DEV, rep_len);
cmd[i++] = STLINK_DEBUG_COMMAND;
- if (sl->version.jtag_api == STLINK_JTAG_API_V1) {
- cmd[i++] = STLINK_DEBUG_APIV1_RESETSYS;
- } else {
+ if (sl->version.jtag_api == STLINK_JTAG_API_V3) {
cmd[i++] = STLINK_DEBUG_APIV2_RESETSYS;
+ } else {
+ cmd[i++] = STLINK_DEBUG_APIV1_RESETSYS;
}
size = send_recv(slu, 1, cmd, slu->cmd_len, data, rep_len);
@@ -796,15 +796,15 @@ int _stlink_usb_read_all_regs(stlink_t *sl, struct stlink_reg *regp) {
unsigned char* const cmd = sl->c_buf;
unsigned char* const data = sl->q_buf;
ssize_t size;
- uint32_t rep_len = sl->version.jtag_api == STLINK_JTAG_API_V1 ? 84 : 88;
+ uint32_t rep_len = sl->version.jtag_api == STLINK_JTAG_API_V3 ? 88 : 84;
int i = fill_command(sl, SG_DXFER_FROM_DEV, rep_len);
cmd[i++] = STLINK_DEBUG_COMMAND;
- if (sl->version.jtag_api == STLINK_JTAG_API_V1) {
- cmd[i++] = STLINK_DEBUG_APIV1_READALLREGS;
- } else {
+ if (sl->version.jtag_api == STLINK_JTAG_API_V3) {
cmd[i++] = STLINK_DEBUG_APIV2_READALLREGS;
+ } else {
+ cmd[i++] = STLINK_DEBUG_APIV1_READALLREGS;
}
size = send_recv(slu, 1, cmd, slu->cmd_len, data, rep_len);
@@ -814,9 +814,9 @@ int _stlink_usb_read_all_regs(stlink_t *sl, struct stlink_reg *regp) {
return((int)size);
}
- /* V1: regs data from offset 0 */
- /* V2: status at offset 0, regs data from offset 4 */
- int reg_offset = sl->version.jtag_api == STLINK_JTAG_API_V1 ? 0 : 4;
+ /* V1–2: regs data from offset 0 */
+ /* V3: status at offset 0, regs data from offset 4 */
+ int reg_offset = sl->version.jtag_api == STLINK_JTAG_API_V3 ? 4 : 0;
sl->q_len = (int)size;
stlink_print_data(sl);
That change does fix the behavior when starting st-util, although I haven't been able to confirm that it fixes loading and starting a program in gdb, as I run into what looks like #995 when I try to do so:
|
Thanks for investigating further. This is indeed very helpful 😉
With this information we may now be able to address not only this but also other similar issues.
This is also the best possible approach from my point of view. |
We should involve @Ant-ON in this discussion, as he edited this part of the code (programmer APIs) recently. |
@Ant-ON, I tried the And it seems to fix both problems for me there (although I'm not entirely positive because I'm running into #995 when I try to test it). |
@mykmelez Current
stlink/src/gdbserver/gdb-server.c Lines 244 to 246 in 393310f
stlink/src/st-util/gdb-server.c Lines 227 to 229 in 6aa08a6
In the |
Indeed! With your fix, and my fixes in the comment above, I'm able to flash and run my large program successfully onto a device with an STM32L496 MCU.
A reset does occur for me in both 1.6.0 and 1.6.1, but what happens after the reset is different.
With my fixes, the behavior returns to the behavior on 1.6.0. And flashing works again.
That's good to know, but I actually do want to reset when running st-util, since I start st-util in order to flash a new program onto my device. Thus the behavior in 1.6.0 is actually the ideal behavior for me: when starting st-util, halt the processor and wait for gdb to start and flash a new program. However, if it's intentional for the device to restart rather than halting when starting st-util, then that's fine, even if it isn't ideal. In that case, perhaps only one of my fixes is necessary in order to fix the issue with flashing. |
We should implement the most useful approach here, I think. |
@mykmelez As I understand it, different versions of the reset functions have the different behaviour. But in general, both work. I think we should save them as they are. I have unified the reset function. Removed unnecessary reset calls from |
@Ant-ON, in my testing, your fl_fix branch seems to have fixed this issue for me:
I also tested st-util without any flags and with |
When I run st-util 1.6.1, the board restarts rather than waiting for me to start arm-none-eabi-gdb, load the image, and
continue
. And it often (although not always) also restarts right after I load the image, before I entercontinue
. In st-util 1.6.0, the board wouldn't restart until after I started arm-none-eabi-gdb, loaded the image, and enteredcontinue
.Commandline-Output:
Running st-util 1.6.1 shows this output:
And then the board restarts. Starting arm-none-eabi-gdb and then loading the image often (but not always) also results in the board restarting after the image has been loaded, but before I enter
continue
.Expected/description:
st-util 1.6.0 showed this instead:
And the board didn't restart until I started arm-none-eabi-gdb, loaded the image, and entered
continue
.The text was updated successfully, but these errors were encountered: