Move post-address device config to Device impl.

3 files changed, 285 insertions, 195 deletions

diff --git a/usbh/src/device.rs b/usbh/src/device.rs
index 3bb0304..ac48c14 100644
--- a/usbh/src/device.rs
+++ b/usbh/src/device.rs
@@ -1,20 +1,56 @@
+use super::pipe::{DataBuf, PipeErr, PipeTable};
+use super::usbproto::*;
+
 use core::convert::TryInto;
+use log::{debug, error, trace};
+
+// FIXME: once again, this doesn't belong here. The issue is that
+// we're using `pipe_for`, which requires it.
+use atsamd_hal::target_device::usb;
 
 const MAX_DEVICES: usize = 16;
 const MAX_ENDPOINTS: usize = 8;
 
-pub struct DeviceTable {
+// How long to wait before talking to the device again after setting
+// its address. cf §9.2.6.3 of USB 2.0
+const SETTLE_DELAY: usize = 300;
+
+#[derive(Copy, Clone, Debug, PartialEq)]
+pub(crate) enum Error {
+    PipeErr(PipeErr),
+}
+impl From<PipeErr> for Error {
+    fn from(e: PipeErr) -> Self {
+        Self::PipeErr(e)
+    }
+}
+
+enum FSM {
+    AddressSet,
+    WaitForSettle(usize),
+    GetConfigDescriptor,
+    SetConfig,
+    Steady,
+}
+
+pub(crate) struct DeviceTable {
     devices: [Option<Device>; MAX_DEVICES],
 }
+// TODO: Untie device address from table index. Right now it's wasting
+// a table slot because addr 0 isn't used here. And rather than just
+// putting in an offset, which can be forgotten, it's better to let
+// something else handle address assignment.
 impl DeviceTable {
-    pub fn new() -> Self {
+    pub(crate) fn new() -> Self {
+        // TODO: should we create addr 0 here for use during
+        // enumeration? Probably.
         Self {
             devices: Default::default(),
         }
     }
 
     /// Return the device at address `addr`.
-    pub fn device_for(&mut self, addr: u8) -> Option<&mut Device> {
+    pub(crate) fn device_for(&mut self, addr: u8) -> Option<&mut Device> {
         if let Some(ref mut d) = self.devices[addr as usize] {
             Some(d)
         } else {
@@ -23,12 +59,13 @@ impl DeviceTable {
     }
 
     /// Allocate a device with the next available address.
-    pub fn next(&mut self) -> Option<&mut Device> {
-        for i in 0..self.devices.len() {
+    // TODO: get rid of the millis argument somehow, but the device
+    // does need a way of tracking time for Settle reasons.
+    pub(crate) fn next(&mut self, millis: &'static dyn Fn() -> usize) -> Option<&mut Device> {
+        for i in 1..self.devices.len() {
             if self.devices[i].is_none() {
                 let a = i.try_into().unwrap();
-                let mut d: Device = Default::default();
-                d.addr = a;
+                let d = Device::new(a, millis);
                 self.devices[i] = Some(d);
                 return self.device_for(a);
             }
@@ -37,24 +74,144 @@ impl DeviceTable {
     }
 
     /// Remove the device at address `addr`.
-    pub fn remove(&mut self, addr: u8) -> Option<Device> {
+    pub(crate) fn remove(&mut self, addr: u8) -> Option<Device> {
         let v = core::mem::replace(&mut self.devices[addr as usize], None);
         v
     }
+
+    pub(crate) fn run(&mut self, pipe_table: &mut PipeTable, host: &mut usb::HOST) {
+        for i in 1..self.devices.len() {
+            // TODO: Woof, this is ugly, but I'm not sure of a better
+            // way to avoid mutably borrowing self twice.
+            let mut remove_addr: Option<u8> = None;
+            if let Some(ref mut d) = self.devices[i] {
+                if let Err(e) = d.fsm(pipe_table, host) {
+                    error!("Removing device {}: {:?}", d.addr, e);
+                    remove_addr = Some(d.addr);
+                } else {
+                    remove_addr = None;
+                }
+            }
+
+            if let Some(addr) = remove_addr {
+                self.remove(addr);
+            }
+        }
+    }
 }
 
 pub struct Device {
-    addr: u8,
-    max_packet_size: u8,
-    endpoints: [Endpoint; MAX_ENDPOINTS],
+    pub addr: u8,
+    pub max_packet_size: u8,
+    pub endpoints: [Option<Endpoint>; MAX_ENDPOINTS],
+
+    state: FSM,
+    millis: &'static dyn Fn() -> usize,
 }
-impl Default for Device {
-    fn default() -> Self {
+impl Device {
+    fn new(addr: u8, millis: &'static dyn Fn() -> usize) -> Self {
         Self {
-            addr: 0,
+            addr: addr,
             max_packet_size: 8,
             endpoints: Default::default(),
+
+            state: FSM::AddressSet,
+
+            // TODO: This doesn't belong here. Ideally the current
+            // time is passed in to the FSM routine.
+            millis: millis,
+        }
+    }
+}
+
+impl Device {
+    pub(crate) fn fsm(
+        &mut self,
+        pipe_table: &mut PipeTable,
+        host: &mut usb::HOST,
+    ) -> Result<(), Error> {
+        match self.state {
+            FSM::AddressSet => self.state = FSM::WaitForSettle((self.millis)() + SETTLE_DELAY),
+
+            FSM::WaitForSettle(until) => {
+                if (self.millis)() >= until {
+                    // Dunno why we get the device descriptor a second time.
+                    let mut pipe = pipe_table.pipe_for(host, self.addr, 0);
+
+                    let mut vol_descr =
+                        ::vcell::VolatileCell::<USBDeviceDescriptor>::new(Default::default());
+                    pipe.control_req(
+                        BMRequestType::get_descr(),
+                        USBRequest::GetDescriptor,
+                        WValue::from((0, USBDescriptor::Device as u8)),
+                        0,
+                        Some(DataBuf::from(&mut vol_descr)),
+                        self.millis,
+                    )?;
+
+                    let desc = vol_descr.get();
+                    trace!(" -- devDesc: {:?}", desc);
+
+                    self.state = FSM::GetConfigDescriptor
+                }
+            }
+
+            FSM::GetConfigDescriptor => {
+                // Get config descriptor with minimal data, to see how much we need to allocate for the full descriptor.
+                let mut pipe = pipe_table.pipe_for(host, self.addr, 0);
+
+                let mut vol_descr =
+                    ::vcell::VolatileCell::<USBConfigurationDescriptor>::new(Default::default());
+                pipe.control_req(
+                    BMRequestType::get_descr(),
+                    USBRequest::GetDescriptor,
+                    WValue::from((0, USBDescriptor::Configuration as u8)),
+                    0,
+                    Some(DataBuf::from(&mut vol_descr)),
+                    self.millis,
+                )?;
+                let desc = vol_descr.get();
+                debug!("config: {:?}", desc);
+
+                // TODO: do real allocation later.
+                assert!(desc.w_total_length < 64);
+                let buf: [u8; 64] = [0; 64];
+                let mut tmp = &buf[..desc.w_total_length as usize];
+                pipe.control_req(
+                    BMRequestType::get_descr(),
+                    USBRequest::GetDescriptor,
+                    WValue::from((0, USBDescriptor::Configuration as u8)),
+                    0,
+                    Some(DataBuf::from(&mut tmp)),
+                    self.millis,
+                )?;
+
+                self.state = FSM::SetConfig
+            }
+
+            FSM::SetConfig => {
+                let mut pipe = pipe_table.pipe_for(host, self.addr, 0);
+
+                debug!("+++ setting configuration");
+                let conf: u8 = 1;
+                pipe.control_req(
+                    BMRequestType::set(),
+                    USBRequest::SetConfiguration,
+                    WValue::from((conf, 0)),
+                    0,
+                    None,
+                    self.millis,
+                )?;
+                debug!(" -- configuration set");
+
+                self.state = FSM::Steady
+            }
+
+            FSM::Steady => {
+                // Now we should be able to access it normally.
+            }
         }
+        Ok(())
     }
 }
 
diff --git a/usbh/src/lib.rs b/usbh/src/lib.rs
index 63b5581..38d1327 100755
--- a/usbh/src/lib.rs
+++ b/usbh/src/lib.rs
@@ -6,7 +6,7 @@ mod pipe;
 mod usbproto;
 
 use device::DeviceTable;
-use pipe::{DataBuf, PipeErr, PipeTable, USBPipeType, USBToken};
+use pipe::{DataBuf, PipeErr, PipeTable, USBPipeType};
 use rb::{Reader, RingBuffer, Writer};
 use usbproto::*;
 
@@ -265,7 +265,6 @@ where
             // };
         }
 
-        self.poll_devices();
         self.fsm();
 
         unsafe {
@@ -342,8 +341,7 @@ where
     fn steady_fsm(&mut self, s: SteadyState) {
         match s {
             SteadyState::Configuring => {
-                let low_speed = 0;
-                self.task_state = match self.configure_dev(0, 0, low_speed) {
+                self.task_state = match self.configure_dev() {
                     Ok(_) => TaskState::Steady(SteadyState::Running),
                     Err(e) => {
                         warn!("Enumeration error: {:?}", e);
@@ -352,198 +350,49 @@ where
                 }
             }
 
-            SteadyState::Running => {}
+            SteadyState::Running => {
+                self.devices.run(&mut self.pipe_table, self.usb.host_mut());
+            }
 
             SteadyState::Error => {}
         }
     }
 
-    fn configure_dev(&mut self, _parent: u32, _port: u32, _low_speed: u32) -> Result<(), PipeErr> {
+    fn configure_dev(&mut self) -> Result<(), PipeErr> {
+        let mut pipe = self.pipe_table.pipe_for(self.usb.host_mut(), 0, 0);
         let mut vol_descr = ::vcell::VolatileCell::<USBDeviceDescriptor>::new(Default::default());
-        self.control_req(
-            0,
-            0,
+        pipe.control_req(
             BMRequestType::get_descr(),
             USBRequest::GetDescriptor,
             WValue::from((0, USBDescriptor::Device as u8)),
             0,
             Some(DataBuf::from(&mut vol_descr)),
+            self.millis,
         )?;
 
         let desc = vol_descr.get();
         trace!(" -- devDesc: {:?}", desc);
 
-        // Assign address to this device and:
-        //  - Stash bMaxPacketSize
-        // Then SET_ADDRESS(newAddr)
-        let new_address: u8 = 5;
-        debug!("Setting address to {}.", new_address);
-        self.control_req(
-            0,
-            0,
-            BMRequestType::set(),
-            USBRequest::SetAddress,
-            WValue::from((new_address, 0)),
-            0,
-            None,
-        )?;
-        // Delay according to §9.2.6.3 of USB 2.0
-        let until = (self.millis)() + 300;
-        while (self.millis)() < until {}
-
-        // Everything after this point is what linux does. So do that?
-
-        // Dunno why we get the device descriptor a second time.
-        trace!(
-            " -- getting devDesc2 from {}:{}: {:?}",
-            new_address,
-            0,
-            desc
-        );
-        let tmp: USBDeviceDescriptor = Default::default();
-        let mut vol_descr = ::vcell::VolatileCell::new(tmp);
-        self.control_req(
-            new_address,
-            0,
-            BMRequestType::get_descr(),
-            USBRequest::GetDescriptor,
-            WValue::from((0, USBDescriptor::Device as u8)),
-            0,
-            Some(DataBuf::from(&mut vol_descr)),
-        )?;
-
-        let desc = vol_descr.get();
-        trace!(" -- devDesc2: {:?}", desc);
-
-        // Get config descriptor with minimal data, to see how much we need to allocate for the full descriptor.
-        let mut vol_descr =
-            ::vcell::VolatileCell::<USBConfigurationDescriptor>::new(Default::default());
-        self.control_req(
-            new_address,
-            0,
-            BMRequestType::get_descr(),
-            USBRequest::GetDescriptor,
-            WValue::from((0, USBDescriptor::Configuration as u8)),
-            0,
-            Some(DataBuf::from(&mut vol_descr)),
-        )?;
-        let desc = vol_descr.get();
-        debug!("config: {:?}", desc);
-
-        // TODO: do real allocation later.
-        assert!(desc.w_total_length < 64);
-        let buf: [u8; 64] = [0; 64];
-        let mut tmp = &buf[..desc.w_total_length as usize];
-        self.control_req(
-            new_address,
-            0,
-            BMRequestType::get_descr(),
-            USBRequest::GetDescriptor,
-            WValue::from((0, USBDescriptor::Configuration as u8)),
-            0,
-            Some(DataBuf::from(&mut tmp)),
-        )?;
-
-        // Once addressed, SET_CONFIGURATION(0)
-        debug!("+++ setting configuration");
-        let conf: u8 = 1;
-        self.control_req(
-            new_address,
-            0,
-            BMRequestType::set(),
-            USBRequest::SetConfiguration,
-            WValue::from((conf, 0)),
-            0,
-            None,
-        )?;
-        debug!(" -- configuration set");
-
-        // Now we should be able to access it normally.
-
-        Ok(())
-    }
-
-    fn control_req(
-        &mut self,
-        addr: u8,
-        ep: u8,
-        bm_request_type: BMRequestType,
-        b_request: USBRequest,
-        w_value: WValue,
-        w_index: u16,
-        buf: Option<DataBuf>,
-    ) -> Result<(), PipeErr> {
-        if let Some(ref b) = buf {
-            assert!(b.ptr as usize & 0x3 == 0);
-            assert!(b.len <= 65_535);
-        }
-
-        // Pipe data toggles for control pipes defined in SAMD21 data
-        // sheet §32.6.3.9
-
-        /*
-         * Setup stage.
-         */
-        let mut setup_packet = USBSetupPacket {
-            bm_request_type: bm_request_type,
-            b_request: b_request,
-            w_value: w_value,
-            w_index: w_index,
-            w_length: match buf {
-                None => 0,
-                Some(ref b) => b.len as u16,
-            },
-        };
-        let mut pipe = self.pipe_table.pipe_for(self.usb.host_mut(), addr, ep);
-        pipe.dtgl_clear();
-        pipe.send(
-            USBToken::Setup,
-            &DataBuf::from(&mut setup_packet),
-            NAK_LIMIT,
-            self.millis,
-        )?;
-
-        /*
-         * Data stage.
-         */
-        pipe.dtgl_set();
-        if let Some(b) = buf {
-            match bm_request_type.direction() {
-                USBSetupDirection::DeviceToHost => {
-                    trace!("buf0: {:?}", &b);
-                    pipe.in_transfer(&b, NAK_LIMIT, self.millis)?;
-                    trace!("buf1: {:?}", &b);
-                }
-
-                USBSetupDirection::HostToDevice => {
-                    debug!("Should OUT for {}b", b.len);
-                }
+        match self.devices.next(self.millis) {
+            // TODO: new error for being out of devices.
+            None => return Err(PipeErr::Other),
+            Some(device) => {
+                device.max_packet_size = desc.b_max_packet_size;
+                debug!("Setting address to {}.", device.addr);
+                pipe.control_req(
+                    BMRequestType::set(),
+                    USBRequest::SetAddress,
+                    WValue::from((device.addr, 0)),
+                    0,
+                    None,
+                    self.millis,
+                )?;
+
+                // Now that the device is addressed, `Device` can handle the
+                // rest of the setup in its FSM.
+                Ok(())
             }
         }
-
-        /*
-         * Status stage.
-         */
-        pipe.dtgl_set();
-        pipe.desc.bank0.pcksize.write(|w| {
-            unsafe { w.byte_count().bits(0) };
-            unsafe { w.multi_packet_size().bits(0) }
-        });
-
-        // PSTATUSSET.DTGL set -- TODO: figure out if this is
-        // necessary.
-        pipe.regs.statusset.write(|w| w.dtgl().set_bit());
-
-        let token = match bm_request_type.direction() {
-            USBSetupDirection::DeviceToHost => USBToken::Out,
-            USBSetupDirection::HostToDevice => USBToken::In,
-        };
-
-        // TODO: should probably make `pipe.send` have optional
-        // `DataBuf`, rather than exposing `dispatch_retries`.
-        trace!("dispatching status stage");
-        pipe.dispatch_retries(token, NAK_LIMIT, self.millis)?;
-        Ok(())
     }
 
     // Set up a default pipe for the control endpoint 0 on pipe 0.
diff --git a/usbh/src/pipe.rs b/usbh/src/pipe.rs
index af2bc33..be4ca57 100644
--- a/usbh/src/pipe.rs
+++ b/usbh/src/pipe.rs
@@ -12,13 +12,14 @@ use pck_size::PckSize;
 use status_bk::StatusBk;
 use status_pipe::StatusPipe;
 
-use core::convert::TryInto;
-use log::trace;
+use super::usbproto::*;
 
 use atsamd_hal::target_device::usb::{
     self,
     host::{BINTERVAL, PCFG, PINTFLAG, PSTATUS, PSTATUSCLR, PSTATUSSET},
 };
+use core::convert::TryInto;
+use log::{debug, trace};
 
 // TODO: verify this timeout against §9.2.6.1 of USB 2.0 spec.
 const USB_TIMEOUT: usize = 5 * 1024; // 5 Seconds
@@ -26,6 +27,8 @@ const USB_TIMEOUT: usize = 5 * 1024; // 5 Seconds
 // samd21 only supports 8 pipes.
 const MAX_PIPES: usize = 8;
 
+const NAK_LIMIT: usize = 15;
+
 #[derive(Copy, Clone, Debug, PartialEq)]
 pub(crate) enum PipeErr {
     ShortPacket,
@@ -86,7 +89,88 @@ pub(crate) struct Pipe<'a, 'b> {
     pub(crate) desc: &'a mut PipeDesc,
 }
 impl Pipe<'_, '_> {
-    pub(crate) fn send(
+    pub(crate) fn control_req(
+        &mut self,
+        bm_request_type: BMRequestType,
+        b_request: USBRequest,
+        w_value: WValue,
+        w_index: u16,
+        buf: Option<DataBuf>,
+        millis: &dyn Fn() -> usize,
+    ) -> Result<(), PipeErr> {
+        if let Some(ref b) = buf {
+            assert!(b.ptr as usize & 0x3 == 0);
+            assert!(b.len <= 65_535);
+        }
+
+        // Pipe data toggles for control pipes defined in SAMD21 data
+        // sheet §32.6.3.9
+
+        /*
+         * Setup stage.
+         */
+        let mut setup_packet = USBSetupPacket {
+            bm_request_type: bm_request_type,
+            b_request: b_request,
+            w_value: w_value,
+            w_index: w_index,
+            w_length: match buf {
+                None => 0,
+                Some(ref b) => b.len as u16,
+            },
+        };
+        self.dtgl_clear();
+        self.send(
+            USBToken::Setup,
+            &DataBuf::from(&mut setup_packet),
+            NAK_LIMIT,
+            millis,
+        )?;
+
+        /*
+         * Data stage.
+         */
+        self.dtgl_set();
+        if let Some(b) = buf {
+            match bm_request_type.direction() {
+                USBSetupDirection::DeviceToHost => {
+                    trace!("buf0: {:?}", &b);
+                    self.in_transfer(&b, NAK_LIMIT, millis)?;
+                    trace!("buf1: {:?}", &b);
+                }
+
+                USBSetupDirection::HostToDevice => {
+                    debug!("Should OUT for {}b", b.len);
+                }
+            }
+        }
+
+        /*
+         * Status stage.
+         */
+        self.dtgl_set();
+        self.desc.bank0.pcksize.write(|w| {
+            unsafe { w.byte_count().bits(0) };
+            unsafe { w.multi_packet_size().bits(0) }
+        });
+
+        // PSTATUSSET.DTGL set -- TODO: figure out if this is
+        // necessary.
+        self.regs.statusset.write(|w| w.dtgl().set_bit());
+
+        let token = match bm_request_type.direction() {
+            USBSetupDirection::DeviceToHost => USBToken::Out,
+            USBSetupDirection::HostToDevice => USBToken::In,
+        };
+
+        // TODO: should probably make `pipe.send` have optional
+        // `DataBuf`, rather than exposing `dispatch_retries`.
+        trace!("dispatching status stage");
+        self.dispatch_retries(token, NAK_LIMIT, millis)?;
+        Ok(())
+    }
+
+    fn send(
         &mut self,
         token: USBToken,
         buf: &DataBuf,