From fb7234a13d4fa22f9bd8ac0b4acddd45c1917953 Mon Sep 17 00:00:00 2001 From: chegewara Date: Mon, 31 May 2021 12:57:08 +0200 Subject: [PATCH 1/4] bootloader: Add selectable level for factory reset pin Closes https://github.com/espressif/esp-idf/pull/7089 --- components/bootloader/Kconfig.projbuild | 8 ++++++++ components/bootloader/subproject/main/bootloader_start.c | 2 +- components/bootloader_support/include/bootloader_common.h | 3 ++- components/bootloader_support/src/bootloader_common.c | 6 +++--- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/components/bootloader/Kconfig.projbuild b/components/bootloader/Kconfig.projbuild index 5bd299c9df..40ff02d3be 100644 --- a/components/bootloader/Kconfig.projbuild +++ b/components/bootloader/Kconfig.projbuild @@ -131,6 +131,14 @@ menu "Bootloader config" To trigger a factory reset, this GPIO must be pulled low on reset. Note that GPIO34-39 do not have an internal pullup and an external one must be provided. + config BOOTLOADER_PIN_LEVEL_FACTORY_RESET + int "Level of the GPIO input for factory reset" + depends on BOOTLOADER_FACTORY_RESET + range 0 1 + default 0 + help + Pin level for factory reset. 0 for LOW and 1 for HIGH. + config BOOTLOADER_OTA_DATA_ERASE bool "Clear OTA data on factory reset (select factory partition)" depends on BOOTLOADER_FACTORY_RESET diff --git a/components/bootloader/subproject/main/bootloader_start.c b/components/bootloader/subproject/main/bootloader_start.c index 29c61e43f7..6677707f73 100644 --- a/components/bootloader/subproject/main/bootloader_start.c +++ b/components/bootloader/subproject/main/bootloader_start.c @@ -79,7 +79,7 @@ static int selected_boot_partition(const bootloader_state_t *bs) if (bootloader_common_get_reset_reason(0) != DEEPSLEEP_RESET) { // Factory firmware. #ifdef CONFIG_BOOTLOADER_FACTORY_RESET - if (bootloader_common_check_long_hold_gpio(CONFIG_BOOTLOADER_NUM_PIN_FACTORY_RESET, CONFIG_BOOTLOADER_HOLD_TIME_GPIO) == 1) { + if (bootloader_common_check_long_hold_gpio(CONFIG_BOOTLOADER_NUM_PIN_FACTORY_RESET, CONFIG_BOOTLOADER_HOLD_TIME_GPIO, CONFIG_BOOTLOADER_PIN_LEVEL_FACTORY_RESET) == 1) { ESP_LOGI(TAG, "Detect a condition of the factory reset"); bool ota_data_erase = false; #ifdef CONFIG_BOOTLOADER_OTA_DATA_ERASE diff --git a/components/bootloader_support/include/bootloader_common.h b/components/bootloader_support/include/bootloader_common.h index 3fb97c65fc..36a027fb33 100644 --- a/components/bootloader_support/include/bootloader_common.h +++ b/components/bootloader_support/include/bootloader_common.h @@ -70,9 +70,10 @@ bool bootloader_common_ota_select_invalid(const esp_ota_select_entry_t *s); * * @param[in] num_pin Number of the GPIO input. * @param[in] delay_sec Input must be driven low for at least this long, continuously. + * @param[in] level Input pin level to trigger lang hold, 0 - LOW, 1 - HIGH * @return esp_comm_gpio_hold_t Defines type of hold a GPIO in low state. */ -esp_comm_gpio_hold_t bootloader_common_check_long_hold_gpio(uint32_t num_pin, uint32_t delay_sec); +esp_comm_gpio_hold_t bootloader_common_check_long_hold_gpio(uint32_t num_pin, uint32_t delay_sec, int level); /** * @brief Erase the partition data that is specified in the transferred list. diff --git a/components/bootloader_support/src/bootloader_common.c b/components/bootloader_support/src/bootloader_common.c index f8541add5e..106b2043a5 100644 --- a/components/bootloader_support/src/bootloader_common.c +++ b/components/bootloader_support/src/bootloader_common.c @@ -39,7 +39,7 @@ static const char* TAG = "boot_comm"; -esp_comm_gpio_hold_t bootloader_common_check_long_hold_gpio(uint32_t num_pin, uint32_t delay_sec) +esp_comm_gpio_hold_t bootloader_common_check_long_hold_gpio(uint32_t num_pin, uint32_t delay_sec, int level) { esp_rom_gpio_pad_select_gpio(num_pin); if (GPIO_PIN_MUX_REG[num_pin]) { @@ -47,11 +47,11 @@ esp_comm_gpio_hold_t bootloader_common_check_long_hold_gpio(uint32_t num_pin, ui } esp_rom_gpio_pad_pullup_only(num_pin); uint32_t tm_start = esp_log_early_timestamp(); - if (gpio_ll_get_level(&GPIO, num_pin) == 1) { + if (gpio_ll_get_level(&GPIO, num_pin) != level) { return GPIO_NOT_HOLD; } do { - if (gpio_ll_get_level(&GPIO, num_pin) != 0) { + if (gpio_ll_get_level(&GPIO, num_pin) != level) { return GPIO_SHORT_HOLD; } } while (delay_sec > ((esp_log_early_timestamp() - tm_start) / 1000L)); From 6bbb58c8c27e8632a1c679362ba0b2733a5c0c1e Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Tue, 15 Jun 2021 11:18:33 +1000 Subject: [PATCH 2/4] bootloader: Small cleanup and docs for factory reset level config - Add to docs & config descriptions - Change to a "choice" to become self-documenting - Keep the bootloader_common_check_long_hold_gpio() function for compatibility --- components/bootloader/Kconfig.projbuild | 26 ++++++++++------- .../subproject/main/bootloader_start.c | 6 +++- .../include/bootloader_common.h | 28 +++++++++++++++---- .../src/bootloader_common.c | 7 ++++- docs/en/api-guides/bootloader.rst | 6 ++-- 5 files changed, 54 insertions(+), 19 deletions(-) diff --git a/components/bootloader/Kconfig.projbuild b/components/bootloader/Kconfig.projbuild index 40ff02d3be..609bc221a6 100644 --- a/components/bootloader/Kconfig.projbuild +++ b/components/bootloader/Kconfig.projbuild @@ -117,8 +117,8 @@ menu "Bootloader config" Allows to reset the device to factory settings: - clear one or more data partitions; - boot from "factory" partition. - The factory reset will occur if there is a GPIO input pulled low while device starts up. - See settings below. + The factory reset will occur if there is a GPIO input held at the configured level while + device starts up. See settings below. config BOOTLOADER_NUM_PIN_FACTORY_RESET int "Number of the GPIO input for factory reset" @@ -127,17 +127,23 @@ menu "Bootloader config" range 0 44 if IDF_TARGET_ESP32S2 default 4 help - The selected GPIO will be configured as an input with internal pull-up enabled. - To trigger a factory reset, this GPIO must be pulled low on reset. - Note that GPIO34-39 do not have an internal pullup and an external one must be provided. + The selected GPIO will be configured as an input with internal pull-up enabled (note that on some SoCs. + not all pins have an internal pull-up, consult the hardware datasheet for details.) To trigger a factory + reset, this GPIO must be held high or low (as configured) on startup. - config BOOTLOADER_PIN_LEVEL_FACTORY_RESET - int "Level of the GPIO input for factory reset" + choice BOOTLOADER_FACTORY_RESET_PIN_LEVEL + bool "Factory reset GPIO level" depends on BOOTLOADER_FACTORY_RESET - range 0 1 - default 0 + default BOOTLOADER_FACTORY_RESET_PIN_LOW help - Pin level for factory reset. 0 for LOW and 1 for HIGH. + Pin level for factory reset, can be triggered on low or high. + + config BOOTLOADER_FACTORY_RESET_PIN_LOW + bool "Reset on GPIO low" + + config BOOTLOADER_FACTORY_RESET_PIN_HIGH + bool "Reset on GPIO high" + endchoice config BOOTLOADER_OTA_DATA_ERASE bool "Clear OTA data on factory reset (select factory partition)" diff --git a/components/bootloader/subproject/main/bootloader_start.c b/components/bootloader/subproject/main/bootloader_start.c index 6677707f73..eeb4060a09 100644 --- a/components/bootloader/subproject/main/bootloader_start.c +++ b/components/bootloader/subproject/main/bootloader_start.c @@ -79,7 +79,11 @@ static int selected_boot_partition(const bootloader_state_t *bs) if (bootloader_common_get_reset_reason(0) != DEEPSLEEP_RESET) { // Factory firmware. #ifdef CONFIG_BOOTLOADER_FACTORY_RESET - if (bootloader_common_check_long_hold_gpio(CONFIG_BOOTLOADER_NUM_PIN_FACTORY_RESET, CONFIG_BOOTLOADER_HOLD_TIME_GPIO, CONFIG_BOOTLOADER_PIN_LEVEL_FACTORY_RESET) == 1) { + bool reset_level = false; +#if CONFIG_BOOTLOADER_FACTORY_RESET_PIN_HIGH + reset_level = true; +#endif + if (bootloader_common_check_long_hold_gpio_level(CONFIG_BOOTLOADER_NUM_PIN_FACTORY_RESET, CONFIG_BOOTLOADER_HOLD_TIME_GPIO, reset_level) == GPIO_LONG_HOLD) { ESP_LOGI(TAG, "Detect a condition of the factory reset"); bool ota_data_erase = false; #ifdef CONFIG_BOOTLOADER_OTA_DATA_ERASE diff --git a/components/bootloader_support/include/bootloader_common.h b/components/bootloader_support/include/bootloader_common.h index 36a027fb33..2e7ad748da 100644 --- a/components/bootloader_support/include/bootloader_common.h +++ b/components/bootloader_support/include/bootloader_common.h @@ -62,18 +62,36 @@ bool bootloader_common_ota_select_valid(const esp_ota_select_entry_t *s); bool bootloader_common_ota_select_invalid(const esp_ota_select_entry_t *s); /** - * @brief Check if the GPIO input is a long hold or a short hold. + * @brief Check if a GPIO input is held low for a long period, short period, or not + * at all. + * + * This function will configure the specified GPIO as an input with internal pull-up enabled. * - * Number of the GPIO input will be configured as an input with internal pull-up enabled. * If the GPIO input is held low continuously for delay_sec period then it is a long hold. * If the GPIO input is held low for less period then it is a short hold. * * @param[in] num_pin Number of the GPIO input. * @param[in] delay_sec Input must be driven low for at least this long, continuously. - * @param[in] level Input pin level to trigger lang hold, 0 - LOW, 1 - HIGH - * @return esp_comm_gpio_hold_t Defines type of hold a GPIO in low state. + * @return esp_comm_gpio_hold_t Type of low level hold detected, if any. */ -esp_comm_gpio_hold_t bootloader_common_check_long_hold_gpio(uint32_t num_pin, uint32_t delay_sec, int level); +esp_comm_gpio_hold_t bootloader_common_check_long_hold_gpio(uint32_t num_pin, uint32_t delay_sec); + +/** + * @brief Check if a GPIO input is held low or high for a long period, short period, or not + * at all. + * + * This function will configure the specified GPIO as an input with internal pull-up enabled. + * + * If the GPIO input is held at 'level' continuously for delay_sec period then it is a long hold. + * If the GPIO input is held at 'level' for less period then it is a short hold. + * + * @param[in] num_pin Number of the GPIO input. + * @param[in] delay_sec Input must be driven to 'level' for at least this long, continuously. + * @param[in] level Input pin level to trigger on hold + * @return esp_comm_gpio_hold_t Type of hold detected, if any. + */ +esp_comm_gpio_hold_t bootloader_common_check_long_hold_gpio_level(uint32_t num_pin, uint32_t delay_sec, bool level); + /** * @brief Erase the partition data that is specified in the transferred list. diff --git a/components/bootloader_support/src/bootloader_common.c b/components/bootloader_support/src/bootloader_common.c index 106b2043a5..ad967ef7ca 100644 --- a/components/bootloader_support/src/bootloader_common.c +++ b/components/bootloader_support/src/bootloader_common.c @@ -39,7 +39,12 @@ static const char* TAG = "boot_comm"; -esp_comm_gpio_hold_t bootloader_common_check_long_hold_gpio(uint32_t num_pin, uint32_t delay_sec, int level) +esp_comm_gpio_hold_t bootloader_common_check_long_hold_gpio(uint32_t num_pin, uint32_t delay_sec) +{ + return bootloader_common_check_long_hold_gpio_level(num_pin, delay_sec, false); +} + +esp_comm_gpio_hold_t bootloader_common_check_long_hold_gpio_level(uint32_t num_pin, uint32_t delay_sec, bool level) { esp_rom_gpio_pad_select_gpio(num_pin); if (GPIO_PIN_MUX_REG[num_pin]) { diff --git a/docs/en/api-guides/bootloader.rst b/docs/en/api-guides/bootloader.rst index eba32ffb5e..1ffded1155 100644 --- a/docs/en/api-guides/bootloader.rst +++ b/docs/en/api-guides/bootloader.rst @@ -70,9 +70,11 @@ Partitions of type "app" cannot be specified here. :ref:`CONFIG_BOOTLOADER_OTA_DATA_ERASE` - the device will boot from "factory" partition after a factory reset. The OTA data partition will be cleared. -:ref:`CONFIG_BOOTLOADER_NUM_PIN_FACTORY_RESET`- number of the GPIO input for factory reset uses to trigger a factory reset, this GPIO must be pulled low on reset to trigger this. +:ref:`CONFIG_BOOTLOADER_NUM_PIN_FACTORY_RESET`- number of the GPIO input for factory reset uses to trigger a factory reset, this GPIO must be pulled low or high (configurable) on reset to trigger this. -:ref:`CONFIG_BOOTLOADER_HOLD_TIME_GPIO`- this is hold time of GPIO for reset/test mode (by default 5 seconds). The GPIO must be held low continuously for this period of time after reset before a factory reset or test partition boot (as applicable) is performed. +:ref:`CONFIG_BOOTLOADER_HOLD_TIME_GPIO`- this is hold time of GPIO for reset/test mode (by default 5 seconds). The GPIO must be held continuously for this period of time after reset before a factory reset or test partition boot (as applicable) is performed. + +:ref:`CONFIG_BOOTLOADER_FACTORY_RESET_PIN_LEVEL` - configure whether a factory reset should trigger on a high or low level of the GPIO. If the GPIO has an internal pullup then this is enabled before the pin is sampled, consult the {IDF_TARGET_NAME} datasheet for details on pin internal pullups. Partition table.:: From 634db50fcbab112da8a236b44ccb37b4c54ed45c Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Wed, 16 Jun 2021 17:51:10 +1000 Subject: [PATCH 3/4] ci: Bump UT count --- .gitlab/ci/target-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab/ci/target-test.yml b/.gitlab/ci/target-test.yml index c66bd7c643..64fa9daa45 100644 --- a/.gitlab/ci/target-test.yml +++ b/.gitlab/ci/target-test.yml @@ -441,7 +441,7 @@ UT_001: UT_002: extends: .unit_test_esp32_template - parallel: 15 + parallel: 16 tags: - ESP32_IDF - UT_T1_1 From bfb602d0fcab19d970265884c7e76786dde9d19d Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Wed, 16 Jun 2021 18:06:10 +1000 Subject: [PATCH 4/4] ci: Fix error message if insufficient jobs for test case tag --- .../tiny_test_fw/Utility/CIAssignTest.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tools/ci/python_packages/tiny_test_fw/Utility/CIAssignTest.py b/tools/ci/python_packages/tiny_test_fw/Utility/CIAssignTest.py index ae3b23425c..491f5c0475 100644 --- a/tools/ci/python_packages/tiny_test_fw/Utility/CIAssignTest.py +++ b/tools/ci/python_packages/tiny_test_fw/Utility/CIAssignTest.py @@ -46,9 +46,12 @@ import re import yaml try: - from yaml import CLoader as Loader + from yaml import CLoader + has_cloader = True except ImportError: - from yaml import Loader as Loader + has_cloader = False + +from yaml import Loader from . import CaseConfig, GitlabCIJob, SearchCases, console_log @@ -180,7 +183,7 @@ class AssignTest(object): def _parse_gitlab_ci_config(self, ci_config_file): with open(ci_config_file, 'r') as f: - ci_config = yaml.load(f, Loader=Loader) + ci_config = yaml.load(f, Loader=CLoader if has_cloader else Loader) job_list = list() for job_name in ci_config: @@ -319,7 +322,7 @@ class AssignTest(object): # failures if failed_to_assign: console_log('Too many test cases vs jobs to run. ' - 'Please increase parallel count in tools/ci/config/target-test.yml ' + 'Please increase parallel count in .gitlab/ci/target-test.yml ' 'for jobs with specific tags:', 'R') failed_group_count = self._count_groups_by_keys(failed_to_assign) for tags in failed_group_count: