From 7a901cfccdcf18fa6f40c4791311b19bdf37a527 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Wed, 8 Nov 2017 10:47:22 -0800 Subject: [PATCH 1/3] atmel-samd: Align SCSI responses. --- ports/atmel-samd/usb_mass_storage.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ports/atmel-samd/usb_mass_storage.c b/ports/atmel-samd/usb_mass_storage.c index d9d9d9f39384e..3e5b13d3212ee 100644 --- a/ports/atmel-samd/usb_mass_storage.c +++ b/ports/atmel-samd/usb_mass_storage.c @@ -63,10 +63,10 @@ static fs_user_mount_t* get_vfs(int lun) { /* Inquiry Information */ // This is designed to handle the common case where we have an internal file // system and an optional SD card. -static uint8_t inquiry_info[2][36]; +COMPILER_ALIGNED(4) static uint8_t inquiry_info[2][36]; /* Capacities of Disk */ -static uint8_t format_capa[2][8]; +COMPILER_ALIGNED(4) static uint8_t format_capa[2][8]; /** * \brief Eject Disk From 971bd7e9a6e38d7bae26872c1a055b45321179e0 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Wed, 8 Nov 2017 12:42:50 -0800 Subject: [PATCH 2/3] 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 3e5b13d3212ee..f20878949bfcc 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; From ef9a7f262a9272e0771cdec4981a54844462add8 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Wed, 8 Nov 2017 16:01:38 -0800 Subject: [PATCH 3/3] atmel-samd: Turn off nvmctl cache and rework active_read. --- ports/atmel-samd/asf4_conf/samd51/hpl_nvmctrl_config.h | 4 ++-- ports/atmel-samd/usb_mass_storage.c | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/ports/atmel-samd/asf4_conf/samd51/hpl_nvmctrl_config.h b/ports/atmel-samd/asf4_conf/samd51/hpl_nvmctrl_config.h index daaba85d4018c..53fcb593abbc4 100644 --- a/ports/atmel-samd/asf4_conf/samd51/hpl_nvmctrl_config.h +++ b/ports/atmel-samd/asf4_conf/samd51/hpl_nvmctrl_config.h @@ -19,14 +19,14 @@ // Indicate whether AHB0 cache is disable or not // nvm_arch_cache0 #ifndef CONF_NVM_CACHE0 -#define CONF_NVM_CACHE0 0 +#define CONF_NVM_CACHE0 1 #endif // AHB1 Cache Disable // Indicate whether AHB1 cache is disable or not // nvm_arch_cache1 #ifndef CONF_NVM_CACHE1 -#define CONF_NVM_CACHE1 0 +#define CONF_NVM_CACHE1 1 #endif // diff --git a/ports/atmel-samd/usb_mass_storage.c b/ports/atmel-samd/usb_mass_storage.c index f20878949bfcc..85c662e774c93 100644 --- a/ports/atmel-samd/usb_mass_storage.c +++ b/ports/atmel-samd/usb_mass_storage.c @@ -260,9 +260,6 @@ int32_t usb_msc_xfer_done(uint8_t lun) { if (active_read) { active_addr += 1; active_nblocks--; - if (active_nblocks == 0) { - active_read = false; - } } if (active_write) { @@ -277,6 +274,10 @@ int32_t usb_msc_xfer_done(uint8_t lun) { // The start_read callback begins a read transaction which we accept but delay our response until the "main thread" calls usb_msc_background. Once it does, we read immediately from the drive into our cache and trigger the USB DMA to output the sector. Once the sector is transmitted, xfer_done will be called. void usb_msc_background(void) { if (active_read && !usb_busy) { + if (active_nblocks == 0) { + active_read = false; + return; + } fs_user_mount_t * vfs = get_vfs(active_lun); disk_read(vfs, sector_buffer, active_addr, 1); CRITICAL_SECTION_ENTER();