Unexpected side effect: `gpio_wakeup_disable` silently re-enables power down during sleep when `CONFIG_ESP_SLEEP_GPIO_RESET_WORKAROUND` or `CONFIG_PM_SLP_DISABLE_GPIO` is set (IDFGH-18198)

[nagmat84]

Answers checklist.

• I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
• I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
• I have searched the issue tracker for a similar issue and not found a similar issue.

IDF version.

v6.0.2

Espressif SoC revision.

ESP32-H2 (revision v1.2)

Operating System used.

Linux

How did you build your project?

Command line with Make

If you are using Windows, please specify command line type.

None

Development Kit.

ESP32-H2-Eval-Kit-1

Power Supply used.

USB

What is the expected behavior?

gpio_wakeup_disable should not modify the power management for the given GPIO during sleep.
gpio_wakeup_disable should only disable the GPIO as a wake-up source, but not attempt to change the GPIO configuration besides that in an unexpected (and undocumented) way.

What is the actual behavior?

If CONFIG_ESP_SLEEP_GPIO_RESET_WORKAROUND or CONFIG_PM_SLP_DISABLE_GPIO are set, gpio_wakeup_disable internally calls gpio_hal_sleep_sel_en for that GPIO which power downs the GPIO during sleep.

Steps to reproduce.

n/a

Debug Logs.

n/a

Diagnostic report archive.

No response

More Information.

I assume the code is written with the wrong assumption that a GPIO which also acts as a wake-up source is an input-only GPIO. For input-only GPIOs it is reasonable to power them down during sleep, because they don't serve a purpose if they aren't a wake-up source.

However, this assumptions falls short for GPIOs which are combined input/output GPIOs and which one wants to actively drive during (light) sleep as an output. In that case, the GPIO must be kept power up during light sleep.

Even if the programmer calls gpio_sleep_sel_dis on those GPIOs as advised by the documentation, but then (re-)configures the wake-up sources the effect of gpio_sleep_sel_dis is silently reverted by gpio_wakeup_disable.

I see three possible solutions to this issue:

1. Extending documentation: Explicitly note this side effect of gpio_wakeup_disable in the documentation. The documentation for CONFIG_PM_SLP_DISABLE_GPIO currently states

   If you want to specifically use some pins normally as chip wakes when chip sleeps, you can call 'gpio_sleep_sel_dis' to disable this feature on those pins.
   You can also keep this feature on and call 'gpio_sleep_set_direction' and 'gpio_sleep_set_pull_mode' to have a different GPIO configuration at sleep.

   It should be extended as follows

   If you want to specifically use some pins normally as chip wakes when chip sleeps, you can call gpio_sleep_sel_dis to disable this feature on those pins.
   Please note, that disabling a GPIO as a wake-up source via gpio_wakeup_disable reverts the effect of gpio_sleep_sel_dis for that pin.
   When you disable a GPIO as wake-up source, but want to keep the GPIO powered during sleep, you must call gpio_sleep_sel_dis again.
   Alternatively, you can also keep this feature on and call 'gpio_sleep_set_direction' and 'gpio_sleep_set_pull_mode' to set an explicit GPIO configuration at sleep which is not affected by gpio_sleep_sel_dis.

   One important point is that gpio_sleep_set_direction and gpio_sleep_set_pull_mode are also of value when the configuration doesn't differ.
   The documentation for gpio_wakeup_disable currently only states

   Disable GPIO wake-up function.

   It should also be extended as follows

   Disable GPIO wake-up function and re-enables power-down mode during sleep.

2. Remove unexpected behavior from gpio_wakeup_disable: Remove the problematic code from gpio_wakeup_disable (and gpio_wakeup_enable, btw.), i.e. everything between #if CONFIG_ESP_SLEEP_GPIO_RESET_WORKAROUND || CONFIG_PM_SLP_DISABLE_GPIO and #endif

3. Make gpio_wakeup_disable more sensitive: Instead of re-enabling power down unconditionally, gpio_wakeup_disable should first check whether the mode of the GPIO is input-only or also covers output. gpio_wakeup_disable should only re-enable power down for input-only GPIO, but don't touch output GPIOs.

Ranking

My ranking would be option 2, 1, then 3.

Rationale

Option 2 is clear and concise. It also follows the principle of least surprise. A function should only do what one can assume from its name, but not try to pamper the developer and do more than expected. Especially, the documentation already asks the developer to call gpio_sleep_sel_dis for GPIO which shall remain powered during sleep. So there is no reason for gpio_wakeup_disable and gpio_wakeup_enable to unexpectedly change the power management for those GPIOs again.

Option 1 is the minimum alternative if option 2 is not viable for any reason. At least the developer must be informed about unexpected side effects.

Option 3 makes things probably worse. It tries to be more sensitive and cover more corner cases, but in the long run it might cause even more trouble, because there might be other scenarios which are still not considered.