Question about the IRAM_ATTR usage with uart ISR

[AxelLin]

Hi @rahult-github

In porting/nimble/src/hal_uart.c, there are IRAM_ATTR_64MCPU / IRAM_ATTR
in ISR / uart_rx_task functions. Which means all the functions called by
ISR or uart_rx_task needs to have IRAM_ATTR as well. Otherwise it could
hit "Guru Meditation Error: Core 0 panic'ed (Illegal instruction). Exception was unhandled".

Is my understanding correct?
If yes, the code in nimble/transport/uart/src/ble_hci_uart.c needs fix because ble_hci_uart_tx_char() and ble_hci_uart_rx_char() are called from an IRAM_ATTR function.
And what's the best way to fix this issue?

I'm aware the default esp-idf does not use ble_hci_uart.c, just use it to clarify if my understanding is correct.

[AxelLin]

Hi @rahult-github

I hit Guru Meditation Error: Core 0 panic'ed (Illegal instruction). Exception was unhandled.
0x40382d86: npl_freertos_callout_is_active at /home/axel/esp/esp-idf/components/bt/host/nimble/nimble/porting/npl/freertos/src/npl_os_freertos.c:856

Just check the npl_freertos_callout_is_active implementation:

npl_freertos_callout_is_active (IRAM_ATTR)
  esp_timer_is_active() // missing IRAM_ATTR?
    timer_armed() (IRAM_ATTR)

It looks like esp_timer_is_active() also needs IRAM_ATTR.
My test is ongoing, but it looks like no longer hit the panic after adding IRAM_ATTR to esp_timer_is_active().
Any comments? Should I report this to esp-idf or you will take care of it?

[rahult-github]

Hi @AxelLin , thanks for pointing out. We will review and make the needed changes .

[AxelLin]

Similarly, esp_timer_delete() is called by npl_freertos_callout_deinit() which may need IRAM_ATTR.
esp_timer_start_once(), esp_timer_start_periodic() and esp_timer_stop have IRAM_ATTR,
then why the esp_timer_restart() does not need IRAM_ATTR? (Isn't this a shortcut for stop/start? )

Just FYI, this probably needs a review as well.

[rahult-github]

Hi @AxelLin

I hit Guru Meditation Error: Core 0 panic'ed (Illegal instruction). Exception was unhandled.

May i know how you are reproducing this issue ? Are you disabling flash cache or any other setting ?

Just FYI, this probably needs a review as well.

Indeed we are reviewing the code , but if there are ways to reproduce the failures , then this information would be helpful for us to test the changes thoroughly before releasing.

[AxelLin]

Hi @AxelLin

I hit Guru Meditation Error: Core 0 panic'ed (Illegal instruction). Exception was unhandled.

May i know how you are reproducing this issue ? Are you disabling flash cache or any other setting ?

Regarding disabling flash cache, do you mean this?

# CONFIG_SPI_FLASH_AUTO_SUSPEND is not set

(It's default setting N, but it's strange the doc says it's default Y: https://docs.espressif.com/projects/esp-idf/en/latest/esp32c3/api-reference/storage/spi_flash_concurrency.html#concurrency-constraints-for-flash-on-spi1)

I cannot provide the code for testing.
(My code is modified from ./components/bt/host/nimble/nimble/porting/nimble/src/hal_uart.c)
My use case is using uart-hci, esp32 as host with a external BT controller.
As I know this is not a common uses case for esp-idf users.

BTW, I hope to understand the IRAM_ATTR functions usage. Can you help to clarify?

1. In non-ISR code path, it's ok a IRAM_ATTR function to call non-IRAM_ATTR function.
2. In ISR code path:
   a. If the ISR function with IRAM_ATTR, all the functions called by the ISR function must has IRAM_ATTR.
   b. If the ISR function without IRAM_ATTR, it's ok to call functions without IRAM_ATTR in ISR function.

Is my understanding correct?

[rahult-github]

Hi @AxelLin ,

IRAM_ATTR implies the code is in IRAM . So, code in IRAM makes execution fast. Also, code in IRAM implies that it has the ability to execute the code when flash cache is disabled. e.g. while programming flash ( say nvs write ) flash cache is disabled, so we can not execute code from flash.

If we place interrupt in IRAM then its entire call chain should be in IRAM. This interrupt will work even if flash cache is disabled.

Basically , its good to have entire calling functions in IRAM_ATTR ( subjected to availabiity of IRAM space ) .

Thanks,
Rahul

[AxelLin]

May i know how you are reproducing this issue ? Are you disabling flash cache or any other setting ?

Hi @rahult-github
I no longer hit any panic if setting CONFIG_SPI_FLASH_AUTO_SUSPEND=y.
But I think it should work no matter SPI_FLASH_AUTO_SUSPEND is enabled or not.
Any hint?

[AxelLin]

Similarly, esp_timer_delete() is called by npl_freertos_callout_deinit() which may need IRAM_ATTR. esp_timer_start_once(), esp_timer_start_periodic() and esp_timer_stop have IRAM_ATTR, then why the esp_timer_restart() does not need IRAM_ATTR? (Isn't this a shortcut for stop/start? )

Just FYI, this probably needs a review as well.

I close this issue and move to esp-idf instead (since the change for esp_timer is not in nimble).