-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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
i2c_master: hangs when SCL is randomly pulled low during communication (IDFGH-12655) #13647
Comments
As a proof of concept, this is one way to prevent the hang, by waiting only while the SCL line is low. To prevent the spinning on With the diff below I can no longer hang the bus. You know the code best, so maybe there is an even better way to solve this: diff --git a/components/esp_driver_i2c/i2c_master.c b/components/esp_driver_i2c/i2c_master.c
index 4ee7c2b409..ef56bb01c5 100644
--- a/components/esp_driver_i2c/i2c_master.c
+++ b/components/esp_driver_i2c/i2c_master.c
@@ -75,6 +75,15 @@ static esp_err_t s_i2c_master_clear_bus(i2c_bus_handle_t handle)
#else
i2c_hal_context_t *hal = &handle->hal;
i2c_ll_master_clr_bus(hal->dev, I2C_LL_RESET_SLV_SCL_PULSE_NUM_DEFAULT);
+
+ // It would be a good idea to add timout logic here and return an error,
+ // so the program can proceed without hanging if SCL is stuck low:
+ i2c_dev_t *hw = hal->dev;
+ while (hw->scl_sp_conf.scl_rst_slv_en && !gpio_get_level(handle->scl_num));
+ hw->ctr.conf_upgate = 1;
+
+ // Log if it would have hung, this is only for debug:
+ if (hw->scl_sp_conf.scl_rst_slv_en)
+ ESP_LOGE(TAG, "********************* I2C transaction would have hung!");
+
#endif
return ESP_OK;
}
diff --git a/components/hal/esp32c6/include/hal/i2c_ll.h b/components/hal/esp32c6/include/hal/i2c_ll.h
index 5d0ce895d3..340c4f8496 100644
--- a/components/hal/esp32c6/include/hal/i2c_ll.h
+++ b/components/hal/esp32c6/include/hal/i2c_ll.h
@@ -764,8 +764,8 @@ static inline void i2c_ll_master_clr_bus(i2c_dev_t *hw, uint32_t slave_pulses)
hw->ctr.conf_upgate = 1;
// hardware will clear scl_rst_slv_en after sending SCL pulses,
// and we should set conf_upgate bit to synchronize register value.
- while (hw->scl_sp_conf.scl_rst_slv_en);
- hw->ctr.conf_upgate = 1;
}
/** |
This is a good question. Yes. Usually I2C fsm got something wrong, we reset the bus, that is send 9 SCL pulses. However, during sending SCL pulses, there is another disturbance work on I2C bus, I2C hardware will completely confused. So, in this part, I will have a talk with our hardware design colleagues to see if there is any better solution. |
Seems I also hit the issue with v5.2.1-443-g2ba5320112 and v5.3-dev-3675-ge486f3b944.
|
Just my $0.02. I have been working with I2C for 30 years now, have also written an extensive driver for ESP8266. The recommended (by Philips/NXP) way to resolve a "stuck" I2C bus (SDA), is to cycle the SCL line at least 8 times (and leave SDA floating high). This will make most state machines in the slaves reset to their initial state. And stop outputting to SDA (data + ack). If a slave hogs SCL (clock stretching) you're on your own. There is no way whatsoever to resolve a bus with a stuck SCL. So I guess Espressif is doing the right thing here. |
That may be true in terms of how the bus reset is performed, but it is the current error handling implementation which is problematic: This issue is that when the bus enters a stuck state, the driver hangs in an interrupt handler and crashes the software via WDT. The correct software fix would return an error (or a timeout) from |
Ah yes ok, clear! |
Just wanted to add, I encountered the exact error originally posted; found out it was due to me using vTaskDelay() to wait for some conversions on an I2C device while also using CONFIG_FREERTOS_USE_TICKLESS_IDLE=y and CONFIG_PM_ENABLE=y with light sleep and Zigbee. Fixed my issue (so far, still testing) by implementing esp_pm_lock_acquire() and related functions around my I2C calls in the application. Seems that the I2C driver (at least the new one, maybe the old also?) might have some compatibility issues with light sleep. |
Hi @mythbuster5,
Thank you for your recent commits that help with I2C stability. At this point the only way that I can lock up/crash the chip (in a way that it cannot do error handling) is by randomly grounding the SCL line by hand with a pushbutton.
Eventually I can get it to hang with this WDT warning/trace:
Which causes it to spin here:
esp-idf/components/hal/esp32c6/include/hal/i2c_ll.h
Line 767 in 636ff35
Note that pushing the reset button always "fixes" it and communication resumes after the reset.
Question
Based on the comment at the top of the function, perhaps nothing else that can be done to prevent this... but before I acquiesce to this limitation, I have a question:Is there a way to know if the 9x SCL pulses been sent by the hardware have completed, so that it can stop spinning and return control to the program, even if the bus really is locked up?
For example, can we just read the GPIO line level and return from the spinning
while
loop if the line is high?Here is the entire function for your reference:
esp-idf/components/hal/esp32c6/include/hal/i2c_ll.h
Lines 749 to 769 in 636ff35
The text was updated successfully, but these errors were encountered: