abcminiuser / lufa

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

ORDERED_EP_CONFIG documentation #111

Closed NicoHood closed 6 years ago

NicoHood commented 6 years ago

Extract from the docs:

ORDERED_EP_CONFIG - (Endpoint Management , Pipe Management) - AVR8, UC3 The USB AVRs do not allow for Endpoints and Pipes to be configured out of order; they must be configured in an ascending order to prevent data corruption issues. However, by default LUFA employs a workaround to allow for unordered Endpoint/Pipe initialization. This compile time token may be used to restrict the initialization order to ascending indexes only in exchange for a smaller compiled binary size. Use caution when applied to applications using the library USB Class drivers; the user application must ensure that all endpoints and pipes are allocated sequentially.

Now If I enable this token in my combined cdc/keyboard program everything still functions properly. Also in the datasheet I cannot find any hint why the order would matter.

  1. Now even if the order is important, my question is, if the config can have holes. For example a keyboard device only using endpoint 2, leaving endpoint 1 unconfigured.

  2. Where do I need to pay attention for the order? Only in EVENT_USB_Device_ConfigurationChanged()?

  3. Regarding the CDC class drivers, wouldnt it be better to already set the EP endpoints in an order that works with ORDERED_EP_CONFIG? In this case it would be:

    #define CDC_NOTIFICATION_EPADDR        (ENDPOINT_DIR_IN  | 3)
    #define CDC_TX_EPADDR                  (ENDPOINT_DIR_IN  | 1)
    #define CDC_RX_EPADDR                  (ENDPOINT_DIR_OUT | 2)

As reference from the class driver:

    if (!(Endpoint_ConfigureEndpointTable(&CDCInterfaceInfo->Config.DataINEndpoint, 1)))
      return false;

    if (!(Endpoint_ConfigureEndpointTable(&CDCInterfaceInfo->Config.DataOUTEndpoint, 1)))
      return false;

    if (!(Endpoint_ConfigureEndpointTable(&CDCInterfaceInfo->Config.NotificationEndpoint, 1)))
      return false;
  1. Do I also need to pay attention to the control ep in that case?

It would be nice if you could clarify this in the docs.

abcminiuser commented 6 years ago

The hardware USB controller allocated memory from the internal (hidden) DPRAM region as endpoints are allocated by the user application. However, the allocator is very simplistic and will only take into account the highest enabled endpoint when it picks the next DPRAM region to allocate to a newly enabled endpoint.

This means that if you allocate your endpoints in a non-sequential order, the hardware can incorrectly allocate endpoint buffers from DPRAM, resulting in endpoints whose bank regions overlap in DPRAM (resulting in corrupted transfers).

If you are careful to enable endpoints in always ascending order, you can disable the LUFA workaround code, which essentially disabled and re-enables all higher number endpoints each timer a lower number endpoint is enabled, to ensure the DPRAM is correctly allocated.

The ascending order applies whenever an endpoint is configured. The control endpoint doesn't really apply here, since it's always the lowest endpoint index and always configured, so subsequent endpoint configurations are guaranteed to be of a higher index.

While I can make sure the class driver examples use sequential endpoint indexes this isn't always possible - different USB AVR parts have different restrictions on bank size, double banking and endpoint type for each endpoint index.

Essentially, the ORDERED_EP_CONFIG token should only really be used for low level API applications where you can guarantee your Endpoint_ConfigureEndpoint* calls always configure your endpoints in ascending order. It can save a few bytes of flash, but if you use it incorrectly you'll get some very odd behaviors with fully or partially overlapping endpoint banks.

NicoHood commented 6 years ago

Thanks, then my implementation should be okay. I am just wondering where to find this information in the datasheet. In the registers for the ep config was not such a notice.

Do you know if using 2 banks should cause any trouble? I know some endpoints do not support double banks and less DPRAM. On my 32u4 every endpoint should work fine with 64byte+2banks. However I once had some problems with it (CDC Serial) which are now resolved for an unknown reason since i started working on my usb config. So you think it is worth to use the CDC config endpoint or HID device with double banks? I just enabled it for CDC RX/TX for now.

abcminiuser commented 6 years ago

I am just wondering where to find this information in the datasheet. In the registers for the ep config was not such a notice.

It's in the overall documentation for the USB controller in the datasheet - on my (admittedly very old version) of the U4 series datasheet, it's in section 21.9 Memory management.

Do you know if using 2 banks should cause any trouble? I know some endpoints do not support double banks and less DPRAM.

As long as it's supported on the given endpoint, it should always be safe. The only issues would be around allocation order (given the above endpoint ordering hazard) or running out of DPRAM, since all double banked endpoints consume twice the normal amount of DPRAM.

NicoHood commented 6 years ago

Thanks. Now that everything seems to work, I will close this issue.