esp-idf icon indicating copy to clipboard operation
esp-idf copied to clipboard

ULP-RISCV I2C does not work when reading/writing multiple bytes (IDFGH-11056)

Open ESP-Marius opened this issue 2 years ago • 18 comments

Answers checklist.

  • [X] I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
  • [X] I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
  • [X] I have searched the issue tracker for a similar issue and not found a similar issue.

IDF version.

master 5.2.2

Operating System used.

Linux

How did you build your project?

Command line with idf.py

If you are using Windows, please specify command line type.

None

Development Kit.

ESP32S3-PICO-1

Power Supply used.

External 3.3V

What is the expected behavior?

I expected to be able to read multiple bytes with ulp_riscv_i2c_master_read_from_device

What is the actual behavior?

The first byte read (or write) works, but he second one fails with those errors,

Steps to reproduce.

 ulp_riscv_i2c_master_set_slave_addr( handle.slave_address );
#if 0
    for( int i = 0; i < data_len; i++ ){
        ulp_riscv_i2c_master_set_slave_reg_addr( mem_address + i );
        ulp_riscv_i2c_master_read_from_device( &data[i], 1 );
        ESP_LOGI( TAG, "multi: 0x%x, reg: 0x%x, READ: 0x%x", handle.slave_address, mem_address + i, data[i] );
    }
#else    
    // FAILS
    ulp_riscv_i2c_master_set_slave_reg_addr( mem_address );
    ulp_riscv_i2c_master_read_from_device( data, data_len );
    ESP_LOGI( TAG, "multi: 0x%x, reg: 0x%x, READ: 0x%x", handle.slave_address, mem_address, data[0] );
#endif

Debug Logs.

No response

More Information.

No response

ESP-Marius avatar Sep 13 '23 01:09 ESP-Marius

Issue split from https://github.com/espressif/esp-idf/issues/12214

ESP-Marius avatar Sep 13 '23 01:09 ESP-Marius

@aircable I'm following up from the issue you reported here. May I ask which I2C sensor are you using for the multi-byte read/write test? Also, would it be possible to try out with the IDF example which uses a BMP180 sensor, just to rule out any setup related issues. Thanks!

sudeep-mohanty avatar Sep 13 '23 07:09 sudeep-mohanty

Of course, the sensor is the KXTJ3 accelerometer. It is able to automatic advance the address and return consecutive results as you can see when using the I2C driver (works fine) and not the RTC hardware. Sorry I don't have a BMP180 but I bet it has the same issue. Can you please decode the register bits in the error message? It is a bit difficult for me not knowing the internals of the hardware.

BTW it appears that when using the ULP for multiple data read it also fails, but I cannot see any error messages, obviously. The second byte read is always zero.

aircable avatar Sep 13 '23 17:09 aircable

Thanks for the reply. I don't have the KXTJ3 at hand but I'll try to scour for one or something similar to reproduce the issue.

The error message E (298) ulp_riscv_i2c: RTC I2C Interrupt Raw Reg 0x154 indicates that there is a Time out error. Meaning the I2C transaction timeout. See the register bits here.

In the meantime, what would give me some more insight is a logic analyzer capture in case you have one at hand. Basically, what I would like to know is if the sensor is indeed sending the second byte during the read operation and if the driver is failing to capture it and then reports an error.

sudeep-mohanty avatar Sep 13 '23 18:09 sudeep-mohanty

Sorry, not yet. Don't have a LA, my Osci may do it, but the hardware is too much integrated. But, when using the functions on the ULP directly:

        ulp_riscv_i2c_master_set_slave_reg_addr( KXTJ3_ZOUT_L );
        ulp_riscv_i2c_master_read_from_device( acc_z, 2 ); 

multiple register read works just fine. Only on the Xtensa processor, I have those problems. And it is verifiable.

aircable avatar Sep 18 '23 01:09 aircable

I meet a lot of problems in ULP-RISCV I2C. Using ULP-RISCV I2C on the XTENSA core, main processor on an ESP32-S3 always get timeout error 0x107 return from ulp_riscv_i2c_wait_for_interrupt() in line 352 of components/ulp/ulp_riscv/ulp_riscv_i2c.c . I find a possible reason that vTaskDelay(1) is too long in line 367 of the same file. vTaskDelay(1) means 10ms with #define CONFIG_FREERTOS_HZ 100. But in ULP-RISCV I2C one byte is about 0.1ms in CLK 100khz. I edit vTaskDelay(1); to esp_rom_delay_us(1); , then no error display. However, ULP-RISCV I2C may make error accidentally,then in next read cmd all bytes readed in wrong place. The 2rd byte readed is the 1st byte of right data. After a ulp_riscv_i2c_master_write_to_device(), ulp_riscv_i2c_master_read_from_device() become right. Maybe in the begain of ulp_riscv_i2c_master_read_from_device() doesn't clear the read buffer.

The I2C slave device or sensor works correctly with the standard I2C master on Espressif SoCs. SoCs: ESP-S3FN8 I2C slave device: RX8025 idf version: ESP-IDF v5.1-rc1-dirty ide: vscode os: win10

shangke1988 avatar Sep 19 '23 13:09 shangke1988

Thanks @aircable @shangke1988 for sharing the details. I can confirm that I can reproduce the issue of multiple byte read/write failing on the BMP180. At the moment, the workaround of doing the read/write in a loop of single-byte transactions should work. I shall investigate more to see if a better solution can be made as the HW is a bit limited in terms of capabilities.

sudeep-mohanty avatar Sep 19 '23 16:09 sudeep-mohanty

Hi @sudeep-mohanty, thanks for confirming. With respect to FreeRTOS, I always run with 1kHz and tickless idle. Please note that the multiple read/write when running on the ULP is working fine. It is very difficult to use the I2C from both sides, the Xtensa and the RiscV. Both are independent and even with riscv_lock semaphores or riscv_halt/reset it happens when both processors start doing I2C at the same time, consider startup procedures e.g. That is when things get weird. I have moved all I2C code to the ULP, to make sure. The only thing I would want is the ulp_riscv_i2c_master_init() to be available on the ULP side and not on the main processor. Maybe it is not possible, but it would make things cleaner. Maybe.

aircable avatar Sep 19 '23 16:09 aircable

Any progress so far on this issue? I am running into problems as soon as I try to write multiple bytes in one go using ulp_riscv_i2c_master_write_to_device. This code:

uint8_t config_values[6] = {0b11111110, 0b11111111, 0b01111110, 0b11110010, 0b00000000, 0b00000000}; ulp_riscv_i2c_master_set_slave_reg_addr(0x02); ulp_riscv_i2c_master_write_to_device(config_values, 6);

causes my ESP32-S3 to initiate this I2C communication: 0xB0,W,0x02 0xFE 0xFE 0xFE 0xFE 0xFE 0xFE (intercepted with logic analyzer). So it always just repeats the first byte for the number of bytes set with "size". I have confirmed this by setting different values for byte [1].

Is there any solution other than to revert to sending the data bytewise? I would like to avoid the overhead that comes with this.

Ethapus avatar Apr 26 '24 09:04 Ethapus

Pull latest version and many i2c bugs are solved

martijnED avatar Apr 26 '24 09:04 martijnED

Thank you for your reply, but I am on ESP-IDF v5.2.1. Or are you referring to the development master branch?

Edit: I also found release v5.3, didn't know that existed. Will see how it goes, thank you.

Ethapus avatar Apr 26 '24 15:04 Ethapus

I am now using the latest master branch, but sadly the issue remains.

I noticed that when I include "esp-idf\components\ulp\ulp_riscv\include\ulp_riscv_i2c.h" in my code, the called functions such as "ulp_riscv_i2c_master_write_to_device" are linked to "esp-idf\components\ulp\ulp_riscv\ulp_core\ulp_riscv_i2c.c", instead of to "esp-idf\components\ulp\ulp_riscv\ulp_riscv_i2c.c". Is this intended? If so, what is the other ulp_riscv_i2c.c for?

Ethapus avatar Apr 27 '24 18:04 Ethapus

Hello @Ethapus, Thank you for poking us on this long standing issue. And apologies for not getting to it earlier. I'd like to answer your queries below:

I am now using the latest master branch, but sadly the issue remains.

Unfortunately, the issue of RTC_I2C failing to perform multiple bytes read/write operations exists on the mainline as well as the release branches currently.

I noticed that when I include "esp-idf\components\ulp\ulp_riscv\include\ulp_riscv_i2c.h" in my code, the called functions such as "ulp_riscv_i2c_master_write_to_device" are linked to "esp-idf\components\ulp\ulp_riscv\ulp_core\ulp_riscv_i2c.c", instead of to "esp-idf\components\ulp\ulp_riscv\ulp_riscv_i2c.c". Is this intended? If so, what is the other ulp_riscv_i2c.c for?

The functions in components/ulp/ulp_riscv/ulp_riscv_i2c.c get linked to your main core application binary (Xtensa core) and provide you with the ability to use the RTC_I2C peripheral. Please note that this peripheral is different from the regular I2C peripheral on the esp32s2/s3 and functions in the RTC clock domain, primarily intended to be used by the ULP RISC-V core when the main core is in sleep mode. The functions in components/ulp/ulp_riscv/ulp_core/ulp_riscv_i2c.c link to your ULP RISC-V core binary and provide the ULP RISC-V core with APIs to use the same RTC_I2C peripheral.

Finally, after debugging the multi-byte read/write issue further, I could see that the Xtensa core APIs are the ones which get stuck doing the multi-byte read or write. This seems to be because of non-atomic peripheral operations. The following patch adds atomicity to the APIs and in my tests I could see successful multi-byte reads/writes. Additionally, I found that the ULP RISC-V core APIs do not suffer from this problem. Before I could finalize the bugfix, it would be great if you could test it out at your end and let me know if it fixes your problem. The following patch is generated on the latest master branch but should also be applicable to any previous release branches as well. Thank you!

rtc_i2c_multi_byte.patch

sudeep-mohanty avatar Apr 29 '24 10:04 sudeep-mohanty

Thank you very much for looking into the matter! Sadly applying the patch fails on my master branch esp-idf. Here is the CMD log:

C:\Users\johnl\esp\master\esp-idf>git pull remote: Enumerating objects: 404, done. remote: Counting objects: 100% (404/404), done. remote: Compressing objects: 100% (128/128), done. remote: Total 404 (delta 242), reused 399 (delta 242), pack-reused 0 Receiving objects: 100% (404/404), 351.15 KiB | 5.32 MiB/s, done. Resolving deltas: 100% (242/242), completed with 56 local objects. From https://github.com/espressif/esp-idf 8f44525dd8..c1f02f6f09 release/v4.4 -> origin/release/v4.4 0ed5284eb9..961ca4f975 release/v5.0 -> origin/release/v5.0 b43fc4d63a..30fce03e35 release/v5.3 -> origin/release/v5.3 Fetching submodule components/bt/controller/lib_esp32 From https://github.com/espressif/esp32-bt-lib 44341b1..43ecd22 master -> origin/master Already up to date.

C:\Users\johnl\esp\master\esp-idf>git apply rtc_i2c_multi_byte.patch -v Checking patch components/ulp/ulp_riscv/ulp_riscv_i2c.c... error: while searching for: /* Read/Write timeout (number of iterations)*/ #define ULP_RISCV_I2C_RW_TIMEOUT CONFIG_ULP_RISCV_I2C_RW_TIMEOUT

static esp_err_t i2c_gpio_is_cfg_valid(gpio_num_t sda_io_num, gpio_num_t scl_io_num) { /* Verify that the SDA and SCL GPIOs are valid RTC I2C io pins */

error: patch failed: components/ulp/ulp_riscv/ulp_riscv_i2c.c:47 error: components/ulp/ulp_riscv/ulp_riscv_i2c.c: patch does not apply

Ethapus avatar Apr 30 '24 08:04 Ethapus

The functions in components/ulp/ulp_riscv/ulp_riscv_i2c.c get linked to your main core application binary (Xtensa core) and provide you with the ability to use the RTC_I2C peripheral. Please note that this peripheral is different from the regular I2C peripheral on the esp32s2/s3 and functions in the RTC clock domain, primarily intended to be used by the ULP RISC-V core when the main core is in sleep mode. The functions in components/ulp/ulp_riscv/ulp_core/ulp_riscv_i2c.c link to your ULP RISC-V core binary and provide the ULP RISC-V core with APIs to use the same RTC_I2C peripheral.

Interestingly the functions in components/ulp/ulp_riscv/ulp_core/ulp_riscv_i2c.c seems to get linked to the main core application binary (Xtensa core) in my code before the ulp application binary is even loaded. Not sure if that is an error on my part or a bug? Anyways, it does work, so long as only byte is written at a time...

Ethapus avatar Apr 30 '24 08:04 Ethapus

HI @Ethapus I tried the patch on the master branch on commit# d4cd437ede and I could apply it with either

git apply rtc_i2c_multi_byte.patch

OR

patch -p1 < rtc_i2c_multi_byte.patch

sudeep-mohanty avatar Apr 30 '24 08:04 sudeep-mohanty

Okay, something must have been awry in my local files, "git reset --hard origin/master" solved the issue. Sorry for not checking that before.

Anyways, I successfully installed the patch and am happy to report that my esp32 s3 now flawlessly reads and writes multiple bytes using ulp_riscv_i2c_master_read_from_device respectively ulp_riscv_i2c_master_write_to_device. This works from both main core application as well as ulp application, even though the main core application still seems to reference components/ulp/ulp_riscv/ulp_core/ulp_riscv_i2c.c for some reason. But it works, and that is what matters.

I thank you very much for the quick response and for solving the issue!

Ethapus avatar Apr 30 '24 10:04 Ethapus

Thanks for the confirmation @Ethapus. I shall add the fix after a few more tests.

sudeep-mohanty avatar Apr 30 '24 10:04 sudeep-mohanty