STM32 USBv1 large copy in interrupt context

[iggarpe]

Please see the following functions in os/hal/STM32/LLD/USBv1/usb_lld.c:

usb_packet_read_to_buffer
usb_packet_read_to_queue
usb_packet_write_from_buffer
usb_packet_write_from_queue

Those functions iterate through the entire chunk of data to copy it from/to the USB FIFO, and they are called in interrupt context. Isn't this a huge no-no in real time systems?

[Giovanni]

It is but the HW lacks a dedicated DMA. The loop goes through the packet buffer, 64 bytes usually, it is not too slow.

It should be verified if one of the standard DMA channels could do the transfer but the buffer organization is strange.

Giovanni

[iggarpe]

Yeah, I know you can't use DMA with USB (except maybe as you suggest afterwards to move the data from memory to memory).

As I see it the problem with this driver is that first you prepare reception saying where you want the data, and then you're notified when the data is there. This works great with peripherals which can use DMA, but in cases like this where no DMA is possible and the peripheral has its own dedicated memory, it would make more sense to first prepare reception (no specific buffer supplied) and then you're notified when the data has been received and where it has been stored.

Can you think of an elegant way to unify both schemes?

[Giovanni]

It is a tradeoff I made initially, not all devices have a PMA, some have queues.

The choice is between simulating a PMA on devices with queues (possible but very inefficient) or the current approach.

A pump thread is not possible in USBv1 because the final callback is supposed to be called from ISR context.

Giovanni

[iggarpe]

A pump thread is not possible in USBv1 because the final callback is supposed to be called from ISR context.

Sorry, don't understand this. Why can't you signal an event for a pump thread to react to? The data will be sitting in the buffer until it is read, worst can happen is it being overwritten by the next packet.

BTW, iscochronous endpoints are broken in current USBv1. Just a couple of things I've noticed:

1- Setting EPR_STAT_TX_NAK / EPR_STAT_RX_NAK is wrong with isochronous endpoints (only "disabled" and "valid" are legal according to RM).

2- Double buffering is always used in isochronous endpoints, which also implies you can't have bidirectional isochronous enpoints because both RX/TX ADDRESS/COUNT stm32 descriptor fields are used for either RX or TX.

[Giovanni]

Hi,

The transfer is terminated by a callback from ISR in current driver, if the transfer is performed by a thread then it is not possible to call the callback from proper context, it would be called by the thread. That would make the driver not compatible with other implementations.

Isochronous endpoints are not implemented at all in USBv1 currently, not broken.

Giovanni

[iggarpe]

The transfer is terminated by a callback from ISR in current driver, if the transfer is performed by a thread then it is not possible to call the callback from proper context, it would be called by the thread. That would make the driver not compatible with other implementations.

Oh, I see, it's not that it can't be done but that it would require a significant change in the API requiring rewritting of other implementations too. No can do.

Isochronous endpoints are not implemented at all in USBv1 currently, not broken.

Ummm... the isochronous type was accounted for in the endpoint initialization, so I assumed they were implemented. In fact, they sort of worked (probably because USBv1 is not as f*cked up as OTGv1). I went ahead and made the necessary changes to make it work, see bug #661.

[Giovanni]

Yes, I also think that the USBv1 peripheral is actually nice and well done. Few registers and it does everything, they should add a link to the DMA engine and it would be perfect.

Giovanni