[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #9586: arch/risc-v/espressif: Add full GPIO support

["pkarashchenko (via GitHub)" <gi...@apache.org>]

pkarashchenko commented on code in PR #9586:
URL: https://github.com/apache/nuttx/pull/9586#discussion_r1239865383


##########
arch/risc-v/src/espressif/esp_gpio.c:
##########
@@ -22,20 +22,33 @@
  * Included Files
  ****************************************************************************/
 
+/* Config */
+
 #include <nuttx/config.h>
 
+/* Libc */
+
 #include <assert.h>
 #include <debug.h>
 #include <stdint.h>
 #include <sys/types.h>
 
+/* NuttX */
+
 #include <arch/irq.h>
 #include <nuttx/arch.h>
 #include <nuttx/irq.h>
 
+/* Arch */
+
 #include "riscv_internal.h"
 
 #include "esp_gpio.h"
+#ifdef CONFIG_ESPRESSIF_GPIO_IRQ
+#include "esp_irq.h"

Review Comment:
   ```suggestion
   #  include "esp_irq.h"
   ```



##########
arch/risc-v/src/espressif/esp_gpio.c:
##########
@@ -49,6 +62,89 @@ static gpio_hal_context_t g_gpio_hal =
   .dev = GPIO_HAL_GET_HW(GPIO_PORT_0)
 };
 
+#ifdef CONFIG_ESPRESSIF_GPIO_IRQ
+static int g_gpio_cpuint;
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: gpio_dispatch
+ *
+ * Description:
+ *   Second level dispatch for GPIO interrupt handling.
+ *
+ * Input Parameters:
+ *   irq           - GPIO IRQ number.
+ *   status        - Value from the GPIO interrupt status clear register.
+ *   regs          - Saved CPU context.
+ *
+ * Returned Value:
+ *   None.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_ESPRESSIF_GPIO_IRQ
+static void gpio_dispatch(int irq, uint32_t status, uint32_t *regs)
+{
+  int i;
+
+  /* Check set bits in the status register */
+
+  while ((i = __builtin_ffs(status)))

Review Comment:
   ```suggestion
     while (i = __builtin_ffs(status))
   ```
   or
   ```suggestion
     while ((i = __builtin_ffs(status)) > 0)
   ```



##########
arch/risc-v/src/espressif/esp_gpio.c:
##########
@@ -49,6 +62,89 @@ static gpio_hal_context_t g_gpio_hal =
   .dev = GPIO_HAL_GET_HW(GPIO_PORT_0)
 };
 
+#ifdef CONFIG_ESPRESSIF_GPIO_IRQ
+static int g_gpio_cpuint;
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: gpio_dispatch
+ *
+ * Description:
+ *   Second level dispatch for GPIO interrupt handling.
+ *
+ * Input Parameters:
+ *   irq           - GPIO IRQ number.
+ *   status        - Value from the GPIO interrupt status clear register.
+ *   regs          - Saved CPU context.
+ *
+ * Returned Value:
+ *   None.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_ESPRESSIF_GPIO_IRQ
+static void gpio_dispatch(int irq, uint32_t status, uint32_t *regs)
+{
+  int i;
+
+  /* Check set bits in the status register */
+
+  while ((i = __builtin_ffs(status)))
+    {
+      irq_dispatch(irq + i - 1, regs);
+      status >>= i;
+    }
+}
+#endif
+
+/****************************************************************************
+ * Name: gpio_interrupt
+ *
+ * Description:
+ *   GPIO interrupt handler.
+ *
+ * Input Parameters:
+ *   irq           - Identifier of the interrupt request.
+ *   context       - Context data from the ISR.
+ *   arg           - Opaque pointer to the internal driver state structure.
+ *
+ * Returned Value:
+ *   Zero (OK) is returned on success; a negated errno value is returned
+ *   on failure.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_ESPRESSIF_GPIO_IRQ
+static int gpio_interrupt(int irq, void *context, void *arg)
+{
+  int i;
+  uint32_t status;
+  uint32_t intr_bitmask;
+  int cpu = up_cpu_index();
+
+  /* Read the lower GPIO interrupt status */
+
+  gpio_hal_get_intr_status(&g_gpio_hal, cpu, &status);
+  intr_bitmask = status;
+
+  while ((i = __builtin_ffs(intr_bitmask)))

Review Comment:
   ditto



##########
arch/risc-v/src/espressif/esp_gpio.c:
##########
@@ -203,3 +299,163 @@ void esp_gpio_matrix_out(uint32_t pin, uint32_t signal_idx, bool out_inv,
   esp_rom_gpio_connect_out_signal(pin, signal_idx, out_inv, oen_inv);
 }
 
+/****************************************************************************
+ * Name: esp_gpiowrite
+ *
+ * Description:
+ *   Write one or zero to the selected GPIO pin
+ *
+ * Input Parameters:
+ *   pin           - GPIO pin to be modified.
+ *   value         - The value to be written (0 or 1).
+ *
+ * Returned Value:
+ *   None.
+ *
+ ****************************************************************************/
+
+void esp_gpiowrite(int pin, bool value)
+{
+  DEBUGASSERT(pin >= 0 && pin <= SOC_GPIO_PIN_COUNT);
+
+  gpio_hal_set_level(&g_gpio_hal, pin, value);
+}
+
+/****************************************************************************
+ * Name: esp_gpioread
+ *
+ * Description:
+ *   Read one or zero from the selected GPIO pin
+ *
+ * Input Parameters:
+ *   pin           - GPIO pin to be read.
+ *
+ * Returned Value:
+ *   The boolean representation of the input value (true/false).
+ *
+ ****************************************************************************/
+
+bool esp_gpioread(int pin)
+{
+  DEBUGASSERT(pin >= 0 && pin <= SOC_GPIO_PIN_COUNT);
+
+  return (bool)gpio_hal_get_level(&g_gpio_hal, pin);

Review Comment:
   Minor, optional
   ```suggestion
     return gpio_hal_get_level(&g_gpio_hal, pin) != 0;
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org