Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

I2C default ISR resets with Cache disabled but cached memory region accessed (IDFGH-6795) #8422

Closed
HassanSaied opened this issue Feb 17, 2022 · 3 comments
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally

Comments

@HassanSaied
Copy link

HassanSaied commented Feb 17, 2022

Environment

  • Development Kit: [none]
  • Module or chip used: [ESP32-WROOM-32]
  • IDF version (run git describe --tags to find it):
    v4.4-rc1-8-g0b46ac1732
  • Build System: [idf.py]
  • Compiler version (run xtensa-esp32-elf-gcc --version to find it):
    xtensa-esp32-elf-gcc (crosstool-NG esp-2021r2) 8.4.0
  • Operating System: [Linux]
  • Using an IDE?: [No|Yes (please give details)]
  • VSCode or CLion
  • Power Supply: [USB + Battery]

Problem Description

When setting the I2C interrupt to ESP_INTR_FLAG_IRAM in the i2c_driver_install, the I2C interrupt will reset the system with a cache disabled error when accessing the SPIFFS at the same time the I2C module is reading data from a slave

Expected Behavior

System doesn't reset when using an ISR allocated in IRAM while cache is disabled

Actual Behavior

System resets

Steps to reproduce

  1. Install I2C driver with ESP_INTR_FLAG_IRAM option
  2. Communicate with an I2C slave, initiate some form of long read operation (reading multiple values)
  3. In another task write a file to the SPIFFS
  4. Make sure they are both happening in the same time, so that the I2C interrupt gets called while the cache is disabled

Debug Logs

Guru Meditation Error: Core  0 panic'ed (Cache disabled but cached memory region accessed). 
Core  0 register dump:
PC      : 0x4015b278  PS      : 0x00060035  A0      : 0x8008af0d  A1      : 0x3ffbe8e0  
0x4015b278: i2c_hal_update_config at /home/hassan/esp/esp-idf/components/hal/i2c_hal.c:223

A2      : 0x00000000  A3      : 0x00000000  A4      : 0x3ffcf370  A5      : 0x3ffbc5e0  
A6      : 0x00000026  A7      : 0x3ffd8210  A8      : 0x8008ae40  A9      : 0x00000000  
A10     : 0x3ffcf370  A11     : 0x00000000  A12     : 0x00000000  A13     : 0x00000800  
A14     : 0x00000000  A15     : 0x3ff53000  SAR     : 0x00000018  EXCCAUSE: 0x00000007  
EXCVADDR: 0x00000000  LBEG    : 0x4000c2e0  LEND    : 0x4000c2f6  LCOUNT  : 0x00000002  
Backtrace:0x4015b275:0x3ffbe8e00x4008af0a:0x3ffbe910 0x40082f95:0x3ffbe940 0x4000c2e5:0x3ffbc5c0 0x40096c6e:0x3ffbc5d0 0x4009ae8d:0x3ffbc600 0x4008d7d7:0x3ffbc680 0x40136a09:0x3ffbc6c0 0x4012a2c7:0x3ffbc6f0 0x4012dd87:0x3ffbc720 0x4012b9bf:0x3ffbc750 0x4012d196:0x3ffbc790 0x4012a948:0x3ffbc7d0 0x4012a0c9:0x3ffbc800 0x400d4331:0x3ffbc820 0x4014567c:0x3ffbc840 0x40145725:0x3ffbc870 0x400dbd48:0x3ffbc890 0x400dd680:0x3ffbc8c0 0x400d6c1d:0x3ffbc8f0 0x400d7c11:0x3ffbc910 0x400d6a3f:0x3ffbc9a0 0x4015ba89:0x3ffbc9c0 0x40095eb5:0x3ffbc9e0 
0x4015b275: i2c_hal_enable_slave_rx_it at /home/hassan/esp/esp-idf/components/hal/i2c_hal.c:170
0x4008af0a: i2c_isr_handler_default at /home/hassan/esp/esp-idf/components/driver/i2c.c:497
0x40082f95: _xt_lowint1 at /home/hassan/esp/esp-idf/components/freertos/port/xtensa/xtensa_vectors.S:1111
0x40096c6e: spi_flash_ll_get_buffer_data at /home/hassan/esp/esp-idf/components/hal/esp32/include/hal/spi_flash_ll.h:148
 (inlined by) spi_flash_hal_read at /home/hassan/esp/esp-idf/components/hal/spi_flash_hal_common.inc:181
0x4009ae8d: spi_flash_chip_generic_read at /home/hassan/esp/esp-idf/components/spi_flash/spi_flash_chip_generic.c:218
0x4008d7d7: esp_flash_read at /home/hassan/esp/esp-idf/components/spi_flash/esp_flash_api.c:832 (discriminator 4)
0x40136a09: esp_partition_read at /home/hassan/esp/esp-idf/components/spi_flash/partition.c:424
0x4012a2c7: spiffs_api_read at /home/hassan/esp/esp-idf/components/spiffs/spiffs_api.c:36
0x4012dd87: spiffs_phys_rd at /home/hassan/esp/esp-idf/components/spiffs/spiffs/src/spiffs_cache.c:161
0x4012b9bf: spiffs_obj_lu_find_entry_visitor at /home/hassan/esp/esp-idf/components/spiffs/spiffs/src/spiffs_nucleus.c:166
0x4012d196: spiffs_object_find_object_index_header_by_name at /home/hassan/esp/esp-idf/components/spiffs/spiffs/src/spiffs_nucleus.c:1694
0x4012a948: SPIFFS_open at /home/hassan/esp/esp-idf/components/spiffs/spiffs/src/spiffs_hydrogen.c:230 (discriminator 2)
0x4012a0c9: vfs_spiffs_open at /home/hassan/esp/esp-idf/components/spiffs/esp_spiffs.c:479
0x400d4331: esp_vfs_open at /home/hassan/esp/esp-idf/components/vfs/vfs.c:399 (discriminator 3)
0x4014567c: _fopen_r at /builds/idf/crosstool-NG/.build/xtensa-esp32-elf/src/newlib/newlib/libc/stdio/fopen.c:129
0x40145725: fopen at /builds/idf/crosstool-NG/.build/xtensa-esp32-elf/src/newlib/newlib/libc/stdio/fopen.c:168
0x400dbd48: readFile at /home/hassan/Documents/Si-Ware/Scanner/ScannerFirmware/components/FileSystem/fileSystem.c:24
0x400dd680: fileSystem_register_gauge_reset at /home/hassan/Documents/Si-Ware/Scanner/ScannerFirmware/components/FileSystem/fileSystem.c:1160
0x400d6c1d: centralManager_run_powerManagement_init at /home/hassan/Documents/Si-Ware/Scanner/ScannerFirmware/components/CentralManager/centralManager.c:40
0x400d7c11: centralManager_systemStart at /home/hassan/Documents/Si-Ware/Scanner/ScannerFirmware/components/CentralManager/centralManager.c:280 (discriminator 13)
0x400d6a3f: app_main at /home/hassan/Documents/Si-Ware/Scanner/ScannerFirmware/main/main.c:5
0x4015ba89: main_task at /home/hassan/esp/esp-idf/components/freertos/port/port_common.c:129 (discriminator 2)
0x40095eb5: vPortTaskWrapper at /home/hassan/esp/esp-idf/components/freertos/port/xtensa/port.c:131

The core dump for the same reset shows a slightly different story, but a much more concise one as i2c_hal_enable_slave_rx_it should never be called, as my I2C is in master mode

#0  i2c_hal_update_config (hal=0x0) at /home/hassan/esp/esp-idf/components/hal/i2c_hal.c:223
#1  0x4008ae40 in i2c_master_cmd_begin_static (i2c_num=0) at /home/hassan/esp/esp-idf/components/driver/i2c.c:1369
#2  0x4008af0d in i2c_isr_handler_default (arg=0x3ffdda4c) at /home/hassan/esp/esp-idf/components/driver/i2c.c:497
#3  0x40082f98 in _xt_lowint1 () at /home/hassan/esp/esp-idf/components/freertos/port/xtensa/xtensa_vectors.S:1111
Backtrace stopped: previous frame inner to this frame (corrupt stack?)

Other items if possible

I did some investigation and I found that the issue is that the function i2c_hal_update_config which is called in i2c_master_cmd_begin_static is not defined with IRAM_ATTR, and it seems to cause the error, because when I comment it every thing works fine.
On my ESP32 this function seems to do nothing, but I think it is necessary for other platforms.
A proposed fix would be to skip the i2c_hal_update_config and directly call the lower layer i2c_ll_update (which is already inline so would pose no problems) via a macro. This fix is in the included patch

  • [ yes] sdkconfig file (attach the sdkconfig file from your project folder)
  • [ yes] elf file in the build folder (note this may contain all the code details and symbols of your project.)
  • [ yes] coredump (This provides stacks of tasks.)
    I2CIssue.zip
@espressif-bot espressif-bot added the Status: Opened Issue is new label Feb 17, 2022
@github-actions github-actions bot changed the title I2C default ISR resets with Cache disabled but cached memory region accessed I2C default ISR resets with Cache disabled but cached memory region accessed (IDFGH-6795) Feb 17, 2022
@HassanSaied
Copy link
Author

diff --git a/components/driver/i2c.c b/components/driver/i2c.c
index 438d1efc12..3637288596 100644
--- a/components/driver/i2c.c
+++ b/components/driver/i2c.c
@@ -1366,7 +1366,7 @@ static void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num)
             break;
         }
     }
-    i2c_hal_update_config(&(i2c_context[i2c_num].hal));
+    i2c_hal_update_config_isr(&(i2c_context[i2c_num].hal));
     i2c_hal_trans_start(&(i2c_context[i2c_num].hal));
     return;
 }
diff --git a/components/esptool_py/esptool b/components/esptool_py/esptool
--- a/components/esptool_py/esptool
+++ b/components/esptool_py/esptool
@@ -1 +1 @@
-Subproject commit b082b0ed2d86b3330134c4854a021dfd14c29b08
+Subproject commit b082b0ed2d86b3330134c4854a021dfd14c29b08-dirty
diff --git a/components/hal/include/hal/i2c_hal.h b/components/hal/include/hal/i2c_hal.h
index b255878cc4..698d0da5d8 100644
--- a/components/hal/include/hal/i2c_hal.h
+++ b/components/hal/include/hal/i2c_hal.h
@@ -91,6 +91,7 @@ typedef struct {
  * @return None
  */
 #define i2c_hal_enable_master_tx_it(hal)    i2c_ll_master_enable_tx_it((hal)->dev)
+#define i2c_hal_update_config_isr(hal)        i2c_ll_update((hal)->dev)
 
 /**
  * @brief  Clear I2C slave TX interrupt

@ginkgm
Copy link
Collaborator

ginkgm commented Feb 17, 2022

Hi @HassanSaied Thanks for reporting this issue, and bringing up this fix.

We will look into this issue more detailedly to find out all the wrong cases. This kind of issues are a bit hard to fix completely, because the inline logic is somehow determined by the compiler behavior under different configs...

Before that, you can work with your fix, or try add FORCE_INLINE_ATTR to the i2c_ll_update function.

@espressif-bot espressif-bot added Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: Opened Issue is new Resolution: NA Issue resolution is unavailable labels Mar 3, 2022
@AxelLin
Copy link
Contributor

AxelLin commented Apr 6, 2022

@ginkgm
Require to backport stable branches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally
Projects
None yet
Development

No branches or pull requests

4 participants