trying proper scsi handling

kernel/src/scsi/mod.rs

@@ -1,5 +1,5 @@
+use crate::storage::{SDCARD, SdCard};
 use core::sync::atomic::{AtomicBool, Ordering};
-use embassy_futures::yield_now;
 use embassy_time::Timer;
 use embassy_usb::Builder;
 use embassy_usb::driver::{Driver, EndpointIn, EndpointOut};
@@ -12,25 +12,24 @@ use scsi_types::*;
 mod error;
 use error::ScsiSense;
 
-use crate::storage::{SDCARD, SdCard};
-use crate::usb::stop_usb;
-
 const BULK_ENDPOINT_PACKET_SIZE: usize = 64;
 
 // indicates that a scsi transaction is occurring and is
 // NOT safe to disable usb, or acquire sdcard
 pub static SCSI_BUSY: AtomicBool = AtomicBool::new(false);
 
-// start no more transfers, usb is trying to stop
+// stop scsi and return sdcard when safe to do so
 pub static SCSI_HALT: AtomicBool = AtomicBool::new(false);
 
+pub static SCSI_EJECTED: AtomicBool = AtomicBool::new(false);
+
 pub struct MassStorageClass<'d, D: Driver<'d>> {
     temp_sd: Option<SdCard>, // temporary owns sdcard when scsi is running
-    pub pending_eject: bool,
-    pub prevent_removal: bool,
     bulk_out: D::EndpointOut,
     bulk_in: D::EndpointIn,
     last_sense: ScsiSense,
+    prevent_removal: bool,
+    pending_eject: bool,
 }
 
 impl<'d, 's, D: Driver<'d>> MassStorageClass<'d, D> {
@@ -44,11 +43,11 @@ impl<'d, 's, D: Driver<'d>> MassStorageClass<'d, D> {
 
         Self {
             temp_sd: None,
-            pending_eject: false,
-            prevent_removal: false,
             bulk_out,
             bulk_in,
             last_sense: ScsiSense::no_sense(),
+            prevent_removal: false,
+            pending_eject: false,
         }
     }
 
@@ -64,6 +63,7 @@ impl<'d, 's, D: Driver<'d>> MassStorageClass<'d, D> {
         }
     }
 
+    // return sdcard to global, so other tasks can use it
     pub async fn return_sdcard(&mut self) {
         if let Some(card) = self.temp_sd.take() {
             let mut guard = SDCARD.get().lock().await;
@@ -73,19 +73,26 @@ impl<'d, 's, D: Driver<'d>> MassStorageClass<'d, D> {
 
     pub async fn poll(&mut self) {
         loop {
-            if !self.prevent_removal {
+            if !self.prevent_removal && (SCSI_HALT.load(Ordering::Acquire) || self.pending_eject) {
-                if SCSI_HALT.load(Ordering::Acquire) || self.pending_eject {
+                if self.temp_sd.is_some() {
-                    break;
+                    self.return_sdcard().await;
+                    SCSI_BUSY.store(false, Ordering::Release);
+                    self.pending_eject = false;
                 }
-            }
+            } else {
-
                 self.handle_cbw().await;
-
-            if !self.prevent_removal && self.pending_eject {
-                break;
             }
 
+            if SCSI_EJECTED.load(Ordering::Acquire) {
+                Timer::after_millis(100).await;
+                continue;
+            }
+
+            if SCSI_BUSY.load(Ordering::Relaxed) {
                 Timer::after_millis(1).await;
+            } else {
+                Timer::after_millis(100).await;
+            }
         }
     }
 
@@ -94,16 +101,12 @@ impl<'d, 's, D: Driver<'d>> MassStorageClass<'d, D> {
         if let Ok(n) = self.bulk_out.read(&mut cbw_buf).await {
             if n == 31 {
                 if let Some(cbw) = CommandBlockWrapper::parse(&cbw_buf[..n]) {
-                    SCSI_BUSY.store(true, Ordering::Release);
-
                     let command = parse_cb(&cbw.CBWCB);
                     if self.handle_command(command).await.is_ok() {
                         self.send_csw_success(cbw.dCBWTag).await
                     } else {
                         self.send_csw_fail(cbw.dCBWTag).await
                     }
-
-                    SCSI_BUSY.store(false, Ordering::Release);
                 } else {
                     self.last_sense = ScsiSense::invalid_cdb();
                     self.send_csw_fail(0).await;
@@ -203,12 +206,26 @@ impl<'d, 's, D: Driver<'d>> MassStorageClass<'d, D> {
                 self.bulk_in.write(&response[..len]).await.map_err(|_| ())
             }
             ScsiCommand::TestUnitReady => {
-                if self.temp_sd.as_ref().unwrap().is_attached() {
+                if !SCSI_EJECTED.load(Ordering::Acquire) && self.temp_sd.is_none() {
+                    SCSI_BUSY.store(true, Ordering::Release);
+                    self.take_sdcard().await;
+                }
+
+                match &self.temp_sd {
+                    Some(sd) => {
+                        if sd.is_attached() {
                             Ok(())
                         } else {
+                            self.last_sense = ScsiSense::medium_not_present();
                             Err(())
                         }
                     }
+                    None => {
+                        self.last_sense = ScsiSense::medium_not_present();
+                        Err(())
+                    }
+                }
+            }
             ScsiCommand::RequestSense { desc: _, alloc_len } => {
                 // Prepare the buffer
                 let mut buf = [0u8; 18];
@@ -262,8 +279,9 @@ impl<'d, 's, D: Driver<'d>> MassStorageClass<'d, D> {
 
                 self.bulk_in.write(&response[..len]).await.map_err(|_| ())
             }
-            ScsiCommand::ReadCapacity10 => {
+            ScsiCommand::ReadCapacity10 => match self.temp_sd.as_ref() {
-                let total_blocks = self.temp_sd.as_ref().unwrap().size() / BLOCK_SIZE as u64;
+                Some(sd) => {
+                    let total_blocks = sd.size() / BLOCK_SIZE as u64;
 
                     let last_lba = total_blocks.checked_sub(1).unwrap_or(0);
 
@@ -272,8 +290,14 @@ impl<'d, 's, D: Driver<'d>> MassStorageClass<'d, D> {
 
                     self.bulk_in.write(&response).await.map_err(|_| ())
                 }
-            ScsiCommand::ReadCapacity16 { alloc_len } => {
+                None => {
-                let total_blocks = self.temp_sd.as_ref().unwrap().size() / BLOCK_SIZE as u64;
+                    self.last_sense = ScsiSense::medium_not_present();
+                    Err(())
+                }
+            },
+            ScsiCommand::ReadCapacity16 { alloc_len } => match self.temp_sd.as_ref() {
+                Some(sd) => {
+                    let total_blocks = sd.size() / BLOCK_SIZE as u64;
 
                     let last_lba = total_blocks.checked_sub(1).unwrap_or(0);
 
@@ -284,8 +308,13 @@ impl<'d, 's, D: Driver<'d>> MassStorageClass<'d, D> {
                     let len = alloc_len.min(response.len() as u32) as usize;
                     self.bulk_in.write(&response[..len]).await.map_err(|_| ())
                 }
-            ScsiCommand::Read { lba, len } => {
+                None => {
-                let sdcard = self.temp_sd.as_ref().unwrap();
+                    self.last_sense = ScsiSense::medium_not_present();
+                    Err(())
+                }
+            },
+            ScsiCommand::Read { lba, len } => match self.temp_sd.as_ref() {
+                Some(sd) => {
                     let mut blocks = len;
                     let mut idx = lba;
 
@@ -293,10 +322,11 @@ impl<'d, 's, D: Driver<'d>> MassStorageClass<'d, D> {
 
                     while blocks > 0 {
                         if blocks >= block_buf.len() as u64 {
-                        sdcard.read_blocks(&mut block_buf, BlockIdx(idx as u32))?;
+                            sd.read_blocks(&mut block_buf, BlockIdx(idx as u32))?;
 
                             for block in &mut block_buf {
-                            for chunk in block.contents.chunks(BULK_ENDPOINT_PACKET_SIZE.into()) {
+                                for chunk in block.contents.chunks(BULK_ENDPOINT_PACKET_SIZE.into())
+                                {
                                     if self.bulk_in.write(chunk).await.map_err(|_| ()).is_err() {
                                         error_occurred = true
                                     }
@@ -306,11 +336,14 @@ impl<'d, 's, D: Driver<'d>> MassStorageClass<'d, D> {
                             blocks -= block_buf.len() as u64;
                             idx += block_buf.len() as u64;
                         } else {
-                        sdcard
+                            sd.read_blocks(
-                            .read_blocks(&mut block_buf[..blocks as usize], BlockIdx(idx as u32))?;
+                                &mut block_buf[..blocks as usize],
+                                BlockIdx(idx as u32),
+                            )?;
 
                             for block in &block_buf[..blocks as usize] {
-                            for chunk in block.contents.chunks(BULK_ENDPOINT_PACKET_SIZE.into()) {
+                                for chunk in block.contents.chunks(BULK_ENDPOINT_PACKET_SIZE.into())
+                                {
                                     if self.bulk_in.write(chunk).await.map_err(|_| ()).is_err() {
                                         error_occurred = true
                                     }
@@ -329,8 +362,13 @@ impl<'d, 's, D: Driver<'d>> MassStorageClass<'d, D> {
 
                     Ok(())
                 }
-            ScsiCommand::Write { lba, len } => {
+                None => {
-                let sdcard = self.temp_sd.as_ref().unwrap();
+                    self.last_sense = ScsiSense::medium_not_present();
+                    Err(())
+                }
+            },
+            ScsiCommand::Write { lba, len } => match self.temp_sd.as_ref() {
+                Some(sd) => {
                     let mut blocks = len;
                     let mut idx = lba;
 
@@ -339,7 +377,8 @@ impl<'d, 's, D: Driver<'d>> MassStorageClass<'d, D> {
                     while blocks > 0 {
                         if blocks >= block_buf.len() as u64 {
                             for block in block_buf.as_mut() {
-                            for chunk in block.contents.chunks_mut(BULK_ENDPOINT_PACKET_SIZE.into())
+                                for chunk in
+                                    block.contents.chunks_mut(BULK_ENDPOINT_PACKET_SIZE.into())
                                 {
                                     if self.bulk_out.read(chunk).await.map_err(|_| ()).is_err() {
                                         error_occurred = true
@@ -347,13 +386,14 @@ impl<'d, 's, D: Driver<'d>> MassStorageClass<'d, D> {
                                 }
                             }
 
-                        sdcard.read_blocks(&mut block_buf, BlockIdx(idx as u32))?;
+                            sd.read_blocks(&mut block_buf, BlockIdx(idx as u32))?;
 
                             blocks -= block_buf.len() as u64;
                             idx += block_buf.len() as u64;
                         } else {
                             for block in block_buf[..blocks as usize].as_mut() {
-                            for chunk in block.contents.chunks_mut(BULK_ENDPOINT_PACKET_SIZE.into())
+                                for chunk in
+                                    block.contents.chunks_mut(BULK_ENDPOINT_PACKET_SIZE.into())
                                 {
                                     if self.bulk_out.read(chunk).await.map_err(|_| ()).is_err() {
                                         error_occurred = true
@@ -361,7 +401,7 @@ impl<'d, 's, D: Driver<'d>> MassStorageClass<'d, D> {
                                 }
                             }
 
-                        sdcard.write_blocks(
+                            sd.write_blocks(
                                 &mut block_buf[..blocks as usize],
                                 BlockIdx(idx as u32),
                             )?;
@@ -378,9 +418,15 @@ impl<'d, 's, D: Driver<'d>> MassStorageClass<'d, D> {
 
                     Ok(())
                 }
-            ScsiCommand::ReadFormatCapacities { alloc_len } => {
+                None => {
+                    self.last_sense = ScsiSense::medium_not_present();
+                    Err(())
+                }
+            },
+            ScsiCommand::ReadFormatCapacities { alloc_len } => match self.temp_sd.as_ref() {
+                Some(sd) => {
                     let block_size = SdCard::BLOCK_SIZE as u32;
-                let num_blocks = (self.temp_sd.as_ref().unwrap().size() / block_size as u64) as u32;
+                    let num_blocks = (sd.size() / block_size as u64) as u32;
 
                     let mut response = [0u8; 12];
 
@@ -398,12 +444,18 @@ impl<'d, 's, D: Driver<'d>> MassStorageClass<'d, D> {
                         .await
                         .map_err(|_| ())
                 }
+                None => {
+                    self.last_sense = ScsiSense::medium_not_present();
+                    Err(())
+                }
+            },
             ScsiCommand::PreventAllowMediumRemoval { prevent } => {
                 self.prevent_removal = prevent;
                 Ok(())
             }
             ScsiCommand::StartStopUnit { start, load_eject } => {
                 if !start && load_eject {
+                    SCSI_EJECTED.store(true, Ordering::Release);
                     self.pending_eject = true;
                 }
                 Ok(())

kernel/src/ui.rs

@@ -3,8 +3,8 @@ use crate::{
     display::FRAMEBUFFER,
     framebuffer::FB_PAUSED,
     peripherals::keyboard,
+    scsi::SCSI_BUSY,
     storage::{FileName, SDCARD},
-    usb::{USB_ACTIVE, start_usb, stop_usb},
 };
 use abi_sys::keyboard::{KeyCode, KeyState};
 use alloc::{str::FromStr, string::String, vec::Vec};
@@ -37,25 +37,25 @@ pub async fn ui_handler() {
         changed: true,
     };
     let mut scsi = ScsiPage { last_bounds: None };
-    let mut overlay = Overlay::new();
     update_selections().await;
 
     loop {
         // reset page, if usb was disabled internally
-        if ui.page == UiPage::Scsi && !USB_ACTIVE.load(Ordering::Acquire) {
+        if ui.page == UiPage::Scsi && !SCSI_BUSY.load(Ordering::Acquire) {
-            menu.clear().await;
+            scsi.clear().await;
             ui.page = UiPage::Menu;
+        } else if ui.page == UiPage::Menu && SCSI_BUSY.load(Ordering::Acquire) {
+            menu.clear().await;
+            ui.page = UiPage::Scsi;
         }
 
         if input_handler(&mut ui, &mut menu, &mut scsi).await {
-            overlay.clear().await;
             return;
         }
         match ui.page {
             UiPage::Menu => menu.draw().await,
             UiPage::Scsi => scsi.draw().await,
         }
-        overlay.draw().await;
         Timer::after_millis(5).await;
     }
 }
@@ -64,17 +64,6 @@ async fn input_handler(ui: &mut UiState, menu: &mut MenuPage, scsi: &mut ScsiPag
     if let Some(event) = keyboard::read_keyboard_fifo().await {
         if event.state == KeyState::Pressed {
             match (&mut ui.page, event.key) {
-                (UiPage::Menu, KeyCode::F1) => {
-                    start_usb();
-                    menu.clear().await;
-                    ui.page = UiPage::Scsi;
-                }
-                (UiPage::Scsi, KeyCode::F1) => {
-                    stop_usb();
-                    scsi.clear().await;
-                    ui.page = UiPage::Menu;
-                    update_selections().await;
-                }
                 (UiPage::Menu, _) => return menu.handle_input(event.key).await,
                 (UiPage::Scsi, _) => return scsi.handle_input(event.key).await,
             }
@@ -83,43 +72,6 @@ async fn input_handler(ui: &mut UiState, menu: &mut MenuPage, scsi: &mut ScsiPag
     false
 }
 
-struct Overlay {
-    f1_label: &'static str,
-    last_bounds: Option<Rectangle>,
-}
-
-impl Overlay {
-    pub fn new() -> Self {
-        Self {
-            f1_label: "Press F1 to enable/disable mass storage",
-            last_bounds: None,
-        }
-    }
-
-    async fn draw(&mut self) {
-        let text_style = MonoTextStyle::new(&FONT_4X6, Rgb565::WHITE);
-        let fb = unsafe { &mut *FRAMEBUFFER.as_mut().unwrap() };
-        let bounds = fb.bounding_box();
-
-        let text = Text::with_alignment(
-            self.f1_label,
-            Point::new(10, bounds.size.height as i32 - 24), // bottom-left corner
-            text_style,
-            Alignment::Left,
-        );
-
-        self.last_bounds = Some(text.bounds());
-        text.draw(fb).unwrap();
-    }
-
-    async fn clear(&mut self) {
-        if let Some(rect) = self.last_bounds {
-            clear_rect(rect).await
-        }
-        self.last_bounds = None;
-    }
-}
-
 #[derive(PartialEq)]
 enum UiPage {
     Menu,
@@ -173,8 +125,8 @@ static SELECTIONS: Mutex<CriticalSectionRawMutex, Vec<FileName>> = Mutex::new(Ve
 static mut SELECTIONS_CHANGED: bool = true;
 
 async fn update_selections() {
-    while USB_ACTIVE.load(Ordering::Acquire) {
+    while SCSI_BUSY.load(Ordering::Acquire) {
-        Timer::after_millis(50).await;
+        Timer::after_millis(100).await;
     }
     let mut guard = SDCARD.get().lock().await;
     let sd = guard.as_mut().unwrap();

kernel/src/usb.rs

@@ -1,18 +1,9 @@
-use crate::scsi::{MassStorageClass, SCSI_BUSY, SCSI_HALT};
+use crate::scsi::{MassStorageClass, SCSI_EJECTED};
-use core::sync::atomic::{AtomicBool, Ordering};
+use core::sync::atomic::Ordering;
-use embassy_futures::select::select;
+use embassy_futures::join::join;
 use embassy_rp::{peripherals::USB, usb::Driver};
-use embassy_sync::{blocking_mutex::raw::CriticalSectionRawMutex, channel::Channel};
-use embassy_time::Timer;
 use embassy_usb::{Builder, Config};
 
-static START_USB: Channel<CriticalSectionRawMutex, (), 1> = Channel::new();
-static STOP_USB: Channel<CriticalSectionRawMutex, (), 1> = Channel::new();
-
-// for other tasks to query the status of usb (like ui)
-// this is read only for ALL other tasks
-pub static USB_ACTIVE: AtomicBool = AtomicBool::new(false);
-
 #[embassy_executor::task]
 pub async fn usb_handler(driver: Driver<'static, USB>) {
     let mut config = Config::new(0xc0de, 0xbabe);
@@ -38,45 +29,15 @@ pub async fn usb_handler(driver: Driver<'static, USB>) {
     let mut scsi = MassStorageClass::new(&mut builder);
     let mut usb = builder.build();
 
+    join(
+        async {
             loop {
-        START_USB.receiver().receive().await;
+                usb.wait_resume().await;
-        USB_ACTIVE.store(true, Ordering::Release);
+                SCSI_EJECTED.store(false, Ordering::Release);
-        SCSI_HALT.store(false, Ordering::Release);
+                usb.run_until_suspend().await;
-        scsi.take_sdcard().await;
-        scsi.pending_eject = false;
-
-        // waits for cancellation signal, and then waits for
-        // transfers to stop before dropping usb future
-        select(
-            async {
-                STOP_USB.receiver().receive().await;
-                SCSI_HALT.store(true, Ordering::Release);
-                while SCSI_BUSY.load(Ordering::Acquire) {
-                    Timer::after_millis(100).await;
             }
         },
-            // runs the usb, until cancelled
-            select(
-                async {
-                    let _ = usb.remote_wakeup().await;
-                    usb.run().await;
-                },
         scsi.poll(),
-            ),
     )
     .await;
-        usb.disable().await;
-        scsi.return_sdcard().await;
-        USB_ACTIVE.store(false, Ordering::Release);
-    }
-}
-
-pub fn start_usb() {
-    let _ = STOP_USB.receiver().try_receive();
-    let _ = START_USB.sender().try_send(());
-}
-
-pub fn stop_usb() {
-    let _ = START_USB.receiver().try_receive();
-    let _ = STOP_USB.sender().try_send(());
 }