[DONE] Serial port losing data when several threads are acti

[Giovanni]

Hi,

It is possible it is related to the internal USART state machine, it is possible it requires a full frame after a reconfiguration.

About the previous problem, I found several workaround but still have to understand the real cause, for example:

Removing optimizations from JUST the ISR eliminates the problem:

Code: Select all

__attribute((optimize("O0")))
static void serve_interrupt(SerialDriver *sdp) {

Removing this (apparently unrelated) snippet from the ISR also eliminates the problem:

Code: Select all

#if 0
  if (sr & USART_SR_LBD) {
    chSysLockFromIsr();
    chnAddFlagsI(sdp, SD_BREAK_DETECTED);
    chSysUnlockFromIsr();
    u->SR &= ~USART_SR_LBD;
  }
#endif

I am narrowing it but it is very elusive.

Giovanni

[Giovanni]

Ok, my conclusions.

Probably we are hitting a compiler bug here, the problem is only present in -O2 and -O3, -Os, -O1 and -O0 are not affected.

It would be a good idea to test the problem with other compiler versions so I am not committing the workaround yet on the repository, you can paste the following code in serial_lld.c if you want to test the fix:

Code: Select all

/**
 * @brief   Error handling routine.
 *
 * @param[in] sdp       pointer to a @p SerialDriver object
 * @param[in] sr        USART SR register value
 */
static void set_error(SerialDriver *sdp, uint16_t sr) {
  flagsmask_t sts = 0;

  if (sr & USART_SR_ORE)
    sts |= SD_OVERRUN_ERROR;
  if (sr & USART_SR_PE)
    sts |= SD_PARITY_ERROR;
  if (sr & USART_SR_FE)
    sts |= SD_FRAMING_ERROR;
  if (sr & USART_SR_NE)
    sts |= SD_NOISE_ERROR;
  chnAddFlagsI(sdp, sts);
}

/**
 * @brief   Common IRQ handler.
 *
 * @param[in] sdp       communication channel associated to the USART
 */
static void serve_interrupt(SerialDriver *sdp) {
  USART_TypeDef *u = sdp->usart;
  uint16_t cr1 = u->CR1;
  uint16_t sr = u->SR;

  /* Special case, LIN break detection.*/
  if (sr & USART_SR_LBD) {
    chSysLockFromIsr();
    chnAddFlagsI(sdp, SD_BREAK_DETECTED);
    chSysUnlockFromIsr();
    u->SR &= ~USART_SR_LBD;
  }

  /* Data available.*/
  chSysLockFromIsr();
  while (sr & USART_SR_RXNE) {
    /* Error condition detection.*/
    if (sr & (USART_SR_ORE | USART_SR_NE | USART_SR_FE  | USART_SR_PE))
      set_error(sdp, sr);
    sdIncomingDataI(sdp, u->DR);
    sr = u->SR;
  }
  chSysUnlockFromIsr();

  /* 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);
      u->CR1 = (cr1 & ~USART_CR1_TXEIE) | USART_CR1_TCIE;
    }
    else
      u->DR = b;
    chSysUnlockFromIsr();
  }

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

Just replace the existing functions with the above ones and everything should be fine.

The fix involved just moving chSysLockFromIsr() and chSysUnlockFromIsr() out of that while loop so there must be some reordering problem involved. I could tell by disabling the GCC optimizations (the ones added between -O1 and -O2) one by one but that would take a lot of time.

BTW, I also modified the code to include the defective frame in the input queue in case of errors, this should improve error handling of some protocols, discarding a frame could trigger a long timeout instead of just a checksum/crc check.

Giovanni

[trepidacious]

Ah, that's a pain, I guess compiler bugs are always the hardest ones to find, great to have a workaround.

I just tried to test the modification, but I'm having trouble getting the fault to occur again even with the original code and compiler, I must have changed something... I noticed the fix is in trunk as well, so I'll update and try my 1-wire code again, thanks again