msdk icon indicating copy to clipboard operation
msdk copied to clipboard

I2C double read err (RESTART-mode)

Open yoshihiko-niino opened this issue 4 months ago • 8 comments

Hello.

I'm reposting this as a new issue. #1394 mentions an I2C double read error. This issue was resolved by replacing i2c_reva.c, but that alone doesn't seem to be a complete solution. I'll explain it below.

The "START" → "STOP" condition alone is not a problem, but when communicating using "RESTART," the address is incremented and data is output at the subsequent stage.

(Example) START → Address 0x56 (W) → Register Address: 0x10,0x00 RESTART → Address 0x56 (R) → 0x07,0x6A ← As expected RESTART → Address 0x56 (R) → 0x00,0x01 ← For some reason, the data at the next address is displayed. STOP

I think some kind of workaround is needed, such as preventing the BUSY check from passing when "RESTART" occurs.

I tried the following solution, which distinguishes between START and RESTART and ignores RESTART. This appears to resolve the issue. However, I'm not sure if there are any side effects.

Fixed lines 883 and onward.

/* ************************************************************************* / / Transaction level functions / / ************************************************************************* */

int MXC_I2C_RevA_MasterTransaction(mxc_i2c_reva_req_t *req) { mxc_i2c_reva_regs_t *i2c = req->i2c; // Save off pointer for faster access unsigned int written = 0; unsigned int read = 0;

int is_first_start = !(req->restart);  // restart==0 のとき最初のSTARTとみなす

if (req->addr > MXC_I2C_REVA_MAX_ADDR_WIDTH) {
    return E_NOT_SUPPORTED;
}

if (MXC_I2C_GET_IDX((mxc_i2c_regs_t *)i2c) < 0) {
    return E_BAD_PARAM;
}

if (!(i2c->ctrl & MXC_F_I2C_REVA_CTRL_MST_MODE)) {
    return E_BAD_STATE;
}

// Check and return BUSY only if this is the first START
if (is_first_start && (i2c->status & MXC_F_I2C_REVA_STATUS_BUSY)) {
    return E_BUSY;
}

// if(!read | write)
//  Start
//  send addr w/ write bit
// if(Write)
//  send tx_len data
//  return if error (or NACK)
// if(Read)
//  if(Write)
//   send restart
//  else
//   send start
//  send addr w/ read bit
//  read rx_len bytes acking all
// stop or restart
// return good or error

MXC_I2C_ClearFlags((mxc_i2c_regs_t *)i2c, MXC_I2C_REVA_INTFL0_MASK,
                   MXC_I2C_REVA_INTFL1_MASK); // Clear all I2C Interrupts
MXC_I2C_ClearTXFIFO((mxc_i2c_regs_t *)i2c);
MXC_I2C_ClearRXFIFO((mxc_i2c_regs_t *)i2c);
i2c->inten0 = 0;
i2c->inten1 = 0;

if ((req->rx_len == 0) || (req->tx_len != 0)) {
    // Load the slave address with write bit set
    i2c->fifo = (req->addr << 1) & ~0x1;
    i2c->mstctrl |= MXC_F_I2C_REVA_MSTCTRL_START;
}

while (req->tx_len > written) {
    if (i2c->intfl0 & MXC_F_I2C_REVA_INTFL0_TX_THD) {
        written += MXC_I2C_WriteTXFIFO((mxc_i2c_regs_t *)i2c, &req->tx_buf[written],
                                       req->tx_len - written);
        i2c->intfl0 = MXC_F_I2C_REVA_INTFL0_TX_THD;
    }

    if (i2c->intfl0 & MXC_I2C_REVA_ERROR) {
        req->tx_len = written;
        MXC_I2C_Stop((mxc_i2c_regs_t *)i2c);
        return E_COMM_ERR;
    }
}

MXC_I2C_ClearFlags((mxc_i2c_regs_t *)i2c,
                   MXC_F_I2C_REVA_INTFL0_DONE | MXC_F_I2C_REVA_INTFL0_RX_THD, 0);

if (req->rx_len != 0) {
    if (req->rx_len > MXC_I2C_REVA_MAX_FIFO_TRANSACTION) {
        i2c->rxctrl1 = 0;
    } else {
        i2c->rxctrl1 = req->rx_len; // 0 for 256, otherwise number of bytes to read
    }

    MXC_I2C_Start((mxc_i2c_regs_t *)i2c); // Start or Restart as needed

    while (i2c->mstctrl & MXC_F_I2C_REVA_MSTCTRL_RESTART) {}

    i2c->fifo = (req->addr << 1) | 0x1; // Load slave address with read bit.
}

if ((req->rx_len != 0) && (req->tx_len != 0)) {
    while (!(i2c->intfl0 & MXC_F_I2C_REVA_INTFL0_DONE)) {}
    // Wait for Transaction to finish

    i2c->intfl0 |= MXC_F_I2C_REVA_INTFL0_DONE;
}

while (req->rx_len > read) {
    if (i2c->intfl0 & (MXC_F_I2C_REVA_INTFL0_RX_THD | MXC_F_I2C_REVA_INTFL0_DONE)) {
        read +=
            MXC_I2C_ReadRXFIFO((mxc_i2c_regs_t *)i2c, &req->rx_buf[read], req->rx_len - read);
        i2c->intfl0 = MXC_F_I2C_REVA_INTFL0_RX_THD;
    }

    if (i2c->intfl0 & MXC_I2C_REVA_ERROR) {
        req->rx_len = read;
        MXC_I2C_Stop((mxc_i2c_regs_t *)i2c);
        return E_COMM_ERR;
    }

    if ((i2c->intfl0 & MXC_F_I2C_REVA_INTFL0_DONE) && (req->rx_len > read) &&
        (MXC_I2C_RevA_GetRXFIFOAvailable(i2c) == 0)) {
        if ((req->rx_len - read) > MXC_I2C_REVA_MAX_FIFO_TRANSACTION) {
            i2c->rxctrl1 = 0;
        } else {
            i2c->rxctrl1 = (req->rx_len - read); // 0 for 256, otherwise number of bytes to read
        }

        i2c->mstctrl |= MXC_F_I2C_REVA_MSTCTRL_RESTART;
        i2c->intfl0 = MXC_F_I2C_REVA_INTFL0_DONE;
        i2c->fifo = (req->addr << 1) | 0x1; // Load slave address with read bit.
    }
}

if (req->restart) {
    i2c->mstctrl |= MXC_F_I2C_REVA_MSTCTRL_RESTART;
} else {
    i2c->mstctrl |= MXC_F_I2C_REVA_MSTCTRL_STOP;

    while ((i2c->mstctrl & MXC_F_I2C_REVA_MSTCTRL_STOP)) {}
    // Wait for Transaction to finish
}

while (!(i2c->intfl0 & MXC_F_I2C_REVA_INTFL0_DONE)) {}
// Wait for Transaction to finish

i2c->intfl0 = MXC_F_I2C_REVA_INTFL0_DONE | MXC_F_I2C_REVA_INTFL0_STOP;

if (i2c->intfl0 & MXC_I2C_REVA_ERROR) {
    return E_COMM_ERR;
}

return E_NO_ERROR;

}

Thank you.

yoshihiko-niino avatar Aug 04 '25 00:08 yoshihiko-niino

Hello, Can you also attach your example code in the main, and then I can reproduce the issue from my side.

Rgds,

dnguye14-adi avatar Aug 11 '25 07:08 dnguye14-adi

Dear @dnguye14-adi -san

Thank you for contacting us. The relevant source code is attached below. The function "MXC_I2C_RevA_MasterTransaction" attached earlier is called.

/**

  • @brief I2C 受信

  • @param [in] device_addr デバイスアドレス

  • @param [in] register_addr レジスタアドレス 2Byte

  • @param [out] rx_buff 受信データのバッファ

  • @param [in] rx_len 受信データのデータ長

  • @param [in] stopBit ストップビット 停止の場合はtrue、繰り返し開始の場合はfalseを指定

  • @return 0:正常、0以外:異常

  • @details 指定したサイズのデータを読み込む / int i2cRead(unsigned int device_addr, uint16_t register_addr, uint8_t rx_buff, unsigned int rx_len, bool stopBit) { //! ライブラリの不具合により、8バイト以上Readすると //! ライブラリ内部で固まってしまい、帰ってこなくなる現象が発生していた

    //! この現象はライブラリの下記ファイルを差し替えることで改善した //! MaximSDK/Libraries/PeriphDrivers/Source/I2C/i2c_reva.c

    int ret = E_NO_ERROR;

    //! レジスタアドレスを送信データに指定 tx_buff[0] = register_addr & 0xFF; tx_buff[1] = register_addr >> 8;

    //! I2C設定 reqMaster.addr = device_addr; reqMaster.tx_buf = tx_buff; reqMaster.tx_len = TX_ADDR_LEN; reqMaster.rx_buf = rx_buff; reqMaster.rx_len = rx_len;

    if (stopBit == true) { //! 停止の場合は0 reqMaster.restart = 0; } else { //! 繰り返し開始の場合は0以外を指定 reqMaster.restart = 1; }

    //! I2C ret = MXC_I2C_MasterTransaction(&reqMaster);

    if (ret != E_NO_ERROR) { printf("I2C Read Error. Code = %d\n", ret); }

    return ret; }

Is this what you wanted?

Best Regards, Yoshihiko Niino

yoshihiko-niino avatar Aug 18 '25 02:08 yoshihiko-niino

Hello.

I'm reposting this as a new issue. #1394 mentions an I2C double read error. This issue was resolved by replacing i2c_reva.c, but that alone doesn't seem to be a complete solution. I'll explain it below.

The "START" → "STOP" condition alone is not a problem, but when communicating using "RESTART," the address is incremented and data is output at the subsequent stage.

(Example) START → Address 0x56 (W) → Register Address: 0x10,0x00 RESTART → Address 0x56 (R) → 0x07,0x6A ← As expected RESTART → Address 0x56 (R) → 0x00,0x01 ← For some reason, the data at the next address is displayed. STOP

What is the data you are expecting after second restart? Same as previous (0x07,0x6A)? Also, can you share the name of the IC you are trying to communicate with, if possible?

ttmut avatar Aug 18 '25 07:08 ttmut

hi, @ttmut -san

Thank you for contacting us. The hardware configuration is "MAX78000" - "IQS9150 (AZOTEQ)". Device address: 0x56 Register address: 0x1000 Read data: 0x076A (Product Number)

I have also inquired about a similar issue, #1416. Thank you.

Best Regards, Yoshihiko Niino

yoshihiko-niino avatar Aug 18 '25 08:08 yoshihiko-niino

Hi @yoshihiko-niino -san,

Isn't this how most I2C devices operate? After each read operation, the internal address pointer is incremented, so for the next read operation, the value from the incremented address is returned.

ttmut avatar Aug 19 '25 07:08 ttmut

hi, @ttmut -san

Thank you for contacting us. I'm not sure if this type of sequence is common, but it does exist in the I2C standard. See the following: https://www.nxp.com/docs/en/user-guide/UM10204.pdf (see page 14) So, we need to be prepared to handle cases like this. Thank you.

Best Regards, Yoshihiko Niino

yoshihiko-niino avatar Aug 19 '25 08:08 yoshihiko-niino

hi @yoshihiko-niino -san, i saw that your code can only work with the MAX78000, but was failed on another boards (eg. MAX32690 ). So, i need more time to get a solution for all the boards.

brgs,

dnguye14-adi avatar Sep 04 '25 02:09 dnguye14-adi

Hi,@dnguye14-adi-san

Thank you for your reply. I understand the content. I apologize for the inconvenience, but please continue to consider this in order to find the optimal solution. Thank you.

Best Regards, Yoshihiko Niino

yoshihiko-niino avatar Sep 04 '25 04:09 yoshihiko-niino