arch/risc-v/espressif: Add full GPIO support

[lucasssvaz]

Summary

Add full GPIO and Buttons support for ESP32-C3/C6/H2 using the Espressif HAL.

Impact

None on existing code. Support for new peripherals when using the Espressif HAL.

Testing

Tested on ESP32-C3-DevKitC.

[gustavonihei]

GPIO interrupts used to be enabled by a specific Kconfig option, disabled by default:

nuttx/arch/xtensa/src/esp32/Kconfig

Lines 811 to 815 in 5e7ce13

	config ESP32_GPIO_IRQ
	bool "GPIO pin interrupts"
	default n
	---help---
	Enable support for interrupting GPIO pins

This PR seems to change this behavior. Please confirm whether this is indeed an intended move.

[lucasssvaz]

GPIO interrupts used to be enabled by a specific Kconfig option, disabled by default:

nuttx/arch/xtensa/src/esp32/Kconfig

Lines 811 to 815 in 5e7ce13

	config ESP32_GPIO_IRQ
	bool "GPIO pin interrupts"
	default n
	---help---
	Enable support for interrupting GPIO pins

This PR seems to change this behavior. Please confirm whether this is indeed an intended move.

Yes, it is. From now on we will follow the same approach as the other chips/boards.

[gustavonihei]

GPIO interrupts used to be enabled by a specific Kconfig option, disabled by default:

nuttx/arch/xtensa/src/esp32/Kconfig

Lines 811 to 815 in 5e7ce13

	config ESP32_GPIO_IRQ
	bool "GPIO pin interrupts"
	default n
	---help---
	Enable support for interrupting GPIO pins

This PR seems to change this behavior. Please confirm whether this is indeed an intended move.

Yes, it is. From now on we will follow the same approach as the other chips/boards.

Ok, but besides that is there any stronger reason for doing so?
I believe the initial motivation for having it disabled by default is to avoid the unnecessary memory overhead if GPIO interrupts are not required.
Enabling GPIO interrupts directly affects NR_IRQS, which is used as the size of the NuttX interrupt vector table:

nuttx/sched/irq/irq_initialize.c

Line 52 in 5e7ce13

struct irq_info_s g_irqvector[NR_IRQS];

nuttx/sched/irq/irq.h

Lines 57 to 71 in 5e7ce13

	struct irq_info_s
	{
	xcpt_t handler; /* Address of the interrupt handler */
	FAR void *arg; /* The argument provided to the interrupt handler. */
	#ifdef CONFIG_SCHED_IRQMONITOR
	clock_t start; /* Time interrupt attached */
	#ifdef CONFIG_HAVE_LONG_LONG
	uint64_t count; /* Number of interrupts on this IRQ */
	#else
	uint32_t mscount; /* Number of interrupts on this IRQ (MS) */
	uint32_t lscount; /* Number of interrupts on this IRQ (LS) */
	#endif
	uint32_t time; /* Maximum execution time on this IRQ */
	#endif
	};

So, considering that ESP32-C3 contains 22 IRQ-capable pins, this move may increase RAM usage by at least 176 bytes.

[mu578]

besides the RAM overhead (2023).

// that's true overhead for nothing ->  (should be chased everywhere)
- gpioinfo("Attach %p\n", irqhandler);
- gpioinfo("Enabling the interrupt\n");

+ gpioinfo(
+     "Attach %p\n"
+     "Enabling the interrupt\n"
+   , irqhandler
+ );

[lucasssvaz]

@gustavonihei Me, @tmedicci and @acassis debated about this and decided that there weren't many applications where disabling the GPIO IRQs would be beneficial. So, for the sake of simplicity and consistency, we decided to remove it.

[tmedicci]

@gustavonihei Me, @tmedicci and @acassis debated about this and decided that there weren't many applications where disabling the GPIO IRQs would be beneficial. So, for the sake of simplicity and consistency, we decided to remove it.

Is there any application on NuttX that use GPIO by polling? Particularly, I don't see any benefits of keeping GPIO support without IRQs.

[acassis]

@gustavonihei I think the "overhead" is too small and normally people will need to use GPIO with interrupt. Another alternative could be to keep the Kconfig entry but enable it by default because normally it will be needed 99% of the time when people are using GPIO.

[gustavonihei]

there weren't many applications where disabling the GPIO IRQs would be beneficial

Is there any application on NuttX that use GPIO by polling? Particularly, I don't see any benefits of keeping GPIO support without IRQs.

normally people will need to use GPIO with interrupt

There is one trivial scenario that you (@tmedicci @lucasssvaz @acassis) seem to ignore is that there are many applications that don't work with the processor pins in general-purpose function at all. Take a look at how many defconfigs define CONFIG_ESP32_GPIO_IRQ=y, just a fraction. A practical example is a gateway device, like the newly release Espressif Thread Border Router board, which most probably won't need any externally generated GPIO interrupts at the simplest form.

I think the "overhead" is too small

At the driver level any overhead should be minimized. It is up to the application developer to quantify the size of the overhead and decide whether the price is worth paying.

[tmedicci]

there weren't many applications where disabling the GPIO IRQs would be beneficial

Is there any application on NuttX that use GPIO by polling? Particularly, I don't see any benefits of keeping GPIO support without IRQs.

normally people will need to use GPIO with interrupt

There is one trivial scenario that you (@tmedicci @lucasssvaz @acassis) seem to ignore is that there are many applications that don't work with the processor pins in general-purpose function at all. Take a look at how many defconfigs define CONFIG_ESP32_GPIO_IRQ=y, just a fraction. A practical example is a gateway device, like the newly release Espressif Thread Border Router board, which most probably won't need any externally generated GPIO interrupts at the simplest form.

I think the "overhead" is too small

At the driver level any overhead should be minimized. It is up to the application developer to quantify the size of the overhead and decide whether the price is worth paying.

You're right about the applications that don't use GPIOs at all... So, let's re-think the problem: considering GPIO is being used, should we provide an option to not use the GPIO with IRQs just for the sake of saving a couple of bytes? Are there any practical applications that could be built relying on that?

I agree that this PR doesn't consider this and should be fixed.

[lucasssvaz]

I've added the toggle back.

[pkarashchenko]

Looks good in general