From bfb2c8e1f1c58af271d62237dffe42020d51bed8 Mon Sep 17 00:00:00 2001 From: ainyan03 <205502311+ainyan03@users.noreply.github.com> Date: Tue, 25 Aug 2026 10:04:16 +0000 Subject: [PATCH] I2C: share a port the application already opened instead of taking it over init() assumed the requested HP I2C port was free. When the application had already opened it - through the ESP-IDF i2c_master driver or Arduino Wire - the reset in init() wiped that driver's configuration, the failing i2c_new_master_bus() went unnoticed (and on ESP-IDF 5.5 that failure path unroutes the pins of the existing bus), and release() later tore the driver's bus down. The first transfer after switching sides then failed on every chip. A port that is already open is now recognised beforehand - i2cIsInit() under Arduino, i2c_master_get_bus_handle() on ESP-IDF 5.3.2 and later - and shared: nothing is reset, acquired or re-routed, each transaction applies the library's configuration and endTransaction() restores the driver's register block, with the completion flags cleared first so the driver's own interrupt does not see a stale one. release() on such a port only drops the flag. A port held by a slave driver is refused, and other i2c_new_master_bus() failures now fail init(). Not covered: serialization between the two sides (the driver's lock is not exposed, the application must not run them concurrently), differing pins, the asynchronous ESP-IDF API, low power ports, and a non-default I2C clock source on ESP32-P4 / ESP32-C61. --- src/lgfx/v1/platforms/esp32/common.cpp | 73 ++++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 4 deletions(-) diff --git a/src/lgfx/v1/platforms/esp32/common.cpp b/src/lgfx/v1/platforms/esp32/common.cpp index 109eb45..3d4cb1f 100644 --- a/src/lgfx/v1/platforms/esp32/common.cpp +++ b/src/lgfx/v1/platforms/esp32/common.cpp @@ -1416,6 +1416,7 @@ namespace lgfx gpio_num_t pin_sda = (gpio_num_t)-1; uint8_t wait_ack_stage = 0; // 0:Not waiting. / 1:Waiting after addressing. / 2:Waiting during data transmission. bool initialized = false; + bool foreign_bus = false; // the port was already open through the platform driver when init() ran: shared, not owned uint32_t freq = 0; void save_reg(i2c_dev_t* dev) @@ -1767,6 +1768,10 @@ namespace lgfx } //ESP_LOGI("LGFX", "I2C stop"); } + // Leave no completion event behind: on a shared port the platform driver enables + // its interrupts without clearing the raw flags first, so a stale one would fire + // as soon as its next transfer starts. + dev->int_clr.val = 0x1FFFF; i2c_context[i2c_port].load_reg(dev); if (res) { @@ -1784,6 +1789,11 @@ namespace lgfx if (i2c_context[i2c_port].initialized) { i2c_context[i2c_port].initialized = false; + if (i2c_context[i2c_port].foreign_bus) + { // Nothing was acquired: the bus and its pins belong to the platform driver. + i2c_context[i2c_port].foreign_bus = false; + return {}; + } #if LGFX_LP_I2C_NUM > 0 if (isLpPort(i2c_port)) { // Opened through the ESP-IDF driver in init(), so it is closed the same way. @@ -1928,6 +1938,48 @@ namespace lgfx release(i2c_port); } + // A port the application already opened through the platform driver (TwoWire, or an + // ESP-IDF i2c_master bus) is shared rather than taken over: nothing is reset or + // acquired here, each transaction applies our configuration and endTransaction() + // puts the driver's back, so transfers from both sides can alternate. What sharing + // does not cover: the two sides are not serialized against each other (the + // application must not run them concurrently), both must use the same pins, and + // only the synchronous ESP-IDF API is compatible (its interrupts are off between + // transfers). release() then only drops the flag. + bool foreign = false; +#if defined ( ARDUINO ) && __has_include () + #if defined ( USE_TWOWIRE_SETPINS ) && defined ( ESP_ARDUINO_VERSION_VAL ) && ( ESP_ARDUINO_VERSION >= ESP_ARDUINO_VERSION_VAL(2, 0, 1) ) + foreign = i2cIsInit(i2c_port); // present from core 2.0.1 + #endif +#elif __has_include() && (ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 3, 2)) + { // Asking for the handle is the only non-destructive way to learn whether the port + // is open (a failing i2c_new_master_bus() unroutes the pins of the existing bus), + // but it logs an error when the port is free, which is the normal case here. + i2c_master_bus_handle_t existing = nullptr; + auto level = esp_log_level_get("i2c.master"); + esp_log_level_set("i2c.master", ESP_LOG_NONE); + bool occupied = (ESP_OK == i2c_master_get_bus_handle(i2c_port, &existing)); + esp_log_level_set("i2c.master", level); + // Occupied without a master handle means a slave driver owns the port (only + // detectable when no master bus was ever created on it: the driver keeps the last + // master handle after deleting the bus). + if (occupied && existing == nullptr) { return cpp::fail(error_t::invalid_arg); } + foreign = occupied; + } +#endif + if (foreign) + { +#if LGFX_LP_I2C_NUM > 0 + if (isLpPort(i2c_port)) { return cpp::fail(error_t::invalid_arg); } // a low power port is not shared +#endif + i2c_context[i2c_port].foreign_bus = true; + i2c_context[i2c_port].initialized = true; + // The pins stay as the driver configured them (routing, pull-ups): they are not + // touched here nor per transaction, which is why both sides must use the same pins. + i2c_context[i2c_port].save_reg(getDev(i2c_port)); + return {}; + } + i2c_stop(i2c_port); #if LGFX_LP_I2C_NUM > 0 @@ -1982,7 +2034,17 @@ namespace lgfx bus_config.flags.enable_internal_pullup = true; bus_config.intr_priority = 1; - i2c_new_master_bus(&bus_config, &bus_handle); + if (ESP_OK != i2c_new_master_bus(&bus_config, &bus_handle)) + { +#if (ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 3, 2)) + // The port was free a moment ago, so the driver could not be set up. + return cpp::fail(error_t::invalid_arg); +#else + // No way to tell a held port from a setup failure here: keep driving the + // controller directly, as before. + bus_handle = nullptr; +#endif + } i2c_context[i2c_port].i2c_bus_handle = bus_handle; #else i2c_periph_enable(i2c_port); @@ -2156,12 +2218,15 @@ namespace lgfx } i2c_context[i2c_port].save_reg(dev); -#if LGFX_LP_I2C_NUM > 0 // A low power port reaches its pads through the low power IO domain, which set_pin() // knows nothing about: routing them through the normal GPIO matrix here would - // disconnect the port from its own pins on every transaction. - if (!isLpPort(i2c_port)) + // disconnect the port from its own pins on every transaction. A shared port keeps + // the pin setup of the driver that owns it. ( see foreign_bus ) + if (!i2c_context[i2c_port].foreign_bus +#if LGFX_LP_I2C_NUM > 0 + && !isLpPort(i2c_port) #endif + ) { set_pin((i2c_port_t)i2c_port, i2c_context[i2c_port].pin_sda, i2c_context[i2c_port].pin_scl); }