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

i2cdev: Fix i2c param config and driver install order for esp32 target #475

Merged
merged 2 commits into from
Nov 17, 2022

Conversation

AxelLin
Copy link
Contributor

@AxelLin AxelLin commented Nov 14, 2022

i2c_driver_install() needs to be called before i2c_param_config(). This fixes panic reported on [1].

Link: [1] espressif/esp-idf#10163
Signed-off-by: Axel Lin axel.lin@ingics.com

i2c_driver_install() needs to be called before i2c_param_config().
This fixes panic reported on [1].

Link: [1] espressif/esp-idf#10163
Signed-off-by: Axel Lin <axel.lin@ingics.com>
@UncleRus
Copy link
Owner

UncleRus commented Nov 14, 2022

I'm sorry, but I can't merge this PR: it will break compatibility with previous versions of the ESP-IDF. Obviously, we need to either wait for Espressif to respond to issue #10163 (and possibly fix it) or use conditional compilation.

@UncleRus UncleRus self-assigned this Nov 14, 2022
@AxelLin
Copy link
Contributor Author

AxelLin commented Nov 16, 2022

I think this won't cause regression because in esp-idf i2c_tools, it calls with below order in all esp-idf versions:
i2c_driver_install()
i2c_param_config() (this is called in i2c_master_driver_initialize()).

e.g.
v4.4: https://github.com/espressif/esp-idf/blob/release/v4.4/examples/peripherals/i2c/i2c_tools/main/cmd_i2ctools.c
master: https://github.com/espressif/esp-idf/blob/master/examples/peripherals/i2c/i2c_tools/main/cmd_i2ctools.c

The problem in current master tree happens if calling i2c_param_config() after i2c_driver_delete().
So probe device function as below will trigger the issue after 1st iteration of the loop.

for_each_i2c_device() {
        i2c_param_config()
        i2c_driver_install()
        ...
        i2c_driver_delete()
}

I believe this is a regression in esp-idf, but I'm not sure when it will be fixed.
I'm fine to wait for espressif's response, just want to clarify this patch should not cause regression.

@UncleRus
Copy link
Owner

Added check for esp-idf version.
@AxelLin Thanks a lot!

@UncleRus UncleRus merged commit 3811e09 into UncleRus:master Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants