[TW#26338] Should gpio routines be in IRAM (for read from ISR)?

[RA5040]

I am reading digital inputs in an interrupt service routine and I read somewhere that a routine that is in IRAM (as the ISR needs to be) should not call a routine that is not in IRAM. I checked to make sure that gpio_get_level is and was surprised to see that it is not. However Arduino digitalRead is.

Is this an error, or is it in fact OK to call a non-IRAM routine from an ISR?

[negativekelvin]

https://docs.espressif.com/projects/esp-idf/en/latest/api-reference/system/intr_alloc.html#iram-safe-interrupt-handlers

[RA5040]

That's useful. However my question relates to calling a routine that is not in IRAM from a routine (the ISR) that is in IRAM.

As I mentioned, the gpio-get-level routine, for instance, does not have the IRAM attribute set, so presumably it will be loaded in flash memory by default. So if I am calling it from my ISR I will (may) be calling a routine in flash memory from a routine that is in IRAM.

On the other hand, the arduino digitalRead routine does have the IRAM attribute set.

[negativekelvin]

The thing that matters is whether the interrupt is marked with ESP_INTR_FLAG_IRAM. If it is not then you can call both iram and flash functions.

If you look at this the service is not marked with ESP_INTR_FLAG_IRAM and the ISRs are not in iram
https:/espressif/esp-idf/blob/6f5820814fbd5e3f21dfbe6899259455f4f27621/components/driver/test/test_gpio.c

But if you do use ESP_INTR_FLAG_IRAM then you can't call flash functions like gpio_get_level. Should it be in IRAM? In general as few functions as possible should be in IRAM by default because it is limited. It is a very small function so you could make both arguments that A) it wouldn't take much space in IRAM or B) that it is easy for the user to reproduce in IRAM if they need it. Another option would be to add a separate IRAM function like gpio_get_level_fromISR.

[RA5040]

Well I definitely want the ISR to be in IRAM for speed. And I can either recode the gpio routines I use or use the arduino routines (which is identical except for the IRAM attr). And I take your point about having to make a value call as to which routines are put into IRAM and which are not. However the fact is that the esp and Arduino implementations of the same routine are inconsistent ... at least they should be the same. Secondly, gpio routines will often be used in ISRs and since they are very small it might make sense to put them in IRAM.

My question also relates to why routines in flash should not be called from an ISR. Is it only for speed reasons, or is there a more fundamental reason?

[stickbreaker]

If iCache is disabled because of a SPIFFs/NVS erase/write cycle, or the code is not already in iCache, the interrupt execution will fail, causing a reboot:

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

During an interrupt, code will NOT be loaded into iCache. So any call from an interrupt context that causes a cache miss will reboot the esp32. No Recovery!

There have been reported instances where operating code suddenly fails, the failure was traced back too changes in placement of functions in source files (code ordering). The root cause was iCache misses. The differing code order changed the location the interrupt code was loaded into iCache. It just happened that the interrupt code shared iCache location with code that was executed often. When the interrupt triggered during this code, the interrupt code was not in iCache, so the ESP32 core dumped at the same point in the app code all the time. This app code had nothing to do with the interrupt code, except that is shared the same iCache page.

Chuck.

[negativekelvin]

However the fact is that the esp and Arduino implementations of the same routine are inconsistent ... at least they should be the same.

Arduino has to worry about compatibility with existing code and existing api standards so that will cause different decisions to be made and there is no reason to try to force consistency between idf and Arduino apis because most of the time they are mutually exclusive.

[me-no-dev]

@negativekelvin it kinda makes sense gpio set functions to be in IRAM ;) why not allow GPIO changes in IRAM ISRs? It's not like they take large space.

[negativekelvin]

I am overly greedy for every word of iram but it's fine either way

[me-no-dev]

it's really a couple of instructions :) no semaphores or anything of the sort.
https:/espressif/esp-idf/blob/master/components/driver/gpio.c#L156

set and get level should be enough :) the rest is not generally used in IRAM ISRs

[igrr]

Aside from code size, these functions depend on the constant GPIO_PIN_MUX_REG array, which is placed into DROM (flash cache) and has significant size.

The way this will be resolved is likely through the use of linker script customization facilities which will be available in v3.2. This will allow users to customize placement of functions and data into IRAM, in cases like this one.

[RA5040]

Aside from code size, these functions depend on the constant GPIO_PIN_MUX_REG array, which is placed into DROM (flash cache) and has significant size.

The way this will be resolved is likely through the use of linker script customization facilities which will be available in v3.2. This will allow users to customize placement of functions and data into IRAM, in cases like this one.

OK, that's good. But what about right now? Fundamentally the question I have is: "Is it safe to call code that is not in IRAM from an ISR which is in IRAM (and which should be for speed reasons, if nothing else)?"

If it is not safe, it would seem to me that gpio routines should not be called from the ISR ... as some of the code/data (GPIO_PIN_MUX_REG array for instance) will be in flash or flash cache.

Or is it OK if some of the code/data is in flash cache? How do I determine if the code/data is accessed directly in flash or is cached?

I need to know what the correct way to handle this is. If putting the ISR in IRAM is unsafe, then it will have to go into flash ... is that safe?

[igrr]

For now, you first need to decide whether you need your ISR to be enabled while flash operations (read/write/erase) are in progress. This depends on your application — whether it uses Flash for storage (NVS/SPIFFS/FATFS/...) and whether interrupt timing is important (i.e. is delaying an interrupt by 30ms due to flash erase operation okay?).

If you decide that ISR does not need to run while flash operation is in progress, then don't mark your ISR with IRAM_ATTR and use interrupt allocation flags = 0. Call any functions you like from ISR, aside from the ones which can block. GPIO driver functions, in particular, are okay.

In the other case, if your ISR needs to run while flash operation is in progress, mark it with IRAM_ATTR, and don't call functions which are not in IRAM or in ROM. If you need to do GPIO operations from such an ISR, implement that by accessing GPIO registers directly. Use driver code (gpio.c) as a reference, as well as the Technical Reference Manual. Alternative is to call ROM GPIO functions, declared in rom/gpio.h.

[RA5040]

Many thanks ... that is really helpful!