search

manuelbl / ttn-esp32

The Things Network device library for ESP32 (ESP-IDF) and SX127x based devices
MIT License
308 stars 63 forks source link

TTN ISR executes hal_ticks() from the flash #33

Closed vladcebo closed 3 years ago

vladcebo commented 3 years ago

Need to add IRAM_ATTR to hal_ticks() to prevent it executing from flash region. That function is called by dioIrqHandler handler. Otherwise the application will crash in case of OTA updates or other things that disables the flash.

cyberman54 commented 3 years ago

@vladcebo Regarding to https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/general-notes.html#dma-capable-requirement, don't we also need DRAM_ATTR for buffers of SPI communication in hal-esp32 ?

vladcebo commented 3 years ago

@cyberman54, not sure about this one. The spi transmit is eventually called in lmic/radio.c file and the buffer passed is in the LMIC struct (LMIC.frame) which is declared as non-const static, so it's already in DRAM:

#define DEFINE_LMIC  struct lmic_t LMIC

I think the DRAM_ATTR is required when we want to send something like a const char* from the flash. For DMA it also requires it to be word aligned, and I think that's already the case (at least I never saw any errors on that so far).

manuelbl commented 3 years ago

Thanks a lot for the PR and the other input. I will check the code and analyze if the relevant data structures end up in DRAM as expected.

manuelbl commented 3 years ago

I've move a buffer used for SPI transmission from the stack to static allocation to be on the safe side even if PSRAM is used. But generally all relevant data structures should be in DRAM and have fixed allocation.