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

ESP32 revisions to support esp32-c3 and s2 variants #2365

Merged
merged 26 commits into from
Sep 16, 2021

Conversation

mikee47
Copy link
Contributor

@mikee47 mikee47 commented Sep 15, 2021

This PR improves support for ESP32 soc variants with testing on real hardware:

  • ESP32 devkit V1 (ESP-WROOM32)
  • AI-thinker esp-c3-32s
  • AI-thinker esp-12k (esp32-s2) devkits

SDK build changes

SDK builds automatically if files or config changed. Ninja is very fast so low overhead.

Create separate SDK project for debug/release builds.
Use separate build directory for Esp32 variants, i.e. {SMING_ARCH}/{ESP_VARIANT}.
Esp8266 & Host unchanged.

Build only selected SDK components: reduces number of files to compile by about 1/3rd.

Remove unused components, including VFS an SDK_FULL_BUILD option.

Move SDK build targets into appropriate help categories

Remove redundant sdk-defconfig target

Fixes

Fix interrupts() function to work with RISCV.
system_os_post() must be in IRAM.
Fix building for esp32c3 with DISABLE_WIFI=1.

Flashing

Fix flashing for esp32c3 & esp32s2 variants, variant must be passed to esptool.

Fix issues with mismatched types, e.g. RISCV compiler for esp32c3 uses different type definitions.

Route _write_r to m_nputs and set IDF logging function to m_vprintf

Rework uart driver using IDF HAL

Works reliably with esp32-s2 and c3
UART default pin allocations selected by variant
Peripheral registers are mapped in two places on s2 (see tech. manual), need to use AHB_FIFO mappings.
Use HAL to accommodate these differences.
Ensure internal memory used for serial buffer
Remove uartHardware structure, located in flash: use GetDriver which is OK for use in ISR.
Register ISR to be called in interrupt context so it remains active whilst flash is disabled.

Enable GDB panic handler

Can now use regular make gdb if panic occurs.

Esp32 Framework changes

Remove redundant network code

Fix flashmem_get_address() for all Esp32 variants

Fix PROGMEM definition

Only applies to DROM, not IROM so simplify `isFlashPtr` also

Implement CPU frequency setting

A bit more involved on the ESP32, requires power management to be enabled via SDK.

HostTests changes

Simplify initorder test

Compile HttpRequest, TcpClient test modules only for Host

Fix heap checking for shared pointers

Move XorOutputStream from Network Component into gen. pop. - not particularly network-centric.
Allows test code to compile with

Clock tests sometimes generate interrupt WDT timeout on ESP32: don't leave interrupts disabled whilst printing to UART.

Ensures that switching between build types uses correct settings.
Remove unused components
Disable VFS (not used by Sming)
e.g. RISCV compiler for esp32c3 uses different type definitions
State `0` doesn't work for esp32s2, so use more appropriate function
Works reliably with esp32-s2 and c3
UART default pin allocations selected by variant
Peripheral registers are mapped in two places on s2 (see tech. manual), need to use AHB_FIFO mappings.
Use HAL to accommodate these differences.
Ensure internal memory used for serial buffer
Remove `uartHardware` structure, located in flash: use `GetDriver` which is OK for use in ISR.
Register ISR to be called in interrupt context so it remains active whilst flash is disabled.
Don't leave interrupts disabled whilst printing to UART
Only applies to DROM, not IROM so simplify `isFlashPtr` also
A bit more involved on the ESP32, requires power management to be enabled via SDK.
Further features can be reintroduced carefully as required.
@mikee47 mikee47 changed the title Feature/esp32c3s2 ESP32 revisions to support esp32-c3 and s2 variants Sep 15, 2021
@slaff slaff added this to the 4.4.0 milestone Sep 15, 2021
@@ -314,11 +329,10 @@ static void IRAM_ATTR uart_isr(smg_uart_instance_t* inst)
}

auto uart = inst->uart;
auto uart_nr = uart->uart_nr;
auto& hw = uartHardware[uart_nr];
auto dev = getDevice(uart->uart_nr);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am getting segfaults pointing to this line. Steps to reproduce:

  1. Use ESP-WROOM-32 chip

  2. Compile the Basic_Wifi sample with the following command:

    make SMING_ARCH=Esp32 DEBUG_VERBOSE_LEVEL=3

  3. Flash the applications.

I am getting always the following error:

(terminal output)

Sming. Let's do smart things!
Re-enable cpu cache.
Guru Meditation Error: Core  1 panic'ed (LoadProhibited). Exception was unhandled.

Core  1 register dump:
PC      : 0x40086c1b  PS      : 0x00060031  A0      : 0x40081dc4  A1      : 0x3ffb0ba0  
A2      : 0x3ffb53f4  A3      : 0x3ffcef94  A4      : 0x00000002  A5      : 0x00000000  
A6      : 0x0000000a  A7      : 0x3ffb0ba0  A8      : 0x80086c18  A9      : 0x3ffb0b80  
A10     : 0xbad00bad  A11     : 0x00000000  A12     : 0x00000000  A13     : 0x3ffb4b20  
A14     : 0xffffffff  A15     : 0x00000004  SAR     : 0x00000000  EXCCAUSE: 0x0000001c  
EXCVADDR: 0xbad00bb5  LBEG    : 0x00000000  LEND    : 0x00000000  LCOUNT  : 0x00000000  

Backtrace:0x40086c18:0x3ffb0ba0 0x40081dc1:0x3ffb0c50 0x4008265c:0x3ffbad80 0x400842fc:0x3ffbada0


ELF file SHA256: b4a6e9970b27af4c

Entering gdb stub now.

(gdb output)

Remote debugging using /dev/ttyUSB0
0x40086c1b in uart_isr (inst=0x3ffb53f4 <uartInstances>)
    at /opt/Sming/Sming/Arch/Esp32/Components/driver/uart.cpp:332
332		auto dev = getDevice(uart->uart_nr);

And decoded stacktrace:

0x40086c18:0x3ffb0ba0 0x40081dc1:0x3ffb0c50 0x4008265c:0x3ffbad80 0x400842fc:0x3ffbada0
0x40086c18: uart_isr(smg_uart_instance_t*) at /opt/Sming/Sming/Arch/Esp32/Components/driver/uart.cpp:335
0x40081dc1: _xt_lowint1 at /opt/esp-idf-4.3/components/freertos/port/xtensa/xtensa_vectors.S:1105
0x4008265c: spi_flash_op_block_func at /opt/esp-idf-4.3/components/spi_flash/cache_utils.c:115 (discriminator 1)
0x400842fc: ipc_task at /opt/esp-idf-4.3/components/esp_ipc/ipc.c:62

Can you try to reproduce the problem? At least for me the same sample works just fine with the current develop.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get the same problem... It can be fixed by reverting line 462 using ESP_INTR_FLAG_LOWMED instead of ESP_INTR_FLAG_IRAM. But that's a hack.

Turns out the real reason is the compiler puts switch tables in flash, even for IRAM_ATTR code blocks. Using if statements and the problem goes away. Lesson learned.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out the real reason is the compiler puts switch tables in flash, even for IRAM_ATTR code blocks. Using if statements and the problem goes away. Lesson learned.

Wow! Thanks for the fix and the explanation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For info., just re-tested the esp32-c3 & esp32-s2 and neither have a problem, however both still put the switch table in flash.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retested with ESP-WROOM-32 and works as expected. Thank you @mikee47 .

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out the real reason is the compiler puts switch tables in flash

I seem to remember that at some point with the old ESP8266 compilers vtables could placed in a desired location (GCC compiler recompilation required.). And this brought to my attention the fact that also jump tables could be taken care of. GCC has an option for the placement of jump tables. Take a look at
https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html and search for option -fno-jump-tables. Unfortunately, as far as I know, it is not possible to instruct the compiler to not use jump tables for switch statements in IRAM code blocks only.

Looks like switch statements are to be avoided inside IRAM_ATTR code.
@slaff slaff removed the 3 - Review label Sep 16, 2021
@slaff slaff merged commit 3f63abb into SmingHub:develop Sep 16, 2021
@slaff slaff mentioned this pull request Sep 17, 2021
5 tasks
slaff pushed a commit that referenced this pull request Sep 27, 2021
This PR improves support for ESP32 soc variants with testing on real hardware:

- ESP32 devkit V1 (ESP-WROOM32)
- AI-thinker `esp-c3-32s`
- AI-thinker `esp-12k` (esp32-s2) devkits


**SDK build changes**

SDK builds automatically if files or config changed. Ninja is very fast so low overhead.

Create separate SDK project for debug/release builds.
Use separate build directory for Esp32 variants, i.e. `{SMING_ARCH}/{ESP_VARIANT}`.
Esp8266 & Host unchanged.

Build only selected SDK components: reduces number of files to compile by about 1/3rd.

Remove unused components, including VFS an SDK_FULL_BUILD option.

Move SDK build targets into appropriate help categories

Remove redundant `sdk-defconfig` target


**Fixes**

Fix `interrupts()` function to work with RISCV.
`system_os_post()` must be in IRAM.
Fix building for esp32c3 with DISABLE_WIFI=1.


**Flashing**

Fix flashing for esp32c3 & esp32s2 variants, variant must be passed to esptool.

Fix issues with mismatched types, e.g. RISCV compiler for esp32c3 uses different type definitions.

Route `_write_r` to `m_nputs` and set IDF logging function to `m_vprintf`


**Rework uart driver using IDF HAL**

Works reliably with esp32-s2 and c3
UART default pin allocations selected by variant
Peripheral registers are mapped in two places on s2 (see tech. manual), need to use AHB_FIFO mappings.
Use HAL to accommodate these differences.
Ensure internal memory used for serial buffer
Remove `uartHardware` structure, located in flash: use `GetDriver` which is OK for use in ISR.
Register ISR to be called in interrupt context so it remains active whilst flash is disabled.

**Enable GDB panic handler**

Can now use regular `make gdb` if panic occurs.


**Esp32 Framework changes**

Remove redundant network code

Fix `flashmem_get_address()` for all Esp32 variants

Fix PROGMEM definition

    Only applies to DROM, not IROM so simplify `isFlashPtr` also

Implement CPU frequency setting

    A bit more involved on the ESP32, requires power management to be enabled via SDK.


**HostTests changes**

Simplify `initorder` test

Compile HttpRequest, TcpClient test modules only for Host

Fix heap checking for shared pointers

Move XorOutputStream from Network Component into gen. pop. - not particularly network-centric.
Allows test code to compile with 

Clock tests sometimes generate interrupt WDT timeout on ESP32: don't leave interrupts disabled whilst printing to UART.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants