Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problems with pico-sdk UART transmission: blocking exits early, and (possibly PL011) 2nd stop bit missing #1274

Closed
sdbbs opened this issue Feb 22, 2023 · 3 comments
Assignees
Milestone

Comments

@sdbbs
Copy link

sdbbs commented Feb 22, 2023

Hi all,

I have prepared a small pico-sdk project, rp2040_uart_err_test, an RP2040 UART TX test, in this gist: rp2040_uart_err_test

I believe that it demonstrates the following two problems:

  1. uart_write_blocking does block, but returns early: when it returns, two bytes (in this test) still remain to be sent
    • The documentation for uart_write_blocking explicitly says:

      "This function will block until all the data has been sent to the UART"

      ... so I would expect either this function to indeed block until all data has been transmitted - or the help text needs to be more precise about what should be expected, otherwise it is misleading as it is.

  2. If UART is set up for 2 stop bits, then when transmitting (TX) over UART, 2nd stop bit is never transmitted
    • The documentation for uart_set_format explicitly says:

      "stop_bits Number of stop bits 1..2 "

      ... so I would expect that two stop bits are sent, if I have configured the UART for two stop bits.


Few words about the UART TX test project, rp2040_uart_err_test:

  • I have tried to make it as "low-level" as possible, to avoid possible setup interference with convenience features of "vanilla" projects; so the project avoids pico_stdlib and builds only against pico_runtime; also stdio_usb and stdio_uart are disabled
  • There is a setup for a UART RX interrupt service routine, on_uart_rx_isr, but it is there simply to go through the setup motions - it is otherwise not expected to fire or do anything in this test
  • Clock is at 133 MHz

So, the main functionality is in the main function:

  • There is a 24-byte global array tx_buffer with some random binary content
  • After setup, main goes into the infinite while loop, where:
    • it sleeps for 250 ms
    • then it sets debug pin HIGH, executes uart_write_blocking with the intent to send/TX all 24 bytes of tx_buffer (via reference variables), and sets debug pin LOW

This would allow for the debug pin indicating the duration of uart_write_blocking. Here is the while loop:

// ...
  static volatile bool mindicator;
  while (1) {
    sleep_ms(250);

    mindicator = true;
    gpio_put(DEBUG_PIN_A, mindicator);

    volatile uint8_t* tx_buffer_p = NULL;
    volatile size_t bytes_to_send = 0;
    tx_buffer_p = (uint8_t*)&tx_buffer;
    bytes_to_send = sizeof(tx_buffer);
    uart_write_blocking(UART_ID, (const uint8_t *)tx_buffer_p, bytes_to_send);

    mindicator = false;
    gpio_put(DEBUG_PIN_A, mindicator);
  }
// ...

I have tried to use volatile variables to hint to the compiler not to reorder statements; looking at the output of arm-none-eabi-objdump -S rp2040_uart_err_test.elf that is not obvious at first, because there are branching jumps - but I've tried to follow the jumps, and it does seem the statements are in order.

Finally, as output of the test program, I am capturing the debug pin and the UART TX pin with Saleae Logic.


Let's look at the stop bit problem first. First, let's recall, that for a 115200 baud traffic, the bit period is 1/115200 ≈ 8.68 μs

The rp2040_uart_err_test code as submitted, sets up the UART (via #defines) for 115200 baud, 8N2 traffic. However, at first I tested with a 8N1 build, which worked great:

build_1152008N1_decode_1152008N1_OK

Note that I've activated the Async Serial decoder from Saleae Logic on the channel with the digital capture of the UART TX signal, and it decodes the right bytes; also, the duration of a byte in this case is measured at 87.68 μs - quite close to 10 bits (start + 8 data + stop) times the bit period 8.68 μs.

So, then I just change the STOP_BITS define to 2, rebuild, capture again, try to capture with Saleae Logic again - and set up the Async Serial decoder accordingly, with stop bits to 2:

build_1152008N2_decode_1152008N2_bad

... and I can see the Async Serial decoder report framing errors; but if I change back the settings of the Async Serial decoder to 8N1, it again decodes no errors:

build_1152008N2_decode_1152008N1_OK

Here we can see that even if I had specified 8N2, a byte with a duration 87.2 μs has been sent, quite close to the previous duration 87.68 μs - whereas 8N2 would require 1+8+1+1 = 11 bits, times expected duration 8.68 μs, that is around 95.48 μs for the full duration.

Conclusion: second stop bit had not been sent, even if I specified 8N2.

To be frank, I don't think this is pico-sdk problem; the PrimeCell® UART (PL011) Technical Reference Manual mentions:

3.3.7 Line Control Register, UARTLCR_H

...

3 STP2 Two stop bits select. If this bit is set to 1, two stop bits are transmitted at the end of the frame. The receive logic does not check for two stop bits being received.

... and uart_set_format in uart.h does indeed set those bits; and I even have a function get_uart_lcr_h in my code, so I can confirm in gdb (also note that actual_baud_rate is 115176, slightly less than requested 115200):

(gdb) p actual_baud_rate
$1 = 115176
(gdb) p actual_clock_sys_hz
$2 = 133000000

(gdb) p /t get_uart_lcr_h()
$3 = 1101000
#   (6543210)

... and it's clear that at runtime, the STP2 (3rd) bit of UART_LCR_H register is indeed enabled - and yet, I've obtained the scope traces seen above, where there is no second stop bit.


And for the first problem: let's look at debug pin (which indicates the duration of uart_write_blocking) versus entire UART TX packet -- where for ease of perception, Async Serial decoder is set up for 8N1:

build_1152008N2_decode_1152008N1_blocking

It is visible that right after the debug pin goes low (where supposedly uart_write_blocking stopped blocking and returned), another two bytes are being sent on wire.


Are there any solutions or workarounds for these issues?


EDIT: also related: UART only works for 8N1; API exposes functions that mishandle UARTLCR · Issue #548 · raspberrypi/pico-sdk

@lurch
Copy link
Contributor

lurch commented Feb 25, 2023

I'm not a hardware engineer, so please take my comments with a grain of salt, but just in case it's helpful...

(also note that actual_baud_rate is 115176, slightly less than requested 115200)

You've set the system-clock to 133MHz, which (currently) sets the peripheral clock to 48MHz, see #841
And then see section 4.2.7.1. of https://datasheets.raspberrypi.com/rp2040/rp2040-datasheet.pdf for how the UART baud-rate is calculated from the peripheral clock.

so I would expect either this function to indeed block until all data has been transmitted

Perhaps the data has been written into the UART's TX FIFO buffer, and so from the CPU's point-of-view the "all the data has been transmitted to the UART block", even though the UART hasn't physically transmitted all the data from its TX FIFO over the physical wire yet? Looks like you could use the UARTTXINTR interrupt (with TXIFLSEL set to 0) to monitor when the UART's TX FIFO is "less than 1/8th full"? Ah, or I guess you could busy-loop checking for when TXFE indicates that the TX FIFO is actually empty, but note that that also says "This bit does not indicate if there is data in the transmit shift register.".

@andygpz11
Copy link
Contributor

andygpz11 commented Mar 1, 2023

@sdbbs Thank you for your observations and data. There are several things here, I will deal with them in sequence.
Your point 1: mentions:

"This function will block until all the data has been sent to the UART"

This is actually correct, the API provided will block until all the data has been written into the UART.
There is a difference between (software) sending data to the UART and the UART machinery actually sending that data over the wire. There will be (at least) a shift register which is not programmer accessible.
Perhaps the word "sent" in this API doesn't make this clear enough, I will see if we can revise this.

I can offer a mitigation for this. You can add the code I have bolded to busy wait on the UART "busy" bit.

...
uart_write_blocking(UART_ID, (const uint8_t *)tx_buffer_p, bytes_to_send);
while (uart_get_hw(UART_ID)->fr & UART_UARTFR_BUSY_BITS) {
tight_loop_contents();
}

...

I observe this delays until about halfway through the last character being transmitter on the wire. So now very close, I'm not sure we can do better from the UART.

2: (and other posts). You are entirely correct, your code does configure 2 stop bits but the UART is (still) sending one.

I have tracked that down to the fact that the uart_set_format(...) is actually called twice and does not work as expected the second time.

The first time it is called from uart_init(...) which is hard coded to 8 data, 1 stop bit. The second call of uart_set_format(...) is from your code and is not observed, I think because the format can not be altered when the UART is enabled.

A temporary mitigation: If you change the call in uart_init(...) to set two stop bits when it calls uart_set_format(...) you will see two bits on the wire. You can remove your own call to uart_set_format(...) if you like.

We will need to decide how we fix this is Pico-SDK, it looks like #556 and #548 you linked are all connected.

For your last observation:

(also note that actual_baud_rate is 115176, slightly less than requested 115200)

That is absolutely fine, the "non-exact" frequency is a result of the maths. Personally I like to see Baud rates within 1% but this link explains what about 2% is accurate enough (and will be even with 11 bit transmission, that is 1 + 8 + 2):

https://community.silabs.com/s/article/uart-rs232-required-clock-accuracy?language=en_US

Your Baudrate is 99.9791% of expected so is actually very, very close.

HTH.

@peterharperuk
Copy link
Contributor

Waiting for this #1709

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

5 participants