pico-feedback icon indicating copy to clipboard operation
pico-feedback copied to clipboard

UARTRSR seems to contain errors for the LAST POPPED value, not the NEXT TO BE POPPED value

Open jamesmunns opened this issue 1 year ago • 6 comments

The datasheet for UARTRSR field BE states:

UARTRSR

Break error. This bit is set to 1 if a break condition was detected, indicating that the received data input was held LOW for longer than a full-word transmission time (defined as start, data, parity, and stop bits). This bit is cleared to 0 after a write to UARTECR.

In FIFO mode, this error is associated with the character at the top of the FIFO. When a break occurs, only one 0 character is loaded into the FIFO. The next character is only enabled after the receive data input goes to a 1 (marking state) and the next valid start bit is received.

From my reading, I assumed that if there IS a character in the RXFIFO, and the "next to be popped" value is the "top of the fifo", then I should see BE == 1 when the FIFO contains that value.

I have some Rust code where I am receiving N characters into an N byte buffer using DMA. I have "dmaonerr" set to true, to stop DMA after the first error. I have an interrupt that is triggered by RX errors including Line Break, and it stores that error in a static variable. The interrupt disables itself after the first error is latched, until a new transaction is started.

After the DMA is completed, I go back and check that error static, and see that the "Line Break" error has been set. At this point, I don't know if the LAST DMA'd byte was the single 0x00 that caused the error to be set, or if a line break occurred as the N + 1 byte.

So in my code, I attempt to check:

  • how many bytes were transferred (using the DMA channel's write_addr field)
  • Is there a byte in the fifo? (using !UARTFR->RXFE)
  • Does the next FIFO byte have the BE error set? (using UARTRSR->BE)

So the sequence of events looks like:

  • Start DMA of 20 bytes
  • Receive DMA of 20 bytes
  • My DMA completes
  • Receive a Line Break
  • My UART error interrupt fires, "break error" is stored
  • I go to check the transaction:
    • All bytes received (20/20)
    • There IS a byte in the fifo UARTFR->RXFE == false
    • This byte does NOT report a break error UARTRSR->BE == false

However at this point, if I manually pop the next byte by reading UARTDR, the popped byte DOES have the Break Error bit set, and if I re-read the register state now, I see:

  • There IS NOT a byte in the fifo UARTFR->RXFE == true
  • RSR DOES report a break error UARTRSR->BE == true

I can provide the Rust code that triggers this if necessary, but I wasn't sure if I was misreading the reference manual, and I didn't see any existing errata about this.

jamesmunns avatar Dec 20 '23 16:12 jamesmunns

Note: this requires an edit to inline asic documentation.

nathan-contino avatar Jan 18 '24 15:01 nathan-contino

@nathan-contino is this being investigated anywhere publicly? I'm interested in the outcome to see if I can rely on my observed assumptions :D

Or: is there anything I can do to help clarify or reproduce? Happy to help if I can! Specifically, the relevant "off label" driver usage is here (you can search for "off label"): https://github.com/embassy-rs/embassy/blob/06b74a30342c527b70acbb8bacf919a5efc67c3d/embassy-rp/src/uart/mod.rs#L567-L637, but I can provide a firmware/Rust project that can reproduce and demonstrate if it'd help.

jamesmunns avatar Jan 19 '24 08:01 jamesmunns

@jamesmunns We have more C-programmers here than we do Rust-programmers, so if you can reproduce your problem in C using the pico-sdk that'd be very useful.

And just to clarify, the UARTs in RP2040 are Arm PL011 peripherals https://developer.arm.com/documentation/ddi0183/latest/ (which I assume are fairly well battle-tested).

lurch avatar Jan 19 '24 10:01 lurch

@lurch I'm very much not familiar with the pico-sdk, so that would likely take me a significant amount of time to reproduce.

I can try to use PAC-level primitives to reproduce (roughly: direct register-level interactions, should map pretty 1:1 to using header defs), all the register and field names will be 1:1 to the datasheet.

Actually the Arm docs you link actually agree with my usage, and I believe disagree with (my understanding) of the RP2040's docs:

https://developer.arm.com/documentation/ddi0183/g/programmers-model/register-descriptions/receive-status-register---error-clear-register--uartrsr-uartecr?lang=en

If the status is read from this register, then the status information for break, framing and parity corresponds to the data character read from the Data Register, UARTDR prior to reading the UARTRSR Register.

(emphasis mine). Contrast this with the RP2040 docs:

In FIFO mode, this error is associated with the character at the top of the FIFO.

I would read this as "a value IN the FIFO", either at the head or tail, depending on how you interpret "top". It makes no mention of "the last data popped from UARTDR", like my original issue description and Arm's docs seem to agree on.

jamesmunns avatar Jan 19 '24 10:01 jamesmunns

Ah, I see, the RP2040 section I quoted appears in both (Table 3.3 in Arm, Table 426 in the RP2040 datasheet) but the useful section I quoted from the Arm docs don't appear in the RP2040 datasheet as far as I can tell.

I think the language from the tables is misleading/unclear, while the quoted section at the top of the page of the Arm docs is very clear and matches my observed behavior.

jamesmunns avatar Jan 19 '24 10:01 jamesmunns

The note at the bottom of the Arm docs is also hugely useful, and not reproduced at all, afaict, in the RP2040 docs:

The received data character must be read first from the Data Register, UARTDR before reading the error status associated with that data character from the UARTRSR Register. This read sequence cannot be reversed, because the UARTRSR Register is updated only when a read occurs from the UARTDR Register. However, the status information can also be obtained by reading the UARTDR Register

Edit (since I'm probably already spamming notifications): So, to update, I WON'T plan on attempting to minimize my reproduction case, since the upstream docs seem to agree with what I observed. My request would be to include one or both of the docs at the top or bottom of the linked PrimeCell docs page (that I quoted in my replies), that clarify that UARTRSR is latched/only updated as you pop from UARTDR. Removing the "top of fifo" verbiage would be nice, but I don't expect you to make editorial changes to the primecell docs, if you are largely mechanically importing them.

jamesmunns avatar Jan 19 '24 11:01 jamesmunns