Loss of UART data

[jpearman]

I've been experiencing intermittent loss of receive data when using the UARTV1 driver on an STM32F103. I normally use ChibiOS V2.6.1 but have also tried the updated driver that's part of V2.6.2 to address bug #442, this version has the same issue.

After investigating, I find that the technique to reset the transmission complete status in the SR register can cause problems. This line of code

Code: Select all

u->SR &= ~USART_SR_TC;

becomes this assembly language with my usual optimization level (-O2)

Code: Select all

292 00c4 2B88           ldrh   r3, [r5, #0]
 293 00c6 23F04003       bic   r3, r3, #64
 294 00ca 1B04           lsls   r3, r3, #16
 295 00cc 1B0C           lsrs   r3, r3, #16
 296 00ce 2B80           strh   r3, [r5, #0]   @ movhi

It's possible that the RXNE bit can be set between the initial read of the SR register and it being set again, this results in loss of one character as the RXNE bit is cleared to 0.

My current solution is to use the alternative way of resetting the TC bit in the status register, read status followed by data register write, testing of this technique is ongoing.

Code: Select all

  /* Physical transmission end.*/
  if (sr & USART_SR_TC) {
    chSysLockFromIsr();
    chnAddFlagsI(sdp, CHN_TRANSMISSION_END);
    sr = u->SR;
    u->DR = 0;
    chSysUnlockFromIsr();
    u->CR1 = cr1 & ~USART_CR1_TCIE;
    //u->SR &= ~USART_SR_TC;
  }

Hopefully you can confirm this issue and correct in V2.6.2 when released.

Regards

James.

[jpearman]

Writing 0 to the data register was obviously a bad idea.

I tried reset of the TC bit using bit banding but seems like there is some problem with that or I don't understand exactly how it works. So in the end I just leave the TC bit alone and only check for it if the interrupt is enabled. There's also a small change to the transmit code involving the cr1 register. Final code as follows.

Code: Select all

  /* Transmission buffer empty.*/
  if ((cr1 & USART_CR1_TXEIE) && (sr & USART_SR_TXE)) {
    msg_t b;
    chSysLockFromIsr();
    b = chOQGetI(&sdp->oqueue);
    if (b < Q_OK) {
      chnAddFlagsI(sdp, CHN_OUTPUT_EMPTY);
      cr1 = (cr1 & ~USART_CR1_TXEIE) | USART_CR1_TCIE;
      u->CR1 = cr1;
    }
    else
      u->DR = b;
    chSysUnlockFromIsr();
  }

  /* Physical transmission end.*/
  if((cr1 & USART_CR1_TCIE) && (sr & USART_SR_TC)) {
    chSysLockFromIsr();
    chnAddFlagsI(sdp, CHN_TRANSMISSION_END);
    u->CR1 = cr1 & ~USART_CR1_TCIE;
    // use bit band access to clear TC flag (bit 6)
    //uint32_t *p = (uint32_t *)((PERIPH_BB_BASE + (((uint32_t)&u->SR - PERIPH_BASE)<<5)) + (6<<2));
    //*p = 0;
    chSysUnlockFromIsr();
    //u->SR &= ~USART_SR_TC;
  }

[Giovanni]

Hi,

Thanks for analyzing the problem, I think the issue is really the line you mentioned:

Code: Select all

    u->CR1 = cr1 & ~USART_CR1_TCIE;
    u->SR &= ~USART_SR_TC;

That &= implies a read-modify-write operation and it is there that the race condition is. It could be changed in:

Code: Select all

    u->CR1 = cr1 & ~USART_CR1_TCIE;
    u->SR = ~USART_SR_TC;

We need just to clear the TC flag so the read-modify-write is not required. Could you please verify if this fixes the problem for you?

If the problem is solved I will include the fix in 2.6.2, to be released during next weekend.

Giovanni

[jpearman]

If the problem is solved I will include the fix in 2.6.2, to be released during next weekend.

Code: Select all

u->SR = ~USART_SR_TC;

This works just as well as the modified code I posted yesterday, no dropped characters.

You may want to use the same technique to reset the LBD bit in the break detection section.

I'm also still a little concerned that the CR1 register may be incorrectly written, interrupt enable flags are changed in the TXE section of code.

Code: Select all

      u->CR1 = (cr1 & ~USART_CR1_TXEIE) | USART_CR1_TCIE;

and then again in the TC detect section.

Code: Select all

    u->CR1 = cr1 & ~USART_CR1_TCIE;

but the cr1 variable was not updated between the two, I guess the assumption is that only one of the two lines will execute in one pass through the interrupt function.

Regards

[Giovanni]

Hi,

I also addressed the above points in repository, I don't want to make that assumption that only one of the two is executed in a single ISR pass.

Giovanni