adafruit / Adafruit_nRF52_Bootloader

USB-enabled bootloaders for the nRF52 BLE SoC chips
MIT License
434 stars 383 forks source link

Windows 10: Error 0x800701B1 when copying UF2 firmware (copy is successful) #120

Closed xobs closed 4 years ago

xobs commented 4 years ago

Describe the bug Windows 10 reports a transfer error after writing the last block. Similar to https://github.com/adafruit/Adafruit_nRF52_Bootloader/issues/46. The transfer is successful.

Set up (please complete the following information)

To Reproduce Steps to reproduce the behavior:

  1. Enter bootloader
  2. Copy file

Expected behavior It shouldn't print an error after copying.

Screenshots image

Additional context This is, admittedly, an extremely hacked-down version of this bootloader located at https://github.com/simmel-project/bootloader. One difference is that we have LTO enabled (-flto) which caused issues with the vector table generation and weak symbols. Symbols such as SVC_Handler were getting stuck as their infinite-loop variants rather than getting overridden.

Having said that, everything appears to work just fine, it's just that Windows is doing something like queueing multiple LUNs, and the bootloader looks like it's rebooting after only one LUN is acknowledged.

The workaround I've made in https://github.com/simmel-project/bootloader/commit/037d3501161663209c37f77a523cd3e7da2c9ca3 fixes the problem, but doesn't feel right, during board teardown just before USB is disconnected:

  unsigned int i;
    for (i = 0; i < 100; i++) {
      tud_task();
      NRFX_DELAY_MS(1);
    }
hathach commented 4 years ago

hmm, since I couldn't reproduce this issue with this repo. This issues appeared once, and we fixed it already. I don't remember the detail. Maybe it is specific to 833 or your folk. So I can only help with analyzing and providing information at best. As far as I can tell, windows is not happy with the end of transfer. It will be more useful if you could provide usb capture and/or the usb stack log to see which command it failed to response to.

Since this is timing issue, you should use either RTT or SWO as logger to prevent messing with timing. I have done rtt/swo logger on my repo. https://github.com/hathach/tinyusb/issues/367 . You can try to follow that to add retarget for the rtt/swo.

PS: a little bit of delay is not that bad, many bootloader uses this as walkaround as well. It doesn't hurt anyone. Although it does hide the issue, it hides it very well that it is not reproducible :smile:

xobs commented 4 years ago

I'm not sure what you're looking for. Here is what I get when I hook up the usb logger: image

So the Status Transport is never coming back from the final packet. Should tud_msc_write10_complete_cb(lun) be called before the final Status packet is acknowledged?

hathach commented 4 years ago

tud_msc_write10_complete_cb() is called when the Data stage is completed, afterward the status will be queued up. I used to put it after the status queuing, but there is a racing condition somehow with samd + qspi on Arduino. So I have to invoke it first. I forgot the detail, there is a commit for that purpose https://github.com/hathach/tinyusb/commit/605129eb6600ffb50ea2d239e0ad78a9636df8e1

Normally the finishing task should be enough for status to complete. Too bad, I won't have time to test this out a gain. A simple delay of a few ms when exiting bootloader mode should be OK though.

PS: I will just put the racing note to the quote, else I may tempt to revert the order and got myself into loop of debugging :D . It could be Arduino lib issue though, unfortunately, we need to stick to whatever works :D.

xobs commented 4 years ago

@hathach Yes, I see the problem in https://github.com/hathach/tinyusb/commit/605129eb6600ffb50ea2d239e0ad78a9636df8e1, and if I revert to the previous commit https://github.com/hathach/tinyusb/commit/2caa1ac07804bef581cd30e6fc5057a6515fb094 the problem goes away. So https://github.com/hathach/tinyusb/commit/605129eb6600ffb50ea2d239e0ad78a9636df8e1is where the problem appears.

hathach commented 4 years ago

Yeah, thanks for testing out. but I don't really have time now to test samd51 + qspi on Arduino again. Maybe later, I put down the note to the code already so that I know what to test when reverting that in the future.

xobs commented 4 years ago

Thanks for narrowing down the commit! I wonder why nobody else is seeing this.

I'll keep the delay for now, since it works around the issue.

hathach commented 4 years ago

@xobs no problems at all, most usb issues are easier to troubleshoot with a hardware analyzer. The more I think the more it is not right currently. I just create an issue as reminder to revert that commit when I have more time in the future and/or more people having the same issue as this.