ST I2Cv2 possible bugs

Report here problems in any of ChibiOS components. This forum is NOT for support.
steved
Posts: 825
Joined: Fri Nov 09, 2012 2:22 pm
Has thanked: 12 times
Been thanked: 135 times

ST I2Cv2 possible bugs

Postby steved » Thu Aug 25, 2016 3:03 pm

Encountered a strange problem running I2C and SPI together on a 32F091, which I think I've fixed.
I have an SPI task running autonomously on one thread - doing two 6-byte transfers every 1-5msec
I have a separate I2C-based task which does a multi-block transfer to a small memory, with some accesses interspersed of an I2C address where there is currently no device - hence these always return a MSG_RESET error, which is fine.
Every so often, the memory transfer immediately following an access to the unpopulated I2C address failed with a MSG_TIMEOUT failed. The time before this happened appeared random. It looked as if the I2C peripheral had the STOP bit set in CR1 - this is set in the ISR when a NACK is received. Changing this fixed the problem; the code I've ended up with is as follows:

Code: Select all

  /* Check for received NACK, the transfer is aborted. Do this in all modes */
  if ((isr & I2C_ISR_NACKF) != 0U)
  {
   i2cDisableReceiveOperation(i2cp);
   i2cDisableTransmitOperation(i2cp);

    /* Error flag.*/
    i2cp->errors |= I2C_ACK_FAILURE;

    /* Make sure no more interrupts. Do this before sending any STOP condition */
    dp->CR1 &= ~(I2C_CR1_TCIE | I2C_CR1_TXIE | I2C_CR1_RXIE | I2C_CR1_STOPIE);

    /* Transaction finished, send the STOP. */
    if ((isr & I2C_ISR_STOPF) == 0U)
    {
      /*
       * Only send if not already got a STOP interrupt pending - other activity can mean
       * that this ISR is delayed a bit. If we then send a STOP, it locks up the next transaction
       */
      dp->CR2 |= I2C_CR2_STOP;
    }
      /*
       * Wake up thread in master mode.
       * It's debatable whether we should do an error wakeup or a normal wakeup.
       * If no data has been transferred, its definitely an error because the addressed
       * device isn't responding.
       * If all data has been transferred, maybe its OK and the slave hasn't ACKed because it
       * is signalling that its received enough
       */
//      _i2c_wakeup_isr(i2cp);          // This is a normal completion
      _i2c_wakeup_error_isr(i2cp);          // Treat as error - changed 25.08.16
 }


This also reorders things a little.
The 'error wakeup' is necessary to generate an error when an addressed slave doesn't respond at all. As per the comment, it may possibly need to be cleverer, since I believe some peripherals give a NACK when they've received all the data they want.

Note: The problem may be aggravated by the SPI defaulting to a higher interrupt priority than the I2C, but I think the fundamental thing is that the I2C interrupt can be delayed sufficiently to cause a problem.

Note: The problem doesn't appear at all if I remove the accesses to the unpopulated I2C address, so won't be very noticeable in most normal operation.


Something else I noticed, but may not normally be a problem (its not in my version of the I2C driver!) is the ordering of some of the actions in i2c_lld_master_transmit_timeout() and i2c_lld_master_receive_timeout(). The DMA/buffer is currently set up before waiting for the peripheral to cease to be busy; should this not be the other way round?

Code: Select all


/* Check for busy first */

  /* Calculating the time window for the timeout on the busy bus condition.*/
  start = osalOsGetSystemTimeX();
  end = start + OSAL_MS2ST(STM32_I2C_BUSY_TIMEOUT);

  /* Waits until BUSY flag is reset or, alternatively, for a timeout
     condition.*/
  while (true) {
    osalSysLock();

    /* If the bus is not busy then the operation can continue, note, the
       loop is exited in the locked state.*/
    if ((dp->ISR & I2C_ISR_BUSY) == 0)
      break;

    /* If the system time went outside the allowed window then a timeout
       condition is returned.*/
    if (!osalOsIsTimeWithinX(osalOsGetSystemTimeX(), start, end)) {
      return MSG_TIMEOUT;
    }

    osalSysUnlock();
  }

/* Set up DMA/buffer now device isn't busy */

#if STM32_I2C_USE_DMA == TRUE
  /* TX DMA setup.*/
  dmaStreamSetMode(i2cp->dmatx, i2cp->txdmamode);
  dmaStreamSetMemory0(i2cp->dmatx, txbuf);
  dmaStreamSetTransactionSize(i2cp->dmatx, txbytes);

  /* RX DMA setup, note, rxbytes can be zero but we write the value anyway.*/
  dmaStreamSetMode(i2cp->dmarx, i2cp->rxdmamode);
  dmaStreamSetMemory0(i2cp->dmarx, rxbuf);
  dmaStreamSetTransactionSize(i2cp->dmarx, rxbytes);
#else
  i2cp->txptr   = txbuf;
  i2cp->txbytes = txbytes;
  i2cp->rxptr   = rxbuf;
  i2cp->rxbytes = rxbytes;
#endif


User avatar
Giovanni
Site Admin
Posts: 14455
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 1076 times
Been thanked: 922 times
Contact:

Re: ST I2Cv2 possible bugs

Postby Giovanni » Mon Oct 03, 2016 11:13 am

Bumped.

I have yet to look into this, do you have news?

Giovanni

steved
Posts: 825
Joined: Fri Nov 09, 2012 2:22 pm
Has thanked: 12 times
Been thanked: 135 times

Re: ST I2Cv2 possible bugs

Postby steved » Mon Oct 10, 2016 9:43 am

Its a bit difficult to give definitive answers because I'm running my own I2C drivers which add slave mode and other features. Certainly they seem to be running fine. The change relating to the STOP bit definitely fixed the problem I was encountering. I've attached the actual driver file I'm using in case that helps.
Attachments
I2Cv2.zip
STM32 driver supporting master and slave modes
(18.48 KiB) Downloaded 162 times

User avatar
Giovanni
Site Admin
Posts: 14455
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 1076 times
Been thanked: 922 times
Contact:

Re: ST I2Cv2 possible bugs

Postby Giovanni » Sun Nov 10, 2019 10:28 am

Could you recommend focused changes to the current driver? your driver contains a lot of changes.

Giovanni


Return to “Bug Reports”

Who is online

Users browsing this forum: No registered users and 10 guests