boards/hip-badge: add HiP Badge board definition #19076

benpicco · 2022-12-28T17:13:49Z

Contribution description

This adds support for the HiP Badge.

What works:

Testing procedure

Install the esp32c3 toolchain:

dist/tools/esptools/install.sh esp32c3
. dist/tools/esptools/export.sh esp32c3

Flash any test / application

make BOARD=hip-badge flash

If stdio does not work, just reset the board by pressing the blue reset button (SW3).
For a yet unknown reason I don't get any TX empty interrupts right after flashing / soft reboot, but it works after a reset.

Issues/PRs references

requires #19096 for stdio to work with any application that uses the radio

gschorcht · 2023-01-05T07:16:23Z

Could you imagine to split off the stdio_usb_serial_jtag as separate PR? This would be really useful to flash and debug boards for ESP32-C3 and ESP32-S3 boards that don't have a USB-to-UART bridge on board because these boards can be reset in bootloader using the USB Serial/JTAG interface, e.g. the ESP32 ProS3 in PR #19088.

gschorcht · 2023-01-06T06:09:38Z

cpu/esp32/include/irq_arch.h

@@ -50,6 +50,7 @@ extern "C" {
 #define CPU_INUM_SYSTIMER   20  /**< Level interrupt with medium priority 2 */
 #define CPU_INUM_BLE        21  /**< Level interrupt with medium priority 2 */
 #define CPU_INUM_CACHEERR   25  /**< Level interrupt with high priority 4 */
+#define CPU_INUM_SERIAL_JTAG 26 /**< Level interrupt with low priority 1 */


Please align the indent of the interrupt values.

gschorcht · 2023-01-06T06:13:25Z

cpu/esp32/include/irq_arch.h

@@ -50,6 +50,7 @@ extern "C" {
 #define CPU_INUM_SYSTIMER   20  /**< Level interrupt with medium priority 2 */
 #define CPU_INUM_BLE        21  /**< Level interrupt with medium priority 2 */
 #define CPU_INUM_CACHEERR   25  /**< Level interrupt with high priority 4 */
+#define CPU_INUM_SERIAL_JTAG 26 /**< Level interrupt with low priority 1 */


The level specifies the priority of the the interrupts which is flexible for RISC-V based ESP32x SoCs but fixed for Xtensa-based SoCs. The value given here is the fixed value as used by the Xtensa-based SoCs which is 5.

Suggested change

#define CPU_INUM_SERIAL_JTAG 26 /**< Level interrupt with low priority 1 */

#define CPU_INUM_SERIAL_JTAG 26 /**< Level interrupt with high priority 5 */

We should check whether we could use another interrupt because level 5 is the highest one and higher than the cache error or WDT interrupts which have a level of 4.

If I'm not wrong, level 1 interrupt 10 should be free.

gschorcht · 2023-01-06T06:13:56Z

cpu/esp32/irq_arch.c

@@ -83,6 +83,9 @@ static const struct intr_handle_data_t _irq_data_table[] = {
 #if defined(CPU_FAM_ESP32S2) || defined(CPU_FAM_ESP32S3)
    { ETS_USB_INTR_SOURCE, CPU_INUM_USB, 1 },
 #endif
+#if defined(CPU_INUM_SERIAL_JTAG)
+    { ETS_USB_SERIAL_JTAG_INTR_SOURCE, CPU_INUM_SERIAL_JTAG, 1 },


Suggested change

{ ETS_USB_SERIAL_JTAG_INTR_SOURCE, CPU_INUM_SERIAL_JTAG, 1 },

{ ETS_USB_SERIAL_JTAG_INTR_SOURCE, CPU_INUM_SERIAL_JTAG, 5 },

See my comment above.

Hm with that, stdio is no longer working.
The highest I can set it to is 3

This is wrong again. Either the level is defined to be 5 if CPU_INUM_SERIAL_JTAG=26 is used or CPU_INUM_SERIAL_JTAG is set to 10 and then the level has to be be 1. The mapping of interrupt numbers and priorities is hardwired in Xtensa cores and can't be set by software.

My concern is that level 5 is much too high because it is higher than cache error exception and WDTs. Therefore, I suggested to use interrupt 10 with level 1.

The whole mapping, I found in past in any former ESP-IDF version in `espressif/soc/soc.hext ended by a column with RIOT usage:

/************************************************************************** * Int Level Type PRO CPU usage APP CPU uasge RIOT Comment * 0 1 extern level WMAC Reserved - wDev_ProcessFiq * 1 1 extern level BT/BLE Host HCI DMA BT/BLE Host HCI DMA * 2 1 extern level GPIO * 3 1 extern level CAN * 4 1 extern level WBB UART * 5 1 extern level BT/BLE Controller BT/BLE Controller - * 6 1 timer FreeRTOS Tick(L1) FreeRTOS Tick(L1) - CCOMPARE0 * 7 1 software BT/BLE VHCI BT/BLE VHCI - * 8 1 extern level BT/BLE BB(RX/TX) BT/BLE BB(RX/TX) USB * 9 1 extern level RTC * 10 1 extern edge * 11 3 profiling * 12 1 extern level I2C * 13 1 extern level WDT * 14 7 nmi Reserved Reserved * 15 3 timer FreeRTOS Tick(L3) FreeRTOS Tick(L3) - CCOMPARE1 * 16 5 timer - CCOMPARE2 * 17 1 extern level thread_yield_higher * 18 1 extern level ETH * 19 2 extern level TIMER * 20 2 extern level FRC2 ESP-IDF FRC2 legacy * 21 2 extern level BLE * 22 3 extern edge * 23 3 extern level * 24 4 extern level TG1_WDT * 25 4 extern level CACHEERR * 26 5 extern level * 27 3 extern level Reserved Reserved * 28 4 extern edge DPORT ACCESS DPORT ACCESS Not in single core * 29 3 software Reserved Reserved * 30 4 extern edge Reserved Reserved * 31 5 extern level **************************************************************************

hm I think I want a level triggered IRQ for stdout though

Does it finally matter?

It doesn't make a difference on the ESP32-C3.

But there the level is configurable, so maybe software already does configure it to be a level interrupt?

I think it is time to implement the dynamic interrupt allocation as ESP-IDF intr_alloc function does where the application just specify requirements like edge/level, priority, ... and the intr_alloc function assigns the next free interrupt that satisfies the reuirements.

I'm not really sure whether all the BT/BLE interrupts are really used. It would be helpful to record a trace which interrupts by the ESP-IDF and the WiFi/BLE libraries are really allocated.

boards/hip-badge/include/periph_conf.h

gschorcht · 2023-01-06T06:22:26Z

I wonder if it would make sense, to call the board directory espc3-hip-badge to have it in the alphabetical order with all the other ESP32x boards in the boards directory. On the other hand, the directories for boards of other architectures don't start with MCU name. Your decision 😄

Please add a Kconfig for the board and add the CPU_MODEL_ESP32C3_FH4AZ definition to cpu/esp32/Kconfig.esp32c3. This model wasn't known when I initially wrote cpu/esp32/Kconfig.esp32c3. My plan is to add Flash and SPI RAM size selection in Kconfig and to use the model to set default values.

boards/hip-badge/include/periph_conf.h

boards/hip-badge/doc.txt

boards/hip-badge/include/board.h

makefiles/pseudomodules.inc.mk

gschorcht · 2023-01-06T06:40:48Z

makefiles/stdio.inc.mk

@@ -10,6 +10,7 @@ STDIO_MODULES = \
  stdio_uart \
  stdio_telnet \
  stdio_tinyusb_cdc_acm \
+  stdio_usb_serial_jtag \


The same here?

No, this is just a list of all possible stdio modules to select stdio_uart if none of them is selected.

... required for the decision whether to enable stdio_acd_acm.

makefiles/stdio.inc.mk

gschorcht · 2023-01-07T00:28:06Z

~~WiFi / ESP Now (not board specific)~~ board has no antenna!

At the back there is J1 which should be something like a connector for the antenna according to the schematic.

bors · 2023-01-07T02:15:23Z

Build failed:

Murdock

gschorcht · 2023-01-07T08:40:12Z

Build failed:

Murdock

Ah, I forgot, the board needs a Kconfig file.

Please add a Kconfig for the board in comment #19076 (comment)

😉

benpicco · 2023-01-08T18:17:06Z

Let's see if this does the job

bors try

bors · 2023-01-08T19:56:26Z

try

Build failed:

Murdock

maribu · 2023-01-08T19:58:57Z

This still needs squashing

benpicco · 2023-01-08T21:24:43Z

bors retry

maribu · 2023-01-08T23:19:19Z

Note that the last bors retry did restart a try run, not a merge run. And given that between the last successful try run and now only squashing was done, I think this wasn't the intended behavior.

benpicco · 2023-01-08T23:24:37Z

There is the added Kconfig, but if that is OK we can directly do a bors merge

bors · 2023-01-08T23:36:20Z

try

Build failed:

Murdock

benpicco · 2023-01-08T23:52:15Z

Ok, the naming of stdio_available() is super confusing.
It should be stdin_available()

bors retry

benpicco · 2023-01-09T10:39:02Z

bors merge

19076: boards/hip-badge: add HiP Badge board definition r=benpicco a=benpicco Co-authored-by: Benjamin Valentin <benpicco@beuth-hochschule.de>

bors · 2023-01-09T12:58:53Z

Build failed:

Murdock

maribu · 2023-01-09T13:33:23Z

bors merge

bors · 2023-01-09T16:20:12Z

Build succeeded:

benpicco · 2023-01-09T16:29:18Z

Thank you for the quick review!

github-actions bot added Area: boards Area: Board ports Area: drivers Area: Device drivers labels Dec 28, 2022

benpicco force-pushed the boards/hip-badge branch from af6f0d5 to f14e6ac Compare December 28, 2022 18:55

github-actions bot added Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Platform: ESP Platform: This PR/issue effects ESP-based platforms labels Dec 28, 2022

benpicco force-pushed the boards/hip-badge branch 2 times, most recently from 618f6da to eafca1a Compare January 5, 2023 22:05

github-actions bot added Area: doc Area: Documentation and removed Area: drivers Area: Device drivers labels Jan 5, 2023

benpicco changed the title ~~[WIP] boards/hip-badge: add HiP Badge board definition~~ boards/hip-badge: add HiP Badge board definition Jan 5, 2023

benpicco marked this pull request as ready for review January 5, 2023 22:06

benpicco requested a review from gschorcht as a code owner January 5, 2023 22:06

benpicco requested a review from miri64 January 5, 2023 22:07

gschorcht added the Type: new feature The issue requests / The PR implemements a new feature for RIOT label Jan 6, 2023