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

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

Open
3 tasks done
sidwarkd opened this issue Sep 13, 2024 · 23 comments
Open
3 tasks done

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

sidwarkd opened this issue Sep 13, 2024 · 23 comments
Assignees
Labels
Awaiting Response awaiting a response from the author Status: In Progress Work is in progress Type: Bug bugs in IDF

Comments

@sidwarkd
Copy link

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.

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 sidwarkd added the Type: Bug bugs in IDF label Sep 13, 2024
@espressif-bot espressif-bot added the Status: Opened Issue is new label Sep 13, 2024
@github-actions github-actions bot changed the title Unable to call ulp_risv_i2c_master_init from ULP code Unable to call ulp_risv_i2c_master_init from ULP code (IDFGH-13699) Sep 13, 2024
@sidwarkd
Copy link
Author

sidwarkd commented Sep 13, 2024

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

sudeep-mohanty commented Sep 16, 2024

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

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

@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Sep 16, 2024
@sidwarkd
Copy link
Author

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

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

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

@espressif-bot espressif-bot added the Awaiting Response awaiting a response from the author label Oct 3, 2024
@sidwarkd
Copy link
Author

sidwarkd commented Oct 8, 2024

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

sidwarkd commented Oct 8, 2024

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

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

sidwarkd commented Oct 8, 2024

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

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

@sudeep-mohanty
Copy link
Collaborator

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

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

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

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

sudeep-mohanty commented Oct 15, 2024

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

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

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

sidwarkd commented Nov 1, 2024

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

@sidwarkd
Copy link
Author

sidwarkd commented Nov 8, 2024

@sudeep-mohanty One additional follow up here. Everything is working great but the addition of the call to ulp_riscv_i2c_master_init causes a very significant increase in the size of the ULP program. In my testing, simply removing the call to that function reduces the ULP program size by 1796 bytes which is a significant portion of the total available ULP program space.

Is there any way to reduce this impact?

@sidwarkd
Copy link
Author

@sudeep-mohanty More information here. Before entering deep sleep our firmware calls esp_sleep_enable_ext0_wakeup(static_cast<gpio_num_t>(PIN_USER_BUTTON), 1); When in sleep it draws around 300uA which is very high. We tried rolling back this patch which allows us to call ulp_riscv_i2c_master_init and slightly modified the ULP program to get it to compile since that function is no longer available. With those changes it now pulls 13uA in deep sleep so it appears something with this patch causes an issue with high current draw in deep sleep when using esp_sleep_enable_ext0_wakeup from the main app.

@sudeep-mohanty
Copy link
Collaborator

HI @sidwarkd, Thanks for these updates. I haven't had the time to look in to this support since my last update. 300 uA is pretty high and isn't what I expected. I don't have an explanation currently to suggest what could be drawing the excess current. The patch itself is a proof-of-concept and isn't well tested so such unusual behavior can't be ruled out. I don't really have a timeline by when we could have this fully tested and merged though.

@sidwarkd
Copy link
Author

Thanks for the response @sudeep-mohanty . I will post any updates here as we research it more on our end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Response awaiting a response from the author Status: In Progress Work is in progress Type: Bug bugs in IDF
Projects
None yet
Development

No branches or pull requests

3 participants