greatscottgadgets / luna

Amaranth HDL framework for monitoring, hacking, and developing USB devices
https://greatscottgadgets.com/cynthion/
BSD 3-Clause "New" or "Revised" License
989 stars 170 forks source link

ECP5 protocol error for descriptors longer than 128 bytes #163

Open BrettRD opened 2 years ago

BrettRD commented 2 years ago

I'm porting a bit of gateware from a Deca to a Colorlight, and I've run into a bug when using GetDescriptorHandlerBlock which is not encountered with GetDescriptorHandlerDistributed Exploration here: https://github.com/amaranth-community-unofficial/deca-usb2-audio-interface/issues/5

If the descriptor grows to form a URB greater than 128 Bytes using GetDescriptorHandlerBlock it wraps and truncates the descriptor.

If I set LUNA_AVOID_BLOCKRAM, it uses a GetDescriptorHandlerDistributed instead and happily enumerates with a descriptor URB of 218 Bytes (packet size of 282 bytes)

This flag affects exactly one condition in Luna here: https://github.com/greatscottgadgets/luna/blob/08b035c9c2b053d7edffb0d220d948b5c2ca927e/luna/gateware/usb/request/standard.py#L129-L132

These two descriptor handlers ought to have identical behaviour aside from timing constraints and resource usage

This bug seems to affect the ECP5 platform and not the DECA platform, so I suspect it's an issue related to platform-specific block-ram features and allocation

jboone commented 2 years ago

I don't know how useful this data point is, but I've had LUNA USB enumeration problems, seemingly at random, which were magnified by adding a bunch of USB UAC descriptors from a DECA-based audio project. If I remove only those descriptors (but leave the ones for the non-audio portion of my project), the generated bitstreams are reliable. If I add back the audio project's descriptors (which are many!), the problem returns. If I use LUNA_AVOID_BLOCKRAM as suggested, the project with the audio descriptors seems so far to be entirely reliable. So I'm guessing there's some sort of block RAM timing closure or mischaracterization issue in prjtrellis? My USB clock timing winds up clearing the 60MHz requirement by more than 10MHz, so nextpnr thinks everything's cool. But maybe not...