espressif / esp-idf

Espressif IoT Development Framework. Official development framework for Espressif SoCs.
Apache License 2.0
13.7k stars 7.3k forks source link

Unable to call `ulp_risv_i2c_master_init` from ULP code (IDFGH-13699) #14571

Open sidwarkd opened 1 month ago

sidwarkd commented 1 month ago

Answers checklist.

IDF version.

v5.3

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

What is the expected behavior?

Should be able to call ulp_riscv_i2c_master_init from ULP C code.

What is the actual behavior?

A linker error is returned

undefined reference to `ulp_riscv_i2c_master_init'

Steps to reproduce.

  1. Clone the ULP I2C example project.
  2. In the ULP main.c add a call to ulp_riscv_i2c_master_init

Project will fail to build.

Build or installation Logs.

main.c:(.text.startup.main+0x24): undefined reference to `ulp_riscv_i2c_master_init'
collect2: error: ld returned 1 exit status

More Information.

In our use case we have muxed pin 3 to also serve as a 1-wire interface. In the ULP domain we use I2C to interact with a temperature sensor, mux pin 3 to the 1-wire bus and read a sensor there. This works as expected. After reading the 1-wire device we mux pin 3 back to the I2C SDA line but all future readings fail.

After reading the 1-wire device I put pin 3 back into the IO state it needs to be for SDA (INPUT_OUTPUT_OD). My guess is that the 1-wire traffic puts the peripheral in a bad state. I can resolve this state by calling ulp_riscv_i2c_master_init from the app code. However, the point of the ULP is to be able to continuously do readings without the main application running. This is the reason why I'm trying to call ulp_riscv_i2c_master_init from ULP code.

If there is some other way to effectively reset the RISCV I2C peripheral from the ULP domain that would also work.

sidwarkd commented 1 month ago

One thing that just occurred to me is that I'm using the ulp_riscv_gpio_xxx interface to reconfigure PIN 3 for 1-wire readings and then set it back to a state for the I2C peripheral to use. However, ulp_riscv_i2c_master_init is using the rtc_gpio_xxx API to configure the pin. What is the difference here and is it possible my problem is caused by the ulp_riscv_gpio_xxx API is "stealing" the pin from the RTC IO domain somehow?

Am I able to use the rtc_gpio_xxx API from ULP code?

sudeep-mohanty commented 1 month ago

Hello @sidwarkd, The ULP RISC-V I2C driver is designed in such a way that all setup/teardown steps are handled by the main core and the ULP core is only responsible for reading to and writing from the peripheral. However, it looks like, in your use case you may need complete control of the peripheral from the ULP core at times. While we do not have APIs to do this currently, I can think of a few ways to achieve this and work around your problem -

  1. Use GPIO 1 for SDA instead. This enables you to use GPIO 3 freely for the 1-wire bus without interfering with the I2C peripheral.

  2. If you cannot switch the pins in your design, then you would need to directly write to the I2C registers from the ULP core to reset and re-init the I2C. This would involve a few more steps, i.e, the same steps as that of ulp_riscv_i2c_master_init(). We will discuss further internally if this can be enabled as an API and keep you posted on this.

sidwarkd commented 1 month ago

@sudeep-mohanty As always, I appreciate the quick response. Unfortunately I can't swap pins at this stage of the product's release but that's good to know as an option. So it looks like my option is to re-init the peripheral in the ULP domain. Any guidance the team can provide would be greatly appreciated on what portions of the init I need to recreate or if I basically have to copy the entire thing. Thanks again.

sidwarkd commented 1 month ago

@sudeep-mohanty Follow up ping to see if there is any guidance on peripheral init in the ULP domain. Thanks again for the help.

sudeep-mohanty commented 1 month ago

Hi @sidwarkd. Apologies! I was off-work for a few days so I couldn't progress on this. I shall get back to you as soon as I have some updates.

sudeep-mohanty commented 1 month ago

Hi @sidwarkd, Apologies for the delay. I might have a potential solution for you but it would be nice if you could give it a try and let me know how it goes.

As I understand, since you multiplex pin 3 for reading the sensor, the pin configuration probably is not open-drain anymore, which is why the I2C peripheral does not read any data correctly after you switch it back. So the approach I have is that you could re-initialize the pins for the I2C peripheral from the ULP RISC-V core. There are few assumptions to this approach -

  1. The RTC I2C must have been configured previously from the main CPU.
  2. The I2C configurations have not been reset and are still valid. In other words, only the GPIO configurations have changed.
  3. The ULP core has no idea if the pin is being used simultaneously from the main CPU so re-initializing it for the I2C may cause undefined behavior on the main CPU if the same pins are being used.

With this in mind, you could use this new API, like below, to re-initialize the IOs for the I2C peripheral from the ULP RISC-V core -

ulp_riscv_i2c_pin_cfg_t cfg = ULP_RISCV_I2C_DEFAULT_GPIO_CONFIG();
ulp_riscv_i2c_master_set_pin(&cfg);

In my limited testing, it works as expected. However, it would be nice to have this run in your setup before I could incorporate the changes. Please find a patch attached.

ulp_i2c_pin.patch

sidwarkd commented 4 weeks ago

@sudeep-mohanty Sorry for the delayed response. I was finally able to test this today. Thank you for providing a patch to try. Unfortunately, I am still having the issue and I think the reason is because the I2C state machine is in a bad state after switching to the one-wire device. I moved my entire logic out of the ULP domain and into the main CPU domain. The following sequence works:

  1. ulp_riscv_i2c_master_init()
  2. Read ENS210 device over I2C
  3. Mux pin 3 to one-wire device
  4. Setup pin 3 for one-wire
  5. Read one-wire device
  6. Disconnect one-wire device with mux

If I repeat steps 1-6 I can read the I2C device and the one-wire device attached to pin 3 continuously over and over. However, if I remove step 1 and do not call the master_init on the I2C bus every time I switch between devices, I see the same behavior as in the ULP and the I2C device is unable to read.

So it appears it's more than just re-configuring the pins. Is there some I2C state that also needs to be reset that we can do in the ULP domain?

I very much appreciate your support and efforts on this issue.

sidwarkd commented 4 weeks ago

@sudeep-mohanty One other thought is if there is a way to pause the I2C peripheral while using one-wire from the ULP domain. Since the pin is shared it really feels like the one-wire communication is putting the I2C peripheral in a bad state that can only be recovered by calling the master_init function again.

sudeep-mohanty commented 4 weeks ago

Hi @sidwarkd, Thank you for trying out. Maybe I did not understand the test steps fully but I'd like to know if you used the ulp_riscv_i2c_master_set_pin() API from the ULP domain? Ideally, here is what I imagine your application would be like -

On main core:

  1. ulp_riscv_i2c_master_init()

On ULP core:

  1. Read ENS210 device over I2C
  2. Mux pin 3 to one-wire device
  3. Setup pin 3 for one-wire
  4. Read one-wire device
  5. Disconnect one-wire device with mux
  6. Reset pin 3 for I2C usage by ulp_riscv_i2c_master_set_pin()
  7. Repeat from step 1 again.

The reason for suggesting this approach was the assumption that I2C peripheral is not in a "bad" state and can recover just by re-initializing the pins. Do let me know if this doesn't solve it and I can explore more on how can I get the complete re-initialization of the I2C from the ULP going.

sidwarkd commented 4 weeks ago

@sudeep-mohanty Sorry the confusion. I tried ulp_riscv_i2c_master_set_pin exactly as you outlined. The first I2C read and one-wire read work great. All subsequent I2C reads fail. In looking at the traffic on a scope I don't see the SCL or SDA lines being driven during subsequent I2C access following a one-wire read. Thank you again for your fast replies.

sudeep-mohanty commented 4 weeks ago

Thanks for the clarification @sidwarkd. I shall investigate it more and get back to you.

sudeep-mohanty commented 3 weeks ago

Hi @sidwarkd, Could you please try the attached patch and let me know the result? With this you should be able to callulp_riscv_i2c_master_init() from the ULP RISC-V core. Thanks!

i2c_ulp_init.patch

sidwarkd commented 3 weeks ago

@sudeep-mohanty I tried to apply the patch but it failed. I reset my working directory and pulled the latest 5.3 but it still didn't apply. Here is the output of a few commands I ran showing the error message, IDF version, and which commit I'm on. image

Please let me know if I'm doing something wrong or if I should be on a different commit of IDF.

sudeep-mohanty commented 3 weeks ago

@sidwarkd My bad. The patch I shared was for the latest master. Please, can you try again with this one - i2c_ulp_init_v5.3.patch

sidwarkd commented 3 weeks ago

@sudeep-mohanty I have now successfully read both I2C and one-wire devices repeatedly in the ULP domain. I'd still like to do some more testing but the preliminary results look very good. Due to some other commitments it will take me about a week to do more testing but I will post results here as soon as I'm able. Thank you for your continued support.

sudeep-mohanty commented 3 weeks ago

Thanks for the confirmation, @sidwarkd! The changes are not final yet so I'd be grateful for any tests that you could do and let me know the results! Thank you! I shall work on finalizing the changes and will also be doing more tests at my end.

sidwarkd commented 1 week ago

@sudeep-mohanty Have run both types of sensors in the ULP domain without any issue. One strange behavior I saw was that calling the init function from the app domain a single time followed by the ulp domain caused issues trying to read from the I2C device (ENS210) unless I called the one-wire code in between. The following flow did not work:

  1. Call ulp_riscv_i2c_master_init() from app code
  2. Call ulp_riscv_i2c_master_init() from ulp code
  3. In ULP code try to read ENS210 (does not read correctly. No failures, the data is simply incorrect)
  4. Finish ULP code and wait for next wake (return to step 2)

The following flow, however, did work.

  1. Call ulp_riscv_i2c_master_init() from app code
  2. Call ulp_riscv_i2c_master_init() from ulp code
  3. In ULP code try to read ENS210 (didn't read correctly)
  4. Mux to one-wire
  5. Do a one-wire read
  6. Mux back to I2C
  7. Call ulp_riscv_i2c_master_init() from ulp code
  8. Read the ENS210 (works correctly this time)
  9. Finish ULP code and wait for next wake (return to step 2)

Unfortunately I don't have a lot of cycles to do more in-depth testing on this. The patch provided is working as long as I maintain the order of operations described and has my project unblocked. I very much appreciate your diligent efforts in identifying and providing a fix. If there are specific tests you would like me to run I will do my best to accommodate and help out.

sudeep-mohanty commented 1 week ago

Thank you for the feedback @sidwarkd. One suggestion here is, it looks like it is not necessary to initialize the I2C from the app code. If it avoids the first incorrect read then that is one update you could make to your flow.

sidwarkd commented 5 days ago

@sudeep-mohanty Just as a follow up we have been running code in the ULP to read both an I2C sensor and one-wire sensor that share a muxed pin 3 and everything has been working fine. No other issues to report.