Fast uart v2

[MrJake222]

I propose this in favor of #58.

Simple enough code, UART's FIFO act as a 24 byte buffered DMA (and has a timeout irq). No alarm code required, works fine at 1MBaud (higher bauds not tested), with CTS line from FPGA not to overflow the 1-byte simple uart module. Continous stream at 1MBaud, no problems.

Possible improvements:

• ice_usb_uart_tx as DMA,
• less polled code, however I can't justify it without an RTOS and without mutexes.

[MrJake222]

Also I'd suggest to increase rx/tx buffer sizes to at least 2x of endpoint buf size. This way, when all data of current packet will be read, a new packet will already be there.

[josuah]

Very interesting how measuring + insights of what happens permits to optimize the right thing first. The other PR was having a great approach of DMA and permitted to lead to this more minimal changeset.

I will collect time to review this but looks pretty good overall. Small nits here if anything.

Is CTS support part of the PR? I suppose not everyone will implement it FPGA_side.

[MrJake222]

Is CTS support part of the PR? I suppose not everyone will implement it FPGA_side.

Actually RP2040 has CTS line support in hardware and it boils down to enable correct function on the gpio. It can be done outside of the sdk and it's not a part of this PR. The image is just to show USB flow control works as expected.

edit: oh no, I'm getting buffer overflows with this one... edit2: it's so unpredictable, after board reset, first transaction fails, then it's okay.

[MrJake222]

Hi,

1. After a long debugging day the board hanged on me 2 times. I think it's due to high reset count and usb device number overflowing or something, rebooting the linux host fixes it.
2. Garbage data fixed by sleep_ms(10) and then clearing the fifos, I didn't find a better way. RP2040 seems to pick up the slightest noise on the line when initializing FPGA. Maybe parity would help, but this aims to be as transparent as possible.

edit: ...still hanging on some edge cases

[MrJake222]

CDC seems to pass only whole 512 byte "packets" now (whole CFG_TUD_CDC_EP_BUFSIZE buffers)... Any ideas why?

edit: of course after posting this I found something interesting. Why endpoint size is defined as 512 bytes in default firmware while tinyusb defines it by default as 64 bytes (for full speed devices)?

edit2: removing the definition of CFG_TUD_CDC_EP_BUFSIZE seems to have fixed the issue, lol

[josuah]

I did find some time to follow your debugging, but did not get a chance to answer until now.

Actually RP2040 has CTS line support in hardware and it boils down to enable correct function on the gpio. It can be done outside of the sdk and it's not a part of this PR. The image is just to show USB flow control works as expected.

Thank you all good then! That also makes sense.

high reset count [...] rebooting the linux host fixes it.

Does this mean you found an existing bug on the pico-ice SDK? I will open an issue to make sure to test it upon next release if so.

RP2040 seems to pick up the slightest noise on the line when initializing FPGA. Maybe parity would help, but this aims to be as transparent as possible.

This is possible that the FPGA init consumes a lot of power at once and the clock signal could be a bit too close to the UART... that could be tested by using another pin if interested.

CDC seems to pass only whole 512 byte "packets" now

For BULK endpoints in FullSpeed, it looks like this should be 64 bytes that is right. It also seems like 64 and 512 are default values picked to match the USB max packet size, and is a good idea to follow the defaults.

From TinyUSB:

// CDC Endpoint transfer buffer size, more is faster
#define CFG_TUD_CDC_EP_BUFSIZE   (TUD_OPT_HIGH_SPEED ? 512 : 64)

#ifndef CFG_TUD_CDC_EP_BUFSIZE
  #define CFG_TUD_CDC_EP_BUFSIZE    (TUD_OPT_HIGH_SPEED ? 512 : 64)
#endif

So yes you are right it can be deleted, thank you!

[MrJake222]

Do you think we should limit flush call count? I saw it generates packets as small as 1 byte. Maybe calling it once every 100ms or so?

[josuah]

Do you think we should limit flush call count? I saw it generates packets as small as 1 byte. Maybe calling it once every 100ms or so?

Feel free to add it to the PR, but it is always possible to add it in other PRs. That permits to avoid having one PR grow and grow and introduce a lot of changes at once. Either are fine!

[MrJake222]

I thought about a different PR solving a different issue!

I vote to merge this first as it works well enough.

[josuah]

Many thanks for all the time spent in this! Feel free to keep discussing on this thread.