Implements USB enumeration workaround (RP2040-E5). (#120)

* Implement rp2040-E5 workaround for usb enumeration.

* Expand documentation and add to pico_usb_serial & pico_usb_twitchy_mouse

* Fix errata-5 documentation around the bus-keep state.

* Update CHANGELOG.md

.github/workflows/build_and_test.yml

@@ -6,7 +6,7 @@ jobs:
     runs-on: ubuntu-20.04
     strategy:
       matrix:
-        features: ["", "--features eh1_0_alpha", "--features chrono"]
+        features: ["", "--features eh1_0_alpha", "--features chrono", "--features rp2040-e5"]
         mode: ["", "--release"]
     steps:
       - uses: actions/checkout@v2

boards/rp-pico/CHANGELOG.md

@@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
-- None
+- `rp2040-e5` feature enabling the workaround for errata 5 on the USB device peripheral.
 
 ### Changed
 

boards/rp-pico/Cargo.toml

@@ -44,3 +44,4 @@ defmt-rtt = "0.3.0"
 default = ["boot2", "rt"]
 boot2 = ["rp2040-boot2"]
 rt = ["cortex-m-rt","rp2040-hal/rt"]
+rp2040-e5 = ["rp2040-hal/rp2040-e5"]

boards/rp-pico/examples/pico_usb_serial.rs

@@ -63,6 +63,17 @@ fn main() -> ! {
     .ok()
     .unwrap();
 
+    #[cfg(feature = "rp2040-e5")]
+    {
+        let sio = hal::Sio::new(pac.SIO);
+        let _pins = rp_pico::Pins::new(
+            pac.IO_BANK0,
+            pac.PADS_BANK0,
+            sio.gpio_bank0,
+            &mut pac.RESETS,
+        );
+    }
+
     // Set up the USB driver
     let usb_bus = UsbBusAllocator::new(hal::usb::UsbBus::new(
         pac.USBCTRL_REGS,

boards/rp-pico/examples/pico_usb_twitchy_mouse.rs

@@ -83,6 +83,17 @@ fn main() -> ! {
     .ok()
     .unwrap();
 
+    #[cfg(feature = "rp2040-e5")]
+    {
+        let sio = hal::Sio::new(pac.SIO);
+        let _pins = rp_pico::Pins::new(
+            pac.IO_BANK0,
+            pac.PADS_BANK0,
+            sio.gpio_bank0,
+            &mut pac.RESETS,
+        );
+    }
+
     // Set up the USB driver
     let usb_bus = UsbBusAllocator::new(hal::usb::UsbBus::new(
         pac.USBCTRL_REGS,

rp2040-hal/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- `rp2040-e5` feature enabling the workaround for errata 5 on the USB device peripheral.
+
 ### Changed
 
 - Update embedded-hal alpha support to version 1.0.0-alpha.8

rp2040-hal/Cargo.toml

@@ -45,6 +45,7 @@ rt = ["rp2040-pac/rt"]
 rom-func-cache = []
 disable-intrinsics = []
 rom-v2-intrinsics = []
+rp2040-e5 = [] # USB errata 5: USB device fails to exit RESET state on busy USB bus.
 
 [[example]]
 # irq example uses cortex-m-rt::interrupt, need rt feature for that

rp2040-hal/src/usb.rs

@@ -10,7 +10,6 @@
 //! const XOSC_CRYSTAL_FREQ: u32 = 12_000_000; // Typically found in BSP crates
 //!
 //! let mut pac = pac::Peripherals::take().unwrap();
-//! let sio = Sio::new(pac.SIO);
 //! let mut watchdog = Watchdog::new(pac.WATCHDOG);
 //! let mut clocks = init_clocks_and_plls(
 //!     XOSC_CRYSTAL_FREQ,
@@ -39,7 +38,7 @@
 //!
 //! During enumeration Windows hosts send a `StatusOut` after the `DataIn` packet of the first
 //! `Get Descriptor` resquest even if the `DataIn` isn't completed (typically when the `max_packet_size_ep0`
-//! is less than 18bytes). The next request request is a `Set Address` that expect a `StatusIn`.
+//! is less than 18bytes). The next request is a `Set Address` that expect a `StatusIn`.
 //!
 //! The issue is that by the time the previous `DataIn` packet is acknoledged and the `StatusOut`
 //! followed by `Setup` are received, the usb stack may have already prepared the next `DataIn` payload
@@ -54,6 +53,57 @@
 //!
 //! If the required timing cannot be met, using an maximum packet size of the endpoint 0 above 18bytes
 //! (e.g. `.max_packet_size_ep0(64)`) should avoid that issue.
+//!
+//! ## Issue on RP2040B0 and RP2040B1: USB device fails to exit RESET state on busy USB bus.
+//!
+//! The feature `rp2040-e5`implements the workaround described by [RP2040-E5](https://datasheets.raspberrypi.com/rp2040/rp2040-datasheet.pdf#%5B%7B%22num%22%3A630%2C%22gen%22%3A0%7D%2C%7B%22name%22%3A%22XYZ%22%7D%2C115%2C158.848%2Cnull%5D).
+//!
+//! The workaround requires the GPIO block to be released from its reset and has for side effect
+//! that GPIO15 will be stolen for a few hundred microseconds each time a Reset is detected on the
+//! USB bus.
+//!
+//! The pin will be temporarily put in "bus keep" mode, weakly pulling the output towards its current
+//! logic level. In absence of external loads, the current logic level will be maintained.
+//! A user will lose control of the pin's output and reading from it may not reflect the actual state
+//! of the external pin.
+//!
+//! ```no_run
+//! # use rp2040_hal::{clocks::init_clocks_and_plls, pac, usb::UsbBus, watchdog::Watchdog};
+//! # use usb_device::class_prelude::UsbBusAllocator;
+//! use rp2040_hal::{gpio::Pins, Sio};
+//!
+//! # const XOSC_CRYSTAL_FREQ: u32 = 12_000_000; // Typically found in BSP crates
+//! #
+//! # let mut pac = pac::Peripherals::take().unwrap();
+//! # let mut watchdog = Watchdog::new(pac.WATCHDOG);
+//! # let mut clocks = init_clocks_and_plls(
+//! #     XOSC_CRYSTAL_FREQ,
+//! #     pac.XOSC,
+//! #     pac.CLOCKS,
+//! #     pac.PLL_SYS,
+//! #     pac.PLL_USB,
+//! #     &mut pac.RESETS,
+//! #     &mut watchdog
+//! # ).ok().unwrap();
+//! #
+//! // required for the errata 5's workaround to function properly.
+//! let sio = Sio::new(pac.SIO);
+//! let _pins = Pins::new(
+//!     pac.IO_BANK0,
+//!     pac.PADS_BANK0,
+//!     sio.gpio_bank0,
+//!     &mut pac.RESETS,
+//! );
+//! #
+//! # let usb_bus = UsbBusAllocator::new(UsbBus::new(
+//! #         pac.USBCTRL_REGS,
+//! #         pac.USBCTRL_DPRAM,
+//! #         clocks.usb_clock,
+//! #         true,
+//! #         &mut pac.RESETS,
+//! #     ));
+//! # // Use the usb_bus as usual.
+//! ```
 
 use core::cell::RefCell;
 
@@ -71,6 +121,9 @@ use usb_device::{
     Result as UsbResult, UsbDirection, UsbError,
 };
 
+#[cfg(feature = "rp2040-e5")]
+mod errata5;
+
 fn ep_addr_to_ep_buf_ctrl_idx(ep_addr: EndpointAddress) -> usize {
     ep_addr.index() * 2 + (if ep_addr.is_in() { 0 } else { 1 })
 }
@@ -114,6 +167,8 @@ struct Inner {
     out_endpoints: [Option<Endpoint>; 16],
     next_offset: u16,
     read_setup: bool,
+    #[cfg(feature = "rp2040-e5")]
+    errata5_state: Option<errata5::Errata5State>,
 }
 impl Inner {
     fn new(ctrl_reg: USBCTRL_REGS, ctrl_dpram: USBCTRL_DPRAM) -> Self {
@@ -124,6 +179,8 @@ impl Inner {
             out_endpoints: Default::default(),
             next_offset: 0,
             read_setup: false,
+            #[cfg(feature = "rp2040-e5")]
+            errata5_state: None,
         }
     }
 
@@ -544,43 +601,68 @@ impl UsbBusTrait for UsbBus {
         interrupt::free(|cs| {
             let mut inner = self.inner.borrow(cs).borrow_mut();
 
-            // check for bus reset
+            #[cfg(feature = "rp2040-e5")]
+            if let Some(state) = inner.errata5_state.take() {
+                unsafe {
+                    inner.errata5_state = state.update();
+                }
+                return if inner.errata5_state.is_some() {
+                    PollResult::None
+                } else {
+                    PollResult::Reset
+                };
+            }
+
+            // check for bus reset and/or suspended states.
             let sie_status = inner.ctrl_reg.sie_status.read();
+            let mut buff_status = inner.ctrl_reg.buff_status.read().bits();
+
             if sie_status.bus_reset().bit_is_set() {
+                #[cfg(feature = "rp2040-e5")]
+                if sie_status.connected().bit_is_clear() {
+                    inner.errata5_state = Some(errata5::Errata5State::start());
+                    return PollResult::None;
+                } else {
+                    return PollResult::Reset;
+                }
+
+                #[cfg(not(feature = "rp2040-e5"))]
                 return PollResult::Reset;
-            } else if sie_status.suspended().bit_is_set() {
+            } else if buff_status == 0 && sie_status.setup_rec().bit_is_clear() {
-                inner.ctrl_reg.sie_status.write(|w| w.suspended().set_bit());
+                if sie_status.suspended().bit_is_set() {
-                return PollResult::Suspend;
+                    inner.ctrl_reg.sie_status.write(|w| w.suspended().set_bit());
-            } else if sie_status.resume().bit_is_set() {
+                    return PollResult::Suspend;
-                inner.ctrl_reg.sie_status.write(|w| w.resume().set_bit());
+                } else if sie_status.resume().bit_is_set() {
-                return PollResult::Resume;
+                    inner.ctrl_reg.sie_status.write(|w| w.resume().set_bit());
+                    return PollResult::Resume;
+                }
+                return PollResult::None;
             }
 
             let (mut ep_out, mut ep_in_complete, mut ep_setup): (u16, u16, u16) = (0, 0, 0);
 
-            let buff_status = inner.ctrl_reg.buff_status.read().bits();
+            // IN Complete shall only be reported once.
-            if buff_status != 0 {
+            inner
-                // IN Complete shall only be reported once.
+                .ctrl_reg
-                inner
+                .buff_status
-                    .ctrl_reg
+                .write(|w| unsafe { w.bits(0x5555_5555) });
-                    .buff_status
-                    .write(|w| unsafe { w.bits(0x5555_5555) });
 
-                for i in 0..32u32 {
+            for i in 0..32u32 {
-                    let mask = 1 << i;
+                if buff_status == 0 {
-                    if (buff_status & mask) == mask {
+                    break;
-                        let is_in = (i & 1) == 0;
+                } else if (buff_status & 1) == 1 {
-                        let ep_idx = i / 2;
+                    let is_in = (i & 1) == 0;
-                        if is_in {
+                    let ep_idx = i / 2;
-                            ep_in_complete |= 1 << ep_idx;
+                    if is_in {
-                        } else {
+                        ep_in_complete |= 1 << ep_idx;
-                            ep_out |= 1 << ep_idx;
+                    } else {
-                        }
+                        ep_out |= 1 << ep_idx;
                     }
                 }
+                buff_status >>= 1;
             }
+
             // check for setup request
-            // Only report setup if OUT has been cleared.
             if sie_status.setup_rec().bit_is_set() {
                 // Small max_packet_size_ep0 Work-Around
                 inner.ctrl_dpram.ep_buffer_control[0].modify(|_, w| w.available_0().clear_bit());
@@ -589,9 +671,6 @@ impl UsbBusTrait for UsbBus {
                 inner.read_setup = true;
             }
 
-            if let (0, 0, 0) = (ep_out, ep_in_complete, ep_setup) {
-                return PollResult::None;
-            }
             PollResult::Data {
                 ep_out,
                 ep_in_complete,

rp2040-hal/src/usb/errata5.rs

@@ -0,0 +1,150 @@
+//! After coming out of reset, the hardware expects 800us of LS_J (linestate J) time
+//! before it will move to the connected state. However on a hub that broadcasts packets
+//! for other devices this isn't the case. The plan here is to wait for the end of the bus
+//! reset, force an LS_J for 1ms and then switch control back to the USB phy. Unfortunately
+//! this requires us to use GPIO15 as there is no other way to force the input path.
+//! We only need to force DP as DM can be left at zero. It will be gated off by GPIO
+//! logic if it isn't func selected.
+
+use rp2040_pac::Peripherals;
+
+pub struct ForceLineStateJ {
+    prev_pads: u32,
+    prev_io_ctrls: u32,
+}
+pub enum Errata5State {
+    WaitEndOfReset,
+    ForceLineStateJ(ForceLineStateJ),
+}
+
+impl Errata5State {
+    pub fn start() -> Self {
+        Self::WaitEndOfReset
+    }
+    /// SAFETY: This method steals the peripherals.
+    /// It makes read only use of TIMER and read/write access to USBCTRL_REGS.
+    /// Both peripherals must be initialized & running.
+    pub unsafe fn update(self) -> Option<Self> {
+        let pac = crate::pac::Peripherals::steal();
+        match self {
+            Self::WaitEndOfReset => {
+                if pac.USBCTRL_REGS.sie_status.read().line_state().is_se0() {
+                    Some(self)
+                } else {
+                    let reset_state = pac.RESETS.reset.read();
+                    assert!(
+                        reset_state.io_bank0().bit_is_clear()
+                            && reset_state.pads_bank0().bit_is_clear(),
+                        "IO Bank 0 must be out of reset for this work around to function properly."
+                    );
+                    Some(Self::ForceLineStateJ(start_force_j(&pac)))
+                }
+            }
+            Self::ForceLineStateJ(ref state) => {
+                if pac
+                    .USBCTRL_REGS
+                    .sie_status
+                    .read()
+                    .connected()
+                    .bit_is_clear()
+                {
+                    Some(self)
+                } else {
+                    finish(&pac, state.prev_pads, state.prev_io_ctrls);
+                    None
+                }
+            }
+        }
+    }
+}
+
+fn start_force_j(pac: &Peripherals) -> ForceLineStateJ {
+    let pads = &pac.PADS_BANK0.gpio[15];
+    let io = &pac.IO_BANK0.gpio[15];
+    let usb_ctrl = &pac.USBCTRL_REGS;
+
+    assert!(!usb_ctrl.sie_status.read().line_state().is_se0());
+    assert!(
+        pac.IO_BANK0.gpio[16].gpio_ctrl.read().funcsel().bits() != 8,
+        "Not expecting DM to be function 8"
+    );
+
+    // backup io ctrl & pad ctrl
+    let prev_pads = pads.read().bits();
+    let prev_io_ctrls = io.gpio_ctrl.read().bits();
+
+    // Enable bus keep and force pin to tristate, so USB DP muxing doesn't affect
+    // pin state
+    pads.modify(|_, w| w.pue().set_bit().pde().set_bit());
+    io.gpio_ctrl.modify(|_, w| w.oeover().disable());
+
+    // Select function 8 (USB debug muxing) without disturbing other controls
+    io.gpio_ctrl.modify(|_, w| unsafe { w.funcsel().bits(8) });
+
+    // J state is a differential 1 for a full speed device so
+    // DP = 1 and DM = 0. Don't actually need to set DM low as it
+    // is already gated assuming it isn't funcseld.
+    io.gpio_ctrl.modify(|_, w| w.inover().high());
+
+    // Force PHY pull up to stay before switching away from the phy
+    //usb_ctrl
+    //    .usbphy_direct
+    //    .modify(|_, w| w.dp_pullup_en().set_bit());
+    //usb_ctrl
+    //    .usbphy_direct_override
+    //    .modify(|_, w| w.dp_pullup_en_override_en().set_bit());
+    // Use the "SET" alias region to only write rather than "read/modify/write"
+    unsafe {
+        let ctrl_regs_set_alias: &pac::usbctrl_regs::RegisterBlock =
+            &*(((pac::USBCTRL_REGS::ptr() as usize) | (2 << 12)) as *const _);
+        ctrl_regs_set_alias
+            .usbphy_direct
+            .write_with_zero(|w| w.dp_pullup_en().set_bit());
+        ctrl_regs_set_alias
+            .usbphy_direct_override
+            .write_with_zero(|w| w.dp_pullup_en_override_en().set_bit());
+    }
+
+    // Switch to GPIO phy with LS_J forced
+    unsafe {
+        usb_ctrl
+            .usb_muxing
+            .write_with_zero(|w| w.to_digital_pad().set_bit().softcon().set_bit());
+    }
+
+    // LS_J is now forced, wait until the signal propagates through the usb logic.
+    loop {
+        let status = usb_ctrl.sie_status.read();
+        if status.line_state().is_j() {
+            break;
+        }
+    }
+
+    ForceLineStateJ {
+        prev_pads,
+        prev_io_ctrls,
+    }
+}
+fn finish(pac: &Peripherals, prev_pads: u32, prev_io_ctrls: u32) {
+    let pads = &pac.PADS_BANK0.gpio[15];
+    let io = &pac.IO_BANK0.gpio[15];
+
+    // Switch back to USB phy
+    pac.USBCTRL_REGS
+        .usb_muxing
+        .write(|w| w.to_phy().set_bit().softcon().set_bit());
+
+    // Get rid of DP pullup override
+    unsafe {
+        let ctrl_regs_clear_alias: &pac::usbctrl_regs::RegisterBlock =
+            &*(((pac::USBCTRL_REGS::ptr() as usize) | (3 << 12)) as *const _);
+        ctrl_regs_clear_alias
+            .usbphy_direct_override
+            .write_with_zero(|w| w.dp_pullup_en_override_en().set_bit());
+
+        // Finally, restore the gpio ctrl value back to GPIO15
+        io.gpio_ctrl.write(|w| w.bits(prev_io_ctrls));
+        // Restore the pad ctrl value
+        pads.write(|w| w.bits(prev_pads));
+    }
+}