stnolting / neorv32

:desktop_computer: A small, customizable and extensible MCU-class 32-bit RISC-V soft-core CPU and microcontroller-like SoC written in platform-independent VHDL.
https://neorv32.org
BSD 3-Clause "New" or "Revised" License
1.54k stars 212 forks source link

UART RTS Issue #429

Closed vhdlnerd closed 1 year ago

vhdlnerd commented 1 year ago

I was having an issue overflowing the UART RX FIFO (16 byte deep) when using hardware handshaking (RTS/CTS). After connecting a logic analyzer to the UART bus, I found the RTS output is asserted/de-asserted at the wrong times. It only de-asserts when receiving a byte instead of when the RX FIFO is almost full -- i.e., RTS should be de-asserted when no room is left in the input buffer/FIFO.

Just for a test (not a final or robust solution), I changed line 549 (in neorv32_uart.vhd) from:

uart_rts      <= (not rx_engine.rtr)  when (ctrl(ctrl_rts_en_c) = '1') else '0'; -- output is low-active

To:

uart_rts      <= rx_buffer.half  when (ctrl(ctrl_rts_en_c) = '1') else '0'; -- output is low-active

This de-asserts RTS when the RX FIFO is half full.

This solved my RX FIFO overflow issue.

Note: using the FIFO not full flag for RTS is not good enough. On my setup when RTS is de-asserted, my Windows machine would still send up to two more characters before stopping it's UART TX traffic. As shown, below:

image

Again, my "solution" above is not the best; really this line should be changed:

rx_engine.rtr <= '1' when (rx_engine.state = S_RX_IDLE) and (ctrl(ctrl_en_c) = '1') else '0';

An almost full flag (or just the half-full flag) from the FIFO should be used instead of the FSM S_RX_IDLE test. Also, I'm not sure if a RX FIFO depth of 1 would work at all, with RTS, if the sender can "skid" two characters past RTS de-asserting.

Thanks, Brian

stnolting commented 1 year ago

I rarely use the RTS signal, but if I do then I also use a huge UART FIFO. Hence, I did not notice this behavior so for. Anyway, thanks for the hint!

RTS should be de-asserted when no room is left in the input buffer/FIFO.

That makes sense! So it should be de-asserted when the RX FIFO is full.

Note: using the FIFO not full flag for RTS is not good enough. On my setup when RTS is de-asserted, my Windows machine would still send up to two more characters before stopping it's UART TX traffic.

I have seen this behavior on my machine, too.

Again, my "solution" above is not the best; really this line should be changed: [...] An almost full flag (or just the half-full flag) from the FIFO should be used instead of the FSM S_RX_IDLE test. Also, I'm not sure if a RX FIFO depth of 1 would work at all, with RTS, if the sender can "skid" two characters past RTS de-asserting.

You are right. Since we do not have an "almost full" flag, the half full flag seems to be the best solutions:

  -- Enough space in RX buffer (+ margin) for a new char? --
  rx_engine.rtr <= '1' when (rx_buffer.half = 0) and (ctrl(ctrl_en_c) = '1') else '0';

Also, I'm not sure if a RX FIFO depth of 1 would work at all, with RTS, if the sender can "skid" two characters past RTS de-asserting.

This is a very special corner case. With a RX FIFO size of 1 the RTS signal will be asserted when the FIFO is empty. I think this okay, but there should be a note regarding this in the documentation.

vhdlnerd commented 1 year ago

The half full flag solution would be adequate. Although, I think the code should be:

  -- Enough space in RX buffer (+ margin) for a new char? --
  rx_engine.rtr <= '1' when (rx_buffer.half = '1') and (ctrl(ctrl_en_c) = '1') else '0';

since RTR (Ready To Receive) is low active.

Edited: OOPS! Never mind I see RTR is inverted later before going to the output.

Brian