ESP32: Add pinmux, GPIO, watchdog, RNG drivers; begin cleaning up ISR tables #724

lpereira · 2017-07-07T20:40:12Z

This patch series add drivers for pin mux, GPIO, watchdog timer, hardware random number generator, and does some minor cleanups in how the interrupt table is generated. Individual commit messages will tell the whole story, as usual.

In addition to more devices being supported by the port, IRQ offloading is also working as a result of the ISR table cleanup.

andrewboie · 2017-07-07T21:03:15Z

include/arch/xtensa/arch.h

-		)) = {isr_param_p, isr_p}; \
-	_irq_priority_set(irq_p, priority_p, flags_p); \
+	extern struct _isr_table_entry _sw_isr_table[]; \
+	_sw_isr_table[IRQ].isr = isr_p; \


why not use _ISR_DECLARE? gen_isr_tables.py is gonna look for interrupts in the intlist section.
See include/sw_isr_table.h

I tried using this macro and gen_isr_tables.py, but got an Invaild bfd target error from objcopy. zephyr_prebuilt.elf is indeed an Xtensa ELF and seems valid, but it beats me why the xtensa-elf32-elf-objcopy isn't recognizing it.

I want to clean up a lot of the exception/interrupt handling in the Xtensa port, but there are higher priority items I need to clean up beforehand.

(Also, it might be the case we'll need a gen_isr_tables.py specific for Xtensa.)

Figured out why objcopy was failing. The OUTPUT_FORMAT variable in the gen_isr_tables Makefile was different than what objcopy for the target expected.

andrewboie · 2017-07-07T21:03:59Z

arch/xtensa/core/xtensa_intr.c

 {
 	ReservedInterruptHandler((unsigned int)arg);
 	CODE_UNREACHABLE;
 }
-#endif /* XCHAL_HAVE_INTERRUPTS */
+
+struct _isr_table_entry __sw_isr_table _sw_isr_table[IRQ_TABLE_SIZE] = {


shouldn't need this.
if gen_isr_tables.py is in use, it should build arch/common/isr_tables.c which creates the dummy table.

I know. This declaration is a copy from that file. But I cannot use that script yet due to issues with the SDK.

andrewboie · 2017-07-07T21:04:35Z

arch/xtensa/core/xtensa_intr.c

@@ -14,14 +14,14 @@
 #include "xtensa_rtos.h"
 #include "xtensa_api.h"
 #include <kernel_structs.h>
+#include <sw_isr_table.h>

 #if XCHAL_HAVE_EXCEPTIONS

 /* Handler table is in xtensa_intr_asm.S */

 extern xt_exc_handler _xt_exception_table[XCHAL_EXCCAUSE_NUM];


we should get rid of all occurrences of this and replace with the actual table, its just an alias and is legacy Cadence cruft

lpereira · 2017-07-19T21:10:09Z

Rebased and addressed @andrewboie's review comments.

andrewboie

The drivers look good to me!

I have some issues with the IRQ changes -- it seems that only the linker scripts and defconfigs for esp32 have been updated to use the new interrupt handling stuff.

I'm concerned that for all the other xtensa targets, they may be broken by this change, since the old sw irq table code was deleted from the core/ directory. I have a vague memory that the timer interrupt is configured in a strange way, outside of IRQ_CONNECT(), so that might still work. But I suspect all our other xtensa SOC / boards (qemu_xtensa, and all the xt-sim targets that only build with the Cadence toolchain) are going to need to be updated as well.

The interrupt changes for esp32 look properly done.

andrewboie · 2017-07-19T22:24:54Z

arch/xtensa/soc/esp32/linker.ld

@@ -36,6 +37,9 @@ MEMORY
  drom0_0_seg(R): org = 0x3F400010, len = 0x800000
  rtc_iram_seg(RWX): org = 0x400C0000, len = 0x2000
  rtc_slow_seg(RW): org = 0x50000000, len = 0x1000
+#ifdef CONFIG_GEN_ISR_TABLES


unfortunately, we are gonna need to make corresponding changes in all the linker scripts for the other Xtensa SOC definitions, otherwise I do not think we can remove the old IRQ handling code unless we update the configuration for everything

andrewboie · 2017-07-19T22:26:45Z

drivers/pinmux/pinmux_esp32.c

+{
+	ARG_UNUSED(dev);
+
+	/* FIXME: Drive strength (FUN_DRV) is also set here to its maximum


do we have a bug open on this?

Yes: ZEP-1024

lpereira · 2017-07-19T22:40:16Z

drivers/gpio/gpio_esp32.c

+	struct gpio_esp32_data *data = dev->driver_data;
+
+	if (access_op == GPIO_ACCESS_BY_PIN) {
+		data->cb_pins = ~BIT(pin);


Forgot an & here. Submitting a new version soon.

lpereira · 2017-07-19T23:54:39Z

Updated, addressing @andrewboie's comments.

@rgundi, you might want to check this one as well, since it touches some generic Xtensa IRQ handling parts.

According to the "ESP32 Technical Reference Manual", the ESP32 SoC series supports up to 6 functions per GPIO pin. Add PINMUX_FUNC_E and PINMUX_FUNC_F. Jira: ZEP-2297 Signed-off-by: Leandro Pereira <leandro.pereira@intel.com>

This implements a driver for the pin multiplexer as present in the ESP32 SoCs. All APIs are supported. Jira: ZEP-2297 Signed-off-by: Leandro Pereira <leandro.pereira@intel.com>

This provides basic GPIO support, with interrupts, and the ability to read and write to ports on a pin-by-pin basis. Jira: ZEP-2286 Signed-off-by: Leandro Pereira <leandro.pereira@intel.com>

Zephyr's watchdog API is badly designed in the sense that it's a 1:1 abstraction on top of whatever Quark D2000 expects for its watchdog, instead of expecting a generic timeout value. This implementation tries as much as possible to calculate the watchdog timeout in a way that's compatible with a Quark D2000 running at 32MHz; a comment in adjust_timeout() explains this in more detail. Jira: ZEP-2296 Signed-off-by: Leandro Pereira <leandro.pereira@intel.com>

The random number generator from ESP32 uses noise from Wi-Fi and Bluetooth radios. If these are off, a pseudo-random number is generated instead; this is currently the case, but even though it's a black box, it's arguably better than returning a timestamp as a pseudo-random number generator. According to the ESP32 Technical Reference manual, the RNG passed the Dieharder Random Number Test suite (version 3.31.1)[1], but nothing has been said about the quality of the PRNG. The RNG register is read directly; no effort is made to use its contents to feed an entropy pool in a way that's similar to /dev/random on POSIX systems, as no such subsystem exists on Zephyr at the moment. [1] http://webhome.phy.duke.edu/~rgb/General/dieharder.php Signed-off-by: Leandro Pereira <leandro.pereira@intel.com>

Signed-off-by: Leandro Pereira <leandro.pereira@intel.com>

Dynamic IRQ allocation has been yanked from Zephyr a few releases ago, so there's no point in keeping these options available. Signed-off-by: Leandro Pereira <leandro.pereira@intel.com>

The Xtensa port was the only one remaining to be converted to the new way of connecting interrupts in Zephyr. Some things are still unconverted, mainly the exception table, and this will be performed another time. Of note: _irq_priority_set() isn't called on _ARCH_IRQ_CONNECT(), since IRQs can't change priority on Xtensa: while the architecture has the concept of interrupt priority levels, each line has a fixed level and can't be changed. Signed-off-by: Leandro Pereira <leandro.pereira@intel.com>

This cleans up the exception handling by removing the table declaration from xtensa_intr_asm.S, and removing the unused _xt_set_exception_handler() function. Signed-off-by: Leandro Pereira <leandro.pereira@intel.com>

lpereira · 2017-07-31T17:46:31Z

Rebased and repushed.

I still don't know what's causing the timeout in the CI server, but I've made a change on a hunch that should help: instead of adding IDT_LIST to the end of the memory space, find an unmapped region and put it there.

This shouldn't be an issue in the first place, but considering that xtensa binaries are huge, it seems that they're not being generated sparsely as they should.

andyross

FWIW: this series still rebases cleanly on master, appears to break nothing, and from what minimal testing I've done seems to work just fine.

It's true that as Anas has mentioned there's a bug in the pre-existing timer driver that causes the timer interrupts to arrive 4x slower than they should, but that isn't related to this patch series at all (you can see it just fine in master -- e.g. with a k_sleep()+printk() loop) and is already tracked in Jira.

I say we should merge these and handle any bugs as they're reported. The new drivers definitely seem not to introduce regressions.

lpereira requested a review from andrewboie July 7, 2017 20:40

lpereira force-pushed the esp32 branch from 469cc67 to 3e7139c Compare July 7, 2017 21:05

andrewboie reviewed Jul 7, 2017

View reviewed changes

lpereira force-pushed the esp32 branch from 3e7139c to b7b9aee Compare July 10, 2017 22:00

lpereira force-pushed the esp32 branch 3 times, most recently from cfdea59 to 709f900 Compare July 19, 2017 21:09

andrewboie suggested changes Jul 19, 2017

View reviewed changes

lpereira commented Jul 19, 2017

View reviewed changes

lpereira force-pushed the esp32 branch from 709f900 to dd013e3 Compare July 19, 2017 23:22

lpereira force-pushed the esp32 branch 3 times, most recently from 1617f98 to 4a953cb Compare July 24, 2017 22:05

andrewboie previously approved these changes Jul 24, 2017

View reviewed changes

lpereira dismissed andrewboie’s stale review via 1091ca5 July 26, 2017 21:04

lpereira force-pushed the esp32 branch 3 times, most recently from 37cfa10 to ca7b4ba Compare July 28, 2017 21:54

lpereira added 9 commits July 31, 2017 10:41

pinmux: Allow up to 6 functions

662377a

According to the "ESP32 Technical Reference Manual", the ESP32 SoC series supports up to 6 functions per GPIO pin. Add PINMUX_FUNC_E and PINMUX_FUNC_F. Jira: ZEP-2297 Signed-off-by: Leandro Pereira <leandro.pereira@intel.com>

drivers: pinmux: Add pin multiplexer driver for ESP32

a3f1c7b

This implements a driver for the pin multiplexer as present in the ESP32 SoCs. All APIs are supported. Jira: ZEP-2297 Signed-off-by: Leandro Pereira <leandro.pereira@intel.com>

drivers: gpio: esp32: Add ESP32 GPIO driver

520b0c8

This provides basic GPIO support, with interrupts, and the ability to read and write to ports on a pin-by-pin basis. Jira: ZEP-2286 Signed-off-by: Leandro Pereira <leandro.pereira@intel.com>

samples: drivers: gpio: Make the sample work with ESP32

a8c885e

Signed-off-by: Leandro Pereira <leandro.pereira@intel.com>

arch: xtensa: Remove mentions of dynamic interrupt settings

7062e7f

Dynamic IRQ allocation has been yanked from Zephyr a few releases ago, so there's no point in keeping these options available. Signed-off-by: Leandro Pereira <leandro.pereira@intel.com>

arch: xtensa: Move exception table to xtensa_intr.c

625b6d6

This cleans up the exception handling by removing the table declaration from xtensa_intr_asm.S, and removing the unused _xt_set_exception_handler() function. Signed-off-by: Leandro Pereira <leandro.pereira@intel.com>

lpereira force-pushed the esp32 branch from ca7b4ba to 625b6d6 Compare July 31, 2017 17:41

andyross approved these changes Aug 9, 2017

View reviewed changes

andrewboie approved these changes Aug 9, 2017

View reviewed changes

andrewboie merged commit 99181eb into zephyrproject-rtos:master Aug 9, 2017

This was referenced Sep 23, 2017

ESP32: watchdog driver #3731

Closed

ESP32: Pin mux driver #3732

Closed

ESP32: Random number generator #3788

Closed

nagineni pushed a commit to nagineni/zephyr that referenced this pull request Nov 20, 2017

[tests] Add tests for Console APIs (zephyrproject-rtos#724)

06004c9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ESP32: Add pinmux, GPIO, watchdog, RNG drivers; begin cleaning up ISR tables #724

ESP32: Add pinmux, GPIO, watchdog, RNG drivers; begin cleaning up ISR tables #724

lpereira commented Jul 7, 2017

andrewboie Jul 7, 2017

lpereira Jul 7, 2017 •

edited

Loading

lpereira Jul 7, 2017

andrewboie Jul 7, 2017

lpereira Jul 7, 2017

andrewboie Jul 7, 2017

lpereira commented Jul 19, 2017

andrewboie left a comment

andrewboie Jul 19, 2017

lpereira Jul 19, 2017

andrewboie Jul 19, 2017

lpereira Jul 19, 2017 •

edited

Loading

lpereira Jul 19, 2017

lpereira commented Jul 19, 2017

lpereira commented Jul 31, 2017

andyross left a comment

ESP32: Add pinmux, GPIO, watchdog, RNG drivers; begin cleaning up ISR tables #724

ESP32: Add pinmux, GPIO, watchdog, RNG drivers; begin cleaning up ISR tables #724

Conversation

lpereira commented Jul 7, 2017

Choose a reason for hiding this comment

lpereira Jul 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpereira commented Jul 19, 2017

andrewboie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpereira Jul 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpereira commented Jul 19, 2017

lpereira commented Jul 31, 2017

andyross left a comment

Choose a reason for hiding this comment

lpereira Jul 7, 2017 •

edited

Loading

lpereira Jul 19, 2017 •

edited

Loading