stm32-rs / synopsys-usb-otg

usb-device implementation for Synopsys USB OTG IP cores
MIT License
41 stars 29 forks source link

Fix Tx FIFO memory calculation #26

Closed dgoodlad closed 2 years ago

dgoodlad commented 2 years ago

This used to allocate 8 * 16 words minimum, no matter how many Tx endpoints where active. This was because tx_fifo_size_words is set to [0; 9] in new. Now we only account for memory allocated to enabled Tx endpoints when determining how much has already been used by limiting the elements we consider to those before the current endpoint being allocated.

Disasm commented 2 years ago

No, this loop doesn't allocate anything, actual allocation happens below. In fact, initialization value should be [16; 9] because as mentioned in STM32F466 Reference (RM0390):

Bits 31:16 INEPTXFD[15:0]: IN endpoint Tx FIFO depth
            This value is in terms of 32-bit words.
            Minimum value is 16
dgoodlad commented 2 years ago

It may not allocate anything, but it calculates the used memory incorrectly, which results in throwing a EndpointMemoryOverflow

dgoodlad commented 2 years ago

RM0390, if I recall correctly (I'll look it up in a bit), says that only applies for active endpoints.

Disasm commented 2 years ago

If this is true (I also think so, but I was unable to find any evidence in RMs), then I'd suggest changing it to something like

used += self.tx_fifo_size_words.iter().sum();
dgoodlad commented 2 years ago

The best quote I can glean from the RM is

In peripheral mode an additional Tx FIFO is instructed for each active IN endpoint. Any FIFO size is software configured to better meet the application requirements.

which seems to imply that it's only for active endpoints.

Practically speaking, my application (an implementation of USB Audio Class 2.0) works fine with this change. It's got two endpoints, one large OUT endpoint for audio data and one 3-byte IN ep for the audio feedback channel.