From 971bd7e9a6e38d7bae26872c1a055b45321179e0 Mon Sep 17 00:00:00 2001
From: Scott Shawcroft <scott.shawcroft@gmail.com>
Date: Wed, 8 Nov 2017 12:42:50 -0800
Subject: [PATCH] atmel-samd: Improve MSC reliability.

* Be more liberal with critical sections to ensure ordering.
* Correct usb_busy so that it is busy when no errors occur on
  transfer. I believe it worked before because it would be false
  momentarily until a second transfer was attempted and a busy
  error was returned, therefore setting usb_busy to true. That
  risks the first "failed" transfer completing before a second one
  is attempted.
---
 ports/atmel-samd/usb_mass_storage.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/ports/atmel-samd/usb_mass_storage.c b/ports/atmel-samd/usb_mass_storage.c
index 3e5b13d321..f20878949b 100644
--- a/ports/atmel-samd/usb_mass_storage.c
+++ b/ports/atmel-samd/usb_mass_storage.c
@@ -256,6 +256,7 @@ int32_t usb_msc_xfer_done(uint8_t lun) {
         return ERR_DENIED;
     }
 
+    CRITICAL_SECTION_ENTER();
     if (active_read) {
         active_addr += 1;
         active_nblocks--;
@@ -268,6 +269,7 @@ int32_t usb_msc_xfer_done(uint8_t lun) {
         sector_loaded = true;
     }
     usb_busy = false;
+    CRITICAL_SECTION_LEAVE();
 
     return ERR_NONE;
 }
@@ -277,9 +279,10 @@ void usb_msc_background(void) {
     if (active_read && !usb_busy) {
         fs_user_mount_t * vfs = get_vfs(active_lun);
         disk_read(vfs, sector_buffer, active_addr, 1);
-        // TODO(tannewt): Check the read result.
-        mscdf_xfer_blocks(true, sector_buffer, 1);
-        usb_busy = true;
+        CRITICAL_SECTION_ENTER();
+        int32_t result = mscdf_xfer_blocks(true, sector_buffer, 1);
+        usb_busy = result == ERR_NONE;
+        CRITICAL_SECTION_LEAVE();
     }
     if (active_write && !usb_busy) {
         if (sector_loaded) {
@@ -306,8 +309,14 @@ void usb_msc_background(void) {
         }
         // Load more blocks from USB if they are needed.
         if (active_nblocks > 0) {
+            // Turn off interrupts because with them on,
+            // usb_msc_xfer_done could be called before we update
+            // usb_busy. If that happened, we'd overwrite the fact that
+            // the transfer actually already finished.
+            CRITICAL_SECTION_ENTER();
             int32_t result = mscdf_xfer_blocks(false, sector_buffer, 1);
-            usb_busy = result != ERR_NONE;
+            usb_busy = result == ERR_NONE;
+            CRITICAL_SECTION_LEAVE();
         } else {
             mscdf_xfer_blocks(false, NULL, 0);
             active_write = false;