Fix I2C recovery ESP32 esp-idf (#2438)

Co-authored-by: Maurice Makaay <mmakaay1@xs4all.net>

esphome/components/i2c/i2c_bus_arduino.cpp

@@ -12,6 +12,7 @@ static const char *const TAG = "i2c.arduino";
 
 void ArduinoI2CBus::setup() {
   recover_();
+
 #ifdef USE_ESP32
   static uint8_t next_bus_num = 0;
   if (next_bus_num == 0)
@@ -109,11 +110,19 @@ ErrorCode ArduinoI2CBus::writev(uint8_t address, WriteBuffer *buffers, size_t cn
 void ArduinoI2CBus::recover_() {
   ESP_LOGI(TAG, "Performing I2C bus recovery");
 
-  // Activate the pull up resistor on the SCL pin. This should make the
+  // For the upcoming operations, target for a 100kHz toggle frequency.
-  // signal on the line HIGH. If SCL is pulled low on the I2C bus however,
+  // This is the maximum frequency for I2C running in standard-mode.
-  // then some device is interfering with the SCL line. In that case,
+  // The actual frequency will be lower, because of the additional
-  // the I2C bus cannot be recovered.
+  // function calls that are done, but that is no problem.
-  pinMode(scl_pin_, INPUT_PULLUP);     // NOLINT
+  const auto half_period_usec = 1000000 / 100000 / 2;
+
+  // Activate input and pull up resistor for the SCL pin.
+  pinMode(scl_pin_, INPUT_PULLUP);  // NOLINT
+
+  // This should make the signal on the line HIGH. If SCL is pulled low
+  // on the I2C bus however, then some device is interfering with the SCL
+  // line. In that case, the I2C bus cannot be recovered.
+  delayMicroseconds(half_period_usec);
   if (digitalRead(scl_pin_) == LOW) {  // NOLINT
     ESP_LOGE(TAG, "Recovery failed: SCL is held LOW on the I2C bus");
     recovery_result_ = RECOVERY_FAILED_SCL_LOW;
@@ -125,25 +134,13 @@ void ArduinoI2CBus::recover_() {
   //  device that held the bus LOW should release it sometime within
   //  those nine clocks."
   // We don't really have to detect if SDA is stuck low. We'll simply send
-  // nine clock pulses here, just in case SDA is stuck.
+  // nine clock pulses here, just in case SDA is stuck. Actual checks on
+  // the SDA line status will be done after the clock pulses.
 
-  // Use a 100kHz toggle frequency (i.e. the maximum frequency for I2C
+  // Make sure that switching to output mode will make SCL low, just in
-  // running in standard-mode). The resulting frequency will be lower,
+  // case other code has setup the pin for a HIGH signal.
-  // because of the additional function calls that are done, but that
-  // is no problem.
-  const auto half_period_usec = 1000000 / 100000 / 2;
-
-  // Make sure that switching to mode OUTPUT will make SCL low, just in
-  // case other code has setup the pin to output a HIGH signal.
   digitalWrite(scl_pin_, LOW);  // NOLINT
 
-  // Activate the pull up resistor for SDA, so after the clock pulse cycle
-  // we can verify if SDA is pulled high. Also make sure that switching to
-  // mode OUTPUT will make SDA low.
-  pinMode(sda_pin_, INPUT_PULLUP);  // NOLINT
-  digitalWrite(sda_pin_, LOW);      // NOLINT
-
-  ESP_LOGI(TAG, "Sending 9 clock pulses to drain any stuck device output");
   delayMicroseconds(half_period_usec);
   for (auto i = 0; i < 9; i++) {
     // Release pull up resistor and switch to output to make the signal LOW.
@@ -171,6 +168,11 @@ void ArduinoI2CBus::recover_() {
     }
   }
 
+  // Activate input and pull resistor for the SDA pin, so we can verify
+  // that SDA is pulled HIGH in the following step.
+  pinMode(sda_pin_, INPUT_PULLUP);  // NOLINT
+  digitalWrite(sda_pin_, LOW);      // NOLINT
+
   // By now, any stuck device ought to have sent all remaining bits of its
   // transation, meaning that it should have freed up the SDA line, resulting
   // in SDA being pulled up.
@@ -191,10 +193,9 @@ void ArduinoI2CBus::recover_() {
   // out of this state.
   // SCL and SDA are already high at this point, so we can generate a START
   // condition by making the SDA signal LOW.
-  ESP_LOGI(TAG, "Generate START condition to reset bus logic of I2C devices");
+  delayMicroseconds(half_period_usec);
   pinMode(sda_pin_, INPUT);   // NOLINT
   pinMode(sda_pin_, OUTPUT);  // NOLINT
-  delayMicroseconds(half_period_usec);
 
   // From the specification:
   // "A START condition immediately followed by a STOP condition (void
@@ -202,7 +203,7 @@ void ArduinoI2CBus::recover_() {
   //  operate properly under this condition."
   // Finally, we'll bring the I2C bus into a starting state by generating
   // a STOP condition.
-  ESP_LOGI(TAG, "Generate STOP condition to finalize recovery");
+  delayMicroseconds(half_period_usec);
   pinMode(sda_pin_, INPUT);         // NOLINT
   pinMode(sda_pin_, INPUT_PULLUP);  // NOLINT
 

esphome/components/i2c/i2c_bus_esp_idf.cpp

@@ -43,6 +43,17 @@ void IDFI2CBus::dump_config() {
   ESP_LOGCONFIG(TAG, "  SDA Pin: GPIO%u", this->sda_pin_);
   ESP_LOGCONFIG(TAG, "  SCL Pin: GPIO%u", this->scl_pin_);
   ESP_LOGCONFIG(TAG, "  Frequency: %u Hz", this->frequency_);
+  switch (this->recovery_result_) {
+    case RECOVERY_COMPLETED:
+      ESP_LOGCONFIG(TAG, "  Recovery: bus successfully recovered");
+      break;
+    case RECOVERY_FAILED_SCL_LOW:
+      ESP_LOGCONFIG(TAG, "  Recovery: failed, SCL is held low on the bus");
+      break;
+    case RECOVERY_FAILED_SDA_LOW:
+      ESP_LOGCONFIG(TAG, "  Recovery: failed, SDA is held low on the bus");
+      break;
+  }
   if (this->scan_) {
     ESP_LOGI(TAG, "Scanning i2c bus for active devices...");
     uint8_t found = 0;
@@ -144,39 +155,113 @@ ErrorCode IDFI2CBus::writev(uint8_t address, WriteBuffer *buffers, size_t cnt) {
   return ERROR_OK;
 }
 
+/// Perform I2C bus recovery, see:
+/// https://www.nxp.com/docs/en/user-guide/UM10204.pdf
+/// https://www.analog.com/media/en/technical-documentation/application-notes/54305147357414AN686_0.pdf
 void IDFI2CBus::recover_() {
-  // Perform I2C bus recovery, see
+  ESP_LOGI(TAG, "Performing I2C bus recovery");
-  // https://www.analog.com/media/en/technical-documentation/application-notes/54305147357414AN686_0.pdf
-  // or see the linux kernel implementation, e.g.
-  // https://elixir.bootlin.com/linux/v5.14.6/source/drivers/i2c/i2c-core-base.c#L200
 
-  // try to get about 100kHz toggle frequency
-  const auto half_period_usec = 1000000 / 100000 / 2;
-  const auto recover_scl_periods = 9;
   const gpio_num_t scl_pin = static_cast<gpio_num_t>(scl_pin_);
+  const gpio_num_t sda_pin = static_cast<gpio_num_t>(sda_pin_);
 
-  // configure scl as output
+  // For the upcoming operations, target for a 60kHz toggle frequency.
-  gpio_config_t conf{};
+  // 1000kHz is the maximum frequency for I2C running in standard-mode,
-  conf.pin_bit_mask = 1ULL << static_cast<uint32_t>(scl_pin_);
+  // but lower frequencies are not a problem.
-  conf.mode = GPIO_MODE_OUTPUT;
+  // Note: the timing that is used here is chosen manually, to get
-  conf.pull_up_en = GPIO_PULLUP_DISABLE;
+  // results that are close to the timing that can be archieved by the
-  conf.pull_down_en = GPIO_PULLDOWN_DISABLE;
+  // implementation for the Arduino framework.
-  conf.intr_type = GPIO_INTR_DISABLE;
+  const auto half_period_usec = 7;
 
-  gpio_config(&conf);
+  // Configure SCL pin for open drain input/output, with a pull up resistor.
-
-  // set scl high
   gpio_set_level(scl_pin, 1);
+  gpio_config_t scl_config{};
+  scl_config.pin_bit_mask = 1ULL << scl_pin_;
+  scl_config.mode = GPIO_MODE_INPUT_OUTPUT_OD;
+  scl_config.pull_up_en = GPIO_PULLUP_ENABLE;
+  scl_config.pull_down_en = GPIO_PULLDOWN_DISABLE;
+  scl_config.intr_type = GPIO_INTR_DISABLE;
+  gpio_config(&scl_config);
 
-  // in total generate 9 falling-rising edges
+  // Configure SDA pin for open drain input/output, with a pull up resistor.
-  for (auto i = 0; i < recover_scl_periods; i++) {
+  gpio_set_level(sda_pin, 1);
-    delayMicroseconds(half_period_usec);
+  gpio_config_t sda_conf{};
+  sda_conf.pin_bit_mask = 1ULL << sda_pin_;
+  sda_conf.mode = GPIO_MODE_INPUT_OUTPUT_OD;
+  sda_conf.pull_up_en = GPIO_PULLUP_ENABLE;
+  sda_conf.pull_down_en = GPIO_PULLDOWN_DISABLE;
+  sda_conf.intr_type = GPIO_INTR_DISABLE;
+  gpio_config(&sda_conf);
+
+  // If SCL is pulled low on the I2C bus, then some device is interfering
+  // with the SCL line. In that case, the I2C bus cannot be recovered.
+  delayMicroseconds(half_period_usec);
+  if (gpio_get_level(scl_pin) == 0) {
+    ESP_LOGE(TAG, "Recovery failed: SCL is held LOW on the I2C bus");
+    recovery_result_ = RECOVERY_FAILED_SCL_LOW;
+    return;
+  }
+
+  // From the specification:
+  // "If the data line (SDA) is stuck LOW, send nine clock pulses. The
+  //  device that held the bus LOW should release it sometime within
+  //  those nine clocks."
+  // We don't really have to detect if SDA is stuck low. We'll simply send
+  // nine clock pulses here, just in case SDA is stuck. Actual checks on
+  // the SDA line status will be done after the clock pulses.
+  for (auto i = 0; i < 9; i++) {
     gpio_set_level(scl_pin, 0);
     delayMicroseconds(half_period_usec);
     gpio_set_level(scl_pin, 1);
+    delayMicroseconds(half_period_usec);
+
+    // When SCL is kept LOW at this point, we might be looking at a device
+    // that applies clock stretching. Wait for the release of the SCL line,
+    // but not forever. There is no specification for the maximum allowed
+    // time. We'll stick to 500ms here.
+    auto wait = 20;
+    while (wait-- && gpio_get_level(scl_pin) == 0) {
+      delay(25);
+    }
+    if (gpio_get_level(scl_pin) == 0) {
+      ESP_LOGE(TAG, "Recovery failed: SCL is held LOW during clock pulse cycle");
+      recovery_result_ = RECOVERY_FAILED_SCL_LOW;
+      return;
+    }
   }
 
+  // By now, any stuck device ought to have sent all remaining bits of its
+  // transation, meaning that it should have freed up the SDA line, resulting
+  // in SDA being pulled up.
+  if (gpio_get_level(sda_pin) == 0) {
+    ESP_LOGE(TAG, "Recovery failed: SDA is held LOW after clock pulse cycle");
+    recovery_result_ = RECOVERY_FAILED_SDA_LOW;
+    return;
+  }
+
+  // From the specification:
+  // "I2C-bus compatible devices must reset their bus logic on receipt of
+  //  a START or repeated START condition such that they all anticipate
+  //  the sending of a target address, even if these START conditions are
+  //  not positioned according to the proper format."
+  // While the 9 clock pulses from above might have drained all bits of a
+  // single byte within a transaction, a device might have more bytes to
+  // transmit. So here we'll generate a START condition to snap the device
+  // out of this state.
+  // SCL and SDA are already high at this point, so we can generate a START
+  // condition by making the SDA signal LOW.
   delayMicroseconds(half_period_usec);
+  gpio_set_level(sda_pin, 0);
+
+  // From the specification:
+  // "A START condition immediately followed by a STOP condition (void
+  //  message) is an illegal format. Many devices however are designed to
+  //  operate properly under this condition."
+  // Finally, we'll bring the I2C bus into a starting state by generating
+  // a STOP condition.
+  delayMicroseconds(half_period_usec);
+  gpio_set_level(sda_pin, 1);
+
+  recovery_result_ = RECOVERY_COMPLETED;
 }
 
 }  // namespace i2c

esphome/components/i2c/i2c_bus_esp_idf.h

@@ -9,6 +9,12 @@
 namespace esphome {
 namespace i2c {
 
+enum RecoveryCode {
+  RECOVERY_FAILED_SCL_LOW,
+  RECOVERY_FAILED_SDA_LOW,
+  RECOVERY_COMPLETED,
+};
+
 class IDFI2CBus : public I2CBus, public Component {
  public:
   void setup() override;
@@ -26,6 +32,7 @@ class IDFI2CBus : public I2CBus, public Component {
 
  private:
   void recover_();
+  RecoveryCode recovery_result_;
 
  protected:
   i2c_port_t port_;