Does this code need interrupts disabled?

[peter-h]

I tried that; no difference to the errors. Also

The problem with the serial_transmit_active flag is that it is still set for one character time after the TXE interrupt has been disabled, and the serial_transmit function wont trigger a new transmit during that time

would that explain a data loss? ISTM that this merely produces a little gap in the transmitted data.

BTW the issue with large data loss on small packets was a redherring - caused by too short a timeout on the loopback test rig. Now I cannot reproduce any data loss on short packets.

The data loss is 1 or 2 bytes.

EDIT: dropping all baud rates a long way, to 9600 baud, doesn't improve things, so I think the data loss is a "structural" thing.

[NorthGuy]

EDIT: dropping all baud rates a long way, to 9600 baud, doesn't improve things, so I think the data loss is a "structural" thing.

Most likely a problem with how you handle the hardware, such as killing the transmitter before the last character gets transmitted or something of that sort.

[wek]

This is why I asked for the ISR.

JW

[peter-h]

Code: [Select]



// Common interrupt handler for all USART interrupts
static void usart_common_IRQHandler(UART_HandleTypeDef *huart, uint8_t port)
{
// check if we are here because of RXNE interrupt
if (huart->Instance->SR & USART_SR_RXNE)
{ //if RX is not empty
serial_receive_irq_handler(huart, port);
}

// check if we are here because of TXEIE interrupt
if (huart->Instance->SR & USART_SR_TXE)
{ //if TX is empty
serial_transmit_irq_handler(huart, port);
}

// check if we are here because of the TC interrupt
if (huart->Instance->SR & USART_SR_TC)
{ //if TX complete
serial_transmit_complete_irq_handler(huart, port);
}
}

[wek]

One possible problem with this may be, that if "at" this line:

> // check if we are here because of the TC interrupt

(i.e. between the blocks checking TXE and TXC), a higher-priority interrupt kicks in and lasts more than around two frames, TC gets set and if this is RS485, it switches off the driver.

It may happen at any point so if you consistently see 1 character missing it's not likely the issue, but something worth thinking about IMO.

Also, don't RMW the status register. Again it's unlikely to be the culprit here (I don't see any mechanism how it would be), but, just don't. They are rc_w0 for a reason.

The problem may lay elsewhere, so as others said, perhaps start finding out which character is missing - first, last, whichever.

JW

[Sauli]

would that explain a data loss? ISTM that this merely produces a little gap in the transmitted data.

It may not explain the data loss, but should nevertheless be fixed. It is not too difficult to imagine a scenario where this can cause problems. For instance transmit is active while a task is writing a new message to the transmit buffer. However this task is pre-empted by a higher priority task. By the time the task resumes writing the data we are in between the last TXE interrupt and the TC interrupt. The data written to the buffer between these interrupts then remains in the buffer until the next time the buffer write routine is called. Your application may be transmitting constantly so there is only a small gap, but I think most applications only transmit occasionally and the consequences are more serious.

[Berni]

The best way to test this is to rewrite your transmitting routine into a blocking one.

This way you can fill up the buffer, then call the routine to feed it to the UART in a loop. This keeps your program functionally the same apart from the task taking longer to execute since it has to wait for the data to be pumped out. This removes all the multitasking stuff and interrupts, as well as being easier to step trough with a debugger.

Then once that works flawlessly start gradualy moving things into the multithreaded way. Once you pull interrupts into the mix debugging via JTAG becomes more difficult. So it is a good idea to start using GPIO pins as "interrupt indicators" by setting a pin to 1 at the start of the interrupt and setting it back to 0 at the end of it. This way you can hook a scope up to the pins and have a nice visual diagram of exactly when and how long interrupts are executing. The GPIO write is also very fast so it causes minimal effect on the timing.

[tggzzz]

Then once that works flawlessly start gradualy moving things into the multithreaded way. Once you pull interrupts into the mix debugging via JTAG becomes more difficult. So it is a good idea to start using GPIO pins as "interrupt indicators" by setting a pin to 1 at the start of the interrupt and setting it back to 0 at the end of it. This way you can hook a scope up to the pins and have a nice visual diagram of exactly when and how long interrupts are executing. The GPIO write is also very fast so it causes minimal effect on the timing.

Using any form of debugger a real-time system is an exercise in Heisenbugs

The GPIO concept works well provided flipping one and only one bit is fast. Some MCUs have specific instructions to flip individual io bits.

It can usefully be extended to marking the begin/end of anything interesting, e.g. put/get FIFO operations. If you can read back the state of a GPIO pin, you may find it useful to "if the bit is already set by something else, then I know I'm in trouble"; the potential problems with that are obvious! Reading the generated object code is a useful skill!

Since the event you are looking for will be rare and complex, you may find a logic analyser with good arming/trigger/filtering is necessary to spot "if...and...then...but not..." events on the pins. That excludes anything which simply waits for a one input condition, captures everything, and relies on post-processing to filter out the 99.99999% of boring captures.

[peter-h]

if this is RS485, it switches off the driver.

This is just RS232.

Also, don't RMW the status register.

Good point. I've been aware of this but this is someone else's code from ~2 years ago. But that RMW code comes from ST Cube MX's code generator AFAIK. So I changed it to this

Code: [Select]


__disable_irq();
if (!kde_serial_transmit_active[port])
{
// Transmitter is not already running so load the first byte and enable the TXE and TC interrupts
// First clear TC. This is done with a read of SR and a write to DR.
//kde_serial_port_handles[port]->Instance->SR &= ~(UART_FLAG_TC);

// This does a read of SR without doing RMW which the line above did
kde_serial_port_handles[port]->Instance->SR;

// This is a write of DR
kde_serial_port_handles[port]->Instance->DR = kde_serial_transmit_buffer[port][kde_serial_transmit_buffer_tx_index[port]++];

kde_serial_transmit_buffer_tx_index[port] %= KDE_SERIAL_TX_BUFFER_SIZE;
kde_serial_transmit_active[port] = true;

// Enable TX complete and TX empty interrupts. The order may matter!
__HAL_UART_ENABLE_IT(kde_serial_port_handles[port], UART_IT_TC);
__HAL_UART_ENABLE_IT(kde_serial_port_handles[port], UART_IT_TXE);
}
__enable_irq();

I hope that
kde_serial_port_handles[port]->Instance->SR;
does actually do a read of SR; I have used this elsewhere. A funny feature of C!

and in the TC ISR I now have

Code: [Select]


// Clear and disable the TC interrupt. This is supposed to be done by a read of SR and a write to DR
// but in this case we have nothing to write to DR so we clear the TC bit by writing a 0 to it but
// avoiding RMW - see http://efton.sk/STM32/gotcha/g9.html
huart->Instance->SR = ~(UART_FLAG_TC);
__HAL_UART_DISABLE_IT(huart, UART_IT_TC);
So far, no change to existence of the 1 byte loss. It happens once every 50-100 1k packets. But, overall the reliability has improved with every change I have done here, which is kind of weird. Normally there is only one thing causing something Removing the SR RMW ops has significantly improved reliability, practically removing errors from shorter packets (which would be more likely cases where TX gets disabled and needs re-enabling by loading a byte into DR.

I've discovered that the loss is possibly related to whether the packets sent are smaller than a particular buffer along the data path. This is currently 512 bytes. That in turn might point to something way more complex which I can't really debug.

I am now running with the above-suggested code which doesn't need ints disabled during the copy (in both tx and rx instances) and that seems to run well

Code: [Select]

uint32_t index = serial_receive_buffer_get_index[port];
data[i] = serial_receive_buffer[port][index++];
serial_receive_buffer_get_index[port] = index % SERIAL_RX_BUFFER_SIZE;

Since the event you are looking for will be rare and complex, you may find a logic analyser with good arming/trigger/filtering is necessary to spot "if...and...then...but not..." events on the pins. That excludes anything which simply waits for a one input condition, captures everything, and relies on post-processing to filter out the 99.99999% of boring captures.

A DSO is OK. I have a LeCroy 3034. That can decode RS232, SPI, etc, but is a PITA to set up for each such thing.

[tggzzz]

Since the event you are looking for will be rare and complex, you may find a logic analyser with good arming/trigger/filtering is necessary to spot "if...and...then...but not..." events on the pins. That excludes anything which simply waits for a one input condition, captures everything, and relies on post-processing to filter out the 99.99999% of boring captures.

A DSO is OK. I have a LeCroy 3034. That can decode RS232, SPI, etc, but is a PITA to set up for each such thing.

Possibly, possibly not. I'm not familiar with the arm/trigger on that, but I doubt it has any filtering. The serial decode capabilities are irrelevant. A lot will depend on whether you can predict the possible cause of the problem, detect it in software, and flip a GPIO bit when it occurs.

While I'm sure that scope is better than nothing, a good logic analyser can have beneficial capabilities for spotting rare complex patterns and complex sequences, and ignoring everything else.

[peter-h]

I've had an idea. I can't get my head around the exact operation of this function, but have a feeling that it contains some subtle gotcha.

Code: [Select]


// Get the number of bytes free in the transmit buffer (how much more data can be queued for transmission)
// Port is numbered 0 to 4, with 0 being the USB VCP

uint16_t serial_get_opqspace(uint8_t port)
{
uint16_t space = 0; // total queue size
int32_t count;

if (port > 0 && port <= SERIAL_MAX_PORT)
{
__disable_irq(); // prevent interrupts while updating the index

// Port number is 1 to 4, so -1 to offset from 0
count = (int32_t)serial_transmit_buffer_put_index[port-1] - (int32_t)serial_transmit_buffer_tx_index[port-1];

if (count < 0)
{
// put index has wrapped and is now before the tx index
count = SERIAL_TX_BUFFER_SIZE + count;
}

if (count > (SERIAL_TX_BUFFER_SIZE-1))
{
// This should never be allowed to happen
count = 0;
serial_transmit_buffer_tx_index[port-1] = 0;
serial_transmit_buffer_put_index[port-1] = 0;
}

// Space is buffer size - count -1 and the -1 is needed because full buffer = buffer_size-1
space = (SERIAL_TX_BUFFER_SIZE - count) -1;

__enable_irq();
}
else if (port == 0)
{
// USB CDC port
space = CDC_get_opqspace();
}

return space;
}

The function returns the space in a circular buffer. It does it by subtracting the difference between the two pointers, and there is no "queue size" variable which would be the other way to do it (that's how I normally do it because you get an immediate "queue size" value, you don't waste 1 byte buffer space, but you need DI/EI around it).

The disable/enable interrupts macros above were originally not present but I put them in because I could not be sure.

In particular I wonder if
count = SERIAL_TX_BUFFER_SIZE + count;
is correct and there isn't supposed to be a +1 in there.

The CDC mode above (at the bottom) uses this code to get the number of bytes in the buffer

Code: [Select]


int32_t count = (int32_t) cdc_transmit_buffer_put_index - (int32_t) cdc_transmit_buffer_tx_index;

if (count < 0) {
// tx index has wrapped and is now before the get index
count = CDC_TX_BUFFER_SIZE + count;
}

and to get space you then do

Code: [Select]

uint16_t space = CDC_TX_BUFFER_SIZE - CDC_get_opqcount() -1;
where opqcount is the CDC code above.

[paulca]

Code: [Select]

int32_t count;


[peter-h]

I don't get it

OK; maybe you mean

space = (SERIAL_TX_BUFFER_SIZE - count) -1;

where space is a uint16_t but count is a int32_t, but the real world values should never produce a wrong value for space. Or maybe they can?

If SERIAL_TX_BUFFER_SIZE == count, that would break due to the -1, but count should never exceed SERIAL_TX_BUFFER_SIZE - 1.

[Sauli]

Peter

In your reply #29 you posted this function:

Code: [Select]

// Get the number of bytes available in the receive buffer for this port
// Port is numbered 0 to 4, with 0 being the USB VCP

uint16_t serial_get_ipqcount(uint8_t port)
{
int32_t count;

portENTER_CRITICAL(); // prevent interrupts while updating the index

// Port number is 1 to 4, so -1 to offset from 0
count = (int32_t)serial_receive_buffer_rx_index[port-1] - (int32_t)serial_receive_buffer_get_index[port-1];

if (count < 0)
{
// rx index has wrapped and is now before the get index
count = SERIAL_RX_BUFFER_SIZE + count;
}

if (count > (SERIAL_RX_BUFFER_SIZE-1))
{
// This should never be allowed to happen
count = 0;
serial_receive_buffer_rx_index[port-1] = 0;
serial_receive_buffer_get_index[port-1] = 0;
}

portEXIT_CRITICAL();
}

return (uint16_t) count;
}
The comment says it returns the number of available bytes in the buffer. What it actually returns is the number of used bytes in the buffer.

The check for count > (SERIAL_RX_BUFFER_SIZE-1) is useless because the indexes can never reach SERIAL_RX_BUFFER_SIZE. Unless you suspect that somewhere somehow a bigger value is written to the indexes, as indeed happens if you keep using code like: serial_transmit_buffer[port][serial_transmit_buffer_put_index[port]++] = data;

[peter-h]

Yes; it returns the number of bytes available to be read out

I don't understand the bit about dodgy code but guess it is to do with undefined instruction sequence, no? Now I am using the other version which does not need interrupts disabled:

Code: [Select]

uint32_t index = serial_transmit_buffer_put_index[port];
serial_transmit_buffer[port][index++] = data[i];
serial_transmit_buffer_put_index[port] = index % SERIAL_TX_BUFFER_SIZE;
I've made a useful discovery: adding a byte count into the TX code wherever DR gets loaded (only 2 places) shows the right number of bytes is getting loaded.

So the UART TX DR must be loaded somewhere when TXE=0. One place is the TX ISR which is hardly possible. The other place is the TX "kick off" code, so I put in a TXE test and the missing byte is no longer missing

Code: [Select]


__disable_irq();
// This code gets run only if TX not active and the UART can accept a byte
if ( (!kde_serial_transmit_active[port]) && ((kde_serial_port_handles[port]->Instance->SR & USART_SR_TXE)!=0) )
{

// Transmitter is not already running so load the first byte and enable the TXE and TC interrupts
// First clear TC. This is done with a read of SR and a write to DR.
// This does a read of SR without doing RMW which the line above did
//serial_port_handles[port]->Instance->SR; // already done in the if( above

// This is the write of DR
serial_port_handles[port]->Instance->DR = serial_transmit_buffer[port][serial_transmit_buffer_tx_index[port]++];
tx_bytecount++;

serial_transmit_buffer_tx_index[port] %= SERIAL_TX_BUFFER_SIZE;
serial_transmit_active[port] = true;

// Enable TX complete and TX empty interrupts. The order may matter!
__HAL_UART_ENABLE_IT(serial_port_handles[port], UART_IT_TC);
__HAL_UART_ENABLE_IT(serial_port_handles[port], UART_IT_TXE);
}
__enable_irq();

I think someone above suggested this...

[nctnico]

I don't know the internals but perhaps the problem is using the TC interrupt which fires too early. I'd check which interrupt fires when the UART is really ready to accept to new character into the TX buffer and not use any other interrupt.

[peter-h]

I think the normal TX interrupt (triggered by TXE) is running OK.

This issue was in the code which restarts TX after TX "ended", which happens when you get the TXE interrupt but there is no data in the TX queue. Then you have to disable the TX interrupts, and when new TX data is available, you have to restart the TX process by re-enabling TX interrupts and placing a byte into the UART.

[nctnico]

I think the normal TX interrupt (triggered by TXE) is running OK.

This issue was in the code which restarts TX after TX "ended", which happens when you get the TXE interrupt but there is no data in the TX queue. Then you have to disable the TX interrupts, and when new TX data is available, you have to restart the TX process by re-enabling TX interrupts and placing a byte into the UART.

In such a case you'd need to load the byte into the UART first and then enable the interrupt. Otherwise the interrupt will fire immediately which creates a race condition.

[peter-h]

Yes, as in this

Code: [Select]

__disable_irq();
// This code gets run only if TX not active and the UART can accept a byte
if ( (!serial_transmit_active[port]) && ((serial_port_handles[port]->Instance->SR & USART_SR_TXE)!=0) )
{
// Transmitter is not already running so load the first byte and enable the TXE and TC interrupts
// First clear TC. This is done with a read of SR and a write to DR.
// This SR read was already done in the if() line above
// and this is the write of DR
serial_port_handles[port]->Instance->DR = serial_transmit_buffer[port][serial_transmit_buffer_tx_index[port]++];

serial_transmit_buffer_tx_index[port] %= SERIAL_TX_BUFFER_SIZE;
serial_transmit_active[port] = true;

// Enable TX complete and TX empty interrupts. The order may matter!
__HAL_UART_ENABLE_IT(serial_port_handles[port], UART_IT_TC);
__HAL_UART_ENABLE_IT(serial_port_handles[port], UART_IT_TXE);
}
__enable_irq();


[NorthGuy]

Code: [Select]

// Transmitter is not already running so load the first byte and enable the TXE and TC interrupts


What if it is running? Like you have loaded the last character from the previous transmission, done with it, and now start a new transmission. Then you'll lose the first byte.

[peter-h]

It should not be running because the serial_transmit_active flag gets cleared by the TC ISR:

Code: [Select]


void serial_transmit_complete_irq_handler(UART_HandleTypeDef *huart, uint8_t port)
{

// Transmit complete
serial_transmit_active[port-1] = false;

// Clear and disable the TC interrupt. This is supposed to be done by a read of SR and a write to DR
// but in this case we have nothing to write to DR so we clear the TC bit by writing a 0 to it but
// avoiding RMW - see http://efton.sk/STM32/gotcha/g9.html
huart->Instance->SR = ~(UART_FLAG_TC);
__HAL_UART_DISABLE_IT(huart, UART_IT_TC);

}


[Sauli]

Yes; it returns the number of bytes available to be read out

Ok, I misunderstood the comment.

I am posting here the relevant pieces of code which is my view of how this transmit stuff should be done. Experience tells me that in so many lines there is bound to be at least one error. You are welcome to point it out.

Code: [Select]

// Get the number of bytes stored in the receive buffer for this UART port.
// UART ports are numbered from 1 on (port 0 is the USB VCP).

uint16_t serial_get_ipqcount(uint8_t port)
{
int32_t count;

// These arrays are for the UART ports which are numbered starting from 1, so offset the port by -1.
count = (int32_t)serial_receive_buffer_rx_index[port-1] - (int32_t)serial_receive_buffer_get_index[port-1];

if (count < 0)
{
// rx index has wrapped and is now before the get index
count += SERIAL_RX_BUFFER_SIZE;
}

return (uint16_t)count;
}

// Get the number of free bytes in the transmit buffer for this UART port.
// UART ports are numbered from 1 on (port 0 is the USB VCP).

uint16_t serial_get_opqcount(uint8_t port)
{
int32_t count;

// These arrays are for the UART ports which are numbered starting from 1, so offset the port by -1.
count = (int32_t)serial_transmit_buffer_put_index[port-1] - (int32_t)serial_transmit_buffer_tx_index[port-1];

if (count < 0)
{
// rx index has wrapped and is now before the get index
count += SERIAL_RX_BUFFER_SIZE;
}

// Maximum number of free bytes is SERIAL_TX_BUFFER_SIZE - 1.
return (uint16_t)(SERIAL_TX_BUFFER_SIZE - count - 1);
}

/*
* Write a number of bytes to the transmit queue and start transmission if necessary
* Returns false if there is insufficient space for the given length
* Port is numbered 0 to 4, with 0 being the USB VCP
*
* The port 0 processing is different for port 0 because there is flow control on tx (to PC)
* which necessitates a circular buffer before it just like ports 1-4 have. The difference
* is that with serial ports the flow control is the baud rate whereas with port 0 is it
* how fast the PC application is reading the data out of the VCP.
*
* This function is NON blocking, if the data fits into the TX buffer, otherwise no action.
* However, it delays for the time g_485_DE_wait if the port is using RS485 mode (not always). Note
* that interrupts are disabled during this time!
*
*/

bool serial_transmit(uint8_t port, uint8_t *data, uint16_t length)
{
bool ok = false;

if (port > 0 && port <= SERIAL_MAX_PORT)
{
if (serial_get_opqspace(port) >= length)
{
// Use a local write index.
uint32_t index = serial_transmit_buffer_put_index[ --port];

// Copy the data to the circular buffer
for (uint16_t i = 0; i < length; i++)
{
serial_transmit_buffer[port][index++] = data[i];
if( index >= SERIAL_TX_BUFFER_SIZE)
{
index = 0;
}
}
// Only update the public write index after all data copied.
serial_transmit_buffer_put_index[port] = index;

// No need to trigger transmit unless transmit interrupt is disabled.
if( (serial_port_handles[port]->Instance->CR1 & UART_IT_TXE) == 0)
{
// We do not enable TC interrupt for RS485 ports here. If we did, there would be a one character time deadline
// for servicing the transmit interrupts. If that deadline was missed, there would be a TC interrupt.
// Also, we do a read-modify-write for CR1 to enable TXE interrupt. Protect that against simultaneous
// modification in interrupt service by using read/write exclusive.
while( __STREXW( __LDREXW( &serial_port_handles[port]->Instance->CR1) | UART_IT_TXE, &serial_port_handles[port]->Instance->CR1) == 1);
}

ok = true;
}
}
else if (port == 0)
{
// USB CDC port. Same as port 1-4 above but no need to mess with interrupts etc
// The circular buffer gets copied to CDC in the USB RTOS task (in main.c)

CDC_serial_transmit(data, length);
ok=true;

}

return ok;
}

void serial_transmit_irq_handler(UART_HandleTypeDef *huart, uint8_t port)
{

port--; // offset from 0

// Is there anything left to send?
if (serial_transmit_buffer_tx_index[port] == serial_transmit_buffer_put_index[port])
{
// Disable the TXEmpty interrupt
huart->Instance->CR1 &= ~(UART_IT_TXE);

// If sending to a RS485 port, enable TC interrupt.
// If we are already late, the interrupt is triggered immediately.
// The TC flag in the status register is cleared by a read SR write DR sequence.
// Therefore we do not have to clear the flag because if it was set before,
// it was cleared when the last character was written to the DR.
if( (port == 2) || (port == 3))
{
huart->Instance->CR1 |= UART_IT_TC;
}
}
else
{
// Send the next byte
huart->Instance->DR = serial_transmit_buffer[port][serial_transmit_buffer_tx_index[port]++];
if( serial_transmit_buffer_tx_index[port] >= SERIAL_TX_BUFFER_SIZE)
{
serial_transmit_buffer_tx_index[port] = 0;
}
}
}

// This ISR turns off the 485 driver on p3/p4
// https://peter-ftp.co.uk/screenshots/202210015115900110.jpg

void serial_transmit_complete_irq_handler(UART_HandleTypeDef *huart, uint8_t port)
{

if ( (port == 3) || (port == 4) )
{
// If this is an RS485 port and auto driver mode is set, disable the driver now
if (serial_driver_mode[port-1] == 2)
{
serial_485_driver_control_internal(port, 0);
}
}

// RS485 Transmit complete
// No need to use this flag for transmit interrupt enable control
// in transmit buffer write function. Other purposes for RS485
// control may exist.
serial_transmit_active[port-1] = false;

// Disable the TC interrupt
huart->Instance->CR1 &= ~(UART_IT_TC);
}


[peter-h]

You have a lot of code there - thank you! I will go through it today.

Never heard of __STREXW - an interesting method. What is the advantage over some form of DI/EI?

I have taken note of your use of TXE in CR1 as the "tx running" flag, instead of my serial_transmit_active flag. This I think is more elegant because it keeps the flag in just one place, but I am not doing that just yet because of possible effects on the RS485 driver enable operation, which was thoroughly tested in the past and which is not only difficult to test but is also configurable (one of the ports is 2 wire 485 and the other is 4 wire 422/485). I also no longer have any of that 1-byte data loss.

Everything else I implemented - thank you! Reliability is very high now.

[peter-h]

Well, I followed an implemented everything I could suggested here. The biggest difference was using the "TX int enabled" status bit as the flag which determines whether to re-enable TX ints.

Why? I don't know because the flag I used previously was only ever set/reset inside an ISR.

This suggests that the TX ISR was being interrupted by some other ISR. The 32F4 interrupt system is unbelievably complicated and I totally struggle trying to understand it. Reading stuff from others struggling online suggests I am not alone. In particular I wonder if a TX ISR for say UART2 can be interrupted by a TX ISR for UART1, or UART3. In previous CPUs I have used this was not possible; the entry to an ISR did a "DI". So often I would do an "EI" near the start of a 100Hz timer ISR which would otherwise block ints for 10ms.

But even if one UART ISR was interrupting another UART ISR, it doesn't explain it because each ISR is supplied (from the interrupt handler) with the serial port index, so all data is kept separate. Maybe there is a tiny window where things can go wrong. For example an int is cleared by reading SR and writing DR. This is inside an ISR. But if another ISR is interrupting this one, maybe ... ? In foreground code I use

__disable_irq()

which is

Code: [Select]

/**
  \brief   Disable IRQ Interrupts
  \details Disables IRQ interrupts by setting the I-bit in the CPSR.
           Can only be executed in Privileged modes.
 */
__STATIC_FORCEINLINE void __disable_irq(void)
{
  __ASM volatile ("cpsid i" : : : "memory");
}
and maybe this is needed also inside a UART ISR? That would be quite a different idea from all my previous experience.

There is still a very rare 1 byte loss, which would be nice to fix. Perhaps one every 10MB. Could be anywhere, of course, but losing exactly 1 byte is curious. But is probably in the UART stuff because one of the serial ports is a USB VCP and that runs fine. Of course USB VCP (CDC profile) is unbelievably complex too; it was only with the help of several people that I was able to fix the ex-ST USB code properly. Byte counters in various parts of the code show no loss and I have narrowed it down to the TX code. Even if I set up a data analyser to pick up a 1999 byte packet against a million 2000 byte packets, the UART is all internal to the CPU so cannot be probed. And 100% reliability on non error corrected serial comms is impossible anyway.

[cv007]

In foreground code I use

__disable_irq()

Whether its now among your set of problems or not, blindly disabling irq's without knowing the irq state will eventually lead to where you are in a disabled state for a good reason, another piece of code is called/inlined that does the same thing, and you now have irq's enabled when you thought they were disabled because all code just assumes irq's were enabled and simply enables irq's when done.

There is no way one can keep track of all the potential call paths when you arrive at your interrupt protected code, so you just do not assume irq's are enabled and instead get/restore the state of irq's to keep from breaking other code that may have called you.

In C++ I use something like the following, where the irq state is saved/restored in whatever scope I put the InterruptLock instance. It becomes easy to use and do not have to worry about who gets called inside the scope since everyone else is doing the same. Obviously C can do the same thing with a little less ease of use.

Code: [Select]

/*-----------------------------------------------------------------------------
    use to temporarily disable interrupts for various reasons
    atomic access, etc.
    constructor saves PRIMASK bit 0, then interrupts disabled
    at end of scope deconstructor restores PRIMASK
    void myfunc(int v){
        InterruptLock lock; //instance name unimportant
        shared_var += v;     //interupts off
    }
    deconstructor called at end of scope
    interrupts now restored to previous value
-----------------------------------------------------------------------------*/
class InterruptLock {
                u32 status;
public:
InterruptLock   ()
                : status( __get_PRIMASK() )
                { __disable_irq(); }

~InterruptLock  () { __set_PRIMASK(status); }
};
This is mcu 101 and is nothing new, and maybe you are already doing this but are not showing it, or you are confident you know all the potential call paths so you are happy disabling/enabling irq's without knowing its current state. Its a lot easier to save/restore irq state than it is to figure out all the potential paths you may have arrived to a particular piece of code. Again, whether its currently a problem or not, no need to potentially add more problems in the future when there is a simple solution to avoid it.