Add usb-device support to atmega32u4

[TeXitoi]

Do you plan to support https://github.com/mvirkkunen/usb-device on atmega32u4? It would validate the usb-device abstraction and would allow to use rust to implement USB keyboard on most QMK compatible keyboards.

[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!

[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));
    }

[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.

[Rahix]

Can you try adding this to your target json spec?

  "max-atomic-width": 16,

[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.

[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.

[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...

[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.

[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.

[TheButlah]

are trait objects still broken on avr?

[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:

• VTables which end up in the data segment are miscompiled on AVR. rust-lang/rust#79889
  • Fixed in upstream LLVM - D87631 - not yet merged into the rust fork
• AVR: Compilation in --release mode fails with LLVM ERROR: Not supported instr rust-lang/rust#82104
  • Latest proposed patch to LLVM (not yet merged) - D114611

[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.

[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.

[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.

[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]

@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?

[agausmann]

Sure, you have my blessing to do whatever you want with the code. Its license is 0BSD for that reason

[supersimple33]

@Rahix I can take a stab at the implementation if you can explain here what that would look like to you as I have limited experience with all these low level systems. Specifically where code should lie and what abstractions should take place at each level from generic down to board-specific setup.