Clear stale open-drain pad state in QSPI, parallel bus, and PWM backlight init - #252
Merged
Conversation
The fix for SPI bus pins left in open-drain mode on ESP-IDF v6 (m5stack#192) covered only the single-bit spi::init path. spi::initQuad (used by QSPI panels) calls spi_bus_initialize the same way and is affected by the same behavior: ESP-IDF v6 routes bus pins with gpio_matrix_output() and no longer clears the pad open-drain flag. Factor the pad restoration out of spi::init into a shared helper and apply it to SCLK/IO0-3 in spi::initQuad as well. Verified on M5StopWatch (CO5300, QSPI): with the bus pins deliberately set to open-drain before init, the pads remained open-drain after init on ESP-IDF v6.0.1 without this fix, and are restored to push-pull with it. No regression on ESP-IDF v5.5.4.
The LEDC pin attach does not fully normalize the pad state: the pad open-drain flag is never cleared (on any IDF version), and starting with ESP-IDF v6 the ledc driver no longer selects the GPIO function in IO_MUX either. If the backlight pin was previously configured as an input or open-drain output (e.g. by probing code or a previous application state), the PWM output cannot drive the pin high and the backlight stays off. Explicitly normalize the pin as a push-pull GPIO output at the start of init. The pin is first latched to the "off" level to avoid a visible glitch while the pin is temporarily a plain GPIO output. Verified on M5Stack Core FIRE (backlight on GPIO32) with ESP-IDF v6.0.1: with the pin deliberately set to open-drain before init, the pad remained open-drain after init without this fix, and is restored to push-pull with it.
Neither esp_lcd_new_i80_bus (ESP32-S3, since ESP-IDF v5.4 removed its gpio_set_direction call) nor the plain GPIO matrix routing used on ESP32-S2 configures the pad direction or clears the open-drain flag on the data pins. If a data pin was previously configured as an input or open-drain output, the bus cannot drive it high and the panel receives corrupted data. Bus_EPD already guards against this by calling pinMode(output) on its data pins; apply the same guard to Bus_Parallel8/16. The WR/RD/RS control pins were already normalized via gpio_set_direction. Verified on ESP32-S3 (ESP-IDF v6.0.1) by pulse-counting bus edges with PCNT on a scope-free rig: with the data pins deliberately set to open-drain before init, a 0x00/0xFF pattern produced 0 edges on D0/D7 (WR counting normally) without this fix, and the expected edge count with it.
Light_PWM::init ignored the result of the LEDC configuration calls and always returned true. Propagate failures instead: init now fails early when the configured pin cannot be driven as a GPIO output, the ESP-IDF path checks both ledc_channel_config and ledc_timer_config, the Arduino 3.x path returns the ledcAttach result, and the pre-3.x path checks the frequency returned by ledcSetup (0 on failure) and only attaches the pin when setup succeeded. On failure the initial setBrightness call is skipped. The caller (Panel_Device::init) ignores the return value, so panel initialization is unaffected; the result is now meaningful for callers that choose to check it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #251 (#192). ESP-IDF v6 delegates pad state management (open-drain etc.) to the peripheral user, and an audit of the remaining init paths found three more places that relied on driver side effects that no longer exist on v6 — or never existed on any version:
spi::initQuad(QSPI panels) — same issue as M5Stack CoreS3 display (ILI9342C) blank on ESP-IDF v6.0.1 #192:spi_bus_initialize()on v6 no longer clears the pad open-drain flag, and Fix CoreS3 LCD not working on ESP-IDF v6 (SPI bus pins left in open-drain) #251 covered only the single-bitspi::initpath. The pad restoration is factored into a sharedclear_open_drain()helper and applied to SCLK/IO0-3 as well.Light_PWM::init(PWM backlight) — the LEDC driver never clears the open-drain flag (on any IDF version), and v6 no longer selects the GPIO function in IO_MUX either. The pin is now explicitly normalized as a push-pull GPIO output at the start of init, latched to the "off" level first to avoid a visible glitch.Bus_Parallel8/16on ESP32-S2/S3 — neitheresp_lcd_new_i80_bus(S3; itsgpio_set_directioncall was removed in ESP-IDF v5.4) nor the plain GPIO matrix routing (S2) configures pad direction / open-drain on the data pins. The samepinMode(output)normalization already used byBus_EPDis applied. The WR/RD/RS control pins were already normalized viagpio_set_directionand are unaffected.A fourth commit makes
Light_PWM::initreport failure: it previously ignored the LEDC API results and always returnedtrue. It now fails early for pins that cannot be driven as outputs and propagates LEDC setup errors. (Panel_Device::initignores the return value, so panel initialization behavior is unchanged.)Verification
Each fix was verified A/B on hardware, with the pads deliberately set to open-drain before init (the state left behind by e.g. probing pins as inputs), checking the
GPIO_PINn.pad_driverregister after init: