diff --git a/services/usb-device-xous/src/hw.rs b/services/usb-device-xous/src/hw.rs index 43e0214b2..276e67147 100644 --- a/services/usb-device-xous/src/hw.rs +++ b/services/usb-device-xous/src/hw.rs @@ -8,7 +8,6 @@ use std::collections::BTreeMap; use std::convert::TryInto; const WRITE_TIMEOUT_MS: u64 = 10_000; -const READ_TIMEOUT_MS: u64 = 10_000; fn handle_usb(_irq_no: usize, arg: *mut usize) { let usb = unsafe { &mut *(arg as *mut SpinalUsbDevice) }; @@ -23,7 +22,7 @@ fn handle_usb(_irq_no: usize, arg: *mut usize) { } pub struct SpinalUsbMgmt { - csr: AtomicCsr, // consider using VolatileCell and/or refactory AtomicCsr so it is non-mutable + csr: AtomicCsr, // consider using VolatileCell and/or refactoring AtomicCsr so it is non-mutable usb: AtomicPtr, eps: AtomicPtr, // can't use the srmem macro because the memory region window is not exactly the size of the memory to be stored. @@ -190,10 +189,10 @@ pub struct SpinalUsbDevice { pub(crate) conn: xous::CID, usb: xous::MemoryRange, csr_addr: u32, - csr: AtomicCsr, // consider using VolatileCell and/or refactory AtomicCsr so it is non-mutable + csr: AtomicCsr, // consider using VolatileCell and/or refactoring AtomicCsr so it is non-mutable regs: SpinalUdcRegs, // 1:1 mapping of endpoint structures to offsets in the memory space for the actual ep storage - // data must be committed to this in a single write, and not composed dynamcally using this as scratch space + // data must be committed to this in a single write, and not composed dynamically using this as scratch space eps: AtomicPtr, // different views into allocation, depending on the configuration view: AllocView, @@ -204,7 +203,6 @@ pub struct SpinalUsbDevice { // last descriptor in write (IN packet; device->host) chain. Used to check if a transaction is in progress. last_wr_desc: Arc::; 16]>>, // last descriptor in read (OUT packet; host->device) chain. Used to check if a transaction is in progress. - last_rd_desc: Arc::; 16]>>, } impl SpinalUsbDevice { pub fn new(sid: xous::SID) -> SpinalUsbDevice { @@ -241,7 +239,6 @@ impl SpinalUsbDevice { address: AtomicUsize::new(0), read_allowed: AtomicU16::new(0), last_wr_desc: Arc::new(Mutex::new([None, None, None, None, None, None, None, None, None, None, None, None, None, None, None, None,])), - last_rd_desc: Arc::new(Mutex::new([None, None, None, None, None, None, None, None, None, None, None, None, None, None, None, None,])), }; xous::claim_interrupt( @@ -277,7 +274,6 @@ impl SpinalUsbDevice { address: AtomicUsize::new(0), read_allowed: AtomicU16::new(0), last_wr_desc: Arc::new(Mutex::new([None, None, None, None, None, None, None, None, None, None, None, None, None, None, None, None,])), - last_rd_desc: Arc::new(Mutex::new([None, None, None, None, None, None, None, None, None, None, None, None, None, None, None, None,])), } } pub fn get_iface(&self) -> SpinalUsbMgmt { @@ -445,73 +441,6 @@ impl SpinalUsbDevice { ); setup } -/* - pub(crate) fn read_blocking_bulk(&self, ep_addr: EndpointAddress, buf: &mut [u8]) -> Result { - // in the case that a transfer longer than a packet is expected, this call blocks until - // we have read in as much data as we can hold given our descriptor chain depth. - assert!(ep_addr.index() != 0); // long buffers are not allowed on EP0 - let mut ep_status = self.status_read_volatile(ep_addr.index()); - let mut descriptor = self.descriptor_from_head_and_chain(head_offset, 0, packet_len); - - for (chain_offset, sub_buf) in buf.chunks_mut(packet_len).enumerate() { - descriptor = self.descriptor_from_head_and_chain(head_offset, chain_offset, packet_len); - descriptor.set_desc_flags(UsbDirection::Out, - false, true, false); - for b in sub_buf.iter_mut() { - *b = 0; // zeroize the buffer between transactions, to avoid data leakage in unused areas - } - - if chain_offset == (max_chain - 1) || sub_buf.len() != packet_len { - // on the very last packet (either by chain exhaustion, or buffer source exhaustion), set completion_on_full interrupt - descriptor.set_next_desc_and_len(0, sub_buf.len()); - } else { - // if not the last packet, setup the linked list pointer - let next_addr = self.next_from_head_and_chain(head_offset, chain_offset, packet_len); - descriptor.set_next_desc_and_len(next_addr, sub_buf.len()); - } - descriptor.set_offset(0); // reset the write pointer to 0, also sets in_progress - log::info!("bulk descriptor {} setup: {:x?}", chain_offset, descriptor); - } - - ep_status.set_max_packet_size(packet_len as _); - // this is reset to 0 after every transaction by the hardware, so we must reset it - ep_status.set_head_offset(head_offset as u32); - - // this is required to commit the ep_status record once all the setup is done - self.status_write_volatile(ep_addr.index(), ep_status); - - core::sync::atomic::compiler_fence(core::sync::atomic::Ordering::SeqCst); - // zeroize the return buffer now -- gives us something to do while the data comes in! - for d in buf.iter_mut() { - *d = 0; - } - - // now busy-wait for the packet to come in! - let now = self.tt.elapsed_ms(); - let mut last_secs = 5; - while descriptor.in_progress() && (self.tt.elapsed_ms() - now < READ_TIMEOUT_MS) { - xous::yield_slice(); - let secs = (self.tt.elapsed_ms() - now) / 1_0000; - if last_secs != secs { - log::info!("waiting for rd linked list to complete..."); - last_secs = secs; - } - } - - // now copy results to buf - for (chain_offset, sub_buf) in buf.chunks_mut(packet_len).enumerate() { - let descriptor = self.descriptor_from_head_and_chain(head_offset, chain_offset, packet_len); - // buffer was zeroized as we were waiting for data to come in, so we can safely do partial copies of data here - for (desc_offset, word_chunk) in sub_buf.chunks_mut(size_of::()).enumerate() { - word_chunk.copy_from_slice( - &descriptor.read_data(desc_offset) - .to_le_bytes()[..word_chunk.len()]) // the upper limit index ensures that if sub_buf is an odd number of bytes, we don't panic - } - } - log::info!("bulk read {} bytes: {:x?}", buf.len(), buf); - Ok(buf.len()) - } -*/ } const CHAINED_BUF_LEN: usize = 512; @@ -532,7 +461,7 @@ impl UsbBus for SpinalUsbDevice { /// # Errors /// /// * [`EndpointOverflow`](crate::UsbError::EndpointOverflow) - Available total number of - /// endpoints, endpoints of the specified type, or endpoind packet memory has been exhausted. + /// endpoints, endpoints of the specified type, or endpoint packet memory has been exhausted. /// This is generally caused when a user tries to add too many classes to a composite device. /// * [`InvalidEndpoint`](crate::UsbError::InvalidEndpoint) - A specific `ep_addr` was specified /// but the endpoint in question has already been allocated. @@ -597,7 +526,7 @@ impl UsbBus for SpinalUsbDevice { ep_dir, ep_dir == UsbDirection::Out, true, // this should be equal to "packet_end", but this driver doesn't have that...? - index == 0, // only trigger for ep0 (per spinal linux driver) + index == 0, // only trigger for ep0 (per spinal Linux driver) ); // clear the descriptor to 0 let init_vec = vec![0u8; max_packet_size as _]; @@ -751,7 +680,7 @@ impl UsbBus for SpinalUsbDevice { } else if buf.len() <= packet_len { // no-chain path // TODO: merge this with the path below, mostly to reduce code space. This was kept "intact" so enumeration // would work while we figured out the details of chaining on bulk transfers. - #[cfg(feature="mjolnir")] // mjolnir is so powerful, one must halt the USB core entirely for it to be weilded + #[cfg(feature="mjolnir")] // mjolnir is so powerful, one must halt the USB core entirely for it to be wielded if ep_addr.index() == 1 { self.udc_hard_halt(ep_addr.index()); } let mut ep_status = self.status_read_volatile(ep_addr.index()); @@ -814,7 +743,7 @@ impl UsbBus for SpinalUsbDevice { self.status_write_volatile(ep_addr.index(), ep_status); #[cfg(feature="mjolnir")] - { // mjolnir is so powerful, one must halt the USB core entirely for it to be weilded + { // mjolnir is so powerful, one must halt the USB core entirely for it to be wielded if ep_addr.index() == 1 { self.ll_debug(); } @@ -1000,7 +929,7 @@ impl UsbBus for SpinalUsbDevice { for (chain_offset, sub_buf) in buf.chunks_mut(packet_len).enumerate() { let descriptor = self.descriptor_from_head_and_chain(head_offset, chain_offset, packet_len); if descriptor.offset() != 0 { - log::info!("appending {:x?}", descriptor); + log::debug!("appending {:x?}", descriptor); let available_len = sub_buf.len().min(descriptor.offset()); for (desc_offset, word_chunk) in sub_buf[..available_len].chunks_mut(size_of::()).enumerate() { word_chunk.copy_from_slice( @@ -1012,7 +941,7 @@ impl UsbBus for SpinalUsbDevice { } len += available_len; } else { - log::info!("breaking at {:x?}", descriptor); + log::debug!("breaking at {:x?}", descriptor); break; } } diff --git a/services/usb-device-xous/src/main_hw.rs b/services/usb-device-xous/src/main_hw.rs index 4bfda4d6a..e51d5aba7 100644 --- a/services/usb-device-xous/src/main_hw.rs +++ b/services/usb-device-xous/src/main_hw.rs @@ -174,22 +174,13 @@ pub(crate) fn main_hw() -> ! { let bd = block_device::BlockDevice::new(); let mut ums = usbd_scsi::Scsi::new( - &ums_alloc, - 64, - bd, - "Kosagi".as_bytes(), - "Kosagi Precursor".as_bytes(), + &ums_alloc, + 64, + bd, + "Kosagi".as_bytes(), + "Kosagi Precursor".as_bytes(), "1".as_bytes() - ); - - //let mut ums = usbd_serial::CdcAcmClass::new(&ums_alloc, 64); - - // let mut ums = usbd_mass_storage::MscClass::new( - // &ums_alloc, - // 64, - // usbd_mass_storage::InterfaceSubclass::ScsiTransparentCommandSet, - // usbd_mass_storage::InterfaceProtocol::BulkOnlyTransport - // ); + ); let mut ums_device = UsbDeviceBuilder::new(&ums_alloc, UsbVidPid(0x1209, 0x3613)) .manufacturer("Kosagi") diff --git a/services/usb-device-xous/stm32-usb.rs/firmware/usbd_bulk_only_transport/src/bulk_only_transport/bulk_only_transport.rs b/services/usb-device-xous/stm32-usb.rs/firmware/usbd_bulk_only_transport/src/bulk_only_transport/bulk_only_transport.rs index 296efd110..8db694008 100644 --- a/services/usb-device-xous/stm32-usb.rs/firmware/usbd_bulk_only_transport/src/bulk_only_transport/bulk_only_transport.rs +++ b/services/usb-device-xous/stm32-usb.rs/firmware/usbd_bulk_only_transport/src/bulk_only_transport/bulk_only_transport.rs @@ -143,7 +143,7 @@ pub struct BulkOnlyTransport<'a, B: UsbBus> { /// Used to determine if a ZLP is required or not last_packet_full: bool, - /// Indicates we are going to end the current data transaction after draining the current + /// Indicates we are going to end the current data transaction after draining the current /// buffer regardless of data_residue data_done: bool, } @@ -151,16 +151,16 @@ pub struct BulkOnlyTransport<'a, B: UsbBus> { impl BulkOnlyTransport<'_, B> { pub const BUFFER_BYTES: usize = BUFFER_BYTES; pub fn new( - alloc: &UsbBusAllocator, - max_packet_size: u16, + alloc: &UsbBusAllocator, + max_packet_size: u16, subclass: InterfaceSubclass, max_lun: u8, ) -> BulkOnlyTransport<'_, B> { assert!(max_lun < 16); BulkOnlyTransport { inner: MscClass::new( - alloc, - max_packet_size, + alloc, + max_packet_size, subclass, InterfaceProtocol::BulkOnlyTransport, ), @@ -242,7 +242,7 @@ impl BulkOnlyTransport<'_, B> { // read that initiated the command should have drained the RX buffer. Depending on if // logging is enabled or not it appears to sometimes work and sometimes not which makes // me think it's a timing issue. In the hardware example I'm testing with RTFM appears - // to be clearing the + // to be clearing the // self.read()?; } @@ -271,7 +271,7 @@ impl BulkOnlyTransport<'_, B> { // Reset the positions in the buffer self.buffer_i = 0; self.data_i = 0; - + // Reset the data_done override self.data_done = false; @@ -305,18 +305,18 @@ impl BulkOnlyTransport<'_, B> { State::SendingDataToHost | State::ReceivingDataFromHost => Some(self.command_status_wrapper.data_residue), _ => None, - } + } } pub fn transfer_state(&self) -> TransferState { trace_bot_buffer!("BUFFER> i: {}, di: {}", self.buffer_i, self.data_i); match self.state { - State::ReceivingDataFromHost => TransferState::ReceivingDataFromHost { + State::ReceivingDataFromHost => TransferState::ReceivingDataFromHost { bytes_available: self.buffer_i - self.data_i, full: self.buffer_i == self.buffer.len(), done: self.command_status_wrapper.data_residue == 0, }, - State::SendingDataToHost => TransferState::SendingDataToHost { + State::SendingDataToHost => TransferState::SendingDataToHost { bytes_remaining: self.buffer_i - self.data_i, empty: self.buffer_i == 0, }, @@ -385,7 +385,7 @@ impl BulkOnlyTransport<'_, B> { log::info!("take_buffered_data() s: {} e: {}", s, e); Ok(&self.buffer[s..e]) */ - log::info!("take_buffered_data() len: {}", len); + log::debug!("take_buffered_data() len: {}", len); loop { match self.inner.read_packet(&mut self.buffer[..len]) { Ok(l) => { @@ -487,7 +487,7 @@ impl BulkOnlyTransport<'_, B> { fn end_data_transfer(&mut self) -> Result<(), Error> { // Get the csw ready to send - log::info!("end_data_transfer() pack csw for cbw {:x}", self.command_block_wrapper.tag); + log::debug!("end_data_transfer() pack csw for cbw {:x}", self.command_block_wrapper.tag); self.pack_csw(); self.flush()?; @@ -501,7 +501,7 @@ impl BulkOnlyTransport<'_, B> { // send_zlp or flush are called here because we may not get an interrupt in a timley manner // if we don't send immediately and if needs_zlp { - /*trace_bot_zlp!*/ log::info!("ZLP> required"); + trace_bot_zlp!("ZLP> required"); self.change_state(State::NeedZlp); self.send_zlp()?; } else { diff --git a/services/usb-device-xous/stm32-usb.rs/firmware/usbd_scsi/src/lib.rs b/services/usb-device-xous/stm32-usb.rs/firmware/usbd_scsi/src/lib.rs index 017bfa5a0..6baf11b62 100644 --- a/services/usb-device-xous/stm32-usb.rs/firmware/usbd_scsi/src/lib.rs +++ b/services/usb-device-xous/stm32-usb.rs/firmware/usbd_scsi/src/lib.rs @@ -21,6 +21,6 @@ mod logging { pub use itm_logger::stub as trace_scsi_fs; */ - pub use log::info as trace_scsi_fs; + pub use log::debug as trace_scsi_fs; pub use log::info as trace_scsi_command; } diff --git a/services/usb-device-xous/stm32-usb.rs/firmware/usbd_scsi/src/scsi/scsi.rs b/services/usb-device-xous/stm32-usb.rs/firmware/usbd_scsi/src/scsi/scsi.rs index 175ff99f6..6bd08071d 100644 --- a/services/usb-device-xous/stm32-usb.rs/firmware/usbd_scsi/src/scsi/scsi.rs +++ b/services/usb-device-xous/stm32-usb.rs/firmware/usbd_scsi/src/scsi/scsi.rs @@ -118,7 +118,7 @@ impl Scsi<'_, B, BD> { fn process_command(&mut self, new_command: bool) -> Result { use CommandState::*; - /*trace_scsi_command!*/log::info!("COMMAND> {:?} tag {:x}", self.current_command, self.inner.get_tag()); + trace_scsi_command!("COMMAND> {:?} tag {:x}", self.current_command, self.inner.get_tag()); Ok(match self.current_command { // No command, nothing to do @@ -155,7 +155,7 @@ impl Scsi<'_, B, BD> { max_lba, block_size, }; - + let buf = self.inner.take_buffer_space(ReadCapacity10Response::BYTES)?; cap.pack(buf)?; Done @@ -165,13 +165,13 @@ impl Scsi<'_, B, BD> { Command::ModeSense(ModeSenseXCommand { command_length: CommandLength::C6, page_control: PageControl::CurrentValues }) => { let mut header = ModeParameterHeader6::default(); header.increase_length_for_page(PageCode::CachingModePage); - + // Default is both caches disabled let cache_page = CachingModePage::default(); let buf = self.inner.take_buffer_space( ModeParameterHeader6::BYTES + CachingModePage::BYTES - )?; + )?; header.pack(&mut buf[..ModeParameterHeader6::BYTES])?; cache_page.pack(&mut buf[ModeParameterHeader6::BYTES..])?; @@ -195,7 +195,7 @@ impl Scsi<'_, B, BD> { self.lba_end = r.lba + r.transfer_length - 1; } - /*trace_scsi_fs!*/log::info!("FS> Read; new: {}, lba: 0x{:X?}, lba_end: 0x{:X?}, done: {}, tag: {:x}", + trace_scsi_fs!("FS> Read; new: {}, lba: 0x{:X?}, lba_end: 0x{:X?}, done: {}, tag: {:x}", new_command, self.lba, self.lba_end, self.lba == self.lba_end, self.inner.get_tag()); // We only get here if the buffer is empty @@ -221,7 +221,7 @@ impl Scsi<'_, B, BD> { self.lba_end = w.lba + w.transfer_length - 1; } - /*trace_scsi_fs!*/log::info!("FS> Write; new: {}, lba: 0x{:X?}, lba_end: 0x{:X?}, done: {}, tag: {:x}", + trace_scsi_fs!("FS> Write; new: {}, lba: 0x{:X?}, lba_end: 0x{:X?}, done: {}, tag: {:x}", new_command, self.lba, self.lba_end, self.lba == self.lba_end, self.inner.get_tag()); let len = match self.inner.transfer_state() { @@ -233,7 +233,6 @@ impl Scsi<'_, B, BD> { while self.lba <= self.lba_end { // I think this "len" computation isn't right for our purposes.. let buf = self.inner.take_buffered_data(len, true).expect("Buffer should have enough data"); - log::info!("buf: {:x?}", &buf[..32]); self.block_device.write_block(self.lba, buf)?; self.lba += 1; } @@ -280,7 +279,7 @@ impl Scsi<'_, B, BD> { match self.process_command(new_command) { Ok(CommandState::Done) => { - log::info!("command done"); + log::trace!("command done"); // Command is done, send CommandOk self.inner.send_command_ok()?; // Clear the command so we don't try and execute it again @@ -290,7 +289,7 @@ impl Scsi<'_, B, BD> { self.request_sense_response.reset_status(); }, Ok(CommandState::Ongoing) => { - log::info!("transfer_state: {:?}", self.inner.transfer_state()); + log::trace!("transfer_state: {:?}", self.inner.transfer_state()); /* match self.inner.transfer_state() { TransferState::ReceivingDataFromHost { bytes_available: _, full: _, done: _ } => { @@ -335,7 +334,7 @@ impl Scsi<'_, B, BD> { SenseKey::IllegalRequest, AdditionalSenseCode::InvalidCommandOperationCode, ), - + Error::InsufficientDataForCommand => ( SenseKey::IllegalRequest, // Closest thing I could find. Some sources suggest OS does very little with ASC/ASCQ and it's @@ -428,7 +427,7 @@ impl UsbClass for Scsi<'_, B, BD> { self.inner.get_configuration_descriptors(writer) } - fn reset(&mut self) { + fn reset(&mut self) { self.current_command = Command::None; self.request_sense_response.reset_status(); self.lba = 0; @@ -445,7 +444,7 @@ impl UsbClass for Scsi<'_, B, BD> { self.inner.control_out(xfer) } - fn poll(&mut self) { + fn poll(&mut self) { if let Err(e) = self.update() { error!("Error from Scsi::update: {:?}", e); }