abcminiuser / lufa

LUFA - the Lightweight USB Framework for AVRs.
http://www.lufa-lib.org
1.03k stars 321 forks source link

Some confusions please help: LUFA code vs. atmel spec sheets #174

Open liudr opened 3 years ago

liudr commented 3 years ago

It's been a while since I wanted to dive into LUFA code to understand how USB controllers work on 32u4 so I've finally got enough time to understand the code, which is really well written. I mean coming from Arduino USB code and Atmel's own demo code, I can see with the docs that come with LUFA you can actually understand what it's doing.

I'm writing my own USB device stack because of a special use case of 32u4. So my goal is to understand how it works. Been hacking the arduino USB code for some years but the way it's written is just a mess.

References I have:

Tools:

I found some inconsistencies between the code I read and the spec sheet I read. I am hoping the LUFA folks will help me get over the difficulties because strictly following the spec sheets got my project derailed LOL

Confusions (two exactly):

  1. The process of set_address. In LUFA, it is inside DeviceStandardReq.c, the function USB_Device_SetAddress() does the following (my summary):

https://github.com/abcminiuser/lufa/blob/8a785549cf41ec8ba7dac6480e81d0de2c243c98/LUFA/Drivers/USB/Core/DeviceStandardReq.c#L125

Except for the last bullet point, everything else was hardware related. According to 32u4 spec sheet, one shall NOT set ADDEN and address at the same time but LUFA did it anyway. I've seen atmel and arduino code do the same. So isn't this bad idea? The status stage (ZLP IN) should carry default address of 0 instead of new address.

  1. A related confusion was inside Endpoint_ClearStatusStage(), which is in Endpoint_AVR8.c. The last step of status stage is either release ZLP IN or receive and clear ZLP OUT. This function does this depending on the direction of the request. BUT, the Endpoint_ClearIN() and Endpoint_ClearOUT() both clear FIFOCON besides TXINI/RXOUTI. The spec sheet of 32u4 specifically says not to mess with FIFOCON for control endpoints. So why messing with it?

https://github.com/abcminiuser/lufa/blob/8a785549cf41ec8ba7dac6480e81d0de2c243c98/LUFA/Drivers/USB/Core/AVR8/Endpoint_AVR8.c#L129

Related inline declarations: https://github.com/abcminiuser/lufa/blob/8a785549cf41ec8ba7dac6480e81d0de2c243c98/LUFA/Drivers/USB/Core/AVR8/Endpoint_AVR8.h#L453

Unless you define CONTROL_ONLY_DEVICE, which is considered "rare" in LUFA beginner's guide, you're clearing both FIFOCON and respective interrupt flag.

abcminiuser commented 3 years ago

Sorry for the very late response!

Given it's been well over a decade since I wrote the code, I can't honestly say for certain why it is the way it is; it's possible that I just used the same optimizations from the Atmel demo code when debugging my own, misread the datasheet, or just did empirical testing and found those shortcuts to be "stable".

As an older, more seasoned and jaded firmware engineer, I'd strongly suggest following the datasheet's recommendations, as they are almost always important for reasons such as rare timing issues within the peripheral if certain constraints aren't respected.

liudr commented 3 years ago

@abcminiuser Thanks for responding! I totally get it. I can't remember what I did a month ago on an ongoing project. It's a struggle to work on larger projects and remember details if you put it down even for a few weeks. I will definitely follow the spec sheets to see if it would work or not, again. I wrote my own code "according to spec sheet" from ground up, just a few functions, but that didn't work. I'll start with working code from arduino USB and apply spec sheet to it this time. Currently working on something else so it will take me a while to do some tests and post results, one being the loading USB address, prepping a ZLP IN, then setting ADDEN, and the other being clearing TXINI/RXOUTI without clearing FIFOCON for control endpoints.

NicoHood commented 3 years ago

Maybe just try out your suggestions and open a PR for public testing?