## UART FIFO, ring buffer, interrupts and Atomic

bobtrex
Posts: 27
Joined: Mon Jan 25, 2021 6:24 pm

### UART FIFO, ring buffer, interrupts and Atomic

I would like to implement a simple ring buffer for the UART RX interrupt example.
So the Interrupt routine would add data to the ring and update the head index (with bounds checks) and the main program would read data from the ring and update the tail index.

Is is safe to increment the head index (say a unint8/16/32_t) in the interrupt and read it the in the main program. i.e. is the increment atomic so you cannot read an intermediate state in the main program? If it is then I don't need to disable the interrupt while retrieving data from the ring buffer.

Thanks and Regards
Robert.
Last edited by bobtrex on Tue Apr 20, 2021 9:38 am, edited 1 time in total.

bobtrex
Posts: 27
Joined: Mon Jan 25, 2021 6:24 pm

### Re: UART ring buffer, interrupts and Atomic

So google searches on M0+ tend to say that aligned access to 16 and 32 bit writes are atomic and unaligned is not allowed on M0+ (no mention of 8 bit writes). So I am guessing that I am safe to increment a variable in the interrupt routine and read it in the main program. Can anyone from Raspberry confirm.

Also if this is that case does this hold true between cores , increment in one core and read in the other ?

Thanks

joan
Posts: 15564
Joined: Thu Jul 05, 2012 5:09 pm
Location: UK

### Re: UART ring buffer, interrupts and Atomic

I do something similar and in limited testing haven't noticed a problem. I read from one end and write in the other.

Code: Select all

void my_uart_handler()
{
uint32_t i, status, dummy, value, nextPos;

for (i=0; i<2; i++)
{
if (g.uartInited[i])
{
{
g.event_flag |= (1<<(EVT_UART_0_RX+i));

value = uart_getc(UART[i]);

bufPushValue(EVT_UART_0_RX+i, value);
}
}
}
}
...
int bufPushValue(int id, uint value)
{
int nextPos;

nextPos = (pd_buf[id].wPos + 1) % pd_buf[id].size;

if (nextPos != pd_buf[id].rPos)
{
pd_buf[id].buf[pd_buf[id].wPos] = value;

pd_buf[id].wPos = nextPos;
}
}
...
uint32_t bufPopValue(int id, uint32_t default_value)
{
uint32_t value = default_value;
int nextPos;

if (pd_buf[id].rPos != pd_buf[id].wPos)
{
value = pd_buf[id].buf[pd_buf[id].rPos];

nextPos = (pd_buf[id].rPos + 1) % pd_buf[id].size;

pd_buf[id].rPos = nextPos;
}
return value;
}


joan
Posts: 15564
Joined: Thu Jul 05, 2012 5:09 pm
Location: UK

### Re: UART ring buffer, interrupts and Atomic

I do the same between cores with GPIO interrupts on one core being stored in a ring buffer for later retrieval on the other. I have not noticed any problems.

See https://github.com/joan2937/picod/blob/ ... ON/picod.c

kilograham
Raspberry Pi Engineer & Forum Moderator
Posts: 501
Joined: Fri Apr 12, 2019 11:00 am
Location: austin tx

### Re: UART ring buffer, interrupts and Atomic

a single write or read is atomic so therefore not an increment. you can just about cheat in some cases (if you are really careful on M0+ because there is minimal pipeline and no cache and do without normally requisite memory barriers at least, but for anything non trivial, you are much better off performing the proper of locking.

If you want to ensure exclusive access to state shared by IRQ and app code, then use a critical_section from pico_sync or a raw spin lock from hardware_sync

That said of course the SDK already has what you want in pico/util/queue.h (a queue which is multicore/IRQ safe)

bobtrex
Posts: 27
Joined: Mon Jan 25, 2021 6:24 pm

### Re: UART ring buffer, interrupts and Atomic

Thanks Joan, Kilograham

"a single write or read is atomic so therefore not an increment. you can just about cheat in some cases (if you are really careful on M0+ because there is minimal pipeline and no cache and do without normally requisite memory barriers at least, but for anything non trivial, you are much better off performing the proper of locking." - So for a simple increment on M0+ you can get away with it , but its bad practice ?

If you want to ensure exclusive access to state shared by IRQ and app code, then use a critical_section from pico_sync or a raw spin lock from hardware_sync - do you just encase your code in the critical_section_enter_blocking and critical_section_exit

"That said of course the SDK already has what you want in pico/util/queue.h (a queue which is multicore/IRQ safe)" - I looked at the queues but was wondering if I wanted to be efficient as possible and use the UART FIFOs with functions like uart_read_blocking , rather than getting a byte at a time ? It looks to me that you would need to have an intermediate buffer and then load the queue from that.

joan
Posts: 15564
Joined: Thu Jul 05, 2012 5:09 pm
Location: UK

### Re: UART ring buffer, interrupts and Atomic

The code I use reads or writes the pointers in a single instruction so is quite safe. It not as if you are trying to set a semaphore which would need a combined read/write/increment.

kilograham
Raspberry Pi Engineer & Forum Moderator
Posts: 501
Joined: Fri Apr 12, 2019 11:00 am
Location: austin tx

### Re: UART ring buffer, interrupts and Atomic

Yeah, but in general you need memory barriers if you are going to rely on a memory write (even if it is atomic at the bus level) to signal data availability between two threads. That was my point, you can get away with this on Cortex M0+ without memory barriers (or worrying about out of order execution or caches), but you still need to worry about compiler instruction re-ordering

joan
Posts: 15564
Joined: Thu Jul 05, 2012 5:09 pm
Location: UK

### Re: UART ring buffer, interrupts and Atomic

If there is a problem with compiler code generation I expect the effect will be obvious fairly quickly.

bobtrex
Posts: 27
Joined: Mon Jan 25, 2021 6:24 pm

### Re: UART ring buffer, interrupts and Atomic

Hi, thanks for the info and the code. I now have a simple implementation of a ring buffer working with the UART.

To keep things simple at first, can I use the FIFO without DMA ? Using uart_read_blocking , uart_write_blocking.
If this is the case i believe the default water mark level of the FIFOs is Receive FIFO becomes >= 1 / 2 full? I cant see a function to set these levels (direct register access, UARTIFLS ?).

To go further and use DMA for UART RX_FIFO to RX ring buffer (mainly) and TX ring buffer to UART TX FIFO.
For writing to the UART FIFO via DMA should I look at the example pico-examples/dma/control_blocks ?
Is there an example of reading from the UART FIFO via DMA

Mang thanks and regards
Robert.

joan
Posts: 15564
Joined: Thu Jul 05, 2012 5:09 pm
Location: UK

### Re: UART FIFO, ring buffer, interrupts and Atomic

I did use the FIFO but abandoned it because of the problem with the watermark. You end up with data in the FIFO but are not notified as the watermark isn't reached. DMA has similar problems. You are only notified when the transfer is complete, i.e. buffer full.

bobtrex
Posts: 27
Joined: Mon Jan 25, 2021 6:24 pm

### Re: UART FIFO, ring buffer, interrupts and Atomic

Thanks Joan

Yes I have been playing with FIFOs enabled this morning and getting strange results!
of the problem with the watermark. Is this a documented bug in the silicon ? If so there should be a note added to the FAQ section.

joan
Posts: 15564
Joined: Thu Jul 05, 2012 5:09 pm
Location: UK

### Re: UART FIFO, ring buffer, interrupts and Atomic

It's not a bug, just a feature. It would be nice if the feature was changed to make the FIFO more useful. What is needed is an interrupt on FIFO not empty.

bobtrex
Posts: 27
Joined: Mon Jan 25, 2021 6:24 pm

### Re: UART FIFO, ring buffer, interrupts and Atomic

What I see on FIFO enabled, with sending 4 bytes (puts) at 115200, TX linked to RX is that the interrupt fires below the water mark (is there a timeout so the interrupt is fired after a period if the watermark is not reached ?). However in the interrupt handler uart_is_readable only ever returns 1, so I cant determine the number of bytes to use with uart_read_blocking. Is this what you mean with problem with the watermark ?

The SDK doc for uart_read_blocking states 0 if no data available, otherwise the number of bytes, at least, that can be read. So is this a bug or am i likely doing something wrong in code (its return type is a bool, which seems strange.)

joan
Posts: 15564
Joined: Thu Jul 05, 2012 5:09 pm
Location: UK

### Re: UART FIFO, ring buffer, interrupts and Atomic

In my case I would only read the UART when interrupted. But you don't get an interrupt until the watermark is reached. So you don't get interrupted for short messages or the residue of long messages.

bobtrex
Posts: 27
Joined: Mon Jan 25, 2021 6:24 pm

### Re: UART FIFO, ring buffer, interrupts and Atomic

Hi Joan

Yes you are correct, from my code below it appears that with the RX FIFO enabled, no interrupt is called below 4 bytes, does this match your tests).
So I assume the default watermark level set in register is for 4 bytes (do you know how to check the value of a hardware register ?)
Did you find a mechanism to set the watermark level ? (I tried "hw_write_masked", and "UARTIFLS" but probably got it wrong as it does not seem to change!)
I was thinking of using the FIFO but then have a polled check for any remaining bytes (based on time of last Interrupt, Baud and watermark level).

As "uart_read_blocking" always seems to returns 1 when there are one or more bytes in the FIFO (surely this cant be the intent of the function) I would still have to loop through the bytes using "uart_getc", rather than using "uart_read_blocking"

I was also going to ask would you be able to share your code for reading from the UART with DMA ?

Many thanks

Code: Select all

#include <stdio.h>
#include "pico/stdlib.h"
#include "pico/util/queue.h"
#include "pico/multicore.h"
#include <string.h>
#include "hardware/timer.h"
#include "hardware/uart.h"
#include "hardware/irq.h"

critical_section_t acrit;

uint32_t count = 0;

struct data_s
{
uint8_t data[64];
}queue_entry_t;

queue_t rx_queue;
queue_t tx_queue;
queue_t rx_buf;
queue_t tx_buf;

uint8_t buff[256];

uint8_t t1[64];
uint8_t t2[64];
volatile uint8_t last = 0;

uint32_t int_calls = 0;

volatile absolute_time_t int_called_first ;
volatile absolute_time_t int_fin_first ;
volatile absolute_time_t int_start_first ;

uint8_t write_data [64];
#define rx_buff_size 256

volatile uint32_t int_rentered = 0;
volatile uint8_t int_flag = 0;

struct ringbuff_t
{
volatile uint8_t buff[rx_buff_size];
volatile uint32_t tail;
volatile uint32_t consumed;
volatile uint32_t discared;
};

struct ringbuff_t rx_buff;

#define UART_ID uart0
#define BAUD_RATE 300
#define DATA_BITS 8
#define STOP_BITS 1
#define PARITY    UART_PARITY_NONE

// We are using pins 0 and 1, but see the GPIO function select table in the
// datasheet for information on which other pins can be used.
#define UART_TX_PIN 12
#define UART_RX_PIN 13

static int chars_rxed = 0;
static uint32_t chars_rx_last = 0;

// RX interrupt handler
void on_uart_rx() {
if(int_calls==0){int_start_first =  get_absolute_time();}//record first time the interupt was called
if(int_flag == 1){int_rentered++;}// The interupt routine was re-entered
int_flag = 1;
if(int_calls!=0){int_called_first =  get_absolute_time();}//record first time the interupt was called
int_calls++; // update the number of times the interupt is called

//chars_rx_last = 0;
printf("Bytes in FIFO = :%u\n", received);
{
uint8_t ch = uart_getc(UART_ID);
// Can we send it back?

// Change it slightly first!
//   ch++;
//   uart_putc(UART_ID, ch);
//printf("%c\n",ch);

chars_rx_last++;
chars_rxed++;
uint16_t space = 0;
//absolute_time_t before =  get_absolute_time();
// for (int i = 0; i<32; i++)
//{
{
space = rx_buff.tail - rx_buff.head -1;
}
else{
space = (rx_buff_size - rx_buff.head) + rx_buff.tail -1;
}
if(space>0)
{

last = ch;
//printf("%c\n",last);

}
else{
rx_buff.discared++;
// printf("Core1 discarder tail =   %u\n", rx_buff.tail);
}
// }

}
if(int_calls==1){int_fin_first =  get_absolute_time();}//record first time the interupt was called
int_flag = 0;
}

int main()
{

stdio_init_all();
// Set up our UART with a basic baud rate
uart_init(UART_ID, BAUD_RATE);

// Set the TX and RX pins by using the function select on the GPIO
gpio_set_function(UART_TX_PIN, GPIO_FUNC_UART);
gpio_set_function(UART_RX_PIN, GPIO_FUNC_UART);

// Actually, we want a different speed
// The call will return the actual baud rate selected, which will be as close as
// possible to that requested
int actual = uart_set_baudrate(UART_ID, BAUD_RATE);

// Set UART flow control CTS/RTS, we don't want these, so turn them off
uart_set_hw_flow(UART_ID, false, false);

// Set our data format
uart_set_format(UART_ID, DATA_BITS, STOP_BITS, PARITY);

//Attemp to change RX watermark level, does not seem to change.
//hw_set_bits(&uart_get_hw(UART_ID)->lcr_h, UART_UARTIFLS_RXIFLSEL_BITS); //bits 543
hw_set_bits(&uart_get_hw(UART_ID)->lcr_h, _u(0x00000020)); //bits 5 = 1
hw_clear_bits(&uart_get_hw(UART_ID)->lcr_h, _u(0x00000018));//clear bit 4,1

//try to set the watermark level, does not seem to change
uint32_t val_set = 0x20;
UART_UARTIFLS_RXIFLSEL_BITS);

// Turn off FIFO's - we want to do this character by character
//uart_set_fifo_enabled(UART_ID, false);
uart_set_fifo_enabled(UART_ID, true);
// Set up a RX interrupt
// We need to set up the handler first
// Select correct interrupt for the UART we are using
int UART_IRQ = UART_ID == uart0 ? UART0_IRQ : UART1_IRQ;

// And set up and enable the interrupt handlers
irq_set_exclusive_handler(UART_IRQ, on_uart_rx);
irq_set_enabled(UART_IRQ, true);

// Now enable the UART to send interrupts - RX only
uart_set_irq_enables(UART_ID, true, false);

//load data to send on the UART
for(int i =0; i<64; i++)
{
write_data[i]= i+47; //ascii values from "\""
}
critical_section_init(&acrit);

sleep_ms(10000);

printf("Start UART FIFO and Interupt Test\n");
// printf("time 1  %lld\n", time_us_64() );
// printf("time2  %lld\n", time_us_64() );
before_add = get_absolute_time(); // record time we start to put data out on the UART
sleep_us(100);
after_add = get_absolute_time(); // record time we start to put data out on the UART
printf("Absolute time diff test =   %lld\n", time_diff);

//initialise ring buffer
rx_buff.consumed =0;
rx_buff.tail = 0;
rx_buff.discared = 0;
chars_rxed = 0;
int_calls = 0;
int32_t loop_count = 0;
//sleep_ms(1000);
while(1)
{

before_add =  get_absolute_time(); // record time we start to put data out on the UART
sleep_us(100);
//uart_puts(UART_ID, "Loop 123456789 123456789 "); //
//With the FIFO eneabled the interupt seem to be called after 4 bytes
if (loop_count ==0){uart_write_blocking(UART_ID,write_data,16);}
loop_count++;
printf("UART Data Set, wait for 5 seconds\n");

sleep_ms(5000);
// for (int i = 0; i<4; i++)
// {
//     printf("Data: %u\n",rx_buff.buff[i]);
// }
uint32_t num_data = 0;
uint8_t data = 0;
critical_section_enter_blocking(&acrit);  //Seems to be ok without so increment atomic ?
critical_section_exit(&acrit);

printf("*******************************************************\n");
printf("\n");
printf("%c\n",last);
printf("Interupt handler was called, number of times = %u\n", int_calls);// int_flag = 1;
printf("Interupt handler was re-entered , number of times = %u\n", int_rentered);// int_flag = 1;
printf("Approx micorsecond after data available interupt was called:%lld\n", time_diff); //int_called_first
time_diff = absolute_time_diff_us(int_start_first, int_fin_first);
printf("Interupt runtime microsedocns: :%lld\n", time_diff); //int_called_first
printf("Bytes processec by interupt = %u\n", chars_rxed);//rx_buff.tail
printf("Ring buffer tail = %u\n", rx_buff.tail);
//printf("Rx bytes recievd last %u\n", chars_rx_last);//after_add
printf("*******************************************************\n");

//Get waht data in on the buffer and print it.

{

{
num_data = (rx_buff_size - rx_buff.tail) + tmphead ;
}
else
{
num_data = tmphead - rx_buff.tail ;
}
printf("Number of bytes to process in ring buffer: %u\n", num_data);
int32_t tmptail = rx_buff.tail;
printf("Start to decode buffer, tail: %u\n", tmptail);
for(int i = 0; i < num_data; i++)
{
//printf("ring buffer decode for loop\n");
data = rx_buff.buff[tmptail];
printf("\ndecoding at position: %u", tmptail);
printf("  Data:%u",data);
tmptail++;
rx_buff.consumed++;
if(tmptail == rx_buff_size) {tmptail=0;}

}
critical_section_enter_blocking(&acrit);
rx_buff.tail = tmptail;
critical_section_exit(&acrit);
}
printf("\nTotal bytes read from ring: = %u\n", rx_buff.consumed);
printf("Ring buffer tail = %u\n", rx_buff.tail);
printf("\n");
// for (int i = 0; i<4; i++)
//  {
//     printf("Data: %u\n",rx_buff.buff[i]);
// }
sleep_ms(100000);
}

/// \end::setup_multicore[]
return 0;
}

Output from Code

Code: Select all

Absolute time diff test =   135

UART Data Set, wait for 5 seconds

Bytes in FIFO = :1

Bytes in FIFO = :1

Bytes in FIFO = :1

Bytes in FIFO = :1

*******************************************************

>

Interupt handler was called, number of times = 4

Interupt handler was re-entered , number of times = 0

Approx micorsecond after data available interupt was called:532510

Interupt runtime microsedocns: :161

Bytes processec by interupt = 16

Ring buffer tail = 0

Total bytes added to ring: 16

*******************************************************

Number of bytes to process in ring buffer: 16

Start to decode buffer, tail: 0

decoding at position: 0  Data:47

decoding at position: 1  Data:48

decoding at position: 2  Data:49

decoding at position: 3  Data:50

decoding at position: 4  Data:51

decoding at position: 5  Data:52

decoding at position: 6  Data:53

decoding at position: 7  Data:54

decoding at position: 8  Data:55

decoding at position: 9  Data:56

decoding at position: 10  Data:57

decoding at position: 11  Data:58

decoding at position: 12  Data:59

decoding at position: 13  Data:60

decoding at position: 14  Data:61

decoding at position: 15  Data:62

Total bytes read from ring: = 16

Ring buffer processed head = 16

Ring buffer tail = 16


joan
Posts: 15564
Joined: Thu Jul 05, 2012 5:09 pm
Location: UK

### Re: UART FIFO, ring buffer, interrupts and Atomic

I'm not sure I bothered writing any code for UART/DMA. I just thought about what would be needed and decided it was more effort than it was worth (at that time). I did write some for I2C but I couldn't get it to work properly and decided that interrupts were just as good for I2C anyhow.

I'm happy enough with interrupts for the UART for the time being. If high data rates do become a problem I would revisit using the FIFO.

bobtrex
Posts: 27
Joined: Mon Jan 25, 2021 6:24 pm

### Re: UART FIFO, ring buffer, interrupts and Atomic

Thanks very much for your help

I will create a new thread to see if I can get help setting the watermark and understanding why "uart_read_blocking" always seems to returns 1.

joan
Posts: 15564
Joined: Thu Jul 05, 2012 5:09 pm
Location: UK

### Re: UART FIFO, ring buffer, interrupts and Atomic

Re the watermark, I looked at the time and there is no way of setting the level to one byte or more according to the RP2040 datasheet.