Merge branch 'bugfix/multiple_backports_v3.2' into 'release/v3.2'

multiple backports (v3.2)

See merge request idf/esp-idf!4367

.gitlab-ci.yml

@@ -729,8 +729,11 @@ check_ut_cmake_make:
 
 check_submodule_sync:
   <<: *check_job_template
+  tags:
+    - github_sync
   variables:
     GIT_STRATEGY: clone
+  retry: 2
   script:
     # check if all submodules are correctly synced to public repostory
     - git submodule update --init --recursive

components/bootloader_support/src/bootloader_clock.c

@@ -31,14 +31,14 @@ void bootloader_clock_configure()
     /* Set CPU to 80MHz. Keep other clocks unmodified. */
     int cpu_freq_mhz = 80;
 
-    /* On ESP32 rev 0, switching to 80MHz if clock was previously set to
+    /* On ESP32 rev 0, switching to 80/160 MHz if clock was previously set to
      * 240 MHz may cause the chip to lock up (see section 3.5 of the errata
-     * document). For rev. 0, switch to 240 instead if it was chosen in
+     * document). For rev. 0, switch to 240 instead if it has been enabled
-     * menuconfig.
+     * previously.
      */
     uint32_t chip_ver_reg = REG_READ(EFUSE_BLK0_RDATA3_REG);
     if ((chip_ver_reg & EFUSE_RD_CHIP_VER_REV1_M) == 0 &&
-            CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ == 240) {
+            DPORT_REG_GET_FIELD(DPORT_CPU_PER_CONF_REG, DPORT_CPUPERIOD_SEL) == DPORT_CPUPERIOD_SEL_240) {
         cpu_freq_mhz = 240;
     }
 

components/esp32/esp_timer_esp32.c

@@ -97,7 +97,7 @@ static uint32_t s_alarm_overflow_val = DEFAULT_ALARM_OVERFLOW_VAL;
 
 static const char* TAG = "esp_timer_impl";
 
-// Interrupt handle retuned by the interrupt allocator
+// Interrupt handle returned by the interrupt allocator
 static intr_handle_t s_timer_interrupt_handle;
 
 // Function from the upper layer to be called when the interrupt happens.
@@ -123,7 +123,7 @@ static uint32_t s_timer_us_per_overflow;
 // value than the one which caused an interrupt. This can cause interrupt handler
 // to consider that the interrupt has happened due to timer overflow, incrementing
 // s_time_base_us. To avoid this, frequency switch hook sets this flag if
-// it needs to set timer alarm value to ALARM_OVERFLOW_VAL. Interrupt hanler
+// it needs to set timer alarm value to ALARM_OVERFLOW_VAL. Interrupt handler
 // will not increment s_time_base_us if this flag is set.
 static bool s_mask_overflow;
 

components/esp32/test/test_esp_timer.c

@@ -24,7 +24,12 @@ static uint32_t s_old_overflow_val;
 static void setup_overflow()
 {
     s_old_overflow_val = esp_timer_impl_get_overflow_val();
-    esp_timer_impl_set_overflow_val(0x7fffff); /* overflow every ~0.1 sec */}
+    /* Overflow every 0.1 sec.
+     * Chosen so that it is 0 modulo s_timer_ticks_per_us (which is 80),
+     * to prevent roundoff error on each overflow.
+     */
+    esp_timer_impl_set_overflow_val(8000000);
+}
 
 static void teardown_overflow()
 {
@@ -404,6 +409,13 @@ TEST_CASE("esp_timer_get_time call takes less than 1us", "[esp_timer]")
     TEST_PERFORMANCE_LESS_THAN(ESP_TIMER_GET_TIME_PER_CALL, "%dns", ns_per_call);
 }
 
+static int64_t IRAM_ATTR __attribute__((noinline)) get_clock_diff()
+{
+    uint64_t hs_time = esp_timer_get_time();
+    uint64_t ref_time = ref_clock_get();
+    return hs_time - ref_time;
+}
+
 TEST_CASE("esp_timer_get_time returns monotonic values", "[esp_timer]")
 {
     typedef struct {
@@ -411,24 +423,33 @@ TEST_CASE("esp_timer_get_time returns monotonic values", "[esp_timer]")
         bool pass;
         int test_cnt;
         int error_cnt;
-        int64_t total_sq_error;
         int64_t max_error;
+        int64_t avg_diff;
+        int64_t dummy;
     } test_state_t;
 
     void timer_test_task(void* arg) {
         test_state_t* state = (test_state_t*) arg;
         state->pass = true;
-        int64_t start_time = ref_clock_get();
-        int64_t delta = esp_timer_get_time() - start_time;
 
-        int64_t now = start_time;
+        /* make sure both functions are in cache */
+        state->dummy = get_clock_diff();
+
+        /* calculate the difference between the two clocks */
+        portDISABLE_INTERRUPTS();
+        int64_t delta = get_clock_diff();
+        portENABLE_INTERRUPTS();
+        int64_t start_time = ref_clock_get();
         int error_repeat_cnt = 0;
-        while (now - start_time < 10000000) {  /* 10 seconds */
+        while (ref_clock_get() - start_time < 10000000) {  /* 10 seconds */
-            int64_t hs_now = esp_timer_get_time();
+            /* Get values of both clocks again, and check that they are close to 'delta'.
-            now = ref_clock_get();
+             * We don't disable interrupts here, because esp_timer_get_time doesn't lock
-            int64_t diff = hs_now - (now + delta);
+             * interrupts internally, so we check if it can get "broken" by a well placed
+             * interrupt.
+             */
+            int64_t diff = get_clock_diff() - delta;
             /* Allow some difference due to rtos tick interrupting task between
-             * getting 'now' and 'ref_now'.
+             * getting 'hs_now' and 'now'.
              */
             if (abs(diff) > 100) {
                 error_repeat_cnt++;
@@ -440,10 +461,11 @@ TEST_CASE("esp_timer_get_time returns monotonic values", "[esp_timer]")
                 printf("diff=%lld\n", diff);
                 state->pass = false;
             }
+            state->avg_diff += diff;
             state->max_error = MAX(state->max_error, abs(diff));
             state->test_cnt++;
-            state->total_sq_error += diff * diff;
         }
+        state->avg_diff /= state->test_cnt;
         xSemaphoreGive(state->done);
         vTaskDelete(NULL);
     }
@@ -460,10 +482,10 @@ TEST_CASE("esp_timer_get_time returns monotonic values", "[esp_timer]")
 
     for (int i = 0; i < portNUM_PROCESSORS; ++i) {
         TEST_ASSERT_TRUE( xSemaphoreTake(done, portMAX_DELAY) );
-        printf("CPU%d: %s test_cnt=%d error_cnt=%d std_error=%d |max_error|=%d\n",
+        printf("CPU%d: %s test_cnt=%d error_cnt=%d avg_diff=%d |max_error|=%d\n",
                 i, states[i].pass ? "PASS" : "FAIL",
                 states[i].test_cnt, states[i].error_cnt,
-                (int) sqrt(states[i].total_sq_error / states[i].test_cnt), (int) states[i].max_error);
+                (int) states[i].avg_diff, (int) states[i].max_error);
     }
 
     vSemaphoreDelete(done);

components/nvs_flash/src/nvs_item_hash_list.cpp

@@ -64,15 +64,22 @@ void HashList::erase(size_t index, bool itemShouldExist)
 {
     for (auto it = mBlockList.begin(); it != mBlockList.end();) {
         bool haveEntries = false;
+        bool foundIndex = false;
         for (size_t i = 0; i < it->mCount; ++i) {
             if (it->mNodes[i].mIndex == index) {
                 it->mNodes[i].mIndex = 0xff;
-                return;
+                foundIndex = true;
+                /* found the item and removed it */
             }
             if (it->mNodes[i].mIndex != 0xff) {
                 haveEntries = true;
             }
+            if (haveEntries && foundIndex) {
+                /* item was found, and HashListBlock still has some items */
+                return;
             }
+        }
+        /* no items left in HashListBlock, can remove */
         if (!haveEntries) {
             auto tmp = it;
             ++it;
@@ -81,6 +88,10 @@ void HashList::erase(size_t index, bool itemShouldExist)
         } else {
             ++it;
         }
+        if (foundIndex) {
+            /* item was found and empty HashListBlock was removed */
+            return;
+        }
     }
     if (itemShouldExist) {
         assert(false && "item should have been present in cache");

components/nvs_flash/test_nvs_host/test_nvs.cpp

@@ -284,6 +284,47 @@ TEST_CASE("Page handles invalid CRC of variable length items", "[nvs][cur]")
     }
 }
 
+class HashListTestHelper : public HashList
+{
+    public:
+        size_t getBlockCount()
+        {
+            return mBlockList.size();
+        }
+};
+
+TEST_CASE("HashList is cleaned up as soon as items are erased", "[nvs]")
+{
+    HashListTestHelper hashlist;
+    // Add items
+    const size_t count = 128;
+    for (size_t i = 0; i < count; ++i) {
+        char key[16];
+        snprintf(key, sizeof(key), "i%ld", (long int)i);
+        Item item(1, ItemType::U32, 1, key);
+        hashlist.insert(item, i);
+    }
+    INFO("Added " << count << " items, " << hashlist.getBlockCount() << " blocks");
+    // Remove them in reverse order
+    for (size_t i = count; i > 0; --i) {
+        hashlist.erase(i - 1, true);
+    }
+    CHECK(hashlist.getBlockCount() == 0);
+    // Add again
+    for (size_t i = 0; i < count; ++i) {
+        char key[16];
+        snprintf(key, sizeof(key), "i%ld", (long int)i);
+        Item item(1, ItemType::U32, 1, key);
+        hashlist.insert(item, i);
+    }
+    INFO("Added " << count << " items, " << hashlist.getBlockCount() << " blocks");
+    // Remove them in the same order
+    for (size_t i = 0; i < count; ++i) {
+        hashlist.erase(i, true);
+    }
+    CHECK(hashlist.getBlockCount() == 0);
+}
+
 TEST_CASE("can init PageManager in empty flash", "[nvs]")
 {
     SpiFlashEmulator emu(4);
@@ -1716,6 +1757,28 @@ TEST_CASE("Check that orphaned blobs are erased during init", "[nvs]")
     TEST_ESP_OK(storage.writeItem(1, ItemType::BLOB, "key3", blob, sizeof(blob)));
 }
 
+TEST_CASE("nvs blob fragmentation test", "[nvs]")
+{
+    SpiFlashEmulator emu(4);
+    TEST_ESP_OK(nvs_flash_init_custom(NVS_DEFAULT_PART_NAME, 0, 4) );
+    const size_t BLOB_SIZE = 3500;
+    uint8_t *blob = (uint8_t*) malloc(BLOB_SIZE);
+    CHECK(blob != NULL);
+    memset(blob, 0xEE, BLOB_SIZE);
+    const uint32_t magic = 0xff33eaeb;
+    nvs_handle h;
+    TEST_ESP_OK( nvs_open("blob_tests", NVS_READWRITE, &h) );
+    for (int i = 0; i < 128; i++) {
+        INFO("Iteration " << i << "...\n");
+        TEST_ESP_OK( nvs_set_u32(h, "magic", magic) );
+        TEST_ESP_OK( nvs_set_blob(h, "blob", blob, BLOB_SIZE) );
+        char seq_buf[16];
+        sprintf(seq_buf, "seq%d", i);
+        TEST_ESP_OK( nvs_set_u32(h, seq_buf, i) );
+    }
+    free(blob);
+}
+
 TEST_CASE("nvs code handles errors properly when partition is near to full", "[nvs]")
 {
     const size_t blob_size = Page::CHUNK_MAX_SIZE * 0.3 ;

components/soc/esp32/include/soc/dport_reg.h

@@ -179,6 +179,9 @@
 #define DPORT_CPUPERIOD_SEL_M  ((DPORT_CPUPERIOD_SEL_V)<<(DPORT_CPUPERIOD_SEL_S))
 #define DPORT_CPUPERIOD_SEL_V  0x3
 #define DPORT_CPUPERIOD_SEL_S  0
+#define DPORT_CPUPERIOD_SEL_80		0
+#define DPORT_CPUPERIOD_SEL_160		1
+#define DPORT_CPUPERIOD_SEL_240		2
 
 #define DPORT_PRO_CACHE_CTRL_REG          (DR_REG_DPORT_BASE + 0x040)
 /* DPORT_PRO_DRAM_HL : R/W ;bitpos:[16] ;default: 1'b0 ; */

components/soc/esp32/rtc_clk.c

@@ -395,7 +395,6 @@ void rtc_clk_cpu_freq_to_xtal(int freq, int div)
     REG_WRITE(APB_CTRL_XTAL_TICK_CONF_REG, freq * MHZ / REF_CLK_FREQ - 1);
     /* switch clock source */
     REG_SET_FIELD(RTC_CNTL_CLK_CONF_REG, RTC_CNTL_SOC_CLK_SEL, RTC_CNTL_SOC_CLK_SEL_XTL);
-    DPORT_REG_WRITE(DPORT_CPU_PER_CONF_REG, 0); /* clear DPORT_CPUPERIOD_SEL */
     rtc_clk_apb_freq_update(freq * MHZ);
     /* lower the voltage */
     if (freq <= 2) {
@@ -411,7 +410,6 @@ static void rtc_clk_cpu_freq_to_8m()
     REG_SET_FIELD(RTC_CNTL_REG, RTC_CNTL_DIG_DBIAS_WAK, DIG_DBIAS_XTAL);
     REG_SET_FIELD(APB_CTRL_SYSCLK_CONF_REG, APB_CTRL_PRE_DIV_CNT, 0);
     REG_SET_FIELD(RTC_CNTL_CLK_CONF_REG, RTC_CNTL_SOC_CLK_SEL, RTC_CNTL_SOC_CLK_SEL_8M);
-    DPORT_REG_WRITE(DPORT_CPU_PER_CONF_REG, 0); // clear DPORT_CPUPERIOD_SEL
     rtc_clk_apb_freq_update(RTC_FAST_CLK_FREQ_8M);
 }
 
@@ -452,14 +450,14 @@ static void rtc_clk_bbpll_enable()
 static void rtc_clk_cpu_freq_to_pll_mhz(int cpu_freq_mhz)
 {
     int dbias = DIG_DBIAS_80M_160M;
-    int per_conf = 0;
+    int per_conf = DPORT_CPUPERIOD_SEL_80;
     if (cpu_freq_mhz == 80) {
         /* nothing to do */
     } else if (cpu_freq_mhz == 160) {
-        per_conf = 1;
+        per_conf = DPORT_CPUPERIOD_SEL_160;
     } else if (cpu_freq_mhz == 240) {
         dbias = DIG_DBIAS_240M;
-        per_conf = 2;
+        per_conf = DPORT_CPUPERIOD_SEL_240;
     } else {
         SOC_LOGE(TAG, "invalid frequency");
         abort();
@@ -687,15 +685,15 @@ void rtc_clk_cpu_freq_get_config(rtc_cpu_freq_config_t* out_config)
         case RTC_CNTL_SOC_CLK_SEL_PLL: {
             source = RTC_CPU_FREQ_SRC_PLL;
             uint32_t cpuperiod_sel = DPORT_REG_GET_FIELD(DPORT_CPU_PER_CONF_REG, DPORT_CPUPERIOD_SEL);
-            if (cpuperiod_sel == 0) {
+            if (cpuperiod_sel == DPORT_CPUPERIOD_SEL_80) {
                 source_freq_mhz = RTC_PLL_FREQ_320M;
                 div = 4;
                 freq_mhz = 80;
-            } else if (cpuperiod_sel == 1) {
+            } else if (cpuperiod_sel == DPORT_CPUPERIOD_SEL_160) {
                 source_freq_mhz = RTC_PLL_FREQ_320M;
                 div = 2;
                 freq_mhz = 160;
-            } else if (cpuperiod_sel == 2) {
+            } else if (cpuperiod_sel == DPORT_CPUPERIOD_SEL_240) {
                 source_freq_mhz = RTC_PLL_FREQ_480M;
                 div = 2;
                 freq_mhz = 240;

make/project.mk

@@ -133,6 +133,9 @@ ifndef COMPONENT_DIRS
 EXTRA_COMPONENT_DIRS ?=
 COMPONENT_DIRS := $(PROJECT_PATH)/components $(EXTRA_COMPONENT_DIRS) $(IDF_PATH)/components $(PROJECT_PATH)/main
 endif
+# Make sure that every directory in the list is an absolute path without trailing slash.
+# This is necessary to split COMPONENT_DIRS into SINGLE_COMPONENT_DIRS and MULTI_COMPONENT_DIRS below. 
+COMPONENT_DIRS := $(foreach cd,$(COMPONENT_DIRS),$(abspath $(cd)))
 export COMPONENT_DIRS
 
 ifdef SRCDIRS
@@ -140,20 +143,31 @@ $(warning SRCDIRS variable is deprecated. These paths can be added to EXTRA_COMP
 COMPONENT_DIRS += $(abspath $(SRCDIRS))
 endif
 
-# The project Makefile can define a list of components, but if it does not do this we just take all available components
+# List of component directories, i.e. directories which contain a component.mk file 
-# in the component dirs. A component is COMPONENT_DIRS directory, or immediate subdirectory,
+SINGLE_COMPONENT_DIRS := $(abspath $(dir $(dir $(foreach cd,$(COMPONENT_DIRS),\
+                             $(wildcard $(cd)/component.mk)))))
+
+# List of components directories, i.e. directories which may contain components 
+MULTI_COMPONENT_DIRS := $(filter-out $(SINGLE_COMPONENT_DIRS),$(COMPONENT_DIRS))
+
+# The project Makefile can define a list of components, but if it does not do this
+# we just take all available components in the component dirs.
+# A component is COMPONENT_DIRS directory, or immediate subdirectory,
 # which contains a component.mk file.
 #
 # Use the "make list-components" target to debug this step.
 ifndef COMPONENTS
 # Find all component names. The component names are the same as the
 # directories they're in, so /bla/components/mycomponent/component.mk -> mycomponent.
-COMPONENTS := $(dir $(foreach cd,$(COMPONENT_DIRS),                           \
+# We need to do this for MULTI_COMPONENT_DIRS only, since SINGLE_COMPONENT_DIRS
-					$(wildcard $(cd)/*/component.mk) $(wildcard $(cd)/component.mk) \
+# are already known to contain component.mk.
-				))
+COMPONENTS := $(dir $(foreach cd,$(MULTI_COMPONENT_DIRS),$(wildcard $(cd)/*/component.mk))) \
+              $(SINGLE_COMPONENT_DIRS)
 COMPONENTS := $(sort $(foreach comp,$(COMPONENTS),$(lastword $(subst /, ,$(comp)))))
 endif
-# After a full manifest of component names is determined, subtract the ones explicitly omitted by the project Makefile.
+# After a full manifest of component names is determined, subtract the ones explicitly
+# omitted by the project Makefile.
+EXCLUDE_COMPONENTS ?=
 ifdef EXCLUDE_COMPONENTS
 COMPONENTS := $(filter-out $(subst ",,$(EXCLUDE_COMPONENTS)), $(COMPONENTS))
 # to keep syntax highlighters happy: "))
@@ -161,12 +175,23 @@ endif
 export COMPONENTS
 
 # Resolve all of COMPONENTS into absolute paths in COMPONENT_PATHS.
+# For each entry in COMPONENT_DIRS:
+# - either this is directory with multiple components, in which case check that
+#   a subdirectory with component name exists, and it contains a component.mk file.
+# - or, this is a directory of a single component, in which case the name of this
+#   directory has to match the component name
 #
 # If a component name exists in multiple COMPONENT_DIRS, we take the first match.
 #
 # NOTE: These paths must be generated WITHOUT a trailing / so we
 # can use $(notdir x) to get the component name.
-COMPONENT_PATHS := $(foreach comp,$(COMPONENTS),$(firstword $(foreach cd,$(COMPONENT_DIRS),$(wildcard $(dir $(cd))$(comp) $(cd)/$(comp)))))
+COMPONENT_PATHS := $(foreach comp,$(COMPONENTS),\
+                        $(firstword $(foreach cd,$(COMPONENT_DIRS),\
+                            $(if $(findstring $(cd),$(MULTI_COMPONENT_DIRS)),\
+                                 $(abspath $(dir $(wildcard $(cd)/$(comp)/component.mk))),)\
+                            $(if $(findstring $(cd),$(SINGLE_COMPONENT_DIRS)),\
+                                 $(if $(filter $(comp),$(notdir $(cd))),$(cd),),)\
+                   )))
 export COMPONENT_PATHS
 
 TEST_COMPONENTS ?=

tools/ci/test_build_system.sh

@@ -238,7 +238,7 @@ function run_tests()
     ( make 2>&1 | grep "does not fit in configured flash size 1MB" ) || failure "Build didn't fail with expected flash size failure message"
     mv sdkconfig.bak sdkconfig
 
-    print_status "sdkconfig should have contents both files: sdkconfig and sdkconfig.defaults"
+    print_status "sdkconfig should have contents of both files: sdkconfig and sdkconfig.defaults"
     make clean > /dev/null;
     rm -f sdkconfig.defaults;
     rm -f sdkconfig;
@@ -247,6 +247,35 @@ function run_tests()
     make defconfig > /dev/null;
     grep "CONFIG_PARTITION_TABLE_OFFSET=0x10000" sdkconfig || failure "The define from sdkconfig.defaults should be into sdkconfig"
     grep "CONFIG_PARTITION_TABLE_TWO_OTA=y" sdkconfig || failure "The define from sdkconfig should be into sdkconfig"
+    rm sdkconfig sdkconfig.defaults
+    make defconfig
+
+    print_status "Empty directory not treated as a component"
+    mkdir -p components/esp32
+    make || failure "Failed to build with empty esp32 directory in components"
+    rm -rf components
+
+    print_status "If a component directory is added to COMPONENT_DIRS, its subdirectories are not added"
+    mkdir -p main/test
+    touch main/test/component.mk
+    echo "#error This should not be built" > main/test/test.c
+    make || failure "COMPONENT_DIRS has added component subdirectory to the build"
+    rm -rf main/test
+
+    print_status "If a component directory is added to COMPONENT_DIRS, its sibling directories are not added"
+    mkdir -p mycomponents/mycomponent
+    touch mycomponents/mycomponent/component.mk
+    # first test by adding single component directory to EXTRA_COMPONENT_DIRS
+    mkdir -p mycomponents/esp32
+    touch mycomponents/esp32/component.mk
+    make EXTRA_COMPONENT_DIRS=$PWD/mycomponents/mycomponent || failure "EXTRA_COMPONENT_DIRS has added a sibling directory"
+    rm -rf mycomponents/esp32
+    # now the same thing, but add a components directory
+    mkdir -p esp32
+    touch esp32/component.mk
+    make EXTRA_COMPONENT_DIRS=$PWD/mycomponents || failure "EXTRA_COMPONENT_DIRS has added a sibling directory"
+    rm -rf esp32
+    rm -rf mycomponents
 
     print_status "All tests completed"
     if [ -n "${FAILURES}" ]; then

tools/unit-test-app/components/unity/ref_clock.c

@@ -119,6 +119,8 @@ void ref_clock_init()
     PCNT.ctrl.val |= BIT(REF_CLOCK_PCNT_UNIT * 2);
     PCNT.ctrl.val &= ~BIT(REF_CLOCK_PCNT_UNIT * 2);
 
+    ets_delay_us(10000);
+
     // Enable interrupt
     s_milliseconds = 0;
     ESP_ERROR_CHECK(esp_intr_alloc(ETS_PCNT_INTR_SOURCE, ESP_INTR_FLAG_IRAM, pcnt_isr, NULL, &s_intr_handle));