3 Replies Latest reply on Jun 8, 2020 4:40 AM by RashiV_61

    FX3 boot_lib I2C transfers do not clean up after errors correctly

    OlJo_4577026

      I have some bootloader firmware that uses the FX3 boot_lib library (v1.3.4)

       

      One of the things it does is provide read/write access to an I2C-connected EEPROM (for example, the one on the FX3 devkit board). While an EEPROM write cycle is in progress, the EEPROM will NAK further I2C commands, so when back-to-back EEPROM page writes are done the second write typically fails and needs to be retried.

       

      I've found that after writing a large batch of data, spanning many pages, to the EEPROM, the data actually written to the EEPROM has been corrupted in a very specific way. Around 3 extra duplicate bytes are being inserted, starting at the point where the first retry happens (i.e. at the start of the second page written).

       

      The EEPROM-writing code is also used in firmware that uses the full FX3 firmware library (with the appropriate I2C calls swapped out), and notably there is no corruption when that code runs with the full firmware library. It's only when the boot_lib I2C routines are used that problems happen.

       

      The code looks like this (when compiled with boot_lib, I have typedefs in place to translate the CyU3P types to their boot_fw equivalents):

       

      /* single write, caller needs to ensure the address/length don't wrap. handles retry on busy. */

      static CyU3PReturnStatus_t eeprom_write_single(uint8_t i2c_address, uint16_t address, const uint8_t *buffer, uint16_t length)

      {

          CyU3PReturnStatus_t error;

          CyU3PI2cPreamble_t preamble;

       

          preamble.buffer[0] = i2c_address & 0xFE;    /* direction bit = 0 (write) */

          preamble.buffer[1] = (address >> 8) & 0xFF; /* address high byte */

          preamble.buffer[2] = address & 0xFF;        /* address low byte */

          preamble.length = 3;                        /* three bytes of preamble to write */

          preamble.ctrlMask = 0;                      /* no additional START/STOP bits */

       

          if (!!(error = i2c_write(&preamble,          /* preamble to write */

                                   buffer,             /* data to write */

                                   length,             /* bytes to write */

                                   EEPROM_I2C_RETRIES, /* # of retries */

                                   1))) {              /* retry interval, ms */

              log_error("EEPROM I2C write", error);

              currentStatus.errorflags |= EF_EEPROM_I2C_ERROR;

          }

       

          return error;

      }

       

      /* wrapper around CyFx3BootI2cTransmitBytes that handles retries with delay

      * and retrying on NAK */

      CyU3PReturnStatus_t i2c_write(const CyU3PI2cPreamble_t *preamble, const uint8_t *buffer, uint16_t length, unsigned retries, unsigned retry_delay_ms)

      {

          debug_printf("i2c_write(%x,%u)", preamble->buffer[0], length);

          CyFx3BootErrorCode_t error;

          unsigned retry;

          for (retry = 0; retry <= retries; ++retry) {       

              error = CyFx3BootI2cTransmitBytes((CyFx3BootI2cPreamble_t *)preamble, /* preamble to write */

                                                (uint8_t *)buffer,                  /* where to write from */

                                                length,                             /* bytes to write */

                                                0);                                 /* no retries - we will handle that */

              if (error) {

                  i2c_error_recovery(); /* see below! */

              }

              if (!error || (error != CY_FX3_BOOT_ERROR_XFER_FAILURE && error != CY_FX3_BOOT_ERROR_TIMEOUT))

                  break;

       

              ++currentStatus.i2c_retries;

       

              for (unsigned ms = 0; ms < retry_delay_ms; ++ms)

                  CyFx3BootBusyWait(1000);

          }

       

          if (retry > 0) {

              debug_printf("I2C write (%x): %s after %u retries", preamble->buffer[0], errorcode_string(error), retry);

          }

       

          return error;

      }

       

      (note that the caller of eeprom_write_single ensures that each write is entirely contained within a single EEPROM page)

       

      Comparing the boot_lib I2C code with the main library's I2C code, the boot_lib code is missing an equivalent to CyU3PI2cErrorRecovery, which, notably, drains the TX and RX FIFOs following an error. I believe what's happening is that when the I2C write sees a NAK, there are still pending bytes in the transmit FIFO. These are not cleaned up, so when the write is later retried, those extra bytes are duplicated.

       

      If I copy the error-recovery logic from the full firmware library and call it on error (see the call to "i2c_error_recovery" above) then the corruption goes away and everything works as expected.

       

      Could a Cypress engineer take a look at this and confirm the problem / suggest a proper fix?

       

      Thanks!