Add usb-device support to atmega32u4 about avr-hal

[agausmann]

I almost have a working implementation; still some features missing and plenty of cleanup before it's ready to submit, but we're close!

https://github.com/agausmann/rust-avr-usb

I've gotten usb-device to send device & configuration descriptors; now I just need a way to reconcile usb-device's out-of-order endpoint allocation (ep0 last) with the ATmega's requirement that endpoints be allocated in-order. I'll probably implement a table sort of like what LUFA has but more tailored to internal use.

Another thing on my to-do list is attach/detach detection for self-powered devices; just waiting on hardware to arrive to test that.

[haata]

I'm working on a usb-device implementation for atsam4 (https://github.com/atsam-rs/atsam4-hal). This hardware is a lot closer to AVR than the existing usb-device implementations. Hoping to have some code pushed in the next week or two.

While I don't have time to specifically work on avr rust things I am quite familiar the USB devices (https://github.com/kiibohd/controller/) and I don't mind giving advice and helping work through issues.

[agausmann]

I've done a bit of cleanup and actually refactored it into two separate projects:

• atmega-usbd - a library crate that houses the UsbBus implementation. This is where most development/feedback should take place (e.g. bug reports & patches)

• rust-avr-usb still lives, but it has been refactored to use atmega-usbd and is sort of an example usage.

usbd-hid now works - sort of. It is recognized by Linux as an HID device but it seems to have trouble sending reports.

[Rahix]

Yes, definitely! Though I don't have much experience with low-level USB so it'll take some time ... If anyone else wants to take a stab at it, please go for it!

[agausmann]

@TheButlah Yes, here are the relevant issues that I know of - these fixes are necessary to be able to use the usb-device crate on AVR:

• rust-lang/rust#79889
  • Fixed in upstream LLVM - D87631 - not yet merged into the rust fork
• rust-lang/rust#82104
  • Latest proposed patch to LLVM (not yet merged) - D114611

[Rahix]

FWIW, with the latest working compiler version (2021-01-07), even before hitting the trait object problems, we're hitting a miscompilation related to enums. I have not yet tracked it down entirely, but this also blocks integration of usb-device for the time being.

[agausmann]

I'm not quite sure about publishing yet. You can always add it as a git dependency:

atmega-usbd = { git = "https://github.com/agausmann/atmega-usbd.git" }

[agausmann]

Just want to put out a PSA - I haven't worked with atmega+USB in a while, and I don't think I can spend much time on it for the foreseeable future.

I'll still try to keep up with basic maintenance of my repositories, but it's difficult for me to allocate time to review and test PRs. Long-term, I think someone else should take over, if there are any development milestones that you want to meet (like publishing on crates.io, or avr-hal integration).

@Rahix If you're willing, can I add you as a collaborator?

[Rahix]

I think you misunderstood how .write() works. write() will overwrite the entire register, setting all fields you didn't set explicitly to their reset value. So in

dp.PLL.pllcsr.write(|w| w.pindiv().set_bit()); // usb 16 mhz clock source
dp.PLL.pllcsr.write(|w| w.plle().set_bit()); // enable PLL

only PLLE will actually be set in the end. There are two options to fix this:

1. Write all fields in one go:

   dp.PLL.pllcsr.write(|w| w
       .pindiv().set_bit() // usb 16 mhz clock source
       .plle().set_bit() // enable PLL
   );

2. Use .modify() to keep all unmodified fields as their old value:

   dp.PLL.pllcsr.modify(|_, w| w.pindiv().set_bit()); // usb 16 mhz clock source
   dp.PLL.pllcsr.modify(|_, w| w.plle().set_bit()); // enable PLL

For initialization, option 1 is usually the right choice and when modifying a single field, option 2. Keep in mind that option 1 will compile to 1 or 2 instructions while option 2 will become a read-modify-write cycle (about 4 instructions and not preemption-safe).

Related: If you use .write(), the .reset() before that is superfluous as .write() will reset all unwritten fields to their reset value.

For more details on this, take a look at the official svd2rust Documentation.

[lperlaki]

Hello I am trying to implement usb-device for the atmega32u4.
Here is my current progress. master...lperlaki:usb
I have not tested my implementation nor is it complete.

One problem apart from my lack of usb knowledge is that the usb-device crate uses AtomicPtr for its implementation and these are not available on the atmage32u4.

[lperlaki]

Hello @ExplodingWaffle , @wusyong
I don't really have that much experience with usb. I just tried too implement the usb-device trait based on the atmega32u4 documentation. So far I couldn't get it to work. Any help, PR's or advice are welcome.

[Rahix]

Can you try adding this to your target json spec?

  "max-atomic-width": 16,

[mogenson]

Hey, I'm trying to bring up the USB hardware, based on the LUFA and Teensy libraries. However, I can't seem to get the USB_GEN() interrupt to trigger from an end of reset event. Here's an example below. Do you see the same behavior?

#![no_std]
#![no_main]
#![feature(abi_avr_interrupt)]

use arduino_leonardo::prelude::*;
use panic_halt as _;

use arduino_leonardo::hal::port;
static mut PIN: Option<port::portc::PC7<port::mode::Output>> = None;
static mut USB: Option<arduino_leonardo::atmega32u4::USB_DEVICE> = None;

#[arduino_leonardo::entry]
fn main() -> ! {
    let dp = arduino_leonardo::Peripherals::take().unwrap();

    let mut pins = arduino_leonardo::Pins::new(dp.PORTB, dp.PORTC, dp.PORTD, dp.PORTE);

    let mut led0 = pins.led_rx.into_output(&mut pins.ddr);
    let mut led1 = pins.led_tx.into_output(&mut pins.ddr);

    let mut led = pins.d13.into_output(&mut pins.ddr);

    led0.set_high().void_unwrap();
    led1.set_low().void_unwrap();
    led.set_low().void_unwrap();

    unsafe {
        PIN = Some(led);
    }

    dp.USB_DEVICE.usbcon.reset();
    dp.USB_DEVICE.uhwcon.reset();
    dp.USB_DEVICE.udcon.reset();
    dp.USB_DEVICE.udien.reset();

    dp.USB_DEVICE.uhwcon.write(|w| w.uvrege().set_bit()); // enable pad regulator
    dp.USB_DEVICE.usbcon.write(|w| w.frzclk().set_bit()); // freeze usb clock
    dp.PLL.pllcsr.write(|w| w.pindiv().set_bit()); // usb 16 mhz clock source
    dp.PLL.pllcsr.write(|w| w.plle().set_bit()); // enable PLL
    while dp.PLL.pllcsr.read().plock().bit_is_clear() {} // wait for PLL lock
    dp.USB_DEVICE.usbcon.write(|w| w.otgpade().set_bit()); // enable vusb pad
    dp.USB_DEVICE.usbcon.write(|w| w.usbe().set_bit()); // enable usb controller
    dp.USB_DEVICE.udcon.write(|w| w.detach().clear_bit()); // enable pull up resistors
    dp.USB_DEVICE.udien.write(|w| w.eorste().set_bit()); // enable end of reset interrupt

    unsafe {
        USB = Some(dp.USB_DEVICE);
    }

    unsafe {
        avr_device::interrupt::enable();
    }

    loop {
        led0.toggle().void_unwrap();
        led1.toggle().void_unwrap();
        arduino_leonardo::delay_ms(300);
    }
}

#[avr_device::interrupt(atmega32u4)]
unsafe fn USB_GEN() {
    PIN.as_mut().unwrap().set_high().void_unwrap();

    let usb = USB.as_mut().unwrap();
    // end of reset interrupt
    if usb.udint.read().eorsti().bit_is_set() {
        usb.udint.write(|w| w.eorsti().clear_bit()); // clear interrupt flag
        usb.uenum.write(|w| w.bits(0)); // select ep 0
        usb.ueconx.write(|w| w.epen().set_bit()); // enable selected ep
        usb.uecfg0x.write(|w| w.eptype().bits(0)); // set ep to control type
        usb.uecfg0x.write(|w| w.epdir().clear_bit()); // set ep to out direction
        usb.uecfg1x.write(|w| w.epsize().bits(1)); // set ep size to 16 bytes
        usb.uecfg1x.write(|w| w.epbk().bits(0)); // use single bank
        usb.uecfg1x.write(|w| w.alloc().set_bit()); // allocate ep
        usb.ueienx.write(|w| w.rxstpe().set_bit()); // interrupt on setup packet
    }
}

[mogenson]

Never mind. I was paying attention to the bits being set, not the bits being cleared. The following, less readable, version enumerates correctly:

    unsafe {
        dp.USB_DEVICE.uhwcon.write(|w| w.bits(0x01));
        dp.USB_DEVICE.usbcon.write(|w| w.bits(0xA0));
        dp.PLL.pllcsr.write(|w| w.bits(0x12));
        while dp.PLL.pllcsr.read().plock().bit_is_clear() {}
        dp.USB_DEVICE.usbcon.write(|w| w.bits(0x90));
        dp.USB_DEVICE.udcon.write(|w| w.bits(0x00));
        dp.USB_DEVICE.udien.write(|w| w.bits(0x0C));
    }

[lperlaki]

Yes, now it builds.

[ExplodingWaffle]

Hey @lperlaki. I know you said it isn't tested, but I was curious if you had any example code or tips on getting this working?
I've had a go over at this repo but didn't have much success- windows complaining about "Device Descriptor Request Failed" is as far as I've got. I think I'm probably missing something obvious 😅 USB is a bit complicated for my skill.

[wusyong]

@lperlaki Hello, I would like to help too!
Are there only suspend and resume need to be done? Or there're some more issues that need to be addressed?
I can also see if it could work on my keyboard for testing.

[wusyong]

Sure thing! I'll see anything I can help.

[Rahix]

Just as a short note: Using https://github.com/mvirkkunen/usb-device will not work at the moment because this crate uses trait objects which are currently still miscompiled by LLVM (on the AVR target). I'm not sure how much sense there is in hacking a custom solution in the meantime...

[TheButlah]

are trait objects still broken on avr?

[alexmadeathing]

Great work @agausmann !

Is your crate ready for publishing? Also, may I recommend you add your lib to https://github.com/rust-embedded-community/usb-device so that it's easier for people to find?

I'm going to attempt to use this for a midi device today.

[Rahix]

@agausmann, sorry for the late reply :( I guess this serves to demonstrate that I'm also not really in the best position regarding maintenance duties...

In any case, I'd suggest that maybe we move atmega-usbd into atmega-hal as a part of this repository? Most other HALs have their USB support as part of the HAL crate so I think it wouldn't be bad to follow suit.

This would of course also mean that I will then take care of maintenance for the future.

What do you think of it?