Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
3 tasks done
ESP-Marius opened this issue Sep 13, 2023 · 18 comments
Closed
3 tasks done
Assignees
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Type: Bug bugs in IDF

Comments

@ESP-Marius
Copy link
Collaborator

Answers checklist.

  • I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
  • I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
  • 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 ESP-Marius added the Type: Bug bugs in IDF label Sep 13, 2023
@ESP-Marius
Copy link
Collaborator Author

Issue split from #12214

@espressif-bot espressif-bot added the Status: Opened Issue is new label Sep 13, 2023
@github-actions github-actions bot changed the title ULP-RISCV I2C does not work when reading/writing multiple bytes ULP-RISCV I2C does not work when reading/writing multiple bytes (IDFGH-11056) Sep 13, 2023
@sudeep-mohanty
Copy link
Collaborator

sudeep-mohanty commented Sep 13, 2023

@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!

@aircable
Copy link

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.

@sudeep-mohanty
Copy link
Collaborator

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.

@aircable
Copy link

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.

@shangke1988
Copy link

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

@sudeep-mohanty
Copy link
Collaborator

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.

@aircable
Copy link

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.

@Ethapus
Copy link

Ethapus commented Apr 26, 2024

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.

@martijnED
Copy link

Pull latest version and many i2c bugs are solved

@Ethapus
Copy link

Ethapus commented Apr 26, 2024

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
Copy link

Ethapus commented Apr 27, 2024

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?

@sudeep-mohanty
Copy link
Collaborator

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

@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Apr 29, 2024
@Ethapus
Copy link

Ethapus commented Apr 30, 2024

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
8f44525..c1f02f6 release/v4.4 -> origin/release/v4.4
0ed5284..961ca4f release/v5.0 -> origin/release/v5.0
b43fc4d..30fce03 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
Copy link

Ethapus commented Apr 30, 2024

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...

@sudeep-mohanty
Copy link
Collaborator

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

git apply rtc_i2c_multi_byte.patch

OR

patch -p1 < rtc_i2c_multi_byte.patch

@Ethapus
Copy link

Ethapus commented Apr 30, 2024

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!

@sudeep-mohanty
Copy link
Collaborator

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

@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: NA Issue resolution is unavailable and removed Status: In Progress Work is in progress labels Jun 24, 2024
espressif-bot pushed a commit that referenced this issue Jun 25, 2024
…SC-V

This commit fixes an issue where multi-byte reads and writes over the
RTC I2C peripheral got stuck on the esp32s2 and esp32s3.

Closes #12235
espressif-bot pushed a commit that referenced this issue Jul 3, 2024
…SC-V

This commit fixes an issue where multi-byte reads and writes over the
RTC I2C peripheral got stuck on the esp32s2 and esp32s3.

Closes #12235
espressif-bot pushed a commit that referenced this issue Jul 31, 2024
…SC-V

This commit fixes an issue where multi-byte reads and writes over the
RTC I2C peripheral got stuck on the esp32s2 and esp32s3.

Closes #12235
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Type: Bug bugs in IDF
Projects
None yet
Development

No branches or pull requests

7 participants