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

ADC continuous does crash when CONFIG_GDMA_ISR_IRAM_SAFE=yes (IDFGH-10537) #11781

Closed
3 tasks done
pggh opened this issue Jun 30, 2023 · 6 comments
Closed
3 tasks done
Assignees
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Type: Bug bugs in IDF

Comments

@pggh
Copy link

pggh commented Jun 30, 2023

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.

5.0

Operating System used.

Windows

How did you build your project?

Command line with idf.py

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

None

Development Kit.

ESP32-S3

Power Supply used.

USB

What is the expected behavior?

ADC continuous driver should work

What is the actual behavior?

Guru Meditation Error: Core 0 panic'ed (Cache disabled but cached memory region accessed).

Core 0 register dump:
PC : 0x4206a48a PS : 0x00060034 A0 : 0x8037998c A1 : 0x3fc9a3c0
0x4206a48a: adc_hal_get_reading_result at C:/Applications/Develop/SDK/esp-idf-5.0/frameworks/esp-idf-v5.0/components/hal/adc_hal.c:291

A2 : 0x00000000 A3 : 0x3fca7ba8 A4 : 0x3fca7ba4 A5 : 0x3fca6740
A6 : 0x3fc9ac48 A7 : 0x3fca6884 A8 : 0x803798eb A9 : 0x00000000
A10 : 0x3fca7c28 A11 : 0x3fca7c28 A12 : 0x3fc9a3c4 A13 : 0x3fca6720
A14 : 0x3fc9acb8 A15 : 0x00000002 SAR : 0x00000008 EXCCAUSE: 0x00000007
EXCVADDR: 0x00000000 LBEG : 0x400557e1 LEND : 0x400557e5 LCOUNT : 0x00000000
Core 0 was running in ISR context:
EPC1 : 0x40041a6b EPC2 : 0x4004f3e7 EPC3 : 0x40045c48 EPC4 : 0x4206a48a
0x4206a48a: adc_hal_get_reading_result at C:/Applications/Develop/SDK/esp-idf-5.0/frameworks/esp-idf-v5.0/components/hal/adc_hal.c:291

Backtrace: 0x4206a487:0x3fc9a3c0 0x40379989:0x3fc9a3f0 0x40379b9e:0x3fc9a410 0x403770c6:0x3fc9a440 0x4004f3e4:0x3fca66a0 |<-CORRUPTED

Steps to reproduce.

Use the ADC continuous mode for ADC1 of an ESP32-S3.

In my case I have the following code fragment for init:

  const adc_continuous_handle_cfg_t adc_config = {
      .max_store_buf_size = 30 * sizeof(adc_digi_output_data_t) * 2,
      .conv_frame_size = 30 * sizeof(adc_digi_output_data_t),
   };

   adc_digi_pattern_config_t pattern[] = {
      {.atten = ADC_ATTEN_DB_0, .channel = ADC1_CH_LDRR, .unit = ADC_UNIT_1, .bit_width = 12},
      {.atten = ADC_ATTEN_DB_0, .channel = ADC1_CH_LDR1, .unit = ADC_UNIT_1, .bit_width = 12},
      {.atten = ADC_ATTEN_DB_0, .channel = ADC1_CH_LDR2, .unit = ADC_UNIT_1, .bit_width = 12},
   };

   ESP_ERROR_CHECK(adc_continuous_new_handle(&adc_config, &adc_handle));

   const adc_continuous_config_t cc = {
      .pattern_num = std::size(pattern),
      .adc_pattern = pattern,
      .sample_freq_hz = 3 * 800,
      .conv_mode = ADC_CONV_SINGLE_UNIT_1,
      .format = ADC_DIGI_OUTPUT_FORMAT_TYPE2
   };

   ESP_ERROR_CHECK(adc_continuous_config(adc_handle, &cc));
   ESP_ERROR_CHECK(adc_continuous_start(adc_handle));

Debug Logs.

No response

More Information.

With CONFIG_GDMA_ISR_IRAM_SAFE=yes, the adc_dma_in_suc_eof_callback function in adc_continuous.c is called while the cache is disabled.

adc_dma_in_suc_eof_callback calls s_adc_dma_intr which calls adc_hal_get_reading_result in adc_hal.c.

adc_dma_in_suc_eof_callback and s_adc_dma_intr are marked with IRAM_ATTR, but adc_hal_get_reading_result is not.
Same for adc_hal_digi_start and everything called in it.

@pggh pggh added the Type: Bug bugs in IDF label Jun 30, 2023
@espressif-bot espressif-bot added the Status: Opened Issue is new label Jun 30, 2023
@github-actions github-actions bot changed the title ADC continuous does crash when CONFIG_GDMA_ISR_IRAM_SAFE=yes ADC continuous does crash when CONFIG_GDMA_ISR_IRAM_SAFE=yes (IDFGH-10537) Jun 30, 2023
@Icarus113
Copy link
Collaborator

This isn't as expected. The Hal functions you referred are placed into internal ram by linker.lf and we have test cases for this.

Maybe there're something that tests didn't cover. Which optimisation level are you using? And could you help upload your sdkconfig here? @pggh

@pggh
Copy link
Author

pggh commented Jul 3, 2023

The linker script was not on my radar. When looking at it, I think the problem is caused by CONFIG_ADC_CONTINUOUS_ISR_IRAM_SAFE not being set (see below)
In the code CONFIG_ADC_CONTINUOUS_ISR_IRAM_SAFE is only used in adc_continuous_register_event_callbacks which I don't use. The problem is, that whether the ISR is executed when cache disabled is determined by CONFIG_GDMA_ISR_IRAM_SAFE but whether the HAL-functions are placed in RAM is controlled by CONFIG_ADC_CONTINUOUS_ISR_IRAM_SAFE. This does not make sense. There should be only one setting that controls the behavior or at least a compile time consistency check.
Note: When not in IRAM-Safe-Mode, adc_dma_in_suc_eof_callback and s_adc_dma_intr should not be placed in RAM either.

sdkconfig.defaults:

CONFIG_BOOTLOADER_CUSTOM_RESERVE_RTC=y
CONFIG_BOOTLOADER_CUSTOM_RESERVE_RTC_SIZE=0x10
CONFIG_ESPTOOLPY_FLASHMODE_QIO=y
CONFIG_ESPTOOLPY_FLASHSIZE_4MB=y
CONFIG_PARTITION_TABLE_CUSTOM=y
CONFIG_ESPTOOLPY_BEFORE_RESET_USB=y
# CONFIG_ETH_USE_SPI_ETHERNET is not set
CONFIG_GDMA_ISR_IRAM_SAFE=y
# CONFIG_ESP_PHY_CALIBRATION_AND_DATA_STORAGE is not set
# CONFIG_ESP_PHY_REDUCE_TX_POWER is not set
CONFIG_ESP_DEFAULT_CPU_FREQ_MHZ_80=y
CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG=y
CONFIG_ESP_BROWNOUT_DET_LVL_SEL_3=y
# CONFIG_ESP32_WIFI_NVS_ENABLED is not set
CONFIG_FREERTOS_UNICORE=y
CONFIG_FREERTOS_TIMER_TASK_PRIORITY=8
CONFIG_FREERTOS_USE_TRACE_FACILITY=y
CONFIG_FREERTOS_USE_STATS_FORMATTING_FUNCTIONS=y
CONFIG_FREERTOS_VTASKLIST_INCLUDE_COREID=y
CONFIG_FREERTOS_PLACE_SNAPSHOT_FUNS_INTO_FLASH=y
# CONFIG_LOG_COLORS is not set
CONFIG_LWIP_SO_RCVBUF=y
CONFIG_LWIP_NETBUF_RECVINFO=y
# CONFIG_LWIP_DHCP_DOES_ARP_CHECK is not set
# CONFIG_LWIP_IPV6 is not set
CONFIG_LWIP_TCP_TMR_INTERVAL=30
CONFIG_LWIP_TCP_SND_BUF_DEFAULT=5760
CONFIG_LWIP_TCP_WND_DEFAULT=5760
# CONFIG_SPI_FLASH_YIELD_DURING_ERASE is not set

@Icarus113
Copy link
Collaborator

Icarus113 commented Jul 4, 2023

You may refer to this line, where CONFIG_ADC_CONTINUOUS_ISR_IRAM_SAFE will select CONFIG_GDMA_ISR_IRAM_SAFE. If you only set CONFIG_GDMA_ISR_IRAM_SAFE, we won't know if you're going to use ADC with DMA or I2S with DMA or others, so we won't let the CONFIG_GDMA_ISR_IRAM_SAFE select other peripheral-specific IRAM-SAFE kconfig options.

Besides, here is a doc about the IRAM-SAFE part of the driver.

The ADC_CONTINUOUS_ISR_IRAM_SAFE and its prompt also say it's to make the driver ISR IRAM-SAFE, callback is part of it.

@pggh

@pggh
Copy link
Author

pggh commented Jul 4, 2023

I had set CONFIG_GDMA_ISR_IRAM_SAFE for something else, not the continuous-mode driver.
If a user needs CONFIG_GDMA_ISR_IRAM_SAFE but does not need the ADC continuous-mode to work when cache is disabled, why should the user have the idea to set CONFIG_ADC_CONTINUOUS_ISR_IRAM_SAFE?
It is not mentioned in the doc for CONFIG_ADC_CONTINUOUS_ISR_IRAM_SAFE or CONFIG_GDMA_ISR_IRAM_SAFE.

I suggest to have only one setting in the long term and implement a compile time consistency check for now because:

  • CONFIG_GDMA_ISR_IRAM_SAFE without CONFIG_ADC_CONTINUOUS_ISR_IRAM_SAFE -> crash
  • CONFIG_ADC_CONTINUOUS_ISR_IRAM_SAFE without CONFIG_GDMA_ISR_IRAM_SAFE -> no ISR execution when cache disabled

Also the doc linked above should include this information.

@Icarus113
Copy link
Collaborator

Ah ok, thanks for clarifying the condition. Now I understand your situation~

It's because if CONFIG_GDMA_ISR_IRAM_SAFE is set, then the GDMA ISR dispatcher will be placed into / marked as internal ram, and peripheral-specific ISR callbacks will get run when cache is disabled. Then the peripheral functions called in its ISR (which is dispatched by the gdma ISR callbacks) must be placed into internal ram otherwise lead to crash.

We'll discuss this then and give you a reply or solution.

@pggh

@espressif-bot espressif-bot added Status: Reviewing Issue is being reviewed and removed Status: Opened Issue is new labels Jul 5, 2023
@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: NA Issue resolution is unavailable and removed Status: Reviewing Issue is being reviewed labels Jul 17, 2023
@FrancoisB-HEX
Copy link

I had set CONFIG_GDMA_ISR_IRAM_SAFE for something else, not the continuous-mode driver. If a user needs CONFIG_GDMA_ISR_IRAM_SAFE but does not need the ADC continuous-mode to work when cache is disabled, why should the user have the idea to set CONFIG_ADC_CONTINUOUS_ISR_IRAM_SAFE? It is not mentioned in the doc for CONFIG_ADC_CONTINUOUS_ISR_IRAM_SAFE or CONFIG_GDMA_ISR_IRAM_SAFE.

I suggest to have only one setting in the long term and implement a compile time consistency check for now because:

  • CONFIG_GDMA_ISR_IRAM_SAFE without CONFIG_ADC_CONTINUOUS_ISR_IRAM_SAFE -> crash
  • CONFIG_ADC_CONTINUOUS_ISR_IRAM_SAFE without CONFIG_GDMA_ISR_IRAM_SAFE -> no ISR execution when cache disabled

Also the doc linked above should include this information.

I completely agree. Spent too much time trying to figure out why I was seeing intermittent Cache disabled but cached memory region accessed errors when writing to NVS with continuous ADC running, even though CONFIG_ADC_CONTINUOUS_ISR_IRAM_SAFE IS NOT SET, but CONFIG_GDMA_ISR_IRAM_SAFE is.
I referenced the documentation on those configuration flags multiple times and it makes no mention of the fact that "peripheral-specific ISR callbacks will get run when cache is disabled".

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

No branches or pull requests

4 participants